openvswitch
14 years agoAdd Nicira extension to OpenFlow for dropping spoofed ARP packets.
Ben Pfaff [Tue, 24 Aug 2010 23:00:27 +0000 (16:00 -0700)]
Add Nicira extension to OpenFlow for dropping spoofed ARP packets.

"ARP spoofing" is when a host claims an incorrect association between an
IP address and a MAC address for deceptive purposes.  OpenFlow by itself
can prevent a host from sending out ARP replies from an incorrect MAC
address in the Ethernet L2 header, but it cannot control the MAC addresses
inside the ARP L3 packet.  This commit adds a new action that can be used
to drop these spoofed packets.

CC: Paul Ingram <paul@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agovswitchd: Fix 100% CPU usage with bonds and --fake-proc-net.
Ben Pfaff [Thu, 26 Aug 2010 16:56:25 +0000 (09:56 -0700)]
vswitchd: Fix 100% CPU usage with bonds and --fake-proc-net.

The current date in milliseconds since the epoch is ~1,282,841,552,000,
which is greater than LONG_MAX of 4,294,967,295 on 32-bit systems, so
no matter what was stored into bond_next_fake_iface_update, it would always
appear to be expired.  It really needs to be a 64-bit number.  (This was
just a typo really.)

Since XenServer 5.5 requires --fake-proc-net, this probably fixes an
important bug there.

Reported-by: Luiz Henrique Ozaki <luiz.ozaki@gmail.com>
14 years agoxenserver: Add type-checking to monitor-external-ids script.
Ben Pfaff [Thu, 26 Aug 2010 16:39:54 +0000 (09:39 -0700)]
xenserver: Add type-checking to monitor-external-ids script.

Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agoxenserver: Block until change in poll loop to avoid 100% CPU consumption.
Ben Pfaff [Thu, 26 Aug 2010 16:38:52 +0000 (09:38 -0700)]
xenserver: Block until change in poll loop to avoid 100% CPU consumption.

Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agovswitchd: Fix "updelay" configuration for bonds.
Ben Pfaff [Thu, 26 Aug 2010 17:06:36 +0000 (10:06 -0700)]
vswitchd: Fix "updelay" configuration for bonds.

Reported-by: Michael Mao <mmao@nicira.com>
Bug #3521.

14 years agodatapath: Free up flow_extract() return value for reporting errors.
Ben Pfaff [Thu, 12 Aug 2010 22:12:28 +0000 (15:12 -0700)]
datapath: Free up flow_extract() return value for reporting errors.

flow_extract() can fail due to memory allocation errors in pskb_may_pull().
Currently it doesn't return those properly, instead just reporting a bogus
flow to the caller.  But its return value is currently in use for reporting
whether the packet was an IPv4 fragment.  This commit switches to reporting
that in the skb itself so that the return value can be reused to report
errors.

Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agodatapath: Remove skb->len >= ETH_HLEN check from flow_extract().
Ben Pfaff [Fri, 13 Aug 2010 17:47:44 +0000 (10:47 -0700)]
datapath: Remove skb->len >= ETH_HLEN check from flow_extract().

The callers ensure that this is already the case.

Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agodatapath: Use 'bool' instead of 'int' where appropriate.
Ben Pfaff [Fri, 13 Aug 2010 17:18:28 +0000 (10:18 -0700)]
datapath: Use 'bool' instead of 'int' where appropriate.

'bool' is better modern kernel style.

Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agodatapath: Use min() instead of open-coding it.
Ben Pfaff [Fri, 13 Aug 2010 16:43:04 +0000 (09:43 -0700)]
datapath: Use min() instead of open-coding it.

Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agoxenserver: Add monitor-external-ids daemon
Justin Pettit [Tue, 24 Aug 2010 21:50:06 +0000 (14:50 -0700)]
xenserver: Add monitor-external-ids daemon

The monitor-external-ids daemon monitors the external_ids columns of the
Bridge and Interface OVSDB tables.  Its primary responsibility is to
set the "bridge-id" and "iface-id" keys in the Bridge and Interface
tables, respectively.  It also looks for the use of "network-uuids" in
the Bridge table and duplicates its value to the preferred
"xs-network-uuids".

Signed-off-by: Justin Pettit <jpettit@nicira.com>
14 years agoxenserver: Prepend XenServer external ids with "xs-"
Justin Pettit [Mon, 9 Aug 2010 22:07:32 +0000 (15:07 -0700)]
xenserver: Prepend XenServer external ids with "xs-"

Signed-off-by: Justin Pettit <jpettit@nicira.com>
14 years agoxenserver: Report the system type and version as external ids
Justin Pettit [Thu, 5 Aug 2010 01:52:17 +0000 (18:52 -0700)]
xenserver: Report the system type and version as external ids

The configuration schema defines the system-type and system-version
external-ids for the Open_vSwitch table.  This commit adds support for
reporting them on XenServer.

Signed-off-by: Justin Pettit <jpettit@nicira.com>
14 years agovswitch: Break out XenServer-specific external ids
Justin Pettit [Wed, 4 Aug 2010 06:00:04 +0000 (23:00 -0700)]
vswitch: Break out XenServer-specific external ids

As we move to new platforms, not all external identifiers will be
universally unique, but the "-uuid" suffix seemingly prevents their use.
Create new identifiers without the "-uuid" suffix.  Change the existing
XenServer-specific external identifiers to contain a "xs-" prefix.  This
also allows a XenServer integrator to define identifiers different from
the XenServer UUIDs, but still leave them in the config database to be
used by other programs.

14 years agovswitch: Add port status column to Port table
Justin Pettit [Wed, 4 Aug 2010 05:21:43 +0000 (22:21 -0700)]
vswitch: Add port status column to Port table

Add "status" map to provide optional status of ports to Port table.

14 years agoDelete local copy of simplejson.
Ben Pfaff [Sat, 21 Aug 2010 22:39:27 +0000 (15:39 -0700)]
Delete local copy of simplejson.

