Subject: Re: [PATCHv2] ipv6: ip6mr: Recalc UDP checksum before forwarding
To: David Miller <davem@davemloft.net>
Cc: eric.dumazet@gmail.com, marcelo.leitner@gmail.com,
        kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <1513187564.25033.65.camel@gmail.com>
 <1513251423-4166-1-git-send-email-redmcg@redmandi.dyndns.org>
 <20171215.132728.1988249158186497781.davem@davemloft.net>
From: Brendan McGrath <redmcg@redmandi.dyndns.org>
Message-ID: <57a7e96d-2e48-4755-d91b-f84b224af4b1@redmandi.dyndns.org>
Date: Sat, 16 Dec 2017 23:19:18 +1100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.0
MIME-Version: 1.0
In-Reply-To: <20171215.132728.1988249158186497781.davem@davemloft.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3169
Lines: 78

The network diagram is very simple. It is just:
VM <------* MR --------> PH

Where:
MR = Multicast Router
VM = Virtual Machine (connected by a Virtual Interface using the 
'virtio_net' driver)
PH = Physical Host (connected by a physical Ethernet connection)
* = The interface where the packet originates (the 'virtio net' interface)

Due to a MFC entry - the packet is forwarded from the virtio interface 
to the physical interface. There is an assumption in this forwarding 
process that the checksum would already be calculated.

But I have found that with 'tx checksum offloading' on - the 
'virtio_net' driver does not appear to generate a checksum at all. The 
assumption here is that the packet will only ever be seen internal to 
the virtio network.

But this scenario sits outside both those assumptions - hence the issue.

This patch looked to address the assumption made in the forwarding 
process - but I now think the issue is with the virtio assumption. Some 
research on the Internet highlighted other issues with the virtio 
assumption. They are:
1. Applications that look at the entire packet and validate checksum 
themselves (such as some DHCP clients); and
2. Tunnels - where the packet is placed inside a tunnel as is and 
delivered elsewhere

And of course this scenario.

This archived libvirt-users post actually gave me a couple of ideas to try:
https://www.redhat.com/archives/libvirt-users/2016-March/msg00034.html

When I disable tx checksum offloading on the virtio interface (via 
'ethtool -K virbr0 tx off') - the checksum is calculated correctly and 
everything works.

I can also get it to work by adding the following ip6filter entry:
ip6tables -t mangle -A POSTROUTING -o virbr0 -d ff00::/8 -j CHECKSUM 
--checksum-fill

So I think this patch can be withdrawn in favour of one of these two 
workarounds.

On 16/12/17 05:27, David Miller wrote:
> From: Brendan McGrath <redmcg@redmandi.dyndns.org>
> Date: Thu, 14 Dec 2017 22:37:03 +1100
>
>> Currently, when forwarding a multicast packet originating from a
>> Virtual Interface on a Multicast Router to one of its Physical
>> Interfaces, ip_summed is set to a value of CHECKSUM_UNNECESSARY and
>> the UDP checksum is not calculated.
>>
>> The checksum value of the forwarded packet is left as is and
>> therefore rejected by the receiving machine(s).
>>
>> This patch ensures the checksum is recalculated before forwarding.
>>
>> Signed-off-by: Brendan McGrath <redmcg@redmandi.dyndns.org>
> I still don't like this, UDP can't be the only protocol that goes
> over multicast and might therefore have this problem.
>
> Actually, I'm still also having trouble figuring out how this happens
> in the first place.
>
> Please draw a specific detailed network diagram, show the exact
> configuration of each interface and exactly what driver runs that
> interface, and show where the packet comes from, who creates it, and
> where these checksum settings are done that lead to this problem.
>
> Like Eric, I'm very suspicious and I think that there is some problem
> with whoever builds or modifies this packet, and the code you are
> touching has no business "fixing it up"
>
> Thank you.