On Wed, Jun 5, 2024 at 12:12 PM Lorenzo Bianconi < lorenzo.bianc...@redhat.com> wrote:
> > On 5/14/24 18:44, Lorenzo Bianconi wrote: > > > Skip proxy-arp logical flows for traffic that is entering the logical > > > switch pipeline from a lsp of type router. This patch will avoid > > > recirculating back the traffic entering by the logical router > > > pipeline if proxy-arp hasn been configured by the CMS. > > > > > > Reported-at: https://issues.redhat.com/browse/FDP-96 > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com> > > > --- > > > > Hi Lorenzo, > > > > This change looks OK but I'd like to double check that it doesn't break > > CNV/ovn-kubernetes use cases. > > > > Hi Enrique, > > > > Would you mind double checking that on one of your setups? > > > > If I'm not wrong our upstream ovn-kubernetes (in ovn-org/ovn) tests > > don't enable anything CNV specific: > > https://github.com/ovsrobot/ovn/actions/runs/9083258143 > > > > Thanks, > > Dumitru > > Hi Dumitru, > > Thx, for the review. That's a good idea :) > Live migration tests look fine with this change, both IC and non IC, let's also activate the kubevirt lanes so we gate with them too. Tested-by: <ellor...@redhat.com> > > Regards, > Lorenzo > > > > > > northd/northd.c | 15 +++++++++++++-- > > > tests/ovn.at | 8 ++++---- > > > tests/system-ovn.at | 22 +++++++++++++++++++++- > > > 3 files changed, 38 insertions(+), 7 deletions(-) > > > > > > diff --git a/northd/northd.c b/northd/northd.c > > > index 0cabda7ea..29dc58ef4 100644 > > > --- a/northd/northd.c > > > +++ b/northd/northd.c > > > @@ -118,6 +118,7 @@ static bool default_acl_drop; > > > #define REGBIT_PORT_SEC_DROP "reg0[15]" > > > #define REGBIT_ACL_STATELESS "reg0[16]" > > > #define REGBIT_ACL_HINT_ALLOW_REL "reg0[17]" > > > +#define REGBIT_FROM_ROUTER_PORT "reg0[18]" > > > > > > #define REG_ORIG_DIP_IPV4 "reg1" > > > #define REG_ORIG_DIP_IPV6 "xxreg1" > > > @@ -5785,6 +5786,13 @@ build_lswitch_port_sec_op(struct ovn_port *op, > struct lflow_table *lflows, > > > &op->od->localnet_ports[0]->nbsp->header_, > > > op->lflow_ref); > > > } > > > + } else if (lsp_is_router(op->nbsp)) { > > > + ds_put_format(actions, REGBIT_FROM_ROUTER_PORT" = 1; next;"); > > > + ovn_lflow_add_with_lport_and_hint(lflows, op->od, > > > + S_SWITCH_IN_CHECK_PORT_SEC, > 70, > > > + ds_cstr(match), > ds_cstr(actions), > > > + op->key, &op->nbsp->header_, > > > + op->lflow_ref); > > > } > > > } > > > > > > @@ -9051,7 +9059,9 @@ build_lswitch_arp_nd_responder_known_ips(struct > ovn_port *op, > > > if (op->proxy_arp_addrs.n_ipv4_addrs) { > > > /* Match rule on all proxy ARP IPs. */ > > > ds_clear(match); > > > - ds_put_cstr(match, "arp.op == 1 && arp.tpa == {"); > > > + ds_put_cstr(match, > > > + REGBIT_FROM_ROUTER_PORT" == 0 " > > > + "&& arp.op == 1 && arp.tpa == {"); > > > > > > for (i = 0; i < op->proxy_arp_addrs.n_ipv4_addrs; i++) { > > > ds_put_format(match, "%s/%u,", > > > @@ -9105,7 +9115,8 @@ build_lswitch_arp_nd_responder_known_ips(struct > ovn_port *op, > > > ds_truncate(&nd_target_match, nd_target_match.length - 2); > > > ds_clear(match); > > > ds_put_format(match, > > > - "nd_ns " > > > + REGBIT_FROM_ROUTER_PORT" == 0 " > > > + "&& nd_ns " > > > "&& ip6.dst == { %s } " > > > "&& nd.target == { %s }", > > > ds_cstr(&ip6_dst_match), > > > diff --git a/tests/ovn.at b/tests/ovn.at > > > index 486680649..e419516a7 100644 > > > --- a/tests/ovn.at > > > +++ b/tests/ovn.at > > > @@ -32640,7 +32640,7 @@ AT_CHECK([ovn-sbctl dump-flows | > > > grep ls_in_arp_rsp | > > > grep "${arp_proxy_ls1[[1]]}" | > > > ovn_strip_lflows], [0], [dnl > > > - table=??(ls_in_arp_rsp ), priority=30 , match=(arp.op == 1 > && dnl > > > + table=??(ls_in_arp_rsp ), priority=30 , match=(reg0[[18]] == > 0 && arp.op == 1 && dnl > > > arp.tpa == {169.254.238.0/24,169.254.239.2/32} > <http://169.254.238.0/24,169.254.239.2/32%7D>), dnl > > > action=(eth.dst = eth.src; eth.src = 00:00:00:01:02:f1; arp.op = 2; > dnl > > > /* ARP reply */ arp.tha = arp.sha; arp.sha = 00:00:00:01:02:f1; dnl > > > @@ -32653,7 +32653,7 @@ AT_CHECK([ovn-sbctl dump-flows | > > > grep "${arp_proxy_ls1[[3]]}" | > > > ovn_strip_lflows], [0], [dnl > > > table=??(ls_in_arp_rsp ), priority=30 , dnl > > > -match=(nd_ns && ip6.dst == { fd7b:6b4d:7b25:d22d::/64, > ff02::1:ff00:0/64, dnl > > > +match=(reg0[[18]] == 0 && nd_ns && ip6.dst == { > fd7b:6b4d:7b25:d22d::/64, ff02::1:ff00:0/64, dnl > > > fd7b:6b4d:7b25:d22f::1/128, ff02::1:ff00:1/128 } && dnl > > > nd.target == { fd7b:6b4d:7b25:d22d::/64, fd7b:6b4d:7b25:d22f::1/128 > }), dnl > > > action=(nd_na_router { eth.src = 00:00:00:01:02:f1; ip6.src = > nd.target; dnl > > > @@ -32667,7 +32667,7 @@ AT_CHECK([ovn-sbctl dump-flows | > > > grep "${arp_proxy_ls2[[2]]}" | > > > ovn_strip_lflows], [0], [dnl > > > table=??(ls_in_arp_rsp ), priority=30 , dnl > > > -match=(arp.op == 1 && arp.tpa == {169.254.236.0/24,169.254.237.2/32} > <http://169.254.236.0/24,169.254.237.2/32%7D>), dnl > > > +match=(reg0[[18]] == 0 && arp.op == 1 && arp.tpa == { > 169.254.236.0/24,169.254.237.2/32} > <http://169.254.236.0/24,169.254.237.2/32%7D>), dnl > > > action=(eth.dst = eth.src; eth.src = 00:00:00:02:02:f1; arp.op = 2; > dnl > > > /* ARP reply */ arp.tha = arp.sha; arp.sha = 00:00:00:02:02:f1; dnl > > > arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;) > > > @@ -32679,7 +32679,7 @@ AT_CHECK([ovn-sbctl dump-flows | > > > grep "${arp_proxy_ls2[[4]]}" | > > > ovn_strip_lflows], [0], [dnl > > > table=??(ls_in_arp_rsp ), priority=30 , dnl > > > -match=(nd_ns && ip6.dst == { fd7b:6b4d:7b25:d22b::/64, > ff02::1:ff00:0/64, dnl > > > +match=(reg0[[18]] == 0 && nd_ns && ip6.dst == { > fd7b:6b4d:7b25:d22b::/64, ff02::1:ff00:0/64, dnl > > > fd7b:6b4d:7b25:d22c::1/128, ff02::1:ff00:1/128 } && dnl > > > nd.target == { fd7b:6b4d:7b25:d22b::/64, fd7b:6b4d:7b25:d22c::1/128 > }), dnl > > > action=(nd_na_router { eth.src = 00:00:00:02:02:f1; ip6.src = > nd.target; dnl > > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > > index 86fd240d2..e6cfb07f6 100644 > > > --- a/tests/system-ovn.at > > > +++ b/tests/system-ovn.at > > > @@ -10756,7 +10756,7 @@ check ovn-nbctl ls-add bar > > > # Connect foo to R1 > > > check ovn-nbctl lrp-add R1 foo 00:00:01:01:02:03 192.168.1.1/24 > > > check ovn-nbctl lsp-add foo rp-foo -- set Logical_Switch_Port rp-foo \ > > > - type=router options:arp_proxy="0a:58:a9:fe:01:01 169.254.239.254 > 169.254.239.2 169.254.238.0/24 192.168.1.100" options:router-port=foo > addresses='"router"' > > > + type=router options:arp_proxy="0a:58:a9:fe:01:01 169.254.239.254 > 169.254.239.2 169.254.238.0/24 192.168.1.100 192.168.1.200" > options:router-port=foo addresses='"router"' > > > > > > # Connect bar to R1 > > > check ovn-nbctl lrp-add R1 bar 00:00:01:01:02:04 192.168.2.1/24 > > > @@ -10785,6 +10785,12 @@ ADD_VETH(foo3, foo3, br-int, "192.168.1.4/24", > "f0:00:00:01:02:05", \ > > > check ovn-nbctl lsp-add foo foo3 \ > > > -- lsp-set-addresses foo3 "f0:00:00:01:02:05 192.168.1.4" > > > > > > +ADD_NAMESPACES(foo4) > > > +ADD_VETH(foo4, foo4, br-int, "192.168.1.6/24", "f0:00:00:01:02:11", \ > > > + "192.168.1.1") > > > +check ovn-nbctl lsp-add foo foo4 \ > > > +-- lsp-set-addresses foo4 "f0:00:00:01:02:11 192.168.1.6" > > > + > > > # Logical port 'ext1' in switch 'foo' > > > ADD_NAMESPACES(ext1) > > > ADD_VETH(ext1, ext1, br-ext, "192.168.1.5/24", "f0:00:00:01:02:06", \ > > > @@ -10800,6 +10806,12 @@ ADD_VETH(bar1, bar1, br-int, "192.168.2.2/24", > "f0:00:00:01:02:07", \ > > > check ovn-nbctl lsp-add bar bar1 \ > > > -- lsp-set-addresses bar1 "f0:00:00:01:02:07 192.168.2.2" > > > > > > +ADD_NAMESPACES(bar2) > > > +ADD_VETH(bar2, bar2, br-int, "192.168.2.3/24", "f0:00:00:01:02:10", \ > > > +"192.168.2.1") > > > +check ovn-nbctl lsp-add bar bar2 \ > > > +-- lsp-set-addresses bar2 "f0:00:00:01:10:10 192.168.2.3" > > > + > > > # wait for ovn-controller to catch up. > > > check ovn-nbctl --wait=hv sync > > > > > > @@ -10851,6 +10863,14 @@ OVS_WAIT_UNTIL([ > > > test "${total_pkts}" = "3" > > > ]) > > > > > > +check ovn-nbctl lr-route-add R1 169.254.240.0/24 192.168.1.200 > > > +NETNS_START_TCPDUMP([foo4], [-nn -c 4 -e -i foo4 > arp[[24:4]]=0xc0a801c8], [foo4-arp]) > > > + > > > +NS_CHECK_EXEC([bar2], [ping -q -c 5 -i 0.3 -w 2 > 169.254.240.10],[ignore],[ignore]) > > > +OVS_WAIT_UNTIL([ > > > + total_pkts=$(cat foo4-arp.tcpdump| wc -l) > > > + test "${total_pkts}" = "4" > > > +]) > > > > > > OVS_APP_EXIT_AND_WAIT([ovn-controller]) > > > > > > -- *Quique Llorente* CNV networking Senior Software Engineer Red Hat EMEA <https://www.redhat.com/> ellor...@redhat.com <arxc...@redhat.com> @RedHat <https://twitter.com/redhat> Red Hat <https://www.linkedin.com/company/red-hat> Red Hat <https://www.facebook.com/RedHatInc> <https://www.redhat.com/> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
