[ovs-dev] Proposal
I have a confidential deal for you, please contact me for more details via this email immediately. God Bless, Ms.Yael Ronen ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto: Return error codes for Rule insertions
Hi Aaron/Ben/All,
Kindly review the patch and let me know your views.
On Thu, Jul 12, 2018 at 11:34 PM, Aravind Prasad S
wrote:
> Currently, rule_insert() API does not have return value. There are some
> possible
> scenarios where rule insertions can fail at run-time even though the static
> checks during rule_construct() had passed previously.
> Some possible scenarios for failure of rule insertions:
> **) Rule insertions can fail dynamically in Hybrid mode (both Openflow and
> Normal switch functioning coexist) where the CAM space could get suddenly
> filled up by Normal switch functioning and Openflow gets devoid of
> available space.
> **) Some deployments could have separate independent layers for HW rule
> insertions and application layer to interact with OVS. HW layer
> could face any dynamic issue during rule handling which application could
> not have predicted/captured in rule-construction phase.
> Rule-insert errors for bundles are handled too in this pull-request.
>
> Signed-off-by: Aravind Prasad S
> ---
> ofproto/ofproto-dpif.c | 4 +-
> ofproto/ofproto-provider.h | 6 +--
> ofproto/ofproto.c | 105 ++
> ---
> 3 files changed, 85 insertions(+), 30 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ad1e8af..d1678ed 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4443,7 +4443,7 @@ rule_construct(struct rule *rule_)
> return 0;
> }
>
> -static void
> +static enum ofperr
> rule_insert(struct rule *rule_, struct rule *old_rule_, bool
> forward_counts)
> OVS_REQUIRES(ofproto_mutex)
> {
> @@ -4473,6 +4473,8 @@ rule_insert(struct rule *rule_, struct rule
> *old_rule_, bool forward_counts)
> ovs_mutex_unlock(>stats_mutex);
> ovs_mutex_unlock(_rule->stats_mutex);
> }
> +
> +return 0;
> }
>
> static void
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 2b77b89..3f3d110 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -1297,8 +1297,8 @@ struct ofproto_class {
> struct rule *(*rule_alloc)(void);
> enum ofperr (*rule_construct)(struct rule *rule)
> /* OVS_REQUIRES(ofproto_mutex) */;
> -void (*rule_insert)(struct rule *rule, struct rule *old_rule,
> -bool forward_counts)
> +enum ofperr (*rule_insert)(struct rule *rule, struct rule *old_rule,
> + bool forward_counts)
> /* OVS_REQUIRES(ofproto_mutex) */;
> void (*rule_delete)(struct rule *rule) /* OVS_REQUIRES(ofproto_mutex)
> */;
> void (*rule_destruct)(struct rule *rule);
> @@ -1952,7 +1952,7 @@ enum ofperr ofproto_flow_mod_learn_start(struct
> ofproto_flow_mod *ofm)
> OVS_REQUIRES(ofproto_mutex);
> void ofproto_flow_mod_learn_revert(struct ofproto_flow_mod *ofm)
> OVS_REQUIRES(ofproto_mutex);
> -void ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
> +enum ofperr ofproto_flow_mod_learn_finish(struct ofproto_flow_mod *ofm,
>struct ofproto *orig_ofproto)
> OVS_REQUIRES(ofproto_mutex);
> void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index f946e27..cb09ee6 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -245,10 +245,12 @@ static void replace_rule_revert(struct ofproto *,
> struct rule *old_rule,
> struct rule *new_rule)
> OVS_REQUIRES(ofproto_mutex);
>
> -static void replace_rule_finish(struct ofproto *, struct ofproto_flow_mod
> *,
> -const struct openflow_mod_requester *,
> -struct rule *old_rule, struct rule
> *new_rule,
> -struct ovs_list *dead_cookies)
> +static enum ofperr replace_rule_finish(struct ofproto *,
> + struct ofproto_flow_mod *,
> + const struct
> openflow_mod_requester *,
> + struct rule *old_rule,
> + struct rule *new_rule,
> + struct ovs_list *dead_cookies)
> OVS_REQUIRES(ofproto_mutex);
> static void delete_flows__(struct rule_collection *,
> enum ofp_flow_removed_reason,
> @@ -270,7 +272,7 @@ static enum ofperr ofproto_flow_mod_start(struct
> ofproto *,
> static void ofproto_flow_mod_revert(struct ofproto *,
> struct ofproto_flow_mod *)
> OVS_REQUIRES(ofproto_mutex);
> -static void ofproto_flow_mod_finish(struct ofproto *,
> +static enum ofperr ofproto_flow_mod_finish(struct ofproto *,
> struct ofproto_flow_mod *,
> const struct openflow_mod_requester *)
>
Re: [ovs-dev] [PATCH] Documentation:
On 7/13/2018 3:44 PM, Greg Rose wrote: Add netstat when mentioning testing. Many check-kmod failures result when it is not present. Signed-off-by: Greg Rose I fat fingered the title - should be "Documentation: Add netstat to testing I can send V2 or it can be fixed on push, either way works. Thanks, - Greg --- Documentation/intro/install/general.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst index fe30c19..537137e 100644 --- a/Documentation/intro/install/general.rst +++ b/Documentation/intro/install/general.rst @@ -156,6 +156,8 @@ The datapath tests for userspace and Linux datapaths also rely upon: - tftpy. Version 0.6.2 is known to work. Earlier versions should also work. +- netstat. Available from various distro specific packages + The ovs-vswitchd.conf.db(5) manpage will include an E-R diagram, in formats other than plain text, only if you have the following: ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] Documentation:
Add netstat when mentioning testing. Many check-kmod failures result when it is not present. Signed-off-by: Greg Rose --- Documentation/intro/install/general.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst index fe30c19..537137e 100644 --- a/Documentation/intro/install/general.rst +++ b/Documentation/intro/install/general.rst @@ -156,6 +156,8 @@ The datapath tests for userspace and Linux datapaths also rely upon: - tftpy. Version 0.6.2 is known to work. Earlier versions should also work. +- netstat. Available from various distro specific packages + The ovs-vswitchd.conf.db(5) manpage will include an E-R diagram, in formats other than plain text, only if you have the following: -- 1.8.3.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-dev, v3, 15 of 17] ovn-nbctl: Initial support for daemon mode.
Bleep bloop. Greetings Jakub Sitnicki, 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) #80 FILE: ovn/utilities/ovn-nbctl.8.xml:1023: http://www.w3.org/2003/XInclude"/> Lines checked: 504, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4] ovn: Allow for automatic dynamic updates of IPAM
OVN offers a method of IP address management that allows for an IPv4 subnet or
IPv6 prefix to be specified on a logical switch. Then by specifying a
switch port's address as "dynamic" or " dynamic", OVN will
automatically assign addresses to the switch port.
While this works great for initial assignment of addresses, addresses do
not automatically adjust when changes are made to the switch's
configuration. For instance:
* If the subnet, ipv6_prefix, or exclude_ips for a logical switch
changes, the affected switch ports are not updated.
* If a switch port with a static IP address is added to the switch, and
that address conflicts with a dynamically assigned IP address, the
dynamic address is not updated.
* If a MAC address switched from being statically assigned to
dynamically assigned, the MAC address would not be updated.
* If a statically assigned MAC address changed, then the IPv6 address
would not be updated.
This patch solves all of the above issues by changing the algorithm for
IPAM assignment. There are essentially three steps.
1) While joining logical ports, all statically-assigned addresses (i.e.
any ports without "dynamic" addresses) have their addresses registered
to IPAM. This gives them top priority.
2) All logical ports with dynamic addresses are inspected. Any changes
that must be made to the addresses are collected to be made later. Any
addresses that do not require change are registered to IPAM. This allows
for previously assigned dynamic addresses to be kept.
3) All gathered changes are enacted.
The change contains new tests that ensure that dynamic addresses are
updated when appropriate.
This patch also alters some existing IPAM tests. Those tests assumed
that dynamic addresses would not be updated automatically, so those
tests either had to be altered or removed.
Signed-off-by: Mark Michelson
---
v3->v4:
Print warning when multiple dynamic addresses are configured on a
switch port. Ensure that dynamic addresses beyond the first on a switch
port are ignored. Found by Ben Pfaff.
v2->v3:
Fixed a checkpatch problem (line too long)
v1->v2:
Rebased
---
ovn/northd/ovn-northd.c | 385 ++--
tests/ovn.at| 97 +---
2 files changed, 350 insertions(+), 132 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 04a072ba8..b75febb44 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -985,9 +985,6 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct
ovn_port *op)
for (size_t i = 0; i < op->nbsp->n_addresses; i++) {
ipam_insert_lsp_addresses(od, op, op->nbsp->addresses[i]);
}
-if (op->nbsp->dynamic_addresses) {
-ipam_insert_lsp_addresses(od, op, op->nbsp->dynamic_addresses);
-}
} else if (op->nbrp) {
struct lport_addresses lrp_networks;
if (!extract_lrp_networks(op->nbrp, _networks)) {
@@ -1060,64 +1057,255 @@ ipam_get_unused_ip(struct ovn_datapath *od)
return od->ipam_info.start_ipv4 + new_ip_index;
}
+enum dynamic_update_type {
+NONE,/* No change to the address */
+REMOVE, /* Address is no longer dynamic */
+STATIC, /* Use static address (MAC only) */
+DYNAMIC, /* Assign a new dynamic address */
+};
+
+struct dynamic_address_update {
+struct ovs_list node; /* In build_ipam()'s list of updates. */
+
+struct ovn_port *op;
+
+struct lport_addresses current_addresses;
+struct eth_addr static_mac;
+enum dynamic_update_type mac;
+enum dynamic_update_type ipv4;
+enum dynamic_update_type ipv6;
+};
+
+static enum dynamic_update_type
+dynamic_mac_changed(const char *lsp_addresses,
+struct dynamic_address_update *update)
+{
+ struct eth_addr ea;
+
+ if (ovs_scan(lsp_addresses, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ea))) {
+ if (eth_addr_equals(ea, update->current_addresses.ea)) {
+ return NONE;
+ } else {
+ /* MAC is still static, but it has changed */
+ update->static_mac = ea;
+ return STATIC;
+ }
+ }
+
+ uint64_t mac64 = eth_addr_to_uint64(update->current_addresses.ea);
+ if ((mac64 ^ MAC_ADDR_PREFIX) >> 24) {
+ return DYNAMIC;
+ } else {
+ return NONE;
+ }
+}
+
+static enum dynamic_update_type
+dynamic_ip4_changed(struct dynamic_address_update *update)
+{
+bool dynamic_ip4 = update->op->od->ipam_info.allocated_ipv4s != NULL;
+
+if (!dynamic_ip4) {
+if (update->current_addresses.n_ipv4_addrs) {
+return REMOVE;
+} else {
+return NONE;
+}
+}
+
+if (!update->current_addresses.n_ipv4_addrs) {
+/* IPv4 was previously static but now is dynamic */
+return DYNAMIC;
+}
+
+uint32_t ip4 = ntohl(update->current_addresses.ipv4_addrs[0].addr);
+if (ip4 < update->op->od->ipam_info.start_ipv4) {
+return DYNAMIC;
+}
+
+uint32_t index = ip4 -
[ovs-dev] [PATCH v3 17/17] tests: Add test for oneline-formatted output for ovn-nbctl.
Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 21 + 1 file changed, 21 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 89daf631a..60b4d0c9c 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1403,3 +1403,24 @@ AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - oneline output]) +OVN_NBCTL_TEST_START + +AT_CHECK([ovn-nbctl ls-add ls0 -- ls-add ls1]) + +# Expect one line for one command. +AT_CHECK([ovn-nbctl --oneline ls-list | uuidfilt], [0], [dnl +<0> (ls0)\n<1> (ls1) +]) + +# Expect lines for two commands. +AT_CHECK([ovn-nbctl --oneline ls-list -- ls-list | uuidfilt], [0], [dnl +<0> (ls0)\n<1> (ls1) +<0> (ls0)\n<1> (ls1) +]) + +OVN_NBCTL_TEST_STOP +AT_CLEANUP -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 16/17] tests: Add test for ovn-nbctl dry run mode.
Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 21 + 1 file changed, 21 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 73a61a4be..89daf631a 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1382,3 +1382,24 @@ inactivity_probe: 3 OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - dry run mode]) +OVN_NBCTL_TEST_START + +# Check that dry run has no permanent effect. +AT_CHECK([ovn-nbctl --dry-run ls-add ls0 -- ls-list | uuidfilt], [0], [dnl +<0> (ls0) +]) +AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl +]) + +# Check that dry-run mode is not sticky. +AT_CHECK([ovn-nbctl ls-add ls0]) +AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl +<0> (ls0) +]) + +OVN_NBCTL_TEST_STOP +AT_CLEANUP -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 15/17] ovn-nbctl: Initial support for daemon mode.
Make ovn-nbctl act as a unixctl server if we were asked to detach. This
turns ovn-nbctl into a long-lived process that acts a proxy for
interacting with NB DB. The main difference to regular mode of ovn-nbctl
is that in the daemon mode, a local copy of database contents has to be
obtained only once.
Just two unixctl commands are supported 'run' and 'exit'. The former can
be used to run any ovn-nbctl command or a batch of them as so:
ovs-appctl -t ovn-nbctl run [OPTIONS] COMMAND [-- [OPTIONS] COMMAND] ...
Running commands that have not yet been converted to not use ctl_fatal()
will result in death of the daemon process. However, --monitor option
can be used to keep the daemon running.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.8.xml | 40 ++
ovn/utilities/ovn-nbctl.c | 308 ++
2 files changed, 322 insertions(+), 26 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index abba4ecdb..2cd2fab30 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -913,6 +913,43 @@
+Daemon Mode
+
+
+ If ovn-nbctl is invoked with the --detach
+ option (see Daemon Options, below), it runs in the
+ background as a daemon and accepts commands from ovs-appctl
+ (or another JSON-RPC client) indefinitely. The currently supported
+ commands are described below.
+
+
+
+
+
+
+
+
+run [options] command
+[arg...] [-- [options]
+command [arg...] ...]
+
+
+Instructs the daemon process to run one or more ovn-nbctl
+commands described above and reply with the results of running these
+commands. Accepts the --no-wait, --wait,
+--timeout, --dry-run, --oneline,
+and the options described under Table Formatting Options
+in addition to the the command-specific options.
+
+
+ exit
+ Causes ovn-nbctl to gracefully terminate.
+
+
+
+ Daemon mode is considered experimental.
+
+
Options
@@ -982,6 +1019,9 @@
+Daemon Options
+http://www.w3.org/2003/XInclude"/>
+
Logging options
http://www.w3.org/2003/XInclude"/>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index c83d03218..46e923cb9 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -20,6 +20,7 @@
#include
#include "command-line.h"
+#include "daemon.h"
#include "db-ctl-base.h"
#include "dirs.h"
#include "fatal-signal.h"
@@ -38,6 +39,7 @@
#include "table.h"
#include "timeval.h"
#include "timer.h"
+#include "unixctl.h"
#include "util.h"
#include "openvswitch/vlog.h"
@@ -80,6 +82,13 @@ OVS_NO_RETURN static void nbctl_exit(int status);
/* --leader-only, --no-leader-only: Only accept the leader in a cluster. */
static int leader_only = true;
+/* --unixctl-path: Path to use for unixctl server, for "monitor" and "snoop"
+ commands. */
+static char *unixctl_path;
+
+static unixctl_cb_func server_cmd_exit;
+static unixctl_cb_func server_cmd_run;
+
static void nbctl_cmd_init(void);
OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
@@ -98,15 +107,13 @@ static char * OVS_WARN_UNUSED_RESULT main_loop(const char
*args,
size_t n_commands,
struct ovsdb_idl *idl,
const struct timer *);
+static void server_loop(struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
{
struct ovsdb_idl *idl;
-struct ctl_command *commands;
struct shash local_options;
-size_t n_commands;
-char *error;
set_program_name(argv[0]);
fatal_ignore_sigpipe();
@@ -119,38 +126,55 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-commands = ctl_parse_commands(argc - optind, argv + optind, _options,
- _commands);
-VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG,
- "Called as %s", args);
-
-if (timeout) {
-time_alarm(timeout);
-}
+argc -= optind;
+argv += optind;
/* Initialize IDL. */
idl = the_idl = ovsdb_idl_create(db, _idl_class, true, false);
ovsdb_idl_set_leader_only(idl, leader_only);
-error = run_prerequisites(commands, n_commands, idl);
-if (error) {
-ctl_fatal("%s", error);
-}
-error = main_loop(args, commands, n_commands, idl, NULL);
-if (error) {
-ctl_fatal("%s", error);
+if (get_detach()) {
+if (argc != 0) {
+ctl_fatal("non-option arguments not supported with --detach "
+ "(use --help for help)");
+}
+server_loop(idl);
+} else {
+struct
[ovs-dev] [PATCH v3 14/17] ovn-nbctl: Extract a helper for appending command options.
Will be reused when parsing options in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 37 +
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index bac1c001d..c83d03218 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -286,6 +286,30 @@ build_short_options(const struct option *long_options)
return short_options;
}
+static struct option * OVS_WARN_UNUSED_RESULT
+append_command_options(const struct option *options, int opt_val)
+{
+struct option *o;
+size_t n_allocated;
+size_t n_existing;
+int i;
+
+for (i = 0; options[i].name; i++) {
+;
+}
+n_allocated = i + 1;
+n_existing = i;
+
+/* We want to parse both global and command-specific options here, but
+ * getopt_long() isn't too convenient for the job. We copy our global
+ * options into a dynamic array, then append all of the command-specific
+ * options. */
+o = xmemdup(options, n_allocated * sizeof *options);
+ctl_add_cmd_options(, _existing, _allocated, opt_val);
+
+return o;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
@@ -307,22 +331,11 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
};
const int n_global_long_options = ARRAY_SIZE(global_long_options) - 1;
char *short_options;
-
struct option *options;
-size_t allocated_options;
-size_t n_options;
size_t i;
short_options = build_short_options(global_long_options);
-
-/* We want to parse both global and command-specific options here, but
- * getopt_long() isn't too convenient for the job. We copy our global
- * options into a dynamic array, then append all of the command-specific
- * options. */
-options = xmemdup(global_long_options, sizeof global_long_options);
-allocated_options = ARRAY_SIZE(global_long_options);
-n_options = n_global_long_options;
-ctl_add_cmd_options(, _options, _options, OPT_LOCAL);
+options = append_command_options(global_long_options, OPT_LOCAL);
for (;;) {
int idx;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 13/17] ovn-nbctl: Extract a helper for building short options string.
Will be reused for parsing options in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 72c78795c..bac1c001d 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -274,6 +274,18 @@ handle_main_loop_option(int opt, const char *arg, bool
*handled)
return NULL;
}
+static char * OVS_WARN_UNUSED_RESULT
+build_short_options(const struct option *long_options)
+{
+char *tmp, *short_options;
+
+tmp = ovs_cmdl_long_options_to_short_options(long_options);
+short_options = xasprintf("+%s", tmp);
+free(tmp);
+
+return short_options;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
@@ -294,16 +306,14 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
{NULL, 0, NULL, 0},
};
const int n_global_long_options = ARRAY_SIZE(global_long_options) - 1;
-char *tmp, *short_options;
+char *short_options;
struct option *options;
size_t allocated_options;
size_t n_options;
size_t i;
-tmp = ovs_cmdl_long_options_to_short_options(global_long_options);
-short_options = xasprintf("+%s", tmp);
-free(tmp);
+short_options = build_short_options(global_long_options);
/* We want to parse both global and command-specific options here, but
* getopt_long() isn't too convenient for the job. We copy our global
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 12/17] ovn-nbctl: Extract handling of options that affect main loop.
Provide a handler for options that change how the main loop behaves.
This will allow code reuse for option parsing in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 136 --
1 file changed, 84 insertions(+), 52 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 3dd24d193..72c78795c 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -200,38 +200,93 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
return NULL;
}
+/* All options that affect the main loop and are not external. */
+#define MAIN_LOOP_OPTION_ENUMS \
+OPT_NO_WAIT,\
+OPT_WAIT, \
+OPT_DRY_RUN,\
+OPT_ONELINE
+
+#define MAIN_LOOP_LONG_OPTIONS \
+{"no-wait", no_argument, NULL, OPT_NO_WAIT}, \
+{"wait", required_argument, NULL, OPT_WAIT}, \
+{"dry-run", no_argument, NULL, OPT_DRY_RUN}, \
+{"oneline", no_argument, NULL, OPT_ONELINE}, \
+{"timeout", required_argument, NULL, 't'}
+
+enum {
+OPT_DB = UCHAR_MAX + 1,
+OPT_NO_SYSLOG,
+OPT_LOCAL,
+OPT_COMMANDS,
+OPT_OPTIONS,
+OPT_BOOTSTRAP_CA_CERT,
+MAIN_LOOP_OPTION_ENUMS,
+VLOG_OPTION_ENUMS,
+TABLE_OPTION_ENUMS,
+SSL_OPTION_ENUMS,
+};
+
+static char * OVS_WARN_UNUSED_RESULT
+handle_main_loop_option(int opt, const char *arg, bool *handled)
+{
+ovs_assert(handled);
+*handled = true;
+
+switch (opt) {
+case OPT_ONELINE:
+oneline = true;
+break;
+
+case OPT_NO_WAIT:
+wait_type = NBCTL_WAIT_NONE;
+break;
+
+case OPT_WAIT:
+if (!strcmp(arg, "none")) {
+wait_type = NBCTL_WAIT_NONE;
+} else if (!strcmp(arg, "sb")) {
+wait_type = NBCTL_WAIT_SB;
+} else if (!strcmp(arg, "hv")) {
+wait_type = NBCTL_WAIT_HV;
+} else {
+return xstrdup("argument to --wait must be "
+ "\"none\", \"sb\", or \"hv\"");
+}
+break;
+
+case OPT_DRY_RUN:
+dry_run = true;
+break;
+
+case 't':
+timeout = strtoul(arg, NULL, 10);
+if (timeout < 0) {
+return xasprintf("value %s on -t or --timeout is invalid", arg);
+}
+break;
+
+default:
+*handled = false;
+break;
+}
+
+return NULL;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
-enum {
-OPT_DB = UCHAR_MAX + 1,
-OPT_NO_SYSLOG,
-OPT_NO_WAIT,
-OPT_WAIT,
-OPT_DRY_RUN,
-OPT_ONELINE,
-OPT_LOCAL,
-OPT_COMMANDS,
-OPT_OPTIONS,
-OPT_BOOTSTRAP_CA_CERT,
-VLOG_OPTION_ENUMS,
-TABLE_OPTION_ENUMS,
-SSL_OPTION_ENUMS,
-};
static const struct option global_long_options[] = {
{"db", required_argument, NULL, OPT_DB},
{"no-syslog", no_argument, NULL, OPT_NO_SYSLOG},
-{"no-wait", no_argument, NULL, OPT_NO_WAIT},
-{"wait", required_argument, NULL, OPT_WAIT},
-{"dry-run", no_argument, NULL, OPT_DRY_RUN},
-{"oneline", no_argument, NULL, OPT_ONELINE},
-{"timeout", required_argument, NULL, 't'},
{"help", no_argument, NULL, 'h'},
{"commands", no_argument, NULL, OPT_COMMANDS},
{"options", no_argument, NULL, OPT_OPTIONS},
{"leader-only", no_argument, _only, true},
{"no-leader-only", no_argument, _only, false},
{"version", no_argument, NULL, 'V'},
+MAIN_LOOP_LONG_OPTIONS,
VLOG_LONG_OPTIONS,
STREAM_SSL_LONG_OPTIONS,
{"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
@@ -268,40 +323,24 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
break;
}
+bool handled;
+char *error = handle_main_loop_option(c, optarg, );
+if (error) {
+ctl_fatal("%s", error);
+}
+if (handled) {
+continue;
+}
+
switch (c) {
case OPT_DB:
db = optarg;
break;
-case OPT_ONELINE:
-oneline = true;
-break;
-
case OPT_NO_SYSLOG:
vlog_set_levels(_module, VLF_SYSLOG, VLL_WARN);
break;
-case OPT_NO_WAIT:
-wait_type = NBCTL_WAIT_NONE;
-break;
-
-case OPT_WAIT:
-if (!strcmp(optarg, "none")) {
-wait_type = NBCTL_WAIT_NONE;
-} else if (!strcmp(optarg, "sb")) {
-wait_type = NBCTL_WAIT_SB;
-} else if (!strcmp(optarg, "hv")) {
-wait_type = NBCTL_WAIT_HV;
-} else {
-ctl_fatal("argument to
[ovs-dev] [PATCH v3 11/17] ovn-nbctl: Extract helper for printing oneline output.
This will allow us to direct oneline-formatted output to other sinks
than stdout if needed. Preparatory work for daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 53 ++-
1 file changed, 34 insertions(+), 19 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 7f83abc40..3dd24d193 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4162,6 +4162,39 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
return NULL;
}
+static void
+oneline_format(struct ds *lines, struct ds *s)
+{
+size_t j;
+
+ds_chomp(lines, '\n');
+for (j = 0; j < lines->length; j++) {
+int ch = lines->string[j];
+switch (ch) {
+case '\n':
+ds_put_cstr(s, "\\n");
+break;
+
+case '\\':
+ds_put_cstr(s, "");
+break;
+
+default:
+ds_put_char(s, ch);
+}
+}
+ds_put_char(s, '\n');
+}
+
+static void
+oneline_print(struct ds *lines)
+{
+struct ds s = DS_EMPTY_INITIALIZER;
+oneline_format(lines, );
+fputs(ds_cstr(), stdout);
+ds_destroy();
+}
+
static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl, const struct timer *wait_timeout, bool *retry)
@@ -4297,25 +4330,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
if (c->table) {
table_print(c->table, _style);
} else if (oneline) {
-size_t j;
-
-ds_chomp(ds, '\n');
-for (j = 0; j < ds->length; j++) {
-int ch = ds->string[j];
-switch (ch) {
-case '\n':
-fputs("\\n", stdout);
-break;
-
-case '\\':
-fputs("", stdout);
-break;
-
-default:
-putchar(ch);
-}
-}
-putchar('\n');
+oneline_print(ds);
} else {
fputs(ds_cstr(ds), stdout);
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 10/17] ovn-nbctl: Introduce a poll_timer based wait timeout.
Extend the main loop and the command runner so that the caller can
specify a timeout for poll_block(). This will allow us to break out of
the main loop when waiting on IDL, like in the blocked '--wait=sb/hv
sync' case.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 154e7799a..7f83abc40 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -37,6 +37,7 @@
#include "svec.h"
#include "table.h"
#include "timeval.h"
+#include "timer.h"
#include "util.h"
#include "openvswitch/vlog.h"
@@ -87,13 +88,16 @@ static char * OVS_WARN_UNUSED_RESULT
run_prerequisites(struct ctl_command[],
struct ovsdb_idl *);
static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
struct ctl_command *, size_t n,
- struct ovsdb_idl *, bool *retry);
+ struct ovsdb_idl *,
+ const struct timer *,
+ bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static char * OVS_WARN_UNUSED_RESULT main_loop(const char *args,
struct ctl_command *commands,
size_t n_commands,
- struct ovsdb_idl *idl);
+ struct ovsdb_idl *idl,
+ const struct timer *);
int
main(int argc, char *argv[])
@@ -132,7 +136,7 @@ main(int argc, char *argv[])
ctl_fatal("%s", error);
}
-error = main_loop(args, commands, n_commands, idl);
+error = main_loop(args, commands, n_commands, idl, NULL);
if (error) {
ctl_fatal("%s", error);
}
@@ -153,7 +157,7 @@ main(int argc, char *argv[])
static char *
main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl)
+ struct ovsdb_idl *idl, const struct timer *wait_timeout)
{
unsigned int seqno;
@@ -177,7 +181,8 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
seqno = ovsdb_idl_get_seqno(idl);
bool retry;
-char *error = do_nbctl(args, commands, n_commands, idl, );
+char *error = do_nbctl(args, commands, n_commands, idl,
+ wait_timeout, );
if (error) {
return error;
}
@@ -4159,7 +4164,7 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl, bool *retry)
+ struct ovsdb_idl *idl, const struct timer *wait_timeout, bool *retry)
{
struct ovsdb_idl_txn *txn;
enum ovsdb_idl_txn_status status;
@@ -4286,8 +4291,6 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
OVS_NOT_REACHED();
}
-ovsdb_symbol_table_destroy(symtab);
-
for (c = commands; c < [n_commands]; c++) {
struct ds *ds = >output;
@@ -4331,11 +4334,19 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
}
}
ovsdb_idl_wait(idl);
+if (wait_timeout) {
+timer_wait(wait_timeout);
+}
poll_block();
+if (wait_timeout && timer_expired(wait_timeout)) {
+error = xstrdup("timeout expired");
+goto out_error;
+}
}
done: ;
}
+ovsdb_symbol_table_destroy(symtab);
ovsdb_idl_txn_destroy(txn);
*retry = false;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 09/17] ovn-nbctl: Propagate errors from prerequisites runner.
Instead of terminating the process, return the error to the caller.
This will allow us to reuse the prerequisites runner in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 747aa63b6..154e7799a 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -82,8 +82,9 @@ static int leader_only = true;
static void nbctl_cmd_init(void);
OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
-static void run_prerequisites(struct ctl_command[], size_t n_commands,
- struct ovsdb_idl *);
+static char * OVS_WARN_UNUSED_RESULT run_prerequisites(struct ctl_command[],
+ size_t n_commands,
+ struct ovsdb_idl *);
static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
struct ctl_command *, size_t n,
struct ovsdb_idl *, bool *retry);
@@ -101,6 +102,7 @@ main(int argc, char *argv[])
struct ctl_command *commands;
struct shash local_options;
size_t n_commands;
+char *error;
set_program_name(argv[0]);
fatal_ignore_sigpipe();
@@ -125,9 +127,12 @@ main(int argc, char *argv[])
/* Initialize IDL. */
idl = the_idl = ovsdb_idl_create(db, _idl_class, true, false);
ovsdb_idl_set_leader_only(idl, leader_only);
-run_prerequisites(commands, n_commands, idl);
+error = run_prerequisites(commands, n_commands, idl);
+if (error) {
+ctl_fatal("%s", error);
+}
-char *error = main_loop(args, commands, n_commands, idl);
+error = main_loop(args, commands, n_commands, idl);
if (error) {
ctl_fatal("%s", error);
}
@@ -4117,7 +4122,7 @@ static const struct ctl_table_class
tables[NBREC_N_TABLES] = {
[NBREC_TABLE_ACL].row_ids[0] = {_acl_col_name, NULL, NULL},
};
-static void
+static char *
run_prerequisites(struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl)
{
@@ -4138,7 +4143,9 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
ctl_context_init(, c, idl, NULL, NULL, NULL);
(c->syntax->prerequisites)();
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+char *error = xstrdup(ctx.error);
+ctl_context_done(, c);
+return error;
}
ctl_context_done(, c);
@@ -4146,6 +4153,8 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
ovs_assert(!c->table);
}
}
+
+return NULL;
}
static char *
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 08/17] ovn-nbctl: Propagate errors from the main loop.
Let the caller handle the errors instead of reporting it and
terminating. Prepare for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 6e136e1d0..747aa63b6 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -89,8 +89,10 @@ static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char
*args,
struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
-static void main_loop(const char *args, struct ctl_command *commands,
- size_t n_commands, struct ovsdb_idl *idl);
+static char * OVS_WARN_UNUSED_RESULT main_loop(const char *args,
+ struct ctl_command *commands,
+ size_t n_commands,
+ struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
@@ -125,7 +127,10 @@ main(int argc, char *argv[])
ovsdb_idl_set_leader_only(idl, leader_only);
run_prerequisites(commands, n_commands, idl);
-main_loop(args, commands, n_commands, idl);
+char *error = main_loop(args, commands, n_commands, idl);
+if (error) {
+ctl_fatal("%s", error);
+}
ovsdb_idl_destroy(idl);
idl = the_idl = NULL;
@@ -141,7 +146,7 @@ main(int argc, char *argv[])
exit(EXIT_SUCCESS);
}
-static void
+static char *
main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl)
{
@@ -169,10 +174,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
bool retry;
char *error = do_nbctl(args, commands, n_commands, idl, );
if (error) {
-ctl_fatal("%s", error);
+return error;
}
if (!retry) {
-return;
+return NULL;
}
}
@@ -181,6 +186,8 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
poll_block();
}
}
+
+return NULL;
}
static void
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 07/17] ovn-nbctl: Propagate the error from do_nbctl().
Instead of terminating the process, return the error to the caller.
This will allow us to reuse the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 46 --
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 360b25a89..6e136e1d0 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -84,8 +84,9 @@ OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
static void run_prerequisites(struct ctl_command[], size_t n_commands,
struct ovsdb_idl *);
-static void do_nbctl(const char *args, struct ctl_command *, size_t n,
- struct ovsdb_idl *, bool *retry);
+static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
+ struct ctl_command *, size_t n,
+ struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static void main_loop(const char *args, struct ctl_command *commands,
@@ -166,7 +167,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
seqno = ovsdb_idl_get_seqno(idl);
bool retry;
-do_nbctl(args, commands, n_commands, idl, );
+char *error = do_nbctl(args, commands, n_commands, idl, );
+if (error) {
+ctl_fatal("%s", error);
+}
if (!retry) {
return;
}
@@ -4137,7 +4141,7 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
}
}
-static void
+static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl, bool *retry)
{
@@ -4148,6 +4152,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
struct ctl_command *c;
struct shash_node *node;
int64_t next_cfg = 0;
+char *error = NULL;
ovs_assert(retry);
@@ -4181,7 +4186,9 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
(c->syntax->run)();
}
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+error = xstrdup(ctx.error);
+ctl_context_done(, c);
+goto out_error;
}
ctl_context_done_command(, c);
@@ -4195,9 +4202,10 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
SHASH_FOR_EACH (node, >sh) {
struct ovsdb_symbol *symbol = node->data;
if (!symbol->created) {
-ctl_fatal("row id \"%s\" is referenced but never created (e.g. "
- "with \"-- --id=%s create ...\")",
- node->name, node->name);
+error = xasprintf("row id \"%s\" is referenced but never created "
+ "(e.g. with \"-- --id=%s create ...\")",
+ node->name, node->name);
+goto out_error;
}
if (!symbol->strong_ref) {
if (!symbol->weak_ref) {
@@ -4222,7 +4230,9 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
ctl_context_init(, c, idl, txn, symtab, NULL);
(c->syntax->postprocess)();
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+error = xstrdup(ctx.error);
+ctl_context_done(, c);
+goto out_error;
}
ctl_context_done(, c);
}
@@ -4236,7 +4246,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
case TXN_ABORTED:
/* Should not happen--we never call ovsdb_idl_txn_abort(). */
-ctl_fatal("transaction aborted");
+error = xstrdup("transaction aborted");
+goto out_error;
case TXN_UNCHANGED:
case TXN_SUCCESS:
@@ -4246,11 +4257,14 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
goto try_again;
case TXN_ERROR:
-ctl_fatal("transaction error: %s", ovsdb_idl_txn_get_error(txn));
+error = xasprintf("transaction error: %s",
+ ovsdb_idl_txn_get_error(txn));
+goto out_error;
case TXN_NOT_LOCKED:
/* Should not happen--we never call ovsdb_idl_set_lock(). */
-ctl_fatal("database not locked");
+error = xstrdup("database not locked");
+goto out_error;
default:
OVS_NOT_REACHED();
@@ -4309,11 +4323,14 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
ovsdb_idl_txn_destroy(txn);
*retry = false;
-return;
+return NULL;
try_again:
/* Our
[ovs-dev] [PATCH v3 06/17] ovn-nbctl: Signal need to try again via an output param.
Introduce an output parameter for the flag that signals need to retry
running the command. This leaves the return value for error reporting.
Preparatory work for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index a700695fe..360b25a89 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -84,8 +84,8 @@ OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
static void run_prerequisites(struct ctl_command[], size_t n_commands,
struct ovsdb_idl *);
-static bool do_nbctl(const char *args, struct ctl_command *, size_t n,
- struct ovsdb_idl *);
+static void do_nbctl(const char *args, struct ctl_command *, size_t n,
+ struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static void main_loop(const char *args, struct ctl_command *commands,
@@ -164,7 +164,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
if (seqno != ovsdb_idl_get_seqno(idl)) {
seqno = ovsdb_idl_get_seqno(idl);
-if (do_nbctl(args, commands, n_commands, idl)) {
+
+bool retry;
+do_nbctl(args, commands, n_commands, idl, );
+if (!retry) {
return;
}
}
@@ -4134,9 +4137,9 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
}
}
-static bool
+static void
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl)
+ struct ovsdb_idl *idl, bool *retry)
{
struct ovsdb_idl_txn *txn;
enum ovsdb_idl_txn_status status;
@@ -4146,6 +4149,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
struct shash_node *node;
int64_t next_cfg = 0;
+ovs_assert(retry);
+
txn = the_idl_txn = ovsdb_idl_txn_create(idl);
if (dry_run) {
ovsdb_idl_txn_set_dry_run(txn);
@@ -4303,7 +4308,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
ovsdb_idl_txn_destroy(txn);
-return true;
+*retry = false;
+return;
try_again:
/* Our transaction needs to be rerun, or a prerequisite was not met. Free
@@ -4318,7 +4324,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
table_destroy(c->table);
free(c->table);
}
-return false;
+*retry = true;
}
/* Frees the current transaction and the underlying IDL and then calls
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 05/17] ovn-nbctl: Pull up releasing IDL from do_nbctl().
Destroy IDL resources in the routine where we allocated them.
Preparatory work for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 51527741b..a700695fe 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -126,6 +126,9 @@ main(int argc, char *argv[])
main_loop(args, commands, n_commands, idl);
+ovsdb_idl_destroy(idl);
+idl = the_idl = NULL;
+
for (struct ctl_command *c = commands; c < [n_commands]; c++) {
ds_destroy(>output);
table_destroy(c->table);
@@ -4299,7 +4302,6 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
}
ovsdb_idl_txn_destroy(txn);
-ovsdb_idl_destroy(idl);
return true;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 04/17] ovn-nbctl: Pull up destroying commands from do_nbctl().
Destroy commands in the same routine where they were allocated.
Preparatory work for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index a027553b7..51527741b 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -126,6 +126,13 @@ main(int argc, char *argv[])
main_loop(args, commands, n_commands, idl);
+for (struct ctl_command *c = commands; c < [n_commands]; c++) {
+ds_destroy(>output);
+table_destroy(c->table);
+free(c->table);
+shash_destroy_free_data(>options);
+}
+free(commands);
free(args);
exit(EXIT_SUCCESS);
}
@@ -4271,13 +4278,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
} else {
fputs(ds_cstr(ds), stdout);
}
-ds_destroy(>output);
-table_destroy(c->table);
-free(c->table);
-
-shash_destroy_free_data(>options);
}
-free(commands);
if (wait_type != NBCTL_WAIT_NONE && status != TXN_UNCHANGED) {
ovsdb_idl_enable_reconnect(idl);
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 03/17] ovn-nbctl: Extract the main loop.
Split out a routine for the main ovn-nbctl loop.
Preparatory work for introducing daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 47df19b23..a027553b7 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -88,6 +88,8 @@ static bool do_nbctl(const char *args, struct ctl_command *,
size_t n,
struct ovsdb_idl *);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
+static void main_loop(const char *args, struct ctl_command *commands,
+ size_t n_commands, struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
@@ -95,7 +97,6 @@ main(int argc, char *argv[])
struct ovsdb_idl *idl;
struct ctl_command *commands;
struct shash local_options;
-unsigned int seqno;
size_t n_commands;
set_program_name(argv[0]);
@@ -123,6 +124,18 @@ main(int argc, char *argv[])
ovsdb_idl_set_leader_only(idl, leader_only);
run_prerequisites(commands, n_commands, idl);
+main_loop(args, commands, n_commands, idl);
+
+free(args);
+exit(EXIT_SUCCESS);
+}
+
+static void
+main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
+ struct ovsdb_idl *idl)
+{
+unsigned int seqno;
+
/* Execute the commands.
*
* 'seqno' is the database sequence number for which we last tried to
@@ -136,14 +149,13 @@ main(int argc, char *argv[])
if (!ovsdb_idl_is_alive(idl)) {
int retval = ovsdb_idl_get_last_error(idl);
ctl_fatal("%s: database connection failed (%s)",
-db, ovs_retval_to_string(retval));
+ db, ovs_retval_to_string(retval));
}
if (seqno != ovsdb_idl_get_seqno(idl)) {
seqno = ovsdb_idl_get_seqno(idl);
if (do_nbctl(args, commands, n_commands, idl)) {
-free(args);
-exit(EXIT_SUCCESS);
+return;
}
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 02/17] ovsdb-idl: Allow monitoring columns that are already monitored.
If IDL was created with monitoring and alerts turned on by default for
all columns, then there is no harm in allowing the API users to ask
again for monitoring and alerts to be enabled for any given column.
This allows us to run prerequisites handlers for db-ctl and ovn-nbctl
commands once the IDL has already ran once.
Signed-off-by: Jakub Sitnicki
---
lib/ovsdb-idl.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 9ab5d6723..ae0a55c3a 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1116,6 +1116,20 @@ ovsdb_idl_db_get_mode(struct ovsdb_idl_db *db,
return >modes[column - table->class_->columns];
}
+static void
+ovsdb_idl_db_set_mode(struct ovsdb_idl_db *db,
+ const struct ovsdb_idl_column *column,
+ unsigned char mode)
+{
+const struct ovsdb_idl_table *table = ovsdb_idl_table_from_column(db,
+ column);
+size_t column_idx = column - table->class_->columns;
+
+if (table->modes[column_idx] != mode) {
+*ovsdb_idl_db_get_mode(db, column) = mode;
+}
+}
+
static void
add_ref_table(struct ovsdb_idl_db *db, const struct ovsdb_base_type *base)
{
@@ -1136,7 +1150,7 @@ static void
ovsdb_idl_db_add_column(struct ovsdb_idl_db *db,
const struct ovsdb_idl_column *column)
{
-*ovsdb_idl_db_get_mode(db, column) = OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT;
+ovsdb_idl_db_set_mode(db, column, OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT);
add_ref_table(db, >type.key);
add_ref_table(db, >type.value);
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 01/17] table: Introduce a constant for default table style.
Having a constant in addition to the constant expression for the default
table style allows us to reset 'struct table_style' variables to default
style.
Signed-off-by: Jakub Sitnicki
---
lib/table.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/table.h b/lib/table.h
index 313ac1dd2..76e65bb70 100644
--- a/lib/table.h
+++ b/lib/table.h
@@ -20,6 +20,7 @@
#include
#include
#include "compiler.h"
+#include "openvswitch/json.h"
struct ds;
struct table_style;
@@ -83,6 +84,7 @@ struct table_style {
};
#define TABLE_STYLE_DEFAULT { TF_LIST, CF_STRING, true, JSSF_SORT, 0 }
+static const struct table_style table_style_default = TABLE_STYLE_DEFAULT;
#define TABLE_OPTION_ENUMS \
OPT_NO_HEADINGS,\
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 00/17] Daemon mode for ovn-nbctl
This series extends ovn-nbctl tool with support for the daemon mode, where
ovn-nbctl acts a long-lived process that accepts commands over a UNIX socket.
The daemon can be started the same way as any other OVS/OVN server:
ovn-nbctl --detach --pidfile --log-file
While commands can be issued to it using the 'ovs-appctl' tool:
ovs-appctl -t ovn-nbctl run [OPTIONS] COMMAND [-- [OPTIONS] COMMAND] ...
Although, the plan for future work is to extend ovn-nbctl so that it can control
the daemon, if one is running.
The motivation and the main benefit from the daemon mode is that the contents of
NB database have to be obtained only once, when the first command is ran. On big
databases (1000's of logical ports) this results in a speed up per command in
the range of 100's of milliseconds.
The changes are functional to the point that all test cases in the ovn-nbctl
test suite (tests/ovn-nbctl.at) and OVN test suite (tests/ovn.at) pass when
running ovn-nbctl as a daemon [1].
However, I'm still thinking how to nicely integrate the daemon mode with the
test suite so that the existing tests can be run using either the normal or the
daemon mode. Any ideas?
A dirty, just-for-development integration with tests is available at:
https://github.com/jsitnicki/ovs/commits/wip-nbctl-daemon-2018-07-13
Also, not all places that call ctl_fatal() have been converted yet to propagate
the error to the caller. As a result hitting an unconverted an error path will
cause the daemon process to die. This can be worked around with the --monitor
option.
Daemon mode should be considered experimental.
Changes v2 -> v3:
- Wait for IDL to connect before detaching and running the unixctl server. Also,
terminate the daemon when the connection to the NB DB has failed. This
behavior is modeled after ovn-trace daemon mode.
- Add a dedicated option parser for the daemon. Parse only the options that are
have an effect on the main loop. Logging options are not supported. Usual way
to change the log levels for daemons is with 'vlog/set' command. Reported by
Mark Michelson.
- Drop the test for 'sync' command. The test is flawed as pointed out by Mark
Michelson. Reading the '*_cfg' value after 'ovn-nbctl --wait=X sync' has
returned does not prove that '--wait' has blocked until the '*_cfg' value has
changed. At the same time, we cannot read out the updated '*_cfg' value in the
same transaction as we change it in.
- To review just the interdiff run:
git fetch --tags https://github.com/jsitnicki/ovs
git diff wip-nbctl-daemon-2018-07-{12,13}
Changes v1 -> v2:
- Work around a limitation in GCC 4.8.5 (RHEL/CentOS 7.5) that doesn't let us
use a constant expression as a RHS value in a structure assigment. Found by
the 0-day bot.
Changes RFC -> v1:
- Rebase onto master @ 61b1c7acb9a2 ("netdev-bsd: Fix crash on FreeBSD.").
- Add support for commands that use tabular output ('find' and 'list') thanks to
recent work in table module by Ben Pfaff. This includes support for options
that control table formatting.
- Run prerequisites routines. In the end this is needed to support the 'sync'
command, which itself is more like an option than command (has to get
processed before the commands run). This is also the reason for minor changes
in the IDL.
- Add support for --dry-run, --wait / --no-wait, --timeout, and --oneline
options. Timeout handling is different than in the normal mode - we only time
out in poll_block(). Still, it serves the purpose avoiding an infinite 'sync'.
- Add tests for ovn-nbctl 'sync' command, dry-run mode, and oneline-formatted
output.
- Extend the ovn-nbctl man-page with description of daemon mode.
- Remove extraneous return at the end of a void function. Pointed out by Ben
Pfaff.
- Drop WIP patch for integration with tests from the series until a right
approach can be found. Integration with tests that was used during development
is available at:
https://github.com/jsitnicki/ovs/commits/wip-nbctl-daemon-2018-07-11
- Minor style cleanups.
Thanks,
Jakub
[1] Except test "2563: ovn -- IPv6 ND Router Solicitation responder" which seems
broken in master @ 61b1c7acb9a2 ("netdev-bsd: Fix crash on FreeBSD.").
Jakub Sitnicki (17):
table: Introduce a constant for default table style.
ovsdb-idl: Allow monitoring columns that are already monitored.
ovn-nbctl: Extract the main loop.
ovn-nbctl: Pull up destroying commands from do_nbctl().
ovn-nbctl: Pull up releasing IDL from do_nbctl().
ovn-nbctl: Signal need to try again via an output param.
ovn-nbctl: Propagate the error from do_nbctl().
ovn-nbctl: Propagate errors from the main loop.
ovn-nbctl: Propagate errors from prerequisites runner.
ovn-nbctl: Introduce a poll_timer based wait timeout.
ovn-nbctl: Extract helper for printing oneline output.
ovn-nbctl: Extract handling of options that affect main loop.
ovn-nbctl: Extract a helper for building short options string.
ovn-nbctl: Extract a
Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for master
Bleep bloop. Greetings Ian Stokes, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Co-authored-by/Signed-off-by corruption WARNING: Line has trailing whitespace #194 FILE: rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh:19: # built using the rhel6 spec file, and run in the post-install for minor WARNING: Line has trailing whitespace #205 FILE: rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh:30: # Lines checked: 270, Warnings: 2, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 08/14] dp-packet: Handle multi-seg mbufs in resize__().
Thanks for the patch.
A few queries inline.
On Wed, Jul 11, 2018 at 11:23 AM, Tiago Lam wrote:
> When enabled with DPDK OvS relies on mbufs allocated by mempools to
> receive and output data on DPDK ports. Until now, each OvS dp_packet has
> had only one mbuf associated, which is allocated with the maximum
> possible size, taking the MTU into account. This approach, however,
> doesn't allow us to increase the allocated size in an mbuf, if needed,
> since an mbuf is allocated and initialised upon mempool creation. Thus,
> in the current implementatin this is dealt with by calling
> OVS_NOT_REACHED() and terminating OvS.
>
> To avoid this, and allow the (already) allocated space to be better
> used, dp_packet_resize__() now tries to use the available room, both the
> tailroom and the headroom, to make enough space for the new data. Since
> this happens for packets of source DPBUF_DPDK, the single-segment mbuf
> case mentioned above is also covered by this new aproach in resize__().
>
> Signed-off-by: Tiago Lam
> Acked-by: Eelco Chaudron
> ---
> lib/dp-packet.c | 48 ++--
> 1 file changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index d6e19eb..87af459 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -237,9 +237,51 @@ dp_packet_resize__(struct dp_packet *b, size_t
> new_headroom, size_t new_tailroom
> new_allocated = new_headroom + dp_packet_size(b) + new_tailroom;
>
> switch (b->source) {
> +/* When resizing mbufs, both a single mbuf and multi-segment mbufs
> (where
> + * data is not contigously held in memory), both the headroom and the
> + * tailroom available will be used to make more space for where data
> needs
> + * to be inserted. I.e if there's not enough headroom, data may be
> shifted
> + * right if there's enough tailroom.
> + * However, this is not bulletproof and in some cases the space
> available
> + * won't be enough - in those cases, an error should be returned and
> the
> + * packet dropped. */
> case DPBUF_DPDK:
> -OVS_NOT_REACHED();
>
Previously, it was a coding error to call this function for a DPDK mbuf
case, which is pretty
clear. But with this patch, presumably that is not longer the case and the
calling the API is
now ok for DPDK mbufs.
> +{
> +size_t miss_len;
> +
> +if (new_headroom == dp_packet_headroom(b)) {
> +/* This is a tailroom adjustment. Since there's no tailroom
> space
> + * left, try and shift data towards the head to free up tail
> space,
> + * if there's enough headroom */
> +
> +miss_len = new_tailroom - dp_packet_tailroom(b);
> +
> +if (miss_len <= new_headroom) {
> +dp_packet_shift(b, -miss_len);
> +} else {
> +/* XXX: Handle error case and report error to caller */
> +OVS_NOT_REACHED();
>
This will not just drop the packet, it will fail the daemon, because a
packet cannot be resized.
If the system is completely depleted of memory, that may be ok, but in the
case, no such
assumption is implied.
Also, why is XXX still left in the patch?
Also, the preexisting API handles two cases:
1/ Tailroom only adjustment
2/ headroom and/or tailroom adjustment
meaning it handles all cases.
The new DPDK addition (part of the same API) defines 2 cases
1/ tailroom only adjustment
2/ headroom only adjustment
So, it looks like a different API, that also does not handle all cases.
> +}
> +} else {
> +/* Otherwise, this is a headroom adjustment. Try to shift data
> + * towards the tail to free up head space, if there's enough
> + * tailroom */
> +
> +miss_len = new_headroom - dp_packet_headroom(b);
>
> +
> +if (miss_len <= new_tailroom) {
> +dp_packet_shift(b, miss_len);
> +} else {
> +/* XXX: Handle error case and report error to caller */
> +OVS_NOT_REACHED();
>
same comments as above.
> +}
> +}
> +
> +new_base = dp_packet_base(b);
> +
> +break;
> +}
> case DPBUF_MALLOC:
> if (new_headroom == dp_packet_headroom(b)) {
> new_base = xrealloc(dp_packet_base(b), new_allocated);
> @@ -263,7 +305,9 @@ dp_packet_resize__(struct dp_packet *b, size_t
> new_headroom, size_t new_tailroom
> OVS_NOT_REACHED();
> }
>
> -dp_packet_set_allocated(b, new_allocated);
> +if (b->source != DPBUF_DPDK) {
> +dp_packet_set_allocated(b, new_allocated);
> +}
> dp_packet_set_base(b, new_base);
>
> new_data = (char *) new_base + new_headroom;
> --
> 2.7.4
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
Re: [ovs-dev] [PATCH] netdev-dpdk: Fix failure to configure flow control at netdev-init.
On 7/10/2018 2:23 PM, Sugesh Chandran wrote:
Configuring flow control at ixgbe netdev-init is throwing error in port
start.
For eg: without this fix, user cannot configure flow control on ixgbe dpdk
port as below,
"
ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
options:dpdk-devargs=:05:00.1 options:rx-flow-ctrl=true
"
Instead, it must be configured as two different commands,
"
ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
options:dpdk-devargs=:05:00.1
ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true
"
The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' fields before
trying to configuring the dpdk ethdev. Hence OVS can no longer set the
'dont care' fields to just '0' as before. This commit make sure all the
'rte_eth_fc_conf' fields are populated with default values before the dev
init.
Thanks for the patch Sugesh, I've applied to dpdk_merge and it will be
part of this weeks pull request.
I assume it should be backported to OVS 2.9 at least, do you know if it
should go to 2.8/2.7 also?
Ian
Signed-off-by: Sugesh Chandran
---
lib/netdev-dpdk.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index bb4d60f26..023b80d3e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1065,14 +1065,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp);
dev->buf_size = mbp_priv->mbuf_data_room_size - RTE_PKTMBUF_HEADROOM;
-
-/* Get the Flow control configuration for DPDK-ETH */
-diag = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
-if (diag) {
-VLOG_DBG("cannot get flow control parameters on port "DPDK_PORT_ID_FMT
- ", err=%d", dev->port_id, diag);
-}
-
return 0;
}
@@ -1773,6 +1765,13 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
autoneg = smap_get_bool(args, "flow-ctrl-autoneg", false);
fc_mode = fc_mode_set[tx_fc_en][rx_fc_en];
+/* Get the Flow control configuration for DPDK-ETH */
+err = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
+if (err) {
+VLOG_INFO("cannot get flow control parameters on port "DPDK_PORT_ID_FMT
+ ", err=%d", dev->port_id, err);
+}
+
if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg) {
dev->fc_conf.mode = fc_mode;
dev->fc_conf.autoneg = autoneg;
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5] dpif-netdev: Avoid reordering of packets in a batch with same megaflow
On 7/13/2018 2:54 PM, Vishal Deep Ajmera wrote: Hi Ian, Ilya, If there are no more comments, can I request to please include the fix in this week's pull request? Hi Vishal, I'll leave Ilya to respond to the changes he requested. This weeks pull request includes the new feature 'SMC cache after EMC cache' (Collaboration work between Intel and Ericsson). Your patch will have to be rebased on top of this as there is a lot of code churn. If you could rebase the v6 after the pull request is merged it would be helpful. I'm aiming to have your patch as part of the next pull request targeted for early next week. The v6 will be specific for master to account for the SMC changes. I should be able to use the v5 to apply to 2.9 and previous releases if there is no other changes requested from Ilya. Thanks Ian Warm Regards, Vishal Ajmera ___ 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] OVS DPDK: dpdk_merge pull request for master
Hi Ben, The following changes since commit 89dd5819cf181a741271d297bc99fea4760f7ba5: rhel: support kmod-openvswitch build against multiple kernels, rhel6 (2018-07-12 17:42:07 -0700) are available in the git repository at: https://github.com/istokes/ovs dpdk_merge for you to fetch changes up to 70f4d53a17d953b0fadf18361b54ce95550ebfb7: netdev-dpdk: Fix failure to configure flow control at netdev-init. (2018-07-13 17:08:56 +0100) Ian Stokes (1): Docs: Improve OVS DPDK version mapping notice. Mark Kavanagh (4): netdev-dpdk: fix mbuf sizing dp-packet: Init specific mbuf fields. netdev-dpdk: copy large packet to multi-seg. mbufs netdev-dpdk: support multi-segment jumbo frames Michael Qiu (1): dp-packet: copy data from multi-seg. DPDK mbuf Sugesh Chandran (1): netdev-dpdk: Fix failure to configure flow control at netdev-init. Tiago Lam (9): dp-packet: Fix allocated size on DPDK init. netdev-dpdk: Serialise non-pmds mbufs' alloc/free. dp-packet: Fix data_len handling multi-seg mbufs. dp-packet: Handle multi-seg mbufs in helper funcs. dp-packet: Handle multi-seg mubfs in shift() func. dp-packet: Handle multi-seg mbufs in resize__(). dpdk-tests: Add uni-tests for multi-seg mbufs. dpdk-tests: Accept other configs in OVS_DPDK_START dpdk-tests: End-to-end tests for multi-seg mbufs. Yipeng Wang (1): dpif-netdev: Add SMC cache after EMC cache Documentation/howto/dpdk.rst | 6 ++- Documentation/intro/install/dpdk.rst | 6 ++- Documentation/topics/dpdk/bridge.rst | 15 ++ Documentation/topics/dpdk/jumbo-frames.rst | 52 +++ Documentation/topics/dpdk/memory.rst | 36 + NEWS | 3 ++ lib/cmap.c | 74 +++ lib/cmap.h | 11 lib/dp-packet.c| 221 +-- lib/dp-packet.h| 214 + lib/dpdk.c | 8 +++ lib/dpif-netdev-perf.h | 1 + lib/dpif-netdev.c | 329 -- lib/netdev-dpdk.c | 259 - lib/netdev-dpdk.h | 2 + tests/automake.mk | 10 +++- tests/dpdk-packet-mbufs.at | 7 +++ tests/pmd.at | 7 ++- tests/system-dpdk-macros.at| 6 ++- tests/system-dpdk-testsuite.at | 1 + tests/system-dpdk.at | 65 tests/test-dpdk-mbufs.c| 518 ++ vswitchd/vswitch.xml | 35 + 23 files changed, 1756 insertions(+), 130 deletions(-) create mode 100644 tests/dpdk-packet-mbufs.at create mode 100644 tests/test-dpdk-mbufs.c Thanks Ian ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] configure: Enable GCC relevant new 8.x warning options.
Ben Pfaff writes: > These don't trigger any new actual warnings in my own build. > > GCC 8.x adds other new warning options that are enabled by -Wall or > -Wextra. This commit doesn't explicitly enable those because OVS already > enables -Wall and -Wextra. > > Signed-off-by: Ben Pfaff > --- LGTM. Sorry it took so long to review. Acked-by: Aaron Conole ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 00/14] Support multi-segment mbufs
On 7/11/2018 7:23 PM, Tiago Lam wrote:
Overview
This patchset introduces support for multi-segment mbufs to OvS-DPDK.
Multi-segment mbufs are typically used when the size of an mbuf is
insufficient to contain the entirety of a packet's data. Instead, the
data is split across numerous mbufs, each carrying a portion, or
'segment', of the packet data. mbufs are chained via their 'next'
attribute (an mbuf pointer).
Thanks to all for the work on this series. I've pushed this to
dpdk_merge and it will be part of the pull request this week.
Ian
Use Cases
=
i. Handling oversized (guest-originated) frames, which are marked
for hardware accelration/offload (TSO, for example).
Packets which originate from a non-DPDK source may be marked for
offload; as such, they may be larger than the permitted ingress
interface's MTU, and may be stored in an oversized dp-packet. In
order to transmit such packets over a DPDK port, their contents
must be copied to a DPDK mbuf (via dpdk_do_tx_copy). However, in
its current implementation, that function only copies data into
a single mbuf; if the space available in the mbuf is exhausted,
but not all packet data has been copied, then it is lost.
Similarly, when cloning a DPDK mbuf, it must be considered
whether that mbuf contains multiple segments. Both issues are
resolved within this patchset.
ii. Handling jumbo frames.
While OvS already supports jumbo frames, it does so by increasing
mbuf size, such that the entirety of a jumbo frame may be handled
in a single mbuf. This is certainly the preferred, and most
performant approach (and remains the default).
Enabling multi-segment mbufs
Multi-segment and single-segment mbufs are mutually exclusive, and the
user must decide on which approach to adopt on init. The introduction
of a new OVSDB field, 'dpdk-multi-seg-mbufs', facilitates this.
This is a global boolean value, which determines how jumbo frames are
represented across all DPDK ports. In the absence of a user-supplied
value, 'dpdk-multi-seg-mbufs' defaults to false, i.e. multi-segment
mbufs must be explicitly enabled / single-segment mbufs remain the
default.
Setting the field is identical to setting existing DPDK-specific OVSDB
fields:
ovs-vsctl set Open_vSwitch . other_config:dpdk-init=true
ovs-vsctl set Open_vSwitch . other_config:dpdk-lcore-mask=0x10
ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-mem=4096,0
==> ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true
Performance notes (based on v8)
=
In order to test for regressions in performance, tests were run on top
of master 88125d6 and v8 of this patchset, both with the multi-segment
mbufs option enabled and disabled.
VSperf was used to run the phy2phy_cont and pvp_cont tests with varying
packet sizes of 64B, 1500B and 7000B, on a 10Gbps interface.
Test | Size | Master | Multi-seg disabled | Multi-seg enabled
-
p2p | 64 | ~22.7 | ~22.65| ~18.3
p2p | 1500 | ~1.6 |~1.6|~1.6
p2p | 7000 | ~0.36 | ~0.36| ~0.36
pvp | 64 | ~6.7 |~6.7|~6.3
pvp | 1500 | ~1.6 |~1.6|~1.6
pvp | 7000 | ~0.36 | ~0.36| ~0.36
Packet size is in bytes, while all packet rates are reported in mpps
(aggregated).
No noticeable regression has been observed (certainly everything is
within the ± 5% margin of existing performance), aside from the 64B
packet size case when multi-segment mbuf is enabled. This is
expected, however, because of how Tx vectoriszed functions are
incompatible with multi-segment mbufs on some PMDs. The PMD under
use during these tests was the i40e (on a Intel X710 NIC), which
indeed doesn't support vectorized Tx functions with multi-segment
mbufs.
---
v5: - Rebase on master 030958a0cc ("conntrack: Fix conn_update_state_alg use
after free.");
- Address Eelco's comments:
- Remove dpdk_mp_sweep() call in netdev_dpdk_mempool_configure(), a
leftover from rebase. Only call should be in dpdk_mp_get();
- Remove NEWS line added by mistake during rebase (about adding
experimental vhost zero copy support).
- Address Ian's comments:
- Drop patch 01 from previous series entirely;
- Patch (now) 01/14 adds a new call to dpdk_buf_size() inside
dpdk_mp_create() to get the correct "mbuf_size" to be used;
- Patch (now) 11/14 modifies dpdk_mp_create() to check if multi-segment
mbufs is enabled, in which case it calculates the new "mbuf_size" to be
used;
- In free_dpdk_buf() and dpdk_buf_alloc(), don't lock and unlock
conditionally.
- Add "per-port-memory=true" to test "Multi-segment mbufs Tx" as the
current
DPDK set up in
Re: [ovs-dev] [PATCH] Docs: Improve OVS DPDK version mapping notice.
On Thu, Jul 12, 2018 at 07:48:42PM +0100, Ian Stokes wrote: > A common issue is users pairing the incorrect version of OVS to DPDK > when working outside of the build tree. > > To avoid this this commit updates the OVS DPDK documentation to explicitly > flag that users should consult the OVS to DPDK release mapping in FAQ if > working outside of the OVS build tree. > > Suggested-by: Ben Pfaff > Signed-off-by: Ian Stokes > --- Acked-by: Flavio Leitner ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5] dpif-netdev: Avoid reordering of packets in a batch with same megaflow
Hi Ian, Ilya, If there are no more comments, can I request to please include the fix in this week's pull request? Warm Regards, Vishal Ajmera ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] tests: Ignore recirc_id in "MPLS xlate action" test.
Ben Pfaff writes: > When I run this test with DPDK enabled, it fails because it ends up using > a different recirculation ID when DPDK is not enabled. I guess that's a > little weird but the recirculation IDs are not supposed to be significant, > so this change makes the test ignore it. > > Signed-off-by: Ben Pfaff > --- Thanks for this patch! I'm pretty sure that with this patch applied, I can re-enable the make distcheck for the bot. I looked at this test a bit earlier, but didn't know if the recirc ID was important. Thanks for detailing that. After this gets applied, I'll wait a bit and see if the tests consistently pass. If that's so, I'll re-enable the make distcheck in the bot for output. Acked-by: Aaron Conole ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/2] test-unixctl.py: Don't suppress exceptions.
On Sat, Jun 16, 2018 at 3:42 AM Ben Pfaff wrote:
> A user reported a failure of test 2364 "vlog - RFC5424 facility - Python2"
> with an exit code that says that the test-unixctl process died from an
> uncaught exception. Unfortunately the exception didn't show up in the log.
> This commit should make the exception show up (it deletes some boilerplate
> we use in our Python-based daemons to make them restart themselves on
> failure, which isn't needed or appropriate for a test script).
>
> Reported-by: Sanket Sudake
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-May/046840.html
> Signed-off-by: Ben Pfaff
>
Acked-by: Numan Siddique
> ---
> tests/test-unixctl.py | 10 +-
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/tests/test-unixctl.py b/tests/test-unixctl.py
> index 5de51d31ecfd..4fa27b09f82d 100644
> --- a/tests/test-unixctl.py
> +++ b/tests/test-unixctl.py
> @@ -13,7 +13,6 @@
> # limitations under the License.
>
> import argparse
> -import sys
>
> import ovs.daemon
> import ovs.unixctl
> @@ -88,11 +87,4 @@ def main():
>
>
> if __name__ == '__main__':
> -try:
> -main()
> -except SystemExit:
> -# Let system.exit() calls complete normally
> -raise
> -except:
> -vlog.exception("traceback")
> -sys.exit(ovs.daemon.RESTART_EXIT_CODE)
> +main()
> --
> 2.16.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 1/2] ovsdb-server: Don't log closing session at program termination.
On Fri, Jul 13, 2018 at 5:04 AM Ben Pfaff wrote:
> This series still needs reviews.
>
> On Fri, Jun 15, 2018 at 03:11:09PM -0700, Ben Pfaff wrote:
> > When ovsdb-server closes a remote connection, it logs a message about it
> > that includes the reason. Until now this has included sessions that it
> > closes when it exits. That meant that, when --run was used, there was a
> > race between noticing that the subprocess exited and noticing that the
> > session that that subprocess (presumably) had open had been closed. If
> > it noticed the latter first, nothing was logged (because it didn't log
> > anything if a session was closed in the ordinary way by the client). If
> > it noticed the former first, it logged a message about closing the
> session
> > itself.
> >
> > This is a benign race that causes no real problems--except that the tests
> > didn't expect to see the log message from the former case and fail with
> > errors like the following:
> >
> > 1826. ovsdb-server.at:92: testing truncating database log with bad
> transaction ...
> > ./ovsdb-server.at:96: ovsdb-tool create db schema
> > stderr:
> > stdout:
> > ./ovsdb-server.at:104: ovsdb-server --remote=punix:socket db
> --run="sh txnfile"
> > --- /dev/null 2018-04-24 08:50:58.76900 +
> > +++
> /root/openvswitch-2.9.2/rpm/rpmbuild/BUILD/openvswitch-2.9.2/tests/testsuite.dir/at-groups/1826/stderr
> 2018-05-29 14:29:56.529257295 +
> > @@ -0,0 +1,2 @@
> > +2018-05-29T14:29:56Z|1|ovsdb_jsonrpc_server|INFO|unix#0:
> disconnecting (removing ordinals database due to server termination)
> > +2018-05-29T14:29:56Z|2|ovsdb_jsonrpc_server|INFO|unix#0:
> disconnecting (removing _Server database due to server termination)
> >
> > This fixes the race. This particular log message isn't too useful since
> > it's pretty obvious that ovsdb-server is closing those sessions, since
> > after all it's exiting!
> >
> > Reported-by: Sanket Sudake
> > Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-May/046840.html
> > Signed-off-by: Ben Pfaff
>
Acked-by: Numan Siddique
> > ---
> > ovsdb/jsonrpc-server.c | 12 +++-
> > ovsdb/ovsdb-server.c | 4 +---
> > 2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> > index 6c58f4102e29..7c7a277f0d49 100644
> > --- a/ovsdb/jsonrpc-server.c
> > +++ b/ovsdb/jsonrpc-server.c
> > @@ -173,8 +173,9 @@ ovsdb_jsonrpc_server_add_db(struct
> ovsdb_jsonrpc_server *svr, struct ovsdb *db)
> >
> > /* Removes 'db' from the set of databases served out by 'svr'.
> > *
> > - * 'comment' should be a human-readable reason for removing the
> database. This
> > - * function frees it. */
> > + * 'comment' should be a human-readable reason for removing the
> database, for
> > + * use in log messages, or NULL to suppress logging. This function
> frees
> > + * it. */
> > void
> > ovsdb_jsonrpc_server_remove_db(struct ovsdb_jsonrpc_server *svr,
> > struct ovsdb *db, char *comment)
> > @@ -339,7 +340,7 @@ ovsdb_jsonrpc_server_free_remote_status(
> >
> > /* Makes all of the JSON-RPC sessions managed by 'svr' disconnect.
> (They
> > * will then generally reconnect.). Uses 'comment' as a human-readable
> comment
> > - * for logging. Frees 'comment'.
> > + * for logging (it may be NULL to suppress logging). Frees 'comment'.
> > *
> > * If 'force' is true, disconnects all sessions. Otherwise,
> disconnects only
> > * sesions that aren't database change aware. */
> > @@ -644,7 +645,8 @@ ovsdb_jsonrpc_session_close_all(struct
> ovsdb_jsonrpc_remote *remote)
> >
> > /* Makes all of the JSON-RPC sessions managed by 'remote' disconnect.
> (They
> > * will then generally reconnect.). 'comment' should be a
> human-readable
> > - * explanation of the reason for disconnection, for use in log messages.
> > + * explanation of the reason for disconnection, for use in log
> messages, or
> > + * NULL to suppress logging.
> > *
> > * If 'force' is true, disconnects all sessions. Otherwise,
> disconnects only
> > * sesions that aren't database change aware. */
> > @@ -657,7 +659,7 @@ ovsdb_jsonrpc_session_reconnect_all(struct
> ovsdb_jsonrpc_remote *remote,
> > LIST_FOR_EACH_SAFE (s, next, node, >sessions) {
> > if (force || !s->db_change_aware) {
> > jsonrpc_session_force_reconnect(s->js);
> > -if (jsonrpc_session_is_connected(s->js)) {
> > +if (comment && jsonrpc_session_is_connected(s->js)) {
> > VLOG_INFO("%s: disconnecting (%s)",
> >jsonrpc_session_get_name(s->js), comment);
> > }
> > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> > index 68f7acae9fa3..8d213b27aae1 100644
> > --- a/ovsdb/ovsdb-server.c
> > +++ b/ovsdb/ovsdb-server.c
> > @@ -459,9 +459,7 @@ main(int argc, char *argv[])
> >
> >
Re: [ovs-dev] [PATCH v2 14/15] tests: Add test for sync command to ovn-nbctl test suite.
On Thu, 12 Jul 2018 17:10:14 -0400 Mark Michelson wrote: > This is tough. The problem with a test like this is that you can't > really tell that the sync is working as intended. It's possible that the > ovn-nbctl is not actually waiting, but by coincidence, the database has > its values updated before you check them. > > You'd need something internal to ovn-nbctl to let you know that the > database value has been updated prior to exiting to know for sure that > the sync is working as intended. How about we switch from 'get' to 'wait-until'? Let me try that. > > On 07/12/2018 09:40 AM, Jakub Sitnicki wrote: > > Signed-off-by: Jakub Sitnicki > > --- > > tests/ovn.at | 19 +++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/tests/ovn.at b/tests/ovn.at > > index d1a8967dd..adb99db77 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -10541,3 +10541,22 @@ OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], > > [vif2.expected]) > > > > OVN_CLEANUP([hv1], [hv2]) > > AT_CLEANUP > > + > > +AT_SETUP([ovn -- sync]) > > +ovn_start > > + > > +AT_CHECK([ovn-nbctl get NB_Global . nb_cfg], [0], [dnl > > +0 > > +]) > > + > > +AT_CHECK([ovn-nbctl --wait=sb sync]) > > +AT_CHECK([ovn-nbctl get NB_Global . sb_cfg], [0], [dnl > > +1 > > +]) > > + > > +AT_CHECK([ovn-nbctl --wait=hv sync]) > > +AT_CHECK([ovn-nbctl get NB_Global . hv_cfg], [0], [dnl > > +2 > > +]) > > + > > +AT_CLEANUP > > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 12/15] ovn-nbctl: Initial support for daemon mode.
On Thu, 12 Jul 2018 17:10:04 -0400
Mark Michelson wrote:
> On 07/12/2018 09:40 AM, Jakub Sitnicki wrote:
> > Make ovn-nbctl act as a unixctl server if we were asked to detach. This
> > turns ovn-nbctl into a long-lived process that acts a proxy for
> > interacting with NB DB. The main difference to regular mode of ovn-nbctl
> > is that in the daemon mode, a local copy of database contents has to be
> > obtained only once.
> >
> > Just two unixctl commands are supported 'run' and 'exit'. The former can
> > be used to run any ovn-nbctl command or a batch of them as so:
> >
> >ovs-appctl -t ovn-nbctl run [OPTIONS] COMMAND [-- [OPTIONS] COMMAND] ...
> >
> > Running commands that have not yet been converted to not use ctl_fatal()
> > will result in death of the daemon process. However, --monitor option
> > can be used to keep the daemon running.
> >
> > Signed-off-by: Jakub Sitnicki
> > ---
> > ovn/utilities/ovn-nbctl.8.xml | 40
> > ovn/utilities/ovn-nbctl.c | 213
> > --
> > 2 files changed, 227 insertions(+), 26 deletions(-)
> >
(...)
> > +static void
> > +server_cmd_run(struct unixctl_conn *conn, int argc, const char **argv_,
> > + void *idl_)
> > +{
> > +struct ovsdb_idl *idl = idl_;
> > +struct ctl_command *commands = NULL;
> > +struct shash local_options;
> > +size_t n_commands = 0;
> > +char *error = NULL;
> > +
> > +/* Copy args so that getopt() can permute them. Leave last entry NULL.
> > */
> > +char **argv = xcalloc(argc + 1, sizeof *argv);
> > +for (int i = 0; i < argc; i++) {
> > +argv[i] = xstrdup(argv_[i]);
> > +}
> > +
> > +/* Reset global state. */
> > +oneline = false;
> > +dry_run = false;
> > +wait_type = NBCTL_WAIT_NONE;
> > +force_wait = false;
> > +timeout = 0;
> > +table_style = table_style_default;
>
> Not all global state is being reset here. The biggest thing I spotted
> was that the vlog level is not reset.
Let me fix that.
>
> > +
> > +/* Parse commands & options. */
> > +char *args = process_escape_args(argv);
> > +shash_init(_options);
> > +optind = 0;
> > +parse_options(argc, argv, _options);
>
> Calling parse_options() here is interesting. There are some options that
> are relevant, some options that are irrelevant and ignored, and there
> are some options that we really don't want to parse here.
>
> For instance, "commands", "help", "version", and "options" will result
> in the server process exiting. I know in your cover letter, you
> mentioned that there were still some places where ctl_fatal() is called.
> I'm not sure if you had noticed these places where exit() is called as well.
Perhaps I need to revisit the idea of having an options parses tailored
to server mode needs. Other option would be to ignore options that don't
make sense for the server.
>
> > +commands = ctl_parse_commands(argc - optind, argv + optind,
> > + _options, _commands);
> > +VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG,
> > + "Running command %s", args);
> > +
> > +struct timer *wait_timeout = NULL;
> > +struct timer wait_timeout_;
> > +if (timeout) {
> > +wait_timeout = _timeout_;
> > +timer_set_duration(wait_timeout, timeout * 1000);
> > +}
> > +
> > +error = run_prerequisites(commands, n_commands, idl);
> > +if (error) {
> > +unixctl_command_reply_error(conn, error);
> > +goto out;
> > +}
> > +error = main_loop(args, commands, n_commands, idl, wait_timeout);
> > +if (error) {
> > +unixctl_command_reply_error(conn, error);
> > +goto out;
> > +}
> > +
> > +struct ds output = DS_EMPTY_INITIALIZER;
> > +for (struct ctl_command *c = commands; c < [n_commands]; c++)
> > {
> > +if (c->table) {
> > +table_format(c->table, _style, );
> > +} else if (oneline) {
> > +oneline_format(>output, );
> > +} else {
> > +ds_put_cstr(, ds_cstr_ro(>output));
> > +}
> > +
> > +ds_destroy(>output);
> > +table_destroy(c->table);
> > +free(c->table);
> > +}
> > +unixctl_command_reply(conn, ds_cstr_ro());
> > +ds_destroy();
> > +
> > +out:
> > +free(error);
> > +for (struct ctl_command *c = commands; c < [n_commands]; c++)
> > {
> > +shash_destroy_free_data(>options);
> > +}
> > +free(commands);
> > +shash_destroy_free_data(_options);
> > +free(args);
> > +for (int i = 0; i < argc; i++) {
> > +free(argv[i]);
> > +}
> > +free(argv);
> > +}
> > +
> > +static void
> > +server_cmd_init(struct ovsdb_idl *idl, bool *exiting)
> > +{
> > +unixctl_command_register("exit", "", 0, 0, server_cmd_exit, exiting);
> > +unixctl_command_register("run", "", 1, INT_MAX, server_cmd_run, idl);
> > +}
> > +
> > +static
Re: [ovs-dev] [PATCH v2 02/15] ovsdb-idl: Allow monitoring columns that are already monitored.
On Thu, 12 Jul 2018 17:09:45 -0400
Mark Michelson wrote:
> On 07/12/2018 09:40 AM, Jakub Sitnicki wrote:
> > If IDL was created with monitoring and alerts turned on by default for
> > all columns, then there is no harm in allowing the API users to ask
> > again for monitoring and alerts to be enabled for any given column.
> >
> > This allows us to run prerequisites handlers for db-ctl and ovn-nbctl
> > commands once the IDL has already ran once.
> >
> > Signed-off-by: Jakub Sitnicki
> > ---
> > lib/ovsdb-idl.c | 16 +++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> > index 9ab5d6723..ae0a55c3a 100644
> > --- a/lib/ovsdb-idl.c
> > +++ b/lib/ovsdb-idl.c
> > @@ -1116,6 +1116,20 @@ ovsdb_idl_db_get_mode(struct ovsdb_idl_db *db,
> > return >modes[column - table->class_->columns];
> > }
> >
> > +static void
> > +ovsdb_idl_db_set_mode(struct ovsdb_idl_db *db,
> > + const struct ovsdb_idl_column *column,
> > + unsigned char mode)
> > +{
> > +const struct ovsdb_idl_table *table = ovsdb_idl_table_from_column(db,
> > +
> > column);
> > +size_t column_idx = column - table->class_->columns;
> > +
> > +if (table->modes[column_idx] != mode) {
> > +*ovsdb_idl_db_get_mode(db, column) = mode;
>
> Calling ovsdb_idl_db_get_mode() here seems wasteful. You already have
> retrieved the table and the column index. So you may as well just do:
>
> table->modes[column_indx] = mode;
That's true but we also want to perform run-time check that IDL has not
ran yet that is in ovsdb_idl_db_get_mode(). That was the main reason
for going through ovsdb_idl_db_get_mode() to set the mode.
>
>
> > +}
> > +}
> > +
> > static void
> > add_ref_table(struct ovsdb_idl_db *db, const struct ovsdb_base_type *base)
> > {
> > @@ -1136,7 +1150,7 @@ static void
> > ovsdb_idl_db_add_column(struct ovsdb_idl_db *db,
> > const struct ovsdb_idl_column *column)
> > {
> > -*ovsdb_idl_db_get_mode(db, column) = OVSDB_IDL_MONITOR |
> > OVSDB_IDL_ALERT;
> > +ovsdb_idl_db_set_mode(db, column, OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT);
> > add_ref_table(db, >type.key);
> > add_ref_table(db, >type.value);
> > }
> >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
