Shashank Ram [Mon, 20 Nov 2017 23:06:14 +0000 (15:06 -0800)]
datapath-windows: Account for VLAN tag in tunnel Decap
Decap functions for tunneling protocols do not compute
the packet header offsets correctly when there is a VLAN
tag in the L2 header. This results in incorrect checksum
computation causing the packet to be dropped.
This patch adds support to account for the VLAN tag in the
packet if its present, and makes use of the OvsExtractLayers()
function to correctly compute the header offsets for different
layers.
Testing done:
- Tested Geneve, STT, Vxlan and Gre and verified that there
are no regressions.
- Verified that packets with VLAN tags are correctly handled
in the decap code of all tunneling protocols. Previously,
this would result in packet drops due to invalid checksums
being computed.
- Verified that non-VLAN tagged packets are handled correctly.
Ben Pfaff [Mon, 27 Nov 2017 01:34:59 +0000 (17:34 -0800)]
odp-util: Fix buffer overread in parsing string form of ODP flows.
scan_u128() should return 0 on an error but it actually returned an errno
value in some cases, so a command like this:
ovs-appctl dpctl/add-flow 'ct_label(1/55555555555555555555555555)' ''
could cause a buffer overread.
This bug is not as severe as it may sound because the string form of ODP
flows is not used over OpenFlow or OVSDB, only through the appctl interface
that is normally used just by local system administrators and not exposed
over a network.
Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Joe Stringer <joe@ovn.org>
Ben Pfaff [Mon, 27 Nov 2017 00:26:41 +0000 (16:26 -0800)]
tc: Fix build breakage on GCC 7 by annotating fall-through.
Open vSwitch enables the GCC 7+ option that warns about fall-through
switch statements. This commit fixes newly introduced warnings.
Fixes: d6118e628988 ("netdev-tc-offloads: Verify csum flags on dump from tc") Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Paul Blakey <paulb@mellanox.com>
Numan Siddique [Wed, 8 Nov 2017 08:59:07 +0000 (14:29 +0530)]
OpenvSwitch logrotate: Use ctl file path as target in ovs-appctl to reset logs
Presently, logrotate script, searches for the pid files in /var/log/openvswitch
and passes the pid file name (without .pid) as target to ovs-appctl. This approach
doesn't work for OVN DB servers since the ctl files are generated as "ovnnb_db.ctl"
and "ovnsb_db.ctl". So search for the .ctl files instead and use them as target to
ovs-appctl.
Suggested-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Mark Michelson <mmichels@redhat.com>
Numan Siddique [Wed, 8 Nov 2017 08:58:49 +0000 (14:28 +0530)]
ovn-ctl: Add -vfile:info option to OVN_NB/SB_LOG options
In the RHEL environment, when OVN db servers are started using ovn-ctl,
log files are empty. Adding "-vfile:info" option to ovsdb-server is
resolving this issue. Running 'ovs-apptctl -t .. vlog/reopen" results in the
logs appearing in the log files. This issue is seen with 2.7.2.
"-vfile:info" option is passed to ovn-northd and ovn-controller when starting.
There is no harm in adding this to OVN db servers.
Signed-off-by: Numan Siddique <nusiddiq@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
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>
projects / mirror_ovs.git / log