Index | Thread | Search

From:
Jan Klemkow <jan@openbsd.org>
Subject:
Re: refactor LRO turn off
To:
tech@openbsd.org
Date:
Wed, 19 Feb 2025 21:46:36 +0100

Download raw body.

Thread
ping?

On Wed, Oct 23, 2024 at 08:14:13PM GMT, Jan Klemkow wrote:
> This diff refactors the mechanism to turn off LRO on network interfaces.
> First, it improves the handling of pseudo devices.  It no longer uses
> their data structures from the outside.  It just forwards the
> SIOCSIFXFLAGS command to their ioctl(2) handler.  So, devices like
> trunk(4) can call ifsetlro() for all its parents interfaces by themself.
> Now, the LRO turn off call is forwarded through trunk(4), vlan(4) and
> friends to their parent interfaces.
> 
> Second, we just need reinit the ix(4) interface to turn off/no LRO. So,
> vio(4) and vmx(4) can switch this feature without going down and up
> again.
> 
> Tests in different environments are welcome.

reattached diff below.

bye,
Jan

Index: dev/pci/if_ix.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
diff -u -p -r1.218 if_ix.c
--- dev/pci/if_ix.c	4 Oct 2024 05:22:10 -0000	1.218
+++ dev/pci/if_ix.c	19 Feb 2025 20:25:42 -0000
@@ -550,6 +550,19 @@ ixgbe_ioctl(struct ifnet * ifp, u_long c
 		}
 		break;
 
+	case SIOCSIFXFLAGS:
+		if (ISSET(ifr->ifr_flags, IFXF_LRO) !=
+		    ISSET(ifp->if_xflags, IFXF_LRO)) {
+			if (ISSET(ifr->ifr_flags, IFXF_LRO))
+				SET(ifp->if_xflags, IFXF_LRO);
+			else
+				CLR(ifp->if_xflags, IFXF_LRO);
+
+			if (ifp->if_flags & IFF_UP)
+				ixgbe_init(sc);
+		}
+		break;
+
 	case SIOCSIFMEDIA:
 	case SIOCGIFMEDIA:
 		IOCTL_DEBUGOUT("ioctl: SIOCxIFMEDIA (Get/Set Interface Media)");
Index: dev/pci/if_vmx.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
diff -u -p -r1.90 if_vmx.c
--- dev/pci/if_vmx.c	24 Jan 2025 10:29:43 -0000	1.90
+++ dev/pci/if_vmx.c	19 Feb 2025 20:25:42 -0000
@@ -1370,6 +1370,19 @@ vmxnet3_reset(struct vmxnet3_softc *sc)
 	WRITE_CMD(sc, VMXNET3_CMD_RESET);
 }
 
+void
+vmxnet4_set_features(struct vmxnet3_softc *sc)
+{
+	struct ifnet *ifp = &sc->sc_arpcom.ac_if;
+
+	/* TCP Large Receive Offload */
+	if (ISSET(ifp->if_xflags, IFXF_LRO))
+		SET(sc->sc_ds->upt_features, UPT1_F_LRO);
+	else
+		CLR(sc->sc_ds->upt_features, UPT1_F_LRO);
+	WRITE_CMD(sc, VMXNET3_CMD_SET_FEATURE);
+}
+
 int
 vmxnet3_init(struct vmxnet3_softc *sc)
 {
@@ -1403,12 +1416,7 @@ vmxnet3_init(struct vmxnet3_softc *sc)
 		return EIO;
 	}
 
-	/* TCP Large Receive Offload */
-	if (ISSET(ifp->if_xflags, IFXF_LRO))
-		SET(sc->sc_ds->upt_features, UPT1_F_LRO);
-	else
-		CLR(sc->sc_ds->upt_features, UPT1_F_LRO);
-	WRITE_CMD(sc, VMXNET3_CMD_SET_FEATURE);
+	vmxnet4_set_features(sc);
 
 	/* Program promiscuous mode and multicast filters. */
 	vmxnet3_iff(sc);
@@ -1474,6 +1482,17 @@ vmxnet3_ioctl(struct ifnet *ifp, u_long 
 		} else {
 			if (ifp->if_flags & IFF_RUNNING)
 				vmxnet3_stop(ifp);
+		}
+		break;
+	case SIOCSIFXFLAGS:
+		if (ISSET(ifr->ifr_flags, IFXF_LRO) !=
+		    ISSET(ifp->if_xflags, IFXF_LRO)) {
+			if (ISSET(ifr->ifr_flags, IFXF_LRO))
+				SET(ifp->if_xflags, IFXF_LRO);
+			else
+				CLR(ifp->if_xflags, IFXF_LRO);
+
+			vmxnet4_set_features(sc);
 		}
 		break;
 	case SIOCSIFMEDIA:
