All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
To: Alexander Duyck <alexander.duyck@gmail.com>
Cc: David Miller <davem@davemloft.net>, Jiri Benc <jbenc@redhat.com>,
	Netdev <netdev@vger.kernel.org>,
	Hannes Frederic Sowa <hannes@stressinduktion.org>,
	Alexander Duyck <aduyck@mirantis.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH net-next] net/vxlan: Avoid unaligned access in vxlan_build_skb()
Date: Fri, 23 Sep 2016 13:20:59 -0400	[thread overview]
Message-ID: <20160923172059.GB2484@oracle.com> (raw)
In-Reply-To: <CAKgT0UcoDb5T9xwfAq83cNwU=XAKZxf0TmPiJZQyHFPr82zggg@mail.gmail.com>

On (09/23/16 07:17), Alexander Duyck wrote:
> >> Is this basically about, e.g., putting the vxlanhdr in its own
> >> skb_frag_t, or something else?
> >
> > Yes, and this way skb_header_pointer() is forced to do a memcpy.
  :
> For Tx it all becomes a bit trickier since it would likely require us
> to shift the frags up by 1 when we go from outer headers to inner
> headers.  

here's how I thought through this so far, based on what I'm seeing for 
mld_newpack/vxlan (not sure if this can be extended for all the 
other tunnelling cases as well)..

today the skb is set up so that we reserve LL_RESERVED_SPACE
in the headroom, and vxlan sets up needed headroom for 
(outer_ether + ip + udp + vxlan + inner_ether). Insterad, if it
set up the needed_headroom for just (outer_ether, ip, udp) and
we had something like a "needed_fragroom" in the net_device, 
maybe we could set up the skb so that we dont have to shift the frags
by 1. 

Drawbacks: this ends up with every skb going through vxlan etc being
non-linear, so it is a lot of churn for several functions (e.g.,
even mld_newpack() cannot just skb_put() things around). Also
this probably gets quickly messy if we are dealing with multiple 
encaapsulations (even in the simple vxlan case we have 
vxlan + inner mac/ip/etc)

BTW, I wonder if there is a small vxlan bug here- are we
accounting for the outer_ether twice in LL_RESERVED_SPACE: once in 
->hard_header_len, and once in ->needed_headroom?

> One thought I had on that is that we could possibly avoid
> having to do any allocation and could just take a reference on the
> head_frag if that is what we are using.  Then we just add a 2 byte pad
> and resume writing headers in place and the pointer offsets for the
> inner headers would remain valid, though they would be past the point
> of skb->tail.

I am not sure I follow, can you elaborate? Doesnt this also assume
that every skb is necessarily non-linear?

--Sowmini

  reply	other threads:[~2016-09-23 17:21 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-20 14:27 [PATCH net-next] net/vxlan: Avoid unaligned access in vxlan_build_skb() Sowmini Varadhan
2016-09-20 15:31 ` Tom Herbert
2016-09-20 15:49   ` Sowmini Varadhan
2016-09-20 16:11 ` Jiri Benc
2016-09-20 16:31   ` Sowmini Varadhan
2016-09-20 16:43     ` Jiri Benc
2016-09-20 17:07       ` Sowmini Varadhan
2016-09-20 17:15         ` Jiri Benc
2016-09-20 17:09       ` Jiri Benc
2016-09-20 17:19         ` Tom Herbert
2016-09-20 17:24         ` Sowmini Varadhan
2016-09-22  5:52         ` David Miller
2016-09-22 21:30           ` Sowmini Varadhan
2016-09-23 12:06             ` David Miller
2016-09-23 14:17               ` Alexander Duyck
2016-09-23 17:20                 ` Sowmini Varadhan [this message]
2016-09-23 17:38                   ` Alexander Duyck
2016-09-23 23:41                     ` Sowmini Varadhan
2016-09-24  0:43                       ` Alexander Duyck
2016-09-28 17:03                         ` Sowmini Varadhan
2016-09-28 18:08                           ` Alexander Duyck
2016-09-28 19:56                             ` Sowmini Varadhan
2016-09-20 16:45 ` Hannes Frederic Sowa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160923172059.GB2484@oracle.com \
    --to=sowmini.varadhan@oracle.com \
    --cc=aduyck@mirantis.com \
    --cc=alexander.duyck@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=hannes@stressinduktion.org \
    --cc=jbenc@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.