This seems like a more reasonable way to do things given that we will soon
need "OpenFlow 1.1 and later" and "OpenFlow 1.1 only" keywords.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Reviewed-by: Simon Horman <horms@verge.net.au>
The error message should mention the keyword that caused the error, not
some other random keyword from previous iterations.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Reviewed-by: Simon Horman <horms@verge.net.au>
Commit 11460e2316
(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.
Bug #10194
Signed-off-by: Jesse Gross <jesse@nicira.com>
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>
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.
Feature #10074
Requested-by: Rajiv Ramanathan <rramanathan@nicira.com>
Signed-off-by: Mehak Mahajan <mmahajan@nicira.com>
We currently check that a packet is IPv4 and TCP before fetching the
TCP flags. This enables fetching from IPv6 packets as well.
Bug #10194
Reported-by: Michael Mao <mmao@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
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>
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>
'path' is usually not the right path (often it's NULL).
Introduced in commit bde9f75de (unixctl: New JSON RPC back-end.).
Signed-off-by: Ben Pfaff <blp@nicira.com>
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>
Internal device mtu does not influence mtu of other internal devices.
So skip MTU update to other devices when internal device mtu is changed.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Commit 1bf2c90968 (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>
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>
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>
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/.
Signed-off-by: Ansis Atteka <aatteka@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>
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>
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.
Signed-off-by: Ethan Jackson <ethan@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>
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.
Good riddance.
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>
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>
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>
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.
Signed-off-by: Ben Pfaff <blp@nicira.com>
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>
Update port does not check changed MTU for internal port which allows
administrator assign larger MTU compared to non-internal port.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
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>
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>
