openvswitch
13 years agodpif-netdev: Add missing 'const' qualifiers to function parameters.
Ben Pfaff [Wed, 29 Dec 2010 06:40:52 +0000 (22:40 -0800)]
dpif-netdev: Add missing 'const' qualifiers to function parameters.

These functions don't modify their flow key arguments but the prototypes
implied that they did.

Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Remove ODPP_NORMAL macro.
Ben Pfaff [Tue, 28 Dec 2010 20:23:16 +0000 (12:23 -0800)]
datapath: Remove ODPP_NORMAL macro.

This macro hasn't ever been used.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Fix 32-bit truncation of argument to ODPAT_OUTPUT_CONTROL.
Ben Pfaff [Tue, 28 Dec 2010 21:28:31 +0000 (13:28 -0800)]
datapath: Fix 32-bit truncation of argument to ODPAT_OUTPUT_CONTROL.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agoovs-dpctl: Reimplement get_port_number() using dpif_port_query_by_name().
Ben Pfaff [Mon, 27 Dec 2010 20:46:48 +0000 (12:46 -0800)]
ovs-dpctl: Reimplement get_port_number() using dpif_port_query_by_name().

Presumably this function was written to iterate all of the ports because
at some point we didn't have a direct way to do this, but now
dpif_port_query_by_name() is the obvious way to do it.

Acked-by: Jesse Gross <jesse@nicira.com>
13 years agoofproto: Use shash instead of svec for uniquifying, in reinit_ports().
Ben Pfaff [Mon, 27 Dec 2010 19:32:08 +0000 (11:32 -0800)]
ofproto: Use shash instead of svec for uniquifying, in reinit_ports().

No point in using an O(n log n) algorithm when an O(n) algorithm is
readily available.

(I'm sure that the actual performance difference, if any, does not matter
in practice.)

Acked-by: Jesse Gross <jesse@nicira.com>
13 years agomultipath: Zero padding bytes in fields before hashing.
Ben Pfaff [Wed, 29 Dec 2010 06:23:43 +0000 (22:23 -0800)]
multipath: Zero padding bytes in fields before hashing.

Otherwise the hash includes 3 bytes of trailing indeterminate data.

Reported-by: Pankaj Thakkar <thakkar@nicira.com>
13 years agovswitch: Provide option to pull cert from SSL table
Justin Pettit [Tue, 28 Dec 2010 01:44:33 +0000 (17:44 -0800)]
vswitch: Provide option to pull cert from SSL table

Introduce "use_ssl_cert" option to "ipsec_gre" interface types, which
will pull certificate and private key options from the SSL table.  In
the future, multiple SSL entries will be supported through the
configuration database, so use of this option is strongly discouraged as
this "feature" will be retired.

13 years agodebian: Require ipsec-tools version 0.8~alpha20101208.
Justin Pettit [Wed, 22 Dec 2010 20:55:30 +0000 (12:55 -0800)]
debian: Require ipsec-tools version 0.8~alpha20101208.

There have been a number of important bug fixes since 0.8~alpha20090903,
so require at least the newer package.

13 years agovswitch: Add support for IPsec certificate authentication.
Justin Pettit [Thu, 16 Dec 2010 07:44:41 +0000 (23:44 -0800)]
vswitch: Add support for IPsec certificate authentication.

Previously, it was possible to fake configuring the use of certificate
authentication for IPsec, but it really just used a static pre-shared key
behind the scenes.  This commit publicly mentions certificate
authentication and finally does the real work behind the scenes.

13 years agovswitch: Remove unnecessary iface_get_options function
Justin Pettit [Thu, 2 Dec 2010 02:03:44 +0000 (18:03 -0800)]
vswitch: Remove unnecessary iface_get_options function

Since GRE-over-IPsec is a proper tunnel type and no longer configured
through "other_config", we can remove this function that folded an
interface's "other_confg" into "options".

13 years agovswitch: Use "ipsec_gre" vport instead of "gre" with "other_config"
Justin Pettit [Thu, 2 Dec 2010 01:23:33 +0000 (17:23 -0800)]
vswitch: Use "ipsec_gre" vport instead of "gre" with "other_config"

Previously, a GRE-over-IPsec tunnel was created as an interface with a
"type" of "gre" and the "other_config" column with "ipsec_cert" or
"ipsec_psk" set.  This could lead to a potential security problem if a user
intended to create a GRE-over-IPsec tunnel, but misconfigured the
"ipsec_*" config and created an unencrypted GRE tunnel.

This commit defines an "ipsec_gre" tunnel type, which should prevent
users from inadvertently establishing insecure tunnels.

13 years agodebian: Don't require ipsec_local_ip to configure IPsec
Justin Pettit [Tue, 30 Nov 2010 02:55:54 +0000 (18:55 -0800)]
debian: Don't require ipsec_local_ip to configure IPsec

Commit e97a103 (Open vSwitch: ovs-monitor-ipsec: Add ability to traverse
NATs) removed the requirement that the "ipsec_local_ip" key must be set
to use IPsec, but other code and documentation was not updated to
reflect this.  This commit does that.

