Re: [PATCH net-next] cxgb4: add driver support for FW_CLIP2_CMD

[Michal Swiatkowski <michal.swiatkowski@linux.intel.com>]

On Wed, Dec 04, 2024 at 07:24:16PM +0530, Anumula Murali Mohan Reddy wrote:
> Query firmware for FW_CLIP2_CMD support and enable it. FW_CLIP2_CMD
> will be used for setting LIP mask for the corresponding entry in the
> CLIP table. If no LIP mask is specified, a default value of ~0 is
> written for mask.
> 
> Signed-off-by: Anumula Murali Mohan Reddy <anumula@chelsio.com>
> Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
> ---
>  drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c | 146 ++++++++++++++----
>  drivers/net/ethernet/chelsio/cxgb4/clip_tbl.h |   6 +-
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4.h    |   1 +
>  .../net/ethernet/chelsio/cxgb4/cxgb4_filter.c |  23 ++-
>  .../net/ethernet/chelsio/cxgb4/cxgb4_main.c   |   3 +
>  drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h |  12 ++
>  6 files changed, 152 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c
> index 5060d3998889..8da9e7fe7f65 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/clip_tbl.c
> @@ -18,6 +18,8 @@
>  #include "cxgb4.h"
>  #include "clip_tbl.h"
>  
> +static const u64 clip_ipv6_exact_mask[2] = { ~0, ~0 };
> +
>  static inline unsigned int ipv4_clip_hash(struct clip_tbl *c, const u32 *key)
>  {
>  	unsigned int clipt_size_half = c->clipt_size / 2;
> @@ -42,36 +44,73 @@ static unsigned int clip_addr_hash(struct clip_tbl *ctbl, const u32 *addr,
>  }
>  
>  static int clip6_get_mbox(const struct net_device *dev,
> -			  const struct in6_addr *lip)
> +			  const struct in6_addr *lip,
> +			  const struct in6_addr *lipm)
>  {
>  	struct adapter *adap = netdev2adap(dev);
> -	struct fw_clip_cmd c;
> +	struct fw_clip2_cmd c;
> +
> +	if (!adap->params.clip2_cmd_support) {
> +		struct fw_clip_cmd old_cmd;
> +
> +		memset(&old_cmd, 0, sizeof(old_cmd));
> +		old_cmd.op_to_write = htonl(FW_CMD_OP_V(FW_CLIP_CMD) |
> +					    FW_CMD_REQUEST_F | FW_CMD_WRITE_F);
> +		old_cmd.alloc_to_len16 = htonl(FW_CLIP_CMD_ALLOC_F |
> +					       FW_LEN16(old_cmd));
> +		*(__be64 *)&old_cmd.ip_hi = *(__be64 *)(lip->s6_addr);
> +		*(__be64 *)&old_cmd.ip_lo = *(__be64 *)(lip->s6_addr + 8);
> +
> +		return t4_wr_mbox_meat(adap, adap->mbox, &old_cmd,
> +				       sizeof(old_cmd), &old_cmd, false);
> +	}
>  
>  	memset(&c, 0, sizeof(c));
> -	c.op_to_write = htonl(FW_CMD_OP_V(FW_CLIP_CMD) |
> +	c.op_to_write = htonl(FW_CMD_OP_V(FW_CLIP2_CMD) |
>  			      FW_CMD_REQUEST_F | FW_CMD_WRITE_F);
>  	c.alloc_to_len16 = htonl(FW_CLIP_CMD_ALLOC_F | FW_LEN16(c));
>  	*(__be64 *)&c.ip_hi = *(__be64 *)(lip->s6_addr);
>  	*(__be64 *)&c.ip_lo = *(__be64 *)(lip->s6_addr + 8);
> +	*(__be64 *)&c.ipm_hi = *(__be64 *)(lipm->s6_addr);
> +	*(__be64 *)&c.ipm_lo = *(__be64 *)(lipm->s6_addr + 8);
>  	return t4_wr_mbox_meat(adap, adap->mbox, &c, sizeof(c), &c, false);
>  }
>  
>  static int clip6_release_mbox(const struct net_device *dev,
> -			      const struct in6_addr *lip)
> +			      const struct in6_addr *lip,
> +			      const struct in6_addr *lipm)
>  {
>  	struct adapter *adap = netdev2adap(dev);
> -	struct fw_clip_cmd c;
> +	struct fw_clip2_cmd c;
> +
> +	if (!adap->params.clip2_cmd_support) {
> +		struct fw_clip_cmd old_cmd;
> +
> +		memset(&old_cmd, 0, sizeof(old_cmd));
> +		old_cmd.op_to_write = htonl(FW_CMD_OP_V(FW_CLIP_CMD) |
> +					    FW_CMD_REQUEST_F | FW_CMD_WRITE_F);
> +		old_cmd.alloc_to_len16 = htonl(FW_CLIP_CMD_FREE_F |
> +					       FW_LEN16(old_cmd));
> +		*(__be64 *)&old_cmd.ip_hi = *(__be64 *)(lip->s6_addr);
> +		*(__be64 *)&old_cmd.ip_lo = *(__be64 *)(lip->s6_addr + 8);
> +
> +		return t4_wr_mbox_meat(adap, adap->mbox, &old_cmd,
> +				       sizeof(old_cmd), &old_cmd, false);
This block is similar to the block for old command in get_mbox, maybe
move to function:
olc_cmd_send(..., cmd_type)
{
	...
	old_cmd.alloc_to_len16 = htonl(cmd_type | FW_LEN16(old_cmd));
	...
}

To be honest the same can be done for whole release/get
clip6_get_mbox(...)
{
	clip6_mbox(..., ALLOC);
}

clip6_release_mbox(...)
{
	clip6_mbox(..., FREE);
}

You will safe some lines.

}
> +	}
>  
>  	memset(&c, 0, sizeof(c));
> -	c.op_to_write = htonl(FW_CMD_OP_V(FW_CLIP_CMD) |
> -			      FW_CMD_REQUEST_F | FW_CMD_READ_F);
> +	c.op_to_write = htonl(FW_CMD_OP_V(FW_CLIP2_CMD) |
> +			      FW_CMD_REQUEST_F | FW_CMD_WRITE_F);
>  	c.alloc_to_len16 = htonl(FW_CLIP_CMD_FREE_F | FW_LEN16(c));
>  	*(__be64 *)&c.ip_hi = *(__be64 *)(lip->s6_addr);
>  	*(__be64 *)&c.ip_lo = *(__be64 *)(lip->s6_addr + 8);
> +	*(__be64 *)&c.ipm_hi = *(__be64 *)(lipm->s6_addr);
> +	*(__be64 *)&c.ipm_lo = *(__be64 *)(lipm->s6_addr + 8);
> +
>  	return t4_wr_mbox_meat(adap, adap->mbox, &c, sizeof(c), &c, false);
>  }
>  
>

[...]

> -- 
> 2.39.3