Travisci fix #3

russellb · 2016-03-07T19:42:24Z

No description provided.

Trying to revert the changes caused by 8520dee to travis build failures Signed-off-by: Babu Shanmugam <[email protected]>

This reverts commit 337bebe, which caused a crash in test 1048 "ofproto-dpif - Flow IPFIX sanity check" (now test 1051) with the following backtrace: #0 hmap_first_with_hash (hmap=<optimized out>, hmap=<optimized out>, hash=<optimized out>) at ../lib/hmap.h:328 #1 smap_find__ (smap=0x94, key=key@entry=0x817f7ab "virtual_obs_id", key_len=14, hash=2537071222) at ../lib/smap.c:366 #2 0x0812b9d7 in smap_get_node (smap=0x9738a276, key=0x817f7ab "virtual_obs_id") at ../lib/smap.c:198 #3 0x0812ba30 in smap_get (smap=0x94, key=0x817f7ab "virtual_obs_id") at ../lib/smap.c:189 #4 0x08055a60 in bridge_configure_ipfix (br=<optimized out>) at ../vswitchd/bridge.c:1237 #5 bridge_reconfigure (ovs_cfg=0x94) at ../vswitchd/bridge.c:666 openvswitch#6 0x080568d3 in bridge_run () at ../vswitchd/bridge.c:2972 #7 0x0804c9dd in main (argc=10, argv=0xffd8b934) at ../vswitchd/ovs-vswitchd.c:112 Signed-off-by: Ben Pfaff <[email protected]>

New 'other_config:pmd-rxq-affinity' field for Interface table to perform manual pinning of RX queues to desired cores. This functionality is required to achieve maximum performance because all kinds of ports have different cost of rx/tx operations and only user can know about expected workload on different ports. Example: # ./bin/ovs-vsctl set interface dpdk0 options:n_rxq=4 \ other_config:pmd-rxq-affinity="0:3,1:7,3:8" Queue #0 pinned to core 3; Queue #1 pinned to core 7; Queue #2 not pinned. Queue #3 pinned to core 8; It's decided to automatically isolate cores that have rxq explicitly assigned to them because it's useful to keep constant polling rate on some performance critical ports while adding/deleting other ports without explicit pinning of all ports. Signed-off-by: Ilya Maximets <[email protected]> Signed-off-by: Daniele Di Proietto <[email protected]>

This patch sets *typep to an empty string instead of letting it uninitialized when no QoS configuration is set. It fixes the following vswitchd crash when no QoS has been set on vhost-user interface: $> ovs-appctl -t ovs-vswitchd qos/show vhost-user1 #0 0x00007efcbadf18d7 in raise () from /lib64/libc.so.6 #1 0x00007efcbadf353a in abort () from /lib64/libc.so.6 #2 0x000000000068d5be in ovs_abort_valist at lib/util.c:335 #3 0x0000000000693d90 in vlog_abort_valist at lib/vlog.c:1204 #4 0x0000000000693e17 in vlog_abort at lib/vlog.c:1218 #5 0x000000000068d3ae in ovs_assert_failure at lib/util.c:72 openvswitch#6 0x000000000060425c in ds_put_format_valist at lib/dynamic-string.c:168 #7 0x00000000006042e7 in ds_put_format at lib/dynamic-string.c:142 #8 0x00000000005a9e75 in qos_unixctl_show at vswitchd/bridge.c:3185 #9 0x000000000068cda1 in process_command at lib/unixctl.c:347 #11 unixctl_server_run at lib/unixctl.c:400 #12 0x000000000040a3ff in main at vswitchd/ovs-vswitchd.c:113 Signed-off-by: Maxime Coquelin <[email protected]> Acked-by: Ian Stokes <[email protected]> Signed-off-by: Daniele Di Proietto <[email protected]>

