From 725e4008efef32dfbe57b3e21635fa80dde8ee38 Mon Sep 17 00:00:00 2001
From: Kevin Bowling <kbowling@FreeBSD.org>
Date: Thu, 24 Aug 2023 13:42:23 -0700
Subject: [PATCH] iflib: invert default restart on VLAN changes

In rS360398, a new iflib device method was added to opt out of VLAN
events needing an interface reset.

I am switching the default to not requiring a restart for:
* VLAN events
* unknown events

After fixing various bugs, I do not think this would be a common need
of hardware and it is undesirable from the user's perspective causing
link flaps and much slower VLAN configuration. Currently, there are no
other restart events besides VLAN events, and setting the
ifdi_needs_restart default to false will alleviate the need to churn
every driver if an odd event is added in the future for specific
hardware.

markj points out this could cause churn in the other direction; I will
solve that problem with an event registration system as he mentions in
the review should we need it in the future.

These drivers will opt into restart and need further inspection or work:
* ixv (needs code audit, 61a8231 fixed principal issue; re-init probably
not necessary)
* axgbe (needs code audit; re-init probably not necessary)
* iavf - (needs code audit; interaction with Malicious Driver Detection
mentioned in rS360398)
* mgb - no VLAN functions are currently implemented. Left a comment.

MFC after:	2 weeks
Sponsored by:	BBOX.io
Differential Revision:	https://reviews.freebsd.org/D41558
---
 sys/dev/e1000/if_em.c | 5 ++---
 sys/dev/igc/if_igc.c  | 5 ++---
 sys/dev/ixgbe/if_ix.c | 5 ++---
 sys/dev/mgb/if_mgb.c  | 1 +
 sys/net/ifdi_if.m     | 2 +-
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c
index 4fa275f08e28..8488f72c4cde 100644
--- a/sys/dev/e1000/if_em.c
+++ b/sys/dev/e1000/if_em.c
@@ -4383,7 +4383,7 @@ em_if_get_counter(if_ctx_t ctx, ift_counter cnt)
  * @ctx: iflib context
  * @event: event code to check
  *
- * Defaults to returning true for unknown events.
+ * Defaults to returning false for unknown events.
  *
  * @returns true if iflib needs to reinit the interface
  */
@@ -4392,9 +4392,8 @@ em_if_needs_restart(if_ctx_t ctx __unused, enum iflib_restart_event event)
 {
 	switch (event) {
 	case IFLIB_RESTART_VLAN_CONFIG:
-		return (false);
 	default:
-		return (true);
+		return (false);
 	}
 }
 
diff --git a/sys/dev/igc/if_igc.c b/sys/dev/igc/if_igc.c
index c59e379ca2c8..a0f0906e30da 100644
--- a/sys/dev/igc/if_igc.c
+++ b/sys/dev/igc/if_igc.c
@@ -2402,7 +2402,7 @@ igc_if_get_counter(if_ctx_t ctx, ift_counter cnt)
  * @ctx: iflib context
  * @event: event code to check
  *
- * Defaults to returning true for unknown events.
+ * Defaults to returning false for unknown events.
  *
  * @returns true if iflib needs to reinit the interface
  */
@@ -2411,9 +2411,8 @@ igc_if_needs_restart(if_ctx_t ctx __unused, enum iflib_restart_event event)
 {
 	switch (event) {
 	case IFLIB_RESTART_VLAN_CONFIG:
-		return (false);
 	default:
-		return (true);
+		return (false);
 	}
 }
 
diff --git a/sys/dev/ixgbe/if_ix.c b/sys/dev/ixgbe/if_ix.c
index e8fb5de493ce..17f1f73a526e 100644
--- a/sys/dev/ixgbe/if_ix.c
+++ b/sys/dev/ixgbe/if_ix.c
@@ -1256,7 +1256,7 @@ ixgbe_if_i2c_req(if_ctx_t ctx, struct ifi2creq *req)
  * @ctx: iflib context
  * @event: event code to check
  *
- * Defaults to returning true for unknown events.
+ * Defaults to returning false for unknown events.
  *
  * @returns true if iflib needs to reinit the interface
  */
@@ -1265,9 +1265,8 @@ ixgbe_if_needs_restart(if_ctx_t ctx __unused, enum iflib_restart_event event)
 {
 	switch (event) {
 	case IFLIB_RESTART_VLAN_CONFIG:
-		return (false);
 	default:
-		return (true);
+		return (false);
 	}
 }
 
diff --git a/sys/dev/mgb/if_mgb.c b/sys/dev/mgb/if_mgb.c
index 4a23ef3cdcaf..6fafb303143c 100644
--- a/sys/dev/mgb/if_mgb.c
+++ b/sys/dev/mgb/if_mgb.c
@@ -251,6 +251,7 @@ static device_method_t mgb_iflib_methods[] = {
 	 */
 	DEVMETHOD(ifdi_vlan_register, mgb_vlan_register),
 	DEVMETHOD(ifdi_vlan_unregister, mgb_vlan_unregister),
+	DEVMETHOD(ifdi_needs_restart, mgb_if_needs_restart),
 
 	/*
 	 * Needed for WOL support
diff --git a/sys/net/ifdi_if.m b/sys/net/ifdi_if.m
index a9c285c4dd31..d52bf66bd110 100644
--- a/sys/net/ifdi_if.m
+++ b/sys/net/ifdi_if.m
@@ -115,7 +115,7 @@ CODE {
 	static bool
 	null_needs_restart(if_ctx_t _ctx __unused, enum iflib_restart_event _event __unused)
 	{
-		return (true);
+		return (false);
 	}
 };