Thanks, Ales and Alin, applied to main, 26.09 and 26.03. Regards, Dumitru
On 10/21/26 11:56 PM, alin wrote: > Acked-by: Alin Serdean <[email protected]> > > Sent from phone > >> On 21 Oct 2026, at 15:55, Ales Musil via dev <[email protected]> wrote: >> >> On Wed, Oct 21, 2026 at 3:49 PM Dumitru Ceara <[email protected]> wrote: >> >>> Thanks, Ales and Alin for the review and testing! >>> >>> I can squash the following unit/system test update into the patch >>> when applying it if you guys agree. I'll wait for your confirmation >>> though. >>> >>> Regards, >>> Dumitru >>> >>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >>> index a5622e3fbe..d9682f78e4 100644 >>> --- a/tests/ovn-northd.at >>> +++ b/tests/ovn-northd.at >>> @@ -5484,7 +5484,7 @@ AT_CLEANUP >>> ]) >>> >>> OVN_FOR_EACH_NORTHD_NO_HV([ >>> -AT_SETUP([LS load balancer logical flows]) >>> +AT_SETUP([LS/LR load balancer logical flows]) >>> ovn_start >>> >>> lbg=$(ovn-nbctl create load_balancer_group name=lbg) >>> @@ -5494,15 +5494,16 @@ check ovn-nbctl \ >>> lbg0=$(fetch_column nb:load_balancer _uuid name=lbg0) >>> >>> check ovn-nbctl \ >>> + -- lr-add lr0 -- set logical_router lr0 options:chassis=hv1 \ >>> + -- lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 \ >>> -- ls-add sw0 \ >>> + -- lsp-add-router-port sw0 sw0-lr0 lr0-sw0 \ >>> -- add logical_switch sw0 load_balancer_group $lbg \ >>> -- ls-lb-add sw0 lb0 \ >>> + -- add logical_router lr0 load_balancer_group $lbg \ >>> + -- lr-lb-add lr0 lb0 \ >>> -- add load_balancer_group $lbg load_balancer $lbg0 >>> >>> -check ovn-nbctl lr-add lr0 >>> -check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 >>> -check <http://10.0.0.1/24-check> ovn-nbctl lsp-add-router-port sw0 >>> sw0-lr0 lr0-sw0 >>> - >>> check ovn-nbctl --wait=sb sync >>> >>> check_stateful_flows() { >>> @@ -5530,6 +5531,14 @@ check_stateful_flows() { >>> table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && >>> ip4.dst == 10.0.0.20 && tcp.dst == 80), action=(reg4 = 10.0.0.20; >>> reg2[[0..15]] = 80; ct_lb_mark;) >>> ]) >>> >>> + AT_CHECK([grep "ls_in_ct_extract" sw0flows | ovn_strip_lflows], [0], >>> [dnl >>> + table=??(ls_in_ct_extract ), priority=0 , match=(1), action=(next;) >>> + table=??(ls_in_ct_extract ), priority=100 , match=(ct.new && ip && >>> ip.is_frag), action=(reg1[[16..23]] = ct_proto(); reg1[[0..15]] = >>> ct_tp_dst(); next;) >>> + table=??(ls_in_ct_extract ), priority=100 , match=(ct.new && ip && >>> sctp && !ip.is_frag), action=(reg1[[16..23]] = ip.proto; reg1[[0..15]] = >>> sctp.dst; next;) >>> + table=??(ls_in_ct_extract ), priority=100 , match=(ct.new && ip && >>> tcp && !ip.is_frag), action=(reg1[[16..23]] = ip.proto; reg1[[0..15]] = >>> tcp.dst; next;) >>> + table=??(ls_in_ct_extract ), priority=100 , match=(ct.new && ip && >>> udp && !ip.is_frag), action=(reg1[[16..23]] = ip.proto; reg1[[0..15]] = >>> udp.dst; next;) >>> +]) >>> + >>> AT_CHECK([grep "ls_in_lb " sw0flows | ovn_strip_lflows], [0], [dnl >>> table=??(ls_in_lb ), priority=0 , match=(1), action=(next;) >>> table=??(ls_in_lb ), priority=110 , match=(ct.trk && ct.est >>> && !ct.rpl && reg0[[19]] == 1 && ip4), action=(reg4 = ct_nw_dst(); >>> reg2[[0..15]] = ct_tp_dst(); next;) >>> @@ -5571,6 +5580,30 @@ check_stateful_flows() { >>> table=??(ls_out_stateful ), priority=110 , match=(reg0[[1]] == 1 && >>> reg0[[13]] == 0 && reg8[[21]] == 1), action=(ct_commit { ct_mark.blocked = >>> 0; ct_mark.allow_established = reg0[[20]]; ct_label.acl_id = >>> reg2[[16..31]]; ct_label.nf = 1; ct_label.nf_id = reg0[[22..29]]; }; >>> next;) >>> table=??(ls_out_stateful ), priority=110 , match=(reg0[[1]] == 1 && >>> reg0[[13]] == 1 && reg8[[21]] == 1), action=(ct_commit { ct_mark.blocked = >>> 0; ct_mark.allow_established = reg0[[20]]; ct_mark.obs_stage = >>> reg8[[19..20]]; ct_mark.obs_collector_id = reg8[[8..15]]; >>> ct_label.obs_point_id = reg9; ct_label.acl_id = reg2[[16..31]]; >>> ct_label.nf = 1; ct_label.nf_id = reg0[[22..29]]; }; next;) >>> ]) >>> + >>> + ovn-sbctl dump-flows lr0 > lr0flows >>> + AT_CAPTURE_FILE([lr0flows]) >>> + >>> + AT_CHECK([grep "lr_in_ct_extract" lr0flows | ovn_strip_lflows], [0], >>> [dnl >>> + table=??(lr_in_ct_extract ), priority=0 , match=(1), action=(next;) >>> + table=??(lr_in_ct_extract ), priority=100 , match=(ct.new && ip && >>> ip.is_frag), action=(reg1[[16..23]] = ct_proto(); reg1[[0..15]] = >>> ct_tp_dst(); next;) >>> + table=??(lr_in_ct_extract ), priority=100 , match=(ct.new && ip && >>> sctp && !ip.is_frag), action=(reg1[[16..23]] = ip.proto; reg1[[0..15]] = >>> sctp.dst; next;) >>> + table=??(lr_in_ct_extract ), priority=100 , match=(ct.new && ip && >>> tcp && !ip.is_frag), action=(reg1[[16..23]] = ip.proto; reg1[[0..15]] = >>> tcp.dst; next;) >>> + table=??(lr_in_ct_extract ), priority=100 , match=(ct.new && ip && >>> udp && !ip.is_frag), action=(reg1[[16..23]] = ip.proto; reg1[[0..15]] = >>> udp.dst; next;) >>> +]) >>> + >>> + AT_CHECK([grep "lr_in_dnat" lr0flows | ovn_strip_lflows], [0], [dnl >>> + table=??(lr_in_dnat ), priority=0 , match=(1), action=(next;) >>> + table=??(lr_in_dnat ), priority=120 , match=(ct.new && >>> !ct.rel && ip4 && ip4.dst == 10.0.0.10 && reg1[[16..23]] == 6 && >>> reg1[[0..15]] == 80), action=(ct_lb_mark(backends=10.0.0.4:8080);) >>> + table=??(lr_in_dnat ), priority=120 , match=(ct.new && >>> !ct.rel && ip4 && ip4.dst == 10.0.0.20 && reg1[[16..23]] == 6 && >>> reg1[[0..15]] == 80), action=(ct_lb_mark(backends=10.0.0.40:8080);) >>> + table=??(lr_in_dnat ), priority=50 , match=(ct.est && !ct.rel >>> && !ct.new && !ct.rpl && ct_mark.natted), action=(next;) >>> + table=??(lr_in_dnat ), priority=50 , match=(ct.rel && !ct.est >>> && !ct.new && !ct.rpl), action=(ct_commit_nat;) >>> + table=??(lr_in_dnat ), priority=70 , match=(ct.est && !ct.rel >>> && !ct.new && !ct.rpl && ct_mark.natted && ct_mark.force_snat == 1), >>> action=(flags.force_snat_for_lb = 1; next;) >>> + table=??(lr_in_dnat ), priority=70 , match=(ct.est && !ct.rel >>> && !ct.new && !ct.rpl && ct_mark.natted && ct_mark.skip_snat == 1), >>> action=(flags.skip_snat_for_lb = 1; next;) >>> + table=??(lr_in_dnat ), priority=70 , match=(ct.rel && !ct.est >>> && !ct.new && !ct.rpl && ct_mark.force_snat == 1), >>> action=(flags.force_snat_for_lb = 1; ct_commit_nat;) >>> + table=??(lr_in_dnat ), priority=70 , match=(ct.rel && !ct.est >>> && !ct.new && !ct.rpl && ct_mark.skip_snat == 1), >>> action=(flags.skip_snat_for_lb = 1; ct_commit_nat;) >>> +]) >>> + >>> } >>> >>> check_stateful_flows "flags.pkt_sampled = 0; ct_clear; next;" >>> @@ -5641,6 +5674,22 @@ AT_CHECK([grep "ls_out_stateful" sw0flows | >>> ovn_strip_lflows], [0], [dnl >>> table=??(ls_out_stateful ), priority=110 , match=(reg0[[1]] == 1 && >>> reg0[[13]] == 1 && reg8[[21]] == 1), action=(ct_commit { ct_mark.blocked = >>> 0; ct_mark.allow_established = reg0[[20]]; ct_mark.obs_stage = >>> reg8[[19..20]]; ct_mark.obs_collector_id = reg8[[8..15]]; >>> ct_label.obs_point_id = reg9; ct_label.acl_id = reg2[[16..31]]; >>> ct_label.nf = 1; ct_label.nf_id = reg0[[22..29]]; }; next;) >>> ]) >>> >>> +# Remove load balancers from lr0 >>> +check ovn-nbctl lr-lb-del lr0 lb0 >>> +check ovn-nbctl clear logical_router lr0 load_balancer_group >>> +check ovn-nbctl --wait=sb sync >>> + >>> +ovn-sbctl dump-flows lr0 > lr0flows >>> +AT_CAPTURE_FILE([lr0flows]) >>> + >>> +AT_CHECK([grep "lr_in_ct_extract" lr0flows | ovn_strip_lflows], [0], [dnl >>> + table=??(lr_in_ct_extract ), priority=0 , match=(1), action=(next;) >>> +]) >>> + >>> +AT_CHECK([grep "lr_in_dnat" lr0flows | ovn_strip_lflows], [0], [dnl >>> + table=??(lr_in_dnat ), priority=0 , match=(1), action=(next;) >>> +]) >>> + >>> # LB with event=false and reject=false >>> AT_CHECK([ovn-nbctl create load_balancer name=lb1 options:reject=false >>> options:event=false vips:\"10.0.0.20\"=\"\" protocol=tcp], [0], [ignore]) >>> check ovn-nbctl --wait=sb ls-lb-add sw0 lb1 >>> @@ -5652,6 +5701,18 @@ AT_CHECK([ovn-sbctl dump-flows sw0 | grep "ls_in_lb >>> " | ovn_strip_lflows ], [0], >>> table=??(ls_in_lb ), priority=110 , match=(ct.trk && ct.est >>> && !ct.rpl && reg0[[19]] == 1 && ip6), action=(xxreg1 = ct_ip6_dst(); >>> reg2[[0..15]] = ct_tp_dst(); next;) >>> ]) >>> >>> +check ovn-nbctl --wait=sb lr-lb-add lr0 lb1 >>> +AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_dnat " | >>> ovn_strip_lflows ], [0], [dnl >>> + table=??(lr_in_dnat ), priority=0 , match=(1), action=(next;) >>> + table=??(lr_in_dnat ), priority=110 , match=(ct.new && >>> !ct.rel && ip4 && ip4.dst == 10.0.0.20), action=(drop;) >>> + table=??(lr_in_dnat ), priority=50 , match=(ct.est && !ct.rel >>> && !ct.new && !ct.rpl && ct_mark.natted), action=(next;) >>> + table=??(lr_in_dnat ), priority=50 , match=(ct.rel && !ct.est >>> && !ct.new && !ct.rpl), action=(ct_commit_nat;) >>> + table=??(lr_in_dnat ), priority=70 , match=(ct.est && !ct.rel >>> && !ct.new && !ct.rpl && ct_mark.natted && ct_mark.force_snat == 1), >>> action=(flags.force_snat_for_lb = 1; next;) >>> + table=??(lr_in_dnat ), priority=70 , match=(ct.est && !ct.rel >>> && !ct.new && !ct.rpl && ct_mark.natted && ct_mark.skip_snat == 1), >>> action=(flags.skip_snat_for_lb = 1; next;) >>> + table=??(lr_in_dnat ), priority=70 , match=(ct.rel && !ct.est >>> && !ct.new && !ct.rpl && ct_mark.force_snat == 1), >>> action=(flags.force_snat_for_lb = 1; ct_commit_nat;) >>> + table=??(lr_in_dnat ), priority=70 , match=(ct.rel && !ct.est >>> && !ct.new && !ct.rpl && ct_mark.skip_snat == 1), >>> action=(flags.skip_snat_for_lb = 1; ct_commit_nat;) >>> +]) >>> + >>> OVN_CLEANUP_NORTHD >>> AT_CLEANUP >>> ]) >>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at >>> index cd2c11cad8..3cb4b858a8 100644 >>> --- a/tests/system-ovn.at >>> +++ b/tests/system-ovn.at >>> @@ -24034,15 +24034,17 @@ start_daemon ovn-controller >>> check ovn-nbctl \ >>> -- lr-add lr0 \ >>> -- lrp-add lr0 lrp aa:bb:cc:dd:ee:03 10.0.0.3/24 \ >>> + -- set logical_router lr0 options:chassis=hv1 \ >>> -- ls-add ls0 \ >>> -- lsp-add ls0 lsp0 \ >>> -- lsp-set-addresses lsp0 "aa:bb:cc:dd:ee:01 10.0.0.1" \ >>> -- lsp-add ls0 lsp1 \ >>> -- lsp-set-addresses lsp1 "aa:bb:cc:dd:ee:02 10.0.0.2" \ >>> - -- lsp-add-router-port ls0 ls0-lr0 lrp >>> - >>> -check ovn-nbctl lb-add lb0 10.0.1.100:5000 10.0.0.2:6000 udp >>> -check ovn-nbctl ls-lb-add ls0 lb0 >>> + -- lsp-add-router-port ls0 ls0-lr0 lrp \ >>> + -- lb-add lb0 10.0.1.100:5000 10.0.0.2:6000 udp \ >>> + -- ls-lb-add ls0 lb0 \ >>> + -- lb-add lb1 10.0.1.101:5001 10.0.0.2:6001 udp \ >>> + -- lr-lb-add lr0 lb1 >>> >>> ADD_NAMESPACES(lsp0, lsp1) >>> ADD_VETH(lsp0, lsp0, br-int, "10.0.0.1/24", "aa:bb:cc:dd:ee:01", >>> "10.0.0.3") >>> @@ -24079,13 +24081,22 @@ AT_CHECK([grep 'dst=4791' dpflows | grep -q >>> 'ct_tuple4'], [1]) >>> >>> dnl --- UDP to VIP is DNAT'd correctly --- >>> dnl >>> -NETNS_DAEMONIZE([lsp1], [ncat -u -l 10.0.0.2 6000], [lsp1_udp.pid]) >>> -NETNS_START_TCPDUMP([lsp1], [-nn -c 1 -i lsp1 'udp and dst port 6000 and >>> dst host 10.0.0.2'], [lb-dnat]) >>> +NETNS_DAEMONIZE([lsp1], [ncat -u -l 10.0.0.2 6000], [lsp1_udp1.pid]) >>> +NETNS_START_TCPDUMP([lsp1], [-nn -c 1 -i lsp1 'udp and dst port 6000 and >>> dst host 10.0.0.2'], [lb-dnat1]) >>> +NETNS_DAEMONIZE([lsp1], [ncat -u -l 10.0.0.2 6001], [lsp1_udp2.pid]) >>> +NETNS_START_TCPDUMP([lsp1], [-nn -c 1 -i lsp1 'udp and dst port 6001 and >>> dst host 10.0.0.2'], [lb-dnat2]) >>> >>> NS_CHECK_EXEC([lsp0], [echo test | ncat -u 10.0.1.100 5000], [ignore], >>> [ignore], [ignore]) >>> >>> OVS_WAIT_UNTIL([ >>> - total_pkts=$(cat lb-dnat.tcpdump | wc -l) >>> + total_pkts=$(cat lb-dnat1.tcpdump | wc -l) >>> + test "${total_pkts}" -ge 1 >>> +]) >>> + >>> +NS_CHECK_EXEC([lsp0], [echo test | ncat -u 10.0.1.101 5001], [ignore], >>> [ignore], [ignore]) >>> + >>> +OVS_WAIT_UNTIL([ >>> + total_pkts=$(cat lb-dnat2.tcpdump | wc -l) >>> test "${total_pkts}" -ge 1 >>> ]) >>> >>> >>> >> Fine by me. >> >> Regards, >> Ales >> >> >>> >>> >>>> On 10/15/26 1:23 PM, Alin Serdean wrote: >>>> Forgot to mention that I tested this and it looks good from the >>>> offloading perspective, there are a still issues but those are on our >>> end. >>>> >>>> Just one small nit instead of testing this just from a logical switch >>>> perspective, >>>> maybe it’s worth to also add a test from the router lflow point of view. >>>> >>>> Acked-by: Alin Serdean <[email protected]> >>>> >>>> *From: *Ales Musil <[email protected]> >>>> *Date: *Thursday, 15 October 2026 at 13:14 >>>> *To: *Dumitru Ceara <[email protected]> >>>> *Cc: *[email protected] <[email protected]>; [email protected] >>>> <[email protected]>; Tim Rozet <[email protected]>; [email protected] >>>> <[email protected]>; Alin Serdean <[email protected]> >>>> *Subject: *Re: [PATCH ovn] northd: Avoid ct_tuple matches for >>> non-fragments. >>>> >>>> *External email: Use caution opening links or attachments* >>>> >>>> >>>> >>>> >>>> On Tue, Oct 13, 2026 at 12:15 PM Dumitru Ceara <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> >>>> Megaflows matching on these are not offloadable on specific hardware >>>> NICs. These were originally introduced by 20a96b9d2c3d ("northd: >>> Allow >>>> fragmented traffic through LB.") in order to address the fact that >>> with >>>> the userspace datapath fragmented IP packets miss L4 headers in later >>>> fragments, breaking load balancing. A follow up commit, 8e6f9a8355e2 >>>> ("northd: Fix HW offload problem related to ct_tuple."), tried to >>>> address the offload issue but missed to add a check to only send the >>>> first packet of a session (ct_state=+new) to the CT EXTRACT side >>> table >>>> if and only if it is a fragmented IP packet. >>>> >>>> That means that ct_tuple matches would always be present for >>>> megaflows forwarding unidirectional UDP traffic, even if the IP >>> packets >>>> are not fragmented. Some hardware NICs may be capable of offloading >>>> such traffic (see the first Reported-at link) but only if their >>>> megaflows don't match on ct_tuple fields. >>>> >>>> A previous patch attempted to fix a similar issue by limiting switch >>>> conntrack tuple fields extraction only to traffic destined to load >>>> balancers that are configured with L4 ports. That approach has some >>>> downsides and requires an additional logical flow for each load >>> balancer >>>> VIP. >>>> >>>> This patch attempts fixing the issue in a different, a bit more >>> generic >>>> way: it turns out we don't need to send all ct_state=+new traffic to >>>> the CT EXTRACT table, we should instead do that only for fragments. >>>> >>>> Reported-at: https://mail.openvswitch.org/pipermail/ovs- >>>> discuss/2026-October/054041.html <https://mail.openvswitch.org/ >>>> pipermail/ovs-discuss/2026-October/054041.html> >>>> Reported-at: https://patchwork.ozlabs.org/project/ovn/ >>>> patch/[email protected]/ <https:// >>>> patchwork.ozlabs.org/project/ovn/patch/20260814142432.2098055-1- >>>> [email protected]/> >>>> Reported-at: https://redhat.atlassian.net/browse/FDP-4458 <https:// >>>> redhat.atlassian.net/browse/FDP-4458> >>>> Fixes: 20a96b9d2c3d ("northd: Allow fragmented traffic through LB.") >>>> Fixes: 8e6f9a8355e2 ("northd: Fix HW offload problem related to >>>> ct_tuple.") >>>> Assisted-by: Claude Opus 4.6, OpenCode >>>> Signed-off-by: Dumitru Ceara <[email protected] >>>> <mailto:[email protected]>> >>>> --- >>>> northd/northd.c | 34 ++++++++++++++++- >>>> tests/ovn-northd.at <http://ovn-northd.at/> | 62 ++++++++++++++++++ >>>> +++++++++--- >>>> tests/system-ovn.at <http://system-ovn.at/> | 92 ++++++++++++++++++ >>>> +++++++++++++++++++++++++++ >>>> 3 files changed, 180 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/northd/northd.c b/northd/northd.c >>>> index 7c4cb5a62f..84e830428c 100644 >>>> --- a/northd/northd.c >>>> +++ b/northd/northd.c >>>> @@ -18413,10 +18413,25 @@ build_lrouter_in_ct_extract_flows(struct >>>> lflow_table *lflows, >>>> /* Ingress CT_EXTRACT table: Extract connection tracking fields >>>> for new >>>> * connections. */ >>>> ovn_lflow_add(lflows, od, S_ROUTER_IN_CT_EXTRACT, 100, >>>> - "ct.new <http://ct.new/> && ip", >>>> + "ct.new <http://ct.new/> && ip && ip.is_frag", >>>> REG_CT_PROTO " = ct_proto(); " >>>> REG_CT_TP_DST " = ct_tp_dst(); next;", >>>> lflow_ref); >>>> + ovn_lflow_add(lflows, od, S_ROUTER_IN_CT_EXTRACT, 100, >>>> + "ct.new <http://ct.new/> && ip && tcp && ! >>>> ip.is_frag", >>>> + REG_CT_PROTO " = ip.proto; " >>>> + REG_CT_TP_DST " = tcp.dst; next;", >>>> + lflow_ref); >>>> + ovn_lflow_add(lflows, od, S_ROUTER_IN_CT_EXTRACT, 100, >>>> + "ct.new <http://ct.new/> && ip && udp && ! >>>> ip.is_frag", >>>> + REG_CT_PROTO " = ip.proto; " >>>> + REG_CT_TP_DST " = udp.dst; next;", >>>> + lflow_ref); >>>> + ovn_lflow_add(lflows, od, S_ROUTER_IN_CT_EXTRACT, 100, >>>> + "ct.new <http://ct.new/> && ip && sctp && ! >>>> ip.is_frag", >>>> + REG_CT_PROTO " = ip.proto; " >>>> + REG_CT_TP_DST " = sctp.dst; next;", >>>> + lflow_ref); >>>> } >>>> >>>> static void >>>> @@ -19699,10 +19714,25 @@ build_ls_stateful_flows(const struct >>>> ls_stateful_record *ls_stateful_rec, >>>> * balancers. */ >>>> if (ls_stateful_rec->has_lb_vip) { >>>> ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100, >>>> - "ct.new <http://ct.new/> && ip", >>>> + "ct.new <http://ct.new/> && ip && >>> ip.is_frag", >>>> REG_CT_PROTO " = ct_proto(); " >>>> REG_CT_TP_DST " = ct_tp_dst(); next;", >>>> ls_stateful_rec->lflow_ref); >>>> + ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100, >>>> + "ct.new <http://ct.new/> && ip && tcp && ! >>>> ip.is_frag", >>>> + REG_CT_PROTO " = ip.proto; " >>>> + REG_CT_TP_DST " = tcp.dst; next;", >>>> + ls_stateful_rec->lflow_ref); >>>> + ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100, >>>> + "ct.new <http://ct.new/> && ip && udp && ! >>>> ip.is_frag", >>>> + REG_CT_PROTO " = ip.proto; " >>>> + REG_CT_TP_DST " = udp.dst; next;", >>>> + ls_stateful_rec->lflow_ref); >>>> + ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100, >>>> + "ct.new <http://ct.new/> && ip && sctp && ! >>>> ip.is_frag", >>>> + REG_CT_PROTO " = ip.proto; " >>>> + REG_CT_TP_DST " = sctp.dst; next;", >>>> + ls_stateful_rec->lflow_ref); >>>> } >>>> >>>> build_lb_hairpin(ls_stateful_rec, od, lflows, ls_stateful_rec- >>>>> lflow_ref); >>>> diff --git a/tests/ovn-northd.at <http://ovn-northd.at/> b/tests/ >>>> ovn-northd.at <http://ovn-northd.at/> >>>> index 7a0a400c4a..a5622e3fbe 100644 >>>> --- a/tests/ovn-northd.at <http://ovn-northd.at/> >>>> +++ b/tests/ovn-northd.at <http://ovn-northd.at/> >>>> @@ -4335,8 +4335,6 @@ flow="inport == \"lsp1\" && ${flow_eth} && >>>> ${flow_ip} && ${flow_tcp}" >>>> ovn_trace --ct new --ct new ls "${flow}" >>>> AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls >>>> "${flow}"], [0], [dnl >>>> ct_lb_mark { >>>> - reg1[[16..23]] = 6; >>>> - reg1[[0..15]] = 80; >>>> ct_lb_mark { >>>> reg0[[6]] = 0; >>>> reg0[[12]] = 0; >>>> @@ -4350,6 +4348,20 @@ ct_lb_mark { >>>> # UDP packets should go to conntrack. >>>> flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && >>> ${flow_udp}" >>>> AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls >>>> "${flow}"], [0], [dnl >>>> +ct_lb_mark { >>>> + ct_lb_mark { >>>> + reg0[[6]] = 0; >>>> + reg0[[12]] = 0; >>>> + ct_lb_mark /* default (use --ct to customize) */ { >>>> + output("lsp2"); >>>> + }; >>>> + }; >>>> +}; >>>> +]) >>>> + >>>> +# Fragmented UDP packets should go to conntrack. >>>> +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && >>>> ${flow_udp} && ip.is_frag" >>>> +AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls >>>> "${flow}"], [0], [dnl >>>> ct_lb_mark { >>>> reg1[[16..23]] = 17; >>>> reg1[[0..15]] = 80; >>>> @@ -4378,6 +4390,20 @@ output("lsp2"); >>>> # UDP packets still go to conntrack. >>>> flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && >>> ${flow_udp}" >>>> AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls >>>> "${flow}"], [0], [dnl >>>> +ct_lb_mark { >>>> + ct_lb_mark { >>>> + reg0[[6]] = 0; >>>> + reg0[[12]] = 0; >>>> + ct_lb_mark /* default (use --ct to customize) */ { >>>> + output("lsp2"); >>>> + }; >>>> + }; >>>> +}; >>>> +]) >>>> + >>>> +# Fragmented UDP packets still go to conntrack. >>>> +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && >>>> ${flow_udp} && ip.is_frag" >>>> +AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls >>>> "${flow}"], [0], [dnl >>>> ct_lb_mark { >>>> reg1[[16..23]] = 17; >>>> reg1[[0..15]] = 80; >>>> @@ -4478,8 +4504,6 @@ check ovn-nbctl --wait=sb sync >>>> flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && >>> ${flow_tcp}" >>>> AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls >>>> "${flow}"], [0], [dnl >>>> ct_lb_mark { >>>> - reg1[[16..23]] = 6; >>>> - reg1[[0..15]] = 80; >>>> ct_lb_mark { >>>> reg0[[6]] = 0; >>>> reg0[[12]] = 0; >>>> @@ -4493,6 +4517,20 @@ ct_lb_mark { >>>> # UDP packets should go to conntrack. >>>> flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && >>> ${flow_udp}" >>>> AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls >>>> "${flow}"], [0], [dnl >>>> +ct_lb_mark { >>>> + ct_lb_mark { >>>> + reg0[[6]] = 0; >>>> + reg0[[12]] = 0; >>>> + ct_lb_mark /* default (use --ct to customize) */ { >>>> + output("lsp2"); >>>> + }; >>>> + }; >>>> +}; >>>> +]) >>>> + >>>> +# Fragmented UDP packets should go to conntrack. >>>> +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && >>>> ${flow_udp} && ip.is_frag" >>>> +AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls >>>> "${flow}"], [0], [dnl >>>> ct_lb_mark { >>>> reg1[[16..23]] = 17; >>>> reg1[[0..15]] = 80; >>>> @@ -4521,6 +4559,20 @@ output("lsp2"); >>>> # UDP packets still go to conntrack. >>>> flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && >>> ${flow_udp}" >>>> AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls >>>> "${flow}"], [0], [dnl >>>> +ct_lb_mark { >>>> + ct_lb_mark { >>>> + reg0[[6]] = 0; >>>> + reg0[[12]] = 0; >>>> + ct_lb_mark /* default (use --ct to customize) */ { >>>> + output("lsp2"); >>>> + }; >>>> + }; >>>> +}; >>>> +]) >>>> + >>>> +# Fragmented UDP packets still go to conntrack. >>>> +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && >>>> ${flow_udp} && ip.is_frag" >>>> +AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls >>>> "${flow}"], [0], [dnl >>>> ct_lb_mark { >>>> reg1[[16..23]] = 17; >>>> reg1[[0..15]] = 80; >>>> @@ -8580,8 +8632,6 @@ flow="eth.dst == 00:00:00:00:01:00 && inport >>>> == \"rtr-ls\" && ip4.src == 42.42.4 >>>> ovn_trace --ct new "${flow}" --lb-dst 42.42.42.42:4242 >>>> <http://42.42.42.42:4242/> >>>> AT_CHECK_UNQUOTED([ovn_trace --ct new --minimal "${flow}" --lb-dst >>>> 42.42.42.42:4242 <http://42.42.42.42:4242/>], [0], [dnl >>>> ct_dnat /* assuming no un-dnat entry, so no change */ { >>>> - reg1[[16..23]] = 6; >>>> - reg1[[0..15]] = 4343; >>>> ct_lb_mark /* default (use --ct to customize) */ { >>>> ip.ttl--; >>>> eth.src = 00:00:00:00:01:00; >>>> diff --git a/tests/system-ovn.at <http://system-ovn.at/> b/tests/ >>>> system-ovn.at <http://system-ovn.at/> >>>> index 13e62bf9ba..3239d709b4 100644 >>>> --- a/tests/system-ovn.at <http://system-ovn.at/> >>>> +++ b/tests/system-ovn.at <http://system-ovn.at/> >>>> @@ -23978,3 +23978,95 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to >>>> query port patch-.*/d >>>> >>>> AT_CLEANUP >>>> ]) >>>> + >>>> +OVN_FOR_EACH_NORTHD([ >>>> +AT_SETUP([CT extraction limited to LB VIPs]) >>>> +AT_KEYWORDS([ovnlb ct_extract]) >>>> +AT_SKIP_IF([test $HAVE_NCAT = no]) >>>> +AT_SKIP_IF([test $HAVE_TCPDUMP = no]) >>>> + >>>> +dnl Test that the CT_EXTRACT pipeline stage currently matches >>> doesn't >>>> +dnl generate ct_tuple4 megaflow matches for non fragments. >>>> + >>>> +ovn_start >>>> + >>>> +OVS_TRAFFIC_VSWITCHD_START() >>>> +ADD_BR([br-int]) >>>> + >>>> +check 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_daemon ovn-controller >>>> + >>>> +check ovn-nbctl \ >>>> + -- lr-add lr0 \ >>>> + -- lrp-add lr0 lrp aa:bb:cc:dd:ee:03 10.0.0.3/24 >>>> <http://10.0.0.3/24> \ >>>> + -- ls-add ls0 \ >>>> + -- lsp-add ls0 lsp0 \ >>>> + -- lsp-set-addresses lsp0 "aa:bb:cc:dd:ee:01 10.0.0.1" \ >>>> + -- lsp-add ls0 lsp1 \ >>>> + -- lsp-set-addresses lsp1 "aa:bb:cc:dd:ee:02 10.0.0.2" \ >>>> + -- lsp-add-router-port ls0 ls0-lr0 lrp >>>> + >>>> +check ovn-nbctl lb-add lb0 10.0.1.100:5000 <http://10.0.1.100:5000/ >>>>> 10.0.0.2:6000 <http://10.0.0.2:6000/> udp >>>> +check ovn-nbctl ls-lb-add ls0 lb0 >>>> + >>>> +ADD_NAMESPACES(lsp0, lsp1) >>>> +ADD_VETH(lsp0, lsp0, br-int, "10.0.0.1/24 <http://10.0.0.1/24>", >>>> "aa:bb:cc:dd:ee:01", "10.0.0.3") >>>> +ADD_VETH(lsp1, lsp1, br-int, "10.0.0.2/24 <http://10.0.0.2/24>", >>>> "aa:bb:cc:dd:ee:02", "10.0.0.3") >>>> + >>>> +wait_for_ports_up lsp0 lsp1 >>>> +check ovn-nbctl --wait=hv sync >>>> + >>>> +dnl Verify basic connectivity before testing megaflows. >>>> +NS_CHECK_EXEC([lsp0], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | >>>> FORMAT_PING], \ >>>> +[0], [dnl >>>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms >>>> +]) >>>> + >>>> +check ovs-appctl revalidator/pause >>>> +on_exit 'ovs-appctl revalidator/resume' >>>> + >>>> +dnl --- Unfragmented unidirectional UDP to non-VIP must not have >>>> ct_tuple4 --- >>>> +dnl >>>> +dnl Start a UDP listener on lsp1 to absorb traffic. Start a >>> continuous >>>> +dnl unidirectional UDP sender on lsp0 to port 4791 (a non-VIP port). >>>> +dnl The listener does not send replies, so every packet stays >>>> ct.new <http://ct.new/>. >>>> +dnl There should be no ct_tuple4 matches in the megaflows for this >>>> traffic. >>>> +NETNS_DAEMONIZE([lsp1], [ncat -u -l 4791 > /dev/null], [udp- >>>> listener.pid]) >>>> +NETNS_DAEMONIZE([lsp0], [bash -c 'while true; do echo test; sleep >>>> 0.2; done | ncat -u 10.0.0.2 4791'], [udp-sender.pid]) >>>> + >>>> +dnl Wait until the datapath has a cached flow for the UDP traffic >>>> we sent. >>>> +OVS_WAIT_UNTIL([ovs-appctl dpctl/dump-flows | grep -q 'dst=4791']) >>>> + >>>> +ovs-appctl dpctl/dump-flows > dpflows >>>> +AT_CAPTURE_FILE([dpflows]) >>>> + >>>> +AT_CHECK([grep 'dst=4791' dpflows | grep -q 'ct_tuple4'], [1]) >>>> + >>>> +dnl --- UDP to VIP is DNAT'd correctly --- >>>> +dnl >>>> +NETNS_DAEMONIZE([lsp1], [ncat -u -l 10.0.0.2 6000], [lsp1_udp.pid]) >>>> +NETNS_START_TCPDUMP([lsp1], [-nn -c 1 -i lsp1 'udp and dst port >>>> 6000 and dst host 10.0.0.2'], [lb-dnat]) >>>> + >>>> +NS_CHECK_EXEC([lsp0], [echo test | ncat -u 10.0.1.100 5000], >>>> [ignore], [ignore], [ignore]) >>>> + >>>> +OVS_WAIT_UNTIL([ >>>> + total_pkts=$(cat lb-dnat.tcpdump | wc -l) >>>> + test "${total_pkts}" -ge 1 >>>> +]) >>>> + >>>> +check ovs-appctl revalidator/resume >>>> + >>>> +OVN_CLEANUP_CONTROLLER([hv1]) >>>> +OVN_CLEANUP_NORTHD >>>> + >>>> +as >>>> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >>>> +/connection dropped.*/d"]) >>>> +AT_CLEANUP >>>> +]) >>>> -- >>>> 2.55.0 >>>> >>>> >>>> Looks good to me, thanks. >>>> Acked-by: Ales Musil <[email protected] <mailto:[email protected]>> >>> >>> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
