[PATCH] ipv6: Log the explicit address that triggered DAD failure

[PATCH] ipv6: Log the explicit address that triggered DAD failure

[Jens Rosenboom]

If an interface has multiple addresses, the current message for DAD
failure isn't really helpful, so this patch adds the address itself to
the printk.

Signed-off-by: Jens Rosenboom <jens@mcbone.net>

---

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 43b3c9f..01a4b25 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1403,8 +1403,8 @@ void addrconf_dad_failure(struct inet6_ifaddr
*ifp)
 	struct inet6_dev *idev = ifp->idev;
 
 	if (net_ratelimit())
-		printk(KERN_INFO "%s: IPv6 duplicate address detected!\n",
-			ifp->idev->dev->name);
+		printk(KERN_INFO "%s: IPv6 duplicate address %pI6 detected!\n",
+			ifp->idev->dev->name, &ifp->addr);
 
 	if (idev->cnf.accept_dad > 1 && !idev->cnf.disable_ipv6) {
 		struct in6_addr addr;

Re: [PATCH] ipv6: Log the explicit address that triggered DAD failure

[Brian Haley]

Jens Rosenboom wrote:
> If an interface has multiple addresses, the current message for DAD
> failure isn't really helpful, so this patch adds the address itself to
> the printk.
> 
> Signed-off-by: Jens Rosenboom <jens@mcbone.net>
> 
> ---
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 43b3c9f..01a4b25 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -1403,8 +1403,8 @@ void addrconf_dad_failure(struct inet6_ifaddr
> *ifp)
>  	struct inet6_dev *idev = ifp->idev;
>  
>  	if (net_ratelimit())
> -		printk(KERN_INFO "%s: IPv6 duplicate address detected!\n",
> -			ifp->idev->dev->name);
> +		printk(KERN_INFO "%s: IPv6 duplicate address %pI6 detected!\n",
> +			ifp->idev->dev->name, &ifp->addr);
>  
>  	if (idev->cnf.accept_dad > 1 && !idev->cnf.disable_ipv6) {
>  		struct in6_addr addr;

I have no problem with this patch, I should have done this when I
last changed this code.

The other thing I've come across that is similar to this is the
issue that when DAD fails, /sbin/ip doesn't show that it did,
the address just stays in a tentative state forever:

    inet6 dead:beef::1/64 scope global tentative 
       valid_lft forever preferred_lft forever

Does anyone have an issue of adding a "dadfailed" flag to make
this more obvious:

    inet6 dead:beef::1/64 scope global tentative dadfailed
       valid_lft forever preferred_lft forever

-Brian

Re: [PATCH] ipv6: Log the explicit address that triggered DAD failure

[Jens Rosenboom]

On Wed, 2009-08-12 at 21:33 -0400, Brian Haley wrote:
[...]
> The other thing I've come across that is similar to this is the
> issue that when DAD fails, /sbin/ip doesn't show that it did,
> the address just stays in a tentative state forever:
> 
>     inet6 dead:beef::1/64 scope global tentative 
>        valid_lft forever preferred_lft forever
> 
> Does anyone have an issue of adding a "dadfailed" flag to make
> this more obvious:
> 
>     inet6 dead:beef::1/64 scope global tentative dadfailed
>        valid_lft forever preferred_lft forever

It looks like you would have to spend the last available bit in
ifa_flags for that, not sure if that is worth it, how about setting it
to tentative|deprecated instead?

Some action should maybe also happen in the case that the address wasn't
tentative anymore in ndisc_recv_na(). At least it should also log the
address itself:

diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 9eb68e9..1ba42bd 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -955,8 +955,8 @@ static void ndisc_recv_na(struct sk_buff *skb)
 		 */
 		if (skb->pkt_type != PACKET_LOOPBACK)
 			ND_PRINTK1(KERN_WARNING
-			   "ICMPv6 NA: someone advertises our address on %s!\n",
-			   ifp->idev->dev->name);
+			   "ICMPv6 NA: someone advertises our address %pI6 on %s!\n",
+			   &ifp->addr, ifp->idev->dev->name);
 		in6_ifa_put(ifp);
 		return;
 	}

Re: [PATCH] ipv6: Log the explicit address that triggered DAD failure

[Brian Haley]

Jens Rosenboom wrote:
> On Wed, 2009-08-12 at 21:33 -0400, Brian Haley wrote:
> [...]
>> The other thing I've come across that is similar to this is the
>> issue that when DAD fails, /sbin/ip doesn't show that it did,
>> the address just stays in a tentative state forever:
>>
>>     inet6 dead:beef::1/64 scope global tentative 
>>        valid_lft forever preferred_lft forever
>>
>> Does anyone have an issue of adding a "dadfailed" flag to make
>> this more obvious:
>>
>>     inet6 dead:beef::1/64 scope global tentative dadfailed
>>        valid_lft forever preferred_lft forever
> 
> It looks like you would have to spend the last available bit in
> ifa_flags for that, not sure if that is worth it, how about setting it
> to tentative|deprecated instead?

Yes, I saw that it would be the last flag so I didn't know how that would
go over.  My other thought was to define a new flags structure that can
be passed in/out like IFA_CACHEINFO is.  It's a much larger patch...

> Some action should maybe also happen in the case that the address wasn't
> tentative anymore in ndisc_recv_na(). At least it should also log the
> address itself:
> 
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index 9eb68e9..1ba42bd 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -955,8 +955,8 @@ static void ndisc_recv_na(struct sk_buff *skb)
>  		 */
>  		if (skb->pkt_type != PACKET_LOOPBACK)
>  			ND_PRINTK1(KERN_WARNING
> -			   "ICMPv6 NA: someone advertises our address on %s!\n",
> -			   ifp->idev->dev->name);
> +			   "ICMPv6 NA: someone advertises our address %pI6 on %s!\n",
> +			   &ifp->addr, ifp->idev->dev->name);
>  		in6_ifa_put(ifp);
>  		return;
>  	}

That looks good to me too, thanks.

-Brian