sliver-openvswitch.git
13 years agovlan-bitmap: New data structure.
Ben Pfaff [Fri, 8 Apr 2011 20:19:33 +0000 (13:19 -0700)]
vlan-bitmap: New data structure.

13 years agobridge: Move logic for flushing flows and standalone mode into connmgr.
Ben Pfaff [Fri, 8 Apr 2011 20:44:38 +0000 (13:44 -0700)]
bridge: Move logic for flushing flows and standalone mode into connmgr.

This improves the abstraction behind ofproto and connmgr.

Some of this could even go into fail_open, but I'm not sure that it would
make anything easier to understand.

13 years agoovs-controller: Honor --wildcard option.
Ben Pfaff [Tue, 26 Apr 2011 00:08:09 +0000 (17:08 -0700)]
ovs-controller: Honor --wildcard option.

This option was documented but ignored.

13 years agotests: Add ovs-openflowd to programs that need valgrind wrappers.
Ben Pfaff [Thu, 21 Apr 2011 23:38:05 +0000 (16:38 -0700)]
tests: Add ovs-openflowd to programs that need valgrind wrappers.

The tests run ovs-openflowd so "make check-valgrind" should run it under
valgrind.

13 years agobridge: Remove slaves from the bond before closing their netdevs.
Ben Pfaff [Thu, 21 Apr 2011 23:37:38 +0000 (16:37 -0700)]
bridge: Remove slaves from the bond before closing their netdevs.

A bond slave has a pointer to its iface's netdev, so we don't want it to
keep that pointer after the bridge closes the netdev.

This is becoming a bit of a mess so perhaps we need reference counting for
netdevs (although Jesse didn't like the idea when I proposed it before).

13 years agobond: Be more careful about adding and removing netdevs in the monitor.
Ben Pfaff [Thu, 21 Apr 2011 23:34:51 +0000 (16:34 -0700)]
bond: Be more careful about adding and removing netdevs in the monitor.

The code was careless about updating the netdev_monitor.  Newly added
slaves weren't added to the monitor until the next bond_reconfigure() call,
and netdevs were never removed from the monitor.

13 years agoofproto: Adjust netdev_monitor when switching netdevs.
Ben Pfaff [Thu, 21 Apr 2011 23:25:41 +0000 (16:25 -0700)]
ofproto: Adjust netdev_monitor when switching netdevs.

This fixes a segfault in the "ofproto - mod-port" test.  The segfault
should not occur--there must be a bug in the netdev_monitor or possibly
the netdev_dummy implementation--but the netdev_monitor_remove() and
netdev_monitor_add() calls are definitely wanted here in any case to ensure
that the new netdev, not the old one, is what gets monitored.

13 years agobridge: Tolerate missing Port and Interface records for local port.
Ben Pfaff [Wed, 13 Apr 2011 18:10:44 +0000 (11:10 -0700)]
bridge: Tolerate missing Port and Interface records for local port.

Until now, ovs-vswitchd has been unable to configure IP addresses and
routes for bridges whose Bridge records lack a Port and an Interface
record for the bridge's local port (e.g. OFPP_LOCAL, the port with the
same name as the bridge itself).  When such a bridge was reconfigured,
ovs-vswitchd would output a log message that worried people.

This commit fixes the internal limitation that led to the message being
printed.

Bug #5385.

13 years agoofproto: Rework and fix bugs in port change detection.
Ben Pfaff [Wed, 20 Apr 2011 20:48:11 +0000 (13:48 -0700)]
ofproto: Rework and fix bugs in port change detection.

The OpenFlow port change detection code in update_port() is supposed to
send out an OFPT_PORT_STATUS message whenever an OpenFlow port is added or
removed or changes in some way.  This commit fixes a number of bugs that
have persisted until now.

First, if a port with a given name is removed from the datapath and a new
port with the same name but a different port number is added to the
datapath, then update_port() would report this as a port "modify" change.
Reporting this as a "modify" seems likely to confuse controllers, which
have no reason to realize that the old port was deleted and may not
understand why a port that has not been reported as added would be
modified.  (This scenario is more likely than before, because the Linux
datapath implementation no longer quickly reuses port numbers.  This
problem has actually been reported in testing.)  This commit fixes the
problem by changing update_port() to report a "delete" of the old port
followed by an "add" of the new port.

Second, suppose that a datapath initially has "eth1" on port 1 and "eth2"
on port 2.  Then, "eth1" gets removed and "eth2" is reassigned to port 1.
If update_port() is first passed "eth2", then the old implementation would
have sent out an OpenFlow "modify" notification instead of "delete"
followed by "add", which is the same as the previous scenario.  But as a
further wrinkle, it would have failed to remove "eth1", which meant that we
ended up with two "ofports" with port number 1!  This commit fixes this
problem too.

Reported-by: David Tsai <dtsai@nicira.com>
Bug #5466.
NIC-372.

13 years agoofproto: Consistently use netdev's name instead of ofp_phy_port name.
Ben Pfaff [Wed, 20 Apr 2011 20:03:45 +0000 (13:03 -0700)]
ofproto: Consistently use netdev's name instead of ofp_phy_port name.

There are at least two ways to get an ofport's name: from the netdev using
netdev_get_name() or from the ofp_phy_port's 'name' member.  Some code used
one, some used the other.  This switches all relevant code to use only
netdev_get_name(), because the 'name' member in ofp_phy_port is
fixed-length and thus a long name could be truncated.

This isn't a problem under Linux since the maximum length of a network
device's name under Linux is the same as the field width in ofp_phy_port.

