[dpdk-dev] [PATCH 0/2] bonding fixes

[dpdk-dev] [PATCH 0/2] bonding fixes

[Bernard Iremonger]

These patches fix segmentation faults which were occurring when
slave devices were detached before being removed from the bonding
device.
The slave devices must now be removed from the bonding device
before they can be detached.
The bonding device cannot be detached now until all slave devices
have been removed from it.

Bernare Iremonger (2):
  bonding: fix detach of bonded device
  bonding: fix detach of bonded slave devices

 drivers/net/bonding/rte_eth_bond_api.c | 40 +++++++++++++++-------------------
 lib/librte_ether/rte_ethdev.c          |  8 +++++--
 lib/librte_ether/rte_ethdev.h          |  4 +++-
 3 files changed, 26 insertions(+), 26 deletions(-)

-- 
2.6.3

[dpdk-dev] [PATCH 1/2] bonding: fix detach of bonded device

[Bernard Iremonger]

Check that the bonded device has no slaves before detaching it.

Fixes: 8d30fe7fa737 ("bonding: support port hotplug")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 drivers/net/bonding/rte_eth_bond_api.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 8a000c8..484a6f3 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -277,6 +277,7 @@ int
 rte_eth_bond_free(const char *name)
 {
 	struct rte_eth_dev *eth_dev = NULL;
+	struct bond_dev_private *internals;
 
 	/* now free all data allocation - for eth_dev structure,
 	 * dummy pci driver and internal (private) data
@@ -287,6 +288,10 @@ rte_eth_bond_free(const char *name)
 	if (eth_dev == NULL)
 		return -ENODEV;
 
+	internals = eth_dev->data->dev_private;
+	if (internals->slave_count != 0)
+		return -EBUSY;
+
 	if (eth_dev->data->dev_started == 1) {
 		bond_ethdev_stop(eth_dev);
 		bond_ethdev_close(eth_dev);
-- 
2.6.3

[dpdk-dev] [PATCH 2/2] bonding: fix detach of bonded slave devices

[Bernard Iremonger]

Ensure that a bonded slave device is not detached,
until it is removed from the bonded device.

Fixes: 2efb58cbab6e ("bond: new link bonding library")
Fixes: a45b288ef21a ("bond: support link status polling")
Fixes: 494adb7f63f2 ("ethdev: add device fields from PCI layer")
Fixes: b1fb53a39d88 ("ethdev: remove some PCI specific handling")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 drivers/net/bonding/rte_eth_bond_api.c | 33 +++++++++++----------------------
 lib/librte_ether/rte_ethdev.c          |  8 ++++++--
 lib/librte_ether/rte_ethdev.h          |  4 +++-
 3 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 484a6f3..a0995ec 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -314,38 +314,23 @@ __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;
 	struct bond_dev_private *internals;
-	struct bond_dev_private *temp_internals;
 	struct rte_eth_link link_props;
 	struct rte_eth_dev_info dev_info;
 
-	int i, j;
-
 	if (valid_slave_port_id(slave_port_id) != 0)
 		return -1;
 
 	bonded_eth_dev = &rte_eth_devices[bonded_port_id];
 	internals = bonded_eth_dev->data->dev_private;
 
-	/* Verify that new slave device is not already a slave of another
-	 * bonded device */
-	for (i = rte_eth_dev_count()-1; i >= 0; i--) {
-		if (check_for_bonded_ethdev(&rte_eth_devices[i]) == 0) {
-			temp_internals = rte_eth_devices[i].data->dev_private;
-
-			for (j = 0; j < temp_internals->slave_count; j++) {
-				/* Device already a slave of a bonded device */
-				if (temp_internals->slaves[j].port_id == slave_port_id) {
-					RTE_BOND_LOG(ERR, "Slave port %d is already a slave",
-							slave_port_id);
-					return -1;
-				}
-			}
-		}
-	}
-
 	slave_eth_dev = &rte_eth_devices[slave_port_id];
+	if (slave_eth_dev->data->dev_flags & RTE_ETH_DEV_BONDED_SLAVE) {
+		RTE_BOND_LOG(ERR, "Slave device is already a slave of a bonded device");
+		return -1;
+	}
 
 	/* Add slave details to bonded device */
+	slave_eth_dev->data->dev_flags |= RTE_ETH_DEV_BONDED_SLAVE;
 	slave_add(internals, slave_eth_dev);
 
 	rte_eth_dev_info_get(slave_port_id, &dev_info);
@@ -385,6 +370,7 @@ __eth_bond_slave_add_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
 		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);
