Justin Pettit [Wed, 15 May 2013 01:24:43 +0000 (18:24 -0700)]
ofproto-dpif: Store relevant fields for wildcarding in facet.
Dynamically determines the flow fields that were relevant in
processing flows based on the OpenFlow flow table and switch
configuration. The immediate use for this functionality is to
cache action translations for similar flows in facets. This yields
a roughly 80% improvement in flow set up rates for a complicated
flow table.
More importantly, these wildcards will be used to determine what to
wildcard for the forthcoming kernel wildcard (megaflow) patches
that will allow wildcarding in the kernel, which will provide
significant flow set up improvements.
The approach to tracking fields and caching action translations in
facets was based on an impressive prototype by Ethan Jackson.
Co-authored-by: Ethan Jackson <ethan@nicira.com> Signed-off-by: Ethan Jackson <ethan@nicira.com> Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ethan Jackson [Tue, 11 Jun 2013 05:48:58 +0000 (22:48 -0700)]
flow: Add new wildcard functions.
Rename the function flow_wildcards_combine() to flow_wildcards_and().
Add new flow_wildcards_or() and flow_hash_in_wildcards() functions.
These will be useful in a future patch.
Signed-off-by: Ethan Jackson <ethan@nicira.com> Signed-off-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Mon, 10 Jun 2013 18:26:03 +0000 (11:26 -0700)]
ofproto-dpif: Fix incorrect printf format specifiers.
cc1: warnings being treated as errors
../ofproto/ofproto-dpif.c: In function ‘dpif_show_backer’:
../ofproto/ofproto-dpif.c:8305: error: format ‘%llu’ expects type ‘long
long unsigned int’, but argument 4 has type ‘size_t’
../ofproto/ofproto-dpif.c:8305: error: format ‘%llu’ expects type ‘long
long unsigned int’, but argument 5 has type ‘size_t’
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Justin Pettit <jpettit@nicira.com>
Ben Pfaff [Tue, 16 Apr 2013 22:13:21 +0000 (15:13 -0700)]
ofp-util: Don't return static data in ofputil_packet_in_reason_to_string().
Returning a static data buffer makes code more brittle and definitely
not thread-safe, so this commit switches to using a caller-provided
buffer instead.
Ben Pfaff [Mon, 15 Apr 2013 22:40:21 +0000 (15:40 -0700)]
odp-util: Avoid returning static data in ovs_key_attr_to_string().
Returning a static data buffer makes code more brittle and definitely
not thread-safe, so this commit switches to using a caller-provided
buffer instead.
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Ansis Atteka <aatteka@nicira.com>
Ben Pfaff [Mon, 10 Jun 2013 17:25:29 +0000 (10:25 -0700)]
ovsdb-idlc: Write a new-line at the end of "annotate" output.
Some tools do not like text files that lack a trailing new-line. In
particular, Debian's dpkg-source utility complains about a missing new-line
in the file generated by ovsdb-idlc:
dpkg-source: warning: file
openvswitch-1.9.2+git20130605/lib/vswitch-idl.ovsidl has no final
newline (either original or modified version)
This commit fixes the problem.
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Fri, 7 Jun 2013 20:51:31 +0000 (13:51 -0700)]
ofproto-dpif-governor: Dispense with governor name.
In almost all cases, ovs-vswitchd runs with a single datapath, and
therefore a single governor. Therefore, it's usually clear from
context what governor log messages are referring to making the name
redundant. This patch removes it.
Ethan Jackson [Wed, 1 May 2013 01:32:02 +0000 (18:32 -0700)]
ofproto-dpif: Maintain subfacets in dpif_backer.
Conceptually, a subfacet represents a datapath flow key, and
therefore belongs more to a datapath more than it does to a bridge.
This patch moves the subfacet hmap from 'struct ofproto_dpif' to
'struct dpif_backer', simplifying the code in the process.
Ethan Jackson [Tue, 4 Jun 2013 20:22:46 +0000 (13:22 -0700)]
ofproto-dpif: Set flow-eviction-threshold globally.
With the single datapath, it no longer makes sense to have a per
ofproto flow eviction threshold. This patch moves the flow
eviction threshold to the Open_vSwitch table making the setting
global, though still treated separately for each ofproto. A future
patch will unify flow eviction on a per datapath basis.
Ethan Jackson [Tue, 4 Jun 2013 22:48:31 +0000 (15:48 -0700)]
ofproto: Track subfacet stats in the backer.
Subfacets being per-datapath entities, their statistics are really
only interesting at per-datapath granularity. This patch moves
them to the dpif_backer and makes some related simplifications.
Ethan Jackson [Thu, 30 May 2013 20:22:26 +0000 (13:22 -0700)]
ofproto-dpif: More responsive average subfacet statistics.
Before this patch, statistics about subfacet average life span,
and count in the datapath, were calculated over the entire lifetime
of the ofproto. Furthermore, the subfacet lifespan was only
updated when a subfacet was removed from the datapath causing long
lived subfacets to be ignored. It's far more useful to know these
numbers averaged over all subfacets in the recent past. This patch
changes the code to implement an exponentially weighted moving
average updated every time statistics are pulled from the datapath.
These functions will not have the same useful effect when Open vSwitch
becomes multithreaded, because time_disable_restart() will disable time
advancing for every thread, not just for the thread that calls it.
These functions are no longer used, so this commit removes them.
Ben Pfaff [Wed, 1 May 2013 18:05:28 +0000 (11:05 -0700)]
netdev-vport: Don't return static data in netdev_vport_get_dpif_port().
Returning a static data buffer makes code more brittle and definitely
not thread-safe, so this commit switches to using a caller-provided
buffer instead.
Ben Pfaff [Thu, 6 Jun 2013 22:27:15 +0000 (15:27 -0700)]
dpif-netdev: Don't run port names through netdev_vport_get_dpif_port().
The ports that exist within a dpif have already been translated through
netdev_vport_get_dpif_port(), so there is no value to translating them
again in the interfaces that query or dump ports (and possibly a drawback
if somehow the translation could change).
After this change, dpif-netdev translates port names in just one place,
the port_add path, which makes dpif-netdev act the same way as dpif-linux
in this respect.
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Ethan Jackson <ethan@nicira.com>
Simon Horman [Wed, 5 Jun 2013 13:28:47 +0000 (13:28 +0000)]
ofproto-dpif: Actually log errors in facet_check_consistency()
facet_check_consistency() goes to some effort to create informative
error messages, protected by a rate limit, but does not actually
log these messages.
This appears to be an unintended side effect of 4dff90977694784e67e9c08cc72dee28ebc343ae ("ofproto-dpif: Move
odp_actions from subfacet to facet."). This patch restores the
logging behaviour present prior to the above commit.
This patch also reverses a changes made by the same commit to use
ds_put_cstr() instead of ds_put_char() in facet_check_consistency().
Cc: Justin Pettit <jpettit@nicira.com> Cc: Ethan Jackson <ethan@nicira.com> Signed-off-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ethan Jackson <ethan@nicira.com>
Simon Horman [Wed, 5 Jun 2013 05:28:49 +0000 (14:28 +0900)]
odp-execute: Add set skb_mark, set_priority, tunnel support.
The motivation for this is to allow such actions to be honoured
if they are encountered; by the user-space datapath before recirculation;
or by internal processing of actions by ovs-vswitchd before recirculation.
Recirculation will be added by a subsequent patch.
Signed-off-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
Jing Ai [Wed, 5 Jun 2013 20:18:09 +0000 (13:18 -0700)]
ofp-actions: enforce valid range for table_id in goto_table instruction
Found a bug that OVS allows goto_table_id to be smaller than (or equal to)
the current table id where the flow resides. It potentially creates an
infinite loop when composing actions for a packet. To fix it, we just let
OVS returns an error message to prevent such flow to be programmed.
Signed-off-by: Jing Ai <jinga@google.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ed Maste [Wed, 5 Jun 2013 20:22:20 +0000 (16:22 -0400)]
lib: Restore build on FreeBSD
It appears that the size_t definition was supplied through header leakage
on other platforms after commit eee8089cbeffa39eef3858be57628be663b229b7
(signals: Make signal_name() thread-safe.).
Signed-off-by: Ed Maste <emaste@freebsd.org> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Wed, 5 Jun 2013 17:11:55 +0000 (10:11 -0700)]
ofproto-dpif: Do not give stats to rules bypassed by "drop" frag policy.
When the OFPC_FRAG_DROP policy is in effect, IP fragments are supposed to
be dropped before they reach the flow table. Open vSwitch properly dropped
IP fragments in this case, but still accounted them to the packet and byte
counters for the flow that they would have hit if the OFPC_FRAG_NX_MATCh
policy had been in effect.
Reported-by: love you <thunder.love07@gmail.com> Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Ethan Jackson <ethan@nicira.com>
avoid using some GNU sed extensions, which are not available
eg. on NetBSD's /usr/bin/sed.
while there are other places these extenstions are used in the testsuite,
this instance is new and seems the only one currently being executed
for NetBSD.
Signed-off-by: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp> Signed-off-by: Ben Pfaff <blp@nicira.com>
Alex Wang [Tue, 4 Jun 2013 19:46:38 +0000 (12:46 -0700)]
bridge.c: Change variable "ofport" type in "struct if_cfg" and "struct iface"
This patch changes the variable type of "ofport" in "struct if_cfg" and
"struct iface" from int64_t to uint16_t. This is more consistent with
the OpenFlow-1.0 port definition.
Also, before this patch, -1 is used to indicate an unknown port. This
patch uses OFPP_NONE, since "ofport" becomes uint16_t.
Signed-off-by: Alex Wang <alexw@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Simon Horman [Mon, 3 Jun 2013 05:46:30 +0000 (14:46 +0900)]
Always use valid ids pointer in dec_ttl_cnt_ids_from_openflow()
Always update the ids pointer after calling ofpbuf_put()
to ensure that it is valid when accessed.
During testing a case came up where the call to ofpbuf_put() in the
for (i = 0; i < ids->n_controllers; i++) loop would cause the underlying
buffer to be reallocated. This resulted in ids->n_controllers being an
incorrect value, the loop continuing on longer than desired and finally a
segmentation fault.
Reported-by: Joe Stringer <joe@wand.net.nz> Signed-off-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
Alex Wang [Fri, 31 May 2013 01:24:44 +0000 (18:24 -0700)]
ofproto-dpif.c: Modify vsp_realdev_to_vlandev() function
Commit 52a90c29 (Implement new "VLAN splinters" feature) passed in OpenFlow
port number to vsp_realdev_to_vlandev() function which asks for datapath port
number.
This patch fixes this bug by making the vsp_realdev_to_vlandev() function
take in and return OpenFlow port number.
Signed-off-by: Alex Wang <alexw@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
ovs-ctl.in: Restore flows with vswitchd packet processing disabled.
It has been observed that when we upgrade openvswitch with
thousands of openflow flows already established and there is heavy
traffic going through the switch, restoring flows can take a very
long time (8 minutes in one use case) with CPU running at 100%.
This can make the upgrade very expensive.
This commit starts vswitchd by asking it to wait till flow restore is
finished. Once the flows are restored, it resumes normal operations.
ovs-vswitchd: An option to wait for userspace flow restore to complete.
While upgrading openvswitch, it helps to restore openflow flows before
starting packet processing. Typically we want to restart openvswitch,
add the openflow flows and then start packet processing.
To do this, we look for the other_config:flow-restore-wait column
in the Open_vSwitch table during startup. If set as true, we disable
receiving packets from the datapath, expiring or flushing flows and
running any periodic ofproto activities. This option does not prevent
the addition and deletion of ports. Once this option is set to false,
we return to normal processing.
An upcoming commit will use this feature in Open vSwitch startup scripts.
Ethan Jackson [Wed, 29 May 2013 19:38:48 +0000 (12:38 -0700)]
ofproto-dpif: Consolidate facet stat logic.
The logic for updating statistics at the facet level had been
spread through ofproto-dpif in a rather confusing manner. This
patch consolidates as much of this logic as is reasonable into
facet_push_stats().
On a side note, I'd expect this patch to have a marginal positive
performance impact when using learning (though I haven't bothered
to measure it). It combines facet_learn() and facet_push_stats()
into one step allowing us to avoid a redundant xlate_actions().
Ethan Jackson [Wed, 29 May 2013 23:19:20 +0000 (16:19 -0700)]
ofproto-dpif: Fix facet_lookup_valid().
facet_lookup_valid() attempted to re-revalidate its facet after
it had been removed. Added very recently in commit 5bf64ad
(ofproto-dpif: Avoid redundant facet_find() in
facet_lookup_valid()).
Found by inspection (by Justin).
Reported-by: Justin Pettit <jpettit@nicira.com> Signed-off-by: Ethan Jackson <ethan@nicira.com>
Jesse Gross [Sat, 25 May 2013 00:01:34 +0000 (17:01 -0700)]
ofproto-dpif: Don't count misses in OpenFlow table stats.
Originally no rule existed for packets that did not match an
OpenFlow flow and therefore every packet with a rule could be
counted as a hit. However, newer versions of OVS have hidden
miss rules so this is no longer true. To return the correct
table stats, this subtracts packets that hit the miss rule
from the total and removes the separate counter.
Reported-by: love you <thunder.love07@gmail.com> Signed-off-by: Jesse Gross <jesse@nicira.com>
Ethan Jackson [Mon, 13 May 2013 18:18:21 +0000 (11:18 -0700)]
ofproto-dpif: Revamp xlate_actions() interface.
This patch implements a new interface to xlate_actions which, aside
from being simpler and more intuitive, achieves several goals. It
pulls all of xlate_actions() results into a single structure which
will be easier to cache and pass around in future. And it shields
xlate_ctx from the rest of the code, making it possible to factor
xlate_actions() in the future (hopefully).
Ethan Jackson [Mon, 13 May 2013 22:31:31 +0000 (15:31 -0700)]
ofproto-dpif: Rename action_xlate_ctx.
This patch changes the name of action_xlate_ctx to xlate_ctx. Aside
from being a bit snappier, it fits more cleanly with structures
added in future patches.
Ethan Jackson [Sat, 16 Mar 2013 01:03:59 +0000 (18:03 -0700)]
ofproto-dpif: Move odp_actions from subfacet to facet.
Upon close inspection, it appears that it's not possible for
actions to differ between subfacets belonging to a given facet.
Given this fact, it makes sense to move datapath actions from
subfacets to their parent facets. It's both conceptually more
straightforward, and necessary for future threading and megaflow
work.
Co-authored-by: Justin Pettit <jpettit@nicira.com> Signed-off-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Tue, 28 May 2013 18:43:43 +0000 (11:43 -0700)]
odp-util: Make slow_path_reasons mutually exclusive.
It's no longer possible for a single datapath flow to be slow
pathed for two different reasons. This patch updates the code to
reflect this fact (marginally simplifying it).
Ethan Jackson [Thu, 23 May 2013 23:01:20 +0000 (16:01 -0700)]
ofproto: Ditch SLOW_IN_BAND slow path reason.
Before this patch, when in band control was enabled, every DHCP
packet had to be sent to userspace to calculate it's actions.
Those DHCP packets intended for the local port would have a special
action added to ensure they actually make it there. This
unnecessarily complicates the code, so this patch takes a slightly
different approach. When in-band is enabled, *all* DHCP packets
must be sent to the local port. This guarantees that
xlate_actions() returns the same result every time for a given
flow.
Ethan Jackson [Fri, 17 May 2013 20:39:03 +0000 (13:39 -0700)]
ofproto-dpif: Ditch SLOW_MATCH slow path reason.
Before this patch, datapath keys with ODP_FIT_TO_LITTLE, would be
assigned subfacets and installed in the kernel with a SLOW_MATCH
slow path reason. This is problematic, because these flow keys
can't be reliable converted into a 'struct flow' thus breaking a
fundamental assumption of ofproto-dpif. This patch circumvents the
issue by skipping facet creation for these flows altogether. This
approach has the added benefit of simplifying the code for future
patches.
Lorand Jakab [Mon, 27 May 2013 11:54:54 +0000 (04:54 -0700)]
LISP: update documentation for "null" ports
Since commit 0ad90c8 it is possible to set tunnel destination IP address
in the flow. This allows creating a LISP "map-cache" in the flow table.
Update the LISP documentation to reflect these possibilities.
Signed-off-by: Lorand Jakab <lojakab@cisco.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Thu, 23 May 2013 23:07:43 +0000 (16:07 -0700)]
OPENFLOW-1.1+: OFPCML_NO_BUFFER is effectively already implemented.
OpenFlow 1.2 and later define a value of 65535 (OFPCML_NO_BUFFER) for
the max_len field in an output action to mean that the switch should send
the entire packet without buffering it. Open vSwitch never buffers packets
sent via an output action, so it trivially satisfies this requirement.
ovs-xapi-sync: Handle exceptions from XAPI for get_single_bridge_id.
There are possibilities when records disappear underneath ovs-xapi-sync.
In this particular case, when VLAN network was deleted, the corresponding
record in bridge's external_ids:xs_network_ids column was not deleted by
xenserver. In situations like that handle the exceptions cleanly.
Simon Horman [Wed, 22 May 2013 07:08:06 +0000 (16:08 +0900)]
dpif-netdev: Move decoding of data out of dp_netdev_output_userspace()
This is in preparation for making dp_netdev_action_userspace()
more generic and passing dp_netdev_output_userspace() as a callback.
In this case it makes sense to decode userdata in generic code.
Signed-off-by: Simon Horman <horms@verge.net.au> Signed-off-by: Ben Pfaff <blp@nicira.com>
YAMAMOTO Takashi [Tue, 21 May 2013 08:49:55 +0000 (17:49 +0900)]
keep "kernel name" for each netdev
where interface renaming is not supported (NetBSD), remember both of
our netdev name and the correspoinding kernel name separately.
the latter is necessary to talk with kernel using interface names.
eg. ifioctls, bpf
XXX there should be a proper way to query kernel name.
Ben Pfaff suggested ovs-appctl but this patch doesn't implement it. (yet)
Signed-off-by: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Wed, 15 May 2013 21:31:06 +0000 (14:31 -0700)]
cfm: Implement "demand mode".
The new CFM "demand mode" (named after BFD's demand mode) uses
data traffic to indicate interface liveness. It's helpful on
heavily congested networks where CCMs may be dropped.