netdev - Re: [PATCH net] net: Disable NETIF_F_HW_TLS_TX when HW

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] [day] [month] [year] [list]

Message-ID: <e64c2792-c8e6-fc1a-8208-434239073fe1@gmail.com>
Date:   Sun, 8 Nov 2020 17:19:51 +0200
From:   Tariq Toukan <ttoukan.linux@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Saeed Mahameed <saeedm@...dia.com>,
        Moshe Shemesh <moshe@...dia.com>,
        Boris Pismenny <borisp@...dia.com>, tariqt@...dia.com
Subject: Re: [PATCH net] net: Disable NETIF_F_HW_TLS_TX when HW_CSUM is
 disabled



On 11/5/2020 6:00 PM, Jakub Kicinski wrote:
> On Thu, 5 Nov 2020 15:22:53 +0200 Tariq Toukan wrote:
>> On 11/4/2020 11:25 PM, Jakub Kicinski wrote:
>>> On Wed,  4 Nov 2020 12:21:41 +0200 Tariq Toukan wrote:
>>>> With NETIF_F_HW_TLS_TX packets are encrypted in HW. This cannot be
>>>> logically done when HW_CSUM offload is off.
>>>
>>> Right. Do you expect drivers to nack clearing NETIF_F_HW_TLS_TX when
>>> there are active connections, then?  I don't think NFP does.  We either
>>> gotta return -EBUSY when there are offloaded connections, or at least
>>> clearly document the expected behavior.
>>
>> As I see from code, today drivers and TLS stack allow clearing
>> NETIF_F_HW_TLS_TX without doing anything to change behavior in existing
>> sockets, so they continue to do HW offload. Only new sockets will be
>> affected.
> 
> Right, we want to let users turn off the offload when it misbehaves,
> and we don't have a way of un-offloading connections.
> 
>> I think the same behavior should apply when NETIF_F_HW_TLS_TX is cleared
>> implicitly (due to clearing HW_CSUM).
> 
> Right, I don't mind either way. My thinking with the tls feature was
> that offload is likely to be broken, at least initially. Checksum
> offload should work, one would hope, so its less of a risk.
> 
> The question is perhaps - do we care more about consistency with the
> behavior of the TLS feature, or current expectation that csum offload
> off will actually turn it off.
> 
>> If the existing behavior is not expected, and we should force fallback
>> to SW kTLS for existing sockets, then I think this should be fixed
>> independently to this patch, as it introduces no new regression.
>>
>> What do you think?
> 
> The current behavior of the TLS features is documented in
> tls-offload.rst, you can do what you're doing in this patch,
> but you need to document it there.
> 

I documented this in tls-offload.rst.
The preceding paragraph (with the note about old connections) is still 
valid.