DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Liu, Jijiang" <jijiang.liu@intel.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
Date: Thu, 13 Nov 2014 03:17:47 +0000	[thread overview]
Message-ID: <1ED644BD7E0A5F4091CF203DAFB8E4CC01D97183@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <2835075.ZIuhO63ZWU@xps13>



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, November 12, 2014 9:26 PM
> To: Liu, Jijiang
> Cc: Zhang, Helin; dev@dpdk.org; Richardson, Bruce
> Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure
> changes
> 
> Hi guys,
> 
> We still have some problems with the mbuf changes introduced for VXLAN.
> I want to raise the packet type issue here.
> 
> 2014-10-23 02:23, Zhang, Helin:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > > 2014-10-21 14:14, Liu, Jijiang:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > 2014-10-21 16:46, Jijiang Liu:
> > > > > > -	uint16_t reserved2;       /**< Unused field. Required for padding
> */
> > > > > > +
> > > > > > +	/**
> > > > > > +	 * Packet type, which is used to indicate ordinary L2 packet
> format and
> > > > > > +	 * also tunneled packet format such as IP in IP, IP in GRE, MAC in
> GRE
> > > > > > +	 * and MAC in UDP.
> > > > > > +	 */
> > > > > > +	uint16_t packet_type;
> > > > >
> > > > > Why not name it "l2_type"?
> >
> > 'packet_type' is for storing the hardware identified packet type upon
> > different layers of protocols (l2, l3, l4, ...).
> > It is quite useful for user application or middle layer software
> > stacks, it can know what the packet type is without checking the packet too
> much by software.
> > Actually ixgbe already has packet types (less than 10), which is transcoded into
> 'ol_flags'.
> > For i40e, the packet type can represent about 256 types of packet,
> > 'ol_flags' does not have enough bits for it anymore. So put the i40e packet types
> into mbuf would be better.
> > Also this field can be used for NON-Intel NICs, I think there must be
> > the similar concepts of other NICs. And 16 bits 'packet_type' has severl
> reserved bits for future and NON-Intel NICs.
> 
> Thanks Helin, that's the best description of packet_type I've seen so far.
> It's not so clear in the commit log:
> 	http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf
> 
> > > > In datasheet, this term is called packet type(s).
> > >
> > > That's exactly the point I want you really understand!
> > > This is a field in generic mbuf structure, so your datasheet has no value here.
> > >
> > > > Personally , I think packet type is  more clear what meaning of this field is .
> > >
> > > You cannot add an API field without knowing what will be its generic meaning.
> > > Please think about it and describe its scope.
> 
> I integrated this patch with the VXLAN patchset in the hope that you'll improve
> the situation afterwards.
> This is the answer you recently gave to Olivier:
> 	http://dpdk.org/ml/archives/dev/2014-November/007599.html
> "
> 	Regarding adding a packet_type in mbuf, we ever had a lot of discussions
> as follows:
> 	http://dpdk.org/ml/archives/dev/2014-October/007027.html
> 	http://dpdk.org/ml/archives/dev/2014-September/005240.html
> 	http://dpdk.org/ml/archives/dev/2014-September/005241.html
> 	http://dpdk.org/ml/archives/dev/2014-September/005274.html
> "
> 
> To sum up the situation:
> - We don't know what are the possible values of packet_type
> - It's only filled by i40e, while other drivers use ol_flags
> - There is no special value "unknown" which should be set by drivers
>   not supporting this feature.
> - Its only usage is to print a decimal value in app/test-pmd/rxonly.c
> 
> It's now clear that nobody cares about this part of the API.
> So I'm going to remove packet_type from mbuf.
> I don't want to keep something that we don't know how to use, that is not
> consistent across drivers, and that overlap another API part (ol_flags).

The packet type in 40e is very important for user, using packet type can help to speed up packet analysis/identification in their application, especially tunneling packet format.
Now I'm working on implementing packet type definition in rte_ethdev.h file and  translation table in i40e, which is almost done. 
The packet type  definition in in rte_ethdev.h file like below. 
/*
 * Ethernet packet type
 */
enum rte_eth_ptype {
        /* undefined packet type, means HW can't recognise it */
        RTE_PTYPE_UNDEF = 0,
...

