> Quoting Michael S. Tsirkin <[EMAIL PROTECTED]>: > Subject: Re: dst_ifdown breaks infiniband? > > > Quoting Michael S. Tsirkin <[EMAIL PROTECTED]>: > > Subject: Re: dst_ifdown breaks infiniband? > > > > Quoting Alexey Kuznetsov <[EMAIL PROTECTED]>: > > Subject: Re: dst_ifdown breaks infiniband? > > > > Can dst->neighbour be changed to point to NULL instead, and the > > > > neighbour > > > > released? > > > > > > It should be cleared and we should be sure it will not be destroyed > > > before quiescent state. > > > > > > Seems, this is the only correct solution, but to do this we have > > > to audit all the places where dst->neighbour is dereferenced for > > > RCU safety. > > > > > > Actually, it is very good you caught this eventually, the bug was > > > so _disgusting_ that it was "forgotten" all the time, waiting for > > > someone who will point out that the king is naked. :-) > > > > Actually that might not be too bad: > > $grep -rIi 'dst->neighbour' net/ | wc -l > > 36 > > > > I'll try to do it. > > Here's the list. Looks OK to me. What do you think? >
So Alexey, how does the following (lightly tested) patch look? Is this what you had in mind? ----------------------------- Fix dst_ifdown for infiniband. Changing dst->neighbour->dev is unsafe because neigh->parms callbacks are set up for specific device. We should drop the dst->neighbour reference instead. Signed-off-by: Michael S. Tsirkin <[EMAIL PROTECTED]> --- diff --git a/net/core/dst.c b/net/core/dst.c index 764bccb..27091a5 100644 --- a/net/core/dst.c +++ b/net/core/dst.c @@ -15,6 +15,7 @@ #include <linux/skbuff.h> #include <linux/string.h> #include <linux/types.h> +#include <linux/if_arp.h> #include <net/dst.h> @@ -235,6 +236,8 @@ again: static inline void dst_ifdown(struct dst_entry *dst, struct net_device *dev, int unregister) { + struct neighbour *neigh; + if (dst->ops->ifdown) dst->ops->ifdown(dst, dev, unregister); @@ -245,13 +248,13 @@ static inline void dst_ifdown(struct dst_entry *dst, struct net_device *dev, dst->input = dst_discard_in; dst->output = dst_discard_out; } else { + neigh = dst->neighbour; dst->dev = &loopback_dev; dev_hold(&loopback_dev); dev_put(dev); - if (dst->neighbour && dst->neighbour->dev == dev) { - dst->neighbour->dev = &loopback_dev; - dev_put(dev); - dev_hold(&loopback_dev); + if (neigh && neigh->dev == dev) { + dst->neighbour = NULL; + neigh_release(neigh); } } } -- MST - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html