From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 37B6DA0526; Wed, 8 Jul 2020 14:30:15 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 611921DE1C; Wed, 8 Jul 2020 14:29:58 +0200 (CEST) Received: from mail-il1-f193.google.com (mail-il1-f193.google.com [209.85.166.193]) by dpdk.org (Postfix) with ESMTP id A03304F9A for ; Tue, 7 Jul 2020 19:00:36 +0200 (CEST) Received: by mail-il1-f193.google.com with SMTP id x9so36658511ila.3 for ; Tue, 07 Jul 2020 10:00:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ozFlbzkOw0cbPAvYHiPHzEtk73xd0/9woLankYJzYos=; b=lA46khOIoDa6GQe2lsVNVeZY4iVuHhGLXuc1QooQb8JBBoGDkbSOAClJ2nmNkJ5Paq 1cL+SkCwxxTN3m26+kPdrzHv8JGZ73iQQkn4GH9g1HGtetUw3o2n4dm6NgA7C50mRbP8 J9+Kjgs5WK4XmQuNhcgc1Cpg4ddFDceaDaXWO8REJAvtgzD8qwb3ShDlRB8Z9oChWabR FjHPFt8dUv7ZEtWRsbeqgvFRd9QBDUvzeQXcxpm/Cu8P7Gt3LQab00rP68stuOcA701X kP+Jr2mvYLpogKtyuhpKvpF6kU0qLWU5PoyIkI/VJc6lxNB0uPRDXJ3MKMo+FyWdVQUl zENg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ozFlbzkOw0cbPAvYHiPHzEtk73xd0/9woLankYJzYos=; b=rH7RLooem9zCpWwe8b+81KPpcHU3dUF7hn5fk5v1QGzFzH18SYj24sGu879Zi54pjs TPlE9P/IE6rnmgUjgOdJ6qRe61k6IGJ9DkRglHNG64ph1H7xwngldmVt91kulV+hnWBK s62RCJMowu4mgyoZuprGp53i2TQCVZUfVhsBmgG0DUqS+Hm+4fudvqx5aE3pxc/TjxFF 63dJAWyUKami/Jvxy8t4Dfy4Xa3gkUYIJ52trN0qFyDDXOm4SYuGKxmFd+8K66bsLTUD w7BllZ+f7dFQb82N5PxW4xbeTF7tKwmvdqeqLYF70VatIZiHrt48YWQM2Gr8jc78oIxE ZM8g== X-Gm-Message-State: AOAM532eytpJUKN+pRMVYQIhZZuk0Fl8FEZ5iHr77YB3P6vpaRzkOznI iutWBjxDf6+lBjvYzxzAonaIiL1XZOKd3Uk+TSo= X-Google-Smtp-Source: ABdhPJzDxGwzcwGcoBQhLSSfTs1Habcvjqa7qCgtICDzLckJZU8S6mZrjNEIa3uEULvSKBIutJuclXbjI8zMgRaWB/I= X-Received: by 2002:a92:c689:: with SMTP id o9mr37122435ilg.302.1594141235964; Tue, 07 Jul 2020 10:00:35 -0700 (PDT) MIME-Version: 1.0 References: <20200707122643.29222-1-levendsayar@gmail.com> In-Reply-To: From: Levend Sayar Date: Tue, 7 Jul 2020 19:59:59 +0300 Message-ID: To: "Ananyev, Konstantin" Cc: "dev@dpdk.org" X-Mailman-Approved-At: Wed, 08 Jul 2020 14:29:52 +0200 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] acl: Fix RTE_ACL_RULE_SZ macro definition 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Yes you are totally right. I overlooked the alignment of struct components. Sorry for that. But imho, not to have such confusions, Another define can be added such as #define RTE_ACL_RULE_SIZE(xz) sizeof(struct xz) Thanks for your time. Best, Levend On Tue, Jul 7, 2020 at 7:45 PM Ananyev, Konstantin < konstantin.ananyev@intel.com> wrote: > > > > > *From:* Levend Sayar > *Sent:* Tuesday, July 7, 2020 4:25 PM > *To:* Ananyev, Konstantin > *Cc:* dev@dpdk.org > *Subject:* Re: [PATCH] acl: Fix RTE_ACL_RULE_SZ macro definition > > > > Sure. > > I am really sorry for not being verbose enough. > > > > From lib/librte_acl/rte_acl.h > > > > #define RTE_ACL_RULE_DEF(*name*, *fld_num*) *struct* name {\ > > *struct* rte_acl_rule_data data; \ > > *struct* rte_acl_field field[fld_num]; \ > > } > > > > RTE_ACL_RULE_DEF(rte_acl_rule,); > > > > When you put the definition in-place, above line means : > > > > struct rte_acl_rule { > > struct rte_acl_rule_data data; > > struct rte_acl_field field[]; > > } > > > > [KA] Yes. > > > > There is another define to get the size of an acl rule such as > > > > #define RTE_ACL_RULE_SZ(*fld_num*) \ > > (sizeof(struct rte_acl_rule) + sizeof(struct rte_acl_field) * (fld_nu= m > )) > > > > So the above definition gets the size of a "struct rte_acl_rule" which ha= s > fld_num fields. > > which must be > > *sizeof (struct rte_acl_rule_data) *+ (sizeof(struct rte_acl_field) * > fld_num) > > > > Because it adds up the sizes of struct components; > > > > [KA] I don=E2=80=99t think so. > > You forgot about possible gaps between members of rte_acl_rule. > > Let say for 64 bit target it would be a 4B gap between =E2=80=98data=E2= =80=99 and =E2=80=98field=E2=80=99. > > So, for: > > RTE_ACL_RULE_DEF(xz, 1); > > > > sizeof(struct xz) =3D=3D RTE_ACL_RULE_SZ(1) =3D=3D 32 > > > > After changes you suggest > > RTE_ACL_RULE_SZ(1) =3D=3D 28 !=3D sizeof(struct xz) > > Which is wrong. > > > > But according to the current RTE_ACL_RULE_SZ, it is > > > > *sizeof (struct rte_acl_rule)* + (sizeof(struct rte_acl_field) * fld_num) > > > > So my patch only changes the part that I underlined. > > > > sizeof (struct rte_acl_rule) =3D 16; > > sizeof (struct rte_acl_rule_data) =3D 12; > > > > Best, > > Levend > > > > > > On Tue, Jul 7, 2020 at 3:42 PM Ananyev, Konstantin < > konstantin.ananyev@intel.com> wrote: > > > > From: Levend Sayar > > Could you provide some explanation: > What do you think is wrong with current version and why, > and what your fix does. > > > > > Signed-off-by: Levend Sayar > > --- > > lib/librte_acl/rte_acl.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/librte_acl/rte_acl.h b/lib/librte_acl/rte_acl.h > > index aa22e70c6..d34fdbc0e 100644 > > --- a/lib/librte_acl/rte_acl.h > > +++ b/lib/librte_acl/rte_acl.h > > @@ -116,7 +116,7 @@ struct rte_acl_rule_data { > > RTE_ACL_RULE_DEF(rte_acl_rule,); > > > > #define RTE_ACL_RULE_SZ(fld_num) \ > > - (sizeof(struct rte_acl_rule) + sizeof(struct rte_acl_field) * > (fld_num)) > > + (sizeof(struct rte_acl_rule_data) + sizeof(struct rte_acl_field) = * > (fld_num)) > > > > > > /** Max number of characters in name.*/ > > -- > > 2.27.0 > >