[v2] devtools/cocci: update cocci for ethdev namespace

Message ID 20220123172046.1296964-1-aman.deep.singh@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] devtools/cocci: update cocci for ethdev namespace |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Singh, Aman Deep Jan. 23, 2022, 5:20 p.m. UTC
  Added two specific exceptions for ETH_SPEED_10G
and ETH_SPEED_25G to avoid there name change.
Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA

Signed-off-by: Aman Singh <aman.deep.singh@intel.com>
---
 devtools/cocci/namespace_ethdev.cocci | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Thomas Monjalon Feb. 2, 2022, 9:01 p.m. UTC | #1
23/01/2022 18:20, Aman Singh:
> Added two specific exceptions for ETH_SPEED_10G
> and ETH_SPEED_25G to avoid there name change.
> Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA

Please could you explain why?
  
Singh, Aman Deep Feb. 4, 2022, 6:13 a.m. UTC | #2
Hi Thomas

On 2/3/2022 2:31 AM, Thomas Monjalon wrote:
> 23/01/2022 18:20, Aman Singh:
>> Added two specific exceptions for ETH_SPEED_10G
>> and ETH_SPEED_25G to avoid there name change.
>> Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA
> Please could you explain why?
These two macro's ETH_SPEED_10G & ETH_SPEED_25G are used by ifpga
driver and script should no change these.
There are multiple ETH_SPEED_NUM_xxx macro that need to be changed
to RTE_ETH_SPEED_NUM_xxx. So added above two as specific exceptions.


The other two patterns ETH_TUNNEL_FILTER & ETH_RSS_RETA were missed before.
The script should change these to RTE_ETH_TUNNEL_FILTER & RTE_ETH_RSS_RETA
>
>
  
Thomas Monjalon Feb. 4, 2022, 7:47 a.m. UTC | #3
04/02/2022 07:13, Singh, Aman Deep:
> Hi Thomas
> 
> On 2/3/2022 2:31 AM, Thomas Monjalon wrote:
> > 23/01/2022 18:20, Aman Singh:
> >> Added two specific exceptions for ETH_SPEED_10G
> >> and ETH_SPEED_25G to avoid there name change.
> >> Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA
> > Please could you explain why?
> These two macro's ETH_SPEED_10G & ETH_SPEED_25G are used by ifpga
> driver and script should no change these.
> There are multiple ETH_SPEED_NUM_xxx macro that need to be changed
> to RTE_ETH_SPEED_NUM_xxx. So added above two as specific exceptions.

Why doing this exception? What is special with ifpga?

> The other two patterns ETH_TUNNEL_FILTER & ETH_RSS_RETA were missed before.
> The script should change these to RTE_ETH_TUNNEL_FILTER & RTE_ETH_RSS_RETA

The explanations should be part of the commit log please.
  
Singh, Aman Deep Feb. 10, 2022, 1:26 p.m. UTC | #4
On 2/4/2022 1:17 PM, Thomas Monjalon wrote:
> 04/02/2022 07:13, Singh, Aman Deep:
>> Hi Thomas
>>
>> On 2/3/2022 2:31 AM, Thomas Monjalon wrote:
>>> 23/01/2022 18:20, Aman Singh:
>>>> Added two specific exceptions for ETH_SPEED_10G
>>>> and ETH_SPEED_25G to avoid there name change.
>>>> Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA
>>> Please could you explain why?
>> These two macro's ETH_SPEED_10G & ETH_SPEED_25G are used by ifpga
>> driver and script should no change these.
>> There are multiple ETH_SPEED_NUM_xxx macro that need to be changed
>> to RTE_ETH_SPEED_NUM_xxx. So added above two as specific exceptions.
> Why doing this exception? What is special with ifpga?

These two macro's are defined in 'ifpga/base/opae_eth_group.h'
we don't intend to change these. Target is ethdev namespace only.

>
>> The other two patterns ETH_TUNNEL_FILTER & ETH_RSS_RETA were missed before.
>> The script should change these to RTE_ETH_TUNNEL_FILTER & RTE_ETH_RSS_RETA
> The explanations should be part of the commit log please.

