On 27 October 2017 at 08:35, Ben Pfaff <b...@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 <nusid...@redhat.com> wrote: > > > > > From: Numan Siddique <nusid...@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 <nusid...@redhat.com> > > > > > Acked-by: Mark Michelson <mmich...@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 && ip && ip4.dst == > <var>VIP</var> > > > && <var>P</var> && <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 && ip && ip4.dst == > <var>VIP</var> > > > && <var>P</var> && <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 && ip && 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 && ip && 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 && ip4.src == <var>B</var> && > > > + 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 > > > 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 > _______________________________________________ > 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