[ovs-dev] [External] : [PATCH ovn] northd-ddlog: Add proxy arp flows for configured addresses in lsp router port.

Numan Siddique numans at ovn.org
Tue Jun 29 20:16:36 UTC 2021


On Tue, Jun 29, 2021 at 2:29 PM Brendan Doyle <brendan.doyle at oracle.com> wrote:
>
>
> Thanks for doing the ddlog for this, comment below
>
> On 29/06/2021 17:08, numans at ovn.org wrote:
> > From: Numan Siddique <numans at ovn.org>
> >
> > The commit [1] didn't add the ddlog part.
> >
> > [1] - 8087cbc7462("ovn-northd.c: Add proxy ARP support to OVN")
> >
> > Signed-off-by: Numan Siddique <numans at ovn.org>
> > ---
> >   northd/ovn.dl        |  1 +
> >   northd/ovn.rs        | 13 +++++++++++++
> >   northd/ovn_northd.dl | 38 ++++++++++++++++++++++++++++++++++++++
> >   tests/ovn.at         |  4 ++--
> >   4 files changed, 54 insertions(+), 2 deletions(-)
> >
> > diff --git a/northd/ovn.dl b/northd/ovn.dl
> > index f23ea3b9e1..3c7a734ddb 100644
> > --- a/northd/ovn.dl
> > +++ b/northd/ovn.dl
> > @@ -364,6 +364,7 @@ extern function is_dynamic_lsp_address(addr: string): bool
> >   extern function extract_lsp_addresses(address: string): Option<lport_addresses>
> >   extern function extract_addresses(address: string): Option<lport_addresses>
> >   extern function extract_lrp_networks(mac: string, networks: Set<string>): Option<lport_addresses>
> > +extern function extract_ip_addresses(address: string): Option<lport_addresses>
> >
> >   extern function split_addresses(addr: string): (Set<string>, Set<string>)
> >
> > diff --git a/northd/ovn.rs b/northd/ovn.rs
> > index d44f83bc75..5f0939409c 100644
> > --- a/northd/ovn.rs
> > +++ b/northd/ovn.rs
> > @@ -184,6 +184,18 @@ pub fn extract_lrp_networks(mac: &String, networks: &ddlog_std::Set<String>) ->
> >       }
> >   }
> >
> > +pub fn extract_ip_addresses(address: &String) -> ddlog_std::Option<lport_addresses> {
> > +    unsafe {
> > +        let mut laddrs: lport_addresses_c = Default::default();
> > +        if ovn_c::extract_ip_addresses(string2cstr(address).as_ptr(),
> > +                                       &mut laddrs as *mut lport_addresses_c) {
> > +            ddlog_std::Option::Some{x: laddrs.into_ddlog()}
> > +        } else {
> > +            ddlog_std::Option::None
> > +        }
> > +    }
> > +}
> > +
> >   pub fn ovn_internal_version() -> String {
> >       unsafe {
> >           let s = ovn_c::ovn_get_internal_version();
> > @@ -623,6 +635,7 @@ mod ovn_c {
> >           pub fn extract_addresses(address: *const raw::c_char, laddrs: *mut lport_addresses_c, ofs: *mut raw::c_int) -> bool;
> >           pub fn extract_lrp_networks__(mac: *const raw::c_char, networks: *const *const raw::c_char,
> >                                         n_networks: libc::size_t, laddrs: *mut lport_addresses_c) -> bool;
> > +        pub fn extract_ip_addresses(address: *const raw::c_char, laddrs: *mut lport_addresses_c) -> bool;
> >           pub fn destroy_lport_addresses(addrs: *mut lport_addresses_c);
> >           pub fn is_dynamic_lsp_address(address: *const raw::c_char) -> bool;
> >           pub fn split_addresses(addresses: *const raw::c_char, ip4_addrs: *mut ovs_svec, ipv6_addrs: *mut ovs_svec);
> > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> > index 52a6206a18..a7a327c7f0 100644
> > --- a/northd/ovn_northd.dl
> > +++ b/northd/ovn_northd.dl
> > @@ -3360,6 +3360,44 @@ for (CheckLspIsUp[check_lsp_is_up]) {
> >       }
> >   }
> >
> > +Flow(.logical_datapath = sw._uuid,
> > +         .stage            = s_SWITCH_IN_ARP_ND_RSP(),
> > +         .priority         = 50,
> > +         .__match          = __match,
> > +         .actions          = __actions,
> > +         .external_ids     = stage_hint(sp.lsp._uuid)) :-
> > +
> > +    sp in &SwitchPort(.sw = sw, .peer = Some{rp}),
> > +    rp.is_enabled(),
> > +    var proxy_ips = {
> > +        match (sp.lsp.options.get("arp_proxy")) {
> > +            None -> "",
> > +            Some {addresses} -> {
> > +                match (extract_ip_addresses(addresses)) {
> > +                    None -> "",
> > +                    Some{addr} -> {
> > +                        var ip4_addrs = vec_empty();
> > +                        for (ip4 in addr.ipv4_addrs) {
> > +                            ip4_addrs.push("${ip4.addr}")
> > +                        };
> > +                        string_join(ip4_addrs, ",")
> > +                    }
> > +                }
> > +            }
> > +        }
> > +    },
> > +    proxy_ips != "",
> > +    var __match = "arp.op == 1 && arp.tpa == {" ++ proxy_ips ++ "}",
> > +    var __actions = "eth.dst = eth.src; "
> > +                    "eth.src = ${rp.networks.ea}; "
> > +                    "arp.op = 2; /* ARP reply */ "
> > +                    "arp.tha = arp.sha; "
> > +                    "arp.sha = %s; "
> > +                    "arp.tpa <-> arp.spa; "
> > +                    "outport = inport; "
> > +                    "flags.loopback = 1; "
> > +                    "output;".
> > +
> >   /* For ND solicitations, we need to listen for both the
> >    * unicast IPv6 address and its all-nodes multicast address,
> >    * but always respond with the unicast IPv6 address. */
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index db1a0a35c2..31f0b90996 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -26940,7 +26940,7 @@ ovs-vsctl -- add-port br-int vif1 -- \
> >   # And proxy ARP flows for 69.254.239.254 and 169.254.239.2
> >   # and check that SB flows have been added.
> >   ovn-nbctl --wait=hv add Logical_Switch_Port rp-ls1 \
> > -options arp_proxy='"169.254.239.254 169.254.239.2"'
> > +options arp_proxy='"169.254.239.254,169.254.239.2"'
> >   ovn-sbctl dump-flows > sbflows
> >   AT_CAPTURE_FILE([sbflows])
> >
> > @@ -26957,7 +26957,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep ls_in_arp_rsp | grep "169.254.239.2"], [1]
> >
> >   # Add the flows back send arp request and check we see an ARP response
> >   ovn-nbctl --wait=hv add Logical_Switch_Port rp-ls1 \
> > -options arp_proxy='"169.254.239.254 169.254.239.2"'
> > +options arp_proxy='"169.254.239.254,169.254.239.2"'
> I think this change may break the non ddlog test. When I switched the
> northd.c code
> to use 'extract_ip_addresses()', I found that it expected the address
> list to be of the form
> "10.0.0.4 10.0.0.5" rather than comma separated. Hence I updated the
> test in ovn.at
> to use a non comma separated address list.

Thanks.  I think  that's why I'm seeing the issue with this patch.

When I tested manually I used "," instead of space.   I'll test it out again.

Thanks
Numan


> >
> >   ls1_p1_mac=00:00:00:01:02:03
> >   ls1_p1_ip=192.16.1.6
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list