> -----Original Message----- > From: Sean Harte [mailto:sea...@gmail.com] > Sent: Tuesday, October 3, 2017 4:57 PM > To: Adrien Mazarguil <adrien.mazarg...@6wind.com> > Cc: Xing, Beilei <beilei.x...@intel.com>; Wu, Jingjing > <jingjing...@intel.com>; Chilikin, > Andrey <andrey.chili...@intel.com>; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 4/8] ethdev: add GTP items to support flow > API > > On 2 October 2017 at 13:27, Adrien Mazarguil <adrien.mazarg...@6wind.com> > wrote: > > On Fri, Sep 29, 2017 at 10:29:55AM +0100, Sean Harte wrote: > >> On 29 September 2017 at 09:54, Xing, Beilei <beilei.x...@intel.com> wrote: > > <snip> > >> >> > /** > >> >> > + * RTE_FLOW_ITEM_TYPE_GTP. > >> >> > + * > >> >> > + * Matches a GTPv1 header. > >> >> > + */ > >> >> > +struct rte_flow_item_gtp { > >> >> > + /** > >> >> > + * Version (3b), protocol type (1b), reserved (1b), > >> >> > + * Extension header flag (1b), > >> >> > + * Sequence number flag (1b), > >> >> > + * N-PDU number flag (1b). > >> >> > + */ > >> >> > + uint8_t v_pt_rsv_flags; > >> >> > + uint8_t msg_type; /**< Message type. */ > >> >> > + rte_be16_t msg_len; /**< Message length. */ > >> >> > + rte_be32_t teid; /**< Tunnel endpoint identifier. */ }; > >> >> > >> >> In future, you might add support for GTPv2 (which is used since LTE). > >> >> Maybe this structure should have v1 in its name to avoid confusion? > >> > > >> > I considered it before. But I think we can modify it when we support > >> > GTPv2 in future, > and keep concise 'GTP' currently:) since I have described it matches v1 > header. > >> > > >> > >> You could rename v_pt_rsv_flags to version_flags to avoid some future > >> code changes to support GTPv2. There's still the issue that not all > >> GTPv2 messages have a TEID though. > > > > Although they have the same size, the header of these two protocols > > obviously differs. My suggestion would be to go with a separate GTPv2 > > pattern item using its own dedicated structure instead. > > > > -- > > Adrien Mazarguil > > 6WIND > > The 1st four bytes are the same (flags in first byte have different > meanings, but the bits indicating the version are in the same > location). After that, different fields in each version are optional, > and the headers have variable size. A single structure could be used > if the first field is renamed to something like "version_flags", and > then check that the teid field in item->mask is not set if > ((version_flags >> 5 == 2) && ((version_flags >> 4) & 1) == 1). If > there's going to be two structures, it would be good to put v1 and v2 > in the names, in my opinion.
I think the name GTP is OK for now. Due to v1 and v2 are different, why not rename them when the v2 supporting are introduced?
