-
Notifications
You must be signed in to change notification settings - Fork 250
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
Set MAC on host side of veth pair #436
Conversation
I put |
Need to make sure the FAQ entry in projectcalico/calico#1394 is added to the v2.6 docs when this PR is ready for to go into a v2.6 release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm.
utils/network.go
Outdated
@@ -57,6 +57,18 @@ func DoNetworking(args *skel.CmdArgs, conf NetConf, result *current.Result, logg | |||
return err | |||
} | |||
|
|||
mac, err := net.ParseMAC("EE:EE:EE:EE:EE:EE") | |||
if err != nil { | |||
return fmt.Errorf("failed to parse MAC Address: %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, wondering if we should return an error in these cases, or just log an error.
thought process is - we don't do this today and it works on most systems, should we just fall back to today's behavior if for some reason we fail to set the mac on a given system?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Just log an error and only do LinkSetHardwareAddr if it is successful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, and if LinkSetHardwareAddr
fails then we carry on as normal (but log an error).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fine to me. (TBH, I'd be tempted to panic if parsing a fixed MAC fails. That should work 100% of the time so the most likely scenario for that to fail is if we accidentally introduced a typo into the MAC during maintenance.)
8c7f9dd
to
ba7f7af
Compare
- Use hardcoded MAC of EE:EE:EE:EE:EE:EE (matchs libnetwork) - If there is an error log message and fallback to kernel provided MAC
ba7f7af
to
a2ff567
Compare
I have cherry-picked the change from 0038728 that was made against the master branch so I think this is ready to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tmjd! Merge away.
persistent MAC address