Hi Yinan, Still testing, but submitting for early review: https://patches.dpdk.org/patch/81431/
Thanks, On 10/20/20 9:11 AM, Maxime Coquelin wrote: > Hi Yinan, > > On 10/20/20 3:52 AM, Wang, Yinan wrote: >> Hi Adrian/Maxime, >> >> Could you help to take a look at this issue? Most of automated regression >> cases will be blocked due to this issue in 20.11 RC1 test. > > We are working on it. Adrian managed to reproduce it, and fixed one > part of the issue. We''l continue the debugging today. > > BTW, you are mentionning automated regression tests, any chances these > test land in the Intel functionnal CI at some point? I would be great to > catch these issues before the series are merged. > > Thanks! > Maxime > >> Thanks in advance! >> >> BR, >> Yinan >> >>> -----Original Message----- >>> From: Wang, Yinan >>> Sent: 2020?10?16? 13:42 >>> To: Maxime Coquelin <maxime.coque...@redhat.com>; dev@dpdk.org; Xia, >>> Chenbo <chenbo....@intel.com>; Fu, Patrick <patrick...@intel.com>; >>> amore...@redhat.com >>> Subject: RE: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type >>> selection >>> to ethdev >>> >>> Hi Adrian, >>> >>> This patch introduce a regression issue that vhost-user can't launch with >>> server >>> mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559. >>> This issue blocked amount of regression cases, could you help to take a >>> look? >>> Thanks in advance! >>> >>> >>> BR, >>> Yinan >>> >>> >>>> -----Original Message----- >>>> From: dev <dev-boun...@dpdk.org> On Behalf Of Maxime Coquelin >>>> Sent: 2020?9?30? 0:14 >>>> To: dev@dpdk.org; Xia, Chenbo <chenbo....@intel.com>; Fu, Patrick >>>> <patrick...@intel.com>; amore...@redhat.com >>>> Cc: Maxime Coquelin <maxime.coque...@redhat.com> >>>> Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection >>>> to >>>> ethdev >>>> >>>> From: Adrian Moreno <amore...@redhat.com> >>>> >>>> This is a preparation patch with no functional change. >>>> >>>> Use an enum instead of a boolean for the backend type. >>>> Move the detection logic to the ethdev layer (where it is needed for the >>>> first time). >>>> The virtio_user_dev stores the backend type in the virtio_user_dev >>>> struct so the type is only determined once >>>> >>>> Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com> >>>> Signed-off-by: Adrian Moreno <amore...@redhat.com> >>>> --- >>>> .../net/virtio/virtio_user/virtio_user_dev.c | 37 ++++++++----------- >>>> .../net/virtio/virtio_user/virtio_user_dev.h | 11 +++++- >>>> drivers/net/virtio/virtio_user_ethdev.c | 28 +++++++++++++- >>>> 3 files changed, 50 insertions(+), 26 deletions(-) >>>> >>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> index 2a0c861085..b79a9f84aa 100644 >>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev, >>>> return 0; >>>> } >>>> >>>> -int >>>> -is_vhost_user_by_type(const char *path) >>>> -{ >>>> - struct stat sb; >>>> - >>>> - if (stat(path, &sb) == -1) >>>> - return 0; >>>> - >>>> - return S_ISSOCK(sb.st_mode); >>>> -} >>>> - >>>> int >>>> virtio_user_start_device(struct virtio_user_dev *dev) >>>> { >>>> @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev) >>>> rte_mcfg_mem_read_lock(); >>>> pthread_mutex_lock(&dev->mutex); >>>> >>>> - if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0) >>>> + if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER && >>>> + dev->vhostfd < 0) >>>> goto error; >>>> >>>> /* Step 0: tell vhost to create queues */ >>>> @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) >>>> dev->tapfds = NULL; >>>> >>>> if (dev->is_server) { >>>> - if (access(dev->path, F_OK) == 0 && >>>> - !is_vhost_user_by_type(dev->path)) { >>>> - PMD_DRV_LOG(ERR, "Server mode doesn't support >>>> vhost-kernel!"); >>>> + if (dev->backend_type != >>>> VIRTIO_USER_BACKEND_VHOST_USER) { >>>> + PMD_DRV_LOG(ERR, "Server mode only supports >>>> vhost-user!"); >>>> return -1; >>>> } >>>> dev->ops = &virtio_ops_user; >>>> } else { >>>> - if (is_vhost_user_by_type(dev->path)) { >>>> + if (dev->backend_type == >>>> VIRTIO_USER_BACKEND_VHOST_USER) { >>>> dev->ops = &virtio_ops_user; >>>> - } else { >>>> + } else if (dev->backend_type == >>>> + >>>> VIRTIO_USER_BACKEND_VHOST_KERNEL) { >>>> dev->ops = &virtio_ops_kernel; >>>> >>>> dev->vhostfds = malloc(dev->max_queue_pairs * >>>> @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) >>>> int >>>> virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, >>>> int cq, int queue_size, const char *mac, char **ifname, >>>> - int server, int mrg_rxbuf, int in_order, int packed_vq) >>>> + int server, int mrg_rxbuf, int in_order, int packed_vq, >>>> + enum virtio_user_backend_type backend_type) >>>> { >>>> uint64_t protocol_features = 0; >>>> >>>> @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >>>> *path, int queues, >>>> dev->frontend_features = 0; >>>> dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES; >>>> dev->protocol_features = >>>> VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES; >>>> + dev->backend_type = backend_type; >>>> + >>>> parse_mac(dev, mac); >>>> >>>> if (*ifname) { >>>> @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >>>> *path, int queues, >>>> return -1; >>>> } >>>> >>>> - if (!is_vhost_user_by_type(dev->path)) >>>> + if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) >>>> dev->unsupported_features |= >>>> (1ULL << VHOST_USER_F_PROTOCOL_FEATURES); >>>> >>>> @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >>>> *path, int queues, >>>> } >>>> >>>> /* The backend will not report this feature, we add it explicitly */ >>>> - if (is_vhost_user_by_type(dev->path)) >>>> + if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) >>>> dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS); >>>> >>>> /* >>>> @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev >>>> *dev, uint8_t status) >>>> uint64_t arg = status; >>>> >>>> /* Vhost-user only for now */ >>>> - if (!is_vhost_user_by_type(dev->path)) >>>> + if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) >>>> return 0; >>>> >>>> if (!(dev->protocol_features & (1ULL << >>>> VHOST_USER_PROTOCOL_F_STATUS))) >>>> @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev) >>>> int err; >>>> >>>> /* Vhost-user only for now */ >>>> - if (!is_vhost_user_by_type(dev->path)) >>>> + if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) >>>> return 0; >>>> >>>> if (!(dev->protocol_features & (1UL << >>>> VHOST_USER_PROTOCOL_F_STATUS))) >>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h >>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.h >>>> index 9377d5ba66..575bf430c0 100644 >>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h >>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h >>>> @@ -10,6 +10,12 @@ >>>> #include "../virtio_pci.h" >>>> #include "../virtio_ring.h" >>>> >>>> +enum virtio_user_backend_type { >>>> + VIRTIO_USER_BACKEND_UNKNOWN, >>>> + VIRTIO_USER_BACKEND_VHOST_USER, >>>> + VIRTIO_USER_BACKEND_VHOST_KERNEL, >>>> +}; >>>> + >>>> struct virtio_user_queue { >>>> uint16_t used_idx; >>>> bool avail_wrap_counter; >>>> @@ -17,6 +23,7 @@ struct virtio_user_queue { >>>> }; >>>> >>>> struct virtio_user_dev { >>>> + enum virtio_user_backend_type backend_type; >>>> /* for vhost_user backend */ >>>> int vhostfd; >>>> int listenfd; /* listening fd */ >>>> @@ -60,13 +67,13 @@ struct virtio_user_dev { >>>> bool started; >>>> }; >>>> >>>> -int is_vhost_user_by_type(const char *path); >>>> int virtio_user_start_device(struct virtio_user_dev *dev); >>>> int virtio_user_stop_device(struct virtio_user_dev *dev); >>>> int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int >>>> queues, >>>> int cq, int queue_size, const char *mac, char **ifname, >>>> int server, int mrg_rxbuf, int in_order, >>>> - int packed_vq); >>>> + int packed_vq, >>>> + enum virtio_user_backend_type backend_type); >>>> void virtio_user_dev_uninit(struct virtio_user_dev *dev); >>>> void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t >>>> queue_idx); >>>> void virtio_user_handle_cq_packed(struct virtio_user_dev *dev, >>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c >>>> b/drivers/net/virtio/virtio_user_ethdev.c >>>> index 60d17af888..38b49bad5f 100644 >>>> --- a/drivers/net/virtio/virtio_user_ethdev.c >>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c >>>> @@ -6,6 +6,7 @@ >>>> #include <sys/types.h> >>>> #include <unistd.h> >>>> #include <fcntl.h> >>>> +#include <sys/stat.h> >>>> #include <sys/socket.h> >>>> >>>> #include <rte_malloc.h> >>>> @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused, >>>> return -errno; >>>> } >>>> >>>> +static enum virtio_user_backend_type >>>> +virtio_user_backend_type(const char *path) >>>> +{ >>>> + struct stat sb; >>>> + >>>> + if (stat(path, &sb) == -1) >>>> + return VIRTIO_USER_BACKEND_UNKNOWN; >>>> + >>>> + return S_ISSOCK(sb.st_mode) ? >>>> + VIRTIO_USER_BACKEND_VHOST_USER : >>>> + VIRTIO_USER_BACKEND_VHOST_KERNEL; >>>> +} >>>> + >>>> static struct rte_eth_dev * >>>> virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev) >>>> { >>>> @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev) >>>> struct rte_kvargs *kvlist = NULL; >>>> struct rte_eth_dev *eth_dev; >>>> struct virtio_hw *hw; >>>> + enum virtio_user_backend_type backend_type = >>>> VIRTIO_USER_BACKEND_UNKNOWN; >>>> uint64_t queues = VIRTIO_USER_DEF_Q_NUM; >>>> uint64_t cq = VIRTIO_USER_DEF_CQ_EN; >>>> uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ; >>>> @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev) >>>> goto end; >>>> } >>>> >>>> + backend_type = virtio_user_backend_type(path); >>>> + if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) { >>>> + PMD_INIT_LOG(ERR, >>>> + "unable to determine backend type for path %s", >>>> + path); >>>> + goto end; >>>> + } >>>> + >>>> + >>>> if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1) >>>> { >>>> - if (is_vhost_user_by_type(path)) { >>>> + if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) { >>>> PMD_INIT_LOG(ERR, >>>> "arg %s applies only to vhost-kernel backend", >>>> VIRTIO_USER_ARG_INTERFACE_NAME); >>>> @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev) >>>> hw = eth_dev->data->dev_private; >>>> if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq, >>>> queue_size, mac_addr, &ifname, server_mode, >>>> - mrg_rxbuf, in_order, packed_vq) < 0) { >>>> + mrg_rxbuf, in_order, packed_vq, backend_type) < 0) { >>>> PMD_INIT_LOG(ERR, "virtio_user_dev_init fails"); >>>> virtio_user_eth_dev_free(eth_dev); >>>> goto end; >>>> -- >>>> 2.26.2 >> > -- Adrián Moreno
