2014-01-30 13:05:14

by Tom Gundersen

Subject: [PATCH] rtnetlink: return the newly created link in response to newlink

Userspace needs to reliably know the ifindex of the netdevs it creates,
as we cannot rely on the ifname staying unchanged.

Earlier, a simlpe NLMSG_ERROR would be returned, but this returns the
corresponding RTM_NEWLINK on success instead.

Signed-off-by: Tom Gundersen <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: David S. Miller <[email protected]>
---
net/core/rtnetlink.c | 100 ++++++++++++++++++++++++++-------------------------
1 file changed, 52 insertions(+), 48 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index cf67144..31c1322 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1725,6 +1725,54 @@ static int rtnl_group_changelink(struct net *net, int group,
return 0;
}

+static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr* nlh)
+{
+ struct net *net = sock_net(skb->sk);
+ struct ifinfomsg *ifm;
+ char ifname[IFNAMSIZ];
+ struct nlattr *tb[IFLA_MAX+1];
+ struct net_device *dev = NULL;
+ struct sk_buff *nskb;
+ int err;
+ u32 ext_filter_mask = 0;
+
+ err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy);
+ if (err < 0)
+ return err;
+
+ if (tb[IFLA_IFNAME])
+ nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
+
+ if (tb[IFLA_EXT_MASK])
+ ext_filter_mask = nla_get_u32(tb[IFLA_EXT_MASK]);
+
+ ifm = nlmsg_data(nlh);
+ if (ifm->ifi_index > 0)
+ dev = __dev_get_by_index(net, ifm->ifi_index);
+ else if (tb[IFLA_IFNAME])
+ dev = __dev_get_by_name(net, ifname);
+ else
+ return -EINVAL;
+
+ if (dev == NULL)
+ return -ENODEV;
+
+ nskb = nlmsg_new(if_nlmsg_size(dev, ext_filter_mask), GFP_KERNEL);
+ if (nskb == NULL)
+ return -ENOBUFS;
+
+ err = rtnl_fill_ifinfo(nskb, dev, RTM_NEWLINK, NETLINK_CB(skb).portid,
+ nlh->nlmsg_seq, 0, 0, ext_filter_mask);
+ if (err < 0) {
+ /* -EMSGSIZE implies BUG in if_nlmsg_size */
+ WARN_ON(err == -EMSGSIZE);
+ kfree_skb(nskb);
+ } else
+ err = rtnl_unicast(nskb, net, NETLINK_CB(skb).portid);
+
+ return err;
+}
+
static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh)
{
struct net *net = sock_net(skb->sk);
@@ -1871,63 +1919,19 @@ replay:
goto out;
}

+ ifm->ifi_index = dev->ifindex;
+
err = rtnl_configure_link(dev, ifm);
if (err < 0)
unregister_netdevice(dev);
+ else
+ rtnl_getlink(skb, nlh);
out:
put_net(dest_net);
return err;
}
}

-static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr* nlh)
-{
- struct net *net = sock_net(skb->sk);
- struct ifinfomsg *ifm;
- char ifname[IFNAMSIZ];
- struct nlattr *tb[IFLA_MAX+1];
- struct net_device *dev = NULL;
- struct sk_buff *nskb;
- int err;
- u32 ext_filter_mask = 0;
-
- err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy);
- if (err < 0)
- return err;
-
- if (tb[IFLA_IFNAME])
- nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
-
- if (tb[IFLA_EXT_MASK])
- ext_filter_mask = nla_get_u32(tb[IFLA_EXT_MASK]);
-
- ifm = nlmsg_data(nlh);
- if (ifm->ifi_index > 0)
- dev = __dev_get_by_index(net, ifm->ifi_index);
- else if (tb[IFLA_IFNAME])
- dev = __dev_get_by_name(net, ifname);
- else
- return -EINVAL;
-
- if (dev == NULL)
- return -ENODEV;
-
- nskb = nlmsg_new(if_nlmsg_size(dev, ext_filter_mask), GFP_KERNEL);
- if (nskb == NULL)
- return -ENOBUFS;
-
- err = rtnl_fill_ifinfo(nskb, dev, RTM_NEWLINK, NETLINK_CB(skb).portid,
- nlh->nlmsg_seq, 0, 0, ext_filter_mask);
- if (err < 0) {
- /* -EMSGSIZE implies BUG in if_nlmsg_size */
- WARN_ON(err == -EMSGSIZE);
- kfree_skb(nskb);
- } else
- err = rtnl_unicast(nskb, net, NETLINK_CB(skb).portid);
-
- return err;
-}
-
static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
{
struct net *net = sock_net(skb->sk);
--
1.8.5.3

2014-01-30 14:46:05

by Thomas Graf

Subject: Re: [PATCH] rtnetlink: return the newly created link in response to newlink

On 01/30/14 at 02:05pm, Tom Gundersen wrote:
> Userspace needs to reliably know the ifindex of the netdevs it creates,
> as we cannot rely on the ifname staying unchanged.
>
> Earlier, a simlpe NLMSG_ERROR would be returned, but this returns the
> corresponding RTM_NEWLINK on success instead.

This breaks existing Netlink applications in user space. User space
apps are not prepared to receive both a RTM_NEWLINK reply _and_
the ACK unless they have set NLM_F_ECHO in the original request.

You can already reliably retrieve the ifindex by listening to
RTNLGRP_LINK messages and be notified about the link created
including all follow-up renames.

2014-01-31 00:57:46

by Tom Gundersen

Subject: Re: [PATCH] rtnetlink: return the newly created link in response to newlink

Hi Thomas,

Thanks for your reply.

On Thu, Jan 30, 2014 at 3:27 PM, Thomas Graf <[email protected]> wrote:
> On 01/30/14 at 02:05pm, Tom Gundersen wrote:
>> Userspace needs to reliably know the ifindex of the netdevs it creates,
>> as we cannot rely on the ifname staying unchanged.
>>
>> Earlier, a simlpe NLMSG_ERROR would be returned, but this returns the
>> corresponding RTM_NEWLINK on success instead.
>
> This breaks existing Netlink applications in user space. User space
> apps are not prepared to receive both a RTM_NEWLINK reply _and_
> the ACK unless they have set NLM_F_ECHO in the original request.
>
> You can already reliably retrieve the ifindex by listening to
> RTNLGRP_LINK messages and be notified about the link created
> including all follow-up renames.

Ok, we'll keep doing this instead.

Cheers,

Tom