| Age | Commit message (Collapse) | Author |
|---|
|
|
|
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
These tests had a hidden dependency on the hash function in use,
which yields different results on big-endian and little-endian
architectures. This commit fixes the problem by properly
parameterizing the parts that can differ.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Found by valgrind.
Reported-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
update_port() can delete the port for which it is called, if the underlying
network device has been destroyed, so HMAP_FOR_EACH is unsafe in
ofproto_run().
Less obviously, update_port() can delete unrelated ports. For example,
suppose that initially device A is port 1 and device B is port 2. If
update_port("A") runs just after this, then it will ofport_remove() both
ports, then ofport_install() A as the new port 2.
So this commit first assembles a list of ports to update, then updates them
in a separate loop.
Without this commit, running "ovs-dpctl del-dp" while ovs-vswitchd is
running consistently causes a crash for me within a few seconds.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
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>
|
|
Somehow we forgot to put the md5sums for 5.6-SP2 so users were getting
scary error messages.
Bug #10210.
Reported-by: Ronald Lee <rlee@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
The PKI directory is mutable state, so it should be in /var, not in /usr.
This commit changes its location and, on systems upgraded from earlier
versions, moves the existing PKI and leaves behind a symlink.
CC: 661090@bugs.debian.org
Reported-by: Andreas Beckmann <debian@abeckmann.de>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
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:
ovs-ofctl del-flows br0
ovs-ofctl add-flow br0 "table=0 actions=learn(table=1, \
idle_timeout=600, NXM_OF_VLAN_TCI[0..11], \
NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[], \
output:NXM_OF_IN_PORT[], fin_idle_timeout=10), resubmit(,1)"
ovs-ofctl add-flow br0 "table=1 priority=0 actions=flood"
Without this patch, flows don't get properly invalidated upon initial MAC
learning, so one sees warnings like the following:
in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),
eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,
ttl=64,frag=no),tcp(src=13966,dst=0): inconsistency in subfacet
(actions were: 3,0,1) (correct actions: 1)
This patch fixes the problem and thus avoids these warnings.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
"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:
ovs-ofctl del-flows br0
ovs-ofctl add-flow br0 "table=0 actions=learn(table=1, \
idle_timeout=600, NXM_OF_VLAN_TCI[0..11], \
NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[], \
output:NXM_OF_IN_PORT[], fin_idle_timeout=10), resubmit(,1)"
ovs-ofctl add-flow br0 "table=1 priority=0 actions=flood"
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>
|
|
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.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
tag_type is currently uint32_t but using uint32_t directly is conceptually
wrong.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
A fake bridge for VLAN 0 is useful, because it provides a way to create
access ports for VLAN 0. There is no good reason to prevent it.
NIC-464.
Reported-by: Rob Hoes <Rob.Hoes@citrix.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
This makes the manpage indexable by standard system tools.
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>
|
|
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>
|
|
Found by lintian.
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
No other changes necessary.
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
This commit removes useless files from the dkms package that caused
lintian warnings.
(Many of the other files in the dkms package are also useless but do not
cause lintian warnings so they are less important.)
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Open vSwitch no longer uses Debconf at all, for some time now.
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Open vSwitch should support all Python versions in the distribution. This
is the way to do it.
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
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>
|
|
Narrow Python can't handle Unicode characters outside the BMP, so skip the
test.
Reported-by: Michael Shigorin <mike@osdn.org.ua>
Tested-by: Michael Shigorin <mike@osdn.org.ua>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
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.
Signed-off-by: Ben Pfaff <blp@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.
v2: Do not fail buildtests
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Reported-by: Niklas Andersson <nandersson@nicira.com>
|
|
This reverts commit 5e2a9988bb7853cad67a36e869d532d9d2f4533a.
Signed-off-by: Ansis Atteka <aatteka@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.
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
Reported-by: Niklas Andersson <nandersson@nicira.com>
|
|
The bond/enable-slave and bond/disable-slave ovs-appctl commands
incorrectly reported the 501 error code upon success.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
The logic in ovs-xapi-sync didn't handle the case where ovsdb-server sends
a database update before it replies to a transaction that ovs-xapi-sync
sent, like this:
ovs-xapi-sync ovsdb-server
------------- ------------
.
.
.
transaction request --->
<--- database contents update
<--- transaction reply
.
.
.
The update was not lost but ovs-xapi-sync would not process it until the
database changed again.
Bug #10082.
Reported-by: Krishna Miriyala <krishna@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
The error handling path here failed to clean up bound sockets, by removing
them. This fixes the problem.
It was easy to observe this bug by running "ovs-vsctl" without
"ovsdb-server" running.
Bug #9811.
Bug #9769.
Reported-by: Michael <mhu@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Reported-by: likunyun <kunyunli@hotmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
The reconfiguration code only deleted unwanted ports for bridges that had
been created in previous (re)configurations. In fact, we should run this
step even for bridges that are newly added, e.g. to delete ports that
were added by a previous run of ovs-vswitchd and deleted from the database
between runs.
Before this commit, the following left "int" in datapath br0. After this
commit, "int" is properly deleted:
1. With ovs-vswitchd running:
# ovs-vsctl add-br br0
# ovs-vsctl add-port br0 int -- set interface int type=internal
2. Kill ovs-vswitchd, then:
# ovs-vsctl --no-wait -- del-port br0 int
3. Restart ovs-vswitchd.
Bug #9957.
Reported-by: Hiroshi Tanaka <htanaka@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Found by inspection.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
s/ovsdmonitor/ovsdbmonitor/
Signed-off-by: Chris Wright <chrisw@sous-sol.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
A developer may have Open vSwitch installed, in which case many of
the Python files which are tested will be in both the development
tree and the system Python library. When running unit tests, we
want to test the development tree, so it's better to prefer
importing those files.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
|
|
Some users never restart OVS, they just reload the kernel module on
each new version. Since ovs-xapi-sync is a daemon, a restart is
required to use the new code. Therefore, without this patch, users
could unwittingly use stale versions of ovs-xapi-sync.
Bug #9919.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Diagnosed-by: Ben Pfaff <blp@nicira.com>
|
|
In some cases we were seeing this column get stale.
Bug #9929.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Diagnosed-by: Justin Pettit <jpettit@nicira.com>
|
|
The init script ends with an explicit "exit 0" so nonzero exit codes from
"ovs-ctl status" were being lost.
Bug #9714.
Reported-by: Paul Ingram <paul@nicira.com>
CC: Sujatha Shetty <sshetty@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
The kernel will report a vport with the given name in any datapath, but
userspace only wants a vport with the given name in a specific datapath.
Receiving information on a vport in an unexpected datapath yields bizarre
and hard-to-debug problems.
Bug #9889.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
When STP is enabled on a bridge, the STP module puts its ports in
an STP_LISTENING state until STP converges. This causes all
traffic destined for these ports to be dropped. If STP is disabled
on the bridge, but not explicitly disabled on its ports, the bridge
fails to remove the STP state from these ports. Therefore, if a
port is in an STP_LISTENING state, it will remain in that state and
continue to drop all traffic indefinitely. This patch fixes the
issue.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Bug #9157.
|
|
OVS needs to inspect the headers in the kernel source directory at build
time. Debian keeps moving the source directory relative to the build
directory and doesn't provide an obvious way to find the source directory,
so in the past we've used some name-based heuristics to essentially guess
where it is.
This commit introduces a new heuristic that I hope will be more reliable:
extracting the source directory from the Makefile in the build directory.
In Debian's case, it looks like the Makefile generally contains a line of
the form "MAKEARGS := -C <srcdir> O=<outdir>". This commit extracts the
source directory from that line.
To avoid regressions this commit retains the older heuristics as fallbacks.
CC: 659685@bugs.debian.org
Reported-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
When an interface has QoS configured but no default queue (queue 0), OVS
has until now installed that QoS configuration literally, which causes all
packets destined for this default queue to be dropped. This behavior is
usually both unexpected and undesirable.
This commit changes behavior so that, when no default queue is configured,
OVS uses a default "empty" configuration for the default queue. This
behavior should be more acceptable when QoS is slightly misconfigured.
I tested that, without this patch, configuring only queue 1 causes "tc
class show" to show only queue 1 (handle 1:2) for linux-htb and linux-hfsc,
and that with this patch it shows configurations for both queue 0 (handle
1:1) and queue 1.
Bug #5583.
Feature #7413.
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
|
Fix bug in commit 3249bb907a1dab9b0, which incorrectly assumed that
get_all_records_where() returned a list. It in fact returns a
dictionary and the list iteratory needs to change to account for this.
Thanks to Nicira for pointing this out.
NIC-454.
Reported-by: David Tsai <dtsai@nicira.com>
Acked-by: Rob Hoes <rob.hoes@citrix.com>
Signed-off-by: Dominic Curran <dominic.curran@citrix.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
|