The newly added replication logic makes it possible for a monitor to receive delete and insertion of the same row back to back, which was not possible before. Add logic (and comment) to handle this case to avoid follow crash reported by Valgrind: #0 0x0000000000453edd in ovsdb_datum_compare_3way (a=0x5efbe60, b=0x0, type=0x5e6a848) at lib/ovsdb-data.c:1626 #1 0x0000000000453ea4 in ovsdb_datum_equals (a=0x5efbe60, b=0x0, type=0x5e6a848) at lib/ovsdb-data.c:1616 #2 0x000000000041b651 in update_monitor_row_data (mt=0x5eda4a0, row=0x5efbe00, data=0x0) at ovsdb/monitor.c:310 #3 0x000000000041ed14 in ovsdb_monitor_changes_update (old=0x0, new=0x5efbe00, mt=0x5eda4a0, changes=0x5ef7180) at ovsdb/monitor.c:1255 #4 0x000000000041f12e in ovsdb_monitor_change_cb (old=0x0, new=0x5efbe00, changed=0x5efc218, aux_=0xffefff040) at ovsdb/monitor.c:1339 #5 0x000000000042ded9 in ovsdb_txn_for_each_change (txn=0x5efbd90, cb=0x41ef50 <ovsdb_monitor_change_cb>, aux=0xffefff040) at ovsdb/transaction.c:906 openvswitch#6 0x0000000000420155 in ovsdb_monitor_commit (replica=0x5eda2c0, txn=0x5efbd90, durable=false) at ovsdb/monitor.c:1553 #7 0x000000000042dc04 in ovsdb_txn_commit_ (txn=0x5efbd90, durable=false) at ovsdb/transaction.c:868 #8 0x000000000042ddd4 in ovsdb_txn_commit (txn=0x5efbd90, durable=false) at ovsdb/transaction.c:893 #9 0x0000000000422e0c in process_notification (table_updates=0x5efad10, db=0x5e6bd40) at ovsdb/replication.c:575 #10 0x0000000000420ff3 in replication_run () at ovsdb/replication.c:184 #11 0x0000000000405cc8 in main_loop (jsonrpc=0x5e67770, all_dbs=0xffefff3a0, unixctl=0x5ebd980, remotes=0xffefff360, run_process=0x0, exiting=0xffefff3c0, is_backup=0xffefff2de) at ovsdb/ovsdb-server.c:198 #12 0x0000000000406edb in main (argc=1, argv=0xffefff550) at ovsdb/ovsdb-server.c:429 Reported-by: Joe Stringer <[email protected]> Reported-at: http://openvswitch.org/pipermail/dev/2016-September/079315.html Reported-by: Alin Serdean <[email protected]> Reported-at: http://openvswitch.org/pipermail/dev/2016-September/079586.html Co-authored-by: Joe Stringer <[email protected]> Signed-off-by: Andy Zhou <[email protected]> Acked-by: Ben Pfaff <[email protected]>

