Message ID | 5554E390.9050308@linaro.org |
---|---|
State | New |
Headers | show |
Hi Maxim, what kind of machine you used for testing? I used a BE machine. Alex On 14 May 2015 at 21:04, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Hello Alexandru, > > Your patch fixes check summ now that function is better: > > > 20:49:45.613467 IP (tos 0x0, id 35630, offset 0, flags [none], proto UDP > (17), length 84) > 192.168.0.1.0 > 192.168.0.2.0: [bad udp cksum 0x1a7e -> 0x7e1a!] UDP, > length 56 > 20:49:45.613470 IP (tos 0x0, id 35662, offset 0, flags [none], proto UDP > (17), length 84) > 192.168.0.1.0 > 192.168.0.2.0: [bad udp cksum 0x1a7e -> 0x7e1a!] UDP, > length 56 > > I used packets generated with our odp_generator. > > After that fix check summ is valid: > > --- a/example/generator/odp_generator.c > +++ b/example/generator/odp_generator.c > @@ -238,7 +238,6 @@ 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; > > That will work on both BE/LE right? > > Thanks, > Maxim. > > > > > On 05/05/2015 15:07, alexandru.badicioiu@linaro.org wrote: > >> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >> >> v1 - fixed compile error due to -Wcast-align >> >> Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >> --- >> helper/include/odp/helper/udp.h | 37 >> +++++++++++++------------------------ >> 1 files changed, 13 insertions(+), 24 deletions(-) >> >> diff --git a/helper/include/odp/helper/udp.h >> b/helper/include/odp/helper/udp.h >> index 99a96f2..d14cbef 100644 >> --- a/helper/include/odp/helper/udp.h >> +++ b/helper/include/odp/helper/udp.h >> @@ -33,15 +33,6 @@ typedef struct ODP_PACKED { >> uint16be_t chksum; /**< UDP header and data checksum (0 if not >> used)*/ >> } odph_udphdr_t; >> -/** UDP pseudo header */ >> -typedef struct ODPH_PACKET { >> - uint32be_t src_addr; /**< Source addr */ >> - uint32be_t dst_addr; /**< Destination addr */ >> - uint8_t pad; /**< pad byte */ >> - uint8_t proto; /**< UDP protocol */ >> - uint16be_t length; /**< data length */ >> -} odph_udpphdr_t; >> - >> /** >> * UDP checksum >> * >> @@ -53,10 +44,10 @@ typedef struct ODPH_PACKET { >> static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) >> { >> uint32_t sum = 0; >> - odph_udpphdr_t phdr; >> odph_udphdr_t *udph; >> odph_ipv4hdr_t *iph; >> uint16_t udplen; >> + uint8_t *buf; >> if (!odp_packet_l3_offset(pkt)) >> return 0; >> @@ -68,24 +59,22 @@ static inline uint16_t >> odph_ipv4_udp_chksum(odp_packet_t pkt) >> udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL); >> udplen = odp_be_to_cpu_16(udph->length); >> - /* the source ip */ >> - phdr.src_addr = iph->src_addr; >> - /* the dest ip */ >> - phdr.dst_addr = iph->dst_addr; >> - /* proto */ >> - phdr.pad = 0; >> - phdr.proto = ODPH_IPPROTO_UDP; >> - /* the length */ >> - phdr.length = udph->length; >> - >> - /* calc UDP pseudo header chksum */ >> - sum = (__odp_force uint32_t) odp_chksum(&phdr, >> sizeof(odph_udpphdr_t)); >> - /* calc udp header and data chksum */ >> - sum += (__odp_force uint32_t) odp_chksum(udph, udplen); >> + /* 32-bit sum of all 16-bit words covered by UDP chksum */ >> + sum = (iph->src_addr & 0xFFFF) + (iph->src_addr >> 16) + >> + (iph->dst_addr & 0xFFFF) + (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 & 0xFFFF) + (sum >> 16); >> + >> + /* 1's complement */ >> + sum = ~sum; >> + >> /* set computation result */ >> sum = (sum == 0x0) ? 0xFFFF : sum; >> >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 05/15/2015 11:07, Alexandru Badicioiu wrote: > Hi Maxim, what kind of machine you used for testing? I used a BE machine. > > Alex just my x86 laptop, i.e. le. Maxim. > > On 14 May 2015 at 21:04, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > Hello Alexandru, > > Your patch fixes check summ now that function is better: > > > 20:49:45.613467 IP (tos 0x0, id 35630, offset 0, flags [none], > proto UDP (17), length 84) > 192.168.0.1.0 > 192.168.0.2.0: [bad udp cksum 0x1a7e -> > 0x7e1a!] UDP, length 56 > 20:49:45.613470 IP (tos 0x0, id 35662, offset 0, flags [none], > proto UDP (17), length 84) > 192.168.0.1.0 > 192.168.0.2.0: [bad udp cksum 0x1a7e -> > 0x7e1a!] UDP, length 56 > > I used packets generated with our odp_generator. > > After that fix check summ is valid: > > --- a/example/generator/odp_generator.c > +++ b/example/generator/odp_generator.c > @@ -238,7 +238,6 @@ 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; > > That will work on both BE/LE right? > > Thanks, > Maxim. > > > > > On 05/05/2015 15:07, alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org> wrote: > > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>> > > v1 - fixed compile error due to -Wcast-align > > Signed-off-by: Alexandru Badicioiu > <alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>> > --- > helper/include/odp/helper/udp.h | 37 > +++++++++++++------------------------ > 1 files changed, 13 insertions(+), 24 deletions(-) > > diff --git a/helper/include/odp/helper/udp.h > b/helper/include/odp/helper/udp.h > index 99a96f2..d14cbef 100644 > --- a/helper/include/odp/helper/udp.h > +++ b/helper/include/odp/helper/udp.h > @@ -33,15 +33,6 @@ typedef struct ODP_PACKED { > uint16be_t chksum; /**< UDP header and data checksum > (0 if not used)*/ > } odph_udphdr_t; > -/** UDP pseudo header */ > -typedef struct ODPH_PACKET { > - uint32be_t src_addr; /**< Source addr */ > - uint32be_t dst_addr; /**< Destination addr */ > - uint8_t pad; /**< pad byte */ > - uint8_t proto; /**< UDP protocol */ > - uint16be_t length; /**< data length */ > -} odph_udpphdr_t; > - > /** > * UDP checksum > * > @@ -53,10 +44,10 @@ typedef struct ODPH_PACKET { > static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) > { > uint32_t sum = 0; > - odph_udpphdr_t phdr; > odph_udphdr_t *udph; > odph_ipv4hdr_t *iph; > uint16_t udplen; > + uint8_t *buf; > if (!odp_packet_l3_offset(pkt)) > return 0; > @@ -68,24 +59,22 @@ static inline uint16_t > odph_ipv4_udp_chksum(odp_packet_t pkt) > udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL); > udplen = odp_be_to_cpu_16(udph->length); > - /* the source ip */ > - phdr.src_addr = iph->src_addr; > - /* the dest ip */ > - phdr.dst_addr = iph->dst_addr; > - /* proto */ > - phdr.pad = 0; > - phdr.proto = ODPH_IPPROTO_UDP; > - /* the length */ > - phdr.length = udph->length; > - > - /* calc UDP pseudo header chksum */ > - sum = (__odp_force uint32_t) odp_chksum(&phdr, > sizeof(odph_udpphdr_t)); > - /* calc udp header and data chksum */ > - sum += (__odp_force uint32_t) odp_chksum(udph, udplen); > + /* 32-bit sum of all 16-bit words covered by UDP chksum */ > + sum = (iph->src_addr & 0xFFFF) + (iph->src_addr >> 16) + > + (iph->dst_addr & 0xFFFF) + (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 & 0xFFFF) + (sum >> 16); > + > + /* 1's complement */ > + sum = ~sum; > + > /* set computation result */ > sum = (sum == 0x0) ? 0xFFFF : sum; > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > >
Then it means it works on both BE/LE. Alex On 15 May 2015 at 15:26, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 05/15/2015 11:07, Alexandru Badicioiu wrote: > >> Hi Maxim, what kind of machine you used for testing? I used a BE machine. >> >> Alex >> > > just my x86 laptop, i.e. le. > > Maxim. > > >> On 14 May 2015 at 21:04, Maxim Uvarov <maxim.uvarov@linaro.org <mailto: >> maxim.uvarov@linaro.org>> wrote: >> >> Hello Alexandru, >> >> Your patch fixes check summ now that function is better: >> >> >> 20:49:45.613467 IP (tos 0x0, id 35630, offset 0, flags [none], >> proto UDP (17), length 84) >> 192.168.0.1.0 > 192.168.0.2.0: [bad udp cksum 0x1a7e -> >> 0x7e1a!] UDP, length 56 >> 20:49:45.613470 IP (tos 0x0, id 35662, offset 0, flags [none], >> proto UDP (17), length 84) >> 192.168.0.1.0 > 192.168.0.2.0: [bad udp cksum 0x1a7e -> >> 0x7e1a!] UDP, length 56 >> >> I used packets generated with our odp_generator. >> >> After that fix check summ is valid: >> >> --- a/example/generator/odp_generator.c >> +++ b/example/generator/odp_generator.c >> @@ -238,7 +238,6 @@ 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; >> >> That will work on both BE/LE right? >> >> Thanks, >> Maxim. >> >> >> >> >> On 05/05/2015 15:07, alexandru.badicioiu@linaro.org >> <mailto:alexandru.badicioiu@linaro.org> wrote: >> >> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org >> <mailto:alexandru.badicioiu@linaro.org>> >> >> v1 - fixed compile error due to -Wcast-align >> >> Signed-off-by: Alexandru Badicioiu >> <alexandru.badicioiu@linaro.org >> <mailto:alexandru.badicioiu@linaro.org>> >> >> --- >> helper/include/odp/helper/udp.h | 37 >> +++++++++++++------------------------ >> 1 files changed, 13 insertions(+), 24 deletions(-) >> >> diff --git a/helper/include/odp/helper/udp.h >> b/helper/include/odp/helper/udp.h >> index 99a96f2..d14cbef 100644 >> --- a/helper/include/odp/helper/udp.h >> +++ b/helper/include/odp/helper/udp.h >> @@ -33,15 +33,6 @@ typedef struct ODP_PACKED { >> uint16be_t chksum; /**< UDP header and data checksum >> (0 if not used)*/ >> } odph_udphdr_t; >> -/** UDP pseudo header */ >> -typedef struct ODPH_PACKET { >> - uint32be_t src_addr; /**< Source addr */ >> - uint32be_t dst_addr; /**< Destination addr */ >> - uint8_t pad; /**< pad byte */ >> - uint8_t proto; /**< UDP protocol */ >> - uint16be_t length; /**< data length */ >> -} odph_udpphdr_t; >> - >> /** >> * UDP checksum >> * >> @@ -53,10 +44,10 @@ typedef struct ODPH_PACKET { >> static inline uint16_t odph_ipv4_udp_chksum(odp_packet_t pkt) >> { >> uint32_t sum = 0; >> - odph_udpphdr_t phdr; >> odph_udphdr_t *udph; >> odph_ipv4hdr_t *iph; >> uint16_t udplen; >> + uint8_t *buf; >> if (!odp_packet_l3_offset(pkt)) >> return 0; >> @@ -68,24 +59,22 @@ static inline uint16_t >> odph_ipv4_udp_chksum(odp_packet_t pkt) >> udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL); >> udplen = odp_be_to_cpu_16(udph->length); >> - /* the source ip */ >> - phdr.src_addr = iph->src_addr; >> - /* the dest ip */ >> - phdr.dst_addr = iph->dst_addr; >> - /* proto */ >> - phdr.pad = 0; >> - phdr.proto = ODPH_IPPROTO_UDP; >> - /* the length */ >> - phdr.length = udph->length; >> - >> - /* calc UDP pseudo header chksum */ >> - sum = (__odp_force uint32_t) odp_chksum(&phdr, >> sizeof(odph_udpphdr_t)); >> - /* calc udp header and data chksum */ >> - sum += (__odp_force uint32_t) odp_chksum(udph, udplen); >> + /* 32-bit sum of all 16-bit words covered by UDP chksum */ >> + sum = (iph->src_addr & 0xFFFF) + (iph->src_addr >> 16) + >> + (iph->dst_addr & 0xFFFF) + (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 & 0xFFFF) + (sum >> 16); >> + >> + /* 1's complement */ >> + sum = ~sum; >> + >> /* set computation result */ >> sum = (sum == 0x0) ? 0xFFFF : sum; >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >
On 05/15/2015 15:59, Alexandru Badicioiu wrote: > Then it means it works on both BE/LE. > > Alex Ok, then would you like to send updated patch? Maxim. > > On 15 May 2015 at 15:26, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > On 05/15/2015 11:07, Alexandru Badicioiu wrote: > > Hi Maxim, what kind of machine you used for testing? I used a > BE machine. > > Alex > > > just my x86 laptop, i.e. le. > > Maxim. > > > On 14 May 2015 at 21:04, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org> > <mailto:maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>>> wrote: > > Hello Alexandru, > > Your patch fixes check summ now that function is better: > > > 20:49:45.613467 IP (tos 0x0, id 35630, offset 0, flags [none], > proto UDP (17), length 84) > 192.168.0.1.0 > 192.168.0.2.0: [bad udp cksum 0x1a7e -> > 0x7e1a!] UDP, length 56 > 20:49:45.613470 IP (tos 0x0, id 35662, offset 0, flags [none], > proto UDP (17), length 84) > 192.168.0.1.0 > 192.168.0.2.0: [bad udp cksum 0x1a7e -> > 0x7e1a!] UDP, length 56 > > I used packets generated with our odp_generator. > > After that fix check summ is valid: > > --- a/example/generator/odp_generator.c > +++ b/example/generator/odp_generator.c > @@ -238,7 +238,6 @@ 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; > > That will work on both BE/LE right? > > Thanks, > Maxim. > > > > > On 05/05/2015 15:07, alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org> > <mailto:alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>> wrote: > > From: Alexandru Badicioiu > <alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org> > <mailto:alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>>> > > v1 - fixed compile error due to -Wcast-align > > Signed-off-by: Alexandru Badicioiu > <alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org> > <mailto:alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>>> > > --- > helper/include/odp/helper/udp.h | 37 > +++++++++++++------------------------ > 1 files changed, 13 insertions(+), 24 deletions(-) > > diff --git a/helper/include/odp/helper/udp.h > b/helper/include/odp/helper/udp.h > index 99a96f2..d14cbef 100644 > --- a/helper/include/odp/helper/udp.h > +++ b/helper/include/odp/helper/udp.h > @@ -33,15 +33,6 @@ typedef struct ODP_PACKED { > uint16be_t chksum; /**< UDP header and data > checksum > (0 if not used)*/ > } odph_udphdr_t; > -/** UDP pseudo header */ > -typedef struct ODPH_PACKET { > - uint32be_t src_addr; /**< Source addr */ > - uint32be_t dst_addr; /**< Destination addr */ > - uint8_t pad; /**< pad byte */ > - uint8_t proto; /**< UDP protocol */ > - uint16be_t length; /**< data length */ > -} odph_udpphdr_t; > - > /** > * UDP checksum > * > @@ -53,10 +44,10 @@ typedef struct ODPH_PACKET { > static inline uint16_t > odph_ipv4_udp_chksum(odp_packet_t pkt) > { > uint32_t sum = 0; > - odph_udpphdr_t phdr; > odph_udphdr_t *udph; > odph_ipv4hdr_t *iph; > uint16_t udplen; > + uint8_t *buf; > if (!odp_packet_l3_offset(pkt)) > return 0; > @@ -68,24 +59,22 @@ static inline uint16_t > odph_ipv4_udp_chksum(odp_packet_t pkt) > udph = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, > NULL); > udplen = odp_be_to_cpu_16(udph->length); > - /* the source ip */ > - phdr.src_addr = iph->src_addr; > - /* the dest ip */ > - phdr.dst_addr = iph->dst_addr; > - /* proto */ > - phdr.pad = 0; > - phdr.proto = ODPH_IPPROTO_UDP; > - /* the length */ > - phdr.length = udph->length; > - > - /* calc UDP pseudo header chksum */ > - sum = (__odp_force uint32_t) odp_chksum(&phdr, > sizeof(odph_udpphdr_t)); > - /* calc udp header and data chksum */ > - sum += (__odp_force uint32_t) odp_chksum(udph, > udplen); > + /* 32-bit sum of all 16-bit words covered by > UDP chksum */ > + sum = (iph->src_addr & 0xFFFF) + > (iph->src_addr >> 16) + > + (iph->dst_addr & 0xFFFF) + > (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 & 0xFFFF) + (sum >> 16); > + > + /* 1's complement */ > + sum = ~sum; > + > /* set computation result */ > sum = (sum == 0x0) ? 0xFFFF : sum; > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > <mailto:lng-odp@lists.linaro.org > <mailto:lng-odp@lists.linaro.org>> > https://lists.linaro.org/mailman/listinfo/lng-odp > > > >
--- a/example/generator/odp_generator.c +++ b/example/generator/odp_generator.c @@ -238,7 +238,6 @@ 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);