Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bf56b01d-4c05-0d0b-e85b-219e55606803@fb.com>
Date:   Fri, 15 Jul 2022 21:33:32 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.11.0
Subject: Re: [PATCH bpf-next v6 06/23] selftests/bpf: Add tests for kfunc
 returning a memory pointer
Content-Language: en-US
To:     Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Greg KH <gregkh@linuxfoundation.org>,
        Jiri Kosina <jikos@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>, Shuah Khan <shuah@kernel.org>,
        Dave Marchevsky <davemarchevsky@fb.com>,
        Joe Stringer <joe@cilium.io>, Jonathan Corbet <corbet@lwn.net>
Cc:     Tero Kristo <tero.kristo@linux.intel.com>,
        linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
        netdev@vger.kernel.org, bpf@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org
References: <20220712145850.599666-1-benjamin.tissoires@redhat.com>
 <20220712145850.599666-7-benjamin.tissoires@redhat.com>
From:   Yonghong Song <yhs@fb.com>
In-Reply-To: <20220712145850.599666-7-benjamin.tissoires@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Mml1S1RWcndKd3BuWCtLV0tLQk9hdGJlNEQwQjh1Qm9mTkgvRUsySDZ3T1Y5?=
 =?utf-8?B?bExkL0VCaHM0ZVRJMm0ydzk0aEVabHlDek4xUmdzUVJ0WDMyUTlCNERXempv?=
 =?utf-8?B?ZDczZTdHSmNJOTVwTHh2T1FqZnJJM25JTm13bkJXbnlTaDExdm5sNThyM0Uv?=
 =?utf-8?B?SGEwZXM4WW8zUUR0dEFVZFBSQnVFWmRSK1BJQi8rZDNNSElhYjlaQ3l1TGRu?=
 =?utf-8?B?Ty8yRStTaCtHQnc2YWx6d013b3lWS0RNNnk2Z2poTlIxcEk0cHQzTFptaFE5?=
 =?utf-8?B?aDFJdWR4alFBeDFSTTB0MU4xT09wbkJ2ZnUzLzVYMGxRSGYrQmUramRqYnRE?=
 =?utf-8?B?bVBoSmlDZ0lIR2NtOGE1V01JRVJURFZUSE43ZVdJWE42WXR4clpkdE00akVP?=
 =?utf-8?B?a2cxRjNXczdDL245WEJoQytOT1JpeE9yN3BnRnRVV0IvZC81VC9CT0QvV2hk?=
 =?utf-8?B?czFOc3ozeFpBN3VwanVmcERkam9aZ1ZYcFB6OWw4T2hrOEZuRENnN2JiOTB6?=
 =?utf-8?B?dnFYT3ZmZjdpeTJvSkF1RnNkM0t6SkszY1ZnM0Y1eE5XdjZEOVZCRThoV2Mv?=
 =?utf-8?B?Zk5OL3kxaTJ1aC9HZUhnTWJJWDZUMVQ5VjA4ci9hMFdPLzZ3bE9zTWRYYjVz?=
 =?utf-8?B?WTZ5cng2VmZ2VGpObzFrWS81c3VXZGU5bTFWcDdLSVZHRnNhajVZemwwbHgy?=
 =?utf-8?B?cWtwZ3gxTElBUEFhb3pOV3BkQmZlVHhDVkUyOHpDSHltZkFKZ1NYWWhZUlB6?=
 =?utf-8?B?bUNZell2Ny94anBCRXp3MWxCWnQ3eC8yR3RsUjd5bDZ4aUFzZWdiaktIOU9o?=
 =?utf-8?B?MWtmaW9aRFVGUkNQMlhpVnI4QitheTVJa3VBeDM0bjVnQVk3ZUYxTGx6UDhk?=
 =?utf-8?B?Rzk5bCtiZ1NLRWVpYUZmWkdGc3poMW5vU3Z6b3RiekU4VXZZQmtNNlpHOHlV?=
 =?utf-8?B?UkQ1MHBNOGVXcU5TK3dDNCs1L0tTUm03TmF5RkVrTDhUUDdFU21OZlBCRmlJ?=
 =?utf-8?B?aGx3eWs2YkFITy9JWXpNUTJ4eVZ2Mml1aVFaSmNRVjJkUUZNY3ZVYXBxbXFu?=
 =?utf-8?B?R1UwM3I2VXBBc3padnNWU0Fjc0VGaHgzOUt2a1J3bXJhVm85eldPR05ZYzA0?=
 =?utf-8?B?OTg4WDZBRTRZVGdMZHlOTmR5bnlUeFhlaytuS3FkU0xSbkZXdk1jRkFmci80?=
 =?utf-8?B?TWVRR1VQcThOSm9iNlAzSUJoQlpJdkJDSUZLNDJBUG1GWVFXVGZTVUJKRXVQ?=
 =?utf-8?B?RnBQNGNBYjl4cWJVbENtdGpvS2FUMEx5WHh5bEw3WStwaDhIUzJDRUtYc3Z0?=
 =?utf-8?B?QXJ1MHEyd2F4Y0FtS3R6aXBGOVdjUHlSL0NOU1JNRXZLRm9Pck96L2JjcnNN?=
 =?utf-8?B?WkhhdFg3QzY0VHVaWVhmd3BKaW1FT1hzSk1TOXEyNDFmUTgwdGpNOElVTFBV?=
 =?utf-8?B?L1FmQXVIV1BnaGtwTkQ2VVplSGdUajJxTzQyMWNzQWxCU3haeVYyK2gyTVRS?=
 =?utf-8?B?TUtHWDc3WFNtRjNwdHhnQmdXRmk3RGdHQ09TbXplZlVaSmFieGFJVDcwQTM5?=
 =?utf-8?B?L1Z3cXlzWlpOUjZPZkE5ejhBTWRlYjZzZWZPYVErVTl0Ni94UUVlUDFTMERC?=
 =?utf-8?B?ZkFicElRbzFyY3U0aGpkMHkwTGpYanpjSDI0NUJBaXBZalRqM0lOVSs2THBO?=
 =?utf-8?B?MWIvZ29hQUhmYjh1bllLV0hGL21NeDhHbk1WaitLK3pYTGI4ZGt0c3JBSEl1?=
 =?utf-8?B?eVZ4UHFOaGUzTjU3WDd3T2FZdTJhYU1IU3dUSEZmZWVxeTY5dEFXQ1NBd0g0?=
 =?utf-8?B?ejFXVHpreVZDYlg3WkFIOGlvZDRtbGpwTDFndHJlVjVQNy9MVG9FcjZ3dDFo?=
 =?utf-8?B?b3FRdG9PV0ZZZTVMZXZudGE2bGkzdnRxNWhuTEhrKy8vL2pwMS9KdUZ5R3Ez?=
 =?utf-8?B?dzV3c21xT2lOQnEwYTBwc2s0blg0OUIyUGZtSFNxZ3FXZVdqWGpYT3lja2N0?=
 =?utf-8?B?dUhiNXZrdm8vbzJJTjd1UmlES1ZFS1BscFFlVmFEa0FKQXlqckk2WVNydVZ1?=
 =?utf-8?B?VDRaTk9PUHJHZ01xYUF2M2k4NGZxQ2RXRzRqWG9jNnhmTDJFNHhjRWdKdjVF?=
 =?utf-8?B?ZU1LSm1CWkVSMkVzaVVUTkJ3WDYzbHIrVmltRTJURzFuUzJnQUowNDFrek5a?=
 =?utf-8?B?VXc9PQ==?=
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ef47c817-a301-4782-c33e-08da66e45852
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jul 2022 04:33:34.8304
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: GR/l59+y0MXzcTSNqWfPu5iumA3mRXgPfsOC5q5GeDFZJ/jamd5O7nltmu90e6Ob
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR15MB4645
X-Proofpoint-GUID: C0GSx3o68rdKKte8Pwxehp_JkOIZR0eL
X-Proofpoint-ORIG-GUID: C0GSx3o68rdKKte8Pwxehp_JkOIZR0eL
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-07-16_03,2022-07-15_01,2022-06-22_01
X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 7/12/22 7:58 AM, Benjamin Tissoires wrote:
> We add 2 new kfuncs that are following the RET_PTR_TO_MEM
> capability from the previous commit.
> Then we test them in selftests:
> the first tests are testing valid case, and are not failing,
> and the later ones are actually preventing the program to be loaded
> because they are wrong.
> 
> To work around that, we mark the failing ones as not autoloaded
> (with SEC("?tc")), and we manually enable them one by one, ensuring
> the verifier rejects them.
> 
> To be able to use bpf_program__set_autoload() from libbpf, we need
> to use a plain skeleton, not a light-skeleton, and this is why we
> also change the Makefile to generate both for kfunc_call_test.c
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> ---
> 
> new in v6
> ---
>   include/linux/btf.h                           |  4 +-
>   net/bpf/test_run.c                            | 22 +++++
>   tools/testing/selftests/bpf/Makefile          |  5 +-
>   .../selftests/bpf/prog_tests/kfunc_call.c     | 48 ++++++++++
>   .../selftests/bpf/progs/kfunc_call_test.c     | 89 +++++++++++++++++++
>   5 files changed, 165 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 31da4273c2ec..6f46ff2128ae 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -422,7 +422,9 @@ static inline int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dt
>   
>   static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type *t)
>   {
> -	/* t comes in already as a pointer */
> +	if (!btf_type_is_ptr(t))
> +		return false;

Why we have a change here?

> +
>   	t = btf_type_by_id(btf, t->type);
>   
>   	/* allow const */
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 9da2a42811e8..0b4026ea4652 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -606,6 +606,24 @@ noinline void bpf_kfunc_call_memb1_release(struct prog_test_member1 *p)
>   	WARN_ON_ONCE(1);
>   }
>   
> +static int *__bpf_kfunc_call_test_get_mem(struct prog_test_ref_kfunc *p, const int size)
> +{
> +	if (size > 2 * sizeof(int))
> +		return NULL;
> +
> +	return (int *)p;
> +}
> +
> +noinline int *bpf_kfunc_call_test_get_rdwr_mem(struct prog_test_ref_kfunc *p, const int rdwr_buf_size)
> +{
> +	return __bpf_kfunc_call_test_get_mem(p, rdwr_buf_size);
> +}
> +
> +noinline int *bpf_kfunc_call_test_get_rdonly_mem(struct prog_test_ref_kfunc *p, const int rdonly_buf_size)
> +{
> +	return __bpf_kfunc_call_test_get_mem(p, rdonly_buf_size);
> +}
> +
>   noinline struct prog_test_ref_kfunc *
>   bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **pp, int a, int b)
>   {
> @@ -704,6 +722,8 @@ BTF_ID(func, bpf_kfunc_call_memb_acquire)
>   BTF_ID(func, bpf_kfunc_call_test_release)
>   BTF_ID(func, bpf_kfunc_call_memb_release)
>   BTF_ID(func, bpf_kfunc_call_memb1_release)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdwr_mem)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdonly_mem)
>   BTF_ID(func, bpf_kfunc_call_test_kptr_get)
>   BTF_ID(func, bpf_kfunc_call_test_pass_ctx)
>   BTF_ID(func, bpf_kfunc_call_test_pass1)
> @@ -731,6 +751,8 @@ BTF_SET_END(test_sk_release_kfunc_ids)
>   BTF_SET_START(test_sk_ret_null_kfunc_ids)
>   BTF_ID(func, bpf_kfunc_call_test_acquire)
>   BTF_ID(func, bpf_kfunc_call_memb_acquire)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdwr_mem)
> +BTF_ID(func, bpf_kfunc_call_test_get_rdonly_mem)
>   BTF_ID(func, bpf_kfunc_call_test_kptr_get)
>   BTF_SET_END(test_sk_ret_null_kfunc_ids)
>   
[...]