dp_netdev_get_pmd() is allowed to return NULL (even if we call it with NON_PMD_CORE_ID) for different reasons: * Since we use RCU to protect pmd threads, it is possible that ovs_refcount_try_ref_rcu() has failed. * During reconfiguration we destroy every thread. This commit makes sure that we always handle the case when dp_netdev_get_pmd() returns NULL without crashing (the change in dpif_netdev_run() doesn't fix anything, because everything is happening in the main thread, but it's better to honor the interface in case we change our threading model). This actually fixes a pretty serious crash that happens if dpif_netdev_execute() is called from a non pmd thread while reconfiguration is happening. It can be triggered by enabling bfd (because it's handled by the monitor thread, which is a non pmd thread) on an interface and changing something that requires datapath reconfiguration (n_rxq, pmd-cpu-mask, mtu). A testcase that reproduces the race condition is included. This is a possible backtrace of the segfault: #0 0x000000000060c7f1 in dp_execute_cb (aux_=0x7f1dd2d2a320, packets_=0x7f1dd2d2a370, a=0x7f1dd2d2a658, may_steal=false) at ../lib/dpif-netdev.c:4357 #1 0x00000000006448b2 in odp_execute_actions (dp=0x7f1dd2d2a320, batch=0x7f1dd2d2a370, steal=false, actions=0x7f1dd2d2a658, actions_len=8, dp_execute_action=0x60c7a5 <dp_execute_cb>) at ../lib/odp-execute.c:538 #2 0x000000000060d00c in dp_netdev_execute_actions (pmd=0x0, packets=0x7f1dd2d2a370, may_steal=false, flow=0x7f1dd2d2ae70, actions=0x7f1dd2d2a658, actions_len=8, now=44965873) at ../lib/dpif-netdev.c:4577 #3 0x000000000060834a in dpif_netdev_execute (dpif=0x2b67b70, execute=0x7f1dd2d2a578) at ../lib/dpif-netdev.c:2624 #4 0x0000000000608441 in dpif_netdev_operate (dpif=0x2b67b70, ops=0x7f1dd2d2a5c8, n_ops=1) at ../lib/dpif-netdev.c:2654 #5 0x0000000000610a30 in dpif_operate (dpif=0x2b67b70, ops=0x7f1dd2d2a5c8, n_ops=1) at ../lib/dpif.c:1268 openvswitch#6 0x000000000061098c in dpif_execute (dpif=0x2b67b70, execute=0x7f1dd2d2aa50) at ../lib/dpif.c:1233 #7 0x00000000005b9008 in ofproto_dpif_execute_actions__ (ofproto=0x2b69360, version=18446744073709551614, flow=0x7f1dd2d2ae70, rule=0x0, ofpacts=0x7f1dd2d2b100, ofpacts_len=16, indentation=0, depth=0, resubmits=0, packet=0x7f1dd2d2b5c0) at ../ofproto/ofproto-dpif.c:3806 #8 0x00000000005b907a in ofproto_dpif_execute_actions (ofproto=0x2b69360, version=18446744073709551614, flow=0x7f1dd2d2ae70, rule=0x0, ofpacts=0x7f1dd2d2b100, ofpacts_len=16, packet=0x7f1dd2d2b5c0) at ../ofproto/ofproto-dpif.c:3823 #9 0x00000000005dea9b in xlate_send_packet (ofport=0x2b98380, oam=false, packet=0x7f1dd2d2b5c0) at ../ofproto/ofproto-dpif-xlate.c:5792 #10 0x00000000005bab12 in ofproto_dpif_send_packet (ofport=0x2b98380, oam=false, packet=0x7f1dd2d2b5c0) at ../ofproto/ofproto-dpif.c:4628 #11 0x00000000005c3fc8 in monitor_mport_run (mport=0x2b8cd00, packet=0x7f1dd2d2b5c0) at ../ofproto/ofproto-dpif-monitor.c:287 #12 0x00000000005c3d9b in monitor_run () at ../ofproto/ofproto-dpif-monitor.c:227 #13 0x00000000005c3cab in monitor_main (args=0x0) at ../ofproto/ofproto-dpif-monitor.c:189 #14 0x00000000006a183a in ovsthread_wrapper (aux_=0x2b8afd0) at ../lib/ovs-thread.c:342 #15 0x00007f1dd75eb444 in start_thread (arg=0x7f1dd2d2c700) at pthread_create.c:333 openvswitch#16 0x00007f1dd6e1d20d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109 Signed-off-by: Daniele Di Proietto <[email protected]> Acked-by: Ben Pfaff <[email protected]>

When the datapath, whose type is "netdev", processes packets in userspce action, it may cause a segmentation fault. In the dp_execute_userspace_action(), we pass the "wc" argument to dp_netdev_upcall() using NULL. In the dp_netdev_upcall() call tree, the "wc" will be used. For example, dp_netdev_upcall() uses the &wc->masks for debugging, and flow_wildcards_init_for_packet() uses the "wc" if we disable megaflow, which is described in more detail below. Segmentation fault in flow_wildcards_init_for_packet: #0 0x0000000000468fe8 flow_wildcards_init_for_packet lib/flow.c:1275 #1 0x0000000000436c0b upcall_cb ofproto/ofproto-dpif-upcall.c:1231 #2 0x000000000045bd96 dp_netdev_upcall lib/dpif-netdev.c:3857 #3 0x0000000000461bf3 dp_execute_userspace_action lib/dpif-netdev.c:4388 #4 dp_execute_cb lib/dpif-netdev.c:4521 #5 0x0000000000486ae2 odp_execute_actions lib/odp-execute.c:538 openvswitch#6 0x00000000004607f9 dp_netdev_execute_actions lib/dpif-netdev.c:4627 #7 packet_batch_per_flow_execute lib/dpif-netdev.c:3927 #8 dp_netdev_input__ lib/dpif-netdev.c:4229 #9 0x0000000000460ba8 dp_netdev_input lib/dpif-netdev.c:4238 #10 dp_netdev_process_rxq_port lib/dpif-netdev.c:2873 #11 0x000000000046126e dpif_netdev_run lib/dpif-netdev.c:3000 #12 0x000000000042baf5 type_run ofproto/ofproto-dpif.c:504 #13 0x00000000004192bf ofproto_type_run ofproto/ofproto.c:1687 #14 0x0000000000409965 bridge_run__ vswitchd/bridge.c:2875 #15 0x000000000040f145 bridge_run vswitchd/bridge.c:2938 openvswitch#16 0x00000000004062e5 main vswitchd/ovs-vswitchd.c:111 Signed-off-by: nickcooper-zhangtonghao <[email protected]> Signed-off-by: Daniele Di Proietto <[email protected]>

nx_put_match() needs a non-NULL tunnel metadata table, otherwise it will crash if a flow matches on tunnel metadata. This wasn't handled in ofputil_append_flow_update(), causing a crash when the controller sent a flow monitor request. To fix the problem, this commit changes ofputil_append_flow_update() to behave like ofputil_append_flow_stats_reply(). Since ofputil_append_flow_update() now needs to temporarily modify the match, this commits also embeds 'struct match' into 'struct ofputil_flow_update', to be safer. This is more similar to 'struct ofputil_flow_stats'. A regression test is added and a comment is updated in ovs-ofctl.c #0 0x000055699bd82fa0 in memcpy_from_metadata (dst=0x7ffc770930d0, src=0x7ffc77093698, loc=0x18) at ../lib/tun-metadata.c:451 #1 0x000055699bd83c2e in metadata_loc_from_match_read (map=0x0, match=0x7ffc77093410, idx=0, mask=0x7ffc77093658, is_masked=0x7ffc77093287) at ../lib/tun-metadata.c:848 #2 0x000055699bd83d9b in tun_metadata_to_nx_match (b=0x55699d3f0300, oxm=0, match=0x7ffc77093410) at ../lib/tun-metadata.c:871 #3 0x000055699bce523d in nx_put_raw (b=0x55699d3f0300, oxm=0, match=0x7ffc77093410, cookie=0, cookie_mask=0) at ../lib/nx-match.c:1052 #4 0x000055699bce5580 in nx_put_match (b=0x55699d3f0300, match=0x7ffc77093410, cookie=0, cookie_mask=0) at ../lib/nx-match.c:1116 #5 0x000055699bd3926f in ofputil_append_flow_update (update=0x7ffc770940b0, replies=0x7ffc77094e00) at ../lib/ofp-util.c:6805 openvswitch#6 0x000055699bc4b5a9 in ofproto_compose_flow_refresh_update (rule=0x55699d405b40, flags=(NXFMF_INITIAL | NXFMF_ACTIONS), msgs=0x7ffc77094e00) at ../ofproto/ofproto.c:5915 #7 0x000055699bc4b5f6 in ofmonitor_compose_refresh_updates (rules=0x7ffc77094e10, msgs=0x7ffc77094e00) at ../ofproto/ofproto.c:5929 #8 0x000055699bc4bafc in handle_flow_monitor_request (ofconn=0x55699d404090, oh=0x55699d404220) at ../ofproto/ofproto.c:6082 #9 0x000055699bc4f46d in handle_openflow__ (ofconn=0x55699d404090, msg=0x55699d404910) at ../ofproto/ofproto.c:7912 #10 0x000055699bc4f5df in handle_openflow (ofconn=0x55699d404090, ofp_msg=0x55699d404910) at ../ofproto/ofproto.c:8002 #11 0x000055699bc88154 in ofconn_run (ofconn=0x55699d404090, handle_openflow=0x55699bc4f5bc <handle_openflow>) at ../ofproto/connmgr.c:1427 #12 0x000055699bc85934 in connmgr_run (mgr=0x55699d3adb90, handle_openflow=0x55699bc4f5bc <handle_openflow>) at ../ofproto/connmgr.c:363 #13 0x000055699bc422c9 in ofproto_run (p=0x55699d3c85e0) at ../ofproto/ofproto.c:1798 #14 0x000055699bc31ec6 in bridge_run__ () at ../vswitchd/bridge.c:2881 #15 0x000055699bc320a6 in bridge_run () at ../vswitchd/bridge.c:2938 openvswitch#16 0x000055699bc3784e in main (argc=10, argv=0x7ffc770952c8) at ../vswitchd/ovs-vswitchd.c:111 Fixes: 8d8ab6c ("tun-metadata: Manage tunnel TLV mapping table on a per-bridge basis.") Signed-off-by: Daniele Di Proietto <[email protected]> Acked-by: Ben Pfaff <[email protected]>

revalidator_sweep__() splits checking for whether to delete a ukey from the actual deletion to prevent taking the umap lock for too long. However it uses information gathered from the first critical section to decide to call ukey_delete() - ie, the second critical section. Since 67f0898 ("upcall: Replace ukeys for deleted flows."), it is possible for a handler thread to receive an upcall for the same flow and to replace the ukey which is being deleted with a new one, in between these critical sections. This will remove the ukey from the cmap, rcu-defer its deletion, and update the ukey state. If this occurs in between the critical sections of revalidator cleanup of the flow, then the revalidator will subsequently call ukey_delete() to delete the original ukey, which was already deleted by the handler thread. This leads to a segfault in cmap_replace__(). Guard against this by checking the ukey state in ukey_delete() while holding the ukey lock. Backtrace: Program terminated with signal 11, Segmentation fault. #0 0x00007fe969b13da3 in cmap_replace__ () #1 0x00007fe969b14491 in cmap_replace () #2 0x00007fe969aee9ff in ukey_delete () #3 0x00007fe969aefd42 in revalidator_sweep__ () #4 0x00007fe969af1bad in udpif_revalidator () #5 0x00007fe969b8b2a6 in ovsthread_wrapper () openvswitch#6 0x00007fe968e07dc5 in start_thread () from /lib64/libpthread.so.0 #7 0x00007fe96862c73d in clone () from /lib64/libc.so.6 Fixes: 54ebeff ("upcall: Track ukey states.") Fixes: 67f0898 ("upcall: Replace ukeys for deleted flows.") Reported-by: Numan Siddique <[email protected]> Signed-off-by: Joe Stringer <[email protected]> Acked-by: Jarno Rajahalme <[email protected]>

When we use the 'ovs-appctl ofproto/trace' to send packets, which include the 'vlan' field, but exclude the 'encap', the ovs-vswitchd will crash. We should check 'encap' field in parse_8021q_onward(), before using it. ovs-appctl ofproto/trace ovs-system \ 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07), eth_type(0x8100),vlan(vid=99,pcp=0)' #0 nl_attr_get_size (nla=nla@entry=0x0) at lib/netlink.c:567 #1 parse_8021q_onward (src_flow=0x7ffd0ec77540, key_len=40, key=0x1207e00, flow=0x7ffd0ec77540, expected_attrs=<optimized out>, out_of_range_attr=0, present_attrs=120, attrs=0x7ffd0ec77170) at lib/odp-util.c:5359 #2 odp_flow_key_to_flow__ (key=0x1207e00, key_len=40, flow=flow@entry=0x7ffd0ec77540, src_flow=src_flow@entry=0x7ffd0ec77540) at lib/odp-util.c:5520 #3 odp_flow_key_to_flow (key=<optimized out>, key_len=<optimized out>, flow=flow@entry=0x7ffd0ec77540) at lib/odp-util.c:5555 #4 parse_flow_and_packet (argc=3, argv=0x12b2220, ofprotop=ofprotop@entry=0x7ffd0ec77510, flow=flow@entry=0x7ffd0ec77540, packetp=packetp@entry=0x7ffd0ec77518) at ofproto/ofproto-dpif-trace.c:211 #5 ofproto_unixctl_trace (conn=0x1268c20, argc=<optimized out>, argv=<optimized out>, aux=<optimized out>) at ofproto/ofproto-dpif-trace.c:309 openvswitch#6 process_command (request=<optimized out>, conn=0x1268c20) at lib/unixctl.c:313 #7 run_connection (conn=0x1268c20) at lib/unixctl.c:347 #8 unixctl_server_run (server=0x1180970) at lib/unixctl.c:400 #9 main (argc=5, argv=0x7ffd0ec779c8) at vswitchd/ovs-vswitchd.c:120 Signed-off-by: nickcooper-zhangtonghao <[email protected]> Acked-by: Eric Garver <[email protected]> Signed-off-by: Joe Stringer <[email protected]>

