[dpdk-dev] [PATCH 0/2] bonding link validation and vlan filters

[dpdk-dev] [PATCH 0/2] bonding link validation and vlan filters

[Eric Kinzie]

This series contains a fix to the validation of interfaces added to a
bond group and an enhancement to allow reception of tagged frames on
slave interfaces that are virtual functions.

Eric Kinzie (2):
  net/bonding: validate speed after link up
  net/bonding: enable slave VLAN filter

 drivers/net/bonding/rte_eth_bond_api.c     |   83 +++++++++++++++++++++++-----
 drivers/net/bonding/rte_eth_bond_pmd.c     |   46 +++++++++++++++
 drivers/net/bonding/rte_eth_bond_private.h |    4 ++
 3 files changed, 118 insertions(+), 15 deletions(-)

-- 
1.7.10.4

[dpdk-dev] [PATCH 1/2] net/bonding: validate speed after link up

[Eric Kinzie]

It's possible for the bonding driver to mistakenly reject an interface
based in it's, as yet, unnegotiated link speed and duplex.  Always allow
the interface to be added to the bonding interface but require link
properties validation to succeed before slave is activated.

Fixes: 2efb58cbab6e ("bond: new link bonding library")

Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
---
 drivers/net/bonding/rte_eth_bond_api.c |   15 ---------------
 drivers/net/bonding/rte_eth_bond_pmd.c |   10 ++++++++++
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 203ebe9..3c16973 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -373,21 +373,6 @@ __eth_bond_slave_add_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
 		internals->candidate_max_rx_pktlen = dev_info.max_rx_pktlen;
 
 	} else {
-		/* Check slave link properties are supported if props are set,
-		 * all slaves must be the same */
-		if (internals->link_props_set) {
-			if (link_properties_valid(&(bonded_eth_dev->data->dev_link),
-									  &(slave_eth_dev->data->dev_link))) {
-				slave_eth_dev->data->dev_flags &= (~RTE_ETH_DEV_BONDED_SLAVE);
-				RTE_BOND_LOG(ERR,
-						"Slave port %d link speed/duplex not supported",
-						slave_port_id);
-				return -1;
-			}
-		} else {
-			link_properties_set(bonded_eth_dev,
-					&(slave_eth_dev->data->dev_link));
-		}
 		internals->rx_offload_capa &= dev_info.rx_offload_capa;
 		internals->tx_offload_capa &= dev_info.tx_offload_capa;
 		internals->flow_type_rss_offloads &= dev_info.flow_type_rss_offloads;
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index b20a272..25fe00a 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1985,6 +1985,16 @@ bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
 			/* Inherit eth dev link properties from first active slave */
 			link_properties_set(bonded_eth_dev,
 					&(slave_eth_dev->data->dev_link));
+		} else {
+			if (link_properties_valid(
+				&bonded_eth_dev->data->dev_link, &link) != 0) {
+				slave_eth_dev->data->dev_flags &=
+					(~RTE_ETH_DEV_BONDED_SLAVE);
+				RTE_LOG(ERR, PMD,
+					"port %u invalid speed/duplex\n",
+					port_id);
+				return;
+			}
 		}
 
 		activate_slave(bonded_eth_dev, port_id);
-- 
1.7.10.4

[dpdk-dev] [PATCH 2/2] net/bonding: enable slave VLAN filter

[Eric Kinzie]

