Message-ID: <c02274b9-1ba0-d5e9-848f-5d6761df20f4@gmail.com>
Date:   Tue, 31 Mar 2020 16:07:17 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Charles DAYMAND <charles.daymand@...irst.fr>,
        Eric Dumazet <edumazet@...gle.com>
Cc:     netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

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@...gle.com <mailto:edumazet@...gle.com>> a écrit :
> 
>     On Fri, Mar 27, 2020 at 12:17 PM Heiner Kallweit <hkallweit1@...il.com <mailto:hkallweit1@...il.com>> wrote:
>     >
>     > On 27.03.2020 19:52, Eric Dumazet wrote:
>     > > On Fri, Mar 27, 2020 at 10:41 AM Heiner Kallweit <hkallweit1@...il.com <mailto:hkallweit1@...il.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@...irst.fr <mailto:charles.daymand@...irst.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
> 

Powered by blists - more mailing lists