[00/11] Add basic flow support for corenic firmware

Message ID 20231103062606.2632012-1-chaoyong.he@corigine.com (mailing list archive)
Headers
Series Add basic flow support for corenic firmware |

Message

Chaoyong He Nov. 3, 2023, 6:25 a.m. UTC
  Add the very basic rte_flow support for corenic firmware.

Chaoyong He (11):
  net/nfp: move some source files
  drivers: add the structures and functions for flow offload
  net/nfp: add the control message channel
  net/nfp: support flow API for CoreNIC firmware
  net/nfp: support Ethernet flow item
  net/nfp: support drop flow action
  net/nfp: support IPv4 flow item
  net/nfp: support IPv6 flow item
  net/nfp: support TCP/UDP/SCTP flow items
  drivers: support MARK flow action
  net/nfp: support QUEUE flow action

 drivers/common/nfp/nfp_common_ctrl.h          |    2 +
 drivers/net/nfp/flower/nfp_conntrack.h        |    2 +-
 drivers/net/nfp/flower/nfp_flower_cmsg.h      |    2 +-
 .../{nfp_flow.c => flower/nfp_flower_flow.c}  |    4 +-
 .../{nfp_flow.h => flower/nfp_flower_flow.h}  |   10 +-
 .../net/nfp/flower/nfp_flower_representor.c   |    2 +-
 drivers/net/nfp/meson.build                   |    4 +-
 drivers/net/nfp/nfp_ethdev.c                  |   27 +-
 drivers/net/nfp/nfp_net_cmsg.c                |   66 ++
 drivers/net/nfp/nfp_net_cmsg.h                |  176 +++
 drivers/net/nfp/nfp_net_common.h              |   12 +
 drivers/net/nfp/nfp_net_ctrl.h                |    1 +
 drivers/net/nfp/nfp_net_flow.c                | 1003 +++++++++++++++++
 drivers/net/nfp/nfp_net_flow.h                |   30 +
 drivers/net/nfp/nfp_rxtx.c                    |   18 +
 15 files changed, 1341 insertions(+), 18 deletions(-)
 rename drivers/net/nfp/{nfp_flow.c => flower/nfp_flower_flow.c} (99%)
 rename drivers/net/nfp/{nfp_flow.h => flower/nfp_flower_flow.h} (96%)
 create mode 100644 drivers/net/nfp/nfp_net_cmsg.c
 create mode 100644 drivers/net/nfp/nfp_net_cmsg.h
 create mode 100644 drivers/net/nfp/nfp_net_flow.c
 create mode 100644 drivers/net/nfp/nfp_net_flow.h
  

Comments

Ferruh Yigit Nov. 3, 2023, 4:12 p.m. UTC | #1
On 11/3/2023 6:25 AM, Chaoyong He wrote:
> Add the very basic rte_flow support for corenic firmware.
> 
> Chaoyong He (11):
>   net/nfp: move some source files
>   drivers: add the structures and functions for flow offload
>   net/nfp: add the control message channel
>   net/nfp: support flow API for CoreNIC firmware
>   net/nfp: support Ethernet flow item
>   net/nfp: support drop flow action
>   net/nfp: support IPv4 flow item
>   net/nfp: support IPv6 flow item
>   net/nfp: support TCP/UDP/SCTP flow items
>   drivers: support MARK flow action
>   net/nfp: support QUEUE flow action
> 

Recheck-request: iol-compile-amd64-testing, iol-unit-amd64-testing
  
Ferruh Yigit Nov. 3, 2023, 5:01 p.m. UTC | #2
On 11/3/2023 6:25 AM, Chaoyong He wrote:
> Add the very basic rte_flow support for corenic firmware.
> 
> Chaoyong He (11):
>   net/nfp: move some source files
>   drivers: add the structures and functions for flow offload
>   net/nfp: add the control message channel
>   net/nfp: support flow API for CoreNIC firmware
>   net/nfp: support Ethernet flow item
>   net/nfp: support drop flow action
>   net/nfp: support IPv4 flow item
>   net/nfp: support IPv6 flow item
>   net/nfp: support TCP/UDP/SCTP flow items
>   drivers: support MARK flow action
>   net/nfp: support QUEUE flow action
>

`./devtools/check-doc-vs-code.sh` gives some warnings [1], can you
please check them?

Please update the .ini file in the relevant patch where feature is
added, don't update it in a separate patch.


[1]
$ ./devtools/check-doc-vs-code.sh
rte_flow doc out of sync for nfp
        action mark
        action queue
  
