[dpdk-dev] [PATCH] fm10k: fix wrong VLAN value in RX mbuf

[dpdk-dev] [PATCH] fm10k: fix wrong VLAN value in RX mbuf

[Shaopeng He]

VLAN value should be copied from RX descriptor to mbuf,
this patch fixes this issue.

Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
---
 drivers/net/fm10k/fm10k_rxtx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
index 1bac28d..eeb635e 100644
--- a/drivers/net/fm10k/fm10k_rxtx.c
+++ b/drivers/net/fm10k/fm10k_rxtx.c
@@ -146,6 +146,7 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 #endif
 
 		mbuf->hash.rss = desc.d.rss;
+		mbuf->vlan_tci = desc.w.vlan & FM10K_RXD_VLAN_ID_MASK;
 
 		rx_pkts[count] = mbuf;
 		if (++next_dd == q->nb_desc) {
@@ -292,6 +293,7 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		rx_desc_to_ol_flags(first_seg, &desc);
 #endif
 		first_seg->hash.rss = desc.d.rss;
+		first_seg->vlan_tci = desc.w.vlan & FM10K_RXD_VLAN_ID_MASK;
 
 		/* Prefetch data of first segment, if configured to do so. */
 		rte_packet_prefetch((char *)first_seg->buf_addr +
-- 
1.9.3

Re: [dpdk-dev] [PATCH] fm10k: fix wrong VLAN value in RX mbuf

[Chen, Jing D]

Hi, 

Worth to adding comments that vlan_tci is only valid in case RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE  is turned on and 
Flag PKT_RX_VLAN_PKT is set.
 
Best Regards,
Mark


> -----Original Message-----
> From: He, Shaopeng
> Sent: Wednesday, November 18, 2015 4:50 PM
> To: dev@dpdk.org
> Cc: Chen, Jing D; Qiu, Michael; He, Shaopeng
> Subject: [PATCH] fm10k: fix wrong VLAN value in RX mbuf
> 
> VLAN value should be copied from RX descriptor to mbuf,
> this patch fixes this issue.
> 
> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> ---
>  drivers/net/fm10k/fm10k_rxtx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/fm10k/fm10k_rxtx.c
> b/drivers/net/fm10k/fm10k_rxtx.c
> index 1bac28d..eeb635e 100644
> --- a/drivers/net/fm10k/fm10k_rxtx.c
> +++ b/drivers/net/fm10k/fm10k_rxtx.c
> @@ -146,6 +146,7 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>  #endif
> 
>  		mbuf->hash.rss = desc.d.rss;
> +		mbuf->vlan_tci = desc.w.vlan &
> FM10K_RXD_VLAN_ID_MASK;
> 
>  		rx_pkts[count] = mbuf;
>  		if (++next_dd == q->nb_desc) {
> @@ -292,6 +293,7 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct
> rte_mbuf **rx_pkts,
>  		rx_desc_to_ol_flags(first_seg, &desc);
>  #endif
>  		first_seg->hash.rss = desc.d.rss;
> +		first_seg->vlan_tci = desc.w.vlan &
> FM10K_RXD_VLAN_ID_MASK;
> 
>  		/* Prefetch data of first segment, if configured to do so. */
>  		rte_packet_prefetch((char *)first_seg->buf_addr +
> --
> 1.9.3

Re: [dpdk-dev] [PATCH] fm10k: fix wrong VLAN value in RX mbuf

[Stephen Hemminger]

On Wed, 18 Nov 2015 16:50:09 +0800
Shaopeng He <shaopeng.he@intel.com> wrote:

> VLAN value should be copied from RX descriptor to mbuf,
> this patch fixes this issue.
> 
> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> ---
>  drivers/net/fm10k/fm10k_rxtx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
> index 1bac28d..eeb635e 100644
> --- a/drivers/net/fm10k/fm10k_rxtx.c
> +++ b/drivers/net/fm10k/fm10k_rxtx.c
> @@ -146,6 +146,7 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  #endif
>  
>  		mbuf->hash.rss = desc.d.rss;
> +		mbuf->vlan_tci = desc.w.vlan & FM10K_RXD_VLAN_ID_MASK;
>  
>  		rx_pkts[count] = mbuf;
>  		if (++next_dd == q->nb_desc) {
> @@ -292,6 +293,7 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		rx_desc_to_ol_flags(first_seg, &desc);
>  #endif
>  		first_seg->hash.rss = desc.d.rss;
> +		first_seg->vlan_tci = desc.w.vlan & FM10K_RXD_VLAN_ID_MASK;
>  
>  		/* Prefetch data of first segment, if configured to do so. */
>  		rte_packet_prefetch((char *)first_seg->buf_addr +

Good catch, this bug means VLAN's would not work right now.
But I think it isn't quite right.

By masking the VLAN you are losing the received priority bits.
Other drivers like ixgbe preserve the priority bits.

Re: [dpdk-dev] [PATCH] fm10k: fix wrong VLAN value in RX mbuf

[He, Shaopeng]

Hi, Mark

> -----Original Message-----
> From: Chen, Jing D
> Sent: Thursday, November 19, 2015 3:05 PM
> To: He, Shaopeng; dev@dpdk.org
> Cc: Qiu, Michael
> Subject: RE: [PATCH] fm10k: fix wrong VLAN value in RX mbuf
> 
> Hi,
> 
> Worth to adding comments that vlan_tci is only valid in case
> RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE  is turned on and Flag
> PKT_RX_VLAN_PKT is set.
Good suggestion. But, actually, we have a little different case in fm10k:
fm10k has a switch core inside, which associates a VLAN ID for each packet.
For those packets coming in without a VLAN, the port default VLAN ID will
be used. So for fm10k, vlan_tci is always valid. 
For Flag PKT_RX_VLAN_PKT, currently it will be set when there is inner VLAN
in the packet. This is not correct, I will fix it by setting PKT_RX_VLAN_PKT
always on in next version.

To verify this, different situations have been tested on
current implementation:
1. Ixia sent VLAN 51 packet, DPDK received: vlan_tci=51, PKT_RX_VLAN_PKT=0
2. sent non-VLAN packet, ether port's default VLAN ID is 1
    received: vlan_tci=1, PKT_RX_VLAN_PKT=0
3. sent double-VLAN packet (outer 51, inner 17),
    received: vlan_tci=51, PKT_RX_VLAN_PKT=1

Thanks,
--Shaopeng
> 
> Best Regards,
> Mark
> 
> 
> > -----Original Message-----
> > From: He, Shaopeng
> > Sent: Wednesday, November 18, 2015 4:50 PM
> > To: dev@dpdk.org
> > Cc: Chen, Jing D; Qiu, Michael; He, Shaopeng
> > Subject: [PATCH] fm10k: fix wrong VLAN value in RX mbuf
> >
> > VLAN value should be copied from RX descriptor to mbuf, this patch
> > fixes this issue.
> >
> > Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> > ---
> >  drivers/net/fm10k/fm10k_rxtx.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/fm10k/fm10k_rxtx.c
> > b/drivers/net/fm10k/fm10k_rxtx.c index 1bac28d..eeb635e 100644
> > --- a/drivers/net/fm10k/fm10k_rxtx.c
> > +++ b/drivers/net/fm10k/fm10k_rxtx.c
> > @@ -146,6 +146,7 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf
> > **rx_pkts,  #endif
> >
> >  		mbuf->hash.rss = desc.d.rss;
> > +		mbuf->vlan_tci = desc.w.vlan &
> > FM10K_RXD_VLAN_ID_MASK;
> >
> >  		rx_pkts[count] = mbuf;
> >  		if (++next_dd == q->nb_desc) {
> > @@ -292,6 +293,7 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct
> > rte_mbuf **rx_pkts,
> >  		rx_desc_to_ol_flags(first_seg, &desc);  #endif
> >  		first_seg->hash.rss = desc.d.rss;
> > +		first_seg->vlan_tci = desc.w.vlan &
> > FM10K_RXD_VLAN_ID_MASK;
> >
> >  		/* Prefetch data of first segment, if configured to do so. */
> >  		rte_packet_prefetch((char *)first_seg->buf_addr +
> > --
> > 1.9.3

Re: [dpdk-dev] [PATCH] fm10k: fix wrong VLAN value in RX mbuf

[He, Shaopeng]

Hi, Stephen

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, November 20, 2015 1:26 AM
> To: He, Shaopeng
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] fm10k: fix wrong VLAN value in RX mbuf
> 
> On Wed, 18 Nov 2015 16:50:09 +0800
> Shaopeng He <shaopeng.he@intel.com> wrote:
> 
> > VLAN value should be copied from RX descriptor to mbuf, this patch
> > fixes this issue.
> >
> > Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> > ---
> >  drivers/net/fm10k/fm10k_rxtx.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/fm10k/fm10k_rxtx.c
> > b/drivers/net/fm10k/fm10k_rxtx.c index 1bac28d..eeb635e 100644
> > --- a/drivers/net/fm10k/fm10k_rxtx.c
> > +++ b/drivers/net/fm10k/fm10k_rxtx.c
> > @@ -146,6 +146,7 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf
> > **rx_pkts,  #endif
> >
> >  		mbuf->hash.rss = desc.d.rss;
> > +		mbuf->vlan_tci = desc.w.vlan &
> FM10K_RXD_VLAN_ID_MASK;
> >
> >  		rx_pkts[count] = mbuf;
> >  		if (++next_dd == q->nb_desc) {
> > @@ -292,6 +293,7 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct
> rte_mbuf **rx_pkts,
> >  		rx_desc_to_ol_flags(first_seg, &desc);  #endif
> >  		first_seg->hash.rss = desc.d.rss;
> > +		first_seg->vlan_tci = desc.w.vlan &
> FM10K_RXD_VLAN_ID_MASK;
> >
> >  		/* Prefetch data of first segment, if configured to do so. */
> >  		rte_packet_prefetch((char *)first_seg->buf_addr +
> 
> Good catch, this bug means VLAN's would not work right now.
> But I think it isn't quite right.

Actually, VLAN filter works well, only the vlan_tci in mbuf has wrong value.
And because not much places use the vlan_tci, this bug was not
found  before.

> 
> By masking the VLAN you are losing the received priority bits.
> Other drivers like ixgbe preserve the priority bits.

Yes, you are right, in next version, I will preserve the priority bits like others.

Thanks,
--Shaopeng

[dpdk-dev] [PATCH v2] fm10k: fix wrong VLAN value in RX mbuf

[Shaopeng He]

vlan_tci should be copied from RX descriptor to mbuf,
and flag PKT_RX_VLAN_PKT should be set for every RX packet,
this patch fixes this issue.
fm10k's Ethernet switch core associates a VLAN ID and VLAN PRI
for each packet. For those packets coming in without a VLAN,
the port default VLAN ID will be used.
So in fm10k, always PKT_RX_VLAN_PKT flag is set and vlan_tci
is valid for each RX packet's mbuf.

Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
---
ChangeLog:

v2:
* change flag PKT_RX_VLAN_PKT to always set
* preserve the priority bits in vlan_tci

 drivers/net/fm10k/fm10k_rxtx.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
index 1bac28d..764f4f4 100644
--- a/drivers/net/fm10k/fm10k_rxtx.c
+++ b/drivers/net/fm10k/fm10k_rxtx.c
@@ -104,8 +104,14 @@ rx_desc_to_ol_flags(struct rte_mbuf *m, const union fm10k_rx_desc *d)
 		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)))
 		m->ol_flags |= PKT_RX_L4_CKSUM_BAD;
 
-	if (d->d.staterr & FM10K_RXD_STATUS_VEXT)
-		m->ol_flags |= PKT_RX_VLAN_PKT;
+	/**
+	 * fm10k's Ethernet switch core associates a VLAN ID and VLAN PRI
+	 * for each packet. For those packets coming in without a VLAN,
+	 * the port default VLAN ID will be used.
+	 * So in fm10k, always PKT_RX_VLAN_PKT flag is set and vlan_tci
+	 * is valid for each RX packet's mbuf.
+	 */
+	m->ol_flags |= PKT_RX_VLAN_PKT;
 
 	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_HBO))
 		m->ol_flags |= PKT_RX_HBUF_OVERFLOW;
@@ -146,6 +152,8 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 #endif
 
 		mbuf->hash.rss = desc.d.rss;
+		/* in fm10k, vlan_tci is always valid for RX packet */
+		mbuf->vlan_tci = desc.w.vlan;
 
 		rx_pkts[count] = mbuf;
 		if (++next_dd == q->nb_desc) {
@@ -292,6 +300,8 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		rx_desc_to_ol_flags(first_seg, &desc);
 #endif
 		first_seg->hash.rss = desc.d.rss;
+		/* in fm10k, vlan_tci is always valid for RX packet */
+		first_seg->vlan_tci = desc.w.vlan;
 
 		/* Prefetch data of first segment, if configured to do so. */
 		rte_packet_prefetch((char *)first_seg->buf_addr +
-- 
1.9.3

Re: [dpdk-dev] [PATCH v2] fm10k: fix wrong VLAN value in RX mbuf

[Chen, Jing D]

> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> ---
> ChangeLog:
> 
> v2:
> * change flag PKT_RX_VLAN_PKT to always set
> * preserve the priority bits in vlan_tci
> 
>  drivers/net/fm10k/fm10k_rxtx.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/fm10k/fm10k_rxtx.c
> b/drivers/net/fm10k/fm10k_rxtx.c
> index 1bac28d..764f4f4 100644
> --- a/drivers/net/fm10k/fm10k_rxtx.c
> +++ b/drivers/net/fm10k/fm10k_rxtx.c
> @@ -104,8 +104,14 @@ rx_desc_to_ol_flags(struct rte_mbuf *m, const
> union fm10k_rx_desc *d)
>  		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)))
>  		m->ol_flags |= PKT_RX_L4_CKSUM_BAD;
> 
> -	if (d->d.staterr & FM10K_RXD_STATUS_VEXT)
> -		m->ol_flags |= PKT_RX_VLAN_PKT;
> +	/**
> +	 * fm10k's Ethernet switch core associates a VLAN ID and VLAN PRI

Change to Packets in fm10k device always carries a vlan tag?

> +	 * for each packet. For those packets coming in without a VLAN,
> +	 * the port default VLAN ID will be used.
> +	 * So in fm10k, always PKT_RX_VLAN_PKT flag is set and vlan_tci
> +	 * is valid for each RX packet's mbuf.
> +	 */
> +	m->ol_flags |= PKT_RX_VLAN_PKT;

Since vlan_tci is always valid, is it better to move above line to below added lines?

> 
>  	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_HBO))
>  		m->ol_flags |= PKT_RX_HBUF_OVERFLOW;
> @@ -146,6 +152,8 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>  #endif
> 
>  		mbuf->hash.rss = desc.d.rss;
> +		/* in fm10k, vlan_tci is always valid for RX packet */
> +		mbuf->vlan_tci = desc.w.vlan;
> 
>  		rx_pkts[count] = mbuf;
>  		if (++next_dd == q->nb_desc) {
> @@ -292,6 +300,8 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct
> rte_mbuf **rx_pkts,
>  		rx_desc_to_ol_flags(first_seg, &desc);
>  #endif
>  		first_seg->hash.rss = desc.d.rss;
> +		/* in fm10k, vlan_tci is always valid for RX packet */
> +		first_seg->vlan_tci = desc.w.vlan;
> 
>  		/* Prefetch data of first segment, if configured to do so. */
>  		rte_packet_prefetch((char *)first_seg->buf_addr +
> --
> 1.9.3

[dpdk-dev] [PATCH v3] fm10k: fix wrong VLAN value in RX mbuf

[Shaopeng He]

vlan_tci should be copied from RX descriptor to mbuf,
and flag PKT_RX_VLAN_PKT should be set for every RX packet,
this patch fixes this issue.
Packets in fm10k device always carry at least one VLAN tag.
For those packets coming in without VLAN tag,
the port default VLAN tag will be used.
So in fm10k, always PKT_RX_VLAN_PKT flag is set and vlan_tci
is valid for each RX packet's mbuf.

Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
---
ChangeLog:

v3:
* move flag PKT_RX_VLAN_PKT logic together with vlan_tci
* reword comments

v2:
* change flag PKT_RX_VLAN_PKT to always set
* preserve the priority bits in vlan_tci

 drivers/net/fm10k/fm10k_rxtx.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
index 1bac28d..e958865 100644
--- a/drivers/net/fm10k/fm10k_rxtx.c
+++ b/drivers/net/fm10k/fm10k_rxtx.c
@@ -104,9 +104,6 @@ rx_desc_to_ol_flags(struct rte_mbuf *m, const union fm10k_rx_desc *d)
 		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)))
 		m->ol_flags |= PKT_RX_L4_CKSUM_BAD;
 
