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;
Subject: Re: [PATCH] libbpf: replace '.' with '_' in legacy kprobe event name
To:     menglong8.dong@gmail.com, andrii@kernel.org
Cc:     ast@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev,
        song@kernel.org, yhs@fb.com, john.fastabend@gmail.com,
        kpsingh@kernel.org, sdf@google.com, haoluo@google.com,
        jolsa@kernel.org, bpf@vger.kernel.org,
        linux-kernel@vger.kernel.org, Menglong Dong <imagedong@tencent.com>
References: <20230113093427.1666466-1-imagedong@tencent.com>
From:   Alan Maguire <alan.maguire@oracle.com>
Message-ID: <bdca73eb-07e3-2187-c46f-a3f14a9e50a4@oracle.com>
Date:   Fri, 13 Jan 2023 14:12:41 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.4.3
In-Reply-To: <20230113093427.1666466-1-imagedong@tencent.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?QVcrM2pZb1hUSkhNa1QyanYxSEdTRlRvay8vOGY0Z1drTDJadm45Zkl4cWRC?=
 =?utf-8?B?MGhtRVUrVGRsbGxsRmkrN2tZalRNbzF5eHQwcWJCdWtLUVIybTRpc1krUkZp?=
 =?utf-8?B?d3o4amNMUGxPVjVzT2pGcVdOaU9xMFRPRmNhS0lqRzF5Tmo2UkFkM1V1SG9r?=
 =?utf-8?B?RlZzTEdzclRseXBYbEdtYWNHTldZaEdYU1JpSElITlcxeGFCSkg2WjJjS2la?=
 =?utf-8?B?QldldjJQUThtb0dxVTNCWXJVaXVyNFVrNml4MXVVN2VOdXNIUDh5aXIvbDM0?=
 =?utf-8?B?WkNQV25VbWJTUktzY3FERUdSdUtFb3RUMFFlenhRRW1JdWhkMDBjd3Z2eXlx?=
 =?utf-8?B?RHVVTGxEcjNLd3VlY05mYnVOZDJGWVE4b0dHSENpMk1JZmorY3R5bFZMd1VZ?=
 =?utf-8?B?Q21vbTVHVzNZTTdHUFU4WVcxbitzd1p2UGpUVFN6UFVYdGRQY1pySENkQ2tT?=
 =?utf-8?B?dDcvUmg3V1lFalpYNDNyRldrS2RJM1JsR3l5ay9wcVBIeG1rTytUTkpRMmFn?=
 =?utf-8?B?NUN4aHhod0pSRDAzc1crSjRaZzcwQ2VHUzJvYlJxZHJaS0xzaDVRcm9LZ3hU?=
 =?utf-8?B?TzZTNzc4K09kaEw0ZHVYWjlwVjFoMHZlWExXMnZZaGJBVHBNZ0tkVHRQeGVW?=
 =?utf-8?B?VUNvcFYyMjhXemNsSzA2alphaWQ2RjZqTVJsMnJBWGp6WW13MWVFK1g4c251?=
 =?utf-8?B?eGhoSStaUHNMVThUMzhhUklFWGpFVWVxNnlJZlUvblV5RStLNE5SUTA3L1VW?=
 =?utf-8?B?L0NYQnJiNGk1L05BZG5PWUFid3dsWHU4R1VjY2Z6NWs0cXQvcFc3OTBjVGJU?=
 =?utf-8?B?dzFiZ1FpTzdKMGY1MFNENzJ3UzhIRWpvSGRjbDVES0ZQS1lqWXM1eXFGYkZD?=
 =?utf-8?B?Z3orUU45cTM0OU42eUoybHEvdm9kTnBWRG8yS0JHeUJxRmg4QUxRQVpvUFdp?=
 =?utf-8?B?YUkwL1BDYjFqdUdzaHd3YjEwY0RPMnFISFZQdlFMMDJ0RHJLZkVIdVErd3Zu?=
 =?utf-8?B?KzVJUkl0bDUzVUpVbVVYMXJya1JVN0ZXSmczQVZucGgwK2lqZmFZbU9VZmYy?=
 =?utf-8?B?TENLZXZzaDJyTmVudXBKeUkxNHdRMFNoQTJKS3F5ZUFCcWZzbERSSVowLzky?=
 =?utf-8?B?UitBK3puV1pOTFJCTittQlRrUVgwYmgzc0NWYnF3ZThxN1luZmlEaVlHekpP?=
 =?utf-8?B?Tmt1Q0xQdTFNcXI2M3BIaG5RN21EZ1VBOU51SnNha3JXcHZ0RXVwaVQ3Z3NL?=
 =?utf-8?B?dnU4aFNHU1ZkdndpMjRpWE9kK09oOGY0aWhqc0hmQnJYVTMxdEJSS0dLY0E4?=
 =?utf-8?B?L0s3SXM5T3VSZlVXbURpMll0U0pNY2s0b2kyOGRqYkNWTVN2WkJTd0U2UmRD?=
 =?utf-8?B?VS8wNmtMWFJlTmdyK09aR1drNkVCNy90b3laNC9HZnczOXpTK1ZkMnFEVzdj?=
 =?utf-8?B?OGI0Q1VGd29VV1RacUNMT2FYL0FyakZDWktxblg5dUVXRnR0L0pzTEs5SmJs?=
 =?utf-8?B?ZEVCTndHTUtteStGaSt5d3VDZ1pxdkJ1ZmlraW1PT0JjdHpBa0hpVHl2R3kv?=
 =?utf-8?B?VVBINGZaRytyMmhYWEl5M0k3T2F4dWdrcFEwRmtSS3hDTTVEUXRMcDdYbjZo?=
 =?utf-8?B?amltam9xaW5DNytCODlPeUNsUGRXSmduYm5xL3RadklKM0syZ0JIVUhVTWYr?=
 =?utf-8?B?SERqTm83TE5zMStLeEQ4RmhZU2NvUHJHMHdINC9KQW4weVl1azh0empZV0k0?=
 =?utf-8?B?L3lDdVFoT0hXS0hyR3preDZIL0Q0a3VpckpvQUVFTXNYTFpQVXNQcDJuTThS?=
 =?utf-8?B?WWl5UzQ2eWFMVENxUTByZDMrd05ZNFYyZW14dVlLUGpOTGMwSlZtNzNrbE15?=
 =?utf-8?B?eTBQVlN2VzlYU0UxY0p5WTdoK0ttV0NVVGRKdzVnRGJsWlNIRXRyVGxVK00z?=
 =?utf-8?B?N1U3cTdPWjZhUGhwdDJZRXlrejBkTjhiTDgyZWpheVk3Y1FEb1VCeVR5aWVY?=
 =?utf-8?B?WW8wT1JGbkxPM3VyWE8wc3JhSG9NSTdvd1BTd0lpNWl6ZENpNVZWMkdsdjBw?=
 =?utf-8?B?R1h2MWtWY2wraUwrL29VOGVERlZ3ZUt5aDhIWmZRRVpIc21jN1ZTbmJFWE5j?=
 =?utf-8?B?TG41M3QwbzR6TjJ5eGNHb1lDbGZ3NHoycnRsa1dvSXA4aDFKb1NVcGpUL040?=
 =?utf-8?B?VjFQeDFpcjByMG5IOUJxYkZvUnNrd1oyK2R3OUdQR2Z2S0V3S25ZMnJvZlgw?=
 =?utf-8?B?cUtaTW5HT2tYUUxkT2M1WUFrR01RPT0=?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?YW1mNGY4cDlyZE1PdlNERkNMNjRIdXVBUjNzYWZxYkhNV3cyZWUxWGNWSXhO?=
 =?utf-8?B?c2FTYlNXeFFPMFZzMWRjakhzYStvWXE1OUNpbkhMdHF4MFJDMmVhODdHMmY4?=
 =?utf-8?B?c05lR2piQnhsZDFxa3d0YllhTklmSEREcVcvTlhJKzU5YWVRTmpPcmYzdmtn?=
 =?utf-8?B?QnNKWXhXSHZoY21EZmswM3NqK054VS95NG9WVTN1L0wxSGRSczlFOS9rMG0r?=
 =?utf-8?B?ZjBkZ3hiMmNJaUNERVMwcGFUWFg1bVJkU1FUU21mM3BmM3NuQkVwcWtrVFI1?=
 =?utf-8?B?SGVST2NLZ0tLdzIwcnpteWN3cXNEekU2cnNTRjFBdXFCZE14alFWQ0wyTFlC?=
 =?utf-8?B?UitJOWR0UEY3STFRMzgrSWpmclFpSUdxaGYvU0dOOHV3TDZjM3QrRHVJY0JX?=
 =?utf-8?B?UHU4ZlI3bUltcStqZTJzZERMSngvb2dERDZCTEhjV3lwK2RQWlRSa0h4eGd1?=
 =?utf-8?B?N2xZNjExR1NoOSt6YktzMVhXZUtOaHJkNk5pZjMreVJrcEpPcmlOMTJ0czkw?=
 =?utf-8?B?VU1EdWxlVFFmMlRiTzZsZUd1OWtFVmxhZjVtZHljUlJKL3B1U21qbFU4ZGVQ?=
 =?utf-8?B?bngrT1BmazlxSVkxNkVWMXh3bDdIMnordDEwTmdHRys2a2FYa3gzQWRKTnE4?=
 =?utf-8?B?T2E1ODRXT0h2SVo5TWNLaEpxWnVyaW5OTlp1dXFld2pJL0gzL0F2eDBuMEx0?=
 =?utf-8?B?bU9YaHRFNVVvckpncC9YUDNQNTY2cS9nQmtVVjRUc0VNRTJxQTZET3VQNEIy?=
 =?utf-8?B?YVBDTHdDU2lmeWE2OGl4K2RUbitUMm93ZmJhTEFXTlhxM3dEdDNIZW56UlEy?=
 =?utf-8?B?ZWNNRTRhby9TdTJHUTluQ25yR0l2eHZyUkgra1ZxK0dsQWhrbDQ2VzUrTzE5?=
 =?utf-8?B?bG1TVlVrZ3VBTlBpVzZzTE5WUC9GZ3dLVmMrR0pTZW9sTk9QcHZjT1dCYUxW?=
 =?utf-8?B?dFlDcUhEMWZRZXZLalBWdE56UThRbHdMVWQxK3k1T3lyZ0VEdDk3YUYyaFBY?=
 =?utf-8?B?Vmd5KzRmcFZudWgzMStLVmlSRm12aGUyaG1ucDNORW56Y2hZZm1TTVY5cUJo?=
 =?utf-8?B?Y2hGcTcvNnc2dHUybTVTR2MwTVNsRkFGWUNnQ3RxeERFZWtRSzZ6UnNzeExm?=
 =?utf-8?B?dytFd2F0ZTVScDJJeHBTLzdncEJZb3JOVjZ1enNyVFNXZ2tUbDFGZmNOZVNW?=
 =?utf-8?B?bjJtVVI4Nm9uL2tYWWRGeVUzbEY5cUptV3lRM3l2REhZQnY3YnByWThVWXdR?=
 =?utf-8?B?a0RwUk9nM2x3djkrR1lCU2lxL2p1cDBSUkhseFIvTWtIZ1Yxbk1MRWxKRkNr?=
 =?utf-8?B?L0FMSVRoTnVKOE44eFNaYmFRY0Z5T2JYYlRMTzhRUjBScUZLcTgrZ2VXdWNr?=
 =?utf-8?B?dHQ2N2ZlaWZYSy8ycmdGQVNldGQySnQydStPVGRqMy95YkF3cEJZRENJZDli?=
 =?utf-8?B?VDdDaXZpQW4yM2JqeUNnVmpKTTYrVTNUL2dTSzhtMWNyeVpHdW1pcFh5SjZa?=
 =?utf-8?B?NDB6UEJEeE1vWGxpeDR2SWl1ZmxLdHB4WjlnWElMb2JRaW1SWWt6N2tUMUtD?=
 =?utf-8?Q?L/roxeEaPKWI0JqASFMy0teaY=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ef7eeea6-0bae-49ac-a1ff-08daf5703efe
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB5267.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jan 2023 14:12:47.0550
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: V6nvxjWLcFqlsa0rp8qO/qShMsfA9+02gc5Onm0U8iymQKz+A2KXJ6XSo1vjJ9hesUDgtglaX9+RlUoiL6RMug==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6625
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.923,Hydra:6.0.562,FMLib:17.11.122.1
 definitions=2023-01-13_06,2023-01-13_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 mlxscore=0 spamscore=0
 bulkscore=0 adultscore=0 suspectscore=0 malwarescore=0 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000
 definitions=main-2301130093
