Re: [dpdk-dev] [PATCH v2 12/12] net/vhost: support to run in the secondary process

[Yuanhan Liu <yliu@fridaylinux.org>]

On Sat, Sep 30, 2017 at 12:06:44PM 0000, Jianfeng Tan wrote:
> >>>> +	/* share callfd and kickfd */
> >>>> +	params->type = VHOST_MSG_TYPE_SET_FDS;
> >>>> +	vring_num = rte_vhost_get_vring_num(vid);
> >>>> +	for (i = 0; i < vring_num; i++) {
> >>>> +		if (rte_vhost_get_vhost_vring(vid, i, &vring) < 0) {
> >>> If you save the fds here, you don't have to get it every time when there
> >>> is a new secondary process attached. Then as I have suggested firstly,
> >>> you don't have to introduce callfd_pri in the vhost lib.
> >> If we don't introduce callfd_pri, when we do virtqueue cleanup (or similar
> >> operations) in vhost lib, it will wrongly close fds belonging to secondary
> >> process.
> >>
> >> You remind me that, instead of introduce callfd_pri, we can introduce
> >> callfd_effective, to reduce the modification lines.
> > It's not about how many lines are modified. You were adding "effective"
> > fds, which is a semantic change. It makes the logic a bit more complex.
> > What's worse, it even doesn't resolve the issue completely.
> 
> Yes, it still has limitation. Just wonder if possible to move event fd 
> write out of vhost lib.

It could be another solution: we have the valid fd and vring_avail
outside the vhost lib. It's just that the callfd write is inside
the vhost lib. If you just take it out (remove it), it breaks some
users who rely on it: it's an API behaviour change.

	--yliu