ipv6 addrconfg warn_on hit: WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);

ipv6 addrconfg warn_on hit: WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);

[Jiri Pirko]

Hi Hannes.

WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);

We did hit once this warning during the tests. The person who hit this
says that it was during the setup of many macvlan devices.

I examined the code but I'm not sure how this could happen. Looks like a
race condition between addrconf_dad_completed() and addrconf_ifdown(). 
Not sure how to easily resolve that though.

Would you please take a look?

Thanks!

Jiri

Re: ipv6 addrconfg warn_on hit: WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);

[Jiri Pirko]

Thu, Jan 16, 2014 at 02:53:23PM CET, jiri@resnulli.us wrote:
>Hi Hannes.
>
>WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);
>
>We did hit once this warning during the tests. The person who hit this
>says that it was during the setup of many macvlan devices.
>
>I examined the code but I'm not sure how this could happen. Looks like a
>race condition between addrconf_dad_completed() and addrconf_ifdown(). 
>Not sure how to easily resolve that though.
>
>Would you please take a look?
>
>Thanks!
>
>Jiri

[16661.669422] Call Trace:
[16661.684417]  [<ffffffff815b4dde>] dump_stack+0x19/0x1b
[16661.714596]  [<ffffffff81058bd1>] warn_slowpath_common+0x61/0x80
[16661.748587]  [<ffffffff81058caa>] warn_slowpath_null+0x1a/0x20
[16661.780334]  [<ffffffff81551e1a>] update_valid_ll_addr_cnt+0x9a/0xa0
[16661.816145]  [<ffffffff81555ca5>] __ipv6_ifa_notify+0xb5/0x2e0
[16661.849222]  [<ffffffff8155819e>] addrconf_ifdown+0x21e/0x3e0
[16661.880245]  [<ffffffff815594be>] addrconf_notify+0x5e/0x430
[16661.912233]  [<ffffffff815662ce>] ? ndisc_netdev_event+0x4e/0x100
[16661.945485]  [<ffffffff815c064c>] notifier_call_chain+0x4c/0x70
[16661.979227]  [<ffffffff81084756>] raw_notifier_call_chain+0x16/0x20
[16662.013434]  [<ffffffff814a5fad>] call_netdevice_notifiers+0x2d/0x60
[16662.051709]  [<ffffffff814a6217>] dev_close_many+0xb7/0x100
[16662.084728]  [<ffffffff814a62c0>] dev_close+0x60/0x80
[16662.114689]  [<ffffffff814a9134>] dev_change_net_namespace+0x94/0x220
[16662.152638]  [<ffffffff814ba043>] do_setlink+0x83/0x9f0
[16662.183447]  [<ffffffff812bae36>] ? nla_parse+0x96/0xe0
[16662.214146]  [<ffffffff814bb614>] rtnl_newlink+0x394/0x5e0
[16662.247097]  [<ffffffff8123cc0e>] ? selinux_capable+0x2e/0x40
[16662.281035]  [<ffffffff814b8009>] rtnetlink_rcv_msg+0x99/0x260
[16662.313053]  [<ffffffff81239e25>] ? sock_has_perm+0x75/0x90
[16662.344678]  [<ffffffff814b7f70>] ? rtnetlink_rcv+0x30/0x30
[16662.375627]  [<ffffffff814d6fc9>] netlink_rcv_skb+0xa9/0xc0
[16662.409066]  [<ffffffff814b7f68>] rtnetlink_rcv+0x28/0x30
[16662.438982]  [<ffffffff814d662d>] netlink_unicast+0xdd/0x190
[16662.470946]  [<ffffffff814d69df>] netlink_sendmsg+0x2ff/0x730
[16662.502797]  [<ffffffff81493289>] sock_sendmsg+0x99/0xd0
[16662.532625]  [<ffffffff8149367c>] ___sys_sendmsg+0x36c/0x380
[16662.564507]  [<ffffffff8149b329>] ? skb_dequeue+0x59/0x70
[16662.596497]  [<ffffffff8115d4b5>] ? handle_mm_fault+0x285/0x3a0
[16662.631456]  [<ffffffff815c0174>] ? __do_page_fault+0x1f4/0x510
[16662.666968]  [<ffffffff811b3a85>] ? d_free+0x55/0x60
[16662.695923]  [<ffffffff811bcc1e>] ? mntput_no_expire+0x3e/0x120
[16662.731922]  [<ffffffff811bcd24>] ? mntput+0x24/0x40
[16662.761353]  [<ffffffff8119f823>] ? __fput+0x183/0x270
[16662.792218]  [<ffffffff81494462>] __sys_sendmsg+0x42/0x80
[16662.824368]  [<ffffffff814944b2>] SyS_sendmsg+0x12/0x20
[16662.855854]  [<ffffffff815c4c99>] system_call_fastpath+0x16/0x1b
[16662.892006] ---[ end trace 9f96f71eeb981f7e ]---

