Hello Dumitru,
On 04.04.2025 13:55, Dumitru Ceara wrote:
> On 4/3/25 6:57 PM, Aleksandra Rukomoinikova wrote:
>> On 03.04.2025 11:58, Dumitru Ceara wrote:
>> /Hi, Dumitru/
>> Thank you for your time, I will respond to your comments below.
> Hi Alexandra,
>
>>> On 3/23/25 3:07 PM, Vladislav Odintsov wrote:
>>>> Hi Dumitru, Venu,
>>>>
>>> Hi Vladislav,
>>>
>>>> thanks for your time spent digging into this!
>>>>
>>>> The code within current patch sends reply traffic to conntrack only if it
>>>> sees that it needs to do that (LB is created within the datapath). If
>>>> there is no LB, so traffic will not be sent to conntrack and HW-offload
>>>> will probably work.
>>>> I’m worried why load balancer is created even if it doesn’t work in such
>>>> scenario? Shouldn’t that be fixed from the orchestrator side (not to
>>>> create LB)?
>>>>
>>> I guess I'm a bit confused by the statement above. I don't see all
>>> reply traffic being sent to conntrack. Instead the patch only sends
>>> traffic destined to the LB VIP to conntrack.
>>>
>>> @@ -7752,8 +7806,7 @@ build_lb_rules_pre_stateful(struct lflow_table
>>> *lflows,
>>> }
>>> ds_put_cstr(action, "ct_lb_mark;");
>>>
>>> - ds_put_format(match, REGBIT_CONNTRACK_NAT" == 1 && %s.dst == %s",
>>> - ip_match, lb_vip->vip_str);
>>> + ds_put_format(match, "%s.dst == %s", ip_match, lb_vip->vip_str);
>>> if (lb_vip->port_str) {
>>> ds_put_format(match, " && %s.dst == %s", lb->proto,
>>> lb_vip->port_str);
>>> @@ -7763,6 +7816,22 @@ build_lb_rules_pre_stateful(struct lflow_table
>>> *lflows,
>>> lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths),
>>> S_SWITCH_IN_PRE_STATEFUL, 120, ds_cstr(match),
>>> ds_cstr(action),
>>> &lb->nlb->header_, lb_dps->lflow_ref);
>>> +
>>> + /* Pass all VIP traffic to the conntrack to support
>>> + load balancers in the case of stateless acl. */
>>> + if (lb_vip->port_str && lb->proto) {
>>> + ds_clear(action);
>>> + ds_clear(match);
>>> +
>>> + ds_put_cstr(action, "ct_lb_mark;");
>>> + ds_put_format(match, "%s && %s.dst == %s",
>>> + lb->proto, ip_match, lb_vip->vip_str);
>>> +
>>> + ovn_lflow_add_with_dp_group(
>>> + lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths),
>>> + S_SWITCH_IN_PRE_STATEFUL, 115, ds_cstr(match),
>>> ds_cstr(action),
>>> + &lb->nlb->header_, lb_dps->lflow_ref);
>>> + }
>>> }
>>> }
>>>
>>> Also, the code above generates logical flows in in_pre_stateful, at priority
>>> 115, that can never be matched.
>>>
>>> E.g., "make sandbox" and then:
>>> $ ovn-nbctl ls-add sw1
>>> $ ovn-nbctl lb-add lb 1.1.1.1:80 2.2.2.2:80 tcp
>>> $ ovn-nbctl ls-lb-add sw1 lb
>>> $ ovn-nbctl acl-add sw1 from-lport 1002 'ip4 || ip6' allow-stateless
>>>
>>> Dump the in_pre_stateful relevant logical flows:
>>>
>>> table=6 (ls_in_pre_stateful ), priority=120 , match=(ip4.dst ==
>>> 1.1.1.1 && tcp.dst == 80), action=(reg4 = 1.1.1.1; reg2[0..15] = 80;
>>> ct_lb_mark;)
>>> table=6 (ls_in_pre_stateful ), priority=115 , match=(reg0[2] == 1 &&
>>> ip.is_frag), action=(reg0[19] = 1; ct_lb_mark;)
>>> table=6 (ls_in_pre_stateful ), priority=115 , match=(tcp && ip4.dst ==
>>> 1.1.1.1), action=(ct_lb_mark;)
>>>
>>> The first flow match (prio 120) is a super set of the match of the third
> I had missed the tcp.dst == 80 match here, it's not a super set. But
> still, please see my comment on LB hairpin below.
>
>>> flow match (prio 115). That means the third flow will never be hit.
>>>
>>> This seems a bit weird.
>>>
>>> Regards,
>>> Dumitru
>>>
>>>> regards,
>>>> Vladislav Odintsov
>>>>
>>>>> On 23 Mar 2025, at 11:56, Dumitru Ceara via dev <ovs-dev@openvswitch.org>
>>>>> wrote:
>>>>>
>>>>> On 3/22/25 2:01 AM, Venu Iyer wrote:
>>>>>> Sorry for the much delayed response Dumitru; I am just catching up on
>>>>>> this and will look at the patch and follow-ups.
>>>>>> Just a quick response below.
>>>>>>
>>>>> Hi Venu,
>>>>>
>>>>>> ________________________________
>>>>>> From: Dumitru Ceara <dce...@redhat.com>
>>>>>> Sent: Wednesday, March 19, 2025 6:00 AM
>>>>>> To: Aleksandra Rukomoinikova <ARukomoinikova@k2.cloud>;
>>>>>> d...@openvswitch.org <d...@openvswitch.org>
>>>>>> Cc: Venu Iyer <venugop...@nvidia.com>; Han Zhou <hz...@ovn.org>; Numan
>>>>>> Siddique <num...@ovn.org>
>>>>>> Subject: Re: [PATCH ovn v2] northd: Make work load balancer with
>>>>>> stateless ACL.
>>>>>>
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>>> On 3/18/25 5:08 PM, Aleksandra Rukomoinikova wrote:
>>>>>>> Hi Dumitru,
>>>>>>>
>>>>>> Hi Alexandra,
>>>>>>
>>>>>>> I want to clarify a little my question that I asked yesterday at the
>>>>>>> meeting,
>>>>>>> it was formulated incorrectly by me
>>>>>>>
>>>>>>> in a situation when we have stateless acl and related one (or lb
>>>>>>> configured)
>>>>>>>
>>>>>>> before patch [0] (that is, before version v23.03.0 judging by the log)
>>>>>>> there was a lookup in ingress
>>>>>>> and in the egress of all packages in the conntrack
>>>>>>>
>>>>>>> because the packages fell under:
>>>>>>>
>>>>>>> table=4 (ls_in_pre_acl), priority=100, match=(ip), action=(reg0[0] = 1;
>>>>>>> next;)
>>>>>>> table=0 (ls_out_pre_lb), priority=100, match=(ip), action=(reg0[2] = 1;
>>>>>>> next;)
>>>>>>>
>>>>>>> and then the packages were not committed because:
>>>>>>> table=8 (ls_in_acl), priority=2002, match=(stateless acl match),
>>>>>>> action=(next;)
>>>>>>>
>>>>>>> That is, we were cheating traffic to the contract, but did not commit,
>>>>>>> an invalid state was returned.
>>>>>>>
>>>>>>> That is, such a situation arose when using stateless acls, which are
>>>>>>> higher priority than statefull ones
>>>>>>> or also when using lb.
>>>>>>>
>>>>>>> A similar situation is now occurring in my patch, I bypass it this way:
>>>>>>>
>>>>>>> Despite the enabled ct_match_inv option, we add it only if there is no
>>>>>>> stateless acl + lb bundle
>>>>>>>
>>>>>>> What do you think?
>>>>>>>
>>>>>>> [0] -
>>>>>>> https://github.com/ovn-org/ovn/commit/a0f82efdd9dfd3ef2d9606c1890e353df1097a51.
>>>>>>>
>>>>>> Thanks for the additional details!
>>>>>>
>>>>>> I'm CC-ing Venu, Han and Numan for more input on this. Venu, you added
>>>>>> this restriction that doesn't allow traffic matched by stateless ACLs to
>>>>>> be load balanced, what is your opinion with respect to Alexandra's
>>>>>> suggested change below?
>>>>>>
>>>>>>> [0] Removed support for using load balancers in conjunction with
>>>>>>> stateless ACL.
>>>>>>> This commit removes the ability to use load balancers alongside
>>>>>>> stateless ACL.
>>>>>>> If a load balancer is created, the datapath is no longer fully
>>>>>>> stateless.
>>>>>>> Therefore, to avoid traffic being directed to the contract, it is
>>>>>>> recommended
>>>>>>> to refrain from creating a load balancer entirely.
>>>>>> This was an issue with ovn-k8s where the logical switch has LB for the
>>>>>> service IPs ..
>>>>>> and this made configuring any stateless rules impossible - e.g if there
>>>>>> is a application
>>>>>> with unidirection UDP flows such flows will not be offloaded without
>>>>>> stateless.
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Commit [0] ensures the separation of stateful and stateless scenarios
>>>>>>> in the absence of load balancers, without altering the functionality
>>>>>>> of load balancers themselves.
>>>>>>>
>>>>>>> When a logical switch is configured with stateless ACL and a load
>>>>>>> balancer,
>>>>>>> the check for the `REGBIT_CONNTRACK_NAT` flag in the `pre_lb` stage of
>>>>>>> the ingress pipeline becomes redundant. Traffic directed to the load
>>>>>>> balancer
>>>>>>> must be processed through the conntrack.
>>>>>>>
>>>>>>> To ensure proper load balancer operation, a rule must be added to match
>>>>>>> the load balancer's VIP address and its protocol (if applicable). This
>>>>>>> rule
>>>>>>> is added to the datapath group and does not negatively impact
>>>>>>> performance.
>>>>>>> Packets matching this rule would still be directed to the contract via
>>>>>>> lower-priority rules in the absence of stateless ACL. However, with
>>>>>>> stateless ACL,
>>>>>>> this rule enables load balancing when the client balances traffic to
>>>>>>> itself.
>>>>>>>
>>>>>>> In the egress pipeline, the stateless register should only be set if no
>>>>>>> load balancers are present on the datapath. This maintains a clear
>>>>>>> separation
>>>>>>> between Stateful and Stateless modes when using ACL.
>>>>>>> If a user creates a load balancer on a logical switch, they should be
>>>>>>> aware
>>>>>>> that the traffic will no longer be fully stateless.
>>>>>> I think statelesss and LB should not overlap and, hence, should be able
>>>>>> to
>>>>>> coexist. Maybe Han can provide some input as well.
>>>>>>
>>>>> Ah, I see, so the problem was for unidirectional traffic that needs to
>>>>> be offloaded (which doesn't happen if it's forwarded on
>>>>> ct_state=+trk+new).
>>>>>
>>>>> I guess Alexandra's change here doesn't suffice though:
>>>>>
>>>>> @@ -7763,6 +7816,22 @@ build_lb_rules_pre_stateful(struct lflow_table
>>>>> *lflows,
>>>>> lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths),
>>>>> S_SWITCH_IN_PRE_STATEFUL, 120, ds_cstr(match),
>>>>> ds_cstr(action),
>>>>> &lb->nlb->header_, lb_dps->lflow_ref);
>>>>> +
>>>>> + /* Pass all VIP traffic to the conntrack to support
>>>>> + load balancers in the case of stateless acl. */
>>>>> + if (lb_vip->port_str && lb->proto) {
>>>>> + ds_clear(action);
>>>>> + ds_clear(match);
>>>>> +
>>>>> + ds_put_cstr(action, "ct_lb_mark;");
>>>>> + ds_put_format(match, "%s && %s.dst == %s",
>>>>> + lb->proto, ip_match, lb_vip->vip_str);
>>>>> +
>>>>> + ovn_lflow_add_with_dp_group(
>>>>> + lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths),
>>>>> + S_SWITCH_IN_PRE_STATEFUL, 115, ds_cstr(match),
>>>>> ds_cstr(action),
>>>>> + &lb->nlb->header_, lb_dps->lflow_ref);
>>>>> + }
>>>>> }
>>>>>
>>>>> This will send to conntrack in the original direction only traffic that is
>>>>> destined to the LB VIP.
>>>>>
>>>>> But for replies we'll send everything to conntrack:
>>>>>
>>>>> @@ -5894,7 +5894,11 @@ build_stateless_filter(const struct ovn_datapath
>>>>> *od,
>> you said that you don't see how all the replay traffic ends up in the
>> conntrack:
>>
>> this flow will be added to the egress pipeline only if there are no lb,
>> and it is responsible for sending packets to the conntrack.
>>
> I still don't understand, sorry. Let's assume we have a TCP LB with
> VIP=42.42.42.42:80 and backends=10.10.10.3:8080 configured on a switch;
> and a client LSP (10.10.10.2) and a server LSP (10.10.10.3).
>
> The client tries to connect to the LB VIP:
>
> 1. TCP SYN:
> ip_src=10.10.10.2, tcp_src=x, ip_dst=42.42.42.42, tcp_dst=80
>
> 2. TCP SYN is DNATed by the OVN LB:
> ip_src=10.10.10.2, tcp_src=x, ip_dst=10.10.10.3, tcp_dst=8080
>
> 3. The SYN reaches the server and the server replies with SYN+ACK:
> ip_src=10.10.10.3, tcp_src=8080, ip_dst=10.10.10.2, tcp_dst=x
>
> This packet should be unDNATed. How is this packet matched by the flow
> you mentioned above? That flow matches on ip.dst == lb.vip.
I meant using this flow for the hairpin case (the client is the server),
but i missed the ability to configure ip for snat option. i will fix it
like that?
table=6 (ls_in_pre_stateful ), priority=115 , match=(tcp && ip4.dst == lb
vip/snai_hairpin if configured), action=(ct_lb_mark;)
>
>>>>> action,
>>>>> &acl->header_,
>>>>> lflow_ref);
>>>>> - } else {
>>>>> + } else if (!od->nbs->n_load_balancer){
>>>>> + /* For cases when we have statefull ACLs but no load
>>>>> + balancer configured on logical switch - we should
>>>>> + completely bypass conntrack on egress, otherwise
>>>>> + it is necessary to check the balanced traffic. */
>>>>> ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL,
>>>>> acl->priority + OVN_ACL_PRI_OFFSET,
>>>>> acl->match,
>>>>>
>>>>> So, I'm afraid we are also going to need something that restricts traffic
>>>>> that is sent to conntrack in the reply direction. E.g., logical flows
>>>>> that match on traffic originated by load balancer backends.
>>>>>
>>>>> We need to be careful when we do that though. We don't want to create
>>>>> a logical flow explosion (there can be quite a few backends so a logical
>>>>> flow per backend might be very costly for both ovn-northd and the SB DB).
>>>>>
>> regarding the added flows in ingress:
>>
>> table=6 (ls_in_pre_stateful ), priority=120 , match=(ip4.dst == 1.1.1.1
>> && tcp.dst == 80), action=(reg4 = 1.1.1.1; reg2[0..15] = 80; ct_lb_mark;)
>> table=6 (ls_in_pre_stateful ), priority=115 , match=(reg0[2] == 1 &&
>> ip.is_frag), action=(reg0[19] = 1; ct_lb_mark;)
>> table=6 (ls_in_pre_stateful ), priority=115 , match=(tcp && ip4.dst ==
>> 1.1.1.1), action=(ct_lb_mark;)
>>
>> priorities overlapped, i think, i should fix it like this ?
>>
>> table=6 (ls_in_pre_stateful ), priority=125 , match=(ip4.dst == 1.1.1.1
>> && tcp.dst == 80), action=(reg4 = 1.1.1.1; reg2[0..15] = 80; ct_lb_mark;)
>> table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[2] == 1 &&
>> ip.is_frag), action=(reg0[19] = 1; ct_lb_mark;)
>> table=6 (ls_in_pre_stateful ), priority=115 , match=(tcp && ip4.dst ==
>> 1.1.1.1), action=(ct_lb_mark;)
>>
>> the first rule will work for lb in general, the third rule is important
>> in balancing from the client to itself to establish a connection before
>> dnat occurs.
> I'm not sure I follow. For hairpin traffic, client -> LB -> client,
> there's an additional SNAT happening after DNAT in the original
> direction. The SNAT IP is not necessarily the LB VIP, it's configurable
> through LB.options.hairpin_snat_ip. And we already support this kind of
> connection so I'm not sure why we need that third flow. We already have
> LB hairpin system tests for the regular case. Would it be possible to
> add new ones for the case when we have (stateless) ACLs too?
>
>>>>>> Sorry again for not being on top of this.
>>>>>>
>>>>> No worries, thanks for the reply!
>>>>>
>>>>> Regards,
>>>>> Dumitru
>>>>>
>>>>>> -venu
>>>>>>
>>>>>>> Also in case of lb configured with stateless ACLs we no longer take
>>>>>>> into account
>>>>>>> ct.inv packets in egress. They will be dropped further, at the
>>>>>>> hypervisor level.
>>>>>>>
>>>>>>> [0] - ovn-org@a0f82ef.
>>>>>>>
>>>>>>> Signed-off-by: Alexandra Rukomoinikova
>>>>>>> <arukomoinikova@k2.cloud><mailto:arukomoinikova@k2.cloud>
>>>>>>> ---
>>>>>>> northd/northd.c | 79 ++++++++++++++++++++++++++++--
>>>>>>> northd/ovn-northd.8.xml | 13 ++++-
>>>>>>> tests/ovn-northd.at | 106 +++++++++++++++++++++++++++++++++++-----
>>>>>>> tests/ovn.at | 8 +--
>>>>>>> tests/system-ovn.at | 61 ++++++++++++++++++++---
>>>>>>> 5 files changed, 238 insertions(+), 29 deletions(-)
>>>>>> The "237: system-ovn-kmod.at:1031 Load Balancer LS hairpin IPv4 UDP -
>>>>>> larger than MTU" test is failing with this patch applied, see CI run
>>>>>> here:
>>>>>> https://github.com/ovsrobot/ovn/actions/runs/13928303850/job/38978852134#step:9:4712
>>>>>>
>>>>>> Something is weird because the test in question doesn't use stateless
>>>>>> ACLs at all. So there must be a bug that's introduced by your patch.
>>>>>>
>>>>>> I did a cursory review of the rest of the patch and shared some more
>>>>>> comments below.
>>>>>>
>>>>>> However, I'd like to hear from Venu/Han/Numan first, if possible, before
>>>>>> moving towards a v3.
>>>>>>
>>>>>>> diff --git a/northd/northd.c b/northd/northd.c
>>>>>>> index 1d3e132d4..fab420784 100644
>>>>>>> --- a/northd/northd.c
>>>>>>> +++ b/northd/northd.c
>>>>>>> @@ -5894,7 +5894,11 @@ build_stateless_filter(const struct ovn_datapath
>>>>>>> *od,
>>>>>>> action,
>>>>>>> &acl->header_,
>>>>>>> lflow_ref);
>>>>>>> - } else {
>>>>>>> + } else if (!od->nbs->n_load_balancer){
>>>>>>> + /* For cases when we have statefull ACLs but no load
>>>>>>> + balancer configured on logical switch - we should
>>>>>>> + completely bypass conntrack on egress, otherwise
>>>>>>> + it is necessary to check the balanced traffic. */
>>>>>>> ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL,
>>>>>>> acl->priority + OVN_ACL_PRI_OFFSET,
>>>>>>> acl->match,
>>>>>>> @@ -7355,6 +7359,49 @@ choose_max_acl_tier(const struct
>>>>>>> ls_stateful_record *ls_stateful_rec,
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>> +/* In the case of stateless ACLs and load balancers, all traffic
>>>>>>> + * is passed to conntrack during egress pipeline. Conntrack will
>>>>>>> + * mark the packets as 'invalid,' but since stateless systems do
>>>>>>> + * not rely on conntrack state, these invalid packets will be
>>>>>>> + * discarded during processing on the hypervisor level.
>>>>>>> + */
>>>>>>> +static bool
>>>>>>> +stateless_inv_match(const struct ls_stateful_record *ls_stateful_rec,
>>>>>>> + const struct ovn_datapath *od,
>>>>>>> + const struct ls_port_group_table *ls_port_groups)
>>>>>>> +{
>>>>>>> + bool inv_match = true;
>>>>>> There's no need for this variable, we always return after setting its
>>>>>> value.
>>>>>>
>>>>>>> +
>>>>>>> + if (ls_stateful_rec->has_lb_vip) {
>>>>>>> + for (size_t i = 0; i < od->nbs->n_acls; i++) {
>>>>>>> + const struct nbrec_acl *acl = od->nbs->acls[i];
>>>>>>> + if (!strcmp(acl->action, "allow-stateless")) {
>>>>>>> + inv_match = false;
>>>>>>> + return inv_match;
>>>>>> Just return false here.
>>>>>>
>>>>>> However, a cleaner and slightly more efficient way of doing this would
>>>>>> be to add a "has_stateless_acl" field to "struct ls_stateful_record" and
>>>>>> populate it in ls_stateful_record_set_acl_flags().
>>>>>>
>>>>>>> + }
>>>>>>> + }
>>>>>>> +
>>>>>>> + const struct ls_port_group *ls_pg =
>>>>>>> + ls_port_group_table_find(ls_port_groups, od->nbs);
>>>>>>> + if (!ls_pg) {
>>>>>>> + return inv_match;
>>>>>>> + }
>>>>>>> +
>>>>>>> + const struct ls_port_group_record *ls_pg_rec;
>>>>>>> + HMAP_FOR_EACH (ls_pg_rec, key_node, &ls_pg->nb_pgs) {
>>>>>>> + for (size_t i = 0; i < ls_pg_rec->nb_pg->n_acls; i++) {
>>>>>>> + const struct nbrec_acl *acl =
>>>>>>> ls_pg_rec->nb_pg->acls[i];
>>>>>>> + if (!strcmp(acl->action, "allow-stateless")) {
>>>>>>> + inv_match = false;
>>>>>>> + return inv_match;
>>>>>>> + }
>>>>>>> + }
>>>>>>> + }
>>>>>>> + }
>>>>>>> +
>>>>>>> + return inv_match;
>>>>>>> +}
>>>>>>> +
>>>>>>> static void
>>>>>>> build_acls(const struct ls_stateful_record *ls_stateful_rec,
>>>>>>> const struct ovn_datapath *od,
>>>>>>> @@ -7457,8 +7504,15 @@ build_acls(const struct ls_stateful_record
>>>>>>> *ls_stateful_rec,
>>>>>>> *
>>>>>>> * This is enforced at a higher priority than ACLs can be
>>>>>>> defined. */
>>>>>>> ds_clear(&match);
>>>>>>> - ds_put_format(&match, "%s(ct.est && ct.rpl && ct_mark.blocked
>>>>>>> == 1)",
>>>>>>> - use_ct_inv_match ? "ct.inv || " : "");
>>>>>>> +
>>>>>>> + if (use_ct_inv_match && stateless_inv_match(ls_stateful_rec,
>>>>>>> + od,
>>>>>>> ls_port_groups)) {
>>>>>>> + ds_put_cstr(&match, "ct.inv || (ct.est && ct.rpl && "
>>>>>>> + "ct_mark.blocked == 1)");
>>>>>>> + } else {
>>>>>>> + ds_put_cstr(&match, "ct.est && ct.rpl && ct_mark.blocked
>>>>>>> == 1");
>>>>>>> + }
>>>>>> If we go this way we should probably also update the "use_ct_inv_match"
>>>>>> documentation in ovn-nb.xml.
>>>>>>
>>>>>> This is also a very noticeable behavior change, we should at least
>>>>>> mention it in NEWS if we accept this patch.
>>>>>>
>>>>>>> +
>>>>>>> ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_EVAL, UINT16_MAX -
>>>>>>> 3,
>>>>>>> ds_cstr(&match), REGBIT_ACL_VERDICT_DROP " = 1;
>>>>>>> next;",
>>>>>>> lflow_ref);
>>>>>>> @@ -7752,8 +7806,7 @@ build_lb_rules_pre_stateful(struct lflow_table
>>>>>>> *lflows,
>>>>>>> }
>>>>>>> ds_put_cstr(action, "ct_lb_mark;");
>>>>>>>
>>>>>>> - ds_put_format(match, REGBIT_CONNTRACK_NAT" == 1 && %s.dst ==
>>>>>>> %s",
>>>>>>> - ip_match, lb_vip->vip_str);
>>>>>>> + ds_put_format(match, "%s.dst == %s", ip_match,
>>>>>>> lb_vip->vip_str);
>>>>>>> if (lb_vip->port_str) {
>>>>>>> ds_put_format(match, " && %s.dst == %s", lb->proto,
>>>>>>> lb_vip->port_str);
>>>>>>> @@ -7763,6 +7816,22 @@ build_lb_rules_pre_stateful(struct lflow_table
>>>>>>> *lflows,
>>>>>>> lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths),
>>>>>>> S_SWITCH_IN_PRE_STATEFUL, 120, ds_cstr(match),
>>>>>>> ds_cstr(action),
>>>>>>> &lb->nlb->header_, lb_dps->lflow_ref);
>>>>>>> +
>>>>>>> + /* Pass all VIP traffic to the conntrack to support
>>>>>>> + load balancers in the case of stateless acl. */
>>>>>>> + if (lb_vip->port_str && lb->proto) {
>>>>>> lb->proto is always non-NULL, it can be omitted from the condition.
>>>>>>
>>>>>> What about load balancers that have a port specified?
>>>>>>
>>>>>>> + ds_clear(action);
>>>>>>> + ds_clear(match);
>>>>>>> +
>>>>>>> + ds_put_cstr(action, "ct_lb_mark;");
>>>>>>> + ds_put_format(match, "%s && %s.dst == %s",
>>>>>>> + lb->proto, ip_match, lb_vip->vip_str);
>>>>>> Why not restrict this flow to only match traffic destined to the LB
>>>>>> port, if the LB has a port configured?
>>>>>>
>>>>>>> +
>>>>>>> + ovn_lflow_add_with_dp_group(
>>>>>>> + lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths),
>>>>>>> + S_SWITCH_IN_PRE_STATEFUL, 115, ds_cstr(match),
>>>>>>> ds_cstr(action),
>>>>>>> + &lb->nlb->header_, lb_dps->lflow_ref);
>>>>>>> + }
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
>>>>>>> index 155ba8a49..2719ac77b 100644
>>>>>>> --- a/northd/ovn-northd.8.xml
>>>>>>> +++ b/northd/ovn-northd.8.xml
>>>>>>> @@ -507,7 +507,7 @@
>>>>>>> <ref table="Logical_Switch_Port" db="OVN_Northbound"/>.
>>>>>>> Multicast, IPv6
>>>>>>> Neighbor Discovery and MLD traffic also skips stateful ACLs. For
>>>>>>> "allow-stateless" ACLs, a flow is added to bypass setting the
>>>>>>> hint for
>>>>>>> - connection tracker processing when there are stateful ACLs or LB
>>>>>>> rules;
>>>>>>> + connection tracker processing when there are stateful ACLs
>>>>>>> without LB;
>>>>>>> <code>REGBIT_ACL_STATELESS</code> is set for traffic matching
>>>>>>> stateless
>>>>>>> ACL flows.
>>>>>>> </p>
>>>>>>> @@ -624,6 +624,14 @@
>>>>>>> <code>ct_lb_mark;</code> action.
>>>>>>> </li>
>>>>>>>
>>>>>>> + <li>
>>>>>>> + A priority-115 flow sends all packet directed to VIP to
>>>>>>> connection
>>>>>>> + tracker. Packets that match this rule would still be subject
>>>>>>> to
>>>>>>> + connection tracking via lower-priority rules in the absence of
>>>>>>> + stateless ACLs. However, with stateless ACLs in place, this
>>>>>>> rule
>>>>>>> + enables load balancing when the client balances traffic to
>>>>>>> itself.
>>>>>>> + </li>
>>>>>>> +
>>>>>>> <li>
>>>>>>> A priority-100 flow sends the packets to connection tracker
>>>>>>> based
>>>>>>> on a hint provided by the previous tables
>>>>>>> @@ -770,7 +778,8 @@
>>>>>>> </li>
>>>>>>> <li>
>>>>>>> <code>allow-stateless</code> ACLs translate into logical
>>>>>>> flows that set
>>>>>>> - the allow bit and advance to the next table.
>>>>>>> + the allow bit and advance to the next table. In case of load
>>>>>>> balancers
>>>>>>> + with stateless ACLs in this table we allow ct.inv packets go
>>>>>>> further.
>>>>>>> </li>
>>>>>>> <li>
>>>>>>> <code>reject</code> ACLs translate into logical flows with
>>>>>>> that set the
>>>>>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>>>>>>> index 419130aa8..c6540f90d 100644
>>>>>>> --- a/tests/ovn-northd.at
>>>>>>> +++ b/tests/ovn-northd.at
>>>>>>> @@ -3747,10 +3747,18 @@ for direction in from to; do
>>>>>>> done
>>>>>>> check ovn-nbctl --wait=sb sync
>>>>>>>
>>>>>>> -# TCP packets should not go to conntrack for load balancing.
>>>>>>> +# TCP packets should go to conntrack for load balancing.
>>>>>>> flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
>>>>>>> AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls "${flow}"],
>>>>>>> [0], [dnl
>>>>>>> -output("lsp2");
>>>>>>> +ct_lb_mark {
>>>>>>> + ct_lb_mark {
>>>>>>> + reg0[[6]] = 0;
>>>>>>> + reg0[[12]] = 0;
>>>>>>> + ct_lb_mark /* default (use --ct to customize) */ {
>>>>>>> + output("lsp2");
>>>>>>> + };
>>>>>>> + };
>>>>>>> +};
>>>>>>> ])
>>>>>>>
>>>>>>> # UDP packets still go to conntrack.
>>>>>>> @@ -3883,10 +3891,18 @@ for direction in from to; do
>>>>>>> done
>>>>>>> check ovn-nbctl --wait=sb sync
>>>>>>>
>>>>>>> -# TCP packets should not go to conntrack for load balancing.
>>>>>>> +# TCP packets should go to conntrack for load balancing.
>>>>>>> flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
>>>>>>> AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls "${flow}"],
>>>>>>> [0], [dnl
>>>>>>> -output("lsp2");
>>>>>>> +ct_lb_mark {
>>>>>>> + ct_lb_mark {
>>>>>>> + reg0[[6]] = 0;
>>>>>>> + reg0[[12]] = 0;
>>>>>>> + ct_lb_mark /* default (use --ct to customize) */ {
>>>>>>> + output("lsp2");
>>>>>>> + };
>>>>>>> + };
>>>>>>> +};
>>>>>>> ])
>>>>>>>
>>>>>>> # UDP packets still go to conntrack.
>>>>>>> @@ -4770,8 +4786,10 @@ check_stateful_flows() {
>>>>>>> table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] ==
>>>>>>> 1), action=(ct_next;)
>>>>>>> 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.10 && tcp.dst == 80), action=(reg4 = 10.0.0.10;
>>>>>>> reg2[[0..15]] = 80; ct_lb_mark;)
>>>>>>> - 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;)
>>>>>>> + table=??(ls_in_pre_stateful ), priority=115 , match=(tcp && ip4.dst
>>>>>>> == 10.0.0.10), action=(ct_lb_mark;)
>>>>>>> + table=??(ls_in_pre_stateful ), priority=115 , match=(tcp && ip4.dst
>>>>>>> == 10.0.0.20), action=(ct_lb_mark;)
>>>>>>> + table=??(ls_in_pre_stateful ), priority=120 , match=(ip4.dst ==
>>>>>>> 10.0.0.10 && tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] =
>>>>>>> 80; ct_lb_mark;)
>>>>>>> + table=??(ls_in_pre_stateful ), priority=120 , match=(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_lb " sw0flows | ovn_strip_lflows], [0], [dnl
>>>>>>> @@ -5065,16 +5083,16 @@ AT_CAPTURE_FILE([sw0flows])
>>>>>>>
>>>>>>> AT_CHECK([grep -w "ls_in_acl_eval" sw0flows | grep 6553 |
>>>>>>> ovn_strip_lflows], [0], [dnl
>>>>>>> table=??(ls_in_acl_eval ), priority=65532, match=(!ct.est &&
>>>>>>> ct.rel && !ct.new && ct_mark.blocked == 0), action=(reg0[[17]] = 1;
>>>>>>> reg8[[16]] = 1; ct_commit_nat;)
>>>>>>> - table=??(ls_in_acl_eval ), priority=65532, match=((ct.est &&
>>>>>>> ct.rpl && ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;)
>>>>>>> table=??(ls_in_acl_eval ), priority=65532, match=(ct.est &&
>>>>>>> !ct.rel && !ct.new && ct.rpl && ct_mark.blocked == 0),
>>>>>>> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; reg8[[16]] = 1;
>>>>>>> next;)
>>>>>>> + table=??(ls_in_acl_eval ), priority=65532, match=(ct.est &&
>>>>>>> ct.rpl && ct_mark.blocked == 1), action=(reg8[[17]] = 1; next;)
>>>>>>> table=??(ls_in_acl_eval ), priority=65532, match=(ct.est &&
>>>>>>> ct_mark.allow_established == 1), action=(reg0[[21]] = 1; reg8[[16]] =
>>>>>>> 1; next;)
>>>>>>> table=??(ls_in_acl_eval ), priority=65532, match=(nd || nd_ra
>>>>>>> || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;)
>>>>>>> ])
>>>>>>>
>>>>>>> AT_CHECK([grep -w "ls_out_acl_eval" sw0flows | grep 6553 |
>>>>>>> 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[[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.new && ct.rpl && ct_mark.blocked == 0),
>>>>>>> action=(reg8[[16]] = 1; next;)
>>>>>>> + 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_mark.allow_established == 1), action=(reg8[[16]] = 1; next;)
>>>>>>> table=??(ls_out_acl_eval ), priority=65532, match=(nd || nd_ra
>>>>>>> || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;)
>>>>>>> ])
>>>>>>> @@ -14333,7 +14351,7 @@ ovn-sbctl dump-flows s1 > s1flows
>>>>>>> AT_CAPTURE_FILE([s1flows])
>>>>>>>
>>>>>>> AT_CHECK([grep "ls_in_pre_stateful" s1flows | ovn_strip_lflows | grep
>>>>>>> "30.0.0.1"], [0], [dnl
>>>>>>> - table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1
>>>>>>> && ip4.dst == 30.0.0.1), action=(reg4 = 30.0.0.1; ct_lb_mark;)
>>>>>>> + table=??(ls_in_pre_stateful ), priority=120 , match=(ip4.dst ==
>>>>>>> 30.0.0.1), action=(reg4 = 30.0.0.1; ct_lb_mark;)
>>>>>>> ])
>>>>>>> AT_CHECK([grep "ls_in_lb" s1flows | ovn_strip_lflows | grep
>>>>>>> "30.0.0.1"], [0], [dnl
>>>>>>> table=??(ls_in_lb ), priority=110 , match=(ct.new &&
>>>>>>> ip4.dst == 30.0.0.1), action=(reg4 = 30.0.0.1;
>>>>>>> ct_lb_mark(backends=172.16.0.103,172.16.0.102,172.16.0.101);)
>>>>>>> @@ -14447,7 +14465,7 @@ ovn-sbctl dump-flows s1 > s1flows
>>>>>>> AT_CAPTURE_FILE([s1flows])
>>>>>>>
>>>>>>> AT_CHECK([grep "ls_in_pre_stateful" s1flows | ovn_strip_lflows | grep
>>>>>>> "2001:db8:cccc::1"], [0], [dnl
>>>>>>> - table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1
>>>>>>> && ip6.dst == 2001:db8:cccc::1), action=(xxreg1 = 2001:db8:cccc::1;
>>>>>>> ct_lb_mark;)
>>>>>>> + table=??(ls_in_pre_stateful ), priority=120 , match=(ip6.dst ==
>>>>>>> 2001:db8:cccc::1), action=(xxreg1 = 2001:db8:cccc::1; ct_lb_mark;)
>>>>>>> ])
>>>>>>> AT_CHECK([grep "ls_in_lb" s1flows | ovn_strip_lflows | grep
>>>>>>> "2001:db8:cccc::1"], [0], [dnl
>>>>>>> table=??(ls_in_lb ), priority=110 , match=(ct.new &&
>>>>>>> ip6.dst == 2001:db8:cccc::1), action=(xxreg1 = 2001:db8:cccc::1;
>>>>>>> ct_lb_mark(backends=2001:db8:aaaa:3::103,2001:db8:aaaa:3::102,2001:db8:aaaa:3::101);)
>>>>>>> @@ -14558,7 +14576,7 @@ ovn-sbctl dump-flows s1 > s1flows
>>>>>>> AT_CAPTURE_FILE([s1flows])
>>>>>>>
>>>>>>> AT_CHECK([grep "ls_in_pre_stateful" s1flows | ovn_strip_lflows | grep
>>>>>>> "30.0.0.1"], [0], [dnl
>>>>>>> - table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1
>>>>>>> && ip4.dst == 30.0.0.1), action=(reg4 = 30.0.0.1; ct_lb_mark;)
>>>>>>> + table=??(ls_in_pre_stateful ), priority=120 , match=(ip4.dst ==
>>>>>>> 30.0.0.1), action=(reg4 = 30.0.0.1; ct_lb_mark;)
>>>>>>> ])
>>>>>>> AT_CHECK([grep "ls_in_lb" s1flows | ovn_strip_lflows | grep
>>>>>>> "30.0.0.1"], [0], [dnl
>>>>>>> table=??(ls_in_lb ), priority=110 , match=(ct.new &&
>>>>>>> ip4.dst == 30.0.0.1), action=(reg4 = 30.0.0.1;
>>>>>>> ct_lb_mark(backends=172.16.0.103,172.16.0.102,172.16.0.101);)
>>>>>>> @@ -16753,3 +16771,69 @@ check grep -q "Bad configuration: The peer of
>>>>>>> the switch port 'ls-lrp1' (LRP pee
>>>>>>>
>>>>>>> AT_CLEANUP
>>>>>>> ])
>>>>>>> +
>>>>>>> +OVN_FOR_EACH_NORTHD_NO_HV([
>>>>>>> +AT_SETUP([Load Balancers with stateless ACLs])
>>>>>>> +ovn_start ovn-northd
>>>>>>> +
>>>>>>> +AS_BOX([create logical switches and ports])
>>>>>>> +check ovn-nbctl ls-add sw0
>>>>>>> +check ovn-nbctl --wait=sb lsp-add sw0 sw0-p1 -- lsp-set-addresses
>>>>>>> sw0-p1 \
>>>>>>> +"00:00:00:00:00:02 10.0.0.2"
>>>>>>> +
>>>>>>> +check ovn-nbctl --wait=sb lsp-add sw0 sw0-p2 -- lsp-set-addresses
>>>>>>> sw0-p2 \
>>>>>>> +"00:00:00:00:00:03 10.0.0.3"
>>>>>>> +
>>>>>>> +AS_BOX([create stateless ACLs])
>>>>>>> +check ovn-nbctl acl-add sw0 from-lport 1001 "ip" allow-stateless
>>>>>>> +check ovn-nbctl acl-add sw0 to-lport 1001 "ip" allow-stateless
>>>>>>> +
>>>>>>> +AS_BOX([create statefull ACLs])
>>>>>>> +# check if allow-stateless acls have higher priority we skip conntrack.
>>>>>>> +check ovn-nbctl acl-add sw0 from-lport 1000 "ip" allow-related
>>>>>>> +check ovn-nbctl acl-add sw0 to-lport 1000 "ip" allow-related
>>>>>>> +
>>>>>>> +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_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.inv && ct_mark.blocked == 0),
>>>>>>> action=(reg8[[16]] = 1; ct_commit_nat;)
>>>>>>> + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est &&
>>>>>>> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
>>>>>>> action=(reg8[[16]] = 1; next;)
>>>>>>> + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est &&
>>>>>>> ct_mark.allow_established == 1), action=(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;)
>>>>>>> +])
>>>>>>> +
>>>>>>> +AS_BOX([create Load Balancer])
>>>>>>> +check ovn-nbctl lb-add lb1 10.0.0.4:80 10.0.0.2:80,10.0.0.3:80
>>>>>>> +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;)
>>>>>>> +])
>>>>>>> +
>>>>>>> +# we do not match conntrack invalide 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.inv && ct_mark.blocked == 0),
>>>>>>> action=(reg8[[16]] = 1; ct_commit_nat;)
>>>>>>> + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est &&
>>>>>>> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
>>>>>>> action=(reg8[[16]] = 1; next;)
>>>>>>> + 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_mark.allow_established == 1), action=(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_CLEANUP
>>>>>>> +])
>>>>>>> +
>>>>>>> diff --git a/tests/ovn.at b/tests/ovn.at
>>>>>>> index afde2576f..3e299711b 100644
>>>>>>> --- a/tests/ovn.at
>>>>>>> +++ b/tests/ovn.at
>>>>>>> @@ -26345,7 +26345,7 @@ OVS_WAIT_FOR_OUTPUT(
>>>>>>> [ovn-sbctl dump-flows > sbflows
>>>>>>> ovn-sbctl dump-flows sw0 | grep ct_lb_mark | grep priority=120 |
>>>>>>> sed 's/table=..//'], 0,
>>>>>>> [dnl
>>>>>>> - (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 &&
>>>>>>> ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg4 = 10.0.0.10;
>>>>>>> reg2[[0..15]] = 80; ct_lb_mark;)
>>>>>>> + (ls_in_pre_stateful ), priority=120 , match=(ip4.dst == 10.0.0.10
>>>>>>> && tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] = 80;
>>>>>>> ct_lb_mark;)
>>>>>>> (ls_in_lb ), priority=120 , match=(ct.new && ip4.dst ==
>>>>>>> 10.0.0.10 && tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] =
>>>>>>> 80; ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80;
>>>>>>> hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");)
>>>>>>> ])
>>>>>>>
>>>>>>> @@ -26390,7 +26390,7 @@ AT_CHECK(
>>>>>>> [grep "ip4.dst == 10.0.0.10 && tcp.dst == 80" sbflows3 | grep
>>>>>>> priority=120 |\
>>>>>>> ovn_strip_lflows], [0], [dnl
>>>>>>> table=??(ls_in_lb ), priority=120 , match=(ct.new &&
>>>>>>> ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(drop;)
>>>>>>> - table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1
>>>>>>> && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg4 = 10.0.0.10;
>>>>>>> reg2[[0..15]] = 80; ct_lb_mark;)
>>>>>>> + table=??(ls_in_pre_stateful ), priority=120 , match=(ip4.dst ==
>>>>>>> 10.0.0.10 && tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] =
>>>>>>> 80; ct_lb_mark;)
>>>>>>> ])
>>>>>>>
>>>>>>> AT_CAPTURE_FILE([sbflows4])
>>>>>>> @@ -26551,7 +26551,7 @@ OVS_WAIT_FOR_OUTPUT(
>>>>>>> [ovn-sbctl dump-flows > sbflows
>>>>>>> ovn-sbctl dump-flows sw0 | grep ct_lb_mark | grep priority=120 |
>>>>>>> sed 's/table=..//'], 0,
>>>>>>> [dnl
>>>>>>> - (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 &&
>>>>>>> ip6.dst == 2002::a && tcp.dst == 80), action=(xxreg1 = 2002::a;
>>>>>>> reg2[[0..15]] = 80; ct_lb_mark;)
>>>>>>> + (ls_in_pre_stateful ), priority=120 , match=(ip6.dst == 2002::a &&
>>>>>>> tcp.dst == 80), action=(xxreg1 = 2002::a; reg2[[0..15]] = 80;
>>>>>>> ct_lb_mark;)
>>>>>>> (ls_in_lb ), priority=120 , match=(ct.new && ip6.dst ==
>>>>>>> 2002::a && tcp.dst == 80), action=(xxreg1 = 2002::a; reg2[[0..15]] =
>>>>>>> 80; ct_lb_mark(backends=[[2001::3]]:80,[[2002::3]]:80;
>>>>>>> hash_fields="ipv6_dst,ipv6_src,tcp_dst,tcp_src");)
>>>>>>> ])
>>>>>>>
>>>>>>> @@ -26595,7 +26595,7 @@ AT_CHECK(
>>>>>>> [grep "ip6.dst == 2002::a && tcp.dst == 80" sbflows3 | grep
>>>>>>> priority=120 |\
>>>>>>> ovn_strip_lflows], [0], [dnl
>>>>>>> table=??(ls_in_lb ), priority=120 , match=(ct.new &&
>>>>>>> ip6.dst == 2002::a && tcp.dst == 80), action=(drop;)
>>>>>>> - table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1
>>>>>>> && ip6.dst == 2002::a && tcp.dst == 80), action=(xxreg1 = 2002::a;
>>>>>>> reg2[[0..15]] = 80; ct_lb_mark;)
>>>>>>> + table=??(ls_in_pre_stateful ), priority=120 , match=(ip6.dst ==
>>>>>>> 2002::a && tcp.dst == 80), action=(xxreg1 = 2002::a; reg2[[0..15]] =
>>>>>>> 80; ct_lb_mark;)
>>>>>>> ])
>>>>>>>
>>>>>>> AT_CAPTURE_FILE([sbflows4])
>>>>>>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
>>>>>>> index dd13575e4..658e3eb26 100644
>>>>>>> --- a/tests/system-ovn.at
>>>>>>> +++ b/tests/system-ovn.at
>>>>>>> @@ -10087,6 +10087,28 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'],
>>>>>>> [0], [dnl
>>>>>>>
>>>>>>> AT_CHECK([ovs-appctl dpctl/flush-conntrack])
>>>>>>>
>>>>>>> +# del acls
>>>>>>> +check ovn-nbctl acl-del foo
>>>>>>> +
>>>>>>> +# add statefull acl
>>>>>> Comments should be sentences: start with a capital letter and end with a
>>>>>> period. This applies to most comments added by this patch.
>>>>>>
>>>>>> Also, I might be wrong but I think the correct version is "stateful"
>>>>>> instead of "statefull". This remark also applies to multiple places in
>>>>>> this patch.
>>>>>>
>>>>>>> +check ovn-nbctl acl-add foo from-lport 1 1 allow-related
>>>>>>> +check ovn-nbctl --wait=hv acl-add foo to-lport 1 1 allow-related
>>>>>>> +
>>>>>>> +# add stateless acl
>>>>>>> +check ovn-nbctl acl-add foo from-lport 2 1 allow-stateless
>>>>>>> +check ovn-nbctl --wait=hv acl-add foo to-lport 2 1 allow-stateless
>>>>>>> +
>>>>>>> +AT_CHECK([ip netns exec foo1 wget 192.168.2.2 -t 1 -T 1], [0],
>>>>>>> [ignore], [ignore])
>>>>>>> +
>>>>>>> +# check conntrack zone has no entry
>>>>>>> +AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_id | \
>>>>>>> +FORMAT_CT(192.168.2.2) | \
>>>>>>> +sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
>>>>>>> +])
>>>>>>> +
>>>>>>> +AT_CHECK([ovs-appctl dpctl/flush-conntrack])
>>>>>>> +
>>>>>>> +# when lb configued: pass conntrack for vip traffic.
>>>>>> Typo: configued
>>>>>>
>>>>>>> # add lb back
>>>>>>> check ovn-nbctl ls-lb-add foo lb1
>>>>>>>
>>>>>>> @@ -10096,13 +10118,14 @@ check ovn-nbctl --wait=hv sync
>>>>>>> OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-groups br-int | \
>>>>>>> grep 'nat(dst=192.168.2.2:80)'])
>>>>>>>
>>>>>>> -# should not dnat so will not be able to connect
>>>>>>> -AT_CHECK([ip netns exec foo1 curl 30.30.30.30 --retry 3 --max-time 1],
>>>>>>> [28], [ignore], [ignore])
>>>>>>> +# now check with VIP
>>>>>>> +AT_CHECK([ip netns exec foo1 wget 30.30.30.30 -t 3 -T 1], [0],
>>>>>>> [ignore], [ignore])
>>>>>>>
>>>>>>> -# check conntrack zone has no tcp entry
>>>>>>> +# check conntrack zone has tcp entry
>>>>>>> AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_id | \
>>>>>>> FORMAT_CT(30.30.30.30) | \
>>>>>>> sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
>>>>>>> +tcp,orig=(src=192.168.1.2,dst=30.30.30.30,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>)
>>>>>>> ])
>>>>>>>
>>>>>>> AT_CHECK([ovs-appctl dpctl/flush-conntrack])
>>>>>>> @@ -10217,6 +10240,29 @@ AT_CHECK([ovs-appctl dpctl/flush-conntrack])
>>>>>>> # remove lb
>>>>>>> check ovn-nbctl ls-lb-del foo lb1
>>>>>>>
>>>>>>> +# del acls
>>>>>>> +check ovn-nbctl acl-del foo
>>>>>>> +
>>>>>>> +# add statefull acl
>>>>>>> +check ovn-nbctl acl-add foo from-lport 1 1 allow-related
>>>>>>> +check ovn-nbctl --wait=hv acl-add foo to-lport 1 1 allow-related
>>>>>>> +
>>>>>>> +# add stateless acl
>>>>>>> +check ovn-nbctl acl-add foo from-lport 2 1 allow-stateless
>>>>>>> +check ovn-nbctl --wait=hv acl-add foo to-lport 2 1 allow-stateless
>>>>>>> +
>>>>>>> +AT_CHECK([ip netns exec foo1 wget http://[[fd12::2]] -t 1 -T 1], [0],
>>>>>>> [ignore], [ignore])
>>>>>>> +
>>>>>>> +# check conntrack zone has no tcp entry
>>>>>>> +AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_id | \
>>>>>>> +FORMAT_CT(fd12::2) | grep -v fe80 | \
>>>>>>> +sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
>>>>>>> +])
>>>>>>> +
>>>>>>> +AT_CHECK([ovs-appctl dpctl/flush-conntrack])
>>>>>>> +
>>>>>>> +check ovn-nbctl acl-del foo
>>>>>>> +
>>>>>>> # add stateless acl
>>>>>>> check ovn-nbctl acl-add foo from-lport 1 1 allow-stateless
>>>>>>> check ovn-nbctl --wait=hv acl-add foo to-lport 1 1 allow-stateless
>>>>>>> @@ -10240,13 +10286,14 @@ check ovn-nbctl --wait=hv sync
>>>>>>> OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-groups br-int | \
>>>>>>> grep 'nat(dst=\[[fd12::2\]]:80)'])
>>>>>>>
>>>>>>> -# should not dnat so will not be able to connect
>>>>>>> -AT_CHECK([ip netns exec foo1 curl http://[[fd30::2]] --retry 3
>>>>>>> --max-time 1], [28], [ignore], [ignore])
>>>>>>> -#
>>>>>>> -# check conntrack zone has no tcp entry
>>>>>>> +# now check with VIP
>>>>>>> +AT_CHECK([ip netns exec foo1 wget http://[[fd30::2]] -t 3 -T 1],
>>>>>>> [0], [ignore], [ignore])
>>>>>>> +
>>>>>>> +# check conntrack zone has tcp entry
>>>>>>> AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_id | \
>>>>>>> FORMAT_CT(fd30::2) | grep -v fe80 | \
>>>>>>> sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
>>>>>>> +tcp,orig=(src=fd11::2,dst=fd30::2,sport=<cleared>,dport=<cleared>),reply=(src=fd12::2,dst=fd11::2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>)
>>>>>>> ])
>>>>>>>
>>>>>>> AT_CHECK([ovs-appctl dpctl/flush-conntrack])
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> regards,
>>>>>>> Alexandra.
> Regards,
> Dumitru
>
--
regards,
Alexandra.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
