> -----Original Message-----
> From: Sean Harte [mailto:sea...@gmail.com]
> Sent: Thursday, September 28, 2017 9:43 PM
> To: Xing, Beilei <beilei.x...@intel.com>
> Cc: Wu, Jingjing <jingjing...@intel.com>; Chilikin, Andrey
> <andrey.chili...@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 4/8] ethdev: add GTP items to support
> flow API
>
> On 28 September 2017 at 09:13, Beilei Xing <beilei.x...@intel.com> wrote:
> > This patch adds GTP, GTPC and GTPU items for generic flow API, and
> > also exposes item fields through the flow command.
> >
> > Signed-off-by: Beilei Xing <beilei.x...@intel.com>
> > Acked-by: Adrien Mazarguil <adrien.mazarg...@6wind.com>
> > ---
> > app/test-pmd/cmdline_flow.c | 40 ++++++++++++++++++++++
> > app/test-pmd/config.c | 3 ++
> > doc/guides/prog_guide/rte_flow.rst | 18 ++++++++++
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 +++
> > lib/librte_ether/rte_flow.h | 52
> +++++++++++++++++++++++++++++
> > 5 files changed, 117 insertions(+)
> <snip>
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -955,6 +955,24 @@ Usage example, fuzzy match a TCPv4 packets:
> > | 4 | END |
> > +-------+----------+
> >
> > +Item: ``GTP``, ``GTPC``, ``GTPU``
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Matches a GTP header.
> > +
> > +Note: GTP, GTPC and GTPU use the same structure. Since only UDP
> > +destination port is used to distinguish GTP_C (port is 2123) and
> > +GTP_U packets (port is 2152), GTPC and GTPU item are defined for a
> > +user-friendly API when creating GTP-C and GTP-U flow.
>
> In GTPv1-C, request messages are sent from any port to port 2123, and in the
> response message the ports are reversed (in GTPv2-C, it's a little more
> complicated). Is the intention to only match requests?
> It's not clear.
>
> Also, it should be mentioned that GTPv0 is not included (it uses port 3386)
Thanks for the comments, will clarify them in next version.
>
> > +
> > +- ``v_pt_rsv_flags``: version (3b), protocol type (1b), reserved
> > +(1b),
> > + extension header flag (1b), sequence number flag (1b), N-PDU number
> > + flag (1b).
> > +- ``msg_type``: message type.
> > +- ``msg_len``: message length.
> > +- ``teid``: tunnel endpoint identifier.
> > +- Default ``mask`` matches teid only.
> > +
> > Actions
> > ~~~~~~~
> >
> <snip>
> > /**
> > + * RTE_FLOW_ITEM_TYPE_GTP.
> > + *
> > + * Matches a GTP header.
> > + */
> > +struct rte_flow_item_gtp {
> > + /**
> > + * Version (2b), protocol type (1b), reserved (1b),
> > + * Extension header flag (1b),
> > + * Sequence number flag (1b),
> > + * N-PDU number flag (1b).
> > + */
> > + uint8_t v_pt_rsv_flags;
>
> The version field has 3 bits, not 2 (it was correct above). The meaning of
> the 5
> flags in this byte is different in GTPv2-C compared to GTPv1-C. Is the
> intention to only support GTPv1? If so that should be stated. If GTPv2 is
> supported, then the teid field below is not present in a few cases and
> matching on it could cause some strange behaviour.
Thanks for the correction, I will change version filed in next version.
And yes, we only support GTPv1 only, will clarify it.
>
> > + uint8_t msg_type; /**< Message type. */
> > + rte_be16_t msg_len; /**< Message length. */
> > + rte_be32_t teid; /**< Tunnel endpoint identifier. */ };
> > +
> > +/** Default mask for RTE_FLOW_ITEM_TYPE_GTP. */ #ifndef __cplusplus
> > +static const struct rte_flow_item_gtp rte_flow_item_gtp_mask = {
> > + .teid = RTE_BE32(0xffffffff),
> > +};
> > +#endif
> > +
> > +/**
> > * Matching pattern item definition.
> > *
> > * A pattern is formed by stacking items starting from the lowest
> > protocol
> > --
> > 2.5.5
> >
