[PATCH net] r8169: fix multicast tx issue with macvlan interface

[PATCH net] r8169: fix multicast tx issue with macvlan interface

[Charles Daymand]

During kernel upgrade testing on our hardware, we found that macvlan
interface were no longer able to send valid multicast packet.

tcpdump run on our hardware was correctly showing our multicast
packet but when connecting a laptop to our hardware we didn't see any
packets.

Bisecting turned up commit 93681cd7d94f
"r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
which is responsible for the drop of packet in case of macvlan
interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
case since TSO was keep disabled.

Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
issue, but we believe that this hardware issue is important enough to
keep tx checksum off by default on this revision.

The change is deactivating the default value of NETIF_F_IP_CSUM for this
specific revision.

Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr>
---
 net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
index a9bdafd15a35..3b69135fc500 100644
--- a/net/drivers/net/ethernet/realtek/r8169_main.c
+++ b/net/drivers/net/ethernet/realtek/r8169_main.c
@@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
 		dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
 		dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
+		if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
+			dev->features &= ~NETIF_F_IP_CSUM;
+		}
 	}
 
 	dev->hw_features |= NETIF_F_RXALL;
-- 
2.20.1

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Heiner Kallweit]

On 27.03.2020 10:08, Charles Daymand wrote:
> During kernel upgrade testing on our hardware, we found that macvlan
> interface were no longer able to send valid multicast packet.
> 
> tcpdump run on our hardware was correctly showing our multicast
> packet but when connecting a laptop to our hardware we didn't see any
> packets.
> 
> Bisecting turned up commit 93681cd7d94f
> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
> which is responsible for the drop of packet in case of macvlan
> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
> case since TSO was keep disabled.
> 
> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
> issue, but we believe that this hardware issue is important enough to
> keep tx checksum off by default on this revision.
> 
> The change is deactivating the default value of NETIF_F_IP_CSUM for this
> specific revision.
> 

The referenced commit may not be the root cause but just reveal another
issue that has been existing before. Root cause may be in the net core
or somewhere else. Did you check with other RTL8168 versions to verify
that it's indeed a HW issue with this specific chip version?

What you could do: Enable tx checksumming manually (via ethtool) on
older kernel versions and check whether they are fine or not.
If an older version is fine, then you can start a new bisect with tx
checksumming enabled.

And did you capture and analyze traffic to verify that actually the
checksum is incorrect (and packets discarded therefore on receiving end)?


> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr>
> ---
>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
> index a9bdafd15a35..3b69135fc500 100644
> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>  		dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>  		dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> +		if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
> +			dev->features &= ~NETIF_F_IP_CSUM;
> +		}
>  	}
>  
>  	dev->hw_features |= NETIF_F_RXALL;
> 

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Heiner Kallweit]

On 27.03.2020 10:39, Heiner Kallweit wrote:
> On 27.03.2020 10:08, Charles Daymand wrote:
>> During kernel upgrade testing on our hardware, we found that macvlan
>> interface were no longer able to send valid multicast packet.
>>
>> tcpdump run on our hardware was correctly showing our multicast
>> packet but when connecting a laptop to our hardware we didn't see any
>> packets.
>>
>> Bisecting turned up commit 93681cd7d94f
>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
>> which is responsible for the drop of packet in case of macvlan
>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
>> case since TSO was keep disabled.
>>
>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
>> issue, but we believe that this hardware issue is important enough to
>> keep tx checksum off by default on this revision.
>>
>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
>> specific revision.
>>
> 
> The referenced commit may not be the root cause but just reveal another
> issue that has been existing before. Root cause may be in the net core
> or somewhere else. Did you check with other RTL8168 versions to verify
> that it's indeed a HW issue with this specific chip version?
> 
> What you could do: Enable tx checksumming manually (via ethtool) on
> older kernel versions and check whether they are fine or not.
> If an older version is fine, then you can start a new bisect with tx
> checksumming enabled.
> 
> And did you capture and analyze traffic to verify that actually the
> checksum is incorrect (and packets discarded therefore on receiving end)?
> 
> 
>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr>
>> ---
>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
>> index a9bdafd15a35..3b69135fc500 100644
>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  		dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>  		dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>  		dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>> +		if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
>> +			dev->features &= ~NETIF_F_IP_CSUM;
>> +		}
>>  	}
>>  
>>  	dev->hw_features |= NETIF_F_RXALL;
>>
> 

After looking a little bit at the macvlen code I think there might be an
issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
seem to have a dedicated maintainer).

r8169 implements a ndo_features_check callback that disables tx checksumming
for the chip version in question and small packets (due to a HW issue).
macvlen uses passthru_features_check() as ndo_features_check callback, this
seems to indicate to me that the ndo_features_check callback of lowerdev is
ignored. This could explain the issue you see.

Would be interesting to see whether it fixes your issue if you let the
macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?

By the way:
Also the ndo_fix_features callback of lowerdev seems to be ignored.

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Eric Dumazet]

On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 27.03.2020 10:39, Heiner Kallweit wrote:
> > On 27.03.2020 10:08, Charles Daymand wrote:
> >> During kernel upgrade testing on our hardware, we found that macvlan
> >> interface were no longer able to send valid multicast packet.
> >>
> >> tcpdump run on our hardware was correctly showing our multicast
> >> packet but when connecting a laptop to our hardware we didn't see any
> >> packets.
> >>
> >> Bisecting turned up commit 93681cd7d94f
> >> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
> >> which is responsible for the drop of packet in case of macvlan
> >> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
> >> case since TSO was keep disabled.
> >>
> >> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
> >> issue, but we believe that this hardware issue is important enough to
> >> keep tx checksum off by default on this revision.
> >>
> >> The change is deactivating the default value of NETIF_F_IP_CSUM for this
> >> specific revision.
> >>
> >
> > The referenced commit may not be the root cause but just reveal another
> > issue that has been existing before. Root cause may be in the net core
> > or somewhere else. Did you check with other RTL8168 versions to verify
> > that it's indeed a HW issue with this specific chip version?
> >
> > What you could do: Enable tx checksumming manually (via ethtool) on
> > older kernel versions and check whether they are fine or not.
> > If an older version is fine, then you can start a new bisect with tx
> > checksumming enabled.
> >
> > And did you capture and analyze traffic to verify that actually the
> > checksum is incorrect (and packets discarded therefore on receiving end)?
> >
> >
> >> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
> >> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr>
> >> ---
> >>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
> >> index a9bdafd15a35..3b69135fc500 100644
> >> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
> >> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
> >> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> >>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
> >> +                    dev->features &= ~NETIF_F_IP_CSUM;
> >> +            }
> >>      }
> >>
> >>      dev->hw_features |= NETIF_F_RXALL;
> >>
> >
>
> After looking a little bit at the macvlen code I think there might be an
> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
> seem to have a dedicated maintainer).
>
> r8169 implements a ndo_features_check callback that disables tx checksumming
> for the chip version in question and small packets (due to a HW issue).
> macvlen uses passthru_features_check() as ndo_features_check callback, this
> seems to indicate to me that the ndo_features_check callback of lowerdev is
> ignored. This could explain the issue you see.
>

macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
so the second __dev_queue_xmit() should eventually call the real_dev
ndo_features_check()



> Would be interesting to see whether it fixes your issue if you let the
> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
>
> By the way:
> Also the ndo_fix_features callback of lowerdev seems to be ignored.

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Heiner Kallweit]

On 27.03.2020 19:52, Eric Dumazet wrote:
> On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> On 27.03.2020 10:39, Heiner Kallweit wrote:
>>> On 27.03.2020 10:08, Charles Daymand wrote:
>>>> During kernel upgrade testing on our hardware, we found that macvlan
>>>> interface were no longer able to send valid multicast packet.
>>>>
>>>> tcpdump run on our hardware was correctly showing our multicast
>>>> packet but when connecting a laptop to our hardware we didn't see any
>>>> packets.
>>>>
>>>> Bisecting turned up commit 93681cd7d94f
>>>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
>>>> which is responsible for the drop of packet in case of macvlan
>>>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
>>>> case since TSO was keep disabled.
>>>>
>>>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
>>>> issue, but we believe that this hardware issue is important enough to
>>>> keep tx checksum off by default on this revision.
>>>>
>>>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
>>>> specific revision.
>>>>
>>>
>>> The referenced commit may not be the root cause but just reveal another
>>> issue that has been existing before. Root cause may be in the net core
>>> or somewhere else. Did you check with other RTL8168 versions to verify
>>> that it's indeed a HW issue with this specific chip version?
>>>
>>> What you could do: Enable tx checksumming manually (via ethtool) on
>>> older kernel versions and check whether they are fine or not.
>>> If an older version is fine, then you can start a new bisect with tx
>>> checksumming enabled.
>>>
>>> And did you capture and analyze traffic to verify that actually the
>>> checksum is incorrect (and packets discarded therefore on receiving end)?
>>>
>>>
>>>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
>>>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr>
>>>> ---
>>>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
>>>> index a9bdafd15a35..3b69135fc500 100644
>>>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
>>>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
>>>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
>>>> +                    dev->features &= ~NETIF_F_IP_CSUM;
>>>> +            }
>>>>      }
>>>>
>>>>      dev->hw_features |= NETIF_F_RXALL;
>>>>
>>>
>>
>> After looking a little bit at the macvlen code I think there might be an
>> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
>> seem to have a dedicated maintainer).
>>
>> r8169 implements a ndo_features_check callback that disables tx checksumming
>> for the chip version in question and small packets (due to a HW issue).
>> macvlen uses passthru_features_check() as ndo_features_check callback, this
>> seems to indicate to me that the ndo_features_check callback of lowerdev is
>> ignored. This could explain the issue you see.
>>
> 
> macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
> so the second __dev_queue_xmit() should eventually call the real_dev
> ndo_features_check()
> 
Thanks, Eric. There's a second path in macvlan_queue_xmit() calling
dev_forward_skb(vlan->lowerdev, skb). Does what you said apply also there?

Still I find it strange that a tx hw checksumming issue should affect multicasts
only. Also the chip version in question is quite common and I would expect
others to have hit the same issue.
Maybe best would be to re-test on the affected system w/o involving macvlen.

> 
> 
>> Would be interesting to see whether it fixes your issue if you let the
>> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
>>
>> By the way:
>> Also the ndo_fix_features callback of lowerdev seems to be ignored.

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Eric Dumazet]

On Fri, Mar 27, 2020 at 12:17 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 27.03.2020 19:52, Eric Dumazet wrote:
> > On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>
> >> On 27.03.2020 10:39, Heiner Kallweit wrote:
> >>> On 27.03.2020 10:08, Charles Daymand wrote:
> >>>> During kernel upgrade testing on our hardware, we found that macvlan
> >>>> interface were no longer able to send valid multicast packet.
> >>>>
> >>>> tcpdump run on our hardware was correctly showing our multicast
> >>>> packet but when connecting a laptop to our hardware we didn't see any
> >>>> packets.
> >>>>
> >>>> Bisecting turned up commit 93681cd7d94f
> >>>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
> >>>> which is responsible for the drop of packet in case of macvlan
> >>>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
> >>>> case since TSO was keep disabled.
> >>>>
> >>>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
> >>>> issue, but we believe that this hardware issue is important enough to
> >>>> keep tx checksum off by default on this revision.
> >>>>
> >>>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
> >>>> specific revision.
> >>>>
> >>>
> >>> The referenced commit may not be the root cause but just reveal another
> >>> issue that has been existing before. Root cause may be in the net core
> >>> or somewhere else. Did you check with other RTL8168 versions to verify
> >>> that it's indeed a HW issue with this specific chip version?
> >>>
> >>> What you could do: Enable tx checksumming manually (via ethtool) on
> >>> older kernel versions and check whether they are fine or not.
> >>> If an older version is fine, then you can start a new bisect with tx
> >>> checksumming enabled.
> >>>
> >>> And did you capture and analyze traffic to verify that actually the
> >>> checksum is incorrect (and packets discarded therefore on receiving end)?
> >>>
> >>>
> >>>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
> >>>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr>
> >>>> ---
> >>>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
> >>>>  1 file changed, 3 insertions(+)
> >>>>
> >>>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
> >>>> index a9bdafd15a35..3b69135fc500 100644
> >>>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
> >>>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
> >>>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> >>>>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >>>>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >>>>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >>>> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
> >>>> +                    dev->features &= ~NETIF_F_IP_CSUM;
> >>>> +            }
> >>>>      }
> >>>>
> >>>>      dev->hw_features |= NETIF_F_RXALL;
> >>>>
> >>>
> >>
> >> After looking a little bit at the macvlen code I think there might be an
> >> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
> >> seem to have a dedicated maintainer).
> >>
> >> r8169 implements a ndo_features_check callback that disables tx checksumming
> >> for the chip version in question and small packets (due to a HW issue).
> >> macvlen uses passthru_features_check() as ndo_features_check callback, this
> >> seems to indicate to me that the ndo_features_check callback of lowerdev is
> >> ignored. This could explain the issue you see.
> >>
> >
> > macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
> > so the second __dev_queue_xmit() should eventually call the real_dev
> > ndo_features_check()
> >
> Thanks, Eric. There's a second path in macvlan_queue_xmit() calling
> dev_forward_skb(vlan->lowerdev, skb). Does what you said apply also there?

