iflib: Stop interface before (un)registering VLAN

This patch is intended to solve a specific problem that iavf(4)
encounters, but what it does can be extended to solve other issues.

To summarize the iavf(4) issue, if the PF driver configures VLAN
anti-spoof, then the VF driver needs to make sure no untagged traffic is
sent if a VLAN is configured, and vice-versa. This can be an issue when
a VLAN is being registered or unregistered, e.g. when a packet may be on
the ring with a VLAN in it, but the VLANs are being unregistered. This
can cause that tagged packet to go out and cause an MDD event.

To fix this, include a new interface-dependent function that drivers can
implement named IFDI_NEEDS_RESTART(). Right now, this function is called
in iflib_vlan_unregister/register() to determine whether the interface
needs to be stopped and started when a VLAN is registered or
unregistered. The default return value of IFDI_NEEDS_RESTART() is true,
so this fixes the MDD problem that iavf(4) encounters, since the
interface rings are flushed during a stop/init.

A future change to iavf(4) will implement that function just in case the
default value changes, and to make it explicit that this interface reset
is required when a VLAN is added or removed.

Reviewed by:	gallatin@
MFC after:	1 week
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D22086
This commit is contained in:
Eric Joyner 2020-04-27 22:02:44 +00:00
parent 02343a67c2
commit 45818bf1a0
3 changed files with 32 additions and 6 deletions

View File

@ -169,6 +169,12 @@ CODE {
}
return (0);
}
static bool
null_needs_restart(if_ctx_t _ctx __unused, enum iflib_restart_event _event __unused)
{
return (true);
}
};
#
@ -456,3 +462,8 @@ METHOD int sysctl_int_delay {
METHOD void debug {
if_ctx_t _ctx;
} DEFAULT null_void_op;
METHOD bool needs_restart {
if_ctx_t _ctx;
enum iflib_restart_event _event;
} DEFAULT null_needs_restart;

View File

@ -4308,10 +4308,13 @@ iflib_vlan_register(void *arg, if_t ifp, uint16_t vtag)
return;
CTX_LOCK(ctx);
/* Driver may need all untagged packets to be flushed */
if (IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_VLAN_CONFIG))
iflib_stop(ctx);
IFDI_VLAN_REGISTER(ctx, vtag);
/* Re-init to load the changes */
if (if_getcapenable(ifp) & IFCAP_VLAN_HWFILTER)
iflib_if_init_locked(ctx);
/* Re-init to load the changes, if required */
if (IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_VLAN_CONFIG))
iflib_init_locked(ctx);
CTX_UNLOCK(ctx);
}
@ -4327,10 +4330,13 @@ iflib_vlan_unregister(void *arg, if_t ifp, uint16_t vtag)
return;
CTX_LOCK(ctx);
/* Driver may need all tagged packets to be flushed */
if (IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_VLAN_CONFIG))
iflib_stop(ctx);
IFDI_VLAN_UNREGISTER(ctx, vtag);
/* Re-init to load the changes */
if (if_getcapenable(ifp) & IFCAP_VLAN_HWFILTER)
iflib_if_init_locked(ctx);
/* Re-init to load the changes, if required */
if (IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_VLAN_CONFIG))
iflib_init_locked(ctx);
CTX_UNLOCK(ctx);
}

View File

@ -376,6 +376,15 @@ typedef enum {
*/
#define IFLIB_SINGLE_IRQ_RX_ONLY 0x40000
/*
* These enum values are used in iflib_needs_restart to indicate to iflib
* functions whether or not the interface needs restarting when certain events
* happen.
*/
enum iflib_restart_event {
IFLIB_RESTART_VLAN_CONFIG,
};
/*
* field accessors
*/