netdev - Re: [PATCH net 1/2] sh_eth: Fix promiscuous mode on chips without TSU

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: <54B9663D.2040008@cogentembedded.com>
Date:	Fri, 16 Jan 2015 22:27:57 +0300
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	Ben Hutchings <ben.hutchings@...ethink.co.uk>,
	ct178-internal@...ts.codethink.co.uk,
	"David S. Miller" <davem@...emloft.net>
CC:	netdev@...r.kernel.org, linux-kernel@...ts.codethink.co.uk,
	Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@...esas.com>,
	Mitsuhiro Kimura <mitsuhiro.kimura.kc@...esas.com>,
	Hisashi Nakamura <hisashi.nakamura.ak@...esas.com>,
	Yoshihiro Kaneko <ykaneko0929@...il.com>
Subject: Re: [PATCH net 1/2] sh_eth: Fix promiscuous mode on chips without
 TSU

Hello.

On 01/16/2015 08:51 PM, Ben Hutchings wrote:

> Currently net_device_ops::set_rx_mode is only implemented for
> chips with a TSU (multiple address table).  However we do need
> to turn the PRM (promiscuous) flag on and off for other chips.

> - Remove the unlikely() from the TSU functions that we may safely
>    call for chips without a TSU

    This is just optimization, worth pushing thru net-next instead.

> - Make setting of the MCT flag conditional on the tsu capability flag
> - Rename sh_eth_set_multicast_list() to sh_eth_set_rx_mode() and plumb
>    it into both net_device_ops structures
> - Remove the previously-unreachable branch in sh_eth_rx_mode() that
>    would otherwise reset the flags to defaults for non-TSU chips

    It couldn't be default for non-TSU chips, they don't seem to have 
ECMR.MCT. So it was just wrong.

    It would have been better if you did one thing per patch or at least 
didn't mix fixes with clean-ups...

> Signed-off-by: Ben Hutchings <ben.hutchings@...ethink.co.uk>
> ---
>   drivers/net/ethernet/renesas/sh_eth.c |   18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)

> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 167737f..0c4d5b5 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -2417,7 +2417,7 @@ static int sh_eth_tsu_purge_all(struct net_device *ndev)
>   	struct sh_eth_private *mdp = netdev_priv(ndev);
>   	int i, ret;
>
> -	if (unlikely(!mdp->cd->tsu))
> +	if (!mdp->cd->tsu)
>   		return 0;
>
>   	for (i = 0; i < SH_ETH_TSU_CAM_ENTRIES; i++) {
> @@ -2440,7 +2440,7 @@ static void sh_eth_tsu_purge_mcast(struct net_device *ndev)
>   	void *reg_offset = sh_eth_tsu_get_offset(mdp, TSU_ADRH0);
>   	int i;
>
> -	if (unlikely(!mdp->cd->tsu))
> +	if (!mdp->cd->tsu)

    But we don't call this function on non-TSU SoCs, do we?

[...]

> @@ -2462,7 +2462,9 @@ static void sh_eth_set_multicast_list(struct net_device *ndev)
>   	/* Initial condition is MCT = 1, PRM = 0.
>   	 * Depending on ndev->flags, set PRM or clear MCT
>   	 */
> -	ecmr_bits = (sh_eth_read(ndev, ECMR) & ~ECMR_PRM) | ECMR_MCT;
> +	ecmr_bits = sh_eth_read(ndev, ECMR) & ~ECMR_PRM;
> +	if (mdp->cd->tsu)
> +		ecmr_bits |= ECMR_MCT;

    Seems OK, looking at the RZ/A1H manuals (this SoC does have TSU and this 
bit too).

[...]

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html