2023-06-06 14:20:47

by Patrick Thompson

[permalink] [raw]
Subject: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46

MAC_VER_46 ethernet adapters fail to detect IPv6 multicast packets
unless allmulti is enabled. Add exception for VER_46 in the same way
VER_35 has an exception.

Signed-off-by: Patrick Thompson <[email protected]>
---

drivers/net/ethernet/realtek/r8169_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 4b19803a7dd01..96245e96ee507 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -2583,7 +2583,8 @@ static void rtl_set_rx_mode(struct net_device *dev)
rx_mode |= AcceptAllPhys;
} else if (netdev_mc_count(dev) > MC_FILTER_LIMIT ||
dev->flags & IFF_ALLMULTI ||
- tp->mac_version == RTL_GIGA_MAC_VER_35) {
+ tp->mac_version == RTL_GIGA_MAC_VER_35 ||
+ tp->mac_version == RTL_GIGA_MAC_VER_46) {
/* accept all multicasts */
} else if (netdev_mc_empty(dev)) {
rx_mode &= ~AcceptMulticast;
--
2.41.0.rc0.172.g3f132b7071-goog



2023-06-06 14:33:06

by Patrick Thompson

[permalink] [raw]
Subject: Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46

For added context I came across this issue on this realtek adapter:
10ec:8168:103c:8267. The device erroneously filters multicast packets
that I can see with other adapters using the same netdev settings.


On Tue, Jun 6, 2023 at 10:00 AM Patrick Thompson <[email protected]> wrote:
>
> MAC_VER_46 ethernet adapters fail to detect IPv6 multicast packets
> unless allmulti is enabled. Add exception for VER_46 in the same way
> VER_35 has an exception.
>
> Signed-off-by: Patrick Thompson <[email protected]>
> ---
>
> drivers/net/ethernet/realtek/r8169_main.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 4b19803a7dd01..96245e96ee507 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -2583,7 +2583,8 @@ static void rtl_set_rx_mode(struct net_device *dev)
> rx_mode |= AcceptAllPhys;
> } else if (netdev_mc_count(dev) > MC_FILTER_LIMIT ||
> dev->flags & IFF_ALLMULTI ||
> - tp->mac_version == RTL_GIGA_MAC_VER_35) {
> + tp->mac_version == RTL_GIGA_MAC_VER_35 ||
> + tp->mac_version == RTL_GIGA_MAC_VER_46) {
> /* accept all multicasts */
> } else if (netdev_mc_empty(dev)) {
> rx_mode &= ~AcceptMulticast;
> --
> 2.41.0.rc0.172.g3f132b7071-goog
>

2023-06-06 15:24:22

by Heiner Kallweit

[permalink] [raw]
Subject: Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46

On 06.06.2023 16:15, Patrick Thompson wrote:
> For added context I came across this issue on this realtek adapter:
> 10ec:8168:103c:8267. The device erroneously filters multicast packets
> that I can see with other adapters using the same netdev settings.
>
>
> On Tue, Jun 6, 2023 at 10:00 AM Patrick Thompson <[email protected]> wrote:
>>
>> MAC_VER_46 ethernet adapters fail to detect IPv6 multicast packets
>> unless allmulti is enabled. Add exception for VER_46 in the same way
>> VER_35 has an exception.
>>
>> Signed-off-by: Patrick Thompson <[email protected]>
>> ---
>>
>> drivers/net/ethernet/realtek/r8169_main.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>> index 4b19803a7dd01..96245e96ee507 100644
>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>> @@ -2583,7 +2583,8 @@ static void rtl_set_rx_mode(struct net_device *dev)
>> rx_mode |= AcceptAllPhys;
>> } else if (netdev_mc_count(dev) > MC_FILTER_LIMIT ||
>> dev->flags & IFF_ALLMULTI ||
>> - tp->mac_version == RTL_GIGA_MAC_VER_35) {
>> + tp->mac_version == RTL_GIGA_MAC_VER_35 ||
>> + tp->mac_version == RTL_GIGA_MAC_VER_46) {
>> /* accept all multicasts */
>> } else if (netdev_mc_empty(dev)) {
>> rx_mode &= ~AcceptMulticast;
>> --
>> 2.41.0.rc0.172.g3f132b7071-goog
>>

Thanks for the report and the patch. I just asked a contact in Realtek
whether more chip versions may be affected. Then the patch should be
extended accordingly. Let's wait few days for a response.

I think we should make this a fix. Add the following as Fixes tag
and annotate the patch as "net" (see netdev FAQ).

6e1d0b898818 ("r8169:add support for RTL8168H and RTL8107E")


2023-06-06 22:18:17

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46

On Tue, 6 Jun 2023 17:11:27 +0200 Heiner Kallweit wrote:
> Thanks for the report and the patch. I just asked a contact in Realtek
> whether more chip versions may be affected. Then the patch should be
> extended accordingly. Let's wait few days for a response.
>
> I think we should make this a fix. Add the following as Fixes tag
> and annotate the patch as "net" (see netdev FAQ).
>
> 6e1d0b898818 ("r8169:add support for RTL8168H and RTL8107E")

Perhaps it's best if you repost with the Fixes tag included once
Realtek responded.

2023-09-05 20:06:12

by Heiner Kallweit

[permalink] [raw]
Subject: Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46

On 01.09.2023 16:20, Patrick Thompson wrote:
> Okay sounds good. By the way, here's the patch where the VER_35 logic
> was added. So one question would be are there more chips without
> multicast hardware filters?

There's no public datasheets, therefore nobody but Realtek knows.
Only public reference is their r8168 driver, and interestingly
it uses mc filtering for all chip variants, including what is
version 35 in r8169.

> ------------------
>
> From: Nathan Walp <faceprint@xxxxxxxxxxxxx>
>
> commit 0481776b7a70f09acf7d9d97c288c3a8403fbfe4 upstream.
>
> RTL_GIGA_MAC_VER_35 includes no multicast hardware filter.
>
> Signed-off-by: Nathan Walp <faceprint@xxxxxxxxxxxxx>
> Suggested-by: Hayes Wang <hayeswang@xxxxxxxxxxx>
> Acked-by: Francois Romieu <romieu@xxxxxxxxxxxxx>
> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
> Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@xxxxxxxxxxxxx>
> ---
> drivers/net/ethernet/realtek/r8169.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c
> b/drivers/net/ethernet/realtek/r8169.c
> index eb81da4..e19e1f1 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -4271,6 +4271,9 @@ static void rtl_set_rx_mode(struct net_device *dev)
> mc_filter[1] = swab32(data);
> }
>
> + if (tp->mac_version == RTL_GIGA_MAC_VER_35)
> + mc_filter[1] = mc_filter[0] = 0xffffffff;
> +
> RTL_W32(MAR0 + 4, mc_filter[1]);
> RTL_W32(MAR0 + 0, mc_filter[0]);
>
> --
> 1.7.9.5
>
> --
>
> On Fri, Sep 1, 2023 at 8:48 AM Heiner Kallweit <[email protected]> wrote:
>>
>> On 01.09.2023 14:28, Patrick Thompson wrote:
>>> Hello,
>>>
>>> I was wondering if this should be extended to all RTL_GIGA_MAC_VERs
>>> greater than 35 as well.
>>>
>> I *think* the mc filtering issue with version 35 is different from the
>> one you're seeing. So not every chip version may be affected.
>> As there's no public errata information let's wait for a statement
>> from Realtek.
>>
>>> Realtek responded to me but I was slow to get them packet captures
>>> that they needed. I am hoping to restart things and get back to this
>>> over the finish line if it's a valid patch.
>>>
>>> I will add the appropriate tags and annotations once I hear back.
>>>
>>> On Tue, Jun 6, 2023 at 5:29 PM Jakub Kicinski <[email protected]> wrote:
>>>>
>>>> On Tue, 6 Jun 2023 17:11:27 +0200 Heiner Kallweit wrote:
>>>>> Thanks for the report and the patch. I just asked a contact in Realtek
>>>>> whether more chip versions may be affected. Then the patch should be
>>>>> extended accordingly. Let's wait few days for a response.
>>>>>
>>>>> I think we should make this a fix. Add the following as Fixes tag
>>>>> and annotate the patch as "net" (see netdev FAQ).
>>>>>
>>>>> 6e1d0b898818 ("r8169:add support for RTL8168H and RTL8107E")
>>>>
>>>> Perhaps it's best if you repost with the Fixes tag included once
>>>> Realtek responded.
>>