Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 27 Apr 2020 22:02:44 +0000 (UTC)
From:      Eric Joyner <erj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r360398 - head/sys/net
Message-ID:  <202004272202.03RM2iR2012757@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: erj
Date: Mon Apr 27 22:02:44 2020
New Revision: 360398
URL: https://svnweb.freebsd.org/changeset/base/360398

Log:
  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

Modified:
  head/sys/net/ifdi_if.m
  head/sys/net/iflib.c
  head/sys/net/iflib.h

Modified: head/sys/net/ifdi_if.m
==============================================================================
--- head/sys/net/ifdi_if.m	Mon Apr 27 21:51:22 2020	(r360397)
+++ head/sys/net/ifdi_if.m	Mon Apr 27 22:02:44 2020	(r360398)
@@ -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;

Modified: head/sys/net/iflib.c
==============================================================================
--- head/sys/net/iflib.c	Mon Apr 27 21:51:22 2020	(r360397)
+++ head/sys/net/iflib.c	Mon Apr 27 22:02:44 2020	(r360398)
@@ -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 vt
 		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);
 }
 

Modified: head/sys/net/iflib.h
==============================================================================
--- head/sys/net/iflib.h	Mon Apr 27 21:51:22 2020	(r360397)
+++ head/sys/net/iflib.h	Mon Apr 27 22:02:44 2020	(r360398)
@@ -377,6 +377,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
  */
 void *iflib_get_softc(if_ctx_t ctx);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202004272202.03RM2iR2012757>