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

["Kylulin, Yury" <yury.kylulin@intel.com>]

Hi Wenzhuo,

> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Monday, February 15, 2016 4:14 AM
> To: Kylulin, Yury <yury.kylulin@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH v2] e1000: enable promiscuous and allmulticast support
> for VF
> 
> 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?
In case of PF to alter these settings you need to change the value of the register,
but for VF we need to send message via mailbox. Assuming that in promiscuous mode
we already set both allmulticast and unicast promiscuous I tried to exclude and avoid
useless messages via mailbox between PF and VF.
> 
> > +
> > +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