All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Woudstra <ericwouds@gmail.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Sean Wang <sean.wang@mediatek.com>,
	Landen Chao <landen.chao@mediatek.com>,
	Andrew Lunn <andrew@lunn.ch>,Vivien Didelot
	<vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	 Jakub Kicinski <kuba@kernel.org>,Matthias Brugger
	<matthias.bgg@gmail.com>,
	netdev@vger.kernel.org,linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,linux-kernel@vger.kernel.org,
	DENG Qingfang <dqfext@gmail.com>,Frank Wunderlich
	<frank-w@public-files.de>
Subject: Re: [PATCH] mt7530 fix mt7530_fdb_write vid missing ivl bit
Date: Sat, 17 Jul 2021 10:09:53 +0200	[thread overview]
Message-ID: <eda300a2-4e36-4d0c-8ea8-eae5e6d62bea@gmail.com> (raw)
In-Reply-To: <20210716210655.i5hxcwau5tdq4zhb@skbuf>


You are right now there is a problem with vlan unaware bridge.

We need to change the line to:

if (vid > 1) reg[1] |= ATA2_IVL;

I have just tested this with a vlan unaware bridge and also with vlan bridge option disabled in the kernel. Only after applying the if statement it works for vlan unaware bridges/kernel.

On Jul 16, 2021, 11:06 PM, at 11:06 PM, Vladimir Oltean <olteanv@gmail.com> wrote:
>On Fri, Jul 16, 2021 at 05:36:39PM +0200, ericwouds@gmail.com wrote:
>> From: Eric Woudstra <ericwouds@gmail.com>
>>
>> According to reference guides mt7530 (mt7620) and mt7531:
>>
>> NOTE: When IVL is reset, MAC[47:0] and FID[2:0] will be used to
>> read/write the address table. When IVL is set, MAC[47:0] and
>CVID[11:0]
>> will be used to read/write the address table.
>>
>> Since the function only fills in CVID and no FID, we need to set the
>> IVL bit. The existing code does not set it.
>>
>> This is a fix for the issue I dropped here earlier:
>>
>>
>http://lists.infradead.org/pipermail/linux-mediatek/2021-June/025697.html
>>
>> With this patch, it is now possible to delete the 'self' fdb entry
>> manually. However, wifi roaming still has the same issue, the entry
>> does not get deleted automatically. Wifi roaming also needs a fix
>> somewhere else to function correctly in combination with vlan.
>>
>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>> ---
>>  drivers/net/dsa/mt7530.c | 1 +
>>  drivers/net/dsa/mt7530.h | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
>> index 93136f7e6..9e4df35f9 100644
>> --- a/drivers/net/dsa/mt7530.c
>> +++ b/drivers/net/dsa/mt7530.c
>> @@ -366,6 +366,7 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16
>vid,
>>  	int i;
>>
>>  	reg[1] |= vid & CVID_MASK;
>> +	reg[1] |= ATA2_IVL;
>>  	reg[2] |= (aging & AGE_TIMER_MASK) << AGE_TIMER;
>>  	reg[2] |= (port_mask & PORT_MAP_MASK) << PORT_MAP;
>>  	/* STATIC_ENT indicate that entry is static wouldn't
>> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
>> index 334d610a5..b19b389ff 100644
>> --- a/drivers/net/dsa/mt7530.h
>> +++ b/drivers/net/dsa/mt7530.h
>> @@ -79,6 +79,7 @@ enum mt753x_bpdu_port_fw {
>>  #define  STATIC_EMP			0
>>  #define  STATIC_ENT			3
>>  #define MT7530_ATA2			0x78
>> +#define  ATA2_IVL			BIT(15)
>>
>>  /* Register for address table write data */
>>  #define MT7530_ATWD			0x7c
>> --
>> 2.25.1
>>
>
>Can VLAN-unaware FDB entries still be manipulated successfully after
>this patch, since it changes 'fid 0' to be interpreted as 'vid 0'?
>
>What is your problem with roaming exactly? Have you tried to disable
>hardware address learning on the CPU port and set
>ds->assisted_learning_on_cpu_port = true for mt7530?
>
>Please note that since kernel v5.14, raw 'self' entries can no longer
>be
>managed directly using 'bridge fdb', you need to use 'master static'
>and
>go through the bridge:
>https://www.kernel.org/doc/html/latest/networking/dsa/configuration.html#forwarding-database-fdb-management
>You will need to update your 'bridgefdbd' program, if it proves to be
>at
>all necessary to achieve what you want.


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
From: Eric Woudstra <ericwouds@gmail.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Sean Wang <sean.wang@mediatek.com>,
	Landen Chao <landen.chao@mediatek.com>,
	Andrew Lunn <andrew@lunn.ch>,Vivien Didelot
	<vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	 Jakub Kicinski <kuba@kernel.org>,Matthias Brugger
	<matthias.bgg@gmail.com>,
	netdev@vger.kernel.org,linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,linux-kernel@vger.kernel.org,
	DENG Qingfang <dqfext@gmail.com>,Frank Wunderlich
	<frank-w@public-files.de>
