[dpdk-dev] [PATCH v9 1/4] librte_flow_classify: add flow classify library

Iremonger, Bernard bernard.iremonger at intel.com
Mon Oct 23 15:37:39 CEST 2017


Hi Jasvinder,

Thanks for reviewing.

> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Monday, October 23, 2017 2:22 PM
> To: Iremonger, Bernard <bernard.iremonger at intel.com>; dev at dpdk.org;
> Yigit, Ferruh <ferruh.yigit at intel.com>; Ananyev, Konstantin
> <konstantin.ananyev at intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu at intel.com>; adrien.mazarguil at 6wind.com
> Subject: RE: [PATCH v9 1/4] librte_flow_classify: add flow classify library
> 
> <snip>
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -83,6 +83,9 @@ DIRS-$(CONFIG_RTE_LIBRTE_POWER) +=
> librte_power
> > DEPDIRS-librte_power := librte_eal
> >  DIRS-$(CONFIG_RTE_LIBRTE_METER) += librte_meter  DEPDIRS-
> librte_meter
> > := librte_eal
> > +DIRS-$(CONFIG_RTE_LIBRTE_FLOW_CLASSIFY) += librte_flow_classify
> > +DEPDIRS-librte_flow_classify := librte_eal librte_ether librte_net
> > +DEPDIRS-librte_flow_classify += librte_table librte_acl librte_port
> 
> Please check dependency, I think you don't need librte_port, librte_eal,
> librte_ether

I will check dependency

> >  DIRS-$(CONFIG_RTE_LIBRTE_SCHED) += librte_sched  DEPDIRS-
> librte_sched
> > := librte_eal librte_mempool librte_mbuf librte_net
> > DEPDIRS-librte_sched += librte_timer diff --git
> > a/lib/librte_eal/common/include/rte_log.h
> > b/lib/librte_eal/common/include/rte_log.h
> > index 2fa1199..67209ae 100644
> > --- a/lib/librte_eal/common/include/rte_log.h
> > +++ b/lib/librte_eal/common/include/rte_log.h
> > @@ -88,6 +88,7 @@ struct rte_logs {
> >  #define RTE_LOGTYPE_EFD       18 /**< Log related to EFD. */
> >  #define RTE_LOGTYPE_EVENTDEV  19 /**< Log related to eventdev. */
> >  #define RTE_LOGTYPE_GSO       20 /**< Log related to GSO. */
> > +#define RTE_LOGTYPE_CLASSIFY  21 /**< Log related to flow classify.
> > +*/
> >
> 
> <snip>
> 
> > +static int
> > +flow_classifier_run(struct rte_flow_classifier *cls,
> > +		uint32_t table_id,
> > +		struct rte_mbuf **pkts,
> > +		const uint16_t nb_pkts)
> > +{
> > +	int ret = -EINVAL;
> > +	uint64_t pkts_mask;
> > +	uint64_t lookup_hit_mask;
> > +
> > +	if (!cls || !pkts || nb_pkts == 0 || table_id >= cls->num_tables)
> > +		return ret;
> > +
> > +	if (cls->tables[table_id].ops.f_lookup != NULL) {
> > +		pkts_mask = RTE_LEN2MASK(nb_pkts, uint64_t);
> > +		ret = cls->tables[table_id].ops.f_lookup(
> > +			cls->tables[table_id].h_table,
> > +			pkts, pkts_mask, &lookup_hit_mask,
> > +			(void **)cls->entries);
> > +
> > +		if (!ret && lookup_hit_mask)
> > +			cls->nb_pkts = nb_pkts;
> > +		else
> > +			cls->nb_pkts = 0;
> > +	}
> > +
> > +	return ret;
> > +}
> 
> Remove checks in the above function as these are already checked in query
> function below.

Ok, will do.
 
> > +static int
> > +action_apply(struct rte_flow_classifier *cls,
> > +		struct rte_flow_classify_rule *rule,
> > +		struct rte_flow_classify_stats *stats) {
> > +	struct rte_flow_classify_ipv4_5tuple_stats *ntuple_stats;
> > +	uint64_t count = 0;
> > +	int i;
> > +	int ret = -ENODATA;
> > +
> > +	switch (rule->action.type) {
> > +	case RTE_FLOW_ACTION_TYPE_COUNT:
> > +		for (i = 0; i < cls->nb_pkts; i++) {
> > +			if (rule->id == cls->entries[i]->rule_id)
> > +				count++;
> > +		}
> > +		if (count) {
> > +			ret = 0;
> > +			ntuple_stats =
> > +				(struct rte_flow_classify_ipv4_5tuple_stats
> > *)
> > +				stats->stats;
> > +			ntuple_stats->counter1 = count;
> > +			ntuple_stats->ipv4_5tuple = rule-
> > >rules.u.ipv4_5tuple;
> > +		}
> > +		break;
> > +	default:
> > +		ret = -ENOTSUP;
> > +		break;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +int
> > +rte_flow_classifier_query(struct rte_flow_classifier *cls,
> > +		uint32_t table_id,
> > +		struct rte_mbuf **pkts,
> > +		const uint16_t nb_pkts,
> > +		struct rte_flow_classify_rule *rule,
> > +		struct rte_flow_classify_stats *stats) {
> > +	int ret = -EINVAL;
> > +
> > +	if (!cls || !rule || !stats || !pkts  || nb_pkts == 0 ||
> > +		table_id >= cls->num_tables)
> > +		return ret;
> > +
> > +	ret = flow_classifier_run(cls, table_id, pkts, nb_pkts);
> > +	if (!ret)
> > +		ret = action_apply(cls, rule, stats);
> > +	return ret;
> > +}
> 
> 
> Also, there are some compilation warnings as below;
> 
> Failed Build #1:
> OS: FreeBSD10.3_64
> Target: x86_64-native-bsdapp-clang,   x86_64-native-bsdapp-gcc
> SYMLINK-FILE
> include/rte_flow_classify.h/home/patchWorkOrg/compilation/lib/librte_flo
> w_classify/rte_flow_classify.c:642:13: error: use of undeclared identifier
> 'ENODATA'
>         int ret = -ENODATA;

Ok, I will replace -ENODAT with -EINVAL
 
 
> Thanks,
> Jasvinder

I will seed a v10 patch set.

Regards,

Bernard.



More information about the dev mailing list