From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <bernard.iremonger@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
 by dpdk.org (Postfix) with ESMTP id 433D91B79B
 for <dev@dpdk.org>; Tue, 24 Oct 2017 12:09:12 +0200 (CEST)
Received: from fmsmga004.fm.intel.com ([10.253.24.48])
 by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 24 Oct 2017 03:09:11 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.43,427,1503385200"; d="scan'208";a="326974532"
Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155])
 by fmsmga004.fm.intel.com with ESMTP; 24 Oct 2017 03:09:09 -0700
Received: from irsmsx108.ger.corp.intel.com ([169.254.11.167]) by
 IRSMSX102.ger.corp.intel.com ([169.254.2.180]) with mapi id 14.03.0319.002;
 Tue, 24 Oct 2017 11:09:09 +0100
From: "Iremonger, Bernard" <bernard.iremonger@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>
CC: "dev@dpdk.org" <dev@dpdk.org>, "Yigit, Ferruh" <ferruh.yigit@intel.com>,
 "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "Dumitrescu, Cristian"
 <cristian.dumitrescu@intel.com>, "adrien.mazarguil@6wind.com"
 <adrien.mazarguil@6wind.com>, "Singh, Jasvinder" <jasvinder.singh@intel.com>
Thread-Topic: [dpdk-dev] [PATCH v10 1/4] librte_flow_classify: add flow
 classify library
Thread-Index: AQHTTBHp1kj9Ii1ZX0KKq7myXHop26Lysi0AgAAUT7A=
Date: Tue, 24 Oct 2017 10:09:08 +0000
Message-ID: <8CEF83825BEC744B83065625E567D7C24E0556BD@IRSMSX108.ger.corp.intel.com>
References: <1508679124-5922-1-git-send-email-bernard.iremonger@intel.com>
 <1508771778-617-2-git-send-email-bernard.iremonger@intel.com>
 <2744961.4TGXLb1DrC@xps>
In-Reply-To: <2744961.4TGXLb1DrC@xps>
Accept-Language: en-GB, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiODQyMWZhNjMtOWI1OC00OTI3LTlmMmYtMjhjNWYyNmNlZDdkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6Im0rWEJHWWRSUEVqZkptdmFQbzlISlFxUTFHbkk2elNQWmhxNEE3VytqUFk9In0=
x-ctpclassification: CTP_IC
dlp-product: dlpe-windows
dlp-version: 11.0.0.116
dlp-reaction: no-action
x-originating-ip: [163.33.239.182]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v10 1/4] librte_flow_classify: add flow
 classify library
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 24 Oct 2017 10:09:12 -0000

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Tuesday, October 24, 2017 10:51 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; adrien.mazarguil@6wind.com; Singh,
> Jasvinder <jasvinder.singh@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v10 1/4] librte_flow_classify: add flow
> classify library
>=20
> Hi,
>=20
> Few comments detailed below.
> The new compilation dependencies management needs changes in the
> Makefile.
> And the new log system should be used.

I will send a v11 patch set.
=20
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -739,6 +739,13 @@ F: doc/guides/prog_guide/pdump_lib.rst
> >  F: app/pdump/
> >  F: doc/guides/tools/pdump.rst
> >
> > +Flow classify
> > +M: Bernard Iremonger <bernard.iremonger@intel.com>
> > +F: lib/librte_flow_classify/
> > +F: test/test/test_flow_classify*
> > +F: examples/flow_classify/
> > +F: doc/guides/sample_app_ug/flow_classify.rst
> > +F: doc/guides/prog_guide/flow_classify_lib.rst
>=20
> I don't how to classify this classify library :) If it is using librte_ta=
ble, it should
> be part of Packet Framework?

No, it is not intended to be part of Packet Framework.

> If not part of Packet Framework, please move it before "Distributor".

Ok, I will move it to before "Distributor"

=20
> The library is missing in the release notes (.so section and new features=
).

I will add it to the release notes.
=20
> > --- 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.
> > +*/
>=20
> We must stop adding the legacy log types.
> Please switch to dynamic logs and remove
> CONFIG_RTE_LIBRTE_CLASSIFY_DEBUG.

Ok, will do.

> > +CFLAGS +=3D -O3
> > +CFLAGS +=3D $(WERROR_FLAGS) -I$(SRCDIR)
>=20
> Now the dependencies to internal libraries must be explicitly declared in
> LDLIBS.

Ok, will do.
=20
> > --- a/mk/rte.app.mk
> > +++ b/mk/rte.app.mk
> > @@ -58,6 +58,7 @@ _LDLIBS-y +=3D -L$(RTE_SDK_BIN)/lib  #  # Order is
> > important: from higher level to lower level  #
> > +_LDLIBS-$(CONFIG_RTE_LIBRTE_FLOW_CLASSIFY)  +=3D -lrte_flow_classify
> >  _LDLIBS-$(CONFIG_RTE_LIBRTE_PIPELINE)       +=3D -lrte_pipeline
> >  _LDLIBS-$(CONFIG_RTE_LIBRTE_TABLE)          +=3D -lrte_table
> >  _LDLIBS-$(CONFIG_RTE_LIBRTE_PORT)           +=3D -lrte_port
>=20
> Yes, rte_flow_classify is on top of packet framework.

Regards,

Bernard.