NetBSD Problem Report #57650

From www@netbsd.org  Tue Oct 10 01:00:02 2023
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id EB5C01A9238
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 10 Oct 2023 01:00:01 +0000 (UTC)
Message-Id: <20231010010000.7F6BA1A923A@mollari.NetBSD.org>
Date: Tue, 10 Oct 2023 01:00:00 +0000 (UTC)
From: sfroyen@gmail.com
Reply-To: sfroyen@gmail.com
To: gnats-bugs@NetBSD.org
Subject: Unable to change MTU on lagg device
X-Send-Pr-Version: www-1.0

>Number:         57650
>Category:       kern
>Synopsis:       Unable to change MTU on lagg device
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Oct 10 01:05:00 +0000 2023
>Last-Modified:  Thu Oct 19 07:25:01 +0000 2023
>Originator:     Sverre Froyen
>Release:        10.99.10
>Organization:
>Environment:
NetBSD gateway.fesk.me 10.99.10 NetBSD 10.99.10 (GENERIC) #4: Sat Oct  7 16:19:15 MDT 2023  sverre@gateway.fesk.me:/usr/obj/sys/arch/amd64/compile/GENERIC amd64
>Description:
There are two separate issues:

1) The command "ifconfig lagg0 MTU <any value>" results in a locking against myself panic.

crash output and backtrace:

# crash -M netbsd.5.core -N netbsd.5
Crash version 10.99.10, image version 10.99.10.
crash: _kvm_kvatop(0)
Kernel compiled without options LOCKDEBUG.
System panicked: lock error: Mutex: mutex_vector_enter,549: locking against myself: lock 0xffffb357a1535800 cpu 1 lwp 0xffffb357b6bf1400
Backtrace from time of crash is available.

crash> bt
__kernel_end() at 0
kern_reboot() at sys_reboot
vpanic() at vpanic+0x17d
panic() at printf_nostamp
lockdebug_abort() at lockdebug_abort+0x114
mutex_vector_enter() at mutex_vector_enter+0x371
lagg_init() at lagg_init+0x13
lagg_ioctl() at lagg_ioctl+0x828
doifioctl() at doifioctl+0x30c
sys_ioctl() at sys_ioctl+0x56d
syscall() at syscall+0x17a
--- syscall (number 54) ---
syscall+0x17a:

Checking the source, the problem is obvious:
lagg_init is called with the lock already set (by lagg_ioctl)

Possible fixes:
should lagg_init_locked be used instead?
or
should lagg_ioctl not set the lock?

2) Attempting to configure a jumbo MTU on boot fails with an invalid value error

With /etc/ifconfig.lagg0 containing

!ifconfig wm0 up mtu 1500
!ifconfig wm1 up mtu 1500
create
laggproto lacp laggport wm0 laggport wm1 192.168.2.1 netmask 255.255.255.0
up

lagg0 works perfectly. If I change the MTUs on the wm interfaces to 9000 (or anything > 1500), I get an invalid error and lagg0 does not work.

PS The wm interfaces are I211 Ethernet (COPPER) and work fine with MTU 9000.

>How-To-Repeat:
1) With the system running and lagg0 configured as in the description with MTU 1500, enter the command

ifconfig lagg0 mtu 9000

and observe the panic

2) Change /etc/ifconfig.lagg0 to

!ifconfig wm0 up mtu 9000
!ifconfig wm1 up mtu 9000
create
laggproto lacp laggport wm0 laggport wm1 192.168.2.1 netmask 255.255.255.0
up

and reboot

Watch the error message in the network configuration output

>Fix:
1) See my comments in the description regarding the locking

2) ?

>Audit-Trail:
From: "Shoichi YAMAGUCHI" <yamaguchi@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57650 CVS commit: src/sys/net/lagg
Date: Mon, 16 Oct 2023 07:49:01 +0000

 Module Name:	src
 Committed By:	yamaguchi
 Date:		Mon Oct 16 07:49:01 UTC 2023

 Modified Files:
 	src/sys/net/lagg: if_lagg.c

 Log Message:
 lagg(4): release LAGG_LOCK before mtu changing

 PR kern/57650


 To generate a diff of this commit:
 cvs rdiff -u -r1.48 -r1.49 src/sys/net/lagg/if_lagg.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Shoichi YAMAGUCHI" <yamaguchi@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57650 CVS commit: src/tests/net/if_lagg
