Hi Jakub Kicinski, Thanks for the feedback. > Subject: Re: [PATCH net-next v3 00/14] Add functional support for Gigabit > Ethernet driver > > On Tue, 12 Oct 2021 18:28:07 +0000 Biju Das wrote: > > > On Tue, 12 Oct 2021 17:35:59 +0100 Biju Das wrote: > > > > set_feature patch will send as separate RFC patch along with > > > > rx_checksum patch, as it needs further discussion related to HW > > > checksum. > > > > > > Is this part relating to the crash you observed because of TCP csum > > > offload? > > > > Yes, you are correct. Sergey, suggested use R-Car RX-HW checksum with > > RCSC/RCPT and But the TOE gives either 0x0 or 0xffff as csum output > > and feeding this value to skb->csum lead to kernel crash. > > That's quite concerning. Do you have any of the > > /proc/sys/kernel/panic_on_io_nmi > /proc/sys/kernel/panic_on_oops > /proc/sys/kernel/panic_on_rcu_stall > /proc/sys/kernel/panic_on_unrecovered_nmi > /proc/sys/kernel/panic_on_warn > > knobs set? I'm guessing we hit do_netdev_rx_csum_fault() when the checksum > is incorrect, but I'm surprised that causes a panic. > I tested this last week, if I remember correctly It was not panic, rather do_netdev_rx_csum_fault. I will recheck and will send you the stack trace next time. > > BTW are you seeing 0 / ffff with good or bad checksums? If the device does > a checksum over the IP pseudo-header + TCP only - 0 and ffff would be > correct for a packet with has a valid checksum. But you can't set it to > skb->csum and use CSUM_COMPLETE, you'd need to do something like: > > if (dev_csum == 0 || dev_csum == 0xffff) > skb->ip_summed = CHECKSUM_UNNECESSARY; Yes, it computes IPv4 header checksum and TCP/UDP/ICMP checksum. It will set to 0x0 for both csums, if there is no error. For IPV6, ipv4 header checksum is always set to 0xffff and 0 for TCP/UDP/ICMP checksum. Ok I will use this logic on next RFC. Regards, Biju
