Re: [ovs-dev] [patch_v4 3/6] Userspace Datapath: Introduce NAT support.

2017-01-27 Thread Daniele Di Proietto 
2017-01-24 20:40 GMT-08:00 Darrell Ball :
> This patch introduces NAT support for the userspace datapath.
> The conntrack module changes are in this patch.
>
> The per packet scope of lookups for NAT and un_NAT is at
> the bucket level rather than global. One hash table is
> introduced to support create/delete handling. The create/delete
> events may be further optimized, if the need becomes clear.
>
> Some NAT options with limited utility (persistent, random) are
> not supported yet, but will be supported in a later patch.
>
> Signed-off-by: Darrell Ball 

Sparse reports some problems:

../lib/conntrack.c:1375:16: warning: constant 0x is so
big it is unsigned long
../lib/conntrack.c:1398:9: warning: constant 0x is so
big it is unsigned long
../lib/conntrack.c:1400:36: warning: constant 0x is so
big it is unsigned long
../lib/conntrack.c:1403:33: warning: constant 0x is so
big it is unsigned long
../lib/conntrack.c:214:30: error: no member 's6_addr32' in struct in6_addr
../lib/conntrack.c:240:30: error: no member 's6_addr32' in struct in6_addr
../lib/conntrack.c:1360:52: error: no member 's6_addr32' in struct in6_addr
../lib/conntrack.c:1362:52: error: no member 's6_addr32' in struct in6_addr
../lib/conntrack.c:1365:52: error: no member 's6_addr32' in struct in6_addr
../lib/conntrack.c:1367:52: error: no member 's6_addr32' in struct in6_addr
../lib/conntrack.c:1395:44: error: no member 's6_addr32' in struct in6_addr
../lib/conntrack.c:1396:44: error: no member 's6_addr32' in struct in6_addr
../lib/conntrack.c:1409:25: error: no member 's6_addr32' in struct in6_addr
../lib/conntrack.c:1410:25: error: no member 's6_addr32' in struct in6_addr

There are some minor coding style problems, you can find them with
utilities/checkpatch.py

