net/bonding: per-slave intermediate rx ring
Subsystems (2)
PATCH 1/1 net/bonding: per-slave intermediate rx ring
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
This small change improves performances of the bonding driver considerably. Vyatta has been using it for years in production.
--- drivers/net/bonding/rte_eth_bond_api.c | 13 ++++ drivers/net/bonding/rte_eth_bond_pmd.c | 87 +++++++++++++++++----- drivers/net/bonding/rte_eth_bond_private.h | 4 + 3 files changed, 85 insertions(+), 19 deletions(-)
drivers/net/bonding/rte_eth_bond_api.c +13 -0
@@ -524,6 +524,10 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, sizeof(*(rte_eth_devices[bonded_port_id].data->mac_addrs))); }if (internals->slave_count ==0 ) { +/* Remove any remaining packets in the receive ring */ +struct rte_mbuf *bufs[PMD_BOND_RECV_PKTS_PER_SLAVE ]; +unsigned j, count; + internals->rx_offload_capa =0 ; internals->tx_offload_capa =0 ; internals->rx_queue_offload_capa =0 ;
@@ -532,6 +536,15 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, internals->reta_size =0 ; internals->candidate_max_rx_pktlen =0 ; internals->max_rx_pktlen =0 ; + +do { + count =rte_ring_dequeue_burst (internals->rx_ring, + (void **)bufs, +PMD_BOND_RECV_PKTS_PER_SLAVE , +NULL ); +for (j =0 ; j < count; j++) +rte_pktmbuf_free (bufs[j]); + }while (count >0 ); }return 0 ; }
drivers/net/bonding/rte_eth_bond_pmd.c +68 -19
164 lines not on this page · raw message
drivers/net/bonding/rte_eth_bond_private.h +4 -0
@@ -26,6 +26,8 @@#define PMD_BOND_LSC_POLL_PERIOD_KVARG ("lsc_poll_period_ms")#define PMD_BOND_LINK_UP_PROP_DELAY_KVARG ("up_delay")#define PMD_BOND_LINK_DOWN_PROP_DELAY_KVARG ("down_delay") +#define PMD_BOND_RECV_RING_PKTS 512 +#define PMD_BOND_RECV_PKTS_PER_SLAVE 32#define PMD_BOND_XMIT_POLICY_LAYER2_KVARG ("l2")#define PMD_BOND_XMIT_POLICY_LAYER23_KVARG ("l23")
@@ -175,6 +177,8 @@ struct bond_dev_private {void *vlan_filter_bmpmem;/* enabled vlan filter bitmap */ struct rte_bitmap *vlan_filter_bmp; + +struct rte_ring *rx_ring; };extern const struct eth_dev_ops default_dev_ops;
Signature · 1 line
2.18.0
PATCH v2 1/1 net/bonding: per-slave intermediate rx ring
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
This small change improves performances of the bonding driver considerably. Vyatta has been using it for years in production.
---
v2: Fix some checkpatch warnings. Not all, as some of the 80-characters line
limit just look silly with excessive line wrappings.
drivers/net/bonding/rte_eth_bond_api.c | 13 +++ drivers/net/bonding/rte_eth_bond_pmd.c | 97 +++++++++++++++++----- drivers/net/bonding/rte_eth_bond_private.h | 4 + 3 files changed, 94 insertions(+), 20 deletions(-)
drivers/net/bonding/rte_eth_bond_api.c +13 -0
@@ -524,6 +524,10 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, sizeof(*(rte_eth_devices[bonded_port_id].data->mac_addrs))); }if (internals->slave_count ==0 ) { +/* Remove any remaining packets in the receive ring */ +struct rte_mbuf *bufs[PMD_BOND_RECV_PKTS_PER_SLAVE ]; +unsigned int j, count; + internals->rx_offload_capa =0 ; internals->tx_offload_capa =0 ; internals->rx_queue_offload_capa =0 ;
@@ -532,6 +536,15 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, internals->reta_size =0 ; internals->candidate_max_rx_pktlen =0 ; internals->max_rx_pktlen =0 ; + +do { + count =rte_ring_dequeue_burst (internals->rx_ring, + (void **)bufs, +PMD_BOND_RECV_PKTS_PER_SLAVE , +NULL ); +for (j =0 ; j < count; j++) +rte_pktmbuf_free (bufs[j]); + }while (count >0 ); }return 0 ; }
drivers/net/bonding/rte_eth_bond_pmd.c +77 -20
173 lines not on this page · raw message
drivers/net/bonding/rte_eth_bond_private.h +4 -0
@@ -26,6 +26,8 @@#define PMD_BOND_LSC_POLL_PERIOD_KVARG ("lsc_poll_period_ms")#define PMD_BOND_LINK_UP_PROP_DELAY_KVARG ("up_delay")#define PMD_BOND_LINK_DOWN_PROP_DELAY_KVARG ("down_delay") +#define PMD_BOND_RECV_RING_PKTS 512 +#define PMD_BOND_RECV_PKTS_PER_SLAVE 32#define PMD_BOND_XMIT_POLICY_LAYER2_KVARG ("l2")#define PMD_BOND_XMIT_POLICY_LAYER23_KVARG ("l23")
@@ -175,6 +177,8 @@ struct bond_dev_private {void *vlan_filter_bmpmem;/* enabled vlan filter bitmap */ struct rte_bitmap *vlan_filter_bmp; + +struct rte_ring *rx_ring; };extern const struct eth_dev_ops default_dev_ops;
Signature · 1 line
2.18.0
PATCH v3 1/1 net/bonding: per-slave intermediate rx ring
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
This small change improves performances of the bonding driver considerably. Vyatta has been using it for years in production.
---
v2 and v3: a couple more checkpatch warnings fixed
drivers/net/bonding/rte_eth_bond_api.c | 13 +++ drivers/net/bonding/rte_eth_bond_pmd.c | 98 +++++++++++++++++----- drivers/net/bonding/rte_eth_bond_private.h | 4 + 3 files changed, 95 insertions(+), 20 deletions(-)
drivers/net/bonding/rte_eth_bond_api.c +13 -0
@@ -524,6 +524,10 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, sizeof(*(rte_eth_devices[bonded_port_id].data->mac_addrs))); }if (internals->slave_count ==0 ) { +/* Remove any remaining packets in the receive ring */ +struct rte_mbuf *bufs[PMD_BOND_RECV_PKTS_PER_SLAVE ]; +unsigned int j, count; + internals->rx_offload_capa =0 ; internals->tx_offload_capa =0 ; internals->rx_queue_offload_capa =0 ;
@@ -532,6 +536,15 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, internals->reta_size =0 ; internals->candidate_max_rx_pktlen =0 ; internals->max_rx_pktlen =0 ; + +do { + count =rte_ring_dequeue_burst (internals->rx_ring, + (void **)bufs, +PMD_BOND_RECV_PKTS_PER_SLAVE , +NULL ); +for (j =0 ; j < count; j++) +rte_pktmbuf_free (bufs[j]); + }while (count >0 ); }return 0 ; }
drivers/net/bonding/rte_eth_bond_pmd.c +78 -20
174 lines not on this page · raw message
drivers/net/bonding/rte_eth_bond_private.h +4 -0
@@ -26,6 +26,8 @@#define PMD_BOND_LSC_POLL_PERIOD_KVARG ("lsc_poll_period_ms")#define PMD_BOND_LINK_UP_PROP_DELAY_KVARG ("up_delay")#define PMD_BOND_LINK_DOWN_PROP_DELAY_KVARG ("down_delay") +#define PMD_BOND_RECV_RING_PKTS 512 +#define PMD_BOND_RECV_PKTS_PER_SLAVE 32#define PMD_BOND_XMIT_POLICY_LAYER2_KVARG ("l2")#define PMD_BOND_XMIT_POLICY_LAYER23_KVARG ("l23")
@@ -175,6 +177,8 @@ struct bond_dev_private {void *vlan_filter_bmpmem;/* enabled vlan filter bitmap */ struct rte_bitmap *vlan_filter_bmp; + +struct rte_ring *rx_ring; };extern const struct eth_dev_ops default_dev_ops;
Signature · 1 line
2.18.0
PATCH v4 1/1 net/bonding: per-slave intermediate rx ring
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
This small change improves performances of the bonding driver considerably. Vyatta has been using it for years in production.
---
v2 and v3: fix checkpatch warnings
v4: add Eric's original signed-off-by from the Vyatta internal repo
drivers/net/bonding/rte_eth_bond_api.c | 13 +++ drivers/net/bonding/rte_eth_bond_pmd.c | 98 +++++++++++++++++----- drivers/net/bonding/rte_eth_bond_private.h | 4 + 3 files changed, 95 insertions(+), 20 deletions(-)
drivers/net/bonding/rte_eth_bond_api.c +13 -0
@@ -524,6 +524,10 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, sizeof(*(rte_eth_devices[bonded_port_id].data->mac_addrs))); }if (internals->slave_count ==0 ) { +/* Remove any remaining packets in the receive ring */ +struct rte_mbuf *bufs[PMD_BOND_RECV_PKTS_PER_SLAVE ]; +unsigned int j, count; + internals->rx_offload_capa =0 ; internals->tx_offload_capa =0 ; internals->rx_queue_offload_capa =0 ;
@@ -532,6 +536,15 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, internals->reta_size =0 ; internals->candidate_max_rx_pktlen =0 ; internals->max_rx_pktlen =0 ; + +do { + count =rte_ring_dequeue_burst (internals->rx_ring, + (void **)bufs, +PMD_BOND_RECV_PKTS_PER_SLAVE , +NULL ); +for (j =0 ; j < count; j++) +rte_pktmbuf_free (bufs[j]); + }while (count >0 ); }return 0 ; }
drivers/net/bonding/rte_eth_bond_pmd.c +78 -20
174 lines not on this page · raw message
drivers/net/bonding/rte_eth_bond_private.h +4 -0
@@ -26,6 +26,8 @@#define PMD_BOND_LSC_POLL_PERIOD_KVARG ("lsc_poll_period_ms")#define PMD_BOND_LINK_UP_PROP_DELAY_KVARG ("up_delay")#define PMD_BOND_LINK_DOWN_PROP_DELAY_KVARG ("down_delay") +#define PMD_BOND_RECV_RING_PKTS 512 +#define PMD_BOND_RECV_PKTS_PER_SLAVE 32#define PMD_BOND_XMIT_POLICY_LAYER2_KVARG ("l2")#define PMD_BOND_XMIT_POLICY_LAYER23_KVARG ("l23")
@@ -175,6 +177,8 @@ struct bond_dev_private {void *vlan_filter_bmpmem;/* enabled vlan filter bitmap */ struct rte_bitmap *vlan_filter_bmp; + +struct rte_ring *rx_ring; };extern const struct eth_dev_ops default_dev_ops;
Signature · 1 line
2.18.0
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Quoted 6 lines · Luca Boccassi
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
Quoted 8 lines
This small change improves performances of the bonding driver considerably. Vyatta has been using it for years in production.
Signed-off-by Eric Kinzie <[email protected]>Signed-off-by Luca Boccassi <[email protected]>
Quoted 295 lines
---
v2 and v3: fix checkpatch warnings
v4: add Eric's original signed-off-by from the Vyatta internal repodrivers/net/bonding/rte_eth_bond_api.c | 13 +++ drivers/net/bonding/rte_eth_bond_pmd.c | 98 +++++++++++++++++----- drivers/net/bonding/rte_eth_bond_private.h | 4 + 3 files changed, 95 insertions(+), 20 deletions(-)drivers/net/bonding/rte_eth_bond_api.c +4
-0@@ -524,6 +524,10 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, sizeof(*(rte_eth_devices[bonded_port_id].data->mac_addrs))); }if (internals->slave_count ==0 ) { +/* Remove any remaining packets in the receive ring */ +struct rte_mbuf *bufs[PMD_BOND_RECV_PKTS_PER_SLAVE ]; +unsigned int j, count; + internals->rx_offload_capa =0 ;internals->tx_offload_capa = 0; internals->rx_queue_offload_capa = 0;drivers/net/bonding/rte_eth_bond_api.c +9
-0@@ -532,6 +536,15 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, internals->reta_size =0 ; internals->candidate_max_rx_pktlen =0 ; internals->max_rx_pktlen =0 ; + +do { + count =rte_ring_dequeue_burst (internals->rx_ring, + (void **)bufs, +PMD_BOND_RECV_PKTS_PER_SLAVE , +NULL ); +for (j =0 ; j < count; j++) +rte_pktmbuf_free (bufs[j]); + }while (count >0 ); }return 0 ;}
drivers/net/bonding/rte_eth_bond_pmd.c +5
-0@@ -18,6 +18,8 @@#include <rte_alarm.h> #include <rte_cycles.h> #include <rte_string_fns.h> +#include <rte_errno.h> +#include <rte_lcore.h> #include "rte_eth_bond.h" #include "rte_eth_bond_private.h" @@ -402,10 +404,15 @@ bond_ethdev_rx_burst_8023ad(void *queue, structrte_mbuf **bufs, struct bond_dev_private *internals = bd_rx_q->dev_private;struct ether_addr bond_mac; +unsigned int rx_ring_avail =rte_ring_free_count (internals->rx_ring); +struct rte_mbuf *mbuf_bounce[PMD_BOND_RECV_PKTS_PER_SLAVE ]; +struct ether_hdr *hdr;const uint16_t ether_type_slow_be =rte_be_to_cpu_16 (ETHER_TYPE_SLOW );uint16_t num_rx_total =0 ;/* Total number of received packets */
drivers/net/bonding/rte_eth_bond_pmd.c +5
-0+uint16_t num_rx_slave; +uint16_t num_enq_slave;uint16_t slaves[RTE_MAX_ETHPORTS ];uint16_t slave_count, idx;@@ -414,6 +421,9 @@ bond_ethdev_rx_burst_8023ad(void *queue, structrte_mbuf **bufs, uint8_t i, j, k;uint8_t subtype; +if (rx_ring_avail <PMD_BOND_RECV_PKTS_PER_SLAVE ) + goto dequeue; +rte_eth_macaddr_get (internals->port_id, &bond_mac);/* Copy slave list to protect against slave up/down changes during tx
* bursting */drivers/net/bonding/rte_eth_bond_pmd.c +16
-11@@ -426,62 +436,96 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, internals->active_slave =0 ; idx =0 ; } - for (i = 0; i < slave_count && num_rx_total < nb_pkts; i++) { - j = num_rx_total; +for (i =0 ; i < slave_count && num_rx_total < rx_ring_avail; i++) { + j =0 ; collecting =ACTOR_STATE (&mode_8023ad_ports[slaves[idx]],COLLECTING );/* Read packets from this slave */ - num_rx_total += rte_eth_rx_burst(slaves[idx], bd_rx_q->queue_id, - &bufs[num_rx_total], nb_pkts - num_rx_total); +if (unlikely (rx_ring_avail - num_rx_total < +PMD_BOND_RECV_PKTS_PER_SLAVE )) +continue ; + num_rx_slave =rte_eth_rx_burst (slaves[idx], bd_rx_q->queue_id, + mbuf_bounce,PMD_BOND_RECV_PKTS_PER_SLAVE ); - for (k = j; k < 2 && k < num_rx_total; k++) - rte_prefetch0(rte_pktmbuf_mtod(bufs[k], void *)); +for (k = j; k <2 && k < num_rx_slave; k++) +rte_prefetch0 (rte_pktmbuf_mtod (mbuf_bounce[k], void *));/* Handle slow protocol packets. */ - while (j < num_rx_total) { +while (j < num_rx_slave) {/* If packet is not pure L2 and is known, skip it */ - if ((bufs[j]->packet_type & ~RTE_PTYPE_L2_ETHER) != 0) { +if ((mbuf_bounce[j]->packet_type & ~RTE_PTYPE_L2_ETHER ) !=0 ) { j++;continue ; } - if (j + 3 < num_rx_total) - rte_prefetch0(rte_pktmbuf_mtod(bufs[j + 3], void *)); +if (j +3 < num_rx_slave) +rte_prefetch0 (rte_pktmbuf_mtod (mbuf_bounce[j +3 ], + void *)); - hdr = rte_pktmbuf_mtod(bufs[j], struct ether_hdr *); + hdr =rte_pktmbuf_mtod (mbuf_bounce[j], + struct ether_hdr *); subtype = ((struct slow_protocol_frame *)hdr)->slow_protocol.subtype;/* Remove packet from array if it is slow packet or slave is not
* in collecting state or bonding interface is not
in promiscuous
* mode and packet address does not match. */
- if (unlikely(is_lacp_packets(hdr->ether_type,
subtype, bufs[j]) ||drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ if (unlikely(is_lacp_packets(hdr->ether_type, + subtype,mbuf_bounce[j]) ||
!collecting || (!promisc &&!is_multicast_ether_addr(&hdr->d_addr) &&
!is_same_ether_addr(&bond_mac,
&hdr->d_addr)))) {if (hdr->ether_type == ether_type_slow_be) { bond_mode_8023ad_handle_slow_pkt(- internals, slaves[idx],
bufs[j]);drivers/net/bonding/rte_eth_bond_pmd.c +3
-1+ internals, slaves[idx], + mbuf_bounce[j]); }else -rte_pktmbuf_free (bufs[j]); +rte_pktmbuf_free (mbuf_bounce[j]);/* Packet is managed by mode 4 or dropped, shift the array */
drivers/net/bonding/rte_eth_bond_pmd.c +0
-3- num_rx_total--; -if (j < num_rx_total) { - memmove(&bufs[j], &bufs[j + 1],
sizeof(bufs[0])*drivers/net/bonding/rte_eth_bond_pmd.c +15
-2- (num_rx_total - j)); + num_rx_slave--; +if (j < num_rx_slave) { +memmove (&mbuf_bounce[j], + &mbuf_bounce[j +1 ], +sizeof (mbuf_bounce[0 ]) * + (num_rx_slave - j)); } - }else + }else { j++; + } } + +if (num_rx_slave >0 ) { +if (mbuf_bounce[0 ] ==NULL ) +RTE_LOG (ERR ,PMD ,"%s: Enqueue a NULL??\n" , + __func__); + + num_enq_slave =rte_ring_enqueue_burst(internals->rx_ring,
+ (void
**)mbuf_bounce,
+num_rx_slave,drivers/net/bonding/rte_eth_bond_pmd.c +8
-0+ NULL); + +if (num_enq_slave < num_rx_slave) { +RTE_LOG (ERR ,PMD , +"%s: failed to enqueue %u packets" , + __func__, + (num_rx_slave - num_enq_slave)); + for (j = num_enq_slave; j < num_rx_slave;j++)
drivers/net/bonding/rte_eth_bond_pmd.c +8
-1+rte_pktmbuf_free (mbuf_bounce[j]); + } + num_rx_total += num_enq_slave; + } +if (unlikely (++idx == slave_count)) idx =0 ; } internals->active_slave = idx; -return num_rx_total; +dequeue: +return rte_ring_dequeue_burst (internals->rx_ring, (void **)bufs, + nb_pkts,NULL ); } #if defined(RTE_LIBRTE_BOND_DEBUG_ALB) ||
defined(RTE_LIBRTE_BOND_DEBUG_ALB_L1)drivers/net/bonding/rte_eth_bond_pmd.c +1
-0@@ -3065,6 +3109,7 @@ bond_alloc(struct rte_vdev_device *dev, uint8_t mode)struct bond_dev_private *internals =NULL ;struct rte_eth_dev *eth_dev =NULL ;uint32_t vlan_filter_bmp_size; +char mem_name[RTE_ETH_NAME_MAX_LEN ];/* now do all data allocation - for eth_dev structure, dummy pci driver
* and internal (private) datadrivers/net/bonding/rte_eth_bond_pmd.c +5
-0@@ -3129,6 +3174,19 @@ bond_alloc(struct rte_vdev_device *dev, uint8_t mode) TAILQ_INIT(&internals->flow_list); internals->flow_isolated_valid =0 ; +snprintf (mem_name,RTE_DIM (mem_name),"bond_%u_rx" , internals->port_id); + internals->rx_ring =rte_ring_lookup (mem_name); +if (internals->rx_ring ==NULL ) { + internals->rx_ring = rte_ring_create(mem_name, +rte_align32pow2(PMD_BOND_RECV_RING_PKTS *
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ rte_lcore_count()), + socket_id, 0); +if (internals->rx_ring ==NULL ) + rte_panic("%s: Failed to create rx ring '%s':%s\n", name,
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ mem_name, rte_strerror(rte_errno)); + } + +/* Set mode 4 default configuration */ bond_mode_8023ad_setup (eth_dev,NULL );if (bond_ethdev_mode_set (eth_dev, mode)) {drivers/net/bonding/rte_eth_bond_private.h +2
-0@@ -26,6 +26,8 @@#define PMD_BOND_LSC_POLL_PERIOD_KVARG ("lsc_poll_period_ms")#define PMD_BOND_LINK_UP_PROP_DELAY_KVARG ("up_delay")#define PMD_BOND_LINK_DOWN_PROP_DELAY_KVARG ("down_delay") +#define PMD_BOND_RECV_RING_PKTS 512 +#define PMD_BOND_RECV_PKTS_PER_SLAVE 32#define PMD_BOND_XMIT_POLICY_LAYER2_KVARG ("l2")#define PMD_BOND_XMIT_POLICY_LAYER23_KVARG ("l23")@@ -175,6 +177,8 @@ struct bond_dev_private {void *vlan_filter_bmpmem;/* enabled vlan filter bitmap */
struct rte_bitmap *vlan_filter_bmp;drivers/net/bonding/rte_eth_bond_private.h +2
-1+ +struct rte_ring *rx_ring; };extern const struct eth_dev_ops default_dev_ops; --2.18.0
Hi
From: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Yes, I think it should be done at least to bond_ethdev_rx_burst_8023ad_fast_queue (should be easy) for now.
Quoted 7 lines · Luca Boccassi
Quoted 6 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
Luca,
Can you explain these requirements of ixgbe?
Did you check for other vendor PMDs? It may hurt performance there..
Quoted 321 lines
Quoted 9 lines
This small change improves performances of the bonding driver considerably. Vyatta has been using it for years in production.
Signed-off-by Eric Kinzie <[email protected]>Signed-off-by Luca Boccassi <[email protected]>Acked-by Chas Williams <[email protected]>Quoted 6 lines
---
v2 and v3: fix checkpatch warnings
v4: add Eric's original signed-off-by from the Vyatta internal repodrivers/net/bonding/rte_eth_bond_api.c | 13 +++ drivers/net/bonding/rte_eth_bond_pmd.c | 98 +++++++++++++++++-----
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 53 lines
drivers/net/bonding/rte_eth_bond_private.h | 4 + 3 files changed, 95 insertions(+), 20 deletions(-)drivers/net/bonding/rte_eth_bond_api.c +4
-0@@ -524,6 +524,10 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, sizeof(*(rte_eth_devices[bonded_port_id].data->mac_addrs))); }if (internals->slave_count ==0 ) { +/* Remove any remaining packets in the receive ring */ +struct rte_mbuf *bufs[PMD_BOND_RECV_PKTS_PER_SLAVE ]; +unsigned int j, count; + internals->rx_offload_capa =0 ;internals->tx_offload_capa = 0; internals->rx_queue_offload_capa = 0; @@ -532,6 +536,15 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, internals->reta_size = 0; internals->candidate_max_rx_pktlen = 0; internals->max_rx_pktlen = 0;drivers/net/bonding/rte_eth_bond_api.c +9
-0+ +do { + count =rte_ring_dequeue_burst (internals->rx_ring, + (void **)bufs, +PMD_BOND_RECV_PKTS_PER_SLAVE , +NULL ); +for (j =0 ; j < count; j++) +rte_pktmbuf_free (bufs[j]); + }while (count >0 ); }return 0 ; }drivers/net/bonding/rte_eth_bond_pmd.c +2
-0@@ -18,6 +18,8 @@#include <rte_alarm.h> #include <rte_cycles.h> #include <rte_string_fns.h> +#include <rte_errno.h> +#include <rte_lcore.h> #include "rte_eth_bond.h" #include "rte_eth_bond_private.h" @@ -402,10 +404,15 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, · line 410
Quoted 7 lines
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0rte_mbuf **bufs, struct bond_dev_private *internals = bd_rx_q->dev_private;struct ether_addr bond_mac; +unsigned int rx_ring_avail =rte_ring_free_count (internals->rx_ring); +struct rte_mbuf *mbuf_bounce[PMD_BOND_RECV_PKTS_PER_SLAVE];
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 13 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-0+struct ether_hdr *hdr;const uint16_t ether_type_slow_be =rte_be_to_cpu_16 (ETHER_TYPE_SLOW );uint16_t num_rx_total =0 ;/* Total number of received packets */
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+uint16_t num_rx_slave; +uint16_t num_enq_slave;uint16_t slaves[RTE_MAX_ETHPORTS ];uint16_t slave_count, idx;@@ -414,6 +421,9 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 12 lines
drivers/net/bonding/rte_eth_bond_pmd.c +3
-0rte_mbuf **bufs, uint8_t i, j, k;uint8_t subtype; +if (rx_ring_avail <PMD_BOND_RECV_PKTS_PER_SLAVE ) + goto dequeue; +rte_eth_macaddr_get (internals->port_id, &bond_mac);/* Copy slave list to protect against slave up/down changes during tx
* bursting */drivers/net/bonding/rte_eth_bond_pmd.c +0
-0@@ -426,62 +436,96 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
drivers/net/bonding/rte_eth_bond_pmd.c bond_alloc(struct rte_vdev_device *dev, uint8_t
Quoted 148 lines
drivers/net/bonding/rte_eth_bond_pmd.c +11
-7rte_mbuf **bufs, internals->active_slave =0 ; idx =0 ; } -for (i =0 ; i < slave_count && num_rx_total < nb_pkts; i++) { - j = num_rx_total; +for (i =0 ; i < slave_count && num_rx_total < rx_ring_avail; i++) { + j =0 ; collecting =ACTOR_STATE (&mode_8023ad_ports[slaves[idx]],COLLECTING );/* Read packets from this slave */ - num_rx_total +=rte_eth_rx_burst (slaves[idx], bd_rx_q->queue_id, - &bufs[num_rx_total], nb_pkts - num_rx_total); +if (unlikely (rx_ring_avail - num_rx_total < +PMD_BOND_RECV_PKTS_PER_SLAVE )) +continue ; + num_rx_slave =rte_eth_rx_burst (slaves[idx], bd_rx_q->queue_id, + mbuf_bounce, +PMD_BOND_RECV_PKTS_PER_SLAVE ); - for (k = j; k < 2 && k < num_rx_total; k++) -rte_prefetch0 (rte_pktmbuf_mtod(bufs [k],void *)); +for (k = j; k <2 && k < num_rx_slave; k++) +rte_prefetch0 (rte_pktmbuf_mtod (mbuf_bounce[k], void *));/* Handle slow protocol packets. */ -while (j < num_rx_total) { +while (j < num_rx_slave) {/* If packet is not pure L2 and is known, skip it */
- if ((bufs[j]->packet_type & ~RTE_PTYPE_L2_ETHER)
!= 0) {
+ if ((mbuf_bounce[j]->packet_type &~RTE_PTYPE_L2_ETHER) != 0) { j++; continue; }drivers/net/bonding/rte_eth_bond_pmd.c +0
-2-if (j +3 < num_rx_total) - rte_prefetch0(rte_pktmbuf_mtod(bufs[j +3], void *));
drivers/net/bonding/rte_eth_bond_pmd.c +4
-1+if (j +3 < num_rx_slave) +rte_prefetch0 (rte_pktmbuf_mtod (mbuf_bounce[j +3 ], + void + *)); - hdr = rte_pktmbuf_mtod(bufs[j], struct ether_hdr*);
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ hdr =rte_pktmbuf_mtod (mbuf_bounce[j], + struct ether_hdr *); subtype = ((struct slow_protocol_frame*)hdr)->slow_protocol.subtype;
/* Remove packet from array if it is slowpacket or slave is not
* in collecting state or bonding interface is
not in promiscuous
* mode and packet address does not match. */
- if (unlikely(is_lacp_packets(hdr->ether_type,
subtype, bufs[j]) ||drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ if (unlikely(is_lacp_packets(hdr->ether_type, + subtype,mbuf_bounce[j]) ||
!collecting || (!promisc &&!is_multicast_ether_addr(&hdr->d_addr) &&
!is_same_ether_addr(&bond_mac,
&hdr->d_addr)))) {if (hdr->ether_type == ether_type_slow_be) { bond_mode_8023ad_handle_slow_pkt(- internals, slaves[idx],
bufs[j]);drivers/net/bonding/rte_eth_bond_pmd.c +4
-1+ internals, slaves[idx], + mbuf_bounce[j]); }else -rte_pktmbuf_free (bufs[j]); + +rte_pktmbuf_free (mbuf_bounce[j]);/* Packet is managed by mode 4 or dropped, shift the array */
drivers/net/bonding/rte_eth_bond_pmd.c +0
-3- num_rx_total--; -if (j < num_rx_total) { - memmove(&bufs[j], &bufs[j + 1],
sizeof(bufs[0])*drivers/net/bonding/rte_eth_bond_pmd.c +15
-2- (num_rx_total - j)); + num_rx_slave--; +if (j < num_rx_slave) { +memmove (&mbuf_bounce[j], + &mbuf_bounce[j +1 ], +sizeof (mbuf_bounce[0 ]) * + (num_rx_slave - j)); } - }else + }else { j++; + } } + +if (num_rx_slave >0 ) { +if (mbuf_bounce[0 ] ==NULL ) +RTE_LOG (ERR ,PMD ,"%s: Enqueue a NULL??\n" , + __func__); + + num_enq_slave =rte_ring_enqueue_burst(internals->rx_ring,
+ (void
**)mbuf_bounce,
+num_rx_slave,drivers/net/bonding/rte_eth_bond_pmd.c +9
-0+ NULL); + +if (num_enq_slave < num_rx_slave) { +RTE_LOG (ERR ,PMD , +"%s: failed to enqueue %u packets" , + __func__, + (num_rx_slave - num_enq_slave)); + for (j = num_enq_slave; j < + num_rx_slave;j++)
drivers/net/bonding/rte_eth_bond_pmd.c +8
-1+rte_pktmbuf_free (mbuf_bounce[j]); + } + num_rx_total += num_enq_slave; + } +if (unlikely (++idx == slave_count)) idx =0 ; } internals->active_slave = idx; -return num_rx_total; +dequeue: +return rte_ring_dequeue_burst (internals->rx_ring, (void **)bufs, + nb_pkts,NULL ); } #if defined(RTE_LIBRTE_BOND_DEBUG_ALB) ||
defined(RTE_LIBRTE_BOND_DEBUG_ALB_L1)drivers/net/bonding/rte_eth_bond_pmd.c +0
-0@@ -3065,6 +3109,7 @@ bond_alloc(struct rte_vdev_device *dev, uint8_tmode)
Quoted 38 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-0struct bond_dev_private *internals =NULL ;struct rte_eth_dev *eth_dev =NULL ;uint32_t vlan_filter_bmp_size; +char mem_name[RTE_ETH_NAME_MAX_LEN ];/* now do all data allocation - for eth_dev structure, dummy pci driver
* and internal (private) data @@ -3129,6 +3174,19 @@bond_alloc(struct rte_vdev_device *dev, uint8_t mode)TAILQ_INIT(&internals->flow_list); internals->flow_isolated_valid = 0;+ snprintf(mem_name,
RTE_DIM(mem_name), "bond_%u_rx",internals->port_id);drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ internals->rx_ring =rte_ring_lookup (mem_name); +if (internals->rx_ring ==NULL ) { + internals->rx_ring = rte_ring_create(mem_name, +rte_align32pow2(PMD_BOND_RECV_RING_PKTS *
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ rte_lcore_count()), + socket_id, 0); +if (internals->rx_ring ==NULL ) + rte_panic("%s: Failed to create rx ring '%s':%s\n", name,
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ mem_name, rte_strerror(rte_errno)); + } + +/* Set mode 4 default configuration */ bond_mode_8023ad_setup (eth_dev,NULL ); if (bond_ethdev_mode_set(eth_dev, mode)) { diff --git
a/drivers/net/bonding/rte_eth_bond_private.hb/drivers/net/bonding/rte_eth_bond_private.h
index43e0e448df..80261c6b14100644drivers/net/bonding/rte_eth_bond_private.h line 26
Quoted 1 line
drivers/net/bonding/rte_eth_bond_private.h +0
-0@@ -26,6 +26,8 @@#define PMD_BOND_LSC_POLL_PERIOD_KVARG ("
lsc_poll_period_ms")drivers/net/bonding/rte_eth_bond_private.h line 28
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_private.h +0
-0#define PMD_BOND_LINK_UP_PROP_DELAY_KVARG ("up_delay")#define PMD_BOND_LINK_DOWN_PROP_DELAY_KVARG ("
down_delay")Quoted 19 lines
drivers/net/bonding/rte_eth_bond_private.h +2
-0+#define PMD_BOND_RECV_RING_PKTS 512 +#define PMD_BOND_RECV_PKTS_PER_SLAVE 32#define PMD_BOND_XMIT_POLICY_LAYER2_KVARG ("l2")#define PMD_BOND_XMIT_POLICY_LAYER23_KVARG ("l23")@@ -175,6 +177,8 @@ struct bond_dev_private {void *vlan_filter_bmpmem;/* enabled vlan filter bitmap */
struct rte_bitmap *vlan_filter_bmp;drivers/net/bonding/rte_eth_bond_private.h +2
-1+ +struct rte_ring *rx_ring; };extern const struct eth_dev_ops default_dev_ops; --2.18.0
Quoted 24 lines · Matan Azrad
Hi
From: Chas Williams
Quoted 5 lines
This will need to be implemented for some of the other RX burst methods at
some point for other modes to see this performance improvement (with the
exception of active-backup).Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) for now.Quoted 11 lines
Quoted 8 lines · Luca Boccassi
During bond 802.3ad receive, a burst of packets is fetched from each
slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the
caller. The number of mbufs provided to each slave is sufficient to
meet the requirements of the ixgbe vector receive.
Quoted 3 lines
Luca,
Can you explain these requirements of ixgbe?
I think Chas knows this better than I do. Chas?
Quoted 1 line
Did you check for other vendor PMDs? It may hurt performance there..
Yeah we do support the appliance with more drivers - off the top of my head at the very least e1000, i40e, bnxt, and I'm pretty sure there are users that run with bonded virtio interfaces (yeah I know). No issues were reported as far as I'm aware.
Signature · 2 lines
Kind regards, Luca Boccassi
Quoted 11 lines · Matan Azrad
Hi
From: Chas Williams
Quoted 1 line
This will need to be implemented for some of the other RX burst methods
at
Quoted 2 lines
some point for other modes to see this performance improvement (with the exception of active-backup).
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) for now.
There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to give that some thought first.
Quoted 9 lines
Quoted 8 lines · Luca Boccassi
Quoted 6 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
Quoted 4 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or more.
Quoted 2 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
Empirically, this patch has been around of years in one form or another. No complaints other than "bonding is slow" which is why it was written.
Quoted 336 lines
Quoted 42 lines
Quoted 9 lines
This small change improves performances of the bonding driver considerably. Vyatta has been using it for years in production.
Signed-off-by Eric Kinzie <[email protected]>Signed-off-by Luca Boccassi <[email protected]>Acked-by Chas Williams <[email protected]>Quoted 6 lines
---
v2 and v3: fix checkpatch warnings
v4: add Eric's original signed-off-by from the Vyatta internal repodrivers/net/bonding/rte_eth_bond_api.c | 13 +++ drivers/net/bonding/rte_eth_bond_pmd.c | 98 +++++++++++++++++-----
Quoted 21 lines
drivers/net/bonding/rte_eth_bond_private.h | 4 + 3 files changed, 95 insertions(+), 20 deletions(-)drivers/net/bonding/rte_eth_bond_api.c +4
-0@@ -524,6 +524,10 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, sizeof(*(rte_eth_devices[bonded_port_id].data->mac_addrs))); }if (internals->slave_count ==0 ) { +/* Remove any remaining packets in the receive ring */ +struct rte_mbuf *bufs[PMD_BOND_RECV_PKTS_PER_SLAVE ]; +unsigned int j, count; + internals->rx_offload_capa =0 ;internals->tx_offload_capa = 0; internals->rx_queue_offload_capa = 0; @@ -532,6Quoted 8 lines
drivers/net/bonding/rte_eth_bond_api.c __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
Quoted 8 lines
+536,15 @@ __eth_bond_slave_remove_lock_free(uint16_t
bonded_port_id,internals->reta_size = 0; internals->candidate_max_rx_pktlen = 0; internals->max_rx_pktlen = 0;drivers/net/bonding/rte_eth_bond_api.c +3
-0+ + do { + count =rte_ring_dequeue_burst(internals->rx_ring,
Quoted 51 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 24 lines
drivers/net/bonding/rte_eth_bond_api.c +6
-0+ (void **)bufs, +PMD_BOND_RECV_PKTS_PER_SLAVE , + NULL); +for (j =0 ; j < count; j++) +rte_pktmbuf_free (bufs[j]); + }while (count >0 ); }return 0 ; }drivers/net/bonding/rte_eth_bond_pmd.c +2
-0@@ -18,6 +18,8 @@#include <rte_alarm.h> #include <rte_cycles.h> #include <rte_string_fns.h> +#include <rte_errno.h> +#include <rte_lcore.h> #include "rte_eth_bond.h" #include "rte_eth_bond_private.h" @@ -402,10 +404,15 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, · line 410
Quoted 7 lines
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0rte_mbuf **bufs, struct bond_dev_private *internals = bd_rx_q->dev_private;struct ether_addr bond_mac; +unsigned int rx_ring_avail =rte_ring_free_count (internals->rx_ring); +struct rte_mbuf *mbuf_bounce[PMD_BOND_RECV_PKTS_PER_SLAVE];
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 13 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-0+struct ether_hdr *hdr;const uint16_t ether_type_slow_be =rte_be_to_cpu_16 (ETHER_TYPE_SLOW );uint16_t num_rx_total =0 ;/* Total number of received packets */
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+uint16_t num_rx_slave; +uint16_t num_enq_slave;uint16_t slaves[RTE_MAX_ETHPORTS ];uint16_t slave_count, idx;@@ -414,6 +421,9 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
Quoted 4 lines
drivers/net/bonding/rte_eth_bond_pmd.c +0
-0rte_mbuf **bufs, uint8_t i, j, k;uint8_t subtype;Quoted 16 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 8 lines
drivers/net/bonding/rte_eth_bond_pmd.c +3
-0+if (rx_ring_avail <PMD_BOND_RECV_PKTS_PER_SLAVE ) + goto dequeue; +rte_eth_macaddr_get (internals->port_id, &bond_mac);/* Copy slave list to protect against slave up/down changes during tx
* bursting */drivers/net/bonding/rte_eth_bond_pmd.c +0
-0@@ -426,62 +436,96 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
Quoted 7 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-2rte_mbuf **bufs, internals->active_slave =0 ; idx =0 ; } -for (i =0 ; i < slave_count && num_rx_total < nb_pkts; i++) { - j = num_rx_total; + for (i = 0; i < slave_count && num_rx_total < rx_ring_avail;i++) {
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, · line 442
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-0+ j =0 ; collecting =ACTOR_STATE(&mode_8023ad_ports[slaves[idx]],
Quoted 15 lines
Quoted 15 lines
drivers/net/bonding/rte_eth_bond_pmd.c +6
-2COLLECTING);/* Read packets from this slave */ - num_rx_total +=rte_eth_rx_burst (slaves[idx], bd_rx_q->queue_id, - &bufs[num_rx_total], nb_pkts - num_rx_total); +if (unlikely (rx_ring_avail - num_rx_total < +PMD_BOND_RECV_PKTS_PER_SLAVE )) +continue ; + num_rx_slave =rte_eth_rx_burst (slaves[idx], bd_rx_q->queue_id, + mbuf_bounce, +PMD_BOND_RECV_PKTS_PER_SLAVE );Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, · line 443
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c +0
-2-for (k = j; k <2 && k < num_rx_total; k++) - rte_prefetch0(rte_pktmbuf_mtod(bufs[k], void*));
Quoted 11 lines
Quoted 11 lines
drivers/net/bonding/rte_eth_bond_pmd.c +3
-1+for (k = j; k <2 && k < num_rx_slave; k++) +rte_prefetch0 (rte_pktmbuf_mtod (mbuf_bounce[k], void *));/* Handle slow protocol packets. */ -while (j < num_rx_total) { +while (j < num_rx_slave) {/* If packet is not pure L2 and is known, skip it */
- if ((bufs[j]->packet_type &~RTE_PTYPE_L2_ETHER)
Quoted 16 lines
Quoted 16 lines
!= 0) {
+ if ((mbuf_bounce[j]->packet_type &~RTE_PTYPE_L2_ETHER) != 0) { j++; continue; }drivers/net/bonding/rte_eth_bond_pmd.c +0
-2-if (j +3 < num_rx_total) - rte_prefetch0(rte_pktmbuf_mtod(bufs[j +3], void *));
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+if (j +3 < num_rx_slave) +rte_prefetch0 (rte_pktmbuf_mtod (mbuf_bounce[j +3 ], + void + *));Quoted 1 line
Quoted 1 line
- hdr = rte_pktmbuf_mtod(bufs[j], struct
ether_hdrQuoted 18 lines
Quoted 18 lines
*);
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ hdr =rte_pktmbuf_mtod (mbuf_bounce[j], + struct ether_hdr *); subtype = ((struct slow_protocol_frame*)hdr)->slow_protocol.subtype;
/* Remove packet from array if it is slowpacket or slave is not
* in collecting state or bonding interface is
not in promiscuous
* mode and packet address does not match. */
- if (unlikely(is_lacp_packets(hdr->ether_type,
subtype, bufs[j]) ||drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ if (unlikely(is_lacp_packets(hdr->ether_type, + subtype,mbuf_bounce[j]) ||
!collecting || (!promisc &&Quoted 7 lines
Quoted 7 lines
!is_multicast_ether_addr(&hdr->d_addr) &&
!is_same_ether_addr(&bond_mac,
&hdr->d_addr)))) {if (hdr->ether_type ==
ether_type_slow_be) {bond_mode_8023ad_handle_slow_pkt(
Quoted 9 lines
Quoted 9 lines
- internals, slaves[idx],
bufs[j]);drivers/net/bonding/rte_eth_bond_pmd.c +4
-1+ internals, slaves[idx], + mbuf_bounce[j]); }else -rte_pktmbuf_free (bufs[j]); + +rte_pktmbuf_free (mbuf_bounce[j]);Quoted 12 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
Quoted 12 lines
/* Packet is managed by mode 4 ordropped, shift the array */
drivers/net/bonding/rte_eth_bond_pmd.c +0
-3- num_rx_total--; -if (j < num_rx_total) { - memmove(&bufs[j], &bufs[j + 1],
sizeof(bufs[0])*drivers/net/bonding/rte_eth_bond_pmd.c +5
-1- (num_rx_total - j)); + num_rx_slave--; +if (j < num_rx_slave) { +memmove (&mbuf_bounce[j], + &mbuf_bounce[j +1 ], + sizeof(mbuf_bounce[0])*
Quoted 11 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
Quoted 11 lines
drivers/net/bonding/rte_eth_bond_pmd.c +7
-1+ (num_rx_slave - j)); } - }else + }else { j++; + } } + +if (num_rx_slave >0 ) { +if (mbuf_bounce[0 ] ==NULL ) + RTE_LOG(ERR, PMD, "%s: Enqueue aNULL??\n",
Quoted 5 lines
Quoted 5 lines
drivers/net/bonding/rte_eth_bond_pmd.c +3
-0+ __func__); + + num_enq_slave =rte_ring_enqueue_burst(internals->rx_ring,
+ (voidQuoted 8 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
Quoted 8 lines
**)mbuf_bounce,
+num_rx_slave,drivers/net/bonding/rte_eth_bond_pmd.c +5
-0+ NULL); + +if (num_enq_slave < num_rx_slave) { + RTE_LOG(ERR, PMD, + "%s: failed to enqueue %upackets",
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ __func__, + (num_rx_slave -
num_enq_slave));Quoted 4 lines
Quoted 4 lines
+ for (j =
num_enq_slave; j <
+num_rx_slave;
j++)
+
rte_pktmbuf_free(mbuf_bounce[j]);Quoted 8 lines
Quoted 8 lines
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ } + num_rx_total += num_enq_slave; + } +if (unlikely (++idx == slave_count)) idx =0 ; }Quoted 4 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
Quoted 4 lines
internals->active_slave = idx;drivers/net/bonding/rte_eth_bond_pmd.c +2
-1-return num_rx_total; +dequeue: + return rte_ring_dequeue_burst(internals->rx_ring, (void**)bufs,
Quoted 20 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_alloc(struct rte_vdev_device *dev, uint8_t
Quoted 6 lines
+
nb_pkts, NULL);
}#if
defined(RTE_LIBRTE_BOND_DEBUG_ALB)||defined(RTE_LIBRTE_BOND_DEBUG_ALB_L1)drivers/net/bonding/rte_eth_bond_pmd.c +0
-0@@ -3065,6 +3109,7 @@ bond_alloc(struct rte_vdev_device *dev, uint8_tmode)
Quoted 13 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-0struct bond_dev_private *internals =NULL ;struct rte_eth_dev *eth_dev =NULL ;uint32_t vlan_filter_bmp_size; +char mem_name[RTE_ETH_NAME_MAX_LEN ];/* now do all data allocation - for eth_dev structure, dummy pci driver
* and internal (private) data @@ -3129,6 +3174,19 @@bond_alloc(struct rte_vdev_device *dev, uint8_t mode)TAILQ_INIT(&internals->flow_list); internals->flow_isolated_valid = 0;Quoted 8 lines
Quoted 8 lines
+ snprintf(mem_name,
RTE_DIM(mem_name), "bond_%u_rx",internals->port_id);drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ internals->rx_ring =rte_ring_lookup (mem_name); +if (internals->rx_ring ==NULL ) { + internals->rx_ring = rte_ring_create(mem_name, +rte_align32pow2(PMD_BOND_RECV_RING_PKTS *
+
rte_lcore_count()),Quoted 41 lines
Quoted 17 lines
drivers/net/bonding/rte_eth_bond_pmd.c +3
-0+ socket_id, 0); +if (internals->rx_ring ==NULL ) + rte_panic("%s: Failed to create rx ring '%s':%s\n", name,
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ mem_name, rte_strerror(rte_errno)); + } + +/* Set mode 4 default configuration */ bond_mode_8023ad_setup (eth_dev,NULL ); if (bond_ethdev_mode_set(eth_dev, mode)) { diff --git
a/drivers/net/bonding/rte_eth_bond_private.hb/drivers/net/bonding/rte_eth_bond_private.h
index43e0e448df..80261c6b14100644drivers/net/bonding/rte_eth_bond_private.h line 26
Quoted 1 line
drivers/net/bonding/rte_eth_bond_private.h +0
-0@@ -26,6 +26,8 @@#define PMD_BOND_LSC_POLL_PERIOD_KVARG ("
lsc_poll_period_ms")drivers/net/bonding/rte_eth_bond_private.h line 28
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_private.h +0
-0#define PMD_BOND_LINK_UP_PROP_DELAY_KVARG ("up_delay")#define PMD_BOND_LINK_DOWN_PROP_DELAY_KVARG ("
down_delay")Quoted 19 lines
drivers/net/bonding/rte_eth_bond_private.h +2
-0+#define PMD_BOND_RECV_RING_PKTS 512 +#define PMD_BOND_RECV_PKTS_PER_SLAVE 32#define PMD_BOND_XMIT_POLICY_LAYER2_KVARG ("l2")#define PMD_BOND_XMIT_POLICY_LAYER23_KVARG ("l23")@@ -175,6 +177,8 @@ struct bond_dev_private {void *vlan_filter_bmpmem;/* enabled vlan filter bitmap */
struct rte_bitmap *vlan_filter_bmp;drivers/net/bonding/rte_eth_bond_private.h +2
-1+ +struct rte_ring *rx_ring; };extern const struct eth_dev_ops default_dev_ops; --2.18.0
Hi Chas
From: Chas Williams
Quoted 9 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad <[email protected]> wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Quoted 6 lines
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) for now.There is some duplicated code between the various RX paths.
I would like to eliminate that as much as possible, so I was going to give that some thought first.
There is no reason to stay this function as is while its twin is changed.
Quoted 10 lines
Quoted 9 lines · Luca Boccassi
Quoted 6 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
Quoted 7 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or more.
So, why to do it in bond which is a generic driver for all the vendors PMDs,
If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets and to manage a ring by themselves.
Quoted 6 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
Quoted 330 lines
Empirically, this patch has been around of years in one form or another. No complaints other than "bonding is slow" which is why it was written.
Quoted 4 lines
Quoted 4 lines
This small change improves performances of the bonding driver considerably. Vyatta has been using it for years in production.
Quoted 16 lines
Quoted 5 lines
Signed-off-by Eric Kinzie <[email protected]>Signed-off-by Luca Boccassi <[email protected]>Acked-by Chas Williams <[email protected]>Quoted 6 lines
---
v2 and v3: fix checkpatch warnings
v4: add Eric's original signed-off-by from the Vyatta internal repodrivers/net/bonding/rte_eth_bond_api.c | 13 +++ drivers/net/bonding/rte_eth_bond_pmd.c | 98+++++++++++++++++--
Quoted 305 lines
---
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 53 lines
drivers/net/bonding/rte_eth_bond_private.h | 4 + 3 files changed, 95 insertions(+), 20 deletions(-)drivers/net/bonding/rte_eth_bond_api.c +4
-0@@ -524,6 +524,10 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, sizeof(*(rte_eth_devices[bonded_port_id].data->mac_addrs))); }if (internals->slave_count ==0 ) { +/* Remove any remaining packets in the receive ring */ +struct rte_mbuf *bufs[PMD_BOND_RECV_PKTS_PER_SLAVE ]; +unsigned int j, count; + internals->rx_offload_capa =0 ;internals->tx_offload_capa = 0; internals->rx_queue_offload_capa = 0; @@ -532,6 +536,15 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id, internals->reta_size = 0; internals->candidate_max_rx_pktlen = 0; internals->max_rx_pktlen = 0;drivers/net/bonding/rte_eth_bond_api.c +9
-0+ +do { + count =rte_ring_dequeue_burst (internals->rx_ring, + (void **)bufs, +PMD_BOND_RECV_PKTS_PER_SLAVE , +NULL ); +for (j =0 ; j < count; j++) +rte_pktmbuf_free (bufs[j]); + }while (count >0 ); }return 0 ; }drivers/net/bonding/rte_eth_bond_pmd.c +2
-0@@ -18,6 +18,8 @@#include <rte_alarm.h> #include <rte_cycles.h> #include <rte_string_fns.h> +#include <rte_errno.h> +#include <rte_lcore.h> #include "rte_eth_bond.h" #include "rte_eth_bond_private.h" @@ -402,10 +404,15 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, · line 410
Quoted 7 lines
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0rte_mbuf **bufs, struct bond_dev_private *internals = bd_rx_q->dev_private;struct ether_addr bond_mac; +unsigned int rx_ring_avail =rte_ring_free_count (internals->rx_ring); +struct rte_mbuf *mbuf_bounce[PMD_BOND_RECV_PKTS_PER_SLAVE];
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 13 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-0+struct ether_hdr *hdr;const uint16_t ether_type_slow_be =rte_be_to_cpu_16 (ETHER_TYPE_SLOW );uint16_t num_rx_total =0 ;/* Total number of received packets */
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+uint16_t num_rx_slave; +uint16_t num_enq_slave;uint16_t slaves[RTE_MAX_ETHPORTS ];uint16_t slave_count, idx;@@ -414,6 +421,9 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 12 lines
drivers/net/bonding/rte_eth_bond_pmd.c +3
-0rte_mbuf **bufs, uint8_t i, j, k;uint8_t subtype; +if (rx_ring_avail <PMD_BOND_RECV_PKTS_PER_SLAVE ) + goto dequeue; +rte_eth_macaddr_get (internals->port_id, &bond_mac);/* Copy slave list to protect against slave up/down changes during tx
* bursting */drivers/net/bonding/rte_eth_bond_pmd.c +0
-0@@ -426,62 +436,96 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
Quoted 151 lines
drivers/net/bonding/rte_eth_bond_pmd.c +12
-7rte_mbuf **bufs, internals->active_slave =0 ; idx =0 ; } -for (i =0 ; i < slave_count && num_rx_total < nb_pkts; i++) { - j = num_rx_total; +for (i =0 ; i < slave_count && num_rx_total < rx_ring_avail; i++) { + j =0 ; collecting =ACTOR_STATE (&mode_8023ad_ports[slaves[idx]],COLLECTING );/* Read packets from this slave */ - num_rx_total +=rte_eth_rx_burst (slaves[idx], bd_rx_q->queue_id, - &bufs[num_rx_total], nb_pkts - num_rx_total); +if (unlikely (rx_ring_avail - num_rx_total < +PMD_BOND_RECV_PKTS_PER_SLAVE )) +continue ; + num_rx_slave =rte_eth_rx_burst (slaves[idx], bd_rx_q->queue_id, + mbuf_bounce, +PMD_BOND_RECV_PKTS_PER_SLAVE ); - for (k = j; k < 2 && k < num_rx_total; k++) -rte_prefetch0 (rte_pktmbuf_mtod(bufs [k],void *)); +for (k = j; k <2 && k < num_rx_slave; k++) + +rte_prefetch0 (rte_pktmbuf_mtod (mbuf_bounce[k], void *));/* Handle slow protocol packets. */ -while (j < num_rx_total) { +while (j < num_rx_slave) {/* If packet is not pure L2 and is known, skip it */
- if ((bufs[j]->packet_type & ~RTE_PTYPE_L2_ETHER)
!= 0) {
+ if ((mbuf_bounce[j]->packet_type &~RTE_PTYPE_L2_ETHER) != 0) { j++; continue; }drivers/net/bonding/rte_eth_bond_pmd.c +0
-2-if (j +3 < num_rx_total) - rte_prefetch0(rte_pktmbuf_mtod(bufs[j +3], void *));
drivers/net/bonding/rte_eth_bond_pmd.c +4
-1+if (j +3 < num_rx_slave) +rte_prefetch0 (rte_pktmbuf_mtod (mbuf_bounce[j +3 ], + void + *)); - hdr = rte_pktmbuf_mtod(bufs[j], struct ether_hdr*);
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ hdr =rte_pktmbuf_mtod (mbuf_bounce[j], + struct ether_hdr *); subtype = ((struct slow_protocol_frame*)hdr)->slow_protocol.subtype;
/* Remove packet from array if it is slowpacket or slave is not
* in collecting state or bonding interface
is not in promiscuous
* mode and packet address does not match. */
- if (unlikely(is_lacp_packets(hdr->ether_type,
subtype, bufs[j]) ||drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ if (unlikely(is_lacp_packets(hdr->ether_type, + subtype,mbuf_bounce[j]) ||
!collecting || (!promisc &&!is_multicast_ether_addr(&hdr->d_addr) &&
!is_same_ether_addr(&bond_mac,
&hdr->d_addr)))) {if (hdr->ether_type == ether_type_slow_be) { bond_mode_8023ad_handle_slow_pkt(- internals, slaves[idx],
bufs[j]);drivers/net/bonding/rte_eth_bond_pmd.c +4
-1+ internals, slaves[idx], + mbuf_bounce[j]); }else -rte_pktmbuf_free (bufs[j]); + +rte_pktmbuf_free (mbuf_bounce[j]);/* Packet is managed by mode 4 or dropped, shift the array */
drivers/net/bonding/rte_eth_bond_pmd.c +0
-3- num_rx_total--; -if (j < num_rx_total) { - memmove(&bufs[j], &bufs[j + 1],
sizeof(bufs[0])*drivers/net/bonding/rte_eth_bond_pmd.c +15
-2- (num_rx_total - j)); + num_rx_slave--; +if (j < num_rx_slave) { +memmove (&mbuf_bounce[j], + &mbuf_bounce[j +1 ], +sizeof (mbuf_bounce[0 ]) * + (num_rx_slave - j)); } - }else + }else { j++; + } } + +if (num_rx_slave >0 ) { +if (mbuf_bounce[0 ] ==NULL ) +RTE_LOG (ERR ,PMD ,"%s: Enqueue a NULL??\n" , + __func__); + + num_enq_slave =rte_ring_enqueue_burst(internals->rx_ring,
+ (void
**)mbuf_bounce,
+num_rx_slave,drivers/net/bonding/rte_eth_bond_pmd.c +10
-0+ + NULL); + +if (num_enq_slave < num_rx_slave) { +RTE_LOG (ERR ,PMD , +"%s: failed to enqueue %u packets" , + __func__, + (num_rx_slave - num_enq_slave)); + for (j = num_enq_slave; j < + num_rx_slave;j++)
drivers/net/bonding/rte_eth_bond_pmd.c +8
-1+rte_pktmbuf_free (mbuf_bounce[j]); + } + num_rx_total += num_enq_slave; + } +if (unlikely (++idx == slave_count)) idx =0 ; } internals->active_slave = idx; -return num_rx_total; +dequeue: +return rte_ring_dequeue_burst (internals->rx_ring, (void **)bufs, + nb_pkts,NULL ); } #if defined(RTE_LIBRTE_BOND_DEBUG_ALB) ||
defined(RTE_LIBRTE_BOND_DEBUG_ALB_L1)drivers/net/bonding/rte_eth_bond_pmd.c +0
-0@@ -3065,6 +3109,7 @@ bond_alloc(struct rte_vdev_device *dev,Quoted 1 line
uint8_tmode)
Quoted 38 lines
struct bond_dev_private *internals = NULL; struct rte_eth_dev *eth_dev = NULL; uint32_t vlan_filter_bmp_size;+ char mem_name[RTE_ETH_NAME_MAX_LEN];
/* now do all data allocation - for eth_dev structure, dummypci driver
* and internal (private) data @@ -3129,6 +3174,19 @@bond_alloc(struct rte_vdev_device *dev, uint8_t mode)TAILQ_INIT(&internals->flow_list); internals->flow_isolated_valid = 0;+ snprintf(mem_name,
RTE_DIM(mem_name), "bond_%u_rx",internals->port_id);drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ internals->rx_ring =rte_ring_lookup (mem_name); +if (internals->rx_ring ==NULL ) { + internals->rx_ring = rte_ring_create(mem_name, +rte_align32pow2(PMD_BOND_RECV_RING_PKTS *
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ rte_lcore_count()), + socket_id, 0); +if (internals->rx_ring ==NULL ) + rte_panic("%s: Failed to create rx ring '%s':%s\n", name,
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ mem_name, rte_strerror(rte_errno)); + } + +/* Set mode 4 default configuration */ bond_mode_8023ad_setup (eth_dev,NULL ); if (bond_ethdev_mode_set(eth_dev, mode)) { diff --git
a/drivers/net/bonding/rte_eth_bond_private.hb/drivers/net/bonding/rte_eth_bond_private.h
index43e0e448df..80261c6b14100644drivers/net/bonding/rte_eth_bond_private.h line 26
Quoted 1 line
drivers/net/bonding/rte_eth_bond_private.h +0
-0@@ -26,6 +26,8 @@#define PMD_BOND_LSC_POLL_PERIOD_KVARG ("
lsc_poll_period_ms")drivers/net/bonding/rte_eth_bond_private.h line 28
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_private.h +0
-0#define PMD_BOND_LINK_UP_PROP_DELAY_KVARG ("up_delay")#define PMD_BOND_LINK_DOWN_PROP_DELAY_KVARG ("
down_delay")Quoted 19 lines
drivers/net/bonding/rte_eth_bond_private.h +2
-0+#define PMD_BOND_RECV_RING_PKTS 512 +#define PMD_BOND_RECV_PKTS_PER_SLAVE 32#define PMD_BOND_XMIT_POLICY_LAYER2_KVARG ("l2")#define PMD_BOND_XMIT_POLICY_LAYER23_KVARG ("l23")@@ -175,6 +177,8 @@ struct bond_dev_private {void *vlan_filter_bmpmem;/* enabled vlan filter bitmap */
struct rte_bitmap *vlan_filter_bmp;drivers/net/bonding/rte_eth_bond_private.h +2
-1+ +struct rte_ring *rx_ring; };extern const struct eth_dev_ops default_dev_ops; --2.18.0
Quoted 15 lines · Matan Azrad
Hi Chas
From: Chas Williams
Quoted 12 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad <[email protected]> wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) for now.
Quoted 7 lines
Quoted 2 lines
There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to
give that
Quoted 1 line
some thought first.
There is no reason to stay this function as is while its twin is changed.
Unfortunately, this is all the patch I have at this time.
Quoted 20 lines
Quoted 4 lines
Quoted 4 lines
On Thu, Aug 16, 2018 at 9:32 AM Luca Boccassi <[email protected]>
Quoted 7 lines · wrote:
Quoted 6 lines
Quoted 6 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
Quoted 5 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that
are
Quoted 2 lines
more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or
Quoted 8 lines
more.
So, why to do it in bond which is a generic driver for all the vendors PMDs,
If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets
and to manage a ring by themselves.
The drawback of the ring is some additional latency on the receive path. In testing, the additional latency hasn't been an issue for bonding. The bonding PMD has a fair bit of overhead associated with the RX and TX path calculations. Most applications can just arrange to call the RX path with a sufficiently large receive. Bonding can't do this.
Quoted 12 lines
Quoted 4 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are
typically
Quoted 3 lines
reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
Quoted 350 lines
Quoted 1 line
Empirically, this patch has been around of years in one form or
another. No
Quoted 36 lines
complaints other than "bonding is slow" which is why it was written.
Quoted 4 lines
Quoted 4 lines
This small change improves performances of the bonding driver considerably. Vyatta has been using it for years in production.
Quoted 16 lines
Quoted 5 lines
Signed-off-by Eric Kinzie <[email protected]>Signed-off-by Luca Boccassi <[email protected]>Acked-by Chas Williams <[email protected]>Quoted 6 lines
---
v2 and v3: fix checkpatch warnings
v4: add Eric's original signed-off-by from the Vyatta internal repodrivers/net/bonding/rte_eth_bond_api.c | 13 +++ drivers/net/bonding/rte_eth_bond_pmd.c | 98+++++++++++++++++--
Quoted 12 lines
---
Quoted 11 lines
drivers/net/bonding/rte_eth_bond_private.h | 4 + 3 files changed, 95 insertions(+), 20 deletions(-)drivers/net/bonding/rte_eth_bond_api.c +0
-0@@ -524,6 +524,10 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,Quoted 4 lines
Quoted 4 lines
drivers/net/bonding/rte_eth_bond_api.c __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
Quoted 4 lines
drivers/net/bonding/rte_eth_bond_api.c +1
-0sizeof(*(rte_eth_devices[bonded_port_id].data->mac_addrs))); }if (internals->slave_count ==0 ) { +/* Remove any remaining packets in the receive ring */
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
drivers/net/bonding/rte_eth_bond_api.c +3
-0+struct rte_mbuf *bufs[PMD_BOND_RECV_PKTS_PER_SLAVE ]; +unsigned int j, count; + internals->rx_offload_capa =0 ;internals->tx_offload_capa = 0; internals->rx_queue_offload_capa = 0; @@ -532,6Quoted 8 lines
Quoted 8 lines
drivers/net/bonding/rte_eth_bond_api.c __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
Quoted 8 lines
+536,15 @@ __eth_bond_slave_remove_lock_free(uint16_t
bonded_port_id,internals->reta_size = 0; internals->candidate_max_rx_pktlen = 0; internals->max_rx_pktlen = 0;drivers/net/bonding/rte_eth_bond_api.c +3
-0+ + do { + count =rte_ring_dequeue_burst(internals->rx_ring,
Quoted 60 lines
Quoted 60 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 24 lines
drivers/net/bonding/rte_eth_bond_api.c +6
-0+ (void **)bufs, +PMD_BOND_RECV_PKTS_PER_SLAVE , + NULL); +for (j =0 ; j < count; j++) +rte_pktmbuf_free (bufs[j]); + }while (count >0 ); }return 0 ; }drivers/net/bonding/rte_eth_bond_pmd.c +2
-0@@ -18,6 +18,8 @@#include <rte_alarm.h> #include <rte_cycles.h> #include <rte_string_fns.h> +#include <rte_errno.h> +#include <rte_lcore.h> #include "rte_eth_bond.h" #include "rte_eth_bond_private.h" @@ -402,10 +404,15 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, · line 410
Quoted 7 lines
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0rte_mbuf **bufs, struct bond_dev_private *internals = bd_rx_q->dev_private;struct ether_addr bond_mac; +unsigned int rx_ring_avail =rte_ring_free_count (internals->rx_ring); +struct rte_mbuf *mbuf_bounce[PMD_BOND_RECV_PKTS_PER_SLAVE];
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 13 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-0+struct ether_hdr *hdr;const uint16_t ether_type_slow_be =rte_be_to_cpu_16 (ETHER_TYPE_SLOW );uint16_t num_rx_total =0 ;/* Total number of received packets */
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+uint16_t num_rx_slave; +uint16_t num_enq_slave;uint16_t slaves[RTE_MAX_ETHPORTS ];uint16_t slave_count, idx;@@ -414,6 +421,9 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue,
Quoted 12 lines
drivers/net/bonding/rte_eth_bond_pmd.c +3
-0rte_mbuf **bufs, uint8_t i, j, k;uint8_t subtype; +if (rx_ring_avail <PMD_BOND_RECV_PKTS_PER_SLAVE ) + goto dequeue; +rte_eth_macaddr_get (internals->port_id, &bond_mac);/* Copy slave list to protect against slave up/down changes during tx
* bursting */drivers/net/bonding/rte_eth_bond_pmd.c +0
-0@@ -426,62 +436,96 @@ bond_ethdev_rx_burst_8023ad(void *queue,struct
Quoted 7 lines
Quoted 7 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
Quoted 7 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-2rte_mbuf **bufs, internals->active_slave =0 ; idx =0 ; } -for (i =0 ; i < slave_count && num_rx_total < nb_pkts; i++) { - j = num_rx_total; + for (i = 0; i < slave_count && num_rx_total < rx_ring_avail;i++) {
Quoted 2 lines
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, · line 442
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-0+ j =0 ; collecting =ACTOR_STATE(&mode_8023ad_ports[slaves[idx]],
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
drivers/net/bonding/rte_eth_bond_pmd.c +6
-2COLLECTING);/* Read packets from this slave */ - num_rx_total +=rte_eth_rx_burst (slaves[idx], bd_rx_q->queue_id, - &bufs[num_rx_total], nb_pkts - num_rx_total); +if (unlikely (rx_ring_avail - num_rx_total < +PMD_BOND_RECV_PKTS_PER_SLAVE )) +continue ; + num_rx_slave =rte_eth_rx_burst (slaves[idx], bd_rx_q->queue_id, + mbuf_bounce, +PMD_BOND_RECV_PKTS_PER_SLAVE );Quoted 2 lines
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, · line 443
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c +0
-2-for (k = j; k <2 && k < num_rx_total; k++) - rte_prefetch0(rte_pktmbuf_mtod(bufs[k], void*));
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
drivers/net/bonding/rte_eth_bond_pmd.c +3
-0+for (k = j; k <2 && k < num_rx_slave; k++) + +rte_prefetch0 (rte_pktmbuf_mtod (mbuf_bounce[k], void *));Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
drivers/net/bonding/rte_eth_bond_pmd.c +1
-1/* Handle slow protocol packets. */ -while (j < num_rx_total) { +while (j < num_rx_slave) {/* If packet is not pure L2 and is known, skip it */
- if ((bufs[j]->packet_type &~RTE_PTYPE_L2_ETHER)
Quoted 9 lines
Quoted 9 lines
drivers/net/bonding/rte_eth_bond_pmd.c
Quoted 9 lines
!= 0) {
+ if ((mbuf_bounce[j]->packet_type &~RTE_PTYPE_L2_ETHER) != 0) { j++; continue; }drivers/net/bonding/rte_eth_bond_pmd.c +0
-2-if (j +3 < num_rx_total) -rte_prefetch0(rte_pktmbuf_mtod(bufs[j +
Quoted 8 lines
Quoted 8 lines
drivers/net/bonding/rte_eth_bond_pmd.c
Quoted 8 lines
3], void *));
drivers/net/bonding/rte_eth_bond_pmd.c +4
-1+if (j +3 < num_rx_slave) +rte_prefetch0 (rte_pktmbuf_mtod (mbuf_bounce[j +3 ], + void + *)); - hdr = rte_pktmbuf_mtod(bufs[j], struct
ether_hdrQuoted 18 lines
Quoted 18 lines
Quoted 18 lines
*);
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ hdr =rte_pktmbuf_mtod (mbuf_bounce[j], + struct ether_hdr *); subtype = ((struct slow_protocol_frame*)hdr)->slow_protocol.subtype;
/* Remove packet from array if it is slowpacket or slave is not
* in collecting state or bonding interface
is not in promiscuous
* mode and packet address does not match. */
- if (unlikely(is_lacp_packets(hdr->ether_type,
subtype, bufs[j]) ||drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ if (unlikely(is_lacp_packets(hdr->ether_type, + subtype,mbuf_bounce[j]) ||
!collecting || (!promisc &&Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
!is_multicast_ether_addr(&hdr->d_addr) &&
!is_same_ether_addr(&bond_mac,
&hdr->d_addr)))) {if (hdr->ether_type ==
ether_type_slow_be) {bond_mode_8023ad_handle_slow_pkt(
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
- internals, slaves[idx],
bufs[j]);drivers/net/bonding/rte_eth_bond_pmd.c +4
-1+ internals, slaves[idx], + mbuf_bounce[j]); }else -rte_pktmbuf_free (bufs[j]); + +rte_pktmbuf_free (mbuf_bounce[j]);Quoted 5 lines
Quoted 5 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
Quoted 5 lines
/* Packet is managed by mode 4 ordropped, shift the array */
drivers/net/bonding/rte_eth_bond_pmd.c +0
-3- num_rx_total--; - if (j < num_rx_total) { - memmove(&bufs[j], &bufs[j +1],
Quoted 7 lines
Quoted 7 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, · line 499
Quoted 7 lines
sizeof(bufs[0])*drivers/net/bonding/rte_eth_bond_pmd.c +5
-1- (num_rx_total - j)); + num_rx_slave--; +if (j < num_rx_slave) { + memmove(&mbuf_bounce[j], + &mbuf_bounce[j + 1], +
sizeof(mbuf_bounce[0])*Quoted 11 lines
Quoted 11 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
Quoted 11 lines
drivers/net/bonding/rte_eth_bond_pmd.c +7
-1+ (num_rx_slave - j)); } - }else + }else { j++; + } } + +if (num_rx_slave >0 ) { +if (mbuf_bounce[0 ] ==NULL ) + RTE_LOG(ERR, PMD, "%s: Enqueue aNULL??\n",
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
drivers/net/bonding/rte_eth_bond_pmd.c +3
-0+ __func__); + + num_enq_slave =rte_ring_enqueue_burst(internals->rx_ring,
+ (void
**)mbuf_bounce,Quoted 8 lines
Quoted 8 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
Quoted 8 lines
+
num_rx_slave,drivers/net/bonding/rte_eth_bond_pmd.c +6
-0+ + NULL); + +if (num_enq_slave < num_rx_slave) { + RTE_LOG(ERR, PMD, + "%s: failed to enqueue %upackets",
Quoted 2 lines
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_pmd.c +2
-0+ __func__, + (num_rx_slave -
num_enq_slave));Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
+ for (j =
num_enq_slave; j <
+num_rx_slave;
j++)
+
rte_pktmbuf_free(mbuf_bounce[j]);Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ } + num_rx_total += num_enq_slave; + } +if (unlikely (++idx == slave_count)) idx =0 ; }Quoted 4 lines
Quoted 4 lines
drivers/net/bonding/rte_eth_bond_pmd.c bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
Quoted 4 lines
internals->active_slave = idx;drivers/net/bonding/rte_eth_bond_pmd.c +2
-1-return num_rx_total; +dequeue: + return rte_ring_dequeue_burst(internals->rx_ring, (void**)bufs,
Quoted 21 lines
Quoted 21 lines
Quoted 7 lines
+
nb_pkts, NULL);
}#if
defined(RTE_LIBRTE_BOND_DEBUG_ALB)||defined(RTE_LIBRTE_BOND_DEBUG_ALB_L1)drivers/net/bonding/rte_eth_bond_pmd.c +0
-0@@ -3065,6 +3109,7 @@ bond_alloc(struct rte_vdev_device *dev,
uint8_tmode)
Quoted 13 lines
struct bond_dev_private *internals = NULL; struct rte_eth_dev *eth_dev = NULL; uint32_t vlan_filter_bmp_size;+ char mem_name[RTE_ETH_NAME_MAX_LEN];
/* now do all data allocation - for eth_dev structure, dummypci driver
* and internal (private) data @@ -3129,6 +3174,19 @@bond_alloc(struct rte_vdev_device *dev, uint8_t mode)TAILQ_INIT(&internals->flow_list); internals->flow_isolated_valid = 0;Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
+ snprintf(mem_name,
RTE_DIM(mem_name), "bond_%u_rx",internals->port_id);drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ internals->rx_ring =rte_ring_lookup (mem_name); +if (internals->rx_ring ==NULL ) { + internals->rx_ring = rte_ring_create(mem_name, +rte_align32pow2(PMD_BOND_RECV_RING_PKTS *
+
rte_lcore_count()),Quoted 22 lines
Quoted 22 lines
Quoted 17 lines
drivers/net/bonding/rte_eth_bond_pmd.c +3
-0+ socket_id, 0); +if (internals->rx_ring ==NULL ) + rte_panic("%s: Failed to create rx ring '%s':%s\n", name,
drivers/net/bonding/rte_eth_bond_pmd.c +4
-0+ mem_name, rte_strerror(rte_errno)); + } + +/* Set mode 4 default configuration */ bond_mode_8023ad_setup (eth_dev,NULL ); if (bond_ethdev_mode_set(eth_dev, mode)) { diff --git
a/drivers/net/bonding/rte_eth_bond_private.hb/drivers/net/bonding/rte_eth_bond_private.h
index43e0e448df..80261c6b14100644drivers/net/bonding/rte_eth_bond_private.h line 26
Quoted 1 line
drivers/net/bonding/rte_eth_bond_private.h +0
-0@@ -26,6 +26,8 @@#define PMD_BOND_LSC_POLL_PERIOD_KVARG ("
lsc_poll_period_ms")drivers/net/bonding/rte_eth_bond_private.h line 28
Quoted 2 lines
drivers/net/bonding/rte_eth_bond_private.h +0
-0#define PMD_BOND_LINK_UP_PROP_DELAY_KVARG ("up_delay")#define PMD_BOND_LINK_DOWN_PROP_DELAY_KVARG ("
down_delay")Quoted 8 lines
Quoted 8 lines
drivers/net/bonding/rte_eth_bond_private.h struct bond_dev_private { · line 178
Quoted 8 lines
drivers/net/bonding/rte_eth_bond_private.h +2
-0+#define PMD_BOND_RECV_RING_PKTS 512 +#define PMD_BOND_RECV_PKTS_PER_SLAVE 32#define PMD_BOND_XMIT_POLICY_LAYER2_KVARG ("l2")#define PMD_BOND_XMIT_POLICY_LAYER23_KVARG ("l23")@@ -175,6 +177,8 @@ struct bond_dev_private {void *vlan_filter_bmpmem;/* enabled vlan filter
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
bitmap */
struct rte_bitmap *vlan_filter_bmp;drivers/net/bonding/rte_eth_bond_private.h +2
-1+ +struct rte_ring *rx_ring; };extern const struct eth_dev_ops default_dev_ops; --2.18.0
Hi Chas
From: Chas Williams
Quoted 43 lines · Matan Azrad
Hi Chas
From: Chas Williams
Quoted 9 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad <mailto:[email protected]> wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Quoted 6 lines
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) for now.There is some duplicated code between the various RX paths.
I would like to eliminate that as much as possible, so I was going to give that some thought first.There is no reason to stay this function as is while its twin is changed.
Unfortunately, this is all the patch I have at this time.
Quoted 10 lines
Quoted 9 lines · Luca Boccassi
Quoted 6 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
Quoted 7 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or more.
Quoted 6 lines
So, why to do it in bond which is a generic driver for all the vendors PMDs,
If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets and to manage a ring by themselves.The drawback of the ring is some additional latency on the receive path. In testing, the additional latency hasn't been an issue for bonding.
When bonding does processing slower it may be a bottleneck for the packet processing for some application.
Quoted 3 lines
The bonding PMD has a fair bit of overhead associated with the RX and TX path calculations. Most applications can just arrange to call the RX path with a sufficiently large receive. Bonding can't do this.
I didn't talk on application I talked on the slave PMDs,
The slave PMD can manage a ring by itself if it helps for its own performance. The bonding should not be oriented to specific PMDs.
Quoted 11 lines
Quoted 6 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
We should not do things worse than they are.
Quoted 78 lines · Matan Azrad
Hi Chas
From: Chas Williams
Quoted 61 lines
On Tue, Aug 21, 2018 at 11:43 AM Matan Azrad <mailto:matan@mellanox .com> wrote:
Hi ChasFrom: Chas Williams
Quoted 10 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad <mailto:matan@mellano x.com>
wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Quoted 7 lines
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) for now.There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to give that
some thought first.There is no reason to stay this function as is while its twin is changed.
Unfortunately, this is all the patch I have at this time.
Quoted 17 lines
Quoted 5 lines
On Thu, Aug 16, 2018 at 9:32 AM Luca Boccassi <mailto:bluca@deb ian.org> wrote:
Quoted 9 lines
Quoted 9 lines
During bond 802.3ad receive, a burst of packets is fetched from each
slave into a local array and appended to per-slave ring buffer.
Packets are taken from the head of the ring buffer and returned to
the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.Luca,
Quoted 8 lines
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are
more efficient (if not faster) taking advantage of some advanced CPU
instructions. I think you need to be receiving at least 32 packets or more.So, why to do it in bond which is a generic driver for all the vendors PMDs,
If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets
and to manage a ring by themselves.Quoted 4 lines
The drawback of the ring is some additional latency on the receive path.
In testing, the additional latency hasn't been an issue for bonding.When bonding does processing slower it may be a bottleneck for the packet processing for some application.
Quoted 5 lines
The bonding PMD has a fair bit of overhead associated with the RX and TX path
calculations. Most applications can just arrange to call the RX path
with a sufficiently large receive. Bonding can't do this.
Quoted 4 lines
I didn't talk on application I talked on the slave PMDs, The slave PMD can manage a ring by itself if it helps for its own performance.
The bonding should not be oriented to specific PMDs.
The issue though is that the performance problem is not with the individual PMDs - it's with bonding. There were no reports regarding the individual PMDs.
This comes from reports from customers from real world production deployments - the issue of bonding being too slow was raised multiple times. This patch addresses those issues, again in production deployments, where it's been used for years, to users and customers satisfaction.
So I'd like to share this improvement rather than keeping it private - because I'm nice that way :-P
Quoted 10 lines
Quoted 10 lines
Quoted 9 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically
reading almost up to the vector requirement. But if one slave has just a
single packet, then you can't vectorize on the next slave.
Quoted 7 lines
Quoted 5 lines
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
We should not do things worse than they are.
There were no reports that this made things worse. The feedback from production was that it made things better.
Signature · 2 lines
Kind regards, Luca Boccassi
Hi Luca
From: Luca Boccassi
Quoted 75 lines · Matan Azrad
Quoted 65 lines
Hi Chas
From: Chas Williams
Quoted 50 lines
On Tue, Aug 21, 2018 at 11:43 AM Matan Azrad <mailto:matan@mellanox .com> wrote:
Hi ChasFrom: Chas Williams
Quoted 10 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad <mailto:matan@mellano x.com>
wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Quoted 6 lines
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) for now.There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to give that some thought first.
There is no reason to stay this function as is while its twin is changed.
Unfortunately, this is all the patch I have at this time.
Quoted 13 lines
Quoted 5 lines
On Thu, Aug 16, 2018 at 9:32 AM Luca Boccassi <mailto:bluca@deb ian.org> wrote:
Quoted 7 lines
Quoted 7 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer.
Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.Quoted 8 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or more.
Quoted 8 lines
So, why to do it in bond which is a generic driver for all the vendors PMDs, If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets and to manage a ring by themselves.
The drawback of the ring is some additional latency on the receive path.
In testing, the additional latency hasn't been an issue for bonding.When bonding does processing slower it may be a bottleneck for the packet processing for some application.
Quoted 8 lines
Quoted 4 lines
The bonding PMD has a fair bit of overhead associated with the RX and TX path calculations. Most applications can just arrange to call the RX path with a sufficiently large receive. Bonding can't do this.
I didn't talk on application I talked on the slave PMDs, The slave PMD can manage a ring by itself if it helps for its own performance. The bonding should not be oriented to specific PMDs.
Quoted 7 lines
The issue though is that the performance problem is not with the individual PMDs - it's with bonding. There were no reports regarding the individual PMDs.
This comes from reports from customers from real world production deployments - the issue of bonding being too slow was raised multiple times. This patch addresses those issues, again in production deployments, where it's been used for years, to users and customers satisfaction.
From Chas I understood that using burst of 32 helps for some slave PMDs performance which makes sense. I can't understand how the extra copy phases improves the bonding itself performance:
You added 2 copy phases in the bonding RX function:
1. Get packets from the slave to a local array.
2. Copy packet pointers from a local array to the ring array.
3. Copy packet pointers from the ring array to the application array.
Each packet arriving to the application must pass the above 3 phases(in a specific call or in previous calls).
Without this patch we have only -
Get packets from the slave to the application array.
Can you explain how the extra copies improves the bonding performance?
Looks like it improves the slaves PMDs and because of that the bonding PMD performance becomes better.
Quoted 15 lines
So I'd like to share this improvement rather than keeping it private - because I'm nice that way :-P
Quoted 12 lines
Quoted 12 lines
Quoted 8 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
We should not do things worse than they are.
There were no reports that this made things worse. The feedback from production was that it made things better.
Yes, It may be good for specific slaves drivers but hurt another slaves drivers, So maybe it should stay private to specific costumers using specific nics.
Again, I can understand how this patch improves performance of some PMDs therefore I think the bonding is not the place to add it but maybe some PMDs.
Quoted 94 lines · Matan Azrad
Hi Luca
From: Luca Boccassi
Quoted 75 lines · Matan Azrad
Quoted 65 lines
Hi Chas
From: Chas Williams
Quoted 50 lines
On Tue, Aug 21, 2018 at 11:43 AM Matan Azrad <mailto:matan@mellanox .com> wrote:
Hi ChasFrom: Chas Williams
Quoted 10 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad <mailto:matan@mellano x.com>
wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Quoted 6 lines
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) for now.There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to give that some thought first.
There is no reason to stay this function as is while its twin is changed.
Unfortunately, this is all the patch I have at this time.
Quoted 13 lines
Quoted 5 lines
On Thu, Aug 16, 2018 at 9:32 AM Luca Boccassi <mailto:bluca@deb ian.org> wrote:
Quoted 7 lines
Quoted 7 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer.
Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.Quoted 8 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or more.
Quoted 8 lines
So, why to do it in bond which is a generic driver for all the vendors PMDs, If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets and to manage a ring by themselves.
The drawback of the ring is some additional latency on the receive path.
In testing, the additional latency hasn't been an issue for bonding.When bonding does processing slower it may be a bottleneck for the packet processing for some application.
Quoted 8 lines
Quoted 4 lines
The bonding PMD has a fair bit of overhead associated with the RX and TX path calculations. Most applications can just arrange to call the RX path with a sufficiently large receive. Bonding can't do this.
I didn't talk on application I talked on the slave PMDs, The slave PMD can manage a ring by itself if it helps for its own performance. The bonding should not be oriented to specific PMDs.
Quoted 7 lines
The issue though is that the performance problem is not with the individual PMDs - it's with bonding. There were no reports regarding the individual PMDs.
This comes from reports from customers from real world production deployments - the issue of bonding being too slow was raised multiple times. This patch addresses those issues, again in production deployments, where it's been used for years, to users and customers satisfaction.From Chas I understood that using burst of 32 helps for some slave PMDs performance which makes sense. I can't understand how the extra copy phases improves the bonding itself performance:
You added 2 copy phases in the bonding RX function:
1. Get packets from the slave to a local array.
2. Copy packet pointers from a local array to the ring array.
3. Copy packet pointers from the ring array to the application array.
Quoted 8 lines
Each packet arriving to the application must pass the above 3 phases(in a specific call or in previous calls).
Without this patch we have only -
Get packets from the slave to the application array.Can you explain how the extra copies improves the bonding performance?
Looks like it improves the slaves PMDs and because of that the bonding PMD performance becomes better.
I'm not sure that adding more buffer management to the vector PMDs will improve the drivers' performance; it's just that calling the rx function in such a way that it returns no data wastes time. The bonding driver is already an exercise in buffer management so adding this layer of indirection here makes sense in my opinion, as does hiding the details of the consituent interfaces where possible.
Quoted 28 lines
Quoted 15 lines
So I'd like to share this improvement rather than keeping it private - because I'm nice that way :-P
Quoted 12 lines
Quoted 12 lines
Quoted 8 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
We should not do things worse than they are.
There were no reports that this made things worse. The feedback from production was that it made things better.
Yes, It may be good for specific slaves drivers but hurt another slaves drivers, So maybe it should stay private to specific costumers using specific nics.
Again, I can understand how this patch improves performance of some PMDs therefore I think the bonding is not the place to add it but maybe some PMDs.
Hi
From: Eric Kinzie
Quoted 105 lines · Matan Azrad
Quoted 24 lines
Hi Luca
From: Luca Boccassi
Quoted 21 lines · Matan Azrad
Quoted 20 lines
Hi Chas
From: Chas Williams
Quoted 17 lines
On Tue, Aug 21, 2018 at 11:43 AM Matan Azrad
<mailto:matan@mellanox .com> wrote:
Hi ChasFrom: Chas Williams
Quoted 12 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad
<mailto:matan@mellano x.com>
wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) fornow.
Quoted 54 lines
Quoted 54 lines
Quoted 53 lines
Quoted 33 lines
Quoted 4 lines
There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to give that some thought first.
There is no reason to stay this function as is while its twin is changed.
Unfortunately, this is all the patch I have at this time.
Quoted 13 lines
Quoted 5 lines
On Thu, Aug 16, 2018 at 9:32 AM Luca Boccassi
<mailto:bluca@deb ian.org> wrote:Quoted 7 lines
Quoted 7 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer.
Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.Quoted 8 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or more.
Quoted 8 lines
So, why to do it in bond which is a generic driver for all the vendors PMDs, If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets and to manage a ring by themselves.
The drawback of the ring is some additional latency on the receive path.
In testing, the additional latency hasn't been an issue for bonding.When bonding does processing slower it may be a bottleneck for the packet processing for some application.
Quoted 4 lines
The bonding PMD has a fair bit of overhead associated with the RX and TX path calculations. Most applications can just arrange to call the RX path with a sufficiently large receive. Bonding can't do this.
I didn't talk on application I talked on the slave PMDs, The slave PMD can manage a ring by itself if it helps for its own performance. The bonding should not be oriented to specific PMDs.
Quoted 5 lines
Quoted 5 lines
The issue though is that the performance problem is not with the individual PMDs - it's with bonding. There were no reports regarding the individual PMDs.
This comes from reports from customers from real world production deployments - the issue of bonding being too slow was raised multipletimes.
Quoted 4 lines
Quoted 2 lines
This patch addresses those issues, again in production deployments, where it's been used for years, to users and customers satisfaction.
From Chas I understood that using burst of 32 helps for some slave PMDs
performance which makes sense.
Quoted 1 line
I can't understand how the extra copy phases improves the bonding itself
performance:
Quoted 7 lines
You added 2 copy phases in the bonding RX function:
1. Get packets from the slave to a local array.
2. Copy packet pointers from a local array to the ring array.
3. Copy packet pointers from the ring array to the application array.Each packet arriving to the application must pass the above 3 phases(in a
specific call or in previous calls).
Quoted 4 lines
Without this patch we have only -
Get packets from the slave to the application array.
Quoted 8 lines
Quoted 3 lines
Can you explain how the extra copies improves the bonding performance?
Looks like it improves the slaves PMDs and because of that the bonding
PMD performance becomes better.
I'm not sure that adding more buffer management to the vector PMDs will improve the drivers' performance; it's just that calling the rx function in such a way that it returns no data wastes time.
Sorry, I don't fully understand what you said here, please rephrase.
Quoted 2 lines
The bonding driver is already an exercise in buffer management so adding this layer of indirection here makes
sense in my opinion, as does hiding the details of the consituent interfaces where possible.
Can you explain how this new buffer management with the extra pointer copies improves the bonding itself performance? Looks really strange to me.
Quoted 30 lines
Quoted 18 lines
Quoted 18 lines
So I'd like to share this improvement rather than keeping it private
- because I'm nice that way :-PQuoted 15 lines
Quoted 14 lines
Quoted 8 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
We should not do things worse than they are.
There were no reports that this made things worse. The feedback from production was that it made things better.
Yes, It may be good for specific slaves drivers but hurt another slaves drivers, So maybe it should stay private to specific costumers using
specific nics.
Quoted 3 lines
Again, I can understand how this patch improves performance of some PMDs therefore I think the bonding is not the place to add it but maybe
some PMDs.
Quoted 125 lines · Matan Azrad
Hi
From: Eric Kinzie
Quoted 18 lines · Matan Azrad
Quoted 17 lines
Hi Luca
From: Luca Boccassi
Quoted 14 lines · Matan Azrad
Quoted 13 lines
Hi Chas
From: Chas Williams
Quoted 10 lines
On Tue, Aug 21, 2018 at 11:43 AM Matan Azrad
<mailto:matan@mellanox .com> wrote:
Hi ChasFrom: Chas Williams
Quoted 5 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad
<mailto:matan@mellano x.com>
wrote:
HiQuoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
From: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of
active-backup).
Quoted 45 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) fornow.
Quoted 41 lines
Quoted 41 lines
Quoted 41 lines
Quoted 41 lines
Quoted 4 lines
There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to give that some thought first.
There is no reason to stay this function as is while its twin is changed.
Unfortunately, this is all the patch I have at this time.
Quoted 13 lines
Quoted 5 lines
On Thu, Aug 16, 2018 at 9:32 AM Luca Boccassi
<mailto:bluca@deb ian.org> wrote:Quoted 7 lines
Quoted 7 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer.
Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.Quoted 8 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or more.
So, why to do it in bond which is a generic driver for all the vendors PMDs, If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets and to manage a ring by themselves.
The drawback of the ring is some additional latency on the receive path.
In testing, the additional latency hasn't been an issue forbonding.
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
When bonding does processing slower it may be a bottleneck for the packet processing for some application.
Quoted 4 lines
The bonding PMD has a fair bit of overhead associated with the RX and TX path calculations. Most applications can just arrange to call the RX path with a sufficiently large receive. Bonding can't do this.
I didn't talk on application I talked on the slave PMDs, The slave PMD can manage a ring by itself if it helps for its own
performance.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
The bonding should not be oriented to specific PMDs.
The issue though is that the performance problem is not with the individual PMDs - it's with bonding. There were no reports regarding the individual PMDs.
This comes from reports from customers from real world production deployments - the issue of bonding being too slow was raised multipleQuoted 7 lines
times.
Quoted 4 lines
Quoted 2 lines
This patch addresses those issues, again in production deployments, where it's been used for years, to users and customers satisfaction.
From Chas I understood that using burst of 32 helps for some slave PMDs
performance which makes sense.
Quoted 1 line
I can't understand how the extra copy phases improves the bonding
itself
Quoted 8 lines
performance:
Quoted 7 lines
You added 2 copy phases in the bonding RX function:
1. Get packets from the slave to a local array.
2. Copy packet pointers from a local array to the ring array.
3. Copy packet pointers from the ring array to the application array.Each packet arriving to the application must pass the above 3
phases(in a
Quoted 5 lines
specific call or in previous calls).
Quoted 4 lines
Without this patch we have only -
Get packets from the slave to the application array.Quoted 7 lines
Quoted 3 lines
Can you explain how the extra copies improves the bonding performance?
Looks like it improves the slaves PMDs and because of that the bonding
PMD performance becomes better.
I'm not sure that adding more buffer management to the vector PMDs will improve the drivers' performance; it's just that calling the rx function
in such
Quoted 1 line
a way that it returns no data wastes time.
Sorry, I don't fully understand what you said here, please rephrase.
Quoted 9 lines
Quoted 1 line
The bonding driver is already an exercise in buffer management so
adding this layer of indirection here makes
Quoted 1 line
sense in my opinion, as does hiding the details of the consituent
interfaces where possible.
Can you explain how this new buffer management with the extra pointer copies improves the bonding itself performance?
Looks really strange to me.
Because rings are generally quite efficient.
Bonding is in a middle ground between application and PMD. What bonding is doing, may not improve all applications. If using a ring to buffer the vectorized receive routines, improves your particular application, that's great. However, I don't think I can say that it would help all applications. As you point out, there is overhead associated with a ring.
Bonding's receive burst isn't especially efficient (in mode 4). Bonding benefits from being able to read as much as possible (within limits of course, large reads would blow out caches) from each slave. It can't return all that data though because applications tend to use the burst size that would be efficient for a typical PMD. An alternative might be to ask bonding applications to simply issue larger reads for certain modes. That's probably not as easy as it sounds given the way that the burst length effects multiplexing.
Another solution might be just alternatively poll the individual slaves on each rx burst. But that means you need to poll at a faster rate. Depending on your application, you might not be able to do that. We can avoid this scheduling overhead by just doing the extra reads in bonding and buffering in a ring.
Since bonding is going to be buffering everything in a ring, it makes sense to just read as much as is as efficiently possible. For the Intel adapters, this means using a read big enough trigger the vectorized versions.
Quoted 33 lines
Quoted 19 lines
Quoted 19 lines
Quoted 19 lines
So I'd like to share this improvement rather than keeping it private
- because I'm nice that way :-PQuoted 15 lines
Quoted 14 lines
Quoted 8 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
We should not do things worse than they are.
There were no reports that this made things worse. The feedback from production was that it made things better.
Yes, It may be good for specific slaves drivers but hurt another slaves drivers, So maybe it should stay private to specific costumers
using
Quoted 5 lines
specific nics.
Quoted 3 lines
Again, I can understand how this patch improves performance of some PMDs therefore I think the bonding is not the place to add it but maybe
some PMDs.
From: Chas Williams <[email protected]>
Quoted 120 lines · Matan Azrad
Hi
From: Eric Kinzie
Quoted 105 lines · Matan Azrad
Quoted 24 lines
Hi Luca
From: Luca Boccassi
Quoted 21 lines · Matan Azrad
Quoted 20 lines
Hi Chas
From: Chas Williams
Quoted 17 lines
On Tue, Aug 21, 2018 at 11:43 AM Matan Azrad
<mailto:mailto:matan@mellanox .com> wrote:
Hi ChasFrom: Chas Williams
Quoted 12 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad
<mailto:mailto:matan@mellano https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fx.com&data=02%7C01%7Cmatan%40mellanox.com%7Cc662ec1ee7734d12025808d609104474%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636706362822778011&sdata=MNAx1E5TgOrzXO9N8SWCOojrWmbqD8DPND%2BCXorOYhQ%3D&reserved=0>
wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) fornow.
Quoted 54 lines
Quoted 54 lines
Quoted 53 lines
Quoted 33 lines
Quoted 4 lines
There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to give that some thought first.
There is no reason to stay this function as is while its twin is changed.
Unfortunately, this is all the patch I have at this time.
Quoted 13 lines
Quoted 12 lines
On Thu, Aug 16, 2018 at 9:32 AM Luca Boccassi
<mailto:mailto:bluca@deb https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fian.org&data=02%7C01%7Cmatan%40mellanox.com%7Cc662ec1ee7734d12025808d609104474%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636706362822778011&sdata=AAcm%2FcbAA4CQsOnXFZWIqii6T%2BFUcc8xxT7%2Fs3tKIfY%3D&reserved=0> wrote:Quoted 7 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer.
Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.Quoted 8 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or more.
Quoted 8 lines
So, why to do it in bond which is a generic driver for all the vendors PMDs, If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets and to manage a ring by themselves.
The drawback of the ring is some additional latency on the receive path.
In testing, the additional latency hasn't been an issue for bonding.When bonding does processing slower it may be a bottleneck for the packet processing for some application.
Quoted 4 lines
The bonding PMD has a fair bit of overhead associated with the RX and TX path calculations. Most applications can just arrange to call the RX path with a sufficiently large receive. Bonding can't do this.
I didn't talk on application I talked on the slave PMDs, The slave PMD can manage a ring by itself if it helps for its own performance. The bonding should not be oriented to specific PMDs.
Quoted 5 lines
Quoted 5 lines
The issue though is that the performance problem is not with the individual PMDs - it's with bonding. There were no reports regarding the individual PMDs.
This comes from reports from customers from real world production deployments - the issue of bonding being too slow was raised multipletimes.
Quoted 4 lines
Quoted 2 lines
This patch addresses those issues, again in production deployments, where it's been used for years, to users and customers satisfaction.
From Chas I understood that using burst of 32 helps for some slave PMDs
performance which makes sense.
Quoted 1 line
I can't understand how the extra copy phases improves the bonding itself
performance:
Quoted 7 lines
You added 2 copy phases in the bonding RX function:
1. Get packets from the slave to a local array.
2. Copy packet pointers from a local array to the ring array.
3. Copy packet pointers from the ring array to the application array.Each packet arriving to the application must pass the above 3 phases(in a
specific call or in previous calls).
Quoted 4 lines
Without this patch we have only -
Get packets from the slave to the application array.Quoted 8 lines
Quoted 3 lines
Can you explain how the extra copies improves the bonding performance?
Looks like it improves the slaves PMDs and because of that the bonding
PMD performance becomes better.
I'm not sure that adding more buffer management to the vector PMDs will improve the drivers' performance; it's just that calling the rx function in such a way that it returns no data wastes time.
Sorry, I don't fully understand what you said here, please rephrase.
Quoted 7 lines
Quoted 2 lines
The bonding driver is already an exercise in buffer management so adding this layer of indirection here makes sense in my opinion, as does hiding the details of the consituent interfaces where possible.
Can you explain how this new buffer management with the extra pointer copies improves the bonding itself performance? Looks really strange to me.
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it must hurt performance of the bonding PMD (means the bonding itself - not the slaves PMDs which are called from the bonding)
Quoted 2 lines
Bonding is in a middle ground between application and PMD.
Yes.
Quoted 1 line
What bonding is doing, may not improve all applications.
Yes, but it can be solved using some bonding modes.
Quoted 2 lines
If using a ring to buffer the vectorized receive routines, improves your particular application, that's great.
It may be not great and even bad for some other PMDs which are not vectororized.
Quoted 3 lines
However, I don't think I can say that it would help all applications. As you point out, there is overhead associated with a ring.
Yes.
Quoted 1 line
Bonding's receive burst isn't especially efficient (in mode 4).
Why?
Quoted 2 lines
Bonding benefits from being able to read as much as possible (within limits of course, large reads would blow out caches) from each slave.
The slaves PMDs can benefits in the same way.
Quoted 2 lines
It can't return all that data though because applications tend to use the burst size that would be efficient for a typical PMD.
What is the preferred burst size of the bonding? Maybe the application should use it when they are using bonding.
Quoted 3 lines
An alternative might be to ask bonding applications to simply issue larger reads for certain modes. That's probably not as easy as it sounds given the
way that the burst length effects multiplexing.
Can you explain it more?
Quoted 4 lines
Another solution might be just alternatively poll the individual slaves on each rx burst. But that means you need to poll at a faster rate. Depending on your application, you might not be able to do that.
Again, can you be more precise in the above explanation?
Quoted 4 lines
We can avoid this scheduling overhead by just doing the extra reads in bonding and buffering in a ring.
Since bonding is going to be buffering everything in a ring,
? I don't familiar with it. For now I don't think we need a ring.
Quoted 34 lines
it makes sense to just read as much as is as efficiently possible. For the Intel adapters, this means using a read big enough trigger the vectorized versions.
Quoted 30 lines
Quoted 18 lines
Quoted 18 lines
So I'd like to share this improvement rather than keeping it private
- because I'm nice that way :-PQuoted 15 lines
Quoted 14 lines
Quoted 8 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
We should not do things worse than they are.
There were no reports that this made things worse. The feedback from production was that it made things better.
Yes, It may be good for specific slaves drivers but hurt another slaves drivers, So maybe it should stay private to specific costumers using
specific nics.
Quoted 3 lines
Again, I can understand how this patch improves performance of some PMDs therefore I think the bonding is not the place to add it but maybe
some PMDs.
Quoted 162 lines · Matan Azrad
From: Chas Williams <[email protected]>
Quoted 1 line
On Thu, Aug 23, 2018 at 3:28 AM Matan Azrad <mailto:[email protected]>
Quoted 18 lines · wrote:
Hi
From: Eric Kinzie
Quoted 15 lines · Matan Azrad
Quoted 14 lines
Hi Luca
From: Luca Boccassi
Quoted 11 lines · Matan Azrad
Quoted 10 lines
Hi Chas
From: Chas Williams
Quoted 7 lines
On Tue, Aug 21, 2018 at 11:43 AM Matan Azrad
<mailto:mailto:matan@mellanox .com> wrote:
Hi ChasFrom: Chas Williams
Quoted 2 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad
<mailto:mailto:matan@mellanoQuoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of
active-backup).
Quoted 12 lines
Quoted 12 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) fornow.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to give that some thought first.
There is no reason to stay this function as is while its twin is changed.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Unfortunately, this is all the patch I have at this time.
Quoted 4 lines
Quoted 4 lines
On Thu, Aug 16, 2018 at 9:32 AM Luca Boccassi
<mailto:mailto:bluca@debQuoted 18 lines · wrote:
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 17 lines
Quoted 8 lines
Quoted 8 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer.
Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or more.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
So, why to do it in bond which is a generic driver for all the vendors PMDs, If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets and to manage a ring by themselves.
The drawback of the ring is some additional latency on the receive path.
In testing, the additional latency hasn't been an issue forbonding.
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
When bonding does processing slower it may be a bottleneck for the packet processing for some application.
Quoted 4 lines
The bonding PMD has a fair bit of overhead associated with the RX and TX path calculations. Most applications can just arrange to call the RX path with a sufficiently large receive. Bonding can't do this.
I didn't talk on application I talked on the slave PMDs, The slave PMD can manage a ring by itself if it helps for its own
performance.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
The bonding should not be oriented to specific PMDs.
The issue though is that the performance problem is not with the individual PMDs - it's with bonding. There were no reports regarding the individual PMDs.
This comes from reports from customers from real world production deployments - the issue of bonding being too slow was raisedmultiple
Quoted 5 lines
Quoted 5 lines
times.
Quoted 4 lines
Quoted 2 lines
This patch addresses those issues, again in production deployments, where it's been used for years, to users and customers satisfaction.
From Chas I understood that using burst of 32 helps for some slave
PMDs
Quoted 2 lines
Quoted 2 lines
performance which makes sense.
Quoted 1 line
I can't understand how the extra copy phases improves the bonding
itself
Quoted 8 lines
Quoted 8 lines
performance:
Quoted 7 lines
You added 2 copy phases in the bonding RX function:
1. Get packets from the slave to a local array.
2. Copy packet pointers from a local array to the ring array.
3. Copy packet pointers from the ring array to the application array.Each packet arriving to the application must pass the above 3
phases(in a
Quoted 5 lines
Quoted 5 lines
specific call or in previous calls).
Quoted 4 lines
Without this patch we have only -
Get packets from the slave to the application array.Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
Can you explain how the extra copies improves the bonding performance?
Looks like it improves the slaves PMDs and because of that the bonding
PMD performance becomes better.
I'm not sure that adding more buffer management to the vector PMDs will improve the drivers' performance; it's just that calling the rx
function in such
Quoted 5 lines
Quoted 1 line
a way that it returns no data wastes time.
Sorry, I don't fully understand what you said here, please rephrase.
Quoted 1 line
The bonding driver is already an exercise in buffer management so
adding this layer of indirection here makes
Quoted 1 line
Quoted 1 line
sense in my opinion, as does hiding the details of the consituent
interfaces where possible.
Quoted 2 lines
Can you explain how this new buffer management with the extra pointer
copies improves the bonding itself performance?
Quoted 3 lines
Looks really strange to me.
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it must hurt performance of the bonding PMD
(means the bonding itself - not the slaves PMDs which are called from the bonding)Quoted 2 lines
Bonding is in a middle ground between application and PMD.
Yes.
Quoted 1 line
What bonding is doing, may not improve all applications.
Yes, but it can be solved using some bonding modes.
Quoted 1 line
If using a ring to buffer the vectorized receive routines, improves your
particular application,
Quoted 1 line
that's great.
It may be not great and even bad for some other PMDs which are not vectororized.
Quoted 8 lines
Quoted 3 lines
However, I don't think I can say that it would help all applications. As you point out, there is overhead associated with a ring.
Yes.
Quoted 1 line
Bonding's receive burst isn't especially efficient (in mode 4).
Why?
It makes a copy of the slaves, has a fair bit of stack usage, needs to check the slave status, and needs to examine each packet to see if it is a slow protocol packet. So each packet is essentially read twice. The fast queue code for mode 4 avoids some of this (and probably ignores checking collecting incorrectly). If you find a slow protocol packet, you need to chop it out of the array with memmove due to overlap.
Quoted 5 lines
Quoted 2 lines
Bonding benefits from being able to read as much as possible (within
limits of
Quoted 1 line
course, large reads would blow out caches) from each slave.
Quoted 8 lines
The slaves PMDs can benefits in the same way.
Quoted 2 lines
It can't return all that data though because applications tend to use the burst size that would be efficient for a typical PMD.
What is the preferred burst size of the bonding? Maybe the application should use it when they are using bonding.
The preferred burst size for bonding would be the sum of all the slaves ideal read size. However, that's not likely to be simple since most applications decide early the size for the read/write burst operations.
Quoted 8 lines
Quoted 2 lines
An alternative might be to ask bonding applications to simply issue
larger reads for
Quoted 2 lines
certain modes. That's probably not as easy as it sounds given the way that the burst length effects multiplexing.
Can you explain it more?
A longer burst size on one PMD will tend to favor that PMD over others. It will fill your internal queues with more of its packets.
Quoted 8 lines
Quoted 5 lines
Another solution might be just alternatively poll the individual slaves on each rx burst. But that means you need to poll at a faster rate. Depending on your application, you might not be able to do that.
Again, can you be more precise in the above explanation?
If the application knows that there are two slaves backing a bonding interface, the application could just read twice from the bonding interface, knowing that the bonding interface is going to alternate between the slaves. But this requires the application to know things about the bonding PMD, like the number of slaves.
Quoted 8 lines
Quoted 5 lines
We can avoid this scheduling overhead by just
doing the extra reads in bonding and buffering in a ring.Since bonding is going to be buffering everything in a ring,
? I don't familiar with it. For now I don't think we need a ring.
We have some empirical testing that shows performance improvements. How do you explain this? Rings aren't inefficient. There's significant overhead of calling the rx burst routine on any PMD. You need to get the PMD data structures into local memory. Reading as much as possible makes sense. Could you generally do this for all PMDs? Yes, but the ring adds latency. Latency that isn't an issue for the bonding PMD because of all the other inefficiencies (for mode 4).
Quoted 39 lines
Quoted 2 lines
it makes sense to just read as much as is as efficiently possible. For
the
Quoted 27 lines
Intel adapters, this means using a read big enough trigger the vectorized versions.
Quoted 25 lines
Quoted 25 lines
Quoted 15 lines
So I'd like to share this improvement rather than keeping it private
- because I'm nice that way :-PQuoted 12 lines
Quoted 12 lines
Quoted 8 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
We should not do things worse than they are.
There were no reports that this made things worse. The feedback from production was that it made things better.
Yes, It may be good for specific slaves drivers but hurt another slaves drivers, So maybe it should stay private to specific costumers
using
Quoted 4 lines
Quoted 4 lines
specific nics.
Quoted 3 lines
Again, I can understand how this patch improves performance of some PMDs therefore I think the bonding is not the place to add it but
maybe
Quoted 2 lines
Quoted 1 line
some PMDs.
Hi Chas
From: Chas Williams <[email protected]>
Quoted 151 lines · Matan Azrad
From: Chas Williams <mailto:[email protected]>
Quoted 120 lines · Matan Azrad
Hi
From: Eric Kinzie
Quoted 105 lines · Matan Azrad
Quoted 24 lines
Hi Luca
From: Luca Boccassi
Quoted 21 lines · Matan Azrad
Quoted 20 lines
Hi Chas
From: Chas Williams
Quoted 17 lines
On Tue, Aug 21, 2018 at 11:43 AM Matan Azrad
<mailto:mailto:mailto:matan@mellanox .com> wrote:
Hi ChasFrom: Chas Williams
Quoted 12 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad
<mailto:mailto:mailto:matan@mellano https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fx.com&data=02%7C01%7Cmatan%40mellanox.com%7C7ee011bf19224cb17d9a08d60c202379%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636709729507525613&sdata=TiWaRq8A%2FvUaqPIT6ajfJdcY7eIAGPqB%2BiHppcFdZJo%3D&reserved=0>
wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of active-backup).
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) fornow.
Quoted 54 lines
Quoted 54 lines
Quoted 53 lines
Quoted 33 lines
Quoted 4 lines
There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to give that some thought first.
There is no reason to stay this function as is while its twin is changed.
Unfortunately, this is all the patch I have at this time.
Quoted 13 lines
Quoted 12 lines
On Thu, Aug 16, 2018 at 9:32 AM Luca Boccassi
<mailto:mailto:mailto:bluca@deb https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fian.org&data=02%7C01%7Cmatan%40mellanox.com%7C7ee011bf19224cb17d9a08d60c202379%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636709729507525613&sdata=0ALmgN%2B6Xnrl3kKeeoJRGTlJmeLZlmcJwsOTcncMUWE%3D&reserved=0> wrote:Quoted 7 lines
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer.
Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.Quoted 8 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you need to be receiving at least 32 packets or more.
Quoted 8 lines
So, why to do it in bond which is a generic driver for all the vendors PMDs, If for ixgbe and other Intel nics it is better you can force those PMDs to receive always 32 packets and to manage a ring by themselves.
The drawback of the ring is some additional latency on the receive path.
In testing, the additional latency hasn't been an issue for bonding.When bonding does processing slower it may be a bottleneck for the packet processing for some application.
Quoted 4 lines
The bonding PMD has a fair bit of overhead associated with the RX and TX path calculations. Most applications can just arrange to call the RX path with a sufficiently large receive. Bonding can't do this.
I didn't talk on application I talked on the slave PMDs, The slave PMD can manage a ring by itself if it helps for its own performance. The bonding should not be oriented to specific PMDs.
Quoted 5 lines
Quoted 5 lines
The issue though is that the performance problem is not with the individual PMDs - it's with bonding. There were no reports regarding the individual PMDs.
This comes from reports from customers from real world production deployments - the issue of bonding being too slow was raised multipletimes.
Quoted 4 lines
Quoted 2 lines
This patch addresses those issues, again in production deployments, where it's been used for years, to users and customers satisfaction.
From Chas I understood that using burst of 32 helps for some slave PMDs
performance which makes sense.
Quoted 1 line
I can't understand how the extra copy phases improves the bonding itself
performance:
Quoted 7 lines
You added 2 copy phases in the bonding RX function:
1. Get packets from the slave to a local array.
2. Copy packet pointers from a local array to the ring array.
3. Copy packet pointers from the ring array to the application array.Each packet arriving to the application must pass the above 3 phases(in a
specific call or in previous calls).
Quoted 4 lines
Without this patch we have only -
Get packets from the slave to the application array.Quoted 8 lines
Quoted 3 lines
Can you explain how the extra copies improves the bonding performance?
Looks like it improves the slaves PMDs and because of that the bonding
PMD performance becomes better.
I'm not sure that adding more buffer management to the vector PMDs will improve the drivers' performance; it's just that calling the rx function in such a way that it returns no data wastes time.
Sorry, I don't fully understand what you said here, please rephrase.
Quoted 7 lines
Quoted 2 lines
The bonding driver is already an exercise in buffer management so adding this layer of indirection here makes sense in my opinion, as does hiding the details of the consituent interfaces where possible.
Can you explain how this new buffer management with the extra pointer copies improves the bonding itself performance? Looks really strange to me.
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it must hurt performance of the bonding PMD (means the bonding itself - not the slaves PMDs which are called from the bonding)
Quoted 2 lines
Bonding is in a middle ground between application and PMD.
Yes.
Quoted 1 line
What bonding is doing, may not improve all applications.
Yes, but it can be solved using some bonding modes.
Quoted 2 lines
If using a ring to buffer the vectorized receive routines, improves your particular application, that's great.
It may be not great and even bad for some other PMDs which are not vectororized.
Quoted 3 lines
However, I don't think I can say that it would help all applications. As you point out, there is overhead associated with a ring.
Yes.
Quoted 1 line
Bonding's receive burst isn't especially efficient (in mode 4).
Why?
Quoted 7 lines
It makes a copy of the slaves, has a fair bit of stack usage, needs to check the slave status, and needs to examine each packet to see if it is a slow protocol packet. So each packet is essentially read twice. The fast queue code for mode 4 avoids some of this (and probably ignores checking collecting incorrectly). If you find a slow protocol packet, you need to chop it out of the array with memmove due to overlap.
Agree.
So to improve the bonding performance you need to optimize the aboves problems. There is no connection to the ring.
Quoted 19 lines
Quoted 2 lines
Bonding benefits from being able to read as much as possible (within limits of course, large reads would blow out caches) from each slave.
The slaves PMDs can benefits in the same way.
Quoted 2 lines
It can't return all that data though because applications tend to use the burst size that would be efficient for a typical PMD.
What is the preferred burst size of the bonding? Maybe the application should use it when they are using bonding.
The preferred burst size for bonding would be the sum of all the slaves ideal read size. However, that's not likely to be simple since most applications decide early the size for the read/write burst operations.
Quoted 3 lines
An alternative might be to ask bonding applications to simply issue larger reads for certain modes. That's probably not as easy as it sounds given the
way that the burst length effects multiplexing.
Quoted 5 lines
Can you explain it more?
A longer burst size on one PMD will tend to favor that PMD over others. It will fill your internal queues with more of its packets.
Agree, it's about fairness.
Quoted 6 lines
Quoted 5 lines
Another solution might be just alternatively poll the individual slaves on each rx burst. But that means you need to poll at a faster rate. Depending on your application, you might not be able to do that.
Quoted 8 lines
Again, can you be more precise in the above explanation?
If the application knows that there are two slaves backing a bonding interface, the application could just read twice from the bonding interface, knowing that the bonding interface is going to alternate between the slaves. But this requires the application to know things about the bonding PMD, like the number of slaves.
Why should the application poll twice?
Poll slave 0, than process it's packets, poll slave 1 than process its packets... What is the problem?
Quoted 9 lines
Quoted 4 lines
We can avoid this scheduling overhead by just doing the extra reads in bonding and buffering in a ring.
Since bonding is going to be buffering everything in a ring,
? I don't familiar with it. For now I don't think we need a ring.
We have some empirical testing that shows performance improvements. How do you explain this?
You are using a hack in bonding which hurt the bonding but improves the vectorized PMD you use.
Quoted 1 line
Rings aren't inefficient.
Only when there is a need to use it.
Quoted 2 lines
There's significant overhead of calling the rx burst routine on any PMD. You need to get the PMD data structures into local memory.
Yes.
Quoted 1 line
Reading as much as possible makes sense.
Agree.
Quoted 3 lines
Could you generally do this for all PMDs? Yes, but the ring adds latency. Latency that isn't an issue for the bonding PMD because of all the
other inefficiencies (for mode 4).
Enlarging the bonding latency by that way makes some vectorized slave PMDs happy and makes the bonding worst for other slaves PMDs - this is not a good idea to put it upstream.
Improving the other problems in the bonding(reduce the bonding latency) will do the job for you and for others.
Quoted 35 lines
>it makes sense to just read as much as is as efficiently possible. For the
Quoted 33 lines
Intel adapters, this means using a read big enough trigger the vectorized versions.
Quoted 30 lines
Quoted 18 lines
Quoted 18 lines
So I'd like to share this improvement rather than keeping it private
- because I'm nice that way :-PQuoted 15 lines
Quoted 14 lines
Quoted 8 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector requirement. But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
We should not do things worse than they are.
There were no reports that this made things worse. The feedback from production was that it made things better.
Yes, It may be good for specific slaves drivers but hurt another slaves drivers, So maybe it should stay private to specific costumers using
specific nics.
Quoted 3 lines
Again, I can understand how this patch improves performance of some PMDs therefore I think the bonding is not the place to add it but maybe
some PMDs.
Quoted 158 lines · Matan Azrad
Hi Chas
From: Chas Williams <[email protected]>
Quoted 1 line
On Sun, Aug 26, 2018 at 3:40 AM Matan Azrad <mailto:[email protected]>
Quoted 3 lines · wrote:
From: Chas Williams <mailto:[email protected]>
Quoted 1 line
On Thu, Aug 23, 2018 at 3:28 AM Matan Azrad <mailto:mailto:
Quoted 10 lines · [email protected]> wrote:
Quoted 10 lines
Hi
From: Eric Kinzie
Quoted 7 lines · Matan Azrad
Quoted 6 lines
Hi Luca
From: Luca Boccassi
Quoted 3 lines · Matan Azrad
Quoted 2 lines
Hi Chas
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
From: Chas Williams
Quoted 7 lines
On Tue, Aug 21, 2018 at 11:43 AM Matan Azrad
<mailto:mailto:mailto:matan@mellanox .com> wrote:
Hi ChasFrom: Chas Williams
Quoted 2 lines
On Tue, Aug 21, 2018 at 6:56 AM Matan Azrad
<mailto:mailto:mailto:matan@mellanoQuoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
wrote:
HiFrom: Chas Williams
Quoted 3 lines
This will need to be implemented for some of the other RX burst methods at some point for other modes to see this performance improvement (with the exception of
active-backup).
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Yes, I think it should be done at least to
bond_ethdev_rx_burst_8023ad_fast_queue(should be easy) fornow.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
There is some duplicated code between the various RX paths. I would like to eliminate that as much as possible, so I was going to give that some thought first.
There is no reason to stay this function as is while its twin
is
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
changed.
Unfortunately, this is all the patch I have at this time.
Quoted 4 lines
Quoted 4 lines
On Thu, Aug 16, 2018 at 9:32 AM Luca Boccassi
<mailto:mailto:mailto:bluca@debQuoted 2 lines · wrote:
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
During bond 802.3ad receive, a burst of packets is
fetched
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Quoted 6 lines
from each slave into a local array and appended to per-slave ring buffer.
Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Luca,
Can you explain these requirements of ixgbe?
The ixgbe (and some other Intel PMDs) have vectorized RX routines that are more efficient (if not faster) taking advantage of some advanced CPU instructions. I think you
need
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
to be receiving at least 32 packets or more.
So, why to do it in bond which is a generic driver for all the vendors PMDs, If for ixgbe and other Intel nics it is better
you
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
can force those PMDs to receive always 32 packets and to manage a ring by themselves.
The drawback of the ring is some additional latency on the receive path.
In testing, the additional latency hasn't been an issue forbonding.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
When bonding does processing slower it may be a bottleneck for
the
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
packet processing for some application.
Quoted 2 lines
The bonding PMD has a fair bit of overhead associated with the RX and TX path calculations. Most applications can just
arrange
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
to call the RX path with a sufficiently large receive. Bonding can't do this.
I didn't talk on application I talked on the slave PMDs, The
slave
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
PMD can manage a ring by itself if it helps for its own
performance.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
The bonding should not be oriented to specific PMDs.
The issue though is that the performance problem is not with the individual PMDs - it's with bonding. There were no reports
regarding
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
the individual PMDs.
This comes from reports from customers from real world production deployments - the issue of bonding being too slow was raisedmultiple
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
times.
Quoted 2 lines
Quoted 2 lines
This patch addresses those issues, again in production deployments, where it's been used for years, to users and customers
satisfaction.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
From Chas I understood that using burst of 32 helps for some slave
PMDs
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
performance which makes sense.
Quoted 1 line
I can't understand how the extra copy phases improves the bonding
itself
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
performance:
Quoted 7 lines
You added 2 copy phases in the bonding RX function:
1. Get packets from the slave to a local array.
2. Copy packet pointers from a local array to the ring array.
3. Copy packet pointers from the ring array to the application array.Each packet arriving to the application must pass the above 3
phases(in a
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
specific call or in previous calls).
Quoted 5 lines
Without this patch we have only -
Get packets from the slave to the application array.Can you explain how the extra copies improves the bonding
performance?
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Looks like it improves the slaves PMDs and because of that the
bonding
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
PMD performance becomes better.
I'm not sure that adding more buffer management to the vector PMDs will improve the drivers' performance; it's just that calling the rx
function in such
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
a way that it returns no data wastes time.
Sorry, I don't fully understand what you said here, please rephrase.
Quoted 1 line
The bonding driver is already an exercise in buffer management so
adding this layer of indirection here makes
Quoted 1 line
Quoted 1 line
Quoted 1 line
sense in my opinion, as does hiding the details of the consituent
interfaces where possible.
Quoted 2 lines
Quoted 2 lines
Can you explain how this new buffer management with the extra pointer
copies improves the bonding itself performance?
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
Looks really strange to me.
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it must
hurt performance of the bonding PMD
Quoted 1 line
(means the bonding itself - not the slaves PMDs which are called from the
bonding)
It adds latency. It increases performance because we spend less CPU time reading from the PMDs. This means we have more CPU to use for post processing (i.e. routing).
Quoted 28 lines
Quoted 7 lines
Quoted 3 lines
Bonding is in a middle ground between application and PMD.
Yes.
Quoted 1 line
What bonding is doing, may not improve all applications.
Yes, but it can be solved using some bonding modes.
Quoted 1 line
If using a ring to buffer the vectorized receive routines, improves
your particular application,
Quoted 2 lines
Quoted 1 line
that's great.
It may be not great and even bad for some other PMDs which are not
vectororized.
Quoted 9 lines
Quoted 4 lines
However, I don't think I can say that it would help all applications. As you point out, there is overhead associated with a ring.
Yes.
Quoted 1 line
Bonding's receive burst isn't especially efficient (in mode 4).
Why?
Quoted 7 lines
It makes a copy of the slaves, has a fair bit of stack usage, needs to check the slave status, and needs to examine each packet to see if it is a slow protocol packet. So each packet is essentially read twice. The fast queue code for mode 4 avoids some of this (and probably ignores checking collecting incorrectly). If you find a slow protocol packet, you need to chop it out of the array with memmove due to overlap.
Quoted 5 lines
Agree.
So to improve the bonding performance you need to optimize the aboves problems.
There is no connection to the ring.
And as I have described numerous times, these problems can't be easily fixed and preserve the existing API.
Quoted 47 lines
Quoted 2 lines
Quoted 2 lines
Bonding benefits from being able to read as much as possible (within
limits of
Quoted 5 lines
Quoted 1 line
course, large reads would blow out caches) from each slave.
The slaves PMDs can benefits in the same way.
Quoted 1 line
It can't return all that data though because applications tend to use
the
Quoted 3 lines
Quoted 1 line
burst size that would be efficient for a typical PMD.
What is the preferred burst size of the bonding? Maybe the application
should use it when they are using bonding.
Quoted 7 lines
The preferred burst size for bonding would be the sum of all the slaves ideal read size. However, that's not likely to be simple since most applications decide early the size for the read/write burst operations.
Quoted 1 line
An alternative might be to ask bonding applications to simply issue
larger reads for
Quoted 8 lines
Quoted 2 lines
certain modes. That's probably not as easy as it sounds given the way that the burst length effects multiplexing.
Can you explain it more?
A longer burst size on one PMD will tend to favor that PMD over others. It will fill your internal queues with more of its packets.
Agree, it's about fairness.
Quoted 6 lines
Quoted 5 lines
Another solution might be just alternatively poll the individual slaves on each rx burst. But that means you need to poll at a faster rate. Depending on your application, you might not be able to do that.
Quoted 8 lines
Again, can you be more precise in the above explanation?
If the application knows that there are two slaves backing a bonding interface, the application could just read twice from the bonding interface, knowing that the bonding interface is going to alternate between the slaves. But this requires the application to know things about the bonding PMD, like the number of slaves.
Quoted 5 lines
Why should the application poll twice?
Poll slave 0, than process it's packets, poll slave 1 than process its packets...
What is the problem?
B
ecause let's say that each slave is 10G and you are using
link aggregation with two slaves. If you have tuned your
application on the assumption that a PMD is approximately 10G, then you are going to be under polling the bonding PMD.
For the above to work, you need to ensure that the application
is polling sufficiently to keep up.
Quoted 11 lines
Quoted 10 lines
Quoted 5 lines
We can avoid this scheduling overhead by just
doing the extra reads in bonding and buffering in a ring.Since bonding is going to be buffering everything in a ring,
? I don't familiar with it. For now I don't think we need a ring.
We have some empirical testing that shows performance improvements. How do you explain this?
Quoted 7 lines
You are using a hack in bonding which hurt the bonding but improves the vectorized PMD you use.
Quoted 1 line
Rings aren't inefficient.
Only when there is a need to use it.
I believe we have identified a need.
Quoted 15 lines
Quoted 3 lines
There's significant overhead of calling the rx burst routine on any PMD. You need to get the PMD data structures into local memory.
Yes.
Quoted 1 line
Reading as much as possible makes sense.
Agree.
Quoted 1 line
Could you generally do this for all PMDs? Yes, but the ring adds
latency. Latency
Quoted 2 lines
that isn't an issue for the bonding PMD because of all the other inefficiencies (for mode 4).
Quoted 7 lines
Enlarging the bonding latency by that way makes some vectorized slave PMDs happy and makes the bonding worst
for other slaves PMDs - this is not a good idea to put it upstream.Improving the other problems in the bonding(reduce the bonding latency) will do the job for you and for others.
Latency is not the issue with bonding. The issue with bonding is the overhead associated with making an rx burst call. We add latency (via rings) to make part of the bonding driver more efficient.
Again, I suspect it even helps the non-vectorized PMDs. Calling a PMD's rx burst routine is costly and we are switching between PMD's inside the bonding driver. Bonding is halfway between an application and a PMD. What we are doing in the bonding PMD is what an application would typically do. But instead of forcing all the bonding users to do this, we are doing it for them.
Quoted 43 lines
Quoted 2 lines
Quoted 2 lines
it makes sense to just read as much as is as efficiently possible. For
the
Quoted 3 lines
Quoted 3 lines
Intel adapters, this means using a read big enough trigger the vectorized versions.
Quoted 1 line
Quoted 1 line
Quoted 1 line
So I'd like to share this improvement rather than keeping it
private
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
- because I'm nice that way :-P
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Did you check for other vendor PMDs? It may hurt performance there..
I don't know, but I suspect probably not. For the most part you are typically reading almost up to the vector
requirement.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
But if one slave has just a single packet, then you can't vectorize on the next slave.
I don't think that the ring overhead is better for PMDs which are not using the vectorized instructions.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
The non-vectorized PMDs are usually quite slow. The additional overhead doesn't make a difference in their performance.
We should not do things worse than they are.
There were no reports that this made things worse. The feedback
from
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
production was that it made things better.
Yes, It may be good for specific slaves drivers but hurt another slaves drivers, So maybe it should stay private to specific
costumers using
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
specific nics.
Quoted 3 lines
Again, I can understand how this patch improves performance of some PMDs therefore I think the bonding is not the place to add it but
maybe
Quoted 3 lines
Quoted 2 lines
Quoted 1 line
some PMDs.
From: Chas Williams
Quoted 1 line · Matan Azrad
<snip>
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it must hurt performance of the bonding PMD (means the bonding itself - not the slaves PMDs which are called from the bonding)
It adds latency.
And by that hurts the application performance because it takes more CPU time in the bonding PMD.
Quoted 1 line
It increases performance because we spend less CPU time reading from the PMDs.
So, it's hack in the bonding PMD to improve some slaves code performance but hurt the bonding code performance, Over all the performance we gain for those slaves improves the application performance only when working with those slaves. But may hurt the application performance when working with other slaves.
Quoted 29 lines
This means we have more CPU to use for
post processing (i.e. routing).Quoted 16 lines
Quoted 1 line
Bonding is in a middle ground between application and PMD.
Yes.
Quoted 1 line
What bonding is doing, may not improve all applications.
Yes, but it can be solved using some bonding modes.
Quoted 2 lines
If using a ring to buffer the vectorized receive routines, improves your particular application, that's great.
It may be not great and even bad for some other PMDs which are not vectororized.
Quoted 3 lines
However, I don't think I can say that it would help all applications. As you point out, there is overhead associated with a ring.
Yes.
Quoted 1 line
Bonding's receive burst isn't especially efficient (in mode 4).
Why?
Quoted 7 lines
It makes a copy of the slaves, has a fair bit of stack usage, needs to check the slave status, and needs to examine each packet to see if it is a slow protocol packet. So each packet is essentially read twice. The fast queue code for mode 4 avoids some of this (and probably ignores checking collecting incorrectly). If you find a slow protocol packet, you need to chop it out of the array with memmove due to overlap.
Quoted 6 lines
Agree.
So to improve the bonding performance you need to optimize the aboves problems. There is no connection to the ring.And as I have described numerous times, these problems can't be easily fixed and preserve the existing API.
Sometimes we need to work harder to see a gain for all.
We should not apply a patch because it is easy and show a gain for specific scenarios.
Quoted 46 lines
Quoted 19 lines
Quoted 2 lines
Bonding benefits from being able to read as much as possible (within limits of course, large reads would blow out caches) from each slave.
The slaves PMDs can benefits in the same way.
Quoted 2 lines
It can't return all that data though because applications tend to use the burst size that would be efficient for a typical PMD.
What is the preferred burst size of the bonding? Maybe the application should use it when they are using bonding.
The preferred burst size for bonding would be the sum of all the slaves ideal read size. However, that's not likely to be simple since most applications decide early the size for the read/write burst operations.
Quoted 3 lines
An alternative might be to ask bonding applications to simply issue larger reads for certain modes. That's probably not as easy as it sounds given the
way that the burst length effects multiplexing.Quoted 5 lines
Can you explain it more?
A longer burst size on one PMD will tend to favor that PMD over others. It will fill your internal queues with more of its packets.
Agree, it's about fairness.
Quoted 6 lines
Quoted 5 lines
Another solution might be just alternatively poll the individual slaves on each rx burst. But that means you need to poll at a faster rate. Depending on your application, you might not be able to do that.
Quoted 8 lines
Again, can you be more precise in the above explanation?
If the application knows that there are two slaves backing a bonding interface, the application could just read twice from the bonding interface, knowing that the bonding interface is going to alternate between the slaves. But this requires the application to know things about the bonding PMD, like the number of slaves.
Why should the application poll twice?
Poll slave 0, than process it's packets, poll slave 1 than process its packets... What is the problem?
Quoted 8 lines
Because let's say that each slave is 10G and you are using
link aggregation with two slaves. If you have tuned your
application on the assumption that a PMD is approximately 10G, then you are going to be under polling the bonding PMD. For the above to work, you need to ensure that the application is polling sufficiently to keep up.
But each poll will be shorter, no slaves loops, only one slave call.
Quoted 35 lines
Quoted 9 lines
Quoted 4 lines
We can avoid this scheduling overhead by just doing the extra reads in bonding and buffering in a ring.
Since bonding is going to be buffering everything in a ring,
? I don't familiar with it. For now I don't think we need a ring.
We have some empirical testing that shows performance improvements. How do you explain this?
You are using a hack in bonding which hurt the bonding but improves the vectorized PMD you use.
Quoted 1 line
Rings aren't inefficient.
Only when there is a need to use it.
I believe we have identified a need.
Quoted 2 lines
There's significant overhead of calling the rx burst routine on any PMD. You need to get the PMD data structures into local memory.
Yes.
Quoted 1 line
Reading as much as possible makes sense.
Agree.
Quoted 3 lines
Could you generally do this for all PMDs? Yes, but the ring adds latency. Latency that isn't an issue for the bonding PMD because of all the
other inefficiencies (for mode 4).Enlarging the bonding latency by that way makes some vectorized slave PMDs happy and makes the bonding worst for other slaves PMDs - this is not a good idea to put it upstream.
Quoted 8 lines
Improving the other problems in the bonding(reduce the bonding latency) will do the job for you and for others.
Latency is not the issue with bonding. The issue with bonding is the overhead associated with making an rx burst call. We add latency (via rings) to make part of the bonding driver more efficient.
Again, I suspect it even helps the non-vectorized PMDs.
>Calling a PMD's rx burst routine is costly and we are switching between
Quoted 4 lines
PMD's inside the bonding driver. Bonding is halfway between an application and a PMD. What we are doing in the bonding PMD is what an application would typically do. But instead of forcing all the bonding users to do this, we are doing it for them.
Not agree.
The overhead in bonding may be critical for some other PMDs\scenario.
To summarize:
We are not agree.
Bottom line in my side,
It is not a good Idea to add overhead in the bonding PMD which is a generic PMD just to get gain for some specific scenarios in some specific PMDs while for other scenarios\PMDs it is bad.
Matan.
Quoted 9 lines · Matan Azrad
From: Chas Williams
Quoted 1 line
On Mon, Aug 27, 2018 at 11:30 AM Matan Azrad <mailto:[email protected]>
wrote:
<snip>Quoted 3 lines
Quoted 3 lines
Quoted 1 line
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it
Quoted 9 lines
must hurt performance of the bonding PMD
Quoted 1 line
Quoted 1 line
(means the bonding itself - not the slaves PMDs which are called from
the bonding)
Quoted 2 lines
It adds latency.
And by that hurts the application performance because it takes more CPU time in the bonding PMD.
No, as I said before it takes _less_ CPU time in the bonding PMD because we use a more optimal read from the slaves.
Quoted 10 lines
Quoted 2 lines
It increases performance because we spend less CPU time reading from the
PMDs.
So, it's hack in the bonding PMD to improve some slaves code performance but hurt the bonding code performance,
Over all the performance we gain for those slaves improves the application performance only when working with those slaves.
But may hurt the application performance when working with other slaves.
What is your evidence that is hurts bonding performance? Your argument is purely theoretical. I could easily argue than even for non-vectorized PMDs there is a performance gain because we spend less time switching between PMDs. If you are going to read from a PMD you should attempt to read as much as possible. It's expensive to read the cards registers and perform the queue manipulations.
Quoted 97 lines
Quoted 11 lines
This means we have more CPU to use for
post processing (i.e. routing).Quoted 5 lines
Quoted 1 line
Bonding is in a middle ground between application and PMD.
Yes.
Quoted 1 line
What bonding is doing, may not improve all applications.
Yes, but it can be solved using some bonding modes.
Quoted 1 line
If using a ring to buffer the vectorized receive routines, improves
your particular application,
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
that's great.
It may be not great and even bad for some other PMDs which are not
vectororized.
Quoted 17 lines
Quoted 9 lines
Quoted 4 lines
However, I don't think I can say that it would help all applications. As you point out, there is overhead associated with a ring.
Yes.
Quoted 1 line
Bonding's receive burst isn't especially efficient (in mode 4).
Why?
Quoted 7 lines
It makes a copy of the slaves, has a fair bit of stack usage, needs to check the slave status, and needs to examine each packet to see if it is a slow protocol packet. So each packet is essentially read twice. The fast queue code for mode 4 avoids some of this (and probably ignores checking collecting incorrectly). If you find a slow protocol packet, you need to chop it out of the array with memmove due to overlap.
Quoted 2 lines
Agree.
So to improve the bonding performance you need to optimize the abovesproblems.
Quoted 4 lines
There is no connection to the ring.
And as I have described numerous times, these problems can't be easily fixed and preserve the existing API.
Sometimes we need to work harder to see a gain for all. We should not apply a patch because it is easy and show a gain for specific scenarios.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Bonding benefits from being able to read as much as possible (within
limits of
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
course, large reads would blow out caches) from each slave.
The slaves PMDs can benefits in the same way.
Quoted 1 line
It can't return all that data though because applications tend to use
the
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
burst size that would be efficient for a typical PMD.
What is the preferred burst size of the bonding? Maybe the application
should use it when they are using bonding.
Quoted 7 lines
Quoted 7 lines
The preferred burst size for bonding would be the sum of all the slaves ideal read size. However, that's not likely to be simple since most applications decide early the size for the read/write burst operations.
Quoted 1 line
An alternative might be to ask bonding applications to simply issue
larger reads for
Quoted 26 lines
Quoted 8 lines
Quoted 2 lines
certain modes. That's probably not as easy as it sounds given the way that the burst length effects multiplexing.
Can you explain it more?
A longer burst size on one PMD will tend to favor that PMD over others. It will fill your internal queues with more of its packets.
Agree, it's about fairness.
Quoted 6 lines
Quoted 5 lines
Another solution might be just alternatively poll the individual slaves on each rx burst. But that means you need to poll at a faster rate. Depending on your application, you might not be able to do that.
Quoted 8 lines
Again, can you be more precise in the above explanation?
If the application knows that there are two slaves backing a bonding interface, the application could just read twice from the bonding interface, knowing that the bonding interface is going to alternate between the slaves. But this requires the application to know things about the bonding PMD, like the number of slaves.
Quoted 2 lines
Why should the application poll twice?
Poll slave 0, than process it's packets, poll slave 1 than process itspackets...
Quoted 4 lines
What is the problem?
Because let's say that each slave is 10G and you are using
Quoted 9 lines
Quoted 6 lines
link aggregation with two slaves. If you have tuned your
application on the assumption that a PMD is approximately 10G, then you are going to be under polling the bonding PMD. For the above to work, you need to ensure that the application is polling sufficiently to keep up.
But each poll will be shorter, no slaves loops, only one slave call.
But you still need to poll the bonding PMD N times as fast where N is the number of slaves. The "scheduler" in the application may not be aware of that. That was (apparently) the point of the way the current bonding PMD. It hides everything from the application.
Quoted 54 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
We can avoid this scheduling overhead by just
doing the extra reads in bonding and buffering in a ring.Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Since bonding is going to be buffering everything in a ring,
? I don't familiar with it. For now I don't think we need a ring.
We have some empirical testing that shows performance improvements. How do you explain this?
You are using a hack in bonding which hurt the bonding but improves the
vectorized PMD you use.
Quoted 8 lines
Quoted 2 lines
Rings aren't inefficient.
Only when there is a need to use it.
I believe we have identified a need.
Quoted 10 lines
Quoted 2 lines
There's significant overhead of calling the rx burst routine on any PMD. You need to get the PMD data structures into local memory.
Yes.
Quoted 1 line
Reading as much as possible makes sense.
Agree.
Quoted 1 line
Could you generally do this for all PMDs? Yes, but the ring adds
latency. Latency
Quoted 4 lines
Quoted 2 lines
that isn't an issue for the bonding PMD because of all the other inefficiencies (for mode 4).
Enlarging the bonding latency by that way makes some vectorized slave
PMDs happy and makes the bonding worst
Quoted 3 lines
for other slaves PMDs - this is not a good idea to put it upstream.
Improving the other problems in the bonding(reduce the bonding latency)
will do the job for you and for others.
Quoted 6 lines
Latency is not the issue with bonding. The issue with bonding is the overhead associated with making an rx burst call. We add latency (via rings) to make part of the bonding driver more efficient.
Quoted 6 lines
Again, I suspect it even helps the non-vectorized PMDs. Calling a PMD's rx burst routine is costly and we are switching between PMD's inside the bonding driver. Bonding is halfway between an application and a PMD. What we are doing in the bonding PMD is what an application would typically do. But instead of forcing all the bonding users to do this, we are doing it for them.
Quoted 3 lines
Not agree.
The overhead in bonding may be critical for some other PMDs\scenario.
The overhead is bonding is undesirable. It's not easy to address because of the initial design goals.
Quoted 9 lines
To summarize:
We are not agree.Bottom line in my side,
It is not a good Idea to add overhead in the bonding PMD which is a generic PMD just to get gain for some specific scenarios in some specific PMDs while for other scenarios\PMDs it is bad.
You are making some assumptions which are simply not valid. BOnding is _not_ a generic PMD. As discussed above, bonding is somewhere between application and PMD. The choices made for bonding where to make it easy to integrate into an existing application with the application having to know anything about bonding.
Quoted 6 lines
Matan.
From: Chas Williams
Quoted 11 lines · Matan Azrad
From: Chas Williams
Quoted 1 line · Matan Azrad
<snip>
Quoted 5 lines
Quoted 4 lines
Quoted 1 line
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it must hurt performance of the bonding PMD (means the bonding itself - not the slaves PMDs which are called from the bonding)
Quoted 6 lines
Quoted 1 line
It adds latency.
And by that hurts the application performance because it takes more CPU time in the bonding PMD.
No, as I said before it takes _less_ CPU time in the bonding PMD because we use a more optimal read from the slaves.
Each packet pointer should be copied more 2 times because of this patch + some management(the ring overhead) So in the bonding code you lose performance.
Quoted 9 lines
Quoted 2 lines
It increases performance because we spend less CPU time reading from the PMDs.
So, it's hack in the bonding PMD to improve some slaves code performance but hurt the bonding code performance, Over all the performance we gain for those slaves improves the application performance only when working with those slaves. But may hurt the application performance when working with other slaves.
What is your evidence that is hurts bonding performance? Your argument is purely theoretical.
Yes, we cannot test all the scenarios cross the PMDs.
Quoted 2 lines
I could easily argue than even for non-vectorized PMDs there is a performance gain because we spend less time switching between PMDs.
But spend more time in the bonding part.
> If you are going to read from a PMD you should attempt to read as much as possible. It's
Quoted 2 lines
expensive to read the cards registers and perform the queue manipulations.
You do it anyway.
The context changing is expensive but also the extra copies per packet and the ring management.
We have here tradeoff that may be affect differently for other scenarios.
Quoted 95 lines
Quoted 30 lines
This means we have more CPU to use for
post processing (i.e. routing).Quoted 16 lines
Quoted 1 line
Bonding is in a middle ground between application and PMD.
Yes.
Quoted 1 line
What bonding is doing, may not improve all applications.
Yes, but it can be solved using some bonding modes.
Quoted 2 lines
If using a ring to buffer the vectorized receive routines, improves your particular application, that's great.
It may be not great and even bad for some other PMDs which are not vectororized.
Quoted 3 lines
However, I don't think I can say that it would help all applications. As you point out, there is overhead associated with a ring.
Yes.
Quoted 1 line
Bonding's receive burst isn't especially efficient (in mode 4).
Why?
Quoted 7 lines
It makes a copy of the slaves, has a fair bit of stack usage, needs to check the slave status, and needs to examine each packet to see if it is a slow protocol packet. So each packet is essentially read twice. The fast queue code for mode 4 avoids some of this (and probably ignores checking collecting incorrectly). If you find a slow protocol packet, you need to chop it out of the array with memmove due to overlap.
Quoted 6 lines
Agree.
So to improve the bonding performance you need to optimize the aboves problems. There is no connection to the ring.And as I have described numerous times, these problems can't be easily fixed and preserve the existing API.
Sometimes we need to work harder to see a gain for all.
We should not apply a patch because it is easy and show a gain for specific scenarios.Quoted 46 lines
Quoted 19 lines
Quoted 2 lines
Bonding benefits from being able to read as much as possible (within limits of course, large reads would blow out caches) from each slave.
The slaves PMDs can benefits in the same way.
Quoted 2 lines
It can't return all that data though because applications tend to use the burst size that would be efficient for a typical PMD.
What is the preferred burst size of the bonding? Maybe the application should use it when they are using bonding.
The preferred burst size for bonding would be the sum of all the slaves ideal read size. However, that's not likely to be simple since most applications decide early the size for the read/write burst operations.
Quoted 3 lines
An alternative might be to ask bonding applications to simply issue larger reads for certain modes. That's probably not as easy as it sounds given the
way that the burst length effects multiplexing.Quoted 5 lines
Can you explain it more?
A longer burst size on one PMD will tend to favor that PMD over others. It will fill your internal queues with more of its packets.
Agree, it's about fairness.
Quoted 6 lines
Quoted 5 lines
Another solution might be just alternatively poll the individual slaves on each rx burst. But that means you need to poll at a faster rate. Depending on your application, you might not be able to do that.
Quoted 8 lines
Again, can you be more precise in the above explanation?
If the application knows that there are two slaves backing a bonding interface, the application could just read twice from the bonding interface, knowing that the bonding interface is going to alternate between the slaves. But this requires the application to know things about the bonding PMD, like the number of slaves.
Why should the application poll twice?
Poll slave 0, than process it's packets, poll slave 1 than process its packets... What is the problem?Quoted 8 lines
Because let's say that each slave is 10G and you are using
link aggregation with two slaves. If you have tuned your
application on the assumption that a PMD is approximately 10G, then you are going to be under polling the bonding PMD. For the above to work, you need to ensure that the application is polling sufficiently to keep up.
Quoted 7 lines
But each poll will be shorter, no slaves loops, only one slave call.
But you still need to poll the bonding PMD N times as fast where N is the number of slaves. The "scheduler" in the application may not be aware of that. That was (apparently) the point of the way the current bonding PMD. It hides everything from the application.
Sorry, I don't sure I understand you here.
Also here we have tradeoff:
Read full burst from each slave + less bonding code per burst
against
More bonding calls(it is not in N relation it depends in heavy and the burst size).
Quoted 56 lines
Quoted 35 lines
Quoted 9 lines
Quoted 4 lines
We can avoid this scheduling overhead by just doing the extra reads in bonding and buffering in a ring.
Since bonding is going to be buffering everything in a ring,
? I don't familiar with it. For now I don't think we need a ring.
We have some empirical testing that shows performance improvements. How do you explain this?
You are using a hack in bonding which hurt the bonding but improves the vectorized PMD you use.
Quoted 1 line
Rings aren't inefficient.
Only when there is a need to use it.
I believe we have identified a need.
Quoted 2 lines
There's significant overhead of calling the rx burst routine on any PMD. You need to get the PMD data structures into local memory.
Yes.
Quoted 1 line
Reading as much as possible makes sense.
Agree.
Quoted 3 lines
Could you generally do this for all PMDs? Yes, but the ring adds latency. Latency that isn't an issue for the bonding PMD because of all the
other inefficiencies (for mode 4).Enlarging the bonding latency by that way makes some vectorized slave PMDs happy and makes the bonding worst for other slaves PMDs - this is not a good idea to put it upstream.
Quoted 8 lines
Improving the other problems in the bonding(reduce the bonding latency) will do the job for you and for others.
Latency is not the issue with bonding. The issue with bonding is the overhead associated with making an rx burst call. We add latency (via rings) to make part of the bonding driver more efficient.
Again, I suspect it even helps the non-vectorized PMDs.
>Calling a PMD's rx burst routine is costly and we are switching between
Quoted 4 lines
PMD's inside the bonding driver. Bonding is halfway between an application and a PMD. What we are doing in the bonding PMD is what an application would typically do. But instead of forcing all the bonding users to do this, we are doing it for them.
Not agree.
The overhead in bonding may be critical for some other PMDs\scenario.The overhead is bonding is undesirable. It's not easy to address because of the initial design goals.
Quoted 8 lines
To summarize:
We are not agree.Bottom line in my side,
It is not a good Idea to add overhead in the bonding PMD which is a generic PMD just to get gain for some specific scenarios in some specific PMDs while for other scenarios\PMDs it is bad.You are making some assumptions which are simply not valid. BOnding is _not_ a generic PMD.
What?
Should the bonding be good only for specific scenarios with specific PMDs? If no, it is generic.
Quoted 2 lines
As discussed above, bonding
is somewhere between application and PMD.
Yes.
Quoted 4 lines
The choices made
for bonding where to make it easy to integrate into an existing application with the application having to know anything about bonding.
The bonding should be generic at least from the next perspectives:
Different PMDs.
Different application scenarios.
Quoted 6 lines
Matan.
Quoted 41 lines · Matan Azrad
From: Chas Williams
Quoted 16 lines
On Tue, Aug 28, 2018 at 5:51 AM Matan Azrad <mailto:matan@mellanox. com> wrote:
From: Chas Williams
Quoted 2 lines
On Mon, Aug 27, 2018 at 11:30 AM Matan Azrad <mailto:mailto:matan @mellanox.com> wrote:
<snip>
Quoted 7 lines
Quoted 6 lines
Quoted 1 line
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it must hurt performance of the bonding PMD (means the bonding itself - not the slaves PMDs which are called from the bonding)
Quoted 7 lines
Quoted 1 line
It adds latency.
And by that hurts the application performance because it takes more CPU time in the bonding PMD.
No, as I said before it takes _less_ CPU time in the bonding PMD because we use a more optimal read from the slaves.
Each packet pointer should be copied more 2 times because of this patch + some management(the ring overhead)
So in the bonding code you lose performance.Quoted 11 lines
Quoted 3 lines
It increases performance because we spend less CPU time reading from the PMDs.
So, it's hack in the bonding PMD to improve some slaves code performance but hurt the bonding code performance, Over all the performance we gain for those slaves improves the application performance only when working with those slaves. But may hurt the application performance when working with other slaves.
Quoted 4 lines
Quoted 2 lines
What is your evidence that is hurts bonding performance? Your argument is purely theoretical.
Yes, we cannot test all the scenarios cross the PMDs.
Chas has evidence that this helps, a _lot_, in some very common cases. We haven't seen evidence of negative impact anywhere in 2 years. Given this, surely it's not unreasonable to ask to substantiate theoretical arguments with some testing?
Quoted 259 lines
Quoted 3 lines
I could easily argue than even for non-vectorized PMDs there is a performance gain because we
spend less time switching between PMDs.But spend more time in the bonding part.
> If you are going to read from a PMD you should attempt to read as much as possible. It's
Quoted 2 lines
expensive to read the cards registers and perform the queue manipulations.
You do it anyway.
The context changing is expensive but also the extra copies per packet and the ring management.
We have here tradeoff that may be affect differently for other scenarios.
Quoted 116 lines
Quoted 39 lines
This means we have more CPU to use for
post processing (i.e. routing).Quoted 24 lines
Quoted 1 line
Bonding is in a middle ground between application and PMD.
Yes.
Quoted 1 line
What bonding is doing, may not improve all applications.
Yes, but it can be solved using some bonding modes.
Quoted 3 lines
If using a ring to buffer the vectorized receive routines, improves your particular application,
that's great.It may be not great and even bad for some other PMDs which are not vectororized.
Quoted 4 lines
However, I don't think I can say that it would help all applications. As you point out, there is overhead associated with
a ring.Yes.
Quoted 2 lines
Bonding's receive burst isn't especially efficient (in mode 4).
Why?
Quoted 8 lines
It makes a copy of the slaves, has a fair bit of stack usage, needs to check the slave status, and needs to examine each packet to see if it is a slow protocol packet. So each packet is essentially read twice. The fast queue code for mode 4
avoids some of this (and probably ignores checking collecting incorrectly). If you find a slow protocol packet, you need to chop it out of the array with memmove due to overlap.Quoted 7 lines
Agree.
So to improve the bonding performance you need to optimize the aboves problems.
There is no connection to the ring.And as I have described numerous times, these problems can't be easily fixed and preserve the existing API.
Sometimes we need to work harder to see a gain for all. We should not apply a patch because it is easy and show a gain for specific scenarios.
Quoted 56 lines
Quoted 27 lines
Quoted 3 lines
Bonding benefits from being able to read as much as possible (within limits of
course, large reads would blow out caches) from each slave.The slaves PMDs can benefits in the same way.
Quoted 3 lines
It can't return all that data though because applications tend to use the
burst size that would be efficient for a typical PMD.What is the preferred burst size of the bonding? Maybe the application should use it when they are using bonding.
The preferred burst size for bonding would be the sum of all the
slaves ideal read size. However, that's not likely to be simple
since most applications decide early the size for the read/write
burst operations.Quoted 5 lines
An alternative might be to ask bonding applications to simply issue larger reads for
certain modes. That's probably not as easy as it sounds given the
way that the burst length effects multiplexing.Quoted 5 lines
Can you explain it more?
A longer burst size on one PMD will tend to favor that PMD over others. It will fill your internal queues with more of its packets.
Agree, it's about fairness.
Quoted 7 lines
Quoted 7 lines
Another solution might be just alternatively poll the individual
slaves on each rx burst. But that means you need to poll at a
faster rate. Depending on your application, you might not be able to do that.Quoted 8 lines
Again, can you be more precise in the above explanation?
If the application knows that there are two slaves backing a bonding interface, the application could just read twice from the bonding interface, knowing that the bonding interface is going to alternate between the slaves. But this requires the application to know things about the bonding PMD, like the number of slaves.
Why should the application poll twice?
Poll slave 0, than process it's packets, poll slave 1 than process its packets...
What is the problem?Quoted 8 lines
Because let's say that each slave is 10G and you are using
link aggregation with two slaves. If you have tuned your
application on the assumption that a PMD is approximately 10G, then you are going to be under polling the bonding PMD. For the above to work, you need to ensure that the application is polling sufficiently to keep up.
Quoted 8 lines
But each poll will be shorter, no slaves loops, only one slave call.
But you still need to poll the bonding PMD N times as fast where N is the number of slaves. The "scheduler" in the application may not be aware of that. That was (apparently) the point of the way the current bonding PMD. It hides everything from the application.
Sorry, I don't sure I understand you here.
Also here we have tradeoff:
Read full burst from each slave + less bonding code per burst against
More bonding calls(it is not in N relation it depends in heavy and the burst size).Quoted 70 lines
Quoted 45 lines
Quoted 11 lines
Quoted 4 lines
We can avoid this scheduling overhead by just doing the extra reads in bonding and buffering in a ring.
Since bonding is going to be buffering everything in a ring,
? I don't familiar with it. For now I don't think we need a ring.
We have some empirical testing that shows performance improvements.
How do you explain this?You are using a hack in bonding which hurt the bonding but improves the vectorized PMD you use.
Quoted 1 line
Rings aren't inefficient.
Only when there is a need to use it.
I believe we have identified a need.
Quoted 3 lines
There's significant overhead of calling the rx burst routine on any PMD.
You need to get the PMD data structures into local memory.Yes.
Quoted 1 line
Reading as much as possible makes sense.
Agree.
Quoted 4 lines
Could you generally do this for all PMDs? Yes, but the ring adds latency. Latency
that isn't an issue for the bonding PMD because of all the other inefficiencies (for mode 4).Enlarging the bonding latency by that way makes some vectorized slave PMDs happy and makes the bonding worst
for other slaves PMDs - this is not a good idea to put it upstream.Improving the other problems in the bonding(reduce the bonding latency) will do the job for you and for others.
Quoted 6 lines
Latency is not the issue with bonding. The issue with bonding is the overhead associated with making an rx burst call. We add latency (via rings) to make part of the bonding driver more efficient.
Again, I suspect it even helps the non-vectorized PMDs.
>Calling a PMD's rx burst routine is costly and we are switching between
Quoted 4 lines
PMD's inside the bonding driver. Bonding is halfway between an application and a PMD. What we are doing in the bonding PMD is what an application would typically do. But instead of forcing all the bonding users to do this, we are doing it for them.
Not agree.
The overhead in bonding may be critical for some other PMDs\scenario.The overhead is bonding is undesirable. It's not easy to address because of the initial design goals.
To summarize:
We are not agree.Quoted 7 lines
Bottom line in my side,
It is not a good Idea to add overhead in the bonding PMD which is a generic PMD just to get gain for some specific scenarios in some specific PMDs while for other scenarios\PMDs it is bad.You are making some assumptions which are simply not valid. BOnding is _not_ a generic PMD.
What?
Should the bonding be good only for specific scenarios with specific PMDs?
If no, it is generic.Quoted 2 lines
As discussed above, bonding
is somewhere between application and PMD.Yes.
Quoted 4 lines
The choices made
for bonding where to make it easy to integrate into an existing application with the application having to know anything about bonding.The bonding should be generic at least from the next perspectives:
Different PMDs.
Different application scenarios.Quoted 6 lines
Matan.
Signature · 2 lines
Kind regards, Luca Boccassi
Hi Luca\Chas
From: Luca Boccassi
Quoted 45 lines · Matan Azrad
Quoted 44 lines
From: Chas Williams
Quoted 16 lines
On Tue, Aug 28, 2018 at 5:51 AM Matan Azrad <mailto:matan@mellanox. com> wrote:
From: Chas Williams
Quoted 2 lines
On Mon, Aug 27, 2018 at 11:30 AM Matan Azrad <mailto:mailto:matan @mellanox.com> wrote:
<snip>
Quoted 7 lines
Quoted 6 lines
Quoted 1 line
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it must hurt performance of the bonding PMD (means the bonding itself - not the slaves PMDs which are called from the bonding)
Quoted 7 lines
Quoted 1 line
It adds latency.
And by that hurts the application performance because it takes more CPU time in the bonding PMD.
No, as I said before it takes _less_ CPU time in the bonding PMD because we use a more optimal read from the slaves.
Each packet pointer should be copied more 2 times because of this patch + some management(the ring overhead) So in the bonding code you lose performance.
Quoted 13 lines
Quoted 3 lines
It increases performance because we spend less CPU time reading from the PMDs.
So, it's hack in the bonding PMD to improve some slaves code performance but hurt the bonding code performance, Over all the performance we gain for those slaves improves the application performance only when working with those slaves.
But may hurt the application performance when working with other slaves.What is your evidence that is hurts bonding performance? Your argument is purely theoretical.
Quoted 6 lines
Quoted 1 line
Yes, we cannot test all the scenarios cross the PMDs.
Chas has evidence that this helps, a _lot_, in some very common cases. We haven't seen evidence of negative impact anywhere in 2 years. Given this, surely it's not unreasonable to ask to substantiate theoretical arguments with some testing?
What is the common cases of the bond usage?
Do you really know all the variance of the bond usages spreading all over the world?
I’m saying that using a hack in the bond code which helps for some slaves PMDs\application scenarios (your common cases) but hurting the bond code performance and latency is not the right thing to do because it may hurt other scenarios\PMDs using the bond.
Quoted 51 lines · Matan Azrad
Hi Luca\Chas
From: Luca Boccassi
Quoted 47 lines · Matan Azrad
Quoted 45 lines
From: Chas Williams
Quoted 16 lines
On Tue, Aug 28, 2018 at 5:51 AM Matan Azrad <mailto:matan@mellanox. com> wrote:
From: Chas Williams
Quoted 2 lines
On Mon, Aug 27, 2018 at 11:30 AM Matan Azrad <mailto:mailto:matan @mellanox.com> wrote:
<snip>
Quoted 7 lines
Quoted 6 lines
Quoted 1 line
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it must hurt performance of the bonding PMD (means the bonding itself - not the slaves PMDs which are called from the bonding)
Quoted 7 lines
Quoted 1 line
It adds latency.
And by that hurts the application performance because it takes more CPU time in the bonding PMD.
No, as I said before it takes _less_ CPU time in the bonding PMD because we use a more optimal read from the slaves.
Each packet pointer should be copied more 2 times because of this patch + some management(the ring overhead) So in the bonding code you lose performance.
Quoted 11 lines
Quoted 3 lines
It increases performance because we spend less CPU time reading from the PMDs.
So, it's hack in the bonding PMD to improve some slaves code performance but hurt the bonding code performance, Over all the performance we gain for those slaves improves the application performance only when working with those slaves.
But may hurt the application performance when working with other slaves.Quoted 2 lines
What is your evidence that is hurts bonding performance? Your argument is purely theoretical.
Yes, we cannot test all the scenarios cross the PMDs.
Quoted 7 lines
Quoted 4 lines
Chas has evidence that this helps, a _lot_, in some very common cases. We haven't seen evidence of negative impact anywhere in 2 years. Given this, surely it's not unreasonable to ask to substantiate theoretical arguments with some testing?
What is the common cases of the bond usage?
Do you really know all the variance of the bond usages spreading all over the world?
We actually have a fairly large number of deployments using this bonding code across a couple different adapter types (mostly Intel though and some virtual usage). The patch was designed to address starvation of slaves because of the way that vector receives tend on the Intel PMDs. If there isn't enough space
to attempt a vector receive (a minimum of 4 buffers), then the rx burst will return
a value of 0 -- no buffers read. The rx burst in bonding moves to the next adapter.
So this tend to starve any slaves that aren't first in line. The issue doesn't really show up in single streams. You need to run multiple streams that multiplex
across all the slaves.
Quoted 3 lines
I’m saying that using a hack in the bond code which helps for some slaves PMDs\application scenarios (your common cases) but hurting the bond code performance and latency is not the right thing to do because it may hurt other scenarios\PMDs using the bond.
What do you think about the attached patch? It implements an explicit round-robin
for the "first" slave in order to enforce some sort of fairness. Some limited testing
has shown that this our application scan scale polling to read the PMDs fast enough.
Note, I have only tested the 802.3ad paths. The changes are likely necessary for
the other RX burst routines since they should suffer the same issue.
Hi Chas
From: Chas Williams
Quoted 79 lines · Matan Azrad
Quoted 8 lines
Hi Luca\Chas
From: Luca Boccassi
Quoted 4 lines · Matan Azrad
Quoted 3 lines
From: Chas Williams
Quoted 1 line
On Tue, Aug 28, 2018 at 5:51 AM Matan Azrad
<mailto:matan@mellanox.
Quoted 43 lines
Quoted 43 lines
Quoted 42 lines
Quoted 15 lines
com> wrote:
From: Chas Williams
Quoted 2 lines
On Mon, Aug 27, 2018 at 11:30 AM Matan Azrad
<mailto:mailto:matan @mellanox.com> wrote:<snip>
Quoted 7 lines
Quoted 6 lines
Quoted 1 line
Because rings are generally quite efficient.
But you are using a ring in addition to regular array management, it must hurt performance of the bonding PMD (means the bonding itself - not the slaves PMDs which are called from the bonding)
Quoted 7 lines
Quoted 1 line
It adds latency.
And by that hurts the application performance because it takes more CPU time in the bonding PMD.
No, as I said before it takes _less_ CPU time in the bonding PMD because we use a more optimal read from the slaves.
Each packet pointer should be copied more 2 times because of this patch + some management(the ring overhead) So in the bonding code you lose performance.
Quoted 11 lines
Quoted 3 lines
It increases performance because we spend less CPU time reading from the PMDs.
So, it's hack in the bonding PMD to improve some slaves code performance but hurt the bonding code performance, Over all the performance we gain for those slaves improves the application performance only when working with those slaves.
But may hurt the application performance when working with other slaves.Quoted 2 lines
What is your evidence that is hurts bonding performance? Your argument is purely theoretical.
Yes, we cannot test all the scenarios cross the PMDs.
Quoted 7 lines
Quoted 4 lines
Chas has evidence that this helps, a _lot_, in some very common cases. We haven't seen evidence of negative impact anywhere in 2 years. Given this, surely it's not unreasonable to ask to substantiate theoretical arguments with some testing?
What is the common cases of the bond usage?
Do you really know all the variance of the bond usages spreading all overthe world?
We actually have a fairly large number of deployments using this bonding code across a couple different adapter types (mostly Intel though and some virtual usage). The patch was designed to address starvation of slaves because of the way that vector receives tend on the Intel PMDs. If there isn't enough space to attempt a vector receive (a minimum of 4 buffers), then the rx burst will return a value of 0 -- no buffers read. The rx burst in bonding moves to the next adapter.
So this tend to starve any slaves that aren't first in line. The issue doesn't really show up in single streams. You need to run multiple streams that multiplex across all the slaves.Quoted 3 lines
I’m saying that using a hack in the bond code which helps for some slaves PMDs\application scenarios (your common cases) but hurting the
bond code performance and latency is not the right thing to do because it may hurt other scenarios\PMDs using the bond.
Quoted 6 lines
What do you think about the attached patch? It implements an explicit round-robin for the "first" slave in order to enforce some sort of fairness. Some limited testing has shown that this our application scan scale polling to read the PMDs fast enough.
Note, I have only tested the 802.3ad paths. The changes are likely necessary for the other RX burst routines since they should suffer the same issue.
The attached patch idea looks fair to me, no performance\latency impact on the bond code with an improvement for some scenarios. The fairness between slaves is still there.
But please don't use % in the datapath code.
Matan.
Quoted 9 lines · Luca Boccassi
During bond 802.3ad receive, a burst of packets is fetched from each slave into a local array and appended to per-slave ring buffer. Packets are taken from the head of the ring buffer and returned to the caller. The number of mbufs provided to each slave is sufficient to meet the requirements of the ixgbe vector receive.
This small change improves performances of the bonding driver considerably. Vyatta has been using it for years in production.
Quoted 7 lines
Signed-off-by Eric Kinzie <[email protected]>Signed-off-by Luca Boccassi <[email protected]>---
v2 and v3: fix checkpatch warnings
v4: add Eric's original signed-off-by from the Vyatta internal repo
Superseded by: https://patches.dpdk.org/patch/44956/
Signature · 2 lines
Kind regards, Luca Boccassi