Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net-next 0/2] Ease nsid allocation
@ 2019-09-30 16:02 Nicolas Dichtel
  2019-09-30 16:02 ` [PATCH net-next 1/2] netns: move rtnl_net_get_size() and rtnl_net_fill() Nicolas Dichtel
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Nicolas Dichtel @ 2019-09-30 16:02 UTC (permalink / raw)
  To: davem; +Cc: netdev


The goal of the series is to ease nsid allocation from userland.
The first patch is a preparation work and the second enables to receive the
new nsid in the answer to RTM_NEWNSID.

 net/core/net_namespace.c | 118 ++++++++++++++++++++++++++++-------------------
 1 file changed, 71 insertions(+), 47 deletions(-)

Comments are welcomed,
Nicolas


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH net-next 1/2] netns: move rtnl_net_get_size() and rtnl_net_fill()
  2019-09-30 16:02 [PATCH net-next 0/2] Ease nsid allocation Nicolas Dichtel
@ 2019-09-30 16:02 ` Nicolas Dichtel
  2019-09-30 16:02 ` [PATCH net-next 2/2] netns/rtnl: return the new nsid to the user Nicolas Dichtel
  2019-10-02  1:20 ` [PATCH net-next 0/2] Ease nsid allocation David Miller
  2 siblings, 0 replies; 16+ messages in thread
From: Nicolas Dichtel @ 2019-09-30 16:02 UTC (permalink / raw)
  To: davem; +Cc: netdev, Nicolas Dichtel

