RE: [Intel-wired-lan] [PATCH iwl-next v2 08/10] ice: program ACL entry

Loktionov, Aleksandr
2026-04-09T13:36:04Z

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Marcin Szycik
> Sent: Thursday, April 9, 2026 2:00 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Penigalapati, Sandeep
> <sandeep.penigalapati@intel.com>; S, Ananth <ananth.s@intel.com>;
> alexander.duyck@gmail.com; Marcin Szycik
> <marcin.szycik@linux.intel.com>; Cao, Chinh T <chinh.t.cao@intel.com>;
> Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v2 08/10] ice: program ACL
> entry
>
> From: Real Valiquette <real.valiquette@intel.com>
>
> Complete the filter programming process; set the flow entry and action
> into the scenario and write it to hardware. Configure the VSI for ACL
> filters.
>
> Co-developed-by: Chinh Cao <chinh.t.cao@intel.com>
> Signed-off-by: Chinh Cao <chinh.t.cao@intel.com>
> Signed-off-by: Real Valiquette <real.valiquette@intel.com>
> Co-developed-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>
> ---
> v2:
> * Use plain alloc instead of devm_ for ice_flow_entry::acts
> * Use FIELD_PREP_CONST() for ICE_ACL_RX_*_MISS_CNTR
> * Fix wrong struct ice_acl_act_entry alloc count in
> ice_flow_acl_add_scen_entry_sync() - was e->entry_sz, which is an
> unrelated value
> * Only set acts_cnt after successful allocation in
> ice_flow_acl_add_scen_entry_sync()
> * Return -EINVAL instead of -ENOSPC on wrong index in
> ice_acl_scen_free_entry_idx()
> ---
> drivers/net/ethernet/intel/ice/ice.h | 2 +
> drivers/net/ethernet/intel/ice/ice_acl.h | 21 +
> .../net/ethernet/intel/ice/ice_adminq_cmd.h | 2 +
> drivers/net/ethernet/intel/ice/ice_flow.h | 3 +
> drivers/net/ethernet/intel/ice/ice_acl.c | 53 ++-
> drivers/net/ethernet/intel/ice/ice_acl_ctrl.c | 251 +++++++++++
> drivers/net/ethernet/intel/ice/ice_acl_main.c | 4 +
> .../ethernet/intel/ice/ice_ethtool_ntuple.c | 48 ++-
> drivers/net/ethernet/intel/ice/ice_flow.c | 395
> ++++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_lib.c | 10 +-
> 10 files changed, 782 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index 9e6643931022..f9a43daf04fe 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -1061,6 +1061,8 @@ void ice_aq_prep_for_event(struct ice_pf *pf,
> struct ice_aq_task *task,
> u16 opcode);
> int ice_aq_wait_for_event(struct ice_pf *pf, struct ice_aq_task
> *task,
> unsigned long timeout);
> +int ice_ntuple_update_list_entry(struct ice_pf *pf,
> + struct ice_ntuple_fltr *input, int
> fltr_idx);
> int ice_open(struct net_device *netdev); int
> ice_open_internal(struct net_device *netdev); int ice_stop(struct
> net_device *netdev); diff --git
> a/drivers/net/ethernet/intel/ice/ice_acl.h
> b/drivers/net/ethernet/intel/ice/ice_acl.h
> index 3a4adcf368cf..0b5651401eb7 100644
> --- a/drivers/net/ethernet/intel/ice/ice_acl.h
> +++ b/drivers/net/ethernet/intel/ice/ice_acl.h
> @@ -39,6 +39,7 @@ struct ice_acl_tbl {
> DECLARE_BITMAP(avail, ICE_AQC_ACL_ALLOC_UNITS); };
>
> +#define ICE_MAX_ACL_TCAM_ENTRY (ICE_AQC_ACL_TCAM_DEPTH *
> +ICE_AQC_ACL_SLICES)
> enum ice_acl_entry_prio {
> ICE_ACL_PRIO_LOW = 0,
> ICE_ACL_PRIO_NORMAL,
> @@ -65,6 +66,11 @@ struct ice_acl_scen {
> * participate in this scenario
> */
> DECLARE_BITMAP(act_mem_bitmap, ICE_AQC_MAX_ACTION_MEMORIES);

...

> + /* Determine number of cascaded TCAMs */
> + num_cscd = DIV_ROUND_UP(scen->width,
> ICE_AQC_ACL_KEY_WIDTH_BYTES);
> +
> + entry_tcam = ICE_ACL_TBL_TCAM_IDX(scen->start);
> + idx = ICE_ACL_TBL_TCAM_ENTRY_IDX(scen->start + *entry_idx);
> +
> + for (u8 i = 0; i < num_cscd; i++) {
> + /* If the key spans more than one TCAM in the case of
> cascaded
> + * TCAMs, the key and key inverts need to be properly
> split
> + * among TCAMs.E.g.bytes 0 - 4 go to an index in the
> first TCAM
"E.g.bytes" -> "E.g. bytes"

> + * and bytes 5 - 9 go to the same index in the next
> TCAM, etc.
> + * If the entry spans more than one TCAM in a cascaded
> TCAM
> + * mode, the programming of the entries in the TCAMs
> must be in
> + * reversed order - the TCAM entry of the rightmost TCAM
> should
> + * be programmed first; the TCAM entry of the leftmost
> TCAM
> + * should be programmed last.
> + */
> + offset = num_cscd - i - 1;
> + memcpy(&buf.entry_key.val,
> + &keys[offset * sizeof(buf.entry_key.val)],
> + sizeof(buf.entry_key.val));
> + memcpy(&buf.entry_key_invert.val,
> + &inverts[offset *
> sizeof(buf.entry_key_invert.val)],
> + sizeof(buf.entry_key_invert.val));
> + err = ice_aq_program_acl_entry(hw, entry_tcam + offset,
> idx,
> + &buf, NULL);
> + if (err) {
> + ice_debug(hw, ICE_DBG_ACL, "aq program acl entry
> failed status: %d\n",
> + err);
> + goto out;
> + }
> + }
> +
> + err = ice_acl_prog_act(hw, scen, acts, acts_cnt, *entry_idx);
> +
> +out:
> + if (err) {
> + ice_acl_rem_entry(hw, scen, *entry_idx);
> + *entry_idx = 0;
> + }
> +
> + return err;
> +}
> +
> +/**
> + * ice_acl_prog_act - Program a scenario's action memory
> + * @hw: pointer to the HW struct
> + * @scen: scenario to add the entry to
> + * @acts: pointer to a buffer containing formatted actions
> + * @acts_cnt: indicates the number of actions stored in "acts"
> + * @entry_idx: scenario relative index of the added flow entry
> + *
> + * Return: 0 on success, negative on error */ int
> +ice_acl_prog_act(struct ice_hw *hw, struct ice_acl_scen *scen,
> + struct ice_acl_act_entry *acts, u8 acts_cnt, u16
> entry_idx) {
> + u8 entry_tcam, num_cscd, i, actx_idx = 0;
> + struct ice_aqc_actpair act_buf = {};
> + int err = 0;
> + u16 idx;
> +
> + if (entry_idx >= scen->num_entry)
> + return -ENOSPC;
> +
> + /* Determine number of cascaded TCAMs */
> + num_cscd = DIV_ROUND_UP(scen->width,
> ICE_AQC_ACL_KEY_WIDTH_BYTES);
> +
> + entry_tcam = ICE_ACL_TBL_TCAM_IDX(scen->start);
> + idx = ICE_ACL_TBL_TCAM_ENTRY_IDX(scen->start + entry_idx);
> +
> + for_each_set_bit(i, scen->act_mem_bitmap,
> ICE_AQC_MAX_ACTION_MEMORIES) {
> + struct ice_acl_act_mem *mem = &hw->acl_tbl->act_mems[i];
> +
> + if (actx_idx >= acts_cnt)
> + break;
> + if (mem->member_of_tcam >= entry_tcam &&
> + mem->member_of_tcam < entry_tcam + num_cscd) {
> + memcpy(&act_buf.act[0], &acts[actx_idx],
> + sizeof(struct ice_acl_act_entry));
> +
> + if (++actx_idx < acts_cnt) {
> + memcpy(&act_buf.act[1], &acts[actx_idx],
> + sizeof(struct ice_acl_act_entry));
> + }
> +
> + err = ice_aq_program_actpair(hw, i, idx,
> &act_buf,
> + NULL);
> + if (err) {
> + ice_debug(hw, ICE_DBG_ACL, "program actpair
> failed status: %d\n",
> + err);
> + break;
> + }
> + actx_idx++;
> + }
> + }
> +
> + if (!err && actx_idx < acts_cnt)
> + err = -ENOSPC;
> +
> + return err;
> +}
> +
> +/**
> + * ice_acl_rem_entry - Remove a flow entry from an ACL scenario
> + * @hw: pointer to the HW struct
> + * @scen: scenario to remove the entry from
> + * @entry_idx: the scenario-relative index of the flow entry being
> +removed
> + *
> + * Return: 0 on success, negative on error */ int
> +ice_acl_rem_entry(struct ice_hw *hw, struct ice_acl_scen *scen,
> + u16 entry_idx)
> +{
> + struct ice_aqc_actpair act_buf = {};
> + struct ice_aqc_acl_data buf;
> + u8 entry_tcam, num_cscd, i;
> + int err = 0;
> + u16 idx;
> +
> + if (!scen)
> + return -ENOENT;
> +
> + if (entry_idx >= scen->num_entry)
> + return -ENOSPC;
> +
> + if (!test_bit(entry_idx, scen->entry_bitmap))
> + return -ENOENT;
> +
> + /* Determine number of cascaded TCAMs */
> + num_cscd = DIV_ROUND_UP(scen->width,
> ICE_AQC_ACL_KEY_WIDTH_BYTES);
> +
> + entry_tcam = ICE_ACL_TBL_TCAM_IDX(scen->start);
> + idx = ICE_ACL_TBL_TCAM_ENTRY_IDX(scen->start + entry_idx);
> +
> + /* invalidate the flow entry */
> + memset(&buf, 0, sizeof(buf));
> + for (i = 0; i < num_cscd; i++) {
> + err = ice_aq_program_acl_entry(hw, entry_tcam + i, idx,
> &buf,
> + NULL);
> + if (err)
> + ice_debug(hw, ICE_DBG_ACL, "AQ program ACL entry
> failed status: %d\n",
> + err);
> + }
> +
> + for_each_set_bit(i, scen->act_mem_bitmap,
> ICE_AQC_MAX_ACTION_MEMORIES) {
> + struct ice_acl_act_mem *mem = &hw->acl_tbl->act_mems[i];
> +
> + if (mem->member_of_tcam >= entry_tcam &&
> + mem->member_of_tcam < entry_tcam + num_cscd) {
> + /* Invalidate allocated action pairs */
> + err = ice_aq_program_actpair(hw, i, idx,
> &act_buf,
> + NULL);
> + if (err)
> + ice_debug(hw, ICE_DBG_ACL, "program actpair
> failed status: %d\n",
> + err);
> + }
> + }
> +
> + ice_acl_scen_free_entry_idx(scen, entry_idx);
> +
> + return err;
> +}
> diff --git a/drivers/net/ethernet/intel/ice/ice_acl_main.c
> b/drivers/net/ethernet/intel/ice/ice_acl_main.c
> index 53cca0526756..16228be574ed 100644
> --- a/drivers/net/ethernet/intel/ice/ice_acl_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_acl_main.c
> @@ -280,6 +280,10 @@ int ice_acl_add_rule_ethtool(struct ice_vsi *vsi,
> struct ethtool_rxnfc *cmd)
> hw_prof->entry_h[hw_prof->cnt++][0] = entry_h;
> }
>
> + input->acl_fltr = true;
> + /* input struct is added to the HW filter list */
> + ice_ntuple_update_list_entry(pf, input, fsp->location);
> +
> return 0;
>
> free_input:
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool_ntuple.c
> b/drivers/net/ethernet/intel/ice/ice_ethtool_ntuple.c
> index 3e79c0bf40f4..21d4f4e3a1d0 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool_ntuple.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool_ntuple.c
> @@ -1791,6 +1791,21 @@ void ice_vsi_manage_fdir(struct ice_vsi *vsi,
> bool ena)
> mutex_unlock(&hw->fdir_fltr_lock);
> }
>
> +/**
> + * ice_del_acl_ethtool - delete an ACL rule entry
> + * @hw: pointer to HW instance
> + * @fltr: filter structure
> + *
> + * Return: 0 on success, negative on error */ static int
> +ice_del_acl_ethtool(struct ice_hw *hw, struct ice_ntuple_fltr *fltr)
> {
> + u64 entry;
> +
> + entry = ice_flow_find_entry(hw, ICE_BLK_ACL, fltr->fltr_id);
> + return ice_flow_rem_entry(hw, ICE_BLK_ACL, entry); }
> +
> /**
> * ice_fdir_do_rem_flow - delete flow and possibly add perfect flow
> * @pf: PF structure
> @@ -1824,7 +1839,7 @@ ice_fdir_do_rem_flow(struct ice_pf *pf, enum
> ice_fltr_ptype flow_type)
> *
> * Return: 0 on success and negative on errors
> */
> -static int
> +int
> ice_ntuple_update_list_entry(struct ice_pf *pf, struct
> ice_ntuple_fltr *input,
> int fltr_idx)
> {
> @@ -1843,13 +1858,36 @@ ice_ntuple_update_list_entry(struct ice_pf
> *pf, struct ice_ntuple_fltr *input,
>
> old_fltr = ice_fdir_find_fltr_by_idx(hw, fltr_idx);
> if (old_fltr) {
> - err = ice_fdir_write_all_fltr(pf, old_fltr, false);
> - if (err)
> - return err;
> + if (old_fltr->acl_fltr) {
> + /* ACL filter - if the input buffer is present
> + * then this is an update and we don't want to
> + * delete the filter from the HW. We've already
> + * written the change to the HW at this point, so
> + * just update the SW structures to make sure
> + * everything is hunky-dory. If no input then
> this
> + * is a delete so we should delete the filter
> from
> + * the HW and clean up our SW structures.
> + */
> + if (!input) {
> + err = ice_del_acl_ethtool(hw, old_fltr);
> + if (err)
> + return err;
> + }
> + } else {
> + /* FD filter */
> + err = ice_fdir_write_all_fltr(pf, old_fltr,
> false);
> + if (err)
> + return err;
> + }
> +
> ice_fdir_update_cntrs(hw, old_fltr->flow_type, false,
> false);
> /* update sb-filters count, specific to ring->channel */
> ice_update_per_q_fltr(vsi, old_fltr->orig_q_index,
> false);
> - if (!input && !hw->fdir_fltr_cnt[old_fltr->flow_type])
> + /* Also delete the HW filter info if we have just
> deleted the
> + * last filter of flow_type.
> + */
> + if (!old_fltr->acl_fltr && !input &&
> + !hw->fdir_fltr_cnt[old_fltr->flow_type])
> /* we just deleted the last filter of flow_type
> so we
> * should also delete the HW filter info.
> */
> diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c
> b/drivers/net/ethernet/intel/ice/ice_flow.c
> index dce6d2ffcb15..144d8326d4f9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_flow.c
> +++ b/drivers/net/ethernet/intel/ice/ice_flow.c
> @@ -1744,6 +1744,16 @@ static int ice_flow_rem_entry_sync(struct
> ice_hw *hw, enum ice_block blk,
> return -EINVAL;
>
> if (blk == ICE_BLK_ACL) {
> + int err;
> +
> + if (!entry->prof)
> + return -EINVAL;
> +
> + err = ice_acl_rem_entry(hw, entry->prof->cfg.scen,
> + entry->scen_entry_idx);
> + if (err)
> + return err;
> +
> if (entry->acts_cnt && entry->acts)
> ice_flow_acl_free_act_cntr(hw, entry->acts,
> entry->acts_cnt);
> @@ -1879,10 +1889,34 @@ ice_flow_rem_prof_sync(struct ice_hw *hw, enum
> ice_block blk,
> }
>
> if (blk == ICE_BLK_ACL) {
> + struct ice_aqc_acl_prof_generic_frmt buf;
> + u8 prof_id = 0;
> +
> /* Disassociate the scenario from the profile for the PF
> */
> status = ice_flow_acl_disassoc_scen(hw, prof);
> if (status)
> return status;
> +
> + status = ice_flow_get_hw_prof(hw, blk, prof->id,
> &prof_id);
> + if (status)
> + return status;
> +
> + status = ice_query_acl_prof(hw, prof_id, &buf, NULL);
> + if (status)
> + return status;
> +
> + /* Clear the range-checker if the profile ID is no
> longer
> + * used by any PF
> + */
> + if (!ice_flow_acl_is_prof_in_use(&buf)) {
> + /* Clear the range-checker value for profile ID
> */
> + struct ice_aqc_acl_profile_ranges query_rng_buf =
> {};
> +
> + status = ice_prog_acl_prof_ranges(hw, prof_id,
> + &query_rng_buf,
> NULL);
> + if (status)
> + return status;
> + }
> }
>
> /* Remove all hardware profiles associated with this flow
> profile */ @@ -2214,6 +2248,44 @@ int ice_flow_rem_prof(struct ice_hw
> *hw, enum ice_block blk, u64 prof_id)
> return status;
> }
>
> +/**
> + * ice_flow_find_entry - look for a flow entry using its unique ID
> + * @hw: pointer to the HW struct
> + * @blk: classification stage
> + * @entry_id: unique ID to identify this flow entry
> + *
> + * Look for the flow entry with the specified unique ID in all flow
> +profiles of
> + * the specified classification stage.
> + *
> + * Return: flow entry handle if entry found, ICE_FLOW_ENTRY_ID_INVAL
> +otherwise */
> +u64 ice_flow_find_entry(struct ice_hw *hw, enum ice_block blk, u64
> +entry_id) {
> + struct ice_flow_entry *found = NULL;
> + struct ice_flow_prof *p;
> +
> + mutex_lock(&hw->fl_profs_locks[blk]);
> +
> + list_for_each_entry(p, &hw->fl_profs[blk], l_entry) {
> + struct ice_flow_entry *e;
> +
> + mutex_lock(&p->entries_lock);
> + list_for_each_entry(e, &p->entries, l_entry)
> + if (e->id == entry_id) {
> + found = e;
> + break;
> + }
> + mutex_unlock(&p->entries_lock);
> +
> + if (found)
> + break;
> + }
> +
> + mutex_unlock(&hw->fl_profs_locks[blk]);
> +
> + return found ? ICE_FLOW_ENTRY_HNDL(found) :
> +ICE_FLOW_ENTRY_HANDLE_INVAL; }
> +
> /**
> * ice_flow_acl_check_actions - Checks the ACL rule's actions
> * @hw: pointer to the hardware structure @@ -2541,6 +2613,325 @@
> static int ice_flow_acl_frmt_entry(struct ice_hw *hw,
>
> return err;
> }
> +
> +/**
> + * ice_flow_acl_find_scen_entry_cond - Find an ACL scenario entry
> that matches
> + * the compared data
> + * @prof: pointer to flow profile
> + * @e: pointer to the comparing flow entry
> + * @do_chg_action: decide if we want to change the ACL action
> + * @do_add_entry: decide if we want to add the new ACL entry
> + * @do_rem_entry: decide if we want to remove the current ACL entry
> + *
> + * Find an ACL scenario entry that matches the compared data. Also
> figure out:
> + * a) If we want to change the ACL action
> + * b) If we want to add the new ACL entry
> + * c) If we want to remove the current ACL entry
> + *
> + * Return: ACL scenario entry, or NULL if not found */ static struct
> +ice_flow_entry * ice_flow_acl_find_scen_entry_cond(struct
> ice_flow_prof
> +*prof,
> + struct ice_flow_entry *e, bool
> *do_chg_action,
> + bool *do_add_entry, bool *do_rem_entry) {
> + struct ice_flow_entry *p, *return_entry = NULL;
> +
> + /* Check if:
> + * a) There exists an entry with same matching data, but
> different
> + * priority, then we remove this existing ACL entry. Then,
> we
> + * will add the new entry to the ACL scenario.
> + * b) There exists an entry with same matching data, priority,
> and
> + * result action, then we do nothing
> + * c) There exists an entry with same matching data, priority,
> but
> + * different, action, then do only change the action's
> entry.
Too much of commas, please reduce the number.

> + * d) Else, we add this new entry to the ACL scenario.
> + */
> + *do_chg_action = false;
> + *do_add_entry = true;
> + *do_rem_entry = false;
> + list_for_each_entry(p, &prof->entries, l_entry) {
> + if (memcmp(p->entry, e->entry, p->entry_sz))
> + continue;
> +
> + /* From this point, we have the same matching_data. */
> + *do_add_entry = false;
> + return_entry = p;
> +
> + if (p->priority != e->priority) {
> + /* matching data && !priority */
> + *do_add_entry = true;
> + *do_rem_entry = true;
> + break;
> + }
> +
> + /* From this point, we will have matching_data &&
> priority */
> + if (p->acts_cnt != e->acts_cnt)
> + *do_chg_action = true;
> + for (int i = 0; i < p->acts_cnt; i++) {
> + bool found_not_match = false;
> +
> + for (int j = 0; j < e->acts_cnt; j++)
> + if (memcmp(&p->acts[i], &e->acts[j],
> + sizeof(struct ice_flow_action)))
Due to comment above it should be if (!memcmp(&p->acts[i], &e->acts[j],
Please fix the comment or code.

Otherwise, it looks good.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

> {
> + found_not_match = true;
> + break;

...

> }
>
> /**
> --
> 2.49.0