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 851C145B42 for ; Tue, 15 Oct 2024 10:02:18 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 79D1B400EF; Tue, 15 Oct 2024 10:02:18 +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 B07E2400EF for ; Tue, 15 Oct 2024 10:02:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1728979336; 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=qwlei1n52bb13P8Oom4Y9it1TzdKNXfnggc8mORVUaY=; b=OqhX/YjvjJ8s8TuPLoSVQakSOStiuDiSLdY3I6EpVr5t7Hb1KGpnVQ+ADxatP5VI98u8nb ZaQQSXcGJYkhQVCyvbE44SYpaqlErdK3kilTjCImcQfvQkAzsjWNTmcvMoNVdQoT1cYSPB lveaEe2f+tsw+PW021I9AFzgL8AUATE= Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-42-0lytbJk5OmaSJpKYQYXkTw-1; Tue, 15 Oct 2024 04:02:15 -0400 X-MC-Unique: 0lytbJk5OmaSJpKYQYXkTw-1 Received: by mail-lf1-f70.google.com with SMTP id 2adb3069b0e04-539ea0fcd4bso2059746e87.2 for ; Tue, 15 Oct 2024 01:02:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728979333; x=1729584133; 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=qwlei1n52bb13P8Oom4Y9it1TzdKNXfnggc8mORVUaY=; b=C0PRmdHALX8onVlBNlSOm08Vc9Mu2rIGYLc+MN9hijB5o6CqXexqj2rgcw2c4Z7rdh U7PGWXf40t20zDMcZxe/5c78QAycjGxm3XJBV6qx+ZiTJIu2T541kHWCO7gluUGKPL4m KayRKXH7bXsXhDa+K/X2lDhzzt1gzIofKVDvU3XFZ1uFZeyoAmBumqwjfyLtkjHpJUxf 5aStt6nozL3QnPR6l2CivLU7Osuc7o+lohxC0aHc/KhQ4hIYFnh9L8g4YywcZs0fgALT Uc/ZM9M0w4UcmkGszQCV7u7l7rrVX59PTOu8GqyGpauQbDGGa1++dtDd0+dvejAeq1ey ZCcw== X-Forwarded-Encrypted: i=1; AJvYcCVKQE93A9GvlGwnrEndsLqkMUlMVFlCGwR8NMbaivRF/7+BjXvyMnJLN4GKQR0ZpOgOBIdG2o4=@dpdk.org X-Gm-Message-State: AOJu0Yz6reVu2lVh3wDq+v6ye83zrg1xlLnJA0VdV/fWPxBRCotrzOl9 kso5ujSUZcdMPux8t9UPfgwPsDqKpg62SRfLo0COC/D1rmtgXVfalc+mLlWCxI6VMcDjckei6Lv 8nYeK7oyJxiLC+ZA/4L5E6wRvUxZ+pPRIBt18QgaleIvk2PXJj3mJ3FxRDy5u7cn1J1rnppanRr mF7i/A1zKsSlMo2zAhajk= X-Received: by 2002:a05:6512:33cb:b0:539:f65b:3f9 with SMTP id 2adb3069b0e04-539f65b15bdmr3161995e87.10.1728979333436; Tue, 15 Oct 2024 01:02:13 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGrcRt7QsC8vo05cLs4KaaBuRHKgr8GtxlfDEUTejvmEdM28MFOtv1pTtZJR3l9qWtxyiwEfwEHgrtVVRmTFOk= X-Received: by 2002:a05:6512:33cb:b0:539:f65b:3f9 with SMTP id 2adb3069b0e04-539f65b15bdmr3161969e87.10.1728979332966; Tue, 15 Oct 2024 01:02:12 -0700 (PDT) MIME-Version: 1.0 References: <20241002155709.2522273-1-david.marchand@redhat.com> <20241014193237.1992382-1-rjarry@redhat.com> <20241014193237.1992382-3-rjarry@redhat.com> <20241014130124.32c7b4bf@hermes.local> In-Reply-To: <20241014130124.32c7b4bf@hermes.local> From: David Marchand Date: Tue, 15 Oct 2024 10:02:01 +0200 Message-ID: Subject: Re: [PATCH dpdk v3 2/2] ethdev: fix potential race in telemetry endpoints To: Stephen Hemminger , Robin Jarry Cc: dev@dpdk.org, Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Ciara Power , Bruce Richardson , Keith Wiles , stable@dpdk.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On Mon, Oct 14, 2024 at 10:01=E2=80=AFPM Stephen Hemminger wrote: > > 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 *cm= d __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 =3D arg; > > + rte_spinlock_lock(rte_mcfg_ethdev_get_lock()); > > + ret =3D 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. Yes, this was an option mentionned when we discussed the issue in Montr=C3= =A9al. For now, a spinlock seems enough. > > Also, would be best to add a comment here as to what is being protected > if you do another version. I can add something when applying, like: @@ -1400,6 +1400,7 @@ static int eth_dev_telemetry_do(const char *cmd, const char *params, { int ret; telemetry_cb fn =3D arg; + /* Protect against port removal while invoking callback, calling ethdev API. */ rte_spinlock_lock(rte_mcfg_ethdev_get_lock()); ret =3D fn(cmd, params, d); rte_spinlock_unlock(rte_mcfg_ethdev_get_lock()); --=20 David Marchand