On Fri, May 25, 2012 at 3:04 PM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> Il 24/05/2012 19:59, zwu.kernel@xxxxxxxxx ha scritto:
>> From: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
>>
>> Signed-off-by: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
>> ---
>> net/hub.c | 35 ++++++++++++++++++++++++++++++++---
>> net/hub.h | 2 ++
>> net/queue.c | 5 +++++
>> 3 files changed, 39 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/hub.c b/net/hub.c
>> index 8a583ab..d27c52a 100644
>> --- a/net/hub.c
>> +++ b/net/hub.c
>> @@ -28,6 +28,7 @@ typedef struct NetHubPort {
>> QLIST_ENTRY(NetHubPort) next;
>> NetHub *hub;
>> unsigned int id;
>> + uint64_t nr_packets;
>> } NetHubPort;
>>
>> struct NetHub {
>> @@ -39,19 +40,37 @@ struct NetHub {
>>
>> static QLIST_HEAD(, NetHub) hubs = QLIST_HEAD_INITIALIZER(&hubs);
>>
>> +static void net_hub_receive_completed(NetClientState *nc, ssize_t len)
>> +{
>> + NetHubPort *port = DO_UPCAST(NetHubPort, nc, nc);
>> + port->nr_packets--;
>> + if (!port->nr_packets) {
>> + qemu_net_queue_flush(nc->peer->send_queue);
>> + }
>> +}
>> +
>> +void net_hub_port_packet_stats(NetClientState *nc)
>> +{
>> + NetHubPort *port = DO_UPCAST(NetHubPort, nc, nc);
>> +
>> + port->nr_packets++;
>> +}
>
> Isn't this being called also for non-hubport clients?
>
>> static ssize_t net_hub_receive(NetHub *hub, NetHubPort *source_port,
>> const uint8_t *buf, size_t len)
>> {
>> NetHubPort *port;
>> + ssize_t ret = 0;
>>
>> QLIST_FOREACH(port, &hub->ports, next) {
>> if (port == source_port) {
>> continue;
>> }
>>
>> - qemu_send_packet(&port->nc, buf, len);
>> + ret = qemu_send_packet_async(&port->nc, buf, len,
>> + net_hub_receive_completed);
>
> Just increment nr_packets here:
>
> ret = qemu_send_packet_async
> if (ret == 0) {
> port->nr_packets++;
> }
>
>> }
>> - return len;
>> + return ret;
>
> You can return len here. In fact returning ret is wrong because the
> value comes from a random port (the last one).
>
>> }
>>
>> static ssize_t net_hub_receive_iov(NetHub *hub, NetHubPort *source_port,
>> @@ -65,7 +84,8 @@ static ssize_t net_hub_receive_iov(NetHub *hub, NetHubPort *source_port,
>> continue;
>> }
>>
>> - ret = qemu_sendv_packet(&port->nc, iov, iovcnt);
>> + ret = qemu_sendv_packet_async(&port->nc, iov, iovcnt,
>> + net_hub_receive_completed);
>
> Same here (increment nr_packets)
>
>> }
>> return ret;
>
> Same here (return len).
>
>> }
>> @@ -84,6 +104,13 @@ static NetHub *net_hub_new(unsigned int id)
>> return hub;
>> }
>>
>> +static int net_hub_port_can_receive(NetClientState *nc)
>> +{
>> + NetHubPort *port = DO_UPCAST(NetHubPort, nc, nc);
>> +
>> + return port->nr_packets ? 0 : 1;
>> +}
>
> This is "return port->nr_packets == 0;".
>
> But I think you need to implement this on the hub rather than on the
> port, and return true only if port->nr_packets == 0 for all ports.
I don't think so.
e.g. guest <---> hubport1 - hubport2 <--> network backend.
hubport1->nr_packets == 0 mean if guest can send packet through
hubport1 to outside.
while hubport2->nr_packets == 0 mean if network backend can send
packet through hubport1 to guest.
Their direction is different. So i don't understand why to need
"port->nr_packets == 0 for all ports"?
> Probably you can move nr_packets to the hub itself rather than the port?
>
>> static ssize_t net_hub_port_receive(NetClientState *nc,
>> const uint8_t *buf, size_t len)
>> {
>> @@ -110,6 +137,7 @@ static void net_hub_port_cleanup(NetClientState *nc)
>> static NetClientInfo net_hub_port_info = {
>> .type = NET_CLIENT_TYPE_HUB,
>> .size = sizeof(NetHubPort),
>> + .can_receive = net_hub_port_can_receive,
>> .receive = net_hub_port_receive,
>> .receive_iov = net_hub_port_receive_iov,
>> .cleanup = net_hub_port_cleanup,
>> @@ -128,6 +156,7 @@ static NetHubPort *net_hub_port_new(NetHub *hub)
>> port = DO_UPCAST(NetHubPort, nc, nc);
>> port->id = id;
>> port->hub = hub;
>> + port->nr_packets = 0;
>>
>> QLIST_INSERT_HEAD(&hub->ports, port, next);
>>
>
> Everything below this has to go away (sender is not necessarily a hub
> port!).
>
>> diff --git a/net/hub.h b/net/hub.h
>> index d04f1b1..542e657 100644
>> --- a/net/hub.h
>> +++ b/net/hub.h
>> @@ -23,4 +23,6 @@ void net_hub_info(Monitor *mon);
>> int net_hub_id_for_client(NetClientState *nc, unsigned int *id);
>> void net_hub_check_clients(void);
>>
>> +void net_hub_port_packet_stats(NetClientState *nc);
>> +
>> #endif /* NET_HUB_H */
>> diff --git a/net/queue.c b/net/queue.c
>> index 7484d2a..ebf18aa 100644
>> --- a/net/queue.c
>> +++ b/net/queue.c
>> @@ -22,6 +22,7 @@
>> */
>>
>> #include "net/queue.h"
>> +#include "net/hub.h"
>> #include "qemu-queue.h"
>> #include "net.h"
>>
>> @@ -101,6 +102,8 @@ static ssize_t qemu_net_queue_append(NetQueue *queue,
>>
>> QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
>>
>> + net_hub_port_packet_stats(sender);
>> +
>> return size;
>> }
>>
>> @@ -134,6 +137,8 @@ static ssize_t qemu_net_queue_append_iov(NetQueue *queue,
>>
>> QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
>>
>> + net_hub_port_packet_stats(sender);
>> +
>> return packet->size;
>> }
>>
>
--
Regards,
Zhi Yong Wu
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
