From 1ca2983aaa01233dc1ea63ef1b483678fcece6c8 Mon Sep 17 00:00:00 2001 From: Wei Liu Date: Mon, 20 May 2013 01:05:12 +0000 Subject: xen-netfront: avoid leaking resources when setup_netfront fails We should correctly free related resources (grant ref, memory page, evtchn) when setup_netfront fails. Signed-off-by: Wei Liu Signed-off-by: David S. Miller --- drivers/net/xen-netfront.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) (limited to 'drivers/net/xen-netfront.c') diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 1db10141506..5770e3b0ba7 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -1532,40 +1532,49 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info) FRONT_RING_INIT(&info->tx, txs, PAGE_SIZE); err = xenbus_grant_ring(dev, virt_to_mfn(txs)); - if (err < 0) { - free_page((unsigned long)txs); - goto fail; - } + if (err < 0) + goto grant_tx_ring_fail; info->tx_ring_ref = err; rxs = (struct xen_netif_rx_sring *)get_zeroed_page(GFP_NOIO | __GFP_HIGH); if (!rxs) { err = -ENOMEM; xenbus_dev_fatal(dev, err, "allocating rx ring page"); - goto fail; + goto alloc_rx_ring_fail; } SHARED_RING_INIT(rxs); FRONT_RING_INIT(&info->rx, rxs, PAGE_SIZE); err = xenbus_grant_ring(dev, virt_to_mfn(rxs)); - if (err < 0) { - free_page((unsigned long)rxs); - goto fail; - } + if (err < 0) + goto grant_rx_ring_fail; info->rx_ring_ref = err; err = xenbus_alloc_evtchn(dev, &info->evtchn); if (err) - goto fail; + goto alloc_evtchn_fail; err = bind_evtchn_to_irqhandler(info->evtchn, xennet_interrupt, 0, netdev->name, netdev); if (err < 0) - goto fail; + goto bind_fail; netdev->irq = err; return 0; - fail: + /* If we fail to setup netfront, it is safe to just revoke access to + * granted pages because backend is not accessing it at this point. + */ +bind_fail: + xenbus_free_evtchn(dev, info->evtchn); +alloc_evtchn_fail: + gnttab_end_foreign_access_ref(info->rx_ring_ref, 0); +grant_rx_ring_fail: + free_page((unsigned long)rxs); +alloc_rx_ring_fail: + gnttab_end_foreign_access_ref(info->tx_ring_ref, 0); +grant_tx_ring_fail: + free_page((unsigned long)txs); +fail: return err; } -- cgit v1.2.3-70-g09d2 From d634bf2cb96639b7eddb646dc1fc17b1bc6e567f Mon Sep 17 00:00:00 2001 From: Wei Liu Date: Wed, 22 May 2013 06:34:46 +0000 Subject: xen-netfront: split event channels support for Xen frontend driver This patch adds a new feature called feature-split-event-channels for netfront, enabling it to handle TX and RX events separately. If netback does not support this feature, it falls back to use single event channel. If netfront fails to setup split event channels, it will try falling back to single event channel. Signed-off-by: Wei Liu Reviewed-by: David Vrabel Signed-off-by: David S. Miller --- drivers/net/xen-netfront.c | 178 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 149 insertions(+), 29 deletions(-) (limited to 'drivers/net/xen-netfront.c') diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 5770e3b0ba7..62238a08cb5 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -85,7 +85,15 @@ struct netfront_info { struct napi_struct napi; - unsigned int evtchn; + /* Split event channels support, tx_* == rx_* when using + * single event channel. + */ + unsigned int tx_evtchn, rx_evtchn; + unsigned int tx_irq, rx_irq; + /* Only used when split event channels support is enabled */ + char tx_irq_name[IFNAMSIZ+4]; /* DEVNAME-tx */ + char rx_irq_name[IFNAMSIZ+4]; /* DEVNAME-rx */ + struct xenbus_device *xbdev; spinlock_t tx_lock; @@ -330,7 +338,7 @@ no_skb: push: RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&np->rx, notify); if (notify) - notify_remote_via_irq(np->netdev->irq); + notify_remote_via_irq(np->rx_irq); } static int xennet_open(struct net_device *dev) @@ -623,7 +631,7 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&np->tx, notify); if (notify) - notify_remote_via_irq(np->netdev->irq); + notify_remote_via_irq(np->tx_irq); u64_stats_update_begin(&stats->syncp); stats->tx_bytes += skb->len; @@ -1254,23 +1262,35 @@ static int xennet_set_features(struct net_device *dev, return 0; } -static irqreturn_t xennet_interrupt(int irq, void *dev_id) +static irqreturn_t xennet_tx_interrupt(int irq, void *dev_id) { - struct net_device *dev = dev_id; - struct netfront_info *np = netdev_priv(dev); + struct netfront_info *np = dev_id; + struct net_device *dev = np->netdev; unsigned long flags; spin_lock_irqsave(&np->tx_lock, flags); + xennet_tx_buf_gc(dev); + spin_unlock_irqrestore(&np->tx_lock, flags); - if (likely(netif_carrier_ok(dev))) { - xennet_tx_buf_gc(dev); - /* Under tx_lock: protects access to rx shared-ring indexes. */ - if (RING_HAS_UNCONSUMED_RESPONSES(&np->rx)) + return IRQ_HANDLED; +} + +static irqreturn_t xennet_rx_interrupt(int irq, void *dev_id) +{ + struct netfront_info *np = dev_id; + struct net_device *dev = np->netdev; + + if (likely(netif_carrier_ok(dev) && + RING_HAS_UNCONSUMED_RESPONSES(&np->rx))) napi_schedule(&np->napi); - } - spin_unlock_irqrestore(&np->tx_lock, flags); + return IRQ_HANDLED; +} +static irqreturn_t xennet_interrupt(int irq, void *dev_id) +{ + xennet_tx_interrupt(irq, dev_id); + xennet_rx_interrupt(irq, dev_id); return IRQ_HANDLED; } @@ -1451,9 +1471,14 @@ static void xennet_disconnect_backend(struct netfront_info *info) spin_unlock_irq(&info->tx_lock); spin_unlock_bh(&info->rx_lock); - if (info->netdev->irq) - unbind_from_irqhandler(info->netdev->irq, info->netdev); - info->evtchn = info->netdev->irq = 0; + if (info->tx_irq && (info->tx_irq == info->rx_irq)) + unbind_from_irqhandler(info->tx_irq, info); + if (info->tx_irq && (info->tx_irq != info->rx_irq)) { + unbind_from_irqhandler(info->tx_irq, info); + unbind_from_irqhandler(info->rx_irq, info); + } + info->tx_evtchn = info->rx_evtchn = 0; + info->tx_irq = info->rx_irq = 0; /* End access and free the pages */ xennet_end_access(info->tx_ring_ref, info->tx.sring); @@ -1503,12 +1528,82 @@ static int xen_net_read_mac(struct xenbus_device *dev, u8 mac[]) return 0; } +static int setup_netfront_single(struct netfront_info *info) +{ + int err; + + err = xenbus_alloc_evtchn(info->xbdev, &info->tx_evtchn); + if (err < 0) + goto fail; + + err = bind_evtchn_to_irqhandler(info->tx_evtchn, + xennet_interrupt, + 0, info->netdev->name, info); + if (err < 0) + goto bind_fail; + info->rx_evtchn = info->tx_evtchn; + info->rx_irq = info->tx_irq = err; + + return 0; + +bind_fail: + xenbus_free_evtchn(info->xbdev, info->tx_evtchn); + info->tx_evtchn = 0; +fail: + return err; +} + +static int setup_netfront_split(struct netfront_info *info) +{ + int err; + + err = xenbus_alloc_evtchn(info->xbdev, &info->tx_evtchn); + if (err < 0) + goto fail; + err = xenbus_alloc_evtchn(info->xbdev, &info->rx_evtchn); + if (err < 0) + goto alloc_rx_evtchn_fail; + + snprintf(info->tx_irq_name, sizeof(info->tx_irq_name), + "%s-tx", info->netdev->name); + err = bind_evtchn_to_irqhandler(info->tx_evtchn, + xennet_tx_interrupt, + 0, info->tx_irq_name, info); + if (err < 0) + goto bind_tx_fail; + info->tx_irq = err; + + snprintf(info->rx_irq_name, sizeof(info->rx_irq_name), + "%s-rx", info->netdev->name); + err = bind_evtchn_to_irqhandler(info->rx_evtchn, + xennet_rx_interrupt, + 0, info->rx_irq_name, info); + if (err < 0) + goto bind_rx_fail; + info->rx_irq = err; + + return 0; + +bind_rx_fail: + unbind_from_irqhandler(info->tx_irq, info); + info->tx_irq = 0; +bind_tx_fail: + xenbus_free_evtchn(info->xbdev, info->rx_evtchn); + info->rx_evtchn = 0; +alloc_rx_evtchn_fail: + xenbus_free_evtchn(info->xbdev, info->tx_evtchn); + info->tx_evtchn = 0; +fail: + return err; +} + static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info) { struct xen_netif_tx_sring *txs; struct xen_netif_rx_sring *rxs; int err; struct net_device *netdev = info->netdev; + unsigned int feature_split_evtchn; info->tx_ring_ref = GRANT_INVALID_REF; info->rx_ring_ref = GRANT_INVALID_REF; @@ -1516,6 +1611,12 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info) info->tx.sring = NULL; netdev->irq = 0; + err = xenbus_scanf(XBT_NIL, info->xbdev->otherend, + "feature-split-event-channels", "%u", + &feature_split_evtchn); + if (err < 0) + feature_split_evtchn = 0; + err = xen_net_read_mac(dev, netdev->dev_addr); if (err) { xenbus_dev_fatal(dev, err, "parsing %s/mac", dev->nodename); @@ -1550,22 +1651,23 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info) goto grant_rx_ring_fail; info->rx_ring_ref = err; - err = xenbus_alloc_evtchn(dev, &info->evtchn); + if (feature_split_evtchn) + err = setup_netfront_split(info); + /* setup single event channel if + * a) feature-split-event-channels == 0 + * b) feature-split-event-channels == 1 but failed to setup + */ + if (!feature_split_evtchn || (feature_split_evtchn && err)) + err = setup_netfront_single(info); + if (err) goto alloc_evtchn_fail; - err = bind_evtchn_to_irqhandler(info->evtchn, xennet_interrupt, - 0, netdev->name, netdev); - if (err < 0) - goto bind_fail; - netdev->irq = err; return 0; /* If we fail to setup netfront, it is safe to just revoke access to * granted pages because backend is not accessing it at this point. */ -bind_fail: - xenbus_free_evtchn(dev, info->evtchn); alloc_evtchn_fail: gnttab_end_foreign_access_ref(info->rx_ring_ref, 0); grant_rx_ring_fail: @@ -1610,11 +1712,27 @@ again: message = "writing rx ring-ref"; goto abort_transaction; } - err = xenbus_printf(xbt, dev->nodename, - "event-channel", "%u", info->evtchn); - if (err) { - message = "writing event-channel"; - goto abort_transaction; + + if (info->tx_evtchn == info->rx_evtchn) { + err = xenbus_printf(xbt, dev->nodename, + "event-channel", "%u", info->tx_evtchn); + if (err) { + message = "writing event-channel"; + goto abort_transaction; + } + } else { + err = xenbus_printf(xbt, dev->nodename, + "event-channel-tx", "%u", info->tx_evtchn); + if (err) { + message = "writing event-channel-tx"; + goto abort_transaction; + } + err = xenbus_printf(xbt, dev->nodename, + "event-channel-rx", "%u", info->rx_evtchn); + if (err) { + message = "writing event-channel-rx"; + goto abort_transaction; + } } err = xenbus_printf(xbt, dev->nodename, "request-rx-copy", "%u", @@ -1727,7 +1845,9 @@ static int xennet_connect(struct net_device *dev) * packets. */ netif_carrier_on(np->netdev); - notify_remote_via_irq(np->netdev->irq); + notify_remote_via_irq(np->tx_irq); + if (np->tx_irq != np->rx_irq) + notify_remote_via_irq(np->rx_irq); xennet_tx_buf_gc(dev); xennet_alloc_rx_buffers(dev); -- cgit v1.2.3-70-g09d2 From 8249152c472e10c18936b774737fd58c60335154 Mon Sep 17 00:00:00 2001 From: Li RongQing Date: Thu, 6 Jun 2013 14:35:18 +0800 Subject: xen-netfront: use skb_partial_csum_set() to simplify the codes use skb_partial_csum_set() to simplify the codes Cc: Jason Wang Signed-off-by: Li RongQing Acked-by: Jason Wang Signed-off-by: David S. Miller --- drivers/net/xen-netfront.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'drivers/net/xen-netfront.c') diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 62238a08cb5..76a22365d4e 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -858,7 +858,6 @@ static RING_IDX xennet_fill_frags(struct netfront_info *np, static int checksum_setup(struct net_device *dev, struct sk_buff *skb) { struct iphdr *iph; - unsigned char *th; int err = -EPROTO; int recalculate_partial_csum = 0; @@ -883,27 +882,27 @@ static int checksum_setup(struct net_device *dev, struct sk_buff *skb) goto out; iph = (void *)skb->data; - th = skb->data + 4 * iph->ihl; - if (th >= skb_tail_pointer(skb)) - goto out; - skb->csum_start = th - skb->head; switch (iph->protocol) { case IPPROTO_TCP: - skb->csum_offset = offsetof(struct tcphdr, check); + if (!skb_partial_csum_set(skb, 4 * iph->ihl, + offsetof(struct tcphdr, check))) + goto out; if (recalculate_partial_csum) { - struct tcphdr *tcph = (struct tcphdr *)th; + struct tcphdr *tcph = tcp_hdr(skb); tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr, skb->len - iph->ihl*4, IPPROTO_TCP, 0); } break; case IPPROTO_UDP: - skb->csum_offset = offsetof(struct udphdr, check); + if (!skb_partial_csum_set(skb, 4 * iph->ihl, + offsetof(struct udphdr, check))) + goto out; if (recalculate_partial_csum) { - struct udphdr *udph = (struct udphdr *)th; + struct udphdr *udph = udp_hdr(skb); udph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr, skb->len - iph->ihl*4, IPPROTO_UDP, 0); @@ -917,9 +916,6 @@ static int checksum_setup(struct net_device *dev, struct sk_buff *skb) goto out; } - if ((th + skb->csum_offset + 2) > skb_tail_pointer(skb)) - goto out; - err = 0; out: -- cgit v1.2.3-70-g09d2 From 383eda32b891002d5064cc97243c94b7cf78137a Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Thu, 27 Jun 2013 21:57:49 -0700 Subject: xen: Use more current logging styles Instead of mixing printk and pr_ forms, just use pr_ Miscellaneous changes around these conversions: Add a missing newline to avoid message interleaving, coalesce formats, reflow modified lines to 80 columns. Signed-off-by: Joe Perches Acked-by: Ian Campbell Acked-by: Konrad Rzeszutek Wilk Signed-off-by: David S. Miller --- drivers/net/xen-netback/netback.c | 7 +++---- drivers/net/xen-netfront.c | 28 +++++++++++++--------------- 2 files changed, 16 insertions(+), 19 deletions(-) (limited to 'drivers/net/xen-netfront.c') diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index 130bcb217d2..64828de25d9 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -1890,9 +1890,8 @@ static int __init netback_init(void) return -ENODEV; if (fatal_skb_slots < XEN_NETBK_LEGACY_SLOTS_MAX) { - printk(KERN_INFO - "xen-netback: fatal_skb_slots too small (%d), bump it to XEN_NETBK_LEGACY_SLOTS_MAX (%d)\n", - fatal_skb_slots, XEN_NETBK_LEGACY_SLOTS_MAX); + pr_info("fatal_skb_slots too small (%d), bump it to XEN_NETBK_LEGACY_SLOTS_MAX (%d)\n", + fatal_skb_slots, XEN_NETBK_LEGACY_SLOTS_MAX); fatal_skb_slots = XEN_NETBK_LEGACY_SLOTS_MAX; } @@ -1921,7 +1920,7 @@ static int __init netback_init(void) "netback/%u", group); if (IS_ERR(netbk->task)) { - printk(KERN_ALERT "kthread_create() fails at netback\n"); + pr_alert("kthread_create() fails at netback\n"); del_timer(&netbk->net_timer); rc = PTR_ERR(netbk->task); goto failed_init; diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 76a22365d4e..ff7f111fffe 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -29,6 +29,8 @@ * IN THE SOFTWARE. */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -385,9 +387,8 @@ static void xennet_tx_buf_gc(struct net_device *dev) skb = np->tx_skbs[id].skb; if (unlikely(gnttab_query_foreign_access( np->grant_tx_ref[id]) != 0)) { - printk(KERN_ALERT "xennet_tx_buf_gc: warning " - "-- grant still in use by backend " - "domain.\n"); + pr_alert("%s: warning -- grant still in use by backend domain\n", + __func__); BUG(); } gnttab_end_foreign_access_ref( @@ -804,14 +805,14 @@ static int xennet_set_skb_gso(struct sk_buff *skb, { if (!gso->u.gso.size) { if (net_ratelimit()) - printk(KERN_WARNING "GSO size must not be zero.\n"); + pr_warn("GSO size must not be zero\n"); return -EINVAL; } /* Currently only TCPv4 S.O. is supported. */ if (gso->u.gso.type != XEN_NETIF_GSO_TYPE_TCPV4) { if (net_ratelimit()) - printk(KERN_WARNING "Bad GSO type %d.\n", gso->u.gso.type); + pr_warn("Bad GSO type %d\n", gso->u.gso.type); return -EINVAL; } @@ -910,9 +911,8 @@ static int checksum_setup(struct net_device *dev, struct sk_buff *skb) break; default: if (net_ratelimit()) - printk(KERN_ERR "Attempting to checksum a non-" - "TCP/UDP packet, dropping a protocol" - " %d packet", iph->protocol); + pr_err("Attempting to checksum a non-TCP/UDP packet, dropping a protocol %d packet\n", + iph->protocol); goto out; } @@ -1359,14 +1359,14 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev) /* A grant for every tx ring slot */ if (gnttab_alloc_grant_references(TX_MAX_TARGET, &np->gref_tx_head) < 0) { - printk(KERN_ALERT "#### netfront can't alloc tx grant refs\n"); + pr_alert("can't alloc tx grant refs\n"); err = -ENOMEM; goto exit_free_stats; } /* A grant for every rx ring slot */ if (gnttab_alloc_grant_references(RX_MAX_TARGET, &np->gref_rx_head) < 0) { - printk(KERN_ALERT "#### netfront can't alloc rx grant refs\n"); + pr_alert("can't alloc rx grant refs\n"); err = -ENOMEM; goto exit_free_tx; } @@ -1430,16 +1430,14 @@ static int netfront_probe(struct xenbus_device *dev, err = register_netdev(info->netdev); if (err) { - printk(KERN_WARNING "%s: register_netdev err=%d\n", - __func__, err); + pr_warn("%s: register_netdev err=%d\n", __func__, err); goto fail; } err = xennet_sysfs_addif(info->netdev); if (err) { unregister_netdev(info->netdev); - printk(KERN_WARNING "%s: add sysfs failed err=%d\n", - __func__, err); + pr_warn("%s: add sysfs failed err=%d\n", __func__, err); goto fail; } @@ -2116,7 +2114,7 @@ static int __init netif_init(void) if (xen_hvm_domain() && !xen_platform_pci_unplug) return -ENODEV; - printk(KERN_INFO "Initialising Xen virtual ethernet driver.\n"); + pr_info("Initialising Xen virtual ethernet driver\n"); return xenbus_register_frontend(&netfront_driver); } -- cgit v1.2.3-70-g09d2