[PATCHv2 net-next] rtnetlink: Honour NLM_F_ECHO flag in rtnl_{new, set, del}link

Subsystems: networking [general], the rest

4 messages, 2 authors, 2022-09-26 · open the first message on its own page

[PATCHv2 net-next] rtnetlink: Honour NLM_F_ECHO flag in rtnl_{new, set, del}link

From: Hangbin Liu <hidden>
Date: 2022-09-26 07:13:56

Netlink messages are used for communicating between user and kernel space.
When user space configures the kernel with netlink messages, it can set the
NLM_F_ECHO flag to request the kernel to send the applied configuration back
to the caller. This allows user space to retrieve configuration information
that are filled by the kernel (either because these parameters can only be
set by the kernel or because user space let the kernel choose a default
value).

The kernel has support this feature in some places like RTM_{NEW, DEL}ADDR,
RTM_{NEW, DEL}ROUTE. This patch handles NLM_F_ECHO flag and send link info
back after rtnl_{new, set, del}link.

Suggested-by: Guillaume Nault <redacted>
Signed-off-by: Hangbin Liu <redacted>
---
v2:
1) rename rtnl_echo_link_info() to rtnl_link_notify().
2) remove IFLA_LINK_NETNSID and IFLA_EXT_MASK, which do not fit here.
3) Add NLM_F_ECHO in rtnl_dellink. But we can't re-use the rtnl_link_notify()
   helper as we need to get the link info before rtnl_delete_link().
---
 net/core/rtnetlink.c | 66 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 58 insertions(+), 8 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 74864dc46a7e..0897cb6cc931 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2645,13 +2645,41 @@ static int do_set_proto_down(struct net_device *dev,
 	return 0;
 }
 
+static void rtnl_link_notify(struct net_device *dev, u32 pid,
+			     struct nlmsghdr *nlh)
+{
+	struct sk_buff *skb;
+	int err = -ENOBUFS;
+
+	skb = nlmsg_new(if_nlmsg_size(dev, 0), GFP_KERNEL);
+	if (!skb)
+		goto errout;
+
+	err = rtnl_fill_ifinfo(skb, dev, dev_net(dev), RTM_NEWLINK, pid,
+			       nlh->nlmsg_seq, 0, 0, 0, 0, NULL, 0, 0,
+			       GFP_KERNEL);
+	if (err < 0) {
+		/* -EMSGSIZE implies BUG in if_nlmsg_size */
+		WARN_ON(err == -EMSGSIZE);
+		kfree_skb(skb);
+		goto errout;
+	}
+
+	rtnl_notify(skb, dev_net(dev), pid, RTM_NEWLINK, nlh, GFP_KERNEL);
+
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(dev_net(dev), RTM_NEWLINK, err);
+}
+
 #define DO_SETLINK_MODIFIED	0x01
 /* notify flag means notify + modified. */
 #define DO_SETLINK_NOTIFY	0x03
 static int do_setlink(const struct sk_buff *skb,
 		      struct net_device *dev, struct ifinfomsg *ifm,
 		      struct netlink_ext_ack *extack,
-		      struct nlattr **tb, int status)
+		      struct nlattr **tb, int status,
+		      struct nlmsghdr *nlh)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
 	char ifname[IFNAMSIZ];
@@ -3009,6 +3037,8 @@ static int do_setlink(const struct sk_buff *skb,
 		}
 	}
 
+	rtnl_link_notify(dev, NETLINK_CB(skb).portid, nlh);
+
 errout:
 	if (status & DO_SETLINK_MODIFIED) {
 		if ((status & DO_SETLINK_NOTIFY) == DO_SETLINK_NOTIFY)
@@ -3069,7 +3099,8 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto errout;
 	}
 
-	err = do_setlink(skb, dev, ifm, extack, tb, 0);
+	err = do_setlink(skb, dev, ifm, extack, tb, 0, nlh);
+
 errout:
 	return err;
 }
@@ -3130,10 +3161,12 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh,
 			struct netlink_ext_ack *extack)
 {
 	struct net *net = sock_net(skb->sk);
+	u32 pid = NETLINK_CB(skb).portid;
 	struct net *tgt_net = net;
 	struct net_device *dev = NULL;
 	struct ifinfomsg *ifm;
 	struct nlattr *tb[IFLA_MAX+1];
+	struct sk_buff *nskb;
 	int err;
 	int netnsid = -1;
 
@@ -3171,7 +3204,20 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out;
 	}
 
