DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] mbuf: dump Tx offload metadata
@ 2025-10-31 13:11 David Marchand
  2025-10-31 13:28 ` Morten Brørup
  2025-11-04 10:02 ` [PATCH v2] " David Marchand
  0 siblings, 2 replies; 9+ messages in thread
From: David Marchand @ 2025-10-31 13:11 UTC (permalink / raw)
  To: dev; +Cc: Morten Brørup

When debugging TSO and other checksum offloads, dumping the various
l*_len fields and TSO segmentation size can be helpful.

Example in OVS that dumps a mbuf on rte_eth_tx_prepare failure:

Before:
netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864
  pkt_len=6804, ol_flags=0x3114800000000102, nb_segs=1, port=65535, ptype=0
  segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50, refcnt=1
  Dump data at [0x22008005b2], len=6804
...

After:
netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864, pkt_len=6804
  outer_l2_len=14, outer_l3_len=40, l2_len=38, l3_len=40, l4_len=32, tso_segsz=0
  ol_flags=0x3114800000000102, nb_segs=1, port=65535, ptype=0
  segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50, refcnt=1
  Dump data at [0x22008005b2], len=6804
...

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/mbuf/rte_mbuf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
index 0d931c7a15..74dcb6c934 100644
--- a/lib/mbuf/rte_mbuf.c
+++ b/lib/mbuf/rte_mbuf.c
@@ -750,10 +750,12 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len)
 
 	__rte_mbuf_sanity_check(m, 1);
 
-	fprintf(f, "dump mbuf at %p, iova=%#" PRIx64 ", buf_len=%u\n", m, rte_mbuf_iova_get(m),
-		m->buf_len);
-	fprintf(f, "  pkt_len=%u, ol_flags=%#"PRIx64", nb_segs=%u, port=%u",
-		m->pkt_len, m->ol_flags, m->nb_segs, m->port);
+	fprintf(f, "dump mbuf at %p, iova=%#" PRIx64 ", buf_len=%u, pkt_len=%u\n",
+		m, rte_mbuf_iova_get(m), m->buf_len, m->pkt_len);
+	fprintf(f, "  outer_l2_len=%u, outer_l3_len=%u, l2_len=%u, l3_len=%u, l4_len=%u, tso_segsz=%u\n",
+		m->outer_l2_len, m->outer_l3_len, m->l2_len, m->l3_len, m->l4_len, m->tso_segsz);
+	fprintf(f, "  ol_flags=%#"PRIx64", nb_segs=%u, port=%u",
+		m->ol_flags, m->nb_segs, m->port);
 
 	if (m->ol_flags & (RTE_MBUF_F_RX_QINQ | RTE_MBUF_F_TX_QINQ))
 		fprintf(f, ", vlan_tci_outer=%u", m->vlan_tci_outer);
-- 
2.51.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] mbuf: dump Tx offload metadata
  2025-10-31 13:11 [PATCH] mbuf: dump Tx offload metadata David Marchand