        /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv4 */
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4FRAG_PAY3,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_PAY3,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_UDP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_TCP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_SCTP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_ICMP_PAY4,
 
        /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv6 */
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6FRAG_PAY3
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_PAY3,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_UDP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_TCP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_SCTP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_ICMP_PAY4,
 
        /* IPv4 --> GRE/Teredo/VXLAN --> MAC/VLAN */
        RTE_PTYPE_IPv4_GRENAT_MACVLAN_PAY3,
... 
}

Yes, we don't use packet type in many places now, which doesn't mean we don't use it  in the future( when supporting another tunneling packet).

It is ok for me if you want to remove the packet_type filed in mbuf,  but we will send a separate patch set for introducing packet type in the future, which includes 1g/10/40g PMD changes.

> --
> Thomas

  parent reply	other threads:[~2014-11-13  3:07 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-21  8:45 [dpdk-dev] [PATCH v6 0/9] Support VxLAN on Fortville Jijiang Liu
2014-10-21  8:46 ` [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes Jijiang Liu
2014-10-21 10:26   ` Thomas Monjalon
2014-10-21 14:14     ` Liu, Jijiang
2014-10-22  8:45       ` Thomas Monjalon
2014-10-22  8:53         ` Liu, Jijiang
2014-10-23  2:23         ` Zhang, Helin
2014-11-12 13:26           ` Thomas Monjalon
2014-11-12 14:31             ` Zhang, Helin
2014-11-12 15:23               ` Thomas Monjalon
2014-11-13  3:17             ` Liu, Jijiang [this message]
2014-11-13  8:53               ` Thomas Monjalon
2014-11-13 11:24                 ` Liu, Jijiang
2014-11-13 11:35                   ` Thomas Monjalon
2014-10-21  8:46 ` [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet identification API in librte_ether Jijiang Liu
2014-10-21 10:51   ` Thomas Monjalon
2014-10-21 13:48     ` Liu, Jijiang
2014-10-21 21:19       ` Thomas Monjalon
2014-10-22  1:46         ` Liu, Jijiang
2014-10-22  9:52           ` Thomas Monjalon
2014-10-22 12:47             ` Liu, Jijiang
2014-10-22 13:25               ` Thomas Monjalon
2014-10-22  5:21         ` Liu, Jijiang
2014-10-21  8:46 ` [dpdk-dev] [PATCH v6 3/9] i40e:support VxLAN packet identification in librte_pmd_i40e Jijiang Liu
2014-10-21  8:46 ` [dpdk-dev] [PATCH v6 4/9] app/test-pmd:test VxLAN packet identification Jijiang Liu
2014-10-21  8:46 ` [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of VxLAN filter Jijiang Liu
2014-10-21 15:13   ` Thomas Monjalon
2014-10-22  2:25     ` Liu, Jijiang
2014-10-22  9:31       ` Thomas Monjalon
2014-10-22 11:03         ` Liu, Jijiang
2014-10-23  9:06           ` Chilikin, Andrey
2014-10-22  6:45     ` Liu, Jijiang
2014-10-22  9:25       ` Thomas Monjalon
2014-10-22 13:54         ` Liu, Jijiang
2014-10-21  8:46 ` [dpdk-dev] [PATCH v6 6/9] i40e:implement API of VxLAN packet filter in librte_pmd_i40e Jijiang Liu
2014-10-21  8:46 ` [dpdk-dev] [PATCH v6 7/9] app/testpmd:test VxLAN packet filter Jijiang Liu
2014-10-21  8:46 ` [dpdk-dev] [PATCH v6 8/9] i40e:support VxLAN Tx checksum offload Jijiang Liu
2014-10-21  8:46 ` [dpdk-dev] [PATCH v6 9/9] app/testpmd:test " Jijiang Liu
2014-10-21 14:54 ` [dpdk-dev] [PATCH v6 0/9] Support VxLAN on Fortville Liu, Yong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1ED644BD7E0A5F4091CF203DAFB8E4CC01D97183@SHSMSX101.ccr.corp.intel.com \
    --to=jijiang.liu@intel.com \
    --cc=dev@dpdk.org \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).