[ovs-dev] [PATCH 3/9] Add OF11 set IP ECN action.

[Jarno Rajahalme]

On Oct 17, 2013, at 3:02 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Thu, Oct 17, 2013 at 02:59:07PM -0700, Jarno Rajahalme wrote:
>> 
>> On Oct 17, 2013, at 11:47 AM, Ben Pfaff <blp at nicira.com> wrote:
>> 
>>> On Wed, Oct 16, 2013 at 04:16:05PM -0700, Jarno Rajahalme wrote:
>>>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
>>> 
>>> This raises an issue that I had resolved one way, and you resolved
>>> another (even if you did not realize it).  I am not certain that I chose
>>> the right way, so let me present the issue for discussion.
>>> 
>>> The definition of the "set-ecn" action in OpenFlow 1.1 is:
>>> 
>>>   Replace the existing IP ECN value and up-
>>>   date the IP checksum. Only applies to IPv4
>>>   packets.
>>> 
>>> It's the "only applies to IPv4" that concerns me.  According to a plain
>>> reading of the specification, this makes the action do nothing (or be
>>> invalid) for IPv6.  But a spec standards lawyer[*] would notice that
>>> OF1.1 did not support IPv6, and so "only applies to IPv4" could be read
>>> as meaning that the action does not apply to the other protocols that
>>> OF1.1 does support (e.g. it is meaningless for ARP, MPLS, ?)
>> 
>> I did not read it in the sense of "must not apply on IPv6". Considering that even OF1.1 *has* minimal support for IPv6, in the sense that it is legal to match on IPv6 ether type, in which case my reading of the spec would change the ECN in a case where your more strict reading would not.
>> 
>>>  [*] Catch me playing a spec standards lawyer in a special guest
>>>      appearance on an upcoming episode of "CSI: OpenFlow", next
>>>      Wednesday at ten (nine central)!
>>> 
>>> I interpreted this according to the plain wording in other cases, and
>>> so, for example, OFPAT_SET_DSCP only sets the DSCP for IPv4 and becomes
>>> a no-op for IPv6.  Maybe I was wrong.  Either way, I would like to be
>>> consistent here.
>> 
>> I noticed this, and kept wondering why, without thinking to read the spec that strictly.
>> I think your reading is safer, since a controller might actually depend on it.
> 
> OK.  I'm happy with that (also would have been happy the other way).
> Thanks.

I went along with the direction of allowing these actions only for IPv4, as the spec says. This brings difficulties down the road, however. Basically it becomes impossible to translate between these set actions and the OF 1.2+ set field action, due to the different semantics. I found that in some cases we can track that the action is being performed on an IPv4 packet, by checking the flow->dl_type while checking the actions. This check is not possible with flows that do not match on the dl_type, or any indirect actions, like with group buckets.

Therefore I feel the decision how we treat these actions also decides whether it makes sense to even try to translate the set actions deprecated in OF 1.2 to set field actions. I'm now inclined to allow the set actions on DSCP, ECN, and TTL to act on both IPv4 and IPv6 packets. Any possible negative impact of this choice is limited to an OF 1.1 controller setting these fields without matching the dl_type to IPv4 in any phase of a pipeline. I think the chances to encounter this in the wild are rather slim :-)

  Jarno