[ovs-dev] [PACKET_OUT v2] ofproto-dpif: treat non-datapath ports as local port for OFPT_PACKET_OUT

[YAMAMOTO Takashi]

hi,

> Yamamoto San, thanks for following up on this issue.  Yes, you are
> right,  both DP_HASH and RECIRC actions are not handled in
> SLOW_ACTION.
> 
> RECIRC action can be handled like output action, by using the helper function.
> 
> DP_HASH action, in theory, can also be handled in a helper function so
> that the hash value can be calculated by the datapath.
> On the other hand, this seems overkill (in terms of overhead), at
> least with the current bond use cases.  If we simply supply a
> user space hash implementation, the worst case is that user space flow
> handling may choose a different bond port than
> the port chosen by the datapath port. This can also happen with bond
> load re-balancing.   In summary, handling DP_HASH
> in user space seems to be the best option for the time being.

what would the new definition of DP_HASH action look like?

YAMAMOTO Takashi

> 
> Please let me know if you have additional comments. I will work out a
> patch in the mean time.
> 
> On Thu, May 22, 2014 at 11:01 PM, YAMAMOTO Takashi
> <yamamoto at valinux.co.jp> wrote:
>>>> On Wed, Apr 30, 2014 at 11:59 PM, YAMAMOTO Takashi
>>>> <yamamoto at valinux.co.jp> wrote:
>>>>>>> Something like this (I have not tested either scenario):
>>>>>>>
>>>>>>>   I think this will fail to match but that may not be obvious to users:
>>>>>>>   packet_out: in_port=CONTROLLER actions=goto_table:1
>>>>>>>   table 1: match=in_port=CONTROLLER actions=normal
>>>>>>>
>>>>>>>   I think this will match but that may not be obvious to users:
>>>>>>>   packet_out: in_port=CONTROLLER actions=goto_table:1
>>>>>>>   table 1: match=in_port=LOCAL actions=normal
>>>>>>>
>>>>>>>   Where CONTROLLER could be any port covered by this patch.
>>>>>>>
>>>>>> Only ODP ports are changed to LOCAL, so simple rule matches as
>>>>>> outlined should work.
>>>>>> However, this scenario is valid when recirculation is involved. I am
>>>>>> not sure what we should
>>>>>> do about this edge case either.
>>>>>
>>>>> how about having a userspace-only special variant of RECIRC action,
>>>>> which can specify in_port as (ofproto, ofp port) and thus can be
>>>>> used even when odp port is not available?
>>>>>
>>>>> if a translation detects the condition (ie. recirculation is necessary
>>>>> but in_port does not have the corresponding odp port), it can use
>>>>> the userspace variant with SLOW_ACTION instead of normal RECIRC.
>>>>
>>>> Not all actions can be executed in the user space. Hash action can
>>>> only be executed
>>>> in datapath. User space, by design, can not predict the hash value
>>>> computed by the
>>>> datapath.
>>>
>>> then HASH action ought to be handle specially.  (like OUTPUT?)
>>>
>>> the current code seems broken in that regard.
>>> if some SLOW_ACTIONs are involved, the upcall hander would
>>> use DPIF_OP_EXECUTE with needs_help=true, which ends up with
>>> dpif_execute_with_help, to execute actions, which might include
>>> HASH and RECIRC.  but dpif_execute_helper_cb can't handle these
>>> actions.  ie. OVS_NOT_REACHED in dpif_execute_helper_cb seems
>>> actually reachable.
>>>
>>> YAMAMOTO Takashi
>>
>> here's a recipe to demonstrate the above mentioned bug in a sandbox.
>> andy, can you take a look?
>>
>> YAMAMOTO Takashi
>>
>>
>> ovs-vsctl --if-exists del-br s1
>> ovs-vsctl --if-exists del-br s2
>> ovs-vsctl --if-exists del-br s3
>>
>> ovs-vsctl add-br s1 -- set bridge s1 datapath_type=netdev protocols='[OpenFlow10,OpenFlow13]'
>> ovs-vsctl add-br s2 -- set bridge s2 datapath_type=netdev protocols='[OpenFlow10,OpenFlow13]'
>> ovs-vsctl add-br s3 -- set bridge s3 datapath_type=netdev protocols='[OpenFlow10,OpenFlow13]'
>>
>> ovs-vsctl add-port s1 p7 -- set interface p7 type=patch options:peer=p8
>> ovs-vsctl add-port s2 p8 -- set interface p8 type=patch options:peer=p7
>>
>> ovs-vsctl add-bond s2 b1 p1 p2 lacp=active bond_mode=balance-tcp
>> ovs-vsctl add-bond s3 b2 p3 p4 lacp=active bond_mode=balance-tcp
>> ovs-vsctl set interface p1 options:stream=unix:p1
>> ovs-vsctl set interface p2 options:stream=unix:p2
>> ovs-vsctl set interface p3 options:pstream=punix:p1
>> ovs-vsctl set interface p4 options:pstream=punix:p2
>>
>> ovs-appctl netdev-dummy/set-admin-state up
>>
>> ovs-ofctl add-flow s2 'arp, actions=set_field:11:11:11:11:11:11->arp_sha,output:normal'
>>
>> ovs-appctl netdev-dummy/receive s2 'in_port(2),eth(src=50:54:00:00:00:05,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=192.168.0.1,tip=192.168.0.2,op=1,sha=50:54:00:00:00:05,tha=00:00:00:00:00:00)'
>>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev