This patch separates the structures for thread-local flow dump state
("state") from the shared flow dump state ("iter") in dpif-linux and
dpif-netdev. Future patches will make use of this to allow multiple
threads to dump flows from the same flow dump operation.
Signed-off-by: Joe Stringer <joestringer@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This patch makes all of the users of 'struct nl_dump' allocate their own
buffers to pass down to nl_dump_next(). This paves the way for allowing
multithreaded flow dumping.
Signed-off-by: Joe Stringer <joestringer@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
When creating tap ports in dpif-linux, the "tap" type is treated the
same as "system", and the type is discarded. When dumping datapath
port types, this would cause "tap" type to be reported as a "system"
type.
Each time we see a port of the wrong type in bridge_reconfigure(), we
remove it and add a port with the correct configuration. This would
always occur for tap ports, causing deletion and re-creation of all tap
ports each time the bridge was reconfigured.
This patch makes dpif-linux use netdev to look up port types if the
datapath reports that they are of type OVS_VPORT_TYPE_NETDEV.
Bug #1196289.
Reported-by: James Schmidt <jschmidt@vmware.com>
Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
The 'dpif_' parameter of dpif_linux_flow_dump_next() was marked as
OVS_UNUSED, even though it's passed down to dpif_linux_flow_get__().
Signed-off-by: Joe Stringer <joestringer@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This helps reduce confusion about when a flow is a flow and when it is
just metadata.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Following commit (''netlink: Do not enforce alignment of last Netlink
attribute''), signal the ability to receive unaligned Netlink messages
to the datapath to enable utilization of zerocopy optimizations.
Opening a datapath is now done by issueing a OVS_DP_CMD_SET in order
to overwrite previously set user features.
Signed-off-by: Thomas Graf <tgraf@redhat.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
We already set all the fields of the upcall, so memsetting right before
is unnecessary.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
This commit adds check of sock pointer in dpif_linux_port_get_pid().
If the pointer is NULL, do not call nl_sock_pid().
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Commit 222837 (dpif-linux: Factor out port dumping helper functions.)
introduced a bug by making dpif_linux_port_dump_next__() return 'bool'
instead of 'int' as defined in dpif-provider.h. This bug causes ovs-
vswitchd failure with SEGFAULT when processing slow-path packet.
This commit fixes the bug by following the dpif-provider specification.
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
This allows other libraries to use util.h that has already
defined NOT_REACHED.
Signed-off-by: Harold Lim <haroldl@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Commit da546e0 (dpif: Allow execute to modify the packet.) introduced
a bug by subtracting the zero-value ofpbuf size by "sizeof(struct
nlattr)" and assigning the result back to the ofpbuf size. This bug
causes the ovs-assert failure in facet_push_stats().
This commit fixes the bug by assigning the right value to the ofpbuf
size.
Signed-off-by: Alex Wang <alexw@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
Allowing the packet to be modified by execution allows less data
copying for userspace action execution. Some users of the
dpif_execute already expect that the packet may be modified. This
patch makes this behavior uniform and makes the userspace datapath and
the execution helpers modify the packet as it is being executed.
Userspace action now steals the packet if given permission, as the
packet is normally not needed after it. The only exception is the
sample action, and this is accounted for my keeping track of any
actions that could be following the userspace action.
The packet in dpif_upcall is changed from a pointer to a struct,
allowing the packet to be honest about it's headroom. After this
change the packet can safely be pushed on over the precarious 4 byte
limit earlier allowed by the netlink data preceding the packet.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
When ovs-vswitchd deletes a port with dpif_linux_port_del(), that function
uses del_channel() to delete the corresponding channel, including closing
its Netlink socket fd. However, if the vport gets removed by some other
process (e.g. "ip link delete" for veths) then this function never gets
called and thus the channel never gets deleted.
This commit fixes the problem by resynchronizing channels with the
datapath whenever a vport gets deleted.
Bug #16784.
Reported-by: Paul Ingram <paul@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Until now, the code in dpif_linux_recv_set() has assumed it is starting
from a clean slate with no channels at all. This commit generalizes it to
compare the existing set of channels against the ones that should exist
given the vports that the kernel has, adding any new ones that are missing
and removing any that should not exist. A followup commit will make good
use of this functionality.
Signed-off-by: Ben Pfaff <blp@nicira.com>
The MSVC C library printf() implementation does not support the 'z', 't',
'j', or 'hh' format specifiers. This commit changes the Open vSwitch code
to avoid those format specifiers, switching to standard macros from
<inttypes.h> where available and inventing new macros resembling them
where necessary. It also updates CodingStyle to specify the macros' use
and adds a Makefile rule to report violations.
Signed-off-by: Alin Serdean <aserdean@cloudbasesolutions.com>
Co-authored-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Both the old (wrong) and new (correct) enums have the same value, so this
doesn't fix a visible bug.
Reported-by: ankur dwivedi <ankurengg2003@gmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The declaration of 'get_max_ports()' to return odp_port_t adds
unwanted complexity to coding. This commit changes it back to
return uint32_t type.
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
We've seen a number of deadlocks in the tree since thread safety was
introduced. So far, all of these are self-deadlocks, that is, a single
thread acquiring a lock and then attempting to re-acquire the same lock
recursively. When this has happened, the process simply hung, and it was
somewhat difficult to find the cause.
POSIX "error-checking" mutexes check for this specific problem (and
others). This commit switches from other types of mutexes to
error-checking mutexes everywhere that we can, that is, everywhere that
we're not using recursive mutexes. This ought to help find problems more
quickly in the future.
There might be performance advantages to other kinds of mutexes in some
cases. However, the existing mutex type choices were just guesses, so I'd
rather go for easy detection of errors until we know that other mutex
types actually perform better in specific cases. Also, I did a quick
microbenchmark of glibc mutex types on my host and found that the
error checking mutexes weren't any slower than the other types, at least
when the mutex is uncontended.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
If a notification is bigger than 4 kB (I doubt it one would be), then the
lack of ofpbuf_uninit() would cause a memory leak.
Signed-off-by: Ben Pfaff <blp@nicira.com>
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>
This seems to be the last remaining thread-unsafe code in dpif-linux.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
This disentangles "struct nl_dump" from "struct nl_sock", clearing the way
to make the use of either one thread-safe in an obviously correct manner.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Until now, datapath ports and openflow ports were both represented by
unsigned integers of various sizes. With implicit conversions, etc., it is
easy to mix them up and use one where the other is expected. This commit
creates two typedefs, ofp_port_t and odp_port_t. Both of these two types
are marked by "__attribute__((bitwise))" so that sparse can be used to
detect any misuse.
Signed-off-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Added support to allow mega flow specified and displayed. ovs-dpctl tool
is mainly used as debugging tool.
This patch also implements the low level user space routines to send
and receive mega flow netlink messages. Those netlink suppor
routines are required for forthcoming user space mega flow patches.
Added a unit test to test parsing and display of mega flows.
Ethan contributed the ovs-dpctl mega flow output function.
Co-authored-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
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>
When ovs-vswitchd deletes a port with dpif_linux_port_del(), that function
uses del_channel() to delete the corresponding channel, including closing
its Netlink socket fd. However, if the vport gets removed by some other
process (e.g. "ip link delete" for veths) then this function never gets
called and thus the channel never gets deleted.
This commit partially fixes the problem. Now, if a port number gets
reused, add_channel() closes the old Netlink socket assigned to that port
before it installs the new one.
Bug #16784.
Reported-by: Paul Ingram <paul@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
MAX_PORTS is currently USHRT_MAX (also 65535). I think that's a
coincidence; I don't remember MAX_PORTS being mentioned when the new
dpif_channel code was written.
Signed-off-by: Ben Pfaff <blp@nicira.com>
The list of epoll events contains references to channels which may
be stale when one of those channels is deleted. The safest thing
to do is simply refresh epoll() whenever a channel is deleted.
Bug #16057.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Since userspace flow based tunneling code is checked in, the kernel
port based tunneling code can be removed.
Patch removes following components:
- tunnel ports hash table and moved tunnel ports list to individual
vports.
- Cleaned per tnl-port config.
- OVS_KEY_ATTR_TUN_ID action is removed.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #15078
LISP is an experimental layer 3 tunneling protocol, described in RFC
6830. This patch adds support for LISP tunneling. Since LISP
encapsulated packets do not carry an Ethernet header, it is removed
before encapsulation, and added with hardcoded source and destination
MAC addresses after decapsulation. The harcoded MAC chosen for this
purpose is the locally administered address 02:00:00:00:00:00. Flow
actions can be used to rewrite this MAC for correct reception. As such,
this patch is intended to be used for static network configurations, or
with a LISP capable controller.
Signed-off-by: Lorand Jakab <lojakab@cisco.com>
Signed-off-by: Kyle Mestery <kmestery@cisco.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
The CAPWAP implementation is just the encapsulation format and
therefore really not the full protocol. While there were some
uses of it (primarily hardware support and UDP transport). But
these are most likely better provided by VXLAN.
Following patch removes CAPWAP tunneling support.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Until now, the optional OVS_USERSPACE_ATTR_USERDATA attribute had to be
exactly 64 bits long, if it was present. However, 64 bits is not enough
space to associate as much information with a flow as would be convenient
for some userspace features now under development. This commit generalizes
the attribute, allowing it to be any length.
This generalization is backward-compatible: if userspace only uses 64-bit
attributes, then it will not see any change in behavior.
CC: Romain Lenglet <rlenglet@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
OVS_TUNNEL_ATTR_DST_PORT expects a u16, tnl_cfg->dst_port is a be16, so
we want ntohs() instead of htons().
In practice htons() and ntohs() perform the same operation, so this does
not fix a real bug.
Found by sparse.
Signed-off-by: Ben Pfaff <blp@nicira.com>
This patch removes the final bit of linux specific code which
prevents building netdev-vport everywhere. With this, other
platforms automatically get access to patch ports, and (if their
datapath supports it), flow based tunneling.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
With this patch, ovs-vswitchd uses flow based tunneling
exclusively. I.E. each kind of tunnel shares a single tunnel
backer in the datapath. Tunnel headers are set by userspace using
the ipv4_tunnel datapath action. And, the configuration of
individual tunnels is now a userspace responsibility, so
netdev-vport no longer marshals and unmarshals Netlink attributes
for tunnel configuration, instead only storing the configuration
internally. There are still some significant pieces of work to do,
but the basic building blocks are there to begin testing.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Co-authored-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
In future patches, a netdev's datapath port name may not
necessarily be the same as its device name. This patch prepares for
this by making the distinction in the netdev and dpif layers.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
The ability to retrieve and set MAC addresses on vports is only
necessary for tunnel ports (the addresses for actual devices can be
retrieved through direct Linux mechanisms). Tunnel ports only used
the information for the purpose of generating path MTU discovery
packets, which has now been removed. Current userspace code already
reflects these changes, so this drops the functionality from the
kernel.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>
Messages about packets being lost are logged at level WARN, but when
they were generated at a high rate, those consolidated messages were
logged at ERR. This changes to consolidated messages to be logged at
WARN, too.
Thanks to Ben Pfaff for quickly suggesting the culprit.
Bug #14783
Reported-by: James Schmidt <jschmidt@nicira.com>
Signed-off-by: Justin Pettit <jpettit@nicira.com>
