Skip to content

Commit

Permalink
net: switchdev: remove the transaction structure from port attributes
Browse files Browse the repository at this point in the history
Since the introduction of the switchdev API, port attributes were
transmitted to drivers for offloading using a two-step transactional
model, with a prepare phase that was supposed to catch all errors, and a
commit phase that was supposed to never fail.

Some classes of failures can never be avoided, like hardware access, or
memory allocation. In the latter case, merely attempting to move the
memory allocation to the preparation phase makes it impossible to avoid
memory leaks, since commit 91cf8ec ("switchdev: Remove unused
transaction item queue") which has removed the unused mechanism of
passing on the allocated memory between one phase and another.

It is time we admit that separating the preparation from the commit
phase is something that is best left for the driver to decide, and not
something that should be baked into the API, especially since there are
no switchdev callers that depend on this.

This patch removes the struct switchdev_trans member from switchdev port
attribute notifier structures, and converts drivers to not look at this
member.

In part, this patch contains a revert of my previous commit 2e554a7
("net: dsa: propagate switchdev vlan_filtering prepare phase to
drivers").

For the most part, the conversion was trivial except for:
- Rocker's world implementation based on Broadcom OF-DPA had an odd
  implementation of ofdpa_port_attr_bridge_flags_set. The conversion was
  done mechanically, by pasting the implementation twice, then only
  keeping the code that would get executed during prepare phase on top,
  then only keeping the code that gets executed during the commit phase
  on bottom, then simplifying the resulting code until this was obtained.
- DSA's offloading of STP state, bridge flags, VLAN filtering and
  multicast router could be converted right away. But the ageing time
  could not, so a shim was introduced and this was left for a further
  commit.

Signed-off-by: Vladimir Oltean <[email protected]>
Acked-by: Linus Walleij <[email protected]>
Acked-by: Jiri Pirko <[email protected]>
Reviewed-by: Kurt Kanzenbach <[email protected]> # hellcreek
Reviewed-by: Linus Walleij <[email protected]> # RTL8366RB
Reviewed-by: Ido Schimmel <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
Signed-off-by: Jakub Kicinski <[email protected]>
  • Loading branch information
vladimiroltean authored and kuba-moo committed Jan 12, 2021
1 parent cf6def5 commit bae33f2
Show file tree
Hide file tree
Showing 33 changed files with 162 additions and 424 deletions.
6 changes: 1 addition & 5 deletions drivers/net/dsa/b53/b53_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -1374,14 +1374,10 @@ void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
}
EXPORT_SYMBOL(b53_phylink_mac_link_up);

int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
struct switchdev_trans *trans)
int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
{
struct b53_device *dev = ds->priv;

if (switchdev_trans_ph_prepare(trans))
return 0;

b53_enable_vlan(dev, dev->vlan_enabled, vlan_filtering);

return 0;
Expand Down
3 changes: 1 addition & 2 deletions drivers/net/dsa/b53/b53_priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,7 @@ void b53_phylink_mac_link_up(struct dsa_switch *ds, int port,
struct phy_device *phydev,
int speed, int duplex,
bool tx_pause, bool rx_pause);
int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
struct switchdev_trans *trans);
int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering);
int b53_vlan_prepare(struct dsa_switch *ds, int port,
const struct switchdev_obj_port_vlan *vlan);
void b53_vlan_add(struct dsa_switch *ds, int port,
Expand Down
3 changes: 1 addition & 2 deletions drivers/net/dsa/dsa_loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,7 @@ static void dsa_loop_port_stp_state_set(struct dsa_switch *ds, int port,
}

static int dsa_loop_port_vlan_filtering(struct dsa_switch *ds, int port,
bool vlan_filtering,
struct switchdev_trans *trans)
bool vlan_filtering)
{
dev_dbg(ds->dev, "%s: port: %d, vlan_filtering: %d\n",
__func__, port, vlan_filtering);
Expand Down
6 changes: 1 addition & 5 deletions drivers/net/dsa/hirschmann/hellcreek.c
Original file line number Diff line number Diff line change
Expand Up @@ -858,14 +858,10 @@ static int hellcreek_fdb_dump(struct dsa_switch *ds, int port,
}

static int hellcreek_vlan_filtering(struct dsa_switch *ds, int port,
bool vlan_filtering,
struct switchdev_trans *trans)
bool vlan_filtering)
{
struct hellcreek *hellcreek = ds->priv;

if (switchdev_trans_ph_prepare(trans))
return 0;

dev_dbg(hellcreek->dev, "%s VLAN filtering on port %d\n",
vlan_filtering ? "Enable" : "Disable", port);

Expand Down
26 changes: 5 additions & 21 deletions drivers/net/dsa/lantiq_gswip.c
Original file line number Diff line number Diff line change
Expand Up @@ -727,23 +727,14 @@ static int gswip_pce_load_microcode(struct gswip_priv *priv)
}

