[PATCH 0/7] net: replace global NetRxPackets arrays with per-interface allocation

[PATCH 0/7] net: replace global NetRxPackets arrays with per-interface allocation

[Ahmad Fatoum]

Drivers using NetRxPackets can't be combined into the same barebox build
without clobbering each other when receiving packets. Even having the
same driver twice would lead to issues.

We certainly don't want to encourage new network drivers to use this
global array, so let's just get rid of it.

Ahmad Fatoum (7):
  net: add net_alloc_packets helper
  net: ep93xx: replace global NetRxPackets with per-interface allocation
  net: enc28j60: replace global NetRxPackets with per-interface
    allocation
  net: gianfar: replace global NetRxPackets with per-interface
    allocation
  net: ethoc: replace global NetRxPackets with per-interface allocation
  net: cpsw: replace global NetRxPackets with per-interface allocation
  net: retire global NetRxPackets arrays

 drivers/net/cpsw.c     |  8 +++++++-
 drivers/net/enc28j60.c |  6 ++++--
 drivers/net/ep93xx.c   | 16 +++++++++++++---
 drivers/net/ethoc.c    | 10 +++++++++-
 drivers/net/gianfar.c  | 14 +++++++-------
 drivers/net/gianfar.h  |  6 ++++++
 include/net.h          | 10 ++++++++--
 net/net.c              | 26 ++++++++++++++++++++++----
 8 files changed, 76 insertions(+), 20 deletions(-)

-- 
2.39.2

[PATCH 1/7] net: add net_alloc_packets helper

[Ahmad Fatoum]

We have a number of drivers that call net_alloc_packet in a loop and
will gain some more in the quest to drop NetRxPackets.
Let's provide a helper that can be used for this and a function to free
the packets as well.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/net.h |  8 ++++++++
 net/net.c     | 27 ++++++++++++++++++++++++---
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/include/net.h b/include/net.h
index ffc1093ae6e7..38fd6f8d3c41 100644
--- a/include/net.h
+++ b/include/net.h
@@ -551,6 +551,14 @@ static inline char *net_alloc_packet(void)
 	return dma_alloc(PKTSIZE);
 }
 
+static inline void net_free_packet(char *pkt)
+{
+	return dma_free(pkt);
+}
+
+int net_alloc_packets(void **packets, int count);
+void net_free_packets(void **packets, unsigned count);
+
 struct net_connection *net_udp_new(IPaddr_t dest, uint16_t dport,
 		rx_handler_f *handler, void *ctx);
 
diff --git a/net/net.c b/net/net.c
index d72ed81e10a0..6f418df0538b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -789,12 +789,33 @@ int net_receive(struct eth_device *edev, unsigned char *pkt, int len)
 	return ret;
 }
 
