Project

General

Profile

Actions

Bug #12465

closed

vioif needs length for tso checksum

Added by Robert Mustacchi about 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Category:
driver - device drivers
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

For a while I'd noticed that pkg update between two VMs on a Linux laptop were having pretty terrible transfer performance. When Josh suggested that he was seeing issues in a virtualized environment and that disabling all of the virtio driver's checksum negotiation was an issue, I used that as a prompt to further investigate my issue. I quickly found that disable TSO via ndd (ip_lso_outbound) made the problem go away. With that, i wrote a small C program that just spit out a 10k packet every few seconds with a deterministic pattern. Confirming the same pattern as during the pkg update, I found retransmits happening quite frequently. With that, I set to work with dtrace to figure out where drops were happening on the receiving end.

With that in mind, I was able to find that we were triggered the ip drop-in probes with the reason of a bad ULP checksum. With that in mind, I started focusing on code in the vioif driver, the spec, and others. At first I noticed a few things:

1. That we weren't setting the hdr_len hint
2. That we weren't passing the ECN flag when that bit was set.

As luck would have it, modifying things for this didn't really solve any problems. With that in mind, I took a deeper look at the spec and a few other aspects of different implementations. That brought me back to a comment in the viona driver and triggered some memories.

When performing checksum offload, you often need need to include the pseudo-header checksum for say TCP or UDP when submitting the packet. As a result, networking stacks, including illumos, do this. However, when TSO is on the scene, most hardware does not want the length of the packet included in the pseudo-header, since they'll be changing it all the time. The result of this is that on illumos, we don't actually include the length in that psuedo-header calculation. This simplifies pretty much every other driver. However, the virtio spec (and other networking stacks) do expect the length in the case of TSO. As a result, we need to go back and actually include it.

To make this a reality, I added some logic to do this and reused some of the packet walking functions that I put together originally for i40e. With this in place, I was able to properly perform IPv4 TSO without retransmits between the two guests. What used to be a 30-45 minute pkg update finished within a minute or two now.

Actions #1

Updated by Electric Monk about 2 years ago

  • Status changed from New to Closed
  • % Done changed from 90 to 100

git commit d240edaf609c558d5a1f981b09a577823b54fae2

commit  d240edaf609c558d5a1f981b09a577823b54fae2
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2020-04-08T11:07:04.000Z

    12465 vioif needs length for tso checksum
    Reviewed by: Jason King <jason.brian.king@gmail.com>
    Reviewed by: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
    Reviewed by: Paul Winder <paul@winders.demon.co.uk>
    Approved by: Garrett D'Amore <garrett@damore.org>

Actions

Also available in: Atom PDF