| Message ID | 1359110143-42984-12-git-send-email-jasowang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang <jasowang@redhat.com> 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 <jasowang@redhat.com> > --- > 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() > +} > + > +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? > 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
On 01/26/2013 03:13 AM, Blue Swirl wrote: > On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang <jasowang@redhat.com> 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 <jasowang@redhat.com> >> --- >> 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. 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
On Tue, Jan 29, 2013 at 1:50 PM, Jason Wang <jasowang@redhat.com> wrote: > On 01/26/2013 03:13 AM, Blue Swirl wrote: >> On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang <jasowang@redhat.com> 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 <jasowang@redhat.com> >>> --- >>> 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). > > 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
On Tue, Jan 29, 2013 at 08:10:26PM +0000, Blue Swirl wrote: > On Tue, Jan 29, 2013 at 1:50 PM, Jason Wang <jasowang@redhat.com> wrote: > > On 01/26/2013 03:13 AM, Blue Swirl wrote: > >> On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang <jasowang@redhat.com> 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 <jasowang@redhat.com> > >>> --- > >>> 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. > > > > 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
"Michael S. Tsirkin" <mst@redhat.com> 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 <jasowang@redhat.com> wrote: >> > On 01/26/2013 03:13 AM, Blue Swirl wrote: >> >> On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang <jasowang@redhat.com> 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 <jasowang@redhat.com> >> >>> --- >> >>> 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 -- 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
On Tue, Jan 29, 2013 at 04:55:25PM -0600, Anthony Liguori wrote: > "Michael S. Tsirkin" <mst@redhat.com> 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 <jasowang@redhat.com> wrote: > >> > On 01/26/2013 03:13 AM, Blue Swirl wrote: > >> >> On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang <jasowang@redhat.com> 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 <jasowang@redhat.com> > >> >>> --- > >> >>> 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 Confused. Do you want the coding style rework of net/tap.c switching it from assert(0)/bitfields to abort()/bool for 1.4? > > > > > >> > > >> > 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 -- 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
On 01/30/2013 07:03 AM, Michael S. Tsirkin wrote: > On Tue, Jan 29, 2013 at 04:55:25PM -0600, Anthony Liguori wrote: >> "Michael S. Tsirkin" <mst@redhat.com> 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 <jasowang@redhat.com> wrote: >>>>> On 01/26/2013 03:13 AM, Blue Swirl wrote: >>>>>> On Fri, Jan 25, 2013 at 10:35 AM, Jason Wang <jasowang@redhat.com> 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 <jasowang@redhat.com> >>>>>>> --- >>>>>>> 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 > Confused. Do you want the coding style rework of net/tap.c > switching it from assert(0)/bitfields to abort()/bool for 1.4? I will send a new series with the patches that addresses Blue's comments on assert(0) and bitfields. Thanks >>> >>>>> 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 -- 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
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); +} + +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; 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; + } +}
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 <jasowang@redhat.com> --- include/net/tap.h | 2 ++ net/tap-win32.c | 10 ++++++++++ net/tap.c | 43 ++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 52 insertions(+), 3 deletions(-)