From: Eric Dumazet <eduma...@google.com> While trying to understand why bnx2x RSS hash was not matching Toeplitz specifications, I implemented driver code to support ethtool -x|-X to try various RSS key combinations easily.
Then, after some debugging, I understood bnx2x was reading the rss_key in reverse order. If we want consistent Toeplitz hashes with different NIC, we need to byte swap user key. Tested: lpk51:~# cat /proc/sys/net/core/netdev_rss_key 61:4a:0d:3e:2e:b1:30:cd:42:f5:0f:9c:a9:71:dd:1c:2f:f1:d6:60:d1:de:b2:b2:1e:bd:3c:ed:bf:84:e3:e3:d5:88:a2:12:e4:f4:4a:f6:1a:10:c0:16:b4:56:4f:aa:a4:fa:0a:b1 lpk51:~# ethtool -x eth1 | tail -2 RSS hash key: 61:4a:0d:3e:2e:b1:30:cd:42:f5:0f:9c:a9:71:dd:1c:2f:f1:d6:60:d1:de:b2:b2:1e:bd:3c:ed:bf:84:e3:e3:d5:88:a2:12:e4:f4:4a:f6 lpk51:~# ethtool -X eth1 hkey 03:0e:e2:43:fa:82:0e:73:14:2d:c0:68:21:9e:82:99:b9:84:d0:22:e2:b3:64:9f:4a:af:00:fa:cc:05:b4:4a:17:05:14:73:76:58:bd:2f lpk51:~# ethtool -x eth1 | tail -2 RSS hash key: 03:0e:e2:43:fa:82:0e:73:14:2d:c0:68:21:9e:82:99:b9:84:d0:22:e2:b3:64:9f:4a:af:00:fa:cc:05:b4:4a:17:05:14:73:76:58:bd:2f Signed-off-by: Eric Dumazet <eduma...@google.com> --- Since rss_key was random, I guess there is no hurry to fix this bug in stable kernels, and/or to split this patch in two parts. drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 7 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 4 drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 77 +++++----- drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c | 11 + drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h | 5 5 files changed, 60 insertions(+), 44 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c index 44173be..5f06267 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c @@ -2073,7 +2073,7 @@ static int bnx2x_init_rss(struct bnx2x *bp) * For 57712 and newer on the other hand it's a per-function * configuration. */ - return bnx2x_config_rss_eth(bp, bp->port.pmf || !CHIP_IS_E1x(bp)); + return bnx2x_config_rss_eth(bp); } int bnx2x_rss(struct bnx2x *bp, struct bnx2x_rss_config_obj *rss_obj, @@ -2122,11 +2122,8 @@ int bnx2x_rss(struct bnx2x *bp, struct bnx2x_rss_config_obj *rss_obj, memcpy(params.ind_table, rss_obj->ind_table, sizeof(params.ind_table)); - if (config_hash) { - /* RSS keys */ - netdev_rss_key_fill(params.rss_key, T_ETH_RSS_KEY * 4); + if (config_hash) __set_bit(BNX2X_RSS_SET_SRCH, ¶ms.rss_flags); - } if (IS_PF(bp)) return bnx2x_config_rss(bp, ¶ms); diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h index b7d32e8..c5d6cce 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h @@ -913,8 +913,10 @@ static inline int func_by_vn(struct bnx2x *bp, int vn) return 2 * vn + BP_PORT(bp); } -static inline int bnx2x_config_rss_eth(struct bnx2x *bp, bool config_hash) +static inline int bnx2x_config_rss_eth(struct bnx2x *bp) { + bool config_hash = bp->port.pmf || !CHIP_IS_E1x(bp); + return bnx2x_rss(bp, &bp->rss_conf_obj, config_hash, true); } diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c index aeb7ce6..0c613fa 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c @@ -3419,6 +3419,11 @@ static u32 bnx2x_get_rxfh_indir_size(struct net_device *dev) return T_ETH_INDIRECTION_TABLE_SIZE; } +static u32 bnx2x_get_rxfh_key_size(struct net_device *dev) +{ + return T_ETH_RSS_KEY * 4; +} + static int bnx2x_get_rxfh(struct net_device *dev, u32 *indir, u8 *key, u8 *hfunc) { @@ -3428,24 +3433,25 @@ static int bnx2x_get_rxfh(struct net_device *dev, u32 *indir, u8 *key, if (hfunc) *hfunc = ETH_RSS_HASH_TOP; - if (!indir) - return 0; + if (indir) { - /* Get the current configuration of the RSS indirection table */ - bnx2x_get_rss_ind_table(&bp->rss_conf_obj, ind_table); - - /* - * We can't use a memcpy() as an internal storage of an - * indirection table is a u8 array while indir->ring_index - * points to an array of u32. - * - * Indirection table contains the FW Client IDs, so we need to - * align the returned table to the Client ID of the leading RSS - * queue. - */ - for (i = 0; i < T_ETH_INDIRECTION_TABLE_SIZE; i++) - indir[i] = ind_table[i] - bp->fp->cl_id; + /* Get the current configuration of the RSS indirection table */ + bnx2x_get_rss_ind_table(&bp->rss_conf_obj, ind_table); + /* + * We can't use a memcpy() as an internal storage of an + * indirection table is a u8 array while indir->ring_index + * points to an array of u32. + * + * Indirection table contains the FW Client IDs, so we need to + * align the returned table to the Client ID of the leading RSS + * queue. + */ + for (i = 0; i < T_ETH_INDIRECTION_TABLE_SIZE; i++) + indir[i] = ind_table[i] - bp->fp->cl_id; + } + if (key) + memcpy(key, bp->rss_conf_obj.rss_key, T_ETH_RSS_KEY * 4); return 0; } @@ -3453,32 +3459,34 @@ static int bnx2x_set_rxfh(struct net_device *dev, const u32 *indir, const u8 *key, const u8 hfunc) { struct bnx2x *bp = netdev_priv(dev); - size_t i; /* We require at least one supported parameter to be changed and no * change in any of the unsupported parameters */ - if (key || - (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)) + if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP) return -EOPNOTSUPP; - if (!indir) - return 0; - - for (i = 0; i < T_ETH_INDIRECTION_TABLE_SIZE; i++) { - /* - * The same as in bnx2x_get_rxfh: we can't use a memcpy() - * as an internal storage of an indirection table is a u8 array - * while indir->ring_index points to an array of u32. - * - * Indirection table contains the FW Client IDs, so we need to - * align the received table to the Client ID of the leading RSS - * queue - */ - bp->rss_conf_obj.ind_table[i] = indir[i] + bp->fp->cl_id; + if (indir) { + size_t i; + + for (i = 0; i < T_ETH_INDIRECTION_TABLE_SIZE; i++) { + /* + * The same as in bnx2x_get_rxfh: we can't use a memcpy() + * as an internal storage of an indirection table is a u8 array + * while indir->ring_index points to an array of u32. + * + * Indirection table contains the FW Client IDs, so we need to + * align the received table to the Client ID of the leading RSS + * queue + */ + bp->rss_conf_obj.ind_table[i] = indir[i] + bp->fp->cl_id; + } } - return bnx2x_config_rss_eth(bp, false); + if (key) + memcpy(bp->rss_conf_obj.rss_key, key, T_ETH_RSS_KEY * 4); + + return bnx2x_config_rss_eth(bp); } /** @@ -3627,6 +3635,7 @@ static const struct ethtool_ops bnx2x_ethtool_ops = { .get_rxnfc = bnx2x_get_rxnfc, .set_rxnfc = bnx2x_set_rxnfc, .get_rxfh_indir_size = bnx2x_get_rxfh_indir_size, + .get_rxfh_key_size = bnx2x_get_rxfh_key_size, .get_rxfh = bnx2x_get_rxfh, .set_rxfh = bnx2x_set_rxfh, .get_channels = bnx2x_get_channels, diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c index c9bd7f1..90d2a2f 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c @@ -4319,8 +4319,14 @@ static int bnx2x_setup_rss(struct bnx2x *bp, /* RSS keys */ if (test_bit(BNX2X_RSS_SET_SRCH, &p->rss_flags)) { - memcpy(&data->rss_key[0], &p->rss_key[0], - sizeof(data->rss_key)); + u8 *dst = (u8 *)(data->rss_key) + sizeof(data->rss_key); + const u8 *src = (const u8 *)bp->rss_conf_obj.rss_key; + int i; + + /* Apparently, bnx2x reads this array in reverse order */ + for (i = 0; i < sizeof(data->rss_key); i++) + *--dst = *src++; + caps |= ETH_RSS_UPDATE_RAMROD_DATA_UPDATE_RSS_KEY; } @@ -4408,6 +4414,7 @@ void bnx2x_init_rss_config_obj(struct bnx2x *bp, bnx2x_init_raw_obj(&rss_obj->raw, cl_id, cid, func_id, rdata, rdata_mapping, state, pstate, type); + netdev_rss_key_fill(rss_obj->rss_key, sizeof(rss_obj->rss_key)); rss_obj->engine_id = engine_id; rss_obj->config_rss = bnx2x_setup_rss; } diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h index 4048fc5..6431979 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h @@ -734,8 +734,6 @@ struct bnx2x_config_rss_params { /* Indirection table */ u8 ind_table[T_ETH_INDIRECTION_TABLE_SIZE]; - /* RSS hash values */ - u32 rss_key[10]; /* valid only iff BNX2X_RSS_UPDATE_TOE is set */ u16 toe_rss_bitmap; @@ -754,6 +752,9 @@ struct bnx2x_rss_config_obj { u8 udp_rss_v4; u8 udp_rss_v6; + /* RSS hash values */ + u32 rss_key[T_ETH_RSS_KEY]; + int (*config_rss)(struct bnx2x *bp, struct bnx2x_config_rss_params *p); }; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html