When we increment the Open vSwitch version number, we tend to forget to
update it in debian/changelog at the same time. Right now this gets
fixed up automatically at "make dist" time, but it's even better if we can
always have it be correct in the repository. This commit should help with
that, by making both "make" and "make dist" refuse to proceed if the
version number is out of sync.
This test tends to break when run with lcov profiling since the lcov
wrapper script can't synchronize access to profiling data across all the
ovs-vsctl instances running in parallel.
Linux traffic control handles with minor number 0 refer to qdiscs, not
to classes. This commit deals with this by using a conversion function:
OpenFlow queue 0 maps to minor 1, queue 1 to minor 2, and so on.
A netdev-linux traffic control implementation has to dump all of a port's
traffic classes in a couple of different situations. start_queue_dump()
is supposed to do that. But it was specifying TC_H_ROOT as tcm_parent,
which only dumped classes that were direct children of the root. This
commit changes tcm_parent to 0, which obtains all traffic classes
regardless of parent.
When the QoS code was integrated, I didn't yet know how to abstract the
translation from a queue ID in an OpenFlow OFPAT_ENQUEUE action into a
priority value for an ODP ODPAT_SET_PRIORITY action. This commit is a
first attempt that works OK for Linux, so far. It's possible that in fact
this translation needs the 'netdev' as an argument too, but it's not needed
yet.
Before, ovs-controller always sent packets using OFPAT_OUTPUT, which always
uses the default OpenFlow queue. To help me debug the Open vSwitch QoS
implementation, I want to be able to send packets on other queues, so
this commit adds that feature.
Before, an lswitch always sent packets using OFPAT_OUTPUT, which always
uses the default OpenFlow queue. To help me debug the Open vSwitch QoS
implementation, I want to be able to send packets on other queues, so
this commit adds that feature.
There's no need to calculate the wildcards to use for each flow, since it
is a constant across every flow. In my opinion this also makes
process_packet_in() a little easier to understand, since it deletes a few
lines of code from a relatively complicated function.
A switch is not supposed to forward packets directed to MAC addresses
01:80:c2:00:00:0x. This code was instead dropping packets *from* those
addresses.
(This code is only used by ovs-controller, so the bug is not a big deal.)
The compiler pointed out two variables that it thought were used
without being initialized. The first was just a spurious warning
but the second could result in an unitialized pointer being freed.
This fixes both of those issues.
It's pretty risky to allow record names to be abbreviated. If eth1 through
eth20 all exist, and then someone deletes eth1, then until now an ovs-vsctl
command that mentioned eth1 would actually use eth10. This is too much of
a caveat to let loose on unsuspecting scripts, so this commit removes that
functionality.
This fixes the converse of the problem addressed by commit fe19e820
"xenserver: Kill bond master's dhclient when bringing up bond slave". In
that commit's log message, I claimed that the converse was not a problem,
but I was wrong. I must have screwed up in testing, because it really is
a problem. This commit fixes it.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
CC: Dominic Curran <dominic.curran@citrix.com>
Reported-by: Michael Mao <mmao@nicira.com>
Bug #2668.
Many people who are looking for an OpenFlow switch assume that
the only way to do it is using ovs-openflowd. Sometimes they also
run ovs-vswitchd at the same time. Try to clarify that neither
of these are necessary and ovs-vswitchd can handle OpenFlow by
itself and is the preferred method of doing so.
Currently the flow key is updated to match an action that is applied
to a packet but these field are never looked at again. Not only is
this a waste of time it also makes optimizations involving caching
the flow key more difficult.
GRE is a somewhat annoying protocol because the header is variable
length. However, it does have a few fields that are always present
so we can make the parsing seem less magical by using a struct for
those fields instead of building it up field by field.
We currently remove ports from the GRE hash table and then immediately
free the ports. Since received packets could be using that port this
can lead to a crash (the port has already been detached from the
datapath so this can't happen for transmitted packets). As a result
we need to wait for an RCU grace period to elapse before actually
freeing the port.
In an ideal world we would actually remove the port from the hash
table in a hypothetical gre_detach() function since this is one of
the purposes of detaching. However, we also use the hash table to
look for collisions in the lookup criteria and don't want to allow
two identical ports to exist. It doesn't matter though because we
aren't blocking on the freeing of resources.
In some places we would put the return type on the same line as
the rest of the function definition and other places we wouldn't.
Reformat everything to match kernel style.
We currently use EEXIST to represent both a device that is already
attached and for GRE devices that are the same as another one.
Instead use EBUSY for already attached devices to disambiguate the
two situations.
The offsets for checksum offsets should always be positive so make
that explicit by using unsigned ints. This helps bug checks that
test if the offsets are greater than their upper limits.
The \" escape is not part of POSIX, but it is a common extension. The
dash shell's built-in "printf" implementation does not include this
extension, which caused the testsuite to be generated incorrectly if it
is used as the default shell (as it is on newer versions of Debian and
Ubuntu).
However, there's no reason to use a backslash here at all, so just omit
it.
Until now, ofpbuf_trim() has not handled the case where the ofpbuf has
nonzero headroom. This causes an assertion failure when pinsched_send()
queues a packet to be sent later, because such packets have been
guaranteed to have nonzero headroom since commit 43253595 "ofproto: Avoid
buffer copy in OFPT_PACKET_IN path." This commit fixes the problem by
implementing the until-now unsupported case.
This commit factors code out of ofpbuf_prealloc_tailroom() into two new
functions, ofpbuf_rebase__() and ofpbuf_resize_tailroom__(), and uses the
latter to implement both ofpbuf_prealloc_tailroom() and ofpbuf_trim().
ofpbuf_rebase__() isn't used on its own at all, but it seems potentially
useful so I made it an independent function.
Reported-by: Tom Everman <teverman@google.com>
Our code that handles checksumming does essentially the same thing
as skb_checksum_help() except it folds the process into copying to
userspace. This makes the two functions more closely resemble
each other in structure, including adding a couple of BUG() checks.
This should have no functional change but makes comparision easier
when debugging.
If we need to make a copy we add a little bit extra to the headroom
that we think we need in order to avoid future copies. Previously we
would always just allocate max(headroom, 64) which is generally
enough space. However, if we are using IPsec our expected headroom
is large and there is no extra. Instead, it is better to just tack on
a few extra bytes so we'll always have a cushion.
We currently check for packets that are over the MTU in do_output().
There is a one-to-one correlation between this function and
vport_send() so move the MTU check there for consistency with
other error checking.
New types of vports such as patch and GRE make it possible to
connect multiple (or the same) datapaths together, which introduces
the possibility of loops on a single machine. Local loops are
even worse than network loops because they will quickly crash the
machine once the kernel stack is exhausted. This adds loop
checking within the OVS datapath that will drop packets that have
been seen more than 5 times.
CC: Paul Ingram <paul@nicira.com>
OVS can usually find the kernel source itself, but when it can't it's
handy if the user can just specify it on the "configure" command line.
Signed-off-by: Ian Campbell <Ian.Campbell@citrix.com>.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Reported-by: Jad Naous <jnaous@gmail.com>
A "feature" that ovs-vswitchd inherited from its previous form of
configuration is that every mirror has a name. Names are not necessarily
meaningful, and there is no reason that they should be unique. But the
existing implementation depends on them being unique within a given
bridge, and if they are not drops one of the duplicates.
This commit drops the uniqueness requirement. Instead, it distinguishes
mirrors based on UUID alone.
This commit does not drop the concept of names for mirrors. There is no
technical reason to retain them, but it is possible that users find them
useful for management reasons. The names appear in log messages related
to mirrors, which may make the messages easier to understand.
Bug #2416.
It can be useful to get direct access to the "struct ovsdb_datum"s that
underlie the "cooked" data structures maintained by the IDL code. This
commit provides a convenient interface through the IDL.
The existing ovsdb_idl_txn_read() was somewhat difficult and expensive to
use, because it always made a copy of the data in the column. This was
necessary at the time it was introduced, because there was no way for it
to return a "default" value for columns that had not yet been populated
without allocating data and hence requiring the caller to free it.
Now that ovsdb_datum_default() exists, this is no longer required. This
commit introduces a pair of new functions, ovsdb_idl_read() and
ovsdb_idl_get(), that return a pointer to existing data and do not do any
copying. It also transitions all of ovsdb_idl_txn_read()'s callers to
the new interfaces.
ovsdb_datum has an important invariant (documented in the large comment
on the declaration of struct ovsdb_datum) that a lot of code relies upon:
keys must be unique and in sorted order. Most code that creates or
modifies ovsdb_datum structures maintains this invariant. However, the
"set" functions generated by ovsdb-idlc.in do not check or restore it.
This commit adds that checking.
This might fix an actual bug or two--none have been reported--but mostly it
is just to add some additional checking to avoid future subtle bugs.
These new functions are more forgiving than the corresponding functions
without "_unique". The goal is to be more tolerant of data provided by
IDL clients, which will happen in a followup patch.
Until now, "monitor" has only allowed the client to choose the kinds of
changes that will be monitored on a per-table basis. However, it makes
sense to be able to choose operations on a per-column basis. The
immediate need for this is to make sure that the final statistics of
deleted Interface records are known at time of deletion, even though the
intermediate values of the statistics are not important.
CC: Jeremy Stribling <strib@nicira.com>
This code assumed that the types of operations that were selected were
default-off, so it only added JSON to the query to turn on the ones that
were wanted, but in fact they are default-on, so this commit changes it
to add JSON for each possible operation type.
RFC 4627 (which defines JSON) says:
The names within an object SHOULD be unique.
In my view, this means that the treatment of duplicate names within a
JSON object is more or less up to the implementation. Until now, the OVS
JSON parser has dealt with duplicates fairly badly: they all get shoved
into the hash table and you get one or the other value semi-randomly
(typically the one added later). This commit makes the behavior
predictable: old values are deleted and replaced by newer values.
By breaking out the hash calculation we can enable operations that need
to do both to avoid duplicating the hash calculations. A following commit
will add such an operation.