@ 2025-10-31 13:28 ` Morten Brørup
  2025-10-31 14:07   ` David Marchand
  2025-11-04 10:02 ` [PATCH v2] " David Marchand
  1 sibling, 1 reply; 9+ messages in thread
From: Morten Brørup @ 2025-10-31 13:28 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Friday, 31 October 2025 14.12
> 
> When debugging TSO and other checksum offloads, dumping the various
> l*_len fields and TSO segmentation size can be helpful.
> 
> Example in OVS that dumps a mbuf on rte_eth_tx_prepare failure:
> 
> Before:
> netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
> dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864
>   pkt_len=6804, ol_flags=0x3114800000000102, nb_segs=1, port=65535,
> ptype=0
>   segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50,
> refcnt=1
>   Dump data at [0x22008005b2], len=6804
> ...
> 
> After:
> netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
> dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864,
> pkt_len=6804
>   outer_l2_len=14, outer_l3_len=40, l2_len=38, l3_len=40, l4_len=32,
> tso_segsz=0
>   ol_flags=0x3114800000000102, nb_segs=1, port=65535, ptype=0
>   segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50,
> refcnt=1
>   Dump data at [0x22008005b2], len=6804
> ...
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Good idea.

With or without suggested improvement inline below,
Acked-by: Morten Brørup <mb@smartsharesystems.com>

> ---
>  lib/mbuf/rte_mbuf.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
> index 0d931c7a15..74dcb6c934 100644
> --- a/lib/mbuf/rte_mbuf.c
> +++ b/lib/mbuf/rte_mbuf.c
> @@ -750,10 +750,12 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf
> *m, unsigned dump_len)
> 
>  	__rte_mbuf_sanity_check(m, 1);
> 
> -	fprintf(f, "dump mbuf at %p, iova=%#" PRIx64 ", buf_len=%u\n", m,
> rte_mbuf_iova_get(m),
> -		m->buf_len);
> -	fprintf(f, "  pkt_len=%u, ol_flags=%#"PRIx64", nb_segs=%u,
> port=%u",
> -		m->pkt_len, m->ol_flags, m->nb_segs, m->port);
> +	fprintf(f, "dump mbuf at %p, iova=%#" PRIx64 ", buf_len=%u,
> pkt_len=%u\n",
> +		m, rte_mbuf_iova_get(m), m->buf_len, m->pkt_len);
> +	fprintf(f, "  outer_l2_len=%u, outer_l3_len=%u, l2_len=%u,
> l3_len=%u, l4_len=%u, tso_segsz=%u\n",
> +		m->outer_l2_len, m->outer_l3_len, m->l2_len, m->l3_len, m-
> >l4_len, m->tso_segsz);

Printing of outer_l2_len and outer_l3_len could be omitted if not present, like the VLAN IDs, e.g.:

fprintf(f, "  ");
if (m->outer_l2_len != 0)
	fprintf(f, "outer_l2_len=%u, ", m->outer_l2_len);
if (m->outer_l3_len != 0)
	fprintf(f, "outer_l3_len=%u, ", m->outer_l3_len);
fprintf(f, "l2_len=%u, l3_len=%u, l4_len=%u, tso_segsz=%u\n",
	m->l2_len, m->l3_len, m->l4_len, m->tso_segsz);

> +	fprintf(f, "  ol_flags=%#"PRIx64", nb_segs=%u, port=%u",
> +		m->ol_flags, m->nb_segs, m->port);
> 
>  	if (m->ol_flags & (RTE_MBUF_F_RX_QINQ | RTE_MBUF_F_TX_QINQ))
>  		fprintf(f, ", vlan_tci_outer=%u", m->vlan_tci_outer);
> --
> 2.51.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mbuf: dump Tx offload metadata
  2025-10-31 13:28 ` Morten Brørup
@ 2025-10-31 14:07   ` David Marchand
  2025-10-31 14:29     ` Morten Brørup
  0 siblings, 1 reply; 9+ messages in thread
From: David Marchand @ 2025-10-31 14:07 UTC (permalink / raw)
  To: Morten Brørup; +Cc: dev

On Fri, 31 Oct 2025 at 14:28, Morten Brørup <mb@smartsharesystems.com> wrote:
> Printing of outer_l2_len and outer_l3_len could be omitted if not present, like the VLAN IDs, e.g.:
>
> fprintf(f, "  ");
> if (m->outer_l2_len != 0)
>         fprintf(f, "outer_l2_len=%u, ", m->outer_l2_len);
> if (m->outer_l3_len != 0)
>         fprintf(f, "outer_l3_len=%u, ", m->outer_l3_len);
> fprintf(f, "l2_len=%u, l3_len=%u, l4_len=%u, tso_segsz=%u\n",
>         m->l2_len, m->l3_len, m->l4_len, m->tso_segsz);

