Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH bpf-next v7 5/7] bpf: Implement bpf_local_storage for
 inodes
To:     Martin KaFai Lau <kafai@fb.com>
Cc:     linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Paul Turner <pjt@google.com>, Jann Horn <jannh@google.com>,
        Florent Revest <revest@chromium.org>
References: <20200730140716.404558-1-kpsingh@chromium.org>
 <20200730140716.404558-6-kpsingh@chromium.org>
 <20200731010822.fctk5lawnr3p7blf@kafai-mbp.dhcp.thefacebook.com>
 <adbfc73e-bd32-d9ba-4dab-4ccc39b40fdd@chromium.org>
 <20200731190226.6ugmk6cnl2yortgt@kafai-mbp.dhcp.thefacebook.com>
From:   KP Singh <kpsingh@chromium.org>
Message-ID: <c376c46c-f8e9-8a4c-3f81-300faddac831@chromium.org>
Date:   Mon, 3 Aug 2020 17:41:54 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0)
 Gecko/20100101 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20200731190226.6ugmk6cnl2yortgt@kafai-mbp.dhcp.thefacebook.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 31.07.20 21:02, Martin KaFai Lau wrote:
> On Fri, Jul 31, 2020 at 02:08:55PM +0200, KP Singh wrote:
> [ ... ]
>>>> +const struct bpf_map_ops inode_storage_map_ops = {

[...]

>>
>> btf  dump file /sys/kernel/btf/vmlinux | grep "STRUCT 'inode'"
>> "[915] STRUCT 'inode' size=984 vlen=48
>>
>> So it seems like btf_id[0] and btf_id[1] are set to the BTF ID
>> for inode. Now I think this might just be a coincidence as
>> the next helper (bpf_inode_storage_delete) 
>> also has a BTF argument of type inode.
> It seems the next BTF_ID_LIST(bpf_inode_storage_delete_btf_ids)
> is not needed because they are the same.  I think one
> BTF_ID_LIST(bpf_inode_btf_ids) can be used for both helpers.
> 

Cool, yeah. I have fixed it and also for sock helpers. Will
send a new series out.

- KP

>>
>> and sure enough if I call:
>>
>> bpf_inode_storage_delete from my selftests program, 
>> it does not load:

[...]

>> ./test_progs -t test_local_storage
>> [   20.577223] btf_ids[0]=0
>> [   20.577702] btf_ids[1]=915
>>
>> Thanks for noticing this! 
>>
>> - KP
>>
>>>
>>>> +
>>>> +const struct bpf_func_proto bpf_inode_storage_get_proto = {
>>>> +	.func		= bpf_inode_storage_get,
>>>> +	.gpl_only	= false,
>>>> +	.ret_type	= RET_PTR_TO_MAP_VALUE_OR_NULL,
>>>> +	.arg1_type	= ARG_CONST_MAP_PTR,
>>>> +	.arg2_type	= ARG_PTR_TO_BTF_ID,
>>>> +	.arg3_type	= ARG_PTR_TO_MAP_VALUE_OR_NULL,
>>>> +	.arg4_type	= ARG_ANYTHING,
>>>> +	.btf_id		= bpf_inode_storage_get_btf_ids,
>>>> +};
>>>> +
>>>> +BTF_ID_LIST(bpf_inode_storage_delete_btf_ids)
>>>> +BTF_ID(struct, inode)
>>>> +
>>>> +const struct bpf_func_proto bpf_inode_storage_delete_proto = {
>>>> +	.func		= bpf_inode_storage_delete,
>>>> +	.gpl_only	= false,
>>>> +	.ret_type	= RET_INTEGER,
>>>> +	.arg1_type	= ARG_CONST_MAP_PTR,
>>>> +	.arg2_type	= ARG_PTR_TO_BTF_ID,
>>>> +	.btf_id		= bpf_inode_storage_delete_btf_ids,
>>>> +};