From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id A5A94A0C45;
Wed, 27 Oct 2021 10:16:05 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 7925B4068C;
Wed, 27 Oct 2021 10:16:05 +0200 (CEST)
Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com
[209.85.221.44]) by mails.dpdk.org (Postfix) with ESMTP id 10A354003F
for <dev@dpdk.org>; Wed, 27 Oct 2021 10:16:04 +0200 (CEST)
Received: by mail-wr1-f44.google.com with SMTP id e12so2646237wra.4
for <dev@dpdk.org>; Wed, 27 Oct 2021 01:16:04 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google;
h=date:from:to:cc:subject:message-id:references:mime-version
:content-disposition:content-transfer-encoding:in-reply-to;
bh=lF8lnN5ze8MTA4E3Hbfea3YL/rrWHZFZW+dMUK2vNFM=;
b=Kf8S6YqV2hBHS0CBQkTB5th/Il7NR2tyku/nhJK/O365RsOb59y2uKBszKOE2EQ7Mn
wM1QSqLK9ECCKht+9AJNZ7L0BXEWXkbkhTndifvpIOEwxvi9buKlpPCfCuRUbISgGKdR
/GCVC6H2fgJRJbCbRhGnmzBxFUQcE41m2Q63r/LjrnhDSQz5GVsmNqui3ztK0+8zZrtY
fMI9Q0PrN/WugkC47gQw4uI8MmSOgKfOPibL/nrWvzwp0B5dJPTZeZ7Znc7Hs0G30nyp
9hPkvPTao8XmHhSLjCDRlRS/FA3zseZhAArhxUPVa87bMokeYl4s1/YtcGMUIjy91cn8
QGEQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20210112;
h=x-gm-message-state:date:from:to:cc:subject:message-id:references
:mime-version:content-disposition:content-transfer-encoding
:in-reply-to;
bh=lF8lnN5ze8MTA4E3Hbfea3YL/rrWHZFZW+dMUK2vNFM=;
b=jUuIWwvgNxOJN55p9GQChXrD4QtO/hMcyxo6oqkN2LopX5UzvCQTnY3AN6yqi8apa3
Ll7C3osjAJJhzr3hE7fuO8tiZiytvH1yme2/glG6QGkWnxUtuoEeWhaXy01kGEDYt/Rh
dA4DNET3/3x3hWkr5hMCE7Psjvc5Fj0vliEEesS2aoIeryveDMLKCt+rki5kk+2OQF6P
1nMPE3zLnJfQ3fO+wyoxUWwRWDnDilbzRss+pDg3TRH769wL8LPDU6WcvtALEgVW+HSa
ZD+IIjHynRmweWmLF9HC0IE1CFLZuCR5QijMm8sg1CscAlb2GUxjMtllBrKk9dfPgxu2
CfMw==
X-Gm-Message-State: AOAM530r/PpRXFE/voFYtgLIo+xiC7DIWfT01y7/kghavd2DnFBiCa1j
M0RsjfGTwFaNjWGDIu+/0pRyaw==
X-Google-Smtp-Source: ABdhPJwLv3ge6Zi+1bOKltR/0NwQAJOlB61qkTIKLv8UhLocZS/Qyqh6Xy4G0Xe7AAZllPmjbbJAvw==
X-Received: by 2002:a5d:494d:: with SMTP id r13mr38663451wrs.222.1635322563818;
Wed, 27 Oct 2021 01:16:03 -0700 (PDT)
Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25])
by smtp.gmail.com with ESMTPSA id u16sm1022508wrt.97.2021.10.27.01.16.03
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Wed, 27 Oct 2021 01:16:03 -0700 (PDT)
Date: Wed, 27 Oct 2021 10:16:02 +0200
From: Olivier Matz <olivier.matz@6wind.com>
To: David Marchand <david.marchand@redhat.com>
Cc: Thomas Monjalon <thomas@monjalon.net>, dev <dev@dpdk.org>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"Yigit, Ferruh" <ferruh.yigit@intel.com>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
Bing Zhao <bingz@nvidia.com>, Xiaoyun Li <xiaoyun.li@intel.com>
Message-ID: <YXkKwpILAopmV/mF@platinum>
References: <20211026145851.21944-1-david.marchand@redhat.com>
<3415992.2jfn0xn0IN@thomas>
<CAJFAV8xpfLgxGR=vB0u558HKc=0jEMmvjqjYtjqExomtWuVe8Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAJFAV8xpfLgxGR=vB0u558HKc=0jEMmvjqjYtjqExomtWuVe8Q@mail.gmail.com>
Subject: Re: [dpdk-dev] [PATCH] ethdev: warn only once for badly behaving
applications
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Hi,
On Wed, Oct 27, 2021 at 09:20:52AM +0200, David Marchand wrote:
> On Tue, Oct 26, 2021 at 5:57 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 26/10/2021 16:58, David Marchand:
> > > Warning continuously is a pain when developping or if a unit test
> > > is/gets broken.
> > >
> > > It could also be a problem if application behaves badly only in some
> > > corner cases and a DoS results of those logs being continuously displayed.
> > >
> > > Let's warn once per port and per rx/tx.
> > >
> > > Getting such a log is scary, but let's make it more eye catching by
> > > dumping a backtrace with it.
> > [...]
> > > Fixes: c87d435a4d79 ("ethdev: copy fast-path API into separate structure")
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > [...]
> > > +static struct dummy_queue *dummy_queues_ref[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> > > +static struct dummy_queue dummy_queues[RTE_MAX_ETHPORTS];
> >
> > I feel we could better name those arrays, maybe adding a comment.
> > First one is really queues array while the second one is to share
> > the same value with all queues of a port. Right?
>
> Yes, look fwd to v2 for better names.
>
>
> >
> > > +RTE_INIT(dummy_queue_init)
> > > +{
> > > + uint16_t port_id;
> > > +
> > > + for (port_id = 0; port_id < RTE_DIM(dummy_queues); port_id++) {
> > > + unsigned int i;
> >
> > q would be a better name than i
>
> Ok, and I'll rename other variable q for actual queue objects later in
> the patch.
>
>
> > > eth_dev_fp_ops_reset(struct rte_eth_fp_ops *fpo)
> > > {
> > > static void *dummy_data[RTE_MAX_QUEUES_PER_PORT];
> > > - static const struct rte_eth_fp_ops dummy_ops = {
> > > + uint16_t port_id = fpo - rte_eth_fp_ops;
> > > +
> > > + dummy_queues[port_id].rx_warn_once = false;
> > > + dummy_queues[port_id].tx_warn_once = false;
> > > + *fpo = (struct rte_eth_fp_ops) {
> > > .rx_pkt_burst = dummy_eth_rx_burst,
> > > .tx_pkt_burst = dummy_eth_tx_burst,
> > > - .rxq = {.data = dummy_data, .clbk = dummy_data,},
> > > - .txq = {.data = dummy_data, .clbk = dummy_data,},
> > > + .rxq = (struct rte_ethdev_qdata) {
> >
> > Why this cast? rte_eth_fp_ops.rxq is of type rte_ethdev_qdata.
>
> Funny how the compiler complains about:
>
> ../lib/ethdev/ethdev_private.c: In function ‘eth_dev_fp_ops_reset’:
> ../lib/ethdev/ethdev_private.c:243:9: error: expected expression
> before ‘{’ token
> *fpo = {
> ^
> if we don't explicitely tell this anonymous struct is of type struct
> rte_eth_fp_ops (note that *fpo is of type struct rte_eth_fp_ops).
> But otoh, compiler silently understands that, in .rxq case, the
> anonymous struct is of type rte_ethdev_qdata.
>
> So indeed, it works without the cast on .rxq and .txq.
> I applied the cast on all anonymous struct in my patch once I hit the
> first compiler complaint.
>
> Do you have the explanation or can you point me at some standard
> explaining the difference in treatment?
Let me try an explanation, hope it is the correct one.
In the first case, this is an assignment as described in 6.5.16 of
the standard [1]:
*fpo = (struct rte_eth_fp_ops) { .rx_pkt_burst = dummy_eth_rx_burst, ... };
The compiler expects the right side to be an expression. The expression
is a "compound literal", as described in 6.5.2.5:
1. The type name shall specify a complete object type or an array of
unknown size, but not avariable length array type.
2. All the constraints for initializer lists in 6.7.9 also apply to
compound literals
The second cast { ..., .rxq = (struct rte_ethdev_qdata) { ... } } is
inside a construction that behaves like an initialization (according to
the second point above). The compiler already knows the type of the
struct (and therefore the types of the fields), so the cast is not
required.
[1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf
> >
> > > + .data = (void **)&dummy_queues_ref[port_id],
> > > + .clbk = dummy_data,
> > > + },
> > > + .txq = (struct rte_ethdev_qdata) {
> > > + .data = (void **)&dummy_queues_ref[port_id],
> > > + .clbk = dummy_data,
> > > + },
> > > };
> > > -
> > > - *fpo = dummy_ops;
> > > }
>
>
> --
> David Marchand
>