Re: ipv6 addrconfg warn_on hit: WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);

[Hannes Frederic Sowa]

On Thu, Jan 16, 2014 at 03:07:01PM +0100, Jiri Pirko wrote:
> Thu, Jan 16, 2014 at 02:53:23PM CET, jiri@resnulli.us wrote:
> >Hi Hannes.
> >
> >WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);
> >
> >We did hit once this warning during the tests. The person who hit this
> >says that it was during the setup of many macvlan devices.
> >
> >I examined the code but I'm not sure how this could happen. Looks like a
> >race condition between addrconf_dad_completed() and addrconf_ifdown(). 
> >Not sure how to easily resolve that though.

That seems to be the case. Actually we don't need to count precisiely
here, we just must precisiely identify the situation where the first
LL address comes into operational state. Maybe we can implement this
somehow differently. I'll play with the code soon.

Thanks,

  Hannes

Re: ipv6 addrconfg warn_on hit: WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);

[Hannes Frederic Sowa]

On Thu, Jan 16, 2014 at 03:38:17PM +0100, Hannes Frederic Sowa wrote:
> On Thu, Jan 16, 2014 at 03:07:01PM +0100, Jiri Pirko wrote:
> > Thu, Jan 16, 2014 at 02:53:23PM CET, jiri@resnulli.us wrote:
> > >Hi Hannes.
> > >
> > >WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);
> > >
> > >We did hit once this warning during the tests. The person who hit this
> > >says that it was during the setup of many macvlan devices.
> > >
> > >I examined the code but I'm not sure how this could happen. Looks like a
> > >race condition between addrconf_dad_completed() and addrconf_ifdown(). 
> > >Not sure how to easily resolve that though.
> 
> That seems to be the case. Actually we don't need to count precisiely
> here, we just must precisiely identify the situation where the first
> LL address comes into operational state. Maybe we can implement this
> somehow differently. I'll play with the code soon.

Maybe something like this and then throw out the whole ll counting stuff:

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6913a82..105105a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3233,6 +3233,19 @@ out:
 	in6_ifa_put(ifp);
 }
 
+/* idev must be at least read locked */
+static bool ipv6_lonely_lladdr(struct inet6_dev *idev, struct inet6_ifaddr *ifp)
+{
+	bool ret = true;
+	struct inet6_ifaddr *ifpiter;
+
+	list_for_each_entry(ifpiter, &idev->addr_list, if_list) {
+		if (ifp != ifpiter && ifpiter->scope == IFA_LINK)
+			ret = false;
+	}
+	return ret;
+}
+
 static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 {
 	struct net_device *dev = ifp->idev->dev;
@@ -3253,8 +3266,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 
 	read_lock_bh(&ifp->idev->lock);
 	spin_lock(&ifp->lock);
-	send_mld = ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL &&
-		   ifp->idev->valid_ll_addr_cnt == 1;
+	send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp->idev, ifp);
 	send_rs = send_mld &&
 		  ipv6_accept_ra(ifp->idev) &&
 		  ifp->idev->cnf.rtr_solicits > 0 &&

Re: ipv6 addrconfg warn_on hit: WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);

[Jiri Pirko]

