Re: git: cf7974fd9e55 - main - sysctl: Update 'master' copy of vnet SYSCTLs on kernel environment variables change

From: Zhenlei Huang <zlei_at_FreeBSD.org>
Date: Sat, 23 Sep 2023 16:32:34 UTC

> On Sep 22, 2023, at 12:59 AM, Zhenlei Huang <zlei@FreeBSD.org> wrote:
> 
> 
> 
>> On Sep 22, 2023, at 12:39 AM, Peter Holm <pho@FreeBSD.org <mailto:pho@FreeBSD.org>> wrote:
>> 
>> On Thu, Sep 21, 2023 at 11:35:49PM +0800, Zhenlei Huang wrote:
>>> 
>>> 
>>>> On Sep 21, 2023, at 11:23 PM, Peter Holm <pho@freebsd.org <mailto:pho@freebsd.org>> wrote:
>>>> 
>>>> On Thu, Sep 21, 2023 at 10:45:20PM +0800, Zhenlei Huang wrote:
>>>>> 
>>>>> 
>>>>>> On Sep 21, 2023, at 5:55 PM, Peter Holm <pho@freebsd.org <mailto:pho@freebsd.org>> wrote:
>>>>>> 
>>>>>> On Thu, Sep 21, 2023 at 04:05:19PM +0800, Zhenlei Huang wrote:
>>>>>>> 
>>>>>>> 
>>>>>>>> On Sep 21, 2023, at 2:07 PM, Peter Holm <pho@FreeBSD.org <mailto:pho@FreeBSD.org>> wrote:
>>>>>>>> 
>>>>>>>> On Thu, Sep 21, 2023 at 04:13:04AM +0000, Zhenlei Huang wrote:
>>>>>>>>> The branch main has been updated by zlei:
>>>>>>>>> 
>>>>>>>>> URL: https://cgit.FreeBSD.org/src/commit/?id=cf7974fd9e554552989237c3d6bc736d672ac7c6 <https://cgit.freebsd.org/src/commit/?id=cf7974fd9e554552989237c3d6bc736d672ac7c6>
>>>>>>>>> 
>>>>>>>>> commit cf7974fd9e554552989237c3d6bc736d672ac7c6
>>>>>>>>> Author:     Zhenlei Huang <zlei@FreeBSD.org <mailto:zlei@FreeBSD.org>>
>>>>>>>>> AuthorDate: 2023-09-21 04:11:28 +0000
>>>>>>>>> Commit:     Zhenlei Huang <zlei@FreeBSD.org <mailto:zlei@FreeBSD.org>>
>>>>>>>>> CommitDate: 2023-09-21 04:11:28 +0000
>>>>>>>>> 
>>>>>>>>> sysctl: Update 'master' copy of vnet SYSCTLs on kernel environment variables change
>>>>>>>>> 
>>>>>>>>> Complete phase three of 3da1cf1e88f8.
>>>>>>>>> 
>>>>>>>>> With commit 110113bc086f, vnet sysctl variables can be loader tunable
>>>>>>>>> but the feature is limited. When the kernel modules have been initialized,
>>>>>>>>> any changes (e.g. via kenv) to kernel environment variable will not affect
>>>>>>>>> subsequently created VNETs.
>>>>>>>>> 
>>>>>>>>> This change relexes the limitation by listening on kernel environment
>>>>>>>>> variable's set / unset events, and then update the 'master' copy of vnet
>>>>>>>>> SYSCTL or restore it to its initial value.
>>>>>>>>> 
>>>>>>>>> With this change, TUNABLE_XXX_FETCH can be greately eliminated for vnet
>>>>>>>>> loader tunables.
>>>>>>>>> 
>>>>>>>>> Reviewed by:    glebius
>>>>>>>>> Fixes:  110113bc086f sysctl(9): Enable vnet sysctl variables to be loader tunable
>>>>>>>>> MFC after:      2 weeks
>>>>>>>>> Differential Revision:  https://reviews.freebsd.org/D41825 <https://reviews.freebsd.org/D41825>
>>>>>>>> 
>>>>>>>> This commit seems to cause:
>>>>>>>> 
>>>>>>>> b> bt
>>>>>>>> Tracing pid 0 tid 100000 td 0xffffffff8196ba00
>>>>>>>> kdb_enter() at kdb_enter+0x32/frame 0xffffffff821658e0
>>>>>>>> vpanic() at vpanic+0x163/frame 0xffffffff82165a10
>>>>>>>> panic() at panic+0x43/frame 0xffffffff82165a70
>>>>>>>> vm_fault() at vm_fault+0x18c9/frame 0xffffffff82165ba0
>>>>>>>> vm_fault_trap() at vm_fault_trap+0x6f/frame 0xffffffff82165be0
>>>>>>>> trap_pfault() at trap_pfault+0x24a/frame 0xffffffff82165c50
>>>>>>>> calltrap() at calltrap+0x8/frame 0xffffffff82165c50
>>>>>>>> --- trap 0xc, rip = 0xffffffff80c6f39b, rsp = 0xffffffff82165d20, rbp = 0xffffffff82165d20 ---
>>>>>>>> strsep() at strsep+0x3b/frame 0xffffffff82165d20
>>>>>>>> name2oid() at name2oid+0x66/frame 0xffffffff82165d70
>>>>>>>> sysctl_setenv_vnet() at sysctl_setenv_vnet+0x38/frame 0xffffffff82165e00
>>>>>>>> kern_setenv() at kern_setenv+0x324/frame 0xffffffff82165e40
>>>>>>>> nfs_setup_diskless() at nfs_setup_diskless+0x4b2/frame 0xffffffff82165f90
>>>>>>> 
>>>>>>> Is that a NFS diskless workstation ?
>>>>>>> 
>>>>>> 
>>>>>> Yes, sort of. It's a host in the netperf cluster, which uses pxeboot &
>>>>>> NFS.
>>>>>> 
>>>>>> - Peter
>>>>>> 
>>>>>>> I have tested that yet. I'll look at that tonight (UTC+8) .
>>>>>>> 
>>>>>> 
>>>>>> Let me know when you have a patch, so I can test it (if you like).
>>>>> 
>>>>> I believe I've found the root cause.
>>>>> 
>>>>> strsep(char **stringp, const char *delim) have side effect, it will modify (* stringp) .
>>>>> If stringp is pointed to some const value, such as "boot.netif.name" in 'sys/nfs/nfs_diskless.c', then
>>>>> the kernel will panic.
>>>>> 
>>>>> I'm preparing the patch, I'll reply when it is done.
>>>>> 
>>>> 
>>>> This is great news!
>>> 
>>> Can you please try the attached patch ?  
>>> 
>> 
>> Yes, the patch works for me:
> 
> Excellent !
> 
> I'm going to send the patch to Phabricator.

Alexander Motin has just committed f80babf906b7 (kern_sysctl: Make name2oid() non-destructive to the name),
that should fix this issue perfectly ;)

> 
>> 
>> root@mercat1:~ # uname -a
>> FreeBSD mercat1.netperf.freebsd.org <http://mercat1.netperf.freebsd.org/> 15.0-CURRENT FreeBSD 15.0-CURRENT #0 main-n265437-cf7974fd9e5545-dirty: Thu Sep 21 18:26:45 CEST 2023 pho@mercat1.netperf.freebsd.org <mailto:pho@mercat1.netperf.freebsd.org>:/usr/src/sys/amd64/compile/PHO amd64
>> root@mercat1:~ # cd /usr/src; git status | grep kern_sysctl.c
>>        modified:   sys/kern/kern_sysctl.c
>> root@mercat1:/usr/src # 
>> 
>> - Peter