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

Message ID 1470335083-6380-2-git-send-email-ehkinzie@gmail.com (mailing list archive)
State Accepted, archived
Delegated to: Bruce Richardson
Headers

Commit Message

Eric Kinzie Aug. 4, 2016, 6:24 p.m. UTC
  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(-)
  

Comments

Iremonger, Bernard Oct. 5, 2016, 12:53 p.m. UTC | #1
> -----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>
  
Bruce Richardson Oct. 12, 2016, 3:34 p.m. UTC | #2
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
  

Patch

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