[PATCH v1 1/2] net/axgbe: add multi-process support

[PATCH v1 1/2] net/axgbe: add multi-process support

[Jesna K E]

Added multi-process support for axgbe PMD
To achieve multi-process support separate out TX and RX function
inside the axgbe driver and call that from a secondary process
when it is attaching to an already-configured NIC

Signed-off-by: Jesna K E <jesna.k.e@amd.com>
---
 .mailmap                               |  1 +
 doc/guides/nics/features/axgbe.ini     |  1 +
 doc/guides/rel_notes/release_23_03.rst |  8 ++++
 drivers/net/axgbe/axgbe_ethdev.c       | 65 +++++++++++++++++++-------
 drivers/net/axgbe/axgbe_rxtx.c         | 11 -----
 drivers/net/axgbe/axgbe_rxtx.h         |  6 +++
 6 files changed, 65 insertions(+), 27 deletions(-)

diff --git a/.mailmap b/.mailmap
index 75884b6fe2..a153c9ab93 100644
--- a/.mailmap
+++ b/.mailmap
@@ -581,6 +581,7 @@ Jerome Jutteau <jerome.jutteau@outscale.com>
 Jerry Hao OS <jerryhao@os.amperecomputing.com>
 Jerry Lilijun <jerry.lilijun@huawei.com>
 Jerry Zhang <jerry.zhang@intel.com>
+Jesna K E <jesna.k.e@amd.com>
 Jesper Wramberg <jesper.wramberg@gmail.com>
 Jesse Brandeburg <jesse.brandeburg@intel.com>
 Jesse Bruni <jesse.bruni@intel.com>
diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini
index 821bb682d4..5e2d6498e5 100644
--- a/doc/guides/nics/features/axgbe.ini
+++ b/doc/guides/nics/features/axgbe.ini
@@ -18,6 +18,7 @@ L3 checksum offload  = Y
 L4 checksum offload  = Y
 Basic stats          = Y
 FW version           = Y
+Multiprocess aware   = Y
 Linux                = Y
 x86-32               = Y
 x86-64               = Y
diff --git a/doc/guides/rel_notes/release_23_03.rst b/doc/guides/rel_notes/release_23_03.rst
index b8c5b68d6c..17e5862c04 100644
--- a/doc/guides/rel_notes/release_23_03.rst
+++ b/doc/guides/rel_notes/release_23_03.rst
@@ -55,6 +55,14 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added multi-process support for axgbe PMD.**
+
+  To achieve multi-process support separate out TX and RX function
+  inside the axgbe driver and call that from a secondary process
+  when it is attaching to an already-configured NIC
+
+  Added new API functions ``axgbe_set_rx_function()``,
+  and ``axgbe_set_tx_function()`` to support multi-process.
 
 Removed Items
 -------------
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index b071e4e460..57dead3adc 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -353,8 +353,6 @@ axgbe_dev_start(struct rte_eth_dev *dev)
 {
 	struct axgbe_port *pdata = dev->data->dev_private;
 	int ret;
-	struct rte_eth_dev_data *dev_data = dev->data;
-	uint16_t max_pkt_len;
 
 	dev->dev_ops = &axgbe_eth_dev_ops;
 
@@ -388,17 +386,8 @@ axgbe_dev_start(struct rte_eth_dev *dev)
 	rte_bit_relaxed_clear32(AXGBE_STOPPED, &pdata->dev_state);
 	rte_bit_relaxed_clear32(AXGBE_DOWN, &pdata->dev_state);
 
-	max_pkt_len = dev_data->mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
-	if ((dev_data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||
-				max_pkt_len > pdata->rx_buf_size)
-		dev_data->scattered_rx = 1;
-
-	/*  Scatter Rx handling */
-	if (dev_data->scattered_rx)
-		dev->rx_pkt_burst = &eth_axgbe_recv_scattered_pkts;
-	else
-		dev->rx_pkt_burst = &axgbe_recv_pkts;
-
+	axgbe_set_rx_function(dev);
+	axgbe_set_tx_function(dev);
 	return 0;
 }
 
@@ -2145,6 +2134,46 @@ get_pci_rc_devid(void)
 	return (uint16_t)device_id;
 }
 
