Skip to contentSkip to conversation

virtio driver phase 2

PATCH In review Stephen Hemminger 3 participants 13 messages Jun 14, 2014 – Jul 22, 2014 3 messages not in the archive

lore
Share & tools
lore
#0 lore raw

cover virtio driver phase 2

This is second group of patches for cleaning up virtio driver prior to the functionality changes in next phase.

#1 lore raw

PATCH 1/8 virtio: maintain stats per queue

Avoid cache collision and thrashing of the software statistics by keeping them per-queue in the driver.

lib/librte_pmd_virtio/virtio_ethdev.c +56 -8
@@ -474,19 +474,67 @@ virtio_dev_atomic_write_link_status(stru
 static void
 virtio_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
-	struct virtio_hw *hw =
-		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	if (stats)
-		memcpy(stats, &hw->eth_stats, sizeof(*stats));
+	unsigned i;
+
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		const struct virtqueue *txvq = dev->data->tx_queues[i];
+		if (txvq == NULL)
+			continue;
+
+		stats->opackets += txvq->packets;
+		stats->obytes += txvq->bytes;
+		stats->oerrors += txvq->errors;
+
+		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+			stats->q_opackets[i] = txvq->packets;
+			stats->q_obytes[i] = txvq->bytes;
+		}
+	}
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		const struct virtqueue *rxvq = dev->data->rx_queues[i];
+		if (rxvq == NULL)
+			continue;
+
+		stats->ipackets += rxvq->packets;
+		stats->ibytes += rxvq->bytes;
+		stats->ierrors += rxvq->errors;
+
+		if (i < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
+			stats->q_ipackets[i] = rxvq->packets;
+			stats->q_ibytes[i] = rxvq->bytes;
+		}
+	}
+
+	stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed;
 }
 
 static void
 virtio_dev_stats_reset(struct rte_eth_dev *dev)
 {
-	struct virtio_hw *hw =
-		VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	/* Reset software totals */
-	memset(&hw->eth_stats, 0, sizeof(hw->eth_stats));
+	unsigned int i;
+
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		struct virtqueue *txvq = dev->data->tx_queues[i];
+		if (txvq == NULL)
+			continue;
+
+		txvq->packets = 0;
+		txvq->bytes = 0;
+		txvq->errors = 0;
+	}
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		struct virtqueue *rxvq = dev->data->rx_queues[i];
+		if (rxvq == NULL)
+			continue;
+
+		rxvq->packets = 0;
+		rxvq->bytes = 0;
+		rxvq->errors = 0;
+	}
+
+	dev->data->rx_mbuf_alloc_failed = 0;
 }
 
 static void
