On Thu, 2016-02-04 at 17:53 -0500, David Miller wrote: > From: Jacob Keller <jacob.e.kel...@intel.com> > Date: Tue, 2 Feb 2016 15:22:06 -0800 > > > Ethtool supports a few operations for modifying and controlling > > a device's RSS table. Sometimes, changes in other features of the > > device > > may require (or desire) changes to the RSS table. Currently there > > is no > > method to indicate to the driver whether the current RSS table > > settings > > should be maintained or overridden. > > Yes, there certainly is a way to indicate this. > > If the user asks for the change in the number of queues, and you > cannot retain the user's requested RSS settings, then you must fail > the queue setting change. > > And vice versa. > > You can't say to the user "I can adhere to your requested > configuration > change, but I might undo it for some unspecified reason" >
The trouble here is the case where the indirection table configurations are valid now, but a change in the number of queues happening at a later time currently causes these settings to be lost. > That's unacceptable behavior, and that's exactly what this dynamic > flag means. > > If you cannot give the user what he asks for, precisely and reliably, > you fail the operation with an error. > > There is no way I am adding code which allows these "maybe" kind of > configuration operations. Either you can or you can't, and you tell > the user when you can't by erroring out on the operation that > invalidates the requirements. > > So you're suggesting instead, to error when the second operation (change number of queues) would fail the current settings? Current driver behaviors for all the drivers I checked work in one of two ways. 1) changing queues will destroy the RSS table as it will be reinitialized regardless of current settings 2) changing queues will maintain the RSS table if possible, unless the previous RSS table can't function. No driver currently fails this operation if the RSS table settings can't be preserved. In addition it results in weird behavior when a driver sets the RSS table at load, then increases the number of queues via an ethtool op, the result is that RSS does not use the new queues added by the ethtool operation. I can instead drop the ethtool changes and just have my driver record when the user has changed the tables, and attempt to error on queue setting operation, which may work. Essentially the idea was to have a flag indicating "use the driver defaults" which the driver can change as necessary when the number of queues changes, or other factors that may require RSS table changes. I can do this all hidden in the driver but then there is nothing exposing how the driver will behave under this circumstance. I'm all for a better suggestion, because I think what we're doing now is wrong, and the proposed solutions so far don't seem right either. If we preserve the RSS table when queues increase, then the user may be confused because RSS settings won't spread to the new queues. If we destroy the RSS settings the user will be possibly confused because their selected RSS settings do not work. If we fail the setting of queues when RSS table is not the default value, then a user might be confused as to why they can't change the number of queues. Also, I am unsure whether or not we can tell from the ethtool op function that we actually are being "reset" to the default, vs just being set. This is because the netlink message of "0 length" which indicates default simply has the ethtool core fill in a standard equal weight default, which I am not sure our driver can tell that it should now be ok to enable queue changes. So, something is missing in the current flow to allow this. I think the best solution is simply prevent changing number of queues while we have a non-default RSS setting, and require RSS to be reset before queues can be changed. Thoughts? Regards, Jake