20 Oct, 2016

4 commits

  • This improves AEN handler for Host Network Controller Driver Status
    Change (HNCDSC):

    * The channel's lock should be hold when accessing its state.
    * Do failover when host driver isn't ready.
    * Configure channel when host driver becomes ready.

    Signed-off-by: Gavin Shan
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • The issue was found on BCM5718 which has two NCSI channels in one
    package: C0 and C1. C0 is in link-up state while C1 is in link-down
    state. C0 is chosen as active channel until unplugging and plugging
    C0's cable: On unplugging C0's cable, LSC (Link State Change) AEN
    packet received on C0 to report link-down event. After that, C1 is
    chosen as active channel. LSC AEN for link-up event is lost on C0
    when plugging C0's cable back. We lose the network even C0 is usable.

    This resolves the issue by recording the (hot) channel that was ever
    chosen as active one. The hot channel is chosen to be active one
    if none of available channels in link-up state. With this, C0 is still
    the active one after unplugging C0's cable. LSC AEN packet received
    on C0 when plugging its cable back.

    Signed-off-by: Gavin Shan
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • The issue was found on BCM5718 which has two NCSI channels in one
    package: C0 and C1. Both of them are connected to different LANs,
    means they are in link-up state and C0 is chosen as the active one
    until resetting BCM5718 happens as below.

    Resetting BCM5718 results in LSC (Link State Change) AEN packet
    received on C0, meaning LSC AEN is missed on C1. When LSC AEN packet
    received on C0 to report link-down, it fails over to C1 because C1
    is in link-up state as software can see. However, C1 is in link-down
    state in hardware. It means the link state is out of synchronization
    between hardware and software, resulting in inappropriate channel (C1)
    selected as active one.

    This resolves the issue by sending separate GLS (Get Link Status)
    commands to all channels in the package before trying to do failover.
    The last link states of all channels in the package are retrieved.
    With it, C0 (not C1) is selected as active one as expected.

    Signed-off-by: Gavin Shan
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • There are several if/else statements in the state machine implemented
    by switch/case in ncsi_suspend_channel() to avoid duplicated code. It
    makes the code a bit hard to be understood.

    This drops if/else statements in ncsi_suspend_channel() to improve the
    code readability as Joel Stanley suggested. Also, it becomes easy to
    add more states in the state machine without affecting current code.
    No logical changes introduced by this.

    Suggested-by: Joel Stanley
    Signed-off-by: Gavin Shan
    Signed-off-by: David S. Miller

    Gavin Shan
     

04 Oct, 2016

7 commits

  • This introduces ncsi_stop_dev(), as counterpart to ncsi_start_dev(),
    to stop the NCSI device so that it can be reenabled in future. This
    API should be called when the network device driver is going to
    shutdown the device. There are 3 things done in the function: Stop
    the channel monitoring; Reset channels to inactive state; Report
    NCSI link down.

    Signed-off-by: Gavin Shan
    Reviewed-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • The original NCSI channel monitoring was implemented based on a
    backoff algorithm: the GLS response should be received in the
    specified interval. Otherwise, the channel is regarded as dead
    and failover should be taken if current channel is an active one.
    There are several problems in the implementation: (A) On BCM5718,
    we found when the IID (Instance ID) in the GLS command packet
    changes from 255 to 1, the response corresponding to IID#1 never
    comes in. It means we cannot make the unfair judgement that the
    channel is dead when one response is missed. (B) The code's
    readability should be improved. (C) We should do failover when
    current channel is active one and the channel monitoring should
    be marked as disabled before doing failover.

    This reworks the channel monitoring to address all above issues.
    The fields for channel monitoring is put into separate struct
    and the state of channel monitoring is predefined. The channel
    is regarded alive if the network controller responses to one of
    two GLS commands or both of them in 5 seconds.

    Signed-off-by: Gavin Shan
    Reviewed-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • There is only one NCSI request property for now: the response for
    the sent command need drive the workqueue or not. So we had one
    field (@driven) for the purpose. We lost the flexibility to extend
    NCSI request properties.

    This replaces @driven with @flags and @req_flags in NCSI request
    and NCSI command argument struct. Each bit of the newly introduced
    field can be used for one property. No functional changes introduced.

    Signed-off-by: Gavin Shan
    Reviewed-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • The NCSI request index (struct ncsi_request::id) is put into instance
    ID (IID) field while sending NCSI command packet. It was designed the
    available IDs are given in round-robin fashion. @ndp->request_id was
    introduced to represent the next available ID, but it has been used
    as number of successively allocated IDs. It breaks the round-robin
    design. Besides, we shouldn't put 0 to NCSI command packet's IID
    field, meaning ID#0 should be reserved according section 6.3.1.1
    in NCSI spec (v1.1.0).

    This fixes above two issues. With it applied, the available IDs will
    be assigned in round-robin fashion and ID#0 won't be assigned.

    Signed-off-by: Gavin Shan
    Reviewed-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • We needn't send CIS (Clear Initial State) command to the NCSI
    reserved channel (0x1f) in the enumeration. We shouldn't receive
    a valid response from CIS on NCSI channel 0x1f.

    Signed-off-by: Gavin Shan
    Reviewed-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • This defines NCSI_RESERVED_CHANNEL as the reserved NCSI channel
    ID (0x1f). No logical changes introduced.

    Signed-off-by: Gavin Shan
    Reviewed-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • xchg() is used to set NCSI channel's state in order for consistent
    access to the state. xchg()'s return value should be used. Otherwise,
    one build warning will be raised (with -Wunused-value) as below message
    indicates. It is reported by ia64-linux-gcc (GCC) 4.9.0.

    net/ncsi/ncsi-manage.c: In function 'ncsi_channel_monitor':
    arch/ia64/include/uapi/asm/cmpxchg.h:56:2: warning: value computed is \
    not used [-Wunused-value]
    ((__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr))))
    ^
    net/ncsi/ncsi-manage.c:202:3: note: in expansion of macro 'xchg'
    xchg(&nc->state, NCSI_CHANNEL_INACTIVE);

    This removes the atomic access to NCSI channel's state avoid the above
    build warning. We have to hold the channel's lock when its state is readed
    or updated. No functional changes introduced.

    Signed-off-by: Gavin Shan
    Reviewed-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     

