Message ID | 1349442173.21172.66.camel@edumazet-glaptop |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
On Fri, 2012-10-05 at 15:02 +0200, Eric Dumazet wrote: > On Fri, 2012-10-05 at 14:51 +0200, Maxime Bizon wrote: > > > > New convention would be : pass number of needed bytes after current > > > tail, not after current end. > > > > Fully agree on this > > > > Here is the proposal : Looks fine What is your plan for the actual pskb_expand_head() code now that you will have absolute values for headroom & tailroom ? Because there will still be callers that have no clue of required tailroom (nor further headroom requirement), like skb_cow() in vlan_reorder_header().
On Fri, 2012-10-05 at 16:50 +0200, Maxime Bizon wrote: > On Fri, 2012-10-05 at 15:02 +0200, Eric Dumazet wrote: > > > On Fri, 2012-10-05 at 14:51 +0200, Maxime Bizon wrote: > > > > > > New convention would be : pass number of needed bytes after current > > > > tail, not after current end. > > > > > > Fully agree on this > > > > > > > Here is the proposal : > > Looks fine > > What is your plan for the actual pskb_expand_head() code now that you > will have absolute values for headroom & tailroom ? > They stay relative values. For example, netlink_trim() really wants to shrink the skb head. > Because there will still be callers that have no clue of required > tailroom (nor further headroom requirement), like skb_cow() in > vlan_reorder_header(). > What I plan is to not shrink size, unless specifically asked. Its 3.8 material anyway, so a stable fix is needed on skb_recycle() and NET_SKB_PAD minimal value. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2012-10-05 at 17:04 +0200, Eric Dumazet wrote: > > > What I plan is to not shrink size, unless specifically asked. > > Its 3.8 material anyway, so a stable fix is needed on skb_recycle() > and NET_SKB_PAD minimal value. You think removing skb_recycle() is too big a change for stable ? Driver change is simple, as recycling is not guaranteed today you have this: if (!try_recycle(skb)) skb = alloc_skb() > we just remove the try_recycle part, we are not adding any new code path. I'm not the right person to give you a correct NET_SKB_PAD value, I have a lot of out of tree patches in my kernels, some custom drivers as well that needs quite a lot of headroom, and uncommon network setup.
On Fri, 2012-10-05 at 17:15 +0200, Maxime Bizon wrote: > You think removing skb_recycle() is too big a change for stable ? > > Driver change is simple, as recycling is not guaranteed today you have > this: > > if (!try_recycle(skb)) > skb = alloc_skb() > > > we just remove the try_recycle part, we are not adding any new code > path. > > > I'm not the right person to give you a correct NET_SKB_PAD value, I have > a lot of out of tree patches in my kernels, some custom drivers as well > that needs quite a lot of headroom, and uncommon network setup. > OK, I'll send the patch to remove skb_recycle() -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c index 95a338c..15760df 100644 --- a/net/xfrm/xfrm_output.c +++ b/net/xfrm/xfrm_output.c @@ -24,18 +24,10 @@ static int xfrm_output2(struct sk_buff *skb); static int xfrm_skb_check_space(struct sk_buff *skb) { struct dst_entry *dst = skb_dst(skb); - int nhead = dst->header_len + LL_RESERVED_SPACE(dst->dev) - - skb_headroom(skb); - int ntail = dst->dev->needed_tailroom - skb_tailroom(skb); - - if (nhead <= 0) { - if (ntail <= 0) - return 0; - nhead = 0; - } else if (ntail < 0) - ntail = 0; - - return pskb_expand_head(skb, nhead, ntail, GFP_ATOMIC); + int nhead = dst->header_len + LL_RESERVED_SPACE(dst->dev); + int ntail = dst->dev->needed_tailroom; + + return pskb_may_expand_head(skb, nhead, ntail, GFP_ATOMIC); } static int xfrm_output_one(struct sk_buff *skb, int err)