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: <4ca27e23-b027-0e39-495b-2ba3376342cc@meta.com>
Date:   Thu, 8 Jun 2023 14:07:05 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.11.2
Subject: Re: [PATCH bpf-next v3 1/3] bpf, x86: allow function arguments up to
 12 for TRACING
Content-Language: en-US
To:     menglong8.dong@gmail.com, alexei.starovoitov@gmail.com
Cc:     davem@davemloft.net, dsahern@kernel.org, ast@kernel.org,
        daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev,
        song@kernel.org, yhs@fb.com, john.fastabend@gmail.com,
        kpsingh@kernel.org, sdf@google.com, x86@kernel.org,
        imagedong@tencent.com, benbjiang@tencent.com,
        netdev@vger.kernel.org, bpf@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
References: <20230607125911.145345-1-imagedong@tencent.com>
 <20230607125911.145345-2-imagedong@tencent.com>
From:   Yonghong Song <yhs@meta.com>
In-Reply-To: <20230607125911.145345-2-imagedong@tencent.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?UG12aUVrWGNFL3FCYllTRGVqSVhiZytIRUJxRE95NE9IRzlGd1NQM3JDUXB6?=
 =?utf-8?B?ekJzWUY5WnR1NXhBUThSUzAyWXFqVS9iQjRZVWlGZnQ1MThqOHVHaWlDOWZ6?=
 =?utf-8?B?d2FlNzYrLzV5L21ySkVJQkFGNGJuNnNzNzFMc3hBbm1YR1JoRFdoNFZmQjVV?=
 =?utf-8?B?RlVkMU0wMGFqZEJHejZpbkp5RExkQUtyZlRzMndjLzNXcDk4emcrOHphMlhD?=
 =?utf-8?B?VC9VUVBiVUhoU1ZKN2RPek9UeHlmSGZNSVFNeXMyT1ZhZ0ZPS09UWm5QRUFF?=
 =?utf-8?B?MWxVNVROclFtbnZuaERBclFjbG8yN2hjZkVBOC80Zjdmd0pXWE1GSnVIQUV5?=
 =?utf-8?B?R2NDMUxIZXEwS2RCNnVSdDVLbnh3ZFZxVTQwYUZHalZyOWxCT0tGNFozL0hv?=
 =?utf-8?B?SWdNeUN3aTE0dm45RisweVJ0NWlYaUl1S0N2YkdNbTVLTW1ISlVpUGFjTVky?=
 =?utf-8?B?bWhIWVZNWTJuT3B3eTd3anU5QU9WV1cxMWd5bC96dGZMY3RjSTcyWXRoaVJH?=
 =?utf-8?B?VTY4VzVRaldLQ2QyZkNKenRlMmtIV1RvVzdTUTU2R3pOZVFXejBuRGUvWFpC?=
 =?utf-8?B?cWhxWWhMNXEzckw4czRwZGdzb0xuQ0p2bGp2a2R0a1dwWVRid29HMUpheEdE?=
 =?utf-8?B?YjB5eHNCcXA2ZDNSaTRTZlNSbXZkVVM5VHFKMHFEaG9LOUJ1UVdraXhKRDVp?=
 =?utf-8?B?ZHhGS0xmcFp0bktXd1hZLzVadDFDZUY3Uzl2T1d5cHRGc3MzQ3RTVTMzeGhF?=
 =?utf-8?B?dGM2bG96U3BJazRPdUNHZ0cwMFoyZndGK0xscE1CYlhVMHN5OXZMK0hPbFpq?=
 =?utf-8?B?ZDdBN0tzWjdwbUF3QlB2SGlRWS8zaWZIYkJlaVk5Z1VxK2orcU8rUzNiTEUw?=
 =?utf-8?B?ZnBOUkhrbFlQQ25jY2hWNUxqTzduNVdnQmNGOG02MjNUenpiaGNTV29OUkQ0?=
 =?utf-8?B?UkZTdjRNYVE2L2JZTXp1dUIybnM5Y2d2eU8yZ0pucFo3dVN6dWV1RWU0MFlR?=
 =?utf-8?B?M2Q0QUtOWHBlaDM2UUs5YWlySmtjZ0JnMzNlVXdxSE84c09xekFSSFY0S3cr?=
 =?utf-8?B?eFZMUkk5Rjc5SmJ4U1gwYjJOcHJHMUdoRm1HYmhPbEFMajViQ1VTaENNKzRF?=
 =?utf-8?B?WndzWUdkYi9iT1NtaTFLRzhwdm51ZHQrZEFaZnlReUh0eXJnOGYwNGQ4LzUx?=
 =?utf-8?B?UW5mY2xKN1hQby9TbTl2Z092bjhpbHZXaVVUYzMzRDFEbVdkaXQ4VTM1RWQ4?=
 =?utf-8?B?bmhObEk5d2hWODVLQUVkWWRDVlA5YWNIR1krby9mMWJEWWMxZ2E5YnYxeXNJ?=
 =?utf-8?B?MVZaYmFsekI2RUtjRFVnSVp0ckJWU1VxVGJoVFU5VGxWU0RPblA4NDUySG8v?=
 =?utf-8?B?U2padUpNY1lqdXN6NW5JUWtYREJ6a24rUUp1MGcvUGZtM0hIQVpLamFKMm4z?=
 =?utf-8?B?YUNGclp0a1F2dWJYQVBOWGJKS1ZDQXVQRCtrVmMrbnc1WGg0MFJrcVdBRGtz?=
 =?utf-8?B?RWtIT0hRRFZpanFheEJlczQwaEZPcVVOY2VzRzUvZjNGMHZoZDNIWk5jQkp2?=
 =?utf-8?B?KzZWK3kvNHZrQ2o1bldSUGVLT3krU09ZUk9WY2ZCU01ud0dKK3VPUGEvWHJ0?=
 =?utf-8?B?QnBHeDNFSzZZejhHSzg3QWp3RHRKYkdZbUtLaUNvU253dVArSnpkUHpMMVMv?=
 =?utf-8?B?MVVFaGh4bUpzUFJWMEZIWDJScmRBUXJHZ1A3bWxRMVdlVUhBU3FtYmtZd2to?=
 =?utf-8?B?RVN0SldFa2hVMmMzRGZlNnpOc3NrWmhUUTJxN3lVb2JvRWNlMnpiTFJnVU1G?=
 =?utf-8?B?dlluNGM1dElBS3RxSEI2ZVBMMU1MQjJkVWJkWVVWeHo3OVdDWXZqUGxmeTV3?=
 =?utf-8?B?azVwcWxWVk1FSlIyYXR5bTRhNDljSXEzckhEb1JQQzdYanFlNjNVem5HcnJW?=
 =?utf-8?B?ZjFYbHRsUVhHbkZ4VHFvWjl3U3VuaHVUR2pFU0xYK3JsWFRGRHM3bW84UUM2?=
 =?utf-8?B?MG9sZ1IrQWx2SU9kSDVJZVg0NVI4UE9vb3IvYVpMdFVHbFNQb2FwT0pjMURW?=
 =?utf-8?B?Q00xUURJZGh6YjFhU2RNb3FydThFS3FBQ2ZjbHljbFpBejZiNE9OOVdnK1dY?=
 =?utf-8?B?T0hRV21yL3VIQW9YaVA4ZEJqdWhFSy83YjFWT0pCeWt3K2QzRVQ2SWdHc2ty?=
 =?utf-8?B?MFE9PQ==?=
