[PATCH] mbuf: expose outer vlan in mbuf dump

[PATCH] mbuf: expose outer vlan in mbuf dump

[Ben Magistro]

Enable printing of the outer vlan if flags indicate it is present.

Signed-off-by: Ben Magistro <koncept1@gmail.com>

---

While troubleshooting some QinQ offloads with various Intel i40e firmware[1], it was
helpful to expose the outer vlan in the dump mbuf calls.  This should be straightforward
to backport and happy to do the work if accepted. I understand that this may not be a
widely supported capability at this time, so we are okay if this is not accepted and
we just maintain a local patch.

---
 lib/mbuf/rte_mbuf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
index 87592faccb..a2307cebe6 100644
--- a/lib/mbuf/rte_mbuf.c
+++ b/lib/mbuf/rte_mbuf.c
@@ -674,6 +674,9 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_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);
 
+	if (m->ol_flags & (RTE_MBUF_F_RX_QINQ | RTE_MBUF_F_TX_QINQ))
+		fprintf(f, ", vlan_tci_outer=%u", m->vlan_tci_outer);
+
 	if (m->ol_flags & (RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_TX_VLAN))
 		fprintf(f, ", vlan_tci=%u", m->vlan_tci);
 
-- 
2.27.0

RE: [PATCH] mbuf: expose outer vlan in mbuf dump

[Morten Brørup]

> From: Ben Magistro [mailto:koncept1@gmail.com]
> Sent: Monday, 4 April 2022 02.57
> 
> Enable printing of the outer vlan if flags indicate it is present.
> 
> Signed-off-by: Ben Magistro <koncept1@gmail.com>
> 
> ---
> 
> While troubleshooting some QinQ offloads with various Intel i40e
> firmware[1], it was
> helpful to expose the outer vlan in the dump mbuf calls.  This should
> be straightforward
> to backport and happy to do the work if accepted. I understand that
> this may not be a
> widely supported capability at this time, so we are okay if this is not
> accepted and
> we just maintain a local patch.

Features are usually not backported, only bug fixes.

However, since this patch proved helpful finding a bug, and it is very simple, it could be considered by the LTS maintainers.

> 
> ---
>  lib/mbuf/rte_mbuf.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
> index 87592faccb..a2307cebe6 100644
> --- a/lib/mbuf/rte_mbuf.c
> +++ b/lib/mbuf/rte_mbuf.c
> @@ -674,6 +674,9 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m,
> unsigned dump_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);
> 
> +	if (m->ol_flags & (RTE_MBUF_F_RX_QINQ | RTE_MBUF_F_TX_QINQ))
> +		fprintf(f, ", vlan_tci_outer=%u", m->vlan_tci_outer);
> +
>  	if (m->ol_flags & (RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_TX_VLAN))
>  		fprintf(f, ", vlan_tci=%u", m->vlan_tci);
> 
> --
> 2.27.0
> 

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

Re: [PATCH] mbuf: expose outer vlan in mbuf dump

[Kevin Traynor]

On 04/04/2022 07:15, Morten Brørup wrote:
>> From: Ben Magistro [mailto:koncept1@gmail.com]
>> Sent: Monday, 4 April 2022 02.57
>>
>> Enable printing of the outer vlan if flags indicate it is present.
>>
>> Signed-off-by: Ben Magistro <koncept1@gmail.com>
>>
>> ---
>>
>> While troubleshooting some QinQ offloads with various Intel i40e
>> firmware[1], it was
>> helpful to expose the outer vlan in the dump mbuf calls.  This should
>> be straightforward
>> to backport and happy to do the work if accepted. I understand that
>> this may not be a
>> widely supported capability at this time, so we are okay if this is not
>> accepted and
>> we just maintain a local patch.
> 
> Features are usually not backported, only bug fixes.
> 
> However, since this patch proved helpful finding a bug, and it is very simple, it could be considered by the LTS maintainers.
> 


Suggest to tag the patch with 'Cc: stable@dpdk.org' to indicate it is 
requested for stable branches. It will be caught by stable maintainer 
filters when it is time for backports and can be discussed further then.

(Note, it would need small rebase for older than 21.11 LTS to account 
for the namespaces that were added)

Kevin.

