[ovs-dev] [PATCH] ofproto: fix incorrect replacement semantics when modifying rules

[Ilya Maximets]

On 11/6/20 9:44 AM, wangyunjian wrote:
> From: Yunjian Wang <wangyunjian at huawei.com>
> 
> Currently we copy values from old rule for modify variables(idle_timeout,
> hard_timeout, importance, flags) in new rule, which causes a failure to
> modify flow. We should not override these variables. This commit fixes
> this issue.
> 
> The steps to reproduce the problem are as follows.
> 1. ovs-ofctl add-flow br0 "table=0,in_port=1,hard_timeout=200 actions=learn
>    (table=0,in_port=2,hard_timeout=20 output:OXM_OF_IN_PORT[]),output:2"
> 2. ovs-ofctl mod-flows br0 "table=0,in_port=1,hard_timeout=100 actions=learn
>    (table=0,in_port=2,hard_timeout=20 output:OXM_OF_IN_PORT[]),output:2"
> 3. ovs-ofctl dump-flows br0
>     cookie=0x0, duration=16.505s, table=0, n_packets=0, n_bytes=0,
>     hard_timeout=200, in_port=vnet0 actions=learn(table=0,hard_timeout=20,
> 	in_port=vnet1,output:OXM_OF_IN_PORT[]),output:vnet1
> Step 2 will modify the rule of flow created in step 1, however, the dump
> result of step 3 is still the old one.

This looks like a correct behavior.  According to OpenFlow 1.5 specification [1]:
"""
The idle_timeout and hard_timeout fields control how quickly flow entries
expire (see 6.5).  When a flow entry is inserted in a table, its idle_timeout
and hard_timeout fields are set with the values from the message.  When a flow
entry is modified (OFPFC_MODIFY or OFPFC_MODIFY_STRICT messages), the
idle_timeout and hard_timeout fields are ignored.
"""

Similar statement for OF 1.0 [2]:
"""
For  MODIFY  requests,  if  a  flow  entry  with  identical  header  fields

does  not current reside in any table, the MODIFY acts like an ADD, and the new
flow entry  must  be  inserted  with  zeroed  counters.   Otherwise,  the  actions
field  is changed on the existing entry and its counters and idle time fields are
left unchanged.

"""

So, to change timeouts you need to remove flow and add a new one.  Flow modification
only updates actions.

[1] https://opennetworking.org/wp-content/uploads/2014/10/openflow-switch-v1.5.1.pdf
[2] https://opennetworking.org/wp-content/uploads/2013/04/openflow-spec-v1.0.0.pdf

Best regards, Ilya Maximets.