[dpdk-dev] [PATCH 0/2] net/failsafe: support multicast MAC address set

[dpdk-dev] [PATCH 0/2] net/failsafe: support multicast MAC address set

[Andrew Rybchenko]

Evgeny Im (2):
  net/failsafe: remove not supported multicast MAC filter
  net/failsafe: support multicast address list set

 doc/guides/rel_notes/release_18_11.rst  |  6 ++++
 drivers/net/failsafe/failsafe.c         |  1 +
 drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
 drivers/net/failsafe/failsafe_ops.c     | 48 +++++++++++++++++++++++++
 drivers/net/failsafe/failsafe_private.h |  2 ++
 5 files changed, 74 insertions(+)

-- 
2.17.1

[dpdk-dev] [PATCH 1/2] net/failsafe: remove not supported multicast MAC filter

[Andrew Rybchenko]

From: Evgeny Im <Evgeny.Im@oktetlabs.com>

set_mc_addr_list method is not implemented by the driver yet.

Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD")
Cc: stable@dpdk.org

Signed-off-by: Evgeny Im <Evgeny.Im@oktetlabs.com>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/nics/features/failsafe.ini | 1 -
 1 file changed, 1 deletion(-)

diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
index 39ee57965..83cc99d19 100644
--- a/doc/guides/nics/features/failsafe.ini
+++ b/doc/guides/nics/features/failsafe.ini
@@ -12,7 +12,6 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Unicast MAC filter   = Y
-Multicast MAC filter = Y
 VLAN filter          = Y
 Flow control         = Y
 Flow API             = Y
-- 
2.17.1

[dpdk-dev] [PATCH 2/2] net/failsafe: support multicast address list set

[Andrew Rybchenko]

From: Evgeny Im <Evgeny.Im@oktetlabs.com>

Signed-off-by: Evgeny Im <Evgeny.Im@oktetlabs.com>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/nics/features/failsafe.ini   |  1 +
 doc/guides/rel_notes/release_18_11.rst  |  6 ++++
 drivers/net/failsafe/failsafe.c         |  1 +
 drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
 drivers/net/failsafe/failsafe_ops.c     | 48 +++++++++++++++++++++++++
 drivers/net/failsafe/failsafe_private.h |  2 ++
 6 files changed, 75 insertions(+)

diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
index 83cc99d19..39ee57965 100644
--- a/doc/guides/nics/features/failsafe.ini
+++ b/doc/guides/nics/features/failsafe.ini
@@ -12,6 +12,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Unicast MAC filter   = Y
+Multicast MAC filter = Y
 VLAN filter          = Y
 Flow control         = Y
 Flow API             = Y
diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 24204e67b..54e0e4ee4 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -54,6 +54,12 @@ New Features
      Also, make sure to start the actual text at the margin.
      =========================================================
 
+* **Updated failsafe driver.**
+
+  Updated the failsafe driver including the following changes:
+
+  * Support multicast MAC address set.
+
 
 API Changes
 -----------
diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index 657919f93..c3999f026 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -304,6 +304,7 @@ fs_rte_eth_free(const char *name)
 	ret = pthread_mutex_destroy(&PRIV(dev)->hotplug_mutex);
 	if (ret)
 		ERROR("Error while destroying hotplug mutex");
+	rte_free(PRIV(dev)->mcast_addrs);
 	rte_free(PRIV(dev));
 	rte_eth_dev_release_port(dev);
 	return ret;
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 5b5cb3b49..5078feabe 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -424,6 +424,23 @@ failsafe_eth_dev_state_sync(struct rte_eth_dev *dev)
 	ret = dev->dev_ops->dev_start(dev);
 	if (ret)
 		goto err_remove;
+	/*
+	 * Propagate multicast MAC addresses to sub-devices,
+	 * if non zero number of addresses is set.
+	 * The condition is required to avoid breakage of failsafe
+	 * for sub-devices which do not support the operation
+	 * if the feature is not used.
+	 */
+	if (PRIV(dev)->nb_mcast_addr > 0) {
+		ret = dev->dev_ops->set_mc_addr_list(dev,
+						     PRIV(dev)->mcast_addrs,
+						     PRIV(dev)->nb_mcast_addr);
+		if (ret) {
+			ERROR("Could not set list of multicast addresses to sub_device %d",
+			      i);
+			goto err_remove;
+		}
+	}
 	return 0;
 err_remove:
 	FOREACH_SUBDEV(sdev, i, dev)
diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index 24e91c931..2583cff58 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -952,6 +952,53 @@ fs_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 	return 0;
 }
 
+static int
+fs_set_mc_addr_list(struct rte_eth_dev *dev,
+		    struct ether_addr *mc_addr_set, uint32_t nb_mc_addr)
+{
+	struct sub_device *sdev;
+	uint8_t i;
+	int ret;
+
+	fs_lock(dev, 0);
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		ret = rte_eth_dev_set_mc_addr_list(PORT_ID(sdev),
+						   mc_addr_set, nb_mc_addr);
+		if (ret != 0) {
+			ERROR("Operation rte_eth_dev_set_mc_addr_list failed for sub_device %d with error %d",
+			      i, ret);
+			goto rollback;
+		}
+	}
+	/* Do not reallocate/save if the method is called from sync */
+	if (mc_addr_set != PRIV(dev)->mcast_addrs) {
+		void *mcast_addrs = rte_realloc(PRIV(dev)->mcast_addrs,
+			nb_mc_addr * sizeof(PRIV(dev)->mcast_addrs[0]), 0);
+		if (mcast_addrs == NULL) {
+			ret = -ENOMEM;
+			goto rollback;
+		}
+		PRIV(dev)->mcast_addrs = mcast_addrs;
+		rte_memcpy(PRIV(dev)->mcast_addrs, mc_addr_set,
+			   nb_mc_addr * sizeof(PRIV(dev)->mcast_addrs[0]));
+	}
+	PRIV(dev)->nb_mcast_addr = nb_mc_addr;
+	fs_unlock(dev, 0);
+	return 0;
+
+rollback:
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		int rc = rte_eth_dev_set_mc_addr_list(PORT_ID(sdev),
+			PRIV(dev)->mcast_addrs,	PRIV(dev)->nb_mcast_addr);
+		if (rc != 0) {
+			ERROR("Multicast MAC address list rollback for sub_device %d failed with error %d",
+			      i, rc);
+		}
+	}
+	fs_unlock(dev, 0);
+	return ret;
+}
+
 static int
 fs_rss_hash_update(struct rte_eth_dev *dev,
 			struct rte_eth_rss_conf *rss_conf)
