From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ferruh.yigit@intel.com>
Received: from mga06.intel.com (mga06.intel.com [134.134.136.31])
 by dpdk.org (Postfix) with ESMTP id 4314B1BDFB
 for <dev@dpdk.org>; Fri, 21 Dec 2018 14:59:23 +0100 (CET)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga003.fm.intel.com ([10.253.24.29])
 by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 21 Dec 2018 05:59:22 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.56,381,1539673200"; d="scan'208";a="120237225"
Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.46])
 ([10.237.221.46])
 by FMSMGA003.fm.intel.com with ESMTP; 21 Dec 2018 05:59:21 -0800
To: =?UTF-8?Q?Ga=c3=abtan_Rivet?= <gaetan.rivet@6wind.com>,
 Andrew Rybchenko <arybchenko@solarflare.com>
Cc: dev@dpdk.org, Ivan Malov <ivan.malov@oktetlabs.ru>
References: <1539344187-21481-1-git-send-email-arybchenko@solarflare.com>
 <a5b9324f-6dbe-5a29-ab1d-b1d7cd1a4043@intel.com>
 <c934d355-919f-9bb6-964f-886dd48a9443@solarflare.com>
 <585c9670-07b6-abfa-027d-e4d07febb7d4@intel.com>
 <969cb7e3-6e1a-f5ac-ed1b-e4334f928b17@solarflare.com>
 <20181221131646.2yqimi7rejnebrvd@bidouze.vm.6wind.com>
