On Mon, Dec 22, 2014 at 1:30 PM, Jesse Gross <je...@nicira.com> wrote: > On Mon, Dec 22, 2014 at 3:08 PM, Pravin Shelar <pshe...@nicira.com> wrote: >> On Mon, Dec 22, 2014 at 11:56 AM, Jesse Gross <je...@nicira.com> wrote: >>> On Mon, Dec 22, 2014 at 1:25 PM, Pravin Shelar <pshe...@nicira.com> wrote: >>>> On Sun, Dec 21, 2014 at 9:39 PM, Jesse Gross <je...@nicira.com> wrote: >>>>> On Sun, Dec 21, 2014 at 6:17 PM, Jesse Gross <je...@nicira.com> wrote: >>>>>> On Fri, Dec 19, 2014 at 7:25 PM, Pravin B Shelar <pshe...@nicira.com> >>>>>> wrote: >>>>>>> Today vport-send has complex error handling because it involves >>>>>>> freeing skb and updating stats depending on return value from >>>>>>> vport send implementation. >>>>>>> This can be simplified by delegating responsibility of freeing >>>>>>> skb to the vport implementation for all cases. So that >>>>>>> vport-send needs just update stats. >>>>>>> >>>>>>> Signed-off-by: Pravin B Shelar <pshe...@nicira.com> >>>>>> >>>>>> It's somewhat non-obvious to me that handle_offloads should free the >>>>>> skb in the event of an error. It seems like this introduces more >>>>>> complexity to this patch as well. >>>>>> >>>>>> As an example of the problem, I think that the previous patch >>>>>> introduces a double free in the non-compat case. This is because it >>>>>> updates the skb freeing for the compat code in handle_offloads() but >>>>>> the equivalent update to the non-compat code isn't made until this >>>>>> patch. >>>>> >>>>> I now see that the upstream iptunnel_handle_offloads() also frees the >>>>> skb on error but it seems like this has introduced some errors as >>>>> well. It looks like several tunnel protocols continue using the skb >>>>> after calling handle_offloads without any kind of check. >>>> >>>> right, Thats the reason. >>>> I am not sure which one still missing the check, Can you point me to >>>> them? I have fixed GRE already. >>> >>> It looks like Geneve is the only one at this point - I thought that >>> VXLAN had a similar problem but I was looking at an old version of the >>> code. >> >> OK, you are looking at upstream Geneve code. > > Yes - I was referring to the upstream tunnel implementations before. > That's what I meant, it seemed like these bugs were somewhat > pervasive. Since almost all of the implementations have been fixed, I > guess it makes sense to just leave it as is and mirror the upstream > into the compat code. Will you send a patch for Geneve upstream as > part of this?
I also want to mirror upstream code for compat. the patch series is first step. I will send vport_send() change and Geneve fix upstream. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
