[ovs-dev] [PATCHv6 1/3] ofp-actions: Add truncate action.

[William Tu]

Hi Ben,

Thanks for the feedback.

>> Signed-off-by: William Tu <u9012063 at gmail.com>
>
> I'm a bit nervous about cutlen.  Can a packet change, for example by
> popping a VLAN header, after cutlen is set?  If so, can this cause the
> packet length to drop below 0, or below 14?
>

I don't want this happen. As a result, at OpenFlow side I only expose
output(max_len=n,port=m) and in datapath, making truncate action
immediately followed by output action. So, after cutlen is set, it
immediately outputs to a port.

However, if the output port is a patch port, then we might have other
actions come in between truncate and output action. And if it is
popping a vlan header, then packet length could drop to below 60 byte
or lower depends on number of vlan_pop. For this reason, the current
patch disallows output to the patch port.

For truncate and output to tunnel port (userspace datapath), we apply
cutlen on the cloned packet before push and pop.

> xlate_output_trunc_action() disallows all special OFPP_* ports.  Is
> there a reason to disallow output_trunc to OFPP_LOCAL or OFPP_IN_PORT?
>
In the beginning I couldn't think of any use case of OFPP_LOCAL or
OFPP_IN_PORT so I disallow them. I could enable it in next version.

> If special OFPP_* ports are disallowed, should we disallow them at
> action decode time, in ofp-actions.c, instead of waiting until action
> translation?
>
yes I will do that.

> In xlate_output_trunc_action(), s/unipredicable/unpredictable/.  Also in
> the same error message it's probably worth giving the name or the number
> of the output port.
>
Thanks,
William