@@ -1036,6 +1083,7 @@ const struct eth_dev_ops failsafe_ops = {
 	.mac_addr_remove = fs_mac_addr_remove,
 	.mac_addr_add = fs_mac_addr_add,
 	.mac_addr_set = fs_mac_addr_set,
+	.set_mc_addr_list = fs_set_mc_addr_list,
 	.rss_hash_update = fs_rss_hash_update,
 	.filter_ctrl = fs_filter_ctrl,
 };
diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
index 886af8616..abbe73e87 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -143,6 +143,8 @@ struct fs_priv {
 	uint32_t nb_mac_addr;
 	struct ether_addr mac_addrs[FAILSAFE_MAX_ETHADDR];
 	uint32_t mac_addr_pool[FAILSAFE_MAX_ETHADDR];
+	uint32_t nb_mcast_addr;
+	struct ether_addr *mcast_addrs;
 	/* current capabilities */
 	struct rte_eth_dev_info infos;
 	struct rte_eth_dev_owner my_owner; /* Unique owner. */
-- 
2.17.1

Re: [dpdk-dev] [PATCH 2/2] net/failsafe: support multicast address list set

[Andrew Rybchenko]

On 08/31/2018 06:53 PM, Andrew Rybchenko wrote:
> From: Evgeny Im <Evgeny.Im@oktetlabs.com>
>
> Signed-off-by: Evgeny Im <Evgeny.Im@oktetlabs.com>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

<...>


> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
> index 24e91c931..2583cff58 100644
> --- a/drivers/net/failsafe/failsafe_ops.c
> +++ b/drivers/net/failsafe/failsafe_ops.c
> @@ -952,6 +952,53 @@ fs_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
>   	return 0;
>   }
>   
> +static int
> +fs_set_mc_addr_list(struct rte_eth_dev *dev,
> +		    struct ether_addr *mc_addr_set, uint32_t nb_mc_addr)
> +{
> +	struct sub_device *sdev;
> +	uint8_t i;
> +	int ret;
> +
> +	fs_lock(dev, 0);
> +	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> +		ret = rte_eth_dev_set_mc_addr_list(PORT_ID(sdev),
> +						   mc_addr_set, nb_mc_addr);
> +		if (ret != 0) {
> +			ERROR("Operation rte_eth_dev_set_mc_addr_list failed for sub_device %d with error %d",
> +			      i, ret);
> +			goto rollback;
> +		}
> +	}
> +	/* Do not reallocate/save if the method is called from sync */
> +	if (mc_addr_set != PRIV(dev)->mcast_addrs) {
> +		void *mcast_addrs = rte_realloc(PRIV(dev)->mcast_addrs,
> +			nb_mc_addr * sizeof(PRIV(dev)->mcast_addrs[0]), 0);
> +		if (mcast_addrs == NULL) {
> +			ret = -ENOMEM;

Self-NACK since it fails for empty list set since rte_realloc() returns 
NULL.
Will fix in v2.

> +			goto rollback;
> +		}
> +		PRIV(dev)->mcast_addrs = mcast_addrs;
> +		rte_memcpy(PRIV(dev)->mcast_addrs, mc_addr_set,
> +			   nb_mc_addr * sizeof(PRIV(dev)->mcast_addrs[0]));
> +	}
> +	PRIV(dev)->nb_mcast_addr = nb_mc_addr;
> +	fs_unlock(dev, 0);
> +	return 0;
> +
> +rollback:
> +	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> +		int rc = rte_eth_dev_set_mc_addr_list(PORT_ID(sdev),
> +			PRIV(dev)->mcast_addrs,	PRIV(dev)->nb_mcast_addr);
> +		if (rc != 0) {
> +			ERROR("Multicast MAC address list rollback for sub_device %d failed with error %d",
> +			      i, rc);
> +		}
> +	}
> +	fs_unlock(dev, 0);
> +	return ret;
> +}
> +
>   static int
>   fs_rss_hash_update(struct rte_eth_dev *dev,
>   			struct rte_eth_rss_conf *rss_conf)

<...>

[dpdk-dev] [PATCH v2 0/2] net/failsafe: support multicast MAC address set

[Andrew Rybchenko]

Evgeny Im (2):
  net/failsafe: remove not supported multicast MAC filter
  net/failsafe: support multicast address list set

 doc/guides/rel_notes/release_18_11.rst  |  6 ++++
 drivers/net/failsafe/failsafe.c         |  1 +
 drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
 drivers/net/failsafe/failsafe_ops.c     | 48 +++++++++++++++++++++++++
 drivers/net/failsafe/failsafe_private.h |  2 ++
 5 files changed, 74 insertions(+)

-- 
2.17.1

[dpdk-dev] [PATCH v2 1/2] net/failsafe: remove not supported multicast MAC filter

[Andrew Rybchenko]

From: Evgeny Im <Evgeny.Im@oktetlabs.com>

set_mc_addr_list method is not implemented by the driver yet.

Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD")
Cc: stable@dpdk.org

Signed-off-by: Evgeny Im <Evgeny.Im@oktetlabs.com>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/nics/features/failsafe.ini | 1 -
 1 file changed, 1 deletion(-)

diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
index 39ee57965..83cc99d19 100644
--- a/doc/guides/nics/features/failsafe.ini
+++ b/doc/guides/nics/features/failsafe.ini
@@ -12,7 +12,6 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Unicast MAC filter   = Y
-Multicast MAC filter = Y
 VLAN filter          = Y
 Flow control         = Y
 Flow API             = Y
-- 
2.17.1

[dpdk-dev] [PATCH v2 2/2] net/failsafe: support multicast address list set

[Andrew Rybchenko]

From: Evgeny Im <Evgeny.Im@oktetlabs.com>

