-
Notifications
You must be signed in to change notification settings - Fork 50
lldp_8023: Preserve MAC Merge verify_enabled setting #121
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.
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
lldp_8023: Preserve MAC Merge verify_enabled setting #121
Conversation
The existing LLDP operation forcefully enables verify_enabled during MAC Merge (MM) verification whenever preemption support is advertised by the link partner. This behavior overrides the intended configuration and conflicts with IEEE 802.3br Section 99.4.3, which allows verification to be disabled. This fix ensures that the verify_enabled configuration is preserved and remains a MAC Merge sublayer responsibility, aligning with IEEE 802.3br Tables 30-8 and 30-9. Signed-off-by: Chwee-Lin Choong <chwee.lin.choong@intel.com>
|
Fixes: 5d54c31 ("lldpad: ieee8023: add Additional Ethernet Capabilities TLV logic") @vladimiroltean Please double check the change. |
|
Thank you for the patch, @chweelinchoong, and thanks @apconole for notifying me of it. I'm sorry, I simply do not have access to a copy of the obsolete IEEE 802.3br-2016 standard. I believe that the equivalent clause you are referring to is IEEE 802.3-2022 99.4.3 Verifying preemption operation. Indeed, there it says that verification can be disabled, and it can be disabled if you control the MAC Merge layer manually with ethtool, and not with LLDP. It also says:
I don't think LLDP is an engineered closed network, quite the opposite in fact. The standards, in my reading, were not very clear on the exact LLDP flow with the MAC Merge feature. But I did have a justification for force-enabling the TX verification feature. Let me quote from the commit message of the blamed patch: In other words: my intended usage flow with LLDP was as follows. Any station capable of frame preemption can advertise this capability. BUT at the same time, it can keep its pMAC turned off by default, for power saving reasons. Only when it connects to a link partner also advertising the frame preemption capability, it turns on its pMAC, so that if the link partner chooses to send preemptible traffic and/or verification frames, it can receive them successfully. Since the pMAC, when enabled, is obligated to respond to verification frames (see rcv_v variable and Respond state diagram), then using the verification feature is a reliable way of waiting until the link partner has finished enabling its RX pMAC. Corollary: if the verification feature were not used, it would mean that implementations would have to keep their pMAC RX always enabled, since there would otherwise exist a race condition leading to packet loss between the moment the link partner computes (based on the LLDP TLV) that frame preemption can be used in its local TX direction (and subsequently starts sending preemptible traffic), and the moment that we compute (based on the LLDP TLV) that the link partner might want to send preemptible traffic, and get prepared for it. With the implementation I chose, packet loss is still possible, but only for verification frames (SMD-V) and thus not data packets. For SMD-V, that is quite ok though, because the standard defines a clear retry mechanism with a generous timeout budget (3 * 128 ms). I don't see a problem with making use of the verification feature, and to my knowledge it is a required MAC Merge functionality, so these were the constraints which led me to make the design choices I made. But maybe I don't understand your motivation for wanting to operate LLDP with verification turned off? |
|
We understand your concern regarding potential race conditions in non-engineered networks and agree that verification is a critical feature in such scenarios to ensure both devices are preemption-active simultaneously. However, the IEEE 802.3-2022 standard (99.4.3) explicitly allows verification to be disabled in engineered closed networks, where race conditions are assumed to be mitigated by design. In such cases, LLDP with verification disabled remains valuable for auxiliary purposes, such as exchanging We believe making this behavior configurable would better align with the standard and provide LLDP with the flexibility to cater to both open and engineered networks. Additionally, we are preparing a patch for the kernel ethtool utility ( We welcome further discussion to refine this approach and ensure it meets both standard compliance and practical needs. Regards, |
|
The way in which you define the boundaries of an engineered network seem arbitrary to me. The whole point of LLDP and verification is that you do not control what the link partner does. If you are instead in control of the link partner, don't you also know the addFragSize? I don't see the purpose of LLDP. IMO it is counter-productive to offer a configuration option of LLDP + verification disabled, because it makes an assumption about the implementation of the LLDP agent on the link partner which just doesn't hold water (that it is ready to accept preemptible traffic as soon as you receive a TLV from it advertising the capability, but it hasn't yet received any TLV from you stating that you also advertise it). Let's take even the simple case where both ends run the openlldp implementation. Unless you patch openlldp to keep pmac_enabled at start time, your patch is simply buggy because it does not prevent the race condition I was talking about. And then, even if you do patch openlldp, there exist potentially other LLDP implementations, which you have no idea what they do. You just want to introduce a configuration option which pushes the responsibility of knowing what the link partner does to the end user. That is very fragile and I see no real benefit to justify it. Even if you force pmac_enabled=true regardless of whether the link partner is preemption-capable, you will need an analysis of the impact that this will have upon existing device drivers. Let aside power saving. For current-generation Intel NICs like i226, verification is handled in software and involves adding extra tests in the packet data path for detecting SMD-V or SMD-R packets. These tests cost CPU cycles and will degrade packet throughput, so we are discussing options to turn them off, using static keys, for the normal path where pmac_enabled=false.
What would be the purpose of this change? Why would the kernel allow a configuration where TX verification is enabled, but preemptible TX in general is disabled? What would it even mean, and why would that be useful? Is it connected at all to the patch proposed here, or is it an altogether separate topic? |
|
Our proposed change aims to ensure compliance with the Avnu Alliance Profile Agnostic Conformance Test Plan for Interspersed Express Traffic (IET) IEEE 802.1Qbu/.3br (Revision: 1.2). The test plan includes specific scenarios where LLDP TLV exchange occur while verification is disabled on the DUT. For instance:
However, if you believe that LLDP + disabled verification represents an invalid scenario in real-world deployments, we are open to engaging with Avnu Alliance to express these concerns and seek clarification on the test requirements. Regards, |
|
I do have access to the IEEE 802.3br-2016 standard and others, but I'll have to review it to remember how this works. It's been a while since I've dug in. |
Hi @vladimiroltean , Thanks for the detailed explanation. a. The race condition that would happen without forceful verification b. Your design that enable forceful verification to mitigate race condition |
|
Hi Faizal,
Bits 1 and 2 on their own are useless/unreliable from a protocol standpoint, because by the time you get an LLDP TLV where the link partner had set them, you don't necessarily have time to react and enable your RX - it may be too late. So no protocol decision is being made based on them. |
|
Essentially, IMO, the Additional Ethernet Capabilities TLV is just a plug-and-play way to communicate using packets sent over the eMAC that the pMAC also exists and is a viable communication channel. When each side sees that preemption is supported on both ends, it should have the courtesy of enabling its RX direction, and then, each side independently has the option of enabling TX if it wants to. LLDP does not have any mechanism that replaces verification, quite the opposite: verification is what makes a plug and play feature "just work". It's a foot gun to want to disable it, and I just don't see any good reason why a user might want to. |
|
Hi @vladimiroltean, thanks for your quick response! Sorry for the slow reply. Some benefits I see: Thoughts ? |
|
I think the positions on both sides have been heard, and I'd like Avnu to make a comment - I know this thread has been brought to their attention. |
|
There was an Avnu Silicon Validation and Certification Task Group meeting today, where this topic was briefly touched, although not settled yet.
|
|
So I've asked myself a lot of questions while implementing this feature, and I still stand by my reasoning while answering them, but one question I admit I've never asked myself is apparently a very fundamental one. |
|
After the Avnu meeting today, I believe the topic was clarified abd I would like to explain here as well. There was something which had escaped me during last meeting: LLDP is required for all Avnu compliance tests. There is a set of tests with +LLDP +verification, a set of tests with +LLDP -verification, but no tests -LLDP. Thus, this option which was proposed last time:
makes a lot more sense now. So the proposal as it stands today is to make the tests with +LLDP -verification "conditional mandatory", dependent on the general ability to disable verification (with LLDP). So, since openlldp does not support disabling verification, this test will be skipped. I now do not reject the future possibility to add a patch to openlldp which goes something like this: add a "strict" mode, originally false, which when set to true, sets pmac_enabled=true regardless of the link partner's advertised TLVs, and does allow verification to be either enabled or disabled. This is just as a matter of principle. Things would have to be sorted out with the standards body as well, to see whether pmac_enabled can become part of the standard management objects, and therefore how are we going to "brand" this feature. In the meantime, I understand that the topic is relevant not because of some special use case requiring verification disabled, but because of the Avnu test which can now be bypassed, so I just want to confirm that everyone is ok with this temporary conclusion. |
|
Hi @vladimiroltean, I'm good with the conclusion. Glad to see the impacted test plan is now conditional. Appreciate your time and effort in explaining the practical deployment versus standard offerings. |
|
I think the conclusion of this thread is that the semantic proposed in this patch needs to be re-worked to be a tri-state, like enforcement: 'strict', 'loose', 'disabled' and that the behavior of turning on the verify_enabled macmerge setting will be based on that configuration knob. Did I understand it correctly? Will @chweelinchoong be reworking the proposed patch? |
|
Aaron, I'm a bit confused by your reference to a tri-state, so here is a more detailed explanation (repeating some things):
|
|
Okay - given the above, I don't know who will work on the pmac emulation patch, but it doesn't matter. i think that means this PR can be closed. |




The existing LLDP operation forcefully enables verify_enabled during MAC Merge (MM) verification whenever preemption support is advertised by the link partner. This behavior overrides the intended configuration and conflicts with IEEE 802.3br Section 99.4.3, which allows verification to be disabled.
This fix ensures that the verify_enabled configuration is preserved and remains a MAC Merge sublayer responsibility, aligning with IEEE 802.3br Tables 30-8 and 30-9.