Ben Pfaff [Mon, 22 Aug 2011 21:52:21 +0000 (14:52 -0700)]
ovsdb-data: Simplify converting an OVSDB datum to JSON by reordering logic.
Putting the "map" case first avoids duplicate tests.
Suggested-by: Reid Price <reid@nicira.com>
Ben Pfaff [Mon, 22 Aug 2011 21:40:09 +0000 (14:40 -0700)]
daemon: Stylistic improvement for __read_pidfile in Python implementation.
Suggested-by: Reid Price <reid@nicira.com>
Ben Pfaff [Mon, 22 Aug 2011 21:26:58 +0000 (14:26 -0700)]
daemon: Correct comment in Python implementation.
Reported-by: Reid Price <reid@nicira.com>
Ben Pfaff [Tue, 23 Aug 2011 17:50:47 +0000 (10:50 -0700)]
python: Avoid using 'tuple' as a variable name.
'tuple' is a Python built-in function, so it's best to avoid using it as a
variable name.
Suggested-by: Reid Price <reid@nicira.com>
Ben Pfaff [Tue, 23 Aug 2011 16:50:46 +0000 (09:50 -0700)]
python: Avoid using 'type' as a variable name.
'type' is a Python built-in function, so it's best to avoid using it as
a variable name.
Reported-by: Reid Price <reid@nicira.com>
Ben Pfaff [Mon, 22 Aug 2011 23:49:53 +0000 (16:49 -0700)]
python: Take advantage of Python "x < y < z" syntax.
Suggested-by: Reid Price <reid@nicira.com>
Ben Pfaff [Mon, 22 Aug 2011 23:54:28 +0000 (16:54 -0700)]
python: Avoid lots of \" in quoted strings by using '' as outermost quotes.
Suggested-by: Reid Price <reid@nicira.com>
Ben Pfaff [Tue, 23 Aug 2011 21:02:03 +0000 (14:02 -0700)]
python: Join a list of strings instead of concatenating a long string.
Python does not do a good job of appending strings: it takes O(n**2) time
to append n strings.
Suggested-by: Reid Price <reid@nicira.com>
Ben Pfaff [Mon, 22 Aug 2011 21:31:18 +0000 (14:31 -0700)]
python: Use getattr() and setattr() instead of __dict__.
This leaves one use of __dict__ used for iterating through attributes.
I could use dir() instead, but I was put off by this note in its
documentation in the Python Library Reference:
Because dir() is supplied primarily as a convenience for use at an
interactive prompt, it tries to supply an interesting set of names more
than it tries to supply a rigorously or consistently defined set of names,
and its detailed behavior may change across releases. For example,
metaclass attributes are not in the result list when the argument is a
class.
Suggested-by: Reid Price <reid@nicira.com>
Ben Pfaff [Wed, 24 Aug 2011 18:57:14 +0000 (11:57 -0700)]
ovs.db.data: Fix bugs in Atom.is_default() and Datum.is_default().
Reported-by: Reid Price <reid@nicira.com>
Ben Pfaff [Tue, 23 Aug 2011 18:09:46 +0000 (11:09 -0700)]
ovs.stream: Fix logic bug in Stream.connect().
The loop here is supposed to run at least once, and to continue looping as
long as the loop body changes the current state, but this bug caused it to
continue looping until the connection completed in success or failure. It
probably didn't cause many problems in practice because only Unix domain
socket connections are currently supported, and those connections normally
complete immediately.
Reported-by: Reid Price <reid@nicira.com>
Simon Horman [Wed, 24 Aug 2011 01:40:55 +0000 (10:40 +0900)]
Debian: set -e in brcompat postinst
As reported by lintian:
The maintainer script doesn't seem to set the -e flag which ensures
that the script's execution is aborted when any executed command
fails.
Refer to Debian Policy Manual section 10.4 (Scripts) for details.
Ben Pfaff [Wed, 24 Aug 2011 16:45:18 +0000 (09:45 -0700)]
doc: Add spaces between commas to avoid overflowing line during formatting.
This resolves a warning reported as
"71: warning [p 11, 5.0i]: cannot adjust line"
Reported-by: Simon Horman <horms@verge.net.au>
Found by lintian.
Simon Horman [Wed, 24 Aug 2011 01:40:53 +0000 (10:40 +0900)]
docs: Add missing escape
This adds what appears to be a missing character to an escape sequence.
This resolves a problem reported as
"a newline character is not allowed in an escape name".
Reported by lintian
Simon Horman [Wed, 24 Aug 2011 01:40:52 +0000 (10:40 +0900)]
docs: Suppress "warning: macro `DD' not defined" warning
Suppress "warning: macro `DD' not defined" warning for ovs-brcompatd.8.
As per the description by Ben Pfaff for the same problem effecting
other files:
deamon.man allows the file that is including it to include extra
text in the description of --detach by defining a macro named DD.
Only some of the manpages that included it did this (only those
manpages that needed extra text there). But it's better to be
quiet in "man --warnings", so this defines DD to an empty value in
the other manpages that include daemon.man.
Reported by lintian
Simon Horman [Wed, 24 Aug 2011 01:40:51 +0000 (10:40 +0900)]
Debian: Add dependency on ${misc:Depends}
Add dependency on ${misc:Depends} to openvswitch-brcompat and ovsdbmonitor.
As reported by Lintian:
The source package uses debhelper, but it does not include
${misc:Depends} in the given binary package's debian/control entry.
Any debhelper command may add dependencies to ${misc:Depends} that
are required for the work that it does, so recommended best
practice is always add ${misc:Depends} to the dependencies of each
binary package if debhelper is in use.
Refer to the debhelper(7) manual page for details.
Ethan Jackson [Sat, 20 Aug 2011 00:50:32 +0000 (17:50 -0700)]
dpif-linux: Call rtnetlink_notifier_run() as required.
I don't think this actually fixes a bug, as netdev-linux calls this
function. However, it seems stylistically more correct.
Ethan Jackson [Mon, 22 Aug 2011 19:47:43 +0000 (12:47 -0700)]
rtnetlink: Notifiers should only run once per poll loop.
rtnetlink_notifier_run() does quite a bit of work, and is likely
only to have interesting effects once per poll loop.
Ben Pfaff [Mon, 22 Aug 2011 17:41:21 +0000 (10:41 -0700)]
Drop spurious 'H' cases from daemon option parsing switch statements.
Help is 'h'. I don't see how 'H' can ever happen.
Ben Pfaff [Fri, 19 Aug 2011 22:43:06 +0000 (15:43 -0700)]
datapath: Remove unneeded { } around single statement.
I noticed this looking around at other code.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Justin Pettit [Thu, 18 Aug 2011 17:35:40 +0000 (10:35 -0700)]
datapath: Use "OVS_*" as opposed to "ODP_*" for user<->kernel interactions.
The prefix "ODP_*" is not overly descriptive in the context of the
larger Linux tree. This commit changes the prefix to "OVS_*" for the
userpace to kernel interactions. The userspace libraries still use
"ODP_" in many of their interfaces since it is more descriptive in the
OVS oeuvre.
Feature #6904
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Fri, 19 Aug 2011 21:29:27 +0000 (14:29 -0700)]
ofproto-dpif: Delete MAC learning entries when they expire.
Commit
fa066f015f716c7 "bridge: Move packet processing functionality into
ofproto" deleted the call to mac_learning_run() that deletes MAC learning
table entries when they expire. This fixes the problem.
Ben Pfaff [Thu, 18 Aug 2011 20:02:22 +0000 (13:02 -0700)]
classifier: Fix typo in comment.
Justin Pettit [Thu, 18 Aug 2011 17:31:15 +0000 (10:31 -0700)]
datapath-protocol: Correct a description in odp_flow_attr structure.
The description referenced "ODPAT_*", but it should be "ODP_ACTION_ATTR_".
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Thu, 18 Aug 2011 16:57:23 +0000 (09:57 -0700)]
datapath: Correct comment for vport_add().
The comment describing vport_add() incorrectly stated that the function
added the vport to the datapath. It is the responsibility of the caller
to do that.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Thu, 18 Aug 2011 17:33:32 +0000 (10:33 -0700)]
odp-util: Fix parsing of Ethertypes 0x8000 and above.
An existing comment in the function being updated explains the problem:
* Many of the sscanf calls in this function use oversized destination
* fields because some sscanf() implementations truncate the range of %i
* directives, so that e.g. "%"SCNi16 interprets input of "0xfedc" as a
* value of 0x7fff. The other alternatives are to allow only a single
* radix (e.g. decimal or hexadecimal) or to write more sophisticated
* parsers.
Philippe Jung [Wed, 17 Aug 2011 18:34:09 +0000 (11:34 -0700)]
INSTALL.Linux: Fix up reference to old option name --with-l26.
Ethan Jackson [Tue, 16 Aug 2011 21:14:33 +0000 (14:14 -0700)]
ofproto: Update 'struct facet''s comments.
Some of the comments in the definition of 'struct facet' had become
out of date.
Ethan Jackson [Tue, 16 Aug 2011 21:21:12 +0000 (14:21 -0700)]
ofproto: Remove extra_bytes parameter of facet_account().
It no longer has interesting users.
Ethan Jackson [Tue, 16 Aug 2011 21:16:58 +0000 (14:16 -0700)]
ofproto: Fix over accounting of byte counters.
The update_stats() function in ofproto was attributing more bytes
to facets than they had actually accrued. This could potentially
throw off bond load balancing.
Found by inspection.
Ethan Jackson [Tue, 16 Aug 2011 20:50:00 +0000 (13:50 -0700)]
ofproto: Clear packet and byte counters on flow additions.
When a flow is added to the flow table, its packet and byte
counters should be reset. This patch efficiently approximates this
behavior. It also does some minor code refactoring.
Bug #3183.
Justin Pettit [Mon, 15 Aug 2011 20:51:09 +0000 (13:51 -0700)]
vswitch.xml: Remove unused "hwaddr" other-config key.
Ben Pfaff [Mon, 15 Aug 2011 22:18:12 +0000 (15:18 -0700)]
ofp-print: Add missing "break".
This is not a bug yet, but it could be the next time someone carelessly
adds a new case.
Ben Pfaff [Mon, 15 Aug 2011 20:17:58 +0000 (13:17 -0700)]
ovs-ofctl: Fix a few formatting typos in manpage.
Ben Pfaff [Wed, 10 Aug 2011 19:46:36 +0000 (12:46 -0700)]
connmgr: Remove unused function ofconn_n_pending_opgroups().
Ben Pfaff [Wed, 10 Aug 2011 19:49:35 +0000 (12:49 -0700)]
poll-loop: Remove static variable n_waiters.
It's always a little risky to track the length of a list by hand, because
it is easy to miss a spot where the length can change. So it seems like
a small cleanup to just measure the length of the 'waiters' list at the
point where we need to know it. list_size() is O(n) in the length of the
list, but the function that calls it is already O(n) in that length so it
seems like a fair trade-off.
Ben Pfaff [Wed, 10 Aug 2011 19:40:10 +0000 (12:40 -0700)]
dpif-netdev: Avoid pointlessly maintaining a port count.
'n_ports' was only used for testing for nonzero, and we can rewrite the
code that does that to more straightforwardly use LIST_FOR_EACH_SAFE.
Ethan Jackson [Fri, 12 Aug 2011 21:10:15 +0000 (14:10 -0700)]
ofproto-dpif: Print register values in trace.
I found this patch useful in tracking down a bug recently.
Ethan Jackson [Wed, 10 Aug 2011 20:05:17 +0000 (13:05 -0700)]
nicra-ext: New action NXAST_OUTPUT_REG.
The NXAST_OUTPUT_REG action outputs to the OpenFlow port contained
in a supplied NXM field.
Ethan Jackson [Wed, 10 Aug 2011 20:32:51 +0000 (13:32 -0700)]
nx-match: New function nxm_read_field_bits().
nxm_read_field_bits() simplifies reading of NXM fields with an
ofs_nbits parameter. This patch updates nxm_execute_reg_move() to
use the new function. A user outside of the nx-match module will
be added in future patches.
Ethan Jackson [Wed, 10 Aug 2011 20:09:18 +0000 (13:09 -0700)]
nx-match: Update register check functions.
This patch simplifies the API of nxm_dst_check() and adds a new
function nxm_src_check() for checking source fields.
Ethan Jackson [Fri, 12 Aug 2011 02:07:35 +0000 (19:07 -0700)]
tests: test "load" and "move" actions.
Ethan Jackson [Fri, 12 Aug 2011 18:15:53 +0000 (11:15 -0700)]
nx-match: Fix bug in "move" action.
This patch fixes a bug introduced in Commit
43edca57 "nx-match: New
helpers.", which caused the "move" action to improperly handle bit
ranges.
Ethan Jackson [Fri, 29 Jul 2011 20:15:09 +0000 (13:15 -0700)]
flow: New FLOW_WC_SEQ build assertion.
Changing "struct flow" or its wildcards requires minor adjustments
in many places in the code. This patch adds a new FLOW_WC_SEQ
sequence number which when incremented will cause build assertion
failures aiding the developer in finding code which needs to
change.
Ethan Jackson [Wed, 10 Aug 2011 21:48:48 +0000 (14:48 -0700)]
tests: Update gitignore.
Ethan Jackson [Thu, 4 Aug 2011 23:50:25 +0000 (16:50 -0700)]
lib: Whitespace cleanup.
Ben Pfaff [Fri, 12 Aug 2011 21:59:11 +0000 (14:59 -0700)]
ofp-parse: Fix parsing of register values 2**31 and greater.
Reported-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Wed, 10 Aug 2011 23:50:41 +0000 (16:50 -0700)]
ovs-ofctl: Document that all actions are supported now.
This comment must be very old. ovs-ofctl has supported all the OpenFlow
and Nicira extensions actions for a long time now.
Ben Pfaff [Wed, 10 Aug 2011 16:13:12 +0000 (09:13 -0700)]
debian: Avoid installing duplicate files in ovsdbmonitor package.
This is just a typo introduced in commit
57483aeda (debian: Fix bug from
commit
211b05b5 "debian: Modernize use of dh_install.) that caused the
ovsdbmonitor package to install too many files.
Bug-report: http://bugs.debian.org/636815
Reported-by: Ralf Treinen <treinen@free.fr>
Ben Pfaff [Mon, 8 Aug 2011 21:48:48 +0000 (14:48 -0700)]
ofp-util: Rename struct flow_stats_request with ofputil_ prefix.
Most of the structs in ofp-util.h have the ofputil_ prefix. Rename this
one for consistency.
Ben Pfaff [Mon, 8 Aug 2011 21:46:38 +0000 (14:46 -0700)]
ofp-util: Rename struct flow_mod to struct ofputil_flow_mod.
Most of the structs in ofp-util.h have the ofputil_ prefix. Rename this
one for consistency.
Ben Pfaff [Tue, 9 Aug 2011 20:46:51 +0000 (13:46 -0700)]
nicira-ext: Fix NXM example.
The code and the specification say that nxm_length includes both value
and mask, but this example showed nxm_length only including the value.
This commit fixes it.
Reported-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Tue, 9 Aug 2011 19:55:13 +0000 (12:55 -0700)]
ovs-ofctl: Fix reading flows from file for "replace-flows", "diff-flows".
Commit
c821124b25e "ovs-ofctl: Accept only valid flow_mod and
flow_stats_request fields" caused actions read by read_flows_from_file()
to be ignored and treated as "drop". This fixes the problem.
Sanjay Sane [Tue, 9 Aug 2011 18:08:27 +0000 (11:08 -0700)]
Option to forward BPDU (Ethernet control class) frames
Currently, a NORMAL action bridge drops reserved-multicast-mac addresses;
01-80-c2-00-00-[f0:ff]. A node that does not implement STP should have an
option to forward such frames.
This commit proposes to have a configuration option to allow forwarding of
BPDU class frames. To ensure backward compatibility, this option is
disabled by default.
This config can be set using bridge's other-config column, for e.g
ovs-vsctl set bridge br0 other-config:forward-bpdu=true
Changing this option can revalidate all flows in a software-OVS
implementation (ofproto-dpif)
--------
unit tests:
------------
make config changes, test runtime behavior
-- test runtime behavior --
continuously send packets to br0 with dest-mac=01:80:c2:00:00:00
ovs-dpctl dump-flows br0
ovs-vsctl set bridge br0 other-config:forward-bpdu=true
ovs-dpctl dump-flows br0
ovs-vsctl set bridge br0 other-config:forward-bpdu=false
ovs-dpctl dump-flows br0
ovs-vsctl set bridge br0 other-config:forward-bpdu=true
ovs-dpctl dump-flows br0
ovs-vsctl remove bridge br0 other-config forward-bpdu=true
ovs-dpctl dump-flows br0
--result--
ovs-dpctl dump-flows br0
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:29550, bytes:
1773000, used:0.004s, actions:drop
ovs-vsctl set bridge br0 other-config:forward-bpdu=true
ovs-dpctl dump-flows br0
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:8209, bytes:492540, used:0.000s, actions:2,0
ovs-vsctl set bridge br0 other-config:forward-bpdu=false
ovs-dpctl dump-flows br0
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:19, bytes:1140, used:0.000s, actions:drop
ovs-vsctl set bridge br0 other-config:forward-bpdu=true
ovs-dpctl dump-flows br0
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:29, bytes:1740, used:0.000s, actions:2,0
ovs-vsctl remove bridge br0 other-config forward-bpdu=true
ovs-dpctl dump-flows br0
in_port(1),eth(src=00:0c:29:d1:39:42,dst=01:80:c2:00:00:00), packets:0, bytes:0, used:never, actions:drop
Bug #6624
Reported-by: Niklas Andersson <nandersson@nicira.com>
Ben Pfaff [Tue, 9 Aug 2011 16:24:18 +0000 (09:24 -0700)]
New action NXAST_RESUBMIT_TABLE.
This makes multiple table support in ofproto-dpif useful, by allowing
resubmits into tables other than 0.
Ben Pfaff [Thu, 28 Jul 2011 22:25:48 +0000 (15:25 -0700)]
ofproto-dpif: Add multiple table support.
Tables other than 0 can be modified and dumped, but they are not yet useful
because actions and flow table lookups never use them.
Ben Pfaff [Thu, 28 Jul 2011 22:02:06 +0000 (15:02 -0700)]
ofproto: New helper macro OFPROTO_FOR_EACH_TABLE.
In my opinion this makes the code slightly easier to read and write.
Ben Pfaff [Thu, 28 Jul 2011 22:01:20 +0000 (15:01 -0700)]
ofproto: Allow ->rule_choose_table() to be NULL regardless of table count.
In the upcoming software switch implementation of multiple tables, there is
no reason to prefer one table over another, so we always put rules into
table 0 by default. This commit allows this to be done simply by
specifying NULL as ->rule_choose_table().
Ben Pfaff [Thu, 28 Jul 2011 21:19:52 +0000 (14:19 -0700)]
ofproto: Make ->construct() and ->destruct() more symmetrical.
The ofproto_provider's ->construct() was required to allocate and
initialize the flow tables, but ->destruct() was not allowed to
uninitialize and free them. This arrangement is oddly asymmetrical (and
undocumented), so this commit changes the code so that the client is
responsible for both allocation and freeing.
Reported-by: Hao Zheng <hzheng@nicira.com>
CC: Hao Zheng <hzheng@nicira.com>
Ben Pfaff [Mon, 8 Aug 2011 23:01:05 +0000 (16:01 -0700)]
ofproto-dpif: Fix pointer arithmetic on null pointer.
rule_dpif_lookup() can return a null pointer as 'rule' so we shouldn't
try to calculate '&rule->up' before it's been found to be nonnull.
This doesn't appear to fix a real bug because 'up' is the first member
of 'rule' so when rule is NULL then &rule->up is also NULL.
Reported-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Mon, 8 Aug 2011 22:43:29 +0000 (15:43 -0700)]
ofproto-dpif: Make ofproto/trace accept an odp_flow in place of a packet.
It's often easier to get a flow than it is to get a packet. For example,
"ovs-dpctl dump-flows" prints flows, but it doesn't print any of the
packets in those flows. It is also easier to construct a flow by hand than
it is to construct packet bytes.
This commit, therefore, makes ofproto/trace accept a flow specification in
place of packet data. In a few cases a flow by itself is not sufficient
to determine what would happen to a packet, so in those cases the command
prints a message to that effect.
An upcoming commit will also start using this feature to unit-test action
translation in ofproto-dpif.
Suggested-by: Reid Price <reid@nicira.com>
Ben Pfaff [Thu, 4 Aug 2011 23:20:34 +0000 (16:20 -0700)]
odp-util: New function odp_flow_key_from_string().
This will be used in upcoming commits.
Ben Pfaff [Thu, 4 Aug 2011 20:32:19 +0000 (13:32 -0700)]
odp-util: Format VLAN headers more like other headers in ODP flow output.
The rest of the headers all follow the form "header(value)" or
"header(key1=value1,key2=value2,...)" but VLAN headers left out the "="
characters. This adds them in for consistency.
Ben Pfaff [Fri, 5 Aug 2011 21:18:06 +0000 (14:18 -0700)]
netdev: Get rid of struct netdev_options and netdev_open_default().
Now that netdev_options only has two members, we might as well pass them
directly as parameters.
Ben Pfaff [Mon, 8 Aug 2011 19:49:17 +0000 (12:49 -0700)]
netdev: Decouple creating and configuring network devices.
Until now, each call to netdev_open() for a particular network device
had to either specify a set of network device arguments that was either
empty or (for devices that already existed) equal to the existing device's
configuration. Unfortunately, the definition of "equality" in the latter
case was mostly done in terms of strict equality of string-to-string maps,
which caused problems in cases where, for example, one set of arguments
specified the default value of an optional argument explicitly and the
other omitted it.
The netdev interface does have provisions for defining equality other ways,
but this had only been done in one case that was especially problematic in
practice. One way to solve this particular problem would be to carefully
define equality in all the problematic cases.
This commit takes another approach based on the realization that there is
really no need to do any comparisons. Instead, it removes configuration
at netdev_open() time entirely, because almost all of netdev_open()'s
callers are not interested in creating and configuring a netdev. Most of
them just want to open a configured device and use it. Therefore, this
commit stops providing any configuration arguments to netdev_open() and the
provider functions that it calls. Instead, a caller that does want to
configure a device does so after it opens it, by calling
netdev_set_config().
This change allows us to simplify the netdev interface a bit. There is no
longer any need to implement argument comparisons. As a result, there is
also no need for "struct netdev_dev" to keep track of configuration at all.
Instead, the network devices that have configuration keep track of it in
their own internal form.
This new interface does mean that it becomes possible to accidentally
create and try to use an unconfigured netdev that requires configuration.
Bug #6677.
Reported-by: Paul Ingram <paul@nicira.com>
Ben Pfaff [Mon, 8 Aug 2011 17:58:38 +0000 (10:58 -0700)]
debian: Ensure that /var/run/openvswitch exists in controller init script.
It would be better to use ovs-ctl from this script, but until now this is
an adequate solution.
Reported-by: Jibesh Patra
Bug-report: https://bugs.launchpad.net/bugs/822142
Ben Pfaff [Fri, 5 Aug 2011 21:15:32 +0000 (14:15 -0700)]
netdev: Clean up and refactor packet receive interface.
The Open vSwitch tree only has one user of the ability for a netdev to
receive packets from a network device. Thus, this commit simplifies the
common-case use of the netdev interface by replacing the "ethertype" option
from "struct netdev_options" by a new netdev_listen() call.
The only user of netdev_listen() wants to receive all packets from a
network device, so this commit also removes the ability to restrict the
received packets to a particular protocol. (This ability was once used by
the Open vSwitch integrated DHCP client, but that code has been removed.)
This commit also simplifies and improves the implementation of the code
in netdev-linux that started listening to a network device. Before, I had
not figured out how to avoid receiving all packets on all devices before
binding to a particular device, but I took a closer look at the kernel code
and figured it out.
I've tested that the userspace datapath (dpif-netdev), the only user of
netdev_recv(), still works after this change.
Ben Pfaff [Fri, 5 Aug 2011 21:14:18 +0000 (14:14 -0700)]
bridge: Add port to datapath before trying to retrieve device stats.
Virtual ports such as GRE tunnels don't exist until after the port is
added to the datapath, so without this change adding such a port yields
a warning like the following:
netdev|WARN|failed to retrieve MTU for network device gre0: No such device
Ben Pfaff [Fri, 5 Aug 2011 22:56:05 +0000 (15:56 -0700)]
ovsdb: Correct specification inconsistency between "lock" and "assert".
The "lock" request requires the lock name to be an <id> but it is shown as
<string> in the "assert" operation. This corrects the "assert"
specification and fixes the suggested naming convention (since ":" is not
valid in an <id>).
This commit also updates the implementation to match the specification.
Reported-by: Jeremy Stribling <strib@nicira.com>
Ben Pfaff [Fri, 5 Aug 2011 22:48:45 +0000 (15:48 -0700)]
ofp-util: Rewrite action decoding to improve compiler warnings.
The previous implementation of ofputil_decode_action() had two weaknesses.
First, the action lengths in its tables were written as literal integers
instead of using "sizeof". Second, it used arrays for tables instead of
switch statements, which meant that GCC didn't warn when someone added a
new action type but forgot to add an entry to the tables.
This rewrite fixes both of those problems.
Suggested-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Fri, 5 Aug 2011 23:58:02 +0000 (16:58 -0700)]
Document and warn that mirroring to a VLAN is incompatible with SLB bonding.
vswitchd/INTERNALS explains the incompatibility:
2. When Open vSwitch forwards a multicast or broadcast packet to a
link in the SLB bond other than the active slave, the remote
switch will forward it to all of the other links in the SLB
bond, including the active slave. Without special handling,
this would mean that Open vSwitch would forward a second copy of
the packet to each switch port (other than the bond), including
the port that originated the packet.
Open vSwitch deals with this case by dropping packets received
on any SLB bonded link that have a source MAC+VLAN that has been
learned on any other port. (This means that SLB as implemented
in Open vSwitch relies critically on MAC learning. Notably, SLB
is incompatible with the "flood_vlans" feature.)
We could go farther than this and automatically change the bonding mode to
a safer one (e.g. active-backup) when flood_vlans are enabled. However,
that would still leave the SLB fallback for LACP modes in place; perhaps
active-backup would have to be the fallback for LACP modes when flood_vlans
are enabled.
Ben Pfaff [Mon, 8 Aug 2011 17:17:30 +0000 (10:17 -0700)]
Document that mirroring to a GRE tunnel works and is better than RSPAN.
Ben Pfaff [Mon, 8 Aug 2011 16:35:01 +0000 (09:35 -0700)]
ChangeLog: Rename NEWS.
Many users seem to miss changes that Open vSwitch clearly states in
ChangeLog. Some of this is probably due to users not reading any
documentation ever, but I wonder whether some of it is because users
expect ChangeLog to be akin to a commit log, because that's what GNU
software puts in ChangeLog. Users don't read GNU ChangeLogs, because
they don't contain information useful to users.
GNU software puts what we have in ChangeLog in a file named NEWS. Some
users read it. Therefore, this commit renames ChangeLog to NEWS.
Also, strip trailing whitespace for Ethan-compliance.
Ben Pfaff [Fri, 5 Aug 2011 21:08:54 +0000 (14:08 -0700)]
ovsdb: Fix typo in SPECS.
Reported-by: Jeremy Stribling <strib@nicira.com>
Casey Barker [Thu, 4 Aug 2011 23:18:59 +0000 (16:18 -0700)]
lib: Adapt headers for use in C++.
This commit makes several library headers suitable for inclusion in C++.
It adds [extern "C"] guards and makes minor changes to fix casting and
keyword issues.
Ethan Jackson [Thu, 4 Aug 2011 20:20:30 +0000 (13:20 -0700)]
util: Fix non-ANSI function declaration.
This patch fixes the following sparse warning:
"non-ANSI function declaration of function 'get_program_version'"
Justin Pettit [Thu, 4 Aug 2011 08:26:54 +0000 (01:26 -0700)]
ovs-appctl: Add common commands to man page.
The common "help" command was not previously mentioned in the ovs-appctl
man page. This commit documents both "help" and the new "version"
commands.
Justin Pettit [Tue, 2 Aug 2011 04:18:00 +0000 (21:18 -0700)]
ovs-appctl: Add "version" command to print version of running daemons.
When debugging a running system, we occasionally see a mismatch of
different versions because someone forgets to restart one or more
daemons. Often times, it would be useful to know what's running as
opposed to what's on the current runpath.
Justin Pettit [Tue, 2 Aug 2011 19:16:44 +0000 (12:16 -0700)]
util: Introduce get_program_version function.
Useful in an upcoming commit.
Simon Horman [Wed, 3 Aug 2011 23:04:10 +0000 (08:04 +0900)]
ovs-dpctl: Show number of flows
Expose the number of flows present in a datapath to user-space
and to users via ovs-dpctl show.
e.g.:
ovs-dpctl show br3
system@br3:
lookups: frags:0, hit:0, missed:0, lost:0
flows: 0
...
Signed-off-by: Simon Horman <horms@verge.net.au>
[Jesse: Add same logic to userspace datapath.]
Signed-off-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Wed, 3 Aug 2011 22:01:11 +0000 (15:01 -0700)]
in-band: Delete remaining rules when disabling in-band control.
in_band_destroy() doesn't remove all of the rules that in-band control
adds (and it cannot, because that might require waiting for an existing
asynchronous flow modification or addition to complete), so turning on
other-config:disable-in-band or deleting all of the OpenFlow controllers
did not delete all of the in-band rules.
This commit fixes the problem by making the in-band control object hang
around until all of the flows that it set up have actually been deleted.
This problem was introduced as part of commit
7ee20df "ofproto: Implement
asynchronous OFPT_FLOW_MOD commands."
Reported-by: Brad Hall <brad@nicira.com>
Ben Pfaff [Wed, 3 Aug 2011 21:31:54 +0000 (14:31 -0700)]
connmgr: Drop 'next_in_band_update' timer.
This timer used to exist because it was possible for in-band remote IP
addresses to change without any intentional configuration change in one
case: where controller discovery found a new controller. Controller
discovery was removed long ago, but the reason for the timer had been
forgotten and so remained. This commit removes it.
Justin Pettit [Wed, 3 Aug 2011 22:10:58 +0000 (15:10 -0700)]
Prepare for a post-1.2.0 world (1.2.90).
Justin Pettit [Wed, 3 Aug 2011 22:09:45 +0000 (15:09 -0700)]
Prepare Open vSwitch 1.2.0 release.
Justin Pettit [Wed, 3 Aug 2011 06:15:30 +0000 (23:15 -0700)]
Update ChangeLog to reflect changes since 1.1.0.
Simon Horman [Wed, 3 Aug 2011 02:19:47 +0000 (11:19 +0900)]
datapath: Allow the number of hash entries to exceed TBL_MAX_BUCKETS
* If the number of entries in a table exceeds
the number of buckets that it has then
an attempt will be made to resize the table.
* There is a limit of TBL_MAX_BUCKETS placed on
the number of buckets of a table.
* If this limit is exceeded keep using the existing table.
This allows a table to hold more than TBL_MAX_BUCKETS
entries at the expense of increased hash collisions.
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Simon Horman [Wed, 3 Aug 2011 02:19:46 +0000 (11:19 +0900)]
datapath: Allow table to expand to have TBL_MAX_BUCKETS buckets
This resolves what appears to be a logic error whereby
the maximum number of buckets is limited to only half of TBL_MAX_BUCKETS.
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Thu, 28 Jul 2011 17:19:43 +0000 (10:19 -0700)]
vlog: Move log level definitions from source code to user documentation.
Users should be able to find out what a log level means without reading
source code comments, so this seems like a better place for it.
Reported-by: David Tsai <dtsai@nicira.com>
Ben Pfaff [Thu, 28 Jul 2011 17:19:42 +0000 (10:19 -0700)]
vlog: Add a new log level "off".
Until now, "emer" has effectively been "off" because no messages were ever
logged at "emer" level. Justin points out that it is useful to use "emer"
for messages that indicate a fatal error. This commit makes that change
and adds a new "off" level to really turn off all logging to a facility.
Justin Pettit [Mon, 1 Aug 2011 17:43:26 +0000 (10:43 -0700)]
debian: Fix path for ovsdb-server in init script.
Reported-by: Brad Hall <brad@nicira.com>
Jesse Gross [Fri, 15 Jul 2011 16:59:06 +0000 (09:59 -0700)]
datapath: Backport flex_arrays.
flex_arrays didn't exist at all until 2.6.30, weren't exported to modules
until 2.6.38, and performed poorly until 3.0, so this backports the
functionality to older kernels.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Fri, 15 Jul 2011 16:59:16 +0000 (09:59 -0700)]
datapath: Don't pass __GFP_ZERO to kmalloc on older kernels.
On new kernels kzalloc() is simply a wrapper around kmalloc with
the addition of the __GFP_ZERO flag. flex_arrays take advantage
of this by expecting the user to just pass in this flag if they
want the memory to be zeroed. However, before 2.6.23, kzalloc()
was a function in its own right and kmalloc really didn't like
receiving __GFP_ZERO. This overrides kmalloc() to intercept the
flags and direct the call to the right function.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Fri, 15 Jul 2011 16:59:05 +0000 (09:59 -0700)]
datapath: Backport reciprocal division.
The reciprocal division library did not exist until 2.6.20 and is
not currently exported in any version, so this backports it.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Thu, 28 Jul 2011 23:49:06 +0000 (16:49 -0700)]
ovs-ofctl: Print human readable nx_pull_match() errors.
I've found this makes debugging a bit easier.
Ethan Jackson [Wed, 27 Jul 2011 22:10:38 +0000 (15:10 -0700)]
nx-match: Shorten line length to 79 characters.
Simon Horman [Thu, 28 Jul 2011 23:38:50 +0000 (16:38 -0700)]
ofproto-dpif: Allow setting of flow eviction threshold
Allow setting the number of flows present in the flow hash
at which point eviction of entries from the kernel flow hash
will begin to occur.
The value may be set using a bridge's other-config column.
e.g.
ovs-vsctl set bridge br3 other-config:flow-eviction-threshold=10000
default is 1000, reflecting constant value previously used.
Increasing this value can result in reduced CPU usage and
packet loss in situations where the number of active flows
is significantly larger than 1000.
pravin shelar [Thu, 28 Jul 2011 16:05:25 +0000 (09:05 -0700)]
Datapath action should not refer to controller
ODP_ACTION_ATTR_CONTROLLER in the kernel actually sends packets to
userspace, not the controller. To make it generic rename this action
to ODP_ACTION_ATTR_USERSPACE.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Ethan Jackson [Thu, 28 Jul 2011 18:39:51 +0000 (11:39 -0700)]
git: Update gitignore in tests and utilities.
Ben Pfaff [Thu, 28 Jul 2011 00:06:50 +0000 (17:06 -0700)]
ofproto-dpif: Fix mirror configuration and deconfiguration.
Deleting or updating a mirror didn't work: mirror_set() didn't set the
'aux' member so mirror_lookup() couldn't find the mirror on that basis.
Bug #6617.
Reported-by: David Tsai <dtsai@nicira.com>
Ben Pfaff [Thu, 28 Jul 2011 16:45:02 +0000 (09:45 -0700)]
vlog: Fix typo in VLOG_IS_ERR_ENABLED macro definition.
This does not fix any real bug because VLOG_IS_ERR_ENABLED is not used
anywhere in the tree.