Page MenuHomeFreeBSD

hyperv: netvsc: support SIOCGIFMEDIA/SIOCSIFMEDIA
AbandonedPublic

Authored by decui_microsoft.com on Dec 18 2015, 9:29 AM.

Details

Summary

Without the patch, 'ifconfig hn0' doesn't have these 2 lines:

media: Ethernet autoselect <full-duplex>
status: active

(If we 'ifconfig down hn0', the second line will be "status: no carrier".)

We got the complaint from a customer, because the customer's
application doesn't expect the support is unavailable.

Signed-off-by: Dexuan Cui <decui@microsoft.com>

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 1720
Build 1726: arc lint + arc unit

Event Timeline

decui_microsoft.com retitled this revision from to hyperv: netvsc: support SIOCGIFMEDIA/SIOCSIFMEDIA.
decui_microsoft.com updated this object.

Please check hv_rf_receive_indicate_status in hv_rndis_filter.c, which has callback from hypervisor about media status. i believe we need integrate with this change so that we can report the correct status

Please check hv_rf_receive_indicate_status in hv_rndis_filter.c, which has callback from hypervisor about media status. i believe we need integrate with this change so that we can report the correct status

Thanks for pointing this out!

Currently hv_rf_receive_indicate_status() -> netvsc_linkstatus_callback() is basically a dummy operation, i.e., sc->hn_carrier is not used at all...
Obviously we need to add the support of handling the link status change and I think we can add the support in another patch.

Here this patch itself is useful because:

  1. it can fix https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=187006
  2. after we add the handling of link status change, here the logic (i.e., "testing sc->hn_if_flags & IFF_UP") is still correct.
sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c
227

Heh, well, we don't need the IFM_FDX here. As we have discussed, maybe set it to IFM_10G_T | IFM_FDX. And please move the HN_MEDIATYPE to the beginning of the source file.

237–242

As Jun Su said, we should rely on hn_carrier.

And logic need adjustment:

if (!hn_carrier) {

ifmr->ifm_active |= IFM_NONE;
return;

}
ifmr->ifm_active |= HN_MEDIATYPE;
ifmr->ifm_status |= IFM_ACTIVE;

Well, I think IFM_AUTO is not a good MEDIATYPE :P

323

You better set hn_media.ifm_media manually to hn_media.ifm_cur->ifm_media after ifmedia_set(), else it points to 0 (IFM_AUTO).

364–365

Extra blank line.

1156

Just return EOPNOTSUPP for SIFMEDIA

Please goto https://reviews.freebsd.org/D4852 for the new version.