Signed-off-by: Evgeny Im <Evgeny.Im@oktetlabs.com>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/nics/features/failsafe.ini   |  1 +
 doc/guides/rel_notes/release_18_11.rst  |  6 ++++
 drivers/net/failsafe/failsafe.c         |  1 +
 drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
 drivers/net/failsafe/failsafe_ops.c     | 48 +++++++++++++++++++++++++
 drivers/net/failsafe/failsafe_private.h |  2 ++
 6 files changed, 75 insertions(+)

diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
index 83cc99d19..39ee57965 100644
--- a/doc/guides/nics/features/failsafe.ini
+++ b/doc/guides/nics/features/failsafe.ini
@@ -12,6 +12,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Unicast MAC filter   = Y
+Multicast MAC filter = Y
 VLAN filter          = Y
 Flow control         = Y
 Flow API             = Y
diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 24204e67b..54e0e4ee4 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -54,6 +54,12 @@ New Features
      Also, make sure to start the actual text at the margin.
      =========================================================
 
+* **Updated failsafe driver.**
+
+  Updated the failsafe driver including the following changes:
+
+  * Support multicast MAC address set.
+
 
 API Changes
 -----------
diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index 657919f93..c3999f026 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -304,6 +304,7 @@ fs_rte_eth_free(const char *name)
 	ret = pthread_mutex_destroy(&PRIV(dev)->hotplug_mutex);
 	if (ret)
 		ERROR("Error while destroying hotplug mutex");
+	rte_free(PRIV(dev)->mcast_addrs);
 	rte_free(PRIV(dev));
 	rte_eth_dev_release_port(dev);
 	return ret;
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 5b5cb3b49..5078feabe 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -424,6 +424,23 @@ failsafe_eth_dev_state_sync(struct rte_eth_dev *dev)
 	ret = dev->dev_ops->dev_start(dev);
 	if (ret)
 		goto err_remove;
+	/*
+	 * Propagate multicast MAC addresses to sub-devices,
+	 * if non zero number of addresses is set.
+	 * The condition is required to avoid breakage of failsafe
+	 * for sub-devices which do not support the operation
+	 * if the feature is really not used.
+	 */
+	if (PRIV(dev)->nb_mcast_addr > 0) {
+		ret = dev->dev_ops->set_mc_addr_list(dev,
+						     PRIV(dev)->mcast_addrs,
+						     PRIV(dev)->nb_mcast_addr);
+		if (ret) {
+			ERROR("Could not set list of multicast addresses to sub_device %d",
+			      i);
+			goto err_remove;
+		}
+	}
 	return 0;
 err_remove:
 	FOREACH_SUBDEV(sdev, i, dev)
diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index 24e91c931..082685bb1 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -952,6 +952,53 @@ fs_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 	return 0;
 }
 
+static int
+fs_set_mc_addr_list(struct rte_eth_dev *dev,
+		    struct ether_addr *mc_addr_set, uint32_t nb_mc_addr)
+{
+	struct sub_device *sdev;
+	uint8_t i;
+	int ret;
+
+	fs_lock(dev, 0);
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		ret = rte_eth_dev_set_mc_addr_list(PORT_ID(sdev),
+						   mc_addr_set, nb_mc_addr);
+		if (ret != 0) {
+			ERROR("Operation rte_eth_dev_set_mc_addr_list failed for sub_device %d with error %d",
+			      i, ret);
+			goto rollback;
+		}
+	}
+	/* Do not reallocate/save if the method is called from sync */
+	if (mc_addr_set != PRIV(dev)->mcast_addrs) {
+		void *mcast_addrs = rte_realloc(PRIV(dev)->mcast_addrs,
+			nb_mc_addr * sizeof(PRIV(dev)->mcast_addrs[0]), 0);
+		if (mcast_addrs == NULL && nb_mc_addr > 0) {
+			ret = -ENOMEM;
+			goto rollback;
+		}
+		PRIV(dev)->mcast_addrs = mcast_addrs;
+		rte_memcpy(PRIV(dev)->mcast_addrs, mc_addr_set,
+			   nb_mc_addr * sizeof(PRIV(dev)->mcast_addrs[0]));
+	}
+	PRIV(dev)->nb_mcast_addr = nb_mc_addr;
+	fs_unlock(dev, 0);
+	return 0;
+
+rollback:
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		int rc = rte_eth_dev_set_mc_addr_list(PORT_ID(sdev),
+			PRIV(dev)->mcast_addrs,	PRIV(dev)->nb_mcast_addr);
+		if (rc != 0) {
+			ERROR("Multicast MAC address list rollback for sub_device %d failed with error %d",
+			      i, rc);
+		}
+	}
+	fs_unlock(dev, 0);
+	return ret;
+}
+
 static int
 fs_rss_hash_update(struct rte_eth_dev *dev,
 			struct rte_eth_rss_conf *rss_conf)
@@ -1036,6 +1083,7 @@ const struct eth_dev_ops failsafe_ops = {
 	.mac_addr_remove = fs_mac_addr_remove,
 	.mac_addr_add = fs_mac_addr_add,
 	.mac_addr_set = fs_mac_addr_set,
+	.set_mc_addr_list = fs_set_mc_addr_list,
 	.rss_hash_update = fs_rss_hash_update,
 	.filter_ctrl = fs_filter_ctrl,
 };
diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
index 886af8616..abbe73e87 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -143,6 +143,8 @@ struct fs_priv {
 	uint32_t nb_mac_addr;
 	struct ether_addr mac_addrs[FAILSAFE_MAX_ETHADDR];
 	uint32_t mac_addr_pool[FAILSAFE_MAX_ETHADDR];
+	uint32_t nb_mcast_addr;
+	struct ether_addr *mcast_addrs;
 	/* current capabilities */
 	struct rte_eth_dev_info infos;
 	struct rte_eth_dev_owner my_owner; /* Unique owner. */
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2 2/2] net/failsafe: support multicast address list set

[Gaëtan Rivet]

Hi,

Sorry about the delay on this, overall it looks ok;
I have an issue however, see inline.