From: Ferruh Yigit <ferruh.yigit@intel.com>
Openpgp: preference=signencrypt
Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata=
 mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy
 qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ
 +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9
 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb
 +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF
 YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy
 ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX
 CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1
 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz
 cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln
 aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJVBBMBAgA/AhsDBgsJCAcDAgYVCAIJCgsE
 FgIDAQIeAQIXgBYhBNI2U4dCLsKE45mBx/kz60PfE2EfBQJbughWBQkHwjOGAAoJEPkz60Pf
 E2Eft84QAIbKWqhgqRfoiw/BbXbA1+qm2o4UgkCRQ0yJgt9QsnbpOmPKydHH0ixCliNz1J8e
 mRXCkMini1bTpnzp7spOjQGLeAFkNFz6BMq8YF2mVWbGEDE9WgnAxZdi0eLY7ZQnHbE6AxKL
 SXmpe9INb6z3ztseFt7mqje/W/6DWYIMnH3Yz9KzxujFWDcq8UCAvPkxVQXLTMpauhFgYeEx
 Nub5HbvhxTfUkapLwRQsSd/HbywzqZ3s/bbYMjj5JO3tgMiM9g9HOjv1G2f1dQjHi5YQiTZl
 1eIIqQ3pTic6ROaiZqNmQFXPsoOOFfXF8nN2zg8kl/sSdoXWHhama5hbwwtl1vdaygQYlmdK
 H2ueiFh/UvT3WG3waNv2eZiEbHV8Rk52Xyn2w1G90lV0fYC6Ket1Xjoch7kjwbx793Kz/RfQ
 rmBY8/S4DTGn3oq3dMdQY+b6+7VMUeLMMh2CXYO9ErkOq+qNTD1IY+cBAkXnaDbQfz0zbste
 ZGWH74FAZ9nCpDOqbRTrBL42aMGhfOWEyeA1x7+hl6JZfabBWAuf4nnCXuorKHzBXTrf7u7p
 fXsKQClWRW77PF1VmzrtKNVSytQAmlCWApQIw20AarFipXmVdIjHmJPU611WoyxZPb4JTOxx
 5cv9B+nr/RIB+v5dcStyHCCwO1be7nBDdCgd4F6kTQPLuQINBFfWTL4BEACnNA29e8TarUsB
 L5n6eLZHXcFvVwNLVlirWOClHXf44o2KnN3ww+eBEmKVfEFo9MSuGDNHS8Zw1NiGMYxLIUgd
 U6gGrVVs/VrQWL82pbMk6jCj98N+BXIri+6K1z+AImz7ax7iF1kDgRAnFWU0znWWBgM2mM8Y
 gDjcxfXk4sCKnvf6Gjo08Ey5zmqx7dekAKU2EEp8Q1EJY3jbymLdZWRP4AFFMTS1rGMk0/tt
 v71NBg1GobCcbNfn9chK/jhqxYhAJqq86RdJQkt3/9x1U1Oq0vXCt4JVVHmkxePtUiuWTTt+
 aYlUAsKYZsWvncExvw77x2ArYDmaK0yfjh37wp0lY7DOJHFxoyT8tyWZlLci/VMRG2Ja33xj
 0CN4C1yBg+QDeV3QFxQo42iA/ykdXPUR3ezmsND3XKvVLTC4DNb3V/EZQ7jBj64+bEK0VW4G
 B31VP00ApNQvSoczsIOAKdk97RNbpmPw6q10ILIB+9T1xbnFYzshzGF17oC0/GENIHATx8vZ
 masOZoDiOZQpeneLgnFE9JfzhLTxv6wNZcc/HLXRQVTkDsQr8ERtkAoHCf1E5+b5Yr7pfnE4
 YuhET746o25S53ELUYPIs49qoJsEJL34/oexMfPGyPIlrbufiNyty5jc/1MRwUlhJlJ5IOHy
 ZUa+6CLR7GdImusFkPJUJwARAQABiQI8BBgBAgAmAhsMFiEE0jZTh0IuwoTjmYHH+TPrQ98T
 YR8FAlu6CHAFCQXE7zIACgkQ+TPrQ98TYR9nXxAAqNBgkYNyGuWUuy0GwDQCbu3iiMyH1+D7
 llafPcK4NYy1Z4AYuVwC9nmLaoj+ozdqS3ncRo57ncRsKEJC46nDJJZYZ5LSJVn63Y3NBF86
 lxQAgjj2oyZEwaLKtKbAFsXL43jv1pUGgSvWwYtDwHITXXFQto9rZEuUDRFSx4sg9OR+Q6/6
 LY+nQQ3OdHlBkflzYMPcWgDcvcTAO6yasLEUf7UcYoSWTyMYjLB4QuNlXzTswzGVMssJF/vo
 V8lD1eqqaSUWG3STF6GVLQOr1NLvN5+kUBiEStHFxBpgSCvYY9sNV8FS6N24CAWMBl+10W+D
 2h1yiiP5dOdPcBDYKsgqDD91/sP0WdyMJkwdQJtD49f9f+lYloxHnSAxMleOpyscg1pldw+i
 mPaUY1bmIknLhhkqfMmjywQOXpac5LRMibAAYkcB8v7y3kwELnt8mhqqZy6LUsqcWygNbH/W
 K3GGt5tRpeIXeJ25x8gg5EBQ0Jnvp/IbBYQfPLtXH0Myq2QuAhk/1q2yEIbVjS+7iowEZNyE
 56K63WBJxsJPB2mvmLgn98GqB4G6GufP1ndS0XDti/2K0o8rep9xoY/JDGi0n0L0tk9BHyoP
 Y7kaEpu7UyY3nVdRLe5H1/MnFG8hdJ97WqnPS0buYZlrbTV0nRFL/NI2VABl18vEEXvNQiO+ vM8=
Message-ID: <305c20be-125f-b521-56a9-93c4f3c79076@intel.com>
Date: Fri, 21 Dec 2018 13:59:20 +0000
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.3.3
MIME-Version: 1.0
In-Reply-To: <20181221131646.2yqimi7rejnebrvd@bidouze.vm.6wind.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free
	capability
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 21 Dec 2018 13:59:23 -0000

