DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>,
	dev@dpdk.org, Ivan Malov <ivan.malov@oktetlabs.ru>
Subject: Re: [dpdk-dev] net/failsafe: add default Tx mbuf fast free capability
Date: Fri, 21 Dec 2018 15:44:21 +0100	[thread overview]
Message-ID: <20181221144421.szwkakqp7vbn5v5v@bidouze.vm.6wind.com> (raw)
In-Reply-To: <305c20be-125f-b521-56a9-93c4f3c79076@intel.com>

On Fri, Dec 21, 2018 at 01:59:20PM +0000, Ferruh Yigit wrote:
> 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?
> 

Yes the patch is ok for me, thank you Ferruh.

> > 
> > 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 |
> >>>>>>
> >>
> > 
> 

-- 
Gaëtan Rivet
6WIND

  reply	other threads:[~2018-12-21 14:44 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-12 11:36 [dpdk-dev] [PATCH] " Andrew Rybchenko
2018-12-21 12:12 ` [dpdk-dev] " Ferruh Yigit
2018-12-21 12:28   ` Andrew Rybchenko
2018-12-21 12:43     ` Ferruh Yigit
2018-12-21 12:52       ` Andrew Rybchenko
2018-12-21 13:16         ` Gaëtan Rivet
2018-12-21 13:59           ` Ferruh Yigit
2018-12-21 14:44             ` Gaëtan Rivet [this message]
2018-12-21 18:10           ` Stephen Hemminger
2018-12-21 13:31         ` Ferruh Yigit
2018-12-21 15:09   ` Ferruh Yigit

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=20181221144421.szwkakqp7vbn5v5v@bidouze.vm.6wind.com \
    --to=gaetan.rivet@6wind.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=ivan.malov@oktetlabs.ru \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).