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: <0de3722d-7111-246c-b558-b26d032dc5d0@meta.com>
Date:   Thu, 4 May 2023 09:48:23 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.10.1
Subject: Re: [PATCH v2 bpf 02/11] bpftool: define a local bpf_perf_link to fix
 accessing its fields
Content-Language: en-US
To:     =?UTF-8?Q?Michal_Such=c3=a1nek?= <msuchanek@suse.de>,
        Quentin Monnet <quentin@isovalent.com>
Cc:     Andrii Nakryiko <andrii.nakryiko@gmail.com>,
        Alexander Lobakin <aleksander.lobakin@intel.com>,
        Alexander Lobakin <alobakin@mailbox.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Shung-Hsi Yu <shung-hsi.yu@suse.com>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
        Song Liu <songliubraving@fb.com>,
        Kumar Kartikeya Dwivedi <memxor@gmail.com>,
        bpf@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220421003152.339542-1-alobakin@pm.me>
 <20220421003152.339542-3-alobakin@pm.me>
 <20230414095457.GG63923@kunlun.suse.cz>
 <9952dc32-f464-c85a-d812-946d6b0ac734@intel.com>
 <20230414162821.GK63923@kunlun.suse.cz>
 <CAEf4BzYx=dSXp-TkpjzyhSP+9WY71uR4Xq4Um5YzerbfOtJOfA@mail.gmail.com>
 <20230421073904.GJ15906@kitsune.suse.cz>
 <CACdoK4+KdM-sQKMO9WXk7kTL-x=Renjd0MuvSRT3JKbtzByyHQ@mail.gmail.com>
 <20230504081858.GV15906@kitsune.suse.cz>
