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

Fix mac address verification #274

Merged

Conversation

SchSeba
Copy link
Collaborator

@SchSeba SchSeba commented Jul 10, 2023

Before this fix there was a problem with upper or lower case mac address

@SchSeba SchSeba force-pushed the fix_mac_address_verification branch from ca2734b to f18e51a Compare July 10, 2023 13:39
@adrianchiris
Copy link
Contributor

LGTM,

@SchSeba can you add a short description in commit msg explaining why its needed ?

@mlguerrero12
Copy link
Contributor

LGTM

@adrianchiris
Copy link
Contributor

There are some issues with mlnx CI should be fixed later today / tommorow.

Without this commit there is a possible bug if the user
request a mac address with upper case.

The issue is that netlink lib will always return the hardware struct
for mac address and it will always be lower case.

So to be able and do a right equal check we convert the user provided mac
address to lower case always

Signed-off-by: Sebastian Sch <[email protected]>
@SchSeba SchSeba force-pushed the fix_mac_address_verification branch from f18e51a to f1d045a Compare July 10, 2023 15:41
@SchSeba
Copy link
Collaborator Author

SchSeba commented Jul 10, 2023

@adrianchiris done

Copy link
Member

@zeeke zeeke left a comment

Choose a reason for hiding this comment

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

LGTM

@adrianchiris
Copy link
Contributor

/test-all

@SchSeba
Copy link
Collaborator Author

SchSeba commented Jul 11, 2023

Two LGTM merging.

Thanks, folks!

@SchSeba SchSeba merged commit 272860a into k8snetworkplumbingwg:master Jul 11, 2023
9 checks passed
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.

4 participants