Chaoyong He Nov. 7, 2023, 1:42 a.m. UTC | #3
> On 11/3/2023 6:25 AM, Chaoyong He wrote:
> > Add the very basic rte_flow support for corenic firmware.
> >
> > Chaoyong He (11):
> >   net/nfp: move some source files
> >   drivers: add the structures and functions for flow offload
> >   net/nfp: add the control message channel
> >   net/nfp: support flow API for CoreNIC firmware
> >   net/nfp: support Ethernet flow item
> >   net/nfp: support drop flow action
> >   net/nfp: support IPv4 flow item
> >   net/nfp: support IPv6 flow item
> >   net/nfp: support TCP/UDP/SCTP flow items
> >   drivers: support MARK flow action
> >   net/nfp: support QUEUE flow action
> >
> 
> Recheck-request: iol-compile-amd64-testing, iol-unit-amd64-testing

Seems both compile problems was introduced by this commit:
https://github.com/Corigine/dpdk-next-net-private/commit/34ff088cc24159c9fa6e61242efb76d0289b4e37

```
ethdev: set and query RSS hash algorithm
Currently, rte_eth_rss_conf supports configuring and querying
RSS hash functions, rss key and it's length, but not RSS hash
algorithm.

The structure ``rte_eth_dev_info`` is extended by adding a new
field "rss_algo_capa". Drivers are responsible for reporting this
capa and configurations of RSS hash algorithm can be verified based
on the capability. The default value of "rss_algo_capa" is
RTE_ETH_HASH_ALGO_CAPA_MASK(DEFAULT) if drivers do not report it.

The structure ``rte_eth_rss_conf`` is extended by adding a new
field "algorithm". This represents the RSS algorithms to apply.
If the value of "algorithm" used for configuration is a gibberish
value, drivers should report the error.

To check whether the drivers report valid "algorithm", it is set
to default value before querying in rte_eth_dev_rss_hash_conf_get().

Signed-off-by: Jie Hai <haijie1@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>
```

Which was not related with this patch series, I think?
  
Ferruh Yigit Nov. 7, 2023, 9:18 a.m. UTC | #4
On 11/7/2023 1:42 AM, Chaoyong He wrote:
>> On 11/3/2023 6:25 AM, Chaoyong He wrote:
>>> Add the very basic rte_flow support for corenic firmware.
>>>
>>> Chaoyong He (11):
>>>   net/nfp: move some source files
>>>   drivers: add the structures and functions for flow offload
>>>   net/nfp: add the control message channel
>>>   net/nfp: support flow API for CoreNIC firmware
>>>   net/nfp: support Ethernet flow item
>>>   net/nfp: support drop flow action
>>>   net/nfp: support IPv4 flow item
>>>   net/nfp: support IPv6 flow item
>>>   net/nfp: support TCP/UDP/SCTP flow items
>>>   drivers: support MARK flow action
>>>   net/nfp: support QUEUE flow action
>>>
>>
>> Recheck-request: iol-compile-amd64-testing, iol-unit-amd64-testing
> 
> Seems both compile problems was introduced by this commit:
> https://github.com/Corigine/dpdk-next-net-private/commit/34ff088cc24159c9fa6e61242efb76d0289b4e37
> 
> ```
> ethdev: set and query RSS hash algorithm
> Currently, rte_eth_rss_conf supports configuring and querying
> RSS hash functions, rss key and it's length, but not RSS hash
> algorithm.
> 
> The structure ``rte_eth_dev_info`` is extended by adding a new
> field "rss_algo_capa". Drivers are responsible for reporting this
> capa and configurations of RSS hash algorithm can be verified based
> on the capability. The default value of "rss_algo_capa" is
> RTE_ETH_HASH_ALGO_CAPA_MASK(DEFAULT) if drivers do not report it.
> 
> The structure ``rte_eth_rss_conf`` is extended by adding a new
> field "algorithm". This represents the RSS algorithms to apply.
> If the value of "algorithm" used for configuration is a gibberish
> value, drivers should report the error.
> 
> To check whether the drivers report valid "algorithm", it is set
> to default value before querying in rte_eth_dev_rss_hash_conf_get().
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Acked-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>
> ```
> 
> Which was not related with this patch series, I think?
>

Yes, build error is not related with this patch, and it is fixed in
next-net, that is why I tried to re-trigger the test, but it didn't work
somehow [1].

Btw, this set is pending because of other change request, not because of
the reported build failure.


[1]
https://patchwork.dpdk.org/project/dpdk/patch/20231103062606.2632012-12-chaoyong.he@corigine.com/
  
