netdev - Re: [PATCH] datapath: 16bit inner_network_header field in struct ovs_gso

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20140521011834.GB25483@verge.net.au>
Date:	Wed, 21 May 2014 10:18:34 +0900
From:	Simon Horman <horms@...ge.net.au>
To:	Jesse Gross <jesse@...ira.com>
Cc:	"dev@...nvswitch.org" <dev@...nvswitch.org>,
	netdev <netdev@...r.kernel.org>, Thomas Graf <tgraf@...g.ch>
Subject: Re: [PATCH] datapath: 16bit inner_network_header field in struct
 ovs_gso_cb

On Tue, May 20, 2014 at 05:14:25PM -0700, Jesse Gross wrote:
> On Tue, May 20, 2014 at 4:31 PM, Simon Horman <horms@...ge.net.au> wrote:
> > On Tue, May 20, 2014 at 12:54:32PM -0700, Jesse Gross wrote:
> >> On Tue, May 20, 2014 at 3:41 AM, Simon Horman <horms@...ge.net.au> wrote:
> >> > diff --git a/datapath/linux/compat/gso.h b/datapath/linux/compat/gso.h
> >> > index b83a4c3..f459c18 100644
> >> > --- a/datapath/linux/compat/gso.h
> >> > +++ b/datapath/linux/compat/gso.h
> >> >  #ifdef NET_SKBUFF_DATA_USES_OFFSET
> >> > -#define skb_inner_network_header rpl_skb_inner_network_header
> >> > -static inline unsigned char *skb_inner_network_header(const struct sk_buff *skb)
> >> > -{
> >> > -       return skb->head + OVS_GSO_CB(skb)->inner_network_header;
> >> > -}
> >> > -
> >> >  #define skb_inner_mac_header rpl_skb_inner_mac_header
> >> >  static inline unsigned char *skb_inner_mac_header(const struct sk_buff *skb)
> >> >  {
> >> >         return skb->head + OVS_GSO_CB(skb)->inner_mac_header;
> >> >  }
> >> >
> >> > +#define skb_inner_network_header rpl_skb_inner_network_header
> >> > +static inline unsigned char *skb_inner_network_header(const struct sk_buff *skb)
> >> > +{
> >> > +       return skb_inner_mac_header(skb) +
> >> > +               OVS_GSO_CB(skb)->inner_network_header;
> >> > +}
> >> > +
> >> >  #else
> >>
> >> This looks mostly fine to me but doesn't this need to be outside the
> >> #ifdef NET_SKBUFF_DATA_USES_OFFSET block? Otherwise we will treat the
> >> offset as a pointer on 32 bit kernels.
> >
> > Thanks, Somehow I completely missed that.
> > How about this?
> >
> >
> > From: Simon Horman <horms@...ge.net.au>
> >
> > [PATCH v1.1] datapath: 16bit inner_network_header field in struct ovs_gso_cb
> >
> > The motivation for this is to create a 16bit hole in struct ovs_gso_cb
> > which may be used for the inner_protocol field which is needed
> > for the proposed implementation of compatibility for MPLS GSO segmentation.
> >
> > This should be safe as inner_network_header is now an offset to
> > the inner_mac_header rather than skb->head.
> >
> > As pointed out by Thomas Graf simply making both inner offsets 16bis is not
> > safe as there have been cases of overflow with "with collapsed TCP frames
> > on IB when the headroom grew beyond 64K. See commit 50bceae9bd ``tcp:
> > Reallocate headroom if it would overflow csum_start'' for additional
> > details."
> >
> > This patch is based on suggestions by Thomas Graf and Jesse Gross.
> >
> > Cc: Thomas Graf <tgraf@...g.ch>
> > Cc: Jesse Gross <jesse@...ira.com>
> > Signed-off-by: Simon Horman <horms@...ge.net.au>
> 
> Applied, thanks.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html