[dpdk-dev] [PATCH 1/4] vhost: move fdset functions from fd_man.c to fd_man.h

[Yang, Zhiyong]

Hi Thomas, Maxime, Jianfeng,

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Yang, Zhiyong
> Sent: Friday, March 16, 2018 4:44 PM
> To: Thomas Monjalon <thomas at monjalon.net>
> Cc: Tan, Jianfeng <jianfeng.tan at intel.com>; Maxime Coquelin
> <maxime.coquelin at redhat.com>; dev at dpdk.org; yliu at fridaylinux.org; Bie,
> Tiwei <tiwei.bie at intel.com>; Wang, Zhihong <zhihong.wang at intel.com>;
> Wang, Dong1 <dong1.wang at intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/4] vhost: move fdset functions from
> fd_man.c to fd_man.h
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas at monjalon.net]
> > Sent: Thursday, March 15, 2018 5:33 PM
> > To: Yang, Zhiyong <zhiyong.yang at intel.com>
> > Cc: Tan, Jianfeng <jianfeng.tan at intel.com>; Maxime Coquelin
> > <maxime.coquelin at redhat.com>; dev at dpdk.org; yliu at fridaylinux.org; Bie,
> > Tiwei <tiwei.bie at intel.com>; Wang, Zhihong <zhihong.wang at intel.com>;
> > Wang, Dong1 <dong1.wang at intel.com>
> > Subject: Re: [PATCH 1/4] vhost: move fdset functions from fd_man.c to
> > fd_man.h
> >
> > 13/03/2018 10:50, Yang, Zhiyong:
> > > From: Thomas Monjalon [mailto:thomas at monjalon.net]
> > > > 13/03/2018 09:46, Yang, Zhiyong:
> > > > > From: Thomas Monjalon [mailto:thomas at monjalon.net]
> > > > > > 05/03/2018 08:43, Yang, Zhiyong:
> > > > > > > From: Thomas Monjalon [mailto:thomas at monjalon.net]
> > > > > > > > I don't see why it should be exported outside of DPDK,
> > > > > > > > except for
> > > > PMDs.
> > > > > > > > I would tend to keep it internal but I understand that it
> > > > > > > > would mean duplicating some code, which is not ideal.
> > > > > > > > Please could you show what would be the content of the .h in
> EAL?
> > > > > > > >
> > > > > > >
> > > > > > > If needed to expose them in eal.h, I think that they should
> > > > > > > be the whole fdset mechanism as followings.
> > > > > > >
> > > > > > > typedef void (*fd_cb)(int fd, void *dat, int *remove);
> > > > > > >
> > > > > > > struct fdentry {
> > > > > > > 	int fd;		/* -1 indicates this entry is empty */
> > > > > > > 	fd_cb rcb;	/* callback when this fd is readable. */
> > > > > > > 	fd_cb wcb;	/* callback when this fd is writeable.*/
> > > > > > > 	void *dat;	/* fd context */
> > > > > > > 	int busy;	/* whether this entry is being used in cb. */
> > > > > > > };
> > > > > > >
> > > > > > > struct fdset {
> > > > > > > 	struct pollfd rwfds[MAX_FDS];
> > > > > > > 	struct fdentry fd[MAX_FDS];
> > > > > > > 	pthread_mutex_t fd_mutex;
> > > > > > > 	int num;	/* current fd number of this fdset */
> > > > > > > };
> > > > > > >
> > > > > > > void fdset_init(struct fdset *pfdset);    (not used in the patchset)
> > > > > > >
> > > > > > > int fdset_add(struct fdset *pfdset, int fd,
> > > > > > > 	fd_cb rcb, fd_cb wcb, void *dat);     (used in this patchset)
> > > > > > >
> > > > > > > void *fdset_del(struct fdset *pfdset, int fd); (not used in
> > > > > > > the
> > > > > > > patchset)
> > > > > > >
> > > > > > > void *fdset_event_dispatch(void *arg);   (used in this patchset)
> > > > > > >
> > > > > > > seems that we have 4 options.
> > > > > > > 1) expose them in librte_vhost
> > > > > > > 2) expose them in other existing or new libs. for example,  eal.
> > > > > > > 3) duplicate the code lines at PMD layer.
> > > > > > > 4) do it as the patch does that.
> > > > > >
> > > > > > It looks to be very close of the interrupt thread.
> > > > > > Can we have all merged in an unique event dispatcher thread?
> > > > >
> > > > > If I understand right, do you mean that we can merge them in lib eal ?
> > right?
> > > >
> > > > Yes merge with interrupt thread in EAL.
> > > > I didn't look at the details, but it seems the right place for such thing.
> > > >
> > > Ok,  we have to expose them as new APIs.  Expect that somebody as
> > > DPDK users can use and like them as well. :)
> >
> > I think you missed my initial question:
> > Is it possible to merge the vhost events needs in the EAL interrupt thread?
> >
> 
> Sorry to miss this question.
> Compared to vhost event mechanism(poll), Eal interrupt uses epoll, From my
> basic understanding,  linux and freeBSD both support poll, Epoll is supported
> by Linux only.
> 
> Hi Maxime,
> I want to know your opinion about Thomas's question.
> 
> This patchset aim to support for virtio-user server mode and just need to call
> event functions, So, Let me try librte_eal epoll mechanism if support for
> server mode.
> 

I have implemented the same functionality calling librte_eal  epoll mechanism instead of
Vhost event and V3 has been sent out.  Please help review and welcome any comments.

Thanks
Zhiyong