| Message ID | 20200910164857.1221202-1-olteanv@gmail.com |
|---|---|
| Headers | show |
| Series | DSA tag_8021q cleanup | expand |
On 9/10/2020 9:48 AM, Vladimir Oltean wrote: > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > This reverts commit 314f76d7a68bab0516aa52877944e6aacfa0fc3f. > > Citing that commit message, the call graph was: > > dsa_slave_vlan_rx_add_vid dsa_port_setup_8021q_tagging > | | > | | > | +-------------+ > | | > v v > dsa_port_vid_add dsa_slave_port_obj_add > | | > +-------+ +-------+ > | | > v v > dsa_port_vlan_add > > Now that tag_8021q has its own ops structure, it no longer relies on > dsa_port_vid_add, and therefore on the dsa_switch_ops to install its > VLANs. > > So dsa_port_vid_add now only has one single caller. So we can simplify > the call graph to what it was before, aka: > > dsa_slave_vlan_rx_add_vid dsa_slave_port_obj_add > | | > +-------+ +-------+ > | | > v v > dsa_port_vlan_add > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> I would be keen on keeping this function just because it encapsulates the details of creating the switchdev object and it may be useful to add additional functionality later on (like the DSA master RX VLAN filtering?), but would not object to its removal if others disagree.
On 9/11/2020 10:30 AM, Vladimir Oltean wrote: > On Thu, Sep 10, 2020 at 12:52:03PM -0700, Florian Fainelli wrote: >> On 9/10/2020 9:48 AM, Vladimir Oltean wrote: >>> From: Vladimir Oltean <vladimir.oltean@nxp.com> >>> >>> This reverts commit 314f76d7a68bab0516aa52877944e6aacfa0fc3f. >>> >>> Citing that commit message, the call graph was: >>> >>> dsa_slave_vlan_rx_add_vid dsa_port_setup_8021q_tagging >>> | | >>> | | >>> | +-------------+ >>> | | >>> v v >>> dsa_port_vid_add dsa_slave_port_obj_add >>> | | >>> +-------+ +-------+ >>> | | >>> v v >>> dsa_port_vlan_add >>> >>> Now that tag_8021q has its own ops structure, it no longer relies on >>> dsa_port_vid_add, and therefore on the dsa_switch_ops to install its >>> VLANs. >>> >>> So dsa_port_vid_add now only has one single caller. So we can simplify >>> the call graph to what it was before, aka: >>> >>> dsa_slave_vlan_rx_add_vid dsa_slave_port_obj_add >>> | | >>> +-------+ +-------+ >>> | | >>> v v >>> dsa_port_vlan_add >>> >>> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> >> >> I would be keen on keeping this function just because it encapsulates the >> details of creating the switchdev object and it may be useful to add >> additional functionality later on (like the DSA master RX VLAN filtering?), >> but would not object to its removal if others disagree. >> -- >> Florian > > Hmm, I don't think there's a lot of value in having it, it's confusing > to have such a layered call stack, and it shouldn't be an exported > symbol any longer in any case. > Also, I already have a patch that calls vlan_vid_add(master) and having > this dsa_port_vid_add() helper doesn't save much at all. OK, I am convinced: Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
From: Vladimir Oltean <olteanv@gmail.com> Date: Thu, 10 Sep 2020 19:48:53 +0300 > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > This small series tries to consolidate the VLAN handling in DSA a little > bit. It reworks tag_8021q to be minimally invasive to the dsa_switch_ops > structure. This makes the rest of the code a bit easier to follow. Series applied, thank you.
From: Vladimir Oltean <vladimir.oltean@nxp.com> This small series tries to consolidate the VLAN handling in DSA a little bit. It reworks tag_8021q to be minimally invasive to the dsa_switch_ops structure. This makes the rest of the code a bit easier to follow. Vladimir Oltean (4): net: dsa: tag_8021q: include missing refcount.h net: dsa: tag_8021q: setup tagging via a single function call net: dsa: tag_8021q: add a context structure Revert "net: dsa: Add more convenient functions for installing port VLANs" drivers/net/dsa/sja1105/sja1105.h | 3 +- drivers/net/dsa/sja1105/sja1105_main.c | 226 ++++++++++++++----------- include/linux/dsa/8021q.h | 49 +++--- net/dsa/dsa_priv.h | 2 - net/dsa/port.c | 33 ---- net/dsa/slave.c | 34 +++- net/dsa/tag_8021q.c | 138 ++++++++------- 7 files changed, 265 insertions(+), 220 deletions(-)