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: <aBKReJUy2Z-JQwr4@mini-arch>
Date: Wed, 30 Apr 2025 14:09:12 -0700
From: Stanislav Fomichev <stfomichev@...il.com>
To: Jon Kohler <jon@...anix.com>
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
	Jason Wang <jasowang@...hat.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Jesper Dangaard Brouer <hawk@...nel.org>,
	John Fastabend <john.fastabend@...il.com>,
	Simon Horman <horms@...nel.org>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH net-next v2] xdp: Add helpers for head length, headroom,
 and metadata length

On 04/30, Jon Kohler wrote:
> Introduce new XDP helpers:
> - xdp_headlen: Similar to skb_headlen
> - xdp_headroom: Similar to skb_headroom
> - xdp_metadata_len: Similar to skb_metadata_len
> 
> Integrate these helpers into tap, tun, and XDP implementation to start.
> 
> No functional changes introduced.
> 
> Signed-off-by: Jon Kohler <jon@...anix.com>
> ---
> v1->v2: Integrate feedback from Willem
> https://patchwork.kernel.org/project/netdevbpf/patch/20250430182921.1704021-1-jon@nutanix.com/
> 
>  drivers/net/tap.c |  6 +++---
>  drivers/net/tun.c | 12 +++++------
>  include/net/xdp.h | 54 +++++++++++++++++++++++++++++++++++++++++++----
>  net/core/xdp.c    | 12 +++++------
>  4 files changed, 65 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index d4ece538f1b2..a62fbca4b08f 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -1048,7 +1048,7 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp)
>  	struct sk_buff *skb;
>  	int err, depth;
>  
> -	if (unlikely(xdp->data_end - xdp->data < ETH_HLEN)) {
> +	if (unlikely(xdp_headlen(xdp) < ETH_HLEN)) {
>  		err = -EINVAL;
>  		goto err;
>  	}
> @@ -1062,8 +1062,8 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp)
>  		goto err;
>  	}
>  
> -	skb_reserve(skb, xdp->data - xdp->data_hard_start);
> -	skb_put(skb, xdp->data_end - xdp->data);
> +	skb_reserve(skb, xdp_headroom(xdp));
> +	skb_put(skb, xdp_headlen(xdp));
>  
>  	skb_set_network_header(skb, ETH_HLEN);
>  	skb_reset_mac_header(skb);
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 7babd1e9a378..4c47eed71986 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1567,7 +1567,7 @@ static int tun_xdp_act(struct tun_struct *tun, struct bpf_prog *xdp_prog,
>  			dev_core_stats_rx_dropped_inc(tun->dev);
>  			return err;
>  		}
> -		dev_sw_netstats_rx_add(tun->dev, xdp->data_end - xdp->data);
> +		dev_sw_netstats_rx_add(tun->dev, xdp_headlen(xdp));
>  		break;
>  	case XDP_TX:
>  		err = tun_xdp_tx(tun->dev, xdp);
> @@ -1575,7 +1575,7 @@ static int tun_xdp_act(struct tun_struct *tun, struct bpf_prog *xdp_prog,
>  			dev_core_stats_rx_dropped_inc(tun->dev);
>  			return err;
>  		}
> -		dev_sw_netstats_rx_add(tun->dev, xdp->data_end - xdp->data);
> +		dev_sw_netstats_rx_add(tun->dev, xdp_headlen(xdp));
>  		break;
>  	case XDP_PASS:
>  		break;
> @@ -2355,7 +2355,7 @@ static int tun_xdp_one(struct tun_struct *tun,
>  		       struct xdp_buff *xdp, int *flush,
>  		       struct tun_page *tpage)
>  {
> -	unsigned int datasize = xdp->data_end - xdp->data;
> +	unsigned int datasize = xdp_headlen(xdp);
>  	struct tun_xdp_hdr *hdr = xdp->data_hard_start;
>  	struct virtio_net_hdr *gso = &hdr->gso;
>  	struct bpf_prog *xdp_prog;
> @@ -2415,14 +2415,14 @@ static int tun_xdp_one(struct tun_struct *tun,
>  		goto out;
>  	}
>  
> -	skb_reserve(skb, xdp->data - xdp->data_hard_start);
> -	skb_put(skb, xdp->data_end - xdp->data);
> +	skb_reserve(skb, xdp_headroom(xdp));
> +	skb_put(skb, xdp_headlen(xdp));
>  
>  	/* The externally provided xdp_buff may have no metadata support, which
>  	 * is marked by xdp->data_meta being xdp->data + 1. This will lead to a
>  	 * metasize of -1 and is the reason why the condition checks for > 0.
>  	 */
> -	metasize = xdp->data - xdp->data_meta;
> +	metasize = xdp_metadata_len(xdp);
>  	if (metasize > 0)
>  		skb_metadata_set(skb, metasize);
>  
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 48efacbaa35d..044345b18305 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -151,10 +151,56 @@ xdp_get_shared_info_from_buff(const struct xdp_buff *xdp)
>  	return (struct skb_shared_info *)xdp_data_hard_end(xdp);
>  }
>  
> +/**
> + * xdp_headlen - Calculate the length of the data in an XDP buffer
> + * @xdp: Pointer to the XDP buffer structure
> + *
> + * Compute the length of the data contained in the XDP buffer. Does not
> + * include frags, use xdp_get_buff_len() for that instead.
> + *
> + * Analogous to skb_headlen().
> + *
> + * Return: The length of the data in the XDP buffer in bytes.
> + */
> +static inline unsigned int xdp_headlen(const struct xdp_buff *xdp)
> +{
> +	return xdp->data_end - xdp->data;
> +}
> +
> +/**
> + * xdp_headroom - Calculate the headroom available in an XDP buffer
> + * @xdp: Pointer to the XDP buffer structure
> + *
> + * Compute the headroom in an XDP buffer.
> + *
> + * Analogous to the skb_headroom().
> + *
> + * Return: The size of the headroom in bytes.
> + */
> +static inline unsigned int xdp_headroom(const struct xdp_buff *xdp)
> +{
> +	return xdp->data - xdp->data_hard_start;
> +}
> +
> +/**
> + * xdp_metadata_len - Calculate the length of metadata in an XDP buffer
> + * @xdp: Pointer to the XDP buffer structure
> + *
> + * Compute the length of the metadata region in an XDP buffer.
> + *
> + * Analogous to skb_metadata_len().
> + *
> + * Return: The length of the metadata in bytes.
> + */
> +static inline unsigned int xdp_metadata_len(const struct xdp_buff *xdp)

I believe this has to return int, not unsigned int. There are places
where we do data_meta = data + 1, and the callers check whether
the result is signed or sunsigned.

> +{
> +	return xdp->data - xdp->data_meta;
> +}
> +
>  static __always_inline unsigned int
>  xdp_get_buff_len(const struct xdp_buff *xdp)
>  {
> -	unsigned int len = xdp->data_end - xdp->data;
> +	unsigned int len = xdp_headlen(xdp);
>  	const struct skb_shared_info *sinfo;
>  
>  	if (likely(!xdp_buff_has_frags(xdp)))
> @@ -364,8 +410,8 @@ int xdp_update_frame_from_buff(const struct xdp_buff *xdp,
>  	int metasize, headroom;
>  
>  	/* Assure headroom is available for storing info */
> -	headroom = xdp->data - xdp->data_hard_start;
> -	metasize = xdp->data - xdp->data_meta;
> +	headroom = xdp_headroom(xdp);
> +	metasize = xdp_metadata_len(xdp);
>  	metasize = metasize > 0 ? metasize : 0;

^^ like here

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