+/* Takes  ethdev as parameter
+ *  Used in dev_start by primary process and then
+ * in dev_init by secondary process when attaching to an existing ethdev.
+ */
+void
+axgbe_set_tx_function(struct rte_eth_dev *dev)
+{
+	struct axgbe_port *pdata = dev->data->dev_private;
+
+	if (pdata->multi_segs_tx)
+		dev->tx_pkt_burst = &axgbe_xmit_pkts_seg;
+#ifdef RTE_ARCH_X86
+	struct axgbe_tx_queue *txq = dev->data->tx_queues[0];
+	if (!txq->vector_disable &&
+			rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_128)
+		dev->tx_pkt_burst = &axgbe_xmit_pkts_vec;
+#else
+		dev->tx_pkt_burst = &axgbe_xmit_pkts;
+#endif
+}
+
+void
+axgbe_set_rx_function(struct rte_eth_dev *dev)
+{
+	struct rte_eth_dev_data *dev_data = dev->data;
+	uint16_t max_pkt_len;
+	struct axgbe_port *pdata;
+
+	pdata = dev->data->dev_private;
+	max_pkt_len = dev_data->mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
+	if ((dev_data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||
+			max_pkt_len > pdata->rx_buf_size)
+		dev_data->scattered_rx = 1;
+	/*  Scatter Rx handling */
+	if (dev_data->scattered_rx)
+		dev->rx_pkt_burst = &eth_axgbe_recv_scattered_pkts;
+	else
+		dev->rx_pkt_burst = &axgbe_recv_pkts;
+}
+
 /*
  * It returns 0 on success.
  */
@@ -2159,19 +2188,23 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 	int ret;
 
 	eth_dev->dev_ops = &axgbe_eth_dev_ops;
-
 	eth_dev->rx_descriptor_status = axgbe_dev_rx_descriptor_status;
 	eth_dev->tx_descriptor_status = axgbe_dev_tx_descriptor_status;
 
+	eth_dev->tx_pkt_burst = &axgbe_xmit_pkts;
+	eth_dev->rx_pkt_burst = &axgbe_recv_pkts;
+
 	/*
 	 * For secondary processes, we don't initialise any further as primary
 	 * has already done this work.
 	 */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+		axgbe_set_tx_function(eth_dev);
+		axgbe_set_rx_function(eth_dev);
 		return 0;
+	}
 
 	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
-
 	pdata = eth_dev->data->dev_private;
 	/* initial state */
 	rte_bit_relaxed_set32(AXGBE_DOWN, &pdata->dev_state);
diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index 7cff79e030..9b283bd9d0 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -629,17 +629,6 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 				RTE_ETH_TX_OFFLOAD_MULTI_SEGS))
 		pdata->multi_segs_tx = true;
 
-	if (pdata->multi_segs_tx)
-		dev->tx_pkt_burst = &axgbe_xmit_pkts_seg;
-	else if (txq->vector_disable ||
-			rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_128)
-		dev->tx_pkt_burst = &axgbe_xmit_pkts;
-	else
-#ifdef RTE_ARCH_X86
-		dev->tx_pkt_burst = &axgbe_xmit_pkts_vec;
-#else
-		dev->tx_pkt_burst = &axgbe_xmit_pkts;
-#endif
 
 	return 0;
 }
