netdev - Re: [RFC PATCH net-next 3/4] net: systemport: use standard netdevice notifier to detect DSA presence

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <34386add-4daf-d8f3-9176-295719095918@gmail.com>
Date:   Fri, 18 Dec 2020 16:26:28 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        bcm-kernel-feedback-list@...adcom.com, netdev@...r.kernel.org
Subject: Re: [RFC PATCH net-next 3/4] net: systemport: use standard netdevice
 notifier to detect DSA presence



On 12/18/2020 2:38 PM, Vladimir Oltean wrote:
> The SYSTEMPORT driver maps each port of the embedded Broadcom DSA switch
> port to a certain queue of the master Ethernet controller. For that it
> currently uses a dedicated notifier infrastructure which was added in
> commit 60724d4bae14 ("net: dsa: Add support for DSA specific notifiers").
> 
> However, since commit 2f1e8ea726e9 ("net: dsa: link interfaces with the
> DSA master to get rid of lockdep warnings"), DSA is actually an upper of
> the Broadcom SYSTEMPORT as far as the netdevice adjacency lists are
> concerned. So naturally, the plain NETDEV_CHANGEUPPER net device notifiers
> are emitted. It looks like there is enough API exposed by DSA to the
> outside world already to make the call_dsa_notifiers API redundant. So
> let's convert its only user to plain netdev notifiers.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  drivers/net/ethernet/broadcom/bcmsysport.c | 76 +++++++++-------------
>  drivers/net/ethernet/broadcom/bcmsysport.h |  2 +-
>  2 files changed, 32 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index 82541352b1eb..c5df235975e7 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -2311,33 +2311,22 @@ static const struct net_device_ops bcm_sysport_netdev_ops = {
>  	.ndo_select_queue	= bcm_sysport_select_queue,
>  };
>  
> -static int bcm_sysport_map_queues(struct notifier_block *nb,
> -				  struct dsa_notifier_register_info *info)
> +static int bcm_sysport_map_queues(struct net_device *dev,
> +				  struct net_device *slave_dev)
>  {
> +	struct dsa_port *dp = dsa_port_from_netdev(slave_dev);
> +	struct bcm_sysport_priv *priv = netdev_priv(dev);
>  	struct bcm_sysport_tx_ring *ring;
> -	struct bcm_sysport_priv *priv;
> -	struct net_device *slave_dev;
>  	unsigned int num_tx_queues;
>  	unsigned int q, qp, port;
> -	struct net_device *dev;
> -
> -	priv = container_of(nb, struct bcm_sysport_priv, dsa_notifier);
> -	if (priv->netdev != info->master)
> -		return 0;

There are systems with two SYSTEMPORT network devices registered and
therfore this check was intended to avoid programmig the incorrect
network device upon notification, however now that we have proper
upper/lower linking, and given the decisions made by bcm_sf2.c, only one
out of the two SYSTEMPORT network devices will act as a DSA master,
therefore this should no longer be necessary.

I would like to test this before giving this patch a Acked-by or
Teteed-by tag, net-next is still closed and this should only take a few
hours if there not any non-maskable real life interrupts showing up.
-- 
Florian