Jesse Gross [Wed, 22 Aug 2012 00:48:54 +0000 (17:48 -0700)]
datapath: Fix namespace refcount leak on failed init.
If a datapath fails to initialze fully (likely due to out-of-memory)
then it's possible that we can take a reference to a network
namespace but never release it. This fixes the problem by releasing
any resources in the event of an error.
Found by code inspection, it's likely to be extremely rare in practice.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
Ansis Atteka [Tue, 21 Aug 2012 23:48:36 +0000 (16:48 -0700)]
stp: port_no counter is off by one
This counter was off by one, because port_num
should be less than STP_MAX_PORTS.
This caused an assert hit later in stp_get_port().
Issue: 13059
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Reported-by: Ram Jothikumar <rjothikumar@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Tue, 21 Aug 2012 20:51:01 +0000 (13:51 -0700)]
bond: Tag flows according to their hash bucket, not just their slave.
The bonding code is supposed to tag flows two ways:
- According to the chosen bond slave, to make it easy to invalidate all
of the flows assigned to a given slave.
- According to the hash value for a flow, to make it easy to invalidate
all of the flows that hash into the same bucket.
However, the code wasn't actually applying the hash-based tags. This
meant that rebalancing didn't take effect immediately, and so after
rebalancing we could get log messages like this:
inconsistency in subfacet (actions were: 5) (correct actions: 4)
specifying some flow that was moved by the rebalance.
This commit fixes the problem by applying the hash-based tags.
Bug #12847.
Reported-by: Pratap Reddy <preddy@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Ed Maste [Wed, 15 Aug 2012 22:00:34 +0000 (22:00 +0000)]
netdev: Map to OpenFlow port for flow lookup
In the flow hash special ports are stored using OpenFlow constants.
For example the "local port" is stored as 0xfffe (OFPP_LOCAL).
Signed-off-by: Ed Maste <emaste@freebsd.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Justin Pettit [Mon, 13 Aug 2012 22:35:55 +0000 (15:35 -0700)]
Prepare for 1.7.1.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Wed, 27 Jun 2012 16:56:20 +0000 (09:56 -0700)]
tests: Fix MockXenAPI to make the ovs-xapi-sync test case pass again.
Commit
a8d9dfc788 (xenserver: Improve efficiency of code by using
get_all_records_where()) updated the ovs-xapi-sync script and caused a unit
test failure. This fixes it.
Bug #12848.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Rob Hoes [Wed, 27 Jun 2012 15:14:21 +0000 (16:14 +0100)]
xenserver: Improve efficiency of code by using get_all_records_where()
Replace the get_record() for network references which caused as many
slave-to-master calls as there are Network records plus one.
The get_all_records_where() call gets exactly what is needed with a single
call.
Bug #12848.
Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
Acked-by: Dominic Curran <dominic.curran@citrix.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 6 Aug 2012 22:03:32 +0000 (15:03 -0700)]
rconn: Treat draining a message from the send queue as activity.
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>
Ben Pfaff [Mon, 6 Aug 2012 20:31:53 +0000 (13:31 -0700)]
rconn: Remove unused functions.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Fri, 3 Aug 2012 01:22:38 +0000 (18:22 -0700)]
datapath: Relax set header validation.
When installing a flow with an action to set a particular field we
need to validate that the packets that are part of the flow actually
contain that header. With IP we use zeroed addresses and with TCP/UDP
the check is for zeroed ports. This check is overly broad and can catch
packets like DHCP requests that have a zero source address in a
legitimate header. This changes the check to look for a zeroed protocol
number for IP or for both ports be zero for TCP/UDP before considering
the header to not exist.
Bug #12769
Reported-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
Ben Pfaff [Mon, 18 Jun 2012 21:02:12 +0000 (14:02 -0700)]
flow: Correctly consider nw_frag_mask in some flow_wildcards_*() functions.
This probably means that some classifier functions based on the fragment
type of packets have never worked properly.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Wed, 1 Aug 2012 20:01:01 +0000 (13:01 -0700)]
flow: Fix wild pointer dereference in flow_compose().
The 'ip' variable in flow_compose() points to some memory allocated
in an ofpbuf. The ofpbuf is modified without making the necessary
updates to the location of 'ip' causing a potential wild memory
access.
Found by inspection.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Justin Pettit [Tue, 31 Jul 2012 00:24:39 +0000 (17:24 -0700)]
Set dates for release of 1.7.0.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ed Maste [Fri, 27 Jul 2012 21:27:15 +0000 (17:27 -0400)]
Avoid implementation-defined strerror behaviour
POSIX states that the string returned by strerror() may be overwritten
by a subsequent call (i.e., because it returns a pointer to a static
buffer). Make a copy of one of the two strerror() strings to avoid
this.
Background: FreeBSD historically returned such a pointer only in the
case of an invalid errno. With the addition of NLS strerror was changed
to do so for all calls.
Prior to this change I had confusing results from the test suite like
"... is 22 (Invalid argument) but should be 0 (Invalid argument)".
Signed-off-by: Ed Maste <emaste@adaranet.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 27 Jul 2012 20:11:20 +0000 (13:11 -0700)]
ovs-ctl: Start the rest of Open vSwitch if loading brcompat module fails.
This may be more useful in practice than failing the entire OVS startup
sequence.
Debian bug #681955.
CC: 681955@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>
Justin Pettit [Mon, 23 Jul 2012 07:33:32 +0000 (00:33 -0700)]
NEWS: Document flow setup improvement in 1.7 release.
Also use consistent capitalization for "DSCP".
Signed-off-by: Justin Pettit <jpettit@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
Debian bug #682384.
CC: 682384@bugs.debian.org
Reported-by: Bastian Blank <waldi@debian.org>
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, 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 [Tue, 10 Jul 2012 21:51:29 +0000 (14:51 -0700)]
cfm: Always initialize CCM "internal_ms_x" extension field.
Found by valgrind:
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>
Ben Pfaff [Tue, 10 Jul 2012 21:11:59 +0000 (14:11 -0700)]
datapath: Check gso_type for correct sk_buff in queue_gso_packets().
At the point where it was used, skb_shinfo(skb)->gso_type referred to a
post-GSO sk_buff. Thus, it would always be 0. We want to know the pre-GSO
gso_type, so we need to obtain it before segmenting.
Before this change, the kernel would pass inconsistent data to userspace:
packets for UDP fragments with nonzero offset would be passed along with
flow keys that indicate a zero offset (that is, the flow key for "later"
fragments claimed to be "first" fragments). This inconsistency tended
to confuse Open vSwitch userspace, causing it to log messages about
"failed to flow_del" the flows with "later" fragments.
Bug #12394.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Fri, 6 Jul 2012 22:13:25 +0000 (15:13 -0700)]
debian: Remove obsolete advice to edit /etc/default/openvswitch-switch.
This hasn't been necessary for a long time.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 6 Jul 2012 22:08:10 +0000 (15:08 -0700)]
debian: Add "netbase" dependency for /etc/protocols.
ovs-ctl.in uses /etc/protocols, which is in the "netbase" package, so a
dependency is required.
Debian bug #680537.
CC: 680537@bugs.debian.org
Reported-by: Bastian Blank <waldi@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 5 Jul 2012 19:57:16 +0000 (12:57 -0700)]
ovs-check-dead-ifs: Tolerate processes that disappear during run.
os.listdir("/proc/%d/fd" % pid) throws OSError if 'pid' died since the
list of pids was obtained.
Bug #12375.
Reported-by: Amey Bhide <abhide@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 5 Jul 2012 15:41:03 +0000 (08:41 -0700)]
ovs-brcompatd: Fix sending replies to kernel requests.
Commit 7d7447 (netlink: Postpone choosing sequence numbers until send
time.) broke ovs-brcompatd because it prevented userspace replies to
kernel requests from using the correct sequence numbers. This commit fixes
it.
Atzm Watanabe found the root cause and provided an alternative patch to
avoid the problem.
Reported-by: André Ruß <andre.russ@hybris.com>
Reported-by: Atzm Watanabe <atzm@stratosphere.co.jp>
Tested-by: Atzm Watanabe <atzm@stratosphere.co.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 2 Jul 2012 17:34:32 +0000 (10:34 -0700)]
python: Call 'wait' methods correctly in jsonrpc and stream code.
Bug #12301.
Reported-by: Mike Kruze <mkruze@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 29 Jun 2012 16:22:59 +0000 (09:22 -0700)]
ovs-vswitchd: Call mlockall() from the daemon, not the parent or monitor.
mlockall(2) says:
Memory locks are not inherited by a child created via fork(2) and are
automatically removed (unlocked) during an execve(2) or when the
process terminates.
which means that --mlockall was ineffective in combination with --detach
or --monitor or both. Both are used in the most common production
configuration of Open vSwitch, so this means that --mlockall has never been
effective in production.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Justin Pettit [Tue, 26 Jun 2012 04:44:56 +0000 (21:44 -0700)]
Update dates and notes for 1.6.1 release.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ed Maste [Tue, 26 Jun 2012 14:43:54 +0000 (14:43 +0000)]
lib: Do not assume sig_atomic_t is int.
On FreeBSD sig_atomic_t is long, which causes the comparison in
fatal_signal_run to be true when no signal has been reported.
Signed-off-by: Ed Maste <emaste@freebsd.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 25 Jun 2012 16:48:44 +0000 (09:48 -0700)]
ofproto-dpif-governor: Improve performance when most flows get set up.
The "flow setup governor" was introduced to avoid the cost of setting up
short flows when there are many of them. It works very well for short
flows, in fact. However, when the bulk of flows are short, but still long
enough to be set up by the governor, we end up with the worst of both
worlds: OVS processes the first 5 packets of every flow "by hand" and then
it still has to set up a flow.
This commit refines the flow setup governor so that, when most of the flows
that go through it actually get set up, it in turn starts setting up most
flows at the first packet. When it does this, it continues to sample a
small fraction of the flows in the governor's usual manner, so that if the
behavior changes it can react to it.
This increases netperf TCP_CRR transactions per second by about 25% in my
test setup, without affecting "ovs-benchmark rate" performance.
(I found that to get relatively stable performance for TCP_CRR, regardless
of whether Open vSwitch or any kind of bridging was involved, I had to pin
the netperf processes on each side of the link to a single core. I found
that my NIC's interrupts were already pinned. Thanks to Luca Giraudo
<lgiraudo@nicira.com> for these hints.)
Bug #12080.
Reported-by: Gurucharan Shetty <gshetty@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Sun, 24 Jun 2012 05:34:39 +0000 (22:34 -0700)]
ofp-util: Avoid use-after-free in ofputil_encode_flow_mod().
nx_put_match() can reallocate the ofpbuf's data so we need to reload the
pointer.
Found by inspection.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 20 Jun 2012 17:55:41 +0000 (10:55 -0700)]
dpif-linux: Zero 'stats' outputs of dpif_operate() ops on failure.
When DPIF_OP_FLOW_PUT or DPIF_OP_FLOW_DEL operations failed, they left
their 'stats' outputs uninitialized. For DPIF_OP_FLOW_DEL, this meant that
the caller would read indeterminate data:
Conditional jump or move depends on uninitialised value(s)
at 0x805C1EB: subfacet_reset_dp_stats (ofproto-dpif.c:4410)
by 0x80637D2: expire_batch (ofproto-dpif.c:3471)
by 0x8066114: run (ofproto-dpif.c:3513)
by 0x8059DF4: ofproto_run (ofproto.c:1035)
by 0x8052E17: bridge_run (bridge.c:2005)
by 0x8053F74: main (ovs-vswitchd.c:108)
It's unusual for a delete operation to fail. The most common reason is an
administrator running "ovs-dpctl del-flows".
The only user of DPIF_OP_FLOW_PUT did not request stats, so this doesn't
fix an actual bug for that case.
Bug #11797.
Reported-by: James Schmidt <jschmidt@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 20 Jun 2012 20:18:25 +0000 (13:18 -0700)]
ofproto-dpif-governor: Wake up only when there is genuinely work to do.
Until now, governor_wait() has awakened the poll loop whenever the
generation timer expires, to allow it to shrink the governor to the next
smaller size in governor_run(). However, if the governor is already the
smallest possible size, then governor_run() will not have anything to do
and will not restart the timer, which means that governor_wait() will again
immediately wake up the poll loop, and we end up using 100% CPU.
This is kind of hard to trigger because normally the client will destroy
a governor in such a case. However, if there are too many subfacets, the
client will keep even a minimum-size governor, triggering the bug.
Bug #12106.
Reported-by: Alex Yip <alex@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 18 Jun 2012 16:33:23 +0000 (09:33 -0700)]
debian: Make DKMS automatically build for running kernel.
By default DKMS doesn't build on demand for each kernel booted or updated.
Adding AUTOINSTALL=yes gives it this behavior. Based on a small sample of
Debian packages and how-to guides for Ubuntu, AUTOINSTALL=yes is what most
packages use and what users expect.
Fix-suggested-by: Kirill Kabardin
Reported-by: Ralf Heiringhoff <ralf@frosty-geek.net>
Reported-at: https://bugs.launchpad.net/bugs/962189
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 12 Jun 2012 23:45:20 +0000 (16:45 -0700)]
vlog: Avoid use-after-free in corner case.
Found by valgrind.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Pravin B Shelar [Tue, 12 Jun 2012 18:19:16 +0000 (11:19 -0700)]
datapath: Check currect return value from skb_gso_segment()
Fix return check typo.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #11933
Ben Pfaff [Sat, 9 Jun 2012 15:55:29 +0000 (11:55 -0400)]
dpif-netdev: Fix use-after-free in dpif_netdev_recv.
Found by valgrind.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Pravin B Shelar [Thu, 7 Jun 2012 22:18:17 +0000 (15:18 -0700)]
datapath: Fix use-after-free bug in dp_notify.
dp_notify, in unregister case, is accessing vport after detaching
it. Following patch fixes it.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Ethan Jackson [Thu, 7 Jun 2012 20:05:41 +0000 (13:05 -0700)]
ofproto: Fix use after free in ofoperation_complete().
In one edge case, ofoperation_complete() destroys its rule, without
updating its ofoperation that the rule is gone. Later in the same
function, ofoperation_destroy() attempts to modify the rule which
already destroyed.
Bug #11797.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Justin Pettit [Fri, 1 Jun 2012 20:21:50 +0000 (13:21 -0700)]
Set dates for release of 1.5.0.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Fri, 1 Jun 2012 20:19:40 +0000 (13:19 -0700)]
NEWS: Move learning timeout changes to 1.5.0 section.
Changes to the default MAC learning timeout and making it configurable
were added in 1.5.0, not 1.6.0.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Sat, 24 Mar 2012 08:02:26 +0000 (01:02 -0700)]
ofp-util: Clean up cookie handling.
Commit
e72e793 (Add ability to restrict flow mods and flow stats
requests to cookies.) modified cookie handling. Some of its behavior
was unintuitive and there was at least one bug (described below).
Commit
f66b87d (DESIGN: Document uses for flow cookies.) attempted to
document a clean design for cookie handling. This commit updates the
DESIGN document and brings the implementation in line with it.
In commit
e72e793, the code that handled processing OpenFlow flow
modification requests set the cookie mask to exact-match. This seems
reasonable for adding flows, but is not correct for matching, since
OpenFlow 1.0 doesn't support matching based on the cookie. This commit
changes to cookie mask to fully wildcarded, which is the correct
behavior for modifications and deletions. It doesn't cause any problems
for flow additions, since the mask is ignored for that operation.
Bug #9742
Reported-by: Luca Giraudo <lgiraudo@nicira.com>
Reported-by: Paul Ingram <paul@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Pravin B Shelar [Thu, 24 May 2012 00:25:41 +0000 (17:25 -0700)]
datapath: Avoid system freeze due to ovs-flow-rehash softlockup.
OVS datapath does periodic flow table rehash which takes genl_lock
in workq context.
In some cases, like ports add or delete, genl_lock can cause softlockup
as vswitchd would take and succeed with genl_lock and rehash workq
would block on the lock. Eventually rehash will proceed, flow rehash
is low priority task so this is not problem for rehashing.
But it is blocking workq thread; some other workq item from other
kernel subsystem would be blocked and can cause system freeze.
To avoid workq blocking and system freeze, we can use OVS compat workq.
It runs in separate kernel thread thus does not block any non-ovs
deferred workq work item.
We will fix it by making genetlink lockless and having fine granular
locking in OVS.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Ethan Jackson [Mon, 21 May 2012 20:20:18 +0000 (13:20 -0700)]
bridge: Ignore "null" interfaces as required.
Commit
bae7208e91a0 (bridge: Refactor bridge_reconfigure().)
introduced a regression in which the switch would attempt to
instantiate "null" interfaces in the datapath. This would, of
course, fail and trigger a warning. Though harmless, these
warnings confused users.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Tue, 22 May 2012 18:39:25 +0000 (11:39 -0700)]
ofproto-dpif-governor: Fix bug in log message.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 22 May 2012 17:12:49 +0000 (10:12 -0700)]
Fix typo in "PYTHONPATH".
Reported-by: Justin Pettit <jpettit@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Jesse Gross [Thu, 17 May 2012 18:43:15 +0000 (11:43 -0700)]
datapath: Reset upper layer protocol info on internal devices.
It's possible that packets that are sent on internal devices (from
the OVS perspective) have already traversed the local IP stack.
After they go through the internal device, they will again travel
through the IP stack which may get confused by the presence of
existing information in the skb. The problem can be observed
when switching between namespaces. This clears out that information
to avoid problems but deliberately leaves other metadata alone.
This is to provide maximum flexibility in chaining together OVS
and other Linux components.
Bug #10995
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Arun Sharma [Wed, 9 May 2012 23:34:21 +0000 (16:34 -0700)]
bugtool: rename label names in plugins related to ovs-appctl
It improves to have proper out file name in bugtool archive with respect
to ovs-appctl commands. E.g. if command is 'ovs-appctl lacp/show' then
the related out file will be 'ovs-appctl-lacp-show.out'
Feature #11283.
Signed-off-by: Arun Sharma <arun.sharma@calsoftinc.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Arun Sharma [Wed, 9 May 2012 06:47:30 +0000 (23:47 -0700)]
bugtool: Collect bond state information from ovs.
This is an enhancement in bugtool archive output to determine the bond
state information. It is implemented as a plugin which internally calls
"ovs-appctl bond/show" command to get bond state.
Feature #11283.
Signed-off-by: Arun Sharma <arun.sharma@calsoftinc.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 15 May 2012 19:50:57 +0000 (12:50 -0700)]
odp-util: Update ODPUTIL_FLOW_KEY_BYTES for current kernel flow format.
Before we submitted the kernel module upstream, we updated the flow format
by adding two fields to the description of packets with VLAN headers, but
we forgot to update ODPUTIL_FLOW_KEY_BYTES to reflect these changes. The
result was that a maximum-length flow did not fit in the given space.
This fixes a crash processing IPv6 neighbor discovery packets with VLAN
headers received in a tunnel configured with key=flow or in_key=flow.
This updates some comments to better describe the implications of
ODPUTIL_FLOW_KEY_BYTES (suggested by Justin).
This also updates test-odp.c so that it would have caught this problem, and
updates odp.at to demonstrate that a full 156 bytes are necessary. (To see
that, revert the change to ODPUTIL_FLOW_KEY_BYTES and run the test.)
Reported-by: Dan Wendlandt <dan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Giuseppe Lettieri [Wed, 9 May 2012 10:17:15 +0000 (12:17 +0200)]
dpif-netdev: allow for proper destruction of netdev datapaths
Until now, bridges with datapath_type=netdev did not destroy the datapath
when deleted. In particular, the tap device implementing the internal
interface was not close()d, and therefore the tap persists until
ovs-vswitchd exit()s.
This behaviour was caused by the missing callback for 'enumerate' in the
dpif-netdev class. Without this callback 'bridge_reconfigure' failed to
realize that there are datapaths with no bridge, and thus cannot destroy
them. Providing an 'enumerate' callback fixes this.
Signed-off-by: Giuseppe Lettieri <g.lettieri@iet.unipi.it>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Thu, 10 May 2012 18:24:36 +0000 (11:24 -0700)]
ofputil: Accept OFPP_NONE as a valid output port.
Ofproto-dpif treats an output to OFPP_NONE as a NOOP, but ofputil
treated it as an error. The former behavior seems more natural.
Reported-by: Teemu Koponen <koponen@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Tue, 8 May 2012 23:54:21 +0000 (16:54 -0700)]
fatal-signal: Log when terminating due to a fatal signal.
This makes it easier to diagnose why and when a daemon exited.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 2 May 2012 21:51:51 +0000 (14:51 -0700)]
ofp-util: Match on NXAST_CONTROLLER as an output to OFPP_CONTROLLER.
Found by inspection.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Sun, 13 May 2012 23:31:15 +0000 (16:31 -0700)]
cfm: Avoid null pointer dereference in cfm_process_heartbeat().
Found by clang.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Gurucharan Shetty [Fri, 11 May 2012 21:49:02 +0000 (14:49 -0700)]
ovs-bugtool: Close file descriptors after use.
In ovs-bugtool, we do a bunch of Popen calls to
get the results of some shell commands with stdout
set to PIPE. Once we are done, we need to
close the file descriptors.
Bug #11083.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Ben Pfaff [Wed, 9 May 2012 19:21:54 +0000 (12:21 -0700)]
odp-util: Fix parsing of actions encapsulated within "sample" actions.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 10 May 2012 16:37:11 +0000 (09:37 -0700)]
Fix "make distcheck" failure since we weren't cleaning up a generated file.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Sat, 5 May 2012 18:07:42 +0000 (11:07 -0700)]
ofproto-dpif: Segregate CFM, LACP, and STP traffic into separate queues.
Until now, packets for these special protocols have been mixed with general
traffic in the kernel-to-userspace queues. This means that a big-enough
storm of new flows in these queues can cause packets for these special
protocols to be dropped at this interface, fooling userspace into believing
that, say, no CFM packets have been received even though they are arriving
at the expected rate.
This commit moves special protocols to a dedicated kernel-to-userspace
queue to avoid the problem.
Bug #7550.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 4 May 2012 21:52:36 +0000 (14:52 -0700)]
ofproto-dpif: Introduce "slow path" datapath flows.
Most exact-match flows can be handled directly in the datapath, but
for various reasons, some cannot: every packet in these flows must
be sent separately to userspace. Until now, flows that cannot be
handled entirely in the kernel have been allowed to miss each time
in the datapath. This is generally OK, but it has a few
disadvantages:
* It can make troubleshooting at the level where one must look
at datapath flows a bit confusing in some cases, because
datapath misses due to genuinely new flows are mixed in with
datapath misses for known flows that cannot be set up.
* It means that the kernel-to-userspace packets for a given
input port always go to a single kernel-to-userspace queue,
even if we'd like to segregate out some of the packets for
known flows. (An upcoming commit has examples.)
This commit therefore introduces the concept of a "slow path" flow,
one that is installed in the datapath with a single action that
sends the flow's packets to userspace. To make troubleshooting
easier, the action includes a reason code (displayed by "ovs-dpctl
dump-flows") that explains why the flow has been slow-pathed.
Bug #7550.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Sat, 5 May 2012 17:55:30 +0000 (10:55 -0700)]
ofproto-dpif: Introduce "internal flows" for handling flow table misses.
The ofproto-dpif implementation of "facet"s requires a facet to be
associated with an OpenFlow rule. Until now, this meant that packets
that miss in the OpenFlow table (and thus didn't have OpenFlow rules)
couldn't be set up as facets and thus couldn't be installed in the
kernel. This commit changes that, by introducing "internal" OpenFlow
rules to associate with such packets.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Sat, 5 May 2012 00:05:08 +0000 (17:05 -0700)]
ofproto-dpif: Move code closer to left margin in facet_check_consistency().
This makes an upcoming commit break up fewer lines.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 4 May 2012 23:56:03 +0000 (16:56 -0700)]
ofproto-dpif: Factor parts of update_stats() out into separate functions.
This makes the code more readable in an upcoming commit.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 4 May 2012 21:56:40 +0000 (14:56 -0700)]
odp-util: Change user_action_cookie from struct to union.
An upcoming commit will introduce a new type and a new use for the
additional members. It seems cleanest to use a union, rather that using
the existing members multiple ways.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 9 May 2012 16:36:08 +0000 (09:36 -0700)]
ofproto-dpif: Clean up and centralize sFlow logic.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 2 May 2012 21:23:28 +0000 (14:23 -0700)]
odp-util: Use switch for checking values of an enum.
The compiler warns when we forget to handle some value of an enum, whereas
it won't for a sequence of 'if' statements.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 2 May 2012 21:22:17 +0000 (14:22 -0700)]
odp-util: Fix typo in comment.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 3 May 2012 22:32:52 +0000 (15:32 -0700)]
ofproto-dpif: Minor style fix.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 4 May 2012 21:52:17 +0000 (14:52 -0700)]
ofproto: Don't limit flows in OpenFlow tables by default.
This bug is ordinarily not exposed because bridge_configure_tables() in
bridge.c configures the max number of flows soon after an ofproto is
created. But an upcoming commit will make construct() in ofproto-dpif.c
try to create some built-in flows before bridge gets control, so we need
to allow creating flows immediately upon initialization.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Brian Kruger [Wed, 9 May 2012 16:13:42 +0000 (09:13 -0700)]
rhel: Add timeouts to network scripts.
If the daemon(s) aren't running for whatever reason, the RHEL ovs
ifup/ifdown scripts don't take that into account and an attempt to reboot a
system could take forever. (literally. endless loop!) Here are a couple of
patches (one of ifup, one for ifdown) to add timeouts (10 seconds), because
it runs per interface you have configured and that could take awhile to
reboot a system if needed.
Signed-off-by: Brian Kruger <bkruger+ovsdev@gmail.com>
[blp@nicira.com fixed up a conflict against master]
Signed-off-by: Ben Pfaff <blp@nicira.com>
Gurucharan Shetty [Wed, 9 May 2012 16:55:53 +0000 (09:55 -0700)]
debian: Remove --no-wait option from the ifupdown script.
Using the --no-wait option in the ifupdown script creates a
race condition where-in the network devices may not yet be created
after ovs-vsctl returns successfully.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Justin Pettit [Mon, 7 May 2012 19:09:02 +0000 (12:09 -0700)]
Prepare for 1.7.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Mon, 7 May 2012 17:33:34 +0000 (10:33 -0700)]
ovs-ofctl: Use port description stat to look up ports when necessary.
Not all ports may fit in a Features Reply, so if that's the case, then
use the new port description stat message for looking up ports.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Sat, 5 May 2012 00:27:16 +0000 (17:27 -0700)]
ovs-ofctl: Support large number of ports with "show" command.
OpenFlow Features Reply messages prior to 1.3 can give users the wrong
impression about how many ports are on the system. With this commit,
the command will check if the number of ports may be truncated. If so,
it will send a Port Description stats request to get the complete list
and ignore the Features Reply port list.
Bug #11087
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Tue, 8 May 2012 07:01:11 +0000 (00:01 -0700)]
ofp-util: Factor out determining physical port size.
There are a few places where we determine the size of a physical port
structure based on the OpenFlow version. Use a helper function to do
that.
Suggested-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Sat, 5 May 2012 00:19:26 +0000 (17:19 -0700)]
ofp-util: Add ofputil_switch_features_ports_trunc function.
Add function to determine whether the max number of ports are contains
in a Features Reply. If so, it removes the port list, since it may be
incomplete. This function will be used in a later commit.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Fri, 4 May 2012 21:42:04 +0000 (14:42 -0700)]
ofproto: Add support for OF1.3 port description multipart message.
OpenFlow 1.0 is limited to displaying 1364 ports in the Features Reply
message, and there is no other way to get consolidated port information.
OpenFlow 1.3 adds a new port description multipart message
(OFPMP_PORT_DESC) that is not limited by size. This commit adds support
through the OpenFlow 1.0 stats mechanism, since they have complimentary
enum values.
Bug #11040
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Justin Pettit [Wed, 2 May 2012 23:35:49 +0000 (16:35 -0700)]
OpenFlow: Move stats message enums into "common".
We will be adding some OpenFlow 1.3 stats (aka multipart request)
messages to our OpenFlow 1.0 implementation. As such, move the
definition of those message numbers to the common location.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Pravin B Shelar [Tue, 8 May 2012 00:04:57 +0000 (17:04 -0700)]
datapath: Validation of IPv6 set port action uses IPv4 header
When the kernel validates set TCP/UDP port actions, it looks at
the ports in the existing flow to make sure that the L4 header exists.
However, these actions always use the IPv4 version of the struct.
Following patch fixes this by checking for flow ip protocol first.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #11205
Ben Pfaff [Mon, 7 May 2012 19:30:54 +0000 (12:30 -0700)]
ofp-util: Treat a packet-out in_port of OFPP_CONTROLLER as OFPP_NONE.
Some OpenFlow 1.0 controllers incorrectly use OPFP_CONTROLLER as the
in_port in packet-out messages, when OFPP_NONE is their intent. Until now,
Open vSwitch has rejected such requests with an error message. This commit
makes Open vSwitch instead treat OFPP_CONTROLLER the same as OFPP_NONE for
compatibility with those controllers.
(Also, as of this writing, OpenFlow 1.0.1 appears to be changing the port
to use from OFPP_NONE to OFPP_CONTROLLER.)
Suggested-by: Rob Sherwood <rob.sherwood@bigswitch.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 4 May 2012 17:05:44 +0000 (10:05 -0700)]
ofproto-dpif: Accept OpenFlow-like flows in "ofproto/trace".
Until now it has not been possible to directly trace flows that include
register values and other concepts that are not in datapath flows, because
"ofproto/trace" requires a flow in the format output by "ovs-dpctl
dump-flows", which doesn't know anything about registers. This commit
makes it possible to instead specify an OpenFlow-like flow.
Feature #10084.
Requested-by: Igor Ganichev <iganichev@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 4 May 2012 16:52:37 +0000 (09:52 -0700)]
ofp-parse: New function parse_ofp_exact_flow().
This function parses a flow rather than a cls_rule. It will be useful
for "ofproto/trace", which currently requires an odp_flow and thus can't
accept values for registers and other concepts that don't exist in the
kernel.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 27 Apr 2012 16:41:02 +0000 (09:41 -0700)]
ofp-util: Avoid ovs_fatal() in ofputil_parse_key_value().
ofputil_parse_key_value() is safe to use from a process that must not abort
except in one case: where the argument contains unbalanced parentheses.
This commit eliminates that call to ovs_fatal(), instead just treating the
end of the string as closing all nested parentheses.
It would be better to propagate the error condition upward, but I'm not
sure that it's worth it just for this one corner case.
The purpose of this commit is to make it possible to use this function
indirectly within the "ofproto/trace" implementation, which must never
abort ovs-vswitchd. (All the current callers are within ovs-ofctl and
other utilities.)
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 27 Apr 2012 15:47:59 +0000 (08:47 -0700)]
ofproto-dpif: Factor out VLAN splinter flow adjustment into new function.
An upcoming commit will add a new user.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 7 May 2012 16:34:12 +0000 (09:34 -0700)]
debian: Synchronize debian/changelog with downstream Debian changelog.
Reported-by: Greg Dahlman <gdahlman@hotmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Gurucharan Shetty [Fri, 4 May 2012 02:23:16 +0000 (19:23 -0700)]
debian: Add network integration scripts.
Add scripts that will allow Open vSwitch bridges and ports to be
configured through /etc/network/interfaces. This patch follows a
very similar style as OVS network integration for rhel.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Ansis Atteka [Thu, 3 May 2012 20:48:40 +0000 (13:48 -0700)]
datapath: Release rtnl_lock if ovs_vport_cmd_build_info() failed
This patch fixes a possible lock-up bug where rtnl_lock might not
get released.
Acked-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Raju Subramanian [Wed, 2 May 2012 22:21:36 +0000 (15:21 -0700)]
Global replace of Nicira Networks.
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>
Justin Pettit [Wed, 2 May 2012 17:20:07 +0000 (10:20 -0700)]
ofproto: Rate-limit STP status errors on non-existent ports.
As part of the bridge's main loop, it queries the STP status of all
ports. If the port doesn't exist, log files can become filled with
warning messages. This situation is very unusual, since system devices
do not normally disappear, but it's easy enough to rate-limit these
messages.
Bug #10936
Reported-by: Reid Price <reid@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Tue, 1 May 2012 23:24:09 +0000 (16:24 -0700)]
DESIGN: Document uses for flow cookies.
The paragraph near the end that starts out "However, unlike OpenFlow 1.1,
..." seems to correctly document OVS behavior, but it also seems like
pretty lousy behavior. Justin says that he's going to fix it before we
put out an OVS release version with this behavior.
CC: Justin Pettit <jpettit@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 1 May 2012 22:36:44 +0000 (15:36 -0700)]
ovs-ctl: Make "force-reload-kmod" warn when DHCP clients must be restarted.
This should make it more obvious when the admin needs to restart a DHCP
client (or other daemon). Without this, unless the admin carefully reads
the documentation, the first notice he gets about a need to restart the
DHCP client can easily be when the lease expires and the machine drops off
the network.
Bug #5391.
Tested-by: Gurucharan Shetty <gshetty@nicira.com>
Suggested-by: Duffie Cooley <dcooley@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 27 Apr 2012 20:32:33 +0000 (13:32 -0700)]
tests: Fix typo in comment.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 1 May 2012 22:28:08 +0000 (15:28 -0700)]
ovs-ofctl: Document padding in "note" actions.
Reported-by: Paul Ingram <paul@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 26 Apr 2012 22:54:54 +0000 (15:54 -0700)]
ofp-print: Add some more white space to flow dumps.
Before idle_age and hard_age were added, in the absence of timeouts there
was a space between the statistics for a flow and the start of the flow
match. This restores that space.
Requested-by: Paul Ingram <paul@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 30 Apr 2012 18:32:13 +0000 (11:32 -0700)]
ovs-ofctl: Document importance of priorities.
Multiple users have asked me about this.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 30 Apr 2012 18:24:59 +0000 (11:24 -0700)]
vswitch.xml: Document more details of CFM intervals.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 30 Apr 2012 18:24:31 +0000 (11:24 -0700)]
ovs-appctl: Point to individual daemon manpages in docs.
Sometimes users seem surprised that appctl commands are documented.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 30 Apr 2012 18:23:38 +0000 (11:23 -0700)]
netdev: Document use for get_etheraddr member of struct netdev_class.
This has confused developers adding hardware support, e.g.:
http://openvswitch.org/pipermail/dev/2012-April/016350.html
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 30 Apr 2012 20:56:49 +0000 (13:56 -0700)]
connmgr: Log when controllers are added and removed.
Otherwise occasionally during debugging it can hard to figure out why a
controller connection seemed to drop for a while (when in fact it happened
because the configuration changed).
Suggested-by: Natasha Gude <natasha@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 30 Apr 2012 19:59:11 +0000 (12:59 -0700)]
INSTALL.Linux: Document use of "sparse" with Open vSwitch.
Signed-off-by: Ben Pfaff <blp@nicira.com>