Ryan Moats [Tue, 20 Sep 2016 15:35:46 +0000 (10:35 -0500)]
ofproto-dpif-xlate: Fix memory leak in execute_controller_action.
commit df70a7731 ("ofproto-dpif-xlate: Allow translating
without side-effects.") created a memory leak by removing the
dp_packet_delete statement in execute_controller_action that
freed the earlier cloned packet. This commit restores this
statement to the end of the method.
Fixes: df70a7731 ("ofproto-dpif-xlate: Allow translating without side-effects.") Signed-off-by: Ryan Moats <rmoats@us.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
We start the communication between VM1 and VM2, for example, ICMP.
At the meantime, disconnect OVS and SDN controller, and reconnect
them again, the process ovs-vswitchd crashes.
backtrace:
0 0x00007f658082ffe4 in cls_rule_make_invisible_in_version ()
1 0x00007f65807f6bb3 in delete_flows_start__ ()
2 0x00007f65807f7ee9 in ofproto_group_mod_start ()
3 0x00007f65807fa07b in handle_openflow ()
4 0x00007f658082119b in connmgr_run ()
5 0x00007f65807f3ba6 in ofproto_run ()
6 0x00007f65807e101c in bridge_run__ ()
7 0x00007f65807e715d in bridge_run ()
8 0x00007f658065784d in main ()
Signed-off-by: Binbin Xu <xu.binbin1@zte.com.cn> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ciara Loftus [Fri, 19 Aug 2016 09:22:30 +0000 (10:22 +0100)]
netdev-dpdk: Add new 'dpdkvhostuserclient' port type
The 'dpdkvhostuser' port type no longer supports both server and client
mode. Instead, 'dpdkvhostuser' ports are always 'server' mode and
'dpdkvhostuserclient' ports are always 'client' mode.
Suggested-by: Daniele Di Proietto <diproiettod@vmware.com> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
tun-metadata: Manage tunnel TLV mapping table on a per-bridge basis.
When using tunnel TLVs (at the moment, this means Geneve options), a
controller must first map the class and type onto an appropriate OXM
field so that it can be used in OVS flow operations. This table is
managed using OpenFlow extensions.
The original code that added support for TLVs made the mapping table
global as a simplification. However, this is not really logically
correct as the OpenFlow management commands are operating on a per-bridge
basis. This removes the original limitation to make the table per-bridge.
One nice result of this change is that it is generally clearer whether
the tunnel metadata is in datapath or OpenFlow format. Rather than
allowing ad-hoc format changes and trying to handle both formats in the
tunnel metadata functions, the format is more clearly separated by function.
Datapaths (both kernel and userspace) use datapath format and it is not
changed during the upcall process. At the beginning of action translation,
tunnel metadata is converted to OpenFlow format and flows and wildcards
are translated back at the end of the process.
As an additional benefit, this change improves performance in some flow
setup situations by keeping the tunnel metadata in the original packet
format in more cases. This helps when copies need to be made as the amount
of data touched is only what is present in the packet rather than the
maximum amount of metadata supported.
datapath: backport: openvswitch: use alias for genetlink family names
Upstream commit:
commit ed227099dac95128e2aecd62af51bb9d922e5977
Author: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
Date: Fri Sep 9 17:42:30 2016 -0300
openvswitch: use alias for genetlink family names
When userspace tries to create datapaths and the module is not loaded,
it will simply fail. With this patch, the module will be automatically
loaded.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> Signed-off-by: Jesse Gross <jesse@kernel.org>
Ben Pfaff [Thu, 15 Sep 2016 03:59:10 +0000 (20:59 -0700)]
ofp-parse: Fix sparse warnings about comparing ofp_port_ts.
Without this, sparse complains:
lib/ofp-parse.c:588:19: warning: restricted ofp_port_t degrades to integer
lib/ofp-parse.c:588:31: warning: restricted ofp_port_t degrades to integer
This is one of the irritating bits of using sparse, but on the whole I
think it saves us pretty often.
CC: Jarno Rajahalme <jarno@ovn.org> Fixes: 6dd3c787f591 ("ofproto: Support packet_outs in bundles.") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Joe Stringer <joe@ovn.org>
Ben Pfaff [Thu, 15 Sep 2016 18:43:46 +0000 (11:43 -0700)]
ofproto-dpif-xlate: Fix treatment of mirrors across patch port.
When the bridges on both sides of a patch port included mirrors, the
translation code incorrectly conflated them instead of treating them as
independent.
Reported-by: Zoltán Balogh <zoltan.balogh@ericsson.com> Reported-by: Sugesh Chandran <sugesh.chandran@intel.com>
Reported-at: http://openvswitch.org/pipermail/discuss/2016-September/022689.html Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Zoltán Balogh <zoltan.balogh@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Add a new select group selection method "dp_hash", which uses minimal
number of bits from the datapath calculated packet hash to inform the
select group bucket selection. This makes the datapath flows more
generic resulting in less upcalls to userspace, but adds recirculation
prior to group selection.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
ofp-parse: Remove double uninit of group mod if parsing fails.
Double ofputil_uninit_group_mod() used to be harmless, but leads to
double free after commit e8dba7197, which will crash if any error in
group parsing happens.
Add a test to prevent this regression from happening again.
datapath: compat: tunnels: Log error during initialization.
At present OVS compat tunneling can fail due to conflict with
already loaded tunneling kernel module. In this case openvswitch
kernel module loading fails silently. Following patch give more
clues about what went wrong.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org> Acked-by: Jesse Gross <jesse@kernel.org>
Patch b0d38b2f17 unified flow mod reporting in ofproto for both
stand-alone flow mods and bundle flow mods, but left bundle-specific
reporting to the bundle removal code. This patch fixes this by
removing the bundle-specific reporting of flow mods.
Found by inspection.
Fixes: b0d38b2f17 ("ofproto: Report flow mods also from bundles.") Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Thu, 15 Sep 2016 03:39:03 +0000 (20:39 -0700)]
socket-util-unix: Avoid buffer read overrun in get_unix_name_len().
If the socket length does not include any of the bytes of the path, then
the code should not read even the first byte of the path.
Found by valgrind.
Reported-by: Joe Stringer <joe@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> Acked-by: Joe Stringer <joe@ovn.org>
After adding log messages to better understand IPAM-related code
in ovn northd, the IPAM tests began to fail occasionally. Adding
--wait=sb to commands triggering address allocation eliminated
these failures (there were no failures with 100 executions when
testing with this change).
Signed-off-by: Lance Richardson <lrichard@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Add support for OFPT_PACKET_OUT messages in bundles.
While ovs-ofctl already has a packet-out command, we did not have a
string parser for it, as the parsing was done directly from command
line arguments.
This patch adds the string parser for packet-out messages, adds
support for it into the 'ovs-ofctl packet-out' command, and adds a new
ofctl/packet-out ovs-appctl command that can be used when ovs-ofctl is
used as a flow monitor. The old 'ovs-ofctl packet-out syntax is
deprecated' and will be removed in a later OVS release.
The new packet-out parser is further supported with the ovs-ofctl
bundle command, which allows bundles to mix flow mods, group mods and
packet-out messages. Also the packet-outs in bundles are only
executed if the whole bundle is successful. A failing packet-out
translation may also make the whole bundle to fail.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Refactor handle_packet_out() to prepare for bundle support for packet
outs in a later patch.
Two new callbacks are introduced in ofproto-provider class:
->packet_xlate() and ->packet_execute(). ->packet_xlate() translates
the packet using the flow and actions provided by the caller, but
defers all OpenFlow-visible side-effects (stats, learn actions, actual
packet output, etc.) to be explicitly executed with the
->packet_execute() call.
Adds a new ofproto_rule_reduce_timeouts__() that must be called with
'ofproto_mutex' held. This is used in the next patch.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
ofproto-dpif-xlate: Allow translating without side-effects.
Extend 'may_learn' attribute to also control the treatment of
FIN_TIMEOUT action and asynchronous messages (packet ins,
continuations), so that when 'may_learn' is 'false' and
'resubmit_stats' is 'NULL', no OpenFlow-visible side effects are
generated by the translation.
Correspondingly, add support for one-time asynchronous messages to
xlate cache, so that all side-effects of the translation may be
executed at a later stage. This will be useful for bundle commits.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
ofproto: Use ofproto_flow_mod for learn execution from xlate cache.
Use ofproto_flow_mod with a reference to an existing or new rule
instead of ofputil_flow_mod for learn action execution from xlate
cache
Typically we would find that when a learn xlate cache entry is
created, a preceding upcall has already created the learned flow. In
this case the xlate cache entry takes a reference to that flow and
keeps refreshing it without needing to perform any flow table lookups.
Otherwise the creation of the xlate cache entry creates the new rule,
which is then subsequently added to the classifier. In both cases
this is both faster and shrinks the memory cost of each learn cache
entry from ~3.5kb to about 0.3kb.
If the learned rule does not yet exist, it is created and attached to
the ofproto_flow_mod, from which it is then added. If the referred
rule happens to expire, or is modified in any way and is thus removed
from the classifier tables, we create a new rule using the old rule as
a template, so that we can avoid storing the ofputil_flow_mod in all
cases.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
ofproto-dpif-xlate: Add xlate cache type XC_TABLE.
Xlate cache entry type XC_TABLE is required for the table stats
(number of misses and matches) to be correctly attributed.
It appears that table stats have been off ever since xlate cache was
introduced. This was now revealed by a PACKET_OUT unit test case in a
later patch that checks for table stats explicitly.
Later patches will need to create xlate cache entries from different
modules. This patch refactors the xlate cache code in preparation
without any functional changes, so that the changes are clearly
visible in the following patches.
The definition of XC_ENTRY_FOR_EACH() iterator macro is changed so
that it now does not take the xlate cache pointer to unify the usage
accross all call sites.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Make mac table update functions part of the mac-learning module, which
also helps in figuring what is the minimal set of struct flow fields
needed for the update. Use this to change the xlate cache entry for
XC_NORMAL to not take a copy of the struct flow, but only save the
in_port, dl_src, and some auxiliary fields. This reduces the memory
burden of XC_NORMAL by roughly 0.5kb.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Zongkai LI [Fri, 9 Sep 2016 06:39:17 +0000 (06:39 +0000)]
ovn-northd: add dhcpv6 stateless option support
This patch adds DHCPv6 stateless option support, to allow ovn native dhcpv6
work in stateless mode.
User can add new option dhcpv6_stateless with string value true in
DHCP_Options.options column, to let ovn dhcpv6 only reply other configurations
for DHCPv6 request messages come from VM/VIF ports, and let VM/VIF ports get
their IPv6 addresses configured via stateless way.
Signed-off-by: Zongkai LI <zealokii@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Add a few messages at INFO to help debug the vif lifecycle.
A logsearch on mac or ip helps debug what happened to the
vif and when. This helps easily correlate logs across CMS and ovn.
Logs appear like this:
2016-09-01T18:15:48Z|00014|binding|INFO|Claiming lport eee1a9af-7513-4540-9385-9e3972bfca05 for this chassis.
2016-09-01T18:15:48Z|00015|binding|INFO|Claiming fa:16:3e:01:c3:4a 10.0.0.7 fd93:b509:aa46:0:f816:3eff:fe01:c34a
2016-09-01T18:15:59Z|00016|pinctrl|INFO|DHCPOFFER fa:16:3e:01:c3:4a 10.0.0.7
2016-09-01T18:15:59Z|00017|pinctrl|INFO|DHCPACK fa:16:3e:01:c3:4a 10.0.0.7
2016-09-01T18:16:22Z|00018|binding|INFO|Releasing lport eee1a9af-7513-4540-9385-9e3972bfca05 from this chassis.
2016-09-01T18:16:22Z|00019|binding|INFO|Releasing fa:16:3e:01:c3:4a 10.0.0.7 fd93:b509:aa46:0:f816:3eff:fe01:c34a
Signed-off-by: Ramu Ramamurthy <ramu.ramamurthy@us.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ramu Ramamurthy [Tue, 30 Aug 2016 23:58:34 +0000 (23:58 +0000)]
ovn: add lsp-deletion and bcast-flow removal tests for localnet
Add 2 tests for scenarios around lsp-deletion and flow removal
which have escaped current unit tests.
This test depends on the following patch:
"ovn-controller: Back out incremental processing" and passes
after applying it, but fails currently on master.
1) In the following sequence of events,
createi&bind vif1, create&bind vif2, delete vif1
we find that the localnet patch port
got deleted, whereas it should exist because there is a
bound vif2.
2) The flow broadcasting to tunnels in table=32 must be deleted
when a localnet port gets bound, but we find that the flow remains
in table 32 causing broadcasts to both tunnels and localnet patch.
Signed-off-by: Ramu Ramamurthy <ramu.ramamurthy@us.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
lib: Create $(sysconfdir)/openvswitch upon install
In cases where dbdir and etcdir are not the same, there is a need
for creating etcdir (i.e. $(sysconfdir)/openvswitch) explicitly.
Note that there is no attempt being made here to make the etcdir
configurable as in "--with-dbdir".
Reported-at: http://openvswitch.org/pipermail/dev/2016-September/TBD.html Fixes: f973f2af2fd4 ("Make the location of the database separately configurable.") Signed-off-by: Flavio Fernandes <flavio@flaviof.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Windows: Extend support for binaries which allow detach
On Windows we require service_start to be called to parse and setup
requirements for '--detach' argument.
Affected binaries: ovn-trace, ovsdb-client, ovs-testcontroller.
Subsequent patches will be sent to adapt the tests with the new features.
Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> Signed-off-by: Gurucharan Shetty <guru@ovn.org>
connmgr: Make connmgr_wants_packet_in_on_miss() lock-free.
Make connmgr_wants_packet_in_on_miss() use an atomic int instead of a
list traversal taking the 'ofproto_mutex'. This allows
connmgr_wants_packet_in_on_miss() to be called also when
'ofproto_mutex' is already held, and makes it faster, too.
ofproto: Change rule's 'removed' member to a tri-state 'state'.
As a rule may not be re-inserted to ofproto data structures, it is
cleaner to have three states for the rule, rather than just two. This
will be useful for managing learned flows in later patches.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
ofproto: Add a fixed bundle idle timeout of 10 seconds.
Timing out idle bundles frees memory that would effectively be leaked
if a long standing OpenFlow connection would fail to commit or discard
a bundle.
OpenFlow specification mandates the timeout to be at least one second,
if the switch implements such a timeout. This patch makes the bundle
idle timeout to be 10 seconds.
We do not limit the number of messages in a bundle, so it does not
make sense to limit the number of bundles either, especially now that
idle bundles are timed out.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Add "bundle" keyword to ofp-print.at tests about bundle messages.
Add a missing ofp-print.at test for bundle group mods.
Remove "monitor" keyword from ofproto.at tests that do not use a monitor.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Fix the legal notice section in OVSEXT.SYS properties. Update the MSI to
include the properties mentioned in MSDN - 'Extension driver MSI packaging
requirements' section -
https://msdn.microsoft.com/windows/hardware/drivers/network/extension-driver-msi-packaging-requirements
Ben Pfaff [Sun, 11 Sep 2016 04:23:22 +0000 (21:23 -0700)]
replication: Be more careful about JSON parsing and simplify code.
The code here wasn't careful about parsing JSON received from the remote
OVSDB server. It assumed, for example, that a row that the remote server
implied was new was actually new, without looking to see whether there was
already a row with that UUID. This commit improves this validation. It
also rewrites code that translated updates locally into calls into the
query engine, via JSON, into simple lookups by UUID.
For me, this fixes a test failure in test 1866
(ovsdb-server/active-backup-role-switching), which caused the following
valgrind report:
==18725== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==18725== Access not within mapped region at address 0x0
==18725== at 0x43937E: ovsdb_datum_compare_3way (ovsdb-data.c:1626)
==18725== by 0x439344: ovsdb_datum_equals (ovsdb-data.c:1616)
==18725== by 0x4166CC: update_monitor_row_data (monitor.c:310)
==18725== by 0x414A90: ovsdb_monitor_changes_update (monitor.c:1255)
==18725== by 0x417009: ovsdb_monitor_change_cb (monitor.c:1339)
==18725== by 0x41DB52: ovsdb_txn_for_each_change (transaction.c:906)
==18725== by 0x416CC9: ovsdb_monitor_commit (monitor.c:1553)
==18725== by 0x41D993: ovsdb_txn_commit_ (transaction.c:868)
==18725== by 0x41D6F5: ovsdb_txn_commit (transaction.c:893)
==18725== by 0x418185: process_notification (replication.c:576)
==18725== by 0x417705: replication_run (replication.c:185)
==18725== by 0x408240: main_loop (ovsdb-server.c:198)
==18725== by 0x406432: main (ovsdb-server.c:429)
I don't know the exact cause of the problem, but this new implementation
leaves me more confident due to its simplicity.
Reported-by: Joe Stringer <joe@ovn.org>
Reported-at: http://openvswitch.org/pipermail/dev/2016-September/079315.html Fixes: 60e0cd041958 ("ovsdb: Replication usability improvements") Signed-off-by: Ben Pfaff <blp@ovn.org> Tested-by: Joe Stringer <joe@ovn.org> Acked-by: Andy Zhou <azhou@ovn.org>
Joe Stringer [Fri, 9 Sep 2016 20:48:53 +0000 (13:48 -0700)]
ovsdb: Fix replication memory leak.
Valgrind reports:
==18725== 32 bytes in 1 blocks are definitely lost in loss record 339 of 497
==18725== at 0x4C29BBE: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18725== by 0x450F1F: xmalloc (util.c:112)
==18725== by 0x41748E: replication_add_local_db (replication.c:137)
==18725== by 0x40803B: ovsdb_replication_init (ovsdb-server.c:146)
==18725== by 0x407C9E: ovsdb_server_connect_active_ovsdb_server
(ovsdb-server.c:1165)
==18725== by 0x450AB3: process_command (unixctl.c:313)
==18725== by 0x4500DC: run_connection (unixctl.c:347)
==18725== by 0x44FFB6: unixctl_server_run (unixctl.c:400)
==18725== by 0x4081AC: main_loop (ovsdb-server.c:182)
==18725== by 0x406432: main (ovsdb-server.c:429)
Fixes: 60e0cd041958 ("ovsdb: Replication usability improvements") Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Flavio Fernandes <flavio@flaviof.com> Acked-by: Ben Pfaff <blp@ovn.org>
A bitmap in 'struct group_table' is used to track all the allocated
group_ids. For every run of logical flows action parsing, we
add 'group_info' structure to a hmap called 'desired_groups'. The
group_id assigned to this group_info either comes from an already
installed 'existing groups' or a new reservation done in the bitmap.
In ofctrl_put(), if there is a backlog, we call ovn_group_table_clear().
This could unreserve a group_id that comes from an already existing group.
This could result in re-use of group_id in the future causing errors while
installing new groups.
This commit fixes the above scenario.
Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
When there are hundreds of nodes controlled by OVN, the workflow
to track and allocate unique tags across multiple hosts becomes
complicated. It is much easier to let ovn-northd do the allocation.
Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
ovn-northd: Add load-balancers to gateway routers.
Load-balancers in gateway routers lets us load-balance
north-south traffic.
This commit adds a new table called "DEFRAG" in the
logical router pipeline to defragment packets and to track them.
Once the packet is tracked, new connections get a group id as
an action. The group in turn chooses a DNAT action. Established
connections go through the DNAT table for a regular DNAT.
Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
ovn-controller: Datapath based conntrack zone for load-balancing.
Currently ct_lb() logical action is only added for a logical switch and
we use the conntrack zone allocated for the logical port. A future commit
will use ct_lb() for a logical router too. In that case, use the allocated
DNAT zone.
Rationale for not passing zone as an argument for ct_lb():
One way to look at it would be that a "zone" is an internal implementation
detail and should not be seen in a action of logical flow. But we can then
say that we could rename "zone" as "datapath" in the logical action. But,
then we would be limiting it to 2 anyway (datapath=lswitch or
datapath=lrouter) - in which case we are inferring it with the current patch.
Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Joe Stringer [Fri, 2 Sep 2016 00:01:55 +0000 (17:01 -0700)]
datapath: Use pre-routing hook for conntrack.
The upstream code uses NF_INET_PRE_ROUTING hook for the nf_conntrack_in()
call, which does deeper (eg l4proto) validation. It was previously
thought that using the NF_INET_ROUTING hook for this function on older
kernels would trigger kernel panics due to a dependency on the
unpopulated skb->dev, however during recent testing on a variety of
platforms (Centos7.[12], Ubuntu 1[46].04, Fedora23) using the latest
distribution kernels and the OVS kernel module testsuite, no such kernel
panics were observed. Therefore it appears to be safe to bring this in
line with upstream without any other workarounds.
Ryan Moats [Fri, 9 Sep 2016 12:36:47 +0000 (07:36 -0500)]
ovn-nbctl, tests: Clean up noisy memory leaks
When run with valgrind, ovn-nbctl.c and tests/test-ovn.c reveal
memory leaks of their own. This patch cleans these up so that
they don't create noise when looking for leaks in the OVN daemon
processes.
Signed-off-by: Ryan Moats <rmoats@us.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
rhel: add option to run kernel datapath test when building rpms
Add ability to execute kernel datapath tests when building rpms.
These tests are disabled by default, and can optionally be run
by providing "--with check_datapath_kernel" on the rpmbuild command
line. This is intended to facilitate automated testing, and
should not be used in production environments (it is generally not
recommended to run rpmbuild as root).
ovn-controller: Fix match crieria for dynamic mac binding flows
match struct is not initialized before adding flows for each entry in
mac_bindings table. The matches for IPv4 and IPv6 entries don't have
exactly the same form (IPv4 uses reg0, IPv6 uses xxreg0), so reusing
a match structure can cause problems.
Signed-off-by: Chandra Sekhar Vejendla <csvejend@us.ibm.com> Signed-off-by: Ryan Moats <rmoats@us.ibm.com> Co-authored-by: Ryan Moats <rmoats@us.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
The following leaks are due to missing ds_destroy in a few
places in build_acl.
5,850 bytes in 50 blocks are definitely lost in loss record 93 of 93
at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x449507: xrealloc (util.c:123)
by 0x42CC73: ds_reserve (dynamic-string.c:63)
by 0x42D08F: ds_put_format_valist (dynamic-string.c:161)
by 0x42D176: ds_put_format (dynamic-string.c:142)
by 0x40D380: build_acls (ovn-northd.c:2320)
by 0x40D380: build_lswitch_flows.constprop.36 (ovn-northd.c:2472)
by 0x4072D9: build_lflows (ovn-northd.c:3845)
by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971)
by 0x4072D9: main (ovn-northd.c:4375)
9,360 bytes in 72 blocks are definitely lost in loss record 93 of 93
at 0x4C29BFD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4C2BACB: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x449507: xrealloc (util.c:123)
by 0x42CC73: ds_reserve (dynamic-string.c:63)
by 0x42D08F: ds_put_format_valist (dynamic-string.c:161)
by 0x42D176: ds_put_format (dynamic-string.c:142)
by 0x40D505: build_acls (ovn-northd.c:2346)
by 0x40D505: build_lswitch_flows.constprop.36 (ovn-northd.c:2472)
by 0x4072D9: build_lflows (ovn-northd.c:3845)
by 0x4072D9: ovnnb_db_run (ovn-northd.c:3971)
by 0x4072D9: main (ovn-northd.c:4375)
Signed-off-by: Ramu Ramamurthy <ramu.ramamurthy@us.ibm.com> Acked-by: Ryan Moats <rmoats@us.ibm.com> Signed-off-by: Russell Bryant <russell@ovn.org>
Joe Stringer [Wed, 7 Sep 2016 21:07:41 +0000 (14:07 -0700)]
system-traffic: Add FTP NAT test without seqadj.
The existing FTP with NAT tests all perform NATing from an IP like
10.1.1.1 -> 10.1.1.240, which requires adjusting the length of FTP
control messages as they pass through the connection tracker.
Occasionally this is a source of kernel bugs, so it is useful to have a
regular FTP NAT test between IPs that do not change the message length
in FTP control messages (eg, 10.1.1.1 -> 10.1.1.9) to more clearly
identify failures in this area.
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
xlate: Clarify comment about mac learning table entry locking.
The rationale for locking mac learning table entires wrt. gratuitous
ARP packets and bond interfaces was too cryptic for me to understand.
After reading vswitchd/INTERNALS the issue is understandable, but we
can still improve the comment to prevent such confusion in future.
Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Andy Zhou [Tue, 23 Aug 2016 11:05:11 +0000 (04:05 -0700)]
ovsdb: Replication usability improvements
Based on feedbacks from initial HA manager integration, added the
'--active' command line option and appctl command
"ovsdb-server/sync-status. See man page updates for details.
Added the RPL_S_INIT state in the state machine. This state is
not strictly necessary for the replication state machine, but is
introduced to make sure the state is update immediately when
the state machine is reset, via replication_init(). Without it
ovsdb/sync-status may display "replicating" or crash, if the command
is issued between after replication_init() is called, but before
the state variable is updated from replication_run().
Added a test to simulate the integration of HA manager with OVSDB
server using replication.
Other documentation and API improvements.
Tested-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 2 Sep 2016 23:07:42 +0000 (16:07 -0700)]
tests: Fix recently broken sFlow tests.
A recent improvement to the promptness of sFlow reporting caused some of
the sFlow tests to fail (because the output was reported sooner). This
fixes up sequence numbers in the expected output to match the new behavior.
It also reduces the amount of (virtual) time that the test waits since it's
no longer necessary to wait as long.
ofproto: Honor mtu_request even for internal ports.
By default Open vSwitch tries to configure internal interfaces MTU to
match the bridge minimum, overriding any attempt by the user to
configure it through standard system tools, or the database.
While this works in many simple cases (there are probably many users
that rely on this) it may create problems for more advanced use cases
(like any overlay networks).
This commit allows the user to override the default behavior by
providing an explict MTU in the mtu_request column in the Interface
table.
This means that Open vSwitch will now treat differently database MTU
requests from standard system tools MTU requests (coming from `ip link`
or `ifconfig`), but this seems the best way to remain compatible with
old users while providing a more powerful interface.
Suggested-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> Acked-by: Ben Pfaff <blp@ovn.org> Tested-by: Joe Stringer <joe@ovn.org>
Revert "ofproto: Always set MTU for new internal ports."
This reverts commit 47bf118665a3d0f3c153d1fe80e9af02ac9a4e9c.
While the commit tries to make it more consistent, it breaks some system
tests. The assumptions made on the tests are probably made by many
users, so it's better to revert it.
Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> Acked-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 2 Sep 2016 20:26:50 +0000 (13:26 -0700)]
learn: Fix iteration over learning specs.
struct ofpact_learn_spec is variable-length. The 'n_specs' member of
struct ofpact_learn counted the number of specs, but the iteration loops
over struct ofpact_learn_spec only iterated as far as the *minimum* length
of 'n_specs' specs.
This fixes the problem, which exhibited as consistent failures for test 431
(learning action - TCPv6 port learning), seemingly only on i386 since it
shows up for my personal development machine but appears to not happen for
anyone else.
Fixes: dfe191d5faa6 ("ofp-actions: Waste less memory in learn actions.") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
This patch changes the order of the steps that are followed
every second in the sFlow agent. By moving the receiver_tick()
step to the end, we ensure that any counters that were polled
during the poller_tick() step are flushed immediately to the
sFlow collector. This eliminates what was a variable time-delay
between counters being polled and being flushed.
The variable time-delay that this eliminates could be up to
a second because counters lingering in the output buffer could be
flushed at any time by the arrival of random packet-samples.
Since the sFlow standard does not require that a poll-timestamp be sent
along with the counters the collector must use his receive-time as the
timestamp, so that extra second of variable delay was "stretching or
shrinking" the time between successive counter readings. This
affected any counter-rate calculation that was based only on the delta
between sucessive samples. The effect was small with a polling
interval of 60 seconds: just +/- 2%. But the effect grew larger
when faster polling was configured. For example, if the counters
were pushed every 5 seconds then the instantaneous rate
calculations could wander by +/- 20%. For a thorough analysis
of this problem, see Rick Jones' paper:
"High Frequency sFlow v5 Counter Sampling"
ftp://ftp.netperf.org/papers/high_freq_sflow/hf_sflow_counters.pdf
So this patch makes it possible to obtain usable results even
when high-frequency polling is configured.
Signed-off-by: Neil McKee <neil.mckee@inmon.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
ovsdb-idlc: Fix logic error in IDL parse function.
This was found due to a build error when adding an ovsschema column
with
"type": {"key": "string", "value": "integer"}
with no min or max, only a single instance.
I am rather unfamiliar with IDL, so no tests have been added yet.
I could use some pointers, or someone familiar with IDL tests could
take over.
Signed-off-by: Mickey Spiegel <mickeys.dev@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
learn: Avoid nested zero-sized arrays to fix build with MSVC.
Avoid using nested zero-sized arrays to allow compilation with MSVC.
Also, make sure the immediate data is accessed only if it exists, and
that the size is always calculated from struct learn_spec field
'n_bits'.
Fixes: dfe191d5faa6 ("ofp-actions: Waste less memory in learn actions.") Reported-by: Alin Serdean <aserdean@cloudbasesolutions.com> Signed-off-by: Jarno Rajahalme <jarno@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Joe Stringer [Wed, 31 Aug 2016 18:06:05 +0000 (11:06 -0700)]
upcall: Replace ukeys for deleted flows.
If a revalidator dumps/revalidates a flow during the 'dump' phase,
resulting in the deletion of the flow, then the ukey state moves into
UKEY_EVICTED, and the ukey is kept around until the 'sweep' phase. The
ukey is kept around to ensure that cases like duplicated dumps from the
datapaths do not result in multiple attribution of the same stats.
However, if an upcall for this flow comes for a handler between the
revalidator 'dump' and 'sweep' phases, the handler will lookup the ukey
and find that the ukey exists, then skip installing a new flow entirely.
As a result, for this period all traffic for the flow is slowpathed.
If there is a lot of traffic hitting this flow, then it will all be
handled in userspace until the 'sweep' phase. Eventually the
revalidators will reach the sweep phase and delete the ukey, and
subsequently the handlers should install a new flow.
To reduce the slowpathing of this traffic during flow table transitions,
allow the handler to identify this case during miss upcall handling and
replace the existing ukey with a new ukey. The handler will then be able
to install a flow for this traffic, allowing the traffic flow to return
to the fastpath.
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
Joe Stringer [Wed, 31 Aug 2016 18:06:04 +0000 (11:06 -0700)]
upcall: Track ukey states.
Ukeys have a defined lifetime that starts from being created, inserted
into the umaps, having the corresponding flow installed, then the flow
deleted, the ukey removed from the umap, rcu-deferral of its deletion,
and finally freedom.
However, until now it's all been represented behind a simple boolean
"flow_exists" with a bunch of implicit logic sprinkled around the
accessors. This patch attempts to make the ukey lifetime a bit clearer
by outlining the correct transitions and asserting that their lifetime
proceeds as expected.
This should improve the readability of the current code, and also make
the following patch easier to reason about.
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
Joe Stringer [Wed, 31 Aug 2016 18:06:03 +0000 (11:06 -0700)]
upcall: Only init flow_put if ukey is installed.
Currently when processing a batch of upcalls, all datapath operations
are first initialized, then later the corresponding ukeys are installed.
If the ukey_install fails at this later point, then the code needs to
backtrack a bit to delete the ukey and skip using the initialized
datapath op.
It's a little simpler to only initialize the datapath operation if the
ukey could actually be installed. The locks are held longer, but these
locks aren't heavily contended and the extended holding of the lock will
be removed in a subsequent patch anyway.
Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Jarno Rajahalme <jarno@ovn.org>
Ben Pfaff [Thu, 1 Sep 2016 17:02:53 +0000 (10:02 -0700)]
ovn-controller: Fix memory leak in recv_S_TLV_TABLE_REQUESTED().
Nothing freed 'reply'. This fixes the problem.
Most of this patch is moving coding around. The essential change is that
breaking the code that works with 'reply' out into a separate function
makes it possible to catch all paths out of the function so that 'reply'
can be freed in one place.
Reported-by: Ryan Moats <rmoats@us.ibm.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Ryan Moats <rmoats@us.ibm.com> Acked-by: Flavio Fernandes <flavio@flaviof.com>
Ben Pfaff [Wed, 31 Aug 2016 21:25:41 +0000 (14:25 -0700)]
ovn-controller: Drop incremental processing from encapsulation code.
This commit reverts encaps.c to its content just before commit 1d45d5a9666d
(ovn-controller: Change encaps_run to work incrementally.). I then
reintroduced the UDP checksum support originallly added in commit 36283d7884f3 (ovn-controller: Use UDP checksums when creating Geneve
tunnels.) I also read the other commits following the incremental
processing commit to verify that this change didn't lose any bug fixes.
This commit takes advantage of the "addvalue" and "delvalue" functions
now available in the IDL to simplify some code.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Russell Bryant <russell@ovn.org>
Ben Pfaff [Wed, 31 Aug 2016 21:25:40 +0000 (14:25 -0700)]
ovsdb-idlc: Make set and map update operations take const arguments.
In a call like "ovsrec_bridge_update_ports_delvalue(bridge, port)", there's
no reason for the port argument to be nonconst, because the call doesn't
do anything to the port at all--it only searches the list of ports in the
bridge for that particular port and, if it finds it, removes it.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Russell Bryant <russell@ovn.org>
Andy Zhou [Tue, 23 Aug 2016 20:57:37 +0000 (13:57 -0700)]
ovsdb: Reimplement replication. Using a state machine.
Current replication uses blocking transactions, which are error prone
in practice, especially in handling RPC connection flapping to the
active server.
Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Andy Zhou [Wed, 17 Aug 2016 20:56:02 +0000 (13:56 -0700)]
ovsdb: Add request_ids
When starting, the replication logic may issue multiple requests at
a time, for example, one monitor request for each databases. The
request_ids keeps track of all outsanding request IDs that are used
for matching reply message with. It also provides the 'db' context
for the reply.
Future patches will make use of this facility.
Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>