13 years agosocket-util: Use portable solution for setting Unix socket permissions.
Ben Pfaff [Thu, 21 Apr 2011 16:22:39 +0000 (09:22 -0700)]
socket-util: Use portable solution for setting Unix socket permissions.

Requested-by: Jesse Gross <jesse@nicira.com>
13 years agobond: BM_STABLE consistent hashing.
Ethan Jackson [Wed, 20 Apr 2011 22:53:58 +0000 (15:53 -0700)]
bond: BM_STABLE consistent hashing.

This patch converts stable bonds from modulo n based hashing to
Highest Random Weight based hashing.  This hashing strategy only
redistributes 1/n_slaves traffic when a slave is enabled or
disabled.  It also turns out to have a vastly simpler
implementation.

13 years agobond: New flag "bond_revalidate".
Ethan Jackson [Wed, 20 Apr 2011 23:01:28 +0000 (16:01 -0700)]
bond: New flag "bond_revalidate".

Used in future patches.

13 years agoINSTALL.Linux: Mention that SSL options require building with SSL support.
Ben Pfaff [Fri, 8 Apr 2011 19:40:49 +0000 (12:40 -0700)]
INSTALL.Linux: Mention that SSL options require building with SSL support.

Reported-by: Aaron Rosen <arosen@clemson.edu>
13 years agobond: Revalidate no_slaves_tag when revalidating everything.
Ethan Jackson [Wed, 20 Apr 2011 01:02:53 +0000 (18:02 -0700)]
bond: Revalidate no_slaves_tag when revalidating everything.

13 years agobond: Give stable bonds one tag.
Ethan Jackson [Wed, 20 Apr 2011 00:19:25 +0000 (17:19 -0700)]
bond: Give stable bonds one tag.

Stable bonds require all flows to be revalidated when anything
changes.  Instead of giving each slave a tag, and ORing them
together.  This commit creates one tag representing the entire
bond.  This will cause less false positives when deciding which
flows to revalidate.

13 years agobridge: Avoid memory leak from RSPAN mirrors in bridge_destroy().
Ben Pfaff [Mon, 11 Apr 2011 18:22:39 +0000 (11:22 -0700)]
bridge: Avoid memory leak from RSPAN mirrors in bridge_destroy().

Mirrors that output to ports will be destroyed when their output ports are
destroyed, but mirrors that output to VLANs ("RSPAN" mirrors) don't get
automatically destroyed like this and we need to take care of them in a
separate loop.

13 years agobond: bond_stb_enable_slave() never triggered.
Ethan Jackson [Tue, 19 Apr 2011 21:11:23 +0000 (14:11 -0700)]
bond: bond_stb_enable_slave() never triggered.

bond_stb_enable_slave() depended on bond->stb_slaves being
nonnull.  However, bond_stb_enable_slave() is responsible for
initializing this parameter.  Thus none of it's logic ever ran.

13 years agolacp: Implement custom timing mode.
Ethan Jackson [Mon, 18 Apr 2011 19:22:12 +0000 (12:22 -0700)]
lacp: Implement custom timing mode.

With this patch, the LACP module may be manually configured to use
an arbitrary transmission rate set in the database.

13 years agolacp: Remove LACP_[FAST|SLOW]_TIME_RX macros.
Ethan Jackson [Mon, 18 Apr 2011 19:48:59 +0000 (12:48 -0700)]
lacp: Remove LACP_[FAST|SLOW]_TIME_RX macros.

The receive rate for a LACP packets is simply 3 times the
transmission rate.  It doesn't make sense to maintain separate
macros for these values especially since future patches will allow
arbitrary transmission rates.

13 years agolacp: Move LACP packet data to lacp header file.
Ethan Jackson [Mon, 18 Apr 2011 19:33:14 +0000 (12:33 -0700)]
lacp: Move LACP packet data to lacp header file.

13 years agoofp-util: Properly handle "tun_id"s in tun_id_from_cookie flows.
Ben Pfaff [Mon, 18 Apr 2011 17:11:43 +0000 (10:11 -0700)]
ofp-util: Properly handle "tun_id"s in tun_id_from_cookie flows.

Just setting the tun_id field isn't enough--it's also necessary to set
the tun_id_mask.  Otherwise the call to cls_rule_zero_wildcarded_fields()
at the end of ofputil_cls_rule_from_match() will zero out the tun_id again.

This was broken by commit 8368c090cab "Implement arbitrary bitwise masks
for tun_id field" back in January.  (This makes me wonder whether we can
drop support for tun_id_from_cookie now.)

Reported-by: Dan Wendlandt <dan@nicira.com>
13 years agosocket-util: Properly set socket permissions in make_unix_socket().
Ben Pfaff [Mon, 18 Apr 2011 18:24:50 +0000 (11:24 -0700)]
socket-util: Properly set socket permissions in make_unix_socket().

Under Linux, at least, bind and fchmod interact for Unix sockets in a way
that surprised me.  Calling fchmod() on a Unix socket successfully sets the
permissions for the socket's own inode.  But that has no effect on any
inode that has already been created in the file system by bind(), because
that inode is not the same as the one for the Unix socket itself.

However, if you bind() *after* calling fchmod(), then the bind() takes the
permissions for the new inode from the Unix socket inode, which has the
desired effect.

This also adds a more portable fallback for non-Linux systems.

Reported-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
13 years agobridge: LACP port ID and system ID in database.
Ethan Jackson [Sat, 16 Apr 2011 00:03:37 +0000 (17:03 -0700)]
bridge: LACP port ID and system ID in database.