13 years agoovs-dpctl: Print extended information about vports.
Justin Pettit [Sat, 18 Dec 2010 09:07:06 +0000 (01:07 -0800)]
ovs-dpctl: Print extended information about vports.

When "ovs-dpctl show" is run, return additional information about the
port.  For example, tunnel ports will print the remote_ip, local_ip, and
in_key when defined.

13 years agodatapath: Return vport configuration when queried.
Justin Pettit [Sat, 18 Dec 2010 09:04:37 +0000 (01:04 -0800)]
datapath: Return vport configuration when queried.

Additional configuration is passed down to the kernel in the "config"
array of an odp_port when a vport is created.  This information is not
returned when a vport is queried, though.  This information is useful
for debugging, since it may be used to distinguish ports based on
additional data, such as the peer in tunnels.  In a forthcoming patch, it
will be essential to distinguish between plain GRE and GRE over IPsec.

Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodebian: Fix filename for ovs-vlan-test manpage.
Ben Pfaff [Tue, 28 Dec 2010 18:03:44 +0000 (10:03 -0800)]
debian: Fix filename for ovs-vlan-test manpage.

This fixes the Debian package build.

13 years agotunneling: Don't shadow vport when generating cache.
Jesse Gross [Tue, 28 Dec 2010 05:19:35 +0000 (21:19 -0800)]
tunneling: Don't shadow vport when generating cache.

When generating the tunnel header cache we have two vports that we
are working with: the sender and destination.  Unfortunately, both of
these use the name 'vport'.  This renames the destination to avoid
shadowing the sender.  This doesn't actually fix a bug because the
compiler correctly uses the right one, even when shadowed.

Found with sparse.

Reported-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agoodp-util: Improve formatting of bad ODP actions.
Ben Pfaff [Thu, 23 Dec 2010 22:21:01 +0000 (14:21 -0800)]
odp-util: Improve formatting of bad ODP actions.

nla_len is the size of a Netlink attribute including its header, but we
only want to print the attribute payload, so use nl_attr_get_size()
instead.

Also, make it clear that what follows the "bad action" information is a
dump of the action itself.

Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Clean up use of TBL_* constants.
Ben Pfaff [Tue, 28 Dec 2010 00:20:11 +0000 (16:20 -0800)]
datapath: Clean up use of TBL_* constants.

A lot of the TBL_* constants were being used in conceptually wrong ways,
even though the code was correct because the actual values were correct.
(This is because TBL_L1_BITS, TBL_L2_BITS, and TBL_L1_SHIFT are all 10
and TBL_L1_SIZE and TBL_L2_SIZE are both 1024.)

Signed-off-by: Ben Pfaff <blp@nicira.com>
Reported-by: Jesse Gross <jesse@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Clarify meaning of n_buckets argument to tbl_create().
Ben Pfaff [Tue, 28 Dec 2010 00:06:08 +0000 (16:06 -0800)]
datapath: Clarify meaning of n_buckets argument to tbl_create().

The n_buckets argument to tbl_create() can be zero, but the comment didn't
mention that.  However, there's no reason that the caller can't just pass
in a correct size, so this commit changes them to do that.

Also, TBL_L1_SIZE was conceptually wrong as the minimum size: the minimum
size is one L2 page, e.g. TBL_L2_SIZE.  But TBL_MIN_BUCKETS seems like a
better all-around way to indicate the minimum size, so this commit also
introduces that macro and uses it.

Jesse Gross pointed out inconsistencies in this area.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Do not shadow 'err' variable name in tnl_send().
Ben Pfaff [Mon, 27 Dec 2010 23:28:58 +0000 (15:28 -0800)]
datapath: Do not shadow 'err' variable name in tnl_send().

The sparse checker reported that 'err' was used for two different variables
in tnl_send().  The two variables have different types, so this patch
renames the inner one.

Jesse confirmed that the original code was correct as written.  This patch
does not change its behavior.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Suppress sparse complaints about address spaces.
Ben Pfaff [Mon, 27 Dec 2010 23:23:54 +0000 (15:23 -0800)]
datapath: Suppress sparse complaints about address spaces.

The sparse checker was complaining about incorrect address spaces (e.g.
__user versus non-__user pointers).  I looked at each of them, checked
that the code looked correct to me, and added the appropriate __force
annotations to casts.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Fix type of actions_len_left in modify_vlan_tci().
Ben Pfaff [Mon, 27 Dec 2010 23:21:29 +0000 (15:21 -0800)]
datapath: Fix type of actions_len_left in modify_vlan_tci().

The sparse checker reported that the type of the pointer passed to
nla_next(), as &actions_len_left, was incorrect: whereas the parameter
has type "int *", &actions_len_left is an "unsigned int *".  This fixes
the problem.  It is not a bug fix since the code is equally correct (or
incorrect) either way, but it gets the types right anyhow.

I don't know why GCC was not reporting this as an error.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Remove shadowed 'err' variable.
Ben Pfaff [Mon, 27 Dec 2010 23:18:37 +0000 (15:18 -0800)]
datapath: Remove shadowed 'err' variable.

