Re: [ovs-dev] [PATCH ovn] ovn-sbctl: Fix lflow-list (etc.) in daemon mode and upon races.
On Thu, Jun 03, 2021 at 07:38:01PM -0400, Numan Siddique wrote: > On Thu, Jun 3, 2021 at 3:48 PM Ben Pfaff wrote: > > > > Utilities like ovn-sbctl sometimes need to retry their transactions > > because of races. For this reason, instead of sending user output > > directly to stdout, they buffer it until the transaction succeeds. > > Some of the ovn-sbctl commands didn't do this properly, so they would > > output multiple times upon a race. Another way to see the problem > > was to use daemon mode, in which the output written directly with > > printf() would not appear at all, since the daemon's stdout is not > > connected to ovn-sbctl's stdout. > > > > Signed-off-by: Ben Pfaff > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1965780 > > Reported-by: Alexey Roytman > > Acked-by: Numan Siddique Thanks, I applied this to master and branch-21.06. In theory this could be backported further, because it would also manifest upon races, but I don't think that's too likely because ovn-sbctl is rarely used to both modify things and dump flows in a single transaction. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-sbctl: Fix lflow-list (etc.) in daemon mode and upon races.
On Thu, Jun 3, 2021 at 3:48 PM Ben Pfaff wrote:
>
> Utilities like ovn-sbctl sometimes need to retry their transactions
> because of races. For this reason, instead of sending user output
> directly to stdout, they buffer it until the transaction succeeds.
> Some of the ovn-sbctl commands didn't do this properly, so they would
> output multiple times upon a race. Another way to see the problem
> was to use daemon mode, in which the output written directly with
> printf() would not appear at all, since the daemon's stdout is not
> connected to ovn-sbctl's stdout.
>
> Signed-off-by: Ben Pfaff
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1965780
> Reported-by: Alexey Roytman
Acked-by: Numan Siddique
Numan
> ---
> utilities/ovn-sbctl.c | 151 ++
> 1 file changed, 79 insertions(+), 72 deletions(-)
>
> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> index 53f10cdd897a..4146384e74fb 100644
> --- a/utilities/ovn-sbctl.c
> +++ b/utilities/ovn-sbctl.c
> @@ -618,7 +618,8 @@ sbctl_open_vconn(struct shash *options)
> }
>
> static void
> -sbctl_dump_openflow(struct vconn *vconn, const struct uuid *uuid, bool stats)
> +sbctl_dump_openflow(struct vconn *vconn, const struct uuid *uuid, bool stats,
> +struct ds *s)
> {
> struct ofputil_flow_stats_request fsr = {
> .cookie = htonll(uuid->parts[0]),
> @@ -639,28 +640,26 @@ sbctl_dump_openflow(struct vconn *vconn, const struct
> uuid *uuid, bool stats)
> }
>
> if (n_fses) {
> -struct ds s = DS_EMPTY_INITIALIZER;
> for (size_t i = 0; i < n_fses; i++) {
> const struct ofputil_flow_stats *fs = [i];
>
> -ds_clear();
> +ds_put_cstr(s, "");
> if (stats) {
> -ofputil_flow_stats_format(, fs, NULL, NULL, true);
> +ofputil_flow_stats_format(s, fs, NULL, NULL, true);
> } else {
> -ds_put_format(, "%stable=%s%"PRIu8" ",
> +ds_put_format(s, "%stable=%s%"PRIu8" ",
>colors.special, colors.end, fs->table_id);
> -match_format(>match, NULL, , OFP_DEFAULT_PRIORITY);
> -if (ds_last() != ' ') {
> -ds_put_char(, ' ');
> +match_format(>match, NULL, s, OFP_DEFAULT_PRIORITY);
> +if (ds_last(s) != ' ') {
> +ds_put_char(s, ' ');
> }
>
> -ds_put_format(, "%sactions=%s", colors.actions,
> colors.end);
> -struct ofpact_format_params fp = { .s = };
> +ds_put_format(s, "%sactions=%s", colors.actions, colors.end);
> +struct ofpact_format_params fp = { .s = s };
> ofpacts_format(fs->ofpacts, fs->ofpacts_len, );
> }
> -printf("%s\n", ds_cstr());
> +ds_put_char(s, '\n');
> }
> -ds_destroy();
> }
>
> for (size_t i = 0; i < n_fses; i++) {
> @@ -670,37 +669,37 @@ sbctl_dump_openflow(struct vconn *vconn, const struct
> uuid *uuid, bool stats)
> }
>
> static void
> -print_datapath_name(const struct sbrec_datapath_binding *dp)
> +print_datapath_name(const struct sbrec_datapath_binding *dp, struct ds *s)
> {
> const struct smap *ids = >external_ids;
> const char *name = smap_get(ids, "name");
> const char *name2 = smap_get(ids, "name2");
> if (name && name2) {
> -printf("\"%s\" aka \"%s\"", name, name2);
> +ds_put_format(s, "\"%s\" aka \"%s\"", name, name2);
> } else if (name || name2) {
> -printf("\"%s\"", name ? name : name2);
> +ds_put_format(s, "\"%s\"", name ? name : name2);
> }
> }
>
> static void
> print_vflow_datapath_name(const struct sbrec_datapath_binding *dp,
> - bool do_print)
> + bool do_print, struct ds *s)
> {
> if (!do_print) {
> return;
> }
> -printf("datapath=");
> -print_datapath_name(dp);
> -printf(", ");
> +ds_put_cstr(s, "datapath=");
> +print_datapath_name(dp, s);
> +ds_put_cstr(s, ", ");
> }
>
> static void
> -print_uuid_part(const struct uuid *uuid, bool do_print)
> +print_uuid_part(const struct uuid *uuid, bool do_print, struct ds *s)
> {
> if (!do_print) {
> return;
> }
> -printf("uuid=0x%08"PRIx32", ", uuid->parts[0]);
> +ds_put_format(s, "uuid=0x%08"PRIx32", ", uuid->parts[0]);
> }
>
> static void
> @@ -717,16 +716,17 @@ cmd_lflow_list_port_bindings(struct ctl_context *ctx,
> struct vconn *vconn,
> }
>
> if (!pb_prev) {
> -printf("\nPort Bindings:\n");
> +ds_put_cstr(>output, "\nPort Bindings:\n");
> }
>
> -printf(" ");
> -print_uuid_part(>header_.uuid, print_uuid);
> -print_vflow_datapath_name(pb->datapath, !datapath);
> -printf("logical_port=%s,
Re: [ovs-dev] [PATCH v3 ovn 4/4] NEWS: Add CoPP support.
Bleep bloop. Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Lorenzo Bianconi Lines checked: 26, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 ovn 2/4] ovn-northd: Add support for CoPP.
Bleep bloop. Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line lacks whitespace around operator #829 FILE: utilities/ovn-nbctl.c:430: ls-copp-add SWITCH PROTO METER\n\ WARNING: Line lacks whitespace around operator #832 FILE: utilities/ovn-nbctl.c:433: ls-copp-del SWITCH [PROTO]\n\ WARNING: Line lacks whitespace around operator #836 FILE: utilities/ovn-nbctl.c:437: ls-copp-list SWITCH\n\ WARNING: Line lacks whitespace around operator #839 FILE: utilities/ovn-nbctl.c:440: lr-copp-add ROUTER PROTO METER\n\ WARNING: Line lacks whitespace around operator #842 FILE: utilities/ovn-nbctl.c:443: lr-copp-del ROUTER [PROTO]\n\ WARNING: Line lacks whitespace around operator #846 FILE: utilities/ovn-nbctl.c:447: lr-copp-list ROUTER\n\ Lines checked: 1024, Warnings: 6, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 ovn 2/4] ovn-northd: Add support for CoPP.
From: Dumitru Ceara
Add new 'Copp' (Control plane protection) table to OVN Northbound DB:
- this stores mappings between control plane protocol names and meters
that should be used to rate limit controller-destined traffic for
those protocols.
Add new 'copp' columns to the following OVN Northbound DB tables:
- Logical_Switch
- Logical_Router
For now, no control plane protection policy is installed for any of
the existing flows that punt packets to ovn-controller. This will be
added in follow-up patches.
Add CLI commands in 'ovn-nbctl' to allow the user to manage Control
Plane Protection Policies at different levels (logical switch,
logical router).
Co-authored-by: Lorenzo Bianconi
Signed-off-by: Lorenzo Bianconi
Signed-off-by: Dumitru Ceara
---
lib/automake.mk | 2 +
lib/copp.c| 142 ++
lib/copp.h| 58 +
northd/ovn-northd.c | 44 +++---
ovn-nb.ovsschema | 16 +++-
ovn-nb.xml| 78 +
tests/ovn-controller.at | 48 ++
tests/ovn-northd.at | 92
utilities/ovn-nbctl.8.xml | 72 +++
utilities/ovn-nbctl.c | 178 ++
10 files changed, 718 insertions(+), 12 deletions(-)
create mode 100644 lib/copp.c
create mode 100644 lib/copp.h
diff --git a/lib/automake.mk b/lib/automake.mk
index 781be2109..20e296fff 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -9,6 +9,8 @@ lib_libovn_la_SOURCES = \
lib/actions.c \
lib/chassis-index.c \
lib/chassis-index.h \
+ lib/copp.c \
+ lib/copp.h \
lib/ovn-dirs.h \
lib/expr.c \
lib/extend-table.h \
diff --git a/lib/copp.c b/lib/copp.c
new file mode 100644
index 0..e3d14938a
--- /dev/null
+++ b/lib/copp.c
@@ -0,0 +1,142 @@
+/* Copyright (c) 2021, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+#include
+
+#include "openvswitch/shash.h"
+#include "db-ctl-base.h"
+#include "smap.h"
+#include "lib/ovn-nb-idl.h"
+#include "lib/copp.h"
+
+static char *copp_proto_names[COPP_PROTO_MAX] = {
+[COPP_ARP] = "arp",
+[COPP_ARP_RESOLVE] = "arp-resolve",
+[COPP_DHCPV4_OPTS] = "dhcpv4-opts",
+[COPP_DHCPV6_OPTS] = "dhcpv6-opts",
+[COPP_DNS] = "dns",
+[COPP_EVENT_ELB] = "event-elb",
+[COPP_ICMP4_ERR] = "icmp4-error",
+[COPP_ICMP6_ERR] = "icmp6-error",
+[COPP_IGMP] = "igmp",
+[COPP_ND_NA] = "nd-na",
+[COPP_ND_NS] = "nd-ns",
+[COPP_ND_NS_RESOLVE] = "nd-ns-resolve",
+[COPP_ND_RA_OPTS]= "nd-ra-opts",
+[COPP_TCP_RESET] = "tcp-reset",
+[COPP_BFD] = "bfd",
+};
+
+static const char *
+copp_proto_get_name(enum copp_proto proto)
+{
+if (proto >= COPP_PROTO_MAX) {
+return "";
+}
+return copp_proto_names[proto];
+}
+
+const char *
+copp_meter_get(enum copp_proto proto, const struct nbrec_copp *copp,
+ const struct shash *meter_groups)
+{
+if (!copp || proto >= COPP_PROTO_MAX) {
+return NULL;
+}
+
+const char *meter = smap_get(>meters, copp_proto_names[proto]);
+
+if (meter && shash_find(meter_groups, meter)) {
+return meter;
+}
+
+return NULL;
+}
+
+void
+copp_meter_list(struct ctl_context *ctx, const struct nbrec_copp *copp)
+{
+if (!copp) {
+return;
+}
+
+struct smap_node *node;
+
+SMAP_FOR_EACH (node, >meters) {
+ds_put_format(>output, "%s: %s\n", node->key, node->value);
+}
+}
+
+const struct nbrec_copp *
+copp_meter_add(struct ctl_context *ctx, const struct nbrec_copp *copp,
+ const char *proto_name, const char *meter)
+{
+if (!copp) {
+copp = nbrec_copp_insert(ctx->txn);
+}
+
+struct smap meters;
+smap_init();
+smap_clone(, >meters);
+smap_replace(, proto_name, meter);
+nbrec_copp_set_meters(copp, );
+smap_destroy();
+
+return copp;
+}
+
+void
+copp_meter_del(const struct nbrec_copp *copp, const char *proto_name)
+{
+if (!copp) {
+return;
+}
+
+if (proto_name) {
+if (smap_get(>meters, proto_name)) {
+struct smap meters;
+smap_init();
+smap_clone(, >meters);
+smap_remove(, proto_name);
+nbrec_copp_set_meters(copp, );
+smap_destroy();
+}
+}
[ovs-dev] [PATCH v3 ovn 3/4] ovn-northd: Add CoPP policies for flows that punt packets to ovn-controller.
From: Dumitru Ceara
Change the ovn-northd implementation to set the new 'controller_meter'
field for flows that need to punt packets to ovn-controller.
Protocol packets for which CoPP is enforced when sending packets to
ovn-controller (if configured):
- ARP
- ND_NS
- ND_NA
- ND_RA
- DNS
- IGMP
- packets that require ARP resolution before forwarding
- packets that require ND_NS before forwarding
- packets that need to be replied to with ICMP Errors
- packets that need to be replied to with TCP RST
- packets that need to be replied to with DHCP_OPTS
- packets that trigger a SCTP abort action
- contoller_events
- BFD
Co-authored-by: Lorenzo Bianconi
Signed-off-by: Lorenzo Bianconi
Signed-off-by: Dumitru Ceara
---
include/ovn/actions.h | 1 -
lib/actions.c | 50 +---
lib/copp.c| 1 +
lib/copp.h| 1 +
northd/ovn-northd.c | 493 --
ovn-nb.xml| 3 +
tests/atlocal.in | 3 +
tests/ovn.at | 4 +-
tests/system-ovn.at | 135 +++
utilities/ovn-nbctl.8.xml | 3 +
10 files changed, 469 insertions(+), 225 deletions(-)
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index ab03df12c..b2f2f57c6 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -393,7 +393,6 @@ struct ovnact_controller_event {
int event_type; /* controller event type */
struct ovnact_gen_option *options;
size_t n_options;
-char *meter;
};
/* OVNACT_BIND_VPORT. */
diff --git a/lib/actions.c b/lib/actions.c
index 155b4a45a..f0291afef 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -1613,9 +1613,6 @@ format_TRIGGER_EVENT(const struct ovnact_controller_event
*event,
{
ds_put_format(s, "trigger_event(event = \"%s\"",
event_to_string(event->event_type));
-if (event->meter) {
-ds_put_format(s, ", meter = \"%s\"", event->meter);
-}
for (const struct ovnact_gen_option *o = event->options;
o < >options[event->n_options]; o++) {
ds_put_cstr(s, ", ");
@@ -1790,24 +1787,11 @@ encode_event_empty_lb_backends_opts(struct ofpbuf
*ofpacts,
static void
encode_TRIGGER_EVENT(const struct ovnact_controller_event *event,
- const struct ovnact_encode_params *ep OVS_UNUSED,
+ const struct ovnact_encode_params *ep,
struct ofpbuf *ofpacts)
{
-uint32_t meter_id = NX_CTLR_NO_METER;
-size_t oc_offset;
-
-if (event->meter) {
-meter_id = ovn_extend_table_assign_id(ep->meter_table, event->meter,
- ep->lflow_uuid);
-if (meter_id == EXT_TABLE_ID_INVALID) {
-VLOG_WARN("Unable to assign id for trigger meter: %s",
- event->meter);
-return;
-}
-}
-
-oc_offset = encode_start_controller_op(ACTION_OPCODE_EVENT, false,
- meter_id, ofpacts);
+size_t oc_offset = encode_start_controller_op(ACTION_OPCODE_EVENT, false,
+ ep->ctrl_meter_id, ofpacts);
ovs_be32 ofs = htonl(event->event_type);
ofpbuf_put(ofpacts, , sizeof ofs);
@@ -2341,27 +2325,12 @@ parse_trigger_event(struct action_context *ctx,
sizeof *event->options);
}
-if (lexer_match_id(ctx->lexer, "meter")) {
-if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
-return;
-}
-/* If multiple meters are given, use the most recent. */
-if (ctx->lexer->token.type == LEX_T_STRING &&
-strlen(ctx->lexer->token.s)) {
-free(event->meter);
-event->meter = xstrdup(ctx->lexer->token.s);
-} else if (ctx->lexer->token.type != LEX_T_STRING) {
-lexer_syntax_error(ctx->lexer, "expecting string");
-return;
-}
-lexer_get(ctx->lexer);
-} else {
-struct ovnact_gen_option *o = >options[event->n_options++];
-memset(o, 0, sizeof *o);
-parse_gen_opt(ctx, o,
->pp->controller_event_opts->event_opts[event_type],
-event_to_string(event_type));
-}
+struct ovnact_gen_option *o = >options[event->n_options++];
+memset(o, 0, sizeof *o);
+parse_gen_opt(ctx, o,
+ >pp->controller_event_opts->event_opts[event_type],
+ event_to_string(event_type));
+
if (ctx->lexer->error) {
return;
}
@@ -2382,7 +2351,6 @@ static void
ovnact_controller_event_free(struct ovnact_controller_event *event)
{
free_gen_options(event->options, event->n_options);
-free(event->meter);
}
static void
diff --git a/lib/copp.c b/lib/copp.c
index e3d14938a..bbe66924b 100644
--- a/lib/copp.c
+++
[ovs-dev] [PATCH v3 ovn 4/4] NEWS: Add CoPP support.
From: Dumitru Ceara Signed-off-by: Dumitru Ceara Signed-off-by: Lorenzo Bianconi --- NEWS | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS b/NEWS index 506d7bc56..27138467b 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,7 @@ Post-v21.03.0 * ovn-sbctl now also supports daemon mode. - Added support in native DNS to respond to PTR request types. - New --dry-run option for ovn-northd and ovn-northd-ddlog. + - Added Control Plane Protection support (control plane traffic metering). OVN v21.03.0 - 12 Mar 2021 - -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 ovn 1/4] ovn-controller: Add support for Logical_Flow control meters
From: Dumitru Ceara
Add a new 'controller_meter' column to OVN Southbound Logical_Flow
table. This stores an optional string which should correspond to
the Meter that must be used for rate limiting controller actions
generated by packets hitting the flow.
Add a new 'ofctrl_add_flow_metred' function to create a new 'ovn_flow'
with an attached controller meter.
Change ofctrl_check_and_add_flow to allow specifying a meter ID for
packets that are punted to controller.
Change consider_logical_flow to parse controller_meter from the logical
flow and use it when building openflow entries.
Add a new 'ctrl_meter_id' field to 'struct ovnact_encode_params' to be
used when encoding controller actions from logical flow actions.
Co-authored-by: Lorenzo Bianconi
Signed-off-by: Lorenzo Bianconi
Signed-off-by: Dumitru Ceara
---
controller/lflow.c| 40 +++---
controller/ofctrl.c | 55 +---
controller/ofctrl.h | 21 ++
controller/physical.c | 7 +++--
include/ovn/actions.h | 2 ++
lib/actions.c | 66 +++
ovn-sb.ovsschema | 6 ++--
ovn-sb.xml| 6
8 files changed, 140 insertions(+), 63 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index 680b8cca1..ab7f3d286 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -557,6 +557,27 @@ update_conj_id_ofs(uint32_t *conj_id_ofs, uint32_t n_conjs)
return false;
}
+static void
+lflow_parse_ctrl_meter(const struct sbrec_logical_flow *lflow,
+ struct ovn_extend_table *meter_table,
+ uint32_t *meter_id)
+{
+ovs_assert(meter_id);
+*meter_id = NX_CTLR_NO_METER;
+
+if (lflow->controller_meter) {
+*meter_id = ovn_extend_table_assign_id(meter_table,
+ lflow->controller_meter,
+ lflow->header_.uuid);
+if (*meter_id == EXT_TABLE_ID_INVALID) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+VLOG_WARN_RL(, "Unable to assign id for meter: %s",
+ lflow->controller_meter);
+return;
+}
+}
+}
+
static void
add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
const struct sbrec_datapath_binding *dp,
@@ -572,6 +593,13 @@ add_matches_to_flow_table(const struct sbrec_logical_flow
*lflow,
.dp = dp,
};
+/* Parse any meter to be used if this flow should punt packets to
+ * controller.
+ */
+uint32_t ctrl_meter_id = NX_CTLR_NO_METER;
+lflow_parse_ctrl_meter(lflow, l_ctx_out->meter_table,
+ _meter_id);
+
/* Encode OVN logical actions into OpenFlow. */
uint64_t ofpacts_stub[1024 / 8];
struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
@@ -595,6 +623,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow
*lflow,
.ct_snat_vip_ptable = OFTABLE_CT_SNAT_FOR_VIP,
.fdb_ptable = OFTABLE_GET_FDB,
.fdb_lookup_ptable = OFTABLE_LOOKUP_FDB,
+.ctrl_meter_id = ctrl_meter_id,
};
ovnacts_encode(ovnacts->data, ovnacts->size, , );
@@ -621,9 +650,11 @@ add_matches_to_flow_table(const struct sbrec_logical_flow
*lflow,
}
}
if (!m->n) {
-ofctrl_add_flow(l_ctx_out->flow_table, ptable, lflow->priority,
-lflow->header_.uuid.parts[0], >match, ,
->header_.uuid);
+ofctrl_add_flow_metered(l_ctx_out->flow_table, ptable,
+lflow->priority,
+lflow->header_.uuid.parts[0], >match,
+, >header_.uuid,
+ctrl_meter_id);
} else {
uint64_t conj_stubs[64 / 8];
struct ofpbuf conj;
@@ -641,7 +672,8 @@ add_matches_to_flow_table(const struct sbrec_logical_flow
*lflow,
ofctrl_add_or_append_flow(l_ctx_out->flow_table, ptable,
lflow->priority, 0,
- >match, , >header_.uuid);
+ >match, , >header_.uuid,
+ ctrl_meter_id);
ofpbuf_uninit();
}
}
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index c29c3d180..3293aa6b6 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -66,6 +66,7 @@ struct ovn_flow {
struct ofpact *ofpacts;
size_t ofpacts_len;
uint64_t cookie;
+uint32_t ctrl_meter_id; /* Meter to be used for controller actions. */
};
/* A desired flow, in struct ovn_desired_flow_table, calculated by the
@@ -220,7 +221,8 @@ static struct desired_flow *desired_flow_alloc(
uint16_t priority,
uint64_t cookie,
const struct
[ovs-dev] [PATCH v3 ovn 0/4] respin CoPP series
This series respin CoPP support introduced here [0] by Dumitru rebasing on top of ovn master branch and adding some missing meters (e.g. bfd or acl reject). The main goal of this series is to continue the discussion about the proposed approach and to align on CMS APIs. For the moment no ddlog support has been added. Related bz: https://bugzilla.redhat.com/show_bug.cgi?id=1947913 https://bugzilla.redhat.com/show_bug.cgi?id=1946610 Changes since v2: - add sbctl checks in tests/ovn-northd.at unit tests - remove letfovers in utilities/ovn-nbctl.8.xml Changes since v1: - merge patch 3/5 and 4/5 - cosmetics - improve naming conventions - add more unit-tests/system-tests - remove duplicated flow - remove some leftover entries in ovn-nbctl.8.xml - add metering for sctp abort packets Changes since RFC: - drop per-port metering - add unit/system tests - add reject action metering [0] https://patchwork.ozlabs.org/project/openvswitch/list/?series=140778=* Dumitru Ceara (4): ovn-controller: Add support for Logical_Flow control meters ovn-northd: Add support for CoPP. ovn-northd: Add CoPP policies for flows that punt packets to ovn-controller. NEWS: Add CoPP support. NEWS | 1 + controller/lflow.c| 40 ++- controller/ofctrl.c | 55 ++-- controller/ofctrl.h | 21 +- controller/physical.c | 7 +- include/ovn/actions.h | 3 +- lib/actions.c | 116 - lib/automake.mk | 2 + lib/copp.c| 143 ++ lib/copp.h| 59 + northd/ovn-northd.c | 535 -- ovn-nb.ovsschema | 16 +- ovn-nb.xml| 81 ++ ovn-sb.ovsschema | 6 +- ovn-sb.xml| 6 + tests/atlocal.in | 3 + tests/ovn-controller.at | 48 tests/ovn-northd.at | 92 +++ tests/ovn.at | 4 +- tests/system-ovn.at | 135 ++ utilities/ovn-nbctl.8.xml | 75 ++ utilities/ovn-nbctl.c | 178 + 22 files changed, 1327 insertions(+), 299 deletions(-) create mode 100644 lib/copp.c create mode 100644 lib/copp.h -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3] netlink: removed incorrect optimization
This optimization caused FLOW_TNL_F_UDPIF flag not to be used in
hash calculation for geneve tunnel when revalidating flows which
resulted in different cache hash values and incorrect behaviour.
Added test to prevent regression.
Reported-at: https://github.com/vmware-tanzu/antrea/issues/897
Signed-off-by: Toms Atteka
---
lib/tun-metadata.c | 2 +-
tests/system-traffic.at | 54 +
2 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
index c0b0ae044..af0bcbde8 100644
--- a/lib/tun-metadata.c
+++ b/lib/tun-metadata.c
@@ -828,7 +828,7 @@ tun_metadata_to_geneve_nlattr(const struct flow_tnl *tun,
} else {
tun_metadata_to_geneve_nlattr_mask(key, tun, flow, b);
}
-} else if (flow->metadata.present.len || is_mask) {
+} else {
nl_msg_put_unspec(b, OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS,
tun->metadata.opts.gnv,
flow->metadata.present.len);
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index fb5b9a36d..483cc7e83 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -574,6 +574,60 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2
10.1.1.100 | FORMAT_PI
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([datapath - ping over geneve tunnel, delete flow regression])
+OVS_CHECK_GENEVE()
+
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-underlay])
+
+AT_DATA([flows.txt], [dnl
+priority=100,icmp actions=resubmit(,10)
+priority=0 actions=NORMAL
+table=10, priority=100, ip, actions=ct(table=20,zone=65520)
+table=20, priority=200, ip, ct_state=-new+trk, actions=resubmit(,30)
+table=20, priority=100, ip, ct_state=+new, actions=resubmit(,30)
+table=20, priority=50, ip, actions=DROP
+table=30, priority=100, ip, actions=ct(commit,table=40,zone=65520)
+table=40, actions=normal
+])
+
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
+AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace and with a native
+dnl linux device inside the namespace.
+ADD_OVS_TUNNEL([geneve], [br0], [at_gnv0], [172.31.1.1], [10.1.1.100/24])
+ADD_NATIVE_TUNNEL([geneve], [ns_gnv0], [at_ns0], [172.31.1.100], [10.1.1.1/24],
+ [vni 0])
+
+dnl First, check the underlay
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING],
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl ping over tunnel should work
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PING],
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+AT_CHECK([ovs-ofctl del-flows br0 "ct_state=+new"])
+
+dnl ping should not go through after removal of the flow
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PING],
[0], [dnl
+7 packets transmitted, 0 received, 100% packet loss, time 0ms
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP(["/|ERR|/d
+/|WARN|/d"])
+AT_CLEANUP
+
AT_SETUP([datapath - flow resume with geneve tun_metadata])
OVS_CHECK_GENEVE()
--
2.25.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] Moving of the primary #openvswitch channel to irc.libera.chat ?
[seems I saved this as a draft and never sent it, oops] On Wed, May 26, 2021 at 01:41:01PM +0200, Ilya Maximets wrote: > On 5/26/21 9:39 AM, Frode Nordahl wrote: > > On Thu, May 20, 2021 at 10:08 PM Ihar Hrachyshka > > wrote: > > Our, and some 700 other channels, were just taken over for advertising > > other IRC networks in topic (see attached screenshot). I think this is > > the cue to leave Freenode. > > Yes, this looks like no way back. > > Ben did you lose control over the channel on freenode? Yes. I can't even get into #openvswitch. It's invitation-only now. Somehow I got put into ##openvswitch instead, but that is owned by a freenode bot: ChanServ: (notice) Information on ##openvswitch: ChanServ: (notice) Founder: freenode-placeholder-account ChanServ: (notice) Registered : May 26 02:38:03 2021 (1d 13h 54m 48s ago) ChanServ: (notice) Last used : May 26 02:38:03 2021 (1d 13h 54m 48s ago) ChanServ: (notice) Mode lock : +ntF ChanServ: (notice) Flags : GUARD ChanServ: (notice) *** End of Info *** ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ovn-sbctl: Fix lflow-list (etc.) in daemon mode and upon races.
Utilities like ovn-sbctl sometimes need to retry their transactions
because of races. For this reason, instead of sending user output
directly to stdout, they buffer it until the transaction succeeds.
Some of the ovn-sbctl commands didn't do this properly, so they would
output multiple times upon a race. Another way to see the problem
was to use daemon mode, in which the output written directly with
printf() would not appear at all, since the daemon's stdout is not
connected to ovn-sbctl's stdout.
Signed-off-by: Ben Pfaff
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1965780
Reported-by: Alexey Roytman
---
utilities/ovn-sbctl.c | 151 ++
1 file changed, 79 insertions(+), 72 deletions(-)
diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
index 53f10cdd897a..4146384e74fb 100644
--- a/utilities/ovn-sbctl.c
+++ b/utilities/ovn-sbctl.c
@@ -618,7 +618,8 @@ sbctl_open_vconn(struct shash *options)
}
static void
-sbctl_dump_openflow(struct vconn *vconn, const struct uuid *uuid, bool stats)
+sbctl_dump_openflow(struct vconn *vconn, const struct uuid *uuid, bool stats,
+struct ds *s)
{
struct ofputil_flow_stats_request fsr = {
.cookie = htonll(uuid->parts[0]),
@@ -639,28 +640,26 @@ sbctl_dump_openflow(struct vconn *vconn, const struct
uuid *uuid, bool stats)
}
if (n_fses) {
-struct ds s = DS_EMPTY_INITIALIZER;
for (size_t i = 0; i < n_fses; i++) {
const struct ofputil_flow_stats *fs = [i];
-ds_clear();
+ds_put_cstr(s, "");
if (stats) {
-ofputil_flow_stats_format(, fs, NULL, NULL, true);
+ofputil_flow_stats_format(s, fs, NULL, NULL, true);
} else {
-ds_put_format(, "%stable=%s%"PRIu8" ",
+ds_put_format(s, "%stable=%s%"PRIu8" ",
colors.special, colors.end, fs->table_id);
-match_format(>match, NULL, , OFP_DEFAULT_PRIORITY);
-if (ds_last() != ' ') {
-ds_put_char(, ' ');
+match_format(>match, NULL, s, OFP_DEFAULT_PRIORITY);
+if (ds_last(s) != ' ') {
+ds_put_char(s, ' ');
}
-ds_put_format(, "%sactions=%s", colors.actions, colors.end);
-struct ofpact_format_params fp = { .s = };
+ds_put_format(s, "%sactions=%s", colors.actions, colors.end);
+struct ofpact_format_params fp = { .s = s };
ofpacts_format(fs->ofpacts, fs->ofpacts_len, );
}
-printf("%s\n", ds_cstr());
+ds_put_char(s, '\n');
}
-ds_destroy();
}
for (size_t i = 0; i < n_fses; i++) {
@@ -670,37 +669,37 @@ sbctl_dump_openflow(struct vconn *vconn, const struct
uuid *uuid, bool stats)
}
static void
-print_datapath_name(const struct sbrec_datapath_binding *dp)
+print_datapath_name(const struct sbrec_datapath_binding *dp, struct ds *s)
{
const struct smap *ids = >external_ids;
const char *name = smap_get(ids, "name");
const char *name2 = smap_get(ids, "name2");
if (name && name2) {
-printf("\"%s\" aka \"%s\"", name, name2);
+ds_put_format(s, "\"%s\" aka \"%s\"", name, name2);
} else if (name || name2) {
-printf("\"%s\"", name ? name : name2);
+ds_put_format(s, "\"%s\"", name ? name : name2);
}
}
static void
print_vflow_datapath_name(const struct sbrec_datapath_binding *dp,
- bool do_print)
+ bool do_print, struct ds *s)
{
if (!do_print) {
return;
}
-printf("datapath=");
-print_datapath_name(dp);
-printf(", ");
+ds_put_cstr(s, "datapath=");
+print_datapath_name(dp, s);
+ds_put_cstr(s, ", ");
}
static void
-print_uuid_part(const struct uuid *uuid, bool do_print)
+print_uuid_part(const struct uuid *uuid, bool do_print, struct ds *s)
{
if (!do_print) {
return;
}
-printf("uuid=0x%08"PRIx32", ", uuid->parts[0]);
+ds_put_format(s, "uuid=0x%08"PRIx32", ", uuid->parts[0]);
}
static void
@@ -717,16 +716,17 @@ cmd_lflow_list_port_bindings(struct ctl_context *ctx,
struct vconn *vconn,
}
if (!pb_prev) {
-printf("\nPort Bindings:\n");
+ds_put_cstr(>output, "\nPort Bindings:\n");
}
-printf(" ");
-print_uuid_part(>header_.uuid, print_uuid);
-print_vflow_datapath_name(pb->datapath, !datapath);
-printf("logical_port=%s, tunnel_key=%-5"PRId64"\n",
- pb->logical_port, pb->tunnel_key);
+ds_put_cstr(>output, " ");
+print_uuid_part(>header_.uuid, print_uuid, >output);
+print_vflow_datapath_name(pb->datapath, !datapath, >output);
+ds_put_format(>output,
+ "logical_port=%s,
Re: [ovs-dev] [PATCH ovn v8 5/6] northd: Add options to automatically add routes for NATs and LBs.
Bleep bloop. Greetings Mark Michelson, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line has trailing whitespace #239 FILE: ovn-nb.xml:2051: load balancer IP addresses to the appropriate MAC address. Setting WARNING: Line lacks whitespace around operator #365 FILE: utilities/ovn-nbctl.c:370: [--add-route]\n\ Lines checked: 448, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v8 4/6] northd: Add IP routing and ARP resolution flows for NAT/LB addresses.
Bleep bloop. Greetings Mark Michelson, I am a robot and I have tried out your
patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
ERROR: Inappropriate bracing around statement
#96 FILE: northd/ovn-northd.c:1473:
if (!extract_addresses(nats[i], [i], )){
WARNING: Line is 90 characters long (recommended limit is 79)
#219 FILE: northd/ovn-northd.c:10174:
ds_put_format(match, "outport == %s && "REG_NEXT_HOP_IPV4" == {",
peer->json_key);
Lines checked: 568, Warnings: 1, Errors: 1
Please check this out. If you feel there has been an error, please email
acon...@redhat.com
Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v8 6/6] northd: Flood ARPs to routers for "unreachable" addresses.
Previously, ARP TPAs were filtered down only to "reachable" addresses.
Reachable addresses are all router interface addresses, as well as NAT
external addresses and load balancer VIPs that are within the subnet
handled by a router's port.
However, it is possible that in some configurations, CMSes purposely
configure NAT or load balancer addresses on a router that are outside
the router's subnets, and they expect the router to respond to ARPs for
those addresses.
This commit adds a higher priority flow to logical switches that makes
it so ARPs targeted at "unreachable" addresses are flooded to all ports.
This way, the ARPs can reach the router appropriately and receive a
response.
Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1929901
Signed-off-by: Mark Michelson
---
northd/ovn-northd.8.xml | 8 +++
northd/ovn-northd.c | 153 +++-
northd/ovn_northd.dl| 101 --
tests/ovn-northd.at | 99 ++
tests/system-ovn.at | 102 +++
5 files changed, 391 insertions(+), 72 deletions(-)
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index bb77689de..8bb77bf6c 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1549,6 +1549,14 @@ output;
logical ports.
+
+Priority-80 flows for each IP address/VIP/NAT address configured
+outside its owning router port's subnet. These flows match ARP
+requests and ND packets for the specific IP addresses. Matched packets
+are forwarded to the MC_FLOOD multicast group which
+contains all connected logical ports.
+
+
Priority-75 flows for each port connected to a logical router
matching self originated ARP request/ND packets. These packets
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 414bf9c48..eacbab96a 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6539,44 +6539,48 @@ build_lswitch_rport_arp_req_self_orig_flow(struct
ovn_port *op,
ds_destroy();
}
-/*
- * Ingress table 19: Flows that forward ARP/ND requests only to the routers
- * that own the addresses. Other ARP/ND packets are still flooded in the
- * switching domain as regular broadcast.
- */
static void
-build_lswitch_rport_arp_req_flow_for_ip(struct sset *ips,
-int addr_family,
-struct ovn_port *patch_op,
-struct ovn_datapath *od,
-uint32_t priority,
-struct hmap *lflows,
-const struct ovsdb_idl_row *stage_hint)
+arp_nd_ns_match(struct sset *ips, int addr_family, struct ds *match)
{
-struct ds match = DS_EMPTY_INITIALIZER;
-struct ds actions = DS_EMPTY_INITIALIZER;
/* Packets received from VXLAN tunnels have already been through the
* router pipeline so we should skip them. Normally this is done by the
* multicast_group implementation (VXLAN packets skip table 32 which
* delivers to patch ports) but we're bypassing multicast_groups.
*/
-ds_put_cstr(, FLAGBIT_NOT_VXLAN " && ");
+ds_put_cstr(match, FLAGBIT_NOT_VXLAN " && ");
if (addr_family == AF_INET) {
-ds_put_cstr(, "arp.op == 1 && arp.tpa == { ");
+ds_put_cstr(match, "arp.op == 1 && arp.tpa == {");
} else {
-ds_put_cstr(, "nd_ns && nd.target == { ");
+ds_put_cstr(match, "nd_ns && nd.target == {");
}
const char *ip_address;
SSET_FOR_EACH (ip_address, ips) {
-ds_put_format(, "%s, ", ip_address);
+ds_put_format(match, "%s, ", ip_address);
}
-ds_chomp(, ' ');
-ds_chomp(, ',');
-ds_put_cstr(, "}");
+ds_chomp(match, ' ');
+ds_chomp(match, ',');
+ds_put_cstr(match, "}");
+}
+
+/*
+ * Ingress table 19: Flows that forward ARP/ND requests only to the routers
+ * that own the addresses. Other ARP/ND packets are still flooded in the
+ * switching domain as regular broadcast.
+ */
+static void
+build_lswitch_rport_arp_req_flow_for_reachable_ip(struct sset *ips,
+int addr_family, struct ovn_port *patch_op, struct ovn_datapath *od,
+uint32_t priority, struct hmap *lflows,
+const struct ovsdb_idl_row *stage_hint)
+{
+struct ds match = DS_EMPTY_INITIALIZER;
+struct ds actions = DS_EMPTY_INITIALIZER;
+
+arp_nd_ns_match(ips, addr_family, );
/* Send a the packet to the router pipeline. If the switch has non-router
* ports then flood it there as well.
@@ -6599,6 +6603,30 @@ build_lswitch_rport_arp_req_flow_for_ip(struct sset *ips,
ds_destroy();
}
+/*
+ * Ingress table 19: Flows that forward ARP/ND requests for "unreachable" IPs
+ * (NAT or load balancer IPs configured on a router that are outside the
+ * router's configured subnets).
+ * These ARP/ND
[ovs-dev] [PATCH ovn v8 4/6] northd: Add IP routing and ARP resolution flows for NAT/LB addresses.
Dealing with NAT and load balancer IPs has been a bit of a pain point.
It requires creating static routes if east-west traffic to those
addresses is desired. Further, it requires ARPs to be sent between the
logical routers in order to create MAC Bindings.
This commit seeks to make things easier. NAT and load balancer addresess
automatically have IP routing logical flows and ARP resolution logical
flows created for reachable routers. This eliminates the need to create
static routes, and it also eliminates the need for ARPs to be sent
between logical routers.
In this commit, the behavior is not optional. The next commit will
introduce configuration to make the behavior optional.
Signed-off-by: Mark Michelson
---
northd/ovn-northd.c | 133 ++-
northd/ovn_northd.dl | 57
tests/ovn-northd.at | 214 +++
3 files changed, 399 insertions(+), 5 deletions(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index ef4f5b790..3b9cad80b 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -1353,6 +1353,21 @@ build_datapaths(struct northd_context *ctx, struct hmap
*datapaths,
}
}
+/* Structure representing logical router port
+ * routable addresses. This includes DNAT and Load Balancer
+ * addresses. This structure will only be filled in if the
+ * router port is a gateway router port. Otherwise, all pointers
+ * will be NULL and n_addrs will be 0.
+ */
+struct ovn_port_routable_addresses {
+/* Array of address strings suitable for writing to a database table */
+char **addresses;
+/* The addresses field parsed into component parts */
+struct lport_addresses *laddrs;
+/* Number of items in each of the above arrays */
+size_t n_addrs;
+};
+
/* A logical switch port or logical router port.
*
* In steady state, an ovn_port points to a northbound Logical_Switch_Port
@@ -1396,6 +1411,8 @@ struct ovn_port {
struct lport_addresses lrp_networks;
+struct ovn_port_routable_addresses routables;
+
/* Logical port multicast data. */
struct mcast_port_info mcast_info;
@@ -1422,6 +1439,48 @@ struct ovn_port {
struct ovs_list list; /* In list of similar records. */
};
+static void
+destroy_routable_addresses(struct ovn_port_routable_addresses *ra)
+{
+if (ra->n_addrs == 0) {
+return;
+}
+
+for (size_t i = 0; i < ra->n_addrs; i++) {
+free(ra->addresses[i]);
+destroy_lport_addresses(>laddrs[i]);
+}
+free(ra->addresses);
+free(ra->laddrs);
+}
+
+static char **get_nat_addresses(const struct ovn_port *op, size_t *n);
+
+static void
+assign_routable_addresses(struct ovn_port *op)
+{
+size_t n;
+char **nats = get_nat_addresses(op, );
+
+if (!nats) {
+return;
+}
+
+struct lport_addresses *laddrs = xcalloc(n, sizeof(*laddrs));
+for (size_t i = 0; i < n; i++) {
+int ofs;
+if (!extract_addresses(nats[i], [i], )){
+continue;
+}
+}
+
+/* Everything seems to have worked out */
+op->routables.addresses = nats;
+op->routables.laddrs = laddrs;
+op->routables.n_addrs = n;
+}
+
+
static void
ovn_port_set_nb(struct ovn_port *op,
const struct nbrec_logical_switch_port *nbsp,
@@ -1471,6 +1530,8 @@ ovn_port_destroy(struct hmap *ports, struct ovn_port
*port)
}
free(port->ps_addrs);
+destroy_routable_addresses(>routables);
+
destroy_lport_addresses(>lrp_networks);
free(port->json_key);
free(port->key);
@@ -2378,6 +2439,8 @@ join_logical_ports(struct northd_context *ctx,
* use during flow creation. */
od->l3dgw_port = op;
od->l3redirect_port = crp;
+
+assign_routable_addresses(op);
}
}
}
@@ -2501,7 +2564,7 @@ get_nat_addresses(const struct ovn_port *op, size_t *n)
{
size_t n_nats = 0;
struct eth_addr mac;
-if (!op->nbrp || !op->od || !op->od->nbr
+if (!op || !op->nbrp || !op->od || !op->od->nbr
|| (!op->od->nbr->n_nat && !op->od->nbr->n_load_balancer)
|| !eth_addr_from_string(op->nbrp->mac, )) {
*n = n_nats;
@@ -3089,7 +3152,6 @@ ovn_port_update_sbrec(struct northd_context *ctx,
} else {
sbrec_port_binding_set_options(op->sb, NULL);
}
-
const char *nat_addresses = smap_get(>nbsp->options,
"nat-addresses");
size_t n_nats = 0;
@@ -3145,6 +3207,7 @@ ovn_port_update_sbrec(struct northd_context *ctx,
if (add_router_port_garp) {
struct ds garp_info = DS_EMPTY_INITIALIZER;
ds_put_format(_info, "%s", op->peer->lrp_networks.ea_s);
+
for (size_t i = 0; i < op->peer->lrp_networks.n_ipv4_addrs;
i++) {
[ovs-dev] [PATCH ovn v8 3/6] northd: Factor peer retrieval into its own function.
The same pattern is repeated several times throughout ovn-northd.c, so
this puts it in its own function. This will be used even more in an
upcoming commit.
Signed-off-by: Mark Michelson
---
northd/ovn-northd.c | 70 -
1 file changed, 24 insertions(+), 46 deletions(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 46c590d70..ef4f5b790 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -1546,6 +1546,21 @@ lrport_is_enabled(const struct nbrec_logical_router_port
*lrport)
return !lrport->enabled || *lrport->enabled;
}
+static struct ovn_port *
+ovn_port_get_peer(struct hmap *ports, struct ovn_port *op)
+{
+if (!op->nbsp || !lsp_is_router(op->nbsp) || op->derived) {
+return NULL;
+}
+
+const char *peer_name = smap_get(>nbsp->options, "router-port");
+if (!peer_name) {
+return NULL;
+}
+
+return ovn_port_find(ports, peer_name);
+}
+
static void
ipam_insert_ip_for_datapath(struct ovn_datapath *od, uint32_t ip)
{
@@ -2373,12 +2388,7 @@ join_logical_ports(struct northd_context *ctx,
struct ovn_port *op;
HMAP_FOR_EACH (op, key_node, ports) {
if (op->nbsp && lsp_is_router(op->nbsp) && !op->derived) {
-const char *peer_name = smap_get(>nbsp->options,
"router-port");
-if (!peer_name) {
-continue;
-}
-
-struct ovn_port *peer = ovn_port_find(ports, peer_name);
+struct ovn_port *peer = ovn_port_get_peer(ports, op);
if (!peer || !peer->nbrp) {
continue;
}
@@ -10172,14 +10182,8 @@ build_arp_resolve_flows_for_lrouter_port(
/* Get the Logical_Router_Port that the
* Logical_Switch_Port is connected to, as
* 'peer'. */
-const char *peer_name = smap_get(
->od->router_ports[k]->nbsp->options,
-"router-port");
-if (!peer_name) {
-continue;
-}
-
-struct ovn_port *peer = ovn_port_find(ports, peer_name);
+struct ovn_port *peer = ovn_port_get_peer(
+ports, op->od->router_ports[k]);
if (!peer || !peer->nbrp) {
continue;
}
@@ -10209,14 +10213,8 @@ build_arp_resolve_flows_for_lrouter_port(
/* Get the Logical_Router_Port that the
* Logical_Switch_Port is connected to, as
* 'peer'. */
-const char *peer_name = smap_get(
->od->router_ports[k]->nbsp->options,
-"router-port");
-if (!peer_name) {
-continue;
-}
-
-struct ovn_port *peer = ovn_port_find(ports, peer_name);
+struct ovn_port *peer = ovn_port_get_peer(
+ports, op->od->router_ports[k]);
if (!peer || !peer->nbrp) {
continue;
}
@@ -10264,14 +10262,8 @@ build_arp_resolve_flows_for_lrouter_port(
!op->sb->chassis) {
/* The virtual port is not claimed yet. */
for (size_t i = 0; i < op->od->n_router_ports; i++) {
-const char *peer_name = smap_get(
->od->router_ports[i]->nbsp->options,
-"router-port");
-if (!peer_name) {
-continue;
-}
-
-struct ovn_port *peer = ovn_port_find(ports, peer_name);
+struct ovn_port *peer = ovn_port_get_peer(
+ports, op->od->router_ports[i]);
if (!peer || !peer->nbrp) {
continue;
}
@@ -10306,15 +10298,8 @@ build_arp_resolve_flows_for_lrouter_port(
/* Get the Logical_Router_Port that the
* Logical_Switch_Port is connected to, as
* 'peer'. */
-const char *peer_name = smap_get(
->od->router_ports[j]->nbsp->options,
-"router-port");
-if (!peer_name) {
-continue;
-}
-
struct ovn_port *peer =
-ovn_port_find(ports, peer_name);
+ovn_port_get_peer(ports, vp->od->router_ports[j]);
if (!peer || !peer->nbrp) {
continue;
}
@@ -10351,14 +10336,7 @@ build_arp_resolve_flows_for_lrouter_port(
* we need to add logical flows such that it can resolve
* ARP entries for all the other router ports connected to
* the switch in question. */
-
-const char *peer_name
[ovs-dev] [PATCH ovn v8 5/6] northd: Add options to automatically add routes for NATs and LBs.
Load_Balancer and NAT entries have a new option, "add_route" that can be
set to automatically add routes to those addresses to neighbor routers,
therefore eliminating the need to create static routes.
Signed-off-by: Mark Michelson
---
northd/ovn-northd.c | 29 +
northd/ovn_northd.dl | 23 ---
ovn-nb.xml| 29 -
tests/ovn-nbctl.at| 3 +++
tests/ovn-northd.at | 40
utilities/ovn-nbctl.c | 25 +++--
6 files changed, 123 insertions(+), 26 deletions(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 3b9cad80b..414bf9c48 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -1454,13 +1454,14 @@ destroy_routable_addresses(struct
ovn_port_routable_addresses *ra)
free(ra->laddrs);
}
-static char **get_nat_addresses(const struct ovn_port *op, size_t *n);
+static char **get_nat_addresses(const struct ovn_port *op, size_t *n,
+bool routable_only);
static void
assign_routable_addresses(struct ovn_port *op)
{
size_t n;
-char **nats = get_nat_addresses(op, );
+char **nats = get_nat_addresses(op, , true);
if (!nats) {
return;
@@ -2510,7 +2511,7 @@ join_logical_ports(struct northd_context *ctx,
}
static void
-get_router_load_balancer_ips(const struct ovn_datapath *od,
+get_router_load_balancer_ips(const struct ovn_datapath *od, bool routable_only,
struct sset *all_ips_v4, struct sset *all_ips_v6)
{
if (!od->nbr) {
@@ -2522,6 +2523,11 @@ get_router_load_balancer_ips(const struct ovn_datapath
*od,
struct smap *vips = >vips;
struct smap_node *node;
+if (routable_only &&
+!smap_get_bool(>options, "add_route", false)) {
+continue;
+}
+
SMAP_FOR_EACH (node, vips) {
/* node->key contains IP:port or just IP. */
char *ip_address;
@@ -2560,7 +2566,7 @@ get_router_load_balancer_ips(const struct ovn_datapath
*od,
* The caller must free each of the n returned strings with free(),
* and must free the returned array when it is no longer needed. */
static char **
-get_nat_addresses(const struct ovn_port *op, size_t *n)
+get_nat_addresses(const struct ovn_port *op, size_t *n, bool routable_only)
{
size_t n_nats = 0;
struct eth_addr mac;
@@ -2583,6 +2589,12 @@ get_nat_addresses(const struct ovn_port *op, size_t *n)
const struct nbrec_nat *nat = op->od->nbr->nat[i];
ovs_be32 ip, mask;
+if (routable_only &&
+(!strcmp(nat->type, "snat") ||
+ !smap_get_bool(>options, "add_route", false))) {
+continue;
+}
+
char *error = ip_parse_masked(nat->external_ip, , );
if (error || mask != OVS_BE32_MAX) {
free(error);
@@ -2636,7 +2648,8 @@ get_nat_addresses(const struct ovn_port *op, size_t *n)
/* Two sets to hold all load-balancer vips. */
struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
-get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
+get_router_load_balancer_ips(op->od, routable_only,
+ _ips_v4, _ips_v6);
const char *ip_address;
SSET_FOR_EACH (ip_address, _ips_v4) {
@@ -3159,7 +3172,7 @@ ovn_port_update_sbrec(struct northd_context *ctx,
if (nat_addresses && !strcmp(nat_addresses, "router")) {
if (op->peer && op->peer->od
&& (chassis || op->peer->od->l3redirect_port)) {
-nats = get_nat_addresses(op->peer, _nats);
+nats = get_nat_addresses(op->peer, _nats, false);
}
/* Only accept manual specification of ethernet address
* followed by IPv4 addresses on type "l3gateway" ports. */
@@ -6615,7 +6628,7 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op,
struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
-get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
+get_router_load_balancer_ips(op->od, false, _ips_v4, _ips_v6);
const char *ip_addr;
const char *ip_addr_next;
@@ -11177,7 +11190,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
/* A set to hold all load-balancer vips that need ARP responses. */
struct sset all_ips_v4 = SSET_INITIALIZER(_ips_v4);
struct sset all_ips_v6 = SSET_INITIALIZER(_ips_v6);
-get_router_load_balancer_ips(op->od, _ips_v4, _ips_v6);
+get_router_load_balancer_ips(op->od, false, _ips_v4, _ips_v6);
const char *ip_address;
if (sset_count(_ips_v4)) {
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 610bfbfb2..794b86ee1 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -201,7 +201,7 @@
[ovs-dev] [PATCH ovn v8 2/6] ovn-sb: Remove redundant "nat-addresses" information from Port_Binding.
Signed-off-by: Mark Michelson --- ovn-sb.xml | 10 -- 1 file changed, 10 deletions(-) diff --git a/ovn-sb.xml b/ovn-sb.xml index 258a12b4e..b853a5031 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -3015,16 +3015,6 @@ tcp.flags = RST; The chassis in which the port resides. - -MAC address of the l3gateway port followed by a list of -SNAT and DNAT external IP addresses. This is used to send gratuitous -ARPs for SNAT and DNAT external IP addresses via localnet. -Example: 80:fa:5b:06:72:b7 158.36.44.22 158.36.44.24. -This would result in generation of gratuitous ARPs for IP addresses -158.36.44.22 and 158.36.44.24 with a MAC address of 80:fa:5b:06:72:b7. -This is used in OVS versions prior to 2.8. - - MAC address of the l3gateway port followed by a list of SNAT and DNAT external IP addresses. This is used to send gratuitous -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v8 1/6] northd: Swap src and dst eth addresses in router egress loop.
When a hairpin scenario is detected (i.e. egressing on gateway router
port to a NAT external address), we loop back to the ingress router
pipeline and swap the inport and outport. This is intended to allow for
the traffic to get DNATted. In practice, though, the ethernet
destination has not been updated to be the distributed gateway port's
address, and so the packet is immediately dropped.
This fixes the issue by swapping source and dest eth addressses when
doing the hairpin redirection. This way, the packet appears to be
arriving on the gateway port, and it gets handled as expected.
Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1929901
Signed-off-by: Mark Michelson
---
northd/ovn-northd.c | 1 +
northd/ovn_northd.dl | 1 +
tests/system-ovn.at | 113 ++-
3 files changed, 114 insertions(+), 1 deletion(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index bf09eed59..46c590d70 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -11809,6 +11809,7 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
ds_put_format(actions,
"clone { ct_clear; "
"inport = outport; outport = \"\"; "
+ "eth.dst <-> eth.src; "
"flags = 0; flags.loopback = 1; ");
for (int j = 0; j < MFF_N_LOG_REGS; j++) {
ds_put_format(actions, "reg%d = 0; ", j);
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index cb8418540..60ace7e61 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -5924,6 +5924,7 @@ for (r in (._uuid = lr_uuid,
var actions =
"clone { ct_clear; "
"inport = outport; outport = \"\"; "
+"eth.dst <-> eth.src; "
"flags = 0; flags.loopback = 1; " ++
regs.join("") ++
"${rEGBIT_EGRESS_LOOPBACK()} = 1; "
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 76fc21c87..3824788c4 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -6054,6 +6054,7 @@ AT_CLEANUP
])
OVN_FOR_EACH_NORTHD([
+<<< HEAD
AT_SETUP([ovn -- No ct_state matches in dp flows when no ACLs in an LS])
AT_KEYWORDS([no ct_state match])
ovn_start
@@ -6092,7 +6093,6 @@ check ovn-nbctl pg-add pg1 sw1-p1
check ovn-nbctl acl-add pg1 from-lport 1002 "ip" allow-related
check ovn-nbctl acl-add pg1 to-lport 1002 "ip" allow-related
-
OVN_POPULATE_ARP
ovn-nbctl --wait=hv sync
@@ -6180,5 +6180,116 @@ OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
as
OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
/connection dropped.*/d"])
+
+AT_CLEANUP
+])
+
+AT_SETUP(ovn -- DNAT LR hairpin IPv4)
+AT_KEYWORDS(hairpin)
+
+ovn_start
+
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+-- set Open_vSwitch . external-ids:system-id=hv1 \
+-- set Open_vSwitch .
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+-- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+-- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+-- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+start_daemon ovn-controller
+
+# Logical network:
+# Two VMs
+# * VM1 with IP address 192.168.100.5
+# * VM2 with IP address 192.168.100.6
+# The VMs connect to logical switch ls1.
+#
+# An external router with IP address 172.18.1.2. We simulate this with a
network namespace.
+# There will be no traffic going here in this test.
+# The external router connects to logical switch ls-pub
+#
+# One logical router (lr1) connects to ls1 and ls-pub. The router port
connected to ls-pub is
+# a gateway port.
+# * The subnet connected to ls1 is 192.168.100.0/24. The Router IP address
is 192.168.100.1
+# * The subnet connected to ls-pub is 172.18.1.0/24. The Router IP address
is 172.168.1.1
+# lr1 has the following attributes:
+# * It has a "default" static route that sends traffic out the gateway
router port.
+# * It has a DNAT rule that translates 172.18.2.10 to 192.168.100.6 (VM2)
+#
+# In this test, we want to ensure that a ping from VM1 to IP address
172.18.2.10 reaches VM2.
+
+ovn-nbctl ls-add ls1
+ovn-nbctl lsp-add ls1 vm1 -- lsp-set-addresses vm1 "00:00:00:00:00:05
192.168.100.5"
+ovn-nbctl lsp-add ls1 vm2 -- lsp-set-addresses vm2 "00:00:00:00:00:06
192.168.100.6"
+
+ovn-nbctl ls-add ls-pub
+ovn-nbctl lsp-add ls-pub ext-router -- lsp-set-addresses ext-router
"00:00:00:00:01:02 172.18.1.2"
+
+ovn-nbctl lr-add lr1
+ovn-nbctl lrp-add lr1 lr1-ls1 00:00:00:00:00:01 192.168.100.1/24
+ovn-nbctl lsp-add ls1 ls1-lr1 \
+-- lsp-set-type ls1-lr1 router \
+-- lsp-set-addresses ls1-lr1 00:00:00:00:00:01 \
+-- lsp-set-options ls1-lr1 router-port=lr1-ls1
+
+ovn-nbctl lrp-add lr1 lr1-ls-pub 00:00:00:00:01:01 172.18.1.1/24
+ovn-nbctl
[ovs-dev] [PATCH ovn v8 0/6] ARP and Floating IP Fixes
This patch series aims to fix issues seen in OpenStack deployments when floating IPs were assigned to routers, and those floating IPs were not part of any subnet configured on that router. Originally, this was a two patch series but it has bloomed into a 5 patch series. After v7, a new approach was suggested and it's now 6 patches. Patch 1 fixes the scenario where a VM attempts to reach a floating IP on the directly connected router. This has been part of this patch series since v1. Patch 2 is an incidental fix that removes a redundant paragraph from documenttion. Patch 3 is a small cleanup in ovn-northd.c to factor out peer retrieval into its own function. Patch 4 alters northd to install logical flows to make it so that routers can reach NAT and load balancer addresses on their neighbors without the need to configure static routes or MAC bindings. Patch 5 recognizes that patch 4 may not always be desired, so it makes the behavior opt-in. Finally, patch 6 addresses the situation for when the pre-allocated logical flows cannot be used. For this situation, we will flood the ARP request if the TPA is for a configured IP address that is outside the connected routers' subnets. --- v7 -> v8: First 2 patches are the same as they have been in previous versions. Patch 6 is nearly identical to patch 5 from previous versions. Patches 3, 4, and 5 represent a completely new approach to solving the issue from before. And that's also why previous version are not documented here. --- Mark Michelson (6): northd: Swap src and dst eth addresses in router egress loop. ovn-sb: Remove redundant "nat-addresses" information from Port_Binding. northd: Factor peer retrieval into its own function. northd: Add IP routing and ARP resolution flows for NAT/LB addresses. northd: Add options to automatically add routes for NATs and LBs. northd: Flood ARPs to routers for "unreachable" addresses. northd/ovn-northd.8.xml | 8 + northd/ovn-northd.c | 380 +--- northd/ovn_northd.dl| 180 --- ovn-nb.xml | 29 ++- ovn-sb.xml | 10 -- tests/ovn-nbctl.at | 3 + tests/ovn-northd.at | 345 tests/system-ovn.at | 215 ++- utilities/ovn-nbctl.c | 25 ++- 9 files changed, 1043 insertions(+), 152 deletions(-) -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 1/2] Prepare for 21.06.0
On 6/1/21 7:45 AM, Numan Siddique wrote: On Tue, Jun 1, 2021 at 6:30 AM Ilya Maximets wrote: On 5/27/21 9:55 PM, Mark Michelson wrote: Signed-off-by: Mark Michelson --- NEWS | 2 +- configure.ac | 2 +- debian/changelog | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) Hi, Mark. We discussed during the IRC meeting on Thursday that ovs submodule bump [1] and my patch for memory re-allocation in northd [2] should be applied before branching, but I don't see them on current master. Is there any problem with these patches? Can they still be accepted before branching? Best regards, Ilya Maximets. [1] https://patchwork.ozlabs.org/project/ovn/patch/20210518064932.23852-1-dce...@redhat.com/ [2] https://patchwork.ozlabs.org/project/ovn/patch/20210520190219.758420-1-i.maxim...@ovn.org/ Hi Ilya, I applied both these patches. Thanks Numan I've now created the 21.06 branch and merged the patches. ___ 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
Re: [ovs-dev] [PATCH v2 ovn 0/4] respin CoPP series
> On 24/05/2021 19:44, Lorenzo Bianconi wrote: > > This series respin CoPP support introduced here [0] by Dumitru rebasing on > > top > > of ovn master branch and adding some missing meters (e.g. bfd or acl > > reject). > > The main goal of this series is to continue the discussion about the > > proposed > > approach and to align on CMS APIs. > > For the moment no ddlog support has been added. > > Related bz: > > https://bugzilla.redhat.com/show_bug.cgi?id=1947913 > > https://bugzilla.redhat.com/show_bug.cgi?id=1946610 > > I mentioned the possibility of adding a tutorial. Is that something you > will do in a separate patch? Hi Mark, yes, my intention is to add it in a dedicated patch (sorry to not be clear on it). Regards, Lorenzo > > > > Changes since v1: > > - merge patch 3/5 and 4/5 > > - cosmetics > > - improve naming conventions > > - add more unit-tests/system-tests > > - remove duplicated flow > > - remove some leftover entries in ovn-nbctl.8.xml > > - add metering for sctp abort packets > > > > Changes since RFC: > > - drop per-port metering > > - add unit/system tests > > - add reject action metering > > > > [0] > > https://patchwork.ozlabs.org/project/openvswitch/list/?series=140778=* > > > > Dumitru Ceara (4): > > ovn-controller: Add support for Logical_Flow control meters > > ovn-northd: Add support for CoPP. > > ovn-northd: Add CoPP policies for flows that punt packets to > > ovn-controller. > > NEWS: Add CoPP support. > > > > NEWS | 1 + > > controller/lflow.c| 40 ++- > > controller/ofctrl.c | 55 ++-- > > controller/ofctrl.h | 21 +- > > controller/physical.c | 7 +- > > include/ovn/actions.h | 3 +- > > lib/actions.c | 116 - > > lib/automake.mk | 2 + > > lib/copp.c| 143 ++ > > lib/copp.h| 59 + > > northd/ovn-northd.c | 535 -- > > ovn-nb.ovsschema | 16 +- > > ovn-nb.xml| 81 ++ > > ovn-sb.ovsschema | 6 +- > > ovn-sb.xml| 6 + > > tests/atlocal.in | 3 + > > tests/ovn-controller.at | 48 > > tests/ovn-northd.at | 81 ++ > > tests/ovn.at | 4 +- > > tests/system-ovn.at | 135 ++ > > utilities/ovn-nbctl.8.xml | 119 + > > utilities/ovn-nbctl.c | 178 + > > 22 files changed, 1360 insertions(+), 299 deletions(-) > > create mode 100644 lib/copp.c > > create mode 100644 lib/copp.h > > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-northd.c: Add proxy ARP support to OVN
On Wed, 2021-06-02 at 17:02 +0100, Brendan Doyle wrote:
> Â From a9d3140845175edb7644b2d0d82a95bd6cf94662 Mon Sep 17 00:00:00 2001
> From: Brendan Doyle
> Date: Fri, 28 May 2021 10:01:17 -0700
> Subject: [PATCH ovn] ovn-northd.c: Add proxy ARP support to OVN
>
> This patch provides the ability to configure proxy ARP IPs on a Logical
> Switch Router port. The IPs are added as Options for router ports. This
> provides a useful feature where traffic for a service must be sent to
> an
> address in a logical network address space, but the service is provided
> in a different network. For example an NFS service is provide to
> Logical
> networks at an address in their Logical network space, but the NFS
> server resides in a physical network. A Logical switch Router port can
> be configured to respond to ARP requests sent to the service "Logical
> address", the Logical Router/Gateway can then be configured to forward
> the traffic to the underlay/physical network.
>
> Signed-off-by: Brendan Doyle
> ---
> Â northd/ovn-northd.c |Â 38 +++
>  ovn-nb.xml |  9 +
>  tests/ovn.at   | 103
>
> Â 3 files changed, 150 insertions(+)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 0e5092a..a377e83 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -6943,6 +6943,7 @@ build_lswitch_arp_nd_responder_known_ips(struct
> ovn_port *op,
> Â Â struct ds *match)
> Â {
> Â if (op->nbsp) {
> +Â Â Â const char *arp_proxy;
> Â if (!strcmp(op->nbsp->type, "virtual")) {
> Â /* Handle
> Â Â *Â - GARPs for virtual ip which belongs to a logical
> port
> @@ -7096,6 +7097,43 @@ build_lswitch_arp_nd_responder_known_ips(struct
> ovn_port *op,
> Â }
> Â }
> Â }
> +
> +Â Â Â /*
> + * Add responses for ARP proxies.
> + */
> +Â Â Â arp_proxy = smap_get(>nbsp->options,"arp_proxy");
> +Â Â Â if (arp_proxy && op->peer) {
> +Â Â Â char *ips, *ip, *rest;
> +
> +Â Â Â ips = xstrdup(arp_proxy);
> +Â Â Â rest = ips;
> +
> +Â Â Â while ((ip = strtok_r(rest,",", ))) {
> +Â Â Â ds_clear(match);
> +Â Â Â ds_put_format(match, "arp.tpa == %s && arp.op == 1",
> ip);
> +
> +Â Â Â ds_clear(actions);
> +Â Â Â ds_put_format(actions,
> +Â Â Â "eth.dst = eth.src; "
> +Â Â Â "eth.src = %s; "
> +Â Â Â "arp.op = 2; /* ARP reply */ "
> +Â Â Â "arp.tha = arp.sha; "
> +Â Â Â "arp.sha = %s; "
> +Â Â Â "arp.tpa = arp.spa; "
> +Â Â Â "arp.spa = %s; "
I think you can collapse the two lines above into:
"arp.tpa <-> arp.spa; "
and get rid of one %s. For an example, see build_lrouter_arp_flow().
In fact, you might be able to reduce the number of lflows down to one
by doing something like the following, since now your action is
constant due to the <->.
int i = 0;
ds_clear();
ds_put_cstr(match, "arp.op == 1 && (");
while ((ip = strtok_r(rest,",", ))) {
if (i++ > 0) {
ds_put_cstr(match, " || ");
};
ds_put_format(match, "arp.tpa == %s", ip);
}
ds_put_cstr(match, ")");
An example of that is in build_port_security_ipv6_nd_flow().
---
General comment, this will need a ddlog counterpart too. I'm not an
expert there, so I'll leave it to the OVN team to help. But you could
use Ilya's lrouter arp flow patch as a reference:
http://patchwork.ozlabs.org/project/ovn/patch/20210507162256.3661118-1-i.maxim...@ovn.org/
Looking at that patch, you might need to update the ovn-northd.8
manpage as well?
Thanks!
Dan
> +Â Â Â "outport = inport; "
> +Â Â Â "flags.loopback = 1; "
> +Â Â Â "output;",
> +Â Â Â op->peer->lrp_networks.ea_s,
> +Â Â Â op->peer->lrp_networks.ea_s,
> +Â Â Â ip);
> +
> +Â Â Â ovn_lflow_add_with_hint(lflows, op->od,
> S_SWITCH_IN_ARP_ND_RSP,
> +Â Â Â 50, ds_cstr(match), ds_cstr(actions),
> +Â Â Â >nbsp->header_);
> +Â Â Â }
> +Â Â Â free(ips);
> +Â Â Â }
> Â }
> Â }
>
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 02fd216..4b6c183 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -848,6 +848,15 @@
> Â
> Â Â Â
> Â
> +
> +Â Â Â
> +Â Optional. A comma separated list IPv4 addresses that this
> +Â logical switch router port will reply to ARP
> requests.
> +Â Example: 169.254.239.254,169.254.239.2. The
> +Â 's logical router
> should
> +Â have a route to forward packets sent to configured proxy ARP
> IPs to
> +Â an appropriate destination.
> +Â Â Â
> Â Â Â
>
> Â Â Â
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 2c3c36d..c675cc9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -26527,3 +26527,106 @@
[ovs-dev] [PATCH ovn 4/4] ovn-controller: Handle DNAT/no-NAT conntrack tuple collisions.
Assuming a load balancer, LB1, with:
- VIP: 42.42.42.42:4242
- backend: 42.42.42.1:2121
A client might connect to the backend either directly or through the
VIP. If the first connection is via the VIP and the second connection
is direct but happens to use the same source L4 port, OVN should make
sure that the second connection is SNATed (source port translation) in
order to avoid a tuple collision at commit time.
For example:
1. Session via the VIP:
- original packet: src=42.42.42.2:2000, dst=42.42.42.42:4242
- after DNAT: src=42.42.42.2:2000, dst=42.42.42.1:2121
2. Session directly connected to the backend:
- original packet: src=42.42.42.2:2000, dst=42.42.42.1:2121
- in acl stage committing this connection would fail.
In order to avoid this we now use the all-zero-ip NAT OVS feature when
committing conneections in the ACL stage. This translates to a no-op
SNAT when there's no tuple collision, and performs source port
translation when a tuple collision would happen.
We program flows to perform all-zero-ip NAT conditionally, only if the
datapath being used supports it.
Reported-at: https://bugzilla.redhat.com/1939676
Signed-off-by: Dumitru Ceara
---
controller/lflow.c|1
include/ovn/actions.h |4 +
lib/actions.c | 34 ++
tests/ovn.at |2 -
tests/system-common-macros.at |4 +
tests/system-ovn.at | 138 +
6 files changed, 180 insertions(+), 3 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index 680b8cca1..af6a237d1 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -582,6 +582,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow
*lflow,
.is_switch = datapath_is_switch(dp),
.group_table = l_ctx_out->group_table,
.meter_table = l_ctx_out->meter_table,
+.dp_features = ovs_feature_support_get(),
.lflow_uuid = lflow->header_.uuid,
.pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 040213177..35983498b 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -25,6 +25,7 @@
#include "openvswitch/hmap.h"
#include "openvswitch/uuid.h"
#include "util.h"
+#include "ovn/features.h"
struct expr;
struct lexer;
@@ -756,6 +757,9 @@ struct ovnact_encode_params {
/* A struct to figure out the meter_id for meter actions. */
struct ovn_extend_table *meter_table;
+/* OVS datapath supported features. */
+enum ovs_feature_support dp_features;
+
/* The logical flow uuid that drove this action. */
struct uuid lflow_uuid;
diff --git a/lib/actions.c b/lib/actions.c
index b3433f49e..c8c2443ce 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -732,7 +732,7 @@ format_CT_COMMIT_V1(const struct ovnact_ct_commit_v1 *cc,
struct ds *s)
static void
encode_CT_COMMIT_V1(const struct ovnact_ct_commit_v1 *cc,
-const struct ovnact_encode_params *ep OVS_UNUSED,
+const struct ovnact_encode_params *ep,
struct ofpbuf *ofpacts)
{
struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
@@ -742,6 +742,21 @@ encode_CT_COMMIT_V1(const struct ovnact_ct_commit_v1 *cc,
ct->zone_src.ofs = 0;
ct->zone_src.n_bits = 16;
+/* If the datapath supports all-zero SNAT then use it to avoid tuple
+ * collisions at commit time between NATed and firewalled-only sessions.
+ */
+if (ep->dp_features & OVS_CT_ZERO_SNAT_SUPPORT) {
+size_t nat_offset = ofpacts->size;
+ofpbuf_pull(ofpacts, nat_offset);
+
+struct ofpact_nat *nat = ofpact_put_NAT(ofpacts);
+nat->flags = 0;
+nat->range_af = AF_UNSPEC;
+nat->flags |= NX_NAT_F_SRC;
+ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
+ct = ofpacts->header;
+}
+
size_t set_field_offset = ofpacts->size;
ofpbuf_pull(ofpacts, set_field_offset);
@@ -780,7 +795,7 @@ format_CT_COMMIT_V2(const struct ovnact_nest *on, struct ds
*s)
static void
encode_CT_COMMIT_V2(const struct ovnact_nest *on,
-const struct ovnact_encode_params *ep OVS_UNUSED,
+const struct ovnact_encode_params *ep,
struct ofpbuf *ofpacts)
{
struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
@@ -792,6 +807,21 @@ encode_CT_COMMIT_V2(const struct ovnact_nest *on,
ct->zone_src.ofs = 0;
ct->zone_src.n_bits = 16;
+/* If the datapath supports all-zero SNAT then use it to avoid tuple
+ * collisions at commit time between NATed and firewalled-only sessions.
+ */
+if (ep->dp_features & OVS_CT_ZERO_SNAT_SUPPORT) {
+size_t nat_offset = ofpacts->size;
+ofpbuf_pull(ofpacts, nat_offset);
+
+struct ofpact_nat *nat = ofpact_put_NAT(ofpacts);
+nat->flags = 0;
+nat->range_af = AF_UNSPEC;
+nat->flags
[ovs-dev] [PATCH ovn 3/4] ovn-controller: Detect OVS datapath capabilities.
Automatically create an OVS Datapath record if none exists for the
current br-int datapath type.
Add a 'features' module to track which OVS features are available in
the datapath currently being used. For now, only ct_zero_snat is
tracked, all other features are assumed to be on-par between all
datapaths.
A future commit will make use of the 'features' module to conditionally
program openflows based on available datapath features.
Signed-off-by: Dumitru Ceara
---
controller/ovn-controller.c | 125 ++-
include/ovn/features.h | 16 ++
lib/automake.mk |1
lib/features.c | 39 +
tests/ovn-controller.at | 11 ++--
5 files changed, 159 insertions(+), 33 deletions(-)
create mode 100644 lib/features.c
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d48ddc7a2..15af7a13c 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -46,6 +46,7 @@
#include "openvswitch/vconn.h"
#include "openvswitch/vlog.h"
#include "ovn/actions.h"
+#include "ovn/features.h"
#include "lib/chassis-index.h"
#include "lib/extend-table.h"
#include "lib/ip-mcast-index.h"
@@ -88,6 +89,7 @@ static unixctl_cb_func lflow_cache_show_stats_cmd;
static unixctl_cb_func debug_delay_nb_cfg_report;
#define DEFAULT_BRIDGE_NAME "br-int"
+#define DEFAULT_DATAPATH "system"
#define DEFAULT_PROBE_INTERVAL_MSEC 5000
#define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0
@@ -95,6 +97,8 @@ static unixctl_cb_func debug_delay_nb_cfg_report;
#define OVS_NB_CFG_NAME "ovn-nb-cfg"
+#define OVS_CT_ZERO_SNAT_FEATURE "ct_zero_snat"
+
static char *parse_options(int argc, char *argv[]);
OVS_NO_RETURN static void usage(void);
@@ -319,10 +323,6 @@ static const struct ovsrec_bridge *
create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
const struct ovsrec_open_vswitch_table *ovs_table)
{
-if (!ovs_idl_txn) {
-return NULL;
-}
-
const struct ovsrec_open_vswitch *cfg;
cfg = ovsrec_open_vswitch_table_first(ovs_table);
if (!cfg) {
@@ -386,6 +386,21 @@ create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
return bridge;
}
+static const struct ovsrec_datapath *
+create_br_datapath(struct ovsdb_idl_txn *ovs_idl_txn,
+ const struct ovsrec_open_vswitch *cfg,
+ const char *datapath_type)
+{
+ovsdb_idl_txn_add_comment(ovs_idl_txn,
+ "ovn-controller: creating bridge datapath '%s'",
+ datapath_type);
+
+struct ovsrec_datapath *dp = ovsrec_datapath_insert(ovs_idl_txn);
+ovsrec_open_vswitch_verify_datapaths(cfg);
+ovsrec_open_vswitch_update_datapaths_setkey(cfg, datapath_type, dp);
+return dp;
+}
+
static const struct ovsrec_bridge *
get_br_int(const struct ovsrec_bridge_table *bridge_table,
const struct ovsrec_open_vswitch_table *ovs_table)
@@ -399,33 +414,78 @@ get_br_int(const struct ovsrec_bridge_table *bridge_table,
return get_bridge(bridge_table, br_int_name(cfg));
}
-static const struct ovsrec_bridge *
+static const struct ovsrec_datapath *
+get_br_datapath(const struct ovsrec_open_vswitch *cfg,
+const char *datapath_type)
+{
+for (size_t i = 0; i < cfg->n_datapaths; i++) {
+if (!strcmp(cfg->key_datapaths[i], datapath_type)) {
+return cfg->value_datapaths[i];
+}
+}
+return NULL;
+}
+
+static void
process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
const struct ovsrec_bridge_table *bridge_table,
- const struct ovsrec_open_vswitch_table *ovs_table)
+ const struct ovsrec_open_vswitch_table *ovs_table,
+ const struct ovsrec_bridge **br_int_,
+ const struct ovsrec_datapath **br_int_dp_)
{
-const struct ovsrec_bridge *br_int = get_br_int(bridge_table,
-ovs_table);
-if (!br_int) {
-br_int = create_br_int(ovs_idl_txn, ovs_table);
-}
-if (br_int && ovs_idl_txn) {
-const struct ovsrec_open_vswitch *cfg;
-cfg = ovsrec_open_vswitch_table_first(ovs_table);
-ovs_assert(cfg);
-const char *datapath_type = smap_get(>external_ids,
- "ovn-bridge-datapath-type");
-/* Check for the datapath_type and set it only if it is defined in
- * cfg. */
-if (datapath_type && strcmp(br_int->datapath_type, datapath_type)) {
-ovsrec_bridge_set_datapath_type(br_int, datapath_type);
+const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
+const struct ovsrec_datapath *br_int_dp = NULL;
+if (ovs_idl_txn) {
+if (!br_int) {
+br_int = create_br_int(ovs_idl_txn, ovs_table);
}
-if (!br_int->fail_mode || strcmp(br_int->fail_mode, "secure")) {
-ovsrec_bridge_set_fail_mode(br_int, "secure");
-
[ovs-dev] [PATCH ovn 2/4] system-ovn.at: Use ADD_BR macro instead of bare ovs-vsctl.
This ensures the datapath type is properly set for the bridge.
a8362ff85dee ("northd: Fix the missing force_snat_for_lb flows when router_ip
is configured.")
Signed-off-by: Dumitru Ceara
---
tests/system-ovn.at |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 76fc21c87..310bd3d5a 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -2312,8 +2312,7 @@ CHECK_CONNTRACK_NAT()
ovn_start
OVS_TRAFFIC_VSWITCHD_START()
ADD_BR([br-int])
-check ovs-vsctl add-br br-ext
-
+ADD_BR([br-ext], [set Bridge br-ext fail-mode=standalone])
# Set external-ids in br-int needed for ovn-controller
ovs-vsctl \
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn 1/4] ovs: Include ovs-vswitchd segfault fixes.
Bump the OVS submodule to include the following patches:
dd0f59783e39 ("ofproto: Fix p otential NULL dereference in
ofproto_get_datapath_cap().")
a4b04276ab59 ("ofproto: Fix potential NULL dereference in
ofproto_ct_*_zone_timeout_policy().")
The two commits above fix crashes in ovs-vswitchd when OVS datapath
types are configured with unsupported values and OVSDB Datapath tables
exist for those datapath types. OVN self tests actually test with
invalid datapath types and an upcoming commit will update the
ovn-controller code to always create OVSDB Datapath tables. Without
the two OVS fixes, ovs-vswitchd would crash, causing the OVN tests to
fail.
Signed-off-by: Dumitru Ceara
---
ovs |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ovs b/ovs
index b5bb044fb..a4b04276a 16
--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit b5bb044fbe4c1395dcde5cc7d5081ef0099bb8b3
+Subproject commit a4b04276ab5934d087669ff2d191a23931335c87
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn 0/4] Handle DNAT/no-NAT conntrack tuple collisions if possible.
This depends on the following OVS patch to be accepted: https://patchwork.ozlabs.org/project/openvswitch/patch/162264080113.315078.1220132318734842720.stgit@ebuild/ However, the capability name, "ct_zero_snat" has been agreed upon so it is very unlikely that it will change if new revisions of the OVS patch are required. Dumitru Ceara (4): ovs: Include ovs-vswitchd segfault fixes. system-ovn.at: Use ADD_BR macro instead of bare ovs-vsctl. ovn-controller: Detect OVS datapath capabilities. ovn-controller: Handle DNAT/no-NAT conntrack tuple collisions. controller/lflow.c|1 controller/ovn-controller.c | 125 include/ovn/actions.h |4 + include/ovn/features.h| 16 + lib/actions.c | 34 +- lib/automake.mk |1 lib/features.c| 39 +++ ovs |2 - tests/ovn-controller.at | 11 +-- tests/ovn.at |2 - tests/system-common-macros.at |4 + tests/system-ovn.at | 141 - 12 files changed, 341 insertions(+), 39 deletions(-) create mode 100644 lib/features.c ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 ovn 0/4] respin CoPP series
On 24/05/2021 19:44, Lorenzo Bianconi wrote: > This series respin CoPP support introduced here [0] by Dumitru rebasing on top > of ovn master branch and adding some missing meters (e.g. bfd or acl reject). > The main goal of this series is to continue the discussion about the proposed > approach and to align on CMS APIs. > For the moment no ddlog support has been added. > Related bz: > https://bugzilla.redhat.com/show_bug.cgi?id=1947913 > https://bugzilla.redhat.com/show_bug.cgi?id=1946610 I mentioned the possibility of adding a tutorial. Is that something you will do in a separate patch? > > Changes since v1: > - merge patch 3/5 and 4/5 > - cosmetics > - improve naming conventions > - add more unit-tests/system-tests > - remove duplicated flow > - remove some leftover entries in ovn-nbctl.8.xml > - add metering for sctp abort packets > > Changes since RFC: > - drop per-port metering > - add unit/system tests > - add reject action metering > > [0] > https://patchwork.ozlabs.org/project/openvswitch/list/?series=140778=* > > Dumitru Ceara (4): > ovn-controller: Add support for Logical_Flow control meters > ovn-northd: Add support for CoPP. > ovn-northd: Add CoPP policies for flows that punt packets to > ovn-controller. > NEWS: Add CoPP support. > > NEWS | 1 + > controller/lflow.c| 40 ++- > controller/ofctrl.c | 55 ++-- > controller/ofctrl.h | 21 +- > controller/physical.c | 7 +- > include/ovn/actions.h | 3 +- > lib/actions.c | 116 - > lib/automake.mk | 2 + > lib/copp.c| 143 ++ > lib/copp.h| 59 + > northd/ovn-northd.c | 535 -- > ovn-nb.ovsschema | 16 +- > ovn-nb.xml| 81 ++ > ovn-sb.ovsschema | 6 +- > ovn-sb.xml| 6 + > tests/atlocal.in | 3 + > tests/ovn-controller.at | 48 > tests/ovn-northd.at | 81 ++ > tests/ovn.at | 4 +- > tests/system-ovn.at | 135 ++ > utilities/ovn-nbctl.8.xml | 119 + > utilities/ovn-nbctl.c | 178 + > 22 files changed, 1360 insertions(+), 299 deletions(-) > create mode 100644 lib/copp.c > create mode 100644 lib/copp.h > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 2/5] ovn-northd: Add support for CoPP.
On 24/05/2021 18:04, Lorenzo Bianconi wrote:
>> On 29/04/2021 18:04, Lorenzo Bianconi wrote:
>>> From: Dumitru Ceara
>>>
>
> [...]
>>
>> Could this be changed to copp_proto_get_name()?
>
> ack, I will fix it in v2
>
>>
>>> +{
>>> +if (proto >= COPP_PROTO_MAX) {
>>> +return "";
>>> +}
>>> +return copp_proto_names[proto];
>>> +}
>>> +
>>> +const char *
>>> +copp_meter_get(enum copp_proto proto, const struct nbrec_copp *copp,
>>> + const struct shash *meter_groups)
>>> +{
>>> +if (!copp || proto >= COPP_PROTO_MAX) {
>>> +return NULL;
>>> +}
>>> +
>>> +const char *meter = smap_get(>meters, copp_proto_names[proto]);
>>> +
>>> +if (meter && shash_find(meter_groups, meter)) {
>>> +return meter;
>>> +}
>>> +
>>> +return NULL;
>>> +}
>>> +
>>> +void
>>> +copp_list(struct ctl_context *ctx, const struct nbrec_copp *copp)
>>> +{
>>> +if (!copp) {
>>> +return;
>>> +}
>>> +
>>> +struct smap_node *node;
>>> +
>>> +SMAP_FOR_EACH (node, >meters) {
>>> +ds_put_format(>output, "%s: %s\n", node->key, node->value);
>>> +}
>>> +}
>>> +
>>> +const struct nbrec_copp *
>>> +copp_add_meter(struct ctl_context *ctx, const struct nbrec_copp *copp,
>>> + const char *proto_name, const char *meter)
>>> +{
>>> +if (!copp) {
>>> +copp = nbrec_copp_insert(ctx->txn);
>>> +}
>>> +
>>> +struct smap meters;
>>> +smap_init();
>>> +smap_clone(, >meters);
>>> +smap_replace(, proto_name, meter);
>>> +nbrec_copp_set_meters(copp, );
>>> +smap_destroy();
>>> +
>>> +return copp;
>>> +}
>>> +
>>> +void
>>> +copp_del_meter(const struct nbrec_copp *copp, const char *proto_name)
>>> +{
>>> +if (!copp) {
>>> +return;
>>> +}
>>> +
>>> +if (proto_name) {
>>> +if (smap_get(>meters, proto_name)) {
>>> +struct smap meters;
>>> +smap_init();
>>> +smap_clone(, >meters);
>>> +smap_remove(, proto_name);
>>> +nbrec_copp_set_meters(copp, );
>>> +smap_destroy();
>>> +}
>>> +} else {
>>> +nbrec_copp_delete(copp);
>>> +}
>>> +}
>>> +
>>> +char *
>>> +copp_proto_validate(const char *proto_name)> +{
>>> +for (size_t i = COPP_PROTO_FIRST; i < COPP_PROTO_MAX; i++) {
>>> +if (!strcmp(proto_name, copp_proto_get(i))) {
>>> +return NULL;
>>> +}
>>> +}
>>> +
>>> +struct ds usage = DS_EMPTY_INITIALIZER;
>>> +
>>> +ds_put_cstr(, "Invalid control protocol. Allowed values: ");
>>> +for (size_t i = COPP_PROTO_FIRST; i < COPP_PROTO_MAX; i++) {
>>> +ds_put_format(, "%s, ", copp_proto_get(i));
>>> +}
>>> +ds_chomp(, ' ');
>>> +ds_chomp(, ',');
>>> +ds_put_cstr(, ".");
>>> +
>>> +char *usage_str = xstrdup(ds_cstr());
>>> +ds_destroy();
>>> +return usage_str;
>>> +}
>>> diff --git a/lib/copp.h b/lib/copp.h
>>> new file mode 100644
>>> index 0..82581e7e4
>>> --- /dev/null
>>> +++ b/lib/copp.h
>>> @@ -0,0 +1,60 @@
>>> +/* Copyright (c) 2021, Red Hat, Inc.
>>> + *
>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>> + * you may not use this file except in compliance with the License.
>>> + * You may obtain a copy of the License at:
>>> + *
>>> + * http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + * Unless required by applicable law or agreed to in writing, software
>>> + * distributed under the License is distributed on an "AS IS" BASIS,
>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>> + * See the License for the specific language governing permissions and
>>> + * limitations under the License.
>>> + */
>>> +
>>> +#ifndef OVN_COPP_H
>>> +#define OVN_COPP_H 1
>>> +
>>> +/*
>>> + * Control plane protection - metered actions.
>>> + */
>>> +enum copp_proto {
>>> +COPP_PROTO_FIRST,
>>> +COPP_ARP = COPP_PROTO_FIRST,
>>> +COPP_ARP_RESOLVE,
>>> +COPP_DHCPV4_OPTS,
>>> +COPP_DHCPV6_OPTS,
>>> +COPP_DNS,
>>> +COPP_EVENT_ELB,
>>> +COPP_ICMP4_ERR,
>>> +COPP_ICMP6_ERR,
>>> +COPP_IGMP,
>>> +COPP_ND_NA,
>>> +COPP_ND_NS,
>>> +COPP_ND_NS_RESOLVE,
>>> +COPP_ND_RA_OPTS,
>>> +COPP_TCP_RESET,
>>> +COPP_BFD,
>>> +COPP_PROTO_MAX,
>>> +COPP_PROTO_INVALID = COPP_PROTO_MAX,
>>> +};
>>> +
>>> +struct nbrec_copp;
>>> +struct ctl_context;
>>> +
>>> +const char *copp_proto_get(enum copp_proto);
>>
>> I think this could a static and declared in copp.c as it is not used
>> outside of there.
>
> ack, I will fix it in v2
>>
>>> +
>>> +const char *copp_meter_get(enum copp_proto proto,
>>> + const struct nbrec_copp *copp,
>>> + const struct shash *meter_groups);
>>> +
>>> +void copp_list(struct ctl_context *ctx, const struct nbrec_copp *copp);
>>> +const struct nbrec_copp *
>>> +copp_add_meter(struct ctl_context *ctx, const struct nbrec_copp *copp,
>>> +
Re: [ovs-dev] [PATCH v2 ovn 2/4] ovn-northd: Add support for CoPP.
On 24/05/2021 19:44, Lorenzo Bianconi wrote:
> From: Dumitru Ceara
>
> Add new 'Copp' (Control plane protection) table to OVN Northbound DB:
> - this stores mappings between control plane protocol names and meters
> that should be used to rate limit controller-destined traffic for
> those protocols.
>
> Add new 'copp' columns to the following OVN Northbound DB tables:
> - Logical_Switch
> - Logical_Router
>
> For now, no control plane protection policy is installed for any of
> the existing flows that punt packets to ovn-controller. This will be
> added in follow-up patches.
>
> Add CLI commands in 'ovn-nbctl' to allow the user to manage Control
> Plane Protection Policies at different levels (logical switch,
> logical router).
>
> Co-authored-by: Lorenzo Bianconi
> Signed-off-by: Lorenzo Bianconi
> Signed-off-by: Dumitru Ceara
> ---
> lib/automake.mk | 2 +
> lib/copp.c| 142 ++
> lib/copp.h| 58 +
> northd/ovn-northd.c | 44 +++---
> ovn-nb.ovsschema | 16 +++-
> ovn-nb.xml| 78 +
> tests/ovn-controller.at | 48 ++
> tests/ovn-northd.at | 81 +
> utilities/ovn-nbctl.8.xml | 116 +
> utilities/ovn-nbctl.c | 178 ++
> 10 files changed, 751 insertions(+), 12 deletions(-)
> create mode 100644 lib/copp.c
> create mode 100644 lib/copp.h
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 781be2109..20e296fff 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -9,6 +9,8 @@ lib_libovn_la_SOURCES = \
> lib/actions.c \
> lib/chassis-index.c \
> lib/chassis-index.h \
> + lib/copp.c \
> + lib/copp.h \
> lib/ovn-dirs.h \
> lib/expr.c \
> lib/extend-table.h \
> diff --git a/lib/copp.c b/lib/copp.c
> new file mode 100644
> index 0..e3d14938a
> --- /dev/null
> +++ b/lib/copp.c
> @@ -0,0 +1,142 @@
> +/* Copyright (c) 2021, Red Hat, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include
> +#include
> +
> +#include "openvswitch/shash.h"
> +#include "db-ctl-base.h"
> +#include "smap.h"
> +#include "lib/ovn-nb-idl.h"
> +#include "lib/copp.h"
> +
> +static char *copp_proto_names[COPP_PROTO_MAX] = {
> +[COPP_ARP] = "arp",
> +[COPP_ARP_RESOLVE] = "arp-resolve",
> +[COPP_DHCPV4_OPTS] = "dhcpv4-opts",
> +[COPP_DHCPV6_OPTS] = "dhcpv6-opts",
> +[COPP_DNS] = "dns",
> +[COPP_EVENT_ELB] = "event-elb",
> +[COPP_ICMP4_ERR] = "icmp4-error",
> +[COPP_ICMP6_ERR] = "icmp6-error",
> +[COPP_IGMP] = "igmp",
> +[COPP_ND_NA] = "nd-na",
> +[COPP_ND_NS] = "nd-ns",
> +[COPP_ND_NS_RESOLVE] = "nd-ns-resolve",
> +[COPP_ND_RA_OPTS]= "nd-ra-opts",
> +[COPP_TCP_RESET] = "tcp-reset",
> +[COPP_BFD] = "bfd",
> +};
> +
> +static const char *
> +copp_proto_get_name(enum copp_proto proto)
> +{
> +if (proto >= COPP_PROTO_MAX) {
> +return "";
> +}
> +return copp_proto_names[proto];
> +}
> +
> +const char *
> +copp_meter_get(enum copp_proto proto, const struct nbrec_copp *copp,
> + const struct shash *meter_groups)
> +{
> +if (!copp || proto >= COPP_PROTO_MAX) {
> +return NULL;
> +}
> +
> +const char *meter = smap_get(>meters, copp_proto_names[proto]);
> +
> +if (meter && shash_find(meter_groups, meter)) {
> +return meter;
> +}
> +
> +return NULL;
> +}
> +
> +void
> +copp_meter_list(struct ctl_context *ctx, const struct nbrec_copp *copp)
> +{
> +if (!copp) {
> +return;
> +}
> +
> +struct smap_node *node;
> +
> +SMAP_FOR_EACH (node, >meters) {
> +ds_put_format(>output, "%s: %s\n", node->key, node->value);
> +}
> +}
> +
> +const struct nbrec_copp *
> +copp_meter_add(struct ctl_context *ctx, const struct nbrec_copp *copp,
> + const char *proto_name, const char *meter)
> +{
> +if (!copp) {
> +copp = nbrec_copp_insert(ctx->txn);
> +}
> +
> +struct smap meters;
> +smap_init();
> +smap_clone(, >meters);
> +smap_replace(, proto_name, meter);
> +nbrec_copp_set_meters(copp, );
> +smap_destroy();
> +
> +return copp;
> +}
> +
> +void
> +copp_meter_del(const struct nbrec_copp *copp, const char
Re: [ovs-dev] [PATCH v4] conntrack: document all-zero IP SNAT behavior and add a test case
On 6/3/21 2:38 PM, Aaron Conole wrote:
> Eelco Chaudron writes:
>
>> On 2 Jun 2021, at 18:21, Aaron Conole wrote:
>>
>>> Eelco Chaudron writes:
>>>
Currently, conntrack in the kernel has an undocumented feature referred
to as all-zero IP address SNAT. Basically, when a source port
collision is detected during the commit, the source port will be
translated to an ephemeral port. If there is no collision, no SNAT is
performed.
This patchset documents this behavior and adds a self-test to verify
it's not changing. In addition, a datapath feature flag is added for
the all-zero IP SNAT case. This will help applications on top of OVS,
like OVN, to determine this feature can be used.
Signed-off-by: Eelco Chaudron
---
v4: Added datapath support flag for all-zero SNAT.
v3: Renamed NULL SNAT to all-zero IP SNAT.
v2: Fixed NULL SNAT to only work in the -rpl state to be inline with
OpenShift-SDN's behavior.
lib/ct-dpif.c|8 +++
lib/ct-dpif.h|6 +
lib/dpif-netdev.c|1 +
lib/dpif-netlink.c | 11 +
lib/dpif-provider.h |5
lib/ovs-actions.xml | 10
ofproto/ofproto-dpif.c | 22 +-
ofproto/ofproto-dpif.h |5 +++-
tests/system-kmod-macros.at |7 ++
tests/system-traffic.at | 46
++
tests/system-userspace-macros.at | 10
vswitchd/vswitch.xml |9 +++
12 files changed, 138 insertions(+), 2 deletions(-)
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index 6a5ba052d..cfc2315e3 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -889,3 +889,11 @@ ct_dpif_get_timeout_policy_name(struct dpif *dpif,
uint32_t tp_id,
dpif, tp_id, dl_type, nw_proto, tp_name, is_generic)
: EOPNOTSUPP);
}
+
+int
+ct_dpif_get_features(struct dpif *dpif, enum ct_features *features)
+{
>>>
>>> NIT-mode:
>>>
>>> Are these features or capabilities? I ask because we may want to add
>>> support for things like tcp loose mode, etc, and not sure if there would
>>> need to be a corresponding set function (to enable / disable), and how
>>> that should look. I'm okay with keeping it as-is for here and adding
>>> the corresponding set function later, but it would seem strange to try
>>> and "set" support for all-zero snat, etc.
>>
>> Guess the line between feature and capabilities is rather thin... The
>> code for checking the datapath support, check_support(), is calling
>> all of this features, rather than capabilities.
>>
>> I guess ct_zero_snat is a none configurable feature ;) But more
>> seriously, I could go ahead and change the naming from feature to
>> capability. As most of the configurable "features" have their own
>> callback.
>
> I guess it doesn't matter too much, but I worry about whether we start
> trying to enable. Maybe we just make it unsupportable? I'm just
> concerned that when we add things like tcp_loose or try to make
> tcp_liberal as a configurable in the kernel DP, there could be some
> confusion. Maybe it isn't too important. Okay, we can cross those
> bridges when we get there.
>
+return (dpif->dpif_class->ct_get_features
+? dpif->dpif_class->ct_get_features(dpif, features)
+: EOPNOTSUPP);
+}
>>
>>
>>
diff --git a/tests/system-userspace-macros.at
b/tests/system-userspace-macros.at
index 34f82cee3..9f0d38dfb 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -96,6 +96,16 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP])
#
m4_define([CHECK_CONNTRACK_NAT])
+# CHECK_CONNTRACK_ZEROIP_SNAT()
+#
+# Perform requirements checks for running conntrack all-zero IP SNAT
tests.
+# The userspace datapath does not support all-zero IP SNAT.
+#
+m4_define([CHECK_CONNTRACK_ZEROIP_SNAT],
+[
+AT_SKIP_IF([:])
+])
>>>
>>> I didn't check too close, but maybe it's possible to make this check the
>>> capabilities bits and then it could be extended to everywhere.
>>
>> I was thinking about using "ovs-appctl dpif/show-dp-features" after I
>> added the features check. However none of the other test cases do
>> this, so I thought there might be a reason? It might be that I need to
>> configure/setup OVS to run the test command, and not sure if there is
>> a nice clean way to shut down if the feature is not supported.
>
> Okay. Maybe it's not possible in the test framework.
Hmm. AFAICT, daemons will be stopped by on_exit hooks, so it
should not be a problem. Just call the check after starting the daemon.
BTW, have you checked the windows
Re: [ovs-dev] [PATCH v4] conntrack: document all-zero IP SNAT behavior and add a test case
Eelco Chaudron writes:
> On 2 Jun 2021, at 18:21, Aaron Conole wrote:
>
>> Eelco Chaudron writes:
>>
>>> Currently, conntrack in the kernel has an undocumented feature referred
>>> to as all-zero IP address SNAT. Basically, when a source port
>>> collision is detected during the commit, the source port will be
>>> translated to an ephemeral port. If there is no collision, no SNAT is
>>> performed.
>>>
>>> This patchset documents this behavior and adds a self-test to verify
>>> it's not changing. In addition, a datapath feature flag is added for
>>> the all-zero IP SNAT case. This will help applications on top of OVS,
>>> like OVN, to determine this feature can be used.
>>>
>>> Signed-off-by: Eelco Chaudron
>>> ---
>>> v4: Added datapath support flag for all-zero SNAT.
>>> v3: Renamed NULL SNAT to all-zero IP SNAT.
>>> v2: Fixed NULL SNAT to only work in the -rpl state to be inline with
>>> OpenShift-SDN's behavior.
>>>
>>> lib/ct-dpif.c|8 +++
>>> lib/ct-dpif.h|6 +
>>> lib/dpif-netdev.c|1 +
>>> lib/dpif-netlink.c | 11 +
>>> lib/dpif-provider.h |5
>>> lib/ovs-actions.xml | 10
>>> ofproto/ofproto-dpif.c | 22 +-
>>> ofproto/ofproto-dpif.h |5 +++-
>>> tests/system-kmod-macros.at |7 ++
>>> tests/system-traffic.at | 46
>>> ++
>>> tests/system-userspace-macros.at | 10
>>> vswitchd/vswitch.xml |9 +++
>>> 12 files changed, 138 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
>>> index 6a5ba052d..cfc2315e3 100644
>>> --- a/lib/ct-dpif.c
>>> +++ b/lib/ct-dpif.c
>>> @@ -889,3 +889,11 @@ ct_dpif_get_timeout_policy_name(struct dpif *dpif,
>>> uint32_t tp_id,
>>> dpif, tp_id, dl_type, nw_proto, tp_name, is_generic)
>>> : EOPNOTSUPP);
>>> }
>>> +
>>> +int
>>> +ct_dpif_get_features(struct dpif *dpif, enum ct_features *features)
>>> +{
>>
>> NIT-mode:
>>
>> Are these features or capabilities? I ask because we may want to add
>> support for things like tcp loose mode, etc, and not sure if there would
>> need to be a corresponding set function (to enable / disable), and how
>> that should look. I'm okay with keeping it as-is for here and adding
>> the corresponding set function later, but it would seem strange to try
>> and "set" support for all-zero snat, etc.
>
> Guess the line between feature and capabilities is rather thin... The
> code for checking the datapath support, check_support(), is calling
> all of this features, rather than capabilities.
>
> I guess ct_zero_snat is a none configurable feature ;) But more
> seriously, I could go ahead and change the naming from feature to
> capability. As most of the configurable "features" have their own
> callback.
I guess it doesn't matter too much, but I worry about whether we start
trying to enable. Maybe we just make it unsupportable? I'm just
concerned that when we add things like tcp_loose or try to make
tcp_liberal as a configurable in the kernel DP, there could be some
confusion. Maybe it isn't too important. Okay, we can cross those
bridges when we get there.
>>> +return (dpif->dpif_class->ct_get_features
>>> +? dpif->dpif_class->ct_get_features(dpif, features)
>>> +: EOPNOTSUPP);
>>> +}
>
>
>
>>> diff --git a/tests/system-userspace-macros.at
>>> b/tests/system-userspace-macros.at
>>> index 34f82cee3..9f0d38dfb 100644
>>> --- a/tests/system-userspace-macros.at
>>> +++ b/tests/system-userspace-macros.at
>>> @@ -96,6 +96,16 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP])
>>> #
>>> m4_define([CHECK_CONNTRACK_NAT])
>>>
>>> +# CHECK_CONNTRACK_ZEROIP_SNAT()
>>> +#
>>> +# Perform requirements checks for running conntrack all-zero IP SNAT tests.
>>> +# The userspace datapath does not support all-zero IP SNAT.
>>> +#
>>> +m4_define([CHECK_CONNTRACK_ZEROIP_SNAT],
>>> +[
>>> +AT_SKIP_IF([:])
>>> +])
>>
>> I didn't check too close, but maybe it's possible to make this check the
>> capabilities bits and then it could be extended to everywhere.
>
> I was thinking about using "ovs-appctl dpif/show-dp-features" after I
> added the features check. However none of the other test cases do
> this, so I thought there might be a reason? It might be that I need to
> configure/setup OVS to run the test command, and not sure if there is
> a nice clean way to shut down if the feature is not supported.
Okay. Maybe it's not possible in the test framework.
>>> +
>>> # CHECK_CONNTRACK_TIMEOUT()
>>> #
>>> # Perform requirements checks for running conntrack customized timeout
>>> tests.
>>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>>> index 4597a215d..d8ea287d5 100644
>>> --- a/vswitchd/vswitch.xml
>>> +++ b/vswitchd/vswitch.xml
>>> @@ -6126,6 +6126,15 @@ ovs-vsctl add-port br0
[ovs-dev] [PATCH ovn v9 5/5] controller I-P: ct zone runtime data handler.
From: Numan Siddique
This patch adds an handler for runtime data changes to the ct_zones
engine node. Before this patch, the handler was NULL. With this patch
we do a full recompute of ct_zones engine if:
1. runtime_data's local_lports change.
2. If a new datapath is added to the local datapaths.
For all other changes of runtime data, there is no need to recompute
ct_zones engine node.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1962345
Signed-off-by: Numan Siddique
---
controller/ovn-controller.c | 39 -
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 5b55a45b7..4ab820ca9 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1271,8 +1271,10 @@ runtime_data_sb_port_binding_handler(struct engine_node
*node, void *data)
return false;
}
+rt_data->local_lports_changed = b_ctx_out.local_lports_changed;
if (b_ctx_out.local_lport_ids_changed ||
b_ctx_out.non_vif_ports_changed ||
+b_ctx_out.local_lports_changed ||
!hmap_is_empty(b_ctx_out.tracked_dp_bindings)) {
engine_set_node_state(node, EN_UPDATED);
}
@@ -1786,6 +1788,40 @@ ct_zones_datapath_binding_handler(struct engine_node
*node, void *data)
return true;
}
+static bool
+ct_zones_runtime_data_handler(struct engine_node *node, void *data OVS_UNUSED)
+{
+struct ed_type_runtime_data *rt_data =
+engine_get_input_data("runtime_data", node);
+
+/* There is no tracked data. Fall back to full recompute of ct_zones. */
+if (!rt_data->tracked) {
+return false;
+}
+
+/* If local_lports have changed then fall back to full recompute. */
+if (rt_data->local_lports_changed) {
+return false;
+}
+
+struct hmap *tracked_dp_bindings = _data->tracked_dp_bindings;
+struct tracked_binding_datapath *tdp;
+HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) {
+if (tdp->is_new) {
+/* A new datapath has been added. Fall back to full recompute. */
+return false;
+}
+
+/* When an lport is claimed or released because of port binding,
+ * changes we don't have to compute the ct zone entries for these.
+ * That is because we generate the ct zone entries for each local
+ * OVS interface which has external_ids:iface-id set. For the local
+ * OVS interface changes, rt_data->local_ports_changed will be true. */
+}
+
+return true;
+}
+
/* The data in the ct_zones node is always valid (i.e., no stale pointers). */
static bool
en_ct_zones_is_valid(struct engine_node *node OVS_UNUSED)
@@ -2812,7 +2848,8 @@ main(int argc, char *argv[])
engine_add_input(_ct_zones, _ovs_bridge, NULL);
engine_add_input(_ct_zones, _sb_datapath_binding,
ct_zones_datapath_binding_handler);
-engine_add_input(_ct_zones, _runtime_data, NULL);
+engine_add_input(_ct_zones, _runtime_data,
+ ct_zones_runtime_data_handler);
engine_add_input(_runtime_data, _ofctrl_is_connected, NULL);
--
2.31.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v9 4/5] physical: Set the port binding uuid as cookie for flows where relevant.
From: Numan Siddique
Some of the OF flows added by physical.c in tables LOCAL_OUTPUT,
CHECK_LOOPBACK, SAVE_INPORT can store the port binding uuid as
flow cookie because these flows matches on port binding key
in the MFF_LOG_OUTPORT field. Whereas right now it stores 0 as cookie.
This patch stores the port binding uuid as cookie for these flows.
When a port binding is deleted, the port binding handler for flow_output
engine node flood removes the flows with the port binding uuid and
such flows wont get removed. Right now this is not a problem because
deleting a port binding also triggers in physical_run() being called
and it recomputes all the physical flows.
This patch is required for an upcoming patch which do not call
physical_run() for port binding deletes.
Signed-off-by: Numan Siddique
---
controller/physical.c | 44 ---
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/controller/physical.c b/controller/physical.c
index e70efc71d..17ca5afbb 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -739,20 +739,23 @@ put_replace_router_port_mac_flows(struct ovsdb_idl_index
}
static void
-put_local_common_flows(uint32_t dp_key, uint32_t port_key,
- uint32_t parent_port_key,
+put_local_common_flows(uint32_t dp_key,
+ const struct sbrec_port_binding *pb,
+ const struct sbrec_port_binding *parent_pb,
const struct zone_ids *zone_ids,
struct ofpbuf *ofpacts_p,
struct ovn_desired_flow_table *flow_table)
{
struct match match;
-/* Table 33, priority 100.
+uint32_t port_key = pb->tunnel_key;
+
+/* Table 38, priority 100.
* ===
*
* Implements output to local hypervisor. Each flow matches a
* logical output port on the local hypervisor, and resubmits to
- * table 34.
+ * table 39.
*/
match_init_catchall();
@@ -774,12 +777,13 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key,
}
}
-/* Resubmit to table 34. */
+/* Resubmit to table 39. */
put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
-ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
-, ofpacts_p, hc_uuid);
+ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
+pb->header_.uuid.parts[0], , ofpacts_p,
+>header_.uuid);
-/* Table 34, Priority 100.
+/* Table 39, Priority 100.
* ===
*
* Drop packets whose logical inport and outport are the same
@@ -791,8 +795,9 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key,
0, MLF_ALLOW_LOOPBACK);
match_set_reg(, MFF_LOG_INPORT - MFF_REG0, port_key);
match_set_reg(, MFF_LOG_OUTPORT - MFF_REG0, port_key);
-ofctrl_add_flow(flow_table, OFTABLE_CHECK_LOOPBACK, 100, 0,
-, ofpacts_p, hc_uuid);
+ofctrl_add_flow(flow_table, OFTABLE_CHECK_LOOPBACK, 100,
+pb->header_.uuid.parts[0], , ofpacts_p,
+>header_.uuid);
/* Table 64, Priority 100.
* ===
@@ -806,7 +811,7 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key,
* table 65 for logical-to-physical translation, then restore
* the port number.
*
- * If 'parent_port_key' is set, then the 'port_key' represents a nested
+ * If 'parent_pb' is not NULL, then the 'pb' represents a nested
* container.
*
* Note:We can set in_port to 0 too. But if recirculation happens
@@ -815,7 +820,7 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key,
* in_port is 0.
* */
-bool nested_container = parent_port_key ? true: false;
+bool nested_container = parent_pb ? true: false;
match_init_catchall();
ofpbuf_clear(ofpacts_p);
match_set_metadata(, htonll(dp_key));
@@ -829,8 +834,9 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key,
put_load(ofp_to_u16(OFPP_NONE), MFF_IN_PORT, 0, 16, ofpacts_p);
put_resubmit(OFTABLE_LOG_TO_PHY, ofpacts_p);
put_stack(MFF_IN_PORT, ofpact_put_STACK_POP(ofpacts_p));
-ofctrl_add_flow(flow_table, OFTABLE_SAVE_INPORT, 100, 0,
-, ofpacts_p, hc_uuid);
+ofctrl_add_flow(flow_table, OFTABLE_SAVE_INPORT, 100,
+pb->header_.uuid.parts[0], , ofpacts_p,
+>header_.uuid);
if (nested_container) {
/* It's a nested container and when the packet from the nested
@@ -852,7 +858,8 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key,
match_init_catchall();
ofpbuf_clear(ofpacts_p);
match_set_metadata(, htonll(dp_key));
-match_set_reg(, MFF_LOG_OUTPORT - MFF_REG0, parent_port_key);
+match_set_reg(, MFF_LOG_OUTPORT - MFF_REG0,
+
[ovs-dev] [PATCH ovn v9 3/5] ovn-controller: Handle datapath changes incrementally for ct zone I-P engine node.
From: Numan Siddique
After the commit [1], we do a full recompute of ct zone I-P engine for
any datapath binding change. This results in physical_flow() getting
called. In a highly scaled environment this can result in costly CPU
cycles. This patch addressed this by handling the datapath binding
changes incrementally in the ct_zone engine node. ct zone recomputation
is required only when a datapath binding is deleted or a logical router
datapath binding changes the snat-ct-zone option value. This patch
handles this.
[1] - f9cab11d5fab("Allow explicit setting of the SNAT zone on a gateway
router.")
Signed-off-by: Numan Siddique
---
controller/ovn-controller.c | 55 -
controller/physical.c | 5
tests/ovn-performance.at| 26 ++
3 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index b49786441..5b55a45b7 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1734,6 +1734,58 @@ en_ct_zones_run(struct engine_node *node, void *data)
engine_set_node_state(node, EN_UPDATED);
}
+/* Handles datapath binding changes for the ct_zones engine.
+ * Returns false if the datapath is deleted or if the requested snat
+ * ct zone doesn't match with the ct_zones data. */
+static bool
+ct_zones_datapath_binding_handler(struct engine_node *node, void *data)
+{
+struct ed_type_ct_zones *ct_zones_data = data;
+const struct sbrec_datapath_binding *dp;
+struct ed_type_runtime_data *rt_data =
+engine_get_input_data("runtime_data", node);
+struct sbrec_datapath_binding_table *dp_table =
+(struct sbrec_datapath_binding_table *)EN_OVSDB_GET(
+engine_get_input("SB_datapath_binding", node));
+
+SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_table) {
+if (!get_local_datapath(_data->local_datapaths,
+dp->tunnel_key)) {
+continue;
+}
+
+if (sbrec_datapath_binding_is_deleted(dp)) {
+/* Fall back to full recompute of ct_zones engine. */
+return false;
+}
+
+int req_snat_zone = datapath_snat_ct_zone(dp);
+if (req_snat_zone == -1) {
+/* datapath snat ct zone is not set. This condition will also hit
+ * when CMS clears the snat-ct-zone for the logical router.
+ * In this case there is no harm in using the previosly specified
+ * snat ct zone for this datapath. Also it is hard to know
+ * if this option was cleared or if this option is never set. */
+continue;
+}
+
+/* Check if the requested snat zone has changed for the datapath
+ * or not. If so, then fall back to full recompute of
+ * ct_zone engine. */
+char *snat_dp_zone_key = alloc_nat_zone_key(>header_.uuid, "snat");
+struct simap_node *simap_node = simap_find(_zones_data->current,
+ snat_dp_zone_key);
+free(snat_dp_zone_key);
+if (!simap_node || simap_node->data != req_snat_zone) {
+/* There is no entry yet or the requested snat zone has changed.
+ * Trigger full recompute of ct_zones engine. */
+return false;
+}
+}
+
+return true;
+}
+
/* The data in the ct_zones node is always valid (i.e., no stale pointers). */
static bool
en_ct_zones_is_valid(struct engine_node *node OVS_UNUSED)
@@ -2758,7 +2810,8 @@ main(int argc, char *argv[])
engine_add_input(_ct_zones, _ovs_open_vswitch, NULL);
engine_add_input(_ct_zones, _ovs_bridge, NULL);
-engine_add_input(_ct_zones, _sb_datapath_binding, NULL);
+engine_add_input(_ct_zones, _sb_datapath_binding,
+ ct_zones_datapath_binding_handler);
engine_add_input(_ct_zones, _runtime_data, NULL);
engine_add_input(_runtime_data, _ofctrl_is_connected, NULL);
diff --git a/controller/physical.c b/controller/physical.c
index 04259d44a..e70efc71d 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -15,6 +15,7 @@
#include
#include "binding.h"
+#include "coverage.h"
#include "byte-order.h"
#include "encaps.h"
#include "flow.h"
@@ -48,6 +49,8 @@
VLOG_DEFINE_THIS_MODULE(physical);
+COVERAGE_DEFINE(physical_run);
+
/* Datapath zone IDs for connection tracking and NAT */
struct zone_ids {
int ct; /* MFF_LOG_CT_ZONE. */
@@ -1528,6 +1531,8 @@ void
physical_run(struct physical_ctx *p_ctx,
struct ovn_desired_flow_table *flow_table)
{
+COVERAGE_INC(physical_run);
+
if (!hc_uuid) {
hc_uuid = xmalloc(sizeof(struct uuid));
uuid_generate(hc_uuid);
diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at
index e510c6cef..61356e16d 100644
--- a/tests/ovn-performance.at
+++ b/tests/ovn-performance.at
@@ -462,6 +462,32 @@
[ovs-dev] [PATCH ovn v9 2/5] controller: Handle sbrec_chassis changes in lflow and pflow output engines.
From: Numan Siddique
This patch adds a sbrec_chassis change handler to both the lflow and
plfow output engines. It returns false if a chassis is added or deleted
so that a full recompute is triggered. Any updates to the sbrec_chassis
table is ignored as there is no need to handle these changes.
Signed-off-by: Numan Siddique
---
controller/ovn-controller.c | 31 +--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 5ea3035fb..b49786441 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2454,6 +2454,30 @@ struct ovn_controller_exit_args {
bool *restart;
};
+/* Handles sbrec_chassis changes.
+ * If a new chassis is added or removed return false, so that
+ * flows are recomputed. For any updates, there is no need for
+ * any flow computation. Encap changes will also result in
+ * sbrec_chassis changes, but we handle encap changes separately.
+ */
+static bool
+pflow_lflow_output_sb_chassis_handler(struct engine_node *node,
+ void *data OVS_UNUSED)
+{
+struct sbrec_chassis_table *chassis_table =
+(struct sbrec_chassis_table *)EN_OVSDB_GET(
+engine_get_input("SB_chassis", node));
+
+const struct sbrec_chassis *ch;
+SBREC_CHASSIS_TABLE_FOR_EACH_TRACKED (ch, chassis_table) {
+if (sbrec_chassis_is_deleted(ch) || sbrec_chassis_is_new(ch)) {
+return false;
+}
+}
+
+return true;
+}
+
/* Returns false if the northd internal version stored in SB_Global
* and ovn-controller internal version don't match.
*/
@@ -2672,7 +2696,9 @@ main(int argc, char *argv[])
engine_add_input(_pflow_output, _ovs_interface,
pflow_output_ovs_iface_handler);
engine_add_input(_pflow_output, _ct_zones, NULL);
-engine_add_input(_pflow_output, _sb_chassis, NULL);
+engine_add_input(_pflow_output, _sb_chassis,
+ pflow_lflow_output_sb_chassis_handler);
+
engine_add_input(_pflow_output, _sb_port_binding,
pflow_output_sb_port_binding_handler);
engine_add_input(_pflow_output, _sb_multicast_group,
@@ -2699,7 +2725,8 @@ main(int argc, char *argv[])
engine_add_input(_lflow_output, _sb_multicast_group,
engine_noop_handler);
-engine_add_input(_lflow_output, _sb_chassis, NULL);
+engine_add_input(_lflow_output, _sb_chassis,
+ pflow_lflow_output_sb_chassis_handler);
/* Any changes to the port binding, need not be handled
* for lflow_outout engine. We still need sb_port_binding
--
2.31.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v9 1/5] ovn-controller: Split logical flow and physical flow processing.
From: Numan Siddique
Presently, the 'flow_output' engine node recomputes physical
flows by calling physical_run() in the 'physical_flow_changes'
handler in some scenarios. Because of this, an engine run can
do a full recompute of physical flows but not full recompute
of logical flows. Although this works now, it is problematic
as the same desired flow table is used for both physical and
logical flows.
This patch now separates the handling of logical flows and
physical flows and removes the 'physical_flow_changes' engine
node. Two separate engine nodes are added - lflow_output and
pflow_output with their own flow tables and these two nodes are
now inputs to the main engine node - flow_output. This separation
reflects the data dependency more clearly.
CC: Han Zhou
Signed-off-by: Numan Siddique
---
TODO.rst| 6 +
controller/ofctrl.c | 99 --
controller/ofctrl.h | 6 +-
controller/ovn-controller.c | 691 +---
controller/physical.c | 19 -
controller/physical.h | 4 -
6 files changed, 406 insertions(+), 419 deletions(-)
diff --git a/TODO.rst b/TODO.rst
index c89fe203e..618ea4844 100644
--- a/TODO.rst
+++ b/TODO.rst
@@ -164,3 +164,9 @@ OVN To-do List
to find a way of determining if routing has already been executed (on a
different hypervisor) for the IP multicast packet being processed locally
in the router pipeline.
+
+* ovn-controller Incremental processing
+
+ * physical.c has a global simap -localvif_to_ofport which stores the
+local OVS interfaces and the ofport numbers. Move this to the engine data
+of the engine data node - ed_type_pflow_output.
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index c29c3d180..053631590 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -173,7 +173,7 @@ struct sb_flow_ref {
struct uuid sb_uuid;
};
-/* A installed flow, in static variable installed_flows.
+/* An installed flow, in static variable installed_lflows/installed_pflows.
*
* Installed flows are updated in ofctrl_put for maintaining the flow
* installation to OVS. They are updated according to desired flows: either by
@@ -234,7 +234,7 @@ static struct desired_flow *desired_flow_lookup_conjunctive(
static void desired_flow_destroy(struct desired_flow *);
static struct installed_flow *installed_flow_lookup(
-const struct ovn_flow *target);
+const struct ovn_flow *target, struct hmap *installed_flows);
static void installed_flow_destroy(struct installed_flow *);
static struct installed_flow *installed_flow_dup(struct desired_flow *);
static struct desired_flow *installed_flow_get_active(struct installed_flow *);
@@ -302,9 +302,12 @@ static ovs_be32 xid, xid2;
* zero, to avoid unbounded buffering. */
static struct rconn_packet_counter *tx_counter;
-/* Flow table of "struct ovn_flow"s, that holds the flow table currently
- * installed in the switch. */
-static struct hmap installed_flows;
+/* Flow table of "struct ovn_flow"s, that holds the logical flow table
+ * currently installed in the switch. */
+static struct hmap installed_lflows;
+/* Flow table of "struct ovn_flow"s, that holds the physical flow table
+ * currently installed in the switch. */
+static struct hmap installed_pflows;
/* A reference to the group_table. */
static struct ovn_extend_table *groups;
@@ -343,7 +346,8 @@ ofctrl_init(struct ovn_extend_table *group_table,
swconn = rconn_create(inactivity_probe_interval, 0,
DSCP_DEFAULT, 1 << OFP15_VERSION);
tx_counter = rconn_packet_counter_create();
-hmap_init(_flows);
+hmap_init(_lflows);
+hmap_init(_pflows);
ovs_list_init(_updates);
ovn_init_symtab();
groups = group_table;
@@ -1426,11 +1430,12 @@ desired_flow_lookup_conjunctive(struct
ovn_desired_flow_table *flow_table,
/* Finds and returns an installed_flow in installed_flows whose key is
* identical to 'target''s key, or NULL if there is none. */
static struct installed_flow *
-installed_flow_lookup(const struct ovn_flow *target)
+installed_flow_lookup(const struct ovn_flow *target,
+ struct hmap *installed_flows)
{
struct installed_flow *i;
HMAP_FOR_EACH_WITH_HASH (i, match_hmap_node, target->hash,
- _flows) {
+ installed_flows) {
struct ovn_flow *f = >flow;
if (f->table_id == target->table_id
&& f->priority == target->priority
@@ -1542,8 +1547,14 @@ static void
ovn_installed_flow_table_clear(void)
{
struct installed_flow *f, *next;
-HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, _flows) {
-hmap_remove(_flows, >match_hmap_node);
+HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, _lflows) {
+hmap_remove(_lflows, >match_hmap_node);
+unlink_all_refs_for_installed_flow(f);
+installed_flow_destroy(f);
+}
+
+HMAP_FOR_EACH_SAFE (f, next,
[ovs-dev] [PATCH ovn v9 0/5] ovn-controller: Split logical flow and physical flow processing
From: Numan Siddique This series splits the logical flow and physical flow processing and also handles the runtime data changes for the ct zone engine data. New patch p2 is added in v9 to handle sbrec_chassis changes in pflow_output and lflow_output engine nodes. The patches 2-4 added in v8 are now 3-5 in v9. v8 -> v9 * Addressed review comments from Han. Removed the noop handlers for some of the engine inputs. * Added a new patch - p2 to handle sbrec_chassis changes in pflow_output and lflow_output engine nodes. v7 -> v8 * Added the ct zones I-P for datapath and runtime data changes. * Removed the noop_handler for runtime data changes in pflow engine node. Now there is no handler for runtime data changes for the pflow engine node. v6 -> v7 * Added comments on usage of noop_handler for a couple of engine inputs as suggested by Han. * Addressed other review comments from Han. * Added the check to handle the flow changes if 'skipped_last_time' is true in ofctrl_put(). v5 -> v6 * Missed out checking in the uncommitted code in ofctrl.c in v4. v5 fixes it. * v5 accidently modified ovs submodule commit id. v6 reverts it. v4 -> v5 - * Addressed Han's comments. v3 -> v4 - * Addressed Mark G's comments. * Rebased to resolve conflicts. v2 -> v3 - * Rebased to resolve conflicts. v1 -> v2 - * Rebased to resolve conflicts. Numan Siddique (5): ovn-controller: Split logical flow and physical flow processing. controller: Handle sbrec_chassis changes in lflow and pflow output engines. ovn-controller: Handle datapath changes incrementally for ct zone I-P engine node. physical: Set the port binding uuid as cookie for flows where relevant. controller I-P: ct zone runtime data handler. TODO.rst| 6 + controller/ofctrl.c | 99 +++-- controller/ofctrl.h | 6 +- controller/ovn-controller.c | 812 controller/physical.c | 68 ++- controller/physical.h | 4 - tests/ovn-performance.at| 26 ++ 7 files changed, 581 insertions(+), 440 deletions(-) -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 0/2] ofproto: Fix potential NULL dereference with invalid datapath types.
On 6/3/21 1:48 PM, Ilya Maximets wrote: > On 5/25/21 5:15 PM, Dumitru Ceara wrote: >> Dumitru Ceara (2): >> ofproto: Fix potential NULL dereference in ofproto_get_datapath_cap(). >> ofproto: Fix potential NULL dereference in >> ofproto_ct_*_zone_timeout_policy(). >> >> ofproto/ofproto.c |6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> > > Thanks, Dumitru and Paolo! > > Applied and backported down to 2.13. > > Best regards, Ilya Maximets. > Thanks! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 0/2] ofproto: Fix potential NULL dereference with invalid datapath types.
On 5/25/21 5:15 PM, Dumitru Ceara wrote: > Dumitru Ceara (2): > ofproto: Fix potential NULL dereference in ofproto_get_datapath_cap(). > ofproto: Fix potential NULL dereference in > ofproto_ct_*_zone_timeout_policy(). > > ofproto/ofproto.c |6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Thanks, Dumitru and Paolo! Applied and backported down to 2.13. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4] conntrack: document all-zero IP SNAT behavior and add a test case
On 2 Jun 2021, at 18:21, Aaron Conole wrote:
> Eelco Chaudron writes:
>
>> Currently, conntrack in the kernel has an undocumented feature referred
>> to as all-zero IP address SNAT. Basically, when a source port
>> collision is detected during the commit, the source port will be
>> translated to an ephemeral port. If there is no collision, no SNAT is
>> performed.
>>
>> This patchset documents this behavior and adds a self-test to verify
>> it's not changing. In addition, a datapath feature flag is added for
>> the all-zero IP SNAT case. This will help applications on top of OVS,
>> like OVN, to determine this feature can be used.
>>
>> Signed-off-by: Eelco Chaudron
>> ---
>> v4: Added datapath support flag for all-zero SNAT.
>> v3: Renamed NULL SNAT to all-zero IP SNAT.
>> v2: Fixed NULL SNAT to only work in the -rpl state to be inline with
>> OpenShift-SDN's behavior.
>>
>> lib/ct-dpif.c|8 +++
>> lib/ct-dpif.h|6 +
>> lib/dpif-netdev.c|1 +
>> lib/dpif-netlink.c | 11 +
>> lib/dpif-provider.h |5
>> lib/ovs-actions.xml | 10
>> ofproto/ofproto-dpif.c | 22 +-
>> ofproto/ofproto-dpif.h |5 +++-
>> tests/system-kmod-macros.at |7 ++
>> tests/system-traffic.at | 46
>> ++
>> tests/system-userspace-macros.at | 10
>> vswitchd/vswitch.xml |9 +++
>> 12 files changed, 138 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
>> index 6a5ba052d..cfc2315e3 100644
>> --- a/lib/ct-dpif.c
>> +++ b/lib/ct-dpif.c
>> @@ -889,3 +889,11 @@ ct_dpif_get_timeout_policy_name(struct dpif *dpif,
>> uint32_t tp_id,
>> dpif, tp_id, dl_type, nw_proto, tp_name, is_generic)
>> : EOPNOTSUPP);
>> }
>> +
>> +int
>> +ct_dpif_get_features(struct dpif *dpif, enum ct_features *features)
>> +{
>
> NIT-mode:
>
> Are these features or capabilities? I ask because we may want to add
> support for things like tcp loose mode, etc, and not sure if there would
> need to be a corresponding set function (to enable / disable), and how
> that should look. I'm okay with keeping it as-is for here and adding
> the corresponding set function later, but it would seem strange to try
> and "set" support for all-zero snat, etc.
Guess the line between feature and capabilities is rather thin... The code for
checking the datapath support, check_support(), is calling all of this
features, rather than capabilities.
I guess ct_zero_snat is a none configurable feature ;) But more seriously, I
could go ahead and change the naming from feature to capability. As most of the
configurable "features" have their own callback.
>> +return (dpif->dpif_class->ct_get_features
>> +? dpif->dpif_class->ct_get_features(dpif, features)
>> +: EOPNOTSUPP);
>> +}
>> diff --git a/tests/system-userspace-macros.at
>> b/tests/system-userspace-macros.at
>> index 34f82cee3..9f0d38dfb 100644
>> --- a/tests/system-userspace-macros.at
>> +++ b/tests/system-userspace-macros.at
>> @@ -96,6 +96,16 @@ m4_define([CHECK_CONNTRACK_FRAG_OVERLAP])
>> #
>> m4_define([CHECK_CONNTRACK_NAT])
>>
>> +# CHECK_CONNTRACK_ZEROIP_SNAT()
>> +#
>> +# Perform requirements checks for running conntrack all-zero IP SNAT tests.
>> +# The userspace datapath does not support all-zero IP SNAT.
>> +#
>> +m4_define([CHECK_CONNTRACK_ZEROIP_SNAT],
>> +[
>> +AT_SKIP_IF([:])
>> +])
>
> I didn't check too close, but maybe it's possible to make this check the
> capabilities bits and then it could be extended to everywhere.
I was thinking about using "ovs-appctl dpif/show-dp-features" after I added the
features check. However none of the other test cases do this, so I thought
there might be a reason? It might be that I need to configure/setup OVS to run
the test command, and not sure if there is a nice clean way to shut down if the
feature is not supported.
>> +
>> # CHECK_CONNTRACK_TIMEOUT()
>> #
>> # Perform requirements checks for running conntrack customized timeout
>> tests.
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index 4597a215d..d8ea287d5 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -6126,6 +6126,15 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
>> type=patch options:peer=p1 \
>> True if the datapath supports OVS_ACTION_ATTR_DROP. If false,
>> explicit drop action will not be sent to the datapath.
>>
>> + > + type='{"type": "boolean"}'>
>> +True if the datapath supports all-zero SNAT. This is a special case
>> +if the src IP address is configured as all 0's, i.e.,
>> +nat(src=0.0.0.0). In this case, when a source port
>> +collision is detected during the commit, the source port will be
>> +
