MFC after: 1 week
Sponsored by: NVIDIA Networking
FYI, I'm going to pull out the non-TOE-specific bits of the toe_tls_rxswitch branch and post them for review. Once you are able to rebase on those that will help narrow down the diff a bit.
Some other things we talked about on a call today:
- While using the route works ok in some cases, it doesn't work for lagg really. Eventually this needs to be replaced with storing a leaf_ifp in an mbuf packet header somehow and using mismatches of that to drive resetting the tag as well as using the leaf_ifp to find the ifp to allocate the initial tag on.
- For the case where we are rencrypting an mbuf, I think what we want is to have OCF allocate a separate session just using the cipher (e.g. AES-CTR). We would then encrypt a block of zeroes and then XOR the resulting keystream against the M_ENCRYPTED mbufs to get a fully decrypted TLS record.
- We probably want an optimization for the common case in sbappend_ktls_rx() that directly appends a fully decrypted TLS record to sb_m instead of always running the mbufs through the kthread.
In general I think it is also good to figure out if there are some smaller things we can pull out. Some of these changes above might also be version 2 optimization type things. 3) I will want anyway for future TOE TLS changes (and might get around to doing soon).
Hmm, I need to think about moving this. My TOE patches did move it, but TOE as it is today isn't ready for this to move I think.
I don't think this will really do the right thing if there are multiple TLS records in flight? Or at least, it seems to return an arbitrary future TCP sequence against an older TLS record?
Ah, it seems this needs the first TLS record, and is only really useful in the case that sb_tlscc is zero. For correctness you might also want to subtract off the count of data currently being decrypted as well? We should also document what semantics this returns (first TLS record rather than last).
We might be able to infer this from the existing send tag's type instead and avoid adding the new field to the TLS session if this is the only place it is used.
I would use __assume_notreached() or whatever the wrapper we for that is for cases that are impossible here and in other places.
I think this doesn't handle the case that there are multiple TLS records in the queue? The fetch_info routine tries to handle that case. Or perhaps what happens is that you keep getting TCP sequences that have already passed and can't get in sync until there is only zero or 1 TLS records in the sb_mtls queue?
It might be easier to read if 'state' is an enum of some sort, or at least uses #define values.
I would maybe move these into the case 0 declaring variables here is a bit unusual style.
I would perhaps defer renaming this for now until we rename the underlying type? This would also reduce the diff for now?
Yes, that's OK for now. We also need to implement the RXTLS version of these flags!
kib@ is working on that.
This is not how this mechanism works. The hardware speculates into the TLS header detection, and if a valid chain is found based on a past TCP sequence number which is re-loaded every now and then by the HW, the decryption immediately starts, after that the software confirms this TCP sequence number really points to the start of a valid TLS record.
The callback mechanism into the driver, simply builds a small database record of past TLS TCP start sequence numbers. Later on the driver queries the hardware, where it currently started decrypting, and if a match is found, we are good.
That means this function only provides the TCP SN of the current TLS record being processed and not the last one.