Petri Savolainen(psavol) replied on github web page:
include/odp/api/spec/packet.h
line 74
@@ -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;
Comment:
No, other APIs order checksum checking the same way: per protocol. Application
may be e.g. terminating only UDP and forwarding all others - so it would be
waste to check e.g. TCP checksum.
typedef union odp_pktin_config_opt_t {
/** Option flags */
struct {
/** Timestamp all packets on packet input */
uint64_t ts_all : 1;
/** Timestamp (at least) IEEE1588 / PTP packets
* on packet input */
uint64_t ts_ptp : 1;
/** Check IPv4 header checksum on packet input */
uint64_t ipv4_chksum : 1;
/** Check UDP checksum on packet input */
uint64_t udp_chksum : 1;
/** Check TCP checksum on packet input */
uint64_t tcp_chksum : 1;
> Petri Savolainen(psavol) wrote:
> Param init on fast path is a bit overkill. However, application could call it
> once and store the result. So, it could avoid extra function call for every
> packet. So, I'll add it.
>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>> `return i;` if we adopt the RC == number of successful packets parsed
>> convention.
>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>> For consistency with other `odp_xxx_multi()` APIs, the RC should indicate
>>> the number of input packets that were successfully processed.
>>>> muvarov wrote
>>>> it looks like 2 enums are not needed here. odp_proto_layet_t layer_start;
>>>> and odp_proto_layet_t layer_end; and one enum for layers.
>>>>> muvarov wrote
>>>>> maybe return number of correctly parsed packets? And say that parsing
>>>>> always starts from be beginning of array.
>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>> +1 since other checksum APIs refer to L3 and L4 rather than specific
>>>>>> protocols.
>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>> What about just `l3_chksum`, and `l4_chksum`?
>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>> 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_r149898727
updated_at 2017-11-09 09:04:14
