netdev - Re: [RFC PATCH 09/28] gso: AccECN support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.20.2003192233580.5256@whs-18.cs.helsinki.fi>
Date:   Fri, 20 Mar 2020 00:36:30 +0200 (EET)
From:   "Ilpo Järvinen" <ilpo.jarvinen@...helsinki.fi>
To:     Eric Dumazet <eric.dumazet@...il.com>
cc:     Netdev <netdev@...r.kernel.org>, Yuchung Cheng <ycheng@...gle.com>,
        Neal Cardwell <ncardwell@...gle.com>,
        Olivier Tilmans <olivier.tilmans@...ia-bell-labs.com>
Subject: Re: [RFC PATCH 09/28] gso: AccECN support

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@...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@...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.