There is no functional change in this patch, it only prepares the next one
where rtnl_net_newid() will use rtnl_net_get_size() and rtnl_net_fill().

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/core/net_namespace.c | 92 ++++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 46 deletions(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index a0e0d298c991..8f5fa5d5becd 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -716,6 +716,52 @@ static const struct nla_policy rtnl_net_policy[NETNSA_MAX + 1] = {
 	[NETNSA_TARGET_NSID]	= { .type = NLA_S32 },
 };
 
+struct net_fill_args {
+	u32 portid;
+	u32 seq;
+	int flags;
+	int cmd;
+	int nsid;
+	bool add_ref;
+	int ref_nsid;
+};
+
+static int rtnl_net_get_size(void)
+{
+	return NLMSG_ALIGN(sizeof(struct rtgenmsg))
+	       + nla_total_size(sizeof(s32)) /* NETNSA_NSID */
+	       + nla_total_size(sizeof(s32)) /* NETNSA_CURRENT_NSID */
+	       ;
+}
+
+static int rtnl_net_fill(struct sk_buff *skb, struct net_fill_args *args)
+{
+	struct nlmsghdr *nlh;
+	struct rtgenmsg *rth;
+
+	nlh = nlmsg_put(skb, args->portid, args->seq, args->cmd, sizeof(*rth),
+			args->flags);
+	if (!nlh)
+		return -EMSGSIZE;
+
+	rth = nlmsg_data(nlh);
+	rth->rtgen_family = AF_UNSPEC;
+
+	if (nla_put_s32(skb, NETNSA_NSID, args->nsid))
+		goto nla_put_failure;
+
+	if (args->add_ref &&
+	    nla_put_s32(skb, NETNSA_CURRENT_NSID, args->ref_nsid))
+		goto nla_put_failure;
+
+	nlmsg_end(skb, nlh);
+	return 0;
+
+nla_put_failure:
+	nlmsg_cancel(skb, nlh);
+	return -EMSGSIZE;
+}
+
 static int rtnl_net_newid(struct sk_buff *skb, struct nlmsghdr *nlh,
 			  struct netlink_ext_ack *extack)
 {
@@ -776,52 +822,6 @@ static int rtnl_net_newid(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return err;
 }
 
-static int rtnl_net_get_size(void)
-{
-	return NLMSG_ALIGN(sizeof(struct rtgenmsg))
-	       + nla_total_size(sizeof(s32)) /* NETNSA_NSID */
-	       + nla_total_size(sizeof(s32)) /* NETNSA_CURRENT_NSID */
-	       ;
-}
-
-struct net_fill_args {
-	u32 portid;
-	u32 seq;
-	int flags;
-	int cmd;
-	int nsid;
-	bool add_ref;
-	int ref_nsid;
-};
-
-static int rtnl_net_fill(struct sk_buff *skb, struct net_fill_args *args)
-{
-	struct nlmsghdr *nlh;
-	struct rtgenmsg *rth;
-
-	nlh = nlmsg_put(skb, args->portid, args->seq, args->cmd, sizeof(*rth),
-			args->flags);
-	if (!nlh)
-		return -EMSGSIZE;
-
-	rth = nlmsg_data(nlh);
-	rth->rtgen_family = AF_UNSPEC;
-
-	if (nla_put_s32(skb, NETNSA_NSID, args->nsid))
-		goto nla_put_failure;
-
-	if (args->add_ref &&
-	    nla_put_s32(skb, NETNSA_CURRENT_NSID, args->ref_nsid))
-		goto nla_put_failure;
-
-	nlmsg_end(skb, nlh);
-	return 0;
-
-nla_put_failure:
-	nlmsg_cancel(skb, nlh);
-	return -EMSGSIZE;
-}
-
 static int rtnl_net_valid_getid_req(struct sk_buff *skb,
 				    const struct nlmsghdr *nlh,
 				    struct nlattr **tb,
-- 
2.23.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH net-next 2/2] netns/rtnl: return the new nsid to the user
  2019-09-30 16:02 [PATCH net-next 0/2] Ease nsid allocation Nicolas Dichtel
  2019-09-30 16:02 ` [PATCH net-next 1/2] netns: move rtnl_net_get_size() and rtnl_net_fill() Nicolas Dichtel
@ 2019-09-30 16:02 ` Nicolas Dichtel
  2019-10-02  1:20 ` [PATCH net-next 0/2] Ease nsid allocation David Miller
  2 siblings, 0 replies; 16+ messages in thread
From: Nicolas Dichtel @ 2019-09-30 16:02 UTC (permalink / raw)
  To: davem; +Cc: netdev, Nicolas Dichtel

When the user asks for a new nsid, he can let the kernel choose it (by
providing -1 in NETNSA_NSID). In this case, it's useful to answer to the
netlink message with the chosen nsid.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/core/net_namespace.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 8f5fa5d5becd..266d095296f3 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -810,8 +810,32 @@ static int rtnl_net_newid(struct sk_buff *skb, struct nlmsghdr *nlh,
 	err = alloc_netid(net, peer, nsid);
 	spin_unlock_bh(&net->nsid_lock);
 	if (err >= 0) {
+		struct net_fill_args fillargs = {
+			.portid = NETLINK_CB(skb).portid,
+			.seq = nlh->nlmsg_seq,
+			.cmd = RTM_NEWNSID,
+			.nsid = err,
+		};
+		struct sk_buff *msg;
+
+		/* The id has been allocated, thus first notify listeners */
 		rtnl_net_notifyid(net, RTM_NEWNSID, err);
-		err = 0;
+
+		/* Then, try to send the new nsid to the sender */
+		msg = nlmsg_new(rtnl_net_get_size(), GFP_KERNEL);
+		if (!msg) {
+			err = -ENOMEM;
+			NL_SET_ERR_MSG(extack, "Unable to alloc reply msg");
+			goto out;
+		}
+
+		err = rtnl_net_fill(msg, &fillargs);
+		if (err < 0) {
+			kfree_skb(msg);
+			goto out;
+		}
+
+		err = rtnl_unicast(msg, net, NETLINK_CB(skb).portid);
 	} else if (err == -ENOSPC && nsid >= 0) {
 		err = -EEXIST;
 		NL_SET_BAD_ATTR(extack, tb[NETNSA_NSID]);
-- 
2.23.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next 0/2] Ease nsid allocation
  2019-09-30 16:02 [PATCH net-next 0/2] Ease nsid allocation Nicolas Dichtel
  2019-09-30 16:02 ` [PATCH net-next 1/2] netns: move rtnl_net_get_size() and rtnl_net_fill() Nicolas Dichtel
  2019-09-30 16:02 ` [PATCH net-next 2/2] netns/rtnl: return the new nsid to the user Nicolas Dichtel
@ 2019-10-02  1:20 ` David Miller
  2019-10-02  8:46   ` Nicolas Dichtel
  2 siblings, 1 reply; 16+ messages in thread
From: David Miller @ 2019-10-02  1:20 UTC (permalink / raw)
  To: nicolas.dichtel; +Cc: netdev

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Mon, 30 Sep 2019 18:02:12 +0200

> The goal of the series is to ease nsid allocation from userland.
> The first patch is a preparation work and the second enables to receive the
> new nsid in the answer to RTM_NEWNSID.

The new reply message could break existing apps.

If an app only performs netnsid operations, and fills up the receive
queue because it isn't reading these new replies (it had no reason to,
they didn't exist previously), operations will start failing that
would not fail previously because the receive queue is full.

Given this, I don't see how we can make the change.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next 0/2] Ease nsid allocation
  2019-10-02  1:20 ` [PATCH net-next 0/2] Ease nsid allocation David Miller
@ 2019-10-02  8:46   ` Nicolas Dichtel
  2019-10-02 14:58     ` David Miller
  2019-10-03 16:19     ` Guillaume Nault
  0 siblings, 2 replies; 16+ messages in thread
From: Nicolas Dichtel @ 2019-10-02  8:46 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Le 02/10/2019 à 03:20, David Miller a écrit :
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Mon, 30 Sep 2019 18:02:12 +0200
> 
>> The goal of the series is to ease nsid allocation from userland.
>> The first patch is a preparation work and the second enables to receive the
>> new nsid in the answer to RTM_NEWNSID.
> 
> The new reply message could break existing apps.
> 
> If an app only performs netnsid operations, and fills up the receive
> queue because it isn't reading these new replies (it had no reason to,
> they didn't exist previously), operations will start failing that
> would not fail previously because the receive queue is full.
Yes I see the problem. I was wondering if this was acceptable because the nl ack
is sent at the end. But nl ack are optional :/

> 
> Given this, I don't see how we can make the change.
> 
Is a new flag attribute ok to turn on this reply?


Thank you,
Nicolas

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next 0/2] Ease nsid allocation
  2019-10-02  8:46   ` Nicolas Dichtel
@ 2019-10-02 14:58     ` David Miller
  2019-10-03 16:19     ` Guillaume Nault
  1 sibling, 0 replies; 16+ messages in thread
From: David Miller @ 2019-10-02 14:58 UTC (permalink / raw)
  To: nicolas.dichtel; +Cc: netdev

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Wed, 2 Oct 2019 10:46:03 +0200

> Is a new flag attribute ok to turn on this reply?

That might work, yes.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next 0/2] Ease nsid allocation
  2019-10-02  8:46   ` Nicolas Dichtel
  2019-10-02 14:58     ` David Miller
@ 2019-10-03 16:19     ` Guillaume Nault
  2019-10-04 15:45       ` Nicolas Dichtel
  2019-10-07 11:58       ` [PATCH net] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID Nicolas Dichtel
  1 sibling, 2 replies; 16+ messages in thread
From: Guillaume Nault @ 2019-10-03 16:19 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: David Miller, netdev

On Wed, Oct 02, 2019 at 10:46:03AM +0200, Nicolas Dichtel wrote:
> Le 02/10/2019 à 03:20, David Miller a écrit :
> > From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> > Date: Mon, 30 Sep 2019 18:02:12 +0200
> > 
> >> The goal of the series is to ease nsid allocation from userland.
> >> The first patch is a preparation work and the second enables to receive the
> >> new nsid in the answer to RTM_NEWNSID.
> > 
> > The new reply message could break existing apps.
> > 
> > If an app only performs netnsid operations, and fills up the receive
> > queue because it isn't reading these new replies (it had no reason to,
> > they didn't exist previously), operations will start failing that
> > would not fail previously because the receive queue is full.
> Yes I see the problem. I was wondering if this was acceptable because the nl ack
> is sent at the end. But nl ack are optional :/
> 
> > 
> > Given this, I don't see how we can make the change.
> > 
> Is a new flag attribute ok to turn on this reply?
> 
Why not using the existing NLM_F_ECHO mechanism?

IIUC, if rtnl_net_notifyid() did pass the proper nlmsghdr and portid to
rtnl_notify(), the later would automatically notify the caller with
updated information if the original request had the NLM_F_ECHO flag.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next 0/2] Ease nsid allocation
  2019-10-03 16:19     ` Guillaume Nault
@ 2019-10-04 15:45       ` Nicolas Dichtel
  2019-10-08 23:00         ` Guillaume Nault
  2019-10-07 11:58       ` [PATCH net] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID Nicolas Dichtel
  1 sibling, 1 reply; 16+ messages in thread
From: Nicolas Dichtel @ 2019-10-04 15:45 UTC (permalink / raw)
  To: Guillaume Nault; +Cc: David Miller, netdev

Le 03/10/2019 à 18:19, Guillaume Nault a écrit :
[snip]
> Why not using the existing NLM_F_ECHO mechanism?
> 
> IIUC, if rtnl_net_notifyid() did pass the proper nlmsghdr and portid to
> rtnl_notify(), the later would automatically notify the caller with
> updated information if the original request had the NLM_F_ECHO flag.
> 
Good point. Note that with library like libnl, the auto sequence number check
will fail (seq number is 0 instead of the previous + 1) and thus must be bypassed.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH net] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID
  2019-10-03 16:19     ` Guillaume Nault
  2019-10-04 15:45       ` Nicolas Dichtel
@ 2019-10-07 11:58       ` Nicolas Dichtel
  2019-10-08 23:10         ` Guillaume Nault
  1 sibling, 1 reply; 16+ messages in thread
From: Nicolas Dichtel @ 2019-10-07 11:58 UTC (permalink / raw)
  To: davem; +Cc: gnault, netdev, Nicolas Dichtel

The flag NLM_F_ECHO aims to reply to the user the message notified to all
listeners.
It was not the case with the command RTM_NEWNSID, let's fix this.

Fixes: 0c7aecd4bde4 ("netns: add rtnl cmd to add and get peer netns ids")
Reported-by: Guillaume Nault <gnault@redhat.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/core/net_namespace.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index a0e0d298c991..f496ce0e8da8 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -245,7 +245,8 @@ static int __peernet2id(struct net *net, struct net *peer)
 	return __peernet2id_alloc(net, peer, &no);
 }
 
-static void rtnl_net_notifyid(struct net *net, int cmd, int id);
+static void rtnl_net_notifyid(struct net *net, int cmd, int id, u32 portid,
+			      struct nlmsghdr *nlh);
 /* This function returns the id of a peer netns. If no id is assigned, one will
  * be allocated and returned.
  */
@@ -268,7 +269,7 @@ int peernet2id_alloc(struct net *net, struct net *peer)
 	id = __peernet2id_alloc(net, peer, &alloc);
 	spin_unlock_bh(&net->nsid_lock);
 	if (alloc && id >= 0)
-		rtnl_net_notifyid(net, RTM_NEWNSID, id);
+		rtnl_net_notifyid(net, RTM_NEWNSID, id, 0, NULL);
 	if (alive)
 		put_net(peer);
 	return id;
@@ -532,7 +533,7 @@ static void unhash_nsid(struct net *net, struct net *last)
 			idr_remove(&tmp->netns_ids, id);
 		spin_unlock_bh(&tmp->nsid_lock);
 		if (id >= 0)
-			rtnl_net_notifyid(tmp, RTM_DELNSID, id);
+			rtnl_net_notifyid(tmp, RTM_DELNSID, id, 0, NULL);
 		if (tmp == last)
 			break;
 	}
@@ -764,7 +765,8 @@ static int rtnl_net_newid(struct sk_buff *skb, struct nlmsghdr *nlh,
 	err = alloc_netid(net, peer, nsid);
 	spin_unlock_bh(&net->nsid_lock);
 	if (err >= 0) {
-		rtnl_net_notifyid(net, RTM_NEWNSID, err);
+		rtnl_net_notifyid(net, RTM_NEWNSID, err, NETLINK_CB(skb).portid,
+				  nlh);
 		err = 0;
 	} else if (err == -ENOSPC && nsid >= 0) {
 		err = -EEXIST;
@@ -1051,7 +1053,8 @@ static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
 	return err < 0 ? err : skb->len;
 }
 
-static void rtnl_net_notifyid(struct net *net, int cmd, int id)
+static void rtnl_net_notifyid(struct net *net, int cmd, int id, u32 portid,
+			      struct nlmsghdr *nlh)
 {
 	struct net_fill_args fillargs = {
 		.cmd = cmd,
@@ -1068,7 +1071,7 @@ static void rtnl_net_notifyid(struct net *net, int cmd, int id)
 	if (err < 0)
 		goto err_out;
 
-	rtnl_notify(msg, net, 0, RTNLGRP_NSID, NULL, 0);
+	rtnl_notify(msg, net, portid, RTNLGRP_NSID, nlh, 0);
 	return;
 
 err_out:
-- 
2.23.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next 0/2] Ease nsid allocation
  2019-10-04 15:45       ` Nicolas Dichtel
@ 2019-10-08 23:00         ` Guillaume Nault
  0 siblings, 0 replies; 16+ messages in thread
From: Guillaume Nault @ 2019-10-08 23:00 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: David Miller, netdev

On Fri, Oct 04, 2019 at 05:45:22PM +0200, Nicolas Dichtel wrote:
> Le 03/10/2019 à 18:19, Guillaume Nault a écrit :
> [snip]
> > Why not using the existing NLM_F_ECHO mechanism?
> > 
> > IIUC, if rtnl_net_notifyid() did pass the proper nlmsghdr and portid to
> > rtnl_notify(), the later would automatically notify the caller with
> > updated information if the original request had the NLM_F_ECHO flag.
> > 
> Good point. Note that with library like libnl, the auto sequence number check
> will fail (seq number is 0 instead of the previous + 1) and thus must be bypassed.
>
Well, it's up to the caller of rtnl_notify() to build a netlink message
with proper sequence number. Currently, rtnl_net_notifyid() always sets
it to 0. That's probably why the libnl test fails.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID
  2019-10-07 11:58       ` [PATCH net] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID Nicolas Dichtel
@ 2019-10-08 23:10         ` Guillaume Nault
  2019-10-09  8:07           ` Nicolas Dichtel
  2019-10-09  9:19           ` [PATCH net v2] " Nicolas Dichtel
  0 siblings, 2 replies; 16+ messages in thread
From: Guillaume Nault @ 2019-10-08 23:10 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: davem, netdev

On Mon, Oct 07, 2019 at 01:58:35PM +0200, Nicolas Dichtel wrote:
> The flag NLM_F_ECHO aims to reply to the user the message notified to all
> listeners.
> It was not the case with the command RTM_NEWNSID, let's fix this.
> 
> Fixes: 0c7aecd4bde4 ("netns: add rtnl cmd to add and get peer netns ids")
> Reported-by: Guillaume Nault <gnault@redhat.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>  net/core/net_namespace.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index a0e0d298c991..f496ce0e8da8 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> -static void rtnl_net_notifyid(struct net *net, int cmd, int id)
> +static void rtnl_net_notifyid(struct net *net, int cmd, int id, u32 portid,
> +			      struct nlmsghdr *nlh)
>  {
>  	struct net_fill_args fillargs = {
>  		.cmd = cmd,

We also need to set .portid and .seq otherwise rtnl_net_fill() builds
a netlink message with invalid port id and sequence number (as you
noted in your previous message).


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID
  2019-10-08 23:10         ` Guillaume Nault
@ 2019-10-09  8:07           ` Nicolas Dichtel
  2019-10-09 13:48             ` Guillaume Nault
  2019-10-09  9:19           ` [PATCH net v2] " Nicolas Dichtel
  1 sibling, 1 reply; 16+ messages in thread
From: Nicolas Dichtel @ 2019-10-09  8:07 UTC (permalink / raw)
  To: Guillaume Nault; +Cc: davem, netdev

Le 09/10/2019 à 01:10, Guillaume Nault a écrit :
[snip]
> We also need to set .portid and .seq otherwise rtnl_net_fill() builds
> a netlink message with invalid port id and sequence number (as you
> noted in your previous message).
> 
Yes you're right. I don't know why, I had in mind that nl msg sent by the kernel
should have the portid and seq number set to 0.
Will send a v2.


Thank you,
Nicolas

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH net v2] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID
  2019-10-08 23:10         ` Guillaume Nault
  2019-10-09  8:07           ` Nicolas Dichtel
@ 2019-10-09  9:19           ` " Nicolas Dichtel
  2019-10-09 13:48             ` Guillaume Nault
  2019-10-10  4:06             ` Jakub Kicinski
  1 sibling, 2 replies; 16+ messages in thread
From: Nicolas Dichtel @ 2019-10-09  9:19 UTC (permalink / raw)
  To: davem; +Cc: gnault, netdev, Nicolas Dichtel

The flag NLM_F_ECHO aims to reply to the user the message notified to all
listeners.
It was not the case with the command RTM_NEWNSID, let's fix this.

Fixes: 0c7aecd4bde4 ("netns: add rtnl cmd to add and get peer netns ids")
Reported-by: Guillaume Nault <gnault@redhat.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---

v2:
  fix portid and seq number of the nl msg sent by the kernel

 net/core/net_namespace.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index a0e0d298c991..6d3e4821b02d 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -245,7 +245,8 @@ static int __peernet2id(struct net *net, struct net *peer)
 	return __peernet2id_alloc(net, peer, &no);
 }
 
