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:

quoted

Hi Ferruh,

On 12/21/18 3:43 PM, Ferruh Yigit wrote:

quoted

On 12/21/2018 12:28 PM, Andrew Rybchenko wrote:

quoted

On 12/21/18 3:12 PM, Ferruh Yigit wrote:

quoted

On 10/12/2018 12:36 PM, Andrew Rybchenko wrote:

quoted

From: Ivan Malov <redacted>

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.

quoted

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.

quoted

quoted

quoted

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.

quoted

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.

quoted

quoted

quoted

quoted

Signed-off-by: Ivan Malov <redacted>
Signed-off-by: Andrew Rybchenko <redacted>
---
   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 |