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

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

Hi Ori,

See my comments below.

Regards,
Rory

>> 
>>> One general question why do we want to support only v3 and not also v2?
>> l2tpv3 is more widely used as a tunneling protocol hence it's inclusion.
>> A specific example is the cable industry where DOCSIS cable traffic is 
>> encapsulated using depi and uepi protocols which both make use of 
>> l2tpv3 session ids.
>> Directing flows based on l2tpv3 can be very useful in these cases.
>> 
>
>I'm not saying that v3 is not used more, I just thought from looking at the spec that both can be supported and the only difference is the version, so matching on the version we will be able to support both versions.
>Your decision.
>

There is more difference between l2tpv2 and l2tpv3 than just the version number.
In L2TPv2 there exists a 16 bit Tunnel ID and 16 bit Session ID. This is changed to a 32 bit Session ID in L2TPv3
Please see difference in headers below for v2 and v3.
Due to the differences between v2 and v3 I don't think both can be supported with same flow item.

                                                          L2TPv2
    0...............................................15, 16......................................31
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |T|L|x|x|S|x|O|P|x|x|x|x|  Ver  |          Length (opt)                  |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |           Tunnel ID                               |           Session ID                     |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |             Ns (opt)                               |             Nr (opt)                        |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |      Offset Size (opt)                        |    Offset pad... (opt)               |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                                                          L2TPv3
    0...............................................15, 16......................................31
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |                           Session ID                                                                  |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |               Cookie (optional, maximum 64 bits)...
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
                                                                                                                    |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

>> >> +/**
>> >> + * 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.