This optimization on its own provided about 37% benefit against a
load of a single netperf CRR test, but at the same time it penalized
ovs-benchmark by about 11%. We can get back the CRR performance
loss, and more, other ways, so the first step is to revert this
patch, temporarily accepting the performance loss.
Following patch adds skb-priority to flow key. So userspace will know
what was priority when packet arrived and we can remove the pop/reset
priority action. It's no longer necessary to have a special action for
pop that is based on the kernel remembering original skb->priority.
Userspace can just emit a set priority action with the original value.
Since the priority field is a match field with just a normal set action,
we can convert it into the new model for actions that are based on
matches.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #7715
Currently we hard code the versions of our GENL families to 1 but it's
nicer to have symbolic constants.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Until now, OVS has handled IP fragments more awkwardly than necessary. It
has not been possible to match on L4 headers, even in fragments with offset
0 where they are actually present. This means that there was no way to
implement ACLs that treat, say, different TCP ports differently, on
fragmented traffic; instead, all decisions for fragment forwarding had to
be made on the basis of L2 and L3 headers alone.
This commit improves the situation significantly. It is still not possible
to match on L4 headers in fragments with nonzero offset, because that
information is simply not present in such fragments, but this commit adds
the ability to match on L4 headers for fragments with zero offset. This
means that it becomes possible to implement ACLs that drop such "first
fragments" on the basis of L4 headers. In practice, that effectively
blocks even fragmented traffic on an L4 basis, because the receiving IP
stack cannot reassemble a full packet when the first fragment is missing.
This commit works by adding a new "fragment type" to the kernel flow match
and making it available through OpenFlow as a new NXM field named
NXM_NX_IP_FRAG. Because OpenFlow 1.0 explicitly says that the L4 fields
are always 0 for IP fragments, it adds a new OpenFlow fragment handling
mode that fills in the L4 fields for "first fragments". It also enhances
ovs-ofctl to allow users to configure this new fragment handling mode and
to parse the new field.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Bug #7557.
A fair number of datapath flow operations optionally report back results
to the requester based on whether NLM_F_ECHO is set in the request. When
userspace isn't going to use those results anyway, it wastes memory to
store them and a system call to retrieve them.
This commit omits the NLM_F_ECHO bit in cases where the caller isn't going
to use the results.
(NLM_F_ECHO has no effect on operations whose entire purpose is to retrieve
data, e.g. "get" and "dump" operations, so we need not bother to set it
for those.)
This improves "ovs-benchmark rate" results in my testing by about 4%.
Most of the enum tags in this file are lowercased versions of the uppercase
enum prefixes (or slightly less abbreviated versions, e.g. "dp" becomes
"datapath"). This commit fixes up the others for consistency.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
This increases consistency with the OVS_ACTION_ATTR_USERSPACE action, which
also requires an explicit pid.
Suggested-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Commit b063d9f06 "datapath: Use unicast Netlink sockets for upcalls" that
switched from multicast to unicast Netlink for sending upcalls added a
Netlink PID to each kernel flow, used by OVS_ACTION_ATTR_USERSPACE actions
within the flow as target.
This commit drops this per-flow PID in favor of a per-action PID, because
that is more flexible. It does not yet make use of this additional
flexibility, so behavior should not change.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #7559.
Commit b063d9f0 "datapath: Use unicast Netlink sockets for upcalls" that
introduced an 'upcall_pid' member into struct dpif_linux_vport, struct
dpif_linux_dp, and struct dpif_linux_flow neglected to do so only if the
member was nonzero. This caused every datapath, vport, and flow operation
to supply an upcall_pid. In particular, the netdev_set_config() called at
startup when a vport already existed caused the upcall_pid for that vport
to be reset to 0, which in turn caused all packets received on the vport to
be dropped instead of forwarded to ovs-vswitchd.
Reported-by: Shih-Hao Li <shli@nicira.com>
Bug #7714.
Following patch removes ifIndex attribute of vport which is not
used in userspace.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #7114
Over time we wish to reduce the number of datapath-protocol.h definitions
used directly outside of Linux-specific code. This commit removes use of
"struct ovs_dp_stats" from platform-independent code.
Bug #7559.
Following patch adds sampling action which takes probability and set
of actions as arguments. When probability is hit, actions are executed for
given packet.
USERSPACE action's userdata (u64) is used to store struct
user_action_cookie as cookie. CONTROLLER action is fixed accordingly.
Now we can remove sFlow code from kernel and implement sFlow generically
as SAMPLE action. sFlow is defined as SAMPLE Action with probability (sFlow
sampling rate) and USERSPACE action as argument. USERSPACE action's data
is used as cookie. sFlow uses this cookie to store output-port, number of
output ports and vlan-id. sample-pool is calculated by using vport
stats.
Signed-off-by: Pravin Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Currently it is possible for a client on a single port to generate
a huge number of packets that miss in the kernel flow table and
monopolize the userspace/kernel communication path. This
effectively DoS's the machine because no new flow setups can take
place. This adds some additional fairness by separating each upcall
type for each object in the datapath onto a separate socket, each
with its own queue. Userspace then reads round-robin from each
socket so other flow setups can still succeed.
Since the number of objects can potentially be large, we don't always
have a unique socket for each. Instead, we create 16 sockets and
spread the load around them in a round robin fashion. It's theoretically
possible to do better than this with some kind of active load balancing
scheme but this seems like a good place to start.
Feature #6485
Currently we publish several multicast groups for upcalls and let
userspace sockets subscribe to them. The benefit of this is mostly
that userspace is the one doing the subscription - the actual
multicast capability is not currently used and probably wouldn't be
even if we moved to a multiprocess model. Despite the convenience,
multicast sockets have a number of disadvantages, primarily that
we only have a limited number of them so there could be collisions.
In addition, unicast sockets give additional flexibility to userspace
by allowing every object to potentially have a different socket
chosen by userspace for upcalls. Finally, any future optimizations
for upcalls to reduce copying will likely not be compatible with
multicast anyways so disallowing it potentially simplifies things.
We also never unregistered the multicast groups registered for upcalls
and leaked them on module unload. As a side effect, this solves that
problem.
Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
The nl_lookup_genl_mcgroup() function can fail on older kernels
which do not support the required netlink interface. Before this
patch, dpif-linux would refuse to create a datapath when this
happened. With this patch, it attempts to use a workaround. If
the workaround fails it simply disables the affected features
without completely disabling the dpif.
Before this patch, if dpif-linux failed to register a notifier it
would give up opening the datapath entirely. This seems draconian
as a dpif can still perform the majority of its intended
functionality without vport notifications.
This patch changes the interface of netlink-notifier and
rtnetlink-link. Now nln_notifiers are allocated and destroyed by
the module instead of passed in by callers. This allows the
definition of nln_notifier to be hidden, and generally cleans up
the code.
It makes more sense to call nln_notifier_run() and
nln_notifier_wait() simply nln_run() and nln_wait() since they
don't operate on notifiers but the entire nln object. This patch
changes the nln and the rtnetlink-link modules to the new
convention.
Currently ovs is using device stats for Linux devices and count them
itself in other situations. This leads to overlap with hardware stats,
inconsistencies, etc. It's much better to just always count the packets
flowing through the switch and let userspace do any merging that it wants.
Following patch removes vport->get_stats() interface. vport-stat is changed
to use new `struct ovs_vport_stat` rather than rtnl_link_stats64.
Definitions of rtnl_link_stats64 is removed from OVS. dipf_port->stat is also
removed as aggregate stats are only available at netdev layer.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
There is no need to have vport attribute MTU (OVS_VPORT_ATTR_MTU) as
linux net-dev-ioctl can be used to get/set MTU for linux device.
Following patch removes OVS_VPORT_ATTR_MTU from datapath protocol.
This patch also adds netdev_set_mtu interface. So that MTU adjustments
can be done from OVS userspace. get_mtu() interface is also changed, now
get_mtu() returns EOPNOTSUPP rather than returning 0 and setting *pmtu
to INT_MAX in case there is no MTU attribute for given device.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Remove iflink from vport interface. iflink is not used anywhere in
OVS. So there is not need to have iflink as vport attribute.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Currently dpif-linux listens for vport change events using
rtnetlink notifications. This patch switches to the ovs genl
notification system.
Feature #6809.
This patch renames the rtnetlink module's code to "nln" for
"netlink notifier". Callers are now required to pass in the
netlink protocol to he newly renamed nln_create() function.
The only rtnetlink specific functionality contained in the
rtnetlink module is the use of the NETLINK_ROUTE protocol. This
can easily be passed in by callers.
In preparation for generalization, this patch renames
rtnetlink.[ch] to netlink-notifier.[ch]. Future patches will
complete the transition.
Whenever a port is added to the datapath, LRO is automatically disabled.
In the future, we may want to enable LRO in some circumstances, so have
userspace disable LRO through the ethtool ioctls.
As part of this change, the MTU and LRO checks are moved to
netdev-vport's send(), which is where they're actually needed.
Feature #6810
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
The prefix "ODP_*" is not overly descriptive in the context of the
larger Linux tree. This commit changes the prefix to "OVS_*" for the
userpace to kernel interactions. The userspace libraries still use
"ODP_" in many of their interfaces since it is more descriptive in the
OVS oeuvre.
Feature #6904
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
dpif_linux_vport_send() had duplicated most of the code in
dpif_linux_execute() in order to execute output actions in the
kernel. This forces developers to remember to change both
functions whenever the kernel interface changes. In particular,
commit 80e5eed9 "datapath: Get packet metadata from userspace in
odp_packet_cmd_execute()." broke netdev_linux_vport_send(). This
commit reorganizes the code and fixes the regression.
Bug #5818.
Until now, the tun_id and in_port have been lost when a packet is sent from
the kernel to userspace and then back to the kernel. I didn't think that
this was a problem, but recent behavior made me look closer and see that
it makes a difference if sFlow is turned on or if an
ODP_ATTR_ACTION_CONTROLLER action is present. We could possibly kluge
around those, but for future-proofing it seems better to pass the packet
metadata from userspace to the kernel. That is what this commit does.
This commit introduces a user-kernel protocol break. We could avoid that,
if it is desirable, by making ODP_PACKET_ATTR_KEY optional for
ODP_PACKET_CMD_EXECUTE commands.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
I know already that this breaks the statsfixes that were implemented by the
following commits:
827ab71c97f "ofproto: Datapath statistics accounted twice."
6f1435fc8f7 "ofproto: Resubmit statistics improperly account during..."
These were already broken in a previous merge. I will work on a fix.
Until now, the dp_run() and dp_wait() functions had to be called at the top
level of the program because they applied to every open dpif. By replacing
them by functions that take a specific dpif as an argument, we can call
them only from ofproto, which is currently the correct layer to deal with
dpifs.
When ports are deleted from the datapath they need to be added to
an LRU list maintained in dpif-linux so they may be reallocated.
When using vswitchd to delete the ports this happens automatically.
However, if a port is deleted directly from the datapath it is
never reclaimed by dpif-linux. If this happens often, eventually
no ports will be available for allocation and dpif-linux will fall
back to using the old, kernel implemented, allocation strategy.
This commit fixes the problem by automatically reclaiming ports
missing from the datapath whenever the list of ports in the
datapath is dumped.
Bug #2140.
netdev_linux_get_stats() calls into netdev_vport_get_stats(), which in
turn attempts a transaction on genl_sock. If the kernel module isn't
loaded, then genl_sock won't be there, and in any case there's nothing that
guarantees that it's been initialized yet.
This fixes the problem by ensuring that dpif_linux was initialized properly
before attempting a transaction on genl_sock.
Reported-by: Aaron Rosen <arosen@clemson.edu>
Before this patch the kernel chose the lowest available number for
newly created datapath ports. This patch moves the port number
choosing responsibility to user space, and implements a least
recently used port number queue in an attempt to avoid reuse.
Bug #2140.
In each of the cases converted here, an shash was used simply to maintain
a set of strings, with the shash_nodes' 'data' values set to NULL. This
commit converts them to use sset instead.
Until now, tunnel vports have had a specific MTU, in the same way that
ordinary network devices have an MTU, but treating them this way does not
always make sense. For example, consider a datapath that has three ports:
the local port, a GRE tunnel to another host, and a physical port. If
the physical port is configured with a jumbo MTU, it should be possible to
send jumbo packets across the tunnel: the tunnel can do fragmentation or
the physical port traversed by the tunnel might have a jumbo MTU.
However, until now, tunnels always had a 1500-byte MTU by default. It
could be adjusted using ODP_VPORT_MTU_SET, but nothing actually did this.
One alternative would be to make ovs-vswitchd able to set the vport's MTU.
This commit, however, takes a different approach, of dropping the concept
of MTU entirely for tunnel vports. This also solves the problem described
above, without making any additional work for anyone.
I tested that, without this change, I could not send 1600-byte "pings"
between two machines whose NICs had 2000-byte MTUs that were connected to
vswitches that were in turn connected over GRE tunnels with the default
1500-byte MTU. With this change, it worked OK, regardless of the MTU of
the network traversed by the GRE tunnel.
This patch also makes "patch" ports MTU-less.
It might make sense to remove vport_set_mtu() and the associated callback
now, since ordinary network devices are the only vports that support it
now.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Suggested-by: Jesse Gross <jesse@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #3728.
Expanding an skbuff in a netlink dump handler doesn't work well. We
weren't updating the truesize of the skb or the allocation within the
socket that netlink_dump() had put the skb in. The code had other bugs
too.
This commit fixes the problem (in my tests, anyway) by avoiding expanding
the reply skbuff to fill in the actions. Instead, in such a case the
userspace client has to do a separate "get" action to get the actions.
This commit also updates userspace to do this automatically for dumps in
the cases where the caller cares (only "ovs-dpctl dump-flows" currently
cares).
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Bug #4520.
The kernel expects that ODP_FLOW_NEW always has an ODP_FLOW_ATTR_ACTIONS
attribute, even though that attribute may be empty to drop all of the
packets in the flow. Similarly, ODP_FLOW_SET as used by
dpif_linux_flow_put() should always have such an attribute, since it is
used by OVS to update the flow's actions. So make it possible for
dpif_linux_flow_to_ofpbuf() to pass an empty actions attribute, and make
dpif_linux_flow_put() always force that behavior if the actions_len passed
to it is 0.
This fixes EINVAL error creating flows to drop packets.
Acked-by: Jesse Gross <jesse@nicira.com>
We were never storing the flow used time from the Netlink message
into our local struct, which caused flows to timeout prematurely.
Acked-by: Ben Pfaff <blp@nicira.com>
Flow transactions expect a response after the operation has completed
but the request did not have NLM_F_ECHO set. This caused userspace
to receive only the Netlink ACK instead of a real response, making
it appear that the operation had failed when it actually succeeded.
