Re: [PATCH v9 net-next 00/23] net/tcp: Add TCP-AO support
From: Dmitry Safonov <hidden>
Date: 2023-08-02 19:37:36
Also in:
lkml, netdev
+Cc: Simon I've realized that he wasn't in Cc list, albeit provided valuable feedback in v8. Sorry about it, definitely going to Cc on next versions. On 8/2/23 18:26, Dmitry Safonov wrote:
quoted hunk ↗ jump to hunk
Hi, This is version 9 of TCP-AO support. It's based on net-next as there's a trivial conflict with the commit dfa2f0483360 ("tcp: get rid of sysctl_tcp_adv_win_scale"). Most of the changes in this version address Simon's reviews and polish of patch set to please netdev/patchwork. I ran static analyzers over it, there's currently only one warning introduced, which is Sparse's context imbalance in tcp_sigpool_start(). I've spent some time trying to silence it, here are my findings: - __cond_acquires() is broken: refcount_dec_and_lock() produces Sparse warning - tried __acquires() + __releases(), as in bpf_sk_storage_map_seq_find_next(), yet it doesn't silence Sparse - I thought about moving rcu_read_unlock_bh() out of tcp_sigpool_start(), forcing the callers to call tcp_sigpool_end() even on error-paths, but: it feels wrong semantically and I'd have to initialize @c on error-case and check it in tcp_sigpool_end(). That feels even more wrong. I've placed __cond_acquires() to tcp_sigpool_start() definition, expecting that Sparse may be fixed in future to do proper thing. Worth mentioning that it also complains about many other functions including: sk_clone_lock(), sk_free_unlock_clone(), tcp_conn_request() and etc. Also, more checkpatch.pl warnings addressed, but yet I've left the ones that are more personal preferences (i.e. 80 columns limit). Please, ping me if you have a strong feeling about one of them. Worth mentioning removing in-kernel wiring for TCP-AO key port matching: it was restricted in uAPI and still it is. Removing from initial TCP-AO implementation port matching as it can be added post-merge. The following changes since commit 34093c9fa05df24558d1e2c5d32f7f93b2c97ee9: net: Remove duplicated include in mac.c (2023-08-02 11:42:47 +0100) are available in the Git repository at: git@github.com:0x7f454c46/linux.git tcp-ao-v9 for you to fetch changes up to c1cf20fddf71a9ae9f07cb04a5a1efcce156c5ab: Documentation/tcp: Add TCP-AO documentation (2023-08-02 17:28:15 +0100) ---------------------------------------------------------------- And another branch with selftests, that will be sent later separately: git@github.com:0x7f454c46/linux.git tcp-ao-v9-with-selftests Thanks for your time and reviews, Dmitry--- Changelog ---Changes from v8: - Based on net-next - Now doing git request-pull, rather than GitHub URLs - Fix tmp_key buffer leak, introduced in v7 (Simon) - More checkpatch.pl warning fixes (even to the code that existed but was touched) - More reverse Xmas tree declarations (Simon) - static code analysis fixes - Removed TCP-AO key port matching code - Removed `inline' for for static functions in .c files to make netdev/source_inline happy (I didn't know it's a thing) - Moved tcp_ao_do_lookup() to a commit that uses it (Simon) - __tcp_ao_key_cmp(): prefixlen is bits, but memcmp() uses bytes - Added TCP port matching limitation to Documentation/networking/tcp_ao.rst Version 8: https://lore.kernel.org/all/20230719202631.472019-1-dima@arista.com/T/#u (local)
[..]
Thanks,
Dmitry