On 9/13/22 20:46, fengchengwen wrote:
The main problem is hard to design a tx_prepare for bonding device:
1. as Chas Williams said, there maybe twice hash calc to get target slave
devices.
2. also more important, if the slave devices have changes(e.g. slave device
link down or remove), and if the changes happens between bond-tx-prepare and
bond-tx-burst, the output slave will changes, and this may lead to checksum
failed. (Note: a bond device with slave devices may from different vendors,
and slave devices may have different requirements, e.g. slave-A support calc
IPv4 pseudo-head automatic (no need driver pre-calc), but slave-B need
driver
pre-calc).
Current design cover the above two scenarios by using in-place tx-prepare. and
in addition, bond devices are not transparent to applications, I think it's a
practical method to provide tx-prepare support in this way.
I don't think you need to export an enable/disable routine for the use of
rte_eth_tx_prepare. It's safe to just call that routine, even if it isn't
implemented. You are just trading one branch in DPDK librte_eth_dev for a
branch in drivers/net/bonding.
I think you missed fixing tx_machine in 802.3ad support. We have been using
the following patch locally which I never got around to submitting.
From a458654d68ff5144266807ef136ac3dd2adfcd98 Mon Sep 17 00:00:00 2001
From: "Charles (Chas) Williams" <chwil...@ciena.com>
Date: Tue, 3 May 2022 16:52:37 -0400
Subject: [PATCH] net/bonding: call rte_eth_tx_prepare before rte_eth_tx_burst
Some PMDs might require a call to rte_eth_tx_prepare before sending the
packets for transmission. Typically, the prepare step handles the VLAN
headers, but it may need to do other things.
Signed-off-by: Chas Williams <chwil...@ciena.com>
---
drivers/net/bonding/rte_eth_bond_8023ad.c | 16 +++++++-
drivers/net/bonding/rte_eth_bond_pmd.c | 50 +++++++++++++++++++----
2 files changed, 55 insertions(+), 11 deletions(-)
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c
b/drivers/net/bonding/rte_eth_bond_8023ad.c
index b3cddd8a20..0680be8c06 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -636,9 +636,15 @@ tx_machine(struct bond_dev_private *internals, uint16_t
slave_id)
return;
}
} else {
- uint16_t pkts_sent = rte_eth_tx_burst(slave_id,
+ uint16_t pkts_sent;
+ uint16_t nb_prep;
+
+ nb_prep = rte_eth_tx_prepare(slave_id,
internals->mode4.dedicated_queues.tx_qid,
&lacp_pkt, 1);
+ pkts_sent = rte_eth_tx_burst(slave_id,
+ internals->mode4.dedicated_queues.tx_qid,
+ &lacp_pkt, nb_prep);
if (pkts_sent != 1) {
rte_pktmbuf_free(lacp_pkt);
set_warning_flags(port, WRN_TX_QUEUE_FULL);
@@ -1371,9 +1377,15 @@ bond_mode_8023ad_handle_slow_pkt(struct bond_dev_private
*internals,
}
} else {
/* Send packet directly to the slow queue */
- uint16_t tx_count = rte_eth_tx_burst(slave_id,
+ uint16_t tx_count;
+ uint16_t nb_prep;
+
+ nb_prep = rte_eth_tx_prepare(slave_id,
internals->mode4.dedicated_queues.tx_qid,
&pkt, 1);
+ tx_count = rte_eth_tx_burst(slave_id,
+
internals->mode4.dedicated_queues.tx_qid,
+ &pkt, nb_prep);
if (tx_count != 1) {
/* reset timer */
port->rx_marker_timer = 0;
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
b/drivers/net/bonding/rte_eth_bond_pmd.c
index b305b6a35b..c27073e66f 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -602,8 +602,12 @@ bond_ethdev_tx_burst_round_robin(void *queue, struct
rte_mbuf **bufs,
/* Send packet burst on each slave device */
for (i = 0; i < num_of_slaves; i++) {
if (slave_nb_pkts[i] > 0) {
- num_tx_slave = rte_eth_tx_burst(slaves[i],
bd_tx_q->queue_id,
+ uint16_t nb_prep;
+
+ nb_prep = rte_eth_tx_prepare(slaves[i],
bd_tx_q->queue_id,
slave_bufs[i], slave_nb_pkts[i]);
+ num_tx_slave = rte_eth_tx_burst(slaves[i],
bd_tx_q->queue_id,
+ slave_bufs[i], nb_prep);
/* if tx burst fails move packets to end of bufs */
if (unlikely(num_tx_slave < slave_nb_pkts[i])) {
@@ -628,6 +632,7 @@ bond_ethdev_tx_burst_active_backup(void *queue,
{
struct bond_dev_private *internals;
struct bond_tx_queue *bd_tx_q;
+ uint16_t nb_prep;
bd_tx_q = (struct bond_tx_queue *)queue;
internals = bd_tx_q->dev_private;
@@ -635,8 +640,10 @@ bond_ethdev_tx_burst_active_backup(void *queue,
if (internals->active_slave_count < 1)
return 0;
- return rte_eth_tx_burst(internals->current_primary_port, bd_tx_q->queue_id,
- bufs, nb_pkts);
+ nb_prep = rte_eth_tx_prepare(internals->current_primary_port,
+ bd_tx_q->queue_id, bufs, nb_pkts);
+ return rte_eth_tx_burst(internals->current_primary_port,
+ bd_tx_q->queue_id, bufs, nb_prep);
}
static inline uint16_t
@@ -935,6 +942,8 @@ bond_ethdev_tx_burst_tlb(void *queue, struct rte_mbuf
**bufs, uint16_t nb_pkts)
}
for (i = 0; i < num_of_slaves; i++) {
+ uint16_t nb_prep;
+
rte_eth_macaddr_get(slaves[i], &active_slave_addr);
for (j = num_tx_total; j < nb_pkts; j++) {
if (j + 3 < nb_pkts)
@@ -951,8 +960,10 @@ bond_ethdev_tx_burst_tlb(void *queue, struct rte_mbuf
**bufs, uint16_t nb_pkts)
#endif
}
- num_tx_total += rte_eth_tx_burst(slaves[i], bd_tx_q->queue_id,
+ nb_prep = rte_eth_tx_prepare(slaves[i], bd_tx_q->queue_id,
bufs + num_tx_total, nb_pkts - num_tx_total);
+ num_tx_total += rte_eth_tx_burst(slaves[i], bd_tx_q->queue_id,
+ bufs + num_tx_total, nb_prep);
if (num_tx_total == nb_pkts)
break;
@@ -1064,8 +1075,12 @@ bond_ethdev_tx_burst_alb(void *queue, struct rte_mbuf
**bufs, uint16_t nb_pkts)
/* Send ARP packets on proper slaves */
for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
if (slave_bufs_pkts[i] > 0) {
- num_send = rte_eth_tx_burst(i, bd_tx_q->queue_id,
+ uint16_t nb_prep;
+
+ nb_prep = rte_eth_tx_prepare(i, bd_tx_q->queue_id,
slave_bufs[i], slave_bufs_pkts[i]);
+ num_send = rte_eth_tx_burst(i, bd_tx_q->queue_id,
+ slave_bufs[i], nb_prep);
for (j = 0; j < slave_bufs_pkts[i] - num_send; j++) {
bufs[nb_pkts - 1 - num_not_send - j] =
slave_bufs[i][nb_pkts - 1 - j];
@@ -1088,8 +1103,12 @@ bond_ethdev_tx_burst_alb(void *queue, struct rte_mbuf
**bufs, uint16_t nb_pkts)
/* Send update packets on proper slaves */
for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
if (update_bufs_pkts[i] > 0) {
- num_send = rte_eth_tx_burst(i, bd_tx_q->queue_id,
update_bufs[i],
+ uint16_t nb_prep;
+
+ nb_prep = rte_eth_tx_prepare(i, bd_tx_q->queue_id,
update_bufs[i],
update_bufs_pkts[i]);
+ num_send = rte_eth_tx_burst(i, bd_tx_q->queue_id,
update_bufs[i],
+ nb_prep);
for (j = num_send; j < update_bufs_pkts[i]; j++) {
rte_pktmbuf_free(update_bufs[i][j]);
}
@@ -1155,12 +1174,17 @@ tx_burst_balance(void *queue, struct rte_mbuf **bufs,
uint16_t nb_bufs,
/* Send packet burst on each slave device */
for (i = 0; i < slave_count; i++) {
+ uint16_t nb_prep;
+
if (slave_nb_bufs[i] == 0)
continue;
- slave_tx_count = rte_eth_tx_burst(slave_port_ids[i],
+ nb_prep = rte_eth_tx_prepare(slave_port_ids[i],
bd_tx_q->queue_id, slave_bufs[i],
slave_nb_bufs[i]);
+ slave_tx_count = rte_eth_tx_burst(slave_port_ids[i],
+ bd_tx_q->queue_id, slave_bufs[i],
+ nb_prep);
total_tx_count += slave_tx_count;
@@ -1243,8 +1267,12 @@ tx_burst_8023ad(void *queue, struct rte_mbuf **bufs, uint16_t nb_bufs,
if (rte_ring_dequeue(port->tx_ring,
(void **)&ctrl_pkt) != -ENOENT) {
- slave_tx_count = rte_eth_tx_burst(slave_port_ids[i],
+ uint16_t nb_prep;
+
+ nb_prep = rte_eth_tx_prepare(slave_port_ids[i],
bd_tx_q->queue_id, &ctrl_pkt, 1);
+ slave_tx_count = rte_eth_tx_burst(slave_port_ids[i],
+ bd_tx_q->queue_id, &ctrl_pkt, nb_prep);
/*
* re-enqueue LAG control plane packets to buffering
* ring if transmission fails so the packet isn't lost.
@@ -1322,8 +1350,12 @@ bond_ethdev_tx_burst_broadcast(void *queue, struct
rte_mbuf **bufs,
/* Transmit burst on each active slave */
for (i = 0; i < num_of_slaves; i++) {
- slave_tx_total[i] = rte_eth_tx_burst(slaves[i],
bd_tx_q->queue_id,
+ uint16_t nb_prep;
+
+ nb_prep = rte_eth_tx_prepare(slaves[i], bd_tx_q->queue_id,
bufs, nb_pkts);
+ slave_tx_total[i] = rte_eth_tx_burst(slaves[i],
bd_tx_q->queue_id,
+ bufs, nb_prep);
if (unlikely(slave_tx_total[i] < nb_pkts))
tx_failed_flag = 1;
--
2.30.2