26 Jul, 2016

1 commit

  • gcc-4.9 and higher warn about the newly added NSCI code:

    net/ncsi/ncsi-manage.c: In function 'ncsi_process_next_channel':
    net/ncsi/ncsi-manage.c:1003:2: error: 'old_state' may be used uninitialized in this function [-Werror=maybe-uninitialized]

    The warning is a false positive and therefore harmless, but it would be good to
    avoid it anyway. I have determined that the barrier in the spin_unlock_irqsave()
    is what confuses gcc to the point that it cannot track whether the variable
    was unused or not.

    This rearranges the code in a way that makes it obvious to gcc that old_state
    is always initialized at the time of use, functionally this should not
    change anything.

    Signed-off-by: Arnd Bergmann
    Acked-by: Gavin Shan
    Signed-off-by: David S. Miller

    Arnd Bergmann
     

20 Jul, 2016

5 commits

  • This introduces NCSI AEN packet handlers that result in (A) the
    currently active channel is reconfigured; (B) Currently active
    channel is deconfigured and disabled, another channel is chosen
    as active one and configured. Case (B) won't happen if hardware
    arbitration has been enabled, the channel that was in active
    state is suspended simply.

    Signed-off-by: Gavin Shan
    Acked-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • This manages NCSI packages and channels:

    * The available packages and channels are enumerated in the first
    time of calling ncsi_start_dev(). The channels' capabilities are
    probed in the meanwhile. The NCSI network topology won't change
    until the NCSI device is destroyed.
    * There in a queue in every NCSI device. The element in the queue,
    channel, is waiting for configuration (bringup) or suspending
    (teardown). The channel's state (inactive/active) indicates the
    futher action (configuration or suspending) will be applied on the
    channel. Another channel's state (invisible) means the requested
    action is being applied.
    * The hardware arbitration will be enabled if all available packages
    and channels support it. All available channels try to provide
    service when hardware arbitration is enabled. Otherwise, one channel
    is selected as the active one at once.
    * When channel is in active state, meaning it's providing service, a
    timer started to retrieve the channe's link status. If the channel's
    link status fails to be updated in the determined period, the channel
    is going to be reconfigured. It's the error handling implementation
    as defined in NCSI spec.

    Signed-off-by: Gavin Shan
    Acked-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • The NCSI response packets are sent to MC (Management Controller)
    from the remote end. They are responses of NCSI command packets
    for multiple purposes: completion status of NCSI command packets,
    return NCSI channel's capability or configuration etc.

    This defines struct to represent NCSI response packets and introduces
    function ncsi_rcv_rsp() which will be used to receive NCSI response
    packets and parse them.

    Signed-off-by: Gavin Shan
    Acked-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • The NCSI command packets are sent from MC (Management Controller)
    to remote end. They are used for multiple purposes: probe existing
    NCSI package/channel, retrieve NCSI channel's capability, configure
    NCSI channel etc.

    This defines struct to represent NCSI command packets and introduces
    function ncsi_xmit_cmd(), which will be used to transmit NCSI command
    packet according to the request. The request is represented by struct
    ncsi_cmd_arg.

    Signed-off-by: Gavin Shan
    Acked-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan
     
  • NCSI spec (DSP0222) defines several objects: package, channel, mode,
    filter, version and statistics etc. This introduces the data structs
    to represent those objects and implement functions to manage them.
    Also, this introduces CONFIG_NET_NCSI for the newly implemented NCSI
    stack.

    * The user (e.g. netdev driver) dereference NCSI device by
    "struct ncsi_dev", which is embedded to "struct ncsi_dev_priv".
    The later one is used by NCSI stack internally.
    * Every NCSI device can have multiple packages simultaneously, up
    to 8 packages. It's represented by "struct ncsi_package" and
    identified by 3-bits ID.
    * Every NCSI package can have multiple channels, up to 32. It's
    represented by "struct ncsi_channel" and identified by 5-bits ID.
    * Every NCSI channel has version, statistics, various modes and
    filters. They are represented by "struct ncsi_channel_version",
    "struct ncsi_channel_stats", "struct ncsi_channel_mode" and
    "struct ncsi_channel_filter" separately.
    * Apart from AEN (Asynchronous Event Notification), the NCSI stack
    works in terms of command and response. This introduces "struct
    ncsi_req" to represent a complete NCSI transaction made of NCSI
    request and response.

    link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.1.0.pdf
    Signed-off-by: Gavin Shan
    Acked-by: Joel Stanley
    Signed-off-by: David S. Miller

    Gavin Shan