Skip to content

[LLDP] Fix for LLDP advertisement, PortID and Description#5698

Closed
shlomibitton wants to merge 1 commit intosonic-net:masterfrom
shlomibitton:shlomi_lldp_port_id_fix
Closed

[LLDP] Fix for LLDP advertisement, PortID and Description#5698
shlomibitton wants to merge 1 commit intosonic-net:masterfrom
shlomibitton:shlomi_lldp_port_id_fix

Conversation

@shlomibitton
Copy link
Contributor

@shlomibitton shlomibitton commented Oct 22, 2020

Signed-off-by: Shlomi Bitton shlomibi@nvidia.com

- Why I did it
PortID should reference to the interface name.
Port Description should reference to the interface alias.

- How I did it
Edit 'lldpmgrd' to configure 'lldpd' using 'lldpcli' with the correct values.

- How to verify it
Run tcpdump and observe LLDP packets.

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

  • 201811
  • 201911
  • 202006

- Description for the changelog

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

Fix for LLDP advertisement, PortID should reference interface name, P…
aeef42b 
…ortDescription should reference interface alias

Signed-off-by: Shlomi Bitton <shlomibi@nvidia.com>
@abdosi
Copy link
Contributor

abdosi commented Oct 22, 2020

@shlomibitton can you paste tcpdump o/p for before vs after change

@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Oct 26, 2020

retest vsimage please

@keboliu
Copy link
Collaborator

keboliu commented Oct 27, 2020

@shlomibitton seems that vs test failed at test_lldp_neighbor test, maybe you need to update the test case itself.

@shlomibitton shlomibitton mentioned this pull request Oct 27, 2020
Merged
3 tasks
@shlomibitton
Copy link
Contributor Author

shlomibitton commented Oct 27, 2020

PR dependency on sonic-net/sonic-mgmt#2413

@shlomibitton
Copy link
Contributor Author

shlomibitton commented Oct 28, 2020

@abdosi
tcpdump before:

09:18:01.505804 LLDP, length 202
Chassis ID TLV (1), length 7
Subtype MAC address (4): 0c:42:a1:60:65:d8 (oui Unknown)
0x0000: 040c 42a1 6065 d8
Port ID TLV (2), length 5
Subtype Local (7): etp8
0x0000: 0765 7470 38
Time to Live TLV (3), length 2: TTL 120s
0x0000: 0078
System Name TLV (5), length 10: r-tigon-04
0x0000: 722d 7469 676f 6e2d 3034
System Description TLV (6), length 102
Debian GNU/Linux 9 (stretch) Linux 4.9.0-11-2-amd64 #1 SMP Debian 4.9.189-3+deb9u2 (2019-11-11) x86_64
0x0000: 4465 6269 616e 2047 4e55 2f4c 696e 7578
0x0010: 2039 2028 7374 7265 7463 6829 204c 696e
0x0020: 7578 2034 2e39 2e30 2d31 312d 322d 616d
0x0030: 6436 3420 2331 2053 4d50 2044 6562 6961
0x0040: 6e20 342e 392e 3138 392d 332b 6465 6239
0x0050: 7532 2028 3230 3139 2d31 312d 3131 2920
0x0060: 7838 365f 3634
System Capabilities TLV (7), length 4
System Capabilities [Bridge, WLAN AP, Router, Station Only] (0x009c)
Enabled Capabilities [Bridge, Router] (0x0014)
0x0000: 009c 0014
Management Address TLV (8), length 12
Management Address length 5, AFI IPv4 (1): r-tigon-04.mtr.labs.mlnx
Unknown Interface Numbering (1): 0
0x0000: 0501 0ad2 197b 0100 0000 0000
Port Description TLV (4), length 20: ARISTA13T2:Ethernet2
0x0000: 4152 4953 5441 3133 5432 3a45 7468 6572
0x0010: 6e65 7432
Organization specific TLV (127), length 9: OUI IEEE 802.3 Private (0x00120f)
Link aggregation Subtype (3)
aggregation status [supported], aggregation port ID 0
0x0000: 0012 0f03 0100 0000 00
Organization specific TLV (127), length 9: OUI IEEE 802.3 Private (0x00120f)
MAC/PHY configuration/status Subtype (1)
autonegotiation [none] (0x00)
PMD autoneg capability [unknown] (0x0000)
MAU type Unknown (0x0000)
0x0000: 0012 0f01 0000 0000 00
End TLV (0), length 0

