* [dpdk-dev] [PATCH] app/testpmd: check Rx VLAN offload flag to print VLAN TCI
@ 2018-09-26 3:06 John Daley
2018-10-01 14:01 ` Ferruh Yigit
2018-10-02 8:47 ` Ferruh Yigit
0 siblings, 2 replies; 6+ messages in thread
From: John Daley @ 2018-09-26 3:06 UTC (permalink / raw)
To: wenzhuo.lu, jingjing.wu, bernard.iremonger
Cc: olivier.matz, dev, Hyong Youb Kim
From: Hyong Youb Kim <hyonkim@cisco.com>
Since the following commit, PKT_RX_VLAN indicates the presence of
mbuf's vlan_tci, not PKT_RX_VLAN_STRIPPED.
commit 380a7aab1ae2 ("mbuf: rename deprecated VLAN flags")
Cc: olivier.matz@6wind.com
Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
app/test-pmd/rxonly.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
index a93d80612..e8d226624 100644
--- a/app/test-pmd/rxonly.c
+++ b/app/test-pmd/rxonly.c
@@ -130,7 +130,7 @@ pkt_burst_receive(struct fwd_stream *fs)
}
if (ol_flags & PKT_RX_TIMESTAMP)
printf(" - timestamp %"PRIu64" ", mb->timestamp);
- if (ol_flags & PKT_RX_VLAN_STRIPPED)
+ if (ol_flags & PKT_RX_VLAN)
printf(" - VLAN tci=0x%x", mb->vlan_tci);
if (ol_flags & PKT_RX_QINQ_STRIPPED)
printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
--
2.16.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: check Rx VLAN offload flag to print VLAN TCI
2018-09-26 3:06 [dpdk-dev] [PATCH] app/testpmd: check Rx VLAN offload flag to print VLAN TCI John Daley
@ 2018-10-01 14:01 ` Ferruh Yigit
2018-10-02 2:29 ` Hyong Youb Kim
2018-10-02 8:47 ` Ferruh Yigit
1 sibling, 1 reply; 6+ messages in thread
From: Ferruh Yigit @ 2018-10-01 14:01 UTC (permalink / raw)
To: John Daley, wenzhuo.lu, jingjing.wu, bernard.iremonger
Cc: olivier.matz, dev, Hyong Youb Kim
On 9/26/2018 4:06 AM, John Daley wrote:
> From: Hyong Youb Kim <hyonkim@cisco.com>
>
> Since the following commit, PKT_RX_VLAN indicates the presence of
> mbuf's vlan_tci, not PKT_RX_VLAN_STRIPPED.
>
> commit 380a7aab1ae2 ("mbuf: rename deprecated VLAN flags")
> Cc: olivier.matz@6wind.com
>
> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
> Reviewed-by: John Daley <johndale@cisco.com>
> ---
> app/test-pmd/rxonly.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
> index a93d80612..e8d226624 100644
> --- a/app/test-pmd/rxonly.c
> +++ b/app/test-pmd/rxonly.c
> @@ -130,7 +130,7 @@ pkt_burst_receive(struct fwd_stream *fs)
> }
> if (ol_flags & PKT_RX_TIMESTAMP)
> printf(" - timestamp %"PRIu64" ", mb->timestamp);
> - if (ol_flags & PKT_RX_VLAN_STRIPPED)
> + if (ol_flags & PKT_RX_VLAN)
> printf(" - VLAN tci=0x%x", mb->vlan_tci);
> if (ol_flags & PKT_RX_QINQ_STRIPPED)
> printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
Isn't same also correct for QinQ, PKT_RX_QINQ means mb->vlan_tci &
mb->vlan_tci_outer are set?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: check Rx VLAN offload flag to print VLAN TCI
2018-10-01 14:01 ` Ferruh Yigit
@ 2018-10-02 2:29 ` Hyong Youb Kim
2018-10-02 8:46 ` Ferruh Yigit
0 siblings, 1 reply; 6+ messages in thread
From: Hyong Youb Kim @ 2018-10-02 2:29 UTC (permalink / raw)
To: Ferruh Yigit
Cc: John Daley, wenzhuo.lu, jingjing.wu, bernard.iremonger,
olivier.matz, dev, beilei.xing, qi.z.zhang
On Mon, Oct 01, 2018 at 03:01:40PM +0100, Ferruh Yigit wrote:
> On 9/26/2018 4:06 AM, John Daley wrote:
> > From: Hyong Youb Kim <hyonkim@cisco.com>
> >
> > Since the following commit, PKT_RX_VLAN indicates the presence of
> > mbuf's vlan_tci, not PKT_RX_VLAN_STRIPPED.
> >
> > commit 380a7aab1ae2 ("mbuf: rename deprecated VLAN flags")
> > Cc: olivier.matz@6wind.com
> >
> > Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
> > Reviewed-by: John Daley <johndale@cisco.com>
> > ---
> > app/test-pmd/rxonly.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
> > index a93d80612..e8d226624 100644
> > --- a/app/test-pmd/rxonly.c
> > +++ b/app/test-pmd/rxonly.c
> > @@ -130,7 +130,7 @@ pkt_burst_receive(struct fwd_stream *fs)
> > }
> > if (ol_flags & PKT_RX_TIMESTAMP)
> > printf(" - timestamp %"PRIu64" ", mb->timestamp);
> > - if (ol_flags & PKT_RX_VLAN_STRIPPED)
> > + if (ol_flags & PKT_RX_VLAN)
> > printf(" - VLAN tci=0x%x", mb->vlan_tci);
> > if (ol_flags & PKT_RX_QINQ_STRIPPED)
> > printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
>
> Isn't same also correct for QinQ, PKT_RX_QINQ means mb->vlan_tci &
> mb->vlan_tci_outer are set?
>
That is a good point.
According to rte_mbuf.h, PKT_RX_QINQ means "The RX packet is a double
VLAN, and the outer tci has been saved in in mbuf->vlan_tci_outer."
Here is a summary.
PKT_RX_VLAN => vlan_tci is set
PKT_RX_QINQ => vlan_tci_outer is set
PKT_RX_VLAN_STRIPPED => must also set PKT_RX_VLAN
PKT_RX_QINQ_STRIPPED => must also set PKT_RX_VLAN, PKT_RX_QINQ,
PKT_RX_VLAN_STRIPPED
Looks like i40e is the only driver that is using
PKT_RX_QINQ_STRIPPED. And, it does not set PKT_RX_QINQ. I am CC'ing
i40e maintainers.
Back to rxonly..
+ if (ol_flags & (PKT_RX_QINQ | PKT_RX_VLAN))
printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
mb->vlan_tci, mb->vlan_tci_outer);
A change like this would be technically correct, but may break i40e
test cases. Or, if the above message is really meant for 'stripped',
then perhaps add comment or rephrase the message for now?
As for the use of PKT_RX_VLAN, some drivers like enic and ixgbe can
set PKT_RX_VLAN independent of vlan stripping, which led me to writing
this patch. I think Olivier fixed all drivers when he introduced
PKT_RX_VLAN. So using PKT_RX_VLAN in rxonly shouldn't be breaking
anyone's test cases.
-Hyong
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: check Rx VLAN offload flag to print VLAN TCI
2018-10-02 2:29 ` Hyong Youb Kim
@ 2018-10-02 8:46 ` Ferruh Yigit
0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2018-10-02 8:46 UTC (permalink / raw)
To: Hyong Youb Kim
Cc: John Daley, wenzhuo.lu, jingjing.wu, bernard.iremonger,
olivier.matz, dev, beilei.xing, qi.z.zhang
On 10/2/2018 3:29 AM, Hyong Youb Kim wrote:
> On Mon, Oct 01, 2018 at 03:01:40PM +0100, Ferruh Yigit wrote:
>> On 9/26/2018 4:06 AM, John Daley wrote:
>>> From: Hyong Youb Kim <hyonkim@cisco.com>
>>>
>>> Since the following commit, PKT_RX_VLAN indicates the presence of
>>> mbuf's vlan_tci, not PKT_RX_VLAN_STRIPPED.
>>>
>>> commit 380a7aab1ae2 ("mbuf: rename deprecated VLAN flags")
>>> Cc: olivier.matz@6wind.com
>>>
>>> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
>>> Reviewed-by: John Daley <johndale@cisco.com>
>>> ---
>>> app/test-pmd/rxonly.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
>>> index a93d80612..e8d226624 100644
>>> --- a/app/test-pmd/rxonly.c
>>> +++ b/app/test-pmd/rxonly.c
>>> @@ -130,7 +130,7 @@ pkt_burst_receive(struct fwd_stream *fs)
>>> }
>>> if (ol_flags & PKT_RX_TIMESTAMP)
>>> printf(" - timestamp %"PRIu64" ", mb->timestamp);
>>> - if (ol_flags & PKT_RX_VLAN_STRIPPED)
>>> + if (ol_flags & PKT_RX_VLAN)
>>> printf(" - VLAN tci=0x%x", mb->vlan_tci);
>>> if (ol_flags & PKT_RX_QINQ_STRIPPED)
>>> printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
>>
>> Isn't same also correct for QinQ, PKT_RX_QINQ means mb->vlan_tci &
>> mb->vlan_tci_outer are set?
>>
>
> That is a good point.
>
> According to rte_mbuf.h, PKT_RX_QINQ means "The RX packet is a double
> VLAN, and the outer tci has been saved in in mbuf->vlan_tci_outer."
>
> Here is a summary.
> PKT_RX_VLAN => vlan_tci is set
> PKT_RX_QINQ => vlan_tci_outer is set
Because of the comment on "PKT_RX_QINQ_STRIPPED" I think:
PKT_RX_QINQ => vlan_tci_outer & vlan_tci is set
Although it is not clear from "PKT_RX_QINQ" comment.
> PKT_RX_VLAN_STRIPPED => must also set PKT_RX_VLAN
> PKT_RX_QINQ_STRIPPED => must also set PKT_RX_VLAN, PKT_RX_QINQ,
> PKT_RX_VLAN_STRIPPED
>
> Looks like i40e is the only driver that is using
> PKT_RX_QINQ_STRIPPED. And, it does not set PKT_RX_QINQ. I am CC'ing
> i40e maintainers.
>
> Back to rxonly..
>
> + if (ol_flags & (PKT_RX_QINQ | PKT_RX_VLAN))
> printf(" - QinQ VLAN tci=0x%x, VLAN tci outer=0x%x",
> mb->vlan_tci, mb->vlan_tci_outer);
>
> A change like this would be technically correct, but may break i40e
> test cases. Or, if the above message is really meant for 'stripped',
> then perhaps add comment or rephrase the message for now?
>
> As for the use of PKT_RX_VLAN, some drivers like enic and ixgbe can
> set PKT_RX_VLAN independent of vlan stripping, which led me to writing
> this patch. I think Olivier fixed all drivers when he introduced
> PKT_RX_VLAN. So using PKT_RX_VLAN in rxonly shouldn't be breaking
> anyone's test cases.
+1 to PKT_RX_VLAN update.
I was thinking PKT_RX_QINQ also can be fixed quickly in testpmd with this patch,
taking into account that it may affect other piece of code, agree to get this
patch as it is and consider QINQ changes in different patch.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: check Rx VLAN offload flag to print VLAN TCI
2018-09-26 3:06 [dpdk-dev] [PATCH] app/testpmd: check Rx VLAN offload flag to print VLAN TCI John Daley
2018-10-01 14:01 ` Ferruh Yigit
@ 2018-10-02 8:47 ` Ferruh Yigit
2018-10-03 14:17 ` Ferruh Yigit
1 sibling, 1 reply; 6+ messages in thread
From: Ferruh Yigit @ 2018-10-02 8:47 UTC (permalink / raw)
To: John Daley, wenzhuo.lu, jingjing.wu, bernard.iremonger
Cc: olivier.matz, dev, Hyong Youb Kim
On 9/26/2018 4:06 AM, John Daley wrote:
> From: Hyong Youb Kim <hyonkim@cisco.com>
>
> Since the following commit, PKT_RX_VLAN indicates the presence of
> mbuf's vlan_tci, not PKT_RX_VLAN_STRIPPED.
>
> commit 380a7aab1ae2 ("mbuf: rename deprecated VLAN flags")
> Cc: olivier.matz@6wind.com
>
> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
> Reviewed-by: John Daley <johndale@cisco.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: check Rx VLAN offload flag to print VLAN TCI
2018-10-02 8:47 ` Ferruh Yigit
@ 2018-10-03 14:17 ` Ferruh Yigit
0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2018-10-03 14:17 UTC (permalink / raw)
To: John Daley, wenzhuo.lu, jingjing.wu, bernard.iremonger
Cc: olivier.matz, dev, Hyong Youb Kim
On 10/2/2018 9:47 AM, Ferruh Yigit wrote:
> On 9/26/2018 4:06 AM, John Daley wrote:
>> From: Hyong Youb Kim <hyonkim@cisco.com>
>>
>> Since the following commit, PKT_RX_VLAN indicates the presence of
>> mbuf's vlan_tci, not PKT_RX_VLAN_STRIPPED.
>>
>> commit 380a7aab1ae2 ("mbuf: rename deprecated VLAN flags")
>> Cc: olivier.matz@6wind.com
>>
>> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
>> Reviewed-by: John Daley <johndale@cisco.com>
>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-03 14:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-26 3:06 [dpdk-dev] [PATCH] app/testpmd: check Rx VLAN offload flag to print VLAN TCI John Daley
2018-10-01 14:01 ` Ferruh Yigit
2018-10-02 2:29 ` Hyong Youb Kim
2018-10-02 8:46 ` Ferruh Yigit
2018-10-02 8:47 ` Ferruh Yigit
2018-10-03 14:17 ` Ferruh Yigit
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).