netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46
@ 2023-06-06 14:00 Patrick Thompson
  2023-06-06 14:15 ` Patrick Thompson
  0 siblings, 1 reply; 8+ messages in thread
From: Patrick Thompson @ 2023-06-06 14:00 UTC (permalink / raw)
  To: LKML
  Cc: Patrick Thompson, David S. Miller, Eric Dumazet, Heiner Kallweit,
	Jakub Kicinski, Paolo Abeni, netdev, nic_swsd

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 <ptf@google.com>
---

 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


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

* Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46
  2023-06-06 14:00 [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46 Patrick Thompson
@ 2023-06-06 14:15 ` Patrick Thompson
  2023-06-06 15:11   ` Heiner Kallweit
  0 siblings, 1 reply; 8+ messages in thread
From: Patrick Thompson @ 2023-06-06 14:15 UTC (permalink / raw)
  To: LKML
  Cc: David S. Miller, Eric Dumazet, Heiner Kallweit, Jakub Kicinski,
	Paolo Abeni, netdev, nic_swsd

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 <ptf@google.com> 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 <ptf@google.com>
> ---
>
>  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
>

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

* Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46
  2023-06-06 14:15 ` Patrick Thompson
@ 2023-06-06 15:11   ` Heiner Kallweit
  2023-06-06 21:29     ` Jakub Kicinski
  0 siblings, 1 reply; 8+ messages in thread
From: Heiner Kallweit @ 2023-06-06 15:11 UTC (permalink / raw)
  To: Patrick Thompson, LKML
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev, nic_swsd

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 <ptf@google.com> 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 <ptf@google.com>
>> ---
>>
>>  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")


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

* Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46
  2023-06-06 15:11   ` Heiner Kallweit
@ 2023-06-06 21:29     ` Jakub Kicinski
  2023-09-01 12:28       ` Patrick Thompson
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2023-06-06 21:29 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Patrick Thompson, LKML, David S. Miller, Eric Dumazet,
	Paolo Abeni, netdev, nic_swsd

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. 

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

* Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46
  2023-06-06 21:29     ` Jakub Kicinski
@ 2023-09-01 12:28       ` Patrick Thompson
  2023-09-01 12:48         ` Heiner Kallweit
  0 siblings, 1 reply; 8+ messages in thread
From: Patrick Thompson @ 2023-09-01 12:28 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Heiner Kallweit, LKML, David S. Miller, Eric Dumazet, Paolo Abeni,
	netdev, nic_swsd

Hello,

I was wondering if this should be extended to all RTL_GIGA_MAC_VERs
greater than 35 as well.

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 <kuba@kernel.org> 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.

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

* Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46
  2023-09-01 12:28       ` Patrick Thompson
@ 2023-09-01 12:48         ` Heiner Kallweit
  2023-09-01 14:20           ` Patrick Thompson
  0 siblings, 1 reply; 8+ messages in thread
From: Heiner Kallweit @ 2023-09-01 12:48 UTC (permalink / raw)
  To: Patrick Thompson, Jakub Kicinski
  Cc: LKML, David S. Miller, Eric Dumazet, Paolo Abeni, netdev,
	nic_swsd, Chun-Hao Lin

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 <kuba@kernel.org> 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.


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

* Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46
  2023-09-01 12:48         ` Heiner Kallweit
@ 2023-09-01 14:20           ` Patrick Thompson
  2023-09-01 15:44             ` Heiner Kallweit
  0 siblings, 1 reply; 8+ messages in thread
From: Patrick Thompson @ 2023-09-01 14:20 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Jakub Kicinski, LKML, David S. Miller, Eric Dumazet, Paolo Abeni,
	netdev, nic_swsd, Chun-Hao Lin

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?
------------------

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 <hkallweit1@gmail.com> 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 <kuba@kernel.org> 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.
>

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

* Re: [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46
  2023-09-01 14:20           ` Patrick Thompson
@ 2023-09-01 15:44             ` Heiner Kallweit
  0 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2023-09-01 15:44 UTC (permalink / raw)
  To: Patrick Thompson
  Cc: Jakub Kicinski, LKML, David S. Miller, Eric Dumazet, Paolo Abeni,
	netdev, nic_swsd, Chun-Hao Lin

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 <hkallweit1@gmail.com> 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 <kuba@kernel.org> 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.
>>


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

end of thread, other threads:[~2023-09-01 15:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-06 14:00 [PATCH] r8169: Disable multicast filter for RTL_GIGA_MAC_VER_46 Patrick Thompson
2023-06-06 14:15 ` Patrick Thompson
2023-06-06 15:11   ` Heiner Kallweit
2023-06-06 21:29     ` Jakub Kicinski
2023-09-01 12:28       ` Patrick Thompson
2023-09-01 12:48         ` Heiner Kallweit
2023-09-01 14:20           ` Patrick Thompson
2023-09-01 15:44             ` Heiner Kallweit

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).