From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754545AbbHCQxo (ORCPT ); Mon, 3 Aug 2015 12:53:44 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:35027 "EHLO mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754225AbbHCQxm (ORCPT ); Mon, 3 Aug 2015 12:53:42 -0400 Date: Mon, 3 Aug 2015 09:53:39 -0700 From: Glenn Griffin To: Pravin Shelar Cc: "David S. Miller" , netdev , "dev@openvswitch.org" , LKML Subject: Re: [PATCH] openvswitch: Fix L4 checksum handling when dealing with IP fragments Message-ID: <20150803165339.GA11291@google.com> References: <1438484942.277866.12350@glenng-linux.sea.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Aug 03, 2015 at 09:25:53AM -0700, Pravin Shelar wrote: > On Sat, Aug 1, 2015 at 6:31 PM, Glenn Griffin wrote: > > openvswitch modifies the L4 checksum of a packet when modifying > > the ip address. When an IP packet is fragmented only the first > > fragment contains an L4 header and checksum. Prior to this change > > openvswitch would modify all fragments, modifying application data > > in non-first fragments, causing checksum failures in the > > reassembled packet. > > > > Signed-off-by: Glenn Griffin > > Patch looks good. I have one following comment. > > --- > > net/openvswitch/actions.c | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c > > index 8a8c0b8..bfffb1a 100644 > > --- a/net/openvswitch/actions.c > > +++ b/net/openvswitch/actions.c > > @@ -273,28 +273,36 @@ static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *flow_key, > > return 0; > > } > > > > -static void set_ip_addr(struct sk_buff *skb, struct iphdr *nh, > > - __be32 *addr, __be32 new_addr) > > +static void update_ip_l4_checksum(struct sk_buff *skb, struct iphdr *nh, > > + __be32 addr, __be32 new_addr) > > { > > int transport_len = skb->len - skb_transport_offset(skb); > > > > + if (ntohs(nh->frag_off) & IP_OFFSET) > > + return; > > It is efficient to check frag-offset in network byte order. I'll send a revised patch. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Glenn Griffin Subject: Re: [PATCH] openvswitch: Fix L4 checksum handling when dealing with IP fragments Date: Mon, 3 Aug 2015 09:53:39 -0700 Message-ID: <20150803165339.GA11291@google.com> References: <1438484942.277866.12350@glenng-linux.sea.corp.google.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Cc: "dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org" , netdev , "David S. Miller" , LKML To: Pravin Shelar Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-yBygre7rU0TnMu66kgdUjQ@public.gmane.org Sender: "dev" List-Id: netdev.vger.kernel.org T24gTW9uLCBBdWcgMDMsIDIwMTUgYXQgMDk6MjU6NTNBTSAtMDcwMCwgUHJhdmluIFNoZWxhciB3 cm90ZToKPiBPbiBTYXQsIEF1ZyAxLCAyMDE1IGF0IDY6MzEgUE0sIEdsZW5uIEdyaWZmaW4gPGdn cmlmZmluLmtlcm5lbEBnbWFpbC5jb20+IHdyb3RlOgo+ID4gb3BlbnZzd2l0Y2ggbW9kaWZpZXMg dGhlIEw0IGNoZWNrc3VtIG9mIGEgcGFja2V0IHdoZW4gbW9kaWZ5aW5nCj4gPiB0aGUgaXAgYWRk cmVzcy4gV2hlbiBhbiBJUCBwYWNrZXQgaXMgZnJhZ21lbnRlZCBvbmx5IHRoZSBmaXJzdAo+ID4g ZnJhZ21lbnQgY29udGFpbnMgYW4gTDQgaGVhZGVyIGFuZCBjaGVja3N1bS4gUHJpb3IgdG8gdGhp cyBjaGFuZ2UKPiA+IG9wZW52c3dpdGNoIHdvdWxkIG1vZGlmeSBhbGwgZnJhZ21lbnRzLCBtb2Rp ZnlpbmcgYXBwbGljYXRpb24gZGF0YQo+ID4gaW4gbm9uLWZpcnN0IGZyYWdtZW50cywgY2F1c2lu ZyBjaGVja3N1bSBmYWlsdXJlcyBpbiB0aGUKPiA+IHJlYXNzZW1ibGVkIHBhY2tldC4KPiA+Cj4g PiBTaWduZWQtb2ZmLWJ5OiBHbGVubiBHcmlmZmluIDxnZ3JpZmZpbi5rZXJuZWxAZ21haWwuY29t Pgo+IAo+IFBhdGNoIGxvb2tzIGdvb2QuIEkgaGF2ZSBvbmUgZm9sbG93aW5nIGNvbW1lbnQuCj4g PiAtLS0KPiA+ICBuZXQvb3BlbnZzd2l0Y2gvYWN0aW9ucy5jIHwgMTYgKysrKysrKysrKysrLS0t LQo+ID4gIDEgZmlsZSBjaGFuZ2VkLCAxMiBpbnNlcnRpb25zKCspLCA0IGRlbGV0aW9ucygtKQo+ ID4KPiA+IGRpZmYgLS1naXQgYS9uZXQvb3BlbnZzd2l0Y2gvYWN0aW9ucy5jIGIvbmV0L29wZW52 c3dpdGNoL2FjdGlvbnMuYwo+ID4gaW5kZXggOGE4YzBiOC4uYmZmZmIxYSAxMDA2NDQKPiA+IC0t LSBhL25ldC9vcGVudnN3aXRjaC9hY3Rpb25zLmMKPiA+ICsrKyBiL25ldC9vcGVudnN3aXRjaC9h Y3Rpb25zLmMKPiA+IEBAIC0yNzMsMjggKzI3MywzNiBAQCBzdGF0aWMgaW50IHNldF9ldGhfYWRk cihzdHJ1Y3Qgc2tfYnVmZiAqc2tiLCBzdHJ1Y3Qgc3dfZmxvd19rZXkgKmZsb3dfa2V5LAo+ID4g ICAgICAgICByZXR1cm4gMDsKPiA+ICB9Cj4gPgo+ID4gLXN0YXRpYyB2b2lkIHNldF9pcF9hZGRy KHN0cnVjdCBza19idWZmICpza2IsIHN0cnVjdCBpcGhkciAqbmgsCj4gPiAtICAgICAgICAgICAg ICAgICAgICAgICBfX2JlMzIgKmFkZHIsIF9fYmUzMiBuZXdfYWRkcikKPiA+ICtzdGF0aWMgdm9p ZCB1cGRhdGVfaXBfbDRfY2hlY2tzdW0oc3RydWN0IHNrX2J1ZmYgKnNrYiwgc3RydWN0IGlwaGRy ICpuaCwKPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBfX2JlMzIgYWRkciwg X19iZTMyIG5ld19hZGRyKQo+ID4gIHsKPiA+ICAgICAgICAgaW50IHRyYW5zcG9ydF9sZW4gPSBz a2ItPmxlbiAtIHNrYl90cmFuc3BvcnRfb2Zmc2V0KHNrYik7Cj4gPgo+ID4gKyAgICAgICBpZiAo bnRvaHMobmgtPmZyYWdfb2ZmKSAmIElQX09GRlNFVCkKPiA+ICsgICAgICAgICAgICAgICByZXR1 cm47Cj4gCj4gSXQgaXMgZWZmaWNpZW50IHRvIGNoZWNrIGZyYWctb2Zmc2V0IGluIG5ldHdvcmsg Ynl0ZSBvcmRlci4KCkknbGwgc2VuZCBhIHJldmlzZWQgcGF0Y2guCl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRldiBtYWlsaW5nIGxpc3QKZGV2QG9wZW52 c3dpdGNoLm9yZwpodHRwOi8vb3BlbnZzd2l0Y2gub3JnL21haWxtYW4vbGlzdGluZm8vZGV2Cg==