All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcin Szycik <marcin.szycik@linux.intel.com>
To: "Loktionov, Aleksandr" <aleksandr.loktionov@intel.com>,
	"intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"Penigalapati, Sandeep" <sandeep.penigalapati@intel.com>,
	"S, Ananth" <ananth.s@intel.com>,
	"alexander.duyck@gmail.com" <alexander.duyck@gmail.com>,
	"Cao, Chinh T" <chinh.t.cao@intel.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2 08/10] ice: program ACL entry
Date: Mon, 13 Apr 2026 12:57:30 +0200	[thread overview]
Message-ID: <44f2e76c-d573-4eb0-87f5-0c91e6340715@linux.intel.com> (raw)
In-Reply-To: <IA3PR11MB898666B7AB0330B3C29DC630E5582@IA3PR11MB8986.namprd11.prod.outlook.com>



On 09.04.2026 15:35, Loktionov, Aleksandr wrote:
> 
> 
>> -----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.

I believe the logic is fine, but naming/comments might be a bit confusing.
I'll try to make it more readable.

Thanks,
Marcin

> Otherwise, it looks good.
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> 
>> {
>> +					found_not_match = true;
>> +					break;
> 
> ...
> 
>>  }
>>
>>  /**
>> --
>> 2.49.0
> 


  reply	other threads:[~2026-04-13 10:57 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-09 11:59 [Intel-wired-lan] [PATCH iwl-next v2 00/10] Add ACL support Marcin Szycik
2026-04-09 11:59 ` Marcin Szycik
2026-04-09 11:59 ` [Intel-wired-lan] [PATCH iwl-next v2 01/10] ice: rename shared Flow Director functions and structs Marcin Szycik
2026-04-09 11:59   ` Marcin Szycik
2026-05-06 18:33   ` [Intel-wired-lan] " Nowlin, Alexander
2026-05-06 18:33     ` Nowlin, Alexander
2026-04-09 11:59 ` [Intel-wired-lan] [PATCH iwl-next v2 02/10] ice: initialize ACL table Marcin Szycik
2026-04-09 11:59   ` Marcin Szycik
2026-05-06 18:34   ` [Intel-wired-lan] " Nowlin, Alexander
2026-05-06 18:34     ` Nowlin, Alexander
2026-04-09 11:59 ` [Intel-wired-lan] [PATCH iwl-next v2 03/10] ice: initialize ACL scenario Marcin Szycik
2026-04-09 11:59   ` Marcin Szycik
2026-05-06 18:35   ` [Intel-wired-lan] " Nowlin, Alexander
2026-05-06 18:35     ` Nowlin, Alexander
2026-04-09 11:59 ` [Intel-wired-lan] [PATCH iwl-next v2 04/10] ice: create flow profile Marcin Szycik
2026-04-09 11:59   ` Marcin Szycik
2026-05-06 18:37   ` [Intel-wired-lan] " Nowlin, Alexander
2026-05-06 18:37     ` Nowlin, Alexander
2026-04-09 11:59 ` [Intel-wired-lan] [PATCH iwl-next v2 05/10] Revert "ice: remove unused ice_flow_entry fields" Marcin Szycik
2026-04-09 11:59   ` Marcin Szycik
2026-05-06 18:38   ` [Intel-wired-lan] " Nowlin, Alexander
2026-05-06 18:38     ` Nowlin, Alexander
2026-04-09 11:59 ` [Intel-wired-lan] [PATCH iwl-next v2 06/10] ice: use plain alloc/dealloc for ice_ntuple_fltr Marcin Szycik
2026-04-09 11:59   ` Marcin Szycik
2026-05-06 18:39   ` [Intel-wired-lan] " Nowlin, Alexander
2026-05-06 18:39     ` Nowlin, Alexander
2026-04-09 12:00 ` [Intel-wired-lan] [PATCH iwl-next v2 07/10] ice: create ACL entry Marcin Szycik
2026-04-09 12:00   ` Marcin Szycik
2026-05-06 18:40   ` [Intel-wired-lan] " Nowlin, Alexander
2026-05-06 18:40     ` Nowlin, Alexander
2026-04-09 12:00 ` [Intel-wired-lan] [PATCH iwl-next v2 08/10] ice: program " Marcin Szycik
2026-04-09 12:00   ` Marcin Szycik
2026-04-09 13:35   ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-04-09 13:35     ` Loktionov, Aleksandr
2026-04-13 10:57     ` Marcin Szycik [this message]
2026-05-06 18:42   ` Nowlin, Alexander
2026-05-06 18:42     ` Nowlin, Alexander
2026-04-09 12:00 ` [Intel-wired-lan] [PATCH iwl-next v2 09/10] ice: re-introduce ice_dealloc_flow_entry() helper Marcin Szycik
2026-04-09 12:00   ` Marcin Szycik
2026-05-06 18:42   ` [Intel-wired-lan] " Nowlin, Alexander
2026-05-06 18:42     ` Nowlin, Alexander
2026-04-09 12:00 ` [Intel-wired-lan] [PATCH iwl-next v2 10/10] ice: use ACL for ntuple rules that conflict with FDir Marcin Szycik
2026-04-09 12:00   ` Marcin Szycik
2026-04-09 17:37   ` [Intel-wired-lan] " Przemek Kitszel
2026-05-06 18:43   ` Nowlin, Alexander
2026-05-06 18:43     ` Nowlin, Alexander
2026-05-06 12:50 ` [Intel-wired-lan] [PATCH iwl-next v2 00/10] Add ACL support Marcin Szycik
2026-05-06 12:50   ` Marcin Szycik
2026-05-07 22:22 ` [Intel-wired-lan] " Jacob Keller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44f2e76c-d573-4eb0-87f5-0c91e6340715@linux.intel.com \
    --to=marcin.szycik@linux.intel.com \
    --cc=aleksandr.loktionov@intel.com \
    --cc=alexander.duyck@gmail.com \
    --cc=ananth.s@intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=chinh.t.cao@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=netdev@vger.kernel.org \
    --cc=sandeep.penigalapati@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.