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: <882349c0-123d-3deb-88e8-d400ec702d1f@fb.com>
Date:   Thu, 31 Mar 2022 20:01:52 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.7.0
Subject: Re: [PATCH] sample: bpf: syscall_tp_user: print result of verify_map
Content-Language: en-US
To:     Song Chen <chensong_2000@189.cn>, ast@kernel.org,
        daniel@iogearbox.net, andrii@kernel.org, kafai@fb.com,
        songliubraving@fb.com, john.fastabend@gmail.com,
        kpsingh@kernel.org, netdev@vger.kernel.org, bpf@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <1648777272-21473-1-git-send-email-chensong_2000@189.cn>
From:   Yonghong Song <yhs@fb.com>
In-Reply-To: <1648777272-21473-1-git-send-email-chensong_2000@189.cn>
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?enl5QTFQOUZuZ3lSb0J5bXRTTHc1WFIrU3lDZnBUYjhPc0h0UTVscUU5c29B?=
 =?utf-8?B?dVRCUm1CSFk1WktLV0hHUDlUaTUzcmgraVorOGdXczRWN1VQWVVhOHpGRXdT?=
 =?utf-8?B?Q0pheHBsS2IvL0JRWC9rb1I0SUhvMzdsU09SeWd4QVUweVpCUVdualdhcFBr?=
 =?utf-8?B?eWZrSU5ycWlSUFBnM2ZxVDVXeE9JSXVnbGxkZGoySUg4NjkyRFZTT2kzUnpH?=
 =?utf-8?B?SVUwak5VTmFnZ1daRFE4eFF2TUFyYjk2ZjErVXV0aWFOQ1BTVWRoNVRJcFdC?=
 =?utf-8?B?QzMvRnVMZUppak96dytDVFNPRVQ0akQrSU80RFRRbmUxWTZyMlVlUFhUU2xz?=
 =?utf-8?B?VmQyUm9sMmRPZ0ZKYjNpUnp0UVQ0UktXbXU0YXJOMGJRQ1BiT05pS3BReGRM?=
 =?utf-8?B?bEpFc3BXcjlOaUZ2a2h4US93VUc4QUFOQnZ0cTd2VjVqL0ZSVUtXMC9TTFJE?=
 =?utf-8?B?R05tcVNqYkhtZWFrL2haU1Vma0t2c05yeDh6dDIvb1RPY21lbkd2RVpjckVV?=
 =?utf-8?B?eWlYRnJ0Nnd4dUtraWZBa3FNamNlb2pndDY4VllhQTJhOFdVUm9ualNuc3po?=
 =?utf-8?B?UVBxWXM4bGpyVk1ydlJqSHpTdlM5THF6d1dZSStPM1hscElrYlJoSW9WbXkv?=
 =?utf-8?B?VVdCVUhna0xsQjhpR3RqN1NWL1ZVbDI2T2ZNcmpWZnNLV1IxSkFPaTllcXdN?=
 =?utf-8?B?T0J4NjFjSlBueDB6NHU3MitDZERhMWZQMVdwczJHeHo2SW90VnhqclZLWS9D?=
 =?utf-8?B?VHlTRWJJRmNhbHlXNjFtRU84ejBNUWIvL1J0OGxDdk11NnhHTTIzTzZJWjJv?=
 =?utf-8?B?K0EzT0RkM0JkUmhTaHdvQ3l5MXVIMDh0MVZaeWpBd1RNZGxiRHFGYnZOMDlB?=
 =?utf-8?B?WWRadm1GUmpIYldaVmwwMFdleUs2MEVubXhZd2VQRjlLUFhqWENJbW0yTndh?=
 =?utf-8?B?djltcGtpaGE0eDkxeEt1dzJsQkVQcFkzandsdUpLb0RqVHBYdmxhdjFtKzFP?=
 =?utf-8?B?MnlRMUd6OVkzanhrV3JxTnlHSjdXS0V4MGtkUkZXR3NMN0RXTGZ0ZWNvWGdI?=
 =?utf-8?B?VnAxVTAxbFVxZjVGek1Rckg3VG1pYlUwcm13Rk9DMCtZQk5takJXeUtLclpR?=
 =?utf-8?B?RzV3a0xMWE12TUZMQWFzdEVhK2hFeE85UHUraWtmZGxGRjE0cEJDR2xuUzhO?=
 =?utf-8?B?cDF2NFVBT0FHWVpSRThpSkpna2xIeVJiaGI1ZTlseVZET2tDU2wxTU5FUitv?=
 =?utf-8?B?VGpxaGFZejVsVS9wZDE3YXJSVGx4UXd2UnlZWlpSZlpIMkJFKzJWaVd3S3Mr?=
 =?utf-8?B?OEcxekRGZnlrRlp2bVJtUlNGanhyWW5NYnFaNmhUKzdoaThsQTlGRkpybUtS?=
 =?utf-8?B?cU1INGFJMThRcjVVVnBUbk9JV2JEUDd4aU56RnJ4VFBsaEE4eGlzeW1EcWhu?=
 =?utf-8?B?SW8zWG1PVSsvWURhWFFkaTczcVJzaWVTaW1UU2cxTk9CUnQwNHZIZWxYck5M?=
 =?utf-8?B?Y2IydHFaNWxoMzR0b1BoNGFScHFjcjFUZ013Ujkyd2c5c0NuSVJSRk1WdUFV?=
 =?utf-8?B?cllYaEk0a2dmYW8yNkRhY2x4YldDQXhXR0YvNVp2dHdyMjMyVTlIRGtuclhn?=
 =?utf-8?B?a3UzeXNPRGdQYWwwcU04WlRCSG5nV3Btc25Vc05oYXcxQWJNNVc0eG9CREVz?=
 =?utf-8?B?TnV4emVwUW9Fa2NoN0NrKzBRY3VGYVM4eWQyTjRGTVpyM3dMc0dSRGNpNFUy?=
 =?utf-8?B?TGFwZFFGbWtEaWVUOXh3Vk0vbCt3d05uM0xTajdUTUc0NDRFeFFSV09rbmsv?=
 =?utf-8?B?UWIyRFpQQWRWM0I4aW5OUWhEajlKU2xxRnFqOThaUjZYUGx5ZnVDc3Z5TkVN?=
 =?utf-8?B?ZXc5UVFiL2tuNGtubklTU1dTSDlrU1lqa3I1eVNXb1pqZGptck1Dd0Y2YTds?=
 =?utf-8?B?NUhwT3JIYnZ4ZTh2bVI1RTNxeEwzUzBFTGJKbmhOTW1sell6K0Z0bURwRDZn?=
 =?utf-8?B?cVJYbzJyMXdwY3JYZmpUeUp6RlZRZndDS2ZBUjdodnBXVHllS2lGc0FHT0cy?=
 =?utf-8?B?L29YRmNkOE5xWlFsZmJKVjFzR1I4akZNRWJmeis4d2hPMjZHMUUyZUNYRXRS?=
 =?utf-8?B?ZldFK2xXWVdXSnNxallBd0F0YTE2VThWZmpUK2pZZGZxV0dhc1B4ZWhWdUFX?=
 =?utf-8?B?Q3ZrRDVUSk13cEkxWFVJcm1sbHhTSS9FMzFhMVdUeWpYeCt5K3I1dzNnOHRP?=
 =?utf-8?B?MmdVcytTc0VLZjhBZFlEcCtia1RnbHFhamZGYmE3ZzgybUVvVjluYnhNZmtJ?=
 =?utf-8?B?Q2RBRDRhbUhxNFM4NC9ZTmtxaGt2cHRmcHRkY1ZFd1dTTllLUTFRMHoydmJW?=
 =?utf-8?Q?xdmEtNW9YtLjIKKM=3D?=
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6cd1ba6f-15b1-4a9d-8f6d-08da138bfabb
X-MS-Exchange-CrossTenant-AuthSource: DM5PR1501MB2055.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Apr 2022 03:01:55.6355
 (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: WvPI73syI2+tmrlCvprbBAlwfPtYlIyQMOOTernpYTG1YwrfxFJGf4NJYg3v1bRh
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR15MB5275
X-Proofpoint-ORIG-GUID: qa2xWRwyBUqjLhMy0jgPDNb1AlnRRnce
X-Proofpoint-GUID: qa2xWRwyBUqjLhMy0jgPDNb1AlnRRnce
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.850,Hydra:6.0.425,FMLib:17.11.64.514
 definitions=2022-03-31_06,2022-03-31_01,2022-02-23_01
X-Spam-Status: No, score=-2.1 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,T_SCC_BODY_TEXT_LINE 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 3/31/22 6:41 PM, Song Chen wrote:
> syscall_tp only prints the map id and messages when something goes wrong,
> but it doesn't print the value passed from bpf map. I think it's better
> to show that value to users.
> 
> What's more, i also added a 2-second sleep before calling verify_map,
> to make the value more obvious.
> 
> Signed-off-by: Song Chen <chensong_2000@189.cn>
> ---
>   samples/bpf/syscall_tp_user.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/samples/bpf/syscall_tp_user.c b/samples/bpf/syscall_tp_user.c
> index a0ebf1833ed3..1faa7f08054e 100644
> --- a/samples/bpf/syscall_tp_user.c
> +++ b/samples/bpf/syscall_tp_user.c
> @@ -36,6 +36,9 @@ static void verify_map(int map_id)
>   		fprintf(stderr, "failed: map #%d returns value 0\n", map_id);
>   		return;
>   	}
> +
> +	printf("verify map:%d val: %d\n", map_id, val);

I am not sure how useful it is or anybody really cares.
This is just a sample to demonstrate how bpf tracepoint works.
The error path has error print out already.

> +
>   	val = 0;
>   	if (bpf_map_update_elem(map_id, &key, &val, BPF_ANY) != 0) {
>   		fprintf(stderr, "map_update failed: %s\n", strerror(errno));
> @@ -98,6 +101,7 @@ static int test(char *filename, int num_progs)
>   	}
>   	close(fd);
>   
> +	sleep(2);

The commit message mentioned this sleep(2) is
to make the value more obvious. I don't know what does this mean.
sleep(2) can be added only if it fixed a bug.

>   	/* verify the map */
>   	for (i = 0; i < num_progs; i++) {
>   		verify_map(map0_fds[i]);