[ofa-general] Re: [openib-general] [RFC] [PATCH v2] IB/ipoib: Add bonding support to IPoIB

[Michael S. Tsirkin]

> Quoting Moni Shoua <monisonlists at gmail.com>:
> Subject: Re: [openib-general] [RFC] [PATCH v2] IB/ipoib: Add bonding support to IPoIB
> 
> Michael S. Tsirkin wrote:
> >> Quoting Moni Shoua <monisonlists at gmail.com>:
> >> Subject: Re: [openib-general] [RFC] [PATCH v2] IB/ipoib: Add bonding support to IPoIB
> >>
> >> This version of the patch tracks the allocs and releases of ipoib_neigh and
> >>  keeps a list of them.  Before IPoIB net device unregisters the list is passed
> >> to destroy ipoib_neighs that ride on on a bond neighbour.
> >>
> >> This is a replacement to the method of scanning the arp and ndisc
> >> tables.
> > 
> > Why does the list need to be global?
> > We already have a per-device list of paths, and each of these in turn
> > has a list of neighbours. Can't this be used?
> > 
> OK, It's a good point but coming to think of it now I have a question
> 
> When a device unregisters ipoib_stop() is called and all ipoib_neighs are destroyed.
>    Isn't it enough to ensure that ipoib_neigh_destructor will not try to
>    "touch" one of the ib devs?  or in other words: Isn't it that the work to
>    clean ipoib_neighs is unnecessary?
>
> BTW: I guess that idea of global list was influenced from the ipoib_8111... patch. 
> Why was it used there?

AFAIK, the point is to check whether (in pre-2.6.17 kernels) some neighbour
shares the same ops pointer. Only after no such neighbours are left is it safe
to set the destructor to NULL.

This backport is not raceless BTW - some neighbour not related to IPoIB could
be running the destructor. But I think it's the best I could come up with
for these old kernels.

-- 
MST