netlink: add netlink interfaces to if_clone
ClosedPublic
Actions

Authored by melifaro on Mar 12 2023, 7:55 PM.

Details

Reviewers

Group Reviewers

network

Commits

rG089104e0e01f: netlink: add netlink interfaces to if_clone

Summary

This change adds netlink create/modify/dump interfaces to the if_clone.c.

The previous attempt stored this logic inside netlink/route/iface_drivers.c did not quite work, as, for example, dumping interface-specific state (like vlan id or vlan parent) required some peeking into the private interfaces.

The new interfaces are added in a compatible way - callers don't have to do anything unless they are extended with Netlink.

Test Plan

 s kyua debug -k /usr/tests/sys/netlink/Kyuafile test_rtnl_iface.py:TestRtNlIface::test_create_vlan_plain
...
============= >> TX MESSAGE =============
len=0, type=RTM_NEWLINK, flags=0x1,0x4,NLM_F_EXCL,NLM_F_CREATE(0x605), seq=1, pid=1665
 family=af#0, ifi_type=0, ifi_index=3, ifi_flags=0, ifi_change=0
  len=8 type=IFLA_LINK(5) val=3
  len=11 type=IFLA_IFNAME(3) val="vlan22"
  len=24 type=IFLA_LINKINFO(18) {
    len=8 type=IFLA_INFO_KIND(1) val="vlan"
    len=12 type=IFLA_INFO_DATA(2) {
      len=6 type=IFLA_VLAN_ID(1) val=22
    }
  }

...

======= ifconfig =====
...
vlan22: flags=8842<BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
	ether 02:b2:1c:96:7e:0a
	groups: vlan
	vlan: 22 vlanproto: 802.1q vlanpcp: 0 parent interface: epair0a
	media: Ethernet 10Gbase-T (10Gbase-T <full-duplex>)
	status: active
	nd6 options=29<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL>

============= >> TX MESSAGE =============
len=0, type=RTM_GETLINK, flags=0x1,0x4(0x5), seq=3, pid=1665
 family=af#0, ifi_type=0, ifi_index=0, ifi_flags=0, ifi_change=0
  len=11 type=IFLA_IFNAME(3) val="vlan22"
--------------------

============= << RX MESSAGE =============
len=340, type=RTM_NEWLINK, flags=0x1,0x4(0x5), seq=3, pid=0
 family=af#0, ifi_type=135, ifi_index=8, ifi_flags=16812098, ifi_change=0
  len=11 type=IFLA_IFNAME(3) val="vlan22"
  len=5 type=IFLA_OPERSTATE(16) val=6
  len=5 type=IFLA_CARRIER(33) val=1
  len=10 type=IFLA_ADDRESS(1) mac="02:B2:1C:96:7E:0A"
  len=10 type=IFLA_BROADCAST(2) mac="FF:FF:FF:FF:FF:FF"
  len=8 type=IFLA_MTU(4) val=1500
  len=196 type=IFLA_STATS64(23) stats={...}
  len=8 type=IFLA_PROMISCUITY(30) val=0
  len=8 type=IFLA_LINK(5) val=3
  len=36 type=IFLA_LINKINFO(18) {
    len=9 type=IFLA_INFO_KIND(1) val="vlan"
    len=20 type=IFLA_INFO_DATA(2) x06 x00 x01 x00 x16 x00 x00 x00 x06 x00 x05 x00 x00 x81 x00 x00
  }

