On 3/21/25 2:51 PM, Lorenzo Bianconi wrote:
> Do not drop traffic destinated to the template load-balancer vip if the
> vip var is set to one of the router interface IPs.
>
> Reported-at: https://issues.redhat.com/browse/FDP-988
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
> ---
Hi Lorenzo,
Thanks for the patch!
> Changes in v2:
> - Fix logical flows in table S_ROUTER_IN_IP_INPUT
> ---
> lib/lb.c | 1 +
> lib/lb.h | 1 +
> northd/northd.c | 32 +++++++++
> northd/ovn-northd.8.xml | 9 +++
> tests/system-ovn.at | 145 ++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 188 insertions(+)
>
> diff --git a/lib/lb.c b/lib/lb.c
> index 2b2904d1e..6e7a4e296 100644
> --- a/lib/lb.c
> +++ b/lib/lb.c
> @@ -248,6 +248,7 @@ ovn_lb_vip_init_template(struct ovn_lb_vip *lb_vip, const
> char *lb_key_,
> success = true;
> if (!lb_vip->vip_str) {
> lb_vip->vip_str = xstrdup(atom);
> + lb_vip->template_vips = !!strchr(atom, LEX_TEMPLATE_PREFIX);
> } else {
> lb_vip->port_str = xstrdup(atom);
> }
> diff --git a/lib/lb.h b/lib/lb.h
> index e758765bd..b1a89d63c 100644
> --- a/lib/lb.h
> +++ b/lib/lb.h
> @@ -36,6 +36,7 @@ struct ovn_lb_vip {
> char *port_str; /* Actual port string representation. To be used
> * in ovn-northd.
> */
> + bool template_vips; /* True if the vips are templates. */
> struct ovn_lb_backend *backends;
> size_t n_backends;
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 1d3e132d4..4b75f81da 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -12335,6 +12335,35 @@ build_lrouter_defrag_flows_for_lb(struct
> ovn_lb_datapaths *lb_dps,
> }
> }
>
> +static void
> +build_lrouter_allow_vip_traffic_template(struct lflow_table *lflows,
> + struct ovn_lb_datapaths *lb_dps,
> + struct ovn_lb_vip *lb_vip,
> + const struct ovn_northd_lb *lb,
> + const struct ovn_datapaths *lr_dps)
> +{
> + if (!lb_vip->template_vips) {
> + return;
> + }
> +
> + struct ds match = DS_EMPTY_INITIALIZER;
> +
> + size_t index;
> + BITMAP_FOR_EACH_1 (index, ods_size(lr_dps), lb_dps->nb_lr_map) {
> + struct ovn_datapath *od = lr_dps->array[index];
> + /* Do not drop ip traffic with destination the template VIP. */
> + ds_clear(&match);
> + ds_put_format(&match, "ip%d.dst == %s",
> + lb_vip->address_family == AF_INET ? 4 : 6,
> + lb_vip->vip_str);
> + ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, 100,
> + ds_cstr(&match), "next;",
> + &lb->nlb->header_, lb_dps->lflow_ref);
> + }
> +
> + ds_destroy(&match);
> +}
> +
> static void
> build_lrouter_flows_for_lb(struct ovn_lb_datapaths *lb_dps,
> struct lflow_table *lflows,
> @@ -12359,6 +12388,9 @@ build_lrouter_flows_for_lb(struct ovn_lb_datapaths
> *lb_dps,
> match, action, meter_groups,
> svc_monitor_map);
>
> + build_lrouter_allow_vip_traffic_template(lflows, lb_dps, lb_vip, lb,
> + lr_datapaths);
> +
> if (!build_empty_lb_event_flow(lb_vip, lb, match, action)) {
> continue;
> }
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 155ba8a49..3164b4fff 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -3100,6 +3100,15 @@ icmp6_error {
> router.
> </li>
>
> + <li>
> + For each load balancer applied to this logical router configured
> + with <code>VIP</code> template, a priority-100 flow matching
> + <code>ip4.dst</code> or <code>ip6.dst</code> with the configured
> + load balancer <code>VIP</code> and action <code>next;</code>.
> + These flows avoid dropping the packet if the <code>VIP</code> is
> + set to one of the router IPs.
> + </li>
> +
> <li>
> <p>
> ICMP echo reply. These flows reply to ICMP echo requests received
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 66deeb30e..85d8ac7d0 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -17444,3 +17444,148 @@ OVS_TRAFFIC_VSWITCHD_STOP()
>
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([load balancing lb_force_snat_ip with template])
> +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 \
Nit: check
> + -- 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:
> +# Two LRs - R1 and R2 that are connected to each other via LS "join"
> +# in 20.0.0.0/24 network. R1 has switchess foo (192.168.1.0/24) and
> +# bar (192.168.2.0/24) connected to it. R2 has alice (172.16.1.0/24)
> connected
> +# to it. R2 is a gateway router on which we add load-balancing rules.
> +#
> +# foo -- R1 -- join - R2 -- alice
> +# |
> +# bar ----
> +
> +check_uuid ovn-nbctl create Logical_Router name=R1
> +check_uuid ovn-nbctl create Logical_Router name=R2 options:chassis=hv1
> +
> +check ovn-nbctl ls-add foo
> +check ovn-nbctl ls-add bar
> +check ovn-nbctl ls-add alice
> +check ovn-nbctl ls-add join
> +
> +# Connect foo to R1
> +check ovn-nbctl lrp-add R1 foo 00:00:01:01:02:03 192.168.1.1/24
> +check ovn-nbctl lsp-add foo rp-foo -- set Logical_Switch_Port rp-foo \
> + type=router options:router-port=foo addresses=\"00:00:01:01:02:03\"
> +
> +# Connect bar to R1
> +check ovn-nbctl lrp-add R1 bar 00:00:01:01:02:04 192.168.2.1/24
> +check ovn-nbctl lsp-add bar rp-bar -- set Logical_Switch_Port rp-bar \
> + type=router options:router-port=bar addresses=\"00:00:01:01:02:04\"
> +
> +# Connect alice to R2
> +check ovn-nbctl lrp-add R2 alice 00:00:02:01:02:03 172.16.1.1/24
> +check ovn-nbctl lsp-add alice rp-alice -- set Logical_Switch_Port rp-alice \
> + type=router options:router-port=alice addresses=\"00:00:02:01:02:03\"
> +
> +# Connect R1 to join
> +check ovn-nbctl lrp-add R1 R1_join 00:00:04:01:02:03 20.0.0.1/24
> +check ovn-nbctl lsp-add join r1-join -- set Logical_Switch_Port r1-join \
> + type=router options:router-port=R1_join addresses='"00:00:04:01:02:03"'
> +
> +# Connect R2 to join
> +check ovn-nbctl lrp-add R2 R2_join 00:00:04:01:02:04 20.0.0.2/24
> +check ovn-nbctl lsp-add join r2-join -- set Logical_Switch_Port r2-join \
> + type=router options:router-port=R2_join addresses='"00:00:04:01:02:04"'
> +
> +# Static routes.
> +check ovn-nbctl lr-route-add R1 172.16.1.0/24 20.0.0.2
> +check ovn-nbctl lr-route-add R2 192.168.0.0/16 20.0.0.1
> +
> +# 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")
> +check ovn-nbctl lsp-add foo foo1 \
> +-- lsp-set-addresses foo1 "f0:00:00:01:02:03 192.168.1.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:04", \
> + "172.16.1.1")
> +check ovn-nbctl lsp-add alice alice1 \
> +-- lsp-set-addresses alice1 "f0:00:00:01:02:04 172.16.1.2"
> +
> +# 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:05", \
> +"192.168.2.1")
> +check ovn-nbctl lsp-add bar bar1 \
> +-- lsp-set-addresses bar1 "f0:00:00:01:02:05 192.168.2.2"
> +
> +check ovn-nbctl set logical_router R2 options:lb_force_snat_ip="20.0.0.5"
> +
> +AT_CHECK([ovn-nbctl -- create chassis_template_var chassis="hv1" \
> +
> variables="{vip=172.16.1.1,vport1=8000,backends1=\"192.168.1.2:80,192.168.2.2:80\"}"],
> + [0], [ignore])
> +
> +# Config OVN load-balancer with a VIP.
> +check ovn-nbctl --template lb-add lb1 "^vip:^vport1" "^backends1" tcp
> +check ovn-nbctl lr-lb-add R2 lb1
> +# Wait for ovn-controller to catch up.
> +check ovn-nbctl --wait=hv sync
> +
> +# Start webservers in 'foo1', 'bar1'.
> +OVS_START_L7([foo1], [http])
> +OVS_START_L7([bar1], [http])
> +
> +NETNS_START_TCPDUMP([foo1], [-neei foo1 src 20.0.0.5 and tcp], [foo1])
> +NETNS_START_TCPDUMP([bar1], [-neei bar1 src 20.0.0.5 and tcp], [bar1])
> +
> +check ovs-appctl dpctl/flush-conntrack
> +for i in $(seq 10); do
> + NS_EXEC([alice1], [wget 172.16.1.1:8000 -t 5 -T 1 --retry-connrefused -v
> -o wget$i.log])
> +done
> +
> +OVS_WAIT_UNTIL([
> + n_pkt=$(cat foo1.tcpdump | wc -l)
> + test "${n_pkt}" -ge 1
> +])
> +OVS_WAIT_UNTIL([
> + n_pkt=$(cat bar1.tcpdump | wc -l)
> + test "${n_pkt}" -ge 1
> +])
> +
> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(20.0.0.5) | \
> +sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
> +tcp,orig=(src=172.16.1.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=20.0.0.5,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>)
> +tcp,orig=(src=172.16.1.2,dst=192.168.2.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=20.0.0.5,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>)
> +])
> +
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
This should be OVN_CLEANUP_CONTROLLER([hv1]) instead.
> +
> +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
> +])
I fixed up the two minor things above and pushed this fix to main,
25.03, 24.09 and 24.03.
Regards,
Dumitru
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