On Mon, Sep 03, 2018 at 07:55:22AM +0100, Andrew Rybchenko wrote:
> From: Evgeny Im <Evgeny.Im@oktetlabs.com>
> 
> Signed-off-by: Evgeny Im <Evgeny.Im@oktetlabs.com>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  doc/guides/nics/features/failsafe.ini   |  1 +
>  doc/guides/rel_notes/release_18_11.rst  |  6 ++++
>  drivers/net/failsafe/failsafe.c         |  1 +
>  drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
>  drivers/net/failsafe/failsafe_ops.c     | 48 +++++++++++++++++++++++++
>  drivers/net/failsafe/failsafe_private.h |  2 ++
>  6 files changed, 75 insertions(+)
> 
> diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
> index 83cc99d19..39ee57965 100644
> --- a/doc/guides/nics/features/failsafe.ini
> +++ b/doc/guides/nics/features/failsafe.ini
> @@ -12,6 +12,7 @@ Jumbo frame          = Y
>  Promiscuous mode     = Y
>  Allmulticast mode    = Y
>  Unicast MAC filter   = Y
> +Multicast MAC filter = Y
>  VLAN filter          = Y
>  Flow control         = Y
>  Flow API             = Y
> diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
> index 24204e67b..54e0e4ee4 100644
> --- a/doc/guides/rel_notes/release_18_11.rst
> +++ b/doc/guides/rel_notes/release_18_11.rst
> @@ -54,6 +54,12 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =========================================================
>  
> +* **Updated failsafe driver.**
> +
> +  Updated the failsafe driver including the following changes:
> +
> +  * Support multicast MAC address set.
> +
>  
>  API Changes
>  -----------
> diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
> index 657919f93..c3999f026 100644
> --- a/drivers/net/failsafe/failsafe.c
> +++ b/drivers/net/failsafe/failsafe.c
> @@ -304,6 +304,7 @@ fs_rte_eth_free(const char *name)
>  	ret = pthread_mutex_destroy(&PRIV(dev)->hotplug_mutex);
>  	if (ret)
>  		ERROR("Error while destroying hotplug mutex");
> +	rte_free(PRIV(dev)->mcast_addrs);
>  	rte_free(PRIV(dev));
>  	rte_eth_dev_release_port(dev);
>  	return ret;
> diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
> index 5b5cb3b49..5078feabe 100644
> --- a/drivers/net/failsafe/failsafe_ether.c
> +++ b/drivers/net/failsafe/failsafe_ether.c
> @@ -424,6 +424,23 @@ failsafe_eth_dev_state_sync(struct rte_eth_dev *dev)
>  	ret = dev->dev_ops->dev_start(dev);
>  	if (ret)
>  		goto err_remove;
> +	/*
> +	 * Propagate multicast MAC addresses to sub-devices,
> +	 * if non zero number of addresses is set.
> +	 * The condition is required to avoid breakage of failsafe
> +	 * for sub-devices which do not support the operation
> +	 * if the feature is really not used.
> +	 */
> +	if (PRIV(dev)->nb_mcast_addr > 0) {
> +		ret = dev->dev_ops->set_mc_addr_list(dev,
> +						     PRIV(dev)->mcast_addrs,
> +						     PRIV(dev)->nb_mcast_addr);
> +		if (ret) {
> +			ERROR("Could not set list of multicast addresses to sub_device %d",
> +			      i);
> +			goto err_remove;
> +		}
> +	}

Using here the dev-ops instead of calling the rte_eth_* API as is done for the
other configuration items, is unorthodox and I believe could lead to
issues.

Why didn't you call rte_eth_dev_set_mc_addr_list on the new port only instead,
the same way it is done for the other configuration item?

Using the dev-ops, you are making the other sub-device re-apply the
same configuration periodically (in case of repeated hotplug error),
twice per sub-device upkeep cycle. This is unnecessary and seems to
foster instability for no clear benefit. Can you justify it?

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH v2 2/2] net/failsafe: support multicast address list set

[Gaëtan Rivet]

On Wed, Sep 19, 2018 at 04:50:57PM +0200, Gaëtan Rivet wrote:
> Hi,
> 
> Sorry about the delay on this, overall it looks ok;
> I have an issue however, see inline.
> 
> On Mon, Sep 03, 2018 at 07:55:22AM +0100, Andrew Rybchenko wrote:
> > From: Evgeny Im <Evgeny.Im@oktetlabs.com>
> > 
> > Signed-off-by: Evgeny Im <Evgeny.Im@oktetlabs.com>
> > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > ---
> >  doc/guides/nics/features/failsafe.ini   |  1 +
> >  doc/guides/rel_notes/release_18_11.rst  |  6 ++++
> >  drivers/net/failsafe/failsafe.c         |  1 +
> >  drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
> >  drivers/net/failsafe/failsafe_ops.c     | 48 +++++++++++++++++++++++++
> >  drivers/net/failsafe/failsafe_private.h |  2 ++
> >  6 files changed, 75 insertions(+)
> > 
> > diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
> > index 83cc99d19..39ee57965 100644
> > --- a/doc/guides/nics/features/failsafe.ini
> > +++ b/doc/guides/nics/features/failsafe.ini
> > @@ -12,6 +12,7 @@ Jumbo frame          = Y
> >  Promiscuous mode     = Y
> >  Allmulticast mode    = Y
> >  Unicast MAC filter   = Y
> > +Multicast MAC filter = Y
> >  VLAN filter          = Y
> >  Flow control         = Y
> >  Flow API             = Y
> > diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
> > index 24204e67b..54e0e4ee4 100644
> > --- a/doc/guides/rel_notes/release_18_11.rst
> > +++ b/doc/guides/rel_notes/release_18_11.rst
> > @@ -54,6 +54,12 @@ New Features
> >       Also, make sure to start the actual text at the margin.
> >       =========================================================
> >  
> > +* **Updated failsafe driver.**
> > +
> > +  Updated the failsafe driver including the following changes:
> > +
> > +  * Support multicast MAC address set.
> > +
> >  
> >  API Changes
> >  -----------
> > diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
> > index 657919f93..c3999f026 100644
> > --- a/drivers/net/failsafe/failsafe.c
> > +++ b/drivers/net/failsafe/failsafe.c
> > @@ -304,6 +304,7 @@ fs_rte_eth_free(const char *name)
> >  	ret = pthread_mutex_destroy(&PRIV(dev)->hotplug_mutex);
> >  	if (ret)
> >  		ERROR("Error while destroying hotplug mutex");
> > +	rte_free(PRIV(dev)->mcast_addrs);
> >  	rte_free(PRIV(dev));
> >  	rte_eth_dev_release_port(dev);
> >  	return ret;
> > diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
> > index 5b5cb3b49..5078feabe 100644
> > --- a/drivers/net/failsafe/failsafe_ether.c
> > +++ b/drivers/net/failsafe/failsafe_ether.c
> > @@ -424,6 +424,23 @@ failsafe_eth_dev_state_sync(struct rte_eth_dev *dev)
> >  	ret = dev->dev_ops->dev_start(dev);
> >  	if (ret)
> >  		goto err_remove;
> > +	/*
> > +	 * Propagate multicast MAC addresses to sub-devices,
> > +	 * if non zero number of addresses is set.
> > +	 * The condition is required to avoid breakage of failsafe
> > +	 * for sub-devices which do not support the operation
> > +	 * if the feature is really not used.
> > +	 */
> > +	if (PRIV(dev)->nb_mcast_addr > 0) {
> > +		ret = dev->dev_ops->set_mc_addr_list(dev,
> > +						     PRIV(dev)->mcast_addrs,
> > +						     PRIV(dev)->nb_mcast_addr);
> > +		if (ret) {
> > +			ERROR("Could not set list of multicast addresses to sub_device %d",
> > +			      i);
> > +			goto err_remove;
> > +		}
> > +	}
> 
> Using here the dev-ops instead of calling the rte_eth_* API as is done for the
> other configuration items, is unorthodox and I believe could lead to
> issues.

