Message ID | 1551285539-31225-1-git-send-email-nitin.katiyar@ericsson.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev,1/2] Avoid packet drop on LACP bond after link up | expand |
Bleep bloop. Greetings Nitin Katiyar, 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: /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/jhash.lo -MD -MP -MF $depbase.Tpo -c -o lib/jhash.lo lib/jhash.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/jhash.lo -MD -MP -MF lib/.deps/jhash.Tpo -c lib/jhash.c -o lib/jhash.o depbase=`echo lib/json.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/json.lo -MD -MP -MF $depbase.Tpo -c -o lib/json.lo lib/json.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/json.lo -MD -MP -MF lib/.deps/json.Tpo -c lib/json.c -o lib/json.o depbase=`echo lib/jsonrpc.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/jsonrpc.lo -MD -MP -MF $depbase.Tpo -c -o lib/jsonrpc.lo lib/jsonrpc.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/jsonrpc.lo -MD -MP -MF lib/.deps/jsonrpc.Tpo -c lib/jsonrpc.c -o lib/jsonrpc.o depbase=`echo lib/lacp.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/lacp.lo -MD -MP -MF $depbase.Tpo -c -o lib/lacp.lo lib/lacp.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/lacp.lo -MD -MP -MF lib/.deps/lacp.Tpo -c lib/lacp.c -o lib/lacp.o lib/lacp.c: In function 'lacp_process_packet': lib/lacp.c:329:52: error: unused parameter 'bond' [-Werror=unused-parameter] lacp_process_packet(struct lacp *lacp, const void *bond, const void *slave_, ^ cc1: all warnings being treated as errors make[2]: *** [lib/lacp.lo] Error 1 make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make: *** [all] Error 2 Please check this out. If you feel there has been an error, please email aconole@bytheb.org Thanks, 0-day Robot
Nitin Katiyar <nitin.katiyar@ericsson.com> writes: > Hi, > I got this mail reporting build failure. It complains for bond > variable not being used. I had sent patch earlier after correcting > this. So, I am not sure which patch version it is complaining for. In > my latest patch I see that it is being used. Am I missing something? Looking at the message ID to which you're replying: Robot mail: 1551257882-9544-1-git-send-email-robot@bytheb.org Looking at that message id, it is a reply to: Nitin Katiyar: 1551285539-31225-1-git-send-email-nitin.katiyar@ericsson.com Looking at that message, I see that it includes: @@ -324,8 +325,8 @@ lacp_is_active(const struct lacp *lacp) OVS_EXCLUDED(mutex) /* Processes 'packet' which was received on 'slave_'. This function should be * called on all packets received on 'slave_' with Ethernet Type ETH_TYPE_LACP. */ -void -lacp_process_packet(struct lacp *lacp, const void *slave_, +bool +lacp_process_packet(struct lacp *lacp, const void *bond, const void *slave_, const struct dp_packet *packet) OVS_EXCLUDED(mutex) { Which indeed looks like the bond argument is added, but never referenced. > On other hand, it will be good if report captures the patch or version of the patch. I agree, I'll fix that up. On the other hand, the email in question is at url: https://mail.openvswitch.org/pipermail/ovs-dev/2019-February/356740.html It does not contain a version string. So it seems this is a valid report. > Regards, > Nitin > > -----Original Message----- > From: 0-day Robot [mailto:robot@bytheb.org] > Sent: Wednesday, February 27, 2019 2:28 PM > To: Nitin Katiyar <nitin.katiyar@ericsson.com> > Cc: dev@openvswitch.org > Subject: Re: Avoid packet drop on LACP bond after link up > > Bleep bloop. Greetings Nitin Katiyar, 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: > /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/jhash.lo -MD -MP -MF $depbase.Tpo -c -o > lib/jhash.lo lib/jhash.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/jhash.lo -MD -MP -MF lib/.deps/jhash.Tpo -c > lib/jhash.c -o lib/jhash.o depbase=`echo lib/json.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/json.lo -MD -MP -MF $depbase.Tpo -c -o > lib/json.lo lib/json.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/json.lo -MD -MP -MF lib/.deps/json.Tpo -c > lib/json.c -o lib/json.o depbase=`echo lib/jsonrpc.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/jsonrpc.lo -MD -MP -MF $depbase.Tpo -c -o > lib/jsonrpc.lo lib/jsonrpc.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/jsonrpc.lo -MD -MP -MF lib/.deps/jsonrpc.Tpo -c > lib/jsonrpc.c -o lib/jsonrpc.o depbase=`echo lib/lacp.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/lacp.lo -MD -MP -MF $depbase.Tpo -c -o > lib/lacp.lo lib/lacp.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/lacp.lo -MD -MP -MF lib/.deps/lacp.Tpo -c > lib/lacp.c -o lib/lacp.o > lib/lacp.c: In function 'lacp_process_packet': > lib/lacp.c:329:52: error: unused parameter 'bond' > [-Werror=unused-parameter] lacp_process_packet(struct lacp *lacp, > const void *bond, const void *slave_, > ^ > cc1: all warnings being treated as errors > make[2]: *** [lib/lacp.lo] Error 1 > make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' > make[1]: *** [all-recursive] Error 1 > make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' > make: *** [all] Error 2 > > > Please check this out. If you feel there has been an error, please email aconole@bytheb.org > > Thanks, > 0-day Robot
This series adds a warning: ../lib/lacp.c:360:47: error: passing 'const void *' to parameter of type 'void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] ../ofproto/bond.h:83:60: note: passing argument to parameter 'slave_' here Please fix it. Thanks!
Aaron Conole <aconole@bytheb.org> writes: > Nitin Katiyar <nitin.katiyar@ericsson.com> writes: > >> Hi, >> I got this mail reporting build failure. It complains for bond >> variable not being used. I had sent patch earlier after correcting >> this. So, I am not sure which patch version it is complaining for. In >> my latest patch I see that it is being used. Am I missing something? > > Looking at the message ID to which you're replying: > > Robot mail: 1551257882-9544-1-git-send-email-robot@bytheb.org > > Looking at that message id, it is a reply to: > > Nitin Katiyar: 1551285539-31225-1-git-send-email-nitin.katiyar@ericsson.com > > Looking at that message, I see that it includes: > > @@ -324,8 +325,8 @@ lacp_is_active(const struct lacp *lacp) OVS_EXCLUDED(mutex) > /* Processes 'packet' which was received on 'slave_'. This function should be > * called on all packets received on 'slave_' with Ethernet Type ETH_TYPE_LACP. > */ > -void > -lacp_process_packet(struct lacp *lacp, const void *slave_, > +bool > +lacp_process_packet(struct lacp *lacp, const void *bond, const void *slave_, > const struct dp_packet *packet) > OVS_EXCLUDED(mutex) > { > > > Which indeed looks like the bond argument is added, but never referenced. > >> On other hand, it will be good if report captures the patch or version of the patch. > > I agree, I'll fix that up. Just following up. This should be fixed now. > On the other hand, the email in question is at url: > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-February/356740.html > > It does not contain a version string. So it seems this is a valid > report. > >> Regards, >> Nitin >> >> -----Original Message----- >> From: 0-day Robot [mailto:robot@bytheb.org] >> Sent: Wednesday, February 27, 2019 2:28 PM >> To: Nitin Katiyar <nitin.katiyar@ericsson.com> >> Cc: dev@openvswitch.org >> Subject: Re: Avoid packet drop on LACP bond after link up >> >> Bleep bloop. Greetings Nitin Katiyar, 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: >> /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/jhash.lo -MD -MP -MF $depbase.Tpo -c -o >> lib/jhash.lo lib/jhash.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/jhash.lo -MD -MP -MF lib/.deps/jhash.Tpo -c >> lib/jhash.c -o lib/jhash.o depbase=`echo lib/json.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/json.lo -MD -MP -MF $depbase.Tpo -c -o >> lib/json.lo lib/json.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/json.lo -MD -MP -MF lib/.deps/json.Tpo -c >> lib/json.c -o lib/json.o depbase=`echo lib/jsonrpc.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/jsonrpc.lo -MD -MP -MF $depbase.Tpo -c -o >> lib/jsonrpc.lo lib/jsonrpc.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/jsonrpc.lo -MD -MP -MF lib/.deps/jsonrpc.Tpo -c >> lib/jsonrpc.c -o lib/jsonrpc.o depbase=`echo lib/lacp.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/lacp.lo -MD -MP -MF $depbase.Tpo -c -o >> lib/lacp.lo lib/lacp.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/lacp.lo -MD -MP -MF lib/.deps/lacp.Tpo -c >> lib/lacp.c -o lib/lacp.o >> lib/lacp.c: In function 'lacp_process_packet': >> lib/lacp.c:329:52: error: unused parameter 'bond' >> [-Werror=unused-parameter] lacp_process_packet(struct lacp *lacp, >> const void *bond, const void *slave_, >> ^ >> cc1: all warnings being treated as errors >> make[2]: *** [lib/lacp.lo] Error 1 >> make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' >> make[1]: *** [all-recursive] Error 1 >> make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' >> make: *** [all] Error 2 >> >> >> Please check this out. If you feel there has been an error, please email aconole@bytheb.org >> >> Thanks, >> 0-day Robot
======== During port DOWN->UP of link (slave) in a LACP bond, after receiving the LACPDU with SYNC set for both actor and partner, the bond-slave remains "disabled" until OVS main thread runs LACP state machine and eventually "enables" the bond-slave. With this, we have observed delays in the order of 350ms and packets are dropped in OVS due to bond-admissibility check (packets received on slave in "disabled" state are dropped). Fix: ==== When a LACPDU is received, evaluate whether LACP slave can be enabled (slave_may_enable()) and set LACP slave's may_enable from the datapath thread itself. When may_enable = TRUE, it means L1 state is UP and LACP-SYNC is done and it is waiting for the main thread to enable the slave. Relax the check in bond_check_admissibility() to check for both "enable" and "may_enable" of the LACP slave. This would avoid dropping of packets until the main thread enables the slave from bundle_run(). Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com> Signed-off-by: Manohar Krishnappa Chidambaraswamy <manukc@gmail.com> Co-authored-by: Manohar Krishnappa Chidambaraswamy <manukc@gmail.com> --- lib/lacp.c | 12 ++++++++++-- lib/lacp.h | 3 ++- ofproto/bond.c | 16 +++++++++++++--- ofproto/ofproto-dpif-xlate.c | 11 ++++++++++- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/lib/lacp.c b/lib/lacp.c index d6b36aa..7ac85f9 100644 --- a/lib/lacp.c +++ b/lib/lacp.c @@ -154,6 +154,7 @@ static struct slave *slave_lookup(const struct lacp *, const void *slave) OVS_REQUIRES(mutex); static bool info_tx_equal(struct lacp_info *, struct lacp_info *) OVS_REQUIRES(mutex); +static bool slave_may_enable__(struct slave *slave) OVS_REQUIRES(mutex); static unixctl_cb_func lacp_unixctl_show; static unixctl_cb_func lacp_unixctl_show_stats; @@ -324,8 +325,8 @@ lacp_is_active(const struct lacp *lacp) OVS_EXCLUDED(mutex) /* Processes 'packet' which was received on 'slave_'. This function should be * called on all packets received on 'slave_' with Ethernet Type ETH_TYPE_LACP. */ -void -lacp_process_packet(struct lacp *lacp, const void *slave_, +bool +lacp_process_packet(struct lacp *lacp, const void *bond, const void *slave_, const struct dp_packet *packet) OVS_EXCLUDED(mutex) { @@ -333,6 +334,7 @@ lacp_process_packet(struct lacp *lacp, const void *slave_, const struct lacp_pdu *pdu; long long int tx_rate; struct slave *slave; + bool lacp_may_enable = false; lacp_lock(); slave = slave_lookup(lacp, slave_); @@ -362,8 +364,14 @@ lacp_process_packet(struct lacp *lacp, const void *slave_, slave->partner = pdu->actor; } + /* Evaluate may_enable here to avoid dropping of packets till main thread + * sets may_enable to true. */ + lacp_may_enable = slave_may_enable__(slave); + out: lacp_unlock(); + + return lacp_may_enable; } /* Returns the lacp_status of the given 'lacp' object (which may be NULL). */ diff --git a/lib/lacp.h b/lib/lacp.h index f35cff5..1505c2c 100644 --- a/lib/lacp.h +++ b/lib/lacp.h @@ -46,7 +46,8 @@ struct lacp *lacp_ref(const struct lacp *); void lacp_configure(struct lacp *, const struct lacp_settings *); bool lacp_is_active(const struct lacp *); -void lacp_process_packet(struct lacp *, const void *slave, +bool lacp_process_packet(struct lacp *, const void *bond, + const void *slave, const struct dp_packet *packet); enum lacp_status lacp_status(const struct lacp *); diff --git a/ofproto/bond.c b/ofproto/bond.c index d2a8b1f..cd6fd33 100644 --- a/ofproto/bond.c +++ b/ofproto/bond.c @@ -794,6 +794,7 @@ bond_check_admissibility(struct bond *bond, const void *slave_, { enum bond_verdict verdict = BV_DROP; struct bond_slave *slave; + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); ovs_rwlock_rdlock(&rwlock); slave = bond_slave_lookup(bond, slave_); @@ -811,7 +812,11 @@ bond_check_admissibility(struct bond *bond, const void *slave_, * drop all incoming traffic except if lacp_fallback_ab is enabled. */ switch (bond->lacp_status) { case LACP_NEGOTIATED: - verdict = slave->enabled ? BV_ACCEPT : BV_DROP; + /* To reduce packet-drops due to delay in enabling of slave (post + * LACP-SYNC), from main thread, check for may_enable as well. + * When may_enable is TRUE, it means LACP is UP and waiting for the + * main thread to run LACP state machine and enable the slave. */ + verdict = (slave->enabled || slave->may_enable) ? BV_ACCEPT : BV_DROP; goto out; case LACP_CONFIGURED: if (!bond->lacp_fallback_ab) { @@ -847,8 +852,6 @@ bond_check_admissibility(struct bond *bond, const void *slave_, /* Drop all packets which arrive on backup slaves. This is similar to * how Linux bonding handles active-backup bonds. */ if (bond->active_slave != slave) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); - VLOG_DBG_RL(&rl, "active-backup bond received packet on backup" " slave (%s) destined for " ETH_ADDR_FMT, slave->name, ETH_ADDR_ARGS(eth_dst)); @@ -870,6 +873,13 @@ bond_check_admissibility(struct bond *bond, const void *slave_, OVS_NOT_REACHED(); out: + if (slave && (verdict != BV_ACCEPT)) { + VLOG_DBG_RL(&rl, "slave= %s actv-slave= %d may_enable %d enable %d " + "LACP %d verdict(A/D/M=0/1/2) %d\n", slave->name, + (bond->active_slave == slave), slave->may_enable, + slave->enabled, bond->lacp_status, verdict); + } + ovs_rwlock_unlock(&rwlock); return verdict; diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index acd4817..a4a3a2b 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3288,6 +3288,7 @@ process_special(struct xlate_ctx *ctx, const struct xport *xport) const struct xbridge *xbridge = ctx->xbridge; const struct dp_packet *packet = ctx->xin->packet; enum slow_path_reason slow; + bool lacp_may_enable; if (!xport) { slow = 0; @@ -3308,7 +3309,15 @@ process_special(struct xlate_ctx *ctx, const struct xport *xport) } else if (xport->xbundle && xport->xbundle->lacp && flow->dl_type == htons(ETH_TYPE_LACP)) { if (packet) { - lacp_process_packet(xport->xbundle->lacp, xport->ofport, packet); + lacp_may_enable = lacp_process_packet(xport->xbundle->lacp, + xport->xbundle->bond, + xport->ofport, packet); + /* Update LACP status in bond-slave to avoid packet-drops until + * LACP state machine is run by the main thread. */ + if (xport->xbundle->bond && lacp_may_enable) { + bond_slave_set_may_enable(xport->xbundle->bond, xport->ofport, + lacp_may_enable); + } } slow = SLOW_LACP; } else if ((xbridge->stp || xbridge->rstp) &&