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

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

Hi Yury,

Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

> -----Original Message-----
> From: Kylulin, Yury
> Sent: Monday, February 15, 2016 6:45 PM
> To: Lu, Wenzhuo; dev@dpdk.org
> Subject: RE: [PATCH v2] e1000: enable promiscuous and allmulticast support for
> VF
> 
> 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.
Thanks for the explanation. I think you're right:)

> >
> > > +
> > > +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