> ALG infra is added with support for FTP and TFTP. > Both V4 and V6 are supported. Also, NAT is supported. > > Three passive ftp system tests are added to complete testing > coverage of ftp for the userspace datapath, as the existing > coverage of passive ftp was limited to one part of one test > for V4 only. > Another system test is added covering tftp with NAT which > was not previously exercised. > > v5->v6: Re-instated include inadvertently removed. > Improve 2 of the new system tests in terms of > potential races. > > v4->v5: Address Ben's code review comments. > First 3 patches were committed. > > v3->v4: Fix tftp with NAT. > Add a system test covering tftp with NAT. > > v2->v3: Fix v4 passive ftp with NAT. > Fix V6 passive ftp; parse check was broken. > Add 3 tests covering v4/v6 passive ftp to > complete ALG coverage in the system tests. > > Code review caught a memory leak of the alg > string such as "ftp" that could occurs during > nat tuple exhaustion. This is a pathological > user error case whose fix was tested by > instrumentated simulation. > Code review also pointed out that a connection > context copy was unclear; this was moved to the > caller where all allocation and error cleanup is > done. > Added several lock annotations that were missing > from the original conntrack code and nat code. > Other review comments were fixed. > > v1->v2: > Mostly the addition of V6 FTP and TFTP support. > > Removed define for unused FTP server port 20. > > Add overflow checks for port numbers. > > Instead of bypassing FTP bounce exploit with > auto-correct, explicitly flag packet as invalid. > > Seq number overflow and underflow checks added. > > Darrell Ball (5): > Userspace Datapath: Add ALG infra and FTP. > Userspace Datapath: Add TFTP support. > System tests: Enable ALGs for userspace. > System tests: Add 4 new ftp and tftp tests. > NEWS: Announce userspace datapath ALG support.
Hi Darrell, This is not a full review. I just wanted to ask you to rename the patches and stop using 'Userspace Datapath' or 'System tests' as a module name for changes localized to only one particular module. I already raised this issue for 'dpdk' prefix previously here: https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335139.html So, about current patch-set: The first two patches are localized to lib/conntrack* files and should have 'conntrack' prefix as a module name. 3rd and 4th patches should have 'system-userspace-macros' and 'system-traffic' module names respectively. Such names will be more accurate and conformed to existing commits and contribution guide. Best regards, Ilya Maximets. > > NEWS | 1 + > include/sparse/netinet/in.h | 1 + > lib/conntrack-private.h | 35 +- > lib/conntrack.c | 1088 > +++++++++++++++++++++++++++++++++++--- > lib/conntrack.h | 10 +- > tests/system-traffic.at | 242 +++++++++ > tests/system-userspace-macros.at | 7 +- > 7 files changed, 1306 insertions(+), 78 deletions(-) _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
