* [dpdk-dev] [PATCH] acl: Fix RTE_ACL_RULE_SZ macro definition @ 2020-07-07 12:26 levendsayar 2020-07-07 12:42 ` Ananyev, Konstantin 0 siblings, 1 reply; 5+ messages in thread From: levendsayar @ 2020-07-07 12:26 UTC (permalink / raw) To: konstantin.ananyev; +Cc: dev, Levend Sayar From: Levend Sayar <levendsayar@gmail.com> Signed-off-by: Levend Sayar <levendsayar@gmail.com> --- 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] acl: Fix RTE_ACL_RULE_SZ macro definition 2020-07-07 12:26 [dpdk-dev] [PATCH] acl: Fix RTE_ACL_RULE_SZ macro definition levendsayar @ 2020-07-07 12:42 ` Ananyev, Konstantin 2020-07-07 15:25 ` Levend Sayar 0 siblings, 1 reply; 5+ messages in thread From: Ananyev, Konstantin @ 2020-07-07 12:42 UTC (permalink / raw) To: levendsayar; +Cc: dev > From: Levend Sayar <levendsayar@gmail.com> 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 <levendsayar@gmail.com> > --- > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] acl: Fix RTE_ACL_RULE_SZ macro definition 2020-07-07 12:42 ` Ananyev, Konstantin @ 2020-07-07 15:25 ` Levend Sayar 2020-07-07 16:44 ` Ananyev, Konstantin 0 siblings, 1 reply; 5+ messages in thread From: Levend Sayar @ 2020-07-07 15:25 UTC (permalink / raw) To: Ananyev, Konstantin; +Cc: dev 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[]; } 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_num)) So the above definition gets the size of a "struct rte_acl_rule" which has 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; 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) = 16; sizeof (struct rte_acl_rule_data) = 12; Best, Levend On Tue, Jul 7, 2020 at 3:42 PM Ananyev, Konstantin < konstantin.ananyev@intel.com> wrote: > > > From: Levend Sayar <levendsayar@gmail.com> > > 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 <levendsayar@gmail.com> > > --- > > 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 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] acl: Fix RTE_ACL_RULE_SZ macro definition 2020-07-07 15:25 ` Levend Sayar @ 2020-07-07 16:44 ` Ananyev, Konstantin 2020-07-07 16:59 ` Levend Sayar 0 siblings, 1 reply; 5+ messages in thread From: Ananyev, Konstantin @ 2020-07-07 16:44 UTC (permalink / raw) To: Levend Sayar; +Cc: dev From: Levend Sayar <levendsayar@gmail.com> Sent: Tuesday, July 7, 2020 4:25 PM To: Ananyev, Konstantin <konstantin.ananyev@intel.com> 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_num)) So the above definition gets the size of a "struct rte_acl_rule" which has 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’t 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 ‘data’ and ‘field’. So, for: RTE_ACL_RULE_DEF(xz, 1); sizeof(struct xz) == RTE_ACL_RULE_SZ(1) == 32 After changes you suggest RTE_ACL_RULE_SZ(1) == 28 != 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) = 16; sizeof (struct rte_acl_rule_data) = 12; Best, Levend On Tue, Jul 7, 2020 at 3:42 PM Ananyev, Konstantin <konstantin.ananyev@intel.com<mailto:konstantin.ananyev@intel.com>> wrote: > From: Levend Sayar <levendsayar@gmail.com<mailto:levendsayar@gmail.com>> 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 <levendsayar@gmail.com<mailto:levendsayar@gmail.com>> > --- > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] acl: Fix RTE_ACL_RULE_SZ macro definition 2020-07-07 16:44 ` Ananyev, Konstantin @ 2020-07-07 16:59 ` Levend Sayar 0 siblings, 0 replies; 5+ messages in thread From: Levend Sayar @ 2020-07-07 16:59 UTC (permalink / raw) To: Ananyev, Konstantin; +Cc: 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 <levendsayar@gmail.com> > *Sent:* Tuesday, July 7, 2020 4:25 PM > *To:* Ananyev, Konstantin <konstantin.ananyev@intel.com> > *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_num > )) > > > > So the above definition gets the size of a "struct rte_acl_rule" which has > 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’t 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 ‘data’ and ‘field’. > > So, for: > > RTE_ACL_RULE_DEF(xz, 1); > > > > sizeof(struct xz) == RTE_ACL_RULE_SZ(1) == 32 > > > > After changes you suggest > > RTE_ACL_RULE_SZ(1) == 28 != 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) = 16; > > sizeof (struct rte_acl_rule_data) = 12; > > > > Best, > > Levend > > > > > > On Tue, Jul 7, 2020 at 3:42 PM Ananyev, Konstantin < > konstantin.ananyev@intel.com> wrote: > > > > From: Levend Sayar <levendsayar@gmail.com> > > 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 <levendsayar@gmail.com> > > --- > > 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 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-08 12:30 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-07 12:26 [dpdk-dev] [PATCH] acl: Fix RTE_ACL_RULE_SZ macro definition levendsayar 2020-07-07 12:42 ` Ananyev, Konstantin 2020-07-07 15:25 ` Levend Sayar 2020-07-07 16:44 ` Ananyev, Konstantin 2020-07-07 16:59 ` Levend Sayar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).