On Wed, Apr 22, 2015 at 11:31 AM, Jesse Gross <je...@nicira.com> wrote: > On Wed, Apr 22, 2015 at 11:09 AM, Pravin Shelar <pshe...@nicira.com> wrote: >> On Wed, Apr 22, 2015 at 10:52 AM, Jesse Gross <je...@nicira.com> wrote: >>> On Wed, Apr 22, 2015 at 10:03 AM, Pravin Shelar <pshe...@nicira.com> wrote: >>>> On Mon, Apr 20, 2015 at 9:54 PM, Jesse Gross <je...@nicira.com> wrote: >>>>> On Mon, Apr 20, 2015 at 7:33 PM, Pravin Shelar <pshe...@nicira.com> wrote: >>>>>> On Mon, Apr 20, 2015 at 5:56 PM, Jesse Gross <je...@nicira.com> wrote: >>>>>>> On Mon, Apr 20, 2015 at 3:54 PM, Pravin Shelar <pshe...@nicira.com> >>>>>>> wrote: >>>>>>>> On Mon, Apr 20, 2015 at 3:36 PM, Jesse Gross <je...@nicira.com> wrote: >>>>>>>>> On Mon, Apr 20, 2015 at 1:29 PM, Pravin B Shelar <pshe...@nicira.com> >>>>>>>>> wrote: >>>>>>>>>> diff --git a/datapath/linux/compat/stt.c >>>>>>>>>> b/datapath/linux/compat/stt.c >>>>>>>>>> new file mode 100644 >>>>>>>>>> index 0000000..209bf1a >>>>>>>>>> --- /dev/null >>>>>>>>>> +++ b/datapath/linux/compat/stt.c >>>>>>>>>> +static void update_headers(struct sk_buff *skb, bool head, >>>>>>>>>> + unsigned int l4_offset, unsigned int >>>>>>>>>> hdr_len, >>>>>>>>>> + bool ipv4, u32 tcp_seq) >>>>>>>>> [...] >>>>>>>>>> + skb->truesize = SKB_TRUESIZE(skb_end_offset(skb)) + >>>>>>>>>> skb->data_len; >>>>>>>>>> +} >>>>>>>>> >>>>>>>>> I wonder if there are any possible edge cases with resetting truesize >>>>>>>>> where the packet is still in someone's transmit queue (such as if we >>>>>>>>> are looping back packet). Do we need to orphan it first? >>>>>>>>> >>>>>>>> ok, I will orphan it in update_headers. >>>>>>> >>>>>>> Just to clarify - I was mostly just thinking aloud on orphaning it. >>>>>>> I'm not totally sure if that is the right thing to do or if this is >>>>>>> the right place to do it. I'm not sure what the conceptual >>>>>>> justification would be for it and it could potentially result in the >>>>>>> sender's buffers not being properly limited. Perhaps not resetting the >>>>>>> truesize is the right thing too... >>>>>>> >>>>>> I have seen warning msg if we do no keep truesize update along with >>>>>> changes to skb. >>>>> >>>>> Hmm, interesting, what is the warning? I don't think that I have seen >>>>> that before. >>>> >>>> Actually skb_try_coalesce() is updating it correctly. so there no need >>>> to change truesize anymore. I will update patch accordingly. >>> >>> That's much nicer. I also checked and other receive side code (like >>> TCP input) doesn't worry about the case where a local sender may still >>> be accounting for the packet since any type of loopback device does >>> call skb_orphan() in some form. >>> >>> I hate to bring this up but what about on transmit? In cases where we >>> merge or split skbs (skb_try_coalesce() and normalize_frag_list() >>> respectively) we do track the truesize for correctly for the result >>> but the individual pieces might not have the right destructors or >>> might not have their truesize updated for the destructor they do have. >> >> How about about we update merged skb stats (len, data_len, truesize) >> according to *delta_truesize we get from skb_try_coalesce() and then >> free the skb? > > I think that would work for the skb_try_coalesce() case (although I > would only worry about truesize, not the lengths). For > normalize_frag_list() I think we would either have to add a destructor > or not update truesize. I'm not sure that the condition where we have > frag_lists and a destructor actually ever happens in practice though.
I am not sure why do we need a destructor for normalize_frag_list changes. Even with the changes in the function truesize is consistent for given skb memory usage. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