Mm, why specifically those two only?
If the intention is to make this debug output shorter, I would apply
the same to all those fields, and omit the whole (new) line if no
field is set.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] mbuf: dump Tx offload metadata
  2025-10-31 14:07   ` David Marchand
@ 2025-10-31 14:29     ` Morten Brørup
  2025-11-04  7:43       ` David Marchand
  0 siblings, 1 reply; 9+ messages in thread
From: Morten Brørup @ 2025-10-31 14:29 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Friday, 31 October 2025 15.08
> 
> On Fri, 31 Oct 2025 at 14:28, Morten Brørup <mb@smartsharesystems.com>
> wrote:
> > Printing of outer_l2_len and outer_l3_len could be omitted if not
> present, like the VLAN IDs, e.g.:
> >
> > fprintf(f, "  ");
> > if (m->outer_l2_len != 0)
> >         fprintf(f, "outer_l2_len=%u, ", m->outer_l2_len);
> > if (m->outer_l3_len != 0)
> >         fprintf(f, "outer_l3_len=%u, ", m->outer_l3_len);
> > fprintf(f, "l2_len=%u, l3_len=%u, l4_len=%u, tso_segsz=%u\n",
> >         m->l2_len, m->l3_len, m->l4_len, m->tso_segsz);
> 
> Mm, why specifically those two only?
> If the intention is to make this debug output shorter, I would apply
> the same to all those fields, and omit the whole (new) line if no
> field is set.

The intention is not do make the output shorter, but to reduce the amount of "noise" (useless data) in the output.
My suggestion omits noise for non-tunneled packets.

Maybe tso_segsz could be conditional on non-zero too.

It depends on what the user/developer is debugging, whether l2/l3/l4_len are considered noise. I don't object to making them conditional too, if you prefer.

Anyway, I just realized that the tx_offload field is not cleared on RX, so the discussed zero-check is useless (checking uninitialized data) when dumping an mbuf at an early stage in the application's packet processing pipeline.
I have an idea to get around this... The TX flags in ol_flags are cleared at RX, so we can consider tx_offload uninitialized until one of the TX flags in ol_flags has been set.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] mbuf: dump Tx offload metadata
  2025-10-31 14:29     ` Morten Brørup
@ 2025-11-04  7:43       ` David Marchand
  0 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2025-11-04  7:43 UTC (permalink / raw)
  To: Morten Brørup; +Cc: dev

On Fri, 31 Oct 2025 at 15:29, Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > From: David Marchand [mailto:david.marchand@redhat.com]
> > Sent: Friday, 31 October 2025 15.08
> >
> > On Fri, 31 Oct 2025 at 14:28, Morten Brørup <mb@smartsharesystems.com>
> > wrote:
> > > Printing of outer_l2_len and outer_l3_len could be omitted if not
> > present, like the VLAN IDs, e.g.:
> > >
> > > fprintf(f, "  ");
> > > if (m->outer_l2_len != 0)
> > >         fprintf(f, "outer_l2_len=%u, ", m->outer_l2_len);
> > > if (m->outer_l3_len != 0)
> > >         fprintf(f, "outer_l3_len=%u, ", m->outer_l3_len);
> > > fprintf(f, "l2_len=%u, l3_len=%u, l4_len=%u, tso_segsz=%u\n",
> > >         m->l2_len, m->l3_len, m->l4_len, m->tso_segsz);
> >
> > Mm, why specifically those two only?
> > If the intention is to make this debug output shorter, I would apply
> > the same to all those fields, and omit the whole (new) line if no
> > field is set.
>
> The intention is not do make the output shorter, but to reduce the amount of "noise" (useless data) in the output.
> My suggestion omits noise for non-tunneled packets.
>
> Maybe tso_segsz could be conditional on non-zero too.
>
> It depends on what the user/developer is debugging, whether l2/l3/l4_len are considered noise. I don't object to making them conditional too, if you prefer.
>
> Anyway, I just realized that the tx_offload field is not cleared on RX, so the discussed zero-check is useless (checking uninitialized data) when dumping an mbuf at an early stage in the application's packet processing pipeline.
> I have an idea to get around this... The TX flags in ol_flags are cleared at RX, so we can consider tx_offload uninitialized until one of the TX flags in ol_flags has been set.

LGTM, it is stated that those fields are for Tx offloads.
I'll post a v2.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] mbuf: dump Tx offload metadata
  2025-10-31 13:11 [PATCH] mbuf: dump Tx offload metadata David Marchand
  2025-10-31 13:28 ` Morten Brørup