The previous commit dropped usage of simplejson from the Python
code in the tree, because it did not provide adequate features to
support the JSON-RPC engine.  In particular simplejson is not a
"push parser"--you can't give it a byte or a character at a time
and have it tell you when it has read a whole JSON value.

14 years agoImplement initial Python bindings for Open vSwitch database.
Ben Pfaff [Wed, 25 Aug 2010 17:26:40 +0000 (10:26 -0700)]
Implement initial Python bindings for Open vSwitch database.

These initial bindings pass a few hundred of the corresponding tests
for C implementations of various bits of the Open vSwitch library API.
The poorest part of them is actually the Python IDL interface in
ovs.db.idl, which has not received enough attention yet.  It appears
to work, but it doesn't yet support writes (transactions) and it is
difficult to use.  I hope to improve it as it becomes clear what
semantics Python applications actually want from an IDL.

14 years agoreconnect: Refactor tests to use common macro.
Ben Pfaff [Sat, 21 Aug 2010 03:50:17 +0000 (20:50 -0700)]
reconnect: Refactor tests to use common macro.

This will make it easier to add tests for the Python implemenentation.

14 years agoovsdbmonitor: Fix uninstall.
Ben Pfaff [Mon, 23 Aug 2010 23:56:25 +0000 (16:56 -0700)]
ovsdbmonitor: Fix uninstall.

We install an ovsdbmonitor binary so we should uninstall it too.

14 years agodaemon: Improve comments.
Ben Pfaff [Mon, 23 Aug 2010 06:13:35 +0000 (23:13 -0700)]
daemon: Improve comments.

Elsewhere we put the name of command-line options that control global
variables in the comment, so do so here as well.

Also fix a comment typo.

14 years agoreconnect: Fix typo in comment.
Ben Pfaff [Sun, 22 Aug 2010 20:39:43 +0000 (13:39 -0700)]
reconnect: Fix typo in comment.

14 years agojson: Remove unused return value from json_parser_push().
Ben Pfaff [Sun, 22 Aug 2010 20:38:39 +0000 (13:38 -0700)]
json: Remove unused return value from json_parser_push().

No point in returning a value that no caller uses.

14 years agopoll-loop: Fix obsolete comment.
Ben Pfaff [Sun, 22 Aug 2010 19:52:35 +0000 (12:52 -0700)]
poll-loop: Fix obsolete comment.

The poll loop used to have support for autonomous subroutines, but it no
longer does.

14 years agojsonrpc: Indentation fix.
Ben Pfaff [Sat, 21 Aug 2010 05:26:25 +0000 (22:26 -0700)]
jsonrpc: Indentation fix.

14 years agostream, vconn: Fix comments.
Ben Pfaff [Mon, 23 Aug 2010 19:18:05 +0000 (12:18 -0700)]
stream, vconn: Fix comments.

All streams and all vconns are "active", so there's no point in noting that
requirement in comments.  (A long time ago, active and passive vconns were
conflated instead of having passive vconns broken out as pvconns.  But
active and passive streams have always been distinct.)

14 years agojson: Remove write-only variable from json_lex_number().
Ben Pfaff [Fri, 20 Aug 2010 16:13:20 +0000 (09:13 -0700)]
json: Remove write-only variable from json_lex_number().

14 years agoovsdb: Remove unused ovsdb_datum_from_json_unique().
Ben Pfaff [Tue, 17 Aug 2010 19:49:31 +0000 (12:49 -0700)]
ovsdb: Remove unused ovsdb_datum_from_json_unique().

This function was not used outside of the test-ovsdb program.  It seems
like we might as well remove it.

14 years agoxenserver: Add ovs-parse-leaks manpage to list of files.
Ben Pfaff [Wed, 25 Aug 2010 20:04:34 +0000 (13:04 -0700)]
xenserver: Add ovs-parse-leaks manpage to list of files.

Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agoutilities: Remove ovs-wdt.
Ben Pfaff [Wed, 25 Aug 2010 16:59:26 +0000 (09:59 -0700)]
utilities: Remove ovs-wdt.

We used ovs-wdt at Nicira for a while when we were working on building
hardware switches.  We don't use it anymore, so remove it from the tree.

CC: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agoutilities: Remove ovs-monitor.
Ben Pfaff [Wed, 25 Aug 2010 16:57:11 +0000 (09:57 -0700)]
utilities: Remove ovs-monitor.

The ovs-monitor script is now more than adequately replaced by the
--monitor option to the various daemons.

CC: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agoovs-parse-leaks: Add manpage.
Ben Pfaff [Wed, 25 Aug 2010 00:49:13 +0000 (17:49 -0700)]
ovs-parse-leaks: Add manpage.

CC: Simon Horman <horms@verge.net.au>
14 years agodatapath: Unconditionally call kfree_skb()
Simon Horman [Wed, 25 Aug 2010 03:10:32 +0000 (12:10 +0900)]
datapath: Unconditionally call kfree_skb()

kfree_skb() will ignore a NULL pointer.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodebian: Use pfaffben@debian.org as uploader address for Ben Pfaff.
Ben Pfaff [Wed, 25 Aug 2010 00:12:23 +0000 (17:12 -0700)]
debian: Use pfaffben@debian.org as uploader address for Ben Pfaff.

I use pfaffben@debian.org as my contact address for Debian packages, so
use it here too.  I've had annoyed emails from folks when I am not
consistent about this, so fix it proactively.

14 years agoDebian: make debian/copyright more friendly to the ftpmasters
Simon Horman [Tue, 24 Aug 2010 00:54:58 +0000 (09:54 +0900)]
Debian: make debian/copyright more friendly to the ftpmasters

Signed-off-by: Simon Horman <horms@verge.net.au>
[list of copyright holders adjusted]
Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agonetdev-tunnel: Add CAPWAP userspace interface.
Jesse Gross [Thu, 12 Aug 2010 23:27:19 +0000 (19:27 -0400)]
netdev-tunnel: Add CAPWAP userspace interface.

