Remove RT_LOCK from rte.
ClosedPublic
Actions

Authored by melifaro on Aug 23 2020, 9:38 AM.

Details

Reviewers

None

Group Reviewers

network

Commits

rS364730: Remove RT_LOCK mutex from rte.

Summary

rtentry lock traditionally served 2 purposed: first was protecting refcounts, the second was assuring consistent field access/changes.
Since route nexthop introduction, the need for the former disappeared and the need for the latter reduced.
To be more precise, the following rte field are mutable:

rt_nhop (nexthop pointer, updated with RIB_WLOCK, passed in rib_cmd_info)
rte_flags (only RTF_HOST and RTF_UP, where RTF_UP gets changed at rte removal from the tree)
rt_weight (relative weight, updated with RIB_WLOCK, passed in rib_cmd_info)
rt_expire (time when rte deletion is scheduled, updated with RIB_WLOCK)
rt_chain (deletion chain pointer, updated with RIB_WLOCK)

All of them are updated under RIB_WLOCK, so the only remaining concern is the reading.

rt_nhop and rt_weight (addressed in this review) are read under rib lock and stored in the rib_cmd_info, so the caller has no problem with consitency.
rte_flags is currently read unlocked in rtsock reporting (however the scope is only RTF_UP flag, which is pretty static)
rt_expire is currently read unlocked in rtsock reporting.
rt_chain accesses are safe, as this is only used at route deletion.

rt_expire and rte_flags reads will be dealt in a separate reviews soon.

Test Plan

carp:basic_v4  ->  skipped: This test requires carp  [0.014s]
carp:basic_v6  ->  skipped: This test requires carp  [0.013s]
divert:ipdivert_ip_input_local_success  ->  skipped: ipdivert module is not loaded  [0.035s]
divert:ipdivert_ip_output_remote_success  ->  skipped: ipdivert module is not loaded  [0.016s]
fibs_test:arpresolve_checks_interface_fib  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:default_route_with_multiple_fibs_on_same_subnet  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:default_route_with_multiple_fibs_on_same_subnet_inet6  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:loopback_and_network_routes_on_nondefault_fib  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:loopback_and_network_routes_on_nondefault_fib_inet6  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:same_ip_multiple_ifaces  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:same_ip_multiple_ifaces_fib0  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:same_ip_multiple_ifaces_inet6  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:slaac_on_nondefault_fib6  ->  skipped: Required configuration property 'allow_sysctl_side_effects' not defined  [0.001s]
fibs_test:subnet_route_with_multiple_fibs_on_same_subnet  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:subnet_route_with_multiple_fibs_on_same_subnet_inet6  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:udp_dontroute  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
fibs_test:udp_dontroute6  ->  skipped: Required configuration property 'fibs' not defined  [0.001s]
forward:fwd_ip_icmp_gw_fast_success  ->  passed  [0.560s]
forward:fwd_ip_icmp_gw_slow_success  ->  passed  [0.354s]
forward:fwd_ip_icmp_iface_fast_success  ->  passed  [0.351s]
forward:fwd_ip_icmp_iface_slow_success  ->  passed  [0.370s]
ip_reass_test:ip_reass__large_fragment  ->  passed  [0.014s]
ip_reass_test:ip_reass__multiple_last_fragments  ->  passed  [0.029s]
ip_reass_test:ip_reass__zero_length_fragment  ->  passed  [0.013s]
lpm:lpm_test1_success  ->  passed  [0.119s]
lpm:lpm_test2_success  ->  passed  [0.121s]
output:output_raw_flowid_mpath_success  ->  skipped: This test requires ROUTE_MPATH enabled  [0.012s]
output:output_raw_success  ->  passed  [0.060s]
output:output_tcp_flowid_mpath_success  ->  skipped: This test requires ROUTE_MPATH enabled  [0.012s]
output:output_tcp_setup_success  ->  passed  [0.094s]
output:output_udp_flowid_mpath_success  ->  skipped: This test requires ROUTE_MPATH enabled  [0.012s]
output:output_udp_setup_success  ->  passed  [1.221s]
redirect:valid_redirect  ->  passed  [0.354s]
so_reuseport_lb_test:basic_ipv4  ->  passed  [0.420s]
so_reuseport_lb_test:basic_ipv6  ->  passed  [0.460s]
socket_afinet:socket_afinet  ->  passed  [0.002s]
socket_afinet:socket_afinet_bind_ok  ->  passed  [0.002s]
socket_afinet:socket_afinet_bind_zero  ->  passed  [0.002s]

Results file id is usr_tests_sys_netinet.20200823-093641-547590
Results saved to /home/melifaro/.kyua/store/results.usr_tests_sys_netinet.20200823-093641-547590.db

