Use ofputil_format_port() to print port numbers so that special ports
will be printed out as symbolic names (e.g., LOCAL) instead of numbers.
Signed-off-by: Jarno Rajahalme <jarno.rajahalme@nsn.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
There are a lot of tunnels, and they all use the exact same
functions, so it makes sense to collapse their initialization into
a macro.
Suggested-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Nothing ever breaks out of the loop, so this statement couldn't be reached.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>
This function cannot easily be reentrant because the inner call would
interrupt and corrupt the data being sent by the outer call.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>
The following call stack was possible:
poll_block()
-> vlog
-> worker_send_iovec()
-> poll_block()
which caused corruption because poll_block() is not reentrant.
Bug #14616.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
The following call stack was possible:
vlog
-> worker_request()
-> poll_block()
-> vlog
-> worker_request()
which caused problems because worker_request() is not reentrant. In a
little more detail, the second worker_request() shoves its RPC protocol
data into the middle of the first. This means that, first, you get
some binary crud in the log (the header for the second RPC). And,
second, text from the first RPC log message gets treated by the worker
as the subsequent RPC's header. That, in turn, typically causes the
worker to try to xmalloc() a huge number of bytes (0x20000000 or more,
since "space" has ASCII value 0x20), which causes the worker to die
with "virtual memory exhausted". The main process then dies because
the worker's death closes the socket it uses to communicate with it
("connection reset").
Bug #14616.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Userspace-kernel communication is a possible bottleneck when OVS is
receiving a large number of flow set up requests. To help prevent a bad
actor from consuming too much of this resource, we introduced channels
to segegrate traffic. Previously, we created 17 channels and
round-robin assigned ports to one of 16 channels (the 17th was reserved
for use by the system). This meant if there were more than 16 ports,
sharing of channels would occur.
This commit creates a new channel for each port, so that there is no
more sharing and better isolation. The special system port uses the
"ovs-system"'s channel (port 0), since it is not heavily loaded.
This also fixes an issue introduced in commit acf60855 (ofproto-dpif:
Use a single underlying datapath across multiple bridges.) where ports
that were added at run-time were given the special system channel.
Issue #12073
Signed-off-by: Justin Pettit <jpettit@nicira.com>
When adding a port, the code previously logged the requested port number
(which is generally UINT32_MAX) instead of the assigned port number.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
The open-coded version of destroy_table() in classifier_destroy() didn't
free the table's minimatch. Use destroy_table() to do it properly.
This is only a theoretical leak because all the existing callers actually
remove all the rules from their classifiers before they destroy them
(outside of the tests/ directory, which I didn't examine) and so they don't
ever have anything left to remove in classifier_destroy().
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Currently we use "*" or ANY to mark a field in flow syntax
as a wildcard. With ANY being a valid openflow port now,
there is a conflict for in_port field. So at the least, we
need to remove ANY from being considered as a wildcard for
in_port. But this may cause general confusion and it may be
a better idea to remove 'ANY' as a wildcard for all fields.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Currently, when we add a flow with in_port=65534, we get the
following warning.
"ofp_util|WARN|referring to port LOCAL as 65534 is deprecated
for compatibility with future versions of OpenFlow."
But ovs-ofctl, while dumping flows uses 65534 instead of LOCAL.
The same is true for other reserved ports.
This patch corrects that.
Bug #14118.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Also, use ovs_strlcpy() instead of strcpy() just to be a teensy bit safer.
Found by valgrind.
Bug #14357.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Also, add an assertion that the field is the expected size.
This bug was introduced in commit 2fdf762a006f (vswitchd: Log all tunnel
parameters of given flow.)
Found by valgrind.
Bug #14357.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
The old algorithm tries to converge to 0, despite it would mean a very
unbalanced situation. We're aiming for an ideal ratio of 1, meaning both
the 'from' and 'to' slave have the same load. Therefore, we only move an
entry if it decreases the load on 'from', and brings us closer to equal
traffic load.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The customary parameter order in Open vSwitch is to put input parameters
before output parameters, but vconn_open() and pvconn_open() had the 'dscp'
input parameter at the end, which bugged me a bit. Also,
vconn_open_block() didn't take a 'dscp' parameter at all even though it's
otherwise a wrapper around vconn_open(). This commit fixes all that up.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
Letting netdev-vport manage ethernet addresses itself instead of
relying on the datapath has several advantages. It simplifies the
code, is significantly more efficient, and will work when there is
no longer a one to one mapping from netdev-vports to datapath
vports.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
This patch removes path MTU discovery from userspace. The feature
still exists in the kernel where it will need to be removed in the
future.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Theoretically, its possible for netdev_get_status() to be called
on a netdev-vport which hasn't had its configuration set yet. In
this case, netdev-vport would dereference a null pointer.
Reported-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Open vSwitch enforces that, when instructions appear as Nicira extensions
in OpenFlow 1.0 action lists, the instructions appear in the order that
an OpenFlow 1.1+ switch would execute them and that no duplicates appear.
But the check wasn't general enough, because it had been implemented when
only one instruction was implemented and never later generalized. This
commit fixes the problem.
One of the tests was actually testing for the wrong behavior that the
check implemented, so this commit corrects that test. It also updates
another test with updated log messages.
Reported-by: Jing Ai <ai_jing2000@hotmail.com>
Tested-by: Jing Ai <ai_jing2000@hotmail.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
While dumping lacp information using ovs-appctl, the "aggregation
key" field displays the port_id even though the aggregation key is
set using "other-config:lacp-aggregation-key".
Signed-off-by: Anoob Soman <anoob.soman@citrix.com>
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Split the L3 and above portion of flow_extract() out into
flow_extract_l3_onwards() and call flow_extract_l3_onwards()
from flow_extract().
This is to allow re-extraction of l3 and higher information using
flow->encap_dl_type which may be set using information contained
in actions.
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>
The only user of netdev_set_stats() is bonding (for updating the
fake interface). This interface is never a vport, so it seems
quite a bit cleaner to keep the relevant code in the netdev-linux
library where it's needed, instead of in netdev-vport, where it
adds needless complexity.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Fixes the following sparse warnings:
meta-flow.c:947:21: warning: incorrect type in assignment (different base types)
meta-flow.c:947:21: expected restricted __be32 [usertype] be32
meta-flow.c:947:21: got unsigned int const [unsigned] [usertype] skb_priority
meta-flow.c:951:21: warning: incorrect type in assignment (different base types)
meta-flow.c:951:21: expected restricted __be32 [usertype] be32
meta-flow.c:951:21: got unsigned int const [unsigned] [usertype] skb_mark
Signed-off-by: Ben Pfaff <blp@nicira.com>
This patch adds logging support for skb_mark and skb_priority.
Acked-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Ansis Atteka <aatteka@nicira.com>
This function can be implemented as a trivial wrapper around
mf_get_value(), which I hadn't noticed before, so it's better to do it
that way. Also, examining the code that is removed, it had some bugs in
it (for example, all MFF_TUN_* fields were treated as if they were
MFF_TUN_ID) which mf_get_value() does not have, so this fixes bugs too.
Signed-off-by: Ben Pfaff <blp@nicira.com>
If a negative number is supplied, the parsing code used to convert it
into a signed one. We ran into an incident where a third-party script
was attempting to get the OpenFlow port number for an interface, but got
-1 from the database, since the number had not yet been assigned. This
was converted to 65535, which maps to OFPP_NONE and all flows with
ingress port OFPP_NONE were modified. This commit disallows negative
port numbers to help prevent broken integration scripts from disturbing
the flow table.
Issue #14036
Signed-off-by: Justin Pettit <jpettit@nicira.com>
To keep control+C and other signals in the initiating session from killing
the monitor process, we need to put the monitor process into its own
session. However, until this point, we've only done that for the daemon
processes that the monitor started, which means that control+C would kill
the monitor but not the daemons that it launched.
I don't know of a benefit to putting the monitor and daemon processes in
different sessions, as opposed to one new session for both of them, so
this change does the latter.
daemonize_post_detach() is called from one additional context where we'd
want to be in a new session, the worker_start() function, but that function
is documented as to be called after daemonize_start(), in which case we
will (after this commit) already have called setsid(), so no additional
change is required there.
Bug #14280.
Reported-by: Gordon Good <ggood@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
An ovs_be32 is a more obvious way to represent an IP address than a
pointer to one. It is also more type-safe, especially since "sparse" is
able to check that the argument is in network byte order.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>
The OpenFlow 1.0 specification says:
Emergency flow mod messages must have timeout values set to zero.
Otherwise, the switch must refuse the addition and respond with an
ofp_error_msg with OFPET_FLOW_MOD_FAILED type and
OFPFMFC_BAD_EMERG_TIMEOUT code.
but Open vSwitch reported OFPFMFC_TABLE_FULL in this case. This commit
fixes the problem.
Fixes detailed_contr_sw_messages.EmerFlowTimeout failure in OFTest.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>
We've had a couple of requests for this over the years. It's easy to do,
so let's implement it.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>
A few times while troubleshooting it would have been useful to get
complete logs, rather than post-rate-limiting snapshots of them. These
ovs-appctl commands make that possible.
Signed-off-by: Ben Pfaff <blp@nicira.com>
This was prompted by a conversation on the openflow-discuss mailing list
where developers of some OpenFlow switches mentioned that they save an
entire copy of raw flows passed in by controllers because of the
possibility that there might be wildcarded 1-bits, e.g. something like
192.168.1.1/255.255.0.0 instead of 192.168.0.0/255.255.0.0. I've always
intended that this not be necessary, but it was never explicitly written
down. This commit starts the process of updating OVS to make this a
requirement, by logging a warning whenever such a NXM or OXM entry is seen,
and by updating the spec in nicira-ext.h to describe my intent.
This is related to issue EXT-238 (OXM should require that 0-bits in mask
be 0-bits in value) in the Open Networking Foundation's "extensibility"
bugtracker at https://www.opennetworking.org/bugs/browse/EXT-238.
(Unfortunately one must be an employee of an ONF member company to
access this bug tracker. It's the network that's open, not the
foundation.)
Thanks to Zoltán Lajos Kis, Dan Talayco, Rob Sherwood, and HIDEyuki
Shimonishi for participating in the discussion on openflow-discuss.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Kyle Mestery <kmestery@cisco.com>
This is a trivial port to netdev-bsd of Justin Pettit's netdev-linux
commit 33d82a56d (netdev-linux: Use underlying tap device on
netdev_linux_listen().), which had the commit message:
Commit acf608 (ofproto-dpif: Use a single underlying datapath across
multiple bridges.) broke connectivity to userspace datapath devices. The
code assumed the first caller to open a tap device with
netdev_linux_open() wanted to write to it. This commit moves that logic
to when netdev_linux_listen() is called.
This fixes the userspace datapath on FreeBSD.
Signed-off-by: Ed Maste <emaste@freebsd.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Commit acf608 (ofproto-dpif: Use a single underlying datapath across
multiple bridges.) broke connectivity to userspace datapath devices.
The code assumed the first caller to open a tap device with
netdev_linux_open() wanted to write to it. This commit moves that logic
to when netdev_linux_listen() is called.
Thanks to Ben Pfaff for helping debug the issue.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Reported-by: Simon Horman <horms@verge.net.au>
Tested-by: Simon Horman <horms@verge.net.au>