IP fragmentation engine may not only steal the packet but also add more. For example, after receiving the last fragment, it will add all previous fragments to a batch. Unfortunately, it will also free the original last fragment and replace it with a copy. This invalidates the 'packet_clone' pointer in the dpif_netdev_execute() leading to the use-after-free: ==3525086==ERROR: AddressSanitizer: heap-use-after-free on address 0x61600020439c at pc 0x000000688a6d READ of size 1 at 0x61600020439c thread T0 #0 0x688a6c in dp_packet_swap ./lib/dp-packet.h:265:5 #1 0x68781d in dpif_netdev_execute lib/dpif-netdev.c:4103:9 #2 0x6675db in dpif_netdev_operate lib/dpif-netdev.c:4129:25 #3 0x691e5e in dpif_operate lib/dpif.c:1367:13 #4 0x692909 in dpif_execute lib/dpif.c:1321:9 #5 0x5b19c6 in packet_execute ofproto/ofproto-dpif.c:4991:5 openvswitch#6 0x5a2861 in ofproto_packet_out_finish ofproto/ofproto.c:3662:5 #7 0x5a65c6 in do_bundle_commit ofproto/ofproto.c:8270:13 #8 0x5a0cae in handle_bundle_control ofproto/ofproto.c:8309:17 #9 0x59a476 in handle_single_part_openflow ofproto/ofproto.c:8593:16 #10 0x5877ac in handle_openflow ofproto/ofproto.c:8674:21 #11 0x6296f1 in ofconn_run ofproto/connmgr.c:1329:13 #12 0x62925d in connmgr_run ofproto/connmgr.c:356:9 #13 0x586904 in ofproto_run ofproto/ofproto.c:1879:5 #14 0x55c830 in bridge_run__ vswitchd/bridge.c:3251:9 #15 0x55c015 in bridge_run vswitchd/bridge.c:3310:5 openvswitch#16 0x575f31 in main vswitchd/ovs-vswitchd.c:127:9 #17 0x7f01099d3492 in __libc_start_main (/lib64/libc.so.6+0x23492) openvswitch#18 0x47d96d in _start (vswitchd/ovs-vswitchd+0x47d96d) 0x61600020439c is located 28 bytes inside of 560-byte region freed by thread T0 here: #0 0x5177a8 in free (vswitchd/ovs-vswitchd+0x5177a8) #1 0x6b17b6 in dp_packet_delete ./lib/dp-packet.h:256:9 #2 0x6afeee in ipf_extract_frags_from_batch lib/ipf.c:947:17 #3 0x6afd63 in ipf_preprocess_conntrack lib/ipf.c:1232:9 #4 0x946b2c in conntrack_execute lib/conntrack.c:1446:5 #5 0x67e3ed in dp_execute_cb lib/dpif-netdev.c:8277:9 openvswitch#6 0x7097d7 in odp_execute_actions lib/odp-execute.c:865:17 #7 0x66409e in dp_netdev_execute_actions lib/dpif-netdev.c:8322:5 #8 0x6877ad in dpif_netdev_execute lib/dpif-netdev.c:4090:5 #9 0x6675db in dpif_netdev_operate lib/dpif-netdev.c:4129:25 #10 0x691e5e in dpif_operate lib/dpif.c:1367:13 #11 0x692909 in dpif_execute lib/dpif.c:1321:9 #12 0x5b19c6 in packet_execute ofproto/ofproto-dpif.c:4991:5 #13 0x5a2861 in ofproto_packet_out_finish ofproto/ofproto.c:3662:5 #14 0x5a65c6 in do_bundle_commit ofproto/ofproto.c:8270:13 #15 0x5a0cae in handle_bundle_control ofproto/ofproto.c:8309:17 openvswitch#16 0x59a476 in handle_single_part_openflow ofproto/ofproto.c:8593:16 #17 0x5877ac in handle_openflow ofproto/ofproto.c:8674:21 openvswitch#18 0x6296f1 in ofconn_run ofproto/connmgr.c:1329:13 #19 0x62925d in connmgr_run ofproto/connmgr.c:356:9 #20 0x586904 in ofproto_run ofproto/ofproto.c:1879:5 #21 0x55c830 in bridge_run__ vswitchd/bridge.c:3251:9 openvswitch#22 0x55c015 in bridge_run vswitchd/bridge.c:3310:5 openvswitch#23 0x575f31 in main vswitchd/ovs-vswitchd.c:127:9 #24 0x7f01099d3492 in __libc_start_main (/lib64/libc.so.6+0x23492) The issue can be reproduced with system-userspace testsuite on the 'conntrack - IPv4 fragmentation with fragments specified' test. Previously, there was a leak inside the IP fragmentation module that kept the original segment, so 'packet_clone' remained a valid pointer. But commit 803ed12 ("ipf: release unhandled packets from the batch") fixed the leak leading to use-after-free. Using the packet from a batch instead of 'packet_clone' to swap packet content to avoid the issue. While investigating this problem, more issues uncovered. One of them is that IP fragmentation engine can add more packets to the batch, but there is no way to get them to a caller. Adding an extra branch for this case with a 'FIXME' comment in order to highlight the issue. Another one is that IP fragmentation engine will keep only 32 fragments dropping all other fragments while refilling a batch, but that should be fixed separately. Fixes: 7e6b41a ("dpif-netdev: Fix crash when PACKET_OUT is metered.") Signed-off-by: Ilya Maximets <[email protected]> Acked-by: Aaron Conole <[email protected]>

