Subject: Re: [PATCH 5/7] CAN: Add virtual CAN netdevice driver

From: Patrick McHardy <[email-masked]>
Date: Wed, 14 Nov 2007 13:51:59 +0100

Urs Thuermann wrote:
> +static int vcan_open(struct net_device *dev)
> +{
> +	DBG("%s: interface up\n", dev->name);
> +
> +	netif_start_queue(dev);
> +	return 0;
> +}
> +
> +static int vcan_stop(struct net_device *dev)
> +{
> +	DBG("%s: interface down\n", dev->name);
> +
> +	netif_stop_queue(dev);
> +	return 0;
> +}

These two functions look unnecessary, there's no need to manage
the queue for pure software devices.

> +static int vcan_tx(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct net_device_stats *stats = &dev->stats;
> +	int loop;
> +
> +	DBG("sending skbuff on interface %s\n", dev->name);
> +
> +	stats->tx_packets++;
> +	stats->tx_bytes += skb->len;
> +
> +	/* set flag whether this packet has to be looped back */
> +	loop = skb->pkt_type == PACKET_LOOPBACK;
> +
> +	if (!echo) {
> +		/* no echo handling available inside this driver */
> +
> +		if (loop) {
> +			/*
> +			 * only count the packets here, because the
> +			 * CAN core already did the echo for us
> +			 */
> +			stats->rx_packets++;
> +			stats->rx_bytes += skb->len;
> +		}
> +		kfree_skb(skb);
> +		return 0;

Please use the NETDEV_TX codes.

Besides these minor issues, looks fine.