Sure, if this explanation is fine? Will update in next version.

>
>
>
  
Thomas Monjalon Feb. 10, 2022, 3:30 p.m. UTC | #5
10/02/2022 14:26, Singh, Aman Deep:
> On 2/4/2022 1:17 PM, Thomas Monjalon wrote:
> > 04/02/2022 07:13, Singh, Aman Deep:
> >> Hi Thomas
> >>
> >> On 2/3/2022 2:31 AM, Thomas Monjalon wrote:
> >>> 23/01/2022 18:20, Aman Singh:
> >>>> Added two specific exceptions for ETH_SPEED_10G
> >>>> and ETH_SPEED_25G to avoid there name change.
> >>>> Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA
> >>> Please could you explain why?
> >> These two macro's ETH_SPEED_10G & ETH_SPEED_25G are used by ifpga
> >> driver and script should no change these.
> >> There are multiple ETH_SPEED_NUM_xxx macro that need to be changed
> >> to RTE_ETH_SPEED_NUM_xxx. So added above two as specific exceptions.
> > Why doing this exception? What is special with ifpga?
> 
> These two macro's are defined in 'ifpga/base/opae_eth_group.h'
> we don't intend to change these. Target is ethdev namespace only.

So we will miss future use of a deprecated macro
because ifpga is redefining it?
I think it is a wrong approach.
We should not make any exception in the check.
Instead we can just ignore the warning for ifpga.

> >> The other two patterns ETH_TUNNEL_FILTER & ETH_RSS_RETA were missed before.
> >> The script should change these to RTE_ETH_TUNNEL_FILTER & RTE_ETH_RSS_RETA
> > The explanations should be part of the commit log please.
> 
> Sure, if this explanation is fine? Will update in next version.

Yes thanks.
  
Singh, Aman Deep Feb. 11, 2022, 8:07 a.m. UTC | #6
On 2/10/2022 9:00 PM, Thomas Monjalon wrote:
> 10/02/2022 14:26, Singh, Aman Deep:
>> On 2/4/2022 1:17 PM, Thomas Monjalon wrote:
>>> 04/02/2022 07:13, Singh, Aman Deep:
>>>> Hi Thomas
>>>>
>>>> On 2/3/2022 2:31 AM, Thomas Monjalon wrote:
>>>>> 23/01/2022 18:20, Aman Singh:
>>>>>> Added two specific exceptions for ETH_SPEED_10G
>>>>>> and ETH_SPEED_25G to avoid there name change.
>>>>>> Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA
>>>>> Please could you explain why?
>>>> These two macro's ETH_SPEED_10G & ETH_SPEED_25G are used by ifpga
>>>> driver and script should no change these.
>>>> There are multiple ETH_SPEED_NUM_xxx macro that need to be changed
>>>> to RTE_ETH_SPEED_NUM_xxx. So added above two as specific exceptions.
>>> Why doing this exception? What is special with ifpga?
>> These two macro's are defined in 'ifpga/base/opae_eth_group.h'
>> we don't intend to change these. Target is ethdev namespace only.
> So we will miss future use of a deprecated macro
> because ifpga is redefining it?
> I think it is a wrong approach.
> We should not make any exception in the check.
> Instead we can just ignore the warning for ifpga.

Actually ifpga is not redefining these two macro's ETH_SPEED_10G & ETH_SPEED_25G,
they are unique to it. Only there prefix, matches with ethdev macro's
ETH_SPEED_NUM_xxx, which caused coccinelle script to modify these to
RTE_ETH_SPEED_10G & RTE_ETH_SPEED_25G. So just avoiding it by this change.

>
>>>> The other two patterns ETH_TUNNEL_FILTER & ETH_RSS_RETA were missed before.
>>>> The script should change these to RTE_ETH_TUNNEL_FILTER & RTE_ETH_RSS_RETA
>>> The explanations should be part of the commit log please.
>> Sure, if this explanation is fine? Will update in next version.
> Yes thanks.
>
>
  