Index: dev/pv/if_vio.c
===================================================================
RCS file: /cvs/src/sys/dev/pv/if_vio.c,v
diff -u -p -r1.69 if_vio.c
--- dev/pv/if_vio.c	30 Jan 2025 07:48:50 -0000	1.69
+++ dev/pv/if_vio.c	19 Feb 2025 20:25:42 -0000
@@ -352,6 +352,7 @@ int	vio_set_rx_filter(struct vio_softc *
 void	vio_iff(struct vio_softc *);
 int	vio_media_change(struct ifnet *);
 void	vio_media_status(struct ifnet *, struct ifmediareq *);
+void	vio_set_offloads(struct ifnet *);
 int	vio_ctrleof(struct virtqueue *);
 int	vio_ctrl_start(struct vio_softc *, uint8_t, uint8_t, int, int *);
 int	vio_ctrl_submit(struct vio_softc *, int);
@@ -953,6 +954,28 @@ vio_media_status(struct ifnet *ifp, stru
 		imr->ifm_status |= IFM_ACTIVE|IFM_FDX;
 }
 
+void
+vio_set_offloads(struct ifnet *ifp)
+{
+	struct vio_softc	*sc = ifp->if_softc;
+	struct virtio_softc	*vsc = sc->sc_virtio;
+	uint64_t		 features = 0;
+
+	if (virtio_has_feature(vsc, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) {
+		if (virtio_has_feature(vsc, VIRTIO_NET_F_GUEST_CSUM))
+			SET(features, VIRTIO_NET_F_GUEST_CSUM);
+
+		if (ISSET(ifp->if_xflags, IFXF_LRO)) {
+			if (virtio_has_feature(vsc, VIRTIO_NET_F_GUEST_TSO4))
+				SET(features, VIRTIO_NET_F_GUEST_TSO4);
+			if (virtio_has_feature(vsc, VIRTIO_NET_F_GUEST_TSO6))
+				SET(features, VIRTIO_NET_F_GUEST_TSO6);
+		}
+
+		vio_ctrl_guest_offloads(sc, features);
+	}
+}
+
 /*
  * Interface functions for ifnet
  */
@@ -960,7 +983,6 @@ int
 vio_init(struct ifnet *ifp)
 {
 	struct vio_softc *sc = ifp->if_softc;
-	struct virtio_softc *vsc = sc->sc_virtio;
 	int qidx;
 
 	vio_stop(ifp, 0);
@@ -977,22 +999,7 @@ vio_init(struct ifnet *ifp)
 	}
 	vio_iff(sc);
 	vio_link_state(ifp);
-
-	if (virtio_has_feature(vsc, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) {
-		uint64_t features = 0;
-
-		if (virtio_has_feature(vsc, VIRTIO_NET_F_GUEST_CSUM))
-			SET(features, VIRTIO_NET_F_GUEST_CSUM);
-
-		if (ISSET(ifp->if_xflags, IFXF_LRO)) {
-			if (virtio_has_feature(vsc, VIRTIO_NET_F_GUEST_TSO4))
-				SET(features, VIRTIO_NET_F_GUEST_TSO4);
-			if (virtio_has_feature(vsc, VIRTIO_NET_F_GUEST_TSO6))
-				SET(features, VIRTIO_NET_F_GUEST_TSO6);
-		}
-
-		vio_ctrl_guest_offloads(sc, features);
-	}
+	vio_set_offloads(ifp);
 
 	SET(ifp->if_flags, IFF_RUNNING);
 
@@ -1312,6 +1319,17 @@ vio_ioctl(struct ifnet *ifp, u_long cmd,
 		} else {
 			if (ifp->if_flags & IFF_RUNNING)
 				vio_stop(ifp, 1);
+		}
+		break;
+	case SIOCSIFXFLAGS:
+		if (ISSET(ifr->ifr_flags, IFXF_LRO) !=
+		    ISSET(ifp->if_xflags, IFXF_LRO)) {
+			if (ISSET(ifr->ifr_flags, IFXF_LRO))
+				SET(ifp->if_xflags, IFXF_LRO);
+			else
+				CLR(ifp->if_xflags, IFXF_LRO);
+
+			vio_set_offloads(ifp);
 		}
 		break;
 	case SIOCGIFMEDIA:
Index: net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
diff -u -p -r1.726 if.c
--- net/if.c	3 Feb 2025 08:58:52 -0000	1.726
+++ net/if.c	19 Feb 2025 20:25:42 -0000
@@ -3353,29 +3353,26 @@ ifpromisc(struct ifnet *ifp, int pswitch
 int
 ifsetlro(struct ifnet *ifp, int on)
 {
-	struct ifreq ifrq;
 	int error = 0;
 	int s = splnet();
-	struct if_parent parent;
+	struct ifreq ifr;
 
-	memset(&parent, 0, sizeof(parent));
-	if ((*ifp->if_ioctl)(ifp, SIOCGIFPARENT, (caddr_t)&parent) != -1) {
-		struct ifnet *ifp0 = if_unit(parent.ifp_parent);
-
-		if (ifp0 != NULL) {
-			ifsetlro(ifp0, on);
-			if_put(ifp0);
-		}
-	}
+	NET_ASSERT_LOCKED();	/* for ioctl */
+	KERNEL_ASSERT_LOCKED();	/* for if_flags */
+
+	memset(&ifr, 0, sizeof ifr);
+	if (on)
+		SET(ifr.ifr_flags, IFXF_LRO);
+
+	error = ((*ifp->if_ioctl)(ifp, SIOCSIFXFLAGS, (caddr_t)&ifr));
+	if (error == 0)
+		goto out;
 
 	if (!ISSET(ifp->if_capabilities, IFCAP_LRO)) {
 		error = ENOTSUP;
 		goto out;
 	}
 
-	NET_ASSERT_LOCKED();	/* for ioctl */
-	KERNEL_ASSERT_LOCKED();	/* for if_flags */
-
 	if (on && !ISSET(ifp->if_xflags, IFXF_LRO)) {
 		if (ifp->if_type == IFT_ETHER && ether_brport_isset(ifp)) {
 			error = EBUSY;
@@ -3384,21 +3381,7 @@ ifsetlro(struct ifnet *ifp, int on)
 		SET(ifp->if_xflags, IFXF_LRO);
 	} else if (!on && ISSET(ifp->if_xflags, IFXF_LRO))
 		CLR(ifp->if_xflags, IFXF_LRO);
-	else
-		goto out;
 
-	/* restart interface */
-	if (ISSET(ifp->if_flags, IFF_UP)) {
-		/* go down for a moment... */
-		CLR(ifp->if_flags, IFF_UP);
-		ifrq.ifr_flags = ifp->if_flags;
-		(*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t)&ifrq);
-
-		/* ... and up again */
-		SET(ifp->if_flags, IFF_UP);
-		ifrq.ifr_flags = ifp->if_flags;
-		(*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t)&ifrq);
-	}
  out:
 	splx(s);
 
Index: net/if_aggr.c
===================================================================
RCS file: /cvs/src/sys/net/if_aggr.c,v
diff -u -p -r1.47 if_aggr.c
--- net/if_aggr.c	18 Dec 2024 01:56:05 -0000	1.47
+++ net/if_aggr.c	19 Feb 2025 20:25:42 -0000
@@ -856,6 +856,7 @@ aggr_ioctl(struct ifnet *ifp, u_long cmd
 {
 	struct aggr_softc *sc = ifp->if_softc;
 	struct ifreq *ifr = (struct ifreq *)data;
+	struct aggr_port *p;
 	int error = 0;
 
 	if (sc->sc_dead)
@@ -875,6 +876,11 @@ aggr_ioctl(struct ifnet *ifp, u_long cmd
 			if (ISSET(ifp->if_flags, IFF_RUNNING))
 				error = aggr_down(sc);
 		}
+		break;
+
+	case SIOCSIFXFLAGS:
+		TAILQ_FOREACH(p, &sc->sc_ports, p_entry)
+			ifsetlro(p->p_ifp0, ISSET(ifr->ifr_flags, IFXF_LRO));
 		break;
 
 	case SIOCSIFLLADDR:
Index: net/if_bpe.c
===================================================================
RCS file: /cvs/src/sys/net/if_bpe.c,v
diff -u -p -r1.22 if_bpe.c
--- net/if_bpe.c	23 Dec 2023 10:52:54 -0000	1.22
+++ net/if_bpe.c	19 Feb 2025 20:25:42 -0000
@@ -305,6 +305,7 @@ bpe_ioctl(struct ifnet *ifp, u_long cmd,
 	struct bpe_softc *sc = ifp->if_softc;
 	struct ifreq *ifr = (struct ifreq *)data;
 	struct ifbrparam *bparam = (struct ifbrparam *)data;
+	struct ifnet *ifp0;
 	int error = 0;
 
 	switch (cmd) {
@@ -317,6 +318,13 @@ bpe_ioctl(struct ifnet *ifp, u_long cmd,
 		} else {
 			if (ISSET(ifp->if_flags, IFF_RUNNING))
 				error = bpe_down(sc);
+		}
+		break;
+
+	case SIOCSIFXFLAGS:
+		if ((ifp0 = if_get(sc->sc_key.k_if)) != NULL) {
+			ifsetlro(ifp0, ISSET(ifr->ifr_flags, IFXF_LRO));
+			if_put(ifp0);
 		}
 		break;
 
Index: net/if_trunk.c
===================================================================
RCS file: /cvs/src/sys/net/if_trunk.c,v
diff -u -p -r1.154 if_trunk.c
--- net/if_trunk.c	23 Dec 2023 10:52:54 -0000	1.154
+++ net/if_trunk.c	19 Feb 2025 20:25:42 -0000
@@ -846,6 +846,12 @@ trunk_ioctl(struct ifnet *ifp, u_long cm
 	case SIOCSIFFLAGS:
 		error = ENETRESET;
 		break;
+
+	case SIOCSIFXFLAGS:
+		SLIST_FOREACH(tp, &tr->tr_ports, tp_entries)
+			ifsetlro(tp->tp_if, ISSET(ifr->ifr_flags, IFXF_LRO));
+		break;
+
 	case SIOCADDMULTI:
 		error = trunk_ether_addmulti(tr, ifr);
 		break;
Index: net/if_vlan.c
===================================================================
RCS file: /cvs/src/sys/net/if_vlan.c,v
diff -u -p -r1.219 if_vlan.c
--- net/if_vlan.c	9 Jun 2024 16:25:28 -0000	1.219
+++ net/if_vlan.c	19 Feb 2025 20:25:42 -0000
@@ -701,6 +701,13 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd
 		}
 		break;
 
+	case SIOCSIFXFLAGS:
+		if ((ifp0 = if_get(sc->sc_ifidx0)) != NULL) {
+			ifsetlro(ifp0, ISSET(ifr->ifr_flags, IFXF_LRO));
+			if_put(ifp0);
+		}
+		break;
+
 	case SIOCSVNETID:
 		if (ifr->ifr_vnetid < EVL_VLID_MIN ||
 		    ifr->ifr_vnetid > EVL_VLID_MAX) {
Index: net/if_vxlan.c
===================================================================
RCS file: /cvs/src/sys/net/if_vxlan.c,v
diff -u -p -r1.100 if_vxlan.c
--- net/if_vxlan.c	31 Oct 2024 11:41:31 -0000	1.100
+++ net/if_vxlan.c	19 Feb 2025 20:25:42 -0000
@@ -729,6 +729,7 @@ vxlan_ioctl(struct ifnet *ifp, u_long cm
 	struct vxlan_softc *sc = ifp->if_softc;
 	struct ifreq *ifr = (struct ifreq *)data;
 	struct ifbrparam *bparam = (struct ifbrparam *)data;
+	struct ifnet *ifp0;
 	int error = 0;
 
 	switch (cmd) {
@@ -743,6 +744,13 @@ vxlan_ioctl(struct ifnet *ifp, u_long cm
 		} else {
 			if (ISSET(ifp->if_flags, IFF_RUNNING))
 				error = vxlan_down(sc);
+		}
+		break;
+
+	case SIOCSIFXFLAGS:
+		if ((ifp0 = if_get(sc->sc_if_index0)) != NULL) {
+			ifsetlro(ifp0, ISSET(ifr->ifr_flags, IFXF_LRO));
+			if_put(ifp0);
 		}
 		break;
 
Index: netinet/ip_carp.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
diff -u -p -r1.365 ip_carp.c
--- netinet/ip_carp.c	19 Dec 2024 22:10:35 -0000	1.365
+++ netinet/ip_carp.c	19 Feb 2025 20:25:42 -0000
@@ -2045,6 +2045,13 @@ carp_ioctl(struct ifnet *ifp, u_long cmd
 		}
 		break;
 
+	case SIOCSIFXFLAGS:
+		if ((ifp0 = if_get(sc->sc_carpdevidx)) != NULL) {
+			ifsetlro(ifp0, ISSET(ifr->ifr_flags, IFXF_LRO));
+			if_put(ifp0);
+		}
+		break;
+
 	case SIOCSVH:
 		KERNEL_ASSERT_LOCKED(); /* touching carp_vhosts */
 		vhe = SRPL_FIRST_LOCKED(&sc->carp_vhosts);