[ovs-dev] [PATCH] datapath: 16bit inner_network_header field in struct ovs_gso_cb

Jesse Gross jesse at nicira.com
Wed May 21 00:14:25 UTC 2014


On Tue, May 20, 2014 at 4:31 PM, Simon Horman <horms at verge.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 at verge.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 at verge.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 at suug.ch>
> Cc: Jesse Gross <jesse at nicira.com>
> Signed-off-by: Simon Horman <horms at verge.net.au>

Applied, thanks.



More information about the dev mailing list