@ 2025-11-04 10:02 ` David Marchand
  2025-11-04 11:45   ` Morten Brørup
  2025-11-04 13:58   ` David Marchand
  1 sibling, 2 replies; 9+ messages in thread
From: David Marchand @ 2025-11-04 10:02 UTC (permalink / raw)
  To: dev; +Cc: Morten Brørup

When debugging TSO and other checksum offloads, dumping the various
l*_len fields and TSO segmentation size can be helpful.

Example in OVS that dumps a mbuf on rte_eth_tx_prepare failure:

Before:
netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864
  pkt_len=6804, ol_flags=0x3114800000000102, nb_segs=1, port=65535, ptype=0
  segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50, refcnt=1
  Dump data at [0x22008005b2], len=6804
...

After:
netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864, pkt_len=6804
  outer_l2_len=14, outer_l3_len=40, l2_len=38, l3_len=40, l4_len=32
  ol_flags=0x3114800000000102, nb_segs=1, port=65535, ptype=0
  segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50, refcnt=1
  Dump data at [0x22008005b2], len=6804
...

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/mbuf/rte_mbuf.c | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
index 0d931c7a15..12710a59df 100644
--- a/lib/mbuf/rte_mbuf.c
+++ b/lib/mbuf/rte_mbuf.c
@@ -750,10 +750,43 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len)
 
 	__rte_mbuf_sanity_check(m, 1);
 
-	fprintf(f, "dump mbuf at %p, iova=%#" PRIx64 ", buf_len=%u\n", m, rte_mbuf_iova_get(m),
-		m->buf_len);
-	fprintf(f, "  pkt_len=%u, ol_flags=%#"PRIx64", nb_segs=%u, port=%u",
-		m->pkt_len, m->ol_flags, m->nb_segs, m->port);
+	fprintf(f, "dump mbuf at %p, iova=%#" PRIx64 ", buf_len=%u, pkt_len=%u\n",
+		m, rte_mbuf_iova_get(m), m->buf_len, m->pkt_len);
+	if (m->ol_flags & RTE_MBUF_F_TX_OFFLOAD_MASK) {
+		const char *sep = "";
+
+		fprintf(f, "  ");
+		if (m->outer_l2_len != 0) {
+			fprintf(f, "%souter_l2_len=%u", sep, m->outer_l2_len);
+			sep = ", ";
+		}
+		if (m->outer_l3_len != 0) {
+			fprintf(f, "%souter_l3_len=%u", sep, m->outer_l3_len);
+			sep = ", ";
+		}
+		if (m->l2_len != 0) {
+			fprintf(f, "%sl2_len=%u", sep, m->l2_len);
+			sep = ", ";
+		}
+		if (m->l3_len != 0) {
+			fprintf(f, "%sl3_len=%u", sep, m->l3_len);
+			sep = ", ";
+		}
+		if (m->l4_len != 0) {
+			fprintf(f, "%sl4_len=%u", sep, m->l4_len);
+			sep = ", ";
+		}
+		if (m->tso_segsz != 0) {
+			fprintf(f, "%stso_segsz=%u", sep, m->tso_segsz);
+			sep = ", ";
+		}
+
+		if (sep[0] == '\0')
+			fprintf(f, "no tx offload length set");
+		fprintf(f, "\n");
+	}
+	fprintf(f, "  ol_flags=%#"PRIx64", nb_segs=%u, port=%u",
+		m->ol_flags, m->nb_segs, m->port);
 
 	if (m->ol_flags & (RTE_MBUF_F_RX_QINQ | RTE_MBUF_F_TX_QINQ))
 		fprintf(f, ", vlan_tci_outer=%u", m->vlan_tci_outer);
-- 
2.51.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2] mbuf: dump Tx offload metadata
  2025-11-04 10:02 ` [PATCH v2] " David Marchand
@ 2025-11-04 11:45   ` Morten Brørup
  2025-11-04 13:58   ` David Marchand
  1 sibling, 0 replies; 9+ messages in thread
