Subject: Re: [PATCH net 06/10] net: rmnet: print error message when command fails

From: [email-masked]
Date: Wed, 26 Feb 2020 13:37:17 -0700

On 2020-02-26 10:47, Taehee Yoo wrote:
> When rmnet netlink command fails, it doesn't print any error message.
> So, users couldn't know the exact reason.
> In order to tell the exact reason to the user, the extack error message
> is used in this patch.
> 
> Signed-off-by: Taehee Yoo <[email-masked]>
> ---
>  .../ethernet/qualcomm/rmnet/rmnet_config.c    | 26 ++++++++++++-------
>  .../net/ethernet/qualcomm/rmnet/rmnet_vnd.c   | 10 +++----
>  .../net/ethernet/qualcomm/rmnet/rmnet_vnd.h   |  3 ++-
>  3 files changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> index c8b1bfe127ac..93745cd45c29 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> @@ -141,11 +141,10 @@ static int rmnet_newlink(struct net *src_net,
> struct net_device *dev,
>  	}
> 
>  	real_dev = __dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
> -	if (!real_dev || !dev)
> +	if (!real_dev || !dev) {
> +		NL_SET_ERR_MSG_MOD(extack, "link does not exist");
>  		return -ENODEV;
> -
> -	if (!data[IFLA_RMNET_MUX_ID])
> -		return -EINVAL;
> +	}
> 
>  	ep = kzalloc(sizeof(*ep), GFP_ATOMIC);
>  	if (!ep)
> @@ -158,7 +157,7 @@ static int rmnet_newlink(struct net *src_net,
> struct net_device *dev,
>  		goto err0;
> 
>  	port = rmnet_get_port_rtnl(real_dev);
> -	err = rmnet_vnd_newlink(mux_id, dev, port, real_dev, ep);
> +	err = rmnet_vnd_newlink(mux_id, dev, port, real_dev, ep, extack);
>  	if (err)
>  		goto err1;
> 
> @@ -275,12 +274,16 @@ static int rmnet_rtnl_validate(struct nlattr
> *tb[], struct nlattr *data[],
>  {
>  	u16 mux_id;
> 
> -	if (!data || !data[IFLA_RMNET_MUX_ID])
> +	if (!data || !data[IFLA_RMNET_MUX_ID]) {
> +		NL_SET_ERR_MSG_MOD(extack, "MUX ID not specifies");
>  		return -EINVAL;
> +	}
> 
>  	mux_id = nla_get_u16(data[IFLA_RMNET_MUX_ID]);
> -	if (mux_id > (RMNET_MAX_LOGICAL_EP - 1))
> +	if (mux_id > (RMNET_MAX_LOGICAL_EP - 1)) {
> +		NL_SET_ERR_MSG_MOD(extack, "invalid MUX ID");
>  		return -ERANGE;
> +	}
> 
>  	return 0;
>  }
> @@ -414,11 +417,16 @@ int rmnet_add_bridge(struct net_device 
> *rmnet_dev,
>  	/* If there is more than one rmnet dev attached, its probably being
>  	 * used for muxing. Skip the briding in that case
>  	 */
> -	if (port->nr_rmnet_devs > 1)
> +	if (port->nr_rmnet_devs > 1) {
> +		NL_SET_ERR_MSG_MOD(extack, "more than one rmnet dev attached");
>  		return -EINVAL;
> +	}
> 
> -	if (rmnet_is_real_dev_registered(slave_dev))
> +	if (rmnet_is_real_dev_registered(slave_dev)) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "dev is already attached another rmnet dev");
>  		return -EBUSY;
> +	}
> 
>  	err = rmnet_register_real_device(slave_dev);
>  	if (err)
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> index a26e76e9d382..90c19033ebe0 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
> @@ -224,16 +224,16 @@ void rmnet_vnd_setup(struct net_device 
> *rmnet_dev)
>  int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
>  		      struct rmnet_port *port,
>  		      struct net_device *real_dev,
> -		      struct rmnet_endpoint *ep)
> +		      struct rmnet_endpoint *ep,
> +		      struct netlink_ext_ack *extack)
>  {
>  	struct rmnet_priv *priv = netdev_priv(rmnet_dev);
>  	int rc;
> 
> -	if (ep->egress_dev)
> -		return -EINVAL;
> -
> -	if (rmnet_get_endpoint(port, id))
> +	if (rmnet_get_endpoint(port, id)) {
> +		NL_SET_ERR_MSG_MOD(extack, "MUX ID already exists");
>  		return -EBUSY;
> +	}
> 
>  	rmnet_dev->hw_features = NETIF_F_RXCSUM;
>  	rmnet_dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
> index 54cbaf3c3bc4..d8fa76e8e9c4 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h
> @@ -11,7 +11,8 @@ int rmnet_vnd_do_flow_control(struct net_device
> *dev, int enable);
>  int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
>  		      struct rmnet_port *port,
>  		      struct net_device *real_dev,
> -		      struct rmnet_endpoint *ep);
> +		      struct rmnet_endpoint *ep,
> +		      struct netlink_ext_ack *extack);
>  int rmnet_vnd_dellink(u8 id, struct rmnet_port *port,
>  		      struct rmnet_endpoint *ep);
>  void rmnet_vnd_rx_fixup(struct sk_buff *skb, struct net_device *dev);

This patch and [PATCH net 02/10] "net: rmnet: add missing module alias" 
seem
to be adding new functionality. Perhaps it can be sent to net-next 
instead
of net.