From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 0B72D58F4 for ; Mon, 8 Dec 2014 08:10:58 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 07 Dec 2014 23:10:58 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,537,1413270000"; d="scan'208";a="634284633" Received: from pgsmsx108.gar.corp.intel.com ([10.221.44.103]) by fmsmga001.fm.intel.com with ESMTP; 07 Dec 2014 23:10:57 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by PGSMSX108.gar.corp.intel.com (10.221.44.103) with Microsoft SMTP Server (TLS) id 14.3.195.1; Mon, 8 Dec 2014 15:10:46 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.182]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.240]) with mapi id 14.03.0195.001; Mon, 8 Dec 2014 15:10:45 +0800 From: "Jiajia, SunX" To: "Doherty, Declan" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] bond: fix for mac assignment to slaves device Thread-Index: AQHQELH84+2v9D55/EWKSoYtT4RcUZyFSVpw Date: Mon, 8 Dec 2014 07:10:45 +0000 Message-ID: References: <1417800885-18643-1-git-send-email-declan.doherty@intel.com> In-Reply-To: <1417800885-18643-1-git-send-email-declan.doherty@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] bond: fix for mac assignment to slaves device X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Dec 2014 07:10:59 -0000 Tested-by: Jiajia, SunX - Tested Commit: 29d03f7aa33edc3292bf75730ec684dd4cbe5054 - OS: Fedora20 3.11.10-301.fc20.x86_64=20 - 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 [80= 86: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 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D Tester has 4 ports(portA--portD), and DUT has 4 ports(port0--port3),=20 then connect portA to port0, portB to port1, portC to port2, portD to port3= . - Case: Basic bonding--MAC Address Test Description:=20 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@dpdk.org] On Behalf Of Declan Doherty > Sent: Saturday, December 06, 2014 1:35 AM > To: dev@dpdk.org > Subject: [dpdk-dev] [PATCH] bond: fix for mac assignment to slaves > device >=20 > 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 >=20 > Changed removing slave logic to use memmove instead of memcpy to move > data within the same array, as this was corrupting the slave array. >=20 > Adding unit test to cover failing assignment scenarios >=20 > Signed-off-by: Declan Doherty > --- > 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(-) >=20 > 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) >=20 > mac_addr =3D (struct ether_addr *)slave_mac; > mac_addr->addr_bytes[ETHER_ADDR_LEN-1] =3D > - test_params->slave_port_ids[test_params- > >bonded_slave_count-1]; > + test_params->bonded_slave_count-1; >=20 > 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); >=20 > expected_mac_addr =3D (struct ether_addr *)&slave_mac; > - expected_mac_addr->addr_bytes[ETHER_ADDR_LEN-1] =3D > - test_params->slave_port_ids[i]; > + expected_mac_addr->addr_bytes[ETHER_ADDR_LEN-1] =3D i; >=20 > /* 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; > } >=20 > +#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 =3D 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 =3D 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] =3D i + 100; > + > + snprintf(pmd_name, RTE_ETH_NAME_MAX_LEN, "eth_slave_%d", i); > + > + slave_port_ids[i] =3D virtual_ethdev_create(pmd_name, > + &slave_mac_addr, rte_socket_id(), 1); > + > + TEST_ASSERT(slave_port_ids[i] >=3D 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 =3D 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 =3D 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] =3D 0xFF; > + bonded_mac_addr.addr_bytes[ETHER_ADDR_LEN-1] =3D 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 =3D 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] =3D 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] =3D 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] =3D 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] =3D 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 =3D 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 =3D 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] =3D 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] =3D 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] =3D 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; > +} > + >=20 > 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 =3D { > 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, >=20 > memcpy(eth_dev->data->mac_addrs, mac_addr, > sizeof(*eth_dev->data->mac_addrs)); > - eth_dev->data->mac_addrs->addr_bytes[5] =3D eth_dev->data->port_id; >=20 > eth_dev->data->dev_started =3D 0; > eth_dev->data->promiscuous =3D 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 =3D 0; >=20 > for (i =3D 0; i < internals->slave_count; i++) { > - if (internals->slaves[i].port_id =3D=3D slave_eth_dev->data- > >port_id) > + if (internals->slaves[i].port_id =3D=3D > + slave_eth_dev->data->port_id) > found =3D 1; >=20 > - 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; > + } > } >=20 > 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 =3D port_id; > lsc_flag =3D 1; >=20 > + 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