[pull request][net 00/12] mlx5 fixes 2021-01-26

[pull request][net 00/12] mlx5 fixes 2021-01-26

[Saeed Mahameed]

Hi Jakub, Dave

This series introduces some fixes to mlx5 driver.
Please pull and let me know if there is any problem.

Thanks,
Saeed.

---
The following changes since commit c5e9e8d48acdf3b863282af7f6f6931d39526245:

  Merge tag 'mac80211-for-net-2021-01-26' of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211 (2021-01-26 15:23:18 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git tags/mlx5-fixes-2021-01-26

for you to fetch changes up to e2194a1744e8594e82a861687808c1adca419b85:

  net/mlx5: CT: Fix incorrect removal of tuple_nat_node from nat rhashtable (2021-01-26 15:39:04 -0800)

----------------------------------------------------------------
mlx5-fixes-2021-01-26

----------------------------------------------------------------
Daniel Jurgens (1):
      net/mlx5: Maintain separate page trees for ECPF and PF functions

Maor Dickman (2):
      net/mlx5e: Reduce tc unsupported key print level
      net/mlx5e: Disable hw-tc-offload when MLX5_CLS_ACT config is disabled

Maxim Mikityanskiy (4):
      net/mlx5e: Fix IPSEC stats
      net/mlx5e: Correctly handle changing the number of queues when the interface is down
      net/mlx5e: Revert parameters on errors when changing trust state without reset
      net/mlx5e: Revert parameters on errors when changing MTU and LRO state without reset

Pan Bian (1):
      net/mlx5e: free page before return

Parav Pandit (1):
      net/mlx5e: E-switch, Fix rate calculation for overflow

Paul Blakey (2):
      net/mlx5e: Fix CT rule + encap slow path offload and deletion
      net/mlx5: CT: Fix incorrect removal of tuple_nat_node from nat rhashtable

Roi Dayan (1):
      net/mlx5: Fix memory leak on flow table creation error flow

 .../net/ethernet/mellanox/mlx5/core/en/health.c    |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c | 20 +++++---
 .../mellanox/mlx5/core/en_accel/ipsec_stats.c      |  4 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c | 13 +++--
 .../net/ethernet/mellanox/mlx5/core/en_ethtool.c   |  8 ++-
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  | 39 +++++++++++----
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c   |  2 +
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c    | 22 ++++++--
 drivers/net/ethernet/mellanox/mlx5/core/fs_core.c  |  1 +
 .../net/ethernet/mellanox/mlx5/core/pagealloc.c    | 58 +++++++++++++---------
 10 files changed, 114 insertions(+), 55 deletions(-)

[net 01/12] net/mlx5: Fix memory leak on flow table creation error flow

[Saeed Mahameed]

From: Roi Dayan <roid@nvidia.com>

When we create the ft object we also init rhltable in ft->fgs_hash.
So in error flow before kfree of ft we need to destroy that rhltable.

Fixes: 693c6883bbc4 ("net/mlx5: Add hash table for flow groups in flow table")
Signed-off-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Maor Dickman <maord@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
index b899539a0786..0fcee702b808 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
@@ -1141,6 +1141,7 @@ static struct mlx5_flow_table *__mlx5_create_flow_table(struct mlx5_flow_namespa
 destroy_ft:
 	root->cmds->destroy_flow_table(root, ft);
 free_ft:
+	rhltable_destroy(&ft->fgs_hash);
 	kfree(ft);
 unlock_root:
 	mutex_unlock(&root->chain_lock);
-- 
2.29.2

[net 02/12] net/mlx5e: E-switch, Fix rate calculation for overflow

[Saeed Mahameed]

From: Parav Pandit <parav@nvidia.com>

rate_bytes_ps is a 64-bit field. It passed as 32-bit field to
apply_police_params(). Due to this when police rate is higher
than 4Gbps, 32-bit calculation ignores the carry. This results
in incorrect rate configurationn the device.

Fix it by performing 64-bit calculation.

Fixes: fcb64c0f5640 ("net/mlx5: E-Switch, add ingress rate support")
Signed-off-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Eli Cohen <elic@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 4cdf834fa74a..661235027b47 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -67,6 +67,7 @@
 #include "lib/geneve.h"
 #include "lib/fs_chains.h"
 #include "diag/en_tc_tracepoint.h"
+#include <asm/div64.h>
 
 #define nic_chains(priv) ((priv)->fs.tc.chains)
 #define MLX5_MH_ACT_SZ MLX5_UN_SZ_BYTES(set_add_copy_action_in_auto)
@@ -5007,13 +5008,13 @@ int mlx5e_stats_flower(struct net_device *dev, struct mlx5e_priv *priv,
 	return err;
 }
 
-static int apply_police_params(struct mlx5e_priv *priv, u32 rate,
+static int apply_police_params(struct mlx5e_priv *priv, u64 rate,
 			       struct netlink_ext_ack *extack)
 {
 	struct mlx5e_rep_priv *rpriv = priv->ppriv;
 	struct mlx5_eswitch *esw;
+	u32 rate_mbps = 0;
 	u16 vport_num;
-	u32 rate_mbps;
 	int err;
 
 	vport_num = rpriv->rep->vport;
@@ -5030,7 +5031,11 @@ static int apply_police_params(struct mlx5e_priv *priv, u32 rate,
 	 * Moreover, if rate is non zero we choose to configure to a minimum of
 	 * 1 mbit/sec.
 	 */
-	rate_mbps = rate ? max_t(u32, (rate * 8 + 500000) / 1000000, 1) : 0;
+	if (rate) {
+		rate = (rate * BITS_PER_BYTE) + 500000;
+		rate_mbps = max_t(u32, do_div(rate, 1000000), 1);
+	}
+
 	err = mlx5_esw_modify_vport_rate(esw, vport_num, rate_mbps);
 	if (err)
 		NL_SET_ERR_MSG_MOD(extack, "failed applying action to hardware");
-- 
2.29.2

[net 03/12] net/mlx5e: free page before return

[Saeed Mahameed]

From: Pan Bian <bianpan2016@163.com>

Instead of directly return, goto the error handling label to free
allocated page.

Fixes: 5f29458b77d5 ("net/mlx5e: Support dump callback in TX reporter")
Signed-off-by: Pan Bian <bianpan2016@163.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/health.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/health.c b/drivers/net/ethernet/mellanox/mlx5/core/en/health.c
index 718f8c0a4f6b..84e501e057b4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/health.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/health.c
@@ -273,7 +273,7 @@ int mlx5e_health_rsc_fmsg_dump(struct mlx5e_priv *priv, struct mlx5_rsc_key *key
 
 	err = devlink_fmsg_binary_pair_nest_start(fmsg, "data");
 	if (err)
-		return err;
+		goto free_page;
 
 	cmd = mlx5_rsc_dump_cmd_create(mdev, key);
 	if (IS_ERR(cmd)) {
-- 
2.29.2

[net 04/12] net/mlx5e: Reduce tc unsupported key print level

[Saeed Mahameed]

From: Maor Dickman <maord@nvidia.com>

"Unsupported key used:" appears in kernel log when flows with
unsupported key are used, arp fields for example.

OpenVSwitch was changed to match on arp fields by default that
caused this warning to appear in kernel log for every arp rule, which
can be a lot.

Fix by lowering print level from warning to debug.

Fixes: e3a2b7ed018e ("net/mlx5e: Support offload cls_flower with drop action")
Signed-off-by: Maor Dickman <maord@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 661235027b47..f4ce5e208e02 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2270,8 +2270,8 @@ static int __parse_cls_flower(struct mlx5e_priv *priv,
 	      BIT(FLOW_DISSECTOR_KEY_ENC_OPTS) |
 	      BIT(FLOW_DISSECTOR_KEY_MPLS))) {
 		NL_SET_ERR_MSG_MOD(extack, "Unsupported key");
-		netdev_warn(priv->netdev, "Unsupported key used: 0x%x\n",
-			    dissector->used_keys);
+		netdev_dbg(priv->netdev, "Unsupported key used: 0x%x\n",
+			   dissector->used_keys);
 		return -EOPNOTSUPP;
 	}
 
-- 
2.29.2

[net 05/12] net/mlx5e: Fix IPSEC stats

[Saeed Mahameed]

From: Maxim Mikityanskiy <maximmi@mellanox.com>

When IPSEC offload isn't active, the number of stats is not zero, but
the strings are not filled, leading to exposing stats with empty names.
Fix this by using the same condition for NUM_STATS and FILL_STRS.

Fixes: 0aab3e1b04ae ("net/mlx5e: IPSec, Expose IPsec HW stat only for supporting HW")
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Reviewed-by: Raed Salem <raeds@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_accel/ipsec_stats.c    | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_stats.c
index 6c5c54bcd9be..5cb936541b9e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_stats.c
@@ -76,7 +76,7 @@ static const struct counter_desc mlx5e_ipsec_sw_stats_desc[] = {
 
 static MLX5E_DECLARE_STATS_GRP_OP_NUM_STATS(ipsec_sw)
 {
-	return NUM_IPSEC_SW_COUNTERS;
+	return priv->ipsec ? NUM_IPSEC_SW_COUNTERS : 0;
 }
 
 static inline MLX5E_DECLARE_STATS_GRP_OP_UPDATE_STATS(ipsec_sw) {}
@@ -105,7 +105,7 @@ static MLX5E_DECLARE_STATS_GRP_OP_FILL_STATS(ipsec_sw)
 
 static MLX5E_DECLARE_STATS_GRP_OP_NUM_STATS(ipsec_hw)
 {
-	return (mlx5_fpga_ipsec_device_caps(priv->mdev)) ? NUM_IPSEC_HW_COUNTERS : 0;
+	return (priv->ipsec && mlx5_fpga_ipsec_device_caps(priv->mdev)) ? NUM_IPSEC_HW_COUNTERS : 0;
 }
 
 static MLX5E_DECLARE_STATS_GRP_OP_UPDATE_STATS(ipsec_hw)
-- 
2.29.2

[net 06/12] net/mlx5: Maintain separate page trees for ECPF and PF functions

[Saeed Mahameed]

From: Daniel Jurgens <danielj@nvidia.com>

Pages for the host PF and ECPF were stored in the same tree, so the ECPF
pages were being freed along with the host PF's when the host driver
unloaded.

Combine the function ID and ECPF flag to use as an index into the
x-array containing the trees to get a different tree for the host PF and
ECPF.

Fixes: c6168161f693 ("net/mlx5: Add support for release all pages event")
Signed-off-by: Daniel Jurgens <danielj@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/pagealloc.c   | 58 +++++++++++--------
 1 file changed, 34 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
index eb956ce904bc..eaa8958e24d7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
@@ -58,7 +58,7 @@ struct fw_page {
 	struct rb_node		rb_node;
 	u64			addr;
 	struct page	       *page;
-	u16			func_id;
+	u32			function;
 	unsigned long		bitmask;
 	struct list_head	list;
 	unsigned		free_count;
@@ -74,12 +74,17 @@ enum {
 	MLX5_NUM_4K_IN_PAGE		= PAGE_SIZE / MLX5_ADAPTER_PAGE_SIZE,
 };
 
-static struct rb_root *page_root_per_func_id(struct mlx5_core_dev *dev, u16 func_id)
+static u32 get_function(u16 func_id, bool ec_function)
+{
+	return func_id & (ec_function << 16);
+}
+
+static struct rb_root *page_root_per_function(struct mlx5_core_dev *dev, u32 function)
 {
 	struct rb_root *root;
 	int err;
 
-	root = xa_load(&dev->priv.page_root_xa, func_id);
+	root = xa_load(&dev->priv.page_root_xa, function);
 	if (root)
 		return root;
 
@@ -87,7 +92,7 @@ static struct rb_root *page_root_per_func_id(struct mlx5_core_dev *dev, u16 func
 	if (!root)
 		return ERR_PTR(-ENOMEM);
 
-	err = xa_insert(&dev->priv.page_root_xa, func_id, root, GFP_KERNEL);
+	err = xa_insert(&dev->priv.page_root_xa, function, root, GFP_KERNEL);
 	if (err) {
 		kfree(root);
 		return ERR_PTR(err);
@@ -98,7 +103,7 @@ static struct rb_root *page_root_per_func_id(struct mlx5_core_dev *dev, u16 func
 	return root;
 }
 
-static int insert_page(struct mlx5_core_dev *dev, u64 addr, struct page *page, u16 func_id)
+static int insert_page(struct mlx5_core_dev *dev, u64 addr, struct page *page, u32 function)
 {
 	struct rb_node *parent = NULL;
 	struct rb_root *root;
@@ -107,7 +112,7 @@ static int insert_page(struct mlx5_core_dev *dev, u64 addr, struct page *page, u
 	struct fw_page *tfp;
 	int i;
 
-	root = page_root_per_func_id(dev, func_id);
+	root = page_root_per_function(dev, function);
 	if (IS_ERR(root))
 		return PTR_ERR(root);
 
@@ -130,7 +135,7 @@ static int insert_page(struct mlx5_core_dev *dev, u64 addr, struct page *page, u
 
 	nfp->addr = addr;
 	nfp->page = page;
-	nfp->func_id = func_id;
+	nfp->function = function;
 	nfp->free_count = MLX5_NUM_4K_IN_PAGE;
 	for (i = 0; i < MLX5_NUM_4K_IN_PAGE; i++)
 		set_bit(i, &nfp->bitmask);
@@ -143,14 +148,14 @@ static int insert_page(struct mlx5_core_dev *dev, u64 addr, struct page *page, u
 }
 
 static struct fw_page *find_fw_page(struct mlx5_core_dev *dev, u64 addr,
-				    u32 func_id)
+				    u32 function)
 {
 	struct fw_page *result = NULL;
 	struct rb_root *root;
 	struct rb_node *tmp;
 	struct fw_page *tfp;
 
-	root = xa_load(&dev->priv.page_root_xa, func_id);
+	root = xa_load(&dev->priv.page_root_xa, function);
 	if (WARN_ON_ONCE(!root))
 		return NULL;
 
@@ -194,14 +199,14 @@ static int mlx5_cmd_query_pages(struct mlx5_core_dev *dev, u16 *func_id,
 	return err;
 }
 
-static int alloc_4k(struct mlx5_core_dev *dev, u64 *addr, u16 func_id)
+static int alloc_4k(struct mlx5_core_dev *dev, u64 *addr, u32 function)
 {
 	struct fw_page *fp = NULL;
 	struct fw_page *iter;
 	unsigned n;
 
 	list_for_each_entry(iter, &dev->priv.free_list, list) {
-		if (iter->func_id != func_id)
+		if (iter->function != function)
 			continue;
 		fp = iter;
 	}
@@ -231,7 +236,7 @@ static void free_fwp(struct mlx5_core_dev *dev, struct fw_page *fwp,
 {
 	struct rb_root *root;
 
-	root = xa_load(&dev->priv.page_root_xa, fwp->func_id);
+	root = xa_load(&dev->priv.page_root_xa, fwp->function);
 	if (WARN_ON_ONCE(!root))
 		return;
 
@@ -244,12 +249,12 @@ static void free_fwp(struct mlx5_core_dev *dev, struct fw_page *fwp,
 	kfree(fwp);
 }
 
-static void free_4k(struct mlx5_core_dev *dev, u64 addr, u32 func_id)
+static void free_4k(struct mlx5_core_dev *dev, u64 addr, u32 function)
 {
 	struct fw_page *fwp;
 	int n;
 
-	fwp = find_fw_page(dev, addr & MLX5_U64_4K_PAGE_MASK, func_id);
+	fwp = find_fw_page(dev, addr & MLX5_U64_4K_PAGE_MASK, function);
 	if (!fwp) {
 		mlx5_core_warn_rl(dev, "page not found\n");
 		return;
@@ -263,7 +268,7 @@ static void free_4k(struct mlx5_core_dev *dev, u64 addr, u32 func_id)
 		list_add(&fwp->list, &dev->priv.free_list);
 }
 
-static int alloc_system_page(struct mlx5_core_dev *dev, u16 func_id)
+static int alloc_system_page(struct mlx5_core_dev *dev, u32 function)
 {
 	struct device *device = mlx5_core_dma_dev(dev);
 	int nid = dev_to_node(device);
@@ -291,7 +296,7 @@ static int alloc_system_page(struct mlx5_core_dev *dev, u16 func_id)
 		goto map;
 	}
 
-	err = insert_page(dev, addr, page, func_id);
+	err = insert_page(dev, addr, page, function);
 	if (err) {
 		mlx5_core_err(dev, "failed to track allocated page\n");
 		dma_unmap_page(device, addr, PAGE_SIZE, DMA_BIDIRECTIONAL);
@@ -328,6 +333,7 @@ static void page_notify_fail(struct mlx5_core_dev *dev, u16 func_id,
 static int give_pages(struct mlx5_core_dev *dev, u16 func_id, int npages,
 		      int notify_fail, bool ec_function)
 {
+	u32 function = get_function(func_id, ec_function);
 	u32 out[MLX5_ST_SZ_DW(manage_pages_out)] = {0};
 	int inlen = MLX5_ST_SZ_BYTES(manage_pages_in);
 	u64 addr;
@@ -345,10 +351,10 @@ static int give_pages(struct mlx5_core_dev *dev, u16 func_id, int npages,
 
 	for (i = 0; i < npages; i++) {
 retry:
-		err = alloc_4k(dev, &addr, func_id);
+		err = alloc_4k(dev, &addr, function);
 		if (err) {
 			if (err == -ENOMEM)
-				err = alloc_system_page(dev, func_id);
+				err = alloc_system_page(dev, function);
 			if (err)
 				goto out_4k;
 
@@ -384,7 +390,7 @@ static int give_pages(struct mlx5_core_dev *dev, u16 func_id, int npages,
 
 out_4k:
 	for (i--; i >= 0; i--)
-		free_4k(dev, MLX5_GET64(manage_pages_in, in, pas[i]), func_id);
+		free_4k(dev, MLX5_GET64(manage_pages_in, in, pas[i]), function);
 out_free:
 	kvfree(in);
 	if (notify_fail)
@@ -392,14 +398,15 @@ static int give_pages(struct mlx5_core_dev *dev, u16 func_id, int npages,
 	return err;
 }
 
-static void release_all_pages(struct mlx5_core_dev *dev, u32 func_id,
+static void release_all_pages(struct mlx5_core_dev *dev, u16 func_id,
 			      bool ec_function)
 {
+	u32 function = get_function(func_id, ec_function);
 	struct rb_root *root;
 	struct rb_node *p;
 	int npages = 0;
 
-	root = xa_load(&dev->priv.page_root_xa, func_id);
+	root = xa_load(&dev->priv.page_root_xa, function);
 	if (WARN_ON_ONCE(!root))
 		return;
 
@@ -446,6 +453,7 @@ static int reclaim_pages_cmd(struct mlx5_core_dev *dev,
 	struct rb_root *root;
 	struct fw_page *fwp;
 	struct rb_node *p;
+	bool ec_function;
 	u32 func_id;
 	u32 npages;
 	u32 i = 0;
@@ -456,8 +464,9 @@ static int reclaim_pages_cmd(struct mlx5_core_dev *dev,
 	/* No hard feelings, we want our pages back! */
 	npages = MLX5_GET(manage_pages_in, in, input_num_entries);
 	func_id = MLX5_GET(manage_pages_in, in, function_id);
+	ec_function = MLX5_GET(manage_pages_in, in, embedded_cpu_function);
 
-	root = xa_load(&dev->priv.page_root_xa, func_id);
+	root = xa_load(&dev->priv.page_root_xa, get_function(func_id, ec_function));
 	if (WARN_ON_ONCE(!root))
 		return -EEXIST;
 
@@ -473,9 +482,10 @@ static int reclaim_pages_cmd(struct mlx5_core_dev *dev,
 	return 0;
 }
 
-static int reclaim_pages(struct mlx5_core_dev *dev, u32 func_id, int npages,
+static int reclaim_pages(struct mlx5_core_dev *dev, u16 func_id, int npages,
 			 int *nclaimed, bool ec_function)
 {
+	u32 function = get_function(func_id, ec_function);
 	int outlen = MLX5_ST_SZ_BYTES(manage_pages_out);
 	u32 in[MLX5_ST_SZ_DW(manage_pages_in)] = {};
 	int num_claimed;
@@ -514,7 +524,7 @@ static int reclaim_pages(struct mlx5_core_dev *dev, u32 func_id, int npages,
 	}
 
 	for (i = 0; i < num_claimed; i++)
-		free_4k(dev, MLX5_GET64(manage_pages_out, out, pas[i]), func_id);
+		free_4k(dev, MLX5_GET64(manage_pages_out, out, pas[i]), function);
 
 	if (nclaimed)
 		*nclaimed = num_claimed;
-- 
2.29.2

[net 07/12] net/mlx5e: Disable hw-tc-offload when MLX5_CLS_ACT config is disabled

[Saeed Mahameed]

From: Maor Dickman <maord@nvidia.com>

The cited commit introduce new CONFIG_MLX5_CLS_ACT kconfig variable
to control compilation of TC hardware offloads implementation.
When this configuration is disabled the driver is still wrongly
reports in ethtool that hw-tc-offload is supported.

Fixed by reporting hw-tc-offload is supported only when
CONFIG_MLX5_CLS_ACT is enabled.

Fixes: d956873f908c ("net/mlx5e: Introduce kconfig var for TC support")
Signed-off-by: Maor Dickman <maord@nvidia.com>
Reviewed-by: Vlad Buslov <vladbu@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c  | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 6a852b4901aa..300e0e9f96b6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -5027,7 +5027,7 @@ static void mlx5e_build_nic_netdev(struct net_device *netdev)
 	    FT_CAP(modify_root) &&
 	    FT_CAP(identified_miss_table_mode) &&
 	    FT_CAP(flow_table_modify)) {
-#ifdef CONFIG_MLX5_ESWITCH
+#if IS_ENABLED(CONFIG_MLX5_CLS_ACT)
 		netdev->hw_features      |= NETIF_F_HW_TC;
 #endif
 #ifdef CONFIG_MLX5_EN_ARFS
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 989c70c1eda3..f0ceae65f6cf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -737,7 +737,9 @@ static void mlx5e_build_rep_netdev(struct net_device *netdev)
 
 	netdev->features       |= NETIF_F_NETNS_LOCAL;
 
+#if IS_ENABLED(CONFIG_MLX5_CLS_ACT)
 	netdev->hw_features    |= NETIF_F_HW_TC;
+#endif
 	netdev->hw_features    |= NETIF_F_SG;
 	netdev->hw_features    |= NETIF_F_IP_CSUM;
 	netdev->hw_features    |= NETIF_F_IPV6_CSUM;
-- 
2.29.2

[net 08/12] net/mlx5e: Fix CT rule + encap slow path offload and deletion

[Saeed Mahameed]

From: Paul Blakey <paulb@nvidia.com>

Currently, if a neighbour isn't valid when offloading tunnel encap rules,
we offload the original match and replace the original action with
"goto slow path" action. For this we use a temporary flow attribute based
on the original flow attribute and then change the action. Flow flags,
which among those is the CT flag, are still shared for the slow path rule
offload, so we end up parsing this flow as a CT + goto slow path rule.

Besides being unnecessary, CT action offload saves extra information in
the passed flow attribute, such as created ct_flow and mod_hdr, which
is lost onces the temporary flow attribute is freed.

When a neigh is updated and is valid, we offload the original CT rule
with original CT action, which again creates a ct_flow and mod_hdr
and saves it in the flow's original attribute. Then we delete the slow
path rule with a temporary flow attribute based on original updated
flow attribute, and we free the relevant ct_flow and mod_hdr.

Then when tc deletes this flow, we try to free the ct_flow and mod_hdr
on the flow's attribute again.

To fix the issue, skip all furture proccesing (CT/Sample/Split rules)
in offload/unoffload of slow path rules.

Call trace:
[  758.850525] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000218
[  758.952987] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[  758.964170] Modules linked in: act_csum(E) act_pedit(E) act_tunnel_key(E) act_ct(E) nf_flow_table(E) xt_nat(E) ip6table_filter(E) ip6table_nat(E) xt_comment(E) ip6_tables(E) xt_conntrack(E) xt_MASQUERADE(E) nf_conntrack_netlink(E) xt_addrtype(E) iptable_filter(E) iptable_nat(E) bpfilter(E) br_netfilter(E) bridge(E) stp(E) llc(E) xfrm_user(E) overlay(E) act_mirred(E) act_skbedit(E) rdma_ucm(OE) rdma_cm(OE) iw_cm(OE) ib_ipoib(OE) ib_cm(OE) ib_umad(OE) esp6_offload(E) esp6(E) esp4_offload(E) esp4(E) xfrm_algo(E) mlx5_ib(OE) ib_uverbs(OE) geneve(E) ip6_udp_tunnel(E) udp_tunnel(E) nfnetlink_cttimeout(E) nfnetlink(E) mlx5_core(OE) act_gact(E) cls_flower(E) sch_ingress(E) openvswitch(E) nsh(E) nf_conncount(E) nf_nat(E) mlxfw(OE) psample(E) nf_conntrack(E) nf_defrag_ipv4(E) vfio_mdev(E) mdev(E) ib_core(OE) mlx_compat(OE) crct10dif_ce(E) uio_pdrv_genirq(E) uio(E) i2c_mlx(E) mlxbf_pmc(E) sbsa_gwdt(E) mlxbf_gige(E) gpio_mlxbf2(E) mlxbf_pka(E) mlx_trio(E) mlx_bootctl(E) bluefield_edac(E) knem(O)
[  758.964225]  ip_tables(E) mlxbf_tmfifo(E) ipv6(E) crc_ccitt(E) nf_defrag_ipv6(E)
[  759.154186] CPU: 5 PID: 122 Comm: kworker/u16:1 Tainted: G           OE     5.4.60-mlnx.52.gde81e85 #1
[  759.172870] Hardware name: https://www.mellanox.com BlueField SoC/BlueField SoC, BIOS BlueField:3.5.0-2-gc1b5d64 Jan  4 2021
[  759.195466] Workqueue: mlx5e mlx5e_rep_neigh_update [mlx5_core]
[  759.207344] pstate: a0000005 (NzCv daif -PAN -UAO)
[  759.217003] pc : mlx5_del_flow_rules+0x5c/0x160 [mlx5_core]
[  759.228229] lr : mlx5_del_flow_rules+0x34/0x160 [mlx5_core]
[  759.405858] Call trace:
[  759.410804]  mlx5_del_flow_rules+0x5c/0x160 [mlx5_core]
[  759.421337]  __mlx5_eswitch_del_rule.isra.43+0x5c/0x1c8 [mlx5_core]
[  759.433963]  mlx5_eswitch_del_offloaded_rule_ct+0x34/0x40 [mlx5_core]
[  759.446942]  mlx5_tc_rule_delete_ct+0x68/0x74 [mlx5_core]
[  759.457821]  mlx5_tc_ct_delete_flow+0x160/0x21c [mlx5_core]
[  759.469051]  mlx5e_tc_unoffload_fdb_rules+0x158/0x168 [mlx5_core]
[  759.481325]  mlx5e_tc_encap_flows_del+0x140/0x26c [mlx5_core]
[  759.492901]  mlx5e_rep_update_flows+0x11c/0x1ec [mlx5_core]
[  759.504127]  mlx5e_rep_neigh_update+0x160/0x200 [mlx5_core]
[  759.515314]  process_one_work+0x178/0x400
[  759.523350]  worker_thread+0x58/0x3e8
[  759.530685]  kthread+0x100/0x12c
[  759.537152]  ret_from_fork+0x10/0x18
[  759.544320] Code: 97ffef55 51000673 3100067f 54ffff41 (b9421ab3)
[  759.556548] ---[ end trace fab818bb1085832d ]---

Fixes: 4c3844d9e97e ("net/mlx5e: CT: Introduce connection tracking")
Signed-off-by: Paul Blakey <paulb@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Vlad Buslov <vladbu@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index f4ce5e208e02..dd0bfbacad47 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -1163,6 +1163,9 @@ mlx5e_tc_offload_fdb_rules(struct mlx5_eswitch *esw,
 	struct mlx5e_tc_mod_hdr_acts *mod_hdr_acts;
 	struct mlx5_flow_handle *rule;
 
+	if (attr->flags & MLX5_ESW_ATTR_FLAG_SLOW_PATH)
+		return mlx5_eswitch_add_offloaded_rule(esw, spec, attr);
+
 	if (flow_flag_test(flow, CT)) {
 		mod_hdr_acts = &attr->parse_attr->mod_hdr_acts;
 
@@ -1193,6 +1196,9 @@ mlx5e_tc_unoffload_fdb_rules(struct mlx5_eswitch *esw,
 {
 	flow_flag_clear(flow, OFFLOADED);
 
+	if (attr->flags & MLX5_ESW_ATTR_FLAG_SLOW_PATH)
+		goto offload_rule_0;
+
 	if (flow_flag_test(flow, CT)) {
 		mlx5_tc_ct_delete_flow(get_ct_priv(flow->priv), flow, attr);
 		return;
@@ -1201,6 +1207,7 @@ mlx5e_tc_unoffload_fdb_rules(struct mlx5_eswitch *esw,
 	if (attr->esw_attr->split_count)
 		mlx5_eswitch_del_fwd_rule(esw, flow->rule[1], attr);
 
+offload_rule_0:
 	mlx5_eswitch_del_offloaded_rule(esw, flow->rule[0], attr);
 }
 
-- 
2.29.2

[net 09/12] net/mlx5e: Correctly handle changing the number of queues when the interface is down

[Saeed Mahameed]

From: Maxim Mikityanskiy <maximmi@mellanox.com>

This commit addresses two issues related to changing the number of
queues when the channels are closed:

1. Missing call to mlx5e_num_channels_changed to update
real_num_tx_queues when the number of TCs is changed.

2. When mlx5e_num_channels_changed returns an error, the channel
parameters must be reverted.

Two Fixes: tags correspond to the first commits where these two issues
were introduced.

Fixes: 3909a12e7913 ("net/mlx5e: Fix configuration of XPS cpumasks and netdev queues in corner cases")
Fixes: fa3748775b92 ("net/mlx5e: Handle errors from netif_set_real_num_{tx,rx}_queues")
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 8 +++++++-
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c    | 7 +++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 2d37742a888c..302001d6661e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -447,12 +447,18 @@ int mlx5e_ethtool_set_channels(struct mlx5e_priv *priv,
 		goto out;
 	}
 
-	new_channels.params = priv->channels.params;
+	new_channels.params = *cur_params;
 	new_channels.params.num_channels = count;
 
 	if (!test_bit(MLX5E_STATE_OPENED, &priv->state)) {
+		struct mlx5e_params old_params;
+
+		old_params = *cur_params;
 		*cur_params = new_channels.params;
 		err = mlx5e_num_channels_changed(priv);
+		if (err)
+			*cur_params = old_params;
+
 		goto out;
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 300e0e9f96b6..ac76d32bad7d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3614,7 +3614,14 @@ static int mlx5e_setup_tc_mqprio(struct mlx5e_priv *priv,
 	new_channels.params.num_tc = tc ? tc : 1;
 
 	if (!test_bit(MLX5E_STATE_OPENED, &priv->state)) {
+		struct mlx5e_params old_params;
+
+		old_params = priv->channels.params;
 		priv->channels.params = new_channels.params;
+		err = mlx5e_num_channels_changed(priv);
+		if (err)
+			priv->channels.params = old_params;
+
 		goto out;
 	}
 
-- 
2.29.2

[net 10/12] net/mlx5e: Revert parameters on errors when changing trust state without reset

[Saeed Mahameed]

From: Maxim Mikityanskiy <maximmi@mellanox.com>

Trust state may be changed without recreating the channels. It happens
when the channels are closed, and when channel parameters (min inline
mode) stay the same after changing the trust state. Changing the trust
state is a hardware command that may fail. The current code didn't
restore the channel parameters to their old values if an error happened
and the channels were closed. This commit adds handling for this case.

Fixes: 6e0504c69811 ("net/mlx5e: Change inline mode correctly when changing trust state")
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
index d20243d6a032..f23c67575073 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c
@@ -1151,6 +1151,7 @@ static int mlx5e_set_trust_state(struct mlx5e_priv *priv, u8 trust_state)
 {
 	struct mlx5e_channels new_channels = {};
 	bool reset_channels = true;
+	bool opened;
 	int err = 0;
 
 	mutex_lock(&priv->state_lock);
@@ -1159,22 +1160,24 @@ static int mlx5e_set_trust_state(struct mlx5e_priv *priv, u8 trust_state)
 	mlx5e_params_calc_trust_tx_min_inline_mode(priv->mdev, &new_channels.params,
 						   trust_state);
 
-	if (!test_bit(MLX5E_STATE_OPENED, &priv->state)) {
-		priv->channels.params = new_channels.params;
+	opened = test_bit(MLX5E_STATE_OPENED, &priv->state);
+	if (!opened)
 		reset_channels = false;
-	}
 
 	/* Skip if tx_min_inline is the same */
 	if (new_channels.params.tx_min_inline_mode ==
 	    priv->channels.params.tx_min_inline_mode)
 		reset_channels = false;
 
-	if (reset_channels)
+	if (reset_channels) {
 		err = mlx5e_safe_switch_channels(priv, &new_channels,
 						 mlx5e_update_trust_state_hw,
 						 &trust_state);
-	else
+	} else {
 		err = mlx5e_update_trust_state_hw(priv, &trust_state);
+		if (!err && !opened)
+			priv->channels.params = new_channels.params;
+	}
 
 	mutex_unlock(&priv->state_lock);
 
-- 
2.29.2

[net 11/12] net/mlx5e: Revert parameters on errors when changing MTU and LRO state without reset

[Saeed Mahameed]

From: Maxim Mikityanskiy <maximmi@mellanox.com>

Sometimes, channel params are changed without recreating the channels.
It happens in two basic cases: when the channels are closed, and when
the parameter being changed doesn't affect how channels are configured.
Such changes invoke a hardware command that might fail. The whole
operation should be reverted in such cases, but the code that restores
the parameters' values in the driver was missing. This commit adds this
handling.

Fixes: 2e20a151205b ("net/mlx5e: Fail safe mtu and lro setting")
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 30 +++++++++++++------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index ac76d32bad7d..a9d824a9cb05 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3764,7 +3764,7 @@ static int set_feature_lro(struct net_device *netdev, bool enable)
 	struct mlx5e_priv *priv = netdev_priv(netdev);
 	struct mlx5_core_dev *mdev = priv->mdev;
 	struct mlx5e_channels new_channels = {};
-	struct mlx5e_params *old_params;
+	struct mlx5e_params *cur_params;
 	int err = 0;
 	bool reset;
 
@@ -3777,8 +3777,8 @@ static int set_feature_lro(struct net_device *netdev, bool enable)
 		goto out;
 	}
 
-	old_params = &priv->channels.params;
-	if (enable && !MLX5E_GET_PFLAG(old_params, MLX5E_PFLAG_RX_STRIDING_RQ)) {
+	cur_params = &priv->channels.params;
+	if (enable && !MLX5E_GET_PFLAG(cur_params, MLX5E_PFLAG_RX_STRIDING_RQ)) {
 		netdev_warn(netdev, "can't set LRO with legacy RQ\n");
 		err = -EINVAL;
 		goto out;
@@ -3786,18 +3786,23 @@ static int set_feature_lro(struct net_device *netdev, bool enable)
 
 	reset = test_bit(MLX5E_STATE_OPENED, &priv->state);
 
-	new_channels.params = *old_params;
+	new_channels.params = *cur_params;
 	new_channels.params.lro_en = enable;
 
-	if (old_params->rq_wq_type != MLX5_WQ_TYPE_CYCLIC) {
-		if (mlx5e_rx_mpwqe_is_linear_skb(mdev, old_params, NULL) ==
+	if (cur_params->rq_wq_type != MLX5_WQ_TYPE_CYCLIC) {
+		if (mlx5e_rx_mpwqe_is_linear_skb(mdev, cur_params, NULL) ==
 		    mlx5e_rx_mpwqe_is_linear_skb(mdev, &new_channels.params, NULL))
 			reset = false;
 	}
 
 	if (!reset) {
-		*old_params = new_channels.params;
+		struct mlx5e_params old_params;
+
+		old_params = *cur_params;
+		*cur_params = new_channels.params;
 		err = mlx5e_modify_tirs_lro(priv);
+		if (err)
+			*cur_params = old_params;
 		goto out;
 	}
 
@@ -4074,9 +4079,16 @@ int mlx5e_change_mtu(struct net_device *netdev, int new_mtu,
 	}
 
 	if (!reset) {
+		unsigned int old_mtu = params->sw_mtu;
+
 		params->sw_mtu = new_mtu;
-		if (preactivate)
-			preactivate(priv, NULL);
+		if (preactivate) {
+			err = preactivate(priv, NULL);
+			if (err) {
+				params->sw_mtu = old_mtu;
+				goto out;
+			}
+		}
 		netdev->mtu = params->sw_mtu;
 		goto out;
 	}
-- 
2.29.2

[net 12/12] net/mlx5: CT: Fix incorrect removal of tuple_nat_node from nat rhashtable

[Saeed Mahameed]

From: Paul Blakey <paulb@nvidia.com>

If a non nat tuple entry is inserted just to the regular tuples
rhashtable (ct_tuples_ht) and not to natted tuples rhashtable
(ct_nat_tuples_ht). Commit bc562be9674b ("net/mlx5e: CT: Save ct entries
tuples in hashtables") mixed up the return labels and names sot that on
cleanup or failure we still try to remove for the natted tuples rhashtable.

Fix that by correctly checking if a natted tuples insertion
before removing it. While here make it more readable.

Fixes: bc562be9674b ("net/mlx5e: CT: Save ct entries tuples in hashtables")
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Paul Blakey <paulb@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/en/tc_ct.c    | 20 ++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
index 072363e73f1c..6bc6b48a56dc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
@@ -167,6 +167,12 @@ static const struct rhashtable_params tuples_nat_ht_params = {
 	.min_size = 16 * 1024,
 };
 
+static bool
+mlx5_tc_ct_entry_has_nat(struct mlx5_ct_entry *entry)
+{
+	return !!(entry->tuple_nat_node.next);
+}
+
 static int
 mlx5_tc_ct_rule_to_tuple(struct mlx5_ct_tuple *tuple, struct flow_rule *rule)
 {
@@ -911,13 +917,13 @@ mlx5_tc_ct_block_flow_offload_add(struct mlx5_ct_ft *ft,
 err_insert:
 	mlx5_tc_ct_entry_del_rules(ct_priv, entry);
 err_rules:
-	rhashtable_remove_fast(&ct_priv->ct_tuples_nat_ht,
-			       &entry->tuple_nat_node, tuples_nat_ht_params);
+	if (mlx5_tc_ct_entry_has_nat(entry))
+		rhashtable_remove_fast(&ct_priv->ct_tuples_nat_ht,
+				       &entry->tuple_nat_node, tuples_nat_ht_params);
 err_tuple_nat:
-	if (entry->tuple_node.next)
-		rhashtable_remove_fast(&ct_priv->ct_tuples_ht,
-				       &entry->tuple_node,
-				       tuples_ht_params);
+	rhashtable_remove_fast(&ct_priv->ct_tuples_ht,
+			       &entry->tuple_node,
+			       tuples_ht_params);
 err_tuple:
 err_set:
 	kfree(entry);
@@ -932,7 +938,7 @@ mlx5_tc_ct_del_ft_entry(struct mlx5_tc_ct_priv *ct_priv,
 {
 	mlx5_tc_ct_entry_del_rules(ct_priv, entry);
 	mutex_lock(&ct_priv->shared_counter_lock);
-	if (entry->tuple_node.next)
+	if (mlx5_tc_ct_entry_has_nat(entry))
 		rhashtable_remove_fast(&ct_priv->ct_tuples_nat_ht,
 				       &entry->tuple_nat_node,
 				       tuples_nat_ht_params);
-- 
2.29.2

Re: [net 11/12] net/mlx5e: Revert parameters on errors when changing MTU and LRO state without reset

[Willem de Bruijn]

On Wed, Jan 27, 2021 at 3:58 AM Saeed Mahameed <saeedm@nvidia.com> wrote:
>
> From: Maxim Mikityanskiy <maximmi@mellanox.com>
>
> Sometimes, channel params are changed without recreating the channels.
> It happens in two basic cases: when the channels are closed, and when
> the parameter being changed doesn't affect how channels are configured.
> Such changes invoke a hardware command that might fail. The whole
> operation should be reverted in such cases, but the code that restores
> the parameters' values in the driver was missing. This commit adds this
> handling.
>
> Fixes: 2e20a151205b ("net/mlx5e: Fail safe mtu and lro setting")
> Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> ---
>  .../net/ethernet/mellanox/mlx5/core/en_main.c | 30 +++++++++++++------
>  1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index ac76d32bad7d..a9d824a9cb05 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -3764,7 +3764,7 @@ static int set_feature_lro(struct net_device *netdev, bool enable)
>         struct mlx5e_priv *priv = netdev_priv(netdev);
>         struct mlx5_core_dev *mdev = priv->mdev;
>         struct mlx5e_channels new_channels = {};
> -       struct mlx5e_params *old_params;
> +       struct mlx5e_params *cur_params;
>         int err = 0;
>         bool reset;
>
> @@ -3777,8 +3777,8 @@ static int set_feature_lro(struct net_device *netdev, bool enable)
>                 goto out;
>         }
>
> -       old_params = &priv->channels.params;
> -       if (enable && !MLX5E_GET_PFLAG(old_params, MLX5E_PFLAG_RX_STRIDING_RQ)) {
> +       cur_params = &priv->channels.params;
> +       if (enable && !MLX5E_GET_PFLAG(cur_params, MLX5E_PFLAG_RX_STRIDING_RQ)) {
>                 netdev_warn(netdev, "can't set LRO with legacy RQ\n");
>                 err = -EINVAL;
>                 goto out;
> @@ -3786,18 +3786,23 @@ static int set_feature_lro(struct net_device *netdev, bool enable)
>
>         reset = test_bit(MLX5E_STATE_OPENED, &priv->state);
>
> -       new_channels.params = *old_params;
> +       new_channels.params = *cur_params;
>         new_channels.params.lro_en = enable;
>
> -       if (old_params->rq_wq_type != MLX5_WQ_TYPE_CYCLIC) {
> -               if (mlx5e_rx_mpwqe_is_linear_skb(mdev, old_params, NULL) ==
> +       if (cur_params->rq_wq_type != MLX5_WQ_TYPE_CYCLIC) {
> +               if (mlx5e_rx_mpwqe_is_linear_skb(mdev, cur_params, NULL) ==
>                     mlx5e_rx_mpwqe_is_linear_skb(mdev, &new_channels.params, NULL))
>                         reset = false;
>         }
>
>         if (!reset) {
> -               *old_params = new_channels.params;
> +               struct mlx5e_params old_params;
> +
> +               old_params = *cur_params;
> +               *cur_params = new_channels.params;
>                 err = mlx5e_modify_tirs_lro(priv);
> +               if (err)
> +                       *cur_params = old_params;

No need to explicitly save and restore all params if the only one
changed is lro_en?

Re: [pull request][net 00/12] mlx5 fixes 2021-01-26

[Willem de Bruijn]

On Wed, Jan 27, 2021 at 2:36 AM Saeed Mahameed <saeedm@nvidia.com> wrote:
>
> Hi Jakub, Dave
>
> This series introduces some fixes to mlx5 driver.
> Please pull and let me know if there is any problem.
>
> Thanks,
> Saeed.
>
> ---
> The following changes since commit c5e9e8d48acdf3b863282af7f6f6931d39526245:
>
>   Merge tag 'mac80211-for-net-2021-01-26' of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211 (2021-01-26 15:23:18 -0800)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git tags/mlx5-fixes-2021-01-26
>
> for you to fetch changes up to e2194a1744e8594e82a861687808c1adca419b85:
>
>   net/mlx5: CT: Fix incorrect removal of tuple_nat_node from nat rhashtable (2021-01-26 15:39:04 -0800)
>
> ----------------------------------------------------------------
> mlx5-fixes-2021-01-26
>
> ----------------------------------------------------------------
> Daniel Jurgens (1):
>       net/mlx5: Maintain separate page trees for ECPF and PF functions
>
> Maor Dickman (2):
>       net/mlx5e: Reduce tc unsupported key print level
>       net/mlx5e: Disable hw-tc-offload when MLX5_CLS_ACT config is disabled
>
> Maxim Mikityanskiy (4):
>       net/mlx5e: Fix IPSEC stats
>       net/mlx5e: Correctly handle changing the number of queues when the interface is down
>       net/mlx5e: Revert parameters on errors when changing trust state without reset
>       net/mlx5e: Revert parameters on errors when changing MTU and LRO state without reset
>
> Pan Bian (1):
>       net/mlx5e: free page before return
>
> Parav Pandit (1):
>       net/mlx5e: E-switch, Fix rate calculation for overflow
>
> Paul Blakey (2):
>       net/mlx5e: Fix CT rule + encap slow path offload and deletion
>       net/mlx5: CT: Fix incorrect removal of tuple_nat_node from nat rhashtable
>
> Roi Dayan (1):
>       net/mlx5: Fix memory leak on flow table creation error flow
>
>  .../net/ethernet/mellanox/mlx5/core/en/health.c    |  2 +-
>  drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c | 20 +++++---
>  .../mellanox/mlx5/core/en_accel/ipsec_stats.c      |  4 +-
>  drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c | 13 +++--
>  .../net/ethernet/mellanox/mlx5/core/en_ethtool.c   |  8 ++-
>  drivers/net/ethernet/mellanox/mlx5/core/en_main.c  | 39 +++++++++++----
>  drivers/net/ethernet/mellanox/mlx5/core/en_rep.c   |  2 +
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c    | 22 ++++++--
>  drivers/net/ethernet/mellanox/mlx5/core/fs_core.c  |  1 +
>  .../net/ethernet/mellanox/mlx5/core/pagealloc.c    | 58 +++++++++++++---------
>  10 files changed, 114 insertions(+), 55 deletions(-)

For netdrv:

Acked-by: Willem de Bruijn <willemb@google.com>

I left a small comment in patch 11/12, but no need to change that.

In patch 12/12, the removal of branch if (entry->tuple_node.next) is
not entirely obvious to me, but based on the commit message I assume
that it was intentional. Pointing it out only in the unlikely case
that it wasn't.

Re: [net 11/12] net/mlx5e: Revert parameters on errors when changing MTU and LRO state without reset

[Saeed Mahameed]

On Wed, 2021-01-27 at 15:00 -0500, Willem de Bruijn wrote:
> On Wed, Jan 27, 2021 at 3:58 AM Saeed Mahameed <saeedm@nvidia.com>
> wrote:
> > 
> > From: Maxim Mikityanskiy <maximmi@mellanox.com>
> > 
> > Sometimes, channel params are changed without recreating the
> > channels.
> > It happens in two basic cases: when the channels are closed, and
> > when
> > the parameter being changed doesn't affect how channels are
> > configured.
> > Such changes invoke a hardware command that might fail. The whole
> > operation should be reverted in such cases, but the code that
> > restores
> > the parameters' values in the driver was missing. This commit adds
> > this
> > handling.
> > 
> > Fixes: 2e20a151205b ("net/mlx5e: Fail safe mtu and lro setting")
> > Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
> > Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> > Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> > ---
> >  .../net/ethernet/mellanox/mlx5/core/en_main.c | 30 +++++++++++++--
> > ----
> >  1 file changed, 21 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > index ac76d32bad7d..a9d824a9cb05 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > @@ -3764,7 +3764,7 @@ static int set_feature_lro(struct net_device
> > *netdev, bool enable)
> >         struct mlx5e_priv *priv = netdev_priv(netdev);
> >         struct mlx5_core_dev *mdev = priv->mdev;
> >         struct mlx5e_channels new_channels = {};
> > -       struct mlx5e_params *old_params;
> > +       struct mlx5e_params *cur_params;
> >         int err = 0;
> >         bool reset;
> > 
> > @@ -3777,8 +3777,8 @@ static int set_feature_lro(struct net_device
> > *netdev, bool enable)
> >                 goto out;
> >         }
> > 
> > -       old_params = &priv->channels.params;
> > -       if (enable && !MLX5E_GET_PFLAG(old_params,
> > MLX5E_PFLAG_RX_STRIDING_RQ)) {
> > +       cur_params = &priv->channels.params;
> > +       if (enable && !MLX5E_GET_PFLAG(cur_params,
> > MLX5E_PFLAG_RX_STRIDING_RQ)) {
> >                 netdev_warn(netdev, "can't set LRO with legacy
> > RQ\n");
> >                 err = -EINVAL;
> >                 goto out;
> > @@ -3786,18 +3786,23 @@ static int set_feature_lro(struct
> > net_device *netdev, bool enable)
> > 
> >         reset = test_bit(MLX5E_STATE_OPENED, &priv->state);
> > 
> > -       new_channels.params = *old_params;
> > +       new_channels.params = *cur_params;
> >         new_channels.params.lro_en = enable;
> > 
> > -       if (old_params->rq_wq_type != MLX5_WQ_TYPE_CYCLIC) {
> > -               if (mlx5e_rx_mpwqe_is_linear_skb(mdev, old_params,
> > NULL) ==
> > +       if (cur_params->rq_wq_type != MLX5_WQ_TYPE_CYCLIC) {
> > +               if (mlx5e_rx_mpwqe_is_linear_skb(mdev, cur_params,
> > NULL) ==
> >                     mlx5e_rx_mpwqe_is_linear_skb(mdev,
> > &new_channels.params, NULL))
> >                         reset = false;
> >         }
> > 
> >         if (!reset) {
> > -               *old_params = new_channels.params;
> > +               struct mlx5e_params old_params;
> > +
> > +               old_params = *cur_params;
> > +               *cur_params = new_channels.params;
> >                 err = mlx5e_modify_tirs_lro(priv);
> > +               if (err)
> > +                       *cur_params = old_params;
> 
> No need to explicitly save and restore all params if the only one
> changed is lro_en?

not a big deal, this is a practice we follow in all of our unwind
procedures, 
the code will change in net-next to use a generic function that would
restore all params regardless of what changed and what not .. so we
figured to have a one flow that will be easy to replace in net-next.

Re: [pull request][net 00/12] mlx5 fixes 2021-01-26

[Saeed Mahameed]

On Wed, 2021-01-27 at 15:03 -0500, Willem de Bruijn wrote:
> On Wed, Jan 27, 2021 at 2:36 AM Saeed Mahameed <saeedm@nvidia.com>
> wrote:
> > 
> > Hi Jakub, Dave
> > 
> > This series introduces some fixes to mlx5 driver.
> > Please pull and let me know if there is any problem.
> > 
> > Thanks,
> > Saeed.
> > 
> > ---
> > The following changes since commit
> > c5e9e8d48acdf3b863282af7f6f6931d39526245:
> > 
> >   Merge tag 'mac80211-for-net-2021-01-26' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211 (2021-
> > 01-26 15:23:18 -0800)
> > 
> > are available in the Git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git
> > tags/mlx5-fixes-2021-01-26
> > 
> > for you to fetch changes up to
> > e2194a1744e8594e82a861687808c1adca419b85:
> > 
> >   net/mlx5: CT: Fix incorrect removal of tuple_nat_node from nat
> > rhashtable (2021-01-26 15:39:04 -0800)
> > 
> > ----------------------------------------------------------------
> > mlx5-fixes-2021-01-26
> > 
> > ----------------------------------------------------------------
> > Daniel Jurgens (1):
> >       net/mlx5: Maintain separate page trees for ECPF and PF
> > functions
> > 
> > Maor Dickman (2):
> >       net/mlx5e: Reduce tc unsupported key print level
> >       net/mlx5e: Disable hw-tc-offload when MLX5_CLS_ACT config is
> > disabled
> > 
> > Maxim Mikityanskiy (4):
> >       net/mlx5e: Fix IPSEC stats
> >       net/mlx5e: Correctly handle changing the number of queues
> > when the interface is down
> >       net/mlx5e: Revert parameters on errors when changing trust
> > state without reset
> >       net/mlx5e: Revert parameters on errors when changing MTU and
> > LRO state without reset
> > 
> > Pan Bian (1):
> >       net/mlx5e: free page before return
> > 
> > Parav Pandit (1):
> >       net/mlx5e: E-switch, Fix rate calculation for overflow
> > 
> > Paul Blakey (2):
> >       net/mlx5e: Fix CT rule + encap slow path offload and deletion
> >       net/mlx5: CT: Fix incorrect removal of tuple_nat_node from
> > nat rhashtable
> > 
> > Roi Dayan (1):
> >       net/mlx5: Fix memory leak on flow table creation error flow
> > 
> >  .../net/ethernet/mellanox/mlx5/core/en/health.c    |  2 +-
> >  drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c | 20 +++++---
> >  .../mellanox/mlx5/core/en_accel/ipsec_stats.c      |  4 +-
> >  drivers/net/ethernet/mellanox/mlx5/core/en_dcbnl.c | 13 +++--
> >  .../net/ethernet/mellanox/mlx5/core/en_ethtool.c   |  8 ++-
> >  drivers/net/ethernet/mellanox/mlx5/core/en_main.c  | 39
> > +++++++++++----
> >  drivers/net/ethernet/mellanox/mlx5/core/en_rep.c   |  2 +
> >  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c    | 22 ++++++--
> >  drivers/net/ethernet/mellanox/mlx5/core/fs_core.c  |  1 +
> >  .../net/ethernet/mellanox/mlx5/core/pagealloc.c    | 58
> > +++++++++++++---------
> >  10 files changed, 114 insertions(+), 55 deletions(-)
> 
> For netdrv:
> 
> Acked-by: Willem de Bruijn <willemb@google.com>
> 
> I left a small comment in patch 11/12, but no need to change that.
> 
> In patch 12/12, the removal of branch if (entry->tuple_node.next) is
> not entirely obvious to me, but based on the commit message I assume
> that it was intentional. Pointing it out only in the unlikely case
> that it wasn't.

it was intentional as the check was redundant. 

Thanks for the review!

Re: [net 01/12] net/mlx5: Fix memory leak on flow table creation error flow

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Tue, 26 Jan 2021 15:43:34 -0800 you wrote:
> From: Roi Dayan <roid@nvidia.com>
> 
> When we create the ft object we also init rhltable in ft->fgs_hash.
> So in error flow before kfree of ft we need to destroy that rhltable.
> 
> Fixes: 693c6883bbc4 ("net/mlx5: Add hash table for flow groups in flow table")
> Signed-off-by: Roi Dayan <roid@nvidia.com>
> Reviewed-by: Maor Dickman <maord@nvidia.com>
> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> 
> [...]

Here is the summary with links:
  - [net,01/12] net/mlx5: Fix memory leak on flow table creation error flow
    https://git.kernel.org/netdev/net/c/487c6ef81eb9
  - [net,02/12] net/mlx5e: E-switch, Fix rate calculation for overflow
    https://git.kernel.org/netdev/net/c/1fe3e3166b35
  - [net,03/12] net/mlx5e: free page before return
    https://git.kernel.org/netdev/net/c/258ed19f075f
  - [net,04/12] net/mlx5e: Reduce tc unsupported key print level
    https://git.kernel.org/netdev/net/c/48470a90a42a
  - [net,05/12] net/mlx5e: Fix IPSEC stats
    https://git.kernel.org/netdev/net/c/45c9a30835d8
  - [net,06/12] net/mlx5: Maintain separate page trees for ECPF and PF functions
    https://git.kernel.org/netdev/net/c/0aa128475d33
  - [net,07/12] net/mlx5e: Disable hw-tc-offload when MLX5_CLS_ACT config is disabled
    https://git.kernel.org/netdev/net/c/156878d0e697
  - [net,08/12] net/mlx5e: Fix CT rule + encap slow path offload and deletion
    https://git.kernel.org/netdev/net/c/89e394675818
  - [net,09/12] net/mlx5e: Correctly handle changing the number of queues when the interface is down
    https://git.kernel.org/netdev/net/c/57ac4a31c483
  - [net,10/12] net/mlx5e: Revert parameters on errors when changing trust state without reset
    https://git.kernel.org/netdev/net/c/912c9b5fcca1
  - [net,11/12] net/mlx5e: Revert parameters on errors when changing MTU and LRO state without reset
    https://git.kernel.org/netdev/net/c/8355060f5ec3
  - [net,12/12] net/mlx5: CT: Fix incorrect removal of tuple_nat_node from nat rhashtable
    https://git.kernel.org/netdev/net/c/e2194a1744e8

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [net 01/12] net/mlx5: Fix memory leak on flow table creation error flow

[Saeed Mahameed]

On Thu, 2021-01-28 at 03:30 +0000, patchwork-bot+netdevbpf@kernel.org
wrote:
> Hello:
> 
> This series was applied to netdev/net.git (refs/heads/master):
> 
> 
...

>     https://git.kernel.org/netdev/net/c/89e394675818
>   - [net,09/12] net/mlx5e: Correctly handle changing the number of
> queues when the interface is down



Hi Jakub, 

I just noticed that this patch will conflict with HTB offlaod feature
in net-next, I couldn't foresee it before in my queues since HTB wasn't
submitted through my trees.

anyway to solve the conflict just use this hunk:

 +      /* Don't allow changing the number of channels if HTB offload
is active,
 +       * because the numeration of the QoS SQs will change, while
per-queue
 +       * qdiscs are attached.
 +       */
 +      if (priv->htb.maj_id) {
 +              err = -EINVAL;
 +              netdev_err(priv->netdev, "%s: HTB offload is active,
cannot change the number of channels\n",
 +                         __func__);
 +              goto out;
 +      }
 +
-       new_channels.params = priv->channels.params;
-       new_channels.params.num_channels = count;
+       new_channels.params = *cur_params;



Thanks,
Saeed.

Re: [net 01/12] net/mlx5: Fix memory leak on flow table creation error flow

[Jakub Kicinski]

On Thu, 28 Jan 2021 00:33:20 -0800 Saeed Mahameed wrote:
> On Thu, 2021-01-28 at 03:30 +0000, patchwork-bot+netdevbpf@kernel.org
> wrote:
> > Hello:
> > 
> > This series was applied to netdev/net.git (refs/heads/master):
> > 
> >   
> ...
> 
> >     https://git.kernel.org/netdev/net/c/89e394675818
> >   - [net,09/12] net/mlx5e: Correctly handle changing the number of
> > queues when the interface is down  
> 
> 
> 
> Hi Jakub, 
> 
> I just noticed that this patch will conflict with HTB offlaod feature
> in net-next, I couldn't foresee it before in my queues since HTB wasn't
> submitted through my trees.
> 
> anyway to solve the conflict just use this hunk:
> 
>  +      /* Don't allow changing the number of channels if HTB offload
> is active,
>  +       * because the numeration of the QoS SQs will change, while
> per-queue
>  +       * qdiscs are attached.
>  +       */
>  +      if (priv->htb.maj_id) {
>  +              err = -EINVAL;
>  +              netdev_err(priv->netdev, "%s: HTB offload is active,
> cannot change the number of channels\n",
>  +                         __func__);
>  +              goto out;
>  +      }
>  +
> -       new_channels.params = priv->channels.params;
> -       new_channels.params.num_channels = count;
> +       new_channels.params = *cur_params;
> 

Done, thanks!