Re: RSS configuration in iwlwifi
On Wed, 2016-04-20 at 18:05 +0100, Ben Hutchings wrote: > > I see. You could make this work when the interface is down and > return -EBUSY if the interface is up. > It's slightly more complicated, since it doesn't just affect a single netdev but possibly more than one (since the table is shared by all virtual interfaces) - but yes, it could be done. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RSS configuration in iwlwifi
On Wed, 2016-04-20 at 16:22 +, Grumbach, Emmanuel wrote: > On Wed, 2016-04-20 at 16:47 +0100, Ben Hutchings wrote: > > > > On Wed, 2016-04-20 at 15:30 +, 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 854d773e4ab586924af4ca5d851730849903 > > > Author: Sara Sharon > > > 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 > > > Signed-off-by: Emmanuel Grumbach > > 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. I see. You could make this work when the interface is down and return -EBUSY if the interface is up. Ben. -- Ben Hutchings Never attribute to conspiracy what can adequately be explained by stupidity. signature.asc Description: This is a digitally signed message part
Re: RSS configuration in iwlwifi
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 +, 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 854d773e4ab586924af4ca5d851730849903 > > > Author: Sara Sharon > > > 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 > > > Signed-off-by: Emmanuel Grumbach > > > > > > > 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. > >
Re: RSS configuration in iwlwifi
On Wed, 2016-04-20 at 16:47 +0100, Ben Hutchings wrote: > On Wed, 2016-04-20 at 15:30 +, 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 854d773e4ab586924af4ca5d851730849903 > > Author: Sara Sharon > > 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 > > Signed-off-by: Emmanuel Grumbach > > 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. > Ben. >
Re: RSS configuration in iwlwifi
On Wed, 2016-04-20 at 15:30 +, 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 854d773e4ab586924af4ca5d851730849903 > Author: Sara Sharon > 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 > Signed-off-by: Emmanuel Grumbach 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. Ben. -- Ben Hutchings Never attribute to conspiracy what can adequately be explained by stupidity. signature.asc Description: This is a digitally signed message part
Re: RSS configuration in iwlwifi
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 854d773e4ab586924af4ca5d851730849903
Author: Sara Sharon
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
Signed-off-by: Emmanuel Grumbach
> Ben.N�r��yb�X��ǧv�^�){.n�+{��*ޕ�,�{ay� ʇڙ�,j��f���h���z� �w���
> ���j:+v���w�j�mzZ+�ݢj"��!�i
