Declare the version as "1.4.5".

Signed-off-by: Justin Pettit <jpettit@nicira.com>

Revert "datapath: Avoid null deref when GSO is for verifying header integrity only."

This reverts commit 00c7faf3e5b7d4020e995a1429cf94313f197171.

In general, it should not be possible have a NULL return value from
skb_gso_segment() since we're not actually trying to verify the
header integrity.  No other callers with similar needs have NULL
checks.  The actual cause of the problem was LRO packets, which
OVS isn't equipped to handle.  The commit
33e031e99cc630baf1b0cb9256710dee7d9ab66d (datapath: Move LRO check
from transmit to receive.) solves that problem by fixing the LRO
check.  In order to avoid possibly masking any other problems, this
reverts the GSO check which should no longer be needed.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Conflicts:

datapath/datapath.c

datapath: Move LRO check from transmit to receive.

Commit 24b019f808211a95078efd916064af0975ca5733 (datapath: Disable
LRO from userspace instead of the kernel.) accidentally moved the
check for LRO packets from the receive path to transmit.  Since
this check is supposed to protect OVS (and other parts of the system)
from packets that it cannot handle it is obviously not useful on
egress.  Therefore, this commit moves it back to the receive side.

The primary problem that this caused is upcalls to userspace tried
to segment the packet even though no segmentation information is
available.  This would later cause NULL pointer dereferences when
skb_gso_segment() did nothing.

Bug #14772

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

datapath: Avoid null deref when GSO is for verifying header integrity only.

skb_gso_segment() has the following comment:

 *    It may return NULL if the skb requires no segmentation.  This is
 *    only possible when GSO is used for verifying header integrity.

Somehow queue_gso_packets() has never hit this case before, but some
failures have suddenly been reported.  This commit should fix the problem.

Additional commentary by Jesse: We shouldn't normally be hitting this case
because we're actually trying to do GSO, not header validation.  However, I
guess the guest/backend must be generating a packet with an MSS, which
tricks us into thinking that it's GSO, but no GSO is actually requested.
In the case of the bridge, header validation does take place so the
situation is handled already.  It seems not ideal that the network backend
doesn't sanitize these packets but it's probably good that we handle
it in any case.

Bug #14772.
Reported-by: Deepesh Govindan <dgovindan@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

debian/changelog: Correct date.

Signed-off-by: Justin Pettit <jpettit@nicira.com>

Declare the version as "1.4.4".

Signed-off-by: Justin Pettit <jpettit@nicira.com>

bond: Change rebalance ratio to be more fair

The old algorithm tries to converge to 0, despite it would mean a very
unbalanced situation. We're aiming for an ideal ratio of 1, meaning both
the 'from' and 'to' slave have the same load.  Therefore, we only move an
entry if it decreases the load on 'from', and brings us closer to equal
traffic load.

Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

ovs-ctl: Exit, instead of resuming, after handling fatal signals.

When I wrote the "trap" calls in ovs-ctl, I had the mistaken notion that
"trap $cmd $signal" would execute $cmd and then exit when $signal was
caught.  This is incorrect.  Instead, it executes $cmd and then resumes
executing the shell script.

On the other hand, "trap $cmd 0" does by itself what I wanted: it causes
the shell to execute $cmd and then exits due to the signal.  So this commit
changes the offending traps to use this form.

Bug #14290.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>

meta-flow: Don't allow negative port numbers.

If a negative number is supplied, the parsing code used to convert it
into a signed one.  We ran into an incident where a third-party script
was attempting to get the OpenFlow port number for an interface, but got
-1 from the database, since the number had not yet been assigned.  This
was converted to 65535, which maps to OFPP_NONE and all flows with
ingress port OFPP_NONE were modified.  This commit disallows negative
port numbers to help prevent broken integration scripts from disturbing
the flow table.

Issue #14036

Signed-off-by: Justin Pettit <jpettit@nicira.com>

python/ovs/stream: Fix Stream.connect() retval for incomplete connection.

If the loop condition in Stream.connect() was false, which is especially
likely for TCP connections, then Stream.connect() would return None,
which violates its documented behavior.  This commit fixes the problem.

Reported-by: Isaku Yamahata <yamahata@valinux.co.jp>
Tested-by: Isaku Yamahata <yamahata@valinux.co.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>

datapath: Fix Tunnel options TOS

Use DSCP bits from ToS set on tunnel.

This is a crossport of commit 749ae9504293dbb695dd67402acbd47acbcbeb83
from master.

Bug #8822.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

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.

This is a crossport of commit 7a6c067d1ad65ae4abdb723b25a4ab591d1d2bc3
from master.

Acked-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ansis Atteka <aatteka@nicira.com>

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.

This is a crossport of commit 9c47b45a3bb56009bf2553c493d097eeadd7e5c2
from master.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>

datapath: Move CSUM_MANGLED_0 definition to net checksum header.

Following patch fixes compilation error on older kernel.

This is a crossport of commit 08d19ca9fef29b23826f1fb52e2368a9077783ca
from master.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

datapath: Fix checksum update for actions on UDP packets.

When modifying IP addresses or ports on a UDP packet we don't
correctly follow the rules for unchecksummed packets.  This meant
that packets without a checksum can be given a incorrect new checksum
and packets with a checksum can become marked as being unchecksummed.
This fixes it to handle those requirements.

This is a crossport of commit 55ce87bcd542cc26def11000c9dee7690b7c3155
from master.

Bug #8937.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

datapath: Honor dp_ifindex, when specified, for vport lookup by name.