Subject: Re: [PATCH] mt7530 fix mt7530_fdb_write vid missing ivl bit
Date: Sat, 17 Jul 2021 10:09:53 +0200	[thread overview]
Message-ID: <eda300a2-4e36-4d0c-8ea8-eae5e6d62bea@gmail.com> (raw)
In-Reply-To: <20210716210655.i5hxcwau5tdq4zhb@skbuf>


You are right now there is a problem with vlan unaware bridge.

We need to change the line to:

if (vid > 1) reg[1] |= ATA2_IVL;

I have just tested this with a vlan unaware bridge and also with vlan bridge option disabled in the kernel. Only after applying the if statement it works for vlan unaware bridges/kernel.

On Jul 16, 2021, 11:06 PM, at 11:06 PM, Vladimir Oltean <olteanv@gmail.com> wrote:
>On Fri, Jul 16, 2021 at 05:36:39PM +0200, ericwouds@gmail.com wrote:
>> From: Eric Woudstra <ericwouds@gmail.com>
>>
>> According to reference guides mt7530 (mt7620) and mt7531:
>>
>> NOTE: When IVL is reset, MAC[47:0] and FID[2:0] will be used to
>> read/write the address table. When IVL is set, MAC[47:0] and
>CVID[11:0]
>> will be used to read/write the address table.
>>
>> Since the function only fills in CVID and no FID, we need to set the
>> IVL bit. The existing code does not set it.
>>
>> This is a fix for the issue I dropped here earlier:
>>
>>
>http://lists.infradead.org/pipermail/linux-mediatek/2021-June/025697.html
>>
>> With this patch, it is now possible to delete the 'self' fdb entry
>> manually. However, wifi roaming still has the same issue, the entry
>> does not get deleted automatically. Wifi roaming also needs a fix
>> somewhere else to function correctly in combination with vlan.
>>
>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>> ---
>>  drivers/net/dsa/mt7530.c | 1 +
>>  drivers/net/dsa/mt7530.h | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
>> index 93136f7e6..9e4df35f9 100644
>> --- a/drivers/net/dsa/mt7530.c
>> +++ b/drivers/net/dsa/mt7530.c
>> @@ -366,6 +366,7 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16
>vid,
>>  	int i;
>>
>>  	reg[1] |= vid & CVID_MASK;
>> +	reg[1] |= ATA2_IVL;
>>  	reg[2] |= (aging & AGE_TIMER_MASK) << AGE_TIMER;
>>  	reg[2] |= (port_mask & PORT_MAP_MASK) << PORT_MAP;
>>  	/* STATIC_ENT indicate that entry is static wouldn't
>> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
>> index 334d610a5..b19b389ff 100644
>> --- a/drivers/net/dsa/mt7530.h
>> +++ b/drivers/net/dsa/mt7530.h
>> @@ -79,6 +79,7 @@ enum mt753x_bpdu_port_fw {
>>  #define  STATIC_EMP			0
>>  #define  STATIC_ENT			3
>>  #define MT7530_ATA2			0x78
>> +#define  ATA2_IVL			BIT(15)
>>
>>  /* Register for address table write data */
>>  #define MT7530_ATWD			0x7c
>> --
>> 2.25.1
>>
>
>Can VLAN-unaware FDB entries still be manipulated successfully after
>this patch, since it changes 'fid 0' to be interpreted as 'vid 0'?
>
>What is your problem with roaming exactly? Have you tried to disable
>hardware address learning on the CPU port and set
>ds->assisted_learning_on_cpu_port = true for mt7530?
>
>Please note that since kernel v5.14, raw 'self' entries can no longer
>be
>managed directly using 'bridge fdb', you need to use 'master static'
>and
>go through the bridge:
>https://www.kernel.org/doc/html/latest/networking/dsa/configuration.html#forwarding-database-fdb-management
>You will need to update your 'bridgefdbd' program, if it proves to be
>at
>all necessary to achieve what you want.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Eric Woudstra <ericwouds@gmail.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Sean Wang <sean.wang@mediatek.com>,
	Landen Chao <landen.chao@mediatek.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	DENG Qingfang <dqfext@gmail.com>,
	Frank Wunderlich <frank-w@public-files.de>
