diff mbox series

[net-next] ipv6: send netlink notifications for manually configured addresses

Message ID 934e7797f7fb3541e36c01c5c753e0592679527b.1523956672.git.lorenzo.bianconi@redhat.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series [net-next] ipv6: send netlink notifications for manually configured addresses | expand

Commit Message

Lorenzo Bianconi April 17, 2018, 9:54 a.m. UTC
Send a netlink notification when userspace adds a manually configured
address if DAD is enabled and optimistic flag isn't set.
Moreover send RTM_DELADDR notifications for tentative addresses.

Some userspace applications (e.g. NetworkManager) are interested in
addr netlink events albeit the address is still in tentative state,
however events are not sent if DAD process is not completed.
If the address is added and immediately removed userspace listeners
are not notified. This behaviour can be easily reproduced by using
veth interfaces:

$ ip -b - <<EOF
> link add dev vm1 type veth peer name vm2
> link set dev vm1 up
> link set dev vm2 up
> addr add 2001:db8:a:b:1:2:3:4/64 dev vm1
> addr del 2001:db8:a:b:1:2:3:4/64 dev vm1
EOF

This patch reverts the behaviour introduced by the commit f784ad3d79e5
("ipv6: do not send RTM_DELADDR for tentative addresses")

Suggested-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 net/ipv6/addrconf.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

David Miller April 17, 2018, 6:05 p.m. UTC | #1
From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date: Tue, 17 Apr 2018 11:54:39 +0200

> Send a netlink notification when userspace adds a manually configured
> address if DAD is enabled and optimistic flag isn't set.
> Moreover send RTM_DELADDR notifications for tentative addresses.
> 
> Some userspace applications (e.g. NetworkManager) are interested in
> addr netlink events albeit the address is still in tentative state,
> however events are not sent if DAD process is not completed.
> If the address is added and immediately removed userspace listeners
> are not notified. This behaviour can be easily reproduced by using
> veth interfaces:
> 
> $ ip -b - <<EOF
>> link add dev vm1 type veth peer name vm2
>> link set dev vm1 up
>> link set dev vm2 up
>> addr add 2001:db8:a:b:1:2:3:4/64 dev vm1
>> addr del 2001:db8:a:b:1:2:3:4/64 dev vm1
> EOF
> 
> This patch reverts the behaviour introduced by the commit f784ad3d79e5
> ("ipv6: do not send RTM_DELADDR for tentative addresses")
> 
> Suggested-by: Thomas Haller <thaller@redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

Ok, applied to net-next.

It would be really nice if we clearly documented somewhere the exact
situations where we desire ipv6 address netlink notifications to be
sent out.

Maybe even a common function that guards the event emission, where we
encode the rules.  Or a comment somewhere prominent.  Something like
that.

Right now this isn't spelled out clearly anywhere, and that's probably
why things keep being adjusted back and forth like this.

Thank you.
Lorenzo Bianconi April 19, 2018, 10:23 p.m. UTC | #2
> From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> Date: Tue, 17 Apr 2018 11:54:39 +0200
>
>> Send a netlink notification when userspace adds a manually configured
>> address if DAD is enabled and optimistic flag isn't set.
>> Moreover send RTM_DELADDR notifications for tentative addresses.
>>
>> Some userspace applications (e.g. NetworkManager) are interested in
>> addr netlink events albeit the address is still in tentative state,
>> however events are not sent if DAD process is not completed.
>> If the address is added and immediately removed userspace listeners
>> are not notified. This behaviour can be easily reproduced by using
>> veth interfaces:
>>
>> $ ip -b - <<EOF
>>> link add dev vm1 type veth peer name vm2
>>> link set dev vm1 up
>>> link set dev vm2 up
>>> addr add 2001:db8:a:b:1:2:3:4/64 dev vm1
>>> addr del 2001:db8:a:b:1:2:3:4/64 dev vm1
>> EOF
>>
>> This patch reverts the behaviour introduced by the commit f784ad3d79e5
>> ("ipv6: do not send RTM_DELADDR for tentative addresses")
>>
>> Suggested-by: Thomas Haller <thaller@redhat.com>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
>
> Ok, applied to net-next.
>
> It would be really nice if we clearly documented somewhere the exact
> situations where we desire ipv6 address netlink notifications to be
> sent out.
>
> Maybe even a common function that guards the event emission, where we
> encode the rules.  Or a comment somewhere prominent.  Something like
> that.
>
> Right now this isn't spelled out clearly anywhere, and that's probably
> why things keep being adjusted back and forth like this.

Sounds good, I will post a patch. Thanks

Regards,
Lorenzo

>
> Thank you.
diff mbox series

Patch

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 62b97722722c..b2c0175125db 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2901,6 +2901,11 @@  static int inet6_addr_add(struct net *net, int ifindex,
 					      expires, flags);
 		}
 
+		/* Send a netlink notification if DAD is enabled and
+		 * optimistic flag is not set
+		 */
+		if (!(ifp->flags & (IFA_F_OPTIMISTIC | IFA_F_NODAD)))
+			ipv6_ifa_notify(0, ifp);
 		/*
 		 * Note that section 3.1 of RFC 4429 indicates
 		 * that the Optimistic flag should not be set for
@@ -5028,14 +5033,6 @@  static void inet6_ifa_notify(int event, struct inet6_ifaddr *ifa)
 	struct net *net = dev_net(ifa->idev->dev);
 	int err = -ENOBUFS;
 
-	/* Don't send DELADDR notification for TENTATIVE address,
-	 * since NEWADDR notification is sent only after removing
-	 * TENTATIVE flag, if DAD has not failed.
-	 */
-	if (ifa->flags & IFA_F_TENTATIVE && !(ifa->flags & IFA_F_DADFAILED) &&
-	    event == RTM_DELADDR)
-		return;
-
 	skb = nlmsg_new(inet6_ifaddr_msgsize(), GFP_ATOMIC);
 	if (!skb)
 		goto errout;