Dmitry Eremin-Solenikov(lumag) replied on github web page:
include/odp/api/spec/packet.h
@@ -71,6 +71,51 @@ extern "C" {
* Packet is red
*/
+/**
+ * Protocol
+ */
+typedef enum odp_proto_t {
+ /** No protocol defined */
+ ODP_PROTO_NONE = 0,
+
+ /** Ethernet (including VLAN) */
+ ODP_PROTO_ETH,
+
+ /** IP (including IPv4 and IPv6) */
Comment:
@Bill-Fischofer-Linaro Because it is a parsing error, if lower layer provides
us IP version that does not correspond to the in-packet version. We should
detect that, rather than silently parsing this header.
> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
> I'm not sure why an API named `odp_packet_parse()` needs help here. It's
> purpose, after all, is to parse packets and determining IPv4 vs IPv6 is part
> of that activity. Moreover, the only way an application can inspect the IP
> header is to access it via other ODP API calls, so I don't see how asking the
> application to do this is any better than having the `odp_packet_parse()`
> implementation do this itself. What's the purpose of having a parse API in
> that case since clearly the application could parse the entire packet "by
> hand" as well.
>> Dmitry Eremin-Solenikov(lumag) wrote:
>> Failure needs to be defined in a more precise way (and maybe for a single
>> packet case). I assume that it means internal ODP error, rather than just
>> packet with wrong headers. What happens in multi-packet case if failure
>> occurs in the middle of parsing?
>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>> @psavol Also for multi-packet parsing, we can change `proto` to be an
>>> array, allowing applications to easily intermix IPv4 and IPv6 packets.
>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>> @psavol yes. I just wanted to focus on cases, when passing packet with
>>>> wrong protocol is an error. E.g. IPv6 packet inside IPsec packet with NH =
>>>> 4. So it is not a question of selecting proper L3 parser, but rather a
>>>> question of nailing down error/malicious packets.
>>>>> Petri Savolainen(psavol) wrote:
>>>>> It's there to enable application to call parsing in parts - e.g. first up
>>>>> to IP and then continue from L4. But since IP and transport protocols are
>>>>> tied together with pseudo headers, it's cleaner to remove L4 as a
>>>>> starting point.
>>>>>> Petri Savolainen(psavol) wrote:
>>>>>> First bits of an IP header marks the version. So, it would be trivial
>>>>>> for both app and implementation to read the version from the data.
>>>>>> Common IP define is easier for application when a burst of packets may
>>>>>> contain both v4 and v6 mixed. Application does not need to sort packets
>>>>>> into two arrays (one for v4 and other for v6) but just pass the entire
>>>>>> array for parsing.
>>>>>>
>>>>>> There are three ways to define the enumeration: IP, IPv4+IPv6,
>>>>>> IP+IPv4+IPv6. I'm OK with any of those. IPv4+IPv6 would be a bit more
>>>>>> robust since version information comes from two sources.
>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>> I felt easier to reparse both L3 and L4 headers in IPsec case,
>>>>>>> especially since transport mode ESP can en/decrypt some of L3 headers
>>>>>>> in IPv6 case.
>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>> @Bill-Fischofer-Linaro In IPsec case Next Header field will contain 4
>>>>>>>> for IPv4 and 41 for IPv6.
>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>> That might be overly complicated since until a decrypted tunnel mode
>>>>>>>>> IPsec packet is parsed you don't know whether it's IPv4 or IPv6. It's
>>>>>>>>> parsing that makes that determination.
>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>> @lumag IPsec operating in transport mode is, I'd imagine, the main
>>>>>>>>>> use case here.
>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>> What is the usecase for parsing a packet starting from L4 header?
>>>>>>>>>>> Also there are several (lots) of other L4 protocols. Do we want to
>>>>>>>>>>> support them all here?
>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>> Maybe it would be better to split this into separate IPv4 and IPv6
>>>>>>>>>>>> packets. It would be an error to pass IPv6 packet with ethtype (or
>>>>>>>>>>>> IP tunnel type) being set to IPv4.
>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>> A vector of packets is CPU vector instructions friendly.
>>>>>>>>>>>>>> Petri Savolainen(psavol) wrote:
>>>>>>>>>>>>>> The use case is mentioned in log message: parse after decrypt/IP
>>>>>>>>>>>>>> reassembly. Application has recreated an inner packet and needs
>>>>>>>>>>>>>> to parse it before continue. This is inherently SW parse which
>>>>>>>>>>>>>> may be accelerated with CPU vector instructions, etc.
>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>> What's the use case for a multi() form of this API? Might VPP
>>>>>>>>>>>>>>> use it? Perhaps Sachin can comment?
>>>>>>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>>>>>>> We had considered an `odp_packet_parse()` function some time
>>>>>>>>>>>>>>>> back but it was rejected as something that would not fit well
>>>>>>>>>>>>>>>> with hardware parsers. What's changed?
https://github.com/Linaro/odp/pull/273#discussion_r149672029
updated_at 2017-11-08 13:48:25
