RE: [PATCH 1/2][net-next] skbuff: move netlink_large_alloc_large_skb() to skbuff.c

["Li,Rongqing" <lirongqing@baidu.com>]

> -----Original Message-----
> From: Yunsheng Lin <linyunsheng@huawei.com>
> Sent: Thursday, November 2, 2023 7:02 PM
> To: Li,Rongqing <lirongqing@baidu.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH 1/2][net-next] skbuff: move netlink_large_alloc_large_skb()
> to skbuff.c
> 
> On 2023/11/2 14:28, Li RongQing wrote:
> > move netlink_alloc_large_skb and netlink_skb_destructor to skbuff.c
> > and rename them more generic, so they can be used elsewhere large
> > non-contiguous physical memory is needed
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >  include/linux/skbuff.h   |  3 +++
> >  net/core/skbuff.c        | 40
> ++++++++++++++++++++++++++++++++++++++++
> >  net/netlink/af_netlink.c | 41
> > ++---------------------------------------
> >  3 files changed, 45 insertions(+), 39 deletions(-)
> >
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index
> > 4174c4b..774a401 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -5063,5 +5063,8 @@ static inline void skb_mark_for_recycle(struct
> > sk_buff *skb)  ssize_t skb_splice_from_iter(struct sk_buff *skb, struct iov_iter
> *iter,
> >  			     ssize_t maxsize, gfp_t gfp);
> >
> > +
> > +void large_skb_destructor(struct sk_buff *skb); struct sk_buff
> > +*alloc_large_skb(unsigned int size, int broadcast);
> >  #endif	/* __KERNEL__ */
> >  #endif	/* _LINUX_SKBUFF_H */
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c index
> > 4570705..20ffcd5 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -6917,3 +6917,43 @@ ssize_t skb_splice_from_iter(struct sk_buff *skb,
> struct iov_iter *iter,
> >  	return spliced ?: ret;
> >  }
> >  EXPORT_SYMBOL(skb_splice_from_iter);
> > +
> > +void large_skb_destructor(struct sk_buff *skb) {
> > +	if (is_vmalloc_addr(skb->head)) {
> > +		if (!skb->cloned ||
> > +		    !atomic_dec_return(&(skb_shinfo(skb)->dataref)))
> > +			vfree(skb->head);
> > +
> > +		skb->head = NULL;
> 
> There seems to be an assumption that skb returned from
> netlink_alloc_large_skb() is not expecting the frag page for shinfo->frags*, as the
> above NULL setting will bypass most of the handling in skb_release_data(),then
> how can we ensure that the user is not breaking the assumption if we make it
> more generic?
> 

How about to add WARN_ON(skb_shinfo(skb)-> nr_frags) to find this condition


-Li RongQing
>