Changeset View
Changeset View
Standalone View
Standalone View
sys/dev/hyperv/netvsc/if_hn.c
Context not available. | |||||
#include <sys/systm.h> | #include <sys/systm.h> | ||||
#include <sys/taskqueue.h> | #include <sys/taskqueue.h> | ||||
#include <sys/buf_ring.h> | #include <sys/buf_ring.h> | ||||
#include <sys/eventhandler.h> | |||||
#include <machine/atomic.h> | #include <machine/atomic.h> | ||||
#include <machine/in_cksum.h> | #include <machine/in_cksum.h> | ||||
Context not available. | |||||
#include <net/bpf.h> | #include <net/bpf.h> | ||||
#include <net/ethernet.h> | #include <net/ethernet.h> | ||||
#include <net/if.h> | #include <net/if.h> | ||||
#include <net/if_dl.h> | |||||
#include <net/if_media.h> | #include <net/if_media.h> | ||||
#include <net/if_types.h> | #include <net/if_types.h> | ||||
#include <net/if_var.h> | #include <net/if_var.h> | ||||
Context not available. | |||||
uint32_t hash_value; | uint32_t hash_value; | ||||
}; | }; | ||||
struct hn_update_vf { | |||||
struct hn_rx_ring *rxr; | |||||
struct ifnet *vf; | |||||
}; | |||||
#define HN_RXINFO_VLAN 0x0001 | #define HN_RXINFO_VLAN 0x0001 | ||||
#define HN_RXINFO_CSUM 0x0002 | #define HN_RXINFO_CSUM 0x0002 | ||||
#define HN_RXINFO_HASHINF 0x0004 | #define HN_RXINFO_HASHINF 0x0004 | ||||
Context not available. | |||||
static int hn_txagg_align_sysctl(SYSCTL_HANDLER_ARGS); | static int hn_txagg_align_sysctl(SYSCTL_HANDLER_ARGS); | ||||
static int hn_polling_sysctl(SYSCTL_HANDLER_ARGS); | static int hn_polling_sysctl(SYSCTL_HANDLER_ARGS); | ||||
static void hn_stop(struct hn_softc *); | static void hn_stop(struct hn_softc *, bool); | ||||
static void hn_init_locked(struct hn_softc *); | static void hn_init_locked(struct hn_softc *); | ||||
static int hn_chan_attach(struct hn_softc *, | static int hn_chan_attach(struct hn_softc *, | ||||
struct vmbus_channel *); | struct vmbus_channel *); | ||||
sepherosa_gmail.com: This is wrong. It breaks the hn_suspend_data() assumptions. I'd prefer to leave it as it is. | |||||
Not Done Inline ActionsPull this rxfilter setting up to hn_rxfilter_config(), that will address your concern then. sepherosa_gmail.com: Pull this rxfilter setting up to hn_rxfilter_config(), that will address your concern then. | |||||
Context not available. | |||||
HN_LOCK_ASSERT(sc); | HN_LOCK_ASSERT(sc); | ||||
if (ifp->if_flags & IFF_PROMISC) { | if ((ifp->if_flags & IFF_PROMISC) || | ||||
(sc->hn_flags & HN_FLAG_VF)) { | |||||
filter = NDIS_PACKET_TYPE_PROMISCUOUS; | filter = NDIS_PACKET_TYPE_PROMISCUOUS; | ||||
} else { | } else { | ||||
filter = NDIS_PACKET_TYPE_DIRECTED; | filter = NDIS_PACKET_TYPE_DIRECTED; | ||||
Context not available. | |||||
ifmr->ifm_active |= IFM_10G_T | IFM_FDX; | ifmr->ifm_active |= IFM_10G_T | IFM_FDX; | ||||
} | } | ||||
static void | |||||
hn_update_vf_task(void *arg, int pending __unused) | |||||
Not Done Inline ActionsMove struct definition to the beginning of this file, e.g. after struct hn_rxinfo sepherosa_gmail.com: Move struct definition to the beginning of this file, e.g. after struct hn_rxinfo | |||||
{ | |||||
struct hn_update_vf *uv = arg; | |||||
uv->rxr->hn_vf = uv->vf; | |||||
} | |||||
static void | |||||
hn_update_vf(struct hn_softc *sc, struct ifnet *vf) | |||||
{ | |||||
Not Done Inline Actionsuv->rxr->hn_vf = uv->vf Save some unnecessary local variables. sepherosa_gmail.com: uv->rxr->hn_vf = uv->vf
Save some unnecessary local variables. | |||||
struct hn_rx_ring *rxr; | |||||
struct hn_update_vf uv; | |||||
struct task task; | |||||
int i; | |||||
HN_LOCK_ASSERT(sc); | |||||
TASK_INIT(&task, 0, hn_update_vf_task, &uv); | |||||
for (i = 0; i < sc->hn_rx_ring_cnt; ++i) { | |||||
Not Done Inline ActionsHN_LOCK_ASSERT() sepherosa_gmail.com: HN_LOCK_ASSERT() | |||||
rxr = &sc->hn_rx_ring[i]; | |||||
if (i < sc->hn_rx_ring_inuse) { | |||||
uv.rxr = rxr; | |||||
uv.vf = vf; | |||||
vmbus_chan_run_task(rxr->hn_chan, &task); | |||||
} else { | |||||
rxr->hn_vf = vf; | |||||
} | |||||
} | |||||
} | |||||
static void | |||||
hn_set_vf(struct hn_softc *sc, struct ifnet *ifp, bool vf) | |||||
{ | |||||
struct ifnet *hn_ifp; | |||||
HN_LOCK(sc); | |||||
if (!(sc->hn_flags & HN_FLAG_SYNTH_ATTACHED)) | |||||
Not Done Inline ActionsHN_LOCK_ASSERT() sepherosa_gmail.com: HN_LOCK_ASSERT() | |||||
goto out; | |||||
hn_ifp = sc->hn_ifp; | |||||
if (ifp == hn_ifp) | |||||
goto out; | |||||
Not Done Inline Actionsif (hn_ifp->if_drv_flags & IFF_DRV_RUNNING) hn_rxfilter_config(sc); else hn_set_rxfilter(sc, NDIS_PACKET_TYPE_NONE); sepherosa_gmail.com: if (hn_ifp->if_drv_flags & IFF_DRV_RUNNING)
hn_rxfilter_config(sc);
else… | |||||
if (ifp->if_alloctype != IFT_ETHER) | |||||
goto out; | |||||
/* Ignore lagg/vlan interfaces */ | |||||
if (strcmp(ifp->if_dname, "lagg") == 0 || | |||||
strcmp(ifp->if_dname, "vlan") == 0) | |||||
goto out; | |||||
if (bcmp(IF_LLADDR(ifp), IF_LLADDR(hn_ifp), ETHER_ADDR_LEN) != 0) | |||||
goto out; | |||||
/* Now we're sure 'ifp' is a real VF device. */ | |||||
if (vf) { | |||||
if (sc->hn_flags & HN_FLAG_VF) | |||||
goto out; | |||||
sc->hn_flags |= HN_FLAG_VF; | |||||
hn_rxfilter_config(sc); | |||||
} else { | |||||
if (!(sc->hn_flags & HN_FLAG_VF)) | |||||
goto out; | |||||
sc->hn_flags &= ~HN_FLAG_VF; | |||||
if (sc->hn_ifp->if_drv_flags & IFF_DRV_RUNNING) | |||||
hn_rxfilter_config(sc); | |||||
else | |||||
hn_set_rxfilter(sc, NDIS_PACKET_TYPE_NONE); | |||||
} | |||||
hn_nvs_set_datapath(sc, | |||||
vf ? HN_NVS_DATAPATH_VF : HN_NVS_DATAPATH_SYNTHETIC); | |||||
hn_update_vf(sc, vf ? ifp : NULL); | |||||
if (vf) { | |||||
hn_suspend_mgmt(sc); | |||||
sc->hn_link_flags &= | |||||
~(HN_LINK_FLAG_LINKUP | HN_LINK_FLAG_NETCHG); | |||||
if_link_state_change(sc->hn_ifp, LINK_STATE_DOWN); | |||||
} else { | |||||
hn_resume_mgmt(sc); | |||||
} | |||||
if (bootverbose) | |||||
if_printf(hn_ifp, "Data path is switched %s %s\n", | |||||
vf ? "to" : "from", if_name(ifp)); | |||||
out: | |||||
HN_UNLOCK(sc); | |||||
} | |||||
static void | |||||
hn_ifnet_event(void *arg, struct ifnet *ifp, int event) | |||||
{ | |||||
if (event != IFNET_EVENT_UP && event != IFNET_EVENT_DOWN) | |||||
return; | |||||
hn_set_vf(arg, ifp, event == IFNET_EVENT_UP); | |||||
} | |||||
static void | |||||
Not Done Inline ActionsHow 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. sepherosa_gmail.com: How about pull the (hn_flags & HN_FLAG_VF) check down into hn_set_vf()? I think it actually… | |||||
hn_ifaddr_event(void *arg, struct ifnet *ifp) | |||||
{ | |||||
hn_set_vf(arg, ifp, ifp->if_flags & IFF_UP); | |||||
} | |||||
/* {F8615163-DF3E-46c5-913F-F2D2F965ED0E} */ | /* {F8615163-DF3E-46c5-913F-F2D2F965ED0E} */ | ||||
static const struct hyperv_guid g_net_vsc_device_type = { | static const struct hyperv_guid g_net_vsc_device_type = { | ||||
.hv_guid = {0x63, 0x51, 0x61, 0xF8, 0x3E, 0xDF, 0xc5, 0x46, | .hv_guid = {0x63, 0x51, 0x61, 0xF8, 0x3E, 0xDF, 0xc5, 0x46, | ||||
Context not available. | |||||
sc->hn_mgmt_taskq = sc->hn_mgmt_taskq0; | sc->hn_mgmt_taskq = sc->hn_mgmt_taskq0; | ||||
hn_update_link_status(sc); | hn_update_link_status(sc); | ||||
sc->hn_ifnet_evthand = EVENTHANDLER_REGISTER(ifnet_event, | |||||
hn_ifnet_event, sc, EVENTHANDLER_PRI_ANY); | |||||
sc->hn_ifaddr_evthand = EVENTHANDLER_REGISTER(ifaddr_event, | |||||
hn_ifaddr_event, sc, EVENTHANDLER_PRI_ANY); | |||||
return (0); | return (0); | ||||
failed: | failed: | ||||
if (sc->hn_flags & HN_FLAG_SYNTH_ATTACHED) | if (sc->hn_flags & HN_FLAG_SYNTH_ATTACHED) | ||||
Context not available. | |||||
struct hn_softc *sc = device_get_softc(dev); | struct hn_softc *sc = device_get_softc(dev); | ||||
struct ifnet *ifp = sc->hn_ifp; | struct ifnet *ifp = sc->hn_ifp; | ||||
if (sc->hn_ifaddr_evthand != NULL) | |||||
EVENTHANDLER_DEREGISTER(ifaddr_event, sc->hn_ifaddr_evthand); | |||||
if (sc->hn_ifnet_evthand != NULL) | |||||
EVENTHANDLER_DEREGISTER(ifnet_event, sc->hn_ifnet_evthand); | |||||
Not Done Inline Actionsif (hndl_tag != NULL) EVENTHANDLER_DEREGISTER(....); sepherosa_gmail.com: if (hndl_tag != NULL)
EVENTHANDLER_DEREGISTER(....); | |||||
if (sc->hn_xact != NULL && vmbus_chan_is_revoked(sc->hn_prichan)) { | if (sc->hn_xact != NULL && vmbus_chan_is_revoked(sc->hn_prichan)) { | ||||
/* | /* | ||||
* In case that the vmbus missed the orphan handler | * In case that the vmbus missed the orphan handler | ||||
Context not available. | |||||
HN_LOCK(sc); | HN_LOCK(sc); | ||||
if (sc->hn_flags & HN_FLAG_SYNTH_ATTACHED) { | if (sc->hn_flags & HN_FLAG_SYNTH_ATTACHED) { | ||||
if (ifp->if_drv_flags & IFF_DRV_RUNNING) | if (ifp->if_drv_flags & IFF_DRV_RUNNING) | ||||
hn_stop(sc); | hn_stop(sc, true); | ||||
/* | /* | ||||
* NOTE: | * NOTE: | ||||
* hn_stop() only suspends data, so managment | * hn_stop() only suspends data, so managment | ||||
Context not available. | |||||
hn_rxpkt(struct hn_rx_ring *rxr, const void *data, int dlen, | hn_rxpkt(struct hn_rx_ring *rxr, const void *data, int dlen, | ||||
const struct hn_rxinfo *info) | const struct hn_rxinfo *info) | ||||
{ | { | ||||
struct ifnet *ifp = rxr->hn_ifp; | struct ifnet *ifp; | ||||
struct mbuf *m_new; | struct mbuf *m_new; | ||||
Not Done Inline ActionsThe 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. sepherosa_gmail.com: The easiest way probably is:
if (rxr->hn_vf != NULL)
ifp = rxr->hn_vf;
else… | |||||
int size, do_lro = 0, do_csum = 1; | int size, do_lro = 0, do_csum = 1; | ||||
int hash_type; | int hash_type; | ||||
/* If the VF is active, inject the packet through the VF */ | |||||
ifp = rxr->hn_vf ? rxr->hn_vf : rxr->hn_ifp; | |||||
if (dlen <= MHLEN) { | if (dlen <= MHLEN) { | ||||
m_new = m_gethdr(M_NOWAIT, MT_DATA); | m_new = m_gethdr(M_NOWAIT, MT_DATA); | ||||
if (m_new == NULL) { | if (m_new == NULL) { | ||||
Context not available. | |||||
} | } | ||||
} else { | } else { | ||||
if (ifp->if_drv_flags & IFF_DRV_RUNNING) | if (ifp->if_drv_flags & IFF_DRV_RUNNING) | ||||
hn_stop(sc); | hn_stop(sc, false); | ||||
} | } | ||||
sc->hn_if_flags = ifp->if_flags; | sc->hn_if_flags = ifp->if_flags; | ||||
Context not available. | |||||
} | } | ||||
static void | static void | ||||
hn_stop(struct hn_softc *sc) | hn_stop(struct hn_softc *sc, bool detaching) | ||||
{ | { | ||||
struct ifnet *ifp = sc->hn_ifp; | struct ifnet *ifp = sc->hn_ifp; | ||||
int i; | int i; | ||||
Context not available. | |||||
atomic_clear_int(&ifp->if_drv_flags, IFF_DRV_OACTIVE); | atomic_clear_int(&ifp->if_drv_flags, IFF_DRV_OACTIVE); | ||||
for (i = 0; i < sc->hn_tx_ring_inuse; ++i) | for (i = 0; i < sc->hn_tx_ring_inuse; ++i) | ||||
sc->hn_tx_ring[i].hn_oactive = 0; | sc->hn_tx_ring[i].hn_oactive = 0; | ||||
/* | |||||
* If the VF is active, make sure the filter is not 0, even if | |||||
* the synthetic NIC is down. | |||||
*/ | |||||
if (!detaching && (sc->hn_flags & HN_FLAG_VF)) | |||||
hn_rxfilter_config(sc); | |||||
} | } | ||||
static void | static void | ||||
Not Done Inline ActionsMove these bits to the end of hn_stop(). It is only required there. sepherosa_gmail.com: Move these bits to the end of hn_stop(). It is only required there. | |||||
Context not available. | |||||
/* Disable polling. */ | /* Disable polling. */ | ||||
hn_polling(sc, 0); | hn_polling(sc, 0); | ||||
if (sc->hn_ifp->if_drv_flags & IFF_DRV_RUNNING) | if ((sc->hn_ifp->if_drv_flags & IFF_DRV_RUNNING) || | ||||
(sc->hn_flags & HN_FLAG_VF)) | |||||
hn_suspend_data(sc); | hn_suspend_data(sc); | ||||
hn_suspend_mgmt(sc); | hn_suspend_mgmt(sc); | ||||
} | } | ||||
Context not available. | |||||
hn_resume(struct hn_softc *sc) | hn_resume(struct hn_softc *sc) | ||||
{ | { | ||||
if (sc->hn_ifp->if_drv_flags & IFF_DRV_RUNNING) | if ((sc->hn_ifp->if_drv_flags & IFF_DRV_RUNNING) || | ||||
(sc->hn_flags & HN_FLAG_VF)) | |||||
hn_resume_data(sc); | hn_resume_data(sc); | ||||
hn_resume_mgmt(sc); | |||||
/* | |||||
* When the VF is activated, the synthetic interface is changed | |||||
* to DOWN in hn_set_vf(). Here, if the VF is still active, we | |||||
* don't call hn_resume_mgmt() until the VF is deactivated in | |||||
* hn_set_vf(). | |||||
*/ | |||||
if (!(sc->hn_flags & HN_FLAG_VF)) | |||||
hn_resume_mgmt(sc); | |||||
/* | /* | ||||
* Re-enable polling if this interface is running and | * Re-enable polling if this interface is running and | ||||
Context not available. |
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.