When OVS_VPORT_ATTR_NAME is specified and dp_ifindex is nonzero, the
logical behavior would be for the vport name lookup scope to be limited
to the specified datapath, but in fact the dp_ifindex value was ignored.
This commit causes the search scope to be honored.

This is a crossport of commit 24ce832d5e076e5686b15d2aadd39e8c0818e932
from master.

Bug #9889.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

netdev-vport: Fix ToS/TTL mixup in unparsing.

When unparsing the kernel tunnel configuration, TTL was incorrectly
converted to "tos".  Although it leads to confusing configuration
output, actual operation is not affected.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>

Declare the version as "1.4.3".

datapath: Fix FLOW_BUFSIZE definition.

This is analogous to the change made in userspace with
2508ac16defd417b94fb69689b6b1da4fbc76282 (odp-util: Update
ODPUTIL_FLOW_KEY_BYTES for current kernel flow format.).  The extra
space for vlan encapsulation was not included in the allocation for
maximum length flows.

Found by code inspection and to my knowledge has never been hit, likely
because skb allocations are padded out to a cacheline, making userspace
more susceptible to this problem than the kernel.  In theory, however,
the right combination of flow and packet size could result in a kernel
panic.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>

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>

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>

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>

ovs-ctl: Add support for newer name for Open vSwitch kernel module.

Open vSwitch 1.4 and later is compatible with the upstream Linux kernel
module but the init scripts hadn't been adapted to work with the upstream
module name.

Debian bug #684057.
Signed-off-by: Ben Pfaff <blp@nicira.com>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

vlog: Avoid use-after-free in corner case.

Found by valgrind.

Signed-off-by: Ben Pfaff <blp@nicira.com>

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

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>

Declare the version as "1.4.2".

Fix typo in "PYTHONPATH".

Reported-by: Justin Pettit <jpettit@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

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>

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>

ofproto: 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>

odp-util: Fix parsing of actions encapsulated within "sample" actions.

Signed-off-by: Ben Pfaff <blp@nicira.com>

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>

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

debian: Synchronize debian/changelog with downstream Debian changelog.

Reported-by: Greg Dahlman <gdahlman@hotmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

Declare the version as "1.4.1".

meta-flow: Correctly set destination MAC in mf_set_flow_value().

Signed-off-by: Ben Pfaff <blp@nicira.com>

tests: Fix mirroring tests on big-endian architectures.

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>

Fix memory leaks.

Found by valgrind.

Reported-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

ofproto: Fix use-after-free error when ports disappear.

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>

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>

xenserver: Recognize XenServer 5.6-SP2 scripts in RPM %post.

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>

debian: Move PKI directory to FHS-compliant location.

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>

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:

    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>

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:

    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>

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.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ofproto-dpif: Fix return type of rule_calculate_tag().

tag_type is currently uint32_t but using uint32_t directly is conceptually
wrong.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ovs-vsctl: Allow "fake bridges" to be created for VLAN 0.

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>

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>

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>

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>

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>

ovsdb-doc: Put NAME section into generated manpage.

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>

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>

doc: Fix typo in manpage.

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>

debian: Bump standards-version to 3.9.3.

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>

debian: Remove some useless files from the dkms pacakge.

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>

debian: Clean .pyc files in "clean" target.

Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>

debian: Remove po-debconf build dependency.

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>

debian: Build-depend on python-all to pull in all Python versions.

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>

debian: Add missing ${python:Depends} to openvswitch-test package.

Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>

debian: Improve long descriptions so as to better describe the packages.

Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>

debian: Bump debhelper compat level to 8 and make build-depends consistent.

Reviewed-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Thomas Goirand <zigo@debian.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>

netlink-socket: Increase Netlink socket receive buffer size.

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>

tests: Skip "strings at least 2 characters long" test for narrow Python.

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>

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.

Signed-off-by: Ben Pfaff <blp@nicira.com>

ovs-monitor-ipsec: Detect correctly IPSEC configuration changes

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>

Revert "ovs-monitor-ipsec: Detect correctly IPSEC configuration changes"

This reverts commit 5e2a9988bb7853cad67a36e869d532d9d2f4533a.

Signed-off-by: Ansis Atteka <aatteka@nicira.com>

ovs-monitor-ipsec: Detect correctly IPSEC configuration changes

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>

bond: Incorrectly reported an error in appctl.

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>

ovs-xapi-sync: Rerun processing when a db update arrives during a commit.

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>

socket-util: Unlink Unix domain sockets that bind but fail to connect.

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>

vswitchd: Document behavior of 802.1p priorities with VLAN splinters.

Reported-by: likunyun <kunyunli@hotmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

bridge: Remove unwanted ports at time of ofproto creation.

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>

netdev: Fix typo in error message.

Found by inspection.

Signed-off-by: Ben Pfaff <blp@nicira.com>

INSTALL.Linux: minor typo

s/ovsdmonitor/ovsdbmonitor/

Signed-off-by: Chris Wright <chrisw@sous-sol.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>

tests: Prefer development Python files over installed ones.

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>

xenserver: Restart ovs-xapi-sync on kmod reload.

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>

xenserver: Always update the bridge ID in ovs-xapi-sync.

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>

debian: Fix exit status of openvswitch-switch init script "status" command.

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>

dpif-linux: Make dpif_linux_port_query_by_name() query only one datapath.

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>

ofproto-dpif: Cleanup STP on ports when disabled on their bridge.

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.

configure: Try to extract kernel source directory from build Makefile.

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>

vswitchd: Always configure a default queue for QoS.

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>

xenserver: Fix iteration of dictionary.

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>