diff mbox

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

Message ID 1455008984-16507-1-git-send-email-yury.kylulin@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Bruce Richardson
Headers

Commit Message

Yury Kylulin Feb. 9, 2016, 9:09 a.m. UTC 
  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(+)

Comments

Wenzhuo Lu Feb. 15, 2016, 1:14 a.m. UTC | #1 
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
Yury Kylulin Feb. 15, 2016, 10:44 a.m. UTC | #2 
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
Wenzhuo Lu Feb. 16, 2016, 12:43 a.m. UTC | #3 
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
Bruce Richardson Feb. 26, 2016, 5:28 p.m. UTC | #4 
On Tue, Feb 16, 2016 at 12:43:38AM +0000, Lu, Wenzhuo wrote:
> Hi Yury,
> 
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> 
Applied to dpdk-next-net/rel_16_04

/Bruce
diff mbox

Patch

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);
+}
+
+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;