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: <e9ac5203-87ba-c14f-8140-5b100808a8a4@gmail.com>
Date:   Wed, 16 Jan 2019 14:20:15 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Andrew Lunn <andrew@...n.ch>,
        Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Vivien Didelot <vivien.didelot@...il.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Gregory Clement <gregory.clement@...tlin.com>,
        Antoine Tenart <antoine.tenart@...tlin.com>,
        Maxime Chevallier <maxime.chevallier@...tlin.com>,
        Nadav Haklai <nadavh@...vell.com>
Subject: Re: [PATCH net-next] net: dsa: mv88e6xxx: Add suspend/resume
 callbacks

On 1/16/19 7:56 AM, Andrew Lunn wrote:
> On Wed, Jan 16, 2019 at 04:34:19PM +0100, Miquel Raynal wrote:
>> Bring S2RAM support to the mv88e6xxx DSA driver.
>>
>> The content of the *_irq_poll() helper is moved in *_do_irq_poll() so
>> that that the function can be called from the ->resume() callback
>> without using the *work pointer.
>>
>> +static int __maybe_unused mv88e6xxx_suspend(struct device *dev)
>> +{
>> +	struct dsa_switch *ds = dev_get_drvdata(dev);
>> +	struct mv88e6xxx_chip *chip = ds->priv;
>> +
>> +	kthread_cancel_delayed_work_sync(&chip->irq_poll_work);
>> +
>> +	return dsa_switch_suspend(ds);
>> +}
> 
> Hi Miquel
> 
> I don't think this is sufficient. You are leaving the switch itself
> running. It will still be forwarding frames. And since the host is
> shutdown, there is nothing doing spanning tree protocol. So you are
> likely to cause loops in your network.
> 
> Take a look at bcm_sf2:

There is a big caveat though, bcm_sf2_port_disable() checks a bitmask of
ports that might have been configured to allow Wake-on-LAN. Technically
you could also leave the switch running during S2RAM on these platforms,
which requires putting it in unmanaged mode, in order to continue having
stations "talk" to each other.

But in general, Andrew, is right, you must quiesce all activity on the
switch while you suspend it.

A possible approach could be to call the port_disable, port_enable
callbacks from dsa_slave_suspend() and dsa_slave_resume(), I might have
some patches doing that already somewhere.

> 
> static int bcm_sf2_sw_suspend(struct dsa_switch *ds)
> {
>         struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
>         unsigned int port;
> 
>         bcm_sf2_intr_disable(priv);
> 
>         /* Disable all ports physically present including the IMP
>          * port, the other ones have already been disabled during
>          * bcm_sf2_sw_setup
>          */
>         for (port = 0; port < DSA_MAX_PORTS; port++) {
>                 if (dsa_is_user_port(ds, port) || dsa_is_cpu_port(ds, port))
>                         bcm_sf2_port_disable(ds, port, NULL);
>         }
> 
>         return 0;
> }
> 
> qca8k does something similar.
> 
> 	Andrew
> 


-- 
Florian

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