Thanks for the review!

On 10.11.2025 17:10, Dumitru Ceara wrote:
> On 11/7/25 5:27 PM, Lorenzo Bianconi via dev wrote:
>>> The commit [1] ("northd: Add support for stateless ACLs with load 
>>> balancers")
>>> incorrectly handled connection tracking when enable-stateless-acl-with-lb 
>>> is enabled,
>>> causing all stateless traffic in egress to be committed to conntrack.
>>>
>>> This fix properly implements the enable-stateless-acl-with-lb behavior by:
>>> When enable-stateless-acl-with-lb is enabled:
>>>     - Still sending stateless traffic through connection tracker lookup
>>>       in egress.
>>>     - Adding new flow in ls_out_stateful to skip committing NEW stateless 
>>> connections.
>>>     - Only committing established connections for proper return traffic 
>>> handling.
>>>
>>> Fixes: abbc272ac771 ("northd: Add support for stateless ACLs with load 
>>> balancers")
>>> Signed-off-by: Alexandra Rukomoinikova <[email protected]>
>>> Acked-by: Mark Michelson <[email protected]>
>> Acked-by: Lorenzo Bianconi <[email protected]>
>>
>>> ---
>>> v1 --> v2: rebased, added ack
>>> ---
> Hi Alexandra, Mark, Lorenzo,
>
> Thanks for the patch and reviews!
>
>>>   northd/northd.c         |  26 +++++++-
>>>   northd/ovn-northd.8.xml |  10 ++-
>>>   tests/ovn-northd.at     |  65 +++++++++----------
>>>   tests/system-ovn.at     | 138 +++++++++++++++++++++++++++++-----------
>>>   4 files changed, 164 insertions(+), 75 deletions(-)
>>>
>>> diff --git a/northd/northd.c b/northd/northd.c
>>> index 55e31659f..cdf12ec86 100644
>>> --- a/northd/northd.c
>>> +++ b/northd/northd.c
>>> @@ -6098,7 +6098,7 @@ build_stateless_filter(const struct ovn_datapath *od,
>>>                                   action,
>>>                                   &acl->header_,
>>>                                   lflow_ref);
>>> -    } else if (!od->lb_with_stateless_mode) {
>>> +    } else {
>>>           ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL,
>>>                                   acl->priority + OVN_ACL_PRI_OFFSET,
>>>                                   acl->match,
>>> @@ -8437,6 +8437,29 @@ build_lrouter_lb_affinity_default_flows(struct 
>>> ovn_datapath *od,
>>>                     lflow_ref);
>>>   }
>>>   
>>> +static void
>>> +build_lb_rules_for_stateless_acl(struct lflow_table *lflows,
>>> +                                 struct ovn_lb_datapaths *lb_dps)
>>> +{
>>> +    /* When enable-stateless-acl-with-lb is enabled:
>>> +     * 1. All stateless traffic must first pass through connection tracker
>>> +     * in egress.
>>> +     * 2. New connections (ct.new) will bypass commit phase.
>>> +     */
>>> +    struct hmapx_node *hmapx_node;
>>> +    struct ovn_datapath *od;
>>> +
>>> +    HMAPX_FOR_EACH (hmapx_node, &lb_dps->ls_lb_with_stateless_mode) {
>>> +        od = hmapx_node->data;
>>> +        ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 115,
>>> +                      REGBIT_ACL_STATELESS" == 1",
>>> +                      REGBIT_CONNTRACK_NAT" = 1; next;", 
>>> lb_dps->lflow_ref);
>>> +        ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 110,
>>> +                      REGBIT_ACL_STATELESS " == 1 && ct.new",
>>> +                      "next;", lb_dps->lflow_ref);
>>> +    }
>>> +}
>>> +
>>>   static void
>>>   build_lb_rules(struct lflow_table *lflows, struct ovn_lb_datapaths 
>>> *lb_dps,
>>>                  const struct ovn_datapaths *ls_datapaths,
>>> @@ -12857,6 +12880,7 @@ build_lswitch_flows_for_lb(struct ovn_lb_datapaths 
>>> *lb_dps,
>>>       build_lb_rules_pre_stateful(lflows, lb_dps, ls_datapaths, match, 
>>> action);
>>>       build_lb_rules(lflows, lb_dps, ls_datapaths, match, action,
>>>                      meter_groups, svc_mons_data);
>>> +    build_lb_rules_for_stateless_acl(lflows, lb_dps);
>>>   }
>>>   
>>>   /* If there are any load balancing rules, we should send the packet to
>>> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
>>> index b16d2398d..005fd87d1 100644
>>> --- a/northd/ovn-northd.8.xml
>>> +++ b/northd/ovn-northd.8.xml
>>> @@ -2485,8 +2485,6 @@ output;
>>>       <p>
>>>         This is similar to ingress table <code>Pre-ACLs</code> except for
>>>        <code>to-lport</code> traffic.
>>> -     Except when the option enable-stateless-acl-with-lb is enabled:
>>> -     REGBIT_ACL_STATELESS ignored.
>>>       </p>
>>>   
>>>       <p>
>>> @@ -2555,6 +2553,12 @@ output;
>>>         logical router datapath from logical switch datapath for routing.
>>>       </p>
>>>   
>>> +    <p>
>>> +      When <code>enable-stateless-acl-with-lb</code> is enabled,
>>> +      additional priority-115 flow is added to match traffic with
>>> +      <code>REGBIT_ACL_STATELESS</code> set and pass connection tracking.
>>> +    </p>
>>> +
>>>       <h3>Egress Table 4: Pre-stateful</h3>
>>>   
>>>       <p>
>>> @@ -2705,6 +2709,8 @@ output;
>>>       <p>
>>>         This is similar to ingress table <code>Stateful</code> except that
>>>         there are no rules added for load balancing new connections.
>>> +      When <code>enable-stateless-acl-with-lb</code> is enabled, new
>>> +      stateless connections bypass connection tracking.
>>>       </p>
>>>   
>>>       <ul>
>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>>> index b01cf3e95..452a46b9f 100644
>>> --- a/tests/ovn-northd.at
>>> +++ b/tests/ovn-northd.at
>>> @@ -17423,7 +17423,7 @@ AT_CLEANUP
>>>   ])
>>>   
>>>   OVN_FOR_EACH_NORTHD_NO_HV([
>>> -AT_SETUP([enable-stateless-acl-with-lb usage])
>>> +AT_SETUP([ovn-northd: enable-stateless-acl-with-lb usage])
> Nit: "ovn-northd:" is superfluous.
>
>>>   ovn_start ovn-northd
>>>   
>>>   AS_BOX([Create logical switches and ports.])
>>> @@ -17449,51 +17449,44 @@ check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
>>>   
>>>   ovn-sbctl dump-flows sw0 > sw0flows
>>>   
>>> -AT_CHECK(
>>> -  [grep -E 'ls_(in|out)_pre_acl' sw0flows | grep reg0 | ovn_strip_lflows], 
>>> [0], [dnl
>>> -  table=??(ls_in_pre_acl      ), priority=100  , match=(ip), 
>>> action=(reg0[[0]] = 1; next;)
>>> -  table=??(ls_in_pre_acl      ), priority=2001 , match=(ip), 
>>> action=(reg0[[16]] = 1; next;)
>>> -  table=??(ls_out_pre_acl     ), priority=100  , match=(ip), 
>>> action=(reg0[[0]] = 1; next;)
>>> -  table=??(ls_out_pre_acl     ), priority=2001 , match=(ip), 
>>> action=(reg0[[16]] = 1; next;)
>>> +AT_CHECK([grep -E 'ls_out_pre_lb' sw0flows | ovn_strip_lflows], [0], [dnl
>>> +  table=??(ls_out_pre_lb      ), priority=0    , match=(1), action=(next;)
>>> +  table=??(ls_out_pre_lb      ), priority=100  , match=(ip), 
>>> action=(reg0[[2]] = 1; next;)
>>> +  table=??(ls_out_pre_lb      ), priority=110  , match=(eth.mcast), 
>>> action=(next;)
>>> +  table=??(ls_out_pre_lb      ), priority=110  , match=(eth.src == 
>>> $svc_monitor_mac), action=(next;)
>>> +  table=??(ls_out_pre_lb      ), priority=110  , match=(nd || nd_rs || 
>>> nd_ra || mldv1 || mldv2), action=(next;)
>>> +  table=??(ls_out_pre_lb      ), priority=110  , match=(reg0[[16]] == 1), 
>>> action=(next;)
>>>   ])
>>>   
>>> -AT_CHECK(
>>> -  [grep -E 'ls_out_acl_eval' sw0flows | grep 65532 | ovn_strip_lflows], 
>>> [0], [dnl
>>> -  table=??(ls_out_acl_eval    ), priority=65532, match=(!ct.est && ct.rel 
>>> && !ct.new && ct_mark.blocked == 0), action=(reg8[[21]] = 
>>> ct_label.nf_group; reg8[[16]] = 1; ct_commit_nat;)
>>> -  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && !ct.rel 
>>> && ct.rpl && ct_mark.blocked == 0), action=(reg8[[21]] = ct_label.nf_group; 
>>> reg8[[16]] = 1; next;)
>>> -  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && 
>>> ct_mark.allow_established == 1), action=(reg8[[21]] = ct_label.nf_group; 
>>> reg8[[16]] = 1; next;)
>>> -  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.inv || (ct.est 
>>> && ct.rpl && ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;)
>>> -  table=??(ls_out_acl_eval    ), priority=65532, match=(nd || nd_ra || 
>>> nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;)
>>> +AT_CHECK([grep -E 'ls_out_stateful' sw0flows | ovn_strip_lflows], [0], [dnl
>>> +  table=??(ls_out_stateful    ), priority=0    , match=(1), action=(next;)
>>> +  table=??(ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && 
>>> reg0[[13]] == 0), action=(ct_commit { ct_mark.blocked = 0; 
>>> ct_mark.allow_established = reg0[[20]]; ct_label.acl_id = reg2[[16..31]]; 
>>> ct_label.nf_group = 0; ct_label.nf_group_id = 0; }; next;)
>>> +  table=??(ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && 
>>> reg0[[13]] == 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_group = 0; 
>>> ct_label.nf_group_id = 0; }; next;)
>>> +  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_group = 1; ct_label.nf_group_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_group = 1; ct_label.nf_group_id = reg0[[22..29]]; }; next;)
>>>   ])
>>>   
>>>   AS_BOX([Enable enable-stateless-acl-with-lb option.])
>>>   check ovn-nbctl --wait=sb set logical_switch sw0 
>>> other_config:enable-stateless-acl-with-lb=true
>>>   ovn-sbctl dump-flows sw0 > sw0flows
>>> -AT_CHECK(
>>> -  [grep -E 'ls_(in|out)_pre_acl' sw0flows | grep reg0 | ovn_strip_lflows], 
>>> [0], [dnl
>>> -  table=??(ls_in_pre_acl      ), priority=100  , match=(ip), 
>>> action=(reg0[[0]] = 1; next;)
>>> -  table=??(ls_in_pre_acl      ), priority=2001 , match=(ip), 
>>> action=(reg0[[16]] = 1; next;)
>>> -  table=??(ls_out_pre_acl     ), priority=100  , match=(ip), 
>>> action=(reg0[[0]] = 1; next;)
>>> -])
>>>   
>>> -# We do not match conntrack invalid packets in case of load balancers with 
>>> stateless ACLs.
>>> -AT_CHECK(
>>> -  [grep -E 'ls_out_acl_eval' sw0flows | grep 65532 | ovn_strip_lflows], 
>>> [0], [dnl
>>> -  table=??(ls_out_acl_eval    ), priority=65532, match=(!ct.est && ct.rel 
>>> && !ct.new && ct_mark.blocked == 0), action=(reg8[[21]] = 
>>> ct_label.nf_group; reg8[[16]] = 1; ct_commit_nat;)
>>> -  table=??(ls_out_acl_eval    ), priority=65532, match=((ct.est && ct.rpl 
>>> && ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;)
>>> -  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && !ct.rel 
>>> && ct.rpl && ct_mark.blocked == 0), action=(reg8[[21]] = ct_label.nf_group; 
>>> reg8[[16]] = 1; next;)
>>> -  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && 
>>> ct_mark.allow_established == 1), action=(reg8[[21]] = ct_label.nf_group; 
>>> reg8[[16]] = 1; next;)
>>> -  table=??(ls_out_acl_eval    ), priority=65532, match=(nd || nd_ra || 
>>> nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;)
>>> +AT_CHECK([grep -E 'ls_out_stateful' sw0flows | ovn_strip_lflows], [0], [dnl
>>> +  table=??(ls_out_stateful    ), priority=0    , match=(1), action=(next;)
>>> +  table=??(ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && 
>>> reg0[[13]] == 0), action=(ct_commit { ct_mark.blocked = 0; 
>>> ct_mark.allow_established = reg0[[20]]; ct_label.acl_id = reg2[[16..31]]; 
>>> ct_label.nf_group = 0; ct_label.nf_group_id = 0; }; next;)
>>> +  table=??(ls_out_stateful    ), priority=100  , match=(reg0[[1]] == 1 && 
>>> reg0[[13]] == 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_group = 0; 
>>> ct_label.nf_group_id = 0; }; next;)
>>> +  table=??(ls_out_stateful    ), priority=110  , match=(reg0[[16]] == 1 && 
>>> ct.new), action=(next;)
>>> +  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_group = 1; ct_label.nf_group_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_group = 1; ct_label.nf_group_id = reg0[[22..29]]; }; next;)
>>>   ])
>>>   
>>> -AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], 
>>> [dnl
>>> -  table=??(ls_in_pre_stateful ), priority=0    , match=(1), action=(next;)
>>> -  table=??(ls_in_pre_stateful ), priority=100  , match=(reg0[[0]] == 1), 
>>> action=(ct_next;)
>>> -  table=??(ls_in_pre_stateful ), priority=105  , match=(tcp && ip4.dst == 
>>> 10.0.0.4), action=(ct_lb_mark;)
>>> -  table=??(ls_in_pre_stateful ), priority=110  , match=(reg0[[2]] == 1), 
>>> action=(ct_lb_mark;)
>>> -  table=??(ls_in_pre_stateful ), priority=115  , match=(reg0[[2]] == 1 && 
>>> ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;)
>>> -  table=??(ls_in_pre_stateful ), priority=120  , match=(reg0[[2]] == 1 && 
>>> ip4.dst == 10.0.0.4 && tcp.dst == 80), action=(reg4 = 10.0.0.4; 
>>> reg2[[0..15]] = 80; ct_lb_mark;)
>>> -  table=??(ls_in_pre_stateful ), priority=150  , match=(ip4.dst == 
>>> 10.0.0.4 && tcp.dst == 80), action=(ct_lb_mark;)
>>> +AT_CHECK([grep -E 'ls_out_pre_lb' sw0flows | ovn_strip_lflows], [0], [dnl
>>> +  table=??(ls_out_pre_lb      ), priority=0    , match=(1), action=(next;)
>>> +  table=??(ls_out_pre_lb      ), priority=100  , match=(ip), 
>>> action=(reg0[[2]] = 1; next;)
>>> +  table=??(ls_out_pre_lb      ), priority=110  , match=(eth.mcast), 
>>> action=(next;)
>>> +  table=??(ls_out_pre_lb      ), priority=110  , match=(eth.src == 
>>> $svc_monitor_mac), action=(next;)
>>> +  table=??(ls_out_pre_lb      ), priority=110  , match=(nd || nd_rs || 
>>> nd_ra || mldv1 || mldv2), action=(next;)
>>> +  table=??(ls_out_pre_lb      ), priority=110  , match=(reg0[[16]] == 1), 
>>> action=(next;)
>>> +  table=??(ls_out_pre_lb      ), priority=115  , match=(reg0[[16]] == 1), 
>>> action=(reg0[[2]] = 1; next;)
>>>   ])
>>>   
>>>   AS_BOX([Create Load Balancer without port.])
>>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
>>> index 2b880ec37..2567cd779 100644
>>> --- a/tests/system-ovn.at
>>> +++ b/tests/system-ovn.at
>>> @@ -5099,13 +5099,14 @@ AT_CLEANUP
>>>   ])
>>>   
>>>   OVN_FOR_EACH_NORTHD([
>>> -AT_SETUP([enable-stateless-acl-with-lb usage])
>>> +AT_SETUP([ovn-system: enable-stateless-acl-with-lb usage])
> Nit: "ovn-system:" is superfluous.
>
>>>   AT_SKIP_IF([test $HAVE_NC = no])
>>>   
>>> +CHECK_CONNTRACK()
>>>   ovn_start
>>>   OVS_TRAFFIC_VSWITCHD_START()
>>> -
>>>   ADD_BR([br-int])
>>> +ADD_BR([br-ext], [set Bridge br-ext fail-mode=standalone])
>>>   
>>>   # Set external-ids in br-int needed for ovn-controller
>>>   ovs-vsctl \
>>> @@ -5115,62 +5116,127 @@ ovs-vsctl \
>>>           -- 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
>>>   
>>> +ovs-vsctl set Open_Vswitch . external_ids:ovn-bridge-mappings=phynet:br-ext
>>> +
>>>   # Start ovn-controller
>>>   start_daemon ovn-controller
>>>   
>>>   # Logical network:
>>> -# One logical switch with IPv4 load balancers that hairpin the traffic.
>>> -check ovn-nbctl ls-add sw
>>> -check ovn-nbctl lsp-add sw lsp1 -- lsp-set-addresses lsp1 00:00:00:00:00:01
>>> -check ovn-nbctl lsp-add sw lsp2 -- lsp-set-addresses lsp2 00:00:00:00:00:02
>>> +# Two LSs and one Lr - outside ls has access to a physical network
>>> +#                    - ls1 has load balancers
>>> +#   outside - lr1 - ls1
>>> +# Сheck that lb work with stateless acl, external traffic not related
>>> +# to lb doesn't create conntrack records.
>>> +# In switches egress pipeline, on which the balancers and stateless ACL
>>> +# are condigured together - all traffic is checked for connection tracker,
>>> +# but only traffic related to balancing is committed (established 
>>> connection)
>>> +
>>> +check ovn-nbctl ls-add outside
>>> +
>>> +check ovn-nbctl lsp-add outside public
>>> +check ovn-nbctl lsp-set-type public localnet
> We have new nbctl helpers now, "lsp-add-localnet-port" and 
> "lsp-add-router-port",
> it's better to use them.
>
>>> +check ovn-nbctl lsp-set-addresses public unknown
>>> +check ovn-nbctl lsp-set-options public network_name=phynet
>>> +
>>> +check ovn-nbctl lsp-add outside outside-down
>>> +check ovn-nbctl lsp-set-addresses outside-down router
>>> +check ovn-nbctl lsp-set-type outside-down router
>>> +check ovn-nbctl lsp-set-options outside-down router-port=lr1-up
>>>   
>>> -check ovn-nbctl lb-add lb-ipv4-tcp 88.88.88.88:8080 42.42.42.1:4041 tcp
>>> -check ovn-nbctl ls-lb-add sw lb-ipv4-tcp
>>> +check ovn-nbctl lr-add lr1
>>>   
>>> -check ovn-nbctl lr-add rtr
>>> -check ovn-nbctl lrp-add rtr rtr-sw 00:00:00:00:01:00 42.42.42.254/24
>>> -check ovn-nbctl lsp-add-router-port sw sw-rtr rtr-sw
>>> +check ovn-nbctl lrp-add lr1 lr1-up 00:00:01:01:02:03 169.254.0.1/24
>>> +check ovn-nbctl lrp-add lr1 lr1-down 00:00:02:01:02:03 192.168.0.1/24 \
>>> +      -- lrp-set-gateway-chassis lr1-up hv1
>>> +
>>> +check ovn-nbctl ls-add ls1
>>> +
>>> +check ovn-nbctl lsp-add ls1 ls1-up
>>> +check ovn-nbctl lsp-set-addresses ls1-up router
>>> +check ovn-nbctl lsp-set-type ls1-up router
>>> +check ovn-nbctl lsp-set-options ls1-up router-port=lr1-down
>>> +
>>> +check ovn-nbctl lb-add lb-ipv4-tcp 192.168.0.1:8080 192.168.0.101:4041 tcp
>>> +check ovn-nbctl ls-lb-add ls1 lb-ipv4-tcp
>>> +
>>> +check ovn-nbctl lb-add lb-ipv4-udp 192.168.0.1:8081 192.168.0.101:4042 udp
>>> +check ovn-nbctl ls-lb-add ls1 lb-ipv4-udp
>>>   
>>>   ADD_NAMESPACES(lsp1)
>>> -ADD_VETH(lsp1, lsp1, br-int, "42.42.42.1/24", "00:00:00:00:00:01", \
>>> -         "42.42.42.254")
>>> +ADD_VETH(lsp1, lsp1, br-int, "192.168.0.101/24", "00:00:00:00:00:01", \
>>> +         "192.168.0.1")
>>> +check ovn-nbctl lsp-add ls1 lsp1 \
>>> +-- lsp-set-addresses lsp1 "00:00:00:00:00:01 192.168.0.101"
>>>   
>>>   ADD_NAMESPACES(lsp2)
>>> -ADD_VETH(lsp2, lsp2, br-int, "42.42.42.2/24", "00:00:00:00:00:02", \
>>> -         "42.42.42.254")
>>> +ADD_VETH(lsp2, lsp2, br-int, "192.168.0.102/24", "00:00:00:00:00:02", \
>>> +         "192.168.0.1")
>>> +check ovn-nbctl lsp-add ls1 lsp2 \
>>> +-- lsp-set-addresses lsp2 "00:00:00:00:00:02 192.168.0.102"
>>>   
>>>   # Wait for ovn-controller to catch up.
>>> -wait_for_ports_up
> Instead of removing the check we can restrict it to wait for lsp1
> and lsp2:
>
> wait_for_ports_up lsp1 lsp2
>
>>>   check ovn-nbctl --wait=hv sync
>>>   
>>> -# Start IPv4 TCP server on lsp1.
>>> -NETNS_DAEMONIZE([lsp1], [nc -l -k 42.42.42.1 4041], [lsp1.pid])
>>> +ADD_NAMESPACES(external)
>>> +ADD_VETH(external, external, br-ext, "169.254.0.101/24", 
>>> "00:00:00:00:00:04", \
>>> +         "169.254.0.1")
>>>   
>>> -# Send the packet to VIP.
>>> -NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore])
>>> -NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore])
>>> +NS_EXEC([external], [ip r add 192.168.0.0/24 via 169.254.0.1])
>>> +NS_EXEC([lsp1], [ip r add 169.254.0.0/24 via 192.168.0.1])
>>> +NS_EXEC([lsp2], [ip r add 169.254.0.1/24 via 192.168.0.1])
>>>   
>>> -check ovn-nbctl --wait=hv acl-add sw to-lport 2000 'ip' allow-stateless
>>> -check ovn-nbctl --wait=hv acl-add sw from-lport 2000 'ip' allow-stateless
>>> +# Add stateless acl with load balancers.
>>> +check ovn-nbctl acl-add ls1 to-lport 2000 1 allow-stateless
>>> +check ovn-nbctl acl-add ls1 from-lport 2000 1 allow-stateless
>>>   
>>> -# To provide work of load balancer with stateless ACL this is necessary
>>> -# to set enable-stateless-acl-lb to true.
>>> -check ovn-nbctl set logical_switch sw 
>>> other_config:enable-stateless-acl-with-lb=true
>>> +check ovn-nbctl --wait=sb set logical_switch ls1 
>>> other_config:enable-stateless-acl-with-lb=true
>>>   
>>> -check ovn-nbctl --wait=hv sync
>>> +# Checking connectivity
>>> +NS_CHECK_EXEC([external], [ping -q -c 3 -i 0.3 -w 2 192.168.0.101 | 
>>> FORMAT_PING], \
>>> +[0], [dnl
>>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>>> +])
>>>   
>>> -# Send the packet to VIP after add stateless acl.
>>> -NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore])
>>> -NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore])
>>> +NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 192.168.0.102 | 
>>> FORMAT_PING], \
>>> +[0], [dnl
>>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>>> +])
>>>   
>>> -check ovn-nbctl --wait=hv acl-add sw to-lport 2001 'ip' allow-related
>>> -check ovn-nbctl --wait=hv acl-add sw from-lport 2001 'ip' allow-related
>>> +zone_lsp1_id=$(ovn-appctl -t ovn-controller ct-zone-list | grep lsp1 | cut 
>>> -d ' ' -f2)
>>> +zone_lsp2_id=$(ovn-appctl -t ovn-controller ct-zone-list | grep lsp2 | cut 
>>> -d ' ' -f2)
>>>   
>>> -# Send the packet to VIP after add related acls.
>>> -NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore])
>>> -NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore])
>>> +AT_CHECK([ovs-appctl dpctl/flush-conntrack])
>>>   
>>> -OVN_CLEANUP_CONTROLLER([hv1])
>>> +# Start IPv4 TCP and UDP server on lsp1.
>>> +NETNS_DAEMONIZE([lsp1], [nc -l -k 192.168.0.101 4041], [lsp1_tcp.pid])
>>> +NETNS_DAEMONIZE([lsp1], [nc -u -l 192.168.0.101 4042], [lsp1_udp.pid])
>>> +
>>> +# Create another server without load balancer to check that it
>>> +# does not create conntrack records.
>>> +NETNS_DAEMONIZE([lsp1], [nc -l -k 192.168.0.101 4043], [lsp1_non_lb.pid])
>>> +
>>> +# Send the packet to VIP from private network.
>>> +NS_CHECK_EXEC([lsp1], [nc -z 192.168.0.1 8080], [0], [ignore], [ignore])
>>> +
>>> +# Udp connections
>>> +NS_CHECK_EXEC([lsp1], [echo a | nc -u 192.168.0.1 8081], [ignore], 
>>> [ignore], [ignore])
>>> +
>>> +# Check conntrack zone of lsp1 has tcp entry for lb
>>> +AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_lsp1_id | \
>>> +FORMAT_CT(192.168.0.1) | \
>>> +sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
>>> +tcp,orig=(src=192.168.0.101,dst=192.168.0.1,sport=<cleared>,dport=<cleared>),reply=(src=192.168.0.101,dst=192.168.0.101,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>)
>>> +udp,orig=(src=192.168.0.101,dst=192.168.0.1,sport=<cleared>,dport=<cleared>),reply=(src=192.168.0.101,dst=192.168.0.101,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2
>>> +])
>>> +
>>> +AT_CHECK([ovs-appctl dpctl/flush-conntrack])
>>> +
>>> +# Check that internal traffic not related to lb doesn't create conntrack 
>>> records
>>> +NS_CHECK_EXEC([external], [nc -z 192.168.0.101 4043], [0], [])
>>> +
>>> +AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_lsp1_id | 
>>> FORMAT_CT(192.168.0.101) | sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], 
>>> [dnl])
>>> +
>>> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
>>>   
>>>   as ovn-sb
>>>   OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>>> -- 
>>> 2.48.1
>>>
> I went ahead and applied the patch to main and 25.09 after fixing
> up the small issues listed above.
>
> Regards,
> Dumitru
>
>

-- 
regards,
Alexandra.

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to