Re: [dpdk-dev] [PATCH v2] e1000: enable promiscuous and allmulticast support for VF

["Lu, Wenzhuo" <wenzhuo.lu@intel.com>]

Hi Yury,

> -----Original Message-----
> From: Kylulin, Yury
> Sent: Tuesday, February 9, 2016 5:10 PM
> To: dev@dpdk.org
> Cc: Kylulin, Yury; Lu, Wenzhuo
> Subject: [PATCH v2] e1000: enable promiscuous and allmulticast support for VF
> 
> Enable promiscuous and allmulticast mode control from the VF using
> rte_eth_promiscuous_enable()/rte_eth_promiscuous_disable() and
> rte_eth_allmulticast_enable()/rte_eth_allmulticast_disable().
> 
> For promiscuous mode host/PF igb driver should be built with
> IGB_ENABLE_VF_PROMISC.
> 
> For allmulticast mode "allmulti" flag should be set for appropriate PF ifconfig
> eth0 allmulti
> 
> Signed-off-by: Yury Kylulin <yury.kylulin@intel.com>
> ---
> v2
> * Added promiscuous mode control
> * Switching logic is the same like in igb PF driver
> 
>  drivers/net/e1000/igb_ethdev.c |   49
> ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index d1bbcda..677f9a2 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -152,6 +152,10 @@ static int igbvf_dev_configure(struct rte_eth_dev *dev);
> static int igbvf_dev_start(struct rte_eth_dev *dev);  static void
> igbvf_dev_stop(struct rte_eth_dev *dev);  static void igbvf_dev_close(struct
> rte_eth_dev *dev);
> +static void igbvf_promiscuous_enable(struct rte_eth_dev *dev); static
> +void igbvf_promiscuous_disable(struct rte_eth_dev *dev); static void
> +igbvf_allmulticast_enable(struct rte_eth_dev *dev); static void
> +igbvf_allmulticast_disable(struct rte_eth_dev *dev);
>  static int eth_igbvf_link_update(struct e1000_hw *hw);  static void
> eth_igbvf_stats_get(struct rte_eth_dev *dev,
>  				struct rte_eth_stats *rte_stats);
> @@ -369,6 +373,10 @@ static const struct eth_dev_ops igbvf_eth_dev_ops = {
>  	.dev_start            = igbvf_dev_start,
>  	.dev_stop             = igbvf_dev_stop,
>  	.dev_close            = igbvf_dev_close,
> +	.promiscuous_enable   = igbvf_promiscuous_enable,
> +	.promiscuous_disable  = igbvf_promiscuous_disable,
> +	.allmulticast_enable  = igbvf_allmulticast_enable,
> +	.allmulticast_disable = igbvf_allmulticast_disable,
>  	.link_update          = eth_igb_link_update,
>  	.stats_get            = eth_igbvf_stats_get,
>  	.xstats_get           = eth_igbvf_xstats_get,
> @@ -2829,6 +2837,47 @@ igbvf_dev_close(struct rte_eth_dev *dev)
>  	igb_dev_free_queues(dev);
>  }
> 
> +static void
> +igbvf_promiscuous_enable(struct rte_eth_dev *dev) {
> +	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
> +	/* Set both unicast and multicast promisc */
> +	e1000_promisc_set_vf(hw, e1000_promisc_enabled); }
> +
> +static void
> +igbvf_promiscuous_disable(struct rte_eth_dev *dev) {
> +	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
> +	/* If in allmulticast mode leave multicast promisc */
> +	if (dev->data->all_multicast == 1)
> +		e1000_promisc_set_vf(hw, e1000_promisc_multicast);
> +	else
> +		e1000_promisc_set_vf(hw, e1000_promisc_disabled); }
> +
> +static void
> +igbvf_allmulticast_enable(struct rte_eth_dev *dev) {
> +	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
> +	/* In promiscuous mode multicast promisc already set */
> +	if (dev->data->promiscuous == 0)
> +		e1000_promisc_set_vf(hw, e1000_promisc_multicast); }
Comparing with PF, I think PF will enable multicast promiscuous mode no matter dev->data->promiscuous is 0 or not.
Should the VF have the same behavior? 

> +
> +static void
> +igbvf_allmulticast_disable(struct rte_eth_dev *dev) {
> +	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
> +	/* In promiscuous mode leave multicast promisc enabled */
> +	if (dev->data->promiscuous == 0)
> +		e1000_promisc_set_vf(hw, e1000_promisc_disabled); }
> +
>  static int igbvf_set_vfta(struct e1000_hw *hw, uint16_t vid, bool on)  {
>  	struct e1000_mbx_info *mbx = &hw->mbx;
> --
> 1.7.9.5