DPDK patches and discussions
 help / color / mirror / Atom feed
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
To: "Tan, Jianfeng" <jianfeng.tan@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"david.marchand@6wind.com" <david.marchand@6wind.com>
Subject: Re: [dpdk-dev] [PATCH 3/5] net/virtio-user: support to report net status
Date: Wed, 29 Mar 2017 16:00:28 +0800	[thread overview]
Message-ID: <20170329080028.GJ18844@yliu-dev.sh.intel.com> (raw)
In-Reply-To: <006baeef-4167-edcf-407a-e149e01e8d65@intel.com>

On Wed, Mar 29, 2017 at 03:48:04PM +0800, Tan, Jianfeng wrote:
> 
> 
> On 3/29/2017 3:14 PM, Yuanhan Liu wrote:
> >On Wed, Mar 29, 2017 at 03:07:28PM +0800, Tan, Jianfeng wrote:
> >>
> >>On 3/29/2017 2:33 PM, Yuanhan Liu wrote:
> >>>On Mon, Mar 27, 2017 at 07:46:32AM +0000, Tan, Jianfeng wrote:
> >>>>>>diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>>index 9777d6b..cc6f557 100644
> >>>>>>--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>>+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> >>>>>>@@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev
> >>>>>*dev, uint8_t portid)
> >>>>>>  	features &= ~(1ull << VIRTIO_NET_F_MAC);
> >>>>>>  	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to
> >>>>>know */
> >>>>>>  	features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
> >>>>>>+	features &= ~(1ull << VIRTIO_NET_F_STATUS);
> >>>>>>  	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES,
> >>>>>&features);
> >>>>>>  	if (ret < 0)
> >>>>>>  		goto error;
> >>>>>>diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>b/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>>index fa79419..fbdd0a8 100644
> >>>>>>--- a/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>>+++ b/drivers/net/virtio/virtio_user_ethdev.c
> >>>>>>@@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw)
> >>>>>>  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> >>>>>>
> >>>>>>  	/* unmask feature bits defined in vhost user protocol */
> >>>>>>-	return dev->device_features &
> >>>>>VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> >>>>>>+	return (dev->device_features | (1 << VIRTIO_NET_F_STATUS))
> >>>>>>+		& VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
> >>>>>Why not handle the features at virtio_user_dev_init()?
> >>>>You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device.
> >>>>
> >>>Aren't you adding the F_STATUS features to this device?
> >>>
> >>For virtio driver, yes, we are adding F_STATUS feature so the it sees a
> >>device supporting LSC.
> >That means you are adding a device feature (F_STATUS) and reporting it to
> >the driver that this feature is always on, no matter whether the device
> >actually supports it or not? This looks wrong to me.
> 
> Why?

Because you were doing hack to make virtio-user work, while there is no
such hack in the QEMU virtio-pci implementation.

> IMO, device is not necessary to know this feature.

F_STATUS is a device feature. If such feature is not claimed to be
supported by the device, the driver should ignore it. But you are
unconditionally letting the driver handle it, even though  the
virito-user device does not claim to support it.

Note that F_STATUS is set in the QEMU virtio-net PCI device.

	--yliu

