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

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

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?

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?

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... else it will stay an i40e-only
mbuf field, which is probably not what we want.

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, ...)
- each type must be well defined: what does having this type means? We
   *need* to know what was checked by the hw.
- remove similar things in ol_flags to avoid having a redundant API.

Regards,
Olivier