RE: [PATCH net-next v3 00/14] Add functional support for Gigabit Ethernet driver

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:

quoted

quoted

On Tue, 12 Oct 2021 17:35:59 +0100 Biju Das wrote:

quoted

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