On Mon, 13 May 2019 17:18:28 +0200
Nicolas Dichtel <nicolas.dich...@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 <d...@redhat.com>
> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >> Signed-off-by: Sabrina Dubroca <s...@queasysnail.net>
> >> Acked-by: Nicolas Dichtel <nicolas.dich...@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.