This path wont send packets to the physical NIC, packets are injected
back via dev_forward_skb()

>
> Still I find it strange that a tx hw checksumming issue should affect multicasts
> only. Also the chip version in question is quite common and I would expect
> others to have hit the same issue.
> Maybe best would be to re-test on the affected system w/o involving macvlen.
>
> >
> >
> >> Would be interesting to see whether it fixes your issue if you let the
> >> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
> >>
> >> By the way:
> >> Also the ndo_fix_features callback of lowerdev seems to be ignored.
>

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Charles DAYMAND]

Hello,
We tested to enable tx checksumming manually (via ethtool) on a kernel
4.19.0-5-amd64 which is the oldest kernel compatible with our software
and we observed exactly the same issue.
For information when connecting a laptop directly to the interface we
can't see any multicast packet when tx checksumming is enabled on
tcpdump.
Our network is composed of a cisco switch and we can still see the
multicast counters correctly increasing even when we have the issue.

I also confirm that when not using macvlan but the real interface
there is no issue on the multicast packets, they are correctly sent
and received.
I have a stupid question, if the IP checksum was bad on the multicast
packet, would the receiver NIC drop the packet or would it be seen by
tcpdump by the receiver ?

Le ven. 27 mars 2020 à 20:43, Eric Dumazet <edumazet@google.com> a écrit :
>
> On Fri, Mar 27, 2020 at 12:17 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >
> > On 27.03.2020 19:52, Eric Dumazet wrote:
> > > On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> > >>
> > >> On 27.03.2020 10:39, Heiner Kallweit wrote:
> > >>> On 27.03.2020 10:08, Charles Daymand wrote:
> > >>>> During kernel upgrade testing on our hardware, we found that macvlan
> > >>>> interface were no longer able to send valid multicast packet.
> > >>>>
> > >>>> tcpdump run on our hardware was correctly showing our multicast
> > >>>> packet but when connecting a laptop to our hardware we didn't see any
> > >>>> packets.
> > >>>>
> > >>>> Bisecting turned up commit 93681cd7d94f
> > >>>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
> > >>>> which is responsible for the drop of packet in case of macvlan
> > >>>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
> > >>>> case since TSO was keep disabled.
> > >>>>
> > >>>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
> > >>>> issue, but we believe that this hardware issue is important enough to
> > >>>> keep tx checksum off by default on this revision.
> > >>>>
> > >>>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
> > >>>> specific revision.
> > >>>>
> > >>>
> > >>> The referenced commit may not be the root cause but just reveal another
> > >>> issue that has been existing before. Root cause may be in the net core
> > >>> or somewhere else. Did you check with other RTL8168 versions to verify
> > >>> that it's indeed a HW issue with this specific chip version?
> > >>>
> > >>> What you could do: Enable tx checksumming manually (via ethtool) on
> > >>> older kernel versions and check whether they are fine or not.
> > >>> If an older version is fine, then you can start a new bisect with tx
> > >>> checksumming enabled.
> > >>>
> > >>> And did you capture and analyze traffic to verify that actually the
> > >>> checksum is incorrect (and packets discarded therefore on receiving end)?
> > >>>
> > >>>
> > >>>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
> > >>>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr>
> > >>>> ---
> > >>>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
> > >>>>  1 file changed, 3 insertions(+)
> > >>>>
> > >>>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
> > >>>> index a9bdafd15a35..3b69135fc500 100644
> > >>>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
> > >>>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
> > >>>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >>>>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> > >>>>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> > >>>>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> > >>>> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
> > >>>> +                    dev->features &= ~NETIF_F_IP_CSUM;
> > >>>> +            }
> > >>>>      }
> > >>>>
> > >>>>      dev->hw_features |= NETIF_F_RXALL;
> > >>>>
> > >>>
> > >>
> > >> After looking a little bit at the macvlen code I think there might be an
> > >> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
> > >> seem to have a dedicated maintainer).
> > >>
> > >> r8169 implements a ndo_features_check callback that disables tx checksumming
> > >> for the chip version in question and small packets (due to a HW issue).
> > >> macvlen uses passthru_features_check() as ndo_features_check callback, this
> > >> seems to indicate to me that the ndo_features_check callback of lowerdev is
> > >> ignored. This could explain the issue you see.
> > >>
> > >
> > > macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
> > > so the second __dev_queue_xmit() should eventually call the real_dev
> > > ndo_features_check()
> > >
> > Thanks, Eric. There's a second path in macvlan_queue_xmit() calling
> > dev_forward_skb(vlan->lowerdev, skb). Does what you said apply also there?
>
> This path wont send packets to the physical NIC, packets are injected
> back via dev_forward_skb()
>
> >
> > Still I find it strange that a tx hw checksumming issue should affect multicasts
> > only. Also the chip version in question is quite common and I would expect
> > others to have hit the same issue.
> > Maybe best would be to re-test on the affected system w/o involving macvlen.
> >
> > >
> > >
> > >> Would be interesting to see whether it fixes your issue if you let the
> > >> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
> > >>
> > >> By the way:
> > >> Also the ndo_fix_features callback of lowerdev seems to be ignored.
> >



-- 

Charles Daymand

Développeur infrastructure

26 rue de Berri 75008 Paris

Assistance dédiée responsable de site - 01 70 70 46 70
Assistance utilisateur - 01 70 70 46 26



-- 

Charles Daymand

Développeur infrastructure

26 rue de Berri 75008 Paris

Assistance dédiée responsable de site - 01 70 70 46 70
Assistance utilisateur - 01 70 70 46 26

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Heiner Kallweit]

Thanks for further testing! The good news from my perspective is that the issue doesn't occur
w/o macvlen, therefore it doesn't seem to be a r8169 network driver issue.

W/o knowing tcpdump in detail I think it switches the NIC to promiscuous mode, means
it should see all packets, incl. the ones with checksum errors.
Maybe you can mirror the port to which the problematic system is connected and
analyze the traffic. Or for whatever reason the switch doesn't forward the multicast
packets to your notebook.

Heiner


On 31.03.2020 15:44, Charles DAYMAND wrote:
> Hello,
> We tested to enable tx checksumming manually (via ethtool) on a kernel 4.19.0-5-amd64 which is the oldest kernel compatible with our software and we observed exactly the same issue.
> For information when connecting a laptop directly to the interface we can't see any multicast packet when tx checksumming is enabled on tcpdump.
> Our network is composed of a cisco switch and we can still see the multicast counters correctly increasing even when we have the issue.
> 
> I also confirm that when not using macvlan but the real interface there is no issue on the multicast packets, they are correctly sent and received.
> I have a stupid question, if the IP checksum was bad on the multicast packet, would the receiver NIC drop the packet or would it be seen by tcpdump by the receiver ?
> 
> Le ven. 27 mars 2020 à 20:43, Eric Dumazet <edumazet@google.com <mailto:edumazet@google.com>> a écrit :
> 
>     On Fri, Mar 27, 2020 at 12:17 PM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
>     >
>     > On 27.03.2020 19:52, Eric Dumazet wrote:
>     > > On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
>     > >>
>     > >> On 27.03.2020 10:39, Heiner Kallweit wrote:
>     > >>> On 27.03.2020 10:08, Charles Daymand wrote:
>     > >>>> During kernel upgrade testing on our hardware, we found that macvlan
>     > >>>> interface were no longer able to send valid multicast packet.
>     > >>>>
>     > >>>> tcpdump run on our hardware was correctly showing our multicast
>     > >>>> packet but when connecting a laptop to our hardware we didn't see any
>     > >>>> packets.
>     > >>>>
>     > >>>> Bisecting turned up commit 93681cd7d94f
>     > >>>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
>     > >>>> which is responsible for the drop of packet in case of macvlan
>     > >>>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
>     > >>>> case since TSO was keep disabled.
>     > >>>>
>     > >>>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
>     > >>>> issue, but we believe that this hardware issue is important enough to
>     > >>>> keep tx checksum off by default on this revision.
>     > >>>>
>     > >>>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
>     > >>>> specific revision.
>     > >>>>
>     > >>>
>     > >>> The referenced commit may not be the root cause but just reveal another
>     > >>> issue that has been existing before. Root cause may be in the net core
>     > >>> or somewhere else. Did you check with other RTL8168 versions to verify
>     > >>> that it's indeed a HW issue with this specific chip version?
>     > >>>
>     > >>> What you could do: Enable tx checksumming manually (via ethtool) on
>     > >>> older kernel versions and check whether they are fine or not.
>     > >>> If an older version is fine, then you can start a new bisect with tx
>     > >>> checksumming enabled.
>     > >>>
>     > >>> And did you capture and analyze traffic to verify that actually the
>     > >>> checksum is incorrect (and packets discarded therefore on receiving end)?
>     > >>>
>     > >>>
>     > >>>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
>     > >>>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr <mailto:charles.daymand@wifirst.fr>>
>     > >>>> ---
>     > >>>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>     > >>>>  1 file changed, 3 insertions(+)
>     > >>>>
>     > >>>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
>     > >>>> index a9bdafd15a35..3b69135fc500 100644
>     > >>>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
>     > >>>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
>     > >>>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>     > >>>>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>     > >>>>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>     > >>>>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>     > >>>> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
>     > >>>> +                    dev->features &= ~NETIF_F_IP_CSUM;
>     > >>>> +            }
>     > >>>>      }
>     > >>>>
>     > >>>>      dev->hw_features |= NETIF_F_RXALL;
>     > >>>>
>     > >>>
>     > >>
>     > >> After looking a little bit at the macvlen code I think there might be an
>     > >> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
>     > >> seem to have a dedicated maintainer).
>     > >>
>     > >> r8169 implements a ndo_features_check callback that disables tx checksumming
>     > >> for the chip version in question and small packets (due to a HW issue).
>     > >> macvlen uses passthru_features_check() as ndo_features_check callback, this
>     > >> seems to indicate to me that the ndo_features_check callback of lowerdev is
>     > >> ignored. This could explain the issue you see.
>     > >>
>     > >
>     > > macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
>     > > so the second __dev_queue_xmit() should eventually call the real_dev
>     > > ndo_features_check()
>     > >
>     > Thanks, Eric. There's a second path in macvlan_queue_xmit() calling
>     > dev_forward_skb(vlan->lowerdev, skb). Does what you said apply also there?
> 
>     This path wont send packets to the physical NIC, packets are injected
>     back via dev_forward_skb()
> 
>     >
>     > Still I find it strange that a tx hw checksumming issue should affect multicasts
>     > only. Also the chip version in question is quite common and I would expect
>     > others to have hit the same issue.
>     > Maybe best would be to re-test on the affected system w/o involving macvlen.
>     >
>     > >
>     > >
>     > >> Would be interesting to see whether it fixes your issue if you let the
>     > >> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
>     > >>
>     > >> By the way:
>     > >> Also the ndo_fix_features callback of lowerdev seems to be ignored.
>     >
> 
> 
> 
> -- 
> 
> logo wifirst <http://www.wifirst.fr/en> 	
> 
> Charles Daymand
> 
> Développeur infrastructure
> 
> 26 rue de Berri 75008 Paris
> 
> Assistance dédiée responsable de site - 01 70 70 46 70
> Assistance utilisateur - 01 70 70 46 26
> 

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Charles DAYMAND]