38/38 passed (0 failed)
divert:ipdivert_ip6_output_remote_success  ->  skipped: ipdivert module is not loaded  [0.014s]
forward6:fwd_ip6_gu_icmp_gw_gu_fast_success  ->  passed  [1.899s]
forward6:fwd_ip6_gu_icmp_gw_gu_slow_success  ->  passed  [2.313s]
forward6:fwd_ip6_gu_icmp_gw_ll_fast_success  ->  passed  [1.922s]
forward6:fwd_ip6_gu_icmp_gw_ll_slow_success  ->  passed  [2.050s]
forward6:fwd_ip6_gu_icmp_iface_fast_success  ->  passed  [2.373s]
forward6:fwd_ip6_gu_icmp_iface_slow_success  ->  passed  [2.096s]
lpm6:lpm6_test1_success  ->  passed  [2.099s]
lpm6:lpm6_test2_success  ->  passed  [2.005s]
mld:mldraw01  ->  passed  [3.389s]
output6:output6_raw_flowid_mpath_success  ->  skipped: This test requires ROUTE_MPATH enabled  [0.012s]
output6:output6_raw_success  ->  passed  [1.603s]
output6:output6_tcp_flowid_mpath_success  ->  skipped: This test requires ROUTE_MPATH enabled  [0.012s]
output6:output6_tcp_setup_success  ->  passed  [1.948s]
output6:output6_udp_flowid_mpath_success  ->  skipped: This test requires ROUTE_MPATH enabled  [0.013s]
output6:output6_udp_setup_success  ->  passed  [3.327s]
redirect:valid_redirect  ->  failed: atf-check failed; see the output of the test for details  [2.154s]
scapyi386:scapyi386  ->  passed  [3.426s]

Results file id is usr_tests_sys_netinet6.20200823-093649-262794
Results saved to /root/.kyua/store/results.usr_tests_sys_netinet6.20200823-093649-262794.db

17/18 passed (1 failed)
test_rtsock_l3:rtm_add_v4_gu_ifa_ordered_success  ->  passed  [0.122s]
test_rtsock_l3:rtm_add_v4_gw_direct_success  ->  passed  [0.115s]
test_rtsock_l3:rtm_add_v4_no_rtf_host_failure  ->  passed  [0.114s]
test_rtsock_l3:rtm_add_v4_temporal1_success  ->  passed  [0.126s]
test_rtsock_l3:rtm_add_v6_gu_gw_gu_direct_success  ->  passed  [0.116s]
test_rtsock_l3:rtm_add_v6_gu_ifa_hostroute_success  ->  passed  [0.120s]
test_rtsock_l3:rtm_add_v6_gu_ifa_ordered_success  ->  passed  [0.116s]
test_rtsock_l3:rtm_add_v6_gu_ifa_prefixroute_success  ->  passed  [0.117s]
test_rtsock_l3:rtm_add_v6_temporal1_success  ->  passed  [0.124s]
test_rtsock_l3:rtm_change_v4_gw_success  ->  passed  [0.118s]
test_rtsock_l3:rtm_change_v4_mtu_success  ->  passed  [0.117s]
test_rtsock_l3:rtm_change_v6_gw_success  ->  passed  [0.121s]
test_rtsock_l3:rtm_change_v6_mtu_success  ->  passed  [0.116s]
test_rtsock_l3:rtm_del_v4_gu_ifa_prefixroute_success  ->  passed  [0.119s]
test_rtsock_l3:rtm_del_v4_prefix_nogw_success  ->  passed  [0.115s]
test_rtsock_l3:rtm_del_v6_gu_ifa_hostroute_success  ->  passed  [0.121s]
test_rtsock_l3:rtm_del_v6_gu_ifa_prefixroute_success  ->  passed  [0.121s]
test_rtsock_l3:rtm_del_v6_gu_prefix_nogw_success  ->  passed  [0.122s]
test_rtsock_l3:rtm_get_v4_empty_dst_failure  ->  passed  [0.003s]
test_rtsock_l3:rtm_get_v4_exact_success  ->  passed  [0.123s]
test_rtsock_l3:rtm_get_v4_hostbits_failure  ->  passed  [0.120s]
test_rtsock_l3:rtm_get_v4_lpm_success  ->  passed  [0.017s]
test_rtsock_lladdr:rtm_add_v4_gu_lle_success  ->  passed  [0.017s]
test_rtsock_lladdr:rtm_add_v6_gu_lle_success  ->  passed  [0.115s]
test_rtsock_lladdr:rtm_add_v6_ll_lle_success  ->  passed  [0.116s]
test_rtsock_lladdr:rtm_del_v4_gu_lle_success  ->  passed  [0.115s]
test_rtsock_lladdr:rtm_del_v6_gu_lle_success  ->  passed  [0.112s]
test_rtsock_lladdr:rtm_del_v6_ll_lle_success  ->  passed  [0.114s]

// redirect:valid_redirect is apparently failing b/c of scapy reading IPv6 routes in IPv6 code.