mlxsw: spectrum: Avoid copying sample values and use RCU pointer direcly instead

Currently, only the psample_group is accessed using RCU on RX path.
However, it is possible (unlikely) that other sample values get change
during RX processing. Fix this by having the port->sample struct
accessed as RCU pointer, containing all sample values including
psample_group pointer. That avoids extra alloc per-port, copying the
values and the race condition described above.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index ff25f8f..5952ec2 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3527,13 +3527,6 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
                goto err_alloc_stats;
        }
 
-       mlxsw_sp_port->sample = kzalloc(sizeof(*mlxsw_sp_port->sample),
-                                       GFP_KERNEL);
-       if (!mlxsw_sp_port->sample) {
-               err = -ENOMEM;
-               goto err_alloc_sample;
-       }
-
        INIT_DELAYED_WORK(&mlxsw_sp_port->periodic_hw_stats.update_dw,
                          &update_stats_cache);
 
@@ -3720,8 +3713,6 @@ err_dev_addr_init:
 err_port_swid_set:
        mlxsw_sp_port_module_unmap(mlxsw_sp_port);
 err_port_module_map:
-       kfree(mlxsw_sp_port->sample);
-err_alloc_sample:
        free_percpu(mlxsw_sp_port->pcpu_stats);
 err_alloc_stats:
        free_netdev(dev);
@@ -3749,7 +3740,6 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u8 local_port)
        mlxsw_sp_port_tc_mc_mode_set(mlxsw_sp_port, false);
        mlxsw_sp_port_swid_set(mlxsw_sp_port, MLXSW_PORT_SWID_DISABLED_PORT);
        mlxsw_sp_port_module_unmap(mlxsw_sp_port);
-       kfree(mlxsw_sp_port->sample);
        free_percpu(mlxsw_sp_port->pcpu_stats);
        WARN_ON_ONCE(!list_empty(&mlxsw_sp_port->vlans_list));
        free_netdev(mlxsw_sp_port->dev);
@@ -4236,7 +4226,7 @@ static void mlxsw_sp_rx_listener_sample_func(struct sk_buff *skb, u8 local_port,
 {
        struct mlxsw_sp *mlxsw_sp = priv;
        struct mlxsw_sp_port *mlxsw_sp_port = mlxsw_sp->ports[local_port];
-       struct psample_group *psample_group;
+       struct mlxsw_sp_port_sample *sample;
        u32 size;
 
        if (unlikely(!mlxsw_sp_port)) {
@@ -4244,22 +4234,14 @@ static void mlxsw_sp_rx_listener_sample_func(struct sk_buff *skb, u8 local_port,
                                     local_port);
                goto out;
        }
-       if (unlikely(!mlxsw_sp_port->sample)) {
-               dev_warn_ratelimited(mlxsw_sp->bus_info->dev, "Port %d: sample skb received on unsupported port\n",
-                                    local_port);
-               goto out;
-       }
-
-       size = mlxsw_sp_port->sample->truncate ?
-                 mlxsw_sp_port->sample->trunc_size : skb->len;
 
        rcu_read_lock();
-       psample_group = rcu_dereference(mlxsw_sp_port->sample->psample_group);
-       if (!psample_group)
+       sample = rcu_dereference(mlxsw_sp_port->sample);
+       if (!sample)
                goto out_unlock;
-       psample_sample_packet(psample_group, skb, size,
-                             mlxsw_sp_port->dev->ifindex, 0,
-                             mlxsw_sp_port->sample->rate);
+       size = sample->truncate ? sample->trunc_size : skb->len;
+       psample_sample_packet(sample->psample_group, skb, size,
+                             mlxsw_sp_port->dev->ifindex, 0, sample->rate);
 out_unlock:
        rcu_read_unlock();
 out:

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 5c2f1af..4cdb7f1 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -192,7 +192,7 @@ struct mlxsw_sp_port_pcpu_stats {
 };
 
 struct mlxsw_sp_port_sample {
-       struct psample_group __rcu *psample_group;
+       struct psample_group *psample_group;
        u32 trunc_size;
        u32 rate;
        bool truncate;
@@ -262,7 +262,7 @@ struct mlxsw_sp_port {
                struct mlxsw_sp_port_xstats xstats;
                struct delayed_work update_dw;
        } periodic_hw_stats;
-       struct mlxsw_sp_port_sample *sample;
+       struct mlxsw_sp_port_sample __rcu *sample;
        struct list_head vlans_list;
        struct mlxsw_sp_port_vlan *default_vlan;
        struct mlxsw_sp_qdisc_state *qdisc;

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_matchall.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_matchall.c
index 4130102..bda5fb3 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_matchall.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_matchall.c
@@ -29,6 +29,7 @@ struct mlxsw_sp_mall_entry {
                struct mlxsw_sp_mall_mirror_entry mirror;
                struct mlxsw_sp_port_sample sample;
        };
+       struct rcu_head rcu;
 };
 
 static struct mlxsw_sp_mall_entry *
@@ -90,17 +91,11 @@ mlxsw_sp_mall_port_sample_add(struct mlxsw_sp_port *mlxsw_sp_port,
 {
        int err;
 
-       if (!mlxsw_sp_port->sample)
-               return -EOPNOTSUPP;
-       if (rtnl_dereference(mlxsw_sp_port->sample->psample_group)) {
+       if (rtnl_dereference(mlxsw_sp_port->sample)) {
                netdev_err(mlxsw_sp_port->dev, "sample already active\n");
                return -EEXIST;
        }
-       rcu_assign_pointer(mlxsw_sp_port->sample->psample_group,
-                          mall_entry->sample.psample_group);
-       mlxsw_sp_port->sample->truncate = mall_entry->sample.truncate;
-       mlxsw_sp_port->sample->trunc_size = mall_entry->sample.trunc_size;
-       mlxsw_sp_port->sample->rate = mall_entry->sample.rate;
+       rcu_assign_pointer(mlxsw_sp_port->sample, &mall_entry->sample);
 
        err = mlxsw_sp_mall_port_sample_set(mlxsw_sp_port, true,
                                            mall_entry->sample.rate);
@@ -109,7 +104,7 @@ mlxsw_sp_mall_port_sample_add(struct mlxsw_sp_port *mlxsw_sp_port,
        return 0;
 
 err_port_sample_set:
-       RCU_INIT_POINTER(mlxsw_sp_port->sample->psample_group, NULL);
+       RCU_INIT_POINTER(mlxsw_sp_port->sample, NULL);
        return err;
 }
 
@@ -120,7 +115,7 @@ mlxsw_sp_mall_port_sample_del(struct mlxsw_sp_port *mlxsw_sp_port)
                return;
 
        mlxsw_sp_mall_port_sample_set(mlxsw_sp_port, false, 1);
-       RCU_INIT_POINTER(mlxsw_sp_port->sample->psample_group, NULL);
+       RCU_INIT_POINTER(mlxsw_sp_port->sample, NULL);
 }
 
 static int
@@ -221,5 +216,5 @@ void mlxsw_sp_mall_destroy(struct mlxsw_sp_port *mlxsw_sp_port,
        mlxsw_sp_mall_port_rule_del(mlxsw_sp_port, mall_entry);
 
        list_del(&mall_entry->list);
-       kfree(mall_entry);
+       kfree_rcu(mall_entry, rcu); /* sample RX packets may be in-flight */
 }