Re: [Qemu-devel] [RFC PATCH v2 08/10] net/colo-proxy: Handle packet and connection

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Zhang Chen (zhangchen.fnst@cn.fujitsu.com) wrote:
> 
> 
> On 02/20/2016 04:04 AM, Dr. David Alan Gilbert wrote:
> >* Zhang Chen (zhangchen.fnst@cn.fujitsu.com) wrote:
> >>From: zhangchen <zhangchen.fnst@cn.fujitsu.com>
> >>
> >>In here we will handle ip packet and connection
> >>
> >>Signed-off-by: zhangchen <zhangchen.fnst@cn.fujitsu.com>
> >>Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> >>---
> >>  net/colo-proxy.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 130 insertions(+)
> >>
> >>diff --git a/net/colo-proxy.c b/net/colo-proxy.c
> >>index 5e5c72e..06bab80 100644
> >>--- a/net/colo-proxy.c
> >>+++ b/net/colo-proxy.c
> >>@@ -167,11 +167,141 @@ static int connection_key_equal(const void *opaque1, const void *opaque2)
> >>      return memcmp(opaque1, opaque2, sizeof(ConnectionKey)) == 0;
> >>  }
> >>+static void connection_destroy(void *opaque)
> >>+{
> >>+    Connection *conn = opaque;
> >>+
> >>+    g_queue_foreach(&conn->primary_list, packet_destroy, NULL);
> >>+    g_queue_free(&conn->primary_list);
> >>+    g_queue_foreach(&conn->secondary_list, packet_destroy, NULL);
> >Be careful about these lists and which threads access them;
> >I found I could occasionally trigger a seg fault as two
> >threads tried to manipulate them at once; I just put a 'list_lock'
> >in the connection, which seems to fix it, but I might have to be
> >more careful with deadlocks.
> 
> Thanks for your work to colo.
> and where can I  see your code for colo-proxy?

I'll clean it up and post it in the next couple of days.

> maybe I need it to make my code better.

Maybe, but it's a bit hacky at the moment,  I added
on sequence number compensation, like in the old kernel proxy,
but I've only done it for inbound connections, and the code
doesn't yet:
   a) Handle sequence numbers after failover
   b) deal with socket shutdown properly
   c) try to deal with TCP fragmentation.

There's lots of different places we get random data from that
throws the comparison.

Dave

