[ofa-general] Re: [PATCH V4 10/10] net/bonding: Destroy bonding master when last slave is gone

[Jay Vosburgh]

Moni Shoua <monisonlists at gmail.com> wrote:

>Jay Vosburgh wrote:
>> Moni Shoua <monis at voltaire.com> wrote:
>> 
>>> When bonding enslaves non Ethernet devices it takes pointers to functions 
>>> in the module that owns the slaves. In this case it becomes unsafe
>>> to keep the bonding master registered after last slave was unenslaved 
>>> because we don't know if the pointers are still valid.  Destroying the bond when slave_cnt is zero
>>> ensures that these functions be used anymore.
>> 
>> 	Would it not be simpler to run the bonding master through
>> ether_setup() again when the final slave is released (to reset all of
>> the pointers to their "ethernet" values)?  I'm presuming here the
>> pointers of questionable validity are the ones set in the
>> bond_setup_by_slave() copied from the slave_dev->hard_header, et al.
>> 
>> 	Having the bonding master disappear (but only sometimes) after
>> the last slave is removed is a semantic change I'd rather not introduce
>> if it's not necessary.
>
>Thanks for the comments.
>
>Having the master disappear is one way I could think of to solve the problem of leaving
>the bonding module with pointers to illegal addresses.
>The other way is to increase the usage count, with try_module_get(), of the module which owns of the slave.
>To do that I  have to restore the field  owner in structure net_device (it was removed in 2.6).

	What I was asking above is really whether or not it's feasible
to simply reset the affected pointers back to the "ethernet" values from
ether_setup().  I would think this should return the bonding master back
to the original state it started in before any slaves were added.
Unless I'm missing something; I'm willing to believe there's some
IB-specific tidbit I'm unaware of that makes this more complicated than
it seems.

	This presumes that I'm correct in thinking that the pointers
you're talking about (as being unsafe after removal of last slave) are
the ones copied in your new function bond_setup_by_slave().

	I don't think it's desirable to acquire a reference to the slave
driver module.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar at us.ibm.com