diff mbox series

[net-next,v1,5/9] net: dsa: qca: ar9331: add forwarding database support

Message ID 20210403114848.30528-6-o.rempel@pengutronix.de (mailing list archive)
State Superseded
Headers show
Series ar9331: mainline some parts of switch functionality | expand

Commit Message

Oleksij Rempel April 3, 2021, 11:48 a.m. UTC
This switch provides simple address resolution table, without VLAN or
multicast specific information.
With this patch we are able now to read, modify unicast and mulicast
addresses.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/qca/ar9331.c | 356 +++++++++++++++++++++++++++++++++++
 1 file changed, 356 insertions(+)

Comments

Andrew Lunn April 3, 2021, 3:25 p.m. UTC | #1
> +static int ar9331_sw_port_fdb_rmw(struct ar9331_sw_priv *priv,
> +				  const unsigned char *mac,
> +				  u8 port_mask_set,
> +				  u8 port_mask_clr)
> +{
> +	port_mask = FIELD_GET(AR9331_SW_AT_DES_PORT, f2);
> +	status = FIELD_GET(AR9331_SW_AT_STATUS, f2);
> +	if (status > 0 && status < AR9331_SW_AT_STATUS_STATIC) {
> +		dev_err_ratelimited(priv->dev, "%s: found existing dynamic entry on %x\n",
> +				    __func__, port_mask);
> +
> +		if (port_mask_set && port_mask_set != port_mask)
> +			dev_err_ratelimited(priv->dev, "%s: found existing dynamic entry on %x, replacing it with static on %x\n",
> +					    __func__, port_mask, port_mask_set);
> +		port_mask = 0;
> +	} else if (!status && !port_mask_set) {
> +		return 0;
> +	}

As a generate rule of thumb, use rate limiting where you have no
control of the number of prints, e.g. it is triggered by packet
processing, and there is potentially a lot of them, which could DOS
the box by a remote or unprivileged attacker.

FDB changes should not happen often. Yes, root my be able to DOS the
box by doing bridge fdb add commands in a loop, but only root should
be able to do that.

Plus, i'm not actually sure we should be issuing warnings here. What
does the bridge code do in this case? Is it silent and just does it,
or does it issue a warning?




> +
> +	port_mask_new = port_mask & ~port_mask_clr;
> +	port_mask_new |= port_mask_set;
> +
> +	if (port_mask_new == port_mask &&
> +	    status == AR9331_SW_AT_STATUS_STATIC) {
> +		dev_info(priv->dev, "%s: no need to overwrite existing valid entry on %x\n",
> +				    __func__, port_mask_new);

This one should probably be dev_dbg().

     Andrew
Vladimir Oltean April 3, 2021, 11:48 p.m. UTC | #2
On Sat, Apr 03, 2021 at 05:25:16PM +0200, Andrew Lunn wrote:
> > +static int ar9331_sw_port_fdb_rmw(struct ar9331_sw_priv *priv,
> > +				  const unsigned char *mac,
> > +				  u8 port_mask_set,
> > +				  u8 port_mask_clr)
> > +{
> > +	port_mask = FIELD_GET(AR9331_SW_AT_DES_PORT, f2);
> > +	status = FIELD_GET(AR9331_SW_AT_STATUS, f2);
> > +	if (status > 0 && status < AR9331_SW_AT_STATUS_STATIC) {
> > +		dev_err_ratelimited(priv->dev, "%s: found existing dynamic entry on %x\n",
> > +				    __func__, port_mask);
> > +
> > +		if (port_mask_set && port_mask_set != port_mask)
> > +			dev_err_ratelimited(priv->dev, "%s: found existing dynamic entry on %x, replacing it with static on %x\n",
> > +					    __func__, port_mask, port_mask_set);
> > +		port_mask = 0;
> > +	} else if (!status && !port_mask_set) {
> > +		return 0;
> > +	}
> 
> As a generate rule of thumb, use rate limiting where you have no
> control of the number of prints, e.g. it is triggered by packet
> processing, and there is potentially a lot of them, which could DOS
> the box by a remote or unprivileged attacker.
> 
> FDB changes should not happen often. Yes, root my be able to DOS the
> box by doing bridge fdb add commands in a loop, but only root should
> be able to do that.

+1
The way I see it, rate limiting should only be done when the user can't
stop the printing from spamming the console, and they just go "argh,
kill it with fire!!!" and throw the box away. As a side note, most of
the time when I can't stop the kernel from printing in a loop, the rate
limit isn't enough to stop me from wanting to throw the box out the
window, but I digress.

> Plus, i'm not actually sure we should be issuing warnings here. What
> does the bridge code do in this case? Is it silent and just does it,
> or does it issue a warning?

:D

What Oleksij doesn't know, I bet, is that he's using the bridge bypass
commands:

bridge fdb add dev lan0 00:01:02:03:04:05

That is the deprecated way of managing FDB entries, and has several
disadvantages such as:
- the bridge software FDB never gets updated with this entry, so other
  drivers which might be subscribed to DSA's FDB (imagine a non-DSA
  driver having the same logic as our ds->assisted_learning_on_cpu_port)
  will never see these FDB entries
- you have to manage duplicates yourself

The correct way to install a static bridge FDB entry is:

bridge fdb add dev lan0 00:01:02:03:04:05 master static

That will error out on duplicates for you.

From my side I would even go as far as deleting the bridge bypass
operations (.ndo_fdb_add and .ndo_fdb_del). The more integration we add
between DSA and the bridge/switchdev, the worse it will be when users
just keep using the bridge bypass. To start that process, I guess we
should start emitting a deprecation warning and then pull the trigger
after a few kernel release cycles.

> > +
> > +	port_mask_new = port_mask & ~port_mask_clr;
> > +	port_mask_new |= port_mask_set;
> > +
> > +	if (port_mask_new == port_mask &&
> > +	    status == AR9331_SW_AT_STATUS_STATIC) {
> > +		dev_info(priv->dev, "%s: no need to overwrite existing valid entry on %x\n",
> > +				    __func__, port_mask_new);
> 
> This one should probably be dev_dbg().

Or deleted, along with the overwrite logic.
Andrew Lunn April 4, 2021, 12:46 a.m. UTC | #3
> > Plus, i'm not actually sure we should be issuing warnings here. What
> > does the bridge code do in this case? Is it silent and just does it,
> > or does it issue a warning?
> 
> :D
> 
> What Oleksij doesn't know, I bet, is that he's using the bridge bypass
> commands:
> 
> bridge fdb add dev lan0 00:01:02:03:04:05
> 
> That is the deprecated way of managing FDB entries, and has several
> disadvantages such as:
> - the bridge software FDB never gets updated with this entry, so other
>   drivers which might be subscribed to DSA's FDB (imagine a non-DSA
>   driver having the same logic as our ds->assisted_learning_on_cpu_port)
>   will never see these FDB entries
> - you have to manage duplicates yourself

I was actually meaning a pure software bridge, with unaccelerated
interfaces. It has a dynamic MAC address in its tables, and the user
adds a static. Ideally, we want the same behaviour.

And i think the answer is:

static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
                  const unsigned char *addr, u16 vid)
{
        struct net_bridge_fdb_entry *fdb;

        if (!is_valid_ether_addr(addr))
                return -EINVAL;

        fdb = br_fdb_find(br, addr, vid);
        if (fdb) {
                /* it is okay to have multiple ports with same
                 * address, just use the first one.
                 */
                if (test_bit(BR_FDB_LOCAL, &fdb->flags))
                        return 0;
                br_warn(br, "adding interface %s with same address as a received packet (addr:%pM, vlan:%u)\n",
                       source ? source->dev->name : br->dev->name, addr, vid);
                fdb_delete(br, fdb, true);
        }

        fdb = fdb_create(br, source, addr, vid,
                         BIT(BR_FDB_LOCAL) | BIT(BR_FDB_STATIC));
        if (!fdb)
                return -ENOMEM;

        fdb_add_hw_addr(br, addr);
        fdb_notify(br, fdb, RTM_NEWNEIGH, true);
        return 0;
}

So it looks like it warns and then replaces the dynamic entry.

So having the DSA driver also warn is maybe O.K. Having said that, i
don't think any other DSA driver does.

   Andrew
diff mbox series

Patch

diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
index a3de3598fbf5..4a98f14f31f4 100644
--- a/drivers/net/dsa/qca/ar9331.c
+++ b/drivers/net/dsa/qca/ar9331.c
@@ -66,6 +66,47 @@ 
 #define AR9331_SW_REG_GLOBAL_CTRL		0x30
 #define AR9331_SW_GLOBAL_CTRL_MFS_M		GENMASK(13, 0)
 
+/* Size of the address resolution table (ARL) */
+#define AR9331_SW_NUM_ARL_RECORDS		1024
+
+#define AR9331_SW_REG_ADDR_TABLE_FUNCTION0	0x50
+#define AR9331_SW_AT_ADDR_BYTES4		GENMASK(31, 24)
+#define AR9331_SW_AT_ADDR_BYTES5		GENMASK(23, 16)
+#define AR9331_SW_AT_FULL_VIO			BIT(12)
+#define AR9331_SW_AT_PORT_NUM			GENMASK(11, 8)
+#define AR9331_SW_AT_FLUSH_STATIC_EN		BIT(4)
+#define AR9331_SW_AT_BUSY			BIT(3)
+#define AR9331_SW_AT_FUNC			GENMASK(2, 0)
+#define AR9331_SW_AT_FUNC_NOP			0
+#define AR9331_SW_AT_FUNC_FLUSH_ALL		1
+#define AR9331_SW_AT_FUNC_LOAD_ENTRY		2
+#define AR9331_SW_AT_FUNC_PURGE_ENTRY		3
+#define AR9331_SW_AT_FUNC_FLUSH_ALL_UNLOCKED	4
+#define AR9331_SW_AT_FUNC_FLUSH_PORT		5
+#define AR9331_SW_AT_FUNC_GET_NEXT		6
+#define AR9331_SW_AT_FUNC_FIND_MAC		7
+
+#define AR9331_SW_REG_ADDR_TABLE_FUNCTION1	0x54
+#define AR9331_SW_AT_ADDR_BYTES0		GENMASK(31, 24)
+#define AR9331_SW_AT_ADDR_BYTES1		GENMASK(23, 16)
+#define AR9331_SW_AT_ADDR_BYTES2		GENMASK(15, 8)
+#define AR9331_SW_AT_ADDR_BYTES3		GENMASK(7, 0)
+
+#define AR9331_SW_REG_ADDR_TABLE_FUNCTION2	0x58
+#define AR9331_SW_AT_COPY_TO_CPU		BIT(26)
+#define AR9331_SW_AT_REDIRECT_TOCPU		BIT(25)
+#define AR9331_SW_AT_LEAKY_EN			BIT(24)
+#define AR9331_SW_AT_STATUS			GENMASK(19, 16)
+#define AR9331_SW_AT_STATUS_EMPTY		0
+/* STATUS values from 7 to 1 are different aging levels */
+#define AR9331_SW_AT_STATUS_STATIC		0xf
+
+#define AR9331_SW_AT_SA_DROP_EN			BIT(14)
+#define AR9331_SW_AT_MIRROR_EN			BIT(13)
+#define AR9331_SW_AT_PRIORITY_EN		BIT(12)
+#define AR9331_SW_AT_PRIORITY			GENMASK(11, 10)
+#define AR9331_SW_AT_DES_PORT			GENMASK(5, 0)
+
 #define AR9331_SW_REG_ADDR_TABLE_CTRL		0x5c
 #define AR9331_SW_AT_ARP_EN			BIT(20)
 #define AR9331_SW_AT_LEARN_CHANGE_EN		BIT(18)
@@ -266,6 +307,12 @@  struct ar9331_sw_port {
 	struct spinlock stats_lock;
 };
 
+struct ar9331_sw_fdb {
+	u8 port_mask;
+	u8 aging;
+	u8 mac[ETH_ALEN];
+};
+
 struct ar9331_sw_priv {
 	struct device *dev;
 	struct dsa_switch ds;
@@ -765,6 +812,309 @@  static void ar9331_get_stats64(struct dsa_switch *ds, int port,
 	spin_unlock(&p->stats_lock);
 }
 
+static int ar9331_sw_fdb_wait(struct ar9331_sw_priv *priv, u32 *f0)
+{
+	struct regmap *regmap = priv->regmap;
+
+	return regmap_read_poll_timeout(regmap,
+					AR9331_SW_REG_ADDR_TABLE_FUNCTION0,
+					*f0, !(*f0 & AR9331_SW_AT_BUSY),
+					10, 2000);
+}
+
+static int ar9331_sw_port_fdb_write(struct ar9331_sw_priv *priv,
+				    u32 f0, u32 f1, u32 f2)
+{
+	struct regmap *regmap = priv->regmap;
+	int ret;
+
+	ret = regmap_write(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION2, f2);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION1, f1);
+	if (ret)
+		return ret;
+
+	return regmap_write(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION0, f0);
+}
+
+static int ar9331_sw_fdb_next(struct ar9331_sw_priv *priv,
+			      struct ar9331_sw_fdb *fdb, int port)
+{
+	struct regmap *regmap = priv->regmap;
+	unsigned int status, ports;
+	u32 f0, f1, f2;
+	int ret;
+
+	/* Keep AT_ADDR_BYTES4/5 to search next entry after current */
+	ret = regmap_update_bits(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION0,
+				 AR9331_SW_AT_FUNC | AR9331_SW_AT_BUSY,
+				 AR9331_SW_AT_BUSY |
+				 FIELD_PREP(AR9331_SW_AT_FUNC,
+					    AR9331_SW_AT_FUNC_GET_NEXT));
+	if (ret)
+		return ret;
+
+	ret = ar9331_sw_fdb_wait(priv, &f0);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION2, &f2);
+	if (ret)
+		return ret;
+
+	/*
+	 * If the hardware returns an MAC != 0 and the AT_STATUS is zero, there
+	 * is no next valid entry in the address table.
+	 */
+	status = FIELD_GET(AR9331_SW_AT_STATUS, f2);
+	fdb->aging = status;
+	if (!status)
+		return 0;
+
+	ret = regmap_read(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION1, &f1);
+	if (ret)
+		return ret;
+
+	fdb->mac[0] = FIELD_GET(AR9331_SW_AT_ADDR_BYTES0, f1);
+	fdb->mac[1] = FIELD_GET(AR9331_SW_AT_ADDR_BYTES1, f1);
+	fdb->mac[2] = FIELD_GET(AR9331_SW_AT_ADDR_BYTES2, f1);
+	fdb->mac[3] = FIELD_GET(AR9331_SW_AT_ADDR_BYTES3, f1);
+	fdb->mac[4] = FIELD_GET(AR9331_SW_AT_ADDR_BYTES4, f0);
+	fdb->mac[5] = FIELD_GET(AR9331_SW_AT_ADDR_BYTES5, f0);
+
+	ports = FIELD_GET(AR9331_SW_AT_DES_PORT, f2);
+	if (!(ports & BIT(port)))
+		return -EAGAIN;
+
+	return 0;
+}
+
+static void ar9331_sw_port_fdb_prepare(const unsigned char *mac, u32 *f0,
+				       u32 *f1, unsigned int func)
+{
+	*f1 = FIELD_PREP(AR9331_SW_AT_ADDR_BYTES0, mac[0]) |
+	      FIELD_PREP(AR9331_SW_AT_ADDR_BYTES1, mac[1]) |
+	      FIELD_PREP(AR9331_SW_AT_ADDR_BYTES2, mac[2]) |
+	      FIELD_PREP(AR9331_SW_AT_ADDR_BYTES3, mac[3]);
+	*f0 = FIELD_PREP(AR9331_SW_AT_ADDR_BYTES4, mac[4]) |
+	      FIELD_PREP(AR9331_SW_AT_ADDR_BYTES5, mac[5]) |
+	      FIELD_PREP(AR9331_SW_AT_FUNC, func) | AR9331_SW_AT_BUSY;
+}
+
+static int ar9331_sw_port_fdb_dump(struct dsa_switch *ds, int port,
+				   dsa_fdb_dump_cb_t *cb, void *data)
+{
+	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+	int cnt = AR9331_SW_NUM_ARL_RECORDS;
+	struct ar9331_sw_fdb _fdb = { 0 };
+	bool is_static;
+	int ret;
+	u32 f0;
+
+	/*
+	 * Make sure no pending operation is in progress. Since no timeout and
+	 * interval values are documented, we use here "seems to be sane, works
+	 * for me" values.
+	 */
+	ret = ar9331_sw_fdb_wait(priv, &f0);
+	if (ret)
+		return ret;
+
+	/*
+	 * If the address and the AT_STATUS are both zero, the hardware will
+	 * search the first valid entry from entry0.
+	 * If the address is set to zero and the AT_STATUS is not zero, the
+	 * hardware will discover the next valid entry which has an address
+	 * of 0x0.
+	 */
+	ret = ar9331_sw_port_fdb_write(priv, 0, 0, 0);
+	if (ret)
+		return ret;
+
+	while (cnt--) {
+		ret = ar9331_sw_fdb_next(priv, &_fdb, port);
+		if (ret == -EAGAIN)
+			continue;
+		else if (ret)
+			return ret;
+
+		if (!_fdb.aging)
+			break;
+
+		is_static = (_fdb.aging == AR9331_SW_AT_STATUS_STATIC);
+		ret = cb(_fdb.mac, 0, is_static, data);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+static int ar9331_sw_port_fdb_rmw(struct ar9331_sw_priv *priv,
+				  const unsigned char *mac,
+				  u8 port_mask_set,
+				  u8 port_mask_clr)
+{
+	struct regmap *regmap = priv->regmap;
+	u32 f0, f1, f2;
+	u8 port_mask, port_mask_new, status, func;
+	int ret;
+
+	ret = ar9331_sw_fdb_wait(priv, &f0);
+	if (ret)
+		return ret;
+
+	ar9331_sw_port_fdb_prepare(mac, &f0, &f1, AR9331_SW_AT_FUNC_FIND_MAC);
+
+	ret = ar9331_sw_port_fdb_write(priv, f0, f1, f2);
+	if (ret)
+		return ret;
+
+	ret = ar9331_sw_fdb_wait(priv, &f0);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION2, &f2);
+	if (ret)
+		return ret;
+
+	port_mask = FIELD_GET(AR9331_SW_AT_DES_PORT, f2);
+	status = FIELD_GET(AR9331_SW_AT_STATUS, f2);
+	if (status > 0 && status < AR9331_SW_AT_STATUS_STATIC) {
+		dev_err_ratelimited(priv->dev, "%s: found existing dynamic entry on %x\n",
+				    __func__, port_mask);
+
+		if (port_mask_set && port_mask_set != port_mask)
+			dev_err_ratelimited(priv->dev, "%s: found existing dynamic entry on %x, replacing it with static on %x\n",
+					    __func__, port_mask, port_mask_set);
+		port_mask = 0;
+	} else if (!status && !port_mask_set) {
+		return 0;
+	}
+
+	port_mask_new = port_mask & ~port_mask_clr;
+	port_mask_new |= port_mask_set;
+
+	if (port_mask_new == port_mask &&
+	    status == AR9331_SW_AT_STATUS_STATIC) {
+		dev_info(priv->dev, "%s: no need to overwrite existing valid entry on %x\n",
+				    __func__, port_mask_new);
+		return 0;
+	}
+
+	if (port_mask_new) {
+		func = AR9331_SW_AT_FUNC_LOAD_ENTRY;
+	} else {
+		func = AR9331_SW_AT_FUNC_PURGE_ENTRY;
+		port_mask_new = port_mask;
+	}
+
+	f2 = FIELD_PREP(AR9331_SW_AT_DES_PORT, port_mask_new) |
+		FIELD_PREP(AR9331_SW_AT_STATUS, AR9331_SW_AT_STATUS_STATIC);
+
+	ar9331_sw_port_fdb_prepare(mac, &f0, &f1, func);
+
+	ret = ar9331_sw_port_fdb_write(priv, f0, f1, f2);
+	if (ret)
+		return ret;
+
+	ret = ar9331_sw_fdb_wait(priv, &f0);
+	if (ret)
+		return ret;
+
+	if (f0 & AR9331_SW_AT_FULL_VIO) {
+		/* cleanup error status */
+		regmap_write(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION0, 0);
+		dev_err_ratelimited(priv->dev, "%s: can't add new entry, ATU is full\n", __func__);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+
+
+static int ar9331_sw_port_fdb_add(struct dsa_switch *ds, int port,
+				  const unsigned char *mac, u16 vid)
+{
+	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+	u16 port_mask = BIT(port);
+
+	dev_info(priv->dev, "%s(%pM, %x)\n", __func__, mac, port);
+
+	if (vid)
+		return -EINVAL;
+
+	return ar9331_sw_port_fdb_rmw(priv, mac, port_mask, 0);
+}
+
+static int ar9331_sw_port_fdb_del(struct dsa_switch *ds, int port,
+			       const unsigned char *mac, u16 vid)
+{
+	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+	u16 port_mask = BIT(port);
+
+	if (vid)
+		return -EINVAL;
+
+	return ar9331_sw_port_fdb_rmw(priv, mac, 0, port_mask);
+}
+
+static int ar9331_sw_port_mdb_add(struct dsa_switch *ds, int port,
+				  const struct switchdev_obj_port_mdb *mdb)
+{
+	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+	u16 port_mask = BIT(port);
+
+	if (mdb->vid)
+		return -EOPNOTSUPP;
+
+	return ar9331_sw_port_fdb_rmw(priv, mdb->addr, port_mask, 0);
+}
+
+static int ar9331_sw_port_mdb_del(struct dsa_switch *ds, int port,
+				  const struct switchdev_obj_port_mdb *mdb)
+{
+	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+	u16 port_mask = BIT(port);
+
+	if (mdb->vid)
+		return -EOPNOTSUPP;
+
+	return ar9331_sw_port_fdb_rmw(priv, mdb->addr, 0, port_mask);
+}
+
+static void ar9331_sw_port_fast_age(struct dsa_switch *ds, int port)
+{
+	struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+	struct regmap *regmap = priv->regmap;
+	int ret;
+	u32 f0;
+
+	ret = ar9331_sw_fdb_wait(priv, &f0);
+	if (ret)
+		goto error;
+
+	/* Flush all non static unicast address on a given port */
+	f0 = FIELD_PREP(AR9331_SW_AT_PORT_NUM, port) |
+		FIELD_PREP(AR9331_SW_AT_FUNC, AR9331_SW_AT_FUNC_FLUSH_PORT) |
+		AR9331_SW_AT_BUSY;
+
+	ret = regmap_write(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION0, f0);
+	if (ret)
+		goto error;
+
+	ret = ar9331_sw_fdb_wait(priv, &f0);
+	if (ret)
+		goto error;
+
+	return;
+error:
+	dev_err_ratelimited(priv->dev, "%s: error: %i\n", __func__, ret);
+}
+
 static const struct dsa_switch_ops ar9331_sw_ops = {
 	.get_tag_protocol	= ar9331_sw_get_tag_protocol,
 	.setup			= ar9331_sw_setup,
@@ -774,6 +1124,12 @@  static const struct dsa_switch_ops ar9331_sw_ops = {
 	.phylink_mac_link_down	= ar9331_sw_phylink_mac_link_down,
 	.phylink_mac_link_up	= ar9331_sw_phylink_mac_link_up,
 	.get_stats64		= ar9331_get_stats64,
+	.port_fast_age          = ar9331_sw_port_fast_age,
+	.port_fdb_del		= ar9331_sw_port_fdb_del,
+	.port_fdb_add		= ar9331_sw_port_fdb_add,
+	.port_fdb_dump		= ar9331_sw_port_fdb_dump,
+	.port_mdb_add           = ar9331_sw_port_mdb_add,
+	.port_mdb_del           = ar9331_sw_port_mdb_del,
 };
 
 static irqreturn_t ar9331_sw_irq(int irq, void *data)