From:   Yonghong Song <yhs@meta.com>
In-Reply-To: <20230504081858.GV15906@kitsune.suse.cz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VlBhNDVXK01mckVkaWVPY1JuSkFiVVpUNlRJNzdRN2V3UjN2SHlsUWhrZnl4?=
 =?utf-8?B?TjNGRFpKVDR0bXV5TzJlVkFRa1ZKV3loOE9ueFBsMjZFdHM5YlY3OFpRQjNw?=
 =?utf-8?B?WU1KRHR3STkydUlEYUg2OTljRksrV08vdm1BT1Rsck1NM0NoVytlRXNnWTNY?=
 =?utf-8?B?dHhHYjB5WTBPRGNOcjRxZUJUeG5VZ25tMVdSVXZzSTc4RFpCdHU5dDZGckR2?=
 =?utf-8?B?MHcwYktoZXR0Mm1tTkphTXRraTIzQnJlQjZtM1JCeUZoWVhxYWZ3MmFhY1R2?=
 =?utf-8?B?YWZBV1hxZDcwVXVDanB5b0liTytwd2REc3J2SFo3NDQrUzkxTVltOHRGSnlW?=
 =?utf-8?B?RENSU01vSEVDZDFJR3FXVitjQlRyUnc2Zmd5eVFPZEh4SHNDVGp3M1lVOVlK?=
 =?utf-8?B?UG05YTZFRUMyVG1ObkFma0tBMGJTWnJhQWVTRjhQeEF6TnQ3ZTkzUU84TDJB?=
 =?utf-8?B?WWZqNUxIa1ZmQnQ1ZDd6L01wY2dTSnQ4ZDBIWitXbzFCVlU3R3F5bGhMNDRK?=
 =?utf-8?B?MGJBekFiS0lWTFM4Z0NLTVV1N1FpOWYvQzJneEg4ZXFLaWNkREF5OEJ3ZEJE?=
 =?utf-8?B?di9aZHpjeVMrY2UzdWd0SFBySkxlajQ2cTBTTTlqdStMdGdDWG1yOEVXUFlK?=
 =?utf-8?B?akwrd3NRN240OWNnYkk2TFBlYTVzdGxJUjVDcDdyMk91V3JKRFJtb0x4WlAx?=
 =?utf-8?B?SVFEWGN2TXJtTjZFMVhtb201RzBaSC9iOU56cUJrVVVCY3RxdktwTVdkVUJj?=
 =?utf-8?B?L2IxZy9ybVp0SGo0K3RwQlpaMUR4dko4dzdkK0o1Mlc4TXpQajk2MXVndmZ2?=
 =?utf-8?B?SG1meWUxMmVIRGo0amZDZGFrdWhibjVJVUp1aEhRYzFaM3pjaWRJbnRKNjdR?=
 =?utf-8?B?bTR0amVKMndBNUZscjBoYUFSb3pHK211UjRZNW11RlVTQUdpZXpaYW9kQW5a?=
 =?utf-8?B?cERXZmNHcTVTQ2ZHSGMvYXU1Q1ZLRWxuN1dFVStRUjFBVzJQRDYrWFlrSEEz?=
 =?utf-8?B?MVlxaGo3WWdDOVFsU3dIS3FiME1xVTZwNmFsdnpyNUMzSzBrUTNUaGl2bkNX?=
 =?utf-8?B?elZhZnF5a09TR0RocUw1bFpsdDNLeFJXWUNOUldXL29qN096TTd4dVBFUllI?=
 =?utf-8?B?RVY5elFlS09lamFpV1hzTnkzSGtpeTlkT2tLSkdqaGZ0R2NHTkRWZU93eklO?=
 =?utf-8?B?OWJIZHZSRGoyc0hJUnZJV1FWOEFtTi9oaEZPMWVYamdXZzdtNkJYUHZzU09L?=
 =?utf-8?B?MWtyMlBvZ2o0T25tZGhrTXlLc1RGdlVqb0hJUnJLNDVHU2VxYy96NENuSXBk?=
 =?utf-8?B?NzdlOTlYKzBBY29RZXVmVUNjMWJtOUZ0YmhVWEo3Snp2UGhzWktKYlhobkxt?=
 =?utf-8?B?Z1lDcEVvaEJwb3lhWEQ1ZzhEOWVVTTJUUG9OVjVBQ0JKdGh6NmVmcXlvUVVk?=
 =?utf-8?B?dVpsek8wNGtNVmpYZkVWT0xSNVg5RFlJelRydEZZYlRUa3hkWlgvUFAyRm5J?=
 =?utf-8?B?anZLNWg4RWNTR0cxSENwMEdMRkl2Vktkckp0STEwVk9zeXFyU1FmRUNCd0Q1?=
 =?utf-8?B?eksxTFNhM3p0TzhqZysrMTB6TW9kZUZSWXljdHpVQzdnejZXUExpRVZDakdk?=
 =?utf-8?B?S2NtSmFIdzRUdVNVcm5Sc211U05DajFBa1YxVFNML2RsakYxUFAzL1RVbE82?=
 =?utf-8?B?cGZLempIeTZxc1RlY1NvWWVEK0ZxRER4b285eTk2UnBTdW54OVhSYUMrbEFw?=
 =?utf-8?B?cTNXQVBpakVlSEZuY0JzckI4MzhHeDJNRFQzdVA0ZEFwSHBIcnhWUWlsT0I3?=
 =?utf-8?B?cFhJK1o1T0E0VHB1QWZiS2EvRUR6VThWOE5Za00vQTJYMDhWRURMYXljR2Rn?=
 =?utf-8?B?bzZhQ3FIaGRMZEdrdEhNYTFiWFI1ZEhhRCtETWxKWVp5K3JCQzJMZmMwc2pP?=
 =?utf-8?B?YlRlRzNPM1ZzbnFqZ1VpblFzQldJTU5wZjIvb05WT1krZUFlRm4yMXJtcUwz?=
 =?utf-8?B?T3NzRlVhTy92djQ0blpqc1B6TURSeFc5U0FaU2x3Y1B6NnozVUs3MlpxTjRE?=
 =?utf-8?B?VEcvNmJtajNUeHBLZzFWYXRxNUhrdXVTQTRsQ0ZNS0VvNTVJQlFhcVdlZHBY?=
 =?utf-8?B?c1NlM1pSdklWQnltZXNoNzByMVB4alJwZjJaWXRQRE1HQkd0QktvRXVrNFhz?=
 =?utf-8?B?aHc9PQ==?=
