[PATCH] net/ring: Set mbuf->port for received packets

[PATCH] net/ring: Set mbuf->port for received packets

[Sriram Yagnaraman]

When using ring based ethdev, mbuf->port is not set on received packets.

For applications that use the mbuf->port to identify the incoming port,
especially when eventdev RX adapter pulls the packet on a different core
and the application running on a worker core has no clue on the incoming
port. This change adds some cycles at receive, to set the port of course.

Please advise if this change is something that can be upstreamed.

Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
---
 drivers/net/ring/rte_eth_ring.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index b16f5d55f2..1346a0dba3 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -44,6 +44,7 @@ enum dev_action {
 
 struct ring_queue {
 	struct rte_ring *rng;
+	uint16_t in_port;
 	RTE_ATOMIC(uint64_t) rx_pkts;
 	RTE_ATOMIC(uint64_t) tx_pkts;
 };
@@ -75,10 +76,13 @@ RTE_LOG_REGISTER_DEFAULT(eth_ring_logtype, NOTICE);
 static uint16_t
 eth_ring_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
 {
+	unsigned int i;
 	void **ptrs = (void *)&bufs[0];
 	struct ring_queue *r = q;
 	const uint16_t nb_rx = (uint16_t)rte_ring_dequeue_burst(r->rng,
 			ptrs, nb_bufs, NULL);
+	for (i = 0; i < nb_rx; i++)
+		bufs[i]->port = r->in_port;
 	if (r->rng->flags & RING_F_SC_DEQ)
 		r->rx_pkts += nb_rx;
 	else
@@ -155,7 +159,7 @@ eth_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 				    struct rte_mempool *mb_pool __rte_unused)
 {
 	struct pmd_internals *internals = dev->data->dev_private;
-
+	internals->rx_ring_queues[rx_queue_id].in_port = dev->data->port_id;
 	dev->data->rx_queues[rx_queue_id] = &internals->rx_ring_queues[rx_queue_id];
 	return 0;
 }
@@ -414,10 +418,12 @@ do_eth_dev_ring_create(const char *name,
 	internals->max_tx_queues = nb_tx_queues;
 	for (i = 0; i < nb_rx_queues; i++) {
 		internals->rx_ring_queues[i].rng = rx_queues[i];
+		internals->rx_ring_queues[i].in_port = -1;
 		data->rx_queues[i] = &internals->rx_ring_queues[i];
 	}
 	for (i = 0; i < nb_tx_queues; i++) {
 		internals->tx_ring_queues[i].rng = tx_queues[i];
+		internals->tx_ring_queues[i].in_port = -1;
 		data->tx_queues[i] = &internals->tx_ring_queues[i];
 	}
 
-- 
2.31.1

RE: [PATCH] net/ring: Set mbuf->port for received packets

[Morten Brørup]

> From: Sriram Yagnaraman [mailto:sriram.yagnaraman@ericsson.com]
> 
> When using ring based ethdev, mbuf->port is not set on received packets.
> 
> For applications that use the mbuf->port to identify the incoming port,
> especially when eventdev RX adapter pulls the packet on a different core
> and the application running on a worker core has no clue on the incoming
> port. This change adds some cycles at receive, to set the port of
> course.

I agree that the mbuf->port field must be set before returning from rte_eth_rx_burst().

I'm not aware how applications use the ring based ethdev, so I might be asking silly questions...

How about all the other mbuf fields normally set by the PMD before returning from rte_eth_rx_burst()?

Is the enqueueing core supposed to set them?

Or if the ring is only used for queueing packets originally received (at a physical port) by the enqueueing core, why not keep the mbuf->port value from the original reception?

> 
> Please advise if this change is something that can be upstreamed.
> 
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>

RE: [PATCH] net/ring: Set mbuf->port for received packets

[Sriram Yagnaraman]

Hi Morten,

> -----Original Message-----
> From: Morten Brørup <mb@smartsharesystems.com>
> Sent: Tuesday, 18 June 2024 18:01
> To: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>
> Cc: dev@dpdk.org; Bruce Richardson <bruce.richardson@intel.com>
> Subject: RE: [PATCH] net/ring: Set mbuf->port for received packets
> 
> [Du får inte e-post ofta från mb@smartsharesystems.com. Läs om varför det
> här är viktigt på https://aka.ms/LearnAboutSenderIdentification ]
> 
> > From: Sriram Yagnaraman [mailto:sriram.yagnaraman@ericsson.com]
> >
> > When using ring based ethdev, mbuf->port is not set on received packets.
> >
> > For applications that use the mbuf->port to identify the incoming
> > port, especially when eventdev RX adapter pulls the packet on a
> > different core and the application running on a worker core has no
> > clue on the incoming port. This change adds some cycles at receive, to
> > set the port of course.
> 
> I agree that the mbuf->port field must be set before returning from
> rte_eth_rx_burst().
> 
> I'm not aware how applications use the ring based ethdev, so I might be
> asking silly questions...
> 
> How about all the other mbuf fields normally set by the PMD before
> returning from rte_eth_rx_burst()?
> 
> Is the enqueueing core supposed to set them?

I am integrating a couple of existing DPDK applications, and they use rte_eth_* API between them today. To keep the API consistent, and not make too many changes in the applications themselves, net_ring seems to be the best option. If someone has a better idea, I would love to hear.

There are no "HW" offloads when using net_ring, and IIUC there are no HW related fields that can be set at rx_burst.
So, apart from port field, I didn't see much else that needed to be set.

> 
> Or if the ring is only used for queueing packets originally received (at a
> physical port) by the enqueueing core, why not keep the mbuf->port value
> from the original reception?

In my case the enqueuing application originates the flow from SW, so the rte_mbuf does not come from a NIC.

> 
> >
> > Please advise if this change is something that can be upstreamed.
> >
> > Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>

RE: [PATCH] net/ring: Set mbuf->port for received packets

[Morten Brørup]

> From: Sriram Yagnaraman [mailto:sriram.yagnaraman@ericsson.com]
> 
> Hi Morten,
> 
> > From: Morten Brørup <mb@smartsharesystems.com>
> >
> > > From: Sriram Yagnaraman [mailto:sriram.yagnaraman@ericsson.com]
> > >
> > > When using ring based ethdev, mbuf->port is not set on received packets.
> > >
> > > For applications that use the mbuf->port to identify the incoming
> > > port, especially when eventdev RX adapter pulls the packet on a
> > > different core and the application running on a worker core has no
> > > clue on the incoming port. This change adds some cycles at receive, to
> > > set the port of course.
> >
> > I agree that the mbuf->port field must be set before returning from
> > rte_eth_rx_burst().
> >
> > I'm not aware how applications use the ring based ethdev, so I might be
> > asking silly questions...
> >
> > How about all the other mbuf fields normally set by the PMD before
> > returning from rte_eth_rx_burst()?
> >
> > Is the enqueueing core supposed to set them?
> 
> I am integrating a couple of existing DPDK applications, and they use
> rte_eth_* API between them today. To keep the API consistent, and not make too
> many changes in the applications themselves, net_ring seems to be the best
> option. If someone has a better idea, I would love to hear.
> 
> There are no "HW" offloads when using net_ring, and IIUC there are no HW
> related fields that can be set at rx_burst.
> So, apart from port field, I didn't see much else that needed to be set.
> 
> >
> > Or if the ring is only used for queueing packets originally received (at a
> > physical port) by the enqueueing core, why not keep the mbuf->port value
> > from the original reception?
> 
> In my case the enqueuing application originates the flow from SW, so the
> rte_mbuf does not come from a NIC.

Thank you for the explanation.

Acked-by: Morten Brørup <mb@smartsharesystems.com>

> 
> >
> > >
> > > Please advise if this change is something that can be upstreamed.
> > >
> > > Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>