[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0c201b3e-fc3a-4cee-b056-8338da7261b9@arista.com>
Date: Wed, 2 Aug 2023 20:37:19 +0100
From: Dmitry Safonov <dima@...sta.com>
To: David Ahern <dsahern@...nel.org>,
Eric Dumazet <edumazet@...gle.com>,
Paolo Abeni <pabeni@...hat.com>,
Jakub Kicinski <kuba@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Simon Horman <simon.horman@...igine.com>
Cc: linux-kernel@...r.kernel.org,
Andy Lutomirski <luto@...capital.net>,
Ard Biesheuvel <ardb@...nel.org>,
Bob Gilligan <gilligan@...sta.com>,
Dan Carpenter <error27@...il.com>,
David Laight <David.Laight@...lab.com>,
Dmitry Safonov <0x7f454c46@...il.com>,
Donald Cassidy <dcassidy@...hat.com>,
Eric Biggers <ebiggers@...nel.org>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Francesco Ruggeri <fruggeri05@...il.com>,
"Gaillardetz, Dominik" <dgaillar@...na.com>,
Herbert Xu <herbert@...dor.apana.org.au>,
Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
Ivan Delalande <colona@...sta.com>,
Leonard Crestez <cdleonard@...il.com>,
Salam Noureddine <noureddine@...sta.com>,
"Tetreault, Francois" <ftetreau@...na.com>, netdev@...r.kernel.org,
Steen Hegelund <Steen.Hegelund@...rochip.com>,
Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org
Subject: Re: [PATCH v9 net-next 00/23] net/tcp: Add TCP-AO support
+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@...hub.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@...hub.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
[..]
Thanks,
Dmitry
Powered by blists - more mailing lists