This commit improves handling of packets where the allocated memory is less than 64 bytes. For packets recevied from DPDK ports this never matters, as an mbuf always pre-allocates enough space, however this can occur in cases where packet received from a kernel interface or injected by an OpenFlow controller. The fix is required to ensure OVS doesn't overread the allocated memory, e.g.: ==49944==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6060000d8181 at pc 0x000001cb9d24 bp 0x7ffce3b385d0 sp 0x7ffce3b385c8 READ of size 64 at 0x6060000d8181 thread T0 #0 0x1cb9d23 in mfex_avx512_process lib/dpif-netdev-extract-avx512.c:491:26 #1 0x1cb9d23 in mfex_avx512_ip_udp lib/dpif-netdev-extract-avx512.c:625:1 #2 0x18786a1 in dpif_miniflow_extract_autovalidator lib/dpif-netdev-private-extract.c:277:29 #3 0x1cbca5c in dp_netdev_input_outer_avx512 lib/dpif-netdev-avx512.c:159:19 #4 0x1853048 in dp_netdev_process_rxq_port lib/dpif-netdev.c:4900:19 #5 0x1837c76 in dpif_netdev_run lib/dpif-netdev.c:6197:25 openvswitch#6 0x1727a02 in type_run ofproto/ofproto-dpif.c:370:9 #7 0x16f6e07 in ofproto_type_run ofproto/ofproto.c:1778:31 #8 0x16c1a8b in bridge_run__ vswitchd/bridge.c:3245:9 #9 0x16bd2fd in bridge_run vswitchd/bridge.c:3310:5 #10 0x16db8fe in main vswitchd/ovs-vswitchd.c:127:9 #11 0x7fbc0c5b61a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) #12 0xedabbd in _start (vswitchd/ovs-vswitchd+0xedabbd) 0x6060000d8181 is located 9 bytes to the right of 56-byte region [0x6060000d8140,0x6060000d8178) allocated by thread T0 here: #0 0xf7b09f in malloc (vswitchd/ovs-vswitchd+0xf7b09f) #1 0x1aff3b9 in xmalloc__ lib/util.c:137:15 #2 0x1aff3b9 in xmalloc lib/util.c:172:12 #3 0x1afe211 in process_command lib/unixctl.c:310:13 #4 0x1afe211 in run_connection lib/unixctl.c:344:17 #5 0x1afe211 in unixctl_server_run lib/unixctl.c:395:21 openvswitch#6 0x16db918 in main vswitchd/ovs-vswitchd.c:128:9 #7 0x7fbc0c5b61a2 in __libc_start_main (/lib64/libc.so.6+0x271a2) The solution implemented uses a mask-to-zero if the available buffer size is less than 64 bytes, and a branch for which type of load is used. Fixes: 250cedd ("dpif-netdev/mfex: Add AVX512 based optimized miniflow extract") Reported-by: Ilya Maximets <[email protected]> Acked-by: Eelco Chaudron <[email protected]> Signed-off-by: Harry van Haaren <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>

