Re: [lng-odp] [PATCHv8] helper : Fix UDP checksum computation
On 04.01.2016 15:48, ion.grig...@freescale.com wrote: > From: Grigore Ion <ion.grig...@freescale.com> > > This patch fixes the following problems: > - checksum computation for LE platforms > - checksum computation for packets having the UDP length not a > multiple of 2 > - checksum computation in the test and the example applications > > Signed-off-by: Grigore Ion <ion.grig...@freescale.com> > --- > v8: > - Checksum in BE endianness (Ilya Maximets) > v7: > - Make code simpler and more understandable as it was > sugessted by Ilya Maximets > v6: > - Make code more understandable. Not done as it was > sugessted by Ilya Maximets. > v5: > - Checksum in CPU endianness fix added (Ilya Maximets) > v4: > - Verify checksum in CPU endianness in the associated test > (Ilya Maximets) > v3: > - fix the UDP checksum computation in the associated test > (Maxim Uvarov) > v2: > - patch updated to the last master (Maxim Uvarov) > v1: > - Move variables declaration on top of block. (Maxim Uvarov) > - Check patch with checkpatch script. (Maxim Uvarov) > - L3 header presence is tested twice. (Alexandru Badicioiu) > - Remove unnecessary check for L3 header presence. (Bill Fischofer) > > example/generator/odp_generator.c | 2 +- > helper/include/odp/helper/udp.h | 65 > --- > helper/test/odp_chksum.c | 4 +-- > test/validation/pktio/pktio.c | 2 +- > 4 files changed, 37 insertions(+), 36 deletions(-) > > diff --git a/example/generator/odp_generator.c > b/example/generator/odp_generator.c > index cdbc761..757dc54 100644 > --- a/example/generator/odp_generator.c > +++ b/example/generator/odp_generator.c > @@ -242,7 +242,7 @@ static odp_packet_t pack_udp_pkt(odp_pool_t pool) > udp->dst_port = 0; > udp->length = odp_cpu_to_be_16(args->appl.payload + ODPH_UDPHDR_LEN); > udp->chksum = 0; > - udp->chksum = odp_cpu_to_be_16(odph_ipv4_udp_chksum(pkt)); > + udp->chksum = odph_ipv4_udp_chksum(pkt); > > return pkt; > } > diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h > index 06c439b..afdb4fc 100644 > --- a/helper/include/odp/helper/udp.h > +++ b/helper/include/odp/helper/udp.h > @@ -4,7 +4,6 @@ > * SPDX-License-Identifier: BSD-3-Clause > */ > > - > /** > * @file > * > @@ -22,7 +21,6 @@ extern "C" { > #include > #include > > - > /** @addtogroup odph_header ODPH HEADER > * @{ > */ > @@ -44,46 +42,49 @@ typedef struct ODP_PACKED { > * This function uses odp packet to calc checksum > * > * @param pkt calculate chksum for pkt > - * @return checksum value > + * @return checksum value in BE endianness > */ > static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) > { > - uint32_t sum = 0; > - odph_udphdr_t *udph; > - odph_ipv4hdr_t *iph; > - uint16_t udplen; > - uint8_t *buf; > - > - if (!odp_packet_l3_offset(pkt)) > + odph_ipv4hdr_t *iph; > + odph_udphdr_t *udph; > + uint32_tsum; > + uint16_tudplen, *buf; > + union { > + uint8_t v8[2]; > + uint16_t v16; > + } val; > + > + if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID) > return 0; > - > - if (!odp_packet_l4_offset(pkt)) > - return 0; > - > iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL); > udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL); > - udplen = odp_be_to_cpu_16(udph->length); > - > - /* 32-bit sum of all 16-bit words covered by UDP chksum */ > + /* 32-bit sum of UDP pseudo-header */ I think, it should be specified, that it's a sum of 16-bit words. > sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) + > - (iph->dst_addr & 0x) + (iph->dst_addr >> 16) + > - (uint16_t)iph->proto + udplen; > - for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) { > - sum += ((*buf << 8) + *(buf + 1)); > - buf += 2; > + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) + > + udph->length; > + val.v8[0] = 0; > + val.v8[1] = iph->proto; > + sum += val.v16; > + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */ Same here. > + udplen = odp_be_to_cpu_16(udph->length); > + buf = (uint16_t *)((void *)udph); > + for ( ; udplen > 1; udplen -= 2) > + sum += *buf++; > + /* Length is not a multiple of 2 bytes */ >
Re: [lng-odp] [PATCHv7] helper : Fix UDP checksum computation
This version almost good. 2 more comments: * no need to remove blank lines at the beginning of the file. * Why checksum is computed in the CPU endianness? There is no reason to do so. I can't imagine case where checksum will be needed in CPU endiannes. IMO, it's better not to convert it on return: + /* Return checksum in BE endianness */ + return (sum == 0x0) ? 0x : sum; And remove conversion to BE in all other files: $ git grep odph_ipv4_udp_chksum example/generator/odp_generator.c: udp->chksum = odp_cpu_to_be_16(odph_ipv4_udp_chksum(pkt)); helper/include/odp/helper/udp.h:static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) helper/test/odp_chksum.c: udp->chksum = odp_cpu_to_be_16(odph_ipv4_udp_chksum(test_packet)); And modify only checksum value in helper/test/odp_chksum.c. Best regards, Ilya Maximets. On 30.12.2015 18:15, Maxim Uvarov wrote: > Ilya is that version good for you? Do you want to add your Review-by? > > Maxim. > > On 12/17/2015 19:36, Ion Grigore wrote: >> Ping >> >> -Original Message- >> From: ion.grig...@freescale.com [mailto:ion.grig...@freescale.com] >> Sent: Monday, December 14, 2015 12:52 PM >> To: lng-odp@lists.linaro.org >> Cc: Grigore Ion-B17953 <ion.grig...@freescale.com> >> Subject: [PATCHv7] helper : Fix UDP checksum computation >> >> From: Grigore Ion <ion.grig...@freescale.com> >> >> This patch fixes the following problems: >> - checksum computation for LE platforms >> - checksum is computed in the CPU endianness. The returned result must be >> converted to the BE ordering when it is used to update the UDP checksum in a >> packet. >> - checksum computation for packets having the UDP length not a multiple of 2 >> - fixes the UDP checksum associated validation test >> >> Signed-off-by: Grigore Ion <ion.grig...@freescale.com> >> --- >> v7: >> - Make code simpler and more understandable as it was sugessted by Ilya >> Maximets >> v6: >> - Make code more understandable. Not done as it was sugessted by Ilya >> Maximets. >> v5: >> - Checksum in CPU endianness fix added (Ilya Maximets) >> v4: >> - Verify checksum in CPU endianness in the associated test (Ilya Maximets) >> v3: >> - fix the UDP checksum computation in the associated test (Maxim Uvarov) >> v2: >> - patch updated to the last master (Maxim Uvarov) >> v1: >> - Move variables declaration on top of block. (Maxim Uvarov) >> - Check patch with checkpatch script. (Maxim Uvarov) >> - L3 header presence is tested twice. (Alexandru Badicioiu) >> - Remove unnecessary check for L3 header presence. (Bill Fischofer) >> - Modify check of odp_packet_l4_offset() return. (Bill Fischofer) >> >> helper/include/odp/helper/udp.h | 65 >> + >> helper/test/odp_chksum.c| 4 +-- >> 2 files changed, 35 insertions(+), 34 deletions(-) >> >> diff --git a/helper/include/odp/helper/udp.h >> b/helper/include/odp/helper/udp.h index 06c439b..f43fa54 100644 >> --- a/helper/include/odp/helper/udp.h >> +++ b/helper/include/odp/helper/udp.h >> @@ -4,7 +4,6 @@ >>* SPDX-License-Identifier: BSD-3-Clause >>*/ >> - >> /** >>* @file >>* >> @@ -22,7 +21,6 @@ extern "C" { >> #include >> #include >> - >> /** @addtogroup odph_header ODPH HEADER >>* @{ >>*/ >> @@ -44,46 +42,49 @@ typedef struct ODP_PACKED { >>* This function uses odp packet to calc checksum >>* >>* @param pkt calculate chksum for pkt >> - * @return checksum value >> + * @return checksum value in CPU endianness >>*/ >> static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) { >> -uint32_t sum = 0; >> -odph_udphdr_t *udph; >> -odph_ipv4hdr_t *iph; >> -uint16_t udplen; >> -uint8_t *buf; >> - >> -if (!odp_packet_l3_offset(pkt)) >> +odph_ipv4hdr_t*iph; >> +odph_udphdr_t*udph; >> +uint32_tsum; >> +uint16_tudplen, *buf; >> +union { >> +uint8_t v8[2]; >> +uint16_t v16; >> +} val; >> + >> +if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID) >> return 0; >> - >> -if (!odp_packet_l4_offset(pkt)) >> -return 0; >> - >> iph = (odph_ipv4hdr_t *)odp_pa
Re: [lng-odp] [PATCH] linux-generic: fix tap compilation
Thanks. Reviewed-by: Ilya Maximets <i.maxim...@samsung.com> On 14.12.2015 18:26, Maxim Uvarov wrote: > pktio/tap.c:267:21: error: comparison between signed and > unsigned integer expressions [-Werror=sign-compare] >} else if (retval != pkt_len) { > > Cc: Ilya Maximets <i.maxim...@samsung.com> > Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org> > --- > platform/linux-generic/pktio/tap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/platform/linux-generic/pktio/tap.c > b/platform/linux-generic/pktio/tap.c > index 7ecb300..99e15ce 100644 > --- a/platform/linux-generic/pktio/tap.c > +++ b/platform/linux-generic/pktio/tap.c > @@ -264,7 +264,7 @@ static int tap_pktio_send(pktio_entry_t *pktio_entry, > odp_packet_t pkts[], > return -1; > } > break; > - } else if (retval != pkt_len) { > + } else if ((uint32_t)retval != pkt_len) { > ODP_ERR("sent partial ethernet packet\n"); > if (i == 0) { > __odp_errno = EMSGSIZE; > ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCHv5 0/4] TAP pktio
On 11.12.2015 19:51, Stuart Haslam wrote: > On Thu, Dec 10, 2015 at 06:32:00PM +0300, Ilya Maximets wrote: >> Creates a new pktio type that allows for creating and >> sending/receiving packets through TAP interface. >> Detailed description in commit-message of patch >> "[PATCHv5 3/4] linux-generic: pktio: add tap pktio type". >> >> Changelog: >> >> Version 5: >> * nothing changed. New patch added to add ability >>to wait some time right after pktio_open() to >>be sure that tap interface switched to enabled state >>inside the bridge. Fixes occasional first test failures. >>ODP_WAIT_FOR_NETWORK used to run tests. >> * rebased on current master >> >> Version 4: >> * changed error handling part in tap_pktio_send. (Stuart Haslam) >> >> Version 3: >> * return 77 (TEST_SKIPPED) if user is not root. (Stuart Haslam) >> >> Version 2: >> * Validation tests added >> * Pktio tests fixed to work with real-world >>interfaces. >> * MAC of pktio now is not a kernel interface's MAC >> * Interfaces are UP after pktio_open() >> * Fixed getting mtu, getting/setting promisc mode >> * Misclenious fixes >> >> Ilya Maximets (4): >> validation: pktio: initialize mac addresses for all packets >> validation: pktio: ability to wait for external network >> linux-generic: pktio: add tap pktio type >> linux-generic: pktio: add test for tap pktio > > For the series: > > Reviewed-and-Tested-by: Stuart Haslam <stuart.has...@linaro.org> Thanks. > >> >> platform/linux-generic/Makefile.am | 2 + >> .../linux-generic/include/odp_packet_io_internal.h | 3 + >> platform/linux-generic/include/odp_packet_tap.h| 21 ++ >> platform/linux-generic/pktio/io_ops.c | 1 + >> platform/linux-generic/pktio/tap.c | 327 >> + >> platform/linux-generic/test/Makefile.am| 1 + >> platform/linux-generic/test/pktio/Makefile.am | 3 +- >> platform/linux-generic/test/pktio/pktio_run_tap| 115 >> test/validation/pktio/pktio.c | 72 - >> 9 files changed, 536 insertions(+), 9 deletions(-) >> create mode 100644 platform/linux-generic/include/odp_packet_tap.h >> create mode 100644 platform/linux-generic/pktio/tap.c >> create mode 100755 platform/linux-generic/test/pktio/pktio_run_tap >> >> -- >> 2.1.4 >> > > ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation
On 11.12.2015 12:19, Maxim Uvarov wrote: > On 12/11/2015 10:39, Ilya Maximets wrote: >> Comments inlined. >> >> On 10.12.2015 12:59, ion.grig...@freescale.com wrote: >>> From: Grigore Ion <ion.grig...@freescale.com> >>> >>> This patch fixes the following problems: >>> - checksum computation for LE platforms >>> - checksum is computed in the CPU endianness. The returned result >>> must be converted to the BE ordering when it is used to update >>> the UDP checksum in a packet. >>> - checksum computation for packets having the UDP length not a >>> multiple of 2 >>> - fixes the UDP checksum associated validation test >>> >>> Signed-off-by: Grigore Ion <ion.grig...@freescale.com> >>> --- >>> v6: >>> - Make code more understandable (Ilya Maximets) >>> v5: >>> - Checksum in CPU endianness fix added (Ilya Maximets) >>> v4: >>> - Verify checksum in CPU endianness in the associated test >>> (Ilya Maximets) >>> v3: >>> - fix the UDP checksum computation in the associated test >>> (Maxim Uvarov) >>> v2: >>> - patch updated to the last master (Maxim Uvarov) >>> v1: >>> - Move variables declaration on top of block. (Maxim Uvarov) >>> - Check patch with checkpatch script. (Maxim Uvarov) >>> - L3 header presence is tested twice. (Alexandru Badicioiu) >>> - Remove unnecessary check for L3 header presence. (Bill Fischofer) >>> - Modify check of odp_packet_l4_offset() return. (Bill Fischofer) >>> >>> helper/include/odp/helper/udp.h | 81 >>> + >>> helper/test/odp_chksum.c| 4 +- >>> 2 files changed, 51 insertions(+), 34 deletions(-) >>> >>> diff --git a/helper/include/odp/helper/udp.h >>> b/helper/include/odp/helper/udp.h >>> index 06c439b..d0599b1 100644 >>> --- a/helper/include/odp/helper/udp.h >>> +++ b/helper/include/odp/helper/udp.h >>> @@ -4,7 +4,6 @@ >>>* SPDX-License-Identifier: BSD-3-Clause >>>*/ >>> - >>> /** >>>* @file >>>* >>> @@ -22,7 +21,6 @@ extern "C" { >>> #include >>> #include >>> - >>> /** @addtogroup odph_header ODPH HEADER >>>* @{ >>>*/ >>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED { >>> } odph_udphdr_t; >>> /** >>> + * Perform byte swap required by UDP checksum computation algorithm >>> + */ >>> +static inline uint16_t csum_bswap(uint16_t val) >>> +{ >>> +#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN >>> +val = __odp_builtin_bswap16((__odp_force uint16_t)val); >>> +#endif >>> +return val; >>> +} >> Please, don't duplicate the code. >> >>> + >>> +/** >>> + * Pad a byte value to the left or to the right as required by UDP checksum >>> + * computation algorithm and convert the result to CPU native uint16_t. >>> + * Left padding is performed for the IP protocol field in the UDP >>> + * pseudo-header (RFC 768). Right padding is performed in the case of the >>> odd >>> + * byte in a UDP packet having the length not a 2 multiple. >>> + */ >>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) >>> +{ >>> +uint16_tret; >>> + >>> +ret = (left) ? val : val << 8; >>> +return csum_bswap(ret); >>> +} >>> + >>> +/** >>>* UDP checksum >>>* >>>* This function uses odp packet to calc checksum >>>* >>>* @param pkt calculate chksum for pkt >>> - * @return checksum value >>> + * @return checksum value in CPU endianness >>>*/ >>> static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) >>> { >>> -uint32_t sum = 0; >>> -odph_udphdr_t *udph; >>> -odph_ipv4hdr_t *iph; >>> -uint16_t udplen; >>> -uint8_t *buf; >>> - >>> -if (!odp_packet_l3_offset(pkt)) >>> -return 0; >>> +odph_ipv4hdr_t*iph; >>> +odph_udphdr_t*udph; >>> +uint32_tsum; >>> +uint16_tudplen, *buf; >>> -if (!odp_packet_l4_offset(pkt)) >>> +if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID) >>> return 0; >>> -
Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation
On 11.12.2015 18:28, Ion Grigore wrote: > > Comments inlined > > -Original Message----- > From: Ilya Maximets [mailto:i.maxim...@samsung.com] > Sent: Friday, December 11, 2015 5:24 PM > To: Grigore Ion-B17953 <ion.grig...@freescale.com>; Maxim Uvarov > <maxim.uva...@linaro.org>; lng-odp@lists.linaro.org > Subject: Re: [PATCHv6] helper : Fix UDP checksum computation > > On 11.12.2015 17:07, Ion Grigore wrote: >> Comments inlined >> >> -Original Message- >> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org] >> Sent: Friday, December 11, 2015 11:20 AM >> To: Ilya Maximets <i.maxim...@samsung.com>; Grigore Ion-B17953 >> <ion.grig...@freescale.com>; lng-odp@lists.linaro.org >> Subject: Re: [PATCHv6] helper : Fix UDP checksum computation >> >> On 12/11/2015 10:39, Ilya Maximets wrote: >>> Comments inlined. >>> >>> On 10.12.2015 12:59, ion.grig...@freescale.com wrote: >>>> From: Grigore Ion <ion.grig...@freescale.com> >>>> >>>> This patch fixes the following problems: >>>> - checksum computation for LE platforms >>>> - checksum is computed in the CPU endianness. The returned result >>>> must be converted to the BE ordering when it is used to update the >>>> UDP checksum in a packet. >>>> - checksum computation for packets having the UDP length not a >>>> multiple of 2 >>>> - fixes the UDP checksum associated validation test >>>> >>>> Signed-off-by: Grigore Ion <ion.grig...@freescale.com> >>>> --- >>>> v6: >>>> - Make code more understandable (Ilya Maximets) >>>> v5: >>>> - Checksum in CPU endianness fix added (Ilya Maximets) >>>> v4: >>>> - Verify checksum in CPU endianness in the associated test >>>> (Ilya Maximets) >>>> v3: >>>> - fix the UDP checksum computation in the associated test >>>> (Maxim Uvarov) >>>> v2: >>>> - patch updated to the last master (Maxim Uvarov) >>>> v1: >>>> - Move variables declaration on top of block. (Maxim Uvarov) >>>> - Check patch with checkpatch script. (Maxim Uvarov) >>>> - L3 header presence is tested twice. (Alexandru Badicioiu) >>>> - Remove unnecessary check for L3 header presence. (Bill Fischofer) >>>> - Modify check of odp_packet_l4_offset() return. (Bill Fischofer) >>>> >>>> helper/include/odp/helper/udp.h | 81 >>>> + >>>> helper/test/odp_chksum.c| 4 +- >>>> 2 files changed, 51 insertions(+), 34 deletions(-) >>>> >>>> diff --git a/helper/include/odp/helper/udp.h >>>> b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644 >>>> --- a/helper/include/odp/helper/udp.h >>>> +++ b/helper/include/odp/helper/udp.h >>>> @@ -4,7 +4,6 @@ >>>>* SPDX-License-Identifier: BSD-3-Clause >>>>*/ >>>> >>>> - >>>> /** >>>>* @file >>>>* >>>> @@ -22,7 +21,6 @@ extern "C" { >>>> #include >>>> #include >>>> >>>> - >>>> /** @addtogroup odph_header ODPH HEADER >>>>* @{ >>>>*/ >>>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED { >>>> } odph_udphdr_t; >>>> >>>> /** >>>> + * Perform byte swap required by UDP checksum computation algorithm >>>> +*/ static inline uint16_t csum_bswap(uint16_t val) { #if >>>> +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN >>>> + val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif >>>> + return val; >>>> +} >>> Please, don't duplicate the code. >>> >>>> + >>>> +/** >>>> + * Pad a byte value to the left or to the right as required by UDP >>>> +checksum >>>> + * computation algorithm and convert the result to CPU native uint16_t. >>>> + * Left padding is performed for the IP protocol field in the UDP >>>> + * pseudo-header (RFC 768). Right padding is performed in the case >>>> +of the odd >>>> + * byte in a UDP packet having the length not a 2 multiple. >>>> + */ >>>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) { >>>> + uint16_tret; >>>>
Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation
On 11.12.2015 18:47, Ion Grigore wrote: > A more precise question. Did you check on a LE platform ? Yes. On my LE i7. > - /* Fold sum to 16 bits: add carrier to result */ > - while (sum >> 16) > - sum = (sum & 0x) + (sum >> 16); > - > + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) + > + u8_pad_to_u16(iph->proto, 1) + udph->length; I meant something like: union { uint8_t v8[2]; uint16_t v16; } val; val.v8[0] = 0; val.v8[1] = iph->proto; >>> >>> The val.val16 must be swapped on LE platforms. >> >> NO. >> >> Did you checked this ? > > And you? > >> >>> >>> sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) + val.v16 + udph->length; > + udplen = odp_be_to_cpu_16(udph->length); > + buf = (uint16_t *)((void *)udph); > + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */ > + for ( ; udplen > 1; udplen -= 2) > + sum += *buf++; > + /* Length is not a multiple of 2 bytes */ > + if (udplen) > + sum += u8_pad_to_u16(*buf, 0); val.v8[0] = *buf; val.v8[1] = 0; sum += val.v16; Isn't it more understandable, then artificial byte swapping? >>> >>> The val.val16 must be swapped on LE platforms. >> >> NO. >> >> Did you checked this ? > > And you? > > P.S. Just to be honest, I checked this a few minutes ago. > Helper test below works. > > Yes on BE/LE platforms, but not with the change you proposed. So why you talking that val.val16 must be swapped if you do not understand how it works and you have not tested it? ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation
On 11.12.2015 17:07, Ion Grigore wrote: > Comments inlined > > -Original Message- > From: Maxim Uvarov [mailto:maxim.uva...@linaro.org] > Sent: Friday, December 11, 2015 11:20 AM > To: Ilya Maximets <i.maxim...@samsung.com>; Grigore Ion-B17953 > <ion.grig...@freescale.com>; lng-odp@lists.linaro.org > Subject: Re: [PATCHv6] helper : Fix UDP checksum computation > > On 12/11/2015 10:39, Ilya Maximets wrote: >> Comments inlined. >> >> On 10.12.2015 12:59, ion.grig...@freescale.com wrote: >>> From: Grigore Ion <ion.grig...@freescale.com> >>> >>> This patch fixes the following problems: >>> - checksum computation for LE platforms >>> - checksum is computed in the CPU endianness. The returned result >>> must be converted to the BE ordering when it is used to update the >>> UDP checksum in a packet. >>> - checksum computation for packets having the UDP length not a >>> multiple of 2 >>> - fixes the UDP checksum associated validation test >>> >>> Signed-off-by: Grigore Ion <ion.grig...@freescale.com> >>> --- >>> v6: >>> - Make code more understandable (Ilya Maximets) >>> v5: >>> - Checksum in CPU endianness fix added (Ilya Maximets) >>> v4: >>> - Verify checksum in CPU endianness in the associated test >>> (Ilya Maximets) >>> v3: >>> - fix the UDP checksum computation in the associated test >>> (Maxim Uvarov) >>> v2: >>> - patch updated to the last master (Maxim Uvarov) >>> v1: >>> - Move variables declaration on top of block. (Maxim Uvarov) >>> - Check patch with checkpatch script. (Maxim Uvarov) >>> - L3 header presence is tested twice. (Alexandru Badicioiu) >>> - Remove unnecessary check for L3 header presence. (Bill Fischofer) >>> - Modify check of odp_packet_l4_offset() return. (Bill Fischofer) >>> >>> helper/include/odp/helper/udp.h | 81 >>> + >>> helper/test/odp_chksum.c| 4 +- >>> 2 files changed, 51 insertions(+), 34 deletions(-) >>> >>> diff --git a/helper/include/odp/helper/udp.h >>> b/helper/include/odp/helper/udp.h index 06c439b..d0599b1 100644 >>> --- a/helper/include/odp/helper/udp.h >>> +++ b/helper/include/odp/helper/udp.h >>> @@ -4,7 +4,6 @@ >>>* SPDX-License-Identifier: BSD-3-Clause >>>*/ >>> >>> - >>> /** >>>* @file >>>* >>> @@ -22,7 +21,6 @@ extern "C" { >>> #include >>> #include >>> >>> - >>> /** @addtogroup odph_header ODPH HEADER >>>* @{ >>>*/ >>> @@ -39,51 +37,70 @@ typedef struct ODP_PACKED { >>> } odph_udphdr_t; >>> >>> /** >>> + * Perform byte swap required by UDP checksum computation algorithm >>> +*/ static inline uint16_t csum_bswap(uint16_t val) { #if >>> +ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN >>> + val = __odp_builtin_bswap16((__odp_force uint16_t)val); #endif >>> + return val; >>> +} >> Please, don't duplicate the code. >> >>> + >>> +/** >>> + * Pad a byte value to the left or to the right as required by UDP >>> +checksum >>> + * computation algorithm and convert the result to CPU native uint16_t. >>> + * Left padding is performed for the IP protocol field in the UDP >>> + * pseudo-header (RFC 768). Right padding is performed in the case >>> +of the odd >>> + * byte in a UDP packet having the length not a 2 multiple. >>> + */ >>> +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) { >>> + uint16_tret; >>> + >>> + ret = (left) ? val : val << 8; >>> + return csum_bswap(ret); >>> +} >>> + >>> +/** >>>* UDP checksum >>>* >>>* This function uses odp packet to calc checksum >>>* >>>* @param pkt calculate chksum for pkt >>> - * @return checksum value >>> + * @return checksum value in CPU endianness >>>*/ >>> static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) >>> { >>> - uint32_t sum = 0; >>> - odph_udphdr_t *udph; >>> - odph_ipv4hdr_t *iph; >>> - uint16_t udplen; >>> - uint8_t *buf; >>> - >>> - if (!odp_packet_l3_offset(pkt)) >>> -
[lng-odp] [BUG] linux-generic: timer: race for odp_timer_pool
Timer pool may be freed while another thread executes timer_notify(). This leads to segmentation fault. Example scenario: Time main_thread timer_notify_thread 1call odp_timer_pool_del(tp) call timer_notify(sigval) 2odp_lock(>lock);tp = sigval.sival_ptr; 3... ... 4odp_lock(>itimer_running); ... 5itimer_fini(tp);timer_getoverrun(tp->timerid); 6odp_shm_free(tp->shm); ... 7... odp_atomic_fetch_inc_u64(>cur_tick); Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fffedffb700 (LWP 1744)] in odp_atomic_fetch_inc_u64 (atom=...) at ./include/odp/atomic.h:158 158 return __atomic_fetch_add(>v, 1, __ATOMIC_RELAXED); (gdb) bt #0 odp_atomic_fetch_inc_u64 (atom=...) at ./include/odp/atomic.h:158 #1 timer_notify (sigval=..., sigval@entry=...) at odp_timer.c:650 #2 timer_sigev_thread (arg=...) at ../unix/sysv/linux/timer_routines.c:62 #3 start_thread (arg=0x7fffedffb700) at pthread_create.c:333 #4 clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109 Segmentation fault appears approximately 1 time from 200 runs of ./test/validation/timer/timer_main on my system. Hint: There is no way to protect memory from feeing by lock located inside that memory. I have no time to fix that. Tag below may be added to patch that fixes that. Reported-by: Ilya Maximets <i.maxim...@samsung.com> ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCHv6] helper : Fix UDP checksum computation
Comments inlined. On 10.12.2015 12:59, ion.grig...@freescale.com wrote: > From: Grigore Ion <ion.grig...@freescale.com> > > This patch fixes the following problems: > - checksum computation for LE platforms > - checksum is computed in the CPU endianness. The returned result > must be converted to the BE ordering when it is used to update > the UDP checksum in a packet. > - checksum computation for packets having the UDP length not a > multiple of 2 > - fixes the UDP checksum associated validation test > > Signed-off-by: Grigore Ion <ion.grig...@freescale.com> > --- > v6: > - Make code more understandable (Ilya Maximets) > v5: > - Checksum in CPU endianness fix added (Ilya Maximets) > v4: > - Verify checksum in CPU endianness in the associated test > (Ilya Maximets) > v3: > - fix the UDP checksum computation in the associated test > (Maxim Uvarov) > v2: > - patch updated to the last master (Maxim Uvarov) > v1: > - Move variables declaration on top of block. (Maxim Uvarov) > - Check patch with checkpatch script. (Maxim Uvarov) > - L3 header presence is tested twice. (Alexandru Badicioiu) > - Remove unnecessary check for L3 header presence. (Bill Fischofer) > - Modify check of odp_packet_l4_offset() return. (Bill Fischofer) > > helper/include/odp/helper/udp.h | 81 > + > helper/test/odp_chksum.c| 4 +- > 2 files changed, 51 insertions(+), 34 deletions(-) > > diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h > index 06c439b..d0599b1 100644 > --- a/helper/include/odp/helper/udp.h > +++ b/helper/include/odp/helper/udp.h > @@ -4,7 +4,6 @@ > * SPDX-License-Identifier: BSD-3-Clause > */ > > - > /** > * @file > * > @@ -22,7 +21,6 @@ extern "C" { > #include > #include > > - > /** @addtogroup odph_header ODPH HEADER > * @{ > */ > @@ -39,51 +37,70 @@ typedef struct ODP_PACKED { > } odph_udphdr_t; > > /** > + * Perform byte swap required by UDP checksum computation algorithm > + */ > +static inline uint16_t csum_bswap(uint16_t val) > +{ > +#if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN > + val = __odp_builtin_bswap16((__odp_force uint16_t)val); > +#endif > + return val; > +} Please, don't duplicate the code. > + > +/** > + * Pad a byte value to the left or to the right as required by UDP checksum > + * computation algorithm and convert the result to CPU native uint16_t. > + * Left padding is performed for the IP protocol field in the UDP > + * pseudo-header (RFC 768). Right padding is performed in the case of the odd > + * byte in a UDP packet having the length not a 2 multiple. > + */ > +static inline uint16_t u8_pad_to_u16(uint8_t val, odp_bool_t left) > +{ > + uint16_tret; > + > + ret = (left) ? val : val << 8; > + return csum_bswap(ret); > +} > + > +/** > * UDP checksum > * > * This function uses odp packet to calc checksum > * > * @param pkt calculate chksum for pkt > - * @return checksum value > + * @return checksum value in CPU endianness > */ > static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) > { > - uint32_t sum = 0; > - odph_udphdr_t *udph; > - odph_ipv4hdr_t *iph; > - uint16_t udplen; > - uint8_t *buf; > - > - if (!odp_packet_l3_offset(pkt)) > - return 0; > + odph_ipv4hdr_t *iph; > + odph_udphdr_t *udph; > + uint32_tsum; > + uint16_tudplen, *buf; > > - if (!odp_packet_l4_offset(pkt)) > + if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID) > return 0; > - > iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL); > udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL); > - udplen = odp_be_to_cpu_16(udph->length); > - > - /* 32-bit sum of all 16-bit words covered by UDP chksum */ > + /* 32-bit sum of UDP pseudo-header */ > sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) + > - (iph->dst_addr & 0x) + (iph->dst_addr >> 16) + > - (uint16_t)iph->proto + udplen; > - for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) { > - sum += ((*buf << 8) + *(buf + 1)); > - buf += 2; > - } > - > - /* Fold sum to 16 bits: add carrier to result */ > - while (sum >> 16) > - sum = (sum & 0x) + (sum >> 16); > - > + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) + > + u8_pad_to_u16(i
[lng-odp] [PATCHv5 1/4] validation: pktio: initialize mac addresses for all packets
For the purpose of testing of real-world interfaces the packet's content should be valid or kernel will throw them away. Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- test/validation/pktio/pktio.c | 46 --- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index 52e5414..53633dd 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -86,16 +86,16 @@ static void set_pool_len(odp_pool_param_t *params) } static void pktio_pkt_set_macs(odp_packet_t pkt, - pktio_info_t *src, pktio_info_t *dst) + odp_pktio_t src, odp_pktio_t dst) { uint32_t len; odph_ethhdr_t *eth = (odph_ethhdr_t *)odp_packet_l2_ptr(pkt, ); int ret; - ret = odp_pktio_mac_addr(src->id, >src, sizeof(eth->src)); + ret = odp_pktio_mac_addr(src, >src, sizeof(eth->src)); CU_ASSERT(ret == ODPH_ETHADDR_LEN); - ret = odp_pktio_mac_addr(dst->id, >dst, sizeof(eth->dst)); + ret = odp_pktio_mac_addr(dst, >dst, sizeof(eth->dst)); CU_ASSERT(ret == ODPH_ETHADDR_LEN); } @@ -412,12 +412,16 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, pktio_info_t *pktio_b, break; tx_seq[i] = pktio_init_packet(tx_pkt[i]); - if (tx_seq[i] == TEST_SEQ_INVALID) + if (tx_seq[i] == TEST_SEQ_INVALID) { + odp_packet_free(tx_pkt[i]); break; + } - pktio_pkt_set_macs(tx_pkt[i], pktio_a, pktio_b); - if (pktio_fixup_checksums(tx_pkt[i]) != 0) + pktio_pkt_set_macs(tx_pkt[i], pktio_a->id, pktio_b->id); + if (pktio_fixup_checksums(tx_pkt[i]) != 0) { + odp_packet_free(tx_pkt[i]); break; + } tx_ev[i] = odp_packet_to_event(tx_pkt[i]); } @@ -740,6 +744,13 @@ static void pktio_test_start_stop(void) if (pkt == ODP_PACKET_INVALID) break; pktio_init_packet(pkt); + + pktio_pkt_set_macs(pkt, pktio[0], pktio[1]); + if (pktio_fixup_checksums(pkt) != 0) { + odp_packet_free(pkt); + break; + } + tx_ev[alloc] = odp_packet_to_event(pkt); } @@ -787,6 +798,13 @@ static void pktio_test_start_stop(void) if (pkt == ODP_PACKET_INVALID) break; pktio_init_packet(pkt); + if (num_ifaces > 1) { + pktio_pkt_set_macs(pkt, pktio[0], pktio[1]); + if (pktio_fixup_checksums(pkt) != 0) { + odp_packet_free(pkt); + break; + } + } tx_ev[alloc] = odp_packet_to_event(pkt); } @@ -911,8 +929,17 @@ static void pktio_test_send_failure(void) break; pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); - if (pkt_seq[i] == TEST_SEQ_INVALID) + + pktio_pkt_set_macs(pkt_tbl[i], pktio_tx, pktio_rx); + if (pktio_fixup_checksums(pkt_tbl[i]) != 0) { + odp_packet_free(pkt_tbl[i]); + break; + } + + if (pkt_seq[i] == TEST_SEQ_INVALID) { + odp_packet_free(pkt_tbl[i]); break; + } } alloc_pkts = i; @@ -956,6 +983,11 @@ static void pktio_test_send_failure(void) odp_packet_len(pkt_tbl[i]) - PKT_LEN_NORMAL); pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); + + pktio_pkt_set_macs(pkt_tbl[i], pktio_tx, pktio_rx); + ret = pktio_fixup_checksums(pkt_tbl[i]); + CU_ASSERT_FATAL(ret == 0); + CU_ASSERT_FATAL(pkt_seq[i] != TEST_SEQ_INVALID); ret = odp_pktio_send(pktio_tx, _tbl[i], TX_BATCH_LEN - i); CU_ASSERT_FATAL(ret == (TX_BATCH_LEN - i)); -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv5 3/4] linux-generic: pktio: add tap pktio type
Creates a new pktio type that allows for creating and sending/receiving packets through TAP interface. It is intended for use as a simple conventional communication method between applications that use kernel network stack (ping, ssh, iperf, etc.) and ODP applications for the purpose of functional testing and can be used as it is with some of the existing example applications. To use this interface the name passed to odp_pktio_open() must begin with "tap:" and be in the format: tap:iface iface the name of TAP device to be created. TUN/TAP kernel module should be loaded to use this pktio. There should be no device named 'iface' in the system. The total length of the 'iface' is limited by IF_NAMESIZE. Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- platform/linux-generic/Makefile.am | 2 + .../linux-generic/include/odp_packet_io_internal.h | 3 + platform/linux-generic/include/odp_packet_tap.h| 21 ++ platform/linux-generic/pktio/io_ops.c | 1 + platform/linux-generic/pktio/tap.c | 327 + 5 files changed, 354 insertions(+) create mode 100644 platform/linux-generic/include/odp_packet_tap.h create mode 100644 platform/linux-generic/pktio/tap.c diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index 70bd8fe..4639ebc 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -92,6 +92,7 @@ noinst_HEADERS = \ ${srcdir}/include/odp_packet_io_queue.h \ ${srcdir}/include/odp_packet_netmap.h \ ${srcdir}/include/odp_packet_socket.h \ + ${srcdir}/include/odp_packet_tap.h \ ${srcdir}/include/odp_pool_internal.h \ ${srcdir}/include/odp_queue_internal.h \ ${srcdir}/include/odp_schedule_internal.h \ @@ -120,6 +121,7 @@ __LIB__libodp_la_SOURCES = \ pktio/netmap.c \ pktio/socket.c \ pktio/socket_mmap.c \ + pktio/tap.c \ odp_pool.c \ odp_queue.c \ odp_rwlock.c \ diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h index 1a1118c..de29557 100644 --- a/platform/linux-generic/include/odp_packet_io_internal.h +++ b/platform/linux-generic/include/odp_packet_io_internal.h @@ -22,6 +22,7 @@ extern "C" { #include #include #include +#include #include #include #include @@ -78,6 +79,7 @@ struct pktio_entry { #ifdef HAVE_PCAP pkt_pcap_t pkt_pcap;/**< Using pcap for IO */ #endif + pkt_tap_t pkt_tap; /**< using TAP for IO */ }; enum { STATE_START = 0, @@ -157,6 +159,7 @@ extern const pktio_if_ops_t loopback_pktio_ops; #ifdef HAVE_PCAP extern const pktio_if_ops_t pcap_pktio_ops; #endif +extern const pktio_if_ops_t tap_pktio_ops; extern const pktio_if_ops_t * const pktio_if_ops[]; #ifdef __cplusplus diff --git a/platform/linux-generic/include/odp_packet_tap.h b/platform/linux-generic/include/odp_packet_tap.h new file mode 100644 index 000..7877586 --- /dev/null +++ b/platform/linux-generic/include/odp_packet_tap.h @@ -0,0 +1,21 @@ +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#ifndef ODP_PACKET_TAP_H_ +#define ODP_PACKET_TAP_H_ + +#include + +typedef struct { + int fd; /**< file descriptor for tap interface*/ + int skfd; /**< socket descriptor */ + uint32_t mtu; /**< cached mtu */ + unsigned char if_mac[ETH_ALEN]; /**< MAC address of pktio side (not a +MAC address of kernel interface)*/ + odp_pool_t pool;/**< pool to alloc packets from */ +} pkt_tap_t; + +#endif diff --git a/platform/linux-generic/pktio/io_ops.c b/platform/linux-generic/pktio/io_ops.c index 3b344e6..1933abc 100644 --- a/platform/linux-generic/pktio/io_ops.c +++ b/platform/linux-generic/pktio/io_ops.c @@ -18,6 +18,7 @@ const pktio_if_ops_t * const pktio_if_ops[] = { #ifdef HAVE_PCAP _pktio_ops, #endif + _pktio_ops, _mmap_pktio_ops, _mmsg_pktio_ops, NULL diff --git a/platform/linux-generic/pktio/tap.c b/platform/linux-generic/pktio/tap.c new file mode 100644 index 000..7ecb300 --- /dev/null +++ b/platform/linux-generic/pktio/tap.c @@ -0,0 +1,327 @@ +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + * + * TAP pktio type + * + * This file provides a pk
[lng-odp] [PATCHv5 0/4] TAP pktio
Creates a new pktio type that allows for creating and sending/receiving packets through TAP interface. Detailed description in commit-message of patch "[PATCHv5 3/4] linux-generic: pktio: add tap pktio type". Changelog: Version 5: * nothing changed. New patch added to add ability to wait some time right after pktio_open() to be sure that tap interface switched to enabled state inside the bridge. Fixes occasional first test failures. ODP_WAIT_FOR_NETWORK used to run tests. * rebased on current master Version 4: * changed error handling part in tap_pktio_send. (Stuart Haslam) Version 3: * return 77 (TEST_SKIPPED) if user is not root. (Stuart Haslam) Version 2: * Validation tests added * Pktio tests fixed to work with real-world interfaces. * MAC of pktio now is not a kernel interface's MAC * Interfaces are UP after pktio_open() * Fixed getting mtu, getting/setting promisc mode * Misclenious fixes Ilya Maximets (4): validation: pktio: initialize mac addresses for all packets validation: pktio: ability to wait for external network linux-generic: pktio: add tap pktio type linux-generic: pktio: add test for tap pktio platform/linux-generic/Makefile.am | 2 + .../linux-generic/include/odp_packet_io_internal.h | 3 + platform/linux-generic/include/odp_packet_tap.h| 21 ++ platform/linux-generic/pktio/io_ops.c | 1 + platform/linux-generic/pktio/tap.c | 327 + platform/linux-generic/test/Makefile.am| 1 + platform/linux-generic/test/pktio/Makefile.am | 3 +- platform/linux-generic/test/pktio/pktio_run_tap| 115 test/validation/pktio/pktio.c | 72 - 9 files changed, 536 insertions(+), 9 deletions(-) create mode 100644 platform/linux-generic/include/odp_packet_tap.h create mode 100644 platform/linux-generic/pktio/tap.c create mode 100755 platform/linux-generic/test/pktio/pktio_run_tap -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv5 2/4] validation: pktio: ability to wait for external network
'ODP_WAIT_FOR_NETWORK' environment variable may be used to wait some time right after pktio_open(). Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- 'ODP_WAIT_FOR_NETWORK' also may be used to remove sleep(1) from netmap pktio in the future, beacause IMO waiting for external network is a work for application and not for pktio itself. test/validation/pktio/pktio.c | 26 +- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index 53633dd..28bfd30 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -29,6 +29,11 @@ static const char *iface_name[MAX_NUM_IFACES]; /** number of interfaces being used (1=loopback, 2=pair) */ static int num_ifaces; +/** while testing real-world interfaces additional time may be +needed for external network to enable link to pktio +interface that just become up.*/ +static bool wait_for_network; + /** local container for pktio attributes */ typedef struct { const char *name; @@ -252,6 +257,17 @@ static int default_pool_create(void) return 0; } +static void spin_wait(uint64_t ns) +{ + odp_time_t start, now, diff; + + start = odp_time_local(); + do { + now = odp_time_local(); + diff = odp_time_diff(now, start); + } while (odp_time_to_ns(diff) < ns); +} + static odp_pktio_t create_pktio(int iface_idx, odp_pktio_input_mode_t imode, odp_pktio_output_mode_t omode) { @@ -271,6 +287,9 @@ static odp_pktio_t create_pktio(int iface_idx, odp_pktio_input_mode_t imode, CU_ASSERT(odp_pktio_to_u64(pktio) != odp_pktio_to_u64(ODP_PKTIO_INVALID)); + if (wait_for_network) + spin_wait(ODP_TIME_SEC_IN_NS / 4); + return pktio; } @@ -1105,11 +1124,16 @@ static int create_pool(const char *iface, int num) static int pktio_suite_init(void) { + int i; + odp_atomic_init_u32(_seq, 0); + + if (getenv("ODP_WAIT_FOR_NETWORK")) + wait_for_network = true; + iface_name[0] = getenv("ODP_PKTIO_IF0"); iface_name[1] = getenv("ODP_PKTIO_IF1"); num_ifaces = 1; - int i; if (!iface_name[0]) { printf("No interfaces specified, using default \"loop\".\n"); -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCHv5] helper : Fix UDP checksum computation
Some comments below. On 09.12.2015 11:59, ion.grig...@freescale.com wrote: > From: Grigore Ion <ion.grig...@freescale.com> > > This patch fixes the following problems: > - checksum computation for LE platforms > - checksum is computed in the CPU endianness. The returned result > must be converted to the BE ordering when it is used to update > the UDP checksum in a packet. > - checksum computation for packets having the UDP length not a > multiple of 2 > - fixes the UDP checksum associated validation test > > Signed-off-by: Grigore Ion <ion.grig...@freescale.com> > --- > v5: > - Checksum in CPU endianness fix added (Ilya Maximets) > v4: > - Verify checksum in CPU endianness in the associated test > (Ilya Maximets) > v3: > - fix the UDP checksum computation in the associated test > (Maxim Uvarov) > v2: > - patch updated to the last master (Maxim Uvarov) > v1: > - Move variables declaration on top of block. (Maxim Uvarov) > - Check patch with checkpatch script. (Maxim Uvarov) > - L3 header presence is tested twice. (Alexandru Badicioiu) > - Remove unnecessary check for L3 header presence. (Bill Fischofer) > - Modify check of odp_packet_l4_offset() return. (Bill Fischofer) > > helper/include/odp/helper/udp.h | 55 > + > helper/test/odp_chksum.c| 4 +-- > 2 files changed, 25 insertions(+), 34 deletions(-) > > diff --git a/helper/include/odp/helper/udp.h b/helper/include/odp/helper/udp.h > index 06c439b..9e7256d 100644 > --- a/helper/include/odp/helper/udp.h > +++ b/helper/include/odp/helper/udp.h > @@ -4,7 +4,6 @@ > * SPDX-License-Identifier: BSD-3-Clause > */ > > - > /** > * @file > * > @@ -22,7 +21,6 @@ extern "C" { > #include > #include > > - > /** @addtogroup odph_header ODPH HEADER > * @{ > */ > @@ -44,46 +42,39 @@ typedef struct ODP_PACKED { > * This function uses odp packet to calc checksum > * > * @param pkt calculate chksum for pkt > - * @return checksum value > + * @return checksum value in CPU endianness > */ > static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) > { > - uint32_t sum = 0; > - odph_udphdr_t *udph; > - odph_ipv4hdr_t *iph; > - uint16_t udplen; > - uint8_t *buf; > - > - if (!odp_packet_l3_offset(pkt)) > - return 0; > + odph_ipv4hdr_t *iph; > + odph_udphdr_t *udph; > + uint32_tsum; > + uint16_tudplen, *buf; > > - if (!odp_packet_l4_offset(pkt)) > + if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID) > return 0; > - > iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL); > udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL); > - udplen = odp_be_to_cpu_16(udph->length); > - > - /* 32-bit sum of all 16-bit words covered by UDP chksum */ > + /* 32-bit sum of UDP pseudo-header */ > sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) + > - (iph->dst_addr & 0x) + (iph->dst_addr >> 16) + > - (uint16_t)iph->proto + udplen; > - for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) { > - sum += ((*buf << 8) + *(buf + 1)); > - buf += 2; > - } > - > - /* Fold sum to 16 bits: add carrier to result */ > - while (sum >> 16) > - sum = (sum & 0x) + (sum >> 16); > - > + (iph->dst_addr & 0x) + (iph->dst_addr >> 16) + > + odp_be_to_cpu_16(iph->proto) + udph->length; This line is also strange. You're using odp_be_to_cpu_16 for 8-bit digit. At this point, iph->proto firstly will be extended to u16 in CPU endianness and after that odp_be_to_cpu_16() will return it to BE. You will have right position of all bytes (8 zero bits + 8 bits of iph->proto), but the way how this done is wrong. > + udplen = odp_be_to_cpu_16(udph->length); > + buf = (uint16_t *)((void *)udph); > + /* 32-bit sum of UDP header (checksum field cleared) and UDP data */ > + for ( ; udplen > 1; udplen -= 2) > + sum += *buf++; > + /* Length is not a multiple of 2 bytes */ > + if (udplen) > + sum += odp_be_to_cpu_16(*((uint8_t *)buf) << 8); Same at this point. You're using odp_be_to_cpu_16() to perform opposite operation. May be, better to use byte arrays here to avoid that strange endianness and compiler aware operations. > + /* Fold sum to 16 bits */ > + sum = (sum & 0x) + (sum >> 16); > + /* Add carrier (0/1) to result */ > + sum += (sum >
Re: [lng-odp] [PATCHv5] helper : Fix UDP checksum computation
On 09.12.2015 15:26, Ion Grigore wrote: > See inline comments. > > -Original Message- > From: Ilya Maximets [mailto:i.maxim...@samsung.com] > Sent: Wednesday, December 09, 2015 12:04 PM > To: Grigore Ion-B17953 <ion.grig...@freescale.com>; lng-odp@lists.linaro.org > Subject: Re: [PATCHv5] helper : Fix UDP checksum computation > > Some comments below. > > On 09.12.2015 11:59, ion.grig...@freescale.com wrote: >> From: Grigore Ion <ion.grig...@freescale.com> >> >> This patch fixes the following problems: >> - checksum computation for LE platforms >> - checksum is computed in the CPU endianness. The returned result must >> be converted to the BE ordering when it is used to update the UDP >> checksum in a packet. >> - checksum computation for packets having the UDP length not a >> multiple of 2 >> - fixes the UDP checksum associated validation test >> >> Signed-off-by: Grigore Ion <ion.grig...@freescale.com> >> --- >> v5: >> - Checksum in CPU endianness fix added (Ilya Maximets) >> v4: >> - Verify checksum in CPU endianness in the associated test (Ilya >> Maximets) >> v3: >> - fix the UDP checksum computation in the associated test (Maxim >> Uvarov) >> v2: >> - patch updated to the last master (Maxim Uvarov) >> v1: >> - Move variables declaration on top of block. (Maxim Uvarov) >> - Check patch with checkpatch script. (Maxim Uvarov) >> - L3 header presence is tested twice. (Alexandru Badicioiu) >> - Remove unnecessary check for L3 header presence. (Bill Fischofer) >> - Modify check of odp_packet_l4_offset() return. (Bill Fischofer) >> >> helper/include/odp/helper/udp.h | 55 >> + >> helper/test/odp_chksum.c| 4 +-- >> 2 files changed, 25 insertions(+), 34 deletions(-) >> >> diff --git a/helper/include/odp/helper/udp.h >> b/helper/include/odp/helper/udp.h index 06c439b..9e7256d 100644 >> --- a/helper/include/odp/helper/udp.h >> +++ b/helper/include/odp/helper/udp.h >> @@ -4,7 +4,6 @@ >> * SPDX-License-Identifier: BSD-3-Clause >> */ >> >> - >> /** >> * @file >> * >> @@ -22,7 +21,6 @@ extern "C" { >> #include >> #include >> >> - >> /** @addtogroup odph_header ODPH HEADER >> * @{ >> */ >> @@ -44,46 +42,39 @@ typedef struct ODP_PACKED { >> * This function uses odp packet to calc checksum >> * >> * @param pkt calculate chksum for pkt >> - * @return checksum value >> + * @return checksum value in CPU endianness >> */ >> static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) { >> -uint32_t sum = 0; >> -odph_udphdr_t *udph; >> -odph_ipv4hdr_t *iph; >> -uint16_t udplen; >> -uint8_t *buf; >> - >> -if (!odp_packet_l3_offset(pkt)) >> -return 0; >> +odph_ipv4hdr_t *iph; >> +odph_udphdr_t *udph; >> +uint32_tsum; >> +uint16_tudplen, *buf; >> >> -if (!odp_packet_l4_offset(pkt)) >> +if (odp_packet_l4_offset(pkt) == ODP_PACKET_OFFSET_INVALID) >> return 0; >> - >> iph = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL); >> udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL); >> -udplen = odp_be_to_cpu_16(udph->length); >> - >> -/* 32-bit sum of all 16-bit words covered by UDP chksum */ >> +/* 32-bit sum of UDP pseudo-header */ >> sum = (iph->src_addr & 0x) + (iph->src_addr >> 16) + >> - (iph->dst_addr & 0x) + (iph->dst_addr >> 16) + >> - (uint16_t)iph->proto + udplen; >> -for (buf = (uint8_t *)udph; udplen > 1; udplen -= 2) { >> -sum += ((*buf << 8) + *(buf + 1)); >> -buf += 2; >> -} >> - >> -/* Fold sum to 16 bits: add carrier to result */ >> -while (sum >> 16) >> -sum = (sum & 0x) + (sum >> 16); >> - >> +(iph->dst_addr & 0x) + (iph->dst_addr >> 16) + >> +odp_be_to_cpu_16(iph->proto) + udph->length; > > This line is also strange. You're using odp_be_to_cpu_16 for 8-bit digit. > At this point, iph->proto firstly will be extended to u16 in CPU endianness > and after that odp_be_to_cpu_16() will return it to BE. You will have right > position of all bytes (8 zero bits + 8 bits of iph->proto), but the way how > th
Re: [lng-odp] [PATCHv3 2/3] linux-generic: pktio: add tap pktio type
On 08.12.2015 14:24, Stuart Haslam wrote: > On Mon, Dec 07, 2015 at 01:55:31PM +0300, Ilya Maximets wrote: >> Creates a new pktio type that allows for creating and >> sending/receiving packets through TAP interface. >> It is intended for use as a simple conventional communication >> method between applications that use kernel network stack >> (ping, ssh, iperf, etc.) and ODP applications for the purpose >> of functional testing and can be used as it is with some >> of the existing example applications. >> >> To use this interface the name passed to odp_pktio_open() must >> begin with "tap:" and be in the format: >> >> tap:iface >> >>iface the name of TAP device to be created. >> >> TUN/TAP kernel module should be loaded to use this pktio. >> There should be no device named 'iface' in the system. >> The total length of the 'iface' is limited by IF_NAMESIZE. >> >> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> >> --- >> platform/linux-generic/Makefile.am | 2 + >> .../linux-generic/include/odp_packet_io_internal.h | 3 + >> platform/linux-generic/include/odp_packet_tap.h| 21 ++ >> platform/linux-generic/pktio/io_ops.c | 1 + >> platform/linux-generic/pktio/tap.c | 317 >> + >> 5 files changed, 344 insertions(+) >> create mode 100644 platform/linux-generic/include/odp_packet_tap.h >> create mode 100644 platform/linux-generic/pktio/tap.c >> >> diff --git a/platform/linux-generic/Makefile.am >> b/platform/linux-generic/Makefile.am >> index 70bd8fe..4639ebc 100644 >> --- a/platform/linux-generic/Makefile.am >> +++ b/platform/linux-generic/Makefile.am >> @@ -92,6 +92,7 @@ noinst_HEADERS = \ >>${srcdir}/include/odp_packet_io_queue.h \ >>${srcdir}/include/odp_packet_netmap.h \ >>${srcdir}/include/odp_packet_socket.h \ >> + ${srcdir}/include/odp_packet_tap.h \ >>${srcdir}/include/odp_pool_internal.h \ >>${srcdir}/include/odp_queue_internal.h \ >>${srcdir}/include/odp_schedule_internal.h \ >> @@ -120,6 +121,7 @@ __LIB__libodp_la_SOURCES = \ >> pktio/netmap.c \ >> pktio/socket.c \ >> pktio/socket_mmap.c \ >> + pktio/tap.c \ >> odp_pool.c \ >> odp_queue.c \ >> odp_rwlock.c \ >> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h >> b/platform/linux-generic/include/odp_packet_io_internal.h >> index 1a1118c..de29557 100644 >> --- a/platform/linux-generic/include/odp_packet_io_internal.h >> +++ b/platform/linux-generic/include/odp_packet_io_internal.h >> @@ -22,6 +22,7 @@ extern "C" { >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -78,6 +79,7 @@ struct pktio_entry { >> #ifdef HAVE_PCAP >> pkt_pcap_t pkt_pcap;/**< Using pcap for IO */ >> #endif >> +pkt_tap_t pkt_tap; /**< using TAP for IO */ >> }; >> enum { >> STATE_START = 0, >> @@ -157,6 +159,7 @@ extern const pktio_if_ops_t loopback_pktio_ops; >> #ifdef HAVE_PCAP >> extern const pktio_if_ops_t pcap_pktio_ops; >> #endif >> +extern const pktio_if_ops_t tap_pktio_ops; >> extern const pktio_if_ops_t * const pktio_if_ops[]; >> >> #ifdef __cplusplus >> diff --git a/platform/linux-generic/include/odp_packet_tap.h >> b/platform/linux-generic/include/odp_packet_tap.h >> new file mode 100644 >> index 000..7877586 >> --- /dev/null >> +++ b/platform/linux-generic/include/odp_packet_tap.h >> @@ -0,0 +1,21 @@ >> +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +#ifndef ODP_PACKET_TAP_H_ >> +#define ODP_PACKET_TAP_H_ >> + >> +#include >> + >> +typedef struct { >> +int fd; /**< file descriptor for tap interface*/ >> +int skfd; /**< socket descriptor */ >> +uint32_t mtu; /**< cached mtu */ >> +unsigned char if_mac[ETH_ALEN]; /**< MAC address of pktio side (not a >> +
Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
Matias, I think, better to make separate patch for that. It will not intersect with mine in case of only 2 deletions. Would you prepare it? Best regards, Ilya Maximets. On 08.12.2015 15:21, Elo, Matias (Nokia - FI/Espoo) wrote: > Hi Ilya, > > I had completely missed your previous patch. You could also remove the two > errno value checks in pktio_test_send_failure(). They are not defined in the > API, so they should not be tested. > > -Matias > >> -Original Message----- >> From: EXT Ilya Maximets [mailto:i.maxim...@samsung.com] >> Sent: Tuesday, December 08, 2015 12:44 PM >> To: Elo, Matias (Nokia - FI/Espoo) <matias@nokia.com>; lng- >> o...@lists.linaro.org >> Subject: Re: validation: pktio: fix start_stop and send_failure tests >> >> Matias, >> please check out my patch ( https://lists.linaro.org/pipermail/lng-odp/2015- >> December/018057.html ) >> from my 'TAP pktio' patch set. >> >> Best regards, Ilya Maximets. >> >> On 08.12.2015 13:36, Elo, Matias (Nokia - FI/Espoo) wrote: >>> In both tests set MAC addresses to test packets to enable >>> testing with real pktio interfaces. >>> >>> In pktio_test_send_failure() don't check for errno values >>> as they are not defined in the API. >>> >>> Signed-off-by: Matias Elo <matias@nokia.com> >>> --- >>> test/validation/pktio/pktio.c | 26 +- >>> 1 file changed, 17 insertions(+), 9 deletions(-) >>> >>> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c >>> index 50a6d8a..4ded924 100644 >>> --- a/test/validation/pktio/pktio.c >>> +++ b/test/validation/pktio/pktio.c >>> @@ -695,6 +695,7 @@ void pktio_test_inq(void) >>> static void pktio_test_start_stop(void) >>> { >>> odp_pktio_t pktio[MAX_NUM_IFACES]; >>> + pktio_info_t pktio_info[MAX_NUM_IFACES]; >>> odp_packet_t pkt; >>> odp_event_t tx_ev[1000]; >>> odp_event_t ev; >>> @@ -705,6 +706,7 @@ static void pktio_test_start_stop(void) >>> for (i = 0; i < num_ifaces; i++) { >>> pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); >>> CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); >>> + pktio_info[i].id = pktio[i]; >>> create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); >>> } >>> >>> @@ -724,6 +726,8 @@ static void pktio_test_start_stop(void) >>> if (pkt == ODP_PACKET_INVALID) >>> break; >>> pktio_init_packet(pkt); >>> + pktio_pkt_set_macs(pkt, _info[0], >>> + _info[1]); >>> tx_ev[alloc] = odp_packet_to_event(pkt); >>> } >>> >>> @@ -775,6 +779,9 @@ static void pktio_test_start_stop(void) >>> if (pkt == ODP_PACKET_INVALID) >>> break; >>> pktio_init_packet(pkt); >>> + if (num_ifaces > 1) >>> + pktio_pkt_set_macs(pkt, _info[0], >>> + _info[1]); >>> tx_ev[alloc] = odp_packet_to_event(pkt); >>> } >>> >>> @@ -849,13 +856,14 @@ static void pktio_test_send_failure(void) >>> odp_pool_param_t pool_params; >>> odp_pool_t pkt_pool; >>> int long_pkt_idx = TX_BATCH_LEN / 2; >>> - pktio_info_t info_rx; >>> + pktio_info_t info_tx, info_rx; >>> >>> pktio_tx = create_pktio(0, ODP_PKTIN_MODE_RECV); >>> if (pktio_tx == ODP_PKTIO_INVALID) { >>> CU_FAIL("failed to open pktio"); >>> return; >>> } >>> + info_tx.id = pktio_tx; >>> >>> /* read the MTU from the transmit interface */ >>> mtu = odp_pktio_mtu(pktio_tx); >>> @@ -880,6 +888,10 @@ static void pktio_test_send_failure(void) >>> } else { >>> pktio_rx = pktio_tx; >>> } >>> + info_rx.id = pktio_rx; >>> + info_rx.outq = ODP_QUEUE_INVALID; >>> + info_rx.inq = ODP_QUEUE_INVALID; >>> + info_rx.in_mode = ODP_PKTIN_MODE_RECV; >>> >>> /* generate a batch of packets with a single overly long packet >>> * in the middle */ >>> @@ -898,6 +910,8 @@ static void pktio_test_send_failure(void) >>> pkt_seq[i] = pktio_init_p
Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
Matias, please check out my patch ( https://lists.linaro.org/pipermail/lng-odp/2015-December/018057.html ) from my 'TAP pktio' patch set. Best regards, Ilya Maximets. On 08.12.2015 13:36, Elo, Matias (Nokia - FI/Espoo) wrote: > In both tests set MAC addresses to test packets to enable > testing with real pktio interfaces. > > In pktio_test_send_failure() don't check for errno values > as they are not defined in the API. > > Signed-off-by: Matias Elo <matias@nokia.com> > --- > test/validation/pktio/pktio.c | 26 +- > 1 file changed, 17 insertions(+), 9 deletions(-) > > diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c > index 50a6d8a..4ded924 100644 > --- a/test/validation/pktio/pktio.c > +++ b/test/validation/pktio/pktio.c > @@ -695,6 +695,7 @@ void pktio_test_inq(void) > static void pktio_test_start_stop(void) > { > odp_pktio_t pktio[MAX_NUM_IFACES]; > + pktio_info_t pktio_info[MAX_NUM_IFACES]; > odp_packet_t pkt; > odp_event_t tx_ev[1000]; > odp_event_t ev; > @@ -705,6 +706,7 @@ static void pktio_test_start_stop(void) > for (i = 0; i < num_ifaces; i++) { > pktio[i] = create_pktio(i, ODP_PKTIN_MODE_SCHED); > CU_ASSERT_FATAL(pktio[i] != ODP_PKTIO_INVALID); > + pktio_info[i].id = pktio[i]; > create_inq(pktio[i], ODP_QUEUE_TYPE_SCHED); > } > > @@ -724,6 +726,8 @@ static void pktio_test_start_stop(void) > if (pkt == ODP_PACKET_INVALID) > break; > pktio_init_packet(pkt); > + pktio_pkt_set_macs(pkt, _info[0], > +_info[1]); > tx_ev[alloc] = odp_packet_to_event(pkt); > } > > @@ -775,6 +779,9 @@ static void pktio_test_start_stop(void) > if (pkt == ODP_PACKET_INVALID) > break; > pktio_init_packet(pkt); > + if (num_ifaces > 1) > + pktio_pkt_set_macs(pkt, _info[0], > +_info[1]); > tx_ev[alloc] = odp_packet_to_event(pkt); > } > > @@ -849,13 +856,14 @@ static void pktio_test_send_failure(void) > odp_pool_param_t pool_params; > odp_pool_t pkt_pool; > int long_pkt_idx = TX_BATCH_LEN / 2; > - pktio_info_t info_rx; > + pktio_info_t info_tx, info_rx; > > pktio_tx = create_pktio(0, ODP_PKTIN_MODE_RECV); > if (pktio_tx == ODP_PKTIO_INVALID) { > CU_FAIL("failed to open pktio"); > return; > } > + info_tx.id = pktio_tx; > > /* read the MTU from the transmit interface */ > mtu = odp_pktio_mtu(pktio_tx); > @@ -880,6 +888,10 @@ static void pktio_test_send_failure(void) > } else { > pktio_rx = pktio_tx; > } > + info_rx.id = pktio_rx; > + info_rx.outq = ODP_QUEUE_INVALID; > + info_rx.inq = ODP_QUEUE_INVALID; > + info_rx.in_mode = ODP_PKTIN_MODE_RECV; > > /* generate a batch of packets with a single overly long packet >* in the middle */ > @@ -898,6 +910,8 @@ static void pktio_test_send_failure(void) > pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); > if (pkt_seq[i] == TEST_SEQ_INVALID) > break; > + > + pktio_pkt_set_macs(pkt_tbl[i], _tx, _rx); > } > alloc_pkts = i; > > @@ -907,12 +921,6 @@ static void pktio_test_send_failure(void) > odp_errno_zero(); > ret = odp_pktio_send(pktio_tx, pkt_tbl, TX_BATCH_LEN); > CU_ASSERT(ret == long_pkt_idx); > - CU_ASSERT(odp_errno() == 0); > - > - info_rx.id = pktio_rx; > - info_rx.outq = ODP_QUEUE_INVALID; > - info_rx.inq = ODP_QUEUE_INVALID; > - info_rx.in_mode = ODP_PKTIN_MODE_RECV; > > for (i = 0; i < ret; ++i) { > pkt_tbl[i] = wait_for_packet(_rx, pkt_seq[i], > @@ -929,9 +937,8 @@ static void pktio_test_send_failure(void) >_tbl[long_pkt_idx], >TX_BATCH_LEN - long_pkt_idx); > CU_ASSERT(ret == -1); > - CU_ASSERT(odp_errno() != 0); > } else { > - CU_FAIL("failed to receive transmitted packets\n"); > + CU_FAIL("failed to receive transmitted packets"); > } > > /* now reduce the size of the lon
Re: [lng-odp] [PATCHv3 2/3] linux-generic: pktio: add tap pktio type
On 08.12.2015 15:16, Stuart Haslam wrote: > On Tue, Dec 08, 2015 at 02:51:40PM +0300, Ilya Maximets wrote: >> On 08.12.2015 14:24, Stuart Haslam wrote: >>> On Mon, Dec 07, 2015 at 01:55:31PM +0300, Ilya Maximets wrote: >>>> Creates a new pktio type that allows for creating and >>>> sending/receiving packets through TAP interface. >>>> It is intended for use as a simple conventional communication >>>> method between applications that use kernel network stack >>>> (ping, ssh, iperf, etc.) and ODP applications for the purpose >>>> of functional testing and can be used as it is with some >>>> of the existing example applications. >>>> >>>> To use this interface the name passed to odp_pktio_open() must >>>> begin with "tap:" and be in the format: >>>> >>>> tap:iface >>>> >>>>iface the name of TAP device to be created. >>>> >>>> TUN/TAP kernel module should be loaded to use this pktio. >>>> There should be no device named 'iface' in the system. >>>> The total length of the 'iface' is limited by IF_NAMESIZE. >>>> >>>> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> >>>> --- >>>> platform/linux-generic/Makefile.am | 2 + >>>> .../linux-generic/include/odp_packet_io_internal.h | 3 + >>>> platform/linux-generic/include/odp_packet_tap.h| 21 ++ >>>> platform/linux-generic/pktio/io_ops.c | 1 + >>>> platform/linux-generic/pktio/tap.c | 317 >>>> + >>>> 5 files changed, 344 insertions(+) >>>> create mode 100644 platform/linux-generic/include/odp_packet_tap.h >>>> create mode 100644 platform/linux-generic/pktio/tap.c >>>> >>>> diff --git a/platform/linux-generic/Makefile.am >>>> b/platform/linux-generic/Makefile.am >>>> index 70bd8fe..4639ebc 100644 >>>> --- a/platform/linux-generic/Makefile.am >>>> +++ b/platform/linux-generic/Makefile.am >>>> @@ -92,6 +92,7 @@ noinst_HEADERS = \ >>>> ${srcdir}/include/odp_packet_io_queue.h \ >>>> ${srcdir}/include/odp_packet_netmap.h \ >>>> ${srcdir}/include/odp_packet_socket.h \ >>>> +${srcdir}/include/odp_packet_tap.h \ >>>> ${srcdir}/include/odp_pool_internal.h \ >>>> ${srcdir}/include/odp_queue_internal.h \ >>>> ${srcdir}/include/odp_schedule_internal.h \ >>>> @@ -120,6 +121,7 @@ __LIB__libodp_la_SOURCES = \ >>>> pktio/netmap.c \ >>>> pktio/socket.c \ >>>> pktio/socket_mmap.c \ >>>> + pktio/tap.c \ >>>> odp_pool.c \ >>>> odp_queue.c \ >>>> odp_rwlock.c \ >>>> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h >>>> b/platform/linux-generic/include/odp_packet_io_internal.h >>>> index 1a1118c..de29557 100644 >>>> --- a/platform/linux-generic/include/odp_packet_io_internal.h >>>> +++ b/platform/linux-generic/include/odp_packet_io_internal.h >>>> @@ -22,6 +22,7 @@ extern "C" { >>>> #include >>>> #include >>>> #include >>>> +#include >>>> #include >>>> #include >>>> #include >>>> @@ -78,6 +79,7 @@ struct pktio_entry { >>>> #ifdef HAVE_PCAP >>>>pkt_pcap_t pkt_pcap;/**< Using pcap for IO */ >>>> #endif >>>> + pkt_tap_t pkt_tap; /**< using TAP for IO */ >>>>}; >>>>enum { >>>>STATE_START = 0, >>>> @@ -157,6 +159,7 @@ extern const pktio_if_ops_t loopback_pktio_ops; >>>> #ifdef HAVE_PCAP >>>> extern const pktio_if_ops_t pcap_pktio_ops; >>>> #endif >>>> +extern const pktio_if_ops_t tap_pktio_ops; >>>> extern const pktio_if_ops_t * const pktio_if_ops[]; >>>> >>>> #ifdef __cplusplus >>>> diff --git a/platform/linux-generic/include/odp_packet_tap.h >>>> b/platform/linux-generic/include/odp_packet_tap.h >>>> new file mode 100644 >>>> index 000..7877586 >>>> --- /dev/null >>>>
[lng-odp] [PATCHv4 3/3] linux-generic: pktio: add test for tap pktio
Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- platform/linux-generic/test/Makefile.am | 1 + platform/linux-generic/test/pktio/Makefile.am | 3 +- platform/linux-generic/test/pktio/pktio_run_tap | 114 3 files changed, 117 insertions(+), 1 deletion(-) create mode 100755 platform/linux-generic/test/pktio/pktio_run_tap diff --git a/platform/linux-generic/test/Makefile.am b/platform/linux-generic/test/Makefile.am index 1475589..28bd3dc 100644 --- a/platform/linux-generic/test/Makefile.am +++ b/platform/linux-generic/test/Makefile.am @@ -5,6 +5,7 @@ ODP_MODULES = pktio if test_vald TESTS = pktio/pktio_run \ + pktio/pktio_run_tap \ ${top_builddir}/test/validation/buffer/buffer_main$(EXEEXT) \ ${top_builddir}/test/validation/classification/classification_main$(EXEEXT) \ ${top_builddir}/test/validation/cpumask/cpumask_main$(EXEEXT) \ diff --git a/platform/linux-generic/test/pktio/Makefile.am b/platform/linux-generic/test/pktio/Makefile.am index 4d39372..ea0ad87 100644 --- a/platform/linux-generic/test/pktio/Makefile.am +++ b/platform/linux-generic/test/pktio/Makefile.am @@ -1,5 +1,6 @@ dist_check_SCRIPTS = pktio_env \ -pktio_run +pktio_run \ +pktio_run_tap if HAVE_PCAP dist_check_SCRIPTS += pktio_run_pcap diff --git a/platform/linux-generic/test/pktio/pktio_run_tap b/platform/linux-generic/test/pktio/pktio_run_tap new file mode 100755 index 000..59bcd29 --- /dev/null +++ b/platform/linux-generic/test/pktio/pktio_run_tap @@ -0,0 +1,114 @@ +#!/bin/sh +# +# Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause +# + +# directories where pktio_main binary can be found: +# -in the validation dir when running make check (intree or out of tree) +# -in the script directory, when running after 'make install', or +# -in the validation when running standalone intree. +# -in the current directory. +# running stand alone out of tree requires setting PATH +PATH=${TEST_DIR}/pktio:$PATH +PATH=$(dirname $0):$PATH +PATH=$(dirname $0)/../../../../test/validation/pktio:$PATH +PATH=.:$PATH + +pktio_main_path=$(which pktio_main${EXEEXT}) +if [ -x "$pktio_main_path" ] ; then + echo "running with $pktio_main_path" +else + echo "cannot find pktio_main${EXEEXT}: please set you PATH for it." +fi + +# exit code expected by automake for skipped tests +TEST_SKIPPED=77 + +TAP_BASE_NAME=iotap_vald +IF0=${TAP_BASE_NAME}0 +IF1=${TAP_BASE_NAME}1 +BR=${TAP_BASE_NAME}_br + +export ODP_PKTIO_IF0="tap:$IF0" +export ODP_PKTIO_IF1="tap:$IF1" + +tap_cleanup() +{ + ret=$? + + for iface in $IF0 $IF1; do + ip link set dev $iface nomaster + done + + ip link delete $BR type bridge + + for iface in $IF0 $IF1; do + ip tuntap del mode tap $iface + done + + trap - EXIT + exit $ret +} + +tap_setup() +{ + if [ "$(id -u)" != "0" ]; then + echo "pktio: need to be root to setup TAP interfaces." + return $TEST_SKIPPED + fi + + for iface in $IF0 $IF1 $BR; do + ip link show $iface 2> /dev/null + if [ $? -eq 0 ]; then + echo "pktio: interface $iface already exist $?" + return 2 + fi + done + + trap tap_cleanup EXIT + + for iface in $IF0 $IF1; do + ip tuntap add mode tap $iface + if [ $? -ne 0 ]; then + echo "pktio: error: unable to create TAP device $iface" + return 3 + fi + done + + ip link add name $BR type bridge + if [ $? -ne 0 ]; then + echo "pktio: error: unable to create bridge $BR" + return 3 + fi + + for iface in $IF0 $IF1; do + ip link set dev $iface master $BR + if [ $? -ne 0 ]; then + echo "pktio: error: unable to add $iface to bridge $BR" + return 4 + fi + done + + for iface in $IF0 $IF1 $BR; do + ifconfig $iface -arp + sysctl -w net.ipv6.conf.${iface}.disable_ipv6=1 + ip link set dev $iface mtu 9216 up + done + + return 0 +} + +tap_setup +ret=$? +if [ $ret -ne 0 ]; then + echo "pktio: tap_setup() FAILED!" + exit $ret +fi + +pktio_main${EXEEXT} +ret=$? + +exit $ret -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv4 1/3] validation: pktio: initialize mac addresses for all packets
For the purpose of testing of real-world interfaces the packet's content should be valid or kernel will throw them away. Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- test/validation/pktio/pktio.c | 46 --- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index 50a6d8a..4c7d458 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -86,16 +86,16 @@ static void set_pool_len(odp_pool_param_t *params) } static void pktio_pkt_set_macs(odp_packet_t pkt, - pktio_info_t *src, pktio_info_t *dst) + odp_pktio_t src, odp_pktio_t dst) { uint32_t len; odph_ethhdr_t *eth = (odph_ethhdr_t *)odp_packet_l2_ptr(pkt, ); int ret; - ret = odp_pktio_mac_addr(src->id, >src, sizeof(eth->src)); + ret = odp_pktio_mac_addr(src, >src, sizeof(eth->src)); CU_ASSERT(ret == ODPH_ETHADDR_LEN); - ret = odp_pktio_mac_addr(dst->id, >dst, sizeof(eth->dst)); + ret = odp_pktio_mac_addr(dst, >dst, sizeof(eth->dst)); CU_ASSERT(ret == ODPH_ETHADDR_LEN); } @@ -410,12 +410,16 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, pktio_info_t *pktio_b, break; tx_seq[i] = pktio_init_packet(tx_pkt[i]); - if (tx_seq[i] == TEST_SEQ_INVALID) + if (tx_seq[i] == TEST_SEQ_INVALID) { + odp_packet_free(tx_pkt[i]); break; + } - pktio_pkt_set_macs(tx_pkt[i], pktio_a, pktio_b); - if (pktio_fixup_checksums(tx_pkt[i]) != 0) + pktio_pkt_set_macs(tx_pkt[i], pktio_a->id, pktio_b->id); + if (pktio_fixup_checksums(tx_pkt[i]) != 0) { + odp_packet_free(tx_pkt[i]); break; + } tx_ev[i] = odp_packet_to_event(tx_pkt[i]); } @@ -724,6 +728,13 @@ static void pktio_test_start_stop(void) if (pkt == ODP_PACKET_INVALID) break; pktio_init_packet(pkt); + + pktio_pkt_set_macs(pkt, pktio[0], pktio[1]); + if (pktio_fixup_checksums(pkt) != 0) { + odp_packet_free(pkt); + break; + } + tx_ev[alloc] = odp_packet_to_event(pkt); } @@ -775,6 +786,13 @@ static void pktio_test_start_stop(void) if (pkt == ODP_PACKET_INVALID) break; pktio_init_packet(pkt); + if (num_ifaces > 1) { + pktio_pkt_set_macs(pkt, pktio[0], pktio[1]); + if (pktio_fixup_checksums(pkt) != 0) { + odp_packet_free(pkt); + break; + } + } tx_ev[alloc] = odp_packet_to_event(pkt); } @@ -896,8 +914,17 @@ static void pktio_test_send_failure(void) break; pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); - if (pkt_seq[i] == TEST_SEQ_INVALID) + + pktio_pkt_set_macs(pkt_tbl[i], pktio_tx, pktio_rx); + if (pktio_fixup_checksums(pkt_tbl[i]) != 0) { + odp_packet_free(pkt_tbl[i]); + break; + } + + if (pkt_seq[i] == TEST_SEQ_INVALID) { + odp_packet_free(pkt_tbl[i]); break; + } } alloc_pkts = i; @@ -941,6 +968,11 @@ static void pktio_test_send_failure(void) odp_packet_len(pkt_tbl[i]) - PKT_LEN_NORMAL); pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); + + pktio_pkt_set_macs(pkt_tbl[i], pktio_tx, pktio_rx); + ret = pktio_fixup_checksums(pkt_tbl[i]); + CU_ASSERT_FATAL(ret == 0); + CU_ASSERT_FATAL(pkt_seq[i] != TEST_SEQ_INVALID); ret = odp_pktio_send(pktio_tx, _tbl[i], TX_BATCH_LEN - i); CU_ASSERT_FATAL(ret == (TX_BATCH_LEN - i)); -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv4 2/3] linux-generic: pktio: add tap pktio type
Creates a new pktio type that allows for creating and sending/receiving packets through TAP interface. It is intended for use as a simple conventional communication method between applications that use kernel network stack (ping, ssh, iperf, etc.) and ODP applications for the purpose of functional testing and can be used as it is with some of the existing example applications. To use this interface the name passed to odp_pktio_open() must begin with "tap:" and be in the format: tap:iface iface the name of TAP device to be created. TUN/TAP kernel module should be loaded to use this pktio. There should be no device named 'iface' in the system. The total length of the 'iface' is limited by IF_NAMESIZE. Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- platform/linux-generic/Makefile.am | 2 + .../linux-generic/include/odp_packet_io_internal.h | 3 + platform/linux-generic/include/odp_packet_tap.h| 21 ++ platform/linux-generic/pktio/io_ops.c | 1 + platform/linux-generic/pktio/tap.c | 327 + 5 files changed, 354 insertions(+) create mode 100644 platform/linux-generic/include/odp_packet_tap.h create mode 100644 platform/linux-generic/pktio/tap.c diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index 70bd8fe..4639ebc 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -92,6 +92,7 @@ noinst_HEADERS = \ ${srcdir}/include/odp_packet_io_queue.h \ ${srcdir}/include/odp_packet_netmap.h \ ${srcdir}/include/odp_packet_socket.h \ + ${srcdir}/include/odp_packet_tap.h \ ${srcdir}/include/odp_pool_internal.h \ ${srcdir}/include/odp_queue_internal.h \ ${srcdir}/include/odp_schedule_internal.h \ @@ -120,6 +121,7 @@ __LIB__libodp_la_SOURCES = \ pktio/netmap.c \ pktio/socket.c \ pktio/socket_mmap.c \ + pktio/tap.c \ odp_pool.c \ odp_queue.c \ odp_rwlock.c \ diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h index 1a1118c..de29557 100644 --- a/platform/linux-generic/include/odp_packet_io_internal.h +++ b/platform/linux-generic/include/odp_packet_io_internal.h @@ -22,6 +22,7 @@ extern "C" { #include #include #include +#include #include #include #include @@ -78,6 +79,7 @@ struct pktio_entry { #ifdef HAVE_PCAP pkt_pcap_t pkt_pcap;/**< Using pcap for IO */ #endif + pkt_tap_t pkt_tap; /**< using TAP for IO */ }; enum { STATE_START = 0, @@ -157,6 +159,7 @@ extern const pktio_if_ops_t loopback_pktio_ops; #ifdef HAVE_PCAP extern const pktio_if_ops_t pcap_pktio_ops; #endif +extern const pktio_if_ops_t tap_pktio_ops; extern const pktio_if_ops_t * const pktio_if_ops[]; #ifdef __cplusplus diff --git a/platform/linux-generic/include/odp_packet_tap.h b/platform/linux-generic/include/odp_packet_tap.h new file mode 100644 index 000..7877586 --- /dev/null +++ b/platform/linux-generic/include/odp_packet_tap.h @@ -0,0 +1,21 @@ +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#ifndef ODP_PACKET_TAP_H_ +#define ODP_PACKET_TAP_H_ + +#include + +typedef struct { + int fd; /**< file descriptor for tap interface*/ + int skfd; /**< socket descriptor */ + uint32_t mtu; /**< cached mtu */ + unsigned char if_mac[ETH_ALEN]; /**< MAC address of pktio side (not a +MAC address of kernel interface)*/ + odp_pool_t pool;/**< pool to alloc packets from */ +} pkt_tap_t; + +#endif diff --git a/platform/linux-generic/pktio/io_ops.c b/platform/linux-generic/pktio/io_ops.c index 3b344e6..1933abc 100644 --- a/platform/linux-generic/pktio/io_ops.c +++ b/platform/linux-generic/pktio/io_ops.c @@ -18,6 +18,7 @@ const pktio_if_ops_t * const pktio_if_ops[] = { #ifdef HAVE_PCAP _pktio_ops, #endif + _pktio_ops, _mmap_pktio_ops, _mmsg_pktio_ops, NULL diff --git a/platform/linux-generic/pktio/tap.c b/platform/linux-generic/pktio/tap.c new file mode 100644 index 000..7ecb300 --- /dev/null +++ b/platform/linux-generic/pktio/tap.c @@ -0,0 +1,327 @@ +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + * + * TAP pktio type + * + * This file provides a pk
Re: [lng-odp] [PATCHv3 2/3] linux-generic: pktio: add tap pktio type
On 08.12.2015 15:35, Stuart Haslam wrote: > On Tue, Dec 08, 2015 at 03:25:01PM +0300, Ilya Maximets wrote: >> >> >> On 08.12.2015 15:16, Stuart Haslam wrote: >>> On Tue, Dec 08, 2015 at 02:51:40PM +0300, Ilya Maximets wrote: >>>> On 08.12.2015 14:24, Stuart Haslam wrote: >>>>> On Mon, Dec 07, 2015 at 01:55:31PM +0300, Ilya Maximets wrote: >>>>>> Creates a new pktio type that allows for creating and >>>>>> sending/receiving packets through TAP interface. >>>>>> It is intended for use as a simple conventional communication >>>>>> method between applications that use kernel network stack >>>>>> (ping, ssh, iperf, etc.) and ODP applications for the purpose >>>>>> of functional testing and can be used as it is with some >>>>>> of the existing example applications. >>>>>> >>>>>> To use this interface the name passed to odp_pktio_open() must >>>>>> begin with "tap:" and be in the format: >>>>>> >>>>>> tap:iface >>>>>> >>>>>>iface the name of TAP device to be created. >>>>>> >>>>>> TUN/TAP kernel module should be loaded to use this pktio. >>>>>> There should be no device named 'iface' in the system. >>>>>> The total length of the 'iface' is limited by IF_NAMESIZE. >>>>>> >>>>>> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> >>>>>> --- >>>>>> platform/linux-generic/Makefile.am | 2 + >>>>>> .../linux-generic/include/odp_packet_io_internal.h | 3 + >>>>>> platform/linux-generic/include/odp_packet_tap.h| 21 ++ >>>>>> platform/linux-generic/pktio/io_ops.c | 1 + >>>>>> platform/linux-generic/pktio/tap.c | 317 >>>>>> + >>>>>> 5 files changed, 344 insertions(+) >>>>>> create mode 100644 platform/linux-generic/include/odp_packet_tap.h >>>>>> create mode 100644 platform/linux-generic/pktio/tap.c >>>>>> >>>>>> diff --git a/platform/linux-generic/Makefile.am >>>>>> b/platform/linux-generic/Makefile.am >>>>>> index 70bd8fe..4639ebc 100644 >>>>>> --- a/platform/linux-generic/Makefile.am >>>>>> +++ b/platform/linux-generic/Makefile.am >>>>>> @@ -92,6 +92,7 @@ noinst_HEADERS = \ >>>>>>${srcdir}/include/odp_packet_io_queue.h \ >>>>>>${srcdir}/include/odp_packet_netmap.h \ >>>>>>${srcdir}/include/odp_packet_socket.h \ >>>>>> + ${srcdir}/include/odp_packet_tap.h \ >>>>>>${srcdir}/include/odp_pool_internal.h \ >>>>>>${srcdir}/include/odp_queue_internal.h \ >>>>>>${srcdir}/include/odp_schedule_internal.h \ >>>>>> @@ -120,6 +121,7 @@ __LIB__libodp_la_SOURCES = \ >>>>>> pktio/netmap.c \ >>>>>> pktio/socket.c \ >>>>>> pktio/socket_mmap.c \ >>>>>> + pktio/tap.c \ >>>>>> odp_pool.c \ >>>>>> odp_queue.c \ >>>>>> odp_rwlock.c \ >>>>>> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h >>>>>> b/platform/linux-generic/include/odp_packet_io_internal.h >>>>>> index 1a1118c..de29557 100644 >>>>>> --- a/platform/linux-generic/include/odp_packet_io_internal.h >>>>>> +++ b/platform/linux-generic/include/odp_packet_io_internal.h >>>>>> @@ -22,6 +22,7 @@ extern "C" { >>>>>> #include >>>>>> #include >>>>>> #include >>>>>> +#include >>>>>> #include >>>>>> #include >>>>>> #include >>>>>> @@ -78,6 +79,7 @@ struct pktio_entry { >>>>>> #ifdef HAVE_PCAP >>>>>> pkt_pcap_t pkt_pcap;/**< Using pcap for IO >>>>>> */ >>>>>> #endif >>>>>&
Re: [lng-odp] validation: pktio: fix start_stop and send_failure tests
On 08.12.2015 17:38, Stuart Haslam wrote: > On Tue, Dec 08, 2015 at 12:21:06PM +, Elo, Matias (Nokia - FI/Espoo) > wrote: >> Hi Ilya, >> >> I had completely missed your previous patch. You could also remove the two >> errno value checks in pktio_test_send_failure(). They are not defined in the >> API, so they should not be tested. >> >> -Matias >> > > It realise it's back-to-front, but IMO it would be better to document > that the errno must be set. > I agree with that. There is so little set of functions, according to documentation, sets up errno. My opinion is that almost all functions should set it on errors, otherwise it useless. Best regards, Ilya Maximets. ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv4 0/3] TAP pktio
Creates a new pktio type that allows for creating and sending/receiving packets through TAP interface. Detailed description in commit-message of patch "[PATCHv4 2/3] linux-generic: pktio: add tap pktio type". Changelog: Version 4: * changed error handling part in tap_pktio_send. (Stuart Haslam) Version 3: * return 77 (TEST_SKIPPED) if user is not root. (Stuart Haslam) Version 2: * Validation tests added * Pktio tests fixed to work with real-world interfaces. * MAC of pktio now is not a kernel interface's MAC * Interfaces are UP after pktio_open() * Fixed getting mtu, getting/setting promisc mode * Misclenious fixes Ilya Maximets (3): validation: pktio: initialize mac addresses for all packets linux-generic: pktio: add tap pktio type linux-generic: pktio: add test for tap pktio platform/linux-generic/Makefile.am | 2 + .../linux-generic/include/odp_packet_io_internal.h | 3 + platform/linux-generic/include/odp_packet_tap.h| 21 ++ platform/linux-generic/pktio/io_ops.c | 1 + platform/linux-generic/pktio/tap.c | 327 + platform/linux-generic/test/Makefile.am| 1 + platform/linux-generic/test/pktio/Makefile.am | 3 +- platform/linux-generic/test/pktio/pktio_run_tap| 114 +++ test/validation/pktio/pktio.c | 46 ++- 9 files changed, 510 insertions(+), 8 deletions(-) create mode 100644 platform/linux-generic/include/odp_packet_tap.h create mode 100644 platform/linux-generic/pktio/tap.c create mode 100755 platform/linux-generic/test/pktio/pktio_run_tap -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCHv3] helper : Fix UDP checksum computation
_size + ODPH_UDPHDR_LEN); > udp->chksum = 0; > - udp->chksum = odph_ipv4_udp_chksum(test_packet); > + udp->chksum = odp_cpu_to_be_16(odph_ipv4_udp_chksum(test_packet)); > > if (udp->chksum == 0) > return -1; > > printf("chksum = 0x%x\n", udp->chksum); > > - if (udp->chksum != 0xab2d) > + if (udp->chksum != odp_be_to_cpu_16(0x7e5a)) 0x7e5a is in CPU endianness at compile time. You're using odp_be_to_cpu_16() that leads to converting that digit to BE endianness. That is why that code works. It works because udp->chksum is in BE and resulting digit in right side is also in BE. But, that is illogical to use function odp_be_to_cpu_16() to convert from CPU to BE. On 07.12.2015 19:27, Ion Grigore wrote: > > The comparison is done in the CPU endianness. NO, comparison is done in BE endianness. Best regards, Ilya Maximets. ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCH] linux-generic: socket: set up __odp_errno on ioctl failures
Also debug output enhanced. Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- platform/linux-generic/pktio/socket.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/platform/linux-generic/pktio/socket.c b/platform/linux-generic/pktio/socket.c index 5f5e0ae..ef2e031 100644 --- a/platform/linux-generic/pktio/socket.c +++ b/platform/linux-generic/pktio/socket.c @@ -126,7 +126,9 @@ int mtu_get_fd(int fd, const char *name) snprintf(ifr.ifr_name, IF_NAMESIZE, "%s", name); ret = ioctl(fd, SIOCGIFMTU, ); if (ret < 0) { - ODP_DBG("ioctl SIOCGIFMTU error\n"); + __odp_errno = errno; + ODP_DBG("ioctl(SIOCGIFMTU): %s: \"%s\".\n", strerror(errno), + ifr.ifr_name); return -1; } return ifr.ifr_mtu; @@ -145,7 +147,9 @@ int promisc_mode_set_fd(int fd, const char *name, int enable) snprintf(ifr.ifr_name, IF_NAMESIZE, "%s", name); ret = ioctl(fd, SIOCGIFFLAGS, ); if (ret < 0) { - ODP_DBG("ioctl SIOCGIFFLAGS error\n"); + __odp_errno = errno; + ODP_DBG("ioctl(SIOCGIFFLAGS): %s: \"%s\".\n", strerror(errno), + ifr.ifr_name); return -1; } @@ -156,7 +160,9 @@ int promisc_mode_set_fd(int fd, const char *name, int enable) ret = ioctl(fd, SIOCSIFFLAGS, ); if (ret < 0) { - ODP_DBG("ioctl SIOCSIFFLAGS error\n"); + __odp_errno = errno; + ODP_DBG("ioctl(SIOCSIFFLAGS): %s: \"%s\".\n", strerror(errno), + ifr.ifr_name); return -1; } return 0; @@ -175,7 +181,9 @@ int promisc_mode_get_fd(int fd, const char *name) snprintf(ifr.ifr_name, IF_NAMESIZE, "%s", name); ret = ioctl(fd, SIOCGIFFLAGS, ); if (ret < 0) { - ODP_DBG("ioctl SIOCGIFFLAGS error\n"); + __odp_errno = errno; + ODP_DBG("ioctl(SIOCGIFFLAGS): %s: \"%s\".\n", strerror(errno), + ifr.ifr_name); return -1; } -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCHv1] helper:test: Fix UDP checksum computation
On 07.12.2015 17:29, ion.grig...@freescale.com wrote: > From: Grigore Ion <ion.grig...@freescale.com> > > The UDP checksum is computed in the CPU endianness. The returned result > must be converted to the BE ordering when it is used to update the UDP > checksum in a packet. > > Signed-off-by: Grigore Ion <ion.grig...@freescale.com> > --- > v1: > Fix UDP checksum computation > > helper/test/odp_chksum.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/helper/test/odp_chksum.c b/helper/test/odp_chksum.c > index 1d417a8..2b32111 100644 > --- a/helper/test/odp_chksum.c > +++ b/helper/test/odp_chksum.c > @@ -189,14 +189,14 @@ int main(int argc TEST_UNUSED, char *argv[] TEST_UNUSED) > udp->dst_port = 0; > udp->length = odp_cpu_to_be_16(udat_size + ODPH_UDPHDR_LEN); > udp->chksum = 0; > - udp->chksum = odph_ipv4_udp_chksum(test_packet); > + udp->chksum = odp_cpu_to_be_16(odph_ipv4_udp_chksum(test_packet)); > > if (udp->chksum == 0) > return -1; > > printf("chksum = 0x%x\n", udp->chksum); > > - if (udp->chksum != 0xab2d) > + if (udp->chksum != odp_be_to_cpu_16(0x7e5a)) Isn't it illogical, that you're trying to compare BE integer with integer in CPU endianness? I mean odp_cpu_to_be_16() != odp_be_to_cpu_16() . Best regards, Ilya Maximets; > status = -1; > > odp_packet_free(test_packet); > ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv3 0/3] TAP pktio
Creates a new pktio type that allows for creating and sending/receiving packets through TAP interface. Detailed description in commit-message of patch "[PATCHv3 2/3] linux-generic: pktio: add tap pktio type". Changelog: Version 3: * return 77 (TEST_SKIPPED) if user is not root. (Stuart Haslam) Version 2: * Validation tests added * Pktio tests fixed to work with real-world interfaces. * MAC of pktio now is not a kernel interface's MAC * Interfaces are UP after pktio_open() * Fixed getting mtu, getting/setting promisc mode * Misclenious fixes Ilya Maximets (3): validation: pktio: initialize mac addresses for all packets linux-generic: pktio: add tap pktio type linux-generic: pktio: add test for tap pktio platform/linux-generic/Makefile.am | 2 + .../linux-generic/include/odp_packet_io_internal.h | 3 + platform/linux-generic/include/odp_packet_tap.h| 21 ++ platform/linux-generic/pktio/io_ops.c | 1 + platform/linux-generic/pktio/tap.c | 317 + platform/linux-generic/test/Makefile.am| 1 + platform/linux-generic/test/pktio/Makefile.am | 3 +- platform/linux-generic/test/pktio/pktio_run_tap| 114 test/validation/pktio/pktio.c | 46 ++- 9 files changed, 500 insertions(+), 8 deletions(-) create mode 100644 platform/linux-generic/include/odp_packet_tap.h create mode 100644 platform/linux-generic/pktio/tap.c create mode 100755 platform/linux-generic/test/pktio/pktio_run_tap -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv3 2/3] linux-generic: pktio: add tap pktio type
Creates a new pktio type that allows for creating and sending/receiving packets through TAP interface. It is intended for use as a simple conventional communication method between applications that use kernel network stack (ping, ssh, iperf, etc.) and ODP applications for the purpose of functional testing and can be used as it is with some of the existing example applications. To use this interface the name passed to odp_pktio_open() must begin with "tap:" and be in the format: tap:iface iface the name of TAP device to be created. TUN/TAP kernel module should be loaded to use this pktio. There should be no device named 'iface' in the system. The total length of the 'iface' is limited by IF_NAMESIZE. Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- platform/linux-generic/Makefile.am | 2 + .../linux-generic/include/odp_packet_io_internal.h | 3 + platform/linux-generic/include/odp_packet_tap.h| 21 ++ platform/linux-generic/pktio/io_ops.c | 1 + platform/linux-generic/pktio/tap.c | 317 + 5 files changed, 344 insertions(+) create mode 100644 platform/linux-generic/include/odp_packet_tap.h create mode 100644 platform/linux-generic/pktio/tap.c diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index 70bd8fe..4639ebc 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -92,6 +92,7 @@ noinst_HEADERS = \ ${srcdir}/include/odp_packet_io_queue.h \ ${srcdir}/include/odp_packet_netmap.h \ ${srcdir}/include/odp_packet_socket.h \ + ${srcdir}/include/odp_packet_tap.h \ ${srcdir}/include/odp_pool_internal.h \ ${srcdir}/include/odp_queue_internal.h \ ${srcdir}/include/odp_schedule_internal.h \ @@ -120,6 +121,7 @@ __LIB__libodp_la_SOURCES = \ pktio/netmap.c \ pktio/socket.c \ pktio/socket_mmap.c \ + pktio/tap.c \ odp_pool.c \ odp_queue.c \ odp_rwlock.c \ diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h index 1a1118c..de29557 100644 --- a/platform/linux-generic/include/odp_packet_io_internal.h +++ b/platform/linux-generic/include/odp_packet_io_internal.h @@ -22,6 +22,7 @@ extern "C" { #include #include #include +#include #include #include #include @@ -78,6 +79,7 @@ struct pktio_entry { #ifdef HAVE_PCAP pkt_pcap_t pkt_pcap;/**< Using pcap for IO */ #endif + pkt_tap_t pkt_tap; /**< using TAP for IO */ }; enum { STATE_START = 0, @@ -157,6 +159,7 @@ extern const pktio_if_ops_t loopback_pktio_ops; #ifdef HAVE_PCAP extern const pktio_if_ops_t pcap_pktio_ops; #endif +extern const pktio_if_ops_t tap_pktio_ops; extern const pktio_if_ops_t * const pktio_if_ops[]; #ifdef __cplusplus diff --git a/platform/linux-generic/include/odp_packet_tap.h b/platform/linux-generic/include/odp_packet_tap.h new file mode 100644 index 000..7877586 --- /dev/null +++ b/platform/linux-generic/include/odp_packet_tap.h @@ -0,0 +1,21 @@ +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#ifndef ODP_PACKET_TAP_H_ +#define ODP_PACKET_TAP_H_ + +#include + +typedef struct { + int fd; /**< file descriptor for tap interface*/ + int skfd; /**< socket descriptor */ + uint32_t mtu; /**< cached mtu */ + unsigned char if_mac[ETH_ALEN]; /**< MAC address of pktio side (not a +MAC address of kernel interface)*/ + odp_pool_t pool;/**< pool to alloc packets from */ +} pkt_tap_t; + +#endif diff --git a/platform/linux-generic/pktio/io_ops.c b/platform/linux-generic/pktio/io_ops.c index 3b344e6..1933abc 100644 --- a/platform/linux-generic/pktio/io_ops.c +++ b/platform/linux-generic/pktio/io_ops.c @@ -18,6 +18,7 @@ const pktio_if_ops_t * const pktio_if_ops[] = { #ifdef HAVE_PCAP _pktio_ops, #endif + _pktio_ops, _mmap_pktio_ops, _mmsg_pktio_ops, NULL diff --git a/platform/linux-generic/pktio/tap.c b/platform/linux-generic/pktio/tap.c new file mode 100644 index 000..b11e64f --- /dev/null +++ b/platform/linux-generic/pktio/tap.c @@ -0,0 +1,317 @@ +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + * + * TAP pktio type + * + * This file provides a pk
Re: [lng-odp] [PATCHv2 3/3] linux-generic: pktio: add test for tap pktio
On 07.12.2015 13:01, Stuart Haslam wrote: > On Wed, Dec 02, 2015 at 04:13:25PM +0300, Ilya Maximets wrote: >> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> >> --- >> platform/linux-generic/test/Makefile.am | 1 + >> platform/linux-generic/test/pktio/Makefile.am | 3 +- >> platform/linux-generic/test/pktio/pktio_run_tap | 111 >> >> 3 files changed, 114 insertions(+), 1 deletion(-) >> create mode 100755 platform/linux-generic/test/pktio/pktio_run_tap >> >> diff --git a/platform/linux-generic/test/Makefile.am >> b/platform/linux-generic/test/Makefile.am >> index 1475589..28bd3dc 100644 >> --- a/platform/linux-generic/test/Makefile.am >> +++ b/platform/linux-generic/test/Makefile.am >> @@ -5,6 +5,7 @@ ODP_MODULES = pktio >> >> if test_vald >> TESTS = pktio/pktio_run \ >> +pktio/pktio_run_tap \ >> ${top_builddir}/test/validation/buffer/buffer_main$(EXEEXT) \ >> >> ${top_builddir}/test/validation/classification/classification_main$(EXEEXT) \ >> ${top_builddir}/test/validation/cpumask/cpumask_main$(EXEEXT) \ >> diff --git a/platform/linux-generic/test/pktio/Makefile.am >> b/platform/linux-generic/test/pktio/Makefile.am >> index 4d39372..ea0ad87 100644 >> --- a/platform/linux-generic/test/pktio/Makefile.am >> +++ b/platform/linux-generic/test/pktio/Makefile.am >> @@ -1,5 +1,6 @@ >> dist_check_SCRIPTS = pktio_env \ >> - pktio_run >> + pktio_run \ >> + pktio_run_tap >> >> if HAVE_PCAP >> dist_check_SCRIPTS += pktio_run_pcap >> diff --git a/platform/linux-generic/test/pktio/pktio_run_tap >> b/platform/linux-generic/test/pktio/pktio_run_tap >> new file mode 100755 >> index 000..4e2526f >> --- /dev/null >> +++ b/platform/linux-generic/test/pktio/pktio_run_tap >> @@ -0,0 +1,111 @@ >> +#!/bin/sh >> +# >> +# Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> >> +# All rights reserved. >> +# >> +# SPDX-License-Identifier: BSD-3-Clause >> +# >> + >> +# directories where pktio_main binary can be found: >> +# -in the validation dir when running make check (intree or out of tree) >> +# -in the script directory, when running after 'make install', or >> +# -in the validation when running standalone intree. >> +# -in the current directory. >> +# running stand alone out of tree requires setting PATH >> +PATH=${TEST_DIR}/pktio:$PATH >> +PATH=$(dirname $0):$PATH >> +PATH=$(dirname $0)/../../../../test/validation/pktio:$PATH >> +PATH=.:$PATH >> + >> +pktio_main_path=$(which pktio_main${EXEEXT}) >> +if [ -x "$pktio_main_path" ] ; then >> +echo "running with $pktio_main_path" >> +else >> +echo "cannot find pktio_main${EXEEXT}: please set you PATH for it." >> +fi >> + >> +TAP_BASE_NAME=iotap_vald >> +IF0=${TAP_BASE_NAME}0 >> +IF1=${TAP_BASE_NAME}1 >> +BR=${TAP_BASE_NAME}_br >> + >> +export ODP_PKTIO_IF0="tap:$IF0" >> +export ODP_PKTIO_IF1="tap:$IF1" >> + >> +tap_cleanup() >> +{ >> +ret=$? >> + >> +for iface in $IF0 $IF1; do >> +ip link set dev $iface nomaster >> +done >> + >> +ip link delete $BR type bridge >> + >> +for iface in $IF0 $IF1; do >> +ip tuntap del mode tap $iface >> +done >> + >> +trap - EXIT >> +exit $ret >> +} >> + >> +tap_setup() >> +{ >> +if [ "$(id -u)" != "0" ]; then >> +echo "pktio: need to be root to setup TAP interfaces." >> +return 1 > > This needs to exit with a code of 77 so that the test gets recorded as > SKIPped rather than FAILed. > > -- > Stuart. > > Thanks. New version posted. Best regards, Ilya Maximets. ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv3 1/3] validation: pktio: initialize mac addresses for all packets
For the purpose of testing of real-world interfaces the packet's content should be valid or kernel will throw them away. Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- test/validation/pktio/pktio.c | 46 --- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index 146e741..b0deac7 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -86,16 +86,16 @@ static void set_pool_len(odp_pool_param_t *params) } static void pktio_pkt_set_macs(odp_packet_t pkt, - pktio_info_t *src, pktio_info_t *dst) + odp_pktio_t src, odp_pktio_t dst) { uint32_t len; odph_ethhdr_t *eth = (odph_ethhdr_t *)odp_packet_l2_ptr(pkt, ); int ret; - ret = odp_pktio_mac_addr(src->id, >src, sizeof(eth->src)); + ret = odp_pktio_mac_addr(src, >src, sizeof(eth->src)); CU_ASSERT(ret == ODPH_ETHADDR_LEN); - ret = odp_pktio_mac_addr(dst->id, >dst, sizeof(eth->dst)); + ret = odp_pktio_mac_addr(dst, >dst, sizeof(eth->dst)); CU_ASSERT(ret == ODPH_ETHADDR_LEN); } @@ -410,12 +410,16 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, pktio_info_t *pktio_b, break; tx_seq[i] = pktio_init_packet(tx_pkt[i]); - if (tx_seq[i] == TEST_SEQ_INVALID) + if (tx_seq[i] == TEST_SEQ_INVALID) { + odp_packet_free(tx_pkt[i]); break; + } - pktio_pkt_set_macs(tx_pkt[i], pktio_a, pktio_b); - if (pktio_fixup_checksums(tx_pkt[i]) != 0) + pktio_pkt_set_macs(tx_pkt[i], pktio_a->id, pktio_b->id); + if (pktio_fixup_checksums(tx_pkt[i]) != 0) { + odp_packet_free(tx_pkt[i]); break; + } tx_ev[i] = odp_packet_to_event(tx_pkt[i]); } @@ -724,6 +728,13 @@ static void pktio_test_start_stop(void) if (pkt == ODP_PACKET_INVALID) break; pktio_init_packet(pkt); + + pktio_pkt_set_macs(pkt, pktio[0], pktio[1]); + if (pktio_fixup_checksums(pkt) != 0) { + odp_packet_free(pkt); + break; + } + tx_ev[alloc] = odp_packet_to_event(pkt); } @@ -775,6 +786,13 @@ static void pktio_test_start_stop(void) if (pkt == ODP_PACKET_INVALID) break; pktio_init_packet(pkt); + if (num_ifaces > 1) { + pktio_pkt_set_macs(pkt, pktio[0], pktio[1]); + if (pktio_fixup_checksums(pkt) != 0) { + odp_packet_free(pkt); + break; + } + } tx_ev[alloc] = odp_packet_to_event(pkt); } @@ -896,8 +914,17 @@ static void pktio_test_send_failure(void) break; pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); - if (pkt_seq[i] == TEST_SEQ_INVALID) + + pktio_pkt_set_macs(pkt_tbl[i], pktio_tx, pktio_rx); + if (pktio_fixup_checksums(pkt_tbl[i]) != 0) { + odp_packet_free(pkt_tbl[i]); + break; + } + + if (pkt_seq[i] == TEST_SEQ_INVALID) { + odp_packet_free(pkt_tbl[i]); break; + } } alloc_pkts = i; @@ -941,6 +968,11 @@ static void pktio_test_send_failure(void) odp_packet_len(pkt_tbl[i]) - PKT_LEN_NORMAL); pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); + + pktio_pkt_set_macs(pkt_tbl[i], pktio_tx, pktio_rx); + ret = pktio_fixup_checksums(pkt_tbl[i]); + CU_ASSERT_FATAL(ret == 0); + CU_ASSERT_FATAL(pkt_seq[i] != TEST_SEQ_INVALID); ret = odp_pktio_send(pktio_tx, _tbl[i], TX_BATCH_LEN - i); CU_ASSERT_FATAL(ret == (TX_BATCH_LEN - i)); -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv2 1/3] validation: pktio: initialize mac addresses for all packets
For the purpose of testing of real-world interfaces the packet's content should be valid or kernel will throw them away. Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- test/validation/pktio/pktio.c | 46 --- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c index 146e741..b0deac7 100644 --- a/test/validation/pktio/pktio.c +++ b/test/validation/pktio/pktio.c @@ -86,16 +86,16 @@ static void set_pool_len(odp_pool_param_t *params) } static void pktio_pkt_set_macs(odp_packet_t pkt, - pktio_info_t *src, pktio_info_t *dst) + odp_pktio_t src, odp_pktio_t dst) { uint32_t len; odph_ethhdr_t *eth = (odph_ethhdr_t *)odp_packet_l2_ptr(pkt, ); int ret; - ret = odp_pktio_mac_addr(src->id, >src, sizeof(eth->src)); + ret = odp_pktio_mac_addr(src, >src, sizeof(eth->src)); CU_ASSERT(ret == ODPH_ETHADDR_LEN); - ret = odp_pktio_mac_addr(dst->id, >dst, sizeof(eth->dst)); + ret = odp_pktio_mac_addr(dst, >dst, sizeof(eth->dst)); CU_ASSERT(ret == ODPH_ETHADDR_LEN); } @@ -410,12 +410,16 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, pktio_info_t *pktio_b, break; tx_seq[i] = pktio_init_packet(tx_pkt[i]); - if (tx_seq[i] == TEST_SEQ_INVALID) + if (tx_seq[i] == TEST_SEQ_INVALID) { + odp_packet_free(tx_pkt[i]); break; + } - pktio_pkt_set_macs(tx_pkt[i], pktio_a, pktio_b); - if (pktio_fixup_checksums(tx_pkt[i]) != 0) + pktio_pkt_set_macs(tx_pkt[i], pktio_a->id, pktio_b->id); + if (pktio_fixup_checksums(tx_pkt[i]) != 0) { + odp_packet_free(tx_pkt[i]); break; + } tx_ev[i] = odp_packet_to_event(tx_pkt[i]); } @@ -724,6 +728,13 @@ static void pktio_test_start_stop(void) if (pkt == ODP_PACKET_INVALID) break; pktio_init_packet(pkt); + + pktio_pkt_set_macs(pkt, pktio[0], pktio[1]); + if (pktio_fixup_checksums(pkt) != 0) { + odp_packet_free(pkt); + break; + } + tx_ev[alloc] = odp_packet_to_event(pkt); } @@ -775,6 +786,13 @@ static void pktio_test_start_stop(void) if (pkt == ODP_PACKET_INVALID) break; pktio_init_packet(pkt); + if (num_ifaces > 1) { + pktio_pkt_set_macs(pkt, pktio[0], pktio[1]); + if (pktio_fixup_checksums(pkt) != 0) { + odp_packet_free(pkt); + break; + } + } tx_ev[alloc] = odp_packet_to_event(pkt); } @@ -896,8 +914,17 @@ static void pktio_test_send_failure(void) break; pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); - if (pkt_seq[i] == TEST_SEQ_INVALID) + + pktio_pkt_set_macs(pkt_tbl[i], pktio_tx, pktio_rx); + if (pktio_fixup_checksums(pkt_tbl[i]) != 0) { + odp_packet_free(pkt_tbl[i]); + break; + } + + if (pkt_seq[i] == TEST_SEQ_INVALID) { + odp_packet_free(pkt_tbl[i]); break; + } } alloc_pkts = i; @@ -941,6 +968,11 @@ static void pktio_test_send_failure(void) odp_packet_len(pkt_tbl[i]) - PKT_LEN_NORMAL); pkt_seq[i] = pktio_init_packet(pkt_tbl[i]); + + pktio_pkt_set_macs(pkt_tbl[i], pktio_tx, pktio_rx); + ret = pktio_fixup_checksums(pkt_tbl[i]); + CU_ASSERT_FATAL(ret == 0); + CU_ASSERT_FATAL(pkt_seq[i] != TEST_SEQ_INVALID); ret = odp_pktio_send(pktio_tx, _tbl[i], TX_BATCH_LEN - i); CU_ASSERT_FATAL(ret == (TX_BATCH_LEN - i)); -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv2 0/3] TAP pktio
Creates a new pktio type that allows for creating and sending/receiving packets through TAP interface. Detailed description in commit-message of patch "[PATCHv2 2/3] linux-generic: pktio: add tap pktio type". Changelog: Version 2: * Validation tests added * Pktio tests fixed to work with real-world interfaces. * MAC of pktio now is not a kernel interface's MAC * Interfaces are UP after pktio_open() * Fixed getting mtu, getting/setting promisc mode * Misclenious fixes Ilya Maximets (3): validation: pktio: initialize mac addresses for all packets linux-generic: pktio: add tap pktio type linux-generic: pktio: add test for tap pktio platform/linux-generic/Makefile.am | 2 + .../linux-generic/include/odp_packet_io_internal.h | 3 + platform/linux-generic/include/odp_packet_tap.h| 21 ++ platform/linux-generic/pktio/io_ops.c | 1 + platform/linux-generic/pktio/tap.c | 317 + platform/linux-generic/test/Makefile.am| 1 + platform/linux-generic/test/pktio/Makefile.am | 3 +- platform/linux-generic/test/pktio/pktio_run_tap| 111 test/validation/pktio/pktio.c | 46 ++- 9 files changed, 497 insertions(+), 8 deletions(-) create mode 100644 platform/linux-generic/include/odp_packet_tap.h create mode 100644 platform/linux-generic/pktio/tap.c create mode 100755 platform/linux-generic/test/pktio/pktio_run_tap -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
[lng-odp] [PATCHv2 2/3] linux-generic: pktio: add tap pktio type
Creates a new pktio type that allows for creating and sending/receiving packets through TAP interface. It is intended for use as a simple conventional communication method between applications that use kernel network stack (ping, ssh, iperf, etc.) and ODP applications for the purpose of functional testing and can be used as it is with some of the existing example applications. To use this interface the name passed to odp_pktio_open() must begin with "tap:" and be in the format: tap:iface iface the name of TAP device to be created. TUN/TAP kernel module should be loaded to use this pktio. There should be no device named 'iface' in the system. The total length of the 'iface' is limited by IF_NAMESIZE. Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- platform/linux-generic/Makefile.am | 2 + .../linux-generic/include/odp_packet_io_internal.h | 3 + platform/linux-generic/include/odp_packet_tap.h| 21 ++ platform/linux-generic/pktio/io_ops.c | 1 + platform/linux-generic/pktio/tap.c | 317 + 5 files changed, 344 insertions(+) create mode 100644 platform/linux-generic/include/odp_packet_tap.h create mode 100644 platform/linux-generic/pktio/tap.c diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index 70bd8fe..4639ebc 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -92,6 +92,7 @@ noinst_HEADERS = \ ${srcdir}/include/odp_packet_io_queue.h \ ${srcdir}/include/odp_packet_netmap.h \ ${srcdir}/include/odp_packet_socket.h \ + ${srcdir}/include/odp_packet_tap.h \ ${srcdir}/include/odp_pool_internal.h \ ${srcdir}/include/odp_queue_internal.h \ ${srcdir}/include/odp_schedule_internal.h \ @@ -120,6 +121,7 @@ __LIB__libodp_la_SOURCES = \ pktio/netmap.c \ pktio/socket.c \ pktio/socket_mmap.c \ + pktio/tap.c \ odp_pool.c \ odp_queue.c \ odp_rwlock.c \ diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h index 1a1118c..de29557 100644 --- a/platform/linux-generic/include/odp_packet_io_internal.h +++ b/platform/linux-generic/include/odp_packet_io_internal.h @@ -22,6 +22,7 @@ extern "C" { #include #include #include +#include #include #include #include @@ -78,6 +79,7 @@ struct pktio_entry { #ifdef HAVE_PCAP pkt_pcap_t pkt_pcap;/**< Using pcap for IO */ #endif + pkt_tap_t pkt_tap; /**< using TAP for IO */ }; enum { STATE_START = 0, @@ -157,6 +159,7 @@ extern const pktio_if_ops_t loopback_pktio_ops; #ifdef HAVE_PCAP extern const pktio_if_ops_t pcap_pktio_ops; #endif +extern const pktio_if_ops_t tap_pktio_ops; extern const pktio_if_ops_t * const pktio_if_ops[]; #ifdef __cplusplus diff --git a/platform/linux-generic/include/odp_packet_tap.h b/platform/linux-generic/include/odp_packet_tap.h new file mode 100644 index 000..7877586 --- /dev/null +++ b/platform/linux-generic/include/odp_packet_tap.h @@ -0,0 +1,21 @@ +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#ifndef ODP_PACKET_TAP_H_ +#define ODP_PACKET_TAP_H_ + +#include + +typedef struct { + int fd; /**< file descriptor for tap interface*/ + int skfd; /**< socket descriptor */ + uint32_t mtu; /**< cached mtu */ + unsigned char if_mac[ETH_ALEN]; /**< MAC address of pktio side (not a +MAC address of kernel interface)*/ + odp_pool_t pool;/**< pool to alloc packets from */ +} pkt_tap_t; + +#endif diff --git a/platform/linux-generic/pktio/io_ops.c b/platform/linux-generic/pktio/io_ops.c index 3b344e6..1933abc 100644 --- a/platform/linux-generic/pktio/io_ops.c +++ b/platform/linux-generic/pktio/io_ops.c @@ -18,6 +18,7 @@ const pktio_if_ops_t * const pktio_if_ops[] = { #ifdef HAVE_PCAP _pktio_ops, #endif + _pktio_ops, _mmap_pktio_ops, _mmsg_pktio_ops, NULL diff --git a/platform/linux-generic/pktio/tap.c b/platform/linux-generic/pktio/tap.c new file mode 100644 index 000..b11e64f --- /dev/null +++ b/platform/linux-generic/pktio/tap.c @@ -0,0 +1,317 @@ +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + * + * TAP pktio type + * + * This file provides a pk
[lng-odp] [PATCHv2 3/3] linux-generic: pktio: add test for tap pktio
Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- platform/linux-generic/test/Makefile.am | 1 + platform/linux-generic/test/pktio/Makefile.am | 3 +- platform/linux-generic/test/pktio/pktio_run_tap | 111 3 files changed, 114 insertions(+), 1 deletion(-) create mode 100755 platform/linux-generic/test/pktio/pktio_run_tap diff --git a/platform/linux-generic/test/Makefile.am b/platform/linux-generic/test/Makefile.am index 1475589..28bd3dc 100644 --- a/platform/linux-generic/test/Makefile.am +++ b/platform/linux-generic/test/Makefile.am @@ -5,6 +5,7 @@ ODP_MODULES = pktio if test_vald TESTS = pktio/pktio_run \ + pktio/pktio_run_tap \ ${top_builddir}/test/validation/buffer/buffer_main$(EXEEXT) \ ${top_builddir}/test/validation/classification/classification_main$(EXEEXT) \ ${top_builddir}/test/validation/cpumask/cpumask_main$(EXEEXT) \ diff --git a/platform/linux-generic/test/pktio/Makefile.am b/platform/linux-generic/test/pktio/Makefile.am index 4d39372..ea0ad87 100644 --- a/platform/linux-generic/test/pktio/Makefile.am +++ b/platform/linux-generic/test/pktio/Makefile.am @@ -1,5 +1,6 @@ dist_check_SCRIPTS = pktio_env \ -pktio_run +pktio_run \ +pktio_run_tap if HAVE_PCAP dist_check_SCRIPTS += pktio_run_pcap diff --git a/platform/linux-generic/test/pktio/pktio_run_tap b/platform/linux-generic/test/pktio/pktio_run_tap new file mode 100755 index 000..4e2526f --- /dev/null +++ b/platform/linux-generic/test/pktio/pktio_run_tap @@ -0,0 +1,111 @@ +#!/bin/sh +# +# Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause +# + +# directories where pktio_main binary can be found: +# -in the validation dir when running make check (intree or out of tree) +# -in the script directory, when running after 'make install', or +# -in the validation when running standalone intree. +# -in the current directory. +# running stand alone out of tree requires setting PATH +PATH=${TEST_DIR}/pktio:$PATH +PATH=$(dirname $0):$PATH +PATH=$(dirname $0)/../../../../test/validation/pktio:$PATH +PATH=.:$PATH + +pktio_main_path=$(which pktio_main${EXEEXT}) +if [ -x "$pktio_main_path" ] ; then + echo "running with $pktio_main_path" +else + echo "cannot find pktio_main${EXEEXT}: please set you PATH for it." +fi + +TAP_BASE_NAME=iotap_vald +IF0=${TAP_BASE_NAME}0 +IF1=${TAP_BASE_NAME}1 +BR=${TAP_BASE_NAME}_br + +export ODP_PKTIO_IF0="tap:$IF0" +export ODP_PKTIO_IF1="tap:$IF1" + +tap_cleanup() +{ + ret=$? + + for iface in $IF0 $IF1; do + ip link set dev $iface nomaster + done + + ip link delete $BR type bridge + + for iface in $IF0 $IF1; do + ip tuntap del mode tap $iface + done + + trap - EXIT + exit $ret +} + +tap_setup() +{ + if [ "$(id -u)" != "0" ]; then + echo "pktio: need to be root to setup TAP interfaces." + return 1 + fi + + for iface in $IF0 $IF1 $BR; do + ip link show $iface 2> /dev/null + if [ $? -eq 0 ]; then + echo "pktio: interface $iface already exist $?" + return 2 + fi + done + + trap tap_cleanup EXIT + + for iface in $IF0 $IF1; do + ip tuntap add mode tap $iface + if [ $? -ne 0 ]; then + echo "pktio: error: unable to create TAP device $iface" + return 3 + fi + done + + ip link add name $BR type bridge + if [ $? -ne 0 ]; then + echo "pktio: error: unable to create bridge $BR" + return 3 + fi + + for iface in $IF0 $IF1; do + ip link set dev $iface master $BR + if [ $? -ne 0 ]; then + echo "pktio: error: unable to add $iface to bridge $BR" + return 4 + fi + done + + for iface in $IF0 $IF1 $BR; do + ifconfig $iface -arp + sysctl -w net.ipv6.conf.${iface}.disable_ipv6=1 + ip link set dev $iface mtu 9216 up + done + + return 0 +} + +tap_setup +ret=$? +if [ $ret -ne 0 ]; then + echo "pktio: tap_setup() FAILED!" + exit $ret +fi + +pktio_main${EXEEXT} +ret=$? + +exit $ret -- 2.1.4 ___ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
Re: [lng-odp] [PATCH] linux-generic: pktio: add tap pktio type
OK. I try to prepare them. Best regards, Ilya Maximets. On 27.11.2015 13:19, Maxim Uvarov wrote: > Hello Ilya, > > Thanks for sending that patch. Tap pktio should be interesting. > To accept it we also need coverage validation tests. > > Best regards, > Maxim. > > > On 11/25/2015 17:15, Ilya Maximets wrote: >> Creates a new pktio type that allows for creating and >> sending/receiving packets through TAP interface. >> It is intended for use as a simple conventional communication >> method between applications that use kernel network stack >> (ping, ssh, iperf, etc.) and ODP applications for the purpose >> of functional testing and can be used as it is with some >> of the existing example applications. >> >> To use this interface the name passed to odp_pktio_open() must >> begin with "tap:" and be in the format: >> >> tap:iface >> >> iface the name of TAP device to be created. >> >> TUN/TAP kernel module should be loaded to use this pktio. >> There should be no device named 'iface' in the system. >> The total length of the 'iface' is limited by IF_NAMESIZE. >> >> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> >> --- >> platform/linux-generic/Makefile.am | 2 + >> .../linux-generic/include/odp_packet_io_internal.h | 3 + >> platform/linux-generic/include/odp_packet_tap.h| 17 ++ >> platform/linux-generic/pktio/io_ops.c | 1 + >> platform/linux-generic/pktio/tap.c | 251 >> + >> 5 files changed, 274 insertions(+) >> create mode 100644 platform/linux-generic/include/odp_packet_tap.h >> create mode 100644 platform/linux-generic/pktio/tap.c >> >> diff --git a/platform/linux-generic/Makefile.am >> b/platform/linux-generic/Makefile.am >> index 610e04d..8395bda 100644 >> --- a/platform/linux-generic/Makefile.am >> +++ b/platform/linux-generic/Makefile.am >> @@ -91,6 +91,7 @@ noinst_HEADERS = \ >>${srcdir}/include/odp_packet_io_queue.h \ >>${srcdir}/include/odp_packet_netmap.h \ >>${srcdir}/include/odp_packet_socket.h \ >> + ${srcdir}/include/odp_packet_tap.h \ >>${srcdir}/include/odp_pool_internal.h \ >>${srcdir}/include/odp_queue_internal.h \ >>${srcdir}/include/odp_schedule_internal.h \ >> @@ -119,6 +120,7 @@ __LIB__libodp_la_SOURCES = \ >> pktio/netmap.c \ >> pktio/socket.c \ >> pktio/socket_mmap.c \ >> + pktio/tap.c \ >> odp_pool.c \ >> odp_queue.c \ >> odp_rwlock.c \ >> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h >> b/platform/linux-generic/include/odp_packet_io_internal.h >> index 1a1118c..de29557 100644 >> --- a/platform/linux-generic/include/odp_packet_io_internal.h >> +++ b/platform/linux-generic/include/odp_packet_io_internal.h >> @@ -22,6 +22,7 @@ extern "C" { >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -78,6 +79,7 @@ struct pktio_entry { >> #ifdef HAVE_PCAP >> pkt_pcap_t pkt_pcap;/**< Using pcap for IO */ >> #endif >> +pkt_tap_t pkt_tap; /**< using TAP for IO */ >> }; >> enum { >> STATE_START = 0, >> @@ -157,6 +159,7 @@ extern const pktio_if_ops_t loopback_pktio_ops; >> #ifdef HAVE_PCAP >> extern const pktio_if_ops_t pcap_pktio_ops; >> #endif >> +extern const pktio_if_ops_t tap_pktio_ops; >> extern const pktio_if_ops_t * const pktio_if_ops[]; >> >> #ifdef __cplusplus >> diff --git a/platform/linux-generic/include/odp_packet_tap.h >> b/platform/linux-generic/include/odp_packet_tap.h >> new file mode 100644 >> index 000..2d442fb >> --- /dev/null >> +++ b/platform/linux-generic/include/odp_packet_tap.h >> @@ -0,0 +1,17 @@ >> +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> >> + * All rights reserved. >> + * >> + * SPDX-License-Identifier: BSD-3-Clause >> + */ >> + >> +#ifndef ODP_PACKET_TAP_H_ >> +#define ODP_PACKET_TAP_H_ >> + >> +#include >> + >> +typedef struct { >> +int fd; /**< file descriptor for tap interface */ >> +odp_pool
[lng-odp] [PATCH] linux-generic: pktio: add tap pktio type
Creates a new pktio type that allows for creating and sending/receiving packets through TAP interface. It is intended for use as a simple conventional communication method between applications that use kernel network stack (ping, ssh, iperf, etc.) and ODP applications for the purpose of functional testing and can be used as it is with some of the existing example applications. To use this interface the name passed to odp_pktio_open() must begin with "tap:" and be in the format: tap:iface iface the name of TAP device to be created. TUN/TAP kernel module should be loaded to use this pktio. There should be no device named 'iface' in the system. The total length of the 'iface' is limited by IF_NAMESIZE. Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> --- platform/linux-generic/Makefile.am | 2 + .../linux-generic/include/odp_packet_io_internal.h | 3 + platform/linux-generic/include/odp_packet_tap.h| 17 ++ platform/linux-generic/pktio/io_ops.c | 1 + platform/linux-generic/pktio/tap.c | 251 + 5 files changed, 274 insertions(+) create mode 100644 platform/linux-generic/include/odp_packet_tap.h create mode 100644 platform/linux-generic/pktio/tap.c diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index 610e04d..8395bda 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -91,6 +91,7 @@ noinst_HEADERS = \ ${srcdir}/include/odp_packet_io_queue.h \ ${srcdir}/include/odp_packet_netmap.h \ ${srcdir}/include/odp_packet_socket.h \ + ${srcdir}/include/odp_packet_tap.h \ ${srcdir}/include/odp_pool_internal.h \ ${srcdir}/include/odp_queue_internal.h \ ${srcdir}/include/odp_schedule_internal.h \ @@ -119,6 +120,7 @@ __LIB__libodp_la_SOURCES = \ pktio/netmap.c \ pktio/socket.c \ pktio/socket_mmap.c \ + pktio/tap.c \ odp_pool.c \ odp_queue.c \ odp_rwlock.c \ diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h index 1a1118c..de29557 100644 --- a/platform/linux-generic/include/odp_packet_io_internal.h +++ b/platform/linux-generic/include/odp_packet_io_internal.h @@ -22,6 +22,7 @@ extern "C" { #include #include #include +#include #include #include #include @@ -78,6 +79,7 @@ struct pktio_entry { #ifdef HAVE_PCAP pkt_pcap_t pkt_pcap;/**< Using pcap for IO */ #endif + pkt_tap_t pkt_tap; /**< using TAP for IO */ }; enum { STATE_START = 0, @@ -157,6 +159,7 @@ extern const pktio_if_ops_t loopback_pktio_ops; #ifdef HAVE_PCAP extern const pktio_if_ops_t pcap_pktio_ops; #endif +extern const pktio_if_ops_t tap_pktio_ops; extern const pktio_if_ops_t * const pktio_if_ops[]; #ifdef __cplusplus diff --git a/platform/linux-generic/include/odp_packet_tap.h b/platform/linux-generic/include/odp_packet_tap.h new file mode 100644 index 000..2d442fb --- /dev/null +++ b/platform/linux-generic/include/odp_packet_tap.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#ifndef ODP_PACKET_TAP_H_ +#define ODP_PACKET_TAP_H_ + +#include + +typedef struct { + int fd; /**< file descriptor for tap interface */ + odp_pool_t pool;/**< pool to alloc packets from */ +} pkt_tap_t; + +#endif diff --git a/platform/linux-generic/pktio/io_ops.c b/platform/linux-generic/pktio/io_ops.c index 3b344e6..1933abc 100644 --- a/platform/linux-generic/pktio/io_ops.c +++ b/platform/linux-generic/pktio/io_ops.c @@ -18,6 +18,7 @@ const pktio_if_ops_t * const pktio_if_ops[] = { #ifdef HAVE_PCAP _pktio_ops, #endif + _pktio_ops, _mmap_pktio_ops, _mmsg_pktio_ops, NULL diff --git a/platform/linux-generic/pktio/tap.c b/platform/linux-generic/pktio/tap.c new file mode 100644 index 000..e629986 --- /dev/null +++ b/platform/linux-generic/pktio/tap.c @@ -0,0 +1,251 @@ +/* Copyright (c) 2015, Ilya Maximets <i.maxim...@samsung.com> + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + * + * TAP pktio type + * + * This file provides a pktio interface that allows for creating and + * send/receive packets through TAP interface. It is intended for use + * as a simple conventional communication method between applications + * that use kernel network stack (ping, ssh, iperf, etc.) and ODP + * applications for the purpose of functional test