On Mon, Apr 25, 2016 at 2:35 PM, Pravin B Shelar <pshe...@ovn.org> wrote: > diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c > index eb397e8..a1b309a 100644 > --- a/datapath/linux/compat/stt.c > +++ b/datapath/linux/compat/stt.c > +static int coalesce_skb(struct sk_buff **headp) > +{ > + struct sk_buff *frag, *head = *headp; > + int tot_len = FRAG_CB(head)->first.tot_len; > + int err; > + > + if (!head->next) > + return 0; > + > + err = pskb_expand_head(head, skb_headroom(head), tot_len, GFP_ATOMIC); > + if (err) > + return err;
The headroom and tailroom size seem a little big - I think they are supposed to be the delta amount, not the absolute value that you need. > + if (unlikely(!__pskb_pull_tail(head, head->data_len))) > + BUG(); > + > + for (frag = head->next; frag; frag = frag->next) { > + skb_copy_bits(frag, 0, skb_put(head, frag->len), frag->len); > + } Are we missing a free somewhere of the old fragments that we copied in? > +#ifdef SKIP_ZERO_COPY > +static int __copy_skb(struct sk_buff *to, struct sk_buff *from, > + int *delta, bool *headstolen) [...] > + *headstolen = false; > + err = pskb_expand_head(to, skb_headroom(to), > + to->len + from->len, GFP_ATOMIC); I think that this allocates more memory than is necessary. I believe that it already takes the existing headroom into account so this would double it. It looks like in the tailroom we should be using to->data_len instead of to->len as well. > + if (unlikely(to->data_len || (from->len > skb_tailroom(to)))) > + return -EINVAL; This is probably a little overly cautious for a fast path check given that we just allocated space. > @@ -1154,8 +1247,15 @@ static struct sk_buff *reassemble(struct sk_buff *skb) [...] > + if (copied_skb) { > + if (headstolen) { > + skb->len = 0; > + skb->data_len = 0; > + skb->truesize -= delta; > + } > + } Do we need to make these adjustments at all? The packet is about to be freed shortly. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev