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

[Ferruh Yigit <ferruh.yigit@intel.com>]

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@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.

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 | \
>>>
>