[Ecn-sane] Fwd: [RFC PATCH 09/28] gso: AccECN support

[Dave Taht]

---------- Forwarded message ---------
From: Ilpo Järvinen <ilpo.jarvinen at cs.helsinki.fi>
Date: Thu, Mar 19, 2020 at 3:49 PM
Subject: Re: [RFC PATCH 09/28] gso: AccECN support
To: Eric Dumazet <eric.dumazet at gmail.com>
Cc: Netdev <netdev at vger.kernel.org>, Yuchung Cheng
<ycheng at google.com>, Neal Cardwell <ncardwell at google.com>, Olivier
Tilmans <olivier.tilmans at nokia-bell-labs.com>


On Wed, 18 Mar 2020, Eric Dumazet wrote:
> On 3/18/20 2:43 AM, Ilpo Järvinen wrote:
> > From: Ilpo Järvinen <ilpo.jarvinen at cs.helsinki.fi>
> >
> > Handling the CWR flag differs between RFC 3168 ECN and AccECN.
> > Take it into account in GSO by not clearing the CWR bit.
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen at cs.helsinki.fi>
> >
>
> Does it means TCP segmentation offload is disabled on all the NIC
> around ?

On general level, yes. HW TSO should be disabled with AccECN (or when CWR
flag is set for an incoming packet). The reason is how CWR flag is handled
by RFC 3168 ECN-aware TSO. It splits the segment such that CWR is cleared
starting from the 2nd segment which is incompatible how AccECN handles the
CWR flag. With AccECN, CWR flag (or more accurately, the ACE field that
also includes ECE & AE flags) changes only when new packet(s) with CE mark
arrives so the flag should not be changed within a super-skb. The new
feature flag is necessary to prevent such TSO engines happily clearing
CWRs (if the CWR handling feature cannot be turned off).

If NIC is completely unaware of RFC3168 ECN (doesn't support
NETIF_F_TSO_ECN) or its TSO engine can be set to not touch CWR flag
despite supporting also NETIF_F_TSO_ECN, TSO could be safely used with
AccECN on such NIC. I've little expertise on TSO HW so I don't know if
some/what NICs can do it. Nonetheless, there's nothing fundamental
preventing TSO being enabled with AccECN by NICs (if either of those
conditions is met).

In the cases, where TSO would fail to keep its hands off CWR flag, it
should fallback to GSO which has always on AccECN support (similar to
always on ECN support). I think that the current GSO changes are capable
of handling AccECN skbs. For the other parts of the idea above I'm not
so sure. There is this NETIF_F_GSO_SOFTWARE with comment that seems to
indicate it's doing what I want but I'm not fully sure if adding a flag
there is enough to achieve the desired effect?

On the rx side, supporting both RFC3168 and AccECN style CWR handling
is slightly more complicated (and possibly not worth the effort given
CWRs should be relatively rare with RFC3168-style ECN).

> Why tun driver is changed and not others ?

I think I didn't really understand why tun.c plays with the TSO_ECN flag
until now (after finding a related comment from tap.c) and so that change
now doesn't make much sense for me now any more. So I'll just remove that
part.

> I believe you need to give much more details in changelog in general,
> because many changes are not that obvious.

I'll try to.

Thanks.

--
 i.


-- 
Make Music, Not War

Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-435-0729