* Re: [PATCH v4 06/11] net: dsa: microchip: ksz8795: change drivers prefix to be generic
From: Jakub Kicinski @ 2020-08-03 15:42 UTC (permalink / raw)
To: Michael Grzeschik; +Cc: andrew, netdev, f.fainelli, davem, kernel
In-Reply-To: <20200803054442.20089-7-m.grzeschik@pengutronix.de>
On Mon, 3 Aug 2020 07:44:37 +0200 Michael Grzeschik wrote:
> The driver can be used on other chips of this type. To reflect
> this we rename the drivers prefix from ksz8795 to ksz8.
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
allmodconfig:
../drivers/net/dsa/microchip/ksz8795.c:415:41: error: using member 'shifts' in incomplete struct ksz8
../drivers/net/dsa/microchip/ksz8795.c:802:46: warning: incorrect type in argument 3 (different type sizes)
../drivers/net/dsa/microchip/ksz8795.c:802:46: expected unsigned int [usertype] *vlan
../drivers/net/dsa/microchip/ksz8795.c:802:46: got unsigned short *
../drivers/net/dsa/microchip/ksz8795.c:843:46: warning: incorrect type in argument 3 (different type sizes)
../drivers/net/dsa/microchip/ksz8795.c:843:46: expected unsigned int [usertype] *vlan
../drivers/net/dsa/microchip/ksz8795.c:843:46: got unsigned short *
../drivers/net/dsa/microchip/ksz8795.c: In function ‘ksz8_r_vlan_entries’:
../drivers/net/dsa/microchip/ksz8795.c:415:34: error: invalid use of undefined type ‘struct ksz8’
415 | struct ksz_shifts *shifts = ksz8->shifts;
| ^~
../drivers/net/dsa/microchip/ksz8795.c:415:21: warning: unused variable ‘shifts’ [-Wunused-variable]
415 | struct ksz_shifts *shifts = ksz8->shifts;
| ^~~~~~
../drivers/net/dsa/microchip/ksz8795.c: In function ‘ksz8_port_vlan_add’:
../drivers/net/dsa/microchip/ksz8795.c:802:31: error: passing argument 3 of ‘ksz8_r_vlan_table’ from incompatible pointer type [-Werror=incompatible-pointer-types]
802 | ksz8_r_vlan_table(dev, vid, &data);
| ^~~~~
| |
| u16 * {aka short unsigned int *}
../drivers/net/dsa/microchip/ksz8795.c:427:69: note: expected ‘u32 *’ {aka ‘unsigned int *’} but argument is of type ‘u16 *’ {aka ‘short unsigned int *’}
427 | static void ksz8_r_vlan_table(struct ksz_device *dev, u16 vid, u32 *vlan)
| ~~~~~^~~~
../drivers/net/dsa/microchip/ksz8795.c: In function ‘ksz8_port_vlan_del’:
../drivers/net/dsa/microchip/ksz8795.c:843:31: error: passing argument 3 of ‘ksz8_r_vlan_table’ from incompatible pointer type [-Werror=incompatible-pointer-types]
843 | ksz8_r_vlan_table(dev, vid, &data);
| ^~~~~
| |
| u16 * {aka short unsigned int *}
../drivers/net/dsa/microchip/ksz8795.c:427:69: note: expected ‘u32 *’ {aka ‘unsigned int *’} but argument is of type ‘u16 *’ {aka ‘short unsigned int *’}
427 | static void ksz8_r_vlan_table(struct ksz_device *dev, u16 vid, u32 *vlan)
| ~~~~~^~~~
cc1: some warnings being treated as errors
make[5]: *** [../scripts/Makefile.build:281: drivers/net/dsa/microchip/ksz8795.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [../scripts/Makefile.build:497: drivers/net/dsa/microchip] Error 2
make[3]: *** [../scripts/Makefile.build:497: drivers/net/dsa] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [../scripts/Makefile.build:497: drivers/net] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/netdev/net-next/Makefile:1771: drivers] Error 2
^ permalink raw reply
* Re: [net-next v2 1/5] devlink: convert flash_update to use params structure
From: Jiri Pirko @ 2020-08-03 15:46 UTC (permalink / raw)
To: Jacob Keller
Cc: netdev, Jiri Pirko, Jakub Kicinski, Jonathan Corbet, Michael Chan,
Bin Luo, Saeed Mahameed, Leon Romanovsky, Ido Schimmel,
Danielle Ratson
In-Reply-To: <20200801002159.3300425-2-jacob.e.keller@intel.com>
Sat, Aug 01, 2020 at 02:21:55AM CEST, jacob.e.keller@intel.com wrote:
>A future change is going to introduce a new parameter for specifying how
>devices should handle overwrite behavior when updating the flash. This
>will introduce a new argument specifying a bitmask of component
>subsections to allow overwriting behavior.
>
>Prepare for this by converting flash_update to use a new
>devlink_flash_update_params structure. For now this just holds the
>file_name and component, but this enables more easily extending the
>function with new parameters in the future.
>
>To avoid the need for modifying every driver when a new parameter is
>introduced, the supported_flash_update_params field is added to
This is not direstly related to the "params" introduction.
Please split at least to 2 patches.
>devlink_ops. Drivers must opt-in to supported parameters by setting the
>appropriate bits in this field. This allows dropping the check in each
>driver that doesn't support component updates.
>
>Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>Cc: Jiri Pirko <jiri@mellanox.com>
>Cc: Jakub Kicinski <kuba@kernel.org>
>Cc: Jonathan Corbet <corbet@lwn.net>
>Cc: Michael Chan <michael.chan@broadcom.com>
>Cc: Bin Luo <luobin9@huawei.com>
>Cc: Saeed Mahameed <saeedm@mellanox.com>
>Cc: Leon Romanovsky <leon@kernel.org>
>Cc: Ido Schimmel <idosch@mellanox.com>
>Cc: Danielle Ratson <danieller@mellanox.com>
>---
>
>Changes since v1
>* Add supported_flash_update_params field, to allow drivers to opt-in to the
> set of supported parameters. This is similar to supported_coalesc_params
> and was suggested by Jakub. This also makes adding future parameters
> easier by reducing the need to modify existing drivers! Due to this, the
> boilerplate check for component is simply removed.
>
>* netdevsim is modified to support the component parameter, and a new simple
> selftest is added to verify that this works.
>
> .../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 19 ++++-------
> .../net/ethernet/huawei/hinic/hinic_devlink.c | 8 ++---
> drivers/net/ethernet/intel/ice/ice_devlink.c | 18 ++++------
> .../net/ethernet/mellanox/mlx5/core/devlink.c | 8 ++---
> drivers/net/ethernet/mellanox/mlxsw/core.c | 6 ++--
> drivers/net/ethernet/mellanox/mlxsw/core.h | 2 +-
> .../net/ethernet/mellanox/mlxsw/spectrum.c | 7 ++--
> .../net/ethernet/netronome/nfp/nfp_devlink.c | 9 +++--
> drivers/net/netdevsim/dev.c | 13 ++++----
> include/net/devlink.h | 33 +++++++++++++++++--
> net/core/devlink.c | 25 ++++++++++----
> .../drivers/net/netdevsim/devlink.sh | 3 ++
> 12 files changed, 87 insertions(+), 64 deletions(-)
>
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>index 3a854195d5b0..d436134bdc40 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>@@ -17,15 +17,13 @@
> #include "bnxt_ethtool.h"
>
> static int
>-bnxt_dl_flash_update(struct devlink *dl, const char *filename,
>- const char *region, struct netlink_ext_ack *extack)
>+bnxt_dl_flash_update(struct devlink *dl,
>+ struct devlink_flash_update_params *params,
>+ struct netlink_ext_ack *extack)
> {
> struct bnxt *bp = bnxt_get_bp_from_dl(dl);
> int rc;
>
>- if (region)
>- return -EOPNOTSUPP;
>-
> if (!BNXT_PF(bp)) {
> NL_SET_ERR_MSG_MOD(extack,
> "flash update not supported from a VF");
>@@ -33,15 +31,12 @@ bnxt_dl_flash_update(struct devlink *dl, const char *filename,
> }
>
> devlink_flash_update_begin_notify(dl);
>- devlink_flash_update_status_notify(dl, "Preparing to flash", region, 0,
>- 0);
>- rc = bnxt_flash_package_from_file(bp->dev, filename, 0);
>+ devlink_flash_update_status_notify(dl, "Preparing to flash", NULL, 0, 0);
>+ rc = bnxt_flash_package_from_file(bp->dev, params->file_name, 0);
> if (!rc)
>- devlink_flash_update_status_notify(dl, "Flashing done", region,
>- 0, 0);
>+ devlink_flash_update_status_notify(dl, "Flashing done", NULL, 0, 0);
> else
>- devlink_flash_update_status_notify(dl, "Flashing failed",
>- region, 0, 0);
>+ devlink_flash_update_status_notify(dl, "Flashing failed", NULL, 0, 0);
> devlink_flash_update_end_notify(dl);
> return rc;
> }
>diff --git a/drivers/net/ethernet/huawei/hinic/hinic_devlink.c b/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
>index c6adc776f3c8..1f45766107e4 100644
>--- a/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
>+++ b/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
>@@ -281,18 +281,14 @@ static int hinic_firmware_update(struct hinic_devlink_priv *priv,
> }
>
> static int hinic_devlink_flash_update(struct devlink *devlink,
>- const char *file_name,
>- const char *component,
>+ struct devlink_flash_update_params *params,
> struct netlink_ext_ack *extack)
> {
> struct hinic_devlink_priv *priv = devlink_priv(devlink);
> const struct firmware *fw;
> int err;
>
>- if (component)
>- return -EOPNOTSUPP;
>-
>- err = request_firmware_direct(&fw, file_name,
>+ err = request_firmware_direct(&fw, params->file_name,
> &priv->hwdev->hwif->pdev->dev);
> if (err)
> return err;
>diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
>index dbbd8b6f9d1a..c8255235f7c4 100644
>--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
>+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
>@@ -233,8 +233,7 @@ static int ice_devlink_info_get(struct devlink *devlink,
> /**
> * ice_devlink_flash_update - Update firmware stored in flash on the device
> * @devlink: pointer to devlink associated with device to update
>- * @path: the path of the firmware file to use via request_firmware
>- * @component: name of the component to update, or NULL
>+ * @params: flash update parameters
> * @extack: netlink extended ACK structure
> *
> * Perform a device flash update. The bulk of the update logic is contained
>@@ -243,8 +242,9 @@ static int ice_devlink_info_get(struct devlink *devlink,
> * Returns: zero on success, or an error code on failure.
> */
> static int
>-ice_devlink_flash_update(struct devlink *devlink, const char *path,
>- const char *component, struct netlink_ext_ack *extack)
>+ice_devlink_flash_update(struct devlink *devlink,
>+ struct devlink_flash_update_params *params,
>+ struct netlink_ext_ack *extack)
> {
> struct ice_pf *pf = devlink_priv(devlink);
> struct device *dev = &pf->pdev->dev;
>@@ -252,20 +252,16 @@ ice_devlink_flash_update(struct devlink *devlink, const char *path,
> const struct firmware *fw;
> int err;
>
>- /* individual component update is not yet supported */
>- if (component)
>- return -EOPNOTSUPP;
>-
> if (!hw->dev_caps.common_cap.nvm_unified_update) {
> NL_SET_ERR_MSG_MOD(extack, "Current firmware does not support unified update");
> return -EOPNOTSUPP;
> }
>
>- err = ice_check_for_pending_update(pf, component, extack);
>+ err = ice_check_for_pending_update(pf, NULL, extack);
> if (err)
> return err;
>
>- err = request_firmware(&fw, path, dev);
>+ err = request_firmware(&fw, params->file_name, dev);
> if (err) {
> NL_SET_ERR_MSG_MOD(extack, "Unable to read file from disk");
> return err;
>@@ -273,7 +269,7 @@ ice_devlink_flash_update(struct devlink *devlink, const char *path,
>
> devlink_flash_update_begin_notify(devlink);
> devlink_flash_update_status_notify(devlink, "Preparing to flash",
>- component, 0, 0);
>+ NULL, 0, 0);
> err = ice_flash_pldm_image(pf, fw, extack);
> devlink_flash_update_end_notify(devlink);
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>index c709e9a385f6..9b14e3f805a2 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>@@ -8,18 +8,14 @@
> #include "eswitch.h"
>
> static int mlx5_devlink_flash_update(struct devlink *devlink,
>- const char *file_name,
>- const char *component,
>+ struct devlink_flash_update_params *params,
> struct netlink_ext_ack *extack)
> {
> struct mlx5_core_dev *dev = devlink_priv(devlink);
> const struct firmware *fw;
> int err;
>
>- if (component)
>- return -EOPNOTSUPP;
>-
>- err = request_firmware_direct(&fw, file_name, &dev->pdev->dev);
>+ err = request_firmware_direct(&fw, params->file_name, &dev->pdev->dev);
> if (err)
> return err;
>
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
>index b01f8f2fab63..6db938708a0d 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
>@@ -1138,8 +1138,7 @@ mlxsw_devlink_core_bus_device_reload_up(struct devlink *devlink,
> }
>
> static int mlxsw_devlink_flash_update(struct devlink *devlink,
>- const char *file_name,
>- const char *component,
>+ struct devlink_flash_update_params *params,
> struct netlink_ext_ack *extack)
> {
> struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>@@ -1147,8 +1146,7 @@ static int mlxsw_devlink_flash_update(struct devlink *devlink,
>
> if (!mlxsw_driver->flash_update)
> return -EOPNOTSUPP;
>- return mlxsw_driver->flash_update(mlxsw_core, file_name,
>- component, extack);
>+ return mlxsw_driver->flash_update(mlxsw_core, params, extack);
> }
>
> static int mlxsw_devlink_trap_init(struct devlink *devlink,
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
>index c1c1e039323a..b6e3faf38305 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
>+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
>@@ -318,7 +318,7 @@ struct mlxsw_driver {
> enum devlink_sb_pool_type pool_type,
> u32 *p_cur, u32 *p_max);
> int (*flash_update)(struct mlxsw_core *mlxsw_core,
>- const char *file_name, const char *component,
>+ struct devlink_flash_update_params *params,
> struct netlink_ext_ack *extack);
> int (*trap_init)(struct mlxsw_core *mlxsw_core,
> const struct devlink_trap *trap, void *trap_ctx);
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>index 519eb44e4097..6bbf0ab7794c 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>@@ -418,17 +418,14 @@ static int mlxsw_sp_fw_rev_validate(struct mlxsw_sp *mlxsw_sp)
> }
>
> static int mlxsw_sp_flash_update(struct mlxsw_core *mlxsw_core,
>- const char *file_name, const char *component,
>+ struct devlink_flash_update_params *params,
> struct netlink_ext_ack *extack)
> {
> struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> const struct firmware *firmware;
> int err;
>
>- if (component)
>- return -EOPNOTSUPP;
>-
>- err = request_firmware_direct(&firmware, file_name,
>+ err = request_firmware_direct(&firmware, params->file_name,
> mlxsw_sp->bus_info->dev);
> if (err)
> return err;
>diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>index be52510d446b..97d2b03208de 100644
>--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
>@@ -329,12 +329,11 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
> }
>
> static int
>-nfp_devlink_flash_update(struct devlink *devlink, const char *path,
>- const char *component, struct netlink_ext_ack *extack)
>+nfp_devlink_flash_update(struct devlink *devlink,
>+ struct devlink_flash_update_params *params,
>+ struct netlink_ext_ack *extack)
> {
>- if (component)
>- return -EOPNOTSUPP;
>- return nfp_flash_update_common(devlink_priv(devlink), path, extack);
>+ return nfp_flash_update_common(devlink_priv(devlink), params->file_name, extack);
> }
>
> const struct devlink_ops nfp_devlink_ops = {
>diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
>index ce719c830a77..29bb25f0f069 100644
>--- a/drivers/net/netdevsim/dev.c
>+++ b/drivers/net/netdevsim/dev.c
>@@ -740,8 +740,8 @@ static int nsim_dev_info_get(struct devlink *devlink,
> #define NSIM_DEV_FLASH_CHUNK_SIZE 1000
> #define NSIM_DEV_FLASH_CHUNK_TIME_MS 10
>
>-static int nsim_dev_flash_update(struct devlink *devlink, const char *file_name,
>- const char *component,
>+static int nsim_dev_flash_update(struct devlink *devlink,
>+ struct devlink_flash_update_params *params,
> struct netlink_ext_ack *extack)
> {
> struct nsim_dev *nsim_dev = devlink_priv(devlink);
>@@ -751,13 +751,13 @@ static int nsim_dev_flash_update(struct devlink *devlink, const char *file_name,
> devlink_flash_update_begin_notify(devlink);
> devlink_flash_update_status_notify(devlink,
> "Preparing to flash",
>- component, 0, 0);
>+ params->component, 0, 0);
> }
>
> for (i = 0; i < NSIM_DEV_FLASH_SIZE / NSIM_DEV_FLASH_CHUNK_SIZE; i++) {
> if (nsim_dev->fw_update_status)
> devlink_flash_update_status_notify(devlink, "Flashing",
>- component,
>+ params->component,
> i * NSIM_DEV_FLASH_CHUNK_SIZE,
> NSIM_DEV_FLASH_SIZE);
> msleep(NSIM_DEV_FLASH_CHUNK_TIME_MS);
>@@ -765,11 +765,11 @@ static int nsim_dev_flash_update(struct devlink *devlink, const char *file_name,
>
> if (nsim_dev->fw_update_status) {
> devlink_flash_update_status_notify(devlink, "Flashing",
>- component,
>+ params->component,
> NSIM_DEV_FLASH_SIZE,
> NSIM_DEV_FLASH_SIZE);
> devlink_flash_update_status_notify(devlink, "Flashing done",
>- component, 0, 0);
>+ params->component, 0, 0);
> devlink_flash_update_end_notify(devlink);
> }
>
>@@ -873,6 +873,7 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink *devlink,
> }
>
> static const struct devlink_ops nsim_dev_devlink_ops = {
>+ .supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT,
> .reload_down = nsim_dev_reload_down,
> .reload_up = nsim_dev_reload_up,
> .info_get = nsim_dev_info_get,
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 19d990c8edcc..192a2c5b6e82 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -511,6 +511,22 @@ enum devlink_param_generic_id {
> /* Firmware bundle identifier */
> #define DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID "fw.bundle_id"
>
>+/**
>+ * struct devlink_flash_update_params - Flash Update parameters
>+ * @file_name: the name of the flash firmware file to update from
>+ * @component: the flash component to update
>+ *
>+ * With the exception of file_name, drivers must opt-in to parameters by
>+ * setting the appropriate bit in the supported_flash_update_params field in
>+ * their devlink_ops structure.
>+ */
>+struct devlink_flash_update_params {
>+ const char *file_name;
>+ const char *component;
>+};
>+
>+#define DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT BIT(0)
>+
> struct devlink_region;
> struct devlink_info_req;
>
>@@ -985,6 +1001,12 @@ enum devlink_trap_group_generic_id {
> }
>
> struct devlink_ops {
>+ /**
>+ * @supported_flash_update_params:
>+ * mask of parameters supported by the driver's .flash_update
>+ * implemementation.
>+ */
>+ u32 supported_flash_update_params;
> int (*reload_down)(struct devlink *devlink, bool netns_change,
> struct netlink_ext_ack *extack);
> int (*reload_up)(struct devlink *devlink,
>@@ -1045,8 +1067,15 @@ struct devlink_ops {
> struct netlink_ext_ack *extack);
> int (*info_get)(struct devlink *devlink, struct devlink_info_req *req,
> struct netlink_ext_ack *extack);
>- int (*flash_update)(struct devlink *devlink, const char *file_name,
>- const char *component,
>+ /**
>+ * @flash_update: Device flash update function
>+ *
>+ * Used to perform a flash update for the device. The set of
>+ * parameters supported by the driver should be set in
>+ * supported_flash_update_params.
>+ */
>+ int (*flash_update)(struct devlink *devlink,
>+ struct devlink_flash_update_params *params,
> struct netlink_ext_ack *extack);
> /**
> * @trap_init: Trap initialization function.
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 0ca89196a367..3ccba85f85c7 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -3113,22 +3113,32 @@ EXPORT_SYMBOL_GPL(devlink_flash_update_status_notify);
> static int devlink_nl_cmd_flash_update(struct sk_buff *skb,
> struct genl_info *info)
> {
>+ struct devlink_flash_update_params params = {};
> struct devlink *devlink = info->user_ptr[0];
>- const char *file_name, *component;
> struct nlattr *nla_component;
>+ u32 supported_params;
>
> if (!devlink->ops->flash_update)
> return -EOPNOTSUPP;
>
> if (!info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME])
> return -EINVAL;
>- file_name = nla_data(info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME]);
>+
>+ supported_params = devlink->ops->supported_flash_update_params;
It is a bit odd to have this "flash_update" specific. Perhaps better to
have it as generic devlink "cap"? I can easily imagine this being handy
for other ops too.
>+
>+ params.file_name = nla_data(info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME]);
>
> nla_component = info->attrs[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT];
>- component = nla_component ? nla_data(nla_component) : NULL;
>+ if (nla_component) {
>+ if (!(supported_params & DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT)) {
>+ NL_SET_ERR_MSG_ATTR(info->extack, nla_component,
>+ "component update is not supported");
>+ return -EOPNOTSUPP;
>+ }
>+ params.component = nla_data(nla_component);
>+ }
>
>- return devlink->ops->flash_update(devlink, file_name, component,
>- info->extack);
>+ return devlink->ops->flash_update(devlink, ¶ms, info->extack);
> }
>
> static const struct devlink_param devlink_param_generic[] = {
>@@ -9527,6 +9537,7 @@ void devlink_compat_running_version(struct net_device *dev,
>
> int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
> {
>+ struct devlink_flash_update_params params = {};
> struct devlink *devlink;
> int ret;
>
>@@ -9539,8 +9550,10 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
> goto out;
> }
>
>+ params.file_name = file_name;
>+
> mutex_lock(&devlink->lock);
>- ret = devlink->ops->flash_update(devlink, file_name, NULL, NULL);
>+ ret = devlink->ops->flash_update(devlink, ¶ms, NULL);
> mutex_unlock(&devlink->lock);
>
> out:
>diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
>index de4b32fc4223..1e7541688978 100755
>--- a/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
>+++ b/tools/testing/selftests/drivers/net/netdevsim/devlink.sh
>@@ -23,6 +23,9 @@ fw_flash_test()
> devlink dev flash $DL_HANDLE file dummy
> check_err $? "Failed to flash with status updates on"
>
>+ devlink dev flash $DL_HANDLE file dummy component fw.mgmt
Perhaps you can rather use something made up instead of "fw.mgmt" which
is looking legit and therefor confusing.
>+ check_err $? "Failed to flash with component attribute"
>+
> echo "n"> $DEBUGFS_DIR/fw_update_status
> check_err $? "Failed to disable status updates"
>
>--
>2.28.0.163.g6104cc2f0b60
>
^ permalink raw reply
* Re: [iproute2-next v2 5/5] devlink: support setting the overwrite mask
From: David Ahern @ 2020-08-03 15:53 UTC (permalink / raw)
To: Jacob Keller, netdev
In-Reply-To: <20200801002159.3300425-6-jacob.e.keller@intel.com>
On 7/31/20 6:21 PM, Jacob Keller wrote:
> Add support for specifying the overwrite sections to allow in the flash
> update command. This is done by adding a new "overwrite" option which
> can take either "settings" or "identifiers" passing the overwrite mode
> multiple times will combine the fields using bitwise-OR.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> devlink/devlink.c | 37 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 35 insertions(+), 2 deletions(-)
>
5/5? I only see 2 - 4/5 and 5/5. Please re-send against latest
iproute2-next.
^ permalink raw reply
* [PATCH net-next 0/9] mlxsw: Add support for buffer drop traps
From: Ido Schimmel @ 2020-08-03 16:11 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, amitc, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Petr says:
A recent patch set added the ability to mirror buffer related drops
(e.g., early drops) through a netdev. This patch set adds the ability to
trap such packets to the local CPU for analysis.
The trapping towards the CPU is configured by using tc-trap action
instead of tc-mirred as was done when the packets were mirrored through
a netdev. A future patch set will also add the ability to sample the
dropped packets using tc-sample action.
The buffer related drop traps are added to devlink, which means that the
dropped packets can be reported to user space via the kernel's
drop_monitor module.
Patch set overview:
Patch #1 adds the early_drop trap to devlink
Patch #2 adds extack to a few devlink operations to facilitate better
error reporting to user space. This is necessary - among other things -
because the action of buffer drop traps cannot be changed in mlxsw
Patch #3 performs a small refactoring in mlxsw, patch #4 fixes a bug that
this patchset would trigger.
Patches #5-#6 add the infrastructure required to support different traps
/ trap groups in mlxsw per-ASIC. This is required because buffer drop
traps are not supported by Spectrum-1
Patch #7 extends mlxsw to register the early_drop trap
Patch #8 adds the offload logic for the "trap" action at a qevent block.
Patch #9 adds a mlxsw-specific selftest.
Amit Cohen (1):
devlink: Add early_drop trap
Ido Schimmel (5):
devlink: Pass extack when setting trap's action and group's parameters
mlxsw: spectrum_trap: Use 'size_t' for array sizes
mlxsw: spectrum_trap: Allow for per-ASIC trap groups initialization
mlxsw: spectrum_trap: Allow for per-ASIC traps initialization
mlxsw: spectrum_trap: Add early_drop trap
Petr Machata (3):
mlxsw: spectrum_span: On policer_id_base_ref_count, use dec_and_test
mlxsw: spectrum_qdisc: Offload action trap for qevents
selftests: mlxsw: RED: Test offload of trapping on RED qevents
.../networking/devlink/devlink-trap.rst | 4 +
drivers/net/ethernet/mellanox/mlxsw/core.c | 10 +-
drivers/net/ethernet/mellanox/mlxsw/core.h | 19 +-
drivers/net/ethernet/mellanox/mlxsw/reg.h | 1 +
.../net/ethernet/mellanox/mlxsw/spectrum.c | 3 +
.../net/ethernet/mellanox/mlxsw/spectrum.h | 14 +-
.../ethernet/mellanox/mlxsw/spectrum_qdisc.c | 75 +++++-
.../ethernet/mellanox/mlxsw/spectrum_span.c | 3 +-
.../ethernet/mellanox/mlxsw/spectrum_trap.c | 255 ++++++++++++++++--
.../ethernet/mellanox/mlxsw/spectrum_trap.h | 18 +-
drivers/net/netdevsim/dev.c | 6 +-
include/net/devlink.h | 9 +-
net/core/devlink.c | 9 +-
.../drivers/net/mlxsw/sch_red_core.sh | 35 ++-
.../drivers/net/mlxsw/sch_red_ets.sh | 11 +
15 files changed, 406 insertions(+), 66 deletions(-)
--
2.26.2
^ permalink raw reply
* [PATCH net-next 1/9] devlink: Add early_drop trap
From: Ido Schimmel @ 2020-08-03 16:11 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, amitc, mlxsw, Ido Schimmel
In-Reply-To: <20200803161141.2523857-1-idosch@idosch.org>
From: Amit Cohen <amitc@mellanox.com>
Add the packet trap that can report packets that were ECN marked due to RED
AQM.
Signed-off-by: Amit Cohen <amitc@mellanox.com>
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
Documentation/networking/devlink/devlink-trap.rst | 4 ++++
include/net/devlink.h | 3 +++
net/core/devlink.c | 1 +
3 files changed, 8 insertions(+)
diff --git a/Documentation/networking/devlink/devlink-trap.rst b/Documentation/networking/devlink/devlink-trap.rst
index 2014307fbe63..7a798352b45d 100644
--- a/Documentation/networking/devlink/devlink-trap.rst
+++ b/Documentation/networking/devlink/devlink-trap.rst
@@ -405,6 +405,10 @@ be added to the following table:
- ``control``
- Traps packets logged during processing of flow action trap (e.g., via
tc's trap action)
+ * - ``early_drop``
+ - ``drop``
+ - Traps packets dropped due to the RED (Random Early Detection) algorithm
+ (i.e., early drops)
Driver-specific Packet Traps
============================
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 0606967cb501..fd3ae0760492 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -703,6 +703,7 @@ enum devlink_trap_generic_id {
DEVLINK_TRAP_GENERIC_ID_PTP_GENERAL,
DEVLINK_TRAP_GENERIC_ID_FLOW_ACTION_SAMPLE,
DEVLINK_TRAP_GENERIC_ID_FLOW_ACTION_TRAP,
+ DEVLINK_TRAP_GENERIC_ID_EARLY_DROP,
/* Add new generic trap IDs above */
__DEVLINK_TRAP_GENERIC_ID_MAX,
@@ -891,6 +892,8 @@ enum devlink_trap_group_generic_id {
"flow_action_sample"
#define DEVLINK_TRAP_GENERIC_NAME_FLOW_ACTION_TRAP \
"flow_action_trap"
+#define DEVLINK_TRAP_GENERIC_NAME_EARLY_DROP \
+ "early_drop"
#define DEVLINK_TRAP_GROUP_GENERIC_NAME_L2_DROPS \
"l2_drops"
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 5fdebb7289e9..bde4c29a30bc 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -8801,6 +8801,7 @@ static const struct devlink_trap devlink_trap_generic[] = {
DEVLINK_TRAP(PTP_GENERAL, CONTROL),
DEVLINK_TRAP(FLOW_ACTION_SAMPLE, CONTROL),
DEVLINK_TRAP(FLOW_ACTION_TRAP, CONTROL),
+ DEVLINK_TRAP(EARLY_DROP, DROP),
};
#define DEVLINK_TRAP_GROUP(_id) \
--
2.26.2
^ permalink raw reply related
* [PATCH net-next 2/9] devlink: Pass extack when setting trap's action and group's parameters
From: Ido Schimmel @ 2020-08-03 16:11 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, amitc, mlxsw, Ido Schimmel
In-Reply-To: <20200803161141.2523857-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
A later patch will refuse to set the action of certain traps in mlxsw
and also to change the policer binding of certain groups. Pass extack so
that failure could be communicated clearly to user space.
Reviewed-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/core.c | 10 ++++++----
drivers/net/ethernet/mellanox/mlxsw/core.h | 6 ++++--
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 6 ++++--
drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c | 13 ++++++++-----
drivers/net/netdevsim/dev.c | 6 ++++--
include/net/devlink.h | 6 ++++--
net/core/devlink.c | 8 +++++---
7 files changed, 35 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 866381e72960..08d101138fbe 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1177,14 +1177,15 @@ static void mlxsw_devlink_trap_fini(struct devlink *devlink,
static int mlxsw_devlink_trap_action_set(struct devlink *devlink,
const struct devlink_trap *trap,
- enum devlink_trap_action action)
+ enum devlink_trap_action action,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
struct mlxsw_driver *mlxsw_driver = mlxsw_core->driver;
if (!mlxsw_driver->trap_action_set)
return -EOPNOTSUPP;
- return mlxsw_driver->trap_action_set(mlxsw_core, trap, action);
+ return mlxsw_driver->trap_action_set(mlxsw_core, trap, action, extack);
}
static int
@@ -1202,14 +1203,15 @@ mlxsw_devlink_trap_group_init(struct devlink *devlink,
static int
mlxsw_devlink_trap_group_set(struct devlink *devlink,
const struct devlink_trap_group *group,
- const struct devlink_trap_policer *policer)
+ const struct devlink_trap_policer *policer,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
struct mlxsw_driver *mlxsw_driver = mlxsw_core->driver;
if (!mlxsw_driver->trap_group_set)
return -EOPNOTSUPP;
- return mlxsw_driver->trap_group_set(mlxsw_core, group, policer);
+ return mlxsw_driver->trap_group_set(mlxsw_core, group, policer, extack);
}
static int
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index c1c1e039323a..219ce89e629a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -326,12 +326,14 @@ struct mlxsw_driver {
const struct devlink_trap *trap, void *trap_ctx);
int (*trap_action_set)(struct mlxsw_core *mlxsw_core,
const struct devlink_trap *trap,
- enum devlink_trap_action action);
+ enum devlink_trap_action action,
+ struct netlink_ext_ack *extack);
int (*trap_group_init)(struct mlxsw_core *mlxsw_core,
const struct devlink_trap_group *group);
int (*trap_group_set)(struct mlxsw_core *mlxsw_core,
const struct devlink_trap_group *group,
- const struct devlink_trap_policer *policer);
+ const struct devlink_trap_policer *policer,
+ struct netlink_ext_ack *extack);
int (*trap_policer_init)(struct mlxsw_core *mlxsw_core,
const struct devlink_trap_policer *policer);
void (*trap_policer_fini)(struct mlxsw_core *mlxsw_core,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 6ab1b6d725af..866a1193f12b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -1177,12 +1177,14 @@ void mlxsw_sp_trap_fini(struct mlxsw_core *mlxsw_core,
const struct devlink_trap *trap, void *trap_ctx);
int mlxsw_sp_trap_action_set(struct mlxsw_core *mlxsw_core,
const struct devlink_trap *trap,
- enum devlink_trap_action action);
+ enum devlink_trap_action action,
+ struct netlink_ext_ack *extack);
int mlxsw_sp_trap_group_init(struct mlxsw_core *mlxsw_core,
const struct devlink_trap_group *group);
int mlxsw_sp_trap_group_set(struct mlxsw_core *mlxsw_core,
const struct devlink_trap_group *group,
- const struct devlink_trap_policer *policer);
+ const struct devlink_trap_policer *policer,
+ struct netlink_ext_ack *extack);
int
mlxsw_sp_trap_policer_init(struct mlxsw_core *mlxsw_core,
const struct devlink_trap_policer *policer);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index 1e38dfe7cf64..00b6cb9d2306 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -1352,7 +1352,8 @@ void mlxsw_sp_trap_fini(struct mlxsw_core *mlxsw_core,
int mlxsw_sp_trap_action_set(struct mlxsw_core *mlxsw_core,
const struct devlink_trap *trap,
- enum devlink_trap_action action)
+ enum devlink_trap_action action,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
const struct mlxsw_sp_trap_item *trap_item;
@@ -1392,7 +1393,7 @@ int mlxsw_sp_trap_action_set(struct mlxsw_core *mlxsw_core,
static int
__mlxsw_sp_trap_group_init(struct mlxsw_core *mlxsw_core,
const struct devlink_trap_group *group,
- u32 policer_id)
+ u32 policer_id, struct netlink_ext_ack *extack)
{
struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
u16 hw_policer_id = MLXSW_REG_HTGT_INVALID_POLICER;
@@ -1422,16 +1423,18 @@ int mlxsw_sp_trap_group_init(struct mlxsw_core *mlxsw_core,
const struct devlink_trap_group *group)
{
return __mlxsw_sp_trap_group_init(mlxsw_core, group,
- group->init_policer_id);
+ group->init_policer_id, NULL);
}
int mlxsw_sp_trap_group_set(struct mlxsw_core *mlxsw_core,
const struct devlink_trap_group *group,
- const struct devlink_trap_policer *policer)
+ const struct devlink_trap_policer *policer,
+ struct netlink_ext_ack *extack)
{
u32 policer_id = policer ? policer->id : 0;
- return __mlxsw_sp_trap_group_init(mlxsw_core, group, policer_id);
+ return __mlxsw_sp_trap_group_init(mlxsw_core, group, policer_id,
+ extack);
}
static int
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index ce719c830a77..32f339fedb21 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -810,7 +810,8 @@ static int nsim_dev_devlink_trap_init(struct devlink *devlink,
static int
nsim_dev_devlink_trap_action_set(struct devlink *devlink,
const struct devlink_trap *trap,
- enum devlink_trap_action action)
+ enum devlink_trap_action action,
+ struct netlink_ext_ack *extack)
{
struct nsim_dev *nsim_dev = devlink_priv(devlink);
struct nsim_trap_item *nsim_trap_item;
@@ -829,7 +830,8 @@ nsim_dev_devlink_trap_action_set(struct devlink *devlink,
static int
nsim_dev_devlink_trap_group_set(struct devlink *devlink,
const struct devlink_trap_group *group,
- const struct devlink_trap_policer *policer)
+ const struct devlink_trap_policer *policer,
+ struct netlink_ext_ack *extack)
{
struct nsim_dev *nsim_dev = devlink_priv(devlink);
diff --git a/include/net/devlink.h b/include/net/devlink.h
index fd3ae0760492..8f3c8a443238 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1077,7 +1077,8 @@ struct devlink_ops {
*/
int (*trap_action_set)(struct devlink *devlink,
const struct devlink_trap *trap,
- enum devlink_trap_action action);
+ enum devlink_trap_action action,
+ struct netlink_ext_ack *extack);
/**
* @trap_group_init: Trap group initialization function.
*
@@ -1094,7 +1095,8 @@ struct devlink_ops {
*/
int (*trap_group_set)(struct devlink *devlink,
const struct devlink_trap_group *group,
- const struct devlink_trap_policer *policer);
+ const struct devlink_trap_policer *policer,
+ struct netlink_ext_ack *extack);
/**
* @trap_policer_init: Trap policer initialization function.
*
diff --git a/net/core/devlink.c b/net/core/devlink.c
index bde4c29a30bc..e674f0f46dc2 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6423,7 +6423,7 @@ static int __devlink_trap_action_set(struct devlink *devlink,
}
err = devlink->ops->trap_action_set(devlink, trap_item->trap,
- trap_action);
+ trap_action, extack);
if (err)
return err;
@@ -6713,7 +6713,8 @@ static int devlink_trap_group_set(struct devlink *devlink,
}
policer = policer_item ? policer_item->policer : NULL;
- err = devlink->ops->trap_group_set(devlink, group_item->group, policer);
+ err = devlink->ops->trap_group_set(devlink, group_item->group, policer,
+ extack);
if (err)
return err;
@@ -9051,7 +9052,8 @@ static void devlink_trap_disable(struct devlink *devlink,
if (WARN_ON_ONCE(!trap_item))
return;
- devlink->ops->trap_action_set(devlink, trap, DEVLINK_TRAP_ACTION_DROP);
+ devlink->ops->trap_action_set(devlink, trap, DEVLINK_TRAP_ACTION_DROP,
+ NULL);
trap_item->action = DEVLINK_TRAP_ACTION_DROP;
}
--
2.26.2
^ permalink raw reply related
* [PATCH net-next 3/9] mlxsw: spectrum_trap: Use 'size_t' for array sizes
From: Ido Schimmel @ 2020-08-03 16:11 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, amitc, mlxsw, Ido Schimmel
In-Reply-To: <20200803161141.2523857-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
Use 'size_t' instead of 'u64' for array sizes, as this this is correct
type to use for expressions involving sizeof().
Suggested-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c | 4 ++--
drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index 00b6cb9d2306..47bc11a861cc 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -1063,10 +1063,10 @@ static int mlxsw_sp_trap_dummy_group_init(struct mlxsw_sp *mlxsw_sp)
static int mlxsw_sp_trap_policer_items_arr_init(struct mlxsw_sp *mlxsw_sp)
{
+ size_t arr_size = ARRAY_SIZE(mlxsw_sp_trap_policer_items_arr);
size_t elem_size = sizeof(struct mlxsw_sp_trap_policer_item);
- u64 arr_size = ARRAY_SIZE(mlxsw_sp_trap_policer_items_arr);
struct mlxsw_sp_trap *trap = mlxsw_sp->trap;
- u64 free_policers = 0;
+ size_t free_policers = 0;
u32 last_id;
int i;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
index 13ac412f4d53..a0560fb030ee 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
@@ -9,13 +9,13 @@
struct mlxsw_sp_trap {
struct mlxsw_sp_trap_policer_item *policer_items_arr;
- u64 policers_count; /* Number of registered policers */
+ size_t policers_count; /* Number of registered policers */
struct mlxsw_sp_trap_group_item *group_items_arr;
- u64 groups_count; /* Number of registered groups */
+ size_t groups_count; /* Number of registered groups */
struct mlxsw_sp_trap_item *trap_items_arr;
- u64 traps_count; /* Number of registered traps */
+ size_t traps_count; /* Number of registered traps */
u16 thin_policer_hw_id;
--
2.26.2
^ permalink raw reply related
* [PATCH net-next 4/9] mlxsw: spectrum_span: On policer_id_base_ref_count, use dec_and_test
From: Ido Schimmel @ 2020-08-03 16:11 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, amitc, mlxsw, Ido Schimmel
In-Reply-To: <20200803161141.2523857-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
When unsetting policer base, the SPAN code currently uses refcount_dec().
However that function splats when the counter reaches zero, because
reaching zero without actually testing is in general indicative of a
missing cleanup. There is no cleanup to be done here, but nonetheless, use
refcount_dec_and_test() as required.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
index 323eaf979aea..5c959a995199 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
@@ -837,7 +837,8 @@ static int mlxsw_sp_span_policer_id_base_set(struct mlxsw_sp_span *span,
static void mlxsw_sp_span_policer_id_base_unset(struct mlxsw_sp_span *span)
{
- refcount_dec(&span->policer_id_base_ref_count);
+ if (refcount_dec_and_test(&span->policer_id_base_ref_count))
+ span->policer_id_base = 0;
}
static struct mlxsw_sp_span_entry *
--
2.26.2
^ permalink raw reply related
* [PATCH net-next 5/9] mlxsw: spectrum_trap: Allow for per-ASIC trap groups initialization
From: Ido Schimmel @ 2020-08-03 16:11 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, amitc, mlxsw, Ido Schimmel
In-Reply-To: <20200803161141.2523857-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
Subsequent patches will need to register different trap groups for
Spectrum-1 and Spectrum-2 onwards.
Enable that by invoking a per-ASIC operation during trap groups
initialization.
Reviewed-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum.c | 3 +
.../net/ethernet/mellanox/mlxsw/spectrum.h | 1 +
.../ethernet/mellanox/mlxsw/spectrum_trap.c | 89 +++++++++++++++++--
.../ethernet/mellanox/mlxsw/spectrum_trap.h | 9 ++
4 files changed, 93 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 519eb44e4097..fdf9aa8314b2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3055,6 +3055,7 @@ static int mlxsw_sp1_init(struct mlxsw_core *mlxsw_core,
mlxsw_sp->ptp_ops = &mlxsw_sp1_ptp_ops;
mlxsw_sp->span_ops = &mlxsw_sp1_span_ops;
mlxsw_sp->policer_core_ops = &mlxsw_sp1_policer_core_ops;
+ mlxsw_sp->trap_ops = &mlxsw_sp1_trap_ops;
mlxsw_sp->listeners = mlxsw_sp1_listener;
mlxsw_sp->listeners_count = ARRAY_SIZE(mlxsw_sp1_listener);
mlxsw_sp->lowest_shaper_bs = MLXSW_REG_QEEC_LOWEST_SHAPER_BS_SP1;
@@ -3084,6 +3085,7 @@ static int mlxsw_sp2_init(struct mlxsw_core *mlxsw_core,
mlxsw_sp->ptp_ops = &mlxsw_sp2_ptp_ops;
mlxsw_sp->span_ops = &mlxsw_sp2_span_ops;
mlxsw_sp->policer_core_ops = &mlxsw_sp2_policer_core_ops;
+ mlxsw_sp->trap_ops = &mlxsw_sp2_trap_ops;
mlxsw_sp->lowest_shaper_bs = MLXSW_REG_QEEC_LOWEST_SHAPER_BS_SP2;
return mlxsw_sp_init(mlxsw_core, mlxsw_bus_info, extack);
@@ -3111,6 +3113,7 @@ static int mlxsw_sp3_init(struct mlxsw_core *mlxsw_core,
mlxsw_sp->ptp_ops = &mlxsw_sp2_ptp_ops;
mlxsw_sp->span_ops = &mlxsw_sp3_span_ops;
mlxsw_sp->policer_core_ops = &mlxsw_sp2_policer_core_ops;
+ mlxsw_sp->trap_ops = &mlxsw_sp2_trap_ops;
mlxsw_sp->lowest_shaper_bs = MLXSW_REG_QEEC_LOWEST_SHAPER_BS_SP3;
return mlxsw_sp_init(mlxsw_core, mlxsw_bus_info, extack);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 866a1193f12b..b808f6b4d670 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -177,6 +177,7 @@ struct mlxsw_sp {
const struct mlxsw_sp_ptp_ops *ptp_ops;
const struct mlxsw_sp_span_ops *span_ops;
const struct mlxsw_sp_policer_core_ops *policer_core_ops;
+ const struct mlxsw_sp_trap_ops *trap_ops;
const struct mlxsw_listener *listeners;
size_t listeners_count;
u32 lowest_shaper_bs;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index 47bc11a861cc..3726be5c02b4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -1159,6 +1159,43 @@ static void mlxsw_sp_trap_policers_fini(struct mlxsw_sp *mlxsw_sp)
mlxsw_sp_trap_policer_items_arr_fini(mlxsw_sp);
}
+static int mlxsw_sp_trap_group_items_arr_init(struct mlxsw_sp *mlxsw_sp)
+{
+ size_t common_groups_count = ARRAY_SIZE(mlxsw_sp_trap_group_items_arr);
+ const struct mlxsw_sp_trap_group_item *spec_group_items_arr;
+ size_t elem_size = sizeof(struct mlxsw_sp_trap_group_item);
+ struct mlxsw_sp_trap *trap = mlxsw_sp->trap;
+ size_t groups_count, spec_groups_count;
+ int err;
+
+ err = mlxsw_sp->trap_ops->groups_init(mlxsw_sp, &spec_group_items_arr,
+ &spec_groups_count);
+ if (err)
+ return err;
+
+ /* The group items array is created by concatenating the common trap
+ * group items and the ASIC-specific trap group items.
+ */
+ groups_count = common_groups_count + spec_groups_count;
+ trap->group_items_arr = kcalloc(groups_count, elem_size, GFP_KERNEL);
+ if (!trap->group_items_arr)
+ return -ENOMEM;
+
+ memcpy(trap->group_items_arr, mlxsw_sp_trap_group_items_arr,
+ elem_size * common_groups_count);
+ memcpy(trap->group_items_arr + common_groups_count,
+ spec_group_items_arr, elem_size * spec_groups_count);
+
+ trap->groups_count = groups_count;
+
+ return 0;
+}
+
+static void mlxsw_sp_trap_group_items_arr_fini(struct mlxsw_sp *mlxsw_sp)
+{
+ kfree(mlxsw_sp->trap->group_items_arr);
+}
+
static int mlxsw_sp_trap_groups_init(struct mlxsw_sp *mlxsw_sp)
{
struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
@@ -1166,13 +1203,9 @@ static int mlxsw_sp_trap_groups_init(struct mlxsw_sp *mlxsw_sp)
struct mlxsw_sp_trap *trap = mlxsw_sp->trap;
int err, i;
- trap->group_items_arr = kmemdup(mlxsw_sp_trap_group_items_arr,
- sizeof(mlxsw_sp_trap_group_items_arr),
- GFP_KERNEL);
- if (!trap->group_items_arr)
- return -ENOMEM;
-
- trap->groups_count = ARRAY_SIZE(mlxsw_sp_trap_group_items_arr);
+ err = mlxsw_sp_trap_group_items_arr_init(mlxsw_sp);
+ if (err)
+ return err;
for (i = 0; i < trap->groups_count; i++) {
group_item = &trap->group_items_arr[i];
@@ -1189,7 +1222,7 @@ static int mlxsw_sp_trap_groups_init(struct mlxsw_sp *mlxsw_sp)
group_item = &trap->group_items_arr[i];
devlink_trap_groups_unregister(devlink, &group_item->group, 1);
}
- kfree(trap->group_items_arr);
+ mlxsw_sp_trap_group_items_arr_fini(mlxsw_sp);
return err;
}
@@ -1205,7 +1238,7 @@ static void mlxsw_sp_trap_groups_fini(struct mlxsw_sp *mlxsw_sp)
group_item = &trap->group_items_arr[i];
devlink_trap_groups_unregister(devlink, &group_item->group, 1);
}
- kfree(trap->group_items_arr);
+ mlxsw_sp_trap_group_items_arr_fini(mlxsw_sp);
}
static bool
@@ -1579,3 +1612,41 @@ mlxsw_sp_trap_policer_counter_get(struct mlxsw_core *mlxsw_core,
return 0;
}
+
+static const struct mlxsw_sp_trap_group_item
+mlxsw_sp1_trap_group_items_arr[] = {
+};
+
+static int
+mlxsw_sp1_trap_groups_init(struct mlxsw_sp *mlxsw_sp,
+ const struct mlxsw_sp_trap_group_item **arr,
+ size_t *p_groups_count)
+{
+ *arr = mlxsw_sp1_trap_group_items_arr;
+ *p_groups_count = ARRAY_SIZE(mlxsw_sp1_trap_group_items_arr);
+
+ return 0;
+}
+
+const struct mlxsw_sp_trap_ops mlxsw_sp1_trap_ops = {
+ .groups_init = mlxsw_sp1_trap_groups_init,
+};
+
+static const struct mlxsw_sp_trap_group_item
+mlxsw_sp2_trap_group_items_arr[] = {
+};
+
+static int
+mlxsw_sp2_trap_groups_init(struct mlxsw_sp *mlxsw_sp,
+ const struct mlxsw_sp_trap_group_item **arr,
+ size_t *p_groups_count)
+{
+ *arr = mlxsw_sp2_trap_group_items_arr;
+ *p_groups_count = ARRAY_SIZE(mlxsw_sp2_trap_group_items_arr);
+
+ return 0;
+}
+
+const struct mlxsw_sp_trap_ops mlxsw_sp2_trap_ops = {
+ .groups_init = mlxsw_sp2_trap_groups_init,
+};
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
index a0560fb030ee..4ae5212b9a48 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
@@ -23,4 +23,13 @@ struct mlxsw_sp_trap {
unsigned long policers_usage[]; /* Usage bitmap */
};
+struct mlxsw_sp_trap_ops {
+ int (*groups_init)(struct mlxsw_sp *mlxsw_sp,
+ const struct mlxsw_sp_trap_group_item **arr,
+ size_t *p_groups_count);
+};
+
+extern const struct mlxsw_sp_trap_ops mlxsw_sp1_trap_ops;
+extern const struct mlxsw_sp_trap_ops mlxsw_sp2_trap_ops;
+
#endif
--
2.26.2
^ permalink raw reply related
* [PATCH net-next 6/9] mlxsw: spectrum_trap: Allow for per-ASIC traps initialization
From: Ido Schimmel @ 2020-08-03 16:11 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, amitc, mlxsw, Ido Schimmel
In-Reply-To: <20200803161141.2523857-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
Subsequent patches will need to register different traps for Spectrum-1
and Spectrum-2 onwards.
Enable that by invoking a per-ASIC operation during traps
initialization.
Reviewed-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_trap.c | 81 ++++++++++++++++---
.../ethernet/mellanox/mlxsw/spectrum_trap.h | 3 +
2 files changed, 75 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index 3726be5c02b4..93dd88abbe23 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -1247,6 +1247,43 @@ mlxsw_sp_trap_listener_is_valid(const struct mlxsw_listener *listener)
return listener->trap_id != 0;
}
+static int mlxsw_sp_trap_items_arr_init(struct mlxsw_sp *mlxsw_sp)
+{
+ size_t common_traps_count = ARRAY_SIZE(mlxsw_sp_trap_items_arr);
+ const struct mlxsw_sp_trap_item *spec_trap_items_arr;
+ size_t elem_size = sizeof(struct mlxsw_sp_trap_item);
+ struct mlxsw_sp_trap *trap = mlxsw_sp->trap;
+ size_t traps_count, spec_traps_count;
+ int err;
+
+ err = mlxsw_sp->trap_ops->traps_init(mlxsw_sp, &spec_trap_items_arr,
+ &spec_traps_count);
+ if (err)
+ return err;
+
+ /* The trap items array is created by concatenating the common trap
+ * items and the ASIC-specific trap items.
+ */
+ traps_count = common_traps_count + spec_traps_count;
+ trap->trap_items_arr = kcalloc(traps_count, elem_size, GFP_KERNEL);
+ if (!trap->trap_items_arr)
+ return -ENOMEM;
+
+ memcpy(trap->trap_items_arr, mlxsw_sp_trap_items_arr,
+ elem_size * common_traps_count);
+ memcpy(trap->trap_items_arr + common_traps_count,
+ spec_trap_items_arr, elem_size * spec_traps_count);
+
+ trap->traps_count = traps_count;
+
+ return 0;
+}
+
+static void mlxsw_sp_trap_items_arr_fini(struct mlxsw_sp *mlxsw_sp)
+{
+ kfree(mlxsw_sp->trap->trap_items_arr);
+}
+
static int mlxsw_sp_traps_init(struct mlxsw_sp *mlxsw_sp)
{
struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
@@ -1254,13 +1291,9 @@ static int mlxsw_sp_traps_init(struct mlxsw_sp *mlxsw_sp)
const struct mlxsw_sp_trap_item *trap_item;
int err, i;
- trap->trap_items_arr = kmemdup(mlxsw_sp_trap_items_arr,
- sizeof(mlxsw_sp_trap_items_arr),
- GFP_KERNEL);
- if (!trap->trap_items_arr)
- return -ENOMEM;
-
- trap->traps_count = ARRAY_SIZE(mlxsw_sp_trap_items_arr);
+ err = mlxsw_sp_trap_items_arr_init(mlxsw_sp);
+ if (err)
+ return err;
for (i = 0; i < trap->traps_count; i++) {
trap_item = &trap->trap_items_arr[i];
@@ -1277,7 +1310,7 @@ static int mlxsw_sp_traps_init(struct mlxsw_sp *mlxsw_sp)
trap_item = &trap->trap_items_arr[i];
devlink_traps_unregister(devlink, &trap_item->trap, 1);
}
- kfree(trap->trap_items_arr);
+ mlxsw_sp_trap_items_arr_fini(mlxsw_sp);
return err;
}
@@ -1293,7 +1326,7 @@ static void mlxsw_sp_traps_fini(struct mlxsw_sp *mlxsw_sp)
trap_item = &trap->trap_items_arr[i];
devlink_traps_unregister(devlink, &trap_item->trap, 1);
}
- kfree(trap->trap_items_arr);
+ mlxsw_sp_trap_items_arr_fini(mlxsw_sp);
}
int mlxsw_sp_devlink_traps_init(struct mlxsw_sp *mlxsw_sp)
@@ -1617,6 +1650,10 @@ static const struct mlxsw_sp_trap_group_item
mlxsw_sp1_trap_group_items_arr[] = {
};
+static const struct mlxsw_sp_trap_item
+mlxsw_sp1_trap_items_arr[] = {
+};
+
static int
mlxsw_sp1_trap_groups_init(struct mlxsw_sp *mlxsw_sp,
const struct mlxsw_sp_trap_group_item **arr,
@@ -1628,14 +1665,29 @@ mlxsw_sp1_trap_groups_init(struct mlxsw_sp *mlxsw_sp,
return 0;
}
+static int mlxsw_sp1_traps_init(struct mlxsw_sp *mlxsw_sp,
+ const struct mlxsw_sp_trap_item **arr,
+ size_t *p_traps_count)
+{
+ *arr = mlxsw_sp1_trap_items_arr;
+ *p_traps_count = ARRAY_SIZE(mlxsw_sp1_trap_items_arr);
+
+ return 0;
+}
+
const struct mlxsw_sp_trap_ops mlxsw_sp1_trap_ops = {
.groups_init = mlxsw_sp1_trap_groups_init,
+ .traps_init = mlxsw_sp1_traps_init,
};
static const struct mlxsw_sp_trap_group_item
mlxsw_sp2_trap_group_items_arr[] = {
};
+static const struct mlxsw_sp_trap_item
+mlxsw_sp2_trap_items_arr[] = {
+};
+
static int
mlxsw_sp2_trap_groups_init(struct mlxsw_sp *mlxsw_sp,
const struct mlxsw_sp_trap_group_item **arr,
@@ -1647,6 +1699,17 @@ mlxsw_sp2_trap_groups_init(struct mlxsw_sp *mlxsw_sp,
return 0;
}
+static int mlxsw_sp2_traps_init(struct mlxsw_sp *mlxsw_sp,
+ const struct mlxsw_sp_trap_item **arr,
+ size_t *p_traps_count)
+{
+ *arr = mlxsw_sp2_trap_items_arr;
+ *p_traps_count = ARRAY_SIZE(mlxsw_sp2_trap_items_arr);
+
+ return 0;
+}
+
const struct mlxsw_sp_trap_ops mlxsw_sp2_trap_ops = {
.groups_init = mlxsw_sp2_trap_groups_init,
+ .traps_init = mlxsw_sp2_traps_init,
};
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
index 4ae5212b9a48..b8df684bedef 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.h
@@ -27,6 +27,9 @@ struct mlxsw_sp_trap_ops {
int (*groups_init)(struct mlxsw_sp *mlxsw_sp,
const struct mlxsw_sp_trap_group_item **arr,
size_t *p_groups_count);
+ int (*traps_init)(struct mlxsw_sp *mlxsw_sp,
+ const struct mlxsw_sp_trap_item **arr,
+ size_t *p_traps_count);
};
extern const struct mlxsw_sp_trap_ops mlxsw_sp1_trap_ops;
--
2.26.2
^ permalink raw reply related
* [PATCH net-next 7/9] mlxsw: spectrum_trap: Add early_drop trap
From: Ido Schimmel @ 2020-08-03 16:11 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, amitc, mlxsw, Ido Schimmel
In-Reply-To: <20200803161141.2523857-1-idosch@idosch.org>
From: Ido Schimmel <idosch@mellanox.com>
As previously explained, packets that are dropped due to buffer related
reasons (e.g., tail drop, early drop) can be mirrored to the CPU port.
These packets are then trapped with one of the "mirror session" traps
and their CQE includes the reason for which the packet was mirrored.
Register with devlink a new trap, early_drop, and initialize the
corresponding Rx listener with the appropriate mirror reason. Return an
error in case user tries to change the traps' action, as this is not
supported.
Since Spectrum-1 does not support these traps, the above is only done
for Spectrum-2 onwards.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/core.h | 13 ++++--
drivers/net/ethernet/mellanox/mlxsw/reg.h | 1 +
.../ethernet/mellanox/mlxsw/spectrum_trap.c | 42 +++++++++++++++++++
3 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 219ce89e629a..11af3308f8cc 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -89,13 +89,15 @@ struct mlxsw_listener {
};
#define __MLXSW_RXL(_func, _trap_id, _en_action, _is_ctrl, _en_trap_group, \
- _dis_action, _enabled_on_register, _dis_trap_group) \
+ _dis_action, _enabled_on_register, _dis_trap_group, \
+ _mirror_reason) \
{ \
.trap_id = MLXSW_TRAP_ID_##_trap_id, \
.rx_listener = \
{ \
.func = _func, \
.local_port = MLXSW_PORT_DONT_CARE, \
+ .mirror_reason = _mirror_reason, \
.trap_id = MLXSW_TRAP_ID_##_trap_id, \
}, \
.en_action = MLXSW_REG_HPKT_ACTION_##_en_action, \
@@ -109,12 +111,17 @@ struct mlxsw_listener {
#define MLXSW_RXL(_func, _trap_id, _en_action, _is_ctrl, _trap_group, \
_dis_action) \
__MLXSW_RXL(_func, _trap_id, _en_action, _is_ctrl, _trap_group, \
- _dis_action, true, _trap_group)
+ _dis_action, true, _trap_group, 0)
#define MLXSW_RXL_DIS(_func, _trap_id, _en_action, _is_ctrl, _en_trap_group, \
_dis_action, _dis_trap_group) \
__MLXSW_RXL(_func, _trap_id, _en_action, _is_ctrl, _en_trap_group, \
- _dis_action, false, _dis_trap_group)
+ _dis_action, false, _dis_trap_group, 0)
+
+#define MLXSW_RXL_MIRROR(_func, _session_id, _trap_group, _mirror_reason) \
+ __MLXSW_RXL(_func, MIRROR_SESSION##_session_id, TRAP_TO_CPU, false, \
+ _trap_group, TRAP_TO_CPU, true, _trap_group, \
+ _mirror_reason)
#define MLXSW_EVENTL(_func, _trap_id, _trap_group) \
{ \
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 28a2576eb783..079b080de7f7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -5614,6 +5614,7 @@ enum mlxsw_reg_htgt_trap_group {
MLXSW_REG_HTGT_TRAP_GROUP_SP_L3_EXCEPTIONS,
MLXSW_REG_HTGT_TRAP_GROUP_SP_TUNNEL_DISCARDS,
MLXSW_REG_HTGT_TRAP_GROUP_SP_ACL_DISCARDS,
+ MLXSW_REG_HTGT_TRAP_GROUP_SP_BUFFER_DISCARDS,
__MLXSW_REG_HTGT_TRAP_GROUP_MAX,
MLXSW_REG_HTGT_TRAP_GROUP_MAX = __MLXSW_REG_HTGT_TRAP_GROUP_MAX - 1
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index 93dd88abbe23..16bf154076b3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -21,6 +21,7 @@ struct mlxsw_sp_trap_group_item {
struct devlink_trap_group group;
u16 hw_group_id;
u8 priority;
+ u8 fixed_policer:1; /* Whether policer binding can change */
};
#define MLXSW_SP_TRAP_LISTENERS_MAX 3
@@ -28,6 +29,7 @@ struct mlxsw_sp_trap_group_item {
struct mlxsw_sp_trap_item {
struct devlink_trap trap;
struct mlxsw_listener listeners_arr[MLXSW_SP_TRAP_LISTENERS_MAX];
+ u8 is_source:1;
};
/* All driver-specific traps must be documented in
@@ -46,6 +48,11 @@ enum {
#define MLXSW_SP_TRAP_METADATA DEVLINK_TRAP_METADATA_TYPE_F_IN_PORT
+enum {
+ /* Packet was early dropped. */
+ MLXSW_SP_MIRROR_REASON_INGRESS_WRED = 9,
+};
+
static int mlxsw_sp_rx_listener(struct mlxsw_sp *mlxsw_sp, struct sk_buff *skb,
u8 local_port,
struct mlxsw_sp_port *mlxsw_sp_port)
@@ -222,6 +229,11 @@ static void mlxsw_sp_rx_sample_listener(struct sk_buff *skb, u8 local_port,
DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id, \
MLXSW_SP_TRAP_METADATA | (_metadata))
+#define MLXSW_SP_TRAP_BUFFER_DROP(_id) \
+ DEVLINK_TRAP_GENERIC(DROP, TRAP, _id, \
+ DEVLINK_TRAP_GROUP_GENERIC_ID_BUFFER_DROPS, \
+ MLXSW_SP_TRAP_METADATA)
+
#define MLXSW_SP_TRAP_DRIVER_DROP(_id, _group_id) \
DEVLINK_TRAP_DRIVER(DROP, DROP, DEVLINK_MLXSW_TRAP_ID_##_id, \
DEVLINK_MLXSW_TRAP_NAME_##_id, \
@@ -248,6 +260,10 @@ static void mlxsw_sp_rx_sample_listener(struct sk_buff *skb, u8 local_port,
TRAP_EXCEPTION_TO_CPU, false, SP_##_en_group_id, \
SET_FW_DEFAULT, SP_##_dis_group_id)
+#define MLXSW_SP_RXL_BUFFER_DISCARD(_mirror_reason) \
+ MLXSW_RXL_MIRROR(mlxsw_sp_rx_drop_listener, 0, SP_BUFFER_DISCARDS, \
+ MLXSW_SP_MIRROR_REASON_##_mirror_reason)
+
#define MLXSW_SP_RXL_EXCEPTION(_id, _group_id, _action) \
MLXSW_RXL(mlxsw_sp_rx_mark_listener, _id, \
_action, false, SP_##_group_id, SET_FW_DEFAULT)
@@ -331,6 +347,9 @@ mlxsw_sp_trap_policer_items_arr[] = {
{
.policer = MLXSW_SP_TRAP_POLICER(19, 1024, 512),
},
+ {
+ .policer = MLXSW_SP_TRAP_POLICER(20, 10240, 4096),
+ },
};
static const struct mlxsw_sp_trap_group_item mlxsw_sp_trap_group_items_arr[] = {
@@ -1429,6 +1448,11 @@ int mlxsw_sp_trap_action_set(struct mlxsw_core *mlxsw_core,
if (WARN_ON(!trap_item))
return -EINVAL;
+ if (trap_item->is_source) {
+ NL_SET_ERR_MSG_MOD(extack, "Changing the action of source traps is not supported");
+ return -EOPNOTSUPP;
+ }
+
for (i = 0; i < MLXSW_SP_TRAP_LISTENERS_MAX; i++) {
const struct mlxsw_listener *listener;
bool enabled;
@@ -1470,6 +1494,11 @@ __mlxsw_sp_trap_group_init(struct mlxsw_core *mlxsw_core,
if (WARN_ON(!group_item))
return -EINVAL;
+ if (group_item->fixed_policer && policer_id != group->init_policer_id) {
+ NL_SET_ERR_MSG_MOD(extack, "Changing the policer binding of this group is not supported");
+ return -EOPNOTSUPP;
+ }
+
if (policer_id) {
struct mlxsw_sp_trap_policer_item *policer_item;
@@ -1682,10 +1711,23 @@ const struct mlxsw_sp_trap_ops mlxsw_sp1_trap_ops = {
static const struct mlxsw_sp_trap_group_item
mlxsw_sp2_trap_group_items_arr[] = {
+ {
+ .group = DEVLINK_TRAP_GROUP_GENERIC(BUFFER_DROPS, 20),
+ .hw_group_id = MLXSW_REG_HTGT_TRAP_GROUP_SP_BUFFER_DISCARDS,
+ .priority = 0,
+ .fixed_policer = true,
+ },
};
static const struct mlxsw_sp_trap_item
mlxsw_sp2_trap_items_arr[] = {
+ {
+ .trap = MLXSW_SP_TRAP_BUFFER_DROP(EARLY_DROP),
+ .listeners_arr = {
+ MLXSW_SP_RXL_BUFFER_DISCARD(INGRESS_WRED),
+ },
+ .is_source = true,
+ },
};
static int
--
2.26.2
^ permalink raw reply related
* [PATCH net-next 8/9] mlxsw: spectrum_qdisc: Offload action trap for qevents
From: Ido Schimmel @ 2020-08-03 16:11 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, amitc, mlxsw, Ido Schimmel
In-Reply-To: <20200803161141.2523857-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
When offloading action trap on a qevent, pass to_dev of NULL to the SPAN
module to trigger the mirror to the CPU port. Query the buffer drops
policer and use it for policing of the trapped traffic.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum.h | 7 ++
.../ethernet/mellanox/mlxsw/spectrum_qdisc.c | 75 +++++++++++++++----
.../ethernet/mellanox/mlxsw/spectrum_trap.c | 26 +++++++
3 files changed, 95 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index b808f6b4d670..f9ba59641b4d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -984,6 +984,10 @@ struct mlxsw_sp_mall_mirror_entry {
int span_id;
};
+struct mlxsw_sp_mall_trap_entry {
+ int span_id;
+};
+
struct mlxsw_sp_mall_entry {
struct list_head list;
unsigned long cookie;
@@ -992,6 +996,7 @@ struct mlxsw_sp_mall_entry {
bool ingress;
union {
struct mlxsw_sp_mall_mirror_entry mirror;
+ struct mlxsw_sp_mall_trap_entry trap;
struct mlxsw_sp_port_sample sample;
};
struct rcu_head rcu;
@@ -1199,6 +1204,8 @@ int
mlxsw_sp_trap_policer_counter_get(struct mlxsw_core *mlxsw_core,
const struct devlink_trap_policer *policer,
u64 *p_drops);
+int mlxsw_sp_trap_group_policer_hw_id_get(struct mlxsw_sp *mlxsw_sp, u16 id,
+ bool *p_enabled, u16 *p_hw_id);
static inline struct net *mlxsw_sp_net(struct mlxsw_sp *mlxsw_sp)
{
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
index a5ce1eec5418..964fd444bb10 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_qdisc.c
@@ -1289,19 +1289,18 @@ struct mlxsw_sp_qevent_binding {
static LIST_HEAD(mlxsw_sp_qevent_block_cb_list);
-static int mlxsw_sp_qevent_mirror_configure(struct mlxsw_sp *mlxsw_sp,
- struct mlxsw_sp_mall_entry *mall_entry,
- struct mlxsw_sp_qevent_binding *qevent_binding)
+static int mlxsw_sp_qevent_span_configure(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_mall_entry *mall_entry,
+ struct mlxsw_sp_qevent_binding *qevent_binding,
+ const struct mlxsw_sp_span_agent_parms *agent_parms,
+ int *p_span_id)
{
struct mlxsw_sp_port *mlxsw_sp_port = qevent_binding->mlxsw_sp_port;
struct mlxsw_sp_span_trigger_parms trigger_parms = {};
- struct mlxsw_sp_span_agent_parms agent_parms = {
- .to_dev = mall_entry->mirror.to_dev,
- };
int span_id;
int err;
- err = mlxsw_sp_span_agent_get(mlxsw_sp, &span_id, &agent_parms);
+ err = mlxsw_sp_span_agent_get(mlxsw_sp, &span_id, agent_parms);
if (err)
return err;
@@ -1320,7 +1319,7 @@ static int mlxsw_sp_qevent_mirror_configure(struct mlxsw_sp *mlxsw_sp,
if (err)
goto err_trigger_enable;
- mall_entry->mirror.span_id = span_id;
+ *p_span_id = span_id;
return 0;
err_trigger_enable:
@@ -1333,13 +1332,13 @@ static int mlxsw_sp_qevent_mirror_configure(struct mlxsw_sp *mlxsw_sp,
return err;
}
-static void mlxsw_sp_qevent_mirror_deconfigure(struct mlxsw_sp *mlxsw_sp,
- struct mlxsw_sp_mall_entry *mall_entry,
- struct mlxsw_sp_qevent_binding *qevent_binding)
+static void mlxsw_sp_qevent_span_deconfigure(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_qevent_binding *qevent_binding,
+ int span_id)
{
struct mlxsw_sp_port *mlxsw_sp_port = qevent_binding->mlxsw_sp_port;
struct mlxsw_sp_span_trigger_parms trigger_parms = {
- .span_id = mall_entry->mirror.span_id,
+ .span_id = span_id,
};
mlxsw_sp_span_trigger_disable(mlxsw_sp_port, qevent_binding->span_trigger,
@@ -1347,7 +1346,51 @@ static void mlxsw_sp_qevent_mirror_deconfigure(struct mlxsw_sp *mlxsw_sp,
mlxsw_sp_span_agent_unbind(mlxsw_sp, qevent_binding->span_trigger, mlxsw_sp_port,
&trigger_parms);
mlxsw_sp_span_analyzed_port_put(mlxsw_sp_port, true);
- mlxsw_sp_span_agent_put(mlxsw_sp, mall_entry->mirror.span_id);
+ mlxsw_sp_span_agent_put(mlxsw_sp, span_id);
+}
+
+static int mlxsw_sp_qevent_mirror_configure(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_mall_entry *mall_entry,
+ struct mlxsw_sp_qevent_binding *qevent_binding)
+{
+ struct mlxsw_sp_span_agent_parms agent_parms = {
+ .to_dev = mall_entry->mirror.to_dev,
+ };
+
+ return mlxsw_sp_qevent_span_configure(mlxsw_sp, mall_entry, qevent_binding,
+ &agent_parms, &mall_entry->mirror.span_id);
+}
+
+static void mlxsw_sp_qevent_mirror_deconfigure(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_mall_entry *mall_entry,
+ struct mlxsw_sp_qevent_binding *qevent_binding)
+{
+ mlxsw_sp_qevent_span_deconfigure(mlxsw_sp, qevent_binding, mall_entry->mirror.span_id);
+}
+
+static int mlxsw_sp_qevent_trap_configure(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_mall_entry *mall_entry,
+ struct mlxsw_sp_qevent_binding *qevent_binding)
+{
+ struct mlxsw_sp_span_agent_parms agent_parms = {};
+ int err;
+
+ err = mlxsw_sp_trap_group_policer_hw_id_get(mlxsw_sp,
+ DEVLINK_TRAP_GROUP_GENERIC_ID_BUFFER_DROPS,
+ &agent_parms.policer_enable,
+ &agent_parms.policer_id);
+ if (err)
+ return err;
+
+ return mlxsw_sp_qevent_span_configure(mlxsw_sp, mall_entry, qevent_binding,
+ &agent_parms, &mall_entry->trap.span_id);
+}
+
+static void mlxsw_sp_qevent_trap_deconfigure(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_mall_entry *mall_entry,
+ struct mlxsw_sp_qevent_binding *qevent_binding)
+{
+ mlxsw_sp_qevent_span_deconfigure(mlxsw_sp, qevent_binding, mall_entry->trap.span_id);
}
static int mlxsw_sp_qevent_entry_configure(struct mlxsw_sp *mlxsw_sp,
@@ -1357,6 +1400,8 @@ static int mlxsw_sp_qevent_entry_configure(struct mlxsw_sp *mlxsw_sp,
switch (mall_entry->type) {
case MLXSW_SP_MALL_ACTION_TYPE_MIRROR:
return mlxsw_sp_qevent_mirror_configure(mlxsw_sp, mall_entry, qevent_binding);
+ case MLXSW_SP_MALL_ACTION_TYPE_TRAP:
+ return mlxsw_sp_qevent_trap_configure(mlxsw_sp, mall_entry, qevent_binding);
default:
/* This should have been validated away. */
WARN_ON(1);
@@ -1371,6 +1416,8 @@ static void mlxsw_sp_qevent_entry_deconfigure(struct mlxsw_sp *mlxsw_sp,
switch (mall_entry->type) {
case MLXSW_SP_MALL_ACTION_TYPE_MIRROR:
return mlxsw_sp_qevent_mirror_deconfigure(mlxsw_sp, mall_entry, qevent_binding);
+ case MLXSW_SP_MALL_ACTION_TYPE_TRAP:
+ return mlxsw_sp_qevent_trap_deconfigure(mlxsw_sp, mall_entry, qevent_binding);
default:
WARN_ON(1);
return;
@@ -1490,6 +1537,8 @@ static int mlxsw_sp_qevent_mall_replace(struct mlxsw_sp *mlxsw_sp,
if (act->id == FLOW_ACTION_MIRRED) {
mall_entry->type = MLXSW_SP_MALL_ACTION_TYPE_MIRROR;
mall_entry->mirror.to_dev = act->dev;
+ } else if (act->id == FLOW_ACTION_TRAP) {
+ mall_entry->type = MLXSW_SP_MALL_ACTION_TYPE_TRAP;
} else {
NL_SET_ERR_MSG(f->common.extack, "Unsupported action");
err = -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index 16bf154076b3..2e41c5519c1b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -1675,6 +1675,32 @@ mlxsw_sp_trap_policer_counter_get(struct mlxsw_core *mlxsw_core,
return 0;
}
+int mlxsw_sp_trap_group_policer_hw_id_get(struct mlxsw_sp *mlxsw_sp, u16 id,
+ bool *p_enabled, u16 *p_hw_id)
+{
+ struct mlxsw_sp_trap_policer_item *pol_item;
+ struct mlxsw_sp_trap_group_item *gr_item;
+ u32 pol_id;
+
+ gr_item = mlxsw_sp_trap_group_item_lookup(mlxsw_sp, id);
+ if (!gr_item)
+ return -ENOENT;
+
+ pol_id = gr_item->group.init_policer_id;
+ if (!pol_id) {
+ *p_enabled = false;
+ return 0;
+ }
+
+ pol_item = mlxsw_sp_trap_policer_item_lookup(mlxsw_sp, pol_id);
+ if (WARN_ON(!pol_item))
+ return -ENOENT;
+
+ *p_enabled = true;
+ *p_hw_id = pol_item->hw_id;
+ return 0;
+}
+
static const struct mlxsw_sp_trap_group_item
mlxsw_sp1_trap_group_items_arr[] = {
};
--
2.26.2
^ permalink raw reply related
* [PATCH net-next 9/9] selftests: mlxsw: RED: Test offload of trapping on RED qevents
From: Ido Schimmel @ 2020-08-03 16:11 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, amitc, mlxsw, Ido Schimmel
In-Reply-To: <20200803161141.2523857-1-idosch@idosch.org>
From: Petr Machata <petrm@mellanox.com>
Add a selftest for RED early_drop and mark qevents when a trap action is
attached at the associated block.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../drivers/net/mlxsw/sch_red_core.sh | 35 +++++++++++++++----
.../drivers/net/mlxsw/sch_red_ets.sh | 11 ++++++
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh b/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
index 45042105ead7..517297a14ecf 100644
--- a/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
@@ -568,17 +568,12 @@ do_drop_test()
busywait 1100 until_counter_is ">= $((base + 1))" $fetch_counter >/dev/null
check_fail $? "Spurious packets observed without buffer pressure"
- qevent_rule_uninstall_$subtest
-
# Push to the queue until it's at the limit. The configured limit is
# rounded by the qdisc and then by the driver, so this is the best we
- # can do to get to the real limit of the system. Do this with the rules
- # uninstalled so that the inevitable drops don't get counted.
+ # can do to get to the real limit of the system.
build_backlog $vlan $((3 * limit / 2)) udp >/dev/null
- qevent_rule_install_$subtest
base=$($fetch_counter)
-
send_packets $vlan udp 11
now=$(busywait 1100 until_counter_is ">= $((base + 10))" $fetch_counter)
@@ -631,3 +626,31 @@ do_drop_mirror_test()
tc filter del dev $h2 ingress pref 1 handle 101 flower
}
+
+qevent_rule_install_trap()
+{
+ tc filter add block 10 pref 1234 handle 102 matchall skip_sw \
+ action trap hw_stats disabled
+}
+
+qevent_rule_uninstall_trap()
+{
+ tc filter del block 10 pref 1234 handle 102 matchall
+}
+
+qevent_counter_fetch_trap()
+{
+ local trap_name=$1; shift
+
+ devlink_trap_rx_packets_get "$trap_name"
+}
+
+do_drop_trap_test()
+{
+ local vlan=$1; shift
+ local limit=$1; shift
+ local trap_name=$1; shift
+
+ do_drop_test "$vlan" "$limit" "$trap_name" trap \
+ "qevent_counter_fetch_trap $trap_name"
+}
diff --git a/tools/testing/selftests/drivers/net/mlxsw/sch_red_ets.sh b/tools/testing/selftests/drivers/net/mlxsw/sch_red_ets.sh
index c8968b041bea..3f007c5f8361 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/sch_red_ets.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/sch_red_ets.sh
@@ -8,6 +8,7 @@ ALL_TESTS="
red_test
mc_backlog_test
red_mirror_test
+ red_trap_test
"
: ${QDISC:=ets}
source sch_red_core.sh
@@ -94,6 +95,16 @@ red_mirror_test()
uninstall_qdisc
}
+red_trap_test()
+{
+ install_qdisc qevent early_drop block 10
+
+ do_drop_trap_test 10 $BACKLOG1 early_drop
+ do_drop_trap_test 11 $BACKLOG2 early_drop
+
+ uninstall_qdisc
+}
+
trap cleanup EXIT
setup_prepare
--
2.26.2
^ permalink raw reply related
* Re: [iproute2-next v2 5/5] devlink: support setting the overwrite mask
From: Jiri Pirko @ 2020-08-03 16:20 UTC (permalink / raw)
To: David Ahern; +Cc: Jacob Keller, netdev
In-Reply-To: <0bb895a2-e233-0426-3e48-d8422fa5b7cf@gmail.com>
Mon, Aug 03, 2020 at 05:53:16PM CEST, dsahern@gmail.com wrote:
>On 7/31/20 6:21 PM, Jacob Keller wrote:
>> Add support for specifying the overwrite sections to allow in the flash
>> update command. This is done by adding a new "overwrite" option which
>> can take either "settings" or "identifiers" passing the overwrite mode
>> multiple times will combine the fields using bitwise-OR.
>>
>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>> ---
>> devlink/devlink.c | 37 +++++++++++++++++++++++++++++++++++--
>> 1 file changed, 35 insertions(+), 2 deletions(-)
>>
>
>5/5? I only see 2 - 4/5 and 5/5. Please re-send against latest
>iproute2-next.
1-3 are kernel.
>
^ permalink raw reply
* Re: [PATCH net-next] fib: Fix undef compile warning
From: Brian Vazquez @ 2020-08-03 16:21 UTC (permalink / raw)
To: YueHaibing; +Cc: David S . Miller, kuba, Randy Dunlap, Linux NetDev, open list
In-Reply-To: <20200803131948.41736-1-yuehaibing@huawei.com>
Acked-By: Brian Vazquez <brianvv@google.com>
On Mon, Aug 3, 2020 at 6:20 AM YueHaibing <yuehaibing@huawei.com> wrote:
>
> net/core/fib_rules.c:26:7: warning: "CONFIG_IP_MULTIPLE_TABLES" is not defined, evaluates to 0 [-Wundef]
> #elif CONFIG_IP_MULTIPLE_TABLES
> ^~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fixes: 8b66a6fd34f5 ("fib: fix another fib_rules_ops indirect call wrapper problem")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> net/core/fib_rules.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
> index a7a3f500a857..51678a528f85 100644
> --- a/net/core/fib_rules.c
> +++ b/net/core/fib_rules.c
> @@ -23,7 +23,7 @@
> #else
> #define INDIRECT_CALL_MT(f, f2, f1, ...) INDIRECT_CALL_1(f, f2, __VA_ARGS__)
> #endif
> -#elif CONFIG_IP_MULTIPLE_TABLES
> +#elif defined(CONFIG_IP_MULTIPLE_TABLES)
> #define INDIRECT_CALL_MT(f, f2, f1, ...) INDIRECT_CALL_1(f, f1, __VA_ARGS__)
> #else
> #define INDIRECT_CALL_MT(f, f2, f1, ...) f(__VA_ARGS__)
> --
> 2.17.1
>
>
^ permalink raw reply
* KMSAN: uninit-value in caif_seqpkt_sendmsg
From: syzbot @ 2020-08-03 16:35 UTC (permalink / raw)
To: alexios.zavras, allison, davem, edumazet, glider, gregkh, kuba,
linux-kernel, netdev, swinslow, syzkaller-bugs, tglx
Hello,
syzbot found the following issue on:
HEAD commit: 8bbbc5cf kmsan: don't compile memmove
git tree: https://github.com/google/kmsan.git master
console output: https://syzkaller.appspot.com/x/log.txt?x=11fbfe09e00000
kernel config: https://syzkaller.appspot.com/x/.config?x=cd0e9a6b0e555cc3
dashboard link: https://syzkaller.appspot.com/bug?extid=09a5d591c1f98cf5efcb
compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
userspace arch: i386
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=150ef74ee00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=170e2109e00000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+09a5d591c1f98cf5efcb@syzkaller.appspotmail.com
=====================================================
BUG: KMSAN: uninit-value in caif_seqpkt_sendmsg+0x693/0xf60 net/caif/caif_socket.c:542
CPU: 1 PID: 11244 Comm: syz-executor620 Not tainted 5.6.0-rc2-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x220 lib/dump_stack.c:118
kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:118
__msan_warning+0x58/0xa0 mm/kmsan/kmsan_instr.c:215
caif_seqpkt_sendmsg+0x693/0xf60 net/caif/caif_socket.c:542
sock_sendmsg_nosec net/socket.c:652 [inline]
sock_sendmsg net/socket.c:672 [inline]
____sys_sendmsg+0x12b6/0x1350 net/socket.c:2343
___sys_sendmsg net/socket.c:2397 [inline]
__sys_sendmmsg+0x808/0xc90 net/socket.c:2480
__compat_sys_sendmmsg net/compat.c:656 [inline]
__do_compat_sys_sendmmsg net/compat.c:663 [inline]
__se_compat_sys_sendmmsg net/compat.c:660 [inline]
__ia32_compat_sys_sendmmsg+0x127/0x180 net/compat.c:660
do_syscall_32_irqs_on arch/x86/entry/common.c:339 [inline]
do_fast_syscall_32+0x3c7/0x6e0 arch/x86/entry/common.c:410
entry_SYSENTER_compat+0x68/0x77 arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7f79d99
Code: 90 e8 0b 00 00 00 f3 90 0f ae e8 eb f9 8d 74 26 00 89 3c 24 c3 90 90 90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 002b:00000000ffbf4d6c EFLAGS: 00000292 ORIG_RAX: 0000000000000159
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000020007600
RDX: 0000000000000001 RSI: 0000000000000000 RDI: 00000000080bb508
RBP: 0000000000000012 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
Local variable ----iovstack.i@__sys_sendmmsg created at:
___sys_sendmsg net/socket.c:2388 [inline]
__sys_sendmmsg+0x6db/0xc90 net/socket.c:2480
___sys_sendmsg net/socket.c:2388 [inline]
__sys_sendmmsg+0x6db/0xc90 net/socket.c:2480
=====================================================
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH bpf-next v3 00/29] bpf: switch to memcg-based memory accounting
From: Daniel Borkmann @ 2020-08-03 16:39 UTC (permalink / raw)
To: Roman Gushchin; +Cc: bpf, netdev, Alexei Starovoitov, kernel-team, linux-kernel
In-Reply-To: <20200803153449.GA1020566@carbon.DHCP.thefacebook.com>
On 8/3/20 5:34 PM, Roman Gushchin wrote:
> On Mon, Aug 03, 2020 at 02:05:29PM +0200, Daniel Borkmann wrote:
>> On 7/30/20 11:22 PM, Roman Gushchin wrote:
>>> Currently bpf is using the memlock rlimit for the memory accounting.
>>> This approach has its downsides and over time has created a significant
>>> amount of problems:
>>>
>>> 1) The limit is per-user, but because most bpf operations are performed
>>> as root, the limit has a little value.
>>>
>>> 2) It's hard to come up with a specific maximum value. Especially because
>>> the counter is shared with non-bpf users (e.g. memlock() users).
>>> Any specific value is either too low and creates false failures
>>> or too high and useless.
>>>
>>> 3) Charging is not connected to the actual memory allocation. Bpf code
>>> should manually calculate the estimated cost and precharge the counter,
>>> and then take care of uncharging, including all fail paths.
>>> It adds to the code complexity and makes it easy to leak a charge.
>>>
>>> 4) There is no simple way of getting the current value of the counter.
>>> We've used drgn for it, but it's far from being convenient.
>>>
>>> 5) Cryptic -EPERM is returned on exceeding the limit. Libbpf even had
>>> a function to "explain" this case for users.
>>>
>>> In order to overcome these problems let's switch to the memcg-based
>>> memory accounting of bpf objects. With the recent addition of the percpu
>>> memory accounting, now it's possible to provide a comprehensive accounting
>>> of memory used by bpf programs and maps.
>>>
>>> This approach has the following advantages:
>>> 1) The limit is per-cgroup and hierarchical. It's way more flexible and allows
>>> a better control over memory usage by different workloads.
>>>
>>> 2) The actual memory consumption is taken into account. It happens automatically
>>> on the allocation time if __GFP_ACCOUNT flags is passed. Uncharging is also
>>> performed automatically on releasing the memory. So the code on the bpf side
>>> becomes simpler and safer.
>>>
>>> 3) There is a simple way to get the current value and statistics.
>>>
>>> The patchset consists of the following parts:
>>> 1) memcg-based accounting for various bpf objects: progs and maps
>>> 2) removal of the rlimit-based accounting
>>> 3) removal of rlimit adjustments in userspace samples
>
>> The diff stat looks nice & agree that rlimit sucks, but I'm missing how this is set
>> is supposed to work reliably, at least I currently fail to see it. Elaborating on this
>> in more depth especially for the case of unprivileged users should be a /fundamental/
>> part of the commit message.
>>
>> Lets take an example: unprivileged user adds a max sized hashtable to one of its
>> programs, and configures the map that it will perform runtime allocation. The load
>> succeeds as it doesn't surpass the limits set for the current memcg. Kernel then
>> processes packets from softirq. Given the runtime allocations, we end up mischarging
>> to whoever ended up triggering __do_softirq(). If, for example, ksoftirq thread, then
>> it's probably reasonable to assume that this might not be accounted e.g. limits are
>> not imposed on the root cgroup. If so we would probably need to drag the context of
>> /where/ this must be charged to __memcg_kmem_charge_page() to do it reliably. Otherwise
>> how do you protect unprivileged users to OOM the machine?
>
> this is a valid concern, thank you for bringing it in. It can be resolved by
> associating a map with a memory cgroup on creation, so that we can charge
> this memory cgroup later, even from a soft-irq context. The question here is
> whether we want to do it for all maps, or just for dynamic hashtables
> (or any similar cases, if there are any)? I think the second option
> is better. With the first option we have to annotate all memory allocations
> in bpf maps code with memalloc_use_memcg()/memalloc_unuse_memcg(),
> so it's easy to mess it up in the future.
> What do you think?
We would need to do it for all maps that are configured with non-prealloc, e.g. not
only hash/LRU table but also others like LPM maps etc. I wonder whether program entry/
exit could do the memalloc_use_memcg() / memalloc_unuse_memcg() and then everything
would be accounted against the prog's memcg from runtime side, but then there's the
usual issue with 'unuse'-restore on tail calls, and it doesn't solve the syscall side.
But seems like the memalloc_{use,unuse}_memcg()'s remote charging is lightweight
anyway compared to some of the other map update work such as taking bucket lock etc.
>> Similarly, what happens to unprivileged users if kmemcg was not configured into the
>> kernel or has been disabled?
>
> Well, I don't think we can address it. Memcg-based memory accounting requires
> enabled memory cgroups, a properly configured cgroup tree and also the kernel
> memory accounting turned on to function properly.
> Because we at Facebook are using cgroup for the memory accounting and control
> everywhere, I might be biased. If there are real !memcg systems which are
> actively using non-privileged bpf, we should keep the old system in place
> and make it optional, so everyone can choose between having both accounting
> systems or just the new one. Or we can disable the rlimit-based accounting
> for root. But eliminating it completely looks so much nicer to me.
Eliminating it entirely feels better indeed. Another option could be that BPF kconfig
would select memcg, so it's always built with it. Perhaps that is an acceptable tradeoff.
Thanks,
Daniel
^ permalink raw reply
* [PATCH net] mptcp: fix bogus sendmsg() return code under pressure
From: Paolo Abeni @ 2020-08-03 16:40 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, mptcp
In case of memory pressure, mptcp_sendmsg() may call
sk_stream_wait_memory() after succesfully xmitting some
bytes. If the latter fails we currently return to the
user-space the error code, ignoring the succeful xmit.
Address the issue always checking for the xmitted bytes
before mptcp_sendmsg() completes.
Fixes: f296234c98a8 ("mptcp: Add handling of incoming MP_JOIN requests")
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/protocol.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c0abe738e7d3..a761d3c613bb 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -880,7 +880,6 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
mptcp_set_timeout(sk, ssk);
if (copied) {
- ret = copied;
tcp_push(ssk, msg->msg_flags, mss_now, tcp_sk(ssk)->nonagle,
size_goal);
@@ -893,7 +892,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
release_sock(ssk);
out:
release_sock(sk);
- return ret;
+ return copied ? : ret;
}
static void mptcp_wait_data(struct sock *sk, long *timeo)
--
2.26.2
^ permalink raw reply related
* Re: KMSAN: uninit-value in caif_seqpkt_sendmsg
From: Eric Dumazet @ 2020-08-03 16:41 UTC (permalink / raw)
To: syzbot, alexios.zavras, allison, davem, edumazet, glider, gregkh,
kuba, linux-kernel, netdev, swinslow, syzkaller-bugs, tglx
In-Reply-To: <0000000000007ab98a05abfbb9a7@google.com>
On 8/3/20 9:35 AM, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 8bbbc5cf kmsan: don't compile memmove
> git tree: https://github.com/google/kmsan.git master
> console output: https://syzkaller.appspot.com/x/log.txt?x=11fbfe09e00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=cd0e9a6b0e555cc3
> dashboard link: https://syzkaller.appspot.com/bug?extid=09a5d591c1f98cf5efcb
> compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
> userspace arch: i386
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=150ef74ee00000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=170e2109e00000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+09a5d591c1f98cf5efcb@syzkaller.appspotmail.com
>
> =====================================================
> BUG: KMSAN: uninit-value in caif_seqpkt_sendmsg+0x693/0xf60 net/caif/caif_socket.c:542
> CPU: 1 PID: 11244 Comm: syz-executor620 Not tainted 5.6.0-rc2-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1c9/0x220 lib/dump_stack.c:118
> kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:118
> __msan_warning+0x58/0xa0 mm/kmsan/kmsan_instr.c:215
> caif_seqpkt_sendmsg+0x693/0xf60 net/caif/caif_socket.c:542
> sock_sendmsg_nosec net/socket.c:652 [inline]
> sock_sendmsg net/socket.c:672 [inline]
> ____sys_sendmsg+0x12b6/0x1350 net/socket.c:2343
> ___sys_sendmsg net/socket.c:2397 [inline]
> __sys_sendmmsg+0x808/0xc90 net/socket.c:2480
> __compat_sys_sendmmsg net/compat.c:656 [inline]
> __do_compat_sys_sendmmsg net/compat.c:663 [inline]
> __se_compat_sys_sendmmsg net/compat.c:660 [inline]
> __ia32_compat_sys_sendmmsg+0x127/0x180 net/compat.c:660
> do_syscall_32_irqs_on arch/x86/entry/common.c:339 [inline]
> do_fast_syscall_32+0x3c7/0x6e0 arch/x86/entry/common.c:410
> entry_SYSENTER_compat+0x68/0x77 arch/x86/entry/entry_64_compat.S:139
> RIP: 0023:0xf7f79d99
> Code: 90 e8 0b 00 00 00 f3 90 0f ae e8 eb f9 8d 74 26 00 89 3c 24 c3 90 90 90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
> RSP: 002b:00000000ffbf4d6c EFLAGS: 00000292 ORIG_RAX: 0000000000000159
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000020007600
> RDX: 0000000000000001 RSI: 0000000000000000 RDI: 00000000080bb508
> RBP: 0000000000000012 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
>
> Local variable ----iovstack.i@__sys_sendmmsg created at:
> ___sys_sendmsg net/socket.c:2388 [inline]
> __sys_sendmmsg+0x6db/0xc90 net/socket.c:2480
> ___sys_sendmsg net/socket.c:2388 [inline]
> __sys_sendmmsg+0x6db/0xc90 net/socket.c:2480
> =====================================================
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> syzbot can test patches for this issue, for details see:
> https://goo.gl/tpsmEJ#testing-patches
>
It is not clear why this code tests iov_base instead of len.
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 3ad0a1df6712834a7f70b21a78173a02c7cba897..e4540023a9404d44089ff11b8b912fa23d8de69f 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -539,7 +539,7 @@ static int caif_seqpkt_sendmsg(struct socket *sock, struct msghdr *msg,
goto err;
ret = -EINVAL;
- if (unlikely(msg->msg_iter.iov->iov_base == NULL))
+ if (unlikely(!len))
goto err;
noblock = msg->msg_flags & MSG_DONTWAIT;
^ permalink raw reply related
* Re: [PATCH net-next] mptcp: use mptcp_for_each_subflow in mptcp_stream_accept
From: Paolo Abeni @ 2020-08-03 16:42 UTC (permalink / raw)
To: Geliang Tang, Mat Martineau, Matthieu Baerts, David S. Miller,
Jakub Kicinski
Cc: netdev, mptcp, linux-kernel
In-Reply-To: <fe531e58a52eae5aa46dd93d30d623f8862c3d09.1596459430.git.geliangtang@gmail.com>
On Mon, 2020-08-03 at 21:00 +0800, Geliang Tang wrote:
> Use mptcp_for_each_subflow in mptcp_stream_accept instead of
> open-coding.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
> net/mptcp/protocol.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index d3fe7296e1c9..400824eabf73 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2249,7 +2249,7 @@ static int mptcp_stream_accept(struct socket *sock, struct socket *newsock,
> * This is needed so NOSPACE flag can be set from tcp stack.
> */
> __mptcp_flush_join_list(msk);
> - list_for_each_entry(subflow, &msk->conn_list, node) {
> + mptcp_for_each_subflow(msk, subflow) {
> struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
>
> if (!ssk->sk_socket)
Acked-by: Paolo Abeni <pabeni@redhat.com>
^ permalink raw reply
* [PATCH net-next] net: dsa: sja1105: use detected device id instead of DT one on mismatch
From: Vladimir Oltean @ 2020-08-03 16:48 UTC (permalink / raw)
To: netdev, davem; +Cc: andrew, f.fainelli, vivien.didelot
Although we can detect the chip revision 100% at runtime, it is useful
to specify it in the device tree compatible string too, because
otherwise there would be no way to assess the correctness of device tree
bindings statically, without booting a board (only some switch versions
have internal RGMII delays and/or an SGMII port).
But for testing the P/Q/R/S support, what I have is a reworked board
with the SJA1105T replaced by a pin-compatible SJA1105Q, and I don't
want to keep a separate device tree blob just for this one-off board.
Since just the chip has been replaced, its RGMII delay setup is
inherently the same (meaning: delays added by the PHY on the slave
ports, and by PCB traces on the fixed-link CPU port).
For this board, I'd rather have the driver shout at me, but go ahead and
use what it found even if it doesn't match what it's been told is there.
[ 2.970826] sja1105 spi0.1: Device tree specifies chip SJA1105T but found SJA1105Q, please fix it!
[ 2.980010] sja1105 spi0.1: Probed switch chip: SJA1105Q
[ 3.005082] sja1105 spi0.1: Enabled switch tagging
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 35 ++++++++++++++++++--------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 5079e4aeef80..c3f6f124e5f0 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -3391,11 +3391,14 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
.devlink_param_set = sja1105_devlink_param_set,
};
+static const struct of_device_id sja1105_dt_ids[];
+
static int sja1105_check_device_id(struct sja1105_private *priv)
{
const struct sja1105_regs *regs = priv->info->regs;
u8 prod_id[SJA1105_SIZE_DEVICE_ID] = {0};
struct device *dev = &priv->spidev->dev;
+ const struct of_device_id *match;
u32 device_id;
u64 part_no;
int rc;
@@ -3405,12 +3408,6 @@ static int sja1105_check_device_id(struct sja1105_private *priv)
if (rc < 0)
return rc;
- if (device_id != priv->info->device_id) {
- dev_err(dev, "Expected device ID 0x%llx but read 0x%x\n",
- priv->info->device_id, device_id);
- return -ENODEV;
- }
-
rc = sja1105_xfer_buf(priv, SPI_READ, regs->prod_id, prod_id,
SJA1105_SIZE_DEVICE_ID);
if (rc < 0)
@@ -3418,13 +3415,29 @@ static int sja1105_check_device_id(struct sja1105_private *priv)
sja1105_unpack(prod_id, &part_no, 19, 4, SJA1105_SIZE_DEVICE_ID);
- if (part_no != priv->info->part_no) {
- dev_err(dev, "Expected part number 0x%llx but read 0x%llx\n",
- priv->info->part_no, part_no);
- return -ENODEV;
+ for (match = sja1105_dt_ids; match->compatible; match++) {
+ const struct sja1105_info *info = match->data;
+
+ /* Is what's been probed in our match table at all? */
+ if (info->device_id != device_id || info->part_no != part_no)
+ continue;
+
+ /* But is it what's in the device tree? */
+ if (priv->info->device_id != device_id ||
+ priv->info->part_no != part_no) {
+ dev_warn(dev, "Device tree specifies chip %s but found %s, please fix it!\n",
+ priv->info->name, info->name);
+ /* It isn't. No problem, pick that up. */
+ priv->info = info;
+ }
+
+ return 0;
}
- return 0;
+ dev_err(dev, "Unexpected {device ID, part number}: 0x%x 0x%llx\n",
+ device_id, part_no);
+
+ return -ENODEV;
}
static int sja1105_probe(struct spi_device *spi)
--
2.25.1
^ permalink raw reply related
* Re: [net-next v2 0/5] devlink flash update overwrite mask
From: Jacob Keller @ 2020-08-03 16:51 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, Jiri Pirko, Jakub Kicinski, Jonathan Corbet, Michael Chan,
Bin Luo, Saeed Mahameed, Leon Romanovsky, Ido Schimmel,
Danielle Ratson
In-Reply-To: <20200803152800.GC2290@nanopsycho>
On 8/3/2020 8:28 AM, Jiri Pirko wrote:
>
> I'm missing examples in the cover letter. It is much easier to
> understand the nature of the patchset with examples. Could you please
> repost with them?
>
> Thanks!
>
I'm not sure what you're asking for here. If by example of the command
line interface in devlink, there are some in the tests for netdevsim
which I could copy to the cover letter I guess?
^ permalink raw reply
* Re: [net-next v2 2/5] devlink: introduce flash update overwrite mask
From: Jacob Keller @ 2020-08-03 16:53 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev
In-Reply-To: <20200803153830.GD2290@nanopsycho>
On 8/3/2020 8:38 AM, Jiri Pirko wrote:
> Sat, Aug 01, 2020 at 02:21:56AM CEST, jacob.e.keller@intel.com wrote:
>
> [...]
>
>> + nla_mask = info->attrs[DEVLINK_ATTR_FLASH_UPDATE_OVERWRITE_MASK];
>> + if (nla_mask) {
>> + if (!(supported_params & DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK)) {
>> + NL_SET_ERR_MSG_ATTR(info->extack, nla_mask,
>> + "overwrite is not supported");
>> + return -EOPNOTSUPP;
>> + }
>> + params.overwrite_mask = nla_get_u32(nla_mask);
>
> It's a bitfield, should be NL_ATTR_TYPE_BITFIELD32.
>
I disagree. BITFIELD32 has both a mask and a field. This doesn't have
the notion of a mask. The bits you allow are set, the bits you don't
allow are not set. Having both a mask and a field over complicates this.
^ permalink raw reply
* Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY
From: Russell King - ARM Linux admin @ 2020-08-03 16:53 UTC (permalink / raw)
To: Madalin Bucur (OSS)
Cc: Andrew Lunn, Vikas Singh, f.fainelli@gmail.com,
hkallweit1@gmail.com, netdev@vger.kernel.org,
Calvin Johnson (OSS), kuldip dwivedi, Vikas Singh
In-Reply-To: <AM6PR04MB3976E2DFF6EAC273B7B1BEABEC4D0@AM6PR04MB3976.eurprd04.prod.outlook.com>
On Mon, Aug 03, 2020 at 02:47:41PM +0000, Madalin Bucur (OSS) wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Sent: 03 August 2020 17:10
> > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh
> > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com; hkallweit1@gmail.com;
> > netdev@vger.kernel.org; Calvin Johnson (OSS) <calvin.johnson@oss.nxp.com>;
> > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh
> > <vikas.singh@nxp.com>
> > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY
> >
> > On Mon, Aug 03, 2020 at 11:45:55AM +0000, Madalin Bucur (OSS) wrote:
> > > > -----Original Message-----
> > > > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > Sent: 03 August 2020 12:07
> > > > To: Madalin Bucur (OSS) <madalin.bucur@oss.nxp.com>
> > > > Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh
> > > > <vikas.singh@puresoftware.com>; f.fainelli@gmail.com;
> > hkallweit1@gmail.com;
> > > > netdev@vger.kernel.org; Calvin Johnson (OSS)
> > <calvin.johnson@oss.nxp.com>;
> > > > kuldip dwivedi <kuldip.dwivedi@puresoftware.com>; Vikas Singh
> > > > <vikas.singh@nxp.com>
> > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed
> > PHY
> > > >
> > > > On Mon, Aug 03, 2020 at 08:33:19AM +0000, Madalin Bucur (OSS) wrote:
> > > > > > -----Original Message-----
> > > > > > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org>
> > On
> > > > > > Behalf Of Andrew Lunn
> > > > > > Sent: 01 August 2020 18:11
> > > > > > To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > > > > > Cc: Vikas Singh <vikas.singh@puresoftware.com>;
> > f.fainelli@gmail.com;
> > > > > > hkallweit1@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS)
> > > > > > <calvin.johnson@oss.nxp.com>; kuldip dwivedi
> > > > > > <kuldip.dwivedi@puresoftware.com>; Madalin Bucur (OSS)
> > > > > > <madalin.bucur@oss.nxp.com>; Vikas Singh <vikas.singh@nxp.com>
> > > > > > Subject: Re: [PATCH 2/2] net: phy: Associate device node with
> > fixed
> > > > PHY
> > > > > >
> > > > > > On Sat, Aug 01, 2020 at 10:41:32AM +0100, Russell King - ARM Linux
> > > > admin
> > > > > > wrote:
> > > > > > > On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote:
> > > > > > > > Hi Andrew,
> > > > > > > >
> > > > > > > > Please refer to the "fman" node under
> > > > > > > > linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
> > > > > > > > I have two 10G ethernet interfaces out of which one is of
> > fixed-
> > > > link.
> > > > > > >
> > > > > > > Please do not top post.
> > > > > > >
> > > > > > > How does XGMII (which is a 10G only interface) work at 1G speed?
> > Is
> > > > > > > what is in DT itself a hack because fixed-phy doesn't support
> > 10G
> > > > > > > modes?
> > > > > >
> > > > > > My gut feeling is there is some hack going on here, which is why
> > i'm
> > > > > > being persistent at trying to understand what is actually going on
> > > > > > here.
> > > > >
> > > > > Hi Andrew,
> > > > >
> > > > > That platform used 1G fixed link there since there was no support
> > for
> > > > > 10G fixed link at the time. PHYlib could have tolerated 10G speed
> > there
> > > > > With a one-liner.
> > > >
> > > > That statement is false. It is not a "one liner". fixed-phy exposes
> > > > the settings to userspace as a Clause 22 PHY register set, and the
> > > > Clause 22 register set does not support 10G. So, a "one liner" would
> > > > just be yet another hack. Adding Clause 45 PHY emulation support
> > > > would be a huge task.
> > > >
> > > > > I understand that PHYLink is working to describe this
> > > > > Better, but it was not there at that time. Adding the dependency on
> > > > > PHYLink was not desirable as most of the users for the DPAA 1
> > platforms
> > > > > were targeting kernels before the PHYLink introduction (and last
> > I've
> > > > > looked, it's still under development, with unstable APIs so we'll
> > > > > take a look at this later, when it settles).
> > > >
> > > > I think you need to read Documentation/process/stable-api-nonsense.rst
> > > > particularly the section "Stable Kernel Source Interfaces".
> > > >
> > > > phylink is going to be under development for quite some time to come
> > > > as requirements evolve. For example, when support for QSFP interfaces
> > > > is eventually worked out, I suspect there will need to be some further
> > > > changes to the driver interface. This is completely normal.
> > > >
> > > > Now, as to the stability of the phylink API to drivers, it has in fact
> > > > been very stable - it has only changed over the course of this year to
> > > > support split PCS, a necessary step for DPAA2 and a few others. It
> > has
> > > > been around in mainline for two years, and has been around much longer
> > > > than that, and during that time it has been in mainline, the MAC
> > facing
> > > > interface has not changed until recently.
> > > >
> > > > So, I find your claim to be quite unreasonable.
> > >
> > > I see you agree that there were and there will be many changes for a
> > while,
> > > It's not a complaint, I know hot it works, it's just a decision based on
> > > required effort vs features offered vs user requirements. Lately it's
> > been
> > > time consuming to try to fix things in this area.
> >
> > No, it hasn't been time consuming. The only API changes as far as
> > drivers are concerned have been:
> >
> > 1. the change to the mac_link_up() prototype to move the setup of the
> > final link parameters out of mac_config() - and almost all of the
> > updates to users were done by me.
> >
> > 2. the addition of split PCS support, introducing new interfaces, has
> > had minimal impact on those drivers that updated in step (1).
> >
> > There have been no other changes as far as users are concerned.
> >
> > Some of the difficulty with (1) has been that users of phylink appeared
> > initially with no proper review, and consequently they got quite a lot
> > wrong. The most common error has been using state->speed, state->duplex
> > in mac_config() methods irrespective of the AN mode, which has _always_
> > since before phylink was merged into mainline, been totally unreliable.
> >
> > That leads me on to the other visible "changes" for users are concerned,
> > which may be interpreted as interface changes, but are not; they have
> > all been clarifications to the documentation, to strengthen things such
> > as "do not use state->speed and state->duplex in mac_config() for
> > various specific AN modes". Nothing has actually changed with any of
> > those clarifications.
> >
> > For example, if in in-band mode, and mac_config() uses state->speed
> > and state->duplex, then it doesn't matter which version of phylink
> > you're using, if someone issues ethtool -s ethN ..., then state->speed
> > and state->duplex will be their respective UNKNOWN values, and if you're
> > using these in that situation, you will mis-program the MAC.
> >
> > Again, that is not something that has changed. Ever. But the
> > documentation has because people just don't seem to get it, and I seemed
> > to be constantly repeating myself in review after review on the same
> > points.
> >
> > So, your assertion that the phylink API is not stable is false. It
> > has been remarkably stable over the two years that it has been around.
> > It is only natural that as the technology that a piece of code supports
> > evolves, so the code evolves with it. That is exactly what has happened
> > this year with the two changes I mention above.
> >
> > Now, if you've found it time consuming to "fix things" (unspecified what
> > "things" are) then I assert that what has needed to be fixed are things
> > that NXP have got wrong. Such as the rtnl cockups. Such as abusing
> > state->speed and state->duplex. None of that is because the interface
> > is unstable - they are down to buggy implementation on NXPs part.
> >
> > Essentially, what I'm saying is that your attempt to paint phylink as
> > being painful on the basis of interface changes is totally and utterly
> > wrong and is just an excuse to justify abusing the fixed-link code and
> > specifying things that are clearly incorrect via DT.
>
> Thank you for the distilled phylink history, it may be easier to comprehend
> with these details. I was not referring to phylink, but PHY related issues
> on the DPAA 1 platforms.
Sigh. No, you were referring to phylink. This is what you said:
> I understand that PHYLink is working to describe this
> Better, but it was not there at that time. Adding the dependency on
> PHYLink was not desirable as most of the users for the DPAA 1 platforms
> were targeting kernels before the PHYLink introduction (and last I've
> looked, it's still under development, with unstable APIs so we'll
> take a look at this later, when it settles).
This discussion stems from your misconception and incorrect statements
concerning phylink, which I am correcting in this discussion.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH v3 bpf-next] bpf/selftests: fold test_current_pid_tgid_new_ns into test_progs.
From: Andrii Nakryiko @ 2020-08-03 16:54 UTC (permalink / raw)
To: Carlos Neira
Cc: Networking, Yonghong Song, Eric W. Biederman,
Jesper Dangaard Brouer, bpf
In-Reply-To: <20200802213321.7445-1-cneirabustos@gmail.com>
On Sun, Aug 2, 2020 at 2:36 PM Carlos Neira <cneirabustos@gmail.com> wrote:
>
> Currently tests for bpf_get_ns_current_pid_tgid() are outside test_progs.
> This change folds a test case into test_progs.
>
> Changes from V2:
> - Tests are now using skeleton.
> - Test not creating a new namespace has been included in test_progs.
> - Test creating a new pid namespace has been added as a standalone test.
>
> Signed-off-by: Carlos Neira <cneirabustos@gmail.com>
> ---
> tools/testing/selftests/bpf/.gitignore | 2 +-
> tools/testing/selftests/bpf/Makefile | 3 +-
> .../bpf/prog_tests/ns_current_pid_tgid.c | 85 -----------------
> .../bpf/prog_tests/ns_current_pidtgid.c | 59 ++++++++++++
> .../bpf/progs/test_ns_current_pid_tgid.c | 37 --------
> .../bpf/progs/test_ns_current_pidtgid.c | 25 +++++
> ...new_ns.c => test_current_pidtgid_new_ns.c} | 0
> .../bpf/test_ns_current_pidtgid_newns.c | 91 +++++++++++++++++++
> 8 files changed, 178 insertions(+), 124 deletions(-)
> delete mode 100644 tools/testing/selftests/bpf/prog_tests/ns_current_pid_tgid.c
> create mode 100644 tools/testing/selftests/bpf/prog_tests/ns_current_pidtgid.c
> delete mode 100644 tools/testing/selftests/bpf/progs/test_ns_current_pid_tgid.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_ns_current_pidtgid.c
> rename tools/testing/selftests/bpf/{test_current_pid_tgid_new_ns.c => test_current_pidtgid_new_ns.c} (100%)
> create mode 100644 tools/testing/selftests/bpf/test_ns_current_pidtgid_newns.c
>
> diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> index 1bb204cee853..022055f23592 100644
> --- a/tools/testing/selftests/bpf/.gitignore
> +++ b/tools/testing/selftests/bpf/.gitignore
> @@ -30,8 +30,8 @@ test_tcpnotify_user
> test_libbpf
> test_tcp_check_syncookie_user
> test_sysctl
> -test_current_pid_tgid_new_ns
> xdping
> +test_ns_current_pidtgid_newns
> test_cpp
> *.skel.h
> /no_alu32
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index e7a8cf83ba48..c1ba9c947196 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -37,7 +37,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
> test_cgroup_storage \
> test_netcnt test_tcpnotify_user test_sock_fields test_sysctl \
> test_progs-no_alu32 \
> - test_current_pid_tgid_new_ns
> + test_ns_current_pidtgid_newns
>
> # Also test bpf-gcc, if present
> ifneq ($(BPF_GCC),)
> @@ -163,6 +163,7 @@ $(OUTPUT)/test_cgroup_storage: cgroup_helpers.c
> $(OUTPUT)/test_netcnt: cgroup_helpers.c
> $(OUTPUT)/test_sock_fields: cgroup_helpers.c
> $(OUTPUT)/test_sysctl: cgroup_helpers.c
> +$(OUTPUT)/test_ns_current_pidtgid_newns: test_ns_current_pidtgid_newns.c
This dependency is implicit and not necessary. What you see above is
*additional* .c files that tests need.
>
> DEFAULT_BPFTOOL := $(SCRATCH_DIR)/sbin/bpftool
> BPFTOOL ?= $(DEFAULT_BPFTOOL)
[...]
> +void test_ns_current_pidtgid(void)
> +{
> + int err, duration = 0;
> + struct test_ns_current_pidtgid *skel;
> + struct test_ns_current_pidtgid__bss *bss;
> + struct stat st;
> + __u64 id;
> +
> + skel = test_ns_current_pidtgid__open();
> + if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
> + return;
> +
> + err = test_ns_current_pidtgid__load(skel);
> + if (CHECK(err, "skel_load", "failed to load skeleton: %d\n", err))
> + goto cleanup;
nit: could be combined into a single test_ns_current_pidtgid__open_and_load()
> +
> + pid_t tid = syscall(SYS_gettid);
> + pid_t pid = getpid();
> +
> + id = (__u64) tid << 32 | pid;
> +
> + if (CHECK_FAIL(stat("/proc/self/ns/pid", &st))) {
please use CHECK instead of CHECK_FAIL
> + perror("Failed to stat /proc/self/ns/pid");
> + goto cleanup;
> + }
> +
> + bss = skel->bss;
> + bss->dev = st.st_dev;
> + bss->ino = st.st_ino;
> + bss->user_pid_tgid = 0;
> +
[...]
> +static int newns_pidtgid(void *arg)
> +{
> + struct test_ns_current_pidtgid__bss *bss;
> + struct test_ns_current_pidtgid *skel;
> + int pidns_fd = 0, err = 0;
> + pid_t pid, tid;
> + struct stat st;
> + __u64 id;
> +
> + skel = test_ns_current_pidtgid__open();
> + err = test_ns_current_pidtgid__load(skel);
if open() fails, NULL pointer dereference happens here. But also
better use open_and_load() variant.
> + if (err) {
> + perror("Failed to load skeleton");
> + goto cleanup;
> + }
> +
> + tid = syscall(SYS_gettid);
> + pid = getpid();
> + id = (__u64) tid << 32 | pid;
> +
> + if (stat("/proc/self/ns/pid", &st)) {
> + printf("Failed to stat /proc/self/ns/pid: %s\n",
> + strerror(errno));
> + goto cleanup;
> + }
> +
> + bss = skel->bss;
> + bss->dev = st.st_dev;
> + bss->ino = st.st_ino;
> + bss->user_pid_tgid = 0;
> +
> + err = test_ns_current_pidtgid__attach(skel);
> + if (err) {
> + printf("Failed to attach: %s err: %d\n", strerror(errno), err);
> + goto cleanup;
> + }
> + /* trigger tracepoint */
> + usleep(1);
> +
> + if (bss->user_pid_tgid != id) {
> + printf("test_ns_current_pidtgid_newns:FAIL\n");
> + err = EXIT_FAILURE;
> + } else {
> + printf("test_ns_current_pidtgid_newns:PASS\n");
> + err = EXIT_SUCCESS;
> + }
> +
> +cleanup:
> + setns(pidns_fd, CLONE_NEWPID);
> + test_ns_current_pidtgid__destroy(skel);
> +
> + return 0;
you don't return err from above
> +}
> +
> +int main(int argc, char **argv)
> +{
> + pid_t cpid;
> + int wstatus;
> +
> + cpid = clone(newns_pidtgid,
> + child_stack + STACK_SIZE,
> + CLONE_NEWPID | SIGCHLD, NULL);
> + if (cpid == -1) {
> + printf("test_ns_current_pidtgid_newns:Failed on CLONE: %s\n",
> + strerror(errno));
> + exit(EXIT_FAILURE);
> + }
> + if (waitpid(cpid, &wstatus, 0) == -1) {
> + printf("test_ns_current_pidtgid_newns:Failed on waitpid: %s\n",
> + strerror(errno));
exit(EXIT_FAILURE) here?
> + }
> + return (WEXITSTATUS(wstatus));
nit: unnecessary ()
> +}
> --
> 2.20.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox