DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Gregory Etelson <getelson@nvidia.com>
Cc: Ori Kam <orika@nvidia.com>,
	"ajit.khaparde@broadcom.com" <ajit.khaparde@broadcom.com>,
	"andrew.rybchenko@oktetlabs.ru" <andrew.rybchenko@oktetlabs.ru>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>,
	"jerinjacobk@gmail.com" <jerinjacobk@gmail.com>,
	"olivier.matz@6wind.com" <olivier.matz@6wind.com>,
	Slava Ovsiienko <viacheslavo@nvidia.com>,
	Matan Azrad <matan@nvidia.com>,
	Raslan Darawsheh <rasland@nvidia.com>
Subject: Re: [dpdk-dev] [PATCH v6 1/2] ethdev: add packet integrity checks
Date: Sun, 18 Apr 2021 23:30:57 +0200	[thread overview]
Message-ID: <5976080.8eoqQYxcXj@thomas> (raw)
In-Reply-To: <BY5PR12MB483496A510C8DB0CE5577993A54A9@BY5PR12MB4834.namprd12.prod.outlook.com>

Please read again the comment I did below,
and try 32-bit bitfield instead of 64-bit.


18/04/2021 21:24, Gregory Etelson:
> Hello Thomas,
> 
> I modified the following drivers/net/mlx5/mlx5_flow_age.c compilation command
> to produce pre-processed source code output:
> 
>      1 # 1 "../drivers/net/mlx5/mlx5_flow_age.c"
>      2 # 1 "/.autodirect/mtrswgwork/getelson/src/dpdk/stable/build-dev//"
>      3 # 1 "<built-in>"
>      4 #define __STDC__ 1
> ** 5 #define __STDC_VERSION__ 201112L
>      6 #define __STDC_UTF_16__ 1
> 
> According to the result, the built-in __STDC_VERSION__ macro was set to 201112L.
> Therefore, in rte_common.h, RTE_STD_C11 macro was evaluated as empty value:
> 
> Source code:
>  30 #ifndef typeof
>  31 #define typeof __typeof__
>  32 #endif
>  33
>  34 #ifndef asm
>  35 #define asm __asm__
>  36 #endif
>  37
>  38 /** C extension macro for environments lacking C11 features. */
>  39 #if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L
>  40 #define RTE_STD_C11 __extension__
>  41 #else
>  42 #define RTE_STD_C11
>  43 #endif
> 
> Preprocessor output:
> # 29 "../lib/librte_eal/include/rte_common.h" 2
> #define typeof __typeof__
> #define asm __asm__
> #define RTE_STD_C11
> 
> According to these results, RTE_STD_C11 location in code has no significance,
> because it will always be replaced with empty string.
> After I changed RTE_STD_C11 condition like this: 
> 
> -  #if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L
> + #if !defined(__STDC_VERSION__) || __STDC_VERSION__ <= 201112L
> 
> -__extension__
> +RTE_STD_C11
>  struct rte_flow_item_integrity {
> 
> the compilation completed successfully both for 32 and 64 bits value.
> 
> Regards,
> Gregory. 
> 
> The compilation command was copied from `ninja --verbose` output:
> cc -Idrivers/libtmp_rte_net_mlx5.a.p -Idrivers -I../drivers -Idrivers/net/mlx5 -I../drivers/net/mlx5 \
> -Idrivers/net/mlx5/linux -I../drivers/net/mlx5/linux -Ilib/librte_ethdev -I../lib/librte_ethdev \
> -I. -I.. -Iconfig -I../config -Ilib/librte_eal/include -I../lib/librte_eal/include -Ilib/librte_eal/linux/include \
> -I../lib/librte_eal/linux/include -Ilib/librte_eal/x86/include -I../lib/librte_eal/x86/include \
> -Ilib/librte_eal/common -I../lib/librte_eal/common -Ilib/librte_eal -I../lib/librte_eal \
> -Ilib/librte_kvargs -I../lib/librte_kvargs -Ilib/librte_metrics -I../lib/librte_metrics \
> -Ilib/librte_telemetry -I../lib/librte_telemetry -Ilib/librte_net -I../lib/librte_net \
> -Ilib/librte_mbuf -I../lib/librte_mbuf -Ilib/librte_mempool -I../lib/librte_mempool \
> -Ilib/librte_ring -I../lib/librte_ring -Ilib/librte_meter -I../lib/librte_meter -Idrivers/bus/pci \
> -I../drivers/bus/pci -I../drivers/bus/pci/linux -Ilib/librte_pci -I../lib/librte_pci \
> -Idrivers/bus/vdev -I../drivers/bus/vdev -Ilib/librte_hash -I../lib/librte_hash \
> -Ilib/librte_rcu -I../lib/librte_rcu -Idrivers/common/mlx5 -I../drivers/common/mlx5 \
> -Idrivers/common/mlx5/linux -I../drivers/common/mlx5/linux -I/usr//usr/include \
> -I/usr/include/libnl3 -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -g \
> -include rte_config.h -Wextra -Wcast-qual -Wdeprecated -Wformat \
> -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes \
> -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -Wstrict-prototypes \
> -Wundef -Wwrite-strings -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC \
> -march=native -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -std=c11 \
> -Wno-strict-prototypes -D_BSD_SOURCE -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600 \
> -pedantic -DPEDANTIC -MD -MQ drivers/libtmp_rte_net_mlx5.a.p/net_mlx5_mlx5_flow_age.c.o \
> -MF drivers/libtmp_rte_net_mlx5.a.p/net_mlx5_mlx5_flow_age.c.o.d \
> -o drivers/libtmp_rte_net_mlx5.a.p/net_mlx5_mlx5_flow_age.c.o -c ../drivers/net/mlx5/mlx5_flow_age.c 
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Sunday, April 18, 2021 21:12
> > To: Gregory Etelson <getelson@nvidia.com>
> > Cc: Ori Kam <orika@nvidia.com>; ajit.khaparde@broadcom.com;
> > andrew.rybchenko@oktetlabs.ru; dev@dpdk.org; ferruh.yigit@intel.com;
> > jerinj@marvell.com; jerinjacobk@gmail.com; olivier.matz@6wind.com;
> > Slava Ovsiienko <viacheslavo@nvidia.com>; Gregory Etelson
> > <getelson@nvidia.com>; Matan Azrad <matan@nvidia.com>; Raslan
> > Darawsheh <rasland@nvidia.com>
> > Subject: Re: [PATCH v6 1/2] ethdev: add packet integrity checks
> > 
> > External email: Use caution opening links or attachments
> > 
> > 
> > 18/04/2021 17:51, Gregory Etelson:
> > > +__extension__
> > 
> > That still doesn't make sense, as in v5.
> > The things which require a macro are anonymous union, anonymous struct
> > and some bit fields with special sizes.
> > 
> > > +struct rte_flow_item_integrity {
> > > +     /**< Tunnel encapsulation level the item should apply to.
> > > +      * @see rte_flow_action_rss
> > > +      */
> > > +     uint32_t level;
> > 
> > Should have RTE_STD_C11 here.
> > 
> > > +     union {
> > 
> > Should have RTE_STD_C11 here.
> > 
> > > +             struct {
> > > +                     /**< The packet is valid after passing all HW checks. */
> > > +                     uint64_t packet_ok:1;
> > > +                     /**< L2 layer is valid after passing all HW checks. */
> > > +                     uint64_t l2_ok:1;
> > > +                     /**< L3 layer is valid after passing all HW checks. */
> > > +                     uint64_t l3_ok:1;
> > > +                     /**< L4 layer is valid after passing all HW checks. */
> > > +                     uint64_t l4_ok:1;
> > > +                     /**< L2 layer CRC is valid. */
> > > +                     uint64_t l2_crc_ok:1;
> > > +                     /**< IPv4 layer checksum is valid. */
> > > +                     uint64_t ipv4_csum_ok:1;
> > > +                     /**< L4 layer checksum is valid. */
> > > +                     uint64_t l4_csum_ok:1;
> > > +                     /**< The l3 length is smaller than the frame length. */
> > > +                     uint64_t l3_len_ok:1;
> > > +                     uint64_t reserved:56;
> > 
> > The reserved space looks useless since it is in an union.
> > 
> > > +             };
> > 
> > I'm not sure about the 64-bit bitfields.
> > Maybe that's why you need __extension__.
> > I feel 32 bits are enough.
> > 
> > > +             uint64_t value;
> > > +     };
> > > +};





  reply	other threads:[~2021-04-18 21:31 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-05 18:04 [dpdk-dev] [PATCH] " Ori Kam
2021-04-06  7:39 ` Jerin Jacob
2021-04-07 10:32   ` Ori Kam
2021-04-07 11:01     ` Jerin Jacob
2021-04-07 22:15       ` Ori Kam
2021-04-08  7:44         ` Jerin Jacob
2021-04-11  4:12           ` Ajit Khaparde
2021-04-11  6:03             ` Ori Kam
2021-04-13 15:16     ` [dpdk-dev] [PATCH v3 0/2] " Gregory Etelson
2021-04-13 15:16       ` [dpdk-dev] [PATCH v3 1/2] ethdev: " Gregory Etelson
2021-04-13 15:16       ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-13 17:15         ` Ferruh Yigit
2021-04-14 12:56     ` [dpdk-dev] [PATCH v4 0/2] add packet integrity checks Gregory Etelson
2021-04-14 12:56       ` [dpdk-dev] [PATCH v4 1/2] ethdev: " Gregory Etelson
2021-04-14 13:27         ` Ferruh Yigit
2021-04-14 13:31           ` Ferruh Yigit
2021-04-14 12:57       ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-14 16:09     ` [dpdk-dev] [PATCH v5 0/2] add packet integrity checks Gregory Etelson
2021-04-14 16:09       ` [dpdk-dev] [PATCH v5 1/2] ethdev: " Gregory Etelson
2021-04-14 17:24         ` Ajit Khaparde
2021-04-15 15:10           ` Ori Kam
2021-04-15 15:25             ` Ajit Khaparde
2021-04-15 16:46         ` Thomas Monjalon
2021-04-16  7:43           ` Ori Kam
2021-04-18  8:15             ` Gregory Etelson
2021-04-18 18:00               ` Thomas Monjalon
2021-04-14 16:09       ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-14 16:26       ` [dpdk-dev] [PATCH v5 0/2] add packet integrity checks Ferruh Yigit
2021-04-18 15:51     ` [dpdk-dev] [PATCH v6 " Gregory Etelson
2021-04-18 15:51       ` [dpdk-dev] [PATCH v6 1/2] ethdev: " Gregory Etelson
2021-04-18 18:11         ` Thomas Monjalon
2021-04-18 19:24           ` Gregory Etelson
2021-04-18 21:30             ` Thomas Monjalon [this message]
2021-04-18 15:51       ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-19  8:29     ` [dpdk-dev] [PATCH v7 0/2] add packet integrity checks Gregory Etelson
2021-04-19  8:29       ` [dpdk-dev] [PATCH v7 1/2] ethdev: " Gregory Etelson
2021-04-19  8:47         ` Thomas Monjalon
2021-04-19  8:29       ` [dpdk-dev] [PATCH v7 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-19 11:20       ` [dpdk-dev] [PATCH v7 0/2] add packet integrity checks Ferruh Yigit
2021-04-19 12:08         ` Gregory Etelson
2021-04-19 12:44     ` [dpdk-dev] [PATCH v8 " Gregory Etelson
2021-04-19 12:44       ` [dpdk-dev] [PATCH v8 1/2] ethdev: " Gregory Etelson
2021-04-19 14:09         ` Ajit Khaparde
2021-04-19 16:34           ` Thomas Monjalon
2021-04-19 17:06             ` Ferruh Yigit
2021-04-19 12:44       ` [dpdk-dev] [PATCH v8 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-19 14:09         ` Ajit Khaparde
2021-04-08  8:04 ` [dpdk-dev] [PATCH] ethdev: add packet integrity checks Andrew Rybchenko
2021-04-08 11:39   ` Ori Kam
2021-04-09  8:08     ` Andrew Rybchenko
2021-04-11  6:42       ` Ori Kam
2021-04-11 17:30         ` Ori Kam
2021-04-11 17:34 ` [dpdk-dev] [PATCH v2 0/2] " Gregory Etelson
2021-04-11 17:34   ` [dpdk-dev] [PATCH v2 1/2] ethdev: " Gregory Etelson
2021-04-12 17:36     ` Ferruh Yigit
2021-04-12 19:26       ` Ori Kam
2021-04-12 23:31         ` Ferruh Yigit
2021-04-13  7:12           ` Ori Kam
2021-04-13  8:03             ` Ferruh Yigit
2021-04-13  8:18               ` Ori Kam
2021-04-13  8:30                 ` Ferruh Yigit
2021-04-13 10:21                   ` Ori Kam
2021-04-13 17:28                     ` Ferruh Yigit
2021-04-11 17:34   ` [dpdk-dev] [PATCH v2 2/2] app/testpmd: add support for integrity item Gregory Etelson
2021-04-12 17:49     ` Ferruh Yigit
2021-04-13  7:53       ` Ori Kam
2021-04-13  8:14         ` Ferruh Yigit
2021-04-13 11:36           ` Ori Kam

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=5976080.8eoqQYxcXj@thomas \
    --to=thomas@monjalon.net \
    --cc=ajit.khaparde@broadcom.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=getelson@nvidia.com \
    --cc=jerinj@marvell.com \
    --cc=jerinjacobk@gmail.com \
    --cc=matan@nvidia.com \
    --cc=olivier.matz@6wind.com \
    --cc=orika@nvidia.com \
    --cc=rasland@nvidia.com \
    --cc=viacheslavo@nvidia.com \
    /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).