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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fe30ab8d-2915-e049-ef30-760960f5efdc@gmail.com>
Date:   Wed, 8 Apr 2020 00:40:32 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Charles DAYMAND <charles.daymand@...irst.fr>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net] r8169: fix multicast tx issue with macvlan interface

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