| Message ID | d5c4710117d390e0f204b7046483727daf452233.1557755096.git.sd@queasysnail.net |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net,v2] rtnetlink: always put ILFA_LINK for links with a link-netnsid | expand |
Le 13/05/2019 à 15:47, Sabrina Dubroca a écrit : > Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when > iflink == ifindex. > > In some cases, a device can be created in a different netns with the > same ifindex as its parent. That device will not dump its IFLA_LINK > attribute, which can confuse some userspace software that expects it. > For example, if the last ifindex created in init_net and foo are both > 8, these commands will trigger the issue: > > ip link add parent type dummy # ifindex 9 > ip link add link parent netns foo type macvlan # ifindex 9 in ns foo > > So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, > always put the IFLA_LINK attribute as well. > > Thanks to Dan Winship for analyzing the original OpenShift bug down to > the missing netlink attribute. > > Analyzed-by: Dan Winship <danw@redhat.com> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") I would say: Fixes: 5e6700b3bf98 ("sit: add support of x-netns") Because before this patch, there was no device with an iflink that can be put in another netns. Regards, Nicolas
On 5/13/19 7:47 AM, Sabrina Dubroca wrote: > Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when > iflink == ifindex. > > In some cases, a device can be created in a different netns with the > same ifindex as its parent. That device will not dump its IFLA_LINK > attribute, which can confuse some userspace software that expects it. > For example, if the last ifindex created in init_net and foo are both > 8, these commands will trigger the issue: > > ip link add parent type dummy # ifindex 9 > ip link add link parent netns foo type macvlan # ifindex 9 in ns foo > > So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, > always put the IFLA_LINK attribute as well. > > Thanks to Dan Winship for analyzing the original OpenShift bug down to > the missing netlink attribute. > > Analyzed-by: Dan Winship <danw@redhat.com> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> > Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> > --- > v2: change Fixes tag, it's been here forever as Nicolas said, and add his Ack > > net/core/rtnetlink.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index 2bd12afb9297..adcc045952c2 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -1496,14 +1496,15 @@ static int put_master_ifindex(struct sk_buff *skb, struct net_device *dev) > return ret; > } > > -static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev) > +static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev, > + bool force) > { > int ifindex = dev_get_iflink(dev); > > - if (dev->ifindex == ifindex) > - return 0; > + if (force || dev->ifindex != ifindex) > + return nla_put_u32(skb, IFLA_LINK, ifindex); > > - return nla_put_u32(skb, IFLA_LINK, ifindex); > + return 0; > } > > static noinline_for_stack int nla_put_ifalias(struct sk_buff *skb, why not always adding the attribute?
2019-05-13, 16:50:51 +0200, Nicolas Dichtel wrote: > Le 13/05/2019 à 15:47, Sabrina Dubroca a écrit : > > Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when > > iflink == ifindex. > > > > In some cases, a device can be created in a different netns with the > > same ifindex as its parent. That device will not dump its IFLA_LINK > > attribute, which can confuse some userspace software that expects it. > > For example, if the last ifindex created in init_net and foo are both > > 8, these commands will trigger the issue: > > > > ip link add parent type dummy # ifindex 9 > > ip link add link parent netns foo type macvlan # ifindex 9 in ns foo > > > > So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, > > always put the IFLA_LINK attribute as well. > > > > Thanks to Dan Winship for analyzing the original OpenShift bug down to > > the missing netlink attribute. > > > > Analyzed-by: Dan Winship <danw@redhat.com> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > I would say: > Fixes: 5e6700b3bf98 ("sit: add support of x-netns") > > Because before this patch, there was no device with an iflink that can be put in > another netns. That tells us how far back we might want to backport this fix, but not which commit introduced the bug. I think Fixes should refer to the introduction of the faulty code, not to what patch made it visible (if we can find both). Thanks,
Le 13/05/2019 à 17:08, Sabrina Dubroca a écrit : > 2019-05-13, 16:50:51 +0200, Nicolas Dichtel wrote: >> Le 13/05/2019 à 15:47, Sabrina Dubroca a écrit : >>> Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when >>> iflink == ifindex. >>> >>> In some cases, a device can be created in a different netns with the >>> same ifindex as its parent. That device will not dump its IFLA_LINK >>> attribute, which can confuse some userspace software that expects it. >>> For example, if the last ifindex created in init_net and foo are both >>> 8, these commands will trigger the issue: >>> >>> ip link add parent type dummy # ifindex 9 >>> ip link add link parent netns foo type macvlan # ifindex 9 in ns foo >>> >>> So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, >>> always put the IFLA_LINK attribute as well. >>> >>> Thanks to Dan Winship for analyzing the original OpenShift bug down to >>> the missing netlink attribute. >>> >>> Analyzed-by: Dan Winship <danw@redhat.com> >>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> I would say: >> Fixes: 5e6700b3bf98 ("sit: add support of x-netns") >> >> Because before this patch, there was no device with an iflink that can be put in >> another netns. > > That tells us how far back we might want to backport this fix, but not > which commit introduced the bug. I think Fixes should refer to the > introduction of the faulty code, not to what patch made it visible (if > we can find both). No sure to follow you. The problem you describe cannot happen before commit 5e6700b3bf98, so there cannot be a "faulty" patch before that commit. Anyway, both commits are very old, so it doesn't matter. Thank you, Nicolas
Le 13/05/2019 à 17:07, David Ahern a écrit : > On 5/13/19 7:47 AM, Sabrina Dubroca wrote: >> Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when >> iflink == ifindex. >> >> In some cases, a device can be created in a different netns with the >> same ifindex as its parent. That device will not dump its IFLA_LINK >> attribute, which can confuse some userspace software that expects it. >> For example, if the last ifindex created in init_net and foo are both >> 8, these commands will trigger the issue: >> >> ip link add parent type dummy # ifindex 9 >> ip link add link parent netns foo type macvlan # ifindex 9 in ns foo >> >> So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, >> always put the IFLA_LINK attribute as well. >> >> Thanks to Dan Winship for analyzing the original OpenShift bug down to >> the missing netlink attribute. >> >> Analyzed-by: Dan Winship <danw@redhat.com> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> >> Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> >> --- >> v2: change Fixes tag, it's been here forever as Nicolas said, and add his Ack >> >> net/core/rtnetlink.c | 16 ++++++++++------ >> 1 file changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c >> index 2bd12afb9297..adcc045952c2 100644 >> --- a/net/core/rtnetlink.c >> +++ b/net/core/rtnetlink.c >> @@ -1496,14 +1496,15 @@ static int put_master_ifindex(struct sk_buff *skb, struct net_device *dev) >> return ret; >> } >> >> -static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev) >> +static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev, >> + bool force) >> { >> int ifindex = dev_get_iflink(dev); >> >> - if (dev->ifindex == ifindex) >> - return 0; >> + if (force || dev->ifindex != ifindex) >> + return nla_put_u32(skb, IFLA_LINK, ifindex); >> >> - return nla_put_u32(skb, IFLA_LINK, ifindex); >> + return 0; >> } >> >> static noinline_for_stack int nla_put_ifalias(struct sk_buff *skb, > > why not always adding the attribute? > Adding this attribute may change the output of 'ip link'. See this patch for example: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=95ec655bc465 Nicolas
From: Nicolas Dichtel <nicolas.dichtel@6wind.com> Date: Mon, 13 May 2019 16:50:51 +0200 > Le 13/05/2019 à 15:47, Sabrina Dubroca a écrit : >> Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when >> iflink == ifindex. >> >> In some cases, a device can be created in a different netns with the >> same ifindex as its parent. That device will not dump its IFLA_LINK >> attribute, which can confuse some userspace software that expects it. >> For example, if the last ifindex created in init_net and foo are both >> 8, these commands will trigger the issue: >> >> ip link add parent type dummy # ifindex 9 >> ip link add link parent netns foo type macvlan # ifindex 9 in ns foo >> >> So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, >> always put the IFLA_LINK attribute as well. >> >> Thanks to Dan Winship for analyzing the original OpenShift bug down to >> the missing netlink attribute. >> >> Analyzed-by: Dan Winship <danw@redhat.com> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > I would say: > Fixes: 5e6700b3bf98 ("sit: add support of x-netns") > > Because before this patch, there was no device with an iflink that can be put in > another netns. I kind of agree. What's important for people is knowing at what point they need to backport a fix in order to actually fix a real problem. Sabrina, please adjust the Fixes tag, thank you.
On Mon, 13 May 2019 17:18:28 +0200 Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote: > Le 13/05/2019 à 17:07, David Ahern a écrit : > > On 5/13/19 7:47 AM, Sabrina Dubroca wrote: > >> Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when > >> iflink == ifindex. > >> > >> In some cases, a device can be created in a different netns with the > >> same ifindex as its parent. That device will not dump its IFLA_LINK > >> attribute, which can confuse some userspace software that expects it. > >> For example, if the last ifindex created in init_net and foo are both > >> 8, these commands will trigger the issue: > >> > >> ip link add parent type dummy # ifindex 9 > >> ip link add link parent netns foo type macvlan # ifindex 9 in ns foo > >> > >> So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, > >> always put the IFLA_LINK attribute as well. > >> > >> Thanks to Dan Winship for analyzing the original OpenShift bug down to > >> the missing netlink attribute. > >> > >> Analyzed-by: Dan Winship <danw@redhat.com> > >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > >> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> > >> Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> > >> --- > >> v2: change Fixes tag, it's been here forever as Nicolas said, and add his Ack > >> > >> net/core/rtnetlink.c | 16 ++++++++++------ > >> 1 file changed, 10 insertions(+), 6 deletions(-) > >> > >> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > >> index 2bd12afb9297..adcc045952c2 100644 > >> --- a/net/core/rtnetlink.c > >> +++ b/net/core/rtnetlink.c > >> @@ -1496,14 +1496,15 @@ static int put_master_ifindex(struct sk_buff *skb, struct net_device *dev) > >> return ret; > >> } > >> > >> -static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev) > >> +static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev, > >> + bool force) > >> { > >> int ifindex = dev_get_iflink(dev); > >> > >> - if (dev->ifindex == ifindex) > >> - return 0; > >> + if (force || dev->ifindex != ifindex) > >> + return nla_put_u32(skb, IFLA_LINK, ifindex); > >> > >> - return nla_put_u32(skb, IFLA_LINK, ifindex); > >> + return 0; > >> } > >> > >> static noinline_for_stack int nla_put_ifalias(struct sk_buff *skb, > > > > why not always adding the attribute? > > > Adding this attribute may change the output of 'ip link'. > See this patch for example: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=95ec655bc465 +1 Printing eno1@eno1 is going to make scripts and users upset.
On 5/13/19 9:18 AM, Nicolas Dichtel wrote: > Adding this attribute may change the output of 'ip link'. > See this patch for example: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=95ec655bc465 > > I figured that would be the response, but wanted to make sure.
IFLA_LINK is typoed in subject line, you might want to fix that if respinning. -Ed
2019-05-13, 17:13:36 +0200, Nicolas Dichtel wrote: > Le 13/05/2019 à 17:08, Sabrina Dubroca a écrit : > > 2019-05-13, 16:50:51 +0200, Nicolas Dichtel wrote: > >> Le 13/05/2019 à 15:47, Sabrina Dubroca a écrit : > >>> Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when > >>> iflink == ifindex. > >>> > >>> In some cases, a device can be created in a different netns with the > >>> same ifindex as its parent. That device will not dump its IFLA_LINK > >>> attribute, which can confuse some userspace software that expects it. > >>> For example, if the last ifindex created in init_net and foo are both > >>> 8, these commands will trigger the issue: > >>> > >>> ip link add parent type dummy # ifindex 9 > >>> ip link add link parent netns foo type macvlan # ifindex 9 in ns foo > >>> > >>> So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, > >>> always put the IFLA_LINK attribute as well. > >>> > >>> Thanks to Dan Winship for analyzing the original OpenShift bug down to > >>> the missing netlink attribute. > >>> > >>> Analyzed-by: Dan Winship <danw@redhat.com> > >>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > >> I would say: > >> Fixes: 5e6700b3bf98 ("sit: add support of x-netns") > >> > >> Because before this patch, there was no device with an iflink that can be put in > >> another netns. > > > > That tells us how far back we might want to backport this fix, but not > > which commit introduced the bug. I think Fixes should refer to the > > introduction of the faulty code, not to what patch made it visible (if > > we can find both). > No sure to follow you. The problem you describe cannot happen before commit > 5e6700b3bf98, so there cannot be a "faulty" patch before that commit. What about macvlan devices? From commit b863ceb7ddce ("[NET]: Add macvlan driver"): static int macvlan_init(struct net_device *dev) { ... dev->iflink = lowerdev->ifindex; ... } vlan devices also had an iflink assigned since commit ddd7bf9fe4e5. What am I missing?
Le 13/05/2019 à 23:46, Sabrina Dubroca a écrit : > 2019-05-13, 17:13:36 +0200, Nicolas Dichtel wrote: >> Le 13/05/2019 à 17:08, Sabrina Dubroca a écrit : >>> 2019-05-13, 16:50:51 +0200, Nicolas Dichtel wrote: >>>> Le 13/05/2019 à 15:47, Sabrina Dubroca a écrit : >>>>> Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when >>>>> iflink == ifindex. >>>>> >>>>> In some cases, a device can be created in a different netns with the >>>>> same ifindex as its parent. That device will not dump its IFLA_LINK >>>>> attribute, which can confuse some userspace software that expects it. >>>>> For example, if the last ifindex created in init_net and foo are both >>>>> 8, these commands will trigger the issue: >>>>> >>>>> ip link add parent type dummy # ifindex 9 >>>>> ip link add link parent netns foo type macvlan # ifindex 9 in ns foo >>>>> >>>>> So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, >>>>> always put the IFLA_LINK attribute as well. >>>>> >>>>> Thanks to Dan Winship for analyzing the original OpenShift bug down to >>>>> the missing netlink attribute. >>>>> >>>>> Analyzed-by: Dan Winship <danw@redhat.com> >>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >>>> I would say: >>>> Fixes: 5e6700b3bf98 ("sit: add support of x-netns") >>>> >>>> Because before this patch, there was no device with an iflink that can be put in >>>> another netns. >>> >>> That tells us how far back we might want to backport this fix, but not >>> which commit introduced the bug. I think Fixes should refer to the >>> introduction of the faulty code, not to what patch made it visible (if >>> we can find both). >> No sure to follow you. The problem you describe cannot happen before commit >> 5e6700b3bf98, so there cannot be a "faulty" patch before that commit. > > What about macvlan devices? > > From commit b863ceb7ddce ("[NET]: Add macvlan driver"): > > static int macvlan_init(struct net_device *dev) > { > ... > dev->iflink = lowerdev->ifindex; > ... > } > > vlan devices also had an iflink assigned since commit ddd7bf9fe4e5. > > What am I missing? You miss the fact that netns have been introduced after both commits. What about this one? Fixes: d8a5ec672768 ("[NET]: netlink support for moving devices between network namespaces.") Regards, Nicolas
2019-05-14, 09:32:32 +0200, Nicolas Dichtel wrote: > Le 13/05/2019 à 23:46, Sabrina Dubroca a écrit : > > 2019-05-13, 17:13:36 +0200, Nicolas Dichtel wrote: > >> Le 13/05/2019 à 17:08, Sabrina Dubroca a écrit : > >>> 2019-05-13, 16:50:51 +0200, Nicolas Dichtel wrote: > >>>> Le 13/05/2019 à 15:47, Sabrina Dubroca a écrit : > >>>>> Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when > >>>>> iflink == ifindex. > >>>>> > >>>>> In some cases, a device can be created in a different netns with the > >>>>> same ifindex as its parent. That device will not dump its IFLA_LINK > >>>>> attribute, which can confuse some userspace software that expects it. > >>>>> For example, if the last ifindex created in init_net and foo are both > >>>>> 8, these commands will trigger the issue: > >>>>> > >>>>> ip link add parent type dummy # ifindex 9 > >>>>> ip link add link parent netns foo type macvlan # ifindex 9 in ns foo > >>>>> > >>>>> So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, > >>>>> always put the IFLA_LINK attribute as well. > >>>>> > >>>>> Thanks to Dan Winship for analyzing the original OpenShift bug down to > >>>>> the missing netlink attribute. > >>>>> > >>>>> Analyzed-by: Dan Winship <danw@redhat.com> > >>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > >>>> I would say: > >>>> Fixes: 5e6700b3bf98 ("sit: add support of x-netns") > >>>> > >>>> Because before this patch, there was no device with an iflink that can be put in > >>>> another netns. > >>> > >>> That tells us how far back we might want to backport this fix, but not > >>> which commit introduced the bug. I think Fixes should refer to the > >>> introduction of the faulty code, not to what patch made it visible (if > >>> we can find both). > >> No sure to follow you. The problem you describe cannot happen before commit > >> 5e6700b3bf98, so there cannot be a "faulty" patch before that commit. > > > > What about macvlan devices? > > > > From commit b863ceb7ddce ("[NET]: Add macvlan driver"): > > > > static int macvlan_init(struct net_device *dev) > > { > > ... > > dev->iflink = lowerdev->ifindex; > > ... > > } > > > > vlan devices also had an iflink assigned since commit ddd7bf9fe4e5. > > > > What am I missing? > You miss the fact that netns have been introduced after both commits. Ah, right. > What about this one? > Fixes: d8a5ec672768 ("[NET]: netlink support for moving devices between network > namespaces.") Nice. Now I think the bug can't really trigger unless one of these commits are present: aa79e66eee5d ("net: Make ifindex generation per-net namespace") 9c7dafbfab15 ("net: Allow to create links with given ifindex") I'll use those two as Fixes tags for v3, unless you want something different. Thanks,
Le 14/05/2019 à 10:01, Sabrina Dubroca a écrit : > 2019-05-14, 09:32:32 +0200, Nicolas Dichtel wrote: [snip] >> What about this one? >> Fixes: d8a5ec672768 ("[NET]: netlink support for moving devices between network >> namespaces.") > > Nice. Now I think the bug can't really trigger unless one of these > commits are present: > > aa79e66eee5d ("net: Make ifindex generation per-net namespace") > 9c7dafbfab15 ("net: Allow to create links with given ifindex") > I don't think so. Please have a look to commit ce286d327341 ("[NET]: Implement network device movement between namespaces"). In dev_change_net_namespace(), there is the following code: /* If there is an ifindex conflict assign a new one */ if (__dev_get_by_index(net, dev->ifindex)) { int iflink = (dev->iflink == dev->ifindex); dev->ifindex = dev_new_index(net); if (iflink) dev->iflink = dev->ifindex; } This code may change the ifindex of an interface when this interface moves to another netns. This may happen even before the commits you propose, because the global ifindex counter can wrap around. Regards, Nicolas
2019-05-14, 12:05:16 +0200, Nicolas Dichtel wrote: > Le 14/05/2019 à 10:01, Sabrina Dubroca a écrit : > > 2019-05-14, 09:32:32 +0200, Nicolas Dichtel wrote: > [snip] > >> What about this one? > >> Fixes: d8a5ec672768 ("[NET]: netlink support for moving devices between network > >> namespaces.") > > > > Nice. Now I think the bug can't really trigger unless one of these > > commits are present: > > > > aa79e66eee5d ("net: Make ifindex generation per-net namespace") > > 9c7dafbfab15 ("net: Allow to create links with given ifindex") > > > I don't think so. > > Please have a look to commit ce286d327341 ("[NET]: Implement network device > movement between namespaces"). > In dev_change_net_namespace(), there is the following code: > > /* If there is an ifindex conflict assign a new one */ > if (__dev_get_by_index(net, dev->ifindex)) { > int iflink = (dev->iflink == dev->ifindex); > dev->ifindex = dev_new_index(net); > if (iflink) > dev->iflink = dev->ifindex; > } > > This code may change the ifindex of an interface when this interface moves to > another netns. This may happen even before the commits you propose, because the > global ifindex counter can wrap around. Yes, that's possible although quite unlikely. I'll go with d8a5ec672768.
Le 14/05/2019 à 12:24, Sabrina Dubroca a écrit : [snip] > Yes, that's possible although quite unlikely. I'll go with d8a5ec672768. > Agreed. Thank you, Nicolas
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 2bd12afb9297..adcc045952c2 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -1496,14 +1496,15 @@ static int put_master_ifindex(struct sk_buff *skb, struct net_device *dev) return ret; } -static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev) +static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev, + bool force) { int ifindex = dev_get_iflink(dev); - if (dev->ifindex == ifindex) - return 0; + if (force || dev->ifindex != ifindex) + return nla_put_u32(skb, IFLA_LINK, ifindex); - return nla_put_u32(skb, IFLA_LINK, ifindex); + return 0; } static noinline_for_stack int nla_put_ifalias(struct sk_buff *skb, @@ -1520,6 +1521,8 @@ static int rtnl_fill_link_netnsid(struct sk_buff *skb, const struct net_device *dev, struct net *src_net) { + bool put_iflink = false; + if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) { struct net *link_net = dev->rtnl_link_ops->get_link_net(dev); @@ -1528,10 +1531,12 @@ static int rtnl_fill_link_netnsid(struct sk_buff *skb, if (nla_put_s32(skb, IFLA_LINK_NETNSID, id)) return -EMSGSIZE; + + put_iflink = true; } } - return 0; + return nla_put_iflink(skb, dev, put_iflink); } static int rtnl_fill_link_af(struct sk_buff *skb, @@ -1617,7 +1622,6 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, #ifdef CONFIG_RPS nla_put_u32(skb, IFLA_NUM_RX_QUEUES, dev->num_rx_queues) || #endif - nla_put_iflink(skb, dev) || put_master_ifindex(skb, dev) || nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) || (dev->qdisc &&
Currently, nla_put_iflink() doesn't put the IFLA_LINK attribute when iflink == ifindex. In some cases, a device can be created in a different netns with the same ifindex as its parent. That device will not dump its IFLA_LINK attribute, which can confuse some userspace software that expects it. For example, if the last ifindex created in init_net and foo are both 8, these commands will trigger the issue: ip link add parent type dummy # ifindex 9 ip link add link parent netns foo type macvlan # ifindex 9 in ns foo So, in case a device puts the IFLA_LINK_NETNSID attribute in a dump, always put the IFLA_LINK attribute as well. Thanks to Dan Winship for analyzing the original OpenShift bug down to the missing netlink attribute. Analyzed-by: Dan Winship <danw@redhat.com> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- v2: change Fixes tag, it's been here forever as Nicolas said, and add his Ack net/core/rtnetlink.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)