hyperv/hn: add the support for VF drivers
ClosedPublic

Authored by decui_microsoft.com on Dec 29 2016, 7:54 AM.

Details

Summary

Hyper-V's NIC SR-IOV implementation needs a Hyper-V synthetic NIC and
a VF NIC to work together (both NICs have the same MAC address), mainly to
support seamless live migration.

When the VF device becomes UP (or DOWN), the synthetic NIC driver needs
to switch the data path from the synthetic NIC to the VF (or the opposite).

Note: multicast/broadcast packets are still received through the synthetic
NIC and we need to inject the packets through the VF interface (if the VF is
UP), even if the synthetic NIC is DOWN (so we need to force the rxfilter
to be NDIS_PACKET_TYPE_PROMISCUOUS, when the VF is UP).

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
decui_microsoft.com retitled this revision from to hyperv/hn: add the support for VF drivers.Dec 29 2016, 7:54 AM
decui_microsoft.com updated this object.
decui_microsoft.com edited the test plan for this revision. (Show Details)
sys/dev/hyperv/netvsc/hn_nvs.c
726 ↗(On Diff #23383)

I'd prefer to pass HN_NVS_DATAPATH_* as the second arg.

sys/dev/hyperv/netvsc/if_hn.c
689–706 ↗(On Diff #23383)

This is wrong. It breaks the hn_suspend_data() assumptions. I'd prefer to leave it as it is. Let's fix other places properly in the next pass.

905–908 ↗(On Diff #23383)

Move struct definition to the beginning of this file, e.g. after struct hn_rxinfo

914–917 ↗(On Diff #23383)
uv->rxr->hn_vf = uv->vf

Save some unnecessary local variables.

927 ↗(On Diff #23383)

HN_LOCK_ASSERT()

947 ↗(On Diff #23383)

HN_LOCK_ASSERT()

953 ↗(On Diff #23383)
if (hn_ifp->if_drv_flags & IFF_DRV_RUNNING)
    hn_rxfilter_config(sc);
else
    hn_set_rxfilter(sc, NDIS_PACKET_TYPE_NONE);
1014 ↗(On Diff #23383)

How about pull the (hn_flags & HN_FLAG_VF) check down into hn_set_vf()? I think it actually belongs there. And you can share code for hn_ifaddr_event/hn_ifnet_event then.

1367–1368 ↗(On Diff #23383)
if (hndl_tag != NULL)
    EVENTHANDLER_DEREGISTER(....);
2259–2260 ↗(On Diff #23383)

The easiest way probably is:

if (rxr->hn_vf != NULL)
    ifp = rxr->hn_vf;
else
    ifp = rxr->hn_ifp;

Rest of the changes in this function is unnecessary then.

sys/dev/hyperv/netvsc/if_hnvar.h
239–240 ↗(On Diff #23383)

How about hn_ifaddr_evthand, and hn_ifnet_evthand?

sys/dev/hyperv/netvsc/if_hn.c
689–706 ↗(On Diff #23383)

Pull this rxfilter setting up to hn_rxfilter_config(), that will address your concern then.

This addressed all the comments.

See the comment, rest looks good.

sys/dev/hyperv/netvsc/if_hn.c
4976–4982 ↗(On Diff #23434)

Move these bits to the end of hn_stop(). It is only required there.

decui_microsoft.com updated this object.
  1. Moved these bits to the end of hn_stop().
  2. Call hn_rxfilter_config() by force after the MTU changes.
  3. fixed a typo in Summary: "to to" -> "to".
This revision has a positive review.Dec 30 2016, 6:28 AM

update the patches per Sephe's comments:

  1. In hn_set_vf(), add the checks for if_alloctype/IFT_ETHER, and lagg/vlan.
  2. add the link management code in hv_set_vf().
  3. in hn_suspend/resume(), if the VF is active, also call hn_suspend/resume_data() .
This revision now requires review to proceed.Jan 9 2017, 8:59 AM
This revision has a positive review.Jan 9 2017, 9:13 AM
This revision was automatically updated to reflect the committed changes.