tcpdump after:

09:18:01.505804 LLDP, length 202
Chassis ID TLV (1), length 7
Subtype MAC address (4): 0c:42:a1:60:65:d8 (oui Unknown)
0x0000: 040c 42a1 6065 d8
Port ID TLV (2), length 10
Subtype Local (7): Ethernet56
0x0000: 0765 7470 38
Time to Live TLV (3), length 2: TTL 120s
0x0000: 0078
System Name TLV (5), length 10: r-tigon-04
0x0000: 722d 7469 676f 6e2d 3034
System Description TLV (6), length 102
Debian GNU/Linux 9 (stretch) Linux 4.9.0-11-2-amd64 #1 SMP Debian 4.9.189-3+deb9u2 (2019-11-11) x86_64
0x0000: 4465 6269 616e 2047 4e55 2f4c 696e 7578
0x0010: 2039 2028 7374 7265 7463 6829 204c 696e
0x0020: 7578 2034 2e39 2e30 2d31 312d 322d 616d
0x0030: 6436 3420 2331 2053 4d50 2044 6562 6961
0x0040: 6e20 342e 392e 3138 392d 332b 6465 6239
0x0050: 7532 2028 3230 3139 2d31 312d 3131 2920
0x0060: 7838 365f 3634
System Capabilities TLV (7), length 4
System Capabilities [Bridge, WLAN AP, Router, Station Only] (0x009c)
Enabled Capabilities [Bridge, Router] (0x0014)
0x0000: 009c 0014
Management Address TLV (8), length 12
Management Address length 5, AFI IPv4 (1): r-tigon-04.mtr.labs.mlnx
Unknown Interface Numbering (1): 0
0x0000: 0501 0ad2 197b 0100 0000 0000
Port Description TLV (4), length 4: etp8
0x0000: 4152 4953 5441 3133 5432 3a45 7468 6572
0x0010: 6e65 7432
Organization specific TLV (127), length 9: OUI IEEE 802.3 Private (0x00120f)
Link aggregation Subtype (3)
aggregation status [supported], aggregation port ID 0
0x0000: 0012 0f03 0100 0000 00
Organization specific TLV (127), length 9: OUI IEEE 802.3 Private (0x00120f)
MAC/PHY configuration/status Subtype (1)
autonegotiation [none] (0x00)
PMD autoneg capability [unknown] (0x0000)
MAU type Unknown (0x0000)
0x0000: 0012 0f01 0000 0000 00
End TLV (0), length 0

@shlomibitton
Copy link
Contributor Author

shlomibitton commented Nov 2, 2020 •
edited
Loading

@abdosi This PR will fail the vsimage build unless sonic-net/sonic-mgmt#2413 is merged.
These two PR's are dependent on each other, the vsimage is failing because the test on sonic-mgmt is failing after this change.

