[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y5CWGgXUwPGBzfWY@nanopsycho>
Date: Wed, 7 Dec 2022 14:33:14 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Shannon Nelson <shannon.nelson@....com>
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
Tue, Dec 06, 2022 at 07:28:33PM CET, shannon.nelson@....com wrote:
>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.
Yep, I think so.
>
>I'll work this to be more generic to the result and not the underlying
>specifics of how.
Thanks!
>
>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
>> >
Powered by blists - more mailing lists