On Mon, 29 Jun 2020 17:38:33 -0700 Michael Chan wrote: > On Mon, Jun 29, 2020 at 5:06 PM Jakub Kicinski <k...@kernel.org> wrote: > > On Mon, 29 Jun 2020 02:34:22 -0400 Michael Chan wrote: > > > With the new infrastructure in place, we can now support the setting of > > > the indirection table from ethtool. > > > > > > The user-configured indirection table will need to be reset to default > > > if we are unable to reserve the requested number of RX rings or if the > > > RSS table size changes. > > > > > > Signed-off-by: Michael Chan <michael.c...@broadcom.com> > > > > Hm. Clearing IFF_RXFH_CONFIGURED seems wrong. The user has clearly > > requested a RSS mapping, if it can't be maintained driver should > > return an error from the operation which attempts to change the ring > > count. > > Right. In this case the user has requested non default RSS map and is > now attempting to change rings. We have a first level check by > calling bnxt_check_rings(). Firmware will tell us if the requested > rings are available or not. If not, we will return error and the > existing rings and RSS map will be kept. This should be the expected > outcome in most cases. > > In rare cases, firmware can return success during bnxt_check_rings() > but during the actual ring reservation, it fails to reserve all the > rings it promised were available earlier. In this case, we fall back > and accept the fewer rings and set the RSS map to default. I have > never seen this scenario but we need to put the code in just in case > it happens. It should be rare.
What's the expected application flow? Every time the application makes a change to NIC settings it has to re-validate that some of the previous configuration didn't get lost? I don't see the driver returning the error if FW gave it less rings than requested. There isn't even a warning printed.. I'd prefer if the driver wrapped the rss indexes, and printed a warning, but left the config intact. And IMO set_channels should return an error.