Thomas Monjalon Feb. 11, 2022, 5:28 p.m. UTC | #7
11/02/2022 09:07, Singh, Aman Deep:
> On 2/10/2022 9:00 PM, Thomas Monjalon wrote:
> > 10/02/2022 14:26, Singh, Aman Deep:
> >> On 2/4/2022 1:17 PM, Thomas Monjalon wrote:
> >>> 04/02/2022 07:13, Singh, Aman Deep:
> >>>> Hi Thomas
> >>>>
> >>>> On 2/3/2022 2:31 AM, Thomas Monjalon wrote:
> >>>>> 23/01/2022 18:20, Aman Singh:
> >>>>>> Added two specific exceptions for ETH_SPEED_10G
> >>>>>> and ETH_SPEED_25G to avoid there name change.
> >>>>>> Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA
> >>>>> Please could you explain why?
> >>>> These two macro's ETH_SPEED_10G & ETH_SPEED_25G are used by ifpga
> >>>> driver and script should no change these.
> >>>> There are multiple ETH_SPEED_NUM_xxx macro that need to be changed
> >>>> to RTE_ETH_SPEED_NUM_xxx. So added above two as specific exceptions.
> >>> Why doing this exception? What is special with ifpga?
> >> These two macro's are defined in 'ifpga/base/opae_eth_group.h'
> >> we don't intend to change these. Target is ethdev namespace only.
> > So we will miss future use of a deprecated macro
> > because ifpga is redefining it?
> > I think it is a wrong approach.
> > We should not make any exception in the check.
> > Instead we can just ignore the warning for ifpga.
> 
> Actually ifpga is not redefining these two macro's ETH_SPEED_10G & ETH_SPEED_25G,
> they are unique to it. Only there prefix, matches with ethdev macro's
> ETH_SPEED_NUM_xxx, which caused coccinelle script to modify these to
> RTE_ETH_SPEED_10G & RTE_ETH_SPEED_25G. So just avoiding it by this change.

Would it work to restrict the match to ETH_SPEED_NUM?
  
Singh, Aman Deep Feb. 15, 2022, 6:51 p.m. UTC | #8
On 2/11/2022 10:58 PM, Thomas Monjalon wrote:
> 11/02/2022 09:07, Singh, Aman Deep:
>> On 2/10/2022 9:00 PM, Thomas Monjalon wrote:
>>> 10/02/2022 14:26, Singh, Aman Deep:
>>>> On 2/4/2022 1:17 PM, Thomas Monjalon wrote:
>>>>> 04/02/2022 07:13, Singh, Aman Deep:
>>>>>> Hi Thomas
>>>>>>
>>>>>> On 2/3/2022 2:31 AM, Thomas Monjalon wrote:
>>>>>>> 23/01/2022 18:20, Aman Singh:
>>>>>>>> Added two specific exceptions for ETH_SPEED_10G
>>>>>>>> and ETH_SPEED_25G to avoid there name change.
>>>>>>>> Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA
>>>>>>> Please could you explain why?
>>>>>> These two macro's ETH_SPEED_10G & ETH_SPEED_25G are used by ifpga
>>>>>> driver and script should no change these.
>>>>>> There are multiple ETH_SPEED_NUM_xxx macro that need to be changed
>>>>>> to RTE_ETH_SPEED_NUM_xxx. So added above two as specific exceptions.
>>>>> Why doing this exception? What is special with ifpga?
>>>> These two macro's are defined in 'ifpga/base/opae_eth_group.h'
>>>> we don't intend to change these. Target is ethdev namespace only.
>>> So we will miss future use of a deprecated macro
>>> because ifpga is redefining it?
>>> I think it is a wrong approach.
>>> We should not make any exception in the check.
>>> Instead we can just ignore the warning for ifpga.
>> Actually ifpga is not redefining these two macro's ETH_SPEED_10G & ETH_SPEED_25G,
>> they are unique to it. Only there prefix, matches with ethdev macro's
>> ETH_SPEED_NUM_xxx, which caused coccinelle script to modify these to
>> RTE_ETH_SPEED_10G & RTE_ETH_SPEED_25G. So just avoiding it by this change.
> Would it work to restrict the match to ETH_SPEED_NUM?