> For vhost-user,
> except explicitly cutting down the unix connection, I don't see a message of
> vhost-user protocol to report that.
> 
> >
> >>But for backend driver, we need hide this feature, it
> >>happens when we set_features to the backend driver.
> >The feature negotion of virtio-user seems broken to me.
> >
> >
> Why?
> vhost does not claim to support this feature, VHOST_SUPPORTED_FEATURES.
> 
> Thanks,
> Jianfeng

  reply	other threads:[~2017-03-29  8:02 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-03 17:56 [dpdk-dev] [PATCH 0/5] add LSC and Rxq interrupt for virtio-user Jianfeng Tan
2017-03-03 17:56 ` [dpdk-dev] [PATCH 1/5] eal/linux: add interrupt type for vdev Jianfeng Tan
2017-03-03 17:56 ` [dpdk-dev] [PATCH 2/5] net/virtio-user: add rxq interrupt mode support Jianfeng Tan
2017-03-17  6:47   ` Yuanhan Liu
2017-03-28  1:33     ` Tan, Jianfeng
2017-03-03 17:56 ` [dpdk-dev] [PATCH 3/5] net/virtio-user: support to report net status Jianfeng Tan
2017-03-17  6:54   ` Yuanhan Liu
2017-03-27  7:46     ` Tan, Jianfeng
2017-03-29  6:33       ` Yuanhan Liu
2017-03-29  7:07         ` Tan, Jianfeng
2017-03-29  7:14           ` Yuanhan Liu
2017-03-29  7:48             ` Tan, Jianfeng
2017-03-29  8:00               ` Yuanhan Liu [this message]
2017-03-29  8:33                 ` Tan, Jianfeng
2017-03-29  8:36                   ` Yuanhan Liu
2017-03-30  3:14                     ` Tan, Jianfeng
2017-03-03 17:56 ` [dpdk-dev] [PATCH 4/5] net/virtio-user: add lsc support with vhost-user adapter Jianfeng Tan
2017-03-17  8:29   ` Yuanhan Liu
2017-03-27  1:51     ` Tan, Jianfeng
2017-03-03 17:56 ` [dpdk-dev] [PATCH 5/5] net/virtio-user: add lsc support with vhost-kernel adapter Jianfeng Tan
2017-03-28  8:21 ` [dpdk-dev] [PATCH v2 0/5] add LSC and Rxq interrupt for virtio-user Jianfeng Tan
2017-03-28  8:21   ` [dpdk-dev] [PATCH v2 1/5] eal/linux: add interrupt type for vdev Jianfeng Tan
2017-03-28  8:21   ` [dpdk-dev] [PATCH v2 2/5] net/virtio: add interrupt configure " Jianfeng Tan
2017-03-29  6:27     ` Yuanhan Liu
2017-03-29  7:03       ` Tan, Jianfeng
2017-03-29  7:09         ` Yuanhan Liu
2017-03-29  7:27           ` Tan, Jianfeng
2017-03-29  7:30             ` Yuanhan Liu
2017-03-28  8:21   ` [dpdk-dev] [PATCH v2 3/5] net/virtio-user: add rxq interrupt mode support Jianfeng Tan
2017-03-28  8:21   ` [dpdk-dev] [PATCH v2 4/5] net/virtio-user: support to report net status Jianfeng Tan
2017-03-28  8:21   ` [dpdk-dev] [PATCH v2 5/5] net/virtio-user: add lsc support Jianfeng Tan
2017-03-31 19:44 ` [dpdk-dev] [PATCH v3 0/5] add LSC and Rxq interrupt for virtio-user Jianfeng Tan
2017-03-31 19:44   ` [dpdk-dev] [PATCH v3 1/5] eal/linux: add interrupt type for vdev Jianfeng Tan
2017-03-31 19:44   ` [dpdk-dev] [PATCH v3 2/5] net/virtio-user: move eventfd open/close into init/uninit Jianfeng Tan
2017-03-31 19:44   ` [dpdk-dev] [PATCH v3 3/5] net/virtio-user: add rxq interrupt mode support Jianfeng Tan
2017-03-31 19:44   ` [dpdk-dev] [PATCH v3 4/5] net/virtio-user: support to report net status Jianfeng Tan
2017-03-31 19:44   ` [dpdk-dev] [PATCH v3 5/5] net/virtio-user: add lsc support Jianfeng Tan
2017-04-01  5:13     ` Yuanhan Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170329080028.GJ18844@yliu-dev.sh.intel.com \
    --to=yuanhan.liu@linux.intel.com \
    --cc=david.marchand@6wind.com \
    --cc=dev@dpdk.org \
    --cc=jianfeng.tan@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).