Ben Pfaff [Fri, 27 Sep 2019 15:44:16 +0000 (08:44 -0700)]
ovsdb-cluster: Use ovs-vsctl instead of ovn-nbctl and ovn-sbctl.
This removes a dependency on OVN from the tests.
This adds some options to ovs-vsctl to allow it to be used for testing
the clustering feature. The new options are undocumented because
they're really just useful for testing clustering.
Acked-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
This patch addresses a conntrack cache issue with timeout policy.
Currently, we do not check if the timeout extension is set properly in the
cached conntrack entry. Thus, after packet recirculate from conntrack
action, the timeout policy is not applied properly. This patch fixes the
aforementioned issue.
Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct action") Reported-by: kbuild test robot <lkp@intel.com> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
William Tu [Fri, 27 Sep 2019 17:22:55 +0000 (10:22 -0700)]
fatal-signal: Catch SIGSEGV and print backtrace.
The patch catches the SIGSEGV signal and prints the backtrace
using libunwind at the monitor daemon. This makes debugging easier
when there is no debug symbol package or gdb installed on production
systems.
The patch works when the ovs-vswitchd compiles even without debug symbol
(no -g option), because the object files still have function symbols.
For example:
|daemon_unix(monitor)|WARN|SIGSEGV detected, backtrace:
|daemon_unix(monitor)|WARN|0x0000000000482752 <fatal_signal_handler+0x52>
|daemon_unix(monitor)|WARN|0x00007fb4900734b0 <killpg+0x40>
|daemon_unix(monitor)|WARN|0x00007fb49013974d <__poll+0x2d>
|daemon_unix(monitor)|WARN|0x000000000052b348 <time_poll+0x108>
|daemon_unix(monitor)|WARN|0x00000000005153ec <poll_block+0x8c>
|daemon_unix(monitor)|WARN|0x000000000058630a <clean_thread_main+0x1aa>
|daemon_unix(monitor)|WARN|0x00000000004ffd1d <ovsthread_wrapper+0x7d>
|daemon_unix(monitor)|WARN|0x00007fb490b3b6ba <start_thread+0xca>
|daemon_unix(monitor)|WARN|0x00007fb49014541d <clone+0x6d>
|daemon_unix(monitor)|ERR|1 crashes: pid 122849 died, killed \
(Segmentation fault), core dumped, restarting
However, if the object files' symbols are stripped, then we can only
get init function plus offset value. This is still useful when trying
to see if two bugs have the same root cause, Example:
|daemon_unix(monitor)|WARN|SIGSEGV detected, backtrace:
|daemon_unix(monitor)|WARN|0x0000000000482752 <_init+0x7d68a>
|daemon_unix(monitor)|WARN|0x00007f5f7c8cf4b0 <killpg+0x40>
|daemon_unix(monitor)|WARN|0x00007f5f7c99574d <__poll+0x2d>
|daemon_unix(monitor)|WARN|0x000000000052b348 <_init+0x126280>
|daemon_unix(monitor)|WARN|0x00000000005153ec <_init+0x110324>
|daemon_unix(monitor)|WARN|0x0000000000407439 <_init+0x2371>
|daemon_unix(monitor)|WARN|0x00007f5f7c8ba830 <__libc_start_main+0xf0>
|daemon_unix(monitor)|WARN|0x0000000000408329 <_init+0x3261>
|daemon_unix(monitor)|ERR|1 crashes: pid 106155 died, killed \
(Segmentation fault), core dumped, restarting
Most C library functions are not async-signal-safe, meaning that
it is not safe to call them from a signal handler, for example
printf() or fflush(). To be async-signal-safe, the handler only
collects the stack info using libunwind, which is signal-safe, and
issues 'write' to the pipe, where the monitor thread reads and
prints to ovs-vswitchd.log.
Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/590503433 Signed-off-by: William Tu <u9012063@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 20 Sep 2019 15:30:42 +0000 (08:30 -0700)]
Require Python 3 and remove support for Python 2.
Python 2 reaches end-of-life on January 1, 2020, which is only
a few months away. This means that OVS needs to stop depending
on in the next release that should occur roughly that same time.
Therefore, this commit removes all support for Python 2. It
also makes Python 3 a mandatory build dependency.
Some of the interesting consequences:
- HAVE_PYTHON, HAVE_PYTHON2, and HAVE_PYTHON3 conditionals have
been removed, since we now know that Python3 is available.
- $PYTHON and $PYTHON2 are removed, and $PYTHON3 is always
available.
- Many tests for Python 2 support have been removed, and the ones
that depended on Python 3 now run unconditionally. This allowed
several macros in the testsuite to be removed, making the code
clearer. This does make some of the changes to the testsuite
files large due to indentation level changes.
- #! lines for Python now use /usr/bin/python3 instead of
/usr/bin/python.
Yi-Hung Wei [Wed, 28 Aug 2019 22:14:29 +0000 (15:14 -0700)]
ofproto-dpif-xlate: Translate timeout policy in ct action
This patch derives the timeout policy based on ct zone from the
internal data structure that we maintain on dpif layer.
It also adds a system traffic test to verify the zone-based conntrack
timeout feature. The test uses ovs-vsctl commands to configure
the customized ICMP and UDP timeout on zone 5 to a shorter period.
It then injects ICMP and UDP traffic to conntrack, and checks if the
corresponding conntrack entry expires after the predefined timeout.
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
ofproto-dpif: Checks if datapath supports OVS_CT_ATTR_TIMEOUT
This patch checks whether datapath supports OVS_CT_ATTR_TIMEOUT. With this
check, ofproto-dpif-xlate can use this information to decide whether to
translate the ct timeout policy.
Yi-Hung Wei [Wed, 28 Aug 2019 22:14:28 +0000 (15:14 -0700)]
datapath: Add support for conntrack timeout policy
This patch adds support for specifying a timeout policy for a
connection in connection tracking system in kernel datapath.
The timeout policy will be attached to a connection when the
connection is committed to conntrack.
This patch introduces a new odp field OVS_CT_ATTR_TIMEOUT in the
ct action that specifies the timeout policy in the datapath.
In the following patch, during the upcall process, the vswitchd will use
the ct_zone to look up the corresponding timeout policy and fill
OVS_CT_ATTR_TIMEOUT if it is available.
The datapath code is from the following two net-next upstream commits.
Add support for fine-grain timeout support to conntrack action.
The new OVS_CT_ATTR_TIMEOUT attribute of the conntrack action
specifies a timeout to be associated with this connection.
If no timeout is specified, it acts as is, that is the default
timeout for the connection will be automatically applied.
Example usage:
$ nfct timeout add timeout_1 inet tcp syn_sent 100 established 200
$ ovs-ofctl add-flow br0 in_port=1,ip,tcp,action=ct(commit,timeout=timeout_1)
CC: Pravin Shelar <pshelar@ovn.org> CC: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net>
commit 6d670497e01803b486aa72cc1a718401ab986896
Author: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue Apr 2 09:53:14 2019 +0300
openvswitch: use after free in __ovs_ct_free_action()
We free "ct_info->ct" and then use it on the next line when we pass it
to nf_ct_destroy_timeout(). This patch swaps the order to avoid the use
after free.
Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct action") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
Yi-Hung Wei [Wed, 28 Aug 2019 22:14:27 +0000 (15:14 -0700)]
datapath: compat: Backport nf_conntrack_timeout support
This patch brings in nf_ct_timeout_put() and nf_ct_set_timeout()
when it is not available in the kernel.
Three symbols are created in acinclude.m4.
* HAVE_NF_CT_SET_TIMEOUT is used to determine if upstream net-next commit 717700d183d65 ("netfilter: Export nf_ct_{set,destroy}_timeout()") is
availabe. If it is defined, the kernel should have all the
nf_conntrack_timeout support that OVS needs.
* HAVE_NF_CT_TIMEOUT is used to check if upstream net-next commit 6c1fd7dc489d9 ("netfilter: cttimeout: decouple timeout policy from
nfnetlink_cttimeout object") is there. If it is not defined, we
will use the old ctnl_timeout interface rather than the nf_ct_timeout
interface that is introduced in this commit.
* HAVE_NF_CT_TIMEOUT_FIND_GET_HOOK_NET is used to check if upstream
commit 19576c9478682 ("netfilter: cttimeout: add netns support") is
there, so that we pass different arguement based on whether the kernel
has netns support.
Yi-Hung Wei [Wed, 28 Aug 2019 22:14:26 +0000 (15:14 -0700)]
ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy tables
This patch consumes the CT_Zone and CT_Timeout_Policy tables, maintains
the zone-based configuration in the vswitchd. Whenever there is a
database change, vswitchd will read the datapath, CT_Zone, and
CT_Timeout_Policy tables from ovsdb, builds an internal snapshot of the
database configuration in bridge.c, and pushes down the change into
ofproto and dpif layer.
If a new zone-based timeout policy is added, it updates the zone to
timeout policy mapping in the per datapath type datapath structure in
dpif-backer, and pushes down the timeout policy into the datapath via
dpif interface.
If a timeout policy is no longer used, for kernel datapath, vswitchd
may not be able to remove it from datapath immediately since
datapath flows can still reference the to-be-deleted timeout policies.
Thus, we keep an timeout policy kill list, that vswitchd will go
back to the list periodically and try to kill the unused timeout policies.
Yi-Hung Wei [Wed, 28 Aug 2019 22:14:24 +0000 (15:14 -0700)]
ct-dpif, dpif-netlink: Add conntrack timeout policy support
This patch first defines the dpif interface for a datapath to support
adding, deleting, getting and dumping conntrack timeout policy.
The timeout policy is identified by a 4 bytes unsigned integer in
datapath, and it currently support timeout for TCP, UDP, and ICMP
protocols.
Moreover, this patch provides the implementation for Linux kernel
datapath in dpif-netlink.
In Linux kernel, the timeout policy is maintained per L3/L4 protocol,
and it is identified by 32 bytes null terminated string. On the other
hand, in vswitchd, the timeout policy is a generic one that consists of
all the supported L4 protocols. Therefore, one of the main task in
dpif-netlink is to break down the generic timeout policy into 6
sub policies (ipv4 tcp, udp, icmp, and ipv6 tcp, udp, icmp),
and push down the configuration using the netlink API in
netlink-conntrack.c.
This patch also adds missing symbols in the windows datapath so
that the build on windows can pass.
David Marchand [Fri, 6 Sep 2019 11:26:02 +0000 (13:26 +0200)]
dpdk: Remove unneeded log message copy.
No need to duplicate and null-terminate the passed buffer.
We can directly give it to the vlog subsystem using a dynamic precision
in the format string.
Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
flow: fix incorrect padding length checking in ipv6_sanity_check
The padding length is (packet size - ipv6 header length - ipv6 plen). This
patch fixes incorrect padding size checking in ipv6_sanity_check.
Acked-by: William Tu <u9012063@gmail.com> Reviewed-by: Gavin Hu <Gavin.Hu@arm.com> Signed-off-by: Yanqin Wei <Yanqin.Wei@arm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
show "rx_missed_errors" counter in interface statisics
Hi all:
Currently OVS maintains several Statistics counters per interface. "rx_missed_errors" counter is amount them and collects pkts not received due to local resource constaints. Many ovs netdevs support collecting this counter, such as netdev-linux, netdev-dpdk, netdev-bsd and so on. But as far as I know, this counter can't be read by command "ovs-vsctl list interface <int-name>|grep statistics". I have found the root cause(may be I was wrong) is in task "iface_refresh_stats", the "rx_missed_errors" is not in the macro IFACE_STATS. So even if this counter is updated by netdev, it woundn't be read by users.
This simple patch tries to solve this problem, many thanks for your kindly reminder.
Signed-off-by: Liu Chang <liuchang@cmss.chinamobile.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Paul Chaignon [Sun, 1 Sep 2019 13:10:05 +0000 (15:10 +0200)]
treewide: Use packet batch APIs
This patch replaces direct accesses to dp_packet_batch and dp_packet
internal components by the appropriate API calls. It extends commit 1270b6e52 (treewide: Wider use of packet batch APIs).
This patch was generated using the following semantic patch (cf.
http://coccinelle.lip6.fr).
Yifeng Sun [Thu, 5 Sep 2019 17:40:28 +0000 (10:40 -0700)]
userspace: Enable non-bridge port as tunnel endpoint.
For userspace datapath, currently only the bridge itself, the LOCAL port,
can be the tunnel endpoint to encap/decap tunnel packets. This patch
enables non-bridge port as tunnel endpoint. One use case is for users to
create a bridge and a vtep port as tap, and configure underlay IP at vtep
port as the tunnel endpoint.
This patch causes failure for test "ptap - L3 over patch port". This is
because this test is already using non-bridge port gre1 as tunnel endpoint.
In this test, a flow is added to redirect tunnel packets to gre1 port,
as shown below:
ovs-ofctl add-flow br1 in_port=p1,actions=output=gre1
It later generates a datapath flow which matches an extra eth field:
- recirc_id(0),...,eth_type(0x0800),...
+ recirc_id(0),...,eth(dst=1e:2c:e9:2a:66:9e),eth_type(0x0800),...
With this patch, this flow need only a NORMAL action.
Signed-off-by: William Tu <u9012063@gmail.com> Co-authored-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
vswitchd: update bond-rebalance-interval maxInteger in vswtich.xml
According to the documentation in the vswtich.xml the maximum
configurable bond-rebalance-interval is 10000ms. However, this is not
enforced anywhere in the code and people are using larger values in
the field.
To avoid confusion this change changes the maximum value the 2147483647 which will fit in 32bit integer. This will allow a value of
~25 days, which should be enough to cover everybody's needs. Note that
a value of 0 disables the automatic rebalancing anyway.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
conntrack: Add option to disable TCP sequence checking.
This may be needed in some special cases, such as to support some hardware
offload implementations. Note that disabling TCP sequence number
verification is not an optimization in itself, but supporting some
hardware offload implementations may offer better performance. TCP
sequence number verification is enabled by default. This option is only
available for the userspace datapath. Access to this option is presently
provided via 'dpctl' commands as the need for this option is quite node
specific, by virtue of which nics are in use on a given node. A test is
added to verify this option.
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-May/359188.html Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Exclude ovn-nb/ovn-sb man and OVN schema files during compilation.
The commit [1] removed OVN, but had to leave out some OVN bits
for the ovsdb-server raft testing. But "make install" is installing
ovn-nb/ovn-sb man entries and OVN schema files.
This patch excludes these.
"make install" is also installing ovn-nbctl/ovn-sbctl and this still needs to
be addressed.
The pid variable is being shadowed by the list comprehension in the
os.execvp() call. This can generate flakes / warnings in some environments
so fix it.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Mon, 26 Aug 2019 16:05:44 +0000 (09:05 -0700)]
conntrack: Optimize recirculations.
Cache the 'conn' context and use it when it is valid. The cached 'conn'
context will get reset if it is not expected to be valid; the cost to do
this is negligible. Besides being most optimal, this also handles corner
cases, such as decapsulation leading to the same tuple, as in tunnel VPN
cases. A negative test is added to check the resetting of the cached
'conn'.
Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
The datatype 'pad' in the function 'reverse_nat_packet()' was incorrectly
declared as 'char' instead of 'uint8_t'. This can affect reverse natting
of icmpX packets with padding > 127 bytes. At the same time, add some
comments regarding 'extract_l3_ipvX' usage in this function. Found by
inspection.
Fixes: edd1bef468c0 ("dpdk: Add more ICMP Related NAT support.") Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Python3 complains if a dict key is changed during the
iteration.
Use list() to create a copy of it.
Traceback (most recent call last):
File "./ovsdb/ovsdb-idlc.in", line 1581, in <module>
func(*args[1:])
File "./ovsdb/ovsdb-idlc.in", line 185, in printCIDLHeader
replace_cplusplus_keyword(schema)
File "./ovsdb/ovsdb-idlc.in", line 179, in replace_cplusplus_keyword
for columnName in table.columns:
RuntimeError: dictionary keys changed during iteration
Signed-off-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Valgrind reported that 'pkt->md.ct_orig_tuple.ipv4.ipv4_proto' is
uninitialized in 'check_orig_tuple()', if 'ct_state' is zero. Although
this is true, the check is superceded, as even if it succeeds the check
for natted packets based on 'ct_state' is an ORed condition and is intended
to catch this case.
The check is '!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT))' which
filters out all packets excepted natted ones. Move this check up to
prevent the Valgrind complaint, which also helps performance and also remove
recenlty added redundant check adding extra cycles.
Fixes: f44733c527da ("conntrack: Validate accessing of conntrack data in pkt_metadata.") CC: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Dumitru Ceara [Fri, 2 Aug 2019 08:29:52 +0000 (10:29 +0200)]
vswitchd: Make packet-in controller queue size configurable
The ofconn packet-in queue for packets that can't be immediately sent
on the rconn connection was limited to 100 packets (hardcoded value).
While increasing this limit is usually not recommended as it might
create buffer bloat and increase latency, in scaled scenarios it is
useful if the administrator (or CMS) can adjust the queue size.
One such situation was noticed while performing scale testing of the
OVN IGMP functionality: triggering ~200 simultaneous IGMP reports
was causing tail drops on the packet-in queue towards ovn-controller.
This commit adds the possibility to configure the queue size for:
- management controller (br-int.mgmt): through the
other_config:controller-queue-size column of the Bridge table. This
value is limited to 512 as large queues definitely affect latency. If
not present the default value of 100 is used. This is done in order to
maintain the same default behavior as before the commit.
- other controllers: through the controller_queue_size column of the
Controller table. This value is also limited to 512. If not present
the code uses the Bridge:other_config:controller-queue-size
configuration.
Acked-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Aliasgar Ginwala [Fri, 30 Aug 2019 15:28:34 +0000 (08:28 -0700)]
ovsdb-tool: Convert clustered db to standalone db.
Add support in ovsdb-tool for migrating clustered dbs to standalone dbs.
E.g. usage to migrate nb/sb db to standalone db from raft:
ovsdb-tool cluster-to-standalone ovnnb_db.db ovnnb_db_cluster.db
Acked-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Aliasgar Ginwala <aginwala@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
It's possible that a port added to the system with certain kinds
of invalid parameters will cause the 'could not add' log to be
triggered. When this happens, the vswitch run loop can continually
re-attempt adding the port. While the parameters remain invalid
the vswitch run loop will re-trigger the warning, flooding the
syslog.
This patch adds a simple rate limit to the log.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
stream_ssl: fix important memory leak in ssl_connect() function
While checking valgrind reports after running "make check-valgrind" I have noticed
reports for several tests similar to the following:
....
==5345== Memcheck, a memory error detector
==5345== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==5345== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==5345== Command: ovsdb-client --private-key=/home/damijan.skvarc/doma/ovs/tests/testpki-privkey.pem --certificate=/home/damijan.skvarc/doma/ovs/tests/testpki-cert.pem --ca-cert=/home/damijan.skvarc/doma/ovs/tests/testpki-cacert.pem transact ssl:127.0.0.1:40111 \ \ \ ["ordinals",
==5345== \ \ \ \ \ \ {"op":\ "update",
==5345== \ \ \ \ \ \ \ "table":\ "ordinals",
==5345== \ \ \ \ \ \ \ "where":\ [["number",\ "==",\ 1]],
==5345== \ \ \ \ \ \ \ "row":\ {"number":\ 2,\ "name":\ "old\ two"}},
==5345== \ \ \ \ \ \ {"op":\ "update",
==5345== \ \ \ \ \ \ \ "table":\ "ordinals",
==5345== \ \ \ \ \ \ \ "where":\ [["name",\ "==",\ "two"]],
==5345== \ \ \ \ \ \ \ "row":\ {"number":\ 1,\ "name":\ "old\ one"}}]
==5345== Parent PID: 5344
==5345==
==5345==
==5345== HEAP SUMMARY:
==5345== in use at exit: 116,551 bytes in 3,341 blocks
==5345== total heap usage: 5,134 allocs, 1,793 frees, 412,290 bytes allocated
==5345==
==5345== 6,221 (184 direct, 6,037 indirect) bytes in 1 blocks are definitely lost in loss record 498 of 500
==5345== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5345== by 0x5105E77: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5345== by 0x51E1D23: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5345== by 0x51E4861: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5345== by 0x51E5414: ASN1_item_ex_d2i (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5345== by 0x51E546A: ASN1_item_d2i (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5345== by 0x4E56B27: ??? (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==5345== by 0x4E5BA11: ??? (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==5345== by 0x4E65145: ??? (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==5345== by 0x4522DF: ssl_connect (stream-ssl.c:530)
==5345== by 0x443D38: scs_connecting (stream.c:315)
==5345== by 0x443D38: stream_connect (stream.c:338)
==5345== by 0x443FA1: stream_open_block (stream.c:266)
==5345== by 0x40AB79: open_jsonrpc (ovsdb-client.c:507)
==5345== by 0x40AB79: open_rpc (ovsdb-client.c:143)
==5345== by 0x40B06B: do_transact__ (ovsdb-client.c:871)
==5345== by 0x40B245: do_transact (ovsdb-client.c:893)
==5345== by 0x405F76: main (ovsdb-client.c:282)
==5345==
==5345== LEAK SUMMARY:
==5345== definitely lost: 184 bytes in 1 blocks
==5345== indirectly lost: 6,037 bytes in 117 blocks
==5345== possibly lost: 0 bytes in 0 blocks
==5345== still reachable: 110,330 bytes in 3,223 blocks
==5345== suppressed: 0 bytes in 0 blocks
==5345== Reachable blocks (those to which a pointer was found) are not shown.
==5345== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==5345==
==5345== For counts of detected and suppressed errors, rerun with: -v
==5345== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
....
This report was extracted from "index uniqueness checking" test and complains about
leaking memory in ovsdb-client application. The problem is not huge, since ovsdb-client
is CLI tool which is constantly reinvoked/restarted, thus leaked memory is not accumulated.
More problematic issue is that for the same test valgrind reports the similar problem also for
ovsdb-server:
....
==5290== Memcheck, a memory error detector
==5290== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==5290== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==5290== Command: ovsdb-server --log-file --detach --no-chdir --pidfile --private-key=/home/damijan.skvarc/doma/ovs/tests/testpki-privkey2.pem --certificate=/home/damijan.skvarc/doma/ovs/tests/testpki-cert2.pem --ca-cert=/home/damijan.skvarc/doma/ovs/tests/testpki-cacert.pem --remote=pssl:0:127.0.0.1 db
==5290== Parent PID: 5289
==5290==
==5292== Warning: noted but unhandled ioctl 0x2403 with no size/direction hints.
==5292== This could cause spurious value errors to appear.
==5292== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==5292== Warning: noted but unhandled ioctl 0x2400 with no size/direction hints.
==5292== This could cause spurious value errors to appear.
==5292== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==5290==
==5290== HEAP SUMMARY:
==5290== in use at exit: 2,066 bytes in 48 blocks
==5290== total heap usage: 87 allocs, 39 frees, 14,152 bytes allocated
==5290==
==5290== LEAK SUMMARY:
==5290== definitely lost: 0 bytes in 0 blocks
==5290== indirectly lost: 0 bytes in 0 blocks
==5290== possibly lost: 0 bytes in 0 blocks
==5290== still reachable: 2,066 bytes in 48 blocks
==5290== suppressed: 0 bytes in 0 blocks
==5290== Reachable blocks (those to which a pointer was found) are not shown.
==5290== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==5290==
==5290== For counts of detected and suppressed errors, rerun with: -v
==5290== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 1 from 1)
==5292== Warning: noted but unhandled ioctl 0x2401 with no size/direction hints.
==5292== This could cause spurious value errors to appear.
==5292== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==5292==
==5292== HEAP SUMMARY:
==5292== in use at exit: 164,018 bytes in 4,252 blocks
==5292== total heap usage: 17,910 allocs, 13,658 frees, 1,907,468 bytes allocated
==5292==
==5292== 49,720 (1,472 direct, 48,248 indirect) bytes in 8 blocks are definitely lost in loss record 580 of 580
==5292== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5292== by 0x5105E77: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5292== by 0x51E1D23: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5292== by 0x51E4861: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5292== by 0x51E5414: ASN1_item_ex_d2i (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5292== by 0x51E546A: ASN1_item_d2i (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5292== by 0x4E53E00: ??? (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==5292== by 0x4E55727: ??? (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==5292== by 0x452C4B: ssl_connect (stream-ssl.c:530)
==5292== by 0x445B18: scs_connecting (stream.c:315)
==5292== by 0x445B18: stream_connect (stream.c:338)
==5292== by 0x445B91: stream_recv (stream.c:369)
==5292== by 0x432A9C: jsonrpc_recv.part.7 (jsonrpc.c:310)
==5292== by 0x433977: jsonrpc_recv (jsonrpc.c:1139)
==5292== by 0x433977: jsonrpc_session_recv (jsonrpc.c:1112)
==5292== by 0x40CCE3: ovsdb_jsonrpc_session_run (jsonrpc-server.c:553)
==5292== by 0x40CCE3: ovsdb_jsonrpc_session_run_all (jsonrpc-server.c:586)
==5292== by 0x40CCE3: ovsdb_jsonrpc_server_run (jsonrpc-server.c:401)
==5292== by 0x40682E: main_loop (ovsdb-server.c:209)
==5292== by 0x40682E: main (ovsdb-server.c:460)
==5292==
==5292== LEAK SUMMARY:
==5292== definitely lost: 1,472 bytes in 8 blocks
==5292== indirectly lost: 48,248 bytes in 936 blocks
==5292== possibly lost: 0 bytes in 0 blocks
==5292== still reachable: 114,298 bytes in 3,308 blocks
==5292== suppressed: 0 bytes in 0 blocks
==5292== Reachable blocks (those to which a pointer was found) are not shown.
==5292== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==5292==
==5292== For counts of detected and suppressed errors, rerun with: -v
==5292== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 1 from 1)
....
In this case ovsdb-server is running as daemon process (--detach option) and leaking memory is
accumulated whenever ovsdb-client is reconnected. Within observed test ovsdb-client CLI tool
connects 8 times to ovsdb-server. Leaked memory in ovsdb-client (for each invocation) is approx.
6K bytes, while leaked memory in ovsdb-server is aprox. 48Kbytes what is actually 8*6K. Thus per
each connection both ovsdb-client and ovsdb-server leak approx. 6K bytes.
I have done a small manual test to check if ovsdb-server is indeed accumulating leaked memory
by dumping ovsdb-server in a loop:
In console2 it was evidently seen ovsdb-server is constantly leaking memory. After a while
(i.e. after a certain number of reconnections) the OOM killer jumps out and kills ovsdb-server.
Very similar situation was already noticed and described in
https://github.com/openvswitch/ovs-issues/issues/168. There, the problem pops up while connecting
controller to ovs-vswitchd daemon.
Valgrind reports point to a problem in openssl library, however after studying openssl code for
a while I have found out the problem is actually in ovs. When connection through SSL channel is
taken place openssl library allocates memory for keeping track of certificate. Reference to this
memory works very similar as std::shared_ptr pointer in recent C++ dialects. i.e. when allocated
memory is referenced its reference counter is incremented and decremented after the memory is
derefered. When reference counter becomes zero allocated memory is automatically deallocated.
In openssl library environment certificate is retrieved by calling SSL_get_peer_certificate()
where its reference counter is incremented. After retrieved certificate is not used any more its
reference counter must be decremented by calling X509_free(). If not, allocated memory is never
freed despite the ssl connection is properly closed.
The problem was caused in stream-ssl.c in function ssl_connect(), which retrieves common peer name
by calling SSL_get_peer_certificate() function and without calling X509_free() function afterwards.
Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Mon, 16 Sep 2019 18:56:59 +0000 (11:56 -0700)]
Documentation: Work with sphinx-build for Python 3 also.
There's nothing in OVS specific to Sphinx for Python 2, but the
compile-time check only looked for a binary named "sphinx-build", which is
typically provided only for Python 2. With Python 3, the binary is
typically called "sphinx-build-3". With this commit, either name is
accepted.
Acked-by: Numan Siddique <nusididq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 11 Sep 2019 21:18:36 +0000 (14:18 -0700)]
conntrack: Validate accessing of conntrack data in pkt_metadata
Valgrind reported:
1305: ofproto-dpif - conntrack - ipv6
==26942== Conditional jump or move depends on uninitialised value(s)
==26942== at 0x587C00: check_orig_tuple (conntrack.c:1006)
==26942== by 0x587C00: process_one (conntrack.c:1141)
==26942== by 0x587C00: conntrack_execute (conntrack.c:1220)
==26942== by 0x47B00F: dp_execute_cb (dpif-netdev.c:7305)
==26942== by 0x4AF756: odp_execute_actions (odp-execute.c:794)
==26942== by 0x477532: dp_netdev_execute_actions (dpif-netdev.c:7349)
==26942== by 0x477532: handle_packet_upcall (dpif-netdev.c:6630)
==26942== by 0x477532: fast_path_processing (dpif-netdev.c:6726)
==26942== by 0x47933C: dp_netdev_input__ (dpif-netdev.c:6814)
==26942== by 0x479AB8: dp_netdev_input (dpif-netdev.c:6852)
==26942== by 0x479AB8: dp_netdev_process_rxq_port (dpif-netdev.c:4287)
==26942== by 0x47A6A9: dpif_netdev_run (dpif-netdev.c:5264)
==26942== by 0x4324E7: type_run (ofproto-dpif.c:342)
==26942== by 0x41C5FE: ofproto_type_run (ofproto.c:1734)
==26942== by 0x40BAAC: bridge_run__ (bridge.c:2965)
==26942== by 0x410CF3: bridge_run (bridge.c:3029)
==26942== by 0x407614: main (ovs-vswitchd.c:127)
==26942== Uninitialised value was created by a heap allocation
==26942== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26942== by 0x532574: xmalloc (util.c:138)
==26942== by 0x46CD62: dp_packet_new (dp-packet.c:153)
==26942== by 0x4A0431: eth_from_flow_str (netdev-dummy.c:1644)
==26942== by 0x4A0431: netdev_dummy_receive (netdev-dummy.c:1783)
==26942== by 0x531990: process_command (unixctl.c:308)
==26942== by 0x531990: run_connection (unixctl.c:342)
==26942== by 0x531990: unixctl_server_run (unixctl.c:393)
==26942== by 0x40761E: main (ovs-vswitchd.c:128)
1316: ofproto-dpif - conntrack - tcp port reuse
==24039== Conditional jump or move depends on uninitialised value(s)
==24039== at 0x587BF5: check_orig_tuple (conntrack.c:1004)
==24039== by 0x587BF5: process_one (conntrack.c:1141)
==24039== by 0x587BF5: conntrack_execute (conntrack.c:1220)
==24039== by 0x47B02F: dp_execute_cb (dpif-netdev.c:7306)
==24039== by 0x4AF7A6: odp_execute_actions (odp-execute.c:794)
==24039== by 0x47755B: dp_netdev_execute_actions (dpif-netdev.c:7350)
==24039== by 0x47755B: handle_packet_upcall (dpif-netdev.c:6631)
==24039== by 0x47755B: fast_path_processing (dpif-netdev.c:6727)
==24039== by 0x47935C: dp_netdev_input__ (dpif-netdev.c:6815)
==24039== by 0x479AD8: dp_netdev_input (dpif-netdev.c:6853)
==24039== by 0x479AD8: dp_netdev_process_rxq_port
(dpif-netdev.c:4287)
==24039== by 0x47A6C9: dpif_netdev_run (dpif-netdev.c:5264)
==24039== by 0x4324F7: type_run (ofproto-dpif.c:342)
==24039== by 0x41C5FE: ofproto_type_run (ofproto.c:1734)
==24039== by 0x40BAAC: bridge_run__ (bridge.c:2965)
==24039== by 0x410CF3: bridge_run (bridge.c:3029)
==24039== by 0x407614: main (ovs-vswitchd.c:127)
==24039== Uninitialised value was created by a heap allocation
==24039== at 0x4C2DB8F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24039== by 0x5325C4: xmalloc (util.c:138)
==24039== by 0x46D144: dp_packet_new (dp-packet.c:153)
==24039== by 0x46D144: dp_packet_new_with_headroom (dp-packet.c:163)
==24039== by 0x51191E: eth_from_hex (packets.c:498)
==24039== by 0x4A03B9: eth_from_packet (netdev-dummy.c:1609)
==24039== by 0x4A03B9: netdev_dummy_receive (netdev-dummy.c:1765)
==24039== by 0x5319E0: process_command (unixctl.c:308)
==24039== by 0x5319E0: run_connection (unixctl.c:342)
==24039== by 0x5319E0: unixctl_server_run (unixctl.c:393)
==24039== by 0x40761E: main (ovs-vswitchd.c:128)
According to comments in pkt_metadata_init(), conntrack data is valid
only if pkt_metadata.ct_state != 0. This patch prevents
check_orig_tuple() get called when conntrack data is uninitialized.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 11 Sep 2019 21:18:35 +0000 (14:18 -0700)]
db-ctl-base: Free leaked ovsdb_datum
Valgrind reported:
2491: database commands -- negative checks
==19245== 36 (32 direct, 4 indirect) bytes in 1 blocks are definitely lost in loss record 36 of 53
==19245== at 0x4C2FD5F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19245== by 0x431AB4: xrealloc (util.c:149)
==19245== by 0x41656D: ovsdb_datum_reallocate (ovsdb-data.c:1883)
==19245== by 0x41656D: ovsdb_datum_union (ovsdb-data.c:1961)
==19245== by 0x4107B2: cmd_add (db-ctl-base.c:1494)
==19245== by 0x406E2E: do_vsctl (ovs-vsctl.c:2626)
==19245== by 0x406E2E: main (ovs-vsctl.c:183)
==19252== 16 bytes in 1 blocks are definitely lost in loss record 9 of 52
==19252== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19252== by 0x430F74: xmalloc (util.c:138)
==19252== by 0x414D07: clone_atoms (ovsdb-data.c:990)
==19252== by 0x4153F6: ovsdb_datum_clone (ovsdb-data.c:1012)
==19252== by 0x4104D3: cmd_remove (db-ctl-base.c:1564)
==19252== by 0x406E2E: do_vsctl (ovs-vsctl.c:2626)
==19252== by 0x406E2E: main (ovs-vsctl.c:183)
This patch fixes them.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 11 Sep 2019 21:18:34 +0000 (14:18 -0700)]
ofproto-dpif: Free leaked 'webster'
Valgrind reported:
1122: ofproto-dpif - select group with explicit dp_hash selection method
==16884== 64 bytes in 1 blocks are definitely lost in loss record 320 of 346
==16884== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16884== by 0x532512: xcalloc (util.c:121)
==16884== by 0x4262B9: group_setup_dp_hash_table (ofproto-dpif.c:4846)
==16884== by 0x4267CB: group_set_selection_method (ofproto-dpif.c:4938)
==16884== by 0x4267CB: group_construct (ofproto-dpif.c:4984)
==16884== by 0x417250: init_group (ofproto.c:7286)
==16884== by 0x41B4FC: add_group_start (ofproto.c:7316)
==16884== by 0x42247A: ofproto_group_mod_start (ofproto.c:7589)
==16884== by 0x4250EC: handle_group_mod (ofproto.c:7744)
==16884== by 0x4250EC: handle_single_part_openflow (ofproto.c:8428)
==16884== by 0x4250EC: handle_openflow (ofproto.c:8606)
==16884== by 0x4579E2: ofconn_run (connmgr.c:1318)
==16884== by 0x4579E2: connmgr_run (connmgr.c:355)
==16884== by 0x41E0F5: ofproto_run (ofproto.c:1845)
==16884== by 0x40BA63: bridge_run__ (bridge.c:2971)
==16884== by 0x410CF3: bridge_run (bridge.c:3029)
==16884== by 0x407614: main (ovs-vswitchd.c:127)
This patch fixes it.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 11 Sep 2019 21:18:33 +0000 (14:18 -0700)]
dns-resolve: Free 'struct ub_result' when callback returns error results
Valgrind reported:
1074: ofproto - flush flows, groups, and meters for controller change
==5499== 695 (288 direct, 407 indirect) bytes in 3 blocks are definitely lost in loss record 344 of 355
==5499== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5499== by 0x5E7F145: ??? (in /usr/lib/x86_64-linux-gnu/libunbound.so.2.4.0)
==5499== by 0x5E6EBDE: ub_resolve_async (in /usr/lib/x86_64-linux-gnu/libunbound.so.2.4.0)
==5499== by 0x55C739: resolve_async__.part.5 (dns-resolve.c:233)
==5499== by 0x55C85C: resolve_async__ (dns-resolve.c:261)
==5499== by 0x55C85C: resolve_callback__ (dns-resolve.c:262)
==5499== by 0x5E6FEF1: ub_process (in /usr/lib/x86_64-linux-gnu/libunbound.so.2.4.0)
==5499== by 0x55CAF3: dns_resolve (dns-resolve.c:153)
==5499== by 0x523864: parse_sockaddr_components_dns (socket-util.c:438)
==5499== by 0x523864: parse_sockaddr_components (socket-util.c:504)
==5499== by 0x524468: inet_parse_active (socket-util.c:541)
==5499== by 0x524564: inet_open_active (socket-util.c:579)
==5499== by 0x5959F9: tcp_open (stream-tcp.c:56)
==5499== by 0x529192: stream_open (stream.c:228)
==5499== by 0x529910: stream_open_with_default_port (stream.c:724)
==5499== by 0x595FAE: vconn_stream_open (vconn-stream.c:81)
==5499== by 0x535C9B: vconn_open (vconn.c:250)
==5499== by 0x517C59: reconnect (rconn.c:467)
==5499== by 0x5184C7: run_BACKOFF (rconn.c:492)
==5499== by 0x5184C7: rconn_run (rconn.c:660)
==5499== by 0x457FE8: ofservice_run (connmgr.c:1992)
==5499== by 0x457FE8: connmgr_run (connmgr.c:367)
==5499== by 0x41E0F5: ofproto_run (ofproto.c:1845)
==5499== by 0x40BA63: bridge_run__ (bridge.c:2971)
In ub_resolve_async's callback function, 'struct ub_result' should be
finally freed even if there is a resolving error. This patch fixes it.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 11 Sep 2019 21:18:32 +0000 (14:18 -0700)]
ovsdb-client: Free ovsdb_schema
Valgrind reported:
1925: schema conversion online - standalone
==10727== 689 (56 direct, 633 indirect) bytes in 1 blocks are definitely lost in loss record 64 of 66
==10727== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10727== by 0x449D42: xcalloc (util.c:121)
==10727== by 0x40F45C: ovsdb_schema_create (ovsdb.c:41)
==10727== by 0x40F7F8: ovsdb_schema_from_json (ovsdb.c:217)
==10727== by 0x40FB4E: ovsdb_schema_from_file (ovsdb.c:101)
==10727== by 0x40B156: do_convert (ovsdb-client.c:1639)
==10727== by 0x4061C6: main (ovsdb-client.c:282)
This patch fixes it.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 11 Sep 2019 21:18:31 +0000 (14:18 -0700)]
trigger: Free leaked ovsdb_schema
Valgrind reported:
1925: schema conversion online - standalone
==10884== 689 (56 direct, 633 indirect) bytes in 1 blocks are definitely lost in loss record 384 of 420
==10884== at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10884== by 0x44A592: xcalloc (util.c:121)
==10884== by 0x40E2EC: ovsdb_schema_create (ovsdb.c:41)
==10884== by 0x40E688: ovsdb_schema_from_json (ovsdb.c:217)
==10884== by 0x416C6F: ovsdb_trigger_try (trigger.c:246)
==10884== by 0x40D4DE: ovsdb_jsonrpc_trigger_create (jsonrpc-server.c:1119)
==10884== by 0x40D4DE: ovsdb_jsonrpc_session_got_request (jsonrpc-server.c:986)
==10884== by 0x40D4DE: ovsdb_jsonrpc_session_run (jsonrpc-server.c:556)
==10884== by 0x40D4DE: ovsdb_jsonrpc_session_run_all (jsonrpc-server.c:586)
==10884== by 0x40D4DE: ovsdb_jsonrpc_server_run (jsonrpc-server.c:401)
==10884== by 0x406A6E: main_loop (ovsdb-server.c:209)
==10884== by 0x406A6E: main (ovsdb-server.c:460)
'new_schema' should also be freed when there is no error.
This patch fixes it.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 11 Sep 2019 21:18:30 +0000 (14:18 -0700)]
ovs-ofctl: Free leaked minimatch
Valgrind reported:
1056: ofproto - bundle with multiple flow mods (OpenFlow 1.4)
==19220== 160 bytes in 2 blocks are definitely lost in loss record 24 of 34
==19220== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19220== by 0x4979A4: xmalloc (util.c:138)
==19220== by 0x42407D: miniflow_alloc (flow.c:3340)
==19220== by 0x4296CF: minimatch_init (match.c:1758)
==19220== by 0x46273D: parse_ofp_str__ (ofp-flow.c:1759)
==19220== by 0x465B9E: parse_ofp_str (ofp-flow.c:1790)
==19220== by 0x465CE0: parse_ofp_flow_mod_str (ofp-flow.c:1817)
==19220== by 0x465DF6: parse_ofp_flow_mod_file (ofp-flow.c:1876)
==19220== by 0x410BA3: ofctl_flow_mod_file.isra.19 (ovs-ofctl.c:1773)
==19220== by 0x417933: ovs_cmdl_run_command__ (command-line.c:223)
==19220== by 0x406F68: main (ovs-ofctl.c:179)
This patch fixes it.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 11 Sep 2019 21:18:29 +0000 (14:18 -0700)]
dpif-netdev: Handle uninitialized value error for 'match.wc'
Valgrind reported that match.wc was not initialized, as below:
1176: ofproto-dpif - fragment handling - actions
==21214== Conditional jump or move depends on uninitialised value(s)
==21214== at 0x4B77C1: odp_flow_key_from_flow__ (odp-util.c:6143)
==21214== by 0x46DB58: dp_netdev_upcall (dpif-netdev.c:6239)
==21214== by 0x4774A7: handle_packet_upcall (dpif-netdev.c:6608)
==21214== by 0x4774A7: fast_path_processing (dpif-netdev.c:6726)
==21214== by 0x47933C: dp_netdev_input__ (dpif-netdev.c:6814)
==21214== by 0x479AB8: dp_netdev_input (dpif-netdev.c:6852)
==21214== by 0x479AB8: dp_netdev_process_rxq_port (dpif-netdev.c:4287)
==21214== by 0x47A6A9: dpif_netdev_run (dpif-netdev.c:5264)
==21214== by 0x4324E7: type_run (ofproto-dpif.c:342)
==21214== by 0x41C5FE: ofproto_type_run (ofproto.c:1734)
==21214== by 0x40BAAC: bridge_run__ (bridge.c:2965)
==21214== by 0x410CF3: bridge_run (bridge.c:3029)
==21214== by 0x407614: main (ovs-vswitchd.c:127)
==21214== Uninitialised value was created by a stack allocation
==21214== at 0x4769C3: fast_path_processing (dpif-netdev.c:6672)
'match' is allocated on stack but its 'wc' is accessed in
odp_flow_key_from_flow__ without proper initialization.
This patch fixes it.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 11 Sep 2019 21:18:28 +0000 (14:18 -0700)]
ofproto-dpif: Uninitialize 'xlate_cache' to free resources
Valgrind reported:
1210: ofproto-dpif - continuation after clone
==32205== 4,392 (1,440 direct, 2,952 indirect) bytes in 12 blocks are definitely lost in loss record 359 of 362
==32205== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==32205== by 0x532574: xmalloc (util.c:138)
==32205== by 0x4F98CA: ofpbuf_init (ofpbuf.c:123)
==32205== by 0x42C07B: nxt_resume (ofproto-dpif.c:5110)
==32205== by 0x41796F: handle_nxt_resume (ofproto.c:3677)
==32205== by 0x424583: handle_single_part_openflow (ofproto.c:8473)
==32205== by 0x424583: handle_openflow (ofproto.c:8606)
==32205== by 0x4579E2: ofconn_run (connmgr.c:1318)
==32205== by 0x4579E2: connmgr_run (connmgr.c:355)
==32205== by 0x41E0F5: ofproto_run (ofproto.c:1845)
==32205== by 0x40BA63: bridge_run__ (bridge.c:2971)
==32205== by 0x410CF3: bridge_run (bridge.c:3029)
==32205== by 0x407614: main (ovs-vswitchd.c:127)
This is because 'xcache' was not destroyed properly. This patch fixes it.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 11 Sep 2019 21:18:27 +0000 (14:18 -0700)]
raft: Free leaked json data
Valgrind reported:
1924: compacting online - cluster
==29312== 2,886 (240 direct, 2,646 indirect) bytes in 6 blocks are definitely lost in loss record 406 of 413
==29312== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==29312== by 0x44A5F4: xmalloc (util.c:138)
==29312== by 0x4308EA: json_create (json.c:1451)
==29312== by 0x4308EA: json_object_create (json.c:254)
==29312== by 0x430ED0: json_parser_push_object (json.c:1273)
==29312== by 0x430ED0: json_parser_input (json.c:1371)
==29312== by 0x431CF1: json_lex_input (json.c:991)
==29312== by 0x43233B: json_parser_feed (json.c:1149)
==29312== by 0x41D87F: parse_body.isra.0 (log.c:411)
==29312== by 0x41E141: ovsdb_log_read (log.c:476)
==29312== by 0x42646D: raft_read_log (raft.c:866)
==29312== by 0x42646D: raft_open (raft.c:951)
==29312== by 0x4151AF: ovsdb_storage_open__ (storage.c:81)
==29312== by 0x408FFC: open_db (ovsdb-server.c:642)
==29312== by 0x40657F: main (ovsdb-server.c:358)
This patch fixes it.
Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
William Tu [Thu, 12 Sep 2019 17:07:45 +0000 (10:07 -0700)]
ovs-bugtool: Add ip -s -s to get_device_stats.out.
The patch adds 'ip -s -s' to file get_device_stats.out to collect
device statistics. When debugging tunnel related issues, the command
shows much more detailed counters, ex: frame, crc, carrier, helping
to understand the root cause when packets are dropped.
Reviewed-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: William Tu <u9012063@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Wed, 18 Sep 2019 14:32:52 +0000 (07:32 -0700)]
db-ctl-base: Give better error messages for ambiguous abbreviations.
Tables and columns may be abbreviated to unique prefixes, but until
now the error messages have just said there's more than one match.
This commit makes the error messages list the possibilities.
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Mark Michelson [Fri, 6 Sep 2019 14:33:03 +0000 (10:33 -0400)]
Remove OVN.
OVN is separated into its own repo. This commit removes the OVN source,
OVN tests, and OVN documentation. It also removes mentions of OVN from
most documentation. The only place where OVN has been left is in
changelogs/NEWS, since we shouldn't mess with the history of the
project.
There is an exception here. The ovsdb-cluster tests rely on ovn-nbctl
and ovn-sbctl to run. Therefore those ovn utilities, as well as their
dependencies remain in the repo with this commit.
Acked-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Mark Michelson <mmichels@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Useful for tracking where the uninitialized memory came from.
Report example:
Thread 13 revalidator11:
Conditional jump or move depends on uninitialised value(s)
at 0x4C35D96: __memcmp_sse4_1 (in vgpreload_memcheck.so)
by 0x9D4404: ofpbuf_equal (ofpbuf.h:273)
by 0x9D4404: revalidate_ukey__ (ofproto-dpif-upcall.c:2219)
<...>
by 0x6AF488E: clone (clone.S:95)
Uninitialised value was created by a stack allocation
at 0x9D4450: compose_slow_path (ofproto-dpif-upcall.c:1062)
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Tested-by: William Tu <u9012063@gmail.com> Acked-by: Ben Pfaff <blp@ovn.org>
This is highly useful to see on which core PMD is running by
only looking at the thread name. Thread Id still allows to
distinguish different threads running on the same core over the time:
In gdb, top or any other utility it's useful to quickly catch up
needed thread without parsing logs, memory or matching threads by port
names they're handling.
Ilya Maximets [Tue, 13 Aug 2019 11:28:26 +0000 (14:28 +0300)]
dpdk: Use ovs-numa provided functions to manage thread affinity.
This allows to decrease code duplication and avoid using Linux-specific
functions (this might be useful in the future if we'll try to allow
running OvS+DPDK on FreeBSD).
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Reviewed-by: David Marchand <david.marchand@redhat.com> Acked-by: William Tu <u9012063@gmail.com>
dpif-netdev-perf: Fix TSC frequency for non-DPDK case.
Unlike 'rte_get_tsc_cycles()' which doesn't need any specific
initialization, 'rte_get_tsc_hz()' could be used only after successfull
call to 'rte_eal_init()'. 'rte_eal_init()' estimates the TSC frequency
for later use by 'rte_get_tsc_hz()'. Fairly said, we're not allowed
to use 'rte_get_tsc_cycles()' before initializing DPDK too, but it
works this way for now and provides correct results.
This patch provides TSC frequency estimation code that will be used
in two cases:
* DPDK is not compiled in, i.e. DPDK_NETDEV not defined.
* DPDK compiled in but not initialized,
i.e. other_config:dpdk-init=false
This change is mostly useful for AF_XDP netdev support, i.e. allows
to use dpif-netdev/pmd-perf-show command and various PMD perf metrics.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Reviewed-by: David Marchand <david.marchand@redhat.com> Acked-by: William Tu <u9012063@gmail.com>
Greg Rose [Tue, 3 Sep 2019 15:50:42 +0000 (08:50 -0700)]
rhel: Revert RHEL 7.4 comp_ver change
I looked at the wrong list of kernels when I changed the value for the
RHEL 7.4 comp_ver variable. Revert that part of commit e64c2c1
("rhel: Fix ovs-kmod-manage.sh to work with RHEL 7.3").
Fixes: e64c2c1 ("rhel: Fix ovs-kmod-manage.sh to work with RHEL 7.3") Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Gurucharan Shetty <guru@ovn.org>
Greg Rose [Thu, 29 Aug 2019 18:56:01 +0000 (11:56 -0700)]
rhel: Fix ovs-kmod-manage.sh to work with RHEL 7.3
Add case for RHEL 7.3. This also fixes commit 22abff2 where I forgot to
update the comp_ver variable for RHEL 7.5 and while I was in there I
updated comp_ver for the RHEL 7.4 case as well.
Fixes: 22abff2 ("rhel: Add case for RHEL 7.5 major version to...") Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Nitin Katiyar [Wed, 28 Aug 2019 16:42:07 +0000 (22:12 +0530)]
packets: Fix using outdated RSS hash after MPLS decapsulation.
When a packet is received, the RSS hash is calculated if it is not
already available. The Exact Match Cache (EMC) entry is then looked up
using this RSS hash.
When a MPLS encapsulated packet is received, the MPLS header is popped
and the packet is recirculated. Since the RSS hash has not been
invalidated here, the EMC lookup for all decapsulated packets will hit
the same entry even though these packets will have different tuple
values. This degrades performance severely as different inner packets
from the same MPLS tunnel would hit the same EMC entry.
This patch invalidates RSS hash (by resetting offload flags) after MPLS
header is popped.
dpif-netdev: Fail port addition if reconfiguration failed.
If the port was destroyed during the initial reconfiguration, we should
report an error to the upper layers. Otherwise, successful addition of
the port will be logged and upper layers will continue to configure
this port. For example, the 'dpif' layer will try to initilaize flow
API for this device.
Fix that by checking for port existence after reconfiguration. We can't
get the real error code here, so let's assume EINVAL. 'ovs-vsctl' will
tell the user to check the logs for a real reason anyway.
Fixes: e32971b8ddb4 ("dpif-netdev: Centralized threads and queues handling code.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Ian Stokes <ian.stokes@intel.com>
Make pid_exists() more robust against empty pid argument
In some of our destructive testing of ovn-dbs inside containers managed
by pacemaker we reached a situation where /var/run/openvswitch had
empty .pid files. The current code does not deal well with them
and pidfile_is_running() returns true in such a case and this confuses
the OCF resource agent.
- Before this change:
Inside a container run:
killall ovsdb-server;
echo -n '' > /var/run/openvswitch/ovnnb_db.pid; echo -n '' > /var/run/openvswitch/ovnsb_db.pid
We will observe that the cluster is unable to ever recover because
it believes the ovn processes to be running when they really aren't and
eventually just fails:
podman container set: ovn-dbs-bundle [192.168.24.1:8787/rhosp15/openstack-ovn-northd:pcmklatest]
ovn-dbs-bundle-0 (ocf::ovn:ovndb-servers): Master controller-0
ovn-dbs-bundle-1 (ocf::ovn:ovndb-servers): Stopped controller-1
ovn-dbs-bundle-2 (ocf::ovn:ovndb-servers): Slave controller-2
Let's make sure pid_exists() returns false when the pid is an empty
string.
- After this change the cluster is able to recover from this state and
correctly start the resource:
podman container set: ovn-dbs-bundle [192.168.24.1:8787/rhosp15/openstack-ovn-northd:pcmklatest]
ovn-dbs-bundle-0 (ocf::ovn:ovndb-servers): Master controller-0
ovn-dbs-bundle-1 (ocf::ovn:ovndb-servers): Slave controller-1
ovn-dbs-bundle-2 (ocf::ovn:ovndb-servers): Slave controller-2
Fixes: 3028ce2595c8 ("ovs-lib: Allow "status" command to work as non-root.") Signed-off-by: Michele Baldessari <michele@acksyn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Darrell Ball [Tue, 27 Aug 2019 23:59:02 +0000 (16:59 -0700)]
conntrack: Fix ICMPv4 error data L4 length check.
The ICMPv4 error data L4 length check was found to be too strict for TCP,
expecting a minimum of 20 rather than 8 bytes. This worked by
hapenstance for other inner protocols. The approach is to explicitly
handle the ICMPv4 error data L4 length check and to do this for all
supported inner protocols in the same way. Making the code common
between protocols also allows the existing ICMPv4 related UDP tests to
cover TCP and ICMP inner protocol cases.
Note that ICMPv6 does not have an 8 byte limit for error L4 data.
Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") CC: Daniele Di Proietto <diproiettod@ovn.org>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-August/361949.html Reported-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Co-authored-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Signed-off-by: Darrell Ball <dlu998@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Wed, 7 Aug 2019 22:25:33 +0000 (15:25 -0700)]
datapath: compat: Backports bugfixes for nf_conncount
This patch backports several critical bug fixes related to
locking and data consistency in nf_conncount code.
This backport is based on the following upstream net-next upstream commits. a007232 ("netfilter: nf_conncount: fix argument order to find_next_bit") c80f10b ("netfilter: nf_conncount: speculative garbage collection on empty lists") 2f971a8 ("netfilter: nf_conncount: move all list iterations under spinlock") df4a902 ("netfilter: nf_conncount: merge lookup and add functions") e8cfb37 ("netfilter: nf_conncount: restart search when nodes have been erased") f7fcc98 ("netfilter: nf_conncount: split gc in two phases") 4cd273b ("netfilter: nf_conncount: don't skip eviction when age is negative") c78e781 ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS with CONNCOUNT_SLOTS") d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of rb_link_node()") 53ca0f2 ("netfilter: nf_conncount: remove wrong condition check routine") 3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of list.") 31568ec ("netfilter: nf_conncount: fix list_del corruption in conn_free") fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of spin_lock")
This patch adds additional compat code so that it can build on
all supported kernel versions.
In addition, this patch helps OVS datapath to always choose bug-fixed
nf_conncount code. If kernel already has these fixes, then kernel's
nf_conncount is being used. Otherwise, OVS falls back to use compat
nf_conncount functions.
Travis tests are at
https://travis-ci.org/yifsun/ovs-travis/builds/569056850
On latest RHEL kernel, 'make check-kmod' runs good.
openvswitch: Clear the L4 portion of the key for "later" fragments.
Only the first fragment in a datagram contains the L4 headers. When the
Open vSwitch module parses a packet, it always sets the IP protocol
field in the key, but can only set the L4 fields on the first fragment.
The original behavior would not clear the L4 portion of the key, so
garbage values would be sent in the key for "later" fragments. This
patch clears the L4 fields in that circumstance to prevent sending those
garbage values as part of the upcall.
Signed-off-by: Justin Pettit <jpettit@ovn.org> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
openvswitch: Properly set L4 keys on "later" IP fragments
When IP fragments are reassembled before being sent to conntrack, the
key from the last fragment is used. Unless there are reordering
issues, the last fragment received will not contain the L4 ports, so the
key for the reassembled datagram won't contain them. This patch updates
the key once we have a reassembled datagram.
The handle_fragments() function works on L3 headers so we pull the L3/L4
flow key update code from key_extract into a new function
'key_extract_l3l4'. Then we add a another new function
ovs_flow_key_update_l3l4() and export it so that it is accessible by
handle_fragments() for conntrack packet reassembly.
Co-authored-by: Justin Pettit <jpettit@ovn.org> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Acked-by: Pravin B Shelar <pshelar@ovn.org> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Justin Pettit <jpettit@ovn.org>
* check that expected bytes and packets stats are correctly read from
every flow.
* check that the expected elements are read for every field type
aggregation.
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yanqin Wei [Thu, 22 Aug 2019 10:09:50 +0000 (18:09 +0800)]
flow: save "vlan_hdrs" memset for untagged traffic
For untagged traffic, it is unnecessary to clear vlan_hdrs as it costs 32B
memset. So the patch improves it by postponing to clear vlan_hdrs until
ethtype check. It can benefit both untagged and single-tagged traffic. From
testing, it does not impact performance of dual-tagged traffic.
Reviewed-by: Gavin Hu <Gavin.Hu@arm.com> Signed-off-by: Yanqin Wei <Yanqin.Wei@arm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Malvika Gupta [Tue, 27 Aug 2019 18:21:08 +0000 (13:21 -0500)]
flow: Reduce metadata connection state branches in miniflow_extract
This patch merges two separate if-else branches for metadata connection state
into one if-else branch to improve performance. It gives an average performance
improvement of ~3% on arm platforms and ~4.5% on x86 platforms.
Signed-off-by: Malvika Gupta <malvika.gupta@arm.com> Reviewed-by: Yanqin Wei <yanqin.wei@arm.com> Reviewed-by: Gavin Hu <gavin.hu@arm.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Dumitru Ceara [Fri, 16 Aug 2019 14:31:28 +0000 (16:31 +0200)]
pinctrl: Fix DNS packet parsing
Due to the use of a uint8_t to index inside the DNS payload we could end
up in an infinite loop when specific (invalid) DNS packets were
processed by ovn-controller. In the infinite loop we keep increasing the
query_name dynamic string until running out of memory.
One way to replicate the issue is to configure DNS on the logical switch
and then inject a manually crafted DNS-like packet. For example, with
Scapy:
>>> p = IP(dst='10.0.0.2',src='10.0.0.3')/UDP(dport=53)/('a'*364)
>>> send(p)
Also add a sanity check on minimum L4 size of packets.
Lorenzo Bianconi [Mon, 26 Aug 2019 09:19:32 +0000 (11:19 +0200)]
OVN: fix memory leak in build_pre_lb
Fix memory leak of ip_address string in build_pre_lb routine if we
install logical flows for empty_lb controller event
Fixes: f49b17a6cbe3 ("OVN: use trigger_event action to report 'empty_lb_rule' events") Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Flavio Leitner [Tue, 13 Aug 2019 16:34:04 +0000 (13:34 -0300)]
tnl-neigh: Use outgoing ofproto version.
When a packet needs to be encapsulated in userspace, the endpoint
address needs to be resolved to fill in the headers. If it is not,
then currently OvS sends either a Neighbor Solicitation (IPv6)
or an ARP Query (IPv4) to resolve it.
The problem is that the NS/ARP packet will go through the flow
rules in the new bridge, but inheriting the ofproto table version
from the original packet to be encapsulated. When those versions
don't match, the result is unexpected because no flow rules might
be visible, which would cause the default table rule to be used
to drop the packet. Or only part of the flow rules would be visible
and so on.
Since the NS/ARP packet is created by OvS and will be injected in
the outgoing bridge, use the corresponding ofproto version instead.
Reviewed-by: David Marchand <david.marchand@redhat.com> Acked-By: Vasu Dasari <vdasari@gmail.com> Signed-off-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Greg Rose [Tue, 27 Aug 2019 21:06:29 +0000 (14:06 -0700)]
rhel: Add case for RHEL 7.5 major version to kmod manage script
A Centos 7.5 kernel with an unencountered set of minor build numbers
caused an upgrade bug. Adding the case for the rhel 7.5 kmod management
script fixes the problem.
Signed-off-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Gurucharan Shetty <guru@ovn.org>
William Tu [Tue, 27 Aug 2019 18:09:13 +0000 (11:09 -0700)]
ovs-lib: Add timeout at ovs-check-dead-ifs.
At SUSE12 SP3, we hit a case where ovs-check-dead-ifs tries to read
an entry in /proc/<pid>/fd/<some fd> but hangs forever. The pid is
a qemu-system-x86_64 process and we suspect it's an issue related to
qemu, not ovs. As a result, force-reload-kmod hangs and OVS bridge
never gets restarted. This patch adds a timeout of 5-seconds to
ovs-check-dead-ifs.
VMware-BZ: #2408059 Signed-off-by: William Tu <u9012063@gmail.com> Cc: Ashish Varma <ashishvarma.ovs@gmail.com> Cc: Gurucharan Shetty <guru@ovn.org> Signed-off-by: Gurucharan Shetty <guru@ovn.org>
Ilya Maximets [Tue, 6 Aug 2019 15:57:09 +0000 (18:57 +0300)]
netdev-dpdk: Refactor vhost custom stats for extensibility.
vHost interfaces currently has only one custom statistic, but there
might be others in the near future. This refactoring makes the code
work in the same way as it done for dpdk and afxdp stats to keep the
common style over the different code places and makes it easily
extensible for the new stats addition.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Reviewed-by: David Marchand <david.marchand@redhat.com> Acked-by: Kevin Traynor <ktraynor@redhat.com>
Ilya Maximets [Tue, 6 Aug 2019 15:46:36 +0000 (18:46 +0300)]
netdev-dpdk: Fix not reporting rx_oversize_errors in stats.
There is a big code duplication issue with DPDK xstats that led to
missed "rx_oversize_errors" statistics. It's defined but not used.
Fix that by actually using this stat along with code refactoring that
will allow us to not make same mistakes in the future.
Macro definitions are perfectly suitable to automate code generation
in such cases and already used in a couple of places in OVS for similar
purposes.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Reviewed-by: David Marchand <david.marchand@redhat.com> Acked-by: Kevin Traynor <ktraynor@redhat.com> Acked-by: Ian Stokes <ian.stokes@intel.com>
Han Zhou [Thu, 22 Aug 2019 21:08:22 +0000 (14:08 -0700)]
raft: Save and read new election timer in header snapshot.
This patch store the latest election timer in snapshot during log
compression, and when server restarts it reads the value from the log.
Without this, any previous changes to election timer will be lost
in the log, and if server restarts, it will use the default value
instead of the changed value.
Fixes: commit 8e35461 ("ovsdb raft: Support leader election time change online.") Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Thu, 22 Aug 2019 21:08:21 +0000 (14:08 -0700)]
raft.c: Election timer initial reset with value from log.
After election timer is changed through cluster/change-election-timer
command, if a server restarts, it firstly initializes with the default
value and use it to reset the timer. Although it reads the latest
timer value later from the log, the first timeout may be much shorter
than expected by other servers that use latest timeout, and it would
start election before it receives the first heartbeat from the leader.
This patch fixes it by changing the order of reading log and resetting
timer so that the latest value is read from the log before the initial
resetting of the timer.
Fixes: commit 8e35461 ("ovsdb raft: Support leader election time change online.") Signed-off-by: Han Zhou <hzhou8@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
ofproto-dpif: Fix using uninitialised memory in user_action_cookie.
Designated initializers are not suitable for initializing non-packed
structures and unions which are subjects for comparison by memcmp().
Whole memory for 'struct user_action_cookie' must be explicitly cleared
before using because it will be copied with memcpy and later compared
by memcmp in ofpbuf_equal().
Few issues found be valgrind:
Thread 13 revalidator11:
Conditional jump or move depends on uninitialised value(s)
at 0x4C35D96: __memcmp_sse4_1 (in vgpreload_memcheck.so)
by 0x9D4404: ofpbuf_equal (ofpbuf.h:273)
by 0x9D4404: revalidate_ukey__ (ofproto-dpif-upcall.c:2219)
by 0x9D4404: revalidate_ukey (ofproto-dpif-upcall.c:2286)
by 0x9D62AC: revalidate (ofproto-dpif-upcall.c:2685)
by 0x9D62AC: udpif_revalidator (ofproto-dpif-upcall.c:942)
by 0xA9C732: ovsthread_wrapper (ovs-thread.c:383)
by 0x5FF86DA: start_thread (pthread_create.c:463)
by 0x6AF488E: clone (clone.S:95)
Uninitialised value was created by a stack allocation
at 0x9D4450: compose_slow_path (ofproto-dpif-upcall.c:1062)
Thread 11 revalidator16:
Conditional jump or move depends on uninitialised value(s)
at 0x4C35D96: __memcmp_sse4_1 (in vgpreload_memcheck.so)
by 0x9D4404: ofpbuf_equal (ofpbuf.h:273)
by 0x9D4404: revalidate_ukey__ (ofproto-dpif-upcall.c:2220)
by 0x9D4404: revalidate_ukey (ofproto-dpif-upcall.c:2287)
by 0x9D62BC: revalidate (ofproto-dpif-upcall.c:2686)
by 0x9D62BC: udpif_revalidator (ofproto-dpif-upcall.c:942)
by 0xA9C6D2: ovsthread_wrapper (ovs-thread.c:383)
by 0x5FF86DA: start_thread (pthread_create.c:463)
by 0x6AF488E: clone (clone.S:95)
Uninitialised value was created by a stack allocation
at 0x9DC4E0: compose_sflow_action (ofproto-dpif-xlate.c:3211)
The struct was never marked as 'packed', however it was manually
adjusted to be so in practice.
Old IPFIX related commit first made the structure non-contiguous.
Commit 8de6ff3ea864 ("ofproto-dpif: Use a fixed size userspace cookie.")
added uninitialized parts of the additional union space and the next
one introduced new holes between structure fields for all cases.
CC: Justin Pettit <jpettit@ovn.org> Fixes: 8b7ea2d48033 ("Extend OVS IPFIX exporter to export tunnel headers") Fixes: 8de6ff3ea864 ("ofproto-dpif: Use a fixed size userspace cookie.") Fixes: fcb9579be3c7 ("ofproto: Add 'ofproto_uuid' and 'ofp_in_port' to user action cookie.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Ben Pfaff <blp@ovn.org>
Lorenzo Bianconi [Thu, 22 Aug 2019 16:51:40 +0000 (18:51 +0200)]
Remove ageing check in run_put_mac_binding
Remove ageing check in run_put_mac_binding routine on mac-binding info
since if ovn-controller main thread is heavy loaded the info will be
discarded and the mac_binding table will not never be updated
Acked-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Lorenzo Bianconi [Mon, 29 Jul 2019 11:41:03 +0000 (13:41 +0200)]
OVN: fix DNAT/SNAT system-ovn unit tests
Fix conntrack checks in the following tests in tests/system-ovn.at:
- ovn -- DNAT and SNAT on distributed router - N/S
- ovn -- DNAT and SNAT on distributed router - E/W
Fixes: a6ee09882283 ("OVN: run local logical flows first in S_ROUTER_OUT_SNAT table") Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Aliasgar Ginwala [Sat, 17 Aug 2019 07:21:45 +0000 (00:21 -0700)]
OVS: Containerize components
1. Start OVS components in containers so that building and shipping
of OVS components is easy.
2. Load OVS kernel modules on host from container to avoid installing ovs
on host.
3. Update documentation about how to build/run ovs in docker.
projects / mirror_ovs.git / log