From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id E99CD98 for ; Tue, 13 Dec 2016 16:13:45 +0100 (CET) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga104.fm.intel.com with ESMTP; 13 Dec 2016 07:13:44 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,342,1477983600"; d="scan'208";a="41867709" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga005.jf.intel.com with ESMTP; 13 Dec 2016 07:13:43 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.43]) by IRSMSX102.ger.corp.intel.com ([169.254.2.79]) with mapi id 14.03.0248.002; Tue, 13 Dec 2016 15:13:43 +0000 From: "Ananyev, Konstantin" To: Michal Miroslaw CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict Thread-Index: AQHSVN2ahB5rgtyRpkyle4teQhaCOKEFry9QgAA3/4CAAANbwIAADPcAgAADYIA= Date: Tue, 13 Dec 2016 15:13:42 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583F0E736D@irsmsx105.ger.corp.intel.com> References: <2601191342CEEE43887BDE71AB9772583F0E6E0B@irsmsx105.ger.corp.intel.com> <20161213135443.ovmlunbh67dr4tew@rere.qmqm.pl> <2601191342CEEE43887BDE71AB9772583F0E7008@irsmsx105.ger.corp.intel.com> <20161213145308.lqqnm6ivryjfxih7@rere.qmqm.pl> In-Reply-To: <20161213145308.lqqnm6ivryjfxih7@rere.qmqm.pl> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="iso-8859-2" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Dec 2016 15:13:46 -0000 > -----Original Message----- > From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl] > Sent: Tuesday, December 13, 2016 2:53 PM > To: Ananyev, Konstantin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict >=20 > On Tue, Dec 13, 2016 at 02:14:19PM +0000, Ananyev, Konstantin wrote: > > > -----Original Message----- > > > From: Michal Miroslaw [mailto:mirq-linux@rere.qmqm.pl] > > > Sent: Tuesday, December 13, 2016 1:55 PM > > > To: Ananyev, Konstantin > > > Cc: dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > > > On Tue, Dec 13, 2016 at 10:36:16AM +0000, Ananyev, Konstantin wrote: > > > > Hi Michal, > > > > > > > > > -----Original Message----- > > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Miros= law > > > > > Sent: Tuesday, December 13, 2016 1:08 AM > > > > > To: dev@dpdk.org > > > > > Subject: [dpdk-dev] [PATCH 04/13] acl: allow zero verdict > > > > > > > > > > Signed-off-by: Micha=B3 Miros=B3aw > > > > > --- > > > > > lib/librte_acl/rte_acl.c | 3 +-- > > > > > lib/librte_acl/rte_acl.h | 2 -- > > > > > lib/librte_table/rte_table_acl.c | 2 +- > > > > > 3 files changed, 2 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c > > > > > index 8b7e92c..d1f40be 100644 > > > > > --- a/lib/librte_acl/rte_acl.c > > > > > +++ b/lib/librte_acl/rte_acl.c > > > > > @@ -313,8 +313,7 @@ acl_check_rule(const struct rte_acl_rule_data= *rd) > > > > > if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, typeof(rd->category_m= ask)) & > > > > > rd->category_mask) =3D=3D 0 || > > > > > rd->priority > RTE_ACL_MAX_PRIORITY || > > > > > - rd->priority < RTE_ACL_MIN_PRIORITY || > > > > > - rd->userdata =3D=3D RTE_ACL_INVALID_USERDATA) > > > > > + rd->priority < RTE_ACL_MIN_PRIORITY) > > > > > return -EINVAL; > > > > > return 0; > > > > > } > > > > > > > > I am not sure, how it supposed to work properly? > > > > Zero value is reserved and ifnicates that no match were found for t= hat input. > > > > > > This is actually in use by us. In our use we don't need to differenti= ate > > > matching a rule with zero verdict vs not matching a rule at all. I al= so > > > have a patch that changes the value returned in non-matching case, bu= t > > > it's in "dirty hack" state, as of yet. > > > > With that chane rte_acl_classify() might produce invalid results. > > Even if you don't need it (I still don't understand how) , it doesn't m= ean other people > > don't need it either and it is ok to change it. > > > > > > > > The ACL code does not treat zero userdata specially, so this is only > > > a policy choice and as such would be better to be made by the user. > > > > I believe it does. > > userdata=3D=3D0 is a reserved value. > > When rte_acl_clasify() returns 0 for that particular input, it means 'n= o matches were found'. >=20 > Dear Konstantin, >=20 > Can you describe how the ACL code treats zero specially? I could not find > anything, really. The only thing I found is that iff I use zero userdata > in a rule I won't be able to differentiate a case where it matched from > a case where no rule matched. Yes, that's what I am talking about. > If I all my rules have non-zero userdata, > then this patch changes nothing. Ok, then why do you remove a code that does checking for invalid userdata= =3D=3D0? That supposed to prevent user to setup invalid value by mistake. But if I have a table where 0 means drop > (default-drop policy) then being able to use zero userdata in DROP rules > makes the ACLs just that more useful. Ok, and what prevents you from do +1 to your policy values before you insert it into the ACL table and -1 after you retrieved it via rte_acl_= classify()?=20 =20 Konstantin