Provide a userspace interface to the CAPWAP UDP transport
tunneling mechanism in the kernel.

Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agonetdev: Don't assume all netdevs are available at runtime.
Jesse Gross [Tue, 17 Aug 2010 22:09:53 +0000 (18:09 -0400)]
netdev: Don't assume all netdevs are available at runtime.

Currently we print a warning if a user tries to configure a
netdev that is not in the list that userspace knows about.
However, it is possible that a given netdev maybe be enabled but
when it tries to create a device it finds out that it can't
(not supported by kernel module, hardware not present, etc.).
This makes the behavior the same in both cases.

Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodatapath: Add support for CAPWAP UDP transport.
Jesse Gross [Thu, 12 Aug 2010 00:55:58 +0000 (20:55 -0400)]
datapath: Add support for CAPWAP UDP transport.

Add support for the transport portion of the CAPWAP protocol as
an alternative to GRE for L2 over L3 tunneling.  This is not
full support for the CAPWAP protocol.  CAPWAP covers management
of wireless access points and describes a control protocol for
setting those devices up.  It also describes a data plane protocol
that allows packets to be tunneled to a controller for inspection.
This data plane protocol is the only component covered by this
commit.

Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodatapath: Add support for tunnel fragmentation.
Jesse Gross [Mon, 16 Aug 2010 14:32:41 +0000 (10:32 -0400)]
datapath: Add support for tunnel fragmentation.

Up until now it was assumed that encapsulated packets larger than
the MTU would be fragmented by the IP stack.  However, some
tunneling protocols provide their own fragmentation mechanism.  This
adds the necessary support to the generic tunnel code to support
fragmentation.

Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agonetdev-gre: Genericize GRE netdev.
Jesse Gross [Wed, 11 Aug 2010 22:29:48 +0000 (18:29 -0400)]
netdev-gre: Genericize GRE netdev.

Since the GRE netdev doesn't actually implement any of the GRE
protocol, none of the code is really specific to GRE.  This commit
makes the netdev a little more generic so that additional tunnel
types can easily piggyback on it in the future.

Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodatapath: Abstract tunneling implementation from GRE.
Jesse Gross [Wed, 11 Aug 2010 00:11:48 +0000 (20:11 -0400)]
datapath: Abstract tunneling implementation from GRE.

Much of the code in the GRE implementation is not specific to the
GRE protocol but is actually common to all types of tunnels.  In
order to support future types of tunnels, move this code into a
common library.

Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodatapath: struct brport_attribute no longer has an owner element
Simon Horman [Mon, 23 Aug 2010 06:30:12 +0000 (15:30 +0900)]
datapath: struct brport_attribute no longer has an owner element

Between 2.6.35 and 2.6.36-rc1 the owner element of struct brport_attribute
was removed.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodatapath: Use rtnl_link_stats64
Simon Horman [Mon, 23 Aug 2010 06:30:11 +0000 (15:30 +0900)]
datapath: Use rtnl_link_stats64

This adds compatibility with a series kernel changesets that
introduces 64bit statistics. The final changeset (to date) being
"net: Document that dev_get_stats() returns the given pointer".
The relevant changesets were added between 2.6.35 and 2.6.36-rc1.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodatapath: use rx_handler_data pointer
Simon Horman [Mon, 23 Aug 2010 06:30:10 +0000 (15:30 +0900)]
datapath: use rx_handler_data pointer

This adds compatibility with kernel changeset
"bridge: use rx_handler_data pointer to store net_bridge_port pointer"
which was added between 2.6.35 and 2.6.36-rc1.

With this change it is now safe to (attempt to) insert both bridge and
datapath with newer (>=2.6.36) kernels, although whichever is inserted
second will fail to initialise on the call to netdev_rx_handler_register()

Signed-off-by: Simon Horman <horms@verge.net.au>
[Jesse: fixed merge conflicts in vport-netdev.c and netdevice.h]
Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodatapath: Take a rcu_dereference() in netdev_get_vport()
Simon Horman [Mon, 23 Aug 2010 06:30:09 +0000 (15:30 +0900)]
datapath: Take a rcu_dereference() in netdev_get_vport()

Although not strictly necessary, this will make this
function more consistent when compatibility for 2.6.36 is added.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodatapath: rtable may not have a u. member
Simon Horman [Mon, 23 Aug 2010 06:30:08 +0000 (15:30 +0900)]
datapath: rtable may not have a u. member

This brings the code up to sync with the kernel as
of changeset "net-next: remove useless union keyword",
which was added between 2.6.35 and 2.6.36-rc1

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodatapath: Handle duplicate netdev in netdev_rx_handler_register()
Simon Horman [Mon, 23 Aug 2010 06:30:07 +0000 (15:30 +0900)]
datapath: Handle duplicate netdev in netdev_rx_handler_register()

For kernels that have netdev_rx_handler_register() (>=2.6.35),
duplicate netdevs are detected by netdev_rx_handler_register().
So by adding duplicate detection to the netdev_rx_handler_register()
compatibility code the explicit check in netdev_create() can be removed.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodatapath: dont use non-existent receive hooks
Simon Horman [Mon, 23 Aug 2010 06:30:06 +0000 (15:30 +0900)]
datapath: dont use non-existent receive hooks

This adds compatibility with kernel changeset
of changeset "net: add rx_handler data pointer"
and thus "net: replace hooks in __netif_receive_skb V5",
which were added between 2.6.35 and 2.6.36-rc1

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agoterminal: Remove vlog modules.
Jesse Gross [Mon, 23 Aug 2010 17:42:19 +0000 (13:42 -0400)]
terminal: Remove vlog modules.

The terminal modules in vlog-modules.def weren't removed when the
code was, which breaks compilation due to a check for this
condition.

14 years agoRemove ezio-term and ovs-switchui utilities.
Ben Pfaff [Fri, 20 Aug 2010 19:37:01 +0000 (12:37 -0700)]
Remove ezio-term and ovs-switchui utilities.

