netdev - Re: [PATCH net-next v4 1/2] net/mlx5e: refactor indr setup block

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <vbfv9muyk1h.fsf@mellanox.com>
Date:   Mon, 23 Mar 2020 20:59:22 +0200
From:   Vlad Buslov <vladbu@...lanox.com>
To:     wenxu@...oud.cn
Cc:     saeedm@...lanox.com, paulb@...lanox.com, vladbu@...lanox.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next v4 1/2] net/mlx5e: refactor indr setup block


On Mon 23 Mar 2020 at 16:27, wenxu@...oud.cn wrote:
> From: wenxu <wenxu@...oud.cn>
>
> Refactor indr setup block for support ft indr setup in the
> next patch. The mlx5e_rep_indr_setup_block can be used for
> both tc and ft subsystem.

This doesn't really explain what is going on in this patch. For example,
it seems you are exposing 'flags' in order set additional flag for FT in
patch 2 of this series. I can only guess because commit message doesn't
provide any explanation.

>
> Signed-off-by: wenxu <wenxu@...oud.cn>
> ---
> v3: add some comments
>
>  drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 42 ++++++++++++------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> index a33d151..057f5f9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> @@ -694,9 +694,9 @@ static void mlx5e_rep_indr_clean_block_privs(struct mlx5e_rep_priv *rpriv)
>  static int
>  mlx5e_rep_indr_offload(struct net_device *netdev,
>  		       struct flow_cls_offload *flower,
> -		       struct mlx5e_rep_indr_block_priv *indr_priv)
> +		       struct mlx5e_rep_indr_block_priv *indr_priv,
> +		       unsigned long flags)
>  {
> -	unsigned long flags = MLX5_TC_FLAG(EGRESS) | MLX5_TC_FLAG(ESW_OFFLOAD);
>  	struct mlx5e_priv *priv = netdev_priv(indr_priv->rpriv->netdev);
>  	int err = 0;
>  
> @@ -717,20 +717,22 @@ static void mlx5e_rep_indr_clean_block_privs(struct mlx5e_rep_priv *rpriv)
>  	return err;
>  }
>  
> -static int mlx5e_rep_indr_setup_block_cb(enum tc_setup_type type,
> -					 void *type_data, void *indr_priv)
> +static int mlx5e_rep_indr_setup_tc_cb(enum tc_setup_type type,
> +				      void *type_data, void *indr_priv)
>  {
> +	unsigned long flags = MLX5_TC_FLAG(EGRESS) | MLX5_TC_FLAG(ESW_OFFLOAD);
>  	struct mlx5e_rep_indr_block_priv *priv = indr_priv;
>  
>  	switch (type) {
>  	case TC_SETUP_CLSFLOWER:
> -		return mlx5e_rep_indr_offload(priv->netdev, type_data, priv);
> +		return mlx5e_rep_indr_offload(priv->netdev, type_data, priv,
> +					      flags);
>  	default:
>  		return -EOPNOTSUPP;
>  	}
>  }
>  
> -static void mlx5e_rep_indr_tc_block_unbind(void *cb_priv)
> +static void mlx5e_rep_indr_block_unbind(void *cb_priv)
>  {
>  	struct mlx5e_rep_indr_block_priv *indr_priv = cb_priv;
>  
> @@ -741,9 +743,10 @@ static void mlx5e_rep_indr_tc_block_unbind(void *cb_priv)
>  static LIST_HEAD(mlx5e_block_cb_list);
>  
>  static int
> -mlx5e_rep_indr_setup_tc_block(struct net_device *netdev,
> -			      struct mlx5e_rep_priv *rpriv,
> -			      struct flow_block_offload *f)
> +mlx5e_rep_indr_setup_block(struct net_device *netdev,
> +			   struct mlx5e_rep_priv *rpriv,
> +			   struct flow_block_offload *f,
> +			   flow_setup_cb_t *setup_cb)
>  {
>  	struct mlx5e_rep_indr_block_priv *indr_priv;
>  	struct flow_block_cb *block_cb;
> @@ -769,9 +772,8 @@ static void mlx5e_rep_indr_tc_block_unbind(void *cb_priv)
>  		list_add(&indr_priv->list,
>  			 &rpriv->uplink_priv.tc_indr_block_priv_list);
>  
> -		block_cb = flow_block_cb_alloc(mlx5e_rep_indr_setup_block_cb,
> -					       indr_priv, indr_priv,
> -					       mlx5e_rep_indr_tc_block_unbind);
> +		block_cb = flow_block_cb_alloc(setup_cb, indr_priv, indr_priv,
> +					       mlx5e_rep_indr_block_unbind);
>  		if (IS_ERR(block_cb)) {
>  			list_del(&indr_priv->list);
>  			kfree(indr_priv);
> @@ -786,9 +788,7 @@ static void mlx5e_rep_indr_tc_block_unbind(void *cb_priv)
>  		if (!indr_priv)
>  			return -ENOENT;
>  
> -		block_cb = flow_block_cb_lookup(f->block,
> -						mlx5e_rep_indr_setup_block_cb,
> -						indr_priv);
> +		block_cb = flow_block_cb_lookup(f->block, setup_cb, indr_priv);
>  		if (!block_cb)
>  			return -ENOENT;
>  
> @@ -802,13 +802,13 @@ static void mlx5e_rep_indr_tc_block_unbind(void *cb_priv)
>  }
>  
>  static
> -int mlx5e_rep_indr_setup_tc_cb(struct net_device *netdev, void *cb_priv,
> -			       enum tc_setup_type type, void *type_data)
> +int mlx5e_rep_indr_setup_cb(struct net_device *netdev, void *cb_priv,
> +			    enum tc_setup_type type, void *type_data)
>  {
>  	switch (type) {
>  	case TC_SETUP_BLOCK:
> -		return mlx5e_rep_indr_setup_tc_block(netdev, cb_priv,
> -						      type_data);
> +		return mlx5e_rep_indr_setup_block(netdev, cb_priv, type_data,
> +						  mlx5e_rep_indr_setup_tc_cb);
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> @@ -820,7 +820,7 @@ static int mlx5e_rep_indr_register_block(struct mlx5e_rep_priv *rpriv,
>  	int err;
>  
>  	err = __flow_indr_block_cb_register(netdev, rpriv,
> -					    mlx5e_rep_indr_setup_tc_cb,
> +					    mlx5e_rep_indr_setup_cb,
>  					    rpriv);
>  	if (err) {
>  		struct mlx5e_priv *priv = netdev_priv(rpriv->netdev);
> @@ -834,7 +834,7 @@ static int mlx5e_rep_indr_register_block(struct mlx5e_rep_priv *rpriv,
>  static void mlx5e_rep_indr_unregister_block(struct mlx5e_rep_priv *rpriv,
>  					    struct net_device *netdev)
>  {
> -	__flow_indr_block_cb_unregister(netdev, mlx5e_rep_indr_setup_tc_cb,
> +	__flow_indr_block_cb_unregister(netdev, mlx5e_rep_indr_setup_cb,
>  					rpriv);
>  }