The script will change ETH_SPEED_NUM_xxx macros to RTE_ETH_SPEED_NUM_xxx
as per the requirement.

>
>
>
  
Thomas Monjalon Feb. 15, 2022, 10:17 p.m. UTC | #9
15/02/2022 19:51, Singh, Aman Deep:
> On 2/11/2022 10:58 PM, Thomas Monjalon wrote:
> > 11/02/2022 09:07, Singh, Aman Deep:
> >> On 2/10/2022 9:00 PM, Thomas Monjalon wrote:
> >>> 10/02/2022 14:26, Singh, Aman Deep:
> >>>> On 2/4/2022 1:17 PM, Thomas Monjalon wrote:
> >>>>> 04/02/2022 07:13, Singh, Aman Deep:
> >>>>>> Hi Thomas
> >>>>>>
> >>>>>> On 2/3/2022 2:31 AM, Thomas Monjalon wrote:
> >>>>>>> 23/01/2022 18:20, Aman Singh:
> >>>>>>>> Added two specific exceptions for ETH_SPEED_10G
> >>>>>>>> and ETH_SPEED_25G to avoid there name change.
> >>>>>>>> Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA
> >>>>>>> Please could you explain why?
> >>>>>> These two macro's ETH_SPEED_10G & ETH_SPEED_25G are used by ifpga
> >>>>>> driver and script should no change these.
> >>>>>> There are multiple ETH_SPEED_NUM_xxx macro that need to be changed
> >>>>>> to RTE_ETH_SPEED_NUM_xxx. So added above two as specific exceptions.
> >>>>> Why doing this exception? What is special with ifpga?
> >>>> These two macro's are defined in 'ifpga/base/opae_eth_group.h'
> >>>> we don't intend to change these. Target is ethdev namespace only.
> >>> So we will miss future use of a deprecated macro
> >>> because ifpga is redefining it?
> >>> I think it is a wrong approach.
> >>> We should not make any exception in the check.
> >>> Instead we can just ignore the warning for ifpga.
> >> Actually ifpga is not redefining these two macro's ETH_SPEED_10G & ETH_SPEED_25G,
> >> they are unique to it. Only there prefix, matches with ethdev macro's
> >> ETH_SPEED_NUM_xxx, which caused coccinelle script to modify these to
> >> RTE_ETH_SPEED_10G & RTE_ETH_SPEED_25G. So just avoiding it by this change.
> > Would it work to restrict the match to ETH_SPEED_NUM?
> 
> The script will change ETH_SPEED_NUM_xxx macros to RTE_ETH_SPEED_NUM_xxx
> as per the requirement.

No there is a misunderstanding.
I am asking to filter on ETH_SPEED_NUM instead of ETH_SPEED with exceptions.
  
