Re: [PATCH iproute2 V3 3/3] tc/act_tunnel: Introduce ip tunnel action

On Wed, Nov 30, 2016 at 03:44:53PM +0100, Jiri Benc wrote:

On Wed, 30 Nov 2016 09:38:40 +0200, Amir Vadai wrote:

quoted

+The
+.I UNSET
+mode is optional - even without using it, the metadata information will be
+released automatically when packet processing will be finished.
+.IR UNSET
+function could be used in cases when traffic is forwarded between two tunnels,
+where the metadata from the first tunnel will be used for encapsulation done by
+the second tunnel.

This looks good.

:)

quoted

+It must be used for offloaded filters, such that hardware drivers can
+realize they need to program the HW to do decapsulation.

However, this is wrong. The hardware offloading must be transparent.
The same configuration that works when processed in software must work
in hardware if the hardware has the necessary capabilities. Requiring
the user to alter the configuration to accommodate hardware
peculiarities is not acceptable.

Or maybe I'm misunderstanding what you mean here. In which case it's
not documented properly :-)

You understood it correctly. We should not force the user to use the
'unset' operation for offloading only. I will remove it from the text
here.

quoted

+.IR SET
+mode requires the source and destination ip
+.I ADDRESS
+and the tunnel key id
+.I KEY_ID
+which will be used by the ip tunnel shared device to create the tunnel header. The
+.B tunnel_key
+action is useful only in combination with a
+.B mirred redirect
+action to a shared IP tunnel device which will use the metadata (for
+.I SET
+) and unset the metadata created by it (for
+.I UNSET
+).
+
+.SH OPTIONS
+.TP
+.B unset
+Decapsulation mode, no further arguments allowed. This function is not
+mandatory and might be used only in some specific use cases.

This is NOT decapsulation. The packet is decapsulated at this point in
any case, whether or not set/unset or whatever is used. These actions
are only and solely about metadata associated with the packet. The
actual encapsulation and decapsulation happens at the target netdevice.

Calling this "decapsulation" is wrong. And if it's implemented as such
in your hardware offloading, then it's doubly wrong as it doesn't match
software processing and hence you must not do that and you must change
that.

Got it. Bad wording by me - will fix it and make sure offloading will
realy be transparent to user.

quoted

+.TP
+.B set
+Encapsulation mode. Requires

Likewise, this is not encapsulation. It just sets metadata.

ack

Thanks,
Amir

 Jiri