Sorry I forgot the mention it, but it seems that this could be done
in fs_eth_dev_conf_apply() instead, which explains why I would consider
using the dev-ops being unorthodox.

> 
> Why didn't you call rte_eth_dev_set_mc_addr_list on the new port only instead,
> the same way it is done for the other configuration item?
> 
> Using the dev-ops, you are making the other sub-device re-apply the
> same configuration periodically (in case of repeated hotplug error),
> twice per sub-device upkeep cycle. This is unnecessary and seems to
> foster instability for no clear benefit. Can you justify it?
> 

If it was necessary to call this after the dev_start, I think it
would be better to restrict the configuration to inactive sub-devices,
in any case.

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH v2 2/2] net/failsafe: support multicast address list set

[Andrew Rybchenko]

On 9/19/18 6:12 PM, Gaëtan Rivet wrote:
> On Wed, Sep 19, 2018 at 04:50:57PM +0200, Gaëtan Rivet wrote:
>> Hi,
>>
>> Sorry about the delay on this, overall it looks ok;
>> I have an issue however, see inline.
>>
>> On Mon, Sep 03, 2018 at 07:55:22AM +0100, Andrew Rybchenko wrote:
>>> From: Evgeny Im <Evgeny.Im@oktetlabs.com>
>>>
>>> Signed-off-by: Evgeny Im <Evgeny.Im@oktetlabs.com>
>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>> ---
>>>   doc/guides/nics/features/failsafe.ini   |  1 +
>>>   doc/guides/rel_notes/release_18_11.rst  |  6 ++++
>>>   drivers/net/failsafe/failsafe.c         |  1 +
>>>   drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
>>>   drivers/net/failsafe/failsafe_ops.c     | 48 +++++++++++++++++++++++++
>>>   drivers/net/failsafe/failsafe_private.h |  2 ++
>>>   6 files changed, 75 insertions(+)
>>>
>>> diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
>>> index 83cc99d19..39ee57965 100644
>>> --- a/doc/guides/nics/features/failsafe.ini
>>> +++ b/doc/guides/nics/features/failsafe.ini
>>> @@ -12,6 +12,7 @@ Jumbo frame          = Y
>>>   Promiscuous mode     = Y
>>>   Allmulticast mode    = Y
>>>   Unicast MAC filter   = Y
>>> +Multicast MAC filter = Y
>>>   VLAN filter          = Y
>>>   Flow control         = Y
>>>   Flow API             = Y
>>> diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
>>> index 24204e67b..54e0e4ee4 100644
>>> --- a/doc/guides/rel_notes/release_18_11.rst
>>> +++ b/doc/guides/rel_notes/release_18_11.rst
>>> @@ -54,6 +54,12 @@ New Features
>>>        Also, make sure to start the actual text at the margin.
>>>        =========================================================
>>>   
>>> +* **Updated failsafe driver.**
>>> +
>>> +  Updated the failsafe driver including the following changes:
>>> +
>>> +  * Support multicast MAC address set.
>>> +
>>>   
>>>   API Changes
>>>   -----------
>>> diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
>>> index 657919f93..c3999f026 100644
>>> --- a/drivers/net/failsafe/failsafe.c
>>> +++ b/drivers/net/failsafe/failsafe.c
>>> @@ -304,6 +304,7 @@ fs_rte_eth_free(const char *name)
>>>   	ret = pthread_mutex_destroy(&PRIV(dev)->hotplug_mutex);
>>>   	if (ret)
>>>   		ERROR("Error while destroying hotplug mutex");
>>> +	rte_free(PRIV(dev)->mcast_addrs);
>>>   	rte_free(PRIV(dev));
>>>   	rte_eth_dev_release_port(dev);
>>>   	return ret;
>>> diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
>>> index 5b5cb3b49..5078feabe 100644
>>> --- a/drivers/net/failsafe/failsafe_ether.c
>>> +++ b/drivers/net/failsafe/failsafe_ether.c
>>> @@ -424,6 +424,23 @@ failsafe_eth_dev_state_sync(struct rte_eth_dev *dev)
>>>   	ret = dev->dev_ops->dev_start(dev);
>>>   	if (ret)
>>>   		goto err_remove;
>>> +	/*
>>> +	 * Propagate multicast MAC addresses to sub-devices,
>>> +	 * if non zero number of addresses is set.
>>> +	 * The condition is required to avoid breakage of failsafe
>>> +	 * for sub-devices which do not support the operation
>>> +	 * if the feature is really not used.
>>> +	 */
>>> +	if (PRIV(dev)->nb_mcast_addr > 0) {
>>> +		ret = dev->dev_ops->set_mc_addr_list(dev,
>>> +						     PRIV(dev)->mcast_addrs,
>>> +						     PRIV(dev)->nb_mcast_addr);
>>> +		if (ret) {
>>> +			ERROR("Could not set list of multicast addresses to sub_device %d",
>>> +			      i);
>>> +			goto err_remove;
>>> +		}
>>> +	}
>> Using here the dev-ops instead of calling the rte_eth_* API as is done for the
>> other configuration items, is unorthodox and I believe could lead to
>> issues.
> Sorry I forgot the mention it, but it seems that this could be done
> in fs_eth_dev_conf_apply() instead, which explains why I would consider
> using the dev-ops being unorthodox.

