[Intel-wired-lan] [PATCH net-next v2 2/4] ice: remove redundant Rx field from rule info
Michal Swiatkowski
michal.swiatkowski at linux.intel.com
Tue Apr 4 11:40:35 UTC 2023
On Tue, Apr 04, 2023 at 12:07:47PM +0200, Alexander Lobakin wrote:
> 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?
>
As You pointed in patch 3 there are, I will move it to avoid holes.
> > 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