[ovs-dev] [PATCH] datapath-windows: Update ICMP-Type and Code comparison in CT lookup
Anand Kumar
kumaranand at vmware.com
Tue Aug 15 22:21:14 UTC 2017
Hi Alin,
Thanks for the review. Please find my responses inline.
Will address the review comments and send out v2 version of the patch.
Thanks,
Anand Kumar
On 8/14/17, 8:04 PM, "Alin Serdean" <aserdean at cloudbasesolutions.com> wrote:
Hi Sai and Anand,
Thanks a lot for the patch. I have a few questions regarding the approach. Please see inline.
> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
> bounces at openvswitch.org] On Behalf Of Anand Kumar
> Sent: Friday, August 11, 2017 11:42 PM
> To: dev at openvswitch.org
> Subject: [ovs-dev] [PATCH] datapath-windows: Update ICMP-Type and Code
> comparison in CT lookup
>
> - Update the CT comparison function to compare individual fields instead of
> NdisEqualMemory.
[Alin Serdean] I don't like this change, especially mixing both members of union, i.e:
> + (ctxKey.dst.port == entryKey.dst.port) &&
> + (ctxKey.dst.icmp_id == entryKey.dst.icmp_id) &&
Why are you trying to change via member by member comparison?
[Anand Kumar]: Done. Previously, we ran into issues while verifying NAT with ICMP, which now is fixed with my other patch.
> - Add in some padding for the ct_endpoint's union.
[Alin Serdean] Why is this needed?
[Anand Kumar] This is needed because size of the union is 32 bits.
Another question do we still need the 'pad' member inside ct_endpoint (https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs_blob_master_datapath-2Dwindows_ovsext_Conntrack.h-23L51&d=DwIFAg&c=uilaK90D4TOVoH58JNXRgQ&r=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us&m=1Rj8RlVaAovz2iLrWHvT6N469qFN036rzjLkBJweZDw&s=hky8AVI9m8GjZOD8r21WYvEHBPcDcVVZiwMKqk07IwU&e= ) ?
[Anand Kumar] Yes, the padding at the end of structure is no longer needed. I will address in next version.
> - Update the Orig Tuple to use ICMP Type and Code instead of Port for ICMP
[Alin Serdean] Agreed
>
> Co-authored-by: Sairam Venugopal <vsairam at vmware.com>
> Signed-off-by: Anand Kumar <kumaranand at vmware.com>
More information about the dev
mailing list