| Message ID | 20230416191223.394805-2-jdamato@fastly.com |
|---|---|
| State | Accepted |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | ixgbe: Multiple RSS bugfixes | expand |
On Sun, Apr 16, 2023 at 07:12:22PM +0000, Joe Damato wrote: > ixgbe currently returns `EINVAL` whenever the flowhash it set by ethtool > because the ethtool code in the kernel passes a non-zero value for hfunc > that ixgbe should allow. > > When ethtool is called with `ETHTOOL_SRXFHINDIR`, > `ethtool_set_rxfh_indir` will call ixgbe's set_rxfh function > with `ETH_RSS_HASH_NO_CHANGE`. This value should be accepted. > > When ethtool is called with `ETHTOOL_SRSSH`, `ethtool_set_rxfh` will > call ixgbe's set_rxfh function with `rxfh.hfunc`, which appears to be > hardcoded in ixgbe to always be `ETH_RSS_HASH_TOP`. This value should > also be accepted. > > Before this patch: > > $ sudo ethtool -L eth1 combined 10 > $ sudo ethtool -X eth1 default > Cannot set RX flow hash configuration: Invalid argument > > After this patch: > > $ sudo ethtool -L eth1 combined 10 > $ sudo ethtool -X eth1 default > $ sudo ethtool -x eth1 > RX flow hash indirection table for eth1 with 10 RX ring(s): > 0: 0 1 2 3 4 5 6 7 > 8: 8 9 0 1 2 3 4 5 > 16: 6 7 8 9 0 1 2 3 > 24: 4 5 6 7 8 9 0 1 > ... > Sorry for the noise, forgot the fixes tag. Fixes: 1c7cf0784e4d ("ixgbe: support for ethtool set_rxfh") > Signed-off-by: Joe Damato <jdamato@fastly.com> > Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c > index 6cfc9dc16537..821dfd323fa9 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c > @@ -3131,8 +3131,8 @@ static int ixgbe_set_rxfh(struct net_device *netdev, const u32 *indir, > int i; > u32 reta_entries = ixgbe_rss_indir_tbl_entries(adapter); > > - if (hfunc) > - return -EINVAL; > + if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP) > + return -EOPNOTSUPP; > > /* Fill out the redirection table */ > if (indir) { > -- > 2.25.1 >
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Joe Damato > Sent: Monday, April 17, 2023 10:28 AM > To: intel-wired-lan@lists.osuosl.org > Cc: netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Brandeburg, Jesse <jesse.brandeburg@intel.com>; kuba@kernel.org > Subject: Re: [Intel-wired-lan] [PATCH net v2 1/2] ixgbe: Allow flow hash to be set via ethtool > > On Sun, Apr 16, 2023 at 07:12:22PM +0000, Joe Damato wrote: > ixgbe currently returns `EINVAL` whenever the flowhash it set by > ethtool because the ethtool code in the kernel passes a non-zero value > for hfunc that ixgbe should allow. > > When ethtool is called with `ETHTOOL_SRXFHINDIR`, > `ethtool_set_rxfh_indir` will call ixgbe's set_rxfh function with > `ETH_RSS_HASH_NO_CHANGE`. This value should be accepted. > > When ethtool is called with `ETHTOOL_SRSSH`, `ethtool_set_rxfh` will > call ixgbe's set_rxfh function with `rxfh.hfunc`, which appears to be > hardcoded in ixgbe to always be `ETH_RSS_HASH_TOP`. This value should > also be accepted. > > Before this patch: > > $ sudo ethtool -L eth1 combined 10 > $ sudo ethtool -X eth1 default > Cannot set RX flow hash configuration: Invalid argument > > After this patch: > > $ sudo ethtool -L eth1 combined 10 > $ sudo ethtool -X eth1 default > $ sudo ethtool -x eth1 > RX flow hash indirection table for eth1 with 10 RX ring(s): > 0: 0 1 2 3 4 5 6 7 > 8: 8 9 0 1 2 3 4 5 > 16: 6 7 8 9 0 1 2 3 > 24: 4 5 6 7 8 9 0 1 > ... > > > Sorry for the noise, forgot the fixes tag. > > Fixes: 1c7cf0784e4d ("ixgbe: support for ethtool set_rxfh") > > Signed-off-by: Joe Damato <jdamato@fastly.com> > Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c index 6cfc9dc16537..821dfd323fa9 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c @@ -3131,8 +3131,8 @@ static int ixgbe_set_rxfh(struct net_device *netdev, const u32 *indir, int i; u32 reta_entries = ixgbe_rss_indir_tbl_entries(adapter); - if (hfunc) - return -EINVAL; + if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP) + return -EOPNOTSUPP; /* Fill out the redirection table */ if (indir) {
ixgbe currently returns `EINVAL` whenever the flowhash it set by ethtool because the ethtool code in the kernel passes a non-zero value for hfunc that ixgbe should allow. When ethtool is called with `ETHTOOL_SRXFHINDIR`, `ethtool_set_rxfh_indir` will call ixgbe's set_rxfh function with `ETH_RSS_HASH_NO_CHANGE`. This value should be accepted. When ethtool is called with `ETHTOOL_SRSSH`, `ethtool_set_rxfh` will call ixgbe's set_rxfh function with `rxfh.hfunc`, which appears to be hardcoded in ixgbe to always be `ETH_RSS_HASH_TOP`. This value should also be accepted. Before this patch: $ sudo ethtool -L eth1 combined 10 $ sudo ethtool -X eth1 default Cannot set RX flow hash configuration: Invalid argument After this patch: $ sudo ethtool -L eth1 combined 10 $ sudo ethtool -X eth1 default $ sudo ethtool -x eth1 RX flow hash indirection table for eth1 with 10 RX ring(s): 0: 0 1 2 3 4 5 6 7 8: 8 9 0 1 2 3 4 5 16: 6 7 8 9 0 1 2 3 24: 4 5 6 7 8 9 0 1 ... Signed-off-by: Joe Damato <jdamato@fastly.com> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)