From: Bruce Richardson <bruce.richardson@intel.com>
To: fengchengwen <fengchengwen@huawei.com>
Cc: Ferruh Yigit <ferruh.yigit@amd.com>,
Dongdong Liu <liudongdong3@huawei.com>, <thomas@monjalon.net>,
<andrew.rybchenko@oktetlabs.ru>, <dev@dpdk.org>,
<ciara.power@intel.com>, <kevin.laatz@intel.com>
Subject: Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
Date: Thu, 16 Feb 2023 12:55:59 +0000 [thread overview]
Message-ID: <Y+4n33Je6VEmle03@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <Y+4nfFlocIkFVo8R@bricha3-MOBL.ger.corp.intel.com>
On Thu, Feb 16, 2023 at 12:54:20PM +0000, Bruce Richardson wrote:
> On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote:
> > On 2023/2/16 20:06, Ferruh Yigit wrote:
> > > On 2/16/2023 11:53 AM, fengchengwen wrote:
> > >> On 2023/2/15 11:19, Dongdong Liu wrote:
> > >>> Hi Chengwen
> > >>>
> > >>> On 2023/2/9 10:32, Chengwen Feng wrote:
> > >>>> The xstats reset is useful for debugging, so add it to the ethdev
> > >>>> telemetry command lists.
> > >>>>
> > >>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > >>> This patch looks good, so
> > >>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
> > >>>
> > >>> A minior question
> > >>> Do we need to support stats reset ?
> > >>
> > >> Stats is contained by xstats, and future direction I think is xstats.
> > >> So I think we don't need support stats reset.
> > >>
> > >
> > > I have similar question with Dongdong, readonly values are safe for
> > > telemetry, but modifying data can be more tricky since we don't have
> > > locking in ethdev APIs, this can cause concurrency issues.
> >
> > Yes, it indeed has concurrency issues.
> >
> > >
> > > Overall do we want telemetry go that way and become something that
> > > alters ethdev data/config?
> >
> > There are at least two part of data: config and status.
> > For stats (which belong status data) could help for debugging, I think it's acceptable.
> >
> > As for concurrency issues. People should know what to do and when to do, just like
> > the don't invoke config API (e.g. dev_configure/dev_start/...) concurrency.
> >
> While this is probably ok for now, I think in next release we should look
> to add some sort of support for locking for destructive ops in a future
> release. For example, we could:
>
> 1. Add support for marking a callback as "destructive" and only allow it to
> be called if only one connection is present or
>
> 2. Make it possible for callbacks to query the number of connections so
> that the callback itself is non-destructive in more than one connection is
> open.
>
> [Both of these will require locking support so that new connections aren't
> openned when the callback is in-flight!]
>
> Any other thoughts or suggestions?
>
Actually, another thought - we could also make the max-connections
configurable at runtime, so that the user can enforce only a single
connection.
/Bruce
next prev parent reply other threads:[~2023-02-16 12:56 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-19 9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
2022-12-19 9:07 ` [PATCH v5 1/5] dmadev: support stats reset telemetry command Chengwen Feng
2022-12-19 9:07 ` [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict Chengwen Feng
2022-12-19 9:33 ` Bruce Richardson
2022-12-26 4:53 ` fengchengwen
2023-01-06 16:07 ` Bruce Richardson
2023-01-06 17:33 ` Bruce Richardson
2023-01-11 12:38 ` fengchengwen
2022-12-19 9:07 ` [PATCH v5 3/5] ethdev: support xstats reset telemetry command Chengwen Feng
2022-12-19 9:07 ` [PATCH v5 4/5] ethdev: telemetry xstats support hide zero Chengwen Feng
2022-12-19 9:07 ` [PATCH v5 5/5] ethdev: add newline to telemetry log string Chengwen Feng
2022-12-26 4:55 ` [PATCH v5 0/5] support dmadev/ethdev stats reset fengchengwen
2023-01-11 12:02 ` [PATCH v2 " Chengwen Feng
2023-01-11 12:02 ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
2023-01-11 12:02 ` [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict Chengwen Feng
2023-01-11 12:02 ` [PATCH v2 3/5] ethdev: add newline to telemetry log string Chengwen Feng
2023-01-11 12:02 ` [PATCH v2 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
2023-01-11 12:02 ` [PATCH v2 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
2023-01-11 12:06 ` [PATCH v2 0/5] support dmadev/ethdev stats reset Chengwen Feng
2023-01-11 12:06 ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
2023-01-11 13:58 ` Bruce Richardson
2023-01-11 12:06 ` [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict Chengwen Feng
2023-01-11 14:01 ` Bruce Richardson
2023-01-11 12:06 ` [PATCH v2 3/5] ethdev: add newline to telemetry log string Chengwen Feng
2023-01-11 12:06 ` [PATCH v2 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
2023-01-11 12:06 ` [PATCH v2 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
2023-01-11 14:08 ` Bruce Richardson
2023-01-20 3:51 ` fengchengwen
2023-01-11 12:36 ` [PATCH v2 0/5] support dmadev/ethdev stats reset fengchengwen
2023-01-20 3:25 ` [PATCH v3 " Chengwen Feng
2023-01-20 3:25 ` [PATCH v3 1/5] dmadev: support stats reset telemetry command Chengwen Feng
2023-01-20 3:25 ` [PATCH v3 2/5] telemetry: fix repeat display when callback don't init dict Chengwen Feng
2023-01-20 3:25 ` [PATCH v3 3/5] ethdev: add newline to telemetry log string Chengwen Feng
2023-01-20 3:25 ` [PATCH v3 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
2023-01-20 3:25 ` [PATCH v3 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
2023-01-20 3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
2023-01-20 3:34 ` [PATCH v4 1/5] dmadev: support stats reset telemetry command Chengwen Feng
2023-01-20 3:34 ` [PATCH v4 2/5] telemetry: fix repeat display when callback don't init dict Chengwen Feng
2023-02-08 14:15 ` Bruce Richardson
2023-02-09 1:33 ` fengchengwen
2023-01-20 3:34 ` [PATCH v4 3/5] ethdev: add newline to telemetry log string Chengwen Feng
2023-01-20 3:34 ` [PATCH v4 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
2023-01-20 3:34 ` [PATCH v4 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
2023-02-06 8:39 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Thomas Monjalon
2023-02-08 14:17 ` Bruce Richardson
2023-02-09 1:45 ` fengchengwen
2023-02-09 2:32 ` [PATCH v5 0/2] " Chengwen Feng
2023-02-09 2:32 ` [PATCH v5 1/2] dmadev: support stats reset telemetry command Chengwen Feng
2023-02-09 2:32 ` [PATCH v5 2/2] ethdev: support xstats " Chengwen Feng
2023-02-15 3:19 ` Dongdong Liu
2023-02-16 11:53 ` fengchengwen
2023-02-16 12:06 ` Ferruh Yigit
2023-02-16 12:42 ` fengchengwen
2023-02-16 12:54 ` Bruce Richardson
2023-02-16 12:55 ` Bruce Richardson [this message]
2023-02-17 9:44 ` fengchengwen
2023-02-20 13:05 ` Thomas Monjalon
2023-07-03 3:58 ` fengchengwen
2023-07-03 7:20 ` Thomas Monjalon
2023-07-03 13:44 ` Morten Brørup
2023-07-04 6:41 ` fengchengwen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y+4n33Je6VEmle03@bricha3-MOBL.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=ciara.power@intel.com \
--cc=dev@dpdk.org \
--cc=fengchengwen@huawei.com \
--cc=ferruh.yigit@amd.com \
--cc=kevin.laatz@intel.com \
--cc=liudongdong3@huawei.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).