[dpdk-dev] [PATCH 1/2] mbuf: support PPPoE and L2TP in software packet type parser

[Xing, Beilei]

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Tuesday, December 19, 2017 6:11 PM
> To: Xing, Beilei <beilei.xing at intel.com>
> Cc: Wu, Jingjing <jingjing.wu at intel.com>; dev at dpdk.org; Chilikin, Andrey
> <andrey.chilikin at intel.com>
> Subject: Re: [PATCH 1/2] mbuf: support PPPoE and L2TP in software packet
> type parser
> 
> Hi Beilei,
> 
> On Tue, Nov 28, 2017 at 06:12:55PM +0800, Beilei Xing wrote:
> > Add support of PPPoE and L2TP in rte_net_get_ptype().
> >
> > Signed-off-by: Beilei Xing <beilei.xing at intel.com>
> > ---
> >  lib/librte_mbuf/rte_mbuf_ptype.c |  2 ++
> > lib/librte_mbuf/rte_mbuf_ptype.h | 26 ++++++++++++++++++++++++++
> >  2 files changed, 28 insertions(+)
> >
> > diff --git a/lib/librte_mbuf/rte_mbuf_ptype.c
> > b/lib/librte_mbuf/rte_mbuf_ptype.c
> > index a623226..e0f2a92 100644
> > --- a/lib/librte_mbuf/rte_mbuf_ptype.c
> > +++ b/lib/librte_mbuf/rte_mbuf_ptype.c
> > @@ -47,6 +47,7 @@ const char *rte_get_ptype_l2_name(uint32_t ptype)
> >  	case RTE_PTYPE_L2_ETHER_NSH: return "L2_ETHER_NSH";
> >  	case RTE_PTYPE_L2_ETHER_VLAN: return "L2_ETHER_VLAN";
> >  	case RTE_PTYPE_L2_ETHER_QINQ: return "L2_ETHER_QINQ";
> > +	case RTE_PTYPE_L2_ETHER_PPPOE: return "L2_ETHER_PPPOE";
> >  	default: return "L2_UNKNOWN";
> >  	}
> >  }
> > @@ -92,6 +93,7 @@ const char *rte_get_ptype_tunnel_name(uint32_t
> ptype)
> >  	case RTE_PTYPE_TUNNEL_GTPC: return "TUNNEL_GTPC";
> >  	case RTE_PTYPE_TUNNEL_GTPU: return "TUNNEL_GTPU";
> >  	case RTE_PTYPE_TUNNEL_ESP: return "TUNNEL_ESP";
> > +	case RTE_PTYPE_TUNNEL_L2TP: return "TUNNEL_L2TP";
> >  	default: return "TUNNEL_UNKNOWN";
> >  	}
> >  }
> > diff --git a/lib/librte_mbuf/rte_mbuf_ptype.h
> > b/lib/librte_mbuf/rte_mbuf_ptype.h
> > index 5c62435..256f024 100644
> > --- a/lib/librte_mbuf/rte_mbuf_ptype.h
> > +++ b/lib/librte_mbuf/rte_mbuf_ptype.h
> > @@ -153,6 +153,13 @@ extern "C" {
> >   */
> >  #define RTE_PTYPE_L2_ETHER_QINQ             0x00000007
> >  /**
> > + * PPPOE packet type.
> > + *
> > + * Packet format:
> > + * <'ether type'=[0x8863|0x8864]>
> > + */
> > +#define RTE_PTYPE_L2_ETHER_PPPOE            0x00000008
> > +/**
> >   * Mask of layer 2 packet types.
> >   * It is used for outer packet for tunneling cases.
> >   */
> > @@ -426,6 +433,25 @@ extern "C" {
> >   */
> >  #define RTE_PTYPE_TUNNEL_ESP                0x00009000
> >  /**
> > + * L2TP (Layer 2 Tunneling Protocol) tunnleing packet type.
> > + *
> > + * Packet format:
> > + * <'ether type'=0x0800
> > + * | 'version'=4, 'protocol'=17>
> > + * | 'destination port'=1701>
> > + * or,
> > + * <'ether type'=0x86DD
> > + * | 'version'=6, 'next header'=17
> > + * | 'destination port'=1701>
> > + * or,
> > + * <'ether type'=0x0800
> > + * | 'version'=4, 'protocol'=115>
> > + * or,
> > + * <'ether type'=0x86DD
> > + * | 'version'=6, 'protocol'=115>
> > + */
> > +#define RTE_PTYPE_TUNNEL_L2TP               0x0000a000
> > +/**
> >   * Mask of tunneling packet types.
> >   */
> >  #define RTE_PTYPE_TUNNEL_MASK               0x0000f000
> 
> The patch looks good, but it should be renamed:
> 
> > mbuf: support PPPoE and L2TP in software packet type parser
> 
> The patch just adds the definition of the packet types, it does not add the
> support of these packet types in the software parser, which is located in
> rte_net_get_ptype().
> 
> So, I suggest this title instead:
> 
>   mbuf: add PPPoE and L2TP packet types

Yes, thanks for the comments. Will update in the next version.

> 
> 
> Thanks
> Olivier