Hello,
Sorry for the long delay, I didn't have physical access to the
hardware last week. I did more testing today by connecting directly my
laptop to the hardware and the issue not only affects multicast but
also unicast with macvlan.
Only ping packets are correctly sent.
Using wireshark I can only see malformed ethernet packets (example
with multicast packet below)
Frame 1: 106 bytes on wire (848 bits), 106 bytes captured (848 bits)
IEEE 802.3 Ethernet
    Destination: IPv4mcast_09 (01:00:5e:00:00:09)
    Source: b2:41:6f:04:c1:86 (b2:41:6f:04:c1:86)
    Length: 96
        [Expert Info (Error/Malformed): Length field value goes past
the end of the payload]
Logical-Link Control
    DSAP: Unknown (0x45)
    SSAP: Unknown (0xc0)
    Control field: I, N(R)=46, N(S)=0 (0x5C00)
Data (88 bytes)
    Data: 50320c780111087fac159401e0000009020802080048207a...
    [Length: 88]
Please find the full pcap file at this url with also a tentative to
establish a ssh connection :
https://fourcot.fr/temp/malformed_ethernet2.pcap

Best regards

Charles


Le mar. 31 mars 2020 à 16:07, Heiner Kallweit <hkallweit1@gmail.com> a écrit :
>
> Thanks for further testing! The good news from my perspective is that the issue doesn't occur
> w/o macvlen, therefore it doesn't seem to be a r8169 network driver issue.
>
> W/o knowing tcpdump in detail I think it switches the NIC to promiscuous mode, means
> it should see all packets, incl. the ones with checksum errors.
> Maybe you can mirror the port to which the problematic system is connected and
> analyze the traffic. Or for whatever reason the switch doesn't forward the multicast
> packets to your notebook.
>
> Heiner
>
>
> On 31.03.2020 15:44, Charles DAYMAND wrote:
> > Hello,
> > We tested to enable tx checksumming manually (via ethtool) on a kernel 4.19.0-5-amd64 which is the oldest kernel compatible with our software and we observed exactly the same issue.
> > For information when connecting a laptop directly to the interface we can't see any multicast packet when tx checksumming is enabled on tcpdump.
> > Our network is composed of a cisco switch and we can still see the multicast counters correctly increasing even when we have the issue.
> >
> > I also confirm that when not using macvlan but the real interface there is no issue on the multicast packets, they are correctly sent and received.
> > I have a stupid question, if the IP checksum was bad on the multicast packet, would the receiver NIC drop the packet or would it be seen by tcpdump by the receiver ?
> >
> > Le ven. 27 mars 2020 à 20:43, Eric Dumazet <edumazet@google.com <mailto:edumazet@google.com>> a écrit :
> >
> >     On Fri, Mar 27, 2020 at 12:17 PM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
> >     >
> >     > On 27.03.2020 19:52, Eric Dumazet wrote:
> >     > > On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
> >     > >>
> >     > >> On 27.03.2020 10:39, Heiner Kallweit wrote:
> >     > >>> On 27.03.2020 10:08, Charles Daymand wrote:
> >     > >>>> During kernel upgrade testing on our hardware, we found that macvlan
> >     > >>>> interface were no longer able to send valid multicast packet.
> >     > >>>>
> >     > >>>> tcpdump run on our hardware was correctly showing our multicast
> >     > >>>> packet but when connecting a laptop to our hardware we didn't see any
> >     > >>>> packets.
> >     > >>>>
> >     > >>>> Bisecting turned up commit 93681cd7d94f
> >     > >>>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
> >     > >>>> which is responsible for the drop of packet in case of macvlan
> >     > >>>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
> >     > >>>> case since TSO was keep disabled.
> >     > >>>>
> >     > >>>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
> >     > >>>> issue, but we believe that this hardware issue is important enough to
> >     > >>>> keep tx checksum off by default on this revision.
> >     > >>>>
> >     > >>>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
> >     > >>>> specific revision.
> >     > >>>>
> >     > >>>
> >     > >>> The referenced commit may not be the root cause but just reveal another
> >     > >>> issue that has been existing before. Root cause may be in the net core
> >     > >>> or somewhere else. Did you check with other RTL8168 versions to verify
> >     > >>> that it's indeed a HW issue with this specific chip version?
> >     > >>>
> >     > >>> What you could do: Enable tx checksumming manually (via ethtool) on
> >     > >>> older kernel versions and check whether they are fine or not.
> >     > >>> If an older version is fine, then you can start a new bisect with tx
> >     > >>> checksumming enabled.
> >     > >>>
> >     > >>> And did you capture and analyze traffic to verify that actually the
> >     > >>> checksum is incorrect (and packets discarded therefore on receiving end)?
> >     > >>>
> >     > >>>
> >     > >>>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
> >     > >>>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr <mailto:charles.daymand@wifirst.fr>>
> >     > >>>> ---
> >     > >>>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
> >     > >>>>  1 file changed, 3 insertions(+)
> >     > >>>>
> >     > >>>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
> >     > >>>> index a9bdafd15a35..3b69135fc500 100644
> >     > >>>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
> >     > >>>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
> >     > >>>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> >     > >>>>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >     > >>>>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >     > >>>>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >     > >>>> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
> >     > >>>> +                    dev->features &= ~NETIF_F_IP_CSUM;
> >     > >>>> +            }
> >     > >>>>      }
> >     > >>>>
> >     > >>>>      dev->hw_features |= NETIF_F_RXALL;
> >     > >>>>
> >     > >>>
> >     > >>
> >     > >> After looking a little bit at the macvlen code I think there might be an
> >     > >> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
> >     > >> seem to have a dedicated maintainer).
> >     > >>
> >     > >> r8169 implements a ndo_features_check callback that disables tx checksumming
> >     > >> for the chip version in question and small packets (due to a HW issue).
> >     > >> macvlen uses passthru_features_check() as ndo_features_check callback, this
> >     > >> seems to indicate to me that the ndo_features_check callback of lowerdev is
> >     > >> ignored. This could explain the issue you see.
> >     > >>
> >     > >
> >     > > macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
> >     > > so the second __dev_queue_xmit() should eventually call the real_dev
> >     > > ndo_features_check()
> >     > >
> >     > Thanks, Eric. There's a second path in macvlan_queue_xmit() calling
> >     > dev_forward_skb(vlan->lowerdev, skb). Does what you said apply also there?
> >
> >     This path wont send packets to the physical NIC, packets are injected
> >     back via dev_forward_skb()
> >
> >     >
> >     > Still I find it strange that a tx hw checksumming issue should affect multicasts
> >     > only. Also the chip version in question is quite common and I would expect
> >     > others to have hit the same issue.
> >     > Maybe best would be to re-test on the affected system w/o involving macvlen.
> >     >
> >     > >
> >     > >
> >     > >> Would be interesting to see whether it fixes your issue if you let the
> >     > >> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
> >     > >>
> >     > >> By the way:
> >     > >> Also the ndo_fix_features callback of lowerdev seems to be ignored.
> >     >
> >
> >
> >
> > --
> >
> > logo wifirst <http://www.wifirst.fr/en>
> >
> > Charles Daymand
> >
> > Développeur infrastructure
> >
> > 26 rue de Berri 75008 Paris
> >
> > Assistance dédiée responsable de site - 01 70 70 46 70
> > Assistance utilisateur - 01 70 70 46 26
> >
>


-- 

Charles Daymand

Développeur infrastructure

26 rue de Berri 75008 Paris

Assistance dédiée responsable de site - 01 70 70 46 70
Assistance utilisateur - 01 70 70 46 26

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Heiner Kallweit]

On 06.04.2020 16:12, Charles DAYMAND wrote:
> Hello,
> Sorry for the long delay, I didn't have physical access to the
> hardware last week. I did more testing today by connecting directly my
> laptop to the hardware and the issue not only affects multicast but
> also unicast with macvlan.
> Only ping packets are correctly sent.
> Using wireshark I can only see malformed ethernet packets (example
> with multicast packet below)
> Frame 1: 106 bytes on wire (848 bits), 106 bytes captured (848 bits)
> IEEE 802.3 Ethernet
>     Destination: IPv4mcast_09 (01:00:5e:00:00:09)
>     Source: b2:41:6f:04:c1:86 (b2:41:6f:04:c1:86)
>     Length: 96
>         [Expert Info (Error/Malformed): Length field value goes past
> the end of the payload]
> Logical-Link Control
>     DSAP: Unknown (0x45)
>     SSAP: Unknown (0xc0)
>     Control field: I, N(R)=46, N(S)=0 (0x5C00)
> Data (88 bytes)
>     Data: 50320c780111087fac159401e0000009020802080048207a...
>     [Length: 88]
> Please find the full pcap file at this url with also a tentative to
> establish a ssh connection :
> https://fourcot.fr/temp/malformed_ethernet2.pcap
> 
> Best regards
> 
> Charles
> 
> 
> Le mar. 31 mars 2020 à 16:07, Heiner Kallweit <hkallweit1@gmail.com> a écrit :
>>
>> Thanks for further testing! The good news from my perspective is that the issue doesn't occur
>> w/o macvlen, therefore it doesn't seem to be a r8169 network driver issue.
>>
>> W/o knowing tcpdump in detail I think it switches the NIC to promiscuous mode, means
>> it should see all packets, incl. the ones with checksum errors.
>> Maybe you can mirror the port to which the problematic system is connected and
>> analyze the traffic. Or for whatever reason the switch doesn't forward the multicast
>> packets to your notebook.
>>
>> Heiner
>>
>>
>> On 31.03.2020 15:44, Charles DAYMAND wrote:
>>> Hello,
>>> We tested to enable tx checksumming manually (via ethtool) on a kernel 4.19.0-5-amd64 which is the oldest kernel compatible with our software and we observed exactly the same issue.
>>> For information when connecting a laptop directly to the interface we can't see any multicast packet when tx checksumming is enabled on tcpdump.
>>> Our network is composed of a cisco switch and we can still see the multicast counters correctly increasing even when we have the issue.
>>>
>>> I also confirm that when not using macvlan but the real interface there is no issue on the multicast packets, they are correctly sent and received.
>>> I have a stupid question, if the IP checksum was bad on the multicast packet, would the receiver NIC drop the packet or would it be seen by tcpdump by the receiver ?
>>>
>>> Le ven. 27 mars 2020 à 20:43, Eric Dumazet <edumazet@google.com <mailto:edumazet@google.com>> a écrit :
>>>
>>>     On Fri, Mar 27, 2020 at 12:17 PM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
>>>     >
>>>     > On 27.03.2020 19:52, Eric Dumazet wrote:
>>>     > > On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
>>>     > >>
>>>     > >> On 27.03.2020 10:39, Heiner Kallweit wrote:
>>>     > >>> On 27.03.2020 10:08, Charles Daymand wrote:
>>>     > >>>> During kernel upgrade testing on our hardware, we found that macvlan
>>>     > >>>> interface were no longer able to send valid multicast packet.
>>>     > >>>>
>>>     > >>>> tcpdump run on our hardware was correctly showing our multicast
>>>     > >>>> packet but when connecting a laptop to our hardware we didn't see any
>>>     > >>>> packets.
>>>     > >>>>
>>>     > >>>> Bisecting turned up commit 93681cd7d94f
>>>     > >>>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
>>>     > >>>> which is responsible for the drop of packet in case of macvlan
>>>     > >>>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
>>>     > >>>> case since TSO was keep disabled.
>>>     > >>>>
>>>     > >>>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
>>>     > >>>> issue, but we believe that this hardware issue is important enough to
>>>     > >>>> keep tx checksum off by default on this revision.
>>>     > >>>>
>>>     > >>>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
>>>     > >>>> specific revision.
>>>     > >>>>
>>>     > >>>
>>>     > >>> The referenced commit may not be the root cause but just reveal another
>>>     > >>> issue that has been existing before. Root cause may be in the net core
>>>     > >>> or somewhere else. Did you check with other RTL8168 versions to verify
>>>     > >>> that it's indeed a HW issue with this specific chip version?
>>>     > >>>
>>>     > >>> What you could do: Enable tx checksumming manually (via ethtool) on
>>>     > >>> older kernel versions and check whether they are fine or not.
>>>     > >>> If an older version is fine, then you can start a new bisect with tx
>>>     > >>> checksumming enabled.
>>>     > >>>
>>>     > >>> And did you capture and analyze traffic to verify that actually the
>>>     > >>> checksum is incorrect (and packets discarded therefore on receiving end)?
>>>     > >>>
>>>     > >>>
>>>     > >>>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
>>>     > >>>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr <mailto:charles.daymand@wifirst.fr>>
>>>     > >>>> ---
>>>     > >>>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>>>     > >>>>  1 file changed, 3 insertions(+)
>>>     > >>>>
>>>     > >>>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
>>>     > >>>> index a9bdafd15a35..3b69135fc500 100644
>>>     > >>>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
>>>     > >>>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
>>>     > >>>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>     > >>>>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>     > >>>>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>     > >>>>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>     > >>>> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
>>>     > >>>> +                    dev->features &= ~NETIF_F_IP_CSUM;
>>>     > >>>> +            }
>>>     > >>>>      }
>>>     > >>>>
>>>     > >>>>      dev->hw_features |= NETIF_F_RXALL;
>>>     > >>>>
>>>     > >>>
>>>     > >>
>>>     > >> After looking a little bit at the macvlen code I think there might be an
>>>     > >> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
>>>     > >> seem to have a dedicated maintainer).
>>>     > >>
>>>     > >> r8169 implements a ndo_features_check callback that disables tx checksumming
>>>     > >> for the chip version in question and small packets (due to a HW issue).
>>>     > >> macvlen uses passthru_features_check() as ndo_features_check callback, this
>>>     > >> seems to indicate to me that the ndo_features_check callback of lowerdev is
>>>     > >> ignored. This could explain the issue you see.
>>>     > >>
>>>     > >
>>>     > > macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
>>>     > > so the second __dev_queue_xmit() should eventually call the real_dev
>>>     > > ndo_features_check()
>>>     > >
>>>     > Thanks, Eric. There's a second path in macvlan_queue_xmit() calling
>>>     > dev_forward_skb(vlan->lowerdev, skb). Does what you said apply also there?
>>>
>>>     This path wont send packets to the physical NIC, packets are injected
>>>     back via dev_forward_skb()
>>>
>>>     >
>>>     > Still I find it strange that a tx hw checksumming issue should affect multicasts
>>>     > only. Also the chip version in question is quite common and I would expect
>>>     > others to have hit the same issue.
>>>     > Maybe best would be to re-test on the affected system w/o involving macvlen.
>>>     >
>>>     > >
>>>     > >
>>>     > >> Would be interesting to see whether it fixes your issue if you let the
>>>     > >> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
>>>     > >>
>>>     > >> By the way:
>>>     > >> Also the ndo_fix_features callback of lowerdev seems to be ignored.
>>>     >
>>>
>>>
>>>
>>> --
>>>
>>> logo wifirst <http://www.wifirst.fr/en>
>>>
>>> Charles Daymand
>>>
>>> Développeur infrastructure
>>>
>>> 26 rue de Berri 75008 Paris
>>>
>>> Assistance dédiée responsable de site - 01 70 70 46 70
>>> Assistance utilisateur - 01 70 70 46 26
>>>
>>
> 
> 

