| Message ID | 1606837724-23775-1-git-send-email-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,branch-20.09] pinctrl: Honor always_learn_from_arp_request for self created MAC_Bindings. | expand |
Bleep bloop. Greetings Dumitru Ceara, 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:
gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -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 controller/pinctrl.o -MD -MP -MF $depbase.Tpo -c -o controller/pinctrl.o controller/pinctrl.c &&\
mv -f $depbase.Tpo $depbase.Po
depbase=`echo controller/patch.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -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 controller/patch.o -MD -MP -MF $depbase.Tpo -c -o controller/patch.o controller/patch.c &&\
mv -f $depbase.Tpo $depbase.Po
depbase=`echo controller/ovn-controller.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -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 controller/ovn-controller.o -MD -MP -MF $depbase.Tpo -c -o controller/ovn-controller.o controller/ovn-controller.c &&\
mv -f $depbase.Tpo $depbase.Po
depbase=`echo controller/physical.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR -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 controller/physical.o -MD -MP -MF $depbase.Tpo -c -o controller/physical.o controller/physical.c &&\
mv -f $depbase.Tpo $depbase.Po
controller/physical.c: In function ‘put_remote_port_redirect_overlay’:
controller/physical.c:387:23: error: ‘struct ofpact_bundle’ has no member named ‘n_slaves’
if (bundle->n_slaves >= BUNDLE_MAX_SLAVES) {
^
controller/physical.c:387:37: error: ‘BUNDLE_MAX_SLAVES’ undeclared (first use in this function)
if (bundle->n_slaves >= BUNDLE_MAX_SLAVES) {
^
controller/physical.c:387:37: note: each undeclared identifier is reported only once for each function it appears in
controller/physical.c:395:19: error: ‘struct ofpact_bundle’ has no member named ‘n_slaves’
bundle->n_slaves++;
^
make[1]: *** [controller/physical.o] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_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
On Tue, Dec 1, 2020 at 9:19 PM Dumitru Ceara <dceara@redhat.com> wrote: > > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-November/377876.html > Reported-by: Renat Nurgaliyev <impleman@gmail.com> > Suggested-by: Han Zhou <hzhou@ovn.org> > Fixes: a2b88dc51365 ("pinctrl: Directly update MAC_Bindings created by self originated GARPs.") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > (cherry-picked from master commit fdf295d5eb3af74ffb0c5ef950d6b1ad3902599a) Thanks. I applied to branch-20.09 Numan > --- > controller/pinctrl.c | 7 +++++++ > northd/ovn-northd.c | 10 ++++++++++ > tests/ovn.at | 19 +++++++++++++++++++ > 3 files changed, 36 insertions(+) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 9eee432..8372da2 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -3757,6 +3757,13 @@ send_garp_locally(struct ovsdb_idl_txn *ovnsb_idl_txn, > continue; > } > > + /* Skip datapaths that don't automatically learn ARPs from requests. */ > + if (!smap_get_bool(&remote->datapath->external_ids, > + "always_learn_from_arp_request", > + true)) { > + continue; > + } > + > struct ds ip_s = DS_EMPTY_INITIALIZER; > > ip_format_masked(ip, OVS_BE32_MAX, &ip_s); > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index ff3de89..40caed5 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -1138,6 +1138,16 @@ ovn_datapath_update_external_ids(struct ovn_datapath *od) > smap_add(&ids, "interconn-ts", ts); > } > } > + > + if (od->nbr) { > + bool learn_from_arp_request = > + smap_get_bool(&od->nbr->options, "always_learn_from_arp_request", > + true); > + if (!learn_from_arp_request) { > + smap_add(&ids, "always_learn_from_arp_request", "false"); > + } > + } > + > sbrec_datapath_binding_set_external_ids(od->sb, &ids); > smap_destroy(&ids); > } > diff --git a/tests/ovn.at b/tests/ovn.at > index 0ad5dc3..1bd546c 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -15784,6 +15784,25 @@ echo "fffffffffffff0000000010108060001080006040001f00000000101ac1804dd0000000000 > AT_CHECK([sort hv1/br-phys-tx4.pcap], [0], [expout]) > #OVN_CHECK_PACKETS([hv1/br-phys-tx4.pcap], [br-phys.expected]) > > +# Now make sure that always_learn_from_arp_request=false is also honored > +# for locally injected mac bindings. > +check ovn-nbctl set logical_router lr0 options:always_learn_from_arp_request=false > +check ovn-nbctl set logical_router lr1 options:always_learn_from_arp_request=false > +check ovn-nbctl --wait=hv sync > + > +# Recreate two floating IPs, one for each VIF. > +check ovn-nbctl lr-nat-del lr0 dnat_and_snat 172.24.4.100 > +check ovn-nbctl lr-nat-del lr1 dnat_and_snat 172.24.4.200 > + > +check ovn-sbctl --all destroy mac_binding > + > +check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.24.4.100 10.0.0.10 > +check ovn-nbctl lr-nat-add lr1 dnat_and_snat 172.24.4.200 20.0.0.10 > +check ovn-nbctl --wait=hv sync > + > +check_row_count MAC_Binding 0 logical_port=lr0-pub ip=172.24.4.200 > +check_row_count MAC_Binding 0 logical_port=lr1-pub ip=172.24.4.100 > + > OVN_CLEANUP([hv1]) > AT_CLEANUP > > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 9eee432..8372da2 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -3757,6 +3757,13 @@ send_garp_locally(struct ovsdb_idl_txn *ovnsb_idl_txn, continue; } + /* Skip datapaths that don't automatically learn ARPs from requests. */ + if (!smap_get_bool(&remote->datapath->external_ids, + "always_learn_from_arp_request", + true)) { + continue; + } + struct ds ip_s = DS_EMPTY_INITIALIZER; ip_format_masked(ip, OVS_BE32_MAX, &ip_s); diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index ff3de89..40caed5 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -1138,6 +1138,16 @@ ovn_datapath_update_external_ids(struct ovn_datapath *od) smap_add(&ids, "interconn-ts", ts); } } + + if (od->nbr) { + bool learn_from_arp_request = + smap_get_bool(&od->nbr->options, "always_learn_from_arp_request", + true); + if (!learn_from_arp_request) { + smap_add(&ids, "always_learn_from_arp_request", "false"); + } + } + sbrec_datapath_binding_set_external_ids(od->sb, &ids); smap_destroy(&ids); } diff --git a/tests/ovn.at b/tests/ovn.at index 0ad5dc3..1bd546c 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -15784,6 +15784,25 @@ echo "fffffffffffff0000000010108060001080006040001f00000000101ac1804dd0000000000 AT_CHECK([sort hv1/br-phys-tx4.pcap], [0], [expout]) #OVN_CHECK_PACKETS([hv1/br-phys-tx4.pcap], [br-phys.expected]) +# Now make sure that always_learn_from_arp_request=false is also honored +# for locally injected mac bindings. +check ovn-nbctl set logical_router lr0 options:always_learn_from_arp_request=false +check ovn-nbctl set logical_router lr1 options:always_learn_from_arp_request=false +check ovn-nbctl --wait=hv sync + +# Recreate two floating IPs, one for each VIF. +check ovn-nbctl lr-nat-del lr0 dnat_and_snat 172.24.4.100 +check ovn-nbctl lr-nat-del lr1 dnat_and_snat 172.24.4.200 + +check ovn-sbctl --all destroy mac_binding + +check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.24.4.100 10.0.0.10 +check ovn-nbctl lr-nat-add lr1 dnat_and_snat 172.24.4.200 20.0.0.10 +check ovn-nbctl --wait=hv sync + +check_row_count MAC_Binding 0 logical_port=lr0-pub ip=172.24.4.200 +check_row_count MAC_Binding 0 logical_port=lr1-pub ip=172.24.4.100 + OVN_CLEANUP([hv1]) AT_CLEANUP
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-November/377876.html Reported-by: Renat Nurgaliyev <impleman@gmail.com> Suggested-by: Han Zhou <hzhou@ovn.org> Fixes: a2b88dc51365 ("pinctrl: Directly update MAC_Bindings created by self originated GARPs.") Signed-off-by: Dumitru Ceara <dceara@redhat.com> (cherry-picked from master commit fdf295d5eb3af74ffb0c5ef950d6b1ad3902599a) --- controller/pinctrl.c | 7 +++++++ northd/ovn-northd.c | 10 ++++++++++ tests/ovn.at | 19 +++++++++++++++++++ 3 files changed, 36 insertions(+)