>>
>> ---
>>   lib/mbuf/rte_mbuf.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
>> index 87592faccb..a2307cebe6 100644
>> --- a/lib/mbuf/rte_mbuf.c
>> +++ b/lib/mbuf/rte_mbuf.c
>> @@ -674,6 +674,9 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m,
>> unsigned dump_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);
>>
>> +	if (m->ol_flags & (RTE_MBUF_F_RX_QINQ | RTE_MBUF_F_TX_QINQ))
>> +		fprintf(f, ", vlan_tci_outer=%u", m->vlan_tci_outer);
>> +
>>   	if (m->ol_flags & (RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_TX_VLAN))
>>   		fprintf(f, ", vlan_tci=%u", m->vlan_tci);
>>
>> --
>> 2.27.0
>>
> 
> Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
> 

Re: [PATCH] mbuf: expose outer vlan in mbuf dump

[Thomas Monjalon]

04/04/2022 12:33, Kevin Traynor:
> On 04/04/2022 07:15, Morten Brørup wrote:
> >> From: Ben Magistro [mailto:koncept1@gmail.com]
> >> Sent: Monday, 4 April 2022 02.57
> >>
> >> Enable printing of the outer vlan if flags indicate it is present.
> >>
> >> Signed-off-by: Ben Magistro <koncept1@gmail.com>
> >>
> >> ---
> >>
> >> While troubleshooting some QinQ offloads with various Intel i40e
> >> firmware[1], it was
> >> helpful to expose the outer vlan in the dump mbuf calls.  This should
> >> be straightforward
> >> to backport and happy to do the work if accepted. I understand that
> >> this may not be a
> >> widely supported capability at this time, so we are okay if this is not
> >> accepted and
> >> we just maintain a local patch.
> > 
> > Features are usually not backported, only bug fixes.
> > 
> > However, since this patch proved helpful finding a bug, and it is very simple, it could be considered by the LTS maintainers.
> 
> 
> Suggest to tag the patch with 'Cc: stable@dpdk.org' to indicate it is 
> requested for stable branches. It will be caught by stable maintainer 
> filters when it is time for backports and can be discussed further then.

I think it is a bug.
What was introduced first? the function or the field?
Please find the commit where it should have been done
and mark it with "Fixes:" syntax + Cc:stable.

Re: [PATCH] mbuf: expose outer vlan in mbuf dump

[Olivier Matz]

Hi,

On Tue, Apr 05, 2022 at 05:51:05PM +0200, Thomas Monjalon wrote:
> 04/04/2022 12:33, Kevin Traynor:
> > On 04/04/2022 07:15, Morten Brørup wrote:
> > >> From: Ben Magistro [mailto:koncept1@gmail.com]
> > >> Sent: Monday, 4 April 2022 02.57
> > >>
> > >> Enable printing of the outer vlan if flags indicate it is present.
> > >>
> > >> Signed-off-by: Ben Magistro <koncept1@gmail.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

> > >>
> > >> ---
> > >>
> > >> While troubleshooting some QinQ offloads with various Intel i40e
> > >> firmware[1], it was
> > >> helpful to expose the outer vlan in the dump mbuf calls.  This should
> > >> be straightforward
> > >> to backport and happy to do the work if accepted. I understand that
> > >> this may not be a
> > >> widely supported capability at this time, so we are okay if this is not
> > >> accepted and
> > >> we just maintain a local patch.
> > > 
> > > Features are usually not backported, only bug fixes.
> > > 
> > > However, since this patch proved helpful finding a bug, and it is very simple, it could be considered by the LTS maintainers.
> > 
> > 
> > Suggest to tag the patch with 'Cc: stable@dpdk.org' to indicate it is 
> > requested for stable branches. It will be caught by stable maintainer 
> > filters when it is time for backports and can be discussed further then.
> 
> I think it is a bug.
> What was introduced first? the function or the field?
> Please find the commit where it should have been done
> and mark it with "Fixes:" syntax + Cc:stable.

