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.

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_dcbx_nl: Fix Copy-Paste Typo #101

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

liuhangbin
Copy link
Contributor

It should check the RX PGID when configuring RX PG per TC settings.

Fixes: da0da5e ("lldpad: dcbx, map PG up2tc config onto hardware limits")

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
It should check the RX PGID when configuring RX PG per TC settings.

Fixes: da0da5e ("lldpad: dcbx, map PG up2tc config onto hardware limits")
Signed-off-by: Hangbin Liu <[email protected]>
@penguin359
Copy link
Contributor

This looks like a legit typo that should be fixed, although how this affects the resulting priority groups is less clear to me. It seems like this should present a very obvious change in the behavior of the NIC under test if it supports hardware PGs.

@apconole do you have access to any actual DCBX-capable switches to test on? I might be able to get this scenario set up in our datacenter lab sandbox and get the software validation team to validate the bug and fix from this PR. If this is a bug, it dates back to at least 2011 which makes me question whether this is a bug that's gone unnoticed or is actually intended behavior. This will need a closer look to say one way or the other.

@apconole
Copy link
Contributor

It would be good to get a test run in. I'm inclined to believe that this functionality wasn't covered anywhere, to be honest. I don't have access to any DCB-x switches to test, though.

@penguin359
Copy link
Contributor

penguin359 commented Jul 10, 2024

It just seems odd that this line hasn't been touched since 2012, and was originally tx when it was added in 2011, that this bug wasn't caught when I was involved in the extensive DCBX validation and hardware-based priority groups and PFC/ETS that we did on-site here in Oregon until 2019. Now, that has moved elsewhere and I don't see it anymore, but this seems like it should be something that would have affected our testing if the code is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants