On Fri, Mar 26, 2021 at 9:16 AM Ben Pfaff <b...@ovn.org> wrote:
>
> On Mon, Mar 22, 2021 at 04:29:11PM +0530, num...@ovn.org wrote:
> > From: Numan Siddique <num...@ovn.org>
> >
> > Presently we add 65535 priority lflows in the stages -
> > 'ls_in_acl' and 'ls_out_acl' to drop packets which
> > match on 'ct.inv'.
> >
> > This patch provides an option to not use this field in the
> > logical flow matches for the following
> > reasons:
> >
> > • Some of the smart NICs which support offloading datapath
> > flows may not support this field. In such cases, almost
> > all the datapath flows cannot be offloaded if ACLs/load balancers
> > are configured on the logical switch datapath.
> >
> > • A recent commit in kernel ovs datapath sets the committed
> > connection tracking entry to be liberal for out-of-window
> > tcp packets (nf_ct_set_tcp_be_liberal()). Such TCP
> > packets will not be marked as invalid.
> >
> > There are still certain scenarios where a packet can be marked
> > invalid. Like
> > • If the tcp flags are not correct.
> >
> > • If there are checksum errors.
> >
> > In such cases, the packet will be delivered to the destination
> > port. So CMS should evaluate their usecases before enabling
> > this option.
> >
> > Signed-off-by: Numan Siddique <num...@ovn.org>
>
> Thanks for writing dldog code! I have a few comments.
>
> This looks correct:
>
> function can_use_ct_inv_match(options: Map<string,string>): bool {
> map_get_bool_def(options, "use_ct_inv_match", true)
> }
>
> relation UseCtInvMatch(use_ct_inv_match: bool)
> UseCtInvMatch(can_use_ct_inv_match(options)) :-
> nb::NB_Global(._uuid = uuid, .options = options).
>
> I would make a few stylistic changes if I were writing it. I would drop
> the _uuid match because it is not used for anything. I would use a
> relation that just contains a bool, instead of a struct that contains a
> bool, because then there are fewer names to invent. I would usually
> drop the function, since it is only used in one place and it is a
> one-liner. So I'd end up with this:
>
> relation UseCtInvMatch[bool]
> UseCtInvMatch[use_ct_inv_match] :-
> nb::NB_Global(.options = options),
> var use_ct_inv_match = map_get_bool_def(options, "use_ct_inv_match",
> true).
>
> Or possibly I'd keep the map_get_bool_def(...) expression inside the
> brackets, I haven't decided what's really the best way:
>
> relation UseCtInvMatch[bool]
> UseCtInvMatch[map_get_bool_def(options, "use_ct_inv_match", true)] :-
> nb::NB_Global(.options = options).
>
> I'd also add a rule to handle the case where NB_Global doesn't exist.
> This is really out of paranoia. I don't know whether this can really
> happen in practice, but it costs only a few lines and avoids a problem
> if it does ever somehow happen:
>
> UseCtInvMatch[true] :-
> Unit(),
> not nb in nb::NB_Global().
>
> I noticed is that this global feature flag is getting copied inside
> every Switch. That wastes a little memory and I do not know of a
> benefit. So I'd drop it from Switch and join with UseCtInvMatch in the
> one place where it's needed.
>
> Finally, there's are lots of redundant strings in ovn_northd.dl. We can
> make that better with a little parameterization, something like this:
> @@ -2290,10 +2290,15 @@ for (Reject(lsuuid, pipeline, stage, acl,
> fair_meter, extra_match_, extra_action
> .actions = actions,
> .external_ids = stage_hint(acl._uuid))
> }
>
> /* build_acls */
> +for (UseCtInvMatch[use_ct_inv_match]) {
> + (var ct_inv_or, var and_not_ct_inv) = match (use_ct_inv_match) {
> + true -> ("ct.inv || ", "&& !ct.inv "),
> + false -> ("", ""),
> + } in
> for (sw in &Switch(.ls = ls))
> var has_stateful = sw.has_stateful_acl or sw.has_lb_vip in
> {
> /* Ingress and Egress ACL Table (Priority 0): Packets are
> allowed by
> * default. A related rule at priority 1 is added below if there
> @@ -2354,17 +2359,17 @@ var has_stateful = sw.has_stateful_acl or
> sw.has_lb_vip in
> *
> * This is enforced at a higher priority than ACLs can be
> defined. */
> Flow(.logical_datapath = ls._uuid,
> .stage = switch_stage(IN, ACL),
> .priority = 65535,
> - .__match = "ct.inv || (ct.est && ct.rpl &&
> ct_label.blocked == 1)",
> + .__match = ct_inv_or ++ "(ct.est && ct.rpl &&
> ct_label.blocked == 1)",
> .actions = "drop;",
> .external_ids = map_empty());
> Flow(.logical_datapath = ls._uuid,
> .stage = switch_stage(OUT, ACL),
> .priority = 65535,
> - .__match = "ct.inv || (ct.est && ct.rpl &&
> ct_label.blocked == 1)",
> + .__match = ct_inv_or ++ "(ct.est && ct.rpl &&
> ct_label.blocked == 1)",
> .actions = "drop;",
> .external_ids = map_empty());
>
> /* Ingress and Egress ACL Table (Priority 65535).
> *
>
> When I put all that together, I get the following replacement overall
> patch. I haven't tested it, so maybe I made some mistakes, but it did
> compile. The change to ovn_northd.dl looks big but most of it is
> reindentation; if you do a "git show -b" after applying it you'll see
> the real changes.
>
> Thanks,
Thanks for all the suggestions. I will apply those changes and test it out.
I knew that copying the global flag to each switch is redundant and a waste
of memory, but I couldn't figure out a better way. Now it makes all sense.
Regards
Numan
>
> Ben.
>
> -8<--------------------------cut here-------------------------->8--
>
> From: Numan Siddique <num...@ovn.org>
> Date: Mon, 22 Mar 2021 16:29:11 +0530
> Subject: [PATCH ovn] northd: Provide the option to not use ct.inv in lflows.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Presently we add 65535 priority lflows in the stages -
> 'ls_in_acl' and 'ls_out_acl' to drop packets which
> match on 'ct.inv'.
>
> This patch provides an option to not use this field in the
> logical flow matches for the following
> reasons:
>
> • Some of the smart NICs which support offloading datapath
> flows may not support this field. In such cases, almost
> all the datapath flows cannot be offloaded if ACLs/load balancers
> are configured on the logical switch datapath.
>
> • A recent commit in kernel ovs datapath sets the committed
> connection tracking entry to be liberal for out-of-window
> tcp packets (nf_ct_set_tcp_be_liberal()). Such TCP
> packets will not be marked as invalid.
>
> There are still certain scenarios where a packet can be marked
> invalid. Like
> • If the tcp flags are not correct.
>
> • If there are checksum errors.
>
> In such cases, the packet will be delivered to the destination
> port. So CMS should evaluate their usecases before enabling
> this option.
>
> Signed-off-by: Numan Siddique <num...@ovn.org>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
> NEWS | 5 +
> northd/lswitch.dl | 8 ++
> northd/ovn-northd.c | 41 +++---
> northd/ovn_northd.dl | 304 ++++++++++++++++++++++---------------------
> ovn-nb.xml | 11 ++
> 5 files changed, 204 insertions(+), 165 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 530c5d42fe85..3181649ba822 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -7,6 +7,11 @@ Post-v21.03.0
> (This may take testing and tuning to be effective.) This version of OVN
> requires DDLog 0.36.
> - Introduce ovn-controller incremetal processing engine statistics
> + - Add a new NB Global option - us_ct_inv_match with the default value of
> true.
> + If this is set to false, then the logical field - "ct.inv" will not be
> + used in the logical flow matches. CMS can consider setting this to
> false,
> + if they want to use smart NICs which don't support offloading datapath
> flows
> + with this field used.
>
> OVN v21.03.0 - 12 Mar 2021
> -------------------------
> diff --git a/northd/lswitch.dl b/northd/lswitch.dl
> index 4bf8a5b907a9..db33bad52ffe 100644
> --- a/northd/lswitch.dl
> +++ b/northd/lswitch.dl
> @@ -232,6 +232,7 @@ function ipv6_parse_prefix(s: string): Option<in6_addr> {
> LogicalSwitchLocalnetPorts(ls._uuid, localnet_ports),
> LogicalSwitchHasNonRouterPort(ls._uuid, has_non_router_port),
> mcast_cfg in &McastSwitchCfg(.datapath = ls._uuid),
> + UseCtInvMatch[use_ct_inv_match],
> var subnet =
> match (ls.other_config.get("subnet")) {
> None -> None,
> @@ -744,3 +745,10 @@ function put_svc_monitor_mac(options: Map<string,string>,
> relation SvcMonitorMac(mac: eth_addr)
> SvcMonitorMac(get_svc_monitor_mac(options, uuid)) :-
> nb::NB_Global(._uuid = uuid, .options = options).
> +
> +relation UseCtInvMatch[bool]
> +UseCtInvMatch[map_get_bool_def(options, "use_ct_inv_match", true)] :-
> + nb::NB_Global(.options = options).
> +UseCtInvMatch[true] :-
> + Unit(),
> + not nb in nb::NB_Global().
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 57df62b9207a..f1332370eb54 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -4068,6 +4068,10 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct
> ovn_datapath *od,
> * logical datapath only by creating a datapath group. */
> static bool use_logical_dp_groups = false;
>
> +/* If this option is 'true' northd will make use of ct.inv match fields.
> + * Otherwise, it will avoid using it. The default is true. */
> +static bool use_ct_inv_match = true;
> +
> /* Adds a row with the specified contents to the Logical_Flow table. */
> static void
> ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od,
> @@ -5647,12 +5651,13 @@ build_acls(struct ovn_datapath *od, struct hmap
> *lflows,
> * for deletion (bit 0 of ct_label is set).
> *
> * This is enforced at a higher priority than ACLs can be defined. */
> + char *match = xasprintf("%s(ct.est && ct.rpl && ct_label.blocked ==
> 1)",
> + use_ct_inv_match ? "ct.inv || " : "");
> ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
> - "ct.inv || (ct.est && ct.rpl && ct_label.blocked ==
> 1)",
> - "drop;");
> + match, "drop;");
> ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
> - "ct.inv || (ct.est && ct.rpl && ct_label.blocked ==
> 1)",
> - "drop;");
> + match, "drop;");
> + free(match);
>
> /* Ingress and Egress ACL Table (Priority 65535).
> *
> @@ -5663,14 +5668,15 @@ build_acls(struct ovn_datapath *od, struct hmap
> *lflows,
> * direction to hit the currently defined policy from ACLs.
> *
> * This is enforced at a higher priority than ACLs can be defined. */
> + match = xasprintf("ct.est && !ct.rel && !ct.new%s && "
> + "ct.rpl && ct_label.blocked == 0",
> + use_ct_inv_match ? " && !ct.inv" : "");
> +
> ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
> - "ct.est && !ct.rel && !ct.new && !ct.inv "
> - "&& ct.rpl && ct_label.blocked == 0",
> - "next;");
> + match, "next;");
> ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
> - "ct.est && !ct.rel && !ct.new && !ct.inv "
> - "&& ct.rpl && ct_label.blocked == 0",
> - "next;");
> + match, "next;");
> + free(match);
>
> /* Ingress and Egress ACL Table (Priority 65535).
> *
> @@ -5683,14 +5689,14 @@ build_acls(struct ovn_datapath *od, struct hmap
> *lflows,
> * a dynamically negotiated FTP data channel), but will allow
> * related traffic such as an ICMP Port Unreachable through
> * that's generated from a non-listening UDP port. */
> + match = xasprintf("!ct.est && ct.rel && !ct.new%s && "
> + "ct_label.blocked == 0",
> + use_ct_inv_match ? " && !ct.inv" : "");
> ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
> - "!ct.est && ct.rel && !ct.new && !ct.inv "
> - "&& ct_label.blocked == 0",
> - "next;");
> + match, "next;");
> ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
> - "!ct.est && ct.rel && !ct.new && !ct.inv "
> - "&& ct_label.blocked == 0",
> - "next;");
> + match, "next;");
> + free(match);
>
> /* Ingress and Egress ACL Table (Priority 65535).
> *
> @@ -12974,6 +12980,9 @@ ovnnb_db_run(struct northd_context *ctx,
>
> use_logical_dp_groups = smap_get_bool(&nb->options,
> "use_logical_dp_groups", false);
> + use_ct_inv_match = smap_get_bool(&nb->options,
> + "use_ct_inv_match", true);
> +
> /* deprecated, use --event instead */
> controller_event_en = smap_get_bool(&nb->options,
> "controller_event", false);
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 4262b83b9760..4f966e50fb7b 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -2292,173 +2292,179 @@ for (Reject(lsuuid, pipeline, stage, acl,
> fair_meter, extra_match_, extra_action
> }
>
> /* build_acls */
> -for (sw in &Switch(.ls = ls))
> -var has_stateful = sw.has_stateful_acl or sw.has_lb_vip in
> -{
> - /* Ingress and Egress ACL Table (Priority 0): Packets are allowed by
> - * default. A related rule at priority 1 is added below if there
> - * are any stateful ACLs in this datapath. */
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(IN, ACL),
> - .priority = 0,
> - .__match = "1",
> - .actions = "next;",
> - .external_ids = map_empty());
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(OUT, ACL),
> - .priority = 0,
> - .__match = "1",
> - .actions = "next;",
> - .external_ids = map_empty());
> -
> - if (has_stateful) {
> - /* Ingress and Egress ACL Table (Priority 1).
> - *
> - * By default, traffic is allowed. This is partially handled by
> - * the Priority 0 ACL flows added earlier, but we also need to
> - * commit IP flows. This is because, while the initiater's
> - * direction may not have any stateful rules, the server's may
> - * and then its return traffic would not have an associated
> - * conntrack entry and would return "+invalid".
> - *
> - * We use "ct_commit" for a connection that is not already known
> - * by the connection tracker. Once a connection is committed,
> - * subsequent packets will hit the flow at priority 0 that just
> - * uses "next;"
> - *
> - * We also check for established connections that have
> ct_label.blocked
> - * set on them. That's a connection that was disallowed, but is
> - * now allowed by policy again since it hit this default-allow flow.
> - * We need to set ct_label.blocked=0 to let the connection continue,
> - * which will be done by ct_commit() in the "stateful" stage.
> - * Subsequent packets will hit the flow at priority 0 that just
> - * uses "next;". */
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(IN, ACL),
> - .priority = 1,
> - .__match = "ip && (!ct.est || (ct.est &&
> ct_label.blocked == 1))",
> - .actions = "${rEGBIT_CONNTRACK_COMMIT()} = 1; next;",
> - .external_ids = map_empty());
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(OUT, ACL),
> - .priority = 1,
> - .__match = "ip && (!ct.est || (ct.est &&
> ct_label.blocked == 1))",
> - .actions = "${rEGBIT_CONNTRACK_COMMIT()} = 1; next;",
> - .external_ids = map_empty());
> -
> - /* Ingress and Egress ACL Table (Priority 65535).
> - *
> - * Always drop traffic that's in an invalid state. Also drop
> - * reply direction packets for connections that have been marked
> - * for deletion (bit 0 of ct_label is set).
> - *
> - * This is enforced at a higher priority than ACLs can be defined. */
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(IN, ACL),
> - .priority = 65535,
> - .__match = "ct.inv || (ct.est && ct.rpl &&
> ct_label.blocked == 1)",
> - .actions = "drop;",
> - .external_ids = map_empty());
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(OUT, ACL),
> - .priority = 65535,
> - .__match = "ct.inv || (ct.est && ct.rpl &&
> ct_label.blocked == 1)",
> - .actions = "drop;",
> - .external_ids = map_empty());
> -
> - /* Ingress and Egress ACL Table (Priority 65535).
> - *
> - * Allow reply traffic that is part of an established
> - * conntrack entry that has not been marked for deletion
> - * (bit 0 of ct_label). We only match traffic in the
> - * reply direction because we want traffic in the request
> - * direction to hit the currently defined policy from ACLs.
> - *
> - * This is enforced at a higher priority than ACLs can be defined. */
> +for (UseCtInvMatch[use_ct_inv_match]) {
> + (var ct_inv_or, var and_not_ct_inv) = match (use_ct_inv_match) {
> + true -> ("ct.inv || ", "&& !ct.inv "),
> + false -> ("", ""),
> + } in
> + for (sw in &Switch(.ls = ls))
> + var has_stateful = sw.has_stateful_acl or sw.has_lb_vip in
> + {
> + /* Ingress and Egress ACL Table (Priority 0): Packets are allowed by
> + * default. A related rule at priority 1 is added below if there
> + * are any stateful ACLs in this datapath. */
> Flow(.logical_datapath = ls._uuid,
> .stage = switch_stage(IN, ACL),
> - .priority = 65535,
> - .__match = "ct.est && !ct.rel && !ct.new && !ct.inv "
> - "&& ct.rpl && ct_label.blocked == 0",
> + .priority = 0,
> + .__match = "1",
> .actions = "next;",
> .external_ids = map_empty());
> Flow(.logical_datapath = ls._uuid,
> .stage = switch_stage(OUT, ACL),
> - .priority = 65535,
> - .__match = "ct.est && !ct.rel && !ct.new && !ct.inv "
> - "&& ct.rpl && ct_label.blocked == 0",
> + .priority = 0,
> + .__match = "1",
> .actions = "next;",
> .external_ids = map_empty());
>
> - /* Ingress and Egress ACL Table (Priority 65535).
> - *
> - * Allow traffic that is related to an existing conntrack entry that
> - * has not been marked for deletion (bit 0 of ct_label).
> - *
> - * This is enforced at a higher priority than ACLs can be defined.
> - *
> - * NOTE: This does not support related data sessions (eg,
> - * a dynamically negotiated FTP data channel), but will allow
> - * related traffic such as an ICMP Port Unreachable through
> - * that's generated from a non-listening UDP port. */
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(IN, ACL),
> - .priority = 65535,
> - .__match = "!ct.est && ct.rel && !ct.new && !ct.inv "
> - "&& ct_label.blocked == 0",
> - .actions = "next;",
> - .external_ids = map_empty());
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(OUT, ACL),
> - .priority = 65535,
> - .__match = "!ct.est && ct.rel && !ct.new && !ct.inv "
> - "&& ct_label.blocked == 0",
> - .actions = "next;",
> - .external_ids = map_empty());
> + if (has_stateful) {
> + /* Ingress and Egress ACL Table (Priority 1).
> + *
> + * By default, traffic is allowed. This is partially handled by
> + * the Priority 0 ACL flows added earlier, but we also need to
> + * commit IP flows. This is because, while the initiater's
> + * direction may not have any stateful rules, the server's may
> + * and then its return traffic would not have an associated
> + * conntrack entry and would return "+invalid".
> + *
> + * We use "ct_commit" for a connection that is not already known
> + * by the connection tracker. Once a connection is committed,
> + * subsequent packets will hit the flow at priority 0 that just
> + * uses "next;"
> + *
> + * We also check for established connections that have
> ct_label.blocked
> + * set on them. That's a connection that was disallowed, but is
> + * now allowed by policy again since it hit this default-allow
> flow.
> + * We need to set ct_label.blocked=0 to let the connection
> continue,
> + * which will be done by ct_commit() in the "stateful" stage.
> + * Subsequent packets will hit the flow at priority 0 that just
> + * uses "next;". */
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(IN, ACL),
> + .priority = 1,
> + .__match = "ip && (!ct.est || (ct.est &&
> ct_label.blocked == 1))",
> + .actions = "${rEGBIT_CONNTRACK_COMMIT()} = 1;
> next;",
> + .external_ids = map_empty());
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(OUT, ACL),
> + .priority = 1,
> + .__match = "ip && (!ct.est || (ct.est &&
> ct_label.blocked == 1))",
> + .actions = "${rEGBIT_CONNTRACK_COMMIT()} = 1;
> next;",
> + .external_ids = map_empty());
>
> - /* Ingress and Egress ACL Table (Priority 65535).
> - *
> - * Not to do conntrack on ND packets. */
> + /* Ingress and Egress ACL Table (Priority 65535).
> + *
> + * Always drop traffic that's in an invalid state. Also drop
> + * reply direction packets for connections that have been marked
> + * for deletion (bit 0 of ct_label is set).
> + *
> + * This is enforced at a higher priority than ACLs can be
> defined. */
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(IN, ACL),
> + .priority = 65535,
> + .__match = ct_inv_or ++ "(ct.est && ct.rpl &&
> ct_label.blocked == 1)",
> + .actions = "drop;",
> + .external_ids = map_empty());
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(OUT, ACL),
> + .priority = 65535,
> + .__match = ct_inv_or ++ "(ct.est && ct.rpl &&
> ct_label.blocked == 1)",
> + .actions = "drop;",
> + .external_ids = map_empty());
> +
> + /* Ingress and Egress ACL Table (Priority 65535).
> + *
> + * Allow reply traffic that is part of an established
> + * conntrack entry that has not been marked for deletion
> + * (bit 0 of ct_label). We only match traffic in the
> + * reply direction because we want traffic in the request
> + * direction to hit the currently defined policy from ACLs.
> + *
> + * This is enforced at a higher priority than ACLs can be
> defined. */
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(IN, ACL),
> + .priority = 65535,
> + .__match = "ct.est && !ct.rel && !ct.new " ++
> and_not_ct_inv ++
> + "&& ct.rpl && ct_label.blocked == 0",
> + .actions = "next;",
> + .external_ids = map_empty());
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(OUT, ACL),
> + .priority = 65535,
> + .__match = "ct.est && !ct.rel && !ct.new " ++
> and_not_ct_inv ++
> + "&& ct.rpl && ct_label.blocked == 0",
> + .actions = "next;",
> + .external_ids = map_empty());
> +
> + /* Ingress and Egress ACL Table (Priority 65535).
> + *
> + * Allow traffic that is related to an existing conntrack entry
> that
> + * has not been marked for deletion (bit 0 of ct_label).
> + *
> + * This is enforced at a higher priority than ACLs can be
> defined.
> + *
> + * NOTE: This does not support related data sessions (eg,
> + * a dynamically negotiated FTP data channel), but will allow
> + * related traffic such as an ICMP Port Unreachable through
> + * that's generated from a non-listening UDP port. */
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(IN, ACL),
> + .priority = 65535,
> + .__match = "!ct.est && ct.rel && !ct.new " ++
> and_not_ct_inv ++
> + "&& ct_label.blocked == 0",
> + .actions = "next;",
> + .external_ids = map_empty());
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(OUT, ACL),
> + .priority = 65535,
> + .__match = "!ct.est && ct.rel && !ct.new " ++
> and_not_ct_inv ++
> + "&& ct_label.blocked == 0",
> + .actions = "next;",
> + .external_ids = map_empty());
> +
> + /* Ingress and Egress ACL Table (Priority 65535).
> + *
> + * Not to do conntrack on ND packets. */
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(IN, ACL),
> + .priority = 65535,
> + .__match = "nd || nd_ra || nd_rs || mldv1 ||
> mldv2",
> + .actions = "next;",
> + .external_ids = map_empty());
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(OUT, ACL),
> + .priority = 65535,
> + .__match = "nd || nd_ra || nd_rs || mldv1 ||
> mldv2",
> + .actions = "next;",
> + .external_ids = map_empty())
> + };
> +
> + /* Add a 34000 priority flow to advance the DNS reply from
> ovn-controller,
> + * if the CMS has configured DNS records for the datapath.
> + */
> + if (sw.has_dns_records) {
> + Flow(.logical_datapath = ls._uuid,
> + .stage = switch_stage(OUT, ACL),
> + .priority = 34000,
> + .__match = "udp.src == 53",
> + .actions = if has_stateful "ct_commit; next;" else
> "next;",
> + .external_ids = map_empty())
> + };
> +
> + /* Add a 34000 priority flow to advance the service monitor reply
> + * packets to skip applying ingress ACLs. */
> Flow(.logical_datapath = ls._uuid,
> .stage = switch_stage(IN, ACL),
> - .priority = 65535,
> - .__match = "nd || nd_ra || nd_rs || mldv1 || mldv2",
> + .priority = 34000,
> + .__match = "eth.dst == $svc_monitor_mac",
> .actions = "next;",
> .external_ids = map_empty());
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(OUT, ACL),
> - .priority = 65535,
> - .__match = "nd || nd_ra || nd_rs || mldv1 || mldv2",
> - .actions = "next;",
> - .external_ids = map_empty())
> - };
> -
> - /* Add a 34000 priority flow to advance the DNS reply from
> ovn-controller,
> - * if the CMS has configured DNS records for the datapath.
> - */
> - if (sw.has_dns_records) {
> Flow(.logical_datapath = ls._uuid,
> .stage = switch_stage(OUT, ACL),
> .priority = 34000,
> - .__match = "udp.src == 53",
> - .actions = if has_stateful "ct_commit; next;" else
> "next;",
> + .__match = "eth.src == $svc_monitor_mac",
> + .actions = "next;",
> .external_ids = map_empty())
> - };
> -
> - /* Add a 34000 priority flow to advance the service monitor reply
> - * packets to skip applying ingress ACLs. */
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(IN, ACL),
> - .priority = 34000,
> - .__match = "eth.dst == $svc_monitor_mac",
> - .actions = "next;",
> - .external_ids = map_empty());
> - Flow(.logical_datapath = ls._uuid,
> - .stage = switch_stage(OUT, ACL),
> - .priority = 34000,
> - .__match = "eth.src == $svc_monitor_mac",
> - .actions = "next;",
> - .external_ids = map_empty())
> + }
> }
>
> /* This stage builds hints for the IN/OUT_ACL stage. Based on various
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index b0a4adffe537..c3ff3b586ad1 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -227,6 +227,17 @@
> </p>
> </column>
>
> + <column name="options" key="use_ct_inv_match">
> + <p>
> + If set to false, <code>ovn-northd</code> will not use the
> + <code>ct.inv</code> field in any of the logical flow matches.
> + The default value is true. CMS can consider setting this to
> + false, in case the NIC doesn't support offloading OVS datapath
> + flows having matches <code>ct_state(..+inv..)</code> or
> + <code>ct_state(..-inv..)</code>.
> + </p>
> + </column>
> +
> <group title="Options for configuring interconnection route
> advertisement">
> <p>
> These options control how routes are advertised between OVN
> --
> 2.29.2
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
