From: Peng Tao <bergwolf at gmail.com> Date: Wed, 1 Mar 2017 11:56:24 +0800> So that we can cancel a queued pkt later if necessary. > > Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> > Signed-off-by: Peng Tao <bergwolf at gmail.com> > --- > include/linux/virtio_vsock.h | 2 ++ > net/vmw_vsock/virtio_transport_common.c | 7 +++++++ > 2 files changed, 9 insertions(+) > > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h > index 9638bfe..193ad3a 100644 > --- a/include/linux/virtio_vsock.h > +++ b/include/linux/virtio_vsock.h > @@ -48,6 +48,7 @@ struct virtio_vsock_pkt { > struct virtio_vsock_hdr hdr; > struct work_struct work; > struct list_head list; > + void *cancel_token; /* only used for cancellation */The type here is fixed, you only store vhost_sock object pointers here, so don't use "void *" please.
On Fri, Mar 3, 2017 at 5:13 AM, David Miller <davem at davemloft.net> wrote:> From: Peng Tao <bergwolf at gmail.com> > Date: Wed, 1 Mar 2017 11:56:24 +0800 > >> So that we can cancel a queued pkt later if necessary. >> >> Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> >> Signed-off-by: Peng Tao <bergwolf at gmail.com> >> --- >> include/linux/virtio_vsock.h | 2 ++ >> net/vmw_vsock/virtio_transport_common.c | 7 +++++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h >> index 9638bfe..193ad3a 100644 >> --- a/include/linux/virtio_vsock.h >> +++ b/include/linux/virtio_vsock.h >> @@ -48,6 +48,7 @@ struct virtio_vsock_pkt { >> struct virtio_vsock_hdr hdr; >> struct work_struct work; >> struct list_head list; >> + void *cancel_token; /* only used for cancellation */ > > The type here is fixed, you only store vhost_sock object pointers > here, so don't use "void *" please.It used to be "struct vhost_sock *" but no refcount is held. Stefan suggested to use "void *cancel_token" to make the code harder to misuse. Quoting Stefan: "This field is just an opaque token used for cancellation rather than a struct vsock_sock pointer that we are allowed to dereference. You could change this field to void *cancel_token to make the code harder to misuse." Ref: https://www.mail-archive.com/netdev at vger.kernel.org/msg142550.html Cheers, Tao
On Fri, Mar 03, 2017 at 09:25:54AM +0800, Peng Tao wrote:> On Fri, Mar 3, 2017 at 5:13 AM, David Miller <davem at davemloft.net> wrote: > > From: Peng Tao <bergwolf at gmail.com> > > Date: Wed, 1 Mar 2017 11:56:24 +0800 > > > >> So that we can cancel a queued pkt later if necessary. > >> > >> Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> > >> Signed-off-by: Peng Tao <bergwolf at gmail.com> > >> --- > >> include/linux/virtio_vsock.h | 2 ++ > >> net/vmw_vsock/virtio_transport_common.c | 7 +++++++ > >> 2 files changed, 9 insertions(+) > >> > >> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h > >> index 9638bfe..193ad3a 100644 > >> --- a/include/linux/virtio_vsock.h > >> +++ b/include/linux/virtio_vsock.h > >> @@ -48,6 +48,7 @@ struct virtio_vsock_pkt { > >> struct virtio_vsock_hdr hdr; > >> struct work_struct work; > >> struct list_head list; > >> + void *cancel_token; /* only used for cancellation */ > > > > The type here is fixed, you only store vhost_sock object pointers > > here, so don't use "void *" please. > It used to be "struct vhost_sock *" but no refcount is held. Stefan > suggested to use "void *cancel_token" to make the code harder to > misuse. > > Quoting Stefan: > "This field is just an opaque token used for cancellation rather than > a struct vsock_sock pointer that we are allowed to dereference. You > could change this field to void *cancel_token to make the code harder > to misuse." > > Ref: > https://www.mail-archive.com/netdev at vger.kernel.org/msg142550.htmlYes, the key point is that it shouldn't be used as a struct vsock_sock since we don't hold a reference. It's purely a token so the queued packet can be found later for cancellation. Stefan -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 455 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20170310/4cf4d47c/attachment.sig>