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.