diff --git a/drivers/net/axgbe/axgbe_rxtx.h b/drivers/net/axgbe/axgbe_rxtx.h
index eeef908ceb..a904939de3 100644
--- a/drivers/net/axgbe/axgbe_rxtx.h
+++ b/drivers/net/axgbe/axgbe_rxtx.h
@@ -158,6 +158,12 @@ struct axgbe_tx_queue {
  * RX/TX function prototypes
  */
 
+/* Takes an ethdev as parameter
+ * Used in dev_start by primary process and then
+ * in dev_init by secondary process when attaching to an existing ethdev.
+ */
+void axgbe_set_tx_function(struct rte_eth_dev *dev);
+void axgbe_set_rx_function(struct rte_eth_dev *dev);
 
 void axgbe_dev_tx_queue_release(struct rte_eth_dev *dev, uint16_t queue_idx);
 int  axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
-- 
2.25.1

[PATCH v1 2/2] net/axgbe: move offloads to Rx/Tx queue setup

[Jesna K E]

For Multiprocess dpdk applications retrieving the offload parameter
for receive packet directly from struct eth_dev
giving segmentation fault since rxmode.offloads from eth_dev was
null duirng recv_pkts. So retrieved offload from rx/tx_queue_setup()

Signed-off-by: Jesna K E <jesna.k.e@amd.com>
---
 drivers/net/axgbe/axgbe_rxtx.c | 10 ++++------
 drivers/net/axgbe/axgbe_rxtx.h |  4 ++--
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index 9b283bd9d0..5dc4fe7369 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -86,6 +86,7 @@ int axgbe_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 	if (rxq->free_thresh >  rxq->nb_desc)
 		rxq->free_thresh = rxq->nb_desc >> 3;
 
+	rxq->offloads = rx_conf->offloads | dev->data->dev_conf.rxmode.offloads;
 	/* Allocate RX ring hardware descriptors */
 	size = rxq->nb_desc * sizeof(union axgbe_rx_desc);
 	dma = rte_eth_dma_zone_reserve(dev, "rx_ring", queue_idx, size, 128,
@@ -211,7 +212,6 @@ axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	unsigned int err, etlt;
 	uint32_t error_status;
 	uint16_t idx, pidx, pkt_len;
-	uint64_t offloads;
 
 	idx = AXGBE_GET_DESC_IDX(rxq, rxq->cur);
 	while (nb_rx < nb_pkts) {
@@ -278,14 +278,13 @@ axgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 			mbuf->hash.rss = rte_le_to_cpu_32(desc->write.desc1);
 		etlt = AXGMAC_GET_BITS_LE(desc->write.desc3,
 				RX_NORMAL_DESC3, ETLT);
-		offloads = rxq->pdata->eth_dev->data->dev_conf.rxmode.offloads;
 		if (!err || !etlt) {
 			if (etlt == RX_CVLAN_TAG_PRESENT) {
 				mbuf->ol_flags |= RTE_MBUF_F_RX_VLAN;
 				mbuf->vlan_tci =
 					AXGMAC_GET_BITS_LE(desc->write.desc0,
 							RX_NORMAL_DESC0, OVT);
-				if (offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP)
+				if (rxq->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP)
 					mbuf->ol_flags |= RTE_MBUF_F_RX_VLAN_STRIPPED;
 				else
 					mbuf->ol_flags &= ~RTE_MBUF_F_RX_VLAN_STRIPPED;
@@ -345,7 +344,6 @@ uint16_t eth_axgbe_recv_scattered_pkts(void *rx_queue,
 	unsigned int err = 0, etlt;
 	uint32_t error_status = 0;
 	uint16_t idx, pidx, data_len = 0, pkt_len = 0;
-	uint64_t offloads;
 	bool eop = 0;
 
 	idx = AXGBE_GET_DESC_IDX(rxq, rxq->cur);
@@ -441,14 +439,13 @@ uint16_t eth_axgbe_recv_scattered_pkts(void *rx_queue,
 				rte_le_to_cpu_32(desc->write.desc1);
 		etlt = AXGMAC_GET_BITS_LE(desc->write.desc3,
 				RX_NORMAL_DESC3, ETLT);
-		offloads = rxq->pdata->eth_dev->data->dev_conf.rxmode.offloads;
 		if (!err || !etlt) {
 			if (etlt == RX_CVLAN_TAG_PRESENT) {
 				first_seg->ol_flags |= RTE_MBUF_F_RX_VLAN;
 				first_seg->vlan_tci =
 					AXGMAC_GET_BITS_LE(desc->write.desc0,
 							RX_NORMAL_DESC0, OVT);
-				if (offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP)
+				if (rxq->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP)
 					first_seg->ol_flags |=
 						RTE_MBUF_F_RX_VLAN_STRIPPED;
 				else
@@ -606,6 +603,7 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 	txq->desc = tz->addr;
 	txq->queue_id = queue_idx;
 	txq->port_id = dev->data->port_id;
+	txq->offloads = offloads;
 	txq->dma_regs = (void *)((uint8_t *)pdata->xgmac_regs + DMA_CH_BASE +
 		(DMA_CH_INC * txq->queue_id));
 	txq->dma_tail_reg = (volatile uint32_t *)((uint8_t *)txq->dma_regs +
diff --git a/drivers/net/axgbe/axgbe_rxtx.h b/drivers/net/axgbe/axgbe_rxtx.h
index a904939de3..3e38d321f2 100644
--- a/drivers/net/axgbe/axgbe_rxtx.h
+++ b/drivers/net/axgbe/axgbe_rxtx.h
@@ -100,7 +100,7 @@ struct axgbe_rx_queue {
 	uint64_t rx_mbuf_alloc_failed;
 	/* Number of mbufs allocated from pool*/
 	uint64_t mbuf_alloc;
-
+	uint64_t offloads; /**< Rx offloads with RTE_ETH_RX_OFFLOAD_**/
 } __rte_cache_aligned;
 
 /*Tx descriptor format */
@@ -149,7 +149,7 @@ struct axgbe_tx_queue {
 	uint64_t pkts;
 	uint64_t bytes;
 	uint64_t errors;
-
+	uint64_t offloads; /**< Tx offload flags of RTE_ETH_TX_OFFLOAD_* */
 } __rte_cache_aligned;
 
 /*Queue related APIs */
-- 
2.25.1

[PATCH v1 1/2] net/axgbe: add multi-process support

[Jesna K E]

Added multi-process support for axgbe PMD
To achieve multi-process support separate out TX and RX function
inside the axgbe driver and call that from a secondary process
when it is attaching to an already-configured NIC

Signed-off-by: Jesna K E <jesna.k.e@amd.com>
---
 doc/guides/nics/features/axgbe.ini |  1 +
 drivers/net/axgbe/axgbe_ethdev.c   | 65 ++++++++++++++++++++++--------
 drivers/net/axgbe/axgbe_rxtx.c     | 11 -----
 drivers/net/axgbe/axgbe_rxtx.h     |  7 +++-
 4 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini
index 821bb682d4..5e2d6498e5 100644
--- a/doc/guides/nics/features/axgbe.ini
+++ b/doc/guides/nics/features/axgbe.ini
@@ -18,6 +18,7 @@ L3 checksum offload  = Y
 L4 checksum offload  = Y
 Basic stats          = Y
 FW version           = Y
+Multiprocess aware   = Y
 Linux                = Y
 x86-32               = Y
 x86-64               = Y
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index b071e4e460..4b3d655bd3 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -353,8 +353,6 @@ axgbe_dev_start(struct rte_eth_dev *dev)
 {
 	struct axgbe_port *pdata = dev->data->dev_private;
 	int ret;
-	struct rte_eth_dev_data *dev_data = dev->data;
-	uint16_t max_pkt_len;
 
 	dev->dev_ops = &axgbe_eth_dev_ops;
 
@@ -388,17 +386,8 @@ axgbe_dev_start(struct rte_eth_dev *dev)
 	rte_bit_relaxed_clear32(AXGBE_STOPPED, &pdata->dev_state);
 	rte_bit_relaxed_clear32(AXGBE_DOWN, &pdata->dev_state);
 
-	max_pkt_len = dev_data->mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
-	if ((dev_data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||
-				max_pkt_len > pdata->rx_buf_size)
-		dev_data->scattered_rx = 1;
-
-	/*  Scatter Rx handling */
-	if (dev_data->scattered_rx)
-		dev->rx_pkt_burst = &eth_axgbe_recv_scattered_pkts;
-	else
-		dev->rx_pkt_burst = &axgbe_recv_pkts;
-
+	axgbe_set_rx_function(dev);
+	axgbe_set_tx_function(dev);
 	return 0;
 }
 
@@ -2145,6 +2134,46 @@ get_pci_rc_devid(void)
 	return (uint16_t)device_id;
 }
 
+/* Takes  ethdev as parameter
+ *  Used in dev_start by primary process and then
+ * in dev_init by secondary process when attaching to an existing ethdev.
+ */
+void
+axgbe_set_tx_function(struct rte_eth_dev *dev)
+{
+	struct axgbe_port *pdata = dev->data->dev_private;
+	struct axgbe_tx_queue *txq = dev->data->tx_queues[0];
+
+	if (pdata->multi_segs_tx)
+		dev->tx_pkt_burst = &axgbe_xmit_pkts_seg;
+#ifdef RTE_ARCH_X86
+	if (!txq->vector_disable &&
+			rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_128)
+		dev->tx_pkt_burst = &axgbe_xmit_pkts_vec;
+#else
+		dev->tx_pkt_burst = &axgbe_xmit_pkts;
+#endif
+}
+
+void
+axgbe_set_rx_function(struct rte_eth_dev *dev)
+{
+	struct rte_eth_dev_data *dev_data = dev->data;
+	uint16_t max_pkt_len;
+	struct axgbe_port *pdata;
+
+	pdata = dev->data->dev_private;
+	max_pkt_len = dev_data->mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
+	if ((dev_data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||
+			max_pkt_len > pdata->rx_buf_size)
+		dev_data->scattered_rx = 1;
+	/*  Scatter Rx handling */
+	if (dev_data->scattered_rx)
+		dev->rx_pkt_burst = &eth_axgbe_recv_scattered_pkts;
+	else
+		dev->rx_pkt_burst = &axgbe_recv_pkts;
+}
+
 /*
  * It returns 0 on success.
  */
@@ -2159,17 +2188,20 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 	int ret;
 
 	eth_dev->dev_ops = &axgbe_eth_dev_ops;
-
 	eth_dev->rx_descriptor_status = axgbe_dev_rx_descriptor_status;
 	eth_dev->tx_descriptor_status = axgbe_dev_tx_descriptor_status;
 
+	eth_dev->tx_pkt_burst = &axgbe_xmit_pkts;
+	eth_dev->rx_pkt_burst = &axgbe_recv_pkts;
 	/*
 	 * For secondary processes, we don't initialise any further as primary
 	 * has already done this work.
 	 */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+		axgbe_set_tx_function(eth_dev);
+		axgbe_set_rx_function(eth_dev);
 		return 0;
-
+	}
 	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
 
 	pdata = eth_dev->data->dev_private;
@@ -2177,7 +2209,6 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 	rte_bit_relaxed_set32(AXGBE_DOWN, &pdata->dev_state);
 	rte_bit_relaxed_set32(AXGBE_STOPPED, &pdata->dev_state);
 	pdata->eth_dev = eth_dev;
-
 	pci_dev = RTE_DEV_TO_PCI(eth_dev->device);
 	pdata->pci_dev = pci_dev;
 
diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index 7cff79e030..9b283bd9d0 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -629,17 +629,6 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 				RTE_ETH_TX_OFFLOAD_MULTI_SEGS))
 		pdata->multi_segs_tx = true;
 
-	if (pdata->multi_segs_tx)
-		dev->tx_pkt_burst = &axgbe_xmit_pkts_seg;
-	else if (txq->vector_disable ||
-			rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_128)
-		dev->tx_pkt_burst = &axgbe_xmit_pkts;
-	else
-#ifdef RTE_ARCH_X86
-		dev->tx_pkt_burst = &axgbe_xmit_pkts_vec;
-#else
-		dev->tx_pkt_burst = &axgbe_xmit_pkts;
-#endif
 
 	return 0;
 }
diff --git a/drivers/net/axgbe/axgbe_rxtx.h b/drivers/net/axgbe/axgbe_rxtx.h
index eeef908ceb..d5660f5c4b 100644
--- a/drivers/net/axgbe/axgbe_rxtx.h
+++ b/drivers/net/axgbe/axgbe_rxtx.h
@@ -158,7 +158,12 @@ struct axgbe_tx_queue {
  * RX/TX function prototypes
  */
 
-
+/* Takes an ethdev and sets up the tx function to be used based on
+ * the queue parameters. Used in dev_start by primary process and then
+ * in dev_init by secondary process when attaching to an existing ethdev.
+ */
+void axgbe_set_tx_function(struct rte_eth_dev *dev);
+void axgbe_set_rx_function(struct rte_eth_dev *dev);
 void axgbe_dev_tx_queue_release(struct rte_eth_dev *dev, uint16_t queue_idx);
 int  axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 			      uint16_t nb_tx_desc, unsigned int socket_id,
-- 
2.25.1

Re: [PATCH v1 1/2] net/axgbe: add multi-process support

[Ferruh Yigit]

On 12/21/2022 2:52 AM, Jesna K E wrote:
> Added multi-process support for axgbe PMD
> To achieve multi-process support separate out TX and RX function
> inside the axgbe driver and call that from a secondary process
> when it is attaching to an already-configured NIC
> 
> Signed-off-by: Jesna K E <jesna.k.e@amd.com>
> ---
>  doc/guides/nics/features/axgbe.ini |  1 +
>  drivers/net/axgbe/axgbe_ethdev.c   | 65 ++++++++++++++++++++++--------
>  drivers/net/axgbe/axgbe_rxtx.c     | 11 -----
>  drivers/net/axgbe/axgbe_rxtx.h     |  7 +++-
>  4 files changed, 55 insertions(+), 29 deletions(-)
> 

Overall looks good, do you think does it worth to document this in
release notes (doc/guides/rel_notes/release_23_03.rst)?

> diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini
> index 821bb682d4..5e2d6498e5 100644
> --- a/doc/guides/nics/features/axgbe.ini
> +++ b/doc/guides/nics/features/axgbe.ini
> @@ -18,6 +18,7 @@ L3 checksum offload  = Y
>  L4 checksum offload  = Y
>  Basic stats          = Y
>  FW version           = Y
> +Multiprocess aware   = Y
>  Linux                = Y
>  x86-32               = Y
>  x86-64               = Y
> diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
> index b071e4e460..4b3d655bd3 100644
> --- a/drivers/net/axgbe/axgbe_ethdev.c
> +++ b/drivers/net/axgbe/axgbe_ethdev.c
> @@ -353,8 +353,6 @@ axgbe_dev_start(struct rte_eth_dev *dev)
>  {
>  	struct axgbe_port *pdata = dev->data->dev_private;
>  	int ret;
> -	struct rte_eth_dev_data *dev_data = dev->data;
> -	uint16_t max_pkt_len;
>  
>  	dev->dev_ops = &axgbe_eth_dev_ops;
>  
> @@ -388,17 +386,8 @@ axgbe_dev_start(struct rte_eth_dev *dev)
>  	rte_bit_relaxed_clear32(AXGBE_STOPPED, &pdata->dev_state);
>  	rte_bit_relaxed_clear32(AXGBE_DOWN, &pdata->dev_state);
>  
> -	max_pkt_len = dev_data->mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
> -	if ((dev_data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||
> -				max_pkt_len > pdata->rx_buf_size)
> -		dev_data->scattered_rx = 1;
> -
> -	/*  Scatter Rx handling */
> -	if (dev_data->scattered_rx)
> -		dev->rx_pkt_burst = &eth_axgbe_recv_scattered_pkts;
> -	else
> -		dev->rx_pkt_burst = &axgbe_recv_pkts;
> -
> +	axgbe_set_rx_function(dev);
> +	axgbe_set_tx_function(dev);
>  	return 0;
>  }
>  
> @@ -2145,6 +2134,46 @@ get_pci_rc_devid(void)
>  	return (uint16_t)device_id;
>  }
>  
> +/* Takes  ethdev as parameter
> + *  Used in dev_start by primary process and then
> + * in dev_init by secondary process when attaching to an existing ethdev.
> + */
> +void
> +axgbe_set_tx_function(struct rte_eth_dev *dev)
> +{
> +	struct axgbe_port *pdata = dev->data->dev_private;
> +	struct axgbe_tx_queue *txq = dev->data->tx_queues[0];
> +
> +	if (pdata->multi_segs_tx)
> +		dev->tx_pkt_burst = &axgbe_xmit_pkts_seg;
> +#ifdef RTE_ARCH_X86
> +	if (!txq->vector_disable &&
> +			rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_128)
> +		dev->tx_pkt_burst = &axgbe_xmit_pkts_vec;
> +#else
> +		dev->tx_pkt_burst = &axgbe_xmit_pkts;
> +#endif
> +}
> +
> +void
> +axgbe_set_rx_function(struct rte_eth_dev *dev)
> +{
> +	struct rte_eth_dev_data *dev_data = dev->data;
> +	uint16_t max_pkt_len;
> +	struct axgbe_port *pdata;
> +
> +	pdata = dev->data->dev_private;
> +	max_pkt_len = dev_data->mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
> +	if ((dev_data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||
> +			max_pkt_len > pdata->rx_buf_size)
> +		dev_data->scattered_rx = 1;
> +	/*  Scatter Rx handling */
> +	if (dev_data->scattered_rx)
> +		dev->rx_pkt_burst = &eth_axgbe_recv_scattered_pkts;
> +	else
> +		dev->rx_pkt_burst = &axgbe_recv_pkts;
> +}
> +
>  /*
>   * It returns 0 on success.
>   */
> @@ -2159,17 +2188,20 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
>  	int ret;
>  
>  	eth_dev->dev_ops = &axgbe_eth_dev_ops;
> -
>  	eth_dev->rx_descriptor_status = axgbe_dev_rx_descriptor_status;
>  	eth_dev->tx_descriptor_status = axgbe_dev_tx_descriptor_status;
>  
> +	eth_dev->tx_pkt_burst = &axgbe_xmit_pkts;
> +	eth_dev->rx_pkt_burst = &axgbe_recv_pkts;
>  	/*
>  	 * For secondary processes, we don't initialise any further as primary
>  	 * has already done this work.
>  	 */
> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> +		axgbe_set_tx_function(eth_dev);
> +		axgbe_set_rx_function(eth_dev);
>  		return 0;
> -

What do you think to keep empty lines before and after secondary process
related block to group it?

> +	}
>  	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
>  
>  	pdata = eth_dev->data->dev_private;
> @@ -2177,7 +2209,6 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
>  	rte_bit_relaxed_set32(AXGBE_DOWN, &pdata->dev_state);
>  	rte_bit_relaxed_set32(AXGBE_STOPPED, &pdata->dev_state);
>  	pdata->eth_dev = eth_dev;
> -

Please remove noise in the patch

>  	pci_dev = RTE_DEV_TO_PCI(eth_dev->device);
>  	pdata->pci_dev = pci_dev;
>  
> diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
> index 7cff79e030..9b283bd9d0 100644
> --- a/drivers/net/axgbe/axgbe_rxtx.c
> +++ b/drivers/net/axgbe/axgbe_rxtx.c
> @@ -629,17 +629,6 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
>  				RTE_ETH_TX_OFFLOAD_MULTI_SEGS))
>  		pdata->multi_segs_tx = true;
>  
> -	if (pdata->multi_segs_tx)
> -		dev->tx_pkt_burst = &axgbe_xmit_pkts_seg;
> -	else if (txq->vector_disable ||
> -			rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_128)
> -		dev->tx_pkt_burst = &axgbe_xmit_pkts;
> -	else
> -#ifdef RTE_ARCH_X86
> -		dev->tx_pkt_burst = &axgbe_xmit_pkts_vec;
> -#else
> -		dev->tx_pkt_burst = &axgbe_xmit_pkts;
> -#endif
>  
>  	return 0;
>  }
> diff --git a/drivers/net/axgbe/axgbe_rxtx.h b/drivers/net/axgbe/axgbe_rxtx.h
> index eeef908ceb..d5660f5c4b 100644
> --- a/drivers/net/axgbe/axgbe_rxtx.h
> +++ b/drivers/net/axgbe/axgbe_rxtx.h
> @@ -158,7 +158,12 @@ struct axgbe_tx_queue {
>   * RX/TX function prototypes
>   */
>  
> -
> +/* Takes an ethdev and sets up the tx function to be used based on
> + * the queue parameters. Used in dev_start by primary process and then
> + * in dev_init by secondary process when attaching to an existing ethdev.

Is the comment only for Tx, or for burst functions? Since comment refers
only to Tx.

> + */
> +void axgbe_set_tx_function(struct rte_eth_dev *dev);
> +void axgbe_set_rx_function(struct rte_eth_dev *dev);

You can put an empty line here to highlight comment is for these two
functions.

>  void axgbe_dev_tx_queue_release(struct rte_eth_dev *dev, uint16_t queue_idx);
>  int  axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
>  			      uint16_t nb_tx_desc, unsigned int socket_id,

Re: [PATCH v1 1/2] net/axgbe: add multi-process support

[Ferruh Yigit]

On 12/21/2022 2:52 AM, Jesna K E wrote:
> +/* Takes  ethdev as parameter
> + *  Used in dev_start by primary process and then
> + * in dev_init by secondary process when attaching to an existing ethdev.
> + */
> +void
> +axgbe_set_tx_function(struct rte_eth_dev *dev)
> +{
> +	struct axgbe_port *pdata = dev->data->dev_private;
> +	struct axgbe_tx_queue *txq = dev->data->tx_queues[0];
> +
> +	if (pdata->multi_segs_tx)
> +		dev->tx_pkt_burst = &axgbe_xmit_pkts_seg;
> +#ifdef RTE_ARCH_X86
> +	if (!txq->vector_disable &&
> +			rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_128)
> +		dev->tx_pkt_burst = &axgbe_xmit_pkts_vec;
> +#else
> +		dev->tx_pkt_burst = &axgbe_xmit_pkts;
> +#endif
> +}

'txq' is used only for 'RTE_ARCH_X86', that is why it gives "unused
variable" warning for it.
Can you please declare 'txq' within 'RTE_ARCH_X86' macro?

Re: [PATCH v1 1/2] net/axgbe: add multi-process support

[Ferruh Yigit]

On 12/23/2022 10:44 AM, Ferruh Yigit wrote:
> On 12/21/2022 2:52 AM, Jesna K E wrote:
>> +/* Takes  ethdev as parameter
>> + *  Used in dev_start by primary process and then
>> + * in dev_init by secondary process when attaching to an existing ethdev.
>> + */
>> +void
>> +axgbe_set_tx_function(struct rte_eth_dev *dev)
>> +{
>> +	struct axgbe_port *pdata = dev->data->dev_private;
>> +	struct axgbe_tx_queue *txq = dev->data->tx_queues[0];
>> +
>> +	if (pdata->multi_segs_tx)
>> +		dev->tx_pkt_burst = &axgbe_xmit_pkts_seg;
>> +#ifdef RTE_ARCH_X86
>> +	if (!txq->vector_disable &&
>> +			rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_128)
>> +		dev->tx_pkt_burst = &axgbe_xmit_pkts_vec;
>> +#else
>> +		dev->tx_pkt_burst = &axgbe_xmit_pkts;

btw, indentation of this line looks wrong

>> +#endif
>> +}
> 
> 'txq' is used only for 'RTE_ARCH_X86', that is why it gives "unused
> variable" warning for it.
> Can you please declare 'txq' within 'RTE_ARCH_X86' macro?

[PATCH v1 1/2] net/axgbe: add multi-process support

[Jesna K E]

Added multi-process support for axgbe PMD
To achieve multi-process support separate out TX and RX function
inside the axgbe driver and call that from a secondary process
when it is attaching to an already-configured NIC

Signed-off-by: Jesna K E <jesna.k.e@amd.com>
---
 doc/guides/nics/features/axgbe.ini |  1 +
 drivers/net/axgbe/axgbe_ethdev.c   | 65 ++++++++++++++++++++++--------
 drivers/net/axgbe/axgbe_rxtx.c     | 11 -----
 drivers/net/axgbe/axgbe_rxtx.h     |  7 +++-
 4 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini
index 821bb682d4..5e2d6498e5 100644
--- a/doc/guides/nics/features/axgbe.ini
+++ b/doc/guides/nics/features/axgbe.ini
@@ -18,6 +18,7 @@ L3 checksum offload  = Y
 L4 checksum offload  = Y
 Basic stats          = Y
 FW version           = Y
+Multiprocess aware   = Y
 Linux                = Y
 x86-32               = Y
 x86-64               = Y
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index b071e4e460..4b3d655bd3 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -353,8 +353,6 @@ axgbe_dev_start(struct rte_eth_dev *dev)
 {
 	struct axgbe_port *pdata = dev->data->dev_private;
 	int ret;
-	struct rte_eth_dev_data *dev_data = dev->data;
-	uint16_t max_pkt_len;
 
 	dev->dev_ops = &axgbe_eth_dev_ops;
 
@@ -388,17 +386,8 @@ axgbe_dev_start(struct rte_eth_dev *dev)
 	rte_bit_relaxed_clear32(AXGBE_STOPPED, &pdata->dev_state);
 	rte_bit_relaxed_clear32(AXGBE_DOWN, &pdata->dev_state);
 
-	max_pkt_len = dev_data->mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
-	if ((dev_data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||
-				max_pkt_len > pdata->rx_buf_size)
-		dev_data->scattered_rx = 1;
-
-	/*  Scatter Rx handling */
-	if (dev_data->scattered_rx)
-		dev->rx_pkt_burst = &eth_axgbe_recv_scattered_pkts;
-	else
-		dev->rx_pkt_burst = &axgbe_recv_pkts;
-
+	axgbe_set_rx_function(dev);
+	axgbe_set_tx_function(dev);
 	return 0;
 }
 
@@ -2145,6 +2134,46 @@ get_pci_rc_devid(void)
 	return (uint16_t)device_id;
 }
 
+/* Takes  ethdev as parameter
+ *  Used in dev_start by primary process and then
+ * in dev_init by secondary process when attaching to an existing ethdev.
+ */
+void
+axgbe_set_tx_function(struct rte_eth_dev *dev)
+{
+	struct axgbe_port *pdata = dev->data->dev_private;
+	struct axgbe_tx_queue *txq = dev->data->tx_queues[0];
+
+	if (pdata->multi_segs_tx)
+		dev->tx_pkt_burst = &axgbe_xmit_pkts_seg;
+#ifdef RTE_ARCH_X86
+	if (!txq->vector_disable &&
+			rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_128)
+		dev->tx_pkt_burst = &axgbe_xmit_pkts_vec;
+#else
+		dev->tx_pkt_burst = &axgbe_xmit_pkts;
+#endif
+}
+
+void
+axgbe_set_rx_function(struct rte_eth_dev *dev)
+{
+	struct rte_eth_dev_data *dev_data = dev->data;
+	uint16_t max_pkt_len;
+	struct axgbe_port *pdata;
+
+	pdata = dev->data->dev_private;
+	max_pkt_len = dev_data->mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
+	if ((dev_data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ||
+			max_pkt_len > pdata->rx_buf_size)
+		dev_data->scattered_rx = 1;
+	/*  Scatter Rx handling */
+	if (dev_data->scattered_rx)
+		dev->rx_pkt_burst = &eth_axgbe_recv_scattered_pkts;
+	else
+		dev->rx_pkt_burst = &axgbe_recv_pkts;
+}
+
 /*
  * It returns 0 on success.
  */
@@ -2159,17 +2188,20 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 	int ret;
 
 	eth_dev->dev_ops = &axgbe_eth_dev_ops;
-
 	eth_dev->rx_descriptor_status = axgbe_dev_rx_descriptor_status;
 	eth_dev->tx_descriptor_status = axgbe_dev_tx_descriptor_status;
 
+	eth_dev->tx_pkt_burst = &axgbe_xmit_pkts;
+	eth_dev->rx_pkt_burst = &axgbe_recv_pkts;
 	/*
 	 * For secondary processes, we don't initialise any further as primary
 	 * has already done this work.
 	 */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+		axgbe_set_tx_function(eth_dev);
+		axgbe_set_rx_function(eth_dev);
 		return 0;
-
+	}
 	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
 
 	pdata = eth_dev->data->dev_private;
@@ -2177,7 +2209,6 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 	rte_bit_relaxed_set32(AXGBE_DOWN, &pdata->dev_state);
 	rte_bit_relaxed_set32(AXGBE_STOPPED, &pdata->dev_state);
 	pdata->eth_dev = eth_dev;
-
 	pci_dev = RTE_DEV_TO_PCI(eth_dev->device);
 	pdata->pci_dev = pci_dev;
 
diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index 7cff79e030..9b283bd9d0 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -629,17 +629,6 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 				RTE_ETH_TX_OFFLOAD_MULTI_SEGS))
 		pdata->multi_segs_tx = true;
 
-	if (pdata->multi_segs_tx)
-		dev->tx_pkt_burst = &axgbe_xmit_pkts_seg;
-	else if (txq->vector_disable ||
-			rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_128)
-		dev->tx_pkt_burst = &axgbe_xmit_pkts;
-	else
-#ifdef RTE_ARCH_X86
-		dev->tx_pkt_burst = &axgbe_xmit_pkts_vec;
-#else
-		dev->tx_pkt_burst = &axgbe_xmit_pkts;
-#endif
 
 	return 0;
 }
diff --git a/drivers/net/axgbe/axgbe_rxtx.h b/drivers/net/axgbe/axgbe_rxtx.h
index eeef908ceb..d5660f5c4b 100644
--- a/drivers/net/axgbe/axgbe_rxtx.h
+++ b/drivers/net/axgbe/axgbe_rxtx.h
@@ -158,7 +158,12 @@ struct axgbe_tx_queue {
  * RX/TX function prototypes
  */
 
-
+/* Takes an ethdev and sets up the tx function to be used based on
+ * the queue parameters. Used in dev_start by primary process and then
+ * in dev_init by secondary process when attaching to an existing ethdev.
+ */
+void axgbe_set_tx_function(struct rte_eth_dev *dev);
+void axgbe_set_rx_function(struct rte_eth_dev *dev);
 void axgbe_dev_tx_queue_release(struct rte_eth_dev *dev, uint16_t queue_idx);
 int  axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 			      uint16_t nb_tx_desc, unsigned int socket_id,
-- 
2.25.1