The vlan dump was introduced by commit 5b6eaea8ea7c ("mbuf: display more
fields in dump"), but I don't think we can say it's a bug. To me, it is
an enhancement that could be backported, because there is a benefit with
a very low risk.

Re: [PATCH] mbuf: expose outer vlan in mbuf dump

[Ben Magistro]

[-- Attachment #1: Type: text/plain, Size: 2212 bytes --]

Was there a consensus on if this should be re-drafted as a bug and fixes or
left as a backportable feature/improvement?  I am good either way, just
wanted to clarify if I had an additional action at this time.

Cheers

On Wed, Apr 6, 2022 at 3:33 AM Olivier Matz <olivier.matz@6wind.com> wrote:

> Hi,
>
> On Tue, Apr 05, 2022 at 05:51:05PM +0200, Thomas Monjalon wrote:
> > 04/04/2022 12:33, Kevin Traynor:
> > > On 04/04/2022 07:15, Morten Brørup wrote:
> > > >> From: Ben Magistro [mailto:koncept1@gmail.com]
> > > >> Sent: Monday, 4 April 2022 02.57
> > > >>
> > > >> Enable printing of the outer vlan if flags indicate it is present.
> > > >>
> > > >> Signed-off-by: Ben Magistro <koncept1@gmail.com>
>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
>
> > > >>
> > > >> ---
> > > >>
> > > >> While troubleshooting some QinQ offloads with various Intel i40e
> > > >> firmware[1], it was
> > > >> helpful to expose the outer vlan in the dump mbuf calls.  This
> should
> > > >> be straightforward
> > > >> to backport and happy to do the work if accepted. I understand that
> > > >> this may not be a
> > > >> widely supported capability at this time, so we are okay if this is
> not
> > > >> accepted and
> > > >> we just maintain a local patch.
> > > >
> > > > Features are usually not backported, only bug fixes.
> > > >
> > > > However, since this patch proved helpful finding a bug, and it is
> very simple, it could be considered by the LTS maintainers.
> > >
> > >
> > > Suggest to tag the patch with 'Cc: stable@dpdk.org' to indicate it is
> > > requested for stable branches. It will be caught by stable maintainer
> > > filters when it is time for backports and can be discussed further
> then.
> >
> > I think it is a bug.
> > What was introduced first? the function or the field?
> > Please find the commit where it should have been done
> > and mark it with "Fixes:" syntax + Cc:stable.
>
> The vlan dump was introduced by commit 5b6eaea8ea7c ("mbuf: display more
> fields in dump"), but I don't think we can say it's a bug. To me, it is
> an enhancement that could be backported, because there is a benefit with
> a very low risk.
>

[-- Attachment #2: Type: text/html, Size: 3178 bytes --]

Re: [PATCH] mbuf: expose outer vlan in mbuf dump

[Thomas Monjalon]

12/04/2022 18:11, Ben Magistro:
> Was there a consensus on if this should be re-drafted as a bug and fixes or
> left as a backportable feature/improvement?  I am good either way, just
> wanted to clarify if I had an additional action at this time.

There is no consensus but that's a detail.

In any case we agree to backport it.
I'll merge it with Cc: stable@dpdk.org


> On Wed, Apr 6, 2022 at 3:33 AM Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> > Hi,
> >
> > On Tue, Apr 05, 2022 at 05:51:05PM +0200, Thomas Monjalon wrote:
> > > 04/04/2022 12:33, Kevin Traynor:
> > > > On 04/04/2022 07:15, Morten Brørup wrote:
> > > > >> From: Ben Magistro [mailto:koncept1@gmail.com]
> > > > >> Sent: Monday, 4 April 2022 02.57
> > > > >>
> > > > >> Enable printing of the outer vlan if flags indicate it is present.
> > > > >>
> > > > >> Signed-off-by: Ben Magistro <koncept1@gmail.com>
> >
> > Acked-by: Olivier Matz <olivier.matz@6wind.com>
> >
> > > > >>
> > > > >> ---
> > > > >>
> > > > >> While troubleshooting some QinQ offloads with various Intel i40e
> > > > >> firmware[1], it was
> > > > >> helpful to expose the outer vlan in the dump mbuf calls.  This
> > should
> > > > >> be straightforward
> > > > >> to backport and happy to do the work if accepted. I understand that
> > > > >> this may not be a
> > > > >> widely supported capability at this time, so we are okay if this is
> > not
> > > > >> accepted and
> > > > >> we just maintain a local patch.
> > > > >
> > > > > Features are usually not backported, only bug fixes.
> > > > >
> > > > > However, since this patch proved helpful finding a bug, and it is
> > very simple, it could be considered by the LTS maintainers.
> > > >
> > > >
> > > > Suggest to tag the patch with 'Cc: stable@dpdk.org' to indicate it is
> > > > requested for stable branches. It will be caught by stable maintainer
> > > > filters when it is time for backports and can be discussed further
> > then.
> > >
> > > I think it is a bug.
> > > What was introduced first? the function or the field?
> > > Please find the commit where it should have been done
> > > and mark it with "Fixes:" syntax + Cc:stable.
> >
> > The vlan dump was introduced by commit 5b6eaea8ea7c ("mbuf: display more
> > fields in dump"), but I don't think we can say it's a bug. To me, it is
> > an enhancement that could be backported, because there is a benefit with
> > a very low risk.
> >
> 

RE: [PATCH] mbuf: expose outer vlan in mbuf dump

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, 13 April 2022 14.03
> 
> 12/04/2022 18:11, Ben Magistro:
> > Was there a consensus on if this should be re-drafted as a bug and
> fixes or
> > left as a backportable feature/improvement?  I am good either way,
> just
> > wanted to clarify if I had an additional action at this time.
> 
> There is no consensus but that's a detail.
> 
> In any case we agree to backport it.
> I'll merge it with Cc: stable@dpdk.org

+1 to that, Thomas!

> 
> 
> > On Wed, Apr 6, 2022 at 3:33 AM Olivier Matz <olivier.matz@6wind.com>
> wrote:
> >
> > > Hi,
> > >
> > > On Tue, Apr 05, 2022 at 05:51:05PM +0200, Thomas Monjalon wrote:
> > > > 04/04/2022 12:33, Kevin Traynor:
> > > > > On 04/04/2022 07:15, Morten Brørup wrote:
> > > > > >> From: Ben Magistro [mailto:koncept1@gmail.com]
> > > > > >> Sent: Monday, 4 April 2022 02.57
> > > > > >>
> > > > > >> Enable printing of the outer vlan if flags indicate it is
> present.
> > > > > >>
> > > > > >> Signed-off-by: Ben Magistro <koncept1@gmail.com>
> > >
> > > Acked-by: Olivier Matz <olivier.matz@6wind.com>
> > >
> > > > > >>
> > > > > >> ---
> > > > > >>
> > > > > >> While troubleshooting some QinQ offloads with various Intel
> i40e
> > > > > >> firmware[1], it was
> > > > > >> helpful to expose the outer vlan in the dump mbuf calls.
> This
> > > should
> > > > > >> be straightforward
> > > > > >> to backport and happy to do the work if accepted. I
> understand that
> > > > > >> this may not be a
> > > > > >> widely supported capability at this time, so we are okay if
> this is
> > > not
> > > > > >> accepted and
> > > > > >> we just maintain a local patch.
> > > > > >
> > > > > > Features are usually not backported, only bug fixes.
> > > > > >
> > > > > > However, since this patch proved helpful finding a bug, and
> it is
> > > very simple, it could be considered by the LTS maintainers.
> > > > >
> > > > >
> > > > > Suggest to tag the patch with 'Cc: stable@dpdk.org' to indicate
> it is
> > > > > requested for stable branches. It will be caught by stable
> maintainer
> > > > > filters when it is time for backports and can be discussed
> further
> > > then.
> > > >
> > > > I think it is a bug.
> > > > What was introduced first? the function or the field?
> > > > Please find the commit where it should have been done
> > > > and mark it with "Fixes:" syntax + Cc:stable.
> > >
> > > The vlan dump was introduced by commit 5b6eaea8ea7c ("mbuf: display
> more
> > > fields in dump"), but I don't think we can say it's a bug. To me,
> it is
> > > an enhancement that could be backported, because there is a benefit
> with
> > > a very low risk.
> > >
> >
> 

Re: [PATCH] mbuf: expose outer vlan in mbuf dump

[Thomas Monjalon]

13/04/2022 14:59, Morten Brørup:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Wednesday, 13 April 2022 14.03
> > 
> > 12/04/2022 18:11, Ben Magistro:
> > > Was there a consensus on if this should be re-drafted as a bug and
> > fixes or
> > > left as a backportable feature/improvement?  I am good either way,
> > just
> > > wanted to clarify if I had an additional action at this time.
> > 
> > There is no consensus but that's a detail.
> > 
> > In any case we agree to backport it.
> > I'll merge it with Cc: stable@dpdk.org
> 
> +1 to that, Thomas!
> 
> > 
> > 
> > > On Wed, Apr 6, 2022 at 3:33 AM Olivier Matz <olivier.matz@6wind.com>
> > wrote:
> > >
> > > > Hi,
> > > >
> > > > On Tue, Apr 05, 2022 at 05:51:05PM +0200, Thomas Monjalon wrote:
> > > > > 04/04/2022 12:33, Kevin Traynor:
> > > > > > On 04/04/2022 07:15, Morten Brørup wrote:
> > > > > > >> From: Ben Magistro [mailto:koncept1@gmail.com]
> > > > > > >> Sent: Monday, 4 April 2022 02.57
> > > > > > >>
> > > > > > >> Enable printing of the outer vlan if flags indicate it is
> > present.
> > > > > > >>
> > > > > > >> Signed-off-by: Ben Magistro <koncept1@gmail.com>
> > > >
> > > > Acked-by: Olivier Matz <olivier.matz@6wind.com>

Applied, thanks.