Re: [ovs-dev] [PATCH ovn 3/3] northd: Use dynamic mac-binding for virtual port IPs.
On Wed, Feb 22, 2023 at 10:35 PM Han Zhou wrote:
>
> Today ARP resolve flows (static mac-bindings) are programmed for virtual
> ports once their virtual parent is claimed. As a result, during virtual
> parent failover, the traffic won't switch to the new parent until the
> ARP resolve flow is updated by ovn-northd, triggered by the port-binding
> update. When the scale is big, the ovn-northd compute may take a long
> time, e.g. 10s or even more, which is too long data plane break during
> virtual parent failover.
>
> This patch removes the dependency of ovn-northd from the failover
> scenario by removing the ARP resolve flows, so that it relies on dynamic
> mac-bindings to resolve virtual parent's MAC for the virtual IP. This
> avoids the logical flow recompute during failover thus make it much
> faster. Functionally there is no difference.
>
> Signed-off-by: Han Zhou
> ---
> northd/northd.c | 103
> northd/ovn-northd.8.xml | 37 +++
> tests/ovn.at| 55 ++---
> tests/system-ovn.at | 1 -
> 4 files changed, 11 insertions(+), 185 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 770a5b50e2c0..65cfb7975d1b 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -12455,109 +12455,6 @@ build_arp_resolve_flows_for_lrouter_port(
> }
> }
> }
> -} else if (op->od->n_router_ports && !lsp_is_router(op->nbsp)
> - && !strcmp(op->nbsp->type, "virtual")) {
> -/* This is a virtual port. Add ARP replies for the virtual ip
with
> - * the mac of the present active virtual parent.
> - * If the logical port doesn't have virtual parent set in
> - * Port_Binding table, then add the flow to set eth.dst to
> - * 00:00:00:00:00:00 and advance to next table so that ARP is
> - * resolved by router pipeline using the arp{} action.
> - * The MAC_Binding entry for the virtual ip might be invalid. */
> -
> -const char *vip = smap_get(>nbsp->options,
> - "virtual-ip");
> -const char *virtual_parents = smap_get(>nbsp->options,
> - "virtual-parents");
> -
> -if (!vip || !virtual_parents || !op->sb) {
> -return;
> -}
> -
> -bool is_ipv4 = strchr(vip, '.') ? true : false;
> -if (is_ipv4) {
> -ovs_be32 ipv4;
> -if (!ip_parse(vip, )) {
> - return;
> -}
> -} else {
> -struct in6_addr ipv6;
> -if (!ipv6_parse(vip, )) {
> - return;
> -}
> -}
> -
> -if (!op->sb->virtual_parent || !op->sb->virtual_parent[0] ||
> -!op->sb->chassis) {
> -/* The virtual port is not claimed yet. */
> -for (size_t i = 0; i < op->od->n_router_ports; i++) {
> -struct ovn_port *peer = ovn_port_get_peer(
> -ports, op->od->router_ports[i]);
> -if (!peer || !peer->nbrp) {
> -continue;
> -}
> -
> -if (find_lrp_member_ip(peer, vip)) {
> -ds_clear(match);
> -ds_put_format(
> -match, "outport == %s && " "%s == %s",
peer->json_key,
> -is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6,
vip);
> -
> -const char *arp_actions =
> - "eth.dst = 00:00:00:00:00:00; next;";
> -ovn_lflow_add_with_hint(lflows, peer->od,
> -S_ROUTER_IN_ARP_RESOLVE, 100,
> -ds_cstr(match),
> -arp_actions,
> ->nbsp->header_);
> -break;
> -}
> -}
> -} else {
> -struct ovn_port *vp =
> -ovn_port_find(ports, op->sb->virtual_parent);
> -if (!vp || !vp->nbsp) {
> -return;
> -}
> -
> -for (size_t i = 0; i < vp->n_lsp_addrs; i++) {
> -bool found_vip_network = false;
> -const char *ea_s = vp->lsp_addrs[i].ea_s;
> -for (size_t j = 0; j < vp->od->n_router_ports; j++) {
> -/* Get the Logical_Router_Port that the
> -* Logical_Switch_Port is connected to, as
> -* 'peer'. */
> -struct ovn_port *peer =
> -ovn_port_get_peer(ports,
vp->od->router_ports[j]);
> -if (!peer || !peer->nbrp) {
> -continue;
> -}
> -
> -if (!find_lrp_member_ip(peer, vip)) {
> -continue;
> -
[ovs-dev] [PATCH ovn 3/3] northd: Use dynamic mac-binding for virtual port IPs.
Today ARP resolve flows (static mac-bindings) are programmed for virtual
ports once their virtual parent is claimed. As a result, during virtual
parent failover, the traffic won't switch to the new parent until the
ARP resolve flow is updated by ovn-northd, triggered by the port-binding
update. When the scale is big, the ovn-northd compute may take a long
time, e.g. 10s or even more, which is too long data plane break during
virtual parent failover.
This patch removes the dependency of ovn-northd from the failover
scenario by removing the ARP resolve flows, so that it relies on dynamic
mac-bindings to resolve virtual parent's MAC for the virtual IP. This
avoids the logical flow recompute during failover thus make it much
faster. Functionally there is no difference.
Signed-off-by: Han Zhou
---
northd/northd.c | 103
northd/ovn-northd.8.xml | 37 +++
tests/ovn.at| 55 ++---
tests/system-ovn.at | 1 -
4 files changed, 11 insertions(+), 185 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 770a5b50e2c0..65cfb7975d1b 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -12455,109 +12455,6 @@ build_arp_resolve_flows_for_lrouter_port(
}
}
}
-} else if (op->od->n_router_ports && !lsp_is_router(op->nbsp)
- && !strcmp(op->nbsp->type, "virtual")) {
-/* This is a virtual port. Add ARP replies for the virtual ip with
- * the mac of the present active virtual parent.
- * If the logical port doesn't have virtual parent set in
- * Port_Binding table, then add the flow to set eth.dst to
- * 00:00:00:00:00:00 and advance to next table so that ARP is
- * resolved by router pipeline using the arp{} action.
- * The MAC_Binding entry for the virtual ip might be invalid. */
-
-const char *vip = smap_get(>nbsp->options,
- "virtual-ip");
-const char *virtual_parents = smap_get(>nbsp->options,
- "virtual-parents");
-
-if (!vip || !virtual_parents || !op->sb) {
-return;
-}
-
-bool is_ipv4 = strchr(vip, '.') ? true : false;
-if (is_ipv4) {
-ovs_be32 ipv4;
-if (!ip_parse(vip, )) {
- return;
-}
-} else {
-struct in6_addr ipv6;
-if (!ipv6_parse(vip, )) {
- return;
-}
-}
-
-if (!op->sb->virtual_parent || !op->sb->virtual_parent[0] ||
-!op->sb->chassis) {
-/* The virtual port is not claimed yet. */
-for (size_t i = 0; i < op->od->n_router_ports; i++) {
-struct ovn_port *peer = ovn_port_get_peer(
-ports, op->od->router_ports[i]);
-if (!peer || !peer->nbrp) {
-continue;
-}
-
-if (find_lrp_member_ip(peer, vip)) {
-ds_clear(match);
-ds_put_format(
-match, "outport == %s && " "%s == %s", peer->json_key,
-is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6, vip);
-
-const char *arp_actions =
- "eth.dst = 00:00:00:00:00:00; next;";
-ovn_lflow_add_with_hint(lflows, peer->od,
-S_ROUTER_IN_ARP_RESOLVE, 100,
-ds_cstr(match),
-arp_actions,
->nbsp->header_);
-break;
-}
-}
-} else {
-struct ovn_port *vp =
-ovn_port_find(ports, op->sb->virtual_parent);
-if (!vp || !vp->nbsp) {
-return;
-}
-
-for (size_t i = 0; i < vp->n_lsp_addrs; i++) {
-bool found_vip_network = false;
-const char *ea_s = vp->lsp_addrs[i].ea_s;
-for (size_t j = 0; j < vp->od->n_router_ports; j++) {
-/* Get the Logical_Router_Port that the
-* Logical_Switch_Port is connected to, as
-* 'peer'. */
-struct ovn_port *peer =
-ovn_port_get_peer(ports, vp->od->router_ports[j]);
-if (!peer || !peer->nbrp) {
-continue;
-}
-
-if (!find_lrp_member_ip(peer, vip)) {
-continue;
-}
-
-ds_clear(match);
-ds_put_format(
-match, "outport == %s && " "%s == %s", peer->json_key,
-is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6, vip);
-
-
