tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: link-state change for virtual interfaces



 ---- On Wed, 02 Oct 2024 06:55:04 +0100  Rin Okuyama  wrote --- 
 > Hi,
 > 
 > On 2024/09/26 1:54, Roy Marples wrote:
 > > 1) shmif_mediachange() should not test ifp->if_link_state, it should just call if_link_state_change()
 > > 
 > > 2) shmif_mediastatus() does not set IFM_AVALID or IFM_ACTIVE in ifm_status
 > 
 > I've committed fix and requested to pullup to netbsd-10:
 > https://releng.netbsd.org/cgi-bin/req-10.cgi?show=924
 > 
 > Let me thank you again for careful review!!

Nice!

Two more recommendations:

1)  You could hookup ifmedia to ethercom in softc so you don't have to handle the ifmedia ioctls.
     Should shrink the code a little.

2)  Call shmif_mediachange() after if_register() in allocif() so that ifconfig reports link_state_up rather than link_state_unknown from the get go.

I've never been a fan of link_state_unknown, but I understand why we need it as some hardware just doesn't have a link state to report.
On a "modern" interface, every time the hardware says "i have carrier" we set link_state_up. This happens so fast we just never see link_state_unknown.

I would much rather link_state_unknown be treated as "this interface does not support link status" rather than "i dunno .... maybe up?"

 > > This probably let to the addition of linkstate to `ifconfig -v` which wasn't really needed.
 > > If it sets IFM_AVALID then the status line as shown anove will appear.
 > 
 > Yes, now "linkstate:" line became redundant for shmif(4). But
 > we hope not to remove this feature from ifconfig(8); It is still
 > useful for debugging, e.g., broken if/mii devices. Also, it gets
 > output only if `-v` option is specified.

We can keep it, it does not harm, and aids in working out link state when the interface has buggy ifmedia code.

Roy


Home | Main Index | Thread Index | Old Index