On Thu, Nov 24, 2016 at 04:33:55PM +0100, Jiri Benc wrote: > On Thu, 24 Nov 2016 17:06:33 +0200, Amir Vadai wrote: > > So you mean to just unconditionally call skb_dst_drop() from > > act_mirred()? > > That's one option. Or just leave the dst there, it shouldn't matter? > (Except for forwarding to a different tunnel but as I said, it's a > corner case and we may have a "tunnel_key unset" action for that.) Ok, so I will write in the docs that it is optional to use the "unset" operation (and will rename it from "release" to "unset")
> > > The use case we already have that uses the release action is the > > hardware offload support, which is already in the kernel. > > It is using the "tunnel_key release" action to signal the hardware to > > strip off the ip tunnel headers. > > The tunnel headers must be removed upon reception on the tunnel > interface without specifying anything, because that's how the Linux > kernel behaves currently. If this is offloaded, this behavior must be > preserved. I don't see how "tunnel_key release" might be used for > stripping the headers. Maybe I didn't express myself right: I need to tell the hardware explicitly during the filter initialization to redirect the packets arriving from one interface to another and to strip off the tunnel headers. This is what happens when a "tunnel_key unset" action is created and offloaded - it configures the hardware respectively. So this is one usecase where this operation is needed - and yes, in this use case the actual skb_dst_drop() is not important or needed, but I don't think it makes any harm. In the tunnel dev to tunnel dev use case, the operation could be meaningful, if the user don't want to reuse the metadata created by the origin tunnel dev. > > Jiri