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.