delta/openvswitch.git/ofproto, branch master

ofproto-dpif-xlate: Fix use-after-free when xlate_actions().

2023-05-10T19:40:41+00:00

Currently, bundle->cvlans and xbundle->cvlans are pointing to the
same memory location. This can cause issues if the main thread
modifies bundle->cvlans and frees it while the revalidator thread
is still accessing xbundle->cvlans. This leads to use-after-free
error.

AddressSanitizer: heap-use-after-free on address 0x615000007b08 at
                        pc 0x0000004ede1e bp 0x7f3120ee0310 sp 0x7f3120ee0300
READ of size 8 at 0x615000007b08 thread T25 (revalidator25)
    0 0x4ede1d in bitmap_is_set lib/bitmap.h:91
    1 0x4fcb26 in xbundle_allows_cvlan ofproto/ofproto-dpif-xlate.c:2028
    2 0x4fe279 in input_vid_is_valid ofproto/ofproto-dpif-xlate.c:2294
    3 0x502abf in xlate_normal ofproto/ofproto-dpif-xlate.c:3051
    4 0x5164dc in xlate_output_action ofproto/ofproto-dpif-xlate.c:5361
    5 0x522576 in do_xlate_actions ofproto/ofproto-dpif-xlate.c:7047
    6 0x52a751 in xlate_actions ofproto/ofproto-dpif-xlate.c:8061
    7 0x4e2b66 in xlate_key ofproto/ofproto-dpif-upcall.c:2212
    8 0x4e2e13 in xlate_ukey ofproto/ofproto-dpif-upcall.c:2227
    9 0x4e345d in revalidate_ukey__ ofproto/ofproto-dpif-upcall.c:2276
    10 0x4e3f85 in revalidate_ukey ofproto/ofproto-dpif-upcall.c:2395
    11 0x4e7ac5 in revalidate ofproto/ofproto-dpif-upcall.c:2858
    12 0x4d9ed3 in udpif_revalidator ofproto/ofproto-dpif-upcall.c:1010
    13 0x7cd92e in ovsthread_wrapper lib/ovs-thread.c:423
    14 0x7f312ff01f3a  (/usr/lib64/libpthread.so.0+0x8f3a)
    15 0x7f312fc8f51f in clone (/usr/lib64/libc.so.6+0xf851f)

0x615000007b08 is located 8 bytes inside of 512-byte region
                                        [0x615000007b00,0x615000007d00)
freed by thread T0 here:
    0 0x7f3130378ad8 in free (/usr/lib64/libasan.so.4+0xe0ad8)
    1 0x49044e in bundle_set ofproto/ofproto-dpif.c:3431
    2 0x444f92 in ofproto_bundle_register ofproto/ofproto.c:1455
    3 0x40e6c9 in port_configure vswitchd/bridge.c:1300
    4 0x40bcfd in bridge_reconfigure vswitchd/bridge.c:921
    5 0x41f1a9 in bridge_run vswitchd/bridge.c:3313
    6 0x42d4fb in main vswitchd/ovs-vswitchd.c:132
    7 0x7f312fbbcc86 in __libc_start_main (/usr/lib64/libc.so.6+0x25c86)

previously allocated by thread T0 here:
    0 0x7f3130378e70 in __interceptor_malloc
    1 0x8757fe in xmalloc__ lib/util.c:140
    2 0x8758da in xmalloc lib/util.c:175
    3 0x875927 in xmemdup lib/util.c:188
    4 0x475f63 in bitmap_clone lib/bitmap.h:79
    5 0x47797c in vlan_bitmap_clone lib/vlan-bitmap.h:40
    6 0x49048d in bundle_set ofproto/ofproto-dpif.c:3433
    7 0x444f92 in ofproto_bundle_register ofproto/ofproto.c:1455
    8 0x40e6c9 in port_configure vswitchd/bridge.c:1300
    9 0x40bcfd in bridge_reconfigure vswitchd/bridge.c:921
    10 0x41f1a9 in bridge_run vswitchd/bridge.c:3313
    11 0x42d4fb in main vswitchd/ovs-vswitchd.c:132
    12 0x7f312fbbcc86 in __libc_start_main (/usr/lib64/libc.so.6+0x25c86)

Fixes: fed8962aff57 ("Add new port VLAN mode "dot1q-tunnel"")
Signed-off-by: Yunjian Wang 
Signed-off-by: Ilya Maximets

ofproto-dpif-xlate: Always mask ip proto field.

2023-04-06T11:17:15+00:00

The ofproto layer currently treats nw_proto field as overloaded to mean
both that a proper nw layer exists, as well as the value contained in
the header for the nw proto.  However, this is incorrect behavior as
relevant standards permit that any value, including '0' should be treated
as a valid value.

Because of this overload, when the ofproto layer builds action list for
a packet with nw_proto of 0, it won't build the complete action list that
we expect to be built for the packet.  That will cause a bad behavior
where all packets passing the datapath will fall into an incomplete
action set.

The fix here is to unwildcard nw_proto, allowing us to preserve setting
actions for protocols which we know have support for the actions we
program.  This means that a traffic which contains nw_proto == 0 cannot
cause connectivity breakage with other traffic on the link.

Reported-by: David Marchand 
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2134873
Acked-by: Ilya Maximets 
Signed-off-by: Aaron Conole 
Signed-off-by: Ilya Maximets

userspace: Add SRv6 tunnel support.

2023-03-29T20:16:04+00:00

SRv6 (Segment Routing IPv6) tunnel vport is responsible
for encapsulation and decapsulation the inner packets with
IPv6 header and an extended header called SRH
(Segment Routing Header). See spec in:

https://datatracker.ietf.org/doc/html/rfc8754

This patch implements SRv6 tunneling in userspace datapath.
It uses `remote_ip` and `local_ip` options as with existing
tunnel protocols. It also adds a dedicated `srv6_segs` option
to define a sequence of routers called segment list.

Signed-off-by: Nobuhiro MIKI 
Signed-off-by: Ilya Maximets

ofproto-dpif-upcall: Remove redundant time_msec() in revalidate().

2023-03-15T20:23:14+00:00

Remove one of two consecutive time_msec() calls in the revalidate() function.

We take the time stamp after udpif_get_n_flows(), to avoid any potential
delays in getting the number of offloaded flows.

Signed-off-by: Eelco Chaudron 
Reviewed-by: Simon Horman 
Signed-off-by: Ilya Maximets

ofproto-dpif-upcall: Wait for valid hw flow stats before applying min-revalidate-pps.

2023-03-15T20:22:22+00:00

Depending on the driver implementation, it can take from 0.2 seconds
up to 2 seconds before offloaded flow statistics are updated. This is
true for both TC and rte_flow-based offloading. This is causing a
problem with min-revalidate-pps, as old statistic values are used
during this period.

This fix will wait for at least 2 seconds, by default, before assuming no
packets where received during this period.

Reviewed-by: Simon Horman 
Signed-off-by: Eelco Chaudron 
Signed-off-by: Ilya Maximets

ovs-router: Introduce src option in ovs/route/add command.

2023-03-07T17:29:16+00:00

When adding a route with ovs/route/add command, the source address
in "ovs_router_entry" structure is always the FIRST address that the
interface has. See "ovs_router_get_netdev_source_address"
function for more information.

If an interface has multiple ipv4 and/or ipv6 addresses, there are use
cases where the user wants to control the source address. This patch
therefore addresses this issue by adding a src parameter.

Note that same constraints also exist when caching routes from
Kernel FIB with Netlink, but are not dealt with in this patch.

Acked-by: Eelco Chaudron 
Reviewed-by: Simon Horman 
Signed-off-by: Nobuhiro MIKI 
Signed-off-by: Ilya Maximets

ofproto: Fix man page for tunnel related commands.

2023-03-07T17:23:12+00:00

Fixed the manual page to indicate that both IPv4/IPv6
are supported. Also added missing pkt_mark on one side
and fixed the "gw" and "bridge" notation quirks.

Acked-by: Eelco Chaudron 
Reviewed-by: Simon Horman 
Signed-off-by: Nobuhiro MIKI 
Signed-off-by: Ilya Maximets

treewide: Remove uses of ATOMIC_VAR_INIT.

2023-03-06T18:27:19+00:00

ATOMIC_VAR_INIT has a trivial definition
`#define ATOMIC_VAR_INIT(value) (value)`,
is deprecated in C17/C++20, and will be removed in newer standards in
newer GCC/Clang (e.g. https://reviews.llvm.org/D144196).

Signed-off-by: Fangrui Song 
Signed-off-by: Ilya Maximets

ofproto-dpif-upcall: Include hardware offloaded flows in total flows.

2023-03-03T21:27:37+00:00

The revalidator process uses the internal call udpif_get_n_flows()
to get the total number of flows installed in the system. It uses
this value for various decisions on flow installation and removal.
With the tc offload this values is incorrect, as the hardware
offloaded are not included. With rte_flow offload this is not a
problem as dpif netdev keeps both in sync.

This patch will include the hardware offloaded flows if the
underlying dpif implementation is not syncing them.

Reviewed-by: Simon Horman 
Signed-off-by: Eelco Chaudron 
Signed-off-by: Ilya Maximets

ofproto-dpif-upcall: Reset ukey's last stats value if the datapath changed.

2023-03-03T21:27:37+00:00

When the ukey's action set changes, it could cause the flow to use a
different datapath, for example, when it moves from tc to kernel.
This will cause the the cached previous datapath statistics to be used.

This change will reset the cached statistics when a change in
datapath is discovered.

Reviewed-by: Simon Horman 
Signed-off-by: Eelco Chaudron 
Signed-off-by: Ilya Maximets