Justin Pettit [Mon, 26 Oct 2009 19:02:02 +0000 (12:02 -0700)]
flow: Differentiate between "port" when printing flows
When printing a flow, there were two references to "port": one the
interface the packet arrived on and the other the L4 ports. This could
be a bit confusing to new users looking at the output of a command such
as "ovs-ofctl dump-flows". This commit changes the incoming interface
field from "port" to "in_port".
Jesse Gross [Thu, 22 Oct 2009 18:40:04 +0000 (11:40 -0700)]
bridge: Eject NORMAL flows without a learning entry from datapath.
When revalidating NORMAL flows we consult the learning table, which
could be empty if a packet hasn't come to userspace in a while or we
just did a bridge flush. If there is no learning entry then existing
flows will begin flooding packets until a new flow is setup. The
problem is worse with bonding because we can receive one of the flooded
packets back on a bond slave and learn that port, causing us to send
traffic to the wrong location.
Jesse Gross [Wed, 21 Oct 2009 02:26:55 +0000 (19:26 -0700)]
bonding: Balance bond slaves based on ratio.
Previously when deciding whether to migrate a hash between slaves
we would never move it if it would cause more load on the new slave
than the old. This could lead to a situation where the slaves would
be imbalanced but no migration would occur since it would flip the
load. This will do the migration if it will decrease the ratio.
Jesse Gross [Mon, 12 Oct 2009 20:09:51 +0000 (13:09 -0700)]
bonding: Drop unicast packets which have a different learned port.
Drop packets received on a bond port if we have learned a different
source port for that MAC. We were already doing this for multicast
packets but extend the logic to unicast packets as well since the
same situation can occur if the connected switch has not learned the
MAC address and is flooding. Otherwise vSwitch will learn the bond
port as the source of that MAC.
Jesse Gross [Tue, 20 Oct 2009 03:14:31 +0000 (20:14 -0700)]
xen: Restore state files for VIF VLANs
A change on master to use ovs-vsctl instead of state files for VLANs
was ported to the citrix branch, which does not have ovs-vsctl. The
interface reconfigure portion, which does not store the state files
was ported but the vif-hotput script portion was not. This restores
interface reconfigure to again save the state files.
Ben Pfaff [Thu, 15 Oct 2009 17:39:10 +0000 (10:39 -0700)]
Make sure that time advances in a daemon between calls to time_refresh().
Open vSwitch uses an interval timer signal to tell it that its cached idea
of the current time has expired. However, this didn't work in a daemon
detached from the foreground session (invoked with --detach) because a
child created with fork() does not inherit the parent's interval timer and
we did not re-set it after calling fork().
This commit fixes the problem by setting the interval timer back up after
calling fork() from daemonize().
This fix is based on code inspection (which was then verified to be correct
through testing). It may not fix any actual problems in practice, because
time_refresh() is called every time through the poll loop, and the poll
loop typically runs more quickly than the periodic timer fires (1 ms or so
average in ovs-vswitchd, vs. 100 ms timer interval).
Ben Pfaff [Wed, 24 Jun 2009 21:58:57 +0000 (14:58 -0700)]
datapath: Ignore return value from rtnl_notify().
In Linux 2.6.30, the rtnl_notify() return type was changed from int to
void along with the following commit message:
This patch also modifies the rtnetlink code to ignore the return
value of rtnl_notify() in all callers. The function rtnl_notify()
(before this patch) returned the error of the unicast notification
which makes rtnl_set_sk_err() reports errors to all listeners. This
is not of any help since the origin of the change (the socket that
requested the echoing) notices the ENOBUFS error if the notification
fails and should resync itself.
Thus there's no point in checking the return value, even in older versions
of the kernel, and so this commit changes our code to ignore it, even
on older kernel versions. We also update the rtnl_notify() wrapper macros
to make the return type void on older kernel versions.
This has not been tested, just built.
Thanks to Mikio for spurring me to try building with Linux 2.6.29 and
2.6.30.
Jesse Gross [Thu, 8 Oct 2009 19:31:03 +0000 (12:31 -0700)]
vlan: Compare vlan tags before implicit tagging when RSPANing.
We check that a packet is not sent out the on the in port on the
same VLAN when performing RSPAN. However, we were comparing the
vlan tag from a packet after implicit tagging with a tag from
before implicit tagging. This ensures that we always compare them
before such tagging.
Ben Pfaff [Fri, 9 Oct 2009 16:41:29 +0000 (09:41 -0700)]
datapath: Fix build with Centos 5.3 kernel.
Centos 5.3 backports more functions from later kernel versions to 2.6.18,
so the kernel version number is no longer a reliable way to check for these
functions. Thus, add a "configure" test for them.
Ben Pfaff [Wed, 7 Oct 2009 19:07:27 +0000 (12:07 -0700)]
xenserver: Crossport "master" interface-reconfigure to "citrix".
This commit copies the interface-reconfigure script from "master" into
"citrix" and fixes up a few incompatibilities: the location of ovs-cfg-mod,
which in master is in /usr/bin and in citrix is in /root/vswitch/bin, and
the RPM spec file fragments needed to initialize the database cache.
The purpose of this commit is to obtain the bug fixes that have been
applied (mainly by Ian Campbell) to "master" but which are not in "citrix".
It's difficult to understand the changes from this commit alone. It is
more meaningful to compare the resulting files against those currently
on the master branch.
Jesse Gross [Fri, 14 Aug 2009 20:47:28 +0000 (13:47 -0700)]
ofproto: Make current packet counts more accurate.
When the stats for a currently active flow are requested this
ensures that the packets not handled by the kernel are counted
immediately. Before, these packets would only be counted once
the kernel flow expired and the counts were combined.
Jesse Gross [Thu, 8 Oct 2009 19:20:10 +0000 (12:20 -0700)]
bonding: Compare ports, not interfaces, for loop checks.
In order to avoid loops we check that the input and output ports
are not equal. When selecting mirror outputs for RSPAN we were
checking interfaces instead of ports. This lead to loops when
using RSPAN with bonded ports.
Jesse Gross [Mon, 5 Oct 2009 20:25:19 +0000 (13:25 -0700)]
netflow: Increase maximum number of NetFlow records to 30.
NetFlow v5 allows up to 30 records per packet but we were incorrectly
limiting to 29. This corrects that and also uses the count of the
number of records in the header rather than the packet size since
it is easier to reason about.
Jesse Gross [Sat, 3 Oct 2009 01:08:05 +0000 (18:08 -0700)]
bonding: Update the link status on the bond fake interface.
Brings the fake bond interface up and down to match our notion of
whether the bond is currently active. This solves an issue where
XenCenter would always show the bond as disconnected.
Ben Pfaff [Fri, 2 Oct 2009 20:29:01 +0000 (13:29 -0700)]
vswitch: Allow user to set Ethernet address of any internal interface.
Until now the vswitch configuration file has allowed the user to configure
the MAC address on bridge local ports only. This commit adds the ability
to configure them on any internal interface.
It would be logical to extend this to any bridge port, period, but many
network devices must be brought down before their Ethernet addresses may be
changed. Bringing a network interface down and then back up can reset a
lot of state, so as we don't actually need the ability to change any bridge
port's MAC address yet this commit does not implement it.
Ben Pfaff [Mon, 28 Sep 2009 23:03:49 +0000 (16:03 -0700)]
xenserver: Add script refresh-xs-network-uuids.
On pool join, the bridge.<bridge>.xs-network-uuids key is not updated
properly for the primary management interface. We don't have a proper
fix for this problem yet, and probably won't ever have one for XenServer
5.5.0, so this commit adds a script that works around the problem.
Running the script is a shortcut for rebooting the XenServer host,
which should also solve the problem.
There have been numerous attempts at getting in-band correct. If
history is at all an example, it probably still isn't. However, this is
an attempt to document its current design, so that we can understand
what our current thinking is.
ovs-appctl: Correct "target" option synopsis in man page
The synopsis section of the man page for ovs-appctl incorrectly stated
that the target option takes "pid" as an argument. This commit corrects
that to say "socket".
Ben Pfaff [Wed, 16 Sep 2009 22:12:19 +0000 (15:12 -0700)]
secchan: Better tolerate failing controller admission control in fail-open.
When the switch is configured to connect to a controller that accepts
connections, waits a few seconds, and then disconnects without setting up
flows, currently this causes "fail-open" to flush the flow table and
stop setting up new flows during the connection duration. This is OK if
it happens once, but it can easily happen every 8 seconds with typical
backoff settings, and that isn't so great.
This commit changes fail-open to only flush the flow table once the switch
appears to have been admitted by the controller, which prevents these
frequent network interruptions.
Thanks to Jesse Gross for especially valuable feedback.
QA notes: Behavior in fail-open and especially behavior with a controller
that rejects the switch after it connects needs to be re-tested. The
ovs-controller --mute switch added by this commit is one simple way to
create such a controller.
CC: Peter Balland <peter@nicira.com>
Bug #1695. Bug #2055.
Ben Pfaff [Tue, 15 Sep 2009 22:23:45 +0000 (15:23 -0700)]
Factor out code for composing OFPT_PACKET_IN messages.
Currently only ofproto.c ever composes OFPT_PACKET_IN messages, but some
upcoming code wants to do the same thing, so factor this out into a new
function to avoid code duplication.
Ben Pfaff [Tue, 15 Sep 2009 22:22:17 +0000 (15:22 -0700)]
Factor out code for composing benign packets.
The bonding code in vswitch sends out gratuitous learning packets that
are supposed to teach switches but not cause anything else to happen on
the network. Some upcoming code wants to synthesize packets with similar
properties, so factor this code into a new function so that it can be
used in both places.
vswitch: Correctly identify slave status in /proc compatibility layer
In the /proc compatibility layer, the bond member was reported as up
immediately after link recovery, regardless of the updelay. I believe
the compatibility code was correct if the check had been done with carrier,
but since 'iface->enabled' already does that calculation, we can use it
directly.
Additinally, when a bond slave was enabled or disabled, the bond
compatibility code was not being told to update its state. This commit
makes that call.
The original xen-bugtool did not collect any OVS logs. Now that more
logging is moving from /var/log/messages to ovs-vswitchd's and
ovs-brcompatd's private log files, we should include them in the
information collected for bug reports.
Jean Tourrihles [Fri, 11 Sep 2009 22:19:15 +0000 (15:19 -0700)]
vconn: Fix byte-swapping of message type when validating OpenFlow actions.
It seems really strange that this one slipped through. Perhaps this
means that we have never tested with any action other than OFPAT_OUTPUT
(which has value 0 and thus is not affected by byte-swapping).
xenserver: Set default file log level and fix brcompatd file
Set the default log level for file logging to INFO for ovs-vswitchd and
ovs-brcompatd. This is done so that coverage messages are kept in the
log file, since we no longer log them through syslog due its synchronous
writing on Xen hosts. The issue is described in detail in commit 6bc995e.
Fix test for whether file logging should be enabled for ovs-brcompatd.
logging: Lower logging level for coverage messages due to slow poll loops
By default, many OVS processes keep track of their time through a poll
loop. If it takes an unusually long time (measured as some distance
from the mean), the processes will log stats it has been keeping about
coverage. It was doing this at level WARN.
On Xen systems, syslog messages written at level INFO and higher are
written to /var/log/messages synchronously. This would mean that there
would be dire messages that it took a few dozen milliseconds to go
through the loop, meanwhile, it would take up to 6(!) seconds writing
those. Meanwhile, the process would do no other processing, which could
be quite serious in the case of a process such as ovs-vswitchd.
This problem was somewhat masked because the time used by this logging
was not used in the calculations for determining how long it was taking
to get through the loop.
This commit lowers the default log level for those coverage messages to
INFO. On Xen systems, it raises the default level at which messages are
written to syslog to WARN.
Diagnosed and fixed with the help of Ian Campbell.
Ben Pfaff [Wed, 9 Sep 2009 17:52:26 +0000 (10:52 -0700)]
Increase the default priority of vswitchd and brcompatd
Our hope is that this will resolve many of the issues we have seen
where temporary delays in the forwarding of packets have caused issues
of various types.
This is a crossport from master of commit 44cb492 by Ian Campbell.
datapath: Don't drop MTU-sized VLAN packets from userspace
Before transimitting a packet, the datapath checks that the packet
length is not greater than the MTU. It determines the length based on
the 'protocol' field in the skb. If 'protocol' is ETH_P_8021Q, it reduces
the packet length as stored in the 'len' field by four bytes, which
is the size of a VLAN tag header. Unfortunately, packets that arrived
from userspace were not having the 'protocol' field set, which would
cause MTU-sized packets to be dropped. This commit sets the 'protocol'
field appropriately.
Thanks to Ben Pfaff for the help diagnosing this issue.
Ben Pfaff [Thu, 3 Sep 2009 21:28:44 +0000 (14:28 -0700)]
xenserver: Install /etc/sysconfig/vswitch by default.
Until now, the vswitch RPM has installed /etc/sysconfig/vswitch.example
and made the system administrator copy it to /etc/sysconfig/vswitch if he
desires. This is slightly inconvenient, since it is slightly easier for
the admin if he can just edit /etc/sysconfig/vswitch directly. This commit
changes to the latter behavior.
Ben Pfaff [Thu, 3 Sep 2009 19:51:51 +0000 (12:51 -0700)]
xenserver: Create vswitchd configuration file if it does not exist.
/etc/ovs-vswitchd.conf should always be there. Nevertheless, it is not
nice to entirely break vswitch if it is accidentally deleted. This commit
makes /etc/init.d/vswitch create an empty configuration file if it is
missing.
Ben Pfaff [Wed, 2 Sep 2009 19:52:50 +0000 (12:52 -0700)]
rconn: Speed up in-band control connections, by caching the remote address.
In-band control needs to know the IP and port of the controller, so that
it can set up the correct flows to talk to that controller. Until now,
the rconn code has only made this available when a connection was actually
in progress. This means that, say, ARP packets will not be allowed through
when the rconn backs off. The same is true of packets sent by switches
that access the controller through this one.
This commit makes the rconn cache the remote IP and port and local IP
across connection attempts, improving the situation. In particular, it
reduces the overall amount of time that it takes to connect in my own
simple test case from over 10 seconds to about 2 seconds.
Justin Pettit [Mon, 31 Aug 2009 22:18:43 +0000 (15:18 -0700)]
in-band: Implement L3-based in-band control
Previously, in-band control was L2-based. This worked well when the
controller was on the same network segment as the switch. However, many
configurations are not set up this way. These changes allow a switch and
controller to be on different subnets.
This set of changes also fixes some problems related to passing DHCP
traffic as described in Bug #1618.
A full description of the reasoning and supported configurations of
in-band will be forthcoming.
Add ability for the datapath to match IP address in ARPs
The ability to match the IP addresses in ARP packets allows for fine-grained
control of ARP processing. Some forthcoming changes to allow in-band
control to operate over L3 requires this support if we don't want to
allow overly broad rules regarding ARPs to always be white-listed.
Unfortunately, OpenFlow does not support this sort of processing yet, so
we must treat OpenFlow ARP rules as having wildcarded those L3 fields.
secchan: Tighten in-band traffic always allowed into switch
In-band control sets up a bunch of invisible flows that allow the switch
and controller to communicate over OpenFlow. The rules may have been a
bit too permissive, since it allowed any traffic to reach the
connection's interface. This set of changes tries to tighten that to
only OpenFlow traffic and ARPs.
Justin Pettit [Thu, 27 Aug 2009 22:13:59 +0000 (15:13 -0700)]
in-band: Fix status checks that could prevent in-band updates
The method the status callback was using to retrieve the local and
remote MAC addresses pushed back the refresh timer. If this were done
frequently, it could prevent in-band control from updating its rules.
Ben Pfaff [Tue, 1 Sep 2009 17:31:32 +0000 (10:31 -0700)]
datapath: Use hash table more tolerant of collisions for flow table.
The hash table used until now in the kernel datapath for storing the flow
table provides only two slots that a given flow can occupy. If both of
those slots are already full, for a given flow, then that flow cannot be
added at all and its packets must be handled entirely in userspace, taking
a performance hit. The code does attempt to compensate for this by making
the flow table rather large: 8 slots per flow actually in the flow table.
In practice, this is usually good enough, but some of the tests that we
have run show bad enough performance degradation or even timeouts of
various kinds that we want to implement something better.
This commit replaces the existing hash table by one with a completely
different design in which buckets are flexibly sized and can accept any
number of collisions. By use of suitable levels of indirection, this
design is both simple and RCU-compatible. I did consider other schemes,
but none of the ones that I came up with shared both of those two
properties.
This commit also adds kerneldoc comments for all of the flow table
non-static functions and data structures.
This has been lightly tested for correctness. It has not been tested for
performance.
Ben Pfaff [Fri, 28 Aug 2009 20:05:48 +0000 (13:05 -0700)]
corekeeper: Always include PID in core dump names.
Some distributions automatically set /proc/sys/kernel/core_uses_pid to 1
and others leave it at its default setting of 0. That means that, with the
core_pattern that corekeeper was setting, on the former distributions the
PID would be included in core names and on the latter the PID would be
omitted. For consistency, this commit forces the PID to be in the core
file name in either case (note that putting %p in core_pattern causes
the core_uses_pid setting to be disregarded).
Ben Pfaff [Fri, 28 Aug 2009 21:59:42 +0000 (14:59 -0700)]
secchan: Avoid sending NetFlow packets for empty flows.
There is no value in sending out NetFlow messages when the byte counter
(hence, packet counter) is 0. This does not often happen, but it can in
corner cases where a flow gets installed but never sees any traffic before
it is uninstalled.
Ben Pfaff [Mon, 24 Aug 2009 18:06:34 +0000 (11:06 -0700)]
vswitchd: Mirror nothing, not everything, if mirror ports don't exist.
If all of the ports specified as mirror selection criteria actually do not
exist, then until now the bridge would mirror all incoming packets (on
specified VLAN(s), if any). This matches the behavior that occurs if no
mirror selection ports were specified at all, and so it makes a certain
amount of logical sense.
But it is far more likely that the user simply misspelled a port name, or
specified the name of a port that does not always exist. In fact we have
seen this behavior in practice when the controller has not caught up to
the switch's current configuration. So this commit changes the bridge to
instead disable a mirror if ports are specified and none of those ports
exist.
Ben Pfaff [Mon, 24 Aug 2009 17:42:44 +0000 (10:42 -0700)]
vswitchd: Avoid output port explosion with mirrors that output to VLANs.
compose_dsts() was updating the VLAN of packets sent to VLAN mirrors
before it changed the VLAN value, but of course it's the final VLAN value
that actually matters.
Thanks to Reid for his good work tracking this one down.
Justin Pettit [Tue, 25 Aug 2009 19:34:45 +0000 (12:34 -0700)]
mgmt: Cleanup handling of extended messages
OpenFlow has a maximum messages size of 65536 bytes, but management
messages can be greater than that. The management protocol's Extended
Data message is used to get around that limitation. This commit cleans
up some problems with our implementation and adds some additional
sanity-checking to received messages.
Ben Pfaff [Wed, 19 Aug 2009 22:59:18 +0000 (15:59 -0700)]
xenserver: Renice netback process to priority 0 by default.
Under heavy VM network load, we have observed that ovs-vswitchd can be
starved for CPU time, which prevents flows from being set up. This can
in turn cause connections to XAPI in Dom0 to time out (among other issues).
It is probably not necessary to renice netback all the way to priority 0
as done in this commit. That is simply the value that we have tested. QA
has not reported any ill side-effects of this choice of value (yet). One
reasonable alternative, should any problems be noticed, would be to leave
netback at its default -5 priority and simply boost ovs-vswitchd's priority
to say -6 or -7.
Ben Pfaff [Wed, 19 Aug 2009 21:14:40 +0000 (14:14 -0700)]
xenserver: Fix "brctl show" compatibility by introducing "brctl" wrapper.
Bug NIC-19, which reported that "brctl show" did not format its output in
the way expected by Citrix QA scripts, was believed fixed by commit 35c979bff4 "vswitchd: Support creating fake bond device interfaces."
Unfortunately, this commit was not tested on a XenServer before it was
committed. Due to differences in the actual test environment and the
XenServer environment, which have different versions of the bridge-utils
package that contains brctl, that commit did not fix the problem observed
by Citrix QA. In particular, the XenServer brctl uses sysfs to obtain
the information displayed by "brctl show", but the previous commit only
fixed up the information output by the bridge ioctls.
The natural way to fix this problem would be to fix up the sysfs support
as well. I started out along that path, but became bogged down in all
the details of the kernel sysfs.
This commit takes an alternate approach, by introducing a wrapper around
the system brctl binary that implements "brctl show" itself and delegates
all other functionality to the original binary (in a different location).
This will not fix tools that do not call into brctl, but to the best of
my knowledge there are no such tools used in the Citrix QA process.
Ben Pfaff [Wed, 19 Aug 2009 19:59:56 +0000 (12:59 -0700)]
xenserver: Completely ignore datapath devices for renaming purposes.
Commit 2bb451b69 "xenserver: Rename network devices to match MAC addresses
of physical PIFs" started renaming network devices so that they match
the MAC address that we expect them to have. This worked OK at the time.
Commit 35c979bff "vswitchd: Support creating fake bond device interfaces"
later started creating fake bond devices to make the Citrix QA scripts
happier.
Unfortunately these commits interact badly: the bond devices created by
the latter commit are sometimes chosen as the physical devices to be
renamed over the physical PIF device names. This is because we do allow
datapath internal ports to be chosen as "physical devices" as a last
resort. This commit reverses this decision, eliminating that possibility.
This probably won't become a problem unless somehow we encounter a physical
Ethernet card driver that lacks a queue, but that is unlikely since the
performance would be awful.
Ben Pfaff [Wed, 19 Aug 2009 19:51:27 +0000 (12:51 -0700)]
datapath: Additional fixes for datapath device renaming.
Commit c874dc6d6b "secchan: Fix behavior when a network device is renamed."
fixed a crash in the datapath when network devices within a datapath were
renamed. However, this missed the case where the device that was renamed
was a datapath's internal port: these devices have their br_port members
set to NULL, so we have to determine that they belong to a datapath another
way. This commit does so.
This commit also changes the initialization order in dp_dev_create().
Otherwise, dp_device_event() will dereference null when it is called via
register_netdevice(), because the newly created device is a datapath device
but its members are not yet initialized.
Justin Pettit [Mon, 17 Aug 2009 19:43:59 +0000 (12:43 -0700)]
xenserver: Add missing argument for fake-iface config
The interface-reconfigure script may add the 'fake-iface' configuration
option to the ovs-vswitchd.conf, but neglects to mention the interface.
This resulted in a "bonding.%s.fake-iface=true" line, which is clearly
wrong. This commit corrects that behavior.