From: Robert Sanford [mailto:rsanfo...@gmail.com] Sent: Monday, June 30, 2014 11:30 PM To: Doherty, Declan Cc: dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH v11 1/5] bond: new link bonding library
Hi Declan, On Sun, Jun 29, 2014 at 1:49 PM, Declan Doherty <declan.doherty at intel.com<mailto:declan.doherty at intel.com>> wrote: Initial release with support for Mode 0 - Round Robin Mode 1 - Active Backup Mode 2 - Balance -> Supports 3 transmit polices (layer 2, layer 2+3, layer 3+4) Mode 3 - Broadcast Signed-off-by: Declan Doherty <declan.doherty at intel.com<mailto:declan.doherty at intel.com>> --- config/common_bsdapp | 5 + config/common_linuxapp | 5 + doc/doxy-api-index.md<http://doxy-api-index.md> | 1 + doc/doxy-api.conf | 1 + lib/Makefile | 1 + lib/librte_pmd_bond/Makefile | 61 ++ lib/librte_pmd_bond/rte_eth_bond.h | 255 ++++++ lib/librte_pmd_bond/rte_eth_bond_api.c | 662 +++++++++++++++ lib/librte_pmd_bond/rte_eth_bond_args.c | 252 ++++++ lib/librte_pmd_bond/rte_eth_bond_pmd.c | 1212 ++++++++++++++++++++++++++++ lib/librte_pmd_bond/rte_eth_bond_private.h | 215 +++++ mk/rte.app.mk<http://rte.app.mk> | 4 + 12 files changed, 2674 insertions(+), 0 deletions(-) create mode 100644 lib/librte_pmd_bond/Makefile create mode 100644 lib/librte_pmd_bond/rte_eth_bond.h create mode 100644 lib/librte_pmd_bond/rte_eth_bond_api.c create mode 100644 lib/librte_pmd_bond/rte_eth_bond_args.c create mode 100644 lib/librte_pmd_bond/rte_eth_bond_pmd.c create mode 100644 lib/librte_pmd_bond/rte_eth_bond_private.h I see a potential problem with bond_ethdev_rx_burst( ). We could receive more packets than the caller asked for, and overrun the caller's rte_mbuf * array. The fix could be something like this: --- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c +++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c @@ -73,13 +73,15 @@ bond_ethdev_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) case BONDING_MODE_ROUND_ROBIN: case BONDING_MODE_BROADCAST: case BONDING_MODE_BALANCE: - for (i = 0; i < internals->active_slave_count; i++) { + for (i = 0; i < internals->active_slave_count && nb_pkts; i++) { /* Offset of pointer to *bufs increases as packets are received * from other slaves */ num_rx_slave = rte_eth_rx_burst(internals->active_slaves[i], bd_rx_q->queue_id, bufs + num_rx_total, nb_pkts); - if (num_rx_slave) + if (num_rx_slave) { num_rx_total += num_rx_slave; + nb_pkts -= num_rx_slave; + } } break; case BONDING_MODE_ACTIVE_BACKUP: -- Regards, Robert Hi Robert, yes I see this could be an issue, although this currently shouldn?t cause an issue, as the currently supported bonding modes only expected to receive data on one slave at any time, if they are being used as part of a ether channel but when we add further bonding modes are which receive data on multiple slaves this would definitely be a problem. Thomas, I?ve tested the above changes, is it possible to integrate this patch with the patchset for link bonding for the rc3. Thanks Declan
