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 CF450A0C46; Wed, 9 Jun 2021 11:37:12 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6F8BD4069B; Wed, 9 Jun 2021 11:37:12 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id B86F14003C for ; Wed, 9 Jun 2021 11:37:10 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 51BC47F52B; Wed, 9 Jun 2021 12:37:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 51BC47F52B DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1623231430; bh=KNKnSmGi2as+ZsO2aMNkDLjvwja11D9IA9L2AK82eeU=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=OJWneRsLg126mihFyofusHxPx+TA6esLptNX5j3mrJsrbFBpqPsA04HtrHLbnhGkd +dondUkiquSXdoxTrz1x07iSTM5rpRJKRcrdFeQFypMuydrXRbzVoo3IQkHoyhampo LY+gmqgdx3Galec/fgiiKiKSHBnudY9FRhYRdK7M= To: "Ananyev, Konstantin" , Chengchang Tang , "dev@dpdk.org" Cc: "linuxarm@huawei.com" , "chas3@att.com" , "humin29@huawei.com" , "Yigit, Ferruh" References: <1618571071-5927-1-git-send-email-tangchengchang@huawei.com> <1619171202-28486-1-git-send-email-tangchengchang@huawei.com> <1619171202-28486-3-git-send-email-tangchengchang@huawei.com> <4b1e8435-c25c-b490-c196-9aebdee5733a@huawei.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <4fa26208-464d-e255-e5e7-21d4e0160bab@oktetlabs.ru> Date: Wed, 9 Jun 2021 12:37:10 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 2/2] net/bonding: support configuring Tx offloading for bonding 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 Sender: "dev" On 6/9/21 12:11 PM, Ananyev, Konstantin wrote: > >> >> >> On 2021/6/8 17:49, Andrew Rybchenko wrote: >>> "for bonding" is redundant in the summary since it is already >>> "net/bonding" >>> >>> On 4/23/21 12:46 PM, Chengchang Tang wrote: >>>> Currently, the TX offloading of the bonding device will not take effect by >>> >>> TX -> Tx >>> >>>> using dev_configure. Because the related configuration will not be >>>> delivered to the slave devices in this way. >>> >>> I think it is a major problem that Tx offloads are actually >>> ignored. It should be a patches with "Fixes:" which addresses >>> it. >>> >>>> The Tx offloading capability of the bonding device is the intersection of >>>> the capability of all slave devices. Based on this, the following functions >>>> are added to the bonding driver: >>>> 1. If a Tx offloading is within the capability of the bonding device (i.e. >>>> all the slave devices support this Tx offloading), the enabling status of >>>> the offloading of all slave devices depends on the configuration of the >>>> bonding device. >>>> >>>> 2. For the Tx offloading that is not within the Tx offloading capability >>>> of the bonding device, the enabling status of the offloading on the slave >>>> devices is irrelevant to the bonding device configuration. And it depends >>>> on the original configuration of the slave devices. >>>> >>>> Signed-off-by: Chengchang Tang >>>> --- >>>> drivers/net/bonding/rte_eth_bond_pmd.c | 13 +++++++++++++ >>>> 1 file changed, 13 insertions(+) >>>> >>>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c >>>> index 84af348..9922657 100644 >>>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c >>>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c >>>> @@ -1712,6 +1712,8 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, >>>> struct rte_flow_error flow_error; >>>> >>>> struct bond_dev_private *internals = bonded_eth_dev->data->dev_private; >>>> + uint64_t tx_offload_cap = internals->tx_offload_capa; >>>> + uint64_t tx_offload; >>>> >>>> /* Stop slave */ >>>> errval = rte_eth_dev_stop(slave_eth_dev->data->port_id); >>>> @@ -1759,6 +1761,17 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev, >>>> slave_eth_dev->data->dev_conf.rxmode.offloads &= >>>> ~DEV_RX_OFFLOAD_JUMBO_FRAME; >>>> >>>> + while (tx_offload_cap != 0) { >>>> + tx_offload = 1ULL << __builtin_ctzll(tx_offload_cap); >>>> + if (bonded_eth_dev->data->dev_conf.txmode.offloads & tx_offload) >>>> + slave_eth_dev->data->dev_conf.txmode.offloads |= >>>> + tx_offload; >>>> + else >>>> + slave_eth_dev->data->dev_conf.txmode.offloads &= >>>> + ~tx_offload; >>>> + tx_offload_cap &= ~tx_offload; >>>> + } >>>> + >>> >>> Frankly speaking I don't understand why it is that complicated. >>> ethdev rejects of unsupported Tx offloads. So, can't we simply: >>> slave_eth_dev->data->dev_conf.txmode.offloads = >>> bonded_eth_dev->data->dev_conf.txmode.offloads; >>> >> >> Using such a complicated method is to increase the flexibility of the slave devices, >> allowing the Tx offloading of the slave devices to be incompletely consistent with >> the bond device. If some offloading can be turned on without bond device awareness, >> they can be retained in this case. > > > Not sure how that can that happen... +1 @Chengchang could you provide an example how it could happen. > From my understanding tx_offload for bond device has to be intersection of tx_offloads > of all slaves, no? Otherwise bond device might be misconfigured. > Anyway for that code snippet above, wouldn't the same be achived by: > slave_eth_dev->data->dev_conf.txmode.offloads &= internals->tx_offload_capa & bonded_eth_dev->data->dev_conf.txmode.offloads; > ?