Page MenuHomeFreeBSD

netinet: Generate a random RSS key on boot.
Needs RevisionPublic

Authored by neel_neelc.org on May 24 2020, 8:58 PM.

Details

Reviewers
markm
Group Reviewers
csprng
Summary

netinet: Generate a random RSS key on boot.

Submitted by: Neel Chauhan <neel AT neelc DOT org>

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

neel_neelc.org created this revision.May 24 2020, 8:58 PM
neel_neelc.org requested review of this revision.May 24 2020, 8:58 PM

Did you mean to add some reviewers for this too? :)
Looks like @rwatson might have some thoughts on this :)

melifaro added inline comments.May 25 2020, 10:04 PM
sys/net/rss_config.c
251

Are we sure that there is enough entropy available at the moment of calling it?

neel_neelc.org added inline comments.May 30 2020, 8:03 PM
sys/net/rss_config.c
251

I'm not sure, I'm not an expert on entropy, but we could wait for entropy before calling this.

kevans added a subscriber: kevans.May 30 2020, 8:06 PM
kevans added inline comments.
sys/net/rss_config.c
251

When in doubt, perhaps CC csprng

neel_neelc.org added a project: csprng.
neel_neelc.org added a subscriber: csprng.

@kevans thanks for the suggestion, I added csprng to this thread.

cem added a subscriber: cem.May 30 2020, 9:35 PM
cem added inline comments.
sys/net/rss_config.c
251

With csprng hat on: The random subsystem does not guarantee it has been seeded at any point during auto-configuration (SYSINIT).

If it has not been seeded yet, arc4random currently gives you a weakly random key (hash of the CPU clock cycle counter, essentially); that is still marginally more random than the hardcoded value this revision deletes, but not especially random. The API doesn't tell you which kind of output you got.

As far as waiting for entropy before initializing the key, doesn't that break RSS for existing connections? If that is tolerable, then that seems like a good solution. I'd want someone familiar with networking to approve that design (and this design!).

I'm not sure what the deleted XXXRW comment is referring to as far as rss_isbadkey().

As far as the actual arc4random_buf invocation, s/&rss_key/rss_key/.

@cem thanks for the comments.

I fixed arc4random_buf().

About waiting for entropy, I think you're right.

markm requested changes to this revision.Jun 1 2020, 8:14 AM
markm added a subscriber: markm.
markm added inline comments.
sys/net/rss_config.c
251

This scares me less than the previously hard-coded key, but if we are going to the trouble to randomise, then we need to sequence it properly.

This revision now requires changes to proceed.Jun 1 2020, 8:14 AM
avg added a subscriber: avg.Jun 1 2020, 10:17 AM

I have a vague memory, maybe wrong, that commonly used fixed RSS keys were selected because they had some property (-ies).
So, maybe just being random is not good enough?
I think that hypothetical rss_isbadkey was mentioned for a reason?

ae added a comment.Jun 1 2020, 11:44 AM
In D24989#552576, @avg wrote:

I have a vague memory, maybe wrong, that commonly used fixed RSS keys were selected because they had some property (-ies).
So, maybe just being random is not good enough?
I think that hypothetical rss_isbadkey was mentioned for a reason?

I also have such feeling. For example, you have some server that handles some serious workload, but after reboot due to the new key it will not be able to handle the same workload.

neel_neelc.org added a comment.EditedJun 1 2020, 4:29 PM
In D24989#552576, @avg wrote:

I have a vague memory, maybe wrong, that commonly used fixed RSS keys were selected because they had some property (-ies).
So, maybe just being random is not good enough?
I think that hypothetical rss_isbadkey was mentioned for a reason?

I Google searched this and haven't found much evidence on RSS keys. I could be wrong as well.

EDIT: There is evidence on "Symmetric" RSS keys like seen here: http://galsagie.github.io/2015/02/26/dpdk-tips-1/

Is that what you're talking about?

Should I sequence the key into "symmetrical RSS" where the first 32 bits == the next 32 bits, and all subsequent 16 bits chunks are equal?. Or is it something else?

avg added a comment.Jun 2 2020, 8:30 AM

You are right. So, my concern was invalid.

adrian added a subscriber: adrian.Jun 3 2020, 11:10 PM

i remember there was some concern in the past where there were very bad rss key choices out there. is there a reason for actually pushing for a random rss key?

I stuck with the microsoft rss key (and a symmetric rss key at norse) specifically so there wouldn't be boot to boot variation in traffic patterns when doing testing/evaluation.

Hi everyone,

At stormshield we are using a similar hand-made patch so i can give you some feedback about this feature.

We support symmetric and non-symmetric static or random rss key, using the following systctls:

  • net.inet.rss.random
  • net.inet.rss.symmetric (using a 16bit pattern)
  • net.inet.rss.symmetric_byte0 + net.inet.rss.symmetric_byte1 (to force the 16bits pattern to make the hash symmetric)

We were also forced to add a reseed sysctl proc after noticing that the initial entropy is too low during rss key init, and we call it after boot just before loading our network kernel modules. Before this we had some product that were using RSS key that fail to provide proper distribution of packets. This change was made before we start using the initial entropy feature of the loader so i am not sure if it is still relevant.

If some of you have interest i can share our patch (in private) which only support the rss_getkey() API and not all the other rss_xxx functions has we are not using them in our codebase.

Damien for Stormshield.