mbox series

[v2,0/8] start cleanup of rte_flow_item_*

Message ID 20230120171902.4188088-1-ferruh.yigit@amd.com (mailing list archive)
Headers
Series start cleanup of rte_flow_item_* |

Message

Ferruh Yigit Jan. 20, 2023, 5:18 p.m. UTC
  There was a plan to have structures from lib/net/ at the beginning
of corresponding flow item structures.
Unfortunately this plan has not been followed up so far.
This series is a step to make the most used items,
compliant with the inheritance design explained above.
The old API is kept in anonymous union for compatibility,
but the code in drivers and apps is updated to use the new API.


v2: (by Ferruh)
 * Rebased on latest next-net for v23.03
 * 'struct rte_gre_hdr' endianness annotation added to protocol field
 * more driver code updated for rte_flow_item_eth & rte_flow_item_vlan
 * 'struct rte_gre_hdr' updated to have a combined "rte_be16_t c_rsvd0_ver"
   field and updated drivers accordingly
 * more driver code updated for rte_flow_item_gre
 * more driver code updated for rte_flow_item_gtp


Cc: David Marchand <david.marchand@redhat.com>

Thomas Monjalon (8):
  ethdev: use Ethernet protocol struct for flow matching
  net: add smaller fields for VXLAN
  ethdev: use VXLAN protocol struct for flow matching
  ethdev: use GRE protocol struct for flow matching
  ethdev: use GTP protocol struct for flow matching
  ethdev: use ARP protocol struct for flow matching
  doc: fix description of L2TPV2 flow item
  net: mark all big endian types

For series,
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

 app/test-flow-perf/actions_gen.c         |   2 +-
 app/test-flow-perf/items_gen.c           |  24 +--
 app/test-pmd/cmdline_flow.c              | 180 +++++++++++------------
 doc/guides/prog_guide/rte_flow.rst       |  57 ++-----
 doc/guides/rel_notes/deprecation.rst     |   6 +-
 drivers/net/bnxt/bnxt_flow.c             |  54 +++----
 drivers/net/bnxt/tf_ulp/ulp_rte_parser.c | 112 +++++++-------
 drivers/net/bonding/rte_eth_bond_pmd.c   |  12 +-
 drivers/net/cxgbe/cxgbe_flow.c           |  44 +++---
 drivers/net/dpaa2/dpaa2_flow.c           |  60 ++++----
 drivers/net/dpaa2/dpaa2_mux.c            |   2 +-
 drivers/net/e1000/igb_flow.c             |  14 +-
 drivers/net/enic/enic_flow.c             |  24 +--
 drivers/net/enic/enic_fm_flow.c          |  16 +-
 drivers/net/hinic/hinic_pmd_flow.c       |  14 +-
 drivers/net/hns3/hns3_flow.c             |  40 ++---
 drivers/net/i40e/i40e_fdir.c             |  14 +-
 drivers/net/i40e/i40e_flow.c             | 124 ++++++++--------
 drivers/net/i40e/i40e_hash.c             |   4 +-
 drivers/net/iavf/iavf_fdir.c             |  18 +--
 drivers/net/iavf/iavf_fsub.c             |  10 +-
 drivers/net/iavf/iavf_ipsec_crypto.c     |   4 +-
 drivers/net/ice/ice_acl_filter.c         |  20 +--
 drivers/net/ice/ice_fdir_filter.c        |  24 +--
 drivers/net/ice/ice_switch_filter.c      |  64 ++++----
 drivers/net/igc/igc_flow.c               |   8 +-
 drivers/net/ipn3ke/ipn3ke_flow.c         |  12 +-
 drivers/net/ixgbe/ixgbe_flow.c           |  58 ++++----
 drivers/net/mlx4/mlx4_flow.c             |  38 ++---
 drivers/net/mlx5/hws/mlx5dr_definer.c    |  48 +++---
 drivers/net/mlx5/mlx5_flow.c             |  62 ++++----
 drivers/net/mlx5/mlx5_flow_dv.c          | 178 +++++++++++-----------
 drivers/net/mlx5/mlx5_flow_hw.c          |  80 +++++-----
 drivers/net/mlx5/mlx5_flow_verbs.c       |  46 +++---
 drivers/net/mlx5/mlx5_trigger.c          |  28 ++--
 drivers/net/mvpp2/mrvl_flow.c            |  28 ++--
 drivers/net/nfp/nfp_flow.c               |  21 +--
 drivers/net/sfc/sfc_flow.c               |  52 +++----
 drivers/net/sfc/sfc_mae.c                |  46 +++---
 drivers/net/tap/tap_flow.c               |  58 ++++----
 drivers/net/txgbe/txgbe_flow.c           |  28 ++--
 lib/ethdev/rte_flow.h                    | 117 ++++++++++-----
 lib/net/rte_arp.h                        |  28 ++--
 lib/net/rte_gre.h                        |   7 +-
 lib/net/rte_higig.h                      |   6 +-
 lib/net/rte_mpls.h                       |   2 +-
 lib/net/rte_vxlan.h                      |  35 ++++-
 47 files changed, 982 insertions(+), 947 deletions(-)