> ---
>  lib/conntrack-private.h |  16 +-
>  lib/conntrack.c | 740 
> ++--
>  lib/conntrack.h |  44 +++
>  3 files changed, 717 insertions(+), 83 deletions(-)
>
> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> index 493865f..b71af37 100644
> --- a/lib/conntrack-private.h
> +++ b/lib/conntrack-private.h
> @@ -51,14 +51,23 @@ struct conn_key {
>  uint16_t zone;
>  };
>
> +struct nat_conn_key_node {
> +struct hmap_node node;
> +struct conn_key key;
> +struct conn_key value;
> +};
> +
>  struct conn {
>  struct conn_key key;
>  struct conn_key rev_key;
>  long long expiration;
>  struct ovs_list exp_node;
>  struct hmap_node node;
> -uint32_t mark;
>  ovs_u128 label;
> +/* XXX: consider flattening. */
> +struct nat_action_info_t *nat_info;
> +uint32_t mark;
> +uint8_t conn_type;
>  };
>
>  enum ct_update_res {
> @@ -67,6 +76,11 @@ enum ct_update_res {
>  CT_UPDATE_NEW,
>  };
>
> +enum ct_conn_type {
> +CT_CONN_TYPE_DEFAULT,
> +   CT_CONN_TYPE_UN_NAT,
> +};
> +
>  struct ct_l4_proto {
>  struct conn *(*new_conn)(struct conntrack_bucket *, struct dp_packet 
> *pkt,
>   long long now);
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 0a611a2..34728a6 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -76,6 +76,20 @@ static void set_label(struct dp_packet *, struct conn *,
>const struct ovs_key_ct_labels *mask);
>  static void *clean_thread_main(void *f_);
>
> +static struct nat_conn_key_node *
> +nat_conn_keys_lookup(struct hmap *nat_conn_keys,
> + const struct conn_key *key,
> + uint32_t basis);
> +
> +static void
> +nat_conn_keys_remove(struct hmap *nat_conn_keys,
> +const struct conn_key *key,
> +uint32_t basis);
> +
> +static bool
> +nat_select_range_tuple(struct conntrack *ct, const struct conn *conn,
> +  struct conn *nat_conn);
> +
>  static struct ct_l4_proto *l4_protos[] = {
>  [IPPROTO_TCP] = _proto_tcp,
>  [IPPROTO_UDP] = _proto_other,
> @@ -90,9 +104,11 @@ long long ct_timeout_val[] = {
>  };
>
>  /* If the total number of connections goes above this value, no new 
> connections
> - * are accepted */
> + * are accepted; this is for CT_CONN_TYPE_DEFAULT connections. */
>  #define DEFAULT_N_CONN_LIMIT 300
>
> +#define DT
> +

I guess this is left here from debugging

>  /* Initializes the connection tracker 'ct'.  The caller is responsible for
>   * calling 'conntrack_destroy()', when the instance is not needed anymore */
>  void
> @@ -101,6 +117,11 @@ conntrack_init(struct conntrack *ct)
>  unsigned i, j;
>  long long now = time_msec();
>
> +ct_rwlock_init(>nat_resources_lock);
> +ct_rwlock_wrlock(>nat_resources_lock);
> +hmap_init(>nat_conn_keys);
> +ct_rwlock_unlock(>nat_resources_lock);
> +
>  for (i = 0; i < CONNTRACK_BUCKETS; i++) {
>  struct conntrack_bucket *ctb = >buckets[i];
>
> @@ -139,13

[ovs-dev] [patch_v4 3/6] Userspace Datapath: Introduce NAT support.

2017-01-24 Thread Darrell Ball 
This patch introduces NAT support for the userspace datapath.
The conntrack module changes are in this patch.

The per packet scope of lookups for NAT and un_NAT is at
the bucket level rather than global. One hash table is
introduced to support create/delete handling. The create/delete
events may be further optimized, if the need becomes clear.

Some NAT options with limited utility (persistent, random) are
not supported yet, but will be supported in a later patch.

Signed-off-by: Darrell Ball 
---
 lib/conntrack-private.h |  16 +-
 lib/conntrack.c | 740 ++--
 lib/conntrack.h |  44 +++
 3 files changed, 717 insertions(+), 83 deletions(-)

diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 493865f..b71af37 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -51,14 +51,23 @@ struct conn_key {
 uint16_t zone;
 };
 
+struct nat_conn_key_node {
+struct hmap_node node;
+struct conn_key key;
+struct conn_key value;
+};
+
 struct conn {
 struct conn_key key;
 struct conn_key rev_key;
 long long expiration;
 struct ovs_list exp_node;
 struct hmap_node node;
-uint32_t mark;
 ovs_u128 label;
+/* XXX: consider flattening. */
+struct nat_action_info_t *nat_info;
+uint32_t mark;
+uint8_t conn_type;
 };
 
 enum ct_update_res {
@@ -67,6 +76,11 @@ enum ct_update_res {
 CT_UPDATE_NEW,
 };
 
+enum ct_conn_type {
+CT_CONN_TYPE_DEFAULT,
+   CT_CONN_TYPE_UN_NAT,
+};
+
 struct ct_l4_proto {
 struct conn *(*new_conn)(struct conntrack_bucket *, struct dp_packet *pkt,
  long long now);
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 0a611a2..34728a6 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -76,6 +76,20 @@ static void set_label(struct dp_packet *, struct conn *,
   const struct ovs_key_ct_labels *mask);
 static void *clean_thread_main(void *f_);
 
+static struct nat_conn_key_node *
+nat_conn_keys_lookup(struct hmap *nat_conn_keys,
+ const struct conn_key *key,
+ uint32_t basis);
+
+static void
+nat_conn_keys_remove(struct hmap *nat_conn_keys,
+const struct conn_key *key,
+uint32_t basis);
+
+static bool
+nat_select_range_tuple(struct conntrack *ct, const struct conn *conn,
+  struct conn *nat_conn);
+
 static struct ct_l4_proto *l4_protos[] = {
 [IPPROTO_TCP] = _proto_tcp,
 [IPPROTO_UDP] = _proto_other,
@@ -90,9 +104,11 @@ long long ct_timeout_val[] = {
 };
 
 /* If the total number of connections goes above this value, no new connections
- * are accepted */
+ * are accepted; this is for CT_CONN_TYPE_DEFAULT connections. */
 #define DEFAULT_N_CONN_LIMIT 300
 
+#define DT
+
 /* Initializes the connection tracker 'ct'.  The caller is responsible for
  * calling 'conntrack_destroy()', when the instance is not needed anymore */
 void
@@ -101,6 +117,11 @@ conntrack_init(struct conntrack *ct)
 unsigned i, j;
 long long now = time_msec();
 
+ct_rwlock_init(>nat_resources_lock);
+ct_rwlock_wrlock(>nat_resources_lock);
+hmap_init(>nat_conn_keys);
+ct_rwlock_unlock(>nat_resources_lock);
+
 for (i = 0; i < CONNTRACK_BUCKETS; i++) {
 struct conntrack_bucket *ctb = >buckets[i];
 
@@ -139,13 +160,24 @@ conntrack_destroy(struct conntrack *ct)
 ovs_mutex_destroy(>cleanup_mutex);
 ct_lock_lock(>lock);
 HMAP_FOR_EACH_POP(conn, node, >connections) {
-atomic_count_dec(>n_conn);
+if (conn->conn_type == CT_CONN_TYPE_DEFAULT) {
+atomic_count_dec(>n_conn);
+}
 delete_conn(conn);
 }
 hmap_destroy(>connections);
 ct_lock_unlock(>lock);
 ct_lock_destroy(>lock);
 }
+ct_rwlock_wrlock(>nat_resources_lock);
+struct nat_conn_key_node *nat_conn_key_node;
+HMAP_FOR_EACH_POP(nat_conn_key_node, node, >nat_conn_keys) {
+free(nat_conn_key_node);
+}
+hmap_destroy(>nat_conn_keys);
+ct_rwlock_unlock(>nat_resources_lock);
+ct_rwlock_destroy(>nat_resources_lock);
+
 }
 
 static unsigned hash_to_bucket(uint32_t hash)
@@ -167,10 +199,175 @@ write_ct_md(struct dp_packet *pkt, uint16_t state, 
uint16_t zone,
 pkt->md.ct_label = label;
 }
 
+static void
+nat_packet(struct dp_packet *pkt, const struct conn *conn, uint16_t *state)
+{
+if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+   *state |= CS_SRC_NAT;
+if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
+struct ip_header *nh = dp_packet_l3(pkt);
+packet_set_ipv4_addr(pkt, >ip_src,
+conn->rev_key.dst.addr.ipv4_aligned);
+} else if (conn->key.dl_type == htons(ETH_TYPE_IPV6)) {
+struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt);
+struct in6_addr ipv6_addr;
+