On Mon, Apr 22, 2024 at 2:54 AM Vasyl Saienko <vsaie...@mirantis.com> wrote:
>
> Reply only if target ethernet address is broadcast, if
> address is specified explicitly do noting to let target
> reply by itself. This technique allows to monitor target
> aliveness with arping.

Thanks for the patch.

This patch does change the behavior of OVN.  Having ARP responder
flows avoids tunnelling the request packet if the destination is on a
different compute node.
But I don't think its a big deal.

I see 3 options here:

1.  Accept your patch
2.  Use the NB Global option - "ignore_lsp_down" and set it to false,
so that ovn-northd will install the ARP responder flows only if the
logical port is up.
     Have you considered this approach for your use case ?
3.  Add another global option - "disable_arp_responder" which when set
to true, ovn-northd will not install ARP responder flows.

Given that OpenStack neutron ml2ovs uses ARP responder flows only for
broad cast my preference would be (1).

I'd like to know other's opinion on this.

Thanks
Numan

>
> Closes  #239
>
> Signed-off-by: Vasyl Saienko <vsaie...@mirantis.com>
> ---
>  northd/northd.c     | 11 +++++++++--
>  tests/ovn-northd.at |  4 ++--
>  2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 37f443e70..e80e1885d 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -8832,8 +8832,15 @@ build_lswitch_arp_nd_responder_known_ips(struct 
> ovn_port *op,
>          for (size_t i = 0; i < op->n_lsp_addrs; i++) {
>              for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) {
>                  ds_clear(match);
> -                ds_put_format(match, "arp.tpa == %s && arp.op == 1",
> -                            op->lsp_addrs[i].ipv4_addrs[j].addr_s);
> +                /* NOTE(vsaienko): Do not reply on unicast ARPs, forward
> +                 * them to the target to have ability to monitor target
> +                 * aliveness via ARPs.
> +                */
> +                ds_put_format(match,
> +                    "arp.tpa == %s && "
> +                    "arp.op == 1 && "
> +                    "eth.dst == ff:ff:ff:ff:ff:ff",
> +                    op->lsp_addrs[i].ipv4_addrs[j].addr_s);
>                  ds_clear(actions);
>                  ds_put_format(actions,
>                      "eth.dst = eth.src; "
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index be006fb32..4162196f4 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -9283,9 +9283,9 @@ AT_CAPTURE_FILE([S1flows])
>
>  AT_CHECK([grep -e "ls_in_arp_rsp" S1flows | ovn_strip_lflows], [0], [dnl
>    table=??(ls_in_arp_rsp      ), priority=0    , match=(1), action=(next;)
> -  table=??(ls_in_arp_rsp      ), priority=100  , match=(arp.tpa == 
> 192.168.0.10 && arp.op == 1 && inport == "S1-vm1"), action=(next;)
> +  table=??(ls_in_arp_rsp      ), priority=100  , match=(arp.tpa == 
> 192.168.0.10 && arp.op == 1 && eth.dst == ff:ff:ff:ff:ff:ff && inport == 
> "S1-vm1"), action=(next;)
>    table=??(ls_in_arp_rsp      ), priority=100  , match=(nd_ns && ip6.dst == 
> {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10 && inport == "S1-vm1"), 
> action=(next;)
> -  table=??(ls_in_arp_rsp      ), priority=50   , match=(arp.tpa == 
> 192.168.0.10 && arp.op == 1), action=(eth.dst = eth.src; eth.src = 
> 50:54:00:00:00:10; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 
> 50:54:00:00:00:10; arp.tpa = arp.spa; arp.spa = 192.168.0.10; outport = 
> inport; flags.loopback = 1; output;)
> +  table=??(ls_in_arp_rsp      ), priority=50   , match=(arp.tpa == 
> 192.168.0.10 && arp.op == 1 && eth.dst == ff:ff:ff:ff:ff:ff), action=(eth.dst 
> = eth.src; eth.src = 50:54:00:00:00:10; arp.op = 2; /* ARP reply */ arp.tha = 
> arp.sha; arp.sha = 50:54:00:00:00:10; arp.tpa = arp.spa; arp.spa = 
> 192.168.0.10; outport = inport; flags.loopback = 1; output;)
>    table=??(ls_in_arp_rsp      ), priority=50   , match=(nd_ns && ip6.dst == 
> {fd00::10, ff02::1:ff00:10} && nd.target == fd00::10), action=(nd_na { 
> eth.src = 50:54:00:00:00:10; ip6.src = fd00::10; nd.target = fd00::10; nd.tll 
> = 50:54:00:00:00:10; outport = inport; flags.loopback = 1; output; };)
>  ])
>
> --
> 2.40.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

Reply via email to