From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id B40BAA0563;
	Mon, 23 Mar 2020 10:19:28 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 3430D1C065;
	Mon, 23 Mar 2020 10:19:28 +0100 (CET)
Received: from mail-io1-f68.google.com (mail-io1-f68.google.com
 [209.85.166.68]) by dpdk.org (Postfix) with ESMTP id 3E2DD1C038
 for <dev@dpdk.org>; Mon, 23 Mar 2020 10:19:27 +0100 (CET)
Received: by mail-io1-f68.google.com with SMTP id e20so12948225ios.12
 for <dev@dpdk.org>; Mon, 23 Mar 2020 02:19:27 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=mime-version:references:in-reply-to:from:date:message-id:subject:to
 :cc:content-transfer-encoding;
 bh=yfH3Ro1BAOKGwvJazdWgRd+SWW8ksryGrBwIl4YG1Co=;
 b=IladZ/skgunR4ebOPmXVRbOLWFzBzGtcuDPZ/eeT4RxJKW1MzNbSMddJc3EpbvszBN
 bTkIQIDzDcSMbCc6ANrFiyT527dpBEiFwrzq2XPzNFOXuozYRrKvaT2zxo4ydcl2bAKk
 vPwveYjOFLPpW1OwCd/UeXlAxvetxhu8crKvea34UZ9CboM+LN3MQ56hwj4xnRbCr0yg
 ao5SRY4xKZlosXWAd7cP6NCuatcXml69SFU0X+77EiC1FUb6R2c6M11jVgGzlkHcAqr7
 LjBLJ0PBX4zSnM7+qIh8HaSmPtO4cX1NtUq3MR1Tz/BclIkM0XuMrnfOYMwn1aNo5QED
 R8NQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc:content-transfer-encoding;
 bh=yfH3Ro1BAOKGwvJazdWgRd+SWW8ksryGrBwIl4YG1Co=;
 b=dXBu0YPUt3T7j1f2xjBbi8lIDABdWoCYE/3WrO/cfyoCyU4KtPkQtXS83i/pCxc6Ih
 rmRWB+G7yDkRs4/pPEhp9YRt/DDRu1iWxULxiaMP8WtOTceI/F/QumuDaU+GwXyk++g9
 YBUIaEIpe9HuoPLn5sEpmggu7frZKE4kzoniyoVzIhMPkqKqF+yoYHDi6cravesrh1iQ
 Pp65EAb4MTByhqZq+53W6ChZEei818bjRhZVvQ+Xlkc8JVB9ceFqryNrpu6devN5z5tG
 EZCiTJu3mNECyvVzcJm2pcm9k8CvOk0/wc1IjhA4ZTILgI8n1FNcC1Um5vTEYg10wjFP
 IG+w==
X-Gm-Message-State: ANhLgQ2Iy9tCUV3/mgsC5+hr4Y6Y4bpj1s3naql0uYc5UvwUU+uHAv+8
 IsGgjXItR7uzMMTqXy11NFJMnU3YKVF441a0wxk=
X-Google-Smtp-Source: ADFU+vspYitksiE8tJlNBkhHZEyFSOOhoISPnzwivNq7JPxbivNaWSZhZuLeDvnClijwrCxrqoQMy593ME6NF/cyX2E=
X-Received: by 2002:a5d:8796:: with SMTP id f22mr18278923ion.163.1584955166411; 
 Mon, 23 Mar 2020 02:19:26 -0700 (PDT)
MIME-Version: 1.0
References: <20200318190241.3150971-1-jerinj@marvell.com>
 <20200318190241.3150971-14-jerinj@marvell.com>
 <dcd2d249-8fac-4954-d4d3-8ade64b0b018@ericsson.com>
In-Reply-To: <dcd2d249-8fac-4954-d4d3-8ade64b0b018@ericsson.com>
From: Jerin Jacob <jerinjacobk@gmail.com>
Date: Mon, 23 Mar 2020 14:49:10 +0530
Message-ID: <CALBAE1P1A1SHMLmMkYe6aQhwBxp2x_VD+_K+32zzkD3PuE+nUQ@mail.gmail.com>
To: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= <mattias.ronnblom@ericsson.com>
Cc: "jerinj@marvell.com" <jerinj@marvell.com>,
 Sunil Kumar Kori <skori@marvell.com>, "dev@dpdk.org" <dev@dpdk.org>, 
 "thomas@monjalon.net" <thomas@monjalon.net>, 
 "bruce.richardson@intel.com" <bruce.richardson@intel.com>, 
 "david.marchand@redhat.com" <david.marchand@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [dpdk-dev] [PATCH v1 13/32] eal/trace: implement provider
	payload
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
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>

On Fri, Mar 20, 2020 at 12:58 AM Mattias R=C3=B6nnblom
<mattias.ronnblom@ericsson.com> wrote:

Thanks for the review.

>
> On 2020-03-18 20:02, jerinj@marvell.com wrote:
> > From: Jerin Jacob <jerinj@marvell.com>
> >
> > The trace function payloads such as rte_trace_ctf_* have
> > dual functions. The first to emit the payload for the registration
> > function and the second one to act as trace mem emitters aka
> > provider payload.

> > +
> > +#define rte_trace_ctf_u64(in) __rte_trace_emit_datatype(in)
>
> Would it be worth to do a type check here? To avoid having someone do
> something like:
>
> uint32_t v =3D 42;
>
> rte_trace_ctf_u64(v);
>
> which would spew out a 32-bit number, where there should be 64 bits.

It is taken care with the register version of this macro by adding
RTE_BUILD_BUG_ON.

http://patches.dpdk.org/patch/66861/

See:
#define rte_trace_ctf_i64(in)\
        RTE_BUILD_BUG_ON(sizeof(int64_t) !=3D sizeof(typeof(in)));\
        __rte_trace_emit_ctf_field(sizeof(int64_t), RTE_STR(in), "int64_t")


>
> Or maybe better: do an assignment, allowing type conversion (promotion
> at least), and type-checking, of sorts. The macro-generated code could
> look something like:
>
> do {
>
>      uint64_t _in =3D in;
>
>      __rte_trace_emit_datatype(_in);
>
> } while (0)
>
> If you add a type parameter to __rte_trace_emit_datatype(), it can do
> the job.
>

> > +
> > +#define rte_trace_ctf_string(in)\
> Add the usual do { /../ } while (0) here?

Ack. Will add it in  the place where do { /../ } while (0) can be added in =
v2.

> > +     if (unlikely(in =3D=3D NULL))\
> > +             return;\
> > +     rte_strscpy(mem, in, __RTE_TRACE_EMIT_STRING_LEN_MAX);\
> > +     mem =3D RTE_PTR_ADD(mem, __RTE_TRACE_EMIT_STRING_LEN_MAX)
> > +
> >   #endif /* _RTE_TRACE_PROVIDER_H_ */
>
>