X-OriginatorOrg: meta.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d781b75c-4733-41f5-6d3b-08db6864520a
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jun 2023 21:07:08.9294
 (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: qSWejcwrRj5TwA/CTWfDHqTWtaVG4UtUrNfnze6Y307H5qyX45TW/kPN2oc6P7QA
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR15MB5647
X-Proofpoint-GUID: 2JpqSjwo74rTcOOZSe2HqLP4KWF56roJ
X-Proofpoint-ORIG-GUID: 2JpqSjwo74rTcOOZSe2HqLP4KWF56roJ
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26
 definitions=2023-06-08_16,2023-06-08_01,2023-05-22_02
X-Spam-Status: No, score=-2.9 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 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 6/7/23 5:59 AM, menglong8.dong@gmail.com wrote:
> From: Menglong Dong <imagedong@tencent.com>
> 
> For now, the BPF program of type BPF_PROG_TYPE_TRACING can only be used
> on the kernel functions whose arguments count less than 6. This is not
> friendly at all, as too many functions have arguments count more than 6.

Since you already have some statistics, maybe listed in the commit message.

> 
> Therefore, let's enhance it by increasing the function arguments count
> allowed in arch_prepare_bpf_trampoline(), for now, only x86_64.
> 
> For the case that we don't need to call origin function, which means
> without BPF_TRAMP_F_CALL_ORIG, we need only copy the function arguments
> that stored in the frame of the caller to current frame. The arguments
> of arg6-argN are stored in "$rbp + 0x18", we need copy them to
> "$rbp - regs_off + (6 * 8)".

Maybe I missed something, could you explain why it is '$rbp + 0x18'?

In the current upstream code, we have

         /* Generated trampoline stack layout:
          *
          * RBP + 8         [ return address  ]
          * RBP + 0         [ RBP             ]
          *
          * RBP - 8         [ return value    ]  BPF_TRAMP_F_CALL_ORIG or
          * 
BPF_TRAMP_F_RET_FENTRY_RET flags
          *
          *                 [ reg_argN        ]  always
          *                 [ ...             ]
          * RBP - regs_off  [ reg_arg1        ]  program's ctx pointer
          *
          * RBP - nregs_off [ regs count      ]  always
          *
          * RBP - ip_off    [ traced function ]  BPF_TRAMP_F_IP_ARG flag
          *
          * RBP - run_ctx_off [ bpf_tramp_run_ctx ]
          */

Next on-stack argument will be RBP + 16, right?

> 
> For the case with BPF_TRAMP_F_CALL_ORIG, we need prepare the arguments
> in stack before call origin function, which means we need alloc extra
> "8 * (arg_count - 6)" memory in the top of the stack. Note, there should
> not be any data be pushed to the stack before call the origin function.
> Then, we have to store rbx with 'mov' instead of 'push'.
> 
> We use EMIT3_off32() or EMIT4() for "lea" and "sub". The range of the
> imm in "lea" and "sub" is [-128, 127] if EMIT4() is used. Therefore,
> we use EMIT3_off32() instead if the imm out of the range.
> 
> It works well for the FENTRY and FEXIT, I'm not sure if there are other
> complicated cases.

MODIFY_RETURN is also impacted by this patch.

> 
> Reviewed-by: Jiang Biao <benbjiang@tencent.com>
> Signed-off-by: Menglong Dong <imagedong@tencent.com>
[...]