You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
209 lines
8.3 KiB
209 lines
8.3 KiB
4 years ago
|
From d614648206c87c5d068f9dec596c9dfb152a5618 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Alaa Hleihel <ahleihel@redhat.com>
|
||
|
|
Date: Sun, 10 May 2020 15:04:42 -0400
|
||
|
|
Subject: [PATCH 117/312] [netdrv] net/mlx5e: Avoid duplicating rule
|
||
|
|
destinations
|
||
|
|
|
||
|
|
Message-id: <20200510150452.10307-78-ahleihel@redhat.com>
|
||
|
|
Patchwork-id: 306701
|
||
|
|
Patchwork-instance: patchwork
|
||
|
|
O-Subject: [RHEL8.3 BZ 1789380 v2 77/87] net/mlx5e: Avoid duplicating rule destinations
|
||
|
|
Bugzilla: 1789380
|
||
|
|
RH-Acked-by: Kamal Heib <kheib@redhat.com>
|
||
|
|
RH-Acked-by: Jarod Wilson <jarod@redhat.com>
|
||
|
|
RH-Acked-by: Tony Camuso <tcamuso@redhat.com>
|
||
|
|
RH-Acked-by: Jonathan Toppins <jtoppins@redhat.com>
|
||
|
|
|
||
|
|
Bugzilla: http://bugzilla.redhat.com/1789380
|
||
|
|
Upstream: v5.5-rc6
|
||
|
|
Conflicts:
|
||
|
|
- drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
|
||
|
|
Context diff due to missing commit:
|
||
|
|
84179981317f ("net/mlx5: TC: Offload flow table rules")
|
||
|
|
---> Local bool ft_flow variable does not exist yet in function
|
||
|
|
parse_tc_fdb_actions
|
||
|
|
|
||
|
|
commit 554fe75c1b3f679b1eebf193a4e56492837d3f5a
|
||
|
|
Author: Dmytro Linkin <dmitrolin@mellanox.com>
|
||
|
|
Date: Thu Oct 31 18:15:51 2019 +0200
|
||
|
|
|
||
|
|
net/mlx5e: Avoid duplicating rule destinations
|
||
|
|
|
||
|
|
Following scenario easily break driver logic and crash the kernel:
|
||
|
|
1. Add rule with mirred actions to same device.
|
||
|
|
2. Delete this rule.
|
||
|
|
In described scenario rule is not added to database and on deletion
|
||
|
|
driver access invalid entry.
|
||
|
|
Example:
|
||
|
|
|
||
|
|
$ tc filter add dev ens1f0_0 ingress protocol ip prio 1 \
|
||
|
|
flower skip_sw \
|
||
|
|
action mirred egress mirror dev ens1f0_1 pipe \
|
||
|
|
action mirred egress redirect dev ens1f0_1
|
||
|
|
$ tc filter del dev ens1f0_0 ingress protocol ip prio 1
|
||
|
|
|
||
|
|
Dmesg output:
|
||
|
|
|
||
|
|
[ 376.634396] mlx5_core 0000:82:00.0: mlx5_cmd_check:756:(pid 3439): DESTROY_FLOW_GROUP(0x934) op_mod(0x0) failed, status bad resource state(0x9), syndrome (0x563e2f)
|
||
|
|
[ 376.654983] mlx5_core 0000:82:00.0: del_hw_flow_group:567:(pid 3439): flow steering can't destroy fg 89 of ft 3145728
|
||
|
|
[ 376.673433] kasan: CONFIG_KASAN_INLINE enabled
|
||
|
|
[ 376.683769] kasan: GPF could be caused by NULL-ptr deref or user memory access
|
||
|
|
[ 376.695229] general protection fault: 0000 [#1] PREEMPT SMP KASAN PTI
|
||
|
|
[ 376.705069] CPU: 7 PID: 3439 Comm: tc Not tainted 5.4.0-rc5+ #76
|
||
|
|
[ 376.714959] Hardware name: Supermicro SYS-2028TP-DECTR/X10DRT-PT, BIOS 2.0a 08/12/2016
|
||
|
|
[ 376.726371] RIP: 0010:mlx5_del_flow_rules+0x105/0x960 [mlx5_core]
|
||
|
|
[ 376.735817] Code: 01 00 00 00 48 83 eb 08 e8 28 d9 ff ff 4c 39 e3 75 d8 4c 8d bd c0 02 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e 84 04 00 00 48 8d 7d 28 8b 9 d
|
||
|
|
[ 376.761261] RSP: 0018:ffff888847c56db8 EFLAGS: 00010202
|
||
|
|
[ 376.770054] RAX: dffffc0000000000 RBX: ffff8888582a6da0 RCX: ffff888847c56d60
|
||
|
|
[ 376.780743] RDX: 0000000000000058 RSI: 0000000000000008 RDI: 0000000000000282
|
||
|
|
[ 376.791328] RBP: 0000000000000000 R08: fffffbfff0c60ea6 R09: fffffbfff0c60ea6
|
||
|
|
[ 376.802050] R10: fffffbfff0c60ea5 R11: ffffffff8630752f R12: ffff8888582a6da0
|
||
|
|
[ 376.812798] R13: dffffc0000000000 R14: ffff8888582a6da0 R15: 00000000000002c0
|
||
|
|
[ 376.823445] FS: 00007f675f9a8840(0000) GS:ffff88886d200000(0000) knlGS:0000000000000000
|
||
|
|
[ 376.834971] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
|
||
|
|
[ 376.844179] CR2: 00000000007d9640 CR3: 00000007d3f26003 CR4: 00000000001606e0
|
||
|
|
[ 376.854843] Call Trace:
|
||
|
|
[ 376.868542] __mlx5_eswitch_del_rule+0x49/0x300 [mlx5_core]
|
||
|
|
[ 376.877735] mlx5e_tc_del_fdb_flow+0x6ec/0x9e0 [mlx5_core]
|
||
|
|
[ 376.921549] mlx5e_flow_put+0x2b/0x50 [mlx5_core]
|
||
|
|
[ 376.929813] mlx5e_delete_flower+0x5b6/0xbd0 [mlx5_core]
|
||
|
|
[ 376.973030] tc_setup_cb_reoffload+0x29/0xc0
|
||
|
|
[ 376.980619] fl_reoffload+0x50a/0x770 [cls_flower]
|
||
|
|
[ 377.015087] tcf_block_playback_offloads+0xbd/0x250
|
||
|
|
[ 377.033400] tcf_block_setup+0x1b2/0xc60
|
||
|
|
[ 377.057247] tcf_block_offload_cmd+0x195/0x240
|
||
|
|
[ 377.098826] tcf_block_offload_unbind+0xe7/0x180
|
||
|
|
[ 377.107056] __tcf_block_put+0xe5/0x400
|
||
|
|
[ 377.114528] ingress_destroy+0x3d/0x60 [sch_ingress]
|
||
|
|
[ 377.122894] qdisc_destroy+0xf1/0x5a0
|
||
|
|
[ 377.129993] qdisc_graft+0xa3d/0xe50
|
||
|
|
[ 377.151227] tc_get_qdisc+0x48e/0xa20
|
||
|
|
[ 377.165167] rtnetlink_rcv_msg+0x35d/0x8d0
|
||
|
|
[ 377.199528] netlink_rcv_skb+0x11e/0x340
|
||
|
|
[ 377.219638] netlink_unicast+0x408/0x5b0
|
||
|
|
[ 377.239913] netlink_sendmsg+0x71b/0xb30
|
||
|
|
[ 377.267505] sock_sendmsg+0xb1/0xf0
|
||
|
|
[ 377.273801] ___sys_sendmsg+0x635/0x900
|
||
|
|
[ 377.312784] __sys_sendmsg+0xd3/0x170
|
||
|
|
[ 377.338693] do_syscall_64+0x95/0x460
|
||
|
|
[ 377.344833] entry_SYSCALL_64_after_hwframe+0x49/0xbe
|
||
|
|
[ 377.352321] RIP: 0033:0x7f675e58e090
|
||
|
|
|
||
|
|
To avoid this, for every mirred action check if output device was
|
||
|
|
already processed. If so - drop rule with EOPNOTSUPP error.
|
||
|
|
|
||
|
|
Signed-off-by: Dmytro Linkin <dmitrolin@mellanox.com>
|
||
|
|
Reviewed-by: Roi Dayan <roid@mellanox.com>
|
||
|
|
Reviewed-by: Vlad Buslov <vladbu@mellanox.com>
|
||
|
|
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
|
||
|
|
|
||
|
|
Signed-off-by: Alaa Hleihel <ahleihel@redhat.com>
|
||
|
|
Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
|
||
|
|
---
|
||
|
|
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 58 ++++++++++++++++++++++++-
|
||
|
|
1 file changed, 57 insertions(+), 1 deletion(-)
|
||
|
|
|
||
|
|
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
|
||
|
|
index 8afafb7eeb55..0af1d5b1e438 100644
|
||
|
|
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
|
||
|
|
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
|
||
|
|
@@ -2988,6 +2988,25 @@ static struct ip_tunnel_info *dup_tun_info(const struct ip_tunnel_info *tun_info
|
||
|
|
return kmemdup(tun_info, tun_size, GFP_KERNEL);
|
||
|
|
}
|
||
|
|
|
||
|
|
+static bool is_duplicated_encap_entry(struct mlx5e_priv *priv,
|
||
|
|
+ struct mlx5e_tc_flow *flow,
|
||
|
|
+ int out_index,
|
||
|
|
+ struct mlx5e_encap_entry *e,
|
||
|
|
+ struct netlink_ext_ack *extack)
|
||
|
|
+{
|
||
|
|
+ int i;
|
||
|
|
+
|
||
|
|
+ for (i = 0; i < out_index; i++) {
|
||
|
|
+ if (flow->encaps[i].e != e)
|
||
|
|
+ continue;
|
||
|
|
+ NL_SET_ERR_MSG_MOD(extack, "can't duplicate encap action");
|
||
|
|
+ netdev_err(priv->netdev, "can't duplicate encap action\n");
|
||
|
|
+ return true;
|
||
|
|
+ }
|
||
|
|
+
|
||
|
|
+ return false;
|
||
|
|
+}
|
||
|
|
+
|
||
|
|
static int mlx5e_attach_encap(struct mlx5e_priv *priv,
|
||
|
|
struct mlx5e_tc_flow *flow,
|
||
|
|
struct net_device *mirred_dev,
|
||
|
|
@@ -3023,6 +3042,12 @@ static int mlx5e_attach_encap(struct mlx5e_priv *priv,
|
||
|
|
|
||
|
|
/* must verify if encap is valid or not */
|
||
|
|
if (e) {
|
||
|
|
+ /* Check that entry was not already attached to this flow */
|
||
|
|
+ if (is_duplicated_encap_entry(priv, flow, out_index, e, extack)) {
|
||
|
|
+ err = -EOPNOTSUPP;
|
||
|
|
+ goto out_err;
|
||
|
|
+ }
|
||
|
|
+
|
||
|
|
mutex_unlock(&esw->offloads.encap_tbl_lock);
|
||
|
|
wait_for_completion(&e->res_ready);
|
||
|
|
|
||
|
|
@@ -3209,6 +3234,26 @@ bool mlx5e_is_valid_eswitch_fwd_dev(struct mlx5e_priv *priv,
|
||
|
|
same_hw_devs(priv, netdev_priv(out_dev));
|
||
|
|
}
|
||
|
|
|
||
|
|
+static bool is_duplicated_output_device(struct net_device *dev,
|
||
|
|
+ struct net_device *out_dev,
|
||
|
|
+ int *ifindexes, int if_count,
|
||
|
|
+ struct netlink_ext_ack *extack)
|
||
|
|
+{
|
||
|
|
+ int i;
|
||
|
|
+
|
||
|
|
+ for (i = 0; i < if_count; i++) {
|
||
|
|
+ if (ifindexes[i] == out_dev->ifindex) {
|
||
|
|
+ NL_SET_ERR_MSG_MOD(extack,
|
||
|
|
+ "can't duplicate output to same device");
|
||
|
|
+ netdev_err(dev, "can't duplicate output to same device: %s\n",
|
||
|
|
+ out_dev->name);
|
||
|
|
+ return true;
|
||
|
|
+ }
|
||
|
|
+ }
|
||
|
|
+
|
||
|
|
+ return false;
|
||
|
|
+}
|
||
|
|
+
|
||
|
|
static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
|
||
|
|
struct flow_action *flow_action,
|
||
|
|
struct mlx5e_tc_flow *flow,
|
||
|
|
@@ -3220,10 +3265,11 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
|
||
|
|
struct mlx5e_tc_flow_parse_attr *parse_attr = attr->parse_attr;
|
||
|
|
struct mlx5e_rep_priv *rpriv = priv->ppriv;
|
||
|
|
const struct ip_tunnel_info *info = NULL;
|
||
|
|
+ int ifindexes[MLX5_MAX_FLOW_FWD_VPORTS];
|
||
|
|
const struct flow_action_entry *act;
|
||
|
|
+ int err, i, if_count = 0;
|
||
|
|
bool encap = false;
|
||
|
|
u32 action = 0;
|
||
|
|
- int err, i;
|
||
|
|
|
||
|
|
if (!flow_action_has_entries(flow_action))
|
||
|
|
return -EINVAL;
|
||
|
|
@@ -3292,6 +3338,16 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
|
||
|
|
struct net_device *uplink_dev = mlx5_eswitch_uplink_get_proto_dev(esw, REP_ETH);
|
||
|
|
struct net_device *uplink_upper;
|
||
|
|
|
||
|
|
+ if (is_duplicated_output_device(priv->netdev,
|
||
|
|
+ out_dev,
|
||
|
|
+ ifindexes,
|
||
|
|
+ if_count,
|
||
|
|
+ extack))
|
||
|
|
+ return -EOPNOTSUPP;
|
||
|
|
+
|
||
|
|
+ ifindexes[if_count] = out_dev->ifindex;
|
||
|
|
+ if_count++;
|
||
|
|
+
|
||
|
|
rcu_read_lock();
|
||
|
|
uplink_upper =
|
||
|
|
netdev_master_upper_dev_get_rcu(uplink_dev);
|
||
|
|
--
|
||
|
|
2.13.6
|
||
|
|
|