[ovs-dev,v6,3/4,ovn] OVN: Vlan backed DVR N-S, avoid get_arp on non redirect chassis.
diff mbox series

Message ID 1566002197-29398-4-git-send-email-ankur.sharma@nutanix.com
State New
Headers show
Series
  • OVN: Vlan backed DVR, enable N-S packet flow
Related show

Commit Message

Ankur Sharma Aug. 17, 2019, 12:36 a.m. UTC
Background:
With c0974331b7a19a87ab8f1f2cec8fbe366af92fa2, we have added
support for E-W workflow for vlan backed DVRs.

This series enables N-S workflow for vlan backed DVRs.

Key difference between E-W and N-S traffic flow is that
N-S flow requires a gateway chassis. A gateway chassis
will be respondible for following:
a. Doing Network Address Translation (NAT).
b. Becoming entry and exit point for North->South
   and South->North traffic respectively.

OVN by default always uses overlay encapsulation to redirect
the packet to gateway chassis. This series will enable
the redirection to gateway chassis in the absence of encapsulation.

This patch:
a. Make sure that ARP request for endpoint behind the gateway
   router port is sent from gateway chassis only and not from
   host(compute) chassis.

b. This is achieved by adding a new logical flow in
   lr_in_arp_resolve at priority=50.

c. This flow run on non gateway chassis and sets the destination
   mac to router port mac, if outport is a gateway chassis attached
   router port and redirect-type is set as "vlan".
   Example logical flow:
   table=9 (lr_in_arp_resolve  ), priority=50   , match=(outport == "router-to-underlay" && !is_chassis_resident("cr-router-to-underlay")), action=(eth.dst = 00:00:01:01:02:04; next;)

d. This change is needed because other wise for non resolved ARPs,
   we will end up doing get_arp in host chassis. Doing so will
   have following issues:
   i. We want all the interation with North bound endpoints via
      gateway chassis only, doing so on host chassis will violate
      that.

  ii. With get_arp, ovn-controller will generate the ARP using router
      port's mac as source mac, which will lead us to the same issue,
      where router port mac will be going through continous mac moves
      in physical network. Worst, it would affect the redirection,
      since it uses router port mac as destination mac.

Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com>
---
 northd/ovn-northd.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Numan Siddique Aug. 24, 2019, 6:58 p.m. UTC | #1
On Sat, Aug 17, 2019 at 6:08 AM Ankur Sharma <ankur.sharma@nutanix.com>
wrote:

> Background:
> With c0974331b7a19a87ab8f1f2cec8fbe366af92fa2, we have added
> support for E-W workflow for vlan backed DVRs.
>
> This series enables N-S workflow for vlan backed DVRs.
>
> Key difference between E-W and N-S traffic flow is that
> N-S flow requires a gateway chassis. A gateway chassis
> will be respondible for following:
> a. Doing Network Address Translation (NAT).
> b. Becoming entry and exit point for North->South
>    and South->North traffic respectively.
>
> OVN by default always uses overlay encapsulation to redirect
> the packet to gateway chassis. This series will enable
> the redirection to gateway chassis in the absence of encapsulation.
>
> This patch:
> a. Make sure that ARP request for endpoint behind the gateway
>    router port is sent from gateway chassis only and not from
>    host(compute) chassis.
>
> b. This is achieved by adding a new logical flow in
>    lr_in_arp_resolve at priority=50.
>
> c. This flow run on non gateway chassis and sets the destination
>    mac to router port mac, if outport is a gateway chassis attached
>    router port and redirect-type is set as "vlan".
>    Example logical flow:
>    table=9 (lr_in_arp_resolve  ), priority=50   , match=(outport ==
> "router-to-underlay" && !is_chassis_resident("cr-router-to-underlay")),
> action=(eth.dst = 00:00:01:01:02:04; next;)
>
> d. This change is needed because other wise for non resolved ARPs,
>    we will end up doing get_arp in host chassis. Doing so will
>    have following issues:
>    i. We want all the interation with North bound endpoints via
>       gateway chassis only, doing so on host chassis will violate
>       that.
>
>   ii. With get_arp, ovn-controller will generate the ARP using router
>       port's mac as source mac, which will lead us to the same issue,
>       where router port mac will be going through continous mac moves
>       in physical network. Worst, it would affect the redirection,
>       since it uses router port mac as destination mac.
>
> Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com>
>

Right now for the S/N traffic with overlay networks, if ovn-controller
needs to learn the mac of the next hop,
the source chassis ovn-controller generates the arp. This arp packet would
be tunneled to the
redirect chassis and it will send the arp packet out. And the reply arp is
learnt
and the mac_binding table is updated by the redirect chassis. If the packet
is buffered by the source chassis,
it will re inject the actual IP packet.

I think we can extend this patch to all kind of networks. Any thoughts on
this ?

CC'ing Lorenzo if he has any comments.

Also you need to update documentation in ovn-northd.8.xml

Thanks
Numan


> ---
>  northd/ovn-northd.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 89ca8df..e13a5af 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -3516,6 +3516,16 @@ lsp_is_external(const struct
> nbrec_logical_switch_port *nbsp)
>      return !strcmp(nbsp->type, "external");
>  }
>
> +/* Returns true if lrp has either gateway chassis or ha chassis group
> + * attached to it. */
> +static bool
> +lrp_has_gateway(const struct nbrec_logical_router_port *nbrp)
> +{
> +    return (nbrp->n_gateway_chassis ||
> +            (nbrp->ha_chassis_group &&
> nbrp->ha_chassis_group->n_ha_chassis))
> +            ? true : false;
> +}
> +
>  static bool
>  build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,
>                      struct ds *options_action, struct ds *response_action,
> @@ -7568,6 +7578,28 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>                                    100, ds_cstr(&match),
> ds_cstr(&actions));
>                  }
>              }
> +
> +            if (!op->derived && lrp_has_gateway(op->nbrp)) {
> +                const char *redirect_type = smap_get(&op->nbrp->options,
> +                                                     "redirect-type");
> +                if (redirect_type && !strcasecmp(redirect_type, "vlan")) {
> +                    /* Packet is on a non gateway chassis and
> +                     * has an unresolved ARP on a network behind gateway
> +                     * chassis attached router port. Since, redirect type
> +                     * is set to vlan, hence instead of calling "get_arp"
> +                     * on this node, we will redirect the packet to
> gateway
> +                     * chassis, by setting destination mac router port
> mac.*/
> +                    ds_clear(&match);
> +                    ds_put_format(&match, "outport == %s && "
> +                                  "!is_chassis_resident(%s)",
> op->json_key,
> +                                  op->od->l3redirect_port->json_key);
> +                    ds_clear(&actions);
> +                    ds_put_format(&actions, "eth.dst = %s; next;",
> +                                  op->lrp_networks.ea_s);
> +                    ovn_lflow_add(lflows, op->od, S_ROUTER_IN_ARP_RESOLVE,
> +                                  50, ds_cstr(&match), ds_cstr(&actions));
> +                }
> +            }
>          } else if (op->od->n_router_ports && strcmp(op->nbsp->type,
> "router")
>                     && strcmp(op->nbsp->type, "virtual")) {
>              /* This is a logical switch port that backs a VM or a
> container.
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Ankur Sharma Aug. 28, 2019, 2:01 a.m. UTC | #2
Hi Numan,

Sent out V7/8 with changes in ovn-northd.8.xml.

You have raised a very good point regarding ARP resolution and in this aspect patch differs from overlay redirection because of following reasons:

SENDER SIDE:
============
a. Since, there is no encapsulation, hence generated ARP request will not be encapsulated.
b. To handle above, we could either convert ARP request to a unicast ARP request, i.e redirect the ARP request to gateway chassis.
c. OR modify get_arp to NOT to generate ARP request and send the regular packet, while buffering a copy of it.

REDIRECT CHASSIS:
=============
a. Since there is no encap, hence on redirect chassis packet will enter the router ingress pipeline again and will hit get_arp again.
b. But since, source chassis has already buffered the packet, hence on redirect chassis we will generate the ARP request, but will not buffer the packet.


To achieve the consistent behavior both packet forwarding behavior and code changes in get_arp would be complex and non-intuitive.
Hence, we took a simpler path of simply forwarding all the “to be” redirected packets to gateway/redirect chassis and let buffering
and arp resolution happen there itself.


Let me know your thoughts on this. I will be happy to discuss further and make additional changes, if needed.

Appreciate your feedback.

Regards,
Ankur



From: Numan Siddique <nusiddiq@redhat.com>
Sent: Saturday, August 24, 2019 11:59 AM
To: Ankur Sharma <ankur.sharma@nutanix.com>
Cc: ovs-dev@openvswitch.org; Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Subject: Re: [ovs-dev] [PATCH v6 3/4 ovn] OVN: Vlan backed DVR N-S, avoid get_arp on non redirect chassis.



On Sat, Aug 17, 2019 at 6:08 AM Ankur Sharma <ankur.sharma@nutanix.com<mailto:ankur.sharma@nutanix.com>> wrote:
Background:
With c0974331b7a19a87ab8f1f2cec8fbe366af92fa2, we have added
support for E-W workflow for vlan backed DVRs.

This series enables N-S workflow for vlan backed DVRs.

Key difference between E-W and N-S traffic flow is that
N-S flow requires a gateway chassis. A gateway chassis
will be respondible for following:
a. Doing Network Address Translation (NAT).
b. Becoming entry and exit point for North->South
   and South->North traffic respectively.

OVN by default always uses overlay encapsulation to redirect
the packet to gateway chassis. This series will enable
the redirection to gateway chassis in the absence of encapsulation.

This patch:
a. Make sure that ARP request for endpoint behind the gateway
   router port is sent from gateway chassis only and not from
   host(compute) chassis.

b. This is achieved by adding a new logical flow in
   lr_in_arp_resolve at priority=50.

c. This flow run on non gateway chassis and sets the destination
   mac to router port mac, if outport is a gateway chassis attached
   router port and redirect-type is set as "vlan".
   Example logical flow:
   table=9 (lr_in_arp_resolve  ), priority=50   , match=(outport == "router-to-underlay" && !is_chassis_resident("cr-router-to-underlay")), action=(eth.dst = 00:00:01:01:02:04; next;)

d. This change is needed because other wise for non resolved ARPs,
   we will end up doing get_arp in host chassis. Doing so will
   have following issues:
   i. We want all the interation with North bound endpoints via
      gateway chassis only, doing so on host chassis will violate
      that.

  ii. With get_arp, ovn-controller will generate the ARP using router
      port's mac as source mac, which will lead us to the same issue,
      where router port mac will be going through continous mac moves
      in physical network. Worst, it would affect the redirection,
      since it uses router port mac as destination mac.

Signed-off-by: Ankur Sharma <ankur.sharma@nutanix.com<mailto:ankur.sharma@nutanix.com>>

Right now for the S/N traffic with overlay networks, if ovn-controller needs to learn the mac of the next hop,
the source chassis ovn-controller generates the arp. This arp packet would be tunneled to the
redirect chassis and it will send the arp packet out. And the reply arp is learnt
and the mac_binding table is updated by the redirect chassis. If the packet is buffered by the source chassis,
it will re inject the actual IP packet.

I think we can extend this patch to all kind of networks. Any thoughts on this ?

CC'ing Lorenzo if he has any comments.

Also you need to update documentation in ovn-northd.8.xml

Thanks
Numan

---
 northd/ovn-northd.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 89ca8df..e13a5af 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3516,6 +3516,16 @@ lsp_is_external(const struct nbrec_logical_switch_port *nbsp)
     return !strcmp(nbsp->type, "external");
 }

+/* Returns true if lrp has either gateway chassis or ha chassis group
+ * attached to it. */
+static bool
+lrp_has_gateway(const struct nbrec_logical_router_port *nbrp)
+{
+    return (nbrp->n_gateway_chassis ||
+            (nbrp->ha_chassis_group && nbrp->ha_chassis_group->n_ha_chassis))
+            ? true : false;
+}
+
 static bool
 build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,
                     struct ds *options_action, struct ds *response_action,
@@ -7568,6 +7578,28 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                                   100, ds_cstr(&match), ds_cstr(&actions));
                 }
             }