+	nskb = nlmsg_new(if_nlmsg_size(dev, 0), GFP_KERNEL);
+	if (nskb) {
+		err = rtnl_fill_ifinfo(nskb, dev, dev_net(dev), RTM_DELLINK, pid,
+				       nlh->nlmsg_seq, 0, 0, 0, 0, NULL, 0, 0,
+				       GFP_KERNEL);
+		if (err < 0) {
+			WARN_ON(err == -EMSGSIZE);
+			kfree_skb(nskb);
+		}
+	}
+
 	err = rtnl_delete_link(dev);
+	if (!err && nskb)
+		rtnl_notify(nskb, net, pid, RTM_DELLINK, nlh, GFP_KERNEL);
 
 out:
 	if (netnsid >= 0)
@@ -3293,14 +3339,14 @@ static int rtnl_group_changelink(const struct sk_buff *skb,
 		struct net *net, int group,
 		struct ifinfomsg *ifm,
 		struct netlink_ext_ack *extack,
-		struct nlattr **tb)
+		struct nlattr **tb, struct nlmsghdr *nlh)
 {
 	struct net_device *dev, *aux;
 	int err;
 
 	for_each_netdev_safe(net, dev, aux) {
 		if (dev->group == group) {
-			err = do_setlink(skb, dev, ifm, extack, tb, 0);
+			err = do_setlink(skb, dev, ifm, extack, tb, 0, nlh);
 			if (err < 0)
 				return err;
 		}
@@ -3312,7 +3358,8 @@ static int rtnl_group_changelink(const struct sk_buff *skb,
 static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm,
 			       const struct rtnl_link_ops *ops,
 			       struct nlattr **tb, struct nlattr **data,
-			       struct netlink_ext_ack *extack)
+			       struct netlink_ext_ack *extack,
+			       struct nlmsghdr *nlh)
 {
 	unsigned char name_assign_type = NET_NAME_USER;
 	struct net *net = sock_net(skb->sk);
@@ -3382,6 +3429,9 @@ static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm,
 		if (err)
 			goto out_unregister;
 	}
+
+	rtnl_link_notify(dev, NETLINK_CB(skb).portid, nlh);
+
 out:
 	if (link_net)
 		put_net(link_net);
@@ -3544,7 +3594,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 			status |= DO_SETLINK_NOTIFY;
 		}
 
-		return do_setlink(skb, dev, ifm, extack, tb, status);
+		return do_setlink(skb, dev, ifm, extack, tb, status, nlh);
 	}
 
 	if (!(nlh->nlmsg_flags & NLM_F_CREATE)) {
@@ -3556,7 +3606,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		if (tb[IFLA_GROUP])
 			return rtnl_group_changelink(skb, net,
 						nla_get_u32(tb[IFLA_GROUP]),
-						ifm, extack, tb);
+						ifm, extack, tb, nlh);
 		return -ENODEV;
 	}
 
@@ -3578,7 +3628,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		return -EOPNOTSUPP;
 	}
 
-	return rtnl_newlink_create(skb, ifm, ops, tb, data, extack);
+	return rtnl_newlink_create(skb, ifm, ops, tb, data, extack, nlh);
 }
 
 static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
-- 
2.37.2

Re: [PATCHv2 net-next] rtnetlink: Honour NLM_F_ECHO flag in rtnl_{new, set, del}link

From: Nicolas Dichtel <hidden>
Date: 2022-09-26 08:38:01

Le 26/09/2022 à 09:12, Hangbin Liu a écrit :
quoted hunk
Netlink messages are used for communicating between user and kernel space.
When user space configures the kernel with netlink messages, it can set the
NLM_F_ECHO flag to request the kernel to send the applied configuration back
to the caller. This allows user space to retrieve configuration information
that are filled by the kernel (either because these parameters can only be
set by the kernel or because user space let the kernel choose a default
value).

The kernel has support this feature in some places like RTM_{NEW, DEL}ADDR,
RTM_{NEW, DEL}ROUTE. This patch handles NLM_F_ECHO flag and send link info
back after rtnl_{new, set, del}link.