-static int net_init(void)
+void net_free_packets(void **packets, unsigned count)
 {
+	while (count-- > 0)
+		net_free_packet(packets[count]);
+}
+
+int net_alloc_packets(void **packets, int count)
+{
+	void *packet;
 	int i;
 
-	for (i = 0; i < PKTBUFSRX; i++)
-		NetRxPackets[i] = net_alloc_packet();
+	for (i = 0; i < count; i++) {
+		packet = net_alloc_packet();
+		if (!packet)
+			goto free;
+		packets[i] = packet;
+	}
+
+	return 0;
+free:
+	net_free_packets(packets, i);
+	return -ENOMEM;
+}
+
+static int net_init(void)
+{
+	net_alloc_packets((void **)NetRxPackets, PKTBUFSRX);
 
 	globalvar_add_simple_ip("net.nameserver", &net_nameserver);
 	globalvar_add_simple_string("net.domainname", &net_domainname);
-- 
2.39.2

[PATCH 2/7] net: ep93xx: replace global NetRxPackets with per-interface allocation

[Ahmad Fatoum]

NetRxPackets is a remnant of times, where a board had at most one
Ethernet controller. This is outdated and we should drop NetRxPackets.

The driver already had an unused rx_buffer, so let's make use of it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/net/ep93xx.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ep93xx.c b/drivers/net/ep93xx.c
index 77f8aa63db76..bd954e7a17b0 100644
--- a/drivers/net/ep93xx.c
+++ b/drivers/net/ep93xx.c
@@ -62,7 +62,7 @@ static void dump_dev(struct eth_device *edev)
 	printf("  rx_sq.end	     %p\n", priv->rx_sq.end);
 
 	for (i = 0; i < NUMRXDESC; i++)
-		printf("  rx_buffer[%2.d]      %p\n", i, NetRxPackets[i]);
+		printf("  rx_buffer[%2.d]      %p\n", i, priv->rx_buffer[i]);
 
 	printf("  tx_dq.base	     %p\n", priv->tx_dq.base);
 	printf("  tx_dq.current	     %p\n", priv->tx_dq.current);
@@ -258,7 +258,7 @@ static int ep93xx_eth_open(struct eth_device *edev)
 	 */
 	for (i = 0; i < NUMRXDESC; i++) {
 		/* set buffer address */
-		(priv->rx_dq.base + i)->word1 = (uint32_t)NetRxPackets[i];
+		(priv->rx_dq.base + i)->word1 = (uint32_t)priv->rx_buffer[i];
 
 		/* set buffer length, clear buffer index and NSOF */
 		(priv->rx_dq.base + i)->word2 = EP93XX_MAX_PKT_SIZE;
@@ -324,7 +324,7 @@ static int ep93xx_eth_rcv_packet(struct eth_device *edev)
 			/*
 			 * We have a good frame. Extract the frame's length
 			 * from the current rx_status_queue entry, and copy
-			 * the frame's data into NetRxPackets[] of the
+			 * the frame's data into priv->rx_buffer of the
 			 * protocol stack. We track the total number of
 			 * bytes in the frame (nbytes_frame) which will be
 			 * used when we pass the data off to the protocol
@@ -532,6 +532,12 @@ static int ep93xx_eth_probe(struct device *dev)
 		goto eth_probe_failed_3;
 	}
 
+	ret = net_alloc_packets(priv->rx_buffer, NUMRXDESC);
+	if (ret) {
+		pr_err("net_alloc_packet() failed: rx_buffer");
+		goto eth_probe_failed_4;
+	}
+
 	mdiobus_register(&priv->miibus);
 	eth_register(edev);
 
@@ -539,6 +545,10 @@ static int ep93xx_eth_probe(struct device *dev)
 
 	goto eth_probe_done;
 
+eth_probe_failed_4:
+	free(priv->rx_sq.base);
+	/* Fall through */
+
 eth_probe_failed_3:
 	free(priv->rx_dq.base);
 	/* Fall through */
-- 
2.39.2

[PATCH 3/7] net: enc28j60: replace global NetRxPackets with per-interface allocation

[Ahmad Fatoum]

NetRxPackets is a remnant of times, where a board had at most one
Ethernet controller. This is outdated and we should drop NetRxPackets.

The driver doesn't maintain a queue of packets, so let's just allocate
it per interface.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/net/enc28j60.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/enc28j60.c b/drivers/net/enc28j60.c
index 51556ae01366..9455c6f5ea99 100644
--- a/drivers/net/enc28j60.c
+++ b/drivers/net/enc28j60.c
@@ -46,6 +46,7 @@ struct enc28j60_net {
 	/* store MAC address here while hardware is in the reset state */
 	u8 hwaddr[ETH_ALEN];
 	struct mii_bus miibus;
+	void *rx_buffer;
 };
 
 /*
@@ -793,9 +794,9 @@ static void enc28j60_hw_rx(struct eth_device *edev)
 		/* copy the packet from the receive buffer */
 		enc28j60_mem_read(priv,
 			rx_packet_start(priv->next_pk_ptr),
-			len, NetRxPackets[0]);
+			len, priv->rx_buffer);
 
-		net_receive(edev, NetRxPackets[0], len);
+		net_receive(edev, priv->rx_buffer, len);
 	}
 
 	/*
@@ -931,6 +932,7 @@ static int enc28j60_probe(struct device *dev)
 	priv = xzalloc(sizeof(*priv));
 
 	priv->spi = (struct spi_device *)dev->type_data;
+	priv->rx_buffer = net_alloc_packet();
 
 	edev = &priv->edev;
 	edev->priv = priv;
-- 
2.39.2

[PATCH 4/7] net: gianfar: replace global NetRxPackets with per-interface allocation

[Ahmad Fatoum]

NetRxPackets is a remnant of times, where a board had at most one
Ethernet controller. This is outdated and we should drop NetRxPackets.

Switch over the driver to allocate the receive buffers needed.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/net/gianfar.c | 14 +++++++-------
 drivers/net/gianfar.h |  6 ++++++
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 1a07059db4f3..21ffe822e1cc 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -21,11 +21,6 @@
 #include <linux/err.h>
 #include "gianfar.h"
 
-/* 2 seems to be the minimum number of TX descriptors to make it work. */
-#define TX_BUF_CNT 	2
-#define RX_BUF_CNT 	PKTBUFSRX
-#define BUF_ALIGN	8
-
 /*
  * Initialize required registers to appropriate values, zeroing
  * those we don't care about (unless zero is bad, in which case,
@@ -199,7 +194,7 @@ static int gfar_open(struct eth_device *edev)
 	for (ix = 0; ix < RX_BUF_CNT; ix++) {
 		out_be16(&priv->rxbd[ix].status, RXBD_EMPTY);
 		out_be16(&priv->rxbd[ix].length, 0);
-		out_be32(&priv->rxbd[ix].bufPtr, (uint) NetRxPackets[ix]);
+		out_be32(&priv->rxbd[ix].bufPtr, (uint) priv->rx_buffer[ix]);
 	}
 	out_be16(&priv->rxbd[RX_BUF_CNT - 1].status, RXBD_EMPTY | RXBD_WRAP);
 
@@ -407,7 +402,7 @@ static int gfar_recv(struct eth_device *edev)
 	/* Send the packet up if there were no errors */
 	status = in_be16(&priv->rxbd[priv->rxidx].status);
 	if (!(status & RXBD_STATS))
