| Message ID | 1442405768-23019-5-git-send-email-yanghy@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 09/16/2015 08:16 PM, Yang Hongyang wrote: > qemu_deliver_packet_iov already have the compat delivery, we > can drop qemu_deliver_packet. > > Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> > --- > include/net/net.h | 5 ----- > net/net.c | 40 +++++++--------------------------------- > net/queue.c | 6 +++++- > 3 files changed, 12 insertions(+), 39 deletions(-) > > diff --git a/include/net/net.h b/include/net/net.h > index 36e5fab..7af3e15 100644 > --- a/include/net/net.h > +++ b/include/net/net.h > @@ -152,11 +152,6 @@ void qemu_check_nic_model(NICInfo *nd, const char *model); > int qemu_find_nic_model(NICInfo *nd, const char * const *models, > const char *default_model); > > -ssize_t qemu_deliver_packet(NetClientState *sender, > - unsigned flags, > - const uint8_t *data, > - size_t size, > - void *opaque); > ssize_t qemu_deliver_packet_iov(NetClientState *sender, > unsigned flags, > const struct iovec *iov, > diff --git a/net/net.c b/net/net.c > index ad37419..ca35b27 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -597,36 +597,6 @@ static ssize_t filter_receive(NetClientState *nc, NetFilterChain chain, > return filter_receive_iov(nc, chain, sender, flags, &iov, 1, sent_cb); > } > > -ssize_t qemu_deliver_packet(NetClientState *sender, > - unsigned flags, > - const uint8_t *data, > - size_t size, > - void *opaque) > -{ > - NetClientState *nc = opaque; > - ssize_t ret; > - > - if (nc->link_down) { > - return size; > - } > - > - if (nc->receive_disabled) { > - return 0; > - } > - > - if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { > - ret = nc->info->receive_raw(nc, data, size); > - } else { > - ret = nc->info->receive(nc, data, size); > - } > - > - if (ret == 0) { > - nc->receive_disabled = 1; > - } > - > - return ret; > -} > - > void qemu_purge_queued_packets(NetClientState *nc) > { > if (!nc->peer) { > @@ -717,14 +687,18 @@ ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size) > } > > static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov, > - int iovcnt) > + int iovcnt, unsigned flags) > { > uint8_t buffer[NET_BUFSIZE]; > size_t offset; > > offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer)); > > - return nc->info->receive(nc, buffer, offset); > + if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { > + return nc->info->receive_raw(nc, buffer, offset); > + } else { > + return nc->info->receive(nc, buffer, offset); > + } Then for net clients that doesn't support receive_iov, an extra memcpy is introduced. This seems unnecessary. And this patch looks independent, so please drop this patch from the series (This can also help to reduce the iteration). I think we may need this after all net clients support receive_iov(). > } > > ssize_t qemu_deliver_packet_iov(NetClientState *sender, > @@ -747,7 +721,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender, > if (nc->info->receive_iov) { > ret = nc->info->receive_iov(nc, iov, iovcnt); > } else { > - ret = nc_sendv_compat(nc, iov, iovcnt); > + ret = nc_sendv_compat(nc, iov, iovcnt, flags); > } > > if (ret == 0) { > diff --git a/net/queue.c b/net/queue.c > index ebbe2bb..cf8db3a 100644 > --- a/net/queue.c > +++ b/net/queue.c > @@ -152,9 +152,13 @@ static ssize_t qemu_net_queue_deliver(NetQueue *queue, > size_t size) > { > ssize_t ret = -1; > + struct iovec iov = { > + .iov_base = (void *)data, > + .iov_len = size > + }; > > queue->delivering = 1; > - ret = qemu_deliver_packet(sender, flags, data, size, queue->opaque); > + ret = qemu_deliver_packet_iov(sender, flags, &iov, 1, queue->opaque); > queue->delivering = 0; > > return ret;
Hi Jason, Thanks for the review, On 09/22/2015 03:30 PM, Jason Wang wrote: > > > On 09/16/2015 08:16 PM, Yang Hongyang wrote: >> qemu_deliver_packet_iov already have the compat delivery, we >> can drop qemu_deliver_packet. >> >> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> >> --- >> include/net/net.h | 5 ----- >> net/net.c | 40 +++++++--------------------------------- >> net/queue.c | 6 +++++- >> 3 files changed, 12 insertions(+), 39 deletions(-) >> >> diff --git a/include/net/net.h b/include/net/net.h >> index 36e5fab..7af3e15 100644 >> --- a/include/net/net.h >> +++ b/include/net/net.h >> @@ -152,11 +152,6 @@ void qemu_check_nic_model(NICInfo *nd, const char *model); >> int qemu_find_nic_model(NICInfo *nd, const char * const *models, >> const char *default_model); >> >> -ssize_t qemu_deliver_packet(NetClientState *sender, >> - unsigned flags, >> - const uint8_t *data, >> - size_t size, >> - void *opaque); >> ssize_t qemu_deliver_packet_iov(NetClientState *sender, >> unsigned flags, >> const struct iovec *iov, >> diff --git a/net/net.c b/net/net.c >> index ad37419..ca35b27 100644 >> --- a/net/net.c >> +++ b/net/net.c >> @@ -597,36 +597,6 @@ static ssize_t filter_receive(NetClientState *nc, NetFilterChain chain, >> return filter_receive_iov(nc, chain, sender, flags, &iov, 1, sent_cb); >> } >> >> -ssize_t qemu_deliver_packet(NetClientState *sender, >> - unsigned flags, >> - const uint8_t *data, >> - size_t size, >> - void *opaque) >> -{ >> - NetClientState *nc = opaque; >> - ssize_t ret; >> - >> - if (nc->link_down) { >> - return size; >> - } >> - >> - if (nc->receive_disabled) { >> - return 0; >> - } >> - >> - if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { >> - ret = nc->info->receive_raw(nc, data, size); >> - } else { >> - ret = nc->info->receive(nc, data, size); >> - } >> - >> - if (ret == 0) { >> - nc->receive_disabled = 1; >> - } >> - >> - return ret; >> -} >> - >> void qemu_purge_queued_packets(NetClientState *nc) >> { >> if (!nc->peer) { >> @@ -717,14 +687,18 @@ ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size) >> } >> >> static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov, >> - int iovcnt) >> + int iovcnt, unsigned flags) >> { >> uint8_t buffer[NET_BUFSIZE]; >> size_t offset; >> >> offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer)); >> >> - return nc->info->receive(nc, buffer, offset); >> + if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { >> + return nc->info->receive_raw(nc, buffer, offset); >> + } else { >> + return nc->info->receive(nc, buffer, offset); >> + } > > Then for net clients that doesn't support receive_iov, an extra memcpy > is introduced. This seems unnecessary. And this patch looks independent, > so please drop this patch from the series (This can also help to reduce > the iteration). I think we may need this after all net clients support > receive_iov(). This patch is required by the next patch which introduce a +/* Returns: + * >0 - success + * 0 - queue packet for future redelivery + * <0 - failure (discard packet) + */ +typedef ssize_t (NetQueueDeliverFunc)(NetClientState *sender, + unsigned flags, + const struct iovec *iov, + int iovcnt, + void *opaque); If we drop this patch, we will need to introduce 2 deliver func, which seems not clean and simple. > > >> } >> >> ssize_t qemu_deliver_packet_iov(NetClientState *sender, >> @@ -747,7 +721,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender, >> if (nc->info->receive_iov) { >> ret = nc->info->receive_iov(nc, iov, iovcnt); >> } else { >> - ret = nc_sendv_compat(nc, iov, iovcnt); >> + ret = nc_sendv_compat(nc, iov, iovcnt, flags); >> } >> >> if (ret == 0) { >> diff --git a/net/queue.c b/net/queue.c >> index ebbe2bb..cf8db3a 100644 >> --- a/net/queue.c >> +++ b/net/queue.c >> @@ -152,9 +152,13 @@ static ssize_t qemu_net_queue_deliver(NetQueue *queue, >> size_t size) >> { >> ssize_t ret = -1; >> + struct iovec iov = { >> + .iov_base = (void *)data, >> + .iov_len = size >> + }; >> >> queue->delivering = 1; >> - ret = qemu_deliver_packet(sender, flags, data, size, queue->opaque); >> + ret = qemu_deliver_packet_iov(sender, flags, &iov, 1, queue->opaque); >> queue->delivering = 0; >> >> return ret; > > > . >
On 09/22/2015 03:44 PM, Yang Hongyang wrote: > Hi Jason, > > Thanks for the review, > > On 09/22/2015 03:30 PM, Jason Wang wrote: >> >> >> On 09/16/2015 08:16 PM, Yang Hongyang wrote: >>> qemu_deliver_packet_iov already have the compat delivery, we >>> can drop qemu_deliver_packet. >>> >>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> >>> --- >>> include/net/net.h | 5 ----- >>> net/net.c | 40 +++++++--------------------------------- >>> net/queue.c | 6 +++++- >>> 3 files changed, 12 insertions(+), 39 deletions(-) >>> >>> diff --git a/include/net/net.h b/include/net/net.h >>> index 36e5fab..7af3e15 100644 >>> --- a/include/net/net.h >>> +++ b/include/net/net.h >>> @@ -152,11 +152,6 @@ void qemu_check_nic_model(NICInfo *nd, const >>> char *model); >>> int qemu_find_nic_model(NICInfo *nd, const char * const *models, >>> const char *default_model); >>> >>> -ssize_t qemu_deliver_packet(NetClientState *sender, >>> - unsigned flags, >>> - const uint8_t *data, >>> - size_t size, >>> - void *opaque); >>> ssize_t qemu_deliver_packet_iov(NetClientState *sender, >>> unsigned flags, >>> const struct iovec *iov, >>> diff --git a/net/net.c b/net/net.c >>> index ad37419..ca35b27 100644 >>> --- a/net/net.c >>> +++ b/net/net.c >>> @@ -597,36 +597,6 @@ static ssize_t filter_receive(NetClientState >>> *nc, NetFilterChain chain, >>> return filter_receive_iov(nc, chain, sender, flags, &iov, 1, >>> sent_cb); >>> } >>> >>> -ssize_t qemu_deliver_packet(NetClientState *sender, >>> - unsigned flags, >>> - const uint8_t *data, >>> - size_t size, >>> - void *opaque) >>> -{ >>> - NetClientState *nc = opaque; >>> - ssize_t ret; >>> - >>> - if (nc->link_down) { >>> - return size; >>> - } >>> - >>> - if (nc->receive_disabled) { >>> - return 0; >>> - } >>> - >>> - if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { >>> - ret = nc->info->receive_raw(nc, data, size); >>> - } else { >>> - ret = nc->info->receive(nc, data, size); >>> - } >>> - >>> - if (ret == 0) { >>> - nc->receive_disabled = 1; >>> - } >>> - >>> - return ret; >>> -} >>> - >>> void qemu_purge_queued_packets(NetClientState *nc) >>> { >>> if (!nc->peer) { >>> @@ -717,14 +687,18 @@ ssize_t qemu_send_packet_raw(NetClientState >>> *nc, const uint8_t *buf, int size) >>> } >>> >>> static ssize_t nc_sendv_compat(NetClientState *nc, const struct >>> iovec *iov, >>> - int iovcnt) >>> + int iovcnt, unsigned flags) >>> { >>> uint8_t buffer[NET_BUFSIZE]; >>> size_t offset; >>> >>> offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer)); >>> >>> - return nc->info->receive(nc, buffer, offset); >>> + if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { >>> + return nc->info->receive_raw(nc, buffer, offset); >>> + } else { >>> + return nc->info->receive(nc, buffer, offset); >>> + } >> >> Then for net clients that doesn't support receive_iov, an extra memcpy >> is introduced. This seems unnecessary. And this patch looks independent, >> so please drop this patch from the series (This can also help to reduce >> the iteration). I think we may need this after all net clients support >> receive_iov(). > > This patch is required by the next patch which introduce a > +/* Returns: > + * >0 - success > + * 0 - queue packet for future redelivery > + * <0 - failure (discard packet) > + */ > +typedef ssize_t (NetQueueDeliverFunc)(NetClientState *sender, > + unsigned flags, > + const struct iovec *iov, > + int iovcnt, > + void *opaque); > > If we drop this patch, we will need to introduce 2 deliver func, which > seems not clean and simple. Then you can probably skip the iov_to_buf() if iov_cnt is one in the above code? > >> >> >>> } >>> >>> ssize_t qemu_deliver_packet_iov(NetClientState *sender, >>> @@ -747,7 +721,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState >>> *sender, >>> if (nc->info->receive_iov) { >>> ret = nc->info->receive_iov(nc, iov, iovcnt); >>> } else { >>> - ret = nc_sendv_compat(nc, iov, iovcnt); >>> + ret = nc_sendv_compat(nc, iov, iovcnt, flags); >>> } >>> >>> if (ret == 0) { >>> diff --git a/net/queue.c b/net/queue.c >>> index ebbe2bb..cf8db3a 100644 >>> --- a/net/queue.c >>> +++ b/net/queue.c >>> @@ -152,9 +152,13 @@ static ssize_t qemu_net_queue_deliver(NetQueue >>> *queue, >>> size_t size) >>> { >>> ssize_t ret = -1; >>> + struct iovec iov = { >>> + .iov_base = (void *)data, >>> + .iov_len = size >>> + }; >>> >>> queue->delivering = 1; >>> - ret = qemu_deliver_packet(sender, flags, data, size, >>> queue->opaque); >>> + ret = qemu_deliver_packet_iov(sender, flags, &iov, 1, >>> queue->opaque); >>> queue->delivering = 0; >>> >>> return ret; >> >> >> . >> >
On 09/22/2015 04:14 PM, Jason Wang wrote: > > > On 09/22/2015 03:44 PM, Yang Hongyang wrote: >> Hi Jason, >> >> Thanks for the review, >> >> On 09/22/2015 03:30 PM, Jason Wang wrote: >>> >>> >>> On 09/16/2015 08:16 PM, Yang Hongyang wrote: >>>> qemu_deliver_packet_iov already have the compat delivery, we >>>> can drop qemu_deliver_packet. >>>> >>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> >>>> --- >>>> include/net/net.h | 5 ----- >>>> net/net.c | 40 +++++++--------------------------------- >>>> net/queue.c | 6 +++++- >>>> 3 files changed, 12 insertions(+), 39 deletions(-) >>>> >>>> diff --git a/include/net/net.h b/include/net/net.h >>>> index 36e5fab..7af3e15 100644 >>>> --- a/include/net/net.h >>>> +++ b/include/net/net.h >>>> @@ -152,11 +152,6 @@ void qemu_check_nic_model(NICInfo *nd, const >>>> char *model); >>>> int qemu_find_nic_model(NICInfo *nd, const char * const *models, >>>> const char *default_model); >>>> >>>> -ssize_t qemu_deliver_packet(NetClientState *sender, >>>> - unsigned flags, >>>> - const uint8_t *data, >>>> - size_t size, >>>> - void *opaque); >>>> ssize_t qemu_deliver_packet_iov(NetClientState *sender, >>>> unsigned flags, >>>> const struct iovec *iov, >>>> diff --git a/net/net.c b/net/net.c >>>> index ad37419..ca35b27 100644 >>>> --- a/net/net.c >>>> +++ b/net/net.c >>>> @@ -597,36 +597,6 @@ static ssize_t filter_receive(NetClientState >>>> *nc, NetFilterChain chain, >>>> return filter_receive_iov(nc, chain, sender, flags, &iov, 1, >>>> sent_cb); >>>> } >>>> >>>> -ssize_t qemu_deliver_packet(NetClientState *sender, >>>> - unsigned flags, >>>> - const uint8_t *data, >>>> - size_t size, >>>> - void *opaque) >>>> -{ >>>> - NetClientState *nc = opaque; >>>> - ssize_t ret; >>>> - >>>> - if (nc->link_down) { >>>> - return size; >>>> - } >>>> - >>>> - if (nc->receive_disabled) { >>>> - return 0; >>>> - } >>>> - >>>> - if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { >>>> - ret = nc->info->receive_raw(nc, data, size); >>>> - } else { >>>> - ret = nc->info->receive(nc, data, size); >>>> - } >>>> - >>>> - if (ret == 0) { >>>> - nc->receive_disabled = 1; >>>> - } >>>> - >>>> - return ret; >>>> -} >>>> - >>>> void qemu_purge_queued_packets(NetClientState *nc) >>>> { >>>> if (!nc->peer) { >>>> @@ -717,14 +687,18 @@ ssize_t qemu_send_packet_raw(NetClientState >>>> *nc, const uint8_t *buf, int size) >>>> } >>>> >>>> static ssize_t nc_sendv_compat(NetClientState *nc, const struct >>>> iovec *iov, >>>> - int iovcnt) >>>> + int iovcnt, unsigned flags) >>>> { >>>> uint8_t buffer[NET_BUFSIZE]; >>>> size_t offset; >>>> >>>> offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer)); >>>> >>>> - return nc->info->receive(nc, buffer, offset); >>>> + if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { >>>> + return nc->info->receive_raw(nc, buffer, offset); >>>> + } else { >>>> + return nc->info->receive(nc, buffer, offset); >>>> + } >>> >>> Then for net clients that doesn't support receive_iov, an extra memcpy >>> is introduced. This seems unnecessary. And this patch looks independent, >>> so please drop this patch from the series (This can also help to reduce >>> the iteration). I think we may need this after all net clients support >>> receive_iov(). >> >> This patch is required by the next patch which introduce a >> +/* Returns: >> + * >0 - success >> + * 0 - queue packet for future redelivery >> + * <0 - failure (discard packet) >> + */ >> +typedef ssize_t (NetQueueDeliverFunc)(NetClientState *sender, >> + unsigned flags, >> + const struct iovec *iov, >> + int iovcnt, >> + void *opaque); >> >> If we drop this patch, we will need to introduce 2 deliver func, which >> seems not clean and simple. > > Then you can probably skip the iov_to_buf() if iov_cnt is one in the > above code? Seems like a good idea to avoid the extra memcpy, thank you! BTW, can I do this on top of the series if there's no other comments? so that the first 10 patch can be merged as is. if not, I will send another version v12 to fix this. > >> >>> >>> >>>> } >>>> >>>> ssize_t qemu_deliver_packet_iov(NetClientState *sender, >>>> @@ -747,7 +721,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState >>>> *sender, >>>> if (nc->info->receive_iov) { >>>> ret = nc->info->receive_iov(nc, iov, iovcnt); >>>> } else { >>>> - ret = nc_sendv_compat(nc, iov, iovcnt); >>>> + ret = nc_sendv_compat(nc, iov, iovcnt, flags); >>>> } >>>> >>>> if (ret == 0) { >>>> diff --git a/net/queue.c b/net/queue.c >>>> index ebbe2bb..cf8db3a 100644 >>>> --- a/net/queue.c >>>> +++ b/net/queue.c >>>> @@ -152,9 +152,13 @@ static ssize_t qemu_net_queue_deliver(NetQueue >>>> *queue, >>>> size_t size) >>>> { >>>> ssize_t ret = -1; >>>> + struct iovec iov = { >>>> + .iov_base = (void *)data, >>>> + .iov_len = size >>>> + }; >>>> >>>> queue->delivering = 1; >>>> - ret = qemu_deliver_packet(sender, flags, data, size, >>>> queue->opaque); >>>> + ret = qemu_deliver_packet_iov(sender, flags, &iov, 1, >>>> queue->opaque); >>>> queue->delivering = 0; >>>> >>>> return ret; >>> >>> >>> . >>> >> > > . >
On 09/22/2015 04:21 PM, Yang Hongyang wrote: > > > On 09/22/2015 04:14 PM, Jason Wang wrote: >> >> >> On 09/22/2015 03:44 PM, Yang Hongyang wrote: >>> Hi Jason, >>> >>> Thanks for the review, >>> >>> On 09/22/2015 03:30 PM, Jason Wang wrote: >>>> >>>> >>>> On 09/16/2015 08:16 PM, Yang Hongyang wrote: >>>>> qemu_deliver_packet_iov already have the compat delivery, we >>>>> can drop qemu_deliver_packet. >>>>> >>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> >>>>> --- >>>>> include/net/net.h | 5 ----- >>>>> net/net.c | 40 +++++++--------------------------------- >>>>> net/queue.c | 6 +++++- >>>>> 3 files changed, 12 insertions(+), 39 deletions(-) >>>>> >>>>> diff --git a/include/net/net.h b/include/net/net.h >>>>> index 36e5fab..7af3e15 100644 >>>>> --- a/include/net/net.h >>>>> +++ b/include/net/net.h >>>>> @@ -152,11 +152,6 @@ void qemu_check_nic_model(NICInfo *nd, const >>>>> char *model); >>>>> int qemu_find_nic_model(NICInfo *nd, const char * const *models, >>>>> const char *default_model); >>>>> >>>>> -ssize_t qemu_deliver_packet(NetClientState *sender, >>>>> - unsigned flags, >>>>> - const uint8_t *data, >>>>> - size_t size, >>>>> - void *opaque); >>>>> ssize_t qemu_deliver_packet_iov(NetClientState *sender, >>>>> unsigned flags, >>>>> const struct iovec *iov, >>>>> diff --git a/net/net.c b/net/net.c >>>>> index ad37419..ca35b27 100644 >>>>> --- a/net/net.c >>>>> +++ b/net/net.c >>>>> @@ -597,36 +597,6 @@ static ssize_t filter_receive(NetClientState >>>>> *nc, NetFilterChain chain, >>>>> return filter_receive_iov(nc, chain, sender, flags, &iov, 1, >>>>> sent_cb); >>>>> } >>>>> >>>>> -ssize_t qemu_deliver_packet(NetClientState *sender, >>>>> - unsigned flags, >>>>> - const uint8_t *data, >>>>> - size_t size, >>>>> - void *opaque) >>>>> -{ >>>>> - NetClientState *nc = opaque; >>>>> - ssize_t ret; >>>>> - >>>>> - if (nc->link_down) { >>>>> - return size; >>>>> - } >>>>> - >>>>> - if (nc->receive_disabled) { >>>>> - return 0; >>>>> - } >>>>> - >>>>> - if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { >>>>> - ret = nc->info->receive_raw(nc, data, size); >>>>> - } else { >>>>> - ret = nc->info->receive(nc, data, size); >>>>> - } >>>>> - >>>>> - if (ret == 0) { >>>>> - nc->receive_disabled = 1; >>>>> - } >>>>> - >>>>> - return ret; >>>>> -} >>>>> - >>>>> void qemu_purge_queued_packets(NetClientState *nc) >>>>> { >>>>> if (!nc->peer) { >>>>> @@ -717,14 +687,18 @@ ssize_t qemu_send_packet_raw(NetClientState >>>>> *nc, const uint8_t *buf, int size) >>>>> } >>>>> >>>>> static ssize_t nc_sendv_compat(NetClientState *nc, const struct >>>>> iovec *iov, >>>>> - int iovcnt) >>>>> + int iovcnt, unsigned flags) >>>>> { >>>>> uint8_t buffer[NET_BUFSIZE]; >>>>> size_t offset; >>>>> >>>>> offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer)); >>>>> >>>>> - return nc->info->receive(nc, buffer, offset); >>>>> + if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { >>>>> + return nc->info->receive_raw(nc, buffer, offset); >>>>> + } else { >>>>> + return nc->info->receive(nc, buffer, offset); >>>>> + } >>>> >>>> Then for net clients that doesn't support receive_iov, an extra memcpy >>>> is introduced. This seems unnecessary. And this patch looks >>>> independent, >>>> so please drop this patch from the series (This can also help to >>>> reduce >>>> the iteration). I think we may need this after all net clients support >>>> receive_iov(). >>> >>> This patch is required by the next patch which introduce a >>> +/* Returns: >>> + * >0 - success >>> + * 0 - queue packet for future redelivery >>> + * <0 - failure (discard packet) >>> + */ >>> +typedef ssize_t (NetQueueDeliverFunc)(NetClientState *sender, >>> + unsigned flags, >>> + const struct iovec *iov, >>> + int iovcnt, >>> + void *opaque); >>> >>> If we drop this patch, we will need to introduce 2 deliver func, which >>> seems not clean and simple. >> >> Then you can probably skip the iov_to_buf() if iov_cnt is one in the >> above code? > > Seems like a good idea to avoid the extra memcpy, thank you! BTW, > can I do this on top of the series if there's no other comments? Better squash the changes in this patch or another independent patch before this patch. (We don't want to break bisection). > so that the first 10 patch can be merged as is. if not, I will send > another > version v12 to fix this. Post V12 or if you wish, I can fix this myself :) Thanks
On 09/22/2015 05:19 PM, Jason Wang wrote: [...] >>>> >>>> If we drop this patch, we will need to introduce 2 deliver func, which >>>> seems not clean and simple. >>> >>> Then you can probably skip the iov_to_buf() if iov_cnt is one in the >>> above code? >> >> Seems like a good idea to avoid the extra memcpy, thank you! BTW, >> can I do this on top of the series if there's no other comments? > > Better squash the changes in this patch or another independent patch > before this patch. (We don't want to break bisection). > >> so that the first 10 patch can be merged as is. if not, I will send >> another >> version v12 to fix this. > > Post V12 or if you wish, I can fix this myself :) It's great if you can fix this, then I can focus on the multiqueue support later, thanks a lot for the help! :) > > Thanks > > > . >
On 09/22/2015 05:26 PM, Yang Hongyang wrote: > On 09/22/2015 05:19 PM, Jason Wang wrote: > [...] >>>>> >>>>> If we drop this patch, we will need to introduce 2 deliver func, >>>>> which >>>>> seems not clean and simple. >>>> >>>> Then you can probably skip the iov_to_buf() if iov_cnt is one in the >>>> above code? >>> >>> Seems like a good idea to avoid the extra memcpy, thank you! BTW, >>> can I do this on top of the series if there's no other comments? >> >> Better squash the changes in this patch or another independent patch >> before this patch. (We don't want to break bisection). >> >>> so that the first 10 patch can be merged as is. if not, I will send >>> another >>> version v12 to fix this. >> >> Post V12 or if you wish, I can fix this myself :) > > It's great if you can fix this, then I can focus on the multiqueue > support later, thanks a lot for the help! :) Ok, will do.
diff --git a/include/net/net.h b/include/net/net.h index 36e5fab..7af3e15 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -152,11 +152,6 @@ void qemu_check_nic_model(NICInfo *nd, const char *model); int qemu_find_nic_model(NICInfo *nd, const char * const *models, const char *default_model); -ssize_t qemu_deliver_packet(NetClientState *sender, - unsigned flags, - const uint8_t *data, - size_t size, - void *opaque); ssize_t qemu_deliver_packet_iov(NetClientState *sender, unsigned flags, const struct iovec *iov, diff --git a/net/net.c b/net/net.c index ad37419..ca35b27 100644 --- a/net/net.c +++ b/net/net.c @@ -597,36 +597,6 @@ static ssize_t filter_receive(NetClientState *nc, NetFilterChain chain, return filter_receive_iov(nc, chain, sender, flags, &iov, 1, sent_cb); } -ssize_t qemu_deliver_packet(NetClientState *sender, - unsigned flags, - const uint8_t *data, - size_t size, - void *opaque) -{ - NetClientState *nc = opaque; - ssize_t ret; - - if (nc->link_down) { - return size; - } - - if (nc->receive_disabled) { - return 0; - } - - if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { - ret = nc->info->receive_raw(nc, data, size); - } else { - ret = nc->info->receive(nc, data, size); - } - - if (ret == 0) { - nc->receive_disabled = 1; - } - - return ret; -} - void qemu_purge_queued_packets(NetClientState *nc) { if (!nc->peer) { @@ -717,14 +687,18 @@ ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size) } static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov, - int iovcnt) + int iovcnt, unsigned flags) { uint8_t buffer[NET_BUFSIZE]; size_t offset; offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer)); - return nc->info->receive(nc, buffer, offset); + if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { + return nc->info->receive_raw(nc, buffer, offset); + } else { + return nc->info->receive(nc, buffer, offset); + } } ssize_t qemu_deliver_packet_iov(NetClientState *sender, @@ -747,7 +721,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender, if (nc->info->receive_iov) { ret = nc->info->receive_iov(nc, iov, iovcnt); } else { - ret = nc_sendv_compat(nc, iov, iovcnt); + ret = nc_sendv_compat(nc, iov, iovcnt, flags); } if (ret == 0) { diff --git a/net/queue.c b/net/queue.c index ebbe2bb..cf8db3a 100644 --- a/net/queue.c +++ b/net/queue.c @@ -152,9 +152,13 @@ static ssize_t qemu_net_queue_deliver(NetQueue *queue, size_t size) { ssize_t ret = -1; + struct iovec iov = { + .iov_base = (void *)data, + .iov_len = size + }; queue->delivering = 1; - ret = qemu_deliver_packet(sender, flags, data, size, queue->opaque); + ret = qemu_deliver_packet_iov(sender, flags, &iov, 1, queue->opaque); queue->delivering = 0; return ret;
qemu_deliver_packet_iov already have the compat delivery, we can drop qemu_deliver_packet. Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com> --- include/net/net.h | 5 ----- net/net.c | 40 +++++++--------------------------------- net/queue.c | 6 +++++- 3 files changed, 12 insertions(+), 39 deletions(-)