[ovs-dev] [PATCH] compat: Initialize IPv4 reassembly secret timer
The RHEL 7 kernels expect the secret timer interval to be initialized
before calling the inet_frags_init() function. By not initializing it
the inet_frags_secret_rebuild() function was running on every tick
rather than on the expected interval. This caused occasional panics
from page faults when inet_frags_secret_rebuild() would try to rearm a
timer from the openvswitch kernel module which had just been removed.
Also remove the prior, and now unnecessary, work around.
VMware BZ 2094203
Fixes: 595e069a ("compat: Backport IPv4 reassembly.")
Signed-off-by: Greg Rose
---
datapath/datapath.c| 10 --
datapath/linux/compat/ip_fragment.c| 3 +++
datapath/linux/compat/nf_conntrack_reasm.c | 3 +++
3 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 43f0d74..3ea240a 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -2478,16 +2478,6 @@ error:
static void dp_cleanup(void)
{
-#if RHEL_RELEASE_CODE < RHEL_RELEASE_VERSION(8,0)
- /* On RHEL 7.x kernels we hit a kernel paging error without
-* this barrier and subsequent hefty delay. A process will
-* attempt to access openvwitch memory after it has been
-* unloaded. Further debugging is needed on that but for
-* now let's not let customer machines panic.
-*/
- rcu_barrier();
- msleep(3000);
-#endif
dp_unregister_genl(ARRAY_SIZE(dp_genl_families));
ovs_netdev_exit();
unregister_netdevice_notifier(_dp_device_notifier);
diff --git a/datapath/linux/compat/ip_fragment.c
b/datapath/linux/compat/ip_fragment.c
index 8f2012b..f910b99 100644
--- a/datapath/linux/compat/ip_fragment.c
+++ b/datapath/linux/compat/ip_fragment.c
@@ -812,6 +812,9 @@ int __init rpl_ipfrag_init(void)
#ifdef HAVE_INET_FRAGS_WITH_FRAGS_WORK
ip4_frags.frags_cache_name = ip_frag_cache_name;
#endif
+#if RHEL_RELEASE_CODE < RHEL_RELEASE_VERSION(8,0)
+ ip4_frags.secret_interval = 10 * 60 * HZ;
+#endif
if (inet_frags_init(_frags)) {
pr_warn("IP: failed to allocate ip4_frags cache\n");
return -ENOMEM;
diff --git a/datapath/linux/compat/nf_conntrack_reasm.c
b/datapath/linux/compat/nf_conntrack_reasm.c
index ea153c3..ce13112 100644
--- a/datapath/linux/compat/nf_conntrack_reasm.c
+++ b/datapath/linux/compat/nf_conntrack_reasm.c
@@ -643,6 +643,9 @@ int rpl_nf_ct_frag6_init(void)
#ifdef HAVE_INET_FRAGS_WITH_FRAGS_WORK
nf_frags.frags_cache_name = nf_frags_cache_name;
#endif
+#if RHEL_RELEASE_CODE < RHEL_RELEASE_VERSION(8,0)
+ nf_frags.secret_interval = 10 * 60 * HZ;
+#endif
ret = inet_frags_init(_frags);
if (ret)
goto out;
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] Introduce ovs-appctl command to monitor HVs sb connection status
Signed-off-by: aginwala
On Thu, Jul 19, 2018 at 4:01 AM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:
> Add 'connection-status' command to ovs-appctl utility in order to check
> if a given chassis is currently connected to SB db
>
> Co-authored-by: aginwala
> Signed-off-by: Lorenzo Bianconi
> ---
> lib/jsonrpc.c | 6 ++
> lib/jsonrpc.h | 1 +
> lib/ovsdb-idl.c | 6 ++
> lib/ovsdb-idl.h | 1 +
> ovn/controller/ovn-controller.8.xml | 5 +
> ovn/controller/ovn-controller.c | 18 ++
> 6 files changed, 37 insertions(+)
>
> diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
> index 4c2c1ba84..21de06003 100644
> --- a/lib/jsonrpc.c
> +++ b/lib/jsonrpc.c
> @@ -1162,6 +1162,12 @@ jsonrpc_session_is_connected(const struct
> jsonrpc_session *s)
> return s->rpc != NULL;
> }
>
> +bool
> +jsonrpc_session_is_rconnected(const struct jsonrpc_session *s)
> +{
> +return reconnect_is_connected(s->reconnect);
> +}
> +
> /* Returns a sequence number for 's'. The sequence number increments
> every
> * time 's' connects or disconnects. Thus, a caller can use the change
> (or
> * lack of change) in the sequence number to figure out whether the
> underlying
> diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h
> index a44114e8d..cf8351aaf 100644
> --- a/lib/jsonrpc.h
> +++ b/lib/jsonrpc.h
> @@ -125,6 +125,7 @@ void jsonrpc_session_recv_wait(struct jsonrpc_session
> *);
>
> bool jsonrpc_session_is_alive(const struct jsonrpc_session *);
> bool jsonrpc_session_is_connected(const struct jsonrpc_session *);
> +bool jsonrpc_session_is_rconnected(const struct jsonrpc_session *);
> unsigned int jsonrpc_session_get_seqno(const struct jsonrpc_session *);
> int jsonrpc_session_get_status(const struct jsonrpc_session *);
> int jsonrpc_session_get_last_error(const struct jsonrpc_session *);
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index 9ab5d6723..2c057a0ba 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -922,6 +922,12 @@ ovsdb_idl_is_alive(const struct ovsdb_idl *idl)
> idl->state != IDL_S_ERROR;
> }
>
> +bool
> +ovsdb_idl_is_rconnected(const struct ovsdb_idl *idl)
> +{
> +return jsonrpc_session_is_rconnected(idl->session);
> +}
> +
> /* Returns the last error reported on a connection by 'idl'. The return
> value
> * is 0 only if no connection made by 'idl' has ever encountered an error
> and
> * a negative response to a schema request has never been received. See
> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> index ea18b22f5..e657829c7 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -80,6 +80,7 @@ void ovsdb_idl_force_reconnect(struct ovsdb_idl *);
> void ovsdb_idl_verify_write_only(struct ovsdb_idl *);
>
> bool ovsdb_idl_is_alive(const struct ovsdb_idl *);
> +bool ovsdb_idl_is_rconnected(const struct ovsdb_idl *idl);
> int ovsdb_idl_get_last_error(const struct ovsdb_idl *);
>
> void ovsdb_idl_set_probe_interval(const struct ovsdb_idl *, int
> probe_interval);
> diff --git a/ovn/controller/ovn-controller.8.xml
> b/ovn/controller/ovn-controller.8.xml
> index 0eff2113f..0861edbc4 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -388,6 +388,11 @@
> 0x800.
>
>
> +
> + connection-status
> +
> +Show OVN SBDB connection status for the chassis.
> +
>
>
>
> diff --git a/ovn/controller/ovn-controller.c
> b/ovn/controller/ovn-controller.c
> index 6ee72a9fa..a05098e9b 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -60,12 +60,14 @@
> #include "timeval.h"
> #include "timer.h"
> #include "stopwatch.h"
> +#include "lib/reconnect.h"
>
> VLOG_DEFINE_THIS_MODULE(main);
>
> static unixctl_cb_func ovn_controller_exit;
> static unixctl_cb_func ct_zone_list;
> static unixctl_cb_func inject_pkt;
> +static unixctl_cb_func ovn_controller_rconn_show;
>
> #define DEFAULT_BRIDGE_NAME "br-int"
> #define DEFAULT_PROBE_INTERVAL_MSEC 5000
> @@ -588,6 +590,9 @@ main(int argc, char *argv[])
> ovsdb_idl_create(ovnsb_remote, _idl_class, true, true));
> ovsdb_idl_set_leader_only(ovnsb_idl_loop.idl, false);
>
> +unixctl_command_register("connection-status", "", 0, 0,
> + ovn_controller_rconn_show,
> ovnsb_idl_loop.idl);
> +
> struct ovsdb_idl_index *sbrec_chassis_by_name
> = chassis_index_create(ovnsb_idl_loop.idl);
> struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath
> @@ -1061,3 +1066,16 @@ update_probe_interval(const struct
> ovsrec_open_vswitch_table *ovs_table,
>
> ovsdb_idl_set_probe_interval(ovnsb_idl, interval);
> }
> +
> +static void
> +ovn_controller_rconn_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> + const char *argv[] OVS_UNUSED, void *idl_)
> +{
> +char *result = "not
Re: [ovs-dev] [PATCH v2 2/2] Datapath: Remove ovs_vport_init unreachable code
Thanks for the work. Signed-off-by: Yifeng Sun On Thu, Jul 19, 2018 at 11:34 AM, Alin Gabriel Serdean wrote: > The line "ovs_stt_cleanup_module();" was unreachable. > Found using static analysis tools. > > Signed-off-by: Alin Gabriel Serdean > Co-authored-by: Yifeng Sun > --- > v2: Remove unreachable code only. > --- > datapath/vport.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/datapath/vport.c b/datapath/vport.c > index 28ddb865c..55f40255b 100644 > --- a/datapath/vport.c > +++ b/datapath/vport.c > @@ -97,7 +97,6 @@ int ovs_vport_init(void) > goto err_stt; > > return 0; > - ovs_stt_cleanup_module(); > err_stt: > vxlan_cleanup_module(); > err_vxlan: > -- > 2.16.1.windows.1 > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/2] Datapath: Fix ovs_vport_init unreachable code and goto labels
> Subiect: Re: [ovs-dev] [PATCH 2/2] Datapath: Fix ovs_vport_init unreachable > code and goto labels > > I think the correct fix may be as follows, do you mind rechecking it? > Thanks. > > diff --git a/datapath/vport.c b/datapath/vport.c index > 02f6b56d3243..fcf0fea0a245 100644 > --- a/datapath/vport.c > +++ b/datapath/vport.c > @@ -93,7 +93,6 @@ int ovs_vport_init(void) > goto err_stt; > > return 0; > - ovs_stt_cleanup_module(); > err_stt: > vxlan_cleanup_module(); > err_vxlan: > Thanks for the suggestion. I folded in the changes and sent out a v2: https://patchwork.ozlabs.org/patch/946556/ Do you mind giving a Signed-off? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 2/2] Datapath: Remove ovs_vport_init unreachable code
The line "ovs_stt_cleanup_module();" was unreachable. Found using static analysis tools. Signed-off-by: Alin Gabriel Serdean Co-authored-by: Yifeng Sun --- v2: Remove unreachable code only. --- datapath/vport.c | 1 - 1 file changed, 1 deletion(-) diff --git a/datapath/vport.c b/datapath/vport.c index 28ddb865c..55f40255b 100644 --- a/datapath/vport.c +++ b/datapath/vport.c @@ -97,7 +97,6 @@ int ovs_vport_init(void) goto err_stt; return 0; - ovs_stt_cleanup_module(); err_stt: vxlan_cleanup_module(); err_vxlan: -- 2.16.1.windows.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 00/21] Daemon mode for ovn-nbctl
Thanks for starting the discussion, Mark.
On Thu, 19 Jul 2018 12:26:24 -0400
Mark Michelson wrote:
> On 07/19/2018 09:51 AM, Jakub Sitnicki wrote:
(...)
> > 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 [2].
> >
> > 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?
>
> When I think about this, I think of two obstacles:
> 1) What is a good way to run a test under both daemon mode and
> non-daemon mode?
> 2) What is the best way to compose a test that will run under both
> daemon mode and non-daemon mode
>
> (2) seems like the easier problem to mechanically solve. You can perform
> a find and replace to change all current instances of `ovn-nbctl` to
> some bash function. The bash function could determine whether daemon
> mode is in use. If not in daemon mode, call `ovn-nbctl` directly. If in
> daemon mode, call `ovs-appctl -t ovn-nbctl run`.
>
> This isn't a good long term solution though. It will be difficult to
> enforce the use of the bash function instead of calling ovn-nbctl. A
> potentially better long-term solution would be not to require ovs-appctl
> when communicating with the ovn-nbctl daemon. If ovn-nbctl could be
> called directly, it would make things easier.
As it happens, in the test suite, ovn-nbctl is an alias for a shell
function already [1].
That is something I thought we can leverage. At least until we turn
ovn-nbctl tool into a JSON-RPC client that will control the daemon
(which ideally should work out-of-the-box if the daemon is running).
For now, we could have a wrapper around 'ovs-appctl -t ovn-nbctl run'
that tries to act like ovn-nbctl. I have a very crude prototype of
that [2] which screams for a rewrite in Python :-)
>
> (1) is a bit more tricky in my opinion.
>
> One idea would be to pass in a flag to the testsuite that says to start
> an ovn-nbctl daemon when ovn_start() is called. This way, the entire
> runthrough of the test would be for ovn-nbctl daemon mode. In CI
> systems, we'd want to perform two runs of the testsuite, one in daemon
> mode, and one not in daemon mode. This idea is a bit overkill though
> since most tests in the testsuite are not even OVN tests. This also
> doesn't scale well in case there become other ways we want to run
> variants of existing tests.
>
> A better idea is to somehow make existing tests that use ovn-nbctl run
> once in daemon mode and once in non-daemon mode. This is difficult,
> though, because the structure of the tests doesn't allow for an easy way
> to repeat the test. The "easiest" way I can think to do this is to
> modify the existing ovn test files to be .in files that at build time
> generate a daemon version of the test and a non-daemon version of the
> test. Notice "easiest" in quotation marks though. The advantage of doing
> it this way is that it simultaneously clears up problem (2) as well
> since the autogenerated files will have the proper invocations in them.
I agree this is the trickier part.
I was also thinking about introducing a flag that controls if ovn-nbctl
runs in daemon mode or regular mode throughout the tests. If we had
that we could do a second pass only on tests that match on the 'ovn'
keyword.
I haven't thought of generating two variants of tests. That also sounds
interesting. Maybe that would be doable straight from m4, without going
through generating files from templates.
One more thing to consider is that point where we have to start/stop
the daemon differs from test suite to test suite. For tests/ovn.at that
would be ovn_start/OVN_CLEANUP (with one exception [3]). But for
tests/ovn-nbctl.at this would be OVN_NBCTL_TEST_{START,STOP}. That
seems less of problem though.
-Jakub
[1] https://github.com/openvswitch/ovs/blob/master/tests/ovs-macros.at#L135
[2]
https://github.com/jsitnicki/ovs/commit/c34ef03fc11e94e61c57edbfb3e968eacbf07ffb#diff-0c2aae317387234fe3f3e3f28313cf63R32
[3] https://github.com/openvswitch/ovs/blob/master/tests/ovn.at#L6376
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 00/11] Get rid of ctl_fatal() calls in ovn-nbctl (part 2)
This is a continuation of an earlier series that aims to replace calls to ctl_fatal() in command handlers in ovn-nbctl. The motivation is to handle errors gracefully when running commands in daemon mode because as a long-lived process we shouldn't terminate on errors that we can recover from. After this series there are no more ctl_fatal() calls in ovn-nbctl that affect the daemon mode. The only remaining function left to convert is the commands parser in db-ctl-base module (ctl_parse_commands()), which I intend to deal with separately. Either as a part of ovn-nbctl daemon series (already in review [1]), or as a follow-up to it. Thanks, Jakub [1] https://patchwork.ozlabs.org/project/openvswitch/list/?series=55472 Jakub Sitnicki (11): ovn-nbctl: Don't die in pg_by_name_or_uuid(). ovn-nbctl: Don't die in gc_by_name_or_uuid(). ovn-nbctl: Don't die in lsp_to_ls(). ovn-nbctl: Don't die in lrp_to_lr(). ovn-nbctl: Don't die in parse_priority(). ovn-nbctl: Don't die in parse_direction(). ovn-nbctl: Don't die in dhcp_options_get(). ovn-nbctl: Propagate error thru the context. ovn-nbctl: Use ctl_error() in command handlers. ovn-nbctl: Remove pointless "return;" at ends of functions. ovn-nbctl: Fix mem leak in nbctl_lrp_set_gateway_chassis(). ovn/utilities/ovn-nbctl.c | 360 -- tests/ovn-nbctl.at| 5 + 2 files changed, 260 insertions(+), 105 deletions(-) -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] build: Add gitattribute file to build-aux
> On 19 Jul 2018, at 20:42, Ben Pfaff wrote: > > On Thu, Jul 19, 2018 at 07:39:42PM +0300, Alin Gabriel Serdean wrote: >> The command: `make check-tabs` fails on Windows due to line ending >> conversions >> caused by the following setting: `git config --global core.autocrlf true` >> (the whitelist `build-aux/initial-tab-whitelist` becomes a blacklist) >> >> This patch adds a .gittatribute file to build-aux to force LF endings >> on Windows. >> >> Signed-off-by: Alin Gabriel Serdean >> Co-authored-by: Aaron Conole >> --- >> v2: fold in changes Suggested-by: Aaron Conole > > I guess you should wait for a Signed-off-by from Aaron, but as long as > you get it: > > Acked-by: Ben Pfaff Thanks Aaron and Ben! I applied it on master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for master
On 7/13/2018 5:56 PM, Ian Stokes wrote: 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) Hi Ben, would it be easier for me to rebase this to the head of master and submit a new pull request with the sparse fixes you submitted as part of it also? Whatever you think is easiest. Thanks Ian 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 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [branch-2.9 PATCH] netdev-dpdk: Fix failure to configure flow control at netdev-init.
This patch backports the commit from the latest OVS master to OVS-2.9.
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.
Signed-off-by: Sugesh Chandran
---
lib/netdev-dpdk.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 5310373..58f77d2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -911,13 +911,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;
}
@@ -1618,6 +1611,14 @@ 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_WARN("cannot get flow control parameters on port %"PRIu16
+ ", 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;
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] build: Add gitattribute file to build-aux
On Thu, Jul 19, 2018 at 07:39:42PM +0300, Alin Gabriel Serdean wrote: > The command: `make check-tabs` fails on Windows due to line ending conversions > caused by the following setting: `git config --global core.autocrlf true` > (the whitelist `build-aux/initial-tab-whitelist` becomes a blacklist) > > This patch adds a .gittatribute file to build-aux to force LF endings > on Windows. > > Signed-off-by: Alin Gabriel Serdean > Co-authored-by: Aaron Conole > --- > v2: fold in changes Suggested-by: Aaron Conole I guess you should wait for a Signed-off-by from Aaron, but as long as you get it: Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] build: Add gitattribute file to build-aux
Alin Gabriel Serdean writes: > The command: `make check-tabs` fails on Windows due to line ending conversions > caused by the following setting: `git config --global core.autocrlf true` > (the whitelist `build-aux/initial-tab-whitelist` becomes a blacklist) > > This patch adds a .gittatribute file to build-aux to force LF endings > on Windows. > > Signed-off-by: Alin Gabriel Serdean > Co-authored-by: Aaron Conole > --- Signed-off-by: Aaron Conole Thanks, Alin! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-dev, v2] build: Add gitattribute file to build-aux
Bleep bloop. Greetings Alin Gabriel Serdean, 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: Too many signoffs; are you missing Co-authored-by lines? ERROR: Co-authored-by/Signed-off-by corruption Lines checked: 56, Warnings: 0, Errors: 2 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] build: Add gitattribute file to build-aux
On Tue, Jul 17, 2018 at 03:26:20PM +0300, Alin Gabriel Serdean wrote: > The command: `make check-tabs` fails on Windows due to line ending conversions > caused by the following setting: `git config --global core.autocrlf true` > (the whitelist `build-aux/initial-tab-whitelist` becomes a blacklist) > > This patch adds a .gittatribute file to build-aux to force LF endings > on Windows. > > Signed-off-by: Alin Gabriel Serdean With Aaron's suggestion, this makes sense to me. Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] build: Add gitattribute file to build-aux
> -Mesaj original- > De la: ovs-dev-boun...@openvswitch.org boun...@openvswitch.org> În numele Aaron Conole > Trimis: Wednesday, July 18, 2018 6:02 PM > Către: Alin Gabriel Serdean > Cc: d...@openvswitch.org > Subiect: Re: [ovs-dev] build: Add gitattribute file to build-aux > > Hi Alin, > > 0-day Robot writes: > > > Bleep bloop. Greetings Alin Gabriel Serdean, 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. > > > > > > I suggest folding in something like the following (since I don't think it makes > sense to worry about distributing gitattributes files): > Thanks for the suggestion Aaron. I sent out an incremental: https://patchwork.ozlabs.org/patch/946511/ Do you mind looking over it and adding a Signed-off? Thanks, Alin. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] build: Add gitattribute file to build-aux
The command: `make check-tabs` fails on Windows due to line ending conversions caused by the following setting: `git config --global core.autocrlf true` (the whitelist `build-aux/initial-tab-whitelist` becomes a blacklist) This patch adds a .gittatribute file to build-aux to force LF endings on Windows. Signed-off-by: Alin Gabriel Serdean Co-authored-by: Aaron Conole --- v2: fold in changes Suggested-by: Aaron Conole --- .gitignore | 1 - Makefile.am | 1 + build-aux/.gitattributes | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 build-aux/.gitattributes diff --git a/.gitignore b/.gitignore index 81faf270d..60e7818c3 100644 --- a/.gitignore +++ b/.gitignore @@ -31,7 +31,6 @@ .libs .tmp_versions .vagrant -.gitattributes /Makefile /Makefile.in /aclocal.m4 diff --git a/Makefile.am b/Makefile.am index e02799a90..fd6620a9b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -199,6 +199,7 @@ dist-hook-git: distfiles (cat distfiles; sed 's|^|datapath/|' datapath/distfiles) | \ LC_ALL=C sort -u > all-distfiles; \ (cd $(srcdir) && git ls-files) | grep -v '\.gitignore$$' | \ + grep -v '\.gitattributes$$' | \ LC_ALL=C sort -u > all-gitfiles; \ LC_ALL=C comm -1 -3 all-distfiles all-gitfiles > missing-distfiles; \ if test -s missing-distfiles; then \ diff --git a/build-aux/.gitattributes b/build-aux/.gitattributes new file mode 100644 index 0..fcadb2cf9 --- /dev/null +++ b/build-aux/.gitattributes @@ -0,0 +1 @@ +* text eol=lf -- 2.16.1.windows.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 00/21] Daemon mode for ovn-nbctl
On 07/19/2018 09:51 AM, Jakub Sitnicki wrote:
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.
Mark Michelson has benchmarked previous version of this patchset and
demonstrated a significant speed-up in the run-time of his benchmark [1].
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 [2].
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?
When I think about this, I think of two obstacles:
1) What is a good way to run a test under both daemon mode and
non-daemon mode?
2) What is the best way to compose a test that will run under both
daemon mode and non-daemon mode
(2) seems like the easier problem to mechanically solve. You can perform
a find and replace to change all current instances of `ovn-nbctl` to
some bash function. The bash function could determine whether daemon
mode is in use. If not in daemon mode, call `ovn-nbctl` directly. If in
daemon mode, call `ovs-appctl -t ovn-nbctl run`.
This isn't a good long term solution though. It will be difficult to
enforce the use of the bash function instead of calling ovn-nbctl. A
potentially better long-term solution would be not to require ovs-appctl
when communicating with the ovn-nbctl daemon. If ovn-nbctl could be
called directly, it would make things easier.
(1) is a bit more tricky in my opinion.
One idea would be to pass in a flag to the testsuite that says to start
an ovn-nbctl daemon when ovn_start() is called. This way, the entire
runthrough of the test would be for ovn-nbctl daemon mode. In CI
systems, we'd want to perform two runs of the testsuite, one in daemon
mode, and one not in daemon mode. This idea is a bit overkill though
since most tests in the testsuite are not even OVN tests. This also
doesn't scale well in case there become other ways we want to run
variants of existing tests.
A better idea is to somehow make existing tests that use ovn-nbctl run
once in daemon mode and once in non-daemon mode. This is difficult,
though, because the structure of the tests doesn't allow for an easy way
to repeat the test. The "easiest" way I can think to do this is to
modify the existing ovn test files to be .in files that at build time
generate a daemon version of the test and a non-daemon version of the
test. Notice "easiest" in quotation marks though. The advantage of doing
it this way is that it simultaneously clears up problem (2) as well
since the autogenerated files will have the proper invocations in them.
I suspect other people might have much better ideas than me, though :)
A dirty, just-for-development integration with tests is available at:
https://github.com/jsitnicki/ovs/commits/wip-nbctl-daemon-2018-07-19
There is a related series pending review/merge that converts the remaining
ctl_fatal() callers to propate the error [3]. It goes together with this
patchset but they don't depend on each other.
Daemon mode should be considered experimental.
Thanks,
Jakub
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349742.html
[2] Except test "2563: ovn -- IPv6 ND Router Solicitation responder" which seems
broken in master @ 61b1c7acb9a2 ("netdev-bsd: Fix crash on FreeBSD.").
[3] https://patchwork.ozlabs.org/project/openvswitch/list/?series=55975
Changes v4 -> v5:
- Fix a build error in the middle of the series. Reported by 0-day robot:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349864.html
- Reword patch 7 description.
Changes v3 -> v4:
- Propagate errors from the commands parser and send them back to the JSON-RPC
client when running in daemon mode.
- Fix-up getopt()'s global state initialization before parsing a command line.
- Distinguish between unknown-option and missing-argument errors when parsing
command line options in daemon mode.
- Fix a double-free of transaction resources on error path.
- Add test for commands parser error paths.
- Use 'dnl' instead of '#' for comments in tests.
- To review just the interdiff run:
git fetch --tags
[ovs-dev] Loan Offer
Get a private loan today @ 3% interest rate. We offer a friendly service. ___ 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 Ben/Aaron/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 *)
>
[ovs-dev] [PATCH v5 21/21] tests: Add test for ovn-nbctl's command parser error paths.
Preparatory work for getting rid of ctl_fatal() in command parser. Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 76 ++ 1 file changed, 76 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 4e6986a5e..ffbd2a140 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1424,3 +1424,79 @@ AT_CHECK([ovn-nbctl --oneline ls-list -- ls-list | uuidfilt], [0], [dnl OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - commands parser error paths]) +OVN_NBCTL_TEST_START + +dnl FIXME: Duplicate options are allowed when passed with global options. +dnlFor example: ovn-nbctl --if-exists --if-exists list Logical_Switch + +dnl Duplicate option +AT_CHECK([ovn-nbctl -- --if-exists --if-exists list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option specified multiple times' stderr], [0], [ignore]) + +dnl Missing command +AT_CHECK([ovn-nbctl], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl --if-exists], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl --], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --if-exists], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +dnl Unknown command +AT_CHECK([ovn-nbctl foo], [1], [], [stderr]) +AT_CHECK([grep 'unknown command' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- foo], [1], [], [stderr]) +AT_CHECK([grep 'unknown command' stderr], [0], [ignore]) + +dnl Unknown option +AT_CHECK([ovn-nbctl --foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'unrecognized option' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'command has no .* option' stderr], [0], [ignore]) + +dnl Missing option argument +AT_CHECK([ovn-nbctl --columns], [1], [], [stderr]) +AT_CHECK([grep 'option .* requires an argument' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --columns list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'missing argument to .* option' stderr], [0], [ignore]) + +dnl Unexpected option argument +AT_CHECK([ovn-nbctl --if-exists=foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option .* doesn'\''t allow an argument' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --if-exists=foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option on .* does not accept an argument' stderr], [0], [ignore]) + +dnl Not enough arguments +AT_CHECK([ovn-nbctl list], [1], [], [stderr]) +AT_CHECK([grep 'command requires at least .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- list], [1], [], [stderr]) +AT_CHECK([grep 'command requires at least .* arguments' stderr], [0], [ignore]) + +dnl Too many arguments +AT_CHECK([ovn-nbctl show foo bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- show foo bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl show foo --bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- show foo --bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +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 v5 20/21] 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 585967568..4e6986a5e 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]) + +dnl Expect one line for one command. +AT_CHECK([ovn-nbctl --oneline ls-list | uuidfilt], [0], [dnl +<0> (ls0)\n<1> (ls1) +]) + +dnl 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 v5 19/21] 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..585967568 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 + +dnl 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 +]) + +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 v5 18/21] 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 | 335 ++
2 files changed, 344 insertions(+), 31 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 7a0d6ecd4..920f9b8ca 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,14 +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;
set_program_name(argv[0]);
fatal_ignore_sigpipe();
@@ -118,41 +126,59 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-char *error = ctl_parse_commands(argc - optind, argv + optind,
- _options, , _commands);
-if (error) {
-ctl_fatal("%s", error);
-}
-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)");
+}
+
[ovs-dev] [PATCH v5 17/21] 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 490577238..7a0d6ecd4 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -288,6 +288,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)
{
@@ -309,22 +333,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 v5 16/21] 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 ca0f71300..490577238 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -276,6 +276,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)
{
@@ -296,16 +308,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 v5 15/21] 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 e8fd898d3..ca0f71300 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -202,38 +202,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},
@@ -270,40 +325,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 v5 14/21] 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 b84976b26..e8fd898d3 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4164,6 +4164,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)
@@ -4299,25 +4332,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 v5 13/21] 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 c7f03cd21..b84976b26 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[])
@@ -134,7 +138,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);
}
@@ -155,7 +159,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;
@@ -179,7 +183,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;
}
@@ -4161,7 +4166,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;
@@ -4288,8 +4293,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;
@@ -4333,11 +4336,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);
the_idl_txn = NULL;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 12/21] 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 | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index fe28e2293..c7f03cd21 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);
@@ -128,7 +129,10 @@ 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);
+}
error = main_loop(args, commands, n_commands, idl);
if (error) {
@@ -4120,7 +4124,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)
{
@@ -4141,7 +4145,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);
@@ -4149,6 +4155,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 v5 11/21] 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 e660b2cdf..fe28e2293 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[])
@@ -128,7 +130,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);
+error = main_loop(args, commands, n_commands, idl);
+if (error) {
+ctl_fatal("%s", error);
+}
ovsdb_idl_destroy(idl);
idl = the_idl = NULL;
@@ -144,7 +149,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)
{
@@ -172,10 +177,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;
}
}
@@ -184,6 +189,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 v5 10/21] 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 465f8f6d1..e660b2cdf 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,
@@ -169,7 +170,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;
}
@@ -4140,7 +4144,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)
{
@@ -4151,6 +4155,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);
@@ -4184,7 +4189,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);
@@ -4198,9 +4205,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) {
@@ -4225,7 +4233,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);
}
@@ -4239,7 +4249,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:
@@ -4249,11 +4260,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();
@@ -4313,11 +4327,14 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
the_idl_txn = NULL;
*retry = false;
-return;
+return NULL;
try_again:
/* Our transaction
[ovs-dev] [PATCH v5 09/21] db-ctl-base: Propagate errors from the commands parser.
Let the caller decide how to handle the error. Prepare for using the
parser in ovn-nbctl daemon mode.
Signed-off-by: Jakub Sitnicki
---
lib/db-ctl-base.c | 30 +-
lib/db-ctl-base.h | 6 +++---
ovn/utilities/ovn-nbctl.c | 7 +--
ovn/utilities/ovn-sbctl.c | 7 +--
utilities/ovs-vsctl.c | 7 +--
vtep/vtep-ctl.c | 7 +--
6 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index f92da7c61..4e0eb9c5c 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -2251,22 +2251,24 @@ ctl_add_cmd_options(struct option **options_p, size_t
*n_options_p,
}
/* Parses command-line input for commands. */
-struct ctl_command *
+char *
ctl_parse_commands(int argc, char *argv[], struct shash *local_options,
- size_t *n_commandsp)
+ struct ctl_command **commandsp, size_t *n_commandsp)
{
struct ctl_command *commands;
size_t n_commands, allocated_commands;
int i, start;
+char *error;
commands = NULL;
n_commands = allocated_commands = 0;
+*commandsp = NULL;
+*n_commandsp = 0;
+
for (start = i = 0; i <= argc; i++) {
if (i == argc || !strcmp(argv[i], "--")) {
if (i > start) {
-char *error;
-
if (n_commands >= allocated_commands) {
struct ctl_command *c;
@@ -2277,21 +2279,31 @@ ctl_parse_commands(int argc, char *argv[], struct shash
*local_options,
}
}
error = parse_command(i - start, [start], local_options,
- [n_commands++]);
+ [n_commands]);
if (error) {
-ctl_fatal("%s", error);
+struct ctl_command *c;
+
+for (c = commands; c < [n_commands]; c++) {
+shash_destroy_free_data(>options);
+}
+free(commands);
+
+return error;
}
+
+n_commands++;
} else if (!shash_is_empty(local_options)) {
-ctl_fatal("missing command name (use --help for help)");
+return xstrdup("missing command name (use --help for help)");
}
start = i + 1;
}
}
if (!n_commands) {
-ctl_fatal("missing command name (use --help for help)");
+return xstrdup("missing command name (use --help for help)");
}
+*commandsp = commands;
*n_commandsp = n_commands;
-return commands;
+return NULL;
}
/* Prints all registered commands. */
diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
index ba771a180..284b573d0 100644
--- a/lib/db-ctl-base.h
+++ b/lib/db-ctl-base.h
@@ -166,9 +166,9 @@ void ctl_print_options(const struct option *);
void ctl_add_cmd_options(struct option **, size_t *n_options_p,
size_t *allocated_options_p, int opt_val);
void ctl_register_commands(const struct ctl_command_syntax *);
-struct ctl_command *ctl_parse_commands(int argc, char *argv[],
- struct shash *local_options,
- size_t *n_commandsp);
+char * OVS_WARN_UNUSED_RESULT ctl_parse_commands(
+int argc, char *argv[], struct shash *local_options,
+struct ctl_command **commandsp, size_t *n_commandsp);
/* Sometimes, it is desirable to print the table with weak reference to
* rows in a 'cmd_show_table' table. In that case, the 'weak_ref_table'
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 25194b2fa..465f8f6d1 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -110,8 +110,11 @@ 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);
+char *error = ctl_parse_commands(argc - optind, argv + optind,
+ _options, , _commands);
+if (error) {
+ctl_fatal("%s", error);
+}
VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG,
"Called as %s", args);
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index c47cf6df9..7022347ed 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -112,8 +112,11 @@ 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);
+char *error = ctl_parse_commands(argc - optind, argv + optind,
+ _options, , _commands);
+if
[ovs-dev] [PATCH v5 08/21] db-ctl-base: Propagate error from parse_command().
Let the caller handle the error. Needed for ovn-nbctl daemon mode.
Signed-off-by: Jakub Sitnicki
---
lib/db-ctl-base.c | 63 +--
1 file changed, 43 insertions(+), 20 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index ab35d6333..f92da7c61 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1880,7 +1880,7 @@ cmd_wait_until(struct ctl_context *ctx)
}
/* Parses one command. */
-static void
+static char * OVS_WARN_UNUSED_RESULT
parse_command(int argc, char *argv[], struct shash *local_options,
struct ctl_command *command)
{
@@ -1888,6 +1888,7 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
struct shash_node *node;
int n_arg;
int i;
+char *error;
shash_init(>options);
shash_swap(local_options, >options);
@@ -1910,17 +1911,23 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
}
if (shash_find(>options, key)) {
-ctl_fatal("'%s' option specified multiple times", argv[i]);
+free(key);
+free(value);
+error = xasprintf("'%s' option specified multiple times", argv[i]);
+goto error;
}
shash_add_nocopy(>options, key, value);
}
if (i == argc) {
-ctl_fatal("missing command name (use --help for help)");
+error = xstrdup("missing command name (use --help for help)");
+goto error;
}
p = shash_find_data(_commands, argv[i]);
if (!p) {
-ctl_fatal("unknown command '%s'; use --help for help", argv[i]);
+error = xasprintf("unknown command '%s'; use --help for help",
+ argv[i]);
+goto error;
}
SHASH_FOR_EACH (node, >options) {
@@ -1928,43 +1935,54 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
int end = s ? s[strlen(node->name)] : EOF;
if (!strchr("=,? ", end)) {
-ctl_fatal("'%s' command has no '%s' option",
- argv[i], node->name);
+error = xasprintf("'%s' command has no '%s' option",
+ argv[i], node->name);
+goto error;
}
if (end != '?' && (end == '=') != (node->data != NULL)) {
if (end == '=') {
-ctl_fatal("missing argument to '%s' option on '%s' "
- "command", node->name, argv[i]);
+error = xasprintf("missing argument to '%s' option on '%s' "
+ "command", node->name, argv[i]);
+goto error;
} else {
-ctl_fatal("'%s' option on '%s' does not accept an "
- "argument", node->name, argv[i]);
+error = xasprintf("'%s' option on '%s' does not accept an "
+ "argument", node->name, argv[i]);
+goto error;
}
}
}
n_arg = argc - i - 1;
if (n_arg < p->min_args) {
-ctl_fatal("'%s' command requires at least %d arguments",
- p->name, p->min_args);
+error = xasprintf("'%s' command requires at least %d arguments",
+ p->name, p->min_args);
+goto error;
} else if (n_arg > p->max_args) {
int j;
for (j = i + 1; j < argc; j++) {
if (argv[j][0] == '-') {
-ctl_fatal("'%s' command takes at most %d arguments "
- "(note that options must precede command "
- "names and follow a \"--\" argument)",
- p->name, p->max_args);
+error = xasprintf("'%s' command takes at most %d arguments "
+ "(note that options must precede command "
+ "names and follow a \"--\" argument)",
+ p->name, p->max_args);
+goto error;
}
}
-ctl_fatal("'%s' command takes at most %d arguments",
- p->name, p->max_args);
+error = xasprintf("'%s' command takes at most %d arguments",
+ p->name, p->max_args);
+goto error;
}
command->syntax = p;
command->argc = n_arg + 1;
command->argv = [i];
+return NULL;
+
+error:
+shash_destroy_free_data(>options);
+return error;
}
static void
@@ -2247,6 +2265,8 @@ ctl_parse_commands(int argc, char *argv[], struct shash
*local_options,
for (start = i = 0; i <= argc; i++) {
if (i == argc || !strcmp(argv[i], "--")) {
if (i > start) {
+char *error;
+
if (n_commands >= allocated_commands) {
struct ctl_command *c;
@@ -2256,8 +2276,11 @@ ctl_parse_commands(int argc, char *argv[], struct shash
*local_options,
[ovs-dev] [PATCH v5 07/21] ovn-nbctl: Don't destroy the transaction twice on error.
Reset the global state, if transaction succeeded. Otherwise nbctl_exit() callback will try to clean up on any fatal error. Signed-off-by: Jakub Sitnicki --- ovn/utilities/ovn-nbctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index 360b25a89..25194b2fa 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -4307,6 +4307,7 @@ do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands, } ovsdb_idl_txn_destroy(txn); +the_idl_txn = NULL; *retry = false; return; -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 06/21] 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 v5 05/21] 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 v5 03/21] 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 v5 01/21] 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 v5 02/21] 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 v5 00/21] 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.
Mark Michelson has benchmarked previous version of this patchset and
demonstrated a significant speed-up in the run-time of his benchmark [1].
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 [2].
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-19
There is a related series pending review/merge that converts the remaining
ctl_fatal() callers to propate the error [3]. It goes together with this
patchset but they don't depend on each other.
Daemon mode should be considered experimental.
Thanks,
Jakub
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349742.html
[2] Except test "2563: ovn -- IPv6 ND Router Solicitation responder" which seems
broken in master @ 61b1c7acb9a2 ("netdev-bsd: Fix crash on FreeBSD.").
[3] https://patchwork.ozlabs.org/project/openvswitch/list/?series=55975
Changes v4 -> v5:
- Fix a build error in the middle of the series. Reported by 0-day robot:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349864.html
- Reword patch 7 description.
Changes v3 -> v4:
- Propagate errors from the commands parser and send them back to the JSON-RPC
client when running in daemon mode.
- Fix-up getopt()'s global state initialization before parsing a command line.
- Distinguish between unknown-option and missing-argument errors when parsing
command line options in daemon mode.
- Fix a double-free of transaction resources on error path.
- Add test for commands parser error paths.
- Use 'dnl' instead of '#' for comments in tests.
- To review just the interdiff run:
git fetch --tags https://github.com/jsitnicki/ovs
git diff wip-nbctl-daemon-2018-07-{13,19}
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
Re: [ovs-dev] Introduce ovs-appctl command to monitor HVs sb connection status
0-day Robot writes: > Bleep bloop. Greetings Lorenzo Bianconi, I am a robot and I have > tried out your patch. > Thanks for your contribution. > > I encountered some error that I wasn't expecting. See the details below. > > > checkpatch: > ERROR: Too many signoffs; are you missing Co-authored-by lines? > ERROR: Co-authored-by/Signed-off-by corruption > Lines checked: 144, Warnings: 0, Errors: 2 Probably we can improve this log to be a bit more informative. In the meantime, from the submitting-patches.rst: All co-authors must also sign off. So, you'll need to get the sign-off tag. I think patchwork can automatically record it from a reply, so it would probably be sufficient to just have aginwala (CCd) to reply to the patch with the appropriate tag. Thanks, Lorenzo and aginwala! -Aaron > 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 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 2/3] Send gateway port ARP through router internal ports
This patch had some compilation errors. It may be due to changes in
master since this patchset was posted:
ovn/controller/pinctrl.c: In function ‘send_garp_run’:
ovn/controller/pinctrl.c:2303:48: error: passing argument 8 of
‘get_nat_addresses_and_keys’ discards ‘const’ qualifier from pointer
target type [-Werror=discarded-qualifiers]
_addresses, local_datapaths);
^~~
ovn/controller/pinctrl.c:2182:1: note: expected ‘struct hmap *’ but
argument is of type ‘const struct hmap *’
get_nat_addresses_and_keys(struct ovsdb_idl_index *sbrec_chassis_by_name,
^~
On 06/25/2018 03:53 PM, vkomm...@redhat.com wrote:
From: venkata anil
External switches should learn the distributed gateway port MAC address
as they have to forward the packet tagged with tenant vlan network but
with this MAC as destination MAC address. So router has to send ARP
reply and gARP for this MAC address through router internal patch ports
connecting tenant vlan networks.
Signed-off-by: Venkata Anil
---
v5->v6:
* Rebased
v4->v5:
* No changes in this patch
ovn/controller/pinctrl.c | 57 +++-
ovn/northd/ovn-northd.c | 57
tests/ovn.at | 6 +
3 files changed, 114 insertions(+), 6 deletions(-)
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index a0bf602..37157aa 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -2185,8 +2185,47 @@ get_nat_addresses_and_keys(struct ovsdb_idl_index
*sbrec_chassis_by_name,
struct sset *local_l3gw_ports,
const struct sbrec_chassis *chassis,
const struct sset *active_tunnels,
- struct shash *nat_addresses)
+ struct shash *nat_addresses,
+ struct hmap *local_datapaths)
{
+/* When a router has tenant vlan networks, gARP for distributed gateway
+ * router port has to be sent through internal tenant vlan network's
+ * localnet port, so that external switches can learn this MAC and forward
+ * tenant vlan network traffic with distributed gateway router port MAC
+ * as destination MAC address */
+
+struct local_datapath *ldp;
+struct shash router_vlan_ports;
+
+shash_init(_vlan_ports);
+HMAP_FOR_EACH (ldp, hmap_node, local_datapaths) {
+const struct sbrec_port_binding *crp;
+crp = ldp->chassisredirect_port;
+/* check if it a router with chassis redirect port,
+ * get corresponding distributed port */
+if (crp && crp->chassis &&
+!strcmp(crp->chassis->name, chassis->name)) {
+const struct sbrec_port_binding *dp = NULL;
+for (int i = 0; i < ldp->n_peer_dps; i++) {
+if (!strcmp(ldp->peer_dps[i]->patch->logical_port,
+smap_get(>options, "distributed-port"))) {
+dp = ldp->peer_dps[i]->peer;
+break;
+}
+}
+
It seems possible to reach this point and have dp be NULL. You probably
should continue the hmap traversal if dp is NULL.
+/* Save router internal port (patch port on tenant vlan network)
+ * along with distributed port. */
+for (int i = 0; i < ldp->n_peer_dps; i++) {
+if (strcmp(ldp->peer_dps[i]->patch->logical_port,
+ smap_get(>options, "distributed-port"))) {
+shash_add(_vlan_ports,
+ ldp->peer_dps[i]->peer->logical_port, dp);
+}
+}
+}
+}
+
const char *gw_port;
SSET_FOR_EACH(gw_port, local_l3gw_ports) {
const struct sbrec_port_binding *pb;
@@ -2196,11 +2235,16 @@ get_nat_addresses_and_keys(struct ovsdb_idl_index
*sbrec_chassis_by_name,
continue;
}
-if (pb->n_nat_addresses) {
-for (int i = 0; i < pb->n_nat_addresses; i++) {
+/* Router internatl ports should send gARP for distributed port
s/internatl/internal/
+ * NAT addresses */
+const struct sbrec_port_binding *dp;
+dp = shash_find_data(_vlan_ports, pb->logical_port);
+const struct sbrec_port_binding *nat_port = dp ? dp : pb;
+if (nat_port->n_nat_addresses) {
+for (int i = 0; i < nat_port->n_nat_addresses; i++) {
consider_nat_address(sbrec_chassis_by_name,
sbrec_port_binding_by_name,
- pb->nat_addresses[i], pb,
+ nat_port->nat_addresses[i], pb,
nat_address_keys, chassis,
active_tunnels,
Re: [ovs-dev] [ovs-dev, v4, 11 of 21] ovn-nbctl: Propagate errors from the main loop.
On Thu, 19 Jul 2018 09:15:14 -0400 0-day Robot wrote: (...) > ovn/utilities/ovn-nbctl.c: In function ‘main’: > ovn/utilities/ovn-nbctl.c:133:11: error: redefinition of ‘error’ > char *error = main_loop(args, commands, n_commands, idl); >^ > ovn/utilities/ovn-nbctl.c:116:11: note: previous definition of ‘error’ was > here > char *error = ctl_parse_commands(argc - optind, argv + optind, >^ My fault - rebase gone wrong. This is fixed in the next commit. I will roll out v5 with a fix so that there are no build failures in the middle of the series. -Jakub ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 1/3] Avoid tunneling for VLAN packets redirected to a gateway chassis
I've had a look through and have some notes in-line. I know some of them
will be a bit nit-picky, but...sometimes that's just how I am :)
On 06/25/2018 03:53 PM, vkomm...@redhat.com wrote:
From: venkata anil
When a vm on a vlan tenant network sends traffic to an external network,
it is tunneled from host chassis to gateway chassis. In the earlier
discussion [1], Russel (also in his doc [2]) suggested if we can figure
out a way for OVN to do this redirect to the gateway host over a VLAN
network. This patch implements his suggestion i.e will redirect to
gateway chassis using incoming tenant vlan network. Gateway chassis are
expected to be configured with tenant vlan networks. In this approach,
new logical and physical flows introduced for packet processing in both
host and gateway chassis.
Packet processing in the host chassis:
1) A new ovs flow added in physical table 65, which sets MLF_RCV_FROM_VLAN
flag for packets from vlan network entering into router pipeline
2) A new flow added in lr_in_ip_routing, for packets output through
distributed gateway port and matching MLF_RCV_FROM_VLAN flag,
set REGBIT_NAT_REDIRECT i.e
table=7 (lr_in_ip_routing ), priority=2, match=(
ip4.dst == 0.0.0.0/0 && flags.rcv_from_vlan == 1 &&
!is_chassis_resident("cr-alice")), action=(reg9[0] = 1; next;)
This flow will be set only on chassis not hosting chassisredirect
port i.e compute node.
When REGBIT_NAT_REDIRECT set,
a) lr_in_arp_resolve, will set packet eth.dst to distibuted gateway
port MAC
b) lr_in_gw_redirect, will set chassisredirect port as outport
3) A new ovs flow added in physical table 32 will use source vlan tenant
network tag as vlan ID for sending the packet to gateway chassis.
As this vlan packet destination MAC is distibuted gateway port MAC,
packet will only reach the gateway chassis.
table=32,priority=150,reg10=0x20/0x20,reg14=0x3,reg15=0x6,metadata=0x4
actions=mod_vlan_vid:2010,output:25,strip_vlan
This flow will be set only on chassis not hosting chassisredirect
port i.e compute node.
Packet processing in the gateway chassis:
1) A new ovs flow added in physical table 0 for vlan traffic coming
from localnet port with router distributed gateway port MAC as
destination MAC address, resubmit to connected router ingress pipeline
(i.e router attached to vlan tenant network).
table=0,priority=150,in_port=67,dl_vlan=2010,dl_dst=00:00:02:01:02:03
actions=strip_vlan,load:0x4->OXM_OF_METADATA[],load:0x3->NXM_NX_REG14[],
load:0x1->NXM_NX_REG10[5],resubmit(,8)
This flow will be set only on chassis hosting chassisredirect
port i.e gateway node.
2) A new flow added in lr_in_admission which checks MLF_RCV_FROM_VLAN
and allows the packet. This flow will be set only on chassis hosting
chassisredirect port i.e gateway node.
table=0 (lr_in_admission), priority=100 , match=(
flags.rcv_from_vlan == 1 && inport == "lrp-44383893-613a-4bfe-b483-
e7d0dc3055cd" && is_chassis_resident("cr-lrp-a6e3d2ab-313a-4ea3-
8ec4-c3c774a11f49")), action=(next;)
Then packet will pass through router ingress and egress pipelines and
then to external switch pipeline.
[1] https://mail.openvswitch.org/pipermail/ovs-discuss/2018-April/046557.html
[2] Point 3 in section 3.3.1 - Future Enhancements
https://docs.google.com/document/d/1JecGIXPH0RAqfGvD0nmtBdEU1zflHACp8WSRnKCFSgg/edit#
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-April/046543.html
Signed-off-by: Venkata Anil
---
v5->v6:
* Rebased
v4->v5:
* No changes in this patch
v3->v4:
* Previous v3 patch became this patch of v4
* Updated the newly added flow in physical table 0 on gateway chassis
to check for distributed gateway port MAC and then resubmit to
router ingress pipeline
* Improved the test
* Added more comments
ovn/controller/bfd.c| 3 +-
ovn/controller/binding.c| 10 +-
ovn/controller/ovn-controller.c | 3 +
ovn/controller/ovn-controller.h | 17 ++-
ovn/controller/physical.c | 121 -
ovn/lib/logical-fields.c| 4 +
ovn/lib/logical-fields.h| 2 +
ovn/northd/ovn-northd.c | 35 +
tests/ovn.at| 278
9 files changed, 465 insertions(+), 8 deletions(-)
diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
index 051781f..c696741 100644
--- a/ovn/controller/bfd.c
+++ b/ovn/controller/bfd.c
@@ -139,8 +139,9 @@ bfd_travel_gw_related_chassis(
LIST_FOR_EACH_POP (dp_binding, node, _list) {
dp = dp_binding->dp;
free(dp_binding);
+const struct sbrec_datapath_binding *pdp;
for (size_t i = 0; i < dp->n_peer_dps; i++) {
-const struct sbrec_datapath_binding *pdp = dp->peer_dps[i];
+pdp = dp->peer_dps[i]->peer_dp;
if (!pdp) {
continue;
}
diff
Re: [ovs-dev] [ovs-dev, v4, 11 of 21] ovn-nbctl: Propagate errors from the main loop.
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. build: /bin/sh ./libtool --tag=CC --mode=link gcc -std=gnu99 -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -o ovn/controller-vtep/ovn-controller-vtep ovn/controller-vtep/binding.o ovn/controller-vtep/gateway.o ovn/controller-vtep/ovn-controller-vtep.o ovn/controller-vtep/vtep.o ovn/lib/libovn.la lib/libopenvswitch.la vtep/libvtep.la -lpthread -lrt -lm -lunbound libtool: link: gcc -std=gnu99 -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -o ovn/controller-vtep/ovn-controller-vtep ovn/controller-vtep/binding.o ovn/controller-vtep/gateway.o ovn/controller-vtep/ovn-controller-vtep.o ovn/controller-vtep/vtep.o ovn/lib/.libs/libovn.a lib/.libs/libopenvswitch.a -lssl -lcrypto -lcap-ng vtep/.libs/libvtep.a -lpthread -lrt -lm -lunbound depbase=`echo ovn/northd/ovn-northd.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\ gcc -std=gnu99 -DHAVE_CONFIG_H -I.-I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT ovn/northd/ovn-northd.o -MD -MP -MF $depbase.Tpo -c -o ovn/northd/ovn-northd.o ovn/northd/ovn-northd.c &&\ mv -f $depbase.Tpo $depbase.Po /bin/sh ./libtool --tag=CC --mode=link gcc -std=gnu99 -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -o ovn/northd/ovn-northd ovn/northd/ovn-northd.o ovn/lib/libovn.la ovsdb/libovsdb.la lib/libopenvswitch.la -lpthread -lrt -lm -lunbound libtool: link: gcc -std=gnu99 -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -o ovn/northd/ovn-northd ovn/northd/ovn-northd.o ovn/lib/.libs/libovn.a ovsdb/.libs/libovsdb.a lib/.libs/libopenvswitch.a -lssl -lcrypto -lcap-ng -lpthread -lrt -lm -lunbound depbase=`echo ovn/utilities/ovn-nbctl.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\ gcc -std=gnu99 -DHAVE_CONFIG_H -I.-I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT ovn/utilities/ovn-nbctl.o -MD -MP -MF $depbase.Tpo -c -o ovn/utilities/ovn-nbctl.o ovn/utilities/ovn-nbctl.c &&\ mv -f $depbase.Tpo $depbase.Po ovn/utilities/ovn-nbctl.c: In function ‘main’: ovn/utilities/ovn-nbctl.c:133:11: error: redefinition of ‘error’ char *error = main_loop(args, commands, n_commands, idl); ^ ovn/utilities/ovn-nbctl.c:116:11: note: previous definition of ‘error’ was here char *error = ctl_parse_commands(argc - optind, argv + optind, ^ ovn/utilities/ovn-nbctl.c: At top level: cc1: error: unrecognized command line option "-Wno-null-pointer-arithmetic" [-Werror] cc1: all warnings being treated as errors make[2]: *** [ovn/utilities/ovn-nbctl.o] Error 1 make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make: *** [all] Error 2 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 21/21] tests: Add test for ovn-nbctl's command parser error paths.
Preparatory work for getting rid of ctl_fatal() in command parser. Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 76 ++ 1 file changed, 76 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 4e6986a5e..ffbd2a140 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1424,3 +1424,79 @@ AT_CHECK([ovn-nbctl --oneline ls-list -- ls-list | uuidfilt], [0], [dnl OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - commands parser error paths]) +OVN_NBCTL_TEST_START + +dnl FIXME: Duplicate options are allowed when passed with global options. +dnlFor example: ovn-nbctl --if-exists --if-exists list Logical_Switch + +dnl Duplicate option +AT_CHECK([ovn-nbctl -- --if-exists --if-exists list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option specified multiple times' stderr], [0], [ignore]) + +dnl Missing command +AT_CHECK([ovn-nbctl], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl --if-exists], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl --], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --if-exists], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +dnl Unknown command +AT_CHECK([ovn-nbctl foo], [1], [], [stderr]) +AT_CHECK([grep 'unknown command' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- foo], [1], [], [stderr]) +AT_CHECK([grep 'unknown command' stderr], [0], [ignore]) + +dnl Unknown option +AT_CHECK([ovn-nbctl --foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'unrecognized option' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'command has no .* option' stderr], [0], [ignore]) + +dnl Missing option argument +AT_CHECK([ovn-nbctl --columns], [1], [], [stderr]) +AT_CHECK([grep 'option .* requires an argument' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --columns list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'missing argument to .* option' stderr], [0], [ignore]) + +dnl Unexpected option argument +AT_CHECK([ovn-nbctl --if-exists=foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option .* doesn'\''t allow an argument' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --if-exists=foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option on .* does not accept an argument' stderr], [0], [ignore]) + +dnl Not enough arguments +AT_CHECK([ovn-nbctl list], [1], [], [stderr]) +AT_CHECK([grep 'command requires at least .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- list], [1], [], [stderr]) +AT_CHECK([grep 'command requires at least .* arguments' stderr], [0], [ignore]) + +dnl Too many arguments +AT_CHECK([ovn-nbctl show foo bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- show foo bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl show foo --bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- show foo --bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +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 v4 19/21] 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..585967568 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 + +dnl 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 +]) + +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 v4 20/21] 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 585967568..4e6986a5e 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]) + +dnl Expect one line for one command. +AT_CHECK([ovn-nbctl --oneline ls-list | uuidfilt], [0], [dnl +<0> (ls0)\n<1> (ls1) +]) + +dnl 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 v4 18/21] 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 | 336 ++
2 files changed, 344 insertions(+), 32 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 b6ad78985..920f9b8ca 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,41 +126,59 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-error = ctl_parse_commands(argc - optind, argv + optind, _options,
- , _commands);
-if (error) {
-ctl_fatal("%s", error);
-}
-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)");
+}
+
[ovs-dev] [PATCH v4 17/21] 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 339a182e8..b6ad78985 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -289,6 +289,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)
{
@@ -310,22 +334,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 v4 16/21] 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 a9113b9e0..339a182e8 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -277,6 +277,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)
{
@@ -297,16 +309,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 v4 15/21] 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 cce3ae023..a9113b9e0 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -203,38 +203,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},
@@ -271,40 +326,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 v4 14/21] 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 00b75aa6c..cce3ae023 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4165,6 +4165,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)
@@ -4300,25 +4333,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 v4 12/21] 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 | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index ee423577e..ecb351a80 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();
@@ -113,8 +115,8 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-char *error = ctl_parse_commands(argc - optind, argv + optind,
- _options, , _commands);
+error = ctl_parse_commands(argc - optind, argv + optind, _options,
+ , _commands);
if (error) {
ctl_fatal("%s", error);
}
@@ -128,9 +130,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);
}
@@ -4120,7 +4125,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)
{
@@ -4141,7 +4146,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);
@@ -4149,6 +4156,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 v4 13/21] 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 ecb351a80..00b75aa6c 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[])
@@ -135,7 +139,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);
}
@@ -156,7 +160,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;
@@ -180,7 +184,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;
}
@@ -4162,7 +4167,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;
@@ -4289,8 +4294,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;
@@ -4334,11 +4337,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);
the_idl_txn = NULL;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 11/21] 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 e660b2cdf..ee423577e 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[])
@@ -128,7 +130,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;
@@ -144,7 +149,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)
{
@@ -172,10 +177,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;
}
}
@@ -184,6 +189,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 v4 10/21] 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 465f8f6d1..e660b2cdf 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,
@@ -169,7 +170,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;
}
@@ -4140,7 +4144,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)
{
@@ -4151,6 +4155,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);
@@ -4184,7 +4189,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);
@@ -4198,9 +4205,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) {
@@ -4225,7 +4233,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);
}
@@ -4239,7 +4249,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:
@@ -4249,11 +4260,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();
@@ -4313,11 +4327,14 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
the_idl_txn = NULL;
*retry = false;
-return;
+return NULL;
try_again:
/* Our transaction
[ovs-dev] [PATCH v4 09/21] db-ctl-base: Propagate errors from the commands parser.
Let the caller decide how to handle the error. Prepare for using the
parser in ovn-nbctl daemon mode.
Signed-off-by: Jakub Sitnicki
---
lib/db-ctl-base.c | 30 +-
lib/db-ctl-base.h | 6 +++---
ovn/utilities/ovn-nbctl.c | 7 +--
ovn/utilities/ovn-sbctl.c | 7 +--
utilities/ovs-vsctl.c | 7 +--
vtep/vtep-ctl.c | 7 +--
6 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index f92da7c61..4e0eb9c5c 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -2251,22 +2251,24 @@ ctl_add_cmd_options(struct option **options_p, size_t
*n_options_p,
}
/* Parses command-line input for commands. */
-struct ctl_command *
+char *
ctl_parse_commands(int argc, char *argv[], struct shash *local_options,
- size_t *n_commandsp)
+ struct ctl_command **commandsp, size_t *n_commandsp)
{
struct ctl_command *commands;
size_t n_commands, allocated_commands;
int i, start;
+char *error;
commands = NULL;
n_commands = allocated_commands = 0;
+*commandsp = NULL;
+*n_commandsp = 0;
+
for (start = i = 0; i <= argc; i++) {
if (i == argc || !strcmp(argv[i], "--")) {
if (i > start) {
-char *error;
-
if (n_commands >= allocated_commands) {
struct ctl_command *c;
@@ -2277,21 +2279,31 @@ ctl_parse_commands(int argc, char *argv[], struct shash
*local_options,
}
}
error = parse_command(i - start, [start], local_options,
- [n_commands++]);
+ [n_commands]);
if (error) {
-ctl_fatal("%s", error);
+struct ctl_command *c;
+
+for (c = commands; c < [n_commands]; c++) {
+shash_destroy_free_data(>options);
+}
+free(commands);
+
+return error;
}
+
+n_commands++;
} else if (!shash_is_empty(local_options)) {
-ctl_fatal("missing command name (use --help for help)");
+return xstrdup("missing command name (use --help for help)");
}
start = i + 1;
}
}
if (!n_commands) {
-ctl_fatal("missing command name (use --help for help)");
+return xstrdup("missing command name (use --help for help)");
}
+*commandsp = commands;
*n_commandsp = n_commands;
-return commands;
+return NULL;
}
/* Prints all registered commands. */
diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
index ba771a180..284b573d0 100644
--- a/lib/db-ctl-base.h
+++ b/lib/db-ctl-base.h
@@ -166,9 +166,9 @@ void ctl_print_options(const struct option *);
void ctl_add_cmd_options(struct option **, size_t *n_options_p,
size_t *allocated_options_p, int opt_val);
void ctl_register_commands(const struct ctl_command_syntax *);
-struct ctl_command *ctl_parse_commands(int argc, char *argv[],
- struct shash *local_options,
- size_t *n_commandsp);
+char * OVS_WARN_UNUSED_RESULT ctl_parse_commands(
+int argc, char *argv[], struct shash *local_options,
+struct ctl_command **commandsp, size_t *n_commandsp);
/* Sometimes, it is desirable to print the table with weak reference to
* rows in a 'cmd_show_table' table. In that case, the 'weak_ref_table'
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 25194b2fa..465f8f6d1 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -110,8 +110,11 @@ 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);
+char *error = ctl_parse_commands(argc - optind, argv + optind,
+ _options, , _commands);
+if (error) {
+ctl_fatal("%s", error);
+}
VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG,
"Called as %s", args);
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index c47cf6df9..7022347ed 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -112,8 +112,11 @@ 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);
+char *error = ctl_parse_commands(argc - optind, argv + optind,
+ _options, , _commands);
+if
[ovs-dev] [PATCH v4 08/21] db-ctl-base: Propagate error from parse_command().
Let the caller handle the error. Needed for ovn-nbctl daemon mode.
Signed-off-by: Jakub Sitnicki
---
lib/db-ctl-base.c | 63 +--
1 file changed, 43 insertions(+), 20 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index ab35d6333..f92da7c61 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1880,7 +1880,7 @@ cmd_wait_until(struct ctl_context *ctx)
}
/* Parses one command. */
-static void
+static char * OVS_WARN_UNUSED_RESULT
parse_command(int argc, char *argv[], struct shash *local_options,
struct ctl_command *command)
{
@@ -1888,6 +1888,7 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
struct shash_node *node;
int n_arg;
int i;
+char *error;
shash_init(>options);
shash_swap(local_options, >options);
@@ -1910,17 +1911,23 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
}
if (shash_find(>options, key)) {
-ctl_fatal("'%s' option specified multiple times", argv[i]);
+free(key);
+free(value);
+error = xasprintf("'%s' option specified multiple times", argv[i]);
+goto error;
}
shash_add_nocopy(>options, key, value);
}
if (i == argc) {
-ctl_fatal("missing command name (use --help for help)");
+error = xstrdup("missing command name (use --help for help)");
+goto error;
}
p = shash_find_data(_commands, argv[i]);
if (!p) {
-ctl_fatal("unknown command '%s'; use --help for help", argv[i]);
+error = xasprintf("unknown command '%s'; use --help for help",
+ argv[i]);
+goto error;
}
SHASH_FOR_EACH (node, >options) {
@@ -1928,43 +1935,54 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
int end = s ? s[strlen(node->name)] : EOF;
if (!strchr("=,? ", end)) {
-ctl_fatal("'%s' command has no '%s' option",
- argv[i], node->name);
+error = xasprintf("'%s' command has no '%s' option",
+ argv[i], node->name);
+goto error;
}
if (end != '?' && (end == '=') != (node->data != NULL)) {
if (end == '=') {
-ctl_fatal("missing argument to '%s' option on '%s' "
- "command", node->name, argv[i]);
+error = xasprintf("missing argument to '%s' option on '%s' "
+ "command", node->name, argv[i]);
+goto error;
} else {
-ctl_fatal("'%s' option on '%s' does not accept an "
- "argument", node->name, argv[i]);
+error = xasprintf("'%s' option on '%s' does not accept an "
+ "argument", node->name, argv[i]);
+goto error;
}
}
}
n_arg = argc - i - 1;
if (n_arg < p->min_args) {
-ctl_fatal("'%s' command requires at least %d arguments",
- p->name, p->min_args);
+error = xasprintf("'%s' command requires at least %d arguments",
+ p->name, p->min_args);
+goto error;
} else if (n_arg > p->max_args) {
int j;
for (j = i + 1; j < argc; j++) {
if (argv[j][0] == '-') {
-ctl_fatal("'%s' command takes at most %d arguments "
- "(note that options must precede command "
- "names and follow a \"--\" argument)",
- p->name, p->max_args);
+error = xasprintf("'%s' command takes at most %d arguments "
+ "(note that options must precede command "
+ "names and follow a \"--\" argument)",
+ p->name, p->max_args);
+goto error;
}
}
-ctl_fatal("'%s' command takes at most %d arguments",
- p->name, p->max_args);
+error = xasprintf("'%s' command takes at most %d arguments",
+ p->name, p->max_args);
+goto error;
}
command->syntax = p;
command->argc = n_arg + 1;
command->argv = [i];
+return NULL;
+
+error:
+shash_destroy_free_data(>options);
+return error;
}
static void
@@ -2247,6 +2265,8 @@ ctl_parse_commands(int argc, char *argv[], struct shash
*local_options,
for (start = i = 0; i <= argc; i++) {
if (i == argc || !strcmp(argv[i], "--")) {
if (i > start) {
+char *error;
+
if (n_commands >= allocated_commands) {
struct ctl_command *c;
@@ -2256,8 +2276,11 @@ ctl_parse_commands(int argc, char *argv[], struct shash
*local_options,
[ovs-dev] [PATCH v4 07/21] ovn-nbctl: Don't destroy the transaction twice on error.
If transaction succeeded reset the state. Otherwise nbctl_exit() callback will try to clean up on any fatal error. Signed-off-by: Jakub Sitnicki --- ovn/utilities/ovn-nbctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index 360b25a89..25194b2fa 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -4307,6 +4307,7 @@ do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands, } ovsdb_idl_txn_destroy(txn); +the_idl_txn = NULL; *retry = false; return; -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 06/21] 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 v4 03/21] 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 v4 04/21] 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 v4 02/21] 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 v4 01/21] 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 v4 00/21] 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.
Mark Michelson has benchmarked previous version of this patchset and
demonstrated a significant speed-up in the run-time of his benchmark [1].
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 [2].
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-19
There is a related series pending review/merge that converts the remaining
ctl_fatal() callers to propate the error [3]. It goes together with this
patchset but they don't depend on each other.
Daemon mode should be considered experimental.
Thanks,
Jakub
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349742.html
[2] Except test "2563: ovn -- IPv6 ND Router Solicitation responder" which seems
broken in master @ 61b1c7acb9a2 ("netdev-bsd: Fix crash on FreeBSD.").
[3] https://patchwork.ozlabs.org/project/openvswitch/list/?series=55975
Changes v3 -> v4:
- Propagate errors from the commands parser and send them back to the JSON-RPC
client when running in daemon mode.
- Fix-up getopt()'s global state initialization before parsing a command line.
- Distinguish between unknown-option and missing-argument errors when parsing
command line options in daemon mode.
- Fix a double-free of transaction resources on error path.
- Add test for commands parser error paths.
- Use 'dnl' instead of '#' for comments in tests.
- To review just the interdiff run:
git fetch --tags https://github.com/jsitnicki/ovs
git diff wip-nbctl-daemon-2018-07-{13,19}
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:
Re: [ovs-dev] Introduce ovs-appctl command to monitor HVs sb connection status
Bleep bloop. Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Too many signoffs; are you missing Co-authored-by lines? ERROR: Co-authored-by/Signed-off-by corruption Lines checked: 144, Warnings: 0, Errors: 2 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] Introduce ovs-appctl command to monitor HVs sb connection status
Add 'connection-status' command to ovs-appctl utility in order to check
if a given chassis is currently connected to SB db
Co-authored-by: aginwala
Signed-off-by: Lorenzo Bianconi
---
lib/jsonrpc.c | 6 ++
lib/jsonrpc.h | 1 +
lib/ovsdb-idl.c | 6 ++
lib/ovsdb-idl.h | 1 +
ovn/controller/ovn-controller.8.xml | 5 +
ovn/controller/ovn-controller.c | 18 ++
6 files changed, 37 insertions(+)
diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index 4c2c1ba84..21de06003 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -1162,6 +1162,12 @@ jsonrpc_session_is_connected(const struct
jsonrpc_session *s)
return s->rpc != NULL;
}
+bool
+jsonrpc_session_is_rconnected(const struct jsonrpc_session *s)
+{
+return reconnect_is_connected(s->reconnect);
+}
+
/* Returns a sequence number for 's'. The sequence number increments every
* time 's' connects or disconnects. Thus, a caller can use the change (or
* lack of change) in the sequence number to figure out whether the underlying
diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h
index a44114e8d..cf8351aaf 100644
--- a/lib/jsonrpc.h
+++ b/lib/jsonrpc.h
@@ -125,6 +125,7 @@ void jsonrpc_session_recv_wait(struct jsonrpc_session *);
bool jsonrpc_session_is_alive(const struct jsonrpc_session *);
bool jsonrpc_session_is_connected(const struct jsonrpc_session *);
+bool jsonrpc_session_is_rconnected(const struct jsonrpc_session *);
unsigned int jsonrpc_session_get_seqno(const struct jsonrpc_session *);
int jsonrpc_session_get_status(const struct jsonrpc_session *);
int jsonrpc_session_get_last_error(const struct jsonrpc_session *);
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 9ab5d6723..2c057a0ba 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -922,6 +922,12 @@ ovsdb_idl_is_alive(const struct ovsdb_idl *idl)
idl->state != IDL_S_ERROR;
}
+bool
+ovsdb_idl_is_rconnected(const struct ovsdb_idl *idl)
+{
+return jsonrpc_session_is_rconnected(idl->session);
+}
+
/* Returns the last error reported on a connection by 'idl'. The return value
* is 0 only if no connection made by 'idl' has ever encountered an error and
* a negative response to a schema request has never been received. See
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index ea18b22f5..e657829c7 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -80,6 +80,7 @@ void ovsdb_idl_force_reconnect(struct ovsdb_idl *);
void ovsdb_idl_verify_write_only(struct ovsdb_idl *);
bool ovsdb_idl_is_alive(const struct ovsdb_idl *);
+bool ovsdb_idl_is_rconnected(const struct ovsdb_idl *idl);
int ovsdb_idl_get_last_error(const struct ovsdb_idl *);
void ovsdb_idl_set_probe_interval(const struct ovsdb_idl *, int
probe_interval);
diff --git a/ovn/controller/ovn-controller.8.xml
b/ovn/controller/ovn-controller.8.xml
index 0eff2113f..0861edbc4 100644
--- a/ovn/controller/ovn-controller.8.xml
+++ b/ovn/controller/ovn-controller.8.xml
@@ -388,6 +388,11 @@
0x800.
+
+ connection-status
+
+Show OVN SBDB connection status for the chassis.
+
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 6ee72a9fa..a05098e9b 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -60,12 +60,14 @@
#include "timeval.h"
#include "timer.h"
#include "stopwatch.h"
+#include "lib/reconnect.h"
VLOG_DEFINE_THIS_MODULE(main);
static unixctl_cb_func ovn_controller_exit;
static unixctl_cb_func ct_zone_list;
static unixctl_cb_func inject_pkt;
+static unixctl_cb_func ovn_controller_rconn_show;
#define DEFAULT_BRIDGE_NAME "br-int"
#define DEFAULT_PROBE_INTERVAL_MSEC 5000
@@ -588,6 +590,9 @@ main(int argc, char *argv[])
ovsdb_idl_create(ovnsb_remote, _idl_class, true, true));
ovsdb_idl_set_leader_only(ovnsb_idl_loop.idl, false);
+unixctl_command_register("connection-status", "", 0, 0,
+ ovn_controller_rconn_show, ovnsb_idl_loop.idl);
+
struct ovsdb_idl_index *sbrec_chassis_by_name
= chassis_index_create(ovnsb_idl_loop.idl);
struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath
@@ -1061,3 +1066,16 @@ update_probe_interval(const struct
ovsrec_open_vswitch_table *ovs_table,
ovsdb_idl_set_probe_interval(ovnsb_idl, interval);
}
+
+static void
+ovn_controller_rconn_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
+ const char *argv[] OVS_UNUSED, void *idl_)
+{
+char *result = "not connected";
+struct ovsdb_idl *idl = idl_;
+
+if (idl && ovsdb_idl_is_rconnected(idl)) {
+ result = "connected";
+}
+unixctl_command_reply(conn, result);
+}
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 0/4] ovn : Support native dhcp in ovn controller
LOL!, I need more coffee, ignore my last message. It's a very old patch, not sure how I got to it, but I thought it was related to answering also DHCP requests on the provider networks. For example, to server DHCP requests in the case of mixing SR-IOV with OVN using VLANs On Thu, Jul 19, 2018 at 10:38 AM Miguel Angel Ajo Pelayo < majop...@redhat.com> wrote: > > Nice, I’ve been thinking about this last week. > > One question, are we able to limit which chassis will respond to the DHCP > requests? > > I say this, because, otherwise, if we have a lot of chassis, a single > request on > the network would be responded by all chassis which have connectivity to > such network. > > On 19 November 2015 at 19:36:46, Russell Bryant (russell@gmail.com) > wrote: > > On 11/18/2015 11:06 AM, Numan Siddique wrote: > > This patch series introduces native dhcp support in ovn controller. > > > > Babu Shanmugam (3): > > ovn: Dedicated connection handler for packet-ins > > ovn: New flows for DHCP tranffic > > tranffic -> traffic > > > > ovn: Process dhcp packet-ins and respond through packet-outs > > > > Numan Siddique (1): > > ovn: Add tests for ovn dhcp > > I tried to apply the first patch and it was corrupt. > > Can you try posting the series using "git send-email" ? That should > help avoid problems like that. > > -- > Russell Bryant > ___ > dev mailing list > d...@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 0/4] ovn : Support native dhcp in ovn controller
Nice, I’ve been thinking about this last week. One question, are we able to limit which chassis will respond to the DHCP requests? I say this, because, otherwise, if we have a lot of chassis, a single request on the network would be responded by all chassis which have connectivity to such network. On 19 November 2015 at 19:36:46, Russell Bryant (russell@gmail.com) wrote: On 11/18/2015 11:06 AM, Numan Siddique wrote: > This patch series introduces native dhcp support in ovn controller. > > Babu Shanmugam (3): > ovn: Dedicated connection handler for packet-ins > ovn: New flows for DHCP tranffic tranffic -> traffic > ovn: Process dhcp packet-ins and respond through packet-outs > > Numan Siddique (1): > ovn: Add tests for ovn dhcp I tried to apply the first patch and it was corrupt. Can you try posting the series using "git send-email" ? That should help avoid problems like that. -- Russell Bryant ___ dev mailing list d...@openvswitch.org http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net 2/2] openvswitch: check for null return for nla_nest_start in datapath
On Wed, Jul 18, 2018 at 9:12 AM, Stephen Hemminger
wrote:
> The call to nla_nest_start when forming packet messages can lead to a NULL
> return so it's possible for attr to become NULL and we can potentially
> get a NULL pointer dereference on attr. Fix this by checking for
> a NULL return.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200537
> Fixes: 8f0aad6f35f7 ("openvswitch: Extend packet attribute for egress tunnel
> info")
> Signed-off-by: Stephen Hemminger
> ---
> net/openvswitch/datapath.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 0f5ce77460d4..93c3eb635827 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -460,6 +460,10 @@ static int queue_userspace_packet(struct datapath *dp,
> struct sk_buff *skb,
>
> if (upcall_info->egress_tun_info) {
> nla = nla_nest_start(user_skb,
> OVS_PACKET_ATTR_EGRESS_TUN_KEY);
> + if (!nla) {
> + err = -EMSGSIZE;
> + goto out;
> + }
> err = ovs_nla_put_tunnel_info(user_skb,
> upcall_info->egress_tun_info);
> BUG_ON(err);
> @@ -468,6 +472,10 @@ static int queue_userspace_packet(struct datapath *dp,
> struct sk_buff *skb,
>
> if (upcall_info->actions_len) {
> nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_ACTIONS);
> + if (!nla) {
> + err = -EMSGSIZE;
> + goto out;
> + }
> err = ovs_nla_put_actions(upcall_info->actions,
> upcall_info->actions_len,
> user_skb);
Acked-by: Pravin B Shelar
Thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net 1/2] openvswitch: check for null return for nla_nest_start
On Wed, Jul 18, 2018 at 9:12 AM, Stephen Hemminger
wrote:
> The call to nla_nest_start in conntrack can lead to a NULL
> return so it's possible for attr to become NULL and we can potentially
> get a NULL pointer dereference on attr. Fix this by checking for
> a NULL return.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=200533
> Fixes: 11efd5cb04a1 ("openvswitch: Support conntrack zone limit")
> Signed-off-by: Stephen Hemminger
> ---
> net/openvswitch/conntrack.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index 284aca2a252d..2e316f641df8 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -2132,6 +2132,8 @@ static int ovs_ct_limit_cmd_get(struct sk_buff *skb,
> struct genl_info *info)
> return PTR_ERR(reply);
>
> nla_reply = nla_nest_start(reply, OVS_CT_LIMIT_ATTR_ZONE_LIMIT);
> + if (!nla_reply)
> + return PRT_ERR(-EMSGSIZE);
>
> if (a[OVS_CT_LIMIT_ATTR_ZONE_LIMIT]) {
> err = ovs_ct_limit_get_zone_limit(
> --
Acked-by: Pravin B Shelar
Thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
