| Message ID | 20231226205536.32003-1-gerhard@engleder-embedded.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 501869fecfbc00d20d07c1a5f1a49af8fb903d44 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: ethtool: Fix symmetric-xor RSS RX flow hash check | expand |
On 12/27/23 2:25 AM, Gerhard Engleder wrote: > Commit 13e59344fb9d ("net: ethtool: add support for symmetric-xor RSS hash") > adds a check to the ethtool set_rxnfc operation, which checks the RX > flow hash if the flag RXH_XFRM_SYM_XOR is set. This flag is introduced > with the same commit. It calls the ethtool get_rxfh operation to get the > RX flow hash data. If get_rxfh is not supported, then EOPNOTSUPP is > returned. > > There are driver like tsnep, macb, asp2, genet, gianfar, mtk, ... which > support the ethtool operation set_rxnfc but not get_rxfh. This results > in EOPNOTSUPP returned by ethtool_set_rxnfc() without actually calling > the ethtool operation set_rxnfc. Thus, set_rxnfc got broken for all > these drivers. > > Check RX flow hash in ethtool_set_rxnfc() only if driver supports RX > flow hash. > > Fixes: 13e59344fb9d ("net: ethtool: add support for symmetric-xor RSS hash") > Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com> > --- > net/ethtool/ioctl.c | 33 ++++++++++++++++++--------------- > 1 file changed, 18 insertions(+), 15 deletions(-) > > diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c > index 86d47425038b..ec27897d1b24 100644 > --- a/net/ethtool/ioctl.c > +++ b/net/ethtool/ioctl.c > @@ -973,32 +973,35 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev, > u32 cmd, void __user *useraddr) > { > const struct ethtool_ops *ops = dev->ethtool_ops; > - struct ethtool_rxfh_param rxfh = {}; > struct ethtool_rxnfc info; > size_t info_size = sizeof(info); > int rc; > > - if (!ops->set_rxnfc || !ops->get_rxfh) > + if (!ops->set_rxnfc) > return -EOPNOTSUPP; > > rc = ethtool_rxnfc_copy_struct(cmd, &info, &info_size, useraddr); > if (rc) > return rc; > > - rc = ops->get_rxfh(dev, &rxfh); > - if (rc) > - return rc; > + if (ops->get_rxfh) { > + struct ethtool_rxfh_param rxfh = {}; > > - /* Sanity check: if symmetric-xor is set, then: > - * 1 - no other fields besides IP src/dst and/or L4 src/dst > - * 2 - If src is set, dst must also be set > - */ > - if ((rxfh.input_xfrm & RXH_XFRM_SYM_XOR) && > - ((info.data & ~(RXH_IP_SRC | RXH_IP_DST | > - RXH_L4_B_0_1 | RXH_L4_B_2_3)) || > - (!!(info.data & RXH_IP_SRC) ^ !!(info.data & RXH_IP_DST)) || > - (!!(info.data & RXH_L4_B_0_1) ^ !!(info.data & RXH_L4_B_2_3)))) > - return -EINVAL; > + rc = ops->get_rxfh(dev, &rxfh); > + if (rc) > + return rc; > + > + /* Sanity check: if symmetric-xor is set, then: > + * 1 - no other fields besides IP src/dst and/or L4 src/dst > + * 2 - If src is set, dst must also be set > + */ > + if ((rxfh.input_xfrm & RXH_XFRM_SYM_XOR) && > + ((info.data & ~(RXH_IP_SRC | RXH_IP_DST | > + RXH_L4_B_0_1 | RXH_L4_B_2_3)) || > + (!!(info.data & RXH_IP_SRC) ^ !!(info.data & RXH_IP_DST)) || > + (!!(info.data & RXH_L4_B_0_1) ^ !!(info.data & RXH_L4_B_2_3)))) > + return -EINVAL; > + } > > rc = ops->set_rxnfc(dev, &info); > if (rc) Reviewed-by: Ravi Gunasekaran <r-gunasekaran@ti.com>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 26 Dec 2023 21:55:36 +0100 you wrote: > Commit 13e59344fb9d ("net: ethtool: add support for symmetric-xor RSS hash") > adds a check to the ethtool set_rxnfc operation, which checks the RX > flow hash if the flag RXH_XFRM_SYM_XOR is set. This flag is introduced > with the same commit. It calls the ethtool get_rxfh operation to get the > RX flow hash data. If get_rxfh is not supported, then EOPNOTSUPP is > returned. > > [...] Here is the summary with links: - [net-next] net: ethtool: Fix symmetric-xor RSS RX flow hash check https://git.kernel.org/netdev/net-next/c/501869fecfbc You are awesome, thank you!
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 86d47425038b..ec27897d1b24 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -973,32 +973,35 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev, u32 cmd, void __user *useraddr) { const struct ethtool_ops *ops = dev->ethtool_ops; - struct ethtool_rxfh_param rxfh = {}; struct ethtool_rxnfc info; size_t info_size = sizeof(info); int rc; - if (!ops->set_rxnfc || !ops->get_rxfh) + if (!ops->set_rxnfc) return -EOPNOTSUPP; rc = ethtool_rxnfc_copy_struct(cmd, &info, &info_size, useraddr); if (rc) return rc; - rc = ops->get_rxfh(dev, &rxfh); - if (rc) - return rc; + if (ops->get_rxfh) { + struct ethtool_rxfh_param rxfh = {}; - /* Sanity check: if symmetric-xor is set, then: - * 1 - no other fields besides IP src/dst and/or L4 src/dst - * 2 - If src is set, dst must also be set - */ - if ((rxfh.input_xfrm & RXH_XFRM_SYM_XOR) && - ((info.data & ~(RXH_IP_SRC | RXH_IP_DST | - RXH_L4_B_0_1 | RXH_L4_B_2_3)) || - (!!(info.data & RXH_IP_SRC) ^ !!(info.data & RXH_IP_DST)) || - (!!(info.data & RXH_L4_B_0_1) ^ !!(info.data & RXH_L4_B_2_3)))) - return -EINVAL; + rc = ops->get_rxfh(dev, &rxfh); + if (rc) + return rc; + + /* Sanity check: if symmetric-xor is set, then: + * 1 - no other fields besides IP src/dst and/or L4 src/dst + * 2 - If src is set, dst must also be set + */ + if ((rxfh.input_xfrm & RXH_XFRM_SYM_XOR) && + ((info.data & ~(RXH_IP_SRC | RXH_IP_DST | + RXH_L4_B_0_1 | RXH_L4_B_2_3)) || + (!!(info.data & RXH_IP_SRC) ^ !!(info.data & RXH_IP_DST)) || + (!!(info.data & RXH_L4_B_0_1) ^ !!(info.data & RXH_L4_B_2_3)))) + return -EINVAL; + } rc = ops->set_rxnfc(dev, &info); if (rc)
Commit 13e59344fb9d ("net: ethtool: add support for symmetric-xor RSS hash") adds a check to the ethtool set_rxnfc operation, which checks the RX flow hash if the flag RXH_XFRM_SYM_XOR is set. This flag is introduced with the same commit. It calls the ethtool get_rxfh operation to get the RX flow hash data. If get_rxfh is not supported, then EOPNOTSUPP is returned. There are driver like tsnep, macb, asp2, genet, gianfar, mtk, ... which support the ethtool operation set_rxnfc but not get_rxfh. This results in EOPNOTSUPP returned by ethtool_set_rxnfc() without actually calling the ethtool operation set_rxnfc. Thus, set_rxnfc got broken for all these drivers. Check RX flow hash in ethtool_set_rxnfc() only if driver supports RX flow hash. Fixes: 13e59344fb9d ("net: ethtool: add support for symmetric-xor RSS hash") Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com> --- net/ethtool/ioctl.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-)