Subject: Re: [PATCH] mt7530 fix mt7530_fdb_write vid missing ivl bit
Date: Sat, 17 Jul 2021 10:09:53 +0200	[thread overview]
Message-ID: <eda300a2-4e36-4d0c-8ea8-eae5e6d62bea@gmail.com> (raw)
In-Reply-To: <20210716210655.i5hxcwau5tdq4zhb@skbuf>


You are right now there is a problem with vlan unaware bridge.

We need to change the line to:

if (vid > 1) reg[1] |= ATA2_IVL;

I have just tested this with a vlan unaware bridge and also with vlan bridge option disabled in the kernel. Only after applying the if statement it works for vlan unaware bridges/kernel.

On Jul 16, 2021, 11:06 PM, at 11:06 PM, Vladimir Oltean <olteanv@gmail.com> wrote:
>On Fri, Jul 16, 2021 at 05:36:39PM +0200, ericwouds@gmail.com wrote:
>> From: Eric Woudstra <ericwouds@gmail.com>
>>
>> According to reference guides mt7530 (mt7620) and mt7531:
>>
>> NOTE: When IVL is reset, MAC[47:0] and FID[2:0] will be used to
>> read/write the address table. When IVL is set, MAC[47:0] and
>CVID[11:0]
>> will be used to read/write the address table.
>>
>> Since the function only fills in CVID and no FID, we need to set the
>> IVL bit. The existing code does not set it.
>>
>> This is a fix for the issue I dropped here earlier:
>>
>>
>http://lists.infradead.org/pipermail/linux-mediatek/2021-June/025697.html
>>
>> With this patch, it is now possible to delete the 'self' fdb entry
>> manually. However, wifi roaming still has the same issue, the entry
>> does not get deleted automatically. Wifi roaming also needs a fix
>> somewhere else to function correctly in combination with vlan.
>>
>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>> ---
>>  drivers/net/dsa/mt7530.c | 1 +
>>  drivers/net/dsa/mt7530.h | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
>> index 93136f7e6..9e4df35f9 100644
>> --- a/drivers/net/dsa/mt7530.c
>> +++ b/drivers/net/dsa/mt7530.c
>> @@ -366,6 +366,7 @@ mt7530_fdb_write(struct mt7530_priv *priv, u16
>vid,
>>  	int i;
>>
>>  	reg[1] |= vid & CVID_MASK;
>> +	reg[1] |= ATA2_IVL;
>>  	reg[2] |= (aging & AGE_TIMER_MASK) << AGE_TIMER;
>>  	reg[2] |= (port_mask & PORT_MAP_MASK) << PORT_MAP;
>>  	/* STATIC_ENT indicate that entry is static wouldn't
>> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
>> index 334d610a5..b19b389ff 100644
>> --- a/drivers/net/dsa/mt7530.h
>> +++ b/drivers/net/dsa/mt7530.h
>> @@ -79,6 +79,7 @@ enum mt753x_bpdu_port_fw {
>>  #define  STATIC_EMP			0
>>  #define  STATIC_ENT			3
>>  #define MT7530_ATA2			0x78
>> +#define  ATA2_IVL			BIT(15)
>>
>>  /* Register for address table write data */
>>  #define MT7530_ATWD			0x7c
>> --
>> 2.25.1
>>
>
>Can VLAN-unaware FDB entries still be manipulated successfully after
>this patch, since it changes 'fid 0' to be interpreted as 'vid 0'?
>
>What is your problem with roaming exactly? Have you tried to disable
>hardware address learning on the CPU port and set
>ds->assisted_learning_on_cpu_port = true for mt7530?
>
>Please note that since kernel v5.14, raw 'self' entries can no longer
>be
>managed directly using 'bridge fdb', you need to use 'master static'
>and
>go through the bridge:
>https://www.kernel.org/doc/html/latest/networking/dsa/configuration.html#forwarding-database-fdb-management
>You will need to update your 'bridgefdbd' program, if it proves to be
>at
>all necessary to achieve what you want.


  reply	other threads:[~2021-07-17  8:10 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-16 15:36 [PATCH] mt7530 fix mt7530_fdb_write vid missing ivl bit ericwouds
2021-07-16 15:36 ` ericwouds
2021-07-16 15:36 ` ericwouds
2021-07-16 20:30 ` patchwork-bot+netdevbpf
2021-07-16 20:30   ` patchwork-bot+netdevbpf
2021-07-16 20:30   ` patchwork-bot+netdevbpf
2021-07-16 21:06 ` Vladimir Oltean
2021-07-16 21:06   ` Vladimir Oltean
2021-07-16 21:06   ` Vladimir Oltean
2021-07-17  8:09   ` Eric Woudstra [this message]
2021-07-17  8:09     ` Eric Woudstra
2021-07-17  8:09     ` Eric Woudstra
2021-07-17 13:01     ` Vladimir Oltean
2021-07-17 13:01       ` Vladimir Oltean
2021-07-17 13:01       ` Vladimir Oltean
2021-07-17 15:41     ` DENG Qingfang
2021-07-17 15:41       ` DENG Qingfang
2021-07-17 15:41       ` DENG Qingfang
2021-07-17 19:27       ` Eric Woudstra
2021-07-17 19:27         ` Eric Woudstra
2021-07-17 19:27         ` Eric Woudstra
  -- strict thread matches above, loose matches on Subject: below --
2021-07-16 15:22 ericwouds
2021-07-16 15:22 ` ericwouds
2021-07-16 15:22 ` ericwouds
2021-07-16 15:32 ` Andrew Lunn
2021-07-16 15:32   ` Andrew Lunn
2021-07-16 15:32   ` Andrew Lunn
2021-08-08 17:00 ` DENG Qingfang
2021-08-08 17:00   ` DENG Qingfang
2021-08-08 17:00   ` DENG Qingfang
2021-08-08 23:52   ` Vladimir Oltean
2021-08-08 23:52     ` Vladimir Oltean
2021-08-08 23:52     ` Vladimir Oltean

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eda300a2-4e36-4d0c-8ea8-eae5e6d62bea@gmail.com \
    --to=ericwouds@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=dqfext@gmail.com \
    --cc=f.fainelli@gmail.com \
    --cc=frank-w@public-files.de \
    --cc=kuba@kernel.org \
    --cc=landen.chao@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=sean.wang@mediatek.com \
    --cc=vivien.didelot@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.