Re: [dpdk-dev] [PATCH 4/5] virtio: New API to enable/disable multicast and promisc mode

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 25 Aug 2014 10:09:31 +0800
Ouyang Changchun <changchun.ouyang@intel.com> wrote:

> This patch adds new API in virtio for supporting promiscuous and allmulticast enabling and disabling.
> 
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> Acked-by: Huawei Xie <huawei.xie@intel.com>
> Acked-by: Cunming Liang <cunming.liang@intel.com>
> 
> ---
>  lib/librte_pmd_virtio/virtio_ethdev.c | 98 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 97 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
> index 6293ac6..c7f874a 100644
> --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> @@ -66,6 +66,10 @@ static int eth_virtio_dev_init(struct eth_driver *eth_drv,
>  static int  virtio_dev_configure(struct rte_eth_dev *dev);
>  static int  virtio_dev_start(struct rte_eth_dev *dev);
>  static void virtio_dev_stop(struct rte_eth_dev *dev);
> +static void virtio_dev_promiscuous_enable(struct rte_eth_dev *dev);
> +static void virtio_dev_promiscuous_disable(struct rte_eth_dev *dev);
> +static void virtio_dev_allmulticast_enable(struct rte_eth_dev *dev);
> +static void virtio_dev_allmulticast_disable(struct rte_eth_dev *dev);
>  static void virtio_dev_info_get(struct rte_eth_dev *dev,
>  				struct rte_eth_dev_info *dev_info);
>  static int virtio_dev_link_update(struct rte_eth_dev *dev,
> @@ -403,6 +407,94 @@ virtio_dev_close(struct rte_eth_dev *dev)
>  	virtio_dev_stop(dev);
>  }
>  
> +static void
> +virtio_dev_promiscuous_enable(struct rte_eth_dev *dev)
> +{
> +	struct virtio_hw *hw
> +		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct virtio_pmd_ctrl ctrl;
> +	int dlen[1];
> +	int ret;
> +
> +	ctrl.hdr.class = VIRTIO_NET_CTRL_RX;
> +	ctrl.hdr.cmd = VIRTIO_NET_CTRL_RX_PROMISC;
> +	ctrl.data[0] = 1;
> +	dlen[0] = 1;
> +
> +	ret = virtio_send_command(hw->cvq, &ctrl, dlen, 1);
> +
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Promisc enabling but send command "
> +			  "failed, this is too late now...\n");
> +	}
> +}
> +
> +static void
> +virtio_dev_promiscuous_disable(struct rte_eth_dev *dev)
> +{
> +	struct virtio_hw *hw
> +		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct virtio_pmd_ctrl ctrl;
> +	int dlen[1];
> +	int ret;
> +
> +	ctrl.hdr.class = VIRTIO_NET_CTRL_RX;
> +	ctrl.hdr.cmd = VIRTIO_NET_CTRL_RX_PROMISC;
> +	ctrl.data[0] = 0;
> +	dlen[0] = 1;
> +
> +	ret = virtio_send_command(hw->cvq, &ctrl, dlen, 1);
> +
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Promisc disabling but send command "
> +			  "failed, this is too late now...\n");
> +	}
> +}
> +
> +static void
> +virtio_dev_allmulticast_enable(struct rte_eth_dev *dev)
> +{
> +	struct virtio_hw *hw
> +		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct virtio_pmd_ctrl ctrl;
> +	int dlen[1];
> +	int ret;
> +
> +	ctrl.hdr.class = VIRTIO_NET_CTRL_RX;
> +	ctrl.hdr.cmd = VIRTIO_NET_CTRL_RX_ALLMULTI;
> +	ctrl.data[0] = 1;
> +	dlen[0] = 1;
> +
> +	ret = virtio_send_command(hw->cvq, &ctrl, dlen, 1);
> +
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Promisc enabling but send command "
> +			  "failed, this is too late now...\n");
> +	}
> +}
> +
> +static void
> +virtio_dev_allmulticast_disable(struct rte_eth_dev *dev)
> +{
> +	struct virtio_hw *hw
> +		= VIRTIO_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct virtio_pmd_ctrl ctrl;
> +	int dlen[1];
> +	int ret;
> +
> +	ctrl.hdr.class = VIRTIO_NET_CTRL_RX;
> +	ctrl.hdr.cmd = VIRTIO_NET_CTRL_RX_ALLMULTI;
> +	ctrl.data[0] = 0;
> +	dlen[0] = 1;
> +
> +	ret = virtio_send_command(hw->cvq, &ctrl, dlen, 1);
> +
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Promisc disabling but send command "
> +			  "failed, this is too late now...\n");
> +	}
> +}
> +
>  /*
>   * dev_ops for virtio, bare necessities for basic operation
>   */
> @@ -411,6 +503,10 @@ static struct eth_dev_ops virtio_eth_dev_ops = {
>  	.dev_start               = virtio_dev_start,
>  	.dev_stop                = virtio_dev_stop,
>  	.dev_close               = virtio_dev_close,
> +	.promiscuous_enable      = virtio_dev_promiscuous_enable,
> +	.promiscuous_disable     = virtio_dev_promiscuous_disable,
> +	.allmulticast_enable     = virtio_dev_allmulticast_enable,
> +	.allmulticast_disable    = virtio_dev_allmulticast_disable,
>  
>  	.dev_infos_get           = virtio_dev_info_get,
>  	.stats_get               = virtio_dev_stats_get,
> @@ -561,7 +657,7 @@ virtio_negotiate_features(struct virtio_hw *hw)
>  {
>  	uint32_t host_features, mask;
>  
> -	mask = VIRTIO_NET_F_CTRL_RX | VIRTIO_NET_F_CTRL_VLAN;
> +	mask = VIRTIO_NET_F_CTRL_VLAN;
>  	mask |= VIRTIO_NET_F_CSUM | VIRTIO_NET_F_GUEST_CSUM;
>  
>  	/* TSO and LRO are only available when their corresponding

I have similar patches, but you really need to fix the driver
so that control queue is brought up on dev_init, not start.
It makes sense to allow code to change modes independent of whether
driver has been started or not.

Also, the virtio driver is doing reset on stop. This may not be
best solution because it conflicts with what Linux driver is doing.

Another current bug is that virtio DPDK driver is broken on latest
KVM. It works with Debian stable, but not with Debian testing.
Too busy to investigate further.