Commit 806401c2 authored by Roi Dayan's avatar Roi Dayan Committed by Saeed Mahameed

net/mlx5e: CT, Fix multiple allocations and memleak of mod acts

CT clear action offload adds additional mod hdr actions to the
flow's original mod actions in order to clear the registers which
hold ct_state.
When such flow also includes encap action, a neigh update event
can cause the driver to unoffload the flow and then reoffload it.

Each time this happens, the ct clear handling adds that same set
of mod hdr actions to reset ct_state until the max of mod hdr
actions is reached.

Also the driver never releases the allocated mod hdr actions and
causing a memleak.

Fix above two issues by moving CT clear mod acts allocation
into the parsing actions phase and only use it when offloading the rule.
The release of mod acts will be done in the normal flow_put().

 backtrace:
    [<000000007316e2f3>] krealloc+0x83/0xd0
    [<00000000ef157de1>] mlx5e_mod_hdr_alloc+0x147/0x300 [mlx5_core]
    [<00000000970ce4ae>] mlx5e_tc_match_to_reg_set_and_get_id+0xd7/0x240 [mlx5_core]
    [<0000000067c5fa17>] mlx5e_tc_match_to_reg_set+0xa/0x20 [mlx5_core]
    [<00000000d032eb98>] mlx5_tc_ct_entry_set_registers.isra.0+0x36/0xc0 [mlx5_core]
    [<00000000fd23b869>] mlx5_tc_ct_flow_offload+0x272/0x1f10 [mlx5_core]
    [<000000004fc24acc>] mlx5e_tc_offload_fdb_rules.part.0+0x150/0x620 [mlx5_core]
    [<00000000dc741c17>] mlx5e_tc_encap_flows_add+0x489/0x690 [mlx5_core]
    [<00000000e92e49d7>] mlx5e_rep_update_flows+0x6e4/0x9b0 [mlx5_core]
    [<00000000f60f5602>] mlx5e_rep_neigh_update+0x39a/0x5d0 [mlx5_core]

