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

[Amir Vadai <amir@vadai.me>]

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:
> > +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.
:)

> 
> > +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.

> 
> > +.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.

> 
> > +.TP
> > +.B set
> > +Encapsulation mode. Requires
> 
> Likewise, this is not encapsulation. It just sets metadata.
ack

Thanks,
Amir
> 
>  Jiri