hyperv: netvsc: support SIOCGIFMEDIA/SIOCSIFMEDIA
AbandonedPublic
Actions

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

Details

Reviewers

kmacy
howard0su_gmail.com
honzhan_microsoft.com
whu
delphij
sepherosa_gmail.com
royger

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 - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 1720
Build 1726: arc lint + arc unit

Event Timeline

decui_microsoft.com updated this revision to Diff 11409.Dec 18 2015, 9:29 AM

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

decui_microsoft.com updated this object.

decui_microsoft.com added reviewers: whu, royger, delphij, kmacy, howard0su_gmail.com, honzhan_microsoft.com.

Herald added a subscriber: imp. · View Herald TranscriptDec 18 2015, 9:29 AM

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

In D4611#99051, @howard0su_gmail.com wrote:

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:

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

decui_microsoft.com added a reviewer: sepherosa_gmail.com.Jan 8 2016, 2:53 AM

sepherosa_gmail.com added inline comments.Jan 8 2016, 3:25 AM

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