Patch "net/mlx5e: Properly deal with encap flows add/del under neigh update" has been added to the 4.13-stable tree
This is a note to let you know that I've just added the patch titled
net/mlx5e: Properly deal with encap flows add/del under neigh update
to the 4.13-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
net-mlx5e-properly-deal-with-encap-flows-add-del-under-neigh-update.patch
and it can be found in the queue-4.13 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
Quoted 1 line
From foo@baz Wed Nov 15 17:25:34 CET 2017
From: Or Gerlitz <[email protected]>
Date: Tue, 17 Oct 2017 12:33:43 +0200
Subject: net/mlx5e: Properly deal with encap flows add/del under neigh update
From: Or Gerlitz <[email protected]>
[ Upstream commit 3c37745ec614ff048d5dce38f976804b05d307ee ]
Currently, the encap action offload is handled in the actions parse function and not in mlx5e_tc_add_fdb_flow() where we deal with all the other aspects of offloading actions (vlan, modify header) and the rule itself.
When the neigh update code (mlx5e_tc_encap_flows_add()) recreates the encap entry and offloads the related flows, we wrongly call again into mlx5e_tc_add_fdb_flow(), this for itself would cause us to handle again the offloading of vlans and header re-write which puts things in non consistent state and step on freed memory (e.g the modify header parse buffer which is already freed).
Since on error, mlx5e_tc_add_fdb_flow() detaches and may release the encap entry, it causes a corruption at the neigh update code which goes over the list of flows associated with this encap entry, or double free when the tc flow is later deleted by user-space.
When neigh update (mlx5e_tc_encap_flows_del()) unoffloads the flows related to an encap entry which is now invalid, we do a partial repeat of the eswitch flow removal code which is wrong too.
To fix things up we do the following:
(1) handle the encap action offload in the eswitch flow add function mlx5e_tc_add_fdb_flow() as done for the other actions and the rule itself.
(2) modify the neigh update code (mlx5e_tc_encap_flows_add/del) to only deal with the encap entry and rules delete/add and not with any of the other offloaded actions.
--- drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 89 ++++++++++++++---------- 1 file changed, 54 insertions(+), 35 deletions(-)
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c +54 -35
@@ -78,9 +78,11 @@ struct mlx5e_tc_flow { };struct mlx5e_tc_flow_parse_attr { +struct ip_tunnel_info tun_info;struct mlx5_flow_spec spec;int num_mod_hdr_actions;void *mod_hdr_actions; +int mirred_ifindex; };enum {
@@ -322,6 +324,12 @@ static void mlx5e_tc_del_nic_flow(structstatic void mlx5e_detach_encap (struct mlx5e_priv *priv,struct mlx5e_tc_flow *flow); +static int mlx5e_attach_encap (struct mlx5e_priv *priv, +struct ip_tunnel_info *tun_info, +struct net_device *mirred_dev, +struct net_device **encap_dev, +struct mlx5e_tc_flow *flow); +static struct mlx5_flow_handle * mlx5e_tc_add_fdb_flow(struct mlx5e_priv *priv, struct mlx5e_tc_flow_parse_attr *parse_attr,
@@ -329,9 +337,27 @@ mlx5e_tc_add_fdb_flow(struct mlx5e_priv {struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;struct mlx5_esw_flow_attr *attr = flow->esw_attr; -struct mlx5_flow_handle *rule; +struct net_device *out_dev, *encap_dev =NULL ; +struct mlx5_flow_handle *rule =NULL ; +struct mlx5e_rep_priv *rpriv; +struct mlx5e_priv *out_priv;int err; +if (attr->action &MLX5_FLOW_CONTEXT_ACTION_ENCAP ) { + out_dev =__dev_get_by_index (dev_net (priv->netdev), + attr->parse_attr->mirred_ifindex); + err =mlx5e_attach_encap (priv, &parse_attr->tun_info, + out_dev, &encap_dev, flow); +if (err) { + rule =ERR_PTR (err); +if (err != -EAGAIN ) + goto err_attach_encap; + } + out_priv =netdev_priv (encap_dev); + rpriv = out_priv->ppriv; + attr->out_rep = rpriv->rep; + } + err =mlx5_eswitch_add_vlan_action (esw, attr);if (err) { rule =ERR_PTR (err);
@@ -347,10 +373,14 @@ mlx5e_tc_add_fdb_flow(struct mlx5e_priv } } - rule =mlx5_eswitch_add_offloaded_rule (esw, &parse_attr->spec, attr); -if (IS_ERR (rule)) - goto err_add_rule; - +/* we get here if (1) there's no error (rule being null) or when +* (2) there's an encap action and we're on -EAGAIN (no valid neigh) +*/ +if (rule !=ERR_PTR (-EAGAIN )) { + rule =mlx5_eswitch_add_offloaded_rule (esw, &parse_attr->spec, attr); +if (IS_ERR (rule)) + goto err_add_rule; + }return rule; err_add_rule:
@@ -361,6 +391,7 @@ err_mod_hdr: err_add_vlan:if (attr->action &MLX5_FLOW_CONTEXT_ACTION_ENCAP )mlx5e_detach_encap (priv, flow); +err_attach_encap:return rule; }
@@ -389,6 +420,8 @@ static void mlx5e_tc_del_fdb_flow(structvoid mlx5e_tc_encap_flows_add (struct mlx5e_priv *priv,struct mlx5e_encap_entry *e) { +struct mlx5_eswitch *esw = priv->mdev->priv.eswitch; +struct mlx5_esw_flow_attr *esw_attr;struct mlx5e_tc_flow *flow;int err;
@@ -404,10 +437,9 @@ void mlx5e_tc_encap_flows_add(struct mlxmlx5e_rep_queue_neigh_stats_work (priv);list_for_each_entry (flow, &e->flows, encap) { - flow->esw_attr->encap_id = e->encap_id; - flow->rule =mlx5e_tc_add_fdb_flow (priv, - flow->esw_attr->parse_attr, - flow); + esw_attr = flow->esw_attr; + esw_attr->encap_id = e->encap_id; + flow->rule =mlx5_eswitch_add_offloaded_rule (esw, &esw_attr->parse_attr->spec, esw_attr);if (IS_ERR (flow->rule)) { err =PTR_ERR (flow->rule); mlx5_core_warn(priv->mdev, "Failed to update cached encapsulation flow, %d\n",
@@ -421,15 +453,13 @@ void mlx5e_tc_encap_flows_add(struct mlxvoid mlx5e_tc_encap_flows_del (struct mlx5e_priv *priv,struct mlx5e_encap_entry *e) { +struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;struct mlx5e_tc_flow *flow; -struct mlx5_fc *counter;list_for_each_entry (flow, &e->flows, encap) {if (flow->flags &MLX5E_TC_FLOW_OFFLOADED ) { flow->flags &= ~MLX5E_TC_FLOW_OFFLOADED ; - counter =mlx5_flow_rule_counter (flow->rule); -mlx5_del_flow_rules (flow->rule); -mlx5_fc_destroy (priv->mdev, counter); +mlx5_eswitch_del_offloaded_rule (esw, flow->rule, flow->esw_attr); } }
@@ -1871,7 +1901,7 @@ static int parse_tc_fdb_actions(struct mif (is_tcf_mirred_egress_redirect (a)) {int ifindex =tcf_mirred_ifindex (a); -struct net_device *out_dev, *encap_dev =NULL ; +struct net_device *out_dev;struct mlx5e_priv *out_priv; out_dev =__dev_get_by_index (dev_net (priv->netdev), ifindex);
@@ -1884,17 +1914,13 @@ static int parse_tc_fdb_actions(struct m rpriv = out_priv->ppriv; attr->out_rep = rpriv->rep; }else if (encap) { - err =mlx5e_attach_encap (priv, info, - out_dev, &encap_dev, flow); -if (err && err != -EAGAIN ) -return err; + parse_attr->mirred_ifindex = ifindex; + parse_attr->tun_info = *info; + attr->parse_attr = parse_attr; attr->action |=MLX5_FLOW_CONTEXT_ACTION_ENCAP |MLX5_FLOW_CONTEXT_ACTION_FWD_DEST |MLX5_FLOW_CONTEXT_ACTION_COUNT ; - out_priv =netdev_priv (encap_dev); - rpriv = out_priv->ppriv; - attr->out_rep = rpriv->rep; - attr->parse_attr = parse_attr; +/* attr->out_rep is resolved when we handle encap */ }else {pr_err ("devices %s %s not on same switch HW, can't offload forwarding\n" , priv->netdev->name, out_dev->name);
@@ -1972,7 +1998,7 @@ int mlx5e_configure_flower(struct mlx5e_if (flow->flags &MLX5E_TC_FLOW_ESWITCH ) { err =parse_tc_fdb_actions (priv, f->exts, parse_attr, flow);if (err <0 ) - goto err_handle_encap_flow; + goto err_free; flow->rule =mlx5e_tc_add_fdb_flow (priv, parse_attr, flow); }else { err =parse_tc_nic_actions (priv, f->exts, parse_attr, flow);
@@ -1983,10 +2009,13 @@ int mlx5e_configure_flower(struct mlx5e_if (IS_ERR (flow->rule)) { err =PTR_ERR (flow->rule); - goto err_free; +if (err != -EAGAIN ) + goto err_free; } - flow->flags |=MLX5E_TC_FLOW_OFFLOADED ; +if (err != -EAGAIN ) + flow->flags |=MLX5E_TC_FLOW_OFFLOADED ; + err =rhashtable_insert_fast (&tc->ht, &flow->node, tc->ht_params);if (err)
@@ -2000,16 +2029,6 @@ int mlx5e_configure_flower(struct mlx5e_ err_del_rule:mlx5e_tc_del_flow (priv, flow); -err_handle_encap_flow: -if (err == -EAGAIN ) { - err =rhashtable_insert_fast (&tc->ht, &flow->node, - tc->ht_params); -if (err) -mlx5e_tc_del_flow (priv, flow); -else -return 0 ; - } - err_free:kvfree (parse_attr);kfree (flow);
Patches currently in stable-queue which might be from [email protected] are
queue-4.13/net-mlx5e-properly-deal-with-encap-flows-add-del-under-neigh-update.patch