netdev - Re: [RFC PATCH net-next v2 2/2] net: veth: Improving page pool pages recycling

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: <dd263b2b-4030-f274-7fe8-7ba751f04ab6@huawei.com>
Date: Wed, 2 Aug 2023 20:32:03 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Liang Chen <liangchen.linux@...il.com>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>
CC: <hawk@...nel.org>, <ilias.apalodimas@...aro.org>, <daniel@...earbox.net>,
	<ast@...nel.org>, <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH net-next v2 2/2] net: veth: Improving page pool pages
 recycling

On 2023/8/1 14:19, Liang Chen wrote:

> @@ -862,9 +865,18 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
>  	case XDP_PASS:
>  		break;
>  	case XDP_TX:
> -		veth_xdp_get(xdp);
> -		consume_skb(skb);
> -		xdp->rxq->mem = rq->xdp_mem;
> +		if (skb != skb_orig) {
> +			xdp->rxq->mem = rq->xdp_mem_pp;
> +			kfree_skb_partial(skb, true);

For this case, I suppose that we can safely call kfree_skb_partial()
as we allocate the skb in the veth_convert_skb_to_xdp_buff(), but
I am not sure about the !skb->pp_recycle case.

> +		} else if (!skb->pp_recycle) {
> +			xdp->rxq->mem = rq->xdp_mem;
> +			kfree_skb_partial(skb, true);

For consume_skb(), there is skb_unref() checking and other checking/operation.
Can we really assume that we can call kfree_skb_partial() with head_stolen
being true? Is it possible that skb->users is bigger than 1? If it is possible,
don't we free the 'skb' back to skbuff_cache when other may still be using
it?

> +		} else {
> +			veth_xdp_get(xdp);
> +			consume_skb(skb);
> +			xdp->rxq->mem = rq->xdp_mem;
> +		}
> +