From: Morten Brørup @ 2025-11-04 11:45 UTC (permalink / raw)
  To: David Marchand, dev

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Tuesday, 4 November 2025 11.02
> 
> When debugging TSO and other checksum offloads, dumping the various
> l*_len fields and TSO segmentation size can be helpful.
> 
> Example in OVS that dumps a mbuf on rte_eth_tx_prepare failure:
> 
> Before:
> netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
> dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864
>   pkt_len=6804, ol_flags=0x3114800000000102, nb_segs=1, port=65535,
> ptype=0
>   segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50,
> refcnt=1
>   Dump data at [0x22008005b2], len=6804
> ...
> 
> After:
> netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
> dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864,
> pkt_len=6804
>   outer_l2_len=14, outer_l3_len=40, l2_len=38, l3_len=40, l4_len=32
>   ol_flags=0x3114800000000102, nb_segs=1, port=65535, ptype=0
>   segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50,
> refcnt=1
>   Dump data at [0x22008005b2], len=6804
> ...
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mbuf: dump Tx offload metadata
  2025-11-04 10:02 ` [PATCH v2] " David Marchand
  2025-11-04 11:45   ` Morten Brørup
@ 2025-11-04 13:58   ` David Marchand
  2025-11-04 18:45     ` Morten Brørup
  1 sibling, 1 reply; 9+ messages in thread
From: David Marchand @ 2025-11-04 13:58 UTC (permalink / raw)
  To: dev; +Cc: Morten Brørup

