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

Allow index specification at link creation time #283

Merged
merged 1 commit into from
Oct 26, 2017

Conversation

mcastelino
Copy link
Contributor

Allow the caller to specify the desired link index at link creation.

This is equivalent to
ip link add link eth0 name testmacvtap index 1000 type macvtap
ip link add dummy1 index 1001 type dummy

Signed-off-by: Manohar Castelino manohar.r.castelino@intel.com

link_linux.go Outdated
@@ -851,6 +851,14 @@ func (h *Handle) linkModify(link Link, flags int) error {
msg.Change |= syscall.IFF_MULTICAST
msg.Flags |= syscall.IFF_MULTICAST
}
if base.Index != 0 {
if flags&syscall.NLM_F_CREATE == 0 {
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think it should check for NLM_F_CREATE and error here. Specifically this will break BridgeSetMcastSnoop if you attempt to retrieve the link (which sets the index value) and then use the retrieved link to set snoop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me remove that check. I was just being paranoid. I did not realize that there was a case beyond create where it was valid to set the index.

Allow the caller to specify the desired link index at link creation.

This is equivalent to
ip link add link eth0 name testmacvtap index 1000 type macvtap
ip link add dummy1 index 1001 type dummy

Signed-off-by: Manohar Castelino <manohar.r.castelino@intel.com>
@mcastelino mcastelino force-pushed the topic/create_with_index branch from 648aac8 to 35e0334 Compare October 20, 2017 18:20
@mcastelino
Copy link
Contributor Author

@vishvananda addressed the review comment. CI has also 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.

2 participants