Page MenuHomeFreeBSD

Chelsio T4/T5 VF driver.
ClosedPublic

Authored by jhb on Aug 22 2016, 6:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 23, 3:11 PM
Unknown Object (File)
Nov 18 2024, 3:37 PM
Unknown Object (File)
Nov 17 2024, 5:24 AM
Unknown Object (File)
Nov 16 2024, 4:55 PM
Unknown Object (File)
Nov 14 2024, 8:33 AM
Unknown Object (File)
Nov 13 2024, 8:46 AM
Unknown Object (File)
Nov 13 2024, 5:47 AM
Unknown Object (File)
Nov 13 2024, 2:17 AM
Subscribers

Details

Reviewers
jhibbits
np
Group Reviewers
manpages
Commits
rS305549: Chelsio T4/T5 VF driver.
Summary

Chelsio T4/T5 VF driver.

The cxgbev/cxlv driver supports Virtual Function devices for Chelsio
T4 and T4 adapters. The VF devices share most of their code with the
existing PF4 driver (cxgbe/cxl) and as such the VF device driver
currently depends on the PF4 driver.

Similar to the cxgbe/cxl drivers, the VF driver includes a t4vf/t5vf
PCI device driver that attaches to the VF device. It then creates
child cxgbev/cxlv devices representing ports assigned to the VF.
By default, the PF driver assigns a single port to each VF.

t4vf_hw.c contains VF-specific routines from the shared code used to
fetch VF-specific parameters from the firmware.

t4_vf.c contains the VF-specific PCI device driver and includes its
own attach routine.

VF devices are required to use a different firmware request when
transmitting packets (which in turn requires a different CPL message
to encapsulate messages). This alternate firmware request does not
permit chaining multiple packets in a single message, so each packet
results in a firmware request. In addition, the different CPL message
requires more detailed information when enabling hardware checksums,
so parse_pkt() on VF devices must examine L2 and L3 headers for all
packets (not just TSO packets) for VF devices. Finally, L2 checksums
on non-UDP/non-TCP packets do not work reliably (the firmware trashes
the IPv4 fragment field), so IPv4 checksums for such packets are
calculated in software.

Most of the other changes in the non-VF-specific code are to expose
various variables and functions private to the PF driver so that they
can be used by the VF driver.

Note that a limited subset of cxgbetool functions are supported on VF
devices including register dumps, scheduler classes, and clearing of
statistics. In addition, TOE is not supported on VF devices, only for
the PF interfaces.

Test Plan
  • Have tested IPv4 via ping, netperf, ssh.
  • Still need to check IPv6
  • Still need to write a manpage
  • Need to do a make tinderbox build

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jhb retitled this revision from to Chelsio T4/T5 VF driver..
jhb updated this object.
jhb edited the test plan for this revision. (Show Details)
jhb added a reviewer: np.
  • Relax assertion since UDP/IP6 packets on a VF will hit this.
  • Don't break out of the m_advance() loop if len drops to zero.
  • First stab at a manpage for the VF driver.
  • Note that the VF driver depends on the PF driver.
