Ben Pfaff | 10 Jul 2012 23:28

[PATCH] datapath: Check gso_type for correct sk_buff in queue_gso_packets().

At the point where it was used, skb_shinfo(skb)->gso_type referred to a
post-GSO sk_buff.  Thus, it would always be 0.  We want to know the pre-GSO
gso_type, so we need to obtain it before segmenting.

Before this change, the kernel would pass inconsistent data to userspace:
packets for UDP fragments with nonzero offset would be passed along with
flow keys that indicate a zero offset (that is, the flow key for "later"
fragments claimed to be "first" fragments).  This inconsistency tended
to confuse Open vSwitch userspace, causing it to log messages about
"failed to flow_del" the flows with "later" fragments.

Bug #12394.
Signed-off-by: Ben Pfaff <blp@...>
---
 datapath/datapath.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 28e0573..dc2cfad 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
 <at>  <at>  -404,6 +404,7  <at>  <at>  static int queue_gso_packets(struct net *net, int dp_ifindex,
 			     struct sk_buff *skb,
 			     const struct dp_upcall_info *upcall_info)
 {
+	unsigned short gso_type = skb_shinfo(skb)->gso_type;
 	struct dp_upcall_info later_info;
 	struct sw_flow_key later_key;
 	struct sk_buff *segs, *nskb;
 <at>  <at>  -420,7 +421,7  <at>  <at>  static int queue_gso_packets(struct net *net, int dp_ifindex,
(Continue reading)

Jesse Gross | 11 Jul 2012 00:10

Re: [PATCH] datapath: Check gso_type for correct sk_buff in queue_gso_packets().

On Tue, Jul 10, 2012 at 2:28 PM, Ben Pfaff <blp@...> wrote:
> At the point where it was used, skb_shinfo(skb)->gso_type referred to a
> post-GSO sk_buff.  Thus, it would always be 0.  We want to know the pre-GSO
> gso_type, so we need to obtain it before segmenting.
>
> Before this change, the kernel would pass inconsistent data to userspace:
> packets for UDP fragments with nonzero offset would be passed along with
> flow keys that indicate a zero offset (that is, the flow key for "later"
> fragments claimed to be "first" fragments).  This inconsistency tended
> to confuse Open vSwitch userspace, causing it to log messages about
> "failed to flow_del" the flows with "later" fragments.
>
> Bug #12394.
> Signed-off-by: Ben Pfaff <blp@...>

Acked-by: Jesse Gross <jesse@...>
Ben Pfaff | 11 Jul 2012 00:32

Re: [PATCH] datapath: Check gso_type for correct sk_buff in queue_gso_packets().

On Tue, Jul 10, 2012 at 03:10:07PM -0700, Jesse Gross wrote:
> On Tue, Jul 10, 2012 at 2:28 PM, Ben Pfaff <blp@...> wrote:
> > At the point where it was used, skb_shinfo(skb)->gso_type referred to a
> > post-GSO sk_buff.  Thus, it would always be 0.  We want to know the pre-GSO
> > gso_type, so we need to obtain it before segmenting.
> >
> > Before this change, the kernel would pass inconsistent data to userspace:
> > packets for UDP fragments with nonzero offset would be passed along with
> > flow keys that indicate a zero offset (that is, the flow key for "later"
> > fragments claimed to be "first" fragments).  This inconsistency tended
> > to confuse Open vSwitch userspace, causing it to log messages about
> > "failed to flow_del" the flows with "later" fragments.
> >
> > Bug #12394.
> > Signed-off-by: Ben Pfaff <blp@...>
> 
> Acked-by: Jesse Gross <jesse@...>

Thanks, pushed to master and branch-1.[4567].

Gmane