[ovs-dev] [PATCH] ovn: Add LB flows for logical router with gateway port

Guru Shetty guru at ovn.org
Fri Oct 27 17:03:07 UTC 2017


On 27 October 2017 at 08:35, Ben Pfaff <blp at ovn.org> wrote:

> This patch is one where I need help.  I asked Guru because it's an area
> he knows, but if you feel confident about it, Mark, then I'll merge it.
>

No objections from me. If I find issues, I will let know even after the
patch is merged.


>
> Do you have a preference on ordering?
>
> On Fri, Oct 27, 2017 at 03:26:56PM +0000, Mark Michelson wrote:
> > I've had a look over this and it looks fine by me. Ben, do you plan to
> > merge this soon? If so, I'll need to update my IPv6 load balancer patch
> > since it will conflict with this a bit.
> >
> > On Thu, Sep 21, 2017 at 10:53 AM <nusiddiq at redhat.com> wrote:
> >
> > > From: Numan Siddique <nusiddiq at redhat.com>
> > >
> > > This patch adds support for associating a load balancer to a
> > > logical router with gateway router port which was missing earlier.
> > >
> > > Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
> > >
> > Acked-by: Mark Michelson <mmichels at redhat.com>
> >
> > > ---
> > >  ovn/northd/ovn-northd.8.xml |  70 +++++++++++++++-------
> > >  ovn/northd/ovn-northd.c     |  82 +++++++++++++++++++++++---
> > >  tests/system-ovn.at         | 141
> > > ++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 264 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> > > index 0d85ec0d2..2798369ce 100644
> > > --- a/ovn/northd/ovn-northd.8.xml
> > > +++ b/ovn/northd/ovn-northd.8.xml
> > > @@ -1459,61 +1459,69 @@ icmp4 {
> > >        in the reverse direction needs to be unDNATed.
> > >      </p>
> > >
> > > -    <p>Ingress Table 4: DNAT on Gateway Routers</p>
> > > +    <p> Ingress Table 4: Load balancing DNAT rules </p>
> > > +
> > > +    <p>
> > > +      Following load balancing DNAT flows are added for Gateway
> router or
> > > +      Router with gateway port. These flows are programmed only on the
> > > +      <code>redirect-chassis</code>.
> > > +    </p>
> > >
> > >      <ul>
> > >        <li>
> > > -        For all the configured load balancing rules for Gateway
> router in
> > > -        <code>OVN_Northbound</code> database that includes a L4 port
> > > -        <var>PORT</var> of protocol <var>P</var> and IPv4 address
> > > -        <var>VIP</var>, a priority-120 flow that matches on
> > > +        For all the configured load balancing rules for a Gateway
> router
> > > or
> > > +        Router with gateway port in <code>OVN_Northbound</code>
> database
> > > that
> > > +        includes a L4 port <var>PORT</var> of protocol <var>P</var>
> and
> > > IPv4
> > > +        address <var>VIP</var>, a priority-120 flow that matches on
> > >          <code>ct.new &amp;&amp; ip &amp;&amp; ip4.dst ==
> <var>VIP</var>
> > >          &amp;&amp; <var>P</var> &amp;&amp; <var>P</var>.dst ==
> <var>PORT
> > >          </var></code> with an action of
> > > <code>ct_lb(<var>args</var>)</code>,
> > >          where <var>args</var> contains comma separated IPv4 addresses
> (and
> > > -        optional port numbers) to load balance to.  If the Gateway
> router
> > > -        is configured to force SNAT any load-balanced packets, the
> above
> > > -        action will be replaced by <code>flags.force_snat_for_lb = 1;
> > > +        optional port numbers) to load balance to.  If the router is
> > > configured
> > > +        to force SNAT any load-balanced packets, the above action
> will be
> > > +        replaced by <code>flags.force_snat_for_lb = 1;
> > >          ct_lb(<var>args</var>);</code>.
> > >        </li>
> > >
> > >        <li>
> > > -        For all the configured load balancing rules for Gateway
> router in
> > > +        For all the configured load balancing rules for a router in
> > >          <code>OVN_Northbound</code> database that includes a L4 port
> > >          <var>PORT</var> of protocol <var>P</var> and IPv4 address
> > >          <var>VIP</var>, a priority-120 flow that matches on
> > >          <code>ct.est &amp;&amp; ip &amp;&amp; ip4.dst ==
> <var>VIP</var>
> > >          &amp;&amp; <var>P</var> &amp;&amp; <var>P</var>.dst ==
> <var>PORT
> > > -        </var></code> with an action of <code>ct_dnat;</code>.
> > > -        If the Gateway router is configured to force SNAT any
> > > load-balanced
> > > -        packets, the above action will be replaced by
> > > -        <code>flags.force_snat_for_lb = 1; ct_dnat;</code>.
> > > +        </var></code> with an action of <code>ct_dnat;</code>. If the
> > > router is
> > > +        configured to force SNAT any load-balanced packets, the above
> > > action
> > > +        will be replaced by <code>flags.force_snat_for_lb = 1;
> > > ct_dnat;</code>.
> > >        </li>
> > >
> > >        <li>
> > > -        For all the configured load balancing rules for Gateway
> router in
> > > +        For all the configured load balancing rules for a router in
> > >          <code>OVN_Northbound</code> database that includes just an IP
> > > address
> > >          <var>VIP</var> to match on, a priority-110 flow that matches
> on
> > >          <code>ct.new &amp;&amp; ip &amp;&amp; ip4.dst ==
> > >          <var>VIP</var></code> with an action of
> > >          <code>ct_lb(<var>args</var>)</code>, where <var>args</var>
> > > contains
> > > -        comma separated IPv4 addresses.  If the Gateway router
> > > -        is configured to force SNAT any load-balanced packets, the
> above
> > > -        action will be replaced by <code>flags.force_snat_for_lb = 1;
> > > -        ct_lb(<var>args</var>);</code>.
> > > +        comma separated IPv4 addresses.  If the router is configured
> to
> > > force
> > > +        SNAT any load-balanced packets, the above action will be
> replaced
> > > by
> > > +        <code>flags.force_snat_for_lb = 1;
> ct_lb(<var>args</var>);</code>.
> > >        </li>
> > >
> > >        <li>
> > > -        For all the configured load balancing rules for Gateway
> router in
> > > +        For all the configured load balancing rules for a router in
> > >          <code>OVN_Northbound</code> database that includes just an IP
> > > address
> > >          <var>VIP</var> to match on, a priority-110 flow that matches
> on
> > >          <code>ct.est &amp;&amp; ip &amp;&amp; ip4.dst ==
> > >          <var>VIP</var></code> with an action of <code>ct_dnat;</code>.
> > > -        If the Gateway router is configured to force SNAT any
> > > load-balanced
> > > +        If the router is configured to force SNAT any load-balanced
> > >          packets, the above action will be replaced by
> > >          <code>flags.force_snat_for_lb = 1; ct_dnat;</code>.
> > >        </li>
> > > +    </ul>
> > >
> > > +    <p>Ingress Table 4: DNAT on Gateway Routers</p>
> > > +
> > > +    <ul>
> > >        <li>
> > >          For each configuration in the OVN Northbound database, that
> asks
> > >          to change the destination IP address of a packet from
> > > <var>A</var> to
> > > @@ -1892,6 +1900,28 @@ arp {
> > >      <ul>
> > >        <li>
> > >          <p>
> > > +          For all the configured load balancing rules for a router
> with
> > > gateway
> > > +          port in <code>OVN_Northbound</code> database that includes
> an
> > > IPv4
> > > +          address <code>VIP</code>, for every backend IPv4 address
> > > <var>B</var>
> > > +          defined for the <code>VIP</code> a priority-120 flow is
> > > programmed on
> > > +          <code>redirect-chassis</code> that matches
> > > +          <code>ip &amp;&amp; ip4.src == <var>B</var> &amp;&amp;
> > > +          outport == <var>GW</var></code>, where <var>GW</var> is the
> > > logical
> > > +          router gateway port with an action <code>ct_dnat;</code>.
> If the
> > > +          backend IPv4 address <var>B</var> is also configured with L4
> > > port
> > > +          <var>PORT</var> of protocol <var>P</var>, then the
> > > +          match also includes <code>P.src</code> == <var>PORT</var>.
> > > +        </p>
> > > +
> > > +        <p>
> > > +          If the router is configured to force SNAT  any load-balanced
> > > packets,
> > > +          above action will be replaced by
> > > +          <code>flags.force_snat_for_lb = 1; ct_dnat;</code>.
> > > +        </p>
> > > +      </li>
> > > +
> > > +      <li>
> > > +        <p>
> > >            For each configuration in the OVN Northbound database that
> asks
> > >            to change the destination IP address of a packet from an IP
> > >            address of <var>A</var> to <var>B</var>, a priority-100 flow
> > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > > index 49e4ac338..7cc6aab73 100644
> > > --- a/ovn/northd/ovn-northd.c
> > > +++ b/ovn/northd/ovn-northd.c
> > > @@ -4338,7 +4338,8 @@ get_force_snat_ip(struct ovn_datapath *od, const
> > > char *key_type, ovs_be32 *ip)
> > >  static void
> > >  add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
> > >                     struct ds *match, struct ds *actions, int priority,
> > > -                   const char *lb_force_snat_ip)
> > > +                   const char *lb_force_snat_ip, char *backend_ips,
> > > +                   bool is_udp)
> > >  {
> > >      /* A match and actions for new connections. */
> > >      char *new_match = xasprintf("ct.new && %s", ds_cstr(match));
> > > @@ -4365,6 +4366,63 @@ add_router_lb_flow(struct hmap *lflows, struct
> > > ovn_datapath *od,
> > >
> > >      free(new_match);
> > >      free(est_match);
> > > +
> > > +    if (!od->l3dgw_port || !od->l3redirect_port || !backend_ips) {
> > > +        return;
> > > +    }
> > > +
> > > +    /* Add logical flows to UNDNAT the load balanced reverse traffic
> in
> > > +     * the router egress pipleine stage - S_ROUTER_OUT_UNDNAT if the
> > > logical
> > > +     * router has a gateway router port associated.
> > > +     */
> > > +    struct ds undnat_match = DS_EMPTY_INITIALIZER;
> > > +    ds_put_cstr(&undnat_match, "ip4 && (");
> > > +    char *start, *next, *ip_str;
> > > +    start = next = xstrdup(backend_ips);
> > > +    ip_str = strsep(&next, ",");
> > > +    bool backend_ips_found = false;
> > > +    while (ip_str && ip_str[0]) {
> > > +        char *ip_address = NULL;
> > > +        uint16_t port = 0;
> > > +        ip_address_and_port_from_lb_key(ip_str, &ip_address, &port);
> > > +        if (!ip_address) {
> > > +            break;
> > > +        }
> > > +
> > > +        ds_put_format(&undnat_match, "(ip4.src == %s", ip_address);
> > > +        free(ip_address);
> > > +        if (port) {
> > > +            ds_put_format(&undnat_match, " && %s.src == %d) || ",
> > > +                          is_udp ? "udp" : "tcp", port);
> > > +        } else {
> > > +            ds_put_cstr(&undnat_match, ") || ");
> > > +        }
> > > +        ip_str = strsep(&next, ",");
> > > +        backend_ips_found = true;
> > > +    }
> > > +
> > > +    free(start);
> > > +    if (!backend_ips_found) {
> > > +        ds_destroy(&undnat_match);
> > > +        return;
> > > +    }
> > > +    ds_chomp(&undnat_match, ' ');
> > > +    ds_chomp(&undnat_match, '|');
> > > +    ds_chomp(&undnat_match, '|');
> > > +    ds_chomp(&undnat_match, ' ');
> > > +    ds_put_format(&undnat_match, ") && outport == %s && "
> > > +                 "is_chassis_resident(%s)", od->l3dgw_port->json_key,
> > > +                 od->l3redirect_port->json_key);
> > > +    if (lb_force_snat_ip) {
> > > +        ovn_lflow_add(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
> > > +                      ds_cstr(&undnat_match),
> > > +                      "flags.force_snat_for_lb = 1; ct_dnat;");
> > > +    } else {
> > > +        ovn_lflow_add(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
> > > +                      ds_cstr(&undnat_match), "ct_dnat;");
> > > +    }
> > > +
> > > +    ds_destroy(&undnat_match);
> > >  }
> > >
> > >  static void
> > > @@ -5242,8 +5300,8 @@ build_lrouter_flows(struct hmap *datapaths,
> struct
> > > hmap *ports,
> > >          }
> > >
> > >          /* Load balancing and packet defrag are only valid on
> > > -         * Gateway routers. */
> > > -        if (!smap_get(&od->nbr->options, "chassis")) {
> > > +         * Gateway routers or router with gateway port. */
> > > +        if (!smap_get(&od->nbr->options, "chassis") &&
> !od->l3dgw_port) {
> > >              continue;
> > >          }
> > >
> > > @@ -5282,20 +5340,26 @@ build_lrouter_flows(struct hmap *datapaths,
> struct
> > > hmap *ports,
> > >                                ip_address);
> > >                  free(ip_address);
> > >
> > > +                int prio = 110;
> > > +                bool is_udp = lb->protocol && !strcmp(lb->protocol,
> > > "udp") ?
> > > +                    true : false;
> > >                  if (port) {
> > > -                    if (lb->protocol && !strcmp(lb->protocol, "udp"))
> {
> > > +                    if (is_udp) {
> > >                          ds_put_format(&match, " && udp && udp.dst ==
> %d",
> > >                                        port);
> > >                      } else {
> > >                          ds_put_format(&match, " && tcp && tcp.dst ==
> %d",
> > >                                        port);
> > >                      }
> > > -                    add_router_lb_flow(lflows, od, &match, &actions,
> 120,
> > > -                                       lb_force_snat_ip);
> > > -                } else {
> > > -                    add_router_lb_flow(lflows, od, &match, &actions,
> 110,
> > > -                                       lb_force_snat_ip);
> > > +                    prio = 120;
> > > +                }
> > > +
> > > +                if (od->l3redirect_port) {
> > > +                    ds_put_format(&match, " &&
> is_chassis_resident(%s)",
> > > +                                  od->l3redirect_port->json_key);
> > >                  }
> > > +                add_router_lb_flow(lflows, od, &match, &actions, prio,
> > > +                                   lb_force_snat_ip, node->value,
> is_udp);
> > >              }
> > >          }
> > >
> > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> > > index dd62bd116..638c0b661 100644
> > > --- a/tests/system-ovn.at
> > > +++ b/tests/system-ovn.at
> > > @@ -1075,6 +1075,147 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query
> port
> > > patch-.*/d
> > >  /connection dropped.*/d"])
> > >  AT_CLEANUP
> > >
> > > +AT_SETUP([ovn -- load balancing in router with gateway router port])
> > > +AT_KEYWORDS([ovnlb])
> > > +
> > > +CHECK_CONNTRACK()
> > > +CHECK_CONNTRACK_NAT()
> > > +ovn_start
> > > +OVS_TRAFFIC_VSWITCHD_START()
> > > +ADD_BR([br-int])
> > > +
> > > +# Set external-ids in br-int needed for ovn-controller
> > > +ovs-vsctl \
> > > +        -- set Open_vSwitch . external-ids:system-id=hv1 \
> > > +        -- set Open_vSwitch .
> > > external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
> > > +        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> > > +        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
> > > +        -- set bridge br-int fail-mode=secure
> > > other-config:disable-in-band=true
> > > +
> > > +# Start ovn-controller
> > > +start_daemon ovn-controller
> > > +
> > > +# Logical network:
> > > +# One LR R1 with switches foo (192.168.1.0/24), bar (192.168.2.0/24),
> > > +# and alice (172.16.1.0/24) connected to it.  The port between R1 and
> > > +# alice is the router gateway port where the R1 LB rules are applied.
> > > +#
> > > +#    foo -- R1 -- bar
> > > +#           |
> > > +#    alice ----
> > > +
> > > +ovn-nbctl lr-add R1
> > > +
> > > +ovn-nbctl ls-add foo
> > > +ovn-nbctl ls-add bar
> > > +ovn-nbctl ls-add alice
> > > +
> > > +ovn-nbctl lrp-add R1 foo 00:00:01:01:02:03 192.168.1.1/24
> > > +ovn-nbctl lrp-add R1 bar 00:00:01:01:02:04 192.168.2.1/24
> > > +ovn-nbctl lrp-add R1 alice 00:00:02:01:02:03 172.16.1.1/24 \
> > > +    -- set Logical_Router_Port alice options:redirect-chassis=hv1
> > > +
> > > +# Connect foo to R1
> > > +ovn-nbctl lsp-add foo rp-foo -- set Logical_Switch_Port rp-foo \
> > > +    type=router options:router-port=foo \
> > > +    -- lsp-set-addresses rp-foo router
> > > +
> > > +# Connect bar to R1
> > > +ovn-nbctl lsp-add bar rp-bar -- set Logical_Switch_Port rp-bar \
> > > +    type=router options:router-port=bar \
> > > +    -- lsp-set-addresses rp-bar router
> > > +
> > > +# Connect alice to R1
> > > +ovn-nbctl lsp-add alice rp-alice -- set Logical_Switch_Port rp-alice \
> > > +    type=router options:router-port=alice \
> > > +    -- lsp-set-addresses rp-alice router
> > > +
> > > +# Logical port 'foo1' in switch 'foo'.
> > > +ADD_NAMESPACES(foo1)
> > > +ADD_VETH(foo1, foo1, br-int, "192.168.1.2/24", "f0:00:00:01:02:03", \
> > > +         "192.168.1.1")
> > > +ovn-nbctl lsp-add foo foo1 \
> > > +-- lsp-set-addresses foo1 "f0:00:00:01:02:03 192.168.1.2"
> > > +
> > > +# Logical port 'foo2' in switch 'foo'.
> > > +ADD_NAMESPACES(foo2)
> > > +ADD_VETH(foo2, foo2, br-int, "192.168.1.3/24", "f0:00:00:01:02:06", \
> > > +         "192.168.1.1")
> > > +ovn-nbctl lsp-add foo foo2 \
> > > +-- lsp-set-addresses foo2 "f0:00:00:01:02:06 192.168.1.3"
> > > +
> > > +# Logical port 'bar1' in switch 'bar'.
> > > +ADD_NAMESPACES(bar1)
> > > +ADD_VETH(bar1, bar1, br-int, "192.168.2.2/24", "f0:00:00:01:02:04", \
> > > +         "192.168.2.1")
> > > +ovn-nbctl lsp-add bar bar1 \
> > > +-- lsp-set-addresses bar1 "f0:00:00:01:02:04 192.168.2.2"
> > > +
> > > +# Logical port 'alice1' in switch 'alice'.
> > > +ADD_NAMESPACES(alice1)
> > > +ADD_VETH(alice1, alice1, br-int, "172.16.1.2/24",
> "f0:00:00:01:02:05", \
> > > +         "172.16.1.1")
> > > +ovn-nbctl lsp-add alice alice1 \
> > > +-- lsp-set-addresses alice1 "f0:00:00:01:02:05 172.16.1.2"
> > > +
> > > +# Config OVN load-balancer with a VIP.
> > > +uuid=`ovn-nbctl  create load_balancer
> > > vips:172.16.1.10="192.168.1.2,192.168.2.2"`
> > > +ovn-nbctl set logical_router R1 load_balancer=$uuid
> > > +
> > > +# Config OVN load-balancer with another VIP (this time with ports).
> > > +ovn-nbctl set load_balancer $uuid vips:'"172.16.1.11:8000"'='"
> > > 192.168.1.2:80,192.168.2.2:80"'
> > > +
> > > +# Wait for ovn-controller to catch up.
> > > +ovn-nbctl --wait=hv sync
> > > +OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-groups br-int | \
> > > +grep 'nat(dst=192.168.2.2:80)'])
> > > +
> > > +# Start webservers in 'foo1', 'bar1'.
> > > +OVS_START_L7([foo1], [http])
> > > +OVS_START_L7([bar1], [http])
> > > +
> > > +dnl Should work with the virtual IP address through NAT
> > > +for i in `seq 1 20`; do
> > > +    echo Request $i
> > > +    NS_CHECK_EXEC([alice1], [wget 172.16.1.10 -t 5 -T 1
> > > --retry-connrefused -v -o wget$i.log])
> > > +done
> > > +
> > > +dnl Each server should have at least one connection.
> > > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.10) |
> > > +sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
> > >
> > > +tcp,orig=(src=172.16.1.2,dst=172.16.1.10,sport=<cleared>,
> dport=<cleared>),reply=(src=192.168.1.2,dst=172.16.1.2,
> sport=<cleared>,dport=<cleared>),zone=<cleared>,
> protoinfo=(state=<cleared>)
> > >
> > > +tcp,orig=(src=172.16.1.2,dst=172.16.1.10,sport=<cleared>,
> dport=<cleared>),reply=(src=192.168.2.2,dst=172.16.1.2,
> sport=<cleared>,dport=<cleared>),zone=<cleared>,
> protoinfo=(state=<cleared>)
> > > +])
> > > +
> > > +dnl Test load-balancing that includes L4 ports in NAT.
> > > +for i in `seq 1 20`; do
> > > +    echo Request $i
> > > +    NS_CHECK_EXEC([alice1], [wget 172.16.1.11:8000 -t 5 -T 1
> > > --retry-connrefused -v -o wget$i.log])
> > > +done
> > > +
> > > +dnl Each server should have at least one connection.
> > > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.11) |
> > > +sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
> > >
> > > +tcp,orig=(src=172.16.1.2,dst=172.16.1.11,sport=<cleared>,
> dport=<cleared>),reply=(src=192.168.1.2,dst=172.16.1.2,
> sport=<cleared>,dport=<cleared>),zone=<cleared>,
> protoinfo=(state=<cleared>)
> > >
> > > +tcp,orig=(src=172.16.1.2,dst=172.16.1.11,sport=<cleared>,
> dport=<cleared>),reply=(src=192.168.2.2,dst=172.16.1.2,
> sport=<cleared>,dport=<cleared>),zone=<cleared>,
> protoinfo=(state=<cleared>)
> > > +])
> > > +
> > > +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> > > +
> > > +as ovn-sb
> > > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > > +
> > > +as ovn-nb
> > > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > > +
> > > +as northd
> > > +OVS_APP_EXIT_AND_WAIT([ovn-northd])
> > > +
> > > +as
> > > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> > > +/connection dropped.*/d"])
> > > +AT_CLEANUP
> > > +
> > >  AT_SETUP([ovn -- DNAT and SNAT on distributed router - N/S])
> > >  AT_KEYWORDS([ovnnat])
> > >
> > > --
> > > 2.13.3
> > >
> > > _______________________________________________
> > > dev mailing list
> > > dev at openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list