[dpdk-dev] [PATCH v6 1/2] ethdev: add packet integrity checks
Thomas Monjalon
thomas at monjalon.net
Sun Apr 18 23:30:57 CEST 2021
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 at monjalon.net>
> > Sent: Sunday, April 18, 2021 21:12
> > To: Gregory Etelson <getelson at nvidia.com>
> > Cc: Ori Kam <orika at nvidia.com>; ajit.khaparde at broadcom.com;
> > andrew.rybchenko at oktetlabs.ru; dev at dpdk.org; ferruh.yigit at intel.com;
> > jerinj at marvell.com; jerinjacobk at gmail.com; olivier.matz at 6wind.com;
> > Slava Ovsiienko <viacheslavo at nvidia.com>; Gregory Etelson
> > <getelson at nvidia.com>; Matan Azrad <matan at nvidia.com>; Raslan
> > Darawsheh <rasland at 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;
> > > + };
> > > +};
More information about the dev
mailing list