netdev - Re: [PATCH RFC net-next 14/19] net: mvpp2: add ethtool flow control configuration support

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: <20210110181519.GJ1551@shell.armlinux.org.uk>
Date:   Sun, 10 Jan 2021 18:15:19 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     stefanc@...vell.com
Cc:     netdev@...r.kernel.org, thomas.petazzoni@...tlin.com,
        davem@...emloft.net, nadavh@...vell.com, ymarkman@...vell.com,
        linux-kernel@...r.kernel.org, kuba@...nel.org, mw@...ihalf.com,
        andrew@...n.ch, atenart@...nel.org
Subject: Re: [PATCH RFC net-next  14/19] net: mvpp2: add ethtool flow control
 configuration support

On Sun, Jan 10, 2021 at 05:30:18PM +0200, stefanc@...vell.com wrote:
> @@ -5373,6 +5402,30 @@ static int mvpp2_ethtool_set_pause_param(struct net_device *dev,
>  					 struct ethtool_pauseparam *pause)
>  {
>  	struct mvpp2_port *port = netdev_priv(dev);
> +	int i;
> +
> +	if (pause->tx_pause && port->priv->global_tx_fc) {
> +		port->tx_fc = true;
> +		mvpp2_rxq_enable_fc(port);
> +		if (port->priv->percpu_pools) {
> +			for (i = 0; i < port->nrxqs; i++)
> +				mvpp2_bm_pool_update_fc(port, &port->priv->bm_pools[i], true);
> +		} else {
> +			mvpp2_bm_pool_update_fc(port, port->pool_long, true);
> +			mvpp2_bm_pool_update_fc(port, port->pool_short, true);
> +		}
> +
> +	} else if (port->priv->global_tx_fc) {
> +		port->tx_fc = false;
> +		mvpp2_rxq_disable_fc(port);
> +		if (port->priv->percpu_pools) {
> +			for (i = 0; i < port->nrxqs; i++)
> +				mvpp2_bm_pool_update_fc(port, &port->priv->bm_pools[i], false);
> +		} else {
> +			mvpp2_bm_pool_update_fc(port, port->pool_long, false);
> +			mvpp2_bm_pool_update_fc(port, port->pool_short, false);
> +		}
> +	}

This doesn't look correct to me. This function is only called when
ethtool -A is used to change the flow control settings. This is not
the place to be configuring flow control, as flow control is
negotiated with the link partner.

The final resolved flow control settings are available in
mvpp2_mac_link_up() via the tx_pause and rx_pause parameters.

What also concerns me is whether flow control is supported in the
existing driver at all, given this patch set. If it isn't supported
without the firmware's help, then we should _not_ be negotiating flow
control with the link partner unless we actually support it, so the
Pause and Asym_Pause bits in mvpp2_phylink_validate() should be
cleared.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!