Re: [dpdk-dev] [PATCH v2 01/17] net/i40e: store ethertype filter

[Wu, Jingjing]

> +
> +/* Delete ethertype filter in SW list */ static int
> +i40e_sw_ethertype_filter_del(struct i40e_pf *pf,
> +                          struct i40e_ethertype_filter *filter) {
> +     struct i40e_ethertype_rule *ethertype_rule = &pf->ethertype;
> +     int ret = 0;
> +
> +     ret = rte_hash_del_key(ethertype_rule->hash_table,
> +                            &filter->input);
> +     if (ret < 0)
> +             PMD_DRV_LOG(ERR,
> +                         "Failed to delete ethertype filter"
> +                         " to hash table %d!",
> +                         ret);
> +     ethertype_rule->hash_map[ret] = NULL;
> +
> +     TAILQ_REMOVE(&ethertype_rule->ethertype_list, filter, rules);
> +     rte_free(filter);

It's better to free filter out of del function because the filter is also the 
input argument.
Or you can define this function to use key as argument but not filter.

>  /*
>   * Configure ethertype filter, which can director packet by filtering
>   * with mac address and ether_type or only ether_type @@ -7964,6 +8099,8
> @@ i40e_ethertype_filter_set(struct i40e_pf *pf,
>                       bool add)
>  {
>       struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> +     struct i40e_ethertype_rule *ethertype_rule = &pf->ethertype;
> +     struct i40e_ethertype_filter *ethertype_filter, *node;
>       struct i40e_control_filter_stats stats;
>       uint16_t flags = 0;
>       int ret;
> @@ -7982,6 +8119,22 @@ i40e_ethertype_filter_set(struct i40e_pf *pf,
>               PMD_DRV_LOG(WARNING, "filter vlan ether_type in first tag is"
>                       " not supported.");
> 
> +     /* Check if there is the filter in SW list */
> +     ethertype_filter = rte_zmalloc("ethertype_filter",
> +                                    sizeof(*ethertype_filter), 0);
> +     i40e_ethertype_filter_convert(filter, ethertype_filter);
> +     node = i40e_sw_ethertype_filter_lookup(ethertype_rule,
> +                                            &ethertype_filter->input);
> +     if (add && node) {
> +             PMD_DRV_LOG(ERR, "Conflict with existing ethertype rules!");
> +             rte_free(ethertype_filter);
> +             return -EINVAL;
> +     } else if (!add && !node) {
> +             PMD_DRV_LOG(ERR, "There's no corresponding ethertype
> filter!");
> +             rte_free(ethertype_filter);
> +             return -EINVAL;
> +     }
How about malloc ethertype_filter after check? Especially, no need to malloc it 
when delete a filter.

Thanks
Jingjing