DPDK patches and discussions
 help / color / mirror / Atom feed
From: fengchengwen <fengchengwen@huawei.com>
To: Ferruh Yigit <ferruh.yigit@xilinx.com>, <thomas@monjalon.net>,
	<andrew.rybchenko@oktetlabs.ru>, <konstantin.ananyev@intel.com>
Cc: <dev@dpdk.org>, <chas3@att.com>, <humin29@huawei.com>,
	<3chas3@gmail.com>
Subject: Re: [PATCH v2 1/3] net/bonding: support Tx prepare
Date: Wed, 14 Sep 2022 08:46:01 +0800
Message-ID: <4b4af3e8-710a-ae75-8171-331ebfe4e4f7@huawei.com> (raw)
In-Reply-To: <495fb2f0-60c2-f1c9-2985-0d08bb463ad0@xilinx.com>

Hi, Ferruh

On 2022/9/13 18:22, Ferruh Yigit wrote:
> On 7/25/2022 5:08 AM, Chengwen Feng wrote:
> 
>>
>> Normally, to use the HW offloads capability (e.g. checksum and TSO) in
>> the Tx direction, the application needs to call rte_eth_dev_prepare to
>> do some adjustment with the packets before sending them (e.g. processing
>> pseudo headers when Tx checksum offload enabled). But, the tx_prepare
>> callback of the bonding driver is not implemented. Therefore, the
>> sent packets may have errors (e.g. checksum errors).
>>
>> However, it is difficult to design the tx_prepare callback for bonding
>> driver. Because when a bonded device sends packets, the bonded device
>> allocates the packets to different slave devices based on the real-time
>> link status and bonding mode. That is, it is very difficult for the
>> bonded device to determine which slave device's prepare function should
>> be invoked.
>>
>> So, in this patch, the tx_prepare callback of bonding driver is not
>> implemented. Instead, the rte_eth_dev_tx_prepare() will be called for
>> all the fast path packets in mode 0, 1, 2, 4, 5, 6 (mode 3 is not
>> included, see[1]). In this way, all tx_offloads can be processed
>> correctly for all NIC devices in these modes.
>>
>> As previously discussed (see V1), if the tx_prepare fails, the bonding
>> driver will free the cossesponding packets internally, and only the
>> packets of the tx_prepare OK are xmit.
>>
> 
> Please provide link to discussion you refer to.

https://inbox.dpdk.org/dev/1618571071-5927-2-git-send-email-tangchengchang@huawei.com/

Should I push a new version for it?