-static void rtnl_net_notifyid(struct net *net, int cmd, int id);
+static void rtnl_net_notifyid(struct net *net, int cmd, int id, u32 portid,
+			      struct nlmsghdr *nlh);
 /* This function returns the id of a peer netns. If no id is assigned, one will
  * be allocated and returned.
  */
@@ -268,7 +269,7 @@ int peernet2id_alloc(struct net *net, struct net *peer)
 	id = __peernet2id_alloc(net, peer, &alloc);
 	spin_unlock_bh(&net->nsid_lock);
 	if (alloc && id >= 0)
-		rtnl_net_notifyid(net, RTM_NEWNSID, id);
+		rtnl_net_notifyid(net, RTM_NEWNSID, id, 0, NULL);
 	if (alive)
 		put_net(peer);
 	return id;
@@ -532,7 +533,7 @@ static void unhash_nsid(struct net *net, struct net *last)
 			idr_remove(&tmp->netns_ids, id);
 		spin_unlock_bh(&tmp->nsid_lock);
 		if (id >= 0)
-			rtnl_net_notifyid(tmp, RTM_DELNSID, id);
+			rtnl_net_notifyid(tmp, RTM_DELNSID, id, 0, NULL);
 		if (tmp == last)
 			break;
 	}
@@ -764,7 +765,8 @@ static int rtnl_net_newid(struct sk_buff *skb, struct nlmsghdr *nlh,
 	err = alloc_netid(net, peer, nsid);
 	spin_unlock_bh(&net->nsid_lock);
 	if (err >= 0) {
-		rtnl_net_notifyid(net, RTM_NEWNSID, err);
+		rtnl_net_notifyid(net, RTM_NEWNSID, err, NETLINK_CB(skb).portid,
+				  nlh);
 		err = 0;
 	} else if (err == -ENOSPC && nsid >= 0) {
 		err = -EEXIST;
@@ -1051,9 +1053,12 @@ static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
 	return err < 0 ? err : skb->len;
 }
 
-static void rtnl_net_notifyid(struct net *net, int cmd, int id)
+static void rtnl_net_notifyid(struct net *net, int cmd, int id, u32 portid,
+			      struct nlmsghdr *nlh)
 {
 	struct net_fill_args fillargs = {
+		.portid = portid,
+		.seq = nlh ? nlh->nlmsg_seq : 0,
 		.cmd = cmd,
 		.nsid = id,
 	};
@@ -1068,7 +1073,7 @@ static void rtnl_net_notifyid(struct net *net, int cmd, int id)
 	if (err < 0)
 		goto err_out;
 
-	rtnl_notify(msg, net, 0, RTNLGRP_NSID, NULL, 0);
+	rtnl_notify(msg, net, portid, RTNLGRP_NSID, nlh, 0);
 	return;
 
 err_out:
-- 
2.23.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID
  2019-10-09  8:07           ` Nicolas Dichtel
@ 2019-10-09 13:48             ` Guillaume Nault
  0 siblings, 0 replies; 16+ messages in thread
From: Guillaume Nault @ 2019-10-09 13:48 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: davem, netdev

On Wed, Oct 09, 2019 at 10:07:56AM +0200, Nicolas Dichtel wrote:
> Le 09/10/2019 à 01:10, Guillaume Nault a écrit :
> [snip]
> > We also need to set .portid and .seq otherwise rtnl_net_fill() builds
> > a netlink message with invalid port id and sequence number (as you
> > noted in your previous message).
> > 
> Yes you're right. I don't know why, I had in mind that nl msg sent by the kernel
> should have the portid and seq number set to 0.
> Will send a v2.
> 
I guess this idea comes from the fact that portid and seq don't carry
any meaningful information when the message is sent to a multicast
group.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net v2] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID
  2019-10-09  9:19           ` [PATCH net v2] " Nicolas Dichtel
@ 2019-10-09 13:48             ` Guillaume Nault
  2019-10-10  4:06             ` Jakub Kicinski
  1 sibling, 0 replies; 16+ messages in thread
From: Guillaume Nault @ 2019-10-09 13:48 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: davem, netdev

On Wed, Oct 09, 2019 at 11:19:10AM +0200, Nicolas Dichtel wrote:
> The flag NLM_F_ECHO aims to reply to the user the message notified to all
> listeners.
> It was not the case with the command RTM_NEWNSID, let's fix this.
> 
Acked-by: Guillaume Nault <gnault@redhat.com>
Tested-by: Guillaume Nault <gnault@redhat.com>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net v2] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID
  2019-10-09  9:19           ` [PATCH net v2] " Nicolas Dichtel
  2019-10-09 13:48             ` Guillaume Nault
@ 2019-10-10  4:06             ` Jakub Kicinski
  1 sibling, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2019-10-10  4:06 UTC (permalink / raw)
  To: Nicolas Dichtel, gnault; +Cc: davem, netdev

On Wed,  9 Oct 2019 11:19:10 +0200, Nicolas Dichtel wrote:
> The flag NLM_F_ECHO aims to reply to the user the message notified to all
> listeners.
> It was not the case with the command RTM_NEWNSID, let's fix this.
> 
> Fixes: 0c7aecd4bde4 ("netns: add rtnl cmd to add and get peer netns ids")
> Reported-by: Guillaume Nault <gnault@redhat.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Applied, thanks! 

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, back to index

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30 16:02 [PATCH net-next 0/2] Ease nsid allocation Nicolas Dichtel
2019-09-30 16:02 ` [PATCH net-next 1/2] netns: move rtnl_net_get_size() and rtnl_net_fill() Nicolas Dichtel
2019-09-30 16:02 ` [PATCH net-next 2/2] netns/rtnl: return the new nsid to the user Nicolas Dichtel
2019-10-02  1:20 ` [PATCH net-next 0/2] Ease nsid allocation David Miller
2019-10-02  8:46   ` Nicolas Dichtel
2019-10-02 14:58     ` David Miller
2019-10-03 16:19     ` Guillaume Nault
2019-10-04 15:45       ` Nicolas Dichtel
2019-10-08 23:00         ` Guillaume Nault
2019-10-07 11:58       ` [PATCH net] netns: fix NLM_F_ECHO mechanism for RTM_NEWNSID Nicolas Dichtel
2019-10-08 23:10         ` Guillaume Nault
2019-10-09  8:07           ` Nicolas Dichtel
2019-10-09 13:48             ` Guillaume Nault
2019-10-09  9:19           ` [PATCH net v2] " Nicolas Dichtel
2019-10-09 13:48             ` Guillaume Nault
2019-10-10  4:06             ` Jakub Kicinski

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox