From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C26C245A9A; Thu, 3 Oct 2024 11:58:47 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D914F4060B; Thu, 3 Oct 2024 11:58:46 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id CDB3B400D7 for ; Thu, 3 Oct 2024 11:58:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1727949525; h=from:from: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=c8Y94JVryT0oY9tYRHnnqoQQ1Jw6xbX5Q9Z4nBdXohY=; b=PJcylQZR8fNM9YPS/nlRG4YfGmVmDxmV6UUeG2kwTRKVM1zEN1eovBqY0b7MWrTgqyhsu/ airj/yjp1x7+PRlqVHKUVwgJhb39MM3KBbPUe7etJxlFXP51cy+H0YkCNAAj+comODVStA GVccdU3ARwu7WIHC5SJnpKvlfFmjGHo= Received: from mail-lf1-f72.google.com (mail-lf1-f72.google.com [209.85.167.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-84-1xCGCgl0MxOpAVSAGjRMmg-1; Thu, 03 Oct 2024 05:58:42 -0400 X-MC-Unique: 1xCGCgl0MxOpAVSAGjRMmg-1 Received: by mail-lf1-f72.google.com with SMTP id 2adb3069b0e04-5398b9176dcso626204e87.2 for ; Thu, 03 Oct 2024 02:58:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727949521; x=1728554321; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=c8Y94JVryT0oY9tYRHnnqoQQ1Jw6xbX5Q9Z4nBdXohY=; b=N69p2nYL88OFcy5+7ooahHRR01xJRlnvhtn/y+Zs2fYsqBXNcAMx9v62kfbb9/qmN7 mZLmKBiEQYOVEcxxkzG0pIqZE63kYk2rnv6VagAtyE3i20CXGDHGehMxmSMLX6NSRrUm OgQ76lfAuOI4pAw+gpH4c8K1kCOOHPb5bnxuXDQFADzaqcN2uZ6NzQqjPTCkftcpIItV /I5kib2dud/r9X+YIyNNg3rrIho/8ElJZ58Y0dNuS1fqgIhIu+sL25ZUabmuPadBkoA1 bHuIt7mCsSSCoE2FLKEr1hr+BnK8w+Hn20Je4e5L9n0HQpB1YzwgdgtH+6ERI87QNZ+K Q8NA== X-Forwarded-Encrypted: i=1; AJvYcCUrLOVi8mkl8fEO2XNMzU8uto+kj+RrIxm6H0SFyhhJjQY/1B7E7Mn5zJE8mTbxEXTxP+g=@dpdk.org X-Gm-Message-State: AOJu0YyWeWO1eK2ocA1THxDa4hBdM0bi62yv/DdET9sofn8RPRhQgcmf HANkw9f5m1cJNQ/gu2vR1mx3NeRZtFerHrGXZYx28CWsLqFfh+edHVdATTNl2mb+WmggNhXIB8Q DcgNOCdd6W8fqHAiLKp0MlVoVEhsS6F/ucDdMhOp3YIrd9YaO/hwSYYDVzXytVzJVC6Q7QepWZ3 p9mT6zsweTAXEChZQ= X-Received: by 2002:a05:6512:224b:b0:536:54d6:e6d6 with SMTP id 2adb3069b0e04-539a0663876mr3426912e87.17.1727949521257; Thu, 03 Oct 2024 02:58:41 -0700 (PDT) X-Google-Smtp-Source: AGHT+IES3YbfFSQQ2awD03mS6Z+icDSt7Dadl/DPAktL5uU/MFmIvdpeA3XvOvItoyPh2zkjh77u+00+tlZv1MPC7+M= X-Received: by 2002:a05:6512:224b:b0:536:54d6:e6d6 with SMTP id 2adb3069b0e04-539a0663876mr3426893e87.17.1727949520873; Thu, 03 Oct 2024 02:58:40 -0700 (PDT) MIME-Version: 1.0 References: <20241002155709.2522273-1-david.marchand@redhat.com> <20241002155709.2522273-3-david.marchand@redhat.com> In-Reply-To: From: David Marchand Date: Thu, 3 Oct 2024 11:58:28 +0200 Message-ID: Subject: Re: [PATCH 2/2] ethdev: fix race on ports for telemetry commands To: Bruce Richardson Cc: Robin Jarry , dev@dpdk.org, ktraynor@redhat.com, stable@dpdk.org, Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Keith Wiles , Ciara Power X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thu, Oct 3, 2024 at 11:46=E2=80=AFAM Bruce Richardson wrote: > > On Wed, Oct 02, 2024 at 09:26:10PM +0200, Robin Jarry wrote: > > David Marchand, Oct 02, 2024 at 21:18: > > > On Wed, Oct 2, 2024 at 9:09=E2=80=AFPM Robin Jarry wrote: > > > > I was going to suggest adding a rte_spinlock_t* parameter to a new > > > > telemetry register function that would need to be held while the > > > > callback is invoked. Or if we want to keep doors open to other kind= s of > > > > lock, a wrapper callback. > > > > > > Well, as you had experimented this approach, we know this does not > > > work: the ethdev lock is in dpdk shared memory which is not available > > > yet at the time RTE_INIT() is called. > > > > > > A single callback is strange, I guess you mean pre/post callbacks the= n. > > > > It could be a single function that will wrap the callbacks. E.g.: > > > > static int > > eth_dev_telemetry_with_lock( > > telemetry_cb fn, const char *cmd, const char *params, struct rte_tel= _data *d) > > { > > int ret; > > rte_spinlock_lock(rte_mcfg_ethdev_get_lock()); > > ret =3D fn(cmd, params, d); > > rte_spinlock_unlock(rte_mcfg_ethdev_get_lock()); > > return ret; > > } > > > > RTE_INIT(ethdev_init_telemetry) > > { > > .... > > rte_telemetry_register_cmd("/ethdev/stats", eth_dev_handle_port_stat= s, > > "Returns the common stats for a port. Parameters: int port_i= d", > > eth_dev_telemetry_with_lock); > > .... > > } > > > > I'm not sure which solution is the uglier :D > > > > I don't actually mind this latter solution, except that the order of the > parameters should be reversed (and it breaks the ABI, unless we add a > special new function for it) For me, the wrapper function should be the > main callback, and the real (unwrapped) function the extra parameter to b= e > called. That extra parameter to callbacks should just be a generic pointe= r, > so it can be data or function that is passed around. > > rte_telemetry_register_param_cmd(const char *cmd, telemetry_cb fn= , > void *param, const char *help) > > Or more specifically: > > > rte_telemetry_register_param_cmd("/ethdev/stats", > eth_dev_telemetry_with_lock, /* callback */ > eth_dev_handle_port_stats, /* parameter */ > "Returns the common stats for a port. Parameters: int port_id= "); Ok, this way seems nicer. I'll let Robin submit a v2. --=20 David Marchand