From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])
by dpdk.org (Postfix) with ESMTP id 59E4E5584
for <dev@dpdk.org>; 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 <olivier.matz@6wind.com>)
id 1bLBmI-00078z-Ik; Thu, 07 Jul 2016 18:03:52 +0200
To: "Liang, Cunming" <cunming.liang@intel.com>, "dev@dpdk.org" <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>
<D0158A423229094DA7ABF71CF2FA0DA315545242@shsmsx102.ccr.corp.intel.com>
From: Olivier Matz <olivier.matz@6wind.com>
Message-ID: <aa78fc61-c8bb-4547-4c40-8d3c47331bf4@6wind.com>
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: <D0158A423229094DA7ABF71CF2FA0DA315545242@shsmsx102.ccr.corp.intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <cunming.liang@intel.com>; 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 <didier.pallard@6wind.com>
>>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>>> ---
>>>> 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 <rte_tcp.h>
>>>> #include <rte_udp.h>
>>>> #include <rte_sctp.h>
>>>> +#include <rte_mpls.h>
>>>> /* 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