In a similar context Realtek made me aware of a hw issue if IP header
has the options field set. You mentioned problems with multicast packets,
and based on the following code the root cause may be related.

br_ip4_multicast_alloc_query()
-> iph->ihl = 6;

I'd appreciate if you could test (with HW tx checksumming enabled)
whether this experimental patch fixes the issue with invalid/lost
multicasts.


diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index e40e8eaeb..dd251ddb8 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4319,6 +4319,10 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb,
 		    rtl_chip_supports_csum_v2(tp))
 			features &= ~NETIF_F_ALL_TSO;
 	} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		if (ip_hdrlen(skb) > sizeof(struct iphdr)) {
+			pr_info("hk: iphdr has options field set\n");
+			features &= ~NETIF_F_CSUM_MASK;
+		}
 		if (skb->len < ETH_ZLEN) {
 			switch (tp->mac_version) {
 			case RTL_GIGA_MAC_VER_11:
-- 
2.26.0

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Eric Dumazet]

On 4/6/20 3:16 PM, Heiner Kallweit wrote:

> 
> In a similar context Realtek made me aware of a hw issue if IP header
> has the options field set. You mentioned problems with multicast packets,
> and based on the following code the root cause may be related.
> 
> br_ip4_multicast_alloc_query()
> -> iph->ihl = 6;
> 
> I'd appreciate if you could test (with HW tx checksumming enabled)
> whether this experimental patch fixes the issue with invalid/lost
> multicasts.
> 
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index e40e8eaeb..dd251ddb8 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -4319,6 +4319,10 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb,
>  		    rtl_chip_supports_csum_v2(tp))
>  			features &= ~NETIF_F_ALL_TSO;
>  	} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +		if (ip_hdrlen(skb) > sizeof(struct iphdr)) {

Packet could be non IPv4 at this point. (IPv6 for instance)

> +			pr_info("hk: iphdr has options field set\n");
> +			features &= ~NETIF_F_CSUM_MASK;
> +		}
>  		if (skb->len < ETH_ZLEN) {
>  			switch (tp->mac_version) {
>  			case RTL_GIGA_MAC_VER_11:
> 

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Heiner Kallweit]

On 07.04.2020 01:20, Eric Dumazet wrote:
> 
> 
> On 4/6/20 3:16 PM, Heiner Kallweit wrote:
> 
>>
>> In a similar context Realtek made me aware of a hw issue if IP header
>> has the options field set. You mentioned problems with multicast packets,
>> and based on the following code the root cause may be related.
>>
>> br_ip4_multicast_alloc_query()
>> -> iph->ihl = 6;
>>
>> I'd appreciate if you could test (with HW tx checksumming enabled)
>> whether this experimental patch fixes the issue with invalid/lost
>> multicasts.
>>
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>> index e40e8eaeb..dd251ddb8 100644
>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>> @@ -4319,6 +4319,10 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb,
>>  		    rtl_chip_supports_csum_v2(tp))
>>  			features &= ~NETIF_F_ALL_TSO;
>>  	} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> +		if (ip_hdrlen(skb) > sizeof(struct iphdr)) {
> 
> Packet could be non IPv4 at this point. (IPv6 for instance)
> 
Right, I should have mentioned it:
This experimental patch is for IPv4 only. In a final version (if it indeed
fixes the issue) I had to extend the condition and check for IPv4.

>> +			pr_info("hk: iphdr has options field set\n");
>> +			features &= ~NETIF_F_CSUM_MASK;
>> +		}
>>  		if (skb->len < ETH_ZLEN) {
>>  			switch (tp->mac_version) {
>>  			case RTL_GIGA_MAC_VER_11:
>>

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Heiner Kallweit]

On 07.04.2020 08:22, Heiner Kallweit wrote:
> On 07.04.2020 01:20, Eric Dumazet wrote:
>>
>>
>> On 4/6/20 3:16 PM, Heiner Kallweit wrote:
>>
>>>
>>> In a similar context Realtek made me aware of a hw issue if IP header
>>> has the options field set. You mentioned problems with multicast packets,
>>> and based on the following code the root cause may be related.
>>>
>>> br_ip4_multicast_alloc_query()
>>> -> iph->ihl = 6;
>>>
>>> I'd appreciate if you could test (with HW tx checksumming enabled)
>>> whether this experimental patch fixes the issue with invalid/lost
>>> multicasts.
>>>
>>>
>>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>>> index e40e8eaeb..dd251ddb8 100644
>>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>>> @@ -4319,6 +4319,10 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb,
>>>  		    rtl_chip_supports_csum_v2(tp))
>>>  			features &= ~NETIF_F_ALL_TSO;
>>>  	} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>>> +		if (ip_hdrlen(skb) > sizeof(struct iphdr)) {
>>
>> Packet could be non IPv4 at this point. (IPv6 for instance)
>>
> Right, I should have mentioned it:
> This experimental patch is for IPv4 only. In a final version (if it indeed
> fixes the issue) I had to extend the condition and check for IPv4.
> 
>>> +			pr_info("hk: iphdr has options field set\n");
>>> +			features &= ~NETIF_F_CSUM_MASK;
>>> +		}
>>>  		if (skb->len < ETH_ZLEN) {
>>>  			switch (tp->mac_version) {
>>>  			case RTL_GIGA_MAC_VER_11:
>>>
> 

Here comes an updated version of the experimental patch that checks for IPv4.
It's part of a bigger experimental patch here, therefore it's not fully
optimized.


diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index e40e8eaeb..69e35da6c 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4307,6 +4307,23 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 	return NETDEV_TX_BUSY;
 }
 
+static netdev_features_t rtl8168evl_features_check(struct sk_buff *skb,
+						   netdev_features_t features)
+{
+	__be16 proto = vlan_get_protocol(skb);
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		if (proto == htons(ETH_P_IP)) {
+			if (ip_hdrlen(skb) > sizeof(struct iphdr)) {
+				pr_info("hk: iphdr has options field set\n");
+				features &= ~NETIF_F_CSUM_MASK;
+			}
+		}
+	}
+
+	return features;
+}
+
 static netdev_features_t rtl8169_features_check(struct sk_buff *skb,
 						struct net_device *dev,
 						netdev_features_t features)
@@ -4314,6 +4331,9 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb,
 	int transport_offset = skb_transport_offset(skb);
 	struct rtl8169_private *tp = netdev_priv(dev);
 
+	if (tp->mac_version == RTL_GIGA_MAC_VER_34)
+		features = rtl8168evl_features_check(skb, features);
+
 	if (skb_is_gso(skb)) {
 		if (transport_offset > GTTCPHO_MAX &&
 		    rtl_chip_supports_csum_v2(tp))
-- 
2.26.0

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Charles DAYMAND]

Hello,

I just tested your patch, there is no improvement on the issue.
I still have layer2 malformed packets.
I also created a TCP server on my laptop with the hardware
periodically sending udp and tcp packets and these packets are also
layer2 malformed.
Please find below an example of the malformed packet :
Frame 3533: 110 bytes on wire (880 bits), 110 bytes captured (880
bits) on interface 0
Ethernet II, Src: b2:41:6f:04:c1:86 (b2:41:6f:04:c1:86), Dst:
IPv4mcast_09 (01:00:5e:00:00:09)
802.1Q Virtual LAN, PRI: 0, DEI: 0, ID: 2833
    000. .... .... .... = Priority: Best Effort (default) (0)
    ...0 .... .... .... = DEI: Ineligible
    .... 1011 0001 0001 = ID: 2833
    Length: 96
        [Expert Info (Error/Malformed): Length field value goes past
the end of the payload]
            [Length field value goes past the end of the payload]
            [Severity level: Error]
            [Group: Malformed]
Logical-Link Control
Data (88 bytes)