Suggested-by: Guillaume Nault <redacted>
Signed-off-by: Hangbin Liu <redacted>
---
v2:
1) rename rtnl_echo_link_info() to rtnl_link_notify().
2) remove IFLA_LINK_NETNSID and IFLA_EXT_MASK, which do not fit here.
3) Add NLM_F_ECHO in rtnl_dellink. But we can't re-use the rtnl_link_notify()
   helper as we need to get the link info before rtnl_delete_link().
---
 net/core/rtnetlink.c | 66 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 58 insertions(+), 8 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 74864dc46a7e..0897cb6cc931 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2645,13 +2645,41 @@ static int do_set_proto_down(struct net_device *dev,
 	return 0;
 }
 
+static void rtnl_link_notify(struct net_device *dev, u32 pid,
+			     struct nlmsghdr *nlh)
+{
+	struct sk_buff *skb;
+	int err = -ENOBUFS;
+
+	skb = nlmsg_new(if_nlmsg_size(dev, 0), GFP_KERNEL);
+	if (!skb)
+		goto errout;
+
+	err = rtnl_fill_ifinfo(skb, dev, dev_net(dev), RTM_NEWLINK, pid,
+			       nlh->nlmsg_seq, 0, 0, 0, 0, NULL, 0, 0,
+			       GFP_KERNEL);
+	if (err < 0) {
+		/* -EMSGSIZE implies BUG in if_nlmsg_size */
+		WARN_ON(err == -EMSGSIZE);
+		kfree_skb(skb);
+		goto errout;
+	}
+
+	rtnl_notify(skb, dev_net(dev), pid, RTM_NEWLINK, nlh, GFP_KERNEL);
The fourth argument is the group, not the command. It should be RTNLGRP_LINK
here. But it would be better to pass 0 to avoid multicasting a new / duplicate
notification. Calling functions already multicast one.

Re: [PATCHv2 net-next] rtnetlink: Honour NLM_F_ECHO flag in rtnl_{new, set, del}link

From: Hangbin Liu <hidden>
Date: 2022-09-26 09:09:21

On Mon, Sep 26, 2022 at 10:37:46AM +0200, Nicolas Dichtel wrote:
quoted
+	rtnl_notify(skb, dev_net(dev), pid, RTM_NEWLINK, nlh, GFP_KERNEL);
The fourth argument is the group, not the command. It should be RTNLGRP_LINK
here. But it would be better to pass 0 to avoid multicasting a new / duplicate
notification. Calling functions already multicast one.
Thanks for the correcting. I will post a new update.

Hangbin

Re: [PATCHv2 net-next] rtnetlink: Honour NLM_F_ECHO flag in rtnl_{new, set, del}link

From: Hangbin Liu <hidden>
Date: 2022-09-26 09:21:23

On Mon, Sep 26, 2022 at 03:12:46PM +0800, Hangbin Liu wrote:
+static void rtnl_link_notify(struct net_device *dev, u32 pid,
+			     struct nlmsghdr *nlh)
+{
+	struct sk_buff *skb;
+	int err = -ENOBUFS;
+
+	skb = nlmsg_new(if_nlmsg_size(dev, 0), GFP_KERNEL);
+	if (!skb)
+		goto errout;
+
+	err = rtnl_fill_ifinfo(skb, dev, dev_net(dev), RTM_NEWLINK, pid,
+			       nlh->nlmsg_seq, 0, 0, 0, 0, NULL, 0, 0,
+			       GFP_KERNEL);
+	if (err < 0) {
+		/* -EMSGSIZE implies BUG in if_nlmsg_size */
+		WARN_ON(err == -EMSGSIZE);
+		kfree_skb(skb);
+		goto errout;
+	}
+
+	rtnl_notify(skb, dev_net(dev), pid, RTM_NEWLINK, nlh, GFP_KERNEL);
+
+errout:
+	if (err < 0)
+		rtnl_set_sk_err(dev_net(dev), RTM_NEWLINK, err);
+}
+
Oh, I just find there is a similar helper function rtmsg_ifinfo_build_skb().
I will check if I can just update and re-use this helper.

Thanks
Hangbin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help