Extremely advanced users may want fine grained control over the
LACP port and system IDs of a bond.  This would be extremely
unusual for the average user, so this patch puts the configuration
parameters in other_config of the relevant tables.

13 years agolacp: New "strict" lacp mode.
Ethan Jackson [Mon, 18 Apr 2011 22:13:34 +0000 (15:13 -0700)]
lacp: New "strict" lacp mode.

When LACP negotiations are unsuccessful, OVS falls back to standard
balance-slb bonding.  In some cases, users may want to require
successful LACP negotiations for any slaves to be enabled at all.
This patch implements a new "strict" mode which disables all slaves
when LACP negotiations are unsuccessful.

13 years agolacp: Update attached status more often.
Ethan Jackson [Mon, 18 Apr 2011 22:32:30 +0000 (15:32 -0700)]
lacp: Update attached status more often.

The attached status of slaves should be updated when certain global
configuration settings change, or when a slave is destroyed.

13 years agovlog: Fix VLOG and VLOG_RL macros' treatment of LEVEL argument.
Ben Pfaff [Thu, 7 Apr 2011 21:39:36 +0000 (14:39 -0700)]
vlog: Fix VLOG and VLOG_RL macros' treatment of LEVEL argument.

These macros expanded the LEVEL argument without protecting it with
parentheses, which meant that an argument like 'cond ? VLL_DBG : VLL_WARN'
did not have the desired effect (and caused a GCC warning).

This commit fixes the problem and avoids expanding LEVEL more than once,
too.

13 years agobridge: Fix VLAN selection mirroring logic.
Ben Pfaff [Fri, 8 Apr 2011 19:52:23 +0000 (12:52 -0700)]
bridge: Fix VLAN selection mirroring logic.

The logic here did not make sense.  A packet arriving on a port is mirrored
if the port is a mirroring source port AND (not OR) the packet is in one of
the VLANs that is mirrored.

This test has been here since the mirroring code was introduced.  It seems
to me that it was never correct.

13 years agobridge: Reintroduce log message that was lost (and wrong).
Ben Pfaff [Tue, 29 Mar 2011 18:16:31 +0000 (11:16 -0700)]
bridge: Reintroduce log message that was lost (and wrong).

Setting the 'mac' in the Interface record for a bridge's local port has
always been ineffective, but the log message was suppressed because of a
check at too high of a level.  This commit fixes the problem.  It also
fixes the wording of the log message, which has been obsolete since the
introduction of the database.

Finally, it seems better to check for the local port before checking for a
multicast address, so this reverses the order of the checks.

13 years agodaemon: Reduce log level of "pid file is stale" message.
Ben Pfaff [Tue, 5 Apr 2011 19:17:08 +0000 (12:17 -0700)]
daemon: Reduce log level of "pid file is stale" message.

This message will appear repeatedly when ovs-vswitchd is running, if there
is any stale pidfile in /var/run/openvswitch, because ovs-vswitchd reads
all of the pidfiles in that directory periodically to update statistics.

13 years agobridge: Initialize mirrors' uuid member.
Ben Pfaff [Tue, 5 Apr 2011 21:17:55 +0000 (14:17 -0700)]
bridge: Initialize mirrors' uuid member.

Otherwise mirrors get destroyed and re-created on every reconfiguration.

13 years agoofproto: Avoid memory leak in classifier on destruction.
Ben Pfaff [Tue, 5 Apr 2011 22:58:06 +0000 (15:58 -0700)]
ofproto: Avoid memory leak in classifier on destruction.

ofproto_flush_flows() flushes the flow table but then it reintroduces flows
required by fail-open or in-band.  These are then leaked when the
classifier is destroyed a little later.

This fixes the problem by not reintroducing these flows when ofproto is
being destroyed.

13 years agobond: Fix ugly warnings at slave registration.
Ethan Jackson [Mon, 18 Apr 2011 23:17:46 +0000 (16:17 -0700)]
bond: Fix ugly warnings at slave registration.

Before this patch, when a slave was registered for this first time
the following warning would display.

interface (null): enabled

This is because the slave was enabled before having its name
configured.

13 years agobond: Properly indent appctl output.
Ethan Jackson [Mon, 18 Apr 2011 23:14:58 +0000 (16:14 -0700)]
bond: Properly indent appctl output.

13 years agobridge: Report lacp_slave_is_current() in the database.
Ethan Jackson [Wed, 13 Apr 2011 23:06:50 +0000 (16:06 -0700)]
bridge: Report lacp_slave_is_current() in the database.

Whether or not a given slave is current with its LACP protocol
messages can be very interesting to a controller.  If an interface
is not current, it usually indicates a connectivity problem or
misconfiguration of some sort.

13 years agobridge: Generalize CFM rate limiter.
Ethan Jackson [Fri, 15 Apr 2011 19:57:30 +0000 (12:57 -0700)]
bridge: Generalize CFM rate limiter.

In future patches, lacp status will need to be written to the
database in a rate limited manner.  It doesn't make sense to run
two parallel rate limiters.  This patch renames the CFM rate
limiter to something more generic.

13 years agolacp: New function lacp_slave_is_current().
Ethan Jackson [Wed, 13 Apr 2011 22:44:37 +0000 (15:44 -0700)]
lacp: New function lacp_slave_is_current().

Used in future patches.

