Re: [dpdk-dev] [PATCH] ethdev: add L2TPv3 header to flow API

["Sexton, Rory" <rory.sexton@intel.com>]

Hi Ori,

See comments below.

Regards,
Rory

> > > > > > +/**
> > > > > > + * RTE_FLOW_ITEM_TYPE_L2TPV3.
> > > > > > + *
> > > > > > + * Matches a L2TPv3 header.
> > > > > > + */
> > > > > > +struct rte_flow_item_l2tpv3 {
> > > > > > +	rte_be32_t session_id; /**< Session ID. */ };
> > > > >
> > > > >Does it make sense that in future we will want to match on the T / 
> > > > >L / ver /
> > > > Ns / Nr?
> > > > >
> > > > >Please also consider that any change will be ABI / API breakage, 
> > > > >which will
> > > > >be allowed only next year.
> > > > >
> > > >
> > > > I don't foresee us wanting to match on any of the other fields, T / 
> > > > L / ver / Ns/ Nr.
> > > > The session id would typically be the only field of interest in the
> > > > l2tpv3 header.
> > >
> > > I think that adding all fields to the structure will solve the 
> > > possible issue
> > with adding matching later.
> > > Also and even more important you will be able to use it for creating 
> > > the
> > raw_encap / raw_decap buffers.
> > >
> > >What do you think?
> > 
> > Based on the differences between v2 and v3 the only field of interest 
> > in
> > l2tpv3 over IP is the Session ID.
> > I agree it would make sense to add all fields if we were implementing 
> > l2tpv2 however v2 would require a different implementation to v3 due 
> > to the differences between both Protocols.
>
> Even if we just support v3, I think that it is a good idea to add all fields of v3. 
> This will allow the use of the raw_encap / raw_decap actions.
> Please also note that you didn't add the new item to  cmd_set_raw_parsed function.
> this function is used in order to create raw_encap/ raw_decap encapsulations.

I think the confusion here is based on the fact that there are 2 separate types of l2tpv3.
- l2tpv3 over UDP, which is very similar to l2tpv2 with only change being 16 bit Tunnel ID 
  and 16 bit Session ID changing to 32 bit Session ID. All other fields remain (T/L/Ver/Ns/Nr).
- l2tpv3 over IP is another type of l2tpv3 that is carried over IP rather than UDP and as such
  the message format is entirely different and consists of just a 32 bit Session ID. The other
  fields mentioned above do not exist at all in this l2tpv3 header.

This patch was targeted at creating a flow to handle l2tpv3 over IP exclusively. This is why
the Session ID is the only field in the flow item.

I can add the item to cmd_set_raw_parsed function.