From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [Qemu-devel] [PATCH V2 11/20] tap: support enabling or disabling a queue Date: Tue, 29 Jan 2013 16:55:25 -0600 Message-ID: <87zjzrwqea.fsf@codemonkey.ws> References: <1359110143-42984-1-git-send-email-jasowang@redhat.com> <1359110143-42984-12-git-send-email-jasowang@redhat.com> <5107D3B5.8050506@redhat.com> <20130129221148.GA1777@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Jason Wang , krkumar2@in.ibm.com, kvm@vger.kernel.org, mprivozn@redhat.com, rusty@rustcorp.com.au, qemu-devel@nongnu.org, shajnocz@redhat.com, shiyer@redhat.com, jwhan@filewood.snu.ac.kr, gaowanlong@cn.fujitsu.com To: "Michael S. Tsirkin" , Blue Swirl Return-path: Received: from e39.co.us.ibm.com ([32.97.110.160]:37256 "EHLO e39.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751326Ab3A2W4K (ORCPT ); Tue, 29 Jan 2013 17:56:10 -0500 Received: from /spool/local by e39.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 29 Jan 2013 15:56:10 -0700 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 7A2FA3E40045 for ; Tue, 29 Jan 2013 15:55:59 -0700 (MST) Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r0TMtq7M126192 for ; Tue, 29 Jan 2013 15:55:55 -0700 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r0TMtUQw014743 for ; Tue, 29 Jan 2013 15:55:31 -0700 In-Reply-To: <20130129221148.GA1777@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: "Michael S. Tsirkin" writes: > On Tue, Jan 29, 2013 at 08:10:26PM +0000, Blue Swirl wrote: >> On Tue, Jan 29, 2013 at 1:50 PM, Jason Wang wrote: >> > On 01/26/2013 03:13 AM, Blue Swirl wrote: >> >> On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang wrote: >> >>> This patch introduce a new bit - enabled in TAPState which tracks whether a >> >>> specific queue/fd is enabled. The tap/fd is enabled during initialization and >> >>> could be enabled/disabled by tap_enalbe() and tap_disable() which calls platform >> >>> specific helpers to do the real work. Polling of a tap fd can only done when >> >>> the tap was enabled. >> >>> >> >>> Signed-off-by: Jason Wang >> >>> --- >> >>> include/net/tap.h | 2 ++ >> >>> net/tap-win32.c | 10 ++++++++++ >> >>> net/tap.c | 43 ++++++++++++++++++++++++++++++++++++++++--- >> >>> 3 files changed, 52 insertions(+), 3 deletions(-) >> >>> >> >>> diff --git a/include/net/tap.h b/include/net/tap.h >> >>> index bb7efb5..0caf8c4 100644 >> >>> --- a/include/net/tap.h >> >>> +++ b/include/net/tap.h >> >>> @@ -35,6 +35,8 @@ int tap_has_vnet_hdr_len(NetClientState *nc, int len); >> >>> void tap_using_vnet_hdr(NetClientState *nc, int using_vnet_hdr); >> >>> void tap_set_offload(NetClientState *nc, int csum, int tso4, int tso6, int ecn, int ufo); >> >>> void tap_set_vnet_hdr_len(NetClientState *nc, int len); >> >>> +int tap_enable(NetClientState *nc); >> >>> +int tap_disable(NetClientState *nc); >> >>> >> >>> int tap_get_fd(NetClientState *nc); >> >>> >> >>> diff --git a/net/tap-win32.c b/net/tap-win32.c >> >>> index 265369c..a2cd94b 100644 >> >>> --- a/net/tap-win32.c >> >>> +++ b/net/tap-win32.c >> >>> @@ -764,3 +764,13 @@ void tap_set_vnet_hdr_len(NetClientState *nc, int len) >> >>> { >> >>> assert(0); >> >>> } >> >>> + >> >>> +int tap_enable(NetClientState *nc) >> >>> +{ >> >>> + assert(0); >> >> abort() >> > >> > This is just to be consistent with the reset of the helpers in this file. >> >> >> >>> +} >> >>> + >> >>> +int tap_disable(NetClientState *nc) >> >>> +{ >> >>> + assert(0); >> >>> +} >> >>> diff --git a/net/tap.c b/net/tap.c >> >>> index 67080f1..95e557b 100644 >> >>> --- a/net/tap.c >> >>> +++ b/net/tap.c >> >>> @@ -59,6 +59,7 @@ typedef struct TAPState { >> >>> unsigned int write_poll : 1; >> >>> unsigned int using_vnet_hdr : 1; >> >>> unsigned int has_ufo: 1; >> >>> + unsigned int enabled : 1; >> >> bool without bit field? >> > >> > Also to be consistent with other field. If you wish I can send patches >> > to convert all those bit field to bool on top of this series. >> >> That would be nice, likewise for the assert(0). > > OK so let's go ahead with this patchset as is, > and a cleanup patch will be send after 1.4 then. Why? I'd prefer that we didn't rush things into 1.4 just because. There's still ample time to respin a corrected series. Regards, Anthony Liguori > > >> > >> > Thanks >> >>> VHostNetState *vhost_net; >> >>> unsigned host_vnet_hdr_len; >> >>> } TAPState; >> >>> @@ -72,9 +73,9 @@ static void tap_writable(void *opaque); >> >>> static void tap_update_fd_handler(TAPState *s) >> >>> { >> >>> qemu_set_fd_handler2(s->fd, >> >>> - s->read_poll ? tap_can_send : NULL, >> >>> - s->read_poll ? tap_send : NULL, >> >>> - s->write_poll ? tap_writable : NULL, >> >>> + s->read_poll && s->enabled ? tap_can_send : NULL, >> >>> + s->read_poll && s->enabled ? tap_send : NULL, >> >>> + s->write_poll && s->enabled ? tap_writable : NULL, >> >>> s); >> >>> } >> >>> >> >>> @@ -339,6 +340,7 @@ static TAPState *net_tap_fd_init(NetClientState *peer, >> >>> s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0; >> >>> s->using_vnet_hdr = 0; >> >>> s->has_ufo = tap_probe_has_ufo(s->fd); >> >>> + s->enabled = 1; >> >>> tap_set_offload(&s->nc, 0, 0, 0, 0, 0); >> >>> /* >> >>> * Make sure host header length is set correctly in tap: >> >>> @@ -737,3 +739,38 @@ VHostNetState *tap_get_vhost_net(NetClientState *nc) >> >>> assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP); >> >>> return s->vhost_net; >> >>> } >> >>> + >> >>> +int tap_enable(NetClientState *nc) >> >>> +{ >> >>> + TAPState *s = DO_UPCAST(TAPState, nc, nc); >> >>> + int ret; >> >>> + >> >>> + if (s->enabled) { >> >>> + return 0; >> >>> + } else { >> >>> + ret = tap_fd_enable(s->fd); >> >>> + if (ret == 0) { >> >>> + s->enabled = 1; >> >>> + tap_update_fd_handler(s); >> >>> + } >> >>> + return ret; >> >>> + } >> >>> +} >> >>> + >> >>> +int tap_disable(NetClientState *nc) >> >>> +{ >> >>> + TAPState *s = DO_UPCAST(TAPState, nc, nc); >> >>> + int ret; >> >>> + >> >>> + if (s->enabled == 0) { >> >>> + return 0; >> >>> + } else { >> >>> + ret = tap_fd_disable(s->fd); >> >>> + if (ret == 0) { >> >>> + qemu_purge_queued_packets(nc); >> >>> + s->enabled = 0; >> >>> + tap_update_fd_handler(s); >> >>> + } >> >>> + return ret; >> >>> + } >> >>> +} >> >>> -- >> >>> 1.7.1 >> >>> >> >>> >> > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html