On 7/14/23 19:11, Ilya Maximets wrote:
On 7/13/23 11:26, Viacheslav Galaktionov via dev wrote:
Currently, if the user wants to track related connections, they have to
specify a helper in all CT actions, which contradicts the behaviour
described in the documentation.
Fix this by using the helper committed along with the connection whenever
a given CT action does not specify a helper of its own.
Signed-off-by: Viacheslav Galaktionov <[email protected]>
Acked-by: Ivan Malov <[email protected]>
---
lib/conntrack.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 4375c03e2..d505ffed1 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1323,6 +1323,10 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
}
}
+ if (conn && helper == NULL) {
+ helper = conn->alg;
+ }
+
Hi, Viacheslav. Thanks for the fix!
We have tests in the system-traffic-userspace testsuite that should cover
use of alg for FTP, for example. Do you know why they are not triggering
the issue? Could you add a test that demonstrates it?
Best regards, Ilya Maximets.
Hi Ilya!
Sorry, it's been a while since I actually implemented this fix, so one
important detail is missing from the patch's description: this fix is
needed when the L7 server is running on a non-standard port. For example,
my testing involved a mock FTP server running on a random port, so this
problem popped up quickly.
As I understand, there is a certain degree of guessing which L7 protocol
is used by a particular connection, see the get_alg_ctl_type function.
This function appears to look at a packet's src and dst ports and compare
them to standard FTP/TFTP ports. If it worked perfectly, I'd expect it
to be possible to completely omit the alg arguments from the flows used
in tests. However, doing so breaks the tests. I haven't had the time to
figure out why.
It's clear that the helper that was committed along with the connection
is largely (or completely) ignored in the code. This means that only
the helper specified in the ct action can influence the result of this
"guess", i.e. that if an action doesn't specify a helper, then it's all
up to the packet's ports. This is what my patch aims to fix by letting
the connection's helper play its part in this decision process.
Adding a test for this isn't exactly trivial since the test suite can
use only standard ports right now. I managed to reproduce the issue on
my local machine but this required me to comment out some waits and
adjust some hardcoded constants, which probably isn't suitable for
inclusion into the main repo.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
