On Mon, Oct 21, 2019 at 11:52:46AM +0100, Ferruh Yigit wrote:
> On 10/21/2019 10:28 AM, Ferruh Yigit wrote:
> > On 10/21/2019 1:29 PM, Ting Xu wrote:
> >> This patch enables testpmd to forward GTP packet in csum fwd mode.
> >> GTP header structure (without optional fields and extension header)
> >> and parser function are added. GTPU and GTPC packets are both
> >> supported, with respective UDP destination port and GTP message
> >> type.
> >>
> >> Signed-off-by: Ting Xu <ting...@intel.com>
> >>
> >> ---
> >> v4: Move GTP header defination to rte_ether.h
> >> v3: correct coding style issue.
> >> v2: modify commit log
> >> depend on patch: lib/mbuf: add GTP tunnel type flag.
> >> ---
> >> app/test-pmd/csumonly.c | 96 ++++++++++++++++++++++++++++++++++----
> >> lib/librte_net/rte_ether.h | 23 +++++++++
> >> 2 files changed, 109 insertions(+), 10 deletions(-)
> >
> > <...>
> >
> >> --- a/lib/librte_net/rte_ether.h
> >> +++ b/lib/librte_net/rte_ether.h
> >> @@ -342,6 +342,29 @@ struct rte_vxlan_gpe_hdr {
> >> sizeof(struct rte_vxlan_gpe_hdr))
> >> /**< VXLAN-GPE tunnel header length. */
> >>
> >> +/**
> >> + * Simplified GTP protocol header.
> >> + * Contains 8-bit flag, 8-bit message type,
> >> + * 16-bit message length, 32-bit TEID.
> >> + * No optional fields and next extension header.
> >> + */
> >> +struct rte_gtp_hdr {
> >> + uint8_t gtp_hdr_info;
> >> + uint8_t msg_type;
> >> + uint16_t msg_len;
> >> + uint32_t teid;
> >> +} __attribute__((__packed__));
> >> +
> >> +/* GTP header length */
> >> +#define RTE_ETHER_GTP_HLEN \
> >> + (sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
> >> +/* GTP next protocal type */
> >> +#define RTE_GTP_TYPE_IPV4 0x40
> >> +#define RTE_GTP_TYPE_IPV6 0x60
> >> +/* GTP destination port number */
> >> +#define RTE_GTPC_UDP_PORT 2123
> >> +#define RTE_GTPU_UDP_PORT 2152
> >> +
> >> /**
> >> * Extract VLAN tag information into mbuf
> >> *
> >>
> >
> > lgtm, @Oliver, any comment/objection on GTP part in rte_ether.h?
> >
>
> What about moving the defines to its own header, like rte_gtp? Is these
> defines
> part of the ether?
+1
I also think it deserves its own file.
