On 02/05/2016 13:11, "Jarno Rajahalme" <ja...@ovn.org> wrote:

>
>> On Apr 29, 2016, at 5:38 PM, Daniele Di Proietto <diproiet...@vmware.com> 
>> wrote:
>> 
>> flow_wc_map() should include 'tp_src' and 'tp_dst' for ICMPv6 packet,
>> since they're used for ICMPv6 code and type.
>> 
>> This caused installed flows in the userspace datapath to always have
>> ICMPv6 code and type wildcarded (there are no other users of this
>> function).
>> 
>
>Thanks for fixing this. While reviewing I noticed that similar issue exists 
>for IGMP. It also use the tp_src and tp_dst fields, but they are not included 
>in that case. Could you fix that too?

Nice catch!  I've included fix for IGMP as well

>Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>

Thanks, I pushed this to master and branch-2.5

>
>  Jarno
>
>> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>> ---
>> lib/flow.c            |  4 ++--
>> tests/ofproto-dpif.at | 31 +++++++++++++++++++++++++++++++
>> 2 files changed, 33 insertions(+), 2 deletions(-)
>> 
>> diff --git a/lib/flow.c b/lib/flow.c
>> index 560a90f..2521f18 100644
>> --- a/lib/flow.c
>> +++ b/lib/flow.c
>> @@ -1421,6 +1421,8 @@ flow_wc_map(const struct flow *flow, struct flowmap 
>> *map)
>>         FLOWMAP_SET(map, nw_frag);
>>         FLOWMAP_SET(map, nw_tos);
>>         FLOWMAP_SET(map, nw_ttl);
>> +        FLOWMAP_SET(map, tp_src);
>> +        FLOWMAP_SET(map, tp_dst);
>> 
>>         if (OVS_UNLIKELY(flow->nw_proto == IPPROTO_ICMPV6)) {
>>             FLOWMAP_SET(map, nd_target);
>> @@ -1428,8 +1430,6 @@ flow_wc_map(const struct flow *flow, struct flowmap 
>> *map)
>>             FLOWMAP_SET(map, arp_tha);
>>         } else {
>>             FLOWMAP_SET(map, tcp_flags);
>> -            FLOWMAP_SET(map, tp_src);
>> -            FLOWMAP_SET(map, tp_dst);
>>         }
>>     } else if (eth_type_mpls(flow->dl_type)) {
>>         FLOWMAP_SET(map, mpls_lse);
>> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>> index e7445ac..53c512f 100644
>> --- a/tests/ofproto-dpif.at
>> +++ b/tests/ofproto-dpif.at
>> @@ -7269,6 +7269,37 @@ 
>> icmp6,vlan_tci=0x0000,dl_src=00:00:86:05:80:da,dl_dst=00:60:97:07:69:ea,ipv6_src
>> OVS_VSWITCHD_STOP
>> AT_CLEANUP
>> 
>> +AT_SETUP([ofproto-dpif - ICMPv6 type match])
>> +OVS_VSWITCHD_START
>> +add_of_ports br0 1 2 3
>> +
>> +AT_CHECK([ovs-ofctl add-flow br0 'icmp6,icmp_type=128,actions=2'])
>> +AT_CHECK([ovs-ofctl add-flow br0 'icmp6,icmp_type=129,actions=3'])
>> +
>> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
>> +
>> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
>> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=128)'])
>> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
>> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=128)'])
>> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
>> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=129)'])
>> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 
>> 'recirc_id(0),in_port(1),eth(src=f2:49:6e:52:49:0b,dst=02:b7:d7:17:ff:72),eth_type(0x86dd),ipv6(proto=58,frag=no),icmpv6(type=129)'])
>> +
>> +AT_CHECK([ovs-appctl revalidator/purge], [0])
>> +
>> +AT_CHECK([strip_ufid < ovs-vswitchd.log | filter_flow_install | 
>> strip_used], [0], [dnl
>> +recirc_id=0,icmp6,in_port=1,vlan_tci=0x0000,nw_frag=no,icmp_type=0x80/0xff, 
>> actions:2
>> +recirc_id=0,icmp6,in_port=1,vlan_tci=0x0000,nw_frag=no,icmp_type=0x81/0xff, 
>> actions:3
>> +])
>> +
>> +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
>> + n_packets=2, n_bytes=124, icmp6,icmp_type=128 actions=output:2
>> + n_packets=2, n_bytes=124, icmp6,icmp_type=129 actions=output:3
>> +NXST_FLOW reply:
>> +])
>> +
>> +OVS_VSWITCHD_STOP
>> +AT_CLEANUP
>> +
>> +
>> AT_SETUP([ofproto-dpif - Neighbor Discovery set-field with checksum update])
>> OVS_VSWITCHD_START
>> add_of_ports br0 1
>> -- 
>> 2.1.4
>> 
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to