Message ID | DB3PR04MB1073850452F0411F413F395E6360@DB3PR04MB107.eurprd04.prod.outlook.com (mailing list archive) |
---|---|
State | Rejected, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 06D692B8E; Mon, 18 Jul 2016 14:41:08 +0200 (CEST) Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on0086.outbound.protection.outlook.com [104.47.2.86]) by dpdk.org (Postfix) with ESMTP id B3A8FE6D for <dev@dpdk.org>; Mon, 18 Jul 2016 14:41:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=uRdx8LEGjURYgUW1QR4HS8/Y43+kzqclUj4fifWlUuM=; b=HSILX8goo4cDw8C5dq167rI/qe4C8zj/FnLK5ObWLzQEoPUFlNEl0fKrH584WCyCRYXRrC0wTGKRCOn5EG8euGwJ4vKBKcYZ2azK4lWR+qA6ju+HzUu/4oI3YZDiEBKTc9oth1EfuHwyBPsHGqT7xOMbYDzptOnedpKNxCxyank= Received: from DB3PR04MB107.eurprd04.prod.outlook.com (10.242.129.20) by DB3PR04MB108.eurprd04.prod.outlook.com (10.242.129.25) with Microsoft SMTP Server (TLS) id 15.1.539.6; Mon, 18 Jul 2016 12:41:05 +0000 Received: from DB3PR04MB107.eurprd04.prod.outlook.com ([169.254.12.17]) by DB3PR04MB107.eurprd04.prod.outlook.com ([169.254.12.17]) with mapi id 15.01.0539.019; Mon, 18 Jul 2016 12:41:06 +0000 From: Akhil Goyal <akhil.goyal@nxp.com> To: "dev@dpdk.org" <dev@dpdk.org> Thread-Topic: ip_chksum not updated in ipsec-secgw application Thread-Index: AdHg7lCKLMtEfGA5QMe00lC33OFqiw== Date: Mon, 18 Jul 2016 12:41:05 +0000 Message-ID: <DB3PR04MB1073850452F0411F413F395E6360@DB3PR04MB107.eurprd04.prod.outlook.com> Accept-Language: en-IN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=akhil.goyal@nxp.com; x-originating-ip: [192.88.169.1] x-ms-office365-filtering-correlation-id: 223d54f6-487b-43a8-1308-08d3af08c94a x-microsoft-exchange-diagnostics: 1; DB3PR04MB108; 6:e29/hrXKPZFFd4WimBQGUJw+m9I9NetVsGnx7NcC1F+CJaANotwgzv6LIYl9+DpQ++ajlF0uzojY2xjz1I0+Tq6DQKCTTiSPuYVIpj3a94koVo6VtC51Pl5JVKkCz/PDHDlgRCwhl7U24VcYnmA/OjsKHhvtjb7gdEZqC8M+dHkaVx1p0koc9z/C/GbDwECQQrJCqer1gQ8GdAVgcv5gU9VIm89NRILrTCMbP5pjOaAYsuHu6pLKa85Ym0pjbNOqhBpev36ez2nvPHndgcTHKYjEBsKp3LsPAzTGRfj52NJJFBZp7SHpWn/wuaUIcf/H097nfM5oCXrORRhlNUvrlQ==; 5:c+zyBRZ5EFX5roohERvE7gCJuJ0N8qllp8jbbimrOKc58BL1WmE01TxnzLkahBOwZiPLB6QX5gurot+BGW78EzAUTujjC+dbcXgVks1KF2Q/oegMhafxa+B3aCrwaQzrYvznM4Z/BYU9ZI8g8KSiYQ==; 24:xZ4FaII5FqBnC2YqFjugKOQUQt5DBGUcSp9D1yynXWmC6IB2Gb9rM1LSCsewj2j5uW76DWfWRFKIxu6VtgzYVY2BxfiSy1TWFriKJaipG+E=; 7:byr39ZfsG5zOA7tmRFEXW7AD12M/mVLbqV/KfP4+Tkat33JovyETDAu8CyeK5aqeMjlHBry1wGR2o3Nlt1M+7sBPdaBdKVHhHWL3scoc8fSniP5dQ7gvPHI6M1ef+I8mLvucDg62PNnTbb1paj5WUel/yaTv3kV0T49fp9tytzLulmrqtpOx020Pt59cVcOqXKnvFGQ3a1ZCQb0HoBE1tQ7iJIgoFr8Vi4wlI8f1odZUUop21AvfDuDU5nVKFYZv x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DB3PR04MB108; x-microsoft-antispam-prvs: <DB3PR04MB108FBD1D71BA8B2BE22E3C6E6360@DB3PR04MB108.eurprd04.prod.outlook.com> x-exchange-antispam-report-test: UriScan:(21748063052155); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026); SRVR:DB3PR04MB108; BCL:0; PCL:0; RULEID:; SRVR:DB3PR04MB108; x-forefront-prvs: 00073DB75F x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(199003)(189002)(3846002)(3280700002)(15975445007)(101416001)(7846002)(5003600100003)(7696003)(3660700001)(54356999)(7736002)(74316002)(16236675004)(586003)(50986999)(19625215002)(33656002)(86362001)(66066001)(15650500001)(2420400007)(122556002)(87936001)(68736007)(2906002)(9686002)(9326002)(5002640100001)(102836003)(2900100001)(19580395003)(790700001)(6116002)(10710500007)(19300405004)(8936002)(76576001)(450100001)(106356001)(105586002)(5640700001)(5630700001)(2501003)(10400500002)(97736004)(107886002)(110136002)(189998001)(7110500001)(229853001)(92566002)(2351001)(81166006)(81156014)(1730700003)(8676002); DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR04MB108; H:DB3PR04MB107.eurprd04.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Jul 2016 12:41:05.9096 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR04MB108 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: [dpdk-dev] ip_chksum not updated in ipsec-secgw application X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Akhil Goyal
July 18, 2016, 12:41 p.m. UTC
Hi, In Ipsec-secgw application, while adding the outer IP header, it seems that the application does not update the checksum value for outbound packets. This result in incorrect ip->checksum in the encrypted packet. Please let me know if the checksum value is updated somewhere else or not. Also In case of inner ip header also the TTL value is decremented by one but the checksum value is not updated. Is it intentional or it is done somewhere else? After addition of following code, the checksum looks good and the encrypted packets are good. Regards, Akhil
Comments
Hi, On 18/07/2016 13:41, Akhil Goyal wrote: > Hi, > > In Ipsec-secgw application, while adding the outer IP header, it seems that the application does not update the checksum value for outbound packets. This result in incorrect ip->checksum in the encrypted packet. > > Please let me know if the checksum value is updated somewhere else or not. > > Also In case of inner ip header also the TTL value is decremented by one but the checksum value is not updated. Is it intentional or it is done somewhere else? It is intentional. The application is using IP checksum offload but just looking now at the code there is a bug for IPv6 packets where the flag does not get setup. Is it only for IPv6 traffic that you are having this issue? For IPv4 traffic the PKT_TX_IP_CKSUM flag is setup in 'prepare_tx_pkt' function in ipsec-secgw.c Sergio > After addition of following code, the checksum looks good and the encrypted packets are good. > > diff --git a/examples/ipsec-secgw/ipip.h b/examples/ipsec-secgw/ipip.h > index 322076c..0f7b60f 100644 > --- a/examples/ipsec-secgw/ipip.h > +++ b/examples/ipsec-secgw/ipip.h > @@ -41,6 +41,24 @@ > #include <rte_mbuf.h> > > #define IPV6_VERSION (6) > +static inline uint16_t > +ip_sum(const unaligned_uint16_t *hdr, int hdr_len) > +{ > + uint32_t sum = 0; > + > + while (hdr_len > 1) > + { > + sum += *hdr++; > + if (sum & 0x80000000) > + sum = (sum & 0xFFFF) + (sum >> 16); > + hdr_len -= 2; > + } > + > + while (sum >> 16) > + sum = (sum & 0xFFFF) + (sum >> 16); > + > + return ~sum; > +} > > static inline struct ip * > ip4ip_outbound(struct rte_mbuf *m, uint32_t offset, uint32_t src, uint32_t dst) > @@ -71,7 +89,8 @@ ip4ip_outbound(struct rte_mbuf *m, uint32_t offset, uint32_t src, uint32_t dst) > > outip->ip_src.s_addr = src; > outip->ip_dst.s_addr = dst; > - > + outip->ip_sum = 0; > + outip->ip_sum = ip_sum((const unaligned_uint16_t *)outip, sizeof(struct ip)); > return outip; > } > > Regards, > Akhil >
2016-07-18 13:57, Sergio Gonzalez Monroy: > On 18/07/2016 13:41, Akhil Goyal wrote: > > In Ipsec-secgw application, while adding the outer IP header, > > it seems that the application does not update the checksum value > > for outbound packets. This result in incorrect ip->checksum in > > the encrypted packet. [...] > > It is intentional. The application is using IP checksum offload The correct behaviour is to have a software fallback (using rte_ip.h) for drivers which do not support checksum offload. But given it is just an example, it is normal to have this kind of constraint. However I think it should be explained in its doc. And a list of tested NICs would be nice to have.
On 7/18/2016 6:27 PM, Sergio Gonzalez Monroy wrote: > Hi, > > On 18/07/2016 13:41, Akhil Goyal wrote: >> Hi, >> >> In Ipsec-secgw application, while adding the outer IP header, it seems >> that the application does not update the checksum value for outbound >> packets. This result in incorrect ip->checksum in the encrypted packet. >> >> Please let me know if the checksum value is updated somewhere else or >> not. >> >> Also In case of inner ip header also the TTL value is decremented by >> one but the checksum value is not updated. Is it intentional or it is >> done somewhere else? > > It is intentional. The application is using IP checksum offload but just > looking now at the code there is a bug for IPv6 packets where the flag > does not get setup. > Is it only for IPv6 traffic that you are having this issue? > > For IPv4 traffic the PKT_TX_IP_CKSUM flag is setup in 'prepare_tx_pkt' > function in ipsec-secgw.c > > Sergio > Thanks Sergio, got your point. I missed the flag. I was using it for IPv4. Akhil
On 7/18/2016 6:50 PM, Thomas Monjalon wrote: > 2016-07-18 13:57, Sergio Gonzalez Monroy: >> On 18/07/2016 13:41, Akhil Goyal wrote: >>> In Ipsec-secgw application, while adding the outer IP header, >>> it seems that the application does not update the checksum value >>> for outbound packets. This result in incorrect ip->checksum in >>> the encrypted packet. > [...] >> >> It is intentional. The application is using IP checksum offload > > The correct behaviour is to have a software fallback (using rte_ip.h) > for drivers which do not support checksum offload. > But given it is just an example, it is normal to have this kind of > constraint. However I think it should be explained in its doc. > And a list of tested NICs would be nice to have. > Agreed. The driver that I was using did not enable checksum offload. It is good to have a fallback option.
On 18/07/2016 14:53, Akhil Goyal wrote: > On 7/18/2016 6:50 PM, Thomas Monjalon wrote: >> 2016-07-18 13:57, Sergio Gonzalez Monroy: >>> On 18/07/2016 13:41, Akhil Goyal wrote: >>>> In Ipsec-secgw application, while adding the outer IP header, >>>> it seems that the application does not update the checksum value >>>> for outbound packets. This result in incorrect ip->checksum in >>>> the encrypted packet. >> [...] >>> >>> It is intentional. The application is using IP checksum offload >> >> The correct behaviour is to have a software fallback (using rte_ip.h) >> for drivers which do not support checksum offload. >> But given it is just an example, it is normal to have this kind of >> constraint. However I think it should be explained in its doc. >> And a list of tested NICs would be nice to have. >> > Agreed. The driver that I was using did not enable checksum offload. > It is good to have a fallback option. > That's a good point. So would it be enough to call out in the sample app guide that we use IP checksum offload and show a warning in case the Driver does not support such offload? Sergio
On 18/07/2016 14:49, Akhil Goyal wrote: > On 7/18/2016 6:27 PM, Sergio Gonzalez Monroy wrote: >> Hi, >> >> On 18/07/2016 13:41, Akhil Goyal wrote: >>> Hi, >>> >>> In Ipsec-secgw application, while adding the outer IP header, it seems >>> that the application does not update the checksum value for outbound >>> packets. This result in incorrect ip->checksum in the encrypted packet. >>> >>> Please let me know if the checksum value is updated somewhere else or >>> not. >>> >>> Also In case of inner ip header also the TTL value is decremented by >>> one but the checksum value is not updated. Is it intentional or it is >>> done somewhere else? >> >> It is intentional. The application is using IP checksum offload but just >> looking now at the code there is a bug for IPv6 packets where the flag >> does not get setup. >> Is it only for IPv6 traffic that you are having this issue? >> Duh! moment here. No checksum for IPv6, that's why the flag is not setup so the code is correct as it is, it just needs IPv4 checksum offload support. Sergio >> For IPv4 traffic the PKT_TX_IP_CKSUM flag is setup in 'prepare_tx_pkt' >> function in ipsec-secgw.c >> >> Sergio >> > > Thanks Sergio, got your point. I missed the flag. I was using it for > IPv4. > > Akhil > >
2016-07-18 14:57, Sergio Gonzalez Monroy: > On 18/07/2016 14:53, Akhil Goyal wrote: > > On 7/18/2016 6:50 PM, Thomas Monjalon wrote: > >> 2016-07-18 13:57, Sergio Gonzalez Monroy: > >>> On 18/07/2016 13:41, Akhil Goyal wrote: > >>>> In Ipsec-secgw application, while adding the outer IP header, > >>>> it seems that the application does not update the checksum value > >>>> for outbound packets. This result in incorrect ip->checksum in > >>>> the encrypted packet. > >> [...] > >>> > >>> It is intentional. The application is using IP checksum offload > >> > >> The correct behaviour is to have a software fallback (using rte_ip.h) > >> for drivers which do not support checksum offload. > >> But given it is just an example, it is normal to have this kind of > >> constraint. However I think it should be explained in its doc. > >> And a list of tested NICs would be nice to have. > >> > > Agreed. The driver that I was using did not enable checksum offload. > > It is good to have a fallback option. > > That's a good point. > So would it be enough to call out in the sample app guide that we use IP > checksum offload and > show a warning in case the Driver does not support such offload? Yes and a list of tested NICs would make it perfect :)
On 18/07/2016 15:09, Thomas Monjalon wrote: > 2016-07-18 14:57, Sergio Gonzalez Monroy: >> On 18/07/2016 14:53, Akhil Goyal wrote: >>> On 7/18/2016 6:50 PM, Thomas Monjalon wrote: >>>> 2016-07-18 13:57, Sergio Gonzalez Monroy: >>>>> On 18/07/2016 13:41, Akhil Goyal wrote: >>>>>> In Ipsec-secgw application, while adding the outer IP header, >>>>>> it seems that the application does not update the checksum value >>>>>> for outbound packets. This result in incorrect ip->checksum in >>>>>> the encrypted packet. >>>> [...] >>>>> It is intentional. The application is using IP checksum offload >>>> The correct behaviour is to have a software fallback (using rte_ip.h) >>>> for drivers which do not support checksum offload. >>>> But given it is just an example, it is normal to have this kind of >>>> constraint. However I think it should be explained in its doc. >>>> And a list of tested NICs would be nice to have. >>>> >>> Agreed. The driver that I was using did not enable checksum offload. >>> It is good to have a fallback option. >> That's a good point. >> So would it be enough to call out in the sample app guide that we use IP >> checksum offload and >> show a warning in case the Driver does not support such offload? > Yes > and a list of tested NICs would make it perfect :) There is no mention of specific tested hardware in the example guides, is there? I would prefer to just point to doc/guides/nics/overview.rst to check if the NIC supports IP checksum offload and in the application itself check for such capability and display a warning in case it is not supported. Sergio
diff --git a/examples/ipsec-secgw/ipip.h b/examples/ipsec-secgw/ipip.h index 322076c..0f7b60f 100644 --- a/examples/ipsec-secgw/ipip.h +++ b/examples/ipsec-secgw/ipip.h @@ -41,6 +41,24 @@ #include <rte_mbuf.h> #define IPV6_VERSION (6) +static inline uint16_t +ip_sum(const unaligned_uint16_t *hdr, int hdr_len) +{ + uint32_t sum = 0; + + while (hdr_len > 1) + { + sum += *hdr++; + if (sum & 0x80000000) + sum = (sum & 0xFFFF) + (sum >> 16); + hdr_len -= 2; + } + + while (sum >> 16) + sum = (sum & 0xFFFF) + (sum >> 16); + + return ~sum; +} static inline struct ip * ip4ip_outbound(struct rte_mbuf *m, uint32_t offset, uint32_t src, uint32_t dst) @@ -71,7 +89,8 @@ ip4ip_outbound(struct rte_mbuf *m, uint32_t offset, uint32_t src, uint32_t dst) outip->ip_src.s_addr = src; outip->ip_dst.s_addr = dst; - + outip->ip_sum = 0; + outip->ip_sum = ip_sum((const unaligned_uint16_t *)outip, sizeof(struct ip)); return outip; }