| Message ID | 1335195282-28738-1-git-send-email-ogerlitz@mellanox.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Or Gerlitz <ogerlitz@mellanox.com> Date: Mon, 23 Apr 2012 18:34:42 +0300 > From: Shlomo Pongratz <shlomop@mellanox.com> > > Move call the neigh_cleanup call to be done from neigh_destroy, this > ensures the cleanup callback is invoked only when the neighbour reference > count becomes zero (e.g in the same manner as ndo_neigh_destory). > > Note that with this change neigh_destroy will truly revert the action of > neigh_create, as neigh_release which calls neigh_destroy is called directly > from various code paths, and thus neigh->parms->neigh_cleanup is not called > for these code paths. > > Also, with commit 7d26bb103 "bonding: emit event when bonding changes MAC" that > triggers netdev address change event, a race was introduced since neigh cleanup > can be called when there's reference on the neighbour. > > Signed-off-by: Shlomo Pongratz <shlomop@mellanox.com> > Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com> With IPOIB, the neigh_cleanup purges references to the neigh when it liberates the SKBs in the neigh resolution TX backlog during ipoib_neigh_free(). Therefore, with your change, IPOIB neighs will never be destroyed if they have any SKBs in their neigh resolution queues. I'm not applying this, it's buggy. -- 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
On 4/24/2012 7:53 AM, David Miller wrote: > With IPOIB, the neigh_cleanup purges references to the neigh when it > liberates the SKBs in the neigh resolution TX backlog during > ipoib_neigh_free(). Therefore, with your change, IPOIB neighs will > never be destroyed if they have any SKBs in their neigh resolution > queues. I'm not applying this, it's buggy. Oh yes indeed, so we will need to address that dependency too when coming to fix the ipoib neigh related races, thanks for spotting this over. Or. -- 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
diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 0a68045..f4a4e64 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -106,9 +106,6 @@ static int neigh_blackhole(struct neighbour *neigh, struct sk_buff *skb) static void neigh_cleanup_and_release(struct neighbour *neigh) { - if (neigh->parms->neigh_cleanup) - neigh->parms->neigh_cleanup(neigh); - __neigh_notify(neigh, RTM_DELNEIGH, 0); neigh_release(neigh); } @@ -724,6 +721,9 @@ void neigh_destroy(struct neighbour *neigh) skb_queue_purge(&neigh->arp_queue); neigh->arp_queue_len_bytes = 0; + if (neigh->parms->neigh_cleanup) + neigh->parms->neigh_cleanup(neigh); + if (dev->netdev_ops->ndo_neigh_destroy) dev->netdev_ops->ndo_neigh_destroy(neigh);