rconn_recv() calls vconn_recv(), which will report a connection error and
cause rconn_recv() to disconnect the rconn. Most rconn users regularly
call rconn_recv(), so that disconnection happens promptly. However,
the lswitch code only calls rconn_recv() after the connection negotiates an
OpenFlow version, so a connection that failed before negotiation would
never be detected as failed. This commit fixes the problem by making
rconn_run() also detect and handle connection errors.
The lswitch code is only used by the test-controller program, so this is
not an important bug fix.
Reported-by: Vasu Dasari <vdasari@gmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
These functions don't have any ultimate users. The in-band control code
used to use them, but not anymore, so we might as well delete them all.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
This allows other libraries to use util.h that has already
defined NOT_REACHED.
Signed-off-by: Harold Lim <haroldl@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This should make sending OFPT_FLOW_REMOVED and NXST_FLOW_MONITOR safe from
miss handler threads.
Bug #20271.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Added infrastructure to support Openflow OFPT_TABLE_MOD message. This patch
does not include the flexible table miss handling code that is necessary to
support the semantics specified in OFPT_TABLE_MOD messages.
Current flow miss behavior continues to conform to Openflow 1.0. Future
commits to add more flexible table miss support are needed to fully support
OPFT_TABLE_MOD for Openflow-1.1+.
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Otherwise, if a monitor connection happens to be talking to a (misguided?)
peer that sends it messages, such as replies to what the peer perceives as
echo requests meant for it, then the peer will eventually hang trying to
send data because the monitor connection never sinks it.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
rconn_add_monitor() tries to check the version of the controller
connection being monitored, so that it can decide what OpenFlow version to
tell the monitor connection to negotiate. But at any given time an rconn
may not have a controller connection (e.g. during backoff), so rc->vconn
may be null and thus vconn_get_version(rc->vconn) dereferences a null
pointer.
Fixing the problem in a local way would require the rconn to remember the
previous version negotiated, and that fails if the rconn hasn't yet
connected or if the next connection negotiates a new version.
This commit instead adds the ability to a vconn to accept any OpenFlow
message version and modifies "ovs-ofctl snoop" to use that feature, thus
removing the need to negotiate the "correct" version on snoops.
Bug #14265.
Reported-by: Pratap Reddy <preddy@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
This is a straight search-and-replace, except that I also removed #include
<assert.h> from each file where there were no assert calls left.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
The customary parameter order in Open vSwitch is to put input parameters
before output parameters, but vconn_open() and pvconn_open() had the 'dscp'
input parameter at the end, which bugged me a bit. Also,
vconn_open_block() didn't take a 'dscp' parameter at all even though it's
otherwise a wrapper around vconn_open(). This commit fixes all that up.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
rconn_create() was substituting OFPUTIL_DEFAULT_VERSIONS if an
allowed_versions of 0 was passed in. At the same time,
connmgr_set_controllers() compared the adjusted value of allowed_versions
against the original value, saw that they were different, and concluded
that it should kill off and recreate the rconn with the "corrected"
allowed_versions.
This commit fixes the problem by no longer adjusting allowed_versions.
There is no need, because it is only used in contexts where the original
version is OK.
This problem was introduced by commit 90ef0206ea (connmgr:
Reinitialise controllers if protocols changes).
Bug #14126.
CC: Simon Horman <horms@verge.net.au>
Reported-by: Natasha Gude <natasha@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Initial OpenFlow 1.3 support with new include/openflow/openflow-1.3.h.
Most of the messages that differ from 1.2 are implemented. OFPT_SET_ASYNC
is implemented via NX_SET_ASYNC_CONFIG, other new message types are yet to
be implemented. Stats replies that add duration fields are implemented at
encode/decode level only. Test cases for implemented features are included.
Remaining FIXME:s should not cause runtime aborts. Make check comes out
clean.
Signed-off-by: Jarno Rajahalme <jarno.rajahalme@nsn.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Override the allowed versions of the snoop vconn so that
only the version of the controller connection is allowed.
This is because the snoop will see the same messages as the
controller.
Without this change the snoop will try to negotiate a connection
using the default allowed versions, or in other words only
allow OpenFlow 1.0. This breaks snoops for controller connections
using other OpenFlow versions, that is OpenFlow 1.2.
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Add allowed OpenFlow versions to struct rconn to allow
reconnect to use these parameters rather than hard-coded defaults.
This is in preparation for allowing configuration of the
allowed OpenFlow versions.
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This is in preparation for allowing the range of allowed OpenFlow versions
to be configured.
As part of this change pvconn_open() is now paramatised over the allowed
versions. this is to avoid avoids needing to provide version information
as a parameter to pvconn_accept(). This will in turn avoid the need to
pass version information to connmgr_run().
Signed-off-by: Simon Horman <horms@verge.net.au>
[blp@nicira.com simplified slightly and generalize log messages]
Signed-off-by: Ben Pfaff <blp@nicira.com>
Until now, the rconn module has used messages received from the
controller as the sole means to determine that the connection is up.
This can interact badly with the OVS connection manager in ofproto,
which stops reading and processing messages from the receive queue
when there is a backlog in the send queue for a given connection
(because reading and processes messages is the main cause of messages
getting pushed onto the send queue). So, if a send queue backlog
lasts more than twice the inactivity probe interval, then the
connection drops, whether the controller is sending messages or not.
Dumping a large flow table can trigger this behavior if the controller
becomes temporarily busy or if the network between OVS and a
controller is slow. The problem can easily repeat itself, since upon
reconnection the controller will generally dump the flow table.
This commit fixes the problem by expanding the definition of
"activity" to include successfully sending an OpenFlow message that
was previously queued.
Bug #12789.
Reported-by: Natasha Gude <natasha@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
OpenFlow headers are not as uniform as they could be, with size, alignment,
and numbering changes from one version to another and across varieties
(e.g. ordinary messages vs. "stats" messages). Until now the Open vSwitch
internal APIs haven't done a good job of abstracting those differences in
header formats. This commit changes that; from this commit forward very
little code actually needs to understand the header format or numbering.
Instead, it can just encode or decode, or pull or put, the header using
a more abstract API using the ofpraw_, ofptype_, and other APIs in the
new ofp-msgs module.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Tested-by: Simon Horman <horms@verge.net.au>
Reviewed-by: Simon Horman <horms@verge.net.au>
This patch reapplies the changes that were reverted with the commit 59efa47
(Revert DSCP update changes.). It also addresses the problem introduced by
the original commits, cd8fca2 ((jsonrpc: Correctly setting the dscp value
before reconnect.) and b2e18d (No need to restart DB / OVS on changing
dscp value.), that caused numerous unit test failures on some systems (as
diagnosed by valgrind).
With this change there is no need to restart the DB or OVS on configuring a
different value for the manager or controller connection respectively. On
detecting a change in the dscp value on the socket, the previous socket is
closed and a new socket is created and connection is established with the new
configured dscp value.
Signed-off-by: Mehak Mahajan <mmahajan@nicira.com>
This reverts commit cd8fca2ba0 (jsonrpc:
Correctly setting the dscp value before reconnect.) and commit
b2e18db292 (No need to restart DB / OVS on
changing dscp value.), which on some systems causes numerous unit test
failures that valgrind diagnoses as:
Conditional jump or move depends on uninitialised value(s)
at 0x805F63F: jsonrpc_session_set_dscp (jsonrpc.c:1061)
by 0x804F45D: ovsdb_jsonrpc_server_set_remotes (jsonrpc-server.c:417)
by 0x804B775: reconfigure_from_db (ovsdb-server.c:656)
by 0x804C231: main (ovsdb-server.c:159)
Signed-off-by: Ben Pfaff <blp@nicira.com>
With this change there is no need to restart the DB or OVS on configuring a
different value for the manager or controller connection respectively. On
detecting a change in the dscp value on the socket, the previous socket is
closed and a new socket is created and connection is established with the new
configured dscp value.
Signed-off-by: Mehak Mahajan <mmahajan@nicira.com>
Replaced all instances of Nicira Networks(, Inc) to Nicira, Inc.
Feature #10593
Signed-off-by: Raju Subramanian <rsubramanian@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Before this patch, rconn_send() would delete 'b' on success, and
not on error. This is confusing and error-prone. This patch
causes rconn_send() to always delete 'b'.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
The changes allow the user to specify a separate dscp value for the
controller connection and the manager connection. The value will take
effect on resetting the connections. If no value is specified a default
value of 192 is chosen for each of the connections.
Feature #10074
Requested-by: Rajiv Ramanathan <rramanathan@nicira.com>
Signed-off-by: Mehak Mahajan <mmahajan@nicira.com>
The intention is that, as each OpenFlow 1.1 and 1.2 feature is added to Open
vSwitch, the corresponding protocol definitions will be broken up this way:
- Definitions that are the same in OF1.0 and OF1.1 will retain the "OFP"
or "ofp" prefix and move to openflow-common.h.
- Definitions that are specific to OF1.0 will be renamed with an "OFP10"
or "ofp10" prefix and stay in openflow-1.0.h.
- Definitions that are specific to OF1.1 or to OF1.1 and OF1.2 will be
renamed with an "OFP11" or "ofp11" prefix and move to openflow-1.1.h.
- Definitions that are specific to OF1.2 will be renamed with an "OFP12"
or "ofp12" prefix and move to openflow-1.2.h.
This commit starts this process with some basic OpenFlow definitions.
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Open vSwitch already handles a few different protocol variations, but it
does so in a nonuniform manner:
- OpenFlow 1.0 and NXM flow formats are distinguished using the NXFF_*
constant values from nicira-ext.h.
- The "flow_mod_table_id" feature setting is maintained in ofproto as
part of an OpenFlow connection's (ofconn's) state.
There's no way to easily communicate this state among components. It's
not much of a problem yet, but as more protocol support is added it seems
better to have an abstract, uniform way to represent protocol versions and
variants. This commit implements that by introducing a new type
"enum ofputil_protocol". Each ofputil_protocol value represents a variant
of a protocol version. Each value is a separate bit, so a single enum
can also represent a set of protocols, which is often useful as well.
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
I looked at almost every uint<N>_t in the tree to determine whether it was
really in network byte order, and converted the ones that were.
The only remaining ones, modulo my mistakes, are in openflow.h. I'm not
sure whether we should convert those, because there might be some value
in remaining close to upstream for this header.
I've never heard of anyone actually using controller discovery.
It adds a great deal of code to the source tree, and a little
bit of complication to ofproto, so this commit removes it.
ovs-vswitchd writes only the duration of its connection to or disconnection
from each controller to the database. This changes that behavior to write the
time since both the last connection and disconnection events regardless of
connection state. This mirrors the new behavior for reporting database manager
connection status.
Requested-by: Peter Balland <peter@nicira.com>
Bug #4833.
At first glance the vconn_wait() call looks risky because this function
checked whether rc->vconn is nonnull at the top. In fact it's OK because
rc->state will be S_ACTIVE or S_IDLE only if rc->vconn is nonnull, but
there's no harm in putting that check inside the block that only runs if
rc->vconn is nonnull.
Coverity #10714.
ovs_queue doesn't seem very useful; it's just a singly-linked list. It's
more generally useful to use a general-purpose "struct list" for lists of
packets, so this commit adds such a member to "struct ofpbuf" and shifts
the existing users to use it.
Until now, the collection of coverage counters supported by a given OVS
program was not specific to that program. That means that, for example,
even though ovs-dpctl does not have anything to do with mac_learning, it
still has a coverage counter for it. This is confusing, at best.
This commit fixes the problem on some systems, in particular on ones that
use GCC and the GNU linker. It uses the feature of the GNU linker
described in its manual as:
If an orphaned section's name is representable as a C identifier then
the linker will automatically see PROVIDE two symbols: __start_SECNAME
and __end_SECNAME, where SECNAME is the name of the section. These
indicate the start address and end address of the orphaned section
respectively.
Systems that don't support these features retain the earlier behavior.
This commit also fixes the annoyance that files that include coverage
counters must be listed on COVERAGE_FILES in lib/automake.mk.
This commit also fixes the annoyance that modifying any source file that
includes a coverage counter caused all programs that link against
libopenvswitch.a to relink, even programs that the source file was not
linked into. For example, modifying ofproto/ofproto.c (which includes
coverage counters) caused tests/test-aes128 to relink, even though
test-aes128 does not link again ofproto.o.
Adding a macro to define the vlog module in use adds a level of
indirection, which makes it easier to change how the vlog module must be
defined. A followup commit needs to do that, so getting these widespread
changes out of the way first should make that commit easier to review.
Logging a few messages about a failed connection every few seconds for
every bridge is too much. This just logs failed connection messages for a
few attempts, then shuts them off until something changes.
Bug #2610.