13 years agobridge: Properly test for out-of-range values.
Ben Pfaff [Fri, 15 Apr 2011 16:40:50 +0000 (09:40 -0700)]
bridge: Properly test for out-of-range values.

This code was trying to check for priorities greater than UINT16_MAX and
reset them, but it assigned the value to a uint16_t before it checked it,
which of course hid the problem.

Fixes the following GCC warning:

vswitchd/bridge.c:3034: warning: comparison is always false due to limited
range of data type

Reported-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
13 years agoAvoid warnings about comparisons that are always true.
Ben Pfaff [Fri, 15 Apr 2011 16:39:08 +0000 (09:39 -0700)]
Avoid warnings about comparisons that are always true.

The range of "enum" types varies from one ABI to another.  If the enums
being tested in these functions happen to be 16 bits wide, then GCC may
issue a warning because, in such a case, the comparison is always true.

Using an int instead of a uint16_t avoids that particular problem and
should suppress the warning.

Fixes the following reported warnings:

lib/ofp-print.c:240: warning: comparison is always true due to limited
range of data type
lib/ofp-util.c:1973: warning: comparison is always false due to limited
range of data type

Reported-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
13 years agoFix calls to ctype functions.
Ben Pfaff [Fri, 15 Apr 2011 16:31:36 +0000 (09:31 -0700)]
Fix calls to ctype functions.

The ctype functions often need casts to be fully C standards compliant.
Here's the full explanation that I used to post to comp.lang.c from time
to time when the issue came up:

    With the to*() and is*() functions, you should be careful to cast
    `char' arguments to `unsigned char' before calling them.  Type `char'
    may be signed or unsigned, depending on your compiler or its
    configuration.  If `char' is signed, then some characters have
    negative values; however, the arguments to is*() and to*() functions
    must be nonnegative (or EOF).  Casting to `unsigned char' fixes this
    problem by forcing the character to the corresponding positive value.

This fixes the following warnings from some version of GCC:

lib/ofp-parse.c:828: warning: array subscript has type 'char'
lib/ofp-print.c:617: warning: array subscript has type 'char'

Reported-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
13 years agobridge: Destroy bond when port is destroyed.
Ethan Jackson [Fri, 15 Apr 2011 18:06:02 +0000 (11:06 -0700)]
bridge: Destroy bond when port is destroyed.

13 years agobond: Completely pull LACP module out of bond.
Ethan Jackson [Thu, 14 Apr 2011 23:50:26 +0000 (16:50 -0700)]
bond: Completely pull LACP module out of bond.

The bonding code only needs to know whether a given slave may be
enabled, and whether LACP has been negotiated on the bond.  Instead
of passing in the LACP handle and letting the bond query this
information.  This patch passes in the information directly.

13 years agobond: Create new 'stable_id' parameter.
Ethan Jackson [Fri, 15 Apr 2011 00:37:29 +0000 (17:37 -0700)]
bond: Create new 'stable_id' parameter.

For BM_STABLE bonds, instead of choosing the sort key in the
qsort() comparator, this patch makes it a configuration setting of
each slave.  This will help wrest LACP out of the bonding code
further in future patches.

13 years agobond: Give bridge control over LACP module.
Ethan Jackson [Thu, 14 Apr 2011 00:58:26 +0000 (17:58 -0700)]
bond: Give bridge control over LACP module.

Before this patch, the bonding code had taken over responsibility
for running the LACP module.  However, the bonding code only needs
the LACP module for some basic status queries.  LACP and bonding
are actually logically parallel modules and do not really have a
parent child relationship.  Furthermore, we need to be able to run
LACP on non-bonded interfaces which the existing approach
prevented.  This patch gives control of the LACP module back to the
bridge.

13 years agolacp: Remove enabled flag.
Ethan Jackson [Thu, 14 Apr 2011 22:24:18 +0000 (15:24 -0700)]
lacp: Remove enabled flag.

The enabled flag in the LACP module was only used to set the
Collecting and Distributing flags in the LACP protocol.  It was
intended to be set by the bonding code to mimic its enabled flag.

The spec is relatively vague on the precise meaning of these flags,
and most implementations do something completely different with
them.  For these reasons, it seems acceptable to remove the enabled
flag for the sake of simplicity.  A slave is now Collecting and
Distributing if it is attached, or LACP couldn't be negotiated.

13 years agovswitchd: Document how to disable inactivity probes.
Ben Pfaff [Thu, 14 Apr 2011 17:22:21 +0000 (10:22 -0700)]
vswitchd: Document how to disable inactivity probes.

This has always been implemented but it was not documented until now.

Reported-by: Alex Yip <alex@nicira.com>
13 years agobond: New bonding mode "stable".
Ethan Jackson [Tue, 12 Apr 2011 21:15:46 +0000 (14:15 -0700)]
bond: New bonding mode "stable".

Stable bonds attempt to assign a given flow to the same slave
consistently.

13 years agobond: New function bond_is_balanced().
Ethan Jackson [Tue, 12 Apr 2011 20:39:32 +0000 (13:39 -0700)]
bond: New function bond_is_balanced().

As new bond modes are added, it will be nice to have the logic
indicating whether or not a given bond mode requires rebalancing in
one place.

13 years agolacp: New function lacp_slave_get_port_id().
Ethan Jackson [Wed, 13 Apr 2011 21:55:19 +0000 (14:55 -0700)]
lacp: New function lacp_slave_get_port_id().

Will be used in future commits.

13 years agobond: Use bond_enable_slave at slave registration.
Ethan Jackson [Tue, 12 Apr 2011 22:15:32 +0000 (15:15 -0700)]
bond: Use bond_enable_slave at slave registration.

