Eelco Chaudron <[email protected]> writes:
> 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?
ack
> //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?
I think that makes sense.
> [...]
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
