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;
From:   Song Liu <songliubraving@fb.com>
To:     Steven Rostedt <rostedt@goodmis.org>
CC:     Song Liu <song@kernel.org>, Networking <netdev@vger.kernel.org>,
        bpf <bpf@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Kernel Team <Kernel-team@fb.com>,
        "jolsa@kernel.org" <jolsa@kernel.org>,
        "mhiramat@kernel.org" <mhiramat@kernel.org>
Subject: Re: [PATCH v2 bpf-next 5/5] bpf: trampoline: support
 FTRACE_OPS_FL_SHARE_IPMODIFY
Thread-Topic: [PATCH v2 bpf-next 5/5] bpf: trampoline: support
 FTRACE_OPS_FL_SHARE_IPMODIFY
Thread-Index: AQHYdrj21zA3ifq/PEC7Yw0zyJbSw61x8o6AgAAhSYCAAADUgIAACcQA
Date:   Wed, 6 Jul 2022 22:15:47 +0000
Message-ID: <ECD336F1-A130-47BA-8FBB-E3573445380F@fb.com>
References: <20220602193706.2607681-1-song@kernel.org>
 <20220602193706.2607681-6-song@kernel.org>
 <20220706153843.37584b5b@gandalf.local.home>
 <DC04E081-8320-4A39-A058-D0E33F202625@fb.com>
 <20220706174049.6c60250f@gandalf.local.home>
In-Reply-To: <20220706174049.6c60250f@gandalf.local.home>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?EjUl/Y+7Ynifbs4fNNIX09Ov5xMvep6T5x4x7MrlnWhp5JJq1fdxkG7kgnwo?=
 =?us-ascii?Q?u9SurUlVLa7HgOqPup3ZHpfzdo8qhUS2M+GABbPdSjugRJBRN6DZMm5pYBaX?=
 =?us-ascii?Q?OhE4twmt65HsaVarEZkgUjMDl8ZIOF5Z/N34ZBTcibvXW7iOL7+nEw5ubMU+?=
 =?us-ascii?Q?1ib/N9FkwjAYRcqIdeDWHG3CaenDonH3TDefvbrXBEXcSbnRSixdpRIy8kaw?=
 =?us-ascii?Q?32waNLdyS/LXWvqOS2JItccxTqBLGpg8Fk26bBGHX+s+baA1PkkI/mq9TBBb?=
 =?us-ascii?Q?eKFa1ZIv2l0tytLQ/x7LIJe5f03fZCfPg7HzgQ25zY55t7561R1LJh1erBEq?=
 =?us-ascii?Q?R3fdCh8V42tW8jKs003sg9Fc44erPqPV5UHvzep3hj9qM08W7N6Xo2yhGia3?=
 =?us-ascii?Q?wuY4CQitSJN8SWBULxQtHM81GXenolg9BayODLUt6/WACowfODKO4FeZTOu0?=
 =?us-ascii?Q?EZN080XAASqIohIWdbUtK1ZjiezsuWLp8++OySrowfhOeR9rtYIO2BKSSS1x?=
 =?us-ascii?Q?dKNsH9Dy/hvlLvfFXy8he7b5fjKcbDmf0X8DS7/qM31XpTZZRW42/pMyEVsF?=
 =?us-ascii?Q?kvNnx2eXKK7Km8kZhFb5r+QYZH9o3noWSvE9S/qSAodsrrqLCdTGAxwN9cz9?=
 =?us-ascii?Q?mNb36KJoevQfZI+3ZST+OswtvtO/KEuFIJJsD2geWwQoyp90PB2OWcTGVFR9?=
 =?us-ascii?Q?/0tIpfOcVdCOpv4I4jqOMZZRxEDzkCyxfqYz4ySxvUWDtXXmVCdF+px2ZppT?=
 =?us-ascii?Q?N5GzoFjo3qLSD1CKGCEBe01BQ/QZTLjo1nc7UGcFkTBG5f1M8VC7pOZ/O3/p?=
 =?us-ascii?Q?V0j1Rc0WgIOhDijmrEkW5Fi1cxm60TZtySnDL38ciKzZDH2rDI4+hmzAwhVf?=
 =?us-ascii?Q?bsfO2JZkyLrR3ZWSKCG00ud6FR2zto+fKTXiQMwW6HSLC6AIIJFQxkHrHgfR?=
 =?us-ascii?Q?Gr4BfYKWa+jPLyj8KuHwvW4tmTjDv4OWms2bmceqnJTi8DLm0NK5iQppyJDn?=
 =?us-ascii?Q?MQJ80bSWS9T1Is3xGh6a1x8euW/+H0nIyRXXgP/JuFv2KlsumH3zrBtuMZLX?=
 =?us-ascii?Q?ZckeRgPutbK54kmrneM3GLPYA0DB763B592WivPoNZC8nDlY8yf3XN2NPDox?=
 =?us-ascii?Q?uAY2Y0svSJ17UJ3GpCEEEFHYg7ki3Wy2mtK6DjEmAVRMaMbyup2bsBAK75XT?=
 =?us-ascii?Q?lPSRlUg1RaPoa+1294jrtlJdCYXMrQiQ/juip5xkbXdbUqIR4V7SJnTg5kGL?=
 =?us-ascii?Q?f5KBh/Qh8q5DveIAjVOBG3On6+qxIVJso8K65/onslGoWiK+e+cBc/RkCw+t?=
 =?us-ascii?Q?VAkzz/srd+/9m9ef/B+2i8f76A7j97NykuRtolgsnkHZOUFNt4Ld1NyLlozO?=
 =?us-ascii?Q?nSjTpe9SEG3NlD0bNP15k7q1m0++kVNm37+NMFMon/aoMf9tT52pWMJHfTgW?=
 =?us-ascii?Q?Dms7XC7NRnzXBJlwZfjSzILq2Dc5+c20Iy8uKjWafTa04WOi+t1pZJwJzVLM?=
 =?us-ascii?Q?n9MvCFQRBpHwtUcZJ40SBuu8fhFWyrC+fzm1ay1jRtqOBx10PxtxJSOGcLfi?=
 =?us-ascii?Q?LUBNh2IBBYklEOwReErBr5ood6ZKM6ZPUMdPfrCZ1SoyaRUqFSBhSabu1de5?=
 =?us-ascii?Q?QE1QtkrW7XDQxuiiv/KFdgg=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1137F313C5ACA6449731E12E63700FD9@namprd15.prod.outlook.com>