These utilities were useful when Nicira was building switches with 16x2 LCD
front panel displays, but they aren't useful for other environments and
even Nicira does not use that kind of switch any longer.  So remove them
and all the build infrastructure on which they depended.

14 years agodatpath: Avoid reporting half updated statistics.
Jesse Gross [Thu, 29 Jul 2010 01:20:43 +0000 (18:20 -0700)]
datpath: Avoid reporting half updated statistics.

We enforce mutual exclusion when updating statistics by disabling
bottom halves and only writing to per-CPU state.  However, reading
requires looking at the statistics for foreign CPUs, which could be
in the process of updating them since there isn't a lock.  This means
we could get garbage values for 64-bit values on 32-bit machines or
byte counts that don't correspond to packet counts, etc.

This commit introduces a sequence lock for statistics values to avoid
this problem.  Getting a write lock is very cheap - it only requires
incrementing a counter plus a memory barrier (which is compiled away
on x86) to acquire or release the lock and will never block.  On
read we spin until the sequence number hasn't changed in the middle
of the operation, indicating that the we have a consistent set of
values.

Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agogre: Don't require incoming checksum.
Jesse Gross [Sat, 14 Aug 2010 16:25:58 +0000 (12:25 -0400)]
gre: Don't require incoming checksum.

The current meaning of the GRE checksum option is to include
checksums on transmit and require packets to have them on receive.
In addition, incoming packets with checksums are always validated
regardless of this option.  Requiring checksums on receive creates
surprising behavior and interoperability issues.  This disables the
requirement on receive.  The new behavior is that the sender decides
whether to checksum packets and the receiver will validate packets
with checksums (similar to UDP).

Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agogre: Disable checksums by default.
Jesse Gross [Fri, 13 Aug 2010 03:31:03 +0000 (23:31 -0400)]
gre: Disable checksums by default.

GRE checksums aren't really all that useful because they only
add value for the GRE and inner Ethernet header.  However, they
are expensive since they cover the entire packet, even though
most of the data is protected by L3 and L4 checksums.  Therefore
disable checksumming by default to improve performance.  In addition,
since CAPWAP doesn't support checksums this makes it consistent with
GRE.

Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agobacktrace: Use generic code to find the bottom of the stack.
Tsvi Slonim [Fri, 20 Aug 2010 17:43:13 +0000 (10:43 -0700)]
backtrace: Use generic code to find the bottom of the stack.

This fixes an ugly GCC warning without using inline asm.

14 years agosocket-util: Suppress uninitialized variable warning with old GCC.
Bryan Phillippe [Fri, 20 Aug 2010 16:27:16 +0000 (09:27 -0700)]
socket-util: Suppress uninitialized variable warning with old GCC.

14 years agovconn-stream: printf() specifier for int is %d (not %zu)
Bryan Phillippe [Fri, 20 Aug 2010 16:25:34 +0000 (09:25 -0700)]
vconn-stream: printf() specifier for int is %d (not %zu)

14 years agosocket-util: Remove stray printf() from make_unix_socket().
Bryan Phillippe [Fri, 20 Aug 2010 17:42:29 +0000 (10:42 -0700)]
socket-util: Remove stray printf() from make_unix_socket().

14 years agodebian: Use horms@debian.org the uploaders field
Simon Horman [Fri, 20 Aug 2010 02:32:52 +0000 (11:32 +0900)]
debian: Use horms@debian.org the uploaders field

Sorry for the noise, I should have noticed this earlier,
but for consistency with other packages I'd prefer to
use my debian.org address here.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Jesse Gross <jesse@nicira.com>
14 years agodebian: Fix "make dist" by adding corekeeper.override to EXTRA_DIST.
Ben Pfaff [Thu, 19 Aug 2010 20:11:05 +0000 (13:11 -0700)]
debian: Fix "make dist" by adding corekeeper.override to EXTRA_DIST.

Reported-by: Teemu Koponen <koponen@nicira.com>
14 years agoAUTHORS: Add Simon Horman <horms@verge.net.au>.
Ben Pfaff [Thu, 19 Aug 2010 16:53:08 +0000 (09:53 -0700)]
AUTHORS: Add Simon Horman <horms@verge.net.au>.

14 years agoFix SSL boilerplate descriptions in manpages.
Ben Pfaff [Mon, 16 Aug 2010 22:59:26 +0000 (15:59 -0700)]
Fix SSL boilerplate descriptions in manpages.

Some of the SSL boilerplate was specific to switches, but it was included
in OVSDB programs also.  Make it more generic.  Also document SSL options
in some manpages where they were missing.

14 years agoFix typos in manpages.
Ben Pfaff [Mon, 16 Aug 2010 22:57:03 +0000 (15:57 -0700)]
Fix typos in manpages.

14 years agoovs-vsctl: Fix parsing of short SSL options.
Ben Pfaff [Mon, 16 Aug 2010 22:54:47 +0000 (15:54 -0700)]
ovs-vsctl: Fix parsing of short SSL options.

The short versions of the SSL options (e.g. -p, -c, -C) did not work,
because they were not in the string passed to getopt_long().  This commit
fixes the problem and should avoid its recurrence with any other short
options that we add in the future.

14 years agodebian: Add override of non-standard-dir-perm to corekeeper
Simon Horman [Thu, 19 Aug 2010 07:55:38 +0000 (16:55 +0900)]
debian: Add override of non-standard-dir-perm to corekeeper

It is intentional that var/log/core/ has a non standard permission
of 1777 instead of 0755.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agoDebian: Use ${source:Version} for versioned dependencies as appropriate
Simon Horman [Thu, 19 Aug 2010 07:55:37 +0000 (16:55 +0900)]
Debian: Use ${source:Version} for versioned dependencies as appropriate

${source:Version} should be used for versioned dependencies
of arch:any packages on arch:all packages.

${source:Version} may be used for versioned dependencies
of arch:all packages on arch:any packages.

