Hi Ben,
This patch is not working as expected. Need your help here. Not very urgent.
Please see below.
Thanks
Numan
On Tue, Jun 29, 2021 at 12:09 PM <num...@ovn.org> wrote:
>
> From: Numan Siddique <num...@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 <num...@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, ",")
> + }
> + }
> + }
If the - sp.lsp.option.get("arp_proxy") has 2 IP addresses -
"10.0.0.4, 10.0.0.5", then the above
code sets only "10.0.0.4" into the variable "proxy_ips". I was
expecting it to have "10.0.0.4,10.0.0.5".
I'm not sure if the issue is in "extract_ip_addresses" or somewhere else.
> + }
> + },
> + 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"'
>
> ls1_p1_mac=00:00:00:01:02:03
> ls1_p1_ip=192.16.1.6
> --
> 2.31.1
>
> _______________________________________________
> 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
