[2/2] doc: remove flow API from the feature list

Message ID 20191025125118.47189-2-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [1/2] doc: add PMD filtering features back |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Ferruh Yigit Oct. 25, 2019, 12:51 p.m. UTC
  "Flow API" is a method/API to implement various filtering features, on
its own it doesn't give much context on what features are provided. And
it is not really a feature, so doesn't fit into feature table.

Also since other filtering related APIs, 'filter_ctrl', has been
deprecated, flow API is the only supported way in the DPDK to implement
filtering options, if related filter options announced by PMDs, listing
"Flow API" as implemented is redundant information.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Thomas Monjalon <thomas@monjalon.net>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
Cc: Jerin Jacob <jerinj@marvell.com>
---
 doc/guides/nics/features/default.ini | 1 -
 1 file changed, 1 deletion(-)
  

Comments

Thomas Monjalon Oct. 25, 2019, 1:26 p.m. UTC | #1
25/10/2019 14:51, Ferruh Yigit:
> "Flow API" is a method/API to implement various filtering features, on
> its own it doesn't give much context on what features are provided. And
> it is not really a feature, so doesn't fit into feature table.
> 
> Also since other filtering related APIs, 'filter_ctrl', has been
> deprecated, flow API is the only supported way in the DPDK to implement
> filtering options, if related filter options announced by PMDs, listing
> "Flow API" as implemented is redundant information.

I fully agree with this explanation.
rte_flow is the only supported API for flow offloads.
That's why we must remove the legacy API.

> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> --- a/doc/guides/nics/features/default.ini
> +++ b/doc/guides/nics/features/default.ini
> -Flow API             =

Acked-by: Thomas Monjalon <thomas@monjalon.net>
  
Jerin Jacob Oct. 25, 2019, 1:39 p.m. UTC | #2
On Fri, Oct 25, 2019 at 6:56 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 25/10/2019 14:51, Ferruh Yigit:
> > "Flow API" is a method/API to implement various filtering features, on
> > its own it doesn't give much context on what features are provided. And
> > it is not really a feature, so doesn't fit into feature table.
> >
> > Also since other filtering related APIs, 'filter_ctrl', has been
> > deprecated, flow API is the only supported way in the DPDK to implement
> > filtering options, if related filter options announced by PMDs, listing
> > "Flow API" as implemented is redundant information.
>
> I fully agree with this explanation.
> rte_flow is the only supported API for flow offloads.
> That's why we must remove the legacy API.
>
> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > ---
> > --- a/doc/guides/nics/features/default.ini
> > +++ b/doc/guides/nics/features/default.ini
> > -Flow API             =
>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

# Need to remove "Flow API" from doc/guides/nics/features.rst
# Need to remove refference of "Flow API" from "doc/guides/nics/*" as well.

Not specific to this patch,
Probably we need to add a new matrix to enumerate PATTERN and ACTIONS
supported by each PMD as a rte_flow feature matrix.
That some else can take it up if everyone agrees the semantics.
  
Ferruh Yigit April 10, 2020, 8:44 a.m. UTC | #3
On 10/25/2019 2:39 PM, Jerin Jacob wrote:
> On Fri, Oct 25, 2019 at 6:56 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 25/10/2019 14:51, Ferruh Yigit:
>>> "Flow API" is a method/API to implement various filtering features, on
>>> its own it doesn't give much context on what features are provided. And
>>> it is not really a feature, so doesn't fit into feature table.
>>>
>>> Also since other filtering related APIs, 'filter_ctrl', has been
>>> deprecated, flow API is the only supported way in the DPDK to implement
>>> filtering options, if related filter options announced by PMDs, listing
>>> "Flow API" as implemented is redundant information.
>>
>> I fully agree with this explanation.
>> rte_flow is the only supported API for flow offloads.
>> That's why we must remove the legacy API.
>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>> ---
>>> --- a/doc/guides/nics/features/default.ini
>>> +++ b/doc/guides/nics/features/default.ini
>>> -Flow API             =
>>
>> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 
> # Need to remove "Flow API" from doc/guides/nics/features.rst

