[dpdk-dev] [PATCH] bond: fix for mac assignment to slaves device
Jiajia, SunX
sunx.jiajia at intel.com
Mon Dec 8 08:10:45 CET 2014
Tested-by: Jiajia, SunX <sunx.jiajia at intel.com>
- Tested Commit: 29d03f7aa33edc3292bf75730ec684dd4cbe5054
- OS: Fedora20 3.11.10-301.fc20.x86_64
- GCC: gcc version 4.8.3
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection [8086:10fb]
- Default x86_64-native-linuxapp-gcc configuration
- Total 1 cases, 1 passed, 0 failed
TOPO:
* Connections ports between tester/ixia and DUT
- TESTER(Or IXIA)-------DUT
- portA------------------port0
- portB------------------port1
- portC------------------port2
- portD------------------port3
Test Setup#1 for Functional test
================================
Tester has 4 ports(portA--portD), and DUT has 4 ports(port0--port3),
then connect portA to port0, portB to port1, portC to port2, portD to port3.
- Case: Basic bonding--MAC Address Test
Description:
Use Setup#1.
Create bonded device and add some ports as slaves of bonded device,
Check that the changes of the bonded device and slave MAC
Expected test result:
Verify the behavior of bonded device and slave according to the mode.
> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Declan Doherty
> Sent: Saturday, December 06, 2014 1:35 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] bond: fix for mac assignment to slaves
> device
>
> 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 at intel.com>
> ---
> app/test/test_link_bonding.c | 192
> ++++++++++++++++++++++++++++++++-
> app/test/virtual_pmd.c | 1 -
> lib/librte_pmd_bond/rte_eth_bond_pmd.c | 14 ++-
> 3 files changed, 199 insertions(+), 8 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..1f2bd03 100644
> --- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> +++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> @@ -985,12 +985,16 @@ slave_remove(struct bond_dev_private *internals,
> int i, found = 0;
>
> for (i = 0; i < internals->slave_count; i++) {
> - if (internals->slaves[i].port_id == slave_eth_dev->data-
> >port_id)
> + if (internals->slaves[i].port_id ==
> + slave_eth_dev->data->port_id)
> found = 1;
>
> - if (found && i < (internals->slave_count - 1))
> - memcpy(&internals->slaves[i], &internals->slaves[i+1],
> - sizeof(internals->slaves[i]));
> + if (found && (i < (internals->slave_count - 1))) {
> + memmove(&internals->slaves[i], &internals-
> >slaves[i+1],
> + (sizeof(internals->slaves[0]) *
> + internals->slave_count - i - 1));
> + break;
> + }
> }
>
> internals->slave_count--;
> @@ -1501,6 +1505,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
More information about the dev
mailing list