> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@intel.com>
> Sent: Tuesday, October 5, 2021 5:43 PM
> To: Ori Kam <or...@nvidia.com>; Jie Wang <jie1x.w...@intel.com>;
> dev@dpdk.org; andrew.rybche...@oktetlabs.ru
> Cc: NBU-Contact-Thomas Monjalon <tho...@monjalon.net>;
> xiaoyun...@intel.com; jingjing...@intel.com; beilei.x...@intel.com;
> wenjun1...@intel.com; stevex.y...@intel.com
> Subject: Re: [dpdk-dev] [PATCH 3/4] ethdev: support PPPoL2TPv2oUDP RSS Hash
>
> On 9/30/2021 3:38 PM, Ori Kam wrote:
> > Hi Jie,
> >
> > You are missing updating the rte_flow.rst and release notes:
> >
> > Please see more comments below.
> >
> > Thanks,
> > Ori
> >
> >> -----Original Message-----
> >> From: dev <dev-boun...@dpdk.org> On Behalf Of Jie Wang
> >> Sent: Friday, September 24, 2021 6:17 PM
> >> Subject: [dpdk-dev] [PATCH 3/4] ethdev: support PPPoL2TPv2oUDP RSS
> >> Hash
> >>
> >> Add flow pattern items, RSS offload types and header formats of
> >> L2TPv2 and PPP.
> >>
> >> Signed-off-by: Wenjun Wu <wenjun1...@intel.com>
> >> Signed-off-by: Jie Wang <jie1x.w...@intel.com>
> >> ---
> >> lib/ethdev/rte_flow.c | 2 +
> >> lib/ethdev/rte_flow.h | 99
> >> +++++++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 101 insertions(+)
> >>
> >> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
> >> 8cb7a069c8..307fbc3abe 100644
> >> --- a/lib/ethdev/rte_flow.c
> >> +++ b/lib/ethdev/rte_flow.c
> >> @@ -98,6 +98,8 @@ static const struct rte_flow_desc_data
> >> rte_flow_desc_item[] = {
> >> MK_FLOW_ITEM(PFCP, sizeof(struct rte_flow_item_pfcp)),
> >> MK_FLOW_ITEM(ECPRI, sizeof(struct rte_flow_item_ecpri)),
> >> MK_FLOW_ITEM(GENEVE_OPT, sizeof(struct
> rte_flow_item_geneve_opt)),
> >> + MK_FLOW_ITEM(L2TPV2, sizeof(struct rte_flow_item_l2tpv2)),
> >> + MK_FLOW_ITEM(PPP, sizeof(struct rte_flow_item_ppp)),
> >> MK_FLOW_ITEM(INTEGRITY, sizeof(struct rte_flow_item_integrity)),
> >> MK_FLOW_ITEM(CONNTRACK, sizeof(uint32_t)), }; diff --git
> >> a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> >> 70f455d47d..93205b7d1e 100644
> >> --- a/lib/ethdev/rte_flow.h
> >> +++ b/lib/ethdev/rte_flow.h
> >> @@ -554,6 +554,20 @@ enum rte_flow_item_type {
> >> */
> >> RTE_FLOW_ITEM_TYPE_GENEVE_OPT,
> >>
> >> + /**
> >> + * Matches L2TPV2 Header.
> >> + *
> >> + * See struct rte_flow_item_l2tpv2.
> >> + */
> >> + RTE_FLOW_ITEM_TYPE_L2TPV2,
> >> +
> >> + /**
> >> + * Matches PPP Header.
> >> + *
> >> + * See struct rte_flow_item_ppp.
> >> + */
> >> + RTE_FLOW_ITEM_TYPE_PPP,
> >> +
> >
> > Why did you push the new items here and not in the end?
> >
> >> /**
> >> * [META]
> >> *
> >> @@ -1799,6 +1813,91 @@ static const struct rte_flow_item_conntrack
> >> rte_flow_item_conntrack_mask = { }; #endif
> >>
> >> +/**
> >> + * @warning
> >> + * @b EXPERIMENTAL: this structure may change without prior notice
> >> + * RTE_FLOW_ITEM_TYPE_L2TPV2
> >> + *
> >> + * Matches L2TPv2 Header
> >> + */
> >> +RTE_STD_C11
> >> +struct rte_flow_item_l2tpv2 {
> >> + rte_be16_t flags_version; /**< flag(12) version(2). version must be
> >> 2 */
> >> + union{
> >> + struct{
> >> + rte_be16_t length; /**< length(16) */
> >> + rte_be16_t tunnel_id; /**< tunnel id(16) */
> >> + rte_be16_t session_id; /**< session id(16) */
> >> + rte_be32_t ns_nr; /**< Ns(16) + Nr(16) */
> >
> > Why not split those fields?
> >
>
> Hi Ori,
>
> As far as I remember the decision was to define protocol headers separately
> and use them as first element in the flow_item struct, this enables casting
> the
> flow_item to protocol. And Andrew did some cleanups in the past related to
> this.
> Can't we use the same logic for this case?
This is also my thinking but I'm not familiar with this protocol and it looks
like it has many different
options.
Best,
Ori