sparse reported that 'err' was declared in two different places in this
function.  This patch removes the inner one.  I verified that this didn't
affect correctness either way, so this is not a bug fix.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agoImplement database schema versioning.
Ben Pfaff [Mon, 27 Dec 2010 22:26:47 +0000 (14:26 -0800)]
Implement database schema versioning.

As the database schema evolves, it might be useful to have an identifier
for the particular version in use.  This commit adds that feature.

13 years agovswitchd: Add OVS version to database, give system info its own columns.
Ben Pfaff [Mon, 27 Dec 2010 18:18:14 +0000 (10:18 -0800)]
vswitchd: Add OVS version to database, give system info its own columns.

Until now, nothing in the database has reported the Open vSwitch version
number.  This commit adds that.

In addition, this commits moves the system type and version from
external-ids to individual columns, because we decided that these were
important enough not to relegate them to a grab-bag field.

13 years agoofp-util: Improve log messages for bad Nicira extension actions.
Ben Pfaff [Thu, 23 Dec 2010 18:41:17 +0000 (10:41 -0800)]
ofp-util: Improve log messages for bad Nicira extension actions.

check_action_exact_len() will always report that a Nicira extension action
has type 65535 (OFPAT_VENDOR), which isn't very helpful for debugging.
This introduces a new function that reports the subtype.

Also, log the subtype of unknown Nicira vendor actions.

13 years agoofp-util: Upgrade log messages for controller bugs to warning level.
Ben Pfaff [Thu, 23 Dec 2010 18:36:55 +0000 (10:36 -0800)]
ofp-util: Upgrade log messages for controller bugs to warning level.

All of these messages indicate that there is a bug in the controller.  We
want to know about that, so increase the log level from debug to warning.

13 years agoofp-util: Improve log message for bad OpenFlow action length.
Ben Pfaff [Thu, 23 Dec 2010 18:36:02 +0000 (10:36 -0800)]
ofp-util: Improve log message for bad OpenFlow action length.

First, this is an important message since it indicates a bug in the
controller, so log it at warning level instead of debug level--we want to
know about it.

Second, properly byteswap the action type.

Third, use the correct PRIu16 format specified for a uint16_t.

13 years agoutilities: Implement ovs-vlan-test script
Ethan Jackson [Wed, 8 Dec 2010 02:49:28 +0000 (18:49 -0800)]
utilities: Implement ovs-vlan-test script

This patch implements a script which may be used to check for
connectivity issues caused by bugs in Linux drivers relating to
VLAN traffic.

13 years agodatapath: Don't recursively sample packets or reset their "tun_id"s.
Ben Pfaff [Thu, 23 Dec 2010 17:35:15 +0000 (09:35 -0800)]
datapath: Don't recursively sample packets or reset their "tun_id"s.

execute_actions() is called recursively when ODPAT_SET_DL_TCI adds a VLAN
header to a GSO packet, but we don't want to re-sample the sub-packet or
re-reset its tun_id, so break those two actions into a wrapper function.

This commit mostly moves code around without modifying it.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agobacktrace: Only use __builtin_frame_address if GNU C is in use.
Ben Pfaff [Thu, 16 Dec 2010 19:04:14 +0000 (11:04 -0800)]
backtrace: Only use __builtin_frame_address if GNU C is in use.

This eliminates one small GNU C dependency.

13 years agodatapath: Correct argument size for ODP_FLOW_GET.
Ben Pfaff [Thu, 23 Dec 2010 17:36:19 +0000 (09:36 -0800)]
datapath: Correct argument size for ODP_FLOW_GET.

ODP_FLOW_GET takes an odp_flowvec, not an odp_flow.

(This would merely introduce a gratuitous ABI incompatibility for the sake
of pedantic correctness, except that we're breaking the ABI regularly
anyhow.)

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agoINSTALL.Linux: Make pkg-config a requirement to build.
Ben Pfaff [Wed, 22 Dec 2010 18:02:32 +0000 (10:02 -0800)]
INSTALL.Linux: Make pkg-config a requirement to build.

pkg-config is needed to detect the presence of libssl, so it is needed even
if the build system doesn't need to be rebuilt.

In theory we'd only need pkg-config if OpenSSL is to be used, but I don't
see much point in being that explicit.

Reported-by: Bryan Osoro <bosoro@nicira.com>
13 years agodatapath: Avoid calling flow_hash() twice for the same key.
Ben Pfaff [Fri, 17 Dec 2010 22:58:52 +0000 (14:58 -0800)]
datapath: Avoid calling flow_hash() twice for the same key.

This is a small optimization for the case where a new flow is being added
to the flow table.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agoflow: Increase number of registers to 4.
Ben Pfaff [Fri, 17 Dec 2010 00:00:02 +0000 (16:00 -0800)]
flow: Increase number of registers to 4.

Requested-by: Teemu Koponen <koponen@nicira.com>
13 years agoImplement a new Nicira extension action for multipath link selection.
Ben Pfaff [Fri, 17 Dec 2010 22:38:50 +0000 (14:38 -0800)]
Implement a new Nicira extension action for multipath link selection.