share/man/man4/cxgbev.4
254 ↗(On Diff #19555)

I don't recall what you said earlier about congestion control for VFs? Is it that they are effectively forced to do the equivalent of 'cong_drop=1'? Will they ever send PAUSE frames?

sys/dev/cxgbe/t4_sge.c
2084 ↗(On Diff #19555)

This is a recent change that fixed a panic with the UDP_STREAM netperf test on the VF driver. More details in the git commit, but that test generated packets where the first mbuf was the Ethernet header, and the second mbuf contained IP + UDP + payload. This loop terminated after walking past the first mbuf due to len being 0 (when looking for the L3 header) without setting p. As a result it returned a NULL pointer which panic'd. The forever loop ensures it sets p to the start of the second mbuf before breaking out of the loop.

sys/dev/cxgbe/t4_vf.c
59 ↗(On Diff #19555)

I think the Linux VF driver has a module parameter for this. I could add a tunable/sysctl to enforce the same thing.

573 ↗(On Diff #19555)

Not sure if we want to try to be this fancy or if I should just remove the comment or remove the #ifdef around it?

wblock added inline comments.
share/man/man4/cxgbev.4
59 ↗(On Diff #19555)

The second "Terminator" is redundant, can just say "Terminator 4 and 5 ASICs"

71 ↗(On Diff #19555)

Is "Note that" needed again on this second sentence?

83 ↗(On Diff #19555)

That aside should not be there. The sentence is introducing a list of supported cards, and then suddenly veers into a tree.
It should at least use .Ql for the "cxlv" part. Maybe move that to a separate sentence after the list of cards?

(card list)

Ports created for the T5 cards are named
.Ql cxlv .
154 ↗(On Diff #19555)

(Not part of your patch, I know.)

Lists like these generally read better without the redundant introductory article on every item. So just "Number of tx queues..."

242 ↗(On Diff #19555)

"Note that" is probably not needed here, or maybe could be said differently.

jhb marked an inline comment as done.Aug 26 2016, 9:12 PM
jhb added inline comments.
share/man/man4/cxgbev.4
59 ↗(On Diff #19555)

This is actually in the source manpage (cxgbe.4). Note that "Terminator 4" is the product name of the ASIC. I'm not sure if Chelsio would prefer that the names be explicit vs combined.

71 ↗(On Diff #19555)

It is repetitive, but I wanted a way to call out the sharing since it might be a potential "gotcha" for users who may not expect the same variable to affect two different (albeit related) drivers. Do you have a wording suggestion?

83 ↗(On Diff #19555)

this is also copied from the cxgbe.4 page, though I can fix that in both places. We do already mention cxlv vs cxgbe earlier around line 68. I think the desire in cxgbe(4) was to note the difference close to the list of cards. However, it might be that we can just drop the aside entirely?

jhb edited edge metadata.
  • Trim a "Note that.."
  • Exclude cxgbev from platforms that exclude cxgbe.
  • Don't include manual in_cksum to fix !INET build.

FYI, ipv6 has been tested and tinderbox now builds ok.

jhibbits removed a reviewer: jhibbits.
jhb edited edge metadata.

Rebase.

jhb edited edge metadata.
  • Use t4_get_version_info() from upstream.
  • Go back to reading fw and TP versions via firmware requests.

This does end up exporting firmware_version and tp_version sysctl nodes
for VFs.

share/man/man4/cxgbev.4
71 ↗(On Diff #19555)
Both T4 and T5 Physical Function drivers share these same tunables.
83 ↗(On Diff #19555)

I'd say either option is preferable to leaving it where it is now, but don't know which is better. Your choice. :)

jhb edited edge metadata.
  • Wording fixes from Warren.
jhb marked an inline comment as done.Sep 6 2016, 6:47 PM
jhb added inline comments.
share/man/man4/cxgbev.4
71 ↗(On Diff #19555)

I'm not sure that makes it clear that hw.cxgbe.foo affects both cxgbeX devices (T4 PF driver) and cxgbevX (T4 VF driver). Maybe just s/Both/The/ works though. (I think with "both" there is the chance that it could be misread as just saying that the two PF drivers share the tunables since "both" implies exactly 2, whereas in this case it is really 4 things sharing {T4,T5}{VF,PF}) Actually, the previous wording makes it clear that even though there are multiple names, there is a shared driver (which is true, kldload if_cxgbe gives you both cxgbe and cxl). I've gone with:

The Physical Function driver for T4 and T5 adapters shares these tunables.
jhb edited edge metadata.
  • Document congestion behavior for VFs.
np edited edge metadata.

Please commit the m_advance fix separately.

This revision is now accepted and ready to land.Sep 7 2016, 1:02 AM
This revision was automatically updated to reflect the committed changes.