Re: [dpdk-dev] [PATCH 07/49] net/ice/base: replay advanced rule after reset

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 6/4/19 7:42 AM, Leyi Rong wrote:
> Code added to replay the advanced rule per VSI basis and remove the
> advanced rule information from shared code recipe list.
> 
> Signed-off-by: Victor Raj <victor.raj@intel.com>
> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> Signed-off-by: Leyi Rong <leyi.rong@intel.com>
> ---
>   drivers/net/ice/base/ice_switch.c | 81 ++++++++++++++++++++++++++-----
>   1 file changed, 69 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ice/base/ice_switch.c b/drivers/net/ice/base/ice_switch.c
> index c53021aed..ca0497ca7 100644
> --- a/drivers/net/ice/base/ice_switch.c
> +++ b/drivers/net/ice/base/ice_switch.c
> @@ -3033,6 +3033,27 @@ ice_rem_sw_rule_info(struct ice_hw *hw, struct LIST_HEAD_TYPE *rule_head)
>   	}
>   }
>   
> +/**
> + * ice_rem_adv_rule_info
> + * @hw: pointer to the hardware structure
> + * @rule_head: pointer to the switch list structure that we want to delete
> + */
> +static void
> +ice_rem_adv_rule_info(struct ice_hw *hw, struct LIST_HEAD_TYPE *rule_head)
> +{
> +	struct ice_adv_fltr_mgmt_list_entry *tmp_entry;
> +	struct ice_adv_fltr_mgmt_list_entry *lst_itr;
> +
> +	if (LIST_EMPTY(rule_head))
> +		return;


Is it necessary? If the list is empty, LIST_FOR_EACH_ENTRY will not loop
and status will be returned:

#define LIST_FOR_EACH_ENTRY_SAFE(pos, tmp, head, type, member) \
	LIST_FOR_EACH_ENTRY(pos, head, type, member)

with:

#define LIST_FOR_EACH_ENTRY(pos, head, type, member)			       \
	for ((pos) = (head)->lh_first ?					       \
		     container_of((head)->lh_first, struct type, member) :     \
		     0;							       \
	     (pos);							       \
	     (pos) = (pos)->member.next.le_next ?			       \
		     container_of((pos)->member.next.le_next, struct type,     \
				  member) :				       \
		     0)

> +	LIST_FOR_EACH_ENTRY_SAFE(lst_itr, tmp_entry, rule_head,

> +				 ice_adv_fltr_mgmt_list_entry, list_entry) {
> +		LIST_DEL(&lst_itr->list_entry);
> +		ice_free(hw, lst_itr->lkups);
> +		ice_free(hw, lst_itr);
> +	}
> +}
>   
>   /**
>    * ice_rem_all_sw_rules_info
> @@ -3049,6 +3070,8 @@ void ice_rem_all_sw_rules_info(struct ice_hw *hw)
>   		rule_head = &sw->recp_list[i].filt_rules;
>   		if (!sw->recp_list[i].adv_rule)
>   			ice_rem_sw_rule_info(hw, rule_head);
> +		else
> +			ice_rem_adv_rule_info(hw, rule_head);
>   	}
>   }
>   
> @@ -5687,6 +5710,38 @@ ice_replay_vsi_fltr(struct ice_hw *hw, u16 vsi_handle, u8 recp_id,
>   	return status;
>   }
>   
> +/**
> + * ice_replay_vsi_adv_rule - Replay advanced rule for requested VSI
> + * @hw: pointer to the hardware structure
> + * @vsi_handle: driver VSI handle
> + * @list_head: list for which filters need to be replayed
> + *
> + * Replay the advanced rule for the given VSI.
> + */
> +static enum ice_status
> +ice_replay_vsi_adv_rule(struct ice_hw *hw, u16 vsi_handle,
> +			struct LIST_HEAD_TYPE *list_head)
> +{
> +	struct ice_rule_query_data added_entry = { 0 };
> +	struct ice_adv_fltr_mgmt_list_entry *adv_fltr;
> +	enum ice_status status = ICE_SUCCESS;
> +
> +	if (LIST_EMPTY(list_head))
> +		return status;

Ditto, it should be removed.

> +	LIST_FOR_EACH_ENTRY(adv_fltr, list_head, ice_adv_fltr_mgmt_list_entry,
> +			    list_entry) {
> +		struct ice_adv_rule_info *rinfo = &adv_fltr->rule_info;
> +		u16 lk_cnt = adv_fltr->lkups_cnt;
> +
> +		if (vsi_handle != rinfo->sw_act.vsi_handle)
> +			continue;
> +		status = ice_add_adv_rule(hw, adv_fltr->lkups, lk_cnt, rinfo,
> +					  &added_entry);
> +		if (status)
> +			break;
> +	}
> +	return status;
> +}
>   
>   /**
>    * ice_replay_vsi_all_fltr - replay all filters stored in bookkeeping lists
> @@ -5698,23 +5753,23 @@ ice_replay_vsi_fltr(struct ice_hw *hw, u16 vsi_handle, u8 recp_id,
>   enum ice_status ice_replay_vsi_all_fltr(struct ice_hw *hw, u16 vsi_handle)
>   {
>   	struct ice_switch_info *sw = hw->switch_info;
> -	enum ice_status status = ICE_SUCCESS;
> +	enum ice_status status;
>   	u8 i;
>   
> +	/* Update the recipes that were created */
>   	for (i = 0; i < ICE_MAX_NUM_RECIPES; i++) {
> -		/* Update the default recipe lines and ones that were created */
> -		if (i < ICE_MAX_NUM_RECIPES || sw->recp_list[i].recp_created) {
> -			struct LIST_HEAD_TYPE *head;
> +		struct LIST_HEAD_TYPE *head;
>   
> -			head = &sw->recp_list[i].filt_replay_rules;
> -			if (!sw->recp_list[i].adv_rule)
> -				status = ice_replay_vsi_fltr(hw, vsi_handle, i,
> -							     head);
> -			if (status != ICE_SUCCESS)
> -				return status;
> -		}
> +		head = &sw->recp_list[i].filt_replay_rules;
> +		if (!sw->recp_list[i].adv_rule)
> +			status = ice_replay_vsi_fltr(hw, vsi_handle, i, head);
> +		else
> +			status = ice_replay_vsi_adv_rule(hw, vsi_handle, head);
> +		if (status != ICE_SUCCESS)
> +			return status;
>   	}
> -	return status;
> +
> +	return ICE_SUCCESS;
>   }
>   
>   /**
> @@ -5738,6 +5793,8 @@ void ice_rm_all_sw_replay_rule_info(struct ice_hw *hw)
>   			l_head = &sw->recp_list[i].filt_replay_rules;
>   			if (!sw->recp_list[i].adv_rule)
>   				ice_rem_sw_rule_info(hw, l_head);
> +			else
> +				ice_rem_adv_rule_info(hw, l_head);
>   		}
>   	}
>   }
>