On Wed, 2016-04-20 at 19:22 +0300, Emmanuel Grumbach wrote: > On Wed, 2016-04-20 at 16:47 +0100, Ben Hutchings wrote: > > On Wed, 2016-04-20 at 15:30 +0000, Grumbach, Emmanuel wrote: > > > Hi Ben, > > > > > > > > > Thanks for looking at our code. > > > > > > > > > On Wed, 2016-04-20 at 16:08 +0100, Ben Hutchings wrote: > > > > > > > > I'm not sure if you were aware, but there is a standard API for > > > > configuring RSS in network drivers, part of ethtool_ops. I > > > > think > > > > iwlwifi should implement that rather than a driver-specific > > > > debugfs > > > > interface. > > > > > > > You are right, this is why Sara made this commit: > > > > > > commit 854d773e4ab5869200004af4ca5d851730849903 > > > Author: Sara Sharon <sara.sha...@intel.com> > > > Date: Tue Mar 22 15:55:58 2016 +0200 > > > > > > iwlwifi: mvm: improve RSS configuration > > > > > > Improve current RSS configuration: > > > * Use netdev_rss_key instead of keeping a local copy. > > > * Configure also UDP hashing to have UDP traffic spread > > > across > > > queues. > > > Do not direct RSS traffic to our fallback queue. > > > > > > Signed-off-by: Sara Sharon <sara.sha...@intel.com> > > > Signed-off-by: Emmanuel Grumbach <emmanuel.grumb...@intel.com > > > > > > > > That doesn't really address what I said. Yes, it's using the > > common > > RSS key, but it's not implementing the ethtool operations to get > > and > > set the indirection table and the types of flow hashing that are > > enabled. > > > > Hm.. I think that setting the indirection table is a problem in our > case because the PN check is done in the driver. The PN check is the > way WiFi addresses the replay attack, and since the PN check relies > on > per-cpu variables, we cannot *safely* allow users to modify the > indirection table while traffic is flowing. >
Oh - and yes, we do have a debugfs hook for that. It was meant for the very early testing phases. I guess that we should remove it at some point, unless we decide that whoever wants to play with the indirection table while inbound traffic is flowing puts himself at risk. > > Ben. > >