From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 43A30A00C5; Wed, 14 Sep 2022 18:59:37 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 25E134021D; Wed, 14 Sep 2022 18:59:37 +0200 (CEST) Received: from mail-qv1-f45.google.com (mail-qv1-f45.google.com [209.85.219.45]) by mails.dpdk.org (Postfix) with ESMTP id 82A9940156 for ; Wed, 14 Sep 2022 18:59:35 +0200 (CEST) Received: by mail-qv1-f45.google.com with SMTP id o13so12203647qvw.12 for ; Wed, 14 Sep 2022 09:59:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date; bh=oS72WlDDTC9Wnpr02UkW8BbvLFPqd6qOwpLZwyPt0zQ=; b=osraA/GMROQocoOMOvkzEKpwDKuaTGPgEhmAbKM3c6vstiC/L6G8dIAsJFJotei0A4 vNZENN2K/pwBZIXGemwMY2Yge5jq5w/svdDUwoXUooFVbY0nSJpoco+xV0ISToqhkKzJ KpvCqbRQ/Tq9nQC+CuQxQzya2eeM2rOqreHeFitXVDuPIu1nTfFGc28ll742m646toxo aWvt36Nx8dMvJ06m2Dgh08O39q9MTBqZgxZ2AFOta/PVH1ElKatjLP+kn1juCrMuYvos tyhaI3AbBmsM1ZkAUx3Kv8ZMVEiUZJC2m4x6yE9v3HJIMjkevZCbmycEVNv7Bi1L4WGg hP0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date; bh=oS72WlDDTC9Wnpr02UkW8BbvLFPqd6qOwpLZwyPt0zQ=; b=u/97u7rbTigIGapdpF9qzgXhhLeJGgmVWvjM154gjWSSnejTqcW+R+Dz7CaydAPiuL Uu5y60KyuC7s9nNtHvIdGF01OUXawZXg86nG7CGgDd1Z+G8jvT1eJYVZKZ2CKhLKhjuy ncZPmJnhiaF1oHs02wzL0lalsVW2COqZ5VvdvhLozgFEuJAmCrsTprZWSAco0gygt+n2 UZ2+xd3u1yZe0XA7VAxARpx0Y9+g5gXAfiz+TJCeyHjEDxisD/FySIPx3UTY8YUVCa/U naobDSZ5718HofkKlkPhGm662mxtobYtudS8qHv3NXsgsFUo/pQlj45EBqnLqQxlc26h IdPg== X-Gm-Message-State: ACgBeo0xTotlYn6E10KhzM+7+jVRgOMijTdKMGQGqTFDVAjAyNfDBohC 9p9Qoz1xusfXRdhHcUFVLHs= X-Google-Smtp-Source: AA6agR7pqu+4bo6WhZFe0OJuHQjU9TEYQTKRG/mg6qs5W9CQkGy6GxZvqeTcNxhPM34tGklS2dBjwQ== X-Received: by 2002:a0c:8045:0:b0:476:dbbe:f169 with SMTP id 63-20020a0c8045000000b00476dbbef169mr32690943qva.90.1663174774877; Wed, 14 Sep 2022 09:59:34 -0700 (PDT) Received: from ?IPV6:2600:4040:225b:ea00:6063:8c9b:774a:6cf4? ([2600:4040:225b:ea00:6063:8c9b:774a:6cf4]) by smtp.googlemail.com with ESMTPSA id f12-20020a05620a408c00b006bb78d095c5sm2395689qko.79.2022.09.14.09.59.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 14 Sep 2022 09:59:34 -0700 (PDT) Message-ID: <6c91f993-b11d-987c-6d20-38ee11f9f9db@gmail.com> Date: Wed, 14 Sep 2022 12:59:33 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH v2 1/3] net/bonding: support Tx prepare Content-Language: en-US To: fengchengwen , Ferruh Yigit , thomas@monjalon.net, andrew.rybchenko@oktetlabs.ru, konstantin.ananyev@intel.com Cc: dev@dpdk.org, chas3@att.com, humin29@huawei.com References: <1619171202-28486-2-git-send-email-tangchengchang@huawei.com> <20220725040842.35027-1-fengchengwen@huawei.com> <20220725040842.35027-2-fengchengwen@huawei.com> <495fb2f0-60c2-f1c9-2985-0d08bb463ad0@xilinx.com> <4b4af3e8-710a-ae75-8171-331ebfe4e4f7@huawei.com> From: Chas Williams <3chas3@gmail.com> In-Reply-To: <4b4af3e8-710a-ae75-8171-331ebfe4e4f7@huawei.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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" 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 --- 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