Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[voq/inbandif] Support for inband port as regular port #6477

Merged
merged 4 commits into from Apr 1, 2021
Merged

[voq/inbandif] Support for inband port as regular port #6477

merged 4 commits into from Apr 1, 2021

Conversation

vganesan-nokia
Copy link
Contributor

- Why I did it

Inband port can be made available in PORT table. But regular port handlngs are
not applicable for Inband port.

Changes in this PR are to make LLDP to consider Inband port and to avoid regular
port handling on Inband port.

Currently the Inband interface is used for VOQ chassis systems for asic to asic communications.
Ref: VOQ HLD https://github.com/Azure/SONiC/blob/master/doc/voq/voq_hld.md

- How I did it

  • Changes done in supervisor conf for LLDP to include Inband in the interface list
  • Changes done in platform common files to handle Inand port similar to how backplane port is handled

- How to verify it

  • Expose a port with name prefixed with "Inband" to SONiC via port_config.ini
  • Have an entry in the PORT table for this port with name "Inband"
  • When the system is booted, there should not be errors related to Inband interface
  • test_virtual_chassis.py which configures a front panel port as inband interface pass

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

This was referenced Jan 18, 2021
Merged
Merged
Merged
@anshuv-mfst
Copy link

@eswaran (Arista) - could you please review the PR, thanks.

@anshuv-mfst
Copy link

@eswaran (Arista), @abdosi - could you please review the PR, thanks.

mlorrillere
mlorrillere reviewed Feb 3, 2021
View reviewed changes
dockers/docker-lldp/lldpmgrd Outdated
@@ -94,6 +94,11 @@ class LldpManager(daemon_base.DaemonBase):
"""
port_desc = None

# Skip port name prefixed with "Inband". These are recycle ports exposed in PORT_TABLE for
# asic-to-asic communication in VOQ based chassis system. We do not configure LLDP on these.
if 'Inband' in port_name:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be port_name.startswith('Inband') to match only on prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per Recycle port HLD PR discussion, it was recommended to use name starting with Ethernet. For inband interface, it will be Ethernet-Inband0. For recirc port it will be Ethernet-Recirc0. I'll change this to port_name.startswith("Ethernet-Inband") or use the inband_prefix() as recommended in next comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

mlorrillere
mlorrillere reviewed Feb 3, 2021
View reviewed changes
dockers/docker-lldp/lldpmgrd Outdated
@@ -94,6 +94,11 @@ class LldpManager(daemon_base.DaemonBase):
"""
port_desc = None

# Skip port name prefixed with "Inband". These are recycle ports exposed in PORT_TABLE for
# asic-to-asic communication in VOQ based chassis system. We do not configure LLDP on these.
if 'Inband' in port_name:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use inband_prefix() from sonic_py_common/interface.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I'll fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

vedganes added 4 commits February 17, 2021 14:24
[voqinband]Support for inband port as regular port
b87dba8 
Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>

Inband port is avaialable in PORT table. But regular port handlings are
not applicable for Inband port.
Changes are to make lldp to consider Inband port and to avoid regular
port handling on Inband port.
[inbandif]Inband if support - code review comments fix -1
68eecee 
Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>

Fixed code review comments
[inbandif] Avoid lldp on inband interface
d9e446f 
Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>

Changes to avoid LLDP config on Inband interface. Since port type inband
interfaces are recycle ports (or may normal ports) used for asic-to-asic
communication, LLDP is not configured on these interfaces.
[voq/inband]Code review comments fix - 2
b513a1e 
Signed-off-by: vedganes <vedavinayagam.ganesan@nokia.com>

(1) Based on review comments Recyle port HLD
(sonic-net/SONiC#742) the inband port name prefix is
changed from Inband to "Ethernet-IB" similar to what we have for
Ethernet-BP (Ethernet-Backplane) in multi-asic design. Changes are done
in interface.py to handle this changed name
(2) Code review comments fix for lldpmgrd
@vganesan-nokia
Copy link
Contributor Author

/Azurepipelines run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 6477 in repo Azure/sonic-buildimage

@abdosi
Copy link
Contributor

abdosi commented Mar 7, 2021

/Azurepipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vganesan-nokia
Copy link
Contributor Author

@abdosi, thanks for starting the Azurepipeline runs. Would you please complete review and approve so that we can merge? The PRs sonic-net/sonic-platform-common#159, sonic-net/sonic-platform-daemons#145 and sonic-net/sonic-utilities#1363 which you have already approved depend on this PR for build success.

@abdosi abdosi merged commit 973affc into sonic-net:master Apr 1, 2021
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-buildimage that referenced this pull request May 23, 2021
@vganesan-nokia @raphaelt-nvidia
[voq/inbandif] Support for inband port as regular port (sonic-net#6477)
9bce4fc 
Changes in this PR are to make LLDP to consider Inband port and to avoid regular
port handling on Inband port.
@ysmanman ysmanman mentioned this pull request Jun 23, 2021
Merged
4 tasks
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
@vganesan-nokia
[voq/inbandif] Support for inband port as regular port (sonic-net#6477)
75c1efa 
Changes in this PR are to make LLDP to consider Inband port and to avoid regular
port handling on Inband port.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mlorrillere mlorrillere mlorrillere approved these changes

@abdosi abdosi abdosi approved these changes

@lguohan lguohan Awaiting requested review from lguohan lguohan is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@vganesan-nokia @anshuv-mfst @abdosi @mlorrillere