Chaoyong He Nov. 7, 2023, 9:23 a.m. UTC | #5
> On 11/7/2023 1:42 AM, Chaoyong He wrote:
> >> On 11/3/2023 6:25 AM, Chaoyong He wrote:
> >>> Add the very basic rte_flow support for corenic firmware.
> >>>
> >>> Chaoyong He (11):
> >>>   net/nfp: move some source files
> >>>   drivers: add the structures and functions for flow offload
> >>>   net/nfp: add the control message channel
> >>>   net/nfp: support flow API for CoreNIC firmware
> >>>   net/nfp: support Ethernet flow item
> >>>   net/nfp: support drop flow action
> >>>   net/nfp: support IPv4 flow item
> >>>   net/nfp: support IPv6 flow item
> >>>   net/nfp: support TCP/UDP/SCTP flow items
> >>>   drivers: support MARK flow action
> >>>   net/nfp: support QUEUE flow action
> >>>
> >>
> >> Recheck-request: iol-compile-amd64-testing, iol-unit-amd64-testing
> >
> > Seems both compile problems was introduced by this commit:
> > https://github.com/Corigine/dpdk-next-net-
> private/commit/34ff088cc2415
> > 9c9fa6e61242efb76d0289b4e37
> >
> > ```
> > ethdev: set and query RSS hash algorithm Currently, rte_eth_rss_conf
> > supports configuring and querying RSS hash functions, rss key and it's
> > length, but not RSS hash algorithm.
> >
> > The structure ``rte_eth_dev_info`` is extended by adding a new field
> > "rss_algo_capa". Drivers are responsible for reporting this capa and
> > configurations of RSS hash algorithm can be verified based on the
> > capability. The default value of "rss_algo_capa" is
> > RTE_ETH_HASH_ALGO_CAPA_MASK(DEFAULT) if drivers do not report it.
> >
> > The structure ``rte_eth_rss_conf`` is extended by adding a new field
> > "algorithm". This represents the RSS algorithms to apply.
> > If the value of "algorithm" used for configuration is a gibberish
> > value, drivers should report the error.
> >
> > To check whether the drivers report valid "algorithm", it is set to
> > default value before querying in rte_eth_dev_rss_hash_conf_get().
> >
> > Signed-off-by: Jie Hai <haijie1@huawei.com>
> > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> > Acked-by: Huisong Li <lihuisong@huawei.com>
> > Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> > Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com> ```
> >
> > Which was not related with this patch series, I think?
> >
> 
> Yes, build error is not related with this patch, and it is fixed in next-net, that is
> why I tried to re-trigger the test, but it didn't work somehow [1].
> 
> Btw, this set is pending because of other change request, not because of the
> reported build failure.

Oh, yes, I understand, the reason is I need to also update the 'nfp.ini' document.
But we decide delay this patch set to the next version of DPDK as we found still need some
modification to the logic of flower firmware.
Thanks.
> 
> 
> [1]
> https://patchwork.dpdk.org/project/dpdk/patch/20231103062606.263201
> 2-12-chaoyong.he@corigine.com/
>
  
Patrick Robb Nov. 7, 2023, 4:50 p.m. UTC | #6
Recheck-request: iol-compile-amd64-testing, iol-unit-amd64-testing

Weird. I will dig up the original job which polled for the previous retest
request, and see whether this new one runs through properly.
  
Patrick Robb Nov. 7, 2023, 5:04 p.m. UTC | #7
Sorry, now I understand what is causing confusion. When a retest is
requested, that retest is done on the original dpdk artifact we created for
the patchseries. We don't pull again, re-apply, and then rerun the testing.
I think that is the behavior you were expecting, Ferruh.

We have discussed whether this is an important option to support in future
iterations of the retest-request feature. One idea is to add another option
in the retest request schema which allows for the requester to indicate
they want the patch re-applied onto the newer branch, and testing rerun
with that new dpdk. It's good to see that this is a use case which would
see application. Will be aiming to add this support in the future.
  
Ferruh Yigit Nov. 7, 2023, 8:19 p.m. UTC | #8
On 11/7/2023 5:04 PM, Patrick Robb wrote:
> Sorry, now I understand what is causing confusion. When a retest is
> requested, that retest is done on the original dpdk artifact we created
> for the patchseries. We don't pull again, re-apply, and then rerun the
> testing. I think that is the behavior you were expecting, Ferruh. 
> 

Hi Patrick,

Thanks for clarification, yes that was my expectation.

> We have discussed whether this is an important option to support in
> future iterations of the retest-request feature. One idea is to add
> another option in the retest request schema which allows for the
> requester to indicate they want the patch re-applied onto the newer
> branch, and testing rerun with that new dpdk. It's good to see that this
> is a use case which would see application. Will be aiming to add this
> support in the future.  
>

I am also not sure if this is the common usecase, lets record the need,
we can decide what to support based on it.


And as a generic approach, what about following syntax:
Recheck-request,attribute=value: ...

And for above case, attribute can be "pull=true" ...
  
Patrick Robb Nov. 7, 2023, 9:59 p.m. UTC | #9
On Tue, Nov 7, 2023 at 3:19 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:

>
> I am also not sure if this is the common usecase, lets record the need,
> we can decide what to support based on it.
>
>
> And as a generic approach, what about following syntax:
> Recheck-request,attribute=value: ...
>
> And for above case, attribute can be "pull=true" ...
>
> That syntax looks reasonable, and if it's omitted, default is pull=false.
Thanks!