* [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support
@ 2017-04-28 9:47 Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo Zhang Chen
` (9 more replies)
0 siblings, 10 replies; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
If user use -device virtio-net-pci, virtio-net driver will add a header
to raw net packet that colo-proxy can't handle it. COLO-proxy just
focus on the packet payload, so we skip the virtio-net header to compare
the sent packet that primary guest's to secondary guest's.
Zhang Chen (10):
net: Add vnet_hdr_len related callback in NetClientInfo
net/tap.c: Add tap_get_vnet_hdr_len and tap_get_using_vnet_hdr
function
net/netmap.c: Add netmap_get_vnet_hdr_len function
net/filter-mirror.c: Add filter-mirror and filter-redirector vnet
support.
net/net.c: Add vnet header length to SocketReadState
tests/e1000e-test.c : change e1000e test case send data format
tests/virtio-net-test.c : change virtio-net test case iov send data
format
net/colo-compare.c: Make colo-compare support vnet_hdr_len
net/colo.c: Add vnet packet parse feature in colo-proxy
net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare
include/net/net.h | 10 +++++++++-
net/colo-compare.c | 48 +++++++++++++++++++++++++++++++++++++++---------
net/colo.c | 9 +++++----
net/colo.h | 4 +++-
net/filter-mirror.c | 28 +++++++++++++++++++++++-----
net/filter-rewriter.c | 2 +-
net/net.c | 42 ++++++++++++++++++++++++++++++++++++++++--
net/netmap.c | 8 ++++++++
net/tap-win32.c | 12 ++++++++++++
net/tap.c | 20 ++++++++++++++++++++
tests/e1000e-test.c | 10 ++++++++--
tests/virtio-net-test.c | 18 ++++++++++++++----
12 files changed, 182 insertions(+), 29 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
@ 2017-04-28 9:47 ` Zhang Chen
2017-05-02 5:46 ` Jason Wang
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 02/10] net/tap.c: Add tap_get_vnet_hdr_len and tap_get_using_vnet_hdr function Zhang Chen
` (8 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
Add get_vnet_hdr_len and get_using_vnet_hdr callback
that make we get vnet_hdr_len easily.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
include/net/net.h | 6 ++++++
net/net.c | 18 ++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/include/net/net.h b/include/net/net.h
index 99b28d5..402d913 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -57,7 +57,9 @@ typedef RxFilterInfo *(QueryRxFilter)(NetClientState *);
typedef bool (HasUfo)(NetClientState *);
typedef bool (HasVnetHdr)(NetClientState *);
typedef bool (HasVnetHdrLen)(NetClientState *, int);
+typedef int (GetVnetHdrLen)(NetClientState *);
typedef void (UsingVnetHdr)(NetClientState *, bool);
+typedef bool (GetUsingVnetHdr)(NetClientState *);
typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
typedef void (SetVnetHdrLen)(NetClientState *, int);
typedef int (SetVnetLE)(NetClientState *, bool);
@@ -79,7 +81,9 @@ typedef struct NetClientInfo {
HasUfo *has_ufo;
HasVnetHdr *has_vnet_hdr;
HasVnetHdrLen *has_vnet_hdr_len;
+ GetVnetHdrLen *get_vnet_hdr_len;
UsingVnetHdr *using_vnet_hdr;
+ GetUsingVnetHdr *get_using_vnet_hdr;
SetOffload *set_offload;
SetVnetHdrLen *set_vnet_hdr_len;
SetVnetLE *set_vnet_le;
@@ -155,7 +159,9 @@ void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6]);
bool qemu_has_ufo(NetClientState *nc);
bool qemu_has_vnet_hdr(NetClientState *nc);
bool qemu_has_vnet_hdr_len(NetClientState *nc, int len);
+int qemu_get_vnet_hdr_len(NetClientState *nc);
void qemu_using_vnet_hdr(NetClientState *nc, bool enable);
+bool qemu_get_using_vnet_hdr(NetClientState *nc);
void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
int ecn, int ufo);
void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
diff --git a/net/net.c b/net/net.c
index 0ac3b9e..f69260f 100644
--- a/net/net.c
+++ b/net/net.c
@@ -466,6 +466,15 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc, int len)
return nc->info->has_vnet_hdr_len(nc, len);
}
+int qemu_get_vnet_hdr_len(NetClientState *nc)
+{
+ if (!nc || !nc->info->get_vnet_hdr_len) {
+ return false;
+ }
+
+ return nc->info->get_vnet_hdr_len(nc);
+}
+
void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
{
if (!nc || !nc->info->using_vnet_hdr) {
@@ -475,6 +484,15 @@ void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
nc->info->using_vnet_hdr(nc, enable);
}
+bool qemu_get_using_vnet_hdr(NetClientState *nc)
+{
+ if (!nc || !nc->info->get_using_vnet_hdr) {
+ return false;
+ }
+
+ return nc->info->get_using_vnet_hdr(nc);
+}
+
void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
int ecn, int ufo)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH V3 02/10] net/tap.c: Add tap_get_vnet_hdr_len and tap_get_using_vnet_hdr function
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo Zhang Chen
@ 2017-04-28 9:47 ` Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 03/10] net/netmap.c: Add netmap_get_vnet_hdr_len function Zhang Chen
` (7 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
Make tap backend support get_vnet_hdr_len.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
net/tap-win32.c | 12 ++++++++++++
net/tap.c | 20 ++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 662f9b6..337e8ea 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -729,6 +729,11 @@ static void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
{
}
+static void tap_get_using_vnet_hdr(NetClientState *nc)
+{
+ return false;
+}
+
static void tap_set_offload(NetClientState *nc, int csum, int tso4,
int tso6, int ecn, int ufo)
{
@@ -744,6 +749,11 @@ static bool tap_has_vnet_hdr_len(NetClientState *nc, int len)
return false;
}
+static int tap_get_vnet_hdr_len(NetClientState *nc)
+{
+ return 0;
+}
+
static void tap_set_vnet_hdr_len(NetClientState *nc, int len)
{
abort();
@@ -757,7 +767,9 @@ static NetClientInfo net_tap_win32_info = {
.has_ufo = tap_has_ufo,
.has_vnet_hdr = tap_has_vnet_hdr,
.has_vnet_hdr_len = tap_has_vnet_hdr_len,
+ .get_vnet_hdr_len = tap_get_vnet_hdr_len,
.using_vnet_hdr = tap_using_vnet_hdr,
+ .get_using_vnet_hdr = tap_get_using_vnet_hdr,
.set_offload = tap_set_offload,
.set_vnet_hdr_len = tap_set_vnet_hdr_len,
};
diff --git a/net/tap.c b/net/tap.c
index 979e622..214c83d 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -246,6 +246,15 @@ static bool tap_has_vnet_hdr_len(NetClientState *nc, int len)
return !!tap_probe_vnet_hdr_len(s->fd, len);
}
+static int tap_get_vnet_hdr_len(NetClientState *nc)
+{
+ TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+ assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
+
+ return s->host_vnet_hdr_len;
+}
+
static void tap_set_vnet_hdr_len(NetClientState *nc, int len)
{
TAPState *s = DO_UPCAST(TAPState, nc, nc);
@@ -268,6 +277,15 @@ static void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
s->using_vnet_hdr = using_vnet_hdr;
}
+static bool tap_get_using_vnet_hdr(NetClientState *nc)
+{
+ TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+ assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
+
+ return s->using_vnet_hdr;
+}
+
static int tap_set_vnet_le(NetClientState *nc, bool is_le)
{
TAPState *s = DO_UPCAST(TAPState, nc, nc);
@@ -354,7 +372,9 @@ static NetClientInfo net_tap_info = {
.has_ufo = tap_has_ufo,
.has_vnet_hdr = tap_has_vnet_hdr,
.has_vnet_hdr_len = tap_has_vnet_hdr_len,
+ .get_vnet_hdr_len = tap_get_vnet_hdr_len,
.using_vnet_hdr = tap_using_vnet_hdr,
+ .get_using_vnet_hdr = tap_get_using_vnet_hdr,
.set_offload = tap_set_offload,
.set_vnet_hdr_len = tap_set_vnet_hdr_len,
.set_vnet_le = tap_set_vnet_le,
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH V3 03/10] net/netmap.c: Add netmap_get_vnet_hdr_len function
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 02/10] net/tap.c: Add tap_get_vnet_hdr_len and tap_get_using_vnet_hdr function Zhang Chen
@ 2017-04-28 9:47 ` Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support Zhang Chen
` (6 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
Make netmap support get_vnet_hdr_len.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
net/netmap.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/netmap.c b/net/netmap.c
index 2d11a8f..694c340 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -360,6 +360,13 @@ static bool netmap_has_vnet_hdr_len(NetClientState *nc, int len)
return true;
}
+static int netmap_get_vnet_hdr_len(NetClientState *nc)
+{
+ NetmapState *s = DO_UPCAST(NetmapState, nc, nc);
+
+ return s->vnet_hdr_len;
+}
+
/* A netmap interface that supports virtio-net headers always
* supports UFO, so we use this callback also for the has_ufo hook. */
static bool netmap_has_vnet_hdr(NetClientState *nc)
@@ -409,6 +416,7 @@ static NetClientInfo net_netmap_info = {
.has_ufo = netmap_has_vnet_hdr,
.has_vnet_hdr = netmap_has_vnet_hdr,
.has_vnet_hdr_len = netmap_has_vnet_hdr_len,
+ .get_vnet_hdr_len = netmap_get_vnet_hdr_len,
.using_vnet_hdr = netmap_using_vnet_hdr,
.set_offload = netmap_set_offload,
.set_vnet_hdr_len = netmap_set_vnet_hdr_len,
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
` (2 preceding siblings ...)
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 03/10] net/netmap.c: Add netmap_get_vnet_hdr_len function Zhang Chen
@ 2017-04-28 9:47 ` Zhang Chen
2017-05-02 5:47 ` Jason Wang
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState Zhang Chen
` (5 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
In this patch, we change the send packet format from
struct {int size; const uint8_t buf[];} to {int size; int vnet_hdr_len; const uint8_t buf[];}.
make other module(like colo-compare) know how to parse net packet correctly.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
net/filter-mirror.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 72fa7c2..bb9ecf3 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -43,12 +43,14 @@ typedef struct MirrorState {
SocketReadState rs;
} MirrorState;
-static int filter_mirror_send(CharBackend *chr_out,
+static int filter_mirror_send(MirrorState *s,
const struct iovec *iov,
int iovcnt)
{
+ NetFilterState *nf = NETFILTER(s);
int ret = 0;
ssize_t size = 0;
+ ssize_t vnet_hdr_len;
uint32_t len = 0;
char *buf;
@@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend *chr_out,
}
len = htonl(size);
- ret = qemu_chr_fe_write_all(chr_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;
+ }
+
+ /*
+ * We send vnet header len make other module(like colo-compare)
+ * know how to parse net packet correctly.
+ */
+ if (qemu_get_using_vnet_hdr(nf->netdev)) {
+ vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
+ } else {
+ vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
+ }
+
+ 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;
}
buf = g_malloc(size);
iov_to_buf(iov, iovcnt, 0, buf, size);
- ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)buf, size);
+ ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
g_free(buf);
if (ret != size) {
goto err;
@@ -141,7 +159,7 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
MirrorState *s = FILTER_MIRROR(nf);
int ret;
- ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
+ ret = filter_mirror_send(s, iov, iovcnt);
if (ret) {
error_report("filter_mirror_send failed(%s)", strerror(-ret));
}
@@ -164,7 +182,7 @@ static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
int ret;
if (qemu_chr_fe_get_driver(&s->chr_out)) {
- ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
+ ret = filter_mirror_send(s, iov, iovcnt);
if (ret) {
error_report("filter_mirror_send failed(%s)", strerror(-ret));
}
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
` (3 preceding siblings ...)
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support Zhang Chen
@ 2017-04-28 9:47 ` Zhang Chen
2017-05-02 5:53 ` Jason Wang
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 06/10] tests/e1000e-test.c : change e1000e test case send data format Zhang Chen
` (4 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
Address Jason Wang's comments add vnet header length to SocketReadState.
So we change net_fill_rstate() to read
struct {int size; int vnet_hdr_len; const uint8_t buf[];}.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
include/net/net.h | 4 +++-
net/net.c | 24 ++++++++++++++++++++++--
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 402d913..865cb98 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -115,9 +115,11 @@ typedef struct NICState {
} NICState;
struct SocketReadState {
- int state; /* 0 = getting length, 1 = getting data */
+ /* 0 = getting length, 1 = getting vnet header length, 2 = getting data */
+ int state;
uint32_t index;
uint32_t packet_len;
+ uint32_t vnet_hdr_len;
uint8_t buf[NET_BUFSIZE];
SocketReadStateFinalize *finalize;
};
diff --git a/net/net.c b/net/net.c
index f69260f..5a6b5ac 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1639,8 +1639,12 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
unsigned int l;
while (size > 0) {
- /* reassemble a packet from the network */
- switch (rs->state) { /* 0 = getting length, 1 = getting data */
+ /* Reassemble a packet from the network.
+ * 0 = getting length.
+ * 1 = getting vnet header length.
+ * 2 = getting data.
+ */
+ switch (rs->state) {
case 0:
l = 4 - rs->index;
if (l > size) {
@@ -1658,6 +1662,22 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
}
break;
case 1:
+ l = 4 - rs->index;
+ if (l > size) {
+ l = size;
+ }
+ memcpy(rs->buf + rs->index, buf, l);
+ buf += l;
+ size -= l;
+ rs->index += l;
+ if (rs->index == 4) {
+ /* got vnet header length */
+ rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
+ rs->index = 0;
+ rs->state = 2;
+ }
+ break;
+ case 2:
l = rs->packet_len - rs->index;
if (l > size) {
l = size;
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH V3 06/10] tests/e1000e-test.c : change e1000e test case send data format
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
` (4 preceding siblings ...)
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState Zhang Chen
@ 2017-04-28 9:47 ` Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 07/10] tests/virtio-net-test.c : change virtio-net test case iov " Zhang Chen
` (3 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
Because we change net_fill_rstate() date format from
struct {int size; const uint8_t buf[];} to {int size; int vnet_hdr_len; const uint8_t buf[];}.
So, we add fake vnet_hdr_len in e1000e test case.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
tests/e1000e-test.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tests/e1000e-test.c b/tests/e1000e-test.c
index c612dc6..fb3bd77 100644
--- a/tests/e1000e-test.c
+++ b/tests/e1000e-test.c
@@ -329,11 +329,15 @@ static void e1000e_receive_verify(e1000e_device *d)
char test[] = "TEST";
int len = htonl(sizeof(test));
+ int vnet_hdr_len = 0;
struct iovec iov[] = {
{
.iov_base = &len,
.iov_len = sizeof(len),
},{
+ .iov_base = &vnet_hdr_len,
+ .iov_len = sizeof(vnet_hdr_len),
+ },{
.iov_base = test,
.iov_len = sizeof(test),
},
@@ -344,8 +348,10 @@ static void e1000e_receive_verify(e1000e_device *d)
int ret;
/* Send a dummy packet to device's socket*/
- ret = iov_send(test_sockets[0], iov, 2, 0, sizeof(len) + sizeof(test));
- g_assert_cmpint(ret, == , sizeof(test) + sizeof(len));
+ ret = iov_send(test_sockets[0], iov, 3, 0, sizeof(len)
+ + sizeof(vnet_hdr_len) + sizeof(test));
+ g_assert_cmpint(ret, == , sizeof(test) + sizeof(len)
+ + sizeof(vnet_hdr_len));
/* Prepare test data buffer */
uint64_t data = guest_alloc(test_alloc, data_len);
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH V3 07/10] tests/virtio-net-test.c : change virtio-net test case iov send data format
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
` (5 preceding siblings ...)
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 06/10] tests/e1000e-test.c : change e1000e test case send data format Zhang Chen
@ 2017-04-28 9:47 ` Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 08/10] net/colo-compare.c: Make colo-compare support vnet_hdr_len Zhang Chen
` (2 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
Because we change net_fill_rstate() date format from
struct {int size; const uint8_t buf[];} to {int size; int vnet_hdr_len; const uint8_t buf[];}.
So, we add fake vnet_hdr_len in virtio-net test case.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
tests/virtio-net-test.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 8f94360..93f4fe0 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -89,11 +89,15 @@ static void rx_test(QVirtioDevice *dev,
char test[] = "TEST";
char buffer[64];
int len = htonl(sizeof(test));
+ int vnet_hdr_len = 0;
struct iovec iov[] = {
{
.iov_base = &len,
.iov_len = sizeof(len),
}, {
+ .iov_base = &vnet_hdr_len,
+ .iov_len = sizeof(vnet_hdr_len),
+ }, {
.iov_base = test,
.iov_len = sizeof(test),
},
@@ -105,8 +109,9 @@ static void rx_test(QVirtioDevice *dev,
free_head = qvirtqueue_add(vq, req_addr, 64, true, false);
qvirtqueue_kick(dev, vq, free_head);
- ret = iov_send(socket, iov, 2, 0, sizeof(len) + sizeof(test));
- g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len));
+ ret = iov_send(socket, iov, 3, 0, sizeof(len)
+ + sizeof(test) + sizeof(vnet_hdr_len));
+ g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len) + sizeof(vnet_hdr_len));
qvirtio_wait_queue_isr(dev, vq, QVIRTIO_NET_TIMEOUT_US);
memread(req_addr + VNET_HDR_SIZE, buffer, sizeof(test));
@@ -151,12 +156,16 @@ static void rx_stop_cont_test(QVirtioDevice *dev,
char test[] = "TEST";
char buffer[64];
int len = htonl(sizeof(test));
+ int vnet_hdr_len = 0;
QDict *rsp;
struct iovec iov[] = {
{
.iov_base = &len,
.iov_len = sizeof(len),
}, {
+ .iov_base = &vnet_hdr_len,
+ .iov_len = sizeof(vnet_hdr_len),
+ }, {
.iov_base = test,
.iov_len = sizeof(test),
},
@@ -171,8 +180,9 @@ static void rx_stop_cont_test(QVirtioDevice *dev,
rsp = qmp("{ 'execute' : 'stop'}");
QDECREF(rsp);
- ret = iov_send(socket, iov, 2, 0, sizeof(len) + sizeof(test));
- g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len));
+ ret = iov_send(socket, iov, 3, 0, sizeof(len)
+ + sizeof(test) + sizeof(vnet_hdr_len));
+ g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len) + sizeof(vnet_hdr_len));
/* We could check the status, but this command is more importantly to
* ensure the packet data gets queued in QEMU, before we do 'cont'.
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH V3 08/10] net/colo-compare.c: Make colo-compare support vnet_hdr_len
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
` (6 preceding siblings ...)
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 07/10] tests/virtio-net-test.c : change virtio-net test case iov " Zhang Chen
@ 2017-04-28 9:47 ` Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 09/10] net/colo.c: Add vnet packet parse feature in colo-proxy Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 10/10] net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare Zhang Chen
9 siblings, 0 replies; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
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 | 39 ++++++++++++++++++++++++++++++++-------
net/colo.c | 3 ++-
net/colo.h | 4 +++-
net/filter-rewriter.c | 2 +-
4 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 54e6d40..b3e933c 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -99,7 +99,8 @@ enum {
static int compare_chr_send(CharBackend *out,
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)
{
@@ -121,9 +122,13 @@ static int packet_enqueue(CompareState *s, int mode)
Connection *conn;
if (mode == PRIMARY_IN) {
- pkt = packet_new(s->pri_rs.buf, s->pri_rs.packet_len);
+ pkt = packet_new(s->pri_rs.buf,
+ s->pri_rs.packet_len,
+ s->pri_rs.vnet_hdr_len);
} else {
- pkt = packet_new(s->sec_rs.buf, s->sec_rs.packet_len);
+ pkt = packet_new(s->sec_rs.buf,
+ s->sec_rs.packet_len,
+ s->sec_rs.vnet_hdr_len);
}
if (parse_packet_early(pkt)) {
@@ -436,7 +441,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->chr_out,
+ pkt->data,
+ pkt->size,
+ pkt->vnet_hdr_len);
if (ret < 0) {
error_report("colo_send_primary_packet failed");
}
@@ -459,7 +467,8 @@ static void colo_compare_connection(void *opaque, void *user_data)
static int compare_chr_send(CharBackend *out,
const uint8_t *buf,
- uint32_t size)
+ uint32_t size,
+ uint32_t vnet_hdr_len)
{
int ret = 0;
uint32_t len = htonl(size);
@@ -473,6 +482,16 @@ static int compare_chr_send(CharBackend *out,
goto err;
}
+ /*
+ * We send vnet header len make other module(like colo-compare)
+ * know how to parse net packet correctly.
+ */
+ len = htonl(vnet_hdr_len);
+ ret = qemu_chr_fe_write_all(out, (uint8_t *)&len, sizeof(len));
+ if (ret != sizeof(len)) {
+ goto err;
+ }
+
ret = qemu_chr_fe_write_all(out, (uint8_t *)buf, size);
if (ret != size) {
goto err;
@@ -616,7 +635,10 @@ static void compare_pri_rs_finalize(SocketReadState *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->chr_out,
+ 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);
@@ -725,7 +747,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->chr_out,
+ pkt->data,
+ pkt->size,
+ pkt->vnet_hdr_len);
packet_destroy(pkt, NULL);
}
while (!g_queue_is_empty(&conn->secondary_list)) {
diff --git a/net/colo.c b/net/colo.c
index 8cc166b..180eaed 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -153,13 +153,14 @@ void connection_destroy(void *opaque)
g_slice_free(Connection, conn);
}
-Packet *packet_new(const void *data, int size)
+Packet *packet_new(const void *data, int size, int vnet_hdr_len)
{
Packet *pkt = g_slice_new(Packet);
pkt->data = g_memdup(data, size);
pkt->size = size;
pkt->creation_ms = qemu_clock_get_ms(QEMU_CLOCK_HOST);
+ pkt->vnet_hdr_len = vnet_hdr_len;
return pkt;
}
diff --git a/net/colo.h b/net/colo.h
index 7c524f3..caedb0d 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -43,6 +43,8 @@ typedef struct Packet {
int size;
/* Time of packet creation, in wall clock ms */
int64_t creation_ms;
+ /* Get vnet_hdr_len from filter */
+ uint32_t vnet_hdr_len;
} Packet;
typedef struct ConnectionKey {
@@ -82,7 +84,7 @@ Connection *connection_get(GHashTable *connection_track_table,
ConnectionKey *key,
GQueue *conn_list);
void connection_hashtable_reset(GHashTable *connection_track_table);
-Packet *packet_new(const void *data, int size);
+Packet *packet_new(const void *data, int size, int vnet_hdr_len);
void packet_destroy(void *opaque, void *user_data);
#endif /* QEMU_COLO_PROXY_H */
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index afa06e8..63256c7 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -158,7 +158,7 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
char *buf = g_malloc0(size);
iov_to_buf(iov, iovcnt, 0, buf, size);
- pkt = packet_new(buf, size);
+ pkt = packet_new(buf, size, 0);
g_free(buf);
/*
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH V3 09/10] net/colo.c: Add vnet packet parse feature in colo-proxy
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
` (7 preceding siblings ...)
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 08/10] net/colo-compare.c: Make colo-compare support vnet_hdr_len Zhang Chen
@ 2017-04-28 9:47 ` Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 10/10] net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare Zhang Chen
9 siblings, 0 replies; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
Make colo-compare and filter-rewriter can parse vnet packet.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
net/colo.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/colo.c b/net/colo.c
index 180eaed..28ce7c8 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -43,11 +43,11 @@ int parse_packet_early(Packet *pkt)
{
int network_length;
static const uint8_t vlan[] = {0x81, 0x00};
- uint8_t *data = pkt->data;
+ uint8_t *data = pkt->data + pkt->vnet_hdr_len;
uint16_t l3_proto;
ssize_t l2hdr_len = eth_get_l2_hdr_length(data);
- if (pkt->size < ETH_HLEN) {
+ if (pkt->size < ETH_HLEN + pkt->vnet_hdr_len) {
trace_colo_proxy_main("pkt->size < ETH_HLEN");
return 1;
}
@@ -73,7 +73,7 @@ int parse_packet_early(Packet *pkt)
}
network_length = pkt->ip->ip_hl * 4;
- if (pkt->size < l2hdr_len + network_length) {
+ if (pkt->size < l2hdr_len + network_length + pkt->vnet_hdr_len) {
trace_colo_proxy_main("pkt->size < network_header + network_length");
return 1;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [Qemu-devel] [PATCH V3 10/10] net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
` (8 preceding siblings ...)
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 09/10] net/colo.c: Add vnet packet parse feature in colo-proxy Zhang Chen
@ 2017-04-28 9:47 ` Zhang Chen
9 siblings, 0 replies; 28+ messages in thread
From: Zhang Chen @ 2017-04-28 9:47 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
COLO-Proxy just focus on packet payload, So we skip vnet header.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
net/colo-compare.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index b3e933c..1941ad9 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -187,6 +187,8 @@ static int packet_enqueue(CompareState *s, int mode)
*/
static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
{
+ int offset_all;
+
if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
@@ -200,9 +202,12 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
sec_ip_src, sec_ip_dst);
}
+ offset_all = ppkt->vnet_hdr_len + offset;
+
if (ppkt->size == spkt->size) {
- return memcmp(ppkt->data + offset, spkt->data + offset,
- spkt->size - offset);
+ return memcmp(ppkt->data + offset_all,
+ spkt->data + offset_all,
+ spkt->size - offset_all);
} else {
trace_colo_compare_main("Net packet size are not the same");
return -1;
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo Zhang Chen
@ 2017-05-02 5:46 ` Jason Wang
2017-05-03 3:08 ` Zhang Chen
0 siblings, 1 reply; 28+ messages in thread
From: Jason Wang @ 2017-05-02 5:46 UTC (permalink / raw)
To: Zhang Chen, qemu devel
Cc: zhanghailiang, weifuqiang, eddie . dong, bian naimeng, Li Zhijian
On 2017年04月28日 17:47, Zhang Chen wrote:
> Add get_vnet_hdr_len and get_using_vnet_hdr callback
> that make we get vnet_hdr_len easily.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
> include/net/net.h | 6 ++++++
> net/net.c | 18 ++++++++++++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 99b28d5..402d913 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -57,7 +57,9 @@ typedef RxFilterInfo *(QueryRxFilter)(NetClientState *);
> typedef bool (HasUfo)(NetClientState *);
> typedef bool (HasVnetHdr)(NetClientState *);
> typedef bool (HasVnetHdrLen)(NetClientState *, int);
> +typedef int (GetVnetHdrLen)(NetClientState *);
> typedef void (UsingVnetHdr)(NetClientState *, bool);
> +typedef bool (GetUsingVnetHdr)(NetClientState *);
> typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
> typedef void (SetVnetHdrLen)(NetClientState *, int);
> typedef int (SetVnetLE)(NetClientState *, bool);
> @@ -79,7 +81,9 @@ typedef struct NetClientInfo {
> HasUfo *has_ufo;
> HasVnetHdr *has_vnet_hdr;
> HasVnetHdrLen *has_vnet_hdr_len;
> + GetVnetHdrLen *get_vnet_hdr_len;
> UsingVnetHdr *using_vnet_hdr;
> + GetUsingVnetHdr *get_using_vnet_hdr;
> SetOffload *set_offload;
> SetVnetHdrLen *set_vnet_hdr_len;
> SetVnetLE *set_vnet_le;
> @@ -155,7 +159,9 @@ void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6]);
> bool qemu_has_ufo(NetClientState *nc);
> bool qemu_has_vnet_hdr(NetClientState *nc);
> bool qemu_has_vnet_hdr_len(NetClientState *nc, int len);
> +int qemu_get_vnet_hdr_len(NetClientState *nc);
> void qemu_using_vnet_hdr(NetClientState *nc, bool enable);
> +bool qemu_get_using_vnet_hdr(NetClientState *nc);
> void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
> int ecn, int ufo);
> void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
> diff --git a/net/net.c b/net/net.c
> index 0ac3b9e..f69260f 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -466,6 +466,15 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc, int len)
> return nc->info->has_vnet_hdr_len(nc, len);
> }
>
> +int qemu_get_vnet_hdr_len(NetClientState *nc)
> +{
> + if (!nc || !nc->info->get_vnet_hdr_len) {
> + return false;
> + }
> +
> + return nc->info->get_vnet_hdr_len(nc);
> +}
> +
> void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
> {
> if (!nc || !nc->info->using_vnet_hdr) {
> @@ -475,6 +484,15 @@ void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
> nc->info->using_vnet_hdr(nc, enable);
> }
>
> +bool qemu_get_using_vnet_hdr(NetClientState *nc)
> +{
> + if (!nc || !nc->info->get_using_vnet_hdr) {
> + return false;
> + }
> +
> + return nc->info->get_using_vnet_hdr(nc);
> +}
Looks like we can do this simply by:
Introduce two common fields in NetClientState:
bool using_vnet_hdr;
int vnet_hdr_len;
And set them during qemu_using_vnet_hdr() and qemu_set_vnet_hdr_len().
Then we can query them directly without introducing any new callbacks.
Thanks
> +
> void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
> int ecn, int ufo)
> {
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support Zhang Chen
@ 2017-05-02 5:47 ` Jason Wang
2017-05-03 3:18 ` Zhang Chen
0 siblings, 1 reply; 28+ messages in thread
From: Jason Wang @ 2017-05-02 5:47 UTC (permalink / raw)
To: Zhang Chen, qemu devel
Cc: zhanghailiang, weifuqiang, eddie . dong, bian naimeng, Li Zhijian
On 2017年04月28日 17:47, Zhang Chen wrote:
> In this patch, we change the send packet format from
> struct {int size; const uint8_t buf[];} to {int size; int vnet_hdr_len; const uint8_t buf[];}.
> make other module(like colo-compare) know how to parse net packet correctly.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
> net/filter-mirror.c | 28 +++++++++++++++++++++++-----
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 72fa7c2..bb9ecf3 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -43,12 +43,14 @@ typedef struct MirrorState {
> SocketReadState rs;
> } MirrorState;
>
> -static int filter_mirror_send(CharBackend *chr_out,
> +static int filter_mirror_send(MirrorState *s,
> const struct iovec *iov,
> int iovcnt)
> {
> + NetFilterState *nf = NETFILTER(s);
> int ret = 0;
> ssize_t size = 0;
> + ssize_t vnet_hdr_len;
> uint32_t len = 0;
> char *buf;
>
> @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend *chr_out,
> }
>
> len = htonl(size);
> - ret = qemu_chr_fe_write_all(chr_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;
> + }
> +
> + /*
> + * We send vnet header len make other module(like colo-compare)
> + * know how to parse net packet correctly.
> + */
> + if (qemu_get_using_vnet_hdr(nf->netdev)) {
> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
> + } else {
> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
> + }
Any reason to query peer here?
> +
> + 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;
> }
>
> buf = g_malloc(size);
> iov_to_buf(iov, iovcnt, 0, buf, size);
> - ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)buf, size);
> + ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
> g_free(buf);
> if (ret != size) {
> goto err;
> @@ -141,7 +159,7 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
> MirrorState *s = FILTER_MIRROR(nf);
> int ret;
>
> - ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
> + ret = filter_mirror_send(s, iov, iovcnt);
> if (ret) {
> error_report("filter_mirror_send failed(%s)", strerror(-ret));
> }
> @@ -164,7 +182,7 @@ static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
> int ret;
>
> if (qemu_chr_fe_get_driver(&s->chr_out)) {
> - ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
> + ret = filter_mirror_send(s, iov, iovcnt);
> if (ret) {
> error_report("filter_mirror_send failed(%s)", strerror(-ret));
> }
Do we need to strip vnet_hdr_len in redirector_to_filter() ?
Thanks
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState Zhang Chen
@ 2017-05-02 5:53 ` Jason Wang
2017-05-03 3:43 ` Zhang Chen
0 siblings, 1 reply; 28+ messages in thread
From: Jason Wang @ 2017-05-02 5:53 UTC (permalink / raw)
To: Zhang Chen, qemu devel
Cc: zhanghailiang, weifuqiang, eddie . dong, bian naimeng, Li Zhijian
On 2017年04月28日 17:47, Zhang Chen wrote:
> Address Jason Wang's comments add vnet header length to SocketReadState.
Instead of saying this, you can add "Suggested-by: Jason Wang
<jasowang@redhat.com>" above your sign-off.
> So we change net_fill_rstate() to read
> struct {int size; int vnet_hdr_len; const uint8_t buf[];}.
This makes me thinking about the backward compatibility. I think we'd
better try to keep it as much as possible. E.g how about pack
vnet_hdr_len into higher bits of size?
Thanks
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
> include/net/net.h | 4 +++-
> net/net.c | 24 ++++++++++++++++++++++--
> 2 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 402d913..865cb98 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -115,9 +115,11 @@ typedef struct NICState {
> } NICState;
>
> struct SocketReadState {
> - int state; /* 0 = getting length, 1 = getting data */
> + /* 0 = getting length, 1 = getting vnet header length, 2 = getting data */
> + int state;
> uint32_t index;
> uint32_t packet_len;
> + uint32_t vnet_hdr_len;
> uint8_t buf[NET_BUFSIZE];
> SocketReadStateFinalize *finalize;
> };
> diff --git a/net/net.c b/net/net.c
> index f69260f..5a6b5ac 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1639,8 +1639,12 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
> unsigned int l;
>
> while (size > 0) {
> - /* reassemble a packet from the network */
> - switch (rs->state) { /* 0 = getting length, 1 = getting data */
> + /* Reassemble a packet from the network.
> + * 0 = getting length.
> + * 1 = getting vnet header length.
> + * 2 = getting data.
> + */
> + switch (rs->state) {
> case 0:
> l = 4 - rs->index;
> if (l > size) {
> @@ -1658,6 +1662,22 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
> }
> break;
> case 1:
> + l = 4 - rs->index;
> + if (l > size) {
> + l = size;
> + }
> + memcpy(rs->buf + rs->index, buf, l);
> + buf += l;
> + size -= l;
> + rs->index += l;
> + if (rs->index == 4) {
> + /* got vnet header length */
> + rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
> + rs->index = 0;
> + rs->state = 2;
> + }
> + break;
> + case 2:
> l = rs->packet_len - rs->index;
> if (l > size) {
> l = size;
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo
2017-05-02 5:46 ` Jason Wang
@ 2017-05-03 3:08 ` Zhang Chen
0 siblings, 0 replies; 28+ messages in thread
From: Zhang Chen @ 2017-05-03 3:08 UTC (permalink / raw)
To: Jason Wang, qemu devel
Cc: zhangchen.fnst, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
On 05/02/2017 12:46 PM, Jason Wang wrote:
>
>
> On 2017年04月28日 17:47, Zhang Chen wrote:
>> Add get_vnet_hdr_len and get_using_vnet_hdr callback
>> that make we get vnet_hdr_len easily.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>> include/net/net.h | 6 ++++++
>> net/net.c | 18 ++++++++++++++++++
>> 2 files changed, 24 insertions(+)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 99b28d5..402d913 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -57,7 +57,9 @@ typedef RxFilterInfo
>> *(QueryRxFilter)(NetClientState *);
>> typedef bool (HasUfo)(NetClientState *);
>> typedef bool (HasVnetHdr)(NetClientState *);
>> typedef bool (HasVnetHdrLen)(NetClientState *, int);
>> +typedef int (GetVnetHdrLen)(NetClientState *);
>> typedef void (UsingVnetHdr)(NetClientState *, bool);
>> +typedef bool (GetUsingVnetHdr)(NetClientState *);
>> typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
>> typedef void (SetVnetHdrLen)(NetClientState *, int);
>> typedef int (SetVnetLE)(NetClientState *, bool);
>> @@ -79,7 +81,9 @@ typedef struct NetClientInfo {
>> HasUfo *has_ufo;
>> HasVnetHdr *has_vnet_hdr;
>> HasVnetHdrLen *has_vnet_hdr_len;
>> + GetVnetHdrLen *get_vnet_hdr_len;
>> UsingVnetHdr *using_vnet_hdr;
>> + GetUsingVnetHdr *get_using_vnet_hdr;
>> SetOffload *set_offload;
>> SetVnetHdrLen *set_vnet_hdr_len;
>> SetVnetLE *set_vnet_le;
>> @@ -155,7 +159,9 @@ void qemu_format_nic_info_str(NetClientState *nc,
>> uint8_t macaddr[6]);
>> bool qemu_has_ufo(NetClientState *nc);
>> bool qemu_has_vnet_hdr(NetClientState *nc);
>> bool qemu_has_vnet_hdr_len(NetClientState *nc, int len);
>> +int qemu_get_vnet_hdr_len(NetClientState *nc);
>> void qemu_using_vnet_hdr(NetClientState *nc, bool enable);
>> +bool qemu_get_using_vnet_hdr(NetClientState *nc);
>> void qemu_set_offload(NetClientState *nc, int csum, int tso4, int
>> tso6,
>> int ecn, int ufo);
>> void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
>> diff --git a/net/net.c b/net/net.c
>> index 0ac3b9e..f69260f 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -466,6 +466,15 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc,
>> int len)
>> return nc->info->has_vnet_hdr_len(nc, len);
>> }
>> +int qemu_get_vnet_hdr_len(NetClientState *nc)
>> +{
>> + if (!nc || !nc->info->get_vnet_hdr_len) {
>> + return false;
>> + }
>> +
>> + return nc->info->get_vnet_hdr_len(nc);
>> +}
>> +
>> void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
>> {
>> if (!nc || !nc->info->using_vnet_hdr) {
>> @@ -475,6 +484,15 @@ void qemu_using_vnet_hdr(NetClientState *nc,
>> bool enable)
>> nc->info->using_vnet_hdr(nc, enable);
>> }
>> +bool qemu_get_using_vnet_hdr(NetClientState *nc)
>> +{
>> + if (!nc || !nc->info->get_using_vnet_hdr) {
>> + return false;
>> + }
>> +
>> + return nc->info->get_using_vnet_hdr(nc);
>> +}
>
> Looks like we can do this simply by:
>
> Introduce two common fields in NetClientState:
>
> bool using_vnet_hdr;
> int vnet_hdr_len;
>
> And set them during qemu_using_vnet_hdr() and qemu_set_vnet_hdr_len().
> Then we can query them directly without introducing any new callbacks.
Good idea~
I will implement in next version.
Thanks
Zhang Chen
>
> Thanks
>
>> +
>> void qemu_set_offload(NetClientState *nc, int csum, int tso4, int
>> tso6,
>> int ecn, int ufo)
>> {
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.
2017-05-02 5:47 ` Jason Wang
@ 2017-05-03 3:18 ` Zhang Chen
2017-05-03 10:19 ` Jason Wang
0 siblings, 1 reply; 28+ messages in thread
From: Zhang Chen @ 2017-05-03 3:18 UTC (permalink / raw)
To: Jason Wang, qemu devel
Cc: zhangchen.fnst, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
On 05/02/2017 12:47 PM, Jason Wang wrote:
>
>
> On 2017年04月28日 17:47, Zhang Chen wrote:
>> In this patch, we change the send packet format from
>> struct {int size; const uint8_t buf[];} to {int size; int
>> vnet_hdr_len; const uint8_t buf[];}.
>> make other module(like colo-compare) know how to parse net packet
>> correctly.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>> net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>> 1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 72fa7c2..bb9ecf3 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>> SocketReadState rs;
>> } MirrorState;
>> -static int filter_mirror_send(CharBackend *chr_out,
>> +static int filter_mirror_send(MirrorState *s,
>> const struct iovec *iov,
>> int iovcnt)
>> {
>> + NetFilterState *nf = NETFILTER(s);
>> int ret = 0;
>> ssize_t size = 0;
>> + ssize_t vnet_hdr_len;
>> uint32_t len = 0;
>> char *buf;
>> @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend
>> *chr_out,
>> }
>> len = htonl(size);
>> - ret = qemu_chr_fe_write_all(chr_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;
>> + }
>> +
>> + /*
>> + * We send vnet header len make other module(like colo-compare)
>> + * know how to parse net packet correctly.
>> + */
>> + if (qemu_get_using_vnet_hdr(nf->netdev)) {
>> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
>> + } else {
>> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
>> + }
>
> Any reason to query peer here?
That's depend on using NetClientState, If we using nf->netdev that need
to query,
Otherwise we query nf->netdev->peer, then we can get the real
vnet_hdr_len in my test.
Thanks
Zhang Chen
>
>> +
>> + 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;
>> }
>> buf = g_malloc(size);
>> iov_to_buf(iov, iovcnt, 0, buf, size);
>> - ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)buf, size);
>> + ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
>> g_free(buf);
>> if (ret != size) {
>> goto err;
>> @@ -141,7 +159,7 @@ static ssize_t
>> filter_mirror_receive_iov(NetFilterState *nf,
>> MirrorState *s = FILTER_MIRROR(nf);
>> int ret;
>> - ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
>> + ret = filter_mirror_send(s, iov, iovcnt);
>> if (ret) {
>> error_report("filter_mirror_send failed(%s)", strerror(-ret));
>> }
>> @@ -164,7 +182,7 @@ static ssize_t
>> filter_redirector_receive_iov(NetFilterState *nf,
>> int ret;
>> if (qemu_chr_fe_get_driver(&s->chr_out)) {
>> - ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
>> + ret = filter_mirror_send(s, iov, iovcnt);
>> if (ret) {
>> error_report("filter_mirror_send failed(%s)",
>> strerror(-ret));
>> }
>
> Do we need to strip vnet_hdr_len in redirector_to_filter() ?
>
> Thanks
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
2017-05-02 5:53 ` Jason Wang
@ 2017-05-03 3:43 ` Zhang Chen
2017-05-03 10:42 ` Jason Wang
0 siblings, 1 reply; 28+ messages in thread
From: Zhang Chen @ 2017-05-03 3:43 UTC (permalink / raw)
To: Jason Wang, qemu devel
Cc: zhangchen.fnst, zhanghailiang, weifuqiang, eddie . dong,
bian naimeng, Li Zhijian
On 05/02/2017 12:53 PM, Jason Wang wrote:
>
>
> On 2017年04月28日 17:47, Zhang Chen wrote:
>> Address Jason Wang's comments add vnet header length to SocketReadState.
>
> Instead of saying this, you can add "Suggested-by: Jason Wang
> <jasowang@redhat.com>" above your sign-off.
OK.
>
>> So we change net_fill_rstate() to read
>> struct {int size; int vnet_hdr_len; const uint8_t buf[];}.
>
> This makes me thinking about the backward compatibility. I think we'd
> better try to keep it as much as possible. E.g how about pack
> vnet_hdr_len into higher bits of size?
>
Do you means split uint32_t size to uint16_t size and uint16_t
vnet_hdr_len ?
If yes, we also can't keep compatibility with old version.
Old code send a uint32_t size, we read it as uint16_t size is always wrong.
Thanks
Zhang Chen
> Thanks
>
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>> include/net/net.h | 4 +++-
>> net/net.c | 24 ++++++++++++++++++++++--
>> 2 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 402d913..865cb98 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -115,9 +115,11 @@ typedef struct NICState {
>> } NICState;
>> struct SocketReadState {
>> - int state; /* 0 = getting length, 1 = getting data */
>> + /* 0 = getting length, 1 = getting vnet header length, 2 =
>> getting data */
>> + int state;
>> uint32_t index;
>> uint32_t packet_len;
>> + uint32_t vnet_hdr_len;
>> uint8_t buf[NET_BUFSIZE];
>> SocketReadStateFinalize *finalize;
>> };
>> diff --git a/net/net.c b/net/net.c
>> index f69260f..5a6b5ac 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1639,8 +1639,12 @@ int net_fill_rstate(SocketReadState *rs, const
>> uint8_t *buf, int size)
>> unsigned int l;
>> while (size > 0) {
>> - /* reassemble a packet from the network */
>> - switch (rs->state) { /* 0 = getting length, 1 = getting data */
>> + /* Reassemble a packet from the network.
>> + * 0 = getting length.
>> + * 1 = getting vnet header length.
>> + * 2 = getting data.
>> + */
>> + switch (rs->state) {
>> case 0:
>> l = 4 - rs->index;
>> if (l > size) {
>> @@ -1658,6 +1662,22 @@ int net_fill_rstate(SocketReadState *rs, const
>> uint8_t *buf, int size)
>> }
>> break;
>> case 1:
>> + l = 4 - rs->index;
>> + if (l > size) {
>> + l = size;
>> + }
>> + memcpy(rs->buf + rs->index, buf, l);
>> + buf += l;
>> + size -= l;
>> + rs->index += l;
>> + if (rs->index == 4) {
>> + /* got vnet header length */
>> + rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
>> + rs->index = 0;
>> + rs->state = 2;
>> + }
>> + break;
>> + case 2:
>> l = rs->packet_len - rs->index;
>> if (l > size) {
>> l = size;
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.
2017-05-03 3:18 ` Zhang Chen
@ 2017-05-03 10:19 ` Jason Wang
2017-05-05 8:44 ` Zhang Chen
0 siblings, 1 reply; 28+ messages in thread
From: Jason Wang @ 2017-05-03 10:19 UTC (permalink / raw)
To: Zhang Chen, qemu devel
Cc: zhanghailiang, Li Zhijian, weifuqiang, eddie . dong, bian naimeng
On 2017年05月03日 11:18, Zhang Chen wrote:
>
>
> On 05/02/2017 12:47 PM, Jason Wang wrote:
>>
>>
>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>> In this patch, we change the send packet format from
>>> struct {int size; const uint8_t buf[];} to {int size; int
>>> vnet_hdr_len; const uint8_t buf[];}.
>>> make other module(like colo-compare) know how to parse net packet
>>> correctly.
>>>
>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>> ---
>>> net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>>> 1 file changed, 23 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>> index 72fa7c2..bb9ecf3 100644
>>> --- a/net/filter-mirror.c
>>> +++ b/net/filter-mirror.c
>>> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>>> SocketReadState rs;
>>> } MirrorState;
>>> -static int filter_mirror_send(CharBackend *chr_out,
>>> +static int filter_mirror_send(MirrorState *s,
>>> const struct iovec *iov,
>>> int iovcnt)
>>> {
>>> + NetFilterState *nf = NETFILTER(s);
>>> int ret = 0;
>>> ssize_t size = 0;
>>> + ssize_t vnet_hdr_len;
>>> uint32_t len = 0;
>>> char *buf;
>>> @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend
>>> *chr_out,
>>> }
>>> len = htonl(size);
>>> - ret = qemu_chr_fe_write_all(chr_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;
>>> + }
>>> +
>>> + /*
>>> + * We send vnet header len make other module(like colo-compare)
>>> + * know how to parse net packet correctly.
>>> + */
>>> + if (qemu_get_using_vnet_hdr(nf->netdev)) {
>>> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
>>> + } else {
>>> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
>>> + }
>>
>> Any reason to query peer here?
>
> That's depend on using NetClientState, If we using nf->netdev that
> need to query,
> Otherwise we query nf->netdev->peer, then we can get the real
> vnet_hdr_len in my test.
>
> Thanks
> Zhang Chen
Confused, I think nf->netdev won't be a nic?
Thanks
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
2017-05-03 3:43 ` Zhang Chen
@ 2017-05-03 10:42 ` Jason Wang
2017-05-08 3:47 ` Zhang Chen
0 siblings, 1 reply; 28+ messages in thread
From: Jason Wang @ 2017-05-03 10:42 UTC (permalink / raw)
To: Zhang Chen, qemu devel
Cc: zhanghailiang, Li Zhijian, weifuqiang, eddie . dong, bian naimeng
On 2017年05月03日 11:43, Zhang Chen wrote:
>
>
> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>
>>
>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>> Address Jason Wang's comments add vnet header length to
>>> SocketReadState.
>>
>> Instead of saying this, you can add "Suggested-by: Jason Wang
>> <jasowang@redhat.com>" above your sign-off.
>
> OK.
>
>>
>>> So we change net_fill_rstate() to read
>>> struct {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>
>> This makes me thinking about the backward compatibility. I think we'd
>> better try to keep it as much as possible. E.g how about pack
>> vnet_hdr_len into higher bits of size?
>>
>
> Do you means split uint32_t size to uint16_t size and uint16_t
> vnet_hdr_len ?
> If yes, we also can't keep compatibility with old version.
> Old code send a uint32_t size, we read it as uint16_t size is always
> wrong.
>
> Thanks
> Zhang Chen
Consider it's unlikely to send or receive packet >= 64K, we can reuse
higher bits (e.g highest 8 bits). Then we can still read uint32_t and
then check its highest 8 bits. If it was zero, we know vnet header is
zero, if not it could be read as vnet header length.
Thanks
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.
2017-05-03 10:19 ` Jason Wang
@ 2017-05-05 8:44 ` Zhang Chen
2017-05-05 9:25 ` Jason Wang
0 siblings, 1 reply; 28+ messages in thread
From: Zhang Chen @ 2017-05-05 8:44 UTC (permalink / raw)
To: Jason Wang, qemu devel
Cc: zhangchen.fnst, zhanghailiang, Li Zhijian, weifuqiang,
eddie . dong, bian naimeng
On 05/03/2017 06:19 PM, Jason Wang wrote:
>
>
> On 2017年05月03日 11:18, Zhang Chen wrote:
>>
>>
>> On 05/02/2017 12:47 PM, Jason Wang wrote:
>>>
>>>
>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>> In this patch, we change the send packet format from
>>>> struct {int size; const uint8_t buf[];} to {int size; int
>>>> vnet_hdr_len; const uint8_t buf[];}.
>>>> make other module(like colo-compare) know how to parse net packet
>>>> correctly.
>>>>
>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>> ---
>>>> net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>>>> 1 file changed, 23 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>>> index 72fa7c2..bb9ecf3 100644
>>>> --- a/net/filter-mirror.c
>>>> +++ b/net/filter-mirror.c
>>>> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>>>> SocketReadState rs;
>>>> } MirrorState;
>>>> -static int filter_mirror_send(CharBackend *chr_out,
>>>> +static int filter_mirror_send(MirrorState *s,
>>>> const struct iovec *iov,
>>>> int iovcnt)
>>>> {
>>>> + NetFilterState *nf = NETFILTER(s);
>>>> int ret = 0;
>>>> ssize_t size = 0;
>>>> + ssize_t vnet_hdr_len;
>>>> uint32_t len = 0;
>>>> char *buf;
>>>> @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend
>>>> *chr_out,
>>>> }
>>>> len = htonl(size);
>>>> - ret = qemu_chr_fe_write_all(chr_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;
>>>> + }
>>>> +
>>>> + /*
>>>> + * We send vnet header len make other module(like colo-compare)
>>>> + * know how to parse net packet correctly.
>>>> + */
>>>> + if (qemu_get_using_vnet_hdr(nf->netdev)) {
>>>> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
>>>> + } else {
>>>> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
>>>> + }
>>>
>>> Any reason to query peer here?
>>
>> That's depend on using NetClientState, If we using nf->netdev that
>> need to query,
>> Otherwise we query nf->netdev->peer, then we can get the real
>> vnet_hdr_len in my test.
>>
>> Thanks
>> Zhang Chen
>
> Confused, I think nf->netdev won't be a nic?
I don't know whether I fully understand.
I think it's depend on the sender, we must query sender to get real
vnet_hdr_len ,
like that in filter.c:
if (sender == nf->netdev) {
/* This packet is sent by netdev itself */
direction = NET_FILTER_DIRECTION_TX;
} else {
direction = NET_FILTER_DIRECTION_RX;
}
Thanks
Zhang Chen
>
> Thanks
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.
2017-05-05 8:44 ` Zhang Chen
@ 2017-05-05 9:25 ` Jason Wang
2017-05-05 9:43 ` Zhang Chen
0 siblings, 1 reply; 28+ messages in thread
From: Jason Wang @ 2017-05-05 9:25 UTC (permalink / raw)
To: Zhang Chen, qemu devel
Cc: zhanghailiang, Li Zhijian, weifuqiang, eddie . dong, bian naimeng
On 2017年05月05日 16:44, Zhang Chen wrote:
>
>
> On 05/03/2017 06:19 PM, Jason Wang wrote:
>>
>>
>> On 2017年05月03日 11:18, Zhang Chen wrote:
>>>
>>>
>>> On 05/02/2017 12:47 PM, Jason Wang wrote:
>>>>
>>>>
>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>> In this patch, we change the send packet format from
>>>>> struct {int size; const uint8_t buf[];} to {int size; int
>>>>> vnet_hdr_len; const uint8_t buf[];}.
>>>>> make other module(like colo-compare) know how to parse net packet
>>>>> correctly.
>>>>>
>>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>>> ---
>>>>> net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>>>>> 1 file changed, 23 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>>>> index 72fa7c2..bb9ecf3 100644
>>>>> --- a/net/filter-mirror.c
>>>>> +++ b/net/filter-mirror.c
>>>>> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>>>>> SocketReadState rs;
>>>>> } MirrorState;
>>>>> -static int filter_mirror_send(CharBackend *chr_out,
>>>>> +static int filter_mirror_send(MirrorState *s,
>>>>> const struct iovec *iov,
>>>>> int iovcnt)
>>>>> {
>>>>> + NetFilterState *nf = NETFILTER(s);
>>>>> int ret = 0;
>>>>> ssize_t size = 0;
>>>>> + ssize_t vnet_hdr_len;
>>>>> uint32_t len = 0;
>>>>> char *buf;
>>>>> @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend
>>>>> *chr_out,
>>>>> }
>>>>> len = htonl(size);
>>>>> - ret = qemu_chr_fe_write_all(chr_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;
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * We send vnet header len make other module(like colo-compare)
>>>>> + * know how to parse net packet correctly.
>>>>> + */
>>>>> + if (qemu_get_using_vnet_hdr(nf->netdev)) {
>>>>> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
>>>>> + } else {
>>>>> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
>>>>> + }
>>>>
>>>> Any reason to query peer here?
>>>
>>> That's depend on using NetClientState, If we using nf->netdev that
>>> need to query,
>>> Otherwise we query nf->netdev->peer, then we can get the real
>>> vnet_hdr_len in my test.
>>>
>>> Thanks
>>> Zhang Chen
>>
>> Confused, I think nf->netdev won't be a nic?
>
> I don't know whether I fully understand.
> I think it's depend on the sender, we must query sender to get real
> vnet_hdr_len ,
> like that in filter.c:
>
> if (sender == nf->netdev) {
> /* This packet is sent by netdev itself */
> direction = NET_FILTER_DIRECTION_TX;
> } else {
> direction = NET_FILTER_DIRECTION_RX;
> }
>
> Thanks
> Zhang Chen
The problem is nf->netdev->peer should be a nic. But we don't care about
its vnet_hdr_len. Take virtio-net as an example, we only care about
host_hdr_len, since guest will strip the part that host does not care.
Thanks
>
>
>>
>> Thanks
>>
>>
>> .
>>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support.
2017-05-05 9:25 ` Jason Wang
@ 2017-05-05 9:43 ` Zhang Chen
0 siblings, 0 replies; 28+ messages in thread
From: Zhang Chen @ 2017-05-05 9:43 UTC (permalink / raw)
To: Jason Wang, qemu devel
Cc: zhangchen.fnst, zhanghailiang, Li Zhijian, weifuqiang,
eddie . dong, bian naimeng
On 05/05/2017 05:25 PM, Jason Wang wrote:
>
>
> On 2017年05月05日 16:44, Zhang Chen wrote:
>>
>>
>> On 05/03/2017 06:19 PM, Jason Wang wrote:
>>>
>>>
>>> On 2017年05月03日 11:18, Zhang Chen wrote:
>>>>
>>>>
>>>> On 05/02/2017 12:47 PM, Jason Wang wrote:
>>>>>
>>>>>
>>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>>> In this patch, we change the send packet format from
>>>>>> struct {int size; const uint8_t buf[];} to {int size; int
>>>>>> vnet_hdr_len; const uint8_t buf[];}.
>>>>>> make other module(like colo-compare) know how to parse net packet
>>>>>> correctly.
>>>>>>
>>>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>>>> ---
>>>>>> net/filter-mirror.c | 28 +++++++++++++++++++++++-----
>>>>>> 1 file changed, 23 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>>>>> index 72fa7c2..bb9ecf3 100644
>>>>>> --- a/net/filter-mirror.c
>>>>>> +++ b/net/filter-mirror.c
>>>>>> @@ -43,12 +43,14 @@ typedef struct MirrorState {
>>>>>> SocketReadState rs;
>>>>>> } MirrorState;
>>>>>> -static int filter_mirror_send(CharBackend *chr_out,
>>>>>> +static int filter_mirror_send(MirrorState *s,
>>>>>> const struct iovec *iov,
>>>>>> int iovcnt)
>>>>>> {
>>>>>> + NetFilterState *nf = NETFILTER(s);
>>>>>> int ret = 0;
>>>>>> ssize_t size = 0;
>>>>>> + ssize_t vnet_hdr_len;
>>>>>> uint32_t len = 0;
>>>>>> char *buf;
>>>>>> @@ -58,14 +60,30 @@ static int filter_mirror_send(CharBackend
>>>>>> *chr_out,
>>>>>> }
>>>>>> len = htonl(size);
>>>>>> - ret = qemu_chr_fe_write_all(chr_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;
>>>>>> + }
>>>>>> +
>>>>>> + /*
>>>>>> + * We send vnet header len make other module(like colo-compare)
>>>>>> + * know how to parse net packet correctly.
>>>>>> + */
>>>>>> + if (qemu_get_using_vnet_hdr(nf->netdev)) {
>>>>>> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev);
>>>>>> + } else {
>>>>>> + vnet_hdr_len = qemu_get_vnet_hdr_len(nf->netdev->peer);
>>>>>> + }
>>>>>
>>>>> Any reason to query peer here?
>>>>
>>>> That's depend on using NetClientState, If we using nf->netdev that
>>>> need to query,
>>>> Otherwise we query nf->netdev->peer, then we can get the real
>>>> vnet_hdr_len in my test.
>>>>
>>>> Thanks
>>>> Zhang Chen
>>>
>>> Confused, I think nf->netdev won't be a nic?
>>
>> I don't know whether I fully understand.
>> I think it's depend on the sender, we must query sender to get real
>> vnet_hdr_len ,
>> like that in filter.c:
>>
>> if (sender == nf->netdev) {
>> /* This packet is sent by netdev itself */
>> direction = NET_FILTER_DIRECTION_TX;
>> } else {
>> direction = NET_FILTER_DIRECTION_RX;
>> }
>>
>> Thanks
>> Zhang Chen
>
> The problem is nf->netdev->peer should be a nic. But we don't care
> about its vnet_hdr_len. Take virtio-net as an example, we only care
> about host_hdr_len, since guest will strip the part that host does not
> care.
Yes, you are right. In anytime, nf->netdev and nf->netdev->peer both
have a vnet_hdr_len,
Here we just find out which is we need. When filter set RX or TX that
the real vnet_hdr_len are different.
We need query different netdev to get in my test.
Thanks
Zhang Chen
>
> Thanks
>
>>
>>
>>>
>>> Thanks
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
2017-05-03 10:42 ` Jason Wang
@ 2017-05-08 3:47 ` Zhang Chen
2017-05-09 2:40 ` Jason Wang
0 siblings, 1 reply; 28+ messages in thread
From: Zhang Chen @ 2017-05-08 3:47 UTC (permalink / raw)
To: Jason Wang, qemu devel
Cc: zhangchen.fnst, zhanghailiang, Li Zhijian, weifuqiang,
eddie . dong, bian naimeng
On 05/03/2017 06:42 PM, Jason Wang wrote:
>
>
> On 2017年05月03日 11:43, Zhang Chen wrote:
>>
>>
>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>
>>>
>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>> Address Jason Wang's comments add vnet header length to
>>>> SocketReadState.
>>>
>>> Instead of saying this, you can add "Suggested-by: Jason Wang
>>> <jasowang@redhat.com>" above your sign-off.
>>
>> OK.
>>
>>>
>>>> So we change net_fill_rstate() to read
>>>> struct {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>
>>> This makes me thinking about the backward compatibility. I think
>>> we'd better try to keep it as much as possible. E.g how about pack
>>> vnet_hdr_len into higher bits of size?
>>>
>>
>> Do you means split uint32_t size to uint16_t size and uint16_t
>> vnet_hdr_len ?
>> If yes, we also can't keep compatibility with old version.
>> Old code send a uint32_t size, we read it as uint16_t size is always
>> wrong.
>>
>> Thanks
>> Zhang Chen
>
> Consider it's unlikely to send or receive packet >= 64K, we can reuse
> higher bits (e.g highest 8 bits). Then we can still read uint32_t and
> then check its highest 8 bits. If it was zero, we know vnet header is
> zero, if not it could be read as vnet header length.
I got your point, but in this way, packet size must < 64K, if size >=64K
we still can't maintain the backward compatibility,
For the packet sender that didn't know the potential packet size limit,
I think we should make code explicitly declared like
currently code. Otherwise we will make other people confused and make
code difficult to maintain.
Thanks
Zhang Chen
>
> Thanks
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
2017-05-08 3:47 ` Zhang Chen
@ 2017-05-09 2:40 ` Jason Wang
2017-05-09 4:03 ` Zhang Chen
0 siblings, 1 reply; 28+ messages in thread
From: Jason Wang @ 2017-05-09 2:40 UTC (permalink / raw)
To: Zhang Chen, qemu devel
Cc: zhanghailiang, Li Zhijian, weifuqiang, eddie . dong, bian naimeng
On 2017年05月08日 11:47, Zhang Chen wrote:
>
>
> On 05/03/2017 06:42 PM, Jason Wang wrote:
>>
>>
>> On 2017年05月03日 11:43, Zhang Chen wrote:
>>>
>>>
>>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>>
>>>>
>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>> Address Jason Wang's comments add vnet header length to
>>>>> SocketReadState.
>>>>
>>>> Instead of saying this, you can add "Suggested-by: Jason Wang
>>>> <jasowang@redhat.com>" above your sign-off.
>>>
>>> OK.
>>>
>>>>
>>>>> So we change net_fill_rstate() to read
>>>>> struct {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>>
>>>> This makes me thinking about the backward compatibility. I think
>>>> we'd better try to keep it as much as possible. E.g how about pack
>>>> vnet_hdr_len into higher bits of size?
>>>>
>>>
>>> Do you means split uint32_t size to uint16_t size and uint16_t
>>> vnet_hdr_len ?
>>> If yes, we also can't keep compatibility with old version.
>>> Old code send a uint32_t size, we read it as uint16_t size is always
>>> wrong.
>>>
>>> Thanks
>>> Zhang Chen
>>
>> Consider it's unlikely to send or receive packet >= 64K, we can reuse
>> higher bits (e.g highest 8 bits). Then we can still read uint32_t and
>> then check its highest 8 bits. If it was zero, we know vnet header is
>> zero, if not it could be read as vnet header length.
>
> I got your point, but in this way, packet size must < 64K, if size
> >=64K we still can't maintain the backward compatibility,
> For the packet sender that didn't know the potential packet size
> limit, I think we should make code explicitly declared like
> currently code. Otherwise we will make other people confused and make
> code difficult to maintain.
>
> Thanks
> Zhang Chen
>
Yes, this is an possible issue in the future. Rethink about this, what
if we just compare vnet header? Is there any issue of doing this?
(Sorry, I remember this is a reason, but forget now).
Thanks
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
2017-05-09 2:40 ` Jason Wang
@ 2017-05-09 4:03 ` Zhang Chen
2017-05-09 5:36 ` Jason Wang
0 siblings, 1 reply; 28+ messages in thread
From: Zhang Chen @ 2017-05-09 4:03 UTC (permalink / raw)
To: Jason Wang, qemu devel
Cc: zhangchen.fnst, zhanghailiang, Li Zhijian, weifuqiang,
eddie . dong, bian naimeng
On 05/09/2017 10:40 AM, Jason Wang wrote:
>
>
> On 2017年05月08日 11:47, Zhang Chen wrote:
>>
>>
>> On 05/03/2017 06:42 PM, Jason Wang wrote:
>>>
>>>
>>> On 2017年05月03日 11:43, Zhang Chen wrote:
>>>>
>>>>
>>>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>>>
>>>>>
>>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>>> Address Jason Wang's comments add vnet header length to
>>>>>> SocketReadState.
>>>>>
>>>>> Instead of saying this, you can add "Suggested-by: Jason Wang
>>>>> <jasowang@redhat.com>" above your sign-off.
>>>>
>>>> OK.
>>>>
>>>>>
>>>>>> So we change net_fill_rstate() to read
>>>>>> struct {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>>>
>>>>> This makes me thinking about the backward compatibility. I think
>>>>> we'd better try to keep it as much as possible. E.g how about pack
>>>>> vnet_hdr_len into higher bits of size?
>>>>>
>>>>
>>>> Do you means split uint32_t size to uint16_t size and uint16_t
>>>> vnet_hdr_len ?
>>>> If yes, we also can't keep compatibility with old version.
>>>> Old code send a uint32_t size, we read it as uint16_t size is
>>>> always wrong.
>>>>
>>>> Thanks
>>>> Zhang Chen
>>>
>>> Consider it's unlikely to send or receive packet >= 64K, we can
>>> reuse higher bits (e.g highest 8 bits). Then we can still read
>>> uint32_t and then check its highest 8 bits. If it was zero, we know
>>> vnet header is zero, if not it could be read as vnet header length.
>>
>> I got your point, but in this way, packet size must < 64K, if size
>> >=64K we still can't maintain the backward compatibility,
>> For the packet sender that didn't know the potential packet size
>> limit, I think we should make code explicitly declared like
>> currently code. Otherwise we will make other people confused and make
>> code difficult to maintain.
>>
>> Thanks
>> Zhang Chen
>>
>
> Yes, this is an possible issue in the future. Rethink about this, what
> if we just compare vnet header? Is there any issue of doing this?
> (Sorry, I remember this is a reason, but forget now).
Yes, we can compare all packet with vnet header, the compare performance
is very low. but we can't parse raw packet to tcp/udp/icmp packet,
because we didn't know the vnet_hdr_len as the offset.
Thanks
Zhang Chen
>
> Thanks
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
2017-05-09 4:03 ` Zhang Chen
@ 2017-05-09 5:36 ` Jason Wang
2017-05-09 6:59 ` Zhang Chen
0 siblings, 1 reply; 28+ messages in thread
From: Jason Wang @ 2017-05-09 5:36 UTC (permalink / raw)
To: Zhang Chen, qemu devel
Cc: zhanghailiang, Li Zhijian, weifuqiang, eddie . dong, bian naimeng
On 2017年05月09日 12:03, Zhang Chen wrote:
>
>
> On 05/09/2017 10:40 AM, Jason Wang wrote:
>>
>>
>> On 2017年05月08日 11:47, Zhang Chen wrote:
>>>
>>>
>>> On 05/03/2017 06:42 PM, Jason Wang wrote:
>>>>
>>>>
>>>> On 2017年05月03日 11:43, Zhang Chen wrote:
>>>>>
>>>>>
>>>>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>>>>
>>>>>>
>>>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>>>> Address Jason Wang's comments add vnet header length to
>>>>>>> SocketReadState.
>>>>>>
>>>>>> Instead of saying this, you can add "Suggested-by: Jason Wang
>>>>>> <jasowang@redhat.com>" above your sign-off.
>>>>>
>>>>> OK.
>>>>>
>>>>>>
>>>>>>> So we change net_fill_rstate() to read
>>>>>>> struct {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>>>>
>>>>>> This makes me thinking about the backward compatibility. I think
>>>>>> we'd better try to keep it as much as possible. E.g how about
>>>>>> pack vnet_hdr_len into higher bits of size?
>>>>>>
>>>>>
>>>>> Do you means split uint32_t size to uint16_t size and uint16_t
>>>>> vnet_hdr_len ?
>>>>> If yes, we also can't keep compatibility with old version.
>>>>> Old code send a uint32_t size, we read it as uint16_t size is
>>>>> always wrong.
>>>>>
>>>>> Thanks
>>>>> Zhang Chen
>>>>
>>>> Consider it's unlikely to send or receive packet >= 64K, we can
>>>> reuse higher bits (e.g highest 8 bits). Then we can still read
>>>> uint32_t and then check its highest 8 bits. If it was zero, we know
>>>> vnet header is zero, if not it could be read as vnet header length.
>>>
>>> I got your point, but in this way, packet size must < 64K, if size
>>> >=64K we still can't maintain the backward compatibility,
>>> For the packet sender that didn't know the potential packet size
>>> limit, I think we should make code explicitly declared like
>>> currently code. Otherwise we will make other people confused and
>>> make code difficult to maintain.
>>>
>>> Thanks
>>> Zhang Chen
>>>
>>
>> Yes, this is an possible issue in the future. Rethink about this,
>> what if we just compare vnet header? Is there any issue of doing
>> this? (Sorry, I remember this is a reason, but forget now).
>
> Yes, we can compare all packet with vnet header, the compare
> performance is very low. but we can't parse raw packet to tcp/udp/icmp
> packet, because we didn't know the vnet_hdr_len as the offset.
>
> Thanks
> Zhang Chen
Aha, so I think it's time to use the new format but:
- probably need a new option to enable this support for filter
- let's don't touch socket backend, and make it still can work with
filters whose vnet_hder is off
Thanks
>
>>
>> Thanks
>>
>>
>> .
>>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
2017-05-09 5:36 ` Jason Wang
@ 2017-05-09 6:59 ` Zhang Chen
2017-05-09 7:36 ` Jason Wang
0 siblings, 1 reply; 28+ messages in thread
From: Zhang Chen @ 2017-05-09 6:59 UTC (permalink / raw)
To: Jason Wang, qemu devel
Cc: zhangchen.fnst, zhanghailiang, Li Zhijian, weifuqiang,
eddie . dong, bian naimeng
On 05/09/2017 01:36 PM, Jason Wang wrote:
>
>
> On 2017年05月09日 12:03, Zhang Chen wrote:
>>
>>
>> On 05/09/2017 10:40 AM, Jason Wang wrote:
>>>
>>>
>>> On 2017年05月08日 11:47, Zhang Chen wrote:
>>>>
>>>>
>>>> On 05/03/2017 06:42 PM, Jason Wang wrote:
>>>>>
>>>>>
>>>>> On 2017年05月03日 11:43, Zhang Chen wrote:
>>>>>>
>>>>>>
>>>>>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>>>>> Address Jason Wang's comments add vnet header length to
>>>>>>>> SocketReadState.
>>>>>>>
>>>>>>> Instead of saying this, you can add "Suggested-by: Jason Wang
>>>>>>> <jasowang@redhat.com>" above your sign-off.
>>>>>>
>>>>>> OK.
>>>>>>
>>>>>>>
>>>>>>>> So we change net_fill_rstate() to read
>>>>>>>> struct {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>>>>>
>>>>>>> This makes me thinking about the backward compatibility. I think
>>>>>>> we'd better try to keep it as much as possible. E.g how about
>>>>>>> pack vnet_hdr_len into higher bits of size?
>>>>>>>
>>>>>>
>>>>>> Do you means split uint32_t size to uint16_t size and uint16_t
>>>>>> vnet_hdr_len ?
>>>>>> If yes, we also can't keep compatibility with old version.
>>>>>> Old code send a uint32_t size, we read it as uint16_t size is
>>>>>> always wrong.
>>>>>>
>>>>>> Thanks
>>>>>> Zhang Chen
>>>>>
>>>>> Consider it's unlikely to send or receive packet >= 64K, we can
>>>>> reuse higher bits (e.g highest 8 bits). Then we can still read
>>>>> uint32_t and then check its highest 8 bits. If it was zero, we
>>>>> know vnet header is zero, if not it could be read as vnet header
>>>>> length.
>>>>
>>>> I got your point, but in this way, packet size must < 64K, if size
>>>> >=64K we still can't maintain the backward compatibility,
>>>> For the packet sender that didn't know the potential packet size
>>>> limit, I think we should make code explicitly declared like
>>>> currently code. Otherwise we will make other people confused and
>>>> make code difficult to maintain.
>>>>
>>>> Thanks
>>>> Zhang Chen
>>>>
>>>
>>> Yes, this is an possible issue in the future. Rethink about this,
>>> what if we just compare vnet header? Is there any issue of doing
>>> this? (Sorry, I remember this is a reason, but forget now).
>>
>> Yes, we can compare all packet with vnet header, the compare
>> performance is very low. but we can't parse raw packet to
>> tcp/udp/icmp packet, because we didn't know the vnet_hdr_len as the
>> offset.
>>
>> Thanks
>> Zhang Chen
>
> Aha, so I think it's time to use the new format but:
>
> - probably need a new option to enable this support for filter
Do you means we should add the new option for every filter object like that:
Now:
-object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0
Feature:
-object
filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,has_vnet_hdr_len=on
And colo-compare also need this option to get the vnet_hdr_len.
> - let's don't touch socket backend, and make it still can work with
> filters whose vnet_hder is off
OK, Maybe we can add a new variable in net_fill_rstate().
Thanks
Zhang Chen
>
> Thanks
>
>>
>>>
>>> Thanks
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState
2017-05-09 6:59 ` Zhang Chen
@ 2017-05-09 7:36 ` Jason Wang
0 siblings, 0 replies; 28+ messages in thread
From: Jason Wang @ 2017-05-09 7:36 UTC (permalink / raw)
To: Zhang Chen, qemu devel
Cc: zhanghailiang, Li Zhijian, weifuqiang, eddie . dong, bian naimeng
On 2017年05月09日 14:59, Zhang Chen wrote:
>
>
> On 05/09/2017 01:36 PM, Jason Wang wrote:
>>
>>
>> On 2017年05月09日 12:03, Zhang Chen wrote:
>>>
>>>
>>> On 05/09/2017 10:40 AM, Jason Wang wrote:
>>>>
>>>>
>>>> On 2017年05月08日 11:47, Zhang Chen wrote:
>>>>>
>>>>>
>>>>> On 05/03/2017 06:42 PM, Jason Wang wrote:
>>>>>>
>>>>>>
>>>>>> On 2017年05月03日 11:43, Zhang Chen wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 05/02/2017 12:53 PM, Jason Wang wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2017年04月28日 17:47, Zhang Chen wrote:
>>>>>>>>> Address Jason Wang's comments add vnet header length to
>>>>>>>>> SocketReadState.
>>>>>>>>
>>>>>>>> Instead of saying this, you can add "Suggested-by: Jason Wang
>>>>>>>> <jasowang@redhat.com>" above your sign-off.
>>>>>>>
>>>>>>> OK.
>>>>>>>
>>>>>>>>
>>>>>>>>> So we change net_fill_rstate() to read
>>>>>>>>> struct {int size; int vnet_hdr_len; const uint8_t buf[];}.
>>>>>>>>
>>>>>>>> This makes me thinking about the backward compatibility. I
>>>>>>>> think we'd better try to keep it as much as possible. E.g how
>>>>>>>> about pack vnet_hdr_len into higher bits of size?
>>>>>>>>
>>>>>>>
>>>>>>> Do you means split uint32_t size to uint16_t size and uint16_t
>>>>>>> vnet_hdr_len ?
>>>>>>> If yes, we also can't keep compatibility with old version.
>>>>>>> Old code send a uint32_t size, we read it as uint16_t size is
>>>>>>> always wrong.
>>>>>>>
>>>>>>> Thanks
>>>>>>> Zhang Chen
>>>>>>
>>>>>> Consider it's unlikely to send or receive packet >= 64K, we can
>>>>>> reuse higher bits (e.g highest 8 bits). Then we can still read
>>>>>> uint32_t and then check its highest 8 bits. If it was zero, we
>>>>>> know vnet header is zero, if not it could be read as vnet header
>>>>>> length.
>>>>>
>>>>> I got your point, but in this way, packet size must < 64K, if size
>>>>> >=64K we still can't maintain the backward compatibility,
>>>>> For the packet sender that didn't know the potential packet size
>>>>> limit, I think we should make code explicitly declared like
>>>>> currently code. Otherwise we will make other people confused and
>>>>> make code difficult to maintain.
>>>>>
>>>>> Thanks
>>>>> Zhang Chen
>>>>>
>>>>
>>>> Yes, this is an possible issue in the future. Rethink about this,
>>>> what if we just compare vnet header? Is there any issue of doing
>>>> this? (Sorry, I remember this is a reason, but forget now).
>>>
>>> Yes, we can compare all packet with vnet header, the compare
>>> performance is very low. but we can't parse raw packet to
>>> tcp/udp/icmp packet, because we didn't know the vnet_hdr_len as the
>>> offset.
>>>
>>> Thanks
>>> Zhang Chen
>>
>> Aha, so I think it's time to use the new format but:
>>
>> - probably need a new option to enable this support for filter
>
> Do you means we should add the new option for every filter object like
> that:
> Now:
> -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0
> Feature:
> -object
> filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,has_vnet_hdr_len=on
>
> And colo-compare also need this option to get the vnet_hdr_len.
Yes, and you can use make it short like "vnet_hdr".
>
>
>> - let's don't touch socket backend, and make it still can work with
>> filters whose vnet_hder is off
>
> OK, Maybe we can add a new variable in net_fill_rstate().
Right.
Thanks
>
> Thanks
> Zhang Chen
>
>>
>> Thanks
>>
>>>
>>>>
>>>> Thanks
>>>>
>>>>
>>>> .
>>>>
>>>
>>
>>
>>
>> .
>>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2017-05-09 7:36 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-28 9:47 [Qemu-devel] [PATCH V3 00/10] Add COLO-proxy virtio-net support Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 01/10] net: Add vnet_hdr_len related callback in NetClientInfo Zhang Chen
2017-05-02 5:46 ` Jason Wang
2017-05-03 3:08 ` Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 02/10] net/tap.c: Add tap_get_vnet_hdr_len and tap_get_using_vnet_hdr function Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 03/10] net/netmap.c: Add netmap_get_vnet_hdr_len function Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 04/10] net/filter-mirror.c: Add filter-mirror and filter-redirector vnet support Zhang Chen
2017-05-02 5:47 ` Jason Wang
2017-05-03 3:18 ` Zhang Chen
2017-05-03 10:19 ` Jason Wang
2017-05-05 8:44 ` Zhang Chen
2017-05-05 9:25 ` Jason Wang
2017-05-05 9:43 ` Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 05/10] net/net.c: Add vnet header length to SocketReadState Zhang Chen
2017-05-02 5:53 ` Jason Wang
2017-05-03 3:43 ` Zhang Chen
2017-05-03 10:42 ` Jason Wang
2017-05-08 3:47 ` Zhang Chen
2017-05-09 2:40 ` Jason Wang
2017-05-09 4:03 ` Zhang Chen
2017-05-09 5:36 ` Jason Wang
2017-05-09 6:59 ` Zhang Chen
2017-05-09 7:36 ` Jason Wang
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 06/10] tests/e1000e-test.c : change e1000e test case send data format Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 07/10] tests/virtio-net-test.c : change virtio-net test case iov " Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 08/10] net/colo-compare.c: Make colo-compare support vnet_hdr_len Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 09/10] net/colo.c: Add vnet packet parse feature in colo-proxy Zhang Chen
2017-04-28 9:47 ` [Qemu-devel] [PATCH V3 10/10] net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare Zhang Chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.