static int gswip_port_vlan_filtering(struct dsa_switch *ds, int port,
bool vlan_filtering,
struct switchdev_trans *trans)
bool vlan_filtering)
{
struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;
struct gswip_priv *priv = ds->priv;

/* Do not allow changing the VLAN filtering options while in bridge */
if (switchdev_trans_ph_prepare(trans)) {
struct net_device *bridge = dsa_to_port(ds, port)->bridge_dev;

if (!bridge)
return 0;

if (!!(priv->port_vlan_filter & BIT(port)) != vlan_filtering)
return -EIO;

return 0;
}
if (bridge && !!(priv->port_vlan_filter & BIT(port)) != vlan_filtering)
return -EIO;

if (vlan_filtering) {
/* Use port based VLAN tag */
Expand Down Expand Up @@ -781,15 +772,8 @@ static int gswip_setup(struct dsa_switch *ds)

/* disable port fetch/store dma on all ports */
for (i = 0; i < priv->hw_info->max_ports; i++) {
struct switchdev_trans trans;

/* Skip the prepare phase, this shouldn't return an error
* during setup.
*/
trans.ph_prepare = false;

gswip_port_disable(ds, i);
gswip_port_vlan_filtering(ds, i, false, &trans);
gswip_port_vlan_filtering(ds, i, false);
}

/* enable Switch */
Expand Down
6 changes: 1 addition & 5 deletions drivers/net/dsa/microchip/ksz8795.c
Original file line number Diff line number Diff line change
Expand Up @@ -783,14 +783,10 @@ static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
}

static int ksz8795_port_vlan_filtering(struct dsa_switch *ds, int port,
bool flag,
struct switchdev_trans *trans)
bool flag)
{
struct ksz_device *dev = ds->priv;

if (switchdev_trans_ph_prepare(trans))
return 0;

ksz_cfg(dev, S_MIRROR_CTRL, SW_VLAN_ENABLE, flag);

return 0;
Expand Down
6 changes: 1 addition & 5 deletions drivers/net/dsa/microchip/ksz9477.c
Original file line number Diff line number Diff line change
Expand Up @@ -493,14 +493,10 @@ static void ksz9477_flush_dyn_mac_table(struct ksz_device *dev, int port)
}

static int ksz9477_port_vlan_filtering(struct dsa_switch *ds, int port,
bool flag,
struct switchdev_trans *trans)
bool flag)
{
struct ksz_device *dev = ds->priv;

if (switchdev_trans_ph_prepare(trans))
return 0;

if (flag) {
ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL,
PORT_VLAN_LOOKUP_VID_0, true);
Expand Down
6 changes: 1 addition & 5 deletions drivers/net/dsa/mt7530.c
Original file line number Diff line number Diff line change
Expand Up @@ -1376,12 +1376,8 @@ mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid)

