Re: [dpdk-dev] [PATCH v11 1/5] bond: new link bonding library

["Doherty, Declan" <declan.doherty@intel.com>]

From: Robert Sanford [mailto:rsanford2@gmail.com]
Sent: Monday, June 30, 2014 11:30 PM
To: Doherty, Declan
Cc: dev@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@intel.com<mailto:declan.doherty@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@intel.com<mailto:declan.doherty@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