The CLASSIFIER_FOR_EACH_EXACT_RULE_SAFE macro was missing its "MEMBER"
argument. It doesn't currently cause any problems because no one uses
the macro.
If no controllers are specified on the command-line, ovs-openflowd adds
a couple of its own. The code that accounts for the controllers
correctly allocated space for them, but used the command-line count to
determine how many to set. This led to a segfault when later code tried
to dereference them.
Reported-by: Derek Cormier <derek.cormier@lab.ntt.co.jp>
In normal operation it makes sense to keep track of all of the flows that
have been seen recently and to cache all of them in the kernel. Under
unusual conditions, such as those caused by network scanning tools or by an
actual targeted DoS attack against the vswitch, the number of flows can
explode to extremely high numbers (hundreds of thousands or more). In such
a situation the vswitch needs to guard against memory exhaustion by
expiring flows more quickly and more often. This commit implements an
inexpensive technique for determining which flows should be dropped in such
a situation.
A wildcarded flow is idle only if all of its subrules have expired because
they were idle, so unless we expire exact-match rules first it is possible
that a wildcarded flow fails to expire as soon as it should.
(The current implementation of classifier_for_each() iterates through
exact-match rules before wildcarded rules, but nothing in the interface
guarantees that.)
This poll_immediate_wake() is unnecessary because netflow_run() is always
called afterward within the same poll loop. It's better to delete it, to
avoid wasting CPU.
In one or two corner cases, flows cannot be installed because every packet
in the flow must be processed by userspace. The code to expire rules was
ignoring these uninstallable rules, and thus they would never get freed,
even after they became idle. This commit fixes the problem.
This should be a purely stylistic change, with no effect on behavior.
This commit changes the callback pointer passed to the
classifier_for_each() from a pointer to an ofproto to a pointer to a
structure that includes an ofproto. Future commits planned will add
more members to this new structure.
GNU libc treats malloc(0) as malloc(1). Subrules always have an n_actions
of 0, so this code was wasting time and memory for subrules. This commit
stops doing that.
Also audits and fixes some very pedantic potential problems with null
pointers; e.g. the C standard says that NULL may not be compared with the
< operator, even if both arguments are null, and it also says that a null
pointer may not be passed to memcpy() or memcmp(), even if the length is
zero.
XenServer puts our header files in the standard system search path
by default. This is normally OK, except when we introduce new things
which aren't in those headers. Since the system picks up the older files
first this leads to undefined sysmbols.
Signed-off-by: Jesse Gross <jesse@nicira.com>
ovs-external-ids was crashing on startup because it was brought up
before /dev/log exists. The simplest solution to this problem is
to have it log to /var/log/openvswitch/ovs-external-ids.log . This
is consistent with vswitchd and ovsdb-server.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
With header caching we check to see if the next device in the stack
is an OVS device and, if so, cache that flow as well. However, the
test for this called internal_dev_get_vport() assuming that it would
return NULL if the device is not an internal device. It doesn't,
however, it just returns the offset from the device where the vport
data structure would be if it were an internal device. This changes
it to explicitly check for an internal device first to avoid a panic.
Bug #3470
Reported-by: Ram Jothikumar <rjothikumar@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Reviewed-by: Justin Pettit <jpettit@nicira.com>
The OpenFlow OFPAT_ENQUEUE action sets a queue id and outputs the packet
in one shot. There are times in which the queue should be set, but the
output port is not yet known. This commit adds the NXAST_SET_QUEUE and
NXAST_POP_QUEUE Nicira extension actions to modify the queue
configuration without requiring a port argument.
CC: Jeremy Stribling <strib@nicira.com>
CC: Keith Amidon <keith@nicira.com>
If the netflow byte counter is UINT64_MAX, or at any rate much larger than
UINT32_MAX, netflow_expire() could loop for a very long time. This commit
avoids that case.
This is only a theoretical bug fix. I don't know of any actual bug that
would cause a counter to be that high.
This makes it a little easier to test Open vSwitch QoS features using
ovs-controller, by making it possible to assign queues on the basis of
input port, instead of just allowing a single queue for a whole switch.
CC: Michael Mao <mmao@nicira.com>
A couple of people have reported that ovs-controller --with-flows is
confusing. This seems to be because it doesn't read the file with the
flows until the first connection from a switch. Then, if the file has a
syntax error, it exits.
This commit changes the behavior so that it reads the file immediately at
startup instead.
Without this commit, "ovs-ofctl queue-stats br0 ALL 1" will print something
like the following if port 3 has queue 1 but none of the other ports do:
stats_reply (xid=0x7b378): flags=none type=5(queue)
4 queues
port 0 queue 1: bytes=?, pkts=?, errors=?
port 1 queue 1: bytes=?, pkts=?, errors=?
port 2 queue 1: bytes=?, pkts=?, errors=?
port 3 queue 1: bytes=0, pkts=0, errors=0
With this commit, it will print the following instead, which seems more
useful:
stats_reply (xid=0x3ada1): flags=none type=5(queue)
1 queues
port 3 queue 1: bytes=0, pkts=0, errors=0
Linux kernel queue numbers are one greater than OpenFlow queue numbers, for
HTB anyhow. The code to dump queues wasn't compensating for this, so this
commit fixes it up.
This macro is a variant on CONTAINER_OF that takes an object pointer
instead of a type name as its second argument. In the following commit
this will simplify many users of CONTAINER_OF.
The main advantage of a sparse array over a hash table is that it can be
iterated in numerical order. But the OVS implementation of sparse arrays
is quite expensive in terms of memory: on a 32-bit system, a sparse array
with exactly 1 nonnull element has 512 bytes of overhead. In this case,
the sparse array's property of iteration in numerical order is not
important, so this commit converts it to a hash table to save memory.
The main advantage of a sparse array over a hash table is that it can be
iterated in numerical order. But the OVS implementation of sparse arrays
is quite expensive in terms of memory: on a 32-bit system, a sparse array
with exactly 1 nonnull element has 512 bytes of overhead. In this case,
the sparse array's property of iteration in numerical order is not
important, so this commit converts it to a hash table to save memory.
The main advantage of a sparse array over a hash table is that it can be
iterated in numerical order. But the OVS implementation of sparse arrays
is quite expensive in terms of memory: on a 32-bit system, a sparse array
with exactly 1 nonnull element has 512 bytes of overhead. In this case,
the sparse array's property of iteration in numerical order is not
important, so this commit converts it to a hash table to save memory.
The main advantage of a sparse array over a hash table is that it can be
iterated in numerical order. But the OVS implementation of sparse arrays
is quite expensive in terms of memory: on a 32-bit system, a sparse array
with exactly 1 nonnull element has 512 bytes of overhead. In this case,
the sparse array's property of iteration in numerical order is not
important, so this commit converts it to a hash table to save memory.
When ovs-vsctl is not actually going to modify the database, it is less
interesting in the log, so we might as well only log it at "debug" level.
Suggested-by: Neil McKee <neil.mckee@inmon.com>
The ethtool package is only used to help ovs-bugtool create bug
reports and does not prevent the script from running if it's
missing. Making it a requirement for installation is a bit
draconian.
The OVS Python daemon library will restart a process when started with
"--monitor" that exits with RESTART_EXIT_CODE. Have ovs-monitor-ipsec
exit with this code when an uncaught exception occurs.
The init script starts monitor-external-ids with --monitor when
configured to do so. Also made changes to guarantee that --monitor
actually restarts ovs-external-ids.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Header caching speeds up sending tunneled traffic by bypassing the Linux
IP stack. This also causes it to bypass IPsec processing, which will
break connectivity. This commit disables header caching when IPsec is
enabled.
Brad pointed out that openvswitch-ipsec init script defined the variable
DIETIME but attempted to use it as DODTIME. This commit uses DODTIME,
since it's the name used by the openvswitch-switch init script. The
openvswitch-controller init script had the same issue.
As suggested by Ben, the "s" suffixes are removed from sleep commands,
since they are a GNU extension.
Reported-by: Brad Hall <brad@nicira.com>