@wangxin wangxin mentioned this pull request Nov 3, 2020
Merged
3 tasks
wangxin added a commit to sonic-net/sonic-mgmt that referenced this pull request Nov 3, 2020
@wangxin
Temporarily disable test_lldp.py to unblock mutually dependent LLDP f…
e547a1d 
…ixes (#2451)

PR #2413 and
sonic-net/sonic-buildimage#5698 depends on each other.
If PR#2413 is not merged, PR#5698 would fail PR test and cannot be merged.
If PR#2413 is merged firstly, all sonic-mgmt-pr testing would fail before a new
image with PR#5698 is ready. The workaround is to temporarily disable LLDP
for sonic-mgmt-pr testing. Merge PR#2413 to unblock PR#5698. After a new
image with PR#5698 is ready, then enable LLDP KVM testing again.

Signed-off-by: Xin Wang <xiwang5@microsoft.com>
@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Nov 3, 2020

retest vsimage please

2 similar comments
@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Nov 3, 2020

retest vsimage please

@shlomibitton
Copy link
Contributor Author

shlomibitton commented Nov 3, 2020

retest vsimage please

wangxin pushed a commit to sonic-net/sonic-mgmt that referenced this pull request Nov 3, 2020
@shlomibitton
Fix lldp test to align with bug fix on PR: (#2413)
fd6bf4e 
There is a bug fix on PR sonic-net/sonic-buildimage#5698. The test fail after the change, 
this PR is to align the new change to the test case. The fix is to parse correct keys from 
mg_facts.

Signed-off-by: Shlomi Bitton <shlomibi@nvidia.com>
@keboliu
Copy link
Collaborator

keboliu commented Nov 4, 2020

retest vsimage please

2 similar comments
@keboliu
Copy link
Collaborator

keboliu commented Nov 4, 2020

retest vsimage please

@shlomibitton
Copy link
Contributor Author

shlomibitton commented Nov 5, 2020

retest vsimage please

@keboliu
Copy link
Collaborator

keboliu commented Nov 9, 2020

@abdosi this test passed all the PR check, would you please help to review again?

@liat-grozovik liat-grozovik requested a review from abdosi November 10, 2020 16:37
@abdosi
Copy link
Contributor

abdosi commented Nov 10, 2020

@shlomibitton

I assume this impact the information remote side wll display. Still wanted to confirm follwoing things:

a) Impact on Sonic show lldp table and show lldp neighbors command

Also impact on remote side command . As of now without this change I see below:

Port Neighbor Device ID Neighbor Port ID TTL
Et1 str2-xxx-acs-3 Ethernet1/1 120
Et2 str2-xxx-acs-3 Ethernet1/2 120

abdosi
abdosi requested changes Nov 10, 2020
View reviewed changes
Copy link
Contributor

@abdosi abdosi left a comment

Choose a reason for hiding this comment

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

Posted some queries.

@shlomibitton
Copy link
Contributor Author

shlomibitton commented Nov 11, 2020 •
edited
Loading

@abdosi here is the current output after the change:
SONiC side
show lldp table

Capability codes: (R) Router, (B) Bridge, (O) Other
LocalPort RemoteDevice RemotePortID Capability RemotePortDescr
Ethernet224 ARISTA01T1 Ethernet1 BR
Ethernet232 ARISTA02T1 Ethernet1 BR
Ethernet240 ARISTA03T1 Ethernet1 BR
Ethernet248 ARISTA04T1 Ethernet1 BR
eth0 MTR-F-1-S-LAB-R2-5-SW01 35 B 35
Total entries displayed: 5

show lldp neighbors

Interface: eth0, via: LLDP, RID: 1, Time: 0 day, 00:07:01
Chassis:
ChassisID: mac e0:07:1b:38:21:80
SysName: MTR-F-1-S-LAB-R2-5-SW01
SysDescr: HP J9775A 2530-48G Switch, revision YA.16.09.0003, ROM YA.15.20 (/ws/swbuildm/rel_zootopia_qaoff/code/build/lakes(swbuildm_rel_zootopia_qaoff_rel_zootopia))
TTL: 120
MgmtIP: 10.210.100.63
Capability: Bridge, on
Port:
PortID: local 35
PortDescr: 35
PMD autoneg: supported: yes, enabled: yes
Adv: 10Base-T, HD: yes, FD: yes
Adv: 100Base-TX, HD: yes, FD: yes
Adv: 1000Base-T, HD: no, FD: yes
MAU oper type: 1000BaseTFD - Four-pair Category 5 UTP, full duplex mode
MDI Power: supported: no, enabled: no, pair control: no
Device type: PSE
Power pairs: signal
Class: class 0
VLAN: 260, pvid: yes
Unknown TLVs:
TLV: OUI: 00,16,B9, SubType: 2, Len: 2 00,01

Interface: Ethernet224, via: LLDP, RID: 5, Time: 0 day, 00:06:34
Chassis:
ChassisID: mac 52:54:00:5e:4e:8a
SysName: ARISTA01T1
SysDescr: Arista Networks EOS version 4.20.15M running on an Arista Networks vEOS
TTL: 120
MgmtIP: 10.215.22.62
Capability: Bridge, on
Capability: Router, on
Port:
PortID: ifname Ethernet1
MFS: 9236
Port is aggregated. PortAggregID: 1000001

Interface: Ethernet232, via: LLDP, RID: 4, Time: 0 day, 00:06:35
Chassis:
ChassisID: mac 52:54:00:5b:05:8f
SysName: ARISTA02T1
SysDescr: Arista Networks EOS version 4.20.15M running on an Arista Networks vEOS
TTL: 120
MgmtIP: 10.215.22.63
Capability: Bridge, on
Capability: Router, on
Port:
PortID: ifname Ethernet1
MFS: 9236
Port is aggregated. PortAggregID: 1000001

Interface: Ethernet240, via: LLDP, RID: 2, Time: 0 day, 00:06:39
Chassis:
ChassisID: mac 52:54:00:b6:d7:06
SysName: ARISTA03T1
SysDescr: Arista Networks EOS version 4.20.15M running on an Arista Networks vEOS
TTL: 120
MgmtIP: 10.215.22.64
Capability: Bridge, on
Capability: Router, on
Port:
PortID: ifname Ethernet1
MFS: 9236
Port is aggregated. PortAggregID: 1000001

Interface: Ethernet248, via: LLDP, RID: 3, Time: 0 day, 00:06:36
Chassis:
ChassisID: mac 52:54:00:68:0a:06
SysName: ARISTA04T1
SysDescr: Arista Networks EOS version 4.20.15M running on an Arista Networks vEOS
TTL: 120
MgmtIP: 10.215.22.65
Capability: Bridge, on
Capability: Router, on
Port:
PortID: ifname Ethernet1
MFS: 9236
Port is aggregated. PortAggregID: 1000001

Server ARISTA VM side
show lldp neighbors

Last table change time : 0:16:28 ago
Number of table inserts : 147
Number of table deletes : 146
Number of table drops : 0
Number of table age-outs : 77

Port Neighbor Device ID Neighbor Port ID TTL
Et1 r-leopard-01 Ethernet224 120

@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Nov 17, 2020

@abdosi any further comment/asks?

@abdosi
Copy link
Contributor

abdosi commented Nov 17, 2020

@shlomibitton @liat-grozovik this change does not seem to be backward compatible.

The neighbor device 'show lldp neighbor' o/p will change w.r.t to Interface information.
Not sure we want to take this change

Before

Et1 str2-xxx-acs-3 Ethernet1/1 120

After:

Et1 r-leopard-01 Ethernet224 120

@lguohan can you please provide your comment.

@lguohan
Copy link
Collaborator

lguohan commented Nov 17, 2020

why not backward compatible? what is r-leopard-01 v.s. str2-xxx-acs-3?

@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Nov 17, 2020 via email

@abdosi
Copy link
Contributor

abdosi commented Nov 17, 2020

why not backward compatible? what is r-leopard-01 v.s. str2-xxx-acs-3?

@lguohan : Basically Sonic interface on neighbors is currently alias "Ethernet1/1" and after this change it will be Sonic Name "Ethernet0"

@lguohan
Copy link
Collaborator

lguohan commented Nov 17, 2020

the neighbor should see the alias instead of sonic port name.

@lguohan
Copy link
Collaborator

lguohan commented Nov 17, 2020

retest vsimage please

@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Nov 17, 2020

@shlomibitton by design SONiC uses interface Alias and not interface name. So this fix is not according to the design.
please close this PR and revert the change sonic-mgmt test is aligned and no change for interface name.

abdosi pushed a commit to abdosi/sonic-mgmt that referenced this pull request Nov 27, 2020
Revert "Temporarily disable test_lldp.py to unblock mutually dependen…
2d6434d 
…t LLDP fixes (sonic-net#2451)"

This reverts commit e547a1d.

PR# sonic-net#2413 has already been
reverted.

sonic-net/sonic-buildimage#5698 will not be merged
as this is not valid design.

Signed-off-by: Abhishek Dosi <abdosi@micrsoft.com>
@abdosi abdosi mentioned this pull request Nov 27, 2020
Merged
abdosi added a commit to sonic-net/sonic-mgmt that referenced this pull request Nov 27, 2020
@abdosi
Revert "Temporarily disable test_lldp.py to unblock mutually dependen…
345323c 
…t LLDP fixes (#2451)" (#2595)

This reverts commit e547a1d.

PR# #2413 has already been
reverted.

sonic-net/sonic-buildimage#5698 will not be merged
as this is not valid design.

Signed-off-by: Abhishek Dosi <abdosi@micrsoft.com>

Co-authored-by: Abhishek Dosi <abdosi@micrsoft.com>
@shlomibitton shlomibitton deleted the shlomi_lldp_port_id_fix branch March 24, 2021 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abdosi abdosi abdosi requested changes

Assignees

No one assigned

Labels

Request for 201911 Branch :shipit:

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@shlomibitton @abdosi @liat-grozovik @keboliu @lguohan