Add interface link-training command into the CLI doc

[ds952811]

• What I did Add interface link-training command into the CLI doc

• How I did it Update Command-Reference.md

• How to verify it N/A

Signed-off-by: Dante Su [email protected]

What I did

How I did it

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

[adyeung]

@prgeor with all the link training code enhancements merged, pls review and merge the cmd ref

[prgeor]

@dgsudharsan can you review?

[ds952811]

The '-' is observed only in 'LT Admin', not in 'LT Oper' LT Admin: The user admin config, it could be '-' or 'on' or 'off', where '-' means 'not admin configured' LT Oper: The hardware operational state, it could be either 'on' or 'off' Hence we don't need 'N/A'.

Please note LT operational state is either ON or OFF, from a hardware perspective, tri-state is not applicable, and the link down is expected if either local or remote on a link with a different LT operational state.

i.e. LT off + LT off --> This is the default/generic behavior for platforms with or without LT support. LT on + LT off --> Link down is expected, as it gets stuck in not_trained state, and most likely failed with frame_lockup error LT off + LT on --> Link down is expected, as it gets stuck in not_trained state, and most likely failed with frame_lockup error LT on + LT on --> Link comes up when the link is trained, otherwise, it's down and further status could be available via SAI_PORT_ATTR_LINK_TRAINING_FAILURE_STATUS

On the other hand, on a platform with LT enabled without SONiC acknowledgment or even without SAI get attribute support, the possible solution is as follows:

1. Update PortsOrch::initializePort() to publish the actual hardware operational state into STATE_DB via SAI_PORT_ATTR_LINK_TRAINING_RX_STATUS and SAI_PORT_ATTR_LINK_TRAINING_ENABLE
2. Update PortsOrch::initializePort() to publish the platform-specific hard coded LT operational state into STATE_DB
3. Update the vendor-specific SDK config (e.g. config.bcm in the Broadcom chips) to have LT disabled by default

[dgsudharsan]

The '-' is observed only in 'LT Admin', not in 'LT Oper' LT Admin: The user admin config, it could be '-' or 'on' or 'off', where '-' means 'not admin configured' LT Oper: The hardware operational state, it could be either 'on' or 'off' Hence we don't need 'N/A'.

Please note LT operational state is either ON or OFF, from a hardware perspective, tri-state is not applicable, and the link down is expected if either local or remote on a link with a different LT operational state.

i.e. LT off + LT off --> This is the default/generic behavior for platforms with or without LT support. LT on + LT off --> Link down is expected, as it gets stuck in not_trained state, and most likely failed with frame_lockup error LT off + LT on --> Link down is expected, as it gets stuck in not_trained state, and most likely failed with frame_lockup error LT on + LT on --> Link comes up when the link is trained, otherwise, it's down and further status could be available via SAI_PORT_ATTR_LINK_TRAINING_FAILURE_STATUS

On the other hand, on a platform with LT enabled without SONiC acknowledgment or even without SAI get attribute support, the possible solution is as follows:

1. Update PortsOrch::initializePort() to publish the actual hardware operational state into STATE_DB via SAI_PORT_ATTR_LINK_TRAINING_RX_STATUS and SAI_PORT_ATTR_LINK_TRAINING_ENABLE
2. Update PortsOrch::initializePort() to publish the platform-specific hard coded LT operational state into STATE_DB
3. Update the vendor-specific SDK config (e.g. config.bcm in the Broadcom chips) to have LT disabled by default

I don't necessarily agree that '-' needs to be displayed for LT Admin platforms without support. I also don't agree that when support is not present you cannot display LT oper as off. If a specific vendor SAI doesn't support an attribute what we do today is display show as 'N/A' which means not available. We should display LT admin and LT oper as 'N/A' which means those status are not available to be queried from the SAI and hardware. N/A doesn't imply it is enabled or disabled. It simply means OS (SONiC) couldn't query the current state because SAI doesn't support.

[ds952811]

