[dpdk-dev] [PATCH v5 12/14] librte_ethdev: add ESP and AH flow types to RSS

[Andrew Rybchenko]

On 1/15/20 1:44 PM, Ferruh Yigit wrote:
> On 1/15/2020 9:13 AM, Andrew Rybchenko wrote:
>> On 1/14/20 9:45 PM, Ferruh Yigit wrote:
>>> On 1/14/2020 1:55 PM, Bernard Iremonger wrote:
>>>> Add macros for the following protocols in the DDP esp-ah profile:
>>>> ESP
>>>> AH
>>>>
>>>> Add the following RSS macro for IPsec:
>>>> ETH_RSS_IPSEC
>>>>
>>>> Signed-off-by: Bernard Iremonger <bernard.iremonger at intel.com>
>>> +Ori and other ethdev maintainers.
>>>
>>> Ori, can you please check this patch?
>>>
>>>> ---
>>>>  lib/librte_ethdev/rte_ethdev.h | 14 +++++++++++++-
>>>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>>>> index 18a9def..208ec90 100644
>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>> @@ -484,7 +484,9 @@ struct rte_eth_rss_conf {
>>>>  #define RTE_ETH_FLOW_NVGRE              21 /**< NVGRE protocol based flow */
>>>>  #define RTE_ETH_FLOW_VXLAN_GPE          22 /**< VXLAN-GPE protocol based flow */
>>>>  #define RTE_ETH_FLOW_GTPU               23 /**< GTPU protocol based flow */
>>>> -#define RTE_ETH_FLOW_MAX                24
>>>> +#define RTE_ETH_FLOW_AH                 24 /**< AH protocol based flow */
>>>> +#define RTE_ETH_FLOW_ESP                25 /**< ESP protocol based flow */
>>>> +#define RTE_ETH_FLOW_MAX                26
>> Isn't changing RTE_ETH_FLOW_MAX value breaking ABI?
>> Is v20.11 target release of the patch?
> I can't see how this can cause an ABI break, unless the 'RTE_ETH_FLOW_MAX' value
> used as size of an array in the middle of a struct.
> There is 'struct rte_eth_fdir_flex_conf' but the array is at the end there, so
> it should be OK, unless that struct is not in the middle of another struct.

rte_eth_fdir_flex_conf -> rte_fdir_conf -> rte_eth_conf (in the middle)

> And there are values calculated from 'RTE_ETH_FLOW_MAX', like
> 'RTE_FLOW_MASK_ARRAY_SIZE', same concern applies there, it very hard to follow.
>
> Bernard,
>
> Can you please run the ABI version script to be sure this is not breaking the ABI?
>
>
>>>>  
>>>>  /*
>>>>   * Below macros are defined for RSS offload types, they can be used to
>>>> @@ -511,6 +513,12 @@ struct rte_eth_rss_conf {
>>>>  #define ETH_RSS_GENEVE             (1ULL << 20)
>>>>  #define ETH_RSS_NVGRE              (1ULL << 21)
>>>>  #define ETH_RSS_GTPU               (1ULL << 23)
>>>> +#define ETH_RSS_AH                 (1ULL << 24)
>>>> +#define ETH_RSS_ESP                (1ULL << 25)
>>>> +
>>>> +
>>>> +
>>>> +
>>>>  
>>>>  /*
>>>>   * We use the following macros to combine with above ETH_RSS_* for
>>>> @@ -571,6 +579,10 @@ rte_eth_rss_hf_refine(uint64_t rss_hf)
>>>>  	ETH_RSS_NONFRAG_IPV4_SCTP | \
>>>>  	ETH_RSS_NONFRAG_IPV6_SCTP)
>>>>  
>>>> +#define ETH_RSS_IPSEC ( \
>>>> +	ETH_RSS_AH | \
>>>> +	ETH_RSS_ESP)
>>>> +
>>>>  #define ETH_RSS_TUNNEL ( \
>>>>  	ETH_RSS_VXLAN  | \
>>>>  	ETH_RSS_GENEVE | \
>>>>