| Message ID | 20240116031728.2500892-1-liwang@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | tcindex01: fix compilation errors due to missing TCA_TCINDEX_ constants | expand |
Hi Li,, > The change addresses compilation errors encountered in the tcindex01.c > test when compiled against kernel-6.7 and above. "against kernel-6.7". But 82b2545ed9a will be released in 6.8, right? > tcindex01.c:67:4: error: ‘TCA_TCINDEX_MASK’ undeclared here (not in a function); > did you mean ‘TCA_CODEL_MAX’? > {TCA_TCINDEX_MASK, &mask, sizeof(mask), NULL}, > ^~~~~~~~~~~~~~~~ > TCA_CODEL_MAX > tcindex01.c:68:4: error: ‘TCA_TCINDEX_SHIFT’ undeclared here (not in a function); > did you mean ‘TCA_FLOW_RSHIFT’? > {TCA_TCINDEX_SHIFT, &shift, sizeof(shift), NULL}, > ^~~~~~~~~~~~~~~~~ > TCA_FLOW_RSHIFT > CC testcases/cve/cve-2016-7117 > CC utils/sctp/func_tests/test_getname_v6.o > tcindex01.c:69:4: error: ‘TCA_TCINDEX_CLASSID’ undeclared here (not in a function); > did you mean ‘TCA_FLOWER_CLASSID’? > {TCA_TCINDEX_CLASSID, &clsid, sizeof(clsid), NULL}, > ^~~~~~~~~~~~~~~~~~~ > TCA_FLOWER_CLASSID > The errors were due to the removal of certain TCA_TCINDEX_ constants > from the kernel's user API (uapi), as indicated by the kernel commit. > commit 82b2545ed9a (net/sched: Remove uapi support for tcindex classifier) > The reason why I didn't add this into LTP library is that the defines > have been dropped we just need to satisfy this one case. +1 for adding enum here instead of writing m4 autotools check. > Notes: > We need to merge this patch before the new release. +1 Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr
Hi, thanks for the fix. I agree that this doesn't need to go into LAPI headers since we probably won't add another test for tcindex. Reviewed-by: Martin Doucha <mdoucha@suse.cz> On 16. 01. 24 4:17, Li Wang wrote: > The change addresses compilation errors encountered in the tcindex01.c > test when compiled against kernel-6.7 and above. > > tcindex01.c:67:4: error: ‘TCA_TCINDEX_MASK’ undeclared here (not in a function); > did you mean ‘TCA_CODEL_MAX’? > {TCA_TCINDEX_MASK, &mask, sizeof(mask), NULL}, > ^~~~~~~~~~~~~~~~ > TCA_CODEL_MAX > tcindex01.c:68:4: error: ‘TCA_TCINDEX_SHIFT’ undeclared here (not in a function); > did you mean ‘TCA_FLOW_RSHIFT’? > {TCA_TCINDEX_SHIFT, &shift, sizeof(shift), NULL}, > ^~~~~~~~~~~~~~~~~ > TCA_FLOW_RSHIFT > CC testcases/cve/cve-2016-7117 > CC utils/sctp/func_tests/test_getname_v6.o > tcindex01.c:69:4: error: ‘TCA_TCINDEX_CLASSID’ undeclared here (not in a function); > did you mean ‘TCA_FLOWER_CLASSID’? > {TCA_TCINDEX_CLASSID, &clsid, sizeof(clsid), NULL}, > ^~~~~~~~~~~~~~~~~~~ > TCA_FLOWER_CLASSID > > The errors were due to the removal of certain TCA_TCINDEX_ constants > from the kernel's user API (uapi), as indicated by the kernel commit. > > commit 82b2545ed9a (net/sched: Remove uapi support for tcindex classifier) > > The reason why I didn't add this into LTP library is that the defines > have been dropped we just need to satisfy this one case. > --- > > Notes: > We need to merge this patch before the new release. > > testcases/cve/tcindex01.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/testcases/cve/tcindex01.c b/testcases/cve/tcindex01.c > index b4f2bb01a..70e5639f1 100644 > --- a/testcases/cve/tcindex01.c > +++ b/testcases/cve/tcindex01.c > @@ -32,6 +32,23 @@ > > #define DEVNAME "ltp_dummy1" > > +#ifndef TCA_TCINDEX_MAX > +enum { > + TCA_TCINDEX_UNSPEC, > + TCA_TCINDEX_HASH, > + TCA_TCINDEX_MASK, > + TCA_TCINDEX_SHIFT, > + TCA_TCINDEX_FALL_THROUGH, > + TCA_TCINDEX_CLASSID, > + TCA_TCINDEX_POLICE, > + TCA_TCINDEX_ACT, > + __TCA_TCINDEX_MAX > +}; > + > +#define TCA_TCINDEX_MAX (__TCA_TCINDEX_MAX - 1) > +#endif > + > + > static const uint32_t qd_handle = TC_H_MAKE(1 << 16, 0); > static const uint32_t clsid = TC_H_MAKE(1 << 16, 1); > static const uint32_t shift = 10;
Hi Li, Martin, thanks for your review, Martin! > Hi, > thanks for the fix. I agree that this doesn't need to go into LAPI headers > since we probably won't add another test for tcindex. And if yes, we would simply move it to lapi. Li, please go ahead and merge. Kind regards, Petr
On Tue, Jan 16, 2024 at 8:12 PM Petr Vorel <pvorel@suse.cz> wrote: > Hi Li,, > > > The change addresses compilation errors encountered in the tcindex01.c > > test when compiled against kernel-6.7 and above. > > "against kernel-6.7". But 82b2545ed9a will be released in 6.8, right? > It's already contained in the kernel-6.7 release. Patch merged, thanks!
> On Tue, Jan 16, 2024 at 8:12 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Li,, > > > The change addresses compilation errors encountered in the tcindex01.c > > > test when compiled against kernel-6.7 and above. > > "against kernel-6.7". But 82b2545ed9a will be released in 6.8, right? > It's already contained in the kernel-6.7 release. Well, it's not that important (and was already merged anyway), but just for clarity: on updated kernel tree (with fetched tags): $ git tag --contains 82b2545ed9a next-20240104 next-20240105 next-20240109 next-20240110 next-20240112 Also, LTP tree without this patch compiles on Tumbleweed with 6.7 kernel headers. gitk shows: $ gitk -1 82b2545ed9a Follows: v6.7-rc6 Precedes: next-20240104, next-20240105, next-20240109, next-20240110, next-20240112 => IMHO netdev merge window started at v6.7-rc6, but it's window for v6.8-rc1. > Patch merged, thanks! Thanks for spotting this early enough! Kind regards, Petr
On Mon, Jan 15, 2024 at 10:17 PM Li Wang <liwang@redhat.com> wrote: > > The change addresses compilation errors encountered in the tcindex01.c > test when compiled against kernel-6.7 and above. > > tcindex01.c:67:4: error: ‘TCA_TCINDEX_MASK’ undeclared here (not in a function); > did you mean ‘TCA_CODEL_MAX’? > {TCA_TCINDEX_MASK, &mask, sizeof(mask), NULL}, > ^~~~~~~~~~~~~~~~ > TCA_CODEL_MAX > tcindex01.c:68:4: error: ‘TCA_TCINDEX_SHIFT’ undeclared here (not in a function); > did you mean ‘TCA_FLOW_RSHIFT’? > {TCA_TCINDEX_SHIFT, &shift, sizeof(shift), NULL}, > ^~~~~~~~~~~~~~~~~ > TCA_FLOW_RSHIFT > CC testcases/cve/cve-2016-7117 > CC utils/sctp/func_tests/test_getname_v6.o > tcindex01.c:69:4: error: ‘TCA_TCINDEX_CLASSID’ undeclared here (not in a function); > did you mean ‘TCA_FLOWER_CLASSID’? > {TCA_TCINDEX_CLASSID, &clsid, sizeof(clsid), NULL}, > ^~~~~~~~~~~~~~~~~~~ > TCA_FLOWER_CLASSID > > The errors were due to the removal of certain TCA_TCINDEX_ constants > from the kernel's user API (uapi), as indicated by the kernel commit. > > commit 82b2545ed9a (net/sched: Remove uapi support for tcindex classifier) > > The reason why I didn't add this into LTP library is that the defines > have been dropped we just need to satisfy this one case. > --- > > Notes: > We need to merge this patch before the new release. > > testcases/cve/tcindex01.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > Pardon my ignorance - what is this tree? I dont recall seeing this anywhere. If you pull uapi headers from the kernel on your tree you can catch these deletions sooner.. cheers, jamal > diff --git a/testcases/cve/tcindex01.c b/testcases/cve/tcindex01.c > index b4f2bb01a..70e5639f1 100644 > --- a/testcases/cve/tcindex01.c > +++ b/testcases/cve/tcindex01.c > @@ -32,6 +32,23 @@ > > #define DEVNAME "ltp_dummy1" > > +#ifndef TCA_TCINDEX_MAX > +enum { > + TCA_TCINDEX_UNSPEC, > + TCA_TCINDEX_HASH, > + TCA_TCINDEX_MASK, > + TCA_TCINDEX_SHIFT, > + TCA_TCINDEX_FALL_THROUGH, > + TCA_TCINDEX_CLASSID, > + TCA_TCINDEX_POLICE, > + TCA_TCINDEX_ACT, > + __TCA_TCINDEX_MAX > +}; > + > +#define TCA_TCINDEX_MAX (__TCA_TCINDEX_MAX - 1) > +#endif > + > + > static const uint32_t qd_handle = TC_H_MAKE(1 << 16, 0); > static const uint32_t clsid = TC_H_MAKE(1 << 16, 1); > static const uint32_t shift = 10; > -- > 2.40.1 >
Hi Jamal, Jamal Hadi Salim <jhs@mojatatu.com> wrote: Pardon my ignorance - what is this tree? I dont recall seeing this > anywhere. If you pull uapi headers from the kernel on your tree you > can catch these deletions sooner.. > This is an LTP (Linux Test Project <https://linux-test-project.github.io/>) that targets testing the Linux kernel. The commit 82b2545ed9a you made in kernel recently caused the LTP compile break on the latest mainline kernel. So that's why I CCed you in this patch. BTW, It would be helpful to run some LTP test cases against our kernel-patch before sent to LKML next time.
Hi Li Wang, On Wed, Jan 17, 2024 at 7:49 PM Li Wang <liwang@redhat.com> wrote: > > Hi Jamal, > > Jamal Hadi Salim <jhs@mojatatu.com> wrote: > >> Pardon my ignorance - what is this tree? I dont recall seeing this >> anywhere. If you pull uapi headers from the kernel on your tree you >> can catch these deletions sooner.. > > > This is an LTP (Linux Test Project) that targets testing the Linux kernel. > > The commit 82b2545ed9a you made in kernel recently caused the LTP > compile break on the latest mainline kernel. So that's why I CCed you > in this patch. > > BTW, It would be helpful to run some LTP test cases against our kernel-patch > before sent to LKML next time. > Take a look at NIPA. It allows distributed testing - meaning you can run LTP tests on your machine(s) and then share the results. There's a doc here: https://docs.google.com/document/d/1TPlOOvv0GaopC3fzW-wiq8TYpl7rh8Vl_mmal0uFeJc/edit It currently it works on kernel branch snapshots - which at minimal would have caught the issue you fixed on LTP with tcindex. cheers, jamal
diff --git a/testcases/cve/tcindex01.c b/testcases/cve/tcindex01.c index b4f2bb01a..70e5639f1 100644 --- a/testcases/cve/tcindex01.c +++ b/testcases/cve/tcindex01.c @@ -32,6 +32,23 @@ #define DEVNAME "ltp_dummy1" +#ifndef TCA_TCINDEX_MAX +enum { + TCA_TCINDEX_UNSPEC, + TCA_TCINDEX_HASH, + TCA_TCINDEX_MASK, + TCA_TCINDEX_SHIFT, + TCA_TCINDEX_FALL_THROUGH, + TCA_TCINDEX_CLASSID, + TCA_TCINDEX_POLICE, + TCA_TCINDEX_ACT, + __TCA_TCINDEX_MAX +}; + +#define TCA_TCINDEX_MAX (__TCA_TCINDEX_MAX - 1) +#endif + + static const uint32_t qd_handle = TC_H_MAKE(1 << 16, 0); static const uint32_t clsid = TC_H_MAKE(1 << 16, 1); static const uint32_t shift = 10;
The change addresses compilation errors encountered in the tcindex01.c test when compiled against kernel-6.7 and above. tcindex01.c:67:4: error: ‘TCA_TCINDEX_MASK’ undeclared here (not in a function); did you mean ‘TCA_CODEL_MAX’? {TCA_TCINDEX_MASK, &mask, sizeof(mask), NULL}, ^~~~~~~~~~~~~~~~ TCA_CODEL_MAX tcindex01.c:68:4: error: ‘TCA_TCINDEX_SHIFT’ undeclared here (not in a function); did you mean ‘TCA_FLOW_RSHIFT’? {TCA_TCINDEX_SHIFT, &shift, sizeof(shift), NULL}, ^~~~~~~~~~~~~~~~~ TCA_FLOW_RSHIFT CC testcases/cve/cve-2016-7117 CC utils/sctp/func_tests/test_getname_v6.o tcindex01.c:69:4: error: ‘TCA_TCINDEX_CLASSID’ undeclared here (not in a function); did you mean ‘TCA_FLOWER_CLASSID’? {TCA_TCINDEX_CLASSID, &clsid, sizeof(clsid), NULL}, ^~~~~~~~~~~~~~~~~~~ TCA_FLOWER_CLASSID The errors were due to the removal of certain TCA_TCINDEX_ constants from the kernel's user API (uapi), as indicated by the kernel commit. commit 82b2545ed9a (net/sched: Remove uapi support for tcindex classifier) The reason why I didn't add this into LTP library is that the defines have been dropped we just need to satisfy this one case. --- Notes: We need to merge this patch before the new release. testcases/cve/tcindex01.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)