netdev - Re: [PATCH net-next v6 2/2] net: add netmem to skb_frag

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: <cff078e234e94593fb3fcfce9732d7988ead42d3.camel@redhat.com>
Date: Tue, 30 Jan 2024 10:33:56 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Mina Almasry <almasrymina@...gle.com>, linux-kernel@...r.kernel.org, 
	netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>,  Jakub Kicinski <kuba@...nel.org>, Jason Gunthorpe
 <jgg@...dia.com>, Christian König
 <christian.koenig@....com>, Shakeel Butt <shakeelb@...gle.com>, Yunsheng
 Lin <linyunsheng@...wei.com>, Willem de Bruijn
 <willemdebruijn.kernel@...il.com>
Subject: Re: [PATCH net-next v6 2/2] net: add netmem to skb_frag_t

Hi,

I'm sorry for the late feedback.

On Tue, 2024-01-23 at 14:17 -0800, Mina Almasry wrote:
> @@ -845,16 +863,24 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
>  }
>  EXPORT_SYMBOL(__napi_alloc_skb);
>  
> -void skb_add_rx_frag(struct sk_buff *skb, int i, struct page *page, int off,
> -		     int size, unsigned int truesize)
> +void skb_add_rx_frag_netmem(struct sk_buff *skb, int i, netmem_ref netmem,
> +			    int off, int size, unsigned int truesize)
>  {
>  	DEBUG_NET_WARN_ON_ONCE(size > truesize);
>  
> -	skb_fill_page_desc(skb, i, page, off, size);
> +	skb_fill_netmem_desc(skb, i, netmem, off, size);
>  	skb->len += size;
>  	skb->data_len += size;
>  	skb->truesize += truesize;
>  }
> +EXPORT_SYMBOL(skb_add_rx_frag_netmem);
> +
> +void skb_add_rx_frag(struct sk_buff *skb, int i, struct page *page, int off,
> +		     int size, unsigned int truesize)
> +{
> +	skb_add_rx_frag_netmem(skb, i, page_to_netmem(page), off, size,
> +			       truesize);
> +}
>  EXPORT_SYMBOL(skb_add_rx_frag);

Out of sheer ignorance, I'm unsure if the compiler will always inline
the above skb_add_rx_frag_netmem() call. What about moving this helper
to the header file?

> diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
> index 1184d40167b8..145ef22b2b35 100644
> --- a/net/kcm/kcmsock.c
> +++ b/net/kcm/kcmsock.c
> @@ -636,9 +636,14 @@ static int kcm_write_msgs(struct kcm_sock *kcm)
>  		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
>  			msize += skb_frag_size(&skb_shinfo(skb)->frags[i]);
>  
> +		if (WARN_ON_ONCE(!skb_frag_page(&skb_shinfo(skb)->frags[0]))) {
> +			ret = -EINVAL;
> +			goto out;
> +		}

I feel like the following has been already discussed, but I could not
find the relevant reference... Are all frags constrained to carry the
same memref type? If not it would be better to move this check inside
the previous loop, it's already traversing all the skb frags, it should
not add measurable overhead.

Thanks!

Paolo