Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix wrong MAC address value in returned Result. #144

Merged

Conversation

DmytroLinkin
Copy link
Contributor

According to CNI Convention MAC address returned in the Result is the
one configured by the plugin, but plugin return initial VF's MAC addr
even if it was passed in cni config.
MAC address value, returned by SetupVF function, assigned from the
linkObj after mac address was setted. That's not correct, since linkObj
don't point directly to underlying kernel's netlink object, so mac
address still has initial value.
Fix that by doing correct assignments.

Fix: #143
Signed-off-by: Dmytro Linkin dlinkin@nvidia.com

@adrianchiris
Copy link
Contributor

adrianchiris commented Nov 4, 2020

from the code i can see the problem and the fix
so im LGTM

According to CNI Convention MAC address returned in the Result is the
one configured by the plugin, but plugin return initial VF's MAC addr
even if it was passed in cni config.
MAC address value, returned by SetupVF function, assigned from the
linkObj after mac address was setted. That's not correct, since linkObj
don't point directly to underlying kernel's netlink object, so mac
address still has initial value.
Fix that by doing correct assignments.

Fix: k8snetworkplumbingwg#143
Signed-off-by: Dmytro Linkin <dlinkin@nvidia.com>
Copy link
Member

@martinkennelly martinkennelly left a comment

Choose a reason for hiding this comment

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

Thank you for this patch!

@martinkennelly martinkennelly merged commit 19ca0d6 into k8snetworkplumbingwg:master Nov 10, 2020
@DmytroLinkin DmytroLinkin deleted the macaddr_fix branch November 10, 2020 13:20
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong MAC address value returned in Result on CmdAdd
4 participants