Slave registration should go through the normal slave enabling
facilities instead of doing it by hand.  Before this patch, newly
created slaves would have no tag associated with them.
Furthermore, any further changes to how slaves are enabled would
not be picked up by the registration code.

13 years agobond: Reset bond_entry's during massive flow revalidations.
Ethan Jackson [Wed, 13 Apr 2011 20:56:37 +0000 (13:56 -0700)]
bond: Reset bond_entry's during massive flow revalidations.

When all flows in a bond are revalidated, stale bond_entry's can
cause incorrect load balancing.  These issues will naturally
resolve themselves overtime.  However, it's better to deal with
them immediately.

13 years agobond: Revalidate flows when bond_is_tcp_hash() changes;
Ethan Jackson [Wed, 13 Apr 2011 01:28:04 +0000 (18:28 -0700)]
bond: Revalidate flows when bond_is_tcp_hash() changes;

If LACP causes the return of bond_is_tcp_hash to change for
whatever reason, all flows should be revalidated because they will
have a different hash result.

13 years agobond: Reconfigure flows when bond mode changes.
Ethan Jackson [Wed, 13 Apr 2011 00:53:24 +0000 (17:53 -0700)]
bond: Reconfigure flows when bond mode changes.

Changes in the bonding mode can cause drastic changes in flow
assignments to slaves.  This commit causes all flows in a bridge
to be revalidated when bond_reconfigure() changes its bonding mode.
This approach is a bit aggressive, but bond reconfiguration
shouldn't happen often.

13 years agoconfigure: Add option --enable-Werror to add -Werror to CFLAGS.
Ben Pfaff [Tue, 12 Apr 2011 18:43:11 +0000 (11:43 -0700)]
configure: Add option --enable-Werror to add -Werror to CFLAGS.

-Werror is useful for development, but it screws up configure because it's
impossible to guess what new warnings compilers will add in the future.
This commit adds a new configure option to add CFLAGS after the configure
checks are done.

The use of AC_CONFIG_COMMANDS_PRE is based on Eric Blake's suggestion on
the autoconf mailing list: "AC_CONFIG_COMMANDS_PRE probably fits the bill
as the ideal macro to use for guaranteeing that you inject your shell code
at the last possible moment."

Requested-by: Andrew Evans <aevans@nicira.com>
13 years agoxenserver: Fix typo in RPM install message.
Ethan Jackson [Tue, 12 Apr 2011 20:20:13 +0000 (13:20 -0700)]
xenserver: Fix typo in RPM install message.

13 years agoxenserver: Don't openvswitch-xapi-update in bridge mode.
Ethan Jackson [Tue, 12 Apr 2011 01:33:06 +0000 (18:33 -0700)]
xenserver: Don't openvswitch-xapi-update in bridge mode.

This commit causes the init scripts not to call the
openvswitch-cfg-update plugin when in bridge mode.

13 years agoxenserver: Warn when upgrading OVS on a bridged system.
Ethan Jackson [Tue, 12 Apr 2011 01:29:02 +0000 (18:29 -0700)]
xenserver:  Warn when upgrading OVS on a bridged system.

13 years agoovsdb-idl: Suppress "delete" operations for garbage-collected tables.
Ben Pfaff [Tue, 12 Apr 2011 18:31:58 +0000 (11:31 -0700)]
ovsdb-idl: Suppress "delete" operations for garbage-collected tables.

Deciding what delete operations to issue on garbage-collected tables has
been a bit of a difficult issue for ovs-vsctl.  When garbage collection was
introduced in commit c5f341a "ovsdb: Implement garbage collection",
ovs-vsctl did not issue any deletions for these tables at all.  As a side
effect, ovs-vsctl did not notice that records were going to be deleted.
That meant that when multiple commands were issued in one ovs-vsctl run,
ovs-vsctl could get confused by apparent duplicate records that did not
in fact exist.  Commit 28a14bf "ovs-vsctl: Back out garbage collection
changes" fixed the problem by putting all of the explicit deletions back
into ovs-vsctl.

However, adding these explicit deletions had the price that it then became
(again) impossible to use ovs-vsctl commands to delete duplicates, for
example to use "ovs-vsctl del-br" to delete a bridge that points to the
same Port records that some other Bridge record also does.  This commit
makes that possible again, by implementing a compromise:

    * Internally, ovs-vsctl deletes the records that it believes should be
      deleted.

    * ovsdb-idl suppresses the deletions when it makes the RPC call into
      the database server.

Bug #5358.
Reported-by: Henrik Amren <henrik@nicira.com>
13 years agotests: Unit test autopath via ovs-ofctl.
Ethan Jackson [Mon, 11 Apr 2011 23:17:39 +0000 (16:17 -0700)]
tests: Unit test autopath via ovs-ofctl.

This patch adds test designed to verify the correctness of the
parsing function introduced with the autopath action.

13 years agopcap: Silence warnings about fwrite(3) return value being ignored.
Andrew Evans [Tue, 12 Apr 2011 17:40:15 +0000 (10:40 -0700)]
pcap: Silence warnings about fwrite(3) return value being ignored.

13 years agodebian: Do not call obsolete command "ovs-ofctl status" in ovs-bugtool.
Ben Pfaff [Tue, 12 Apr 2011 17:02:40 +0000 (10:02 -0700)]
debian: Do not call obsolete command "ovs-ofctl status" in ovs-bugtool.

