netdev - Re: [PATCH net-next 0/2] TLS TX HW offload for Bond

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 for Android: free password hash cracker in your pocket

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

Message-ID: <0e4a04f2-2ffa-179d-3b7b-ef08b52c9290@gmail.com>
Date:   Thu, 19 Nov 2020 17:59:38 +0200
From:   Tariq Toukan <ttoukan.linux@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>, Tariq Toukan <tariqt@...dia.com>
Cc:     "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Saeed Mahameed <saeedm@...dia.com>,
        Moshe Shemesh <moshe@...dia.com>
Subject: Re: [PATCH net-next 0/2] TLS TX HW offload for Bond



On 11/19/2020 2:02 AM, Jakub Kicinski wrote:
> On Sun, 15 Nov 2020 15:42:49 +0200 Tariq Toukan wrote:
>> This series opens TLS TX HW offload for bond interfaces.
>> This allows bond interfaces to benefit from capable slave devices.
>>
>> The first patch adds real_dev field in TLS context structure, and aligns
>> usages in TLS module and supporting drivers.
>> The second patch opens the offload for bond interfaces.
>>
>> For the configuration above, SW kTLS keeps picking the same slave
>> To keep simple track of the HW and SW TLS contexts, we bind each socket to
>> a specific slave for the socket's whole lifetime. This is logically valid
>> (and similar to the SW kTLS behavior) in the following bond configuration,
>> so we restrict the offload support to it:
>>
>> ((mode == balance-xor) or (mode == 802.3ad))
>> and xmit_hash_policy == layer3+4.
> 
> This does not feel extremely clean, maybe you can convince me otherwise.
> 
> Can we extend netdev_get_xmit_slave() and figure out the output dev
> (and if it's "stable") in a more generic way? And just feed that dev
> into TLS handling? 

Hi Jakub,

I don't see we go through netdev_get_xmit_slave(), but through 
.ndo_start_xmit (bond_start_xmit). Currently I have my check there to 
catch all skbs belonging to offloaded TLS sockets.

The TLS offload get_slave() logic decision is per socket, so the result 
cannot be saved in the bond memory. Currently I save the real_dev field 
in the TLS context structure.
One way to make it more generic is to save it on the sock structure. I 
agree that this replaces the TLS-specific logic, but demands increasing 
the sock struct, and has larger impact on all other flows...

What do you think?
If we decide to go with this, I can provide the patches.

> All non-crypto upper SW devs should be safe to cross
> with .decrypted = 1 skbs, right?
> 

AFAIU yes.