Fixes: 1ef3018f ("net/mlx5e: CT: Support clear action")
Signed-off-by: default avatarRoi Dayan <roid@nvidia.com>
Reviewed-by: default avatarPaul Blakey <paulb@nvidia.com>
Reviewed-by: default avatarMaor Dickman <maord@nvidia.com>
Signed-off-by: default avatarSaeed Mahameed <saeedm@nvidia.com>
parent 38a54cae
...@@ -1356,9 +1356,13 @@ mlx5_tc_ct_match_add(struct mlx5_tc_ct_priv *priv, ...@@ -1356,9 +1356,13 @@ mlx5_tc_ct_match_add(struct mlx5_tc_ct_priv *priv,
int int
mlx5_tc_ct_parse_action(struct mlx5_tc_ct_priv *priv, mlx5_tc_ct_parse_action(struct mlx5_tc_ct_priv *priv,
struct mlx5_flow_attr *attr, struct mlx5_flow_attr *attr,
struct mlx5e_tc_mod_hdr_acts *mod_acts,
const struct flow_action_entry *act, const struct flow_action_entry *act,
struct netlink_ext_ack *extack) struct netlink_ext_ack *extack)
{ {
bool clear_action = act->ct.action & TCA_CT_ACT_CLEAR;
int err;
if (!priv) { if (!priv) {
NL_SET_ERR_MSG_MOD(extack, NL_SET_ERR_MSG_MOD(extack,
"offload of ct action isn't available"); "offload of ct action isn't available");
...@@ -1369,6 +1373,17 @@ mlx5_tc_ct_parse_action(struct mlx5_tc_ct_priv *priv, ...@@ -1369,6 +1373,17 @@ mlx5_tc_ct_parse_action(struct mlx5_tc_ct_priv *priv,
attr->ct_attr.ct_action = act->ct.action; attr->ct_attr.ct_action = act->ct.action;
attr->ct_attr.nf_ft = act->ct.flow_table; attr->ct_attr.nf_ft = act->ct.flow_table;
if (!clear_action)
goto out;
err = mlx5_tc_ct_entry_set_registers(priv, mod_acts, 0, 0, 0, 0);
if (err) {
NL_SET_ERR_MSG_MOD(extack, "Failed to set registers for ct clear");
return err;
}
attr->action |= MLX5_FLOW_CONTEXT_ACTION_MOD_HDR;
out:
return 0; return 0;
} }
...@@ -1898,23 +1913,16 @@ __mlx5_tc_ct_flow_offload_clear(struct mlx5_tc_ct_priv *ct_priv, ...@@ -1898,23 +1913,16 @@ __mlx5_tc_ct_flow_offload_clear(struct mlx5_tc_ct_priv *ct_priv,
memcpy(pre_ct_attr, attr, attr_sz); memcpy(pre_ct_attr, attr, attr_sz);
err = mlx5_tc_ct_entry_set_registers(ct_priv, mod_acts, 0, 0, 0, 0);
if (err) {
ct_dbg("Failed to set register for ct clear");
goto err_set_registers;
}
mod_hdr = mlx5_modify_header_alloc(priv->mdev, ct_priv->ns_type, mod_hdr = mlx5_modify_header_alloc(priv->mdev, ct_priv->ns_type,
mod_acts->num_actions, mod_acts->num_actions,
mod_acts->actions); mod_acts->actions);
if (IS_ERR(mod_hdr)) { if (IS_ERR(mod_hdr)) {
err = PTR_ERR(mod_hdr); err = PTR_ERR(mod_hdr);
ct_dbg("Failed to add create ct clear mod hdr"); ct_dbg("Failed to add create ct clear mod hdr");
goto err_set_registers; goto err_mod_hdr;
} }
pre_ct_attr->modify_hdr = mod_hdr; pre_ct_attr->modify_hdr = mod_hdr;
pre_ct_attr->action |= MLX5_FLOW_CONTEXT_ACTION_MOD_HDR;
rule = mlx5_tc_rule_insert(priv, orig_spec, pre_ct_attr); rule = mlx5_tc_rule_insert(priv, orig_spec, pre_ct_attr);
if (IS_ERR(rule)) { if (IS_ERR(rule)) {
...@@ -1930,7 +1938,7 @@ __mlx5_tc_ct_flow_offload_clear(struct mlx5_tc_ct_priv *ct_priv, ...@@ -1930,7 +1938,7 @@ __mlx5_tc_ct_flow_offload_clear(struct mlx5_tc_ct_priv *ct_priv,
err_insert: err_insert:
mlx5_modify_header_dealloc(priv->mdev, mod_hdr); mlx5_modify_header_dealloc(priv->mdev, mod_hdr);
err_set_registers: err_mod_hdr:
netdev_warn(priv->netdev, netdev_warn(priv->netdev,
"Failed to offload ct clear flow, err %d\n", err); "Failed to offload ct clear flow, err %d\n", err);
kfree(pre_ct_attr); kfree(pre_ct_attr);
......
...@@ -110,6 +110,7 @@ int mlx5_tc_ct_add_no_trk_match(struct mlx5_flow_spec *spec); ...@@ -110,6 +110,7 @@ int mlx5_tc_ct_add_no_trk_match(struct mlx5_flow_spec *spec);
int int
mlx5_tc_ct_parse_action(struct mlx5_tc_ct_priv *priv, mlx5_tc_ct_parse_action(struct mlx5_tc_ct_priv *priv,
struct mlx5_flow_attr *attr, struct mlx5_flow_attr *attr,
struct mlx5e_tc_mod_hdr_acts *mod_acts,
const struct flow_action_entry *act, const struct flow_action_entry *act,
struct netlink_ext_ack *extack); struct netlink_ext_ack *extack);
...@@ -172,6 +173,7 @@ mlx5_tc_ct_add_no_trk_match(struct mlx5_flow_spec *spec) ...@@ -172,6 +173,7 @@ mlx5_tc_ct_add_no_trk_match(struct mlx5_flow_spec *spec)
static inline int static inline int
mlx5_tc_ct_parse_action(struct mlx5_tc_ct_priv *priv, mlx5_tc_ct_parse_action(struct mlx5_tc_ct_priv *priv,
struct mlx5_flow_attr *attr, struct mlx5_flow_attr *attr,
struct mlx5e_tc_mod_hdr_acts *mod_acts,
const struct flow_action_entry *act, const struct flow_action_entry *act,
struct netlink_ext_ack *extack) struct netlink_ext_ack *extack)
{ {
......
...@@ -3608,7 +3608,9 @@ parse_tc_nic_actions(struct mlx5e_priv *priv, ...@@ -3608,7 +3608,9 @@ parse_tc_nic_actions(struct mlx5e_priv *priv,
attr->dest_chain = act->chain_index; attr->dest_chain = act->chain_index;
break; break;
case FLOW_ACTION_CT: case FLOW_ACTION_CT:
err = mlx5_tc_ct_parse_action(get_ct_priv(priv), attr, act, extack); err = mlx5_tc_ct_parse_action(get_ct_priv(priv), attr,
&parse_attr->mod_hdr_acts,
act, extack);
if (err) if (err)
return err; return err;
...@@ -4277,7 +4279,9 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, ...@@ -4277,7 +4279,9 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
NL_SET_ERR_MSG_MOD(extack, "Sample action with connection tracking is not supported"); NL_SET_ERR_MSG_MOD(extack, "Sample action with connection tracking is not supported");
return -EOPNOTSUPP; return -EOPNOTSUPP;
} }
err = mlx5_tc_ct_parse_action(get_ct_priv(priv), attr, act, extack); err = mlx5_tc_ct_parse_action(get_ct_priv(priv), attr,
&parse_attr->mod_hdr_acts,
act, extack);
if (err) if (err)
return err; return err;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment