Skip to contentSkip to conversation

skb free patches

DISCUSSION Martin Townsend 2 participants 2 messages Oct 28, 2014 – Nov 4, 2014 1 message not in the archive IEEE 802.15.4

lore
Share & tools
lore
#0 lore raw

Hi Alex,

I've prepared the patches for freeing skb outside of the decompress routines but now I'm unsure whether this is the best thing to do.

Plus side
it makes the decompression routine neater, no goto's, just return an error code.
Centralises the freeing of the skb.
Easier to maintain, future decompression routines don't have to worry about making sure all error paths have freed the skb.

Negative side
Decompression routines musn't reallocate the skb, ie make a copy. The skb pointer that is passed in must still be valid on return otherwise we get a double free. I think skb_cow is ok as it calls pskb_expand_head which just makes more room in the data but if a routine was to use skb_copy then I think we would be in trouble.

Maybe decompression routines should only be using routines like skb_cow as they are more effecient. What are your thoughts before I send the series?

- Martin.

A message that is not in the archive (sent off-list or not yet received): [email protected]

maintainer
#2 lore raw

Hi Martin,

sorry, I didn't notice this mail.

Quoted 4 lines · Martin Townsend

Hi Alex,

Any thoughts on this?

go ahead and just send patches. For the future I want to change the 802.15.4 6LoWPAN to a common solution. We should do it like 802.15.4 parsing. Not like current mainline, we should do it like frame parsing in the rework branch [0]. I stole this kind of frame parsing mechanism from mac80211. For adaptation in 6LoWPAN just replace the 802.15.4 frame type to the 6LoWPAN dispatch value and you will get something like this [1].

Nevertheless your solution to have a kfree_skb outside of iphc_decompress is welcome here. Because the CALL_RXH function returns on failure 'RX_DROP_UNUSABLE' and hits the handler result function [3] which runs kfree_skb there.

For now to prepare this outside I will ack this patch. But we are not alone anymore this needs also an ack from Jukka Rissanen. Check out:

./scripts/get_maintainer.pl -f net/6lowpan

This architecture change need to be acked from both L2 branches which use 6LoWPAN. Please tag your patch with "6lowpan: $MY_GREAT_CHANGE" and send it to linux-wpan and linux-bluetooth mailinglist, you can get all cc members with:

./scripts/get_maintainer.pl -f net/6lowpan

or

./scripts/get_maintainer.pl $MY_GREAT_PACH_FILE

- Alex

[0] https://github.com/linux-wpan/linux-wpan-next/blob/wpan_rework_rfc/net/mac802154/rx.c
[1] https://github.com/linux-wpan/linux-wpan-next/blob/wpan_rework_rfc/net/ieee802154/6lowpan/rx.c#L174
[3] https://github.com/linux-wpan/linux-wpan-next/blob/wpan_rework_rfc/net/ieee802154/6lowpan/rx.c#L27