Thread (11 messages) 11 messages, 6 authors, 2007-05-10

Re: [patch 7/9] lguest: the net driver

From: Jeff Garzik <hidden>
Date: 2007-05-09 12:28:37
Also in: lkml

akpm@linux-foundation.org wrote:
+static void transfer_packet(struct net_device *dev,
+			    struct sk_buff *skb,
+			    unsigned int peernum)
+{
+	struct lguestnet_info *info = dev->priv;
+	struct lguest_dma dma;
+
+	skb_to_dma(skb, skb_headlen(skb), &dma);
+	pr_debug("xfer length %04x (%u)\n", htons(skb->len), skb->len);
+
+	hcall(LHCALL_SEND_DMA, peer_key(info,peernum), __pa(&dma), 0);
__pa() should not be used in any driver.

At the very least, lguest helper code should wrap this.


+static irqreturn_t lguestnet_rcv(int irq, void *dev_id)
+{
+	struct net_device *dev = dev_id;
+	struct lguestnet_info *info = dev->priv;
+	unsigned int i, done = 0;
+
+	for (i = 0; i < ARRAY_SIZE(info->dma); i++) {
+		unsigned int length;
+		struct sk_buff *skb;
+
+		length = info->dma[i].used_len;
+		if (length == 0)
+			continue;
+
+		done++;
+		skb = info->skb[i];
+		fill_slot(dev, i);
+
+		if (length < ETH_HLEN || length > ETH_HLEN + ETH_DATA_LEN) {
+			pr_debug(KERN_WARNING "%s: unbelievable skb len: %i\n",
+				 dev->name, length);
+			dev_kfree_skb(skb);
+			continue;
+		}
+
+		skb_put(skb, length);
+		skb->protocol = eth_type_trans(skb, dev);
+		/* This is a reliable transport. */
+		if (dev->features & NETIF_F_NO_CSUM)
+			skb->ip_summed = CHECKSUM_UNNECESSARY;
+		pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
+			 ntohs(skb->protocol), skb->len, skb->pkt_type);
+
+		dev->stats.rx_bytes += skb->len;
+		dev->stats.rx_packets++;
+		netif_rx(skb);
+	}
+	return done ? IRQ_HANDLED : IRQ_NONE;
Using NAPI would be preferable...


+static int lguestnet_probe(struct lguest_device *lgdev)
+{
+	int err, irqf = IRQF_SHARED;
+	struct net_device *dev;
+	struct lguestnet_info *info;
+	struct lguest_device_desc *desc = &lguest_devices[lgdev->index];
+
+	pr_debug("lguest_net: probing for device %i\n", lgdev->index);
+
+	dev = alloc_etherdev(sizeof(struct lguestnet_info));
+	if (!dev)
+		return -ENOMEM;
+
+	SET_MODULE_OWNER(dev);
+
+	/* Ethernet defaults with some changes */
+	ether_setup(dev);
+	dev->set_mac_address = NULL;
why NULL?

+	dev->dev_addr[0] = 0x02; /* set local assignment bit (IEEE802) */
+	dev->dev_addr[1] = 0x00;
+	memcpy(&dev->dev_addr[2], &lguest_data.guestid, 2);
+	dev->dev_addr[4] = 0x00;
+	dev->dev_addr[5] = 0x00;
+
+	dev->open = lguestnet_open;
+	dev->stop = lguestnet_close;
+	dev->hard_start_xmit = lguestnet_start_xmit;
+
+	/* Turning on/off promisc will call dev->set_multicast_list.
+	 * We don't actually support multicast yet */
+	dev->set_multicast_list = lguestnet_set_multicast;
+	dev->mem_start = ((unsigned long)desc->pfn << PAGE_SHIFT);
+	dev->mem_end = dev->mem_start + PAGE_SIZE * desc->num_pages;
+	dev->irq = lgdev->index+1;
don't fill in mem_start, mem_end and irq.  they are useless, and for 
lguest, misleading.

+	dev->features = NETIF_F_SG;
+	if (desc->features & LGUEST_NET_F_NOCSUM)
+		dev->features |= NETIF_F_NO_CSUM;
do not set SG without an accompanying csum bitflag

+	info = dev->priv;
use netdev_priv()

+	info->mapsize = PAGE_SIZE * desc->num_pages;
+	info->peer_phys = ((unsigned long)desc->pfn << PAGE_SHIFT);
+	info->peer = (void *)ioremap(info->peer_phys, info->mapsize);
check for NULL

+	/* This stores our peerid (upper bits reserved for future). */
+	info->me = (desc->features & (info->mapsize-1));
+
+	err = register_netdev(dev);
+	if (err) {
+		pr_debug("lguestnet: registering device failed\n");
+		goto free;
+	}
+
+	if (lguest_devices[lgdev->index].features & LGUEST_DEVICE_F_RANDOMNESS)
+		irqf |= IRQF_SAMPLE_RANDOM;
+	if (request_irq(dev->irq, lguestnet_rcv, irqf, "lguestnet", dev) != 0) {
+		pr_debug("lguestnet: could not get net irq %i\n", dev->irq);
+		goto unregister;
+	}
+
+	pr_debug("lguestnet: registered device %s\n", dev->name);
+	lgdev->private = dev;
+	return 0;
+
+unregister:
+	unregister_netdev(dev);
+free:
+	free_netdev(dev);
missing iounmap() on error

+static struct lguest_driver lguestnet_drv = {
+	.name = "lguestnet",
+	.owner = THIS_MODULE,
+	.device_type = LGUEST_DEVICE_T_NET,
+	.probe = lguestnet_probe,
+};
You are distinctly missing module remove support

	Jeff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help