On Sun, May 22, 2016 at 9:44 PM, pravin shelar <pshe...@ovn.org> wrote:
> On Sat, May 21, 2016 at 9:12 AM, Jesse Gross <je...@kernel.org> wrote:
>> On Wed, May 4, 2016 at 4:35 PM, Pravin B Shelar <pshe...@ovn.org> wrote:
>>> From: Pravin B Shelar <pshe...@nicira.com>
>>>
>>> There is return type change in upstream handle-offload functions.
>>> Following patch brings these changes in.
>>>
>>> Signed-off-by: Pravin B Shelar <pshe...@ovn.org>
>>
>> I think at this point, I need to ask you to align these patches with
>> upstream commits as we have done in recent history for backports. It's
>> very hard to review as they are grouped here since the code doesn't
>> really correspond to any point in upstream and I also don't know
>> whether anything from other commits has been missed. In this case,
>> this patch mostly is a backport of aed069df ("ip_tunnel_core:
>> iptunnel_handle_offloads returns int and doesn't free skb") but not
>> exactly since it is out of order with respect to other upstream
>> commits and with some OVS changes mixed it.
>
> I did not aligned patches with upstream patches to make it easier for
> review since you just need to compare to latest net branch. There
> might be some changes in upstream code since I backported these
> patches, but those should be pretty minor. If I break it down
> according to upstream commits you will need to check with each
> upstream patch and then compare the kernel module with net branch to
> see if any commit it missing. So it is easier this way.
> If you still want it to breakup patches according to the commits I canget t
> send updated series.

The problem is that for a given patch in the series there is often no
analogue upstream. I think the only way that it's possible is if I
squash all the patches together, which obviously defeats the purpose
of breaking them up in the first place. I did try to flip ahead to see
what is coming but obviously I also missed some things.

> Specifically about this patch, which part is not related to the commit 
> aed069df?

I think the one that I noticed specifically was the kfree_skb() at the
end of geneve_build_skb(). This isn't part of either aed069df or the
current tree. It's not wrong as of this patch and the last patch in
the series adjusts things to match upstream but I'm not sure how to
review this unless it is as a fresh code review. I don't want to do
that though since the goal is just to get things to match upstream.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to