Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <d0e332b4326eec032b77c7492f501f3c1fbe8242.camel@redhat.com>
Subject: Re: [PATCH v12 net-next 06/23] net/tcp: Add TCP-AO sign to outgoing
 packets
From:   Paolo Abeni <pabeni@redhat.com>
To:     Dmitry Safonov <dima@arista.com>, David Ahern <dsahern@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>
Cc:     linux-kernel@vger.kernel.org,
        Andy Lutomirski <luto@amacapital.net>,
        Ard Biesheuvel <ardb@kernel.org>,
        Bob Gilligan <gilligan@arista.com>,
        Dan Carpenter <error27@gmail.com>,
        David Laight <David.Laight@aculab.com>,
        Dmitry Safonov <0x7f454c46@gmail.com>,
        Donald Cassidy <dcassidy@redhat.com>,
        Eric Biggers <ebiggers@kernel.org>,
        "Eric W. Biederman" <ebiederm@xmission.com>,
        Francesco Ruggeri <fruggeri05@gmail.com>,
        "Gaillardetz, Dominik" <dgaillar@ciena.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        Ivan Delalande <colona@arista.com>,
        Leonard Crestez <cdleonard@gmail.com>,
        "Nassiri, Mohammad" <mnassiri@ciena.com>,
        Salam Noureddine <noureddine@arista.com>,
        Simon Horman <simon.horman@corigine.com>,
        "Tetreault, Francois" <ftetreau@ciena.com>, netdev@vger.kernel.org
Date:   Thu, 21 Sep 2023 13:20:34 +0200
In-Reply-To: <20230918190027.613430-7-dima@arista.com>
References: <20230918190027.613430-1-dima@arista.com>
         <20230918190027.613430-7-dima@arista.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Mon, 2023-09-18 at 20:00 +0100, Dmitry Safonov wrote:
> @@ -615,19 +616,43 @@ static void bpf_skops_write_hdr_opt(struct sock *sk=
, struct sk_buff *skb,
>   * (but it may well be that other scenarios fail similarly).
>   */
>  static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
> -			      struct tcp_out_options *opts)
> +			      struct tcp_out_options *opts,
> +			      struct tcp_key *key)
>  {
>  	__be32 *ptr =3D (__be32 *)(th + 1);
>  	u16 options =3D opts->options;	/* mungable copy */
> =20
> -	if (unlikely(OPTION_MD5 & options)) {
> +	if (tcp_key_is_md5(key)) {
>  		*ptr++ =3D htonl((TCPOPT_NOP << 24) | (TCPOPT_NOP << 16) |
>  			       (TCPOPT_MD5SIG << 8) | TCPOLEN_MD5SIG);
>  		/* overload cookie hash location */
>  		opts->hash_location =3D (__u8 *)ptr;
>  		ptr +=3D 4;
> -	}
> +	} else if (tcp_key_is_ao(key)) {
> +#ifdef CONFIG_TCP_AO
>
> +		struct tcp_ao_key *rnext_key;
> +		struct tcp_ao_info *ao_info;
> +		u8 maclen;
> =20
> +		ao_info =3D rcu_dereference_check(tp->ao_info,
> +				lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
> +		rnext_key =3D READ_ONCE(ao_info->rnext_key);
> +		if (WARN_ON_ONCE(!rnext_key))
> +			goto out_ao;
> +		maclen =3D tcp_ao_maclen(key->ao_key);

It looks like only TCP_AO really accesses 'key', and TCP_AO can easily
fetch it from ao_info.=C2=A0Can the AO key change in between
tcp_get_current_key() and here?

Otherwise I think it would be better omitting the 'key' argument here
and use the 'options' flag to pick TCP_AO vs MD5.

And:

	if (unlikely(OPTION_MD5 & options)) {

could possibly be moved under a CONFIG_MD5 compiler conditional.

Cheers,

Paolo