On Fri, Aug 25, 2017 at 09:40:51AM +0000, Jianfeng Tan wrote:
> static int
> +share_device(int vid)
> +{
> + uint32_t i, vring_num;
> + int len;
> + int fds[8];
> + struct rte_vhost_memory *mem;
> + struct vhost_params *params;
> + struct rte_vhost_vring vring;
> +
> + /* share mem table */
> + if (rte_vhost_get_mem_table(vid, &mem) < 0) {
> + printf("Failed to get mem table\n");
No printf in DPDK lib and pmd, use RTE_LOG instead.
[...]
> +static int
> +eth_dev_vhost_attach(struct rte_vdev_device *dev)
> +{
> + struct rte_eth_dev *eth_dev = NULL;
> + struct rte_eth_dev_data *data = NULL;
> +
> + RTE_LOG(INFO, PMD, "Attach vhost user port\n");
> +
> + /* reserve an ethdev entry */
> + eth_dev = rte_eth_vdev_allocate(dev, sizeof(struct pmd_internal));
> + if (eth_dev == NULL)
> + goto error;
I'd suggest to "return -1" directly here, without introducing goto.
> +
> + eth_dev->dev_ops = &ops;
> +
> + /* finally assign rx and tx ops */
> + eth_dev->rx_pkt_burst = eth_vhost_rx;
> + eth_dev->tx_pkt_burst = eth_vhost_tx;
> +
> + data = eth_dev->data;
> +
> + return data->port_id;
> +
> +error:
> + return -1;
> +}
> +
> static inline int
> open_iface(const char *key __rte_unused, const char *value, void *extra_args)
> {
> @@ -1154,6 +1244,39 @@ open_int(const char *key __rte_unused, const char
> *value, void *extra_args)
> }
>
> static int
> +vhost_pmd_action(const char *params, int len __rte_unused,
> + int fds[], int fds_num)
> +{
> + int i;
> + void *base_addr;
> + const struct vhost_params *p = (const struct vhost_params *)params;
The cast could be avoided if you define the action prototype with
"const void *params".
> + const struct rte_vhost_mem_region *regions;
> +
> + switch (p->type) {
> + case VHOST_MSG_TYPE_REGIONS:
> + regions = (const void *)p->regions;
Unnecessary cast.
> + for (i = 0; i < fds_num; ++i) {
> + base_addr = mmap(regions[i].mmap_addr,
> + regions[i].mmap_size,
> + PROT_READ | PROT_WRITE,
> + MAP_FIXED | MAP_SHARED, fds[i], 0);
> + if (base_addr != regions[i].mmap_addr) {
> + RTE_LOG(INFO, PMD, "mmap error");
The log level should be error, nothing will work if it happens. Also, the
log message should be more informative.
--yliu
