From 80d887dbb673f007938c467fbfa118bba3e9f37d Mon Sep 17 00:00:00 2001 From: stephen hemminger Date: Mon, 24 Jul 2017 21:03:19 -0700 Subject: Revert "netvsc: optimize calculation of number of slots" The logic for computing page buffer scatter does not take into account the impact of compound pages. Therefore the optimization to compute number of slots was incorrect and could cause stack corruption a skb was sent with lots of fragments from huge pages. This reverts commit 60b86665af0dfbeebda8aae43f0ba451cd2dcfe5. Fixes: 60b86665af0d ("netvsc: optimize calculation of number of slots") Signed-off-by: Stephen Hemminger Signed-off-by: David S. Miller --- drivers/net/hyperv/netvsc_drv.c | 43 +++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) (limited to 'drivers/net/hyperv') diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 63c98bbbc596..0d78727f1a14 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -315,14 +315,34 @@ static u32 init_page_array(void *hdr, u32 len, struct sk_buff *skb, return slots_used; } -/* Estimate number of page buffers neede to transmit - * Need at most 2 for RNDIS header plus skb body and fragments. - */ -static unsigned int netvsc_get_slots(const struct sk_buff *skb) +static int count_skb_frag_slots(struct sk_buff *skb) +{ + int i, frags = skb_shinfo(skb)->nr_frags; + int pages = 0; + + for (i = 0; i < frags; i++) { + skb_frag_t *frag = skb_shinfo(skb)->frags + i; + unsigned long size = skb_frag_size(frag); + unsigned long offset = frag->page_offset; + + /* Skip unused frames from start of page */ + offset &= ~PAGE_MASK; + pages += PFN_UP(offset + size); + } + return pages; +} + +static int netvsc_get_slots(struct sk_buff *skb) { - return PFN_UP(offset_in_page(skb->data) + skb_headlen(skb)) - + skb_shinfo(skb)->nr_frags - + 2; + char *data = skb->data; + unsigned int offset = offset_in_page(data); + unsigned int len = skb_headlen(skb); + int slots; + int frag_slots; + + slots = DIV_ROUND_UP(offset + len, PAGE_SIZE); + frag_slots = count_skb_frag_slots(skb); + return slots + frag_slots; } static u32 net_checksum_info(struct sk_buff *skb) @@ -360,18 +380,21 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net) struct hv_page_buffer page_buf[MAX_PAGE_BUFFER_COUNT]; struct hv_page_buffer *pb = page_buf; - /* We can only transmit MAX_PAGE_BUFFER_COUNT number + /* We will atmost need two pages to describe the rndis + * header. We can only transmit MAX_PAGE_BUFFER_COUNT number * of pages in a single packet. If skb is scattered around * more pages we try linearizing it. */ - num_data_pgs = netvsc_get_slots(skb); + + num_data_pgs = netvsc_get_slots(skb) + 2; + if (unlikely(num_data_pgs > MAX_PAGE_BUFFER_COUNT)) { ++net_device_ctx->eth_stats.tx_scattered; if (skb_linearize(skb)) goto no_memory; - num_data_pgs = netvsc_get_slots(skb); + num_data_pgs = netvsc_get_slots(skb) + 2; if (num_data_pgs > MAX_PAGE_BUFFER_COUNT) { ++net_device_ctx->eth_stats.tx_too_big; goto drop; -- cgit v1.2.3-55-g7522 From 4a0dee1ffe0e8f4101e704a325e97f8997b0abcc Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Tue, 1 Aug 2017 12:11:12 -0700 Subject: netvsc: Initialize 64-bit stats seqcount On 32-bit hosts and with CONFIG_DEBUG_LOCK_ALLOC we should be seeing a lockdep splat indicating this seqcount is not correctly initialized, fix that. In commit 6c80f3fc2398 ("netvsc: report per-channel stats in ethtool statistics") netdev_alloc_pcpu_stats() was removed in favor of open-coding the 64-bits statistics, except that u64_stats_init() was missed. Fixes: 6c80f3fc2398 ("netvsc: report per-channel stats in ethtool statistics") Signed-off-by: Florian Fainelli Signed-off-by: Stephen Hemminger Signed-off-by: David S. Miller --- drivers/net/hyperv/netvsc.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/net/hyperv') diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index 0a9167dd72fb..96f90c75d1b7 100644 --- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c @@ -1302,6 +1302,8 @@ int netvsc_device_add(struct hv_device *device, struct netvsc_channel *nvchan = &net_device->chan_table[i]; nvchan->channel = device->channel; + u64_stats_init(&nvchan->tx_stats.syncp); + u64_stats_init(&nvchan->rx_stats.syncp); } /* Enable NAPI handler before init callbacks */ -- cgit v1.2.3-55-g7522 From 732e49850c5e15231e11a0a464748bcbade5e3c2 Mon Sep 17 00:00:00 2001 From: stephen hemminger Date: Thu, 3 Aug 2017 17:13:54 -0700 Subject: netvsc: fix race on sub channel creation The existing sub channel code did not wait for all the sub-channels to completely initialize. This could lead to race causing crash in napi_netif_del() from bad list. The existing code would send an init message, then wait only for the initial response that the init message was received. It thought it was waiting for sub channels but really the init response did the wakeup. The new code keeps track of the number of open channels and waits until that many are open. Other issues here were: * host might return less sub-channels than was requested. * the new init status is not valid until after init was completed. Fixes: b3e6b82a0099 ("hv_netvsc: Wait for sub-channels to be processed during probe") Signed-off-by: Stephen Hemminger Signed-off-by: David S. Miller --- drivers/net/hyperv/hyperv_net.h | 3 ++- drivers/net/hyperv/netvsc.c | 1 + drivers/net/hyperv/rndis_filter.c | 14 ++++++++------ 3 files changed, 11 insertions(+), 7 deletions(-) (limited to 'drivers/net/hyperv') diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h index d6c25580f8dd..12cc64bfcff8 100644 --- a/drivers/net/hyperv/hyperv_net.h +++ b/drivers/net/hyperv/hyperv_net.h @@ -765,7 +765,8 @@ struct netvsc_device { u32 max_chn; u32 num_chn; - refcount_t sc_offered; + atomic_t open_chn; + wait_queue_head_t subchan_open; struct rndis_device *extension; diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index 96f90c75d1b7..d18c3326a1f7 100644 --- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c @@ -78,6 +78,7 @@ static struct netvsc_device *alloc_net_device(void) net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT; net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT; init_completion(&net_device->channel_init_wait); + init_waitqueue_head(&net_device->subchan_open); return net_device; } diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c index 85c00e1c52b6..d6308ffda53e 100644 --- a/drivers/net/hyperv/rndis_filter.c +++ b/drivers/net/hyperv/rndis_filter.c @@ -1048,8 +1048,8 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc) else netif_napi_del(&nvchan->napi); - if (refcount_dec_and_test(&nvscdev->sc_offered)) - complete(&nvscdev->channel_init_wait); + atomic_inc(&nvscdev->open_chn); + wake_up(&nvscdev->subchan_open); } int rndis_filter_device_add(struct hv_device *dev, @@ -1090,8 +1090,6 @@ int rndis_filter_device_add(struct hv_device *dev, net_device->max_chn = 1; net_device->num_chn = 1; - refcount_set(&net_device->sc_offered, 0); - net_device->extension = rndis_device; rndis_device->ndev = net; @@ -1221,11 +1219,11 @@ int rndis_filter_device_add(struct hv_device *dev, rndis_device->ind_table[i] = ethtool_rxfh_indir_default(i, net_device->num_chn); + atomic_set(&net_device->open_chn, 1); num_rss_qs = net_device->num_chn - 1; if (num_rss_qs == 0) return 0; - refcount_set(&net_device->sc_offered, num_rss_qs); vmbus_set_sc_create_callback(dev->channel, netvsc_sc_open); init_packet = &net_device->channel_init_pkt; @@ -1242,15 +1240,19 @@ int rndis_filter_device_add(struct hv_device *dev, if (ret) goto out; + wait_for_completion(&net_device->channel_init_wait); if (init_packet->msg.v5_msg.subchn_comp.status != NVSP_STAT_SUCCESS) { ret = -ENODEV; goto out; } - wait_for_completion(&net_device->channel_init_wait); net_device->num_chn = 1 + init_packet->msg.v5_msg.subchn_comp.num_subchannels; + /* wait for all sub channels to open */ + wait_event(net_device->subchan_open, + atomic_read(&net_device->open_chn) == net_device->num_chn); + /* ignore failues from setting rss parameters, still have channels */ rndis_filter_set_rss_param(rndis_device, netvsc_hash_key, net_device->num_chn); -- cgit v1.2.3-55-g7522 From 9b4e946ce14e20d7addbfb7d9139e604f9fda107 Mon Sep 17 00:00:00 2001 From: stephen hemminger Date: Thu, 24 Aug 2017 16:49:16 -0700 Subject: netvsc: fix deadlock betwen link status and removal There is a deadlock possible when canceling the link status delayed work queue. The removal process is run with RTNL held, and the link status callback is acquring RTNL. Resolve the issue by using trylock and rescheduling. If cancel is in process, that block it from happening. Fixes: 122a5f6410f4 ("staging: hv: use delayed_work for netvsc_send_garp()") Signed-off-by: Stephen Hemminger Signed-off-by: David S. Miller --- drivers/net/hyperv/netvsc_drv.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers/net/hyperv') diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 0d78727f1a14..d91cbc6c3ca4 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -1269,7 +1269,12 @@ static void netvsc_link_change(struct work_struct *w) bool notify = false, reschedule = false; unsigned long flags, next_reconfig, delay; - rtnl_lock(); + /* if changes are happening, comeback later */ + if (!rtnl_trylock()) { + schedule_delayed_work(&ndev_ctx->dwork, LINKCHANGE_INT); + return; + } + net_device = rtnl_dereference(ndev_ctx->nvdev); if (!net_device) goto out_unlock; -- cgit v1.2.3-55-g7522