Tue, Jun 30, 2020 at 05:15:18PM CEST, vasundhara-v.vo...@broadcom.com wrote: >On Tue, Jun 30, 2020 at 6:23 PM Jiri Pirko <j...@resnulli.us> wrote: >> >> Tue, Jun 30, 2020 at 01:34:06PM CEST, vasundhara-v.vo...@broadcom.com wrote: >> >Advanced NICs support live reset of some of the hardware >> >components, that resets the device immediately with all the >> >host drivers loaded. >> > >> >Add devlink reset subcommand to support live and deferred modes >> >of reset. It allows to reset the hardware components of the >> >entire device and supports the following fields: >> > >> >component: >> >---------- >> >1. MGMT : Management processor. >> >2. DMA : DMA engine. >> >3. RAM : RAM shared between multiple components. >> >4. AP : Application processor. >> >5. ROCE : RoCE management processor. >> >6. All : All possible components. >> > >> >Drivers are allowed to reset only a subset of requested components. >> >> I don't understand why would user ever want to do this. He does not care >> about some magic hw entities. He just expects the hw to work. I don't >> undestand the purpose of exposing something like this. Could you please >> explain in details? Thanks! >> >If a user requests multiple components and if the driver is only able >to honor a subset, the driver will return the components unset which >it is able to reset. For example, if a user requests MGMT, RAM and >ROCE components to be reset and driver resets only MGMT and ROCE. >Driver will unset only MGMT and ROCE bits and notifies the user that >RAM is not reset. > >This will be useful for drivers to reset only a subset of components >requested instead of returning error or silently doing only a subset >of components. > >Also, this will be helpful as user will not know the components >supported by different vendors.
Your reply does not seem to be related to my question :/ > >Thanks, >Vasundhara > >> >> > >> >width: >> >------ >> >1. single - single host. >> >2. multi - Multi host. >> > >> >mode: >> >----- >> >1. deferred - Reset will happen after unloading all the host drivers >> > on the device. This is be default reset type, if user >> > does not specify the type. >> >2. live - Reset will happen immediately with all host drivers loaded >> > in real time. If the live reset is not supported, driver >> > will return the error. >> > >> >This patch is a proposal in continuation to discussion to the >> >following thread: >> > >> >"[PATCH v3 net-next 0/6] bnxt_en: Add 'enable_live_dev_reset' and >> >'allow_live_dev_reset' generic devlink params." >> > >> >and here is the URL to the patch series: >> > >> >https://patchwork.ozlabs.org/project/netdev/list/?series=180426&state=* >> > >> >If the proposal looks good, I will re-send the whole patchset >> >including devlink changes and driver usage. >> > >> >Signed-off-by: Vasundhara Volam <vasundhara-v.vo...@broadcom.com> >> >Reviewed-by: Michael Chan <michael.c...@broadcom.com> >> >--- >> >v2: >> >- Switch RAM and AP component definitions. >> >- Remove IRQ, FILTER, OFFLOAD, MAC, PHY components as they are port >> >specific components. >> >- Rename function to host in width parameter. >> >--- >> > Documentation/networking/devlink/devlink-reset.rst | 50 +++++++++++++ >> > include/net/devlink.h | 2 + >> > include/uapi/linux/devlink.h | 46 ++++++++++++ >> > net/core/devlink.c | 85 >> > ++++++++++++++++++++++ >> > 4 files changed, 183 insertions(+) >> > create mode 100644 Documentation/networking/devlink/devlink-reset.rst >> > >> >diff --git a/Documentation/networking/devlink/devlink-reset.rst >> >b/Documentation/networking/devlink/devlink-reset.rst >> >new file mode 100644 >> >index 0000000..652800d >> >--- /dev/null >> >+++ b/Documentation/networking/devlink/devlink-reset.rst >> >@@ -0,0 +1,50 @@ >> >+.. SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> >+ >> >+.. _devlink_reset: >> >+ >> >+============= >> >+Devlink reset >> >+============= >> >+ >> >+The ``devlink-reset`` API allows reset the hardware components of the >> >device. After the reset, >> >+device loads the pending updated firmware image. >> >+Example use:: >> >+ >> >+ $ devlink dev reset pci/0000:05:00.0 components COMPONENTS >> >+ >> >+Note that user can mention multiple components. >> >+ >> >+================ >> >+Reset components >> >+================ >> >+ >> >+List of available components:: >> >+ >> >+``DEVLINK_RESET_COMP_MGMT`` - Management processor. >> >+``DEVLINK_RESET_COMP_DMA`` - DMA engine. >> >+``DEVLINK_RESET_COMP_RAM`` - RAM shared between multiple components. >> >+``DEVLINK_RESET_COMP_AP`` - Application processor. >> >+``DEVLINK_RESET_COMP_ROCE`` - RoCE management processor. >> >+``DEVLINK_RESET_COMP_ALL`` - All components. >> >+ >> >+=========== >> >+Reset width >> >+=========== >> >+ >> >+List of available widths:: >> >+ >> >+``DEVLINK_RESET_WIDTH_SINGLE`` - Device is used by single dedicated host. >> >+``DEVLINK_RESET_WIDTH_MULTI`` - Device is shared across multiple hosts. >> >+ >> >+Note that if user specifies DEVLINK_RESET_WIDTH_SINGLE in a multi-host >> >environment, driver returns >> >+error if it does not support resetting a single host. >> >+ >> >+=========== >> >+Reset modes >> >+=========== >> >+ >> >+List of available reset modes:: >> >+ >> >+``DEVLINK_RESET_MODE_DEFERRED`` - Reset happens after all host drivers >> >are unloaded on the device. >> >+``DEVLINK_RESET_MODE_LIVE`` - Reset happens immediately, with all >> >loaded host drivers in real >> >+ time. >> >diff --git a/include/net/devlink.h b/include/net/devlink.h >> >index 428f55f..a71c8f5 100644 >> >--- a/include/net/devlink.h >> >+++ b/include/net/devlink.h >> >@@ -1129,6 +1129,8 @@ struct devlink_ops { >> > int (*port_function_hw_addr_set)(struct devlink *devlink, struct >> > devlink_port *port, >> > const u8 *hw_addr, int hw_addr_len, >> > struct netlink_ext_ack *extack); >> >+ int (*reset)(struct devlink *devlink, u32 *components, u8 width, u8 >> >mode, >> >+ struct netlink_ext_ack *extack); >> > }; >> > >> > static inline void *devlink_priv(struct devlink *devlink) >> >diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h >> >index 87c83a8..6f32c00 100644 >> >--- a/include/uapi/linux/devlink.h >> >+++ b/include/uapi/linux/devlink.h >> >@@ -122,6 +122,9 @@ enum devlink_command { >> > DEVLINK_CMD_TRAP_POLICER_NEW, >> > DEVLINK_CMD_TRAP_POLICER_DEL, >> > >> >+ DEVLINK_CMD_RESET, >> >+ DEVLINK_CMD_RESET_STATUS, /* notification only */ >> >+ >> > /* add new commands above here */ >> > __DEVLINK_CMD_MAX, >> > DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1 >> >@@ -265,6 +268,44 @@ enum devlink_trap_type { >> > DEVLINK_TRAP_TYPE_CONTROL, >> > }; >> > >> >+/** >> >+ * enum devlink_reset_component - Reset components. >> >+ * @DEVLINK_RESET_COMP_MGMT: Management processor. >> >+ * @DEVLINK_RESET_COMP_DMA: DMA engine. >> >+ * @DEVLINK_RESET_COMP_RAM: RAM shared between multiple components. >> >+ * @DEVLINK_RESET_COMP_AP: Application processor. >> >+ * @DEVLINK_RESET_COMP_ROCE: RoCE management processor. >> >+ * @DEVLINK_RESET_COMP_ALL: All components. >> >+ */ >> >+enum devlink_reset_component { >> >+ DEVLINK_RESET_COMP_MGMT = (1 << 0), >> >+ DEVLINK_RESET_COMP_DMA = (1 << 1), >> >+ DEVLINK_RESET_COMP_RAM = (1 << 2), >> >+ DEVLINK_RESET_COMP_AP = (1 << 3), >> >+ DEVLINK_RESET_COMP_ROCE = (1 << 4), >> >+ DEVLINK_RESET_COMP_ALL = 0xffffffff, >> >+}; >> >+ >> >+/** >> >+ * enum devlink_reset_width - Number of hosts effected by reset. >> >+ * @DEVLINK_RESET_WIDTH_SINGLE: Device is used by single dedicated host. >> >+ * @DEVLINK_RESET_WIDTH_MULTI: Device is shared across multiple hosts. >> >+ */ >> >+enum devlink_reset_width { >> >+ DEVLINK_RESET_WIDTH_SINGLE = 0, >> >+ DEVLINK_RESET_WIDTH_MULTI = 1, >> >+}; >> >+ >> >+/** >> >+ * enum devlink_reset_mode - Modes of reset. >> >+ * @DEVLINK_RESET_MODE_DEFERRED: Reset will happen after host drivers are >> >unloaded. >> >+ * @DEVLINK_RESET_MODE_LIVE: All host drivers also will be reset without >> >reloading manually. >> >+ */ >> >+enum devlink_reset_mode { >> >+ DEVLINK_RESET_MODE_DEFERRED = 0, >> >+ DEVLINK_RESET_MODE_LIVE = 1, >> >+}; >> >+ >> > enum { >> > /* Trap can report input port as metadata */ >> > DEVLINK_ATTR_TRAP_METADATA_TYPE_IN_PORT, >> >@@ -455,6 +496,11 @@ enum devlink_attr { >> > >> > DEVLINK_ATTR_INFO_BOARD_SERIAL_NUMBER, /* string */ >> > >> >+ DEVLINK_ATTR_RESET_COMPONENTS, /* u32 */ >> >+ DEVLINK_ATTR_RESET_WIDTH, /* u8 */ >> >+ DEVLINK_ATTR_RESET_MODE, /* u8 */ >> >+ DEVLINK_ATTR_RESET_STATUS_MSG, /* string */ >> >+ >> > /* add new attributes above here, update the policy in devlink.c */ >> > >> > __DEVLINK_ATTR_MAX, >> >diff --git a/net/core/devlink.c b/net/core/devlink.c >> >index 6ae3680..c0eebc5 100644 >> >--- a/net/core/devlink.c >> >+++ b/net/core/devlink.c >> >@@ -6797,6 +6797,82 @@ static int >> >devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb, >> > return devlink_trap_policer_set(devlink, policer_item, info); >> > } >> > >> >+static int devlink_nl_reset_fill(struct sk_buff *msg, struct devlink >> >*devlink, >> >+ const char *status_msg, u32 components) >> >+{ >> >+ void *hdr; >> >+ >> >+ hdr = genlmsg_put(msg, 0, 0, &devlink_nl_family, 0, >> >DEVLINK_CMD_RESET_STATUS); >> >+ if (!hdr) >> >+ return -EMSGSIZE; >> >+ >> >+ if (devlink_nl_put_handle(msg, devlink)) >> >+ goto nla_put_failure; >> >+ >> >+ if (status_msg && nla_put_string(msg, DEVLINK_ATTR_RESET_STATUS_MSG, >> >status_msg)) >> >+ goto nla_put_failure; >> >+ >> >+ if (nla_put_u32(msg, DEVLINK_ATTR_RESET_COMPONENTS, components)) >> >+ goto nla_put_failure; >> >+ >> >+ genlmsg_end(msg, hdr); >> >+ return 0; >> >+ >> >+nla_put_failure: >> >+ genlmsg_cancel(msg, hdr); >> >+ return -EMSGSIZE; >> >+} >> >+ >> >+static void __devlink_reset_notify(struct devlink *devlink, const char >> >*status_msg, u32 components) >> >+{ >> >+ struct sk_buff *msg; >> >+ int err; >> >+ >> >+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); >> >+ if (!msg) >> >+ return; >> >+ >> >+ err = devlink_nl_reset_fill(msg, devlink, status_msg, components); >> >+ if (err) >> >+ goto out; >> >+ >> >+ genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink), >> >msg, 0, >> >+ DEVLINK_MCGRP_CONFIG, GFP_KERNEL); >> >+ return; >> >+ >> >+out: >> >+ nlmsg_free(msg); >> >+} >> >+ >> >+static int devlink_nl_cmd_reset(struct sk_buff *skb, struct genl_info >> >*info) >> >+{ >> >+ struct devlink *devlink = info->user_ptr[0]; >> >+ u32 components, req_comps; >> >+ struct nlattr *nla_type; >> >+ u8 width, mode; >> >+ int err; >> >+ >> >+ if (!devlink->ops->reset) >> >+ return -EOPNOTSUPP; >> >+ >> >+ if (!info->attrs[DEVLINK_ATTR_RESET_COMPONENTS]) >> >+ return -EINVAL; >> >+ components = nla_get_u32(info->attrs[DEVLINK_ATTR_RESET_COMPONENTS]); >> >+ >> >+ nla_type = info->attrs[DEVLINK_ATTR_RESET_WIDTH]; >> >+ width = nla_type ? nla_get_u8(nla_type) : DEVLINK_RESET_WIDTH_SINGLE; >> >+ >> >+ nla_type = info->attrs[DEVLINK_ATTR_RESET_MODE]; >> >+ mode = nla_type ? nla_get_u8(nla_type) : DEVLINK_RESET_MODE_DEFERRED; >> >+ >> >+ req_comps = components; >> >+ __devlink_reset_notify(devlink, "Reset request", components); >> >+ err = devlink->ops->reset(devlink, &components, width, mode, >> >info->extack); >> >+ __devlink_reset_notify(devlink, "Components reset", req_comps & >> >~components); >> >+ >> >+ return err; >> >+} >> >+ >> > static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = { >> > [DEVLINK_ATTR_UNSPEC] = { .strict_start_type = >> > DEVLINK_ATTR_TRAP_POLICER_ID }, >> >@@ -6842,6 +6918,9 @@ static int >> >devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb, >> > [DEVLINK_ATTR_TRAP_POLICER_RATE] = { .type = NLA_U64 }, >> > [DEVLINK_ATTR_TRAP_POLICER_BURST] = { .type = NLA_U64 }, >> > [DEVLINK_ATTR_PORT_FUNCTION] = { .type = NLA_NESTED }, >> >+ [DEVLINK_ATTR_RESET_COMPONENTS] = { .type = NLA_U32 }, >> >+ [DEVLINK_ATTR_RESET_WIDTH] = { .type = NLA_U8 }, >> >+ [DEVLINK_ATTR_RESET_MODE] = { .type = NLA_U8 }, >> > }; >> > >> > static const struct genl_ops devlink_nl_ops[] = { >> >@@ -7190,6 +7269,12 @@ static int >> >devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb, >> > .flags = GENL_ADMIN_PERM, >> > .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK, >> > }, >> >+ { >> >+ .cmd = DEVLINK_CMD_RESET, >> >+ .doit = devlink_nl_cmd_reset, >> >+ .flags = GENL_ADMIN_PERM, >> >+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK, >> >+ }, >> > }; >> > >> > static struct genl_family devlink_nl_family __ro_after_init = { >> >-- >> >1.8.3.1 >> >
