Re: [dpdk-dev] [PATCH 09/18] mbuf: support Mpls in software packet type parser

[Olivier MATZ <olivier.matz@6wind.com>]

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).

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 ;)

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).

Regards,
Olivier