--
2.25.1
  

Comments

David Marchand Jan. 22, 2023, 10:52 a.m. UTC | #1
Hi Ferruh, Thomas,

On Fri, Jan 20, 2023 at 6:19 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> There was a plan to have structures from lib/net/ at the beginning
> of corresponding flow item structures.
> Unfortunately this plan has not been followed up so far.
> This series is a step to make the most used items,
> compliant with the inheritance design explained above.
> The old API is kept in anonymous union for compatibility,
> but the code in drivers and apps is updated to use the new API.
>
>
> v2: (by Ferruh)
>  * Rebased on latest next-net for v23.03
>  * 'struct rte_gre_hdr' endianness annotation added to protocol field
>  * more driver code updated for rte_flow_item_eth & rte_flow_item_vlan
>  * 'struct rte_gre_hdr' updated to have a combined "rte_be16_t c_rsvd0_ver"
>    field and updated drivers accordingly
>  * more driver code updated for rte_flow_item_gre
>  * more driver code updated for rte_flow_item_gtp
>
>
> Cc: David Marchand <david.marchand@redhat.com>

Note: it is relatively easy to run OVS checks, you only need a github
fork of ovs with a dpdk-latest branch + some github yml update to
point at a dpdk repo + branch of yours (see the last commit in my repo
below).

I ran this series in my dpdk-latest (rebased) OVS branch
https://github.com/david-marchand/ovs/commits/dpdk-latest, through
GHA.

Sparse spotted an issue on rte_flow.h header, following HIGIG2 update.
https://github.com/david-marchand/ovs/actions/runs/3979243283/jobs/6821543439#step:12:2592

2023-01-22T10:31:37.5911785Z ../../lib/ofp-packet.c: note: in included
file (through ../../lib/netdev-dpdk.h, ../../lib/dp-packet.h):
2023-01-22T10:31:37.5918848Z
/home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:645:43:
error: incorrect type in initializer (different base types)
2023-01-22T10:31:37.5919574Z
/home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:645:43:
expected restricted ovs_be16 [usertype] classification
2023-01-22T10:31:37.5920131Z
/home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:645:43:
got int
2023-01-22T10:31:37.5920720Z
/home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:646:32:
error: incorrect type in initializer (different base types)
2023-01-22T10:31:37.5921341Z
/home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:646:32:
expected restricted ovs_be16 [usertype] vid
2023-01-22T10:31:37.5921866Z
/home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:646:32:
got int
2023-01-22T10:31:37.6042168Z make[2]: *** [Makefile:4681:
lib/ofp-packet.lo] Error 1
2023-01-22T10:31:37.6042717Z make[2]: *** Waiting for unfinished jobs....

This should be fixed with:

diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index a215daa836..99f8340f82 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -642,8 +642,8 @@ struct rte_flow_item_higig2_hdr {
 static const struct rte_flow_item_higig2_hdr rte_flow_item_higig2_hdr_mask = {
        .hdr = {
                .ppt1 = {
-                       .classification = 0xffff,
-                       .vid = 0xfff,
+                       .classification = RTE_BE16(0xffff),
+                       .vid = RTE_BE16(0xfff),
                },
        },
 };

However, looking at existing code, and though I don't know HIGIG2, it
is a bit strange to use a 12 bits large mask for vid.


With this fix, OVS sparse check passes:
https://github.com/david-marchand/ovs/actions/runs/3979288868
  
Ferruh Yigit Jan. 24, 2023, 9:07 a.m. UTC | #2
On 1/22/2023 10:52 AM, David Marchand wrote:
> Hi Ferruh, Thomas,
> 
> On Fri, Jan 20, 2023 at 6:19 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>>
>> There was a plan to have structures from lib/net/ at the beginning
>> of corresponding flow item structures.
>> Unfortunately this plan has not been followed up so far.
>> This series is a step to make the most used items,
>> compliant with the inheritance design explained above.
>> The old API is kept in anonymous union for compatibility,
>> but the code in drivers and apps is updated to use the new API.
>>
>>
>> v2: (by Ferruh)
>>  * Rebased on latest next-net for v23.03
>>  * 'struct rte_gre_hdr' endianness annotation added to protocol field
>>  * more driver code updated for rte_flow_item_eth & rte_flow_item_vlan
>>  * 'struct rte_gre_hdr' updated to have a combined "rte_be16_t c_rsvd0_ver"
>>    field and updated drivers accordingly
>>  * more driver code updated for rte_flow_item_gre
>>  * more driver code updated for rte_flow_item_gtp
>>
>>
>> Cc: David Marchand <david.marchand@redhat.com>
> 
> Note: it is relatively easy to run OVS checks, you only need a github
> fork of ovs with a dpdk-latest branch + some github yml update to
> point at a dpdk repo + branch of yours (see the last commit in my repo
> below).
> 
> I ran this series in my dpdk-latest (rebased) OVS branch
> https://github.com/david-marchand/ovs/commits/dpdk-latest, through
> GHA.
> 
> Sparse spotted an issue on rte_flow.h header, following HIGIG2 update.
> https://github.com/david-marchand/ovs/actions/runs/3979243283/jobs/6821543439#step:12:2592
> 
> 2023-01-22T10:31:37.5911785Z ../../lib/ofp-packet.c: note: in included
> file (through ../../lib/netdev-dpdk.h, ../../lib/dp-packet.h):
> 2023-01-22T10:31:37.5918848Z
> /home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:645:43:
> error: incorrect type in initializer (different base types)
> 2023-01-22T10:31:37.5919574Z
> /home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:645:43:
> expected restricted ovs_be16 [usertype] classification
> 2023-01-22T10:31:37.5920131Z
> /home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:645:43:
> got int
> 2023-01-22T10:31:37.5920720Z
> /home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:646:32:
> error: incorrect type in initializer (different base types)
> 2023-01-22T10:31:37.5921341Z
> /home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:646:32:
> expected restricted ovs_be16 [usertype] vid
> 2023-01-22T10:31:37.5921866Z
> /home/runner/work/ovs/ovs/dpdk-dir/build/include/rte_flow.h:646:32:
> got int
> 2023-01-22T10:31:37.6042168Z make[2]: *** [Makefile:4681:
> lib/ofp-packet.lo] Error 1
> 2023-01-22T10:31:37.6042717Z make[2]: *** Waiting for unfinished jobs....
> 
> This should be fixed with:
> 
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index a215daa836..99f8340f82 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -642,8 +642,8 @@ struct rte_flow_item_higig2_hdr {
>  static const struct rte_flow_item_higig2_hdr rte_flow_item_higig2_hdr_mask = {
>         .hdr = {
>                 .ppt1 = {
> -                       .classification = 0xffff,
> -                       .vid = 0xfff,
> +                       .classification = RTE_BE16(0xffff),
> +                       .vid = RTE_BE16(0xfff),
>                 },
>         },
>  };
> 
> However, looking at existing code, and though I don't know HIGIG2, it
> is a bit strange to use a 12 bits large mask for vid.
> 
> 
> With this fix, OVS sparse check passes:
> https://github.com/david-marchand/ovs/actions/runs/3979288868
> 
> 

Thanks David, fixed this as you suggested in v3:
https://patches.dpdk.org/project/dpdk/list/?series=26632

@Thomas, @Andrew, can we get this set for this release, what do you think?