ktls / af_ktls

Linux Kernel TLS/DTLS Module
GNU General Public License v2.0
157 stars 25 forks source link

Buffer changes #55

Closed djwatson closed 8 years ago

djwatson commented 8 years ago

Met with a couple netdev kernel guys (therbert, hannes, alexei) to discuss buffer management in ktls. Consensus was roughly:

1) Preallocated send/recv buffers per-socket aren't going to work, would be too much memory for large numbers of sockets. Need to allocate them lazily as we need to decode/encode. Alternatively, if we aren't pre-emptable, they could possibly be preallocated per-cpu as an optimization.

2) When the tcp socket has data ready, it needs to be pulled off the socket and stored the ktls. Otherwise a tcp socket with a receive buffer size of 4k, and being sent a larger TLS message, would not result in forward progress. There are also other reasons this needs to happen, like out of order receives in tcp interacting in interesting ways with the receive buffer size, that are also fixed by moving the sk_buffs to being owned by ktls instead. KCM already does a version of this.

3) Output buffers should probably be stored as sk_buffs on the socket, and not scattergather lists. This would facilitate passing ktls sockets to other kernel things (KCM), as well as supporting multiple message decode by respecting receive buffer sizes on ktls easily.

fridex commented 8 years ago

ad 1) Yes, this should be dropped for sure. It also limits using kernel_sendpage() (https://github.com/ktls/af_ktls/issues/22 and https://github.com/ktls/af_ktls/commit/0fbc8d145d5f12490e1a51627b9cef7a3e6b8b6b)

ad 2) Applicable only for data records I suppose. However this will limit switching from OpenSSL, GnuTLS or another TLS lib. Once AF_KTLS socket will be instantiated, switching back to the lib will be possible only if there is some error during decryption or non-data record. If we do not introduce a mechanism for explicitly switching this off. Do we want it?

ad 3) Sounds good to me.

djwatson commented 8 years ago

2) Right, only pull them off for data records. Yes, we'd have to figure out something reasonable to do to switch back and forth between ktls and OpenSSL / GnuTLS handling of data. This is similar to #53

nmav commented 8 years ago

2) Right, only pull them off for data records. Yes, we'd have to figure out something reasonable to do to switch back and forth between ktls and OpenSSL / GnuTLS handling of data. This is similar to #53

Correct. In the case of openconnect VPN, we would like to switch to userspace handling when a particular application-data packet is encountered (the particular determined by a future feature, to add bfp). If we could do the packet processing in peek mode, and then discard if successful, we would address such a future use case.

djwatson commented 8 years ago

There were some patches posted upstream that should help with some of the stream parsing

http://comments.gmane.org/gmane.linux.network/420289

djwatson commented 8 years ago

Mostly fixed by https://github.com/ktls/af_ktls/pull/62, at least the first and second

djwatson commented 8 years ago

3) fixed by https://github.com/ktls/af_ktls/pull/84