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

["Jorgen S. Hansen" <jhansen@vmware.com>]

> 
> +/* 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.

> +	if (!skb)
> +		return; /* nevermind if we cannot capture the packet */
> +
> +	hdr = (struct af_vsockmon_hdr *)skb_put(skb, sizeof(*hdr));
> +
> +	/* pkt->hdr is little-endian so no need to byteswap here */
> +	hdr->src_cid = pkt->hdr.src_cid;
> +	hdr->src_port = pkt->hdr.src_port;
> +	hdr->dst_cid = pkt->hdr.dst_cid;
> +	hdr->dst_port = pkt->hdr.dst_port;
> +
> +	hdr->transport = cpu_to_le16(AF_VSOCK_TRANSPORT_VIRTIO);
> +	hdr->len = cpu_to_le16(sizeof(pkt->hdr));
> +	hdr->reserved[0] = hdr->reserved[1] = 0;
> +
> +	switch(cpu_to_le16(pkt->hdr.op)) {
> +	case VIRTIO_VSOCK_OP_REQUEST:
> +	case VIRTIO_VSOCK_OP_RESPONSE:
> +		hdr->op = cpu_to_le16(AF_VSOCK_OP_CONNECT);
> +		break;
> +	case VIRTIO_VSOCK_OP_RST:
> +	case VIRTIO_VSOCK_OP_SHUTDOWN:
> +		hdr->op = cpu_to_le16(AF_VSOCK_OP_DISCONNECT);
> +		break;
> +	case VIRTIO_VSOCK_OP_RW:
> +		hdr->op = cpu_to_le16(AF_VSOCK_OP_PAYLOAD);
> +		break;
> +	case VIRTIO_VSOCK_OP_CREDIT_UPDATE:
> +	case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
> +		hdr->op = cpu_to_le16(AF_VSOCK_OP_CONTROL);
> +		break;
> +	default:
> +		hdr->op = cpu_to_le16(AF_VSOCK_OP_UNKNOWN);
> +		break;
> +	}
> +
> +	t_hdr = skb_put(skb, sizeof(pkt->hdr));
> +	memcpy(t_hdr, &pkt->hdr, sizeof(pkt->hdr));
> +
> +	if (pkt->len) {
> +		payload = skb_put(skb, pkt->len);
> +		memcpy(payload, pkt->buf, pkt->len);
> +	}
> +
> +	vsock_deliver_tap(skb);
> +}
> +EXPORT_SYMBOL_GPL(virtio_transport_deliver_tap_pkt);
> +
> static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> 					  struct virtio_vsock_pkt_info *info)
> {
> -- 
> 2.9.3
>