conntrack: Refactor algs.

Upcoming requirements for new algs make it desirable to split out
alg helpers more cleanly.

Signed-off-by: Darrell Ball <dlu998@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>

lib/conntrack.c

@@ -67,6 +67,12 @@ enum ct_alg_mode {
     CT_TFTP_MODE,
 };
 
+enum ct_alg_ctl_type {
+    CT_ALG_CTL_NONE,
+    CT_ALG_CTL_FTP,
+    CT_ALG_CTL_TFTP,
+};
+
 static bool conn_key_extract(struct conntrack *, struct dp_packet *,
                              ovs_be16 dl_type, struct conn_lookup_ctx *,
                              uint16_t zone);
@@ -142,6 +148,13 @@ static enum ftp_ctl_pkt
 detect_ftp_ctl_type(const struct conn_lookup_ctx *ctx,
                     struct dp_packet *pkt);
 
+static struct ct_l4_proto *l4_protos[] = {
+    [IPPROTO_TCP] = &ct_proto_tcp,
+    [IPPROTO_UDP] = &ct_proto_other,
+    [IPPROTO_ICMP] = &ct_proto_icmp4,
+    [IPPROTO_ICMPV6] = &ct_proto_icmp6,
+};
+
 static void
 handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
                struct dp_packet *pkt,
@@ -150,14 +163,23 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
 
 static void
 handle_tftp_ctl(struct conntrack *ct,
+                const struct conn_lookup_ctx *ctx OVS_UNUSED,
+                struct dp_packet *pkt OVS_UNUSED,
                 const struct conn *conn_for_expectation,
-                long long now);
+                long long now, enum ftp_ctl_pkt ftp_ctl OVS_UNUSED,
+                bool nat OVS_UNUSED);
 
-static struct ct_l4_proto *l4_protos[] = {
+typedef void (*alg_helper)(struct conntrack *ct,
-    [IPPROTO_TCP] = &ct_proto_tcp,
+                           const struct conn_lookup_ctx *ctx,
-    [IPPROTO_UDP] = &ct_proto_other,
+                           struct dp_packet *pkt,
-    [IPPROTO_ICMP] = &ct_proto_icmp4,
+                           const struct conn *conn_for_expectation,
-    [IPPROTO_ICMPV6] = &ct_proto_icmp6,
+                           long long now, enum ftp_ctl_pkt ftp_ctl,
+                           bool nat);
+
+static alg_helper alg_helpers[] = {
+    [CT_ALG_CTL_NONE] = NULL,
+    [CT_ALG_CTL_FTP] = handle_ftp_ctl,
+    [CT_ALG_CTL_TFTP] = handle_tftp_ctl,
 };
 
 long long ct_timeout_val[] = {
@@ -434,35 +456,45 @@ get_ip_proto(const struct dp_packet *pkt)
 }
 
 static bool
-is_ftp_ctl(const struct dp_packet *pkt)
+is_ftp_ctl(const enum ct_alg_ctl_type ct_alg_ctl)
 {
-    uint8_t ip_proto = get_ip_proto(pkt);
+    return ct_alg_ctl == CT_ALG_CTL_FTP;
-    struct tcp_header *th = dp_packet_l4(pkt);
-
-    /* CT_IPPORT_FTP is used because IPPORT_FTP in not defined in OSX,
-     * at least in in.h. Since this value will never change, just remove
-     * the external dependency. */
-#define CT_IPPORT_FTP 21
-
-    return (ip_proto == IPPROTO_TCP &&
-            (th->tcp_src == htons(CT_IPPORT_FTP) ||
-             th->tcp_dst == htons(CT_IPPORT_FTP)));
-
 }
 
-static bool
+static enum ct_alg_ctl_type
-is_tftp_ctl(const struct dp_packet *pkt)
+get_alg_ctl_type(const struct dp_packet *pkt)
 {
     uint8_t ip_proto = get_ip_proto(pkt);
     struct udp_header *uh = dp_packet_l4(pkt);
+    struct tcp_header *th = dp_packet_l4(pkt);
 
-    /* CT_IPPORT_TFTP is used because IPPORT_TFTP in not defined in OSX,
+    /* CT_IPPORT_FTP/TFTP is used because IPPORT_FTP/TFTP in not defined
-     * at least in in.h. Since this value will never change, remove
+     * in OSX, at least in in.h. Since these values will never change, remove
      * the external dependency. */
-#define CT_IPPORT_TFTP 69
+    enum { CT_IPPORT_FTP = 21 };
-    return (ip_proto == IPPROTO_UDP &&
+    enum { CT_IPPORT_TFTP = 69 };
-            uh->udp_dst == htons(CT_IPPORT_TFTP));
 
+    if (ip_proto == IPPROTO_UDP && uh->udp_dst == htons(CT_IPPORT_TFTP)) {
+        return CT_ALG_CTL_TFTP;
+    } else if (ip_proto == IPPROTO_TCP &&
+               (th->tcp_src == htons(CT_IPPORT_FTP) ||
+                th->tcp_dst == htons(CT_IPPORT_FTP))) {
+        return CT_ALG_CTL_FTP;
+    }
+    return CT_ALG_CTL_NONE;
+}
+
+static void
+handle_alg_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
+               struct dp_packet *pkt, enum ct_alg_ctl_type ct_alg_ctl,
+               const struct conn *conn, long long now, bool nat,
+               const struct conn *conn_for_expectation)
+{
+    /* ALG control packet handling with expectation creation. */
+    if (OVS_UNLIKELY(alg_helpers[ct_alg_ctl] && conn)) {
+        alg_helpers[ct_alg_ctl](ct, ctx, pkt, conn_for_expectation, now,
+                                CT_FTP_CTL_INTEREST, nat);
+    }
 }
 
 static void
