LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net] net: dsa: mv88e6xxx: check for address type in port_db_load_purge
@ 2021-07-20  9:24 DENG Qingfang
  2021-07-20 13:45 ` Andrew Lunn
  0 siblings, 1 reply; 2+ messages in thread
From: DENG Qingfang @ 2021-07-20  9:24 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

The same state value of an ATU entry can mean different states,
depending on the entry's address type.
Check for its address type instead of state, to determine if its
portvec should be overridden.

Fixes: f72f2fb8fb6b ("net: dsa: mv88e6xxx: override existent unicast portvec in port_fdb_add")
Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index beb41572d04e..dd4d7fa0da8e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1741,7 +1741,7 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
 		if (!entry.portvec)
 			entry.state = 0;
 	} else {
-		if (state == MV88E6XXX_G1_ATU_DATA_STATE_UC_STATIC)
+		if (is_unicast_ether_addr(addr))
 			entry.portvec = BIT(port);
 		else
 			entry.portvec |= BIT(port);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH net] net: dsa: mv88e6xxx: check for address type in port_db_load_purge
  2021-07-20  9:24 [PATCH net] net: dsa: mv88e6xxx: check for address type in port_db_load_purge DENG Qingfang
@ 2021-07-20 13:45 ` Andrew Lunn
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Lunn @ 2021-07-20 13:45 UTC (permalink / raw)
  To: DENG Qingfang
  Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

On Tue, Jul 20, 2021 at 05:24:26PM +0800, DENG Qingfang wrote:
> The same state value of an ATU entry can mean different states,
> depending on the entry's address type.
> Check for its address type instead of state, to determine if its
> portvec should be overridden.

Please could you expand this description. It is not obvious to me this
change correct.

> 
> Fixes: f72f2fb8fb6b ("net: dsa: mv88e6xxx: override existent unicast portvec in port_fdb_add")
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index beb41572d04e..dd4d7fa0da8e 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1741,7 +1741,7 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
>  		if (!entry.portvec)
>  			entry.state = 0;
>  	} else {
> -		if (state == MV88E6XXX_G1_ATU_DATA_STATE_UC_STATIC)
> +		if (is_unicast_ether_addr(addr))
>  			entry.portvec = BIT(port);
>  		else
>  			entry.portvec |= BIT(port);

I agree that MV88E6XXX_G1_ATU_DATA_STATE_UC_STATIC should only be used
with an address which is know to be unicast. If the address is
multicast, it means it should be considered as a management frame, and
given priority override.

But what exactly are we interested in here? The old code looked like
it wanted to match on static unicast entries. Your change will make it
match on static and dynamic unicast? Do we want dynamic as well? Why
is the fix not this?

> -		if (state == MV88E6XXX_G1_ATU_DATA_STATE_UC_STATIC)
> +		if (is_unicast_ether_addr(addr) && state == MV88E6XXX_G1_ATU_DATA_STATE_UC_STATIC)

which fixes the problem you describe in the commit message.

Thanks
	Andrew

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-07-20 13:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20  9:24 [PATCH net] net: dsa: mv88e6xxx: check for address type in port_db_load_purge DENG Qingfang
2021-07-20 13:45 ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).