Re: [RFC PATCH] vhost: Add VHOST PMD

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 28 Aug 2015 12:21:41 +0900
Tetsuya Mukawa <mukawa@igel.co.jp> wrote:

> The patch introduces a new PMD. This PMD is implemented as thin wrapper
> of librte_vhost. It means librte_vhost is also needed to compile the PMD.
> The PMD can have 'iface' parameter like below to specify a path to connect
> to a virtio-net device.
> 
> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i
> 
> To connect above testpmd, here is qemu command example.
> 
> $ qemu-system-x86_64 \
>         <snip>
>         -chardev socket,id=chr0,path=/tmp/sock0 \
>         -netdev vhost-user,id=net0,chardev=chr0,vhostforce \
>         -device virtio-net-pci,netdev=net0
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

Lots of minor nits from checkpatch...


ERROR: spaces required around that '=' (ctx:WxV)
#248: FILE: drivers/net/vhost/rte_eth_vhost.c:54:
+	.addr_bytes ={
 

CHECK: Unnecessary parentheses around r->rx_pkts
#313: FILE: drivers/net/vhost/rte_eth_vhost.c:119:
+	rte_atomic64_add(&(r->rx_pkts), nb_rx);


CHECK: Unnecessary parentheses around r->tx_pkts
#341: FILE: drivers/net/vhost/rte_eth_vhost.c:147:
+	rte_atomic64_add(&(r->tx_pkts), nb_tx);

CHECK: Unnecessary parentheses around r->err_pkts
#342: FILE: drivers/net/vhost/rte_eth_vhost.c:148:
+	rte_atomic64_add(&(r->err_pkts), nb_bufs - nb_tx);

WARNING: void function return statements are not generally useful
#371: FILE: drivers/net/vhost/rte_eth_vhost.c:177:
+	return;
+}

ERROR: space required after that ',' (ctx:VxV)
#374: FILE: drivers/net/vhost/rte_eth_vhost.c:180:
+eth_rx_queue_setup(struct rte_eth_dev *dev,uint16_t rx_queue_id,
                                           ^

CHECK: Alignment should match open parenthesis
#375: FILE: drivers/net/vhost/rte_eth_vhost.c:181:
+eth_rx_queue_setup(struct rte_eth_dev *dev,uint16_t rx_queue_id,
+				    uint16_t nb_rx_desc __rte_unused,

WARNING: line over 80 characters
#377: FILE: drivers/net/vhost/rte_eth_vhost.c:183:
+				    const struct rte_eth_rxconf *rx_conf __rte_unused,

WARNING: Missing a blank line after declarations
#381: FILE: drivers/net/vhost/rte_eth_vhost.c:187:
+	struct pmd_internal *internal = dev->data->dev_private;
+	internal->rx_vhost_queues[rx_queue_id].mb_pool = mb_pool;

WARNING: line over 80 characters
#382: FILE: drivers/net/vhost/rte_eth_vhost.c:188:
+	dev->data->rx_queues[rx_queue_id] = &internal->rx_vhost_queues[rx_queue_id];

CHECK: Alignment should match open parenthesis
#388: FILE: drivers/net/vhost/rte_eth_vhost.c:194:
+eth_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
+				    uint16_t nb_tx_desc __rte_unused,

WARNING: line over 80 characters
#390: FILE: drivers/net/vhost/rte_eth_vhost.c:196:
+				    const struct rte_eth_txconf *tx_conf __rte_unused)

WARNING: line over 80 characters
#393: FILE: drivers/net/vhost/rte_eth_vhost.c:199:
+	dev->data->tx_queues[tx_queue_id] = &internal->tx_vhost_queues[tx_queue_id];

WARNING: Missing a blank line after declarations
#393: FILE: drivers/net/vhost/rte_eth_vhost.c:199:
+	struct pmd_internal *internal = dev->data->dev_private;
+	dev->data->tx_queues[tx_queue_id] = &internal->tx_vhost_queues[tx_queue_id];

CHECK: Please don't use multiple blank lines
#397: FILE: drivers/net/vhost/rte_eth_vhost.c:203:
+
+

CHECK: Alignment should match open parenthesis
#400: FILE: drivers/net/vhost/rte_eth_vhost.c:206:
+eth_dev_info(struct rte_eth_dev *dev,
+		struct rte_eth_dev_info *dev_info)

WARNING: Missing a blank line after declarations
#403: FILE: drivers/net/vhost/rte_eth_vhost.c:209:
+	struct pmd_internal *internal = dev->data->dev_private;
+	dev_info->driver_name = drivername;

CHECK: Alignment should match open parenthesis
#420: FILE: drivers/net/vhost/rte_eth_vhost.c:226:
+	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
+			i < internal->nb_rx_queues; i++) {

WARNING: line over 80 characters
#421: FILE: drivers/net/vhost/rte_eth_vhost.c:227:
+		igb_stats->q_ipackets[i] = internal->rx_vhost_queues[i].rx_pkts.cnt;

CHECK: Alignment should match open parenthesis
#426: FILE: drivers/net/vhost/rte_eth_vhost.c:232:
+	for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS &&
+			i < internal->nb_tx_queues; i++) {

WARNING: line over 80 characters
#427: FILE: drivers/net/vhost/rte_eth_vhost.c:233:
+		igb_stats->q_opackets[i] = internal->tx_vhost_queues[i].tx_pkts.cnt;

WARNING: line over 80 characters
#428: FILE: drivers/net/vhost/rte_eth_vhost.c:234:
+		igb_stats->q_errors[i] = internal->tx_vhost_queues[i].err_pkts.cnt;

WARNING: Missing a blank line after declarations
#443: FILE: drivers/net/vhost/rte_eth_vhost.c:249:
+	struct pmd_internal *internal = dev->data->dev_private;
+	for (i = 0; i < internal->nb_rx_queues; i++)


WARNING: 'failuer' may be misspelled - perhaps 'failure'?
#527: FILE: drivers/net/vhost/rte_eth_vhost.c:333:
+		RTE_LOG(INFO, PMD, "failuer to find ethdev\n");

WARNING: 'failuer' may be misspelled - perhaps 'failure'?
#570: FILE: drivers/net/vhost/rte_eth_vhost.c:376:
+		RTE_LOG(INFO, PMD, "failuer to find a ethdev\n");

WARNING: 'accesing' may be misspelled - perhaps 'accessing'?
#576: FILE: drivers/net/vhost/rte_eth_vhost.c:382:
+	/* Wait until rx/tx_pkt_burst stops accesing vhost device */

WARNING: void function return statements are not generally useful
#606: FILE: drivers/net/vhost/rte_eth_vhost.c:412:
+	return;
+}

