| Message ID | 1455728431-21976-1-git-send-email-razor@blackwall.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On 2/17/16 10:00 AM, Nikolay Aleksandrov wrote: > From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > > When I used netdev_for_each_lower_dev in commit bad531623253 ("vrf: > remove slave queue and private slave struct") I thought that it acts > like netdev_for_each_lower_private and can be used to remove the current > device from the list while walking, but unfortunately it acts more like > netdev_for_each_lower_private_rcu and doesn't allow it. The difference > is where the "iter" points to, right now it points to the current element > and that makes it impossible to remove it. Change the logic to be > similar to netdev_for_each_lower_private and make it point to the "next" > element so we can safely delete the current one. VRF is the only such > user right now, there's no change for the read-only users. > -----8<----- > > CC: David Ahern <dsa@cumulusnetworks.com> > CC: David S. Miller <davem@davemloft.net> > CC: Roopa Prabhu <roopa@cumulusnetworks.com> > CC: Vlad Yasevich <vyasevic@redhat.com> > Fixes: bad531623253 ("vrf: remove slave queue and private slave struct") > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > --- > include/linux/netdevice.h | 2 +- > net/core/dev.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) Solves the problem for me. Thanks for the quick turnaround, Nik. Reviewed-by / Tested-by: David Ahern <dsa@cumulusnetworks.com>
From: David Ahern <dsa@cumulusnetworks.com> Date: Wed, 17 Feb 2016 12:01:10 -0700 > On 2/17/16 10:00 AM, Nikolay Aleksandrov wrote: >> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> >> >> When I used netdev_for_each_lower_dev in commit bad531623253 ("vrf: >> remove slave queue and private slave struct") I thought that it acts >> like netdev_for_each_lower_private and can be used to remove the >> current >> device from the list while walking, but unfortunately it acts more >> like >> netdev_for_each_lower_private_rcu and doesn't allow it. The difference >> is where the "iter" points to, right now it points to the current >> element >> and that makes it impossible to remove it. Change the logic to be >> similar to netdev_for_each_lower_private and make it point to the >> "next" >> element so we can safely delete the current one. VRF is the only such >> user right now, there's no change for the read-only users. ... >> Fixes: bad531623253 ("vrf: remove slave queue and private slave >> struct") >> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> >> --- >> include/linux/netdevice.h | 2 +- >> net/core/dev.c | 4 ++-- >> 2 files changed, 3 insertions(+), 3 deletions(-) > > Solves the problem for me. Thanks for the quick turnaround, Nik. > > Reviewed-by / Tested-by: David Ahern <dsa@cumulusnetworks.com> David, please explicitly list these tags one by one, patchwork is not able to pick them up if you try to free-form the tags. I had to incorporate them by hand, but that makes more work for me. Applied, thanks.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 289c2314d766..5440b7b705eb 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3718,7 +3718,7 @@ void *netdev_lower_get_next_private_rcu(struct net_device *dev, void *netdev_lower_get_next(struct net_device *dev, struct list_head **iter); #define netdev_for_each_lower_dev(dev, ldev, iter) \ - for (iter = &(dev)->adj_list.lower, \ + for (iter = (dev)->adj_list.lower.next, \ ldev = netdev_lower_get_next(dev, &(iter)); \ ldev; \ ldev = netdev_lower_get_next(dev, &(iter))) diff --git a/net/core/dev.c b/net/core/dev.c index 8cba3d852f25..4392ef45864c 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5379,12 +5379,12 @@ void *netdev_lower_get_next(struct net_device *dev, struct list_head **iter) { struct netdev_adjacent *lower; - lower = list_entry((*iter)->next, struct netdev_adjacent, list); + lower = list_entry(*iter, struct netdev_adjacent, list); if (&lower->list == &dev->adj_list.lower) return NULL; - *iter = &lower->list; + *iter = lower->list.next; return lower->dev; }