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 E6AB545B3C; Mon, 14 Oct 2024 22:01:28 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BA633402A9; Mon, 14 Oct 2024 22:01:28 +0200 (CEST) Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) by mails.dpdk.org (Postfix) with ESMTP id 9815C4026C for ; Mon, 14 Oct 2024 22:01:27 +0200 (CEST) Received: by mail-pf1-f172.google.com with SMTP id d2e1a72fcca58-71e5ae69880so1395205b3a.2 for ; Mon, 14 Oct 2024 13:01:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1728936087; x=1729540887; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=RyWanBT1W/UlzrOBDapMRdafUyfWfU8WLaVBz7GyQq4=; b=kMkssUBzRMOFk2SpG2euwhhPIzgMH2inoum0+Q2i5K6ZifK9SxmdipNEbfRciMMehP 36FB0ShsImvBwSSnqIqTzqu/OCXvUth3O15wOovcKH4IxUD5oTC8fIZ97TG9QArFAVSG g3m7fA36OhKmrAQG7N9nmJkPY/5SNr0irrXBaa0Pz132j7DGO2LDuRMhFQR/aAv3ACBm io6Qy8TtfYxuUEgnq0CVS9+fFY52fDUNau8pSwambf0D5X12A+hvjcs77PHxFRXdk2P6 wOU3M9EOxe/V7xYAd5gJ2OO2nHDnmHuFS7wAPPpehbNgpNEm0US9VtAbTKAR2vjNEyHM KZFQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728936087; x=1729540887; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=RyWanBT1W/UlzrOBDapMRdafUyfWfU8WLaVBz7GyQq4=; b=IcuJUFzAkIi/Nxpw0i+2qOC65Sebqh78a/vzsFrAgitq38X75LxYVJIfwAk79LQD3G WXsqtHzF4H9b7ogYbkBaHn/FSiG3482HnhfixSPDjTKPBOUhDQdaOXMncf6YjG8GRJl1 SEvi9IP5P72YcDVI3GB6vFKCAirl6mIhOgqpokW5ozLlbeqRvIqeID9c9ocYmje1I56+ Ej1I+OSd+XjgZIESQPwS7Trm/K0cjSM1/WBsd63fEU9ZFC3iYomgSXmFdibmX/xmYHtY JN9uPCpgwcz3am9Tn64sAG6hk/FHmepdAOmeHdqJWKXhaEr1xOxCeJZ3Rqd1IVIACvj8 k+Cw== X-Gm-Message-State: AOJu0YxnDxqipK7Id9JGb3VIJ7ny0Hwez0G7z3TbyYNezHn8KTGMDsUA cAw4DFNraEZsFKL2Mt9T2HwgGOGU9niQRWq5IMNNJspEqDmyrJwEWYq3AHj9znA= X-Google-Smtp-Source: AGHT+IG3W8BjYguUABA4imOxLZ1Z114YuONqNxiRMhNHgJ35lGfArW0+fTOaByfryUjZh6NwRnBD4Q== X-Received: by 2002:a05:6a21:e8d:b0:1cc:e409:7d0c with SMTP id adf61e73a8af0-1d8bcf3bcd6mr17992144637.25.1728936086644; Mon, 14 Oct 2024 13:01:26 -0700 (PDT) Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71e2aab9b29sm8168849b3a.147.2024.10.14.13.01.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Oct 2024 13:01:26 -0700 (PDT) Date: Mon, 14 Oct 2024 13:01:24 -0700 From: Stephen Hemminger To: Robin Jarry Cc: dev@dpdk.org, Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Ciara Power , Bruce Richardson , Keith Wiles , stable@dpdk.org Subject: Re: [PATCH dpdk v3 2/2] ethdev: fix potential race in telemetry endpoints Message-ID: <20241014130124.32c7b4bf@hermes.local> In-Reply-To: <20241014193237.1992382-3-rjarry@redhat.com> References: <20241002155709.2522273-1-david.marchand@redhat.com> <20241014193237.1992382-1-rjarry@redhat.com> <20241014193237.1992382-3-rjarry@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 Mon, 14 Oct 2024 21:32:37 +0200 Robin Jarry wrote: > While invoking telemetry commands (which may happen at any time, out of > control of the application), an application thread may concurrently > add/remove ports. The telemetry callbacks may then access partially > initialized/uninitialised ethdev data. > > Reuse the ethdev lock that protects port allocation/destruction and the > new telemetry callback register api that takes an additional private > argument. Pass eth_dev_telemetry_do as the main callback and the actual > endpoint callbacks as private argument. > > Fixes: c190daedb9b1 ("ethdev: add telemetry callbacks") > Cc: stable@dpdk.org > > Signed-off-by: Robin Jarry > Acked-by: Bruce Richardson > --- > lib/ethdev/rte_ethdev_telemetry.c | 66 ++++++++++++++++++++++--------- > 1 file changed, 47 insertions(+), 19 deletions(-) > > diff --git a/lib/ethdev/rte_ethdev_telemetry.c b/lib/ethdev/rte_ethdev_telemetry.c > index 6b873e7abe68..7599fa2852b6 100644 > --- a/lib/ethdev/rte_ethdev_telemetry.c > +++ b/lib/ethdev/rte_ethdev_telemetry.c > @@ -1395,45 +1395,73 @@ eth_dev_handle_port_tm_node_caps(const char *cmd __rte_unused, > return ret; > } > > +static int eth_dev_telemetry_do(const char *cmd, const char *params, > + void *arg, struct rte_tel_data *d) > +{ > + int ret; > + telemetry_cb fn = arg; > + rte_spinlock_lock(rte_mcfg_ethdev_get_lock()); > + ret = fn(cmd, params, d); > + rte_spinlock_unlock(rte_mcfg_ethdev_get_lock()); > + return ret; > +} If this happens often, and the function takes a long time (like doing i/o) it might be worth changing this to reader/writer in future. Also, would be best to add a comment here as to what is being protected if you do another version. Acked-by: Stephen Hemminger