[v5,12/14] librte_ethdev: add ESP and AH flow types to RSS
Checks
Commit Message
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>
---
lib/librte_ethdev/rte_ethdev.h | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
Comments
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
>
> /*
> * 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 | \
>
> -----Original Message-----
> From: Iremonger, Bernard <bernard.iremonger@intel.com>
> Sent: Tuesday, January 14, 2020 9:55 PM
> To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Doherty, Declan <declan.doherty@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Byrne, Stephen1
> <stephen1.byrne@intel.com>; Zhang, Helin <helin.zhang@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>
> Subject: [PATCH v5 12/14] librte_ethdev: add ESP and AH flow types to RSS
>
> 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>
> ---
> 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
>
> /*
> * 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)
> +
> +
> +
> +
Empty lines need to be removed
Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
>
> /*
> * 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 | \
> --
> 2.7.4
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?
>>
>> /*
>> * 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 | \
>>
Hi Qi,
<snip>
> > Subject: [PATCH v5 12/14] librte_ethdev: add ESP and AH flow types to
> > RSS
> >
> > 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>
> > ---
> > 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
> >
> > /*
> > * 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)
> > +
> > +
> > +
> > +
>
> Empty lines need to be removed
>
> Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
I will remove in v6 patch.
Can I carry forward your Reviewed-by: ?
> >
> > /*
> > * 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 | \
> > --
> > 2.7.4
>
Regards,
Bernard.
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 | \
>>>
>
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@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 | \
>>>>
On 1/15/2020 10:55 AM, Andrew Rybchenko wrote:
> 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@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)
Yes, this looks like an ABI break and this is very annoying not able to even add
a new RTE_FLOW type.
We need to find a proper way to handle this, at first glance I can see stop
using _MAX macros for the array size can work and perhaps we can use another big
enough hardcoded value for all similar array size. Any other option?
But we can do this on 20.11, we need a solution until that time.
>
>> 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 | \
>>>>>
>
Hi Ferruh,
<snip>
> Subject: Re: [dpdk-dev] [PATCH v5 12/14] librte_ethdev: add ESP and AH
> flow types to RSS
>
> On 1/15/2020 10:55 AM, Andrew Rybchenko wrote:
> > 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@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)
>
> Yes, this looks like an ABI break and this is very annoying not able to even add
> a new RTE_FLOW type.
>
> We need to find a proper way to handle this, at first glance I can see stop
> using _MAX macros for the array size can work and perhaps we can use
> another big enough hardcoded value for all similar array size. Any other
> option?
>
> But we can do this on 20.11, we need a solution until that time.
This patch is required to handle RSS for the esp-ah.pkg DDP profile, it does not affect the i40e FD and testpmd changes in this patch set.
The esp-ah.pkg DDP profile is not released yet.
Given that the merge deadline is today, I propose to drop this patch from the v6 patch set.
>
> >
> >> 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 | \
> >>>>>
> >
Regards,
Bernard.
On 1/15/2020 2:11 PM, Iremonger, Bernard wrote:
> Hi Ferruh,
>
> <snip>
>
>> Subject: Re: [dpdk-dev] [PATCH v5 12/14] librte_ethdev: add ESP and AH
>> flow types to RSS
>>
>> On 1/15/2020 10:55 AM, Andrew Rybchenko wrote:
>>> 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@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)
>>
>> Yes, this looks like an ABI break and this is very annoying not able to even add
>> a new RTE_FLOW type.
>>
>> We need to find a proper way to handle this, at first glance I can see stop
>> using _MAX macros for the array size can work and perhaps we can use
>> another big enough hardcoded value for all similar array size. Any other
>> option?
>>
>> But we can do this on 20.11, we need a solution until that time.
>
> This patch is required to handle RSS for the esp-ah.pkg DDP profile, it does not affect the i40e FD and testpmd changes in this patch set.
> The esp-ah.pkg DDP profile is not released yet.
>
> Given that the merge deadline is today, I propose to drop this patch from the v6 patch set.
+1, if it can be separated, better to do it to not block rest of the work.
>
>>
>>>
>>>> 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 | \
>>>>>>>
>>>
> Regards,
>
> Bernard.
>
@@ -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
/*
* 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 | \