[MPTCP] [PATCH 00/18] Make TCP_MD5 adopt the extra-option framework

* [MPTCP] [PATCH 00/18] Make TCP_MD5 adopt the extra-option framework
@ 2017-10-03 16:21 Christoph Paasch
  0 siblings, 0 replies; 2+ messages in thread
From: Christoph Paasch @ 2017-10-03 16:21 UTC (permalink / raw)
  To: mptcp 

[-- Attachment #1: Type: text/plain, Size: 4670 bytes --]

Tried to split it in several patches.

Here is the overview:
Patch 1 to 5: Solely TCP-level changes to prepare the stack. Mostly cosmetic,
              besides #1 and #5. #1 should be ok, #5 is a bit more fundamental
	      change but I think it's still ok.
Patch 6 to 11: Mat's patch, together with some more changes to it. My changes
               should be merged into Mat's patch, but I let them out for now
	       so that it's clear what I had to change.
Patch 12 to 14: Changes to TCP_MD5 to enable it to opt-in to the framework.
                Should be all fairly straight-forward.
Patch 15: Here, I move all TCP_MD5-code to its own file.
Patch 16: I should probably move this to the patchset 12 to 14.
Patch 17: Here, I start using the framework
Patch 18: Cleanup thanks to the use of the framework.

In particular, patch 18 shows how the framework allows to cleanup TCP from
TCP_MD5-changes.

Some comments about the extra-option framework:

1) As already discussed, it would be good to have the callbacks on a per-socket
basis. Basically, I think the list should be in tcp_sock where this list would
simply be a list of structs:
struct tcp_extra_option {
	struct list_head teo_list;
	struct tcp_extra_options_opts *teo_ops;
};

Users of this framework would allocate these elements like this (e.g., TCP_MD5):
struct tcp_md5_extra_option {
	struct tcp_extra_option tcp_md5_eo;
	struct tcp_md5sig_info  __rcu *md5sig_info;
};

This allows MD5 to store its state info inside there. We could nicely fit MPTCP
into this.

2)
I chose to leave the extra_options at the end of tcp_syn_options.
This however means that we have to undo what TCP timestamps did in tcp_syn_options.
Unsetting the flags is easy, but the nasty part is with:
+               /* Don't use TCP timestamps with TCP_MD5 */
+               if (opts->options & OPTION_TS)
+                       ret -= TCPOLEN_TSTAMP_ALIGNED;

Yes, this is ugly. But, the only way from what I see if we want to fully
remove TCP_MD5 out of TCP.

3) With request_socks now being full socks, the prepare and prepare_req callbacks
could be merged.

4) We also need this framwork for time_wait_socks. So, in general, we could have
the callback list in struct sock_common as well, that way tcp_sock, tcp_time_wait_sock
and request_socks get it.

This implies that we need to initialize this in tcp_time_wait() and destroy it
in tcp_twsk_destructor. That would allow to remove even more MD5-code out of
TCP.

I can work on these changes to the extra-option framework.

Any thoughts? :)

Christoph Paasch (17):
  tcp: Write options after the header has been fully done
  tcp: Pass sock to tcp_options_write instead of tcp_sock
  tcp: Pass skb in tcp_options_write
  tcp: Pass sock to tcp_synack_options
  tcp6: Prepare for maximum TCP option-space
  tcp_extra_option: add_header callback for TCP extra options
  tcp_extra_option: Add missing static branch for
    tcp_extra_options_prepare
  tcp_extra_option: Use sock instead of tcp_sock in
    tcp_extra_options_parse/prepare/write
  tcp_extra_option: Pass skb to tcp_extra_options_write
  tcp_extra_option: Pass sock to prepare_req
  tcp_md5: Detect key inside tcp_v4_send_ack instead of passing it as an
    argument
  tcp_md5: Detect key inside tcp_v6_send_response instead of passing it
    as an argument
  tcp_md5: Check for TCP_MD5 after TCP Timestamps in
    tcp_established_options
  tcp: Move TCP-MD5 code out of TCP itself
  tcp_md5: Don't pass along md5-key
  tcp_md5: Use tcp_extra_options in output path
  tcp_md5: Cleanup TCP-code

Mat Martineau (1):
  tcp: Register handlers for extra TCP options

 drivers/infiniband/hw/cxgb4/cm.c |    2 +-
 include/linux/inet_diag.h        |    1 +
 include/linux/tcp.h              |   19 +-
 include/linux/tcp_md5.h          |  109 ++++
 include/net/tcp.h                |  157 ++---
 net/ipv4/Makefile                |    1 +
 net/ipv4/syncookies.c            |    2 +-
 net/ipv4/tcp.c                   |  219 +++----
 net/ipv4/tcp_diag.c              |   81 +--
 net/ipv4/tcp_input.c             |   54 +-
 net/ipv4/tcp_ipv4.c              |  496 +---------------
 net/ipv4/tcp_md5.c               | 1169 ++++++++++++++++++++++++++++++++++++++
 net/ipv4/tcp_minisocks.c         |   35 +-
 net/ipv4/tcp_output.c            |  139 ++---
 net/ipv6/syncookies.c            |    2 +-
 net/ipv6/tcp_ipv6.c              |  352 +-----------
 16 files changed, 1560 insertions(+), 1278 deletions(-)
 create mode 100644 include/linux/tcp_md5.h
 create mode 100644 net/ipv4/tcp_md5.c

-- 
2.14.1

^ permalink raw reply	[flat|nested] 2+ messages in thread