13 years agonx-match: Rename and export functions for parsing and formatting subfields.
Ben Pfaff [Thu, 16 Dec 2010 21:57:21 +0000 (13:57 -0800)]
nx-match: Rename and export functions for parsing and formatting subfields.

These will be used from other source files in upcoming commits.

13 years agonx-match: Use [] in format_nxm_field_bits where possible.
Ben Pfaff [Thu, 16 Dec 2010 19:12:03 +0000 (11:12 -0800)]
nx-match: Use [] in format_nxm_field_bits where possible.

This just makes formatting a bit more readable.

13 years agonx-match: Make nxm_field_bytes(), nxm_field_bits() public.
Ben Pfaff [Thu, 16 Dec 2010 21:57:08 +0000 (13:57 -0800)]
nx-match: Make nxm_field_bytes(), nxm_field_bits() public.

13 years agonx-match: New helper functions for working with ofs_nbits in NXM actions.
Ben Pfaff [Thu, 16 Dec 2010 21:56:47 +0000 (13:56 -0800)]
nx-match: New helper functions for working with ofs_nbits in NXM actions.

13 years agonicira-ext: Convert all "uintNN_t"s to "ovs_beNN"s.
Ben Pfaff [Wed, 15 Dec 2010 20:11:26 +0000 (12:11 -0800)]
nicira-ext: Convert all "uintNN_t"s to "ovs_beNN"s.

13 years agohash: Make functions for hashing a few words easier to use.
Ben Pfaff [Thu, 16 Dec 2010 21:53:29 +0000 (13:53 -0800)]
hash: Make functions for hashing a few words easier to use.

It's easier for the client to pass in data directly than it is for the
client to stuff it into an array.

These functions will have new users in upcoming commits.

13 years agoodp-util: Correct length check in format_odp_action().
Jesse Gross [Thu, 16 Dec 2010 22:27:47 +0000 (14:27 -0800)]
odp-util: Correct length check in format_odp_action().

When printing the action list we first check that the size of the
action matches the expected length for that type.  However, when
doing the lookup we were passing in the length of the action, not
the type, leading to bogus values.

13 years agodatapath: Harmonize [get|set]_skb_csum_pointers().
Jesse Gross [Thu, 16 Dec 2010 00:50:40 +0000 (16:50 -0800)]
datapath: Harmonize [get|set]_skb_csum_pointers().

The functions to get and set the checksum pointers consistently across
different kernel versions had different interpretations of what the
csum_offset pointer was relative to, which is confusing, to say the least.
This makes the meaning be the same as skb->csum_offset in modern kernels
and updates the caller.  For a given function the results were consistent
across kernel versions and the callers knew what the meaning should be, so
this doesn't actually fix any bugs.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agotunneling: Refresh IP header pointer after update_header().
Jesse Gross [Wed, 15 Dec 2010 23:38:06 +0000 (15:38 -0800)]
tunneling: Refresh IP header pointer after update_header().

We were assuming that the call to update_header() to finalize tunnel
headers wouldn't cause the skb linear data area to be reallocated.
So far this hasn't been a problem but it's not, generally speaking,
a good assumption to make.  Therefore, refetch the pointer to the IP
header instead of carrying it across the call.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agoofp-print: Improve formatting for bad OpenFlow messages.
Ben Pfaff [Tue, 14 Dec 2010 00:27:20 +0000 (16:27 -0800)]
ofp-print: Improve formatting for bad OpenFlow messages.

13 years agoofp-print: Always print priority exactly once.
Ben Pfaff [Tue, 14 Dec 2010 19:36:04 +0000 (11:36 -0800)]
ofp-print: Always print priority exactly once.

Reported-by: Paul Ingram <paul@nicira.com>
13 years agoclassifier: Insert commas after fields that lacked them in cls_rule_format().
Ben Pfaff [Tue, 14 Dec 2010 00:18:58 +0000 (16:18 -0800)]
classifier: Insert commas after fields that lacked them in cls_rule_format().

13 years agoclassifier: Use OFP_DEFAULT_PRIORITY instead of literal 32768.
Ben Pfaff [Fri, 10 Dec 2010 17:20:49 +0000 (09:20 -0800)]
classifier: Use OFP_DEFAULT_PRIORITY instead of literal 32768.

13 years agoofp-util: Improve error log messages.
Ben Pfaff [Tue, 14 Dec 2010 00:20:24 +0000 (16:20 -0800)]
ofp-util: Improve error log messages.

13 years agoofp-util: Use proper format specifier for uint32_t in ofputil_lookup_openflow_message().
Ben Pfaff [Tue, 14 Dec 2010 00:20:06 +0000 (16:20 -0800)]
ofp-util: Use proper format specifier for uint32_t in ofputil_lookup_openflow_message().

13 years agoofproto: Always use xid 0 for *_FLOW_REMOVED messages.
Ben Pfaff [Tue, 14 Dec 2010 00:25:53 +0000 (16:25 -0800)]
ofproto: Always use xid 0 for *_FLOW_REMOVED messages.

