| Message ID | 20220124214115.1924071-1-mkp@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v1] tc: Add support for TCA_STATS_PKT64 | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | fail | apply and check: fail |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
Bleep bloop. Greetings Mike Pattrick, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
build:
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/netlink-socket.lo -MD -MP -MF lib/.deps/netlink-socket.Tpo -c lib/netlink-socket.c -o lib/netlink-socket.o
depbase=`echo lib/rtnetlink.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/rtnetlink.lo -MD -MP -MF $depbase.Tpo -c -o lib/rtnetlink.lo lib/rtnetlink.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/rtnetlink.lo -MD -MP -MF lib/.deps/rtnetlink.Tpo -c lib/rtnetlink.c -o lib/rtnetlink.o
depbase=`echo lib/route-table.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/route-table.lo -MD -MP -MF $depbase.Tpo -c -o lib/route-table.lo lib/route-table.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/route-table.lo -MD -MP -MF lib/.deps/route-table.Tpo -c lib/route-table.c -o lib/route-table.o
depbase=`echo lib/tc.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/tc.lo -MD -MP -MF $depbase.Tpo -c -o lib/tc.lo lib/tc.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/tc.lo -MD -MP -MF lib/.deps/tc.Tpo -c lib/tc.c -o lib/tc.o
lib/tc.c:1705:6: error: 'TCA_STATS_PKT64' undeclared here (not in a function)
[TCA_STATS_PKT64] = { .type = NL_A_U64,
^
lib/tc.c:1705:5: error: array index in initializer not of integer type
[TCA_STATS_PKT64] = { .type = NL_A_U64,
^
lib/tc.c:1705:5: error: (near initialization for 'stats_policy')
make[2]: *** [lib/tc.lo] Error 1
make[2]: Leaving directory `/var/lib/jenkins/jobs/0day_robot_upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/0day_robot_upstream_build_from_pw/workspace'
make: *** [all] Error 2
Please check this out. If you feel there has been an error, please email aconole@redhat.com
Thanks,
0-day Robot
diff --git a/lib/tc.c b/lib/tc.c index 38a1dfc0e..88e813088 100644 --- a/lib/tc.c +++ b/lib/tc.c @@ -1702,6 +1702,9 @@ static const struct nl_policy stats_policy[] = { [TCA_STATS_BASIC] = { .type = NL_A_UNSPEC, .min_len = sizeof(struct gnet_stats_basic), .optional = false, }, + [TCA_STATS_PKT64] = { .type = NL_A_U64, + .min_len = sizeof(uint64_t), + .optional = true, }, }; static int @@ -1772,8 +1775,13 @@ nl_parse_single_action(struct nlattr *action, struct tc_flower *flower, } bs = nl_attr_get_unspec(stats_attrs[TCA_STATS_BASIC], sizeof *bs); - if (bs->packets) { - put_32aligned_u64(&stats->n_packets, bs->packets); + if (bs->bytes) { + if (stats_attrs[TCA_STATS_PKT64]) { + uint64_t packets = nl_attr_get_u64(stats_attrs[TCA_STATS_PKT64]); + put_32aligned_u64(&stats->n_packets, packets); + } else { + put_32aligned_u64(&stats->n_packets, bs->packets); + } put_32aligned_u64(&stats->n_bytes, bs->bytes); }
Currently tc offload flow packet counters will roll over every ~4 billion packets. This is because the packet counter in struct tc_stats provided by TCA_STATS_BASIC is a 32bit integer. Now we check for the optional TCA_STATS_PKT64 attribute which provides the full 64bit packet counter if the 32bit one has rolled over. This patch also changes the non-empty check to use bytes, in case the 32bit packet counter has rolled over for this update. Fixes: f98e418fbd ("tc: Add tc flower functions") Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1776816 Signed-off-by: Mike Pattrick <mkp@redhat.com> --- lib/tc.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)