[ovs-dev] [PATCH v4 ovn] ovn-northd: introduce new allow-stateless ACL verb

Ihar Hrachyshka ihrachys at redhat.com
Mon Apr 26 23:54:17 UTC 2021


Thanks Dumitru,

all the comments applied in v6 now up for review.

Thanks!
Ihar

On Mon, Apr 26, 2021 at 7:45 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> On 4/14/21 12:39 AM, Ihar Hrachyshka wrote:
> > For allow-stateless ACLs, bypass connection tracking by avoiding
> > setting ct hints for matching traffic. Avoid sending all traffic to ct
> > when a stateful ACL is present.
> >
> > ===
> >
> > Reusing an existing 'allow' verb for stateless matching would have its
> > drawbacks, specifically, when it comes to backwards incompatibility of
> > the new behavior with existing environments. When using "allow" ACLs
> > in mixed allow/allow-related environment, we still commit "allow"
> > traffic to conntrack. This unnecessarily hits performance when mixed
> > ACL action types were used for the same datapath. This is why we
> > introduce a new action verb to describe stateless behavior.
> >
> > Another complexity to consider is the fact that with stateless
> > matching, one would not be able to rely on 'related' magic that
> > guarantees that reply traffic is passed through. Instead, the user
> > would have to accurately define matching rules both for request and
> > reply directions of a protocol session. Specifically, when allowing
> > ICMP for a specific peer host, one has to define 'allow-stateless'
> > rules that would match against ip.dst for request direction and ip.src
> > for reply direction. Other protocols and scenarios will require their
> > own fine grained matching approaches implemented by the user.
> >
> > ===
> >
> > For performance measurements, ovn-fake-multinode environment and qperf
> > were used. Performance measured between two virtual nodes, two ports
> > that belong to different LSs connected via router. Using qperf,
> > performance was measured for UDP, TCP, SCTP protocols (using
> > <proto>_lat and <proto>_bw tests). The qperf version used:
> > 0.4.9-16.fc31.x86_64.  Each test scenario was executed five times and
> > averages compared.
> >
> > Tests were executed with `allow-stateless` rules for the tested
> > protocol and `allow-related` for another protocol set for both ports,
> > both directions, e.g. for TCP scenario, the following ACLs were
> > defined:
> >
> > ovn-nbctl acl-add sw0 to-lport 100 tcp allow-stateless
> > ovn-nbctl acl-add sw0 from-lport 100 tcp allow-stateless
> > ovn-nbctl acl-add sw1 to-lport 100 tcp allow-stateless
> > ovn-nbctl acl-add sw1 from-lport 100 tcp allow-stateless
> >
> > ovn-nbctl acl-add sw0 to-lport 100 sctp allow-related
> > ovn-nbctl acl-add sw0 from-lport 100 sctp allow-related
> > ovn-nbctl acl-add sw1 to-lport 100 sctp allow-related
> > ovn-nbctl acl-add sw1 from-lport 100 sctp allow-related
> >
> > In this particular environment, improvement was seen in send_bw,
> > latency, and msg_rate measurements, where applicable, for all three
> > protocols under test.
> >
> > for UDP, send_bw: 293.6 MB/sec => 313.2 MB/sec (+6.68%)
> >          latency: 16 us => 14.08 us (-12%)
> >          msg_rate: 62.56 K/sec => 71.06 K/sec (+13.59%)
> >
> > for TCP, latency: 18.6 us => 14.88 us (-20%)
> >          msg_rate: 53.8 K/sec => 67.28 K/sec (+25.06%)
> >
> > for SCTP, latency: 21.98 us => 19.42 us (-11.65%)
> >           msg_rate: 45.58 K/sec => 51.54 K/sec (+13.08%)
> >
> > Interestingly, some performance improvement was also seen for the same
> > scenarios with no ACLs set at all, albeit significantly more
> > negligible.
> >
> > for UDP, send_bw: 320.0 MB/sec => 338.6 MB/sec (+5.81%)
> >          latency: 13.74 us => 12.88 us (-6.68%)
> >          msg_rate: 73.02 K/sec => 77.84 K/sec (+6.6%)
> >
> > for TCP, latency: 15.62 us => 14.26 us (-9.54%)
> >          msg_rate: 64.02 K/sec => 70.26 K/sec (+9.75%)
> >
> > for SCTP, latency: 19.56 us => 18.16 us (-7.16%)
> >           msg_rate: 51.16 K/sec => 55.12 K/sec (+7.74%)
> >
> > Comparable numbers can be captured with iperf. It may be useful to run
> > more tests in a more elaborate (bare metal) environment.
> >
> > ===
> >
> > The patch takes inspiration from a now abandoned patch:
> >
> > "ovn-northd: Support mixing stateless/stateful ACLs with
> > Stateless_Filter." by Dumitru Ceara.
> >
> > The original patch assumed CMS doesn't require full flexibility of
> > matching rules for stateless matching (for example, to be used by
> > OpenShift). But other CMS interfaces may require the same
> > customizability for stateless as well as stateful matching, like in
> > OpenStack Neutron API. Which is why this patch reuses existing ACL
> > object type to describe stateless rules.
> >
> > Signed-off-by: Ihar Hrachyshka <ihrachys at redhat.com>
> >
> > ---
>
> Hi Ihar,
>
> This looks good to me overall.  I have a few comments/questions below.
> With those addressed:
>
> Acked-by: Dumitru Ceara <dceara at redhat.com>
>
> Thanks,
> Dumitru
>
> >
> > v1: initial version.
> > v2: rebased after conflict.
> > v3: added ddlog implementation.
> > v3: apply stateless skip-hint-set rules to appropriate direction only.
> > v3: added more background as to implementation in commit message.
> > v3: test stateless scenarios with ddlog too.
> > v3: rebased after conflict.
> > v4: introduce a separate allow-stateless ACL match verb.
> > ---
> >  NEWS                    |   2 +
> >  northd/ovn-northd.8.xml |   9 +-
> >  northd/ovn-northd.c     |  74 +++++++++-
> >  northd/ovn_northd.dl    |  32 +++++
> >  ovn-nb.ovsschema        |   4 +-
> >  ovn-nb.xml              |   8 +-
> >  tests/ovn-northd.at     | 309 ++++++++++++++++++++++++++++++++++++++++
> >  utilities/ovn-nbctl.c   |   6 +-
> >  8 files changed, 430 insertions(+), 14 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index f84d236e4..76bf3e417 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -9,6 +9,8 @@ Post-v21.03.0
> >    - Introduce ovn-controller incremetal processing engine statistics
> >    - Introduced parallel processing in ovn-northd with the NB_Global config option
> >      'use_parallel_build' to enable it.  It is disabled by default.
> > +  - Introduce a new "allow-stateless" ACL verb to always bypass connection
> > +    tracking. The existing "allow" verb behavior is left intact.
> >
> >  OVN v21.03.0 - 12 Mar 2021
> >  -------------------------
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index 8197aa513..5ad70763f 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -419,7 +419,9 @@
> >        before eventually advancing to ingress table <code>ACLs</code>. If
> >        special ports such as route ports or localnet ports can't use ct(), a
> >        priority-110 flow is added to skip over stateful ACLs. IPv6 Neighbor
> > -      Discovery and MLD traffic also skips stateful ACLs.
> > +      Discovery and MLD traffic also skips stateful ACLs. For stateless "allow"
>
> s/stateless "allow"/"allow-stateless"/
>
> > +      ACLs, a flow is added to bypass setting the hint for connection tracker
> > +      processing.
> >      </p>
> >
> >      <p>
> > @@ -603,10 +605,7 @@
> >      <ul>
> >        <li>
> >          <code>allow</code> ACLs translate into logical flows with
> > -        the <code>next;</code> action.  If there are any stateful ACLs
> > -        on this datapath, then <code>allow</code> ACLs translate to
> > -        <code>ct_commit; next;</code> (which acts as a hint for the next tables
> > -        to commit the connection to conntrack),
> > +        the <code>next;</code> action.
>
> With the introduction of "allow-stateless" action and because "allow"
> ACLs behavior has not been changed, this diff is not accurate anymore,
> right?
>
> >        </li>
> >        <li>
> >          <code>allow-related</code> ACLs translate into logical
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 80ee05b8b..76b576964 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -4969,7 +4969,61 @@ skip_port_from_conntrack(struct ovn_datapath *od, struct ovn_port *op,
> >  }
> >
> >  static void
> > -build_pre_acls(struct ovn_datapath *od, struct hmap *lflows)
> > +build_stateless_filter(struct ovn_datapath *od,
> > +                       const struct nbrec_acl *acl,
> > +                       struct hmap *lflows)
> > +{
> > +    /* Stateless filters must be applied in both directions so that reply
> > +     * traffic bypasses conntrack too.
> > +     */
>
> Isn't this comment obsolete now?
>
> > +    if (!strcmp(acl->direction, "from-lport")) {
> > +        ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_ACL,
> > +                                acl->priority + OVN_ACL_PRI_OFFSET,
> > +                                acl->match,
> > +                                "next;",
> > +                                &acl->header_);
> > +    } else {
> > +        ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL,
> > +                                acl->priority + OVN_ACL_PRI_OFFSET,
> > +                                acl->match,
> > +                                "next;",
> > +                                &acl->header_);
> > +    }
> > +}
> > +
> > +static bool
> > +acl_is_stateless(const struct nbrec_acl *acl)
> > +{
> > +    return !strcmp(acl->action, "allow-stateless");
> > +}
>
> Nit: we use this new function only in build_stateless_filters() but in
> build_acl_log() and consider_acl() we explicitly do
> '!strcmp(acl->action, "allow-related")'.  I think we either need to add
> functions for all 5 possible ACL actions or just give up on
> acl_is_stateless() for now.  What do you think?
>
> > +
> > +static void
> > +build_stateless_filters(struct ovn_datapath *od, struct hmap *port_groups,
> > +                        struct hmap *lflows)
> > +{
> > +    for (size_t i = 0; i < od->nbs->n_acls; i++) {
> > +        const struct nbrec_acl *acl = od->nbs->acls[i];
> > +        if (acl_is_stateless(acl)) {
> > +            build_stateless_filter(od, acl, lflows);
> > +        }
> > +    }
> > +
> > +    struct ovn_port_group *pg;
> > +    HMAP_FOR_EACH (pg, key_node, port_groups) {
> > +        if (ovn_port_group_ls_find(pg, &od->nbs->header_.uuid)) {
> > +            for (size_t i = 0; i < pg->nb_pg->n_acls; i++) {
> > +                const struct nbrec_acl *acl = pg->nb_pg->acls[i];
> > +                if (acl_is_stateless(acl)) {
> > +                    build_stateless_filter(od, acl, lflows);
> > +                }
> > +            }
> > +        }
> > +    }
> > +}
> > +
> > +static void
> > +build_pre_acls(struct ovn_datapath *od, struct hmap *port_groups,
> > +               struct hmap *lflows)
> >  {
> >      /* Ingress and Egress Pre-ACL Table (Priority 0): Packets are
> >       * allowed by default. */
> > @@ -4997,6 +5051,8 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows)
> >                                       110, lflows);
> >          }
> >
> > +        build_stateless_filters(od, port_groups, lflows);
> > +
> >          /* Ingress and Egress Pre-ACL Table (Priority 110).
> >           *
> >           * Not to do conntrack on ND and ICMP destination
> > @@ -5364,7 +5420,8 @@ build_acl_log(struct ds *actions, const struct nbrec_acl *acl,
> >      } else if (!strcmp(acl->action, "reject")) {
> >          ds_put_cstr(actions, "verdict=reject, ");
> >      } else if (!strcmp(acl->action, "allow")
> > -        || !strcmp(acl->action, "allow-related")) {
> > +        || !strcmp(acl->action, "allow-related")
> > +        || !strcmp(acl->action, "allow-stateless")) {
> >          ds_put_cstr(actions, "verdict=allow, ");
> >      }
> >
> > @@ -5423,7 +5480,16 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
> >      bool ingress = !strcmp(acl->direction, "from-lport") ? true :false;
> >      enum ovn_stage stage = ingress ? S_SWITCH_IN_ACL : S_SWITCH_OUT_ACL;
> >
> > -    if (!strcmp(acl->action, "allow")
> > +    if (!strcmp(acl->action, "allow-stateless")) {
> > +        struct ds actions = DS_EMPTY_INITIALIZER;
> > +        build_acl_log(&actions, acl, meter_groups);
> > +        ds_put_cstr(&actions, "next;");
> > +        ovn_lflow_add_with_hint(lflows, od, stage,
> > +                                acl->priority + OVN_ACL_PRI_OFFSET,
> > +                                acl->match, ds_cstr(&actions),
> > +                                &acl->header_);
> > +        ds_destroy(&actions);
> > +    } else if (!strcmp(acl->action, "allow")
> >          || !strcmp(acl->action, "allow-related")) {
> >          /* If there are any stateful flows, we must even commit "allow"
> >           * actions.  This is because, while the initiater's
> > @@ -6823,7 +6889,7 @@ build_lswitch_lflows_pre_acl_and_acl(struct ovn_datapath *od,
> >          od->has_stateful_acl = ls_has_stateful_acl(od);
> >          od->has_lb_vip = ls_has_lb_vip(od);
> >
> > -        build_pre_acls(od, lflows);
> > +        build_pre_acls(od, port_groups, lflows);
> >          build_pre_lb(od, lflows, meter_groups, lbs);
> >          build_pre_stateful(od, lflows);
> >          build_acl_hints(od, lflows);
> > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> > index 4c02e2d5a..82f1eeafd 100644
> > --- a/northd/ovn_northd.dl
> > +++ b/northd/ovn_northd.dl
> > @@ -1815,6 +1815,28 @@ for (&Switch(.ls =ls)) {
> >           .external_ids     = map_empty())
> >  }
> >
> > +for (&SwitchACL(.sw = sw@&Switch{.ls = ls}, .acl = &acl, .has_fair_meter = fair_meter)) {
> > +    var has_stateful = sw.has_stateful_acl in
>
> We don't use the 'has_stateful' var, this line can be removed.
>
> > +    if (sw.has_stateful_acl) {
> > +        if (acl.action == "allow-stateless") {
> > +            if (acl.direction == "from-lport") {
> > +                Flow(.logical_datapath = ls._uuid,
> > +                     .stage            = s_SWITCH_IN_PRE_ACL(),
> > +                     .priority         = acl.priority + oVN_ACL_PRI_OFFSET(),
> > +                     .__match          = acl.__match,
> > +                     .actions          = "next;",
> > +                     .external_ids     = stage_hint(acl._uuid))
> > +            } else {
> > +                Flow(.logical_datapath = ls._uuid,
> > +                     .stage            = s_SWITCH_OUT_PRE_ACL(),
> > +                     .priority         = acl.priority + oVN_ACL_PRI_OFFSET(),
> > +                     .__match          = acl.__match,
> > +                     .actions          = "next;",
> > +                     .external_ids     = stage_hint(acl._uuid))
> > +            }
> > +        }
> > +    }
> > +}
> >
> >  /* If there are any stateful ACL rules in this datapath, we must
> >   * send all IP packets through the conntrack action, which handles
> > @@ -2159,6 +2181,9 @@ function build_acl_log(acl: nb::ACL, fair_meter: bool): string =
> >              "allow-related" -> {
> >                  strs.push("verdict=allow")
> >              },
> > +            "allow-stateless" -> {
> > +                strs.push("verdict=allow")
> > +            },
> >              _ -> ()
> >          };
> >          match (acl.meter) {
> > @@ -2537,6 +2562,13 @@ for (&SwitchACL(.sw = sw@&Switch{.ls = ls}, .acl = &acl, .has_fair_meter = fair_
> >                   .actions          = "${acl_log}next;",
> >                   .external_ids     = stage_hint)
> >          }
> > +    } else if (acl.action == "allow-stateless") {
> > +        Flow(.logical_datapath = ls._uuid,
> > +             .stage            = stage,
> > +             .priority         = acl.priority + oVN_ACL_PRI_OFFSET(),
> > +             .__match          = acl.__match,
> > +             .actions          = "${acl_log}next;",
> > +             .external_ids     = stage_hint)
> >      } else if (acl.action == "drop" or acl.action == "reject") {
> >          /* The implementation of "drop" differs if stateful ACLs are in
> >           * use for this datapath.  In that case, the actions differ
> > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> > index 29019809c..58932db14 100644
> > --- a/ovn-nb.ovsschema
> > +++ b/ovn-nb.ovsschema
> > @@ -1,7 +1,7 @@
> >  {
> >      "name": "OVN_Northbound",
> >      "version": "5.31.0",
> > -    "cksum": "2352750632 28701",
> > +    "cksum": "1284599142 28720",
> >      "tables": {
> >          "NB_Global": {
> >              "columns": {
> > @@ -221,7 +221,7 @@
> >                                              "enum": ["set", ["from-lport", "to-lport"]]}}},
> >                  "match": {"type": "string"},
> >                  "action": {"type": {"key": {"type": "string",
> > -                                            "enum": ["set", ["allow", "allow-related", "drop", "reject"]]}}},
> > +                                            "enum": ["set", ["allow", "allow-related", "allow-stateless", "drop", "reject"]]}}},
> >                  "log": {"type": "boolean"},
> >                  "severity": {"type": {"key": {"type": "string",
> >                                                "enum": ["set",
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 1b37066b6..dda0a7312 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -1781,7 +1781,8 @@
> >        <p>The action to take when the ACL rule matches:</p>
> >        <ul>
> >          <li>
> > -          <code>allow</code>: Forward the packet.
> > +          <code>allow</code>: Forward the packet. May bypass connection
> > +          tracking when no <code>allow-related</code> rules exist.
>
> If no allow-related rules exist, "allow" ACLs always bypass connection
> tracking.  I think it's more accurate to say something like "will also
> send the packets through connection tracking if 'allow-related' ACLs
> exist on the logical switch".  What do you think?
>
> >          </li>
> >
> >          <li>
> > @@ -1789,6 +1790,11 @@
> >            (e.g. inbound replies to an outbound connection).
> >          </li>
> >
> > +        <li>
> > +          <code>allow-stateless</code>: Always forward the packet in stateless
> > +          manner. May require defining additional rules for inbound replies.
> > +        </li>
> > +
> >          <li>
> >            <code>drop</code>: Silently drop the packet.
> >          </li>
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index eef360802..3d24aa9f7 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -2575,6 +2575,315 @@ sed 's/reg8\[[0..15\]] == [[0-9]]*/reg8\[[0..15\]] == <cleared>/' | sort], [0],
> >  AT_CLEANUP
> >  ])
> >
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([ovn -- ACL allow-stateless omit conntrack - Logical_Switch])
> > +ovn_start
> > +
> > +ovn-nbctl ls-add ls
> > +ovn-nbctl lsp-add ls lsp1
> > +ovn-nbctl lsp-set-addresses lsp1 00:00:00:00:00:01
> > +ovn-nbctl lsp-add ls lsp2
> > +ovn-nbctl lsp-set-addresses lsp2 00:00:00:00:00:02
> > +
> > +for direction in from to; do
> > +    ovn-nbctl acl-add ls ${direction}-lport 3 "tcp" allow-related
> > +    ovn-nbctl acl-add ls ${direction}-lport 2 "udp" allow-related
> > +    ovn-nbctl acl-add ls ${direction}-lport 1 "ip" drop
> > +done
> > +ovn-nbctl --wait=sb sync
> > +
> > +flow_eth='eth.src == 00:00:00:00:00:01 && eth.dst == 00:00:00:00:00:02'
> > +flow_ip='ip.ttl==64 && ip4.src == 42.42.42.1 && ip4.dst == 66.66.66.66'
> > +flow_tcp='tcp && tcp.dst == 80'
> > +flow_udp='udp && udp.dst == 80'
> > +
> > +lsp1_inport=$(fetch_column Port_Binding tunnel_key logical_port=lsp1)
> > +
> > +# TCP packets should go to conntrack.
> > +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
> > +AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0], [dnl
> > +# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> > +ct_next(ct_state=new|trk) {
> > +    ct_next(ct_state=new|trk) {
> > +        output("lsp2");
> > +    };
> > +};
> > +])
> > +
> > +# 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
> > +# udp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> > +ct_next(ct_state=new|trk) {
> > +    ct_next(ct_state=new|trk) {
> > +        output("lsp2");
> > +    };
> > +};
> > +])
> > +
> > +# Allow stateless for TCP.
> > +for direction in from to; do
> > +    ovn-nbctl acl-add ls ${direction}-lport 1 tcp allow-stateless
> > +done
> > +ovn-nbctl --wait=sb sync
> > +
> > +# TCP packets should not go to conntrack anymore.
> > +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
> > +AT_CHECK_UNQUOTED([ovn-trace --minimal ls "${flow}"], [0], [dnl
> > +# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> > +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
> > +# udp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> > +ct_next(ct_state=new|trk) {
> > +    ct_next(ct_state=new|trk) {
> > +        output("lsp2");
> > +    };
> > +};
> > +])
> > +
> > +# Add a load balancer.
> > +ovn-nbctl lb-add lb-tcp 66.66.66.66:80 42.42.42.2:8080 tcp
> > +ovn-nbctl lb-add lb-udp 66.66.66.66:80 42.42.42.2:8080 udp
> > +ovn-nbctl ls-lb-add ls lb-tcp
> > +ovn-nbctl ls-lb-add ls lb-udp
> > +
> > +# Remove stateless for TCP.
> > +ovn-nbctl acl-del ls
> > +ovn-nbctl --wait=sb sync
> > +
> > +# TCP packets should go to conntrack.
> > +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
> > +AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0], [dnl
> > +# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> > +ct_next(ct_state=new|trk) {
> > +    ct_lb {
> > +        reg0[[6]] = 0;
> > +        *** chk_lb_hairpin_reply action not implemented;
> > +        reg0[[12]] = 0;
> > +        ct_next(ct_state=new|trk) {
> > +            output("lsp2");
> > +        };
> > +    };
> > +};
> > +])
> > +
> > +# 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
> > +# udp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> > +ct_next(ct_state=new|trk) {
> > +    ct_lb {
> > +        reg0[[6]] = 0;
> > +        *** chk_lb_hairpin_reply action not implemented;
> > +        reg0[[12]] = 0;
> > +        ct_next(ct_state=new|trk) {
> > +            output("lsp2");
> > +        };
> > +    };
> > +};
> > +])
> > +
> > +# Allow stateless for TCP.
> > +for direction in from to; do
> > +    ovn-nbctl acl-add ls ${direction}-lport 1 tcp allow-stateless
> > +done
> > +ovn-nbctl --wait=sb sync
> > +
> > +# 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
> > +# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> > +ct_next(ct_state=new|trk) {
> > +    ct_lb {
> > +        reg0[[6]] = 0;
> > +        *** chk_lb_hairpin_reply action not implemented;
> > +        reg0[[12]] = 0;
> > +        ct_next(ct_state=new|trk) {
> > +            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
> > +# udp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> > +ct_next(ct_state=new|trk) {
> > +    ct_lb {
> > +        reg0[[6]] = 0;
> > +        *** chk_lb_hairpin_reply action not implemented;
> > +        reg0[[12]] = 0;
> > +        ct_next(ct_state=new|trk) {
> > +            output("lsp2");
> > +        };
> > +    };
> > +};
> > +])
> > +
> > +AT_CLEANUP
> > +])
> > +
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([ovn -- ACL allow-stateless omit conntrack - Port_Group])
> > +ovn_start
> > +
> > +ovn-nbctl ls-add ls
> > +ovn-nbctl lsp-add ls lsp1
> > +ovn-nbctl lsp-set-addresses lsp1 00:00:00:00:00:01
> > +ovn-nbctl lsp-add ls lsp2
> > +ovn-nbctl lsp-set-addresses lsp2 00:00:00:00:00:02
> > +
> > +ovn-nbctl pg-add pg lsp1 lsp2
> > +
> > +for direction in from to; do
> > +    ovn-nbctl acl-add pg ${direction}-lport 3 "tcp" allow-related
> > +    ovn-nbctl acl-add pg ${direction}-lport 2 "udp" allow-related
> > +    ovn-nbctl acl-add pg ${direction}-lport 1 "ip" drop
> > +done
> > +ovn-nbctl --wait=sb sync
> > +
> > +lsp1_inport=$(fetch_column Port_Binding tunnel_key logical_port=lsp1)
> > +echo $lsp1_inport
> > +
> > +flow_eth='eth.src == 00:00:00:00:00:01 && eth.dst == 00:00:00:00:00:02'
> > +flow_ip='ip.ttl==64 && ip4.src == 42.42.42.1 && ip4.dst == 66.66.66.66'
> > +flow_tcp='tcp && tcp.dst == 80'
> > +flow_udp='udp && udp.dst == 80'
> > +
> > +# TCP packets should go to conntrack.
> > +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
> > +AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0], [dnl
> > +# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> > +ct_next(ct_state=new|trk) {
> > +    ct_next(ct_state=new|trk) {
> > +        output("lsp2");
> > +    };
> > +};
> > +])
> > +
> > +# 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
> > +# udp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> > +ct_next(ct_state=new|trk) {
> > +    ct_next(ct_state=new|trk) {
> > +        output("lsp2");
> > +    };
> > +};
> > +])
> > +
> > +# Allow stateless for TCP.
> > +for direction in from to; do
> > +    ovn-nbctl acl-add pg ${direction}-lport 1 tcp allow-stateless
> > +done
> > +ovn-nbctl --wait=sb sync
> > +
> > +# TCP packets should not go to conntrack anymore.
> > +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
> > +AT_CHECK_UNQUOTED([ovn-trace --minimal ls "${flow}"], [0], [dnl
> > +# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> > +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
> > +# udp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> > +ct_next(ct_state=new|trk) {
> > +    ct_next(ct_state=new|trk) {
> > +        output("lsp2");
> > +    };
> > +};
> > +])
> > +
> > +# Add a load balancer.
> > +ovn-nbctl lb-add lb-tcp 66.66.66.66:80 42.42.42.2:8080 tcp
> > +ovn-nbctl lb-add lb-udp 66.66.66.66:80 42.42.42.2:8080 udp
> > +ovn-nbctl ls-lb-add ls lb-tcp
> > +ovn-nbctl ls-lb-add ls lb-udp
> > +
> > +# Remove stateless for TCP.
> > +ovn-nbctl acl-del pg
> > +ovn-nbctl --wait=sb sync
> > +
> > +# TCP packets should go to conntrack.
> > +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
> > +AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0], [dnl
> > +# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> > +ct_next(ct_state=new|trk) {
> > +    ct_lb {
> > +        reg0[[6]] = 0;
> > +        *** chk_lb_hairpin_reply action not implemented;
> > +        reg0[[12]] = 0;
> > +        ct_next(ct_state=new|trk) {
> > +            output("lsp2");
> > +        };
> > +    };
> > +};
> > +])
> > +
> > +# 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
> > +# udp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> > +ct_next(ct_state=new|trk) {
> > +    ct_lb {
> > +        reg0[[6]] = 0;
> > +        *** chk_lb_hairpin_reply action not implemented;
> > +        reg0[[12]] = 0;
> > +        ct_next(ct_state=new|trk) {
> > +            output("lsp2");
> > +        };
> > +    };
> > +};
> > +])
> > +
> > +# Allow stateless for TCP.
> > +for direction in from to; do
> > +    ovn-nbctl acl-add pg ${direction}-lport 1 tcp allow-stateless
> > +done
> > +ovn-nbctl --wait=sb sync
> > +
> > +# 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
> > +# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
> > +ct_next(ct_state=new|trk) {
> > +    ct_lb {
> > +        reg0[[6]] = 0;
> > +        *** chk_lb_hairpin_reply action not implemented;
> > +        reg0[[12]] = 0;
> > +        ct_next(ct_state=new|trk) {
> > +            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
> > +# udp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
> > +ct_next(ct_state=new|trk) {
> > +    ct_lb {
> > +        reg0[[6]] = 0;
> > +        *** chk_lb_hairpin_reply action not implemented;
> > +        reg0[[12]] = 0;
> > +        ct_next(ct_state=new|trk) {
> > +            output("lsp2");
> > +        };
> > +    };
> > +};
> > +])
> > +
> > +AT_CLEANUP
> > +])
> > +
> >  OVN_FOR_EACH_NORTHD([
> >  AT_SETUP([ovn -- check BFD config propagation to SBDB])
> >  AT_KEYWORDS([northd-bfd])
> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > index 184058356..04ac888da 100644
> > --- a/utilities/ovn-nbctl.c
> > +++ b/utilities/ovn-nbctl.c
> > @@ -2303,9 +2303,11 @@ nbctl_acl_add(struct ctl_context *ctx)
> >
> >      /* Validate action. */
> >      if (strcmp(action, "allow") && strcmp(action, "allow-related")
> > -        && strcmp(action, "drop") && strcmp(action, "reject")) {
> > +        && strcmp(action, "allow-stateless") && strcmp(action, "drop")
> > +        && strcmp(action, "reject")) {
> >          ctl_error(ctx, "%s: action must be one of \"allow\", "
> > -                  "\"allow-related\", \"drop\", and \"reject\"", action);
> > +                  "\"allow-related\", \"allow-stateless\", \"drop\", "
> > +                  "and \"reject\"", action);
> >          return;
> >      }
> >
> >
>



More information about the dev mailing list