Asynchronous messages are never part of a request/reply pair so it doesn't
make sense to allocate a xid, which could confuse the controller anyhow.

13 years agoofproto: Fix encoding of NXST_* replies.
Ben Pfaff [Tue, 14 Dec 2010 00:21:43 +0000 (16:21 -0800)]
ofproto: Fix encoding of NXST_* replies.

This only matter for NXST_AGGREGATE currently since NXST_FLOW has value 0.

13 years agoofp-util: Fix encoding of NXST_AGGREGATE requests.
Ben Pfaff [Tue, 14 Dec 2010 00:20:54 +0000 (16:20 -0800)]
ofp-util: Fix encoding of NXST_AGGREGATE requests.

They were being sent out as NXST_FLOW requests.

13 years agoofproto: Fix write-after-free error in compose_nx_flow_removed().
Ben Pfaff [Wed, 15 Dec 2010 17:48:16 +0000 (09:48 -0800)]
ofproto: Fix write-after-free error in compose_nx_flow_removed().

13 years agodatapath: Correctly return error if percpu allocation fails.
Jesse Gross [Mon, 13 Dec 2010 23:21:28 +0000 (15:21 -0800)]
datapath: Correctly return error if percpu allocation fails.

If the allocation of percpu stats fails when creating a new
datapath, we currently don't return the correct error code.  Since
we don't explicitly set it when the allocation fails it will keep
the value from the previous call.  This means we will return success
when the creation actually failed.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agoMakefile: Check for undistributed files on every make, not just "make dist".
Ben Pfaff [Mon, 13 Dec 2010 22:32:55 +0000 (14:32 -0800)]
Makefile: Check for undistributed files on every make, not just "make dist".

It's really easy to add files to the Git repository but forget to add them
to the distributions created by "make dist".  I do this regularly, for
example.  For some time, we've had a check that runs on "make dist" to
make sure that the distribution is complete, but I still screw up because
I don't run "make dist" all that often.

This commit improves the situation, by doing the check on every "make",
instead of just on "make dist".

13 years agoovs-vswitchd: Release most memory on normal exit.
Ben Pfaff [Mon, 13 Dec 2010 21:08:31 +0000 (13:08 -0800)]
ovs-vswitchd: Release most memory on normal exit.

This makes "valgrind --leak-check=full --show-reachable=yes" output much
easier to read.

13 years agonetdev-linux: Fix pairing of rtnetlink register and unregister calls.
Ben Pfaff [Mon, 13 Dec 2010 21:07:48 +0000 (13:07 -0800)]
netdev-linux: Fix pairing of rtnetlink register and unregister calls.

netdev_linux_create() called rtnetlink_notifier_register() for both system
and internal devices, but netdev_linux_destroy() only did the reverse
accounting for system devices.  This fixes the pairing.

This isn't really much of a bug, since it would only cause the notifier to
be active unnecessarily (not to be removed even though it was needed).  At
most it was a missed opportunity for optimization, but I don't think that
optimization would ever happen anyway.

Found with valgrind --leak-check=full --show-reachable=yes.

13 years agovswitchd: Delete DP_MAX_PORTS.
Ben Pfaff [Mon, 13 Dec 2010 20:20:12 +0000 (12:20 -0800)]
vswitchd: Delete DP_MAX_PORTS.

This is no longer used.

13 years agovswitchd: Fix dependency on DP_MAX_PORTS for allocating "struct dst"s.
Ben Pfaff [Mon, 13 Dec 2010 22:28:53 +0000 (14:28 -0800)]
vswitchd: Fix dependency on DP_MAX_PORTS for allocating "struct dst"s.

Until now, compose_actions() has allocated enough "struct dst"s on the
stack for a worst-case flow, one that floods packets with the maximum
number of ports and mirrors.  When the code was written this was correct.
However, now the number of ports is no longer known at compile time.  The
maximum number, 65535, would require (65536 * (32 + 1) * 4) == 8 MB of
stack space, which is a lot.  So this commit fixes the problem a different
way, by allocating the "struct dst"s dynamically when necessary.

This is a bug fix, but not a very serious one, because it could only
become a buffer overflow with a large number of mirrors.

13 years agobridge: Eliminate bond_rebalance_port() dependency on DP_MAX_PORTS.
Ben Pfaff [Mon, 13 Dec 2010 19:12:37 +0000 (11:12 -0800)]
bridge: Eliminate bond_rebalance_port() dependency on DP_MAX_PORTS.

There's no reason to allocate the bals[] array on the stack here, since
this is not on any fast-path.

As an alternative, we could limit the number of interfaces on a single
bond to some reasonable maximum, such as 8 or 32, but this commit's change
is simpler.

13 years agoofproto: Fix use-after-free error in facet_revalidate().
Ben Pfaff [Mon, 13 Dec 2010 20:25:01 +0000 (12:25 -0800)]
ofproto: Fix use-after-free error in facet_revalidate().

Found by valgrind.

