Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 8 Sep 2020 16:12:43 +0200
From:   Jiri Pirko <jiri@resnulli.us>
To:     Aya Levin <ayal@mellanox.com>
Cc:     "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Jiri Pirko <jiri@mellanox.com>, netdev@vger.kernel.org,
        Moshe Shemesh <moshe@mellanox.com>,
        Eran Ben Elisha <eranbe@mellanox.com>,
        Ido Schimmel <idosch@mellanox.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next RFC v1 1/4] devlink: Wrap trap related lists and
 ops in trap_mngr
Message-ID: <20200908141243.GO2997@nanopsycho.orion>
References: <1599060734-26617-1-git-send-email-ayal@mellanox.com>
 <1599060734-26617-2-git-send-email-ayal@mellanox.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1599060734-26617-2-git-send-email-ayal@mellanox.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Wed, Sep 02, 2020 at 05:32:11PM CEST, ayal@mellanox.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@mellanox.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;
>+};

[...]