+1

> # Need to remove refference of "Flow API" from "doc/guides/nics/*" as well.

"Flow API" is the implementation of the filtering, it may exist in the nic
documentation, only it is not a feature on itself. I will scan the docs for usage.

> 
> Not specific to this patch,
> Probably we need to add a new matrix to enumerate PATTERN and ACTIONS
> supported by each PMD as a rte_flow feature matrix.
> That some else can take it up if everyone agrees the semantics.
> 

+1, there needs a way to figure out which filtering is supported by a
device/driver. It is not documented and it is very hard to got it from the code.

Not sure if a new matrix is the good way to go, but I agree we need some way to
clarify it.
  
Thomas Monjalon April 10, 2020, 8:56 a.m. UTC | #4
10/04/2020 10:44, Ferruh Yigit:
> On 10/25/2019 2:39 PM, Jerin Jacob wrote:
> > On Fri, Oct 25, 2019 at 6:56 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >>
> >> 25/10/2019 14:51, Ferruh Yigit:
> >>> "Flow API" is a method/API to implement various filtering features, on
> >>> its own it doesn't give much context on what features are provided. And
> >>> it is not really a feature, so doesn't fit into feature table.
> >>>
> >>> Also since other filtering related APIs, 'filter_ctrl', has been
> >>> deprecated, flow API is the only supported way in the DPDK to implement
> >>> filtering options, if related filter options announced by PMDs, listing
> >>> "Flow API" as implemented is redundant information.
> >>
> >> I fully agree with this explanation.
> >> rte_flow is the only supported API for flow offloads.
> >> That's why we must remove the legacy API.
> >>
> >>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>> ---
> >>> --- a/doc/guides/nics/features/default.ini
> >>> +++ b/doc/guides/nics/features/default.ini
> >>> -Flow API             =
> >>
> >> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> > 
> > # Need to remove "Flow API" from doc/guides/nics/features.rst
> 
> +1
> 
> > # Need to remove refference of "Flow API" from "doc/guides/nics/*" as well.
> 
> "Flow API" is the implementation of the filtering, it may exist in the nic
> documentation, only it is not a feature on itself. I will scan the docs for usage.
> 
> > 
> > Not specific to this patch,
> > Probably we need to add a new matrix to enumerate PATTERN and ACTIONS
> > supported by each PMD as a rte_flow feature matrix.
> > That some else can take it up if everyone agrees the semantics.
> > 
> 
> +1, there needs a way to figure out which filtering is supported by a
> device/driver. It is not documented and it is very hard to got it from the code.
> 
> Not sure if a new matrix is the good way to go, but I agree we need some way to
> clarify it.

I think we should split the matrix.
Adding a new matrix for flow offloads looks the way to go.
I suggest 3 matrices:
	- port-level features
	- queue-level features
	- flow-level features
  
Jerin Jacob April 10, 2020, 9:04 a.m. UTC | #5
On Fri, Apr 10, 2020 at 2:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 10/04/2020 10:44, Ferruh Yigit:
> > On 10/25/2019 2:39 PM, Jerin Jacob wrote:
> > > On Fri, Oct 25, 2019 at 6:56 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >>
> > >> 25/10/2019 14:51, Ferruh Yigit:
> > >>> "Flow API" is a method/API to implement various filtering features, on
> > >>> its own it doesn't give much context on what features are provided. And
> > >>> it is not really a feature, so doesn't fit into feature table.
> > >>>
> > >>> Also since other filtering related APIs, 'filter_ctrl', has been
> > >>> deprecated, flow API is the only supported way in the DPDK to implement
> > >>> filtering options, if related filter options announced by PMDs, listing
> > >>> "Flow API" as implemented is redundant information.
> > >>
> > >> I fully agree with this explanation.
> > >> rte_flow is the only supported API for flow offloads.
> > >> That's why we must remove the legacy API.
> > >>
> > >>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > >>> ---
> > >>> --- a/doc/guides/nics/features/default.ini
> > >>> +++ b/doc/guides/nics/features/default.ini
> > >>> -Flow API             =
> > >>
> > >> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> > >
> > > # Need to remove "Flow API" from doc/guides/nics/features.rst
> >
> > +1
> >
> > > # Need to remove refference of "Flow API" from "doc/guides/nics/*" as well.
> >
> > "Flow API" is the implementation of the filtering, it may exist in the nic
> > documentation, only it is not a feature on itself. I will scan the docs for usage.
> >
> > >
> > > Not specific to this patch,
> > > Probably we need to add a new matrix to enumerate PATTERN and ACTIONS
> > > supported by each PMD as a rte_flow feature matrix.
> > > That some else can take it up if everyone agrees the semantics.
> > >
> >
> > +1, there needs a way to figure out which filtering is supported by a
> > device/driver. It is not documented and it is very hard to got it from the code.
> >
> > Not sure if a new matrix is the good way to go, but I agree we need some way to
> > clarify it.
>
> I think we should split the matrix.
> Adding a new matrix for flow offloads looks the way to go.
> I suggest 3 matrices:
>         - port-level features
>         - queue-level features
>         - flow-level features

Not sure what will be the details in "flow-level features".

IMO, We need to have a separate matrix for subdomain features for
rte_flow, rte_tm, rte_mtr, etc which part of ethdev.
For instance, rte_flow features can be translated into a matrix of
supported PATTERN and ACTIONS.


>
>
  
Thomas Monjalon April 10, 2020, 9:20 a.m. UTC | #6
10/04/2020 11:04, Jerin Jacob:
> On Fri, Apr 10, 2020 at 2:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > 10/04/2020 10:44, Ferruh Yigit:
> > > On 10/25/2019 2:39 PM, Jerin Jacob wrote:
> > > > On Fri, Oct 25, 2019 at 6:56 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > >>
> > > >> 25/10/2019 14:51, Ferruh Yigit:
> > > >>> "Flow API" is a method/API to implement various filtering features, on
> > > >>> its own it doesn't give much context on what features are provided. And
> > > >>> it is not really a feature, so doesn't fit into feature table.
> > > >>>
> > > >>> Also since other filtering related APIs, 'filter_ctrl', has been
> > > >>> deprecated, flow API is the only supported way in the DPDK to implement
> > > >>> filtering options, if related filter options announced by PMDs, listing
> > > >>> "Flow API" as implemented is redundant information.
> > > >>
> > > >> I fully agree with this explanation.
> > > >> rte_flow is the only supported API for flow offloads.
> > > >> That's why we must remove the legacy API.
> > > >>
> > > >>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > > >>> ---
> > > >>> --- a/doc/guides/nics/features/default.ini
> > > >>> +++ b/doc/guides/nics/features/default.ini
> > > >>> -Flow API             =
> > > >>
> > > >> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> > > >
> > > > # Need to remove "Flow API" from doc/guides/nics/features.rst
> > >
> > > +1
> > >
> > > > # Need to remove refference of "Flow API" from "doc/guides/nics/*" as well.
> > >
> > > "Flow API" is the implementation of the filtering, it may exist in the nic
> > > documentation, only it is not a feature on itself. I will scan the docs for usage.
> > >
> > > >
> > > > Not specific to this patch,
> > > > Probably we need to add a new matrix to enumerate PATTERN and ACTIONS
> > > > supported by each PMD as a rte_flow feature matrix.
> > > > That some else can take it up if everyone agrees the semantics.
> > > >
> > >
> > > +1, there needs a way to figure out which filtering is supported by a
> > > device/driver. It is not documented and it is very hard to got it from the code.
> > >
> > > Not sure if a new matrix is the good way to go, but I agree we need some way to
> > > clarify it.
> >
> > I think we should split the matrix.
> > Adding a new matrix for flow offloads looks the way to go.
> > I suggest 3 matrices:
> >         - port-level features
> >         - queue-level features
> >         - flow-level features
> 
> Not sure what will be the details in "flow-level features".
> 
> IMO, We need to have a separate matrix for subdomain features for
> rte_flow, rte_tm, rte_mtr, etc which part of ethdev.
> For instance, rte_flow features can be translated into a matrix of
> supported PATTERN and ACTIONS.