See: http://lintian.debian.org/tags/not-binnmuable-any-depends-all.html
     http://lintian.debian.org/tags/not-binnmuable-all-depends-any.html

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agoDebian: Add Build-Depends on python
Simon Horman [Thu, 19 Aug 2010 07:55:36 +0000 (16:55 +0900)]
Debian: Add Build-Depends on python

Python is needed in order to run the following tests:

interface-reconfigure

579: non-VLAN, non-bond                              ok
580: VLAN, non-bond                                  ok
581: Bond, non-VLAN                                  ok
582: VLAN on bond                                    ok

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agoDebian: Update to standards version 3.9.1
Simon Horman [Thu, 19 Aug 2010 07:55:35 +0000 (16:55 +0900)]
Debian: Update to standards version 3.9.1

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agoDebian: Add Ben Pfaff and Simon Horman the uploaders
Simon Horman [Thu, 19 Aug 2010 07:55:34 +0000 (16:55 +0900)]
Debian: Add Ben Pfaff and Simon Horman the uploaders

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agoodp-util: Avoid branch in odp_actions_add().
Ben Pfaff [Wed, 4 Aug 2010 17:50:40 +0000 (10:50 -0700)]
odp-util: Avoid branch in odp_actions_add().

I have no idea why, but the test and branch in odp_actions_add() has always
bugged me.  This commit eliminates it.

14 years agouuid: Fix warnings carelessly introduced a few commits ago.
Ben Pfaff [Fri, 13 Aug 2010 16:58:29 +0000 (09:58 -0700)]
uuid: Fix warnings carelessly introduced a few commits ago.

Commit e251c8 "uuid: Break code to read /dev/urandom into a new module"
carelessly introduced a few warnings, which this commit fixes up.

14 years agoofproto: Add support for NXAST_RESUBMIT recursion.
Ben Pfaff [Fri, 13 Aug 2010 16:57:25 +0000 (09:57 -0700)]
ofproto: Add support for NXAST_RESUBMIT recursion.

CC: Teemu Koponen <koponen@nicira.com>
14 years agoRemove vestigial support for Spanning Tree Protocol.
Ben Pfaff [Thu, 12 Aug 2010 00:24:13 +0000 (17:24 -0700)]
Remove vestigial support for Spanning Tree Protocol.

Open vSwitch has never properly supported IEEE 802.1D Spanning Tree
Protocol (STP), but it has various bits and pieces that claim to support
it.  This commit deletes them, to reduce the amount of dead code in the
tree.  We can always reintroduce it later if it proves to be a good idea.

Bug #1175.

14 years agorandom: Get random seed from /dev/urandom.
Ben Pfaff [Thu, 12 Aug 2010 18:12:13 +0000 (11:12 -0700)]
random: Get random seed from /dev/urandom.

Even though this PRNG is not meant to be cryptographically secure, there is
no reason not to get a high-quality seed.

CC: Stephen Hemminger <shemminger@vyatta.com>
14 years agouuid: Break code to read /dev/urandom into a new module.
Ben Pfaff [Thu, 12 Aug 2010 22:47:25 +0000 (15:47 -0700)]
uuid: Break code to read /dev/urandom into a new module.

This code is useful for seeding other random number generators, so we might
as well make it a separate source file.

14 years agoutil: Make ovs_fatal() understand EOF also.
Ben Pfaff [Thu, 12 Aug 2010 18:05:07 +0000 (11:05 -0700)]
util: Make ovs_fatal() understand EOF also.

ovs_error() interprets EOF as "end of file" when printing an error message,
so ovs_fatal() might as well.

14 years agoWait for daemons to die in init.d script "stop" commands.
Ben Pfaff [Thu, 12 Aug 2010 17:18:19 +0000 (10:18 -0700)]
Wait for daemons to die in init.d script "stop" commands.

Sometimes it takes a moment for the OVS daemons to die.  When that happens,
the "start" half of "openvswitch restart" can fail when ovsdb-tool
runs, because ovsdb-server will still have the lock on the database if it
has not exited yet.  So this commit just makes the "stop" half wait for
the daemons to really die.

Bug #3369.

14 years agodaemon: Make sure that vlog is initialized when a process daemonizes.
Ben Pfaff [Thu, 12 Aug 2010 16:47:33 +0000 (09:47 -0700)]
daemon: Make sure that vlog is initialized when a process daemonizes.

If a process daemonizes itself, then it should be possible to control that
process's log levels with "ovs-appctl vlog/set" and related commands.  The
vlog_init() function registers those commands.  But vlog_init() doesn't
normally get called until the first log message is issued.  This can take a
while, especially for ovs-controller, where I first noticed the problem.

This commit fixes the problem by calling vlog_init() from
daemonize_start(), which always gets called as a process daemonizes.

14 years agodebian: Use dh_installmodules instead of calling "depmod" wrongly.
Ben Pfaff [Thu, 5 Aug 2010 17:59:26 +0000 (10:59 -0700)]
debian: Use dh_installmodules instead of calling "depmod" wrongly.

Until now, the postinst for kernel modules built by the Debian packaging
has simply run "depmod -a", which is wrong, since this command rebuilds
the dependencies for the *running* kernel, which is not necessarily the
kernel for which modules are being installed.

The dh_installmodules script automatically adds the correct invocation of
depmod to the postinst script, so this commit switches to using that
instead.

This commit moves the kernel modules from /lib/modules/$KVERS into the
"kernel" subdirectory of that directory because dh_installmodules does not
support modules that are directly in the $KVERS directory.

CC: Sajjad Lateef <slateef@nicira.com>
14 years agorandom: Implement a decent random number generator.
Ben Pfaff [Thu, 5 Aug 2010 17:23:36 +0000 (10:23 -0700)]
random: Implement a decent random number generator.

Until now this library has based its random number upon those returned
by libc's rand() function.  This has always bugged me--it is not a good
solution since rand() varies in quality so much.  This commit changes
the random library to use a simple but high-quality PRNG.

14 years agobridge: Don't pay attention to columns that vswitchd doesn't need.
Ben Pfaff [Wed, 11 Aug 2010 22:29:36 +0000 (15:29 -0700)]
bridge: Don't pay attention to columns that vswitchd doesn't need.

