The 'last_disconn_secs' member determines whether we're currently in
fail-open mode (see fail_open_is_active()), but before this
commit, fail_open_run() could decide to enter fail-open mode even if
that would set 'last_disconn_secs' to 0 (and thus not really enter it).
This could lead to an endless stream of log messages about entering
fail-open mode, none of which actually does anything. This fixes the
problem.
(This patch worries me because this functionality has been stable
and unchanged for many years and I wonder how something so simple
is broken.)
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Ilya Maximets <i.maximets@ovn.org>
The functions to put ofpacts into ofpbufs have always padded them to
OFPACT_ALIGNTO boundaries, but the underlying structures weren't
necessarily padded out. That led to difficulties in a few places where
structures were allocated on the stack instead in an ofpbuf, because
functions like ofpact_init_*() would access beyond the end of the actual
structure. This is true, for example, in test_multipath_main() in
tests/test-multipath.c, which allocates a struct ofpact_multipath on the
stack, and in lswitch_handshake() in learning-switch.c, which allocates
a struct ofpact_output on the stack.
It's possible to fix these individual cases, but it's possible that there
are others that haven't been identified. This commit addresses the issue
another way, by padding all of the ofpact structures to a full multiple
of OFPACT_ALIGNTO and adding assertions to ensure that it can't be screwed
up in the future.
This commit removes the OFPACT_*_SIZE enums, because they are now
equivalent to sizeof(struct ofpact_*) in every case.
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Pull out the NORMAL flow add and deletion. It will be useful for
a follow up patch.
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
ofp-util had been far too large and monolithic for a long time. This
commit breaks it up into units that make some logical sense. It also
moves the pieces of ofp-parse that were specific to each unit into the
relevant unit.
Most of this commit is just moving code around.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Poll-loop is the core to implement main loop. It should be available in
libopenvswitch.
Signed-off-by: Xiao Liang <shaw.leon@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Rconn provides useful features over vconn. Make it available to library
users.
Signed-off-by: Xiao Liang <shaw.leon@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Sparse version 0.5.1 will be released in the near future. Prepare
for it by eliminating the only new warning (as of 0.5.1-rc5):
ofproto/fail-open.c:134:22: error: Initializer entry defined twice
ofproto/fail-open.c:135:22: also defined here
MATCH_CATCHALL_INITIALIZER effectively sets all fields of .flow to
zero, which is redundant because according to the C99 semantics for
structure initialization, initializing any single member of a
structure results in all other members being initialized to zero,
and the next line initializes a member of the same structure.
Signed-off-by: Lance Richardson <lrichard@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Set ofproto's connmgr pointer to NULL after the connmgr has been
destructed, and check for NULL when sending a flow removed
notification.
Verified by sending the flow removed message unconditionally and
observing numerous core dumps in the test suite.
Found by inspection.
Fixes: f695ebfae5 ("ofproto: Postpone sending flow removed messages.")
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
OVS implementation of buffering packets that are sent to the
controller is not compliant with the OpenFlow specifications after
OpenFlow 1.0, which is possibly true since OpenFlow 1.0 is not really
specifying the packet buffering behavior.
OVS implementation executes the buffered packet against the actions of
the modified or added rule, whereas OpenFlow (since 1.1) specifies
that the packet should be matched against the flow table 0 and
processed accordingly.
Rather than fix this behavior, and potentially break OVS users, the
packet buffering feature is removed altogether. After all, such
packet buffering is an optional OpenFlow feature, and as such any
possible users should continue to work without this feature.
This patch also makes OVS check the received 'buffer_id' values more
rigorously, and fixes some internal users accordingly.
Found by inspection.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
This commit also adds several #include directives in source files in
order to make the 'ofp-util.h' move possible
Signed-off-by: Ben Warren <ben@skyportsystems.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
One purpose of OpenFlow packet-in messages is to allow a controller to
interpose on the path of a packet through the flow tables. If, for
example, the controller needs to modify a packet in some way that the
switch doesn't directly support, the controller should be able to
program the switch to send it the packet, then modify the packet and
send it back to the switch to continue through the flow table.
That's the theory. In practice, this doesn't work with any but the
simplest flow tables. Packet-in messages simply don't include enough
context to allow the flow table traversal to continue. For example:
* Via "resubmit" actions, an Open vSwitch packet can have an
effective "call stack", but a packet-in can't describe it, and
so it would be lost.
* A packet-in can't preserve the stack used by NXAST_PUSH and
NXAST_POP actions.
* A packet-in can't preserve the OpenFlow 1.1+ action set.
* A packet-in can't preserve the state of Open vSwitch mirroring
or connection tracking.
This commit introduces a solution called "continuations". A continuation
is the state of a packet's traversal through OpenFlow flow tables. A
"controller" action with the "pause" flag, which is newly implemented in
this commit, generates a continuation and sends it to the OpenFlow
controller in a packet-in asynchronous message (only NXT_PACKET_IN2
supports continuations, so the controller must configure them with
NXT_SET_PACKET_IN_FORMAT). The controller processes the packet-in,
possibly modifying some of its data, and sends it back to the switch with
an NXT_RESUME request, which causes flow table traversal to continue. In
principle, a single packet can be paused and resumed multiple times.
Another way to look at it is:
- "pause" is an extension of the existing OFPAT_CONTROLLER
action. It sends the packet to the controller, with full
pipeline context (some of which is switch implementation
dependent, and may thus vary from switch to switch).
- A continuation is an extension of OFPT_PACKET_IN, allowing for
implementation dependent metadata.
- NXT_RESUME is an extension of OFPT_PACKET_OUT, with the
semantics that the pipeline processing is continued with the
original translation context from where it was left at the time
it was paused.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>
An upcoming commit will introduce another member that has a length, and
it seems weird that bare 'len' would be one or the other.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>
An upcoming commit will add another kind of asynchronous message that
should be handled in the same way as packet-ins.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Packet-in messages have been a bit of a mess. First, their abstraction
in the form of struct ofputil_packet_in has some fields that are used
in a clear way for incoming and outgoing packet-ins, and others
(packet_len, total_len, buffer_id) have have confusing meanings or
usage pattern depending on their direction.
Second, it's very confusing how a packet-in has both a reason (OFPR_*)
and a miss type (OFPROTO_PACKET_IN_*) and how those add up to the
actual reason that is used "on the wire" for each OpenFlow version (and
even whether the packet-in is sent at all!).
Finally, there's all kind of low-level detail randomly scattered between
connmgr, ofproto-dpif-xlate, and ofp-util.
This commit attempts to clear up some of the confusion. It simplifies
the struct ofputil_packet_in abstraction by removing the members that
didn't have a clear and consistent meaning between incoming and outgoing
packet-ins. It gets rid of OFPROTO_PACKET_IN_*, instead adding a couple
of nonstandard OFPR_* reasons that add up to what OFPROTO_PACKET_IN_*
was meant to say (in what I hope is a clearer way). And it consolidates
the tricky parts into ofp-util, where I hope it will be easier to
understand all in one place.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jarno Rajahalme <jarno@ovn.org>
Until now, composing a fixed-length action with ofpact_put_<NAME>() failed
to append any padding required after the action. This commit changes that
so that these calls now add padding. This meant that the function
ofpact_pad(), which was until now required in various unintuitive places,
is no longer required, and removes it.
Variable-length actions still require calling ofpact_update_len() after
composition. I don't see a way to avoid that.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Russell Bryant <russell@ovn.org>
Define struct eth_addr and use it instead of a uint8_t array for all
ethernet addresses in OVS userspace. The struct is always the right
size, and it can be assigned without an explicit memcpy, which makes
code more readable.
"struct eth_addr" is a good type name for this as many utility
functions are already named accordingly.
struct eth_addr can be accessed as bytes as well as ovs_be16's, which
makes the struct 16-bit aligned. All use seems to be 16-bit aligned,
so some algorithms on the ethernet addresses can be made a bit more
efficient making use of this fact.
As the struct fits into a register (in 64-bit systems) we pass it by
value when possible.
This patch also changes the few uses of Linux specific ETH_ALEN to
OVS's own ETH_ADDR_LEN, and removes the OFP_ETH_ALEN, as it is no
longer needed.
This work stemmed from a desire to make all struct flow members
assignable for unrelated exploration purposes. However, I think this
might be a nice code readability improvement by itself.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
We have a special flow_metadata structure to represent the parts
of a packet that aren't carried in the payload itself. This is
used in the case where we need to send the packet as a Packet In
to an OpenFlow controller. This is a subset of the more general
struct flow.
In practice, almost all operations we do on this structure involve
converting it to or from a match or have code that is the same as
a match. Serialization to NXM and back is done as a match. There
is special flow_metadata formatting code that is almost identical
to match formatting.
The uses for struct flow_metadata aren't performance critical
when it comes to memory, so we can save quite a bit of code by
just using a match structure directly instead. In addition, as
metadata increases and becomes more complex (Geneve options require
some special handling beyond just additional fields), using the
match structure means we only have to do this work in one place.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
ofpbuf was complicated due to its wide usage across all
layers of OVS, Now we have introduced independent dp_packet
which can be used for datapath packet, we can simplify ofpbuf.
Following patch removes DPDK mbuf and access API of ofpbuf
members.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Currently dp-packet make use of ofpbuf for managing packet
buffers. That complicates ofpbuf, by making dp-packet
independent of ofpbuf both libraries can be optimized for
their own use case.
This avoids mapping operation between ofpbuf and dp_packet
in datapath upcalls.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
The hidden rules created by in-band control and fail-open should not be
included in the table stats reported via OpenFlow. I seem to recall that
this was done correctly in some previous version but it has broken since
then. This commit fixes the problem and adds a test that should make it
harder to break again in the future.
Reported-by: Ashok Chippa <a.n.chippa@gmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
Also moves definitions for struct vconn and pvconn to the public
header. The provider interface is kept private.
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
A new function vlog_insert_module() is introduced to avoid using
list_insert() from the vlog.h header.
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Replace the generated_by_table_miss field of struct ofproto_packet_in
with a miss_type field.
The generated_by_table_miss field allowed packet-in messages generated
by table-miss rules to be differentiated. This differentiation
is still provided for by miss_type being set to OFPROTO_PACKET_IN_MISS_FLOW.
This patch allows further differentiation by setting miss_type
to OFPROTO_PACKET_IN_MISS_WITHOUT_FLOW if the packet-in message
is generated by a table-miss which is not handled by a table-miss rule.
This is in preparation for OpenFlow 1.3 version-specific
handling of the default action for such misses.
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
These would have found the problem fixed in commit c7be3f5593 (connmgr:
Fix attempt to take mutex recursively when exiting fail-open.).
Signed-off-by: Ben Pfaff <blp@nicira.com>
As per spec, make packet-in reason for OpenFlow1.3 table-miss flow
entries no_match rather than action.
Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>
send_len is not directly part of the OpenFlow packet_in message, at least
given that it is partially redundant with packet_len. send_len is, rather,
a request to the connmgr that expresses how many bytes the action requested
be sent to the controller, but the connmgr cannot always honor it.
Signed-off-by: Ben Pfaff <blp@nicira.com>
struct ofputil_packet_in mixes data included in OpenFlow packet_in messages
with data that used internally by ofproto and connmgr to queue and route
packet_ins. This commit begins disentangling these purposes by adding a
new struct ofproto_packet_in that wraps struct ofputil_packet_in. Adding
this new level of indirection causes a lot of code churn, so this commit
mainly takes care of that to make the remaining changes easier to read.
This commit does move the list node used for queuing packet_ins into the
new wrapper structure.
Signed-off-by: Ben Pfaff <blp@nicira.com>
This makes 'ofproto_mutex' protect the flow table well enough that threads
other than the main one can realistically modify flows.
I need to look at the interface between ofproto and connmgr: I think that
there might need to be some locking there too.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
OpenFlow actions have always been somewhat awkward to handle.
Moreover, over time we've started creating actions that require more
complicated parsing. When we maintain those actions internally in
their wire format, we end up parsing them multiple times, whenever
we have to look at the set of actions.
When we add support for OpenFlow 1.1 or later protocols, the situation
will get worse, because these newer protocols support many of the same
actions but with different representations. It becomes unrealistic to
handle each protocol in its wire format.
This commit adopts a new strategy, by converting OpenFlow actions into
an internal form from the wire format when they are read, and converting
them back to the wire format when flows are dumped. I believe that this
will be more maintainable over time.
Thanks to Simon Horman and Pravin Shelar for reviews.
Signed-off-by: Ben Pfaff <blp@nicira.com>
The name compose_rarp() more clearly describes what it's doing now.
Requested-by: Justin Pettit <jpettit@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Traditionally Open vSwitch had used 802.2 SNAP packets to update
upstream switch learning tables when necessary. This approach had
advantages in that debugging information could be embedded in the
packet helping hapless admins figure out what's going on. However,
since both qemu and VMware use RARP for this purpose, it seems
appropriate to fall in line with the defacto standard.
Requested-by: Ben Basler <bbasler@nicira.com>
Signed-off-by: Ethan Jackson <ethan@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>
Otherwise even controllers that should not receive any packet-ins (via
enable-async-messages=false) still receive the packet-ins that probe for
a controller being up when we're in fail-open.
Bug #9964.
Reported-by: James Schmidt <jschmidt@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
An upcoming commit will start referring to OpenFlow 1.1 actions, which are
renumbered relative to OpenFlow 1.0 actions, so this commit prepares by
changing all the existing uses of OFPAT_* to instead use OFPAT10_*.
This commit also introduces the OFPAT11_* constants.
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Back in 2008 or so, I introduced this extension as a way to provide
information about switch status to the new "switch UI" program. Since
then, the switch UI program has been removed and the important information
that was provided by the switch status extension is now available in the
database, so we might as well get rid of this extension, and that is what
this commit does.
None of the existing callers of ofproto_add_flow() want an idle timeout,
but ofproto_add_flow() was giving them a 5-second timeout anyway. I don't
know how this worked properly--in-band will patiently add the flow back
every few seconds anyway, but the bridge doesn't do that.
Also add an explanatory comment to ofproto_add_flow().
When userspace and the kernel were using the same structure for flows,
flow_t was a useful way to indicate that a structure was really a userspace
flow instead of a kernel one, but now it's better to just write "struct
flow" for consistency, since OVS doesn't use typedefs for structs
elsewhere.
Acked-by: Jesse Gross <jesse@nicira.com>
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.
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.