Le mer. 8 avr. 2020 à 00:40, Heiner Kallweit <hkallweit1@gmail.com> a écrit :
>
> On 07.04.2020 08:22, Heiner Kallweit wrote:
> > On 07.04.2020 01:20, Eric Dumazet wrote:
> >>
> >>
> >> On 4/6/20 3:16 PM, Heiner Kallweit wrote:
> >>
> >>>
> >>> In a similar context Realtek made me aware of a hw issue if IP header
> >>> has the options field set. You mentioned problems with multicast packets,
> >>> and based on the following code the root cause may be related.
> >>>
> >>> br_ip4_multicast_alloc_query()
> >>> -> iph->ihl = 6;
> >>>
> >>> I'd appreciate if you could test (with HW tx checksumming enabled)
> >>> whether this experimental patch fixes the issue with invalid/lost
> >>> multicasts.
> >>>
> >>>
> >>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> >>> index e40e8eaeb..dd251ddb8 100644
> >>> --- a/drivers/net/ethernet/realtek/r8169_main.c
> >>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> >>> @@ -4319,6 +4319,10 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb,
> >>>                 rtl_chip_supports_csum_v2(tp))
> >>>                     features &= ~NETIF_F_ALL_TSO;
> >>>     } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> >>> +           if (ip_hdrlen(skb) > sizeof(struct iphdr)) {
> >>
> >> Packet could be non IPv4 at this point. (IPv6 for instance)
> >>
> > Right, I should have mentioned it:
> > This experimental patch is for IPv4 only. In a final version (if it indeed
> > fixes the issue) I had to extend the condition and check for IPv4.
> >
> >>> +                   pr_info("hk: iphdr has options field set\n");
> >>> +                   features &= ~NETIF_F_CSUM_MASK;
> >>> +           }
> >>>             if (skb->len < ETH_ZLEN) {
> >>>                     switch (tp->mac_version) {
> >>>                     case RTL_GIGA_MAC_VER_11:
> >>>
> >
>
> Here comes an updated version of the experimental patch that checks for IPv4.
> It's part of a bigger experimental patch here, therefore it's not fully
> optimized.
>
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index e40e8eaeb..69e35da6c 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -4307,6 +4307,23 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
>         return NETDEV_TX_BUSY;
>  }
>
> +static netdev_features_t rtl8168evl_features_check(struct sk_buff *skb,
> +                                                  netdev_features_t features)
> +{
> +       __be16 proto = vlan_get_protocol(skb);
> +
> +       if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +               if (proto == htons(ETH_P_IP)) {
> +                       if (ip_hdrlen(skb) > sizeof(struct iphdr)) {
> +                               pr_info("hk: iphdr has options field set\n");
> +                               features &= ~NETIF_F_CSUM_MASK;
> +                       }
> +               }
> +       }
> +
> +       return features;
> +}
> +
>  static netdev_features_t rtl8169_features_check(struct sk_buff *skb,
>                                                 struct net_device *dev,
>                                                 netdev_features_t features)
> @@ -4314,6 +4331,9 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb,
>         int transport_offset = skb_transport_offset(skb);
>         struct rtl8169_private *tp = netdev_priv(dev);
>
> +       if (tp->mac_version == RTL_GIGA_MAC_VER_34)
> +               features = rtl8168evl_features_check(skb, features);
> +
>         if (skb_is_gso(skb)) {
>                 if (transport_offset > GTTCPHO_MAX &&
>                     rtl_chip_supports_csum_v2(tp))
> --
> 2.26.0
>
>


-- 

Charles Daymand

Développeur infrastructure

26 rue de Berri 75008 Paris

Assistance dédiée responsable de site - 01 70 70 46 70
Assistance utilisateur - 01 70 70 46 26

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Heiner Kallweit]

On 06.04.2020 16:12, Charles DAYMAND wrote:
> Hello,
> Sorry for the long delay, I didn't have physical access to the
> hardware last week. I did more testing today by connecting directly my
> laptop to the hardware and the issue not only affects multicast but
> also unicast with macvlan.
> Only ping packets are correctly sent.
> Using wireshark I can only see malformed ethernet packets (example
> with multicast packet below)
> Frame 1: 106 bytes on wire (848 bits), 106 bytes captured (848 bits)
> IEEE 802.3 Ethernet
>     Destination: IPv4mcast_09 (01:00:5e:00:00:09)
>     Source: b2:41:6f:04:c1:86 (b2:41:6f:04:c1:86)
>     Length: 96
>         [Expert Info (Error/Malformed): Length field value goes past
> the end of the payload]
> Logical-Link Control
>     DSAP: Unknown (0x45)
>     SSAP: Unknown (0xc0)
>     Control field: I, N(R)=46, N(S)=0 (0x5C00)
> Data (88 bytes)
>     Data: 50320c780111087fac159401e0000009020802080048207a...
>     [Length: 88]
> Please find the full pcap file at this url with also a tentative to
> establish a ssh connection :
> https://fourcot.fr/temp/malformed_ethernet2.pcap
> 

A problem with HW checksumming should manifest as checksum failure,
I don't think HW checksumming touches length information in headers.
You could enable tracing network events to see whether packets look
suspicious before reaching the network driver.
(or do some printf debugging for the affected multicast packets)

Because you initially suspected a problem with HW checksumming
in a specific RTL8168 chip variant:
- Check macvlan with another network driver supporting HW checksumming
- Check macvlan with r8169 and another RTL8168 chip variant

The fact that the issue doesn't exist w/o macvlan seems to indicate
that the network driver can't be blamed here.

> Best regards
> 
> Charles
> 
> 
> Le mar. 31 mars 2020 à 16:07, Heiner Kallweit <hkallweit1@gmail.com> a écrit :
>>
>> Thanks for further testing! The good news from my perspective is that the issue doesn't occur
>> w/o macvlen, therefore it doesn't seem to be a r8169 network driver issue.
>>
>> W/o knowing tcpdump in detail I think it switches the NIC to promiscuous mode, means
>> it should see all packets, incl. the ones with checksum errors.
>> Maybe you can mirror the port to which the problematic system is connected and
>> analyze the traffic. Or for whatever reason the switch doesn't forward the multicast
>> packets to your notebook.
>>
>> Heiner
>>
>>
>> On 31.03.2020 15:44, Charles DAYMAND wrote:
>>> Hello,
>>> We tested to enable tx checksumming manually (via ethtool) on a kernel 4.19.0-5-amd64 which is the oldest kernel compatible with our software and we observed exactly the same issue.
>>> For information when connecting a laptop directly to the interface we can't see any multicast packet when tx checksumming is enabled on tcpdump.
>>> Our network is composed of a cisco switch and we can still see the multicast counters correctly increasing even when we have the issue.
>>>
>>> I also confirm that when not using macvlan but the real interface there is no issue on the multicast packets, they are correctly sent and received.
>>> I have a stupid question, if the IP checksum was bad on the multicast packet, would the receiver NIC drop the packet or would it be seen by tcpdump by the receiver ?
>>>
>>> Le ven. 27 mars 2020 à 20:43, Eric Dumazet <edumazet@google.com <mailto:edumazet@google.com>> a écrit :
>>>
>>>     On Fri, Mar 27, 2020 at 12:17 PM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
>>>     >
>>>     > On 27.03.2020 19:52, Eric Dumazet wrote:
>>>     > > On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
>>>     > >>
>>>     > >> On 27.03.2020 10:39, Heiner Kallweit wrote:
>>>     > >>> On 27.03.2020 10:08, Charles Daymand wrote:
>>>     > >>>> During kernel upgrade testing on our hardware, we found that macvlan
>>>     > >>>> interface were no longer able to send valid multicast packet.
>>>     > >>>>
>>>     > >>>> tcpdump run on our hardware was correctly showing our multicast
>>>     > >>>> packet but when connecting a laptop to our hardware we didn't see any
>>>     > >>>> packets.
>>>     > >>>>
>>>     > >>>> Bisecting turned up commit 93681cd7d94f
>>>     > >>>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
>>>     > >>>> which is responsible for the drop of packet in case of macvlan
>>>     > >>>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
>>>     > >>>> case since TSO was keep disabled.
>>>     > >>>>
>>>     > >>>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
>>>     > >>>> issue, but we believe that this hardware issue is important enough to
>>>     > >>>> keep tx checksum off by default on this revision.
>>>     > >>>>
>>>     > >>>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
>>>     > >>>> specific revision.
>>>     > >>>>
>>>     > >>>
>>>     > >>> The referenced commit may not be the root cause but just reveal another
>>>     > >>> issue that has been existing before. Root cause may be in the net core
>>>     > >>> or somewhere else. Did you check with other RTL8168 versions to verify
>>>     > >>> that it's indeed a HW issue with this specific chip version?
>>>     > >>>
>>>     > >>> What you could do: Enable tx checksumming manually (via ethtool) on
>>>     > >>> older kernel versions and check whether they are fine or not.
>>>     > >>> If an older version is fine, then you can start a new bisect with tx
>>>     > >>> checksumming enabled.
>>>     > >>>
>>>     > >>> And did you capture and analyze traffic to verify that actually the
>>>     > >>> checksum is incorrect (and packets discarded therefore on receiving end)?
>>>     > >>>
>>>     > >>>
>>>     > >>>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
>>>     > >>>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr <mailto:charles.daymand@wifirst.fr>>
>>>     > >>>> ---
>>>     > >>>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>>>     > >>>>  1 file changed, 3 insertions(+)
>>>     > >>>>
>>>     > >>>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
>>>     > >>>> index a9bdafd15a35..3b69135fc500 100644
>>>     > >>>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
>>>     > >>>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
>>>     > >>>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>     > >>>>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>     > >>>>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>     > >>>>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>     > >>>> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
>>>     > >>>> +                    dev->features &= ~NETIF_F_IP_CSUM;
>>>     > >>>> +            }
>>>     > >>>>      }
>>>     > >>>>
>>>     > >>>>      dev->hw_features |= NETIF_F_RXALL;
>>>     > >>>>
>>>     > >>>
>>>     > >>
>>>     > >> After looking a little bit at the macvlen code I think there might be an
>>>     > >> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
>>>     > >> seem to have a dedicated maintainer).
>>>     > >>
>>>     > >> r8169 implements a ndo_features_check callback that disables tx checksumming
>>>     > >> for the chip version in question and small packets (due to a HW issue).
>>>     > >> macvlen uses passthru_features_check() as ndo_features_check callback, this
>>>     > >> seems to indicate to me that the ndo_features_check callback of lowerdev is
>>>     > >> ignored. This could explain the issue you see.
>>>     > >>
>>>     > >
>>>     > > macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
>>>     > > so the second __dev_queue_xmit() should eventually call the real_dev
>>>     > > ndo_features_check()
>>>     > >
>>>     > Thanks, Eric. There's a second path in macvlan_queue_xmit() calling
>>>     > dev_forward_skb(vlan->lowerdev, skb). Does what you said apply also there?
>>>
>>>     This path wont send packets to the physical NIC, packets are injected
>>>     back via dev_forward_skb()
>>>
>>>     >
>>>     > Still I find it strange that a tx hw checksumming issue should affect multicasts
>>>     > only. Also the chip version in question is quite common and I would expect
>>>     > others to have hit the same issue.
>>>     > Maybe best would be to re-test on the affected system w/o involving macvlen.
>>>     >
>>>     > >
>>>     > >
>>>     > >> Would be interesting to see whether it fixes your issue if you let the
>>>     > >> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
>>>     > >>
>>>     > >> By the way:
>>>     > >> Also the ndo_fix_features callback of lowerdev seems to be ignored.
>>>     >
>>>
>>>
>>>
>>> --
>>>
>>> logo wifirst <http://www.wifirst.fr/en>
>>>
>>> Charles Daymand
>>>
>>> Développeur infrastructure
>>>
>>> 26 rue de Berri 75008 Paris
>>>
>>> Assistance dédiée responsable de site - 01 70 70 46 70
>>> Assistance utilisateur - 01 70 70 46 26
>>>
>>
> 
> 

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Eric Dumazet]

On Mon, Apr 13, 2020 at 11:06 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 06.04.2020 16:12, Charles DAYMAND wrote:
> > Hello,
> > Sorry for the long delay, I didn't have physical access to the
> > hardware last week. I did more testing today by connecting directly my
> > laptop to the hardware and the issue not only affects multicast but
> > also unicast with macvlan.
> > Only ping packets are correctly sent.
> > Using wireshark I can only see malformed ethernet packets (example
> > with multicast packet below)
> > Frame 1: 106 bytes on wire (848 bits), 106 bytes captured (848 bits)
> > IEEE 802.3 Ethernet
> >     Destination: IPv4mcast_09 (01:00:5e:00:00:09)
> >     Source: b2:41:6f:04:c1:86 (b2:41:6f:04:c1:86)
> >     Length: 96
> >         [Expert Info (Error/Malformed): Length field value goes past
> > the end of the payload]
> > Logical-Link Control
> >     DSAP: Unknown (0x45)
> >     SSAP: Unknown (0xc0)
> >     Control field: I, N(R)=46, N(S)=0 (0x5C00)
> > Data (88 bytes)
> >     Data: 50320c780111087fac159401e0000009020802080048207a...
> >     [Length: 88]
> > Please find the full pcap file at this url with also a tentative to
> > establish a ssh connection :
> > https://fourcot.fr/temp/malformed_ethernet2.pcap
> >
>
> A problem with HW checksumming should manifest as checksum failure,
> I don't think HW checksumming touches length information in headers.
> You could enable tracing network events to see whether packets look
> suspicious before reaching the network driver.
> (or do some printf debugging for the affected multicast packets)
>
> Because you initially suspected a problem with HW checksumming
> in a specific RTL8168 chip variant:
> - Check macvlan with another network driver supporting HW checksumming
> - Check macvlan with r8169 and another RTL8168 chip variant
>
> The fact that the issue doesn't exist w/o macvlan seems to indicate
> that the network driver can't be blamed here.