+
+            if (!op->derived && lrp_has_gateway(op->nbrp)) {
+                const char *redirect_type = smap_get(&op->nbrp->options,
+                                                     "redirect-type");
+                if (redirect_type && !strcasecmp(redirect_type, "vlan")) {
+                    /* Packet is on a non gateway chassis and
+                     * has an unresolved ARP on a network behind gateway
+                     * chassis attached router port. Since, redirect type
+                     * is set to vlan, hence instead of calling "get_arp"
+                     * on this node, we will redirect the packet to gateway
+                     * chassis, by setting destination mac router port mac.*/
+                    ds_clear(&match);
+                    ds_put_format(&match, "outport == %s && "
+                                  "!is_chassis_resident(%s)", op->json_key,
+                                  op->od->l3redirect_port->json_key);
+                    ds_clear(&actions);
+                    ds_put_format(&actions, "eth.dst = %s; next;",
+                                  op->lrp_networks.ea_s);
+                    ovn_lflow_add(lflows, op->od, S_ROUTER_IN_ARP_RESOLVE,
+                                  50, ds_cstr(&match), ds_cstr(&actions));
+                }
+            }
         } else if (op->od->n_router_ports && strcmp(op->nbsp->type, "router")
                    && strcmp(op->nbsp->type, "virtual")) {
             /* This is a logical switch port that backs a VM or a container.
--
1.8.3.1

Patch
diff mbox series

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 89ca8df..e13a5af 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3516,6 +3516,16 @@  lsp_is_external(const struct nbrec_logical_switch_port *nbsp)
     return !strcmp(nbsp->type, "external");
 }
 
+/* Returns true if lrp has either gateway chassis or ha chassis group
+ * attached to it. */
+static bool
+lrp_has_gateway(const struct nbrec_logical_router_port *nbrp)
+{
+    return (nbrp->n_gateway_chassis ||
+            (nbrp->ha_chassis_group && nbrp->ha_chassis_group->n_ha_chassis))
+            ? true : false;
+}
+
 static bool
 build_dhcpv4_action(struct ovn_port *op, ovs_be32 offer_ip,
                     struct ds *options_action, struct ds *response_action,
@@ -7568,6 +7578,28 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                                   100, ds_cstr(&match), ds_cstr(&actions));
                 }
             }