13 years agodatapath: Validate lock when handling flow actions.
Jesse Gross [Wed, 8 Dec 2010 19:36:57 +0000 (11:36 -0800)]
datapath: Validate lock when handling flow actions.

When reading actions without rcu_read_lock we need to hold the
datapath lock.  This checks that using lockdep.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Check locks on access to flow table.
Jesse Gross [Wed, 8 Dec 2010 19:32:05 +0000 (11:32 -0800)]
datapath: Check locks on access to flow table.

When accessing the flow table without holding rcu_read_lcok
we need to hold the lock on the datapath.  This enables lockdep
to validate that that is the case.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Validate access to DP array.
Jesse Gross [Wed, 8 Dec 2010 19:07:56 +0000 (11:07 -0800)]
datapath: Validate access to DP array.

When access the array of DPs, we need to hold either rcu_read_lock
or dp_mutex.  This enables lockdep to validate those conditions.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agotunneling: Add checks for header cache lock.
Jesse Gross [Sun, 5 Dec 2010 20:36:36 +0000 (12:36 -0800)]
tunneling: Add checks for header cache lock.

When updating the tunnel header cache, we need to hold a lock to
protect against concurrent access.  This adds annotations to
make sparse happy when we access the data without rcu_read_lock
and enables lockdep to verify that we have the correct lock.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Convert rcu_dereference() to correct variant.
Jesse Gross [Mon, 6 Dec 2010 23:15:47 +0000 (15:15 -0800)]
datapath: Convert rcu_dereference() to correct variant.

Using rcu_dereference() makes lockdep complain if rcu_read_lock
is not held.  This is OK if the update side lock is held.  This
adds checks to see if RTNL lock is held, if that is also a
correct form of protection.  Alternately, it enforces that RTNL
must be held.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Don't directly access RCU protected pointers.
Jesse Gross [Sun, 5 Dec 2010 19:22:04 +0000 (11:22 -0800)]
datapath: Don't directly access RCU protected pointers.

If RTNL lock is used to protected updates to RCU data structures
then it isn't necessary to use rcu_dereference() to access them if
RTNL is held.  This adds rtnl_dereference() to access these pointers
which has several benefits: documents the locking expectations;
checks that RTNL actually is held when run with lockdep; makes
sparse not complain about directly accessing RCU pointers.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Correct byte order annotations.
Jesse Gross [Sat, 4 Dec 2010 20:04:39 +0000 (12:04 -0800)]
datapath: Correct byte order annotations.

We have generally been using the byte order specific data types
(i.e. __be32 instead of u32) in most places.  This corrects a
declaration and adds a few needed casts.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Add usage of __rcu annotation.
Jesse Gross [Sat, 4 Dec 2010 19:50:53 +0000 (11:50 -0800)]
datapath: Add usage of __rcu annotation.

Sparse can warn about incorrect usage of RCU via direct access to
points when used in conjuction with __rcu and CONFIG_SPARSE_RCU.
This adds the necessary annotations.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Add usage of __percpu annotation.
Jesse Gross [Sat, 4 Dec 2010 19:39:53 +0000 (11:39 -0800)]
datapath: Add usage of __percpu annotation.

Sparse can warn if percpu pointers are incorrectly directly
dereference.  This adds the annotation where we declare percpu
pointers.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Correct usage of __user annotation.
Jesse Gross [Sat, 4 Dec 2010 19:17:26 +0000 (11:17 -0800)]
datapath: Correct usage of __user annotation.

We generally have been using the __user annotation but there were
a few places where it was missing or needed a cast.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Compatibility code for RCU check functions.
Jesse Gross [Mon, 6 Dec 2010 23:39:19 +0000 (15:39 -0800)]
datapath: Compatibility code for RCU check functions.

The rcu_dereference_rtnl() and rtnl_dereference() functions will
be introduced in 2.6.37.  They provide nice documentation of
locking expectations as well as checking on recent kernels.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Add compatibility code for sparse annotations.
Jesse Gross [Sat, 20 Nov 2010 01:48:04 +0000 (17:48 -0800)]
datapath: Add compatibility code for sparse annotations.

The __percpu and __rcu annotations for sparse are relatively
recent additions, so provide no-op definitions on older kernels.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Use __packed macro.
Jesse Gross [Sun, 12 Dec 2010 07:29:22 +0000 (23:29 -0800)]
datapath: Use __packed macro.

The __packed macro is preferred instead of an explicit GCC attribute,
so use it instead to deal with structure packing.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Compatibility code for __packed macro.
Jesse Gross [Sun, 12 Dec 2010 07:28:33 +0000 (23:28 -0800)]
datapath: Compatibility code for __packed macro.

The __packed macro for structure packing wasn't introduced until 2.6.24,
so define it ourselves.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agonicira-ext: Correct and extend examples for NXM_OF_VLAN_TCI field.
Ben Pfaff [Tue, 23 Nov 2010 18:15:43 +0000 (10:15 -0800)]
nicira-ext: Correct and extend examples for NXM_OF_VLAN_TCI field.

The final example for this field was wrong.  This corrects it and adds
two more examples.