Singh, Aman Deep Feb. 17, 2022, 6:34 a.m. UTC | #10
On 2/16/2022 3:47 AM, Thomas Monjalon wrote:
> 15/02/2022 19:51, Singh, Aman Deep:
>> On 2/11/2022 10:58 PM, Thomas Monjalon wrote:
>>> 11/02/2022 09:07, Singh, Aman Deep:
>>>> On 2/10/2022 9:00 PM, Thomas Monjalon wrote:
>>>>> 10/02/2022 14:26, Singh, Aman Deep:
>>>>>> On 2/4/2022 1:17 PM, Thomas Monjalon wrote:
>>>>>>> 04/02/2022 07:13, Singh, Aman Deep:
>>>>>>>> Hi Thomas
>>>>>>>>
>>>>>>>> On 2/3/2022 2:31 AM, Thomas Monjalon wrote:
>>>>>>>>> 23/01/2022 18:20, Aman Singh:
>>>>>>>>>> Added two specific exceptions for ETH_SPEED_10G
>>>>>>>>>> and ETH_SPEED_25G to avoid there name change.
>>>>>>>>>> Added check for ETH_TUNNEL_FILTER and ETH_RSS_RETA
>>>>>>>>> Please could you explain why?
>>>>>>>> These two macro's ETH_SPEED_10G & ETH_SPEED_25G are used by ifpga
>>>>>>>> driver and script should no change these.
>>>>>>>> There are multiple ETH_SPEED_NUM_xxx macro that need to be changed
>>>>>>>> to RTE_ETH_SPEED_NUM_xxx. So added above two as specific exceptions.
>>>>>>> Why doing this exception? What is special with ifpga?
>>>>>> These two macro's are defined in 'ifpga/base/opae_eth_group.h'
>>>>>> we don't intend to change these. Target is ethdev namespace only.
>>>>> So we will miss future use of a deprecated macro
>>>>> because ifpga is redefining it?
>>>>> I think it is a wrong approach.
>>>>> We should not make any exception in the check.
>>>>> Instead we can just ignore the warning for ifpga.
>>>> Actually ifpga is not redefining these two macro's ETH_SPEED_10G & ETH_SPEED_25G,
>>>> they are unique to it. Only there prefix, matches with ethdev macro's
>>>> ETH_SPEED_NUM_xxx, which caused coccinelle script to modify these to
>>>> RTE_ETH_SPEED_10G & RTE_ETH_SPEED_25G. So just avoiding it by this change.
>>> Would it work to restrict the match to ETH_SPEED_NUM?
>> The script will change ETH_SPEED_NUM_xxx macros to RTE_ETH_SPEED_NUM_xxx
>> as per the requirement.
> No there is a misunderstanding.
> I am asking to filter on ETH_SPEED_NUM instead of ETH_SPEED with exceptions.
Yes, can do it this way. I will upload new patch with this change.
>
>
>
  

Patch

diff --git a/devtools/cocci/namespace_ethdev.cocci b/devtools/cocci/namespace_ethdev.cocci
index d5de41e117..50a1d296f3 100644
--- a/devtools/cocci/namespace_ethdev.cocci
+++ b/devtools/cocci/namespace_ethdev.cocci
@@ -1,7 +1,7 @@ 
 @rule1@
 identifier I =~  "^(RTE_FC_|ETH_MQ_|ETH_RSS|DEV_RX_|DEV_TX_|ETH_LINK|RTE_RETA|
 |ETH_DCB|RTE_TUNNEL|ETH_VLAN|ETH_4|ETH_8|ETH_16|ETH_32|ETH_64|RTE_FDIR|RTE_L2|
-|ETH_SPEED|ETH_MIRROR|ETH_VMDQ|ETH_NUM|ETH_QINQ|ETH_MAX_)";
+|ETH_SPEED|ETH_TUNNEL_FILT|ETH_RSS_RETA_|ETH_VMDQ|ETH_NUM|ETH_QINQ|ETH_MAX_)";
 @@
 I
 
@@ -16,7 +16,8 @@  exception_matches = ["ETH_VLAN_FILTER_CLASSIFY","ETH_VLAN_FILTER_ANY",
 "RTE_FDIR_NO","RTE_FDIR_REPORT","ETH_MAX_RX_CLIENTS_E1H",
 "ETH_MAX_AGGREGATION_QUEUES_E1","ETH_RSS_ENGINE_NUM","ETH_NUM_MAC_FILTERS",
 "ETH_MAX_NUM_RX_QUEUES_PER_VF_QUAD","ETH_RSS_IND_TABLE_ENTRIES_NUM",
-"ETH_RSS_KEY_SIZE_REGS","ETH_NUM_STATISTIC_COUNTERS","ETH_SPEED_"]
+"ETH_RSS_KEY_SIZE_REGS","ETH_NUM_STATISTIC_COUNTERS","ETH_SPEED_10G",
+"ETH_SPEED_25G"]
 
 if any(x in I for x in exception_matches):
         coccinelle .J= I;