linux-kernel - Re: [PATCH net-next RFC v1 1/4] devlink: Wrap trap related lists and ops in trap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Sep 2020 16:12:43 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Aya Levin <ayal@...lanox.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jiri Pirko <jiri@...lanox.com>, netdev@...r.kernel.org,
        Moshe Shemesh <moshe@...lanox.com>,
        Eran Ben Elisha <eranbe@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next RFC v1 1/4] devlink: Wrap trap related lists and
 ops in trap_mngr

Wed, Sep 02, 2020 at 05:32:11PM CEST, ayal@...lanox.com wrote:
>Bundle the trap related lists: trap_list, trap_group_list and
>trap_policer_list and trap ops like: trap_init, trap_fini,
>trap_action_set... together in trap_mngr. This will be handy in the
>coming patches in the set introducing traps in devlink port context.
>With trap_mngr, code reuse is much simpler.
>
>Signed-off-by: Aya Levin <ayal@...lanox.com>
>---
> drivers/net/ethernet/mellanox/mlxsw/core.c |   4 +
> include/net/devlink.h                      |  59 ++++---
> net/core/devlink.c                         | 255 +++++++++++++++++------------

You need to split this. You do at least 2 separate things in one patch.
Please check it.


> 3 files changed, 188 insertions(+), 130 deletions(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
>index 08d101138fbe..97460f47e537 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
>@@ -1285,6 +1285,9 @@ static const struct devlink_ops mlxsw_devlink_ops = {
> 	.sb_occ_tc_port_bind_get	= mlxsw_devlink_sb_occ_tc_port_bind_get,
> 	.info_get			= mlxsw_devlink_info_get,
> 	.flash_update			= mlxsw_devlink_flash_update,
>+};
>+
>+static const struct devlink_trap_ops mlxsw_devlink_traps_ops = {
> 	.trap_init			= mlxsw_devlink_trap_init,
> 	.trap_fini			= mlxsw_devlink_trap_fini,
> 	.trap_action_set		= mlxsw_devlink_trap_action_set,
>@@ -1321,6 +1324,7 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
> 			err = -ENOMEM;
> 			goto err_devlink_alloc;
> 		}
>+		devlink_traps_ops(devlink, &mlxsw_devlink_traps_ops);
> 	}
> 
> 	mlxsw_core = devlink_priv(devlink);
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 8f3c8a443238..d387ea5518c3 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -21,6 +21,13 @@
> #include <linux/xarray.h>
> 
> struct devlink_ops;
>+struct devlink_trap_ops;
>+struct devlink_trap_mngr {

"Mngr" is odd. This is not a manager. Just a common place to store
trap-related things. Perhaps just "devlink_traps"?


>+	struct list_head trap_list;
>+	struct list_head trap_group_list;
>+	struct list_head trap_policer_list;
>+	const struct devlink_trap_ops *trap_ops;
>+};

[...]