Thu, Jan 16, 2014 at 04:18:12PM CET, hannes@stressinduktion.org wrote:
>On Thu, Jan 16, 2014 at 03:38:17PM +0100, Hannes Frederic Sowa wrote:
>> On Thu, Jan 16, 2014 at 03:07:01PM +0100, Jiri Pirko wrote:
>> > Thu, Jan 16, 2014 at 02:53:23PM CET, jiri@resnulli.us wrote:
>> > >Hi Hannes.
>> > >
>> > >WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);
>> > >
>> > >We did hit once this warning during the tests. The person who hit this
>> > >says that it was during the setup of many macvlan devices.
>> > >
>> > >I examined the code but I'm not sure how this could happen. Looks like a
>> > >race condition between addrconf_dad_completed() and addrconf_ifdown(). 
>> > >Not sure how to easily resolve that though.
>> 
>> That seems to be the case. Actually we don't need to count precisiely
>> here, we just must precisiely identify the situation where the first
>> LL address comes into operational state. Maybe we can implement this
>> somehow differently. I'll play with the code soon.
>
>Maybe something like this and then throw out the whole ll counting stuff:
>
>diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>index 6913a82..105105a 100644
>--- a/net/ipv6/addrconf.c
>+++ b/net/ipv6/addrconf.c
>@@ -3233,6 +3233,19 @@ out:
> 	in6_ifa_put(ifp);
> }
> 
>+/* idev must be at least read locked */
>+static bool ipv6_lonely_lladdr(struct inet6_dev *idev, struct inet6_ifaddr *ifp)
>+{
>+	bool ret = true;
>+	struct inet6_ifaddr *ifpiter;
>+
>+	list_for_each_entry(ifpiter, &idev->addr_list, if_list) {
>+		if (ifp != ifpiter && ifpiter->scope == IFA_LINK)
>+			ret = false;
>+	}
>+	return ret;
>+}
>+
> static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
> {
> 	struct net_device *dev = ifp->idev->dev;
>@@ -3253,8 +3266,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
> 
> 	read_lock_bh(&ifp->idev->lock);
> 	spin_lock(&ifp->lock);
>-	send_mld = ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL &&
>-		   ifp->idev->valid_ll_addr_cnt == 1;
>+	send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp->idev, ifp);
> 	send_rs = send_mld &&
> 		  ipv6_accept_ra(ifp->idev) &&
> 		  ifp->idev->cnf.rtr_solicits > 0 &&

Sounds sane to me. Would you care to submit this please?
Do not forget to remove all the valid_ll_addr_cnt stuff :)

>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ipv6 addrconfg warn_on hit: WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);

[Hannes Frederic Sowa]

On Thu, Jan 16, 2014 at 06:27:06PM +0100, Jiri Pirko wrote:
> Thu, Jan 16, 2014 at 04:18:12PM CET, hannes@stressinduktion.org wrote:
> >On Thu, Jan 16, 2014 at 03:38:17PM +0100, Hannes Frederic Sowa wrote:
> >> On Thu, Jan 16, 2014 at 03:07:01PM +0100, Jiri Pirko wrote:
> >> > Thu, Jan 16, 2014 at 02:53:23PM CET, jiri@resnulli.us wrote:
> >> > >Hi Hannes.
> >> > >
> >> > >WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);
> >> > >
> >> > >We did hit once this warning during the tests. The person who hit this
> >> > >says that it was during the setup of many macvlan devices.
> >> > >
> >> > >I examined the code but I'm not sure how this could happen. Looks like a
> >> > >race condition between addrconf_dad_completed() and addrconf_ifdown(). 
> >> > >Not sure how to easily resolve that though.
> >> 
> >> That seems to be the case. Actually we don't need to count precisiely
> >> here, we just must precisiely identify the situation where the first
> >> LL address comes into operational state. Maybe we can implement this
> >> somehow differently. I'll play with the code soon.
> >
> >Maybe something like this and then throw out the whole ll counting stuff:
> >
> >diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> >index 6913a82..105105a 100644
> >--- a/net/ipv6/addrconf.c
> >+++ b/net/ipv6/addrconf.c
> >@@ -3233,6 +3233,19 @@ out:
> > 	in6_ifa_put(ifp);
> > }
> > 
> >+/* idev must be at least read locked */
> >+static bool ipv6_lonely_lladdr(struct inet6_dev *idev, struct inet6_ifaddr *ifp)
> >+{
> >+	bool ret = true;
> >+	struct inet6_ifaddr *ifpiter;
> >+
> >+	list_for_each_entry(ifpiter, &idev->addr_list, if_list) {
> >+		if (ifp != ifpiter && ifpiter->scope == IFA_LINK)
> >+			ret = false;
> >+	}
> >+	return ret;
> >+}
> >+
> > static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
> > {
> > 	struct net_device *dev = ifp->idev->dev;
> >@@ -3253,8 +3266,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
> > 
> > 	read_lock_bh(&ifp->idev->lock);
> > 	spin_lock(&ifp->lock);
> >-	send_mld = ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL &&
> >-		   ifp->idev->valid_ll_addr_cnt == 1;
> >+	send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp->idev, ifp);
> > 	send_rs = send_mld &&
> > 		  ipv6_accept_ra(ifp->idev) &&
> > 		  ifp->idev->cnf.rtr_solicits > 0 &&
> 
> Sounds sane to me. Would you care to submit this please?
> Do not forget to remove all the valid_ll_addr_cnt stuff :)

