netdev - Re: [PATCH net-next v3 01/11] devlink: prepare to support region operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f670654a-fd17-2dfa-dc58-b55f12fbed4b@intel.com>
Date:   Fri, 27 Mar 2020 13:58:03 -0700
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     tanhuazhong <tanhuazhong@...wei.com>, netdev@...r.kernel.org
Cc:     Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...nulli.us>,
        Jiri Pirko <jiri@...lanox.com>
Subject: Re: [PATCH net-next v3 01/11] devlink: prepare to support region
 operations



On 3/27/2020 1:16 AM, tanhuazhong wrote:
> 
> 
> On 2020/3/27 2:37, Jacob Keller wrote:
>> Modify the devlink region code in preparation for adding new operations
>> on regions.
>>
>> Create a devlink_region_ops structure, and move the name pointer from
>> within the devlink_region structure into the ops structure (similar to
>> the devlink_health_reporter_ops).
>>
>> This prepares the regions to enable support of additional operations in
>> the future such as requesting snapshots, or accessing the region
>> directly without a snapshot.
>>
>> In order to re-use the constant strings in the mlx4 driver their
>> declaration must be changed to 'const char * const' to ensure the
>> compiler realizes that both the data and the pointer cannot change.
>>
>> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
>> Reviewed-by: Jakub Kicinski <kuba@...nel.org>
>> Reviewed-by: Jiri Pirko <jiri@...lanox.com>
>> ---
>> Changes since RFC
>> * Picked up Jiri's Reviewed-by
>>
>>   drivers/net/ethernet/mellanox/mlx4/crdump.c | 16 +++++++++++----
>>   drivers/net/netdevsim/dev.c                 |  6 +++++-
>>   include/net/devlink.h                       | 16 +++++++++++----
>>   net/core/devlink.c                          | 22 ++++++++++-----------
>>   4 files changed, 40 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/crdump.c b/drivers/net/ethernet/mellanox/mlx4/crdump.c
>> index 64ed725aec28..cc2bf596c74b 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/crdump.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/crdump.c
>> @@ -38,8 +38,16 @@
>>   #define CR_ENABLE_BIT_OFFSET		0xF3F04
>>   #define MAX_NUM_OF_DUMPS_TO_STORE	(8)
>>   
>> -static const char *region_cr_space_str = "cr-space";
>> -static const char *region_fw_health_str = "fw-health";
>> +static const char * const region_cr_space_str = "cr-space";
>> +static const char * const region_fw_health_str = "fw-health";
>> +
>> +static const struct devlink_region_ops region_cr_space_ops = {
>> +	.name = region_cr_space_str,
>> +};
>> +
>> +static const struct devlink_region_ops region_fw_health_ops = {
>> +	.name = region_fw_health_str,
>> +};
>>  
> 
> 
> Hi, Jacob.
> 
> After pull net-next, I get below compiler errors:
> drivers/net/ethernet/mellanox//mlx4/crdump.c:45:10: error: initializer 
> element is not constant
>    .name = region_cr_space_str,
>            ^
> drivers/net/ethernet/mellanox//mlx4/crdump.c:45:10: note: (near 
> initialization for ‘region_cr_space_ops.name’)
> drivers/net/ethernet/mellanox//mlx4/crdump.c:50:10: error: initializer 
> element is not constant
>    .name = region_fw_health_str,
> 
> It seems the value of variables region_cr_space_str and 
> region_cr_space_str is unknown during compiling phase.
> 
> Huazhong.
> 

Based on some searching, this appears to be something where newer
versions of GCC and clang can recognize that "const char * const" is
safe to use as a constant expression, but older compilers cannot.

I'm going to submit a fix that uses a macro so that we avoid this
breakage on slightly older compilers.

Thanks,
Jake