| Message ID | 1495549241-23380-7-git-send-email-zhangchen.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 2017年05月23日 22:20, Zhang Chen wrote: > We add the vnet_hdr option for colo-compare, default is disable. > If you use virtio-net-pci net driver, please enable it. > You can use it for example: > -object colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,vnet_hdr=on This is not accurate since virtio-net-pci is not the only card that uses vnet_hdr. E1000E is another one. > > COLO-compare can get vnet header length from filter, > Add vnet_hdr_len to struct packet and output packet with > the vnet_hdr_len. > > Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> > --- > net/colo-compare.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++------- > qemu-options.hx | 4 +-- > 2 files changed, 69 insertions(+), 11 deletions(-) > > diff --git a/net/colo-compare.c b/net/colo-compare.c > index bf0b856..f89b380 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -73,6 +73,7 @@ typedef struct CompareState { > CharBackend chr_out; > SocketReadState pri_rs; > SocketReadState sec_rs; > + bool vnet_hdr; > > /* connection list: the connections belonged to this NIC could be found > * in this list. > @@ -97,9 +98,10 @@ enum { > SECONDARY_IN, > }; > > -static int compare_chr_send(CharBackend *out, > +static int compare_chr_send(CompareState *s, > const uint8_t *buf, > - uint32_t size); > + uint32_t size, > + uint32_t vnet_hdr_len); > > static gint seq_sorter(Packet *a, Packet *b, gpointer data) > { > @@ -472,7 +474,10 @@ static void colo_compare_connection(void *opaque, void *user_data) > } > > if (result) { > - ret = compare_chr_send(&s->chr_out, pkt->data, pkt->size); > + ret = compare_chr_send(s, > + pkt->data, > + pkt->size, > + pkt->vnet_hdr_len); Why not check vnet_hdr support here? And don't we need strip vnet_hdr here? (Since the redirector on destination does not do this) > if (ret < 0) { > error_report("colo_send_primary_packet failed"); > } > @@ -493,9 +498,10 @@ static void colo_compare_connection(void *opaque, void *user_data) > } > } > > -static int compare_chr_send(CharBackend *out, > +static int compare_chr_send(CompareState *s, > const uint8_t *buf, > - uint32_t size) > + uint32_t size, > + uint32_t vnet_hdr_len) > { > int ret = 0; > uint32_t len = htonl(size); > @@ -504,12 +510,24 @@ static int compare_chr_send(CharBackend *out, > return 0; > } > > - ret = qemu_chr_fe_write_all(out, (uint8_t *)&len, sizeof(len)); > + ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len)); > if (ret != sizeof(len)) { > goto err; > } > > - ret = qemu_chr_fe_write_all(out, (uint8_t *)buf, size); > + if (s->vnet_hdr) { > + /* > + * We send vnet header len make other module(like filter-redirector) > + * know how to parse net packet correctly. > + */ But redirector does not strip the vnet header, does this really work? Thanks
On 05/25/2017 02:22 PM, Jason Wang wrote: > > > On 2017年05月23日 22:20, Zhang Chen wrote: >> We add the vnet_hdr option for colo-compare, default is disable. >> If you use virtio-net-pci net driver, please enable it. >> You can use it for example: >> -object >> colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,vnet_hdr=on > > This is not accurate since virtio-net-pci is not the only card that > uses vnet_hdr. E1000E is another one. Good catch, I will add the e1000e in this commit log. > >> >> COLO-compare can get vnet header length from filter, >> Add vnet_hdr_len to struct packet and output packet with >> the vnet_hdr_len. >> >> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> >> --- >> net/colo-compare.c | 76 >> +++++++++++++++++++++++++++++++++++++++++++++++------- >> qemu-options.hx | 4 +-- >> 2 files changed, 69 insertions(+), 11 deletions(-) >> >> diff --git a/net/colo-compare.c b/net/colo-compare.c >> index bf0b856..f89b380 100644 >> --- a/net/colo-compare.c >> +++ b/net/colo-compare.c >> @@ -73,6 +73,7 @@ typedef struct CompareState { >> CharBackend chr_out; >> SocketReadState pri_rs; >> SocketReadState sec_rs; >> + bool vnet_hdr; >> /* connection list: the connections belonged to this NIC >> could be found >> * in this list. >> @@ -97,9 +98,10 @@ enum { >> SECONDARY_IN, >> }; >> -static int compare_chr_send(CharBackend *out, >> +static int compare_chr_send(CompareState *s, >> const uint8_t *buf, >> - uint32_t size); >> + uint32_t size, >> + uint32_t vnet_hdr_len); >> static gint seq_sorter(Packet *a, Packet *b, gpointer data) >> { >> @@ -472,7 +474,10 @@ static void colo_compare_connection(void >> *opaque, void *user_data) >> } >> if (result) { >> - ret = compare_chr_send(&s->chr_out, pkt->data, pkt->size); >> + ret = compare_chr_send(s, >> + pkt->data, >> + pkt->size, >> + pkt->vnet_hdr_len); > > Why not check vnet_hdr support here? And don't we need strip vnet_hdr > here? (Since the redirector on destination does not do this) If we create a normal qemu guest that use virtio-net-pci driver, the guest's send packet have the vnet_hdr, qemu virtio-net-pci driver will strip vnet_hdr then send to external network. In COLO we just redirect or mirror the packet, finally the packet will back to primary qemu virtio-net-pci driver, So we just send the packet. Thanks Zhang Chen > >> if (ret < 0) { >> error_report("colo_send_primary_packet failed"); >> } >> @@ -493,9 +498,10 @@ static void colo_compare_connection(void >> *opaque, void *user_data) >> } >> } >> -static int compare_chr_send(CharBackend *out, >> +static int compare_chr_send(CompareState *s, >> const uint8_t *buf, >> - uint32_t size) >> + uint32_t size, >> + uint32_t vnet_hdr_len) >> { >> int ret = 0; >> uint32_t len = htonl(size); >> @@ -504,12 +510,24 @@ static int compare_chr_send(CharBackend *out, >> return 0; >> } >> - ret = qemu_chr_fe_write_all(out, (uint8_t *)&len, sizeof(len)); >> + ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, >> sizeof(len)); >> if (ret != sizeof(len)) { >> goto err; >> } >> - ret = qemu_chr_fe_write_all(out, (uint8_t *)buf, size); >> + if (s->vnet_hdr) { >> + /* >> + * We send vnet header len make other module(like >> filter-redirector) >> + * know how to parse net packet correctly. >> + */ > > But redirector does not strip the vnet header, does this really work? Sorry, Here is a typo, I will fix it to "like colo-compare". Thanks ZhangChen > > Thanks > > > . >
On 2017年05月25日 21:18, Zhang Chen wrote: >>> @@ -472,7 +474,10 @@ static void colo_compare_connection(void >>> *opaque, void *user_data) >>> } >>> if (result) { >>> - ret = compare_chr_send(&s->chr_out, pkt->data, pkt->size); >>> + ret = compare_chr_send(s, >>> + pkt->data, >>> + pkt->size, >>> + pkt->vnet_hdr_len); >> >> Why not check vnet_hdr support here? And don't we need strip >> vnet_hdr here? (Since the redirector on destination does not do this) > > If we create a normal qemu guest that use virtio-net-pci driver, the > guest's send packet have the vnet_hdr, > qemu virtio-net-pci driver will strip vnet_hdr then send to external > network. In COLO we just redirect > or mirror the packet, finally the packet will back to primary qemu > virtio-net-pci driver, So we just send > the packet. > > Thanks > Zhang Chen But you send vnet_hdr_len unconditionally, which mean colo-compare can not work for redirector with vnet_hdr off now? Thanks
On 2017年05月26日 13:35, Jason Wang wrote: > > > On 2017年05月25日 21:18, Zhang Chen wrote: >>>> @@ -472,7 +474,10 @@ static void colo_compare_connection(void >>>> *opaque, void *user_data) >>>> } >>>> if (result) { >>>> - ret = compare_chr_send(&s->chr_out, pkt->data, >>>> pkt->size); >>>> + ret = compare_chr_send(s, >>>> + pkt->data, >>>> + pkt->size, >>>> + pkt->vnet_hdr_len); >>> >>> Why not check vnet_hdr support here? And don't we need strip >>> vnet_hdr here? (Since the redirector on destination does not do this) >> >> If we create a normal qemu guest that use virtio-net-pci driver, the >> guest's send packet have the vnet_hdr, >> qemu virtio-net-pci driver will strip vnet_hdr then send to external >> network. In COLO we just redirect >> or mirror the packet, finally the packet will back to primary qemu >> virtio-net-pci driver, So we just send >> the packet. >> >> Thanks >> Zhang Chen > > But you send vnet_hdr_len unconditionally, which mean colo-compare can > not work for redirector with vnet_hdr off now? > > Thanks > Btw, you may consider to update the COLO example in docs for the case that needs vnet hdr. Thanks
On 05/26/2017 01:36 PM, Jason Wang wrote: > > > On 2017年05月26日 13:35, Jason Wang wrote: >> >> >> On 2017年05月25日 21:18, Zhang Chen wrote: >>>>> @@ -472,7 +474,10 @@ static void colo_compare_connection(void >>>>> *opaque, void *user_data) >>>>> } >>>>> if (result) { >>>>> - ret = compare_chr_send(&s->chr_out, pkt->data, >>>>> pkt->size); >>>>> + ret = compare_chr_send(s, >>>>> + pkt->data, >>>>> + pkt->size, >>>>> + pkt->vnet_hdr_len); >>>> >>>> Why not check vnet_hdr support here? And don't we need strip >>>> vnet_hdr here? (Since the redirector on destination does not do this) >>> >>> If we create a normal qemu guest that use virtio-net-pci driver, the >>> guest's send packet have the vnet_hdr, >>> qemu virtio-net-pci driver will strip vnet_hdr then send to external >>> network. In COLO we just redirect >>> or mirror the packet, finally the packet will back to primary qemu >>> virtio-net-pci driver, So we just send >>> the packet. >>> >>> Thanks >>> Zhang Chen >> >> But you send vnet_hdr_len unconditionally, which mean colo-compare >> can not work for redirector with vnet_hdr off now? No, we input vnet_hdr_len unconditionally but in compare_chr_send() use s->vnet_hdr to decide whether send the vnet_hdr_len field. so colo-compare can work for all redirector. >> >> Thanks >> > > Btw, you may consider to update the COLO example in docs for the case > that needs vnet hdr. Yes, thanks to remind me. I will add it in next version. Thanks Zhang Chen > > Thanks > > > > . >
diff --git a/net/colo-compare.c b/net/colo-compare.c index bf0b856..f89b380 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -73,6 +73,7 @@ typedef struct CompareState { CharBackend chr_out; SocketReadState pri_rs; SocketReadState sec_rs; + bool vnet_hdr; /* connection list: the connections belonged to this NIC could be found * in this list. @@ -97,9 +98,10 @@ enum { SECONDARY_IN, }; -static int compare_chr_send(CharBackend *out, +static int compare_chr_send(CompareState *s, const uint8_t *buf, - uint32_t size); + uint32_t size, + uint32_t vnet_hdr_len); static gint seq_sorter(Packet *a, Packet *b, gpointer data) { @@ -472,7 +474,10 @@ static void colo_compare_connection(void *opaque, void *user_data) } if (result) { - ret = compare_chr_send(&s->chr_out, pkt->data, pkt->size); + ret = compare_chr_send(s, + pkt->data, + pkt->size, + pkt->vnet_hdr_len); if (ret < 0) { error_report("colo_send_primary_packet failed"); } @@ -493,9 +498,10 @@ static void colo_compare_connection(void *opaque, void *user_data) } } -static int compare_chr_send(CharBackend *out, +static int compare_chr_send(CompareState *s, const uint8_t *buf, - uint32_t size) + uint32_t size, + uint32_t vnet_hdr_len) { int ret = 0; uint32_t len = htonl(size); @@ -504,12 +510,24 @@ static int compare_chr_send(CharBackend *out, return 0; } - ret = qemu_chr_fe_write_all(out, (uint8_t *)&len, sizeof(len)); + ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len)); if (ret != sizeof(len)) { goto err; } - ret = qemu_chr_fe_write_all(out, (uint8_t *)buf, size); + if (s->vnet_hdr) { + /* + * We send vnet header len make other module(like filter-redirector) + * know how to parse net packet correctly. + */ + len = htonl(vnet_hdr_len); + ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len)); + if (ret != sizeof(len)) { + goto err; + } + } + + ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size); if (ret != size) { goto err; } @@ -646,13 +664,38 @@ static void compare_set_outdev(Object *obj, const char *value, Error **errp) s->outdev = g_strdup(value); } +static char *compare_get_vnet_hdr(Object *obj, Error **errp) +{ + CompareState *s = COLO_COMPARE(obj); + + return s->vnet_hdr ? g_strdup("on") : g_strdup("off"); +} + +static void compare_set_vnet_hdr(Object *obj, + const char *value, + Error **errp) +{ + CompareState *s = COLO_COMPARE(obj); + + if (strcmp(value, "on") && strcmp(value, "off")) { + error_setg(errp, "Invalid value for colo-compare vnet_hdr, " + "should be 'on' or 'off'"); + return; + } + + s->vnet_hdr = !strcmp(value, "on"); +} + static void compare_pri_rs_finalize(SocketReadState *pri_rs) { CompareState *s = container_of(pri_rs, CompareState, pri_rs); if (packet_enqueue(s, PRIMARY_IN)) { trace_colo_compare_main("primary: unsupported packet in"); - compare_chr_send(&s->chr_out, pri_rs->buf, pri_rs->packet_len); + compare_chr_send(s, + pri_rs->buf, + pri_rs->packet_len, + pri_rs->vnet_hdr_len); } else { /* compare connection */ g_queue_foreach(&s->conn_list, colo_compare_connection, s); @@ -735,7 +778,9 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp) } net_socket_rs_init(&s->pri_rs, compare_pri_rs_finalize); + s->pri_rs.vnet_hdr = s->vnet_hdr; net_socket_rs_init(&s->sec_rs, compare_sec_rs_finalize); + s->sec_rs.vnet_hdr = s->vnet_hdr; g_queue_init(&s->conn_list); @@ -761,7 +806,10 @@ static void colo_flush_packets(void *opaque, void *user_data) while (!g_queue_is_empty(&conn->primary_list)) { pkt = g_queue_pop_head(&conn->primary_list); - compare_chr_send(&s->chr_out, pkt->data, pkt->size); + compare_chr_send(s, + pkt->data, + pkt->size, + pkt->vnet_hdr_len); packet_destroy(pkt, NULL); } while (!g_queue_is_empty(&conn->secondary_list)) { @@ -779,6 +827,8 @@ static void colo_compare_class_init(ObjectClass *oc, void *data) static void colo_compare_init(Object *obj) { + CompareState *s = COLO_COMPARE(obj); + object_property_add_str(obj, "primary_in", compare_get_pri_indev, compare_set_pri_indev, NULL); @@ -788,6 +838,14 @@ static void colo_compare_init(Object *obj) object_property_add_str(obj, "outdev", compare_get_outdev, compare_set_outdev, NULL); + /* + * The vnet_hdr is disabled by default, if you want to enable + * this option, you must enable all the option on related modules + * (like other filter or colo-compare). + */ + s->vnet_hdr = false; + object_property_add_str(obj, "vnet_hdr", compare_get_vnet_hdr, + compare_set_vnet_hdr, NULL); } static void colo_compare_finalize(Object *obj) diff --git a/qemu-options.hx b/qemu-options.hx index a3c4688..bb3f400 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4057,13 +4057,13 @@ Dump the network traffic on netdev @var{dev} to the file specified by The file format is libpcap, so it can be analyzed with tools such as tcpdump or Wireshark. -@item -object colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid}, -outdev=@var{chardevid} +@item -object colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},outdev=@var{chardevid},vnet_hdr=@var{on|off} Colo-compare gets packet from primary_in@var{chardevid} and secondary_in@var{chardevid}, than compare primary packet with secondary packet. If the packets are same, we will output primary packet to outdev@var{chardevid}, else we will notify colo-frame do checkpoint and send primary packet to outdev@var{chardevid}. +if vnet_hdr = on, colo compare will send/recv packet with vnet_hdr_len. we must use it with the help of filter-mirror and filter-redirector.
We add the vnet_hdr option for colo-compare, default is disable. If you use virtio-net-pci net driver, please enable it. You can use it for example: -object colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,vnet_hdr=on COLO-compare can get vnet header length from filter, Add vnet_hdr_len to struct packet and output packet with the vnet_hdr_len. Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> --- net/colo-compare.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++------- qemu-options.hx | 4 +-- 2 files changed, 69 insertions(+), 11 deletions(-)