Re: [PATCH net-next 2/2] bonding: combine netlink and console error messages

[Leon Romanovsky]

On Fri, Aug 06, 2021 at 11:30:55PM -0400, Jonathan Toppins wrote:
> There seems to be no reason to have different error messages between
> netlink and printk. It also cleans up the function slightly.
> 
> Signed-off-by: Jonathan Toppins <jtoppins@xxxxxxxxxx>
> ---
>  drivers/net/bonding/bond_main.c | 45 ++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 3ba5f4871162..46b95175690b 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1712,6 +1712,16 @@ void bond_lower_state_changed(struct slave *slave)
>  	netdev_lower_state_changed(slave->dev, &info);
>  }
>  
> +#define BOND_NL_ERR(bond_dev, extack, errmsg) do {		\
> +	NL_SET_ERR_MSG(extack, errmsg);				\
> +	netdev_err(bond_dev, "Error: " errmsg "\n");		\
> +} while (0)
> +
> +#define SLAVE_NL_ERR(bond_dev, slave_dev, extack, errmsg) do {	\
> +	NL_SET_ERR_MSG(extack, errmsg);				\
> +	slave_err(bond_dev, slave_dev, "Error: " errmsg "\n");	\
> +} while (0)

I don't think that both extack messages and dmesg prints are needed.

They both will be caused by the same source, and both will be seen by
the caller, but duplicated.

IMHO, errors that came from the netlink, should be printed with NL_SET_ERR_MSG(),
other errors should use netdev_err/slave_err prints.

Thanks