SR-IOV virtual functions cannot rely on promiscuous mode for the reception
of VLAN tagged frames.  Program the vlan filter for each slave when a
vlan is configured for the bonding master.

Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
---
 drivers/net/bonding/rte_eth_bond_api.c     |   68 ++++++++++++++++++++++++++++
 drivers/net/bonding/rte_eth_bond_pmd.c     |   36 +++++++++++++++
 drivers/net/bonding/rte_eth_bond_private.h |    4 ++
 3 files changed, 108 insertions(+)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 3c16973..a556f7b 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -166,6 +166,7 @@ rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
 {
 	struct bond_dev_private *internals = NULL;
 	struct rte_eth_dev *eth_dev = NULL;
+	uint32_t vlan_filter_bmp_size;
 
 	/* now do all data allocation - for eth_dev structure, dummy pci driver
 	 * and internal (private) data
@@ -260,6 +261,27 @@ rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
 		goto err;
 	}
 
+	vlan_filter_bmp_size =
+		rte_bitmap_get_memory_footprint(ETHER_MAX_VLAN_ID+1);
+	internals->vlan_filter_bmpmem = rte_malloc(name, vlan_filter_bmp_size,
+						   RTE_CACHE_LINE_SIZE);
+	if (internals->vlan_filter_bmpmem == NULL) {
+		RTE_BOND_LOG(ERR,
+			     "Failed to allocate vlan bitmap for bonded device %u\n",
+			     eth_dev->data->port_id);
+		goto err;
+	}
+
+	internals->vlan_filter_bmp = rte_bitmap_init(ETHER_MAX_VLAN_ID+1,
+			internals->vlan_filter_bmpmem, vlan_filter_bmp_size);
+	if (internals->vlan_filter_bmp == NULL) {
+		RTE_BOND_LOG(ERR,
+			     "Failed to init vlan bitmap for bonded device %u\n",
+			     eth_dev->data->port_id);
+		rte_free(internals->vlan_filter_bmpmem);
+		goto err;
+	}
+
 	return eth_dev->data->port_id;
 
 err:
@@ -299,6 +321,9 @@ rte_eth_bond_free(const char *name)
 	eth_dev->rx_pkt_burst = NULL;
 	eth_dev->tx_pkt_burst = NULL;
 
+	internals = eth_dev->data->dev_private;
+	rte_bitmap_free(internals->vlan_filter_bmp);
+	rte_free(internals->vlan_filter_bmpmem);
 	rte_free(eth_dev->data->dev_private);
 	rte_free(eth_dev->data->mac_addrs);
 
@@ -308,6 +333,46 @@ rte_eth_bond_free(const char *name)
 }
 
 static int
+slave_vlan_filter_set(uint8_t bonded_port_id, uint8_t slave_port_id)
+{
+	struct rte_eth_dev *bonded_eth_dev;
+	struct bond_dev_private *internals;
+	int found;
+	int res = 0;
+	uint64_t slab = 0;
+	uint32_t pos = 0;
+	uint16_t first;
+
+	bonded_eth_dev = &rte_eth_devices[bonded_port_id];
+	if (bonded_eth_dev->data->dev_conf.rxmode.hw_vlan_filter == 0)
+		return 0;
+
+	internals = bonded_eth_dev->data->dev_private;
+	found = rte_bitmap_scan(internals->vlan_filter_bmp, &pos, &slab);
+	first = pos;
+
+	if (!found)
+		return 0;
+
+	do {
+		uint32_t i;
+		uint64_t mask;
+
+		for (i = 0, mask = 1;
+		     i < RTE_BITMAP_SLAB_BIT_SIZE;
+		     i ++, mask <<= 1) {
+			if (unlikely(slab & mask))
+				res = rte_eth_dev_vlan_filter(slave_port_id,
+							      (uint16_t)pos, 1);
+		}
+		found = rte_bitmap_scan(internals->vlan_filter_bmp,
+					&pos, &slab);
+	} while (found && first != pos && res == 0);
+
+	return res;
+}
+
+static int
 __eth_bond_slave_add_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
 {
 	struct rte_eth_dev *bonded_eth_dev, *slave_eth_dev;
@@ -427,6 +492,9 @@ __eth_bond_slave_add_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
 				activate_slave(bonded_eth_dev, slave_port_id);
 		}
 	}
+
+	slave_vlan_filter_set(bonded_port_id, slave_port_id);
+
 	return 0;
 
 }
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 25fe00a..0b6caf6 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1335,6 +1335,9 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
 				bonded_eth_dev->data->dev_conf.rxmode.mq_mode;
 	}
 
+	slave_eth_dev->data->dev_conf.rxmode.hw_vlan_filter =
+			bonded_eth_dev->data->dev_conf.rxmode.hw_vlan_filter;
+
 	/* Configure device */
 	errval = rte_eth_dev_configure(slave_eth_dev->data->port_id,
 			bonded_eth_dev->data->nb_rx_queues,
@@ -1637,7 +1640,10 @@ bond_ethdev_stop(struct rte_eth_dev *eth_dev)
 void
 bond_ethdev_close(struct rte_eth_dev *dev)
 {
+	struct bond_dev_private *internals = dev->data->dev_private;
+
 	bond_ethdev_free_queues(dev);
+	rte_bitmap_reset(internals->vlan_filter_bmp);
 }
 
 /* forward declaration */
@@ -1667,6 +1673,35 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 }
 
 static int
+bond_ethdev_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
+{
+	int res;
+	uint8_t i;
+	struct bond_dev_private *internals = dev->data->dev_private;
+
+	/* don't do this while a slave is being added */
+	rte_spinlock_lock(&internals->lock);
+
+	if (on)
+		rte_bitmap_set(internals->vlan_filter_bmp, vlan_id);
+	else
+		rte_bitmap_clear(internals->vlan_filter_bmp, vlan_id);
+
+	for (i = 0; i < internals->slave_count; i++) {
+		uint8_t port_id = internals->slaves[i].port_id;
+
+		res = rte_eth_dev_vlan_filter(port_id, vlan_id, on);
+		if (res == ENOTSUP)
+			RTE_LOG(WARNING, PMD,
+				"Setting VLAN filter on slave port %u not supported.\n",
+				port_id);
+	}
+
+	rte_spinlock_unlock(&internals->lock);
+	return 0;
+}
+
+static int
 bond_ethdev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
 		uint16_t nb_rx_desc, unsigned int socket_id __rte_unused,
 		const struct rte_eth_rxconf *rx_conf, struct rte_mempool *mb_pool)
