Ethan Jackson [Sat, 27 Jul 2013 19:24:15 +0000 (12:24 -0700)]
ofproto-dpif-xlate: Don't try to optimize goto table.
This patch reverts commit 5559942 (ofproto-dpif: GOTO_TABLE recursion
removal.) by reintroducing the recursion through xlate_table_action().
The main reason to do this is the introduction of new rule locking in
future patches. The code before this patch was relatively difficult
to lock in a clean straight-forward manner.
Andy Zhou [Thu, 1 Aug 2013 17:49:46 +0000 (10:49 -0700)]
datapath: Accept any 802.2 eth_type mask but override to be exact match
When key.eth_type is absent it is interpreted to be 802.2, which is
represented by a special value. In order to prevent inadvertant matches
on this opaque value, the mask is forced to be either fully wildcarded
or fully exact.
Signed-off-by: Andy Zhou <azhou@nicira.com> Signed-off-by: Jesse Gross <jesse@nicira.com>
Andy Zhou [Thu, 1 Aug 2013 17:49:45 +0000 (10:49 -0700)]
datapath: Accept any in_port mask but override to be exact match
Pre mega flow, netlink allows the in_port key attribute
to be missing. Missing in_port is interpreted as DP_MAX_PORTS.
For backward compatibility, mega flow implementation will always allow
the mask of in_port to be specified, as if the in_port key attribute
is always specified.
To prevent accidental match of the DP_MAX_PORTS, which value is opaque to
the user space, we will always force the mask to be exact match,
regardless of the value supplied by the netline message. Missing
in_port mask continue to mean wildcarded match, same as other masks.
Signed-off-by: Andy Zhou <azhou@nicira.com> Signed-off-by: Jesse Gross <jesse@nicira.com>
Commit 97be153858b4cd175cbe7862b8e1624bf22ab98a (clang: Add
annotations for thread safety check.) defined 'struct ovs_mutex'
variable in 'atomic_flag' in 'ovs-atomic-pthreads.h'. This
casued "mutex: has incomplete type" error in compilation when
'ovs-atomic-pthreads.h' is included.
This commit goes back to use 'pthread_mutex_t' for that variable
and adds test for the 'atomic_flag' related functions.
Reported-by: Gurucharan Shetty <gshetty@nicira.com> Signed-off-by: Alex Wang <alexw@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Wed, 31 Jul 2013 18:29:28 +0000 (11:29 -0700)]
bfd: Downgrade long delay messages to INFO level.
On my system the long delay messages can cause a transient BFD unit
test failure due to the log checking. These messages don't *really*
need to be at WARN level, so this patch downgrades them.
Signed-off-by: Ethan Jackson <ethan@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
Ethan Jackson [Wed, 31 Jul 2013 17:49:34 +0000 (10:49 -0700)]
compiler: Fix OVS_LOCKS_EXCLUDED on non clang compilers.
This patch renames OVS_LOCKS_EXCLUDED to simply OVS_EXCLUDED so it's
more consistent with the other thread safety annotations. It also
adds it to the non-clang compilers.
Andy Zhou [Wed, 31 Jul 2013 02:49:12 +0000 (19:49 -0700)]
datapath: always export priority and skb_mark in netlink message
Handling of missing attributes in netlink can be tricky and turns out
to be error prone. The value (savings in netlink bandwidth) does not
seem to be significant enough to justify allowing them. This patch
series make both kernel and userspace always export priority and
skb_mark attribute. There will be follow on patches in the
direction of making all attributes explicit.
Signed-off-by: Andy Zhou <azhou@nicira.com> Acked-by: Simon Horman <horms@verge.net.au> Signed-off-by: Jesse Gross <jesse@nicira.com>
ovsdb-doc: Add ovsdb-doc to distribution tar ball.
Certain platforms like xenserver do not have the latest
python libraries that are needed by ovsdb-doc (which in-turn
creates ovs-vswitchd.conf.db.5). When we run 'make dist' and
copy over the tar ball to xenserver ddk environemt, we
already include ovs-vswitchd.conf.db.5. But the absence of
ovsdb-doc results in an attempt to regenerate ovs-vswitchd.conf.db.5
and that fails because of the missing python libraries.
Instead of producing ovsdb-doc from ovsdb-doc.in dynamically, we
statically provide ovsdb-doc and pass on the version information
to it through the command line option --version.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
This reverts commit 05d299e0ccca80736cd4438c3224540c5448a7d4 (test-atomic:
Drop atomic read-modify-write tests for the moment.) because the
test for detecting whether GCC support atomic operation built-ins has
been fixed.
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Ethan Jackson <ethan@nicira.com>
Ben Pfaff [Mon, 15 Jul 2013 21:13:53 +0000 (14:13 -0700)]
configure: Add configure-time check for GCC 4.0+ atomic built-ins.
We found out earlier that GCC sometimes produces an error only at link time
for atomic built-ins that are not supported on a platform. This actually
tries the link at configure time and should thus reliably detect whether
the atomic built-ins are really supported.
Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Ethan Jackson <ethan@nicira.com>
Ethan Jackson [Tue, 30 Jul 2013 22:31:48 +0000 (15:31 -0700)]
clang: Add annotations for thread safety check.
This commit adds annotations for thread safety check. And the
check can be conducted by using -Wthread-safety flag in clang.
Co-authored-by: Alex Wang <alexw@nicira.com> Signed-off-by: Alex Wang <alexw@nicira.com> Signed-off-by: Ethan Jackson <ethan@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
ovs-vswitchd.conf.db: Correct the header and footer lines.
Right now, the following 2 lines are how the header and footer
looks like for ovs-vswitchd.conf.db
@VERSION@(5) Open vSwitch Manual @VERSION@(5)
Open vSwitch Open_vSwitch @VERSION@(5)
After this commit, they look like this:
ovs-vswitchd.conf.db(5) Open vSwitch Manual ovs-vswitchd.conf.db(5)
Open vSwitch 1.12.90 ovs-vswitchd.conf.db(5)
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com> Acked-by: Ben Pfaff <blp@nicira.com>
Andy Zhou [Tue, 30 Jul 2013 17:52:34 +0000 (10:52 -0700)]
ofproto-dpif: Unset DPIF_FP_MODIFY flag when creating a new flo
Remove the DPIF_FP_MODIFY flag when creating a new flow. When flows arrive in
a batch, userspace may push down multiple unique flow definitions that
overlap when wildcards are applied. Kernels support flow wildcarding
will reject these flow as duplicates (EEXIST), which will be logged
at a lower logging level.
Signed-off-by: Andy Zhou <azhou@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Ben Pfaff [Mon, 29 Jul 2013 22:24:45 +0000 (15:24 -0700)]
Avoid C preprocessor trick where macro has the same name as a function.
In C, one can do preprocessor tricks by making a macro expansion include
the macro's own name. We actually used this in the tree to automatically
provide function arguments, e.g.:
int f(int x, const char *file, int line);
#define f(x) f(x, __FILE__, __LINE__)
...
f(1); /* Expands to a call like f(1, __FILE__, __LINE__); */
However it's somewhat confusing, so this commit stops using that trick.
Reported-by: Ed Maste <emaste@freebsd.org> Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Ed Maste <emaste@freebsd.org>
Ben Pfaff [Thu, 13 Jun 2013 20:20:17 +0000 (13:20 -0700)]
ofproto-dpif: Correctly refresh all ports on ENOBUFS from dpif_port_poll().
dpif_port_poll() is allowed to return ENOBUFS if something might have
changed, but the specific change isn't easily reportable. type_run()
didn't handle this case, so it wouldn't notice any changes when this
happened.
dpif-netdev (including dpif-dummy) uses ENOBUFS exclusively to report
changes, so this fixes a problem there. dpif-linux rarely uses ENOBUFS but
it can do so if a kernel-to-user Netlink buffer overflows.
Andy Zhou [Mon, 29 Jul 2013 20:26:08 +0000 (13:26 -0700)]
datapath: fix a bug in SF_FLOW_KEY_PUT macro
This bug will cause mask values to corrupt the flow key value. So far
the bug has not showed up because we don't write mask value when
there is no mask Netlink attributes. However, it needs to be fixed for
the next and future commits where we will start to set default
values for key and mask for missing Netlink attributes.
Signed-off-by: Andy Zhou <azhou@nicira.com> Signed-off-by: Jesse Gross <jesse@nicira.com>
datapath: list: Fix double fetch of pointer in hlist_entry_safe()
Following patch backports commit f65846a1800ef8c48d (list: Fix double
fetch of pointer in hlist_entry_safe()) from upstream kernel.
This patch fixes following panic. Thanks to Jesse for helping to
debug this issue.
list: Fix double fetch of pointer in hlist_entry_safe()
The current version of hlist_entry_safe() fetches the pointer twice,
once to test for NULL and the other to compute the offset back to the
enclosing structure. This is OK for normal lock-based use because in
that case, the pointer cannot change. However, when the pointer is
protected by RCU (as in "rcu_dereference(p)"), then the pointer can
change at any time. This use case can result in the following sequence
of events:
1. CPU 0 invokes hlist_entry_safe(), fetches the RCU-protected
pointer as sees that it is non-NULL.
2. CPU 1 invokes hlist_del_rcu(), deleting the entry that CPU 0
just fetched a pointer to. Because this is the last entry
in the list, the pointer fetched by CPU 0 is now NULL.
3. CPU 0 refetches the pointer, obtains NULL, and then gets a
NULL-pointer crash.
This commit therefore applies gcc's "({ })" statement expression to
create a temporary variable so that the specified pointer is fetched
only once, avoiding the above sequence of events. Please note that
it is the caller's responsibility to use rcu_dereference() as needed.
This allows RCU-protected uses to work correctly without imposing
any additional overhead on the non-RCU case.
Many thanks to Eric Dumazet for spotting root cause!
Reported-by: CAI Qian <caiqian@redhat.com> Reported-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Tested-by: Li Zefan <lizefan@huawei.com>
Bug #17099
Signed-off-by: Pravin B Shelar <pshelar@nicira.com> Acked-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Wed, 17 Jul 2013 22:53:26 +0000 (15:53 -0700)]
ofp-util: Fix port and queue stat counting for OpenFlow 1.3.
OpenFlow 1.0, 1.1, and 1.2 all have the same struct size for port and
queue stats. OpenFlow 1.3 has larger structs, but the ofp-util code didn't
realize that. This fixes the problem.
It appears that the only consequence of this problem would have been
printing the wrong count in ofp-print output.
datapath: Use correct type while allocating flex array.
Flex array is used to allocate hash buckets which is type struct
hlist_head, but we use `struct hlist_head *` to calculate
array size. Since hlist_head is of size pointer it works fine.
Following patch use correct type.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com> Acked-by: Jesse Gross <jesse@nicira.com>
Ben Pfaff [Thu, 25 Jul 2013 16:45:43 +0000 (09:45 -0700)]
util: Set thread name via pthreads in set_subprogram_name().
This makes "top" and "ps" output more readable on FreeBSD at least, and
the names are also visible in debuggers.
Suggested-by: Ed Maste <emaste@freebsd.org> Signed-off-by: Ben Pfaff <blp@nicira.com> Acked-by: Andy Zhou <azhou@nicira.com> Tested-by: Andy Zhou <azhou@nicira.com>
ovs-bugtool: Separate capability for general network info.
Current situation is that CAP_NETWORK_STATUS has a max size of 50 MB.
When we have around 100,000 openflow flows, we over-run that size
by just running the "ovs-ofctl dump-flows" command. All the openvswitch
commands run through the plugin scripts in this repo won't have its
data stored in the debug bundle in this case as they are part of
CAP_NETWORK_STATUS too. One option to correct this is to increase
the CAP_NETWORK_STATUS max size to a higher number. But CAP_NETWORK_STATUS
also includes a bunch of general network related information collected
by running commands like ethtool, tc etc. and we probably want to limit
the data collected through those commands.
With this commit, we create a new capability called CAP_NETWORK_INFO
and collect general network related information through them. For OVS
related information, we continue to use CAP_NETWORK_STATUS, but remove
the maximum size restriction. One rationale to keep OVS related
information in CAP_NETWORK_STATUS is because xen-bugtool probably expects
OVS information in that capability.
Alex Wang [Tue, 23 Jul 2013 01:15:49 +0000 (18:15 -0700)]
vlan-splinter: Fix inverted logic bug.
When "other-config:enable-vlan-splinters=true" is set, the existing
vlans with ip address must be retained. The bug actually does the
opposite and retains the vlans without ip address. This commit fixes
it.
Reported-by: Roman Sokolkov <rsokolkov@gmail.com> Signed-off-by: Alex Wang <alexw@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Andy Zhou [Mon, 22 Jul 2013 22:08:00 +0000 (15:08 -0700)]
datapath: remove RCU annotation from flow->mask
After a mask is assigned to a flow, it will not change for the life of
the flow. Since flow access is protected by RCU lock, access to
flow->mask after getting a flow is always safe.
Suggested-by: Jesse Gross <jesse@nicira.com> Reported-by: Ben Pfaff <blp@nicira.com> Signed-off-by: Andy Zhou <azhou@nicira.com> Signed-off-by: Jesse Gross <jesse@nicira.com>
Alex Wang [Mon, 22 Jul 2013 16:19:57 +0000 (09:19 -0700)]
clang: Fix the "expression result unused" warning.
This commit makes macro function "ASSIGN_CONTAINER()" evaluates
to "(void)0". This is to avoid the 'clang' warning: "expression
result unused", since most of time, the final evaluated value
is not used.
Signed-off-by: Alex Wang <alexw@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
Alex Wang [Mon, 22 Jul 2013 16:19:56 +0000 (09:19 -0700)]
clang: Fix segfault in unit tests.
It seems that 'clang' compiler applies strict protection on pointer
dereference. And it causes unexpected execution in macro functions
like "HMAP_FOR_EACH()" and unit test failures. This commit fixes
this issue and pass all unit tests.
Co-authored-by: Ethan Jackson <ethan@nicira.com> Signed-off-by: Alex Wang <alexw@nicira.com> Signed-off-by: Ben Pfaff <blp@nicira.com>
datapath: Conditionally define skb_unclone in datapath compat code
Recent versions of Fedora have skb_unclone included in their
kernels. This patch adds a conditional check into the compat directory so as
not to error out by defining it twice. This allows the latest OVS kernel
module to build on Fedora 19.