This command was removed in commit 9b45d7f5d (ofproto: Get rid of archaic
"switch status" OpenFlow extension) but I didn't notice that ovs-bugtool
uses that command and forgot to remove it at the time.

Bug #5360.
Reported-by: Michael Mao <mmao@nicira.com>
Reported-by: Keith Amidon <keith@nicira.com>
13 years agoRelease Open vSwitch 1.1.0
Justin Pettit [Wed, 6 Apr 2011 05:17:03 +0000 (22:17 -0700)]
Release Open vSwitch 1.1.0

13 years agoautopath: Create the autopath action.
Ethan Jackson [Tue, 5 Apr 2011 19:37:52 +0000 (12:37 -0700)]
autopath: Create the autopath action.

The newly created autopath action will be the way OpenFlow
interacts with the existing bonding infrastructure.

13 years agodpif-linux: Avoid logging error on ENOENT in dpif_linux_is_internal_device().
Ben Pfaff [Fri, 8 Apr 2011 23:38:42 +0000 (16:38 -0700)]
dpif-linux: Avoid logging error on ENOENT in dpif_linux_is_internal_device().

ENOENT can be returned if the kernel module isn't loaded.  If that's the
case then we've already logged that and there's no point in logging it
again.

13 years agodpif-linux: Avoid segfault on netdev_get_stats() without kernel module.
Ben Pfaff [Fri, 8 Apr 2011 23:37:22 +0000 (16:37 -0700)]
dpif-linux: Avoid segfault on netdev_get_stats() without kernel module.

netdev_linux_get_stats() calls into netdev_vport_get_stats(), which in
turn attempts a transaction on genl_sock.  If the kernel module isn't
loaded, then genl_sock won't be there, and in any case there's nothing that
guarantees that it's been initialized yet.

This fixes the problem by ensuring that dpif_linux was initialized properly
before attempting a transaction on genl_sock.

Reported-by: Aaron Rosen <arosen@clemson.edu>
13 years agonetdev-linux: Fix netdev_send() to tap device.
Ben Pfaff [Fri, 8 Apr 2011 23:34:17 +0000 (16:34 -0700)]
netdev-linux: Fix netdev_send() to tap device.

Commit 76c308b50d3 "netdev-linux: Support 'send' for netdevs opened with
NETDEV_ETH_TYPE_NONE" broke sending packets to tap devices.  Sending a
packet to a tap device with an AF_PACKET socket causes that packet to be
looped back to be received on the tap device again, which obviously isn't
useful.

13 years agonetdev-linux: Fix blocking while sending packets.
Ben Pfaff [Fri, 8 Apr 2011 21:23:13 +0000 (14:23 -0700)]
netdev-linux: Fix blocking while sending packets.

The AF_PACKET socket needs to be in nonblocking mode or trying to send
a packet can take a long time.

13 years agonetdev-linux: Avoid "cleverness" in swap_uint64().
Ben Pfaff [Fri, 8 Apr 2011 23:44:31 +0000 (16:44 -0700)]
netdev-linux: Avoid "cleverness" in swap_uint64().

Obviously correct code is easier on everyone.  As the C FAQ says:

20.15c: How can I swap two values without using a temporary?

A: The standard hoary old assembly language programmer's trick is:

a ^= b;
b ^= a;
a ^= b;

But this sort of code has little place in modern, HLL
programming.  Temporary variables are essentially free,
and the idiomatic code using three assignments, namely

int t = a;
a = b;
b = t;

is not only clearer to the human reader, it is more likely to be
recognized by the compiler and turned into the most-efficient
code (e.g. using a swap instruction, if available).  The latter
code is obviously also amenable to use with pointers and
floating-point values, unlike the XOR trick.  See also questions
3.3b and 10.3.

13 years agobridge: Monitor fewer OVSDB columns.
Ben Pfaff [Wed, 30 Mar 2011 00:05:52 +0000 (17:05 -0700)]
bridge: Monitor fewer OVSDB columns.

By omitting columns that ovs-vswitchd does not use at all, and omitting
alerts for columns that ovs-vswitchd writes to but does not read, we can
save CPU time and bandwidth.

13 years agoovsdb-idl: Fix atomicity of writes that don't change a column's value.
Ben Pfaff [Fri, 1 Apr 2011 17:50:52 +0000 (10:50 -0700)]
ovsdb-idl: Fix atomicity of writes that don't change a column's value.

The existing ovsdb_idl_txn_commit() drops any writes that don't change a
column's value from what was last reported by the database.  But this isn't
a valid optimization, because it breaks the atomicity of transactions.
Suppose columns A and B initially have values 1 and 2.  Client 1 writes
value 1 to both columns in one transaction.  Client 2 writes value 2 to
both columns in another transaction.  The only possible valid results for
any serial ordering of transactions are 1,1 or 2,2.  But if both clients
drop writes to columns that they have not modified, then 2,1 also becomes
possible (because client 1 just writes to B and client 2 just writes to A).

However, for write-only columns we can optimize this out because the IDL
can assume it is the only client writing to a column.

Found by inspection.

13 years agodatapath: Update netdev_frame_hook() for 2.6.39 rx handler API change.
Andrew Evans [Thu, 7 Apr 2011 19:43:18 +0000 (19:43 +0000)]
datapath: Update netdev_frame_hook() for 2.6.39 rx handler API change.

netdev_rx_handler_register() changed the type of the skb argument to the
callback function as well as the return type. Special-case
netdev_frame_hook() to do the right thing on 2.6.39 and later.

