diff mbox

rtnetlink: return the newly created link in response to newlink

Message ID 1391087144-24490-1-git-send-email-teg@jklm.no
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Tom Gundersen Jan. 30, 2014, 1:05 p.m. UTC 
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 <teg@jklm.no>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: David S. Miller <davem@davemloft.net>
---
 net/core/rtnetlink.c | 100 ++++++++++++++++++++++++++-------------------------
 1 file changed, 52 insertions(+), 48 deletions(-)

Comments

Thomas Graf Jan. 30, 2014, 2:27 p.m. UTC | #1 
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.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tom Gundersen Jan. 31, 2014, 12:57 a.m. UTC | #2 
Hi Thomas,

Thanks for your reply.

On Thu, Jan 30, 2014 at 3:27 PM, Thomas Graf <tgraf@suug.ch> 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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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);