[v2,1/4] ethdev: introduce encap hash calculation

Message ID 20240208090919.11565-1-orika@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v2,1/4] ethdev: introduce encap hash calculation |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation warning apply patch failure
ci/iol-testing warning apply patch failure

Commit Message

Ori Kam Feb. 8, 2024, 9:09 a.m. UTC
  During encapsulation of a packet, it is possible to change some
outer headers to improve flow destribution.
For example, from VXLAN RFC:
"It is recommended that the UDP source port number
be calculated using a hash of fields from the inner packet --
one example being a hash of the inner Ethernet frame's headers.
This is to enable a level of entropy for the ECMP/load-balancing"

The tunnel protocol defines which outer field should hold this hash,
but it doesn't define the hash calculation algorithm.

An application that uses flow offloads gets the first few packets
(exception path) 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.

the new function rte_flow_calc_encap_hash can query the hash value
fromm the driver for a given packet as if the packet was passed
through the HW.

Signed-off-by: Ori Kam <orika@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst     | 14 +++++++
 doc/guides/rel_notes/release_24_03.rst |  6 ++-
 lib/ethdev/rte_flow.c                  | 25 +++++++++++++
 lib/ethdev/rte_flow.h                  | 51 ++++++++++++++++++++++++++
 lib/ethdev/rte_flow_driver.h           |  5 +++
 lib/ethdev/version.map                 |  1 +
 6 files changed, 101 insertions(+), 1 deletion(-)
  

Comments

Ferruh Yigit Feb. 8, 2024, 5:13 p.m. UTC | #1
On 2/8/2024 9:09 AM, Ori Kam wrote:
> During encapsulation of a packet, it is possible to change some
> outer headers to improve flow destribution.
> For example, from VXLAN RFC:
> "It is recommended that the UDP source port number
> be calculated using a hash of fields from the inner packet --
> one example being a hash of the inner Ethernet frame's headers.
> This is to enable a level of entropy for the ECMP/load-balancing"
> 
> The tunnel protocol defines which outer field should hold this hash,
> but it doesn't define the hash calculation algorithm.
> 
> An application that uses flow offloads gets the first few packets
> (exception path) 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.
> 
> the new function rte_flow_calc_encap_hash can query the hash value
> fromm the driver for a given packet as if the packet was passed
> through the HW.
> 
> Signed-off-by: Ori Kam <orika@nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> 

<...>

> +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)
> +{
> +	int ret;
> +	struct rte_eth_dev *dev;
> +	const struct rte_flow_ops *ops;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	ops = rte_flow_ops_get(port_id, error);
> +	if (!ops || !ops->flow_calc_encap_hash)
> +		return rte_flow_error_set(error, ENOTSUP,
> +					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> +					  "calc encap hash is not supported");
> +	if ((dest_field == RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT && hash_len != 2) ||
> +	    (dest_field == RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID && hash_len != 1))
>

If there is a fixed mapping with the dest_field and the size, instead of
putting this information into check code, what do you think to put it
into the data structure?

I mean instead of using enum for dest_filed, it can be a struct that is
holding enum and its expected size, this clarifies what the expected
size for that field.

> +		return rte_flow_error_set(error, EINVAL,
> +					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> +					  "hash len doesn't match the requested field len");
> +	dev = &rte_eth_devices[port_id];
> +	ret = ops->flow_calc_encap_hash(dev, pattern, dest_field, hash, error);
>

'hash_len' is get by API, but it is not passed to dev_ops, does this
mean this information hardcoded in the driver as well, if so why
duplicate this information in driver instead off passing hash_len to driver?


> +	return flow_err(port_id, ret, error);
> +}
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 1267c146e5..2bdf3a4a17 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -6783,6 +6783,57 @@ rte_flow_calc_table_hash(uint16_t port_id, const struct rte_flow_template_table
>  			 const struct rte_flow_item pattern[], uint8_t pattern_template_index,
>  			 uint32_t *hash, struct rte_flow_error *error);
>  
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Destination field type for the hash calculation, when encap action is used.
> + *
> + * @see function rte_flow_calc_encap_hash
> + */
> +enum rte_flow_encap_hash_field {
> +	/* Calculate hash placed in UDP source port field. */
> +	RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT,
> +	/* Calculate hash placed in NVGRE flow ID field. */
> +	RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID,
> +};
>

Indeed above enum represents a field in a network protocol, right?
Instead of having a 'RTE_FLOW_ENCAP_HASH_' specific one, can re-using
'enum rte_flow_field_id' work?
  