Signed-off-by: Andrew Evans <aevans@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agocfm: Fix broken fault logic.
Ethan Jackson [Thu, 7 Apr 2011 00:23:40 +0000 (17:23 -0700)]
cfm: Fix broken fault logic.

If the last receive time for a remote MP was before the last fault
check, the CFM code would not declare a fault.  This is, of course,
exactly the wrong response.

Bug #5303.

13 years agobridge: Run once before configuring CFM.
Ethan Jackson [Wed, 6 Apr 2011 23:59:22 +0000 (16:59 -0700)]
bridge: Run once before configuring CFM.

CFM configuration requires the ofproto_run function to have been
executed at least once in order to guarantee that the relevant
ports exist.

Bug #5303.

13 years agoUpdate top-level documentation to bring it up to date with latest features.
Ben Pfaff [Wed, 6 Apr 2011 16:15:58 +0000 (09:15 -0700)]
Update top-level documentation to bring it up to date with latest features.

13 years agodpif-linux: Choose port numbers more prudently.
Ethan Jackson [Mon, 4 Apr 2011 23:55:34 +0000 (16:55 -0700)]
dpif-linux: Choose port numbers more prudently.

Before this patch the kernel chose the lowest available number for
newly created datapath ports.  This patch moves the port number
choosing responsibility to user space, and implements a least
recently used port number queue in an attempt to avoid reuse.

Bug #2140.

13 years agobond: Choose slaves randomly.
Ethan Jackson [Sat, 2 Apr 2011 00:37:56 +0000 (17:37 -0700)]
bond: Choose slaves randomly.

When the bonding library encounters a flow it hasn't seen before,
it assigns it to the active slave and waits for load balancing to
move it to a more appropriate place.  This commit causes it to
first attempt a random slave.

13 years agodaemon: Avoid races on pidfile creation.
Ben Pfaff [Mon, 4 Apr 2011 17:59:19 +0000 (10:59 -0700)]
daemon: Avoid races on pidfile creation.

Until now, if two copies of one OVS daemon started up at the same time,
then due to races in pidfile creation it was possible for both of them to
start successfully, instead of just one.  This was made worse when a
previous copy of the daemon had died abruptly, leaving a stale pidfile.

This commit implements a new pidfile creation and removal protocol that I
believe closes these races.  Now, a pidfile is asserted with "link" instead
of "rename", which prevents the race on creation, and a stale pidfile may
only be deleted by a process after it has taken a lock on it.

This may solve mysterious problems seen occasionally on vswitch restart.
I'm still puzzled by these problems, however, because I don't see anything
in our tests cases that would actually cause two copies of a daemon to
start at the same time, which as far as I can see is a necessary
precondition for the problem.

13 years agodaemon: Integrate checking for an existing pidfile into daemonize_start().
Ben Pfaff [Thu, 31 Mar 2011 16:44:30 +0000 (09:44 -0700)]
daemon: Integrate checking for an existing pidfile into daemonize_start().

Until now, it has been the responsibility of an individual daemon to call
die_if_already_running() at an appropriate time.  A long time ago, this
had to happen *before* daemonizing, because once the process daemonized
itself there was no way to report failure to the process that originally
started the daemon.  With the introduction of daemonize_start(), this is
now possible, but we haven't been taking advantage of it.

Therefore, this commit integrates the die_if_already_running() call into
daemonize_start() and deletes the calls to it from individual daemons.

13 years agodaemon: Tolerate EINTR in fork_and_wait_for_startup().
Ben Pfaff [Thu, 31 Mar 2011 16:36:10 +0000 (09:36 -0700)]
daemon: Tolerate EINTR in fork_and_wait_for_startup().

It seems possible that a signal coming in at the wrong time could confuse
this code.  It's always best to loop on EINTR.

13 years agoLog anything that could prevent a daemon from starting.
Ben Pfaff [Thu, 31 Mar 2011 23:23:50 +0000 (16:23 -0700)]
Log anything that could prevent a daemon from starting.

If a daemon doesn't start, we need to know why.  Being able to
consistently consult the log to find out is helpful.

13 years agoutil: New function ovs_fatal_valist().
Ben Pfaff [Thu, 31 Mar 2011 21:50:58 +0000 (14:50 -0700)]
util: New function ovs_fatal_valist().

This commit adds a few initial users but more are coming up.

13 years agosignals: New function signal_name().
Ben Pfaff [Fri, 1 Apr 2011 17:22:51 +0000 (10:22 -0700)]
signals: New function signal_name().

This will acquire a new user in an upcoming commit.

13 years agotype-props: New macro for estimating length of a decimal integer.
Ben Pfaff [Fri, 1 Apr 2011 17:20:17 +0000 (10:20 -0700)]
type-props: New macro for estimating length of a decimal integer.

13 years agoAdd a few more users for ovs_retval_to_string().
Ben Pfaff [Thu, 31 Mar 2011 21:50:20 +0000 (14:50 -0700)]
Add a few more users for ovs_retval_to_string().

13 years agovlog: Use PRINTF_FORMAT macro from compiler.h.
Ben Pfaff [Thu, 31 Mar 2011 21:58:37 +0000 (14:58 -0700)]
vlog: Use PRINTF_FORMAT macro from compiler.h.

PRINTF_FORMAT is more portable than raw __attribute__.  We use it
elsewhere, I don't know why it wasn't used here.

13 years agostream-ssl: Use out_of_memory() to abort due to lack of memory.
Ben Pfaff [Thu, 31 Mar 2011 21:52:36 +0000 (14:52 -0700)]
stream-ssl: Use out_of_memory() to abort due to lack of memory.