Yes, I've overlooked it on my review before submission.

>> Why didn't you call rte_eth_dev_set_mc_addr_list on the new port only instead,
>> the same way it is done for the other configuration item?
>>
>> Using the dev-ops, you are making the other sub-device re-apply the
>> same configuration periodically (in case of repeated hotplug error),
>> twice per sub-device upkeep cycle. This is unnecessary and seems to
>> foster instability for no clear benefit. Can you justify it?
>>
> If it was necessary to call this after the dev_start, I think it
> would be better to restrict the configuration to inactive sub-devices,
> in any case.

In theory, multicast addresses list is not listed in configuration items
retained across stop/start, so, could be wrong to set before start.
I hope it is just incomplete documentation in ethdev and we should
just fix it.

Thanks a lot for review,
Andrew.

[dpdk-dev] [PATCH v3 0/2] net/failsafe: support multicast MAC address set

[Andrew Rybchenko]

v3:
    - move apply on sync to fs_eth_dev_conf_apply() to apply to
      a new subdevice only
    - use ethdev API to apply to sub-device on sync
    - remove unnecessary check the same pointer from the method
      implementation in failsafe

v2:
    - fix setting of zero addresses since rte_realloc() returns NULL

Evgeny Im (2):
  net/failsafe: remove not supported multicast MAC filter
  net/failsafe: support multicast address list set

 doc/guides/rel_notes/release_18_11.rst  |  1 +
 drivers/net/failsafe/failsafe.c         |  1 +
 drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
 drivers/net/failsafe/failsafe_ops.c     | 50 +++++++++++++++++++++++++
 drivers/net/failsafe/failsafe_private.h |  2 +
 5 files changed, 71 insertions(+)

-- 
2.17.1

[dpdk-dev] [PATCH v3 0/2] net/failsafe: support multicast MAC address set

[Andrew Rybchenko]

v3:
    - move apply on sync to fs_eth_dev_conf_apply() to apply to
      a new subdevice only
    - use ethdev API to apply to sub-device on sync
    - remove unnecessary check the same pointer from the method
      implementation in failsafe

v2:
    - fix setting of zero addresses since rte_realloc() returns NULL

Evgeny Im (2):
  net/failsafe: remove not supported multicast MAC filter
  net/failsafe: support multicast address list set

 doc/guides/rel_notes/release_18_11.rst  |  7 ++++
 drivers/net/failsafe/failsafe.c         |  1 +
 drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
 drivers/net/failsafe/failsafe_ops.c     | 50 +++++++++++++++++++++++++
 drivers/net/failsafe/failsafe_private.h |  2 +
 5 files changed, 77 insertions(+)

-- 
2.17.1

[dpdk-dev] [PATCH v3 1/2] net/failsafe: remove not supported multicast MAC filter

[Andrew Rybchenko]

From: Evgeny Im <Evgeny.Im@oktetlabs.com>

set_mc_addr_list method is not implemented by the driver yet.

Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD")
Cc: stable@dpdk.org

Signed-off-by: Evgeny Im <Evgeny.Im@oktetlabs.com>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/nics/features/failsafe.ini | 1 -
 1 file changed, 1 deletion(-)

diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
index e3c4c08f2..89e253df3 100644
--- a/doc/guides/nics/features/failsafe.ini
+++ b/doc/guides/nics/features/failsafe.ini
@@ -15,7 +15,6 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Unicast MAC filter   = Y
-Multicast MAC filter = Y
 VLAN filter          = Y
 Flow control         = Y
 Flow API             = Y
-- 
2.17.1

[dpdk-dev] [PATCH v3 2/2] net/failsafe: support multicast address list set

[Andrew Rybchenko]

From: Evgeny Im <Evgeny.Im@oktetlabs.com>

Signed-off-by: Evgeny Im <Evgeny.Im@oktetlabs.com>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/nics/features/failsafe.ini   |  1 +
 doc/guides/rel_notes/release_18_11.rst  |  1 +
 drivers/net/failsafe/failsafe.c         |  1 +
 drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
 drivers/net/failsafe/failsafe_ops.c     | 50 +++++++++++++++++++++++++
 drivers/net/failsafe/failsafe_private.h |  2 +
 6 files changed, 72 insertions(+)

diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
index 89e253df3..e3c4c08f2 100644
--- a/doc/guides/nics/features/failsafe.ini
+++ b/doc/guides/nics/features/failsafe.ini
@@ -15,6 +15,7 @@ Jumbo frame          = Y
 Promiscuous mode     = Y
 Allmulticast mode    = Y
 Unicast MAC filter   = Y
+Multicast MAC filter = Y
 VLAN filter          = Y
 Flow control         = Y
 Flow API             = Y
diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index f39cb15d2..2f53564a9 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -79,6 +79,7 @@ New Features
   * Support for Rx and Tx queues start and stop.
   * Support for Rx and Tx queues deferred start.
   * Support for runtime Rx and Tx queues setup.
+  * Support multicast MAC address set.
 
 * **Added ability to switch queue deferred start flag on testpmd app.**
 
diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index 657919f93..c3999f026 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -304,6 +304,7 @@ fs_rte_eth_free(const char *name)
 	ret = pthread_mutex_destroy(&PRIV(dev)->hotplug_mutex);
 	if (ret)
 		ERROR("Error while destroying hotplug mutex");
+	rte_free(PRIV(dev)->mcast_addrs);
 	rte_free(PRIV(dev));
 	rte_eth_dev_release_port(dev);
 	return ret;
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 191f95f14..51c96f78b 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -179,6 +179,23 @@ fs_eth_dev_conf_apply(struct rte_eth_dev *dev,
 			return ret;
 		}
 	}
