Re: [RFC PATCH net-next 04/16] net: mscc: ocelot: use a switch-case statement in ocelot_netdevice_event

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

On 08/12/2020 14:07:50+0200, Vladimir Oltean wrote:
> Make ocelot's net device event handler more streamlined by structuring
> it in a similar way with others. The inspiration here was
> dsa_slave_netdevice_event.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/ethernet/mscc/ocelot_net.c | 68 +++++++++++++++++---------
>  1 file changed, 45 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
> index 50765a3b1c44..47b620967156 100644
> --- a/drivers/net/ethernet/mscc/ocelot_net.c
> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
> @@ -1030,49 +1030,71 @@ static int ocelot_netdevice_changeupper(struct net_device *dev,
>  					      info->upper_dev);
>  	}
>  
> -	return err;
> +	return notifier_from_errno(err);
> +}
> +
> +static int
> +ocelot_netdevice_lag_changeupper(struct net_device *dev,
> +				 struct netdev_notifier_changeupper_info *info)
> +{
> +	struct net_device *lower;
> +	struct list_head *iter;
> +	int err = NOTIFY_DONE;
> +
> +	netdev_for_each_lower_dev(dev, lower, iter) {
> +		err = ocelot_netdevice_changeupper(lower, info);
> +		if (err)
> +			return notifier_from_errno(err);
> +	}
> +
> +	return NOTIFY_DONE;
>  }
>  
>  static int ocelot_netdevice_event(struct notifier_block *unused,
>  				  unsigned long event, void *ptr)
>  {
> -	struct netdev_notifier_changeupper_info *info = ptr;
>  	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> -	int ret = 0;
>  
> -	if (event == NETDEV_PRECHANGEUPPER &&
> -	    ocelot_netdevice_dev_check(dev) &&
> -	    netif_is_lag_master(info->upper_dev)) {
> -		struct netdev_lag_upper_info *lag_upper_info = info->upper_info;
> +	switch (event) {
> +	case NETDEV_PRECHANGEUPPER: {
> +		struct netdev_notifier_changeupper_info *info = ptr;
> +		struct netdev_lag_upper_info *lag_upper_info;
>  		struct netlink_ext_ack *extack;
>  
> +		if (!ocelot_netdevice_dev_check(dev))
> +			break;
> +
> +		if (!netif_is_lag_master(info->upper_dev))
> +			break;
> +
> +		lag_upper_info = info->upper_info;
> +
>  		if (lag_upper_info &&
>  		    lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH) {
>  			extack = netdev_notifier_info_to_extack(&info->info);
>  			NL_SET_ERR_MSG_MOD(extack, "LAG device using unsupported Tx type");
>  
> -			ret = -EINVAL;
> -			goto notify;
> +			return NOTIFY_BAD;

This changes the return value in case of error, I'm not sure how
important this is.

>  		}
> +
> +		break;
>  	}
> +	case NETDEV_CHANGEUPPER: {
> +		struct netdev_notifier_changeupper_info *info = ptr;
>  
> -	if (event == NETDEV_CHANGEUPPER) {
> -		if (netif_is_lag_master(dev)) {
> -			struct net_device *slave;
> -			struct list_head *iter;
> +		if (ocelot_netdevice_dev_check(dev))
> +			return ocelot_netdevice_changeupper(dev, info);
>  
> -			netdev_for_each_lower_dev(dev, slave, iter) {
> -				ret = ocelot_netdevice_changeupper(slave, event, info);
> -				if (ret)
> -					goto notify;
> -			}
> -		} else {
> -			ret = ocelot_netdevice_changeupper(dev, event, info);
> -		}
> +		if (netif_is_lag_master(dev))
> +			return ocelot_netdevice_lag_changeupper(dev, info);
> +
> +		break;
> +	}
> +	default:
> +		break;
>  	}
>  
> -notify:
> -	return notifier_from_errno(ret);
> +	return NOTIFY_DONE;

This changes the return value from NOTIFY_OK to NOTIFY_DONE but this is
probably what we want.

>  }
>  
>  struct notifier_block ocelot_netdevice_nb __read_mostly = {
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com