Re: Kernel memory leak in bnx2x driver with vxlan tunnel

On Tue, Jan 19, 2016 at 4:17 PM, Eric Dumazet [off-list ref] wrote:

On Tue, 2016-01-19 at 16:00 -0800, Jesse Gross wrote:

quoted

Oh, I wasn't disagreeing that there is a problem. I was just trying to
point out the conceptual root of the problem rather than the
individual instances in different tunnels implementations.

In any case, I think we can solve this by calling
skb_release_head_state() in napi_skb_finish() and switching early
demux to use skb_valid_dst().

Yes, but it will also have to call skb_dst_drop() or risk a leak of
prior one.

skb_dst_drop() is the key part but I mentioned
skb_release_head_state() (which calls skb_dst_drop()) as a slightly
more generic solution. But either one is fine.

So what is the purpose of having a dst if we need to drop it ?

Adding code in GRO would be fine if someone explains me the purpose of
doing apparently useless work.

(refcounting on dst is not exactly free)

In the GRO case, the dst is only dropped on the packets which have
been merged and therefore need to be freed (the GRO_MERGED_FREE case).
It's not being thrown away for the overall frame, just metadata that
has been duplicated on each individual frame, similar to the metadata
in struct sk_buff itself. And while it is not used by the IP stack
there are other consumers (eBPF/OVS/etc.). This entire process is
controlled by the COLLECT_METADATA flag on tunnels, so there is no
cost in situations where it is not actually used.