+	/*
+	 * Propagate multicast MAC addresses to sub-devices,
+	 * if non zero number of addresses is set.
+	 * The condition is required to avoid breakage of failsafe
+	 * for sub-devices which do not support the operation
+	 * if the feature is really not used.
+	 */
+	if (PRIV(dev)->nb_mcast_addr > 0) {
+		DEBUG("Configuring multicast MAC addresses");
+		ret = rte_eth_dev_set_mc_addr_list(PORT_ID(sdev),
+						   PRIV(dev)->mcast_addrs,
+						   PRIV(dev)->nb_mcast_addr);
+		if (ret) {
+			ERROR("Failed to apply multicast MAC addresses");
+			return ret;
+		}
+	}
 	/* VLAN filter */
 	vfc1 = &dev->data->vlan_filter_conf;
 	vfc2 = &edev->data->vlan_filter_conf;
diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index 7fadf06f5..86d7fa2a0 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -1126,6 +1126,55 @@ fs_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 	return 0;
 }
 
+static int
+fs_set_mc_addr_list(struct rte_eth_dev *dev,
+		    struct ether_addr *mc_addr_set, uint32_t nb_mc_addr)
+{
+	struct sub_device *sdev;
+	uint8_t i;
+	int ret;
+	void *mcast_addrs;
+
+	fs_lock(dev, 0);
+
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		ret = rte_eth_dev_set_mc_addr_list(PORT_ID(sdev),
+						   mc_addr_set, nb_mc_addr);
+		if (ret != 0) {
+			ERROR("Operation rte_eth_dev_set_mc_addr_list failed for sub_device %d with error %d",
+			      i, ret);
+			goto rollback;
+		}
+	}
+
+	mcast_addrs = rte_realloc(PRIV(dev)->mcast_addrs,
+		nb_mc_addr * sizeof(PRIV(dev)->mcast_addrs[0]), 0);
+	if (mcast_addrs == NULL && nb_mc_addr > 0) {
+		ret = -ENOMEM;
+		goto rollback;
+	}
+	rte_memcpy(mcast_addrs, mc_addr_set,
+		   nb_mc_addr * sizeof(PRIV(dev)->mcast_addrs[0]));
+	PRIV(dev)->nb_mcast_addr = nb_mc_addr;
+	PRIV(dev)->mcast_addrs = mcast_addrs;
+
+	fs_unlock(dev, 0);
+	return 0;
+
+rollback:
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		int rc = rte_eth_dev_set_mc_addr_list(PORT_ID(sdev),
+			PRIV(dev)->mcast_addrs,	PRIV(dev)->nb_mcast_addr);
+		if (rc != 0) {
+			ERROR("Multicast MAC address list rollback for sub_device %d failed with error %d",
+			      i, rc);
+		}
+	}
+
+	fs_unlock(dev, 0);
+	return ret;
+}
+
 static int
 fs_rss_hash_update(struct rte_eth_dev *dev,
 			struct rte_eth_rss_conf *rss_conf)
@@ -1214,6 +1263,7 @@ const struct eth_dev_ops failsafe_ops = {
 	.mac_addr_remove = fs_mac_addr_remove,
 	.mac_addr_add = fs_mac_addr_add,
 	.mac_addr_set = fs_mac_addr_set,
+	.set_mc_addr_list = fs_set_mc_addr_list,
 	.rss_hash_update = fs_rss_hash_update,
 	.filter_ctrl = fs_filter_ctrl,
 };
diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h
index 886af8616..abbe73e87 100644
--- a/drivers/net/failsafe/failsafe_private.h
+++ b/drivers/net/failsafe/failsafe_private.h
@@ -143,6 +143,8 @@ struct fs_priv {
 	uint32_t nb_mac_addr;
 	struct ether_addr mac_addrs[FAILSAFE_MAX_ETHADDR];
 	uint32_t mac_addr_pool[FAILSAFE_MAX_ETHADDR];
+	uint32_t nb_mcast_addr;
+	struct ether_addr *mcast_addrs;
 	/* current capabilities */
 	struct rte_eth_dev_info infos;
 	struct rte_eth_dev_owner my_owner; /* Unique owner. */
-- 
2.17.1

Re: [dpdk-dev] [PATCH v3 0/2] net/failsafe: support multicast MAC address set

[Gaëtan Rivet]

Hi,

Seems good, thanks.

Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>

On Fri, Sep 21, 2018 at 04:36:20PM +0100, Andrew Rybchenko wrote:
> v3:
>     - move apply on sync to fs_eth_dev_conf_apply() to apply to
>       a new subdevice only
>     - use ethdev API to apply to sub-device on sync
>     - remove unnecessary check the same pointer from the method
>       implementation in failsafe
> 
> v2:
>     - fix setting of zero addresses since rte_realloc() returns NULL
> 
> Evgeny Im (2):
>   net/failsafe: remove not supported multicast MAC filter
>   net/failsafe: support multicast address list set
> 
>  doc/guides/rel_notes/release_18_11.rst  |  7 ++++
>  drivers/net/failsafe/failsafe.c         |  1 +
>  drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
>  drivers/net/failsafe/failsafe_ops.c     | 50 +++++++++++++++++++++++++
>  drivers/net/failsafe/failsafe_private.h |  2 +
>  5 files changed, 77 insertions(+)
> 
> -- 
> 2.17.1
> 

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH v3 0/2] net/failsafe: support multicast MAC address set

[Stephen Hemminger]

On Fri, 21 Sep 2018 16:36:19 +0100
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> v3:
>     - move apply on sync to fs_eth_dev_conf_apply() to apply to
>       a new subdevice only
>     - use ethdev API to apply to sub-device on sync
>     - remove unnecessary check the same pointer from the method
>       implementation in failsafe
> 
> v2:
>     - fix setting of zero addresses since rte_realloc() returns NULL
> 
> Evgeny Im (2):
>   net/failsafe: remove not supported multicast MAC filter
>   net/failsafe: support multicast address list set
> 
>  doc/guides/rel_notes/release_18_11.rst  |  1 +
>  drivers/net/failsafe/failsafe.c         |  1 +
>  drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
>  drivers/net/failsafe/failsafe_ops.c     | 50 +++++++++++++++++++++++++
>  drivers/net/failsafe/failsafe_private.h |  2 +
>  5 files changed, 71 insertions(+)
> 