@@ -416,6 +402,7 @@ __eth_bond_slave_add_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
 
 	if (bonded_eth_dev->data->dev_started) {
 		if (slave_configure(bonded_eth_dev, slave_eth_dev) != 0) {
+			slave_eth_dev->data->dev_flags &= (~RTE_ETH_DEV_BONDED_SLAVE);
 			RTE_BOND_LOG(ERR, "rte_bond_slaves_configure: port=%d",
 					slave_port_id);
 			return -1;
@@ -468,7 +455,7 @@ __eth_bond_slave_remove_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
 {
 	struct rte_eth_dev *bonded_eth_dev;
 	struct bond_dev_private *internals;
-
+	struct rte_eth_dev *slave_eth_dev;
 	int i, slave_idx;
 
 	if (valid_slave_port_id(slave_port_id) != 0)
@@ -508,7 +495,9 @@ __eth_bond_slave_remove_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
 	mac_address_set(&rte_eth_devices[slave_port_id],
 			&(internals->slaves[slave_idx].persisted_mac_addr));
 
-	slave_remove(internals, &rte_eth_devices[slave_port_id]);
+	slave_eth_dev = &rte_eth_devices[slave_port_id];
+	slave_remove(internals, slave_eth_dev);
+	slave_eth_dev->data->dev_flags &= (~RTE_ETH_DEV_BONDED_SLAVE);
 
 	/*  first slave in the active list will be the primary by default,
 	 *  otherwise use first device in list */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 756b234..a6e83c1 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -495,7 +495,11 @@ rte_eth_dev_is_detachable(uint8_t port_id)
 		return -ENOTSUP;
 	}
 	dev_flags = rte_eth_devices[port_id].data->dev_flags;
-	return !(dev_flags & RTE_ETH_DEV_DETACHABLE);
+	if ((dev_flags & RTE_ETH_DEV_DETACHABLE) &&
+		(!(dev_flags & RTE_ETH_DEV_BONDED_SLAVE)))
+		return 0;
+	else
+		return 1;
 }
 
 /* attach the new physical device, then store port_id of the device */
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 8710dd7..66346a0 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1,7 +1,7 @@
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -1616,6 +1616,8 @@ struct rte_eth_dev_data {
 #define RTE_ETH_DEV_DETACHABLE   0x0001
 /** Device supports link state interrupt */
 #define RTE_ETH_DEV_INTR_LSC     0x0002
+/** Device is a bonded slave */
+#define RTE_ETH_DEV_BONDED_SLAVE 0x0004
 
 /**
  * @internal
-- 
2.6.3

Re: [dpdk-dev] [PATCH 0/2] bonding fixes

[Doherty, Declan]

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Wednesday, February 10, 2016 10:14 AM
> To: dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>
> Subject: [PATCH 0/2] bonding fixes
> 
> These patches fix segmentation faults which were occurring when
> slave devices were detached before being removed from the bonding
> device.
> The slave devices must now be removed from the bonding device
> before they can be detached.
> The bonding device cannot be detached now until all slave devices
> have been removed from it.
> 
> Bernare Iremonger (2):
>   bonding: fix detach of bonded device
>   bonding: fix detach of bonded slave devices
> 
>  drivers/net/bonding/rte_eth_bond_api.c | 40 +++++++++++++++-----------------
> --
>  lib/librte_ether/rte_ethdev.c          |  8 +++++--
>  lib/librte_ether/rte_ethdev.h          |  4 +++-
>  3 files changed, 26 insertions(+), 26 deletions(-)
> 
> --
> 2.6.3

Series Acked-by: Declan Doherty <declan.doherty@intel.com>

Re: [dpdk-dev] [PATCH 0/2] bonding fixes

[Bruce Richardson]

On Mon, Feb 22, 2016 at 01:20:18PM +0000, Doherty, Declan wrote:
> > -----Original Message-----
> > From: Iremonger, Bernard
> > Sent: Wednesday, February 10, 2016 10:14 AM
> > To: dev@dpdk.org
> > Cc: Doherty, Declan <declan.doherty@intel.com>; Iremonger, Bernard
> > <bernard.iremonger@intel.com>
> > Subject: [PATCH 0/2] bonding fixes
> > 
> > These patches fix segmentation faults which were occurring when
> > slave devices were detached before being removed from the bonding
> > device.
> > The slave devices must now be removed from the bonding device
> > before they can be detached.
> > The bonding device cannot be detached now until all slave devices
> > have been removed from it.
> > 
> > Bernare Iremonger (2):
> >   bonding: fix detach of bonded device
> >   bonding: fix detach of bonded slave devices
> > 
> >  drivers/net/bonding/rte_eth_bond_api.c | 40 +++++++++++++++-----------------
> > --
> >  lib/librte_ether/rte_ethdev.c          |  8 +++++--
> >  lib/librte_ether/rte_ethdev.h          |  4 +++-
> >  3 files changed, 26 insertions(+), 26 deletions(-)
> > 
> > --
> > 2.6.3
> 
> Series Acked-by: Declan Doherty <declan.doherty@intel.com>

Applied to dpdk-next-net/rel_16_04

/Bruce