-	if (d->d.staterr & FM10K_RXD_STATUS_VEXT)
-		m->ol_flags |= PKT_RX_VLAN_PKT;
-
 	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_HBO))
 		m->ol_flags |= PKT_RX_HBUF_OVERFLOW;
 
@@ -146,6 +143,15 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 #endif
 
 		mbuf->hash.rss = desc.d.rss;
+		/**
+		 * Packets in fm10k device always carry at least one VLAN tag.
+		 * For those packets coming in without VLAN tag,
+		 * the port default VLAN tag will be used.
+		 * So, always PKT_RX_VLAN_PKT flag is set and vlan_tci
+		 * is valid for each RX packet's mbuf.
+		 */
+		mbuf->ol_flags |= PKT_RX_VLAN_PKT;
+		mbuf->vlan_tci = desc.w.vlan;
 
 		rx_pkts[count] = mbuf;
 		if (++next_dd == q->nb_desc) {
@@ -292,6 +298,15 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		rx_desc_to_ol_flags(first_seg, &desc);
 #endif
 		first_seg->hash.rss = desc.d.rss;
+		/**
+		 * Packets in fm10k device always carry at least one VLAN tag.
+		 * For those packets coming in without VLAN tag,
+		 * the port default VLAN tag will be used.
+		 * So, always PKT_RX_VLAN_PKT flag is set and vlan_tci
+		 * is valid for each RX packet's mbuf.
+		 */
+		mbuf->ol_flags |= PKT_RX_VLAN_PKT;
+		first_seg->vlan_tci = desc.w.vlan;
 
 		/* Prefetch data of first segment, if configured to do so. */
 		rte_packet_prefetch((char *)first_seg->buf_addr +
-- 
1.9.3

Re: [dpdk-dev] [PATCH v2] fm10k: fix wrong VLAN value in RX mbuf

[He, Shaopeng]

Hi, Mark

> -----Original Message-----
> From: Chen, Jing D
> Sent: Friday, November 20, 2015 6:35 PM
> To: He, Shaopeng; dev@dpdk.org
> Cc: Qiu, Michael
> Subject: RE: [PATCH v2] fm10k: fix wrong VLAN value in RX mbuf
> 
> > Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> > ---
> > ChangeLog:
> >
> > v2:
> > * change flag PKT_RX_VLAN_PKT to always set
> > * preserve the priority bits in vlan_tci
> >
> >  drivers/net/fm10k/fm10k_rxtx.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/fm10k/fm10k_rxtx.c
> > b/drivers/net/fm10k/fm10k_rxtx.c index 1bac28d..764f4f4 100644
> > --- a/drivers/net/fm10k/fm10k_rxtx.c
> > +++ b/drivers/net/fm10k/fm10k_rxtx.c
> > @@ -104,8 +104,14 @@ rx_desc_to_ol_flags(struct rte_mbuf *m, const
> > union fm10k_rx_desc *d)
> >  		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)))
> >  		m->ol_flags |= PKT_RX_L4_CKSUM_BAD;
> >
> > -	if (d->d.staterr & FM10K_RXD_STATUS_VEXT)
> > -		m->ol_flags |= PKT_RX_VLAN_PKT;
> > +	/**
> > +	 * fm10k's Ethernet switch core associates a VLAN ID and VLAN PRI
> 
> Change to Packets in fm10k device always carries a vlan tag?
> 
> > +	 * for each packet. For those packets coming in without a VLAN,
> > +	 * the port default VLAN ID will be used.
> > +	 * So in fm10k, always PKT_RX_VLAN_PKT flag is set and vlan_tci
> > +	 * is valid for each RX packet's mbuf.
> > +	 */
> > +	m->ol_flags |= PKT_RX_VLAN_PKT;
> 
> Since vlan_tci is always valid, is it better to move above line to below added
> lines?