Reported-by: Natasha Gude <natasha@nicira.com>
13 years agodatapath-protocol: Include netlink.h.
Jesse Gross [Sun, 12 Dec 2010 18:01:19 +0000 (10:01 -0800)]
datapath-protocol: Include netlink.h.

On older kernels that don't have if_link.h, we use our own, limited
version.  This version doesn't include the netlink header, causing
problems where we were relying on it to define the types in
datapath-protocol.h.  Therefore, directly include it, since it is
better to be explicit about it anyways.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agopinsched: Avoid uninitialized variable warning.
Jesse Gross [Sun, 12 Dec 2010 17:54:46 +0000 (09:54 -0800)]
pinsched: Avoid uninitialized variable warning.

Some compilers warn about the variable 'n_longest' in drop_packet()
being used uninitialized.  This isn't actually possible but explicitly
set it to zero to avoid spurious warnings.

13 years agonx-match: Use correct printf format specifiers.
Jesse Gross [Sun, 12 Dec 2010 06:53:34 +0000 (22:53 -0800)]
nx-match: Use correct printf format specifiers.

A few of the printf format specifiers didn't match the type that
they were printing.  On 32-bit platforms there is some overlap
but on 64-bit they cause a mismatch.

13 years agovswitchd: Consistently use size_t for action lengths.
Jesse Gross [Sun, 12 Dec 2010 06:51:31 +0000 (22:51 -0800)]
vswitchd: Consistently use size_t for action lengths.

Currently the type of the datapath action length is mixture of
size_t and unsigned int.  However, size_t is really defined as an
unsigned long, which causes the build to fail on 64-bit platforms.
This consistently uses size_t.

13 years agoflow: Make size of flow struct a multiple of 8.
Jesse Gross [Sun, 12 Dec 2010 01:31:36 +0000 (17:31 -0800)]
flow: Make size of flow struct a multiple of 8.

The compiler wants to pad structures to a multiple of the native
datatype for the architecture, so a multiple of 4 on 32-bit platforms
and a multiple of 8 on 64-bit.  Currently the size struct flow is
a multiple of 4, so the total size with padding varies depending on
the architecture, causing build asserts to fail.  This explicitly pads
it out to a multiple of 8 for consistency.

13 years agodatapath: Remove explicit 'unlikely' from IS_ERR calls.
Ben Pfaff [Sat, 11 Dec 2010 00:41:33 +0000 (16:41 -0800)]
datapath: Remove explicit 'unlikely' from IS_ERR calls.

As David Miller pointed out on netdev today, IS_ERR has a built-in
'unlikely', so there's no point in adding one of our own.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Introduce more compat support for <net/netlink.h>.
Ben Pfaff [Mon, 13 Dec 2010 18:19:46 +0000 (10:19 -0800)]
datapath: Introduce more compat support for <net/netlink.h>.

With this commit, I have successfully built the datapath, without warnings,
on 2.6.{18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,36} on i386,
2.6.31 on x86-64, and the kernels included with XenServer 5.5.0 and (some
prerelease kernel for) XenServer 5.6.0.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Add compat support for nla_type().
Ben Pfaff [Fri, 10 Dec 2010 22:42:17 +0000 (14:42 -0800)]
datapath: Add compat support for nla_type().

The datapath code uses nla_type() but it was only introduced in 2.6.24.

The NLA_TYPE_MASK definition has to go above the #include <net/netlink.h>
because <net/netlink.h> recursively #include <linux/netlink.h>.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Include <linux/skbuff.h> directly into linux/ip.h compat.
Ben Pfaff [Fri, 10 Dec 2010 22:39:25 +0000 (14:39 -0800)]
datapath: Include <linux/skbuff.h> directly into linux/ip.h compat.

While doing test builds on numerous kernel versions I found that one build
failed because skb_network_header() wasn't visible from flow.h.  I guess
that we accidentally depend on <linux/netlink.h> being included indirectly,
but this didn't always happen.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Include <linux/netlink.h> directly into flow.h.
Ben Pfaff [Fri, 10 Dec 2010 22:38:25 +0000 (14:38 -0800)]
datapath: Include <linux/netlink.h> directly into flow.h.

While doing test builds on numerous kernel versions I found that one build
failed because "struct nlattr" wasn't visible from flow.h.  I guess that
we accidentally depend on <linux/netlink.h> being included indirectly, but
this didn't always happen.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Fix off-by-one error in dev_get_stats() compat code.
Ben Pfaff [Fri, 10 Dec 2010 22:45:38 +0000 (14:45 -0800)]
datapath: Fix off-by-one error in dev_get_stats() compat code.

dev_get_stats() was introduced in 2.6.29, not 2.6.28.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agodatapath: Fix csum_replace4() compatibility implementation.
Ben Pfaff [Fri, 10 Dec 2010 22:13:17 +0000 (14:13 -0800)]
datapath: Fix csum_replace4() compatibility implementation.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
13 years agoutilities: ovs-tcpdump references non-existent exception
Ethan Jackson [Sat, 11 Dec 2010 23:24:40 +0000 (15:24 -0800)]
utilities: ovs-tcpdump references non-existent exception