lib/librte_pmd_virtio/virtio_pci.h +0 -1
@@ -179,7 +179,6 @@ struct virtio_hw {
 	uint8_t     revision_id;
 	uint8_t     mac_addr[ETHER_ADDR_LEN];
 	int         adapter_stopped;
-	struct      rte_eth_stats eth_stats;
 };
 
 /*
lib/librte_pmd_virtio/virtio_rxtx.c +8 -14
@@ -301,7 +301,7 @@ virtio_recv_pkts(void *rx_queue, struct
 			PMD_RX_LOG(ERR, "Packet drop\n");
 			nb_enqueued++;
 			virtio_discard_rxbuf(rxvq, rxm);
-			hw->eth_stats.ierrors++;
+			rxvq->errors++;
 			continue;
 		}
 
@@ -317,20 +317,19 @@ virtio_recv_pkts(void *rx_queue, struct
 		VIRTIO_DUMP_PACKET(rxm, rxm->pkt.data_len);
 
 		rx_pkts[nb_rx++] = rxm;
-		hw->eth_stats.ibytes += len[i] - sizeof(struct virtio_net_hdr);
-		hw->eth_stats.q_ibytes[rxvq->queue_id] += len[i]
-			- sizeof(struct virtio_net_hdr);
+		rxvq->bytes += len[i] - sizeof(struct virtio_net_hdr);
 	}
 
-	hw->eth_stats.ipackets += nb_rx;
-	hw->eth_stats.q_ipackets[rxvq->queue_id] += nb_rx;
+	rxvq->packets += nb_rx;
 
 	/* Allocate new mbuf for the used descriptor */
 	error = ENOSPC;
 	while (likely(!virtqueue_full(rxvq))) {
 		new_mbuf = rte_rxmbuf_alloc(rxvq->mpool);
 		if (unlikely(new_mbuf == NULL)) {
-			hw->eth_stats.rx_nombuf++;
+			struct rte_eth_dev *dev
+				= &rte_eth_devices[rxvq->port_id];
+			dev->data->rx_mbuf_alloc_failed++;
 			break;
 		}
 		error = virtqueue_enqueue_recv_refill(rxvq, new_mbuf);
@@ -359,7 +358,6 @@ virtio_xmit_pkts(void *tx_queue, struct
 	struct rte_mbuf *txm;
 	uint16_t nb_used, nb_tx, num;
 	int error;
-	struct virtio_hw *hw;
 
 	nb_tx = 0;
 
@@ -371,7 +369,6 @@ virtio_xmit_pkts(void *tx_queue, struct
 
 	rmb();
 
-	hw = txvq->hw;
 	num = (uint16_t)(likely(nb_used < VIRTIO_MBUF_BURST_SZ) ? nb_used : VIRTIO_MBUF_BURST_SZ);
 
 	while (nb_tx < nb_pkts) {
@@ -394,9 +391,7 @@ virtio_xmit_pkts(void *tx_queue, struct
 				break;
 			}
 			nb_tx++;
-			hw->eth_stats.obytes += txm->pkt.data_len;
-			hw->eth_stats.q_obytes[txvq->queue_id]
-				+= txm->pkt.data_len;
+			txvq->bytes += txm->pkt.data_len;
 		} else {
 			PMD_TX_LOG(ERR, "No free tx descriptors to transmit\n");
 			break;
@@ -404,8 +399,7 @@ virtio_xmit_pkts(void *tx_queue, struct
 	}
 	vq_update_avail_idx(txvq);
 
-	hw->eth_stats.opackets += nb_tx;
-	hw->eth_stats.q_opackets[txvq->queue_id] += nb_tx;
+	txvq->packets += nb_tx;
 
 	if (unlikely(virtqueue_kick_prepare(txvq))) {
 		virtqueue_notify(txvq);
lib/librte_pmd_virtio/virtqueue.h +5 -0
@@ -154,6 +154,11 @@ struct virtqueue {
 	uint16_t vq_avail_idx;
 	void     *virtio_net_hdr_mem; /**< hdr for each xmit packet */
 
+	/* Statistics */
+	uint64_t	packets;
+	uint64_t	bytes;
+	uint64_t	errors;
+
 	struct vq_desc_extra {
 		void              *cookie;
 		uint16_t          ndescs;
#2 lore raw

PATCH 2/8 virtio: dont double space log messages

In reply to Stephen Hemminger

PMD_INIT_LOG macro already adds a newline, no need to double space.

---
 lib/librte_pmd_virtio/virtio_ethdev.c |   98 +++++++++++++++++-----------------
 lib/librte_pmd_virtio/virtio_rxtx.c   |   28 ++++-----
 lib/librte_pmd_virtio/virtqueue.h     |    4 -
 3 files changed, 66 insertions(+), 64 deletions(-)
lib/librte_pmd_virtio/virtio_ethdev.c +50 -48
@@ -111,13 +111,13 @@ virtio_send_command(struct virtqueue *vq
 
 	if (!vq->hw->cvq) {
 		PMD_INIT_LOG(ERR,
-			     "%s(): Control queue is not supported.\n",
+			     "%s(): Control queue is not supported.",
 			     __func__);
 		return -1;
 	}
 
 	PMD_INIT_LOG(DEBUG, "vq->vq_desc_head_idx = %d, status = %d, "
-		"vq->hw->cvq = %p vq = %p\n",
+		"vq->hw->cvq = %p vq = %p",
 		vq->vq_desc_head_idx, status, vq->hw->cvq, vq);
 
 	if ((vq->vq_free_cnt < ((uint32_t)pkt_num + 2)) || (pkt_num < 1))
@@ -160,7 +160,7 @@ virtio_send_command(struct virtqueue *vq
 	vq_update_avail_ring(vq, head);
 	vq_update_avail_idx(vq);
 
-	PMD_INIT_LOG(DEBUG, "vq->vq_queue_index = %d\n", vq->vq_queue_index);
+	PMD_INIT_LOG(DEBUG, "vq->vq_queue_index = %d", vq->vq_queue_index);
 
 	virtqueue_notify(vq);
 
@@ -191,7 +191,7 @@ virtio_send_command(struct virtqueue *vq
 		vq->vq_free_cnt++;
 	}
 
-	PMD_INIT_LOG(DEBUG, "vq->vq_free_cnt=%d\nvq->vq_desc_head_idx=%d\n",
+	PMD_INIT_LOG(DEBUG, "vq->vq_free_cnt=%d\nvq->vq_desc_head_idx=%d",
 			vq->vq_free_cnt, vq->vq_desc_head_idx);
 
 	memcpy(&result, vq->virtio_net_hdr_mz->addr,
@@ -219,7 +219,7 @@ virtio_set_multiple_queues(struct rte_et
 
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Multiqueue configured but send command "
-			  "failed, this is too late now...\n");
+			  "failed, this is too late now...");
 		return -EINVAL;
 	}
 
@@ -244,24 +244,24 @@ int virtio_dev_queue_setup(struct rte_et
 
 	/* Write the virtqueue index to the Queue Select Field */
 	VIRTIO_WRITE_REG_2(hw, VIRTIO_PCI_QUEUE_SEL, vtpci_queue_idx);
-	PMD_INIT_LOG(DEBUG, "selecting queue: %d\n", vtpci_queue_idx);
+	PMD_INIT_LOG(DEBUG, "selecting queue: %d", vtpci_queue_idx);
 
 	/*
 	 * Read the virtqueue size from the Queue Size field
 	 * Always power of 2 and if 0 virtqueue does not exist
 	 */
 	vq_size = VIRTIO_READ_REG_2(hw, VIRTIO_PCI_QUEUE_NUM);
-	PMD_INIT_LOG(DEBUG, "vq_size: %d nb_desc:%d\n", vq_size, nb_desc);
+	PMD_INIT_LOG(DEBUG, "vq_size: %d nb_desc:%d", vq_size, nb_desc);
 	if (nb_desc == 0)
 		nb_desc = vq_size;
 	if (vq_size == 0) {
-		PMD_INIT_LOG(ERR, "%s: virtqueue does not exist\n", __func__);
+		PMD_INIT_LOG(ERR, "%s: virtqueue does not exist", __func__);
 		return -EINVAL;
 	} else if (!rte_is_power_of_2(vq_size)) {
-		PMD_INIT_LOG(ERR, "%s: virtqueue size is not powerof 2\n", __func__);
+		PMD_INIT_LOG(ERR, "%s: virtqueue size is not powerof 2", __func__);
 		return -EINVAL;
 	} else if (nb_desc != vq_size) {
-		PMD_INIT_LOG(ERR, "Warning: nb_desc(%d) is not equal to vq size (%d), fall to vq size\n",
+		PMD_INIT_LOG(ERR, "Warning: nb_desc(%d) is not equal to vq size (%d), fall to vq size",
 			nb_desc, vq_size);
 		nb_desc = vq_size;
 	}
@@ -287,7 +287,7 @@ int virtio_dev_queue_setup(struct rte_et
 		memcpy(vq->vq_name, vq_name, sizeof(vq->vq_name));
 	}
 	if (vq == NULL) {
-		PMD_INIT_LOG(ERR, "%s: Can not allocate virtqueue\n", __func__);
+		PMD_INIT_LOG(ERR, "%s: Can not allocate virtqueue", __func__);
 		return (-ENOMEM);
 	}
 
@@ -304,7 +304,7 @@ int virtio_dev_queue_setup(struct rte_et
 	 */
 	size = vring_size(vq_size, VIRTIO_PCI_VRING_ALIGN);
 	vq->vq_ring_size = RTE_ALIGN_CEIL(size, VIRTIO_PCI_VRING_ALIGN);
-	PMD_INIT_LOG(DEBUG, "vring_size: %d, rounded_vring_size: %d\n", size, vq->vq_ring_size);
+	PMD_INIT_LOG(DEBUG, "vring_size: %d, rounded_vring_size: %d", size, vq->vq_ring_size);
 
 	mz = rte_memzone_reserve_aligned(vq_name, vq->vq_ring_size,
 		socket_id, 0, VIRTIO_PCI_VRING_ALIGN);
@@ -319,7 +319,7 @@ int virtio_dev_queue_setup(struct rte_et
 	 * Check if the allocated physical memory exceeds 16TB.
 	 */
 	if ((mz->phys_addr + vq->vq_ring_size - 1) >> (VIRTIO_PCI_QUEUE_ADDR_SHIFT + 32)) {
-		PMD_INIT_LOG(ERR, "vring address shouldn't be above 16TB!\n");
+		PMD_INIT_LOG(ERR, "vring address shouldn't be above 16TB!");
 		rte_free(vq);
 		return -ENOMEM;
 	}
@@ -328,8 +328,8 @@ int virtio_dev_queue_setup(struct rte_et
 	vq->mz = mz;
 	vq->vq_ring_mem = mz->phys_addr;
 	vq->vq_ring_virt_mem = mz->addr;
-	PMD_INIT_LOG(DEBUG, "vq->vq_ring_mem:      0x%"PRIx64"\n", (uint64_t)mz->phys_addr);
-	PMD_INIT_LOG(DEBUG, "vq->vq_ring_virt_mem: 0x%"PRIx64"\n", (uint64_t)mz->addr);
+	PMD_INIT_LOG(DEBUG, "vq->vq_ring_mem:      0x%"PRIx64"", (uint64_t)mz->phys_addr);
+	PMD_INIT_LOG(DEBUG, "vq->vq_ring_virt_mem: 0x%"PRIx64"", (uint64_t)mz->addr);
 	vq->virtio_net_hdr_mz  = NULL;
 	vq->virtio_net_hdr_mem = (void *)NULL;
 
@@ -390,7 +390,7 @@ virtio_dev_cq_queue_setup(struct rte_eth
 			vtpci_queue_idx, nb_desc, socket_id, &vq);
 
 	if (ret < 0) {
-		PMD_INIT_LOG(ERR, "control vq initialization failed\n");
+		PMD_INIT_LOG(ERR, "control vq initialization failed");
 		return ret;
 	}
 
@@ -582,12 +582,12 @@ virtio_negotiate_features(struct virtio_
 
 	/* Prepare guest_features: feature that driver wants to support */
 	guest_features = VTNET_FEATURES & ~mask;
-	PMD_INIT_LOG(DEBUG, "guest_features before negotiate = %x\n",
+	PMD_INIT_LOG(DEBUG, "guest_features before negotiate = %x",
 		guest_features);
 
 	/* Read device(host) feature bits */
 	hw->host_features = VIRTIO_READ_REG_4(hw, VIRTIO_PCI_HOST_FEATURES);
-	PMD_INIT_LOG(DEBUG, "host_features before negotiate = %x\n",
+	PMD_INIT_LOG(DEBUG, "host_features before negotiate = %x",
 		hw->host_features);
 
 	/*
@@ -595,7 +595,7 @@ virtio_negotiate_features(struct virtio_
 	 * guest feature bits.
 	 */
 	hw->guest_features = vtpci_negotiate_features(hw, guest_features);
-	PMD_INIT_LOG(DEBUG, "features after negotiate = %x\n",
+	PMD_INIT_LOG(DEBUG, "features after negotiate = %x",
 		hw->guest_features);
 }
 
@@ -609,20 +609,20 @@ parse_sysfs_value(const char *filename,
 
 	f = fopen(filename, "r");
 	if (f == NULL) {
-		PMD_INIT_LOG(ERR, "%s(): cannot open sysfs value %s\n",
+		PMD_INIT_LOG(ERR, "%s(): cannot open sysfs value %s",
 			     __func__, filename);
 		return -1;
 	}
 
 	if (fgets(buf, sizeof(buf), f) == NULL) {
-		PMD_INIT_LOG(ERR, "%s(): cannot read sysfs value %s\n",
+		PMD_INIT_LOG(ERR, "%s(): cannot read sysfs value %s",
 			     __func__, filename);
 		fclose(f);
 		return -1;
 	}
 	*val = strtoul(buf, &end, 0);
 	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
-		PMD_INIT_LOG(ERR, "%s(): cannot parse sysfs value %s\n",
+		PMD_INIT_LOG(ERR, "%s(): cannot parse sysfs value %s",
 			     __func__, filename);
 		fclose(f);
 		return -1;
@@ -652,7 +652,7 @@ static int get_uio_dev(struct rte_pci_ad
 		dir = opendir(dirname);
 
 		if (dir == NULL) {
-			PMD_INIT_LOG(ERR, "Cannot opendir %s\n", dirname);
+			PMD_INIT_LOG(ERR, "Cannot opendir %s", dirname);
 			return -1;
 		}
 	}
@@ -689,7 +689,7 @@ static int get_uio_dev(struct rte_pci_ad
 
 	/* No uio resource found */
 	if (e == NULL) {
-		PMD_INIT_LOG(ERR, "Could not find uio resource\n");
+		PMD_INIT_LOG(ERR, "Could not find uio resource");
 		return -1;
 	}
 
@@ -748,7 +748,7 @@ eth_virtio_dev_init(__rte_unused struct
 		rte_snprintf(filename, sizeof(filename),
 			     "%s/portio/port0/size", dirname);
 		if (parse_sysfs_value(filename, &size) < 0) {
-			PMD_INIT_LOG(ERR, "%s(): cannot parse size\n",
+			PMD_INIT_LOG(ERR, "%s(): cannot parse size",
 				     __func__);
 			return -1;
 		}
@@ -757,14 +757,14 @@ eth_virtio_dev_init(__rte_unused struct
 		rte_snprintf(filename, sizeof(filename),
 			     "%s/portio/port0/start", dirname);
 		if (parse_sysfs_value(filename, &start) < 0) {
-			PMD_INIT_LOG(ERR, "%s(): cannot parse portio start\n",
+			PMD_INIT_LOG(ERR, "%s(): cannot parse portio start",
 				     __func__);
 			return -1;
 		}
 		pci_dev->mem_resource[0].addr = (void *)(uintptr_t)start;
 		pci_dev->mem_resource[0].len =  (uint64_t)size;
 		PMD_INIT_LOG(DEBUG,
-			     "PCI Port IO found start=0x%lx with size=0x%lx\n",
+			     "PCI Port IO found start=0x%lx with size=0x%lx",
 			     start, size);
 	}
 #endif
@@ -800,7 +800,7 @@ eth_virtio_dev_init(__rte_unused struct
 	ether_addr_copy((struct ether_addr *) hw->mac_addr,
 			&eth_dev->data->mac_addrs[0]);
 	PMD_INIT_LOG(DEBUG,
-		     "PORT MAC: %02X:%02X:%02X:%02X:%02X:%02X\n",
+		     "PORT MAC: %02X:%02X:%02X:%02X:%02X:%02X",
 		     hw->mac_addr[0], hw->mac_addr[1], hw->mac_addr[2],
 		     hw->mac_addr[3], hw->mac_addr[4], hw->mac_addr[5]);
 
@@ -811,7 +811,7 @@ eth_virtio_dev_init(__rte_unused struct
 			offset_conf += sizeof(config->status);
 		} else {
 			PMD_INIT_LOG(DEBUG,
-				     "VIRTIO_NET_F_STATUS is not supported\n");
+				     "VIRTIO_NET_F_STATUS is not supported");
 			config->status = 0;
 		}
 
@@ -819,7 +819,7 @@ eth_virtio_dev_init(__rte_unused struct
 			offset_conf += sizeof(config->max_virtqueue_pairs);
 		} else {
 			PMD_INIT_LOG(DEBUG,
-				     "VIRTIO_NET_F_MQ is not supported\n");
+				     "VIRTIO_NET_F_MQ is not supported");
 			config->max_virtqueue_pairs = 1;
 		}
 
@@ -836,11 +836,11 @@ eth_virtio_dev_init(__rte_unused struct
 					config->max_virtqueue_pairs * 2,
 					SOCKET_ID_ANY);
 
-		PMD_INIT_LOG(DEBUG, "config->max_virtqueue_pairs=%d\n",
+		PMD_INIT_LOG(DEBUG, "config->max_virtqueue_pairs=%d",
 				config->max_virtqueue_pairs);
-		PMD_INIT_LOG(DEBUG, "config->status=%d\n", config->status);
+		PMD_INIT_LOG(DEBUG, "config->status=%d", config->status);
 		PMD_INIT_LOG(DEBUG,
-				"PORT MAC: %02X:%02X:%02X:%02X:%02X:%02X\n",
+				"PORT MAC: %02X:%02X:%02X:%02X:%02X:%02X",
 				config->mac[0], config->mac[1],
 				config->mac[2], config->mac[3],
 				config->mac[4], config->mac[5]);
@@ -852,7 +852,7 @@ eth_virtio_dev_init(__rte_unused struct
 	eth_dev->data->nb_rx_queues = hw->max_rx_queues;
 	eth_dev->data->nb_tx_queues = hw->max_tx_queues;
 
-	PMD_INIT_LOG(DEBUG, "hw->max_rx_queues=%d   hw->max_tx_queues=%d\n",
+	PMD_INIT_LOG(DEBUG, "hw->max_rx_queues=%d   hw->max_tx_queues=%d",
 			hw->max_rx_queues, hw->max_tx_queues);
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
 			eth_dev->data->port_id, pci_dev->id.vendor_id,
@@ -934,10 +934,12 @@ virtio_dev_start(struct rte_eth_dev *dev
 				offsetof(struct virtio_net_config, status),
 				&status, sizeof(status));
 		if ((status & VIRTIO_NET_S_LINK_UP) == 0) {
-			PMD_INIT_LOG(ERR,     "Port: %d Link is DOWN\n", dev->data->port_id);
+			PMD_INIT_LOG(ERR, "Port: %d Link is DOWN",
+				     dev->data->port_id);
 			return -EIO;
 		} else {
-			PMD_INIT_LOG(DEBUG, "Port: %d Link is UP\n",  dev->data->port_id);
+			PMD_INIT_LOG(DEBUG, "Port: %d Link is UP",
+				     dev->data->port_id);
 		}
 	}
 	vtpci_reinit_complete(hw);
@@ -952,12 +954,12 @@ virtio_dev_start(struct rte_eth_dev *dev
 			return -EINVAL;
 	}
 
-	PMD_INIT_LOG(DEBUG, "nb_queues=%d\n", nb_queues);
+	PMD_INIT_LOG(DEBUG, "nb_queues=%d", nb_queues);
 
 	for (i = 0; i < nb_queues; i++)
 		virtqueue_notify(dev->data->rx_queues[i]);
 
-	PMD_INIT_LOG(DEBUG, "Notified backend at initialization\n");
+	PMD_INIT_LOG(DEBUG, "Notified backend at initialization");
 
 	for (i = 0; i < dev->data->nb_rx_queues; i++)
 		VIRTQUEUE_DUMP((struct virtqueue *)dev->data->rx_queues[i]);
@@ -975,7 +977,7 @@ static void virtio_dev_free_mbufs(struct
 
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		PMD_INIT_LOG(DEBUG,
-			     "Before freeing rxq[%d] used and unused buf\n", i);
+			     "Before freeing rxq[%d] used and unused buf", i);
 		VIRTQUEUE_DUMP((struct virtqueue *)dev->data->rx_queues[i]);
 
 		while ((buf = (struct rte_mbuf *)virtqueue_detatch_unused(
@@ -984,15 +986,15 @@ static void virtio_dev_free_mbufs(struct
 			mbuf_num++;
 		}
 
-		PMD_INIT_LOG(DEBUG, "free %d mbufs\n", mbuf_num);
+		PMD_INIT_LOG(DEBUG, "free %d mbufs", mbuf_num);
 		PMD_INIT_LOG(DEBUG,
-			     "After freeing rxq[%d] used and unused buf\n", i);
+			     "After freeing rxq[%d] used and unused buf", i);
 		VIRTQUEUE_DUMP((struct virtqueue *)dev->data->rx_queues[i]);
 	}
 
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
 		PMD_INIT_LOG(DEBUG,
-			     "Before freeing txq[%d] used and unused bufs\n",
+			     "Before freeing txq[%d] used and unused bufs",
 			     i);
 		VIRTQUEUE_DUMP((struct virtqueue *)dev->data->tx_queues[i]);
 
@@ -1003,9 +1005,9 @@ static void virtio_dev_free_mbufs(struct
 			mbuf_num++;
 		}
 
-		PMD_INIT_LOG(DEBUG, "free %d mbufs\n", mbuf_num);
-		PMD_INIT_LOG(DEBUG, "After freeing txq[%d] used and "
-			"unused buf\n", i);
+		PMD_INIT_LOG(DEBUG, "free %d mbufs", mbuf_num);
+		PMD_INIT_LOG(DEBUG,
+			     "After freeing txq[%d] used and unused buf", i);
 		VIRTQUEUE_DUMP((struct virtqueue *)dev->data->tx_queues[i]);
 	}
 }
@@ -1037,17 +1039,17 @@ virtio_dev_link_update(struct rte_eth_de
 	link.link_duplex = FULL_DUPLEX;
 	link.link_speed  = SPEED_10G;
 	if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS)) {
-		PMD_INIT_LOG(DEBUG, "Get link status from hw\n");
+		PMD_INIT_LOG(DEBUG, "Get link status from hw");
 		vtpci_read_dev_config(hw,
 				offsetof(struct virtio_net_config, status),
 				&status, sizeof(status));
 		if ((status & VIRTIO_NET_S_LINK_UP) == 0) {
 			link.link_status = 0;
-			PMD_INIT_LOG(DEBUG, "Port %d is down\n",
+			PMD_INIT_LOG(DEBUG, "Port %d is down",
 				     dev->data->port_id);
 		} else {
 			link.link_status = 1;
-			PMD_INIT_LOG(DEBUG, "Port %d is up\n",
+			PMD_INIT_LOG(DEBUG, "Port %d is up",
 				     dev->data->port_id);
 		}
 	} else {
lib/librte_pmd_virtio/virtio_rxtx.c +14 -14
@@ -105,13 +105,13 @@ virtio_dev_vring_start(struct rte_eth_de
 
 	rte_snprintf(vq_name, sizeof(vq_name), "port_%d_rx_vq",
 					dev->data->port_id);
-	PMD_INIT_LOG(DEBUG, "vq name: %s\n", vq->vq_name);
+	PMD_INIT_LOG(DEBUG, "vq name: %s", vq->vq_name);
 
 	/* Only rx virtqueue needs mbufs to be allocated at initialization */
 	if (queue_type == VTNET_RQ) {
 		if (vq->mpool == NULL)
 			rte_exit(EXIT_FAILURE,
-			"Cannot allocate initial mbufs for rx virtqueue\n");
+			"Cannot allocate initial mbufs for rx virtqueue");
 
 		/* Allocate blank mbufs for the each rx descriptor */
 		nbufs = 0;
@@ -135,7 +135,7 @@ virtio_dev_vring_start(struct rte_eth_de
 
 		vq_update_avail_idx(vq);
 
-		PMD_INIT_LOG(DEBUG, "Allocated %d bufs\n", nbufs);
+		PMD_INIT_LOG(DEBUG, "Allocated %d bufs", nbufs);
 
 		VIRTIO_WRITE_REG_2(vq->hw, VIRTIO_PCI_QUEUE_SEL,
 			vq->vq_queue_index);
@@ -207,7 +207,7 @@ virtio_dev_rx_queue_setup(struct rte_eth
 	ret = virtio_dev_queue_setup(dev, VTNET_RQ, queue_idx, vtpci_queue_idx,
 			nb_desc, socket_id, &vq);
 	if (ret < 0) {
-		PMD_INIT_LOG(ERR, "tvq initialization failed\n");
+		PMD_INIT_LOG(ERR, "tvq initialization failed");
 		return ret;
 	}
 
@@ -240,7 +240,7 @@ virtio_dev_tx_queue_setup(struct rte_eth
 	ret = virtio_dev_queue_setup(dev, VTNET_TQ, queue_idx, vtpci_queue_idx,
 			nb_desc, socket_id, &vq);
 	if (ret < 0) {
-		PMD_INIT_LOG(ERR, "rvq initialization failed\n");
+		PMD_INIT_LOG(ERR, "rvq initialization failed");
 		return ret;
 	}
 
@@ -290,15 +290,15 @@ virtio_recv_pkts(void *rx_queue, struct
 		return 0;
 
 	num = virtqueue_dequeue_burst_rx(rxvq, rcv_pkts, len, num);
-	PMD_RX_LOG(DEBUG, "used:%d dequeue:%d\n", nb_used, num);
+	PMD_RX_LOG(DEBUG, "used:%d dequeue:%d", nb_used, num);
 	for (i = 0; i < num ; i++) {
 		rxm = rcv_pkts[i];
 
-		PMD_RX_LOG(DEBUG, "packet len:%d\n", len[i]);
+		PMD_RX_LOG(DEBUG, "packet len:%d", len[i]);
 
 		if (unlikely(len[i]
 			     < (uint32_t)hw->vtnet_hdr_size + ETHER_HDR_LEN)) {
-			PMD_RX_LOG(ERR, "Packet drop\n");
+			PMD_RX_LOG(ERR, "Packet drop");
 			nb_enqueued++;
 			virtio_discard_rxbuf(rxvq, rxm);
 			rxvq->errors++;
@@ -342,7 +342,7 @@ virtio_recv_pkts(void *rx_queue, struct
 	if (likely(nb_enqueued)) {
 		if (unlikely(virtqueue_kick_prepare(rxvq))) {
 			virtqueue_notify(rxvq);
-			PMD_RX_LOG(DEBUG, "Notified\n");
+			PMD_RX_LOG(DEBUG, "Notified");
 		}
 	}
 
@@ -383,17 +383,17 @@ virtio_xmit_pkts(void *tx_queue, struct
 			error = virtqueue_enqueue_xmit(txvq, txm);
 			if (unlikely(error)) {
 				if (error == ENOSPC)
-					PMD_TX_LOG(ERR, "virtqueue_enqueue Free count = 0\n");
+					PMD_TX_LOG(ERR, "virtqueue_enqueue Free count = 0");
 				else if (error == EMSGSIZE)
-					PMD_TX_LOG(ERR, "virtqueue_enqueue Free count < 1\n");
+					PMD_TX_LOG(ERR, "virtqueue_enqueue Free count < 1");
 				else
-					PMD_TX_LOG(ERR, "virtqueue_enqueue error: %d\n", error);
+					PMD_TX_LOG(ERR, "virtqueue_enqueue error: %d", error);
 				break;
 			}
 			nb_tx++;
 			txvq->bytes += txm->pkt.data_len;
 		} else {
-			PMD_TX_LOG(ERR, "No free tx descriptors to transmit\n");
+			PMD_TX_LOG(ERR, "No free tx descriptors to transmit");
 			break;
 		}
 	}
@@ -403,7 +403,7 @@ virtio_xmit_pkts(void *tx_queue, struct
 
 	if (unlikely(virtqueue_kick_prepare(txvq))) {
 		virtqueue_notify(txvq);
-		PMD_TX_LOG(DEBUG, "Notified backend after xmit\n");
+		PMD_TX_LOG(DEBUG, "Notified backend after xmit");
 	}
 
 	return nb_tx;
lib/librte_pmd_virtio/virtqueue.h +2 -2
@@ -393,7 +393,7 @@ virtqueue_dequeue_burst_rx(struct virtqu
 		cookie = (struct rte_mbuf *)vq->vq_descx[desc_idx].cookie;
 
 		if (unlikely(cookie == NULL)) {
-			PMD_DRV_LOG(ERR, "vring descriptor with no mbuf cookie at %u\n",
+			PMD_DRV_LOG(ERR, "vring descriptor with no mbuf cookie at %u",
 				vq->vq_used_cons_idx);
 			break;
 		}
@@ -432,7 +432,7 @@ virtqueue_dequeue_pkt_tx(struct virtqueu
 	PMD_INIT_LOG(DEBUG, \
 	  "VQ: %s - size=%d; free=%d; used=%d; desc_head_idx=%d;" \
 	  " avail.idx=%d; used_cons_idx=%d; used.idx=%d;" \
-	  " avail.flags=0x%x; used.flags=0x%x\n", \
+	  " avail.flags=0x%x; used.flags=0x%x", \
 	  (vq)->vq_name, (vq)->vq_nentries, (vq)->vq_free_cnt, nused, \
 	  (vq)->vq_desc_head_idx, (vq)->vq_ring.avail->idx, \
 	  (vq)->vq_used_cons_idx, (vq)->vq_ring.used->idx, \
#3 lore raw

PATCH 3/8 virtio: deinline some code

In reply to Stephen Hemminger

This driver has lots of functions marked always inline which is actually counterproductive with modern compilers. Better to move the functions to the one file they are used (proper scope) and let compiler decide.

For trivial functions leave them as static inline.

---
 lib/librte_pmd_virtio/virtio_rxtx.c |  160 +++++++++++++++++++++++++++++++++
 lib/librte_pmd_virtio/virtqueue.h   |  170 ------------------------------------
 2 files changed, 164 insertions(+), 166 deletions(-)
lib/librte_pmd_virtio/virtio_rxtx.c +160 -0
@@ -60,6 +60,166 @@
 #define  VIRTIO_DUMP_PACKET(m, len) do { } while (0)
 #endif
 
+static void
+vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx)
+{
+	struct vring_desc *dp, *dp_tail;
+	struct vq_desc_extra *dxp;
+	uint16_t desc_idx_last = desc_idx;
+
+	dp  = &vq->vq_ring.desc[desc_idx];
+	dxp = &vq->vq_descx[desc_idx];
+	vq->vq_free_cnt = (uint16_t)(vq->vq_free_cnt + dxp->ndescs);
+	if ((dp->flags & VRING_DESC_F_INDIRECT) == 0) {
+		while (dp->flags & VRING_DESC_F_NEXT) {
+			desc_idx_last = dp->next;
+			dp = &vq->vq_ring.desc[dp->next];
+		}
+	}
+	dxp->ndescs = 0;
+
+	/*
+	 * We must append the existing free chain, if any, to the end of
+	 * newly freed chain. If the virtqueue was completely used, then
+	 * head would be VQ_RING_DESC_CHAIN_END (ASSERTed above).
+	 */
+	if (vq->vq_desc_tail_idx == VQ_RING_DESC_CHAIN_END) {
+		vq->vq_desc_head_idx = desc_idx;
+	} else {
+		dp_tail = &vq->vq_ring.desc[vq->vq_desc_tail_idx];
+		dp_tail->next = desc_idx;
+	}
+
+	vq->vq_desc_tail_idx = desc_idx_last;
+	dp->next = VQ_RING_DESC_CHAIN_END;
+}
+
+static uint16_t
+virtqueue_dequeue_burst_rx(struct virtqueue *vq, struct rte_mbuf **rx_pkts,
+			   uint32_t *len, uint16_t num)
+{
+	struct vring_used_elem *uep;
+	struct rte_mbuf *cookie;
+	uint16_t used_idx, desc_idx;
+	uint16_t i;
+
+	/*  Caller does the check */
+	for (i = 0; i < num ; i++) {
+		used_idx = (uint16_t)(vq->vq_used_cons_idx & (vq->vq_nentries - 1));
+		uep = &vq->vq_ring.used->ring[used_idx];
+		desc_idx = (uint16_t) uep->id;
+		len[i] = uep->len;
+		cookie = (struct rte_mbuf *)vq->vq_descx[desc_idx].cookie;
+
+		if (unlikely(cookie == NULL)) {
+			PMD_DRV_LOG(ERR, "vring descriptor with no mbuf cookie at %u\n",
+				vq->vq_used_cons_idx);
+			break;
+		}
+
+		rte_prefetch0(cookie);
+		rte_packet_prefetch(cookie->pkt.data);
+		rx_pkts[i]  = cookie;
+		vq->vq_used_cons_idx++;
+		vq_ring_free_chain(vq, desc_idx);
+		vq->vq_descx[desc_idx].cookie = NULL;
+	}
+
+	return i;
+}
+
+static void
+virtqueue_dequeue_pkt_tx(struct virtqueue *vq)
+{
+	struct vring_used_elem *uep;
+	uint16_t used_idx, desc_idx;
+
+	used_idx = (uint16_t)(vq->vq_used_cons_idx & (vq->vq_nentries - 1));
+	uep = &vq->vq_ring.used->ring[used_idx];
+	desc_idx = (uint16_t) uep->id;
+	vq->vq_used_cons_idx++;
+	vq_ring_free_chain(vq, desc_idx);
+}
+
+
+static inline int
+virtqueue_enqueue_recv_refill(struct virtqueue *vq, struct rte_mbuf *cookie)
+{
+	struct vq_desc_extra *dxp;
+	struct vring_desc *start_dp;
+	uint16_t needed = 1;
+	uint16_t head_idx, idx;
+
+	if (unlikely(vq->vq_free_cnt == 0))
+		return -ENOSPC;
+	if (unlikely(vq->vq_free_cnt < needed))
+		return -EMSGSIZE;
+
+	head_idx = vq->vq_desc_head_idx;
+	if (unlikely(head_idx >= vq->vq_nentries))
+		return -EFAULT;
+
+	idx = head_idx;
+	dxp = &vq->vq_descx[idx];
+	dxp->cookie = (void *)cookie;
+	dxp->ndescs = needed;
+
+	start_dp = vq->vq_ring.desc;
+	start_dp[idx].addr  =
+		(uint64_t) (cookie->buf_physaddr + RTE_PKTMBUF_HEADROOM - sizeof(struct virtio_net_hdr));
+	start_dp[idx].len   = cookie->buf_len - RTE_PKTMBUF_HEADROOM + sizeof(struct virtio_net_hdr);
+	start_dp[idx].flags =  VRING_DESC_F_WRITE;
+	idx = start_dp[idx].next;
+	vq->vq_desc_head_idx = idx;
+	if (vq->vq_desc_head_idx == VQ_RING_DESC_CHAIN_END)
+		vq->vq_desc_tail_idx = idx;
+	vq->vq_free_cnt = (uint16_t)(vq->vq_free_cnt - needed);
+	vq_update_avail_ring(vq, head_idx);
+
+	return 0;
+}
+
+static int
+virtqueue_enqueue_xmit(struct virtqueue *txvq, struct rte_mbuf *cookie)
+{
+	struct vq_desc_extra *dxp;
+	struct vring_desc *start_dp;
+	uint16_t needed = 2;
+	uint16_t head_idx, idx;
+
+	if (unlikely(txvq->vq_free_cnt == 0))
+		return -ENOSPC;
+	if (unlikely(txvq->vq_free_cnt < needed))
+		return -EMSGSIZE;
+	head_idx = txvq->vq_desc_head_idx;
+	if (unlikely(head_idx >= txvq->vq_nentries))
+		return -EFAULT;
+
+	idx = head_idx;
+	dxp = &txvq->vq_descx[idx];
+	if (dxp->cookie != NULL)
+		rte_pktmbuf_free_seg(dxp->cookie);
+	dxp->cookie = (void *)cookie;
+	dxp->ndescs = needed;
+
+	start_dp = txvq->vq_ring.desc;
+	start_dp[idx].addr  = (uint64_t)(uintptr_t)txvq->virtio_net_hdr_mem + idx * sizeof(struct virtio_net_hdr);
+	start_dp[idx].len   = sizeof(struct virtio_net_hdr);
+	start_dp[idx].flags = VRING_DESC_F_NEXT;
+	idx = start_dp[idx].next;
+	start_dp[idx].addr  = RTE_MBUF_DATA_DMA_ADDR(cookie);
+	start_dp[idx].len   = cookie->pkt.data_len;
+	start_dp[idx].flags = 0;
+	idx = start_dp[idx].next;
+	txvq->vq_desc_head_idx = idx;
+	if (txvq->vq_desc_head_idx == VQ_RING_DESC_CHAIN_END)
+		txvq->vq_desc_tail_idx = idx;
+	txvq->vq_free_cnt = (uint16_t)(txvq->vq_free_cnt - needed);
+	vq_update_avail_ring(txvq, head_idx);
+
+	return 0;
+}
+
 static inline struct rte_mbuf *
 rte_rxmbuf_alloc(struct rte_mempool *mp)
 {
lib/librte_pmd_virtio/virtqueue.h +4 -164

197 lines not on this page · raw message

#4 lore raw

PATCH 4/8 virtio: check for transmit checksum config error

In reply to Stephen Hemminger

This driver does not support transmit checksum or vlan offload therefore check for this when device is configured.

---
 lib/librte_ether/rte_ethdev.h       |    8 +++++---
 lib/librte_pmd_virtio/virtio_rxtx.c |    9 ++++++++-
 2 files changed, 13 insertions(+), 4 deletions(-)
lib/librte_pmd_virtio/virtio_rxtx.c +8 -1
@@ -390,13 +390,20 @@ virtio_dev_tx_queue_setup(struct rte_eth
 			uint16_t queue_idx,
 			uint16_t nb_desc,
 			unsigned int socket_id,
-			__rte_unused const struct rte_eth_txconf *tx_conf)
+			const struct rte_eth_txconf *tx_conf)
 {
 	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
 	struct virtqueue *vq;
 	int ret;
 
 	PMD_INIT_FUNC_TRACE();
+
+	if ((tx_conf->txq_flags & ETH_TXQ_FLAGS_NOOFFLOADS)
+	    != ETH_TXQ_FLAGS_NOOFFLOADS) {
+		PMD_INIT_LOG(ERR, "TX checksum offload not supported\n");
+		return -EINVAL;
+	}
+
 	ret = virtio_dev_queue_setup(dev, VTNET_TQ, queue_idx, vtpci_queue_idx,
 			nb_desc, socket_id, &vq);
 	if (ret < 0) {
#5 lore raw

PATCH 5/8 virtio: check for ip checksum offload

In reply to Stephen Hemminger

This driver does not support receive IP checksum offload, therefore must check and return error if configured incorrectly.

lib/librte_pmd_virtio/virtio_ethdev.c +10 -1
@@ -901,8 +901,17 @@ virtio_dev_tx_queue_release(__rte_unused
  * It returns 0 on success.
  */
 static int
-virtio_dev_configure(__rte_unused struct rte_eth_dev *dev)
+virtio_dev_configure(struct rte_eth_dev *dev)
 {
+	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
+
+	PMD_INIT_LOG(DEBUG, "configure");
+
+	if (rxmode->hw_ip_checksum){
+		PMD_DRV_LOG(ERR, "HW IP checksum not supported");
+		return (-EINVAL);
+	}
+
 	return 0;
 }
 
#6 lore raw

PATCH 6/8 virtio: remove unused virtqueue name

In reply to Stephen Hemminger

vq_name is only used when setting up queue, and does not need to be saved.

lib/librte_pmd_virtio/virtio_ethdev.c +0 -3
@@ -271,20 +271,17 @@ int virtio_dev_queue_setup(struct rte_et
 			dev->data->port_id, queue_idx);
 		vq = rte_zmalloc(vq_name, sizeof(struct virtqueue) +
 			vq_size * sizeof(struct vq_desc_extra), CACHE_LINE_SIZE);
-		memcpy(vq->vq_name, vq_name, sizeof(vq->vq_name));
 	} else if (queue_type == VTNET_TQ) {
 		rte_snprintf(vq_name, sizeof(vq_name), "port%d_tvq%d",
 			dev->data->port_id, queue_idx);
 		vq = rte_zmalloc(vq_name, sizeof(struct virtqueue) +
 			vq_size * sizeof(struct vq_desc_extra), CACHE_LINE_SIZE);
-		memcpy(vq->vq_name, vq_name, sizeof(vq->vq_name));
 	} else if (queue_type == VTNET_CQ) {
 		rte_snprintf(vq_name, sizeof(vq_name), "port%d_cvq",
 			dev->data->port_id);
 		vq = rte_zmalloc(vq_name, sizeof(struct virtqueue) +
 			vq_size * sizeof(struct vq_desc_extra),
 			CACHE_LINE_SIZE);
-		memcpy(vq->vq_name, vq_name, sizeof(vq->vq_name));
 	}
 	if (vq == NULL) {
 		PMD_INIT_LOG(ERR, "%s: Can not allocate virtqueue", __func__);
lib/librte_pmd_virtio/virtio_rxtx.c +5 -9
@@ -232,13 +232,13 @@ rte_rxmbuf_alloc(struct rte_mempool *mp)
 }
 
 static void
-virtio_dev_vring_start(struct rte_eth_dev *dev, struct virtqueue *vq, int queue_type)
+virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 {
 	struct rte_mbuf *m;
 	int i, nbufs, error, size = vq->vq_nentries;
 	struct vring *vr = &vq->vq_ring;
 	uint8_t *ring_mem = vq->vq_ring_virt_mem;
-	char vq_name[VIRTQUEUE_MAX_NAME_SZ];
+
 	PMD_INIT_FUNC_TRACE();
 
 	/*
@@ -263,10 +263,6 @@ virtio_dev_vring_start(struct rte_eth_de
 	 */
 	virtqueue_disable_intr(vq);
 
-	rte_snprintf(vq_name, sizeof(vq_name), "port_%d_rx_vq",
-					dev->data->port_id);
-	PMD_INIT_LOG(DEBUG, "vq name: %s", vq->vq_name);
-
 	/* Only rx virtqueue needs mbufs to be allocated at initialization */
 	if (queue_type == VTNET_RQ) {
 		if (vq->mpool == NULL)
@@ -320,7 +316,7 @@ virtio_dev_cq_start(struct rte_eth_dev *
 	struct virtio_hw *hw
 		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	virtio_dev_vring_start(dev, hw->cvq, VTNET_CQ);
+	virtio_dev_vring_start(hw->cvq, VTNET_CQ);
 	VIRTQUEUE_DUMP((struct virtqueue *)hw->cvq);
 }
 
@@ -340,13 +336,13 @@ virtio_dev_rxtx_start(struct rte_eth_dev
 
 	/* Start rx vring. */
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
-		virtio_dev_vring_start(dev, dev->data->rx_queues[i], VTNET_RQ);
+		virtio_dev_vring_start(dev->data->rx_queues[i], VTNET_RQ);
 		VIRTQUEUE_DUMP((struct virtqueue *)dev->data->rx_queues[i]);
 	}
 
 	/* Start tx vring. */
 	for (i = 0; i < dev->data->nb_tx_queues; i++) {
-		virtio_dev_vring_start(dev, dev->data->tx_queues[i], VTNET_TQ);
+		virtio_dev_vring_start(dev->data->tx_queues[i], VTNET_TQ);
 		VIRTQUEUE_DUMP((struct virtqueue *)dev->data->tx_queues[i]);
 	}
 }
lib/librte_pmd_virtio/virtqueue.h +0 -1
@@ -122,7 +122,6 @@ struct virtio_pmd_ctrl {
 };
 
 struct virtqueue {
-	char        vq_name[VIRTQUEUE_MAX_NAME_SZ];
 	struct virtio_hw         *hw;     /**< virtio_hw structure pointer. */
 	const struct rte_memzone *mz;     /**< mem zone to populate RX ring. */
 	const struct rte_memzone *virtio_net_hdr_mz; /**< memzone to populate hdr. */
#7 lore raw

PATCH 7/8 virtio: remove unused adapter_stopped field

In reply to Stephen Hemminger

This flag was set to zero (but was already zero)
and never used.

lib/librte_pmd_virtio/virtio_ethdev.c +0 -2
@@ -927,8 +927,6 @@ virtio_dev_start(struct rte_eth_dev *dev
 	/* Tell the host we've known how to drive the device. */
 	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER);
 
-	hw->adapter_stopped = 0;
-
 	virtio_dev_cq_start(dev);
 
 	/* Do final configuration before rx/tx engine starts */
lib/librte_pmd_virtio/virtio_pci.h +0 -1
@@ -178,7 +178,6 @@ struct virtio_hw {
 	uint16_t    subsystem_vendor_id;
 	uint8_t     revision_id;
 	uint8_t     mac_addr[ETHER_ADDR_LEN];
-	int         adapter_stopped;
 };
 
 /*
#8 lore raw

PATCH 8/8 virtio: simplify the hardware structure

In reply to Stephen Hemminger

The host_features are never used after negotiation.
The PCI information is unused (and available in rte_pci if needed).

lib/librte_pmd_virtio/virtio_ethdev.c +5 -7
@@ -559,7 +559,7 @@ virtio_get_hwaddr(struct virtio_hw *hw)
 static void
 virtio_negotiate_features(struct virtio_hw *hw)
 {
-	uint32_t guest_features, mask;
+	uint32_t host_features, mask;
 
 	mask = VIRTIO_NET_F_CTRL_RX | VIRTIO_NET_F_CTRL_VLAN;
 	mask |= VIRTIO_NET_F_CSUM | VIRTIO_NET_F_GUEST_CSUM;
@@ -578,20 +578,20 @@ virtio_negotiate_features(struct virtio_
 	mask |= VIRTIO_RING_F_INDIRECT_DESC;
 
 	/* Prepare guest_features: feature that driver wants to support */
-	guest_features = VTNET_FEATURES & ~mask;
+	hw->guest_features = VTNET_FEATURES & ~mask;
 	PMD_INIT_LOG(DEBUG, "guest_features before negotiate = %x",
 		guest_features);
 
 	/* Read device(host) feature bits */
-	hw->host_features = VIRTIO_READ_REG_4(hw, VIRTIO_PCI_HOST_FEATURES);
+	host_features = VIRTIO_READ_REG_4(hw, VIRTIO_PCI_HOST_FEATURES);
 	PMD_INIT_LOG(DEBUG, "host_features before negotiate = %x",
-		hw->host_features);
+		host_features);
 
 	/*
 	 * Negotiate features: Subset of device feature bits are written back
 	 * guest feature bits.
 	 */
-	hw->guest_features = vtpci_negotiate_features(hw, guest_features);
+	hw->guest_features = vtpci_negotiate_features(hw, host_features);
 	PMD_INIT_LOG(DEBUG, "features after negotiate = %x",
 		hw->guest_features);
 }
@@ -730,8 +730,6 @@ eth_virtio_dev_init(__rte_unused struct
 
 	pci_dev = eth_dev->pci_dev;
 
-	hw->device_id = pci_dev->id.device_id;
-	hw->vendor_id = pci_dev->id.vendor_id;
 #ifdef RTE_EXEC_ENV_LINUXAPP
 	{
 		char dirname[PATH_MAX];
lib/librte_pmd_virtio/virtio_pci.c +2 -2
@@ -82,14 +82,14 @@ vtpci_write_dev_config(struct virtio_hw
 }
 
 uint32_t
-vtpci_negotiate_features(struct virtio_hw *hw, uint32_t guest_features)
+vtpci_negotiate_features(struct virtio_hw *hw, uint32_t host_features)
 {
 	uint32_t features;
 	/*
 	 * Limit negotiated features to what the driver, virtqueue, and
 	 * host all support.
 	 */
-	features = (hw->host_features) & guest_features;
+	features = host_features & hw->guest_features;
 
 	VIRTIO_WRITE_REG_4(hw, VIRTIO_PCI_GUEST_FEATURES, features);
 	return features;
lib/librte_pmd_virtio/virtio_pci.h +2 -10
@@ -162,21 +162,13 @@ struct virtqueue;
 #define VIRTIO_MAX_VIRTQUEUES 8
 
 struct virtio_hw {
+	struct virtqueue *cvq;
 	uint32_t    io_base;
-	uint32_t    host_features;
 	uint32_t    guest_features;
 
-	struct virtqueue *cvq;
-
-	uint16_t    vtnet_hdr_size;
-
 	uint32_t    max_tx_queues;
 	uint32_t    max_rx_queues;
-	uint16_t    device_id;
-	uint16_t    vendor_id;
-	uint16_t    subsystem_device_id;
-	uint16_t    subsystem_vendor_id;
-	uint8_t     revision_id;
+	uint16_t    vtnet_hdr_size;
 	uint8_t     mac_addr[ETHER_ADDR_LEN];
 };
 

A message that is not in the archive (sent off-list or not yet received): 20140614010617.902738763-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org

#10 lore raw
Quoted 4 lines · On Fri, 13 Jun 2014 18:06:17 -0700 Stephen Hemminger…

This is second group of patches for cleaning up virtio driver prior to the functionality changes in next phase.

Ping.. no comments

I have a lot of patches stacked and ready or in testing.
And am trying to pace them out a rate that review is actually possible.

A message that is not in the archive (sent off-list or not yet received): 20140617163541.1debf76e-We1ePj4FEcvRI77zikRAJc56i+j3xesD0e7PPNI6Mm0@public.gmane.org

#12 lore raw
Quoted 16 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Stephen Hemminger
Sent: Wednesday, June 18, 2014 12:36 AM
To: Stephen Hemminger
Cc: [email protected]
Subject: Re: [dpdk-dev] [PATCH 0/8] virtio driver phase 2

Quoted 4 lines · On Fri, 13 Jun 2014 18:06:17 -0700 Stephen Hemminger…

This is second group of patches for cleaning up virtio driver prior to the functionality changes in next phase.

Quoted 4 lines

Ping.. no comments

I have a lot of patches stacked and ready or in testing.
And am trying to pace them out a rate that review is actually possible.

Hi Stephen,

I am currently looking at this patch set, we seem to be missing 7/8?

I am getting an error on 8/8:
error: patch failed: lib/librte_pmd_virtio/virtio_pci.h:162
error: lib/librte_pmd_virtio/virtio_pci.h: patch does not apply Patch failed at 0007 virtio: simplify the hardware structure

Thanks,
Alan.

Acked-by
#13 lore raw
In reply to a message not in the archive

Over eager mail filter attacked 7/8.

Quoted 9 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Stephen Hemminger
Sent: Saturday, June 14, 2014 2:06 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH 0/8] virtio driver phase 2

This is second group of patches for cleaning up virtio driver prior to the functionality changes in next phase.

A message that is not in the archive (sent off-list or not yet received): 0E29434AEE0C3A4180987AB476A6F630593A81F5-kPTMFJFq+rF9qrmMLTLiibfspsVTdybXVpNB7YpNyf8@public.gmane.org