Eli Britstein [Tue, 14 Jan 2020 13:21:15 +0000 (13:21 +0000)]
flow: Fix parsing l3_ofs with partial offloading
l3_ofs should be set all Ethernet packets, not just IPv4/IPv6 ones.
For example for ARP over VLAN tagged packets, it may cause wrong
processing like in changing the VLAN ID action. Fix it.
Fixes: aab96ec4d81e ("dpif-netdev: retrieve flow directly from the flow mark") Signed-off-by: Eli Britstein <elibr@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
William Tu [Thu, 23 Jan 2020 17:03:11 +0000 (09:03 -0800)]
docs: Add header install command for afxdp.
The 'XDP_RING_NEED_WAKEUP' and related flags are defined if_xdp.h, so if
users are building their own kernel, users have to update the kernel's
header files, by doing:
$ make headers_install INSTALL_HDR_PATH=/usr
Otherwise the following error shows:
/usr/local/include/bpf/xsk.h: In function 'xsk_ring_prod__needs_wakeup':
/usr/local/include/bpf/xsk.h:82:21: error: 'XDP_RING_NEED_WAKEUP' undeclared \
(first use in this function)
return *r->flags & XDP_RING_NEED_WAKEUP;
Reported-by: Tomek Osinski <osinstom@gmail.com>
Reported-at: https://osinstom.github.io/en/tutorial/ovs-afxdp-installation/ Signed-off-by: William Tu <u9012063@gmail.com> Acked-by: Ben Pfaff <blp@ovn.org> Signed-off-by: William Tu <u9012063@gmail.com>
Yi-Hung Wei [Fri, 20 Dec 2019 17:51:08 +0000 (09:51 -0800)]
conntrack: Fix conntrack new state
In connection tracking system, a connection is established if we
see packets from both directions. However, in userspace datapath's
conntrack, if we send a connection setup packet in one direction
twice, it will make the connection to be in established state.
This patch fixes the aforementioned issue, and adds a system traffic
test for UDP and TCP traffic to avoid regression.
Fixes: a489b16854b59 ("conntrack: New userspace connection tracker.") Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: William Tu <u9012063@gmail.com>
Ilya Maximets [Thu, 23 Jan 2020 18:10:05 +0000 (19:10 +0100)]
dpif: Fix dp_extra_info leak by reworking the allocation scheme.
dpctl module leaks the 'dp_extra_info' in case the dumped flow doesn't
fit the dump filter while executing dpctl/dump-flows and also while
executing dpctl/get-flow.
This is already a 3rd attempt to fix all the leaks and incorrect usage
of this string that definitely indicates poor initial design of the
feature.
Flow dump/get documentation clearly states that the caller does not own
the data provided in dpif_flow. Datapath still owns all the data and
promises to not free/modify it until the next quiescent period, however
we're requesting the caller to free 'dp_extra_info' and this obviously
breaks the rules.
This patch fixes the issue by by storing 'dp_extra_info' within
'struct dp_netdev_flow' making datapath to own it. 'dp_netdev_flow'
is RCU-protected, so it will be valid until the next quiescent period.
Fixes: 0e8f5c6a38d0 ("dpif-netdev: Modified ovs-appctl dpctl/dump-flows command") Tested-by: Emma Finn <emma.finn@intel.com> Acked-by: Emma Finn <emma.finn@intel.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ben Pfaff [Thu, 9 Jan 2020 20:49:44 +0000 (12:49 -0800)]
ofproto: Do not delete datapath flows on exit by default.
Commit e96a5c24e853 ("upcall: Remove datapath flows when setting
n-threads.") caused OVS to delete datapath flows when it exits through
any graceful means. This is not necessarily desirable, especially when
OVS is being stopped as part of an upgrade. This commit changes OVS so
that it only removes datapath flows when requested, via "ovs-appctl
exit --cleanup".
Ben Pfaff [Thu, 9 Jan 2020 20:49:43 +0000 (12:49 -0800)]
ofproto-dpif-upcall: Get rid of udpif_synchronize().
RCU provides the semantics we want from udpif_synchronize() and it
should be much more lightweight than killing and restarting all the
upcall threads. It looks like udpif_synchronize() was written before
the OVS tree had RCU support, which is probably why we didn't use it
here from the beginning. So we can just change udpif_synchronize()
to a single ovsrcu_synchronize() call.
However, udpif_synchronize() only has a single caller, which calls
ovsrcu_synchronize() anyway just beforehand, via xlate_txn_commit().
So we can get rid of udpif_synchronize() entirely, which this patch
does.
As a side effect, this eliminates one reason why terminating OVS cleanly
clears the datapath flow table. An upcoming patch will eliminate
other reasons.
Acked-by: Numan Siddique <numans@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ning Wu [Wed, 22 Jan 2020 07:46:58 +0000 (23:46 -0800)]
lib/stream-windows.c: Grant Access Privilege of Named Pipe to Creator
Current implementation of ovs on windows only allows LocalSystem and
Administrators to access the named pipe created with API of ovs.
Thus any service that needs to invoke the API to create named pipe
has to run as System account to interactive with ovs. It causes the
system more vulnerable if one of those services was break into.
The patch adds the creator owner account to allowed ACLs.
Signed-off-by: Ning Wu <nwu@vmware.com> Acked-by: Alin Gabriel Serdean <aserdean@ovn.org> Acked-by: Anand Kumar <kumaranand@vmware.com> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
John Hurley [Thu, 19 Dec 2019 14:58:43 +0000 (14:58 +0000)]
tc: handle packet mark of zero
Openstack may set an skb mark of 0 in tunnel rules. This is considered to
be an unused/unset value. However, it prevents the rule from being
offloaded.
Check if the key value of the skb mark is 0 when it is in use (mask is
set to all ones). If it is then ignore the field and continue with TC offload.
Only the exact-match case is covered by this patch as it addresses the
Openstack use-case and seems most robust against feature evolution: f.e. in
future there may exist hardware offload scenarios where an operation, such
as a BPF offload, sets the SKB mark before proceeding tho the in-HW OVS.
datapath.
Signed-off-by: John Hurley <john.hurley@netronome.com> Co-Authored-by: Simon Horman <simon.horman@netronome.com> Signed-off-by: Simon Horman <simon.horman@netronome.com> Acked-by: Aaron Conole <aconole@redhat.com>
Ciara Loftus [Mon, 20 Jan 2020 13:09:36 +0000 (13:09 +0000)]
Documentation: add notes for TSO & i40e
When using TSO in OVS-DPDK with an i40e device, the following
patch is required for DPDK, which fixes an issue on the TSO path:
https://patches.dpdk.org/patch/64136/
Document this as a limitation until a DPDK release with the fix
included is supported by OVS.
Also, document best known methods for performance tuning when
testing TSO with the tool iperf.
Ilya Maximets [Fri, 17 Jan 2020 22:00:05 +0000 (23:00 +0100)]
dpif: Fix leak and usage of uninitialized dp_extra_info.
'dpif_probe_feature'/'revalidate' doesn't free the 'dp_extra_info'
string. Also, all the implementations of dpif_flow_get() should
initialize the value to avoid printing/freeing of random memory.
30 bytes in 1 blocks are definitely lost in loss record 323 of 889
at 0x483AD19: realloc (vg_replace_malloc.c:836)
by 0xDDAD89: xrealloc (util.c:149)
by 0xCE1609: ds_reserve (dynamic-string.c:63)
by 0xCE1A90: ds_put_format_valist (dynamic-string.c:161)
by 0xCE19B9: ds_put_format (dynamic-string.c:142)
by 0xCCCEA9: dp_netdev_flow_to_dpif_flow (dpif-netdev.c:3170)
by 0xCCD2DD: dpif_netdev_flow_get (dpif-netdev.c:3278)
by 0xCCEA0A: dpif_netdev_operate (dpif-netdev.c:3868)
by 0xCDF81B: dpif_operate (dpif.c:1361)
by 0xCDEE93: dpif_flow_get (dpif.c:1002)
by 0xCDECF9: dpif_probe_feature (dpif.c:962)
by 0xC635D2: check_recirc (ofproto-dpif.c:896)
by 0xC65C02: check_support (ofproto-dpif.c:1567)
by 0xC63274: open_dpif_backer (ofproto-dpif.c:818)
by 0xC65E3E: construct (ofproto-dpif.c:1605)
by 0xC4D436: ofproto_create (ofproto.c:549)
by 0xC3931A: bridge_reconfigure (bridge.c:877)
by 0xC3FEAC: bridge_run (bridge.c:3324)
by 0xC4551D: main (ovs-vswitchd.c:127)
CC: Emma Finn <emma.finn@intel.com> Fixes: 0e8f5c6a38d0 ("dpif-netdev: Modified ovs-appctl dpctl/dump-flows command") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Roi Dayan <roid@mellanox.com>
Yi-Hung Wei [Sat, 4 Jan 2020 01:13:26 +0000 (17:13 -0800)]
netdev-afxdp: NUMA-aware memory allocation for XSK related memory.
Currently, the AF_XDP socket (XSK) related memory are allocated by main
thread in the main thread's NUMA domain. With the patch that detects
netdev-linux's NUMA node id, the PMD thread of AF_XDP port will be run on
the AF_XDP netdev's NUMA domain. If the net device's NUMA domain
is different from the main thread's NUMA domain, we will have two
cross-NUMA memory accesses (netdev <-> memory, memory <-> CPU).
This patch addresses the aforementioned issue by allocating
the memory in the net device's NUMA domain.
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> Acked-by: William Tu <u9012063@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
William Tu [Sat, 4 Jan 2020 01:13:25 +0000 (17:13 -0800)]
netdev-linux: Detect numa node id.
The patch detects the numa node id from the name of the netdev,
by reading the '/sys/class/net/<devname>/device/numa_node'.
If not available, ex: virtual device, or any error happens,
return numa id 0. Currently only the afxdp netdev type uses it,
other linux netdev types are disabled due to no use case.
Signed-off-by: William Tu <u9012063@gmail.com> Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Flavio Leitner [Fri, 17 Jan 2020 21:47:55 +0000 (18:47 -0300)]
userspace: Add TCP Segmentation Offload support
Abbreviated as TSO, TCP Segmentation Offload is a feature which enables
the network stack to delegate the TCP segmentation to the NIC reducing
the per packet CPU overhead.
A guest using vhostuser interface with TSO enabled can send TCP packets
much bigger than the MTU, which saves CPU cycles normally used to break
the packets down to MTU size and to calculate checksums.
It also saves CPU cycles used to parse multiple packets/headers during
the packet processing inside virtual switch.
If the destination of the packet is another guest in the same host, then
the same big packet can be sent through a vhostuser interface skipping
the segmentation completely. However, if the destination is not local,
the NIC hardware is instructed to do the TCP segmentation and checksum
calculation.
It is recommended to check if NIC hardware supports TSO before enabling
the feature, which is off by default. For additional information please
check the tso.rst document.
Timothy Redaelli [Thu, 16 Jan 2020 13:21:47 +0000 (14:21 +0100)]
Documentation: Fix building with Python 3.9
open(), io.open(), codecs.open() and fileinput.FileInput no longer accept 'U'
("universal newline") in the file mode.
This flag was deprecated since Python 3.3.
In Python 3, the "universal newline" is used by default when a file is open
in text mode.
Ophir Munk [Thu, 9 Jan 2020 07:46:47 +0000 (07:46 +0000)]
dpif-netdev: Update offloaded flows statistics.
In case a flow is HW offloaded, packets do not reach the SW, thus not
counted for statistics. Use netdev flow get API in order to update the
statistics of flows by the HW statistics.
Co-authored-by: Eli Britstein <elibr@mellanox.com> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> Reviewed-by: Oz Shlomo <ozsh@mellanox.com> Signed-off-by: Eli Britstein <elibr@mellanox.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Eli Britstein [Thu, 9 Jan 2020 07:46:46 +0000 (07:46 +0000)]
dpctl: Support dump-flows filters "dpdk" and "partially-offloaded".
Flows that are offloaded via DPDK can be partially offloaded (matches
only) or fully offloaded (matches and actions). Set partially offloaded
display to (offloaded=partial, dp:ovs), and fully offloaded to
(offloaded=yes, dp:dpdk). Also support filter types "dpdk" and
"partially-offloaded".
Signed-off-by: Eli Britstein <elibr@mellanox.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Eli Britstein [Thu, 9 Jan 2020 07:46:44 +0000 (07:46 +0000)]
netdev-offload-dpdk: Framework for actions offload.
Currently HW offload is accelerating only the rule matching sequence.
Introduce a framework for offloading rule actions as a pre-step for
processing the rule actions in HW. In case of a failure, fallback to the
legacy partial offload scheme.
Note: a flow will be fully offloaded only if it can process all its
actions in HW.
Signed-off-by: Eli Britstein <elibr@mellanox.com> Reviewed-by: Oz Shlomo <ozsh@mellanox.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Eli Britstein [Thu, 9 Jan 2020 07:46:43 +0000 (07:46 +0000)]
netdev-offload-dpdk: Return UFID-rte_flow entry in find method.
Change the find method to return the whole entry of UFID-rte_flow
association instead of only the rte_flow field in it, as a pre-step
towards adding and using more fields into that map entry.
Signed-off-by: Eli Britstein <elibr@mellanox.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Instead of statically allocated pattern items on the stack, dynamically
allocate only the required items while parsing the matches, to simplify
the parsing and make it self-contained, without need of external types,
making it easier to support more matches in the future.
Signed-off-by: Eli Britstein <elibr@mellanox.com> Reviewed-by: Oz Shlomo <ozsh@mellanox.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Action item data structures are pointed by rte_flow_action items.
Refactor the code to free the data structures when freeing the
rte_flow_action items, allowing simpler future actions simpler to add to
the code.
Signed-off-by: Eli Britstein <elibr@mellanox.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Ilya Maximets [Wed, 15 Jan 2020 17:11:51 +0000 (18:11 +0100)]
dpif-netdev: Make datapath port mutex recursive.
Upcoming HW offloading will request flow statistics from the dpdk
offloading module. This operation requires holding datapath port
mutex. However, there is a possible scenario in which flow deletion
happens during datapath reconfiguration process and the mutex already
acquired:
This happens while removing the last port of a particular PMD thread.
Reconfiguration process decides that we need to remove current PMD
thread and calls datapath purge callback in order to clean up resources
assigned to it. This turns into flow removal and flow_del() tries to
request statistics.
Turning the dp->port_mutex into recursive version as a quick fix for
this issue. Better solutions might be to avoid statistics request
somehow, or fully disassociate offloaded flows from the datapath flows.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Ian Stokes <ian.stokes@intel.com>
Adds `mailto:` to email address links in mailing-lists.rst. The
existing syntax resulted in broken links of the form
`http://docs.openvswitch.org/en/latest/internals/mailing-lists/<address>`,
which would result in a 404 error.
Signed-off-by: Lars Kellogg-Stedman <lars@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Eelco Chaudron [Tue, 14 Jan 2020 16:12:47 +0000 (11:12 -0500)]
netdev-dpdk: Add new DPDK RFC 4115 egress policer
This patch adds a new policer to the DPDK datapath based on RFC 4115's
Two-Rate, Three-Color marker. It's a two-level hierarchical policer
which first does a color-blind marking of the traffic at the queue
level, followed by a color-aware marking at the port level. At the end
traffic marked as Green or Yellow is forwarded, Red is dropped. For
details on how traffic is marked, see RFC 4115.
This egress policer can be used to limit traffic at different rated
based on the queues the traffic is in. In addition, it can also be used
to prioritize certain traffic over others at a port level.
For example, the following configuration will limit the traffic rate at a
port level to a maximum of 2000 packets a second (64 bytes IPv4 packets).
100pps as CIR (Committed Information Rate) and 1000pps as EIR (Excess
Information Rate). High priority traffic is routed to queue 10, which marks
all traffic as CIR, i.e. Green. All low priority traffic, queue 20, is
marked as EIR, i.e. Yellow.
This configuration accomplishes that the high priority traffic has a
guaranteed bandwidth egressing the ports at CIR (1000pps), but it can also
use the EIR, so a total of 2000pps at max. These additional 1000pps is
shared with the low priority traffic. The low priority traffic can use at
maximum 1000pps.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Eelco Chaudron [Tue, 14 Jan 2020 16:12:40 +0000 (11:12 -0500)]
netdev-dpdk: Add support for multi-queue QoS to the DPDK datapath
This patch adds support for multi-queue QoS to the DPDK datapath. Most of
the code is based on an earlier patch from a patchset sent out by
zhaozhanxu. The patch was titled "[ovs-dev, v2, 1/4] netdev-dpdk.c: Support
the multi-queue QoS configuration for dpdk datapath"
Ilya Maximets [Tue, 14 Jan 2020 20:18:14 +0000 (21:18 +0100)]
netdev-offload-tc: Fix crash if offloading is not configured on outdev.
If output device is not yet added to netdev-offload, netdev_ports_get()
will not find it leading to NULL pointer dereference inside
netdev_get_ifindex().
Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using tc interface") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ben Pfaff [Mon, 13 Jan 2020 18:03:04 +0000 (10:03 -0800)]
socket-util: Emulate recvmmsg() and sendmmsg() on Linux only.
These functions failed to build on OS X because MSG_WAITFORONE is not
defined there. There are pitfalls for trying to define our own MSG_*
constants, since it's hard to pick a constant that is not used by the
system already. Because OVS only uses recvmmsg() and sendmmsg() on
Linux, it seems easiest to just emulate them there.
Ilya Maximets [Thu, 9 Jan 2020 15:59:15 +0000 (16:59 +0100)]
odp-util: Fix passing uninitialized bytes in OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV*.
Both ovs_key_ct_tuple_ipv* structures contains padding at the end
that mast be cleared before passing attributes to kernel:
Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
at 0x566A607: sendmsg (sendmsg.c:28)
by 0xFC95CE: nl_sock_transact_multiple__ (netlink-socket.c:858)
by 0xFC8580: nl_sock_transact_multiple (netlink-socket.c:1079)
by 0xFC83FF: nl_transact_multiple (netlink-socket.c:1839)
by 0xFA8648: dpif_netlink_operate__ (dpif-netlink.c:1926)
by 0xFA789F: dpif_netlink_operate_chunks (dpif-netlink.c:2219)
by 0xFA25CB: dpif_netlink_operate (dpif-netlink.c:2278)
by 0xE5BB4C: dpif_operate (dpif.c:1377)
by 0xE5B7F6: dpif_flow_put (dpif.c:1048)
by 0xE5B49A: dpif_probe_feature (dpif.c:965)
by 0xDD6BF5: check_ct_orig_tuple (ofproto-dpif.c:1557)
by 0xDD41EC: check_support (ofproto-dpif.c:1590)
by 0xDD3BF3: open_dpif_backer (ofproto-dpif.c:818)
by 0xDC8467: construct (ofproto-dpif.c:1605)
by 0xDAD6BB: ofproto_create (ofproto.c:549)
by 0xD96A19: bridge_reconfigure (bridge.c:877)
by 0xD9625D: bridge_run (bridge.c:3324)
by 0xDA5829: main (ovs-vswitchd.c:127)
Address 0x1ffefe36a5 is on thread 1's stack
in frame #4, created by dpif_netlink_operate__ (dpif-netlink.c:1839)
Uninitialised value was created by a stack allocation
at 0xEB87D0: odp_flow_key_from_flow__ (odp-util.c:5996)
Fixes: daf4d3c18da4 ("odp: Support conntrack orig tuple key.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Timothy Redaelli [Mon, 13 Jan 2020 16:47:03 +0000 (17:47 +0100)]
ovs-tcpdump: Fallback to read /proc/net/dev on Linux
Currently, ovs-tcpdump uses python3-netifaces in order to get the list of
the network interfaces, but python3-netifaces may not be installed nor
available on some distributions (for example on RHEL7).
This commit adds, only for Linux, an alternative way (that is only used
when netifaces is not available) to read the list of the network interfaces
by reading "/proc/net/dev".
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
In "Use of library functions" in the C standard, the following statement
is written to apply to all library functions:
If an argument to a function has an invalid value (such as ... a
null pointer ... the behavior is undefined.
Later, under the "String handling" section, "Comparison functions" no
exception is listed for strcmp, which means NULL is invalid. It may
be possible for the smap_get to return NULL.
Given the above, we must check that new_devargs is not null. The check
against NULL for new_devargs later in the function is still valid.
Fixes: 55e075e65ef9 ("netdev-dpdk: Arbitrary 'dpdk' port naming") Signed-off-by: Aaron Conole <aconole@redhat.com> Acked-by: Ciara Loftus <ciara.loftus@intel.com> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
Yi Yang [Wed, 18 Dec 2019 02:35:27 +0000 (21:35 -0500)]
Use batch process recv for tap and raw socket in netdev datapath
Current netdev_linux_rxq_recv_tap and netdev_linux_rxq_recv_sock
just receive single packet, that is very inefficient, per my test
case which adds two tap ports or veth ports into OVS bridge
(datapath_type=netdev) and use iperf3 to do performance test
between two ports (they are set into different network name space).
The result is as below:
tap: 295 Mbits/sec
veth: 207 Mbits/sec
After I change netdev_linux_rxq_recv_tap and
netdev_linux_rxq_recv_sock to use batch process, the performance
is boosted by about 7 times, here is the result:
tap: 1.96 Gbits/sec
veth: 1.47 Gbits/sec
Undoubtedly this is a huge improvement although it can't match
OVS kernel datapath yet.
FYI: here is thr result for OVS kernel datapath:
tap: 37.2 Gbits/sec
veth: 36.3 Gbits/sec
Note: performance result is highly related with your test machine,
you shouldn't expect the same results on your test machine.
Signed-off-by: Yi Yang <yangyi01@inspur.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Sun, 8 Dec 2019 17:51:09 +0000 (18:51 +0100)]
dpif-netdev: Get rid of broken dpif pointer in dp_netdev structure.
This pointer was introduced in July 2014 by commit 6b31e07347ad ("dpif-netdev: Polling threads directly call ofproto upcall functions.")
and it was broken right from this point because dpif_netdev_open()
updates it on each call with the pointer to a newly allocated
'dpif' structure that becomes invalid on the next dpif_netdev_close().
Since dpif_open/close() always happens asynchronously from different
threads and pointer is not protected by rcu or mutex (it's not even
atomic) it's not possible to safely use it. Thankfully the actual
usage was in repository for less than 3 weeks and was removed by
commit 623540e4617e ("dpif-netdev: Streamline miss handling."). Until
recently this pointer was used in order to pass it to dpif_flow_hash().
Another luck is that dpif_flow_hash() didn't use the 'dpif' argument.
However, we tried to use it while netdev offloading by commit 30115809da2e ("dpif-netdev: Use netdev-offload API for port lookup while offloading.")
and that unveiled the issue.
Now that all the code that used this pointer was cleaned up we can
just remove it from the structure to avoid possible misuse in the
future.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Sun, 8 Dec 2019 17:09:53 +0000 (18:09 +0100)]
dpif: Turn dpif_flow_hash function into generic odp_flow_key_hash.
Current implementation of dpif_flow_hash() doesn't depend on datapath
interface and only complicates the callers by forcing them to figure
out what is their current 'dpif'. If we'll need different hashing
for different 'dpif's we'll implement an API for dpif-providers
and each dpif implementation will be able to use their local function
directly without calling it via dpif API.
This change will allow us to not store 'dpif' pointer in the userspace
datapath implementation which is broken and will be removed in next
commits.
This patch moves dpif_flow_hash() to odp-util module and replaces
unused odp_flow_key_hash() by it, along with removing of unused 'dpif'
argument.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Numan Siddique [Tue, 7 Jan 2020 04:54:48 +0000 (10:24 +0530)]
ovsdb replication: Provide option to configure probe interval.
When ovsdb-server is in backup mode and connects to the active
ovsdb-server for replication, and if takes more than 5 seconds to
get the dump of the whole database, it will drop the connection
soon after as the default probe interval is 5 seconds. This
results in a snowball effect of reconnections to the active
ovsdb-server.
This patch handles or mitigates this issue by setting the
default probe interval value to 60 seconds and provide the option to
configure this value from the unixctl command.
Other option could be increase the value of 'RECONNECT_DEFAULT_PROBE_INTERVAL'
to a higher value.
Acked-by: Mark Michelson <mmichels@redhat.com> Acked-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Numan Siddique <numans@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Anju Thomas [Wed, 18 Dec 2019 04:48:12 +0000 (05:48 +0100)]
userspace: Improved packet drop statistics.
Currently OVS maintains explicit packet drop/error counters only on port
level. Packets that are dropped as part of normal OpenFlow processing
are counted in flow stats of “drop” flows or as table misses in table
stats. These can only be interpreted by controllers that know the
semantics of the configured OpenFlow pipeline. Without that knowledge,
it is impossible for an OVS user to obtain e.g. the total number of
packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be
dropped by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid
further expensive upcalls to the slow path, but subsequent packets
dropped by the datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.This makes it difficult
for OVS users to monitor packet drop in an OVS instance and to alert a
management system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
Ilya Maximets [Sat, 4 Jan 2020 00:07:36 +0000 (01:07 +0100)]
ofproto-dpif-upcall: Fix using uninitialized upcall hash.
upcalls are allocated on stack and 'hash' field must be initialized
regardless of attribute existence because it will be used later.
Conditional jump or move depends on uninitialised value(s)
at 0xFA74A7: dpif_netlink_encode_execute (dpif-netlink.c:1828)
by 0xFA6DE8: dpif_netlink_operate__ (dpif-netlink.c:1906)
by 0xFA612F: dpif_netlink_operate_chunks (dpif-netlink.c:2219)
by 0xFA0E36: dpif_netlink_operate (dpif-netlink.c:2275)
by 0xE5AFAC: dpif_operate (dpif.c:1376)
by 0xDF3922: handle_upcalls (ofproto-dpif-upcall.c:1615)
by 0xDF269B: recv_upcalls (ofproto-dpif-upcall.c:857)
by 0xDF1C49: udpif_upcall_handler (ofproto-dpif-upcall.c:759)
by 0xF3A3FE: ovsthread_wrapper (ovs-thread.c:383)
by 0x565F6DA: start_thread (pthread_create.c:463)
by 0x615988E: clone (clone.S:95)
Uninitialised value was created by a stack allocation
at 0xDF2258: recv_upcalls (ofproto-dpif-upcall.c:773)
Fixes: 0442bfb11d6c ("ofproto-dpif-upcall: Echo HASH attribute back to datapath.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> Acked-by: William Tu <u9012063@gmail.com>
Ilya Maximets [Fri, 3 Jan 2020 23:32:04 +0000 (00:32 +0100)]
ofproto-dpif: Fix using uninitialized execute hash.
Most of callers doesn't initialize dpif_execute.hash leaving random
value from the stack. And this random value used later while encoding
netlink message and might produce unwanted kernel behavior.
Fix that by fully initializing dpif_execute structure. Using
designated initializers to avoid such issues in the future.
Fixes: 0442bfb11d6c ("ofproto-dpif-upcall: Echo HASH attribute back to datapath.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: William Tu <u9012063@gmail.com> Acked-by: Ben Pfaff <blp@ovn.org>
dpif logging functions expects to be called after the operation.
log_flow_del_message() dumps flow stats on success which are not
initialized before the actual call to netdev_flow_del():
Conditional jump or move depends on uninitialised value(s)
at 0x6090875: _itoa_word (_itoa.c:179)
by 0x6093F0D: vfprintf (vfprintf.c:1642)
by 0x60C090F: vsnprintf (vsnprintf.c:114)
by 0xE5E7EC: ds_put_format_valist (dynamic-string.c:155)
by 0xE5E755: ds_put_format (dynamic-string.c:142)
by 0xE5A5E6: dpif_flow_stats_format (dpif.c:903)
by 0xE5B708: log_flow_message (dpif.c:1763)
by 0xE5BCA4: log_flow_del_message (dpif.c:1809)
by 0xFA6076: try_send_to_netdev (dpif-netlink.c:2190)
by 0xFA0D3C: dpif_netlink_operate (dpif-netlink.c:2248)
by 0xE5AFAC: dpif_operate (dpif.c:1376)
by 0xDF176E: push_dp_ops (ofproto-dpif-upcall.c:2367)
by 0xDF04C8: push_ukey_ops (ofproto-dpif-upcall.c:2447)
by 0xDF008F: revalidator_sweep__ (ofproto-dpif-upcall.c:2805)
by 0xDF5DC6: revalidator_sweep (ofproto-dpif-upcall.c:2816)
by 0xDF1E83: udpif_revalidator (ofproto-dpif-upcall.c:949)
by 0xF3A3FE: ovsthread_wrapper (ovs-thread.c:383)
by 0x565F6DA: start_thread (pthread_create.c:463)
by 0x615988E: clone (clone.S:95)
Uninitialised value was created by a stack allocation
at 0xDEFC24: revalidator_sweep__ (ofproto-dpif-upcall.c:2733)
Fixes: 3cd99886191e ("dpif-netlink: Use dpif logging functions") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Ilya Maximets [Fri, 3 Jan 2020 18:48:18 +0000 (19:48 +0100)]
tc: Fix using uninitialized id chain.
tc_make_tcf_id() doesn't initialize the 'chain' field leaving it with a
random value from the stack. This makes request_from_tcf_id() create
request with random TCA_CHAIN included. These requests are obviously
doesn't work as needed leading to broken flow dump and various other
issues. Fix that by using designated initializer instead.
Fixes: acdd544c4c9a ("tc: Introduce tcf_id to specify a tc filter") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Greg Rose [Mon, 6 Jan 2020 21:36:34 +0000 (13:36 -0800)]
compat: Include confirm_neigh parameter if needed
A change backported to the Linux 4.14.162 LTS kernel requires
a boolean parameter. Check for the presence of the parameter
and adjust the caller in that case.
bridge: Split the column updates of rstp statistics and status.
Split the update of rstp_statistics column and rstp_status column in
Port table into two different functions. This helps in controlling the
number of times the rstp_statistics column is updated with the key
"stats-update_interval" in Open_vSwitch table.
Signed-off-by: Krishna Kolakaluri <kkolakaluri@plume.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
'xsk_generic_xmit ()' sends packets synchronously and no more than 16
packets at a time. This means that we have to kick Tx with sendmsg()
for every 16 packets in simple native mode too, otherwise the packets
may never be sent.
Reported-by: William Tu <u9012063@gmail.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-November/365076.html Fixes: e8f5634484e8 ("netdev-afxdp: Best-effort configuration of XDP mode.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Signed-off-by: William Tu <u9012063@gmail.com>
Paul Blakey [Sun, 22 Dec 2019 10:16:40 +0000 (12:16 +0200)]
netdev-offload-tc: Add recirculation support via tc chains
Each recirculation id will create a tc chain, and we translate
the recirculation action to a tc goto chain action.
We check for kernel support for this by probing OvS Datapath for the
tc recirc id sharing feature. If supported, we can offload rules
that match on recirc_id, and recirculation action safely.
Signed-off-by: Paul Blakey <paulb@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Paul Blakey [Sun, 22 Dec 2019 10:16:39 +0000 (12:16 +0200)]
tc: Move tunnel_key unset action before output ports
Since OvS datapath gets packets already decapsulated from tunnel devices,
it doesn't explicitly decapsulate them. So in a recirculation setup,
the tunnel matching continues in the recirculation as the tunnel
metadata still exists on the SKB.
Tunnel key unset action unsets this metadata. Some drivers might rely
on this explicit tunnel key unset to know when to decapsulate the packet
instead of the device type. So instead of removing it completly,
we move it near the output actions.
This way, we also keep SKB metadata through recirculation, and for
non-recirculation rules, the resulting tc rules should remain the same.
Signed-off-by: Paul Blakey <paulb@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Paul Blakey [Sun, 22 Dec 2019 10:16:38 +0000 (12:16 +0200)]
dpif: Add support to set user features
This enables user features on the kernel datapath via the DP_CMD_SET
command, and also retrieves them to check for actual support and
not just an older kernel ignoring the requested features.
This will be used in next patch to enable recirc_id sharing with tc.
Signed-off-by: Paul Blakey <paulb@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Paul Blakey [Sun, 22 Dec 2019 10:16:37 +0000 (12:16 +0200)]
netdev-offload-tc: Implement netdev tc flush via tc filter del
To be consistent with our tc-ufid mapping after flush, and to support tc
chains flushing in the next commit, implement flush operation via
deleting all the filters we actually added and delete their mappings.
This will also not delete the configured qos policing via matchall filters,
while old code did.
Signed-off-by: Paul Blakey <paulb@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Han Zhou [Wed, 4 Dec 2019 01:57:20 +0000 (17:57 -0800)]
ovsdb raft: Fix the problem when cluster restarted after DB compaction.
Cluster doesn't work after all nodes restarted after DB compaction,
unless there is any transaction after DB compaction before the restart.
Error log is like:
raft|ERR|internal error: deferred vote_request message completed but not ready
to send because message index 9 is past last synced index 0: s2 vote_request:
term=6 last_log_index=9 last_log_term=4
The root cause is that the log_synced member is not initialized when
reading the raft header. This patch fixes it and remove the XXX
from the test case.
Signed-off-by: Han Zhou <hzhou@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Han Zhou [Wed, 4 Dec 2019 01:57:19 +0000 (17:57 -0800)]
ovsdb-cluster.at: Wait until leader is elected before DB compact.
In test case "election timer change", before testing DB compact,
we had to insert some data. Otherwise, inserting data after DB
compact will cause busy loop as mentioned in the XXX comment.
The root cause of the busy loop is still not clear, but the test
itself didn't wait until the leader election finish before initiating
DB compact. This patch adds the wait to make sure the test continue
after leader is elected so that the following tests are based on
a clean state. While this wait is added, the busy loop problem is
gone even without inserting the data, so the additional data insertion
is also removed by this patch.
A separate patch will address the busy loop problem in the scenario:
1. Restart cluster
2. DB compact before the cluster is ready
3. Insert data
Signed-off-by: Han Zhou <hzhou@ovn.org> Signed-off-by: Ben Pfaff <blp@ovn.org>
Aliasgar Ginwala [Fri, 20 Dec 2019 00:50:47 +0000 (16:50 -0800)]
ovs container build: Make kernel module configurable
--with-linux can be made configurable while building containers
for leveraging kernel modules installed on host.
KERNEL_VERSION=host should be used in env variable for the same.
Signed-off-by: Aliasgar Ginwala <aginwala@ebay.com> Signed-off-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Wed, 18 Dec 2019 14:38:29 +0000 (15:38 +0100)]
dpctl: Fix referencing DPDK in a flow dump.
Few reasons to replace 'non-dpdk interfaces' with 'the main thread':
* Flows are dumped from threads (from per thread flow tables) not from
the interfaces.
* 'non-dpdk' here sounds like all other flows (dumped from PMDs) has
some relation with DPDK which is not true at least because we have
afxdp and dummy ports that could be polled by PMD threads.
* 'main thread' is the same term as we're using in the output of
ovs-appctl dpif-netdev/pmd-stats-show.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
Ilya Maximets [Mon, 16 Dec 2019 12:54:38 +0000 (13:54 +0100)]
ovs-thread: Avoid huge alignment on a base spinlock structure.
Marking the structure as 64 bytes aligned forces compiler to produce
big holes in the containing structures in order to fulfill this
requirement. Also, any structure that contains this one as a member
automatically inherits this huge alignment making resulted memory
layout not efficient. For example, 'struct umem_pool' currently
uses 3 full cache lines (192 bytes) with only 32 bytes of actual data:
struct umem_pool {
int index; /* 0 4 */
unsigned int size; /* 4 4 */
Actual alignment of a spin lock is required only for Tx queue locks
inside netdev-afxdp to avoid false sharing, in all other cases
alignment only produces inefficient memory usage.
Also, CACHE_LINE_SIZE macro should be used instead of 64 as different
platforms may have different cache line sizes.
Using PADDED_MEMBERS to avoid alignment inheritance.
Fixes: ae36d63d7e3c ("ovs-thread: Make struct spin lock cache aligned.") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: William Tu <u9012063@gmail.com>
Kevin Traynor [Tue, 17 Dec 2019 15:07:37 +0000 (15:07 +0000)]
netdev-dpdk: Fix sw stats perf drop.
Accessing the sw stats in the vhost datapath of a PVP test
can incur a performance drop of ~2%.
Most of the time these stats will just be getting zero added
to them. By checking if there is a non-zero update first, we
can avoid accessing them when they won't be updated and avoid
the performance drop.
Fixes: 2f862c712e52 ("netdev-dpdk: Detailed packet drop statistics.") Signed-off-by: Kevin Traynor <ktraynor@redhat.com> Acked-by: Eelco Chaudron <echaudro@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
William Tu [Mon, 16 Dec 2019 14:18:25 +0000 (06:18 -0800)]
Documentation: Fix ovs-tcpdump options.
Signed-off-by: William Tu <u9012063@gmail.com> Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Signed-off-by: Simon Horman <simon.horman@netronome.com>
Ilya Maximets [Sat, 7 Dec 2019 14:46:17 +0000 (15:46 +0100)]
netdev-afxdp: Avoid removing of XDP program if not loaded.
'bpf_set_link_xdp_fd' generates netlink event regardless of actual
changes it does, so if-notifier will receive link update even if
there was no XDP program previously loaded on the interface.
OVS tries to remove XDP program if device configuration was not
successful triggering if-notifier that triggers bridge reconfiguration
and another attempt to add failed port. And so on in the infinite
loop.
This patch avoids the issue by not removing XDP program if it wasn't
loaded. Since loading of the XDP program is one of the last steps
of port configuration, this should help to avoid infinite re-addition
for most types of misconfiguration.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: William Tu <u9012063@gmail.com>
Ilya Maximets [Sat, 7 Dec 2019 14:46:16 +0000 (15:46 +0100)]
dpif-netdev: Avoid infinite re-addition of misconfigured ports.
Infinite re-addition of failed ports happens if the device in userspace
datapath has a linux network interface and it's not able to be
configured. For example, if the first reconfiguration fails because of
misconfiguration or bad initial device state.
In current code victims are afxdp ports and the Mellanox NIC ports
opened by the DPDK due to their bifurcated drivers (It's unlikely for
usual netdev-linux ports to fail).
The root cause: Every change in the state of the network interface
of a linux kernel device generates if-notifier event and if-notifier
event triggers the OVS code to re-apply the configuration of ports,
i.e. add broken ports back. The most obvious part is that dpif-netdev
changes the device flags before trying to configure it:
1. add_port()
2. set_flags() --> if-notifier event
3. reconfigure() --> port removal from the datapath due to
misconfiguration or any other issue in
the underlying device.
4. setting flags back --> another if-notifier event.
5. There was new if-notifier event?
yes --> re-apply all settings. --> goto step 1.
Easy way to reproduce is to add afxdp port with n_rxq=N, where N is
bigger than device supports.
This patch fixes the most obvious case for this issue by moving
enabling of a promisc mode later to the place where we already know
that device could be added to datapath without errors, i.e. after
its first successful reconfiguration.
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/363038.html Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: William Tu <u9012063@gmail.com>
Eelco Chaudron [Mon, 11 Nov 2019 14:02:37 +0000 (09:02 -0500)]
netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event.
Currently, OVS does not register and therefore not handle the
interface reset event from the DPDK framework. This would cause a
problem in cases where a VF is used as an interface, and its
configuration changes.
As an example in the following scenario the MAC change is not
detected/acted upon until OVS is restarted without the patch applied:
Ilya Maximets [Tue, 5 Nov 2019 17:20:51 +0000 (18:20 +0100)]
bridge: Allow manual notifications about interfaces' updates.
Sometimes interface updates could happen in a way ifnotifier is not
able to catch. For example some heavy operations (device reset) in
netdev-dpdk could require re-applying of the bridge configuration.
For this purpose new manual notifier introduced. Its function
'if_notifier_manual_report()' could be called directly by the code
that aware about changes. This new notifier is thread-safe.
projects / mirror_ovs.git / log