trace includes evidence of 802.1Q traffic though, maybe "macvlan" is a
red herring,
some confusion on wording maybe...

802.1Q Virtual LAN, PRI: 0, DEI: 0, ID: 2833
    000. .... .... .... = Priority: Best Effort (default) (0)
    ...0 .... .... .... = DEI: Ineligible
    .... 1011 0001 0001 = ID: 2833


>
> > Best regards
> >
> > Charles
> >
> >
> > Le mar. 31 mars 2020 à 16:07, Heiner Kallweit <hkallweit1@gmail.com> a écrit :
> >>
> >> Thanks for further testing! The good news from my perspective is that the issue doesn't occur
> >> w/o macvlen, therefore it doesn't seem to be a r8169 network driver issue.
> >>
> >> W/o knowing tcpdump in detail I think it switches the NIC to promiscuous mode, means
> >> it should see all packets, incl. the ones with checksum errors.
> >> Maybe you can mirror the port to which the problematic system is connected and
> >> analyze the traffic. Or for whatever reason the switch doesn't forward the multicast
> >> packets to your notebook.
> >>
> >> Heiner
> >>
> >>
> >> On 31.03.2020 15:44, Charles DAYMAND wrote:
> >>> Hello,
> >>> We tested to enable tx checksumming manually (via ethtool) on a kernel 4.19.0-5-amd64 which is the oldest kernel compatible with our software and we observed exactly the same issue.
> >>> For information when connecting a laptop directly to the interface we can't see any multicast packet when tx checksumming is enabled on tcpdump.
> >>> Our network is composed of a cisco switch and we can still see the multicast counters correctly increasing even when we have the issue.
> >>>
> >>> I also confirm that when not using macvlan but the real interface there is no issue on the multicast packets, they are correctly sent and received.
> >>> I have a stupid question, if the IP checksum was bad on the multicast packet, would the receiver NIC drop the packet or would it be seen by tcpdump by the receiver ?
> >>>
> >>> Le ven. 27 mars 2020 à 20:43, Eric Dumazet <edumazet@google.com <mailto:edumazet@google.com>> a écrit :
> >>>
> >>>     On Fri, Mar 27, 2020 at 12:17 PM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
> >>>     >
> >>>     > On 27.03.2020 19:52, Eric Dumazet wrote:
> >>>     > > On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
> >>>     > >>
> >>>     > >> On 27.03.2020 10:39, Heiner Kallweit wrote:
> >>>     > >>> On 27.03.2020 10:08, Charles Daymand wrote:
> >>>     > >>>> During kernel upgrade testing on our hardware, we found that macvlan
> >>>     > >>>> interface were no longer able to send valid multicast packet.
> >>>     > >>>>
> >>>     > >>>> tcpdump run on our hardware was correctly showing our multicast
> >>>     > >>>> packet but when connecting a laptop to our hardware we didn't see any
> >>>     > >>>> packets.
> >>>     > >>>>
> >>>     > >>>> Bisecting turned up commit 93681cd7d94f
> >>>     > >>>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
> >>>     > >>>> which is responsible for the drop of packet in case of macvlan
> >>>     > >>>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
> >>>     > >>>> case since TSO was keep disabled.
> >>>     > >>>>
> >>>     > >>>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
> >>>     > >>>> issue, but we believe that this hardware issue is important enough to
> >>>     > >>>> keep tx checksum off by default on this revision.
> >>>     > >>>>
> >>>     > >>>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
> >>>     > >>>> specific revision.
> >>>     > >>>>
> >>>     > >>>
> >>>     > >>> The referenced commit may not be the root cause but just reveal another
> >>>     > >>> issue that has been existing before. Root cause may be in the net core
> >>>     > >>> or somewhere else. Did you check with other RTL8168 versions to verify
> >>>     > >>> that it's indeed a HW issue with this specific chip version?
> >>>     > >>>
> >>>     > >>> What you could do: Enable tx checksumming manually (via ethtool) on
> >>>     > >>> older kernel versions and check whether they are fine or not.
> >>>     > >>> If an older version is fine, then you can start a new bisect with tx
> >>>     > >>> checksumming enabled.
> >>>     > >>>
> >>>     > >>> And did you capture and analyze traffic to verify that actually the
> >>>     > >>> checksum is incorrect (and packets discarded therefore on receiving end)?
> >>>     > >>>
> >>>     > >>>
> >>>     > >>>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
> >>>     > >>>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr <mailto:charles.daymand@wifirst.fr>>
> >>>     > >>>> ---
> >>>     > >>>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
> >>>     > >>>>  1 file changed, 3 insertions(+)
> >>>     > >>>>
> >>>     > >>>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
> >>>     > >>>> index a9bdafd15a35..3b69135fc500 100644
> >>>     > >>>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
> >>>     > >>>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
> >>>     > >>>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> >>>     > >>>>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >>>     > >>>>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >>>     > >>>>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
> >>>     > >>>> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
> >>>     > >>>> +                    dev->features &= ~NETIF_F_IP_CSUM;
> >>>     > >>>> +            }
> >>>     > >>>>      }
> >>>     > >>>>
> >>>     > >>>>      dev->hw_features |= NETIF_F_RXALL;
> >>>     > >>>>
> >>>     > >>>
> >>>     > >>
> >>>     > >> After looking a little bit at the macvlen code I think there might be an
> >>>     > >> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
> >>>     > >> seem to have a dedicated maintainer).
> >>>     > >>
> >>>     > >> r8169 implements a ndo_features_check callback that disables tx checksumming
> >>>     > >> for the chip version in question and small packets (due to a HW issue).
> >>>     > >> macvlen uses passthru_features_check() as ndo_features_check callback, this
> >>>     > >> seems to indicate to me that the ndo_features_check callback of lowerdev is
> >>>     > >> ignored. This could explain the issue you see.
> >>>     > >>
> >>>     > >
> >>>     > > macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
> >>>     > > so the second __dev_queue_xmit() should eventually call the real_dev
> >>>     > > ndo_features_check()
> >>>     > >
> >>>     > Thanks, Eric. There's a second path in macvlan_queue_xmit() calling
> >>>     > dev_forward_skb(vlan->lowerdev, skb). Does what you said apply also there?
> >>>
> >>>     This path wont send packets to the physical NIC, packets are injected
> >>>     back via dev_forward_skb()
> >>>
> >>>     >
> >>>     > Still I find it strange that a tx hw checksumming issue should affect multicasts
> >>>     > only. Also the chip version in question is quite common and I would expect
> >>>     > others to have hit the same issue.
> >>>     > Maybe best would be to re-test on the affected system w/o involving macvlen.
> >>>     >
> >>>     > >
> >>>     > >
> >>>     > >> Would be interesting to see whether it fixes your issue if you let the
> >>>     > >> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
> >>>     > >>
> >>>     > >> By the way:
> >>>     > >> Also the ndo_fix_features callback of lowerdev seems to be ignored.
> >>>     >
> >>>
> >>>
> >>>
> >>> --
> >>>
> >>> logo wifirst <http://www.wifirst.fr/en>
> >>>
> >>> Charles Daymand
> >>>
> >>> Développeur infrastructure
> >>>
> >>> 26 rue de Berri 75008 Paris
> >>>
> >>> Assistance dédiée responsable de site - 01 70 70 46 70
> >>> Assistance utilisateur - 01 70 70 46 26
> >>>
> >>
> >
> >
>

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Charles Daymand]

Sorry apparently I had an issue when sending the mail to the netdev mailing list yesterday.
Actually in our architecture we are creating vlan interface from a
macvlan interface (that explains the 802.1Q traffic)
I did more testing on the issue today and I observed that the macvlan
interface itself is not affected by the issue, it's only the vlan
interfaces created from the macvlan interface which is affected by the
issue.

I did the following network configuration on the hardware to simplify
the testing :

    +----------------------------------------+                 +--------------------------+
    | Hardware                               |                 | Laptop                   |
    | +---------+                            |                 |            +---------+   |
    | | vlan2833+-+                          |                 |      +-----+ vlan2833|   |
    | +---------+ |                          |                 |      |     +---------+   |
192.168.200.1/24 |  +---------+          +--+-+             +-+--+   |  192.168.200.2/24 |
    |             +--+mymacvlan+----------+eth0+-------------+eth0+---+--+                |
    |             |  +---------+          +----+             +----+      |                |
    | +---------+ | 192.168.120.1/24 192.168.100.1/24   192.168.100.2/24 |  +---------+   |
    | | vlan1000+-+                          |          192.168.120.2/24 +--+ vlan1000|   |
    | +---------+                            |                 |            +---------+   |
192.168.150.1/24                            |                 |         192.168.150.2/24 |
    +----------------------------------------+                 +--------------------------+

For testing purpose I connected by ssh to 192.168.100.1 and sent udp
packets to the laptop IPs with the following result:
* When sending an UDP packet to 192.168.120.2 ==> No issue
* When sending an UDP packet to 192.168.200.2 (vlan 2833) ==> I have the malformed packet issue
* When sending an UDP packet to 192.168.150.2 (vlan 1000) ==> I receive no network traffic
and I lose the ssh connection during 2 minutes with nothing in dmesg during this period.

