On Montag, 13. Februar 2017 21:00:08 CET Linus Lüssing wrote: [...]
And one more thing which seems fishy to me in this function:
526 /* Make room for the fragment header. */ 527 if (batadv_skb_head_push(skb, header_size) < 0 || 528 pskb_expand_head(skb, header_size + ETH_HLEN, 0, GFP_ATOMIC) < 0) { 529 ret = -ENOMEM; 530 goto put_primary_if; 531 } 532 533 memcpy(skb->data, &frag_header, header_size);
For the pskb_expand_head() case, there is an skb_push(header_size) missing, isn't it?
I am a little bit confused about your remark... and about the code.
So let's check what Martin wrote:
* get header_size more room in our data section * allocate new buffer to get header_size + ETH_HLEN in front (but not part) of our data section
If one of these two fails then it will get in panic mode and leave the function.
I agree that the header_size in pskb_expand_head is slightly odd and I don't see why we would need it. My best guess would be to compensate the extra header which "stole" some bytes from the headroom which the underlying interface may need.
But more importantly, I don't understand why an extra skb_push(header_size) (like you've suggested) would be necessary here. Why would you want to have an empty header_size region in the fragment between the actual header and the fragment data?
Kind regards, Sven