Hi Maxime, I think you are right :-)
Regards, Fan > -----Original Message----- > From: Maxime Coquelin <[email protected]> > Sent: Tuesday, October 6, 2020 8:54 AM > To: Zhang, Roy Fan <[email protected]>; [email protected] > Cc: Xia, Chenbo <[email protected]>; Liu, Changpeng > <[email protected]>; Yigit, Ferruh <[email protected]>; > [email protected] > Subject: Re: [dpdk-dev v2 1/2] vhost: add backend type in driver start > > Hi Fan, > > On 10/2/20 5:36 PM, Fan Zhang wrote: > > This patch adds an internal driver start function with a newly > > added backend type identifier as parameter. With this way > > different built-in driver types (net, crypto) can be identified. > > > > Signed-off-by: Fan Zhang <[email protected]> > > --- > > lib/librte_vhost/socket.c | 39 ++++++++++++++++++++++++++++------- > > lib/librte_vhost/vhost_user.h | 8 +++++++ > > 2 files changed, 39 insertions(+), 8 deletions(-) > > > > diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c > > index 0169d3648..a53e536ac 100644 > > --- a/lib/librte_vhost/socket.c > > +++ b/lib/librte_vhost/socket.c > > @@ -38,7 +38,7 @@ struct vhost_user_socket { > > bool is_server; > > bool reconnect; > > bool iommu_support; > > - bool use_builtin_virtio_net; > > + enum virtio_backend_type backend_type; > > bool extbuf; > > bool linearbuf; > > bool async_copy; > > @@ -224,7 +224,9 @@ vhost_user_add_connection(int fd, struct > vhost_user_socket *vsocket) > > size = strnlen(vsocket->path, PATH_MAX); > > vhost_set_ifname(vid, vsocket->path, size); > > > > - vhost_set_builtin_virtio_net(vid, vsocket->use_builtin_virtio_net); > > + vhost_set_builtin_virtio_net(vid, > > + vsocket->backend_type == > VIRTIO_DEV_BUILTIN_NET ? > > + true : false); > > > > vhost_attach_vdpa_device(vid, vsocket->vdpa_dev); > > > > @@ -632,7 +634,7 @@ rte_vhost_driver_disable_features(const char > *path, uint64_t features) > > pthread_mutex_lock(&vhost_user.mutex); > > vsocket = find_vhost_user_socket(path); > > > > - /* Note that use_builtin_virtio_net is not affected by this function > > + /* Note that backend type is not affected by this function > > * since callers may want to selectively disable features of the > > * built-in vhost net device backend. > > */ > > @@ -681,7 +683,8 @@ rte_vhost_driver_set_features(const char *path, > uint64_t features) > > /* Anyone setting feature bits is implementing their own > vhost > > * device backend. > > */ > > - vsocket->use_builtin_virtio_net = false; > > + if (vsocket->backend_type == VIRTIO_DEV_BUILTIN_NET) > > + vsocket->backend_type = VIRTIO_DEV_UNKNOWN; > > } > > pthread_mutex_unlock(&vhost_user.mutex); > > > > @@ -899,7 +902,7 @@ rte_vhost_driver_register(const char *path, > uint64_t flags) > > * rte_vhost_driver_set_features(), which will overwrite following > > * two values. > > */ > > - vsocket->use_builtin_virtio_net = true; > > + vsocket->backend_type = VIRTIO_DEV_BUILTIN_NET; > > vsocket->supported_features = > VIRTIO_NET_SUPPORTED_FEATURES; > > vsocket->features = VIRTIO_NET_SUPPORTED_FEATURES; > > vsocket->protocol_features = VHOST_USER_PROTOCOL_FEATURES; > > @@ -1117,10 +1120,17 @@ vhost_driver_callback_get(const char *path) > > } > > > > int > > -rte_vhost_driver_start(const char *path) > > +vhost_driver_start(const char *path, enum virtio_backend_type > backend_type) > > { > > struct vhost_user_socket *vsocket; > > static pthread_t fdset_tid; > > + int ret; > > + > > + if (backend_type <= VIRTIO_DEV_UNKNOWN || > > + backend_type > VIRTIO_DEV_BUILTIN_CRYPTO) { > > + VHOST_LOG_CONFIG(ERR, "Wrong backend type\n"); > > + return -1; > > + } > > > > pthread_mutex_lock(&vhost_user.mutex); > > vsocket = find_vhost_user_socket(path); > > @@ -1153,7 +1163,20 @@ rte_vhost_driver_start(const char *path) > > } > > > > if (vsocket->is_server) > > - return vhost_user_start_server(vsocket); > > + ret = vhost_user_start_server(vsocket); > > else > > - return vhost_user_start_client(vsocket); > > + ret = vhost_user_start_client(vsocket); > > + > > + if (ret < 0) > > + return ret; > > + > > + vsocket->backend_type = backend_type; > > + > > + return 0; > > Looking at it again, I think patch 1 is not necessary. > Patch 2 should be enough and would be easier to backport. > > What do you think? > > Regards, > Maxime > > > +} > > + > > +int > > +rte_vhost_driver_start(const char *path) > > +{ > > + return vhost_driver_start(path, VIRTIO_DEV_BUILTIN_NET); > > } > > diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h > > index 16fe03f88..9f526e676 100644 > > --- a/lib/librte_vhost/vhost_user.h > > +++ b/lib/librte_vhost/vhost_user.h > > @@ -158,6 +158,12 @@ typedef struct VhostUserMsg { > > /* The version of the protocol we support */ > > #define VHOST_USER_VERSION 0x1 > > > > +/* virtio backend types */ > > +enum virtio_backend_type { > > + VIRTIO_DEV_UNKNOWN = 0, /* Likely external */ > > + VIRTIO_DEV_BUILTIN_NET, /* Virtio-net device */ > > + VIRTIO_DEV_BUILTIN_CRYPTO, /* Virtio-crypto device */ > > +}; > > > > /* vhost_user.c */ > > int vhost_user_msg_handler(int vid, int fd); > > @@ -167,5 +173,7 @@ int vhost_user_iotlb_miss(struct virtio_net *dev, > uint64_t iova, uint8_t perm); > > int read_fd_message(int sockfd, char *buf, int buflen, int *fds, int > max_fds, > > int *fd_num); > > int send_fd_message(int sockfd, char *buf, int buflen, int *fds, int > fd_num); > > +int vhost_driver_start(const char *path, > > + enum virtio_backend_type backend_type); > > > > #endif > >