Since TAP has no filtering this doesn't really do much for the slowpath.

Re: [dpdk-dev] [PATCH v3 0/2] net/failsafe: support multicast MAC address set

[Andrew Rybchenko]

On 9/21/18 7:21 PM, Stephen Hemminger wrote:
> On Fri, 21 Sep 2018 16:36:19 +0100
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>
>> v3:
>>      - move apply on sync to fs_eth_dev_conf_apply() to apply to
>>        a new subdevice only
>>      - use ethdev API to apply to sub-device on sync
>>      - remove unnecessary check the same pointer from the method
>>        implementation in failsafe
>>
>> v2:
>>      - fix setting of zero addresses since rte_realloc() returns NULL
>>
>> Evgeny Im (2):
>>    net/failsafe: remove not supported multicast MAC filter
>>    net/failsafe: support multicast address list set
>>
>>   doc/guides/rel_notes/release_18_11.rst  |  1 +
>>   drivers/net/failsafe/failsafe.c         |  1 +
>>   drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
>>   drivers/net/failsafe/failsafe_ops.c     | 50 +++++++++++++++++++++++++
>>   drivers/net/failsafe/failsafe_private.h |  2 +
>>   5 files changed, 71 insertions(+)
>>
> Since TAP has no filtering this doesn't really do much for the slowpath.

Yes, I know that TAP does not support it. Is failsafe so tighly bound to 
TAP to block it?

Re: [dpdk-dev] [PATCH v3 0/2] net/failsafe: support multicast MAC address set

[Andrew Rybchenko]

On 9/21/18 7:33 PM, Andrew Rybchenko wrote:
> On 9/21/18 7:21 PM, Stephen Hemminger wrote:
>> On Fri, 21 Sep 2018 16:36:19 +0100
>> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>>
>>> v3:
>>>      - move apply on sync to fs_eth_dev_conf_apply() to apply to
>>>        a new subdevice only
>>>      - use ethdev API to apply to sub-device on sync
>>>      - remove unnecessary check the same pointer from the method
>>>        implementation in failsafe
>>>
>>> v2:
>>>      - fix setting of zero addresses since rte_realloc() returns NULL
>>>
>>> Evgeny Im (2):
>>>    net/failsafe: remove not supported multicast MAC filter
>>>    net/failsafe: support multicast address list set
>>>
>>>   doc/guides/rel_notes/release_18_11.rst  |  1 +
>>>   drivers/net/failsafe/failsafe.c         |  1 +
>>>   drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
>>>   drivers/net/failsafe/failsafe_ops.c     | 50 
>>> +++++++++++++++++++++++++
>>>   drivers/net/failsafe/failsafe_private.h |  2 +
>>>   5 files changed, 71 insertions(+)
>>>
>> Since TAP has no filtering this doesn't really do much for the slowpath.
>
> Yes, I know that TAP does not support it. Is failsafe so tighly bound 
> to TAP to block it?

In fact I've put it incorrectly. TAP supports multicast address set, but 
simply ignores
it since "every packet is received". But it is still makes sense for 
other failsafe
sub-devices which really support it.

Re: [dpdk-dev] [PATCH v3 0/2] net/failsafe: support multicast MAC address set

[Stephen Hemminger]

On Fri, 21 Sep 2018 19:33:16 +0300
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> On 9/21/18 7:21 PM, Stephen Hemminger wrote:
> > On Fri, 21 Sep 2018 16:36:19 +0100
> > Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> >  
> >> v3:
> >>      - move apply on sync to fs_eth_dev_conf_apply() to apply to
> >>        a new subdevice only
> >>      - use ethdev API to apply to sub-device on sync
> >>      - remove unnecessary check the same pointer from the method
> >>        implementation in failsafe
> >>
> >> v2:
> >>      - fix setting of zero addresses since rte_realloc() returns NULL
> >>
> >> Evgeny Im (2):
> >>    net/failsafe: remove not supported multicast MAC filter
> >>    net/failsafe: support multicast address list set
> >>
> >>   doc/guides/rel_notes/release_18_11.rst  |  1 +
> >>   drivers/net/failsafe/failsafe.c         |  1 +
> >>   drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
> >>   drivers/net/failsafe/failsafe_ops.c     | 50 +++++++++++++++++++++++++
> >>   drivers/net/failsafe/failsafe_private.h |  2 +
> >>   5 files changed, 71 insertions(+)
> >>  
> > Since TAP has no filtering this doesn't really do much for the slowpath.  
> 
> Yes, I know that TAP does not support it. Is failsafe so tighly bound to 
> TAP to block it?
> 

failsafe was built to be used with tap and mlx on Azure.
It could be used with virtio and other NIC on KVM, but this is mostly theoretical.

Re: [dpdk-dev] [PATCH v3 0/2] net/failsafe: support multicast MAC address set

[Ferruh Yigit]

On 9/21/2018 5:09 PM, Gaëtan Rivet wrote:
> Hi,
> 
> Seems good, thanks.
> 
> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Series applied to dpdk-next-net/master, thanks.

> 
> On Fri, Sep 21, 2018 at 04:36:20PM +0100, Andrew Rybchenko wrote:
>> v3:
>>     - move apply on sync to fs_eth_dev_conf_apply() to apply to
>>       a new subdevice only
>>     - use ethdev API to apply to sub-device on sync
>>     - remove unnecessary check the same pointer from the method
>>       implementation in failsafe
>>
>> v2:
>>     - fix setting of zero addresses since rte_realloc() returns NULL
>>
>> Evgeny Im (2):
>>   net/failsafe: remove not supported multicast MAC filter
>>   net/failsafe: support multicast address list set
>>
>>  doc/guides/rel_notes/release_18_11.rst  |  7 ++++
>>  drivers/net/failsafe/failsafe.c         |  1 +
>>  drivers/net/failsafe/failsafe_ether.c   | 17 +++++++++
>>  drivers/net/failsafe/failsafe_ops.c     | 50 +++++++++++++++++++++++++
>>  drivers/net/failsafe/failsafe_private.h |  2 +
>>  5 files changed, 77 insertions(+)
>>
>> -- 
>> 2.17.1
>>
>