Ben Pfaff [Tue, 27 Mar 2012 22:57:52 +0000 (15:57 -0700)]
Avoid possibly including an old vswitch-idl.h.
Codes that uses #include "vswitch-idl.h" can get an older version of this
header, because this header file moved from vswitchd/ to lib/ and the
older generated file might still be present.
This helps out two ways:
* "make clean" will delete the generated files from their old
locations.
* Use #include "lib/vswitch-idl.h" to explicitly avoid including the
files from their old locations.
Reported-by: Justin Pettit <jpettit@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 27 Mar 2012 17:20:56 +0000 (10:20 -0700)]
dpif-netdev: Correct type of struct dp_netdev_flow's 'tcp_flags' member.
TCP flags fit in 8 bits so this type seems more appropriate.
Fixes the following "sparse" warnings introduced by commit 734ec5ec1349
(packet: Add additional TCP flags extraction on IPv6.):
dpif-netdev.c:630: warning: incorrect type in assignment (different base types)
dpif-netdev.c:630: expected unsigned char [unsigned] [usertype] tcp_flags
dpif-netdev.c:630: got restricted __be16 [usertype] tcp_flags
dpif-netdev.c:979: warning: invalid assignment: |=
dpif-netdev.c:979: left side has type restricted __be16
dpif-netdev.c:979: right side has type unsigned char
CC: Jesse Gross <jesse@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Simon Horman [Mon, 26 Mar 2012 20:46:35 +0000 (13:46 -0700)]
Add error codes for Open Flow v1.2
* Where Open Flow 1.2 breaks apart error codes defined
in previous versions, provide all new definitions to
previous versions and map the numeric error code to
the first first definition supplied in ofp-errors.h.
The case handled so far is:
OFPERR_OFPBIC_BAD_EXP_TYPE -> { OFPERR_OFPBIC_BAD_EXPERIMENTER,
OFPERR_OFPBIC_BAD_EXP_TYPE }
* Where Open Flow 1.2 adds error codes that were previously
defined as Nicira extension errors define the later in terms
of the new codes.
Signed-off-by: Simon Horman <horms@verge.net.au>
[blp@nicira.com added better error checking in extract-ofp-errors, added
unit tests, miscellaneous cleanup] Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Simon Horman <horms@verge.net.au>
Ben Pfaff [Mon, 26 Mar 2012 18:25:52 +0000 (11:25 -0700)]
ofp-errors: Use OF1.1+ in place of OF1.1 throughout.
In general, I guess that the common case is for most error codes to be
retained without change in future versions of OpenFlow, so to me it seems
best to use the "+" version "by default".
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Simon Horman <horms@verge.net.au>
Jesse Gross [Mon, 26 Mar 2012 19:56:14 +0000 (12:56 -0700)]
packet: Add additional TCP flags extraction on IPv6.
Commit 11460e2316b88f0bd0ea0005d94338d800ea16bd
(flow: Enable retrieval of TCP flags from IPv6 traffic.) updated
one of the TCP flags extraction functions in userspace but missed
the other. This updates that function and converts the other to
use it to reduce duplication.
Raju Subramanian [Fri, 23 Mar 2012 22:49:32 +0000 (15:49 -0700)]
ovs-bugtool: Add ability to prioritize files by date.
When size limit is reached in the middle of processing a dir,
the report ends up containing oldest files. This change adds
an optional param in the plugin to prioritize newer files.
Feature #9937 Requested-by: Ronald Lee <rlee@nicira.com> Signed-off-by: Raju Subramanian <rsubramanian@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Mehak Mahajan [Sat, 10 Mar 2012 23:58:10 +0000 (15:58 -0800)]
Allow configuring DSCP on controller and manager connections.
The changes allow the user to specify a separate dscp value for the
controller connection and the manager connection. The value will take
effect on resetting the connections. If no value is specified a default
value of 192 is chosen for each of the connections.
Jesse Gross [Fri, 23 Mar 2012 20:14:51 +0000 (13:14 -0700)]
flow: Add length check when retrieving TCP flags.
When collecting TCP flags we check that the IP header indicates that
a TCP header is present but not that the packet is actually long
enough to contain the header. This adds a check to prevent reading
off the end of the packet.
In practice, this is only likely to result in reading of bad data and
not a crash due to the presence of struct skb_shared_info at the end
of the packet.
Signed-off-by: Jesse Gross <jesse@nicira.com> Acked-by: Pravin B Shelar <pshelar@nicira.com>
Pravin B Shelar [Fri, 23 Mar 2012 21:14:24 +0000 (14:14 -0700)]
vswitchd: Do not refresh existing iface on new device addition.
There is no need to refresh status and stats for existing devices
if iface mtu is missing in ovs-db as missing MTU could just mean
error in last MTU read for that device.
So we can refresh stats for devices which are just created.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Ben Pfaff [Fri, 23 Mar 2012 18:43:54 +0000 (11:43 -0700)]
treewide: Convert tabs to spaces in C source files written in OVS style.
The Open vSwitch C style doesn't use hard tabs.
This commit doesn't touch files written in kernel style or that are
imported from other projects where we want to minimize changes from
upstream (the sflow files).
Reported-by: Mehak Mahajan <mmahajan@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 22 Mar 2012 20:24:23 +0000 (13:24 -0700)]
ovsdb-idlc: Fix memory leak in "optional bool" columns.
Commit 1bf2c9096858 (idl: Generalize special case boolean exception.)
changed the IDL to do dynamic allocation with (x)malloc() for optional
booleans, but it didn't add the corresponding calls to free(). This
commit fixes the problem.
Bug #10357. Reported-by: Paul Ingram <paul@nicira.com> Reported-by: Krishna Miriyala <krishna@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Thu, 22 Mar 2012 23:04:45 +0000 (16:04 -0700)]
ofproto-dpif: Fix CONTROLLER actions for LLC frames.
The CONTROLLER action assumed that all Ethernet frames stored their
Ethernet Type in the two bytes succeeding the source and
destination addresses. This turns out not to be true for 802.2 LLC
frames, potentially causing an assertion failure. This patch
solves the issue by skipping the assertion in this case.
Bug #10349. Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Thu, 22 Mar 2012 17:20:44 +0000 (10:20 -0700)]
debian: Fix log rotation.
Commit 24e81092a1 (debian: Bring Debian packaging in-line with new file
locations) introduced an ambiguous "--t" option invoking ovs-appctl, so
ovs-vswitchd and ovsdb-server were not reopening their log files following
log rotation. This fixes the problem by correct the option name.
Reported-by: Paul Ingram <paul@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ansis Atteka [Wed, 21 Mar 2012 18:11:36 +0000 (11:11 -0700)]
documentation: use correct rundir in INSTALL.Linux to terminate OVS
This patch fixes a minor documentation flaw where INSTALL.Linux uses
incorrect path to find ovsdb-server and ovs-vswitchd PIDs. The default
rundir is actually /usr/local/var/run/openvswitch/ instead of
/usr/local/var/run/.
Ben Pfaff [Tue, 20 Mar 2012 22:26:57 +0000 (15:26 -0700)]
ofproto-dpif: Fix tag caching for learned flows.
This code in xlate_table_action() is supposed to tag flows in tables that
have special forms so that changes do not require revalidating every flow.
When rule->tag is nonzero, its value can be used, because we know in this
case that rule->cr.wc is the same as table->other_table->wc and that thus
rule->tag caches the return value of the rule_calculate_tag() expression.
When rule->tag is zero (a "catchall" rule) we need to calculate the tag
manually because we have no way to cache it in that case.
I discovered this bug by running an "hping3" between a couple of VMs plus
the following commands on OVS in the middle:
Ben Pfaff [Wed, 21 Mar 2012 16:01:02 +0000 (09:01 -0700)]
ofproto-dpif: Avoid segfault deleting facets that execute LEARN actions.
"ovs-ofctl del-flows <bridge>" can result in the following call path:
delete_flows_loose() in ofproto.c
-> collect_rules_loose() -- uses 'ofproto_node' inside 'struct rule'
-> rule_destruct() in ofproto-dpif.c
-> facet_revalidate()
-> facet_remove()
-> facet_flush_stats()
-> facet_account()
-> xlate_actions()
-> xlate_learn_action()
-> ofproto_flow_mod() back in ofproto.c
-> modify_flow_strict()
-> collect_rules_strict() -- also uses 'ofproto_node'
which goes "boom" when we fall back up the call chain because the nested
use of ofproto_node steps on the outer use of ofproto_node.
This commit fixes the problem by refusing to translate "learn" actions
within facet_flush_stats(), breaking the doubled use.
Another possible approach would be to switch to another way to keep track
of rules in the flow_mod implementations, so that there'd be no fighting
over 'ofproto_node'. But then "ovs-ofctl del-flows" might still leave some
flows around (ones created by "learn" actions as flows are accounted as
facets get deleted), which would be surprising behavior. And it seems in
general a bad idea to allow recursive flow_mods; the consequences have not
been carefully thought through.
Before this commit, one can reproduce the problem by running an "hping3"
between a couple of VMs plus the following commands on OVS in the middle.
Sometimes you have to run them a few times:
This commit has a side effect that leftover unaccounted packets no longer
update the timeouts in MAC learning actions in some cases, when the facets
that cause updates are deleted. At most one second of updates should be
lost.
Bug #10184. Reported-by: Michael Mao <mmao@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Tue, 20 Mar 2012 22:00:46 +0000 (15:00 -0700)]
hmap: New function hmap_contains().
This is useful in a situation where one knows that an hmap_node is in some
hmap, but it's not certain which one, and one needs to know whether it is
in a particular one. This is not a very common case; I don't see any
potential users in the current tree, although an upcoming commit will add
one.
Ethan Jackson [Sat, 10 Mar 2012 02:16:20 +0000 (18:16 -0800)]
cfm: Support random VLAN tag for CCM PDUs.
CCM PDUs may take a different path through the network depending on
the VLAN tag they carry. In order to exercise these paths, it
may be advantageous to use a random VLAN tag.
Ben Pfaff [Mon, 19 Mar 2012 20:47:50 +0000 (13:47 -0700)]
netdev-linux: Fix use-after-free when netdev_dump_queues() deletes queues.
iface_configure_qos() passes a callback to netdev_dump_queues() that can
delete queues. The netdev-linux implementation of this function was
unprepared for the callback to delete queues, so this could cause a
use-after-free. This fixes the problem in netdev_linux_dump_queues() and
documents that netdev_dump_queues() implementations must support deletions
in the callback.
Found by valgrind:
==1593== Invalid read of size 8
==1593== at 0x4A8C43: netdev_linux_dump_queues (hmap.h:326)
==1593== by 0x4305F7: bridge_reconfigure (bridge.c:3084)
==1593== by 0x431384: bridge_run (bridge.c:1892)
==1593== by 0x432749: main (ovs-vswitchd.c:96)
==1593== Address 0x632e078 is 8 bytes inside a block of size 32 free'd
==1593== at 0x4C240FD: free (vg_replace_malloc.c:366)
==1593== by 0x4A4D74: hfsc_class_delete (netdev-linux.c:3250)
==1593== by 0x42AA59: iface_delete_queues (bridge.c:3055)
==1593== by 0x4A8C8C: netdev_linux_dump_queues (netdev-linux.c:1881)
==1593== by 0x4305F7: bridge_reconfigure (bridge.c:3084)
==1593== by 0x431384: bridge_run (bridge.c:1892)
Bug #10164. Reported-by: Ram Jothikumar <ram@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 19 Mar 2012 17:07:09 +0000 (10:07 -0700)]
configure: Remove --with-build-number.
From early days, Nicira used the --with-build-number option to configure to
stamp our internal builds. We've since switched to another scheme, so
this option is obsolete.
Ben Pfaff [Fri, 16 Mar 2012 21:18:05 +0000 (14:18 -0700)]
debian: Use a different way to avoid failing install without kernel module.
The dh_installinit --error-handler option makes a lot of sense, but after
playing with it for a while I could not figure out a nice way to use it
only for openvswitch-switch without either duplicating the dh_installinit
fragments in postinst and prerm (the actual bug that was reported) or
omitting them for some package.
Also, we forgot to write the error handler function for the prerm.
This commit switches to a different way to avoid failing the install when
the kernel module is not available, without using --error-handler.
CC: 663051@bugs.debian.org Reported-by: Thomas Goirand <zigo@debian.org> Reviewed-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
Open vSwitch userspace can set up flows at a high rate, but it is somewhat
"bursty" in opportunities to set up flows, by which I mean that OVS sets up
a batch of flows, then goes off and does some other work for a while, then
sets up another batch of flows, and so on. The result is that, if a large
number of packets that need flow setups come in all at once, then some of
them can overflow the relatively small kernel-to-user buffers.
This commit increases the kernel-to-user buffers from the default of
approximately 120 kB each to 1 MB each. In one somewhat synthetic test
case that I ran based on an "hping3" that generated a load of about 20,000
new flows per second (including both requests and replies), this reduced
the packets dropped at the kernel-to-user interface from about 30% to none.
I expect that it will similarly improve packet loss in workloads where
flow arrival is not easily predictable.
(This has little effect on workloads generated by "ovs-benchmark rate"
because that benchmark is effectively "self-clocking", that is, a new flow
is triggered only by a reply to a request made earlier, which means that
the number of buffered packets at any given has a known, constant upper
limit.)
Bug #10210. Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 12 Mar 2012 21:27:25 +0000 (14:27 -0700)]
fail-open: Use connmgr_send_packet_in() instead of connmgr_broadcast().
Otherwise even controllers that should not receive any packet-ins (via
enable-async-messages=false) still receive the packet-ins that probe for
a controller being up when we're in fail-open.
Bug #9964. Reported-by: James Schmidt <jschmidt@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 12 Mar 2012 21:35:35 +0000 (14:35 -0700)]
ofproto: connmgr_send_packet_in() doesn't need buffer_id and total_len.
Trying to add a new caller for connmgr_send_packet_in(), I wasn't sure
what to put in these members. Investigating, I saw that the function
didn't really need them, so this commit clears that up.
Chris Wright [Tue, 13 Mar 2012 23:21:55 +0000 (16:21 -0700)]
configure: add configure option to disable building brcompat
This adds ability to do:
./configure --disable-brcompat
to disable building userspace and kernel module associated with
providing linux bridge compatibility. Sources should still be
distributed w/ make dist.
While there, update comment referring to long removed veth driver
which is now relevant for brcompat module.
Cc: Jesse Gross <jesse@nicira.com> Signed-off-by: Chris Wright <chrisw@sous-sol.org> Acked-by: Ben Pfaff <blp@nicira.com> Signed-off-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Mon, 12 Mar 2012 17:23:36 +0000 (10:23 -0700)]
Use `pwd` in place of $PWD, treewide.
The Autoconf manual says:
Posix 1003.1-2001 requires that `cd' and `pwd' must update the
`PWD' environment variable to point to the logical name of the
current directory, but traditional shells do not support this.
This can cause confusion if one shell instance maintains `PWD' but
a subsidiary and different shell does not know about `PWD' and
executes `cd'; in this case `PWD' points to the wrong directory.
Use ``pwd`' rather than `$PWD'.
so this commit replaces all uses of $PWD by `pwd`.
Reported-by: Justin Pettit <jpettit@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 12 Mar 2012 19:59:47 +0000 (12:59 -0700)]
ofproto: Fix code that keeps track of MTU.
ofport_install() should set the MTU that it finds into the ofport
before calling set_internal_devs_mtu(), because the latter function might
change the MTU and update ofport->mtu and the caller should not incorrectly
overwrite its changes.
Ben Pfaff [Mon, 12 Mar 2012 17:34:22 +0000 (10:34 -0700)]
python: Fix "make distcheck" error on version.py.
The generated version.py has to go in the srcdir and has to be regenerated
based on config.status, which breaks "make distcheck" because it
write-protects the srcdir. However, the contents of version.py only change
when the version number changes, so we can just "touch" it when it doesn't
really need to change.
The same pattern is used elsewhere in the tree for other files in the same
situation, e.g. the various RPM spec files.
Reported-by: Chris Wright <chrisw@sous-sol.org> Acked-by: Chris Wright <chrisw@sous-sol.org> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 9 Mar 2012 23:10:56 +0000 (15:10 -0800)]
ovsdb-doc: Use minus sign in negative numbers in nroff output.
ovs-vswitchd.conf.db.5 has autogenerated text "at least -1" in one place.
This '-' should be a minus sign, but ovsdb-doc was generating it as a
hyphen.
Found by lintian.
Reported-by: Thomas Goirand <zigo@debian.org> Reviewed-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 9 Mar 2012 22:50:39 +0000 (14:50 -0800)]
ovsdb-doc: Convert '-' preceding a number as a minus sign, not a hyphen.
ovs-vswitchd.conf.db.5 contains the following sentence:
If the interface cannot be added then Open vSwitch sets this column
to -1.
The '-' in "-1" should be a minus sign, not a hyphen, but the heuristic
in ovsdb-doc wasn't smart enough. This commit improves the heuristic and
fixes the problem.
Found by lintian.
Reported-by: Thomas Goirand <zigo@debian.org> Reviewed-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Fri, 9 Mar 2012 22:20:54 +0000 (14:20 -0800)]
debian: Avoid unit test failure when doing "unofficial" builds.
The configure option --with-build-number=0 is interpreted differently in
different places. The configure script itself accepts 0 as an actual
build number and puts '#define BUILDNR "+build0"' into config.h. The
code in python/automake.mk treats 0 as "no build number" and puts
'BUILDNR = ""' into version.py.
This commit avoids the problem by not passing 0 as a build number.
Reviewed-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Sat, 3 Mar 2012 01:50:59 +0000 (17:50 -0800)]
idl: Convert python daemons to utilize SchemaHelper.
The recently added SchemaHelper class significantly simplifies IDL
instantiation in Python. This commit converts all users of the old
method to the new method, and removes support for the old method.
Pravin B Shelar [Fri, 9 Mar 2012 20:59:58 +0000 (12:59 -0800)]
netdev-linux: Cache error code from get-features.
Following patch adds support for caching error code from ETHTOOL_GSET
call. Since internal device is virtual device device feature does not
make much sense, so netdev_get_features op is removed for internal
devices.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Pravin B Shelar [Fri, 9 Mar 2012 20:57:48 +0000 (12:57 -0800)]
netdev-linux: Cache error code from mtu ioctl.
netdev linux devices uses mtu ioctl to get and set MTU for a device.
By caching error code from ioctl we can reduce number of ioctl calls
for device which is unregistered from system.
netdev notification is used to update mtu which saves get-mtu-ioctl.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Pravin B Shelar [Fri, 9 Mar 2012 20:57:13 +0000 (12:57 -0800)]
netdev-linux: Cache drv-info for net device.
Netdev-linux calls ETHTOOL_GDRVINFO on every netdev_linux_get_status()
which is not optimal as drv-info does not change for given device.
So following patch changes netdev_linux_get_status() to read drv-info at
device initialization and cache it.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
The first makes the module name consistent with upstream, and the latter
is just for internal consistency. This makes tools, and documentation
refer to a common module name regardless if it's coming from upstream
linux or built from datapath/ as part of a local build.
Signed-off-by: Chris Wright <chrisw@sous-sol.org> Signed-off-by: Jesse Gross <jesse@nicira.com>
If Open vSwitch has IPSEC tunnel (with certificates) and Interface
table was updated, then ovs-monitor-ipsec daemon would incorrectly
remove and readd all existing IPSEC tunnels.
The root cause for this issue was that "peer_cert_file" key was present in
interfaces dictionary, but it was missing in new_interfaces dictionary.
If Open vSwitch has IPSEC tunnel (with certificates) and Interface
table was updated, then ovs-monitor-ipsec daemon would incorrectly
remove and readd all existing IPSEC tunnels.
The root cause for this issue was that "peer_cert_file" key was present in
interfaces dictionary, but it was not present in new_interfaces dictionary.
Chris Wright [Wed, 7 Mar 2012 20:19:04 +0000 (12:19 -0800)]
datapath: update linux/.gitignore
Remove a couple stale entries:
brc_sysfs_* was renamed to dp_sysfs_* a few years ago
Add some new entries to ignore:
# Untracked file:
# (use "git add <file>..." to include in what will be committed)
# datapath/linux/exthdrs_core.c
# datapath/linux/genl_exec.c
# datapath/linux/net_namespace.c
# datapath/linux/workqueue.c
Signed-off-by: Chris Wright <chrisw@sous-sol.org> Signed-off-by: Jesse Gross <jesse@nicira.com>