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 <[email protected]> on behalf of Nithin Raju <[email protected]> Date: Tuesday, May 24, 2016 at 9:32 AM To: Yin Lin <[email protected]>, "[email protected]" <[email protected]> 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 <[email protected]> on behalf of Yin Lin ><[email protected]> >Date: Friday, May 20, 2016 at 1:57 PM >To: "[email protected]" <[email protected]> >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 >[email protected] >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 [email protected] http://openvswitch.org/mailman/listinfo/dev