Yes I'm also fine with this proposal.
  
Ferruh Yigit April 10, 2020, 9:27 a.m. UTC | #7
On 4/10/2020 10:20 AM, Thomas Monjalon wrote:
> 10/04/2020 11:04, Jerin Jacob:
>> On Fri, Apr 10, 2020 at 2:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 10/04/2020 10:44, Ferruh Yigit:
>>>> On 10/25/2019 2:39 PM, Jerin Jacob wrote:
>>>>> On Fri, Oct 25, 2019 at 6:56 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>>>
>>>>>> 25/10/2019 14:51, Ferruh Yigit:
>>>>>>> "Flow API" is a method/API to implement various filtering features, on
>>>>>>> its own it doesn't give much context on what features are provided. And
>>>>>>> it is not really a feature, so doesn't fit into feature table.
>>>>>>>
>>>>>>> Also since other filtering related APIs, 'filter_ctrl', has been
>>>>>>> deprecated, flow API is the only supported way in the DPDK to implement
>>>>>>> filtering options, if related filter options announced by PMDs, listing
>>>>>>> "Flow API" as implemented is redundant information.
>>>>>>
>>>>>> I fully agree with this explanation.
>>>>>> rte_flow is the only supported API for flow offloads.
>>>>>> That's why we must remove the legacy API.
>>>>>>
>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>> ---
>>>>>>> --- a/doc/guides/nics/features/default.ini
>>>>>>> +++ b/doc/guides/nics/features/default.ini
>>>>>>> -Flow API             =
>>>>>>
>>>>>> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>>>>>
>>>>> # Need to remove "Flow API" from doc/guides/nics/features.rst
>>>>
>>>> +1
>>>>
>>>>> # Need to remove refference of "Flow API" from "doc/guides/nics/*" as well.
>>>>
>>>> "Flow API" is the implementation of the filtering, it may exist in the nic
>>>> documentation, only it is not a feature on itself. I will scan the docs for usage.
>>>>
>>>>>
>>>>> Not specific to this patch,
>>>>> Probably we need to add a new matrix to enumerate PATTERN and ACTIONS
>>>>> supported by each PMD as a rte_flow feature matrix.
>>>>> That some else can take it up if everyone agrees the semantics.
>>>>>
>>>>
>>>> +1, there needs a way to figure out which filtering is supported by a
>>>> device/driver. It is not documented and it is very hard to got it from the code.
>>>>
>>>> Not sure if a new matrix is the good way to go, but I agree we need some way to
>>>> clarify it.
>>>
>>> I think we should split the matrix.
>>> Adding a new matrix for flow offloads looks the way to go.
>>> I suggest 3 matrices:
>>>         - port-level features
>>>         - queue-level features
>>>         - flow-level features
>>
>> Not sure what will be the details in "flow-level features".
>>
>> IMO, We need to have a separate matrix for subdomain features for
>> rte_flow, rte_tm, rte_mtr, etc which part of ethdev.
>> For instance, rte_flow features can be translated into a matrix of
>> supported PATTERN and ACTIONS.
> 
> Yes I'm also fine with this proposal.
> 

My concern is it will be too big and detailed, also hard to maintain which means
it will be out dated a while later.

After above said, I don't have a better solution right now ...
  

Patch

diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index ca3b738ea..192a5168e 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -45,7 +45,6 @@  Flexible filter      =
 Hash filter          =
 Flow director        =
 Flow control         =
-Flow API             =
 Rate limitation      =
 Traffic mirroring    =
 Inline crypto        =