ethtool: rss: don't report key if device doesn't support it

marvell/otx2 and mvpp2 do not support setting different
keys for different RSS contexts. Contexts have separate
indirection tables but key is shared with all other contexts.
This is likely fine, indirection table is the most important
piece.

Don't report the key-related parameters from such drivers.
This prevents driver-errors, e.g. otx2 always writes
the main key, even when user asks to change per-context key.
The second reason is that without this change tracking
the keys by the core gets complicated. Even if the driver
correctly reject setting key with rss_context != 0,
change of the main key would have to be reflected in
the XArray for all additional contexts.

Since the additional contexts don't have their own keys
not including the attributes (in Netlink speak) seems
intuitive. ethtool CLI seems to deal with it just fine.

Having to set the flag in majority of the drivers is
a bit tedious but not reporting the key is a safer
default.

Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>
Reviewed-by: Joe Damato <jdamato@fastly.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 44076e4ebf1a7a3cbac580b03849ade2b8935995..61d6afc3cacb738938855365474e82c2646d19a2 100644 (file)
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -5289,6 +5289,7 @@ void bnxt_ethtool_free(struct bnxt *bp)
 
 const struct ethtool_ops bnxt_ethtool_ops = {
        .cap_link_lanes_supported       = 1,
+       .rxfh_per_ctx_key               = 1,
        .rxfh_max_num_contexts          = BNXT_MAX_ETH_RSS_CTX + 1,
        .rxfh_indir_space               = BNXT_MAX_RSS_TABLE_ENTRIES_P5,
        .rxfh_priv_size                 = sizeof(struct bnxt_rss_ctx),

diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index bc79ba974e4958e171b5f708578fff5e1911a37c..c7641d326ba547ffa740aba331549101df9611f6 100644 (file)
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -4725,6 +4725,7 @@ static const struct ethtool_ops ice_ethtool_ops = {
                                     ETHTOOL_COALESCE_USE_ADAPTIVE |
                                     ETHTOOL_COALESCE_RX_USECS_HIGH,
        .cap_rss_sym_xor_supported = true,
+       .rxfh_per_ctx_key       = true,
        .get_link_ksettings     = ice_get_link_ksettings,
        .set_link_ksettings     = ice_set_link_ksettings,
        .get_fec_stats          = ice_get_fec_stats,

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 8af509397f6302ec185a40e114372d393940d904..4d123dae912c2df26aad1d25e5a0d6c28e1cb7d0 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -2595,6 +2595,7 @@ static void mlx5e_get_ts_stats(struct net_device *netdev,
 
 const struct ethtool_ops mlx5e_ethtool_ops = {
        .cap_rss_ctx_supported  = true,
+       .rxfh_per_ctx_key       = true,
        .supported_coalesce_params = ETHTOOL_COALESCE_USECS |
                                     ETHTOOL_COALESCE_MAX_FRAMES |
                                     ETHTOOL_COALESCE_USE_ADAPTIVE |

diff --git a/drivers/net/ethernet/sfc/ef100_ethtool.c b/drivers/net/ethernet/sfc/ef100_ethtool.c
index 746b5314acb54a33c6a7a969b153a8277be138ff..5c2551369812cb84720e777477d2774f8068e81c 100644 (file)
--- a/drivers/net/ethernet/sfc/ef100_ethtool.c
+++ b/drivers/net/ethernet/sfc/ef100_ethtool.c
@@ -58,6 +58,7 @@ const struct ethtool_ops ef100_ethtool_ops = {
 
        .get_rxfh_indir_size    = efx_ethtool_get_rxfh_indir_size,
        .get_rxfh_key_size      = efx_ethtool_get_rxfh_key_size,
+       .rxfh_per_ctx_key       = true,
        .rxfh_priv_size         = sizeof(struct efx_rss_context_priv),
        .get_rxfh               = efx_ethtool_get_rxfh,
        .set_rxfh               = efx_ethtool_set_rxfh,

diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
index 15245720c9496a5b548d5be4362ad032a7a3b8fe..848b1923133a747b1218a607a79cb1544ca1d945 100644 (file)
--- a/drivers/net/ethernet/sfc/ethtool.c
+++ b/drivers/net/ethernet/sfc/ethtool.c
@@ -267,6 +267,7 @@ const struct ethtool_ops efx_ethtool_ops = {
        .set_rxnfc              = efx_ethtool_set_rxnfc,
        .get_rxfh_indir_size    = efx_ethtool_get_rxfh_indir_size,
        .get_rxfh_key_size      = efx_ethtool_get_rxfh_key_size,
+       .rxfh_per_ctx_key       = true,
        .rxfh_priv_size         = sizeof(struct efx_rss_context_priv),
        .get_rxfh               = efx_ethtool_get_rxfh,
        .set_rxfh               = efx_ethtool_set_rxfh,

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index a149485904d8fad92364f88eab62fb9a84827b46..12f6dc5675987be79baed3517ac0d255fb3ce4cc 100644 (file)
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -731,6 +731,9 @@ struct kernel_ethtool_ts_info {
  *     do not have to set this bit.
  * @cap_rss_sym_xor_supported: indicates if the driver supports symmetric-xor
  *     RSS.
+ * @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.
  * @rxfh_indir_space: max size of RSS indirection tables, if indirection table
  *     size as returned by @get_rxfh_indir_size may change during lifetime
  *     of the device. Leave as 0 if the table size is constant.
@@ -952,6 +955,7 @@ struct ethtool_ops {
        u32     cap_link_lanes_supported:1;
        u32     cap_rss_ctx_supported:1;
        u32     cap_rss_sym_xor_supported:1;
+       u32     rxfh_per_ctx_key:1;
        u32     rxfh_indir_space;
        u16     rxfh_key_space;
        u16     rxfh_priv_size;

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 1698b73812ceeb324a8969d03ace587db4bd955b..18cf9fa32ae3df576a4d58d5cfaa70e10a9cab9e 100644 (file)
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1261,10 +1261,15 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
                if (rxfh_dev.indir)
                        memcpy(rxfh_dev.indir, ethtool_rxfh_context_indir(ctx),
                               indir_bytes);
-               if (rxfh_dev.key)
-                       memcpy(rxfh_dev.key, ethtool_rxfh_context_key(ctx),
-                              user_key_size);
-               rxfh_dev.hfunc = ctx->hfunc;
+               if (!ops->rxfh_per_ctx_key) {
+                       rxfh_dev.key_size = 0;
+               } else {
+                       if (rxfh_dev.key)
+                               memcpy(rxfh_dev.key,
+                                      ethtool_rxfh_context_key(ctx),
+                                      user_key_size);
+                       rxfh_dev.hfunc = ctx->hfunc;
+               }
                rxfh_dev.input_xfrm = ctx->input_xfrm;
                ret = 0;
        } else {
@@ -1281,6 +1286,11 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
                                &rxfh_dev.input_xfrm,
                                sizeof(rxfh.input_xfrm))) {
                ret = -EFAULT;
+       } else if (copy_to_user(useraddr +
+                               offsetof(struct ethtool_rxfh, key_size),
+                               &rxfh_dev.key_size,
+                               sizeof(rxfh.key_size))) {
+               ret = -EFAULT;
        } else if (copy_to_user(useraddr +
                              offsetof(struct ethtool_rxfh, rss_config[0]),
                              rss_config, total_size)) {
@@ -1389,6 +1399,13 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 
        indir_bytes = dev_indir_size * sizeof(rxfh_dev.indir[0]);
 
+       /* Check settings which may be global rather than per RSS-context */
+       if (rxfh.rss_context && !ops->rxfh_per_ctx_key)
+               if (rxfh.key_size ||
+                   (rxfh.hfunc && rxfh.hfunc != ETH_RSS_HASH_NO_CHANGE) ||
+                   (rxfh.input_xfrm && rxfh.input_xfrm != RXH_XFRM_NO_CHANGE))
+                       return -EOPNOTSUPP;
+
        rss_config = kzalloc(indir_bytes + dev_key_size, GFP_USER);
        if (!rss_config)
                return -ENOMEM;

diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c
index a06bdac8b8a2a2659f75f0b74d5c82f6102e3bbb..cd8100d8191963db7e07d7c7e023f8444f01dc56 100644 (file)
--- a/net/ethtool/rss.c
+++ b/net/ethtool/rss.c
@@ -10,6 +10,7 @@ struct rss_req_info {
 
 struct rss_reply_data {
        struct ethnl_reply_data         base;
+       bool                            no_key_fields;
        u32                             indir_size;
        u32                             hkey_size;
        u32                             hfunc;
@@ -60,9 +61,12 @@ rss_prepare_data(const struct ethnl_req_info *req_base,
                return -EOPNOTSUPP;
 
        /* Some drivers don't handle rss_context */
-       if (request->rss_context && !(ops->cap_rss_ctx_supported ||
-                                     ops->create_rxfh_context))
-               return -EOPNOTSUPP;
+       if (request->rss_context) {
+               if (!ops->cap_rss_ctx_supported && !ops->create_rxfh_context)
+                       return -EOPNOTSUPP;
+
+               data->no_key_fields = !ops->rxfh_per_ctx_key;
+       }
 
        ret = ethnl_ops_begin(dev);
        if (ret < 0)
@@ -132,13 +136,18 @@ rss_fill_reply(struct sk_buff *skb, const struct ethnl_req_info *req_base,
            nla_put_u32(skb, ETHTOOL_A_RSS_CONTEXT, request->rss_context))
                return -EMSGSIZE;
 
+       if ((data->indir_size &&
+            nla_put(skb, ETHTOOL_A_RSS_INDIR,
+                    sizeof(u32) * data->indir_size, data->indir_table)))
+               return -EMSGSIZE;
+
+       if (data->no_key_fields)
+               return 0;
+
        if ((data->hfunc &&
             nla_put_u32(skb, ETHTOOL_A_RSS_HFUNC, data->hfunc)) ||
            (data->input_xfrm &&
             nla_put_u32(skb, ETHTOOL_A_RSS_INPUT_XFRM, data->input_xfrm)) ||
-           (data->indir_size &&
-            nla_put(skb, ETHTOOL_A_RSS_INDIR,
-                    sizeof(u32) * data->indir_size, data->indir_table)) ||
            (data->hkey_size &&
             nla_put(skb, ETHTOOL_A_RSS_HKEY, data->hkey_size, data->hkey)))
                return -EMSGSIZE;