From: Declan Doherty <declan.doherty@intel.com>
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v2] bond: fix for mac assignment to slaves device
Date: Mon, 8 Dec 2014 11:19:32 +0000 [thread overview]
Message-ID: <1418037572-6546-1-git-send-email-declan.doherty@intel.com> (raw)
In-Reply-To: <1417800885-18643-1-git-send-email-declan.doherty@intel.com>
-V2:
Tidies up the slave_remove function as per Pawel's comments.
Adding call to mac_address_slaves_update from the lsc handler when the
first slave become active to propagate any mac changes made while
devices are inactive
Changed removing slave logic to use memmove instead of memcpy to move
data within the same array, as this was corrupting the slave array.
Adding unit test to cover failing assignment scenarios
Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
app/test/test_link_bonding.c | 192 ++++++++++++++++++++++++++++++++-
app/test/virtual_pmd.c | 1 -
lib/librte_pmd_bond/rte_eth_bond_pmd.c | 19 ++--
3 files changed, 200 insertions(+), 12 deletions(-)
diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index f62c490..4523de6 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -454,7 +454,7 @@ test_remove_slave_from_bonded_device(void)
mac_addr = (struct ether_addr *)slave_mac;
mac_addr->addr_bytes[ETHER_ADDR_LEN-1] =
- test_params->slave_port_ids[test_params->bonded_slave_count-1];
+ test_params->bonded_slave_count-1;
rte_eth_macaddr_get(
test_params->slave_port_ids[test_params->bonded_slave_count-1],
@@ -810,8 +810,7 @@ test_set_primary_slave(void)
test_params->bonded_port_id);
expected_mac_addr = (struct ether_addr *)&slave_mac;
- expected_mac_addr->addr_bytes[ETHER_ADDR_LEN-1] =
- test_params->slave_port_ids[i];
+ expected_mac_addr->addr_bytes[ETHER_ADDR_LEN-1] = i;
/* Check primary slave MAC */
rte_eth_macaddr_get(test_params->slave_port_ids[i], &read_mac_addr);
@@ -928,6 +927,192 @@ test_set_explicit_bonded_mac(void)
return 0;
}
+#define BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT (3)
+
+static int
+test_set_bonded_port_initialization_mac_assignment(void)
+{
+ int i, slave_count, bonded_port_id;
+
+ uint8_t slaves[RTE_MAX_ETHPORTS];
+ int slave_port_ids[BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT];
+
+ struct ether_addr slave_mac_addr, bonded_mac_addr, read_mac_addr;
+
+ /* Initialize default values for MAC addresses */
+ memcpy(&slave_mac_addr, slave_mac, sizeof(struct ether_addr));
+ memcpy(&bonded_mac_addr, slave_mac, sizeof(struct ether_addr));
+
+ /*
+ * 1. a - Create / configure bonded / slave ethdevs
+ */
+ bonded_port_id = rte_eth_bond_create("ethdev_bond_mac_ass_test",
+ BONDING_MODE_ACTIVE_BACKUP, rte_socket_id());
+ TEST_ASSERT(bonded_port_id > 0, "failed to create bonded device");
+
+ TEST_ASSERT_SUCCESS(configure_ethdev(bonded_port_id, 0, 0),
+ "Failed to configure bonded ethdev");
+
+ for (i = 0; i < BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT; i++) {
+ char pmd_name[RTE_ETH_NAME_MAX_LEN];
+
+ slave_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = i + 100;
+
+ snprintf(pmd_name, RTE_ETH_NAME_MAX_LEN, "eth_slave_%d", i);
+
+ slave_port_ids[i] = virtual_ethdev_create(pmd_name,
+ &slave_mac_addr, rte_socket_id(), 1);
+
+ TEST_ASSERT(slave_port_ids[i] >= 0,
+ "Failed to create slave ethdev %s", pmd_name);
+
+ TEST_ASSERT_SUCCESS(configure_ethdev(slave_port_ids[i], 1, 0),
+ "Failed to configure virtual ethdev %s",
+ pmd_name);
+ }
+
+
+ /*
+ * 2. Add slave ethdevs to bonded device
+ */
+ for (i = 0; i < BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT; i++) {
+ TEST_ASSERT_SUCCESS(rte_eth_bond_slave_add(bonded_port_id,
+ slave_port_ids[i]),
+ "Failed to add slave (%d) to bonded port (%d).",
+ slave_port_ids[i], bonded_port_id);
+ }
+
+ slave_count = rte_eth_bond_slaves_get(bonded_port_id, slaves,
+ RTE_MAX_ETHPORTS);
+ TEST_ASSERT_EQUAL(BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT, slave_count,
+ "Number of slaves (%d) is not as expected (%d)",
+ slave_count, BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT);
+
+
+ /*
+ * 3. Set explicit MAC address on bonded ethdev
+ */
+ bonded_mac_addr.addr_bytes[ETHER_ADDR_LEN-2] = 0xFF;
+ bonded_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = 0xAA;
+
+ TEST_ASSERT_SUCCESS(rte_eth_bond_mac_address_set(
+ bonded_port_id, &bonded_mac_addr),
+ "Failed to set MAC address on bonded port (%d)",
+ bonded_port_id);
+
+
+ /* 4. a - Start bonded ethdev
+ * b - Enable slave devices
+ * c - Verify bonded/slaves ethdev MAC addresses
+ */
+ TEST_ASSERT_SUCCESS(rte_eth_dev_start(bonded_port_id),
+ "Failed to start bonded pmd eth device %d.",
+ bonded_port_id);
+
+ for (i = 0; i < BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT; i++) {
+ virtual_ethdev_simulate_link_status_interrupt(
+ slave_port_ids[i], 1);
+ }
+
+ rte_eth_macaddr_get(bonded_port_id, &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&bonded_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "bonded port mac address not as expected");
+
+ rte_eth_macaddr_get(slave_port_ids[0], &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&bonded_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "slave port 0 mac address not as expected");
+
+ slave_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = 1 + 100;
+ rte_eth_macaddr_get(slave_port_ids[1], &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&slave_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "slave port 1 mac address not as expected");
+
+ slave_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = 2 + 100;
+ rte_eth_macaddr_get(slave_port_ids[2], &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&slave_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "slave port 2 mac address not as expected");
+
+
+ /* 7. a - Change primary port
+ * b - Stop / Start bonded port
+ * d - Verify slave ethdev MAC addresses
+ */
+ TEST_ASSERT_SUCCESS(rte_eth_bond_primary_set(bonded_port_id,
+ slave_port_ids[2]),
+ "failed to set primary port on bonded device.");
+
+ rte_eth_dev_stop(bonded_port_id);
+ TEST_ASSERT_SUCCESS(rte_eth_dev_start(bonded_port_id),
+ "Failed to start bonded pmd eth device %d.",
+ bonded_port_id);
+
+ rte_eth_macaddr_get(bonded_port_id, &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&bonded_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "bonded port mac address not as expected");
+
+ slave_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = 0 + 100;
+ rte_eth_macaddr_get(slave_port_ids[0], &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&slave_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "slave port 0 mac address not as expected");
+
+ slave_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = 1 + 100;
+ rte_eth_macaddr_get(slave_port_ids[1], &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&slave_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "slave port 1 mac address not as expected");
+
+ rte_eth_macaddr_get(slave_port_ids[2], &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&bonded_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "slave port 2 mac address not as expected");
+
+ /* 6. a - Stop bonded ethdev
+ * b - remove slave ethdevs
+ * c - Verify slave ethdevs MACs are restored
+ */
+ rte_eth_dev_stop(bonded_port_id);
+
+ for (i = 0; i < BONDED_INIT_MAC_ASSIGNMENT_SLAVE_COUNT; i++) {
+ TEST_ASSERT_SUCCESS(rte_eth_bond_slave_remove(bonded_port_id,
+ slave_port_ids[i]),
+ "Failed to remove slave %d from bonded port (%d).",
+ slave_port_ids[i], bonded_port_id);
+ }
+
+ slave_count = rte_eth_bond_slaves_get(bonded_port_id, slaves,
+ RTE_MAX_ETHPORTS);
+
+ TEST_ASSERT_EQUAL(slave_count, 0,
+ "Number of slaves (%d) is great than expected (%d).",
+ slave_count, 0);
+
+ slave_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = 0 + 100;
+ rte_eth_macaddr_get(slave_port_ids[0], &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&slave_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "slave port 0 mac address not as expected");
+
+ slave_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = 1 + 100;
+ rte_eth_macaddr_get(slave_port_ids[1], &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&slave_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "slave port 1 mac address not as expected");
+
+ slave_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] = 2 + 100;
+ rte_eth_macaddr_get(slave_port_ids[2], &read_mac_addr);
+ TEST_ASSERT_SUCCESS(memcmp(&slave_mac_addr, &read_mac_addr,
+ sizeof(read_mac_addr)),
+ "slave port 2 mac address not as expected");
+
+ return 0;
+}
+
static int
initialize_bonded_device_with_slaves(uint8_t bonding_mode, uint8_t bond_en_isr,
@@ -4368,6 +4553,7 @@ static struct unit_test_suite link_bonding_test_suite = {
TEST_CASE(test_set_bonding_mode),
TEST_CASE(test_set_primary_slave),
TEST_CASE(test_set_explicit_bonded_mac),
+ TEST_CASE(test_set_bonded_port_initialization_mac_assignment),
TEST_CASE(test_status_interrupt),
TEST_CASE(test_adding_slave_after_bonded_device_started),
TEST_CASE(test_roundrobin_tx_burst),
diff --git a/app/test/virtual_pmd.c b/app/test/virtual_pmd.c
index ade6cb0..9fac95d 100644
--- a/app/test/virtual_pmd.c
+++ b/app/test/virtual_pmd.c
@@ -588,7 +588,6 @@ virtual_ethdev_create(const char *name, struct ether_addr *mac_addr,
memcpy(eth_dev->data->mac_addrs, mac_addr,
sizeof(*eth_dev->data->mac_addrs));
- eth_dev->data->mac_addrs->addr_bytes[5] = eth_dev->data->port_id;
eth_dev->data->dev_started = 0;
eth_dev->data->promiscuous = 0;
diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index 539baa4..3db473b 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -982,16 +982,17 @@ void
slave_remove(struct bond_dev_private *internals,
struct rte_eth_dev *slave_eth_dev)
{
- int i, found = 0;
+ uint8_t i;
- for (i = 0; i < internals->slave_count; i++) {
- if (internals->slaves[i].port_id == slave_eth_dev->data->port_id)
- found = 1;
+ for (i = 0; i < internals->slave_count; i++)
+ if (internals->slaves[i].port_id ==
+ slave_eth_dev->data->port_id)
+ break;
- if (found && i < (internals->slave_count - 1))
- memcpy(&internals->slaves[i], &internals->slaves[i+1],
- sizeof(internals->slaves[i]));
- }
+ if (i < (internals->slave_count - 1))
+ memmove(&internals->slaves[i], &internals->slaves[i + 1],
+ sizeof(internals->slaves[0]) *
+ (internals->slave_count - i - 1));
internals->slave_count--;
}
@@ -1501,6 +1502,8 @@ bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
internals->current_primary_port = port_id;
lsc_flag = 1;
+ mac_address_slaves_update(bonded_eth_dev);
+
/* Inherit eth dev link properties from first active slave */
link_properties_set(bonded_eth_dev,
&(slave_eth_dev->data->dev_link));
--
1.7.12.2
next prev parent reply other threads:[~2014-12-08 11:19 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-05 17:34 [dpdk-dev] [PATCH] " Declan Doherty
2014-12-08 7:10 ` Jiajia, SunX
2014-12-08 9:21 ` Wodkowski, PawelX
2014-12-08 9:31 ` Wodkowski, PawelX
2014-12-08 11:19 ` Declan Doherty [this message]
2014-12-08 14:54 ` [dpdk-dev] [PATCH v2] " Wodkowski, PawelX
2014-12-11 1:01 ` Thomas Monjalon
2014-12-09 5:43 ` Jiajia, SunX
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=1418037572-6546-1-git-send-email-declan.doherty@intel.com \
--to=declan.doherty@intel.com \
--cc=dev@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).