| Message ID | 20250203150039.519301-2-gal@nvidia.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Symmetric OR-XOR RSS hash | expand |
On 03/02/2025 15:00, Gal Pressman wrote: > Add an additional type of symmetric RSS hash type: OR-XOR. > The "Symmetric-OR-XOR" algorithm transforms the input as follows: > > (SRC_IP | DST_IP, SRC_IP ^ DST_IP, SRC_PORT | DST_PORT, SRC_PORT ^ DST_PORT) > > Change 'cap_rss_sym_xor_supported' to 'supported_input_xfrm', a bitmap > of supported RXH_XFRM_* types. > > Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> > Reviewed-by: Tariq Toukan <tariqt@nvidia.com> > Signed-off-by: Gal Pressman <gal@nvidia.com> > --- > Documentation/networking/ethtool-netlink.rst | 2 +- > Documentation/networking/scaling.rst | 14 ++++++++++---- > drivers/net/ethernet/intel/iavf/iavf_ethtool.c | 2 +- > drivers/net/ethernet/intel/ice/ice_ethtool.c | 2 +- > include/linux/ethtool.h | 5 ++--- > include/uapi/linux/ethtool.h | 7 ++++--- > net/ethtool/ioctl.c | 8 ++++---- > 7 files changed, 23 insertions(+), 17 deletions(-) > > diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst > index 3770a2294509..aba83d97ff90 100644 > --- a/Documentation/networking/ethtool-netlink.rst > +++ b/Documentation/networking/ethtool-netlink.rst > @@ -1934,7 +1934,7 @@ ETHTOOL_A_RSS_INDIR attribute returns RSS indirection table where each byte > indicates queue number. > ETHTOOL_A_RSS_INPUT_XFRM attribute is a bitmap indicating the type of > transformation applied to the input protocol fields before given to the RSS > -hfunc. Current supported option is symmetric-xor. > +hfunc. Current supported option is symmetric-xor and symmetric-or-xor. "options are"? > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h > index d1089b88efc7..b10ecc503b26 100644 > --- a/include/uapi/linux/ethtool.h > +++ b/include/uapi/linux/ethtool.h > @@ -2263,12 +2263,13 @@ static inline int ethtool_validate_duplex(__u8 duplex) > #define WOL_MODE_COUNT 8 > > /* RSS hash function data > - * XOR the corresponding source and destination fields of each specified > - * protocol. Both copies of the XOR'ed fields are fed into the RSS and RXHASH > - * calculation. Note that this XORing reduces the input set entropy and could > + * XOR/OR the corresponding source and destination fields of each specified > + * protocol. Both copies of the XOR/OR'ed fields are fed into the RSS and RXHASH > + * calculation. Note that this operation reduces the input set entropy and could > * be exploited to reduce the RSS queue spread. > */ > #define RXH_XFRM_SYM_XOR (1 << 0) > +#define RXH_XFRM_SYM_OR_XOR (1 << 1) > #define RXH_XFRM_NO_CHANGE 0xff I think this should be two separate comments, one on RXH_XFRM_SYM_XOR and one on RXH_XFRM_SYM_OR_XOR, so that you can untangle the phrasing a bit. E.g. there isn't such a thing as "Both copies of the XOR/OR'ed fields"; one has two copies of XOR and the other has a XOR and an OR. Second comment could be something like "Similar to SYM_XOR except that one copy of the XOR'ed fields is replaced by an OR of the same fields." Apart from this, patch LGTM.
On 03/02/2025 21:15, Edward Cree wrote: > On 03/02/2025 15:00, Gal Pressman wrote: >> Add an additional type of symmetric RSS hash type: OR-XOR. >> The "Symmetric-OR-XOR" algorithm transforms the input as follows: >> >> (SRC_IP | DST_IP, SRC_IP ^ DST_IP, SRC_PORT | DST_PORT, SRC_PORT ^ DST_PORT) >> >> Change 'cap_rss_sym_xor_supported' to 'supported_input_xfrm', a bitmap >> of supported RXH_XFRM_* types. >> >> Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com> >> Reviewed-by: Tariq Toukan <tariqt@nvidia.com> >> Signed-off-by: Gal Pressman <gal@nvidia.com> >> --- >> Documentation/networking/ethtool-netlink.rst | 2 +- >> Documentation/networking/scaling.rst | 14 ++++++++++---- >> drivers/net/ethernet/intel/iavf/iavf_ethtool.c | 2 +- >> drivers/net/ethernet/intel/ice/ice_ethtool.c | 2 +- >> include/linux/ethtool.h | 5 ++--- >> include/uapi/linux/ethtool.h | 7 ++++--- >> net/ethtool/ioctl.c | 8 ++++---- >> 7 files changed, 23 insertions(+), 17 deletions(-) >> >> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst >> index 3770a2294509..aba83d97ff90 100644 >> --- a/Documentation/networking/ethtool-netlink.rst >> +++ b/Documentation/networking/ethtool-netlink.rst >> @@ -1934,7 +1934,7 @@ ETHTOOL_A_RSS_INDIR attribute returns RSS indirection table where each byte >> indicates queue number. >> ETHTOOL_A_RSS_INPUT_XFRM attribute is a bitmap indicating the type of >> transformation applied to the input protocol fields before given to the RSS >> -hfunc. Current supported option is symmetric-xor. >> +hfunc. Current supported option is symmetric-xor and symmetric-or-xor. > > "options are"? Yes. > >> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h >> index d1089b88efc7..b10ecc503b26 100644 >> --- a/include/uapi/linux/ethtool.h >> +++ b/include/uapi/linux/ethtool.h >> @@ -2263,12 +2263,13 @@ static inline int ethtool_validate_duplex(__u8 duplex) >> #define WOL_MODE_COUNT 8 >> >> /* RSS hash function data >> - * XOR the corresponding source and destination fields of each specified >> - * protocol. Both copies of the XOR'ed fields are fed into the RSS and RXHASH >> - * calculation. Note that this XORing reduces the input set entropy and could >> + * XOR/OR the corresponding source and destination fields of each specified >> + * protocol. Both copies of the XOR/OR'ed fields are fed into the RSS and RXHASH >> + * calculation. Note that this operation reduces the input set entropy and could >> * be exploited to reduce the RSS queue spread. >> */ >> #define RXH_XFRM_SYM_XOR (1 << 0) >> +#define RXH_XFRM_SYM_OR_XOR (1 << 1) >> #define RXH_XFRM_NO_CHANGE 0xff > > I think this should be two separate comments, one on RXH_XFRM_SYM_XOR and > one on RXH_XFRM_SYM_OR_XOR, so that you can untangle the phrasing a bit. > E.g. there isn't such a thing as "Both copies of the XOR/OR'ed fields"; one > has two copies of XOR and the other has a XOR and an OR. > Second comment could be something like "Similar to SYM_XOR except that one > copy of the XOR'ed fields is replaced by an OR of the same fields." Will change. > > Apart from this, patch LGTM. Thanks for the review!
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst index 3770a2294509..aba83d97ff90 100644 --- a/Documentation/networking/ethtool-netlink.rst +++ b/Documentation/networking/ethtool-netlink.rst @@ -1934,7 +1934,7 @@ ETHTOOL_A_RSS_INDIR attribute returns RSS indirection table where each byte indicates queue number. ETHTOOL_A_RSS_INPUT_XFRM attribute is a bitmap indicating the type of transformation applied to the input protocol fields before given to the RSS -hfunc. Current supported option is symmetric-xor. +hfunc. Current supported option is symmetric-xor and symmetric-or-xor. PLCA_GET_CFG ============ diff --git a/Documentation/networking/scaling.rst b/Documentation/networking/scaling.rst index 4eb50bcb9d42..d8971ce07628 100644 --- a/Documentation/networking/scaling.rst +++ b/Documentation/networking/scaling.rst @@ -49,14 +49,20 @@ destination address) and TCP/UDP (source port, destination port) tuples are swapped, the computed hash is the same. This is beneficial in some applications that monitor TCP/IP flows (IDS, firewalls, ...etc) and need both directions of the flow to land on the same Rx queue (and CPU). The -"Symmetric-XOR" is a type of RSS algorithms that achieves this hash -symmetry by XORing the input source and destination fields of the IP -and/or L4 protocols. This, however, results in reduced input entropy and -could potentially be exploited. Specifically, the algorithm XORs the input +"Symmetric-XOR" and "Symmetric-OR-XOR" are types of RSS algorithms that +achieve this hash symmetry by XOR/ORing the input source and destination +fields of the IP and/or L4 protocols. This, however, results in reduced +input entropy and could potentially be exploited. + +Specifically, the "Symmetric-XOR" algorithm XORs the input as follows:: # (SRC_IP ^ DST_IP, SRC_IP ^ DST_IP, SRC_PORT ^ DST_PORT, SRC_PORT ^ DST_PORT) +The "Symmetric-OR-XOR" algorithm transforms the input as follows:: + + # (SRC_IP | DST_IP, SRC_IP ^ DST_IP, SRC_PORT | DST_PORT, SRC_PORT ^ DST_PORT) + The result is then fed to the underlying RSS algorithm. Some advanced NICs allow steering packets to queues based on diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c index 74a1e9fe1821..288bb5b2e72e 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c @@ -1808,7 +1808,7 @@ static int iavf_set_rxfh(struct net_device *netdev, static const struct ethtool_ops iavf_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_USE_ADAPTIVE, - .cap_rss_sym_xor_supported = true, + .supported_input_xfrm = RXH_XFRM_SYM_XOR, .get_drvinfo = iavf_get_drvinfo, .get_link = ethtool_op_get_link, .get_ringparam = iavf_get_ringparam, diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c index 3072634bf049..a02ce2cea852 100644 --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c @@ -4774,7 +4774,7 @@ static const struct ethtool_ops ice_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_USE_ADAPTIVE | ETHTOOL_COALESCE_RX_USECS_HIGH, - .cap_rss_sym_xor_supported = true, + .supported_input_xfrm = RXH_XFRM_SYM_XOR, .rxfh_per_ctx_key = true, .get_link_ksettings = ice_get_link_ksettings, .set_link_ksettings = ice_set_link_ksettings, diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 64301ddf2f59..846cf5a34c67 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -768,8 +768,7 @@ struct kernel_ethtool_ts_info { * @cap_rss_ctx_supported: indicates if the driver supports RSS * contexts via legacy API, drivers implementing @create_rxfh_context * do not have to set this bit. - * @cap_rss_sym_xor_supported: indicates if the driver supports symmetric-xor - * RSS. + * @supported_input_xfrm: supported types of input xfrm from %RXH_XFRM_*. * @rxfh_per_ctx_key: device supports setting different RSS key for each * additional context. Netlink API should report hfunc, key, and input_xfrm * for every context, not just context 0. @@ -997,7 +996,7 @@ struct kernel_ethtool_ts_info { struct ethtool_ops { u32 cap_link_lanes_supported:1; u32 cap_rss_ctx_supported:1; - u32 cap_rss_sym_xor_supported:1; + u32 supported_input_xfrm:8; u32 rxfh_per_ctx_key:1; u32 cap_rss_rxnfc_adds:1; u32 rxfh_indir_space; diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index d1089b88efc7..b10ecc503b26 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -2263,12 +2263,13 @@ static inline int ethtool_validate_duplex(__u8 duplex) #define WOL_MODE_COUNT 8 /* RSS hash function data - * XOR the corresponding source and destination fields of each specified - * protocol. Both copies of the XOR'ed fields are fed into the RSS and RXHASH - * calculation. Note that this XORing reduces the input set entropy and could + * XOR/OR the corresponding source and destination fields of each specified + * protocol. Both copies of the XOR/OR'ed fields are fed into the RSS and RXHASH + * calculation. Note that this operation reduces the input set entropy and could * be exploited to reduce the RSS queue spread. */ #define RXH_XFRM_SYM_XOR (1 << 0) +#define RXH_XFRM_SYM_OR_XOR (1 << 1) #define RXH_XFRM_NO_CHANGE 0xff /* L2-L4 network traffic flow types */ diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 7bb94875a7ec..3829e24f9c4c 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -1005,11 +1005,11 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev, if (rc) return rc; - /* Sanity check: if symmetric-xor is set, then: + /* Sanity check: if symmetric-xor/symmetric-or-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) && + if ((rxfh.input_xfrm & (RXH_XFRM_SYM_XOR | RXH_XFRM_SYM_OR_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)) || @@ -1382,11 +1382,11 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev, return -EOPNOTSUPP; /* Check input data transformation capabilities */ if (rxfh.input_xfrm && rxfh.input_xfrm != RXH_XFRM_SYM_XOR && + rxfh.input_xfrm != RXH_XFRM_SYM_OR_XOR && rxfh.input_xfrm != RXH_XFRM_NO_CHANGE) return -EINVAL; if (rxfh.input_xfrm != RXH_XFRM_NO_CHANGE && - (rxfh.input_xfrm & RXH_XFRM_SYM_XOR) && - !ops->cap_rss_sym_xor_supported) + rxfh.input_xfrm & ~ops->supported_input_xfrm) return -EOPNOTSUPP; create = rxfh.rss_context == ETH_RXFH_CONTEXT_ALLOC;