Not replicating unneeded columns has some value in avoiding CPU time and
bandwidth to the database.  In ovs-vswitchd, setting cur_cfg as write-only
also have great value in avoiding extra reconfiguration steps.  When
ovs-vsctl is used in its default mode this essentially avoids half of the
reconfigurations that ovs-vswitchd currently does.  What happens now is:

    1. ovs-vsctl updates the database and increments next_cfg.
    2. ovs-vswitchd notices the change to the database, reconfigures
       itself, then increments cur_cfg to match next_cfg.
    3. The database sends the change to cur_cfg back to ovs-vswitchd.
    4. ovs-vswitchd reconfigures itself a second time.

By not replicating cur_cfg we avoid step 3 and save a whole reconfiguration
step.

Also, now that the database contains interface statistics, this avoids
reconfiguring every time that statistics are updated.

14 years agoovsdb-idl: Make it possible to omit or pay less attention to columns.
Ben Pfaff [Wed, 11 Aug 2010 22:41:41 +0000 (15:41 -0700)]
ovsdb-idl: Make it possible to omit or pay less attention to columns.

ovs-vswitchd has no need to replicate some parts of the database.  In
particular, it doesn't need to replicate the bits that it never reads,
such as the external_ids column in the Open_vSwitch table.  This saves
some memory, CPU time, and bandwidth to the database.

Another type of column that benefits from special treatment is "write-only
columns", that is, those that ovs-vswitchd writes and keeps up-to-date but
never expects another client to write, such as the cur_cfg column in the
Open_vSwitch table.  If the IDL reports that the database has changed when
ovs-vswitchd updates such a column, then ovs-vswitchd reconfigures itself
for no reason, wasting CPU time.  This commit also adds support for such
columns.

14 years agostream-ssl: Enable SSL session caching.
Ben Pfaff [Wed, 11 Aug 2010 17:24:40 +0000 (10:24 -0700)]
stream-ssl: Enable SSL session caching.

14 years agostream-ssl: Remove unused 'connect_error' member.
Ben Pfaff [Mon, 9 Aug 2010 21:42:35 +0000 (14:42 -0700)]
stream-ssl: Remove unused 'connect_error' member.

Never read, never written.

14 years agovswitch: Fix speling error in documentation.
Ben Pfaff [Tue, 10 Aug 2010 21:35:36 +0000 (14:35 -0700)]
vswitch: Fix speling error in documentation.

14 years agodpif-netdev: Properly track whether there is a vlan header.
Ben Pfaff [Wed, 28 Jul 2010 00:00:54 +0000 (17:00 -0700)]
dpif-netdev: Properly track whether there is a vlan header.

It looks to me like the current dpif-netdev implementation doesn't handle
the case where a packet comes in without a VLAN and then is subjected to
multiple ODPAT_SET_VLAN_* operations.  dp_netdev_modify_vlan_tci() just
checks the flow key each time to see whether there's a VLAN, but it doesn't
update the flow key to note that there is now a VLAN.

One fix would be to update the flow key, but it's "const" these days.
Instead, add a check for whether the Ethernet type is ETH_TYPE_VLAN,
which should be equivalent.

14 years agodpif-netdev: Tolerate undersized packets.
Ben Pfaff [Tue, 10 Aug 2010 18:38:55 +0000 (11:38 -0700)]
dpif-netdev: Tolerate undersized packets.

Actions that modify packets need to tolerate packets that are too small.
Most of the actions already implicitly do this check, since they check for
appropriate values in the flow key that would only be there if the
corresponding data was present.  But actions to modify the Ethernet header
didn't have a guarantee that the packet was at least 14 bytes long, and
actions to modify the VLAN didn't have such a guarantee either, so this
adds appropriate checks.

Problem found by code inspection.

14 years agodatapath: Fix handling of 802.1Q and SNAP headers.
Ben Pfaff [Tue, 10 Aug 2010 18:35:46 +0000 (11:35 -0700)]
datapath: Fix handling of 802.1Q and SNAP headers.

The kernel and user datapaths have code that assumes that 802.1Q headers
are used only inside Ethernet II frames, not inside SNAP-encapsulated
frames.  But the kernel and user flow_extract() implementations would
interpret 802.1Q headers inside SNAP headers as being valid VLANs.  This
would cause packet corruption if any VLAN-related actions were to be taken,
so change the two flow_extract() implementations only to accept 802.1Q as
an Ethernet II frame type, not as a SNAP-encoded frame type.

802.1Q-2005 says that this is correct anyhow:

    Where the ISS instance used to transmit and receive tagged frames is
    provided by a media access control method that can support Ethernet
    Type encoding directly (e.g., is an IEEE 802.3 or IEEE 802.11 MAC) or
    is media access method independent (e.g., 6.6), the TPID is Ethernet
    Type encoded, i.e., is two octets in length and comprises solely the
    assigned Ethernet Type value.

    Where the ISS instance is provided by a media access method that
    cannot directly support Ethernet Type encoding (e.g., is an IEEE
    802.5 or FDDI MAC), the TPID is encoded according to the rule for
    a Subnetwork Access Protocol (Clause 10 of IEEE Std 802) that
    encapsulates Ethernet frames over LLC, and comprises the SNAP
    header (AA-AA-03) followed by the SNAP PID (00-00-00) followed by
    the two octets of the assigned Ethernet Type value.

All of the media that OVS handles supports Ethernet Type fields, so to me
that means that we don't have to handle 802.1Q-inside-SNAP.

On the other hand, we *do* have to handle SNAP-inside-802.1Q, because this
is actually allowed by the standards.  So this commit also adds that
support.

I verified that, with this change, both SNAP and Ethernet packets are
properly recognized both with and without 802.1Q encapsulation.

I was a bit surprised to find out that Linux does not accept
SNAP-encapsulated IP frames on Ethernet.

Here's a summary of how frames are handled before and after this commit:

