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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240223194445.7537-1-kuniyu@amazon.com>
Date: Fri, 23 Feb 2024 11:44:45 -0800
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <kerneljasonxing@...il.com>
CC: <davem@...emloft.net>, <dsahern@...nel.org>, <edumazet@...gle.com>,
	<kernelxing@...cent.com>, <kuba@...nel.org>, <kuniyu@...zon.com>,
	<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()

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,


>  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
>