I’ll review the reminder of the code too since it looks to be closer to
the final version.
If you are interesting in making the patch smaller, I’d recommend that you
move the changes to OvsTunnelAttrToIPv4TunnelKey() into a separate patch.
-- Nithin
-----Original Message-----
From: dev <dev-boun...@openvswitch.org> on behalf of Nithin Raju
<nit...@vmware.com>
Date: Tuesday, May 24, 2016 at 9:32 AM
To: Yin Lin <li...@vmware.com>, "dev@openvswitch.org" <dev@openvswitch.org>
Subject: Re: [ovs-dev] [PATCH 2/2] [PATCH v4] datapath-windows: Add Geneve
support
>Hi Yin,
>You also need to update datapath-windows/automake.mk to add the new files
>you added.
>
>-----Original Message-----
>From: dev <dev-boun...@openvswitch.org> on behalf of Yin Lin
><li...@vmware.com>
>Date: Friday, May 20, 2016 at 1:57 PM
>To: "dev@openvswitch.org" <dev@openvswitch.org>
>Subject: [ovs-dev] [PATCH 2/2] [PATCH v4] datapath-windows: Add
>Geneve support
>
>> case IPPROTO_UDP:
>> tunnelVport =
>>OvsFindTunnelVportByDstPort(ovsFwdCtx->switchContext,
>>- dstPort,
>>-
>>OVS_VPORT_TYPE_VXLAN);
>>+ dstPort);
>> if (tunnelVport) {
>>- ovsActionStats.rxVxlan++;
>>+ if (tunnelVport->ovsType == OVS_VPORT_TYPE_VXLAN) {
>>+ ovsActionStats.rxVxlan++;
>>+ } else {
>>+ ASSERT(tunnelVport->ovsType ==
>>OVS_VPORT_TYPE_GENEVE);
>>+ ovsActionStats.rxGeneve++;
>>+ }
>
>You cannot remove the port type from OvsFindTunnelVportByDstPort(). OVS
>should support the case where there¹s a STT port (ie. TCP port) on port
>XYZ, and a VXLAN port (i.e.. UDP) on port XYZ. I am not sure if you need
>to support a case with Geneve and VXLAN ports on the same UDP port #.
>
>
>Your code changes are mostly right, except that you should not be removing
>the port type parameter in OvsFindTunnelVportByDstPort(). If you have
>concurrent STT and VXLAN ports on the same L4 port number, we¹ll end up
>returning the first port in the hash table, and that is not the correct
>behavior.
>
>If we are not going to support the case where Geneve and VXLAN ports can
>co-exist on the same UDP port #, we should add a check in the port add
>functionality.
>
>Thanks,
>-- Nithin
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pN
>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=i3FRL3zYPSXu2hduqcxST52V-73irB
>w7HzZilzhX32g&s=WCyaO0MAZPFzldjumzehfvvn0OZ-v293izABXwLDfXU&e=
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
