This is one of the reasons I suggested that as a first-order approximation
we simply support parse none or all.  OVS clearly wants the former (for
now) and my concern is that trying to get overly fancy with selectable
parsing options isn't going to buy much since the SW will be spending
nearly as much time figuring out what to parse as it currently takes to
just do the parse.  The parsing code itself really is very straightforward
and efficient, it's just that with both OVS and ODP doing it it's wasteful
duplication of effort.

On Mon, Apr 27, 2015 at 1:05 PM, Ola Liljedahl <[email protected]>
wrote:

> On 27 April 2015 at 19:29, Zoltan Kiss <[email protected]> wrote:
>
>>
>>
>> On 20/04/15 13:10, Petri Savolainen wrote:
>>
>>> Application can indicate which packet parsing results it is
>>> interested in (all, none or selected).
>>>
>>> Signed-off-by: Petri Savolainen <[email protected]>
>>> ---
>>>   include/odp/api/packet_flags.h | 26 ++++++++++++++++++++++++++
>>>   include/odp/api/packet_io.h    | 19 +++++++++++++++++++
>>>   2 files changed, 45 insertions(+)
>>>
>>> diff --git a/include/odp/api/packet_flags.h
>>> b/include/odp/api/packet_flags.h
>>> index bfbcc94..9444fdc 100644
>>> --- a/include/odp/api/packet_flags.h
>>> +++ b/include/odp/api/packet_flags.h
>>> @@ -26,6 +26,32 @@ extern "C" {
>>>    *  @{
>>>    */
>>>
>>> +
>>> +/**
>>> + * Packet input parsing flags
>>> + *
>>> + * Each flag represents a parser output. See parser output functions for
>>> + * details.
>>>
>>
>> Now that I implement this for linux-generic, I realized this is
>> ambiguous: does disabling a lower layer's parsing means that the parser
>> stops looking into upper layers? Even if their parsing is enabled?
>>
> The application specifies what it is interested in. The ODP implementation
> has to be able to provide those answers. So if it needs to parse L2 headers
> in order to be able to answer questions about L3 headers, then be it. ODP
> could be supporting some HW where the HW provides the offset to the L3
> header and some status bit which says the packet is IPv4. Then you don't
> need SW parsing of the L2 header in order to answer the L3 questions that
> the application specified it was interested in.
>
> E.g. if (jumbo == 0 && ipv4 == 1), we won't parse the ethernet header if
>> it's a jumbo frame, that's fine. But do we continue to look into the rest
>> of the packet for the IPv4 header?
>> I would say no, but it should be mentioned here explicitly.
>>
> My interpretation is that the application is still allowed to ask whether
> a packet has an IPv4 header. This is regardless of the size of the packet
> (e.g. normal or jumbo). And I don't expect the ODP implementation to
> provide the wrong answer.
>
> The most important thing is that the semantics is clear and we seem not to
> be there yet.
>
>
>
>>
>>  + */
>>> +typedef struct odp_packet_parse_flags_t {
>>> +       uint32_t eth:1;       /**< See odp_packet_has_eth() */
>>> +       uint32_t jumbo:1;     /**< See odp_packet_has_jumbo() */
>>> +       uint32_t vlan:1;      /**< See odp_packet_has_vlan() */
>>> +       uint32_t vlan_qinq:1; /**< See odp_packet_has_vlan_qinq() */
>>> +       uint32_t arp:1;       /**< See odp_packet_has_arp() */
>>> +       uint32_t ipv4:1;      /**< See odp_packet_has_ipv4() */
>>> +       uint32_t ipv6:1;      /**< See odp_packet_has_ipv6() */
>>> +       uint32_t ipfrag:1;    /**< See odp_packet_has_ipfrag() */
>>> +       uint32_t ipopt:1;     /**< See odp_packet_has_ipopt() */
>>> +       uint32_t ipsec:1;     /**< See odp_packet_has_ipsec() */
>>> +       uint32_t udp:1;       /**< See odp_packet_has_udp() */
>>> +       uint32_t tcp:1;       /**< See odp_packet_has_tcp() */
>>> +       uint32_t sctp:1;      /**< See odp_packet_has_sctp() */
>>> +       uint32_t icmp:1;      /**< See odp_packet_has_icmp() */
>>> +
>>> +       uint32_t _reserved1:18; /**< Reserved. Do not use. */
>>> +} odp_packet_parse_flags_t;
>>> +
>>>   /**
>>>    * Check for packet errors
>>>    *
>>> diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h
>>> index 77c207e..97f79ee 100644
>>> --- a/include/odp/api/packet_io.h
>>> +++ b/include/odp/api/packet_io.h
>>> @@ -18,6 +18,8 @@
>>>   extern "C" {
>>>   #endif
>>>
>>> +#include <odp/packet_flags.h>
>>> +
>>>   /** @defgroup odp_packet_io ODP PACKET IO
>>>    *  Operations on a packet.
>>>    *  @{
>>> @@ -58,6 +60,19 @@ enum odp_pktio_input_mode {
>>>   };
>>>
>>>   /**
>>> + * Packet parsing mode
>>> + */
>>> +enum odp_pktio_parse_mode {
>>> +       /** Parse all protocols */
>>> +       ODP_PKTIN_PARSE_ALL = 0,
>>> +       /** Parsing not needed */
>>> +       ODP_PKTIN_PARSE_NONE,
>>> +       /** Parsing can be limited to the flags set in
>>> +           odp_packet_parse_flags_t */
>>> +       ODP_PKTIN_PARSE_SELECTED
>>> +};
>>> +
>>> +/**
>>>    * Packet IO parameters
>>>    *
>>>    * In minimum, user must select the input mode. Use 0 for defaults.
>>> Initialize
>>> @@ -66,6 +81,10 @@ enum odp_pktio_input_mode {
>>>   typedef struct odp_pktio_param_t {
>>>         /** Packet input mode */
>>>         enum odp_pktio_input_mode in_mode;
>>> +       /** Packet parse mode */
>>> +       enum odp_pktio_parse_mode parse_mode;
>>> +       /** Parse selection when parse_mode is ODP_PKTIN_PARSE_SELECTED
>>> */
>>> +       odp_packet_parse_flags_t parse;
>>>   } odp_pktio_param_t;
>>>
>>>   /**
>>>
>>>  _______________________________________________
>> lng-odp mailing list
>> [email protected]
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to