[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7d00ad25-abaa-191d-8e80-32674377b053@kernel.org>
Date: Tue, 19 Dec 2023 08:45:26 -0800 (PST)
From: Mat Martineau <martineau@...nel.org>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
cc: Matthieu Baerts <matttbe@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
edumazet@...gle.com, andrii@...nel.org, ast@...nel.org,
bpf@...r.kernel.org, daniel@...earbox.net, kuni1840@...il.com,
martin.lau@...ux.dev, netdev@...r.kernel.org, mptcp@...ts.linux.dev
Subject: Re: [PATCH v6 bpf-next 3/6] bpf: tcp: Handle BPF SYN Cookie in
skb_steal_sock().
On Fri, 15 Dec 2023, Kuniyuki Iwashima wrote:
> From: Eric Dumazet <edumazet@...gle.com>
> Date: Thu, 14 Dec 2023 17:31:15 +0100
>> On Thu, Dec 14, 2023 at 4:56 PM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
>>>
>>> We will support arbitrary SYN Cookie with BPF.
>>>
>>> If BPF prog validates ACK and kfunc allocates a reqsk, it will
>>> be carried to TCP stack as skb->sk with req->syncookie 1. Also,
>>> the reqsk has its listener as req->rsk_listener with no refcnt
>>> taken.
>>>
>>> When the TCP stack looks up a socket from the skb, we steal
>>> inet_reqsk(skb->sk)->rsk_listener in skb_steal_sock() so that
>>> the skb will be processed in cookie_v[46]_check() with the
>>> listener.
>>>
>>> Note that we do not clear skb->sk and skb->destructor so that we
>>> can carry the reqsk to cookie_v[46]_check().
>>>
>>> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
>>> ---
>>> include/net/request_sock.h | 15 +++++++++++++--
>>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
>>> index 26c630c40abb..8839133d6f6b 100644
>>> --- a/include/net/request_sock.h
>>> +++ b/include/net/request_sock.h
>>> @@ -101,10 +101,21 @@ static inline struct sock *skb_steal_sock(struct sk_buff *skb,
>>> }
>>>
>>> *prefetched = skb_sk_is_prefetched(skb);
>>> - if (*prefetched)
>>> + if (*prefetched) {
>>> +#if IS_ENABLED(CONFIG_SYN_COOKIES)
>>> + if (sk->sk_state == TCP_NEW_SYN_RECV && inet_reqsk(sk)->syncookie) {
>>> + struct request_sock *req = inet_reqsk(sk);
>>> +
>>> + *refcounted = false;
>>> + sk = req->rsk_listener;
>>> + req->rsk_listener = NULL;
>>
>> I am not sure about interactions with MPTCP.
>>
>> I would be nice to have their feedback.
>
> Matthieu, Mat, Paolo, could you double check if the change
> above is sane ?
> https://lore.kernel.org/bpf/20231214155424.67136-4-kuniyu@amazon.com/
Hi Kuniyuki -
Yes, we will take a look. Haven't had time to look in detail yet but I
wanted to let you know we saw your message and will follow up.
- Mat
>
>
> Short sumamry:
>
> With this series, tc could allocate reqsk to skb->sk and set a
> listener to reqsk->rsk_listener, then __inet_lookup_skb() returns
> a listener in the same reuseport group, and skb is processed in the
> listener function flow, especially cookie_v[46]_check().
>
> The only difference here is that skb->sk has reqsk, which does not
> have rsk_listener.
>
>
>>
>>> + return sk;
>>> + }
>>> +#endif
>>> *refcounted = sk_is_refcounted(sk);
>>> - else
>>> + } else {
>>> *refcounted = true;
>>> + }
>>>
>>> skb->destructor = NULL;
>>> skb->sk = NULL;
>>> --
>>> 2.30.2
>
>
Powered by blists - more mailing lists