| Message ID | CA+gU81RsRkWmw7Kao-ZrBe7kxMhDDnVUSxXJmH7Um6OuZofb2g@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
If we're going to fix this, it should be a complete fix. This seems to be tied to IPv4 and it should support IPv6 as well. On Fri, May 8, 2015 at 9:02 AM, Alexandru Badicioiu < alexandru.badicioiu@linaro.org> wrote: > Ping. > > ---------- Forwarded message ---------- > From: <alexandru.badicioiu@linaro.org> > Date: 5 May 2015 at 15:07 > Subject: [lng-odp][PATCH 1/1 v1] helpers: fix udp checksum computation > To: lng-odp@lists.linaro.org > Cc: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> > > > 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; > > -- > 1.7.3.4 > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp > >
I think we need to fix this (at least for IPv4) as it is likely to fail pktio validation suite. Packets with wrong checksums are likely to be discarded by ports configured for checksum validation. IPv6 support can be added later. Alex On 8 May 2015 at 17:17, Bill Fischofer <bill.fischofer@linaro.org> wrote: > If we're going to fix this, it should be a complete fix. This seems to be > tied to IPv4 and it should support IPv6 as well. > > On Fri, May 8, 2015 at 9:02 AM, Alexandru Badicioiu < > alexandru.badicioiu@linaro.org> wrote: > >> Ping. >> >> ---------- Forwarded message ---------- >> From: <alexandru.badicioiu@linaro.org> >> Date: 5 May 2015 at 15:07 >> Subject: [lng-odp][PATCH 1/1 v1] helpers: fix udp checksum computation >> To: lng-odp@lists.linaro.org >> Cc: Alexandru Badicioiu <alexandru.badicioiu@linaro.org> >> >> >> 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; >> >> -- >> 1.7.3.4 >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >
On 05/08/2015 17:21, Alexandru Badicioiu wrote: > I think we need to fix this (at least for IPv4) as it is likely to > fail pktio validation suite. Packets with wrong checksums are likely > to be discarded by ports configured for checksum validation. IPv6 > support can be added later. > > Alex > agree, ipv6 can be separate patch. If ipv6 also need fixing we need to create bug for it, not not forget about this issue. Maxim. > On 8 May 2015 at 17:17, Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> wrote: > > If we're going to fix this, it should be a complete fix. This > seems to be tied to IPv4 and it should support IPv6 as well. > > On Fri, May 8, 2015 at 9:02 AM, Alexandru Badicioiu > <alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>> wrote: > > Ping. > > ---------- Forwarded message ---------- > From: <alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>> > Date: 5 May 2015 at 15:07 > Subject: [lng-odp][PATCH 1/1 v1] helpers: fix udp checksum > computation > To: lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > Cc: Alexandru Badicioiu <alexandru.badicioiu@linaro.org > <mailto:alexandru.badicioiu@linaro.org>> > > > 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; > > -- > 1.7.3.4 > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
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