Re: tcp lro by default, call for testing

2023-07-17 Thread Alexander Bluhm
On Mon, Jul 10, 2023 at 02:07:01PM +0200, Jan Klemkow wrote:
> On Sat, Jul 08, 2023 at 05:15:26PM +0300, Alexander Bluhm wrote:
> > I am not aware of any more limitations when enabling LRO for TCP
> > in the network drivers.  The feature allows to receive agregated
> > packets larger than the MTU.  Receiving TCP streams becomes much
> > faster.
> > 
> > As the network hardware is not aware whether a packet is received
> > locally or forwarded, everything is aggregated.  In case of forwarding
> > it is split on output to packets not larger than the original
> > packets.  So path MTU discovery should still work.  If the outgoing
> > interface supports TSO, the packet is chopped in hardware.
> > 
> > Currently only ix(4) and lo(4) support LRO, and ix(4) is limited
> > to IPv4 and newer than the old 82598 model.  If the interface is
> > added to a bridge(4) or aggr(4), LRO is automatically disabled.
> 
> I guess you mean veb(4) not aggr(4).  We just avoid the in heritage
> of the LRO capability in aggr(4) but are using the feature.
> 
> > So in case you possess any ix(4) hardware or do funky pf routing
> > on lo(4) please run this diff.  If you encounter problems, report
> > and turn LRO off per interface with ifconfig -tcplro.
> 
> Diff looks fine to me.  I just would keep mentioning the default
> behavior in the manpage like this:

I think in future default will depend on the driver.  When adding
the LRO feature for a hardware we start with default off.  When we
are confident, we turn it on.  We are more flexible if we do not
mention it in ifconfig(8) man page.

If there are some problems with specific hardware they belong to
the driver in section 4.

> ok jan@

No test reports arrvied me.  So either everything works fine or
nobody has tested it.  I will commit soon to find out.

bluhm