@@ -1069,6 +1101,33 @@ is_un_nat_conn_valid(const struct conn *un_nat_conn)
     return un_nat_conn->conn_type == CT_CONN_TYPE_UN_NAT;
 }
 
+static bool
+conn_update_state_alg(struct conntrack *ct, struct dp_packet *pkt,
+                      struct conn_lookup_ctx *ctx, struct conn *conn,
+                      const struct nat_action_info_t *nat_action_info,
+                      enum ct_alg_ctl_type ct_alg_ctl, long long now,
+                      unsigned bucket, bool *create_new_conn)
+    OVS_REQUIRES(ct->buckets[bucket].lock)
+{
+    if (is_ftp_ctl(ct_alg_ctl)) {
+        /* Keep sequence tracking in sync with the source of the
+         * sequence skew. */
+        if (ctx->reply != conn->seq_skew_dir) {
+            handle_ftp_ctl(ct, ctx, pkt, conn, now, CT_FTP_CTL_OTHER,
+                           !!nat_action_info);
+            *create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now,
+                                                bucket);
+        } else {
+            *create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now,
+                                                bucket);
+            handle_ftp_ctl(ct, ctx, pkt, conn, now, CT_FTP_CTL_OTHER,
+                           !!nat_action_info);
+        }
+        return true;
+    }
+    return false;
+}
+
 static void
 process_one(struct conntrack *ct, struct dp_packet *pkt,
             struct conn_lookup_ctx *ctx, uint16_t zone,
@@ -1121,24 +1180,14 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
     bool create_new_conn = false;
     struct conn conn_for_un_nat_copy;
     conn_for_un_nat_copy.conn_type = CT_CONN_TYPE_DEFAULT;
-    bool ftp_ctl = is_ftp_ctl(pkt);
+
+    enum ct_alg_ctl_type ct_alg_ctl = get_alg_ctl_type(pkt);
 
     if (OVS_LIKELY(conn)) {
-        if (ftp_ctl) {
+        if (OVS_LIKELY(!conn_update_state_alg(ct, pkt, ctx, conn,
-            /* Keep sequence tracking in sync with the source of the
+                                              nat_action_info,
-             * sequence skew. */
+                                              ct_alg_ctl, now, bucket,
-            if (ctx->reply != conn->seq_skew_dir) {
+                                              &create_new_conn))) {
-                handle_ftp_ctl(ct, ctx, pkt, conn, now, CT_FTP_CTL_OTHER,
-                               !!nat_action_info);
-                create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now,
-                                                    bucket);
-            } else {
-                create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now,
-                                                    bucket);
-                handle_ftp_ctl(ct, ctx, pkt, conn, now, CT_FTP_CTL_OTHER,
-                               !!nat_action_info);
-            }
-        } else {
             create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now,
                                                 bucket);
         }
@@ -1187,9 +1236,8 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
         set_label(pkt, conn, &setlabel[0], &setlabel[1]);
     }
 
-    bool tftp_ctl = is_tftp_ctl(pkt);
     struct conn conn_for_expectation;
-    if (OVS_UNLIKELY((ftp_ctl || tftp_ctl) && conn)) {
+    if (OVS_UNLIKELY((ct_alg_ctl != CT_ALG_CTL_NONE) && conn)) {
         conn_for_expectation = *conn;
     }
 
@@ -1199,13 +1247,8 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
         create_un_nat_conn(ct, &conn_for_un_nat_copy, now, !!alg_exp);
     }
 
-    /* FTP control packet handling with expectation creation. */
+    handle_alg_ctl(ct, ctx, pkt, ct_alg_ctl, conn, now, !!nat_action_info,
-    if (OVS_UNLIKELY(ftp_ctl && conn)) {
+                   &conn_for_expectation);
-        handle_ftp_ctl(ct, ctx, pkt, &conn_for_expectation,
-                       now, CT_FTP_CTL_INTEREST, !!nat_action_info);
-    } else if (OVS_UNLIKELY(tftp_ctl && conn)) {
-        handle_tftp_ctl(ct, &conn_for_expectation, now);
-    }
 }
 
 /* Sends the packets in '*pkt_batch' through the connection tracker 'ct'.  All
@@ -1235,8 +1278,8 @@ conntrack_execute(struct conntrack *ct, struct dp_packet_batch *pkt_batch,
             write_ct_md(packet, zone, NULL, NULL, NULL);
             continue;
         }
-        process_one(ct, packet, &ctx, zone, force, commit,
+        process_one(ct, packet, &ctx, zone, force, commit, now, setmark,
-                    now, setmark, setlabel, nat_action_info, helper);
+                    setlabel, nat_action_info, helper);
     }
 
     return 0;
@@ -3099,8 +3142,11 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
 
 static void
 handle_tftp_ctl(struct conntrack *ct,
+                const struct conn_lookup_ctx *ctx OVS_UNUSED,
+                struct dp_packet *pkt OVS_UNUSED,
                 const struct conn *conn_for_expectation,
-                long long now)
+                long long now, enum ftp_ctl_pkt ftp_ctl OVS_UNUSED,
+                bool nat OVS_UNUSED)
 {
     expectation_create(ct, conn_for_expectation->key.src.port, now,
                        CT_TFTP_MODE, conn_for_expectation);