Le 13/04/2020 à 20:20, Eric Dumazet a écrit :
> On Mon, Apr 13, 2020 at 11:06 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>> On 06.04.2020 16:12, Charles DAYMAND wrote:
>>> Hello,
>>> Sorry for the long delay, I didn't have physical access to the
>>> hardware last week. I did more testing today by connecting directly my
>>> laptop to the hardware and the issue not only affects multicast but
>>> also unicast with macvlan.
>>> Only ping packets are correctly sent.
>>> Using wireshark I can only see malformed ethernet packets (example
>>> with multicast packet below)
>>> Frame 1: 106 bytes on wire (848 bits), 106 bytes captured (848 bits)
>>> IEEE 802.3 Ethernet
>>>      Destination: IPv4mcast_09 (01:00:5e:00:00:09)
>>>      Source: b2:41:6f:04:c1:86 (b2:41:6f:04:c1:86)
>>>      Length: 96
>>>          [Expert Info (Error/Malformed): Length field value goes past
>>> the end of the payload]
>>> Logical-Link Control
>>>      DSAP: Unknown (0x45)
>>>      SSAP: Unknown (0xc0)
>>>      Control field: I, N(R)=46, N(S)=0 (0x5C00)
>>> Data (88 bytes)
>>>      Data: 50320c780111087fac159401e0000009020802080048207a...
>>>      [Length: 88]
>>> Please find the full pcap file at this url with also a tentative to
>>> establish a ssh connection :
>>> https://fourcot.fr/temp/malformed_ethernet2.pcap
>>>
>> A problem with HW checksumming should manifest as checksum failure,
>> I don't think HW checksumming touches length information in headers.
>> You could enable tracing network events to see whether packets look
>> suspicious before reaching the network driver.
>> (or do some printf debugging for the affected multicast packets)
>>
>> Because you initially suspected a problem with HW checksumming
>> in a specific RTL8168 chip variant:
>> - Check macvlan with another network driver supporting HW checksumming
>> - Check macvlan with r8169 and another RTL8168 chip variant
>>
>> The fact that the issue doesn't exist w/o macvlan seems to indicate
>> that the network driver can't be blamed here.
> trace includes evidence of 802.1Q traffic though, maybe "macvlan" is a
> red herring,
> some confusion on wording maybe...
>
> 802.1Q Virtual LAN, PRI: 0, DEI: 0, ID: 2833
>      000. .... .... .... = Priority: Best Effort (default) (0)
>      ...0 .... .... .... = DEI: Ineligible
>      .... 1011 0001 0001 = ID: 2833
>
>
>>> Best regards
>>>
>>> Charles
>>>
>>>
>>> Le mar. 31 mars 2020 à 16:07, Heiner Kallweit <hkallweit1@gmail.com> a écrit :
>>>> Thanks for further testing! The good news from my perspective is that the issue doesn't occur
>>>> w/o macvlen, therefore it doesn't seem to be a r8169 network driver issue.
>>>>
>>>> W/o knowing tcpdump in detail I think it switches the NIC to promiscuous mode, means
>>>> it should see all packets, incl. the ones with checksum errors.
>>>> Maybe you can mirror the port to which the problematic system is connected and
>>>> analyze the traffic. Or for whatever reason the switch doesn't forward the multicast
>>>> packets to your notebook.
>>>>
>>>> Heiner
>>>>
>>>>
>>>> On 31.03.2020 15:44, Charles DAYMAND wrote:
>>>>> Hello,
>>>>> We tested to enable tx checksumming manually (via ethtool) on a kernel 4.19.0-5-amd64 which is the oldest kernel compatible with our software and we observed exactly the same issue.
>>>>> For information when connecting a laptop directly to the interface we can't see any multicast packet when tx checksumming is enabled on tcpdump.
>>>>> Our network is composed of a cisco switch and we can still see the multicast counters correctly increasing even when we have the issue.
>>>>>
>>>>> I also confirm that when not using macvlan but the real interface there is no issue on the multicast packets, they are correctly sent and received.
>>>>> I have a stupid question, if the IP checksum was bad on the multicast packet, would the receiver NIC drop the packet or would it be seen by tcpdump by the receiver ?
>>>>>
>>>>> Le ven. 27 mars 2020 à 20:43, Eric Dumazet <edumazet@google.com <mailto:edumazet@google.com>> a écrit :
>>>>>
>>>>>      On Fri, Mar 27, 2020 at 12:17 PM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
>>>>>      >
>>>>>      > On 27.03.2020 19:52, Eric Dumazet wrote:
>>>>>      > > On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
>>>>>      > >>
>>>>>      > >> On 27.03.2020 10:39, Heiner Kallweit wrote:
>>>>>      > >>> On 27.03.2020 10:08, Charles Daymand wrote:
>>>>>      > >>>> During kernel upgrade testing on our hardware, we found that macvlan
>>>>>      > >>>> interface were no longer able to send valid multicast packet.
>>>>>      > >>>>
>>>>>      > >>>> tcpdump run on our hardware was correctly showing our multicast
>>>>>      > >>>> packet but when connecting a laptop to our hardware we didn't see any
>>>>>      > >>>> packets.
>>>>>      > >>>>
>>>>>      > >>>> Bisecting turned up commit 93681cd7d94f
>>>>>      > >>>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
>>>>>      > >>>> which is responsible for the drop of packet in case of macvlan
>>>>>      > >>>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
>>>>>      > >>>> case since TSO was keep disabled.
>>>>>      > >>>>
>>>>>      > >>>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
>>>>>      > >>>> issue, but we believe that this hardware issue is important enough to
>>>>>      > >>>> keep tx checksum off by default on this revision.
>>>>>      > >>>>
>>>>>      > >>>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
>>>>>      > >>>> specific revision.
>>>>>      > >>>>
>>>>>      > >>>
>>>>>      > >>> The referenced commit may not be the root cause but just reveal another
>>>>>      > >>> issue that has been existing before. Root cause may be in the net core
>>>>>      > >>> or somewhere else. Did you check with other RTL8168 versions to verify
>>>>>      > >>> that it's indeed a HW issue with this specific chip version?
>>>>>      > >>>
>>>>>      > >>> What you could do: Enable tx checksumming manually (via ethtool) on
>>>>>      > >>> older kernel versions and check whether they are fine or not.
>>>>>      > >>> If an older version is fine, then you can start a new bisect with tx
>>>>>      > >>> checksumming enabled.
>>>>>      > >>>
>>>>>      > >>> And did you capture and analyze traffic to verify that actually the
>>>>>      > >>> checksum is incorrect (and packets discarded therefore on receiving end)?
>>>>>      > >>>
>>>>>      > >>>
>>>>>      > >>>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
>>>>>      > >>>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr <mailto:charles.daymand@wifirst.fr>>
>>>>>      > >>>> ---
>>>>>      > >>>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>>>>>      > >>>>  1 file changed, 3 insertions(+)
>>>>>      > >>>>
>>>>>      > >>>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
>>>>>      > >>>> index a9bdafd15a35..3b69135fc500 100644
>>>>>      > >>>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
>>>>>      > >>>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
>>>>>      > >>>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>>>      > >>>>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>>>      > >>>>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>>>      > >>>>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>>>      > >>>> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
>>>>>      > >>>> +                    dev->features &= ~NETIF_F_IP_CSUM;
>>>>>      > >>>> +            }
>>>>>      > >>>>      }
>>>>>      > >>>>
>>>>>      > >>>>      dev->hw_features |= NETIF_F_RXALL;
>>>>>      > >>>>
>>>>>      > >>>
>>>>>      > >>
>>>>>      > >> After looking a little bit at the macvlen code I think there might be an
>>>>>      > >> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
>>>>>      > >> seem to have a dedicated maintainer).
>>>>>      > >>
>>>>>      > >> r8169 implements a ndo_features_check callback that disables tx checksumming
>>>>>      > >> for the chip version in question and small packets (due to a HW issue).
>>>>>      > >> macvlen uses passthru_features_check() as ndo_features_check callback, this
>>>>>      > >> seems to indicate to me that the ndo_features_check callback of lowerdev is
>>>>>      > >> ignored. This could explain the issue you see.
>>>>>      > >>
>>>>>      > >
>>>>>      > > macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
>>>>>      > > so the second __dev_queue_xmit() should eventually call the real_dev
>>>>>      > > ndo_features_check()
>>>>>      > >
>>>>>      > Thanks, Eric. There's a second path in macvlan_queue_xmit() calling
>>>>>      > dev_forward_skb(vlan->lowerdev, skb). Does what you said apply also there?
>>>>>
>>>>>      This path wont send packets to the physical NIC, packets are injected
>>>>>      back via dev_forward_skb()
>>>>>
>>>>>      >
>>>>>      > Still I find it strange that a tx hw checksumming issue should affect multicasts
>>>>>      > only. Also the chip version in question is quite common and I would expect
>>>>>      > others to have hit the same issue.
>>>>>      > Maybe best would be to re-test on the affected system w/o involving macvlen.
>>>>>      >
>>>>>      > >
>>>>>      > >
>>>>>      > >> Would be interesting to see whether it fixes your issue if you let the
>>>>>      > >> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
>>>>>      > >>
>>>>>      > >> By the way:
>>>>>      > >> Also the ndo_fix_features callback of lowerdev seems to be ignored.
>>>>>      >
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>>
>>>>> logo wifirst <http://www.wifirst.fr/en>
>>>>>
>>>>> Charles Daymand
>>>>>
>>>>> Développeur infrastructure
>>>>>
>>>>> 26 rue de Berri 75008 Paris
>>>>>
>>>>> Assistance dédiée responsable de site - 01 70 70 46 70
>>>>> Assistance utilisateur - 01 70 70 46 26
>>>>>
>>>

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Heiner Kallweit]

