netdev - Re: [PATCH net-next 2/2] devlink: add enable

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]

Date:   Tue, 6 Dec 2022 10:28:33 -0800
From:   Shannon Nelson <shannon.nelson@....com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        jiri@...dia.com
Subject: Re: [PATCH net-next 2/2] devlink: add enable_migration parameter

On 12/6/22 1:04 AM, Jiri Pirko wrote:
> Mon, Dec 05, 2022 at 06:26:27PM CET, shannon.nelson@....com wrote:
>> To go along with existing enable_eth, enable_roce,
>> enable_vnet, etc., we add an enable_migration parameter.
> 
> In the patch description, you should be alwyas imperative to the
> codebase. Tell it what to do, don't describe what you (plural) do :)

This will be better described when rolled up in the pds_core patchset.

> 
> 
>>
>> This follows from the discussion of this RFC patch
>> https://lore.kernel.org/netdev/20221118225656.48309-11-snelson@pensando.io/
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
>> ---
>> Documentation/networking/devlink/devlink-params.rst | 4 ++++
>> include/net/devlink.h                               | 4 ++++
>> net/core/devlink.c                                  | 5 +++++
>> 3 files changed, 13 insertions(+)
>>
>> diff --git a/Documentation/networking/devlink/devlink-params.rst b/Documentation/networking/devlink/devlink-params.rst
>> index ed62c8a92f17..c56caad32a7c 100644
>> --- a/Documentation/networking/devlink/devlink-params.rst
>> +++ b/Documentation/networking/devlink/devlink-params.rst
>> @@ -141,3 +141,7 @@ own name.
>>       - u8
>>       - In a multi-bank flash device, select the FW memory bank to be
>>         loaded from on the next device boot/reset.
>> +   * - ``enable_migration``
>> +     - Boolean
>> +     - When enabled, the device driver will instantiate a live migration
>> +       specific auxiliary device of the devlink device.
> 
> Devlink has not notion of auxdev. Use objects and terms relevant to
> devlink please.
> 
> I don't really understand what is the semantics of this param at all.

Perhaps we need to update the existing descriptions for enable_eth, 
enable_vnet, etc, as well?  Probably none of them should mention the aux 
device, tho' I know they all came in together after a long discussion.

I'll work this to be more generic to the result and not the underlying 
specifics of how.

sln

> 
> 
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index 8a1430196980..1d35056a558d 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -511,6 +511,7 @@ enum devlink_param_generic_id {
>>        DEVLINK_PARAM_GENERIC_ID_IO_EQ_SIZE,
>>        DEVLINK_PARAM_GENERIC_ID_EVENT_EQ_SIZE,
>>        DEVLINK_PARAM_GENERIC_ID_FW_BANK,
>> +      DEVLINK_PARAM_GENERIC_ID_ENABLE_MIGRATION,
>>
>>        /* add new param generic ids above here*/
>>        __DEVLINK_PARAM_GENERIC_ID_MAX,
>> @@ -572,6 +573,9 @@ enum devlink_param_generic_id {
>> #define DEVLINK_PARAM_GENERIC_FW_BANK_NAME "fw_bank"
>> #define DEVLINK_PARAM_GENERIC_FW_BANK_TYPE DEVLINK_PARAM_TYPE_U8
>>
>> +#define DEVLINK_PARAM_GENERIC_ENABLE_MIGRATION_NAME "enable_migration"
>> +#define DEVLINK_PARAM_GENERIC_ENABLE_MIGRATION_TYPE DEVLINK_PARAM_TYPE_BOOL
>> +
>> #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate)    \
>> {                                                                     \
>>        .id = DEVLINK_PARAM_GENERIC_ID_##_id,                           \
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 6872d678be5b..0e32a4fe7a66 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -5236,6 +5236,11 @@ static const struct devlink_param devlink_param_generic[] = {
>>                .name = DEVLINK_PARAM_GENERIC_FW_BANK_NAME,
>>                .type = DEVLINK_PARAM_GENERIC_FW_BANK_TYPE,
>>        },
>> +      {
>> +              .id = DEVLINK_PARAM_GENERIC_ID_ENABLE_MIGRATION,
>> +              .name = DEVLINK_PARAM_GENERIC_ENABLE_MIGRATION_NAME,
>> +              .type = DEVLINK_PARAM_GENERIC_ENABLE_MIGRATION_TYPE,
>> +      },
>> };
>>
>> static int devlink_param_generic_verify(const struct devlink_param *param)
>> --
>> 2.17.1
>>