Hi Martin,

Thanks for your patch.

It looks good to me, in general.
I just have one small comment below.

On 28/01/2017 9:40 AM, Martin KaFai Lau wrote:
If the rx-queues ever get re-initialized (e.g. by changing the
number of rx-queues with ethtool -L), the existing xdp_prog becomes
inactive.

The bug is that the xdp_prog ptr has not been carried over from
the old rx-queues to the new rx-queues

Fixes: 47a38e155037 ("net/mlx4_en: add support for fast rx drop bpf program")
Signed-off-by: Martin KaFai Lau <ka...@fb.com>
---
...
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c 
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 761f8b12399c..f4179086b3c6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2184,23 +2184,57 @@ static void mlx4_en_update_priv(struct mlx4_en_priv 
*dst,
int mlx4_en_try_alloc_resources(struct mlx4_en_priv *priv,
                                struct mlx4_en_priv *tmp,
-                               struct mlx4_en_port_profile *prof)
+                               struct mlx4_en_port_profile *prof,
+                               bool carry_xdp_prog)
  {
-       int t;
+       struct bpf_prog *xdp_prog = NULL;
+       int err;
+       int i;
mlx4_en_copy_priv(tmp, priv, prof); + if (carry_xdp_prog) {
+               /* All rx_rings has the same xdp_prog.  Pick the first one */
+               xdp_prog = rcu_dereference_protected(
+                       priv->rx_ring[0]->xdp_prog,
+                       lockdep_is_held(&priv->mdev->state_lock));
+
+               if (xdp_prog) {
+                       xdp_prog = bpf_prog_add(xdp_prog, tmp->rx_ring_num);
+                       if (IS_ERR(xdp_prog)) {
+                               err = PTR_ERR(xdp_prog);
+                               xdp_prog = NULL;
+                               goto err_free;
+                       }
+               }
+       }
Why do you prefer dealing with xdp_prog in two stages? You can handle it all at once, after "mlx4_en_alloc_resources()" succeeds.
+
        if (mlx4_en_alloc_resources(tmp)) {
                en_warn(priv,
                        "%s: Resource allocation failed, using previous 
configuration\n",
                        __func__);
-               for (t = 0; t < MLX4_EN_NUM_TX_TYPES; t++) {
-                       kfree(tmp->tx_ring[t]);
-                       kfree(tmp->tx_cq[t]);
-               }
-               return -ENOMEM;
+               err = -ENOMEM;
+               goto err_free;
+       }
+
+       if (xdp_prog) {
+               for (i = 0; i < tmp->rx_ring_num; i++)
+                       rcu_assign_pointer(tmp->rx_ring[i]->xdp_prog,
+                                          xdp_prog);
        }
+
        return 0;
+
+err_free:
+       if (xdp_prog)
+               bpf_prog_sub(xdp_prog, tmp->rx_ring_num);
+
+       for (i = 0; i < MLX4_EN_NUM_TX_TYPES; i++) {
+               kfree(tmp->tx_ring[i]);
+               kfree(tmp->tx_cq[i]);
+       }
+
+       return err;
  }
void mlx4_en_safe_replace_resources(struct mlx4_en_priv *priv,

Regards,
Tariq Toukan.

Reply via email to