netdev - Re: [PATCH net-next v9 13/14] xsc: Add eth reception data path

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: <20250326102737.GB892515@horms.kernel.org>
Date: Wed, 26 Mar 2025 10:27:37 +0000
From: Simon Horman <horms@...nel.org>
To: Xin Tian <tianx@...silicon.com>
Cc: netdev@...r.kernel.org, leon@...nel.org, andrew+netdev@...n.ch,
	kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com,
	davem@...emloft.net, jeff.johnson@....qualcomm.com,
	przemyslaw.kitszel@...el.com, weihg@...silicon.com,
	wanry@...silicon.com, jacky@...silicon.com,
	parthiban.veerasooran@...rochip.com, masahiroy@...nel.org,
	kalesh-anakkur.purayil@...adcom.com, geert+renesas@...der.be,
	geert@...ux-m68k.org
Subject: Re: [PATCH net-next v9 13/14] xsc: Add eth reception data path

On Tue, Mar 18, 2025 at 11:15:21PM +0800, Xin Tian wrote:

...

> diff --git a/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_common.h b/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_common.h

...

> +#define XSC_SET_PFLAG(params, pflag, enable)			\
> +	do {							\
> +		if (enable)					\
> +			(params)->pflags |= BIT(pflag);		\
> +		else						\
> +			(params)->pflags &= ~(BIT(pflag));	\
> +	} while (0)

Hi Xin Tian,

XSC_SET_PFLAG() seems to be unused. Perhaps it is best to drop it and
add it when needed.

And, FWIIW, I would have implemented both XSC_SET_PFLAG() and
XSC_GET_PFLAG() as functions as there doesn't seem to be a reason that they
need to be macros.

> +
> +#define XSC_GET_PFLAG(params, pflag) (!!((params)->pflags & (BIT(pflag))))

...

> diff --git a/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_rx.c b/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_rx.c

...

> +bool xsc_eth_post_rx_wqes(struct xsc_rq *rq)
> +{
> +	struct xsc_wq_cyc *wq = &rq->wqe.wq;
> +	u8 wqe_bulk, wqe_bulk_min;
> +	int alloc;
> +	u16 head;
> +	int err;
> +
> +	wqe_bulk = rq->wqe.info.wqe_bulk;
> +	wqe_bulk_min = rq->wqe.info.wqe_bulk_min;
> +	if (xsc_wq_cyc_missing(wq) < wqe_bulk)
> +		return false;
> +
> +	do {
> +		head = xsc_wq_cyc_get_head(wq);
> +
> +		alloc = min_t(int, wqe_bulk, xsc_wq_cyc_missing(wq));
> +		if (alloc < wqe_bulk && alloc >= wqe_bulk_min)
> +			alloc = alloc & 0xfffffffe;
> +
> +		if (alloc > 0) {
> +			err = xsc_alloc_rx_wqes(rq, head, alloc);
> +			if (unlikely(err))
> +				break;
> +
> +			xsc_wq_cyc_push_n(wq, alloc);
> +		}
> +	} while (xsc_wq_cyc_missing(wq) >= wqe_bulk_min);
> +
> +	dma_wmb();
> +
> +	/* ensure wqes are visible to device before updating doorbell record */
> +	xsc_rq_notify_hw(rq);
> +
> +	return !!err;

Perhaps it can't occur in practice, but err will be used uninitialised here
if the alloc condition in the do loop above is never met.

> +}

...