On 3/13/25 4:53 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! This triggered a failure in ovsrobot CI: https://github.com/ovsrobot/ovn/actions/runs/13839126891/job/38722035267 75: system-ovn.at:6106 load-balancer and firewall tuple conflict IPv6 -- parallelization=yes -- ovn_monitor_all=yes ./system-ovn.at:6106: ovs-appctl dpctl/dump-conntrack | grep 2001 | \ grep "orig=.src=4242::3" | \ sed -e 's/port=2001/port=<clnt_s_port>/g' \ -e 's/sport=4242,dport=[0-9]\+/sport=4242,dport=<rnd_port>/g' \ -e 's/state=[0-9_A-Z]*/state=<cleared>/g' \ -e 's/zone=[0-9]*/zone=<cleared>/' | sort --- - 2025-03-13 16:21:18.297453704 +0000 +++ /workspace/ovn-tmp/tests/system-kmod-testsuite.dir/at-groups/75/stdout 2025-03-13 16:21:18.294824729 +0000 @@ -1,4 +1,5 @@ tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>,dport=4242),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,protoinfo=(state=<cleared>) -tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>,dport=4242),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<rnd_port>),zone=<cleared>,protoinfo=(state=<cleared>) +tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>,dport=4242),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>6),zone=<cleared>,protoinfo=(state=<cleared>) +tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>6,dport=4242),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>6),zone=<cleared>,protoinfo=(state=<cleared>) tcp,orig=(src=4242::3,dst=6666::1,sport=<clnt_s_port>,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) > lib/lb.c | 1 + > lib/lb.h | 1 + > northd/northd.c | 30 +++++++++ > northd/ovn-northd.8.xml | 9 +++ > tests/system-ovn.at | 145 ++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 186 insertions(+) > > diff --git a/lib/lb.c b/lib/lb.c > index e67a5fcfd..2fb4dad51 100644 > --- a/lib/lb.c > +++ b/lib/lb.c > @@ -217,6 +217,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 bcc677f82..71689b333 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; > bool template_backends; /* True if the backends are templates. False if > diff --git a/northd/northd.c b/northd/northd.c > index 1d3e132d4..42b5f5058 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -12335,6 +12335,33 @@ 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; > + } > + > + 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. */ > + if (lb_vip->address_family == AF_INET) { > + ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, 100, > + "ip4.dst == ^vip", "next;", Template variables are defined as: "Chassis_Template_Var": { "columns": { "chassis": {"type": "string"}, "variables": { "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}}}, "indexes": [["chassis"]], "isRoot": true}, According to the documentation: <p> One record per chassis, each containing a map, <code>variables</code>, between template variable names and their value for that specific chassis. A template variable has a name and potentially different values on different hypervisors in the OVN cluster. For example, two rows, <code>R1 = (.chassis=C1, variables={(N: V1)}</code> and <code>R2 = (.chassis=C2, variables={(N: V2)}</code> will make <code>ovn-controller</code> running on chassis <code>C1</code> and <code>C2</code> interpret the token <code>N</code> either as <code>V1</code> (on <code>C1</code>) or as <code>V2</code> (on <code>C2</code>). Users can refer to template variables from within other logical components, e.g., within ACL, QoS or Logical_Router_Policy matches or from Load_Balancer VIP and backend definitions. </p> So each variable has a name and that expands to a chassis-specific value according to the Chassis_Template_Var map. Here you're adding a flow that matches unconditionally on "ip4.dst == ^vip". "vip" is just a template variable name, it can be anything, we can't assume anything about it. Instead, we need to match on the actual template, in this case you probably need something like: ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, 100, "ip4.dst == %s", "next;", lb_vip->vip_str ... I didn't test it though. > + &lb->nlb->header_, lb_dps->lflow_ref); > + } else { > + ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, 100, > + "ip6.dst == ^vip", "next;", > + &lb->nlb->header_, lb_dps->lflow_ref); Same here. > + } > + } > +} > + > static void > build_lrouter_flows_for_lb(struct ovn_lb_datapaths *lb_dps, > struct lflow_table *lflows, > @@ -12359,6 +12386,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..5916b4739 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 with match > + <code>ip4.dst == ^vip</code> or <code>ip6.dst == ^vip</code> for This will need to be updated too. > + IPv4 and IPv6 respectively 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 fb8184cd9..626cd7011 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -17411,3 +17411,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 \ > + -- 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]) > + > +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 > +]) Regards, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
