From nobody Fri May 12 10:04:30 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4QHkrZ3NmDz4Bp05; Fri, 12 May 2023 10:04:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4QHkrZ2qvyz3M0Z; Fri, 12 May 2023 10:04:30 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1683885870; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Vqd1pElul8eQV3cxyxpav44tV+Pseq+i6/7kD/lcqHA=; b=ICmqcMi1QNOvkK+nAliS3xBZuE2aTroSEf8DNznRFK0/KPIriV+MvSXbMMcZfVrxB4xC8G unSaKu62bqHI/FrAhNfS+3EhzrSwQwIFlhSrCsuISoou4cWGiIecjBZpbB+2l8Ylske81o 0sI9eH3aXFuprf8x5nclWY2Qe0aoqIV/+7d//VEU1F2o9EMNg8jNUU4Cs7F1Nxsonc1Le9 7cXPUoSnClFk/kbA9pAdz+AOlbeJEWapC7hmdIStaFCdxav1HMOS8BSpNSnrJneTSH5WMj 291nnVWIo9ey9fQxB2P+aIxUjnl7SdEd1UIe437QlLSYymxdQwWhoONq6Qe7xg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1683885870; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Vqd1pElul8eQV3cxyxpav44tV+Pseq+i6/7kD/lcqHA=; b=dC9h2RwwKwlp4sJ5IzLSfVI6Flr8mD5GEfZUq3iRnVPo4UeVBSVmnZzNb2NxiioO/uxAqV IazPk5zUdZaRooxFLyzWmkxpJGhJfpAA0ks9tt0xdLHU4Fpngn7hWIFyoMj0jzmc9UqXU1 s1J483ah30ZgmLXG8QfmnYYm8CKAkkE1V4mCW0TAwIcAV+mdDtqi8CqCM/OyM0yY3Rena8 VPd0/3bRBrNEctrdsYyQJTev82QNoFQ+C9QfF5YNr+r7/UxRu8V4C3jodH7mPu+7y6kojU lEBvTBQV0pWgrrc6cnQqzSpToduRU4wp5diDGzoioueoxAgEJ8TZu8sgkMQScg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1683885870; a=rsa-sha256; cv=none; b=vRP7KiQr3FUPcsGVFpB4//3CVjUpuiafuQWyoG39BLG/um6siIiI5M2BwGWhZPqR/Aw2IJ ook51N4cLJG9nftBmfYxZDb0XzvEnJt3hj1h5Ol19ULQulffslg9QBzoEO9rYaD7lAt5AC 6rEaTYXIuJkB/eTPfxUatj5vLsmOJLEmBXKiIjahbokAKt318Mqo+jaIavjW34+RQOGmWi 27x2o6bEqHNAWqTZBWloSC3DVDG++u9rn/kHTvbXbcS5p+OWgnsSe8SQd86jdNrtiGNHVZ 8qsKFQDosATjjGKZlwMRDt9/W3RC33dPlTKMJnN0QonPaIYlg3tkGg5A0NP9kQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4QHkrZ1vTNzWVn; Fri, 12 May 2023 10:04:30 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 34CA4U72002656; Fri, 12 May 2023 10:04:30 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 34CA4UFv002655; Fri, 12 May 2023 10:04:30 GMT (envelope-from git) Date: Fri, 12 May 2023 10:04:30 GMT Message-Id: <202305121004.34CA4UFv002655@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 92c23f6d9c20 - main - vlan: fix setting flags on a QinQ interface List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 92c23f6d9c2074f6deb0029d13a8c92b32797059 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=92c23f6d9c2074f6deb0029d13a8c92b32797059 commit 92c23f6d9c2074f6deb0029d13a8c92b32797059 Author: Kristof Provost AuthorDate: 2023-05-12 08:42:48 +0000 Commit: Kristof Provost CommitDate: 2023-05-12 09:12:51 +0000 vlan: fix setting flags on a QinQ interface Setting vlan flags needlessly takes the exclusive VLAN_XLOCK(). If we have stacked vlan devices (i.e. QinQ) and we set vlan flags (e.g. IFF_PROMISC) we call rtnl_handle_ifevent() to send a notification about the interface. This ends up calling SIOCGIFMEDIA, which requires the VLAN_SLOCK(). Trying to take that one with the VLAN_XLOCK() held deadlocks us. There's no need for the exclusive lock though, as we're only accessing parent/trunk information, not modifying it, so a shared lock is sufficient. While here also add a test case for this issue. Backtrace: shared lock of (sx) vlan_sx @ /usr/src/sys/net/if_vlan.c:2192 while exclusively locked from /usr/src/sys/net/if_vlan.c:2307 panic: excl->share cpuid = 29 time = 1683873033 KDB: stack backtrace: db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe015d4ad4b0 vpanic() at vpanic+0x152/frame 0xfffffe015d4ad500 panic() at panic+0x43/frame 0xfffffe015d4ad560 witness_checkorder() at witness_checkorder+0xcb5/frame 0xfffffe015d4ad720 _sx_slock_int() at _sx_slock_int+0x67/frame 0xfffffe015d4ad760 vlan_ioctl() at vlan_ioctl+0xf8/frame 0xfffffe015d4ad7c0 dump_iface() at dump_iface+0x12f/frame 0xfffffe015d4ad840 rtnl_handle_ifevent() at rtnl_handle_ifevent+0xab/frame 0xfffffe015d4ad8c0 if_setflag() at if_setflag+0xf6/frame 0xfffffe015d4ad930 ifpromisc() at ifpromisc+0x2a/frame 0xfffffe015d4ad960 vlan_setflags() at vlan_setflags+0x60/frame 0xfffffe015d4ad990 vlan_ioctl() at vlan_ioctl+0x216/frame 0xfffffe015d4ad9f0 if_setflag() at if_setflag+0xe4/frame 0xfffffe015d4ada60 ifpromisc() at ifpromisc+0x2a/frame 0xfffffe015d4ada90 bridge_ioctl_add() at bridge_ioctl_add+0x499/frame 0xfffffe015d4adb10 bridge_ioctl() at bridge_ioctl+0x328/frame 0xfffffe015d4adbc0 ifioctl() at ifioctl+0x972/frame 0xfffffe015d4adcc0 kern_ioctl() at kern_ioctl+0x1fe/frame 0xfffffe015d4add30 sys_ioctl() at sys_ioctl+0x154/frame 0xfffffe015d4ade00 amd64_syscall() at amd64_syscall+0x140/frame 0xfffffe015d4adf30 fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe015d4adf30 --- syscall (54, FreeBSD ELF64, ioctl), rip = 0x22b0f0ef8d8a, rsp = 0x22b0ec63f2c8, rbp = 0x22b0ec63f380 --- KDB: enter: panic [ thread pid 5715 tid 101132 ] Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/net/if_vlan.c | 4 ++-- tests/sys/net/if_vlan.sh | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/sys/net/if_vlan.c b/sys/net/if_vlan.c index f5b401c446ed..6aa872a19364 100644 --- a/sys/net/if_vlan.c +++ b/sys/net/if_vlan.c @@ -2304,10 +2304,10 @@ vlan_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) * We should propagate selected flags to the parent, * e.g., promiscuous mode. */ - VLAN_XLOCK(); + VLAN_SLOCK(); if (TRUNK(ifv) != NULL) error = vlan_setflags(ifp, 1); - VLAN_XUNLOCK(); + VLAN_SUNLOCK(); break; case SIOCADDMULTI: diff --git a/tests/sys/net/if_vlan.sh b/tests/sys/net/if_vlan.sh index 96426254bb6d..e4c737121bdb 100755 --- a/tests/sys/net/if_vlan.sh +++ b/tests/sys/net/if_vlan.sh @@ -221,6 +221,34 @@ qinq_dot_cleanup() vnet_cleanup } +atf_test_case "qinq_setflags" "cleanup" +qinq_setflags_head() +{ + atf_set descr 'Test setting flags on a QinQ device' + atf_set require.user root +} + +qinq_setflags_body() +{ + vnet_init + + epair=$(vnet_mkepair) + + ifconfig ${epair}a up + vlan1=$(ifconfig vlan create) + ifconfig $vlan1 vlan 1 vlandev ${epair}a + vlan2=$(ifconfig vlan create) + ifconfig $vlan2 vlan 2 vlandev $vlan1 + + # This panics, incorrect locking + ifconfig $vlan2 promisc +} + +qinq_setflags_cleanup() +{ + vnet_cleanup +} + atf_test_case "bpf_pcp" "cleanup" bpf_pcp_head() { @@ -273,5 +301,6 @@ atf_init_test_cases() atf_add_test_case "qinq_deep" atf_add_test_case "qinq_legacy" atf_add_test_case "qinq_dot" + atf_add_test_case "qinq_setflags" atf_add_test_case "bpf_pcp" }