I'll do. ;) Currently testing the changes.

Thanks,

  Hannes

[PATCH net] ipv6: simplify detection of first operational link-local address on interface

[Hannes Frederic Sowa]

In commit 1ec047eb4751e3 ("ipv6: introduce per-interface counter for
dad-completed ipv6 addresses") I build the detection of the first
operational link-local address much to complex. Additionally this code
now has a race condition.

Replace it with a much simpler variant, which just scans the address
list when duplicate address detection completes, to check if this is
the first valid link local address and send RS and MLD reports then.

Fixes: 1ec047eb4751e3 ("ipv6: introduce per-interface counter for dad-completed ipv6 addresses")
Reported-by: Jiri Pirko <jiri@resnulli.us>
Cc: Flavio Leitner <fbl@redhat.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 include/net/if_inet6.h |  1 -
 net/ipv6/addrconf.c    | 38 +++++++++++++++++---------------------
 2 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index b58c36c..9650a3f 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -165,7 +165,6 @@ struct inet6_dev {
 	struct net_device	*dev;
 
 	struct list_head	addr_list;
-	int			valid_ll_addr_cnt;
 
 	struct ifmcaddr6	*mc_list;
 	struct ifmcaddr6	*mc_tomb;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6913a82..f91e107 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3233,6 +3233,22 @@ out:
 	in6_ifa_put(ifp);
 }
 
+/* ifp->idev must be at least read locked */
+static bool ipv6_lonely_lladdr(struct inet6_ifaddr *ifp)
+{
+	struct inet6_ifaddr *ifpiter;
+	struct inet6_dev *idev = ifp->idev;
+
+	list_for_each_entry(ifpiter, &idev->addr_list, if_list) {
+		if (ifp != ifpiter && ifpiter->scope == IFA_LINK &&
+		    (ifpiter->flags & (IFA_F_PERMANENT|IFA_F_TENTATIVE|
+				       IFA_F_OPTIMISTIC|IFA_F_DADFAILED)) ==
+		    IFA_F_PERMANENT)
+			return false;
+	}
+	return true;
+}
+
 static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 {
 	struct net_device *dev = ifp->idev->dev;
@@ -3252,14 +3268,11 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
 	 */
 
 	read_lock_bh(&ifp->idev->lock);
-	spin_lock(&ifp->lock);
-	send_mld = ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL &&
-		   ifp->idev->valid_ll_addr_cnt == 1;
+	send_mld = ifp->scope == IFA_LINK && ipv6_lonely_lladdr(ifp);
 	send_rs = send_mld &&
 		  ipv6_accept_ra(ifp->idev) &&
 		  ifp->idev->cnf.rtr_solicits > 0 &&
 		  (dev->flags&IFF_LOOPBACK) == 0;
-	spin_unlock(&ifp->lock);
 	read_unlock_bh(&ifp->idev->lock);
 
 	/* While dad is in progress mld report's source address is in6_addrany.
@@ -4598,19 +4611,6 @@ errout:
 		rtnl_set_sk_err(net, RTNLGRP_IPV6_PREFIX, err);
 }
 
-static void update_valid_ll_addr_cnt(struct inet6_ifaddr *ifp, int count)
-{
-	write_lock_bh(&ifp->idev->lock);
-	spin_lock(&ifp->lock);
-	if (((ifp->flags & (IFA_F_PERMANENT|IFA_F_TENTATIVE|IFA_F_OPTIMISTIC|
-			    IFA_F_DADFAILED)) == IFA_F_PERMANENT) &&
-	    (ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL))
-		ifp->idev->valid_ll_addr_cnt += count;
-	WARN_ON(ifp->idev->valid_ll_addr_cnt < 0);
-	spin_unlock(&ifp->lock);
-	write_unlock_bh(&ifp->idev->lock);
-}
-
 static void __ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp)
 {
 	struct net *net = dev_net(ifp->idev->dev);
@@ -4619,8 +4619,6 @@ static void __ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp)
 
 	switch (event) {
 	case RTM_NEWADDR:
-		update_valid_ll_addr_cnt(ifp, 1);
-
 		/*
 		 * If the address was optimistic
 		 * we inserted the route at the start of
@@ -4636,8 +4634,6 @@ static void __ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp)
 					      ifp->idev->dev, 0, 0);
 		break;
 	case RTM_DELADDR:
-		update_valid_ll_addr_cnt(ifp, -1);
-
 		if (ifp->idev->cnf.forwarding)
 			addrconf_leave_anycast(ifp);
 		addrconf_leave_solict(ifp->idev, &ifp->addr);
-- 
1.8.4.2

Re: [PATCH net] ipv6: simplify detection of first operational link-local address on interface

[Brian Haley]

On 01/16/2014 02:13 PM, Hannes Frederic Sowa wrote:
> In commit 1ec047eb4751e3 ("ipv6: introduce per-interface counter for
> dad-completed ipv6 addresses") I build the detection of the first
> operational link-local address much to complex. Additionally this code
> now has a race condition.
> 
> Replace it with a much simpler variant, which just scans the address
> list when duplicate address detection completes, to check if this is
> the first valid link local address and send RS and MLD reports then.
> 
> Fixes: 1ec047eb4751e3 ("ipv6: introduce per-interface counter for dad-completed ipv6 addresses")
> Reported-by: Jiri Pirko <jiri@resnulli.us>
> Cc: Flavio Leitner <fbl@redhat.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---

> +/* ifp->idev must be at least read locked */
> +static bool ipv6_lonely_lladdr(struct inet6_ifaddr *ifp)
> +{
> +	struct inet6_ifaddr *ifpiter;
> +	struct inet6_dev *idev = ifp->idev;
> +
> +	list_for_each_entry(ifpiter, &idev->addr_list, if_list) {
> +		if (ifp != ifpiter && ifpiter->scope == IFA_LINK &&
> +		    (ifpiter->flags & (IFA_F_PERMANENT|IFA_F_TENTATIVE|
> +				       IFA_F_OPTIMISTIC|IFA_F_DADFAILED)) ==
> +		    IFA_F_PERMANENT)
> +			return false;
> +	}
> +	return true;
> +}

Just a nit, but the idev->addr_list is sorted by scope, so you could use
list_for_each_entry_reverse(), and break out once the scope is > IFA_LINK to
reduce the number of compares.

-Brian

Re: [PATCH net] ipv6: simplify detection of first operational link-local address on interface

[Hannes Frederic Sowa]

On Thu, Jan 16, 2014 at 03:15:31PM -0500, Brian Haley wrote:
> On 01/16/2014 02:13 PM, Hannes Frederic Sowa wrote:
> > In commit 1ec047eb4751e3 ("ipv6: introduce per-interface counter for
> > dad-completed ipv6 addresses") I build the detection of the first
> > operational link-local address much to complex. Additionally this code
> > now has a race condition.
> > 
> > Replace it with a much simpler variant, which just scans the address
> > list when duplicate address detection completes, to check if this is
> > the first valid link local address and send RS and MLD reports then.
> > 
> > Fixes: 1ec047eb4751e3 ("ipv6: introduce per-interface counter for dad-completed ipv6 addresses")
> > Reported-by: Jiri Pirko <jiri@resnulli.us>
> > Cc: Flavio Leitner <fbl@redhat.com>
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > ---
> 
> > +/* ifp->idev must be at least read locked */
> > +static bool ipv6_lonely_lladdr(struct inet6_ifaddr *ifp)
> > +{
> > +	struct inet6_ifaddr *ifpiter;
> > +	struct inet6_dev *idev = ifp->idev;
> > +
> > +	list_for_each_entry(ifpiter, &idev->addr_list, if_list) {
> > +		if (ifp != ifpiter && ifpiter->scope == IFA_LINK &&
> > +		    (ifpiter->flags & (IFA_F_PERMANENT|IFA_F_TENTATIVE|
> > +				       IFA_F_OPTIMISTIC|IFA_F_DADFAILED)) ==
> > +		    IFA_F_PERMANENT)
> > +			return false;
> > +	}
> > +	return true;
> > +}
> 
> Just a nit, but the idev->addr_list is sorted by scope, so you could use
> list_for_each_entry_reverse(), and break out once the scope is > IFA_LINK to
> reduce the number of compares.

Very good idea, but I would leave this patch for net as-is. I'll send
one for net-next which does this optimization for the other functions
at the weekend (we can do the same optimization to ipv6_get_lladdr etc.).

Thanks,

  Hannes

Re: [PATCH net] ipv6: simplify detection of first operational link-local address on interface

[Flavio Leitner]

On Thu, Jan 16, 2014 at 08:13:04PM +0100, Hannes Frederic Sowa wrote:
> In commit 1ec047eb4751e3 ("ipv6: introduce per-interface counter for
> dad-completed ipv6 addresses") I build the detection of the first
> operational link-local address much to complex. Additionally this code
> now has a race condition.
> 
> Replace it with a much simpler variant, which just scans the address
> list when duplicate address detection completes, to check if this is
> the first valid link local address and send RS and MLD reports then.
> 
> Fixes: 1ec047eb4751e3 ("ipv6: introduce per-interface counter for dad-completed ipv6 addresses")
> Reported-by: Jiri Pirko <jiri@resnulli.us>
> Cc: Flavio Leitner <fbl@redhat.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---

Nice
Acked-by: Flavio Leitner <fbl@redhat.com>

Re: [PATCH net] ipv6: simplify detection of first operational link-local address on interface

[Jiri Pirko]

Thu, Jan 16, 2014 at 08:13:04PM CET, hannes@stressinduktion.org wrote:
>In commit 1ec047eb4751e3 ("ipv6: introduce per-interface counter for
>dad-completed ipv6 addresses") I build the detection of the first
>operational link-local address much to complex. Additionally this code
>now has a race condition.
>
>Replace it with a much simpler variant, which just scans the address
>list when duplicate address detection completes, to check if this is
>the first valid link local address and send RS and MLD reports then.
>
>Fixes: 1ec047eb4751e3 ("ipv6: introduce per-interface counter for dad-completed ipv6 addresses")
>Reported-by: Jiri Pirko <jiri@resnulli.us>
>Cc: Flavio Leitner <fbl@redhat.com>
>Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Acked-by: Jiri Pirko <jiri@resnulli.us>

Re: [PATCH net] ipv6: simplify detection of first operational link-local address on interface

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 16 Jan 2014 20:13:04 +0100

> In commit 1ec047eb4751e3 ("ipv6: introduce per-interface counter for
> dad-completed ipv6 addresses") I build the detection of the first
> operational link-local address much to complex. Additionally this code
> now has a race condition.
> 
> Replace it with a much simpler variant, which just scans the address
> list when duplicate address detection completes, to check if this is
> the first valid link local address and send RS and MLD reports then.
> 
> Fixes: 1ec047eb4751e3 ("ipv6: introduce per-interface counter for dad-completed ipv6 addresses")
> Reported-by: Jiri Pirko <jiri@resnulli.us>
> Cc: Flavio Leitner <fbl@redhat.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Applied and queued up for -stable, thanks.