On 18 May 2026, at 19:13, Aaron Conole wrote:
> The FTP and TFTP helpers were scattered all over the conntrack TU making
> reading the individual FTP parts a bit difficult. Now that the handling
> is more modular, split them out into their own files.
>
> Signed-off-by: Aaron Conole <[email protected]>
Hi Aaron,
One small comment below, the rest looks fine to me.
Maybe we could add 'init' and 'epsv' to the checkpatch_dict.txt file?
//Eelco
> ---
> lib/automake.mk | 2 +
> lib/conntrack-ftp.c | 689 ++++++++++++++++++++++++++++++++++++++
> lib/conntrack-private.h | 42 +++
> lib/conntrack-tftp.c | 47 +++
> lib/conntrack.c | 718 +---------------------------------------
> 5 files changed, 786 insertions(+), 712 deletions(-)
> create mode 100644 lib/conntrack-ftp.c
> create mode 100644 lib/conntrack-tftp.c
[...]
> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> index a5bf1bb519..8eab1d3703 100644
> --- a/lib/conntrack-private.h
> +++ b/lib/conntrack-private.h
> @@ -177,6 +177,9 @@ enum ct_ephemeral_range {
> MAX_NAT_EPHEMERAL_PORT = 65535
> };
>
> +/* The maximum TCP or UDP port number. */
> +#define CT_MAX_L4_PORT 65535
> +
> #define IN_RANGE(curr, min, max) \
> (curr >= min && curr <= max)
>
> @@ -261,6 +264,9 @@ enum ct_alg_ctl_type {
> /* SIP is not enabled through OpenFlow and is present only as an example
> * of an ALG that allows a wildcard source IP address. */
> CT_ALG_CTL_SIP,
> +
> + /* MAX ALG */
> + CT_ALG_CTL_MAX,
> };
>
> extern struct ct_l4_proto ct_proto_tcp;
> @@ -289,6 +295,28 @@ struct conn_lookup_ctx {
> bool icmp_related;
> };
>
> +/* FTP control-packet classification used by ALG helpers.
> + * CT_FTP_CTL_INTEREST carries an address/port specifier (PORT, PASV, EPRT,
> + * EPSV); CT_FTP_CTL_OTHER does not; CT_FTP_CTL_INVALID is malformed. */
> +enum ftp_ctl_pkt {
> + CT_FTP_CTL_INTEREST,
> + CT_FTP_CTL_OTHER,
> + CT_FTP_CTL_INVALID,
> +};
> +
> +/* ALG helper callback signature. Each registered helper receives the
> + * classified control-packet type so it can decide whether to act. */
> +typedef void (*alg_helper)(struct conntrack *ct,
> + const struct conn_lookup_ctx *ctx,
> + struct dp_packet *pkt,
> + struct conn *conn_for_expectation,
> + long long now, enum ftp_ctl_pkt ftp_ctl,
> + bool nat);
> +
> +/* Array indexed by ct_alg_ctl_type; populated by per-module init functions
> + * (conntrack_ftp_init, conntrack_tftp_init, ...) before first use. */
> +extern alg_helper alg_helpers[];
Should this be alg_helper alg_helpers[CT_ALG_CTL_MAX] so the
compiler knows the bounds?
[...]
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
