Re: [ovs-dev] [patch v2] conntrack: Fix ICMPv4 error data L4 length check.
> > Thanks Darrell. I have sent patches for branch 2.8 and 2.9. > For branches before 2.7 & 2.6 it is giving quite a few conflicts. > Can you please have a look at it? > Hi Darrell, Ben I have tried manual merge on OVS 2.6 branch and sent a patch. Can you please review and apply it on branch? Warm Regards, Vishal Ajmera ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH branch-2.6] conntrack: Fix ICMPv4 error data L4 length check.
From: Darrell Ball
The ICMPv4 error data L4 length check was found to be too strict for TCP,
expecting a minimum of 20 rather than 8 bytes. This worked by
hapenstance for other inner protocols. The approach is to explicitly
handle the ICMPv4 error data L4 length check and to do this for all
supported inner protocols in the same way. Making the code common
between protocols also allows the existing ICMPv4 related UDP tests to
cover TCP and ICMP inner protocol cases.
Note that ICMPv6 does not have an 8 byte limit for error L4 data.
Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.")
CC: Daniele Di Proietto
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2019-August/361949.html
Reported-by: Vishal Deep Ajmera
Signed-off-by: Vishal Deep Ajmera
Co-authored-by: Vishal Deep Ajmera
Signed-off-by: Darrell Ball
Signed-off-by: Ben Pfaff
(cherry picked from commit 6c2a93064afe8d812e4506880d1fd8f96108f92a)
Conflicts:
lib/conntrack.c
---
lib/conntrack.c | 35 ---
lib/packets.h | 3 +++
2 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 8abaf7e..d59083e 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -664,11 +664,12 @@ check_l4_icmp6(const struct conn_key *key, const void
*data, size_t size,
}
static inline bool
-extract_l4_tcp(struct conn_key *key, const void *data, size_t size)
+extract_l4_tcp(struct conn_key *key, const void *data, size_t size,
+ size_t *chk_len)
{
const struct tcp_header *tcp = data;
-if (OVS_UNLIKELY(size < TCP_HEADER_LEN)) {
+if (OVS_UNLIKELY(size < (chk_len ? *chk_len : TCP_HEADER_LEN))) {
return false;
}
@@ -680,11 +681,12 @@ extract_l4_tcp(struct conn_key *key, const void *data,
size_t size)
}
static inline bool
-extract_l4_udp(struct conn_key *key, const void *data, size_t size)
+extract_l4_udp(struct conn_key *key, const void *data, size_t size,
+ size_t *chk_len)
{
const struct udp_header *udp = data;
-if (OVS_UNLIKELY(size < UDP_HEADER_LEN)) {
+if (OVS_UNLIKELY(size < (chk_len ? *chk_len : UDP_HEADER_LEN))) {
return false;
}
@@ -696,7 +698,8 @@ extract_l4_udp(struct conn_key *key, const void *data,
size_t size)
}
static inline bool extract_l4(struct conn_key *key, const void *data,
- size_t size, bool *related, const void *l3);
+ size_t size, bool *related, const void *l3,
+ size_t *chk_len);
static uint8_t
reverse_icmp_type(uint8_t type)
@@ -728,11 +731,11 @@ reverse_icmp_type(uint8_t type)
* possible */
static inline int
extract_l4_icmp(struct conn_key *key, const void *data, size_t size,
-bool *related)
+bool *related, size_t *chk_len)
{
const struct icmp_header *icmp = data;
-if (OVS_UNLIKELY(size < ICMP_HEADER_LEN)) {
+if (OVS_UNLIKELY(size < (chk_len ? *chk_len : ICMP_HEADER_LEN))) {
return false;
}
@@ -783,8 +786,9 @@ extract_l4_icmp(struct conn_key *key, const void *data,
size_t size,
key->src = inner_key.src;
key->dst = inner_key.dst;
key->nw_proto = inner_key.nw_proto;
+size_t check_len = ICMP_ERROR_DATA_L4_LEN;
-ok = extract_l4(key, l4, tail - l4, NULL, l3);
+ok = extract_l4(key, l4, tail - l4, NULL, l3, _len);
if (ok) {
conn_key_reverse(key);
*related = true;
@@ -872,7 +876,7 @@ extract_l4_icmp6(struct conn_key *key, const void *data,
size_t size,
key->dst = inner_key.dst;
key->nw_proto = inner_key.nw_proto;
-ok = extract_l4(key, l4, tail - l4, NULL, l3);
+ok = extract_l4(key, l4, tail - l4, NULL, l3, NULL);
if (ok) {
conn_key_reverse(key);
*related = true;
@@ -897,21 +901,22 @@ extract_l4_icmp6(struct conn_key *key, const void *data,
size_t size,
* an ICMP or ICMP6 header. *
*
* If 'related' is NULL, it means that we're already parsing a header nested
- * in an ICMP error. In this case, we skip checksum and length validation. */
+ * in an ICMP error. In this case, we skip the checksum and some length
+ * validations. */
static inline bool
extract_l4(struct conn_key *key, const void *data, size_t size, bool *related,
- const void *l3)
+ const void *l3, size_t *chk_len)
{
if (key->nw_proto == IPPROTO_TCP) {
return (!related || check_l4_tcp(key, data, size, l3))
- && extract_l4_tcp(key, data, size);
+ && extract_l4_tcp(key, data, size, chk_len);
} else if (key->nw_proto == IPPROTO_UDP) {
return (!related || check_l4_udp(key, data, size, l3))
- && extract_l4_udp(key, data, size);
+ && extract_l4_udp(key, data, size, chk_len);
} else if (key->dl_type == htons(ETH_TYPE_IP)
&&
[ovs-dev] [RFC PATCH ovn 09/10] ovn-ic: Interconnection gateway controller.
From: Han Zhou
Sync local and remote gateways between SB and ISB.
Signed-off-by: Han Zhou
---
ic/ovn-ic.c | 94 +
1 file changed, 94 insertions(+)
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 4de3e17..ddc9d0a 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -234,6 +234,99 @@ ts_run(struct ic_context *ctx)
}
static void
+gateway_run(struct ic_context *ctx, const struct isbrec_availability_zone *az)
+{
+if (!ctx->ovnisb_txn || !ctx->ovnsb_txn) {
+return;
+}
+
+struct shash local_gws = SHASH_INITIALIZER(_gws);
+struct shash remote_gws = SHASH_INITIALIZER(_gws);
+const struct isbrec_gateway *gw;
+ISBREC_GATEWAY_FOR_EACH (gw, ctx->ovnisb_idl) {
+if (gw->availability_zone == az) {
+shash_add(_gws, gw->name, gw);
+} else {
+shash_add(_gws, gw->name, gw);
+}
+}
+
+const struct sbrec_chassis *chassis;
+SBREC_CHASSIS_FOR_EACH (chassis, ctx->ovnsb_idl) {
+if (chassis->is_interconn) {
+gw = shash_find_and_delete(_gws, chassis->name);
+if (!gw) {
+gw = isbrec_gateway_insert(ctx->ovnisb_txn);
+isbrec_gateway_set_availability_zone(gw, az);
+isbrec_gateway_set_name(gw, chassis->name);
+isbrec_gateway_set_hostname(gw, chassis->hostname);
+
+/* Sync encaps used by this chassis. */
+ovs_assert(chassis->n_encaps);
+struct isbrec_encap *isb_encap;
+struct isbrec_encap **isb_encaps =
+xmalloc(chassis->n_encaps * sizeof *isb_encap);
+for (int i = 0; i < chassis->n_encaps; i++) {
+isb_encap = isbrec_encap_insert(ctx->ovnisb_txn);
+isbrec_encap_set_gateway_name(isb_encap,
+ chassis->name);
+isbrec_encap_set_ip(isb_encap, chassis->encaps[i]->ip);
+isbrec_encap_set_type(isb_encap,
+ chassis->encaps[i]->type);
+isbrec_encap_set_options(isb_encap,
+ >encaps[i]->options);
+isb_encaps[i] = isb_encap;
+}
+isbrec_gateway_set_encaps(gw, isb_encaps,
+ chassis->n_encaps);
+free(isb_encaps);
+} else {
+/* XXX Update ISB gateway/encaps, if any changes. */
+}
+} else if (chassis->is_remote) {
+gw = shash_find_and_delete(_gws, chassis->name);
+if (!gw) {
+sbrec_chassis_delete(chassis);
+} else {
+/* XXX Update SB chassis/encaps, if any changes. */
+}
+}
+}
+
+/* Delete extra gateways from ISB for the local AZ */
+struct shash_node *node;
+SHASH_FOR_EACH (node, _gws) {
+isbrec_gateway_delete(node->data);
+}
+shash_destroy(_gws);
+
+/* Create SB chassis for remote gateways in ISB */
+SHASH_FOR_EACH (node, _gws) {
+gw = node->data;
+chassis = sbrec_chassis_insert(ctx->ovnsb_txn);
+sbrec_chassis_set_name(chassis, gw->name);
+sbrec_chassis_set_hostname(chassis, gw->hostname);
+sbrec_chassis_set_is_remote(chassis, true);
+/* Sync encaps used by this gateway. */
+ovs_assert(gw->n_encaps);
+struct sbrec_encap *sb_encap;
+struct sbrec_encap **sb_encaps =
+xmalloc(gw->n_encaps * sizeof *sb_encap);
+for (int i = 0; i < gw->n_encaps; i++) {
+sb_encap = sbrec_encap_insert(ctx->ovnsb_txn);
+sbrec_encap_set_chassis_name(sb_encap, gw->name);
+sbrec_encap_set_ip(sb_encap, gw->encaps[i]->ip);
+sbrec_encap_set_type(sb_encap, gw->encaps[i]->type);
+sbrec_encap_set_options(sb_encap, >encaps[i]->options);
+sb_encaps[i] = sb_encap;
+}
+sbrec_chassis_set_encaps(chassis, sb_encaps, gw->n_encaps);
+free(sb_encaps);
+}
+shash_destroy(_gws);
+}
+
+static void
ovn_db_run(struct ic_context *ctx)
{
const struct isbrec_availability_zone *az = az_run(ctx);
@@ -244,6 +337,7 @@ ovn_db_run(struct ic_context *ctx)
}
ts_run(ctx);
+gateway_run(ctx, az);
}
static void
--
2.1.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [RFC PATCH ovn 10/10] ovn-ic: Interconnection port controller.
From: Han Zhou
Sync interconnection logical ports and bindings between NB, SB
and ISB. With this patch, the OVN interconnection works end to
end.
Signed-off-by: Han Zhou
---
controller/binding.c | 6 +-
ic/ovn-ic.c| 342 +
lib/ovn-util.c | 7 +
lib/ovn-util.h | 2 +
northd/ovn-northd.c| 8 +-
ovn-architecture.7.xml | 2 +-
ovn-nb.xml | 11 +-
7 files changed, 367 insertions(+), 11 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index 242163d..e1f41b7 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -744,11 +744,13 @@ binding_evaluate_port_binding_changes(
* - If a regular VIF is unbound from this chassis, the local ovsdb
* interface table will be updated, which will trigger recompute.
*
- * - If the port is not a regular VIF, always trigger recompute. */
+ * - If the port is not a regular VIF, and not a "remote" port,
+ * always trigger recompute. */
if (binding_rec->chassis == chassis_rec
|| is_our_chassis(chassis_rec, binding_rec,
active_tunnels, _to_iface, local_lports)
-|| strcmp(binding_rec->type, "")) {
+|| (strcmp(binding_rec->type, "") && strcmp(binding_rec->type,
+"remote"))) {
changed = true;
break;
}
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index ddc9d0a..2690238 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -326,6 +326,347 @@ gateway_run(struct ic_context *ctx, const struct
isbrec_availability_zone *az)
shash_destroy(_gws);
}
+static const struct nbrec_logical_switch *
+find_ts_in_nb(struct ic_context *ctx, char *ts_name)
+{
+/* XXX: optimize with index */
+const struct nbrec_logical_switch *ls;
+bool found = false;
+NBREC_LOGICAL_SWITCH_FOR_EACH (ls, ctx->ovnnb_idl) {
+const char *ls_ts_name = smap_get(>other_config, "interconn-ts");
+if (ls_ts_name && !strcmp(ts_name, ls_ts_name)) {
+found = true;
+break;
+}
+}
+if (found) {
+return ls;
+}
+return NULL;
+}
+
+static const struct sbrec_port_binding *
+find_peer_port(struct ic_context *ctx,
+ const struct sbrec_port_binding *sb_pb)
+{
+const char *peer_name = smap_get(_pb->options, "peer");
+if (!peer_name) {
+return NULL;
+}
+/* XXX: use index */
+const struct sbrec_port_binding *pb;
+SBREC_PORT_BINDING_FOR_EACH (pb, ctx->ovnsb_idl) {
+if (!strcmp(pb->logical_port, peer_name)) {
+return pb;
+}
+}
+return NULL;
+}
+
+static const struct sbrec_port_binding *
+find_crp_from_lrp(struct ic_context *ctx,
+ const struct sbrec_port_binding *lrp_pb)
+{
+char *crp_name = ovn_chassis_redirect_name(lrp_pb->logical_port);
+
+/* XXX: use index */
+const struct sbrec_port_binding *pb, *ret;
+ret = NULL;
+SBREC_PORT_BINDING_FOR_EACH (pb, ctx->ovnsb_idl) {
+if (!strcmp(pb->logical_port, crp_name)) {
+ret = pb;
+break;
+}
+}
+free(crp_name);
+return ret;
+}
+
+static const struct sbrec_port_binding *
+find_crp_for_sb_pb(struct ic_context *ctx,
+ const struct sbrec_port_binding *sb_pb)
+{
+const struct sbrec_port_binding *peer = find_peer_port(ctx, sb_pb);
+if (!peer) {
+return NULL;
+}
+
+return find_crp_from_lrp(ctx, peer);
+}
+
+static const char *
+get_lrp_address_for_sb_pb(struct ic_context *ctx,
+ const struct sbrec_port_binding *sb_pb)
+{
+const struct sbrec_port_binding *peer = find_peer_port(ctx, sb_pb);
+if (!peer) {
+return NULL;
+}
+
+return peer->n_mac ? *peer->mac : NULL;
+}
+
+static const struct sbrec_chassis *
+find_sb_chassis(struct ic_context *ctx, const char *name)
+{
+/* XXX: use index */
+const struct sbrec_chassis *chassis;
+SBREC_CHASSIS_FOR_EACH (chassis, ctx->ovnsb_idl) {
+if (!strcmp(chassis->name, name)) {
+return chassis;
+}
+}
+return NULL;
+}
+
+/* For each local port:
+ * - Sync from NB to ISB.
+ * - Sync gateway from SB to ISB.
+ * - Sync tunnel key from ISB to SB.
+ */
+static void
+sync_local_port(struct ic_context *ctx,
+const struct isbrec_port_binding *isb_pb,
+const struct sbrec_port_binding *sb_pb)
+{
+/* Sync address from NB to ISB */
+const char *address = get_lrp_address_for_sb_pb(ctx, sb_pb);
+if (!address) {
+VLOG_DBG("Can't get logical router port address for logical"
+ " switch port %s", sb_pb->logical_port);
+if (isb_pb->address[0]) {
+isbrec_port_binding_set_address(isb_pb, "");
+}
+} else {
+if (strcmp(address, isb_pb->address)) {
+
[ovs-dev] [RFC PATCH ovn 08/10] ovn-sb: Add columns is_interconn and is_remote to Chassis.
From: Han Zhou
Support the new columns in Chassis table for OVN interconnection.
Also, populate the is_interconn column according to external_ids:
is-interconn key of Open_vSwitch table on the chassis.
Signed-off-by: Han Zhou
---
controller/chassis.c | 14 ++
northd/ovn-northd.c | 3 ++-
ovn-sb.ovsschema | 8 +---
ovn-sb.xml | 15 +++
4 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/controller/chassis.c b/controller/chassis.c
index 699b662..47ca75e 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -92,6 +92,8 @@ struct ovs_chassis_cfg {
struct sset encap_ip_set;
/* Interface type list formatted in the OVN-SB Chassis required format. */
struct ds iface_types;
+/* Is this chassis an interconnection gateway. */
+bool is_interconn;
};
static void
@@ -172,6 +174,12 @@ get_datapath_type(const struct ovsrec_bridge *br_int)
return "";
}
+static bool
+get_is_interconn(const struct smap *ext_ids)
+{
+return smap_get_bool(ext_ids, "is-interconn", false);
+}
+
static void
update_chassis_transport_zones(const struct sset *transport_zones,
const struct sbrec_chassis *chassis_rec)
@@ -285,6 +293,8 @@ chassis_parse_ovs_config(const struct
ovsrec_open_vswitch_table *ovs_table,
sset_destroy(_cfg->encap_ip_set);
}
+ovs_cfg->is_interconn = get_is_interconn(>external_ids);
+
return true;
}
@@ -539,6 +549,10 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
update_chassis_transport_zones(transport_zones, chassis_rec);
+if (ovs_cfg->is_interconn != chassis_rec->is_interconn) {
+sbrec_chassis_set_is_interconn(chassis_rec, ovs_cfg->is_interconn);
+}
+
/* If any of the encaps should change, update them. */
bool tunnels_changed =
chassis_tunnels_changed(_cfg->encap_type_set,
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 32d70f8..d9c8a0f 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -9351,7 +9351,8 @@ check_and_add_supported_dhcpv6_opts_to_sb_db(struct
northd_context *ctx)
static const char *rbac_chassis_auth[] =
{"name"};
static const char *rbac_chassis_update[] =
-{"nb_cfg", "external_ids", "encaps", "vtep_logical_switches"};
+{"nb_cfg", "external_ids", "encaps", "vtep_logical_switches",
+"is_interconn"};
static const char *rbac_encap_auth[] =
{"chassis_name"};
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index 5c013b1..30b622d 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Southbound",
-"version": "2.5.0",
-"cksum": "1257419092 20387",
+"version": "2.6.0",
+"cksum": "1645368988 20492",
"tables": {
"SB_Global": {
"columns": {
@@ -40,7 +40,9 @@
"min": 0, "max": "unlimited"}},
"transport_zones" : {"type": {"key": "string",
"min": 0,
- "max": "unlimited"}}},
+ "max": "unlimited"}},
+"is_interconn" : {"type": "boolean"},
+"is_remote" : {"type": "boolean"}},
"isRoot": true,
"indexes": [["name"]]},
"Encap": {
diff --git a/ovn-sb.xml b/ovn-sb.xml
index e5fb51a..9ba4274 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -301,6 +301,21 @@
See ovn-controller(8) for more information.
+
+ ovn-controller populates this column with the setting
+ configured in the column of the Open_vSwitch
+ database's table.
+ If set to true, the chassis is used as an interconnection gateway.
+ See ovn-controller(8) for more information.
+
+
+
+ ovn-ic set this column true for remote interconnection
+ gateway chassises learned from the interconnection southbound database.
+ See ovn-ic(8) for more information.
+
+
ovn-controller populates this key with the set of options
configured in the https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [RFC PATCH ovn 07/10] ovn-ic: Transit switch controller.
From: Han Zhou
Processing transit switches and sync between INB, ISB and NB.
Signed-off-by: Han Zhou
---
ic/ovn-ic.c | 113
northd/ovn-northd.c | 8
2 files changed, 121 insertions(+)
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 59f798d..4de3e17 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -126,11 +126,124 @@ az_run(struct ic_context *ctx)
return NULL;
}
+static uint32_t
+allocate_ts_dp_key(struct hmap *dp_tnlids)
+{
+static uint32_t hint = OVN_MIN_DP_KEY_GLOBAL;
+return ovn_allocate_tnlid(dp_tnlids, "transit switch datapath",
+ OVN_MIN_DP_KEY_GLOBAL, OVN_MAX_DP_KEY_GLOBAL,
+ );
+}
+
+static void
+ts_run(struct ic_context *ctx)
+{
+const struct inbrec_transit_switch *ts;
+
+/* Sync INB TS to AZ NB */
+if (ctx->ovnnb_txn) {
+struct shash nb_tses = SHASH_INITIALIZER(_tses);
+const struct nbrec_logical_switch *ls;
+
+/* Get current NB Logical_Switch with other_config:interconn-ts */
+NBREC_LOGICAL_SWITCH_FOR_EACH (ls, ctx->ovnnb_idl) {
+const char *ts_name = smap_get(>other_config, "interconn-ts");
+if (ts_name) {
+shash_add(_tses, ts_name, ls);
+}
+}
+
+/* Create NB Logical_Switch for each TS */
+INBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) {
+ls = shash_find_and_delete(_tses, ts->name);
+if (!ls) {
+ls = nbrec_logical_switch_insert(ctx->ovnnb_txn);
+nbrec_logical_switch_set_name(ls, ts->name);
+nbrec_logical_switch_update_other_config_setkey(ls,
+"interconn-ts",
+ts->name);
+}
+}
+
+/* Delete extra NB Logical_Switch with other_config:interconn-ts */
+struct shash_node *node;
+SHASH_FOR_EACH (node, _tses) {
+nbrec_logical_switch_delete(node->data);
+}
+shash_destroy(_tses);
+}
+
+struct hmap dp_tnlids = HMAP_INITIALIZER(_tnlids);
+struct shash isb_dps = SHASH_INITIALIZER(_dps);
+const struct isbrec_datapath_binding *isb_dp;
+ISBREC_DATAPATH_BINDING_FOR_EACH (isb_dp, ctx->ovnisb_idl) {
+shash_add(_dps, isb_dp->transit_switch, isb_dp);
+ovn_add_tnlid(_tnlids, isb_dp->tunnel_key);
+}
+
+/* Sync ISB TS tunnel key to AZ SB datapath. (AZ SB datapath is created by
+ * northd.) */
+if (ctx->ovnsb_txn) {
+const struct sbrec_datapath_binding *sb_dp;
+SBREC_DATAPATH_BINDING_FOR_EACH (sb_dp, ctx->ovnsb_idl) {
+const char *ts_name = smap_get(_dp->external_ids,
+ "interconn-ts");
+if (ts_name) {
+isb_dp = shash_find_data(_dps, ts_name);
+if (!isb_dp) {
+VLOG_DBG("SB datapath "UUID_FMT" with interconn-ts %s not "
+ "found in ISB, ignore.",
+ UUID_ARGS(_dp->header_.uuid),
+ ts_name);
+continue;
+}
+sbrec_datapath_binding_set_tunnel_key(sb_dp,
+ isb_dp->tunnel_key);
+}
+}
+}
+
+/* Sync TS between INB and ISB. This is performed after syncing with AZ
+ * SB, to avoid uncommitted ISB datapath tunnel key to be synced back to
+ * AZ. */
+if (ctx->ovnisb_txn) {
+/* Create ISB Datapath_Binding */
+INBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) {
+isb_dp = shash_find_and_delete(_dps, ts->name);
+if (!isb_dp) {
+/* Allocate tunnel key */
+int64_t dp_key = allocate_ts_dp_key(_tnlids);
+if (!dp_key) {
+continue;
+}
+
+isb_dp = isbrec_datapath_binding_insert(ctx->ovnisb_txn);
+isbrec_datapath_binding_set_transit_switch(isb_dp, ts->name);
+isbrec_datapath_binding_set_tunnel_key(isb_dp, dp_key);
+}
+}
+
+/* Delete extra ISB Datapath_Binding */
+struct shash_node *node;
+SHASH_FOR_EACH (node, _dps) {
+isbrec_datapath_binding_delete(node->data);
+}
+}
+ovn_destroy_tnlids(_tnlids);
+shash_destroy(_dps);
+}
+
static void
ovn_db_run(struct ic_context *ctx)
{
const struct isbrec_availability_zone *az = az_run(ctx);
VLOG_DBG("Availability zone: %s", az ? az->name : "not created yet.");
+
+if (!az) {
+return;
+}
+
+ts_run(ctx);
}
static void
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index f99adfb..32d70f8 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -823,6 +823,14 @@
[ovs-dev] [RFC PATCH ovn 04/10] ovn-isb: Interconnection southbound DB schema and CLI.
From: Han Zhou
This patch introduces OVN_IC_Southbound DB schema and the CLI
ovn-isbctl that manages the DB.
Signed-off-by: Han Zhou
---
.gitignore | 3 +
automake.mk| 38 ++
debian/ovn-common.install | 1 +
debian/ovn-common.manpages | 2 +
lib/.gitignore | 3 +
lib/automake.mk| 17 +-
lib/ovn-isb-idl.ann| 9 +
lib/ovn-util.c | 13 +
lib/ovn-util.h | 1 +
ovn-isb.ovsschema | 130 +++
ovn-isb.xml| 588 ++
utilities/.gitignore | 2 +
utilities/automake.mk | 5 +
utilities/ovn-isbctl.c | 890 +
14 files changed, 1701 insertions(+), 1 deletion(-)
create mode 100644 lib/ovn-isb-idl.ann
create mode 100644 ovn-isb.ovsschema
create mode 100644 ovn-isb.xml
create mode 100644 utilities/ovn-isbctl.c
diff --git a/.gitignore b/.gitignore
index 1994937..66bb101 100644
--- a/.gitignore
+++ b/.gitignore
@@ -70,6 +70,9 @@
/ovn-inb.5
/ovn-inb.gv
/ovn-inb.pic
+/ovn-isb.5
+/ovn-isb.gv
+/ovn-isb.pic
/package.m4
/stamp-h1
/_build-gcc
diff --git a/automake.mk b/automake.mk
index 3bfbf57..85574fc 100644
--- a/automake.mk
+++ b/automake.mk
@@ -97,6 +97,37 @@ ovn-inb.5: \
$(srcdir)/ovn-inb.xml > $@.tmp && \
mv $@.tmp $@
+# OVN interconnection southbound E-R diagram
+#
+# If "python" or "dot" is not available, then we do not add graphical diagram
+# to the documentation.
+if HAVE_PYTHON
+if HAVE_DOT
+ovn-isb.gv: ${OVSDIR}/ovsdb/ovsdb-dot.in $(srcdir)/ovn-isb.ovsschema
+ $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn-isb.ovsschema > $@
+ovn-isb.pic: ovn-isb.gv ${OVSDIR}/ovsdb/dot2pic
+ $(AM_V_GEN)(dot -T plain < ovn-isb.gv | $(PYTHON)
${OVSDIR}/ovsdb/dot2pic -f 3) > $@.tmp && \
+ mv $@.tmp $@
+OVN_ISB_PIC = ovn-isb.pic
+OVN_ISB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_ISB_PIC)
+CLEANFILES += ovn-isb.gv ovn-isb.pic
+endif
+endif
+
+# OVN interconnection southbound schema documentation
+EXTRA_DIST += ovn-isb.xml
+CLEANFILES += ovn-isb.5
+man_MANS += ovn-isb.5
+
+ovn-isb.5: \
+ ${OVSDIR}/ovsdb/ovsdb-doc $(srcdir)/ovn-isb.xml
$(srcdir)/ovn-isb.ovsschema $(OVN_ISB_PIC)
+ $(AM_V_GEN)$(OVSDB_DOC) \
+ $(OVN_ISB_DOT_DIAGRAM_ARG) \
+ --version=$(VERSION) \
+ $(srcdir)/ovn-isb.ovsschema \
+ $(srcdir)/ovn-isb.xml > $@.tmp && \
+ mv $@.tmp $@
+
# Version checking for ovn-nb.ovsschema.
ALL_LOCAL += ovn-nb.ovsschema.stamp
ovn-nb.ovsschema.stamp: ovn-nb.ovsschema
@@ -114,8 +145,15 @@ ovn-inb.ovsschema.stamp: ovn-inb.ovsschema
$(srcdir)/build-aux/cksum-schema-check $? $@
CLEANFILES += ovn-inb.ovsschema.stamp
+# Version checking for ovn-isb.ovsschema.
+ALL_LOCAL += ovn-isb.ovsschema.stamp
+ovn-isb.ovsschema.stamp: ovn-isb.ovsschema
+ $(srcdir)/build-aux/cksum-schema-check $? $@
+CLEANFILES += ovn-isb.ovsschema.stamp
+
pkgdata_DATA += ovn-nb.ovsschema
pkgdata_DATA += ovn-sb.ovsschema
pkgdata_DATA += ovn-inb.ovsschema
+pkgdata_DATA += ovn-isb.ovsschema
CLEANFILES += ovn-sb.ovsschema.stamp
diff --git a/debian/ovn-common.install b/debian/ovn-common.install
index 9e9bcfb..8f8ab23 100644
--- a/debian/ovn-common.install
+++ b/debian/ovn-common.install
@@ -1,6 +1,7 @@
usr/bin/ovn-nbctl
usr/bin/ovn-sbctl
usr/bin/ovn-inbctl
+usr/bin/ovn-isbctl
usr/bin/ovn-trace
usr/bin/ovn-detrace
usr/share/openvswitch/scripts/ovn-ctl
diff --git a/debian/ovn-common.manpages b/debian/ovn-common.manpages
index 94325dd..895da8d 100644
--- a/debian/ovn-common.manpages
+++ b/debian/ovn-common.manpages
@@ -2,9 +2,11 @@ ovn/ovn-architecture.7
ovn/ovn-nb.5
ovn/ovn-sb.5
ovn/ovn-inb.5
+ovn/ovn-isb.5
ovn/utilities/ovn-ctl.8
ovn/utilities/ovn-nbctl.8
ovn/utilities/ovn-sbctl.8
ovn/utilities/ovn-inbctl.8
+ovn/utilities/ovn-isbctl.8
ovn/utilities/ovn-trace.8
ovn/utilities/ovn-detrace.1
diff --git a/lib/.gitignore b/lib/.gitignore
index e5d9bf3..19cc9f5 100644
--- a/lib/.gitignore
+++ b/lib/.gitignore
@@ -8,4 +8,7 @@
/ovn-inb-idl.c
/ovn-inb-idl.h
/ovn-inb-idl.ovsidl
+/ovn-isb-idl.c
+/ovn-isb-idl.h
+/ovn-isb-idl.ovsidl
/ovn-dirs.c
diff --git a/lib/automake.mk b/lib/automake.mk
index 83fdbcd..dc62be7 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -31,7 +31,9 @@ nodist_lib_libovn_la_SOURCES = \
lib/ovn-sb-idl.c \
lib/ovn-sb-idl.h \
lib/ovn-inb-idl.c \
- lib/ovn-inb-idl.h
+ lib/ovn-inb-idl.h \
+ lib/ovn-isb-idl.c \
+ lib/ovn-isb-idl.h
CLEANFILES += $(nodist_lib_libovn_la_SOURCES)
@@ -89,3 +91,16 @@ lib/ovn-inb-idl.ovsidl: $(OVN_INB_IDL_FILES)
$(AM_V_GEN)$(OVSDB_IDLC) annotate $(OVN_INB_IDL_FILES) > $@.tmp && \
mv $@.tmp $@
+# ovn-isb IDL
+OVSIDL_BUILT += \
+ lib/ovn-isb-idl.c \
+ lib/ovn-isb-idl.h \
+ lib/ovn-isb-idl.ovsidl
+EXTRA_DIST += lib/ovn-isb-idl.ann
+OVN_ISB_IDL_FILES = \
+
[ovs-dev] [RFC PATCH ovn 05/10] ovn-ic: Interconnection controller with AZ registeration.
From: Han Zhou This patch introduces interconnection controller, ovn-ic, and implements the basic AZ registration feature: taking the AZ name from NB DB and create an Availability_Zone entry in IC-SB DB. Signed-off-by: Han Zhou --- Makefile.am | 1 + ic/.gitignore| 2 + ic/automake.mk | 10 ++ ic/ovn-ic.8.xml | 111 +++ ic/ovn-ic.c | 393 +++ ovn-nb.ovsschema | 5 +- ovn-nb.xml | 7 + tutorial/ovs-sandbox | 2 +- 8 files changed, 528 insertions(+), 3 deletions(-) create mode 100644 ic/.gitignore create mode 100644 ic/automake.mk create mode 100644 ic/ovn-ic.8.xml create mode 100644 ic/ovn-ic.c diff --git a/Makefile.am b/Makefile.am index 33c18c5..d22a220 100644 --- a/Makefile.am +++ b/Makefile.am @@ -500,4 +500,5 @@ include selinux/automake.mk include controller/automake.mk include controller-vtep/automake.mk include northd/automake.mk +include ic/automake.mk include build-aux/automake.mk diff --git a/ic/.gitignore b/ic/.gitignore new file mode 100644 index 000..1b73eb4 --- /dev/null +++ b/ic/.gitignore @@ -0,0 +1,2 @@ +/ovn-ic +/ovn-ic.8 diff --git a/ic/automake.mk b/ic/automake.mk new file mode 100644 index 000..8e71bc3 --- /dev/null +++ b/ic/automake.mk @@ -0,0 +1,10 @@ +# ovn-ic +bin_PROGRAMS += ic/ovn-ic +ic_ovn_ic_SOURCES = ic/ovn-ic.c +ic_ovn_ic_LDADD = \ + lib/libovn.la \ + $(OVSDB_LIBDIR)/libovsdb.la \ + $(OVS_LIBDIR)/libopenvswitch.la +man_MANS += ic/ovn-ic.8 +EXTRA_DIST += ic/ovn-ic.8.xml +CLEANFILES += ic/ovn-ic.8 diff --git a/ic/ovn-ic.8.xml b/ic/ovn-ic.8.xml new file mode 100644 index 000..7b2a333 --- /dev/null +++ b/ic/ovn-ic.8.xml @@ -0,0 +1,111 @@ + + +Name +ovn-ic -- Open Virtual Network interconnection controller + +Synopsis +ovn-ic [options] + +Description + + ovn-ic, OVN interconnection controller, is a centralized + daemon which communicates with global interconnection databases INB/ISB + to configure and exchange data with local NB/SB for interconnecting + with other OVN deployments. + + +Options + + --ovnnb-db=database + +The OVSDB database containing the OVN Northbound Database. If the +OVN_NB_DB environment variable is set, its value is used +as the default. Otherwise, the default is +unix:@RUNDIR@/ovnnb_db.sock. + + --ovnsb-db=database + +The OVSDB database containing the OVN Southbound Database. If the +OVN_SB_DB environment variable is set, its value is used +as the default. Otherwise, the default is +unix:@RUNDIR@/ovnsb_db.sock. + + --ovninb-db=database + +The OVSDB database containing the OVN Interconnection Northbound +Database. If the OVN_INB_DB environment variable is set, +its value is used as the default. Otherwise, the default is +unix:@RUNDIR@/ovninb_db.sock. + + --ovnisb-db=database + +The OVSDB database containing the OVN Interconnection Southbound +Database. If the OVN_ISB_DB environment variable is set, +its value is used as the default. Otherwise, the default is +unix:@RUNDIR@/ovnisb_db.sock. + + + + database in the above options must be an OVSDB active or + passive connection method, as described in ovsdb(7). + + +Daemon Options +http://www.w3.org/2003/XInclude"/> + +Logging Options +http://www.w3.org/2003/XInclude"/> + +PKI Options + + PKI configuration is required in order to use SSL for the connections to + the Northbound and Southbound databases. + +http://www.w3.org/2003/XInclude"/> + +Other Options +http://www.w3.org/2003/XInclude"/> + +http://www.w3.org/2003/XInclude"/> + +Runtime Management Commands + + ovs-appctl can send commands to a running + ovn-ic process. The currently supported commands + are described below. + + exit + +Causes ovn-ic to gracefully terminate. + + + pause + +Pauses the ovn-ic operation from processing any Northbound and +Southbound database changes. + + + resume + +Resumes the ovn-ic operation to process Northbound and +Southbound database contents and generate logical flows. + + + is-paused + +Returns "true" if ovn-ic is currently paused, "false" otherwise. + + + + +Active-Standby for High Availability + + You may run ovn-ic more than once in an OVN deployment. + OVN will automatically ensure that only one of them is active at a time. + If multiple instances of ovn-ic are running and the + active ovn-ic fails, one of the hot standby instances + of ovn-ic will automatically take over. + + diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c new file mode
[ovs-dev] [RFC PATCH ovn 06/10] ovn-northd.c: Refactor allocate_tnlid.
From: Han Zhou
Move allocate_tnlid() and related interfaces to ovn_util module,
so that they be reused by ovn-ic (in next patches). At the same
time, define macros for the range of datapath tunnel keys, and
reserve a range with ((1u << 16) - 1) keys for global transit
switch datapaths, among the ((1u << 24) - 1) datapath tunnel key
space.
Signed-off-by: Han Zhou
---
lib/ovn-util.c | 59 +
lib/ovn-util.h | 11 +++
northd/ovn-northd.c | 85 +
3 files changed, 84 insertions(+), 71 deletions(-)
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index 8d7db44..950e86f 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -449,3 +449,62 @@ ovn_logical_flow_hash(const struct uuid *logical_datapath,
hash = hash_string(match, hash);
return hash_string(actions, hash);
}
+
+struct tnlid_node {
+struct hmap_node hmap_node;
+uint32_t tnlid;
+};
+
+void
+ovn_destroy_tnlids(struct hmap *tnlids)
+{
+struct tnlid_node *node;
+HMAP_FOR_EACH_POP (node, hmap_node, tnlids) {
+free(node);
+}
+hmap_destroy(tnlids);
+}
+
+void
+ovn_add_tnlid(struct hmap *set, uint32_t tnlid)
+{
+struct tnlid_node *node = xmalloc(sizeof *node);
+hmap_insert(set, >hmap_node, hash_int(tnlid, 0));
+node->tnlid = tnlid;
+}
+
+static bool
+tnlid_in_use(const struct hmap *set, uint32_t tnlid)
+{
+const struct tnlid_node *node;
+HMAP_FOR_EACH_IN_BUCKET (node, hmap_node, hash_int(tnlid, 0), set) {
+if (node->tnlid == tnlid) {
+return true;
+}
+}
+return false;
+}
+
+static uint32_t
+next_tnlid(uint32_t tnlid, uint32_t min, uint32_t max)
+{
+return tnlid + 1 <= max ? tnlid + 1 : min;
+}
+
+uint32_t
+ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min, uint32_t
max,
+ uint32_t *hint)
+{
+for (uint32_t tnlid = next_tnlid(*hint, min, max); tnlid != *hint;
+ tnlid = next_tnlid(tnlid, min, max)) {
+if (!tnlid_in_use(set, tnlid)) {
+ovn_add_tnlid(set, tnlid);
+*hint = tnlid;
+return tnlid;
+}
+}
+
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+VLOG_WARN_RL(, "all %s tunnel ids exhausted", name);
+return 0;
+}
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 694c953..b9bda8d 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -89,4 +89,15 @@ uint32_t ovn_logical_flow_hash(const struct uuid
*logical_datapath,
uint16_t priority,
const char *match, const char *actions);
+#define OVN_MAX_DP_KEY ((1u << 24) - 1)
+#define OVN_MAX_DP_GLOBAL_NUM ((1u << 16) - 1)
+#define OVN_MIN_DP_KEY_LOCAL 1
+#define OVN_MAX_DP_KEY_LOCAL (OVN_MAX_DP_KEY - OVN_MAX_DP_GLOBAL_NUM)
+#define OVN_MIN_DP_KEY_GLOBAL (OVN_MAX_DP_KEY_LOCAL + 1)
+#define OVN_MAX_DP_KEY_GLOBAL OVN_MAX_DP_KEY
+struct hmap;
+void ovn_destroy_tnlids(struct hmap *tnlids);
+void ovn_add_tnlid(struct hmap *set, uint32_t tnlid);
+uint32_t ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min,
+uint32_t max, uint32_t *hint);
#endif
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 1bb60ce..f99adfb 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -279,65 +279,6 @@ Options:\n\
stream_usage("database", true, true, false);
}
-struct tnlid_node {
-struct hmap_node hmap_node;
-uint32_t tnlid;
-};
-
-static void
-destroy_tnlids(struct hmap *tnlids)
-{
-struct tnlid_node *node;
-HMAP_FOR_EACH_POP (node, hmap_node, tnlids) {
-free(node);
-}
-hmap_destroy(tnlids);
-}
-
-static void
-add_tnlid(struct hmap *set, uint32_t tnlid)
-{
-struct tnlid_node *node = xmalloc(sizeof *node);
-hmap_insert(set, >hmap_node, hash_int(tnlid, 0));
-node->tnlid = tnlid;
-}
-
-static bool
-tnlid_in_use(const struct hmap *set, uint32_t tnlid)
-{
-const struct tnlid_node *node;
-HMAP_FOR_EACH_IN_BUCKET (node, hmap_node, hash_int(tnlid, 0), set) {
-if (node->tnlid == tnlid) {
-return true;
-}
-}
-return false;
-}
-
-static uint32_t
-next_tnlid(uint32_t tnlid, uint32_t min, uint32_t max)
-{
-return tnlid + 1 <= max ? tnlid + 1 : min;
-}
-
-static uint32_t
-allocate_tnlid(struct hmap *set, const char *name, uint32_t min, uint32_t max,
- uint32_t *hint)
-{
-for (uint32_t tnlid = next_tnlid(*hint, min, max); tnlid != *hint;
- tnlid = next_tnlid(tnlid, min, max)) {
-if (!tnlid_in_use(set, tnlid)) {
-add_tnlid(set, tnlid);
-*hint = tnlid;
-return tnlid;
-}
-}
-
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-VLOG_WARN_RL(, "all %s tunnel ids exhausted", name);
-return 0;
-}
-
struct ovn_chassis_qdisc_queues {
struct hmap_node key_node;
uint32_t queue_id;
@@ -488,9 +429,9 @@
[ovs-dev] [RFC PATCH ovn 03/10] ovn-inb: Interconnection northbound DB schema and CLI.
From: Han Zhou
This patch introduces OVN_IC_Northbound DB schema and the CLI
ovn-inbctl that manages the DB.
Signed-off-by: Han Zhou
---
.gitignore | 3 +
automake.mk| 37 ++
debian/ovn-common.install | 1 +
debian/ovn-common.manpages | 2 +
lib/.gitignore | 3 +
lib/automake.mk| 17 +-
lib/ovn-inb-idl.ann| 9 +
lib/ovn-util.c | 13 +
lib/ovn-util.h | 1 +
ovn-inb.ovsschema | 76
ovn-inb.xml| 377 +++
utilities/.gitignore | 2 +
utilities/automake.mk | 5 +
utilities/ovn-inbctl.c | 879 +
14 files changed, 1424 insertions(+), 1 deletion(-)
create mode 100644 lib/ovn-inb-idl.ann
create mode 100644 ovn-inb.ovsschema
create mode 100644 ovn-inb.xml
create mode 100644 utilities/ovn-inbctl.c
diff --git a/.gitignore b/.gitignore
index 6fee075..1994937 100644
--- a/.gitignore
+++ b/.gitignore
@@ -67,6 +67,9 @@
/ovn-sb.5
/ovn-sb.gv
/ovn-sb.pic
+/ovn-inb.5
+/ovn-inb.gv
+/ovn-inb.pic
/package.m4
/stamp-h1
/_build-gcc
diff --git a/automake.mk b/automake.mk
index ad801f1..3bfbf57 100644
--- a/automake.mk
+++ b/automake.mk
@@ -66,6 +66,36 @@ ovn-sb.5: \
$(srcdir)/ovn-sb.xml > $@.tmp && \
mv $@.tmp $@
+# OVN interconnection northbound E-R diagram
+#
+# If "python" or "dot" is not available, then we do not add graphical diagram
+# to the documentation.
+if HAVE_PYTHON
+if HAVE_DOT
+ovn-inb.gv: ${OVSDIR}/ovsdb/ovsdb-dot.in $(srcdir)/ovn-inb.ovsschema
+ $(AM_V_GEN)$(OVSDB_DOT) --no-arrows $(srcdir)/ovn-inb.ovsschema > $@
+ovn-inb.pic: ovn-inb.gv ${OVSDIR}/ovsdb/dot2pic
+ $(AM_V_GEN)(dot -T plain < ovn-inb.gv | $(PYTHON)
${OVSDIR}/ovsdb/dot2pic -f 3) > $@.tmp && \
+ mv $@.tmp $@
+OVN_INB_PIC = ovn-inb.pic
+OVN_INB_DOT_DIAGRAM_ARG = --er-diagram=$(OVN_INB_PIC)
+CLEANFILES += ovn-inb.gv ovn-inb.pic
+endif
+endif
+
+# OVN interconnection northbound schema documentation
+EXTRA_DIST += ovn-inb.xml
+CLEANFILES += ovn-inb.5
+man_MANS += ovn-inb.5
+
+ovn-inb.5: \
+ ${OVSDIR}/ovsdb/ovsdb-doc $(srcdir)/ovn-inb.xml
$(srcdir)/ovn-inb.ovsschema $(OVN_INB_PIC)
+ $(AM_V_GEN)$(OVSDB_DOC) \
+ $(OVN_INB_DOT_DIAGRAM_ARG) \
+ --version=$(VERSION) \
+ $(srcdir)/ovn-inb.ovsschema \
+ $(srcdir)/ovn-inb.xml > $@.tmp && \
+ mv $@.tmp $@
# Version checking for ovn-nb.ovsschema.
ALL_LOCAL += ovn-nb.ovsschema.stamp
@@ -78,7 +108,14 @@ ALL_LOCAL += ovn-sb.ovsschema.stamp
ovn-sb.ovsschema.stamp: ovn-sb.ovsschema
$(srcdir)/build-aux/cksum-schema-check $? $@
+# Version checking for ovn-inb.ovsschema.
+ALL_LOCAL += ovn-inb.ovsschema.stamp
+ovn-inb.ovsschema.stamp: ovn-inb.ovsschema
+ $(srcdir)/build-aux/cksum-schema-check $? $@
+CLEANFILES += ovn-inb.ovsschema.stamp
+
pkgdata_DATA += ovn-nb.ovsschema
pkgdata_DATA += ovn-sb.ovsschema
+pkgdata_DATA += ovn-inb.ovsschema
CLEANFILES += ovn-sb.ovsschema.stamp
diff --git a/debian/ovn-common.install b/debian/ovn-common.install
index 90484d2..9e9bcfb 100644
--- a/debian/ovn-common.install
+++ b/debian/ovn-common.install
@@ -1,5 +1,6 @@
usr/bin/ovn-nbctl
usr/bin/ovn-sbctl
+usr/bin/ovn-inbctl
usr/bin/ovn-trace
usr/bin/ovn-detrace
usr/share/openvswitch/scripts/ovn-ctl
diff --git a/debian/ovn-common.manpages b/debian/ovn-common.manpages
index 249349e..94325dd 100644
--- a/debian/ovn-common.manpages
+++ b/debian/ovn-common.manpages
@@ -1,8 +1,10 @@
ovn/ovn-architecture.7
ovn/ovn-nb.5
ovn/ovn-sb.5
+ovn/ovn-inb.5
ovn/utilities/ovn-ctl.8
ovn/utilities/ovn-nbctl.8
ovn/utilities/ovn-sbctl.8
+ovn/utilities/ovn-inbctl.8
ovn/utilities/ovn-trace.8
ovn/utilities/ovn-detrace.1
diff --git a/lib/.gitignore b/lib/.gitignore
index 3eed458..e5d9bf3 100644
--- a/lib/.gitignore
+++ b/lib/.gitignore
@@ -5,4 +5,7 @@
/ovn-sb-idl.c
/ovn-sb-idl.h
/ovn-sb-idl.ovsidl
+/ovn-inb-idl.c
+/ovn-inb-idl.h
+/ovn-inb-idl.ovsidl
/ovn-dirs.c
diff --git a/lib/automake.mk b/lib/automake.mk
index 0c8245c..83fdbcd 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -29,7 +29,9 @@ nodist_lib_libovn_la_SOURCES = \
lib/ovn-nb-idl.c \
lib/ovn-nb-idl.h \
lib/ovn-sb-idl.c \
- lib/ovn-sb-idl.h
+ lib/ovn-sb-idl.h \
+ lib/ovn-inb-idl.c \
+ lib/ovn-inb-idl.h
CLEANFILES += $(nodist_lib_libovn_la_SOURCES)
@@ -74,3 +76,16 @@ lib/ovn-nb-idl.ovsidl: $(OVN_NB_IDL_FILES)
$(AM_V_GEN)$(OVSDB_IDLC) annotate $(OVN_NB_IDL_FILES) > $@.tmp && \
mv $@.tmp $@
+# ovn-inb IDL
+OVSIDL_BUILT += \
+ lib/ovn-inb-idl.c \
+ lib/ovn-inb-idl.h \
+ lib/ovn-inb-idl.ovsidl
+EXTRA_DIST += lib/ovn-inb-idl.ann
+OVN_INB_IDL_FILES = \
+ $(srcdir)/ovn-inb.ovsschema \
+ $(srcdir)/lib/ovn-inb-idl.ann
+lib/ovn-inb-idl.ovsidl: $(OVN_INB_IDL_FILES)
+ $(AM_V_GEN)$(OVSDB_IDLC) annotate $(OVN_INB_IDL_FILES) > $@.tmp &&
[ovs-dev] [RFC PATCH ovn 02/10] ovn-architecture: Add documentation for OVN interconnection feature.
From: Han Zhou Signed-off-by: Han Zhou --- ovn-architecture.7.xml | 107 - 1 file changed, 106 insertions(+), 1 deletion(-) diff --git a/ovn-architecture.7.xml b/ovn-architecture.7.xml index 7966b65..56b2167 100644 --- a/ovn-architecture.7.xml +++ b/ovn-architecture.7.xml @@ -1246,7 +1246,14 @@ Distributed gateway ports are logical router patch ports that directly connect distributed logical routers to logical -switches with localnet ports. +switches with external connection. + + + +There are two types of external connections. Firstly, connection to +physical network through a localnet port. Secondly, connection to +another OVN deployment, which will be introduced in section "OVN +Deployments Interconnection". @@ -1801,6 +1808,104 @@ + OVN Deployments Interconnection (TODO) + + +It is not uncommon for an operator to deploy multiple OVN clusters, for +two main reasons. Firstly, an operator may prefer to deploy one OVN +cluster for each availability zone, e.g. in different physical regions, +to avoid single point of failure. Secondly, there is always an upper limit +for a single OVN control plane to scale. + + + +Although the control planes of the different availability zone (AZ)s are +independent from each other, the workloads from different AZs may need +to communicate across the zones. The OVN interconnection feature provides +a native way to interconnect different AZs by L3 routing through transit +overlay networks between logical routers of different AZs. + + + +A global OVN Interconnection Northbound database is introduced for the +operator (probably through CMS systems) to configure transit logical +switches that connect logical routers from different AZs. A transit +switch is similar as a regular logical switch, but it is used for +interconnection purpose only. Typically, each transit switch can be used +to connect all logical routers that belong to same tenant across all AZs. + + + +A dedicated daemon process ovn-ic, OVN interconnection +controller, in each AZ will consume this data and populate corresponding +logical switches to their own northbound databases for each AZ, so that +logical routers can be connected to the transit switch by creating +patch port pairs in their northbound databases. Any router ports +connected to the transit switches are considered interconnection ports, +which will be exchanged between AZs. + + + +Physically, when workloads in from different AZs communicate, packets +need to go through multiple hops: source chassis, source gateway, +destination gateway and destination chassis. All these hops are connected +through tunnels so that the packets never leave overlay networks. +A distributed gateway port is required to connect the logical router to a +transit switch, with a gateway chassis specified, so that the traffic can +be forwarded through the gateway chassis. + + + +A global OVN Interconnection Southbound database is introduced for +exchanging control plane information between the AZs. The data in +this database is populated and consumed by the ovn-ic, +of each AZ. The main information in this database includes: + + + + + Datapath bindings for transit switches, which mainly contains the tunnel + keys generated for each transit switch. Separate key ranges are reserved + for transit switches so that they will never conflict with any tunnel + keys locally assigned for datapaths within each AZ. + + + Availability zones, which are registerd by ovn-ic + from each AZ. + + + Gateways. Each AZ specifies chassises that are supposed to work + as interconnection gateways, and the ovn-ic will + populate this information to the interconnection southbound DB. + The ovn-ic from all the other AZs will learn the + gateways and populate to their own southbound DB as a chassis. + + + Port bindings for logical switch ports created on the transit switch. + Each AZ maintains their logical router to transit switch connections + independently, but ovn-ic automatically populates + local port bindings on transit switches to the global interconnection + southbound DB, and learns remote port bindings from other AZs back + to its own northbound and southbound DBs, so that logical flows + can be produced and then translated to OVS flows locally, which finally + enables data plane communication. + + + + +The tunnel keys for transit switch datapaths and related port bindings +must be agreed across all AZs. This is ensured by generating and storing +the keys in the global interconnection southbound database. Any +ovn-ic from any AZ can allocate the key, but race
[ovs-dev] [RFC PATCH ovn 01/10] ovn-northd.c: Fix datapath tunnel key allocation.
From: Han Zhou
The max tunnel key for datapath is defined as (1u << 24) - 1, but
we are using uint16_t variable to hold the value, which will result
in duplicated key when there are enough number of datapath key
allocation and deletions.
Signed-off-by: Han Zhou
---
northd/ovn-northd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 2df95c1..1bb60ce 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -1006,7 +1006,7 @@ build_datapaths(struct northd_context *ctx, struct hmap
*datapaths,
/* Add southbound record for each unmatched northbound record. */
LIST_FOR_EACH (od, list, _only) {
-uint16_t tunnel_key = ovn_datapath_allocate_key(_tnlids);
+uint32_t tunnel_key = ovn_datapath_allocate_key(_tnlids);
if (!tunnel_key) {
break;
}
--
2.1.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [RFC PATCH ovn 00/10] OVN Interconnection
The series supports interconnecting multiple OVN deployments (e.g. located at multiple data centers) through logical routers connected with tansit logical switches with overlay tunnels, managed through OVN control plane. See the ovn-architecture document updates for more details. The series is now RFC to get feedback only. Here are the major TODOs before sending as formal patch: - CLI improvements. Now there are only basic database commands supported. More commands will be supported and documentation will be added. - Test cases. Although it is manually tested with multiple OVN setups with basic senarios, test cases will need to be added and more cases covered. - Sandbox support. - Documentation improvements. - Code optimizations. Below are the steps to try/test: 1. Precondition: two or more independent OVN environments are up and running with this patch, each with its own logical switches/routers and workloads. 2. Create the global IC-NB and IC-SB DBs on any node: ovsdb-tool create inb.db ../../ovn-inb.ovsschema ovsdb-tool create isb.db ../../ovn-isb.ovsschema ovsdb-server --detach --no-chdir --pidfile=icdb.pid -vconsole:off \ --log-file=icdb.log -vsyslog:off \ --unixctl=icdb \ --remote=ptcp:6649:0.0.0.0 \ --remote=punix:icdb.ovsdb inb.db isb.db 3. On central node of each OVN, start ovn-ic daemon: export OVN_INB_DB=tcp::6649 export OVN_ISB_DB=tcp::6649 # Specify NB/SB DB if not using default connection URL. ovn-ic --detach --no-chdir --pidfile=ovn-ic.pid -vconsole:off \ --log-file=ovn-ic.log -vsyslog:off 4. For each OVN, set AZ name in NB: e.g. ovn-nbctl set nb . name="az1" 5. Create a transit switch in global IC-NB: ovn-inbctl create transit_switch name=ts1 6. For each OVN, specify one or more chassis as gateway node(s) for interconnection by running below command on the chassis: ovs-vsctl set open . external_ids:is-interconn=true 7. For each OVN, create distributed gateway port(s) and its peer on transit switch, e.g.: # In OVN az1: ovn-nbctl lrp-add lr1 lrp-lr1-ts1 aa:aa:aa:00:00:01 169.254.100.1/24 ovn-nbctl lrp-set-gateway-chassis lrp-lr1-ts1 gw1 ovn-nbctl lsp-add ts1 lsp-ts1-lr1 ovn-nbctl lsp-set-addresses lsp-ts1-lr1 router ovn-nbctl lsp-set-type lsp-ts1-lr1 router ovn-nbctl lsp-set-options lsp-ts1-lr1 router-port=lrp-lr1-ts1 # In OVN az2: ovn-nbctl lrp-add lr2 lrp-lr2-ts1 aa:aa:aa:00:00:02 169.254.100.2/24 ovn-nbctl lrp-set-gateway-chassis lrp-lr2-ts1 gw2 ovn-nbctl lsp-add ts1 lsp-ts1-lr2 ovn-nbctl lsp-set-addresses lsp-ts1-lr2 router ovn-nbctl lsp-set-type lsp-ts1-lr2 router ovn-nbctl lsp-set-options lsp-ts1-lr2 router-port=lrp-lr2-ts1 8. For each OVN, create the static routes to route to remote workload subnets, e.g.: In OVN az1, there are workloads using 10.0.1.0/24 under lr1; in OVN az2, there are workloads using 10.0.2.0/24 under lr2. # In az1, add below route: ovn-nbctl lr-route-add lr1 10.0.2.0/24 169.254.100.2 # In az1, add below route: ovn-nbctl lr-route-add lr1 10.0.1.0/24 169.254.100.1 9. Try ping between the workloads in different OVNs. Han Zhou (10): ovn-northd.c: Fix datapath tunnel key allocation. ovn-architecture: Add documentation for OVN interconnection feature. ovn-inb: Interconnection northbound DB schema and CLI. ovn-isb: Interconnection southbound DB schema and CLI. ovn-ic: Interconnection controller with AZ registeration. ovn-northd.c: Refactor allocate_tnlid. ovn-ic: Transit switch controller. ovn-sb: Add columns is_interconn and is_remote to Chassis. ovn-ic: Interconnection gateway controller. ovn-ic: Interconnection port controller. .gitignore | 6 + Makefile.am| 1 + automake.mk| 75 controller/binding.c | 6 +- controller/chassis.c | 14 + debian/ovn-common.install | 2 + debian/ovn-common.manpages | 4 + ic/.gitignore | 2 + ic/automake.mk | 10 + ic/ovn-ic.8.xml| 111 ++ ic/ovn-ic.c| 942 + lib/.gitignore | 6 + lib/automake.mk| 32 +- lib/ovn-inb-idl.ann| 9 + lib/ovn-isb-idl.ann| 9 + lib/ovn-util.c | 92 + lib/ovn-util.h | 15 + northd/ovn-northd.c| 106 ++--- ovn-architecture.7.xml | 107 - ovn-inb.ovsschema | 76 ovn-inb.xml| 377 ++ ovn-isb.ovsschema | 130 +++ ovn-isb.xml| 588 ovn-nb.ovsschema | 5 +- ovn-nb.xml | 18 +- ovn-sb.ovsschema | 8 +- ovn-sb.xml | 15 + tutorial/ovs-sandbox | 2 +- utilities/.gitignore | 4 + utilities/automake.mk | 10 + utilities/ovn-inbctl.c | 879
[ovs-dev] [PATCH] datapath: Fix conntrack cache with timeout
This patch is from the following upstream net-next commit along with
an updated system traffic test to avoid regression.
Upstream commit:
commit 7177895154e6a35179d332f4a584d396c50d0612
Author: Yi-Hung Wei
Date: Thu Aug 22 13:17:50 2019 -0700
openvswitch: Fix conntrack cache with timeout
This patch addresses a conntrack cache issue with timeout policy.
Currently, we do not check if the timeout extension is set properly in
the
cached conntrack entry. Thus, after packet recirculate from conntrack
action, the timeout policy is not applied properly. This patch fixes
the
aforementioned issue.
Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct action")
Reported-by: kbuild test robot
Signed-off-by: Yi-Hung Wei
Acked-by: Pravin B Shelar
Signed-off-by: David S. Miller
Signed-off-by: Yi-Hung Wei
---
datapath/conntrack.c| 13 +
tests/system-traffic.at | 18 ++
2 files changed, 31 insertions(+)
diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 35a183aeb33a..c6d523758ff1 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -88,6 +88,7 @@ struct ovs_conntrack_info {
struct md_mark mark;
struct md_labels labels;
char timeout[CTNL_TIMEOUT_NAME_MAX];
+ struct nf_ct_timeout *nf_ct_timeout;
#ifdef CONFIG_NF_NAT_NEEDED
struct nf_nat_range2 range; /* Only present for SRC NAT and DST NAT. */
#endif
@@ -750,6 +751,14 @@ static bool skb_nfct_cached(struct net *net,
if (help && rcu_access_pointer(help->helper) != info->helper)
return false;
}
+ if (info->nf_ct_timeout) {
+ struct nf_conn_timeout *timeout_ext;
+
+ timeout_ext = nf_ct_timeout_find(ct);
+ if (!timeout_ext || info->nf_ct_timeout !=
+ rcu_dereference(timeout_ext->timeout))
+ return false;
+ }
/* Force conntrack entry direction to the current packet? */
if (info->force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) {
/* Delete the conntrack entry if confirmed, else just release
@@ -1709,6 +1718,10 @@ int ovs_ct_copy_action(struct net *net, const struct
nlattr *attr,
ct_info.timeout))
pr_info_ratelimited("Failed to associated timeout "
"policy `%s'\n", ct_info.timeout);
+ else
+ ct_info.nf_ct_timeout = rcu_dereference(
+ nf_ct_timeout_find(ct_info.ct)->timeout);
+
}
if (helper) {
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index bfc6bb5b47c7..3d4e365764b5 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -3242,6 +3242,24 @@ sleep 4
AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
])
+dnl Re-send ICMP and UDP traffic to test conntrack cache
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING],
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1
packet=5054000a505400090800451c0011a4cd0a0101010a010102000100020008
actions=resubmit(,0)"])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sort], [0],
[dnl
+icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=,type=0,code=0),zone=5
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),zone=5
+])
+
+dnl Wait until the timeout expire.
+dnl We intend to wait a bit longer, because conntrack does not recycle the
entry right after it is expired.
+sleep 4
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
+])
+
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/2] ovsdb-cluster: Use ovs-vsctl instead of ovn-nbctl and ovn-sbctl.
On Fri, Sep 27, 2019 at 12:50 PM Ben Pfaff wrote:
>
> This removes a dependency on OVN from the tests.
>
> This adds some options to ovs-vsctl to allow it to be used for testing
> the clustering feature. The new options are undocumented because
> they're really just useful for testing clustering.
>
> Signed-off-by: Ben Pfaff
> ---
> tests/ovsdb-cluster.at | 37 ++---
> utilities/ovs-vsctl.c | 27 ++-
> 2 files changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at
> index de4295a7e60c..24601c887ff0 100644
> --- a/tests/ovsdb-cluster.at
> +++ b/tests/ovsdb-cluster.at
> @@ -249,10 +249,10 @@ ovsdb_cluster_failure_test () {
> new_leader=$5
> fi
>
> -cp $top_srcdir/ovn/ovn-nb.ovsschema schema
> +cp $top_srcdir/vswitchd/vswitch.ovsschema schema
> schema=`ovsdb-tool schema-name schema`
> AT_CHECK([ovsdb-tool '-vPATTERN:console:%c|%p|%m' create-cluster
s1.db schema unix:s1.raft], [0], [], [dnl
> -ovsdb|WARN|schema: changed 2 columns in 'OVN_Northbound' database from
ephemeral to persistent, including 'status' column in 'Connection' table,
because clusters do not support ephemeral columns
> +ovsdb|WARN|schema: changed 30 columns in 'Open_vSwitch' database from
ephemeral to persistent, including 'status' column in 'Manager' table,
because clusters do not support ephemeral columns
> ])
>
> n=3
> @@ -283,7 +283,7 @@ ovsdb|WARN|schema: changed 2 columns in
'OVN_Northbound' database from ephemeral
> for i in `seq $n`; do start_server $i; done
> for i in `seq $n`; do connect_server $i; done
>
> -export OVN_NB_DB=unix:s$remote_1.ovsdb,unix:s$remote_2.ovsdb
> +db=unix:s$remote_1.ovsdb,unix:s$remote_2.ovsdb
>
> # To ensure $new_leader node the new leader, we delay election timer
for
> # the other follower.
> @@ -296,15 +296,15 @@ ovsdb|WARN|schema: changed 2 columns in
'OVN_Northbound' database from ephemeral
> AT_CHECK([ovs-appctl -t "`pwd`"/s$delay_election_node
cluster/failure-test delay-election], [0], [ignore])
> fi
> AT_CHECK([ovs-appctl -t "`pwd`"/s$crash_node cluster/failure-test
$crash_command], [0], [ignore])
> -AT_CHECK([ovn-nbctl -v --timeout=10 --no-leader-only
--no-shuffle-remotes create logical_switch name=ls1], [0], [ignore],
[ignore])
> +AT_CHECK([ovs-vsctl -v --timeout=10 --db="$db" --no-leader-only
--no-shuffle-remotes --no-wait create QoS type=x], [0], [ignore], [ignore])
>
> # Make sure that the node really crashed.
> AT_CHECK([ls s$crash_node.ovsdb], [2], [ignore], [ignore])
> # XXX: Client will fail if remotes contains unix socket that doesn't
exist (killed).
> -if test "$remote_1" == "$crash_node"; then
> -export OVN_NB_DB=unix:s$remote_2.ovsdb
> +if test "$remote_1" = "$crash_node"; then
> +db=unix:s$remote_2.ovsdb
> fi
> -AT_CHECK([ovn-nbctl --no-leader-only ls-list | awk '{ print $2 }'],
[0], [(ls1)
> +AT_CHECK([ovs-vsctl --db="$db" --no-leader-only --no-wait
--columns=type --bare list QoS], [0], [x
> ])
> }
> OVS_END_SHELL_HELPERS
> @@ -407,10 +407,10 @@ ovsdb_torture_test () {
> local n=$1 # Number of cluster members
> local victim=$2 # Cluster member to kill or remove
> local variant=$3# 'kill' and restart or 'remove' and add
> -cp $top_srcdir/ovn/ovn-sb.ovsschema schema
> +cp $top_srcdir/vswitchd/vswitch.ovsschema schema
> schema=`ovsdb-tool schema-name schema`
> AT_CHECK([ovsdb-tool '-vPATTERN:console:%c|%p|%m' create-cluster
s1.db schema unix:s1.raft], [0], [], [dnl
> -ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' database from
ephemeral to persistent, including 'status' column in 'Connection' table,
because clusters do not support ephemeral columns
> +ovsdb|WARN|schema: changed 30 columns in 'Open_vSwitch' database from
ephemeral to persistent, including 'status' column in 'Manager' table,
because clusters do not support ephemeral columns
> ])
>
> join_cluster() {
> @@ -442,7 +442,7 @@ ovsdb|WARN|schema: changed 2 columns in
'OVN_Southbound' database from ephemeral
> remove_server() {
> local i=$1
> printf "\ns$i: removing from cluster\n"
> -AT_CHECK([ovs-appctl --timeout=30 -t "`pwd`"/s$i cluster/leave
OVN_Southbound])
> +AT_CHECK([ovs-appctl --timeout=30 -t "`pwd`"/s$i cluster/leave
Open_vSwitch])
> printf "\ns$i: waiting for removal to complete\n"
> AT_CHECK([ovsdb_client_wait --log-file=remove$i.log
unix:s$i.ovsdb $schema removed])
> stop_server $i
> @@ -462,22 +462,21 @@ ovsdb|WARN|schema: changed 2 columns in
'OVN_Southbound' database from ephemeral
> for i in `seq $n`; do start_server $i; done
> for i in `seq $n`; do connect_server $i; done
>
> -OVN_SB_DB=unix:s1.ovsdb
> +db=unix:s1.ovsdb
> for i in `seq 2 $n`; do
> -
Re: [ovs-dev] [PATCH v2 ovn 0/3] Introduce localnet egress QoS support
Acked-by: Mark Michelson
On 9/24/19 12:39 PM, Lorenzo Bianconi wrote:
OVN applies logical switch QoS settings to egress interfaces. It
currently works by analyzing each br-int interface to see what the remote-ip
is on it, and then adding qdiscs to the tunnel-egress-iface associated
with this br-int interface.
This doesn't work as well when working with VLAN networks. In VLAN
networks, the interface on br-int associated with the localnet port on
the logical switch will be a patch port. It's connected to another
bridge, making it difficult to determine which interface (or interfaces) is
the egress interface on that connected bridge.
This series aims to mend this. On the bridge that is patched to br-int,
interfaces can have the boolean external-ids:ovn-egress-iface set to
true if this is an egress interface. This way, QoS can be applied to
these interfaces the same as is applied to tunnel interfaces.
Moreover rework qos code in order to add the capability to select automatically
the queue_id used to identify the device queue
Finally add the set_queue action to logical flows in Egress Port Security - L2
stage
for localnet QoS capable ports in order to set the physical_interface qdisc id
Changes since v1:
- split build_lswitch_port_sec in build_lswitch_input_port_sec and
build_lswitch_output_port_sec
- removed unnecessary log messages
Changes since RFC:
- introduce build_lswitch_port_sec as a container for {in/out}_port_sec logical
flow configuration
- move ovn-egress-iface lookup in consider_localnet_port in order to avoid an
unnecessary port_binding lookup
Lorenzo Bianconi (3):
Add egress QoS mapping for non-tunnel interfaces
northd: add the possibility to define localnet as qos capable port
northd: interoduce logical flow for localnet egress shaping
controller/binding.c| 51 ++-
controller/binding.h| 4 +
controller/ovn-controller.c | 3 +-
controller/patch.c | 76 +-
controller/patch.h | 4 +
northd/ovn-northd.8.xml | 7 +-
northd/ovn-northd.c | 276 ++--
7 files changed, 276 insertions(+), 145 deletions(-)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv4] fatal-signal: Catch SIGSEGV and print backtrace.
On Fri, Sep 27, 2019 at 10:22:55AM -0700, William Tu wrote: > The patch catches the SIGSEGV signal and prints the backtrace > using libunwind at the monitor daemon. This makes debugging easier > when there is no debug symbol package or gdb installed on production > systems. Thanks. Let's try it. I applied this to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 3/3] debug: print mbuf extra info.
Flavio Leitner writes: > On Thu, Sep 26, 2019 at 07:49:28AM -0700, Ben Pfaff wrote: >> On Thu, Sep 26, 2019 at 11:59:14AM -0400, Aaron Conole wrote: >> > Flavio Leitner writes: >> > >> > > Signed-off-by: Flavio Leitner >> > > --- >> > > lib/netdev-dpdk.c | 18 ++ >> > > 1 file changed, 18 insertions(+) >> > > >> > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> > > index cfbd9a9e5..7965bf57a 100644 >> > > --- a/lib/netdev-dpdk.c >> > > +++ b/lib/netdev-dpdk.c >> > > @@ -2294,6 +2294,24 @@ netdev_dpdk_vhost_update_rx_counters(struct >> > > netdev_stats *stats, >> > > } >> > > >> > > stats->rx_bytes += packet_size; >> > > +#if 0 >> > >> > I've never liked bare '#if 0' in the code. >> > >> > Can you make this something like: >> > >> > #ifndef NDEBUG >> > >> > This means for all debug builds we would get this tracing >> > infrastructure. Maybe even make a special configuration option? >> >> It's so much better if we can avoid #ifdefs. Can we just use VLOG_DBG() >> or its related VLOG_IS_DBG_ENABLED(), VLOG_DBG_RL(), VLOG_DROP_DBG()? > > My bad, I missed to change the subject prefix to PoC or RFC. > Please do NOT apply the patchset, there are bugs and it is complete. > They are just development patches to show the alternative idea. Dismissed without prejudice :-) > fbl ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv2] netdev-afxdp: Detect numa node id.
Bleep bloop. Greetings William Tu, 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
#35 FILE: lib/netdev-afxdp.c:561:
numa_node_path = xasprintf("/sys/class/net/%s/device/numa_node",
Lines checked: 70, 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 v4] Require Python 3 and remove support for Python 2.
On Sat, Sep 28, 2019, 1:03 AM Ben Pfaff wrote:
> On Fri, Sep 27, 2019 at 12:33:05PM +0530, Numan Siddique wrote:
> > On Thu, Sep 26, 2019 at 11:12 PM Ben Pfaff wrote:
> >
> > > Python 2 reaches end-of-life on January 1, 2020, which is only
> > > a few months away. This means that OVS needs to stop depending
> > > on in the next release that should occur roughly that same time.
> > > Therefore, this commit removes all support for Python 2. It
> > > also makes Python 3 a mandatory build dependency.
> > >
> > > Some of the interesting consequences:
> > >
> > > - HAVE_PYTHON, HAVE_PYTHON2, and HAVE_PYTHON3 conditionals have
> > > been removed, since we now know that Python3 is available.
> > >
> > > - $PYTHON and $PYTHON2 are removed, and $PYTHON3 is always
> > > available.
> > >
> > > - Many tests for Python 2 support have been removed, and the ones
> > > that depended on Python 3 now run unconditionally. This allowed
> > > several macros in the testsuite to be removed, making the code
> > > clearer. This does make some of the changes to the testsuite
> > > files large due to indentation level changes.
> > >
> > > - #! lines for Python now use /usr/bin/python3 instead of
> > > /usr/bin/python.
> > >
> > > - Packaging depends on Python 3 packages.
> > >
> >
> > Hi Ben,
> >
> > The patch failed to apply on latest master, but it applied on top of the
> > commit
> > - 15a5e50a("travis: Drop -MD related workaround for sparse.") so I
> > tested
> > it on top of it.
> >
> > I tested this patch on Fedora 30 and centos 7 container. The compilation
> > and make check is successful.
> >
> > I also tested with "make rpm-fedora".
> >
> > Acked-by: Numan Siddique
> > Tested-by: Numan Siddique
> >
> > Few comments on "make rpm-fedora" on centos 7.
> > The build fails with the error python-six not found. After I passed
> > "RPMBUILD_OPT="--with build_python3" it's successful.
> >
> > May be we can enable python3 in openvswitch-fedora.spec.in by default
> since
> > there will
> > be no python2 support. But I think that can be a separate patch.
> >
> > Without this patch, "make rpm-fedora" on centos7 is successful but
> > python3-openvswitch package
> > is not generated.
>
> Thanks. I applied this to master.
>
> Do you want to write up that additional patch?
>
Sure. I will work on it.
Thanks
Numan
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/2] ovsdb-cluster: Use ovs-vsctl instead of ovn-nbctl and ovn-sbctl.
This removes a dependency on OVN from the tests.
This adds some options to ovs-vsctl to allow it to be used for testing
the clustering feature. The new options are undocumented because
they're really just useful for testing clustering.
Signed-off-by: Ben Pfaff
---
tests/ovsdb-cluster.at | 37 ++---
utilities/ovs-vsctl.c | 27 ++-
2 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at
index de4295a7e60c..24601c887ff0 100644
--- a/tests/ovsdb-cluster.at
+++ b/tests/ovsdb-cluster.at
@@ -249,10 +249,10 @@ ovsdb_cluster_failure_test () {
new_leader=$5
fi
-cp $top_srcdir/ovn/ovn-nb.ovsschema schema
+cp $top_srcdir/vswitchd/vswitch.ovsschema schema
schema=`ovsdb-tool schema-name schema`
AT_CHECK([ovsdb-tool '-vPATTERN:console:%c|%p|%m' create-cluster s1.db
schema unix:s1.raft], [0], [], [dnl
-ovsdb|WARN|schema: changed 2 columns in 'OVN_Northbound' database from
ephemeral to persistent, including 'status' column in 'Connection' table,
because clusters do not support ephemeral columns
+ovsdb|WARN|schema: changed 30 columns in 'Open_vSwitch' database from
ephemeral to persistent, including 'status' column in 'Manager' table, because
clusters do not support ephemeral columns
])
n=3
@@ -283,7 +283,7 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Northbound'
database from ephemeral
for i in `seq $n`; do start_server $i; done
for i in `seq $n`; do connect_server $i; done
-export OVN_NB_DB=unix:s$remote_1.ovsdb,unix:s$remote_2.ovsdb
+db=unix:s$remote_1.ovsdb,unix:s$remote_2.ovsdb
# To ensure $new_leader node the new leader, we delay election timer for
# the other follower.
@@ -296,15 +296,15 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Northbound'
database from ephemeral
AT_CHECK([ovs-appctl -t "`pwd`"/s$delay_election_node
cluster/failure-test delay-election], [0], [ignore])
fi
AT_CHECK([ovs-appctl -t "`pwd`"/s$crash_node cluster/failure-test
$crash_command], [0], [ignore])
-AT_CHECK([ovn-nbctl -v --timeout=10 --no-leader-only --no-shuffle-remotes
create logical_switch name=ls1], [0], [ignore], [ignore])
+AT_CHECK([ovs-vsctl -v --timeout=10 --db="$db" --no-leader-only
--no-shuffle-remotes --no-wait create QoS type=x], [0], [ignore], [ignore])
# Make sure that the node really crashed.
AT_CHECK([ls s$crash_node.ovsdb], [2], [ignore], [ignore])
# XXX: Client will fail if remotes contains unix socket that doesn't exist
(killed).
-if test "$remote_1" == "$crash_node"; then
-export OVN_NB_DB=unix:s$remote_2.ovsdb
+if test "$remote_1" = "$crash_node"; then
+db=unix:s$remote_2.ovsdb
fi
-AT_CHECK([ovn-nbctl --no-leader-only ls-list | awk '{ print $2 }'], [0],
[(ls1)
+AT_CHECK([ovs-vsctl --db="$db" --no-leader-only --no-wait --columns=type
--bare list QoS], [0], [x
])
}
OVS_END_SHELL_HELPERS
@@ -407,10 +407,10 @@ ovsdb_torture_test () {
local n=$1 # Number of cluster members
local victim=$2 # Cluster member to kill or remove
local variant=$3# 'kill' and restart or 'remove' and add
-cp $top_srcdir/ovn/ovn-sb.ovsschema schema
+cp $top_srcdir/vswitchd/vswitch.ovsschema schema
schema=`ovsdb-tool schema-name schema`
AT_CHECK([ovsdb-tool '-vPATTERN:console:%c|%p|%m' create-cluster s1.db
schema unix:s1.raft], [0], [], [dnl
-ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' database from
ephemeral to persistent, including 'status' column in 'Connection' table,
because clusters do not support ephemeral columns
+ovsdb|WARN|schema: changed 30 columns in 'Open_vSwitch' database from
ephemeral to persistent, including 'status' column in 'Manager' table, because
clusters do not support ephemeral columns
])
join_cluster() {
@@ -442,7 +442,7 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound'
database from ephemeral
remove_server() {
local i=$1
printf "\ns$i: removing from cluster\n"
-AT_CHECK([ovs-appctl --timeout=30 -t "`pwd`"/s$i cluster/leave
OVN_Southbound])
+AT_CHECK([ovs-appctl --timeout=30 -t "`pwd`"/s$i cluster/leave
Open_vSwitch])
printf "\ns$i: waiting for removal to complete\n"
AT_CHECK([ovsdb_client_wait --log-file=remove$i.log unix:s$i.ovsdb
$schema removed])
stop_server $i
@@ -462,22 +462,21 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound'
database from ephemeral
for i in `seq $n`; do start_server $i; done
for i in `seq $n`; do connect_server $i; done
-OVN_SB_DB=unix:s1.ovsdb
+db=unix:s1.ovsdb
for i in `seq 2 $n`; do
-OVN_SB_DB=$OVN_SB_DB,unix:s$i.ovsdb
+db=$db,unix:s$i.ovsdb
done
-export OVN_SB_DB
n1=10 n2=5 n3=50
-echo "starting $n1*$n2 ovn-sbctl processes..."
+echo "starting $n1*$n2
Re: [ovs-dev] [PATCH v2] odp-util: calc checksum of ip hdr for tunnel encap
Hi Martin, please fix the problems reported by the robot.
Thanks,
Ben.
On Thu, Sep 26, 2019 at 09:58:22PM -0400, 0-day Robot wrote:
> Bleep bloop. Greetings Martin Zhang, 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 non-spaces leading whitespace
> #40 FILE: lib/odp-util.c:1527:
> memset(ip, 0, sizeof(*ip));
>
> WARNING: Line has non-spaces leading whitespace
> #48 FILE: lib/odp-util.c:1541:
> ip->ip_csum = csum(ip, ip_len);
>
> Lines checked: 78, Warnings: 2, Errors: 0
>
>
> build:
> mv tests/ovsdb-cluster-testsuite.tmp tests/ovsdb-cluster-testsuite
> \
> { sed -n -e '/%AUTHORS%/q' -e p < ./debian/copyright.in; \
> sed '34,/^$/d' ./AUTHORS.rst | \
> sed -n -e '/^$/q' -e 's/^/ /p'; \
> sed -e '34,/%AUTHORS%/d' ./debian/copyright.in;\
> } > debian/copyright
> (printf '\043 Generated automatically -- do not modify!-*-
> buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.12.90,g') <
> ./rhel/openvswitch-dkms.spec.in > openvswitch-dkms.spec.tmp || exit 1; if cmp
> -s openvswitch-dkms.spec.tmp rhel/openvswitch-dkms.spec; then touch
> rhel/openvswitch-dkms.spec; rm openvswitch-dkms.spec.tmp; else mv
> openvswitch-dkms.spec.tmp rhel/openvswitch-dkms.spec; fi
> (printf '\043 Generated automatically -- do not modify!-*-
> buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.12.90,g') <
> ./rhel/kmod-openvswitch-rhel6.spec.in > kmod-openvswitch-rhel6.spec.tmp ||
> exit 1; if cmp -s kmod-openvswitch-rhel6.spec.tmp
> rhel/kmod-openvswitch-rhel6.spec; then touch
> rhel/kmod-openvswitch-rhel6.spec; rm kmod-openvswitch-rhel6.spec.tmp; else mv
> kmod-openvswitch-rhel6.spec.tmp rhel/kmod-openvswitch-rhel6.spec; fi
> (printf '\043 Generated automatically -- do not modify!-*-
> buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.12.90,g') <
> ./rhel/openvswitch-kmod-fedora.spec.in > openvswitch-kmod-fedora.spec.tmp ||
> exit 1; if cmp -s openvswitch-kmod-fedora.spec.tmp
> rhel/openvswitch-kmod-fedora.spec; then touch
> rhel/openvswitch-kmod-fedora.spec; rm openvswitch-kmod-fedora.spec.tmp; else
> mv openvswitch-kmod-fedora.spec.tmp rhel/openvswitch-kmod-fedora.spec; fi
> (printf '\043 Generated automatically -- do not modify!-*-
> buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.12.90,g') <
> ./rhel/openvswitch.spec.in > openvswitch.spec.tmp || exit 1; if cmp -s
> openvswitch.spec.tmp rhel/openvswitch.spec; then touch rhel/openvswitch.spec;
> rm openvswitch.spec.tmp; else mv openvswitch.spec.tmp rhel/openvswitch.spec;
> fi
> (printf '\043 Generated automatically -- do not modify!-*-
> buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.12.90,g') <
> ./rhel/openvswitch-fedora.spec.in > openvswitch-fedora.spec.tmp || exit 1; if
> cmp -s openvswitch-fedora.spec.tmp rhel/openvswitch-fedora.spec; then touch
> rhel/openvswitch-fedora.spec; rm openvswitch-fedora.spec.tmp; else mv
> openvswitch-fedora.spec.tmp rhel/openvswitch-fedora.spec; fi
> (printf '\043 Generated automatically -- do not modify!-*-
> buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.12.90,g') \
> < ./xenserver/openvswitch-xen.spec.in > openvswitch-xen.spec.tmp ||
> exit 1; \
> if cmp -s openvswitch-xen.spec.tmp xenserver/openvswitch-xen.spec; then touch
> xenserver/openvswitch-xen.spec; rm openvswitch-xen.spec.tmp; else mv
> openvswitch-xen.spec.tmp xenserver/openvswitch-xen.spec; fi
> make[3]: Entering directory
> `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/datapath'
> make[3]: Leaving directory
> `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/datapath'
> lib/odp-util.c
> See above for files that use tabs for indentation.
> Please use spaces instead.
> make[2]: *** [check-tabs] Error 1
> make[2]: Leaving directory
> `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory
> `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
> make: *** [all] Error 2
>
>
> 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
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv2] netdev-afxdp: Detect numa node id.
The patch detects the numa node id from the name of the netdev,
by reading the '/sys/class/net//device/numa_node'.
If not available, ex: virtual device, or any error happens,
return numa id 0.
Signed-off-by: William Tu
---
v2:
Address feedback from Eelco
fix memory leak of xaspintf
log using INFO instead of WARN
---
lib/netdev-afxdp.c | 41 -
1 file changed, 36 insertions(+), 5 deletions(-)
diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index 6e01803272aa..6ff1473461a6 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -552,11 +552,42 @@ out:
int
netdev_afxdp_get_numa_id(const struct netdev *netdev)
{
-/* FIXME: Get netdev's PCIe device ID, then find
- * its NUMA node id.
- */
-VLOG_INFO("FIXME: Device %s always use numa id 0.",
- netdev_get_name(netdev));
+const char *numa_node_path;
+long int node_id;
+char buffer[4];
+FILE *stream;
+int n;
+
+numa_node_path = xasprintf("/sys/class/net/%s/device/numa_node",
+ netdev_get_name(netdev));
+stream = fopen(numa_node_path, "r");
+if (!stream) {
+/* Virtual device does not have this info. */
+VLOG_INFO_RL(, "Open %s failed: %s, use numa_id 0",
+ numa_node_path, ovs_strerror(errno));
+free(numa_node_path);
+return 0;
+}
+
+n = fread(buffer, 1, sizeof buffer, stream);
+if (!n) {
+goto error;
+}
+
+node_id = strtol(buffer, NULL, 10);
+if (node_id < 0 || node_id > 2) {
+goto error;
+}
+
+fclose(stream);
+free(numa_node_path);
+return (int)node_id;
+
+error:
+VLOG_WARN_RL(, "Error detecting numa node of %s, use numa_id 0",
+ numa_node_path);
+free(numa_node_path);
+fclose(stream);
return 0;
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv4] fatal-signal: Catch SIGSEGV and print backtrace.
The patch catches the SIGSEGV signal and prints the backtrace
using libunwind at the monitor daemon. This makes debugging easier
when there is no debug symbol package or gdb installed on production
systems.
The patch works when the ovs-vswitchd compiles even without debug symbol
(no -g option), because the object files still have function symbols.
For example:
|daemon_unix(monitor)|WARN|SIGSEGV detected, backtrace:
|daemon_unix(monitor)|WARN|0x00482752
|daemon_unix(monitor)|WARN|0x7fb4900734b0
|daemon_unix(monitor)|WARN|0x7fb49013974d <__poll+0x2d>
|daemon_unix(monitor)|WARN|0x0052b348
|daemon_unix(monitor)|WARN|0x005153ec
|daemon_unix(monitor)|WARN|0x0058630a
|daemon_unix(monitor)|WARN|0x004ffd1d
|daemon_unix(monitor)|WARN|0x7fb490b3b6ba
|daemon_unix(monitor)|WARN|0x7fb49014541d
|daemon_unix(monitor)|ERR|1 crashes: pid 122849 died, killed \
(Segmentation fault), core dumped, restarting
However, if the object files' symbols are stripped, then we can only
get init function plus offset value. This is still useful when trying
to see if two bugs have the same root cause, Example:
|daemon_unix(monitor)|WARN|SIGSEGV detected, backtrace:
|daemon_unix(monitor)|WARN|0x00482752 <_init+0x7d68a>
|daemon_unix(monitor)|WARN|0x7f5f7c8cf4b0
|daemon_unix(monitor)|WARN|0x7f5f7c99574d <__poll+0x2d>
|daemon_unix(monitor)|WARN|0x0052b348 <_init+0x126280>
|daemon_unix(monitor)|WARN|0x005153ec <_init+0x110324>
|daemon_unix(monitor)|WARN|0x00407439 <_init+0x2371>
|daemon_unix(monitor)|WARN|0x7f5f7c8ba830 <__libc_start_main+0xf0>
|daemon_unix(monitor)|WARN|0x00408329 <_init+0x3261>
|daemon_unix(monitor)|ERR|1 crashes: pid 106155 died, killed \
(Segmentation fault), core dumped, restarting
Most C library functions are not async-signal-safe, meaning that
it is not safe to call them from a signal handler, for example
printf() or fflush(). To be async-signal-safe, the handler only
collects the stack info using libunwind, which is signal-safe, and
issues 'write' to the pipe, where the monitor thread reads and
prints to ovs-vswitchd.log.
Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/590503433
Signed-off-by: William Tu
--
v4:
add missing include in lib/backtrace.c
v3:
Some refactoring
fix sparse error
v2:
Address comments from Ben about async-signal-safety
---
.travis.yml | 1 +
configure.ac | 1 +
lib/backtrace.c | 39 +++
lib/backtrace.h | 19 +++
lib/daemon-private.h | 1 +
lib/daemon-unix.c| 11 +--
lib/fatal-signal.c | 51 ++-
m4/openvswitch.m4| 10 ++
8 files changed, 126 insertions(+), 7 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index 68026312ba84..b547eb041791 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -25,6 +25,7 @@ addons:
- selinux-policy-dev
- libunbound-dev
- libunbound-dev:i386
+ - libunwind-dev
before_install: ./.travis/${TRAVIS_OS_NAME}-prepare.sh
diff --git a/configure.ac b/configure.ac
index 1d45c4fdd153..15922418062b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -139,6 +139,7 @@ OVS_LIBTOOL_VERSIONS
OVS_CHECK_CXX
AX_FUNC_POSIX_MEMALIGN
OVS_CHECK_UNBOUND
+OVS_CHECK_UNWIND
OVS_CHECK_INCLUDE_NEXT([stdio.h string.h])
AC_CONFIG_FILES([
diff --git a/lib/backtrace.c b/lib/backtrace.c
index 5cb2954f816b..9347634487c8 100644
--- a/lib/backtrace.c
+++ b/lib/backtrace.c
@@ -15,10 +15,15 @@
*/
#include
+#include
+#include
#include
+#include
+#include
#include "backtrace.h"
#include "openvswitch/vlog.h"
+#include "util.h"
VLOG_DEFINE_THIS_MODULE(backtrace);
@@ -76,3 +81,37 @@ log_backtrace_at(const char *msg, const char *where)
ds_destroy();
}
+
+#ifdef HAVE_UNWIND
+void
+log_received_backtrace(int fd) {
+int byte_read;
+struct unw_backtrace backtrace[UNW_MAX_DEPTH];
+
+VLOG_WARN("%s fd %d", __func__, fd);
+fcntl(fd, F_SETFL, O_NONBLOCK);
+memset(backtrace, 0, UNW_MAX_BUF);
+
+byte_read = read(fd, backtrace, UNW_MAX_BUF);
+if (byte_read < 0) {
+VLOG_ERR("Read fd %d failed: %s", fd,
+ ovs_strerror(errno));
+} else if (byte_read > 0) {
+VLOG_WARN("SIGSEGV detected, backtrace:");
+for (int i = 0; i < UNW_MAX_DEPTH; i++) {
+if (backtrace[i].func[0] == 0) {
+break;
+}
+VLOG_WARN("0x%016lx <%s+0x%lx>\n",
+ backtrace[i].ip,
+ backtrace[i].func,
+ backtrace[i].offset);
+}
+}
+}
+#else /* !HAVE_UNWIND */
+void
+log_received_backtrace(int daemonize_fd OVS_UNUSED) {
+VLOG_WARN("Backtrace using libunwind not supported.");
+}
+#endif /* HAVE_UNWIND */
diff --git a/lib/backtrace.h b/lib/backtrace.h
index 384f2700d94c..5708bf9c6831
Re: [ovs-dev] [PATCH ovn] Disable conjunction by force cross product for all the fields.
On Fri, Sep 27, 2019 at 10:15 PM Han Zhou wrote:
>
>
> On Fri, Sep 27, 2019 at 9:42 AM Numan Siddique
> wrote:
>
>>
>>
>> On Fri, Sep 27, 2019 at 10:05 PM Han Zhou wrote:
>>
>>> Thanks Numan. This is cleaner than the RFC. At the same time, shall we
>>> revert the change of commit: 298701dbc996, which is covered by this change?
>>> (It is better to maintain one temporary workaround than two)
>>>
>>> Acked-by: Han Zhou
>>>
>>
>>
>> Thanks. I will submit a patch to revert it. I will go ahead and apply
>> this patch
>> Or You want to revert to be incorporated with this patch ?
>>
>> Separate one seems better to me.
>>
>> I agree with separate patch.
>
Thanks. I applied this patch to master.
I will submit the revert patch soon.
Thanks
Numan
>
>>
>>>
>>> On Fri, Sep 27, 2019 at 8:52 AM wrote:
>>>
From: Numan Siddique
With this ovn-controller will not generate conjunction flows.
There are issues with the conjunction flows generated by ovn-controller.
Please see the commit 298701dbc996 for more information.
Signed-off-by: Numan Siddique
---
TODO.rst | 10 +
lib/expr.c | 20 +-
tests/ovn.at | 1244 +++---
3 files changed, 1204 insertions(+), 70 deletions(-)
diff --git a/TODO.rst b/TODO.rst
index 943d9bf81..ed55ea236 100644
--- a/TODO.rst
+++ b/TODO.rst
@@ -145,3 +145,13 @@ OVN To-do List
* Support FTP ALGs.
* Support reject action.
+
+* Conjunction: Conjunction is disabled in OVN. This needs to be
revisisted
+ to enable conjunction again after addressing the issues related to
it.
+ Like, if there are multiple ACLs with overlapping Conjunction
matches,
+ conjunction flows are not added properly.
+ Eg. match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
+ tcp.dst >= 800 && tcp.dst <= 900) actions=drop
+
+ match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
+ tcp.dst >= 1000 && tcp.dst <= 2000) actions=allow
diff --git a/lib/expr.c b/lib/expr.c
index c0871e1e8..64ea0aafa 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -32,6 +32,24 @@
VLOG_DEFINE_THIS_MODULE(expr);
+/* Right now conjunction flows generated by ovn-controller
+ * has issues. If there are multiple flows with the same
+ * match for different conjunctions, ovn-controller doesn't
+ * handle it properly.
+ * Eg.
+ * match 1 - ip4.src == {IP1, IP2} && tcp.dst >=500 && tcp.src <=600
+ * action - drop
+ *
+ * match 2 - ip4.src == {IP1, IP2} && tcp.dst >=700 && tcp.src <=800
+ * action - allow.
+ *
+ * To handle this issue temporarily force crossproduct so that
conjunction
+ * flows are not generated.
+ *
+ * Remove this once fixed.
+ * */
+static bool force_crossproduct = true;
+
static struct expr *parse_and_annotate(const char *s,
const struct shash *symtab,
struct ovs_list *nesting,
@@ -2633,7 +2651,7 @@ expr_normalize_and(struct expr *expr)
ovs_assert(sub->type == EXPR_T_OR);
const struct expr_symbol *symbol = expr_get_unique_symbol(sub);
-if (!symbol || symbol->must_crossproduct) {
+if (!symbol || force_crossproduct || symbol->must_crossproduct
) {
struct expr *or = expr_create_andor(EXPR_T_OR);
struct expr *k;
diff --git a/tests/ovn.at b/tests/ovn.at
index c32a75c26..16587114e 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -596,16 +596,14 @@ tcp,reg14=0x6,tp_dst=500
tcp,reg14=0x6,tp_dst=501
])
AT_CHECK([expr_to_flow 'outport == {"eth0", "eth1", "eth2"} && ip4 &&
tcp && tcp.src == {400, 401} && tcp.dst == {500, 501}'], [0], [dnl
-conj_id=1,tcp,reg15=0x5
-conj_id=2,tcp,reg15=0x6
-tcp,reg15=0x5,tp_dst=500: conjunction(1, 0/2)
-tcp,reg15=0x5,tp_dst=501: conjunction(1, 0/2)
-tcp,reg15=0x5,tp_src=400: conjunction(1, 1/2)
-tcp,reg15=0x5,tp_src=401: conjunction(1, 1/2)
-tcp,reg15=0x6,tp_dst=500: conjunction(2, 0/2)
-tcp,reg15=0x6,tp_dst=501: conjunction(2, 0/2)
-tcp,reg15=0x6,tp_src=400: conjunction(2, 1/2)
-tcp,reg15=0x6,tp_src=401: conjunction(2, 1/2)
+tcp,reg15=0x5,tp_src=400,tp_dst=500
+tcp,reg15=0x5,tp_src=400,tp_dst=501
+tcp,reg15=0x5,tp_src=401,tp_dst=500
+tcp,reg15=0x5,tp_src=401,tp_dst=501
+tcp,reg15=0x6,tp_src=400,tp_dst=500
+tcp,reg15=0x6,tp_src=400,tp_dst=501
+tcp,reg15=0x6,tp_src=401,tp_dst=500
+tcp,reg15=0x6,tp_src=401,tp_dst=501
])
AT_CHECK([expr_to_flow 'inport == "eth0" && inport == "eth1"'], [0],
[dnl
(no flows)
@@ -727,22 +725,27 @@ reg15=0x11
])
Re: [ovs-dev] [PATCH ovn] Disable conjunction by force cross product for all the fields.
On Fri, Sep 27, 2019 at 9:42 AM Numan Siddique wrote:
>
>
> On Fri, Sep 27, 2019 at 10:05 PM Han Zhou wrote:
>
>> Thanks Numan. This is cleaner than the RFC. At the same time, shall we
>> revert the change of commit: 298701dbc996, which is covered by this change?
>> (It is better to maintain one temporary workaround than two)
>>
>> Acked-by: Han Zhou
>>
>
>
> Thanks. I will submit a patch to revert it. I will go ahead and apply this
> patch
> Or You want to revert to be incorporated with this patch ?
>
> Separate one seems better to me.
>
> I agree with separate patch.
>
>
>>
>> On Fri, Sep 27, 2019 at 8:52 AM wrote:
>>
>>> From: Numan Siddique
>>>
>>> With this ovn-controller will not generate conjunction flows.
>>> There are issues with the conjunction flows generated by ovn-controller.
>>> Please see the commit 298701dbc996 for more information.
>>>
>>> Signed-off-by: Numan Siddique
>>> ---
>>> TODO.rst | 10 +
>>> lib/expr.c | 20 +-
>>> tests/ovn.at | 1244 +++---
>>> 3 files changed, 1204 insertions(+), 70 deletions(-)
>>>
>>> diff --git a/TODO.rst b/TODO.rst
>>> index 943d9bf81..ed55ea236 100644
>>> --- a/TODO.rst
>>> +++ b/TODO.rst
>>> @@ -145,3 +145,13 @@ OVN To-do List
>>>* Support FTP ALGs.
>>>
>>>* Support reject action.
>>> +
>>> +* Conjunction: Conjunction is disabled in OVN. This needs to be
>>> revisisted
>>> + to enable conjunction again after addressing the issues related to it.
>>> + Like, if there are multiple ACLs with overlapping Conjunction matches,
>>> + conjunction flows are not added properly.
>>> + Eg. match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
>>> + tcp.dst >= 800 && tcp.dst <= 900) actions=drop
>>> +
>>> + match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
>>> + tcp.dst >= 1000 && tcp.dst <= 2000) actions=allow
>>> diff --git a/lib/expr.c b/lib/expr.c
>>> index c0871e1e8..64ea0aafa 100644
>>> --- a/lib/expr.c
>>> +++ b/lib/expr.c
>>> @@ -32,6 +32,24 @@
>>>
>>> VLOG_DEFINE_THIS_MODULE(expr);
>>>
>>> +/* Right now conjunction flows generated by ovn-controller
>>> + * has issues. If there are multiple flows with the same
>>> + * match for different conjunctions, ovn-controller doesn't
>>> + * handle it properly.
>>> + * Eg.
>>> + * match 1 - ip4.src == {IP1, IP2} && tcp.dst >=500 && tcp.src <=600
>>> + * action - drop
>>> + *
>>> + * match 2 - ip4.src == {IP1, IP2} && tcp.dst >=700 && tcp.src <=800
>>> + * action - allow.
>>> + *
>>> + * To handle this issue temporarily force crossproduct so that
>>> conjunction
>>> + * flows are not generated.
>>> + *
>>> + * Remove this once fixed.
>>> + * */
>>> +static bool force_crossproduct = true;
>>> +
>>> static struct expr *parse_and_annotate(const char *s,
>>> const struct shash *symtab,
>>> struct ovs_list *nesting,
>>> @@ -2633,7 +2651,7 @@ expr_normalize_and(struct expr *expr)
>>>
>>> ovs_assert(sub->type == EXPR_T_OR);
>>> const struct expr_symbol *symbol = expr_get_unique_symbol(sub);
>>> -if (!symbol || symbol->must_crossproduct) {
>>> +if (!symbol || force_crossproduct || symbol->must_crossproduct
>>> ) {
>>> struct expr *or = expr_create_andor(EXPR_T_OR);
>>> struct expr *k;
>>>
>>> diff --git a/tests/ovn.at b/tests/ovn.at
>>> index c32a75c26..16587114e 100644
>>> --- a/tests/ovn.at
>>> +++ b/tests/ovn.at
>>> @@ -596,16 +596,14 @@ tcp,reg14=0x6,tp_dst=500
>>> tcp,reg14=0x6,tp_dst=501
>>> ])
>>> AT_CHECK([expr_to_flow 'outport == {"eth0", "eth1", "eth2"} && ip4 &&
>>> tcp && tcp.src == {400, 401} && tcp.dst == {500, 501}'], [0], [dnl
>>> -conj_id=1,tcp,reg15=0x5
>>> -conj_id=2,tcp,reg15=0x6
>>> -tcp,reg15=0x5,tp_dst=500: conjunction(1, 0/2)
>>> -tcp,reg15=0x5,tp_dst=501: conjunction(1, 0/2)
>>> -tcp,reg15=0x5,tp_src=400: conjunction(1, 1/2)
>>> -tcp,reg15=0x5,tp_src=401: conjunction(1, 1/2)
>>> -tcp,reg15=0x6,tp_dst=500: conjunction(2, 0/2)
>>> -tcp,reg15=0x6,tp_dst=501: conjunction(2, 0/2)
>>> -tcp,reg15=0x6,tp_src=400: conjunction(2, 1/2)
>>> -tcp,reg15=0x6,tp_src=401: conjunction(2, 1/2)
>>> +tcp,reg15=0x5,tp_src=400,tp_dst=500
>>> +tcp,reg15=0x5,tp_src=400,tp_dst=501
>>> +tcp,reg15=0x5,tp_src=401,tp_dst=500
>>> +tcp,reg15=0x5,tp_src=401,tp_dst=501
>>> +tcp,reg15=0x6,tp_src=400,tp_dst=500
>>> +tcp,reg15=0x6,tp_src=400,tp_dst=501
>>> +tcp,reg15=0x6,tp_src=401,tp_dst=500
>>> +tcp,reg15=0x6,tp_src=401,tp_dst=501
>>> ])
>>> AT_CHECK([expr_to_flow 'inport == "eth0" && inport == "eth1"'], [0],
>>> [dnl
>>> (no flows)
>>> @@ -727,22 +725,27 @@ reg15=0x11
>>> ])
>>> AT_CLEANUP
>>>
>>> -AT_SETUP([ovn -- converting expressions to flows -- conjunction])
>>> -AT_KEYWORDS([conjunction])
>>> +AT_SETUP([ovn -- converting expressions to flows -- no conjunction])
>>> +AT_KEYWORDS([no conjunction])
>>> expr_to_flow () {
>>> echo "$1" | ovstest test-ovn
Re: [ovs-dev] [PATCH ovn] Disable conjunction by force cross product for all the fields.
On Fri, Sep 27, 2019 at 10:05 PM Han Zhou wrote:
> Thanks Numan. This is cleaner than the RFC. At the same time, shall we
> revert the change of commit: 298701dbc996, which is covered by this change?
> (It is better to maintain one temporary workaround than two)
>
> Acked-by: Han Zhou
>
Thanks. I will submit a patch to revert it. I will go ahead and apply this
patch
Or You want to revert to be incorporated with this patch ?
Separate one seems better to me.
Thanks
Numan
>
> On Fri, Sep 27, 2019 at 8:52 AM wrote:
>
>> From: Numan Siddique
>>
>> With this ovn-controller will not generate conjunction flows.
>> There are issues with the conjunction flows generated by ovn-controller.
>> Please see the commit 298701dbc996 for more information.
>>
>> Signed-off-by: Numan Siddique
>> ---
>> TODO.rst | 10 +
>> lib/expr.c | 20 +-
>> tests/ovn.at | 1244 +++---
>> 3 files changed, 1204 insertions(+), 70 deletions(-)
>>
>> diff --git a/TODO.rst b/TODO.rst
>> index 943d9bf81..ed55ea236 100644
>> --- a/TODO.rst
>> +++ b/TODO.rst
>> @@ -145,3 +145,13 @@ OVN To-do List
>>* Support FTP ALGs.
>>
>>* Support reject action.
>> +
>> +* Conjunction: Conjunction is disabled in OVN. This needs to be
>> revisisted
>> + to enable conjunction again after addressing the issues related to it.
>> + Like, if there are multiple ACLs with overlapping Conjunction matches,
>> + conjunction flows are not added properly.
>> + Eg. match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
>> + tcp.dst >= 800 && tcp.dst <= 900) actions=drop
>> +
>> + match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
>> + tcp.dst >= 1000 && tcp.dst <= 2000) actions=allow
>> diff --git a/lib/expr.c b/lib/expr.c
>> index c0871e1e8..64ea0aafa 100644
>> --- a/lib/expr.c
>> +++ b/lib/expr.c
>> @@ -32,6 +32,24 @@
>>
>> VLOG_DEFINE_THIS_MODULE(expr);
>>
>> +/* Right now conjunction flows generated by ovn-controller
>> + * has issues. If there are multiple flows with the same
>> + * match for different conjunctions, ovn-controller doesn't
>> + * handle it properly.
>> + * Eg.
>> + * match 1 - ip4.src == {IP1, IP2} && tcp.dst >=500 && tcp.src <=600
>> + * action - drop
>> + *
>> + * match 2 - ip4.src == {IP1, IP2} && tcp.dst >=700 && tcp.src <=800
>> + * action - allow.
>> + *
>> + * To handle this issue temporarily force crossproduct so that
>> conjunction
>> + * flows are not generated.
>> + *
>> + * Remove this once fixed.
>> + * */
>> +static bool force_crossproduct = true;
>> +
>> static struct expr *parse_and_annotate(const char *s,
>> const struct shash *symtab,
>> struct ovs_list *nesting,
>> @@ -2633,7 +2651,7 @@ expr_normalize_and(struct expr *expr)
>>
>> ovs_assert(sub->type == EXPR_T_OR);
>> const struct expr_symbol *symbol = expr_get_unique_symbol(sub);
>> -if (!symbol || symbol->must_crossproduct) {
>> +if (!symbol || force_crossproduct || symbol->must_crossproduct )
>> {
>> struct expr *or = expr_create_andor(EXPR_T_OR);
>> struct expr *k;
>>
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index c32a75c26..16587114e 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -596,16 +596,14 @@ tcp,reg14=0x6,tp_dst=500
>> tcp,reg14=0x6,tp_dst=501
>> ])
>> AT_CHECK([expr_to_flow 'outport == {"eth0", "eth1", "eth2"} && ip4 &&
>> tcp && tcp.src == {400, 401} && tcp.dst == {500, 501}'], [0], [dnl
>> -conj_id=1,tcp,reg15=0x5
>> -conj_id=2,tcp,reg15=0x6
>> -tcp,reg15=0x5,tp_dst=500: conjunction(1, 0/2)
>> -tcp,reg15=0x5,tp_dst=501: conjunction(1, 0/2)
>> -tcp,reg15=0x5,tp_src=400: conjunction(1, 1/2)
>> -tcp,reg15=0x5,tp_src=401: conjunction(1, 1/2)
>> -tcp,reg15=0x6,tp_dst=500: conjunction(2, 0/2)
>> -tcp,reg15=0x6,tp_dst=501: conjunction(2, 0/2)
>> -tcp,reg15=0x6,tp_src=400: conjunction(2, 1/2)
>> -tcp,reg15=0x6,tp_src=401: conjunction(2, 1/2)
>> +tcp,reg15=0x5,tp_src=400,tp_dst=500
>> +tcp,reg15=0x5,tp_src=400,tp_dst=501
>> +tcp,reg15=0x5,tp_src=401,tp_dst=500
>> +tcp,reg15=0x5,tp_src=401,tp_dst=501
>> +tcp,reg15=0x6,tp_src=400,tp_dst=500
>> +tcp,reg15=0x6,tp_src=400,tp_dst=501
>> +tcp,reg15=0x6,tp_src=401,tp_dst=500
>> +tcp,reg15=0x6,tp_src=401,tp_dst=501
>> ])
>> AT_CHECK([expr_to_flow 'inport == "eth0" && inport == "eth1"'], [0], [dnl
>> (no flows)
>> @@ -727,22 +725,27 @@ reg15=0x11
>> ])
>> AT_CLEANUP
>>
>> -AT_SETUP([ovn -- converting expressions to flows -- conjunction])
>> -AT_KEYWORDS([conjunction])
>> +AT_SETUP([ovn -- converting expressions to flows -- no conjunction])
>> +AT_KEYWORDS([no conjunction])
>> expr_to_flow () {
>> echo "$1" | ovstest test-ovn expr-to-flows | sort
>> }
>>
>> +# conjunction is disabled in OVN until some of the issues
>> +# related to conjunction flows are fixed.
>> +# expr-to-flows should not generate any conjunction flows.
>> lflow="ip4.src ==
Re: [ovs-dev] [PATCH ovn] Disable conjunction by force cross product for all the fields.
Thanks Numan. This is cleaner than the RFC. At the same time, shall we
revert the change of commit: 298701dbc996, which is covered by this change?
(It is better to maintain one temporary workaround than two)
Acked-by: Han Zhou
On Fri, Sep 27, 2019 at 8:52 AM wrote:
> From: Numan Siddique
>
> With this ovn-controller will not generate conjunction flows.
> There are issues with the conjunction flows generated by ovn-controller.
> Please see the commit 298701dbc996 for more information.
>
> Signed-off-by: Numan Siddique
> ---
> TODO.rst | 10 +
> lib/expr.c | 20 +-
> tests/ovn.at | 1244 +++---
> 3 files changed, 1204 insertions(+), 70 deletions(-)
>
> diff --git a/TODO.rst b/TODO.rst
> index 943d9bf81..ed55ea236 100644
> --- a/TODO.rst
> +++ b/TODO.rst
> @@ -145,3 +145,13 @@ OVN To-do List
>* Support FTP ALGs.
>
>* Support reject action.
> +
> +* Conjunction: Conjunction is disabled in OVN. This needs to be revisisted
> + to enable conjunction again after addressing the issues related to it.
> + Like, if there are multiple ACLs with overlapping Conjunction matches,
> + conjunction flows are not added properly.
> + Eg. match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
> + tcp.dst >= 800 && tcp.dst <= 900) actions=drop
> +
> + match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
> + tcp.dst >= 1000 && tcp.dst <= 2000) actions=allow
> diff --git a/lib/expr.c b/lib/expr.c
> index c0871e1e8..64ea0aafa 100644
> --- a/lib/expr.c
> +++ b/lib/expr.c
> @@ -32,6 +32,24 @@
>
> VLOG_DEFINE_THIS_MODULE(expr);
>
> +/* Right now conjunction flows generated by ovn-controller
> + * has issues. If there are multiple flows with the same
> + * match for different conjunctions, ovn-controller doesn't
> + * handle it properly.
> + * Eg.
> + * match 1 - ip4.src == {IP1, IP2} && tcp.dst >=500 && tcp.src <=600
> + * action - drop
> + *
> + * match 2 - ip4.src == {IP1, IP2} && tcp.dst >=700 && tcp.src <=800
> + * action - allow.
> + *
> + * To handle this issue temporarily force crossproduct so that conjunction
> + * flows are not generated.
> + *
> + * Remove this once fixed.
> + * */
> +static bool force_crossproduct = true;
> +
> static struct expr *parse_and_annotate(const char *s,
> const struct shash *symtab,
> struct ovs_list *nesting,
> @@ -2633,7 +2651,7 @@ expr_normalize_and(struct expr *expr)
>
> ovs_assert(sub->type == EXPR_T_OR);
> const struct expr_symbol *symbol = expr_get_unique_symbol(sub);
> -if (!symbol || symbol->must_crossproduct) {
> +if (!symbol || force_crossproduct || symbol->must_crossproduct ) {
> struct expr *or = expr_create_andor(EXPR_T_OR);
> struct expr *k;
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index c32a75c26..16587114e 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -596,16 +596,14 @@ tcp,reg14=0x6,tp_dst=500
> tcp,reg14=0x6,tp_dst=501
> ])
> AT_CHECK([expr_to_flow 'outport == {"eth0", "eth1", "eth2"} && ip4 && tcp
> && tcp.src == {400, 401} && tcp.dst == {500, 501}'], [0], [dnl
> -conj_id=1,tcp,reg15=0x5
> -conj_id=2,tcp,reg15=0x6
> -tcp,reg15=0x5,tp_dst=500: conjunction(1, 0/2)
> -tcp,reg15=0x5,tp_dst=501: conjunction(1, 0/2)
> -tcp,reg15=0x5,tp_src=400: conjunction(1, 1/2)
> -tcp,reg15=0x5,tp_src=401: conjunction(1, 1/2)
> -tcp,reg15=0x6,tp_dst=500: conjunction(2, 0/2)
> -tcp,reg15=0x6,tp_dst=501: conjunction(2, 0/2)
> -tcp,reg15=0x6,tp_src=400: conjunction(2, 1/2)
> -tcp,reg15=0x6,tp_src=401: conjunction(2, 1/2)
> +tcp,reg15=0x5,tp_src=400,tp_dst=500
> +tcp,reg15=0x5,tp_src=400,tp_dst=501
> +tcp,reg15=0x5,tp_src=401,tp_dst=500
> +tcp,reg15=0x5,tp_src=401,tp_dst=501
> +tcp,reg15=0x6,tp_src=400,tp_dst=500
> +tcp,reg15=0x6,tp_src=400,tp_dst=501
> +tcp,reg15=0x6,tp_src=401,tp_dst=500
> +tcp,reg15=0x6,tp_src=401,tp_dst=501
> ])
> AT_CHECK([expr_to_flow 'inport == "eth0" && inport == "eth1"'], [0], [dnl
> (no flows)
> @@ -727,22 +725,27 @@ reg15=0x11
> ])
> AT_CLEANUP
>
> -AT_SETUP([ovn -- converting expressions to flows -- conjunction])
> -AT_KEYWORDS([conjunction])
> +AT_SETUP([ovn -- converting expressions to flows -- no conjunction])
> +AT_KEYWORDS([no conjunction])
> expr_to_flow () {
> echo "$1" | ovstest test-ovn expr-to-flows | sort
> }
>
> +# conjunction is disabled in OVN until some of the issues
> +# related to conjunction flows are fixed.
> +# expr-to-flows should not generate any conjunction flows.
> lflow="ip4.src == {10.0.0.1, 10.0.0.2, 10.0.0.3} && \
> ip4.dst == {20.0.0.1, 20.0.0.2, 20.0.0.3}"
> AT_CHECK([expr_to_flow "$lflow"], [0], [dnl
> -conj_id=1,ip
> -ip,nw_dst=20.0.0.1: conjunction(1, 0/2)
> -ip,nw_dst=20.0.0.2: conjunction(1, 0/2)
> -ip,nw_dst=20.0.0.3: conjunction(1, 0/2)
> -ip,nw_src=10.0.0.1: conjunction(1, 1/2)
> -ip,nw_src=10.0.0.2: conjunction(1, 1/2)
> -ip,nw_src=10.0.0.3:
Re: [ovs-dev] [RFC PATCH ovn] Disable conjunction
On Fri, Sep 27, 2019 at 12:04 PM Numan Siddique wrote:
>
>
> On Fri, Sep 27, 2019 at 11:56 AM Han Zhou wrote:
>
>>
>> Thanks Numan. It looks good to me. Is there a reason why this is RFC?
>>
>
> Yes. I actually was thinking another way to disable conjunction.
>
> I will submit that patch and we can discuss further.
>
Hi Han,
Can you please take a look at this patch -
https://patchwork.ozlabs.org/patch/1168634/
and see if this is better than the previous one?
Thanks
Numan
>
> Thanks
> Numan
>
>
>>
>> On Wed, Sep 25, 2019 at 1:48 AM wrote:
>>
>>> From: Numan Siddique
>>>
>>> The commit 298701dbc996("Exclude inport and outport symbol tables from
>>> conjunction")
>>> was earlier added to disable conjunction for inport and outport symbols.
>>> This patch extends it to all the symbos added in the symbol table by
>>> setting
>>> the 'must_crossproduct' field to 'true'.
>>>
>>> There are issues with the conjunction flows generated by ovn-controller.
>>> Please see the commit 298701dbc996 for more information.
>>>
>>> Signed-off-by: Numan Siddique
>>> ---
>>> TODO.rst | 10 +
>>> lib/expr.c |6 +-
>>> lib/logical-fields.c | 72 +--
>>> tests/ovn.at | 1244 +++---
>>> 4 files changed, 1224 insertions(+), 108 deletions(-)
>>>
>>> diff --git a/TODO.rst b/TODO.rst
>>> index 943d9bf81..ed55ea236 100644
>>> --- a/TODO.rst
>>> +++ b/TODO.rst
>>> @@ -145,3 +145,13 @@ OVN To-do List
>>>* Support FTP ALGs.
>>>
>>>* Support reject action.
>>> +
>>> +* Conjunction: Conjunction is disabled in OVN. This needs to be
>>> revisisted
>>> + to enable conjunction again after addressing the issues related to it.
>>> + Like, if there are multiple ACLs with overlapping Conjunction matches,
>>> + conjunction flows are not added properly.
>>> + Eg. match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
>>> + tcp.dst >= 800 && tcp.dst <= 900) actions=drop
>>> +
>>> + match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
>>> + tcp.dst >= 1000 && tcp.dst <= 2000) actions=allow
>>> diff --git a/lib/expr.c b/lib/expr.c
>>> index c0871e1e8..e6fffa701 100644
>>> --- a/lib/expr.c
>>> +++ b/lib/expr.c
>>> @@ -1483,7 +1483,7 @@ expr_symtab_add_subfield(struct shash *symtab,
>>> const char *name,
>>>name, expr_level_to_string(level), f.symbol->name);
>>> }
>>>
>>> -symbol = add_symbol(symtab, name, f.n_bits, prereqs, level, false,
>>> +symbol = add_symbol(symtab, name, f.n_bits, prereqs, level, true,
>>> f.symbol->rw);
>>> symbol->parent = f.symbol;
>>> symbol->parent_ofs = f.ofs;
>>> @@ -1562,7 +1562,7 @@ expr_symtab_add_predicate(struct shash *symtab,
>>> const char *name,
>>> return NULL;
>>> }
>>>
>>> -symbol = add_symbol(symtab, name, 1, NULL, level, false, false);
>>> +symbol = add_symbol(symtab, name, 1, NULL, level, true, false);
>>> symbol->predicate = xstrdup(expansion);
>>> return symbol;
>>> }
>>> @@ -1575,7 +1575,7 @@ expr_symtab_add_ovn_field(struct shash *symtab,
>>> const char *name,
>>> struct expr_symbol *symbol;
>>>
>>> symbol = add_symbol(symtab, name, ovn_field->n_bits, NULL,
>>> -EXPR_L_NOMINAL, false, true);
>>> +EXPR_L_NOMINAL, true, true);
>>> symbol->ovn_field = ovn_field;
>>> return symbol;
>>> }
>>> diff --git a/lib/logical-fields.c b/lib/logical-fields.c
>>> index 8fb591c0a..cddc86ffe 100644
>>> --- a/lib/logical-fields.c
>>> +++ b/lib/logical-fields.c
>>> @@ -77,7 +77,7 @@ ovn_init_symtab(struct shash *symtab)
>>> * unless they're formally defined as subfields. It's a little
>>> awkward. */
>>> for (int xxi = 0; xxi < MFF_N_LOG_REGS / 4; xxi++) {
>>> char *xxname = xasprintf("xxreg%d", xxi);
>>> -expr_symtab_add_field(symtab, xxname, MFF_XXREG0 + xxi, NULL,
>>> false);
>>> +expr_symtab_add_field(symtab, xxname, MFF_XXREG0 + xxi, NULL,
>>> true);
>>> free(xxname);
>>> }
>>> for (int xi = 0; xi < MFF_N_LOG_REGS / 2; xi++) {
>>> @@ -86,7 +86,7 @@ ovn_init_symtab(struct shash *symtab)
>>> if (xxi < MFF_N_LOG_REGS / 4) {
>>> add_subregister(xname, "xxreg", xxi, 64, 1 - xi % 2,
>>> symtab);
>>> } else {
>>> -expr_symtab_add_field(symtab, xname, MFF_XREG0 + xi, NULL,
>>> false);
>>> +expr_symtab_add_field(symtab, xname, MFF_XREG0 + xi, NULL,
>>> true);
>>> }
>>> free(xname);
>>> }
>>> @@ -99,13 +99,13 @@ ovn_init_symtab(struct shash *symtab)
>>> } else if (xi < MFF_N_LOG_REGS / 2) {
>>> add_subregister(name, "xreg", xi, 32, 1 - i % 2, symtab);
>>> } else {
>>> -expr_symtab_add_field(symtab, name, MFF_REG0 + i, NULL,
>>> false);
>>> +expr_symtab_add_field(symtab, name, MFF_REG0 + i, NULL,
>>> true);
>>> }
>>> free(name);
[ovs-dev] [PATCH ovn] Disable conjunction by force cross product for all the fields.
From: Numan Siddique
With this ovn-controller will not generate conjunction flows.
There are issues with the conjunction flows generated by ovn-controller.
Please see the commit 298701dbc996 for more information.
Signed-off-by: Numan Siddique
---
TODO.rst | 10 +
lib/expr.c | 20 +-
tests/ovn.at | 1244 +++---
3 files changed, 1204 insertions(+), 70 deletions(-)
diff --git a/TODO.rst b/TODO.rst
index 943d9bf81..ed55ea236 100644
--- a/TODO.rst
+++ b/TODO.rst
@@ -145,3 +145,13 @@ OVN To-do List
* Support FTP ALGs.
* Support reject action.
+
+* Conjunction: Conjunction is disabled in OVN. This needs to be revisisted
+ to enable conjunction again after addressing the issues related to it.
+ Like, if there are multiple ACLs with overlapping Conjunction matches,
+ conjunction flows are not added properly.
+ Eg. match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
+ tcp.dst >= 800 && tcp.dst <= 900) actions=drop
+
+ match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
+ tcp.dst >= 1000 && tcp.dst <= 2000) actions=allow
diff --git a/lib/expr.c b/lib/expr.c
index c0871e1e8..64ea0aafa 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -32,6 +32,24 @@
VLOG_DEFINE_THIS_MODULE(expr);
+/* Right now conjunction flows generated by ovn-controller
+ * has issues. If there are multiple flows with the same
+ * match for different conjunctions, ovn-controller doesn't
+ * handle it properly.
+ * Eg.
+ * match 1 - ip4.src == {IP1, IP2} && tcp.dst >=500 && tcp.src <=600
+ * action - drop
+ *
+ * match 2 - ip4.src == {IP1, IP2} && tcp.dst >=700 && tcp.src <=800
+ * action - allow.
+ *
+ * To handle this issue temporarily force crossproduct so that conjunction
+ * flows are not generated.
+ *
+ * Remove this once fixed.
+ * */
+static bool force_crossproduct = true;
+
static struct expr *parse_and_annotate(const char *s,
const struct shash *symtab,
struct ovs_list *nesting,
@@ -2633,7 +2651,7 @@ expr_normalize_and(struct expr *expr)
ovs_assert(sub->type == EXPR_T_OR);
const struct expr_symbol *symbol = expr_get_unique_symbol(sub);
-if (!symbol || symbol->must_crossproduct) {
+if (!symbol || force_crossproduct || symbol->must_crossproduct ) {
struct expr *or = expr_create_andor(EXPR_T_OR);
struct expr *k;
diff --git a/tests/ovn.at b/tests/ovn.at
index c32a75c26..16587114e 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -596,16 +596,14 @@ tcp,reg14=0x6,tp_dst=500
tcp,reg14=0x6,tp_dst=501
])
AT_CHECK([expr_to_flow 'outport == {"eth0", "eth1", "eth2"} && ip4 && tcp &&
tcp.src == {400, 401} && tcp.dst == {500, 501}'], [0], [dnl
-conj_id=1,tcp,reg15=0x5
-conj_id=2,tcp,reg15=0x6
-tcp,reg15=0x5,tp_dst=500: conjunction(1, 0/2)
-tcp,reg15=0x5,tp_dst=501: conjunction(1, 0/2)
-tcp,reg15=0x5,tp_src=400: conjunction(1, 1/2)
-tcp,reg15=0x5,tp_src=401: conjunction(1, 1/2)
-tcp,reg15=0x6,tp_dst=500: conjunction(2, 0/2)
-tcp,reg15=0x6,tp_dst=501: conjunction(2, 0/2)
-tcp,reg15=0x6,tp_src=400: conjunction(2, 1/2)
-tcp,reg15=0x6,tp_src=401: conjunction(2, 1/2)
+tcp,reg15=0x5,tp_src=400,tp_dst=500
+tcp,reg15=0x5,tp_src=400,tp_dst=501
+tcp,reg15=0x5,tp_src=401,tp_dst=500
+tcp,reg15=0x5,tp_src=401,tp_dst=501
+tcp,reg15=0x6,tp_src=400,tp_dst=500
+tcp,reg15=0x6,tp_src=400,tp_dst=501
+tcp,reg15=0x6,tp_src=401,tp_dst=500
+tcp,reg15=0x6,tp_src=401,tp_dst=501
])
AT_CHECK([expr_to_flow 'inport == "eth0" && inport == "eth1"'], [0], [dnl
(no flows)
@@ -727,22 +725,27 @@ reg15=0x11
])
AT_CLEANUP
-AT_SETUP([ovn -- converting expressions to flows -- conjunction])
-AT_KEYWORDS([conjunction])
+AT_SETUP([ovn -- converting expressions to flows -- no conjunction])
+AT_KEYWORDS([no conjunction])
expr_to_flow () {
echo "$1" | ovstest test-ovn expr-to-flows | sort
}
+# conjunction is disabled in OVN until some of the issues
+# related to conjunction flows are fixed.
+# expr-to-flows should not generate any conjunction flows.
lflow="ip4.src == {10.0.0.1, 10.0.0.2, 10.0.0.3} && \
ip4.dst == {20.0.0.1, 20.0.0.2, 20.0.0.3}"
AT_CHECK([expr_to_flow "$lflow"], [0], [dnl
-conj_id=1,ip
-ip,nw_dst=20.0.0.1: conjunction(1, 0/2)
-ip,nw_dst=20.0.0.2: conjunction(1, 0/2)
-ip,nw_dst=20.0.0.3: conjunction(1, 0/2)
-ip,nw_src=10.0.0.1: conjunction(1, 1/2)
-ip,nw_src=10.0.0.2: conjunction(1, 1/2)
-ip,nw_src=10.0.0.3: conjunction(1, 1/2)
+ip,nw_src=10.0.0.1,nw_dst=20.0.0.1
+ip,nw_src=10.0.0.1,nw_dst=20.0.0.2
+ip,nw_src=10.0.0.1,nw_dst=20.0.0.3
+ip,nw_src=10.0.0.2,nw_dst=20.0.0.1
+ip,nw_src=10.0.0.2,nw_dst=20.0.0.2
+ip,nw_src=10.0.0.2,nw_dst=20.0.0.3
+ip,nw_src=10.0.0.3,nw_dst=20.0.0.1
+ip,nw_src=10.0.0.3,nw_dst=20.0.0.2
+ip,nw_src=10.0.0.3,nw_dst=20.0.0.3
])
lflow="ip && (!ct.est || (ct.est && ct_label.blocked == 1))"
@@ -756,12 +759,12 @@ ct_state=-est+trk,ipv6
lflow="ip4.src == {10.0.0.1, 10.0.0.2, 10.0.0.3} &&
Re: [ovs-dev] [PATCH 3/3] debug: print mbuf extra info.
On Fri, Sep 27, 2019 at 10:05:53AM -0300, Flavio Leitner wrote: > On Thu, Sep 26, 2019 at 07:49:28AM -0700, Ben Pfaff wrote: > > On Thu, Sep 26, 2019 at 11:59:14AM -0400, Aaron Conole wrote: > > > Flavio Leitner writes: > > > > > > > Signed-off-by: Flavio Leitner > > > > --- > > > > lib/netdev-dpdk.c | 18 ++ > > > > 1 file changed, 18 insertions(+) > > > > > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > > > > index cfbd9a9e5..7965bf57a 100644 > > > > --- a/lib/netdev-dpdk.c > > > > +++ b/lib/netdev-dpdk.c > > > > @@ -2294,6 +2294,24 @@ netdev_dpdk_vhost_update_rx_counters(struct > > > > netdev_stats *stats, > > > > } > > > > > > > > stats->rx_bytes += packet_size; > > > > +#if 0 > > > > > > I've never liked bare '#if 0' in the code. > > > > > > Can you make this something like: > > > > > > #ifndef NDEBUG > > > > > > This means for all debug builds we would get this tracing > > > infrastructure. Maybe even make a special configuration option? > > > > It's so much better if we can avoid #ifdefs. Can we just use VLOG_DBG() > > or its related VLOG_IS_DBG_ENABLED(), VLOG_DBG_RL(), VLOG_DROP_DBG()? > > My bad, I missed to change the subject prefix to PoC or RFC. > Please do NOT apply the patchset, there are bugs and it is complete. *incomplete ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v16 00/14] Support multi-segment mbufs
On Fri, Sep 27, 2019 at 10:37:55AM +, Obrembski, MichalX wrote: > Hi Flavio, Hi Michal! > Thank you very much for your comments, and given proof of concept! > I understood that CPU cost could be a problem, but we currently haven't found > any other solution. > > I saw your patches. In your patches you've moved logic to select which > mempool to use inside DPDK. > https://github.com/fleitner/dpdk/commit/a034eda95497fcd21d9321cdd3259183d3afd00e#diff-15c7e05c889b2a7513f72da68b277986R1521 > > I am a little bit curious, why such a decision? It's impossible to do that in > OvS? Because OvS has no access to virtio ring. The packets are stored in the virtio ring, and DPDK does the dequeue for OvS. Giving that we can see a mix of packet sizes on a batch, the decision of which mpool to use needs to be per packet, at the dequeue time. Then if mpool_64k is available and the packet is beyond the regular MTU-sized buffer, DPDK selects the mpool_64k mbuf instead. That avoids allocating huge amounts of 64k buffers to sustain line rate speeds with small packets. fbl > > Regards, > Michal Obrembski > > > -Original Message- > From: Flavio Leitner [mailto:f...@sysclose.org] > Sent: Monday, September 23, 2019 4:06 PM > To: Obrembski, MichalX > Cc: d...@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v16 00/14] Support multi-segment mbufs > > > > Hi Michal, > > First of all thank you for continuing with the TSO work. > > I spent a bit of time reviewing the patchset and my impression is that the > multi-segment support is quite expensive. Even when TSO is off, we still have > a non trivial CPU cost which we can't optimize further. Also an additional > complexity when dealing with packets to maintain the code in the long term. > > Giving that CPU is a bottleneck that can't be easily upgraded but memory can > be expanded, I thought we could look at an alternative way that uses more > memory instead of relying on CPU. I hope that this work helps to best decide > and support the final solution. > > Different of Mark's (2016?) approach of changing the default mpool to 64k, > this creates one specific memory pool with element size of 64k enough to hold > the maximum packet size. It would guarantee that the packet is always linear, > so we don't need to change any of packet manipulation functions in OvS. > However, we do need to change DPDK API to accept one more memory pool. > > Therefore, I created one extra mpool to hold 64k data packet without local > cache and handled that to rte_vhost_dequeue_burst() which based on the virtio > buffer length decides whether to use the MTU sized or 64k sized mpool. > > The PoC code is not complete as I am sure packet copies, for example, need to > consider the packet size to allocate the new buffer from the right mpool, but > it allows iperf3 to work from a VM using vhost-user client to a server to > show the potential results. > > These are the results[*] using iperf3: > = Baremetal to Baremetal, over 40G link > [ ID] Interval Transfer Bandwidth Retr > [ 4] 0.00-10.00 sec 40.3 GBytes 34.6 Gbits/sec 83 sender > [ 4] 0.00-10.00 sec 40.3 GBytes 34.6 Gbits/sec receiver > > = VM to baremetal using 2.12 without changes: > [ ID] Interval Transfer Bitrate Retr > [ 5] 0.00-10.00 sec 14.2 GBytes 12.2 Gbits/sec0 sender > [ 5] 0.00-10.00 sec 14.2 GBytes 12.2 Gbits/sec receiver > > = VM to baremetal using 2.12 plus patches: > [ ID] Interval Transfer Bitrate Retr > [ 5] 0.00-10.00 sec 43.9 GBytes 37.7 Gbits/sec 81 sender > [ 5] 0.00-10.00 sec 43.9 GBytes 37.7 Gbits/sec receiver > > As you can see, the VM is pushing at line rate speed. > > I will post the patches as a reply to this email. > The code is also available on my github account: > https://github.com/fleitner/ovs/tree/tso-enabled-vhost-v1 > https://github.com/fleitner/dpdk/tree/ovs2.12-vhost-tso-v1 > > [*] Those are quick results, just to a have a confirmation. I would need to > run many more times and also with testpmd + trex to see pps numbers with > different packet sizes to get a real picture, of course. > > Thanks, > fbl > > > On Wed, Sep 11, 2019 at 03:29:49PM +0200, Michal Obrembski wrote: > > > > Overview > > > > This patchset introduces support for multi-segment mbufs to OvS-DPDK. > > Multi-segment mbufs are typically used when the size of an mbuf is > > insufficient to contain the entirety of a packet's data. Instead, the > > data is split across numerous mbufs, each carrying a portion, or > > 'segment', of the packet data. Mbufs are chained via their 'next' > > attribute (an mbuf pointer). > > > > The main motivation behind the support for multi-segment mbufs is to > > later introduce TSO (use case i. below) / GRO in OvS-DPDK, which is > > planned to be introduced after this series. > > > > Use Cases > > = > > i.
Re: [ovs-dev] [PATCHv5] netdev-afxdp: Add need_wakeup supprt.
On 26 Sep 2019, at 21:29, William Tu wrote:
The patch adds support for using need_wakeup flag in AF_XDP rings.
A new option, use_need_wakeup, is added. When this option is used,
it means that OVS has to explicitly wake up the kernel RX, using
poll()
syscall and wake up TX, using sendto() syscall. This feature improves
the performance by avoiding unnecessary sendto syscalls for TX.
For RX, instead of kernel always busy-spinning on fille queue, OVS
wakes
up the kernel RX processing when fill queue is replenished.
The need_wakeup feature is merged into Linux kernel bpf-next tee with
commit
77cd0d7b3f25 ("xsk: add support for need_wakeup flag in AF_XDP rings")
and
OVS enables it by default. Running the feature before this version
causes
xsk bind fails, please use options:use_need_wakeup=false to disable
it.
If users enable it but runs in an older version of libbpf, then the
need_wakeup feature has no effect, and a warning message is logged.
For virtual interface, it's better set use_need_wakeup=false, since
the virtual device's AF_XDP xmit is synchronous: the sendto syscall
enters kernel and process the TX packet on tx queue directly.
On Intel Xeon E5-2620 v3 2.4GHz system, performance of physical port
to physical port improves from 6.1Mpps to 7.3Mpps.
Suggested-by: Ilya Maximets
Signed-off-by: William Tu
Did a quick test, and all works fine. Acke’ed based on previous review
and delta…
Acked-by: Eelco Chaudron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v16 00/14] Support multi-segment mbufs
Hi Flavio, Thank you very much for your comments, and given proof of concept! I understood that CPU cost could be a problem, but we currently haven't found any other solution. I saw your patches. In your patches you've moved logic to select which mempool to use inside DPDK. https://github.com/fleitner/dpdk/commit/a034eda95497fcd21d9321cdd3259183d3afd00e#diff-15c7e05c889b2a7513f72da68b277986R1521 I am a little bit curious, why such a decision? It's impossible to do that in OvS? Regards, Michal Obrembski -Original Message- From: Flavio Leitner [mailto:f...@sysclose.org] Sent: Monday, September 23, 2019 4:06 PM To: Obrembski, MichalX Cc: d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH v16 00/14] Support multi-segment mbufs Hi Michal, First of all thank you for continuing with the TSO work. I spent a bit of time reviewing the patchset and my impression is that the multi-segment support is quite expensive. Even when TSO is off, we still have a non trivial CPU cost which we can't optimize further. Also an additional complexity when dealing with packets to maintain the code in the long term. Giving that CPU is a bottleneck that can't be easily upgraded but memory can be expanded, I thought we could look at an alternative way that uses more memory instead of relying on CPU. I hope that this work helps to best decide and support the final solution. Different of Mark's (2016?) approach of changing the default mpool to 64k, this creates one specific memory pool with element size of 64k enough to hold the maximum packet size. It would guarantee that the packet is always linear, so we don't need to change any of packet manipulation functions in OvS. However, we do need to change DPDK API to accept one more memory pool. Therefore, I created one extra mpool to hold 64k data packet without local cache and handled that to rte_vhost_dequeue_burst() which based on the virtio buffer length decides whether to use the MTU sized or 64k sized mpool. The PoC code is not complete as I am sure packet copies, for example, need to consider the packet size to allocate the new buffer from the right mpool, but it allows iperf3 to work from a VM using vhost-user client to a server to show the potential results. These are the results[*] using iperf3: = Baremetal to Baremetal, over 40G link [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-10.00 sec 40.3 GBytes 34.6 Gbits/sec 83 sender [ 4] 0.00-10.00 sec 40.3 GBytes 34.6 Gbits/sec receiver = VM to baremetal using 2.12 without changes: [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-10.00 sec 14.2 GBytes 12.2 Gbits/sec0 sender [ 5] 0.00-10.00 sec 14.2 GBytes 12.2 Gbits/sec receiver = VM to baremetal using 2.12 plus patches: [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-10.00 sec 43.9 GBytes 37.7 Gbits/sec 81 sender [ 5] 0.00-10.00 sec 43.9 GBytes 37.7 Gbits/sec receiver As you can see, the VM is pushing at line rate speed. I will post the patches as a reply to this email. The code is also available on my github account: https://github.com/fleitner/ovs/tree/tso-enabled-vhost-v1 https://github.com/fleitner/dpdk/tree/ovs2.12-vhost-tso-v1 [*] Those are quick results, just to a have a confirmation. I would need to run many more times and also with testpmd + trex to see pps numbers with different packet sizes to get a real picture, of course. Thanks, fbl On Wed, Sep 11, 2019 at 03:29:49PM +0200, Michal Obrembski wrote: > > Overview > > This patchset introduces support for multi-segment mbufs to OvS-DPDK. > Multi-segment mbufs are typically used when the size of an mbuf is > insufficient to contain the entirety of a packet's data. Instead, the > data is split across numerous mbufs, each carrying a portion, or > 'segment', of the packet data. Mbufs are chained via their 'next' > attribute (an mbuf pointer). > > The main motivation behind the support for multi-segment mbufs is to > later introduce TSO (use case i. below) / GRO in OvS-DPDK, which is > planned to be introduced after this series. > > Use Cases > = > i. Handling oversized (guest-originated) frames, which are marked > for hardware accelration/offload (TSO, for example). > > Packets which originate from a non-DPDK source may be marked for > offload; as such, they may be larger than the permitted ingress > interface's MTU, and may be stored in an oversized dp-packet. In > order to transmit such packets over a DPDK port, their contents > must be copied to a DPDK mbuf (via dpdk_do_tx_copy). However, in > its current implementation, that function only copies data into > a single mbuf; if the space available in the mbuf is exhausted, > but not all packet data has been copied, then it is lost. > Similarly, when cloning a DPDK mbuf, it must be considered > whether that mbuf contains
Re: [ovs-dev] dpdk-latest rebase
On Fri, Sep 27, 2019 at 11:09 AM Kevin Traynor wrote:
>
> On 27/09/2019 09:04, Ilya Maximets wrote:
> > On 27.09.2019 10:43, David Marchand wrote:
> >> On Thu, Sep 26, 2019 at 9:45 PM Stokes, Ian wrote:
> >>> On 9/26/2019 1:09 PM, David Marchand wrote:
> On Thu, Sep 26, 2019 at 11:41 AM Ilya Maximets
> wrote:
> >
> > On 26.09.2019 12:08, Stokes, Ian wrote:
> >> Hi all,
> >>
> >> the dpdk-latest branch has been lagging behind ovs master for a bit.
> >> Now that the OVS 2.12 release is out it seems timely to rebase it on>
> >> top of OVS master and push, note as before this will be a force push>
> >> on the dpdk-latest branch as the rebase on master will change the
> >> commit history.
> >>
> >> I intend to do this today, are there any objections to this?
> >
> > Sounds good to me. Thanks!
> >
> > BTW, last time I tried to build the dpdk-latest branch there was
> > issues with sparse, i.e. some DPDK headers didn't want to build.
> > So, there is a probability that some additional fixes required.
>
> ovs rte_flow.h (sparse header) is out of sync with dpdk master.
> Either we resync it or we remove it :-).
>
>
> >>>
> >>> I've just pushed the rebase. I can take a look at fixing at the sparse
> >>> header for rte_flow.h in the coming days to help fix travis.
> >>
> >> Just tested it, got bitten by some build issue related to ovn removal,
> >> but I suppose this is because my workdir was already configured.
> >> From scratch, it works fine.
> >> Thanks.
> >>
> >>
> >> About sparse and rte_flow.h, just removing include/sparse/rte_flow.h
> >> header does the trick.
> >> I inserted issues in the rte_flow.h from my compiled dpdk and sparse
> >> properly reported them when building ovs.
> >>
> >> What is the reason for keeping this copy of the file in ovs?
> >
> > I think that you fixed the reason recently on DPDK master by the following
> > patch:
> >fbb25a3878cc ("ethdev: fix endian annotation for SPI item")
> >
> > OVS master branch will not build with DPDK 18.11 and sparse enabled.
> >
>
> I can pull this fix into 18.11.3, so it will remove any workarounds for
> sparse.
I sure don't mind :-).
We have other duplicates (rte_{icmp,ip,sctp,tcp,udp}.h) that could be
dropped, but it needs dpdk headers to make use of the rte_beXX_t
types.
I will send patches on dpdk first.
--
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3] Learn the mac binding only if required
On Fri, Sep 27, 2019 at 11:51 AM Han Zhou wrote: > > > On Tue, Sep 24, 2019 at 1:39 PM wrote: > > diff --git a/ovn-architecture.7.xml b/ovn-architecture.7.xml > > index 6115e84b5..c98db48d2 100644 > > --- a/ovn-architecture.7.xml > > +++ b/ovn-architecture.7.xml > > @@ -970,6 +970,24 @@ > > this temporary use.) > > > > > > + > > +R = lookup_arp(P, A, > M); > > +R = lookup_nd(P, A, > M); > > + > > + > > +Implemented by storing arguments into OpenFlow fields, then > > +resubmitting to table 67, which ovn-controller > > +populates with flows generated from the > MAC_Binding > > +table in the OVN Southbound database. If there is a match > in table > > +66, then its actions set the logical flow flag > MLF_LOOKUP_MAC. > > A typo here: s/66/67 > > Otherwise looks good to me. > > Acked-by: Han Zhou > Thanks for the review. I fixed by the typo and applied this patch to master. Numan ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] dpdk-latest rebase
On 27/09/2019 09:04, Ilya Maximets wrote:
> On 27.09.2019 10:43, David Marchand wrote:
>> On Thu, Sep 26, 2019 at 9:45 PM Stokes, Ian wrote:
>>> On 9/26/2019 1:09 PM, David Marchand wrote:
On Thu, Sep 26, 2019 at 11:41 AM Ilya Maximets wrote:
>
> On 26.09.2019 12:08, Stokes, Ian wrote:
>> Hi all,
>>
>> the dpdk-latest branch has been lagging behind ovs master for a bit.
>> Now that the OVS 2.12 release is out it seems timely to rebase it on>
>> top of OVS master and push, note as before this will be a force push> on
>> the dpdk-latest branch as the rebase on master will change the
>> commit history.
>>
>> I intend to do this today, are there any objections to this?
>
> Sounds good to me. Thanks!
>
> BTW, last time I tried to build the dpdk-latest branch there was
> issues with sparse, i.e. some DPDK headers didn't want to build.
> So, there is a probability that some additional fixes required.
ovs rte_flow.h (sparse header) is out of sync with dpdk master.
Either we resync it or we remove it :-).
>>>
>>> I've just pushed the rebase. I can take a look at fixing at the sparse
>>> header for rte_flow.h in the coming days to help fix travis.
>>
>> Just tested it, got bitten by some build issue related to ovn removal,
>> but I suppose this is because my workdir was already configured.
>> From scratch, it works fine.
>> Thanks.
>>
>>
>> About sparse and rte_flow.h, just removing include/sparse/rte_flow.h
>> header does the trick.
>> I inserted issues in the rte_flow.h from my compiled dpdk and sparse
>> properly reported them when building ovs.
>>
>> What is the reason for keeping this copy of the file in ovs?
>
> I think that you fixed the reason recently on DPDK master by the following
> patch:
>fbb25a3878cc ("ethdev: fix endian annotation for SPI item")
>
> OVS master branch will not build with DPDK 18.11 and sparse enabled.
>
I can pull this fix into 18.11.3, so it will remove any workarounds for
sparse.
thanks,
Kevin.
> Best regards, Ilya Maximets.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] dpdk-latest rebase
On 27.09.2019 10:43, David Marchand wrote:
On Thu, Sep 26, 2019 at 9:45 PM Stokes, Ian wrote:
On 9/26/2019 1:09 PM, David Marchand wrote:
On Thu, Sep 26, 2019 at 11:41 AM Ilya Maximets wrote:
On 26.09.2019 12:08, Stokes, Ian wrote:
Hi all,
the dpdk-latest branch has been lagging behind ovs master for a bit.
Now that the OVS 2.12 release is out it seems timely to rebase it on> top of OVS
master and push, note as before this will be a force push> on the dpdk-latest
branch as the rebase on master will change the
commit history.
I intend to do this today, are there any objections to this?
Sounds good to me. Thanks!
BTW, last time I tried to build the dpdk-latest branch there was
issues with sparse, i.e. some DPDK headers didn't want to build.
So, there is a probability that some additional fixes required.
ovs rte_flow.h (sparse header) is out of sync with dpdk master.
Either we resync it or we remove it :-).
I've just pushed the rebase. I can take a look at fixing at the sparse
header for rte_flow.h in the coming days to help fix travis.
Just tested it, got bitten by some build issue related to ovn removal,
but I suppose this is because my workdir was already configured.
From scratch, it works fine.
Thanks.
About sparse and rte_flow.h, just removing include/sparse/rte_flow.h
header does the trick.
I inserted issues in the rte_flow.h from my compiled dpdk and sparse
properly reported them when building ovs.
What is the reason for keeping this copy of the file in ovs?
I think that you fixed the reason recently on DPDK master by the following
patch:
fbb25a3878cc ("ethdev: fix endian annotation for SPI item")
OVS master branch will not build with DPDK 18.11 and sparse enabled.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] dpdk-latest rebase
On Thu, Sep 26, 2019 at 9:45 PM Stokes, Ian wrote: > On 9/26/2019 1:09 PM, David Marchand wrote: > > On Thu, Sep 26, 2019 at 11:41 AM Ilya Maximets wrote: > >> > >> On 26.09.2019 12:08, Stokes, Ian wrote: > >>> Hi all, > >>> > >>> the dpdk-latest branch has been lagging behind ovs master for a bit. > >>> Now that the OVS 2.12 release is out it seems timely to rebase it on> top > >>> of OVS master and push, note as before this will be a force push> on the > >>> dpdk-latest branch as the rebase on master will change the > >>> commit history. > >>> > >>> I intend to do this today, are there any objections to this? > >> > >> Sounds good to me. Thanks! > >> > >> BTW, last time I tried to build the dpdk-latest branch there was > >> issues with sparse, i.e. some DPDK headers didn't want to build. > >> So, there is a probability that some additional fixes required. > > > > ovs rte_flow.h (sparse header) is out of sync with dpdk master. > > Either we resync it or we remove it :-). > > > > > > I've just pushed the rebase. I can take a look at fixing at the sparse > header for rte_flow.h in the coming days to help fix travis. Just tested it, got bitten by some build issue related to ovn removal, but I suppose this is because my workdir was already configured. >From scratch, it works fine. Thanks. About sparse and rte_flow.h, just removing include/sparse/rte_flow.h header does the trick. I inserted issues in the rte_flow.h from my compiled dpdk and sparse properly reported them when building ovs. What is the reason for keeping this copy of the file in ovs? -- David Marchand ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] DDLog after one week
On Thu, Sep 26, 2019 at 6:45 PM Leonid Ryzhyk wrote:
>
> > > > Q: Why doesn't the ddlog compiler generate Out_IP_Multicast and only
> > > > "input IP_Multicast" in OVN_Southbound.dl.
> > > > A: For every table to which ovn-northd-ddlog needs to write we need to
> > > > update northd/automake.mk. Could we make this more robust or is it
> > > > fine to keep adding tables/keys/readonly fields in the makefile?
> > >
> > > I can see how this is error-prone, but I am not sure how to improve
> > > things other
> > > than adding documentation (which you already mostly did)
> > >
> > > Problem is, I cannot extract this information automatically from the
> > > OVSDB schema,
> > > so somehow the user has to tell the tool about all these attributes. We
> > > could store
> > > these attributes in a separate configuration file or even add them as
> > > annotations
> > > to the OVSDB schema, but I am not sure this will significanly improve the
> > > usability.
> > > I am of course open to suggestions.
> >
> > I agree that it would be hard to automate so I see two options:
> > 1. generate all the tables (including Proxy) for all OVSDB tables in
> > the schema. One issue with that would be the read-only columns and
> > keys which would still need to be manually specified. Is there any
> > other downside to this? Compile time increase?
>
> Generating output tables for everything will actually break things. The way
> DDlog is setup for ovn-northd is that it compares all tables labeled as
> output (`-o` in `automake.mk`) generated by the DDlog program against the
> current state in OVSDB and generates an OVSDB transaction to sync OVSDB state
> with DDlog. If the DDlog program does not produce any records for an OVSDB
> table labeled as output, this will have the effect of deleting everything
> from that table.
I see, thanks for clearing it up for me.
>
> > 2. add a new ovsdb2ddlog argument to pass a "spec" file with the
> > tables that need to be translated to ddlog. Mainly what we have now in
> > automake.mk but isolated in its own file to have a clearer view.
> >
> > If it's not too much work I would go for option 2. Any thoughts?
>
> Agreed, this will also resolve https://github.com/ovn-org/ovn/issues/15
>
> I created an issue for this:
> https://github.com/vmware/differential-datalog/issues/393
>
> Also, just want to mention that, once we have the simplified OVSDB interface,
> we should no longer need to specify keys (the `-k` switch), since the `uuid`
> column will always act as a key.
Nice!
>
> > > > Q: Is there a way to leave a field uninitialized in an output
> > > > relation? For example "eth_src" in sb.IP_Multicast is of type "string"
> > > > in the schema but I couldn't find a way to leave it uninitialized
> > > > (NULL) in DDlog.
> > >
> > > If a column is declared with multiplicity `"min": 0, "max": 1`, then it
> > > ends up being compiled
> > > into a DDlog `Set<>` (it should really be `Option<>` , but sets made
> > > life a bit easier for me;
> > > this is one of the things I would like to improve in the future), and
> > > then an empty
> > > set can be used to represent `NULL`. I was under the impression that a
> > > column with
> > > multilplicity of exactly `1`, such as `sb.IP_Multicast` is not allowed to
> > > be NULL. Am I wrong?
> > > If so, when exactly are NULLs allowed by OVSDB?
> >
> > Actually the definition of sb.IP_Multicast.eth_src is:
> >
> > "eth_src": {"type": "string"}
>
> Which, I think, implies multiplicity 1.
>
> > I guess the only case when NULLs occur in OVSDB is for strings.
>
> Do you know if this is specified anywhere? I could not find anything relevant
> in the OVSDB RFC.
I'm not really sure tbh. Maybe the others can comment on this too.
>
> > > > - Maybe the ddlog runtime could pass more information to the callback
> > > > that is registered in ddlog_run. Right now we can only dump the record
> > > > and deduce the operation from the weight argument. I didn't
> > > > investigate enough but it would be nice if we could filter what types
> > > > of records we want the callback to be called for.
> > >
> > > The callback also takes `table_id`, which can be used to filter out only
> > > records that belong to a subset of relations of interest. It can be used
> > > in conjunction with `ddlog_get_table_id`, which maps table name to id.
> > > We could also allow the user to subscribe to a subset of relations as you
> > > are suggesting. My only concern is that it will make the DDlog API more
> > > complex.
> >
> > Ah, right, we have the table_id. I missed that. Is there also a
> > ddlog_get_table_name(table_id) API? I didn't see any when I looked
> > last.
>
> Nope, but there has to be one:
> https://github.com/vmware/differential-datalog/issues/394
Ack.
>
> > > Also, I want to mention that a better way to observe changes to relations
> > > is
> > > to use `ddlog_transaction_commit_dump_changes` instead of
> > > `ddlog_transaction_commit`. It takes a
[ovs-dev] IMPORTANT! You hαve been recorded ʍasturbating! I hαve Ovs Dev.mp4!
ATTN: ovs-dev@openvswitch.org The last time you visited a Ƿorn website with teenagers, you downloaded and installed the vίrus I developed. My program has turned on your cam and recorded the act of your ʍasturbation.. My software also downloaded all your email contact lίsts and a list of your friends on Facebook. I have the - Ovs Dev.mp4 - with you jerkίng off to teens, as well as a file with all your contacts on my computer. You are very Ƿerverted! If you want me to delete both files and keep the secret, you must send me the Bitcoin payment. I gıve you 72 houɼs onɭy to transƒer the funds. If you don't know how to pay with Bitcoin, visit Google and search - how to buy bitcoin. * Send 2,000 USD = 0.2575470 BTC to this Bitcoin address as soon as possible: 39PqHha5HZMWwx8RLTn8ezLi9armpGaihW (copy & pαste) * 1 BTC = 7,820 USD right now, so send exactly 0.2575470 BTC to the address above. Do not try to cheat me! As soon as you open this Email I will know you opened it. I am tracking all actions on your device.. This Bitcoin address is linked to you onɭy, so I will know when you send the correct amount. When you pay in full, I will remove both files and deactivate my program. If you choose to not send the btc... I will send your ʍasturbation vίdeo to ALL YOUR FRIENDS AND ASSOCIATES from your contact lists that I hacked. Here are the payment details again: Send 2,000 USD (0.2575470 BTC) to this Bitcoin address: * 0.2575470 BTC to: 39PqHha5HZMWwx8RLTn8ezLi9armpGaihW (copy & pαste) * You саn visit police but nobody can help you. I know what I am doing. I don't live in your country and I know how to stay anonymous. Don't try to deceive me - I will know it immediately - my spy software is recording all the websites you visit and all your key presses. If you do - I will send this ugly vίd to eveɼyone you know, INCLUDING YOUR FAMILY MEMBERS. Don't cheat me! Don't forget the shame and if you ignore this message your life will be ruined. I am waiting for your Bitcoin payment. You have 72 houɼs ɭeft. Aʼnonym0us Hʌcker P.S. If you need more time to buy and send BTC, open your notepad and write '48h more'. This way you can contact me. I will consider gıving you another 48 houɼs befoɼe I releɑse the vίd, but onɭy when I see that you are really struggling to buy bitcoin. I KNOW you can afford it - so don't play around... ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC PATCH ovn] Disable conjunction
On Fri, Sep 27, 2019 at 11:56 AM Han Zhou wrote:
>
> Thanks Numan. It looks good to me. Is there a reason why this is RFC?
>
Yes. I actually was thinking another way to disable conjunction.
I will submit that patch and we can discuss further.
Thanks
Numan
>
> On Wed, Sep 25, 2019 at 1:48 AM wrote:
>
>> From: Numan Siddique
>>
>> The commit 298701dbc996("Exclude inport and outport symbol tables from
>> conjunction")
>> was earlier added to disable conjunction for inport and outport symbols.
>> This patch extends it to all the symbos added in the symbol table by
>> setting
>> the 'must_crossproduct' field to 'true'.
>>
>> There are issues with the conjunction flows generated by ovn-controller.
>> Please see the commit 298701dbc996 for more information.
>>
>> Signed-off-by: Numan Siddique
>> ---
>> TODO.rst | 10 +
>> lib/expr.c |6 +-
>> lib/logical-fields.c | 72 +--
>> tests/ovn.at | 1244 +++---
>> 4 files changed, 1224 insertions(+), 108 deletions(-)
>>
>> diff --git a/TODO.rst b/TODO.rst
>> index 943d9bf81..ed55ea236 100644
>> --- a/TODO.rst
>> +++ b/TODO.rst
>> @@ -145,3 +145,13 @@ OVN To-do List
>>* Support FTP ALGs.
>>
>>* Support reject action.
>> +
>> +* Conjunction: Conjunction is disabled in OVN. This needs to be
>> revisisted
>> + to enable conjunction again after addressing the issues related to it.
>> + Like, if there are multiple ACLs with overlapping Conjunction matches,
>> + conjunction flows are not added properly.
>> + Eg. match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
>> + tcp.dst >= 800 && tcp.dst <= 900) actions=drop
>> +
>> + match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
>> + tcp.dst >= 1000 && tcp.dst <= 2000) actions=allow
>> diff --git a/lib/expr.c b/lib/expr.c
>> index c0871e1e8..e6fffa701 100644
>> --- a/lib/expr.c
>> +++ b/lib/expr.c
>> @@ -1483,7 +1483,7 @@ expr_symtab_add_subfield(struct shash *symtab,
>> const char *name,
>>name, expr_level_to_string(level), f.symbol->name);
>> }
>>
>> -symbol = add_symbol(symtab, name, f.n_bits, prereqs, level, false,
>> +symbol = add_symbol(symtab, name, f.n_bits, prereqs, level, true,
>> f.symbol->rw);
>> symbol->parent = f.symbol;
>> symbol->parent_ofs = f.ofs;
>> @@ -1562,7 +1562,7 @@ expr_symtab_add_predicate(struct shash *symtab,
>> const char *name,
>> return NULL;
>> }
>>
>> -symbol = add_symbol(symtab, name, 1, NULL, level, false, false);
>> +symbol = add_symbol(symtab, name, 1, NULL, level, true, false);
>> symbol->predicate = xstrdup(expansion);
>> return symbol;
>> }
>> @@ -1575,7 +1575,7 @@ expr_symtab_add_ovn_field(struct shash *symtab,
>> const char *name,
>> struct expr_symbol *symbol;
>>
>> symbol = add_symbol(symtab, name, ovn_field->n_bits, NULL,
>> -EXPR_L_NOMINAL, false, true);
>> +EXPR_L_NOMINAL, true, true);
>> symbol->ovn_field = ovn_field;
>> return symbol;
>> }
>> diff --git a/lib/logical-fields.c b/lib/logical-fields.c
>> index 8fb591c0a..cddc86ffe 100644
>> --- a/lib/logical-fields.c
>> +++ b/lib/logical-fields.c
>> @@ -77,7 +77,7 @@ ovn_init_symtab(struct shash *symtab)
>> * unless they're formally defined as subfields. It's a little
>> awkward. */
>> for (int xxi = 0; xxi < MFF_N_LOG_REGS / 4; xxi++) {
>> char *xxname = xasprintf("xxreg%d", xxi);
>> -expr_symtab_add_field(symtab, xxname, MFF_XXREG0 + xxi, NULL,
>> false);
>> +expr_symtab_add_field(symtab, xxname, MFF_XXREG0 + xxi, NULL,
>> true);
>> free(xxname);
>> }
>> for (int xi = 0; xi < MFF_N_LOG_REGS / 2; xi++) {
>> @@ -86,7 +86,7 @@ ovn_init_symtab(struct shash *symtab)
>> if (xxi < MFF_N_LOG_REGS / 4) {
>> add_subregister(xname, "xxreg", xxi, 64, 1 - xi % 2, symtab);
>> } else {
>> -expr_symtab_add_field(symtab, xname, MFF_XREG0 + xi, NULL,
>> false);
>> +expr_symtab_add_field(symtab, xname, MFF_XREG0 + xi, NULL,
>> true);
>> }
>> free(xname);
>> }
>> @@ -99,13 +99,13 @@ ovn_init_symtab(struct shash *symtab)
>> } else if (xi < MFF_N_LOG_REGS / 2) {
>> add_subregister(name, "xreg", xi, 32, 1 - i % 2, symtab);
>> } else {
>> -expr_symtab_add_field(symtab, name, MFF_REG0 + i, NULL,
>> false);
>> +expr_symtab_add_field(symtab, name, MFF_REG0 + i, NULL,
>> true);
>> }
>> free(name);
>> }
>>
>> /* Flags used in logical to physical transformation. */
>> -expr_symtab_add_field(symtab, "flags", MFF_LOG_FLAGS, NULL, false);
>> +expr_symtab_add_field(symtab, "flags", MFF_LOG_FLAGS, NULL, true);
>> char flags_str[16];
>> snprintf(flags_str, sizeof flags_str, "flags[%d]",
>> MLF_ALLOW_LOOPBACK_BIT);
>>
Re: [ovs-dev] [RFC PATCH ovn] Disable conjunction
Thanks Numan. It looks good to me. Is there a reason why this is RFC?
On Wed, Sep 25, 2019 at 1:48 AM wrote:
> From: Numan Siddique
>
> The commit 298701dbc996("Exclude inport and outport symbol tables from
> conjunction")
> was earlier added to disable conjunction for inport and outport symbols.
> This patch extends it to all the symbos added in the symbol table by
> setting
> the 'must_crossproduct' field to 'true'.
>
> There are issues with the conjunction flows generated by ovn-controller.
> Please see the commit 298701dbc996 for more information.
>
> Signed-off-by: Numan Siddique
> ---
> TODO.rst | 10 +
> lib/expr.c |6 +-
> lib/logical-fields.c | 72 +--
> tests/ovn.at | 1244 +++---
> 4 files changed, 1224 insertions(+), 108 deletions(-)
>
> diff --git a/TODO.rst b/TODO.rst
> index 943d9bf81..ed55ea236 100644
> --- a/TODO.rst
> +++ b/TODO.rst
> @@ -145,3 +145,13 @@ OVN To-do List
>* Support FTP ALGs.
>
>* Support reject action.
> +
> +* Conjunction: Conjunction is disabled in OVN. This needs to be revisisted
> + to enable conjunction again after addressing the issues related to it.
> + Like, if there are multiple ACLs with overlapping Conjunction matches,
> + conjunction flows are not added properly.
> + Eg. match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
> + tcp.dst >= 800 && tcp.dst <= 900) actions=drop
> +
> + match(ip4.src == {IP1, IP2, IP3} && ip4.dst == {IP4, IP5, IP6} &&
> + tcp.dst >= 1000 && tcp.dst <= 2000) actions=allow
> diff --git a/lib/expr.c b/lib/expr.c
> index c0871e1e8..e6fffa701 100644
> --- a/lib/expr.c
> +++ b/lib/expr.c
> @@ -1483,7 +1483,7 @@ expr_symtab_add_subfield(struct shash *symtab, const
> char *name,
>name, expr_level_to_string(level), f.symbol->name);
> }
>
> -symbol = add_symbol(symtab, name, f.n_bits, prereqs, level, false,
> +symbol = add_symbol(symtab, name, f.n_bits, prereqs, level, true,
> f.symbol->rw);
> symbol->parent = f.symbol;
> symbol->parent_ofs = f.ofs;
> @@ -1562,7 +1562,7 @@ expr_symtab_add_predicate(struct shash *symtab,
> const char *name,
> return NULL;
> }
>
> -symbol = add_symbol(symtab, name, 1, NULL, level, false, false);
> +symbol = add_symbol(symtab, name, 1, NULL, level, true, false);
> symbol->predicate = xstrdup(expansion);
> return symbol;
> }
> @@ -1575,7 +1575,7 @@ expr_symtab_add_ovn_field(struct shash *symtab,
> const char *name,
> struct expr_symbol *symbol;
>
> symbol = add_symbol(symtab, name, ovn_field->n_bits, NULL,
> -EXPR_L_NOMINAL, false, true);
> +EXPR_L_NOMINAL, true, true);
> symbol->ovn_field = ovn_field;
> return symbol;
> }
> diff --git a/lib/logical-fields.c b/lib/logical-fields.c
> index 8fb591c0a..cddc86ffe 100644
> --- a/lib/logical-fields.c
> +++ b/lib/logical-fields.c
> @@ -77,7 +77,7 @@ ovn_init_symtab(struct shash *symtab)
> * unless they're formally defined as subfields. It's a little
> awkward. */
> for (int xxi = 0; xxi < MFF_N_LOG_REGS / 4; xxi++) {
> char *xxname = xasprintf("xxreg%d", xxi);
> -expr_symtab_add_field(symtab, xxname, MFF_XXREG0 + xxi, NULL,
> false);
> +expr_symtab_add_field(symtab, xxname, MFF_XXREG0 + xxi, NULL,
> true);
> free(xxname);
> }
> for (int xi = 0; xi < MFF_N_LOG_REGS / 2; xi++) {
> @@ -86,7 +86,7 @@ ovn_init_symtab(struct shash *symtab)
> if (xxi < MFF_N_LOG_REGS / 4) {
> add_subregister(xname, "xxreg", xxi, 64, 1 - xi % 2, symtab);
> } else {
> -expr_symtab_add_field(symtab, xname, MFF_XREG0 + xi, NULL,
> false);
> +expr_symtab_add_field(symtab, xname, MFF_XREG0 + xi, NULL,
> true);
> }
> free(xname);
> }
> @@ -99,13 +99,13 @@ ovn_init_symtab(struct shash *symtab)
> } else if (xi < MFF_N_LOG_REGS / 2) {
> add_subregister(name, "xreg", xi, 32, 1 - i % 2, symtab);
> } else {
> -expr_symtab_add_field(symtab, name, MFF_REG0 + i, NULL,
> false);
> +expr_symtab_add_field(symtab, name, MFF_REG0 + i, NULL, true);
> }
> free(name);
> }
>
> /* Flags used in logical to physical transformation. */
> -expr_symtab_add_field(symtab, "flags", MFF_LOG_FLAGS, NULL, false);
> +expr_symtab_add_field(symtab, "flags", MFF_LOG_FLAGS, NULL, true);
> char flags_str[16];
> snprintf(flags_str, sizeof flags_str, "flags[%d]",
> MLF_ALLOW_LOOPBACK_BIT);
> expr_symtab_add_subfield(symtab, "flags.loopback", NULL, flags_str);
> @@ -119,12 +119,12 @@ ovn_init_symtab(struct shash *symtab)
> flags_str);
>
> /* Connection tracking state. */
> -expr_symtab_add_field(symtab, "ct_mark", MFF_CT_MARK, NULL, false);
> +expr_symtab_add_field(symtab,
Re: [ovs-dev] [PATCH ovn v3] Learn the mac binding only if required
On Tue, Sep 24, 2019 at 1:39 PM wrote: > diff --git a/ovn-architecture.7.xml b/ovn-architecture.7.xml > index 6115e84b5..c98db48d2 100644 > --- a/ovn-architecture.7.xml > +++ b/ovn-architecture.7.xml > @@ -970,6 +970,24 @@ > this temporary use.) > > > + > +R = lookup_arp(P, A, M); > +R = lookup_nd(P, A, M); > + > + > +Implemented by storing arguments into OpenFlow fields, then > +resubmitting to table 67, which ovn-controller > +populates with flows generated from the MAC_Binding > +table in the OVN Southbound database. If there is a match in table > +66, then its actions set the logical flow flag MLF_LOOKUP_MAC. A typo here: s/66/67 Otherwise looks good to me. Acked-by: Han Zhou ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
