[dpdk-dev] [PATCH] e1000: enable allmulticast support for VF

[dpdk-dev] [PATCH] e1000: enable allmulticast support for VF

[Yury Kylulin]

Add support to enable and disable reception of all multicast packets by the VF using standard API
rte_eth_allmulticast_enable()/rte_eth_allmulticast_disable().

Signed-off-by: Yury Kylulin <yury.kylulin@intel.com>
---
 drivers/net/e1000/igb_ethdev.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index e4b370d..5196bd5 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -135,6 +135,8 @@ 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_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);
 static void eth_igbvf_stats_reset(struct rte_eth_dev *dev);
@@ -280,6 +282,8 @@ 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,
+	.allmulticast_enable  = igbvf_allmulticast_enable,
+	.allmulticast_disable = igbvf_allmulticast_disable,
 	.link_update          = eth_igb_link_update,
 	.stats_get            = eth_igbvf_stats_get,
 	.stats_reset          = eth_igbvf_stats_reset,
@@ -2272,6 +2276,22 @@ igbvf_dev_close(struct rte_eth_dev *dev)
 	igbvf_dev_stop(dev);
 }
 
+static void
+igbvf_allmulticast_enable(struct rte_eth_dev *dev)
+{
+	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	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);
+
+	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

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

[Thomas Monjalon]

We still have no maintainer for e1000.
Anyone to double-check this short patch?

2015-05-28 17:46, Yury Kylulin:
> Add support to enable and disable reception of all multicast packets by the VF using standard API
> rte_eth_allmulticast_enable()/rte_eth_allmulticast_disable().
> 
> Signed-off-by: Yury Kylulin <yury.kylulin@intel.com>
[...]
> +static void
> +igbvf_allmulticast_enable(struct rte_eth_dev *dev)
> +{
> +	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> +	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);
> +
> +	e1000_promisc_set_vf(hw, e1000_promisc_disabled);
> +}

The values come from this enum:
enum e1000_promisc_type {
	e1000_promisc_disabled = 0,   /* all promisc modes disabled */
	e1000_promisc_unicast = 1,    /* unicast promiscuous enabled */
	e1000_promisc_multicast = 2,  /* multicast promiscuous enabled */
	e1000_promisc_enabled = 3,    /* both uni and multicast promisc */

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

[Lu, Wenzhuo]

Hi Yury,
I have comments. I think the function name " igbvf_allmulticast_disable" is a little misleading. Because this function will disable not only multicast but also unicast, right?
Thanks.

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yury Kylulin
Sent: Thursday, May 28, 2015 10:47 PM
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH] e1000: enable allmulticast support for VF

Add support to enable and disable reception of all multicast packets by the VF using standard API rte_eth_allmulticast_enable()/rte_eth_allmulticast_disable().

Signed-off-by: Yury Kylulin <yury.kylulin@intel.com>
---
 drivers/net/e1000/igb_ethdev.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index e4b370d..5196bd5 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -135,6 +135,8 @@ 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_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);  static void eth_igbvf_stats_reset(struct rte_eth_dev *dev); @@ -280,6 +282,8 @@ 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,
+	.allmulticast_enable  = igbvf_allmulticast_enable,
+	.allmulticast_disable = igbvf_allmulticast_disable,
 	.link_update          = eth_igb_link_update,
 	.stats_get            = eth_igbvf_stats_get,
 	.stats_reset          = eth_igbvf_stats_reset,
@@ -2272,6 +2276,22 @@ igbvf_dev_close(struct rte_eth_dev *dev)
 	igbvf_dev_stop(dev);
 }
 
+static void
+igbvf_allmulticast_enable(struct rte_eth_dev *dev) {
+	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	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);
+
+	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

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

[Lu, Wenzhuo]

Hi Yury,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yury Kylulin
> Sent: Thursday, May 28, 2015 10:47 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] e1000: enable allmulticast support for VF
> 
> Add support to enable and disable reception of all multicast packets by the VF
> using standard API rte_eth_allmulticast_enable()/rte_eth_allmulticast_disable().
> 
> Signed-off-by: Yury Kylulin <yury.kylulin@intel.com>
> ---
>  drivers/net/e1000/igb_ethdev.c |   20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index e4b370d..5196bd5 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -135,6 +135,8 @@ 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_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);
> static void eth_igbvf_stats_reset(struct rte_eth_dev *dev); @@ -280,6 +282,8
> @@ 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,
> +	.allmulticast_enable  = igbvf_allmulticast_enable,
> +	.allmulticast_disable = igbvf_allmulticast_disable,
>  	.link_update          = eth_igb_link_update,
>  	.stats_get            = eth_igbvf_stats_get,
>  	.stats_reset          = eth_igbvf_stats_reset,
> @@ -2272,6 +2276,22 @@ igbvf_dev_close(struct rte_eth_dev *dev)
>  	igbvf_dev_stop(dev);
>  }
> 
> +static void
> +igbvf_allmulticast_enable(struct rte_eth_dev *dev) {
> +	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
> +	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);
> +
> +	e1000_promisc_set_vf(hw, e1000_promisc_disabled); }
This function disables both multicast and unicast. The PF function eth_igb_allmulticast_disable disables multicast but not unicast. I think it's better that VF and PF have the behavior.

Wenzhuo

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

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

[Yury Kylulin]

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

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

[Lu, Wenzhuo]

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

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

[Kylulin, Yury]

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

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

[Lu, Wenzhuo]

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

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

[Bruce Richardson]

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