static int
mt7530_port_vlan_filtering(struct dsa_switch *ds, int port,
bool vlan_filtering,
struct switchdev_trans *trans)
bool vlan_filtering)
{
if (switchdev_trans_ph_prepare(trans))
return 0;

if (vlan_filtering) {
/* The port is being kept as VLAN-unaware port when bridge is
* set up with vlan_filtering not being set, Otherwise, the
Expand Down
7 changes: 3 additions & 4 deletions drivers/net/dsa/mv88e6xxx/chip.c
Original file line number Diff line number Diff line change
Expand Up @@ -1583,16 +1583,15 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
}

static int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
bool vlan_filtering,
struct switchdev_trans *trans)
bool vlan_filtering)
{
struct mv88e6xxx_chip *chip = ds->priv;
u16 mode = vlan_filtering ? MV88E6XXX_PORT_CTL2_8021Q_MODE_SECURE :
MV88E6XXX_PORT_CTL2_8021Q_MODE_DISABLED;
int err;

if (switchdev_trans_ph_prepare(trans))
return mv88e6xxx_max_vid(chip) ? 0 : -EOPNOTSUPP;
if (!mv88e6xxx_max_vid(chip))
return -EOPNOTSUPP;

mv88e6xxx_reg_lock(chip);
err = mv88e6xxx_port_set_8021q_mode(chip, port, mode);
Expand Down
5 changes: 2 additions & 3 deletions drivers/net/dsa/ocelot/felix.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,11 @@ static int felix_vlan_prepare(struct dsa_switch *ds, int port,
flags & BRIDGE_VLAN_INFO_UNTAGGED);
}

static int felix_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
struct switchdev_trans *trans)
static int felix_vlan_filtering(struct dsa_switch *ds, int port, bool enabled)
{
struct ocelot *ocelot = ds->priv;

return ocelot_port_vlan_filtering(ocelot, port, enabled, trans);
return ocelot_port_vlan_filtering(ocelot, port, enabled);
}

static void felix_vlan_add(struct dsa_switch *ds, int port,
Expand Down
6 changes: 1 addition & 5 deletions drivers/net/dsa/qca8k.c
Original file line number Diff line number Diff line change
Expand Up @@ -1294,14 +1294,10 @@ qca8k_port_fdb_dump(struct dsa_switch *ds, int port,
}

static int
qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
struct switchdev_trans *trans)
qca8k_port_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
{
struct qca8k_priv *priv = ds->priv;

if (switchdev_trans_ph_prepare(trans))
return 0;

if (vlan_filtering) {
qca8k_rmw(priv, QCA8K_PORT_LOOKUP_CTRL(port),
QCA8K_PORT_LOOKUP_VLAN_MODE,
Expand Down
3 changes: 1 addition & 2 deletions drivers/net/dsa/realtek-smi-core.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,7 @@ int rtl8366_enable_vlan(struct realtek_smi *smi, bool enable);
int rtl8366_reset_vlan(struct realtek_smi *smi);
int rtl8366_init_vlan(struct realtek_smi *smi);
int rtl8366_vlan_filtering(struct dsa_switch *ds, int port,
bool vlan_filtering,
struct switchdev_trans *trans);
bool vlan_filtering);
int rtl8366_vlan_prepare(struct dsa_switch *ds, int port,
const struct switchdev_obj_port_vlan *vlan);
void rtl8366_vlan_add(struct dsa_switch *ds, int port,
Expand Down
11 changes: 3 additions & 8 deletions drivers/net/dsa/rtl8366.c
Original file line number Diff line number Diff line change
Expand Up @@ -340,20 +340,15 @@ int rtl8366_init_vlan(struct realtek_smi *smi)
}
EXPORT_SYMBOL_GPL(rtl8366_init_vlan);

int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering,
struct switchdev_trans *trans)
int rtl8366_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
{
struct realtek_smi *smi = ds->priv;
struct rtl8366_vlan_4k vlan4k;
int ret;

/* Use VLAN nr port + 1 since VLAN0 is not valid */
if (switchdev_trans_ph_prepare(trans)) {
if (!smi->ops->is_vlan_valid(smi, port + 1))
return -EINVAL;

return 0;
}
if (!smi->ops->is_vlan_valid(smi, port + 1))
return -EINVAL;

dev_info(smi->dev, "%s filtering on port %d\n",
vlan_filtering ? "enable" : "disable",
Expand Down
3 changes: 1 addition & 2 deletions drivers/net/dsa/sja1105/sja1105.h
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,7 @@ enum sja1105_reset_reason {

int sja1105_static_config_reload(struct sja1105_private *priv,
enum sja1105_reset_reason reason);
int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
struct switchdev_trans *trans);
int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled);
void sja1105_frame_memory_partitioning(struct sja1105_private *priv);

/* From sja1105_devlink.c */
Expand Down
9 changes: 1 addition & 8 deletions drivers/net/dsa/sja1105/sja1105_devlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ static int sja1105_best_effort_vlan_filtering_set(struct sja1105_private *priv,

rtnl_lock();
for (port = 0; port < ds->num_ports; port++) {
struct switchdev_trans trans;
struct dsa_port *dp;

if (!dsa_is_user_port(ds, port))
Expand All @@ -144,13 +143,7 @@ static int sja1105_best_effort_vlan_filtering_set(struct sja1105_private *priv,
dp = dsa_to_port(ds, port);
vlan_filtering = dsa_port_is_vlan_filtering(dp);

trans.ph_prepare = true;
rc = sja1105_vlan_filtering(ds, port, vlan_filtering, &trans);
if (rc)
break;

trans.ph_prepare = false;
rc = sja1105_vlan_filtering(ds, port, vlan_filtering, &trans);
rc = sja1105_vlan_filtering(ds, port, vlan_filtering);
if (rc)
break;
}
Expand Down
17 changes: 6 additions & 11 deletions drivers/net/dsa/sja1105/sja1105_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -2631,8 +2631,7 @@ static int sja1105_vlan_prepare(struct dsa_switch *ds, int port,
* which can only be partially reconfigured at runtime (and not the TPID).
* So a switch reset is required.
*/
int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
struct switchdev_trans *trans)
int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled)
{
struct sja1105_l2_lookup_params_entry *l2_lookup_params;
struct sja1105_general_params_entry *general_params;
Expand All @@ -2644,16 +2643,12 @@ int sja1105_vlan_filtering(struct dsa_switch *ds, int port, bool enabled,
u16 tpid, tpid2;
int rc;

if (switchdev_trans_ph_prepare(trans)) {
list_for_each_entry(rule, &priv->flow_block.rules, list) {
if (rule->type == SJA1105_RULE_VL) {
dev_err(ds->dev,
"Cannot change VLAN filtering with active VL rules\n");
return -EBUSY;
}
list_for_each_entry(rule, &priv->flow_block.rules, list) {
if (rule->type == SJA1105_RULE_VL) {
dev_err(ds->dev,
"Cannot change VLAN filtering with active VL rules\n");
return -EBUSY;
}

return 0;
}

if (enabled) {
Expand Down
Loading

0 comments on commit bae33f2

Please sign in to comment.