From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jennifer Dahm <jennifer.dahm@ni.com>
Subject: [RFC PATCH 0/2] net: macb: Disable TX checksum offloading on all Zynq
Date: Fri, 25 May 2018 16:44:12 -0500
Message-ID: <1527284654-24835-1-git-send-email-jennifer.dahm@ni.com>
Mime-Version: 1.0
Content-Type: text/plain
Cc: Nathan Sullivan <nathan.sullivan@ni.com>,
        Jennifer Dahm <jennifer.dahm@ni.com>
To: <netdev@vger.kernel.org>, "David S . Miller" <davem@davemloft.net>,
        Nicolas Ferre <nicolas.ferre@microchip.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mx0a-00010702.pphosted.com ([148.163.156.75]:58112 "EHLO
        mx0b-00010702.pphosted.com" rhost-flags-OK-OK-OK-FAIL)
        by vger.kernel.org with ESMTP id S1030341AbeEYVoa (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 25 May 2018 17:44:30 -0400
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

During testing, I discovered that the Zynq GEM hardware overwrites all
outgoing UDP packet checksums, which is illegal in packet forwarding
cases. This happens both with and without the checksum-zeroing
behavior  introduced  in  007e4ba3ee137f4700f39aa6dbaf01a71047c5f6
("net: macb: initialize checksum when using checksum offloading"). The
only solution to both the small packet bug and the packet forwarding
bug that I can find is to disable TX checksum offloading entirely.

There's still the possibility that these bugs are actually with the
driver software and not with the hardware. I've found several places
where the checksum is set to 0xFFFF (the incorrect checksum found in
small packets) when something goes wrong, and I can imagine a buggy
driver writing over the checksum blindly when TX checksum offloading
is enabled.

I would like feedback on two things:
1. Is it possible that the two bugs described above are caused by the
   driver and not by the hardware? If so, where should I look to
   implicate the driver?
2. Is this a problem we care enough about to completely disable TX
   checksum offloading?

Here is the testing procedure I used to reproduce these bugs on my
machine. Specifically, without this patchset, step 9 fails. Without
007e4ba3ee, step 8 also fails.

1. Set up the test environment:
  a. Acquire a Zynq device with two ethernet ports. This is the DUT.
  b. Acquire a USB-Ethernet adapter.
  c. Acquire two ethernet cables.
  d. Connect one Ethernet port on the DUT to your computer's network
     switch.
  e. Connect the other Ethernet port to the USB-Ethernet adapter and
     plug that adapter into your computer.
  f. Set up a Linux VM to send packets through the DUT. I recommend
     using a VM here so that you can easily detach it from the primary
     network to force outgoing traffic through the DUT.
  g. Set up a computer with a packet inspecting program to receive and
     inspect packets. This doesn't need to be a VM. For the purposes
     of this test, I'll be using a Windows instance with WireShark.
2. Load the kernel you want to test onto the DUT, making sure to
   include the `bridge` module.
3. Set up a bridge on the DUT. The following commands on the DUT
   should work, replacing `eth0` and `eth1` with the two ethernet
   interfaces on the DUT:
   ```
   brctl addbr test
   brctl addif test eth0 eth1
   ifconfig eth0 0.0.0.0
   ifconfig eth1 0.0.0.0
   dhclient test -v
   ```
4. Disconnect the Linux VM from your host computer's network and
   connect it to the USB-Ethernet adapter in order to force outgoing
   network traffic through the DUT. If necessary, run dhclient on the
   Linux VM to acquire an IP address.
5. Ensure that you can reach your Windows instance from your Linux VM
   through the DUT (e.g. ping).
6. Start WireShark on your Windows instance and start monitoring
   traffic on a specific, unused port (e.g. 61557).
7. Using netcat, send a few not-tiny UDP packets from your Linux VM to
   your Windows instance to ensure that valid UDP packets are properly
   forwarded. Ex:
   ```
   echo "hello world" | netcat -u <WindowsIP> 61557
   ```
   Inspect these packets to ensure that the data arrived intact and
   that the checksum looks reasonable (i.e. not 0x0000 or 0xFFFF).
8. Using netcat, send a few tiny UDP packets (2 bytes or fewer) from
   Linux VM to your Windows instance to ensure that the checksum is
   reasonable. Ex:
   ```
   echo "h" | netcat -u <WindowsIP> 61557
   ```
9. Using a custom program, send UDP packets with broken checksums
   (e.g. 0xABCD) from your Linux VM to your Windows instance. Inspect
   these packets with WireShark and make sure that the packet arrived
   with the same checksum you sent it with.

For step 9, I wrote a C program using the Linux socket API that will
send a properly formatted UDP packet with the payload "Hello!" and a
(broken) checksum of 0xABCD to port 61557 on the host provided at the
command line. I can send the full program if you would like, but here
is the important part of it:
```
struct custom_udp {
	int16_t s_port;
	int16_t d_port;
	int16_t length;
	int16_t check;
	char data[];
};

int send_message(int sockfd, in_port_t port, const char *message) {
	struct custom_udp *frame;
	int16_t message_len;
	int16_t frame_len;
	int ret;

	message_len = strlen(message) * sizeof(char);
	frame_len = sizeof(struct custom_udp) + message_len;
	frame = malloc(frame_len);
	frame->s_port = htons(0);
	frame->d_port = htons(port);
	frame->length = htons(frame_len);
	frame->check = htons(0xABCD);
	memmove(frame->data, message, message_len);

	ret = write(sockfd, frame, frame_len);
	free(frame);

	return ret;
}
```

Jennifer Dahm (1):
  net/macb: Disable TX checksum offloading on all Zynq-7000

 drivers/net/ethernet/cadence/macb.h      |  1 +
 drivers/net/ethernet/cadence/macb_main.c | 11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.7.4