Re: [PATCH] release oob skb when received in-band and fix setsockopt ret

Brandon Ho <[email protected]> writes:

> Signed-off-by: Brandon Ho <[email protected]>
> ---
>  kernel/evl/net/skb.c | 3 +++
>  net/core/skbuff.c    | 2 ++
>  net/core/sock.c      | 1 +
>  3 files changed, 6 insertions(+)
>
> diff --git a/kernel/evl/net/skb.c b/kernel/evl/net/skb.c
> index 50090c181..9cd52e3e7 100644
> --- a/kernel/evl/net/skb.c
> +++ b/kernel/evl/net/skb.c
> @@ -55,6 +55,9 @@
>   *                                                -> free_skb_oob(skb)
>   *                        -> dev_kfree_skb(skb)
>   *                                -> consume_skb(skb) [1]
> + * 			   |
> + * 			     __consume_stateless_skb(skb)
> + * 						  -> free_skb_oob(skb)
>   */
>  
>  #define SKB_RECYCLING_THRESHOLD 32
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index e532fd636..b4dc5e1c9 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -1621,6 +1621,8 @@ EXPORT_SYMBOL(consume_skb);
>  void __consume_stateless_skb(struct sk_buff *skb)
>  {
>  	trace_consume_skb(skb, __builtin_return_address(0));
> +	if (skb_release_oob(skb))
> +		return;
>  	skb_release_data(skb, SKB_CONSUMED);
>  	kfree_skbmem(skb);
>  }

__consume_stateless_skb() should never be called from the oob stage, or
receive any oob-managed skb, which makes the redirection via
skb_release_oob() pointless. Did you identify any code path which would
hand over oob skbs to skb_consume_udp() for release?

> diff --git a/net/core/sock.c b/net/core/sock.c
> index 3321ff386..70a313c1f 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1721,6 +1721,7 @@ int sk_getsockopt(struct sock *sk, int level, int optname,
>  			goto lenout;
>  		if (ret != -ENOIOCTLCMD)
>  			return ret;
> +		ret = 0;
>  	}
>  
>  	memset(&v, 0, sizeof(v));

We don't need this, all following code paths either assign this variable
prior to using it, or simply return an immediate value. The original
code does not initialize this variable on entry, which confirms the
latter.

PS: on a general note, please split patches addressing unrelated topics.

-- 
Philippe.