Common cases
------------

       Ethernet
    +------------+
1.  |dst|src|TYPE|
    +------------+

       Ethernet       LLC         SNAP
    +------------+ +--------+ +-----------+
2.  |dst|src| len| |aa|aa|03| |000000|TYPE|
    +------------+ +--------+ +-----------+

       Ethernet       802.1Q
    +------------+ +---------+
3.  |dst|src|8100| |VLAN|TYPE|
    +------------+ +---------+

       Ethernet       802.1Q      LLC         SNAP
    +------------+ +---------+ +--------+ +-----------+
4.  |dst|src|8100| |VLAN| LEN| |aa|aa|03| |000000|TYPE|
    +------------+ +---------+ +--------+ +-----------+

Unusual cases
-------------

       Ethernet       LLC         SNAP         802.1Q
    +------------+ +--------+ +-----------+ +---------+
5.  |dst|src| len| |aa|aa|03| |000000|8100| |VLAN|TYPE|
    +------------+ +--------+ +-----------+ +---------+

       Ethernet       LLC
    +------------+ +--------+
6.  |dst|src| len| |xx|xx|xx|
    +------------+ +--------+

       Ethernet       LLC         SNAP
    +------------+ +--------+ +-----------+
7.  |dst|src| len| |aa|aa|03| |xxxxxx|xxxx|
    +------------+ +--------+ +-----------+

       Ethernet       802.1Q      LLC
    +------------+ +---------+ +--------+
8.  |dst|src|8100| |VLAN| LEN| |xx|xx|xx|
    +------------+ +---------+ +--------+

       Ethernet       802.1Q      LLC         SNAP
    +------------+ +---------+ +--------+ +-----------+
9.  |dst|src|8100| |VLAN| LEN| |aa|aa|03| |xxxxxx|xxxx|
    +------------+ +---------+ +--------+ +-----------+

Behavior
--------

   ---------------  ---------------  -------------------------------------
       Before           After
     this commit      this commit
   dl_type dl_vlan  dl_type dl_vlan  Notes
   ------- -------  ------- -------  -------------------------------------
1.   TYPE    ffff     TYPE    ffff   no change
2.   TYPE    ffff     TYPE    ffff   no change
3.   TYPE    VLAN     TYPE    VLAN   no change
4.    LEN    VLAN     TYPE    VLAN   proposal fixes behavior
5.   TYPE    VLAN     8100    ffff   802.1Q says this is invalid framing
6.   05ff    ffff     05ff    ffff   no change
7.   05ff    ffff     05ff    ffff   no change
8.    LEN    VLAN     05ff    VLAN   proposal fixes behavior
9.    LEN    VLAN     05ff    VLAN   proposal fixes behavior

Signed-off-by: Ben Pfaff <blp@nicira.com>
14 years agovswitch: Clarify "arguments" versus "options".
Ben Pfaff [Fri, 6 Aug 2010 18:36:39 +0000 (11:36 -0700)]
vswitch: Clarify "arguments" versus "options".

Interface has an "options" column but some text referred to "arguments"
instead, which confused some readers.  Also be even more explicit about
syntax, since this also confused some readers.

CC: Dan Wendlandt <dan@nicira.com>
14 years agoofproto: Add support for remote "service controllers".
Ben Pfaff [Fri, 6 Aug 2010 23:49:20 +0000 (16:49 -0700)]
ofproto: Add support for remote "service controllers".

CC: Dan Wendlandt <dan@nicira.com>
14 years agoovs-openflowd: Fix support for multiple controllers.
Ben Pfaff [Fri, 6 Aug 2010 23:49:14 +0000 (16:49 -0700)]
ovs-openflowd: Fix support for multiple controllers.

The multiple controller support here has apparently never been tested.  I
still haven't tested it, but I fixed a few obvious problems in the source
code and in the manpage.

14 years agoofproto: Improve terminology.
Ben Pfaff [Thu, 5 Aug 2010 21:56:53 +0000 (14:56 -0700)]
ofproto: Improve terminology.

To me, "primary" and "service" connections seem like better terminology
than "controller" and "transient".

14 years agojson: Add extern "C" { ... } to headers.
Ben Pfaff [Fri, 6 Aug 2010 23:05:17 +0000 (16:05 -0700)]
json: Add extern "C" { ... } to headers.

This makes it easier for external C++ projects to import the header.

CC: Jeremy Stribling <strib@nicira.com>
14 years agoovs-pki: Create private keys with restricted permissions.
Ben Pfaff [Fri, 6 Aug 2010 17:24:13 +0000 (10:24 -0700)]
ovs-pki: Create private keys with restricted permissions.

OpenSSL will happily create private keys world-writable, but we probably
should not do that.

CC: Keith Amidon <keith@nicira.com>
14 years agoovs-pki: Create log directory if it does not exist.
Ben Pfaff [Fri, 6 Aug 2010 20:32:47 +0000 (13:32 -0700)]
ovs-pki: Create log directory if it does not exist.

Otherwise "ovs-pki init" can create the pkidir and then fail when it tries
to create the log file, if it is in a directory that does not exist.

14 years agoovs-pki: Consistently write error messages to stderr.
Ben Pfaff [Fri, 6 Aug 2010 17:22:29 +0000 (10:22 -0700)]
ovs-pki: Consistently write error messages to stderr.

14 years agovswitchd: Only re-learn from flows that output to OFPP_NORMAL.
Ben Pfaff [Fri, 6 Aug 2010 18:46:24 +0000 (11:46 -0700)]
vswitchd: Only re-learn from flows that output to OFPP_NORMAL.

Commit e96a4d8035 "bridge: Feed flow stats into learning table." started
feeding flow statistics back into the learning table, but it did not
distinguish between flows with and flows without an action that outputs to
OFPP_NORMAL.  Flows without such an action are not put into the learning
table initially, because bridge_normal_ofhook_cb() is not called for them,
but since that commit they have been put into the learning table when their
flows are reassessed.

