Re: [PATCH v4 3/3] VSOCK: Add virtio vsock vsockmon hooks

From: Stefan Hajnoczi <stefanha@redhat.com>
To: "Jorgen S. Hansen" <jhansen@vmware.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Zhu Yanjun <zyjzyj2000@gmail.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Gerard Garcia <ggarcia@abra.uab.cat>
Subject: Re: [PATCH v4 3/3] VSOCK: Add virtio vsock vsockmon hooks
Date: Thu, 20 Apr 2017 15:24:11 +0100	[thread overview]
Message-ID: <20170420142411.GA28450@stefanha-x1.localdomain> (raw)
In-Reply-To: <5E1B910C-0113-494A-A46D-41F9686BD525@vmware.com>

[-- Attachment #1: Type: text/plain, Size: 788 bytes --]

On Thu, Apr 20, 2017 at 12:35:40PM +0000, Jorgen S. Hansen wrote:
> > 
> > +/* Packet capture */
> > +void virtio_transport_deliver_tap_pkt(struct virtio_vsock_pkt *pkt)
> > +{
> > +	struct sk_buff *skb;
> > +	struct af_vsockmon_hdr *hdr;
> > +	unsigned char *t_hdr, *payload;
> > +
> > +	skb = alloc_skb(sizeof(*hdr) + sizeof(pkt->hdr) + pkt->len,
> > +			GFP_ATOMIC);
> 
> So with the current API, an skb will always be allocated, even if there are no listeners? And we’ll copy the payload into the skb as well, if there is any. Would it make sense to introduce a check here (exposed as part of the vsock tap API), that skips all that in the case of no listeners? In the common case, there won’t be any listeners, so it would save some cycles.

Good point, will fix.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]