Re: NET_MPSAFE and ether_ioctl()

[Jason Thorpe <thorpej%me.com@localhost>]

> On Jan 16, 2020, at 11:09 AM, Michael van Elst <mlelstv%serpens.de@localhost> wrote:
> 
> On Thu, Jan 16, 2020 at 09:43:31AM -0800, Jason Thorpe wrote:
>> 
>>>> I'm a little confused as to why it's at all correct for a driver to be using splnet when running in NET_MPSAFE mode.
> 
> When it calls into non-MPSAFE code, then taking KERNEL_LOCK and possibly
> raising spl is obviously necessary.

Yes, and in places where taking KERNEL_LOCK is necessary, the code that does that should raise the IPL itself, IMO.  We should be pushing the complexity of this OUT of drivers and into the common code to make finishing the feature easier.  (This is what my change for ifmedia_ioctl() is all about.)

>> Actually, it looks to me that ether_ioctl() IS MP-safe, and that's backed up by the fact that no path to ether_ioctl() or inside ether_ioctl() directly takes the KERNEL_LOCK, as it would otherwise need to do; splnet() is NOT sufficient, and I guess my point here is that I think it's not necessary :-).
> 
> To me this just says that the NET_MPSAFE code path is buggy and must also
> take KERNEL_LOCK when calling into ether_* and ifmedia_* code.
> 
> ether_ioctl being MP-safe isn't obvious to me, it touches ifp fields,
> calls into ifmedia itself and even arp code.

The ifp fields (well, at least some of them) are protected by the if_ioctl_lock according to the comments, which will be held at the time ether_ioctl() is called.  (I am happy to add KASSERTs to verify this.)

Again, I already have a local change to push the "go to splnet()" complexity inside ifmedia_ioctl() itself.  I will double-check the ARP code.

-- thorpej