ovs-tcpdump would not behave properly when users attempted to pass
invalid arguments.

13 years agodatpath: Fix memory leak when a loop is detected.
Jesse Gross [Fri, 10 Dec 2010 00:40:15 +0000 (16:40 -0800)]
datpath: Fix memory leak when a loop is detected.

If we detect a packet that is looping we kill the flow but then
don't do anything with the packet that caused the problem in the
first place, so this frees the packet.  This isn't a very serious
leak because we try to shut off the flow that lead to the loop
as early as possible.  Once this happens, packets will no longer
hit the loop detector and will be freed just as any other packet
that should be dropped.

It also fixes an issue where the offset to the stats counter is
uninitialized after a loop is detected.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapth: Drop check for impossible condition after skb_gso_segment().
Jesse Gross [Thu, 9 Dec 2010 07:55:20 +0000 (23:55 -0800)]
datapth: Drop check for impossible condition after skb_gso_segment().

It's possible for skb_gso_segment to return NULL but only if the
hardware supports the correct form of segmentation offload but just
wants software to verify the offload parameters.  However, since we're
not hardware and don't support any kind of segmentation offload natively,
we can never get in this situation.  Therefore drop the check and
comment.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Drop synchronize_rcu() in internal dev destroy.
Jesse Gross [Thu, 9 Dec 2010 07:29:10 +0000 (23:29 -0800)]
datapath: Drop synchronize_rcu() in internal dev destroy.

unregister_netdevice() contains a call to synchronize_rcu(), so there
is no need to directly call it ourselves immediately beforehand.
We were relying on the call during unregistration anyways to stop
packets from being transmited on the device, so our version was
both misleading and had a performance penalty.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Take advantage of IFF_OVS_DATAPATH.
Jesse Gross [Thu, 9 Dec 2010 03:28:32 +0000 (19:28 -0800)]
datapath: Take advantage of IFF_OVS_DATAPATH.

Starting in 2.6.37 we have our own unique identifier to be able
to find ports attached to OVS.  Take advantage of it to avoid
ugly workarounds.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agodatapath: Don't use RCU for internal dev vport.
Jesse Gross [Thu, 9 Dec 2010 03:21:40 +0000 (19:21 -0800)]
datapath: Don't use RCU for internal dev vport.

The vports are now attached and ready to go when they are allocated,
so we don't have to worry about future changes.  As a result, we can
directly store the pointer in the internal dev's netdevice private
space before it is registered.  The registration process will handle
the necessary write memory barriers and anyone who has a reference
to the netdev will have done the read side barriers, we don't need
to use RCU at all.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
13 years agoofproto: Fix problem that caused facets not to be installed into datapath.
Justin Pettit [Sat, 11 Dec 2010 04:50:58 +0000 (20:50 -0800)]
ofproto: Fix problem that caused facets not to be installed into datapath.

Commit cdee00f (datapath: Replace "struct odp_action" by Netlink
attributes.) stopped initializing some elements in facet structures
in certain cases.  This caused flows to not be installed into the datapath.
This commit sets that again based on the action context.

13 years agoExpand tunnel IDs from 32 to 64 bits.
Ben Pfaff [Fri, 10 Dec 2010 18:42:42 +0000 (10:42 -0800)]
Expand tunnel IDs from 32 to 64 bits.

We have a need to identify tunnels with keys longer than 32 bits.  This
commit adds basic datapath and OpenFlow support for such keys.  It doesn't
actually add any tunnel protocols that support 64-bit keys, so this is not
very useful yet.

The 'arg' member of struct odp_msg had to be expanded to 64-bits also,
because it sometimes contains a tunnel ID.  This member also contains the
argument passed to ODPAT_CONTROLLER, so I expanded that action's argument
to 64 bits also so that it can use the full width of the expanded 'arg'.
Userspace doesn't take advantage of the new space though (it was only
using 16 bits anyhow).

This commit has been tested only to the extent that it doesn't disrupt
basic Open vSwitch operation.  I have not tested it with tunnel traffic.

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

13 years agoofp-util: Make ofputil_cls_rule_to_match() help with flow cookies too.
Ben Pfaff [Thu, 9 Dec 2010 22:19:51 +0000 (14:19 -0800)]
ofp-util: Make ofputil_cls_rule_to_match() help with flow cookies too.

This fixes OpenFlow 1.0 flow stats reporting of flows added via NXM.

I noticed this problem while implementing 64-bit tunnel IDs, hence the
positioning.  The following commit adds a test.

Acked-by: Jesse Gross <jesse@nicira.com>
13 years agoofproto: Format entire rule when dumping all flows.
Ben Pfaff [Thu, 9 Dec 2010 22:16:56 +0000 (14:16 -0800)]
ofproto: Format entire rule when dumping all flows.

cls_rule_format() formats the entire classifier rule, whereas
ofp_print_match() just shows the parts that are visible in OpenFlow 1.0.

Acked-by: Jesse Gross <jesse@nicira.com>