RE: [PATCH 1/4] ethdev: introduce encap hash calculation

[Ori Kam <orika@nvidia.com>]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, February 7, 2024 12:40 AM
> 
> 28/01/2024 10:39, Ori Kam:
> > During the encapsulation of a packet, it is expected to calculate the
> > hash value which is based on the original packet (the outer values,
> > which will become the inner values).
> 
> It is not clear what the hash is for.

Will add explanation.

> 
> > The tunnel protocol defines which tunnel field should hold this hash,
> > but it doesn't define the hash calculation algorithm.
> 
> If the hash is stored in the packet header,
> I expect it to be reproducible when being checked.
> How the algorithm may be undefined?
> 
The hash is not being checked it is used for hash the packets to different queues.
the actual value is not important. It is critical that all packets that belongs to the same
flow will have the same hash value.

> > An application that uses flow offloads gets the first few packets
> > and then decides to offload the flow. As a result, there are two
> > different paths that a packet from a given flow may take.
> > SW for the first few packets or HW for the rest.
> > When the packet goes through the SW, the SW encapsulates the packet
> > and must use the same hash calculation as the HW will do for
> > the rest of the packets in this flow.
> >
> > This patch gives the SW a way to query the hash value
> > for a given packet as if the packet was passed through the HW.
> >
> > Signed-off-by: Ori Kam <orika@nvidia.com>
> > ---
> > +Calculate encap hash
> > +~~~~~~~~~~~~~~~~~~~~
> > +
> > +Calculating hash of a packet in SW as it would be calculated in HW for the
> encap action
> 
> We should give the real full name of the flow action.
> 
> > +
> > +When the HW execute an encapsulation action, it may calculate an hash
> value which is based
> > +on the original packet. This hash is stored depending on the encapsulation
> protocol, in one
> > +of the outer fields.
> 
> Give an example of such encapsulation protocol?

Sure,
Just to be clear something like this?
When the HW execute an encapsulation action for example for VXLAN tunnel,, it may ...

> 
> > +This function allows the application to calculate the hash for a given
> packet as if the
> > +encapsulation was done in HW.
> > +
> > +.. code-block:: c
> > +
> > +   int
> > +   rte_flow_calc_encap_hash(uint16_t port_id,
> > +                            const struct rte_flow_item pattern[],
> > +			                   enum rte_flow_encap_hash_field
> dest_field,
> > +                            uint8_t hash_len,
> > +                            uint8_t *hash,
> > +			                   struct rte_flow_error *error);
> 
> I don't think we should add the complete prototype in this guide.

Agree, will remove.

> 
> [...]
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Simulates HW hash calculation that is done when encap action is being
> used.
> 
> s/Simulates/Simulate/

Will fix.
> 
> > + *
> > + * @param[in] port_id
> > + *   Port identifier of Ethernet device.
> > + * @param[in] pattern
> > + *   The values to be used in the hash calculation.
> > + * @param[in] dest_field
> > + *   Type of destination field for hash calculation.
> > + * @param[in] hash_len
> > + *   The length of the hash pointer in bytes. Should be according to
> encap_hash_field.
> > + * @param[out] hash
> > + *   Used to return the calculated hash. It will be written in network order,
> > + *   so hash[0] is the MSB.
> > + *   The number of bytes is based on the destination field type.
> > + * @param[out] error
> > + *   Perform verbose error reporting if not NULL.
> > + *   PMDs initialize this structure in case of error only.
> > + *
> > + * @return
> > + *   - (0) if success.
> > + *   - (-ENODEV) if *port_id* invalid.
> > + *   - (-ENOTSUP) if underlying device does not support this functionality.
> > + *   - (-EINVAL) if *pattern* doesn't hold enough information to calculate
> the hash
> > + *               or the dest is not supported.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_calc_encap_hash(uint16_t port_id, const struct rte_flow_item
> pattern[],
> > +			 enum rte_flow_encap_hash_field dest_field, uint8_t
> hash_len,
> > +			 uint8_t *hash, struct rte_flow_error *error);
> 
>