11:42 [0] m@devel3 s kyua test -k /uss kyua test -k /usr/tests/sys/net/Kyuafile
if_bridge_test:bridge_transmit_ipv4_unicast  ->  passed  [2.099s]
if_bridge_test:delete_with_members  ->  passed  [0.044s]
if_bridge_test:gif  ->  passed  [0.100s]
if_bridge_test:inherit_mac  ->  passed  [0.055s]
if_bridge_test:mac_conflict  ->  passed  [2.096s]
if_bridge_test:mtu  ->  passed  [0.068s]
if_bridge_test:span  ->  passed  [5.644s]
if_bridge_test:static  ->  passed  [0.102s]
if_bridge_test:stp  ->  passed  [5.166s]
if_bridge_test:stp_validation  ->  failed: atf-check failed; see the output of the test for details  [19.963s]
if_bridge_test:stp_vlan  ->  passed  [5.183s]
if_clone_test:epair_destroy_race  ->  passed  [1.795s]
if_clone_test:epair_ipv6_up_stress  ->  passed  [10.018s]
if_clone_test:epair_stress  ->  passed  [10.021s]
if_clone_test:epair_up_stress  ->  passed  [10.022s]
if_clone_test:faith_ipv6_up_stress  ->  passed  [10.032s]
if_clone_test:faith_stress  ->  passed  [10.062s]
if_clone_test:faith_up_stress  ->  passed  [10.031s]
if_clone_test:gif_ipv6_up_stress  ->  passed  [10.016s]
if_clone_test:gif_stress  ->  passed  [10.027s]
if_clone_test:gif_up_stress  ->  passed  [10.021s]
if_clone_test:lo_ipv6_up_stress  ->  passed  [10.027s]
if_clone_test:lo_stress  ->  passed  [10.093s]
if_clone_test:lo_up_stress  ->  passed  [10.020s]
if_clone_test:tap_ipv6_up_stress  ->  skipped: Quickly panics: if_delmulti_locked: inconsistent ifp 0xfffff80150e44000  [0.008s]
if_clone_test:tap_stress  ->  passed  [10.024s]
if_clone_test:tap_up_stress  ->  passed  [10.022s]
if_clone_test:tun_ipv6_up_stress  ->  passed  [10.013s]
if_clone_test:tun_stress  ->  passed  [10.067s]
if_clone_test:tun_up_stress  ->  passed  [10.029s]
if_clone_test:vlan_ipv6_up_stress  ->  passed  [10.066s]
if_clone_test:vlan_stress  ->  passed  [10.244s]
if_clone_test:vlan_up_stress  ->  passed  [10.023s]
if_clone_test:vmnet_ipv6_up_stress  ->  passed  [10.020s]
if_clone_test:vmnet_stress  ->  passed  [10.111s]
if_clone_test:vmnet_up_stress  ->  passed  [10.018s]
if_epair:params  ->  passed  [0.017s]
if_gif:basic  ->  passed  [0.060s]
if_lagg_test:create  ->  passed  [0.059s]
if_lagg_test:create_destroy_stress  ->  skipped: Skipping this test because it easily panics the machine  [0.007s]
if_lagg_test:lacp_linkstate_destroy_stress  ->  passed  [60.084s]
if_lagg_test:set_ether  ->  passed  [0.055s]
if_lagg_test:status_stress  ->  passed  [60.044s]
if_lagg_test:up_destroy_stress  ->  skipped: Skipping this test because it panics the machine fairly often  [0.007s]
if_lagg_test:updown  ->  expected_failure: PR 226144 Upping a lagg interrface should automatically up its children: atf-check failed; see the output of the test for details  [0.028s]
if_lagg_test:witness  ->  failed: Lock-order reversals involving if_lagg.c detected  [0.047s]
if_stf:6rd  ->  passed  [0.084s]
if_stf:6rd_peer  ->  passed  [0.116s]
if_stf:6to4  ->  passed  [0.071s]
if_tun_test:235704  ->  passed  [0.034s]
if_vlan:basic  ->  passed  [0.053s]
if_vlan:bpf_pcp  ->  passed  [26.881s]
if_vlan:qinq_deep  ->  passed  [0.058s]
if_vlan:qinq_dot  ->  passed  [0.055s]
if_vlan:qinq_legacy  ->  passed  [0.093s]
if_vlan:qinq_simple  ->  passed  [0.122s]
routing/test_routing_l3.py:TestIfOps::test_change_prefix_route[inet6]  ->  passed  [0.394s]
routing/test_routing_l3.py:TestIfOps::test_change_prefix_route[inet]  ->  passed  [0.442s]
routing/test_routing_l3.py:TestIfOps::test_change_prefix_route_same_iface[inet6]  ->  skipped: /usr/tests/sys/net/routing/test_routing_l3.py:88: AssertionError  [0.368s]
routing/test_routing_l3.py:TestIfOps::test_change_prefix_route_same_iface[inet]  ->  passed  [0.349s]
routing/test_routing_l3.py:TestRouteCornerCase1::test_add_direct_route_p2p_wo_ifa[inet6]  ->  passed  [0.312s]
routing/test_routing_l3.py:TestRouteCornerCase1::test_add_direct_route_p2p_wo_ifa[inet]  ->  passed  [0.322s]
routing/test_rtsock_l3:rtm_add_v4_gu_ifa_ordered_success  ->  passed  [0.122s]
routing/test_rtsock_l3:rtm_add_v4_gw_direct_success  ->  passed  [0.020s]
routing/test_rtsock_l3:rtm_add_v4_no_rtf_host_success  ->  passed  [0.120s]
routing/test_rtsock_l3:rtm_add_v4_temporal1_success  ->  passed  [0.155s]
routing/test_rtsock_l3:rtm_add_v6_gu_gw_gu_direct_success  ->  passed  [0.115s]
routing/test_rtsock_l3:rtm_add_v6_gu_ifa_hostroute_success  ->  passed  [0.119s]
routing/test_rtsock_l3:rtm_add_v6_gu_ifa_ordered_success  ->  passed  [0.115s]
routing/test_rtsock_l3:rtm_add_v6_gu_ifa_prefixroute_success  ->  passed  [0.119s]
routing/test_rtsock_l3:rtm_add_v6_temporal1_success  ->  passed  [0.128s]
routing/test_rtsock_l3:rtm_change_v4_flags_success  ->  passed  [0.122s]
routing/test_rtsock_l3:rtm_change_v4_gw_success  ->  passed  [0.125s]
routing/test_rtsock_l3:rtm_change_v4_mtu_success  ->  passed  [0.125s]
routing/test_rtsock_l3:rtm_change_v6_flags_success  ->  passed  [0.123s]
routing/test_rtsock_l3:rtm_change_v6_gw_success  ->  passed  [0.121s]
routing/test_rtsock_l3:rtm_change_v6_mtu_success  ->  passed  [0.113s]
routing/test_rtsock_l3:rtm_del_v4_gu_ifa_prefixroute_success  ->  passed  [0.116s]
routing/test_rtsock_l3:rtm_del_v4_prefix_nogw_success  ->  passed  [0.116s]
routing/test_rtsock_l3:rtm_del_v6_gu_ifa_hostroute_success  ->  passed  [0.125s]
routing/test_rtsock_l3:rtm_del_v6_gu_ifa_prefixroute_success  ->  passed  [0.125s]
routing/test_rtsock_l3:rtm_del_v6_gu_prefix_nogw_success  ->  passed  [0.011s]
routing/test_rtsock_l3:rtm_get_v4_empty_dst_failure  ->  passed  [0.002s]
routing/test_rtsock_l3:rtm_get_v4_exact_success  ->  passed  [0.118s]
routing/test_rtsock_l3:rtm_get_v4_hostbits_success  ->  passed  [0.113s]
routing/test_rtsock_l3:rtm_get_v4_lpm_success  ->  passed  [0.124s]
routing/test_rtsock_lladdr:rtm_add_v4_gu_lle_success  ->  passed  [0.122s]
routing/test_rtsock_lladdr:rtm_add_v6_gu_lle_success  ->  passed  [0.118s]
routing/test_rtsock_lladdr:rtm_add_v6_ll_lle_success  ->  passed  [0.122s]
routing/test_rtsock_lladdr:rtm_del_v4_gu_lle_success  ->  passed  [0.125s]
routing/test_rtsock_lladdr:rtm_del_v6_gu_lle_success  ->  passed  [0.009s]
routing/test_rtsock_lladdr:rtm_del_v6_ll_lle_success  ->  passed  [0.117s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[change_new_weight1]  ->  passed  [0.363s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[change_new_weight2]  ->  passed  [0.366s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[change_new_weight3]  ->  passed  [0.329s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[change_same_weight1]  ->  passed  [0.325s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[change_same_weight2]  ->  passed  [0.372s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[correctness1]  ->  passed  [0.351s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[correctness2]  ->  passed  [0.349s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[transition_multi]  ->  passed  [0.308s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[transition_single1]  ->  passed  [0.349s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[transition_single2]  ->  passed  [0.316s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[weight1]  ->  passed  [0.326s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[weight2]  ->  passed  [0.312s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4[weight3_max]  ->  passed  [0.301s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4_add_same_eexist  ->  passed  [0.319s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4_add_zero_weight  ->  passed  [0.295s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4_change_unknown_esrch  ->  passed  [0.325s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4_del_unknown_esrch  ->  passed  [0.366s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath4_getroute  ->  passed  [0.344s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[change_new_weight1]  ->  passed  [0.332s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[change_new_weight2]  ->  passed  [0.305s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[change_new_weight3]  ->  passed  [0.323s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[change_same_weight1]  ->  passed  [0.351s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[change_same_weight2]  ->  passed  [0.369s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[correctness1]  ->  passed  [0.332s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[correctness2]  ->  passed  [0.372s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[transition_multi]  ->  passed  [0.342s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[transition_single1]  ->  passed  [0.348s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[transition_single2]  ->  passed  [0.332s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[weight1]  ->  passed  [0.398s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[weight2]  ->  passed  [0.347s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[weight3_max0]  ->  passed  [0.326s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6[weight3_max1]  ->  passed  [0.329s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6_add_same_eexist  ->  passed  [0.330s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6_change_unknown_esrch  ->  passed  [0.320s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6_del_unknown_esrch  ->  passed  [0.317s]
routing/test_rtsock_multipath.py:TestRtmMultipath::test_rtm_multipath6_getroute  ->  passed  [0.309s]
routing/test_rtsock_ops:socket_rtsock_openclose  ->  passed  [0.002s]

Results file id is usr_tests_sys_net.20230420-114258-264945
Results saved to /home/melifaro/.kyua/store/results.usr_tests_sys_net.20230420-114258-264945.db

128/129 passed (1 failed)

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Errors

Severity	Location	Code	Message
Error	tests/sys/netlink/test_rtnl_iface.py:42	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:78	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:102	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:118	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:127	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:136	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:157	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:166	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:197	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:206	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:255	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:256	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:258	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:272	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:282	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:309	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:310	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:315	E501	PEP8 E501
Error	tests/sys/netlink/test_rtnl_iface.py:326	E501	PEP8 E501

Unit

No Test Coverage

Build Status

Buildable 51034
Build 47925: arc lint + arc unit

Event Timeline

melifaro created this revision.Mar 12 2023, 7:55 PM

Herald added subscribers: glebius, ae, imp. · View Herald TranscriptMar 12 2023, 7:55 PM

melifaro requested review of this revision.Mar 12 2023, 7:55 PM

Harbormaster completed remote builds in B50302: Diff 118692.Mar 12 2023, 7:55 PM

melifaro retitled this revision from netlink: combine cloners to netlink: add netlink interfaces to if_clone.Mar 12 2023, 8:04 PM

melifaro edited the summary of this revision. (Show Details)

melifaro edited the test plan for this revision. (Show Details)

melifaro added reviewers: network, kp.

melifaro mentioned this in D39046: net: refactor if_clone.c #1.Mar 13 2023, 10:44 AM

melifaro mentioned this in rG83b5c80c6923: net: refactor if_clone.c #1.Mar 15 2023, 1:56 PM

Sync to recent HEAD.

Harbormaster completed remote builds in B51034: Diff 120654.Apr 19 2023, 2:57 PM

melifaro edited the summary of this revision. (Show Details)Apr 19 2023, 2:58 PM

kp added inline comments.Apr 20 2023, 9:07 AM

sys/net/if_clone.c
413	There's a lot of duplication between if_clone_createif_nl() and if_clone_createif(). Can we implement the latter by converting struct ifc_data to struct ifc_data_nl and calling if_clone_createif_nl()?
sys/net/if_vlan.c
994	Copy/paste error? That presumably needs to be if_vlan where it says linux_common.
1314	Is it safe to exit epoch while holding an ifp?

Address comments.

Harbormaster completed remote builds in B51060: Diff 120717.Apr 20 2023, 11:56 AM

melifaro marked an inline comment as done.Apr 20 2023, 12:01 PM

melifaro added inline comments.

sys/net/if_vlan.c
1314	We're not holding any ifp beyond the epoch scope. I wish I can express it in "block" version like ENTER_NET_EPOCH() { struct ifnet *ifp_parent = ifnet_byindex(lattrs->ifi_index); if (ifp_parent != NULL) strlcpy(params.vlr_parent, if_name(ifp_parent), sizeof(params.vlr_parent)); } but that's not easily possible in C we currently use, unfortunately.

melifaro edited the test plan for this revision. (Show Details)Apr 20 2023, 12:08 PM

melifaro added inline comments.Apr 20 2023, 12:11 PM

sys/net/if_clone.c
413	That's a good one! IIRC originally I didn't want to store ioctl `params` pointer in the new structure, so it was the reason for having separate functions. Including it made it possible to simplify things a bit, thank you!