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:04:46 +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

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 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.


>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
>