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

[Charles DAYMAND <charles.daymand@wifirst.fr>]

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