On 5/27/2021 8:56 PM, Jakub Kicinski wrote:

On Thu, 27 May 2021 17:07:06 +0300 Tariq Toukan wrote:

quoted

On 5/27/2021 3:47 AM, Jakub Kicinski wrote:

quoted

On Wed, 26 May 2021 12:57:41 +0300 Tariq Toukan wrote:

quoted

This RFC series suggests a solution for the following problem:

Bond interface and lower interface are both up with TLS RX/TX offloads on.
TX/RX csum offload is turned off for the upper, hence RX/TX TLS is turned off
for it as well.
Yet, although it indicates that feature is disabled, new connections are still
offloaded by the lower, as Bond has no way to impact that:
Return value of bond_sk_get_lower_dev() is agnostic to this change.

One way to solve this issue, is to bring back the Bond TLS operations callbacks,
i.e. provide implementation for struct tlsdev_ops in Bond.
This gives full control for the Bond over its features, making it aware of every
new TLS connection offload request.
This direction was proposed in the original Bond TLS implementation, but dropped
during ML review. Probably it's right to re-consider now.

Here I suggest another solution, which requires generic changes out of the bond
driver.

Fixes in patches 1 and 4 are needed anyway, independently to which solution
we choose. I'll probably submit them separately soon.

No opinions here, semantics of bond features were always clear
as mud to me. What does it mean that bond survived 20 years without
rx-csum? And it so why would TLS offload be different from what one
may presume the semantics of rx-csum are today?

Advanced device offloads have basic logical dependencies, that are
applied for all kind of netdevs, agnostic to internal details of each
netdev.

Nothing special with TLS really.
TLS device offload behaves similarly to TSO (needs HW_CSUM), and GRO_HW
(needs RXCSUM).
[...]

Right, the inter-dependency between features is obvious enough.
What makes a feature be part of UPPER_DISABLES though?

Regarding UPPER_DISABLES:
I propose using it here as an attempt to give the bond device some 
control over kTLS offloaded connections, to avoid cases where:
(*) UPPER.ktls_device_offload==OFF
(*) LOWER.ktls_device_offload==ON
(*) Newly created connections are offloaded!! Simply ignoring and 
bypassing the UPPER device state (this is how .ndo_sk_get_lower_dev works).

This is not my preferred solution though.
I think we should reconsider introducing bond implementation for "struct 
tlsdev_ops" callbacks, which gives bond interface full control and 
awareness to new TLS connections.