ERROR: do not initialise statics to 0 or NULL
#610: FILE: drivers/net/vhost/rte_eth_vhost.c:416:
+	static struct virtio_net_device_ops *vhost_ops = NULL;

WARNING: Missing a blank line after declarations
#611: FILE: drivers/net/vhost/rte_eth_vhost.c:417:
+	static struct virtio_net_device_ops *vhost_ops = NULL;
+	vhost_ops = rte_zmalloc(NULL, sizeof(*vhost_ops), 0);

CHECK: Alignment should match open parenthesis
#642: FILE: drivers/net/vhost/rte_eth_vhost.c:448:
+eth_dev_vhost_create(const char *name, int index,
+		char *iface_name,

CHECK: Alignment should match open parenthesis
#654: FILE: drivers/net/vhost/rte_eth_vhost.c:460:
+	RTE_LOG(INFO, PMD, "Creating VHOST-USER backend on numa socket %u\n",
+			numa_node);

CHECK: Comparison to NULL could be written "!data"
#660: FILE: drivers/net/vhost/rte_eth_vhost.c:466:
+	if (data == NULL)

WARNING: networking block comments don't use an empty /* line, use /* Comment...
#716: FILE: drivers/net/vhost/rte_eth_vhost.c:522:
+	/*
+	 * We'll replace the 'data' originally allocated by eth_dev. So the

CHECK: Comparison to NULL could be written "!value"
#747: FILE: drivers/net/vhost/rte_eth_vhost.c:553:
+	if (value == NULL)

CHECK: Comparison to NULL could be written "!kvlist"
#766: FILE: drivers/net/vhost/rte_eth_vhost.c:572:
+	if (kvlist == NULL)

CHECK: Alignment should match open parenthesis
#778: FILE: drivers/net/vhost/rte_eth_vhost.c:584:
+		ret = rte_kvargs_process(kvlist, ETH_VHOST_IFACE_ARG,
+				&open_iface, &iface_name);