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