Re: [Intel-wired-lan] [PATCH net v1] i40e: Fix ethtool rx-flow-hash setting for X722

[Tony Nguyen <anthony.l.nguyen@intel.com>]

On 6/10/2022 5:23 AM, Loktionov, Aleksandr wrote:
> From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> 
> When enabling flow type for RSS hash via ethtool:
> 
>    ethtool -N $pf rx-flow-hash tcp4|tcp6|udp4|udp6 s|d
> 
> the driver would fail to setup this setting on X722
> device since it was using the mask on the register
> dedicated for X710 devices.
> 
> Implement a bitmap to collect the flow pc types that
> shall be applied on the inset and hena registers.
> Apply a different mask on the register when setting the
> RSS hash for the X722 device.
> 
> When displaying the flow types enabled via ethtool:
> 
>    ethtool -n $pf rx-flow-hash tcp4|tcp6|udp4|udp6
> 
> the driver would print wrong values for X722 device.
> 
> Fix this issue by testing masks for X722 device in
> i40e_get_rss_hash_opts function.
> 
> Fixes: eb0dd6e4a3b3 (i40e: Allow RSS Hash set with less than four parameters)
> Signed-off-by: Slawomir Laba slawomirx.laba@intel.com

The signoff is missing the '<' '>' around the email.

Also, who's the author of this? If it's you, your sign off should be 
first. If it's Slawomir, you should fix the patch author to reflect that.

> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   .../net/ethernet/intel/i40e/i40e_ethtool.c    | 103 +++++++++++-------
>   drivers/net/ethernet/intel/i40e/i40e_type.h   |   4 +
>   2 files changed, 67 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index 5584181..08587a1 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -3190,10 +3190,17 @@ static int i40e_get_rss_hash_opts(struct i40e_pf *pf, struct ethtool_rxnfc *cmd)
>   
>   		if (cmd->flow_type == TCP_V4_FLOW ||
>   		    cmd->flow_type == UDP_V4_FLOW) {
> -			if (i_set & I40E_L3_SRC_MASK)
> -				cmd->data |= RXH_IP_SRC;
> -			if (i_set & I40E_L3_DST_MASK)
> -				cmd->data |= RXH_IP_DST;
> +			if (hw->mac.type == I40E_MAC_X722) {
> +				if (i_set & I40E_X722_L3_SRC_MASK)
> +					cmd->data |= RXH_IP_SRC;
> +				if (i_set & I40E_X722_L3_DST_MASK)
> +					cmd->data |= RXH_IP_DST;
> +			} else {
> +				if (i_set & I40E_L3_SRC_MASK)
> +					cmd->data |= RXH_IP_SRC;
> +				if (i_set & I40E_L3_DST_MASK)
> +					cmd->data |= RXH_IP_DST;
> +			}
>   		} else if (cmd->flow_type == TCP_V6_FLOW ||
>   			  cmd->flow_type == UDP_V6_FLOW) {
>   			if (i_set & I40E_L3_V6_SRC_MASK)
> @@ -3556,7 +3563,9 @@ static int i40e_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
>    *
>    * Returns value of bits to be set per user request
>    **/
> -static u64 i40e_get_rss_hash_bits(struct ethtool_rxnfc *nfc, u64 i_setc)
> +static u64 i40e_get_rss_hash_bits(struct i40e_hw *hw,
> +				  struct ethtool_rxnfc *nfc,
> +				  u64 i_setc)

kdoc needs to be updated for hw

>   {
>   	u64 i_set = i_setc;
>   	u64 src_l3 = 0, dst_l3 = 0;
> @@ -3575,8 +3584,13 @@ static u64 i40e_get_rss_hash_bits(struct ethtool_rxnfc *nfc, u64 i_setc)
>   		dst_l3 = I40E_L3_V6_DST_MASK;
>   	} else if (nfc->flow_type == TCP_V4_FLOW ||
>   		  nfc->flow_type == UDP_V4_FLOW) {
> -		src_l3 = I40E_L3_SRC_MASK;
> -		dst_l3 = I40E_L3_DST_MASK;
> +		if (hw->mac.type == I40E_MAC_X722) {
> +			src_l3 = I40E_X722_L3_SRC_MASK;
> +			dst_l3 = I40E_X722_L3_DST_MASK;
> +		} else {
> +			src_l3 = I40E_L3_SRC_MASK;
> +			dst_l3 = I40E_L3_DST_MASK;
> +		}
>   	} else {
>   		/* Any other flow type are not supported here */
>   		return i_set;
> @@ -3606,9 +3620,12 @@ static int i40e_set_rss_hash_opt(struct i40e_pf *pf, struct ethtool_rxnfc *nfc)
>   	struct i40e_hw *hw = &pf->hw;
>   	u64 hena = (u64)i40e_read_rx_ctl(hw, I40E_PFQF_HENA(0)) |
>   		   ((u64)i40e_read_rx_ctl(hw, I40E_PFQF_HENA(1)) << 32);
> -	u8 flow_pctype = 0;
> +#define FLOW_PCTYPES_SIZE 64

defines should not be in the function either before the function, at the 
top of the file, header file, etc.

> +	DECLARE_BITMAP(flow_pctypes, FLOW_PCTYPES_SIZE);
>   	u64 i_set, i_setc;
>   
> +	bitmap_zero(flow_pctypes, FLOW_PCTYPES_SIZE);
> +
>   	if (pf->flags & I40E_FLAG_MFP_ENABLED) {
>   		dev_err(&pf->pdev->dev,
>   			"Change of RSS hash input set is not supported when MFP mode is enabled\n");
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan