On 07/06/2024 14:20, Dumitru Ceara wrote:
On 6/7/24 12:24, brendan.do...@oracle.com wrote:

On 06/06/2024 17:34, Dumitru Ceara wrote:
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
The arp_proxy option allows an lsp prot connected to an LR to respond
to ARP requests sent from say a VM connected to the same LS.

Right, we have a test for that:

https://urldefense.com/v3/__https://github.com/ovn-org/ovn/blob/main/tests/system-ovn.at*L10721-L10869__;Iw!!ACWV5N9M2RV99hQ!K8qbDDzmm6Fz7yPm3lubG0kMbM_MmKQPqAW5ewepc1iqUAdif248P613_b0CNhmCXnEozbrwpIVCStm3Jg$

And the patch doesn't change it, it just expands it.  Or am I missing
something?

No, I think we are good then just checking.

Regards,
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://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!ACWV5N9M2RV99hQ!O8DJQyfkM12yHUI2p0lARreiYWj6CBQnuFOWboNDonA-W-FvQYibstvjYS5OYCN67lmPyzTRIEY2JVNjyw$

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to