The '-' is observed only in 'LT Admin', not in 'LT Oper' LT Admin: The user admin config, it could be '-' or 'on' or 'off', where '-' means 'not admin configured' LT Oper: The hardware operational state, it could be either 'on' or 'off' Hence we don't need 'N/A'. Please note LT operational state is either ON or OFF, from a hardware perspective, tri-state is not applicable, and the link down is expected if either local or remote on a link with a different LT operational state. i.e. LT off + LT off --> This is the default/generic behavior for platforms with or without LT support. LT on + LT off --> Link down is expected, as it gets stuck in not_trained state, and most likely failed with frame_lockup error LT off + LT on --> Link down is expected, as it gets stuck in not_trained state, and most likely failed with frame_lockup error LT on + LT on --> Link comes up when the link is trained, otherwise, it's down and further status could be available via SAI_PORT_ATTR_LINK_TRAINING_FAILURE_STATUS On the other hand, on a platform with LT enabled without SONiC acknowledgment or even without SAI get attribute support, the possible solution is as follows:

1. Update PortsOrch::initializePort() to publish the actual hardware operational state into STATE_DB via SAI_PORT_ATTR_LINK_TRAINING_RX_STATUS and SAI_PORT_ATTR_LINK_TRAINING_ENABLE
2. Update PortsOrch::initializePort() to publish the platform-specific hard coded LT operational state into STATE_DB
3. Update the vendor-specific SDK config (e.g. config.bcm in the Broadcom chips) to have LT disabled by default

I don't necessarily agree that '-' needs to be displayed for LT Admin platforms without support. I also don't agree that when support is not present you cannot display LT oper as off. If a specific vendor SAI doesn't support an attribute what we do today is display show as 'N/A' which means not available. We should display LT admin and LT oper as 'N/A' which means those status are not available to be queried from the SAI and hardware. N/A doesn't imply it is enabled or disabled. It simply means OS (SONiC) couldn't query the current state because SAI doesn't support.

In the case of 'LT Admin', it's okay to replace '-' with 'N/A' to indicate a missing user admin config, but it should not be messed up with LT support availability in the SAI library. A dedicated show command for the port features is a better choice, and it's better to create a dedicated PR for this.

In the case of 'LT Oper', while it's not a good idea to use 'N/A' as it's less useful when debugging the link failure, but yes, it's also a bad idea to assume 'LT Oper' is OFF when the state is unavailable in STATE_DB due to the missing SAI support, it's not accurate for chips without the feature support in the SAI .

Honestly, the same issue is applicable to AutoNeg as well, while it could be enabled by default in the vendor-specific SDK config, the SONiC does not fetch the actual hardware operational state when initializing the port object, and the current implementation actually assumes both AN and LT are disabled by default, and could only be configurable via SONiC configuration system.

Again, it's better to review these out-of-bound configurations for SONiC in a dedicated PR for features that are optionally available and could be enabled outside SONiC configuration system.

[ds952811]

I double-checked the changes in this PR against sonic-utilities, it does not mention the possible values for 'LT Oper'. On the other hand, such information should be introduced into the HLD below

https://github.com/sonic-net/SONiC/pull/925/files#diff-116bb65f69b25d13cb802e3e6e41d8260fb79aa67c4f5b4cf69230a7b814aa1bR185

Hence could we have this CLI command manual merged, and then move on the enhancement for the chips without SAI attribute support or out-of-bound SDK configurtation? Thanks.

[ds952811]

I think again, and you're right, we could have 'N/A' for both LT Oper/Admin before moving on the enhacnement, as it's in the same repo. with this doc.

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: ds952811 / name: Dante (Kuo-Jung) Su (a6b1aa3ad08b3461fd11ed8aede59a9974911029, 9fcf20ca22a3bd00c547cab5e287040b778c3a55)

[ds952811]

@prgeor Excuse me, but do you have a chance to review the current changeset? Thanks you

[dgsudharsan]

@ds952811 Can you please resolve conflicts?

[ds952811]

@ds952811 Can you please resolve conflicts?

Sorry for the late update, it's ready now