On 6/5/24 16:52, Brendan Doyle via dev wrote: > > > So I'm wondering will this break the LSP option: >> *o**p**t**i**o**n**s* *:* *a**r**p**_**p**r**o**x**y*: >> optional string >> Optional. A list of MAC and addresses/cidrs or >> just ad‐ >> dresses/cidrs that this logical >> switch*r**o**u**t**e**r* port will reply to >> ARP/NDP requests. >> Examples:*1**6**9**.**2**5**4**.**2**3**9**.**2**5**4* >> *1**6**9**.**2**5**4**.**2**3**9**.**2*, >> >> *0**a**:**5**8**:**a**9**:**f**e**:**0**1**:**0**1* >> *1**6**9**.**2**5**4**.**2**3**9**.**2**5**4* >> *1**6**9**.**2**5**4**.**2**3**9**.**2* >> >> *1**6**9**.**2**5**4**.**2**3**8**.**0**/**2**4*,*f**d**7**b**:**6**b**4**d**:**7**b**2**5**:**d**2**2**f**:**:**1* >> *f**d**7**b**:**6**b**4**d**:**7**b**2**5**:**d**2**2**f**:**:**2*, >> *0**a**:**5**8**:**a**9**:**f**e**:**0**1**:**0**1* >> *f**d**7**b**:**6**b**4**d**:**7**b**2**5**:**d**2**2**f**:**:**0**/**6**4*. >> The*o**p**t**i**o**n**s**:**r**o**u**t**e**r**-* >> *p**o**r**t*’s logical router should have a route >> to forward packets >> sent to configured proxy ARP MAC/IPs to an appropriate >> destina‐ >> tion. >
Hi Brendan, I'm not sure I understand what breaks. Do you have a specific scenario in mind? The patch is for the arp-proxy feature. I doubt that people rely on ARP requests originated by logical routers being handled by the arp proxy on the connected logical switch port. But I might be wrong. Thanks, Dumitru > > On 05/06/2024 13:03, Enrique Llorente Pastora wrote: >> 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://urldefense.com/v3/__https://issues.redhat.com/browse/FDP-96__;!!ACWV5N9M2RV99hQ!LZCOnrD03VQXa0QBfUgAD0IntqpkCU4P2Mz0jhlfAcJ0Joe1ehsAIloa28JszFGBjchui8U4OL2MfnKJQEhn$ >>>>> 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://urldefense.com/v3/__https://github.com/ovsrobot/ovn/actions/runs/9083258143__;!!ACWV5N9M2RV99hQ!LZCOnrD03VQXa0QBfUgAD0IntqpkCU4P2Mz0jhlfAcJ0Joe1ehsAIloa28JszFGBjchui8U4OL2MflAqga4Q$ >>>> >>>> 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} >>> <https://urldefense.com/v3/__http://169.254.238.0/24,169.254.239.2/32*7D__;JQ!!ACWV5N9M2RV99hQ!LZCOnrD03VQXa0QBfUgAD0IntqpkCU4P2Mz0jhlfAcJ0Joe1ehsAIloa28JszFGBjchui8U4OL2MfjM6HO9o$ >>> >), 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} >>> <https://urldefense.com/v3/__http://169.254.236.0/24,169.254.237.2/32*7D__;JQ!!ACWV5N9M2RV99hQ!LZCOnrD03VQXa0QBfUgAD0IntqpkCU4P2Mz0jhlfAcJ0Joe1ehsAIloa28JszFGBjchui8U4OL2Mfuwz0Yd3$ >>> >), dnl >>>>> +match=(reg0[[18]] == 0 && arp.op == 1 && arp.tpa == { >>> 169.254.236.0/24,169.254.237.2/32} >>> <https://urldefense.com/v3/__http://169.254.236.0/24,169.254.237.2/32*7D__;JQ!!ACWV5N9M2RV99hQ!LZCOnrD03VQXa0QBfUgAD0IntqpkCU4P2Mz0jhlfAcJ0Joe1ehsAIloa28JszFGBjchui8U4OL2Mfuwz0Yd3$ >>> >), 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]) >>>>> >> > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
