svn commit: r353292 - in head/sys: contrib/ipfilter/netinet dev/firewire dev/iicbus dev/usb/net kern net netgraph netinet netinet6 netipsec netpfil/ipfw netpfil/pf ofed/drivers/infiniband/ulp/ipoib

[Kristof Provost]

On 11 Oct 2019, at 16:29, Kristof Provost wrote:
> On 7 Oct 2019, at 15:40, Gleb Smirnoff wrote:
>> Author: glebius
>> Date: Mon Oct  7 22:40:05 2019
>> New Revision: 353292
>> URL: https://svnweb.freebsd.org/changeset/base/353292
>>
>> Log:
>>   Widen NET_EPOCH coverage.
>>
>>   When epoch(9) was introduced to network stack, it was basically
>>   dropped in place of existing locking, which was mutexes and
>>   rwlocks. For the sake of performance mutex covered areas were
>>   as small as possible, so became epoch covered areas.
>>
>>   However, epoch doesn't introduce any contention, it just delays
>>   memory reclaim. So, there is no point to minimise epoch covered
>>   areas in sense of performance. Meanwhile entering/exiting epoch
>>   also has non-zero CPU usage, so doing this less often is a win.
>>
>>   Not the least is also code maintainability. In the new paradigm
>>   we can assume that at any stage of processing a packet, we are
>>   inside network epoch. This makes coding both input and output
>>   path way easier.
>>
>>   On output path we already enter epoch quite early - in the
>>   ip_output(), in the ip6_output().
>>
>>   This patch does the same for the input path. All ISR processing,
>>   network related callouts, other ways of packet injection to the
>>   network stack shall be performed in net_epoch. Any leaf function
>>   that walks network configuration now asserts epoch.
>>
>>   Tricky part is configuration code paths - ioctls, sysctls. They
>>   also call into leaf functions, so some need to be changed.
>>
>>   This patch would introduce more epoch recursions (see EPOCH_TRACE)
>>   than we had before. They will be cleaned up separately, as several
>>   of them aren't trivial. Note, that unlike a lock recursion the
>>   epoch recursion is safe and just wastes a bit of resources.
>>
>>   Reviewed by:	gallatin, hselasky, cy, adrian, kristof
>>   Differential Revision:	https://reviews.freebsd.org/D19111
>>
> This appears to have broken vlan.
>
> To reproduce do:
>
> sudo ifconfig epair create
> sudo ifconfig vlan create vlandev epair0a vlan 42
>
> I see the following panic:
>
> 	panic: Assertion in_epoch(net_epoch_preempt) failed at 
> /usr/src/sys/net/if_vlan.c:1353
> 	cpuid = 5
> 	time = 1570828155
> 	KDB: stack backtrace:
> 	db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 
> 0xfffffe0060d894c0
> 	vpanic() at vpanic+0x17e/frame 0xfffffe0060d89520
> 	panic() at panic+0x43/frame 0xfffffe0060d89580
> 	vlan_config() at vlan_config+0x599/frame 0xfffffe0060d895c0
> 	vlan_clone_create() at vlan_clone_create+0x29b/frame 
> 0xfffffe0060d89630
> 	if_clone_createif() at if_clone_createif+0x4a/frame 
> 0xfffffe0060d89680
> 	ifioctl() at ifioctl+0x6f4/frame 0xfffffe0060d89750
> 	kern_ioctl() at kern_ioctl+0x295/frame 0xfffffe0060d897b0
> 	sys_ioctl() at sys_ioctl+0x15c/frame 0xfffffe0060d89880
> 	amd64_syscall() at amd64_syscall+0x2b5/frame 0xfffffe0060d899b0
> 	fast_syscall_common() at fast_syscall_common+0x101/frame 
> 0xfffffe0060d899b0
> 	--- syscall (54, FreeBSD ELF64, sys_ioctl), rip = 0x80047dc2a, rsp = 
> 0x7fffffffe1a8, rbp = 0x7fffffffe1b0 ---
>
> I guess that we need to enter net_epoch in vlan_clone_create(). Or 
> perhaps that’s something that’s generically needed and it should 
> be done on if_clone_createif().
>
Alternatively, this test case seems to trigger a number of different 
issues (I’ve tried entering the epoch and removing the assertions and 
keep running into different issues):

	diff --git a/tests/sys/net/Makefile b/tests/sys/net/Makefile
	index 03e0631f625..bf8fb174e47 100644
	--- a/tests/sys/net/Makefile
	+++ b/tests/sys/net/Makefile
	@@ -8,6 +8,7 @@ BINDIR=         ${TESTSDIR}
	 ATF_TESTS_SH+= if_lagg_test
	 ATF_TESTS_SH+= if_clone_test
	 ATF_TESTS_SH+= if_tun_test
	+ATF_TESTS_SH+= if_vlan

	 # The tests are written to be run in parallel, but doing so leads to 
random
	 # panics.  I think it's because the kernel's list of interfaces isn't 
properly
	diff --git a/tests/sys/net/if_vlan.sh b/tests/sys/net/if_vlan.sh
	new file mode 100644
	index 00000000000..020ec59830f
	--- /dev/null
	+++ b/tests/sys/net/if_vlan.sh
	@@ -0,0 +1,38 @@
	+# $FreeBSD$
	+
	+. $(atf_get_srcdir)/../common/vnet.subr
	+
	+atf_test_case "basic" "cleanup"
	+basic_head()
	+{
	+       atf_set descr 'Basic VLAN test'
	+       atf_set require.user root
	+}
	+
	+basic_body()
	+{
	+       vnet_init
	+
	+       epair_vlan=$(vnet_mkepair)
	+
	+       vnet_mkjail alcatraz ${epair_vlan}a
	+       vnet_mkjail singsing ${epair_vlan}b
	+
	+       vlan0=$(jexec alcatraz ifconfig vlan create vlandev 
${epair_vlan}a vlan 42)
	+       jexec alcatraz ifconfig ${vlan0} 10.0.0.1/24 up
	+
	+       vlan1=$(jexec singsing ifconfig vlan create vlandev 
${epair_vlan}b vlan 42)
	+       jexec singsing ifconfig ${vlan1} 10.0.0.2/24 up
	+
	+       atf_check -s exit:0 jexec singsing ping -c 1 10.0.0.1
	+}
	+
	+basic_cleanup()
	+{
	+       vnet_cleanup
	+}
	+
	+atf_init_test_cases()
	+{
	+       atf_add_test_case "basic"
	+}

Best regards,
Kristof