@@ -2171,6 +2206,7 @@ const struct eth_dev_ops default_dev_ops = {
 	.dev_close            = bond_ethdev_close,
 	.dev_configure        = bond_ethdev_configure,
 	.dev_infos_get        = bond_ethdev_info,
+	.vlan_filter_set      = bond_ethdev_vlan_filter_set,
 	.rx_queue_setup       = bond_ethdev_rx_queue_setup,
 	.tx_queue_setup       = bond_ethdev_tx_queue_setup,
 	.rx_queue_release     = bond_ethdev_rx_queue_release,
diff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h
index 2bdc9ef..d95d440 100644
--- a/drivers/net/bonding/rte_eth_bond_private.h
+++ b/drivers/net/bonding/rte_eth_bond_private.h
@@ -36,6 +36,7 @@
 
 #include <rte_ethdev.h>
 #include <rte_spinlock.h>
+#include <rte_bitmap.h>
 
 #include "rte_eth_bond.h"
 #include "rte_eth_bond_8023ad_private.h"
@@ -172,6 +173,9 @@ struct bond_dev_private {
 
 	uint32_t candidate_max_rx_pktlen;
 	uint32_t max_rx_pktlen;
+
+	void *vlan_filter_bmpmem;		/* enabled vlan filter bitmap */
+	struct rte_bitmap *vlan_filter_bmp;
 };
 
 extern const struct eth_dev_ops default_dev_ops;
-- 
1.7.10.4

Re: [dpdk-dev] [PATCH 0/2] bonding link validation and vlan filters

[Ferruh Yigit]

On 8/4/2016 7:24 PM, Eric Kinzie wrote:
> This series contains a fix to the validation of interfaces added to a
> bond group and an enhancement to allow reception of tagged frames on
> slave interfaces that are virtual functions.
> 
> Eric Kinzie (2):
>   net/bonding: validate speed after link up
>   net/bonding: enable slave VLAN filter
> 
>  drivers/net/bonding/rte_eth_bond_api.c     |   83 +++++++++++++++++++++++-----
>  drivers/net/bonding/rte_eth_bond_pmd.c     |   46 +++++++++++++++
>  drivers/net/bonding/rte_eth_bond_private.h |    4 ++
>  3 files changed, 118 insertions(+), 15 deletions(-)
> 

+cc bonding maintainer, can you please review the patch?

Re: [dpdk-dev] [PATCH 1/2] net/bonding: validate speed after link up

[Iremonger, Bernard]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Eric Kinzie
> Sent: Thursday, August 4, 2016 7:25 PM
> To: dev@dpdk.org
> Cc: Jan Blunck <jblunck@infradead.org>
> Subject: [dpdk-dev] [PATCH 1/2] net/bonding: validate speed after link up
> 
> It's possible for the bonding driver to mistakenly reject an interface based in
> it's, as yet, unnegotiated link speed and duplex.  Always allow the interface
> to be added to the bonding interface but require link properties validation to
> succeed before slave is activated.
> 
> Fixes: 2efb58cbab6e ("bond: new link bonding library")
> 
> Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

Re: [dpdk-dev] [PATCH 2/2] net/bonding: enable slave VLAN filter

[Iremonger, Bernard]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Eric Kinzie
> Sent: Thursday, August 4, 2016 7:25 PM
> To: dev@dpdk.org
> Cc: Jan Blunck <jblunck@infradead.org>
> Subject: [dpdk-dev] [PATCH 2/2] net/bonding: enable slave VLAN filter
> 
> SR-IOV virtual functions cannot rely on promiscuous mode for the reception
> of VLAN tagged frames.  Program the vlan filter for each slave when a vlan is
> configured for the bonding master.
> 
> Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

Re: [dpdk-dev] [PATCH 1/2] net/bonding: validate speed after link up

[Bruce Richardson]

On Wed, Oct 05, 2016 at 12:53:00PM +0000, Iremonger, Bernard wrote:
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Eric Kinzie
> > Sent: Thursday, August 4, 2016 7:25 PM
> > To: dev@dpdk.org
> > Cc: Jan Blunck <jblunck@infradead.org>
> > Subject: [dpdk-dev] [PATCH 1/2] net/bonding: validate speed after link up
> > 
> > It's possible for the bonding driver to mistakenly reject an interface based in
> > it's, as yet, unnegotiated link speed and duplex.  Always allow the interface
> > to be added to the bonding interface but require link properties validation to
> > succeed before slave is activated.
> > 
> > Fixes: 2efb58cbab6e ("bond: new link bonding library")
> > 
> > Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
> 
> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 
Series applied to dpdk-next-net/rel_16_11

/Bruce