| Message ID | 20220123172046.1296964-1-aman.deep.singh@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | [v2] devtools/cocci: update cocci for ethdev namespace | expand |
| Context | Check | Description |
|---|---|---|
| ci/iol-aarch64-compile-testing | success | Testing PASS |
| ci/iol-x86_64-compile-testing | success | Testing PASS |
| ci/iol-aarch64-unit-testing | success | Testing PASS |
| ci/iol-x86_64-unit-testing | success | Testing PASS |
| ci/iol-abi-testing | success | Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/iol-intel-Functional | success | Functional Testing PASS |
| ci/iol-broadcom-Functional | success | Functional Testing PASS |
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/github-robot: build | success | github build: passed |
| ci/iol-broadcom-Performance | success | Performance Testing PASS |
| ci/intel-Testing | success | Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | warning | coding style issues |
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?
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 > >
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.
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. > > >
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.
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. > >
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?
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. > > >
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.
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. > > >
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;
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(-)