+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:
> 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@xxxxxxxxxx: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@xxxxxxxxxx: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@xxxxxxxxxx/T/#u
[..]
Thanks,
Dmitry
