* Re: [PATCH] openvswitch: Fix L4 checksum handling when dealing with IP fragments @ 2015-08-03 16:25 ` Pravin Shelar 0 siblings, 0 replies; 4+ messages in thread From: Pravin Shelar @ 2015-08-03 16:25 UTC (permalink / raw) To: Glenn Griffin; +Cc: David S. Miller, netdev, dev@openvswitch.org, LKML On Sat, Aug 1, 2015 at 6:31 PM, Glenn Griffin <ggriffin.kernel@gmail.com> 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 <ggriffin.kernel@gmail.com> 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] openvswitch: Fix L4 checksum handling when dealing with IP fragments @ 2015-08-03 16:25 ` Pravin Shelar 0 siblings, 0 replies; 4+ messages in thread From: Pravin Shelar @ 2015-08-03 16:25 UTC (permalink / raw) To: Glenn Griffin Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev, David S. Miller, LKML On Sat, Aug 1, 2015 at 6:31 PM, Glenn Griffin <ggriffin.kernel@gmail.com> 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 <ggriffin.kernel@gmail.com> 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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] openvswitch: Fix L4 checksum handling when dealing with IP fragments @ 2015-08-03 16:53 ` Glenn Griffin 0 siblings, 0 replies; 4+ messages in thread From: Glenn Griffin @ 2015-08-03 16:53 UTC (permalink / raw) To: Pravin Shelar; +Cc: David S. Miller, netdev, dev@openvswitch.org, LKML On Mon, Aug 03, 2015 at 09:25:53AM -0700, Pravin Shelar wrote: > On Sat, Aug 1, 2015 at 6:31 PM, Glenn Griffin <ggriffin.kernel@gmail.com> 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 <ggriffin.kernel@gmail.com> > > 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] openvswitch: Fix L4 checksum handling when dealing with IP fragments @ 2015-08-03 16:53 ` Glenn Griffin 0 siblings, 0 replies; 4+ messages in thread From: Glenn Griffin @ 2015-08-03 16:53 UTC (permalink / raw) To: Pravin Shelar Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev, David S. Miller, LKML On Mon, Aug 03, 2015 at 09:25:53AM -0700, Pravin Shelar wrote: > On Sat, Aug 1, 2015 at 6:31 PM, Glenn Griffin <ggriffin.kernel@gmail.com> 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 <ggriffin.kernel@gmail.com> > > 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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-08-03 16:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1438484942.277866.12350@glenng-linux.sea.corp.google.com>
2015-08-03 16:25 ` [PATCH] openvswitch: Fix L4 checksum handling when dealing with IP fragments Pravin Shelar
2015-08-03 16:25 ` Pravin Shelar
2015-08-03 16:53 ` Glenn Griffin
2015-08-03 16:53 ` Glenn Griffin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.