From: Alexander Aring <alex.aring@gmail.com>
To: Martin Townsend <martin.townsend@xsilon.com>
Cc: Martin Townsend <mtownsend1973@gmail.com>,
linux-bluetooth@vger.kernel.org, linux-wpan@vger.kernel.org,
marcel@holtmann.org, jukka.rissanen@linux.intel.com
Subject: Re: [PATCH v5 bluetooth-next] 6lowpan: Use pskb_expand_head in IPHC decompression.
Date: Mon, 13 Oct 2014 11:41:00 +0200 [thread overview]
Message-ID: <20141013094058.GB20544@omega> (raw)
In-Reply-To: <543B9B4C.1010303@xsilon.com>
On Mon, Oct 13, 2014 at 10:28:44AM +0100, Martin Townsend wrote:
...
> >> If it's ok with everyone I'll change pskb_expand_head to skb_cow or skb_cow_head and leave the bluetooth code as it is with this v5 patch.
> > For me it's confusing, you do that on skb, but we never used skb
> > afterwards (maybe on freeing) but we use local_skb, the local_skb should
> > always be cloned because we running skb_clone before.
> skb_clone does ensure that the skb isn't shared anymore so I think it will be safe to remove the skb_share_check. Or I can move it to the start of the function like we do for 802.15.4?
We need to clarify the following points:
skb_is_shared:
It's used twice in some packet handler functions or elsewhere.
now a skb_clone:
A skb clone is only a copy of struct sk_buff, so we can safely run
skb_pull and skb_push, skb->dev = foo. Simple for modify skb attributes.
now a skb_copy:
It's a complete private copy with private data buffer which allow us to
make skb->data[#] = foobar;
Now skb_share_check do:
if skb_is_shared true, then make a clone. This doesn't allow to
manipulate the data, but skb_cow make a copy of the private data so this
is okay for IPHC call. Means after skb_cow, we have something like
skb_copy, but skb_cow should be faster. ;-)
Also skb_share_check checks if skb_is_shared is false, then we don't
need a skb_clone, because it's not shared, then we are allow to
manipulate skb attributes, because nobody else use it.
I would add skb_share_check at beginning of this function like 802.15.4
but the IPV6 DISPATCH of bluetooth do a complete copy of the data buffer
which seems not necessary, but this is another issue and maybe I think
that the IPV6 DISPATCH value isn't needed by bluetooth 6lowpan. <-- But I
am not sure about this. I also can't see that bluetooth 6lowpan runs
skb_pull for the one byte dispatch value. Jukka need to check that, if
he has time and feel like to doing it.
- Alex
prev parent reply other threads:[~2014-10-13 9:41 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-09 7:46 [PATCH v5 bluetooth-next] 6lowpan: Use pskb_expand_head in IPHC decompression Martin Townsend
2014-10-09 7:46 ` Martin Townsend
2014-10-10 19:41 ` Alexander Aring
2014-10-11 6:55 ` Alexander Aring
2014-10-11 7:36 ` Alexander Aring
2014-10-13 8:44 ` Martin Townsend
2014-10-13 8:55 ` Alexander Aring
2014-10-13 9:28 ` Martin Townsend
2014-10-13 9:41 ` Alexander Aring [this message]
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=20141013094058.GB20544@omega \
--to=alex.aring@gmail.com \
--cc=jukka.rissanen@linux.intel.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-wpan@vger.kernel.org \
--cc=marcel@holtmann.org \
--cc=martin.townsend@xsilon.com \
--cc=mtownsend1973@gmail.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.