This is inconsistent--flows without OFPP_NORMAL should either be learned
from all the time or never, not sometimes.  I can see valid arguments both
ways, but since it was always my intention not to learn from such flows,
this commit disables learning from them.

Problem found by code inspection.  I don't know of any observed bug that
this fixes.

14 years agotag: Be more precise about choosing tags to add, in tag_set_add().
Ben Pfaff [Fri, 6 Aug 2010 18:42:07 +0000 (11:42 -0700)]
tag: Be more precise about choosing tags to add, in tag_set_add().

It is not necessary to add a "tag" if all of the bits in it are already
present in some member of the set.  This commit adds that optimization.

14 years agotag: Use existing macro instead of constant.
Ben Pfaff [Tue, 3 Aug 2010 21:37:22 +0000 (14:37 -0700)]
tag: Use existing macro instead of constant.

N_TAG_BITS is always 32, currently, because tag_type is a typedef
for uint32_t, so this does not fix an actual bug.

14 years agovswitchd: Refresh SSL keys and certificates more frequently.
Ben Pfaff [Thu, 5 Aug 2010 16:58:58 +0000 (09:58 -0700)]
vswitchd: Refresh SSL keys and certificates more frequently.

Until now, the ovs-vswitchd main loop has refreshed keys and certificates
from their files only when the database changes.  This works fine if new
keys and certificates are installed with new file names, because the update
to the database to point to the new files will cause them to be read.  But
if the new keys and certificates are copied over the existing files, then
the delay until they are read is indefinite.

This commit fixes the problem by changing the SSL configuration so that it
is rechecked on every trip through the ovs-vswitchd main loop.

Bug #2921.

14 years agostream-ssl: Make changing keys and certificate at runtime reliable.
Ben Pfaff [Thu, 5 Aug 2010 16:24:00 +0000 (09:24 -0700)]
stream-ssl: Make changing keys and certificate at runtime reliable.

OpenSSL is picky about the order in which keys and certificates are
changed: you have to change the certificate first, then the key.  It
doesn't document this, but deep in the source code, in a function that sets
a new certificate, it has this comment:

    /* don't fail for a cert/key mismatch, just free
     * current private key (when switching to a different
     * cert & key, first this function should be used,
     * then ssl_set_pkey */

Brilliant, guys, thanks a lot.

Bug #2921.

14 years agodatapath: Detect and suppress flows that are implicated in loops.
Ben Pfaff [Tue, 3 Aug 2010 21:40:29 +0000 (14:40 -0700)]
datapath: Detect and suppress flows that are implicated in loops.

In-kernel loops need to be suppressed; otherwise, they cause high CPU
consumption, even to the point that the machine becomes unusable.  Ideally
these flows should never be added to the Open vSwitch flow table, but it
is fairly easy for a buggy controller to create them given the menagerie
of tunnels, patches, etc. that OVS makes available.

Commit ecbb6953b "datapath: Add loop checking" did the initial work
toward suppressing loops, by dropping packets that recursed more than 5
times.  This at least prevented the kernel stack from overflowing and
thereby OOPSing the machine.  But even with this commit, it is still
possible to waste a lot of CPU time due to loops.  The problem is not
limited to 5 recursive calls per packet: any packet can be sent to
multiple destinations, which in turn can themselves be sent to multiple
destinations, and so on.  We have actually seen in practice a case where
each packet was, apparently, sent to at least 2 destinations per hop, so
that each packet actually consumed CPU time for 2**5 == 32 packets,
possibly more.

This commit takes loop suppression a step further, by clearing the actions
of flows that are implicated in loops.  Thus, after the first packet in
such a flow, later packets for either the "root" flow or for flows that
it ends up looping through are simply discarded, saving a huge amount of
CPU time.

This version of the commit just clears the actions from the flows that a
part of the loop.  Probably, there should be some additional action to tell
ovs-vswitchd that a loop has been detected, so that it can in turn inform
the controller one way or another.

My test case was this:

ovs-controller -H --max-idle=permanent punix:/tmp/controller
ovs-vsctl -- \
    set-controller br0 unix:/tmp/controller -- \
    add-port br0 patch00 -- \
    add-port br0 patch01 -- \
    add-port br0 patch10 -- \
    add-port br0 patch11 -- \
    add-port br0 patch20 -- \
    add-port br0 patch21 -- \
    add-port br0 patch30 -- \
    add-port br0 patch31 -- \
    set Interface patch00 type=patch options:peer=patch01 -- \
    set Interface patch01 type=patch options:peer=patch00 -- \
    set Interface patch10 type=patch options:peer=patch11 -- \
    set Interface patch11 type=patch options:peer=patch10 -- \
    set Interface patch20 type=patch options:peer=patch21 -- \
    set Interface patch21 type=patch options:peer=patch20 -- \
    set Interface patch30 type=patch options:peer=patch31 -- \
    set Interface patch31 type=patch options:peer=patch30

followed by sending a single "ping" packet from an attached Ethernet
port into the bridge.  After this, without this commit the vswitch
userspace and kernel consume 50-75% of the machine's CPU (in my KVM
test setup on a single physical host); with this commit, some CPU is
consumed initially but it converges on 0% quickly.

A more challenging test sends a series of packets in multiple flows;
I used "hping3" with its default options.  Without this commit, the
vswitch consumes 100% of the machine's CPU, most of which is in the
kernel.  With this commit, the vswitch consumes "only" 33-50% CPU,
most of which is in userspace, so the machine is more responsive.

A refinement on this commit would be to pass the loop counter down to
userspace as part of the odp_msg struct and then back up as part of
the ODP_EXECUTE command arguments.  This would, presumably, reduce
the CPU requirements, since it would allow loop detection to happen
earlier, during initial setup of flows, instead of just on the second
and subsequent packets of flows.

14 years agodatapath: Inline flow_cast().
Ben Pfaff [Fri, 23 Jul 2010 20:05:25 +0000 (13:05 -0700)]
datapath: Inline flow_cast().

This function is both trivial and on the packet processing fast path, so
expand it inline.