[dpdk-dev] [PATCH v1 1/1] examples/l2fwd-crypto: improve random key generator
> 2016-05-25 15:34, Piotr Azarewicz: > > This patch improve generate_random_key() function by replacing rand() > > function with reading from /dev/urandom. > > > > CID 120136 : Calling risky function (DC.WEAK_CRYPTO) > > dont_call: rand should not be used for security related applications, > > as linear congruential algorithms are too easy to break > > > > Coverity issue: 120136 > > > > Signed-off-by: Piotr Azarewicz > > --- > > examples/l2fwd-crypto/main.c | 18 +- > > Is it relevant for this example? Maybe not. But it don't break anything, and in the end make Coverity tool happy. Declan, please share your opinion.
[dpdk-dev] [PATCH v2 0/7] examples/ip_pipeline: CLI rework and improvements
> > Piotr Azarewicz (7): > > examples/ip_pipeline: add helper functions for parsing string > > examples/ip_pipeline: modifies common pipeline CLI > > examples/ip_pipeline: modifies firewall pipeline CLI > > examples/ip_pipeline: modifies flow classifications pipeline CLI > > examples/ip_pipeline: modifies flow action pipeline CLI > > examples/ip_pipeline: modifies routing pipeline CLI > > examples/ip_pipeline: update edge router usecase > > Please take care of the authorship in patches 2, 3 and 4. > It is probably wrong. You can fix it with git commit --amend --author in an > interactive rebase. > To avoid such issue, you must use git-am to apply patches. Thanks Thomas for hints.
[dpdk-dev] [PATCH v2] i40e: fix olflags for vector RX
> Problem: > The flag for RSS and flow director is not set correctly in the vector RX > function, so the upper layer APP which base on the related flags will not work > correctly. > > Fix this problem by change the shuffle table. the original shuffle table is > not > correct. > > Fixes: 9ed94 (i40e: add vector Rx) > > Signed-off-by: Zhe Tao Reviewed-by: Piotr Azarewicz
[dpdk-dev] [PATCH v3] i40e: fix olflags for vector Rx
> Problem:
> The flag for RSS and flow director is not set correctly in the vector Rx
> function, so the upper layer APP which base on the related flags will not work
> correctly.
>
> Fix this problem by change the shuffle table. the original shuffle table is
> not
> correct.
>
> Fixes: 9ed94e5bb04e ("i40e: add vector Rx")
>
> Signed-off-by: Zhe Tao
Reviewed-by: Piotr Azarewicz
[dpdk-dev] [PATCH v1 1/1] examples/bond: fix unchecked return value
> The example is calling rte_eal_wait_lcore without checking return value.
> Now it is fixed by checking the value and print proper message.
>
> Coverity issue: 37789
> Coverity issue: 37790
> Fixes: cc7e8ae84faa ("examples/bond: add example application for link
> bonding mode 6")
>
> Signed-off-by: Piotr Azarewicz
As you may see its acked by Declan:
http://article.gmane.org/gmane.comp.networking.dpdk.devel/43015
Sorry for my issue with email.
Acked-by: Declan Doherty
[dpdk-dev] [PATCH] doc: ip_pipeline app user guide
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Cristian Dumitrescu > Sent: Thursday, August 6, 2015 3:48 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH] doc: ip_pipeline app user guide > > Added more extensive documentation for ip_pipeline application. > > Signed-off-by: Cristian Dumitrescu > --- Acked-by: Piotr Azarewicz -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[dpdk-dev] [PATCH v1 1/1] example/ip_pipeline: fix memcpy issue
> -Original Message-
> From: Mcnamara, John
> Sent: Tuesday, December 8, 2015 6:00 PM
> To: Mcnamara, John ; Azarewicz, PiotrX T
> ; dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v1 1/1] example/ip_pipeline: fix memcpy
> issue
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Mcnamara, John
> > Sent: Tuesday, December 8, 2015 2:47 PM
> > To: Azarewicz, PiotrX T; dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v1 1/1] example/ip_pipeline: fix memcpy
> > issue
> >
> > > -Original Message-
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Piotr Azarewicz
> > > Sent: Tuesday, December 8, 2015 2:17 PM
> > > To: dev at dpdk.org
> > > Subject: [dpdk-dev] [PATCH v1 1/1] example/ip_pipeline: fix memcpy
> > > issue
> > >
> > > The cmds and thread_cmds both are the arrays of cmdline_parse_ctx_t.
> > > So the goal is to copy elements size of cmdline_parse_ctx_t not
> > > cmdline_parse_ctx_t*.
> > >
> > > Coverity issue: 120412
> > > Fixes: b4aee0fb9c6d ("examples/ip_pipeline: reconfigure thread
> > > binding
> > > dynamically")
> > >
> > > Signed-off-by: Piotr Azarewicz
> >
> > Acked-by: John McNamara
>
> Hi Piotr,
>
> This issue occurs copy and pasted in two other locations as well:
>
> examples/ip_pipeline/pipeline/pipeline_common_fe.c
> 1295: n_cmds * sizeof(cmdline_parse_ctx_t *));
>
> examples/ip_pipeline/thread_fe.c
> 340:n_cmds * sizeof(cmdline_parse_ctx_t *));
>
> examples/ip_pipeline/init.c
> 1475: n_cmds * sizeof(cmdline_parse_ctx_t *));
>
> Perhaps you could fix those in the same patch.
>
> Thanks,
>
> John
>
>
Yes, you are right, thanks. I will send v2.
Piotr
[dpdk-dev] [PATCH v2 5/5] doc: modify release notes and deprecation notice for table and pipeline
Hi Thomas,
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Thursday, October 8, 2015 1:42 PM
> To: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 5/5] doc: modify release notes and
> deprecation notice for table and pipeline
>
> Hi Maciej,
>
> 2015-09-11 12:31, Maciej Gajdzica:
> > The LIBABIVER number is incremented for table and pipeline libraries.
> > The release notes is updated and the deprecation announce is removed.
> [...]
> > --- a/lib/librte_pipeline/rte_pipeline_version.map
> > +++ b/lib/librte_pipeline/rte_pipeline_version.map
> > @@ -29,3 +29,11 @@ DPDK_2.1 {
> > rte_pipeline_table_stats_read;
> >
> > } DPDK_2.0;
> > +
> > +DPDK_2.2 {
> > + global:
> > +
> > + rte_pipeline_table_entry_add_bulk;
> > + rte_pipeline_table_entry_delete_bulk;
> > +
> > +} DPDK_2.1;
>
> The previous block was DPDK_2.0 for this library.
> So I think you should inherit from it, not DPDK_2.1 which doesn't exist in
> this
> context.
The previous block was DPDK_2.1 :
DPDK_2.1 {
global:
rte_pipeline_port_in_stats_read;
rte_pipeline_port_out_stats_read;
rte_pipeline_table_stats_read;
} DPDK_2.0;
So I think this patch is okay.
Correct me if I am wrong with my understanding, please.
Thanks,
Piotr
[dpdk-dev] [PATCH v2 2/5] pipeline: added bulk add/delete functions for table
Hi Thomas, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > Sent: Thursday, October 8, 2015 1:42 PM > To: Gajdzica, MaciejX T > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 2/5] pipeline: added bulk add/delete > functions for table > > 2015-09-11 12:31, Maciej Gajdzica: > > +/** > > + * Pipeline table entry delete bulk > > + * > > + * @param p > > + * Handle to pipeline instance > > + * @param table_id > > + * Table ID (returned by previous invocation of pipeline table create) > > + * @param keys > > + * Array containing table entry keys > > + * @param key_found > > + * On successful invocation, key_found for every item in the array is set > to > > + * TRUE (value different than 0) if key was found in the table before the > > + * delete operation and to FALSE (value 0) if not > > + * @param entries > > + * If entries pointer is NULL, this pointer is ignored for every entry > > found. > > + * Else, after successful invocation, if specific key is found in the > > table > > + * and entry points to a valid buffer, the table entry contents (as it > > was > > + * before the delete was performed) is copied to this buffer. > > + * @return > > + * 0 on success, error code otherwise > > + */ > > +int rte_pipeline_table_entry_delete_bulk(struct rte_pipeline *p, > > + uint32_t table_id, > > + void **keys, > > + uint32_t n_keys, > > + int *key_found, > > + struct rte_pipeline_table_entry **entries); > > The parameter n_keys is not documented. > Doxygen is reporting the error. Thanks for the finding. Piotr
[dpdk-dev] [PATCH v2 5/5] doc: modify release notes and deprecation notice for table and pipeline
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Monday, October 12, 2015 10:23 AM
> To: Azarewicz, PiotrX T
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 5/5] doc: modify release notes and
> deprecation notice for table and pipeline
>
> 2015-10-12 07:53, Azarewicz, PiotrX T:
> > Hi Thomas,
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > > Hi Maciej,
> > >
> > > 2015-09-11 12:31, Maciej Gajdzica:
> > > > --- a/lib/librte_pipeline/rte_pipeline_version.map
> > > > +++ b/lib/librte_pipeline/rte_pipeline_version.map
> > > > @@ -29,3 +29,11 @@ DPDK_2.1 {
> > > > rte_pipeline_table_stats_read;
> > > >
> > > > } DPDK_2.0;
> > > > +
> > > > +DPDK_2.2 {
> > > > + global:
> > > > +
> > > > + rte_pipeline_table_entry_add_bulk;
> > > > + rte_pipeline_table_entry_delete_bulk;
> > > > +
> > > > +} DPDK_2.1;
> > >
> > > The previous block was DPDK_2.0 for this library.
> > > So I think you should inherit from it, not DPDK_2.1 which doesn't
> > > exist in this context.
> >
> > The previous block was DPDK_2.1 :
> >
> > DPDK_2.1 {
> > global:
> >
> > rte_pipeline_port_in_stats_read;
> > rte_pipeline_port_out_stats_read;
> > rte_pipeline_table_stats_read;
> >
> > } DPDK_2.0;
> >
> > So I think this patch is okay.
> > Correct me if I am wrong with my understanding, please.
>
> You are perfectly right.
> Sorry for the confusion.
No problem, thank for the answer.
Piotr
[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17
> > Tested-by: Yongjie Gu > > Applied OS: UB1204 GCC: 4.6.4 Kernel: 3.13.0-45 glibc 2.15 x86_64-native-linuxapp-gcc: FAIL
[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17
> > > > Tested-by: Yongjie Gu > > > > > > Applied > > > > OS: UB1204 > > GCC: 4.6.4 > > Kernel: 3.13.0-45 > > glibc 2.15 > > > > x86_64-native-linuxapp-gcc: FAIL > > Please don't be a bot and explain us the error you see. I was trying rc3 + fix and latest (today) dpdk version. The same fail message: /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_timer.o): In function `get_tsc_freq': eal_timer.c:(.text+0x128): undefined reference to `clock_gettime' eal_timer.c:(.text+0x166): undefined reference to `clock_gettime' /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function `eal_alarm_callback': eal_alarm.c:(.text+0xda): undefined reference to `clock_gettime' /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function `rte_eal_alarm_set': eal_alarm.c:(.text+0x211): undefined reference to `clock_gettime'
[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17
> > > > > > Tested-by: Yongjie Gu > > > > > > > > > > Applied > > > > > > > > OS: UB1204 > > > > GCC: 4.6.4 > > > > Kernel: 3.13.0-45 > > > > glibc 2.15 > > > > > > > > x86_64-native-linuxapp-gcc: FAIL > > > > > > Please don't be a bot and explain us the error you see. > > > > I was trying rc3 + fix and latest (today) dpdk version. The same fail > message: > > > > /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_timer.o): In function > `get_tsc_freq': > > eal_timer.c:(.text+0x128): undefined reference to `clock_gettime' > > eal_timer.c:(.text+0x166): undefined reference to `clock_gettime' > > /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function > `eal_alarm_callback': > > eal_alarm.c:(.text+0xda): undefined reference to `clock_gettime' > > /x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o): In function > `rte_eal_alarm_set': > > eal_alarm.c:(.text+0x211): undefined reference to `clock_gettime' > > Interesting. > Could check the command line in verbose mode to see where is -lrt please? Here you are. -lrt is in separate line: gcc -o test -m64 -pthread -march=native -DRTE_MACHINE_CPUFLAG_SSE -DRTE_MACHINE_CPUFLAG_SSE2 -DRTE_MACHINE_CPUFLAG_SSE3 -DRTE_MACHINE_CPUFLAG_SSSE3 -DRTE_MACHINE_CPUFLAG_SSE4_1 -DRTE_MACHINE_CPUFLAG_SSE4_2 -DRTE_MACHINE_CPUFLAG_AES -DRTE_MACHINE_CPUFLAG_PCLMULQDQ -DRTE_MACHINE_CPUFLAG_AVX -I/home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/include -include /home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/include/rte_config.h -O3 -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wpointer-arith -Wcast-align -Wnested-externs -Wcast-qual -Wformat-nonliteral -Wformat-security -Wundef -Wwrite-strings -Werror -Wno-missing-field-initializers -Wno-uninitialized -D_GNU_SOURCE commands.o test.o resource.o test_resource.o test_resource_c.res.o test_prefetch.o test_byteorder.o test_per_lcore.o test_atomic.o test_malloc.o test_cycles.o test_spinlock.o test_memory.o test_memzone.o test_ring.o test_ring_perf.o test_pmd_perf.o test_table.o test_table_pipeline.o test_table_tables.o test_table_ports.o test_table_combined.o test_table_acl.o test_rwlock.o test_timer.o test_timer_perf.o test_timer_racecond.o test_mempool.o test_mempool_perf.o test_mbuf.o test_logs.o test_memcpy.o test_memcpy_perf.o test_hash.o test_thash.o test_hash_perf.o test_hash_functions.o test_hash_scaling.o test_hash_multiwriter.o test_lpm.o test_lpm_perf.o test_lpm6.o test_lpm6_perf.o test_debug.o test_errno.o test_tailq.o test_string_fns.o test_cpuflags.o test_mp_secondary.o test_eal_flags.o test_eal_fs.o test_alarm.o test_interrupts.o test_version.o test_func_reentrancy.o test_cmdline.o test_cmdline_num.o test_cmdline_etheraddr.o test_cmdline_portlist.o test_cmdline_ipaddr.o test_cmdline_cirbuf.o test_cmdline_string.o test_cmdline_lib.o test_red.o test_sched.o test_meter.o test_kni.o test_power.o test_power_acpi_cpufreq.o test_power_kvm_vm.o test_common.o test_distributor.o test_distributor_perf.o test_reorder.o test_devargs.o virtual_pmd.o packet_burst_generator.o test_acl.o test_link_bonding.o test_link_bonding_mode4.o test_link_bonding_rssconf.o test_pmd_ring.o test_pmd_ring_perf.o test_cryptodev_aes.o test_cryptodev_perf.o test_cryptodev.o test_kvargs.o -Wl, -lrt -Wl,-lm -L/home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/lib -Wl,-lrte_kni -Wl,-lrte_pipeline -Wl,-lrte_table -Wl,-lrte_port -Wl,-lrte_pdump -Wl,-lrte_distributor -Wl,-lrte_reorder -Wl,-lrte_ip_frag -Wl,-lrte_meter -Wl,-lrte_sched -Wl,-lrte_lpm -Wl,--whole-archive -Wl,-lrte_acl -Wl,--no-whole-archive -Wl,-lrte_jobstats -Wl,-lrte_power -Wl,--whole-archive -Wl,-lrte_timer -Wl,-lrte_hash -Wl,-lrte_vhost -Wl,-lrte_kvargs -Wl,-lrte_mbuf -Wl,-lethdev -Wl,-lrte_cryptodev -Wl,-lrte_mempool -Wl,-lrte_ring -Wl,-lrte_eal -Wl,-lrte_cmdline -Wl,-lrte_cfgfile -Wl,-lrte_pmd_bond -Wl,-lrte_pmd_af_packet -Wl,-lrte_pmd_bnxt -Wl,-lrte_pmd_cxgbe -Wl,-lrte_pmd_e1000 -Wl,-lrte_pmd_ena -Wl,-lrte_pmd_enic -Wl,-lrte_pmd_fm10k -Wl,-lrte_pmd_i40e -Wl,-lrte_pmd_ixgbe -Wl,-lrte_pmd_null -Wl,-lrte_pmd_ring -Wl,-lrte_pmd_virtio -Wl,-lrte_pmd_vhost -Wl,-lrte_pmd_vmxnet3_uio -Wl,-lrte_pmd_null_crypto -Wl,--no-whole-archive -Wl,-ldl -Wl,-export-dynamic -Wl,-export-dynamic -Wl,-export-dynamic -L/home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/lib -Wl,--as-needed -Wl,-Map=test.map -Wl,--cref /home/ptazarex/dpdk_master/x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_timer.o): In function `get_tsc_freq': eal_timer.c:(.text+0x128): undefined reference to `clock_gettime'
[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17
> The problem is that -lrt appears before -lrte_eal. > The question is: where does it come from? > It is even before _LDLIBS-y += -L$(RTE_SDK_BIN)/lib... mystery root cause: commit c7cda4d8b4ea9cb0f209dda36882d225354b1db9 and my workaround is: /app/test/Makefile ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y) -LDLIBS += -lrt SRCS-y += test_red.c SRCS-y += test_sched.c endif
[dpdk-dev] [PATCH] mk: fix static link with glibc < 2.17
> > > The problem is that -lrt appears before -lrte_eal.
> > > The question is: where does it come from?
> > > It is even before _LDLIBS-y += -L$(RTE_SDK_BIN)/lib... mystery
> >
> > root cause:
> > commit c7cda4d8b4ea9cb0f209dda36882d225354b1db9
>
> The error is seen after this commit, yes.
> But I would not say it is the root cause.
Yes, you are right.
> The root cause is adding -lrt before other libs:
> 281948b4753e ("mk: fix missing librt dependencies")
>
> > and my workaround is:
> > /app/test/Makefile
> >
> > ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y)
> > -LDLIBS += -lrt
> > SRCS-y += test_red.c
> > SRCS-y += test_sched.c
> > endif
>
> Yes it is what I've done in this patch:
> http://dpdk.org/patch/15008
Great, thanks.
[dpdk-dev] [PATCH v1] i40e: fix olflags for vector RX
Hi, > --- a/drivers/net/i40e/i40e_rxtx_vec.c > +++ b/drivers/net/i40e/i40e_rxtx_vec.c > @@ -149,7 +149,7 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf > **rx_pkts) > > /* mask everything except rss and vlan flags > *bit2 is for vlan tag, bits 13:12 for rss > */ Please, update or remove the comment above. Thanks, Piotr > const __m128i rss_vlan_msk = _mm_set_epi16( > 0x, 0x, 0x, 0x, > - 0x3004, 0x3004, 0x3004, 0x3004); > + 0x3804, 0x3804, 0x3804, 0x3804); >
[dpdk-dev] [PATCH v3 12/13] enic: expand local Tx mbuf flags variable to 64-bits
> Coverity issue: 13218
> Fixes: fefed3d1e62c ("enic: new driver")
>
> Suggested-by: Piotr Azarewicz
> Signed-off-by: John Daley
> ---
> This is essentially patch http://www.dpdk.org/dev/patchwork/patch/12642
> applied after the enic_send_packet function was melded into the main
> transmit funciton.
>
Acked-by: Piotr Azarewicz
Re: [dpdk-dev] [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding bytes for GCM
Hi Arek,
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Arek Kusztal
> Sent: Friday, December 23, 2016 9:25 AM
> To: dev@dpdk.org
> Cc: Trahe, Fiona ; De Lara Guarch, Pablo
> ; Griffin, John ;
> Jain, Deepak K ; Doherty, Declan
> ; Kusztal, ArkadiuszX
>
> Subject: [dpdk-dev] [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding bytes
> for GCM
>
> This commit fixes pre-counter block (J0) padding by clearing four most
> significant bytes before setting initial counter value.
>
> Fixes: b2bb3597470c ("crypto/aesni_gcm: move pre-counter block to driver")
>
> Signed-off-by: Arek Kusztal
> ---
> drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> index dba5e15..af3d60f 100644
> --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> @@ -40,6 +40,7 @@
> #include
> #include
> #include
> +#include
>
> #include "aesni_gcm_pmd_private.h"
>
> @@ -241,7 +242,8 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp,
> struct rte_crypto_sym_op *op,
>* to set BE LSB to 1, driver expects that 16B is allocated
I think that 16B expected by driver while only 12B IV is supported is not clear
from user perspective.
I think that we should expect 12B only and allocate 16B locally.
>*/
> if (op->cipher.iv.length == 12) {
> - op->cipher.iv.data[15] = 1;
> + uint32_t *iv_padd = (uint32_t *)&op->cipher.iv.data[12];
> + *iv_padd = rte_bswap32(1);
Should not be that the last byte (number 15) always be set to 1?
> }
>
> if (op->auth.aad.length != 12 && op->auth.aad.length != 8 &&
> --
> 2.1.0
Re: [dpdk-dev] [PATCH v2 2/3] crypto/aesni_gcm: fix iv size in PMD capabilities
> Subject: [dpdk-dev] [PATCH v2 2/3] crypto/aesni_gcm: fix iv size in PMD
> capabilities
>
> This patch sets iv size in aesni gcm PMD to 12 bytes to be conformant with
> nist SP800-38D.
>
> Fixes: eec136f3c54f ("aesni_gcm: add driver for AES-GCM crypto
> operations")
>
> Signed-off-by: Arek Kusztal
> ---
> drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> index e824d4b..c51f82a 100644
> --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> @@ -77,8 +77,8 @@ static const struct rte_cryptodev_capabilities
> aesni_gcm_pmd_capabilities[] = {
> .increment = 0
> },
> .iv_size = {
> - .min = 16,
> - .max = 16,
> + .min = 12,
> + .max = 12,
> .increment = 0
> }
> }, }
I think that we should also remove 16 na 0 bytes allowed in
process_gcm_crypto_op() function:
if (op->cipher.iv.length != 16 && op->cipher.iv.length != 12 &&
op->cipher.iv.length != 0) {
GCM_LOG_ERR("iv");
return -1;
}
Regards,
Piotr
Re: [dpdk-dev] [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding bytes for GCM
Hi Arek,
> > Subject: [dpdk-dev] [PATCH v2 1/3] crypto/aesni_gcm: fix J0 padding
> > bytes for GCM
> >
> > This commit fixes pre-counter block (J0) padding by clearing four most
> > significant bytes before setting initial counter value.
> >
> > Fixes: b2bb3597470c ("crypto/aesni_gcm: move pre-counter block to
> > driver")
> >
> > Signed-off-by: Arek Kusztal
> > ---
> > drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > index dba5e15..af3d60f 100644
> > --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > @@ -40,6 +40,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include "aesni_gcm_pmd_private.h"
> >
> > @@ -241,7 +242,8 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp,
> > struct rte_crypto_sym_op *op,
> > * to set BE LSB to 1, driver expects that 16B is allocated
>
> I think that 16B expected by driver while only 12B IV is supported is not
> clear
> from user perspective.
> I think that we should expect 12B only and allocate 16B locally.
I didn't notice that this exception is also described in rte_crypto_sym.h, so
this is fine.
>
> > */
> > if (op->cipher.iv.length == 12) {
> > - op->cipher.iv.data[15] = 1;
> > + uint32_t *iv_padd = (uint32_t *)&op->cipher.iv.data[12];
> > + *iv_padd = rte_bswap32(1);
>
> Should not be that the last byte (number 15) always be set to 1?
I didn't notice that this code will always run in little-endian machine, so
this is fine too.
>
> > }
> >
> > if (op->auth.aad.length != 12 && op->auth.aad.length != 8 &&
> > --
> > 2.1.0
Acked-by: Piotr Azarewicz
Re: [dpdk-dev] [PATCH v2 2/3] crypto/aesni_gcm: fix iv size in PMD capabilities
> Subject: Re: [dpdk-dev] [PATCH v2 2/3] crypto/aesni_gcm: fix iv size in PMD
> capabilities
>
> > Subject: [dpdk-dev] [PATCH v2 2/3] crypto/aesni_gcm: fix iv size in
> > PMD capabilities
> >
> > This patch sets iv size in aesni gcm PMD to 12 bytes to be conformant
> > with nist SP800-38D.
> >
> > Fixes: eec136f3c54f ("aesni_gcm: add driver for AES-GCM crypto
> > operations")
> >
> > Signed-off-by: Arek Kusztal
> > ---
> > drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> > b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> > index e824d4b..c51f82a 100644
> > --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> > +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
> > @@ -77,8 +77,8 @@ static const struct rte_cryptodev_capabilities
> > aesni_gcm_pmd_capabilities[] = {
> > .increment = 0
> > },
> > .iv_size = {
> > - .min = 16,
> > - .max = 16,
> > + .min = 12,
> > + .max = 12,
> > .increment = 0
> > }
> > }, }
>
> I think that we should also remove 16 na 0 bytes allowed in
> process_gcm_crypto_op() function:
> if (op->cipher.iv.length != 16 && op->cipher.iv.length != 12 &&
> op->cipher.iv.length != 0) {
> GCM_LOG_ERR("iv");
> return -1;
> }
I found this notice about IV in rte_crypto_sym.h :
* - For GCM mode, this is either the IV (if the length
* is 96 bits) or J0 (for other sizes), where J0 is as
* defined by NIST SP800-38D. Regardless of the IV
* length, a full 16 bytes needs to be allocated.
So it is fine to leave unchanged above code.
Acked-by: Piotr Azarewicz
Re: [dpdk-dev] [PATCH v5] crypto/aesni_gcm: migration from MB library to ISA-L
> 2017-01-16 13:37, Piotr Azarewicz: > > app/test/test_cryptodev.c| 753 > > +++--- > > app/test/test_cryptodev_gcm_test_vectors.h | 491 +- > > devtools/test-build.sh |4 +- > > doc/guides/cryptodevs/aesni_gcm.rst | 24 +- > > doc/guides/rel_notes/release_17_02.rst | 12 + > > drivers/crypto/aesni_gcm/Makefile|8 +- > > drivers/crypto/aesni_gcm/aesni_gcm_ops.h | 95 +-- > > drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 324 +- > > drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c | 49 +- > > drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h | 15 +- > > mk/rte.app.mk|3 +- > > 11 files changed, 1363 insertions(+), 415 deletions(-) > > That's the kind of patch where test and implementation could be split. Okay I will.
[dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string
Hi Olivier, > -Original Message- > From: Olivier Matz [mailto:olivier.matz at 6wind.com] > Sent: Monday, April 4, 2016 10:01 AM > To: Azarewicz, PiotrX T > Cc: dev at dpdk.org > Subject: Re: [PATCH v1 1/1] cmdline: add any multi string mode to token > string > > Hi Piotr, > > This is globally ok for me. Please see a comment below. > Good to know it, thanks. > Using token_len + 1 as the buffer size in the snprintf looks a bit dangerous, > as > it won't protect from overflows. > > See the following example: > That's why snprintf() should still use STR_TOKEN_SIZE. > Okay, I see it. But this is a problem that we may need longer string than STR_TOKEN_SIZE in multi token case. So what you think about adding new typedef cmdline_multi_string_t for this case? For example: #define STR_MULTI_TOKEN_SIZE 1024 typedef char cmdline_multi_string_t[STR_MULTI_TOKEN_SIZE]; > > Regards, > Olivier
[dpdk-dev] [PATCH v1 1/1] cmdline: add any multi string mode to token string
Hi Olivier, > -Original Message- > From: Olivier Matz [mailto:olivier.matz at 6wind.com] > Sent: Monday, April 4, 2016 5:58 PM > >> Using token_len + 1 as the buffer size in the snprintf looks a bit > >> dangerous, as it won't protect from overflows. > >> > >> See the following example: > > > > > That's why snprintf() should still use STR_TOKEN_SIZE. > >> > > Okay, I see it. > > But this is a problem that we may need longer string than STR_TOKEN_SIZE > in multi token case. > > So what you think about adding new typedef cmdline_multi_string_t for > this case? > > For example: > > #define STR_MULTI_TOKEN_SIZE 1024 > > typedef char cmdline_multi_string_t[STR_MULTI_TOKEN_SIZE]; > > It should do the job, indeed. That's great. We want to set the value of the buffer to 4096, to not to regret in the future. > By the way, it would be nice to have an example of use. Based on this we plan a lot of changes in ip_pipeline example next release. Regards, Piotr
[dpdk-dev] [PATCH] doc: announce ABI change for rte_port_source_params structure
> > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Fan Zhang > > Sent: Thursday, March 31, 2016 2:29 PM > > To: dev at dpdk.org > > Subject: [dpdk-dev] [PATCH] doc: announce ABI change for > > rte_port_source_params structure > > > > Several new fields will be added to structure rte_port_source_params > > for source port enhancement with pcap file reading support. > > > > Signed-off-by: Fan Zhang > > Acked-by: Cristian Dumitrescu > > Acked-by: Jasvinder Singh Acked-by: Piotr Azarewicz
[dpdk-dev] [PATCH v2 1/1] cmdline: add any multi string mode to token string
> But I agree that a comment could be added above the definition of > TOKEN_STRING_MULTI that would explain what is the behavior in that case. > > Piotr, do you think this is something you can do? Okay, I will. Regards, Piotr
[dpdk-dev] [PATCH v6 3/4] app/test: added tests for libcrypto PMD
Hi Fiona, > This patch breaks autotests for all PMDs, due to increasing the MBUF size to > UNIT16_MAX. > USER1: Can't create CRYPTO_MBUFPOOL > > It needs more than 500MBs in the MBUFPOOL to run this test. > Setting this back to MBUF_SIZE fixes the issue, but breaks 2 tests in > libcrypto_autotest > > TestCase create_gmac_operation() line 5060 failed: no room to append aad > + TestCase [27] : test_AES_GMAC_authentication_test_case_4 failed > TestCase create_gmac_operation() line 5060 failed: no room to append aad > + TestCase [28] : test_AES_GMAC_authentication_verify_test_case_4 failed > > Can you investigate if it's possible to run these 2 tests using a different > smaller MBUFPOOL - maybe with only a few large mbufs? Yes, I think that it is possible to run these 2 tests using a different mempoll. I will make a fix for the issue. Piotr
[dpdk-dev] [PATCH v1] Modify and modularize l3fwd code
Hi Thomas, > Hi Ravi, > > 2016-02-02 12:01, Kulasek, TomaszX: > > Tested-by: Tomasz Kulasek > > Acked-by: Tomasz Kulasek > > I'm sorry that another patch was applied before this one. > Please could you rebase it? > It is too big to do it myself safely. > Thanks I rebased this patch. May I send it ? and if yes should I add Signed-off-by? Piotr
[dpdk-dev] [PATCH v3 1/1] examples/l3fwd: modify and modularize l3fwd code
>
> Hi,
>
> When compiling for i686, there are some errors:
>
> 2016-02-18 10:08, Piotr Azarewicz:
> > + printf("Hash: Adding 0x%" PRIx64 " keys\n",
> IPV4_L3FWD_EM_NUM_ROUTES);
> [...]
> > + printf("Hash: Adding 0x%" PRIx64 "keys\n",
> IPV6_L3FWD_EM_NUM_ROUTES);
>
> examples/l3fwd/l3fwd_em.c:437:9: error:
> format ?%llx? expects argument of type ?long long unsigned int?, but
> argument 2 has type ?unsigned int?
Thanks Thomas, v4 will be done.
Piotr
[dpdk-dev] [PATCH] lpm: unchecked return value
Hi,
I handle Coverity defect ID 13201. It is about unchecked return value from
rte_lpm6_delete() instances in rte_lpm6_add() function.
Next I found this thread and I see that both defects (ID 13205 and ID 13201)
may be resolved all together.
> >> Fix issue reported by Coverity.
> >>
> >> Coverity ID 13205: Unchecked return value Unchecked return value
> >> check_return: Calling rte_lpm6_add without checking return value
> >> Fixes: 5c510e13a9cb ("lpm: add IPv6 support")
> >>
> >> Signed-off-by: Slawomir Mrozowicz
> >> ---
> >> lib/librte_lpm/rte_lpm6.c | 10 ++
> >> 1 file changed, 6 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
> >> index ba4353c..f4db3fa 100644
> >> --- a/lib/librte_lpm/rte_lpm6.c
> >> +++ b/lib/librte_lpm/rte_lpm6.c
> >> @@ -749,6 +749,7 @@ rte_lpm6_delete(struct rte_lpm6 *lpm, uint8_t
> >> *ip,
> >uint8_t depth)
> >>int32_t rule_to_delete_index;
> >>uint8_t ip_masked[RTE_LPM6_IPV6_ADDR_SIZE];
> >>unsigned i;
> >> + int status = 0;
> >>
> >>/*
> >> * Check input arguments.
> >> @@ -790,12 +791,13 @@ rte_lpm6_delete(struct rte_lpm6 *lpm, uint8_t
> >*ip, uint8_t depth)
> >> * Add every rule again (except for the one that was removed from
> >> * the rules table).
> >> */
> >> - for (i = 0; i < lpm->used_rules; i++) {
> >> - rte_lpm6_add(lpm, lpm->rules_tbl[i].ip, lpm-
> >>rules_tbl[i].depth,
> >> - lpm->rules_tbl[i].next_hop);
> >> + for (i = 0; i < lpm->used_rules && status >= 0; i++) {
> >> + status = rte_lpm6_add(
> >> + lpm, lpm->rules_tbl[i].ip, lpm->rules_tbl[i].depth,
> >> + lpm->rules_tbl[i].next_hop);
> >>}
> >>
> >> - return 0;
> >> + return status;
> >> }
> >
> >Hi,
> >
> >I'm not sure that this patch is actually necessary, as I'm not sure
> >that the lpm6_add calls can fail in this instance. Looking through the
> >code, this function deletes the rule and then clears the actual lpm
> >lookup tables before re-adding all other routes to it again. The only
> >error condition that could be returned, that I can see, is -ENOSPC,
> >which should never occur here since the original rules fitted in the first
> place.
I agree that -ENOSPC should never occur here. So rte_lpm6_add() instance should
never fail here.
Next I looked at rte_lpm6_add() and if rte_lpm6_delete() instances in it may
fail?
The only suspicious place that I found is place when add every rule again but
that should work as discussed above.
> >
> >If it was possible to fail, then I think we would have a worse problem,
> >in that deleting a single rule has wiped out our lpm table and left it
> >in an inconsistent state, so the error handling probably needs to be better
> than just quitting.
> >
> >Finally, one other thing I spot looking through the code, is that there
> >seems to be a worrying set of calls between add and delete. If the add
> >function fails, then it calls delete which in turn will call add again,
> >etc. etc. This may all work correctly, but it seems fragile and error
> >prone to me - especially if we allow calls from one to another to fail.
> >
> >This looks like it might need some further examination to verify what
> >the possible failure cases are and what happens in each scenario.
I see no failure scenarios in here. I mean I see no possibility to create test
that show that add function fail in del and opposite.
The only scenario what I have in my mind is that someone call add or/and del
functions on different threads with the same lpm table instance, but this is
not allowed, cause we know that this functions are not thread safe.
> >
> >Regards,
> >/Bruce
>
>
> Hi Bruce,
>
> In my opinion the worst-case scenario should be take into account. If
> function like rte_lpm6_add() returns false then it should be handled.
>
> Anyway I agree with you that if the function fail then we have serious
> problem.
> I see two problems:
> 1. Code construction: calls between function rte_lpm6_add() and
> rte_lpm6_delete(). As you said it should be examined.
> 2. How we should handle situation if the rules table are not reconstructed
> after delete operation.
>
> I propose to add new issue in ClearQuest to proceed solve the problems
> because there are extend the original issue (CID 13205 Unchecked return
> value) from Coverity.
>
> Regards,
> S?awomir
I propose to classify this Coverity issues (ID 13205 and ID 13201) as
Intentional.
Regards,
Piotr
[dpdk-dev] [PATCH v2 09/11] enic: optimize the Tx function
Hi,
> uint16_t enic_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
> uint16_t nb_pkts)
> {
> uint16_t index;
> - unsigned int frags;
> - unsigned int pkt_len;
> - unsigned int seg_len;
> - unsigned int inc_len;
> + unsigned int pkt_len, data_len;
> unsigned int nb_segs;
> - struct rte_mbuf *tx_pkt, *next_tx_pkt;
> + struct rte_mbuf *tx_pkt;
> struct vnic_wq *wq = (struct vnic_wq *)tx_queue;
> struct enic *enic = vnic_dev_priv(wq->vdev);
> unsigned short vlan_id;
> unsigned short ol_flags;
Above ol_flags should be uint64_t.
> - uint8_t last_seg, eop;
> - unsigned int host_tx_descs = 0;
> + unsigned int wq_desc_avail;
> + int head_idx;
> + struct vnic_wq_buf *buf;
> + unsigned int hw_ip_cksum_enabled;
> + unsigned int desc_count;
> + struct wq_enet_desc *descs, *desc_p, desc_tmp;
> + uint16_t mss;
> + uint8_t vlan_tag_insert;
> + uint8_t eop;
> + uint64_t bus_addr;
>
> + enic_cleanup_wq(enic, wq);
> + wq_desc_avail = vnic_wq_desc_avail(wq);
> + head_idx = wq->head_idx;
> + desc_count = wq->ring.desc_count;
> +
> + nb_pkts = RTE_MIN(nb_pkts, ENIC_TX_XMIT_MAX);
> +
> + hw_ip_cksum_enabled = enic->hw_ip_checksum;
> for (index = 0; index < nb_pkts; index++) {
> tx_pkt = *tx_pkts++;
> - inc_len = 0;
> nb_segs = tx_pkt->nb_segs;
> - if (nb_segs > vnic_wq_desc_avail(wq)) {
> + if (nb_segs > wq_desc_avail) {
> if (index > 0)
> - enic_post_wq_index(wq);
> -
> - /* wq cleanup and try again */
> - if (!enic_cleanup_wq(enic, wq) ||
> - (nb_segs > vnic_wq_desc_avail(wq))) {
> - return index;
> - }
> + goto post;
> + goto done;
> }
>
> pkt_len = tx_pkt->pkt_len;
> + data_len = tx_pkt->data_len;
> vlan_id = tx_pkt->vlan_tci;
> ol_flags = tx_pkt->ol_flags;
Cause you may miss a lot of flags in here.
Piotr
> - for (frags = 0; inc_len < pkt_len; frags++) {
> - if (!tx_pkt)
> - break;
> - next_tx_pkt = tx_pkt->next;
> - seg_len = tx_pkt->data_len;
> - inc_len += seg_len;
> -
> - host_tx_descs++;
> - last_seg = 0;
> - eop = 0;
> - if ((pkt_len == inc_len) || !next_tx_pkt) {
> - eop = 1;
> - /* post if last packet in batch or > thresh */
> - if ((index == (nb_pkts - 1)) ||
> -(host_tx_descs > ENIC_TX_POST_THRESH))
> {
> - last_seg = 1;
> - host_tx_descs = 0;
> - }
> +
> + mss = 0;
> + vlan_tag_insert = 0;
> + bus_addr = (dma_addr_t)
> +(tx_pkt->buf_physaddr + tx_pkt->data_off);
> +
> + descs = (struct wq_enet_desc *)wq->ring.descs;
> + desc_p = descs + head_idx;
> +
> + eop = (data_len == pkt_len);
> +
> + if (ol_flags & PKT_TX_VLAN_PKT)
> + vlan_tag_insert = 1;
> +
> + if (hw_ip_cksum_enabled && (ol_flags &
> PKT_TX_IP_CKSUM))
> + mss |= ENIC_CALC_IP_CKSUM;
> +
> + if (hw_ip_cksum_enabled && (ol_flags &
> PKT_TX_TCP_UDP_CKSUM))
> + mss |= ENIC_CALC_TCP_UDP_CKSUM;
> +
> + wq_enet_desc_enc(&desc_tmp, bus_addr, data_len, mss, 0,
> 0, eop,
> + eop, 0, vlan_tag_insert, vlan_id, 0);
> +
> + *desc_p = desc_tmp;
> + buf = &wq->bufs[head_idx];
> + buf->mb = (void *)tx_pkt;
> + head_idx = enic_ring_incr(desc_count, head_idx);
> + wq_desc_avail--;
> +
> + if (!eop) {
> + for (tx_pkt = tx_pkt->next; tx_pkt; tx_pkt =
> + tx_pkt->next) {
> + data_len = tx_pkt->data_len;
> +
> + if (tx_pkt->next == NULL)
> + eop = 1;
> + desc_p = descs + head_idx;
> + bus_addr = (dma_addr_t)(tx_pkt-
> >buf_physaddr
> ++ tx_pkt->data_off);
> + wq_enet_desc_enc((struct wq_enet_desc *)
> + &desc_tmp, bus_addr,
> data_len,
> + mss, 0, 0, eop, eop, 0,
> + vlan_tag_insert
