On Mon, Feb 6, 2017 at 9:15 AM, David Miller <da...@davemloft.net> wrote: > From: Pravin Shelar <pshe...@ovn.org> > Date: Mon, 6 Feb 2017 09:06:29 -0800 > >> On Sun, Feb 5, 2017 at 2:28 PM, David Miller <da...@davemloft.net> wrote: >>> From: Jarno Rajahalme <ja...@ovn.org> >>> Date: Thu, 2 Feb 2017 17:10:00 -0800 >>> >>>> This does not match either of the conntrack tuples above. Normally >>>> this does not matter, as the conntrack lookup was already done using >>>> the tuple (B,A), but if the current packet does not match any flow in >>>> the OVS datapath, the packet is sent to userspace via an upcall, >>>> during which the packet's skb is freed, and the conntrack entry >>>> pointer in the skb is lost. >>> >>> This is the real bug. >>> >>> If the metadata for a packet is important, as it obviously is here, >>> then upcalls should preserve that metadata across the upcall. This >>> is exactly how NF_QUEUE handles this problem and so should OVS. >> >> OVS kernel datapath serializes skb context and sends it along with skb >> during upcall via genetlink parameters. userspace echoes same >> information back to kernel modules. This way OVS does maintains >> metadata across upcall. > > Then the conntrack state looked up before the NAT operation done in > the upcall should be maintained and therefore this bug should not > exist.
I think it fails because after upcall OVS is performing lookup for nonexistent conntrack entry. This is due to the fact that the packet is already Nat-ed. So one could argue that there is already enough context available in OVS to lookup original conntract entry after the upcall as shown in this patch. But I am also fine with using original context to lookup the conntract entry as Joe has suggested.