Ori Kam Feb. 11, 2024, 7:29 a.m. UTC | #2
Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Thursday, February 8, 2024 7:13 PM
> To: Ori Kam <orika@nvidia.com>; Dariusz Sosnowski
> 
> On 2/8/2024 9:09 AM, Ori Kam wrote:
> > During encapsulation of a packet, it is possible to change some
> > outer headers to improve flow destribution.
> > For example, from VXLAN RFC:
> > "It is recommended that the UDP source port number
> > be calculated using a hash of fields from the inner packet --
> > one example being a hash of the inner Ethernet frame's headers.
> > This is to enable a level of entropy for the ECMP/load-balancing"
> >
> > The tunnel protocol defines which outer field should hold this hash,
> > but it doesn't define the hash calculation algorithm.
> >
> > An application that uses flow offloads gets the first few packets
> > (exception path) 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.
> >
> > the new function rte_flow_calc_encap_hash can query the hash value
> > fromm the driver for a given packet as if the packet was passed
> > through the HW.
> >
> > Signed-off-by: Ori Kam <orika@nvidia.com>
> > Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> >
> 
> <...>
> 
> > +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)
> > +{
> > +	int ret;
> > +	struct rte_eth_dev *dev;
> > +	const struct rte_flow_ops *ops;
> > +
> > +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > +	ops = rte_flow_ops_get(port_id, error);
> > +	if (!ops || !ops->flow_calc_encap_hash)
> > +		return rte_flow_error_set(error, ENOTSUP,
> > +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> > +					  "calc encap hash is not supported");
> > +	if ((dest_field == RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT &&
> hash_len != 2) ||
> > +	    (dest_field == RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID
> && hash_len != 1))
> >
> 
> If there is a fixed mapping with the dest_field and the size, instead of
> putting this information into check code, what do you think to put it
> into the data structure?
> 
> I mean instead of using enum for dest_filed, it can be a struct that is
> holding enum and its expected size, this clarifies what the expected
> size for that field.
> 

From my original email I think we only need the type, we don't need the size.
On the RFC thread there was an objection. So I added the size, 
If you think it is not needed lets remove it.

> > +		return rte_flow_error_set(error, EINVAL,
> > +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> > +					  "hash len doesn't match the
> requested field len");
> > +	dev = &rte_eth_devices[port_id];
> > +	ret = ops->flow_calc_encap_hash(dev, pattern, dest_field, hash,
> error);
> >
> 
> 'hash_len' is get by API, but it is not passed to dev_ops, does this
> mean this information hardcoded in the driver as well, if so why
> duplicate this information in driver instead off passing hash_len to driver?

Not sure I understand, like I wrote above this is pure verification from my point of view.
The driver knows the size based on the dest.

> 
> 
> > +	return flow_err(port_id, ret, error);
> > +}
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index 1267c146e5..2bdf3a4a17 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -6783,6 +6783,57 @@ rte_flow_calc_table_hash(uint16_t port_id,
> const struct rte_flow_template_table
> >  			 const struct rte_flow_item pattern[], uint8_t
> pattern_template_index,
> >  			 uint32_t *hash, struct rte_flow_error *error);
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Destination field type for the hash calculation, when encap action is
> used.
> > + *
> > + * @see function rte_flow_calc_encap_hash
> > + */
> > +enum rte_flow_encap_hash_field {
> > +	/* Calculate hash placed in UDP source port field. */
> > +	RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT,
> > +	/* Calculate hash placed in NVGRE flow ID field. */
> > +	RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID,
> > +};
> >
> 
> Indeed above enum represents a field in a network protocol, right?
> Instead of having a 'RTE_FLOW_ENCAP_HASH_' specific one, can re-using
> 'enum rte_flow_field_id' work?

Since the option are really limited and defined by standard, I prefer to have dedicated options. 


Best,
Ori
  
Ferruh Yigit Feb. 12, 2024, 5:05 p.m. UTC | #3
On 2/11/2024 7:29 AM, Ori Kam wrote:
> Hi Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Thursday, February 8, 2024 7:13 PM
>> To: Ori Kam <orika@nvidia.com>; Dariusz Sosnowski
>>
>> On 2/8/2024 9:09 AM, Ori Kam wrote:
>>> During encapsulation of a packet, it is possible to change some
>>> outer headers to improve flow destribution.
>>> For example, from VXLAN RFC:
>>> "It is recommended that the UDP source port number
>>> be calculated using a hash of fields from the inner packet --
>>> one example being a hash of the inner Ethernet frame's headers.
>>> This is to enable a level of entropy for the ECMP/load-balancing"
>>>
>>> The tunnel protocol defines which outer field should hold this hash,
>>> but it doesn't define the hash calculation algorithm.
>>>
>>> An application that uses flow offloads gets the first few packets
>>> (exception path) 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.
>>>
>>> the new function rte_flow_calc_encap_hash can query the hash value
>>> fromm the driver for a given packet as if the packet was passed
>>> through the HW.
>>>
>>> Signed-off-by: Ori Kam <orika@nvidia.com>
>>> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
>>>
>>
>> <...>
>>
>>> +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)
>>> +{
>>> +	int ret;
>>> +	struct rte_eth_dev *dev;
>>> +	const struct rte_flow_ops *ops;
>>> +
>>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>> +	ops = rte_flow_ops_get(port_id, error);
>>> +	if (!ops || !ops->flow_calc_encap_hash)
>>> +		return rte_flow_error_set(error, ENOTSUP,
>>> +
>> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
>>> +					  "calc encap hash is not supported");
>>> +	if ((dest_field == RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT &&
>> hash_len != 2) ||
>>> +	    (dest_field == RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID
>> && hash_len != 1))
>>>
>>
>> If there is a fixed mapping with the dest_field and the size, instead of
>> putting this information into check code, what do you think to put it
>> into the data structure?
>>
>> I mean instead of using enum for dest_filed, it can be a struct that is
>> holding enum and its expected size, this clarifies what the expected
>> size for that field.
>>
> 
> From my original email I think we only need the type, we don't need the size.
> On the RFC thread there was an objection. So I added the size, 
> If you think it is not needed lets remove it.
> 

I am not saying length is not needed, but
API gets 'dest_field' & 'hash_len', and according checks in the API for
each 'dest_field' there is an exact 'hash_len' requirement, this
requirement is something impacts user but this information is embedded
in the API, my suggestion is make it more visible to user.

My initial suggestion was put this into an object, like:
```
struct x {
	enum rte_flow_encap_hash_field dest_field;
	size_t expected size;
} y[] = {
	{ RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT, 2 },
	{ RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID, 1 }
};
```

But as you mentioned this is a limited set, perhaps it is sufficient to
document size requirement in the "enum rte_flow_encap_hash_field" API
doxygen comment.



>>> +		return rte_flow_error_set(error, EINVAL,
>>> +
>> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
>>> +					  "hash len doesn't match the
>> requested field len");
>>> +	dev = &rte_eth_devices[port_id];
>>> +	ret = ops->flow_calc_encap_hash(dev, pattern, dest_field, hash,
>> error);
>>>
>>
>> 'hash_len' is get by API, but it is not passed to dev_ops, does this
>> mean this information hardcoded in the driver as well, if so why
>> duplicate this information in driver instead off passing hash_len to driver?
> 
> Not sure I understand, like I wrote above this is pure verification from my point of view.
> The driver knows the size based on the dest.
> 

My intention was similar to above comment, like dest_field type
RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT implies that required size should be
2 bytes, and it seems driver already knows about this requirement.

Instead, it can be possible to verify 'hash_len' in the API level, pass
this information to the driver and driver use 'hash_len' directly for
its size parameter, so driver will rely on API provided 'hash_len' value
instead of storing this information within driver.

Lets assume 10 drivers are implementing this feature, should all of them
define MLX5DR_CRC_ENCAP_ENTROPY_HASH_SIZE_16 equivalent enum/define
withing the driver?

>>
>>
>>> +	return flow_err(port_id, ret, error);
>>> +}
>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>>> index 1267c146e5..2bdf3a4a17 100644
>>> --- a/lib/ethdev/rte_flow.h
>>> +++ b/lib/ethdev/rte_flow.h
>>> @@ -6783,6 +6783,57 @@ rte_flow_calc_table_hash(uint16_t port_id,
>> const struct rte_flow_template_table
>>>  			 const struct rte_flow_item pattern[], uint8_t
>> pattern_template_index,
>>>  			 uint32_t *hash, struct rte_flow_error *error);
>>>
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Destination field type for the hash calculation, when encap action is
>> used.
>>> + *
>>> + * @see function rte_flow_calc_encap_hash
>>> + */
>>> +enum rte_flow_encap_hash_field {
>>> +	/* Calculate hash placed in UDP source port field. */
>>>

Just recognized that comments are not doxygen comments.

>>> +	RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT,
>>> +	/* Calculate hash placed in NVGRE flow ID field. */
>>> +	RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID,
>>> +};
>>>
>>
>> Indeed above enum represents a field in a network protocol, right?
>> Instead of having a 'RTE_FLOW_ENCAP_HASH_' specific one, can re-using
>> 'enum rte_flow_field_id' work?
> 
> Since the option are really limited and defined by standard, I prefer to have dedicated options. 
> 

OK, my intention is to reduce the duplication. Just for brainstorm, what
is the benefit of having 'RTE_FLOW_ENCAP_HASH_' specific enums, if we
can present them as generic protocol fiels, like
'RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT' vs 'RTE_FLOW_FIELD_UDP_PORT_SRC,'?
  
Ori Kam Feb. 12, 2024, 6:44 p.m. UTC | #4
Hi Ferruh

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Monday, February 12, 2024 7:05 PM
> 
> On 2/11/2024 7:29 AM, Ori Kam wrote:
> > Hi Ferruh,
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@amd.com>
> >> Sent: Thursday, February 8, 2024 7:13 PM
> >> To: Ori Kam <orika@nvidia.com>; Dariusz Sosnowski
> >>
> >> On 2/8/2024 9:09 AM, Ori Kam wrote:
> >>> During encapsulation of a packet, it is possible to change some
> >>> outer headers to improve flow destribution.
> >>> For example, from VXLAN RFC:
> >>> "It is recommended that the UDP source port number
> >>> be calculated using a hash of fields from the inner packet --
> >>> one example being a hash of the inner Ethernet frame's headers.
> >>> This is to enable a level of entropy for the ECMP/load-balancing"
> >>>
> >>> The tunnel protocol defines which outer field should hold this hash,
> >>> but it doesn't define the hash calculation algorithm.
> >>>
> >>> An application that uses flow offloads gets the first few packets
> >>> (exception path) 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.
> >>>
> >>> the new function rte_flow_calc_encap_hash can query the hash value
> >>> fromm the driver for a given packet as if the packet was passed
> >>> through the HW.
> >>>
> >>> Signed-off-by: Ori Kam <orika@nvidia.com>
> >>> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> >>>
> >>
> >> <...>
> >>
> >>> +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)
> >>> +{
> >>> +	int ret;
> >>> +	struct rte_eth_dev *dev;
> >>> +	const struct rte_flow_ops *ops;
> >>> +
> >>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> >>> +	ops = rte_flow_ops_get(port_id, error);
> >>> +	if (!ops || !ops->flow_calc_encap_hash)
> >>> +		return rte_flow_error_set(error, ENOTSUP,
> >>> +
> >> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> >>> +					  "calc encap hash is not supported");
> >>> +	if ((dest_field == RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT &&
> >> hash_len != 2) ||
> >>> +	    (dest_field == RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID
> >> && hash_len != 1))
> >>>
> >>
> >> If there is a fixed mapping with the dest_field and the size, instead of
> >> putting this information into check code, what do you think to put it
> >> into the data structure?
> >>
> >> I mean instead of using enum for dest_filed, it can be a struct that is
> >> holding enum and its expected size, this clarifies what the expected
> >> size for that field.
> >>
> >
> > From my original email I think we only need the type, we don't need the
> size.
> > On the RFC thread there was an objection. So I added the size,
> > If you think it is not needed lets remove it.
> >
> 
> I am not saying length is not needed, but
> API gets 'dest_field' & 'hash_len', and according checks in the API for
> each 'dest_field' there is an exact 'hash_len' requirement, this
> requirement is something impacts user but this information is embedded
> in the API, my suggestion is make it more visible to user.
> 
> My initial suggestion was put this into an object, like:
> ```
> struct x {
> 	enum rte_flow_encap_hash_field dest_field;
> 	size_t expected size;
> } y[] = {
> 	{ RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT, 2 },
> 	{ RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID, 1 }
> };
> ```
> 
> But as you mentioned this is a limited set, perhaps it is sufficient to
> document size requirement in the "enum rte_flow_encap_hash_field" API
> doxygen comment.

Will add it to the doxygen.

> 
> 
> 
> >>> +		return rte_flow_error_set(error, EINVAL,
> >>> +
> >> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> >>> +					  "hash len doesn't match the
> >> requested field len");
> >>> +	dev = &rte_eth_devices[port_id];
> >>> +	ret = ops->flow_calc_encap_hash(dev, pattern, dest_field, hash,
> >> error);
> >>>
> >>
> >> 'hash_len' is get by API, but it is not passed to dev_ops, does this
> >> mean this information hardcoded in the driver as well, if so why
> >> duplicate this information in driver instead off passing hash_len to driver?
> >
> > Not sure I understand, like I wrote above this is pure verification from my
> point of view.
> > The driver knows the size based on the dest.
> >
> 
> My intention was similar to above comment, like dest_field type
> RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT implies that required size should
> be
> 2 bytes, and it seems driver already knows about this requirement.

That is correct, that is why I don't think we need the size, add added it
only for validation due to community request.

> 
> Instead, it can be possible to verify 'hash_len' in the API level, pass
> this information to the driver and driver use 'hash_len' directly for
> its size parameter, so driver will rely on API provided 'hash_len' value
> instead of storing this information within driver.
> 
> Lets assume 10 drivers are implementing this feature, should all of them
> define MLX5DR_CRC_ENCAP_ENTROPY_HASH_SIZE_16 equivalent
> enum/define
> withing the driver?

No, the driver implements hard-coded logic, which means that it just needs to know
the dest field, in order to know what hash to calculate
It is possible that for each field the HW will calculate the hash using different algorithm.

Also it is possible that the HW doesn't support writing to the expected field, in which case we 
want the driver call to fail.

Field implies size.
Size doesn't implies field.

> 
> >>
> >>
> >>> +	return flow_err(port_id, ret, error);
> >>> +}
> >>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> >>> index 1267c146e5..2bdf3a4a17 100644
> >>> --- a/lib/ethdev/rte_flow.h
> >>> +++ b/lib/ethdev/rte_flow.h
> >>> @@ -6783,6 +6783,57 @@ rte_flow_calc_table_hash(uint16_t port_id,
> >> const struct rte_flow_template_table
> >>>  			 const struct rte_flow_item pattern[], uint8_t
> >> pattern_template_index,
> >>>  			 uint32_t *hash, struct rte_flow_error *error);
> >>>
> >>> +/**
> >>> + * @warning
> >>> + * @b EXPERIMENTAL: this API may change without prior notice.
> >>> + *
> >>> + * Destination field type for the hash calculation, when encap action is
> >> used.
> >>> + *
> >>> + * @see function rte_flow_calc_encap_hash
> >>> + */
> >>> +enum rte_flow_encap_hash_field {
> >>> +	/* Calculate hash placed in UDP source port field. */
> >>>
> 
> Just recognized that comments are not doxygen comments.

Thanks,
Will fix.
> 
> >>> +	RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT,
> >>> +	/* Calculate hash placed in NVGRE flow ID field. */
> >>> +	RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID,
> >>> +};
> >>>
> >>
> >> Indeed above enum represents a field in a network protocol, right?
> >> Instead of having a 'RTE_FLOW_ENCAP_HASH_' specific one, can re-using
> >> 'enum rte_flow_field_id' work?
> >
> > Since the option are really limited and defined by standard, I prefer to have
> dedicated options.
> >
> 
> OK, my intention is to reduce the duplication. Just for brainstorm, what
> is the benefit of having 'RTE_FLOW_ENCAP_HASH_' specific enums, if we
> can present them as generic protocol fiels, like
> 'RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT' vs
> 'RTE_FLOW_FIELD_UDP_PORT_SRC,'?

I guess you want to go with 'RTE_FLOW_FIELD_UDP_PORT_SRC
right?
The main issue is since the options are really limited and used for a very dedicated function.
When app developers / DPDK developers will look at it, it will be very unclear what is the use of this enum.
We already have an enum for fields. Like you suggested we could have used it,
but this will show much more option than there are really.


Best,
Ori
  
Ferruh Yigit Feb. 12, 2024, 8:09 p.m. UTC | #5
On 2/12/2024 6:44 PM, Ori Kam wrote:
> Hi Ferruh
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Monday, February 12, 2024 7:05 PM
>>
>> On 2/11/2024 7:29 AM, Ori Kam wrote:
>>> Hi Ferruh,
>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>>>> Sent: Thursday, February 8, 2024 7:13 PM
>>>> To: Ori Kam <orika@nvidia.com>; Dariusz Sosnowski
>>>>
>>>> On 2/8/2024 9:09 AM, Ori Kam wrote:
>>>>> During encapsulation of a packet, it is possible to change some
>>>>> outer headers to improve flow destribution.
>>>>> For example, from VXLAN RFC:
>>>>> "It is recommended that the UDP source port number
>>>>> be calculated using a hash of fields from the inner packet --
>>>>> one example being a hash of the inner Ethernet frame's headers.
>>>>> This is to enable a level of entropy for the ECMP/load-balancing"
>>>>>
>>>>> The tunnel protocol defines which outer field should hold this hash,
>>>>> but it doesn't define the hash calculation algorithm.
>>>>>
>>>>> An application that uses flow offloads gets the first few packets
>>>>> (exception path) 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.
>>>>>
>>>>> the new function rte_flow_calc_encap_hash can query the hash value
>>>>> fromm the driver for a given packet as if the packet was passed
>>>>> through the HW.
>>>>>
>>>>> Signed-off-by: Ori Kam <orika@nvidia.com>
>>>>> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
>>>>>
>>>>
>>>> <...>
>>>>
>>>>> +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)
>>>>> +{
>>>>> +	int ret;
>>>>> +	struct rte_eth_dev *dev;
>>>>> +	const struct rte_flow_ops *ops;
>>>>> +
>>>>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>>>> +	ops = rte_flow_ops_get(port_id, error);
>>>>> +	if (!ops || !ops->flow_calc_encap_hash)
>>>>> +		return rte_flow_error_set(error, ENOTSUP,
>>>>> +
>>>> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
>>>>> +					  "calc encap hash is not supported");
>>>>> +	if ((dest_field == RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT &&
>>>> hash_len != 2) ||
>>>>> +	    (dest_field == RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID
>>>> && hash_len != 1))
>>>>>
>>>>
>>>> If there is a fixed mapping with the dest_field and the size, instead of
>>>> putting this information into check code, what do you think to put it
>>>> into the data structure?
>>>>
>>>> I mean instead of using enum for dest_filed, it can be a struct that is
>>>> holding enum and its expected size, this clarifies what the expected
>>>> size for that field.
>>>>
>>>
>>> From my original email I think we only need the type, we don't need the
>> size.
>>> On the RFC thread there was an objection. So I added the size,
>>> If you think it is not needed lets remove it.
>>>
>>
>> I am not saying length is not needed, but
>> API gets 'dest_field' & 'hash_len', and according checks in the API for
>> each 'dest_field' there is an exact 'hash_len' requirement, this
>> requirement is something impacts user but this information is embedded
>> in the API, my suggestion is make it more visible to user.
>>
>> My initial suggestion was put this into an object, like:
>> ```
>> struct x {
>> 	enum rte_flow_encap_hash_field dest_field;
>> 	size_t expected size;
>> } y[] = {
>> 	{ RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT, 2 },
>> 	{ RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID, 1 }
>> };
>> ```
>>
>> But as you mentioned this is a limited set, perhaps it is sufficient to
>> document size requirement in the "enum rte_flow_encap_hash_field" API
>> doxygen comment.
> 
> Will add it to the doxygen.
> 
>>
>>
>>
>>>>> +		return rte_flow_error_set(error, EINVAL,
>>>>> +
>>>> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
>>>>> +					  "hash len doesn't match the
>>>> requested field len");
>>>>> +	dev = &rte_eth_devices[port_id];
>>>>> +	ret = ops->flow_calc_encap_hash(dev, pattern, dest_field, hash,
>>>> error);
>>>>>
>>>>
>>>> 'hash_len' is get by API, but it is not passed to dev_ops, does this
>>>> mean this information hardcoded in the driver as well, if so why
>>>> duplicate this information in driver instead off passing hash_len to driver?
>>>
>>> Not sure I understand, like I wrote above this is pure verification from my
>> point of view.
>>> The driver knows the size based on the dest.
>>>
>>
>> My intention was similar to above comment, like dest_field type
>> RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT implies that required size should
>> be
>> 2 bytes, and it seems driver already knows about this requirement.
> 
> That is correct, that is why I don't think we need the size, add added it
> only for validation due to community request.
> 
>>
>> Instead, it can be possible to verify 'hash_len' in the API level, pass
>> this information to the driver and driver use 'hash_len' directly for
>> its size parameter, so driver will rely on API provided 'hash_len' value
>> instead of storing this information within driver.
>>
>> Lets assume 10 drivers are implementing this feature, should all of them
>> define MLX5DR_CRC_ENCAP_ENTROPY_HASH_SIZE_16 equivalent
>> enum/define
>> withing the driver?
> 
> No, the driver implements hard-coded logic, which means that it just needs to know
> the dest field, in order to know what hash to calculate
> It is possible that for each field the HW will calculate the hash using different algorithm.
> 

OK if HW already needs to know the size in advance, lets go with enum
doxygen update only.

> Also it is possible that the HW doesn't support writing to the expected field, in which case we 
> want the driver call to fail.
> 
> Field implies size.
> Size doesn't implies field.
> 
>>
>>>>
>>>>
>>>>> +	return flow_err(port_id, ret, error);
>>>>> +}
>>>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>>>>> index 1267c146e5..2bdf3a4a17 100644
>>>>> --- a/lib/ethdev/rte_flow.h
>>>>> +++ b/lib/ethdev/rte_flow.h
>>>>> @@ -6783,6 +6783,57 @@ rte_flow_calc_table_hash(uint16_t port_id,
>>>> const struct rte_flow_template_table
>>>>>  			 const struct rte_flow_item pattern[], uint8_t
>>>> pattern_template_index,
>>>>>  			 uint32_t *hash, struct rte_flow_error *error);
>>>>>
>>>>> +/**
>>>>> + * @warning
>>>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>>>> + *
>>>>> + * Destination field type for the hash calculation, when encap action is
>>>> used.
>>>>> + *
>>>>> + * @see function rte_flow_calc_encap_hash
>>>>> + */
>>>>> +enum rte_flow_encap_hash_field {
>>>>> +	/* Calculate hash placed in UDP source port field. */
>>>>>
>>
>> Just recognized that comments are not doxygen comments.
> 
> Thanks,
> Will fix.
>>
>>>>> +	RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT,
>>>>> +	/* Calculate hash placed in NVGRE flow ID field. */
>>>>> +	RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID,
>>>>> +};
>>>>>
>>>>
>>>> Indeed above enum represents a field in a network protocol, right?
>>>> Instead of having a 'RTE_FLOW_ENCAP_HASH_' specific one, can re-using
>>>> 'enum rte_flow_field_id' work?
>>>
>>> Since the option are really limited and defined by standard, I prefer to have
>> dedicated options.
>>>
>>
>> OK, my intention is to reduce the duplication. Just for brainstorm, what
>> is the benefit of having 'RTE_FLOW_ENCAP_HASH_' specific enums, if we
>> can present them as generic protocol fiels, like
>> 'RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT' vs
>> 'RTE_FLOW_FIELD_UDP_PORT_SRC,'?
> 
> I guess you want to go with 'RTE_FLOW_FIELD_UDP_PORT_SRC
> right?
>

I just want to discuss if redundancy can be eliminated.

> The main issue is since the options are really limited and used for a very dedicated function.
> When app developers / DPDK developers will look at it, it will be very unclear what is the use of this enum.
> We already have an enum for fields. Like you suggested we could have used it,
> but this will show much more option than there are really.
> 

OK, lets use dedicated enums to clarify to the users the specific fields
available for this set of APIs.

Btw, is boundary check like following required for the APIs:
```
if (dest_field > RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID)
	return -EINVAL;
```
In case user pass an invalid value as 'dest_filed'

(Note: I intentionally not used MAX enum something like
'RTE_FLOW_ENCAP_HASH_FIELD_MAX' to not need to deal with ABI issues in
the future.)
  
Ori Kam Feb. 13, 2024, 7:05 a.m. UTC | #6
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Monday, February 12, 2024 10:10 PM
> 
> On 2/12/2024 6:44 PM, Ori Kam wrote:
> > Hi Ferruh
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@amd.com>
> >> Sent: Monday, February 12, 2024 7:05 PM
> >>
> >> On 2/11/2024 7:29 AM, Ori Kam wrote:
> >>> Hi Ferruh,
> >>>
> >>>> -----Original Message-----
> >>>> From: Ferruh Yigit <ferruh.yigit@amd.com>
> >>>> Sent: Thursday, February 8, 2024 7:13 PM
> >>>> To: Ori Kam <orika@nvidia.com>; Dariusz Sosnowski
> >>>>
> >>>> On 2/8/2024 9:09 AM, Ori Kam wrote:
> >>>>> During encapsulation of a packet, it is possible to change some
> >>>>> outer headers to improve flow destribution.
> >>>>> For example, from VXLAN RFC:
> >>>>> "It is recommended that the UDP source port number
> >>>>> be calculated using a hash of fields from the inner packet --
> >>>>> one example being a hash of the inner Ethernet frame's headers.
> >>>>> This is to enable a level of entropy for the ECMP/load-balancing"
> >>>>>
> >>>>> The tunnel protocol defines which outer field should hold this hash,
> >>>>> but it doesn't define the hash calculation algorithm.
> >>>>>
> >>>>> An application that uses flow offloads gets the first few packets
> >>>>> (exception path) 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.
> >>>>>
> >>>>> the new function rte_flow_calc_encap_hash can query the hash value
> >>>>> fromm the driver for a given packet as if the packet was passed
> >>>>> through the HW.
> >>>>>
> >>>>> Signed-off-by: Ori Kam <orika@nvidia.com>
> >>>>> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> >>>>>
> >>>>
> >>>> <...>
> >>>>
> >>>>> +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)
> >>>>> +{
> >>>>> +	int ret;
> >>>>> +	struct rte_eth_dev *dev;
> >>>>> +	const struct rte_flow_ops *ops;
> >>>>> +
> >>>>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> >>>>> +	ops = rte_flow_ops_get(port_id, error);
> >>>>> +	if (!ops || !ops->flow_calc_encap_hash)
> >>>>> +		return rte_flow_error_set(error, ENOTSUP,
> >>>>> +
> >>>> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> >>>>> +					  "calc encap hash is not
> supported");
> >>>>> +	if ((dest_field == RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT
> &&
> >>>> hash_len != 2) ||
> >>>>> +	    (dest_field ==
> RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID
> >>>> && hash_len != 1))
> >>>>>
> >>>>
> >>>> If there is a fixed mapping with the dest_field and the size, instead of
> >>>> putting this information into check code, what do you think to put it
> >>>> into the data structure?
> >>>>
> >>>> I mean instead of using enum for dest_filed, it can be a struct that is
> >>>> holding enum and its expected size, this clarifies what the expected
> >>>> size for that field.
> >>>>
> >>>
> >>> From my original email I think we only need the type, we don't need the
> >> size.
> >>> On the RFC thread there was an objection. So I added the size,
> >>> If you think it is not needed lets remove it.
> >>>
> >>
> >> I am not saying length is not needed, but
> >> API gets 'dest_field' & 'hash_len', and according checks in the API for
> >> each 'dest_field' there is an exact 'hash_len' requirement, this
> >> requirement is something impacts user but this information is embedded
> >> in the API, my suggestion is make it more visible to user.
> >>
> >> My initial suggestion was put this into an object, like:
> >> ```
> >> struct x {
> >> 	enum rte_flow_encap_hash_field dest_field;
> >> 	size_t expected size;
> >> } y[] = {
> >> 	{ RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT, 2 },
> >> 	{ RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID, 1 }
> >> };
> >> ```
> >>
> >> But as you mentioned this is a limited set, perhaps it is sufficient to
> >> document size requirement in the "enum rte_flow_encap_hash_field" API
> >> doxygen comment.
> >
> > Will add it to the doxygen.
> >
> >>
> >>
> >>
> >>>>> +		return rte_flow_error_set(error, EINVAL,
> >>>>> +
> >>>> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> >>>>> +					  "hash len doesn't match the
> >>>> requested field len");
> >>>>> +	dev = &rte_eth_devices[port_id];
> >>>>> +	ret = ops->flow_calc_encap_hash(dev, pattern, dest_field,
> hash,
> >>>> error);
> >>>>>
> >>>>
> >>>> 'hash_len' is get by API, but it is not passed to dev_ops, does this
> >>>> mean this information hardcoded in the driver as well, if so why
> >>>> duplicate this information in driver instead off passing hash_len to
> driver?
> >>>
> >>> Not sure I understand, like I wrote above this is pure verification from my
> >> point of view.
> >>> The driver knows the size based on the dest.
> >>>
> >>
> >> My intention was similar to above comment, like dest_field type
> >> RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT implies that required size
> should
> >> be
> >> 2 bytes, and it seems driver already knows about this requirement.
> >
> > That is correct, that is why I don't think we need the size, add added it
> > only for validation due to community request.
> >
> >>
> >> Instead, it can be possible to verify 'hash_len' in the API level, pass
> >> this information to the driver and driver use 'hash_len' directly for
> >> its size parameter, so driver will rely on API provided 'hash_len' value
> >> instead of storing this information within driver.
> >>
> >> Lets assume 10 drivers are implementing this feature, should all of them
> >> define MLX5DR_CRC_ENCAP_ENTROPY_HASH_SIZE_16 equivalent
> >> enum/define
> >> withing the driver?
> >
> > No, the driver implements hard-coded logic, which means that it just needs
> to know
> > the dest field, in order to know what hash to calculate
> > It is possible that for each field the HW will calculate the hash using
> different algorithm.
> >
> 
> OK if HW already needs to know the size in advance, lets go with enum
> doxygen update only.
> 
> > Also it is possible that the HW doesn't support writing to the expected field,
> in which case we
> > want the driver call to fail.
> >
> > Field implies size.
> > Size doesn't implies field.
> >
> >>
> >>>>
> >>>>
> >>>>> +	return flow_err(port_id, ret, error);
> >>>>> +}
> >>>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> >>>>> index 1267c146e5..2bdf3a4a17 100644
> >>>>> --- a/lib/ethdev/rte_flow.h
> >>>>> +++ b/lib/ethdev/rte_flow.h
> >>>>> @@ -6783,6 +6783,57 @@ rte_flow_calc_table_hash(uint16_t
> port_id,
> >>>> const struct rte_flow_template_table
> >>>>>  			 const struct rte_flow_item pattern[], uint8_t
> >>>> pattern_template_index,
> >>>>>  			 uint32_t *hash, struct rte_flow_error *error);
> >>>>>
> >>>>> +/**
> >>>>> + * @warning
> >>>>> + * @b EXPERIMENTAL: this API may change without prior notice.
> >>>>> + *
> >>>>> + * Destination field type for the hash calculation, when encap action
> is
> >>>> used.
> >>>>> + *
> >>>>> + * @see function rte_flow_calc_encap_hash
> >>>>> + */
> >>>>> +enum rte_flow_encap_hash_field {
> >>>>> +	/* Calculate hash placed in UDP source port field. */
> >>>>>
> >>
> >> Just recognized that comments are not doxygen comments.
> >
> > Thanks,
> > Will fix.
> >>
> >>>>> +	RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT,
> >>>>> +	/* Calculate hash placed in NVGRE flow ID field. */
> >>>>> +	RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID,
> >>>>> +};
> >>>>>
> >>>>
> >>>> Indeed above enum represents a field in a network protocol, right?
> >>>> Instead of having a 'RTE_FLOW_ENCAP_HASH_' specific one, can re-
> using
> >>>> 'enum rte_flow_field_id' work?
> >>>
> >>> Since the option are really limited and defined by standard, I prefer to
> have
> >> dedicated options.
> >>>
> >>
> >> OK, my intention is to reduce the duplication. Just for brainstorm, what
> >> is the benefit of having 'RTE_FLOW_ENCAP_HASH_' specific enums, if we
> >> can present them as generic protocol fiels, like
> >> 'RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT' vs
> >> 'RTE_FLOW_FIELD_UDP_PORT_SRC,'?
> >
> > I guess you want to go with 'RTE_FLOW_FIELD_UDP_PORT_SRC
> > right?
> >
> 
> I just want to discuss if redundancy can be eliminated.
> 
> > The main issue is since the options are really limited and used for a very
> dedicated function.
> > When app developers / DPDK developers will look at it, it will be very
> unclear what is the use of this enum.
> > We already have an enum for fields. Like you suggested we could have
> used it,
> > but this will show much more option than there are really.
> >
> 
> OK, lets use dedicated enums to clarify to the users the specific fields
> available for this set of APIs.
> 
> Btw, is boundary check like following required for the APIs:
> ```
> if (dest_field > RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID)
> 	return -EINVAL;
> ```
> In case user pass an invalid value as 'dest_filed'
> 
> (Note: I intentionally not used MAX enum something like
> 'RTE_FLOW_ENCAP_HASH_FIELD_MAX' to not need to deal with ABI issues in
> the future.)
Good idea will add
Best,
Ori
  

Patch

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 900fdaefb6..d3491a3ac5 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -4211,6 +4211,20 @@  as it would be calculated in the HW.
                             uint8_t pattern_template_index,
 			                   uint32_t *hash, struct rte_flow_error *error);
 
+Calculate encapsulation hash
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Calculating hash of a packet as it would be calculated by the HW, when encapsulating
+a packet.
+
+When the HW execute an encapsulation action, for example VXLAN tunnel,
+it may calculate an hash of the packet to be encapsulated.
+This hash is stored in the outer header of the tunnel.
+This allow better spreading of traffic.
+
+This function can be used for packets of a flow that are not offloaded and
+pass through the SW instead of the HW, for example, SYN/FIN packets.
+
 .. _flow_isolated_mode:
 
 Flow isolated mode
diff --git a/doc/guides/rel_notes/release_24_03.rst b/doc/guides/rel_notes/release_24_03.rst
index 2b91217943..9508dda362 100644
--- a/doc/guides/rel_notes/release_24_03.rst
+++ b/doc/guides/rel_notes/release_24_03.rst
@@ -60,6 +60,11 @@  New Features
   * Added new function ``rte_eth_find_rss_algo`` to get RSS hash
     algorithm by its name.
 
+* **Added hash calculation of an encapsulated packet as done by the HW.**
+
+  Added function to calculate hash when doing tunnel encapsulation:
+  ``rte_flow_calc_encap_hash()``
+
 * **Added flow matching of random value.**
 
   * Added ``RTE_FLOW_ITEM_TYPE_RANDOM`` to match random value.
@@ -91,7 +96,6 @@  New Features
   * Added HW steering support for modify field ``RTE_FLOW_FIELD_GENEVE_OPT_CLASS`` flow action.
   * Added HW steering support for modify field ``RTE_FLOW_FIELD_GENEVE_OPT_DATA`` flow action.
 
-
 Removed Items
 -------------
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index 3f58d792f9..7fce754be1 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -2482,3 +2482,28 @@  rte_flow_calc_table_hash(uint16_t port_id, const struct rte_flow_template_table
 					hash, error);
 	return flow_err(port_id, ret, error);
 }
