Re: [dpdk-dev] [PATCH 1/4] rte_mbuf:add packet types

["Liu, Jijiang" <jijiang.liu@intel.com>]

Hi Olivier,


> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, November 19, 2014 6:39 PM
> To: Liu, Jijiang; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/4] rte_mbuf:add packet types
> 
> Hi Jijiang,
> 
> On 11/18/2014 08:37 AM, Jijiang Liu wrote:
> > This patch abstracts packet types of L2 packet, Non Tunneled IPv4/6, IP in IP, IP
> in GRE, MAC in GRE and MAC in UDP, and add 4 MACROS to check packet IP
> header.
> >
> > Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
> > ---
> >   lib/librte_mbuf/rte_mbuf.h |  223
> ++++++++++++++++++++++++++++++++++++++++++++
> >   1 files changed, 223 insertions(+), 0 deletions(-)
> >
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index f5f8658..678db0d 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -125,6 +125,229 @@ extern "C" {
> >    */
> >   #define PKT_TX_OFFLOAD_MASK (PKT_TX_VLAN_PKT | PKT_TX_IP_CKSUM |
> > PKT_TX_L4_MASK)
> >
> > +/**
> > + * Ethernet packet type
> > + */
> > +enum rte_eth_packet_type {
> > +
> > +	/* undefined packet type, means HW can't recognise it */
> > +	RTE_PTYPE_UNDEF = 0,
> > +
> > +	/* L2 Packet types */
> > +	RTE_PTYPE_PAY2,
> > +	RTE_PTYPE_TimeSync_PAY2, /**< IEEE1588 and 802.1AS */
> > +	RTE_PTYPE_FIP_PAY2,      /**< FCoE Initiation Protocol */
> > +	RTE_PTYPE_LLDP_PAY2,     /**< Link Layer Discovery Protocol */
> > +	RTE_PTYPE_ECP_PAY2,      /**< Edge Control Protocol */
> > [...]
> 
> I have one question about the packet_type: it is not clear to me what the
> software can expect, for instance when RTE_PTYPE_IPv4_IPv4 is set. What does
> that mean exactly? Which fields must be valid in the packet to have this type?
> - L2 ethertype
> - Presence of vlan?
> - IP version
> - IP checksum
> - IP header length
> - IP length (compared to packet len)
> - anything about IP options?
The RTE_PTYPE_IPv4_IPv4 means that packet format is MAC, IPV4, IPV4, PAY3. The following fields are valid,
L2 ethertype
No VLAN
IPv4,

The RTE_PTYPE_IPv4_GRENAT_MACVLAN_IPv6_ICMP means that the packet format is MAC without VLAN, IPv4,GRE or UDP tunneling header, MAC with VLAN, IPv6, ICMP, PAY4
In all the packet types, I omitted MAC part.

> 
> This question applies to all types of course.
> 
> If I want to use packet type in an IP stack, I need to know which fields are
> checked by hardware (and what "checked" means for some of them), so I can do
> the remaining work in my application.
> If I want to write a new PMD (maybe a virtual one, full software), what do I need
> to check in the packet if I want to set the
> RTE_PTYPE_IPv4_IPv4 type?

For RTE_PTYPE_IPv4_IPv4, you just need to check PAY3 directly because you have already known the packet format, so you don't need to check if there is VLAN or IPv6.

I admit that the RTE_PTYPE_IPv4_IPv4 is little i40e specific. It is not standard format.

> I also feel it can be redundant with the current flags ("header is IPv4"
> for instance).

> To me, these types look very "i40e" oriented. If tomorrow (or today ?) we want to
> write a PMD for a hardware that is able to recognize IPv4, but does not do exactly
> the same checks than i40e. It is crucial that what having a packet type set
> means... 

If the packet type can't match these defined packet type, and I think we can add a new packet type in rte_eth_packet_type.

>else it will stay an i40e-only mbuf field, which is probably not what we
> want.
It is open if you don't like which name/definition of the packet types,  I can change it.

> 
> So, I think if we really want packet types to be integrated in mbuf, we need to:
> - start with a small list (maybe ipv4, ipv6, vxlan tunnels, ...).
Ok, makes sense. But a question is how to map i40e paket type if  the packet type is not defined.

For example, if the following packet type are not defined, how to map i40e packet type, we will probably omit these for i40e.
	/* L2 Packet types */
+	RTE_PTYPE_PAY2,
+	RTE_PTYPE_TimeSync_PAY2, /**< IEEE1588 and 802.1AS */
+	RTE_PTYPE_FIP_PAY2,      /**< FCoE Initiation Protocol */
+	RTE_PTYPE_LLDP_PAY2,     /**< Link Layer Discovery Protocol */
+	RTE_PTYPE_ECP_PAY2,      /**< Edge Control Protocol */
+	RTE_PTYPE_EAPOL_PAY2,
+	/**< IEEE 802.1X Extensible Authentication Protocol over LAN */
+	RTE_PTYPE_ARP,
+	RTE_PTYPE_FCOE_PAY3,
+	RTE_PTYPE_FCOE_FCDATA,
+	RTE_PTYPE_FCOE_FCRDY,
+	RTE_PTYPE_FCOE_FCRSP,
+	RTE_PTYPE_FCOE_FCOTHER,
+	RTE_PTYPE_FCOE_VFT,
+	RTE_PTYPE_FCOE_VFT_FCDATA,
+	RTE_PTYPE_FCOE_VFT_FCRDY,
+	RTE_PTYPE_FCOE_VFT_FCRSP,
+	RTE_PTYPE_FCOE_VFT_FCOTHER,
+
> - each type must be well defined: what does having this type means? We
>    *need* to know what was checked by the hw.
Current packet name have already had clear meaning, I thought.

> - remove similar things in ol_flags to avoid having a redundant API.

Yes, when all i40e/ixgbe/igb PMDs done, the related IP header offload should be removed.
 I just changed for i40e, there still are igb&ixgbe need to be changed in DPDK2.0, so we can't remove the IP ol_flags now.

Actually, I think it will be a good time to integrate all the changes for packet type when all the PMDs done in DPDK2.0.
 Thomas, do you agree on this?

> 
> Regards,
> Olivier