Petri Savolainen(psavol) replied on github web page:
include/odp/api/spec/packet.h
line 129
@@ -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;
+
+/**
+ * Parse packet
+ *
+ * Parse protocol headers in packet data. Parsing starts at 'offset', which
+ * is the first header byte of protocol 'param.proto'. Parameter 'param.layer'
+ * defines the last layer application is interested about.
+ * Use ODP_PROTO_LAYER_ALL for all layers. The operation sets or resets packet
+ * metadata for all layers from the layer of 'param.proto' to the application
+ * defined last layer. Metadata of other layers have undefined values.
+ * When operation fails, metadata of all protocol layers have undefined values.
+ *
+ * @param pkt Packet handle
+ * @param offset Byte offset into the packet
+ * @param param Parse parameters. Proto and layer fields must be set. Clear
+ * all check bits that are not used.
+ *
+ * @retval 0 on success
+ * @retval <0 on failure
+ */
+int odp_packet_parse(odp_packet_t pkt, uint32_t offset,
+ const odp_packet_parse_param_t *param);
+
+/**
+ * Parse multiple packets
+ *
+ * Otherwise like odp_packet_parse(), but parses multiple packets. Packets may
+ * have unique offsets, but must start with the same protocol. Also, packets
are
+ * parsed up to the same protocol layer.
+ *
+ * @param pkt Packet handle array
+ * @param offset Byte offsets into the packets
+ * @param num Number of packets and offsets
+ * @param param Parse parameters. Proto and layer fields must be set. Clear
+ * all check bits that are not used.
+ *
+ * @retval 0 on success
+ * @retval <0 on failure
+ */
+int odp_packet_parse_multi(const odp_packet_t pkt[], const uint32_t offset[],
+ int num, const odp_packet_parse_param_t *param);
+
Comment:
This function is different from e.g. enqueue multi, where the destination queue
may be close to full and that's why only part of events were enqueued. Here
operation fails due to bad params or internal error.
Anyway, I'll change it to return num processed. It's more flexible.
> bogdanPricope wrote
> What about: "When operation fails, metadata of checked layers indicates the
> error condition."?
>> bogdanPricope wrote
>> What about: "Metadata of other layers remains unmodified."?
>>> Petri Savolainen(psavol) wrote:
>>> Layer is not enough information for the parser. It needs to know the first
>>> protocol (e.g. MPLS vs IP) to be able to start the parsing.
>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>> @psavol `_param_init` seems like an overkill in this case. From my point
>>>> of view, `param_init` are good for object creation params (where we might
>>>> not know platform-optimized defaults) but are an overkill for operation
>>>> functions (we do not have `param_init` for ipsec, crypto, etc operational
>>>> params).
>>>>> Petri Savolainen(psavol) wrote:
>>>>> 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_r149913862
updated_at 2017-11-09 12:49:44
