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

_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to