Found by AddressSanitizer when running OVN tests: Direct leak of 64 byte(s) in 1 object(s) allocated from: #0 0x498fb2 in calloc (/ic/ovn-ic+0x498fb2) #1 0x5f681e in xcalloc__ ovs/lib/util.c:121:31 #2 0x5f681e in xzalloc__ ovs/lib/util.c:131:12 #3 0x5f681e in xzalloc ovs/lib/util.c:165:12 #4 0x5e3697 in ovsdb_idl_txn_add_map_op ovs/lib/ovsdb-idl.c:4057:29 #5 0x4d3f25 in update_isb_pb_external_ids ic/ovn-ic.c:576:5 openvswitch#6 0x4cc4cc in create_isb_pb ic/ovn-ic.c:716:5 #7 0x4cc4cc in port_binding_run ic/ovn-ic.c:803:21 #8 0x4cc4cc in ovn_db_run ic/ovn-ic.c:1700:5 #9 0x4c9c1c in main ic/ovn-ic.c:1984:17 #10 0x7f9ad9f4a0b2 in __libc_start_main Signed-off-by: Dumitru Ceara <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>

Remove the forced cache-line size alignment markers from struct dp_netdev_pmd_thread and struct dp_netdev as discussed at [0]. They don't seem to add any benefit and cause 64 byte alignment requirements. UB Sanitizer report: lib/dpif-netdev.c:6758:13: runtime error: member access within misaligned address 0x7f7f24d25010 for type 'struct dp_netdev_pmd_thread', which requires 64 byte alignment 0x7f7f24d25010: note: pointer points here 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ... ^ #0 0x5fbfde in dp_netdev_configure_pmd lib/dpif-netdev.c:6758 #1 0x5fbde9 in dp_netdev_set_nonpmd lib/dpif-netdev.c:6715 #2 0x5d6fdd in create_dp_netdev lib/dpif-netdev.c:1769 #3 0x5d72d0 in dpif_netdev_open lib/dpif-netdev.c:1807 #4 0x61c83f in do_open lib/dpif.c:347 [...] lib/dpif-netdev.c:1724:6: runtime error: member access within misaligned address 0x000002005eb0 for type 'struct dp_netdev', which requires 64 byte alignment 0x000002005eb0: note: pointer points here 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ... ^ #0 0x5d6660 in create_dp_netdev lib/dpif-netdev.c:1724 #1 0x5d72d0 in dpif_netdev_open lib/dpif-netdev.c:1807 #2 0x61c846 in do_open lib/dpif.c:347 #3 0x61ca9c in dpif_create lib/dpif.c:402 #4 0x61cac9 in dpif_create_and_open lib/dpif.c:415 #5 0x48f235 in open_dpif_backer ofproto/ofproto-dpif.c:776 [...] [0] https://mail.openvswitch.org/pipermail/ovs-dev/2021-December/390256.html Suggested-by: Ilya Maximets <[email protected]> Signed-off-by: Dumitru Ceara <[email protected]> Acked-by: Paolo Valerio <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>