On Tue, 4 Nov 2025 at 11:02, David Marchand <david.marchand@redhat.com> wrote:
>
> When debugging TSO and other checksum offloads, dumping the various
> l*_len fields and TSO segmentation size can be helpful.
>
> Example in OVS that dumps a mbuf on rte_eth_tx_prepare failure:
>
> Before:
> netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
> dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864
>   pkt_len=6804, ol_flags=0x3114800000000102, nb_segs=1, port=65535, ptype=0
>   segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50, refcnt=1
>   Dump data at [0x22008005b2], len=6804
> ...
>
> After:
> netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
> dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864, pkt_len=6804
>   outer_l2_len=14, outer_l3_len=40, l2_len=38, l3_len=40, l4_len=32
>   ol_flags=0x3114800000000102, nb_segs=1, port=65535, ptype=0
>   segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50, refcnt=1
>   Dump data at [0x22008005b2], len=6804
> ...
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/mbuf/rte_mbuf.c | 41 +++++++++++++++++++++++++++++++++++++----
>  1 file changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
> index 0d931c7a15..12710a59df 100644
> --- a/lib/mbuf/rte_mbuf.c
> +++ b/lib/mbuf/rte_mbuf.c
> @@ -750,10 +750,43 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len)
>
>         __rte_mbuf_sanity_check(m, 1);
>
> -       fprintf(f, "dump mbuf at %p, iova=%#" PRIx64 ", buf_len=%u\n", m, rte_mbuf_iova_get(m),
> -               m->buf_len);
> -       fprintf(f, "  pkt_len=%u, ol_flags=%#"PRIx64", nb_segs=%u, port=%u",
> -               m->pkt_len, m->ol_flags, m->nb_segs, m->port);
> +       fprintf(f, "dump mbuf at %p, iova=%#" PRIx64 ", buf_len=%u, pkt_len=%u\n",
> +               m, rte_mbuf_iova_get(m), m->buf_len, m->pkt_len);
> +       if (m->ol_flags & RTE_MBUF_F_TX_OFFLOAD_MASK) {
> +               const char *sep = "";
> +
> +               fprintf(f, "  ");
> +               if (m->outer_l2_len != 0) {
> +                       fprintf(f, "%souter_l2_len=%u", sep, m->outer_l2_len);

One more round will be needed for MSVC (CI did not report it on v1,
some tests did not run..).
../lib/mbuf/rte_mbuf.c(760): error C2220: the following warning is
treated as an error
../lib/mbuf/rte_mbuf.c(760): warning C4477: 'fprintf' : format string
'%u' requires an argument of type 'unsigned int', but variadic
argument 2 has type 'const uint64_t'
../lib/mbuf/rte_mbuf.c(760): note: consider using '%llu' in the format string
../lib/mbuf/rte_mbuf.c(760): note: consider using '%Iu' in the format string
../lib/mbuf/rte_mbuf.c(760): note: consider using '%I64u' in the format string

This is probably because of the bitfields we have for the tx_offload field.
lib/mbuf/rte_mbuf_core.h:       RTE_MBUF_L2_LEN_BITS = 7,
lib/mbuf/rte_mbuf_core.h:       RTE_MBUF_L3_LEN_BITS = 9,
lib/mbuf/rte_mbuf_core.h:       RTE_MBUF_L4_LEN_BITS = 8,
lib/mbuf/rte_mbuf_core.h:       RTE_MBUF_OUTL3_LEN_BITS = 9,
lib/mbuf/rte_mbuf_core.h:       RTE_MBUF_OUTL2_LEN_BITS = 7,
lib/mbuf/rte_mbuf_core.h:                       uint64_t
l2_len:RTE_MBUF_L2_LEN_BITS;
lib/mbuf/rte_mbuf_core.h:                       uint64_t
l3_len:RTE_MBUF_L3_LEN_BITS;
lib/mbuf/rte_mbuf_core.h:                       uint64_t
l4_len:RTE_MBUF_L4_LEN_BITS;
lib/mbuf/rte_mbuf_core.h:                       uint64_t
outer_l3_len:RTE_MBUF_OUTL3_LEN_BITS;
lib/mbuf/rte_mbuf_core.h:                       uint64_t
outer_l2_len:RTE_MBUF_OUTL2_LEN_BITS;


Passing %PRIu64 instead of %u breaks build with gcc and clang.
../lib/mbuf/rte_mbuf.c: In function ‘rte_pktmbuf_dump’:
../lib/mbuf/rte_mbuf.c:760:36: error: format ‘%lu’ expects argument of
type ‘long unsigned int’, but argument 4 has type ‘int’
[-Werror=format=]
760 | fprintf(f, "%souter_l2_len=%"PRIu64, sep, m->outer_l2_len);
| ^~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~
| |
| int


I'll go with simple casting, like:
+            fprintf(f, "%souter_l2_len=%u", sep, (unsigned
int)m->outer_l2_len);


-- 
David Marchand


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2] mbuf: dump Tx offload metadata
  2025-11-04 13:58   ` David Marchand
@ 2025-11-04 18:45     ` Morten Brørup
  0 siblings, 0 replies; 9+ messages in thread
From: Morten Brørup @ 2025-11-04 18:45 UTC (permalink / raw)
  To: David Marchand, dev

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Tuesday, 4 November 2025 14.59
> 
> On Tue, 4 Nov 2025 at 11:02, David Marchand <david.marchand@redhat.com>
> wrote:
> >
> > When debugging TSO and other checksum offloads, dumping the various
> > l*_len fields and TSO segmentation size can be helpful.
> >
> > Example in OVS that dumps a mbuf on rte_eth_tx_prepare failure:
> >
> > Before:
> > netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
> > dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864
> >   pkt_len=6804, ol_flags=0x3114800000000102, nb_segs=1, port=65535,
> ptype=0
> >   segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50,
> refcnt=1
> >   Dump data at [0x22008005b2], len=6804
> > ...
> >
> > After:
> > netdev_dpdk(pmd-c30/id:11)|DBG|dpdk0: First invalid packet:
> > dump mbuf at 0x2201a916c0, iova=0x2200800580, buf_len=6864,
> pkt_len=6804
> >   outer_l2_len=14, outer_l3_len=40, l2_len=38, l3_len=40, l4_len=32
> >   ol_flags=0x3114800000000102, nb_segs=1, port=65535, ptype=0
> >   segment at 0x2201a916c0, data=0x22008005b2, len=6804, off=50,
> refcnt=1
> >   Dump data at [0x22008005b2], len=6804
> > ...
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  lib/mbuf/rte_mbuf.c | 41 +++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 37 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
> > index 0d931c7a15..12710a59df 100644
> > --- a/lib/mbuf/rte_mbuf.c
> > +++ b/lib/mbuf/rte_mbuf.c
> > @@ -750,10 +750,43 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf
> *m, unsigned dump_len)
> >
> >         __rte_mbuf_sanity_check(m, 1);
> >
> > -       fprintf(f, "dump mbuf at %p, iova=%#" PRIx64 ",
> buf_len=%u\n", m, rte_mbuf_iova_get(m),
> > -               m->buf_len);
> > -       fprintf(f, "  pkt_len=%u, ol_flags=%#"PRIx64", nb_segs=%u,
> port=%u",
> > -               m->pkt_len, m->ol_flags, m->nb_segs, m->port);
> > +       fprintf(f, "dump mbuf at %p, iova=%#" PRIx64 ", buf_len=%u,
> pkt_len=%u\n",
> > +               m, rte_mbuf_iova_get(m), m->buf_len, m->pkt_len);
> > +       if (m->ol_flags & RTE_MBUF_F_TX_OFFLOAD_MASK) {
> > +               const char *sep = "";
> > +
> > +               fprintf(f, "  ");
> > +               if (m->outer_l2_len != 0) {
> > +                       fprintf(f, "%souter_l2_len=%u", sep, m-
> >outer_l2_len);
> 
> One more round will be needed for MSVC (CI did not report it on v1,
> some tests did not run..).
> ../lib/mbuf/rte_mbuf.c(760): error C2220: the following warning is
> treated as an error
> ../lib/mbuf/rte_mbuf.c(760): warning C4477: 'fprintf' : format string
> '%u' requires an argument of type 'unsigned int', but variadic
> argument 2 has type 'const uint64_t'
> ../lib/mbuf/rte_mbuf.c(760): note: consider using '%llu' in the format
> string
> ../lib/mbuf/rte_mbuf.c(760): note: consider using '%Iu' in the format
> string
> ../lib/mbuf/rte_mbuf.c(760): note: consider using '%I64u' in the format
> string
> 
> This is probably because of the bitfields we have for the tx_offload
> field.
> lib/mbuf/rte_mbuf_core.h:       RTE_MBUF_L2_LEN_BITS = 7,
> lib/mbuf/rte_mbuf_core.h:       RTE_MBUF_L3_LEN_BITS = 9,
> lib/mbuf/rte_mbuf_core.h:       RTE_MBUF_L4_LEN_BITS = 8,
> lib/mbuf/rte_mbuf_core.h:       RTE_MBUF_OUTL3_LEN_BITS = 9,
> lib/mbuf/rte_mbuf_core.h:       RTE_MBUF_OUTL2_LEN_BITS = 7,
> lib/mbuf/rte_mbuf_core.h:                       uint64_t
> l2_len:RTE_MBUF_L2_LEN_BITS;
> lib/mbuf/rte_mbuf_core.h:                       uint64_t
> l3_len:RTE_MBUF_L3_LEN_BITS;
> lib/mbuf/rte_mbuf_core.h:                       uint64_t
> l4_len:RTE_MBUF_L4_LEN_BITS;
> lib/mbuf/rte_mbuf_core.h:                       uint64_t
> outer_l3_len:RTE_MBUF_OUTL3_LEN_BITS;
> lib/mbuf/rte_mbuf_core.h:                       uint64_t
> outer_l2_len:RTE_MBUF_OUTL2_LEN_BITS;
> 
> 
> Passing %PRIu64 instead of %u breaks build with gcc and clang.
> ../lib/mbuf/rte_mbuf.c: In function ‘rte_pktmbuf_dump’:
> ../lib/mbuf/rte_mbuf.c:760:36: error: format ‘%lu’ expects argument of
> type ‘long unsigned int’, but argument 4 has type ‘int’
> [-Werror=format=]
> 760 | fprintf(f, "%souter_l2_len=%"PRIu64, sep, m->outer_l2_len);
> | ^~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~
> | |
> | int
> 

Interesting difference in interpretation between the compilers.

> 
> I'll go with simple casting, like:
> +            fprintf(f, "%souter_l2_len=%u", sep, (unsigned
> int)m->outer_l2_len);

Agree.

> 
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-11-04 18:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-31 13:11 [PATCH] mbuf: dump Tx offload metadata David Marchand
2025-10-31 13:28 ` Morten Brørup
2025-10-31 14:07   ` David Marchand
2025-10-31 14:29     ` Morten Brørup
2025-11-04  7:43       ` David Marchand
2025-11-04 10:02 ` [PATCH v2] " David Marchand
2025-11-04 11:45   ` Morten Brørup
2025-11-04 13:58   ` David Marchand
2025-11-04 18:45     ` Morten Brørup

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).