From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: vmxnet3 driver - vlan offload is on end descriptor Date: Thu, 7 Apr 2016 10:35:22 -0700 Message-ID: <20160407103522.2d3d7953@xeon-e3> References: Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" To: John Guzik Return-path: Received: from mail-pf0-f180.google.com (mail-pf0-f180.google.com [209.85.192.180]) by dpdk.org (Postfix) with ESMTP id 42399FFA for ; Thu, 7 Apr 2016 19:35:10 +0200 (CEST) Received: by mail-pf0-f180.google.com with SMTP id c20so59679334pfc.1 for ; Thu, 07 Apr 2016 10:35:10 -0700 (PDT) In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, 7 Apr 2016 17:11:19 +0000 John Guzik wrote: > With the new jumbo frame patch a latent bug has appeared to have been uncovered: the VLAN offload is on the last rx descriptor, when rcd->eop is true, not the first rx descriptor, when rcd->sop is true and rcd->eop could be false. > Most likely the reason this has not been seen before is that for non-jumbo frames rcd->eop has always been true when rcd->sop is true as well. > > The following patch should fix the problem: > > diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c > index 4ac0456..3d4a5eb 100644 > --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c > +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c > @@ -587,12 +587,6 @@ vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t *rxq, uint8_t ring_id) > static void > vmxnet3_rx_offload(const Vmxnet3_RxCompDesc *rcd, struct rte_mbuf *rxm) > { > - /* Check for hardware stripped VLAN tag */ > - if (rcd->ts) { > - rxm->ol_flags |= PKT_RX_VLAN_PKT; > - rxm->vlan_tci = rte_le_to_cpu_16((uint16_t)rcd->tci); > - } > - > /* Check for RSS */ > if (rcd->rssType != VMXNET3_RCD_RSS_TYPE_NONE) { > rxm->ol_flags |= PKT_RX_RSS_HASH; > @@ -737,7 +731,15 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > rxq->last_seg = rxm; > > if (rcd->eop) { > - rx_pkts[nb_rx++] = rxq->start_seg; > + struct rte_mbuf *start = rxq->start_seg; > + > + /* Check for hardware stripped VLAN tag */ > + if (rcd->ts) { > + start->ol_flags |= PKT_RX_VLAN_PKT; > + start->vlan_tci = rte_le_to_cpu_16((uint16_t)rcd->tci); > + } > + > + rx_pkts[nb_rx++] = start; > rxq->start_seg = NULL; > } > > Yes, your analysis and bug fix looks correct. The Linux driver checks for VLAN tag in the last (eop) segment. Acked-by: Stephen Hemminger