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

Message ID 1448028587-7718-1-git-send-email-shaopeng.he@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

He, Shaopeng Nov. 20, 2015, 2:09 p.m. UTC
  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(-)
  

Comments

Stephen Hemminger Nov. 20, 2015, 4:35 p.m. UTC | #1
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>
  
Thomas Monjalon Nov. 24, 2015, 10:28 a.m. UTC | #2
> > 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
  

Patch

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 +