On 12/21/2018 1:16 PM, Gaƫtan Rivet wrote:
> Hi Ferruh, Andrew,
> 
> On Fri, Dec 21, 2018 at 03:52:07PM +0300, Andrew Rybchenko wrote:
>> Hi Ferruh,
>>
>> On 12/21/18 3:43 PM, Ferruh Yigit wrote:
>>> On 12/21/2018 12:28 PM, Andrew Rybchenko wrote:
>>>> On 12/21/18 3:12 PM, Ferruh Yigit wrote:
>>>>> On 10/12/2018 12:36 PM, Andrew Rybchenko wrote:
>>>>>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>>>>>
>>>>>> This capability is reported when supported by the current emitting
>>>>>> sub-device. Failsafe PMD itself does not excercise fast free logic.
>>>>> I think overlay device capability reporting already discussed a few times, the
>>>>> question is if an overlay devices should claim a feature when it depends on
>>>>> underlay devices?
>>>> The capability may be reported by the failsafe since it is transparent from
>>>> fast free logic point of view.
>>> Why it is transparent? If one of the underlying device doesn't support/claim
>>> this feature, application can't use this feature with failsafe, isn't it?
> 
> If a VF is forbidden by its PF from adding MAC addresses, the driver
> should still advertize support for "Unicast MAC filter" right?
> 
> This is the same here. Fail-safe needs to forward configurations items
> to its sub-device for a feature to work. Sometimes, the hardware won't
> be sufficient. But the fail-safe itself still has the parts needed (even
> if it is only a flag to add to a feature list).

I see your point, also I think it may be misleading for an overlay device to
announce a feature which is completely depends underlay devices. Anyway this may
be a nuance.

I am OK with the change after Andrew's explanation, and as far as I understand
you are OK too, may I add your explicit ack to the patch?

> 
> It is necessary, at the fail-safe level, to be able to describe the
> current feature set. This is what the feature list is for. There are
> important caveats to consider however, which is inherent to using the
> fail-safe.
> 
> It does not mean those features should be removed from the fail-safe
> feature list.
> 
>>
>> tx_offload_capa in failsafe is a mask to apply on sub-device capabilities.
>> So, if the capability is not supported by any sub-device it will not be
>> reported.
>> As well if there is the capability bit in the mask, it will not be reported
>> regardless
>> sub-devices capabilities. The description for the patch above tries to
>> explain it -
>> it looks like not that successful.
>>
>>>>> Given that no ack/review given to the patch, I am updating it as rejected.
>>>> Is it a new policy? I thought that it was vice versa before.
>>> Hi Andrew,
>>>
>>> Yes policy is other-way around indeed, when there is no comment at all default
>>> behavior is accept, but please take above paragraph as my comment to the patch.
>>
>> Got it.
>>
>>> And I was thinking it is a little controversial and there is no support to have
>>> it, so lets don't get it. What do you think?
>>
>> I see you motivation.
>>
>>>>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>>>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>>>> ---
>>>>>>    doc/guides/nics/features/failsafe.ini | 1 +
>>>>>>    drivers/net/failsafe/failsafe_ops.c   | 1 +
>>>>>>    2 files changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
>>>>>> index e3c4c08f2..b6f3dcee6 100644
>>>>>> --- a/doc/guides/nics/features/failsafe.ini
>>>>>> +++ b/doc/guides/nics/features/failsafe.ini
>>>>>> @@ -7,6 +7,7 @@
>>>>>>    Link status          = Y
>>>>>>    Link status event    = Y
>>>>>>    Rx interrupt         = Y
>>>>>> +Fast mbuf free       = Y
>>>>>>    Queue start/stop     = Y
>>>>>>    Runtime Rx queue setup = Y
>>>>>>    Runtime Tx queue setup = Y
>>>>>> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
>>>>>> index 7f8bcd4c6..e3add404b 100644
>>>>>> --- a/drivers/net/failsafe/failsafe_ops.c
>>>>>> +++ b/drivers/net/failsafe/failsafe_ops.c
>>>>>> @@ -78,6 +78,7 @@ static struct rte_eth_dev_info default_infos = {
>>>>>>    		DEV_RX_OFFLOAD_SECURITY,
>>>>>>    	.tx_offload_capa =
>>>>>>    		DEV_TX_OFFLOAD_MULTI_SEGS |
>>>>>> +		DEV_TX_OFFLOAD_MBUF_FAST_FREE |
>>>>>>    		DEV_TX_OFFLOAD_IPV4_CKSUM |
>>>>>>    		DEV_TX_OFFLOAD_UDP_CKSUM |
>>>>>>    		DEV_TX_OFFLOAD_TCP_CKSUM |
>>>>>>
>>
>