[Intel-wired-lan] [PATCH net-next v2 2/4] ice: remove redundant Rx field from rule info

Alexander Lobakin aleksander.lobakin at intel.com
Tue Apr 4 10:07:47 UTC 2023


From: Michal Swiatkowski <michal.swiatkowski at linux.intel.com>
Date: Tue,  4 Apr 2023 09:28:31 +0200

> Information about the direction is currently stored in sw_act.flag.
> There is no need to duplicate it in another field.
> 
> Setting direction flag doesn't mean that there is a match criteria for
> direction in rule. It is only a information for HW from where switch id
> should be collected (VSI or port). In current implementation of advance
> rule handling, without matching for direction meta data, we can always
> set one the same flag and everything will work the same.
> 
> Ability to match on direction matadata will be added in follow up
> patches.
> 
> Recipe 0, 3 and 9 loaded from package has direction match
> criteria, but they are handled in other function.
> 
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski at linux.intel.com>
> Reviewed-by: Piotr Raczynski <piotr.raczynski at intel.com>
> Reviewed-by: Simon Horman <simon.horman at corigine.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_eswitch.c |  1 -
>  drivers/net/ethernet/intel/ice/ice_switch.c  | 22 ++++++++++----------
>  drivers/net/ethernet/intel/ice/ice_switch.h  |  2 --
>  drivers/net/ethernet/intel/ice/ice_tc_lib.c  |  5 -----
>  4 files changed, 11 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c b/drivers/net/ethernet/intel/ice/ice_eswitch.c
> index f6dd3f8fd936..2c80d57331d0 100644
> --- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
> +++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c
> @@ -39,7 +39,6 @@ ice_eswitch_add_vf_mac_rule(struct ice_pf *pf, struct ice_vf *vf, const u8 *mac)
>  	rule_info.sw_act.flag |= ICE_FLTR_TX;
>  	rule_info.sw_act.vsi_handle = ctrl_vsi->idx;
>  	rule_info.sw_act.fltr_act = ICE_FWD_TO_Q;
> -	rule_info.rx = false;
>  	rule_info.sw_act.fwd_id.q_id = hw->func_caps.common_cap.rxq_first_id +
>  				       ctrl_vsi->rxq_map[vf->vf_id];
>  	rule_info.flags_info.act |= ICE_SINGLE_ACT_LB_ENABLE;
> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
> index 5c3f266fa80f..4d3a92e0c61f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_switch.c
> +++ b/drivers/net/ethernet/intel/ice/ice_switch.c
> @@ -6121,8 +6121,7 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
>  	if (rinfo->sw_act.fltr_act == ICE_FWD_TO_VSI)
>  		rinfo->sw_act.fwd_id.hw_vsi_id =
>  			ice_get_hw_vsi_num(hw, vsi_handle);
> -	if (rinfo->sw_act.flag & ICE_FLTR_TX)
> -		rinfo->sw_act.src = ice_get_hw_vsi_num(hw, vsi_handle);
> +	rinfo->sw_act.src = ice_get_hw_vsi_num(hw, vsi_handle);
>  
>  	status = ice_add_adv_recipe(hw, lkups, lkups_cnt, rinfo, &rid);
>  	if (status)
> @@ -6190,19 +6189,20 @@ ice_add_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
>  		goto err_ice_add_adv_rule;
>  	}
>  
> -	/* set the rule LOOKUP type based on caller specified 'Rx'
> -	 * instead of hardcoding it to be either LOOKUP_TX/RX
> +	/* If there is no matching criteria for direction there
> +	 * is only one difference between Rx and Tx:
> +	 * - get switch id base on VSI number from source field (Tx)
> +	 * - get switch id base on port number (Rx)
>  	 *
> -	 * for 'Rx' set the source to be the port number
> -	 * for 'Tx' set the source to be the source HW VSI number (determined
> -	 * by caller)
> +	 * If matching on direction metadata is chose rule direction is
> +	 * extracted from type value set here.
>  	 */
> -	if (rinfo->rx) {
> -		s_rule->hdr.type = cpu_to_le16(ICE_AQC_SW_RULES_T_LKUP_RX);
> -		s_rule->src = cpu_to_le16(hw->port_info->lport);
> -	} else {
> +	if (rinfo->sw_act.flag & ICE_FLTR_TX) {
>  		s_rule->hdr.type = cpu_to_le16(ICE_AQC_SW_RULES_T_LKUP_TX);
>  		s_rule->src = cpu_to_le16(rinfo->sw_act.src);
> +	} else {
> +		s_rule->hdr.type = cpu_to_le16(ICE_AQC_SW_RULES_T_LKUP_RX);
> +		s_rule->src = cpu_to_le16(hw->port_info->lport);
>  	}
>  
>  	s_rule->recipe_id = cpu_to_le16(rid);
> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.h b/drivers/net/ethernet/intel/ice/ice_switch.h
> index 68d8e8a6a189..44aa37b80111 100644
> --- a/drivers/net/ethernet/intel/ice/ice_switch.h
> +++ b/drivers/net/ethernet/intel/ice/ice_switch.h
> @@ -10,7 +10,6 @@
>  #define ICE_DFLT_VSI_INVAL 0xff
>  #define ICE_FLTR_RX BIT(0)
>  #define ICE_FLTR_TX BIT(1)
> -#define ICE_FLTR_TX_RX (ICE_FLTR_RX | ICE_FLTR_TX)
>  #define ICE_VSI_INVAL_ID 0xffff
>  #define ICE_INVAL_Q_HANDLE 0xFFFF
>  
> @@ -190,7 +189,6 @@ struct ice_adv_rule_info {
>  	enum ice_sw_tunnel_type tun_type;
>  	struct ice_sw_act_ctrl sw_act;
>  	u32 priority;
> -	u8 rx; /* true means LOOKUP_RX otherwise LOOKUP_TX */
>  	u16 fltr_rule_id;
>  	u16 vlan_type;
>  	struct ice_adv_rule_flags_info flags_info;

That u8 here was really off, was introducing at least 1 byte hole. Good
thing you dropped it.
Have you checked whether there are any holes left, maybe move fields
around a bit?

> diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> index 76f29a5bf8d7..b5af6cd5592b 100644
> --- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> @@ -697,11 +697,9 @@ ice_eswitch_add_tc_fltr(struct ice_vsi *vsi, struct ice_tc_flower_fltr *fltr)
>  	if (fltr->direction == ICE_ESWITCH_FLTR_INGRESS) {
>  		rule_info.sw_act.flag |= ICE_FLTR_RX;
>  		rule_info.sw_act.src = hw->pf_id;
> -		rule_info.rx = true;
>  	} else {
>  		rule_info.sw_act.flag |= ICE_FLTR_TX;
>  		rule_info.sw_act.src = vsi->idx;
> -		rule_info.rx = false;
>  		rule_info.flags_info.act = ICE_SINGLE_ACT_LAN_ENABLE;
>  		rule_info.flags_info.act_valid = true;
>  	}
> @@ -909,7 +907,6 @@ ice_add_tc_flower_adv_fltr(struct ice_vsi *vsi,
>  		rule_info.sw_act.vsi_handle = dest_vsi->idx;
>  		rule_info.priority = ICE_SWITCH_FLTR_PRIO_VSI;
>  		rule_info.sw_act.src = hw->pf_id;
> -		rule_info.rx = true;
>  		dev_dbg(dev, "add switch rule for TC:%u vsi_idx:%u, lkups_cnt:%u\n",
>  			tc_fltr->action.fwd.tc.tc_class,
>  			rule_info.sw_act.vsi_handle, lkups_cnt);
> @@ -920,7 +917,6 @@ ice_add_tc_flower_adv_fltr(struct ice_vsi *vsi,
>  		rule_info.sw_act.vsi_handle = dest_vsi->idx;
>  		rule_info.priority = ICE_SWITCH_FLTR_PRIO_QUEUE;
>  		rule_info.sw_act.src = hw->pf_id;
> -		rule_info.rx = true;
>  		dev_dbg(dev, "add switch rule action to forward to queue:%u (HW queue %u), lkups_cnt:%u\n",
>  			tc_fltr->action.fwd.q.queue,
>  			tc_fltr->action.fwd.q.hw_queue, lkups_cnt);
> @@ -928,7 +924,6 @@ ice_add_tc_flower_adv_fltr(struct ice_vsi *vsi,
>  	case ICE_DROP_PACKET:
>  		rule_info.sw_act.flag |= ICE_FLTR_RX;
>  		rule_info.sw_act.src = hw->pf_id;
> -		rule_info.rx = true;
>  		rule_info.priority = ICE_SWITCH_FLTR_PRIO_VSI;
>  		break;
>  	default:

Thanks,
Olek


More information about the Intel-wired-lan mailing list