Sure, I will send V3

Thanks,
--Shaopeng

> 
> >
> >  	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_HBO))
> >  		m->ol_flags |= PKT_RX_HBUF_OVERFLOW; @@ -146,6
> +152,8 @@
> > fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,  #endif
> >
> >  		mbuf->hash.rss = desc.d.rss;
> > +		/* in fm10k, vlan_tci is always valid for RX packet */
> > +		mbuf->vlan_tci = desc.w.vlan;
> >
> >  		rx_pkts[count] = mbuf;
> >  		if (++next_dd == q->nb_desc) {
> > @@ -292,6 +300,8 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct
> > rte_mbuf **rx_pkts,
> >  		rx_desc_to_ol_flags(first_seg, &desc);  #endif
> >  		first_seg->hash.rss = desc.d.rss;
> > +		/* in fm10k, vlan_tci is always valid for RX packet */
> > +		first_seg->vlan_tci = desc.w.vlan;
> >
> >  		/* Prefetch data of first segment, if configured to do so. */
> >  		rte_packet_prefetch((char *)first_seg->buf_addr +
> > --
> > 1.9.3
> 

Re: [dpdk-dev] [PATCH v3] fm10k: fix wrong VLAN value in RX mbuf

[Stephen Hemminger]

On Fri, 20 Nov 2015 22:09:47 +0800
Shaopeng He <shaopeng.he@intel.com> wrote:

> vlan_tci should be copied from RX descriptor to mbuf,
> and flag PKT_RX_VLAN_PKT should be set for every RX packet,
> this patch fixes this issue.
> Packets in fm10k device always carry at least one VLAN tag.
> For those packets coming in without VLAN tag,
> the port default VLAN tag will be used.
> So in fm10k, always PKT_RX_VLAN_PKT flag is set and vlan_tci
> is valid for each RX packet's mbuf.
> 
> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> ---
> ChangeLog:
> 
> v3:
> * move flag PKT_RX_VLAN_PKT logic together with vlan_tci
> * reword comments
> 
> v2:
> * change flag PKT_RX_VLAN_PKT to always set
> * preserve the priority bits in vlan_tci
> 
>  drivers/net/fm10k/fm10k_rxtx.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
> index 1bac28d..e958865 100644
> --- a/drivers/net/fm10k/fm10k_rxtx.c
> +++ b/drivers/net/fm10k/fm10k_rxtx.c
> @@ -104,9 +104,6 @@ rx_desc_to_ol_flags(struct rte_mbuf *m, const union fm10k_rx_desc *d)
>  		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)))
>  		m->ol_flags |= PKT_RX_L4_CKSUM_BAD;
>  
> -	if (d->d.staterr & FM10K_RXD_STATUS_VEXT)
> -		m->ol_flags |= PKT_RX_VLAN_PKT;
> -
>  	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_HBO))
>  		m->ol_flags |= PKT_RX_HBUF_OVERFLOW;
>  
> @@ -146,6 +143,15 @@ fm10k_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  #endif
>  
>  		mbuf->hash.rss = desc.d.rss;
> +		/**
> +		 * Packets in fm10k device always carry at least one VLAN tag.
> +		 * For those packets coming in without VLAN tag,
> +		 * the port default VLAN tag will be used.
> +		 * So, always PKT_RX_VLAN_PKT flag is set and vlan_tci
> +		 * is valid for each RX packet's mbuf.
> +		 */
> +		mbuf->ol_flags |= PKT_RX_VLAN_PKT;
> +		mbuf->vlan_tci = desc.w.vlan;
>  
>  		rx_pkts[count] = mbuf;
>  		if (++next_dd == q->nb_desc) {
> @@ -292,6 +298,15 @@ fm10k_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		rx_desc_to_ol_flags(first_seg, &desc);
>  #endif
>  		first_seg->hash.rss = desc.d.rss;
> +		/**
> +		 * Packets in fm10k device always carry at least one VLAN tag.
> +		 * For those packets coming in without VLAN tag,
> +		 * the port default VLAN tag will be used.
> +		 * So, always PKT_RX_VLAN_PKT flag is set and vlan_tci
> +		 * is valid for each RX packet's mbuf.
> +		 */
> +		mbuf->ol_flags |= PKT_RX_VLAN_PKT;
> +		first_seg->vlan_tci = desc.w.vlan;
>  
>  		/* Prefetch data of first segment, if configured to do so. */
>  		rte_packet_prefetch((char *)first_seg->buf_addr +

Looks good thanks.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [dpdk-dev] [PATCH v3] fm10k: fix wrong VLAN value in RX mbuf

[Thomas Monjalon]

> > vlan_tci should be copied from RX descriptor to mbuf,
> > and flag PKT_RX_VLAN_PKT should be set for every RX packet,
> > this patch fixes this issue.
> > Packets in fm10k device always carry at least one VLAN tag.
> > For those packets coming in without VLAN tag,
> > the port default VLAN tag will be used.
> > So in fm10k, always PKT_RX_VLAN_PKT flag is set and vlan_tci
> > is valid for each RX packet's mbuf.
> > 
> > Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> 
> Looks good thanks.
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Applied, thanks