Re: [PATCH net-next v2 1/2] net: dsa: mv88e6xxx: Save switch rules

[Florian Fainelli]

Hi Miquel,

On 1/25/19 1:55 AM, Miquel Raynal wrote:
> The user might apply a specific switch configuration, with specific
> forwarding rules, VLAN, bridges, etc.
> 
> During suspend to RAM the switch power will be turned off and the
> switch will lost its configuration. In an attempt to bring S2RAM
> support to the mv88e6xxx DSA, let's first save these rules in a
> per-chip list thanks to the mv88e6xxx_add/del_xxx_rule()
> helpers. These helpers are then called from various callbacks:
> * mv88e6xxx_port_fdb_add/del()
> * mv88e6xxx_port_mdb_add/del()
> * mv88e6xxx_port_vlan_add/del()
> * mv88e6xxx_port_bridge_join/leave()
> * mv88e6xxx_crosschip_bridge_join/leave()
> 
> To avoid recursion problems when replaying the rules, the content of
> the above *_add()/*_join() callbacks has been moved in separate
> helpers with a '_' prefix. Hence, each callback just calls the
> corresponding helper and the corresponding *_add_xxx_rule().

None of this should be done in the driver IMHO, because this is
presumably applicable to all switch devices that lose their state during
suspend/resume, so at best, this should be moved to the core DSA layer,
but doing this means that we should also have a well established
contract between the DSA layer and individual switch drivers as far as
quiescing/saving/restoring state goes.

By moving things to the core we can also more tightly control what data
structures get used to represent e.g.: VLANs, FDBs, MDBs etc and
possibly push/utilize caching into the original subsystem. For instance
VLAN/bridge already do maintain caches of VLANs, so if we could somehow
expose those, we would not bloat the kernel's memory footprint by having
an additional layer to maintain with identical information.

Just my 2 cents.
-- 
Florian