From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DB305CED63A for ; Tue, 18 Nov 2025 12:26:28 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1vLKmg-0007lB-EH; Tue, 18 Nov 2025 07:26:10 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1vLKme-0007ic-EZ for qemu-devel@nongnu.org; Tue, 18 Nov 2025 07:26:08 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1vLKmc-0006K2-Lq for qemu-devel@nongnu.org; Tue, 18 Nov 2025 07:26:08 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1763468765; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GCzUP7yQPI+NTlXcj9N6dkCKdx+oLZ/ERm4s8BXh/1E=; b=R2NIzXeWHppRAi5lzaYd+DNPdMrurwR5w9denEGF8NQflvKh8/0IAIOR6aG0LfM9KTLHtV wkkXsoH/TyV0RLSOHL7t5I45imKH4oPqVilpmfm71tWHxKXKuOlmdcsigcZSdIq0Gr4hi4 qkM2kLtHVK0EDY4qvnDbLexgawKCz4g= Received: from mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-21-pB5aerZ8P2WVLoaHahmMVQ-1; Tue, 18 Nov 2025 07:26:04 -0500 X-MC-Unique: pB5aerZ8P2WVLoaHahmMVQ-1 X-Mimecast-MFC-AGG-ID: pB5aerZ8P2WVLoaHahmMVQ_1763468763 Received: from mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.111]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 84909180009D; Tue, 18 Nov 2025 12:26:03 +0000 (UTC) Received: from redhat.com (unknown [10.42.28.53]) by mx-prod-int-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 7A7BE180047F; Tue, 18 Nov 2025 12:26:01 +0000 (UTC) Date: Tue, 18 Nov 2025 12:25:55 +0000 From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= To: Markus Armbruster Cc: qemu-devel@nongnu.org, Jason Wang , Andrew Melnychenko , Yuri Benditovich Subject: Re: ebpf functions can fail without setting an error Message-ID: References: <87ectns27j.fsf@pond.sub.org> <87sehfsife.fsf@pond.sub.org> <871plwpxpu.fsf@pond.sub.org> <87pl9filnj.fsf@pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87pl9filnj.fsf@pond.sub.org> User-Agent: Mutt/2.2.14 (2025-02-20) X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.111 Received-SPF: pass client-ip=170.10.129.124; envelope-from=berrange@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Incidentally I'm seeing bounce messages for both the @daynix.com addresses CC here. The bounces are fatal reporting that the address does not exist. Guess they need removing from MAINTAINERS unless someone knows an alterantive address for either, and they wish to continue being reviewers. On Tue, Nov 18, 2025 at 01:13:20PM +0100, Markus Armbruster wrote: > Daniel P. Berrangé writes: > > > On Mon, Nov 17, 2025 at 02:58:37PM +0100, Markus Armbruster wrote: > >> Markus Armbruster writes: > >> > >> > Daniel P. Berrangé writes: > >> > > >> >> On Thu, Aug 07, 2025 at 03:14:56PM +0200, Markus Armbruster wrote: > >> >>> Three functions in ebpf_rss.h take an Error ** argument and return bool. > >> >>> Good. > >> >>> > >> >>> They can all fail without setting an error. Not good. > >> >>> > >> >>> The failures without error are: > >> >>> > >> >>> * All three stubs in ebpf_rss-stub.c always. Oversight? > >> >> > >> >> Opps, yes, we really should have added error_setg() calls for diagnosis > >> >> if someone tries to use eBPF when QEMU build has it disabled. > >> > >> Easy enough, but... > >> > >> > Some stubs exist only to mollify the linker. They are not meant to be > >> > called. They should abort(), optionally with lipstick. > >> > > >> > Other stubs are called and should fail nicely. > >> > > >> > Can you tell me offhand which kind these are? > >> > >> If calling these stubs is possible, I'd like to know how I can get them > >> called, so I can test the errors I add. > >> > >> If calling is not possible, I'd rather add abort()s. > >> > >> I tried to figure out whether calling is possible, but it ended in > >> confusion. Can you help? > > > > * ebpf_rss_set_all > > > > Is called from virtio_net_attach_ebpf_rss > > The call is unreachable if ebpf_rss_is_loaded returns false > > Stub for ebpf_rss_is_loaded always returns false > > > > => ebpf_rss_set_all stub is unreachable > > Then the non-stub ebpf_rss_set_all() has a useless check of > ebpf_rss_is_loaded() with an unreachable error message. > > > * ebpf_rss_load_fds, ebpf_rss_load > > > > Is called from virtio_net_load_ebpf_fds, which is called from > > virtio_net_load_ebpf > > > > The call to virtio_net_load_ebpf_fds is unreachable if > > virtio_net_attach_ebpf_to_backend fails > > > > virtio_net_attach_ebpf_to_backend fails if set_steering_ebpf > > fails > > > > set_steering_ebpf fails if ioctl(fd, TUNSETSTEERINGEBPF...) > > fails on Linux; all non-Linux impls of ebpf_rss_load_fds > > return -1 > > > > It is theoretically p9ossible to build QEMU without EBPF > > while both glibc & the kernel support TUNSETSTEERINGEBPF ioctl > > > > => ebpf_rss_load_fds, ebpf_rss_load are reachable in stubs > > So: > > * ebpf_rss_load() and ebpf_rss_load_fds() need a suitable error_setg(). > > * For ebpf_rss_set_all(), we have two sane options: > > - Declare ebpf_rss_is_loaded() a precondition, drop the useless check > from the non-stub version, abort() in the stub. > > - Keep the useless check and error in the non-stub version, add an > equally useless error to the stub. > > Got a preference? > > >> >>> * Non-stub ebpf_rss_load() when ebpf_rss_is_loaded(). Are these > >> >>> reachable? > >> >> > >> >> This scenario should never happen, and we should add a call like > >> >> > >> >> error_setg(errp, "eBPF program is already loaded"); > >> >> > >> >> to report it correctly. > >> > > >> > Is it a programming error when it happens? > >> > >> This question is still open as well. > > > > I'd consider it a programming error. I don't think we have a code > > path that could trigger it currently. > > Then the proper fix is replacing the flawed check by an assertion. > > Thanks! > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|