On Fri, 2018-12-21 at 08:53 +0100, Steffen Klassert wrote: > On fraglist GSO, we don't need to clone the original > skb. So we don't have anything to return to free. > Prepare GSO that it frees the original skb only > if the return pointer really changed. Fraglist > GSO frees the original skb itself on error and > returns -EREMOTE in this case.
I think it would be nicer preseving the same sematic with gro list, so that we don't have to add this special handling. e.g. calling skb_get(skb) in skb_segment_list() when successful, would avoid the special handling for the no error case (at the cost of 2 atomic ops per gso_list packet) > diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c > index 4ae87c5ce2e3..1941dc2a80a0 100644 > --- a/net/xfrm/xfrm_output.c > +++ b/net/xfrm/xfrm_output.c > @@ -183,7 +183,8 @@ static int xfrm_output_gso(struct net *net, struct sock > *sk, struct sk_buff *skb > BUILD_BUG_ON(sizeof(*IPCB(skb)) > SKB_SGO_CB_OFFSET); > BUILD_BUG_ON(sizeof(*IP6CB(skb)) > SKB_SGO_CB_OFFSET); > segs = skb_gso_segment(skb, 0); > - kfree_skb(skb); > + if (segs != skb) > + kfree_skb(skb); > if (IS_ERR(segs)) what if IS_ERR(segs) == -EREMOTE here? > return PTR_ERR(segs); > if (segs == NULL)
