Re: [ovs-dev] [PATCH ovn v4 0/2] Add a way to delete QoS directly
Sorry for the late reply. Please see my response below. On Wed, Jan 8, 2020 at 5:16 PM taoyunxi...@cmss.chinamobile.com < taoyunxi...@cmss.chinamobile.com> wrote: > > Hi Han, > If you have time, Could you review this patch or give a response. > > > Thanks , > Yun > > > > From: taoyunxi...@cmss.chinamobile.com > Date: 2019-12-24 19:55 > To: Han Zhou > CC: ovs-dev > Subject: Re: Re: [ovs-dev] [PATCH ovn v4 0/2] Add a way to delete QoS directly > Hi Han, > Thanks for your review! > > 1. As for point 1 and point 4, I think it is not bad to use external_ids to record association between Neutron and OVN, even we can record LS info also. But it is not directly and clearly. > Actually, I find almost resource tables have "name" column, included ACL table, which summited by this patch[0]. The purpose of adding "name" for ACL, is also to make ACL rule could be easily find and check in OVN. Ok, the name column of ACL table was added for the logging purpose, but I agree with you it can be used the way you intended to. > > 2. As for point 2, I am agreed, but do not know how to do now. There are several ways. Option1: Keep the old command and add a new command for deleting by name. Option2: Add an option to the old command such as "--by-name" to delete by name. Option3: Keep the SWITCH arg when deleting by name, but only for the given switch. If the next argument can be found as a QoS name in the switch, it is treated as delete by name, otherwise, it is handled as before. > > 3. As for point 3, I think we can record LS info, it is not bad. > > > As for why I want to change "qos-del" comand, I want to explain more detailed. Hope to get your advice. > > 1. QoS rule resource of Neutron is not mapping any resource of OVN, and it just contains rate, burst, dircetion and so on. > > 2. In Neutron, when we bind QoS rule to port or network, it will triger OVN to record in QoS table. > > 3. When we update QoS rule in Neutron , it will DIRECTLY update its record of Neutron DB(will not wait OVN to update, because no mapping resource), gradually, it will update port or network which binded this QoS rule. But the QoS rule has already become the lasted one in Neutron DB, and networking-ovn can not get the old QoS rule of Neutron. So we can not update it exactly. > > 4. For the above situaiton, we can only delete old rules that have the same direction as the new rules, or , we will expand the scope of deletion. > This patch [1] is the logic process for networking-ovn to execute QoS rule. The update process is not exactly and suitable, I think the Core OVN may need some changes. > I agree with you that it is more efficient to delete with an ID instead of searching based on fields. I think we need to consider below two factors: 1. If the new "name" column is supposed to uniquely identify a row, it is better to add the "index" constraint in the schema for this new column, to avoid duplicated names. 2. Ben recently worked on the new feature of OVSDB to make it allows client to specify row uuid when creating a row [0]. If this patch is merged, we don't need a "name" column to uniquely identify a row any more. I'd prefer to utilize this new feature and to avoid adding new "name" columns for individual tables. Do you think this is helpful? [0] https://patchwork.ozlabs.org/patch/1220644/ > > [0] https://github.com/ovn-org/ovn/commit/17df18dac9d2fa875c2f86a54bbc84931689d42c > [1] https://review.opendev.org/#/c/692084/ > > > Hope to have your advice. > > > Thanks, > Yun > > > From: Han Zhou > Date: 2019-12-24 01:33 > To: Yunxiang Tao > CC: ovs-dev > Subject: Re: [ovs-dev] [PATCH ovn v4 0/2] Add a way to delete QoS directly > > > On Mon, Dec 23, 2019 at 2:37 AM Yunxiang Tao < taoyunxi...@cmss.chinamobile.com> wrote: > > > > Currently, qos can only be deleted by indirect way which must designate > > switch or more parameters. The first patch add "name" column to QoS table, > > and make QoS table could be listed by "name" witch comand > > "ovn-nbctl list qos "name"". > > > > The second patch change the original "qos-del" to "ls-qos-del", add > > a new "qos-del" command. By the new command, you can delete qos > > by uuid or name of the qos. It is useful. For example, we can associate > > the qos table in neutron and OVN by "name" of qos in OVN, so neutron > > could find and easily delete the corresponding qos in OVN. > > > > Thanks for the patch. I have below comments: > 1. There are other ways to associate QoS between Neutron and OVN. For example, the external-ids column can be used to add any customized key-value pairs, such as external_ids:neutron:qos_name = . Is this sufficient for your use case? > 2. If we believe it is useful for qos-del command to delete qos by name/uuid, it is better to extend the exist command "qos-del" to handle both old and new formats, and we should NOT rename the existing command because it will break existing
Re: [ovs-dev] [PATCH v2] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows.
On Thu, Jan 9, 2020 at 12:48 PM Ben Pfaff wrote:
>
> Requested-by: Leonid Ryzhyk
> Signed-off-by: Ben Pfaff
> ---
> v1->v2: Improve test as suggested by Flavio Fernandes.
>
> Documentation/ref/ovsdb-server.7.rst | 9 +
> NEWS | 1 +
> ovsdb/execution.c| 26 ++
> ovsdb/transaction.c | 22 +-
> ovsdb/transaction.h | 5 -
> tests/ovsdb-execution.at | 24
> tests/uuidfilt.py| 18 --
> 7 files changed, 97 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/ref/ovsdb-server.7.rst
b/Documentation/ref/ovsdb-server.7.rst
> index bc533611cb4a..967761bdfb84 100644
> --- a/Documentation/ref/ovsdb-server.7.rst
> +++ b/Documentation/ref/ovsdb-server.7.rst
> @@ -546,6 +546,15 @@ condition can be either a 3-element JSON array as
described in the RFC or a
> boolean value. In case of an empty array an implicit true boolean value
will be
> considered.
>
> +5.2.1 Insert
> +
> +
> +As an extension, Open vSwitch 2.13 and later allow an optional ``uuid``
member
> +to specify the UUID for the new row. The specified UUID must be unique
within
> +the table when it is inserted and not the UUID of a row previously
deleted
> +within the transaction. If the UUID violates these rules, then the
operation
> +fails with a ``duplicate uuid`` error.
> +
> 5.2.6 Wait, 5.2.7 Commit, 5.2.9 Comment
> ---
>
> diff --git a/NEWS b/NEWS
> index 965facaf852d..d8f82cd18af0 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -34,6 +34,7 @@ Post-v2.12.0
> interval is increased to 60 seconds for the connection to the
> replication server. This value is configurable with the unixctl
> command - ovsdb-server/set-active-ovsdb-server-probe-interval.
> + * ovsdb-server: New OVSDB extension to allow clients to specify row
UUIDs.
>
> v2.12.0 - 03 Sep 2019
> -
> diff --git a/ovsdb/execution.c b/ovsdb/execution.c
> index f2cf3d72d45f..e45f3d6796a7 100644
> --- a/ovsdb/execution.c
> +++ b/ovsdb/execution.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017 Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017, 2019 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -329,11 +329,12 @@ ovsdb_execute_insert(struct ovsdb_execution *x,
struct ovsdb_parser *parser,
> {
> struct ovsdb_table *table;
> struct ovsdb_row *row = NULL;
> -const struct json *uuid_name, *row_json;
> +const struct json *uuid_json, *uuid_name, *row_json;
> struct ovsdb_error *error;
> struct uuid row_uuid;
>
> table = parse_table(x, parser, "table");
> +uuid_json = ovsdb_parser_member(parser, "uuid", OP_STRING |
OP_OPTIONAL);
> uuid_name = ovsdb_parser_member(parser, "uuid-name", OP_ID |
OP_OPTIONAL);
> row_json = ovsdb_parser_member(parser, "row", OP_OBJECT);
> error = ovsdb_parser_get_error(parser);
> @@ -341,6 +342,19 @@ ovsdb_execute_insert(struct ovsdb_execution *x,
struct ovsdb_parser *parser,
> return error;
> }
>
> +if (uuid_json) {
> +if (!uuid_from_string(_uuid, json_string(uuid_json))) {
> +return ovsdb_syntax_error(uuid_json, NULL, "bad uuid");
> +}
> +
> +if (!ovsdb_txn_may_create_row(table, _uuid)) {
> +return ovsdb_syntax_error(uuid_json, "duplicate uuid",
> + "This UUID would duplicate a UUID "
> + "already present within the table
or "
> + "deleted within the same
transaction.");
> +}
> +}
> +
> if (uuid_name) {
> struct ovsdb_symbol *symbol;
>
> @@ -350,9 +364,13 @@ ovsdb_execute_insert(struct ovsdb_execution *x,
struct ovsdb_parser *parser,
>"This \"uuid-name\" appeared on an
"
>"earlier \"insert\" operation.");
> }
> -row_uuid = symbol->uuid;
> +if (uuid_json) {
> +symbol->uuid = row_uuid;
> +} else {
> +row_uuid = symbol->uuid;
> +}
> symbol->created = true;
> -} else {
> +} else if (!uuid_json) {
> uuid_generate(_uuid);
> }
>
> diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
> index 866fabe8df57..369436bffbf5 100644
> --- a/ovsdb/transaction.c
> +++ b/ovsdb/transaction.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017 Nicira,
Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017, 2019
Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with
Re: [ovs-dev] [ovs-dev v3] dpif-netdev: Allow to set max capacity of flow on netdev.
On Fri, Jan 10, 2020 at 2:04 AM Ben Pfaff wrote: > > On Thu, Jan 09, 2020 at 04:14:10PM +0100, Ilya Maximets wrote: > > On 09.01.2020 08:36, xiangxia.m@gmail.com wrote: > > > From: Tonghao Zhang > > > > > > For installing more than MAX_FLOWS (65536) flows to netdev datapath. > > > Add the ovs-appctl subcommand "dpif-netdev/pmd-set-max-flow" which > > > can change the flow number which netdev datapath support. > > > > > > Signed-off-by: Tonghao Zhang > > > > Hi. > > > > I'm wondering why we need the flow limit on the datapath level at all? > > > > MAX_FLOWS constant was there from the introduction of dpif-netdev, > > however, later new flow-limit mechanism was implemented that > > controls number of datapath flows in a dynamic way on ofproto level. > > > > So, maybe we can just remove the limit and fully rely on ofproto > > to decide what flow limit we need? There are no limitations for > > flow table size in dpif-netdev beside the artificial one. > > 'other_config:flow-limit' seems suitable to control this. > > > > Ben, what do you think? > > Hmm, that's a good point. > > Tonghao, do you have a good reason to want to limit flows at this level? Hi Ben, as I explained, we don't use the ofproto layer, and use the ovs-appctl to install flow and offload them to hardware. so the "other_config:flow-limit" and "should_install_flow" function is called when upcall install the rules, and can't limit it in dpcls layer as I know. I am not sure installing rule via ovs-appctl is welcome. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-dev v3] dpif-netdev: Allow to set max capacity of flow on netdev.
On Thu, Jan 9, 2020 at 11:14 PM Ilya Maximets wrote: > > On 09.01.2020 08:36, xiangxia.m@gmail.com wrote: > > From: Tonghao Zhang > > > > For installing more than MAX_FLOWS (65536) flows to netdev datapath. > > Add the ovs-appctl subcommand "dpif-netdev/pmd-set-max-flow" which > > can change the flow number which netdev datapath support. > > > > Signed-off-by: Tonghao Zhang > > Hi. > > I'm wondering why we need the flow limit on the datapath level at all? > > MAX_FLOWS constant was there from the introduction of dpif-netdev, > however, later new flow-limit mechanism was implemented that > controls number of datapath flows in a dynamic way on ofproto level. > > So, maybe we can just remove the limit and fully rely on ofproto > to decide what flow limit we need? There are no limitations for > flow table size in dpif-netdev beside the artificial one. > 'other_config:flow-limit' seems suitable to control this. Hi all, that is good idea. But the reason that I change MAX_FLOWS to a var is that we install the rule via "ovs-appctl dpctl/add-flow" in our product environment, and there may be too many rules(software limit for hardware flow limit). ofproto layer is complicated for some developers. Using the "ovs-appctl dpctl/add-flow" may be easy to control the flows as I know. And we do some work on dpctl, for example, revalidator thread does not delete the flow we installed via ovs-appctl. > Ben, what do you think? > > Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] AWB Notification: You have A Package With Us
Greetings, FYI Your consignment have been booked via TNT EXPRESS is scheduled for delivery on January 14. But your shipment address is missing please kindly review the shipping details attached to confirm if every information is correct to enable us dispatch. Attached is the shipping document for clearing and reference purposes If you would like to find out about the many ways TNT helps you to track your shipment, or if you would like to know more about the services provided by TNT, simply connect to www.tnt.com and select your location at any time. --- This message and any attachment are confidential and may be privileged or otherwise protected from disclosure. If you are not the intended recipient, please telephone or email the sender and delete this message and any attachment from your system. If you are not the intended recipient you must not copy this message or attachment or disclose the contents to any other person. Please consider the environmental impact before printing this document and its attachment(s). Print black and white and double-sided where possible. -- ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows.
Bleep bloop. Greetings Ben Pfaff, 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 is 82 characters long (recommended limit is 79) #116 FILE: ovsdb/transaction.c:1: /* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017, 2019 Nicira, Inc. Lines checked: 240, 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
[ovs-dev] [PATCH repost 1/2] ofproto-dpif-upcall: Get rid of udpif_synchronize().
RCU provides the semantics we want from udpif_synchronize() and it
should be much more lightweight than killing and restarting all the
upcall threads. It looks like udpif_synchronize() was written before
the OVS tree had RCU support, which is probably why we didn't use it
here from the beginning. So we can just change udpif_synchronize()
to a single ovsrcu_synchronize() call.
However, udpif_synchronize() only has a single caller, which calls
ovsrcu_synchronize() anyway just beforehand, via xlate_txn_commit().
So we can get rid of udpif_synchronize() entirely, which this patch
does.
As a side effect, this eliminates one reason why terminating OVS cleanly
clears the datapath flow table. An upcoming patch will eliminate
other reasons.
Signed-off-by: Ben Pfaff
---
ofproto/ofproto-dpif-upcall.c | 17 -
ofproto/ofproto-dpif-upcall.h | 1 -
ofproto/ofproto-dpif-xlate.c | 14 +-
ofproto/ofproto-dpif.c| 4
4 files changed, 9 insertions(+), 27 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 409286ab1587..03cb8a1dd486 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -644,23 +644,6 @@ udpif_set_threads(struct udpif *udpif, size_t n_handlers_,
}
}
-/* Waits for all ongoing upcall translations to complete. This ensures that
- * there are no transient references to any removed ofprotos (or other
- * objects). In particular, this should be called after an ofproto is removed
- * (e.g. via xlate_remove_ofproto()) but before it is destroyed. */
-void
-udpif_synchronize(struct udpif *udpif)
-{
-/* This is stronger than necessary. It would be sufficient to ensure
- * (somehow) that each handler and revalidator thread had passed through
- * its main loop once. */
-size_t n_handlers_ = udpif->n_handlers;
-size_t n_revalidators_ = udpif->n_revalidators;
-
-udpif_stop_threads(udpif);
-udpif_start_threads(udpif, n_handlers_, n_revalidators_);
-}
-
/* Notifies 'udpif' that something changed which may render previous
* xlate_actions() results invalid. */
void
diff --git a/ofproto/ofproto-dpif-upcall.h b/ofproto/ofproto-dpif-upcall.h
index cef1d34198d6..693107ae56c1 100644
--- a/ofproto/ofproto-dpif-upcall.h
+++ b/ofproto/ofproto-dpif-upcall.h
@@ -33,7 +33,6 @@ struct udpif *udpif_create(struct dpif_backer *, struct dpif
*);
void udpif_run(struct udpif *udpif);
void udpif_set_threads(struct udpif *, size_t n_handlers,
size_t n_revalidators);
-void udpif_synchronize(struct udpif *);
void udpif_destroy(struct udpif *);
void udpif_revalidate(struct udpif *);
void udpif_get_memory_usage(struct udpif *, struct simap *usage);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4407f9c97a9e..0b45ecf3d4da 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1171,11 +1171,15 @@ xlate_xport_copy(struct xbridge *xbridge, struct
xbundle *xbundle,
*
* A sample workflow:
*
- * xlate_txn_start();
- * ...
- * edit_xlate_configuration();
- * ...
- * xlate_txn_commit(); */
+ * xlate_txn_start();
+ * ...
+ * edit_xlate_configuration();
+ * ...
+ * xlate_txn_commit();
+ *
+ * The ovsrcu_synchronize() call here also ensures that the upcall threads
+ * retain no references to anything in the previous configuration.
+ */
void
xlate_txn_commit(void)
{
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index d3cb392077df..0222ec82f00a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1751,10 +1751,6 @@ destruct(struct ofproto *ofproto_, bool del)
xlate_remove_ofproto(ofproto);
xlate_txn_commit();
-/* Ensure that the upcall processing threads have no remaining references
- * to the ofproto or anything in it. */
-udpif_synchronize(ofproto->backer->udpif);
-
hmap_remove(_ofproto_dpifs_by_name,
>all_ofproto_dpifs_by_name_node);
hmap_remove(_ofproto_dpifs_by_uuid,
--
2.23.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH repost 2/2] ofproto: Do not delete datapath flows on exit by default.
Commit e96a5c24e853 ("upcall: Remove datapath flows when setting
n-threads.") caused OVS to delete datapath flows when it exits through
any graceful means. This is not necessarily desirable, especially when
OVS is being stopped as part of an upgrade. This commit changes OVS so
that it only removes datapath flows when requested, via "ovs-appctl
exit --cleanup".
I've only tested this in the OVS sandbox.
Signed-off-by: Ben Pfaff
---
ofproto/ofproto-dpif-upcall.c | 26 --
ofproto/ofproto.c | 8
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 03cb8a1dd486..8dfa05b71df4 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -332,7 +332,7 @@ static size_t recv_upcalls(struct handler *);
static int process_upcall(struct udpif *, struct upcall *,
struct ofpbuf *odp_actions, struct flow_wildcards *);
static void handle_upcalls(struct udpif *, struct upcall *, size_t n_upcalls);
-static void udpif_stop_threads(struct udpif *);
+static void udpif_stop_threads(struct udpif *, bool delete_flows);
static void udpif_start_threads(struct udpif *, size_t n_handlers,
size_t n_revalidators);
static void udpif_pause_revalidators(struct udpif *);
@@ -483,7 +483,7 @@ udpif_run(struct udpif *udpif)
void
udpif_destroy(struct udpif *udpif)
{
-udpif_stop_threads(udpif);
+udpif_stop_threads(udpif, false);
dpif_register_dp_purge_cb(udpif->dpif, NULL, udpif);
dpif_register_upcall_cb(udpif->dpif, NULL, udpif);
@@ -504,9 +504,15 @@ udpif_destroy(struct udpif *udpif)
free(udpif);
}
-/* Stops the handler and revalidator threads. */
+/* Stops the handler and revalidator threads.
+ *
+ * If 'delete_flows' is true, we delete ukeys and delete all flows from the
+ * datapath. Otherwise, we end up double-counting stats for flows that remain
+ * in the datapath. If 'delete_flows' is false, we skip this step. This is
+ * appropriate if OVS is about to exit anyway and it is desirable to let
+ * existing network connections continue being forwarded afterward. */
static void
-udpif_stop_threads(struct udpif *udpif)
+udpif_stop_threads(struct udpif *udpif, bool delete_flows)
{
if (udpif && (udpif->n_handlers != 0 || udpif->n_revalidators != 0)) {
size_t i;
@@ -526,10 +532,10 @@ udpif_stop_threads(struct udpif *udpif)
dpif_disable_upcall(udpif->dpif);
ovsrcu_quiesce_end();
-/* Delete ukeys, and delete all flows from the datapath to prevent
- * double-counting stats. */
-for (i = 0; i < udpif->n_revalidators; i++) {
-revalidator_purge(>revalidators[i]);
+if (delete_flows) {
+for (i = 0; i < udpif->n_revalidators; i++) {
+revalidator_purge(>revalidators[i]);
+}
}
latch_poll(>exit_latch);
@@ -627,7 +633,7 @@ udpif_set_threads(struct udpif *udpif, size_t n_handlers_,
if (udpif->n_handlers != n_handlers_
|| udpif->n_revalidators != n_revalidators_) {
-udpif_stop_threads(udpif);
+udpif_stop_threads(udpif, true);
}
if (!udpif->handlers && !udpif->revalidators) {
@@ -681,7 +687,7 @@ udpif_flush(struct udpif *udpif)
size_t n_handlers_ = udpif->n_handlers;
size_t n_revalidators_ = udpif->n_revalidators;
-udpif_stop_threads(udpif);
+udpif_stop_threads(udpif, true);
dpif_flow_flush(udpif->dpif);
udpif_start_threads(udpif, n_handlers_, n_revalidators_);
}
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 08830d837164..5d69a4332f9f 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1601,13 +1601,13 @@ ofproto_rule_delete(struct ofproto *ofproto, struct
rule *rule)
}
static void
-ofproto_flush__(struct ofproto *ofproto)
+ofproto_flush__(struct ofproto *ofproto, bool del)
OVS_EXCLUDED(ofproto_mutex)
{
struct oftable *table;
/* This will flush all datapath flows. */
-if (ofproto->ofproto_class->flush) {
+if (del && ofproto->ofproto_class->flush) {
ofproto->ofproto_class->flush(ofproto);
}
@@ -1710,7 +1710,7 @@ ofproto_destroy(struct ofproto *p, bool del)
return;
}
-ofproto_flush__(p);
+ofproto_flush__(p, del);
HMAP_FOR_EACH_SAFE (ofport, next_ofport, hmap_node, >ports) {
ofport_destroy(ofport, del);
}
@@ -2288,7 +2288,7 @@ void
ofproto_flush_flows(struct ofproto *ofproto)
{
COVERAGE_INC(ofproto_flush);
-ofproto_flush__(ofproto);
+ofproto_flush__(ofproto, false);
connmgr_flushed(ofproto->connmgr);
}
--
2.23.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows.
Requested-by: Leonid Ryzhyk
Signed-off-by: Ben Pfaff
---
v1->v2: Improve test as suggested by Flavio Fernandes.
Documentation/ref/ovsdb-server.7.rst | 9 +
NEWS | 1 +
ovsdb/execution.c| 26 ++
ovsdb/transaction.c | 22 +-
ovsdb/transaction.h | 5 -
tests/ovsdb-execution.at | 24
tests/uuidfilt.py| 18 --
7 files changed, 97 insertions(+), 8 deletions(-)
diff --git a/Documentation/ref/ovsdb-server.7.rst
b/Documentation/ref/ovsdb-server.7.rst
index bc533611cb4a..967761bdfb84 100644
--- a/Documentation/ref/ovsdb-server.7.rst
+++ b/Documentation/ref/ovsdb-server.7.rst
@@ -546,6 +546,15 @@ condition can be either a 3-element JSON array as
described in the RFC or a
boolean value. In case of an empty array an implicit true boolean value will be
considered.
+5.2.1 Insert
+
+
+As an extension, Open vSwitch 2.13 and later allow an optional ``uuid`` member
+to specify the UUID for the new row. The specified UUID must be unique within
+the table when it is inserted and not the UUID of a row previously deleted
+within the transaction. If the UUID violates these rules, then the operation
+fails with a ``duplicate uuid`` error.
+
5.2.6 Wait, 5.2.7 Commit, 5.2.9 Comment
---
diff --git a/NEWS b/NEWS
index 965facaf852d..d8f82cd18af0 100644
--- a/NEWS
+++ b/NEWS
@@ -34,6 +34,7 @@ Post-v2.12.0
interval is increased to 60 seconds for the connection to the
replication server. This value is configurable with the unixctl
command - ovsdb-server/set-active-ovsdb-server-probe-interval.
+ * ovsdb-server: New OVSDB extension to allow clients to specify row UUIDs.
v2.12.0 - 03 Sep 2019
-
diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index f2cf3d72d45f..e45f3d6796a7 100644
--- a/ovsdb/execution.c
+++ b/ovsdb/execution.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017, 2019 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -329,11 +329,12 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct
ovsdb_parser *parser,
{
struct ovsdb_table *table;
struct ovsdb_row *row = NULL;
-const struct json *uuid_name, *row_json;
+const struct json *uuid_json, *uuid_name, *row_json;
struct ovsdb_error *error;
struct uuid row_uuid;
table = parse_table(x, parser, "table");
+uuid_json = ovsdb_parser_member(parser, "uuid", OP_STRING | OP_OPTIONAL);
uuid_name = ovsdb_parser_member(parser, "uuid-name", OP_ID | OP_OPTIONAL);
row_json = ovsdb_parser_member(parser, "row", OP_OBJECT);
error = ovsdb_parser_get_error(parser);
@@ -341,6 +342,19 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct
ovsdb_parser *parser,
return error;
}
+if (uuid_json) {
+if (!uuid_from_string(_uuid, json_string(uuid_json))) {
+return ovsdb_syntax_error(uuid_json, NULL, "bad uuid");
+}
+
+if (!ovsdb_txn_may_create_row(table, _uuid)) {
+return ovsdb_syntax_error(uuid_json, "duplicate uuid",
+ "This UUID would duplicate a UUID "
+ "already present within the table or "
+ "deleted within the same transaction.");
+}
+}
+
if (uuid_name) {
struct ovsdb_symbol *symbol;
@@ -350,9 +364,13 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct
ovsdb_parser *parser,
"This \"uuid-name\" appeared on an "
"earlier \"insert\" operation.");
}
-row_uuid = symbol->uuid;
+if (uuid_json) {
+symbol->uuid = row_uuid;
+} else {
+row_uuid = symbol->uuid;
+}
symbol->created = true;
-} else {
+} else if (!uuid_json) {
uuid_generate(_uuid);
}
diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index 866fabe8df57..369436bffbf5 100644
--- a/ovsdb/transaction.c
+++ b/ovsdb/transaction.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017, 2019 Nicira,
Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -1287,6 +1287,26 @@ ovsdb_txn_row_delete(struct ovsdb_txn *txn, const struct
ovsdb_row *row_)
}
}
+/* Returns true if 'row_uuid' may be used as the UUID for a newly created row
+ * in 'table' (that is, that it is unique within 'table'),
Re: [ovs-dev] [PATCH] [RFC] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows.
On Thu, Jan 09, 2020 at 02:21:18PM -0500, Flavio Fernandes wrote:
> This looks good to me, but I wonder if it would be a good idea to also
> add a test where we exercise the failure expected when trying to
> insert with a duplicate uuid. What do you think?
That is a good idea. We can do it within the same test. Here is an
incremental. I will post a v2.
diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at
index 10472d76fcbe..e72bf0606978 100644
--- a/tests/ovsdb-execution.at
+++ b/tests/ovsdb-execution.at
@@ -251,17 +251,26 @@ OVSDB_CHECK_EXECUTION([insert row, query table],
OVSDB_CHECK_EXECUTION([insert row with uuid, query table],
[ordinal_schema],
+dnl Insert initial row.
"ordinals",
{"op": "insert",
"uuid": "-971b-4cba-bf42-520515973b7e",
"table": "ordinals",
"row": {"number": 0, "name": "zero"}}]]],
+dnl Query row back.
[[["ordinals",
{"op": "select",
"table": "ordinals",
- "where": []},
+ "where": []}]]],
+dnl Attempt to insert second row with same UUID (fails).
+ [[["ordinals",
+ {"op": "insert",
+ "uuid": "-971b-4cba-bf42-520515973b7e",
+ "table": "ordinals",
+ "row": {"number": 0, "name": "zero"}},
[[[{"uuid":["uuid","-971b-4cba-bf42-520515973b7e"]}]
[{"rows":[{"_uuid":["uuid","-971b-4cba-bf42-520515973b7e"],"_version":["uuid","<0>"],"name":"zero","number":0}]}]
+[{"details":"This UUID would duplicate a UUID already present within the table
or deleted within the same transaction.","error":"duplicate
uuid","syntax":"\"-971b-4cba-bf42-520515973b7e\""}]
]])
OVSDB_CHECK_EXECUTION([insert rows, query by value],
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] [RFC] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows.
> On Aug 26, 2019, at 5:00 PM, Ben Pfaff wrote:
>
> Requested-by: Leonid Ryzhyk
> Signed-off-by: Ben Pfaff
> ---
> Documentation/ref/ovsdb-server.7.rst | 9 +
> NEWS | 3 ++-
> ovsdb/execution.c| 26 ++
> ovsdb/transaction.c | 22 +-
> ovsdb/transaction.h | 5 -
> tests/ovsdb-execution.at | 15 +++
> tests/uuidfilt.py| 18 --
> 7 files changed, 89 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/ref/ovsdb-server.7.rst
> b/Documentation/ref/ovsdb-server.7.rst
> index bc533611cb4a..967761bdfb84 100644
> --- a/Documentation/ref/ovsdb-server.7.rst
> +++ b/Documentation/ref/ovsdb-server.7.rst
> @@ -546,6 +546,15 @@ condition can be either a 3-element JSON array as
> described in the RFC or a
> boolean value. In case of an empty array an implicit true boolean value will
> be
> considered.
>
> +5.2.1 Insert
> +
> +
> +As an extension, Open vSwitch 2.13 and later allow an optional ``uuid``
> member
> +to specify the UUID for the new row. The specified UUID must be unique
> within
> +the table when it is inserted and not the UUID of a row previously deleted
> +within the transaction. If the UUID violates these rules, then the operation
> +fails with a ``duplicate uuid`` error.
> +
> 5.2.6 Wait, 5.2.7 Commit, 5.2.9 Comment
> ---
>
> diff --git a/NEWS b/NEWS
> index c5caa13d6374..7cda1e91a138 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,7 @@
> Post-v2.12.0
> -
> -
> + - ovsdb-server: New OVSDB extension to allow clients to specify row UUIDs.
> +
>
> v2.12.0 - xx xxx
> -
> diff --git a/ovsdb/execution.c b/ovsdb/execution.c
> index c55a0b771032..a8083da126dd 100644
> --- a/ovsdb/execution.c
> +++ b/ovsdb/execution.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017 Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017, 2019 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -329,11 +329,12 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct
> ovsdb_parser *parser,
> {
> struct ovsdb_table *table;
> struct ovsdb_row *row = NULL;
> -const struct json *uuid_name, *row_json;
> +const struct json *uuid_json, *uuid_name, *row_json;
> struct ovsdb_error *error;
> struct uuid row_uuid;
>
> table = parse_table(x, parser, "table");
> +uuid_json = ovsdb_parser_member(parser, "uuid", OP_STRING | OP_OPTIONAL);
> uuid_name = ovsdb_parser_member(parser, "uuid-name", OP_ID | OP_OPTIONAL);
> row_json = ovsdb_parser_member(parser, "row", OP_OBJECT);
> error = ovsdb_parser_get_error(parser);
> @@ -341,6 +342,19 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct
> ovsdb_parser *parser,
> return error;
> }
>
> +if (uuid_json) {
> +if (!uuid_from_string(_uuid, json_string(uuid_json))) {
> +return ovsdb_syntax_error(uuid_json, NULL, "bad uuid");
> +}
> +
> +if (!ovsdb_txn_may_create_row(table, _uuid)) {
> +return ovsdb_syntax_error(uuid_json, "duplicate uuid",
> + "This UUID would duplicate a UUID "
> + "already present within the table or "
> + "deleted within the same
> transaction.");
> +}
> +}
> +
> if (uuid_name) {
> struct ovsdb_symbol *symbol;
>
> @@ -350,9 +364,13 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct
> ovsdb_parser *parser,
> "This \"uuid-name\" appeared on an "
> "earlier \"insert\" operation.");
> }
> -row_uuid = symbol->uuid;
> +if (uuid_json) {
> +symbol->uuid = row_uuid;
> +} else {
> +row_uuid = symbol->uuid;
> +}
> symbol->created = true;
> -} else {
> +} else if (!uuid_json) {
> uuid_generate(_uuid);
> }
>
> diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
> index 866fabe8df57..369436bffbf5 100644
> --- a/ovsdb/transaction.c
> +++ b/ovsdb/transaction.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017 Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017, 2019
> Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -1287,6 +1287,26 @@ ovsdb_txn_row_delete(struct ovsdb_txn *txn, const
> struct ovsdb_row *row_)
> }
> }
>
> +/* Returns true if 'row_uuid' may be used as the UUID for a newly created row
> + * in 'table' (that is, that it is
[ovs-dev] CONTACT DIPLOMATIC MIKE BENZ TO RECEIVE YOUR ATM MASTER CARD WORTH $12.8MILLION US DOLLARS,
ATTN DEAR. CONTACT DIPLOMATIC MIKE BENZ TO RECEIVE YOUR ATM MASTER CARD WORTH $12.8MILLION US DOLLARS, CONTACT HIM ON THIS EMAIL ADDRESS EMAIL/ mikebenz...@aol.com PHONE (720) 928-6289 TEXT HIM AS YOU CONTACT HIM TODAY OK. HE IS WAITING TO HEAR FROM YOU NOW. WITH YOUR FULL DELIVERY ADDRESS SUCH AS YOUR LISTED BELOW. YOUR FULL NAME ADDRESS TELEPHONE NUMBERS I WAIT TO HEAR FROM YOU ONCE YOU RECEIVE YOUR ATM MASTER CARD WOTH $12.800,000MILLION US DOLLARS FROM MR. MIKE BENZ TODAY, NOTE YOU ARE JUST REQUIRED TO SEND HIM $25.00 ONCE YOU CONTACT HER FOR YOUR ATM MASTER CARD DELIVERY FEE. ASK HIM HOW YOU CAN SEND THE DELIVERY FEE OF $25.00 ONLY TO HIM IMMEDIATELY TO ENABLE HIM DELIVER TO YOUR HOUSE ADDRESS VERY URGENT TODAY OK. THIS IS ONLY HELP I CAN YOU TO RECEIVE YOUR FUNDS NOW OK. THANKS FOR YOUR COPPERATION. MS. MARYANNA B. THOMASON ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] netdev-dpdk: Avoid undefined behavior processing devargs
In "Use of library functions" in the C standard, the following statement
is written to apply to all library functions:
If an argument to a function has an invalid value (such as ... a
null pointer ... the behavior is undefined.
Later, under the "String handling" section, "Comparison functions" no
exception is listed for strcmp, which means NULL is invalid. It may
be possible for the smap_get to return NULL.
Given the above, we must check that new_devargs is not null. The check
against NULL for new_devargs later in the function is still valid.
Fixes: 55e075e65ef9 ("netdev-dpdk: Arbitrary 'dpdk' port naming")
Signed-off-by: Aaron Conole
---
NOTE: This is just code-review caught while looking elsewhere, and I
didn't test it.
lib/netdev-dpdk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 02120a379..00501f7d5 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1817,7 +1817,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const
struct smap *args,
new_devargs = smap_get(args, "dpdk-devargs");
-if (dev->devargs && strcmp(new_devargs, dev->devargs)) {
+if (dev->devargs && new_devargs && strcmp(new_devargs, dev->devargs)) {
/* The user requested a new device. If we return error, the caller
* will delete this netdev and try to recreate it. */
err = EAGAIN;
--
2.21.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev_afxdp: Detects combined channels and aborts wrong config
OK. I guess that "combined" is what I would would have called "duplex": going in both directions. If there's a need to configure this kind of thing, then I think that the documentation should summarize the kinds of channels and point to further documentation. On Wed, Jan 08, 2020 at 05:59:59PM -0800, Yi-Hung Wei wrote: > Hi Ben, > > After taking closer look at different driver implementations, I found > that the use of combined channels is actually driver dependent. > > From ethtool (8), a channel is an IRQ and the set of queues that can > trigger that IRQ. I tend to think channel as rx/tx queues from > software's point of view. There are 4 types of channels that ethtool > can configure, rx, tx, other, and combined. > > For Intel NICs, (at least for NICs that use ixgbe, i40 driver), we can > only configure channels through 'combined' parameter. It will set both > rx and tx channel to N with the following command. > $ ethool -L eth0 combined N > > However, it does not support configuring rx and tx channel separately. > We can only set/get number of combined channel, but not rx or tx > channel. > For example, > $ ethtool -l eth0 > Channel parameters for eth0: > Pre-set maximums: > RX:0 > TX:0 > Other:1 > Combined:63 > Current hardware settings: > RX:0 > TX:0 > Other:1 > Combined:8 > > On the other hand, NICs that use Broadcom tg3 drivers can configure > its rx and tx channel individually, but can not set with 'combined' > parameter. For Mellanox, mlx4 driver supports set/get rx/tx channels, > but mlx5 driver can only configured through combined channel. > > Another ethtool example with Broadcom NetXtreme BCM5720 NIC. > $ ethtool -L eno1 rx 2 tx 1 > $ ethtool -l eno1 > Channel parameters for eno1: > Pre-set maximums: > RX: 4 > TX: 4 > Other: 0 > Combined: 0 > Current hardware settings: > RX: 2 > TX: 1 > Other: 0 > Combined: 0 > > Back to this patch, what I was trying to do is to make sure that the > number of rx channels reported from ethtool is equal to n_rxq in > AF_XDP's port setting. Since, I was using a testbed with Intel NIC, I > assume to get # of rx channels from 'combined'. To support other > types of NICs, in the next version, I will first compare n_rxq with > 'rx'. In case of 'rx' is 0, 'combined' will be used. I will update > the related documentation as well. > > Thanks, > > -Yi-Hung > > > On Mon, Jan 6, 2020 at 2:15 PM Ben Pfaff wrote: > > > > On Mon, Dec 23, 2019 at 11:33:57AM -0800, Yi-Hung Wei wrote: > > > This patches detects the number of combined channels on a AF_XDP port > > > via using ethtool interface. If the number of combined channels > > > is different from the n_rxq, netdev_afxdp will not be able to get > > > all the packets from that port. Thus, abort the port setup when > > > the case is detected. > > > > > > Signed-off-by: Yi-Hung Wei > > > --- > > > Travis CI: https://travis-ci.org/YiHungWei/ovs/builds/627972465 > > > > I don't know what a combined channel is. Should the documentation talk > > about this? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] odp-util: Fix passing uninitialized bytes in OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV*.
On Thu, Jan 09, 2020 at 05:27:36PM +0100, Ilya Maximets wrote: > Both ovs_key_ct_tuple_ipv* structures contains padding at the end > that mast be cleared before passing attributes to kernel: Good catch. I think that you could save a line for the memset() call by using nl_msg_put_unspec_zero(). Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-dev v3] dpif-netdev: Allow to set max capacity of flow on netdev.
On Thu, Jan 09, 2020 at 04:14:10PM +0100, Ilya Maximets wrote: > On 09.01.2020 08:36, xiangxia.m@gmail.com wrote: > > From: Tonghao Zhang > > > > For installing more than MAX_FLOWS (65536) flows to netdev datapath. > > Add the ovs-appctl subcommand "dpif-netdev/pmd-set-max-flow" which > > can change the flow number which netdev datapath support. > > > > Signed-off-by: Tonghao Zhang > > Hi. > > I'm wondering why we need the flow limit on the datapath level at all? > > MAX_FLOWS constant was there from the introduction of dpif-netdev, > however, later new flow-limit mechanism was implemented that > controls number of datapath flows in a dynamic way on ofproto level. > > So, maybe we can just remove the limit and fully rely on ofproto > to decide what flow limit we need? There are no limitations for > flow table size in dpif-netdev beside the artificial one. > 'other_config:flow-limit' seems suitable to control this. > > Ben, what do you think? Hmm, that's a good point. Tonghao, do you have a good reason to want to limit flows at this level? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] 答复: 答复: 答复: [PATCH] socket-util: Introduce emulation and wrapper for recvmmsg().
OK. I applied both patches to master. Thank you! On Wed, Jan 08, 2020 at 12:27:31AM +, Yi Yang (杨燚)-云服务集团 wrote: > Ben, I think the patch using recvmmsg is ready for merge if you want, > basically 4.15 or later kernels can support TPACKET_V3, I'm not sure if > recvmmsg and TPACKET_V3 can coexist, do you mean we can use config HAVE_ > TPACKET_V3/2 to build different version for different kernel? > > -邮件原件- > 发件人: Ben Pfaff [mailto:b...@ovn.org] > 发送时间: 2020年1月8日 4:11 > 收件人: Yi Yang (杨燚)-云服务集团 > 抄送: d...@openvswitch.org > 主题: Re: 答复: 答复: [PATCH] socket-util: Introduce emulation and wrapper for > recvmmsg(). > > On Mon, Dec 23, 2019 at 12:22:52AM +, Yi Yang (杨燚)-云服务集团 wrote: > > Ben, socket.h in master does include sendmmsg > > > > https://github.com/openvswitch/ovs/blob/master/include/sparse/sys/sock > > et.h#L165 > > > > Per your explanation, I understood why you call recvmsg there, so I don't > > have other comments. > > > > As William explained in his RFC patch, I think TPACKET_V3 is the best way > > to fix this. I tried af_packet to use veth in OVS DPDK, it's performance is > > 2 times more than my patch, about 4Gbps, for my patch, veth performance is > > about 1.47Gbps, af_packet just used TPACKET_V2, TPACKET_V3 should be much > > better than TPACKET_V2 per William's explanation. > > OK. Do you want to continue working to use recvmmsg() in OVS? Or do you > want to withdraw the idea in favor of TPACKET_V3? The possible advantage of > recvmmsg() is that it's going to be available pretty much everywhere, whereas > TPACKET_V3 is a more recent addition to Linux. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn 2/2] ovn-controller: Cache logical flow expr tree for each lflow.
From: Numan Siddique
This patch caches the logical flow expr tree for each logical flow. This
cache is stored as an hashmap in the output_flow engine data. When a logical
flow is deleted, the expr tree for that lflow is deleted in the
flow_output_sb_logical_flow_handler().
Below are the details of the OVN resource in my setup
No of logical switches - 49
No of logical ports- 1191
No of logical routers - 7
No of logical ports- 51
No of ACLs - 1221
No of Logical flows- 664736
On a chassis hosting 7 distributed router ports and around 1090
port bindings, the no of OVS rules was 921162.
Without this patch, the function add_logical_flows() took ~15 seconds.
And with this patch it took ~10 seconds. There is a good 34% improvement
in logical flow processing.
Signed-off-by: Numan Siddique
---
controller/lflow.c | 182 ++--
controller/lflow.h | 9 +-
controller/ovn-controller.c | 17 +++-
3 files changed, 154 insertions(+), 54 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index 93ec53a1c..be46f0661 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -79,7 +79,9 @@ static bool consider_logical_flow(
struct ovn_extend_table *group_table,
struct ovn_extend_table *meter_table,
struct lflow_resource_ref *lfrr,
-uint32_t *conj_id_ofs);
+uint32_t *conj_id_ofs,
+struct hmap *lflow_expr_cache,
+bool delete_expr_from_cache);
static bool
lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
@@ -255,6 +257,59 @@ lflow_resource_destroy_lflow(struct lflow_resource_ref
*lfrr,
free(lfrn);
}
+struct lflow_expr {
+struct hmap_node node;
+struct uuid lflow_uuid; /* key */
+struct expr *expr;
+char *lflow_match;
+};
+
+static void
+lflow_expr_add(struct hmap *lflow_expr_cache,
+ const struct sbrec_logical_flow *lflow,
+ struct expr *lflow_expr)
+{
+struct lflow_expr *le = xmalloc(sizeof *le);
+le->lflow_uuid = lflow->header_.uuid;
+le->expr = lflow_expr;
+le->lflow_match = xstrdup(lflow->match);
+hmap_insert(lflow_expr_cache, >node, uuid_hash(>lflow_uuid));
+}
+
+static struct lflow_expr *
+lflow_expr_get(struct hmap *lflow_expr_cache,
+ const struct sbrec_logical_flow *lflow)
+{
+struct lflow_expr *le;
+size_t hash = uuid_hash(>header_.uuid);
+HMAP_FOR_EACH_WITH_HASH (le, node, hash, lflow_expr_cache) {
+if (!strcmp(lflow->match, le->lflow_match)) {
+return le;
+}
+}
+
+return NULL;
+}
+
+static void
+lflow_expr_delete(struct hmap *lflow_expr_cache, struct lflow_expr *le)
+{
+hmap_remove(lflow_expr_cache, >node);
+free(le->lflow_match);
+expr_destroy(le->expr);
+free(le);
+}
+
+void
+lflow_expr_destroy(struct hmap *lflow_expr_cache)
+{
+struct lflow_expr *le;
+HMAP_FOR_EACH_POP (le, node, lflow_expr_cache) {
+free(le->lflow_match);
+free(le);
+}
+}
+
/* Adds the logical flows from the Logical_Flow table to flow tables. */
static void
add_logical_flows(
@@ -273,7 +328,8 @@ add_logical_flows(
struct ovn_extend_table *group_table,
struct ovn_extend_table *meter_table,
struct lflow_resource_ref *lfrr,
-uint32_t *conj_id_ofs)
+uint32_t *conj_id_ofs,
+struct hmap *lflow_expr_cache)
{
const struct sbrec_logical_flow *lflow;
@@ -308,7 +364,8 @@ add_logical_flows(
addr_sets, port_groups,
active_tunnels, local_lport_ids,
flow_table, group_table, meter_table,
- lfrr, conj_id_ofs)) {
+ lfrr, conj_id_ofs, lflow_expr_cache,
+ false)) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
VLOG_ERR_RL(, "Conjunction id overflow when processing lflow "
UUID_FMT, UUID_ARGS(>header_.uuid));
@@ -338,7 +395,8 @@ lflow_handle_changed_flows(
struct ovn_extend_table *group_table,
struct ovn_extend_table *meter_table,
struct lflow_resource_ref *lfrr,
-uint32_t *conj_id_ofs)
+uint32_t *conj_id_ofs,
+struct hmap *lflow_expr_cache)
{
bool ret = true;
const struct sbrec_logical_flow *lflow;
@@ -373,6 +431,12 @@ lflow_handle_changed_flows(
ofctrl_remove_flows(flow_table, >header_.uuid);
/* Delete entries from lflow resource reference. */
lflow_resource_destroy_lflow(lfrr, >header_.uuid);
+
+/* Delete the expr cache for this lflow. */
+struct lflow_expr *le = lflow_expr_get(lflow_expr_cache, lflow);
+if (le) {
+lflow_expr_delete(lflow_expr_cache, le);
+}
}
}
@@ -401,7 +465,8 @@ lflow_handle_changed_flows(
addr_sets,
[ovs-dev] [PATCH ovn 1/2] expr: Evaluate the condition expression in a separate step.
From: Numan Siddique
A new function is added - expr_evaluate_condition() which evaluates
the condition expression - is_chassis_resident. expr_simply() will
no longer evaluates this condition.
An upcoming commit needs this in order to cache the logical flow expressions
so that every lflow_*() function which calls consider_logical_flow() doesn't
need to convert the logical flow match to an expression. Instead the expr tree
for the logical flow is cached. Since we can't cache the is_chassis_resident
condition, it is separated out.
Signed-off-by: Numan Siddique
---
controller/lflow.c| 3 ++-
include/ovn/expr.h| 10
lib/expr.c| 55 +--
tests/test-ovn.c | 10
utilities/ovn-trace.c | 5 +++-
5 files changed, 60 insertions(+), 23 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index a6893201e..93ec53a1c 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -661,8 +661,9 @@ consider_logical_flow(
.chassis = chassis,
.active_tunnels = active_tunnels,
};
-expr = expr_simplify(expr, is_chassis_resident_cb, _aux);
+expr = expr_simplify(expr);
expr = expr_normalize(expr);
+expr = expr_evaluate_condition(expr, is_chassis_resident_cb, _aux);
uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, ,
);
expr_destroy(expr);
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index 21bf51c22..00a021236 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -404,10 +404,12 @@ void expr_destroy(struct expr *);
struct expr *expr_annotate(struct expr *, const struct shash *symtab,
char **errorp);
-struct expr *expr_simplify(struct expr *,
- bool (*is_chassis_resident)(const void *c_aux,
- const char *port_name),
- const void *c_aux);
+struct expr *expr_simplify(struct expr *);
+struct expr *expr_evaluate_condition(
+struct expr *,
+bool (*is_chassis_resident)(const void *c_aux,
+const char *port_name),
+const void *c_aux);
struct expr *expr_normalize(struct expr *);
bool expr_honors_invariants(const struct expr *);
diff --git a/lib/expr.c b/lib/expr.c
index e5e4d21b7..12557e3ca 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -2033,10 +2033,10 @@ expr_simplify_relational(struct expr *expr)
/* Resolves condition and replaces the expression with a boolean. */
static struct expr *
-expr_simplify_condition(struct expr *expr,
-bool (*is_chassis_resident)(const void *c_aux,
+expr_evaluate_condition__(struct expr *expr,
+ bool (*is_chassis_resident)(const void *c_aux,
const char *port_name),
-const void *c_aux)
+ const void *c_aux)
{
bool result;
@@ -2054,13 +2054,41 @@ expr_simplify_condition(struct expr *expr,
return expr_create_boolean(result);
}
+struct expr *
+expr_evaluate_condition(struct expr *expr,
+bool (*is_chassis_resident)(const void *c_aux,
+const char *port_name),
+const void *c_aux)
+{
+struct expr *sub, *next;
+
+switch (expr->type) {
+case EXPR_T_AND:
+case EXPR_T_OR:
+ LIST_FOR_EACH_SAFE (sub, next, node, >andor) {
+ovs_list_remove(>node);
+struct expr *e = expr_evaluate_condition(sub, is_chassis_resident,
+ c_aux);
+e = expr_fix(e);
+expr_insert_andor(expr, next, e);
+}
+return expr_fix(expr);
+
+case EXPR_T_CONDITION:
+return expr_evaluate_condition__(expr, is_chassis_resident, c_aux);
+
+case EXPR_T_CMP:
+case EXPR_T_BOOLEAN:
+return expr;
+}
+
+OVS_NOT_REACHED();
+}
+
/* Takes ownership of 'expr' and returns an equivalent expression whose
* EXPR_T_CMP nodes use only tests for equality (EXPR_R_EQ). */
struct expr *
-expr_simplify(struct expr *expr,
- bool (*is_chassis_resident)(const void *c_aux,
- const char *port_name),
- const void *c_aux)
+expr_simplify(struct expr *expr)
{
struct expr *sub, *next;
@@ -2075,8 +2103,7 @@ expr_simplify(struct expr *expr,
case EXPR_T_OR:
LIST_FOR_EACH_SAFE (sub, next, node, >andor) {
ovs_list_remove(>node);
-expr_insert_andor(expr, next,
- expr_simplify(sub, is_chassis_resident, c_aux));
+expr_insert_andor(expr, next, expr_simplify(sub));
}
return expr_fix(expr);
@@ -2084,7 +2111,7 @@ expr_simplify(struct expr *expr,
return expr;
case EXPR_T_CONDITION:
-
[ovs-dev] [PATCH ovn 0/2] Caching logical flow expr tree for each lflow
From: Numan Siddique This patch series tries to improve the time taken to proceess logical flows by caching the expr tree. For large scale deployments with lots of logical flows, the logical flow processing to OpenFlow rules takes a lot of time as it is CPU intensive. This patch series tries to improve this by caching the expr tree generated for each logical flow so that we don't have to generate the expr tree for each lflow_run(). Below are the details of the OVN resource in my setup No of logical switches - 49 No of logical ports- 1191 No of logical routers - 7 No of logical ports- 51 No of ACLs - 1221 No of Logical flows- 664736 On a chassis hosting 7 distributed router ports and around 1090 port bindings, the no of OVS rules was 921162. Without this patch, the function add_logical_flows() took ~15 seconds. And with this patch it took ~10 seconds. There is a good 34% improvement in logical flow processing. Numan Siddique (2): expr: Evaluate the condition expression in a separate step. ovn-controller: Cache logical flow expr tree for each lflow. controller/lflow.c | 181 +++- controller/lflow.h | 9 +- controller/ovn-controller.c | 17 +++- include/ovn/expr.h | 10 +- lib/expr.c | 55 --- tests/test-ovn.c| 10 +- utilities/ovn-trace.c | 5 +- 7 files changed, 212 insertions(+), 75 deletions(-) -- 2.24.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] how to use ovn-scale-test when i do not use git repo
It indicates that db duplicate entry exists already. Either try delete previous
deployment and run create deployment create again or create a new deployment
with new name.
Please also check the installation steps/guide
https://ovn-scale-test.readthedocs.io/en/latest/install.html#install-rally if
you want to run sandbox deployment including steps that Dumitru suggested for
multi host physical. Working with rally-ovs requires familiarity with OpenStack
too. Hence, you might want to check that for trouble shooting further.
From: "shenjian...@gmail.com"
Date: Wednesday, January 8, 2020 at 7:39 PM
To: Dumitru Ceara
Cc: "Ginwala, Aliasgar" , "ovs-dev@openvswitch.org"
Subject: Re: [ovs-dev] how to use ovn-scale-test when i do not use git repo
hi，
when I exec command “rally-ovs deployment recreate
/home/sj/ovn-scale-test/samples/deployments/ovn-multihost-sandbox.json
ovn-multihost” get some error，
my json is：
{
{
"type": "OvnMultihostEngine",
"controller": {
"type": "OvnSandboxControllerEngine",
"deployment_name": "ovn-controller-node",
"ovs_repo": "root@node0:/home/git/test.git",
"ovs_branch": "master",
"ovs_user": "root",
"net_dev": "eno1",
"controller_cidr": "192.168.10.10/16",
"provider": {
"type": "OvsSandboxProvider",
"credentials": [
{
"host": "192.168.20.10",
"user": "root"}
]
}
},
"nodes": [
{
"type": "OvnSandboxFarmEngine",
"deployment_name": "ovn-farm-node-0",
"ovs_repo" : "root@node0:/home/git/test.git",
"ovs_branch" : "master",
"ovs_user" : "root",
"provider": {
"type": "OvsSandboxProvider",
"credentials": [
{
"host": "192.168.20.20",
"user": "root"}
]
}
}
]
}
error message：
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine [-] Deployment
adb85ae1-4451-43db-8693-33c3809973a7: Error has occurred into context of the
deployment
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine Traceback (most
recent call last):
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine File
"/usr/lib/python2.7/site-packages/rally/api.py", line 102, in recreate
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine credentials =
deployer.make_deploy()
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine File
"/usr/lib/python2.7/site-packages/rally/common/logging.py", line 197, in wrapper
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine result = f(self,
*args, **kwargs)
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine File
"/usr/lib/python2.7/site-packages/rally/deployment/engine.py", line 117, in
make_deploy
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine credentials =
self.deploy()
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine File
"/usr/lib/python2.7/site-packages/rally_ovs/plugins/ovs/deployment/engines/ovn_multihost.py",
line 57, in deploy
...
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine File
"/usr/lib64/python2.7/site-packages/sqlalchemy/engine/base.py", line 1139, in
_execute_context
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine context)
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine File
"/usr/lib64/python2.7/site-packages/sqlalchemy/engine/default.py", line 450, in
do_execute
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine
cursor.execute(statement, parameters)
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine DBDuplicateEntry:
(sqlite3.IntegrityError) column name is not unique [SQL: u'UPDATE deployments
SET updated_at=?, name=? WHERE deployments.id = ?'] [parameters: ('2020-01-09
03:25:39.300884', u'ovn-controller-node', 4)]
2020-01-09 11:25:39.301 39837 ERROR rally.deployment.engine
Command failed, please check log for more info
ps：does install_method="sandbox" must use git repo？
---Original---
From: "Dumitru Ceara"
Date: Tue, Jan 7, 2020 16:25 PM
To: "shenjian...@gmail.com";
Cc: "Ginwala,
Aliasgar";"ovs-dev@openvswitch.org";
Subject: Re: [ovs-dev] how to use ovn-scale-test when i do not use git repo
On Tue, Jan 7, 2020 at 5:31 AM Ginwala, Aliasgar via dev
wrote:
>
> Sure:
>
> You are refer to
> https://ovn-scale-test.readthedocs.io/en/latest/tutorial/step_1_setting_up_the_environment.html
> which is in doc folder of ovn-scale-test repo including the links in README.
>
>
>
>
> From: "shenjian...@gmail.com"
> Date: Monday, January 6, 2020 at 6:28 PM
> To: "Ginwala, Aliasgar" , "ovs-dev@openvswitch.org"
> Subject: how to use ovn-scale-test when i do not use git repo
>
> hi，aginwala：
>when I use ovn-multihost-physical.json to
> deployment ，it allways fail. Do you have detailed manual for ovn-scale-test？
If you use
Re: [ovs-dev] [PATCH ovn v2] Restrict ARP/IPv6 ND replies for LB VIP only on chassis redirect port
On Thu, Jan 9, 2020 at 7:35 PM Dumitru Ceara wrote:
>
> On Thu, Jan 9, 2020 at 1:18 PM wrote:
> >
> > From: Numan Siddique
> >
> > Presently when ARP/ND request for the load balance VIP is received
> > from the provider network, all the ovn-controllers' reply to the ARP/ND
> > request which have ovn-bridge-mappings configured.
> >
> > This patch restricts these ARP/ND replies only on the chassis where the
> > chassis redirect port of the distributed router port is resident.
> >
> > Signed-off-by: Numan Siddique
>
> Acked-by: Dumitru Ceara
Thanks Dumitru. I applied this patch to master.
Numan
>
> Thanks,
> Dumitru
>
> > ---
> > v1 -> v2
> > -
> > * Added the lflow for IPv6 LB VIPs.
> >
> > northd/ovn-northd.8.xml | 14 ++
> > northd/ovn-northd.c | 8
> > 2 files changed, 22 insertions(+)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index c6d5d96b9..4b227ca71 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -1680,6 +1680,13 @@ flags.loopback = 1;
> > output;
> >
> >
> > +
> > + If the router port P is a distributed gateway router
> > + port, then the is_chassis_resident(P) is
> > + also added in the match condition for the load balancer IPv4
> > + VIP A.
> > +
> > +
> >
> >IPv6: For a configured DNAT IP address or a load balancer
> >IPv6 VIP A, solicited node address S,
> > @@ -1704,6 +1711,13 @@ nd_na {
> > }
> >
> >
> > +
> > + If the router port P is a distributed gateway router
> > + port, then the is_chassis_resident(P)
> > + is also added in the match condition for the load balancer IPv6
> > + VIP A.
> > +
> > +
> >
> >For the gateway port on a distributed logical router with NAT
> >(where one of the logical router ports specifies a
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index d91a008b7..b6dc809d7 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -7511,6 +7511,10 @@ build_lrouter_flows(struct hmap *datapaths, struct
> > hmap *ports,
> >"inport == %s && arp.tpa == %s && arp.op == 1",
> >op->json_key, ip_address);
> >
> > +if (op == op->od->l3dgw_port) {
> > +ds_put_format(, " && is_chassis_resident(%s)",
> > + op->od->l3redirect_port->json_key);
> > +}
> > ds_clear();
> > ds_put_format(,
> >"eth.dst = eth.src; "
> > @@ -7538,6 +7542,10 @@ build_lrouter_flows(struct hmap *datapaths, struct
> > hmap *ports,
> >"inport == %s && nd_ns && nd.target == %s",
> >op->json_key, ip_address);
> >
> > +if (op == op->od->l3dgw_port) {
> > +ds_put_format(, " && is_chassis_resident(%s)",
> > + op->od->l3redirect_port->json_key);
> > +}
> > ds_clear();
> > ds_put_format(,
> >"nd_na { "
> > --
> > 2.24.1
> >
> > ___
> > 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
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] odp-util: Fix passing uninitialized bytes in OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV*.
Both ovs_key_ct_tuple_ipv* structures contains padding at the end
that mast be cleared before passing attributes to kernel:
Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
at 0x566A607: sendmsg (sendmsg.c:28)
by 0xFC95CE: nl_sock_transact_multiple__ (netlink-socket.c:858)
by 0xFC8580: nl_sock_transact_multiple (netlink-socket.c:1079)
by 0xFC83FF: nl_transact_multiple (netlink-socket.c:1839)
by 0xFA8648: dpif_netlink_operate__ (dpif-netlink.c:1926)
by 0xFA789F: dpif_netlink_operate_chunks (dpif-netlink.c:2219)
by 0xFA25CB: dpif_netlink_operate (dpif-netlink.c:2278)
by 0xE5BB4C: dpif_operate (dpif.c:1377)
by 0xE5B7F6: dpif_flow_put (dpif.c:1048)
by 0xE5B49A: dpif_probe_feature (dpif.c:965)
by 0xDD6BF5: check_ct_orig_tuple (ofproto-dpif.c:1557)
by 0xDD41EC: check_support (ofproto-dpif.c:1590)
by 0xDD3BF3: open_dpif_backer (ofproto-dpif.c:818)
by 0xDC8467: construct (ofproto-dpif.c:1605)
by 0xDAD6BB: ofproto_create (ofproto.c:549)
by 0xD96A19: bridge_reconfigure (bridge.c:877)
by 0xD9625D: bridge_run (bridge.c:3324)
by 0xDA5829: main (ovs-vswitchd.c:127)
Address 0x1ffefe36a5 is on thread 1's stack
in frame #4, created by dpif_netlink_operate__ (dpif-netlink.c:1839)
Uninitialised value was created by a stack allocation
at 0xEB87D0: odp_flow_key_from_flow__ (odp-util.c:5996)
Fixes: daf4d3c18da4 ("odp: Support conntrack orig tuple key.")
Signed-off-by: Ilya Maximets
---
lib/odp-util.c | 40 ++--
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 122572415..105b01a88 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -6032,26 +6032,30 @@ odp_flow_key_from_flow__(const struct
odp_flow_key_parms *parms,
if (flow->ct_nw_proto) {
if (parms->support.ct_orig_tuple
&& flow->dl_type == htons(ETH_TYPE_IP)) {
-struct ovs_key_ct_tuple_ipv4 ct = {
-data->ct_nw_src,
-data->ct_nw_dst,
-data->ct_tp_src,
-data->ct_tp_dst,
-data->ct_nw_proto,
-};
-nl_msg_put_unspec(buf, OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4, ,
- sizeof ct);
+struct ovs_key_ct_tuple_ipv4 *ct;
+
+ct = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4,
+ sizeof *ct);
+/* 'struct ovs_key_ct_tuple_ipv4' has padding, clear it. */
+memset(ct, 0, sizeof *ct);
+ct->ipv4_src = data->ct_nw_src;
+ct->ipv4_dst = data->ct_nw_dst;
+ct->src_port = data->ct_tp_src;
+ct->dst_port = data->ct_tp_dst;
+ct->ipv4_proto = data->ct_nw_proto;
} else if (parms->support.ct_orig_tuple6
&& flow->dl_type == htons(ETH_TYPE_IPV6)) {
-struct ovs_key_ct_tuple_ipv6 ct = {
-data->ct_ipv6_src,
-data->ct_ipv6_dst,
-data->ct_tp_src,
-data->ct_tp_dst,
-data->ct_nw_proto,
-};
-nl_msg_put_unspec(buf, OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6, ,
- sizeof ct);
+struct ovs_key_ct_tuple_ipv6 *ct;
+
+ct = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6,
+ sizeof *ct);
+/* 'struct ovs_key_ct_tuple_ipv6' has padding, clear it. */
+memset(ct, 0, sizeof *ct);
+ct->ipv6_src = data->ct_ipv6_src;
+ct->ipv6_dst = data->ct_ipv6_dst;
+ct->src_port = data->ct_tp_src;
+ct->dst_port = data->ct_tp_dst;
+ct->ipv6_proto = data->ct_nw_proto;
}
}
if (parms->support.recirc) {
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-dev v3] dpif-netdev: Allow to set max capacity of flow on netdev.
On 09.01.2020 08:36, xiangxia.m@gmail.com wrote: > From: Tonghao Zhang > > For installing more than MAX_FLOWS (65536) flows to netdev datapath. > Add the ovs-appctl subcommand "dpif-netdev/pmd-set-max-flow" which > can change the flow number which netdev datapath support. > > Signed-off-by: Tonghao Zhang Hi. I'm wondering why we need the flow limit on the datapath level at all? MAX_FLOWS constant was there from the introduction of dpif-netdev, however, later new flow-limit mechanism was implemented that controls number of datapath flows in a dynamic way on ofproto level. So, maybe we can just remove the limit and fully rely on ofproto to decide what flow limit we need? There are no limitations for flow table size in dpif-netdev beside the artificial one. 'other_config:flow-limit' seems suitable to control this. Ben, what do you think? Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 3/3] netdev-dpdk: Add TCP Segmentation Offload support
Abbreviated as TSO, TCP Segmentation Offload is a feature which enables the network stack to delegate the TCP segmentation to the NIC reducing the per packet CPU overhead. A guest using vhostuser interface with TSO enabled can send TCP packets much bigger than the MTU, which saves CPU cycles normally used to break the packets down to MTU size and to calculate checksums. It also saves CPU cycles used to parse multiple packets/headers during the packet processing inside virtual switch. If the destination of the packet is another guest in the same host, then the same big packet can be sent through a vhostuser interface skipping the segmentation completely. However, if the destination is not local, the NIC hardware is instructed to do the TCP segmentation and checksum calculation. It is recommended to check if NIC hardware supports TSO before enabling the feature, which is off by default. For additional information please check the tso.rst document. Signed-off-by: Flavio Leitner --- Documentation/automake.mk | 1 + Documentation/topics/dpdk/index.rst | 1 + Documentation/topics/dpdk/tso.rst | 96 + NEWS| 1 + lib/automake.mk | 2 + lib/conntrack.c | 29 ++- lib/dp-packet.h | 152 +- lib/ipf.c | 32 +-- lib/netdev-dpdk.c | 312 lib/netdev-linux-private.h | 4 + lib/netdev-linux.c | 296 +++--- lib/netdev-provider.h | 10 + lib/netdev.c| 66 +- lib/tso.c | 54 + lib/tso.h | 23 ++ vswitchd/bridge.c | 2 + vswitchd/vswitch.xml| 12 ++ 17 files changed, 1002 insertions(+), 91 deletions(-) create mode 100644 Documentation/topics/dpdk/tso.rst create mode 100644 lib/tso.c create mode 100644 lib/tso.h Changelog: - v3 * Improved the documentation. * Updated copyright year to 2020. * TSO offloaded msg now includes the netdev's name. * Added period at the end of all code comments. * Warn and drop encapsulation of TSO packets. * Fixed travis issue with restricted virtio types. * Fixed double headroom allocation in dpdk_copy_dp_packet_to_mbuf() which caused packet corruption. * Fixed netdev_dpdk_prep_hwol_packet() to unconditionally set PKT_TX_IP_CKSUM only for IPv4 packets. diff --git a/Documentation/automake.mk b/Documentation/automake.mk index f2ca17bad..284327edd 100644 --- a/Documentation/automake.mk +++ b/Documentation/automake.mk @@ -35,6 +35,7 @@ DOC_SOURCE = \ Documentation/topics/dpdk/index.rst \ Documentation/topics/dpdk/bridge.rst \ Documentation/topics/dpdk/jumbo-frames.rst \ + Documentation/topics/dpdk/tso.rst \ Documentation/topics/dpdk/memory.rst \ Documentation/topics/dpdk/pdump.rst \ Documentation/topics/dpdk/phy.rst \ diff --git a/Documentation/topics/dpdk/index.rst b/Documentation/topics/dpdk/index.rst index f2862ea70..400d56051 100644 --- a/Documentation/topics/dpdk/index.rst +++ b/Documentation/topics/dpdk/index.rst @@ -40,4 +40,5 @@ DPDK Support /topics/dpdk/qos /topics/dpdk/pdump /topics/dpdk/jumbo-frames + /topics/dpdk/tso /topics/dpdk/memory diff --git a/Documentation/topics/dpdk/tso.rst b/Documentation/topics/dpdk/tso.rst new file mode 100644 index 0..189c86480 --- /dev/null +++ b/Documentation/topics/dpdk/tso.rst @@ -0,0 +1,96 @@ +.. + Copyright 2020, Red Hat, Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); you may + not use this file except in compliance with the License. You may obtain + a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + License for the specific language governing permissions and limitations + under the License. + + Convention for heading levels in Open vSwitch documentation: + + === Heading 0 (reserved for the title in a document) + --- Heading 1 + ~~~ Heading 2 + +++ Heading 3 + ''' Heading 4 + + Avoid deeper levels because they do not render well. + + +Userspace Datapath - TSO + + +**Note:** This feature is considered experimental. + +TCP Segmentation Offload (TSO) enables a network stack to delegate segmentation +of an oversized TCP segment to the underlying physical NIC. Offload of frame +segmentation achieves computational savings in the core, freeing up CPU cycles +for more useful work. + +A common use case for TSO is when using virtualization, where
[ovs-dev] [PATCH v3 0/3] Add support for TSO with DPDK
Abbreviated as TSO, TCP Segmentation Offload is a feature which enables the network stack to delegate the TCP segmentation to the NIC reducing the per packet CPU overhead. A guest using vhost-user interface with TSO enabled can send TCP packets much bigger than the MTU, which saves CPU cycles normally used to break the packets down to MTU size and to calculate checksums. It also saves CPU cycles used to parse multiple packets/headers during the packet processing inside virtual switch. If the destination of the packet is another guest in the same host, then the same big packet can be sent through a vhost-user interface skipping the segmentation completely. However, if the destination is not local, the NIC hardware is instructed to do the TCP segmentation and checksum calculation. The first 2 patches are not really part of TSO support, but they are required to make sure everything works. There are good improvements sending to or receiving from veth pairs or tap devices as well. See the iperf3 results below: [*] veth with ethtool tx off. VM sending to: Default Enabled Local BR 859 Mbits/sec 9.23 Gbits/sec Net NS (veth) 965 Mbits/sec[*] 9.74 Gbits/sec VM (same host)2.54 Gbits/sec 22.4 Gbits/sec Ext Host 10.3 Gbits/sec 35.0 Gbits/sec Ext Host (vxlan) 8.77 Gbits/sec (not supported) Using VLAN: Local BR 877 Mbits/sec 9.49 Gbits/sec VM (same host)2.35 Gbits/sec 23.3 Gbits/sec Ext Host 5.84 Gbits/sec 34.6 Gbits/sec Using IPv6: Net NS (veth) 937 Mbits/sec[*] 9.32 Gbits/sec VM (same host)2.53 Gbits/sec 21.1 Gbits/sec Ext Host 8.66 Gbits/sec 37.7 Gbits/sec Conntrack: No packet changes: 1.41 Gbits/sec33.1 Gbits/sec VM receiving from: Local BR 221 Mbits/sec 220 Mbits/sec Net NS (veth) 221 Mbits/sec[*] 5.91 Gbits/sec VM (same host)4.79 Gbits/sec 22.2 Gbits/sec Ext Host 10.6 Gbits/sec 10.7 Gbits/sec Ext Host (vxlan) 5.82 Gbits/sec (not supported) Using VLAN: Local BR 223 Mbits/sec 219 Mbits/sec VM (same host)4.21 Gbits/sec 24.1 Gbits/sec Ext Host 10.3 Gbits/sec 10.2 Gbits/sec Using IPv6: Net NS (veth) 217 Mbits/sec[*] 9.32 Gbits/sec VM (same host)4.26 Gbits/sec 23.3 Gbits/sec Ext Host 9.99 Gbits/sec 10.1 Gbits/sec Used iperf3 -u to test UDP traffic limited at default 1Mbits/sec and noticed no change with the exception for tunneled packets (not supported). Travis, AppVeyor, and Cirrus-ci passed. Flavio Leitner (3): dp-packet: preserve headroom when cloning a pkt batch vhost: Disable multi-segmented buffers netdev-dpdk: Add TCP Segmentation Offload support Documentation/automake.mk | 1 + Documentation/topics/dpdk/index.rst | 1 + Documentation/topics/dpdk/tso.rst | 96 + NEWS| 1 + lib/automake.mk | 2 + lib/conntrack.c | 29 ++- lib/dp-packet.h | 158 +- lib/ipf.c | 32 +-- lib/netdev-dpdk.c | 318 lib/netdev-linux-private.h | 4 + lib/netdev-linux.c | 296 +++--- lib/netdev-provider.h | 10 + lib/netdev.c| 66 +- lib/tso.c | 54 + lib/tso.h | 23 ++ vswitchd/bridge.c | 2 + vswitchd/vswitch.xml| 12 ++ 17 files changed, 1013 insertions(+), 92 deletions(-) create mode 100644 Documentation/topics/dpdk/tso.rst create mode 100644 lib/tso.c create mode 100644 lib/tso.h -- 2.24.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 2/3] vhost: Disable multi-segmented buffers
There is no support for multi-segmented buffers, so flag
that to vhost library.
Signed-off-by: Flavio Leitner
---
lib/netdev-dpdk.c | 6 ++
1 file changed, 6 insertions(+)
Changelog:
- v3
* added period at the end of comments.
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 8198a0b7d..5e09786ac 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1297,6 +1297,9 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
dev->vhost_id = xasprintf("%s/%s", dpdk_get_vhost_sock_dir(), name);
dev->vhost_driver_flags &= ~RTE_VHOST_USER_CLIENT;
+
+/* There is no support for multi-segments buffers. */
+dev->vhost_driver_flags |= RTE_VHOST_USER_LINEARBUF_SUPPORT;
err = rte_vhost_driver_register(dev->vhost_id, dev->vhost_driver_flags);
if (err) {
VLOG_ERR("vhost-user socket device setup failure for socket %s\n",
@@ -4423,6 +4426,9 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev
*netdev)
/* Register client-mode device. */
vhost_flags |= RTE_VHOST_USER_CLIENT;
+/* There is no support for multi-segments buffers. */
+vhost_flags |= RTE_VHOST_USER_LINEARBUF_SUPPORT;
+
/* Enable IOMMU support, if explicitly requested. */
if (dpdk_vhost_iommu_enabled()) {
vhost_flags |= RTE_VHOST_USER_IOMMU_SUPPORT;
--
2.24.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 1/3] dp-packet: preserve headroom when cloning a pkt batch
The headroom is useful if the packet needs to insert additional
header, so preserve the original headroom when cloning the batch.
Signed-off-by: Flavio Leitner
---
lib/dp-packet.h | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 3dd59e25d..133942155 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -879,7 +879,11 @@ dp_packet_batch_clone(struct dp_packet_batch *dst,
dp_packet_batch_init(dst);
DP_PACKET_BATCH_FOR_EACH (i, packet, src) {
-dp_packet_batch_add(dst, dp_packet_clone(packet));
+uint32_t headroom = dp_packet_headroom(packet);
+struct dp_packet *pkt_clone;
+
+pkt_clone = dp_packet_clone_with_headroom(packet, headroom);
+dp_packet_batch_add(dst, pkt_clone);
}
dst->trunc = src->trunc;
}
--
2.24.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v10 1/1] Avoid dp_hash recirculation for balance-tcp bond selection mode
On 08.01.2020 21:33, Ben Pfaff wrote:
> On Wed, Jan 08, 2020 at 10:48:04AM +0530, Vishal Deep Ajmera wrote:
>> Problem:
>>
>> In OVS-DPDK, flows with output over a bond interface of type “balance-tcp”
>> (using a hash on TCP/UDP 5-tuple) get translated by the ofproto layer into
>> "HASH" and "RECIRC" datapath actions. After recirculation, the packet is
>> forwarded to the bond member port based on 8-bits of the datapath hash
>> value computed through dp_hash. This causes performance degradation in the
>> following ways:
>
> Thanks for the patch!
>
> I have a few minor stylistic suggestions, see below.
>
> I'd like to hear Ilya's opinion on this.
Thanks Ben for review! I'll take another look at this patch in a next
couple of days.
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 78f9cf928de2..c3a54e96346e 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -378,11 +378,12 @@ struct dp_netdev {
>
> struct conntrack *conntrack;
> struct pmd_auto_lb pmd_alb;
> +
> /* Bonds.
> *
> * Any lookup into 'bonds' requires taking 'bond_mutex'. */
> struct ovs_mutex bond_mutex;
> -struct hmap bonds;
> +struct hmap bonds; /* Contains "struct tx_bond"s. */
> };
>
> static void meter_lock(const struct dp_netdev *dp, uint32_t meter_id)
> @@ -5581,10 +5582,7 @@ pmd_free_cached_bonds(struct dp_netdev_pmd_thread *pmd)
>
> /* Remove bonds from pmd which no longer exists. */
> HMAP_FOR_EACH_SAFE (bond, next, node, >bond_cache) {
> -struct tx_bond *tx = NULL;
> -
> -tx = tx_bond_lookup(>tx_bonds, bond->bond_id);
> -if (!tx) {
> +if (!tx_bond_lookup(>tx_bonds, bond->bond_id)) {
> /* Bond no longer exist. Delete it from pmd. */
> hmap_remove(>bond_cache, >node);
> free(bond);
> @@ -5601,10 +5599,11 @@ pmd_load_cached_bonds(struct dp_netdev_pmd_thread
> *pmd)
> pmd_free_cached_bonds(pmd);
> hmap_shrink(>bond_cache);
>
> -struct tx_bond *tx_bond, *tx_bond_cached;
> +struct tx_bond *tx_bond;
> HMAP_FOR_EACH (tx_bond, node, >tx_bonds) {
> /* Check if bond already exist on pmd. */
> -tx_bond_cached = tx_bond_lookup(>bond_cache, tx_bond->bond_id);
> +struct tx_bond *tx_bond_cached
> += tx_bond_lookup(>bond_cache, tx_bond->bond_id);
>
> if (!tx_bond_cached) {
> /* Create new bond entry in cache. */
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v2] Restrict ARP/IPv6 ND replies for LB VIP only on chassis redirect port
On Thu, Jan 9, 2020 at 1:18 PM wrote:
>
> From: Numan Siddique
>
> Presently when ARP/ND request for the load balance VIP is received
> from the provider network, all the ovn-controllers' reply to the ARP/ND
> request which have ovn-bridge-mappings configured.
>
> This patch restricts these ARP/ND replies only on the chassis where the
> chassis redirect port of the distributed router port is resident.
>
> Signed-off-by: Numan Siddique
Acked-by: Dumitru Ceara
Thanks,
Dumitru
> ---
> v1 -> v2
> -
> * Added the lflow for IPv6 LB VIPs.
>
> northd/ovn-northd.8.xml | 14 ++
> northd/ovn-northd.c | 8
> 2 files changed, 22 insertions(+)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index c6d5d96b9..4b227ca71 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1680,6 +1680,13 @@ flags.loopback = 1;
> output;
>
>
> +
> + If the router port P is a distributed gateway router
> + port, then the is_chassis_resident(P) is
> + also added in the match condition for the load balancer IPv4
> + VIP A.
> +
> +
>
>IPv6: For a configured DNAT IP address or a load balancer
>IPv6 VIP A, solicited node address S,
> @@ -1704,6 +1711,13 @@ nd_na {
> }
>
>
> +
> + If the router port P is a distributed gateway router
> + port, then the is_chassis_resident(P)
> + is also added in the match condition for the load balancer IPv6
> + VIP A.
> +
> +
>
>For the gateway port on a distributed logical router with NAT
>(where one of the logical router ports specifies a
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index d91a008b7..b6dc809d7 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -7511,6 +7511,10 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>"inport == %s && arp.tpa == %s && arp.op == 1",
>op->json_key, ip_address);
>
> +if (op == op->od->l3dgw_port) {
> +ds_put_format(, " && is_chassis_resident(%s)",
> + op->od->l3redirect_port->json_key);
> +}
> ds_clear();
> ds_put_format(,
>"eth.dst = eth.src; "
> @@ -7538,6 +7542,10 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>"inport == %s && nd_ns && nd.target == %s",
>op->json_key, ip_address);
>
> +if (op == op->od->l3dgw_port) {
> +ds_put_format(, " && is_chassis_resident(%s)",
> + op->od->l3redirect_port->json_key);
> +}
> ds_clear();
> ds_put_format(,
>"nd_na { "
> --
> 2.24.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev_afxdp: Detects combined channels and aborts wrong config
On 06.01.2020 22:48, William Tu wrote:
> On Mon, Dec 23, 2019 at 11:34 AM Yi-Hung Wei wrote:
>>
>> This patches detects the number of combined channels on a AF_XDP port
>> via using ethtool interface. If the number of combined channels
>> is different from the n_rxq, netdev_afxdp will not be able to get
>> all the packets from that port. Thus, abort the port setup when
>> the case is detected.
>>
>> Signed-off-by: Yi-Hung Wei
>
> Looks good to me, thanks!
> CC Ilya to see if he has more insight/comments.
>
> Acked-by: William Tu
>
>> ---
>> Travis CI: https://travis-ci.org/YiHungWei/ovs/builds/627972465
>>
>> ---
>> lib/netdev-afxdp.c | 15 +++
>> lib/netdev-linux.c | 27 +++
>> lib/netdev-linux.h | 2 ++
>> 3 files changed, 44 insertions(+)
>>
>> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
>> index 58365ed483e3..6d002882d355 100644
>> --- a/lib/netdev-afxdp.c
>> +++ b/lib/netdev-afxdp.c
>> @@ -601,6 +601,14 @@ netdev_afxdp_set_config(struct netdev *netdev, const
>> struct smap *args,
>> enum afxdp_mode xdp_mode;
>> bool need_wakeup;
>> int new_n_rxq;
>> +int combined_channels;
>> +
>> +if (netdev_linux_ethtool_get_combined_channels(netdev,
>> + _channels)) {
>> +VLOG_INFO("Cannot get the number of combined channels of %s. "
>> + "Defaults it to 1.", netdev_get_name(netdev));
>> +combined_channels = 1;
>> +}
>>
>> ovs_mutex_lock(>mutex);
>> new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1);
>> @@ -611,6 +619,13 @@ netdev_afxdp_set_config(struct netdev *netdev, const
>> struct smap *args,
>> return EINVAL;
>> }
>>
>> +if (new_n_rxq != combined_channels) {
>> +ovs_mutex_unlock(>mutex);
>> +VLOG_ERR("%s: n_rxq: %d != combined channels: %d.",
>> + netdev_get_name(netdev), new_n_rxq, combined_channels);
>> +return EINVAL;
>> +}
>> +
>> str_xdp_mode = smap_get_def(args, "xdp-mode", "best-effort");
>> for (xdp_mode = OVS_AF_XDP_MODE_BEST_EFFORT;
>> xdp_mode < OVS_AF_XDP_MODE_MAX;
>> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
>> index f8e59bacfb13..e3086fc1cbb6 100644
>> --- a/lib/netdev-linux.c
>> +++ b/lib/netdev-linux.c
>> @@ -2166,6 +2166,33 @@ out:
>> netdev->get_features_error = error;
>> }
>>
>> +int
>> +netdev_linux_ethtool_get_combined_channels(struct netdev *netdev_,
>> + int *combined_channels)
>> +{
>> +struct netdev_linux *netdev = netdev_linux_cast(netdev_);
>> +struct ethtool_channels echannels;
>> +int err;
>> +
>> +ovs_mutex_lock(>mutex);
>> +
>> +COVERAGE_INC(netdev_get_ethtool);
>> +
>> +memset(, 0, sizeof echannels);
>> +err = netdev_linux_do_ethtool(netdev_get_name(netdev_),
>> + (struct ethtool_cmd *) ,
>> + ETHTOOL_GCHANNELS, "ETHTOOL_GCHANNELS");
Does this command return maximum possible number of channels or
number of currently enabled channels?
In first case we'll end up with a need to configure huge number of queues.
In second case it's not guaranteed that number of channels will not be
changed later. (Does enabling more channels generates if-notifier event?)
Another point is why we need a configurable parameter that is allowed to
be configured with a single value only?
And there is a possible usecase for not having all the queues attached
to OVS. For example, if you have a custom xdp program loaded, you may
redirect specific traffic to fast afxdp queues and other traffic to
kernel or system datapath. This might be useful for quick handling
of undesired or malicious traffic.
However, this still make sense to limit the maximum number of queues
with the number of actually available channels instead of having a
hardcoded constant (MAX_XSKQ).
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 ovn 1/2] controller: add ipv6 prefix delegation state machine
Introduce IPv6 Prefix delegation state machine according to RFC 3633
https://tools.ietf.org/html/rfc3633.
Add handle_dhcpv6_reply controller action to parse advertise/reply from
IPv6 delegation server. Advertise/reply are parsed running respectively:
- pinctrl_parse_dhcv6_advt
- pinctrl_parse_dhcv6_reply
The IPv6 requesting router starts sending dhcpv6 solicit through the logical
router port marked with ipv6_prefix_delegation set to true.
An IPv6 prefix will be requested for each logical router port marked
with "prefix" set to true in option column of logical router port table.
Save IPv6 prefix received by IPv6 delegation router in the options columns of
SB port binding table in order to be reused by Router Advertisement framework
run by ovn logical router pipeline.
IPv6 Prefix delegation state machine is enabled on Gateway Router or on
a Gateway Router Port
Signed-off-by: Lorenzo Bianconi
---
controller/ovn-controller.c | 1 +
controller/pinctrl.c| 612 +++-
controller/pinctrl.h| 2 +
include/ovn/actions.h | 8 +-
lib/actions.c | 22 ++
lib/ovn-l7.h| 19 ++
ovn-sb.xml | 8 +
tests/ovn.at| 6 +
utilities/ovn-trace.c | 3 +
9 files changed, 673 insertions(+), 8 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 17744d416..d559e845e 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2145,6 +2145,7 @@ main(int argc, char *argv[])
runtime_data = engine_get_data(_runtime_data);
if (runtime_data) {
pinctrl_run(ovnsb_idl_txn,
+ovnsb_idl_loop.idl,
sbrec_datapath_binding_by_key,
sbrec_port_binding_by_datapath,
sbrec_port_binding_by_key,
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index c4752c673..0791566c6 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -270,6 +270,20 @@ static void pinctrl_ip_mcast_handle_igmp(
const struct match *md,
struct ofpbuf *userdata);
+static void init_ipv6_prefixd(void);
+static void destroy_ipv6_prefixd(void);
+static void ipv6_prefixd_wait(long long int timeout);
+static void
+prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn,
+ struct ovsdb_idl *ovnsb_idl,
+ struct ovsdb_idl_index *sbrec_port_binding_by_name,
+ const struct hmap *local_datapaths,
+ const struct sbrec_chassis *chassis,
+ const struct sset *active_tunnels)
+OVS_REQUIRES(pinctrl_mutex);
+static void
+send_ipv6_prefixd(struct rconn *swconn, long long int *send_prefixd_time)
+OVS_REQUIRES(pinctrl_mutex);
static bool may_inject_pkts(void);
static void init_put_vport_bindings(void);
@@ -457,6 +471,7 @@ pinctrl_init(void)
init_put_mac_bindings();
init_send_garps_rarps();
init_ipv6_ras();
+init_ipv6_prefixd();
init_buffered_packets_map();
init_event_table();
ip_mcast_snoop_init();
@@ -544,6 +559,49 @@ set_actions_and_enqueue_msg(struct rconn *swconn,
ofpbuf_uninit();
}
+static struct shash ipv6_prefixd;
+
+enum {
+PREFIX_SOLICIT,
+PREFIX_PENDING,
+PREFIX_DONE,
+};
+
+struct ipv6_prefixd_state {
+long long int next_announce;
+struct in6_addr ipv6_addr;
+struct eth_addr ea;
+struct eth_addr cmac;
+int64_t port_key;
+int64_t metadata;
+struct in6_addr prefix;
+unsigned plife_time;
+unsigned vlife_time;
+unsigned aid;
+unsigned t1;
+unsigned t2;
+int8_t plen;
+int state;
+};
+
+static void
+init_ipv6_prefixd(void)
+{
+shash_init(_prefixd);
+}
+
+static void
+destroy_ipv6_prefixd(void)
+{
+struct shash_node *iter, *next;
+SHASH_FOR_EACH_SAFE (iter, next, _prefixd) {
+struct ipv6_prefixd_state *pfd = iter->data;
+free(pfd);
+shash_delete(_prefixd, iter);
+}
+shash_destroy(_prefixd);
+}
+
struct buffer_info {
struct ofpbuf ofpacts;
struct dp_packet *p;
@@ -967,6 +1025,259 @@ pinctrl_handle_tcp_reset(struct rconn *swconn, const
struct flow *ip_flow,
dp_packet_uninit();
}
+static void
+pinctrl_parse_dhcpv6_advt(struct rconn *swconn, const struct flow *ip_flow,
+ struct dp_packet *pkt_in, const struct match *md,
+ struct ofpbuf *userdata)
+{
+struct udp_header *udp_in = dp_packet_l4(pkt_in);
+size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in));
+unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1);
+uint8_t *data, *end = (uint8_t *)udp_in + dlen;
+int len = 0;
+
+data = xmalloc(dlen);
+/* skip DHCPv6 common header */
+in_dhcpv6_data += 4;
+while (in_dhcpv6_data < end) {
+
[ovs-dev] [PATCH v5 ovn 2/2] northd: add logical flows for dhcpv6 pfd parsing
Introduce logical flows in ovn router pipeline in order to parse dhcpv6
advertise/reply from IPv6 prefix delegation router.
Do not overwrite ipv6_ra_pd_list info in options column of SB port_binding
table written by ovn-controller
Signed-off-by: Lorenzo Bianconi
---
northd/ovn-northd.c | 74 +-
ovn-nb.xml | 17 ++
tests/atlocal.in| 5 +-
tests/system-ovn.at | 127
4 files changed, 221 insertions(+), 2 deletions(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index d91a008b7..0c99a2e05 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -2653,6 +2653,8 @@ ovn_port_update_sbrec(struct northd_context *ctx,
struct sset *active_ha_chassis_grps)
{
sbrec_port_binding_set_datapath(op->sb, op->od->sb);
+const char *ipv6_pd_list = NULL;
+
if (op->nbrp) {
/* If the router is for l3 gateway, it resides on a chassis
* and its port type is "l3gateway". */
@@ -2775,6 +2777,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
smap_add(, "l3gateway-chassis", chassis_name);
}
}
+
+ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
+if (ipv6_pd_list) {
+smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
+}
+
sbrec_port_binding_set_options(op->sb, );
smap_destroy();
@@ -2824,6 +2832,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
smap_add_format(,
"qdisc_queue_id", "%d", queue_id);
}
+
+ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
+if (ipv6_pd_list) {
+smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
+}
+
sbrec_port_binding_set_options(op->sb, );
smap_destroy();
if (ovn_is_known_nb_lsp_type(op->nbsp->type)) {
@@ -2873,6 +2887,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
if (chassis) {
smap_add(, "l3gateway-chassis", chassis);
}
+
+ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
+if (ipv6_pd_list) {
+smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
+}
+
sbrec_port_binding_set_options(op->sb, );
smap_destroy();
} else {
@@ -7129,6 +7149,11 @@ copy_ra_to_sb(struct ovn_port *op, const char
*address_mode)
}
ds_put_format(, "%s/%u ", addrs->network_s, addrs->plen);
}
+
+const char *ra_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
+if (ra_pd_list) {
+ds_put_cstr(, ra_pd_list);
+}
/* Remove trailing space */
ds_chomp(, ' ');
smap_add(, "ipv6_ra_prefixes", ds_cstr());
@@ -7843,7 +7868,36 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
free(snat_ips);
}
-/* Logical router ingress table 3: IP Input for IPv6. */
+/* DHCPv6 reply handling */
+HMAP_FOR_EACH (op, key_node, ports) {
+if (!op->nbrp) {
+continue;
+}
+
+if (op->derived) {
+continue;
+}
+
+struct lport_addresses lrp_networks;
+if (!extract_lrp_networks(op->nbrp, _networks)) {
+continue;
+}
+
+for (size_t i = 0; i < lrp_networks.n_ipv6_addrs; i++) {
+ds_clear();
+ds_clear();
+ds_put_format(, "ip6.dst == %s && udp.src == 547 &&"
+ " udp.dst == 546",
+ lrp_networks.ipv6_addrs[i].addr_s);
+ds_put_format(, "reg0 = 0; handle_dhcpv6_reply { "
+ "eth.dst <-> eth.src; ip6.dst <-> ip6.src; "
+ "outport <-> inport; output; };");
+ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 100,
+ ds_cstr(), ds_cstr());
+}
+}
+
+/* Logical router ingress table 1: IP Input for IPv6. */
HMAP_FOR_EACH (op, key_node, ports) {
if (!op->nbrp) {
continue;
@@ -8644,6 +8698,24 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
continue;
}
+struct smap options;
+/* enable IPv6 prefix delegation */
+bool prefix_delegation = smap_get_bool(>nbrp->options,
+ "prefix_delegation", false);
+if (prefix_delegation) {
+smap_clone(, >sb->options);
+smap_add(, "ipv6_prefix_delegation", "true");
+sbrec_port_binding_set_options(op->sb, );
+smap_destroy();
+}
+
+if (smap_get_bool(>nbrp->options, "prefix", false)) {
+smap_clone(, >sb->options);
+smap_add(, "ipv6_prefix", "true");
+sbrec_port_binding_set_options(op->sb, );
+smap_destroy();
+
[ovs-dev] [PATCH v5 ovn 0/2] Add IPv6 Prefix delegation (RFC3633)
Introduce IPv6 Prefix delegation state machine according to RFC 3633 https://tools.ietf.org/html/rfc3633. Add handle_dhcpv6_reply controller action to parse advertise/reply from IPv6 delegation server. Introduce logical flows in ovn router pipeline in order to parse dhcpv6 advertise/reply from IPv6 prefix delegation router. This series relies on the following OVS commit: https://github.com/openvswitch/ovs/commit/cec89046f72cb044b068ba6a4e30dbcc4292c4c1 Changes since v4: - improve unit test support - fix ovn-controller crash - confifure prefixes received from delegation router in RA - allow the requesting router to rely on lla address for PD protocol Changes since v3: - cosmetics - add a provider bridge in the unit-test deployment and add a localnet port to the deployment to access the underlay network - request IPv6 prefix even for bar router logical port in the unit-test deployment Changes since v2: - add unitest support in system-ovn.at Changes since v1: - rebase on top of ovn master branch - request an IPv6 prefix for each 'downstream' logical router port marked with prefix set to true - add missing documentation - rename dhcp6_server_pkt in handle_dhcpv6_reply Lorenzo Bianconi (2): controller: add ipv6 prefix delegation state machine northd: add logical flows for dhcpv6 pfd parsing controller/ovn-controller.c | 1 + controller/pinctrl.c| 612 +++- controller/pinctrl.h| 2 + include/ovn/actions.h | 8 +- lib/actions.c | 22 ++ lib/ovn-l7.h| 19 ++ northd/ovn-northd.c | 74 - ovn-nb.xml | 17 + ovn-sb.xml | 8 + tests/atlocal.in| 5 +- tests/ovn.at| 6 + tests/system-ovn.at | 127 utilities/ovn-trace.c | 3 + 13 files changed, 894 insertions(+), 10 deletions(-) -- 2.21.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
On 08.01.2020 11:48, Ophir Munk wrote:
> Hi,
> There is an important use case for having OVS change MAC addresses of dpdk
> interfaces.
> OpenStack for example needs to update the MAC address of a VF assigned to a
> VM, where the corresponding VF representor is owned by dpdk.
> For some NIC vendors using "ifconfig" or "ip" commands - is not an option (if
> the NIC is not bifurcated).
> Therefore OpenStack should use the OVS API to set the MAC address for dpdk
> interfaces.
> Along with Ben's explanation it seems right to only allow "internal" or
> "dpdk" port types to set the MAC.
There are still same issues for DPDK ports. Just because OVS
doesn't necessarily "owns" them. In case of Mellanox NICs, ports
are still could be configured by 'ip' tool due to bifurcated
drivers. And this produces the same confusion.
>
> Testing both patches [1] and [2] - passed successfully.
> Acked-by: Ophir Munk
>
> I hope patches [1] and [2] can be merged to master.
>
> [1]
> https://patchwork.ozlabs.org/patch/1186896/
> ("[ovs-dev,v2] netdev-dpdk: Add ability to set MAC address.")
>
> [2]
> https://patchwork.ozlabs.org/patch/1215075/
> ("[ovs-dev,1/1] vswitchd: Allow setting MAC on DPDK interfaces")
>
>> -Original Message-
>> From: Ben Pfaff
>> Sent: Tuesday, January 7, 2020 2:26 AM
>> To: Ilya Maximets
>> Cc: Eveline Raine ; d...@openvswitch.org; Moshe
>> Levi ; Adrian Chiris ;
>> Ophir Munk ; Majd Dibbiny
>> ; Roni Bar Yanai ; Ameer
>> Mahagneh
>> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
>>
>> On Fri, Jan 03, 2020 at 03:56:59PM +0100, Ilya Maximets wrote:
>>> Ben, do you see any other drawbacks that we should handle if we'll
>>> allow changing MAC addresses for non-internal ports? Or, maybe some
>>> issues with my logic?
>>
>> It can cause surprises for interactions with regular system tools.
>> Anyone who uses "ip" or "ifconfig" to change the MAC will find it changed
>> back later (perhaps not immediately). And if you un-set it in the database,
>> OVS doesn't know what to change it back to.
>>
>> These drawbacks aren't there in the same way for devices that OVS "owns"
>> like internal devices or dpdk devices. Well, I guess they are for OVS
>> internal
>> devices to some extent, but for those OVS has some responsibility to pick a
>> reasonable MAC address to begin with. If OVS doesn't, then it causes
>> confusion of its own through things like having a machine's MAC address
>> change if you create a bridge and move a physical NIC onto it. We had lots
>> of
>> experience with that early on with OVS.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] Restrict ARP replies for LB VIP only on chassis redirect port
On Thu, Jan 9, 2020 at 4:11 PM Dumitru Ceara wrote:
>
> On Thu, Jan 9, 2020 at 11:32 AM wrote:
> >
> > From: Numan Siddique
> >
> > Presently when ARP request for the load balance VIP is received
> > from the provider network, all the ovn-controllers' reply to the ARP
> > request which have ovn-bridge-mappings configured.
> >
> > This patch restricts these ARP replies only on the chassis where the
> > chassis redirect port of the distributed router port is resident.
> >
> > Signed-off-by: Numan Siddique
>
> Hi Numan,
>
> The patch looks good to me but you forgot to add the
> "is_chassis_resident" check for IPv6 LB. I see you did add the section
> for v6 to the manpage :)
Oops. Thanks for pointing it out. Submitted v2 -
https://patchwork.ozlabs.org/patch/1220339/
Thanks
Numan
>
> Regards,
> Dumitru
>
> > ---
> > northd/ovn-northd.8.xml | 14 ++
> > northd/ovn-northd.c | 4
> > 2 files changed, 18 insertions(+)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index c6d5d96b9..4b227ca71 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -1680,6 +1680,13 @@ flags.loopback = 1;
> > output;
> >
> >
> > +
> > + If the router port P is a distributed gateway router
> > + port, then the is_chassis_resident(P) is
> > + also added in the match condition for the load balancer IPv4
> > + VIP A.
> > +
> > +
> >
> >IPv6: For a configured DNAT IP address or a load balancer
> >IPv6 VIP A, solicited node address S,
> > @@ -1704,6 +1711,13 @@ nd_na {
> > }
> >
> >
> > +
> > + If the router port P is a distributed gateway router
> > + port, then the is_chassis_resident(P)
> > + is also added in the match condition for the load balancer IPv6
> > + VIP A.
> > +
> > +
> >
> >For the gateway port on a distributed logical router with NAT
> >(where one of the logical router ports specifies a
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index d91a008b7..925f25079 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -7511,6 +7511,10 @@ build_lrouter_flows(struct hmap *datapaths, struct
> > hmap *ports,
> >"inport == %s && arp.tpa == %s && arp.op == 1",
> >op->json_key, ip_address);
> >
> > +if (op == op->od->l3dgw_port) {
> > +ds_put_format(, " && is_chassis_resident(%s)",
> > + op->od->l3redirect_port->json_key);
> > +}
> > ds_clear();
> > ds_put_format(,
> >"eth.dst = eth.src; "
> > --
> > 2.24.1
> >
> > ___
> > 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
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2] Restrict ARP/IPv6 ND replies for LB VIP only on chassis redirect port
From: Numan Siddique
Presently when ARP/ND request for the load balance VIP is received
from the provider network, all the ovn-controllers' reply to the ARP/ND
request which have ovn-bridge-mappings configured.
This patch restricts these ARP/ND replies only on the chassis where the
chassis redirect port of the distributed router port is resident.
Signed-off-by: Numan Siddique
---
v1 -> v2
-
* Added the lflow for IPv6 LB VIPs.
northd/ovn-northd.8.xml | 14 ++
northd/ovn-northd.c | 8
2 files changed, 22 insertions(+)
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index c6d5d96b9..4b227ca71 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1680,6 +1680,13 @@ flags.loopback = 1;
output;
+
+ If the router port P is a distributed gateway router
+ port, then the is_chassis_resident(P) is
+ also added in the match condition for the load balancer IPv4
+ VIP A.
+
+
IPv6: For a configured DNAT IP address or a load balancer
IPv6 VIP A, solicited node address S,
@@ -1704,6 +1711,13 @@ nd_na {
}
+
+ If the router port P is a distributed gateway router
+ port, then the is_chassis_resident(P)
+ is also added in the match condition for the load balancer IPv6
+ VIP A.
+
+
For the gateway port on a distributed logical router with NAT
(where one of the logical router ports specifies a
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index d91a008b7..b6dc809d7 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -7511,6 +7511,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
"inport == %s && arp.tpa == %s && arp.op == 1",
op->json_key, ip_address);
+if (op == op->od->l3dgw_port) {
+ds_put_format(, " && is_chassis_resident(%s)",
+ op->od->l3redirect_port->json_key);
+}
ds_clear();
ds_put_format(,
"eth.dst = eth.src; "
@@ -7538,6 +7542,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
"inport == %s && nd_ns && nd.target == %s",
op->json_key, ip_address);
+if (op == op->od->l3dgw_port) {
+ds_put_format(, " && is_chassis_resident(%s)",
+ op->od->l3redirect_port->json_key);
+}
ds_clear();
ds_put_format(,
"nd_na { "
--
2.24.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] Restrict ARP replies for LB VIP only on chassis redirect port
On Thu, Jan 9, 2020 at 11:32 AM wrote:
>
> From: Numan Siddique
>
> Presently when ARP request for the load balance VIP is received
> from the provider network, all the ovn-controllers' reply to the ARP
> request which have ovn-bridge-mappings configured.
>
> This patch restricts these ARP replies only on the chassis where the
> chassis redirect port of the distributed router port is resident.
>
> Signed-off-by: Numan Siddique
Hi Numan,
The patch looks good to me but you forgot to add the
"is_chassis_resident" check for IPv6 LB. I see you did add the section
for v6 to the manpage :)
Regards,
Dumitru
> ---
> northd/ovn-northd.8.xml | 14 ++
> northd/ovn-northd.c | 4
> 2 files changed, 18 insertions(+)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index c6d5d96b9..4b227ca71 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1680,6 +1680,13 @@ flags.loopback = 1;
> output;
>
>
> +
> + If the router port P is a distributed gateway router
> + port, then the is_chassis_resident(P) is
> + also added in the match condition for the load balancer IPv4
> + VIP A.
> +
> +
>
>IPv6: For a configured DNAT IP address or a load balancer
>IPv6 VIP A, solicited node address S,
> @@ -1704,6 +1711,13 @@ nd_na {
> }
>
>
> +
> + If the router port P is a distributed gateway router
> + port, then the is_chassis_resident(P)
> + is also added in the match condition for the load balancer IPv6
> + VIP A.
> +
> +
>
>For the gateway port on a distributed logical router with NAT
>(where one of the logical router ports specifies a
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index d91a008b7..925f25079 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -7511,6 +7511,10 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>"inport == %s && arp.tpa == %s && arp.op == 1",
>op->json_key, ip_address);
>
> +if (op == op->od->l3dgw_port) {
> +ds_put_format(, " && is_chassis_resident(%s)",
> + op->od->l3redirect_port->json_key);
> +}
> ds_clear();
> ds_put_format(,
>"eth.dst = eth.src; "
> --
> 2.24.1
>
> ___
> 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] [PATCH ovn] Restrict ARP replies for LB VIP only on chassis redirect port
From: Numan Siddique
Presently when ARP request for the load balance VIP is received
from the provider network, all the ovn-controllers' reply to the ARP
request which have ovn-bridge-mappings configured.
This patch restricts these ARP replies only on the chassis where the
chassis redirect port of the distributed router port is resident.
Signed-off-by: Numan Siddique
---
northd/ovn-northd.8.xml | 14 ++
northd/ovn-northd.c | 4
2 files changed, 18 insertions(+)
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index c6d5d96b9..4b227ca71 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1680,6 +1680,13 @@ flags.loopback = 1;
output;
+
+ If the router port P is a distributed gateway router
+ port, then the is_chassis_resident(P) is
+ also added in the match condition for the load balancer IPv4
+ VIP A.
+
+
IPv6: For a configured DNAT IP address or a load balancer
IPv6 VIP A, solicited node address S,
@@ -1704,6 +1711,13 @@ nd_na {
}
+
+ If the router port P is a distributed gateway router
+ port, then the is_chassis_resident(P)
+ is also added in the match condition for the load balancer IPv6
+ VIP A.
+
+
For the gateway port on a distributed logical router with NAT
(where one of the logical router ports specifies a
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index d91a008b7..925f25079 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -7511,6 +7511,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
"inport == %s && arp.tpa == %s && arp.op == 1",
op->json_key, ip_address);
+if (op == op->od->l3dgw_port) {
+ds_put_format(, " && is_chassis_resident(%s)",
+ op->od->l3redirect_port->json_key);
+}
ds_clear();
ds_put_format(,
"eth.dst = eth.src; "
--
2.24.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] acinclude: Use RTE_IBVERBS_LINK_DLOPEN
On 1/8/2020 3:13 PM, Aaron Conole wrote:
Timothy Redaelli writes:
On DPDK 19.11 RTE_IBVERBS_LINK_DLOPEN is used by Mellanox PMDs (mlx4 and
mlx5) instead of RTE_LIBRTE_MLX{4,5}_DLOPEN_DEPS.
Without this commit is not possible to statically link OVS with DPDK when MLX4
or MLX5 PMDs are enabled.
Signed-off-by: Timothy Redaelli
---
Acked-by: Aaron Conole
Thanks for the patch Timothy, push to master.
Regards
Ian
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS 2.13 Soft Freeze date
On 1/8/2020 6:40 PM, Ben Pfaff wrote: On Wed, Jan 08, 2020 at 05:33:59PM +, Stokes, Ian wrote: On 1/8/2020 11:09 AM, Stokes, Ian wrote: Hi Ben, just wanted to confirm if the soft freeze for OVS 2.13 is still confirmed for January 15th? Just a had a few queries so thought it would be best to ask. Apologies, was just pointed out to me on the community call, that soft freeze was January 1st, I meant to refer to the feature freeze. I haven't been paying good enough attention. Apologies. Let's set the feature freeze to January 17 to round off the week to Friday. Thanks, Thanks for the confirmation Ben. Regards Ian ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V6 09/18] dpctl: Support dump-flows filters "dpdk" and "partially-offloaded"
On Wed, Jan 8, 2020 at 11:34 PM Ilya Maximets wrote:
>
> On 08.01.2020 18:25, Eli Britstein wrote:
> >
> > On 1/8/2020 2:17 PM, Ilya Maximets wrote:
> >> On 19.12.2019 12:54, Eli Britstein wrote:
> >>> Flows that are offloaded via DPDK can be partially offloaded (matches
> >>> only) or fully offloaded (matches and actions). Set partially offloaded
> >>> display to (offloaded=partial, dp:ovs), and fully offloaded to
> >>> (offloaded=yes, dp:dpdk). Also support filter types "dpdk" and
> >>> "partially-offloaded".
> >>>
> >>> Signed-off-by: Eli Britstein
> >>> ---
> >>> NEWS | 3 ++
> >>> lib/dpctl.c | 97
> >>> ---
> >>> lib/dpctl.man | 2 ++
> >>> 3 files changed, 78 insertions(+), 24 deletions(-)
> >>>
> >>> diff --git a/NEWS b/NEWS
> >>> index 2acde3fe8..85c4a4812 100644
> >>> --- a/NEWS
> >>> +++ b/NEWS
> >>> @@ -26,6 +26,9 @@ Post-v2.12.0
> >>>* DPDK ring ports (dpdkr) are deprecated and will be removed in
> >>> next
> >>> releases.
> >>>* Add support for DPDK 19.11.
> >>> + - 'ovs-appctl dpctl/dump-flows' can now show offloaded=partial for
> >>> + partially offloaded flows, dp:dpdk for fully offloaded by dpdk, and
> >>> + type filter supports new filters: "dpdk" and "partially-offloaded".
> >>>
> >>> v2.12.0 - 03 Sep 2019
> >>> -
> >>> diff --git a/lib/dpctl.c b/lib/dpctl.c
> >>> index 0ee64718c..387f61ed4 100644
> >>> --- a/lib/dpctl.c
> >>> +++ b/lib/dpctl.c
> >>> @@ -818,7 +818,11 @@ format_dpif_flow(struct ds *ds, const struct
> >>> dpif_flow *f, struct hmap *ports,
> >>>
> >>> dpif_flow_stats_format(>stats, ds);
> >>> if (dpctl_p->verbosity && f->attrs.offloaded) {
> >>> -ds_put_cstr(ds, ", offloaded:yes");
> >>> +if (f->attrs.dp_layer && !strcmp(f->attrs.dp_layer, "ovs")) {
> >>> +ds_put_cstr(ds, ", offloaded:partial");
> >>> +} else {
> >>> +ds_put_cstr(ds, ", offloaded:yes");
> >>> +}
> >>> }
> >>> if (dpctl_p->verbosity && f->attrs.dp_layer) {
> >>> ds_put_format(ds, ", dp:%s", f->attrs.dp_layer);
> >>> @@ -827,20 +831,30 @@ format_dpif_flow(struct ds *ds, const struct
> >>> dpif_flow *f, struct hmap *ports,
> >>> format_odp_actions(ds, f->actions, f->actions_len, ports);
> >>> }
> >>>
> >>> +enum dp_type {
> >>> +DP_TYPE_ANY,
> >>> +DP_TYPE_OVS,
> >>> +DP_TYPE_TC,
> >>> +DP_TYPE_DPDK,
> >>> +};
> >>> +
> >>> +enum ol_type {
> >>> +OL_TYPE_ANY,
> >>> +OL_TYPE_NO,
> >>> +OL_TYPE_YES,
> >>> +OL_TYPE_PARTIAL,
> >>> +};
> >>> +
> >>> struct dump_types {
> >>> -bool ovs;
> >>> -bool tc;
> >>> -bool offloaded;
> >>> -bool non_offloaded;
> >>> +enum dp_type dp_type;
> >>> +enum ol_type ol_type;
> >>> };
> >>>
> >>> static void
> >>> enable_all_dump_types(struct dump_types *dump_types)
> >>> {
> >>> -dump_types->ovs = true;
> >>> -dump_types->tc = true;
> >>> -dump_types->offloaded = true;
> >>> -dump_types->non_offloaded = true;
> >>> +dump_types->dp_type = DP_TYPE_ANY;
> >>> +dump_types->ol_type = OL_TYPE_ANY;
> >>> }
> >>>
> >>> static int
> >>> @@ -862,13 +876,17 @@ populate_dump_types(char *types_list, struct
> >>> dump_types *dump_types,
> >>> current_type[type_len] = '\0';
> >>>
> >>> if (!strcmp(current_type, "ovs")) {
> >>> -dump_types->ovs = true;
> >>> +dump_types->dp_type = DP_TYPE_OVS;
> >>> } else if (!strcmp(current_type, "tc")) {
> >>> -dump_types->tc = true;
> >>> +dump_types->dp_type = DP_TYPE_TC;
> >>> +} else if (!strcmp(current_type, "dpdk")) {
> >>> +dump_types->dp_type = DP_TYPE_DPDK;
> >>> } else if (!strcmp(current_type, "offloaded")) {
> >>> -dump_types->offloaded = true;
> >>> +dump_types->ol_type = OL_TYPE_YES;
> >>> } else if (!strcmp(current_type, "non-offloaded")) {
> >>> -dump_types->non_offloaded = true;
> >>> +dump_types->ol_type = OL_TYPE_NO;
> >>> +} else if (!strcmp(current_type, "partially-offloaded")) {
> >>> +dump_types->ol_type = OL_TYPE_PARTIAL;
> >>> } else if (!strcmp(current_type, "all")) {
> >>> enable_all_dump_types(dump_types);
> >>> } else {
> >>> @@ -884,30 +902,61 @@ static void
> >>> determine_dpif_flow_dump_types(struct dump_types *dump_types,
> >>> struct dpif_flow_dump_types
> >>> *dpif_dump_types)
> >>> {
> >>> -dpif_dump_types->ovs_flows = dump_types->ovs ||
> >>> dump_types->non_offloaded;
> >>> -dpif_dump_types->netdev_flows = dump_types->tc ||
> >>> dump_types->offloaded
> >>> -|| dump_types->non_offloaded;
> >>> +dpif_dump_types->ovs_flows = dump_types->dp_type == DP_TYPE_OVS;
> >>> +
Re: [ovs-dev] [PATCH V7 09/18] dpctl: Support dump-flows filters "dpdk" and "partially-offloaded"
Bleep bloop. Greetings Eli Britstein, 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 is 96 characters long (recommended limit is 79) #126 FILE: lib/dpctl.man:130: \fBpartially-offloaded\fR - displays flows where only part of their proccessing is done in HW Lines checked: 132, 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
