From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 59E4E5584 for ; Thu, 7 Jul 2016 18:01:23 +0200 (CEST) Received: from lfbn-1-8274-170.w81-254.abo.wanadoo.fr ([81.254.171.170] helo=[192.168.1.13]) by mail.droids-corp.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1bLBmI-00078z-Ik; Thu, 07 Jul 2016 18:03:52 +0200 To: "Liang, Cunming" , "dev@dpdk.org" References: <1467733310-20875-1-git-send-email-olivier.matz@6wind.com> <1467733310-20875-10-git-send-email-olivier.matz@6wind.com> <577CAE66.6050606@intel.com> <299f6d61-0ec4-dae4-d79d-fb746dc201b7@6wind.com> From: Olivier Matz Message-ID: Date: Thu, 7 Jul 2016 18:01:14 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 09/18] mbuf: support Mpls in software packet type parser X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Jul 2016 16:01:23 -0000 Hi Cunming, On 07/07/2016 10:48 AM, Liang, Cunming wrote: > Hi Olivier, > >> -----Original Message----- >> From: Olivier MATZ [mailto:olivier.matz@6wind.com] >> Sent: Wednesday, July 06, 2016 4:00 PM >> To: Liang, Cunming ; dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH 09/18] mbuf: support Mpls in software packet >> type parser >> >> Hi Cunming, >> >> On 07/06/2016 09:08 AM, Liang, Cunming wrote: >>> Hi Olivier, >>> >>> On 7/5/2016 11:41 PM, Olivier Matz wrote: >>>> Add a new RTE_PTYPE_L2_ETHER_MPLS packet type, and its support in >>>> rte_pktmbuf_get_ptype(). >>>> >>>> Signed-off-by: Didier Pallard >>>> Signed-off-by: Olivier Matz >>>> --- >>>> lib/librte_mbuf/rte_mbuf_ptype.c | 25 +++++++++++++++++++++++++ >>>> lib/librte_mbuf/rte_mbuf_ptype.h | 9 ++++++++- >>>> lib/librte_net/Makefile | 4 +++- >>>> lib/librte_net/rte_ether.h | 2 ++ >>>> 4 files changed, 38 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/lib/librte_mbuf/rte_mbuf_ptype.c >>>> b/lib/librte_mbuf/rte_mbuf_ptype.c >>>> index 5d46608..0dea600 100644 >>>> --- a/lib/librte_mbuf/rte_mbuf_ptype.c >>>> +++ b/lib/librte_mbuf/rte_mbuf_ptype.c >>>> @@ -41,6 +41,7 @@ >>>> #include >>>> #include >>>> #include >>>> +#include >>>> /* get l3 packet type from ip6 next protocol */ >>>> static uint32_t >>>> @@ -166,6 +167,9 @@ uint32_t rte_pktmbuf_get_ptype(const struct >>>> rte_mbuf *m, >>>> off = sizeof(*eh); >>>> hdr_lens->l2_len = off; >>>> + if (proto == rte_cpu_to_be_16(ETHER_TYPE_IPv4)) >>>> + goto l3; /* fast path if packet is IPv4 */ >>>> + >>>> if (proto == rte_cpu_to_be_16(ETHER_TYPE_VLAN)) { >>>> const struct vlan_hdr *vh; >>>> struct vlan_hdr vh_copy; >>>> @@ -189,8 +193,29 @@ uint32_t rte_pktmbuf_get_ptype(const struct >>>> rte_mbuf *m, >>>> off += 2 * sizeof(*vh); >>>> hdr_lens->l2_len += 2 * sizeof(*vh); >>>> proto = vh->eth_proto; >>>> + } else if ((proto == rte_cpu_to_be_16(ETHER_TYPE_MPLS)) || >>>> + (proto == rte_cpu_to_be_16(ETHER_TYPE_MPLSM))) { >>>> + unsigned int i; >>>> + const struct mpls_hdr *mh; >>>> + struct mpls_hdr mh_copy; >>>> + >>>> +#define MAX_MPLS_HDR 5 >>>> + for (i = 0; i < MAX_MPLS_HDR; i++) { >>>> + mh = rte_pktmbuf_read(m, off + (i * sizeof(*mh)), >>>> + sizeof(*mh), &mh_copy); >>>> + if (unlikely(mh == NULL)) >>>> + return pkt_type; >>>> + if (mh->bs) >>>> + break; >>>> + } >>>> + if (i == MAX_MPLS_HDR) >>>> + return pkt_type; >>>> + pkt_type = RTE_PTYPE_L2_ETHER_MPLS; >>>> + hdr_lens->l2_len += (sizeof(*mh) * (i + 1)); >>> [LC] l2_len includes Eth, Vlan(opt.), MPLS(opt.). For VLAN and MPLS, it >>> may include #n times overlay. >>> These layer recognition knowledge are lost after the detection logic. >>> Once the APP takes the ptype, for the layer(L2, L3, L4) which has more >>> shim-layer, the xxx_len can't help to avoid the re-parse cost. >> >> This is linked with the definition of packet type. Each layer has a >> type, and here we associate it to a length (by the way the length is >> something we may consider integrate inside the packet type in the future). > [LC] Yes, I see. > My point is in some case, the length can represent for different layer. > For who interests on L2 MPLS, the length layer scheme maybe can define as {L2/MPLS/inner_L2/inner_L3}. > The rte_mbuf_hdr_lens likes a meta data which associates with the specific parser(assuming customized runtime instance provided by rte_pktmbuf_get_ptype). > The provider understand the meaning and layout. OK, I see. For VLAN or QinQ, we could consider that it is the same L2 than the Ethernet header. But maybe MPLS should not be part of this patchset, because it's actually a bit different. The choice I've made was to represent MPLS in packet_type like this: Ether - MPLS - IP - TCP \ / | | L2 L3 L4 Another way to represent it would be: Ether - MPLS - IP - TCP | | | | L2 INNER_L2 INNER_L4 INNER_L3 If it's too confusing, we may remove MPLS from this patchset. Regards, Olivier > >> >> The packet_type model allows to describe many packets kinds. Some will >> be difficult to represent (ex: a packet with several different L2 or >> L3). But I think this is a good compromise that could help the >> application to get some information without looking inside the packet. >> >> Changing the packet type structure to something more flexible/complex >> would probably imply to loose time filling it in drivers and parse it in >> the application. And we already have a structure that contains all the >> information needed by the application: the packet data ;) > [LC] Fully agree. Sometimes it's a tradeoff, if the offering meta data by parser is > not enough for further processing, the duplication packet data walking through may happen. > It's hard to define a meta data format for all cases. Probably the raw META is a good choice, which is recognized by the parser provider. > >> >> In any case, this is not really the topic of the patchset, which just >> provide a helper to parse a packet by software and get a packet_type (as >> it is defined today). > [LC] Maybe the conversation is a little beyond. Hope you get my point. > > Thanks. > >> >> Regards, >> Olivier