-		net_receive(edev, NetRxPackets[priv->rxidx], length - 4);
+		net_receive(edev, priv->rx_buffer[priv->rxidx], length - 4);
 	else
 		dev_err(dev, "Got error %x\n", status & RXBD_STATS);
 
@@ -471,9 +466,14 @@ static int gfar_probe(struct device *dev)
 	size_t size;
 	char devname[16];
 	char *p;
+	int ret;
 
 	priv = xzalloc(sizeof(struct gfar_private));
 
+	ret = net_alloc_packets(priv->rx_buffer, ARRAY_SIZE(priv->rx_buffer));
+	if (ret)
+		return ret;
+
 	edev = &priv->edev;
 
 	priv->mdiobus_tbi = gfar_info->mdiobus_tbi;
diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h
index cea41218a71c..8a60c7f38eca 100644
--- a/drivers/net/gianfar.h
+++ b/drivers/net/gianfar.h
@@ -265,6 +265,11 @@ struct gfar_phy {
 	struct mii_bus miibus;
 };
 
+/* 2 seems to be the minimum number of TX descriptors to make it work. */
+#define TX_BUF_CNT 	2
+#define RX_BUF_CNT 	PKTBUFSRX
+#define BUF_ALIGN	8
+
 struct gfar_private {
 	struct eth_device edev;
 	void __iomem *regs;
@@ -282,5 +287,6 @@ struct gfar_private {
 	uint link;
 	uint duplexity;
 	uint speed;
+	void *rx_buffer[PKTBUFSRX];
 };
 #endif /* __GIANFAR_H */
-- 
2.39.2

[PATCH 5/7] net: ethoc: replace global NetRxPackets with per-interface allocation

[Ahmad Fatoum]

NetRxPackets is a remnant of times, where a board had at most one
Ethernet controller. This is outdated and we should drop NetRxPackets.

Switch over the driver to allocate the receive buffers needed.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/net/ethoc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index c878f498a447..a31d3bb52173 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -178,6 +178,8 @@ struct ethoc {
 	u32 cur_rx;
 
 	struct mii_bus miibus;
+
+	void *rx_buffer[PKTBUFSRX];
 };
 
 /**
@@ -266,7 +268,7 @@ static int ethoc_init_ring(struct ethoc *dev)
 		if (i == dev->num_rx - 1)
 			bd.stat |= RX_BD_WRAP;
 
-		bd.addr = (u32)NetRxPackets[i];
+		bd.addr = (u32)dev->rx_buffer[i];
 		ethoc_write_bd(dev, dev->num_tx + i, &bd);
 
 		flush_dcache_range(bd.addr, bd.addr + PKTSIZE);
@@ -534,12 +536,18 @@ static int ethoc_probe(struct device *dev)
 	struct resource *iores;
 	struct eth_device *edev;
 	struct ethoc *priv;
+	int ret;
 
 	edev = xzalloc(sizeof(struct eth_device) +
 		       sizeof(struct ethoc));
 	edev->priv = (struct ethoc *)(edev + 1);
 
 	priv = edev->priv;
+
+	ret = net_alloc_packets(priv->rx_buffer, ARRAY_SIZE(priv->rx_buffer));
+	if (ret)
+		return ret;
+
 	iores = dev_request_mem_resource(dev, 0);
 	if (IS_ERR(iores))
 		return PTR_ERR(iores);
-- 
2.39.2

[PATCH 6/7] net: cpsw: replace global NetRxPackets with per-interface allocation

[Ahmad Fatoum]

NetRxPackets is a remnant of times, where a board had at most one
Ethernet controller. This is outdated and we should drop NetRxPackets.

Switch over the driver to allocate the receive buffers needed.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/net/cpsw.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
index 42fb32bde2f5..3f24c21594c1 100644
--- a/drivers/net/cpsw.c
+++ b/drivers/net/cpsw.c
@@ -215,6 +215,8 @@ struct cpsw_priv {
 	unsigned int			slave_size;
 	unsigned int			sliver_ofs;
 
+	void				*rx_buffer[PKTBUFSRX - 2];
+
 	struct cpdma_desc		*descs;
 	struct cpdma_desc		*desc_free;
 	struct cpdma_chan		rx_chan, tx_chan;
@@ -980,7 +982,7 @@ static int cpsw_setup(struct device *dev)
 
 	/* submit rx descs */
 	for (i = 0; i < PKTBUFSRX - 2; i++) {
-		ret = cpdma_submit(priv, &priv->rx_chan, NetRxPackets[i],
+		ret = cpdma_submit(priv, &priv->rx_chan, priv->rx_buffer[i],
 				   PKTSIZE, 0);
 		if (ret < 0) {
 			dev_err(dev, "error %d submitting rx desc\n", ret);
@@ -1350,6 +1352,10 @@ static int cpsw_probe(struct device *dev)
 	priv = xzalloc(sizeof(*priv));
 	priv->dev = dev;
 
+	ret = net_alloc_packets(priv->rx_buffer, ARRAY_SIZE(priv->rx_buffer));
+	if (ret)
+		goto out;
+
 	if (dev->of_node) {
 		ret = cpsw_probe_dt(priv);
 		if (ret)
-- 
2.39.2

[PATCH 7/7] net: retire global NetRxPackets arrays

[Ahmad Fatoum]

Now that no in-tree user of NetRxPackets remain, no one will miss this
remnant of simpler times.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/net.h | 2 --
 net/net.c     | 3 ---
 2 files changed, 5 deletions(-)

diff --git a/include/net.h b/include/net.h
index 38fd6f8d3c41..5a6dd9ca7b82 100644
--- a/include/net.h
+++ b/include/net.h
@@ -249,8 +249,6 @@ struct icmphdr {
  * (big endian).
  */
 
-extern unsigned char *NetRxPackets[PKTBUFSRX];/* Receive packets		*/
-
 void net_set_ip(struct eth_device *edev, IPaddr_t ip);
 void net_set_serverip(IPaddr_t ip);
 const char *net_get_server(void);
diff --git a/net/net.c b/net/net.c
index 6f418df0538b..c06e88fe532d 100644
--- a/net/net.c
+++ b/net/net.c
@@ -29,7 +29,6 @@
 #include <linux/ctype.h>
 #include <linux/err.h>
 
-unsigned char *NetRxPackets[PKTBUFSRX]; /* Receive packets		*/
 static unsigned int net_ip_id;
 
 char *net_server;
@@ -815,8 +814,6 @@ int net_alloc_packets(void **packets, int count)
 
 static int net_init(void)
 {
-	net_alloc_packets((void **)NetRxPackets, PKTBUFSRX);
-
 	globalvar_add_simple_ip("net.nameserver", &net_nameserver);
 	globalvar_add_simple_string("net.domainname", &net_domainname);
 	globalvar_add_simple_string("net.server", &net_server);
-- 
2.39.2

Re: [PATCH 0/7] net: replace global NetRxPackets arrays with per-interface allocation

[Sascha Hauer]

On Wed, 13 Mar 2024 12:06:57 +0100, Ahmad Fatoum wrote:
> Drivers using NetRxPackets can't be combined into the same barebox build
> without clobbering each other when receiving packets. Even having the
> same driver twice would lead to issues.
> 
> We certainly don't want to encourage new network drivers to use this
> global array, so let's just get rid of it.
> 
> [...]

Applied, thanks!

[1/7] net: add net_alloc_packets helper
      https://git.pengutronix.de/cgit/barebox/commit/?id=73cb4117e235 (link may not be stable)
[2/7] net: ep93xx: replace global NetRxPackets with per-interface allocation
      https://git.pengutronix.de/cgit/barebox/commit/?id=33d0bce33443 (link may not be stable)
[3/7] net: enc28j60: replace global NetRxPackets with per-interface allocation
      https://git.pengutronix.de/cgit/barebox/commit/?id=01e3e40961fd (link may not be stable)
[4/7] net: gianfar: replace global NetRxPackets with per-interface allocation
      https://git.pengutronix.de/cgit/barebox/commit/?id=18e518e7e4d5 (link may not be stable)
[5/7] net: ethoc: replace global NetRxPackets with per-interface allocation
      https://git.pengutronix.de/cgit/barebox/commit/?id=41b8dc7a59d9 (link may not be stable)
[6/7] net: cpsw: replace global NetRxPackets with per-interface allocation
      https://git.pengutronix.de/cgit/barebox/commit/?id=cbb8be478429 (link may not be stable)
[7/7] net: retire global NetRxPackets arrays
      https://git.pengutronix.de/cgit/barebox/commit/?id=d417b8b372b3 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>