Re: seperate LRO/TSO flags
This diff passed a make release. I think it should be commited
now, so that we can proceed with TSO in the driver layer.
bluhm
On Mon, May 15, 2023 at 11:16:59PM +0200, Jan Klemkow wrote:
> Index: sbin/ifconfig/ifconfig.8
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.394
> diff -u -p -r1.394 ifconfig.8
> --- sbin/ifconfig/ifconfig.8 26 Apr 2023 02:38:08 - 1.394
> +++ sbin/ifconfig/ifconfig.8 15 May 2023 18:46:48 -
> @@ -282,8 +282,18 @@ tag.
> As CSUM_TCPv4, but supports IPv6 datagrams.
> .It Sy CSUM_UDPv6
> As above, for UDP.
> -.It Sy TSO
> -The device supports TCP segment offloading (TSO).
> +.It Sy LRO
> +The device supports TCP large receive offload (LRO).
> +.It Sy TSOv4
> +The device supports IPv4 TCP segmentation offload (TSO).
> +TSO is used by default.
> +Use the
> +.Xr sysctl 8
> +variable
> +.Va net.inet.tcp.tso
> +to disable this feature.
> +.It Sy TSOv6
> +As above, for IPv6.
> .It Sy WOL
> The device supports Wake on LAN (WoL).
> .It Sy hardmtu
> @@ -491,25 +501,25 @@ Query and display information and diagno
> modules installed in an interface.
> It is only supported by drivers implementing the necessary functionality
> on hardware which supports it.
> -.It Cm tso
> -Enable TCP segmentation offloading (TSO) if it's supported by the hardware;
> see
> +.It Cm tcprecvoffload
> +Enable TCP large receive offload (LRO) if it's supported by the hardware; see
> .Cm hwfeatures .
> -TSO enabled NICs modify received TCP/IP packets.
> +LRO enabled network interfaces modify received TCP/IP packets.
> This will also affect traffic of upper layer interfaces,
> such as
> .Xr vlan 4 ,
> .Xr aggr 4 ,
> and
> .Xr carp 4 .
> -It is not possible to use TSO with interfaces attached to a
> +It is not possible to use LRO with interfaces attached to a
> .Xr bridge 4 ,
> .Xr veb 4 ,
> or
> .Xr tpmr 4 .
> Changing this option will re-initialize the network interface.
> -.It Cm -tso
> -Disable TSO.
> -TSO is disabled by default.
> +.It Cm -tcprecvoffload
> +Disable LRO.
> +LRO is disabled by default.
> .It Cm up
> Mark an interface
> .Dq up .
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.463
> diff -u -p -r1.463 ifconfig.c
> --- sbin/ifconfig/ifconfig.c 12 May 2023 18:24:13 - 1.463
> +++ sbin/ifconfig/ifconfig.c 15 May 2023 20:27:51 -
> @@ -126,7 +126,7 @@
> #define HWFEATURESBITS
> \
> "\024\1CSUM_IPv4\2CSUM_TCPv4\3CSUM_UDPv4" \
> "\5VLAN_MTU\6VLAN_HWTAGGING\10CSUM_TCPv6" \
> - "\11CSUM_UDPv6\17TSO\20WOL"
> + "\11CSUM_UDPv6\15TSOv4\16TSOv6\17LSO\20WOL"
>
> struct ifencap {
> unsigned int ife_flags;
> @@ -469,8 +469,8 @@ const struct cmd {
> { "-soii", IFXF_INET6_NOSOII, 0, setifxflags },
> { "monitor",IFXF_MONITOR, 0, setifxflags },
> { "-monitor", -IFXF_MONITOR, 0, setifxflags },
> - { "tso",IFXF_TSO, 0, setifxflags },
> - { "-tso", -IFXF_TSO, 0, setifxflags },
> + { "tcprecvoffload", IFXF_LRO, 0, setifxflags },
> + { "-tcprecvoffload", -IFXF_LRO, 0, setifxflags },
> #ifndef SMALL
> { "hwfeatures", NEXTARG0, 0, printifhwfeatures },
> { "metric", NEXTARG,0, setifmetric },
> @@ -674,7 +674,7 @@ const struct cmd {
> "\7RUNNING\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX"\
> "\15LINK0\16LINK1\17LINK2\20MULTICAST" \
> "\23AUTOCONF6TEMP\24MPLS\25WOL\26AUTOCONF6\27INET6_NOSOII" \
> - "\30AUTOCONF4" "\31MONITOR" "\32TSO"
> + "\30AUTOCONF4" "\31MONITOR" "\32LRO"
>
> int getinfo(struct ifreq *, int);
> void getsock(int);
> Index: sys/dev/pci/if_ix.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
> retrieving revision 1.193
> diff -u -p -r1.193 if_ix.c
> --- sys/dev/pci/if_ix.c 28 Apr 2023 10:18:57 - 1.193
> +++ sys/dev/pci/if_ix.c 15 May 2023 21:09:13 -
> @@ -1925,7 +1925,7 @@ ixgbe_setup_interface(struct ix_softc *s
> ifp->if_capabilities |= IFCAP_CSUM_IPv4;
>
> if (sc->hw.mac.type != ixgbe_mac_82598EB)
> - ifp->if_capabilities |= IFCAP_TSO;
> + ifp->if_capabilities |= IFCAP_LRO;
>
> /*
>* Specify the media types supported by this sc and register
> @@ -2873,13 +2873,13 @@ ixgbe_initialize_receive_units(struct ix
> hlreg |= IXGBE_HLREG0_JUMBOEN;
> IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg);
>
> - if (ISSET(ifp->if_xflags, IFXF_TSO)) {
> + if
Re: seperate LRO/TSO flags
On Mon, May 15, 2023 at 11:16:59PM +0200, Jan Klemkow wrote: > - updated the diff to the current source state > - improved the vlan(4) capability handling > > ok? OK bluhm@ > - "\11CSUM_UDPv6\17TSO\20WOL" > + "\11CSUM_UDPv6\15TSOv4\16TSOv6\17LSO\20WOL" typo s/LSO/LRO/
Re: seperate LRO/TSO flags
On Mon, May 15, 2023 at 11:40:20AM +0200, Alexander Bluhm wrote:
> On Mon, May 15, 2023 at 09:34:21AM +0200, Jan Klemkow wrote:
> > @@ -251,12 +251,16 @@ struct if_status_description {
> > #defineIFCAP_VLAN_HWTAGGING0x0020 /* hardware VLAN tag
> > support */
> > #defineIFCAP_CSUM_TCPv60x0080 /* can do IPv6/TCP
> > checksums */
> > #defineIFCAP_CSUM_UDPv60x0100 /* can do IPv6/UDP
> > checksums */
> > -#defineIFCAP_TSO 0x4000 /* TCP segment
> > offloading */
> > +#defineIFCAP_LRO 0x1000 /* TCP large recv
> > offload */
> > +#defineIFCAP_TSOv4 0x2000 /* TCP segmentation
> > offload */
> > +#defineIFCAP_TSOv6 0x4000 /* TCP segmentation
> > offload */
> > #defineIFCAP_WOL 0x8000 /* can do wake on lan */
>
> I would prefer to keep the numbers of IFCAP_TSO/IFCAP_LRO as this
> is just a naming error. Then we have less confusion during the
> ifconfig transition phase.
>
> +#define IFCAP_TSOv4 0x1000
> +#define IFCAP_TSOv6 0x2000
> -#define IFCAP_TSO0x4000
> +#define IFCAP_LRO0x4000
>
> > +#define IFCAP_TSO (IFCAP_TSOv4 | IFCAP_TSOv6)
> > +
>
> Could you please remove this chunk and expand it, where is used?
> This one more define does not make the code clearer. And this flag
> IFCAP_TSO had a different meaning before renaming. When it is not
> introduced again, the compiler makes sure that no renaming was
> forgotten.
done
Also:
- updated the diff to the current source state
- improved the vlan(4) capability handling
@dlg: Whats your opinion about this diff?
ok?
Thanks,
Jan
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.394
diff -u -p -r1.394 ifconfig.8
--- sbin/ifconfig/ifconfig.826 Apr 2023 02:38:08 - 1.394
+++ sbin/ifconfig/ifconfig.815 May 2023 18:46:48 -
@@ -282,8 +282,18 @@ tag.
As CSUM_TCPv4, but supports IPv6 datagrams.
.It Sy CSUM_UDPv6
As above, for UDP.
-.It Sy TSO
-The device supports TCP segment offloading (TSO).
+.It Sy LRO
+The device supports TCP large receive offload (LRO).
+.It Sy TSOv4
+The device supports IPv4 TCP segmentation offload (TSO).
+TSO is used by default.
+Use the
+.Xr sysctl 8
+variable
+.Va net.inet.tcp.tso
+to disable this feature.
+.It Sy TSOv6
+As above, for IPv6.
.It Sy WOL
The device supports Wake on LAN (WoL).
.It Sy hardmtu
@@ -491,25 +501,25 @@ Query and display information and diagno
modules installed in an interface.
It is only supported by drivers implementing the necessary functionality
on hardware which supports it.
-.It Cm tso
-Enable TCP segmentation offloading (TSO) if it's supported by the hardware; see
+.It Cm tcprecvoffload
+Enable TCP large receive offload (LRO) if it's supported by the hardware; see
.Cm hwfeatures .
-TSO enabled NICs modify received TCP/IP packets.
+LRO enabled network interfaces modify received TCP/IP packets.
This will also affect traffic of upper layer interfaces,
such as
.Xr vlan 4 ,
.Xr aggr 4 ,
and
.Xr carp 4 .
-It is not possible to use TSO with interfaces attached to a
+It is not possible to use LRO with interfaces attached to a
.Xr bridge 4 ,
.Xr veb 4 ,
or
.Xr tpmr 4 .
Changing this option will re-initialize the network interface.
-.It Cm -tso
-Disable TSO.
-TSO is disabled by default.
+.It Cm -tcprecvoffload
+Disable LRO.
+LRO is disabled by default.
.It Cm up
Mark an interface
.Dq up .
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.463
diff -u -p -r1.463 ifconfig.c
--- sbin/ifconfig/ifconfig.c12 May 2023 18:24:13 - 1.463
+++ sbin/ifconfig/ifconfig.c15 May 2023 20:27:51 -
@@ -126,7 +126,7 @@
#define HWFEATURESBITS \
"\024\1CSUM_IPv4\2CSUM_TCPv4\3CSUM_UDPv4" \
"\5VLAN_MTU\6VLAN_HWTAGGING\10CSUM_TCPv6" \
- "\11CSUM_UDPv6\17TSO\20WOL"
+ "\11CSUM_UDPv6\15TSOv4\16TSOv6\17LSO\20WOL"
struct ifencap {
unsigned int ife_flags;
@@ -469,8 +469,8 @@ const structcmd {
{ "-soii", IFXF_INET6_NOSOII, 0, setifxflags },
{ "monitor",IFXF_MONITOR, 0, setifxflags },
{ "-monitor", -IFXF_MONITOR, 0, setifxflags },
- { "tso",IFXF_TSO, 0, setifxflags },
- { "-tso", -IFXF_TSO, 0, setifxflags },
+ { "tcprecvoffload", IFXF_LRO, 0, setifxflags },
+ { "-tcprecvoffload", -IFXF_LRO, 0, setifxflags },
#ifndef SMALL
{ "hwfeatures", NEXTARG0, 0, printifhwfeatures },
{ "metric",
Re: seperate LRO/TSO flags
On Mon, May 15, 2023 at 09:34:21AM +0200, Jan Klemkow wrote:
> @@ -251,12 +251,16 @@ struct if_status_description {
> #define IFCAP_VLAN_HWTAGGING0x0020 /* hardware VLAN tag
> support */
> #define IFCAP_CSUM_TCPv60x0080 /* can do IPv6/TCP
> checksums */
> #define IFCAP_CSUM_UDPv60x0100 /* can do IPv6/UDP
> checksums */
> -#define IFCAP_TSO 0x4000 /* TCP segment
> offloading */
> +#define IFCAP_LRO 0x1000 /* TCP large recv
> offload */
> +#define IFCAP_TSOv4 0x2000 /* TCP segmentation
> offload */
> +#define IFCAP_TSOv6 0x4000 /* TCP segmentation
> offload */
> #define IFCAP_WOL 0x8000 /* can do wake on lan */
I would prefer to keep the numbers of IFCAP_TSO/IFCAP_LRO as this
is just a naming error. Then we have less confusion during the
ifconfig transition phase.
+#define IFCAP_TSOv40x1000
+#define IFCAP_TSOv60x2000
-#define IFCAP_TSO 0x4000
+#define IFCAP_LRO 0x4000
> +#define IFCAP_TSO(IFCAP_TSOv4 | IFCAP_TSOv6)
> +
Could you please remove this chunk and expand it, where is used?
This one more define does not make the code clearer. And this flag
IFCAP_TSO had a different meaning before renaming. When it is not
introduced again, the compiler makes sure that no renaming was
forgotten.
bluhm
Re: seperate LRO/TSO flags
On Sat, May 13, 2023 at 04:44:18PM +0200, Christian Weisgerber wrote:
> Jan Klemkow:
>
> > This diff introduces separate flags for TCP offloading. We split this
> > into LRO (large receive offloading) and TSO (TCP segmentation
> > offloading). Thus, we are able to turn it on/off separately.
>
> Wait, why do we even have a knob for TSO?
>
> We specifically decided not to have a knob for checksum offloading,
> because it should just work out of the box, and if it doesn't, then
> it should be disabled by the driver. It should not be the admin's
> task to figure out if the implementation is broken and to fiddle
> with the knobs (hi, FreeBSD!).
>
> I would assume that line of thinking extends to TSO.
You are right. This is reflected in the current state of the diff
below.
We just need a knob for TCP Large Receive Offload (LRO) because it
changes the TCP segments. You may want to avoid this on a forwarding
router.
ok?
Thanks,
Jan
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.394
diff -u -p -r1.394 ifconfig.8
--- sbin/ifconfig/ifconfig.826 Apr 2023 02:38:08 - 1.394
+++ sbin/ifconfig/ifconfig.812 May 2023 06:22:35 -
@@ -282,8 +282,18 @@ tag.
As CSUM_TCPv4, but supports IPv6 datagrams.
.It Sy CSUM_UDPv6
As above, for UDP.
-.It Sy TSO
-The device supports TCP segment offloading (TSO).
+.It Sy LRO
+The device supports TCP large receive offload (LRO).
+.It Sy TSOv4
+The device supports IPv4 TCP segmentation offload (TSO).
+TSO is used by default.
+Use the
+.Xr sysctl 8
+variable
+.Va net.inet.tcp.tso
+to disable this feature.
+.It Sy TSOv6
+As above, for IPv6.
.It Sy WOL
The device supports Wake on LAN (WoL).
.It Sy hardmtu
@@ -491,25 +501,25 @@ Query and display information and diagno
modules installed in an interface.
It is only supported by drivers implementing the necessary functionality
on hardware which supports it.
-.It Cm tso
-Enable TCP segmentation offloading (TSO) if it's supported by the hardware; see
+.It Cm tcprecvoffload
+Enable TCP large receive offload (LRO) if it's supported by the hardware; see
.Cm hwfeatures .
-TSO enabled NICs modify received TCP/IP packets.
+LRO enabled network interfaces modify received TCP/IP packets.
This will also affect traffic of upper layer interfaces,
such as
.Xr vlan 4 ,
.Xr aggr 4 ,
and
.Xr carp 4 .
-It is not possible to use TSO with interfaces attached to a
+It is not possible to use LRO with interfaces attached to a
.Xr bridge 4 ,
.Xr veb 4 ,
or
.Xr tpmr 4 .
Changing this option will re-initialize the network interface.
-.It Cm -tso
-Disable TSO.
-TSO is disabled by default.
+.It Cm -tcprecvoffload
+Disable LRO.
+LRO is disabled by default.
.It Cm up
Mark an interface
.Dq up .
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.462
diff -u -p -r1.462 ifconfig.c
--- sbin/ifconfig/ifconfig.c8 Mar 2023 04:43:06 - 1.462
+++ sbin/ifconfig/ifconfig.c11 May 2023 17:33:55 -
@@ -126,7 +126,7 @@
#define HWFEATURESBITS \
"\024\1CSUM_IPv4\2CSUM_TCPv4\3CSUM_UDPv4" \
"\5VLAN_MTU\6VLAN_HWTAGGING\10CSUM_TCPv6" \
- "\11CSUM_UDPv6\17TSO\20WOL"
+ "\11CSUM_UDPv6\15LRO\16TSOv4\17TSOv6\20WOL"
struct ifencap {
unsigned int ife_flags;
@@ -469,8 +469,8 @@ const structcmd {
{ "-soii", IFXF_INET6_NOSOII, 0, setifxflags },
{ "monitor",IFXF_MONITOR, 0, setifxflags },
{ "-monitor", -IFXF_MONITOR, 0, setifxflags },
- { "tso",IFXF_TSO, 0, setifxflags },
- { "-tso", -IFXF_TSO, 0, setifxflags },
+ { "tcprecvoffload", IFXF_LRO, 0, setifxflags },
+ { "-tcprecvoffload", -IFXF_LRO, 0, setifxflags },
#ifndef SMALL
{ "hwfeatures", NEXTARG0, 0, printifhwfeatures },
{ "metric", NEXTARG,0, setifmetric },
@@ -674,7 +674,7 @@ const structcmd {
"\7RUNNING\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX"\
"\15LINK0\16LINK1\17LINK2\20MULTICAST" \
"\23AUTOCONF6TEMP\24MPLS\25WOL\26AUTOCONF6\27INET6_NOSOII" \
- "\30AUTOCONF4" "\31MONITOR" "\32TSO"
+ "\30AUTOCONF4" "\31MONITOR" "\32LRO"
intgetinfo(struct ifreq *, int);
void getsock(int);
Index: sys/dev/pci/if_ix.c
===
RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
retrieving revision 1.193
diff -u -p -r1.193 if_ix.c
--- sys/dev/pci/if_ix.c 28 Apr 2023 10:18:57 - 1.193
+++ sys/dev/pci/if_ix.c 12 May 2023 06:37:44 -
@@
Re: seperate LRO/TSO flags
Jan Klemkow: > This diff introduces separate flags for TCP offloading. We split this > into LRO (large receive offloading) and TSO (TCP segmentation > offloading). Thus, we are able to turn it on/off separately. Wait, why do we even have a knob for TSO? We specifically decided not to have a knob for checksum offloading, because it should just work out of the box, and if it doesn't, then it should be disabled by the driver. It should not be the admin's task to figure out if the implementation is broken and to fiddle with the knobs (hi, FreeBSD!). I would assume that line of thinking extends to TSO. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: seperate LRO/TSO flags
On 5/11/23 03:58, Todd C. Miller wrote: On Wed, 10 May 2023 20:55:24 +0200, Jan Klemkow wrote: For tcprecvoffload and ix(4) it's not possible to enable/disable it per address family. Its just one flag for the hardware. For tcpsendoffload its possible, but I won't do that till its necessary. Why would you want to differentiate the address families here? I was mostly just curious as I see FreeBSD seems to support this. That made we wonder if there is hardware that only supports offloading for IPv4. I know that em driver only supports TSO4 on FreeBSD. It's not enabled by default but users can enable it. See the following code for detail. https://cgit.freebsd.org/src/tree/sys/dev/e1000/if_em.c#n900 -- Yuichiro NAITO (naito.yuich...@gmail.com)
Re: seperate LRO/TSO flags
On Wed, 10 May 2023 20:55:24 +0200, Jan Klemkow wrote: > For tcprecvoffload and ix(4) it's not possible to enable/disable it per > address family. Its just one flag for the hardware. > > For tcpsendoffload its possible, but I won't do that till its necessary. > > Why would you want to differentiate the address families here? I was mostly just curious as I see FreeBSD seems to support this. That made we wonder if there is hardware that only supports offloading for IPv4. - todd
Re: seperate LRO/TSO flags
On Wed, May 10, 2023 at 11:13:04AM -0600, Todd C. Miller wrote: > On Wed, 10 May 2023 19:03:58 +0200, Jan Klemkow wrote: > > This diff introduces separate flags for TCP offloading. We split this > > into LRO (large receive offloading) and TSO (TCP segmentation > > offloading). Thus, we are able to turn it on/off separately. > > > > For ifconfig(8) we use "tcprecvoffload" and "tcpsendoffload". So, the > > user has a better insight of what this features are doing. > > Is it possible to control these at the address family level? In > other words, is it possible to enable "tcprecvoffload" and > "tcpsendoffload" for inet but not inet6 or vice versa? For tcprecvoffload and ix(4) it's not possible to enable/disable it per address family. Its just one flag for the hardware. For tcpsendoffload its possible, but I won't do that till its necessary. Why would you want to differentiate the address families here? bye, Jan
Re: seperate LRO/TSO flags
On Wed, 10 May 2023 19:03:58 +0200, Jan Klemkow wrote: > This diff introduces separate flags for TCP offloading. We split this > into LRO (large receive offloading) and TSO (TCP segmentation > offloading). Thus, we are able to turn it on/off separately. > > For ifconfig(8) we use "tcprecvoffload" and "tcpsendoffload". So, the > user has a better insight of what this features are doing. Is it possible to control these at the address family level? In other words, is it possible to enable "tcprecvoffload" and "tcpsendoffload" for inet but not inet6 or vice versa? - todd
