netdev - Re: linux-next: manual merge of the net-next tree with the net tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <679ddff6-db6e-4ff6-b177-574e90d0103d@tessares.net>
Date: Mon, 21 Aug 2023 14:09:38 +0200
From: Matthieu Baerts <matthieu.baerts@...sares.net>
To: Stephen Rothwell <sfr@...b.auug.org.au>,
 David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>
Cc: Networking <netdev@...r.kernel.org>, Eric Dumazet <edumazet@...gle.com>,
 Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
 Linux Next Mailing List <linux-next@...r.kernel.org>
Subject: Re: linux-next: manual merge of the net-next tree with the net tree

Hi Stephen,

On 21/08/2023 03:06, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the net-next tree got a conflict in:
> 
>   include/net/inet_sock.h
> 
> between commit:
> 
>   f866fbc842de ("ipv4: fix data-races around inet->inet_id")
> 
> from the net tree and commit:
> 
>   c274af224269 ("inet: introduce inet->inet_flags")
> 
> from the net-next tree.

Thank you for this conflict resolution!

I had the same issue on our side with MPTCP tree when syncing -net and
net-next and I resolved it a bit differently. Here my comment on the
patch you used:

> diff --cc include/net/inet_sock.h
> index 491ceb7ebe5d,acbb93d7607a..000000000000
> --- a/include/net/inet_sock.h
> +++ b/include/net/inet_sock.h
> @@@ -218,12 -218,12 +218,12 @@@ struct inet_sock 
>   #define inet_dport		sk.__sk_common.skc_dport
>   #define inet_num		sk.__sk_common.skc_num
>   
> + 	unsigned long		inet_flags;
>   	__be32			inet_saddr;
>   	__s16			uc_ttl;
> - 	__u16			cmsg_flags;
> - 	struct ip_options_rcu __rcu	*inet_opt;
> - 	atomic_t		inet_id;
>   	__be16			inet_sport;
> ++	atomic_t		inet_id;
> + 	struct ip_options_rcu __rcu	*inet_opt;

I first put "inet_opt", then "inet_id" here to avoid a hole of 4 bytes.
So just switching these two lines.

Here is the output of pahole when using your patch:

> struct inet_sock {
>         struct sock                sk __attribute__((__aligned__(8))); /*     0   768 */
>         /* --- cacheline 12 boundary (768 bytes) --- */
>         struct ipv6_pinfo *        pinet6;               /*   768     8 */
>         long unsigned int          inet_flags;           /*   776     8 */
>         __be32                     inet_saddr;           /*   784     4 */
>         __s16                      uc_ttl;               /*   788     2 */
>         __be16                     inet_sport;           /*   790     2 */
>         atomic_t                   inet_id;              /*   792     4 */
> 
>         /* XXX 4 bytes hole, try to pack */
> 
>         struct ip_options_rcu *    inet_opt;             /*   800     8 */
>         __u8                       tos;                  /*   808     1 */
>         __u8                       min_ttl;              /*   809     1 */
>         __u8                       mc_ttl;               /*   810     1 */
>         __u8                       pmtudisc;             /*   811     1 */
>         __u8                       rcv_tos;              /*   812     1 */
>         __u8                       convert_csum;         /*   813     1 */
> 
>         /* XXX 2 bytes hole, try to pack */
> 
>         int                        uc_index;             /*   816     4 */
>         int                        mc_index;             /*   820     4 */
>         __be32                     mc_addr;              /*   824     4 */

And now when "inet_id" is placed after "inet_opt":

> struct inet_sock {
>         struct sock                sk __attribute__((__aligned__(8))); /*     0   768 */
>         /* --- cacheline 12 boundary (768 bytes) --- */
>         struct ipv6_pinfo *        pinet6;               /*   768     8 */
>         long unsigned int          inet_flags;           /*   776     8 */
>         __be32                     inet_saddr;           /*   784     4 */
>         __s16                      uc_ttl;               /*   788     2 */
>         __be16                     inet_sport;           /*   790     2 */
>         struct ip_options_rcu *    inet_opt;             /*   792     8 */
>         atomic_t                   inet_id;              /*   800     4 */
>         __u8                       tos;                  /*   804     1 */
>         __u8                       min_ttl;              /*   805     1 */
>         __u8                       mc_ttl;               /*   806     1 */
>         __u8                       pmtudisc;             /*   807     1 */
>         __u8                       rcv_tos;              /*   808     1 */
>         __u8                       convert_csum;         /*   809     1 */
> 
>         /* XXX 2 bytes hole, try to pack */
> 
>         int                        uc_index;             /*   812     4 */
>         int                        mc_index;             /*   816     4 */
>         __be32                     mc_addr;              /*   820     4 */

I noticed that in Eric's patch for the net tree -- f866fbc842de ("ipv4:
fix data-races around inet->inet_id") -- he moved "inet_id" above, I
guess for a similar reason.

Just in case you are interested by taking my version, I attached the
patch I used and the Rerere cache is available there:

https://github.com/multipath-tcp/mptcp-upstream-rr-cache/commit/e63f34f8

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
View attachment "47fe53ec8c3d4381989f0541eb0a0abd31b8cbee.patch" of type "text/x-patch" (548 bytes)