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 Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series ar9331: mainline some parts of switch functionality | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 1 maintainers not CCed: linux@rempel-privat.de
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning CHECK: Alignment should match open parenthesis CHECK: Please don't use multiple blank lines WARNING: line length of 99 exceeds 80 columns WARNING: networking block comments don't use an empty /* line, use /* Comment...
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

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

kernel test robot April 3, 2021, 2:20 p.m. UTC | #1 
Hi Oleksij,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Oleksij-Rempel/ar9331-mainline-some-parts-of-switch-functionality/20210403-195131
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 82506665179209e43d3c9d39ffa42f8c8ff968bd
config: powerpc-randconfig-r035-20210403 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 0fe8af94688aa03c01913c2001d6a1a911f42ce6)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc cross compiling tool for clang build
        # apt-get install binutils-powerpc-linux-gnu
        # https://github.com/0day-ci/linux/commit/1d06a86ebc7af4decbdf92e3114a6d983063eca1
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oleksij-Rempel/ar9331-mainline-some-parts-of-switch-functionality/20210403-195131
        git checkout 1d06a86ebc7af4decbdf92e3114a6d983063eca1
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:43:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(insb, (unsigned long p, void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:97:1: note: expanded from here
   __do_insb
   ^
   arch/powerpc/include/asm/io.h:556:56: note: expanded from macro '__do_insb'
   #define __do_insb(p, b, n)      readsb((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
                                          ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/net/dsa/qca/ar9331.c:44:
   In file included from include/linux/of_irq.h:7:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:45:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(insw, (unsigned long p, void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:99:1: note: expanded from here
   __do_insw
   ^
   arch/powerpc/include/asm/io.h:557:56: note: expanded from macro '__do_insw'
   #define __do_insw(p, b, n)      readsw((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
                                          ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/net/dsa/qca/ar9331.c:44:
   In file included from include/linux/of_irq.h:7:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:47:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(insl, (unsigned long p, void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:101:1: note: expanded from here
   __do_insl
   ^
   arch/powerpc/include/asm/io.h:558:56: note: expanded from macro '__do_insl'
   #define __do_insl(p, b, n)      readsl((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
                                          ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/net/dsa/qca/ar9331.c:44:
   In file included from include/linux/of_irq.h:7:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:49:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsb, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:103:1: note: expanded from here
   __do_outsb
   ^
   arch/powerpc/include/asm/io.h:559:58: note: expanded from macro '__do_outsb'
   #define __do_outsb(p, b, n)     writesb((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/net/dsa/qca/ar9331.c:44:
   In file included from include/linux/of_irq.h:7:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:51:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsw, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:105:1: note: expanded from here
   __do_outsw
   ^
   arch/powerpc/include/asm/io.h:560:58: note: expanded from macro '__do_outsw'
   #define __do_outsw(p, b, n)     writesw((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
   In file included from drivers/net/dsa/qca/ar9331.c:44:
   In file included from include/linux/of_irq.h:7:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:53:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsl, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:107:1: note: expanded from here
   __do_outsl
   ^
   arch/powerpc/include/asm/io.h:561:58: note: expanded from macro '__do_outsl'
   #define __do_outsl(p, b, n)     writesl((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
>> drivers/net/dsa/qca/ar9331.c:971:47: warning: variable 'f2' is uninitialized when used here [-Wuninitialized]
           ret = ar9331_sw_port_fdb_write(priv, f0, f1, f2);
                                                        ^~
   drivers/net/dsa/qca/ar9331.c:961:16: note: initialize the variable 'f2' to silence this warning
           u32 f0, f1, f2;
                         ^
                          = 0
   7 warnings generated.


vim +/f2 +971 drivers/net/dsa/qca/ar9331.c

   954	
   955	static int ar9331_sw_port_fdb_rmw(struct ar9331_sw_priv *priv,
   956					  const unsigned char *mac,
   957					  u8 port_mask_set,
   958					  u8 port_mask_clr)
   959	{
   960		struct regmap *regmap = priv->regmap;
   961		u32 f0, f1, f2;
   962		u8 port_mask, port_mask_new, status, func;
   963		int ret;
   964	
   965		ret = ar9331_sw_fdb_wait(priv, &f0);
   966		if (ret)
   967			return ret;
   968	
   969		ar9331_sw_port_fdb_prepare(mac, &f0, &f1, AR9331_SW_AT_FUNC_FIND_MAC);
   970	
 > 971		ret = ar9331_sw_port_fdb_write(priv, f0, f1, f2);
   972		if (ret)
   973			return ret;
   974	
   975		ret = ar9331_sw_fdb_wait(priv, &f0);
   976		if (ret)
   977			return ret;
   978	
   979		ret = regmap_read(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION2, &f2);
   980		if (ret)
   981			return ret;
   982	
   983		port_mask = FIELD_GET(AR9331_SW_AT_DES_PORT, f2);
   984		status = FIELD_GET(AR9331_SW_AT_STATUS, f2);
   985		if (status > 0 && status < AR9331_SW_AT_STATUS_STATIC) {
   986			dev_err_ratelimited(priv->dev, "%s: found existing dynamic entry on %x\n",
   987					    __func__, port_mask);
   988	
   989			if (port_mask_set && port_mask_set != port_mask)
   990				dev_err_ratelimited(priv->dev, "%s: found existing dynamic entry on %x, replacing it with static on %x\n",
   991						    __func__, port_mask, port_mask_set);
   992			port_mask = 0;
   993		} else if (!status && !port_mask_set) {
   994			return 0;
   995		}
   996	
   997		port_mask_new = port_mask & ~port_mask_clr;
   998		port_mask_new |= port_mask_set;
   999	
  1000		if (port_mask_new == port_mask &&
  1001		    status == AR9331_SW_AT_STATUS_STATIC) {
  1002			dev_info(priv->dev, "%s: no need to overwrite existing valid entry on %x\n",
  1003					    __func__, port_mask_new);
  1004			return 0;
  1005		}
  1006	
  1007		if (port_mask_new) {
  1008			func = AR9331_SW_AT_FUNC_LOAD_ENTRY;
  1009		} else {
  1010			func = AR9331_SW_AT_FUNC_PURGE_ENTRY;
  1011			port_mask_new = port_mask;
  1012		}
  1013	
  1014		f2 = FIELD_PREP(AR9331_SW_AT_DES_PORT, port_mask_new) |
  1015			FIELD_PREP(AR9331_SW_AT_STATUS, AR9331_SW_AT_STATUS_STATIC);
  1016	
  1017		ar9331_sw_port_fdb_prepare(mac, &f0, &f1, func);
  1018	
  1019		ret = ar9331_sw_port_fdb_write(priv, f0, f1, f2);
  1020		if (ret)
  1021			return ret;
  1022	
  1023		ret = ar9331_sw_fdb_wait(priv, &f0);
  1024		if (ret)
  1025			return ret;
  1026	
  1027		if (f0 & AR9331_SW_AT_FULL_VIO) {
  1028			/* cleanup error status */
  1029			regmap_write(regmap, AR9331_SW_REG_ADDR_TABLE_FUNCTION0, 0);
  1030			dev_err_ratelimited(priv->dev, "%s: can't add new entry, ATU is full\n", __func__);
  1031			return -ENOMEM;
  1032		}
  1033	
  1034		return 0;
  1035	}
  1036	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Andrew Lunn April 3, 2021, 3:25 p.m. UTC | #2 
> +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 | #3 
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 | #4 
> > 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)