* [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf
@ 2024-05-10 10:03 Ciara Loftus
2024-05-10 10:03 ` [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic Ciara Loftus
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Ciara Loftus @ 2024-05-10 10:03 UTC (permalink / raw)
To: dev; +Cc: stephen, Ciara Loftus, stable
Record the port id in the af_xdp rx queue structure and use it
to set the port id of the mbuf of a received packed.
Bugzilla ID: 1428
Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
cc: stable@dpdk.og
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
drivers/net/af_xdp/rte_eth_af_xdp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 268a130c49..fee0d5d5f3 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -131,6 +131,7 @@ struct pkt_rx_queue {
struct xsk_umem_info *umem;
struct xsk_socket *xsk;
struct rte_mempool *mb_pool;
+ uint16_t port;
struct rx_stats stats;
@@ -360,6 +361,7 @@ af_xdp_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
bufs[i]->data_off = offset - sizeof(struct rte_mbuf) -
rte_pktmbuf_priv_size(umem->mb_pool) -
umem->mb_pool->header_size;
+ bufs[i]->port = rxq->port;
rte_pktmbuf_pkt_len(bufs[i]) = len;
rte_pktmbuf_data_len(bufs[i]) = len;
@@ -426,6 +428,7 @@ af_xdp_rx_cp(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
rte_pktmbuf_data_len(mbufs[i]) = len;
rx_bytes += len;
bufs[i] = mbufs[i];
+ bufs[i]->port = rxq->port;
}
xsk_ring_cons__release(rx, nb_pkts);
@@ -1779,6 +1782,8 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
process_private->rxq_xsk_fds[rx_queue_id] = rxq->fds[0].fd;
+ rxq->port = dev->data->port_id;
+
dev->data->rx_queues[rx_queue_id] = rxq;
return 0;
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic
2024-05-10 10:03 [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf Ciara Loftus
@ 2024-05-10 10:03 ` Ciara Loftus
2024-05-10 12:35 ` Maryam Tahhan
2024-05-10 15:06 ` Stephen Hemminger
2024-05-10 10:03 ` [PATCH 3/3] net/af_xdp: Fix stats reset Ciara Loftus
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Ciara Loftus @ 2024-05-10 10:03 UTC (permalink / raw)
To: dev; +Cc: stephen, Ciara Loftus, stable
Failures to allocate mbufs in the receive path were not being
accounted for in the ethdev statistics. Fix this.
Bugzilla ID: 1429
Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
cc: stable@dpdk.og
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
drivers/net/af_xdp/rte_eth_af_xdp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index fee0d5d5f3..968bbf6d45 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -124,6 +124,7 @@ struct rx_stats {
uint64_t rx_pkts;
uint64_t rx_bytes;
uint64_t rx_dropped;
+ uint64_t alloc_failed;
};
struct pkt_rx_queue {
@@ -339,6 +340,8 @@ af_xdp_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
* xsk_ring_cons__peek
*/
rx->cached_cons -= nb_pkts;
+ rxq->stats.alloc_failed += nb_pkts;
+
return 0;
}
@@ -408,6 +411,7 @@ af_xdp_rx_cp(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
* xsk_ring_cons__peek
*/
rx->cached_cons -= nb_pkts;
+ rxq->stats.alloc_failed += nb_pkts;
return 0;
}
@@ -872,6 +876,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
stats->ibytes += stats->q_ibytes[i];
stats->imissed += rxq->stats.rx_dropped;
stats->oerrors += txq->stats.tx_dropped;
+ dev->data->rx_mbuf_alloc_failed += rxq->stats.alloc_failed;
fd = process_private->rxq_xsk_fds[i];
ret = fd >= 0 ? getsockopt(fd, SOL_XDP, XDP_STATISTICS,
&xdp_stats, &optlen) : -1;
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] net/af_xdp: Fix stats reset
2024-05-10 10:03 [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf Ciara Loftus
2024-05-10 10:03 ` [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic Ciara Loftus
@ 2024-05-10 10:03 ` Ciara Loftus
2024-05-10 12:36 ` Maryam Tahhan
2024-05-10 12:20 ` [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf Maryam Tahhan
2024-05-10 14:58 ` Stephen Hemminger
3 siblings, 1 reply; 11+ messages in thread
From: Ciara Loftus @ 2024-05-10 10:03 UTC (permalink / raw)
To: dev; +Cc: stephen, Ciara Loftus, stable
The imissed statistic was not properly reset because it was
read directly from the kernel statistics. To fix this, take note
of the kernel statistic when the stats are reset and deduct this
value from the kernel statistic read during statistics get.
Bugzilla ID: 1430
Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
cc: stable@dpdk.og
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
drivers/net/af_xdp/rte_eth_af_xdp.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 968bbf6d45..8f25134003 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -125,6 +125,8 @@ struct rx_stats {
uint64_t rx_bytes;
uint64_t rx_dropped;
uint64_t alloc_failed;
+
+ uint64_t imissed_offset;
};
struct pkt_rx_queue {
@@ -884,7 +886,8 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
AF_XDP_LOG(ERR, "getsockopt() failed for XDP_STATISTICS.\n");
return -1;
}
- stats->imissed += xdp_stats.rx_dropped;
+ stats->imissed +=
+ (xdp_stats.rx_dropped - internals->rx_queues[i].stats.imissed_offset);
stats->opackets += stats->q_opackets[i];
stats->obytes += stats->q_obytes[i];
@@ -897,13 +900,25 @@ static int
eth_stats_reset(struct rte_eth_dev *dev)
{
struct pmd_internals *internals = dev->data->dev_private;
- int i;
+ struct pmd_process_private *process_private = dev->process_private;
+ struct xdp_statistics xdp_stats;
+ socklen_t optlen;
+ int i, ret, fd;
for (i = 0; i < internals->queue_cnt; i++) {
memset(&internals->rx_queues[i].stats, 0,
sizeof(struct rx_stats));
memset(&internals->tx_queues[i].stats, 0,
sizeof(struct tx_stats));
+ fd = process_private->rxq_xsk_fds[i];
+ optlen = sizeof(struct xdp_statistics);
+ ret = fd >= 0 ? getsockopt(fd, SOL_XDP, XDP_STATISTICS,
+ &xdp_stats, &optlen) : -1;
+ if (ret != 0) {
+ AF_XDP_LOG(ERR, "getsockopt() failed for XDP_STATISTICS.\n");
+ return -1;
+ }
+ internals->rx_queues[i].stats.imissed_offset = xdp_stats.rx_dropped;
}
return 0;
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf
2024-05-10 10:03 [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf Ciara Loftus
2024-05-10 10:03 ` [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic Ciara Loftus
2024-05-10 10:03 ` [PATCH 3/3] net/af_xdp: Fix stats reset Ciara Loftus
@ 2024-05-10 12:20 ` Maryam Tahhan
2024-05-10 14:58 ` Stephen Hemminger
3 siblings, 0 replies; 11+ messages in thread
From: Maryam Tahhan @ 2024-05-10 12:20 UTC (permalink / raw)
To: dev
On 10/05/2024 11:03, Ciara Loftus wrote:
> Record the port id in the af_xdp rx queue structure and use it
> to set the port id of the mbuf of a received packed.
>
> Bugzilla ID: 1428
> Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
> cc: stable@dpdk.og
>
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
Acked-by:Maryam Tahhan <mtahhan@redhat.com>
> ---
> drivers/net/af_xdp/rte_eth_af_xdp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 268a130c49..fee0d5d5f3 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -131,6 +131,7 @@ struct pkt_rx_queue {
> struct xsk_umem_info *umem;
> struct xsk_socket *xsk;
> struct rte_mempool *mb_pool;
> + uint16_t port;
>
> struct rx_stats stats;
>
> @@ -360,6 +361,7 @@ af_xdp_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> bufs[i]->data_off = offset - sizeof(struct rte_mbuf) -
> rte_pktmbuf_priv_size(umem->mb_pool) -
> umem->mb_pool->header_size;
> + bufs[i]->port = rxq->port;
>
> rte_pktmbuf_pkt_len(bufs[i]) = len;
> rte_pktmbuf_data_len(bufs[i]) = len;
> @@ -426,6 +428,7 @@ af_xdp_rx_cp(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> rte_pktmbuf_data_len(mbufs[i]) = len;
> rx_bytes += len;
> bufs[i] = mbufs[i];
> + bufs[i]->port = rxq->port;
> }
>
> xsk_ring_cons__release(rx, nb_pkts);
> @@ -1779,6 +1782,8 @@ eth_rx_queue_setup(struct rte_eth_dev *dev,
>
> process_private->rxq_xsk_fds[rx_queue_id] = rxq->fds[0].fd;
>
> + rxq->port = dev->data->port_id;
> +
> dev->data->rx_queues[rx_queue_id] = rxq;
> return 0;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic
2024-05-10 10:03 ` [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic Ciara Loftus
@ 2024-05-10 12:35 ` Maryam Tahhan
2024-05-10 15:06 ` Stephen Hemminger
1 sibling, 0 replies; 11+ messages in thread
From: Maryam Tahhan @ 2024-05-10 12:35 UTC (permalink / raw)
To: dev
On 10/05/2024 11:03, Ciara Loftus wrote:
> Failures to allocate mbufs in the receive path were not being
> accounted for in the ethdev statistics. Fix this.
>
> Bugzilla ID: 1429
> Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
> cc: stable@dpdk.og
>
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
Acked-by: Maryam Tahhan <mtahhan@redhat.com>
> ---
> drivers/net/af_xdp/rte_eth_af_xdp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index fee0d5d5f3..968bbf6d45 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -124,6 +124,7 @@ struct rx_stats {
> uint64_t rx_pkts;
> uint64_t rx_bytes;
> uint64_t rx_dropped;
> + uint64_t alloc_failed;
> };
>
> struct pkt_rx_queue {
> @@ -339,6 +340,8 @@ af_xdp_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> * xsk_ring_cons__peek
> */
> rx->cached_cons -= nb_pkts;
> + rxq->stats.alloc_failed += nb_pkts;
> +
> return 0;
> }
>
> @@ -408,6 +411,7 @@ af_xdp_rx_cp(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
> * xsk_ring_cons__peek
> */
> rx->cached_cons -= nb_pkts;
> + rxq->stats.alloc_failed += nb_pkts;
> return 0;
> }
>
> @@ -872,6 +876,7 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
> stats->ibytes += stats->q_ibytes[i];
> stats->imissed += rxq->stats.rx_dropped;
> stats->oerrors += txq->stats.tx_dropped;
> + dev->data->rx_mbuf_alloc_failed += rxq->stats.alloc_failed;
> fd = process_private->rxq_xsk_fds[i];
> ret = fd >= 0 ? getsockopt(fd, SOL_XDP, XDP_STATISTICS,
> &xdp_stats, &optlen) : -1;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] net/af_xdp: Fix stats reset
2024-05-10 10:03 ` [PATCH 3/3] net/af_xdp: Fix stats reset Ciara Loftus
@ 2024-05-10 12:36 ` Maryam Tahhan
0 siblings, 0 replies; 11+ messages in thread
From: Maryam Tahhan @ 2024-05-10 12:36 UTC (permalink / raw)
To: dev
On 10/05/2024 11:03, Ciara Loftus wrote:
> The imissed statistic was not properly reset because it was
> read directly from the kernel statistics. To fix this, take note
> of the kernel statistic when the stats are reset and deduct this
> value from the kernel statistic read during statistics get.
>
> Bugzilla ID: 1430
> Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
> cc: stable@dpdk.og
>
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
> drivers/net/af_xdp/rte_eth_af_xdp.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 968bbf6d45..8f25134003 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -125,6 +125,8 @@ struct rx_stats {
> uint64_t rx_bytes;
> uint64_t rx_dropped;
> uint64_t alloc_failed;
> +
> + uint64_t imissed_offset;
> };
Minor nit (sorry) probably don't need the empty line between the
`uint64_t alloc_failed` and `uint64_t imissed_offset`
otherwise LGTM
Acked-by: Maryam Tahhan <mtahhan@redhat.com>
>
> struct pkt_rx_queue {
> @@ -884,7 +886,8 @@ eth_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
> AF_XDP_LOG(ERR, "getsockopt() failed for XDP_STATISTICS.\n");
> return -1;
> }
> - stats->imissed += xdp_stats.rx_dropped;
> + stats->imissed +=
> + (xdp_stats.rx_dropped - internals->rx_queues[i].stats.imissed_offset);
>
> stats->opackets += stats->q_opackets[i];
> stats->obytes += stats->q_obytes[i];
> @@ -897,13 +900,25 @@ static int
> eth_stats_reset(struct rte_eth_dev *dev)
> {
> struct pmd_internals *internals = dev->data->dev_private;
> - int i;
> + struct pmd_process_private *process_private = dev->process_private;
> + struct xdp_statistics xdp_stats;
> + socklen_t optlen;
> + int i, ret, fd;
>
> for (i = 0; i < internals->queue_cnt; i++) {
> memset(&internals->rx_queues[i].stats, 0,
> sizeof(struct rx_stats));
> memset(&internals->tx_queues[i].stats, 0,
> sizeof(struct tx_stats));
> + fd = process_private->rxq_xsk_fds[i];
> + optlen = sizeof(struct xdp_statistics);
> + ret = fd >= 0 ? getsockopt(fd, SOL_XDP, XDP_STATISTICS,
> + &xdp_stats, &optlen) : -1;
> + if (ret != 0) {
> + AF_XDP_LOG(ERR, "getsockopt() failed for XDP_STATISTICS.\n");
> + return -1;
> + }
> + internals->rx_queues[i].stats.imissed_offset = xdp_stats.rx_dropped;
> }
>
> return 0;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf
2024-05-10 10:03 [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf Ciara Loftus
` (2 preceding siblings ...)
2024-05-10 12:20 ` [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf Maryam Tahhan
@ 2024-05-10 14:58 ` Stephen Hemminger
3 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2024-05-10 14:58 UTC (permalink / raw)
To: Ciara Loftus; +Cc: dev, stable
On Fri, 10 May 2024 10:03:56 +0000
Ciara Loftus <ciara.loftus@intel.com> wrote:
> Record the port id in the af_xdp rx queue structure and use it
> to set the port id of the mbuf of a received packed.
>
> Bugzilla ID: 1428
> Fixes: f1debd77efaf ("net/af_xdp: introduce AF_XDP PMD")
> cc: stable@dpdk.og
>
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic
2024-05-10 10:03 ` [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic Ciara Loftus
2024-05-10 12:35 ` Maryam Tahhan
@ 2024-05-10 15:06 ` Stephen Hemminger
2024-05-13 8:23 ` Maryam Tahhan
2024-05-14 8:37 ` Loftus, Ciara
1 sibling, 2 replies; 11+ messages in thread
From: Stephen Hemminger @ 2024-05-10 15:06 UTC (permalink / raw)
To: Ciara Loftus; +Cc: dev, stable
On Fri, 10 May 2024 10:03:57 +0000
Ciara Loftus <ciara.loftus@intel.com> wrote:
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index fee0d5d5f3..968bbf6d45 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -124,6 +124,7 @@ struct rx_stats {
> uint64_t rx_pkts;
> uint64_t rx_bytes;
> uint64_t rx_dropped;
> + uint64_t alloc_failed;
> };
You don't have to use local statistic for this, there already is one in the dev struct
i.e dev->data->rx_mbuf_alloc_failed. The problem is you need the DPDK port number to find
what dev is.
And the code in ethdev for stats get will put it in the right place.
PS: what is the point of rxq->stats.rx_dropped? It is never incremented.
PPS: Looks like AF_XDP considers kernel full as an error (ie tx_dropped gets counted as error).
This is not what real hardware does.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic
2024-05-10 15:06 ` Stephen Hemminger
@ 2024-05-13 8:23 ` Maryam Tahhan
2024-05-13 15:53 ` Stephen Hemminger
2024-05-14 8:37 ` Loftus, Ciara
1 sibling, 1 reply; 11+ messages in thread
From: Maryam Tahhan @ 2024-05-13 8:23 UTC (permalink / raw)
To: dev
[-- Attachment #1: Type: text/plain, Size: 469 bytes --]
On 10/05/2024 16:06, Stephen Hemminger wrote:
> You don't have to use local statistic for this, there already is one in the dev struct
> i.e dev->data->rx_mbuf_alloc_failed. The problem is you need the DPDK port number to find
> what dev is.
I think the diff id that dev->data->rx_mbuf_alloc_failed would reflect
the pure HW(NIC) failed allocations where as the local alloc_failed
would reflect the failed allocations on the xdp side. Both should be
accounted for.
[-- Attachment #2: Type: text/html, Size: 851 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic
2024-05-13 8:23 ` Maryam Tahhan
@ 2024-05-13 15:53 ` Stephen Hemminger
0 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2024-05-13 15:53 UTC (permalink / raw)
To: Maryam Tahhan; +Cc: dev
On Mon, 13 May 2024 09:23:08 +0100
Maryam Tahhan <mtahhan@redhat.com> wrote:
> On 10/05/2024 16:06, Stephen Hemminger wrote:
> > You don't have to use local statistic for this, there already is one in the dev struct
> > i.e dev->data->rx_mbuf_alloc_failed. The problem is you need the DPDK port number to find
> > what dev is.
>
> I think the diff id that dev->data->rx_mbuf_alloc_failed would reflect
> the pure HW(NIC) failed allocations where as the local alloc_failed
> would reflect the failed allocations on the xdp side. Both should be
> accounted for.
Since this is not the fast path, why not just increment the global statistic.
This is a SW driver, but it can increment statistics just like a HW NIC driver.
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic
2024-05-10 15:06 ` Stephen Hemminger
2024-05-13 8:23 ` Maryam Tahhan
@ 2024-05-14 8:37 ` Loftus, Ciara
1 sibling, 0 replies; 11+ messages in thread
From: Loftus, Ciara @ 2024-05-14 8:37 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev
>
> On Fri, 10 May 2024 10:03:57 +0000
> Ciara Loftus <ciara.loftus@intel.com> wrote:
>
> > diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c
> b/drivers/net/af_xdp/rte_eth_af_xdp.c
> > index fee0d5d5f3..968bbf6d45 100644
> > --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> > +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> > @@ -124,6 +124,7 @@ struct rx_stats {
> > uint64_t rx_pkts;
> > uint64_t rx_bytes;
> > uint64_t rx_dropped;
> > + uint64_t alloc_failed;
> > };
>
> You don't have to use local statistic for this, there already is one in the dev
> struct
> i.e dev->data->rx_mbuf_alloc_failed. The problem is you need the DPDK port
> number to find
> what dev is.
We now have the port number from the first patch in this series so that's no longer an issue.
>
> And the code in ethdev for stats get will put it in the right place.
>
>
> PS: what is the point of rxq->stats.rx_dropped? It is never incremented.
Looks pointless indeed. Will add another patch to the series and remove it.
>
> PPS: Looks like AF_XDP considers kernel full as an error (ie tx_dropped gets
> counted as error).
> This is not what real hardware does.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-05-14 8:37 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-10 10:03 [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf Ciara Loftus
2024-05-10 10:03 ` [PATCH 2/3] net/af_xdp: Fix mbuf alloc failed statistic Ciara Loftus
2024-05-10 12:35 ` Maryam Tahhan
2024-05-10 15:06 ` Stephen Hemminger
2024-05-13 8:23 ` Maryam Tahhan
2024-05-13 15:53 ` Stephen Hemminger
2024-05-14 8:37 ` Loftus, Ciara
2024-05-10 10:03 ` [PATCH 3/3] net/af_xdp: Fix stats reset Ciara Loftus
2024-05-10 12:36 ` Maryam Tahhan
2024-05-10 12:20 ` [PATCH 1/3] net/af_xdp: Fix port id not set in rx mbuf Maryam Tahhan
2024-05-10 14:58 ` Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).