netdev - Re: [net-next PATCH 13/15] eth: fbnic: add basic Rx handling

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: <41a39896-480b-f08d-ba67-17e129e39c0f@huawei.com>
Date: Tue, 9 Apr 2024 19:47:28 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Alexander Duyck <alexander.duyck@...il.com>, <netdev@...r.kernel.org>
CC: Alexander Duyck <alexanderduyck@...com>, <kuba@...nel.org>,
	<davem@...emloft.net>, <pabeni@...hat.com>
Subject: Re: [net-next PATCH 13/15] eth: fbnic: add basic Rx handling

On 2024/4/4 4:09, Alexander Duyck wrote:
> From: Alexander Duyck <alexanderduyck@...com>

...

> +
> +static int fbnic_clean_rcq(struct fbnic_napi_vector *nv,
> +			   struct fbnic_q_triad *qt, int budget)
> +{
> +	struct fbnic_ring *rcq = &qt->cmpl;
> +	struct fbnic_pkt_buff *pkt;
> +	s32 head0 = -1, head1 = -1;
> +	__le64 *raw_rcd, done;
> +	u32 head = rcq->head;
> +	u64 packets = 0;
> +
> +	done = (head & (rcq->size_mask + 1)) ? cpu_to_le64(FBNIC_RCD_DONE) : 0;
> +	raw_rcd = &rcq->desc[head & rcq->size_mask];
> +	pkt = rcq->pkt;
> +
> +	/* Walk the completion queue collecting the heads reported by NIC */
> +	while (likely(packets < budget)) {
> +		struct sk_buff *skb = ERR_PTR(-EINVAL);
> +		u64 rcd;
> +
> +		if ((*raw_rcd & cpu_to_le64(FBNIC_RCD_DONE)) == done)
> +			break;
> +
> +		dma_rmb();
> +
> +		rcd = le64_to_cpu(*raw_rcd);
> +
> +		switch (FIELD_GET(FBNIC_RCD_TYPE_MASK, rcd)) {
> +		case FBNIC_RCD_TYPE_HDR_AL:
> +			head0 = FIELD_GET(FBNIC_RCD_AL_BUFF_ID_MASK, rcd);
> +			fbnic_pkt_prepare(nv, rcd, pkt, qt);
> +
> +			break;
> +		case FBNIC_RCD_TYPE_PAY_AL:
> +			head1 = FIELD_GET(FBNIC_RCD_AL_BUFF_ID_MASK, rcd);
> +			fbnic_add_rx_frag(nv, rcd, pkt, qt);
> +
> +			break;
> +		case FBNIC_RCD_TYPE_OPT_META:
> +			/* Only type 0 is currently supported */
> +			if (FIELD_GET(FBNIC_RCD_OPT_META_TYPE_MASK, rcd))
> +				break;
> +
> +			/* We currently ignore the action table index */
> +			break;
> +		case FBNIC_RCD_TYPE_META:
> +			if (likely(!fbnic_rcd_metadata_err(rcd)))
> +				skb = fbnic_build_skb(nv, pkt);
> +
> +			/* populate skb and invalidate XDP */
> +			if (!IS_ERR_OR_NULL(skb)) {
> +				fbnic_populate_skb_fields(nv, rcd, skb, qt);
> +
> +				packets++;
> +
> +				napi_gro_receive(&nv->napi, skb);
> +			}
> +
> +			pkt->buff.data_hard_start = NULL;
> +
> +			break;
> +		}
> +
> +		raw_rcd++;
> +		head++;
> +		if (!(head & rcq->size_mask)) {
> +			done ^= cpu_to_le64(FBNIC_RCD_DONE);
> +			raw_rcd = &rcq->desc[0];
> +		}
> +	}
> +
> +	/* Unmap and free processed buffers */
> +	if (head0 >= 0)
> +		fbnic_clean_bdq(nv, budget, &qt->sub0, head0);
> +	fbnic_fill_bdq(nv, &qt->sub0);
> +
> +	if (head1 >= 0)
> +		fbnic_clean_bdq(nv, budget, &qt->sub1, head1);
> +	fbnic_fill_bdq(nv, &qt->sub1);

I am not sure how complicated the rx handling will be for the advanced
feature. For the current code, for each entry/desc in both qt->sub0 and
qt->sub1 at least need one page, and the page seems to be only used once
no matter however small the page is used?

I am assuming you want to do 'tightly optimized' operation for this by
calling page_pool_fragment_page(), but manipulating page->pp_ref_count
directly does not seems to add any value for the current code, but seem
to waste a lot of memory by not using the frag API, especially PAGE_SIZE
> 4K?

> +
> +	/* Record the current head/tail of the queue */
> +	if (rcq->head != head) {
> +		rcq->head = head;
> +		writel(head & rcq->size_mask, rcq->doorbell);
> +	}
> +
> +	return packets;
> +}
>  
>