From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46921) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUTuF-000278-Vq for qemu-devel@nongnu.org; Tue, 02 Aug 2016 03:14:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUTuA-0004rH-7K for qemu-devel@nongnu.org; Tue, 02 Aug 2016 03:14:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51220) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUTu9-0004ps-UP for qemu-devel@nongnu.org; Tue, 02 Aug 2016 03:14:22 -0400 References: <1469497794-16976-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> <1469497794-16976-5-git-send-email-zhangchen.fnst@cn.fujitsu.com> From: Jason Wang Message-ID: <585bcaf1-c476-92b5-29e4-86da39df7ea7@redhat.com> Date: Tue, 2 Aug 2016 15:14:13 +0800 MIME-Version: 1.0 In-Reply-To: <1469497794-16976-5-git-send-email-zhangchen.fnst@cn.fujitsu.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH V10 4/7] colo-compare: track connection and enqueue packet List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhang Chen , qemu devel Cc: Li Zhijian , Wen Congyang , zhanghailiang , "eddie . dong" , "Dr . David Alan Gilbert" On 2016=E5=B9=B407=E6=9C=8826=E6=97=A5 09:49, Zhang Chen wrote: > In this patch we use kernel jhash table to track > connection, and then enqueue net packet like this: > > + CompareState ++ > | | > +---------------+ +---------------+ +---------------+ > |conn list +--->conn +--------->conn | > +---------------+ +---------------+ +---------------+ > | | | | | | > +---------------+ +---v----+ +---v----+ +---v----+ +---v----+ > |primary | |secondary |primary | |secondary > |packet | |packet + |packet | |packet + > +--------+ +--------+ +--------+ +--------+ > | | | | > +---v----+ +---v----+ +---v----+ +---v----+ > |primary | |secondary |primary | |secondary > |packet | |packet + |packet | |packet + > +--------+ +--------+ +--------+ +--------+ > | | | | > +---v----+ +---v----+ +---v----+ +---v----+ > |primary | |secondary |primary | |secondary > |packet | |packet + |packet | |packet + > +--------+ +--------+ +--------+ +--------+ > > We use conn_list to record connection info. > When we want to enqueue a packet, firstly get the > connection from connection_track_table. then push > the packet to g_queue(pri/sec) in it's own conn. > > Signed-off-by: Zhang Chen > Signed-off-by: Li Zhijian > Signed-off-by: Wen Congyang > --- > net/colo-base.c | 108 ++++++++++++++++++++++++++++++++++++++++++++= +++++++++ > net/colo-base.h | 30 +++++++++++++++ > net/colo-compare.c | 70 +++++++++++++++++++++++++++++----- > 3 files changed, 198 insertions(+), 10 deletions(-) > > diff --git a/net/colo-base.c b/net/colo-base.c > index f5d5de9..7e91dec 100644 > --- a/net/colo-base.c > +++ b/net/colo-base.c > @@ -16,6 +16,29 @@ > #include "qemu/error-report.h" > #include "net/colo-base.h" > =20 > +uint32_t connection_key_hash(const void *opaque) > +{ > + const ConnectionKey *key =3D opaque; > + uint32_t a, b, c; > + > + /* Jenkins hash */ > + a =3D b =3D c =3D JHASH_INITVAL + sizeof(*key); > + a +=3D key->src.s_addr; > + b +=3D key->dst.s_addr; > + c +=3D (key->src_port | key->dst_port << 16); > + __jhash_mix(a, b, c); > + > + a +=3D key->ip_proto; > + __jhash_final(a, b, c); > + > + return c; > +} > + > +int connection_key_equal(const void *key1, const void *key2) > +{ > + return memcmp(key1, key2, sizeof(ConnectionKey)) =3D=3D 0; > +} > + > int parse_packet_early(Packet *pkt) > { > int network_length; > @@ -47,6 +70,62 @@ int parse_packet_early(Packet *pkt) > return 0; > } > =20 > +void fill_connection_key(Packet *pkt, ConnectionKey *key) > +{ > + uint32_t tmp_ports; > + > + key->ip_proto =3D pkt->ip->ip_p; > + > + switch (key->ip_proto) { > + case IPPROTO_TCP: > + case IPPROTO_UDP: > + case IPPROTO_DCCP: > + case IPPROTO_ESP: > + case IPPROTO_SCTP: > + case IPPROTO_UDPLITE: > + tmp_ports =3D *(uint32_t *)(pkt->transport_layer); > + key->src =3D pkt->ip->ip_src; > + key->dst =3D pkt->ip->ip_dst; > + key->src_port =3D ntohs(tmp_ports & 0xffff); > + key->dst_port =3D ntohs(tmp_ports >> 16); > + break; > + case IPPROTO_AH: > + tmp_ports =3D *(uint32_t *)(pkt->transport_layer + 4); > + key->src =3D pkt->ip->ip_src; > + key->dst =3D pkt->ip->ip_dst; > + key->src_port =3D ntohs(tmp_ports & 0xffff); > + key->dst_port =3D ntohs(tmp_ports >> 16); > + break; > + default: > + key->src_port =3D 0; > + key->dst_port =3D 0; > + break; > + } > +} > + > +Connection *connection_new(ConnectionKey *key) > +{ > + Connection *conn =3D g_slice_new(Connection); > + > + conn->ip_proto =3D key->ip_proto; > + conn->processing =3D false; > + g_queue_init(&conn->primary_list); > + g_queue_init(&conn->secondary_list); > + > + return conn; > +} > + > +void connection_destroy(void *opaque) > +{ > + Connection *conn =3D 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); > + g_queue_free(&conn->secondary_list); > + g_slice_free(Connection, conn); > +} > + > Packet *packet_new(const void *data, int size) > { > Packet *pkt =3D g_slice_new(Packet); > @@ -72,3 +151,32 @@ void connection_hashtable_reset(GHashTable *connect= ion_track_table) > { > g_hash_table_remove_all(connection_track_table); > } > + > +/* if not found, create a new connection and add to hash table */ > +Connection *connection_get(GHashTable *connection_track_table, > + ConnectionKey *key, > + uint32_t *hashtable_size) > +{ > + Connection *conn =3D g_hash_table_lookup(connection_track_table, k= ey); > + > + if (conn =3D=3D NULL) { > + ConnectionKey *new_key =3D g_memdup(key, sizeof(*key)); > + > + conn =3D connection_new(key); > + > + (*hashtable_size) +=3D 1; > + if (*hashtable_size > HASHTABLE_MAX_SIZE) { > + error_report("colo proxy connection hashtable full, clear = it"); > + connection_hashtable_reset(connection_track_table); > + /* > + * when hashtable_size =3D=3D 0, clear the conn_list > + * in place where be called. > + */ Does this mean it requires the caller to do this? If yes, seems not=20 good, why not simply do things here? > + *hashtable_size =3D 0; > + } > + > + g_hash_table_insert(connection_track_table, new_key, conn); Then we lose the track of *hashtable_size here. It should be 1 but we=20 set it to zero if we are out of space. > + } > + > + return conn; > +} > diff --git a/net/colo-base.h b/net/colo-base.h > index 48835e7..0505608 100644 > --- a/net/colo-base.h > +++ b/net/colo-base.h > @@ -30,7 +30,37 @@ typedef struct Packet { > int size; > } Packet; > =20 > +typedef struct ConnectionKey { > + /* (src, dst) must be grouped, in the same way than in IP header *= / > + struct in_addr src; > + struct in_addr dst; > + uint16_t src_port; > + uint16_t dst_port; > + uint8_t ip_proto; > +} QEMU_PACKED ConnectionKey; > + > +typedef struct Connection { > + /* connection primary send queue: element type: Packet */ > + GQueue primary_list; > + /* connection secondary send queue: element type: Packet */ > + GQueue secondary_list; > + /* flag to enqueue unprocessed_connections */ > + bool processing; > + uint8_t ip_proto; > + /* be used by filter-rewriter */ > + tcp_seq primary_seq; > + tcp_seq secondary_seq; > +} Connection; > + > +uint32_t connection_key_hash(const void *opaque); > +int connection_key_equal(const void *opaque1, const void *opaque2); > int parse_packet_early(Packet *pkt); > +void fill_connection_key(Packet *pkt, ConnectionKey *key); > +Connection *connection_new(ConnectionKey *key); > +void connection_destroy(void *opaque); > +Connection *connection_get(GHashTable *connection_track_table, > + ConnectionKey *key, > + uint32_t *hashtable_size); > void connection_hashtable_reset(GHashTable *connection_track_table); > Packet *packet_new(const void *data, int size); > void packet_destroy(void *opaque, void *user_data); > diff --git a/net/colo-compare.c b/net/colo-compare.c > index 7c52cc8..5f87710 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -72,6 +72,11 @@ typedef struct CompareState { > SocketReadState pri_rs; > SocketReadState sec_rs; > =20 > + /* connection list: the connections belonged to this NIC could be = found > + * in this list. > + * element type: Connection > + */ > + GQueue conn_list; > /* hashtable to save connection */ > GHashTable *connection_track_table; > /* to save unprocessed_connections */ > @@ -93,13 +98,30 @@ static int compare_chr_send(CharDriverState *out, > const uint8_t *buf, > uint32_t size); > =20 > +static void colo_rm_connection(void *opaque, void *user_data) > +{ > + Connection *conn =3D opaque; > + Packet *pkt =3D NULL; > + > + while (!g_queue_is_empty(&conn->primary_list)) { > + pkt =3D g_queue_pop_head(&conn->primary_list); > + packet_destroy(pkt, NULL); > + } > + while (!g_queue_is_empty(&conn->secondary_list)) { > + pkt =3D g_queue_pop_head(&conn->secondary_list); > + packet_destroy(pkt, NULL); > + } This looks rather similar to connection_destroy(), can we share some code= s? > +} > + > /* > * Return 0 on success, if return -1 means the pkt > * is unsupported(arp and ipv6) and will be sent later > */ > static int packet_enqueue(CompareState *s, int mode) > { > + ConnectionKey key =3D {{ 0 } }; > Packet *pkt =3D NULL; > + Connection *conn; > =20 > if (mode =3D=3D PRIMARY_IN) { > pkt =3D packet_new(s->pri_rs.buf, s->pri_rs.packet_len); > @@ -112,17 +134,38 @@ static int packet_enqueue(CompareState *s, int mo= de) > pkt =3D NULL; > return -1; > } > - /* TODO: get connection key from pkt */ > + fill_connection_key(pkt, &key); > =20 > - /* > - * TODO: use connection key get conn from > - * connection_track_table > - */ > + conn =3D connection_get(s->connection_track_table, > + &key, > + &s->hashtable_size); > =20 > - /* > - * TODO: insert pkt to it's conn->primary_list > - * or conn->secondary_list > - */ > + if (!s->hashtable_size) { > + g_queue_foreach(&s->conn_list, colo_rm_connection, NULL); > + } > + > + if (!conn->processing) { > + g_queue_push_tail(&s->conn_list, conn); > + conn->processing =3D true; Why not simply do this in connection_get(), you can save a=20 conn->processing flag. > + } > + > + if (mode =3D=3D PRIMARY_IN) { > + if (g_queue_get_length(&conn->primary_list) < > + MAX_QUEUE_SIZE) { > + g_queue_push_tail(&conn->primary_list, pkt); > + } else { > + error_report("colo compare primary queue size too big," > + "drop packet"); I don't see how packet were dropped in this case? > + } > + } else { > + if (g_queue_get_length(&conn->secondary_list) < > + MAX_QUEUE_SIZE) { > + g_queue_push_tail(&conn->secondary_list, pkt); > + } else { > + error_report("colo compare secondary queue size too big," > + "drop packet"); > + } > + } > =20 > return 0; > } > @@ -267,9 +310,14 @@ static void colo_compare_complete(UserCreatable *u= c, Error **errp) > net_socket_rs_init(&s->pri_rs, compare_pri_rs_finalize); > net_socket_rs_init(&s->sec_rs, compare_sec_rs_finalize); > =20 > + g_queue_init(&s->conn_list); > + > s->hashtable_size =3D 0; > =20 > - /* use g_hash_table_new_full() to new a hashtable */ > + s->connection_track_table =3D g_hash_table_new_full(connection_key= _hash, > + connection_key_e= qual, > + g_free, > + connection_destr= oy); > =20 > return; > } > @@ -310,6 +358,8 @@ static void colo_compare_finalize(Object *obj) > qemu_chr_fe_release(s->chr_out); > } > =20 > + g_queue_free(&s->conn_list); > + > g_free(s->pri_indev); > g_free(s->sec_indev); > g_free(s->outdev);