Subject: Re: [PATCH] ipv6: ip6mr: Recalc UDP checksum before forwarding
To: Eric Dumazet <eric.dumazet@gmail.com>,
        "David S . Miller" <davem@davemloft.net>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <1513164048-21368-1-git-send-email-redmcg@redmandi.dyndns.org>
 <1513187564.25033.65.camel@gmail.com>
From: Brendan McGrath <redmcg@redmandi.dyndns.org>
Message-ID: <8b136af8-d319-078f-ff94-4aaee7149aa2@redmandi.dyndns.org>
Date: Thu, 14 Dec 2017 06:02:27 +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: <1513187564.25033.65.camel@gmail.com>
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: 2341
Lines: 67

I should clarify that the packet being forwarded originated on the 
Virtual Interface (i.e. it wasn't received on it). When data is received 
on the Virtual Interface (i.e. sent by a virtual host) then ip_summed is 
CHECKSUM_PARTIAL and the checksum is calculated before transmission on 
the wire.

The scenario I was testing (which caused the csum error) was to:
a) send multicast data to virtual devices over the Virtual Interface; 
and then
b) add an MFC to forward traffic from the Virtual Interface to a 
Physical Interface (so one of the machines on the physical network could 
receive it).

The packet was accepted by the Virtual devices (even though tcpdump 
shows an invalid CRC) but rejected by the Physical devices. My 
assumption here was there is some kind of optimisation (for a 
virtualised Linux kernel on the same host) but I didn't find the code to 
verify that assumption.

My tests and findings were:
MR = Multicast Router
VM = Virtual Machine (connected to the MR via a virtual switch [virbr0])
PH = Physical Machine (connected via a physical switch [br0])

The asterisk indicates where packet originated.
The value of ip_summed is being checked by a netfilter hook registered 
to NF_INET_FORWARD.
The CRC value was checked by tcpdump on the receiving interface.
"Packet accepted" indicates that the application received the packet.

Scenario 1:
VM <------* MR --------> PH

VM CRC: Incorrect (packet accepted)
PH CRC: Incorrect (packet rejected)
ip_summed = 1 (CHECKSUM_UNNECESSARY)


Scenario 2:
VM *------> MR --------> PH

MR(br0) CRC: Incorrect (packet accepted)
PH CRC: Correct (packet accepted)
ip_summed = 3 (CHECKSUM_PARTIAL)


Scenario 3:
VM <------- MR *-------> PH

VM CRC: Correct (packet accepted)
PH CRC: Correct (packet accepted)
ip_summed = 1 (CHECKSUM_UNNECESSARY)


On 14/12/17 04:52, Eric Dumazet wrote:
> On Wed, 2017-12-13 at 22:20 +1100, Brendan McGrath wrote:
>> Currently, when forwarding from a Virtual Interface to a Physical
>> Interface, ip_summed is set to a value of CHECKSUM_UNNECESSARY and
>> the UDP checksum has not been calculated.
>>
> This seems a bug then ?
> CHECKSUM_UNNECESSARY means checksum has been validated.
> Not that we want it being computed later in the stack.
>
> If we force a checksum here, what guarantee do we have packet was not
> corrupted before we do this ?
>