Sounds good :). > -----Original Message----- > From: Sairam Venugopal [mailto:vsai...@vmware.com] > Sent: Tuesday, December 6, 2016 8:53 PM > To: Alin Serdean <aserd...@cloudbasesolutions.com>; > d...@openvswitch.org > Subject: Re: [ovs-dev] [PATCH 3/4] datapath-windows: Conntrack - Introduce > support for tracking related connections > > Alin, > > Thanks for the review. > > Yes, my main intent was to introduce something simple that we can iterate > on. I did look at the safe-string functions. Unfortunately, I couldn¹t find > Rtl* > functions that I could reuse here. That¹s the reason why I had to introduce > the two str functions. > > I will apply the other review changes. > > Thanks, > Sairam > > On 12/5/16, 10:29 AM, "Alin Serdean" <aserd...@cloudbasesolutions.com> > wrote: > > >Thanks for the patch! > > > >It looks good as an initial point that we can add more complexity in > >the future :). > > > >One personal preference would be to use the string Rtl* functions: > >https://urldefense.proofpoint.com/v2/url?u=https- > 3A__msdn.microsoft.com > >_en > >-2Dus_library_windows_hardware_ff563642-28v-3Dvs.85- > 29.aspx&d=DgIFAg&c= > >uil > >aK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5 > Q5Z7uo&m=2sP > >vKu > >qF3qQqckq- > dZ1aozPYKHpCXTBU8HLAK9IdNME&s=yJ2hG5syDtE0Rjr1S7wMgduJky837aX > >bsT > >X8VK1LSYI&e= > >https://urldefense.proofpoint.com/v2/url?u=https- > 3A__msdn.microsoft.com > >_en > >-2Dus_library_windows_hardware_ff563644-28v-3Dvs.85- > 29.aspx&d=DgIFAg&c= > >uil > >aK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5 > Q5Z7uo&m=2sP > >vKu > >qF3qQqckq- > dZ1aozPYKHpCXTBU8HLAK9IdNME&s=2IAdLCggWOSSUAVbyLMnclw5A_lYG > VR > >dwY > >9wTadrzrs&e= > > > >Some white space issues: > >../ovs-dev-3-4-datapath-windows-Conntrack---Introduce-support-for-track > >ing > >-related-connections.patch:591: new blank line at EOF. > >+ > >warning: 1 line adds whitespace errors. > > > >Other small comments inlined. > > > >> -----Original Message----- > >> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > >> boun...@openvswitch.org] On Behalf Of Sairam Venugopal > >> Sent: Thursday, December 1, 2016 11:19 PM > >> To: d...@openvswitch.org > >> Subject: [ovs-dev] [PATCH 3/4] datapath-windows: Conntrack - > >> Introduce support for tracking related connections > >> > >> Introduce a new table to track related connections. This table will > >>be used to track FTP data connections based on the control > >>connection. There is a new Conntrack-ftp.c to parse incoming FTP > >>messages to determine the related data ports. It creates a new entry > >>in the related connections tracker table. If there is a matching FTP > >>data connection, then the state for that connection is marked as > >>RELATED. > >> > >> Signed-off-by: Sairam Venugopal <vsai...@vmware.com> > >> --- > >-------------------------------->cut<-------------------------- > >> + /* End of FTP response is either ) or \r\n */ > >[Alin Serdean] Check for \n as well. > >> + if (*buf == ')' || *buf == '\r') { > >-------------------------------->cut<-------------------------- > >[Alin Serdean] Maybe add define for 227 (Passive mode) and the comment > >above in the define. > >> + if ((len >= 4) && (OvsStrncmp("227", ftpMsg, 3) == 0)) { > >> + ftpType = FTP_TYPE_PASV; > >> + /* There are various formats for PASV command. We try to > >>support > >> + * some of them. This has been addressed by RFC 2428 - > >>EPSV. > >> + * Eg: > >> + * 227 Entering Passive Mode (h1,h2,h3,h4,p1,p2).[Alin > >>Serdean] > >> + * 227 Entering Passive Mode (h1,h2,h3,h4,p1,p2 > >> + * 227 Entering Passive Mode. h1,h2,h3,h4,p1,p2 > >> + * 227 =h1,h2,h3,h4,p1,p2 > >> + */ > >-------------------------------->cut<-------------------------- > >> +VOID > >> +OvsCleanupCtRelated (VOID) > >[Alin Serdean] Extra " " character. > >
_______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev