From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: refactor LRO turn off
To: Jan Klemkow <jan@openbsd.org>
Cc: tech@openbsd.org
Date: Fri, 21 Feb 2025 23:23:10 +0100

On Wed, Feb 19, 2025 at 09:46:36PM +0100, Jan Klemkow wrote:
> 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.

Tested and OK bluhm@

> 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);