Received-SPF: pass (google.com: domain of linux-kernel+bounces-95302-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 7 Mar 2024 10:06:50 +0000
From: Simon Horman <horms@kernel.org>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Jiri Pirko <jiri@resnulli.us>, Ivan Vecera <ivecera@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Andrew Lunn <andrew@lunn.ch>, Eric Dumazet <edumazet@google.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Vladimir Oltean <olteanv@gmail.com>, kernel@pengutronix.de,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH net-next v1 1/1]  net: bridge: switchdev: Improve error
 message clarity for switchdev_port_obj_add/del_deffered operations
Message-ID: <20240307100650.GK281974@kernel.org>
References: <20240306132130.2720721-1-o.rempel@pengutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240306132130.2720721-1-o.rempel@pengutronix.de>

On Wed, Mar 06, 2024 at 02:21:30PM +0100, Oleksij Rempel wrote:
> Enhance the error reporting mechanism in the switchdev framework to
> provide more informative and user-friendly error messages.
> 
> Following feedback from users struggling to understand the implications
> of error messages like "failed (err=-28) to add object (id=2)", this
> update aims to clarify what operation failed and how this might impact
> the system or network.
> 
> With this change, error messages now include a description of the failed
> operation, the specific object involved, and a brief explanation of the
> potential impact on the system. This approach helps administrators and
> developers better understand the context and severity of errors,
> facilitating quicker and more effective troubleshooting.
> 
> Example of the improved logging:
> 
> [   70.516446] ksz-switch spi0.0 uplink: Failed to add Port Multicast
>                Database entry (object id=2) with error: -ENOSPC (-28).
> [   70.516446] Failure in updating the port's Multicast Database could
>                lead to multicast forwarding issues.
> [   70.516446] Current HW/SW setup lacks sufficient resources.
> 
> This comprehensive update includes handling for a range of switchdev
> object IDs, ensuring that most operations within the switchdev framework
> benefit from clearer error reporting.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  net/switchdev/switchdev.c | 106 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 102 insertions(+), 4 deletions(-)
> 
> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
> index c9189a970eec3..d5a30b626fd9c 100644
> --- a/net/switchdev/switchdev.c
> +++ b/net/switchdev/switchdev.c
> @@ -244,6 +244,106 @@ static int switchdev_port_obj_notify(enum switchdev_notifier_type nt,
>  	return 0;
>  }
>  
> +static void switchdev_obj_id_to_helpful_msg(struct net_device *dev,
> +					    enum switchdev_obj_id obj_id,
> +					    int err, bool add)
> +{
> +	char *action = add ? "add" : "del";
> +	char *reason = "";
> +	char *problem;
> +	char *obj_str;

Hi Oleksij,

I think the above variables could be const.
That not withstanding, this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

..