[dpdk-dev] [RFC 0/4] Use Google Test as DPDK unit test framework
> -Original Message- ... > You are not advocating but the unit test must be written in C++. > I don't think it is a good idea to force people to write and maintain the > tests > in a different language than the code it tests. I know where you are coming from on this point, and I general would agree if it were not for the advantages you get from C++ test framework. Having worked with multiple C and C++ frameworks, I've found that one of the biggest advantages of the C++ frameworks is the amount of boilerplate code they can save you from writing. Also nearly all of C frameworks I've used make use macros to the point that they look more like objective C than C. In general I feel that even if the test code is written in C++ the code itself should be simple enough that someone with even a passing knowledge of C++ could easily understand the intent of the test code. > > Some of the major advantages of google test that I see over continuing to > > use > the > > current test include giving a consist feel to all tests, a powerful test > > execution framework which allow individual test suites or tests to be > > specified > > from the command line, support for a standard xunit output which can be > integrated > > into a continuous build systems, and a very powerful mocking library > > which allows much more control over testing failure conditions. > > It would be interesting to better describe in details what is missing > currently > and what such a framework can bring. > (I agree there is a huge room for improvements on unit tests) Some of the things I've come across include: No standard output format to integrated with continuous regression systems No ability to specify specific unit tests or groups of tests to run from the command line No standard set of test assertions used across the test suites. No standard setup and teardown functions across test suites, state from previous test suite can break current Requirement to use a python script to orchestrate test runs. No support for mocking functionality. I know that none of the above couldn't be fixed in our current test application, but I would question if it is effort worthwhile when we take an off the shelf framework, which does all those things and a whole lot more, which has been test and used in a huge variety of projects. I certainly willing to look at other frameworks both C and C++ but I yet to find a C framework which come close to the usability and flexibility of the popular C++ ones.
[dpdk-dev] [PATCH] bonding: fix bond link detect in non-interrupt mode
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of John Daley > Sent: Saturday, March 26, 2016 12:45 AM > To: dev at dpdk.org > Cc: Nelson Escobar ; John Daley > Subject: [dpdk-dev] [PATCH] bonding: fix bond link detect in non-interrupt > mode > > From: Nelson Escobar > > Stopping then re-starting a bond interface containing slaves that > used polling for link detection caused the bond to think all slave > links were down and inactive. > > Move the start of the polling for link from slave_add() to > bond_ethdev_start() and in bond_ethdev_stop() make sure we clear > the last_link_status of the slaves. > > Signed-off-by: Nelson Escobar > Signed-off-by: John Daley > --- . > -- > 2.7.0 Acked-by: Declan Doherty
[dpdk-dev] [PATCH 0/2] bonding fixes
> -Original Message- > From: Iremonger, Bernard > Sent: Wednesday, February 10, 2016 10:14 AM > To: dev at dpdk.org > Cc: Doherty, Declan ; Iremonger, Bernard > > Subject: [PATCH 0/2] bonding fixes > > These patches fix segmentation faults which were occurring when > slave devices were detached before being removed from the bonding > device. > The slave devices must now be removed from the bonding device > before they can be detached. > The bonding device cannot be detached now until all slave devices > have been removed from it. > > Bernare Iremonger (2): > bonding: fix detach of bonded device > bonding: fix detach of bonded slave devices > > drivers/net/bonding/rte_eth_bond_api.c | 40 +++- > -- > lib/librte_ether/rte_ethdev.c | 8 +-- > lib/librte_ether/rte_ethdev.h | 4 +++- > 3 files changed, 26 insertions(+), 26 deletions(-) > > -- > 2.6.3 Series Acked-by: Declan Doherty
[dpdk-dev] [PATCH] cryptodev: mark experimental state
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, November 26, 2015 10:09 AM > To: Panu Matilainen; Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] cryptodev: mark experimental state > > 2015-11-26 10:00, Panu Matilainen: > > On 11/26/2015 09:39 AM, Panu Matilainen wrote: > > > On 11/25/2015 07:38 PM, Thomas Monjalon wrote: > > >> --- a/config/common_linuxapp > > >> +++ b/config/common_linuxapp > > >> @@ -319,6 +319,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y > > >> > > >> # > > >> # Compile generic crypto device library > > >> +# EXPERIMENTAL: API may change without prior notice > > >> # > > >> CONFIG_RTE_LIBRTE_CRYPTODEV=y > > >> CONFIG_RTE_LIBRTE_CRYPTODEV_DEBUG=n > > > [...] > > > > > > I think an experimental library which declares itself exempt from the > > > ABI policy should not be compiled by default. That way anybody wanting > > > to try it out will be forced to notice the experimental status. > > > > > > More generally / longer term, perhaps there should be a > > > CONFIG_RTE_EXPERIMENTAL which wraps all experimental features and > > > defaults to off. > > > > On a related note, librte_mbuf_offload cannot be built if > > CONFIG_RTE_LIBRTE_CRYPTODEV is disabled. Which seems to suggest its (at > > least currently) so tightly couple to cryptodev that perhaps it too > > should be marked experimental and default to off. > > I think you are right. > Declan, what is your opinion? Hey Thomas, yes librte_mbuf_offload should also be set as experimental, it's probably one of the areas which will most likely change in the future. On the issue of turning off experimental libraries in the build by default, my preference would be not to turn them off unless the library has external dependencies, otherwise the possibility of patches being submitted which could break an experimental library will be much higher. In my opinion the fewer build configurations developers have to test against the better.
[dpdk-dev] [PATCH] bond: fix check initial link status of slave
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tomasz Kulasek > Sent: Wednesday, June 24, 2015 1:51 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH] bond: fix check initial link status of slave > > On Fortville NIC, link status change interrupt callback is not executed when > slave in bonding is (re-)started. It causes that slave's NIC is inactive even > if its link status is up on the start. > > This patch invokes lsc callback, just after port's start, to check its initial > link status and manage properly. > > Signed-off-by: Tomasz Kulasek > --- > drivers/net/bonding/rte_eth_bond_pmd.c |5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c > b/drivers/net/bonding/rte_eth_bond_pmd.c > index 8bad2e1..277b310 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -1357,6 +1357,11 @@ slave_configure(struct rte_eth_dev > *bonded_eth_dev, > return -1; > } > > + /* If lsc interrupt is set, check initial slave's link status */ > + if (slave_eth_dev->driver->pci_drv.drv_flags & RTE_PCI_DRV_INTR_LSC) > + bond_ethdev_lsc_event_callback(slave_eth_dev->data->port_id, > + RTE_ETH_EVENT_INTR_LSC, _eth_dev- > >data->port_id); > + > return 0; > } > > -- > 1.7.9.5 Acked-by : Declan Doherty
[dpdk-dev] [PATCH] Make bond_ethdev_stop iterate only over active slaves
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jan Blunck > Sent: Wednesday, June 24, 2015 2:01 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH] Make bond_ethdev_stop iterate only over active > slaves > > When stopping the bond device we don't need to try and free up the LACPDU's > from deactivated devices since this is covered by > bond_mode_8023ad_deactivate_slave(). > > This fixes the following: > [0.100569] PANIC in bond_ethdev_stop(): > [0.100589] line 1172assert "port->rx_ring != NULL" failed > > Signed-off-by: Jan Blunck > --- > drivers/net/bonding/rte_eth_bond_pmd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c > b/drivers/net/bonding/rte_eth_bond_pmd.c > index 5a2fbef..1fd1321 100644 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > @@ -1520,8 +1520,8 @@ bond_ethdev_stop(struct rte_eth_dev *eth_dev) > bond_mode_8023ad_stop(eth_dev); > > /* Discard all messages to/from mode 4 state machines */ > - for (i = 0; i < internals->slave_count; i++) { > - port = _8023ad_ports[internals->slaves[i].port_id]; > + for (i = 0; i < internals->active_slave_count; i++) { > + port = _8023ad_ports[internals->active_slaves[i]]; > > RTE_VERIFY(port->rx_ring != NULL); > while (rte_ring_dequeue(port->rx_ring, ) != - > ENOENT) > -- > 2.1.4 Acked-by : Declan Doherty
[dpdk-dev] [PATCH v2 0/8] Dynamic RSS Configuration for Bonding
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tomasz Kulasek > Sent: Friday, June 19, 2015 7:13 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v2 0/8] Dynamic RSS Configuration for Bonding > > OVERVIEW > > 1) Setting .rxmode.mq_mode for bonding device to ETH_MQ_RX_RSS makes > bonding > device fully RSS-capable, so all slaves are synchronized with its > configuration. > This mode is intended to provide RSS configuration as known from "dynamic RSS > configuration for one port" and made slaves transparent for client application > implementation. > > 2) If .rxmode.mq_mode for bonding device isn't ETH_MQ_RX_RSS, slaves are not > synchronized. That provides an ability to configure them manually. This mode > may > be useful when application wants to manage RSS in an unusual way and the > consistency of RSS configuration for slaves isn't required. > > Turning on/off RSS mode for slaves when bonding is started is not possible. > Other RSS configuration is propagated over slaves, when bonding device API is > used to do it. > > v2 changes: > - added support for keys other than 40 bytes long, > - now, if RSS key is not set for bonding, it is not set also for slaves, > - fix - full initial RSS configuration before any slave is added was not >possible due to the initially zeroed flow_type_rss_offloads for bonding, > - fix - changed error to warning when slave is synchronizing due to the >bonding's initial configuration (to allow use slaves' drivers not > supporting >dynamic RSS configuration in bonding), > - some code cleanups, > - updated documentation, > > Tomasz Kulasek (8): > bond: rss dynamic configuration > ring: dynamic rss configuration > test: dynamic rss configuration > bond: queue stats mapping > ring: queue stats mapping set dummy implementation > examples: dynamic rss configuration for bonding > doc: fixed spellings and typos > doc: dynamic rss configuration for bonding > > app/test/Makefile |1 + > app/test/test_link_bonding_rssconf.c | 674 ++ > .../prog_guide/link_bonding_poll_mode_drv_lib.rst | 42 +- > drivers/net/bonding/rte_eth_bond_api.c | 27 + > drivers/net/bonding/rte_eth_bond_pmd.c | 239 - > drivers/net/bonding/rte_eth_bond_private.h | 12 + > drivers/net/ring/rte_eth_ring.c| 133 ++- > examples/bond_rss/Makefile | 59 ++ > examples/bond_rss/bondrss.c| 293 +++ > examples/bond_rss/bondrss.h| 163 > examples/bond_rss/config.c | 251 ++ > examples/bond_rss/ui.c | 920 > > 12 files changed, 2789 insertions(+), 25 deletions(-) > create mode 100644 app/test/test_link_bonding_rssconf.c > create mode 100644 examples/bond_rss/Makefile > create mode 100644 examples/bond_rss/bondrss.c > create mode 100644 examples/bond_rss/bondrss.h > create mode 100644 examples/bond_rss/config.c > create mode 100644 examples/bond_rss/ui.c > > -- > 1.7.9.5 Acked-by : Declan Doherty
[dpdk-dev] [PATCH v2 0/3] PMD ring MAC management, fix initialization, link up/down
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tomasz Kulasek > Sent: Monday, January 19, 2015 11:57 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v2 0/3] PMD ring MAC management, fix > initialization, > link up/down > > Patch split into smaller parts to separate features from previous version. > > Tomasz Kulasek (3): > PMD Ring - Add link up/down functions > PMD Ring - Add MAC addr add/remove functions > PMD Ring - Fix for per device management > > lib/librte_pmd_ring/rte_eth_ring.c | 62 > +--- > 1 file changed, 57 insertions(+), 5 deletions(-) > > -- > 1.7.9.5 Acked-by: Declan Doherty
[dpdk-dev] rte_eal_init() causes PANIC using latest DPDK code.
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tetsuya Mukawa > Sent: Monday, January 19, 2015 7:11 AM > To: dev at dpdk.org > Subject: [dpdk-dev] rte_eal_init() causes PANIC using latest DPDK code. > > Hi, > > I've got following error using latest DPDK code. > Does someone face the same issue? > In my environment, It seems almost all DPDK apps (at lease l2fwd and > testpmd) cannot work. > > > EAL: Support maximum 128 logical core(s) by configuration. > EAL: Detected 8 lcore(s) > EAL: cannot open VFIO container, error 2 (No such file or directory) > EAL: VFIO support could not be initialized > EAL: Setting up memory... > EAL: Ask a virtual area of 0x28000 bytes > EAL: Virtual area found at 0x7ffd4000 (size = 0x28000) > EAL: Requesting 10 pages of size 1024MB from socket 0 > RING: Cannot reserve memory for tailq > EAL: rte_eal_common_log_init(): cannot create log_history mempool > PANIC in rte_eal_init(): > Cannot init logs > 6: [./build/l2fwd() [0x41cf53]] > 5: [/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5) > [0x77105ec5]] > 4: [./build/l2fwd(main+0x1c) [0x41c52c]] > 3: [./build/l2fwd(rte_eal_init+0x1073) [0x46a653]] > 2: [./build/l2fwd(__rte_panic+0xc9) [0x41c3ff]] > 1: [./build/l2fwd(rte_dump_stack+0x18) [0x471828]] > > > I tried bisect, and it seems below patch causes this. > "eal: fix check for power of 2 in 0 case". > > > Thanks, > Tetsuya > > Hey Tetsuya, This is the issue which my patch (http://dpdk.org/dev/patchwork/patch/2333/) submitted on Friday addresses.
[dpdk-dev] [PATCH] eal / malloc : alignment parameter check failing due to changes in rte_is_power_of_2
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Friday, January 16, 2015 5:02 PM > To: Doherty, Declan > Cc: dev at dpdk.org; De Lara Guarch, Pablo > Subject: Re: [dpdk-dev] [PATCH] eal / malloc : alignment parameter check > failing > due to changes in rte_is_power_of_2 > > 2015-01-16 15:10, Declan Doherty: > > In commit 2fc8d6d the behaviour of function rte_is_power_of_2 was > > changed to not return true for 0. memzone_reserve_aligned_thread_unsafe > > and rte_malloc_socket both make the assumption that for align = 0 > > !rte_is_power_of_2(align) will return false. This patch adds a check > > that align parameter is non-zero before doing the power of 2 check > > > > Signed-off-by: Declan Doherty > [...] > > - if (!rte_is_power_of_2(align)) { > > + if (align ? !rte_is_power_of_2(align) : 0) { > [...] > > - if (size == 0 || !rte_is_power_of_2(align)) > > + if (size == 0 || align ? !rte_is_power_of_2(align) : 0) > > I don't understand why you write "align ? !rte_is_power_of_2(align) : 0" > instead of the more readable "align && !rte_is_power_of_2(align)" ? > > Pablo acked it so I guess there is something obvious I'm missing. > > -- > Thomas No there's nothing you're missing, this is just the way I saw the logic, if align is none zero, then test the power of 2 condition otherwise return 0. I have no problem with your suggestion in you prefer that, at the end of the day the logic test works out equivalent.
[dpdk-dev] [PATCH v2] bond: set {rx|tx}_offload_capa flags
Thanks Thomas, was just about to send another version :) > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, November 27, 2014 9:53 PM > To: Doherty, Declan > Cc: dev at dpdk.org; jyu at vmware.com > Subject: Re: [PATCH v2] bond: set {rx|tx}_offload_capa flags > > 2014-11-27 21:23, Declan Doherty: > > v2: > > rebased to HEAD > > > > From: Jia Yu > > > > Before the fix, bond device's offload capabilities are unset. This fix > > takes the minimum common set of slave devices' capabilities as bond > > device's capabilities. For simplicity, we ensure all slave devices > > to have a capability before bond device can claim this capability, > > even if some slave devices are unused (i.e. linked down, standby). > > > > Signed-off-by: Jia Yu > > Signed-off-by: Declan Doherty > > Applied > > Thanks > -- > Thomas
[dpdk-dev] [PATCH v2] bond: set {rx|tx}_offload_capa flags
Self nack, need to rebase again as mode 4 and 5 patches have been applied. > -Original Message- > From: Doherty, Declan > Sent: Thursday, November 27, 2014 9:24 PM > To: dev at dpdk.org > Cc: jyu at vmware.com; thomas.monjalon at 6wind.com; Doherty, Declan > Subject: [PATCH v2] bond: set {rx|tx}_offload_capa flags > > v2: > rebased to HEAD > > From: Jia Yu > > Before the fix, bond device's offload capabilities are unset. This fix > takes the minimum common set of slave devices' capabilities as bond > device's capabilities. For simplicity, we ensure all slave devices > to have a capability before bond device can claim this capability, > even if some slave devices are unused (i.e. linked down, standby). > > Signed-off-by: Jia Yu > Signed-off-by: Declan Doherty > --- > lib/librte_pmd_bond/rte_eth_bond_api.c | 18 +- > lib/librte_pmd_bond/rte_eth_bond_pmd.c | 5 + > lib/librte_pmd_bond/rte_eth_bond_private.h | 3 +++ > 3 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_pmd_bond/rte_eth_bond_api.c > b/lib/librte_pmd_bond/rte_eth_bond_api.c > index f146bda..5125b57 100644 > --- a/lib/librte_pmd_bond/rte_eth_bond_api.c > +++ b/lib/librte_pmd_bond/rte_eth_bond_api.c > @@ -238,6 +238,8 @@ rte_eth_bond_create(const char *name, uint8_t mode, > uint8_t socket_id) > > internals->slave_count = 0; > internals->active_slave_count = 0; > + internals->rx_offload_capa = 0; > + internals->tx_offload_capa = 0; > > memset(internals->active_slaves, 0, sizeof(internals->active_slaves)); > memset(internals->slaves, 0, sizeof(internals->slaves)); > @@ -265,6 +267,7 @@ __eth_bond_slave_add_lock_free(uint8_t > bonded_port_id, uint8_t slave_port_id) > struct bond_dev_private *internals; > struct bond_dev_private *temp_internals; > struct rte_eth_link link_props; > + struct rte_eth_dev_info dev_info; > > int i, j; > > @@ -296,6 +299,9 @@ __eth_bond_slave_add_lock_free(uint8_t > bonded_port_id, uint8_t slave_port_id) > /* Add slave details to bonded device */ > slave_add(internals, slave_eth_dev); > > + memset(_info, 0, sizeof(dev_info)); > + rte_eth_dev_info_get(slave_port_id, _info); > + > if (internals->slave_count < 1) { > /* if MAC is not user defined then use MAC of first slave add to >* bonded device */ > @@ -308,6 +314,11 @@ __eth_bond_slave_add_lock_free(uint8_t > bonded_port_id, uint8_t slave_port_id) > > /* Make primary slave */ > internals->primary_port = slave_port_id; > + > + /* Take the first dev's offload capabilities */ > + internals->rx_offload_capa = dev_info.rx_offload_capa; > + internals->tx_offload_capa = dev_info.tx_offload_capa; > + > } else { > /* Check slave link properties are supported if props are set, >* all slaves must be the same */ > @@ -323,6 +334,8 @@ __eth_bond_slave_add_lock_free(uint8_t > bonded_port_id, uint8_t slave_port_id) > 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->slave_count++; > @@ -455,7 +468,10 @@ __eth_bond_slave_remove_lock_free(uint8_t > bonded_port_id, uint8_t slave_port_id) > memset(rte_eth_devices[bonded_port_id].data- > >mac_addrs, 0, > > sizeof(*(rte_eth_devices[bonded_port_id].data->mac_addrs))); > } > - > + if (internals->slave_count == 0) { > + internals->rx_offload_capa = 0; > + internals->tx_offload_capa = 0; > + } > return 0; > } > > diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c > b/lib/librte_pmd_bond/rte_eth_bond_pmd.c > index cf2fbab..0d1a36b 100644 > --- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c > +++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c > @@ -788,6 +788,8 @@ static int bond_ethdev_configure(struct rte_eth_dev > *dev); > static void > bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) > { > + struct bond_dev_private *internals = dev->data->dev_private; > + > dev_info->driver_name = driver_name; > dev_info->max_mac_addrs = 1; > > @@ -798,6 +800,9 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct > rte_eth_dev_info *dev_info) > >
[dpdk-dev] [PATCH v4 0/2] ADD mode 5(tlb) to link bonding pmd
Sorry Thomas, this was my fault, I forgot that Daniel mentioned to me earlier today that he was going to submit a new revision of the patch set with updated patch descriptions, but no code changes, due to your comments relating to the link bonding mode 4. I will ack the v4 patchset when all patches for v4 are available. Declan > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, November 27, 2014 4:42 PM > To: Mrzyglod, DanielX T > Cc: dev at dpdk.org; Doherty, Declan > Subject: Re: [dpdk-dev] [PATCH v4 0/2] ADD mode 5(tlb) to link bonding pmd > > 2014-11-27 16:33, Daniel Mrzyglod: > > ADD mode 5(tlb) to link bonding pmd > > > > v4 change: > > Change description of mode 5 in header. > > Add description to cover letter. > > > > v3 change: > > Rebase patch version to HEAD of orgin/master. > > Unit tests moved to the separate patch v3 2/2. > > > > v2 change: > > Add Unit Tests > > Modification that updates obytes structure in virtualpmd driver. > > change internals->slaves[i].last_obytes to have proper values. > > Update codebase to Declan's patches. > > > > v1 change: > > Add support for mode 5 (Transmit load balancing) into pmd driver > > > > > > Daniel Mrzyglod (2): > > This patch add support of mode 5 to link bonding pmd > > Unit tests for Mode 5 of Bonding Transmit Load balancig. > > Please help me to understand. Declan just acked the v3 and you are sending > an incomplete v4 (patch 2 is missing). > When a complete patchset will be sent, it will be applied. > > -- > Thomas
[dpdk-dev] [PATCH v3 0/2] ADD mode 5(tlb) to link bonding pmd
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Mrzyglod, DanielX T > Sent: Wednesday, November 26, 2014 5:18 PM > To: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/2] ADD mode 5(tlb) to link bonding pmd > > v3 change: > Rebase patch version to HEAD of orgin/master. > Unit tests moved to the separate patch v3 2/2. > > v2 change: > Add Unit Tests > Modification that updates obytes structure in virtualpmd driver. > change internals->slaves[i].last_obytes to have proper values. > Update codebase to Declan's patches. > > v1 change: > Add support for mode 5 (Transmit load balancing) into pmd driver > > This mode provides an adaptive transmit load balancing. > It dynamically changes the transmitting slave, according to the computed load. > Statistics are collected in 100ms intervals and scheduled every 10ms. > > > -Original Message- > > From: Mrzyglod, DanielX T > > Sent: Wednesday, November 26, 2014 6:13 PM > > To: dev at dpdk.org > > Cc: Mrzyglod, DanielX T > > Subject: [PATCH v3 0/2] ADD mode 5(tlb) to link bonding pmd > > > > This mode provides an adaptive transmit load balancing. > > It dynamically changes the transmitting slave, according to the computed > > load. > > Statistics are collected in 100ms intervals and scheduled every 10ms. > > > > Daniel Mrzyglod (2): > > This patch add support of mode 5 to link bonding pmd > > Unit tests for Mode 5 of Bonding Transmit Load balancing. > > > > app/test/test_link_bonding.c | 499 > - > > app/test/virtual_pmd.c | 6 +- > > lib/librte_pmd_bond/rte_eth_bond.h | 11 + > > lib/librte_pmd_bond/rte_eth_bond_args.c| 1 + > > lib/librte_pmd_bond/rte_eth_bond_pmd.c | 160 - > > lib/librte_pmd_bond/rte_eth_bond_private.h | 2 +- > > 6 files changed, 673 insertions(+), 6 deletions(-) > > > > -- > > 2.1.0 Acked by Declan Doherty
[dpdk-dev] [PATCH v7 0/7] link bonding
Hey, sorry about that, I submitted the patchset without the typo fix. I'll resubmit the patchset now. What version of checkpatch are you using, I don't get any warning for the missing spaces for the version of the script with kernel 3.15 on Fedora Thanks Declan > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Monday, November 24, 2014 3:36 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [PATCH v7 0/7] link bonding > > Hi Declan, > > There are still 2 kind of errors reported by checkpatch: > - 14 occurrences of this typo: > WARNING:TYPO_SPELLING: 'interupt' may be misspelled - perhaps 'interrupt'? > - 6 occurences of missing space: > WARNING:MISSING_SPACE: break quoted strings at a space character > > Example: > #2016: FILE: lib/librte_pmd_bond/rte_eth_bond_pmd.c:1420: > + "LSC polling interval can be specified only > once for bonded" > + "device %s\n", name); > You should put a space between "bonded" and "device". > > -- > Thomas
[dpdk-dev] [PATCH v6 0/8] link bonding
Hey Thomas, have you had a chance to have a look at this patchset, if you have any comments or issues can you let me know asap, as the patch sets for link bonding modes 4 & 5 are dependent on this set and I don't want to be blocking them from the next release candidate. Thanks Declan > -Original Message- > From: De Lara Guarch, Pablo > Sent: Friday, November 7, 2014 4:41 PM > To: Doherty, Declan; dev at dpdk.org > Cc: thomas.monjalon at 6wind.com > Subject: RE: [PATCH v6 0/8] link bonding > > > > > -Original Message- > > From: Doherty, Declan > > Sent: Friday, November 07, 2014 12:23 PM > > To: dev at dpdk.org > > Cc: De Lara Guarch, Pablo; thomas.monjalon at 6wind.com; Doherty, Declan > > Subject: [PATCH v6 0/8] link bonding > > > > v6: > > - Re-based to dpdk.org addressing associated issues for MBUF_REFCNT > > - Added details to testpmd user guide for new command to set link status > > polling interval. > > > > v5: > > - Fix uninitialized variable in broadcast_tx_burst function which caused a > > build error in 32-bit build > > - Address unit test issue which is exposed by new test in mode 4/5 patch > > sets > > > > v4: > > - Rebased to account for changes in master. > > - Fix for rte_eth_bond_slaves_get() introduced in v3 patch set > > - Addressed issue around disabling/enabling link status polling around > > adding/ > > removing slaves devices. > > > > v3 : > > - Typo fix for the bond free mbufs patch. > > - Rebased to account for changes in the mbuf patches. > > - Add support for slave devices which don't support link status interrupts > > - Tidy up the link bonding unit test so that all tests use the new test > > macros. > > > > v2 : > > Addresses issues with the logic around the handling of fail transmissions. > > In this version all modes behave in a manner similar to a standard PMD, > > returning the number of successfully transmitted mbufs and with the failing > > mbufs at the end of bufs array for freeing / retransmission by the > > application software > > > > v1: > > > > This patch set adds support for link status interrupt in the link bonding > > pmd. It also contains some patches to tidy up the code structure and to > > of the link bonding code and to fix bugs relating to transmission > > failures in the under lying slave pmd which could lead to leaked mbufs. > > > > > > Declan Doherty (8): > > bond: link status interrupt support > > bond: removing switch statement from rx burst method > > bond: fix naming inconsistency in tx_burst_round_robin > > bond: free mbufs if transmission fails in bonding tx_burst functions > > test app: adding support for generating variable sized packet > > testpmd: adding parameter to reconfig method to set socket_id when > > adding new port to portlist > > bond: lsc polling support > > bond: unit test test macro refactor > > > > app/test-pmd/cmdline.c | 65 +- > > app/test-pmd/testpmd.c |3 +- > > app/test-pmd/testpmd.h |2 +- > > app/test/packet_burst_generator.c | 25 +- > > app/test/packet_burst_generator.h |6 +- > > app/test/test.h |7 +- > > app/test/test_link_bonding.c| 3347 > > ++- > > app/test/virtual_pmd.c | 96 +- > > app/test/virtual_pmd.h | 53 +- > > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 19 + > > lib/librte_pmd_bond/rte_eth_bond.h | 80 + > > lib/librte_pmd_bond/rte_eth_bond_api.c | 319 ++- > > lib/librte_pmd_bond/rte_eth_bond_args.c | 28 +- > > lib/librte_pmd_bond/rte_eth_bond_pmd.c | 553 +++-- > > lib/librte_pmd_bond/rte_eth_bond_private.h | 71 +- > > 15 files changed, 2715 insertions(+), 1959 deletions(-) > > > > -- > > 1.7.12.2 > > Acked-by: Pablo de Lara
[dpdk-dev] [PATCH] Bond: set {rx|tx}_offload_capa flags
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jia Yu > Sent: Friday, November 7, 2014 5:36 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH] Bond: set {rx|tx}_offload_capa flags > > Before the fix, bond device's offload capabilities are unset. This fix > takes the minimum common set of slave devices' capabilities as bond > device's capabilities. For simplicity, we ensure all slave devices > to have a capability before bond device can claim this capability, > even if some slave devices are unused (i.e. linked down, standby). > > Signed-off-by: Jia Yu > --- > lib/librte_pmd_bond/rte_eth_bond_api.c | 16 > . > -- > 1.9.1 Acked-by: Declan Doherty
[dpdk-dev] [PATCH] ixgbe: Fix compilation issue in vpmd
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > Sent: Tuesday, October 21, 2014 9:36 AM > To: Ouyang, Changchun; De Lara Guarch, Pablo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] ixgbe: Fix compilation issue in vpmd > > 2014-10-21 08:28, Ouyang, Changchun: > > From: De Lara Guarch, Pablo > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > > > > 2014-10-21 14:59, Ouyang Changchun: > > > > > Fix the compilation issue in vector PMD when macro RTE_MBUF_REFCNT > > > > > is disabled. > > > > > > > > > > Signed-off-by: Changchun Ouyang > > > > > > > > Acked-by: Thomas Monjalon > > > > > > > > Applied > > > > > > I was checking this patch right now, and I come across a second > > > compilation > > > issue, because rte_mbuf_refcnt_update and rte_pktmbuf_attach are not > > > declared, and Bond PMD and IP fragmentation libraries use those functions. > > > > > > I guess that it is late to NACK this :P, but we require a second patch to > > > fix > > > completely this issue. > > > > As it fixes the compilation issue in vpmd, so no reason to NACK it, :-) > > Exact > > > In my config, both BOND and IP fragment is disabled. So I don't come across > > your > issues. > > Yes, agree with you, we need another patch to fix compilation issue in other > both places. > > Yes, I'm aware of these limitations. > Please, first explain why mbuf refcnt is needed for these features. > Then we have 2 options: remove the dependency or add more ifdefs. > > Thanks > -- > Thomas Thomas, for link bonding the refcnt is used in broadcast mode to allow the same mbuf to be transmitted on multiple slaves. By increasing the refcnt, we can safely transmit the mbuf on multiple slaves without the danger of a mbuf being freed by one slave while in use by another. I can provide a patch to disable the broadcast mode of operation if the bonding pmd is built with the macro RTE_MBUF_REFCNT disabled. I don't think there is any other option other than than to completely disable the library if the refcnt parameter isn't available as we don't have access to the transmit mempool to allocate new mbufs to make copies of the original. Declan
[dpdk-dev] [PATCH v2] ADD mode 5(tlb) to link bonding pmd
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Daniel Mrzyglod > Sent: Friday, September 26, 2014 4:41 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v2] ADD mode 5(tlb) to link bonding pmd > > > Signed-off-by: Daniel Mrzyglod > --- > app/test/test_link_bonding.c | 501 > +++- > app/test/virtual_pmd.c |6 +- > app/test/virtual_pmd.h |7 + > lib/librte_pmd_bond/rte_eth_bond.h | 23 ++ > lib/librte_pmd_bond/rte_eth_bond_args.c|1 + > lib/librte_pmd_bond/rte_eth_bond_pmd.c | 161 - > lib/librte_pmd_bond/rte_eth_bond_private.h |3 +- > 7 files changed, 696 insertions(+), 6 deletions(-) > > diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c > index c4fcaf7..77f791f 100644 > --- a/app/test/test_link_bonding.c > +++ b/app/test/test_link_bonding.c > @@ -41,7 +41,7 @@ > #include > #include > #include > - > +#include > #include > #include > #include > @@ -3845,6 +3845,500 @@ testsuite_teardown(void) > return remove_slaves_and_stop_bonded_device(); > } > > +#define NINETY_PERCENT_NUMERAL 90 > +#define ONE_HUNDRED_PERCENT_DENOMINATOR 100 > +#define ONE_HUNDRED_PERCENT_AND_TEN_NUMERAL 110 > +static int > +test_tlb_tx_burst(void) > +{ > + int i, burst_size, nb_tx; > + uint64_t nb_tx2 = 0; > + struct rte_mbuf *pkt_burst[MAX_PKT_BURST]; > + struct rte_eth_stats port_stats[32]; > + uint64_t sum_ports_opackets = 0, all_bond_opackets = 0, > all_bond_obytes = 0; > + uint16_t pktlen; > + > + TEST_ASSERT_SUCCESS(initialize_bonded_device_with_slaves > + > (BONDING_MODE_ADAPTIVE_TRANSMIT_LOAD_BALANCING, 1, 3, 1), > + "Failed to initialise bonded device"); > + > + burst_size = 20 * test_params->bonded_slave_count; > + > + TEST_ASSERT(burst_size < MAX_PKT_BURST, > + "Burst size specified is greater than supported.\n"); > + > + > + /* Generate 40 test bursts in 2s of packets to transmit */ > + for (i = 0; i < 40; i++) { > + /*test two types of mac src own(bonding) and others */ > + if (i % 2 == 0) { > + initialize_eth_header(test_params->pkt_eth_hdr, > + (struct ether_addr *)src_mac, (struct > ether_addr *)dst_mac_0, 0, 0); > + } else { > + initialize_eth_header(test_params->pkt_eth_hdr, > + (struct ether_addr *)test_params- > >default_slave_mac, > + (struct ether_addr *)dst_mac_0, 0, 0); > + } > + pktlen = initialize_udp_header(test_params->pkt_udp_hdr, > src_port, > + dst_port_0, 16); > + pktlen = initialize_ipv4_header(test_params->pkt_ipv4_hdr, > src_addr, > + dst_addr_0, pktlen); > + generate_packet_burst(test_params->mbuf_pool, pkt_burst, > + test_params->pkt_eth_hdr, 0, test_params- > >pkt_ipv4_hdr, > + 1, test_params->pkt_udp_hdr, burst_size, 60, 1); > + /* Send burst on bonded port */ > + nb_tx = rte_eth_tx_burst(test_params->bonded_port_id, 0, > pkt_burst, > + burst_size); > + nb_tx2 += nb_tx; > + > + TEST_ASSERT_EQUAL(nb_tx, burst_size, > + "number of packet not equal burst size"); > + > + rte_delay_us(5); > + } > + > + > + /* Verify bonded port tx stats */ > + rte_eth_stats_get(test_params->bonded_port_id, _stats[0]); > + > + all_bond_opackets = port_stats[0].opackets; > + all_bond_obytes = port_stats[0].obytes; > + > + TEST_ASSERT_EQUAL(port_stats[0].opackets, (uint64_t)nb_tx2, > + "Bonded Port (%d) opackets value (%u) not as expected > (%d)\n", > + test_params->bonded_port_id, (unsigned > int)port_stats[0].opackets, > + burst_size); > + > + > + /* Verify slave ports tx stats */ > + for (i = 0; i < test_params->bonded_slave_count; i++) { > + rte_eth_stats_get(test_params->slave_port_ids[i], > _stats[i]); > + sum_ports_opackets += port_stats[i].opackets; > + } > + > + TEST_ASSERT_EQUAL(sum_ports_opackets, (uint64_t)all_bond_opackets, > + "Total packets sent by slaves is not equalto packets > sent by > bond interface"); > + > + for (i = 0; i < test_params->bonded_slave_count; i++) { > + printf("port stats:%"PRIu64"\n", port_stats[i].opackets); > + /* distribution of packets on each slave within +/- 10% of the > expected value. */ > + TEST_ASSERT(port_stats[i].obytes >= > ((all_bond_obytes*NINETY_PERCENT_NUMERAL)/ > + (test_params- >
[dpdk-dev] [PATCH v2 4/6] bond: free mbufs if transmission fails in bonding tx_burst functions
> -Original Message- > From: Doherty, Declan > Sent: Monday, September 1, 2014 9:31 AM > To: dev at dpdk.org > Cc: thomas.monjalon at 6wind.com; rsanford at akamai.com; Doherty, Declan > Subject: [PATCH v2 4/6] bond: free mbufs if transmission fails in bonding > tx_burst > functions > > Fixing a number of corner cases that if transmission failed on slave devices > then > this > could lead to leaked mbufs > > V2 addresses behaviouraly issues in the first version and packets are no > longer > freed in > the bonding layer, except in the case of broadcast mode where in failures > happen > on > more than a single slave then mbufs will be freed in all slaves except the one > where > the least errors occured. Also contains new unit tests to test the > transmission > failure case in slaves for > round-robin, balance, and broadcast modes. > > > Signed-off-by: Declan Doherty > --- . > -- > 1.7.0.7 I've just noticed that I inserted an extra character into the patch when I was annotating this patch which will cause this a fail compilation. > + TEST_BCAST_SLAVE_TX_FAIL_BURST_SIZE -s > + TEST_BCAST_SLAVE_TX_FAIL_MIN_PACKETS_COUNT); should have been > + TEST_BCAST_SLAVE_TX_FAIL_BURST_SIZE - > + TEST_BCAST_SLAVE_TX_FAIL_MIN_PACKETS_COUNT); Thomas, do you want me to submit a V3 of this? Regards Declan
[dpdk-dev] [PATCH v11 0/5] link bonding library
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Monday, June 30, 2014 10:21 AM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [PATCH v11 0/5] link bonding library > > Hi Declan, > > 2014-06-29 18:49, Declan Doherty: > > This patchset contains the name change fix in the link bonding library > > makefile. I have also put a change into the unit test application makefile > > so that the link bonding tests are not built if the library is also not > > being built. It doesn't make sense to split the bonding libraries APIs into > > a seperate library, as they are directly coupled to with the bonding > > library implmentation > > You fixed optional building but there is still an issue when building shared > library. > As you don't want to split the library to separate the PMD plugin from the > specific API, I suggest to not consider it as a plugin (unlike other PMDs). > > I've sent some patches about these 2 topics. > If you agree, I'll apply them, merged with your serie. > > Thanks > -- > Thomas Hi Thomas, I'm happy for you to apply the makefile changes you have suggested and not consider the bonding PMD as a plugin, I think this makes the most sense considering it is a purely software library. Thanks Declan
[dpdk-dev] [PATCH v7 6/6] Link Bonding Library doxygen additions
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, June 25, 2014 2:44 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [PATCH v7 6/6] Link Bonding Library doxygen additions > > Hi Declan, > > Since bonding library is a PMD, there should have no need of integrate it > in the API documentation. > I hadn't time to review it yet, so I would miss something. > Do you mean it can be used directly by the application with a specific API? > In this case, you could explain it in the commit log. > Hi Thomas, the bonding library has a API which supports creation and management of bonded devices directly from within applications, but more importantly provides the APIs required to dynamically add and remove slaves from a bonded device, this functionality will be required to support live migration of VMs using bonded ports in a future releases.
[dpdk-dev] [PATCH v3 2/5] Link Bonding PMD Library (librte_eal/librte_ether link bonding support changes)
> -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Friday, June 13, 2014 8:38 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 2/5] Link Bonding PMD Library > (librte_eal/librte_ether link bonding support changes) > > On Fri, Jun 13, 2014 at 06:34:04PM +, Doherty, Declan wrote: > > > -Original Message- > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > Sent: Friday, June 13, 2014 5:08 PM > > > To: Doherty, Declan > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v3 2/5] Link Bonding PMD Library > > > (librte_eal/librte_ether link bonding support changes) > > > > > > On Fri, Jun 13, 2014 at 03:41:59PM +0100, Declan Doherty wrote: > > > > Updating functionality in EAL to support adding link bonding > > > > devices via ?vdev option. Link bonding devices will be > > > > initialized after all physical devices have been probed and > > > > initialized. > > > > > > > > Signed-off-by: Declan Doherty > > > > --- > > > > lib/librte_eal/common/eal_common_dev.c | 66 > > > +++-- > > > > lib/librte_eal/common/eal_common_pci.c | 6 +++ > > > > lib/librte_eal/common/include/eal_private.h | 7 +++ > > > > lib/librte_eal/common/include/rte_dev.h | 1 + > > > > lib/librte_ether/rte_ethdev.c | 34 +-- > > > > lib/librte_ether/rte_ethdev.h | 7 ++- > > > > lib/librte_pmd_pcap/rte_eth_pcap.c | 22 +- > > > > lib/librte_pmd_ring/rte_eth_ring.c | 32 +++--- > > > > lib/librte_pmd_ring/rte_eth_ring.h | 3 +- > > > > lib/librte_pmd_xenvirt/rte_eth_xenvirt.c| 2 +- > > > > 10 files changed, 144 insertions(+), 36 deletions(-) > > > > > > > > diff --git a/lib/librte_eal/common/eal_common_dev.c > > > b/lib/librte_eal/common/eal_common_dev.c > > > > index eae5656..b50c908 100644 > > > > --- a/lib/librte_eal/common/eal_common_dev.c > > > > +++ b/lib/librte_eal/common/eal_common_dev.c > > > > @@ -75,14 +75,28 @@ rte_eal_dev_init(void) > > > > > > > > /* call the init function for each virtual device */ > > > > TAILQ_FOREACH(devargs, _list, next) { > > > > + uint8_t bdev = 0; > > > > > > > > if (devargs->type != RTE_DEVTYPE_VIRTUAL) > > > > continue; > > > > > > > > TAILQ_FOREACH(driver, _driver_list, next) { > > > > - if (driver->type != PMD_VDEV) > > > > + /* RTE_DEVTYPE_VIRTUAL can only be a virtual or > > > > bonded > > > device*/ > > > > + if (driver->type != PMD_VDEV && driver->type != > > > PMD_BDEV) > > > > continue; > > > > > > > > + /* > > > > +* Bonded devices are not initialize here, we > > > > do it later in > > > > +* rte_eal_bonded_dev_init() after all physical > > > > devices > > > have been > > > > +* probed and initialized > > > > +*/ > > > > + if (driver->type == PMD_BDEV && > > > > + !strncmp(driver->name, devargs- > > > >virtual.drv_name, > > > > + > > > > strlen(driver->name))) { > > > > + bdev = 1; > > > > + break; > > > > + } > > > > + > > > I really don't think you need to add a new device type for bonded devs. > > > Its got > > > no specific hardware that it drives, and you configure it with a --vdev > command, > > > so treat it as one here. I understand that you need to pass additional > > > information about slaves to a bonded device, which is fine, but you can > > > do that > > > with kvargs pretty easily, at which point its just another vdev. The only > > > other > > > requirement is that you initilize the bonded vdev after the slave vdevs > > &
[dpdk-dev] [PATCH v3 2/5] Link Bonding PMD Library (librte_eal/librte_ether link bonding support changes)
> -Original Message- > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Friday, June 13, 2014 10:59 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 2/5] Link Bonding PMD Library > (librte_eal/librte_ether link bonding support changes) > > On Fri, 13 Jun 2014 15:41:59 +0100 > Declan Doherty wrote: > > > Updating functionality in EAL to support adding link bonding > > devices via ???vdev option. Link bonding devices will be > > initialized after all physical devices have been probed and > > initialized. > > > > Signed-off-by: Declan Doherty > > The DPDK is used by both static (demo style) applications and dynamic > applications that need to be able to modify parameters while running. > The tendency of the code has been to have values that can only be set > at initialization, and this kind of inflexibility is hard to fix. > > The whole dev args style of configuration is a poor design for those > reasons. How do you support creating additional bond devices at runtime? > > Remember what works for a demo doesn't make a good API for real world. > > Hi Stephen, I totally agree but all of the dev argument parsing / static initialization here is based on the rte_eth_bond.h public API. Actually testpmd has full support for dynamic initialization and management of bonding devices in run-time through the interactive command line options which pretty much have options corresponding to all of the public link bonding APIs. It is my intent that this would main interface that this API would be used by user applications and that the static configuration options using --vdev interface would only be used if a user wanted to use a bonded device in a existing application without having to make any changes to their code or add any explicit reference to the link bonding library.
[dpdk-dev] [PATCH v3 2/5] Link Bonding PMD Library (librte_eal/librte_ether link bonding support changes)
> -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Friday, June 13, 2014 5:08 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 2/5] Link Bonding PMD Library > (librte_eal/librte_ether link bonding support changes) > > On Fri, Jun 13, 2014 at 03:41:59PM +0100, Declan Doherty wrote: > > Updating functionality in EAL to support adding link bonding > > devices via ?vdev option. Link bonding devices will be > > initialized after all physical devices have been probed and > > initialized. > > > > Signed-off-by: Declan Doherty > > --- > > lib/librte_eal/common/eal_common_dev.c | 66 > +++-- > > lib/librte_eal/common/eal_common_pci.c | 6 +++ > > lib/librte_eal/common/include/eal_private.h | 7 +++ > > lib/librte_eal/common/include/rte_dev.h | 1 + > > lib/librte_ether/rte_ethdev.c | 34 +-- > > lib/librte_ether/rte_ethdev.h | 7 ++- > > lib/librte_pmd_pcap/rte_eth_pcap.c | 22 +- > > lib/librte_pmd_ring/rte_eth_ring.c | 32 +++--- > > lib/librte_pmd_ring/rte_eth_ring.h | 3 +- > > lib/librte_pmd_xenvirt/rte_eth_xenvirt.c| 2 +- > > 10 files changed, 144 insertions(+), 36 deletions(-) > > > > diff --git a/lib/librte_eal/common/eal_common_dev.c > b/lib/librte_eal/common/eal_common_dev.c > > index eae5656..b50c908 100644 > > --- a/lib/librte_eal/common/eal_common_dev.c > > +++ b/lib/librte_eal/common/eal_common_dev.c > > @@ -75,14 +75,28 @@ rte_eal_dev_init(void) > > > > /* call the init function for each virtual device */ > > TAILQ_FOREACH(devargs, _list, next) { > > + uint8_t bdev = 0; > > > > if (devargs->type != RTE_DEVTYPE_VIRTUAL) > > continue; > > > > TAILQ_FOREACH(driver, _driver_list, next) { > > - if (driver->type != PMD_VDEV) > > + /* RTE_DEVTYPE_VIRTUAL can only be a virtual or bonded > device*/ > > + if (driver->type != PMD_VDEV && driver->type != > PMD_BDEV) > > continue; > > > > + /* > > +* Bonded devices are not initialize here, we do it > > later in > > +* rte_eal_bonded_dev_init() after all physical devices > have been > > +* probed and initialized > > +*/ > > + if (driver->type == PMD_BDEV && > > + !strncmp(driver->name, devargs- > >virtual.drv_name, > > + strlen(driver->name))) { > > + bdev = 1; > > + break; > > + } > > + > I really don't think you need to add a new device type for bonded devs. Its > got > no specific hardware that it drives, and you configure it with a --vdev > command, > so treat it as one here. I understand that you need to pass additional > information about slaves to a bonded device, which is fine, but you can do > that > with kvargs pretty easily, at which point its just another vdev. The only > other > requirement is that you initilize the bonded vdev after the slave vdevs have > been created, which you can do by any of several methods (a priority field to > indicate that bonded drivers should be initilized last/later, a deferral > return > code from the init routine, or by dead reckoning via the careful construction > of > the application command line (placed the bonded --vdev option last on the > command line argument list at run time). > It was my initial intent to do as you have describe above, but the physical devices cause a real issue here, physical devices don't call through to rte_eth_dev_allocate until during rte_eal_pci_probe call, so it's not possible to initialize the bonded device from within rte_eal_dev_init as the physical devices have not been fully initialized at this point, as a port_id has not been allocated and can't be added as bonding slaves. I don't see away around this without changing the EAL API, which I've tried to avoid with this solution. Ordering isn't an issue, and can easily be solved if the above problem didn't exist, and although a new device type isn't technically required, I think it's a cleaner solution than doing string comparisons. > > > /* search a driver prefix in virtual device name */ > > if (!strncmp
[dpdk-dev] [PATCH v2 0/4] Link Bonding Library
> -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Friday, June 6, 2014 3:54 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 0/4] Link Bonding Library > > On Fri, Jun 06, 2014 at 08:23:31AM +, Doherty, Declan wrote: > > > -Original Message- > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > Sent: Thursday, May 29, 2014 12:34 PM > > > To: Doherty, Declan > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH 0/4] Link Bonding Library > > > > > > On Thu, May 29, 2014 at 10:33:00AM +, Doherty, Declan wrote: > > > > -Original Message- > > > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > > > Sent: Wednesday, May 28, 2014 6:49 PM > > > > > To: Doherty, Declan > > > > > Cc: dev at dpdk.org > > > > > Subject: Re: [dpdk-dev] [PATCH 0/4] Link Bonding Library > > > > > > > > > > On Wed, May 28, 2014 at 04:32:00PM +0100, declan.doherty at intel.com > > > wrote: > > > > > > From: Declan Doherty > > > > > > > > > > > > Initial release of Link Bonding Library (lib/librte_bond) with > > > > > > support for bonding modes : > > > > > > 0 - Round Robin > > > > > > 1 - Active Backup > > > > > > 2 - Balance l2 / l23 / l34 > > > > > > 3 - Broadcast > > > > > > > > > > > Why make this a separate library? That requires exposure of yet > > > > > another > > > API to applications. Instead, why > not write a PMD that can enslave > > > other PMD's and treat them all as a single interface? That way this > > > all > > works with the existing API. > > > > > > > > > > Neil > > > > > > > > Hi Neil, > > > > the link bonding device is essentially a software PMD, and as such > > > > supports > > > all the standard PMD APIs, the only new APIs which the link bonding > > > library introduces are for the control operations of the bonded > > > device which are currently unsupported by the standard PMD API. > > > Operations such as creating, adding/removing slaves, and configuring > > > the modes of operation of the device have no analogous APIs in the > > > current PMD API and required new ones to be created . > > > > > > Thats really only true in spirit, in the sense that this library > > > transmits and receives frames like a PMD does. In practice it doesn't > > > work and isn't architected the same way. You don't register the > > > driver using the same method as the other PMDs, which means that using > > > --vdev on the command line wont work for this type of device. It also > > > implies that applications have to be made specifically aware of the > > > fact that they are using a bonded interface (i.e. they need to call > > > the bonding setup routines to create the bond). I would > > > recommend: > > > > > > 1) Register the pmd using the PMD_DRIVER_REGISTER macro, like other > > > PMD's > > > 2) Use the kvargs library to support configuration via the --vdev > > > command line option, so bonds can be created administratively, rather > > > than just programatically > > > 3) Separate the command api from the PMD functionality into separate > > > libraries (use control mbufs to communicate configuration changes to > > > the pmd). This will allow users to dynamically load the pmd > > > functionality (without compile or run time linking requirements), and > > > then optionally use the programatic interface (or not if they want to > > > save memory) > > > > > > Regards > > > Neil > > > -Original Message- > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > Sent: Thursday, June 5, 2014 12:04 PM > > > To: Doherty, Declan > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v2 0/4] Link Bonding Library > > > > > > On Wed, Jun 04, 2014 at 04:18:01PM +0100, declan.doherty at intel.com > > > wrote: > > > > From: Declan Doherty > > > > > > > > v2 patch additions, > > > > fix for tx burst broadcast, incrementing the reference count on each > > > > mbuf by the number of slaves - 1 add/remove slave behavior chnange > > > > to f
[dpdk-dev] [PATCH v2 0/4] Link Bonding Library
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, June 11, 2014 5:33 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 0/4] Link Bonding Library > > Hi Declan, > > Do you think you can send another version of your serie soon? > Some comments need to be addressed. > > 2 formatting comments: > - I saw some strange alignments of comments > - you should send your patches as children of the cover letter > > Thanks > -- > Thomas Hi Thomas, I've just submitted v3 of the patch, which should address the majority of the comments and any formatting issues. Regards Declan
[dpdk-dev] [PATCH v2 1/4] Link Bonding Library
> -Original Message- > From: Eric Kinzie [mailto:ehkinzie at gmail.com] > Sent: Monday, June 9, 2014 10:11 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/4] Link Bonding Library > > On Wed Jun 04 16:18:53 +0100 2014, Declan Doherty wrote: > > - Broadcast TX burst broadcast bug fix > > - Add/remove slave behavior fix > > - Checkpatch fixes > > Declan, would you consider the following change to rte_bond.c? The two > header files from librte_cmdline don't seem to be necessary. > > Eric > > > --- a/lib/librte_bond/rte_bond.c > +++ b/lib/librte_bond/rte_bond.c > @@ -44,9 +44,6 @@ > #include > #include > > -#include > -#include > - > #include "rte_bond.h" > > static const char *driver_name = "Link Bonding PMD"; Hi Eric, I've cleaned up the includes in v3 of the patch, but the those headers are now actually required for the kvarg parsing. Regards Declan
[dpdk-dev] [PATCH v2 1/4] Link Bonding Library
> -Original Message- > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Thursday, June 5, 2014 4:16 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/4] Link Bonding Library > > On Wed, 4 Jun 2014 16:18:02 +0100 > declan.doherty at intel.com wrote: > > > From: Declan Doherty > > > > - Broadcast TX burst broadcast bug fix > > - Add/remove slave behavior fix > > - Checkpatch fixes > > > > Signed-off-by: Declan Doherty > > There are some pretty weak hash functions in there. > > What about using: > . Hi Stephen, I suppose the naming of these as hash functions is probably misleading, the aim is not to create a unique hash for each flow with minimal collisions but to help balance traffic flows across the slaves of the bonded device, and as we are doing a modulus calculation based on the number of slaves attached, which I believe will typically be in the 2-4 range, I'm not sure that a stronger hashing function will give a better balance of flows across the slaves. For example in the Linux kernel implementation of the link bonding driver the mac hashing only uses the last byte of src & dst mac addresses to calculate it's eth hash. /* L2 hash helper */ static inline u32 bond_eth_hash(struct sk_buff *skb) { struct ethhdr *data = (struct ethhdr *)skb->data; if (skb_headlen(skb) >= offsetof(struct ethhdr, h_proto)) return data->h_dest[5] ^ data->h_source[5]; return 0; } I'll investigate and see if a stronger hashing function will result in a better balancing of flows. -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[dpdk-dev] [PATCH v2 0/4] Link Bonding Library
> -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Thursday, May 29, 2014 12:34 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 0/4] Link Bonding Library > > On Thu, May 29, 2014 at 10:33:00AM +, Doherty, Declan wrote: > > -Original Message- > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > Sent: Wednesday, May 28, 2014 6:49 PM > > > To: Doherty, Declan > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH 0/4] Link Bonding Library > > > > > > On Wed, May 28, 2014 at 04:32:00PM +0100, declan.doherty at intel.com > wrote: > > > > From: Declan Doherty > > > > > > > > Initial release of Link Bonding Library (lib/librte_bond) with > > > > support for bonding modes : > > > > 0 - Round Robin > > > > 1 - Active Backup > > > > 2 - Balance l2 / l23 / l34 > > > > 3 - Broadcast > > > > > > > Why make this a separate library? That requires exposure of yet > > > another > API to applications. Instead, why > not write a PMD that can enslave > other PMD's and treat them all as a single interface? That way this > all > > works with the existing API. > > > > > > Neil > > > > Hi Neil, > > the link bonding device is essentially a software PMD, and as such > > supports > all the standard PMD APIs, the only new APIs which the link bonding > library introduces are for the control operations of the bonded > device which are currently unsupported by the standard PMD API. > Operations such as creating, adding/removing slaves, and configuring > the modes of operation of the device have no analogous APIs in the > current PMD API and required new ones to be created . > > Thats really only true in spirit, in the sense that this library > transmits and receives frames like a PMD does. In practice it doesn't > work and isn't architected the same way. You don't register the > driver using the same method as the other PMDs, which means that using > --vdev on the command line wont work for this type of device. It also > implies that applications have to be made specifically aware of the > fact that they are using a bonded interface (i.e. they need to call > the bonding setup routines to create the bond). I would > recommend: > > 1) Register the pmd using the PMD_DRIVER_REGISTER macro, like other > PMD's > 2) Use the kvargs library to support configuration via the --vdev > command line option, so bonds can be created administratively, rather > than just programatically > 3) Separate the command api from the PMD functionality into separate > libraries (use control mbufs to communicate configuration changes to > the pmd). This will allow users to dynamically load the pmd > functionality (without compile or run time linking requirements), and > then optionally use the programatic interface (or not if they want to > save memory) > > Regards > Neil > -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Thursday, June 5, 2014 12:04 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 0/4] Link Bonding Library > > On Wed, Jun 04, 2014 at 04:18:01PM +0100, declan.doherty at intel.com > wrote: > > From: Declan Doherty > > > > v2 patch additions, > > fix for tx burst broadcast, incrementing the reference count on each > > mbuf by the number of slaves - 1 add/remove slave behavior chnange > > to fix primary slave port assignment patchcheck code fixes > > > > > > > This doesn't address any of the comments I made previously. > Neil Hi Neil, sorry for not replying regarding this earlier, in relation to your recommendations 1 and 2, I'm currently working on the implementation of both and should have an additional patch within the next couple of days to add this functionality. Regarding your third recommendation, I have no problem splitting the library into separate API and PMD libraries although this does seem to break convention with the other non hw based pmd's, such as pmd_pcap, so would prefer not to go down that route. Also, I don't see the ability to dynamically load the pmd as justification for the use of a control mbufs interface, this would require that either additional queues be created to handle control messages, or that the data TX queues have the ability to identify and process control mbufs, and either of these options will have a performance hit, for the core handling the separate control messages or directly to the tx burst performance. I am not curre
[dpdk-dev] [PATCH v2 0/4] Link Bonding Library
Sorry for the double submission of the first 3 parts of this patch set. It was meant to be a test directed to my own email account. Regards Declan > -Original Message- > From: Doherty, Declan > Sent: Wednesday, June 4, 2014 4:19 PM > To: dev at dpdk.org; dev at dpdk.org > Cc: Doherty, Declan > Subject: [PATCH v2 0/4] Link Bonding Library > > From: Declan Doherty > > v2 patch additions, > fix for tx burst broadcast, incrementing the reference count on each mbuf by > the number of slaves - 1 > add/remove slave behavior chnange to fix primary slave port assignment > patchcheck code fixes > > Initial release of Link Bonding Library (lib/librte_bond) with support for > bonding modes : > 0 - Round Robin > 1 - Active Backup > 2 - Balance l2 / l23 / l34 > 3 - Broadcast > > patches split: > 1 - library + makefile changes > 2 - Unit test suite, including code to generate packet bursts for > testing rx and tx functionality of bonded device and a > virtual/stubbed out ethdev for use as slave ethdev in testing > 3 - Link bonding integration into testpmd, including : > - Includes the ability to create new bonded devices. > - Add /remove bonding slave devices. > - Interogate bonded device stats/configuration > - Change bonding modes and select balance transmit polices > 4 - Add Link Bonding Library to Doxygen > > > app/test-pmd/cmdline.c| 570 ++ > app/test-pmd/config.c |4 +- > app/test-pmd/parameters.c |4 +- > app/test-pmd/testpmd.c| 37 +- > app/test-pmd/testpmd.h|2 + > app/test/Makefile |3 + > app/test/commands.c |3 + > app/test/packet_burst_generator.c | 289 +++ > app/test/packet_burst_generator.h | 78 + > app/test/test.h |1 + > app/test/test_link_bonding.c | 3943 > + > app/test/virtual_pmd.c| 574 ++ > app/test/virtual_pmd.h| 74 + > config/common_bsdapp |5 + > config/common_linuxapp|5 + > doc/doxy-api-index.md |1 + > doc/doxy-api.conf |1 + > lib/Makefile |1 + > lib/librte_bond/Makefile | 28 + > lib/librte_bond/rte_bond.c| 1682 > lib/librte_bond/rte_bond.h| 228 +++ > mk/rte.app.mk |5 + > 22 files changed, 7531 insertions(+), 7 deletions(-) > create mode 100644 app/test/packet_burst_generator.c > create mode 100644 app/test/packet_burst_generator.h > create mode 100644 app/test/test_link_bonding.c > create mode 100644 app/test/virtual_pmd.c > create mode 100644 app/test/virtual_pmd.h > create mode 100644 lib/librte_bond/Makefile > create mode 100644 lib/librte_bond/rte_bond.c > create mode 100644 lib/librte_bond/rte_bond.h > > -- > 1.8.5.3 -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[dpdk-dev] [PATCH 1/4] Link Bonding Library
> -Original Message- > From: Shaw, Jeffrey B > Sent: Wednesday, May 28, 2014 5:55 PM > To: Doherty, Declan; dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH 1/4] Link Bonding Library > > Hi Declan, > I'm worried about one thing in "bond_ethdev_tx_broadcast()" related to > freeing of the broadcasted packets. > > > +static uint16_t > > +bond_ethdev_tx_broadcast(void *queue, struct rte_mbuf **bufs, > > +uint16_t nb_pkts) { > > + struct bond_dev_private *internals; > > + struct bond_tx_queue *bd_tx_q; > > + > > + uint8_t num_of_slaves; > > + uint8_t slaves[RTE_MAX_ETHPORTS]; > > + > > + uint16_t num_tx_total = 0; > > + > > + int i; > > + > > + bd_tx_q = (struct bond_tx_queue *)queue; > > + internals = bd_tx_q->dev_private; > > + > > + /* Copy slave list to protect against slave up/down changes during tx > > +* bursting */ > > + num_of_slaves = internals->active_slave_count; > > + memcpy(slaves, internals->active_slaves, > > + sizeof(internals->active_slaves[0]) * num_of_slaves); > > + > > + if (num_of_slaves < 1) > > + return 0; > > + > > + > > + for (i = 0; i < num_of_slaves; i++) { > > + num_tx_total += rte_eth_tx_burst(slaves[i], bd_tx_q- > >queue_id, > > + bufs, nb_pkts); > > + } > > + > > + return num_tx_total; > > +} > > + > > Transmitting the same buffers on all slaves will cause problems when the PMD > frees the mbufs for previously transmitted packets. > So if you broadcast 1 packet on 4 slaves, each of the 4 slaves will > (eventually) > have to free the same mbuf. Without updating the refcnt, you will end up > incorrectly freeing the same mbuf 3 extra times. > > Your test application does not catch this case since the max packets that are > tested is 320, which is less than the size of the Tx descriptor rings (512). > Try > testing with more packets and you should see some latent segmentation > faults. You should also see this with testpmd, if you try broadcasting enough > packets. > > > Thanks, > Jeff Hey Jeff, I'm not sure why I didn't see this in testing using on the test-pmd app, but it's obvious that this will cause issues. I'll add code to increment the refcnt in each mbuf by (num_of_slaves -1) in the bond_ethdev_tx_broadcast() function and also add a unit test to validate this in the next version of the patch. Thanks, Declan -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[dpdk-dev] [PATCH 0/4] Link Bonding Library
> From: Cao, Waterman > Sent: Thursday, May 29, 2014 4:23 AM > To: Doherty, Declan; dev at dpdk.org; dev at dpdk.org > Cc: Cao, Waterman > Subject: RE: [dpdk-dev] [PATCH 0/4] Link Bonding Library > > Hi declan, > > Do you send out Patch 1,2, 3 for link bonding? > Only see patch 0 and 4. > > Thanks > Waterman Hi Waterman, As far as I can see all the patches have been submitted, and are visible on the mailing list archives. Declan -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[dpdk-dev] [PATCH 0/4] Link Bonding Library
-Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > Sent: Wednesday, May 28, 2014 6:49 PM > To: Doherty, Declan > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 0/4] Link Bonding Library > > On Wed, May 28, 2014 at 04:32:00PM +0100, declan.doherty at intel.com wrote: > > From: Declan Doherty > > > > Initial release of Link Bonding Library (lib/librte_bond) with support > > for bonding modes : > > 0 - Round Robin > > 1 - Active Backup > > 2 - Balance l2 / l23 / l34 > > 3 - Broadcast > > > Why make this a separate library? That requires exposure of yet another API > to applications. Instead, why > not write a PMD that can enslave other PMD's > and treat them all as a single interface? That way this all > > works with > the existing API. > > Neil Hi Neil, the link bonding device is essentially a software PMD, and as such supports all the standard PMD APIs, the only new APIs which the link bonding library introduces are for the control operations of the bonded device which are currently unsupported by the standard PMD API. Operations such as creating, adding/removing slaves, and configuring the modes of operation of the device have no analogous APIs in the current PMD API and required new ones to be created . Declan -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.