X-Proofpoint-GUID: cqsP72NM5jJNuEzK1V23A2dn6tYwGE4S
X-Proofpoint-ORIG-GUID: cqsP72NM5jJNuEzK1V23A2dn6tYwGE4S
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,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 13/01/2023 09:34, menglong8.dong@gmail.com wrote:
> From: Menglong Dong <imagedong@tencent.com>
> 
> '.' is not allowed in the event name of kprobe. Therefore, we will get a
> EINVAL if the kernel function name has a '.' in legacy kprobe attach
> case, such as 'icmp_reply.constprop.0'.
> 
> In order to adapt this case, we need to replace the '.' with other char
> in gen_kprobe_legacy_event_name(). And I use '_' for this propose.
> 
> Signed-off-by: Menglong Dong <imagedong@tencent.com>
> ---
>  tools/lib/bpf/libbpf.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index fdfb1ca34ced..5d6f6675c2f2 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -9994,9 +9994,16 @@ static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
>  					 const char *kfunc_name, size_t offset)
>  {
>  	static int index = 0;
> +	int i = 0;
>  
>  	snprintf(buf, buf_sz, "libbpf_%u_%s_0x%zx_%d", getpid(), kfunc_name, offset,
>  		 __sync_fetch_and_add(&index, 1));
> +
> +	while (buf[i] != '\0') {
> +		if (buf[i] == '.')
> +			buf[i] = '_';
> +		i++;
> +	}
>  }

probably more naturally expressed as a for() loop as is done in
gen_uprobe_legacy_event_name(), but not a big deal.

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>

One issue with the legacy kprobe code is that we don't get test coverage
with it on new kernels - I wonder if it would be worth adding a force_legacy
option to bpf_kprobe_opts? A separate issue to this change of course, but
if we had that we could add some legacy kprobe tests that would run
for new kernels as well.

Alan
>  
>  static int add_kprobe_event_legacy(const char *probe_name, bool retprobe,
>