> -----Original Message-----
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
> ext Zoltan Kiss
> Sent: Friday, August 21, 2015 9:07 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [API-NEXT PATCH v5] api: packet: allow access to
> packet flow hash values
> 
> Applications can read the computed hash (if any) and set it if they
> changed the packet headers or if the platform haven't calculated the
> hash.
> 
> Signed-off-by: Zoltan Kiss <zoltan.k...@linaro.org>
> ---
> 
> v2:
> - focus on RSS hash only
> - use setter/getter's
> 
> v3:
> - do not mention pointers
> - add a note
> - add new patches for implementation and test
> 
> v4:

These are comments for v5?

> - use separate flag get and clear, as hash can have any value (that
> maps to
> checking ol_flags in DPDK)
> - change terminology to "flow hash", it reflects better what is
> actually hashed
> - add function to generate hash by the platform
> 
>  include/odp/api/packet.h       | 72
> ++++++++++++++++++++++++++++++++++++++++++
>  include/odp/api/packet_flags.h | 18 +++++++++++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
> index 3a454b5..9a74ca4 100644
> --- a/include/odp/api/packet.h
> +++ b/include/odp/api/packet.h
> @@ -605,6 +605,78 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt);
>  int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset);
> 
>  /**
> + * Packet flow hash value
> + *
> + * Returns the hash generated from the packet header. Use
> + * odp_packet_has_flow_hash() to check if packet contains a hash.
> + *
> + * @param      pkt      Packet handle
> + *
> + * @return  Hash value
> + *
> + * @note Zero can be a valid hash value.
> + * @note The hash algorithm and the header fields defining the flow
> (therefore
> + * used for hashing) is platform dependent.
> + */
> +uint32_t odp_packet_flow_hash(odp_packet_t pkt);
> +
> +/**
> + * Set packet flow hash value
> + *
> + * Store the packet flow hash for the packet and sets the flow hash
> flag. This
> + * enables application to recalculate hash for a modified packet.
> + *
> + * @param      pkt              Packet handle
> + * @param      flow_hash        Hash value to set
> + *
> + * @note The platform might use this hash on outgoing packets, so
> application
> + * should call odp_packet_flow_hash_set() with the lazy option at
> least before
> + * handing the packet back to the platform.

I think we could leave out this note and spec hash usage only for application 
usage. We'd need more specific spec for the cases (ODP API calls)  which 
expects a valid hash value. Now it can be thought only as extra metadata for 
application usage.


> + */
> +void odp_packet_flow_hash_set(odp_packet_t pkt, uint32_t flow_hash);
> +
> +/**
> + * Generate packet flow hash value
> + *
> + * Generates hash value from the packet headers. Hash generation is
> identical to
> + * packet input processing. The packet would have identical hash value
> + * when received through a packet input interface. If 'lazy' option
> specified,
> + * and the platform doesn't care about the value of this hash after it
> handed
> + * over the packet to the application, it can opt out generating one.
> + *
> + * @param      pkt      Packet handle
> + * @param      hash     Pointer for storing the hash
> + * @param      lazy     If 1, the platform can opt out
> + *
> + * @retval  0 Hash was generated
> + * @retval  <0 Hash was not generated
> + * @note If the platform wouldn't compute any hash during receive,
> then it
> + * won't generate any here either
> + * @note When the platform can opt out generating a hash (lazy == 1
> and platform
> + * doesn't care about the hash), it doesn't mean it has to. If it can
> do it
> + * really fast, implementors can decide to generate it anyway.
> + */
> +int odp_packet_flow_hash_gen(odp_packet_t pkt, uint32_t *hash, int
> lazy);


We have couple of use cases
1) application has to generate the exact input flow hash, no matter how long it 
takes
2) application needs to generate some flow hash as fast as possible (in low 
number of CPU cycles)
3) application wants to generate the exact input flow hash, but only if it's 
fast

I think 3) can be turned into 2), since application have to be prepared to use 
another hash (than the input flow hash) anyway. This is why I suggested "mode" 
parameter like this:


/**
  * Generate packet flow hash value
  *
  * Generates flow hash value from the packet. In mode 0, hash
  * generation is identical to packet input processing. The same
  * packet would have identical hash value when received through
  * a packet input interface. In mode 1, the fastest possible
  * flow hash generation method is used.
  *
  * @param      pkt      Packet handle
  * @param      hash     Pointer for storing the hash
  * @param      mode     0: Generate packet input flow hash
  *                      1: Generate fast flow hash
  *
  * @retval  0 Hash was generated
  * @retval  <0 Hash was not generated
  */
int odp_packet_flow_hash_gen(odp_packet_t pkt, uint32_t *hash, int mode);


In these two modes, the hash should be normally always generated (return 0), 
but packet errors (no L2/L3 pointer, no valid IP header, some other metadata 
missing, etc) can prevent hash generation.

This defines two (potentially) different hashes: the packet input hash and the 
"fast" hash. I think it's not a problmen, since the flow specification (which 
we define later through some e.g. pktio APIs) can be the same for both hashes. 
So, potential difference of those two modes  would be hash quality and CPU 
intensively. Additional modes could be added later on if needed (e.g. update 
current hash for ODP API X usage).


> +
> +/**
> + * Lazy generate packet flow hash value
> + *
> + * Generates hash value from the packet headers, same as
> + * odp_packet_flow_hash_gen(), but with relaxed requirements. If the
> platform
> + * doesn't care about the value of this hash after it handed over the
> packet to
> + * the application, it can opt out generating one.
> + *
> + * @param      pkt      Packet handle
> + * @param      hash     Pointer for storing the hash
> + *
> + * @retval  0 Hash was generated
> + * @retval  <0 Hash was not generated
> + * @note If the platform wouldn't compute any hash during receive,
> then it
> + * won't generate any here either
> + */
> +int odp_packet_flow_hash_gen_lazy(odp_packet_t pkt, uint32_t *hash);
> +

I guess this is covered by the previous function (this is unintentionally here)


-Petri


> +/**
>   * Tests if packet is segmented
>   *
>   * @param pkt  Packet handle
> diff --git a/include/odp/api/packet_flags.h
> b/include/odp/api/packet_flags.h
> index bfbcc94..7c3b247 100644
> --- a/include/odp/api/packet_flags.h
> +++ b/include/odp/api/packet_flags.h
> @@ -191,6 +191,15 @@ int odp_packet_has_sctp(odp_packet_t pkt);
>  int odp_packet_has_icmp(odp_packet_t pkt);
> 
>  /**
> + * Check for packet flow hash
> + *
> + * @param pkt Packet handle
> + * @retval non-zero if packet contains a hash value
> + * @retval 0 if packet does not contain a hash value
> + */
> +int odp_packet_has_flow_hash(odp_packet_t pkt);
> +
> +/**
>   * Set flag for L2 header, e.g. ethernet
>   *
>   * @param pkt Packet handle
> @@ -327,6 +336,15 @@ void odp_packet_has_sctp_set(odp_packet_t pkt, int
> val);
>  void odp_packet_has_icmp_set(odp_packet_t pkt, int val);
> 
>  /**
> + * Clear flag for packet flow hash
> + *
> + * @param pkt Packet handle
> + *
> + * @note Set this flag is only possible through
> odp_packet_flow_hash_set()
> + */
> +void odp_packet_has_flow_hash_clr(odp_packet_t pkt);
> +
> +/**
>   * @}
>   */
> 
> --
> 1.9.1
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to