netdev - Re: [PATCH net-next v9 09/10] tcp: make the dropreason really work when calling tcp_rcv_state

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL+tcoCcppce8LMPsFjFOE=QQPDjm+XXaByF+Fd=ZN13P98Yxw@mail.gmail.com>
Date: Sat, 24 Feb 2024 07:59:22 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: davem@...emloft.net, dsahern@...nel.org, edumazet@...gle.com, 
	kernelxing@...cent.com, kuba@...nel.org, netdev@...r.kernel.org, 
	pabeni@...hat.com
Subject: Re: [PATCH net-next v9 09/10] tcp: make the dropreason really work
 when calling tcp_rcv_state_process()

On Sat, Feb 24, 2024 at 3:45 AM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
>
> From: Jason Xing <kerneljasonxing@...il.com>
> Date: Fri, 23 Feb 2024 18:28:50 +0800
> > From: Jason Xing <kernelxing@...cent.com>
> >
> > Update three callers including both ipv4 and ipv6 and let the dropreason
> > mechanism work in reality.
> >
> > Signed-off-by: Jason Xing <kernelxing@...cent.com>
> > Reviewed-by: Eric Dumazet <edumazet@...gle.com>
> > Reviewed-by: David Ahern <dsahern@...nel.org>
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@...zon.com>
>
> two nits below.
>
>
> > --
> > v9
> > Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> > 1. add reviewed-by tag (David)
> >
> > v8
> > Link: https://lore.kernel.org/netdev/CANn89i+Uikp=NvB7SVQpYnX-2FqJrH3hWw3sV0XpVcC55MiNUg@mail.gmail.com/
> > 1. add reviewed-by tag (Eric)
> > ---
> >  include/net/tcp.h        | 2 +-
> >  net/ipv4/tcp_ipv4.c      | 3 ++-
> >  net/ipv4/tcp_minisocks.c | 9 +++++----
> >  net/ipv6/tcp_ipv6.c      | 3 ++-
> >  4 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index e5af9a5b411b..1d9b2a766b5e 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -396,7 +396,7 @@ enum tcp_tw_status tcp_timewait_state_process(struct inet_timewait_sock *tw,
> >  struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
> >                          struct request_sock *req, bool fastopen,
> >                          bool *lost_race);
> > -int tcp_child_process(struct sock *parent, struct sock *child,
> > +enum skb_drop_reason tcp_child_process(struct sock *parent, struct sock *child,
> >                     struct sk_buff *skb);
>
> Please fix indentation here,

Thanks for the check. I run the checkpatch locally everytime and it
only says it is just 'check' not 'warning' or even 'error'.

Have no idea whether I should 'fix' it.

>
>
> >  void tcp_enter_loss(struct sock *sk);
> >  void tcp_cwnd_reduction(struct sock *sk, int newly_acked_sacked, int newly_lost, int flag);
> > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> > index 0a944e109088..c79e25549972 100644
> > --- a/net/ipv4/tcp_ipv4.c
> > +++ b/net/ipv4/tcp_ipv4.c
> > @@ -1926,7 +1926,8 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
> >       } else
> >               sock_rps_save_rxhash(sk, skb);
> >
> > -     if (tcp_rcv_state_process(sk, skb)) {
> > +     reason = tcp_rcv_state_process(sk, skb);
> > +     if (reason) {
> >               rsk = sk;
> >               goto reset;
> >       }
> > diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> > index 9e85f2a0bddd..08d5b48540ea 100644
> > --- a/net/ipv4/tcp_minisocks.c
> > +++ b/net/ipv4/tcp_minisocks.c
> > @@ -911,11 +911,12 @@ EXPORT_SYMBOL(tcp_check_req);
> >   * be created.
> >   */
> >
> > -int tcp_child_process(struct sock *parent, struct sock *child,
> > +enum skb_drop_reason
> > +tcp_child_process(struct sock *parent, struct sock *child,
> >                     struct sk_buff *skb)
>
> and here.
>
>
> >       __releases(&((child)->sk_lock.slock))
> >  {
> > -     int ret = 0;
> > +     enum skb_drop_reason reason = SKB_NOT_DROPPED_YET;
> >       int state = child->sk_state;
> >
> >       /* record sk_napi_id and sk_rx_queue_mapping of child. */
> > @@ -923,7 +924,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
> >
> >       tcp_segs_in(tcp_sk(child), skb);
> >       if (!sock_owned_by_user(child)) {
> > -             ret = tcp_rcv_state_process(child, skb);
> > +             reason = tcp_rcv_state_process(child, skb);
> >               /* Wakeup parent, send SIGIO */
> >               if (state == TCP_SYN_RECV && child->sk_state != state)
> >                       parent->sk_data_ready(parent);
> > @@ -937,6 +938,6 @@ int tcp_child_process(struct sock *parent, struct sock *child,
> >
> >       bh_unlock_sock(child);
> >       sock_put(child);
> > -     return ret;
> > +     return reason;
> >  }
> >  EXPORT_SYMBOL(tcp_child_process);
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index 0c180bb8187f..4f8464e04b7f 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -1663,7 +1663,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
> >       } else
> >               sock_rps_save_rxhash(sk, skb);
> >
> > -     if (tcp_rcv_state_process(sk, skb))
> > +     reason = tcp_rcv_state_process(sk, skb);
> > +     if (reason)
> >               goto reset;
> >       if (opt_skb)
> >               goto ipv6_pktoptions;
> > --
> > 2.37.3
> >