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.x...@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
