Re: NET_MPSAFE support for ifmedia / mii

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

> On Feb 17, 2020, at 8:51 PM, SAITOH Masanobu <msaitoh%execsw.org@localhost> wrote:
> 
> Is it required to modify all Ethernet drivers at the same time when
> the above change is committed or not?

It's not intended to be, no.

> I got the following panic with bnx(4):
> 
>> [   1.0191049] bnx0 at pci11 dev 0 function 0: Broadcom NetXtreme II BCM5708 1000Base-T
>> [   1.0191049] bnx0: Ethernet address 00:10:18:15:c9:3a
>> [   1.0191049] bnx0: ASIC BCM5708 B1 (0x57081010)
>> [   1.0191049] bnx0: PCI-X 64bit 133MHz
>> [   1.0191049] bnx0: B/C (1.8.0); Bufs (RX:2;TX:2); Flags ()
>> [   1.0191049] bnx0: Coal (RX:6,6,18,18; TX:20,20,80,80)
>> [   1.0191049] brgphy1 at bnx0 phy 1: BCM5708C 1000BASE-T media interface, rev. 5
>> [   1.0191049] brgphy1: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT, 1000baseT-FDX, auto
>> [   1.0191049] allocated pic ioapic0 type level pin 16 level 6 to cpu0 slot 3 idt entry 109
>> [   1.0191049] bnx0: interrupting at ioapic0 pin 16
>> [   1.0191049] panic: kernel diagnostic assertion "mii_locked(sc->mii_pdata)" failed: file "../../../../dev/mii/mii_physubr.c", line 405
>> [   1.0191049] cpu0: Begin traceback...
>> [   1.0191049] vpanic() at netbsd:vpanic+0x178
>> [   1.0191049] kern_assert() at netbsd:kern_assert+0x48
>> [   1.0191049] mii_phy_reset() at netbsd:mii_phy_reset+0x196
>> [   1.0191049] bnx_ifmedia_upd() at netbsd:bnx_ifmedia_upd+0x39
>> [   1.0191049] bnx_attach() at netbsd:bnx_attach+0x9c0

*snip*

> bnx_attach() calls bnx_mgmt_init() which calls bnx_ifmedia_upd(ifp).

Yes, I see the problem.  If the driver called mii_ifmedia_change(), it would have worked, because it would have handled the locking for the non-converted driver case.  I will make that change (it can be checked in ahead of the rest of the changes).  Actually, mii_mediachg() would have also handled the situation for a non-converted driver, but in box's case, the problem with calling mii_phy_reset().

In addition to that fix, I want to make one additional small tweak (and have also fixed a couple of issues with usbnet-based drivers).  New diff at http://www.netbsd.org/~thorpej/ifmedia-mpsafe-diff.txt.

MD5 (ifmedia-mpsafe-diff.txt) = c1d085c51a2dd474443ae1c1fcf0fabc

I guess I need to do an audit to see what other drivers are calling their "ifm_change" function directly; they could have the same problem.

-- thorpej