> Index: sbin/ifconfig/ifconfig.8
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.397
> diff -u -p -r1.397 ifconfig.8
> --- sbin/ifconfig/ifconfig.8  7 Jun 2023 18:42:40 -   1.397
> +++ sbin/ifconfig/ifconfig.8  10 Jul 2023 11:54:47 -
> @@ -517,9 +517,9 @@ It is not possible to use LRO with inter
>  or
>  .Xr tpmr 4 .
>  Changing this option will re-initialize the network interface.
> +LRO is enabled by default.
>  .It Cm -tcplro
>  Disable LRO.
> -LRO is disabled by default.
>  .It Cm up
>  Mark an interface
>  .Dq up .
> 
> 
> > Index: sys/dev/pci/if_ix.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_ix.c,v
> > retrieving revision 1.198
> > diff -u -p -r1.198 if_ix.c
> > --- sys/dev/pci/if_ix.c 8 Jul 2023 09:01:30 -   1.198
> > +++ sys/dev/pci/if_ix.c 8 Jul 2023 13:51:26 -
> > @@ -1925,8 +1925,10 @@ ixgbe_setup_interface(struct ix_softc *s
> > ifp->if_capabilities |= IFCAP_CSUM_IPv4;
> >  
> > ifp->if_capabilities |= IFCAP_TSOv4 | IFCAP_TSOv6;
> > -   if (sc->hw.mac.type != ixgbe_mac_82598EB)
> > +   if (sc->hw.mac.type != ixgbe_mac_82598EB) {
> > +   ifp->if_xflags |= IFXF_LRO;
> > ifp->if_capabilities |= IFCAP_LRO;
> > +   }
> >  
> > /*
> >  * Specify the media types supported by this sc and register
> > Index: sys/net/if_loop.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_loop.c,v
> > retrieving revision 1.95
> > diff -u -p -r1.95 if_loop.c
> > --- sys/net/if_loop.c   2 Jul 2023 19:59:15 -   1.95
> > +++ sys/net/if_loop.c   8 Jul 2023 13:51:26 -
> > @@ -172,11 +172,11 @@ loop_clone_create(struct if_clone *ifc, 
> > ifp->if_softc = NULL;
> > ifp->if_mtu = LOMTU;
> > ifp->if_flags = IFF_LOOPBACK | IFF_MULTICAST;
> > -   ifp->if_xflags = IFXF_CLONED;
> > +   ifp->if_xflags = IFXF_CLONED | IFXF_LRO;
> > ifp->if_capabilities = IFCAP_CSUM_IPv4 |
> > IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4 |
> > IFCAP_CSUM_TCPv6 | IFCAP_CSUM_UDPv6 |
> > -   IFCAP_LRO;
> > +   IFCAP_LRO | IFCAP_TSOv4 | IFCAP_TSOv6;
> > ifp->if_rtrequest = lortrequest;
> > ifp->if_ioctl = loioctl;
> > ifp->if_input = loinput;
> > Index: sbin/ifconfig/ifconfig.8
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.8,v
> > retrieving revision 1.397
> > diff -u -p -r1.397 ifconfig.8
> > --- sbin/ifconfig/ifconfig.87 Jun 2023 18:42:40 -   1.397
> > +++ sbin/ifconfig/ifconfig.87 Jul 2023 19:57:09 -
> > @@ -519,7 +519,6 @@ or
> >  Changing this option will re-initialize the network interface.
> >  .It Cm -tcplro
> >  Disable LRO.
> > -LRO is disabled by default.
> >  .It Cm up
> >  Mark an interface
> >  .Dq up .
> > 



Re: tcp lro by default, call for testing

2023-07-10 Thread Jan Klemkow
On Sat, Jul 08, 2023 at 05:15:26PM +0300, Alexander Bluhm wrote:
> I am not aware of any more limitations when enabling LRO for TCP
> in the network drivers.  The feature allows to receive agregated
> packets larger than the MTU.  Receiving TCP streams becomes much
> faster.
> 
> As the network hardware is not aware whether a packet is received
> locally or forwarded, everything is aggregated.  In case of forwarding
> it is split on output to packets not larger than the original
> packets.  So path MTU discovery should still work.  If the outgoing
> interface supports TSO, the packet is chopped in hardware.
> 
> Currently only ix(4) and lo(4) support LRO, and ix(4) is limited
> to IPv4 and newer than the old 82598 model.  If the interface is
> added to a bridge(4) or aggr(4), LRO is automatically disabled.

I guess you mean veb(4) not aggr(4).  We just avoid the in heritage
of the LRO capability in aggr(4) but are using the feature.

> So in case you possess any ix(4) hardware or do funky pf routing
> on lo(4) please run this diff.  If you encounter problems, report
> and turn LRO off per interface with ifconfig -tcplro.

Diff looks fine to me.  I just would keep mentioning the default
behavior in the manpage like this:

ok jan@

Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.397
diff -u -p -r1.397 ifconfig.8
--- sbin/ifconfig/ifconfig.87 Jun 2023 18:42:40 -   1.397
+++ sbin/ifconfig/ifconfig.810 Jul 2023 11:54:47 -
@@ -517,9 +517,9 @@ It is not possible to use LRO with inter
 or
 .Xr tpmr 4 .
 Changing this option will re-initialize the network interface.
+LRO is enabled by default.
 .It Cm -tcplro
 Disable LRO.
-LRO is disabled by default.
 .It Cm up
 Mark an interface
 .Dq up .


> Index: sys/dev/pci/if_ix.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_ix.c,v
> retrieving revision 1.198
> diff -u -p -r1.198 if_ix.c
> --- sys/dev/pci/if_ix.c   8 Jul 2023 09:01:30 -   1.198
> +++ sys/dev/pci/if_ix.c   8 Jul 2023 13:51:26 -
> @@ -1925,8 +1925,10 @@ ixgbe_setup_interface(struct ix_softc *s
>   ifp->if_capabilities |= IFCAP_CSUM_IPv4;
>  
>   ifp->if_capabilities |= IFCAP_TSOv4 | IFCAP_TSOv6;
> - if (sc->hw.mac.type != ixgbe_mac_82598EB)
> + if (sc->hw.mac.type != ixgbe_mac_82598EB) {
> + ifp->if_xflags |= IFXF_LRO;
>   ifp->if_capabilities |= IFCAP_LRO;
> + }
>  
>   /*
>* Specify the media types supported by this sc and register
> Index: sys/net/if_loop.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_loop.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 if_loop.c
> --- sys/net/if_loop.c 2 Jul 2023 19:59:15 -   1.95
> +++ sys/net/if_loop.c 8 Jul 2023 13:51:26 -
> @@ -172,11 +172,11 @@ loop_clone_create(struct if_clone *ifc, 
>   ifp->if_softc = NULL;
>   ifp->if_mtu = LOMTU;
>   ifp->if_flags = IFF_LOOPBACK | IFF_MULTICAST;
> - ifp->if_xflags = IFXF_CLONED;
> + ifp->if_xflags = IFXF_CLONED | IFXF_LRO;
>   ifp->if_capabilities = IFCAP_CSUM_IPv4 |
>   IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4 |
>   IFCAP_CSUM_TCPv6 | IFCAP_CSUM_UDPv6 |
> - IFCAP_LRO;
> + IFCAP_LRO | IFCAP_TSOv4 | IFCAP_TSOv6;
>   ifp->if_rtrequest = lortrequest;
>   ifp->if_ioctl = loioctl;
>   ifp->if_input = loinput;
> Index: sbin/ifconfig/ifconfig.8
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.397
> diff -u -p -r1.397 ifconfig.8
> --- sbin/ifconfig/ifconfig.8  7 Jun 2023 18:42:40 -   1.397
> +++ sbin/ifconfig/ifconfig.8  7 Jul 2023 19:57:09 -
> @@ -519,7 +519,6 @@ or
>  Changing this option will re-initialize the network interface.
>  .It Cm -tcplro
>  Disable LRO.
> -LRO is disabled by default.
>  .It Cm up
>  Mark an interface
>  .Dq up .
> 



tcp lro by default, call for testing

2023-07-08 Thread Alexander Bluhm
Hi,

I am not aware of any more limitations when enabling LRO for TCP
in the network drivers.  The feature allows to receive agregated
packets larger than the MTU.  Receiving TCP streams becomes much
faster.

As the network hardware is not aware whether a packet is received
locally or forwarded, everything is aggregated.  In case of forwarding
it is split on output to packets not larger than the original
packets.  So path MTU discovery should still work.  If the outgoing
interface supports TSO, the packet is chopped in hardware.

Currently only ix(4) and lo(4) support LRO, and ix(4) is limited
to IPv4 and newer than the old 82598 model.  If the interface is
added to a bridge(4) or aggr(4), LRO is automatically disabled.

So in case you possess any ix(4) hardware or do funky pf routing
on lo(4) please run this diff.  If you encounter problems, report
and turn LRO off per interface with ifconfig -tcplro.

bluhm

Index: sys/dev/pci/if_ix.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_ix.c,v
retrieving revision 1.198
diff -u -p -r1.198 if_ix.c
--- sys/dev/pci/if_ix.c 8 Jul 2023 09:01:30 -   1.198
+++ sys/dev/pci/if_ix.c 8 Jul 2023 13:51:26 -
@@ -1925,8 +1925,10 @@ ixgbe_setup_interface(struct ix_softc *s
ifp->if_capabilities |= IFCAP_CSUM_IPv4;
 
ifp->if_capabilities |= IFCAP_TSOv4 | IFCAP_TSOv6;
-   if (sc->hw.mac.type != ixgbe_mac_82598EB)
+   if (sc->hw.mac.type != ixgbe_mac_82598EB) {
+   ifp->if_xflags |= IFXF_LRO;
ifp->if_capabilities |= IFCAP_LRO;
+   }
 
/*
 * Specify the media types supported by this sc and register
Index: sys/net/if_loop.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_loop.c,v
retrieving revision 1.95
diff -u -p -r1.95 if_loop.c
--- sys/net/if_loop.c   2 Jul 2023 19:59:15 -   1.95
+++ sys/net/if_loop.c   8 Jul 2023 13:51:26 -
@@ -172,11 +172,11 @@ loop_clone_create(struct if_clone *ifc, 
ifp->if_softc = NULL;
ifp->if_mtu = LOMTU;
ifp->if_flags = IFF_LOOPBACK | IFF_MULTICAST;
-   ifp->if_xflags = IFXF_CLONED;
+   ifp->if_xflags = IFXF_CLONED | IFXF_LRO;
ifp->if_capabilities = IFCAP_CSUM_IPv4 |
IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4 |
IFCAP_CSUM_TCPv6 | IFCAP_CSUM_UDPv6 |
-   IFCAP_LRO;
+   IFCAP_LRO | IFCAP_TSOv4 | IFCAP_TSOv6;
ifp->if_rtrequest = lortrequest;
ifp->if_ioctl = loioctl;
ifp->if_input = loinput;
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.397
diff -u -p -r1.397 ifconfig.8
--- sbin/ifconfig/ifconfig.87 Jun 2023 18:42:40 -   1.397
+++ sbin/ifconfig/ifconfig.87 Jul 2023 19:57:09 -
@@ -519,7 +519,6 @@ or
 Changing this option will re-initialize the network interface.
 .It Cm -tcplro
 Disable LRO.
-LRO is disabled by default.
 .It Cm up
 Mark an interface
 .Dq up .