> 
> >
> >>+    g_queue_free(&conn->secondary_list);
> >>+    g_slice_free(Connection, conn);
> >>+}
> >>+
> >>+static Connection *connection_new(ConnectionKey *key)
> >>+{
> >>+    Connection *conn = g_slice_new(Connection);
> >>+
> >>+    conn->ip_proto = key->ip_proto;
> >>+    conn->processing = false;
> >>+    g_queue_init(&conn->primary_list);
> >>+    g_queue_init(&conn->secondary_list);
> >>+
> >>+    return conn;
> >>+}
> >>+
> >>+/*
> >>+ * Clear hashtable, stop this hash growing really huge
> >>+ */
> >>+static void clear_connection_hashtable(COLOProxyState *s)
> >>+{
> >>+    s->hashtable_size = 0;
> >>+    g_hash_table_remove_all(colo_conn_hash);
> >>+    trace_colo_proxy("clear_connection_hashtable");
> >>+}
> >>+
> >>  bool colo_proxy_query_checkpoint(void)
> >>  {
> >>      return colo_do_checkpoint;
> >>  }
> >>+/* Return 0 on success, or return -1 if the pkt is corrupted */
> >>+static int parse_packet_early(Packet *pkt, ConnectionKey *key)
> >>+{
> >>+    int network_length;
> >>+    uint8_t *data = pkt->data;
> >>+    uint16_t l3_proto;
> >>+    uint32_t tmp_ports;
> >>+    ssize_t l2hdr_len = eth_get_l2_hdr_length(data);
> >>+
> >>+    pkt->network_layer = data + ETH_HLEN;
> >>+    l3_proto = eth_get_l3_proto(data, l2hdr_len);
> >>+    if (l3_proto != ETH_P_IP) {
> >>+        if (l3_proto == ETH_P_ARP) {
> >>+            return -1;
> >>+        }
> >>+        return 0;
> >>+    }
> >>+
> >>+    network_length = pkt->ip->ip_hl * 4;
> >>+    pkt->transport_layer = pkt->network_layer + network_length;
> >>+    key->ip_proto = pkt->ip->ip_p;
> >>+    key->src = pkt->ip->ip_src;
> >>+    key->dst = pkt->ip->ip_dst;
> >>+
> >>+    switch (key->ip_proto) {
> >>+    case IPPROTO_TCP:
> >>+    case IPPROTO_UDP:
> >>+    case IPPROTO_DCCP:
> >>+    case IPPROTO_ESP:
> >>+    case IPPROTO_SCTP:
> >>+    case IPPROTO_UDPLITE:
> >>+        tmp_ports = *(uint32_t *)(pkt->transport_layer);
> >>+        key->src_port = tmp_ports & 0xffff;
> >>+        key->dst_port = tmp_ports >> 16;
> >These fields are not byteswapped; it makes it very confusing
> >when printing them for debug;  I added htons around every
> >reading of the ports from the packets.
> >
> >Dave
> 
> I will fix it in colo-compare module.
> 
> Thanks
> zhangchen
> 
> >>+        break;
> >>+    case IPPROTO_AH:
> >>+        tmp_ports = *(uint32_t *)(pkt->transport_layer + 4);
> >>+        key->src_port = tmp_ports & 0xffff;
> >>+        key->dst_port = tmp_ports >> 16;
> >>+        break;
> >>+    default:
> >>+        break;
> >>+    }
> >>+
> >>+    return 0;
> >>+}
> >>+
> >>+static Packet *packet_new(COLOProxyState *s, void *data,
> >>+                          int size, ConnectionKey *key, NetClientState *sender)
> >>+{
> >>+    Packet *pkt = g_slice_new(Packet);
> >>+
> >>+    pkt->data = data;
> >>+    pkt->size = size;
> >>+    pkt->s = s;
> >>+    pkt->sender = sender;
> >>+
> >>+    if (parse_packet_early(pkt, key)) {
> >>+        packet_destroy(pkt, NULL);
> >>+        pkt = NULL;
> >>+    }
> >>+
> >>+    return pkt;
> >>+}
> >>+
> >>+static void packet_destroy(void *opaque, void *user_data)
> >>+{
> >>+    Packet *pkt = opaque;
> >>+    g_free(pkt->data);
> >>+    g_slice_free(Packet, pkt);
> >>+}
> >>+
> >>+/* if not found, creata a new connection and add to hash table */
> >>+static Connection *colo_proxy_get_conn(COLOProxyState *s,
> >>+            ConnectionKey *key)
> >>+{
> >>+    /* FIXME: protect colo_conn_hash */
> >>+    Connection *conn = g_hash_table_lookup(colo_conn_hash, key);
> >>+
> >>+    if (conn == NULL) {
> >>+        ConnectionKey *new_key = g_malloc(sizeof(*key));
> >>+
> >>+        conn = connection_new(key);
> >>+        memcpy(new_key, key, sizeof(*key));
> >>+
> >>+        s->hashtable_size++;
> >>+        if (s->hashtable_size > hashtable_max_size) {
> >>+            trace_colo_proxy("colo proxy connection hashtable full, clear it");
> >>+            clear_connection_hashtable(s);
> >>+        } else {
> >>+            g_hash_table_insert(colo_conn_hash, new_key, conn);
> >>+        }
> >>+    }
> >>+
> >>+     return conn;
> >>+}
> >>+
> >>  static ssize_t colo_proxy_enqueue_primary_packet(NetFilterState *nf,
> >>                                           NetClientState *sender,
> >>                                           unsigned flags,
> >>-- 
> >>1.9.1
> >>
> >>
> >>
> >>
> >--
> >Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
> >
> >.
> >
> 
> -- 
> Thanks
> zhangchen
> 
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK