net: ipv6: fix regression of no RTM_DELADDR sent after DAD failure

Message ID 1504536775-23533-1-git-send-email-mmanning@brocade.com
State Accepted
Delegated to: David Miller
Headers show
Series
  • net: ipv6: fix regression of no RTM_DELADDR sent after DAD failure
Related show

Commit Message

Mike Manning Sept. 4, 2017, 2:52 p.m.
Commit f784ad3d79e5 ("ipv6: do not send RTM_DELADDR for tentative
addresses") incorrectly assumes that no RTM_NEWADDR are sent for
addresses in tentative state, as this does happen for the standard
IPv6 use-case of DAD failure, see the call to ipv6_ifa_notify() in
addconf_dad_stop(). So as a result of this change, no RTM_DELADDR is
sent after DAD failure for a link-local when strict DAD (accept_dad=2)
is configured, or on the next admin down in other cases. The absence
of this notification breaks backwards compatibility and causes problems
after DAD failure if this notification was being relied on. The
solution is to allow RTM_DELADDR to still be sent after DAD failure.

Fixes: f784ad3d79e5("ipv6: do not send RTM_DELADDR for tentative addresses")
Signed-off-by: Mike Manning <mmanning@brocade.com>
Cc: Mahesh Bandewar <maheshb@google.com>
---
 net/ipv6/addrconf.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

David Miller Sept. 8, 2017, 2:18 a.m. | #1
From: Mike Manning <mmanning@brocade.com>
Date: Mon,  4 Sep 2017 15:52:55 +0100

> Commit f784ad3d79e5 ("ipv6: do not send RTM_DELADDR for tentative
> addresses") incorrectly assumes that no RTM_NEWADDR are sent for
> addresses in tentative state, as this does happen for the standard
> IPv6 use-case of DAD failure, see the call to ipv6_ifa_notify() in
> addconf_dad_stop(). So as a result of this change, no RTM_DELADDR is
> sent after DAD failure for a link-local when strict DAD (accept_dad=2)
> is configured, or on the next admin down in other cases. The absence
> of this notification breaks backwards compatibility and causes problems
> after DAD failure if this notification was being relied on. The
> solution is to allow RTM_DELADDR to still be sent after DAD failure.
> 
> Fixes: f784ad3d79e5("ipv6: do not send RTM_DELADDR for tentative addresses")
> Signed-off-by: Mike Manning <mmanning@brocade.com>
> Cc: Mahesh Bandewar <maheshb@google.com>

Mahesh, please review this patch.
Mike Manning Sept. 18, 2017, 1:06 p.m. | #2
Hi,
In the absence of a reply from Mahesh, I would be most grateful for
anyone familiar with the IPv6 code to review this 1-line fix.

Or if not, then I request that the commit f784ad3d79e5 is backed out,
as its intention is to remove the redundant but harmless RTM_DELADDR
for addresses in tentative state, but is also incorrectly removing the
very necessary RTM_DELADDR when an address is deleted that was previously
notified with an RTM_NEWADDR as being in tentative dadfailed state.

Thanks
Mike

On 08/09/17 03:18, David Miller wrote:
> From: Mike Manning <mmanning@brocade.com>
> Date: Mon,  4 Sep 2017 15:52:55 +0100
> 
>> Commit f784ad3d79e5 ("ipv6: do not send RTM_DELADDR for tentative
>> addresses") incorrectly assumes that no RTM_NEWADDR are sent for
>> addresses in tentative state, as this does happen for the standard
>> IPv6 use-case of DAD failure, see the call to ipv6_ifa_notify() in
>> addconf_dad_stop(). So as a result of this change, no RTM_DELADDR is
>> sent after DAD failure for a link-local when strict DAD (accept_dad=2)
>> is configured, or on the next admin down in other cases. The absence
>> of this notification breaks backwards compatibility and causes problems
>> after DAD failure if this notification was being relied on. The
>> solution is to allow RTM_DELADDR to still be sent after DAD failure.
>>
>> Fixes: f784ad3d79e5("ipv6: do not send RTM_DELADDR for tentative addresses")
>> Signed-off-by: Mike Manning <mmanning@brocade.com>
>> Cc: Mahesh Bandewar <maheshb@google.com>
> 
> Mahesh, please review this patch.
>
David Miller Sept. 19, 2017, 11:43 p.m. | #3
From: Mike Manning <mmanning@brocade.com>
Date: Mon, 18 Sep 2017 14:06:40 +0100

> In the absence of a reply from Mahesh, I would be most grateful for
> anyone familiar with the IPv6 code to review this 1-line fix.
> 
> Or if not, then I request that the commit f784ad3d79e5 is backed out,
> as its intention is to remove the redundant but harmless RTM_DELADDR
> for addresses in tentative state, but is also incorrectly removing the
> very necessary RTM_DELADDR when an address is deleted that was previously
> notified with an RTM_NEWADDR as being in tentative dadfailed state.

I've applied your patch, and queued it up for -stable, thanks.

Patch

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 936e9ab..ba757c2 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4982,9 +4982,10 @@  static void inet6_ifa_notify(int event, struct inet6_ifaddr *ifa)
 
 	/* Don't send DELADDR notification for TENTATIVE address,
 	 * since NEWADDR notification is sent only after removing
-	 * TENTATIVE flag.
+	 * TENTATIVE flag, if DAD has not failed.
 	 */
-	if (ifa->flags & IFA_F_TENTATIVE && event == RTM_DELADDR)
+	if (ifa->flags & IFA_F_TENTATIVE && !(ifa->flags & IFA_F_DADFAILED) &&
+	    event == RTM_DELADDR)
 		return;
 
 	skb = nlmsg_new(inet6_ifaddr_msgsize(), GFP_ATOMIC);