+
+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)
+{
+	int ret;
+	struct rte_eth_dev *dev;
+	const struct rte_flow_ops *ops;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	ops = rte_flow_ops_get(port_id, error);
+	if (!ops || !ops->flow_calc_encap_hash)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					  "calc encap hash is not supported");
+	if ((dest_field == RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT && hash_len != 2) ||
+	    (dest_field == RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID && hash_len != 1))
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					  "hash len doesn't match the requested field len");
+	dev = &rte_eth_devices[port_id];
+	ret = ops->flow_calc_encap_hash(dev, pattern, dest_field, hash, error);
+	return flow_err(port_id, ret, error);
+}
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 1267c146e5..2bdf3a4a17 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -6783,6 +6783,57 @@  rte_flow_calc_table_hash(uint16_t port_id, const struct rte_flow_template_table
 			 const struct rte_flow_item pattern[], uint8_t pattern_template_index,
 			 uint32_t *hash, struct rte_flow_error *error);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Destination field type for the hash calculation, when encap action is used.
+ *
+ * @see function rte_flow_calc_encap_hash
+ */
+enum rte_flow_encap_hash_field {
+	/* Calculate hash placed in UDP source port field. */
+	RTE_FLOW_ENCAP_HASH_FIELD_SRC_PORT,
+	/* Calculate hash placed in NVGRE flow ID field. */
+	RTE_FLOW_ENCAP_HASH_FIELD_NVGRE_FLOW_ID,
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Simulate HW hash calculation that is done when an encap action is being used.
+ * This hash can be stored in tunnel outer header to improve packet distribution.
+ *
+ * @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);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h
index f35f659503..447163655a 100644
--- a/lib/ethdev/rte_flow_driver.h
+++ b/lib/ethdev/rte_flow_driver.h
@@ -370,6 +370,11 @@  struct rte_flow_ops {
 		(struct rte_eth_dev *dev, const struct rte_flow_template_table *table,
 		 const struct rte_flow_item pattern[], uint8_t pattern_template_index,
 		 uint32_t *hash, struct rte_flow_error *error);
+	/** @see rte_flow_calc_encap_hash() */
+	int (*flow_calc_encap_hash)
+		(struct rte_eth_dev *dev, const struct rte_flow_item pattern[],
+		 enum rte_flow_encap_hash_field dest_field, uint8_t *hash,
+		 struct rte_flow_error *error);
 };
 
 /**
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index a050baab0f..360898d067 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -319,6 +319,7 @@  EXPERIMENTAL {
 
 	# added in 24.03
 	rte_eth_find_rss_algo;
+	rte_flow_calc_encap_hash;
 };
 
 INTERNAL {