| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
|
|
|
|
|
|
| |
If tnl_port_should_receive() is false, we're looking for a normal
port, not a tunnel one. And it's better to print recirculation IDs
in hex since they are typically printed this way in flow dumps.
Fixes: d40533fc820c ("odp-util: Improve log messages and error reporting for Netlink parsing.")
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
| |
A packet go through the encap openflow(set_field tun_id/src/dst)
The tunnel wc bits will be set. But it should be clear if the
original packet is non-tunnel. It is not necessary for datapath
wc the tunnel info for match(like the similar logic for vlan).
Signed-off-by: wenxu <wenxu@chinatelecom.cn>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
While creating a new database file and storing a new snapshot
into it, raft module by mistake updates the base offset for the
old file. So, the base offset of a new file remains zero. Then
the old file is getting replaced with the new one, copying new
offsets as well. In the end, after a full compaction, base offset
is always zero. And any offset is twice as large as zero. That
triggers a new compaction again at the earliest scheduled time.
In practice this issue triggers compaction every 10-20 minutes
regardless of the database load, after the first one is triggered
by the actual file growth or by the 24h maximum limit.
Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for clustered databases.")
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-August/051977.html
Reported-by: Oleksandr Mykhalskyi <oleksandr.mykhalskyi@netcracker.com>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Cited commit correctly fixed the issue of incorrect reporting
of zero-length geneve option matches as wildcarded. But as a
side effect, exact match on the metadata length was added to
every tunnel flow match, even if the tunnel is not geneve.
That doesn't generate any functional issues, but it maybe
confusing to see 'tunnel(...,geneve(),...)' while looking at
datapath flow dumps for, e.g., vxlan tunnel flows.
Fix that by checking the port type before parsing geneve options.
tunnel() attribute itself doesn't have enough information to
figure out the tunnel type.
Fixes: 7a6c8074c5d2 ("netdev-offload-tc: Fix the mask for tunnel metadata length.")
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
| |
netdev_tc_flow_put() "consumes" the tunnel.tp_src value, but
it's never passed down to TC, and not parsed back. Fix that.
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Current offloading code supports only limited number of tunnel keys
and silently ignores everything it doesn't understand. This is
causing, for example, offloaded ERSPAN tunnels to not work, because
flow is offloaded, but ERSPAN options are not provided to TC.
There is a number of tunnel keys, which are supported by the userspace,
but silently ignored during offloading:
OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT
OVS_TUNNEL_KEY_ATTR_OAM
OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS
OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS
OVS_TUNNEL_KEY_ATTR_CSUM is kind of supported, but only for actions
and for some reason is set from the tunnel port instead of the
provided action, and not currently supported for the tunnel key in
the match.
Addig a default case to fail offloading of unknown attributes. For
now explicitly allowing incorrect behavior for the DONT_FRAGMENT flag,
otherwise we'll break all tunnel offloading by default. VXLAN and
ERSPAN options has to fail offloading, because the tunnel will not
work otherwise. OAM is not a default configurations, so failing it
as well. The missing DONT_FRAGMENT flag though should, probably,
cause frequent flow revalidation, but that is not new with this patch.
Same for the 'match' key, only clearing masks that was actually
consumed, except for the DONT_FRAGMENT and CSUM flags, which are
explicitly allowed and highlighted as broken.
Also, destination port as well as CSUM configuration for unknown
reason was not taken from the actions list and were passed via HW
offload info instead of being consumed from the set() action.
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2022-July/395522.html
Reported-by: Eelco Chaudron <echaudro@redhat.com>
Fixes: 8f283af89298 ("netdev-tc-offloads: Implement netdev flow put using tc interface")
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
| |
If the key is zero, it doesn't mean we don't need to match on it.
Masks should be checked instead.
Fixes: 49a7961fca65 ("lib/tc: Avoid matching on tunnel ttl or tos if not needed")
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
netdev_tc_flow_put() ignores the tunnel.tp_dst mask. That results
in the exact match on the value. TC supports the masked match on
this field and it does return the mask back during the flow dump
even if it wasn't provided initially. OVS should correctly handle
that. There is a problem though. Some drivers (mlx5) doesn't
support offloading if the destination port is not an exact match [1].
Keeping the logic as-is for now, but making it explicit and somewhat
documented in the comment, so it is clear what is happening and we can
revisit this in the future.
[1] https://patchwork.ozlabs.org/project/openvswitch/patch/20220704224505.1117988-3-i.maximets@ovn.org/#2927396
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
'wc.masks.tunnel.metadata.present.len' must be a mask for the same
field in the flow key, but in the tc_flower structure it's the real
length of metadata masks.
That is correctly handled for the individual opt->length, setting all
the masks to 0x1f like it's done in the tun_metadata_to_geneve_mask__(),
but not handled for the main 'len' field.
Fix that by setting the mask to 0xff like it's done during the flow
translation in xlate_actions() and during the flow dump in the
tun_metadata_from_geneve_nlattr().
Also, flower always has an exact match on the present.len field
regardless of its value and regardless of this field being masked
by OVS flow translation layer while installing the flow. Hence,
all tunnel flows dumped from TC should have an exact match on
present.len and also UDPIF flag, because present.len doesn't make
sense without that flag. Without the change, zero-length options
match is incorrectly reported as a wildcard match. The side effect
though is that zero-length match on geneve options is reported even
for other tunnel types, e.g. vxlan. But that should be fairly
harmless. To avoid reporting a match on empty geneve options for
vxlan/etc. tunnels we'll need to check the tunnel port type, there
is no enough information in the TUNNEL attribute itself.
Extra checks and comments added around the code to better explain
what is going on.
Fixes: a468645c6d33 ("lib/tc: add geneve with option match offload")
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
| |
With release of OVS v3.0.0, according to our release process,
2.17.x becomes a new LTS series.
Acked-by: Ian Stokes <ian.stokes@intel.com>
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The handler and CPU mapping in upcalls are incorrect, and this is
specially noticeable systems with cpu isolation enabled.
Say we have a 12 core system where only every even number CPU is enabled
C0, C2, C4, C6, C8, C10
This means we will create an array of size 6 that will be sent to
kernel that is populated with sockets [S0, S1, S2, S3, S4, S5]
The problem is when the kernel does an upcall it checks the socket array
via the index of the CPU, effectively adding additional load on some
CPUs while leaving no work on other CPUs.
e.g.
C0 indexes to S0
C2 indexes to S2 (should be S1)
C4 indexes to S4 (should be S2)
Modulo of 6 (size of socket array) is applied, so we wrap back to S0
C6 indexes to S0 (should be S3)
C8 indexes to S2 (should be S4)
C10 indexes to S4 (should be S5)
Effectively sockets S0, S2, S4 get overloaded while sockets S1, S3, S5
get no work assigned to them
This leads to the kernel to throw the following message:
"openvswitch: cpu_id mismatch with handler threads"
Instead we will send the kernel a corrected array of sockets the size
of all CPUs in the system, or the largest core_id on the system, which
ever one is greatest. This is to take care of systems with non-continous
core cpus.
In the above example we would create a
corrected array in a round-robin(assuming prime bias) fashion as follows:
[S0, S1, S2, S3, S4, S5, S6, S0, S1, S2, S3, S4]
Fixes: b1e517bd2f81 ("dpif-netlink: Introduce per-cpu upcall dispatch.")
Co-authored-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Michael Santana <msantana@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Additional threads are required to service upcalls when we have CPU
isolation (in per-cpu dispatch mode). The reason additional threads
are required is because it creates a more fair distribution. With more
threads we decrease the load of each thread as more threads would
decrease the number of cores each threads is assigned.
Adding additional threads also increases the chance OVS utilizes all
cores available to use. Some RPS schemas might make some handler
threads get all the workload while others get no workload. This tends
to happen when the handler thread count is low.
An example would be an RPS that sends traffic on all even cores on a
system with only the lower half of the cores available for OVS to use.
In this example we have as many handlers threads as there are
available cores. In this case 50% of the handler threads get all the
workload while the other 50% get no workload. Not only that, but OVS
is only utilizing half of the cores that it can use. This is the worst
case scenario.
The ideal scenario is to have as many threads as there are cores - in
this case we guarantee that all cores OVS can use are utilized
But, adding as many threads are there are cores could have a performance
hit when the number of active cores (which all threads have to share) is
very low. For this reason we avoid creating as many threads as there
are cores and instead meet somewhere in the middle.
The formula used to calculate the number of handler threads to create
is as follows:
handlers_n = min(next_prime(active_cores+1), total_cores)
Assume default behavior when total_cores <= 2, that is do not create
additional threads when we have less than 2 total cores on the system
Fixes: b1e517bd2f81 ("dpif-netlink: Introduce per-cpu upcall dispatch.")
Signed-off-by: Michael Santana <msantana@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
| |
The ip6_hdr is aligned to 4 bytes, but the pointer
from dp_packet_l3 is aligned to 2 bytes. Use
ovs_16aligned_ip6_hdr instead to get 2 bytes alignment.
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
pyOpenSSL was recently switched for the Python standard library ssl
module in the cited commit. Python SSLsocket.send() does not allow
non-zero optional flag and it will explicitly raise an exception for
that. pyOpenSSL did nothing with this flag but kept it to be
compatible with socket API:
https://github.com/pyca/pyopenssl/blob/main/src/OpenSSL/SSL.py#L1844
Fixes: 68543dd523bd ("python: Replace pyOpenSSL with ssl.")
Reported-at: https://bugzilla.redhat.com/2115035
Acked-By: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Miro Tomaska <mtomaska@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
| |
The preprocessor comparison string to check AVX512 capabilities are
lengthy and effecting user readability. Simpify this by aliasing the checks.
Suggested-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Sunil Pai G <sunil.pai.g@intel.com>
Acked-by: Cian Ferriter <cian.ferriter@intel.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
18.04 image is deprecated and will disappear soon. Also some
slowdowns and brownouts are planned to push users away from
this deprecated version:
https://github.com/actions/virtual-environments/issues/6002
Moving to 20.04. Can't move to 22.04 at the moment because of
deprecation warnings from openssl 3.0.
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
OVS kernel datapath and TC are parsing IPv6 fragments differently.
For IPv6 later fragments, according to the original design [1], OVS
always sets nw_proto=44 (IPPROTO_FRAGMENT), regardless of the type
of the L4 protocol.
This leads to situation where flow for nw_proto=44 gets installed
to TC, but packets can not match on it, causing all IPv6 later
fragments to go to userspace significantly degrading performance.
Disabling offload for such packets, so the flow can be installed
to the OVS kernel datapath instead. Disabling for all IPv6 fragments
including the first one, because it doesn't make a lot of sense to
handle them separately. It may also cause potential problems with
conntrack trying to re-assemble a packet from fragments handled by
different datapaths (first in HW, later in OVS kernel).
Checking both 'nw_proto' and 'nw_frag' as classifier might decide
to match only on one of them and also nw_proto will not be 44 for
the first fragment.
The issue was hidden for some time due to incorrect behavior of the
OVS kernel datapath that was recently fixed in kernel commit:
12378a5a75e3 ("net: openvswitch: fix parsing of nw_proto for IPv6 fragments")
To allow offloading in the future either flow dissector in TC
should be changed to parse packets in the same way as OVS does,
or parsing in OVS kernel and userspace should be made configurable,
so users can opt-in to the behavior change. Silent change of the
behavior (change by default) is not an option, because existing
OpenFlow pipelines may depend on a certain behavior.
[1] https://docs.openvswitch.org/en/latest/topics/design/#fragments
Fixes: 83e866067ea6 ("netdev-tc-offloads: Add support for IP fragmentation")
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
| |
When the bridge protocols is not included Openflow10, printing an error
message "version negotiation failed" when doing "Restoring saved flows".
Signed-off-by: Han Ding <handing@chinatelecom.cn>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The following test sequence:
conntrack - IPv4 fragmentation incomplete reassembled packet
conntrack - IPv4 fragmentation with fragments specified
leads to a systematic failure of the latter test on the kernel
datapath (linux). Multiple executions of the former may also lead to
multiple failures.
This is due to the fact that fragments not yet reassembled are kept in
a queue for /proc/sys/net/ipv4/ipfrag_time seconds, and if the
kernel receives a fragment already present in the queue, it returns
-EINVAL.
Below the related log message:
|00058|dpif|WARN|system@ovs-system: execute ct(commit) failed (Invalid argument)
on packet udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,
nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=0,nw_frag=first,tp_src=1,
tp_dst=2 udp_csum:0
Fix the sequence by sending the second fragment in "conntrack - IPv4
fragmentation incomplete reassembled packet", once the checks are
done.
IPv6 tests are not affected as the defrag kernel code path pretends to
add the duplicate fragment to the queue returning -EINPROGRESS, when a
duplicate is detected.
Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The permanent neighbor entry for fc00::1 is added into a wrong
namespace, so in order to reply to a ping from at_ns1, the
address of fc00::1 has to be discovered. Interfaces are attached
to OVS and we're removing flows that can forward ND requests
after initial setup. In case ND request wasn't sent and replied
before that, at_ns1 will not be able to discover fc00:1 and won't
reply to pings.
It's hard to catch this condition while running tests locally,
but for some reason our CI is failing consistently.
Fix the issue by removing all the unnecessary permanent entries
and just allowing all the normal traffic to flow through the
low priority OVS flow, so all addresses can be discovered.
Also adding one more wait to avoid occasional drops of the very
first packet.
Fixes: 2ff43c78c685 ("packets: Re-calculate IPv6 checksum only for first frag upon modify.")
Acked-by: Salem Sol <salems@nvidia.com>
Acked-by: Michael Phelan <michael.phelan@intel.com>
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
OVS kernel module is incorrectly updating checksums while changing
IPv6 fields of later fragments that doesn't really have L4 headers.
This makes the 'ping6 between two ports with header modify' test
fail on most of the distribution kernels.
The issue got indirectly fixed in latest 5.19 with commit:
12378a5a75e3 ("net: openvswitch: fix parsing of nw_proto for IPv6 fragments")
The reason is that set_ipv6() function in net/openvswitch/actions.c
is using the protocol number from the parsed flow key and not from
the packet itself, and nw_proto=44 is not a protocol where we can
update the checksum.
It was backported to all supported upstream stable trees, but didn't
find its way to most of the distributions yet.
Restricting the test to 5.19+ kernels to avoid failures on distro
kernels. Additionally allowing the previous test for later fragments
to be executed in userspace testsuite.
Fixes: 2ff43c78c685 ("packets: Re-calculate IPv6 checksum only for first frag upon modify.")
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Referenced commit changed policer action type from TC_ACT_UNSPEC (continue)
to TC_ACT_PIPE. However, since neither TC hardware offload layer nor mlx5
driver at the time validated action type and always assumed 'continue', the
breakage wasn't caught until later validation code was added. The change
also broke valid configuration when sending from offload-capable device to
non-offload capable. For example, when sending from mlx5 VF to OvS bridge
netdevice the traffic that passed matchall classifier with policer could no
longer match the following flower rule in software:
filter protocol all pref 1 matchall chain 0
filter protocol all pref 1 matchall chain 0 handle 0x1
in_hw (rule hit 7863)
action order 1: police 0x1 rate 32Mbit burst 1000Kb mtu 64Kb action drop/pipe overhead 0b
ref 1 bind 1 installed 17 sec firstused 17 sec
Action statistics:
Sent 152199634 bytes 102550 pkt (dropped 1315, overlimits 1315 requeues 0)
Sent software 74612172 bytes 51275 pkt
Sent hardware 77587462 bytes 51275 pkt
backlog 0b 0p requeues 0
used_hw_stats delayed
filter protocol ip pref 3 flower chain 0
filter protocol ip pref 3 flower chain 0 handle 0x1
dst_mac aa:94:1f:f2:f8:44
src_mac e4:00:01:08:00:02
eth_type ipv4
ip_flags nofrag
not_in_hw
action order 1: skbedit ptype host pipe
index 1 ref 1 bind 1 installed 6 sec used 6 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
action order 2: mirred (Ingress Redirect to device br-ovs) stolen
index 1 ref 1 bind 1 installed 6 sec used 6 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
cookie 401a9c8b3d403c62240d3eb5e21c1604
no_percpu
Fix the issue by restoring policer action type to 'continue'.
Fixes: c2567e533f8a ("add port-based ingress policing based packet-per-second rate-limiting")
Signed-off-by: Vlad Buslov <vladbu@nvidia.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
With just released flake8 5.0 we're getting a bunch of E275 errors:
utilities/bugtool/ovs-bugtool.in:959:23: E275 missing whitespace after keyword
tests/test-ovsdb.py:623:11: E275 missing whitespace after keyword
python/setup.py:105:8: E275 missing whitespace after keyword
python/setup.py:106:8: E275 missing whitespace after keyword
python/ovs/db/idl.py:145:15: E275 missing whitespace after keyword
python/ovs/db/idl.py:167:15: E275 missing whitespace after keyword
make[2]: *** [flake8-check] Error 1
This breaks CI on branches below 2.16. We don't see a problem right
now on newer branches because we're installing extra dependencies
that backtrack flake8 down to 4.1 or even 3.9.
Acked-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Instead of a very long hex string something like this will be printed:
|DBG|tc flower compare failed mask compare:
Expected Mask:
00000000 ff ff 00 00 ff ff ff ff-ff ff ff ff ff ff ff ff
00000020 00 00 00 00 00 00 00 00-00 00 00 00 00 00 03 00
00000090 00 00 00 00 00 00 00 00-ff ff ff ff ff ff ff ff
000000c0 ff 00 00 00 ff ff 00 00-ff ff ff ff ff ff ff ff
Received Mask:
00000000 ff ff 00 00 ff ff ff ff-ff ff ff ff ff ff ff ff
00000020 00 00 00 00 00 00 00 00-00 00 00 00 00 00 03 00
00000090 00 00 00 00 00 00 00 00-ff ff ff ff ff ff ff ff
000000c0 ff 00 00 00 00 00 00 00-ff ff ff ff ff ff ff ff
It's easier to spot the difference this way and count which bytes are
to blame, since offsets are printed as well.
Using a sparse dump to avoid printing huge number of all-zero lines.
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This change extends the debug logging with the sparse
dump of the flow mask structure to make debug process
easier.
Sample output:
|netdev_offload_tc|DBG|offloading isn't supported, unknown attribute
Unused mask bits:
00000270 00 00 00 00 00 00 00 00-00 00 00 ff 00 00 00 00
In this example, 0x270 + 11 = 635, which is an offset of
the nsh.mdtype in the struct flow.
Suggested-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
| |
New function to dump large and sparsely populated data structures
like struct flow.
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior in
lib/dpif-netlink.c:1077:40: runtime error:
left shift of 1 by 31 places cannot be represented in type 'int'
#0 0x73fc31 in dpif_netlink_port_add_compat lib/dpif-netlink.c:1077:40
#1 0x73fc31 in dpif_netlink_port_add lib/dpif-netlink.c:1132:17
#2 0x2c1745 in dpif_port_add lib/dpif.c:597:13
#3 0x07b279 in port_add ofproto/ofproto-dpif.c:3957:17
#4 0x01b209 in ofproto_port_add ofproto/ofproto.c:2124:13
#5 0xfdbfce in iface_do_create vswitchd/bridge.c:2066:13
#6 0xfdbfce in iface_create vswitchd/bridge.c:2109:13
#7 0xfdbfce in bridge_add_ports__ vswitchd/bridge.c:1173:21
#8 0xfb5319 in bridge_add_ports vswitchd/bridge.c:1189:5
#9 0xfb5319 in bridge_reconfigure vswitchd/bridge.c:901:9
#10 0xfae0f9 in bridge_run vswitchd/bridge.c:3334:9
#11 0xfe67dd in main vswitchd/ovs-vswitchd.c:129:9
#12 0x4b6d8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
#13 0x4b6e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
#14 0x562594eed024 in _start (vswitchd/ovs-vswitchd+0x787024)
Fixes: 526df7d8543f ("tunnel: Provide framework for tunnel extensions for VXLAN-GBP and others")
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
On Python 3.12, distutils will be removed and it's currently (3.10+)
deprecated (see PEP 632).
Since the suggested and simplest replacement is setuptools, this commit
replaces distutils to use setuptools instead.
setuptools < 59.0 doesn't have setuptools.errors and so, in this case,
distutils.errors is still used.
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
In case of modifying an IPv6 packet src/dst address the L4 checksum
should be recalculated only for the first frag. Currently it's done
for all frags, leading to incorrect reassembled packet checksum.
Fix it by adding a new flag to recalculate the checksum only for the
first frag.
Fixes: bc7a5acdff08 ("datapath: add ipv6 'set' action")
Signed-off-by: Salem Sol <salems@nvidia.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
LeakSanitizer for some reason reports these json objects as leaked,
even though we do have references to them at the moment ovs_fatal()
called from check_ovsdb_error().
Previously it complained only with -O2, but with newer versions of
clang/llvm it started complaining even with -O1. For example, negative
ovsdb parsing tests are failing on ubuntu 22.04 with clang 14 if built
with ASan and detect_leaks=1.
Fix that by destroying the json object before aborting the process.
And we may also build with default -O2 in CI with that change.
Alternative implementation might be to just pass the json to destroy
to every check_ovsdb_error() call, but indirect registering of the
pointer seems a bit less invasive.
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This fixes the obsolescence warning for AC_TRY_RUN with autoconf 2.70+:
$ ./boot.sh
configure.ac:141: warning: The macro `AC_TRY_RUN' is obsolete.
configure.ac:141: You should run autoupdate.
./lib/autoconf/general.m4:2997: AC_TRY_RUN is expanded from...
lib/m4sugar/m4sh.m4:692: _AS_IF_ELSE is expanded from...
lib/m4sugar/m4sh.m4:699: AS_IF is expanded from...
./lib/autoconf/general.m4:2249: AC_CACHE_VAL is expanded from...
./lib/autoconf/general.m4:2270: AC_CACHE_CHECK is expanded from...
m4/ax_func_posix_memalign.m4:27: AX_FUNC_POSIX_MEMALIGN is expanded from...
configure.ac:141: the top level
Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
AS_HELP_STRING is a direct replacement for AC_HELP_STRING.
It is available since autoconf 2.57a. OVS requires 2.63,
so AS_HELP_STRING can be freely used.
This fixes the following warning on systems with 2.70+:
$ ./boot.sh
...
configure.ac:92: warning: The macro `AC_HELP_STRING' is obsolete.
configure.ac:92: You should run autoupdate.
...
Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
SSL, BPF, lcap-ng and other libraries are in use by a static library,
so they has to be linked while building applications with that static
library, i.e. 'pkg-config --libs --static libopenvswitch' must return
-lssl, -lcap-ng, etc. in the output for a successful build.
For dynamic library (non-private Libs) all these libraries will be
dynamically linked to libopenvswitch.so, so the application will
pick them up without having a direct dependency.
Acked-by: Frode Nordahl <frode.nordahl@canonical.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, openvswitch-devel installs following header tree:
/usr/include
/openflow/*.h
/openvswitch
/*.h
/openflow/*.h
/openvswitch/*.h
/sparse/*.h
/lib/*.h
Few issues with that:
1. openflow and openvswitch headers are installed twice. Once in the
main /usr/include and second time in the /usr/include/openvswitch/.
2. For some reason internal headers such as lib/*.h and fairly useless
headers such as sparse/*.h are installed as well.
One more issue is that current pkg-config files doesn't work with
builds installed with 'make install', because 'make install' doesn't
create this weird header tree.
While double install of same headers is not a huge problem, it doesn't
seem right. Installation of the internal headers is a bigger issue.
They are not part of API/ABI and we do not provide any stability
guarantees for them. We are making incompatible changes constantly in
minor updates, so users should not rely on these headers.
If it's necessary for some external application to use them, this
external application should not link with libopenvswitch dynamically
and also it can't expect the static library to not break these API/ABI,
hence there is no real point installing them. Application should use
OVS as a submodule like OVN does or compile itself by obtaining
required version of OVS sources otherwise. Another option is to
properly export and install required headers.
pkg-config configuration files updated as necessary.
Fixes: 4886d4d2495b ("debian, rhel: Ship ovs shared libraries and header files")
Reviewed-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
PyModule_AddObject() may fail and it doesn't steal references
in this case. The error condition should be handled to avoid
possible memory leaks.
And while it's not strictly specified if PyModule_Create may
fail, most of the examples in python documentation include
handling of a NULL case.
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Bond master netdev may be created without a classification type, due
to routing or tunneling code.
If bond master is not attached to ovs, the ingress block on LAG members
should not be updated.
Simple reproducer:
tc q ls dev net3 ingress
ip a add 10.1.1.1/30 dev bond0
ip l set net3 master bond0
tc q ls dev net3 ingress
Fixes: d22f8927c3c9 ("netdev-linux: monitor and offload LAG slaves to TC")
Signed-off-by: Tao Liu <thomas.liu@ucloud.cn>
Acked-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
When netdev first opened by netdev_open(..., NULL, ...), netdev_class sets
to system by default, and auto_classified sets to true.
If netdev reopens by netdev_open(..., "system", ...), auto_classified
should be cleared. This will be used in next patch to fix lag issue.
Fixes: 8c2c225e481d ("netdev: Fix netdev_open() to track and recreate classless interfaces")
Signed-off-by: Tao Liu <thomas.liu@ucloud.cn>
Acked-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
| |
Ping process remains in the system after the test. Using a proper
macro that will correctly register it for stopping at cleanup stage.
Fixes: 134e6831acca ("system-traffic: Check frozen state handling with TLV map change")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Reviewed-by: David Marchand <david.marchand@redhat.com>
|
| |
|
|
|
|
|
|
|
|
|
|
| |
A connection is established if we see packets from both directions.
The cited commit fixed the issue of sending twice in one direction,
but still an issue if more than that.
Fix it.
Fixes: a867c010ee91 ("conntrack: Fix conntrack new state")
Signed-off-by: Eli Britstein <elibr@nvidia.com>
Acked-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
| |
ovs/_json.c:67:20: warning: assignment discards ‘const’ qualifier from pointer
target type [-Wdiscarded-qualifiers]
ovs/_json.c:132:27: warning: comparison of integer expressions of different
signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
| |
Since Python 2 is not supported anymore, remove Python 2 support from C
extension too
Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When we send parallel flows such as VXLAN to a PF[1] port in OVS with
multiple PMDs. OVS will create a RTE flow with Mark and RSS actions to
send flows to the software data path. But the RSS action does not work
well and all the flows are forwarded to a single PMD. This is because
RSS hash types should be set in RSS action.
[1]: In our testbed, a Mellanox ConnectX-6 is used as a PF port.
[i.maximets]
DPDK PMD drivers supposed to provide "best-effort" RSS configuration
if the type is set to zero. However, they are very inconsistent in
practice and barely put any effort to provide a good configuration.
For example, mlx5 driver seems to use just RTE_ETH_RSS_IP, which is
not enough for most deployments.
Setting the types the same way we configure them for a normal RSS
in netdev-dpdk to workaround the scalability issue.
Signed-off-by: Harold Huang <baymaxhuang@gmail.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
| |
nw_frag was not being printed in the flow key because it was improperly
masked and printed. Since this field is only two bits, it needs to use a
different macro to be masked. During printing, the switch statement
switched on the whole 8 bits rather than just the two that are relevant.
This caused nw_frag to often not be printed at all.
Signed-off-by: Rosemarie O'Riorden <roriorden@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
| |
ovsdb/ directory should not be a dependency, otherwise the man
page is getting re-built every time unrelated files are changed.
Fixes: 6f24c2bc769a ("ovsdb: Add Local_Config schema.")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
calc_offsets() function returns 'data' and 'mask' pointers, which
are pointers somewhere inside struct tc_flower_key, and they are not
aligned, causing misaligned memory access. For example:
ipv6.rewrite_hlimit is at 148 byte offset inside the struct
tc_flower_key. While the actual field is in the 7th byte of
the IPv6 header in the actual packet. So, pedit will need
to write the last byte of the [4-7] range to the actual packet.
So, data pointer is positioned to 145th byte inside the tc_flower_key
with the 000000FF mask. Obviously, 145th byte inside the structure
is not 4-byte aligned.
lib/tc.c:2879:34: runtime error:
load of misaligned address 0x7f2802eaa321 for type 'ovs_be32' (aka
'unsigned int'), which requires 4 byte alignment
0x7f2802eaa321: note: pointer points here
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ...
^
0 0xd7f2fb in nl_msg_put_flower_rewrite_pedits lib/tc.c:2879:34
1 0xd7f2fb in nl_msg_put_flower_acts lib/tc.c:3141:25
2 0xd6ae5a in nl_msg_put_flower_options lib/tc.c:3445:12
3 0xd6a2be in tc_replace_flower lib/tc.c:3712:17
4 0xd2bf25 in netdev_tc_flow_put lib/netdev-offload-tc.c:2224:11
5 0x94f6b7 in netdev_flow_put lib/netdev-offload.c:316:14
6 0xcbd19e in parse_flow_put lib/dpif-netlink.c:2289:11
7 0xcbd19e in try_send_to_netdev lib/dpif-netlink.c:2376:15
8 0xcbd19e in dpif_netlink_operate lib/dpif-netlink.c:2447:23
9 0x86536e in dpif_operate lib/dpif.c:1372:13
10 0x6bc289 in handle_upcalls ofproto/ofproto-dpif-upcall.c:1654:5
11 0x6bc289 in recv_upcalls ofproto/ofproto-dpif-upcall.c:892:9
12 0x6b766a in udpif_upcall_handler ofproto/ofproto-dpif-upcall.c:792:13
13 0xb5015a in ovsthread_wrapper lib/ovs-thread.c:422:12
14 0x7f280b2081ce in start_thread (/lib64/libpthread.so.0+0x81ce)
15 0x7f2809e39dd2 in clone (/lib64/libc.so.6+0x39dd2)
Fix misaligned read by using appropriate functions.
Fixes: 8ada482bbe19 ("tc: Add header rewrite using tc pedit action")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The bash completion scripts shipped with Open vSwitch currently
have the executable bit set. This is problematic because the
files do not start with a shebang and as such a user may end up
executing them using the wrong shell. When installed in a system
the bash shell will source these files and not execute them.
This also triggers Debian lintian warnings [0] and defies Debian
policy [1].
0: https://lintian.debian.org/tags/executable-not-elf-or-script
1: https://www.debian.org/doc/debian-policy/ch-files.html#scripts
Fixes: 423ede182b65 ("utilities: Add bash command-line completion script.")
Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Without running set command first the string matching
fails on get command beacuse DPCLS prio value is different
for different default builds like with --enable-autovalidator
build auto-validator prio is set to 255 and if the build
is a scalar than generic value is default 255.
The same problem is seen with dpif where re-arranging the get
command after set makes it consistent across any builds.
Fixes: cc0a87b11c (pmd.at: Add test-cases for DPCLS and DPIF commands.)
Signed-off-by: Kumar Amber <kumar.amber@intel.com>
Acked-by: Michael Phelan <michael.phelan@intel.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The only way to configure settings on a remote (e.g. inactivity_probe)
is via --remote=db:DB,table,row. There is no way to do this via the
existing CLI options.
For a clustered DB with multiple servers listening on unique addresses
there is no way to store these entries in the DB as the DB is shared.
For example, three servers listening on 1.1.1.1, 1.1.1.2, and 1.1.1.3
respectively would require a Manager/Connection row each, but then
all three servers would try to listen on all three addresses.
It is possible for ovsdb-server to serve multiple databases. This
means that we can have a local "config" database in addition to
the main database we are serving (Open_vSwitch, OVN_Southbound, etc.)
and this patch adds a Local_Config schema that currently just mirrors
the Connection table and a Config table with a 'connections' row that
stores each Connection.
Signed-off-by: Terry Wilson <twilson@redhat.com>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
dp_netdev_input_outer_avx512 allocates a 16KB scratch pad per PMD
thread, but it's never freed. This may cause significant memory
drain in dynamic environments.
==4068109==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 38656 byte(s) in 2 object(s) allocated from:
0 0xf069fd in posix_memalign (vswitchd/ovs-vswitchd+0xf069fd)
1 0x1d7ed14 in xmalloc_size_align lib/util.c:254:13
2 0x1d7ed14 in xmalloc_pagealign lib/util.c:352:12
3 0x2098254 in dp_netdev_input_outer_avx512 lib/dpif-netdev-avx512.c:69:17
4 0x191591a in dp_netdev_process_rxq_port lib/dpif-netdev.c:5332:19
5 0x190a961 in pmd_thread_main lib/dpif-netdev.c:6963:17
6 0x1c4b69a in ovsthread_wrapper lib/ovs-thread.c:422:12
7 0x7fd5ea6f1179 in start_thread pthread_create.c
SUMMARY: AddressSanitizer: 38656 byte(s) leaked in 2 allocation(s).
Fixes: 9ac84a1a3698 ("dpif-avx512: Add ISA implementation of dpif.")
Reviewed-by: David Marchand <david.marchand@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Kumar Amber <kumar.amber@intel.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As described in the bugzilla below, cpu_has_isa code may be compiled
with some AVX512 instructions in it, because cpu.c is built as part of
the libopenvswitchavx512.
This is a problem when this function (supposed to probe for AVX512
instructions availability) is invoked from generic OVS code, on older
CPUs that don't support them.
For the same reason, dpcls_subtable_avx512_gather_probe,
dp_netdev_input_outer_avx512_probe, mfex_avx512_probe and
mfex_avx512_vbmi_probe are potential runtime bombs and can't either be
built as part of libopenvswitchavx512.
Move cpu.c to be part of the "normal" libopenvswitch.
And move other helpers in generic OVS code.
Note:
- dpcls_subtable_avx512_gather_probe is split in two, because it also
needs to do its own magic,
- while moving those helpers, prefer direct calls to cpu_has_isa and
avoid cast to intermediate integer variables when a simple boolean
is enough,
Fixes: 352b6c7116cd ("dpif-lookup: add avx512 gather implementation.")
Fixes: abb807e27dd4 ("dpif-netdev: Add command to switch dpif implementation.")
Fixes: 250ceddcc2d0 ("dpif-netdev/mfex: Add AVX512 based optimized miniflow extract")
Fixes: b366fa2f4947 ("dpif-netdev: Call cpuid for x86 isa availability.")
Reported-at: https://bugzilla.redhat.com/2100393
Reported-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
|
