Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <b1f1e94b-593a-4eb7-907c-cf5d61d74510@arista.com>
Date:   Wed, 11 Oct 2023 20:23:37 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v14 net-next 08/23] net/tcp: Add AO sign to RST packets
Content-Language: en-US
To:     Eric Dumazet <edumazet@google.com>
Cc:     David Ahern <dsahern@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
        Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        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
References: <20231009230722.76268-1-dima@arista.com>
 <20231009230722.76268-9-dima@arista.com>
 <CANn89iLo8sOL=CnAvMv_PSeS_hUQ0cfF6LdFEDnuwGxhSmo+xg@mail.gmail.com>
From:   Dmitry Safonov <dima@arista.com>
In-Reply-To: <CANn89iLo8sOL=CnAvMv_PSeS_hUQ0cfF6LdFEDnuwGxhSmo+xg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 10/11/23 19:04, Eric Dumazet wrote:
> On Tue, Oct 10, 2023 at 1:07 AM Dmitry Safonov <dima@arista.com> wrote:
[..]
>>  struct tcp_key {
>>         union {
>> -               struct tcp_ao_key *ao_key;
>> +               struct {
>> +                       struct tcp_ao_key *ao_key;
>> +                       u32 sne;
>> +                       char *traffic_key;
> 
> Move sne after traffic_key to avoid a hole on 64bit arches.

Sure, thanks!

[..]
>> @@ -435,6 +495,46 @@ struct tcp_ao_key *tcp_v4_ao_lookup(const struct sock *sk, struct sock *addr_sk,
>>         return tcp_ao_do_lookup(sk, addr, AF_INET, sndid, rcvid);
>>  }
>>
>> +int tcp_ao_prepare_reset(const struct sock *sk, struct sk_buff *skb,
>> +                        const struct tcp_ao_hdr *aoh, int l3index,
>> +                        struct tcp_ao_key **key, char **traffic_key,
>> +                        bool *allocated_traffic_key, u8 *keyid, u32 *sne)
>> +{
>> +       struct tcp_ao_info *ao_info;
>> +
>> +       *allocated_traffic_key = false;
>> +       /* If there's no socket - than initial sisn/disn are unknown.
>> +        * Drop the segment. RFC5925 (7.7) advises to require graceful
>> +        * restart [RFC4724]. Alternatively, the RFC5925 advises to
>> +        * save/restore traffic keys before/after reboot.
>> +        * Linux TCP-AO support provides TCP_AO_ADD_KEY and TCP_AO_REPAIR
>> +        * options to restore a socket post-reboot.
>> +        */
>> +       if (!sk)
>> +               return -ENOTCONN;
>> +
>> +       if ((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_NEW_SYN_RECV)) {
>> +               return -1;
>> +       } else {
>> +               struct tcp_ao_key *rnext_key;
>> +
>> +               if (sk->sk_state == TCP_TIME_WAIT)
> 
> Why not adding TCPF_TIME_WAIT in the prior test ?

Well, twsk is wired up here in the next patch "net/tcp: Add TCP-AO sign
to twsk", while the condition for request sockets is the patch after that.
I probably can move it to the same condition just to polish the code in
the middle of patch set, I guess it felt not critical when I split it by
patches.

> 
>> +                       return -1;

[..]

Thanks,
           Dmitry