Re: [dpdk-dev] [PATCH v3 07/10] vmxnet3: support jumbo frames

[Yong Wang <yongwang@vmware.com>]

On 3/5/15, 4:10 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:

>From: Stephen Hemminger <shemming@brocade.com>
>
>Add support for linking multi-segment buffers together to
>handle Jumbo packets.
>
>Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>---
> lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c |  3 +-
> lib/librte_pmd_vmxnet3/vmxnet3_ring.h   |  2 +
> lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c   | 76
>++++++++++++++++++++-------------
> 3 files changed, 50 insertions(+), 31 deletions(-)
>
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>index 35bb561..4f1bc4f 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ethdev.c
>@@ -401,6 +401,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> {
> 	struct rte_eth_conf port_conf = dev->data->dev_conf;
> 	struct vmxnet3_hw *hw = dev->data->dev_private;
>+	uint32_t mtu = dev->data->mtu;
> 	Vmxnet3_DriverShared *shared = hw->shared;
> 	Vmxnet3_DSDevRead *devRead = &shared->devRead;
> 	uint32_t *mac_ptr;
>@@ -418,7 +419,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> 	devRead->misc.driverInfo.vmxnet3RevSpt = 1;
> 	devRead->misc.driverInfo.uptVerSpt     = 1;
> 
>-	devRead->misc.mtu = rte_le_to_cpu_32(dev->data->mtu);
>+	devRead->misc.mtu = rte_le_to_cpu_32(mtu);

I didn’t see where mtu is used to calculate how many rx descriptors will
be needed for each packet.  Furthermore, as pointed out by the following
code comments, the device requires the first rx buffer of a packet be of
type VMXNET3_RXD_BTYPE_HEAD with the remaining buffers of type
VMXNET3_RXD_BTYPE_NODY.  This needs to be taken care of when populating rx
rings in vmxnet3_post_rx_bufs(). Currently we don’t do this because no
scatter-rx is supported and only one descriptor is needed for a packet
(thus all types should be HEAD). Otherwise, the device will complain with
error returned.  For the 2nd rx ring, type needs to be BODY for all
descriptors still.

Related to this, could you share what tests have been done to cover these
new features?

static int
vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t *rxq, uint8_t ring_id)
{
        int err = 0;
        uint32_t i = 0, val = 0;
        struct vmxnet3_cmd_ring *ring = &rxq->cmd_ring[ring_id];

        if (ring_id == 0) {
	/* Usually: One HEAD type buf per packet
	* val = (ring->next2fill % rxq->hw->bufs_per_pkt) ?
	* VMXNET3_RXD_BTYPE_BODY : VMXNET3_RXD_BTYPE_HEAD;
	*/

	/* We use single packet buffer so all heads here */
	val = VMXNET3_RXD_BTYPE_HEAD;
        } else {
	/* All BODY type buffers for 2nd ring */
	val = VMXNET3_RXD_BTYPE_BODY;
        }



> 	devRead->misc.queueDescPA  = hw->queueDescPA;
> 	devRead->misc.queueDescLen = hw->queue_desc_len;
> 	devRead->misc.numTxQueues  = hw->num_tx_queues;
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>index 612487e..55ceadf 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_ring.h
>@@ -171,6 +171,8 @@ typedef struct vmxnet3_rx_queue {
> 	uint32_t                    qid1;
> 	uint32_t                    qid2;
> 	Vmxnet3_RxQueueDesc         *shared;
>+	struct rte_mbuf		    *start_seg;
>+	struct rte_mbuf		    *last_seg;
> 	struct vmxnet3_rxq_stats    stats;
> 	bool                        stopped;
> 	uint16_t                    queue_id;      /**< Device RX queue index.
>*/
>diff --git a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>index ba48a12..5cf187a 100644
>--- a/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>+++ b/lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c
>@@ -571,7 +571,6 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 	vmxnet3_rx_queue_t *rxq;
> 	Vmxnet3_RxCompDesc *rcd;
> 	vmxnet3_buf_info_t *rbi;
>-	Vmxnet3_RxDesc *rxd;
> 	struct rte_mbuf *rxm = NULL;
> 	struct vmxnet3_hw *hw;
> 
>@@ -596,42 +595,18 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 
> 		idx = rcd->rxdIdx;
> 		ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1);
>-		rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
> 		rbi = rxq->cmd_ring[ring_idx].buf_info + idx;
> 
>-		if (unlikely(rcd->sop != 1 || rcd->eop != 1)) {
>-			rte_pktmbuf_free_seg(rbi->m);
>-			PMD_RX_LOG(DEBUG, "Packet spread across multiple buffers\n)");
>-			goto rcd_done;
>-		}
> 
> 		PMD_RX_LOG(DEBUG, "rxd idx: %d ring idx: %d.", idx, ring_idx);
> 
> #ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>+		Vmxnet3_RxDesc *rxd
>+			= (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base + idx;
> 		VMXNET3_ASSERT(rcd->len <= rxd->len);
> 		VMXNET3_ASSERT(rbi->m);
> #endif
>-		if (unlikely(rcd->len == 0)) {
>-			PMD_RX_LOG(DEBUG, "Rx buf was skipped. rxring[%d][%d]\n)",
>-				   ring_idx, idx);
>-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>-			VMXNET3_ASSERT(rcd->sop && rcd->eop);
>-#endif
>-			rte_pktmbuf_free_seg(rbi->m);
>-			goto rcd_done;
>-		}
> 
>-		/* Assuming a packet is coming in a single packet buffer */
>-		if (unlikely(rxd->btype != VMXNET3_RXD_BTYPE_HEAD)) {
>-			PMD_RX_LOG(DEBUG,
>-				   "Alert : Misbehaving device, incorrect "
>-				   " buffer type used. iPacket dropped.");
>-			rte_pktmbuf_free_seg(rbi->m);
>-			goto rcd_done;
>-		}
>-#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>-		VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);
>-#endif
> 		/* Get the packet buffer pointer from buf_info */
> 		rxm = rbi->m;
> 
>@@ -643,7 +618,7 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 		rxq->cmd_ring[ring_idx].next2comp = idx;
> 
> 		/* For RCD with EOP set, check if there is frame error */
>-		if (unlikely(rcd->err)) {
>+		if (unlikely(rcd->eop && rcd->err)) {
> 			rxq->stats.drop_total++;
> 			rxq->stats.drop_err++;
> 
>@@ -669,9 +644,49 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf
>**rx_pkts, uint16_t nb_pkts)
> 		rxm->ol_flags = 0;
> 		rxm->vlan_tci = 0;
> 
>-		vmxnet3_rx_offload(rcd, rxm);
>+		/*
>+		 * If this is the first buffer of the received packet,
>+		 * set the pointer to the first mbuf of the packet
>+		 * Otherwise, update the total length and the number of segments
>+		 * of the current scattered packet, and update the pointer to
>+		 * the last mbuf of the current packet.
>+		 */
>+		if (rcd->sop) {
>+#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>+			VMXNET3_ASSERT(!rxq->start_seg);
>+			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);
>+#endif
>+
>+			if (unlikely(rcd->len == 0)) {
>+				PMD_RX_LOG(DEBUG,
>+					   "Rx buf was skipped. rxring[%d][%d])",
>+					   ring_idx, idx);
>+				rte_pktmbuf_free_seg(rbi->m);
>+				goto rcd_done;
>+			}
>+
>+			rxq->start_seg = rxm;
>+			vmxnet3_rx_offload(rcd, rxm);
>+		} else {
>+			struct rte_mbuf *start = rxq->start_seg;
>+
>+#ifdef RTE_LIBRTE_VMXNET3_DEBUG_DRIVER
>+			VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_BODY);
>+			VMXNET3_ASSERT(start != NULL);
>+#endif
>+
>+			start->pkt_len += rxm->data_len;
>+			start->nb_segs++;
>+
>+			rxq->last_seg->next = rxm;
>+		}
>+		rxq->last_seg = rxm;
>+
>+		if (rcd->eop) {
>+			rx_pkts[nb_rx++] = rxq->start_seg;
>+			rxq->start_seg = NULL;
>+		}
> 
>-		rx_pkts[nb_rx++] = rxm;
> rcd_done:
> 		rxq->cmd_ring[ring_idx].next2comp = idx;
> 		VMXNET3_INC_RING_IDX_ONLY(rxq->cmd_ring[ring_idx].next2comp,
>rxq->cmd_ring[ring_idx].size);
>@@ -975,6 +990,7 @@ vmxnet3_dev_rxtx_init(struct rte_eth_dev *dev)
> 			}
> 		}
> 		rxq->stopped = FALSE;
>+		rxq->start_seg = NULL;
> 	}
> 
> 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
>-- 
>2.1.4
>