UB Sanitizer report: lib/stopwatch.c:119:22: runtime error: index 18446744073709551615 out of bounds for type 'long long unsigned int [50]' #0 0x698358 in calc_percentile lib/stopwatch.c:119 #1 0x69ada1 in add_sample lib/stopwatch.c:231 #2 0x69c086 in stopwatch_end_sample_protected lib/stopwatch.c:386 #3 0x69c522 in stopwatch_thread lib/stopwatch.c:441 #4 0x684bae in ovsthread_wrapper lib/ovs-thread.c:383 #5 0x7f042838b298 in start_thread (/lib64/libpthread.so.0+0x9298) openvswitch#6 0x7f04277f2352 in clone (/lib64/libc.so.6+0x100352) Acked-by: Aaron Conole <[email protected]> Signed-off-by: Dumitru Ceara <[email protected]> Acked-by: Paolo Valerio <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>

Reported by UndefinedBehaviorSanitizer: tests/idltest.c:3602:12: runtime error: member access within null pointer of type 'const struct idltest_simple' #0 0x4295af in idltest_simple_cursor_first_ge tests/idltest.c:3602 #1 0x41c81b in test_idl_compound_index_single_column tests/test-ovsdb.c:3128 #2 0x41e035 in do_idl_compound_index tests/test-ovsdb.c:3277 #3 0x4cf640 in ovs_cmdl_run_command__ lib/command-line.c:247 #4 0x4cf79f in ovs_cmdl_run_command lib/command-line.c:278 #5 0x4072f7 in main tests/test-ovsdb.c:79 openvswitch#6 0x7fa858675b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) #7 0x4060ed in _start (/root/ovs/tests/test-ovsdb+0x4060ed) Acked-by: Aaron Conole <[email protected]> Signed-off-by: Dumitru Ceara <[email protected]> Acked-by: Paolo Valerio <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>

