netdev - Re: [PATCH v2 net-next 3/4] net: dsa: tag

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: <18a9d5f7-0422-5ca2-9d65-10cd5fcdebc0@gmail.com>
Date:   Fri, 11 Sep 2020 16:49:50 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>, jakub.kicinski@...ronome.com,
        davem@...emloft.net
Cc:     andrew@...n.ch, vivien.didelot@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next 3/4] net: dsa: tag_8021q: add a context
 structure



On 9/10/2020 9:48 AM, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@....com>
> 
> While working on another tag_8021q driver implementation, some things
> became apparent:
> 
> - It is not mandatory for a DSA driver to offload the tag_8021q VLANs by
>    using the VLAN table per se. For example, it can add custom TCAM rules
>    that simply encapsulate RX traffic, and redirect & decapsulate rules
>    for TX traffic. For such a driver, it makes no sense to receive the
>    tag_8021q configuration through the same callback as it receives the
>    VLAN configuration from the bridge and the 8021q modules.
> 
> - Currently, sja1105 (the only tag_8021q user) sets a
>    priv->expect_dsa_8021q variable to distinguish between the bridge
>    calling, and tag_8021q calling. That can be improved, to say the
>    least.
> 
> - The crosschip bridging operations are, in fact, stateful already. The
>    list of crosschip_links must be kept by the caller and passed to the
>    relevant tag_8021q functions.
> 
> So it would be nice if the tag_8021q configuration was more
> self-contained. This patch attempts to do that.
> 
> Create a struct dsa_8021q_context which encapsulates a struct
> dsa_switch, and has 2 function pointers for adding and deleting a VLAN.
> These will replace the previous channel to the driver, which was through
> the .port_vlan_add and .port_vlan_del callbacks of dsa_switch_ops.
> 
> Also put the list of crosschip_links into this dsa_8021q_context.
> Drivers that don't support cross-chip bridging can simply omit to
> initialize this list, as long as they dont call any cross-chip function.
> 
> The sja1105_vlan_add and sja1105_vlan_del functions are refactored into
> a smaller sja1105_vlan_add_one, which now has 2 entry points:
> - sja1105_vlan_add, from struct dsa_switch_ops
> - sja1105_dsa_8021q_vlan_add, from the tag_8021q ops
> But even this change is fairly trivial. It just reflects the fact that
> for sja1105, the VLANs from these 2 channels end up in the same hardware
> table. However that is not necessarily true in the general sense (and
> that's the reason for making this change).
> 
> The rest of the patch is mostly plain refactoring of "ds" -> "ctx". The
> dsa_8021q_context structure needs to be propagated because adding a VLAN
> is now done through the ops function pointers inside of it.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>

This was indeed easier to review than v1.
-- 
Florian