| Message ID | 20221205172627.44943-3-shannon.nelson@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | devlink: add params FW_BANK and ENABLE_MIGRATION | expand |
Mon, Dec 05, 2022 at 06:26:27PM CET, shannon.nelson@amd.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@amd.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 >
On 12/6/22 1:04 AM, Jiri Pirko wrote: > Mon, Dec 05, 2022 at 06:26:27PM CET, shannon.nelson@amd.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@amd.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 >>
Tue, Dec 06, 2022 at 07:28:33PM CET, shannon.nelson@amd.com wrote: >On 12/6/22 1:04 AM, Jiri Pirko wrote: >> Mon, Dec 05, 2022 at 06:26:27PM CET, shannon.nelson@amd.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@amd.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 >> >
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. 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)
To go along with existing enable_eth, enable_roce, enable_vnet, etc., we add an enable_migration parameter. 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@amd.com> --- Documentation/networking/devlink/devlink-params.rst | 4 ++++ include/net/devlink.h | 4 ++++ net/core/devlink.c | 5 +++++ 3 files changed, 13 insertions(+)