Paolo Abeni [Fri, 17 Nov 2017 13:36:22 +0000 (14:36 +0100)]
netdev-tc-offloads: update stats properly on flow deletion
Currently, when an offloaded DP flow is deleted, the related stats
are unconditionally cleared. As a result the counters for the
originating open flow are corrupted.
This change addresses the issue updating the DP stats with the current
values provided by the flower APIs before deleting the tc filter, as
currently done by others DP providers.
Tested vs different OVS H/W offload devices, verifying that the open flow
stats are correct after the expiration of the related, H/W offloaded DP
flow.
Fixes: 30b6b047260b ("netdev-tc-offloads: Implement netdev flow del using tc interface") CC: Paul Blakey <paulb@mellanox.com> Reported-by: Kumar Sanghvi <kumaras@chelsio.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com> Acked-by: Flavio Leitner <fbl@sysclose.org> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Ilya Maximets [Fri, 10 Nov 2017 07:12:06 +0000 (10:12 +0300)]
netdev-dpdk: Fix mempool creation with large MTU.
Currently mempool name size limited to 25 characters by
RTE_MEMPOOL_NAMESIZE. netdev-dpdk tries to create mempool with the
following name pattern: "ovs_%{hash}_%{socket}_%{mtu}_%{n_mbuf}".
We have 3 chars for "ovs" + 4 chars for delimiters + 8 chars for
hash (because it's the 32 bit integer printed in hex) + 1 char for
socket_id (mostly 1, but it could be 2 on some systems; larger?) = 16.
Only 25 - 16 = 9 characters remains for mtu + n_mbufs.
Minimum usual value for mtu is 1500 --> 2030 (4 chars) after
dpdk_buf_size conversion and the minimum value for n_mbufs is 16384
(5 chars). So, all the 9 characters are used.
If we'll try to create port with mtu = 9500, mempool creation will
fail, because FRAME_LEN_TO_MTU(dpdk_buf_size(9500)) = 10222 (5 chars)
and this value will overflow the RTE_MEMPOOL_NAMESIZE limit.
Same issue will happen if we'll try to create port with big enough
number of queues or will try to create big enough number of PMD
threads (number of tx queues will enlarge the mempool requirements).
Fix that by removing the delimiters. To keep the readability (at least
partial) of the mempool names exact field sizes with zero padding
are used.
Following limits should be suitable for now:
- Hash length: 8 chars (uint32_t in hex)
- Socket ID : 2 chars (For systems with up to 10 sockets)
- MTU : 5 chars (MTU (10^5 - 1) should be enough for now)
- n_mbufs : 7 chars (Up to 10^7 of mbufs)
Total : 22 + 3 (for "ovs") = 25
CC: Antonio Fischetti <antonio.fischetti@intel.com> CC: Robert Wojciechowicz <robertx.wojciechowicz@intel.com> Fixes: f06546a51dd8 ("Fix mempool names to reflect socket id.") Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Antonio Fischetti <antonio.fischetti@intel.com> Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com> Tested-by: Mark Kavanagh <mark.b.kavanagh@intel.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ilya Maximets [Fri, 6 Oct 2017 10:50:14 +0000 (13:50 +0300)]
netdev-dpdk: Fix calling vhost API with negative vid.
Currently, rx and tx functions for vhost interfaces always obtain
'vid' twice. First time inside 'is_vhost_running' for checking
the value and the second time in enqueue/dequeue function calls to
send/receive packets. But second time we're not checking the
returned value. If vhost device will be destroyed between
checking and enqueue/dequeue, DPDK API will be called with
'-1' instead of valid 'vid'. DPDK API does not validate the 'vid'.
This leads to getting random memory value as a pointer to internal
device structure inside DPDK. Access by this pointer leads to
segmentation fault. For example:
Ilya Maximets [Fri, 10 Nov 2017 15:16:12 +0000 (18:16 +0300)]
netdev-dpdk: Remove unused MAX_NB_MBUF.
MAX_NB_MBUF was used as a default mempool size for almost all ports.
Not used since new per-port mempool allocation introduced.
MIN_NB_MBUF still used as a lower limit.
CC: Robert Wojciechowicz <robertx.wojciechowicz@intel.com> Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Antonio Fischetti <antonio.fischetti@intel.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ilya Maximets [Fri, 10 Nov 2017 15:16:11 +0000 (18:16 +0300)]
netdev-dpdk: Factor out struct dpdk_mp.
Since commit d555d9bded5f ("netdev-dpdk: Create separate memory pool
for each port."), struct dpdk_mp is redundant because each mempool
can be used by single port only and this port already contains all
the information we store in dpdk_mp.
There is no need to duplicate the information.
Fields of this structure currently used only to generate mempool name.
But it's required only while creation and after that we can use
mp->name directly from the struct rte_mempool.
Let's remove this structure and use struct rte_mempool directly
instead.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Antonio Fischetti <antonio.fischetti@intel.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ilya Maximets [Fri, 10 Nov 2017 15:16:10 +0000 (18:16 +0300)]
netdev-dpdk: Fix dpdk_mp leak in case of EEXIST.
In case of EEXIST, 'dpdk_mp_create()' will allocate yet another
'struct dpdk_mp' with same 'mp' pointer inside. We need to free
this structure to avoid the leak.
CC: Robert Wojciechowicz <robertx.wojciechowicz@intel.com> CC: Antonio Fischetti <antonio.fischetti@intel.com> Fixes: d555d9bded5f ("netdev-dpdk: Create separate memory pool for each port.") Fixes: b6b26021d2e2 ("netdev-dpdk: fix management of pre-existing mempools.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Acked-by: Antonio Fischetti <antonio.fischetti@intel.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Ben Pfaff [Tue, 14 Nov 2017 18:15:11 +0000 (10:15 -0800)]
netdev: Eliminate redundant ifindex mapping.
Until now, the code for mapping ODP port number to ifindexes and vice versa
has maintained two completely separate data structures, one for each
direction. It was possible for the two mappings to become out of sync
with each other since either one could change independently. This commit
merges them into a single data structure (with two indexes), which at least
means that if one is removed then the other is as well.
Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: William Tu <u9012063@gmail.com>
Unfortunately MSYS transforms `0::` into the location of the binaries i.e.:
c:\MinGW\msys\1.0\64.
Currently the test:
`testing ovn -- IPv6 Neighbor Solicitation for unknown MAC`
fails because of the above:
"ovn-nbctl: lrp0_ip6: invalid network address: aef0;c:\MinGW\msys\1.0\64"
This patch uses the full form of the IPv6 address instead of its shorter
notation.
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Tue, 14 Nov 2017 00:57:48 +0000 (16:57 -0800)]
tests: Try harder to figure out whether IPv6 is supported.
Until now, the tests have tried to create an IPv6 socket to figure out
whether the system under test supports IPv6. Recently we've seen test
failures on Travis which appear to be because, although the system supports
IPv6, test programs are not allowed to connect or bind IPv6 addresses.
This commit refines the test for IPv6 to also try to bind the IPv6
localhost address, which should convert the test failures to "skip"s.
Acked-by: William Tu <u9012063@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Tested-at: https://travis-ci.org/gvrose8192/ovs-experimental
Reported-at: https://github.com/travis-ci/travis-ci/issues/8711 Signed-off-by: Ben Pfaff <blp@ovn.org>
installer-windows: Call WIX binaries outside of MSBuild on x64
Unfortunately all WIX binaries (candle, heat, etc) are only 32 bit (up to
the latest version 3.11).
For performance reasons they are run as .NET assemblies inside the MSBuild
process. Running 32 bit assemblies inside a 64 bit process (MSBuild) makes
them segfault.
Add a new option for heat to be run as an individual process when the
platform is not x86.
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org> Acked-by: Alin Balutoiu <abalutoiu@cloudbasesolutions.com>
ofp-actions: Add compare to offsetof need for MSVC 2015/17
Unfortunately starting from VS 2015, the "C" definition for `offsetof`
has been changed. Please see:
https://stackoverflow.com/questions/42725929/using-offsetof-with-enum-does-not-compile-in-visual-studio-2015/42726424
Several people reported the bug for 2015 and also 2017 (i.e. :
https://developercommunity.visualstudio.com/content/problem/22196/static-assert-cannot-compile-constexprs-method-tha.html
), but we don't have a fix yet.
This patch adds an explicit compare, although we could redefine the macro
for the same effect.
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org> Acked-by: Anand Kumar <kumaranand@vmware.com>
windows: Add interlocked function definitions for VS 2015
For some unclear and accidental reasons, the Windows 10 SDK
renamed _Interlocked* functions to _InlineInterlocked* (although the
documentation still points to the old form:
https://msdn.microsoft.com/en-us/library/191ca0sk.aspx).
This patch adds mappings for used functions.
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org> Acked-by: Anand Kumar <kumaranand@vmware.com>
windows: _set_output_format is no longer required from VS2015
_set_output_format is deprecated ang no longer required
starting from MSC_VER 1900 (VS 2015):
https://msdn.microsoft.com/en-us/library/bb531344(v=vs.140).aspx .
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org> Acked-by: Anand Kumar <kumaranand@vmware.com>
Ashish Varma [Mon, 6 Nov 2017 20:17:45 +0000 (12:17 -0800)]
netdev, dpif: fix the crash/assert on port delete
a crash is seen in "netdev_ports_remove" when an interface is deleted and added
back in the system and when the interface is part of a bridge configuration.
e.g. steps:
create a tap0 interface using "ip tuntap add.."
add the tap0 interface to br0 using "ovs-vsctl add-port.."
delete the tap0 interface from system using "ip tuntap del.."
add the tap0 interface back in system using "ip tuntap add.."
(this changes the ifindex of the interface)
delete tap0 from br0 using "ovs-vsctl del-port.."
In the function "netdev_ports_insert", two hmap entries were created for
mapping "portnum -> netdev" and "ifindex -> portnum".
When the interface is deleted from the system, the "netdev_ports_remove"
function is not getting called and the old ifindex entry is not getting
cleaned up from the "ifindex_to_port" hmap.
As part of the fix, added function "dpif_port_remove" which will call
"netdev_ports_remove" in the path where the interface deletion from the system
is detected.
Also, in "netdev_ports_remove", added the code where the "ifindex_to_port_data"
(ifindex -> portnum map node) is getting freed when the ifindex is not
available any more. (as the interface is already deleted.)
VMware-BZ: #1975788 Signed-off-by: Ashish Varma <ashishvarma.ovs@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Aaron Conole [Fri, 10 Nov 2017 22:33:20 +0000 (17:33 -0500)]
ovs-tcpdump: handle large interface names on linux
Linux has a fixed size interface name, which will not change. This means
that attempts to dump interfaces whose names are larger than the max size
will result in an error making the tap device.
This commit brings a new function. When the generated name would be too
large, use a random number prefixed by 'ovsmi' instead.
From OVS 2.8, ovs-vswitchd, when it starts, will
load the kernel modules for tunnels. It has logic
inside it to choose either upstream kernel module
or vport-* kernel module.
So, when we run 'force-reload-kmod' to upgrade to
OVS 2.8 from a previous version, we do not need to
remember the vport-* kernel module that was previously
loaded. It is not really harmful to load vport-* kernel
module though.
On RHEL7.x and OVS 2.8, we use the upstream "geneve" kernel
module for tunnels.
But, on RHEL 7.x we have hit a bug caused by iptables
startup script which tries to remove all kernel modules
related to linux conntrack. It fails to unload openvswitch
kernel module because it has a reference count on it. But it
succeeds in unloading vport-geneve and in turn the upstream
"geneve" kernel module. This causes the tunnels to go down.
With this patch, we avoid the above situation, by not loading
vport-geneve kernel module. ovs-vswitchd when it starts will
load upstream geneve. And when "iptables stop" runs, since
"geneve" has nothing to do with conntrack, it spares it.
Ideally, we should fix this by incrementing the refcount
on the kernel modules.
Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: William Tu <u9012063@gmail.com>
Kevin Traynor [Sun, 12 Nov 2017 11:47:18 +0000 (11:47 +0000)]
dpif-netdev: Remove unnecessary resets on new rxqs.
Commit 38259bd7eb21 (dpif-netdev: Initialize new rxqs in
port_reconfigure().) added a memset for the dp_netdev_rxq of new rxq's
to remove a valgrind warning for an index field in that struct. With
the addition of that memset, it also means there are some existing
resets on other fields in that struct that are no longer needed and
gives the opportunity to simplify by removing them.
Signed-off-by: Kevin Traynor <ktraynor@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Thu, 9 Nov 2017 12:34:08 +0000 (15:34 +0300)]
netdev: Remove EOPNOTSUPP related comment for netdev_send().
Since 57eebbb4c315, the caller must make sure that 'netdev' supports
sending. This mentioned at the start of the comment.
Fixes: 57eebbb4c315 ("dpif-netdev: Don't try to output on a device without txqs.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Jan Scheurich [Mon, 6 Nov 2017 23:40:21 +0000 (00:40 +0100)]
NSH: Adjust NSH wire format to the latest IETF draft
This commit adjusts the NSH user space implementation in OVS to
the latest wire format defined in draft-ietf-sfc-nsh-28 (November 3
2017). The NSH_MDTYPE field was reduced from 8 to 4 bits. The FLAGS
field is reduced from 8 to 2 bits. A new 6 bit TTL header field is
added. The TTL field is set to 63 at encap(nsh).
Match and set_field support for the newly introduced TTL header field
and a corresponding dec_nsh_ttl action is not yet included and will be
implemented in a future patch.
Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Jan Scheurich [Tue, 7 Nov 2017 14:48:39 +0000 (15:48 +0100)]
NSH: Minor bugfixes
- Fix 2 incorrect length checks
- Remove unnecessary limit of MD length to 16 bytes
- Remove incorrect comments stating MD2 was not supported
- Pad metadata in encap_nsh with zeroes if not multiple of 4 bytes
Signed-off-by: Jan Scheurich <jan.scheurich@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
rhel: Add support for "systemctl reload openvswitch"
The reload procedure will trigger a script that saves the flows and tlv
maps (using ovs-save) then it restarts ovsdb-server, it stops ovs-vswitchd,
it sets other_config:flow-restore-wait=true (to wait till flow restore is
finished), it starts ovs-vswitchd, it restore the backupped flows/tlv
maps and it removes other_config:flow-restore-wait=true (logic mostly ripped
from ovs-ctl).
It uses systemctl with --job-mode=ignore-dependencies to restart ovsdb-server
and stop and start ovs-vswitchd in order to avoid systemd to restart the other
components due to dependencies (as explained in rhel/README.RHEL.rst).
László Sürü [Wed, 8 Nov 2017 10:20:20 +0000 (10:20 +0000)]
Documentation: Fix for userspace testsuite directory name
Open vSwitch Testing documentation Userspace datapath section shows incorrect directory name for test result.
Morever to this check-system-userspace test fails if another OVS instance is running.
This patch corrects the directory name and adds a note for other running instances.
Signed-off-by: László Sürü <laszlo.suru@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
In ovs-lib there is a function named upgrade_db which tries to convert a
database after OVS {up,down}grades. This function uses ovsdb-tool to
check if the DB needs to be upgraded. If the upgrade fails,
it purges the DB and create an empty one.
ovsdb-tool returns "yes" or "no" to indicate if the DB needs upgrading,
but if the DB is corrupted it returns a list of errors.
Change a condition from "!= no" to "= yes" because in case of DB
corruption upgrade_db would purge the existing DB without writing
anything in the logs.
Signed-off-by: Matteo Croce <mcroce@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
This commit introduces below changes to netdev_dpdk structure.
- Mark cachelines and reorder few member variables.
- Maintain the grouping of related member variables.
- Add comment on the information on pad bytes where ever appropriate, so
new members can be introduced in the future to fill the gaps.
Below is how this structure looks with this commit.
This commit introduces below changes to dp_netdev_pmd_thread
structure.
- Mark cachelines and in this process reorder few members to avoid
holes.
- Align emc_cache to a cacheline.
- Maintain the grouping of related member variables.
- Add comment on the information on pad bytes whereever appropriate so
that new member variables may be introduced to fill the holes in future.
Below is how the structure looks with this commit.
cmap: Use PADDED_MEMBERS_CACHELINE_MARKER in cmap_impl.
Instead of explicitly adding the pad bytes to force the structure an
exact multiple of cacheline size, let the macro do the job. This way
the pad bytes will be auto adjusted when the new members get introduced
in to the structure.
Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Fri, 14 Jul 2017 19:51:43 +0000 (12:51 -0700)]
ofp-util: Update OpenFlow 1.6 port support to track latest proposal.
The latest updates to the OpenFlow 1.6 proposal removes the hw_addr_type
fields from ofp_port and ofp_port_mod. This commit updates the OVS
prototype to match the updated proposal.
ONF-JIRA: EXT-566 Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Yi-Hung Wei <yihung.wei@gmail.com>
Joe Stringer [Wed, 6 Sep 2017 22:12:52 +0000 (15:12 -0700)]
ofproto-dpif-upcall: Transition ukey on dp_ops error.
In most situations, we don't expect that a flow we've successfully
dumped, which we intend to delete, cannot be deleted. However, to make
this code more resilient to ensure that ukeys *will* transition in all
cases (including an error at this stage), grab the lock and transition
this ukey forward to the evicted state, effectively treating a failure
to delete as "this flow is already gone".
If we subsequently find out that it wasn't deleted, then that's ok - we
will re-dump, and validate at that stage, which should lead to creating
a new ukey or deleting the datapath flow when that happens.
Signed-off-by: Joe Stringer <joe@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
William Tu [Thu, 2 Nov 2017 19:01:17 +0000 (12:01 -0700)]
dpif-netlink-rtnl: Fix ovs_geneve probing after restart.
When using the out-of-tree (openvswitch compat) geneve module,
the first time oot tunnel probing returns true (correct).
Without unloading the geneve module, if the userspace ovs-vswitchd
restarts, because the 'geneve_sys_6081' still exists, the probing
incorrectly returns false and loads the in-tree (upstream kernel)
geneve module.
The patch fixes it by querying the geneve device's kind when exists.
The out-of-tree modules uses kind string as 'ovs_geneve', while the
in-tree module uses 'geneve'. To reproduce the issue, start the ovs
> /etc/init.d/openvswitch-switch start
> creat a bridge and attach a geneve port using out-of-tree geneve
> /etc/init.d/openvswitch-switch restart
Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides used interface") Signed-off-by: William Tu <u9012063@gmail.com> Acked-by: Eric Garver <e@erig.me> Signed-off-by: Gurucharan Shetty <guru@ovn.org>
Yifeng Sun [Tue, 31 Oct 2017 17:52:10 +0000 (10:52 -0700)]
ovsdb-server: Fix memory leak
Valgrind testcase 2349 (ovn -- DSCP marking check) reports the leak below:
21 bytes in 21 blocks are definitely lost in loss record 24 of 362
at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x436FD4: xmalloc (util.c:120)
by 0x437044: xmemdup0 (util.c:150)
by 0x408C97: add_manager_options (ovsdb-server.c:709)
by 0x408C97: query_db_remotes (ovsdb-server.c:765)
by 0x408C97: reconfigure_remotes (ovsdb-server.c:926)
by 0x406273: main_loop (ovsdb-server.c:194)
by 0x406273: main (ovsdb-server.c:434)
When options are freed, options->role need to be freed explicitly.
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Tue, 31 Oct 2017 17:52:09 +0000 (10:52 -0700)]
test-ovsdb: Fix memory leak
Valgrind testcase 1465 (integer atom enum from string) reports the leak below:
16 bytes in 1 blocks are definitely lost in loss record 2 of 5
at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x43F5F4: xmalloc (util.c:120)
by 0x424AC6: alloc_default_atoms (ovsdb-data.c:315)
by 0x4271E0: ovsdb_atom_from_string__ (ovsdb-data.c:508)
by 0x4271E0: ovsdb_atom_from_string (ovsdb-data.c:632)
by 0x40ADCC: do_parse_atom_strings (test-ovsdb.c:566)
by 0x41BA73: ovs_cmdl_run_command__ (command-line.c:115)
by 0x4051C9: main (test-ovsdb.c:72)
range_end_atom is allocated in ovsdb_atom_from_string__() and no one is
holding a reference to it at the end of do_parse_atom_strings(). It should
be freed here, as also pointed out by ovsdb_atom_destroy().
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Tue, 31 Oct 2017 17:52:08 +0000 (10:52 -0700)]
ovsdb-idl: Fix memory leak
Valgrind testcase 2339 (ovn -- ipam connectivity) reports the leak below:
45 (32 direct, 13 indirect) bytes in 1 blocks are definitely lost in loss record 65 of 83
at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4A6D64: xmalloc (util.c:120)
by 0x49C847: shash_add_nocopy__ (shash.c:109)
by 0x49C847: shash_add_nocopy (shash.c:121)
by 0x49CA85: shash_add (shash.c:129)
by 0x49CA85: shash_add_once (shash.c:136)
by 0x4914B5: ovsdb_idl_create_index (ovsdb-idl.c:2067)
by 0x406C98: create_ovnsb_indexes (ovn-controller.c:568)
by 0x406C98: main (ovn-controller.c:619)
The leak happens when vsdb_idl_table is freed but its indexes are not freed.
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yifeng Sun [Thu, 2 Nov 2017 13:42:22 +0000 (06:42 -0700)]
netdev-dummy: Avoid double-free in netdev_dummy_ip4addr().
netdev_dummy_ip6addr() calls netdev_close() twice though it increases
netdev's reference only once from netdev_from_name(). As a result, Valgrind
test 788 (tunnel_push_pop - action) reports the error below:
==20465== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Invalid read of size 8
at 0x493FE0: netdev_get_name (netdev.c:911)
by 0x5125D3: tnl_port_map_delete_ipdev (tnl-ports.c:470)
by 0x4E551C: __rt_entry_delete (ovs-router.c:252)
by 0x4E64AA: ovs_router_flush (ovs-router.c:478)
by 0x475CA8: call_hooks.part.2 (fatal-signal.c:254)
by 0x5E53FF7: __run_exit_handlers (exit.c:82)
by 0x5E54044: exit (exit.c:104)
by 0x5E3A836: (below main) (libc-start.c:325)
Address 0x65ea680 is 0 bytes inside a block of size 640 free'd
at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x492BA2: netdev_unref (netdev.c:572)
by 0x41646E: ofport_destroy__ (ofproto.c:2516)
by 0x41FD58: ofproto_destroy (ofproto.c:1645)
by 0x40B96B: bridge_destroy (bridge.c:3273)
by 0x410238: bridge_exit (bridge.c:506)
by 0x40700E: main (ovs-vswitchd.c:135)
Block was alloc'd at
at 0x4C2FB55: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x516A82: xcalloc (util.c:103)
by 0x48D74D: netdev_dummy_alloc (netdev-dummy.c:661)
by 0x4931D1: netdev_open.part.12 (netdev.c:406)
by 0x40A985: iface_do_create (bridge.c:1784)
by 0x40A985: iface_create (bridge.c:1837)
by 0x40A985: bridge_add_ports__ (bridge.c:931)
by 0x40C7EA: bridge_add_ports (bridge.c:947)
by 0x40C7EA: bridge_reconfigure (bridge.c:663)
by 0x410485: bridge_run (bridge.c:2998)
by 0x406F64: main (ovs-vswitchd.c:119)
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Thu, 2 Nov 2017 00:50:02 +0000 (06:20 +0530)]
ovn: Generate Neighbor Solicitation packet for unknown MAC IPv6 packets
In the router ingress pipeline, if the destination mac is unresolved
by the time the packet reaches the ARP_REQUEST stage, OVN should generate an
IPv6 Neighbor Solicitation packet to learn the MAC address. This feature is
presently missing. This patch adds this feature. A new action "nd_ns" is
added which replaces an IPv6 packet being processed with an IPv6 Neighbor
Solicitation packet. ovn-northd adds a flow in the ARP_REQUEST router ingress
pipeline stage if the eth.dst is zero which applies this action. This action is
similar to the IPv4 counterpart "arp" action.
OVN already has the support to learn the MAC from the IPv6 Neighbor Advertisement
packets and storing in the south bound MAC_Binding table.
Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Zongkai LI [Thu, 2 Nov 2017 00:49:45 +0000 (06:19 +0530)]
ovn-northd: Add logical flows to support native IPv6 RA
This patch adds logical flows which sends IPv6 Router Advertisement
packet in response to the IPv6 Router Solicitation request. It uses
the actions "put_nd_ra_opts" to transform the RS packet to RA packet
in the newly added ingress stage "lr_in_nd_ra_options" in router
pipeline. If the action "put_nd_ra_opts" is successful, it sends the
RA packet back to the originating port in the next ingress stage
"lr_in_nd_ra_response".
A new column "ipv6_ra_configs" is added in the Logical_Router_Port
table, which the CMS is expected to configure IPv6 RA
configurations - "address_mode" and "mtu" for adding these flows.
Co-authored-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Zongkai LI <zealokii@gmail.com> Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Acked-by: Miguel Angel Ajo <majopela@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Thu, 2 Nov 2017 00:49:24 +0000 (06:19 +0530)]
ovn-controller: Add a new action - 'put_nd_ra_opts'
This patch adds a new OVN action 'put_nd_ra_opts' to support native
IPv6 Router Advertisement in OVN. This action can be used to respond
to the IPv6 Router Solicitation requests.
ovn-controller parses this action and adds a NXT_PACKET_IN2 OF flow
with 'pause' flag set and the RA options stored in 'userdata' field.
This action is similar to 'put_dhcp_opts' and 'put_dhcpv6_opts'.
When a valid IPv6 RS packet is received by the pinctrl module of
ovn-controller, it frames a new RA packet and sets the RA options
from the 'userdata' field and resumes the packet storing 1 in the
1-bit result sub-field. If the packet is invalid, it resumes the
packet without any modifications storing 0 in the 1-bit result
sub-field.
Numan Siddique [Thu, 2 Nov 2017 00:48:34 +0000 (06:18 +0530)]
ovn util: Refactor dhcp_opts_map to make it generic
Renamed 'struct dhcp_opts_map' to 'struct gen_opts_map' and
renamed ovn-dhcp.h to ovn-l7.h. An upcoming commit to support IPv6
Router Advertisement, will make use of the refactored code to store
the IPv6 ND RA options in 'struct gen_opts_map'.
Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Acked-by: Miguel Angel Ajo <majopela@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Yi-Hung Wei [Wed, 1 Nov 2017 21:40:27 +0000 (14:40 -0700)]
packets: Fix C++ compilation issues when include packets.h
This patch fixes three C++ compilation errors when it includes
"lib/packets.h".
1) Fix in "include/openvswitch/util.h" is to avoid duplicated
named_member__ in struct pkt_metadata.
2) Fix in "lib/packets.h" is because designated initializers are not
implemented in GNU C++ [1].
3) Fix in "lib/util.h" is because __builtin_types_compatible_p and
__builtin_choose_expr are only supported in GCC. I use one solution
for C++ that is type-safe and works at compile time from [2].
Ben Pfaff [Fri, 27 Oct 2017 15:40:23 +0000 (08:40 -0700)]
ofproto-dpif-upcall: Fix null pointer dereference on exit.
When revalidation occurs at the same time that a bridge is being removed
or ovs-vswitchd is exiting, xlate_lookup_ofproto() races with deletion of
the ofproto. This caused a null pointer dereference if revalidation lost
the race. This commit fixes the problem.
Reported-by: Jakub Sitnicki <jkbs@redhat.com> Tested-by: Jakub Sitnicki <jkbs@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>