Ben Pfaff [Thu, 26 Jul 2012 04:37:59 +0000 (21:37 -0700)]
packets: First-hop router redundancy protocol MAC addresses are not BPDUs.
Commit c93f9a78c349 (packets: Update the reserved protocols list.) added
a number of first-hop router redundancy protocol MAC addresses to the
list of BPDU MAC addresses. This means that packets destined to those MAC
addresses are dropped when other-config:forward-bpdu is set to false on a
bridge (the default setting).
However, this behavior is incorrect, because these MAC addresses are not
special in the way that, say, STP frames are special. STP is a
switch-to-switch protocol that end hosts have no use for, but end hosts do
speak directly to routers on the MAC addresses assigned by VRRP and the
other protocols in this category. Therefore, dropping packets in this
category means that end hosts can no longer talk to their first-hop router,
if that router is running one of these protocols.
This commit also refines the match used for EDP and EAPS, and adds Cisco
CFM to the protocols that are dropped.
After this commit, the following destination MACs are dropped:
This patch adds new netdev classes that implement
"system" and "tap" devices on FreeBSD using the
libpcap library. This enables the use of the
"netdev" datapath_type of Open vSwitch on FreeBSD.
Signed-off-by: Gaetano Catalli <gaetano.catalli@gmail.com> Signed-off-by: Ed Maste <emaste@adaranet.com> Signed-off-by: Giuseppe Lettieri <g.lettieri@iet.unipi.it> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 25 Jul 2012 17:28:38 +0000 (10:28 -0700)]
configure: Fix check for GNU make $(if) extension.
As it turns out, the argument to AC_CONFIG_COMMANDS_PRE gets copied into
config.status whether or not it gets run by the shell at "configure" time,
defeating my attempt to support non-GNU make here.
Reported-by: Ed Maste <emaste@freebsd.org> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 23 Jul 2012 16:54:16 +0000 (09:54 -0700)]
Fix race condition in parallel execution of "make install".
ovs-vsctl is listed, incorrectly, in both bin_PROGRAMS and bin_SCRIPTS.
This meant that "make install" with the -j option could try to install
ovs-vsctl two times in parallel, a race that occasionally caused a build
failure, e.g.:
http://buildd.debian.org/status/fetch.php?pkg=openvswitch&arch=s390&ver=1.4.2%2Bgit20120612-5&stamp=1342851603
Ben Pfaff [Mon, 23 Jul 2012 17:16:31 +0000 (10:16 -0700)]
ovs-ofctl: Avoid printing false differences on "ovs-ofctl diff-flows".
It is possible for "struct ofpact"s to differ bytewise even if they are
equivalent when converted to another representation, such as OpenFlow 1.0
action format or a string representation. This can cause "ovs-ofctl
diff-flows" to print surprising false "differences", e.g. as in the bug
report:
- actions=resubmit(,1)
+ actions=resubmit(,1)
This commit fixes the problem by comparing not just the ofpacts but also
the string representation and printing a difference only if both differ.
Bug #8899. Reported-by: Luca Giraudo <lgiraudo@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 20 Jul 2012 20:15:36 +0000 (13:15 -0700)]
debian: Remove controller keys on openvswitch-controller package purge.
A Debian package is expected to remove all its configuration files (which
includes all files in /etc) when it is purged, but the
openvswitch-controller package wasn't doing that. This fixes the problem.
Debian bug #682187. CC: 682187@bugs.debian.org Reported-by: Andreas Beckmann <debian@abeckmann.de> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 20 Jul 2012 17:49:06 +0000 (10:49 -0700)]
tests: Remove bit-rotted support for "lcov".
At one point I got the "lcov" utilities to work well with OVS. Then I
didn't try to use them again for a year or so, and when I did I found that
it didn't work at all. I wasn't able to fix the problem easily, so it
seems better to remove the feature than to leave around broken code.
Ben Pfaff [Wed, 23 May 2012 16:33:22 +0000 (09:33 -0700)]
ofp-print: Print the type of truncated messages, when available.
The function ofputil_decode_msg_type_partial() can figure out the type of
a truncated OpenFlow message, if the message is long enough that its type
can be determined, so we should print this information. This is
especially valuable for printing OFPT_ERROR messages, in which the inner
OpenFlow message is often truncated to 64 bytes.
Ethan Jackson [Fri, 20 Jul 2012 20:07:49 +0000 (13:07 -0700)]
cfm: Clear RDI on new CFM configurations.
When CFM is first configured, it detects no remote endpoints, and
thus sets RDI on its CCMs. This can cause the receiver of these
CCMs to think there is a problem when really things are simply
initializing. This patch fixes the issue by not setting the RDI
bit in CCMs until at least one fault interval has passed.
Bug #12610. Reported-by: Paul Ingram <paul@nicira.com> Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Thu, 19 Jul 2012 03:39:54 +0000 (20:39 -0700)]
cfm: Improve logging.
This patch makes a two improvements to CFM logging which should
make debugging connectivity problems a bit more intuitive. First,
when a remote_mp disappears, the length of time since its last CCM
reception is logged. Second, the "CFM fault status changed"
message is reformatted in a more intuitive way. Instead of
prefixing additions and deletions with pluses and minuses, the full
old fault status and new fault status are logged.
Requested-by: Ben Basler <bbasler@nicira.com>, Signed-off-by: Ethan Jackson <ethan@nicira.com>
ovs-bugtool: Added --ovs option to get only ovs related information
Option --ovs is added for ovs-bugtool command to collect
only OpenvSwitch relevant information. To perform
filtering in plugins, a new xml attribute filters="ovs" (optional)
would be required in element 'command','files','directory' in
openvswitch.xml. Value of 'filters' attribute will be compared
with filtering option in load_plugins to get all relevant operation
to collect information. If no "--ovs" option is passed then it will
behave as earlier.
Fixed an issue which occurs in scenario where option '--yestoall'
is not passed and user keeps entering "y" or "n" on prompt.
Plus, trailing whitespaces are fixed. White space before '=' and
after in function def and call is also fixed.
Signed-off-by: Arun Sharma <arun.sharma@calsoftinc.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Simon Horman [Thu, 19 Jul 2012 16:21:49 +0000 (09:21 -0700)]
OXM: Allow masking of ARP SHA and THA
Signed-off-by: Simon Horman <horms@verge.net.au>
[blp@nicira.com added NEWS, updated a few overlooked meta-flow bits] Signed-off-by: Ben Pfaff <blp@nicira.com>
Simon Horman [Wed, 18 Jul 2012 03:02:20 +0000 (12:02 +0900)]
classifier: Add helpers for setting ethernet addresses
Add helpers for setting ethernet addresses.
This patch makes use of them for setting the dl_src and dl_dst
addresses. A subsequent patch will also use them for arp_sha and arp_tpa.
Signed-off-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 19 Jul 2012 15:42:21 +0000 (08:42 -0700)]
tests: Avoid hash order sensitivity in "ofproto - flow monitoring" test.
The order in which flows appear in an NXST_FLOW_MONITOR reply depends on
the hash order, which makes it depend on the details of the hash and on
system endianness. This avoids sensitivity to the order by sorting the
results.
Reported-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 19 Jul 2012 07:15:19 +0000 (00:15 -0700)]
nx-match: Succeed pulling 0-byte nx-match from NULL buffer.
I don't think this corner case can come up in a real OpenFlow message,
because the presence of the OpenFlow header guarantees that the ofpbuf's
data is nonnull, but it did in a simple test that is coming up in a few
commits.
Ethan Jackson [Wed, 18 Jul 2012 17:56:21 +0000 (10:56 -0700)]
bridge: Segfault when missing Open vSwitch table.
The enable_system_stats() function calls smap_get_bool() on the
ovsrec_open_vswitch passed to it. This was segmentation faulting
when 'null_cfg' defined in bridge_reconfigure() was used because
there was no Open vSwitch table.
Ben Pfaff [Wed, 27 Jun 2012 17:42:34 +0000 (10:42 -0700)]
vlog: Use worker process to write to log file.
Writes to regular files under Unix-like kernels, including Linux, typically
block until the write is complete, regardless of O_NONBLOCK. When the I/O
subsystem is busy, this can cause indefinite delays. We have actually
observed "write" calls sleep for 5 seconds or more for this reason.
Delegating to a subprocess through the worker mechanism should solve the
problem.
Ben Pfaff [Wed, 27 Jun 2012 17:40:50 +0000 (10:40 -0700)]
worker: New library for breaking a daemon into multiple processes.
ovs-vswitchd is effectively a "soft real-time" process, because flows that
do not get set up quickly lead to packet loss or retransmission. We've
done our best to keep it from blocking unnecessarily, but some operations
unavoidably block. This new library allows a daemon to break itself up
into a main process and a worker process, connected by an RPC channel,
with the idea being that the main process will delegate any possibly
blocking operations to the worker.
This commit also modifies ovs-vswitchd to start a worker process, but it
does not actually introduce any uses for the worker process. Upcoming
commits will add those.
Ben Pfaff [Tue, 22 May 2012 18:36:50 +0000 (11:36 -0700)]
vlog: Add VLOG_ABORT() to log and call abort().
Whereas VLOG_FATAL() eventually calls exit(1), VLOG_ABORT()
eventually calls abort(). The key difference is that abort()
will cause a "monitor" process to restart, where exit(1) will
cause it to exit along with the monitored process.
Ben Pfaff [Wed, 18 Jul 2012 17:30:47 +0000 (10:30 -0700)]
util: Introduce "subprogram_name" to identify subprocesses and threads.
This will be more useful later when we introduces "worker" subprocesses.
I don't have any current plans to introduce threading, but I can't
think of a disadvantage to wording this in a general manner.
Ben Pfaff [Fri, 13 Jul 2012 06:08:45 +0000 (23:08 -0700)]
debian: Do not change iptables rules by default.
Debian kernel maintainer Bastian Blank writes, at
http://bugs.debian.org/680537:
The netfilter rules are a shared resource. There is no synchronization,
so the admin have the last word. As kernel maintainer, I see it similar
to a configuration file, so §10.7 policy applies.
The purpose of openvswitch is to provide support for switching, not to
setup filter rules. This means it violates the principle of least
surprise.
I believe that the argument by analogy to configuration files is weak,
given that the Debian policy section in question is very specifically about
files, not about general principles. On the other hand, Debian does not
install any firewall by default, so the presence of a rule that blocks GRE
traffic is a sign that the administrator has taken an explicit action to
install a firewall that blocks GRE, and therefore it is rather rude to
override this. Therefore, this patch simply turns off this behavior on
Debian, given that in ordinary Debian installations it will have no
adverse effect on Open vSwitch.
Debian bug #680537. CC: 680537@bugs.debian.org Reported-by: Bastian Blank <waldi@debian.org> Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Simon Horman <horms@verge.net.au>
Ben Pfaff [Wed, 18 Jul 2012 17:10:20 +0000 (10:10 -0700)]
ofproto-dpif: Make OFPP_TABLE always resubmit to table 0.
Commit 299016266ed1 (New action NXAST_RESUBMIT_TABLE.) changed OFPP_TABLE
from resubmitting to table 0 to resubmitting to the current table. This
wasn't mentioned in the change log and I believe it was a typo. This
commit changes the behavior back.
This isn't a very serious bug because OpenFlow 1.0 says that OFPP_TABLE is
supposed to be used only in packet-out messages, in which case the current
table is 0 anyhow.
OpenFlow 1.3 is explicit:
The action list of an OFPT_PACKET_OUT message can also specify the
OFPP_TABLE reserved port as an output action to process the packet
through the existing flow entries, starting at the first flow table.
Reported-by: Isaku Yamahata <yamahata@valinux.co.jp> Signed-off-by: Ben Pfaff <blp@nicira.com>
Simon Horman [Wed, 18 Jul 2012 01:47:56 +0000 (10:47 +0900)]
ofproto: More vlan tests
I'm not sure if this is the best place for this, but exercising
adding flows with dl_vlan and dl_vlan_pcp in this matter helped
my to find some bugs in changes that I am working on in relation
to the OpenFlow VLAN match.
Signed-off-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 10 Jul 2012 06:45:25 +0000 (23:45 -0700)]
ofp-util: Wildcard VLAN PCP in OF1.0 matches when 802.1Q not present.
When an output OF1.0 match uses OFP_VLAN_NONE to match only when the 802.1Q
header is not present, it is somewhat contradictory to specify any value
for the VLAN PCP, since none can be present without an 802.1Q header, but
the match output by Open vSwitch did so. This fixes it.
Signed-off-by: Ben Pfaff <blp@nicira.com> Tested-by: Simon Horman <horms@verge.net.au>
Ethan Jackson [Tue, 17 Jul 2012 17:07:36 +0000 (10:07 -0700)]
tests: Fix unit test failures related to additional logging.
Commit a890678229 (userspace: Log version on startup.) added
additional logging to ovsdb-server and ovs-vswitchd, but failed to
make certain the unit tests still passed.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
[blp@nicira.com changed the strategy for fixing ovsdb-server.at] Signed-off-by: Ben Pfaff <blp@nicira.com>
Simon Horman [Tue, 17 Jul 2012 08:27:06 +0000 (17:27 +0900)]
OXM, NXM: Add tests for all zero and all one masks
The motivation for this is that I was looking into some
bugs relating to masking of match code that I was developing
and it seemed that having these tests would be worthwhile.
Signed-off-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 16 Jul 2012 17:23:58 +0000 (10:23 -0700)]
ovs-ofctl: Fix use-after-free error.
Commit 4ce9c31573 (ovs-ofctl: Factor code out of read_flows_from_switch().)
introduced a use-after-free error, fixed by this change.
Also adds a unit test for "ovs-ofctl diff-flows" that would have found the
problem. (The bug report cited "diff-flows" but this bug was present in
dump-flows as well because they share common code.)
Bug #12461. Reported-by: James Schmidt <jschmidt@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 12 Jul 2012 23:32:56 +0000 (16:32 -0700)]
ovs-ofctl: Avoid use-after-free upon "ofctl/unblock" when connection dies.
The implementation of "ofctl/block" used a nested poll loop, with an inner
call to unixctl_server_run(). This poll loop always ran inside an outer
call to unixctl_server_run(), since that's the context within which unixctl
command implementations run. That means that, if a unixctl connection got
closed within the inner poll loop, and the outer poll loop happened to be
processing the same unixctl connection, then the outer poll loop would
dereference data in the freed connection.
The simplest solution is to avoid a nested poll loop, so that's what this
commit does.
This didn't cause a failure in the unit tests on i386 (which is why I
didn't catch it before pushing) but it did, reliably, on x86-64, and it
showed up in valgrind everywhere.
Recently released CentOS 6.3 (and probably also RHEL 6.3, I assume)
backported skb_frag_page() and others to their 2.6.32-based kernel,
which caused build failure of Open vSwitch kernel modules.
Ben Pfaff [Thu, 12 Jul 2012 21:18:05 +0000 (14:18 -0700)]
ofproto: New feature to notify controllers of flow table changes.
OpenFlow switching monitoring and controller coordination can be made more
efficient if the switch can notify a controller of flow table changes as
they occur, rather than periodically polling for changes. This commit
implements such a feature.
Feature #6633. CC: Natasha Gude <natasha@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 3 Jul 2012 21:00:38 +0000 (14:00 -0700)]
ofproto: Add extra comments and checking for expiring a pending rule.
A given rule may only have one pending operation at a time, so when an
operation is pending we must not allow a flow expiration to be started on
that rule.
This doesn't fix a user-visible bug in ofproto-dpif because ofproto-dpif
always completes operations immediately, that is, no operations will be
pending when expiration runs. (Technically there is a bug if the user
runs "ovs-appctl ofproto/clog", but that feature is for debugging only and
there is no reason for a user to ever run it.)
Ben Pfaff [Thu, 12 Jul 2012 17:17:10 +0000 (10:17 -0700)]
ofproto: Represent flow cookie changes as operations too.
An upcoming commit will add support for monitoring changes to the flow
table. This feature wants to be able to report changes to flow cookies,
as well as to other properties of a flow. Until now, however, a flow_mod
that modifies only the flow's cookie is treated as a special case that does
not go through the ofoperation mechanism. That makes it harder to report
flow cookie-only changes (it would require an additional special case in
the reporting mechanism) so this commit changes cookie-only changes to
go through ofoperations.
The bulk of this change is to change the meaning of ofoperation's 'ofpacts'
member so that a NULL value indicates that the flow's actions are not
changing. Otherwise a flow-cookie only change would still require copying
and then freeing all the actions, which seems like a waste.
Ben Pfaff [Fri, 6 Jul 2012 17:36:00 +0000 (10:36 -0700)]
ofproto: Revert change in flow cookie when an ofoperation fails.
The flow_cookie member of struct ofoperation has always been there, but it
seems that it's never been used. This fixes the code so that if a modify
operation fails the rule's original flow cookie is restored.
Ben Pfaff [Sat, 30 Jun 2012 05:33:56 +0000 (22:33 -0700)]
ofproto: Finalize all ofoperations in a given ofgroup at the same time.
An upcoming commit will add support for flow table monitoring by
controllers. One feature of this upcoming support is that a controller's
own changes to the flow table can be abbreviated to a summary, since the
controller presumably knows what it has already sent to the switch.
However, the summary only makes sense if a set of flow table changes
completely succeeds or completely fails. If it partially fails, the
switch must not attempt to summarize it, because the controller needs
to know the details. Given that, we have to wait for all of the
operations in an ofgroup to either succeed or fail before the switch
can send its flow table update report to the controllers. This
commit makes that change.
Ben Pfaff [Thu, 12 Jul 2012 20:32:47 +0000 (13:32 -0700)]
ovs-ofctl: Add --sort and --rsort options for "dump-flows" command.
Feature #8754. Signed-off-by: Arun Sharma <arun.sharma@calsoftinc.com>
[blp@nicira.com rewrote most of the code] Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 3 Jul 2012 17:25:35 +0000 (10:25 -0700)]
ovs-ofctl: Use the prepared connection to dump flows in do_dump_flows__().
The logic in do_dump_flows__() went to some trouble to open an OpenFlow
connection and set the correct protocol, but then it allowed
dump_stats_transaction() to create and use a completely different OpenFlow
connection that hadn't been prepared that way. This commit fixes the
problem.
I don't think that there is a real bug here because currently the set of
protocols doesn't influence flow stats replies. But that could change in
the future.
Syscall param socketcall.sendmsg(msg.msg_iov[i]) points to uninitialised
byte(s)
at 0x42D3021: sendmsg (in /lib/libc-2.5.so)
by 0x80E4D23: nl_sock_transact (netlink-socket.c:670)
by 0x80D9086: dpif_linux_execute__ (dpif-linux.c:872)
by 0x807D6AE: dpif_execute__ (dpif.c:957)
by 0x807D6FE: dpif_execute (dpif.c:987)
by 0x805DED9: send_packet (ofproto-dpif.c:4727)
by 0x805F8E1: port_run_fast (ofproto-dpif.c:2441)
by 0x8065CF6: run_fast (ofproto-dpif.c:926)
by 0x805674F: ofproto_run_fast (ofproto.c:1148)
by 0x804C957: bridge_run_fast (bridge.c:1980)
by 0x8053F49: main (ovs-vswitchd.c:123)
Address 0xbea0895c is on thread 1's stack
Bug #11797. Signed-off-by: Ben Pfaff <blp@nicira.com>