struct list is a common name and can't be used in public headers.
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
When a connection takes a few rounds of the state machine to complete,
'error' gets filled with EAGAIN until that completes. This didn't match
the vconn_get_status() documentation, which says that it only returns a
positive errno value if there was an error. One could fix the problem
by updating the documentation (and the callers) or by updating the
implementation. I decided that the latter was the way to go because
the distinction between the TCP connection being in progress or complete
isn't visible to the client; what is visible to the client is the OpenFlow
negotiation being complete.
This problem is difficult to find in the unit tests because TCP connections
to localhost complete immediately.
Bug introduced by commit accaecc419 (rconn: Discover errors in
rconn_run() even if rconn_recv() is never called.)
Reported-by: Anuprem Chalvadi <achalvadi@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
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>
Testers keep wanting to know why this doesn't work. I think it's a silly
test, but it's easy enough to make them happy.
Signed-off-by: Ben Pfaff <blp@nicira.com>
The OVS code has always made a distinction between the unencrypted (TCP)
and SSL port numbers for the OpenFlow and OVSDB protocols. The default
port numbers for both protocols has changed, and there continues to be
no distinction between the unencrypted and SSL versions. This
commit removes the distinction in port numbers. A future patch will
recognize the change in default port number.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
This uninitialized data caused failures in the test "ofproto -
eviction upon table overflow (OpenFlow 1.2)" for some developers and in
some circumstances.
Found by valgrind.
Reported-by: Justin Pettit <jpettit@nicira.com>
Signed-off-by: Ben Pfaff <blp@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>
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>
Allow encoding and decoding of version bitmap in hello messages
as specified in Open Flow 1.3.1.
Signed-off-by: Simon Horman <horms@verge.net.au>
[blp@nicira.com simplified and generalized decode/encode functions]
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, it seems that all vconn users have immediately started reading
messages from the connection. Today, however, I added a new user that
only wants to read packets after the OpenFlow version is negotiated, so
it never called vconn_recv() before that happened. It turns out that if
you do this, the version never gets negotiated at all.
This commit fixes the problem by ensuring that vconn_run() will continue
version negotiation if it isn't done yet.
This changes the error return that I get for Unix sockets in the
test-vconn "accept-then-close" test from EPIPE to ECONNRESET, so this
commit also adjusts that test to accept either error code; both of them
seem reasonable enough to me.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Make ofperr_encode_msg__() use correct Open Flow version in the header
of messages.
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
On FreeBSD, sometimes plain vconn_connect() or vconn_recv() reports EAGAIN
in these cases.
Reported-by: Ed Maste <emaste@freebsd.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Use an enum for ofp_version in ofp-util and ofp-msg.
This in conjunction with the use of switch() statements
allows the compiler to warn when a new ofp_version isn't handled.
Signed-off-by: Simon Horman <horms@verge.net.au>
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>
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>
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>
This commit switches from using the actual protocol values of error codes
internally in Open vSwitch, to using abstract values that are translated to
and from protocol values at message parsing and serialization time. I
believe that this makes the code easier to read and to write.
This is also one step along the way toward OpenFlow 1.1 support because
OpenFlow 1.1 renumbered a bunch of error codes.
Signed-off-by: Ben Pfaff <blp@nicira.com>
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.
The 'xid' in an ofp_header is not interpreted by the receiver but only by
the sender, so it need not be in any particular byte order. OVS used to
try to take advantage of this to avoid host/network byte order conversions
for this field. Older code in OVS, therefore, treats xid as being in host
byte order. However, as time went on, I forgot that I had introduced this
trick, and so newer code treats xid as being in network byte order.
This commit fixes up the situation by consistently treating xid as being
in network byte order. I think that this will be less surprising and
easier to remember in the future.
This doesn't fix any actual bugs except that some log messages would have
printed xids in the wrong byte order.
All streams and all vconns are "active", so there's no point in noting that
requirement in comments. (A long time ago, active and passive vconns were
conflated instead of having passive vconns broken out as pvconns. But
active and passive streams have always been distinct.)
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.
While I was looking at the rconn code for connection backoff and retry, I
noticed that ovs-vswitchd was logging the following on each connection
attempt:
Jun 11 15:17:41|00020|vconn_stream|ERR|send: Connection refused
The "send:" part didn't make much sense. The configured controller was not
actually running, so the vconn code should not have been able to connect
at all, so the message should have been about a connection failing, not
about sending on a completed connection failing.
Investigation showed that different parts of the library have different
ideas about return value semantics. vconn_open() and stream_open() both
return 0 if a connection succeeded or if one is in progress, but some of
its callers thought that it returned 0 if the connection succeeded and
EAGAIN if the connection was in progress. This commit fixes up the callers
that had the wrong idea, by making them instead all vconn_connect() or
stream_connect() to determine whether the connection is complete.
The main purpose of the vconn code is to ship OpenFlow messages across
network connections. Over time a large number of utility functions related
to OpenFlow messages have also crept into vconn.c, but that's really
logically separate. This commit breaks those functions out into a new
file.
Add a tun_id field which contains the ID of the encapsulating tunnel
on which a packet was received (0 if not received on a tunnel). Also
add an action which allows the tunnel ID to be set for outgoing
packets. At this point there aren't any tunnel implementations so
these fields don't have any effect.
The matching is exposed to OpenFlow by overloading the high 32 bits
of the cookie as the tunnel ID. ovs-ofctl is capable of turning
on this special behavior using a new "tun-cookie" command but this
command is intentially undocumented to avoid it being used without
a full understanding of the consequences.
The fatal-signal library notices and records fatal signals (e.g. SIGTERM)
and terminates the process on the next trip through poll_block(). But
some special utilities do not always invoke poll_block() promptly, e.g.
"ovs-ofctl monitor" does not call poll_block() as long as OpenFlow messages
are available. But these special cases seem like they are all likely to
call into functions that themselves block (those with "_block" in their
names). So make a new rule that such functions should always call
fatal_signal_run(), either directly or through poll_block(). This commit
implements and documents that rule.
Bug #2625.
OpenFlow 1.0 adds support for matching on IP ToS/DSCP bits.
NOTE: OVS at this point is not wire-compatible with OpenFlow 1.0 until
the final commit in this OpenFlow 1.0 set.
