Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
include/odp/api/spec/packet.h
line 83
@@ -1140,6 +1179,82 @@ int odp_packet_move_data(odp_packet_t pkt, uint32_t
dst_offset,
*/
/**
+ * Packet parse parameters
+ */
+typedef struct odp_packet_parse_param_t {
+ /** Protocol header at parse starting point. Valid values for this
+ * field are: ODP_PROTO_ETH, ODP_PROTO_IPV4, ODP_PROTO_IPV6. */
+ odp_proto_t proto;
+
+ /** Continue parsing until this layer. Must be the same or higher
+ * layer than the layer of 'proto'. */
+ odp_proto_layer_t layer;
+
+ /** Flags to control payload data checks up to the selected parse
+ * layer. Checksum checking status can be queried for each packet with
+ * odp_packet_l3_chksum_status() and odp_packet_l4_chksum_status().
+ */
+ union {
+ struct {
+ /** Check IPv4 header checksum */
+ uint32_t ipv4_chksum : 1;
+
+ /** Check UDP checksum */
+ uint32_t udp_chksum : 1;
+
+ /** Check TCP checksum */
+ uint32_t tcp_chksum : 1;
+
+ } check;
+
+ /** All check bits. This can be used to set/clear all flags. */
+ uint32_t all_check;
+ };
+
+} odp_packet_parse_param_t;
+
Comment:
Now that we have a `param` struct we should have an
`odp_packet_parse_param_init()` API as well for completeness.
> Dmitry Eremin-Solenikov(lumag) wrote:
> @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_r149673523
updated_at 2017-11-08 13:55:56
