lldp: Tolerate multiple management address TLVs

[thwalsh]

Unlike the other basic TLV types (System Name, Port Description, etc.), the Management Address TLV may appear more than once in a single LLDPDU, e.g. one per address family (IPv4, IPv6). Switches commonly do this.

Commit 44006eb ("lldp: Reject frames with duplicate TLVs") treated a second Management Address TLV as a fatal frame error and aborted processing the entire frame with goto out. This caused all subsequent TLVs (including 802.1Qaz ETS/PFC/APP TLVs and the End TLV) to be skipped, resulting in the ETS state machine never receiving the peer's configuration.

This broke PFC/ETS learning from switches that send multiple Management Address TLVs, such as Nvidia Cumulus Linux switches (MSN4600, MSN4700).

Fix by removing the checks for if an additional TLV type 8 is received. Instead leaving the idempotent check that the RCVD_LLDP_TLV_TYPE8 bit is set and then freeing the tlv and continuing on with the processing of TLVs.

Due to mgmtadd being write-only, it has been removed as there is no need for it in rxProcessFrame.

[penguin359]

First scan through the patch looks completely reasonable. Will take a closer look and try to test this in the lab.

[thwalsh]

Thanks for taking a look. I was testing via scapy and just simply creating a LLDPDU that had > 1 TLV type 8.

[apconole]

Just a quick look through - is tlv_stored being handled properly? Also, why was the assignment location changed with this commit - seems like a separate logical change to me.

[thwalsh]

Hey there @apconole , I appreciate you taking a look.

tlv_stored is still handled properly even though it is no longer handled within the TYPE_8 branch. Now the TYPE_8 branch will simply ensure the RCVD_LLDP_TLV_TYPE8 bit is set, free the TLV and continue with processing of subsequent TLVs.

The assignment was moved to the top of the loop before the parsing, allowing for the TYPE_8 branch to utilize a continue statement instead of a goto that would've required jumping to the bottom of the loop.