X-OriginatorOrg: meta.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9d3e39c6-b5ce-4bf5-3103-08db4cbf61fc
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 May 2023 16:48:27.0773
 (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: nRCebHqp83qkwj31GekiQirQw+Ne7MqsGISAWqmf4GCrgTRWzWEEWfiUH7AzpceR
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR15MB5241
X-Proofpoint-ORIG-GUID: yX2BxgCNVJkDWTZnVRAhBxEZYf4JipHm
X-Proofpoint-GUID: yX2BxgCNVJkDWTZnVRAhBxEZYf4JipHm
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-05-04_10,2023-05-04_01,2023-02-09_01
X-Spam-Status: No, score=-7.1 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_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED 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 5/4/23 1:18 AM, Michal Suchánek wrote:
> Hello,
> 
> On Thu, May 04, 2023 at 12:43:52AM +0100, Quentin Monnet wrote:
>> On Fri, 21 Apr 2023 at 08:39, Michal Suchánek <msuchanek@suse.de> wrote:
>>>
>>> On Thu, Apr 20, 2023 at 04:07:38PM -0700, Andrii Nakryiko wrote:
>>>> On Fri, Apr 14, 2023 at 9:28 AM Michal Suchánek <msuchanek@suse.de> wrote:
>>>>>
>>>>> On Fri, Apr 14, 2023 at 05:18:27PM +0200, Alexander Lobakin wrote:
>>>>>> From: Michal Suchánek <msuchanek@suse.de>
>>>>>> Date: Fri, 14 Apr 2023 11:54:57 +0200
>>>>>>
>>>>>>> Hello,
>>>>>>
>>>>>> Hey-hey,
>>>>>>
>>>>>>>
>>>>>>> On Thu, Apr 21, 2022 at 12:38:58AM +0000, Alexander Lobakin wrote:
>>>>>>>> When building bpftool with !CONFIG_PERF_EVENTS:
>>>>>>>>
>>>>>>>> skeleton/pid_iter.bpf.c:47:14: error: incomplete definition of type 'struct bpf_perf_link'
>>>>>>>>          perf_link = container_of(link, struct bpf_perf_link, link);
>>>>>>>>                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>>>> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:74:22: note: expanded from macro 'container_of'
>>>>>>>>                  ((type *)(__mptr - offsetof(type, member)));    \
>>>>>>>>                                     ^~~~~~~~~~~~~~~~~~~~~~
>>>>>>>> tools/bpf/bpftool/bootstrap/libbpf/include/bpf/bpf_helpers.h:68:60: note: expanded from macro 'offsetof'
>>>>>>>>   #define offsetof(TYPE, MEMBER)  ((unsigned long)&((TYPE *)0)->MEMBER)
>>>>>>>>                                                    ~~~~~~~~~~~^
>>>>>>>> skeleton/pid_iter.bpf.c:44:9: note: forward declaration of 'struct bpf_perf_link'
>>>>>>>>          struct bpf_perf_link *perf_link;
>>>>>>>>                 ^
>>>>>>>>
>>>>>>>> &bpf_perf_link is being defined and used only under the ifdef.
>>>>>>>> Define struct bpf_perf_link___local with the `preserve_access_index`
>>>>>>>> attribute inside the pid_iter BPF prog to allow compiling on any
>>>>>>>> configs. CO-RE will substitute it with the real struct bpf_perf_link
>>>>>>>> accesses later on.
>>>>>>>> container_of() is not CO-REd, but it is a noop for
>>>>>>>> bpf_perf_link <-> bpf_link and the local copy is a full mirror of
>>>>>>>> the original structure.
>>>>>>>>
>>>>>>>> Fixes: cbdaf71f7e65 ("bpftool: Add bpf_cookie to link output")
>>>>>>>
>>>>>>> This does not solve the problem completely. Kernels that don't have
>>>>>>> CONFIG_PERF_EVENTS in the first place are also missing the enum value
>>>>>>> BPF_LINK_TYPE_PERF_EVENT which is used as the condition for handling the
>>>>>>> cookie.
>>>>>>
>>>>>> Sorry, I haven't been working with my home/private stuff for more than a
>>>>>> year already. I may get back to it some day when I'm tired of Lua (curse
>>>>>> words, sorry :D), but for now the series is "a bit" abandoned.
>>>>>
>>>>> This part still appllies and works for me with the caveat that
>>>>> BPF_LINK_TYPE_PERF_EVENT also needs to be defined.
>>>>>
>>>>>> I think there was alternative solution proposed there, which promised to
>>>>>> be more flexible. But IIRC it also doesn't touch the enum (was it added
>>>>>> recently? Because it was building just fine a year ago on config without
>>>>>> perf events).
>>>>>
>>>>> It was added in 5.15. Not sure there is a kernel.org LTS kernel usable
>>>>> for CO-RE that does not have it, technically 5.4 would work if it was
>>>>> built monolithic, it does not have module BTF, only kernel IIRC.
>>>>>
>>>>> Nonetheless, the approach to handling features completely missing in the
>>>>> running kernel should be figured out one way or another. I would be
>>>>> surprised if this was the last feature to be added that bpftool needs to
>>>>> know about.
>>>>
>>>> Are we talking about bpftool built from kernel sources or from Github?
>>>> Kernel source version should have access to latest UAPI headers and so
>>>> BPF_LINK_TYPE_PERF_EVENT should be available. Github version, if it
>>>> doesn't do that already, can use UAPI headers distributed (and used
>>>> for building) with libbpf through submodule.
>>>
>>> It does have a copy of the uapi headers but apparently does not use
>>> them. Using them directly might cause conflict with vmlinux.h, though.
>>
>> Indeed, using the UAPI header here conflicts with vmlinux.h.
>>
>> Looking again at some code I started last year but never finalised, I
>> used the following approach, redefining BPF_LINK_TYPE_PERF_EVENT with
>> CO-RE:
>>
>>      enum bpf_link_type___local {
>>          BPF_LINK_TYPE_PERF_EVENT___local = 7,
>>      };
> 
> That's the same as I did except I used simple define instead of this
> fake enum.
> 
> The enum only has value when it is complete and the compiler can check
> that a switch uses only known values, and can confuse things when values
> are missing.

Currently, enum value CORE is done though a llvm builtin function. So
if the enum value is used in switch cases like
   switch(...)
   case BPF_LINK_TYPE_PERF_EVENT:
      ...
CORE relocation will not work in that case since the compiler
expects BPF_LINK_TYPE_PERF_EVENT to be a constant.

> Thanks
> 
> Michal