Hi Jasvinder,
Thanks for reviewing.
> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Monday, October 23, 2017 2:22 PM
> To: Iremonger, Bernard <bernard.iremon...@intel.com>; dev@dpdk.org;
> Yigit, Ferruh <ferruh.yi...@intel.com>; Ananyev, Konstantin
> <konstantin.anan...@intel.com>; Dumitrescu, Cristian
> <cristian.dumitre...@intel.com>; adrien.mazarg...@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.
