Re: [PATCH net-next] tun: use pskb_may_pull_reason() and remove SKB_DROP_REASON_HDR_TRUNC

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Anton Danilov wrote:
> tun_get_user() is the only user of SKB_DROP_REASON_HDR_TRUNC. It sets
> it when pskb_may_pull() fails to pull the Ethernet header of a frame
> written to a TAP device.
> 
> pskb_may_pull_reason() reports why the pull failed: PKT_TOO_SMALL if
> the packet is shorter than requested, NOMEM if pulling the data into
> the linear area fails. Use it for the Ethernet header pull in
> tun_get_user() and remove the now unused SKB_DROP_REASON_HDR_TRUNC.
> 
> After a successful pull drop_reason is SKB_NOT_DROPPED_YET. The path
> that later finds the queue detached has no reason of its own and relied
> on the SKB_DROP_REASON_NOT_SPECIFIED initializer. Fall back to
> SKB_DROP_REASON_NOT_SPECIFIED at the free_skb label, as vxlan_rcv()
> does, so that this path, and any later one without a reason of its own,
> keeps reporting it.
> 
> The error returned to user space does not change.
> 
> Suggested-by: Ido Schimmel <idosch@nvidia.com>
> Link: https://lore.kernel.org/netdev/20260923145931.GA2859441@shredder/
> Assisted-by: LLM
> Signed-off-by: Anton Danilov <littlesmilingcloud@gmail.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

> 
> ---
>  drivers/net/tun.c             | 5 +++--
>  include/net/dropreason-core.h | 7 -------
>  2 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 5a302709a68a..758956f93979 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1932,9 +1932,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  		skb->dev = tun->dev;
>  		break;
>  	case IFF_TAP:
> -		if (!pskb_may_pull(skb, ETH_HLEN)) {
> +		drop_reason = pskb_may_pull_reason(skb, ETH_HLEN);

Should we update the skb_may_pull in the above IFF_TUN similarly?

> +		if (drop_reason) {
>  			err = -ENOMEM;
> -			drop_reason = SKB_DROP_REASON_HDR_TRUNC;
>  			goto drop;
>  		}
>  		skb->protocol = eth_type_trans(skb, tun->dev);
> @@ -2062,6 +2062,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  		dev_core_stats_rx_dropped_inc(tun->dev);
>  
>  free_skb:
> +	drop_reason = drop_reason ?: SKB_DROP_REASON_NOT_SPECIFIED;
>  	if (!IS_ERR_OR_NULL(skb))
>  		kfree_skb_reason(skb, drop_reason);
>  
> diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
> index 12f909651591..47b44a29daab 100644
> --- a/include/net/dropreason-core.h
> +++ b/include/net/dropreason-core.h
> @@ -82,7 +82,6 @@
>  	FN(DEV_READY)			\
>  	FN(FULL_RING)			\
>  	FN(NOMEM)			\
> -	FN(HDR_TRUNC)			\
>  	FN(TAP_FILTER)			\
>  	FN(TAP_TXFILTER)		\
>  	FN(ICMP_CSUM)			\
> @@ -427,12 +426,6 @@ enum skb_drop_reason {
>  	SKB_DROP_REASON_FULL_RING,
>  	/** @SKB_DROP_REASON_NOMEM: error due to OOM */
>  	SKB_DROP_REASON_NOMEM,
> -	/**
> -	 * @SKB_DROP_REASON_HDR_TRUNC: failed to trunc/extract the header from
> -	 * networking data, e.g., failed to pull the protocol header from
> -	 * frags via pskb_may_pull()
> -	 */
> -	SKB_DROP_REASON_HDR_TRUNC,
>  	/**
>  	 * @SKB_DROP_REASON_TAP_FILTER: dropped by (ebpf) filter directly attached
>  	 * to tun/tap, e.g., via TUNSETFILTEREBPF
> -- 
> 2.47.3
>