Date: Mon, 16 Oct 2023 07:57:40 +0000

 Module Name:	src
 Committed By:	yamaguchi
 Date:		Mon Oct 16 07:57:40 UTC 2023

 Modified Files:
 	src/tests/net/if_lagg: t_lagg.sh

 Log Message:
 Make the lagg interface up before change its MTU

 This change is related to PR kern/57650


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 src/tests/net/if_lagg/t_lagg.sh

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Sverre Froyen <sfroyen@gmail.com>
To: "gnats-bugs@gnats.netbsd.org" <gnats-bugs@gnats.NetBSD.org>
Cc: 
Subject: Re: kern/57650
Date: Tue, 17 Oct 2023 11:48:45 -0600

 Thanks for fixing the locking bug! I can now modify the MTU on the =
 interface.

 There is still an issue with part 2 of the bug. Using this =
 ifconfig.lagg0 file:

 	!ifconfig wm0 up mtu 9000
 	!ifconfig wm1 up mtu 9000
 	create
 	laggproto lacp laggport wm0 laggport wm1 192.168.2.1 netmask =
 255.255.255.0
 	up

 returns the error message (as before):

 	exec_matches: invalid argument

 I suspect it has to do with the interface not being up when the driver =
 attempts to set the MTU.

 I can obviously work around it, but it is confusing, especially since =
 the man page states

 	The MTU of the first interface to be added is used as the lagg =
 MTU=

From: Shoichi Yamaguchi <yamaguchi@netbsd.org>
To: sfroyen@gmail.com
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	gnats-bugs@netbsd.org
Subject: Re: kern/57650
Date: Wed, 18 Oct 2023 17:23:03 +0900

 Hi,

 On Wed, Oct 18, 2023 at 3:24=E2=80=AFAM Sverre Froyen <sfroyen@gmail.com> w=
 rote:
 >
 > The following reply was made to PR kern/57650; it has been noted by GNATS=
 .
 >
 > From: Sverre Froyen <sfroyen@gmail.com>
 > To: "gnats-bugs@gnats.netbsd.org" <gnats-bugs@gnats.NetBSD.org>
 > Cc:
 > Subject: Re: kern/57650
 > Date: Tue, 17 Oct 2023 11:48:45 -0600
 >
 >  Thanks for fixing the locking bug! I can now modify the MTU on the =3D
 >  interface.
 >
 >  There is still an issue with part 2 of the bug. Using this =3D
 >  ifconfig.lagg0 file:
 >
 >         !ifconfig wm0 up mtu 9000
 >         !ifconfig wm1 up mtu 9000
 >         create
 >         laggproto lacp laggport wm0 laggport wm1 192.168.2.1 netmask =3D
 >  255.255.255.0
 >         up
 >
 >  returns the error message (as before):
 >
 >         exec_matches: invalid argument
 >

 I changed lagg to not copy the MTU of the 1st physical interface
 because the other bug that is similar to (1) of this report could not
 be resolved
 without the behavior change.
 For that reason, please use the following command to set MTU to lagg(4) ins=
 tead.

          !ifconfig wm0 up
          !ifconfig wm1 up
          create
          mtu 9000
          laggproto lacp laggport wm0 laggport wm1 192.168.2.1 netmask =3D
          255.255.255.0
          up

 Thanks,
 yamaguchi

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/57650 CVS commit: [netbsd-10] src
Date: Thu, 19 Oct 2023 07:23:51 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Thu Oct 19 07:23:51 UTC 2023

 Modified Files:
 	src/share/man/man4 [netbsd-10]: lagg.4
 	src/sys/net/lagg [netbsd-10]: if_lagg.c
 	src/tests/net/if_lagg [netbsd-10]: t_lagg.sh

 Log Message:
 Pull up following revision(s) (requested by yamaguchi in ticket #429):

 	sys/net/lagg/if_lagg.c: revision 1.50
 	sys/net/lagg/if_lagg.c: revision 1.51
 	tests/net/if_lagg/t_lagg.sh: revision 1.10
 	sys/net/lagg/if_lagg.c: revision 1.49
 	tests/net/if_lagg/t_lagg.sh: revision 1.9
 	share/man/man4/lagg.4: revision 1.5

 lagg(4): release LAGG_LOCK before mtu changing
 PR kern/57650

 Make the lagg interface up before change its MTU
 This change is related to PR kern/57650

 Fix missing IFNET_LOCK holding while destroy the lagg interface
 copy MTU of lagg to a interface added to lagg
 even if the interface is the first member of the lagg

 This change breaks ATF test case for lagg MTU

 Update the test case for MTU of lag to adapt new behavior

 Update lagg(4) manual
 1. corrected the wrong example
    - lagg(4) can not add multiple port and set its priority at once
       - This is the restriction of ifconfig(8)
 2. adapted to changed behavior related to MTU
    - Changed not to copy MTU of the 1st physical interface
      to lagg(4) to prevent locking against myself


 To generate a diff of this commit:
 cvs rdiff -u -r1.4 -r1.4.2.1 src/share/man/man4/lagg.4
 cvs rdiff -u -r1.48 -r1.48.4.1 src/sys/net/lagg/if_lagg.c
 cvs rdiff -u -r1.8 -r1.8.2.1 src/tests/net/if_lagg/t_lagg.sh

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Home	PR Database Search