This matches what xmalloc() does.  It will be handled better by a monitor
process (created with --monitor), which will restart the child instead of
exiting.

13 years agoxenserver: Fix up iface-id after it changes or disappears too.
Ben Pfaff [Fri, 1 Apr 2011 20:47:51 +0000 (13:47 -0700)]
xenserver: Fix up iface-id after it changes or disappears too.

ovs-xapi-sync is supposed to always keep external-ids:iface-id up to date,
but in fact it would only set it when an interface initially appeared.  If
the interface quickly disappeared and reappeared, then it failed to notice
that iface-id had changed or disappeared.  This happens in practice on
Citrix XenServer, where VM "tap" devices often disappear and then reappear
almost immediately during VM boot.  This commit fixes the problem.

This also fixes the similar problem for external-ids:bridge-id in Bridge
records.  Bridges aren't ordinarily destroyed and re-created quickly, so
this problem might never have manifested in practice for bridges.

Many thanks to Reid Price <reid@nicira.com> for identifying the problem
and supplying an initial fix.

Bug #5239.
Reported-by: Henrik Amren <henrik@nicira.com>
13 years agobridge: Avoid partitioning the dst set.
Ben Pfaff [Thu, 24 Mar 2011 19:46:08 +0000 (12:46 -0700)]
bridge: Avoid partitioning the dst set.

Scanning the dsts twice seems may be a little more efficient than
partitioning it, and it now seems more straightforward to me.

13 years agobridge: Separate mirroring logic from forwarding logic.
Ben Pfaff [Thu, 24 Mar 2011 19:51:14 +0000 (12:51 -0700)]
bridge: Separate mirroring logic from forwarding logic.

In my opinion this is easier to understand than the way that these
two logically separate steps were previously entangled.

13 years agobridge: Change "struct dst" from containing a dp_ifidx to a struct iface *.
Ben Pfaff [Thu, 24 Mar 2011 19:30:51 +0000 (12:30 -0700)]
bridge: Change "struct dst" from containing a dp_ifidx to a struct iface *.

The following commit will need to iterate over a set of "struct
dst"s, obtaining the iface for each.  It could look them up using
the hash table that indexes over dp_ifidx, but it's easier if we
simply store the iface pointer directly.

13 years agobridge: Get rid of 'n_ifaces' member of struct port.
Ben Pfaff [Thu, 24 Mar 2011 17:29:36 +0000 (10:29 -0700)]
bridge: Get rid of 'n_ifaces' member of struct port.

If it doesn't exist then it can't have the wrong value.

13 years agobridge: Break bonding implementation out into library.
Ben Pfaff [Wed, 30 Mar 2011 18:03:16 +0000 (11:03 -0700)]
bridge: Break bonding implementation out into library.

This removes over 1000 lines of code from bridge.c and will make it
easier to moving the bonding implementation into ofproto as part of
future development.

13 years agobridge: Simplify and clean up bond slave enable/disable.
Ben Pfaff [Wed, 23 Mar 2011 17:47:15 +0000 (10:47 -0700)]
bridge: Simplify and clean up bond slave enable/disable.

The code that enables and disables bond slaves was a bit of a mess:

    * Disabling a slave could recursively enable a different slave.

    * Processing a flow could enable a slave.

This commit gets rid of both of those properties, which made it difficult
to reason about the code paths along which slaves would be enabled and
disabled.

Bug #5121.

13 years agobridge: Drop obsolete comment.
Ben Pfaff [Thu, 24 Mar 2011 19:46:26 +0000 (12:46 -0700)]
bridge: Drop obsolete comment.

It's quite clear that we don't support double tagging now.

13 years agobridge: Improve comment.
Ben Pfaff [Thu, 24 Mar 2011 18:11:32 +0000 (11:11 -0700)]
bridge: Improve comment.

13 years agobridge: Change Ethernet address array from 8 bytes to ETH_ADDR_LEN bytes.
Ben Pfaff [Tue, 29 Mar 2011 18:33:10 +0000 (11:33 -0700)]
bridge: Change Ethernet address array from 8 bytes to ETH_ADDR_LEN bytes.

I don't know why this was declared as 8 bytes long but I only see 6
actually in use, as one would expect of an Ethernet address.

13 years agobridge: Avoid redundant dpif_flow_flush().
Ben Pfaff [Tue, 29 Mar 2011 17:16:43 +0000 (10:16 -0700)]
bridge: Avoid redundant dpif_flow_flush().

ofproto_create() also calls dpif_flow_flush() very soon afterward.  This
seems more clearly in ofproto's domain anyhow.

13 years agolacp: Encapsulate configuration into new structs.
Ben Pfaff [Mon, 21 Mar 2011 21:30:33 +0000 (14:30 -0700)]
lacp: Encapsulate configuration into new structs.

This makes it easier to pass configuration between modules.

13 years agobridge: Drop LACP configuration members from struct iface and struct port.
Ben Pfaff [Mon, 21 Mar 2011 19:49:44 +0000 (12:49 -0700)]
bridge: Drop LACP configuration members from struct iface and struct port.

There's no reason that I can see to maintain this information in struct
port and struct iface.  It's redundant, since the lacp implementation
maintains the same information.

13 years agolacp: Remove unneeded forward references from header file.
Ben Pfaff [Mon, 21 Mar 2011 20:15:53 +0000 (13:15 -0700)]
lacp: Remove unneeded forward references from header file.