[dpdk-dev] [PATCH v2 12/17] net/i40e: destroy ethertype filter

Tiwei Bie tiwei.bie at intel.com
Wed Dec 28 05:56:17 CET 2016


On Tue, Dec 27, 2016 at 02:26:19PM +0800, Beilei Xing wrote:
> This patch adds i40e_dev_destroy_ethertype_filter function
> to destroy a ethertype filter for users.
> 
> Signed-off-by: Beilei Xing <beilei.xing at intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 10 ++-------
>  drivers/net/i40e/i40e_ethdev.h |  5 +++++
>  drivers/net/i40e/i40e_flow.c   | 51 ++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 56 insertions(+), 10 deletions(-)
> 
[...]
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
> index 2a61c4f..732c411 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
[...]
> @@ -1492,11 +1495,16 @@ i40e_flow_destroy(__rte_unused struct rte_eth_dev *dev,

The `__rte_unused' qualifier should be removed.

>  		  struct rte_flow *flow,
>  		  struct rte_flow_error *error)
>  {
> +	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>  	struct i40e_flow *pmd_flow = (struct i40e_flow *)flow;
>  	enum rte_filter_type filter_type = pmd_flow->filter_type;
>  	int ret;
>  
>  	switch (filter_type) {
> +	case RTE_ETH_FILTER_ETHERTYPE:
> +		ret = i40e_dev_destroy_ethertype_filter(pf,
> +				(struct i40e_ethertype_filter *)pmd_flow->rule);
> +		break;
>  	default:
>  		PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
>  			    filter_type);
> @@ -1504,10 +1512,49 @@ i40e_flow_destroy(__rte_unused struct rte_eth_dev *dev,
>  		break;
>  	}
>  
> -	if (ret)
> +	if (!ret) {
> +		TAILQ_REMOVE(&pf->flow_list, pmd_flow, node);
> +		free(pmd_flow);
> +	} else {
>  		rte_flow_error_set(error, EINVAL,
>  				   RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
>  				   "Failed to destroy flow.");
> +	}

Probably you should introduce the pf related code when introducing
i40e_flow_destroy() in the below patch:

[PATCH v2 11/17] net/i40e: add flow destroy function

> +
> +	return ret;
> +}
> +
> +static int
> +i40e_dev_destroy_ethertype_filter(struct i40e_pf *pf,
> +				  struct i40e_ethertype_filter *filter)
> +{
> +	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> +	struct i40e_ethertype_rule *ethertype_rule = &pf->ethertype;
> +	struct i40e_ethertype_filter *node;
> +	struct i40e_control_filter_stats stats;
> +	uint16_t flags = 0;
> +	int ret = 0;
> +
> +	if (!(filter->flags & RTE_ETHTYPE_FLAGS_MAC))
> +		flags |= I40E_AQC_ADD_CONTROL_PACKET_FLAGS_IGNORE_MAC;
> +	if (filter->flags & RTE_ETHTYPE_FLAGS_DROP)
> +		flags |= I40E_AQC_ADD_CONTROL_PACKET_FLAGS_DROP;
> +	flags |= I40E_AQC_ADD_CONTROL_PACKET_FLAGS_TO_QUEUE;
> +
> +	memset(&stats, 0, sizeof(stats));
> +	ret = i40e_aq_add_rem_control_packet_filter(hw,
> +					    filter->input.mac_addr.addr_bytes,
> +					    filter->input.ether_type,
> +					    flags, pf->main_vsi->seid,
> +					    filter->queue, 0, &stats, NULL);
> +	if (ret < 0)
> +		return ret;
> +
> +	node = i40e_sw_ethertype_filter_lookup(ethertype_rule, &filter->input);
> +	if (node)
> +		ret = i40e_sw_ethertype_filter_del(pf, node);
> +	else
> +		return -EINVAL;

It would be more readable to check whether node equals NULL and return
when it's true, and call i40e_sw_ethertype_filter_del(pf, node) outside
the `if' statement:

	node = i40e_sw_ethertype_filter_lookup(ethertype_rule, &filter->input);
	if (node == NULL)
		return -EINVAL;

	ret = i40e_sw_ethertype_filter_del(pf, node);

Best regards,
Tiwei Bie


More information about the dev mailing list