PROBLEM: multicast router does not fill UDP csum of its own forwarded packets

PROBLEM: multicast router does not fill UDP csum of its own forwarded packets

[Strejc Cyril]

[-- Attachment #1: Type: text/plain, Size: 2715 bytes --]

Hi,

please let me summarize a problem regarding Linux multicast routing in combination with L4 checksum offloading and own (locally produced) multicast packets being forwarded.

* Application observation *

Multicast router does not fill-in UDP checksum into locally produced, looped-back and forwarded UDP datagrams, if an original output NIC the datagrams are sent to has UDP TX checksum offload enabled.

* Full description / User story *

I run an application which uses Linux multicast routing capabilities to send equal multicast UDP datagrams to multiple networks. The application sets IP_MULTICAST_IF  and sends each datagram by a single write to a single socket. Properly configured Linux multicast routing in combination with a multicast loop-back ensures the datagrams are forwarded to other network interfaces.

If the outgoing IP_MULTICAST_IF interface has UDP TX checksum offload enabled, the csum is not calculated and is not filled into skb data by kernel. The NIC with TX csum offload calculates and fills csum during transmission, but does not modify skb data in RAM (at least both NICs I have tested).

Then, packet is looped back in ip_mc_finish_output() and dev_loopback_xmit(), where skb->ip_summed is set to CHECKSUM_UNNECESSARY. Since then, packet traverse the network stack with wrong (not filled in) L4 checksum, is forwarded to multicast routing output interfaces with CHECKSUM_UNNECESSARY and hence not correctly updated.

* Kernel info *

Tested: 5.4, 5.15-rc3
I do not know, when the problem was introduced, probably long time ago, maybe in 35fc92a9 ("[NET]: Allow forwarding of ip_summed except CHECKSUM_COMPLETE").

* NIC tested *

I've tested two drivers (NIC) with TX checksum offloading: e1000e in vanilla kernel and NXP's DPAA with out-of-vanilla-tree open-source drivers.

* Steps to reproduce *

It's possible to use shell, ip, smcroute and socat to reproduce the problem. Tested in Linux Mint with smcrouted shell wrapper.

# UCO_IF=eth0              # Interface with UDP TX Checksum Offload enabled.
# DST_IF=eth1
# ip addr add 192.168.1.1/24 dev $UCO_IF
# ip addr add 192.168.2.1/24 dev $DST_IF
# smcroute -d
# smcroute -a $UCO_IF 192.168.1.1 239.192.0.1 $DST_IF
# echo "check" | socat - UDP:239.192.0.1:9,ip-multicast-ttl=2,ip-multicast-if=192.168.1.1

The "check" datagram is sent with wrong UDP csum out of DST_IF. An other computer or physical wire loopback is needed to capture packet as it leaved the DST_IF.

* Workaround *

I use the attached patch as the workaround, not sure at all if it is correct in all cases.

I would be very pleased if anyone could think about a correct approach to the problem.

Thanks,

Cyril

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-net-multicast-calc-csum-of-looped-back-and-forwarded.patch --]
[-- Type: text/x-patch; name="0001-net-multicast-calc-csum-of-looped-back-and-forwarded.patch", Size: 1421 bytes --]

From 217e6516aa4069a555a758478333e476eadcf461 Mon Sep 17 00:00:00 2001
From: Cyril Strejc <cyril.strejc@skoda.cz>
Date: Fri, 8 Oct 2021 21:30:53 +0200
Subject: [PATCH] net: multicast: calc csum of looped-back and forwarded
 packets

Signed-off-by: Cyril Strejc <cyril.strejc@skoda.cz>
---
 include/linux/skbuff.h | 5 ++++-
 net/core/dev.c         | 1 -
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 841e2f0f5240..bbfa6b259bf3 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4048,7 +4048,10 @@ static inline bool __skb_checksum_validate_needed(struct sk_buff *skb,
 						  bool zero_okay,
 						  __sum16 check)
 {
-	if (skb_csum_unnecessary(skb) || (zero_okay && !check)) {
+	if (skb_csum_unnecessary(skb) ||
+      (zero_okay && !check) ||
+      (skb->pkt_type == PACKET_LOOPBACK))
+  {
 		skb->csum_valid = 1;
 		__skb_decr_checksum_unnecessary(skb);
 		return false;
diff --git a/net/core/dev.c b/net/core/dev.c
index 7ee9fecd3aff..ba4a0994d97b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3906,7 +3906,6 @@ int dev_loopback_xmit(struct net *net, struct sock *sk, struct sk_buff *skb)
 	skb_reset_mac_header(skb);
 	__skb_pull(skb, skb_network_offset(skb));
 	skb->pkt_type = PACKET_LOOPBACK;
-	skb->ip_summed = CHECKSUM_UNNECESSARY;
 	WARN_ON(!skb_dst(skb));
 	skb_dst_force(skb);
 	netif_rx_ni(skb);
-- 
2.25.1

Re: PROBLEM: multicast router does not fill UDP csum of its own forwarded packets

[Jakub Kicinski]

On Fri, 8 Oct 2021 20:08:36 +0000 Strejc Cyril wrote:
> please let me summarize a problem regarding Linux multicast routing
> in combination with L4 checksum offloading and own (locally produced)
> multicast packets being forwarded.

Hi Cyril, thanks for the report, looks like nobody has immediate
feedback to share. Could you resend the patch in more usual form 
so that it's easier to review and harder to ignore? 

Please put your description into the commit message (line wrapped 
at 72 characters), run ./scripts/checkpatch.pl --strict on the patch
and submit it with git send-email?

Re: PROBLEM: multicast router does not fill UDP csum of its own forwarded packets

[Strejc Cyril]

On 10/13/21 4:20 PM, Jakub Kicinski wrote:
> On Fri, 8 Oct 2021 20:08:36 +0000 Strejc Cyril wrote:
>> please let me summarize a problem regarding Linux multicast routing
>> in combination with L4 checksum offloading and own (locally produced)
>> multicast packets being forwarded.
> 
> Hi Cyril, thanks for the report, looks like nobody has immediate
> feedback to share. Could you resend the patch in more usual form
> so that it's easier to review and harder to ignore?

Thanks Jakub, I will do so. The patch I sent meant to be illustrative 
for the problem, but of course, the problem can be discussed around the 
patch.