On Tue, Jan 8, 2019 at 1:33 AM David Marchand <[email protected]> wrote:
> Hello and happy new year to all :-) > > On Tue, Jan 8, 2019 at 5:24 AM Darrell Ball <[email protected]> wrote: > >> Thanks for the fix David. >> >> On Thu, Dec 20, 2018 at 5:33 AM David Marchand <[email protected]> >> wrote: >> >>> When configuring the nat part of an expectation, care must be taken to >>> look at the master nat action and direction to properly reproduce it. >>> >>> The FTP passive mode test is switched to DNAT since the alg only mangles >>> the packet in this case. >>> Other active mode tests titles have been updated to reflect they are >>> dealing with SNAT. >>> >>> Signed-off-by: David Marchand <[email protected]> >>> --- >>> lib/conntrack.c | 12 ++++++++++-- >>> tests/system-traffic.at | 48 >>> ++++++++++++++++++++++++------------------------ >>> 2 files changed, 34 insertions(+), 26 deletions(-) >>> >> >> > [snip] > > >> >> 1/ For the tests, I think we should have both SNAT and DNAT for passive >> mode, since some special cases exist and we also >> want to test for ALGs generally. Hence I added DNAT passive mode as a >> separate test, rather than converting the >> existing SNAT test to DNAT and I changed the name of the existing >> test to "SNAT" from just "NAT". >> >> 2/ I also think we need a DNAT active mode test, so I added one as well. >> > > Adding those tests make sense when searching for regressions and I suppose > this won't take a lot of time when compared to the overall test duration. > > >> 3/ There were a few missing "SNAT" and "DNAT" vs "NAT" changes for the >> FTP/TFTP tests aside from the ones you added, so I added those as well. >> > > Well, I don't really mind updating all the tests titles at once but I was > trying to keep the changes focused on the parts that were impacted by the > fix. > So since you prefer, ok for aligning all tests titles. > Thanks Possibly having the test titles specify "SNAT" and "DNAT" would have made it obvious that FTP DNAT tests were completely missing and prompted adding some, which would have had the obvious effect. > > >> So, we end up with the following >> >> Let me know if this makes sense. >> > > Ok, let me incorporate and test those changes as part of my v3. > Should I add a Co-Authored tag on it for you ? > I prefer to avoid submitting my changes as a separate patch, but either way is fine with me. > > -- > David Marchand > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
