Page MenuHomeFreeBSD

Try to fix vmxnet3 when RSS option is enabled
ClosedPublic

Authored by avg on Jan 12 2020, 7:49 PM.

Details

Summary

We observe at least one problem: if a UDP socket is connect-ed, then a
received packet that matches the connection cannot be matched to the
corresponding PCB because of an incorrect flow ID. That was oberved for DNS
requests from the libc resolver. We got this problem because FreeBSD
r343291 enabled code that can set rsstype of received packets to values
other than M_HASHTYPE_OPAQUE_HASH. Earlier that code was under 'ifdef
notyet'. The commit message did not provide any rationale.

The essence of this change is to use the system-wide RSS key instead of some
historic hardcoded key.

Bug: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=242890
Sponsored by Panzura.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 28624
Build 26656: arc lint + arc unit

Event Timeline

avg created this revision.Jan 12 2020, 7:49 PM

I have updated bug 242890 to include the rationale for enabling (and also modifying) the #ifdef notyet RSS code when converting the driver to iflib, and also raised the question of whether this same issue exists for the bnxt driver.

sys/dev/vmware/vmxnet3/if_vmx.c
1164–1167

I think the correct way to do this is to modify vmxnet3_attach_post() such that when RSS is defined, if rss_gethashalgo() is not TOEPLITZ, the VMXNET3_FLAGS_RSS flag will not get set in sc->vmx_flags. When that flag is not set, vmxnet3_reinit_rss_shared_data() will never be called, and thus can be written assuming that rss->hash_func should always be set to TOEPLITZ.

The way it is written now, we have to wonder whether the vmxnet3 virtual device implementation will tolerate RSS enabled with hash_func set to NONE. We have no documentation for this virtual device implementation and no reference code that tries this mode. We do however know (via the successful operation of both the FreeBSD and Linux drivers) that the virtual device works when RSS is disabled in the virtual device requested features, and also when it is enabled in the requested features with the hash_func sent to TOEPLITZ.

avg added inline comments.Jan 13 2020, 8:03 AM
sys/dev/vmware/vmxnet3/if_vmx.c
1164–1167

Another idea that I've just got is to regress to using M_HASHTYPE_OPAQUE_HASH for all received packets when rss_gethashalgo() is not TOEPLITZ while still configuring the "hardware" to use TOEPLITZ. With that, we still get the benefit of distributing incoming packets between queues / CPUs, but the software RSS will not expect that hardware hashes are the same as software hashes.

pkelsey added inline comments.Jan 13 2020, 5:54 PM
sys/dev/vmware/vmxnet3/if_vmx.c
1164–1167

My understanding is that with that approach, the benefit would be that the work of the hashes in software would then be distributed across multiple cores (it would be done on the core handling the given receive queue prior to dispatching the packet to the core indicated by the computed hash). If that's the case, seems reasonable to me.

avg updated this revision to Diff 66744.Jan 14 2020, 4:58 PM

Always configure the virtual NIC to use Toeplitz algorithm.

If the software RSS is enabled and it is configured with the same algorithm,
then use the same hash key and report flow hash types used by the hardware.
In this configuration, the software and the "hardware" should be in
agreement about hash types and values.

If the software RSS is not enabled or if it uses a different algorithm, then
use the internal hash key and report the flow hash type as opaque regardless
of the hardware determined type. The hardware and the software would not be
in agreement about hash values anyway. And if the software RSS is not
enabled then there is nothing at all to agree about.

pkelsey added inline comments.Jan 15 2020, 5:43 AM
sys/dev/vmware/vmxnet3/if_vmx.c
1193

I think you mean for this to be #endif, and for the #endif below to go away.

avg updated this revision to Diff 66772.Jan 15 2020, 7:19 AM

fix brain-o

avg marked an inline comment as done.Jan 15 2020, 7:23 AM
avg added inline comments.
sys/dev/vmware/vmxnet3/if_vmx.c
1193

Oops!

Looks good to me.

sys/dev/vmware/vmxnet3/if_vmx.c
1544–1545

This could be placed under #ifdef RSS to leave the code out as VMXNET_FLAG_SOFT_RSS can't be set when RSS is not defined. Probably does not make much of a practical difference given this driver is not run on very constrained platforms. I'll leave that up to you as to whether to do it.

pkelsey accepted this revision.Jan 15 2020, 3:36 PM
This revision is now accepted and ready to land.Jan 15 2020, 3:36 PM
This revision was automatically updated to reflect the committed changes.
avg marked an inline comment as done.