| Message ID | 1493372840-24551-5-git-send-email-zhangchen.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
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
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 > > > > . >
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
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 > > > . >
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 >> >> >> . >> >
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 >>> >>> >>> . >>> >> > > > > . >
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)); }
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(-)