Re: [RFC PATCH net-next 03/13] net: dsa: Create a more convenient function for installing port VLANs
From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2019-03-25 17:06:21
On 3/23/19 8:23 PM, Vladimir Oltean wrote:
This refactors the two-phase transaction from dsa_slave_vlan_rx_add_vid and also makes that code available for other functions from within DSA. The newly exposed function either adds or deletes the specified VLAN entry based on a boolean argument. Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
The name of the function does not make it particularly clear that passing false results in deleting the VLAN. Can you just wrap this under a different function name that is only doing the two-step adding of VLANs and keep using dsa_port_vlan_del() explicitly when you want to remove a VLAN?
quoted hunk ↗ jump to hunk
--- net/dsa/dsa_priv.h | 2 ++ net/dsa/port.c | 24 ++++++++++++++++++++++++ net/dsa/slave.c | 16 ++-------------- 3 files changed, 28 insertions(+), 14 deletions(-)diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h index 093b7d145eb1..8048ced3708f 100644 --- a/net/dsa/dsa_priv.h +++ b/net/dsa/dsa_priv.h@@ -164,6 +164,8 @@ int dsa_port_pre_bridge_flags(const struct dsa_port *dp, unsigned long flags, struct switchdev_trans *trans); int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags, struct switchdev_trans *trans); +int dsa_port_trans_vlan_apply(struct dsa_port *dp, u16 vid, u16 flags, + bool enabled); int dsa_port_vlan_add(struct dsa_port *dp, const struct switchdev_obj_port_vlan *vlan, struct switchdev_trans *trans);diff --git a/net/dsa/port.c b/net/dsa/port.c index a86fe3be1261..9c7358f98004 100644 --- a/net/dsa/port.c +++ b/net/dsa/port.c@@ -326,6 +326,30 @@ int dsa_port_vlan_del(struct dsa_port *dp, return 0; } +int dsa_port_trans_vlan_apply(struct dsa_port *dp, u16 vid, u16 flags, + bool enabled) +{ + struct switchdev_obj_port_vlan vlan = { + .obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN, + .flags = flags, + .vid_begin = vid, + .vid_end = vid, + }; + struct switchdev_trans trans; + int err; + + if (!enabled) + return dsa_port_vlan_del(dp, &vlan); + + trans.ph_prepare = true; + err = dsa_port_vlan_add(dp, &vlan, &trans); + if (err == -EOPNOTSUPP) + return 0; + + trans.ph_prepare = false; + return dsa_port_vlan_add(dp, &vlan, &trans); +} + static struct phy_device *dsa_port_get_phy_device(struct dsa_port *dp) { struct device_node *phy_dn;diff --git a/net/dsa/slave.c b/net/dsa/slave.c index 093eef6f2599..3191ef74f6a1 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c@@ -987,13 +987,6 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid) { struct dsa_port *dp = dsa_slave_to_port(dev); - struct switchdev_obj_port_vlan vlan = { - .vid_begin = vid, - .vid_end = vid, - /* This API only allows programming tagged, non-PVID VIDs */ - .flags = 0, - }; - struct switchdev_trans trans; struct bridge_vlan_info info; int ret;@@ -1010,13 +1003,8 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto, return -EBUSY; } - trans.ph_prepare = true; - ret = dsa_port_vlan_add(dp, &vlan, &trans); - if (ret == -EOPNOTSUPP) - return 0; - - trans.ph_prepare = false; - return dsa_port_vlan_add(dp, &vlan, &trans); + /* This API only allows programming tagged, non-PVID VIDs */ + return dsa_port_trans_vlan_apply(dp, vid, 0, true); } static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
-- Florian