linux-kernel - Re: [PATCH net-next] ifb: fix packets checksum

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]

Date:   Tue, 29 May 2018 09:13:02 +1000
From:   Jonathan Maxwell <jmaxwell37@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     David Ahern <dsahern@...il.com>, mschiffer@...verse-factory.net,
        zhangshengju@...s.chinamobile.com, ktkhai@...tuozzo.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jon Maxwell <jmaxwell@...hat.com>
Subject: Re: [PATCH net-next] ifb: fix packets checksum

On Sat, May 26, 2018 at 6:43 AM, David Miller <davem@...emloft.net> wrote:
> From: Jon Maxwell <jmaxwell37@...il.com>
> Date: Fri, 25 May 2018 07:38:29 +1000
>
>> Fixup the checksum for CHECKSUM_COMPLETE when pulling skbs on RX path.
>> Otherwise we get splats when tc mirred is used to redirect packets to ifb.
>>
>> Before fix:
>>
>> nic: hw csum failure
>>
>> Signed-off-by: Jon Maxwell <jmaxwell37@...il.com>
>
> This definitely seems correct, but I am really surprised a bug like this has
> lasted as long as it has.
>

Sorry for the late reply I have been away for a few days. The customer never
saw this on bnx2x. Then they switched to the mlx5 driver and it started
happening continuously when doing iperf3 tests and also for other TCP traffic.

mlx5 uses CHECKSUM_COMPLETE. I think that bnx2x uses CHECKSUM UNNECESSARY
which avoided the code path that triggers the csum failure message in
__skb_checksum_complete(). It only logs the message for CHECKSUM_COMPLETE
skbs. Probably few Linux users are using a combination of NIC drivers that
use CHECKSUM_COMPLETE, tc mirred and ifb, which is why this has never been
reported before.

This is very similar to commit 7be709af2b65.

> So I'll let this sit for another day or two for review.