While offloading header modifications to TC, OVS is using {TCA_PEDIT} +
{TCA_CSUM} combination as that it the only way to represent header
rewrite. However, {TCA_CSUM} is unable to calculate L4 checksums for
IP fragments.
Since TC already applies fragmentation bit masking, this patch simply
needs to prevent these packets from being processed through TC.
Reported-at: https://issues.redhat.com/browse/FDP-545
Acked-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
This patch adds "Don't Fragment" (DF) flag support to the encap action,
if supported by the kernel. If the kernel does not support this,
it falls back to the previous behavior of ignoring the DF request.
Acked-by: Roi Dayan <roid@nvidia.com>
Acked-by: Simon Horman <horms@ovn.org>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
This patch checks to see if the TCA_FLOWER_KEY_ENC_FLAGS key is supported.
Acked-by: Roi Dayan <roid@nvidia.com>
Acked-by: Simon Horman <horms@ovn.org>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Clang 18.1.3-2.fc41 throws a warning:
lib/tc.c:3060:25: error: field 'sel' with variable sized type
'struct tc_pedit_sel' not at the end of a struct or class is a
GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
3060 | struct tc_pedit sel;
| ^
Refactor the structure into a proper union to avoid the build failure.
Interestingly, clang 18.1.3-2.fc41 on Fedora throws a warning, but
relatively the same version 18.1.3 (1) on Ubuntu 24.04 does not.
Acked-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Currently we log the 980-ish byte long tc_action structure as a
single long hex string. That is very hard to read and hard to
spot the difference between two. And most of the fields are zero.
Use the sparse hex dump instead as we do for keys already.
Ex.:
Action 1 mismatch:
- Expected Action:
00000000 f0 3c 00 00 01 00 00 00-00 00 00 00 00 00 00 00
000003d0 00 00 00 00 ff ff ff ff-
- Received Action:
00000000 f0 3c 00 00 01 01 00 00-00 00 00 00 00 00 00 00
000003d0 00 00 00 00 ff ff ff ff-
Without the change, each action would be a 1900+ characters
long string of mostly zeroes.
Acked-by: Simon Horman <horms@ovn.org>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Currently checksum recalculation is not supported with TC offload for
IPIP and GRE packets. This patch adds support for TC offloading of
IPIP and GRE packets by adding the correct csum action.
Without this patch the following warning can be seen in the logging:
Can't offload rewrite of IP/IPV6 with ip_proto: X.
Acked-by: Simon Horman <horms@ovn.org>
Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
When the IP header is modified, for example, by NAT or a ToS/TTL change,
the IP header checksum needs recalculation. In addition to the IP header
checksum, for UDPLITE, its checksum also needs recalculation when any
of the addresses change.
This patch adds support for TC offloading of IGMP, UDPLITE, and SCTP
packets by adding the correct csum action.
Acked-by: Simon Horman <horms@ovn.org>
Signed-off-by: Faicker Mo <faicker.mo@ucloud.cn>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Use TCA_POLICE_RATE64 if the rate cannot be expressed using 32bits.
This breaks the 32Gbps barrier. The new barrier is ~4Tbps caused by
netdev's API expressing kbps rates using 32-bit integers.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2137643
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Add TC offload support for vxlan encap with gbp option.
Reviewed-by: Gavi Teitz <gavi@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Gavin Li <gavinl@nvidia.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Most of the data members of struct tc_action{ } are defined as anonymous
struct in place. Instead of passing all members of an anonymous struct,
which is not flexible to new members being added, expose encap as named
struct and pass it entirely.
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Gavin Li <gavinl@nvidia.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Tc flower tunnel key options were encoded in nl_msg_put_flower_tunnel_opts
and decoded in nl_parse_flower_tunnel_opts. Only geneve was supported.
To avoid adding more arguments to the function to support more vxlan
options in the future, change the function arguments to pass tunnel
entirely to it instead of keep adding new arguments.
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Gavin Li <gavinl@nvidia.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
The tc module combines the use of the `tc_transact` helper
function for communication with the in-kernel tc infrastructure
with assertions on the reply data by `ofpbuf_at_assert` on the
received data prior to further processing.
With the presence of bugs on the kernel side, we need to treat
the kernel as an unreliable service provider and replace assertions
on the reply from it with checks to avoid a fatal crash of OVS.
For the record, the symptom of the crash is this in the log:
EMER|include/openvswitch/ofpbuf.h:194:
assertion offset + size <= b->size failed in ofpbuf_at_assert()
And an excerpt of the backtrace looks like this:
ofpbuf_at_assert (offset=16, size=20) at include/openvswitch/ofpbuf.h:194
tc_replace_flower at lib/tc.c:3223
netdev_tc_flow_put at lib/netdev-offload-tc.c:2096
netdev_flow_put at lib/netdev-offload.c:257
parse_flow_put at lib/dpif-netlink.c:2297
try_send_to_netdev at lib/dpif-netlink.c:2384
Reported-At: https://launchpad.net/bugs/2018500
Fixes: 5c039ddc64ff ("netdev-linux: Add functions to manipulate tc police action")
Fixes: e7f6ba220e10 ("lib/tc: add ingress ratelimiting support for tc-offload")
Fixes: f98e418fbdb6 ("tc: Add tc flower functions")
Fixes: c1c9c9c4b636 ("Implement QoS framework.")
Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Sometimes there is a need to clean empty chains as done in
delete_chains_from_netdev(). The cited commit doesn't remove
the chain completely which cause adding ingress_block later to fail.
This can be reproduced with adding bond as ovs port which makes ovs
use ingress_block for it.
While at it add the netdev name that fails to the log.
Fixes: e1e5eac5b016 ("tc: Add TCA_KIND flower to delete and get operation to avoid rtnl_lock().")
Signed-off-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
If a tc flow was installed but has not yet been used, report it as such.
In addition, add a delay to the "IGMP - flood under normal action" test
case to make it work with many repetitions. This delay is also present
in other ICMP/IGMP tests.
Fixes: f98e418fbdb6 ("tc: Add tc flower functions")
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Tested-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
A long long time ago, an effort was made to make tc flower
rtnl_lock() free. However, on the OVS part we forgot to add
the TCA_KIND "flower" attribute, which tell the kernel to skip
the lock. This patch corrects this by adding the attribute for
the delete and get operations.
The kernel code calls tcf_proto_is_unlocked() to determine the
rtnl_lock() is needed for the specific tc protocol. It does this
in the tc_new_tfilter(), tc_del_tfilter(), and in tc_get_tfilter().
If the name is not set, tcf_proto_is_unlocked() will always return
false. If set, the specific protocol is queried for unlocked support.
Fixes: f98e418fbdb6 ("tc: Add tc flower functions")
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Currently tc offload flow packet counters will roll over every ~4
billion packets. This is because the packet counter in struct
tc_stats provided by TCA_STATS_BASIC is a 32bit integer.
Now we check for the optional TCA_STATS_PKT64 attribute which provides
the full 64bit packet counter if the 32bit one has rolled over. Because
the TCA_STATS_PKT64 attribute may appear multiple times in a netlink
message, the method of parsing attributes was changed.
Fixes: f98e418fbdb6 ("tc: Add tc flower functions")
Reported-at: https://bugzilla.redhat.com/1776816
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
CID 550702 (#1 of 1): Dereference null return value (NULL_RETURNS)
7. dereference: Dereferencing a pointer that might be NULL ex_type when calling nl_attr_get_u16.
Signed-off-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
Fix coverity big parameter passed by value
CID 549858 (#1 of 1): Big parameter passed by value (PASS_BY_VALUE)
pass_by_value: Passing parameter metadata of type struct tun_metadata (size 272 bytes) by value,
which exceeds the medium threshold of 256 bytes
Signed-off-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
Offsets within 'rewrite' action are not 4-byte aligned, so has to
be accessed carefully.
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior lib/tc.c:1132:17 in
lib/tc.c:1132:17: runtime error: store to misaligned address 0x7fba215b2025
for type 'ovs_be32' (aka 'unsigned int'), which requires 4 byte alignment
0 0xd78857 in nl_parse_act_pedit lib/tc.c:1132:24
1 0xd68103 in nl_parse_single_action lib/tc.c:1936:15
2 0xd624ee in nl_parse_flower_actions lib/tc.c:2024:19
3 0xd624ee in nl_parse_flower_options lib/tc.c:2139:12
4 0xd5f082 in parse_netlink_to_tc_flower lib/tc.c:2187:12
5 0xd6a2a1 in tc_replace_flower lib/tc.c:3776:19
6 0xd2ae8f in netdev_tc_flow_put lib/netdev-offload-tc.c:2350:11
7 0x951d07 in netdev_flow_put lib/netdev-offload.c:318:14
8 0xcbb81a in parse_flow_put lib/dpif-netlink.c:2297:11
9 0xcbb81a in try_send_to_netdev lib/dpif-netlink.c:2384:15
10 0xcbb81a in dpif_netlink_operate lib/dpif-netlink.c:2455:23
11 0x8678ae in dpif_operate lib/dpif.c:1372:13
12 0x6bcc89 in handle_upcalls ofproto/ofproto-dpif-upcall.c:1674:5
13 0x6bcc89 in recv_upcalls ofproto/ofproto-dpif-upcall.c:905:9
14 0x6b7f9a in udpif_upcall_handler ofproto/ofproto-dpif-upcall.c:801:13
15 0xb54c5a in ovsthread_wrapper lib/ovs-thread.c:422:12
16 0x7fba2f2081ce in start_thread (/lib64/libpthread.so.0+0x81ce)
17 0x7fba2de39dd2 in clone (/lib64/libc.so.6+0x39dd2)
Fixes: 8ada482bbe19 ("tc: Add header rewrite using tc pedit action")
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Add tc action flags when adding police action to offload meter table.
There is a restriction that the flag of skip_sw/skip_hw should be same for
filter rule and the independent created tc actions the rule uses. In this
case, if we configure the tc-policy as skip_hw, filter rule will be created
with skip_hw flag and the police action according to meter table will have
no action flag, then flower rule will fail to add to tc kernel system.
To fix this issue, we will add tc action flag when adding police action to
offload a meter table, so it will allow meter table to work in tc software
datapath.
Fixes: 5c039ddc64ff ("netdev-linux: Add functions to manipulate tc police action")
Signed-off-by: Baowen Zheng <baowen.zheng@corigine.com>
Acked-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
This reverts commit dd9881ed55e6 ('tc: Fix stats dump when
using same meter table')
This patch doesn't solve the tc flow stats update issue and
will lead to failure of system-offloads-traffic testsuite, it
only counts packets surviving after the tc filter, rather than
hitting the filter
A following patch will come up to solve this flow stats update
issue
Signed-off-by: Tianyu Yuan <tianyu.yuan@corigine.com>
Acked-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
OVN is setting ct drop rule with a ct clear action.
OVS datapath behavior is if there is no forward action
the default is drop.
TC behavior is to continue with next match.
Fix to match tc to ovs behavior by setting last action
attribute as drop instead of pipe.
Also update lastused when parsing ct action.
example rule
recirc_id(0x1),in_port(2),ct_state(+trk),eth(),eth_type(0x0800),ipv4(frag=no),
packets:82, bytes:8036, used:2.108s, actions:ct_clear
Reviewed-by: Maor Dickman <maord@nvidia.com>
Signed-off-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
When we apply meter police on both directions of TCP traffic, the
dumped stats is shown same (as shown below). This issue is introduced
by modifying the stats update strategy.
...,in_port(6),eth(),eth_type(0x0800),ipv4(frag=no), packets:1488557,
bytes:2089059644, used:0.040s, actions:meter(0),9
...,in_port(9),eth(),eth_type(0x0800),ipv4(frag=no), packets:1488557,
bytes:2089059644, used:0.040s, actions:meter(0),6
In previous patch, after parsing police action, the flower stats will
be updated by dumped meter table stats, which will result in the issue
above.
Thus, the stats of meter table should not be used when dumping flow
stats. Ignore the stats update when police.index belongs to meter.
Fixes: a9b8cdde69de ("tc: Add support parsing tc police action")
Signed-off-by: Tianyu Yuan <tianyu.yuan@corigine.com>
Reviewed-by: Baowen Zheng <baowen.zheng@corigine.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
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>
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>
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>
This change implements support for the check_pkt_len
action using the TC police action, which supports MTU
checking.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When offloading rule, tc should be filled with police index, instead
of meter id. As meter is mapped to police action, and the mapping is
inserted into meter_id_to_police_idx hmap, this hmap is used to find
the police index. Besides, the reverse mapping between meter id and
police index is also added, so meter id can be retrieved from this
hashmap and pass to dpif while dumping rules.
Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
As the police actions with indexes of 0x10000000-0x1fffffff are
reserved for meter offload, to provide a clean environment for OVS,
these reserved police actions should be deleted on startup. So dump
all the police actions, delete those actions with indexes in this
range.
Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
Add helpers to add, delete and get stats of police action with
the specified index.
Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
Add function to parse police action from netlink message.
Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
Pointers to gnet_stats_basic and tcf_t are not correctly aligned,
so we need to copy the data before accessing. Found by running
check-offloads testsuite with UBsan:
lib/tc.c:1791:50: runtime error:
member access within misaligned address 0x61900005ce1c for type
'const struct gnet_stats_basic', which requires 8 byte alignment
0x61900005ce1c: note: pointer points here
14 00 07 00 00 00 00 00 00 00 00 00 00 00 00 00 ...
^
0 0xd69044 in nl_parse_single_action lib/tc.c:1791:50
1 0xd69044 in nl_parse_flower_actions lib/tc.c:1841:19
2 0xd57612 in nl_parse_flower_options lib/tc.c:1893:12
3 0xd5468d in parse_netlink_to_tc_flower lib/tc.c:1941:12
4 0xd5a7ec in tc_replace_flower lib/tc.c:3199:19
5 0xd2baea in probe_tc_block_support lib/netdev-offload-tc.c:2226:13
6 0xd2baea in netdev_tc_init_flow_api lib/netdev-offload-tc.c:2279:9
7 0x94d726 in netdev_assign_flow_api lib/netdev-offload.c:183:14
8 0x94d726 in netdev_init_flow_api lib/netdev-offload.c:323:9
9 0x9515c7 in netdev_ports_flow_init lib/netdev-offload.c:775:8
10 0x9515c7 in netdev_set_flow_api_enabled lib/netdev-offload.c:815:13
11 0x562ec8 in bridge_run vswitchd/bridge.c:3292:9
12 0x59a98c in main vswitchd/ovs-vswitchd.c:129:9
13 0x7fb5c583acf2 in __libc_start_main (/lib64/libc.so.6+0x3acf2)
14 0x47e60d in _start (vswitchd/ovs-vswitchd+0x47e60d)
lib/tc.c:1306:43: runtime error:
member access within misaligned address 0x619000140324 for type
'const struct tcf_t', which requires 8 byte alignment
0x619000140324: note: pointer points here
24 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 ...
^
0 0xd6ee6f in nl_parse_tcf lib/tc.c:1306:43
1 0xd6423f in nl_parse_act_mirred lib/tc.c:1389:5
2 0xd6423f in nl_parse_single_action lib/tc.c:1747:15
3 0xd6423f in nl_parse_flower_actions lib/tc.c:1843:19
4 0xd57612 in nl_parse_flower_options lib/tc.c:1895:12
5 0xd5468d in parse_netlink_to_tc_flower lib/tc.c:1943:12
6 0xd5a7ec in tc_replace_flower lib/tc.c:3201:19
7 0xd28ae8 in netdev_tc_flow_put lib/netdev-offload-tc.c:1969:11
8 0x94cc97 in netdev_flow_put lib/netdev-offload.c:257:14
9 0xcba2be in parse_flow_put lib/dpif-netlink.c:2289:11
10 0xcba2be in try_send_to_netdev lib/dpif-netlink.c:2376:15
11 0xcba2be in dpif_netlink_operate lib/dpif-netlink.c:2447:23
12 0x8647ce in dpif_operate lib/dpif.c:1372:13
13 0x6b50a6 in push_dp_ops ofproto/ofproto-dpif-upcall.c:2406:5
14 0x6c9bcd in revalidate ofproto/ofproto-dpif-upcall.c:2792:13
15 0x6b79b5 in udpif_revalidator ofproto/ofproto-dpif-upcall.c:980:9
16 0xb4d5ea in ovsthread_wrapper lib/ovs-thread.c:422:12
17 0x7ff1090081ce in start_thread (/lib64/libpthread.so.0+0x81ce)
18 0x7ff107c39dd2 in clone (/lib64/libc.so.6+0x39dd2)
This patch fixes only problems reported by UBsan in current tests,
there could be more issues like this not currently covered by the
testsuite.
Acked-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When programming NAT rules OVS only sets the minimum value for a
single IP/port value. However, responses from flower will always
return min == max for single IP/port values. This is causing the
verification to fail as the request is different than the response.
To avoid this, we will update the response to match the request.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Regardless of the traffic type, if requested, the conntrack information
should be included to keep the datapath and tc rules in sync.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This change will set the correct VID and PCP masks, as well as the
ethernet type mask.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This patch adds more detailed debug logs on tc verify failures to
ease debugging the actual cause after the fact.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Currently, tc merges all header rewrite actions into one tc pedit
action. So the header rewrite actions order is lost. Save each header
rewrite action into one tc pedit action to keep the order. And only
append one tc csum action to the last pedit action of a series.
Signed-off-by: Chris Mi <cmi@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Fragmented packets with offset=0 are defragmented by tc act_ct, and
only when assembled pass to next action, in ovs offload case,
a goto action. Since stats are overwritten on each action dump,
only the stats for last action in the tc filter action priority
list is taken, the stats on the goto action, which count
only the assembled packets. See below for example.
Hardware updates just part of the actions (gact, ct, mirred) - those
that support stats_update() operation. Since datapath rules end
with either an output (mirred) or recirc/drop (both gact), tc rule
will at least have one action that supports it. For software packets,
the first action will have the max software packets count.
Tc dumps total packets (hw + sw) and hardware packets, then
software packets needs to be calculated from this (total - hw).
To fix the above, get hardware packets and calculate software packets
for each action, take the max of each set, then combine back
to get the total packets that went through software and hardware.
Example by running ping above MTU (ping <IP> -s 2000):
ct_state(-trk),recirc_id(0),...,ipv4(proto=1,frag=first),
packets:14, bytes:19544,..., actions:ct(zone=1),recirc(0x1)
ct_state(-trk),recirc_id(0),...,ipv4(proto=1,frag=later),
packets:14, bytes:28392,..., actions:ct(zone=1),recirc(0x1)
Second rule should have had bytes=14*<size of 'later' frag>, but instead
it's bytes=14*<size of assembled packets - size of 'first' + 'later'
frags>.
Fixes: 576126a931cd ("netdev-offload-tc: Add conntrack support")
Signed-off-by: Paul Blakey <paulb@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When OVS installs the flower rule, it only checks for the OK from the
kernel. It does not check if the rule requested matches the one
actually programmed. This change will add this check and warns the
user if this is not the case.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Reviewed-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Previously, only chain 0 is deleted before attach the ingress block.
If there are rules on the chain other than 0, these rules are not flushed.
In this case, the recreation of qdisc also fails. To fix this issue, flush
rules from all chains.
Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
Fix the reported back value of the bos mask used by the revalidator
threads.
Fixes: 34b1695506f8 ("lib/tc: add single mpls match offload support")
Reported-by: Marcelo Leitner <mleitner@redhat.com>
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Check overlap between current pedit key, which is always 4 bytes
(range [off, off + 3]), and a map entry in flower_pedit_map
sf = ROUND_DOWN(mf, 4) (range [sf|mf, (mf + sz - 1)|ef]).
So for the tos the rewite the off + 3(3) is greater than mf,
and should less than ef(4) but not mf+sz(2).
Signed-off-by: wenxu <wenxu@ucloud.cn>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
