[dpdk-dev] [PATCH v2 01/17] net/i40e: store ethertype filter
Xing, Beilei
beilei.xing at intel.com
Thu Dec 29 05:03:24 CET 2016
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Wednesday, December 28, 2016 10:22 AM
> To: Xing, Beilei <beilei.xing at intel.com>; Zhang, Helin
> <helin.zhang at intel.com>
> Cc: dev at dpdk.org
> Subject: RE: [PATCH v2 01/17] net/i40e: store ethertype filter
>
> > +
> > +/* 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(ðertype_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.
Thanks for the suggestion, I'll use the key as parameter in the next version.
>
> > /*
> > * 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,
> > + ðertype_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.
Malloc ethertype_filter because i40e_ethertype_filter_convert is involved first, and then check if a rule exists with ethertype_filter->input.
>
> Thanks
> Jingjing
More information about the dev
mailing list