UB Sanitizer reports: lib/bfd.c:748:16: runtime error: member access within misaligned address 0x000001f0d6ea for type 'struct msg', which requires 4 byte alignment 0x000001f0d6ea: note: pointer points here 00 20 00 00 20 40 03 18 93 f9 0a 6e 00 00 00 00 00 0f 42 40 00 0f ... ^ #0 0x59008e in bfd_process_packet lib/bfd.c:748 #1 0x52a240 in process_special ofproto/ofproto-dpif-xlate.c:3370 #2 0x553452 in xlate_actions ofproto/ofproto-dpif-xlate.c:7766 #3 0x4fc9e6 in upcall_xlate ofproto/ofproto-dpif-upcall.c:1237 #4 0x4fdecc in process_upcall ofproto/ofproto-dpif-upcall.c:1456 #5 0x4fd936 in upcall_cb ofproto/ofproto-dpif-upcall.c:1358 [...] lib/stp.c:754:15: runtime error: member access within misaligned address 0x000002c4ea61 for type 'const struct stp_bpdu_header', which requires 2 byte alignment 0x000002c4ea61: note: pointer points here 26 42 42 03 00 00 00 00 00 80 00 aa 66 aa 66 00 01 00 00 00 00 80 ... ^ #0 0x8a2bce in stp_received_bpdu lib/stp.c:754 #1 0x51e603 in stp_process_packet ofproto/ofproto-dpif-xlate.c:1788 #2 0x52a96d in process_special ofproto/ofproto-dpif-xlate.c:3394 #3 0x5534df in xlate_actions ofproto/ofproto-dpif-xlate.c:7766 #4 0x4fcb49 in upcall_xlate ofproto/ofproto-dpif-upcall.c:1237 [...] lib/lldp/lldp.c:149:10: runtime error: load of misaligned address 0x7ffcc0ae72bd for type 'ovs_be16', which requires 2 byte alignment 0x7ffcc0ae72bd: note: pointer points here 8e e7 84 ad 04 00 05 46 61 73 74 45 74 68 65 72 6e 65 74 20 31 2f 35 ... ^ #0 0x718d63 in lldp_tlv_end lib/lldp/lldp.c:149 #1 0x7191de in lldp_send lib/lldp/lldp.c:184 #2 0x484d6c in test_aa_send tests/test-aa.c:238 [...] Acked-by: Aaron Conole <[email protected]> Signed-off-by: Dumitru Ceara <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>

UB Sanitizer report: lib/dp-packet.h:297:39: runtime error: applying zero offset to null pointer #0 0x7946f5 in dp_packet_tail ./lib/dp-packet.h:297:39 #1 0x794331 in dp_packet_tailroom ./lib/dp-packet.h:325:49 #2 0x7942a0 in dp_packet_prealloc_tailroom lib/dp-packet.c:297:16 #3 0xc347cf in eth_compose lib/packets.c:1061:5 [...] Signed-off-by: Dumitru Ceara <[email protected]> Acked-by: Paolo Valerio <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>

When compiled with clang and '-fsanitize=undefined' set, running 'ovsdb-client --timestamp monitor Open_vSwitch' in a sandbox triggers the following undefined behavior (flagged by UBSan): lib/dynamic-string.c:207:38: runtime error: applying zero offset to null pointer #0 0x4ebc18 in ds_put_strftime_msec lib/dynamic-string.c:207:38 #1 0x4ebd04 in xastrftime_msec lib/dynamic-string.c:225:5 #2 0x552e6a in table_format_timestamp__ lib/table.c:226:12 #3 0x552852 in table_print_timestamp__ lib/table.c:233:27 #4 0x5506f3 in table_print_table__ lib/table.c:254:5 #5 0x550633 in table_format lib/table.c:601:9 openvswitch#6 0x5524f3 in table_print lib/table.c:633:5 #7 0x44dc5e in monitor_print_table ovsdb/ovsdb-client.c:1019:5 #8 0x44c650 in monitor_print ovsdb/ovsdb-client.c:1040:13 #9 0x44ac56 in do_monitor__ ovsdb/ovsdb-client.c:1500:21 #10 0x44636e in do_monitor ovsdb/ovsdb-client.c:1575:5 #11 0x442c41 in main ovsdb/ovsdb-client.c:283:5 Reported-by: Ilya Maximets <[email protected]> Signed-off-by: Dumitru Ceara <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>

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 openvswitch#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: 526df7d ("tunnel: Provide framework for tunnel extensions for VXLAN-GBP and others") Acked-by: Mike Pattrick <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>

UB Sanitizer report: lib/dp-packet.h:587:22: runtime error: member access within misaligned address 0x000001ecde10 for type 'struct dp_packet', which requires 64 byte alignment #0 in dp_packet_set_base lib/dp-packet.h:587 #1 in dp_packet_use__ lib/dp-packet.c:46 #2 in dp_packet_use lib/dp-packet.c:60 #3 in dp_packet_init lib/dp-packet.c:126 #4 in dp_packet_new lib/dp-packet.c:150 [...] Signed-off-by: Mike Pattrick <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>

russellb force-pushed the travisci-fix branch 4 times, most recently from b21f26d to eba616a Compare March 7, 2016 20:51

rhel: Use ovn-ctl to ovn-controller service

9863bb9

Trying to revert the changes caused by 8520dee to travis build failures Signed-off-by: Babu Shanmugam <[email protected]>

russellb force-pushed the travisci-fix branch from eba616a to 9863bb9 Compare March 8, 2016 13:34

russellb force-pushed the master branch 9 times, most recently from 5d4d5df to 925f669 Compare November 4, 2016 00:42

russellb force-pushed the master branch 2 times, most recently from 23f0ab3 to 090cc60 Compare February 27, 2017 21:06

russellb closed this Jul 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Travisci fix #3

Travisci fix #3

russellb commented Mar 7, 2016

Travisci fix #3

Travisci fix #3

Conversation

russellb commented Mar 7, 2016