netdev - RE: [PATCH net-next 01/13] net: enetc: add initial netc-lib driver to support NTMP

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] [day] [month] [year] [list]

Message-ID:
 <PAXPR04MB851069DEC17319C96837BE9B88102@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Mon, 6 Jan 2025 02:45:45 +0000
From: Wei Fang <wei.fang@....com>
To: Andrew Lunn <andrew@...n.ch>
CC: Claudiu Manoil <claudiu.manoil@....com>, Vladimir Oltean
	<vladimir.oltean@....com>, Clark Wang <xiaoning.wang@....com>,
	"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"christophe.leroy@...roup.eu" <christophe.leroy@...roup.eu>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "imx@...ts.linux.dev"
	<imx@...ts.linux.dev>
Subject: RE: [PATCH net-next 01/13] net: enetc: add initial netc-lib driver to
 support NTMP

> > +#define NTMP_FILL_CRD(crd, tblv, qa, ua) \
> > +({ \
> > +	typeof(crd) _crd = (crd); \
> > +	(_crd)->update_act = cpu_to_le16(ua); \
> > +	(_crd)->tblv_qact = NTMP_TBLV_QACT(tblv, qa); \
> > +})
> > +
> > +#define NTMP_FILL_CRD_EID(req, tblv, qa, ua, eid) \
> > +({ \
> > +	typeof(req) _req = (req); \
> > +	NTMP_FILL_CRD(&(_req)->crd, tblv, qa, ua); \
> > +	(_req)->entry_id = cpu_to_le32(eid); \
> > +})
> 
> 
> These are pretty complex for #defines. Can they be made into
> functions? That will get you type checking, finding bugs where
> parameters are swapped.

The problem is that different tables have different types of 'req'
parameters. Of course, since the headers of these request data
are the same, we can force these pointers to be converted to
common type pointers, but I think the forced conversion has made
this pointless.

> 
> > +int netc_setup_cbdr(struct device *dev, int cbd_num,
> > +		    struct netc_cbdr_regs *regs,
> > +		    struct netc_cbdr *cbdr)
> > +{
> > +	int size;
> > +
> > +	size = cbd_num * sizeof(union netc_cbd) + NTMP_BASE_ADDR_ALIGN;
> > +
> > +	cbdr->addr_base = dma_alloc_coherent(dev, size, &cbdr->dma_base,
> > +					     GFP_KERNEL);
> > +	if (!cbdr->addr_base)
> > +		return -ENOMEM;
> > +
> > +	cbdr->dma_size = size;
> > +	cbdr->bd_num = cbd_num;
> > +	cbdr->regs = *regs;
> > +
> > +	/* The base address of the Control BD Ring must be 128 bytes aligned */
> > +	cbdr->dma_base_align =  ALIGN(cbdr->dma_base,
> NTMP_BASE_ADDR_ALIGN);
> > +	cbdr->addr_base_align = PTR_ALIGN(cbdr->addr_base,
> > +					  NTMP_BASE_ADDR_ALIGN);
> > +
> > +	cbdr->next_to_clean = 0;
> > +	cbdr->next_to_use = 0;
> > +	spin_lock_init(&cbdr->ring_lock);
> > +
> > +	/* Step 1: Configure the base address of the Control BD Ring */
> > +	netc_write(cbdr->regs.bar0, lower_32_bits(cbdr->dma_base_align));
> > +	netc_write(cbdr->regs.bar1, upper_32_bits(cbdr->dma_base_align));
> > +
> > +	/* Step 2: Configure the producer index register */
> > +	netc_write(cbdr->regs.pir, cbdr->next_to_clean);
> > +
> > +	/* Step 3: Configure the consumer index register */
> > +	netc_write(cbdr->regs.cir, cbdr->next_to_use);
> > +
> > +	/* Step4: Configure the number of BDs of the Control BD Ring */
> > +	netc_write(cbdr->regs.lenr, cbdr->bd_num);
> > +
> > +	/* Step 5: Enable the Control BD Ring */
> > +	netc_write(cbdr->regs.mr, NETC_CBDR_MR_EN);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(netc_setup_cbdr);
> 
> I assume there is a version 3 in development, which will need a
> different library, or at least different symbols. Maybe you should
> think about the naming issues now?

I think that version 3 will not exist for a long time or will never exist.
The reason for developing NTMP 2.0 is that NTMP 1.0 cannot be
extended, so its table format is fixed, but for NTMP 2.0, its table
format supports extension. For the same table, multiple versions
can be expanded.

> 
> > diff --git a/include/linux/fsl/ntmp.h b/include/linux/fsl/ntmp.h
> > new file mode 100644
> > index 000000000000..7cf322a1c8e3
> > --- /dev/null
> > +++ b/include/linux/fsl/ntmp.h
> > @@ -0,0 +1,178 @@
> > +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
> > +/* Copyright 2025 NXP */
> > +#ifndef __NETC_NTMP_H
> > +#define __NETC_NTMP_H
> 
> Does this header need to be global? What else will use it outside of
> drivers/net/ethernet/freescale/enetc?
> 
Yes, this library will be used by NETC Switch driver, although there is
no NETC switch driver in the upstream yet, this is already in the plan.