Re: [PATCH net-next 02/15] net: hsr: create an API to get hsr port type

["Łukasz Majewski" <lukma@nabladev.com>]

Hi Vladimir,

> From: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> 
> Since the introduction of HSR_PT_INTERLINK in commit 5055cccfc2d1
> ("net: hsr: Provide RedBox support (HSR-SAN)"), we see that different
> port types require different settings for hardware offload, which was
> not the case before when we only had HSR_PT_SLAVE_A and
> HSR_PT_SLAVE_B. But there is currently no way to know which port is
> which type, so create the hsr_get_port_type() API function and export
> it.
> 
> When hsr_get_port_type() is called from the device driver, the port
> can must be found in the HSR port list. An important use case is for
^^^^^^^^^^ - I think that can is redundant here.

> this function to work from offloading drivers' NETDEV_CHANGEUPPER
> handler, which is triggered by hsr_portdev_setup() ->
> netdev_master_upper_dev_link(). Therefore, we need to move the
> addition of the hsr_port to the HSR port list prior to calling
> hsr_portdev_setup(). This makes the error restoration path also more
> similar to hsr_del_port(), where kfree_rcu(port) is already used.
> 
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Lukasz Majewski <lukma@denx.de>
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  include/linux/if_hsr.h |  9 +++++++++
>  net/hsr/hsr_device.c   | 20 ++++++++++++++++++++
>  net/hsr/hsr_slave.c    |  7 ++++---
>  3 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/if_hsr.h b/include/linux/if_hsr.h
> index d7941fd88032..f4cf2dd36d19 100644
> --- a/include/linux/if_hsr.h
> +++ b/include/linux/if_hsr.h
> @@ -43,6 +43,8 @@ extern bool is_hsr_master(struct net_device *dev);
>  extern int hsr_get_version(struct net_device *dev, enum hsr_version
> *ver); struct net_device *hsr_get_port_ndev(struct net_device *ndev,
>  				     enum hsr_port_type pt);
> +int hsr_get_port_type(struct net_device *hsr_dev, struct net_device
> *dev,
> +		      enum hsr_port_type *type);
>  #else
>  static inline bool is_hsr_master(struct net_device *dev)
>  {
> @@ -59,6 +61,13 @@ static inline struct net_device
> *hsr_get_port_ndev(struct net_device *ndev, {
>  	return ERR_PTR(-EINVAL);
>  }
> +
> +static inline int hsr_get_port_type(struct net_device *hsr_dev,
> +				    struct net_device *dev,
> +				    enum hsr_port_type *type)
> +{
> +	return -EINVAL;
> +}
>  #endif /* CONFIG_HSR */
>  
>  #endif /*_LINUX_IF_HSR_H_*/
> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index 492cbc78ab75..d1bfc49b5f01 100644
> --- a/net/hsr/hsr_device.c
> +++ b/net/hsr/hsr_device.c
> @@ -690,6 +690,26 @@ struct net_device *hsr_get_port_ndev(struct
> net_device *ndev, }
>  EXPORT_SYMBOL(hsr_get_port_ndev);
>  
> +int hsr_get_port_type(struct net_device *hsr_dev, struct net_device
> *dev,
> +		      enum hsr_port_type *type)
> +{
> +	struct hsr_priv *hsr = netdev_priv(hsr_dev);
> +	struct hsr_port *port;
> +
> +	rcu_read_lock();
> +	hsr_for_each_port(hsr, port) {
> +		if (port->dev == dev) {
> +			*type = port->type;
> +			rcu_read_unlock();
> +			return 0;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(hsr_get_port_type);
> +
>  /* Default multicast address for HSR Supervision frames */
>  static const unsigned char def_multicast_addr[ETH_ALEN] __aligned(2)
> = { 0x01, 0x15, 0x4e, 0x00, 0x01, 0x00
> diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
> index 8177ac6c2d26..afe06ba00ea4 100644
> --- a/net/hsr/hsr_slave.c
> +++ b/net/hsr/hsr_slave.c
> @@ -207,14 +207,14 @@ int hsr_add_port(struct hsr_priv *hsr, struct
> net_device *dev, port->type = type;
>  	ether_addr_copy(port->original_macaddress, dev->dev_addr);
>  
> +	list_add_tail_rcu(&port->port_list, &hsr->ports);
> +
>  	if (type != HSR_PT_MASTER) {
>  		res = hsr_portdev_setup(hsr, dev, port, extack);
>  		if (res)
>  			goto fail_dev_setup;
>  	}
>  
> -	list_add_tail_rcu(&port->port_list, &hsr->ports);
> -
>  	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
>  	netdev_update_features(master->dev);
>  	dev_set_mtu(master->dev, hsr_get_max_mtu(hsr));
> @@ -222,7 +222,8 @@ int hsr_add_port(struct hsr_priv *hsr, struct
> net_device *dev, return 0;
>  
>  fail_dev_setup:
> -	kfree(port);
> +	list_del_rcu(&port->port_list);
> +	kfree_rcu(port, rcu);
>  	return res;
>  }
>  

Reviewed-by: Łukasz Majewski <lukma@nabladev.com>

-- 
Best regards,

Lukasz Majewski

--
Nabla Software Engineering GmbH
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office@nabladev.com
Managing Director : Stefano Babic