+
+            if (!op->derived && lrp_has_gateway(op->nbrp)) {
+                const char *redirect_type = smap_get(&op->nbrp->options,
+                                                     "redirect-type");
+                if (redirect_type && !strcasecmp(redirect_type, "vlan")) {
+                    /* Packet is on a non gateway chassis and
+                     * has an unresolved ARP on a network behind gateway
+                     * chassis attached router port. Since, redirect type
+                     * is set to vlan, hence instead of calling "get_arp"
+                     * on this node, we will redirect the packet to gateway
+                     * chassis, by setting destination mac router port mac.*/
+                    ds_clear(&match);
+                    ds_put_format(&match, "outport == %s && "
+                                  "!is_chassis_resident(%s)", op->json_key,
+                                  op->od->l3redirect_port->json_key);
+                    ds_clear(&actions);
+                    ds_put_format(&actions, "eth.dst = %s; next;",
+                                  op->lrp_networks.ea_s);
+                    ovn_lflow_add(lflows, op->od, S_ROUTER_IN_ARP_RESOLVE,
+                                  50, ds_cstr(&match), ds_cstr(&actions));
+                }
+            }
         } else if (op->od->n_router_ports && strcmp(op->nbsp->type, "router")
                    && strcmp(op->nbsp->type, "virtual")) {
             /* This is a logical switch port that backs a VM or a container.