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.