* [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace [not found] <cover.1535712096.git.lorenzo.bianconi@redhat.com> @ 2018-08-31 11:43 ` Lorenzo Bianconi 2018-08-31 15:24 ` David Ahern 0 siblings, 1 reply; 11+ messages in thread From: Lorenzo Bianconi @ 2018-08-31 11:43 UTC (permalink / raw) To: davem; +Cc: netdev When moving a veth device to another namespace, userspace receives a RTM_DELLINK message indicating the device has been removed from current netns. However, the other peer does not receive a netlink event containing new values for IFLA_LINK_NETNSID and IFLA_LINK veth attributes. Fix that behaviour sending to userspace a RTM_NEWLINK message in the peer namespace to report new IFLA_LINK_NETNSID/IFLA_LINK values Reviewed-by: Stefano Brivio <sbrivio@redhat.com> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- Changes since RFC: - export rtmsg_ifinfo_build_skb() symbol and do not use rtmsg_ifinfo_newnet() directly --- drivers/net/veth.c | 66 +++++++++++++++++++++++++++++++++++++++++++- net/core/rtnetlink.c | 1 + 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/drivers/net/veth.c b/drivers/net/veth.c index 8d679c8b7f25..0caffc93d74d 100644 --- a/drivers/net/veth.c +++ b/drivers/net/veth.c @@ -1242,18 +1242,82 @@ static struct rtnl_link_ops veth_link_ops = { .get_link_net = veth_get_link_net, }; +static int veth_notify(struct notifier_block *this, + unsigned long event, void *ptr) +{ + struct net_device *peer, *dev = netdev_notifier_info_to_dev(ptr); + struct net *peer_net, *net = dev_net(dev); + int nsid, ret = NOTIFY_DONE; + struct veth_priv *priv; + struct sk_buff *skb; + + if (dev->netdev_ops != &veth_netdev_ops) + return NOTIFY_DONE; + + if (event != NETDEV_REGISTER) + return NOTIFY_DONE; + + priv = netdev_priv(dev); + + rcu_read_lock(); + + peer = rcu_dereference(priv->peer); + if (!peer) + goto out; + + peer_net = dev_net(peer); + /* do not forward events if both veth devices + * are in the same namespace + */ + if (peer_net == net) + goto out; + + /* notify on peer namespace new IFLA_LINK_NETNSID + * and IFLA_LINK values + */ + nsid = peernet2id_alloc(peer_net, net); + skb = rtmsg_ifinfo_build_skb(RTM_NEWLINK, peer, ~0U, + IFLA_EVENT_NONE, GFP_ATOMIC, + &nsid, dev->ifindex); + if (skb) { + rtnl_notify(skb, peer_net, 0, RTNLGRP_LINK, NULL, + GFP_ATOMIC); + ret = NOTIFY_OK; + } + +out: + rcu_read_unlock(); + + return ret; +} + +static struct notifier_block veth_notifier = { + .notifier_call = veth_notify, +}; + /* * init/fini */ static __init int veth_init(void) { - return rtnl_link_register(&veth_link_ops); + int err; + + err = register_netdevice_notifier(&veth_notifier); + if (err < 0) + return err; + + err = rtnl_link_register(&veth_link_ops); + if (err < 0) + unregister_netdevice_notifier(&veth_notifier); + + return err; } static __exit void veth_exit(void) { rtnl_link_unregister(&veth_link_ops); + unregister_netdevice_notifier(&veth_notifier); } module_init(veth_init); diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 24431e578310..b2df84db7670 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3326,6 +3326,7 @@ struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev, rtnl_set_sk_err(net, RTNLGRP_LINK, err); return NULL; } +EXPORT_SYMBOL(rtmsg_ifinfo_build_skb); void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev, gfp_t flags) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace 2018-08-31 11:43 ` [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace Lorenzo Bianconi @ 2018-08-31 15:24 ` David Ahern 2018-08-31 16:19 ` Lorenzo Bianconi 0 siblings, 1 reply; 11+ messages in thread From: David Ahern @ 2018-08-31 15:24 UTC (permalink / raw) To: Lorenzo Bianconi, davem; +Cc: netdev On 8/31/18 5:43 AM, Lorenzo Bianconi wrote: > When moving a veth device to another namespace, userspace receives a > RTM_DELLINK message indicating the device has been removed from current > netns. However, the other peer does not receive a netlink event > containing new values for IFLA_LINK_NETNSID and IFLA_LINK veth > attributes. > Fix that behaviour sending to userspace a RTM_NEWLINK message in the peer > namespace to report new IFLA_LINK_NETNSID/IFLA_LINK values > A newlink message is generated in the new namespace. What information is missing from that message? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace 2018-08-31 15:24 ` David Ahern @ 2018-08-31 16:19 ` Lorenzo Bianconi 2018-08-31 16:21 ` David Ahern 0 siblings, 1 reply; 11+ messages in thread From: Lorenzo Bianconi @ 2018-08-31 16:19 UTC (permalink / raw) To: David Ahern; +Cc: davem, netdev > On 8/31/18 5:43 AM, Lorenzo Bianconi wrote: > > When moving a veth device to another namespace, userspace receives a > > RTM_DELLINK message indicating the device has been removed from current > > netns. However, the other peer does not receive a netlink event > > containing new values for IFLA_LINK_NETNSID and IFLA_LINK veth > > attributes. > > Fix that behaviour sending to userspace a RTM_NEWLINK message in the peer > > namespace to report new IFLA_LINK_NETNSID/IFLA_LINK values > > > > A newlink message is generated in the new namespace. What information is > missing from that message? > Hi David, let's assume we have two veth paired devices (veth0 and veth1) on inet namespace. When moving a veth1 to another namespace, userspace is notified with RTM_DELLINK event on inet namespace to indicate that veth1 has been moved to another namespace. However some userspace applications (e.g. NetworkManager), listening for events on inet namespace, are interested in veth1 ifindex in the new namespace. This patch sends a new RTM_NEWLINK event in inet namespace to provide new values for IFLA_LINK_NETNSID/IFLA_LINK Regards, Lorenzo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace 2018-08-31 16:19 ` Lorenzo Bianconi @ 2018-08-31 16:21 ` David Ahern 2018-08-31 16:54 ` Lorenzo Bianconi 0 siblings, 1 reply; 11+ messages in thread From: David Ahern @ 2018-08-31 16:21 UTC (permalink / raw) To: Lorenzo Bianconi; +Cc: davem, netdev On 8/31/18 10:19 AM, Lorenzo Bianconi wrote: >> On 8/31/18 5:43 AM, Lorenzo Bianconi wrote: >>> When moving a veth device to another namespace, userspace receives a >>> RTM_DELLINK message indicating the device has been removed from current >>> netns. However, the other peer does not receive a netlink event >>> containing new values for IFLA_LINK_NETNSID and IFLA_LINK veth >>> attributes. >>> Fix that behaviour sending to userspace a RTM_NEWLINK message in the peer >>> namespace to report new IFLA_LINK_NETNSID/IFLA_LINK values >>> >> >> A newlink message is generated in the new namespace. What information is >> missing from that message? >> > > Hi David, > > let's assume we have two veth paired devices (veth0 and veth1) on inet > namespace. When moving a veth1 to another namespace, userspace is notified > with RTM_DELLINK event on inet namespace to indicate that veth1 has been > moved to another namespace. However some userspace applications > (e.g. NetworkManager), listening for events on inet namespace, are interested > in veth1 ifindex in the new namespace. This patch sends a new RTM_NEWLINK event > in inet namespace to provide new values for IFLA_LINK_NETNSID/IFLA_LINK This is in init namespace $ ip li set veth2 netns foo $ ip monitor Deleted 20: veth2@veth1: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default link/ether c6:d0:d6:c5:23:7d brd ff:ff:ff:ff:ff:ff new-netns foo new-ifindex 20 It shows the new namespace in the delete message. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace 2018-08-31 16:21 ` David Ahern @ 2018-08-31 16:54 ` Lorenzo Bianconi 2018-09-01 9:05 ` Lorenzo Bianconi 0 siblings, 1 reply; 11+ messages in thread From: Lorenzo Bianconi @ 2018-08-31 16:54 UTC (permalink / raw) To: David Ahern; +Cc: davem, netdev > On 8/31/18 10:19 AM, Lorenzo Bianconi wrote: > >> On 8/31/18 5:43 AM, Lorenzo Bianconi wrote: > >>> When moving a veth device to another namespace, userspace receives a > >>> RTM_DELLINK message indicating the device has been removed from current > >>> netns. However, the other peer does not receive a netlink event > >>> containing new values for IFLA_LINK_NETNSID and IFLA_LINK veth > >>> attributes. > >>> Fix that behaviour sending to userspace a RTM_NEWLINK message in the peer > >>> namespace to report new IFLA_LINK_NETNSID/IFLA_LINK values > >>> > >> > >> A newlink message is generated in the new namespace. What information is > >> missing from that message? > >> > > > > Hi David, > > > > let's assume we have two veth paired devices (veth0 and veth1) on inet > > namespace. When moving a veth1 to another namespace, userspace is notified > > with RTM_DELLINK event on inet namespace to indicate that veth1 has been > > moved to another namespace. However some userspace applications > > (e.g. NetworkManager), listening for events on inet namespace, are interested > > in veth1 ifindex in the new namespace. This patch sends a new RTM_NEWLINK event > > in inet namespace to provide new values for IFLA_LINK_NETNSID/IFLA_LINK > > This is in init namespace > $ ip li set veth2 netns foo > > $ ip monitor > Deleted 20: veth2@veth1: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc > noop state DOWN group default > link/ether c6:d0:d6:c5:23:7d brd ff:ff:ff:ff:ff:ff new-netns foo > new-ifindex 20 > > It shows the new namespace in the delete message. Ops, I have not noticed this info has been already introduced in the commit 38e01b30563a ("dev: advertise the new ifindex when the netns iface changes"). Thanks for the hint. DaveM please drop this patch. Regards, Lorenzo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace 2018-08-31 16:54 ` Lorenzo Bianconi @ 2018-09-01 9:05 ` Lorenzo Bianconi 2018-09-01 23:45 ` David Ahern 0 siblings, 1 reply; 11+ messages in thread From: Lorenzo Bianconi @ 2018-09-01 9:05 UTC (permalink / raw) To: David Ahern; +Cc: David S. Miller, Network Development, Thomas Haller > > > On 8/31/18 10:19 AM, Lorenzo Bianconi wrote: > > >> On 8/31/18 5:43 AM, Lorenzo Bianconi wrote: > > >>> When moving a veth device to another namespace, userspace receives a > > >>> RTM_DELLINK message indicating the device has been removed from current > > >>> netns. However, the other peer does not receive a netlink event > > >>> containing new values for IFLA_LINK_NETNSID and IFLA_LINK veth > > >>> attributes. > > >>> Fix that behaviour sending to userspace a RTM_NEWLINK message in the peer > > >>> namespace to report new IFLA_LINK_NETNSID/IFLA_LINK values > > >>> > > >> > > >> A newlink message is generated in the new namespace. What information is > > >> missing from that message? > > >> > > > > > > Hi David, > > > > > > let's assume we have two veth paired devices (veth0 and veth1) on inet > > > namespace. When moving a veth1 to another namespace, userspace is notified > > > with RTM_DELLINK event on inet namespace to indicate that veth1 has been > > > moved to another namespace. However some userspace applications > > > (e.g. NetworkManager), listening for events on inet namespace, are interested > > > in veth1 ifindex in the new namespace. This patch sends a new RTM_NEWLINK event > > > in inet namespace to provide new values for IFLA_LINK_NETNSID/IFLA_LINK > > > > This is in init namespace > > $ ip li set veth2 netns foo > > > > $ ip monitor > > Deleted 20: veth2@veth1: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc > > noop state DOWN group default > > link/ether c6:d0:d6:c5:23:7d brd ff:ff:ff:ff:ff:ff new-netns foo > > new-ifindex 20 > > > > It shows the new namespace in the delete message. > Hi David, I was thinking about the commit 38e01b30563a and then I realized I misread the code yesterday. The commit 38e01b30563a provides all relevant info but it emits the event for veth1 (the device moved in the new namespace). An userspace application will not receive that message if it filters events for just a specific device (veth0 in this case) despite that some device properties have changed (since veth0 and veth1 are paired devices). To fix that behavior in veth_notify routine I emits a RTM_NEWLINK event for veth0. Regards, Lorenzo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace 2018-09-01 9:05 ` Lorenzo Bianconi @ 2018-09-01 23:45 ` David Ahern 2018-09-03 9:10 ` Thomas Haller 0 siblings, 1 reply; 11+ messages in thread From: David Ahern @ 2018-09-01 23:45 UTC (permalink / raw) To: Lorenzo Bianconi; +Cc: David S. Miller, Network Development, Thomas Haller On 9/1/18 3:05 AM, Lorenzo Bianconi wrote: > > I was thinking about the commit 38e01b30563a and then I realized I > misread the code > yesterday. The commit 38e01b30563a provides all relevant info but it > emits the event > for veth1 (the device moved in the new namespace). > An userspace application will not receive that message if it filters > events for just > a specific device (veth0 in this case) despite that some device > properties have changed > (since veth0 and veth1 are paired devices). To fix that behavior in > veth_notify routine > I emits a RTM_NEWLINK event for veth0. Userspace is managing a veth a pair. It moves one of them to a new namespace and decides to filter messages related to that device before the move. If it did not filter the DELLINK message it would get the information you want. That to me is a userspace problem. What am I missing? Fundamentally, your proposal means 3 messages when moving a device to a new namespace which is what I think is unnecessary. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace 2018-09-01 23:45 ` David Ahern @ 2018-09-03 9:10 ` Thomas Haller 2018-09-04 2:54 ` David Ahern 0 siblings, 1 reply; 11+ messages in thread From: Thomas Haller @ 2018-09-03 9:10 UTC (permalink / raw) To: David Ahern, Lorenzo Bianconi; +Cc: David S. Miller, Network Development [-- Attachment #1: Type: text/plain, Size: 2516 bytes --] Hi, On Sat, 2018-09-01 at 17:45 -0600, David Ahern wrote: > On 9/1/18 3:05 AM, Lorenzo Bianconi wrote: > > > > I was thinking about the commit 38e01b30563a and then I realized I > > misread the code > > yesterday. The commit 38e01b30563a provides all relevant info but > > it > > emits the event > > for veth1 (the device moved in the new namespace). > > An userspace application will not receive that message if it > > filters > > events for just > > a specific device (veth0 in this case) despite that some device > > properties have changed > > (since veth0 and veth1 are paired devices). To fix that behavior in > > veth_notify routine > > I emits a RTM_NEWLINK event for veth0. > > Userspace is managing a veth a pair. It moves one of them to a new > namespace and decides to filter messages related to that device > before > the move. If it did not filter the DELLINK message it would get the > information you want. That to me is a userspace problem. What am I > missing? The userspace component which moves the veth peer is likely not the same that is listening to these events. > Fundamentally, your proposal means 3 messages when moving a device to > a > new namespace which is what I think is unnecessary. You are correct, the necessary information is signaled in this case. Usually, one can manage the information about one link by considering only RTM_NEWLINK/RTM_DELLINK message for that link. That seems desirable behavior of the netlink API, as it simplifies user space implementations. For example, libnl3's cache for links doesn't properly handles this, and you end up with invalid data in the cache. You may call that a userspace problem and bug. But does it really need to be that complicated? FWIW, a similar thing was also NACK'ed earlier: http://lists.openwall.net/netdev/2016/06/12/8 Paolo and Lorenzo pointed out another issue when moving the peer to a third namespace: >>>>>>>>>>>>>>>>>>>>>>>>>>> ip netns del ns1 ip netns del ns2 ip netns del ns3 ip netns add ns1 ip netns add ns2 ip netns add ns3 ip -n ns1 link add dev veth0 type veth peer name veth1 ip -n ns1 monitor link & ip -n ns1 link set veth1 netns ns2 # Deleted 9: veth1@veth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default # link/ether 86:79:1d:c6:45:bc brd ff:ff:ff:ff:ff:ff new-netnsid 0 new-ifindex 9 ip -n ns2 link set veth1 netns ns3 # no event. >>>>>>>>>>>>>>>>>>>>>> best, Thomas [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace 2018-09-03 9:10 ` Thomas Haller @ 2018-09-04 2:54 ` David Ahern 2018-09-07 18:52 ` Thomas Haller 0 siblings, 1 reply; 11+ messages in thread From: David Ahern @ 2018-09-04 2:54 UTC (permalink / raw) To: Thomas Haller, Lorenzo Bianconi; +Cc: David S. Miller, Network Development On 9/3/18 3:10 AM, Thomas Haller wrote: > Hi, > > On Sat, 2018-09-01 at 17:45 -0600, David Ahern wrote: >> On 9/1/18 3:05 AM, Lorenzo Bianconi wrote: >>> >>> I was thinking about the commit 38e01b30563a and then I realized I >>> misread the code >>> yesterday. The commit 38e01b30563a provides all relevant info but >>> it >>> emits the event >>> for veth1 (the device moved in the new namespace). >>> An userspace application will not receive that message if it >>> filters >>> events for just >>> a specific device (veth0 in this case) despite that some device >>> properties have changed >>> (since veth0 and veth1 are paired devices). To fix that behavior in >>> veth_notify routine >>> I emits a RTM_NEWLINK event for veth0. >> >> Userspace is managing a veth a pair. It moves one of them to a new >> namespace and decides to filter messages related to that device >> before >> the move. If it did not filter the DELLINK message it would get the >> information you want. That to me is a userspace problem. What am I >> missing? > > The userspace component which moves the veth peer is likely not the > same that is listening to these events. > >> Fundamentally, your proposal means 3 messages when moving a device to >> a >> new namespace which is what I think is unnecessary. > > You are correct, the necessary information is signaled in this case. > > Usually, one can manage the information about one link by considering > only RTM_NEWLINK/RTM_DELLINK message for that link. That seems > desirable behavior of the netlink API, as it simplifies user space > implementations. > > For example, libnl3's cache for links doesn't properly handles this, > and you end up with invalid data in the cache. You may call that a > userspace problem and bug. But does it really need to be that > complicated? > > FWIW, a similar thing was also NACK'ed earlier: > http://lists.openwall.net/netdev/2016/06/12/8 And from what I can see, I agree with that NACK; a 3rd message is not necessary. Userspace has the ability to learn what it needs and should be looking at the existing options that bleed data across namespaces. > > > > Paolo and Lorenzo pointed out another issue when moving the peer to a > third namespace: > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > ip netns del ns1 > ip netns del ns2 > ip netns del ns3 > ip netns add ns1 > ip netns add ns2 > ip netns add ns3 > > > ip -n ns1 link add dev veth0 type veth peer name veth1 > ip -n ns1 monitor link & > > ip -n ns1 link set veth1 netns ns2 > # Deleted 9: veth1@veth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default > # link/ether 86:79:1d:c6:45:bc brd ff:ff:ff:ff:ff:ff new-netnsid 0 new-ifindex 9 > > ip -n ns2 link set veth1 netns ns3 > # no event. ip li add veth1 type veth peer name veth2 ip netns add foo ip netns add bar --> start ip monitor here; see below ip li set veth2 netns foo ip -netns foo li set veth2 netns bar >From init_net: $ ip monitor all-nsid [nsid current]Deleted inet veth2 [nsid current]Deleted inet6 veth2 [nsid current]nsid 0 (iproute2 netns name: foo) [nsid current]Deleted 16: veth2@veth1: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default link/ether 0a:28:87:f9:17:7c brd ff:ff:ff:ff:ff:ff new-netns foo new-ifindex 16 [nsid current]inet if16 forwarding on rp_filter off mc_forwarding off proxy_neigh off ignore_routes_with_linkdown off [nsid current]inet6 if16 forwarding off mc_forwarding off proxy_neigh off ignore_routes_with_linkdown off [nsid current]nsid 0 (iproute2 netns name: foo) [nsid current]16: veth2@if17: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default link/ether 0a:28:87:f9:17:7c brd ff:ff:ff:ff:ff:ff link-netnsid 0 [nsid current]Deleted inet veth2 [nsid current]Deleted inet6 veth2 [nsid current]nsid 1 [nsid current]Deleted 16: veth2@if17: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default link/ether 0a:28:87:f9:17:7c brd ff:ff:ff:ff:ff:ff link-netnsid 0 new-netnsid 1 new-ifindex 16 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace 2018-09-04 2:54 ` David Ahern @ 2018-09-07 18:52 ` Thomas Haller 2018-09-09 1:16 ` David Ahern 0 siblings, 1 reply; 11+ messages in thread From: Thomas Haller @ 2018-09-07 18:52 UTC (permalink / raw) To: David Ahern, Lorenzo Bianconi; +Cc: David S. Miller, Network Development [-- Attachment #1: Type: text/plain, Size: 1454 bytes --] Hi David, On Mon, 2018-09-03 at 20:54 -0600, David Ahern wrote: > From init_net: > $ ip monitor all-nsid I thought the concern of the patch is the overhead of sending one additional RTM_NEWLINK message. This workaround has likely higher overhead. More importantly, it's so cumbersome, that I doubt anybody would implementing such a solution. When the events of one namespace are not sufficient to get all relevant information (local to the namespace itself), the solution is not monitor all-nsid. You might save complexity and performance overhead in kernel. But what you save here is just moved to user-space, which faces higher complexity (at multiple places/projects, where developers are not experts in netlink) and higher overhead. RTM_GETLINK/NLM_F_DUMP allows to fetch information. The same information is usually also emitted on changes via RTM_NEWLINK notifications. Yes, the information may be avilable somehow, but how cumbersome: a) receive RTM_DELLINK, recognize that the message belongs to a veth that was moved to another netns, and recognize that the peer's IFLA_LINK changed. This approach only works when the veth is moved away from the current namespace. b) or, enable monitor all-nsid, receive RTM_NEWLINK, recognize that the event is for moving a veth between netns, find the peer in the other netns and recognize that the peer's IFLA_LINK changed. best, THomas [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace 2018-09-07 18:52 ` Thomas Haller @ 2018-09-09 1:16 ` David Ahern 0 siblings, 0 replies; 11+ messages in thread From: David Ahern @ 2018-09-09 1:16 UTC (permalink / raw) To: Thomas Haller, Lorenzo Bianconi; +Cc: David S. Miller, Network Development Hi Thomas: On 9/7/18 12:52 PM, Thomas Haller wrote: > Hi David, > > > On Mon, 2018-09-03 at 20:54 -0600, David Ahern wrote: > >> From init_net: >> $ ip monitor all-nsid > > I thought the concern of the patch is the overhead of sending one > additional RTM_NEWLINK message. This workaround has likely higher > overhead. More importantly, it's so cumbersome, that I doubt anybody > would implementing such a solution. Yes, the concern is additional messages. Each one adds non-negligible overhead to build and send, something that is measurable in a system at scale. For example, consider an interface manager creating 4k VLANs on one or more bridges. The RTM_NEWLINK messages (there are 2) adds 33% to the overhead and time of creating the vlans. Just in some quick tests I see times vary from as little as 15 usec to over 1 msec - per message with the RTNL held. The norm seems to be somewhere between 35 and 40 usecs, but those add up when it is per link and per action. We have to more disciplined about the size and frequency of these messages. > > When the events of one namespace are not sufficient to get all relevant > information (local to the namespace itself), the solution is not > monitor all-nsid. > > You might save complexity and performance overhead in kernel. But what > you save here is just moved to user-space, which faces higher > complexity (at multiple places/projects, where developers are not > experts in netlink) and higher overhead. First, this is one use case that seems to care about a message coming back on the veth pairs. The message is sent for everyone, always. Adding a 3rd message puts additional load on all use cases. Second, I am questioning this use case driving a kernel change. You say a userspace app cares about a veth pair but only wants to monitor events for one end of it. That's a userspace choice. Another argument brought up is that the other end of the pair can change namespaces faster than the app can track it. Sure. But the monitored end can move as well - and faster than the app can track it. e.g., 1. veth1 and veth2 are a pair. 2. app filters events for veth1 only 3. veth2 is moved to namespace foo 4. app is sent notification of move 5. veth2 is moved to namespace bar 6. app processes notification of the event in step 3, looks in namespace foo and link is gone. That seems to be the fundamentals of your request for this new message, yes? What happens when veth1 is moved to another namespace and then another namespace - faster than the app can not track it? You have the same problem. Your use case may not have this problem, but generically it can happen and this solution does not cover it. The movement of links is provided to userspace, and userspace needs to be smart about what it wants to do. Alternatively, you could provide an API for your interface manager -- whatever is handling the link movement. It know where it is moving interfaces. You could have it respond to inquiries of where device X is at any given moment. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-09-09 6:04 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <cover.1535712096.git.lorenzo.bianconi@redhat.com> 2018-08-31 11:43 ` [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace Lorenzo Bianconi 2018-08-31 15:24 ` David Ahern 2018-08-31 16:19 ` Lorenzo Bianconi 2018-08-31 16:21 ` David Ahern 2018-08-31 16:54 ` Lorenzo Bianconi 2018-09-01 9:05 ` Lorenzo Bianconi 2018-09-01 23:45 ` David Ahern 2018-09-03 9:10 ` Thomas Haller 2018-09-04 2:54 ` David Ahern 2018-09-07 18:52 ` Thomas Haller 2018-09-09 1:16 ` David Ahern
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.