On 15.04.2020 13:55, Charles Daymand wrote:
> Sorry apparently I had an issue when sending the mail to the netdev mailing list yesterday.
> Actually in our architecture we are creating vlan interface from a
> macvlan interface (that explains the 802.1Q traffic)
> I did more testing on the issue today and I observed that the macvlan
> interface itself is not affected by the issue, it's only the vlan
> interfaces created from the macvlan interface which is affected by the
> issue.
> 
> I did the following network configuration on the hardware to simplify
> the testing :
> 
>    +----------------------------------------+                 +--------------------------+
>    | Hardware                               |                 | Laptop                   |
>    | +---------+                            |                 |            +---------+   |
>    | | vlan2833+-+                          |                 |      +-----+ vlan2833|   |
>    | +---------+ |                          |                 |      |     +---------+   |
> 192.168.200.1/24 |  +---------+          +--+-+             +-+--+   |  192.168.200.2/24 |
>    |             +--+mymacvlan+----------+eth0+-------------+eth0+---+--+                |
>    |             |  +---------+          +----+             +----+      |                |
>    | +---------+ | 192.168.120.1/24 192.168.100.1/24   192.168.100.2/24 |  +---------+   |
>    | | vlan1000+-+                          |          192.168.120.2/24 +--+ vlan1000|   |
>    | +---------+                            |                 |            +---------+   |
> 192.168.150.1/24                            |                 |         192.168.150.2/24 |
>    +----------------------------------------+                 +--------------------------+
> 
> For testing purpose I connected by ssh to 192.168.100.1 and sent udp
> packets to the laptop IPs with the following result:
> * When sending an UDP packet to 192.168.120.2 ==> No issue
> * When sending an UDP packet to 192.168.200.2 (vlan 2833) ==> I have the malformed packet issue
> * When sending an UDP packet to 192.168.150.2 (vlan 1000) ==> I receive no network traffic
> and I lose the ssh connection during 2 minutes with nothing in dmesg during this period.
> 
> Le 13/04/2020 à 20:20, Eric Dumazet a écrit :
>> On Mon, Apr 13, 2020 at 11:06 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>> On 06.04.2020 16:12, Charles DAYMAND wrote:
>>>> Hello,
>>>> Sorry for the long delay, I didn't have physical access to the
>>>> hardware last week. I did more testing today by connecting directly my
>>>> laptop to the hardware and the issue not only affects multicast but
>>>> also unicast with macvlan.
>>>> Only ping packets are correctly sent.
>>>> Using wireshark I can only see malformed ethernet packets (example
>>>> with multicast packet below)
>>>> Frame 1: 106 bytes on wire (848 bits), 106 bytes captured (848 bits)
>>>> IEEE 802.3 Ethernet
>>>>      Destination: IPv4mcast_09 (01:00:5e:00:00:09)
>>>>      Source: b2:41:6f:04:c1:86 (b2:41:6f:04:c1:86)
>>>>      Length: 96
>>>>          [Expert Info (Error/Malformed): Length field value goes past
>>>> the end of the payload]
>>>> Logical-Link Control
>>>>      DSAP: Unknown (0x45)
>>>>      SSAP: Unknown (0xc0)
>>>>      Control field: I, N(R)=46, N(S)=0 (0x5C00)
>>>> Data (88 bytes)
>>>>      Data: 50320c780111087fac159401e0000009020802080048207a...
>>>>      [Length: 88]
>>>> Please find the full pcap file at this url with also a tentative to
>>>> establish a ssh connection :
>>>> https://fourcot.fr/temp/malformed_ethernet2.pcap
>>>>
>>> A problem with HW checksumming should manifest as checksum failure,
>>> I don't think HW checksumming touches length information in headers.
>>> You could enable tracing network events to see whether packets look
>>> suspicious before reaching the network driver.
>>> (or do some printf debugging for the affected multicast packets)
>>>
>>> Because you initially suspected a problem with HW checksumming
>>> in a specific RTL8168 chip variant:
>>> - Check macvlan with another network driver supporting HW checksumming
>>> - Check macvlan with r8169 and another RTL8168 chip variant
>>>
>>> The fact that the issue doesn't exist w/o macvlan seems to indicate
>>> that the network driver can't be blamed here.
>> trace includes evidence of 802.1Q traffic though, maybe "macvlan" is a
>> red herring,
>> some confusion on wording maybe...
>>
>> 802.1Q Virtual LAN, PRI: 0, DEI: 0, ID: 2833
>>      000. .... .... .... = Priority: Best Effort (default) (0)
>>      ...0 .... .... .... = DEI: Ineligible
>>      .... 1011 0001 0001 = ID: 2833
>>
>>
>>>> Best regards
>>>>
>>>> Charles
>>>>
>>>>
>>>> Le mar. 31 mars 2020 à 16:07, Heiner Kallweit <hkallweit1@gmail.com> a écrit :
>>>>> Thanks for further testing! The good news from my perspective is that the issue doesn't occur
>>>>> w/o macvlen, therefore it doesn't seem to be a r8169 network driver issue.
>>>>>
>>>>> W/o knowing tcpdump in detail I think it switches the NIC to promiscuous mode, means
>>>>> it should see all packets, incl. the ones with checksum errors.
>>>>> Maybe you can mirror the port to which the problematic system is connected and
>>>>> analyze the traffic. Or for whatever reason the switch doesn't forward the multicast
>>>>> packets to your notebook.
>>>>>
>>>>> Heiner
>>>>>
>>>>>
>>>>> On 31.03.2020 15:44, Charles DAYMAND wrote:
>>>>>> Hello,
>>>>>> We tested to enable tx checksumming manually (via ethtool) on a kernel 4.19.0-5-amd64 which is the oldest kernel compatible with our software and we observed exactly the same issue.
>>>>>> For information when connecting a laptop directly to the interface we can't see any multicast packet when tx checksumming is enabled on tcpdump.
>>>>>> Our network is composed of a cisco switch and we can still see the multicast counters correctly increasing even when we have the issue.
>>>>>>
>>>>>> I also confirm that when not using macvlan but the real interface there is no issue on the multicast packets, they are correctly sent and received.
>>>>>> I have a stupid question, if the IP checksum was bad on the multicast packet, would the receiver NIC drop the packet or would it be seen by tcpdump by the receiver ?
>>>>>>
>>>>>> Le ven. 27 mars 2020 à 20:43, Eric Dumazet <edumazet@google.com <mailto:edumazet@google.com>> a écrit :
>>>>>>
>>>>>>      On Fri, Mar 27, 2020 at 12:17 PM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
>>>>>>      >
>>>>>>      > On 27.03.2020 19:52, Eric Dumazet wrote:
>>>>>>      > > On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@gmail.com <mailto:hkallweit1@gmail.com>> wrote:
>>>>>>      > >>
>>>>>>      > >> On 27.03.2020 10:39, Heiner Kallweit wrote:
>>>>>>      > >>> On 27.03.2020 10:08, Charles Daymand wrote:
>>>>>>      > >>>> During kernel upgrade testing on our hardware, we found that macvlan
>>>>>>      > >>>> interface were no longer able to send valid multicast packet.
>>>>>>      > >>>>
>>>>>>      > >>>> tcpdump run on our hardware was correctly showing our multicast
>>>>>>      > >>>> packet but when connecting a laptop to our hardware we didn't see any
>>>>>>      > >>>> packets.
>>>>>>      > >>>>
>>>>>>      > >>>> Bisecting turned up commit 93681cd7d94f
>>>>>>      > >>>> "r8169: enable HW csum and TSO" activates the feature NETIF_F_IP_CSUM
>>>>>>      > >>>> which is responsible for the drop of packet in case of macvlan
>>>>>>      > >>>> interface. Note that revision RTL_GIGA_MAC_VER_34 was already a specific
>>>>>>      > >>>> case since TSO was keep disabled.
>>>>>>      > >>>>
>>>>>>      > >>>> Deactivating NETIF_F_IP_CSUM using ethtool is correcting our multicast
>>>>>>      > >>>> issue, but we believe that this hardware issue is important enough to
>>>>>>      > >>>> keep tx checksum off by default on this revision.
>>>>>>      > >>>>
>>>>>>      > >>>> The change is deactivating the default value of NETIF_F_IP_CSUM for this
>>>>>>      > >>>> specific revision.
>>>>>>      > >>>>
>>>>>>      > >>>
>>>>>>      > >>> The referenced commit may not be the root cause but just reveal another
>>>>>>      > >>> issue that has been existing before. Root cause may be in the net core
>>>>>>      > >>> or somewhere else. Did you check with other RTL8168 versions to verify
>>>>>>      > >>> that it's indeed a HW issue with this specific chip version?
>>>>>>      > >>>
>>>>>>      > >>> What you could do: Enable tx checksumming manually (via ethtool) on
>>>>>>      > >>> older kernel versions and check whether they are fine or not.
>>>>>>      > >>> If an older version is fine, then you can start a new bisect with tx
>>>>>>      > >>> checksumming enabled.
>>>>>>      > >>>
>>>>>>      > >>> And did you capture and analyze traffic to verify that actually the
>>>>>>      > >>> checksum is incorrect (and packets discarded therefore on receiving end)?
>>>>>>      > >>>
>>>>>>      > >>>
>>>>>>      > >>>> Fixes: 93681cd7d94f ("r8169: enable HW csum and TSO")
>>>>>>      > >>>> Signed-off-by: Charles Daymand <charles.daymand@wifirst.fr <mailto:charles.daymand@wifirst.fr>>
>>>>>>      > >>>> ---
>>>>>>      > >>>>  net/drivers/net/ethernet/realtek/r8169_main.c | 3 +++
>>>>>>      > >>>>  1 file changed, 3 insertions(+)
>>>>>>      > >>>>
>>>>>>      > >>>> diff --git a/net/drivers/net/ethernet/realtek/r8169_main.c b/net/drivers/net/ethernet/realtek/r8169_main.c
>>>>>>      > >>>> index a9bdafd15a35..3b69135fc500 100644
>>>>>>      > >>>> --- a/net/drivers/net/ethernet/realtek/r8169_main.c
>>>>>>      > >>>> +++ b/net/drivers/net/ethernet/realtek/r8169_main.c
>>>>>>      > >>>> @@ -5591,6 +5591,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>>>>>>      > >>>>              dev->vlan_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>>>>      > >>>>              dev->hw_features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>>>>      > >>>>              dev->features &= ~(NETIF_F_ALL_TSO | NETIF_F_SG);
>>>>>>      > >>>> +            if (tp->mac_version == RTL_GIGA_MAC_VER_34) {
>>>>>>      > >>>> +                    dev->features &= ~NETIF_F_IP_CSUM;
>>>>>>      > >>>> +            }
>>>>>>      > >>>>      }
>>>>>>      > >>>>
>>>>>>      > >>>>      dev->hw_features |= NETIF_F_RXALL;
>>>>>>      > >>>>
>>>>>>      > >>>
>>>>>>      > >>
>>>>>>      > >> After looking a little bit at the macvlen code I think there might be an
>>>>>>      > >> issue in it, but I'm not sure, therefore let me add Eric (as macvlen doesn't
>>>>>>      > >> seem to have a dedicated maintainer).
>>>>>>      > >>
>>>>>>      > >> r8169 implements a ndo_features_check callback that disables tx checksumming
>>>>>>      > >> for the chip version in question and small packets (due to a HW issue).
>>>>>>      > >> macvlen uses passthru_features_check() as ndo_features_check callback, this
>>>>>>      > >> seems to indicate to me that the ndo_features_check callback of lowerdev is
>>>>>>      > >> ignored. This could explain the issue you see.
>>>>>>      > >>
>>>>>>      > >
>>>>>>      > > macvlan_queue_xmit() calls dev_queue_xmit_accel() after switching skb->dev,
>>>>>>      > > so the second __dev_queue_xmit() should eventually call the real_dev
>>>>>>      > > ndo_features_check()
>>>>>>      > >
>>>>>>      > Thanks, Eric. There's a second path in macvlan_queue_xmit() calling
>>>>>>      > dev_forward_skb(vlan->lowerdev, skb). Does what you said apply also there?
>>>>>>
>>>>>>      This path wont send packets to the physical NIC, packets are injected
>>>>>>      back via dev_forward_skb()
>>>>>>
>>>>>>      >
>>>>>>      > Still I find it strange that a tx hw checksumming issue should affect multicasts
>>>>>>      > only. Also the chip version in question is quite common and I would expect
>>>>>>      > others to have hit the same issue.
>>>>>>      > Maybe best would be to re-test on the affected system w/o involving macvlen.
>>>>>>      >
>>>>>>      > >
>>>>>>      > >
>>>>>>      > >> Would be interesting to see whether it fixes your issue if you let the
>>>>>>      > >> macvlen ndo_features_check call lowerdev's ndo_features_check. Can you try this?
>>>>>>      > >>
>>>>>>      > >> By the way:
>>>>>>      > >> Also the ndo_fix_features callback of lowerdev seems to be ignored.
>>>>>>      >
>>>>>>
>>>>>>
>>>>>>
>>>>>> -- 
>>>>>>
>>>>>> logo wifirst <http://www.wifirst.fr/en>
>>>>>>
>>>>>> Charles Daymand
>>>>>>
>>>>>> Développeur infrastructure
>>>>>>
>>>>>> 26 rue de Berri 75008 Paris
>>>>>>
>>>>>> Assistance dédiée responsable de site - 01 70 70 46 70
>>>>>> Assistance utilisateur - 01 70 70 46 26
>>>>>>
>>>>

I wonder whether the issue reported here could be related to the one you're facing.
It's also about multicast and macvlan.

https://marc.info/?l=linux-netdev&m=158929343517262&w=2

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Florent Fourcot]

Hello Eric, Hello Heiner,

Just for a small follow-up on your questions and current status on our 
side for this strange topic:

  * As explained by Charles, the setup includes a mix of macvlan + vlan. 
So 802.1q frames are not a typo in the topic (in our real case, the 
second macvlan interface is sent to a network namespace, but it does not 
matter for the current issue).

  * We tested the same setup with other networking driver (bnx2, tg3) 
with tx-checksum enabled (exactly same configuration than the failing 
one on r8169 for checksum offloading), and it's perfectly working.

  * Packets are seen as valid by tcpdump on the sender (buggy) host.

  * Multicast *and* unicast packets are buggy. We detected it first on 
multicast (since the relevant hardware was sending RIP packets), but all 
tcp/udp packets are corrupted. ICMP packets are valid.

  * We expected as well a "simple" checksum failure, but it does not 
look so simple. Something, on our opinion probably the driver or 
hardware, is corrupting the packet.

  * Experimental patches from Heiner are not solving issue.

  * We have the same behavior on at least RTL8168d/8111d, 
RTL8169sc/8110sc and RTL8168h/8111h variants.


As proposed by Heiner, we will try to debug step by step packet path 
inside the kernel to find where it's become corrupted. But it looks time 
consuming for people like us, not really experts in driver development. 
If you have any tips or ideas concerning this topic, it could help us a lot.

Best regards,

-- 
Florent.

Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

[Heiner Kallweit]

On 27.04.2020 23:12, Florent Fourcot wrote:
> Hello Eric, Hello Heiner,
> 
> Just for a small follow-up on your questions and current status on our side for this strange topic:
> 
>  * As explained by Charles, the setup includes a mix of macvlan + vlan. So 802.1q frames are not a typo in the topic (in our real case, the second macvlan interface is sent to a network namespace, but it does not matter for the current issue).
> 
>  * We tested the same setup with other networking driver (bnx2, tg3) with tx-checksum enabled (exactly same configuration than the failing one on r8169 for checksum offloading), and it's perfectly working.
> 
>  * Packets are seen as valid by tcpdump on the sender (buggy) host.
> 
>  * Multicast *and* unicast packets are buggy. We detected it first on multicast (since the relevant hardware was sending RIP packets), but all tcp/udp packets are corrupted. ICMP packets are valid.
> 
>  * We expected as well a "simple" checksum failure, but it does not look so simple. Something, on our opinion probably the driver or hardware, is corrupting the packet.
> 
>  * Experimental patches from Heiner are not solving issue.
> 
>  * We have the same behavior on at least RTL8168d/8111d, RTL8169sc/8110sc and RTL8168h/8111h variants.
> 
> 
> As proposed by Heiner, we will try to debug step by step packet path inside the kernel to find where it's become corrupted. But it looks time consuming for people like us, not really experts in driver development. If you have any tips or ideas concerning this topic, it could help us a lot.
> 
> Best regards,
> 

Previously in this thread you sent an example of a corrupted packet, a LLC packet.
For my understanding, is LLC configured in your config? See net/llc/Kconfig.
If yes, then does the issue also occur with this option disabled?

Heiner