DPDK patches and discussions
 help / color / mirror / Atom feed
From: Chas Williams <3chas3@gmail.com>
To: dev@dpdk.org
Cc: Chas Williams <chas3@att.com>, stable@dpdk.org
Subject: [dpdk-dev] [PATCH] net/bonding: fix invalid link status
Date: Thu, 14 Feb 2019 14:11:12 -0500	[thread overview]
Message-ID: <20190214191112.31018-1-3chas3@gmail.com> (raw)

From: Chas Williams <chas3@att.com>

Copying the link properties of the first slave added may copy an
invalid link status. The speed and duplex of the slave may not
be known at this time. Delay setting the properties until the
first slave reports as link up. Note that we are still ignoring
an error from link_properties_valid. For some bonding modes,
802.3ad, we should not activate the slave if it does not have
matching link properties.

Fixes: a45b288ef21a ("bond: support link status polling")
Cc: stable@dpdk.org
Signed-off-by: Chas Williams <chas3@att.com>
---
 drivers/net/bonding/rte_eth_bond_api.c     |  4 ---
 drivers/net/bonding/rte_eth_bond_pmd.c     | 31 +++++++++++++---------
 drivers/net/bonding/rte_eth_bond_private.h |  7 -----
 3 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index e5e146540..57ef2f001 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -484,10 +484,6 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
 			}
 		}
 
-		/* Inherit eth dev link properties from first slave */
-		link_properties_set(bonded_eth_dev,
-				&(slave_eth_dev->data->dev_link));
-
 		/* Make primary slave */
 		internals->primary_port = slave_port_id;
 		internals->current_primary_port = slave_port_id;
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 61e731a8f..c4a2b955c 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1449,7 +1449,7 @@ bond_ethdev_tx_burst_broadcast(void *queue, struct rte_mbuf **bufs,
 	return max_nb_of_tx_pkts;
 }
 
-void
+static void
 link_properties_set(struct rte_eth_dev *ethdev, struct rte_eth_link *slave_link)
 {
 	struct bond_dev_private *bond_ctx = ethdev->data->dev_private;
@@ -1474,7 +1474,7 @@ link_properties_set(struct rte_eth_dev *ethdev, struct rte_eth_link *slave_link)
 	}
 }
 
-int
+static int
 link_properties_valid(struct rte_eth_dev *ethdev,
 		struct rte_eth_link *slave_link)
 {
@@ -2693,16 +2693,6 @@ bond_ethdev_lsc_event_callback(uint16_t port_id, enum rte_eth_event_type type,
 		if (active_pos < internals->active_slave_count)
 			goto link_update;
 
-		/* if no active slave ports then set this port to be primary port */
-		if (internals->active_slave_count < 1) {
-			/* If first active slave, then change link status */
-			bonded_eth_dev->data->dev_link.link_status = ETH_LINK_UP;
-			internals->current_primary_port = port_id;
-			lsc_flag = 1;
-
-			mac_address_slaves_update(bonded_eth_dev);
-		}
-
 		/* check link state properties if bonded link is up*/
 		if (bonded_eth_dev->data->dev_link.link_status == ETH_LINK_UP) {
 			if (link_properties_valid(bonded_eth_dev, &link) != 0)
@@ -2714,9 +2704,24 @@ bond_ethdev_lsc_event_callback(uint16_t port_id, enum rte_eth_event_type type,
 			link_properties_set(bonded_eth_dev, &link);
 		}
 
+		/* If no active slave ports then set this port to be
+		 * the primary port.
+		 */
+		if (internals->active_slave_count < 1) {
+			/* If first active slave, then change link status */
+			bonded_eth_dev->data->dev_link.link_status =
+								ETH_LINK_UP;
+			internals->current_primary_port = port_id;
+			lsc_flag = 1;
+
+			mac_address_slaves_update(bonded_eth_dev);
+		}
+
 		activate_slave(bonded_eth_dev, port_id);
 
-		/* If user has defined the primary port then default to using it */
+		/* If the user has defined the primary port then default to
+		 * using it.
+		 */
 		if (internals->user_defined_primary_port &&
 				internals->primary_port == port_id)
 			bond_ethdev_primary_set(internals, port_id);
diff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h
index 3ea5d686b..032ffed02 100644
--- a/drivers/net/bonding/rte_eth_bond_private.h
+++ b/drivers/net/bonding/rte_eth_bond_private.h
@@ -222,13 +222,6 @@ deactivate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id);
 void
 activate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id);
 
-void
-link_properties_set(struct rte_eth_dev *bonded_eth_dev,
-		struct rte_eth_link *slave_dev_link);
-int
-link_properties_valid(struct rte_eth_dev *bonded_eth_dev,
-		struct rte_eth_link *slave_dev_link);
-
 int
 mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr);
 
-- 
2.17.2

             reply	other threads:[~2019-02-14 19:11 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-14 19:11 Chas Williams [this message]
2019-02-18 16:21 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190214191112.31018-1-3chas3@gmail.com \
    --to=3chas3@gmail.com \
    --cc=chas3@att.com \
    --cc=dev@dpdk.org \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).