> 
>> To minimize performance impact, this patch adds one new
>> 'tx_prepare_enabled' field, and corresponding control and get API:
>> rte_eth_bond_tx_prepare_set() and rte_eth_bond_tx_prepare_get().
>>
>> [1]: In bond mode 3 (broadcast), a packet needs to be sent by all slave
>> ports. Different slave PMDs process the packets differently in
>> tx_prepare. If call tx_prepare before each slave port sending, the sent
>> packet may be incorrect.
>>
>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> 
> <...>
> 
>> +static inline uint16_t
>> +bond_ethdev_tx_wrap(struct bond_tx_queue *bd_tx_q, uint16_t slave_port_id,
>> +                   struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>> +{
>> +       struct bond_dev_private *internals = bd_tx_q->dev_private;
>> +       uint16_t queue_id = bd_tx_q->queue_id;
>> +       struct rte_mbuf *fail_pkts[nb_pkts];
>> +       uint8_t fail_mark[nb_pkts];
>> +       uint16_t nb_pre, index;
>> +       uint16_t fail_cnt = 0;
>> +       int i;
>> +
>> +       if (!internals->tx_prepare_enabled)
>> +               goto tx_burst;
>> +
>> +       nb_pre = rte_eth_tx_prepare(slave_port_id, queue_id, tx_pkts, nb_pkts);
>> +       if (nb_pre == nb_pkts)
>> +               goto tx_burst;
>> +
>> +       fail_pkts[fail_cnt++] = tx_pkts[nb_pre];
>> +       memset(fail_mark, 0, sizeof(fail_mark));
>> +       fail_mark[nb_pre] = 1;
>> +       for (i = nb_pre + 1; i < nb_pkts; /* update in inner loop */) {
>> +               nb_pre = rte_eth_tx_prepare(slave_port_id, queue_id,
>> +                                           tx_pkts + i, nb_pkts - i);
> 
> 
> I assume intention is to make this as transparent as possible to the user, that is why you are using a wrapper that combines `rte_eth_tx_prepare()` & `rte_eth_tx_burst()` APIs. But for other PMDs `rte_eth_tx_burst()` is called explicitly by the application.
> 
> Path is also adding two new bonding specific APIs to enable/disable Tx prepare.
> Instead if you leave calling `rte_eth_tx_prepare()` decision to user, there will be no need for the enable/disable Tx prepare APIs and the wrapper.
> 
> The `tx_pkt_prepare()` implementation in bonding can do the mode check, call Tx prepare for all slaves and apply failure recovery, as done in this wrapper function, what do you think, will it work?

I see Chas Williams also reply this thread, thanks.

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.

> 
> .

  parent reply	other threads:[~2022-09-14  0:46 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-16 11:04 [dpdk-dev] [RFC 0/2] add Tx prepare support for bonding device Chengchang Tang
2021-04-16 11:04 ` [dpdk-dev] [RFC 1/2] net/bonding: add Tx prepare for bonding Chengchang Tang
2021-04-16 11:04 ` [dpdk-dev] [RFC 2/2] app/testpmd: add cmd for bonding Tx prepare Chengchang Tang
2021-04-16 11:12 ` [dpdk-dev] [RFC 0/2] add Tx prepare support for bonding device Min Hu (Connor)
2021-04-20  1:26 ` Ferruh Yigit
2021-04-20  2:44   ` Chengchang Tang
2021-04-20  8:33     ` Ananyev, Konstantin
2021-04-20 12:44       ` Chengchang Tang
2021-04-20 13:18         ` Ananyev, Konstantin
2021-04-20 14:06           ` Chengchang Tang
2021-04-23  9:46 ` [dpdk-dev] [PATCH " Chengchang Tang
2021-04-23  9:46   ` [dpdk-dev] [PATCH 1/2] net/bonding: support Tx prepare for bonding Chengchang Tang
2021-06-08  9:49     ` Andrew Rybchenko
2021-06-09  6:42       ` Chengchang Tang
2021-06-09  9:35         ` Andrew Rybchenko
2021-06-10  7:32           ` Chengchang Tang
2021-06-14 14:16             ` Andrew Rybchenko
2021-06-09 10:25         ` Ananyev, Konstantin
2021-06-10  6:46           ` Chengchang Tang
2021-06-14 11:36             ` Ananyev, Konstantin
2022-05-24 12:11       ` Min Hu (Connor)
2022-07-25  4:08     ` [PATCH v2 0/3] add Tx prepare support for bonding driver Chengwen Feng
2022-07-25  4:08       ` [PATCH v2 1/3] net/bonding: support Tx prepare Chengwen Feng
2022-09-13 10:22         ` Ferruh Yigit
2022-09-13 15:08           ` Chas Williams
2022-09-14  0:46           ` fengchengwen [this message]
2022-09-14 16:59             ` Chas Williams
2022-09-17  2:35               ` fengchengwen
2022-09-17 13:38                 ` Chas Williams
2022-09-19 14:07                   ` Konstantin Ananyev
2022-09-19 23:02                     ` Chas Williams
2022-09-22  2:12                       ` fengchengwen
2022-09-25 10:32                         ` Chas Williams
2022-09-26 10:18                       ` Konstantin Ananyev
2022-09-26 16:36                         ` Chas Williams
2022-07-25  4:08       ` [PATCH v2 2/3] net/bonding: support Tx prepare fail stats Chengwen Feng
2022-07-25  4:08       ` [PATCH v2 3/3] net/bonding: add testpmd cmd for Tx prepare Chengwen Feng
2022-07-25  7:04       ` [PATCH v2 0/3] add Tx prepare support for bonding driver humin (Q)
2022-09-13  1:41       ` fengchengwen
2022-09-17  4:15     ` [PATCH v3 " Chengwen Feng
2022-09-17  4:15       ` [PATCH v3 1/3] net/bonding: support Tx prepare Chengwen Feng
2022-09-17  4:15       ` [PATCH v3 2/3] net/bonding: support Tx prepare fail stats Chengwen Feng
2022-09-17  4:15       ` [PATCH v3 3/3] net/bonding: add testpmd cmd for Tx prepare Chengwen Feng
2022-10-09  3:36     ` [PATCH v4] net/bonding: call Tx prepare before Tx burst Chengwen Feng
2022-10-10 19:42       ` Chas Williams
2022-10-11 13:28         ` fengchengwen
2022-10-11 13:20     ` [PATCH v5] " Chengwen Feng
2022-10-15 15:26       ` Chas Williams
2022-10-18 14:25         ` fengchengwen
2022-10-20  7:07         ` Andrew Rybchenko
2021-04-23  9:46   ` [dpdk-dev] [PATCH 2/2] net/bonding: support configuring Tx offloading for bonding Chengchang Tang
2021-06-08  9:49     ` Andrew Rybchenko
2021-06-09  6:57       ` Chengchang Tang
2021-06-09  9:11         ` Ananyev, Konstantin
2021-06-09  9:37           ` Andrew Rybchenko
2021-06-10  6:29             ` Chengchang Tang
2021-06-14 11:05               ` Ananyev, Konstantin
2021-06-14 14:13                 ` Andrew Rybchenko
2021-04-30  6:26   ` [dpdk-dev] [PATCH 0/2] add Tx prepare support for bonding device Chengchang Tang
2021-04-30  6:47     ` Min Hu (Connor)
2021-06-03  1:44   ` Chengchang Tang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4b4af3e8-710a-ae75-8171-331ebfe4e4f7@huawei.com \
    --to=fengchengwen@huawei.com \
    --cc=3chas3@gmail.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=chas3@att.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@xilinx.com \
    --cc=humin29@huawei.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git