MIME-Version: 1.0
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR15MB5109.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 22d7418e-111f-4571-9d48-08da5f9d13ad
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jul 2022 22:15:47.0526
 (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: ynCvYGVqBvOj6r5BczO0/GU2vT1gI+IKVeSXnjo5hHghP+CR6mFCVzLT6hzCTnQdI04te/+PIrHxvE5T02N3qA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR15MB3621
X-Proofpoint-GUID: tbwnVVbSelYWqiorSYooPnGYS6UskxAd
X-Proofpoint-ORIG-GUID: tbwnVVbSelYWqiorSYooPnGYS6UskxAd
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-06_12,2022-06-28_01,2022-06-22_01
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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 Jul 6, 2022, at 2:40 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> On Wed, 6 Jul 2022 21:37:52 +0000
> Song Liu <songliubraving@fb.com> wrote:
> 
>>> Can you comment here that returning -EAGAIN will not cause this to repeat.
>>> That it will change things where the next try will not return -EGAIN?  
>> 
>> Hmm.. this is not the guarantee here. This conflict is a real race condition 
>> that an IPMODIFY function (i.e. livepatch) is being registered at the same time 
>> when something else, for example bpftrace, is updating the BPF trampoline. 
>> 
>> This EAGAIN will propagate to the user of the IPMODIFY function (i.e. livepatch),
>> and we need to retry there. In the case of livepatch, the retry is initiated 
>> from user space. 
> 
> We need to be careful here then. If there's a userspace application that
> runs at real-time and does a:
> 
> 	do {
> 		errno = 0;
> 		regsiter_bpf();
> 	} while (errno != -EAGAIN);

Actually, do you mean:

	do {
		errno = 0;
		regsiter_bpf();
	} while (errno == -EAGAIN);

(== -EAGAIN) here?

In this specific race condition, register_bpf() will succeed, as it already
got tr->mutex. But the IPMODIFY (livepatch) side will fail and retry. 

Since both livepatch and bpf trampoline changes are rare operations, I think 
the chance of the race condition is low enough. 

Does this make sense?

Thanks,
Song