Skip to contentSkip to conversation

netvsc: bug fixes and cleanups

PATCH In review Stephen Hemminger 2 participants 11 messages Mar 22, 2017 – Mar 23, 2017

Subsystems (3)
Networking Network drivers Hyper-V/Azure core and drivers
lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

cover netvsc: bug fixes and cleanups

These fix NAPI issues and bugs found during testing of shutdown testing.

Stephen Hemminger (9):
netvsc: fix NAPI performance regression
netvsc: handle offline mtu and channel change
netvsc: change max channel calculation
netvsc: use RCU to protect inner device structure
netvsc: uses RCU instead of removal flag
netvsc: use refcount_t for keeping track of sub channels
netvsc: remove unnecessary lock on shutdown
netvsc: eliminate unnecessary skb == NULL checks
netvsc: fix and cleanup rndis_filter_set_packet_filter

 drivers/net/hyperv/hyperv_net.h   |  12 ++--
 drivers/net/hyperv/netvsc.c       |  70 ++++++++++------------
 drivers/net/hyperv/netvsc_drv.c   | 121 ++++++++++++++++++++++----------------
 drivers/net/hyperv/rndis_filter.c |  91 +++++++++-------------------
 4 files changed, 138 insertions(+), 156 deletions(-)
Signature · 1 line
2.11.0
author
#1 lore raw

PATCH 1/9 netvsc: fix NAPI performance regression

When using NAPI, the single stream performance declined signifcantly because the poll routine was updating host after every burst of packets. This excess signalling caused host throttling.

This fix restores the old behavior. Host is only signalled after the ring has been emptied.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/hyperv/hyperv_net.h |  1 +
 drivers/net/hyperv/netvsc.c     | 41 ++++++++++++++++++-----------------------
 2 files changed, 19 insertions(+), 23 deletions(-)
drivers/net/hyperv/hyperv_net.h +1 -0
index 6b5f75217694..a33f2ee86044 100644
@@ -723,6 +723,7 @@ struct net_device_context {
 /* Per channel data */
 struct netvsc_channel {
 	struct vmbus_channel *channel;
+	const struct vmpacket_descriptor *desc;
 	struct napi_struct napi;
 	struct multi_send_data msd;
 	struct multi_recv_comp mrc;
drivers/net/hyperv/netvsc.c +18 -23

77 lines not on this page · raw message

Signature · 1 line
2.11.0
author
#2 lore raw

PATCH 2/9 netvsc: handle offline mtu and channel change

In reply to Stephen Hemminger

If device is not up, then changing MTU (or number of channels) should not re-enable the device.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/hyperv/netvsc_drv.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)
drivers/net/hyperv/netvsc_drv.c +20 -9
index 191372486a87..b3a7f508434b 100644
@@ -743,6 +743,7 @@ static int netvsc_set_channels(struct net_device *net,
 	struct hv_device *dev = net_device_ctx->device_ctx;
 	struct netvsc_device *nvdev = net_device_ctx->nvdev;
 	unsigned int count = channels->combined_count;
+	bool was_running;
 	int ret;
 
 	/* We do not support separate count for rx, tx, or other */

@@ -762,9 +763,12 @@ static int netvsc_set_channels(struct net_device *net,
 	if (count > nvdev->max_chn)
 		return -EINVAL;
 
-	ret = netvsc_close(net);
-	if (ret)
-		return ret;
+	was_running = netif_running(net);
+	if (was_running) {
+		ret = netvsc_close(net);
+		if (ret)
+			return ret;
+	}
 
 	net_device_ctx->start_remove = true;
 	rndis_filter_device_remove(dev, nvdev);

@@ -775,9 +779,11 @@ static int netvsc_set_channels(struct net_device *net,
 	else
 		netvsc_set_queues(net, dev, nvdev->num_chn);
 
-	netvsc_open(net);
 	net_device_ctx->start_remove = false;
 
+	if (was_running)
+		ret = netvsc_open(net);
+
 	/* We may have missed link change notifications */
 	schedule_delayed_work(&net_device_ctx->dwork, 0);
 

@@ -845,14 +851,18 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	struct netvsc_device *nvdev = ndevctx->nvdev;
 	struct hv_device *hdev = ndevctx->device_ctx;
 	struct netvsc_device_info device_info;
+	bool was_running;
 	int ret;
 
 	if (ndevctx->start_remove || !nvdev || nvdev->destroy)
 		return -ENODEV;
 
-	ret = netvsc_close(ndev);
-	if (ret)
-		goto out;
+	was_running = netif_running(ndev);
+	if (was_running) {
+		ret = netvsc_close(ndev);
+		if (ret)
+			return ret;
+	}
 
 	memset(&device_info, 0, sizeof(device_info));
 	device_info.ring_size = ring_size;

@@ -872,10 +882,11 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 
 	rndis_filter_device_add(hdev, &device_info);
 
-out:
-	netvsc_open(ndev);
 	ndevctx->start_remove = false;
 
+	if (was_running)
+		ret = netvsc_open(ndev);
+
 	/* We may have missed link change notifications */
 	schedule_delayed_work(&ndevctx->dwork, 0);
Signature · 1 line
2.11.0
author
#3 lore raw

PATCH 3/9 netvsc: change max channel calculation

In reply to Stephen Hemminger

The default number of maximum channels should be limited to the number of cpus available on the numa node of the primary channel. This also makes sure maximum channels <= num_online_cpus

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/hyperv/netvsc_drv.c   |  3 +--
 drivers/net/hyperv/rndis_filter.c | 25 ++++++++++---------------
 2 files changed, 11 insertions(+), 17 deletions(-)
drivers/net/hyperv/netvsc_drv.c +1 -2
index b3a7f508434b..2f9de2e9f38e 100644
@@ -1503,8 +1503,7 @@ static int netvsc_probe(struct hv_device *dev,
 	/* Notify the netvsc driver of the new device */
 	memset(&device_info, 0, sizeof(device_info));
 	device_info.ring_size = ring_size;
-	device_info.max_num_vrss_chns = min_t(u32, VRSS_CHANNEL_DEFAULT,
-					      num_online_cpus());
+	device_info.num_chn = VRSS_CHANNEL_DEFAULT;
 	ret = rndis_filter_device_add(dev, &device_info);
 	if (ret != 0) {
 		netdev_err(net, "unable to add netvsc device (ret %d)\n", ret);
drivers/net/hyperv/rndis_filter.c +10 -15
index 382b9a62e3c4..d193d549cec6 100644
@@ -1181,33 +1181,28 @@ int rndis_filter_device_add(struct hv_device *dev,
 	if (ret || rsscap.num_recv_que < 2)
 		goto out;
 
-	net_device->max_chn = min_t(u32, VRSS_CHANNEL_MAX, rsscap.num_recv_que);
-
-	num_rss_qs = min(device_info->max_num_vrss_chns, net_device->max_chn);
-
 	/*
 	 * We will limit the VRSS channels to the number CPUs in the NUMA node
 	 * the primary channel is currently bound to.
+	 *
+	 * This also guarantees that num_possible_rss_qs <= num_online_cpus
 	 */
 	node_cpu_mask = cpumask_of_node(cpu_to_node(dev->channel->target_cpu));
-	num_possible_rss_qs = cpumask_weight(node_cpu_mask);
+	num_possible_rss_qs = min_t(u32, cpumask_weight(node_cpu_mask),
+				    rsscap.num_recv_que);
 
-	/* We will use the given number of channels if available. */
-	if (device_info->num_chn && device_info->num_chn < net_device->max_chn)
-		net_device->num_chn = device_info->num_chn;
-	else
-		net_device->num_chn = min(num_possible_rss_qs, num_rss_qs);
+	net_device->max_chn = min_t(u32, VRSS_CHANNEL_MAX, num_possible_rss_qs);
 
-	num_rss_qs = net_device->num_chn - 1;
+	/* We will use the given number of channels if available. */
+	net_device->num_chn = min(net_device->max_chn, device_info->num_chn);
 
 	for (i = 0; i < ITAB_NUM; i++)
 		rndis_device->ind_table[i] = ethtool_rxfh_indir_default(i,
 							net_device->num_chn);
 
-	net_device->num_sc_offered = num_rss_qs;
-
-	if (net_device->num_chn == 1)
-		goto out;
+	num_rss_qs = net_device->num_chn - 1;
+	if (num_rss_qs == 0)
+		return 0;
 
 	vmbus_set_sc_create_callback(dev->channel, netvsc_sc_open);
Signature · 1 line
2.11.0
author
#4 lore raw

PATCH 4/9 netvsc: use RCU to protect inner device structure

In reply to Stephen Hemminger

The netvsc driver has an internal structure (netvsc_device) which is created when device is opened and released when device is closed. And also opened/released when MTU or number of channels change.

Since this is referenced in the receive and transmit path, it is safer to use RCU to protect/prevent use after free problems.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/hyperv/hyperv_net.h |  4 ++-
 drivers/net/hyperv/netvsc.c     | 16 +++++++----
 drivers/net/hyperv/netvsc_drv.c | 61 +++++++++++++++++++++++++++++------------
 3 files changed, 57 insertions(+), 24 deletions(-)
drivers/net/hyperv/hyperv_net.h +3 -1
index a33f2ee86044..0ade21f95d71 100644
@@ -686,7 +686,7 @@ struct net_device_context {
 	/* point back to our device context */
 	struct hv_device *device_ctx;
 	/* netvsc_device */
-	struct netvsc_device *nvdev;
+	struct netvsc_device __rcu *nvdev;
 	/* reconfigure work */
 	struct delayed_work dwork;
 	/* last reconfig time */

@@ -780,6 +780,8 @@ struct netvsc_device {
 	atomic_t open_cnt;
 
 	struct netvsc_channel chan_table[VRSS_CHANNEL_MAX];
+
+	struct rcu_head rcu;
 };
 
 static inline struct netvsc_device *
drivers/net/hyperv/netvsc.c +11 -5
index 727762d0f13b..ab9118d620ab 100644
@@ -80,8 +80,10 @@ static struct netvsc_device *alloc_net_device(void)
 	return net_device;
 }
 
-static void free_netvsc_device(struct netvsc_device *nvdev)
+static void free_netvsc_device(struct rcu_head *head)
 {
+	struct netvsc_device *nvdev
+		= container_of(head, struct netvsc_device, rcu);
 	int i;
 
 	for (i = 0; i < VRSS_CHANNEL_MAX; i++)

@@ -90,6 +92,10 @@ static void free_netvsc_device(struct netvsc_device *nvdev)
 	kfree(nvdev);
 }
 
+static void free_netvsc_device_rcu(struct netvsc_device *nvdev)
+{
+	call_rcu(&nvdev->rcu, free_netvsc_device);
+}
 
 static struct netvsc_device *get_outbound_net_device(struct hv_device *device)
 {

@@ -551,7 +557,7 @@ void netvsc_device_remove(struct hv_device *device)
 
 	netvsc_disconnect_vsp(device);
 
-	net_device_ctx->nvdev = NULL;
+	RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
 
 	/*
 	 * At this point, no one should be accessing net_device

@@ -566,7 +572,7 @@ void netvsc_device_remove(struct hv_device *device)
 		napi_disable(&net_device->chan_table[i].napi);
 
 	/* Release all resources */
-	free_netvsc_device(net_device);
+	free_netvsc_device_rcu(net_device);
 }
 
 #define RING_AVAIL_PERCENT_HIWATER 20

@@ -1322,7 +1328,7 @@ int netvsc_device_add(struct hv_device *device,
 	 */
 	wmb();
 
-	net_device_ctx->nvdev = net_device;
+	rcu_assign_pointer(net_device_ctx->nvdev, net_device);
 
 	/* Connect with the NetVsp */
 	ret = netvsc_connect_vsp(device);

@@ -1341,7 +1347,7 @@ int netvsc_device_add(struct hv_device *device,
 	vmbus_close(device->channel);
 
 cleanup:
-	free_netvsc_device(net_device);
+	free_netvsc_device(&net_device->rcu);
 
 	return ret;
 }
drivers/net/hyperv/netvsc_drv.c +43 -18

197 lines not on this page · raw message

Signature · 1 line
2.11.0
author
#5 lore raw

PATCH 5/9 netvsc: uses RCU instead of removal flag

In reply to Stephen Hemminger

It is cleaner to use RCU protected pointer (nvdev_ctx->nvdev) to indicate device is in removed state, rather than having a separate boolean flag. By using the pointer the context can be checked by static checkers and dynamic lockdep.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/hyperv/hyperv_net.h |  3 ---
 drivers/net/hyperv/netvsc.c     |  4 ----
 drivers/net/hyperv/netvsc_drv.c | 34 ++++++++++------------------------
 3 files changed, 10 insertions(+), 31 deletions(-)
drivers/net/hyperv/hyperv_net.h +0 -3
index 0ade21f95d71..907f55960ba8 100644
@@ -708,9 +708,6 @@ struct net_device_context {
 	u32 speed;
 	struct netvsc_ethtool_stats eth_stats;
 
-	/* the device is going away */
-	bool start_remove;
-
 	/* State to manage the associated VF interface. */
 	struct net_device __rcu *vf_netdev;
drivers/net/hyperv/netvsc.c +0 -4
index ab9118d620ab..1f17d948f9b0 100644
@@ -605,7 +605,6 @@ static void netvsc_send_tx_complete(struct netvsc_device *net_device,
 {
 	struct sk_buff *skb = (struct sk_buff *)(unsigned long)desc->trans_id;
 	struct net_device *ndev = hv_get_drvdata(device);
-	struct net_device_context *net_device_ctx = netdev_priv(ndev);
 	struct vmbus_channel *channel = device->channel;
 	u16 q_idx = 0;
 	int queue_sends;

@@ -639,7 +638,6 @@ static void netvsc_send_tx_complete(struct netvsc_device *net_device,
 		wake_up(&net_device->wait_drain);
 
 	if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx)) &&
-	    !net_device_ctx->start_remove &&
 	    (hv_ringbuf_avail_percent(&channel->outbound) > RING_AVAIL_PERCENT_HIWATER ||
 	     queue_sends < 1))
 		netif_tx_wake_queue(netdev_get_tx_queue(ndev, q_idx));

@@ -1326,8 +1324,6 @@ int netvsc_device_add(struct hv_device *device,
 	/* Writing nvdev pointer unlocks netvsc_send(), make sure chn_table is
 	 * populated.
 	 */
-	wmb();
-
 	rcu_assign_pointer(net_device_ctx->nvdev, net_device);
 
 	/* Connect with the NetVsp */
drivers/net/hyperv/netvsc_drv.c +10 -24
index d8a70d07eeec..eb7ae79d47bb 100644
@@ -760,7 +760,7 @@ static int netvsc_set_channels(struct net_device *net,
 	if (count > net->num_tx_queues || count > net->num_rx_queues)
 		return -EINVAL;
 
-	if (net_device_ctx->start_remove || !nvdev || nvdev->destroy)
+	if (!nvdev || nvdev->destroy)
 		return -ENODEV;
 
 	if (nvdev->nvsp_version < NVSP_PROTOCOL_VERSION_5)

@@ -776,7 +776,6 @@ static int netvsc_set_channels(struct net_device *net,
 			return ret;
 	}
 
-	net_device_ctx->start_remove = true;
 	rndis_filter_device_remove(dev, nvdev);
 
 	ret = netvsc_set_queues(net, dev, count);

@@ -785,8 +784,6 @@ static int netvsc_set_channels(struct net_device *net,
 	else
 		netvsc_set_queues(net, dev, nvdev->num_chn);
 
-	net_device_ctx->start_remove = false;
-
 	if (was_running)
 		ret = netvsc_open(net);
 

@@ -860,7 +857,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	bool was_running;
 	int ret;
 
-	if (ndevctx->start_remove || !nvdev || nvdev->destroy)
+	if (!nvdev || nvdev->destroy)
 		return -ENODEV;
 
 	was_running = netif_running(ndev);

@@ -875,7 +872,6 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	device_info.num_chn = nvdev->num_chn;
 	device_info.max_num_vrss_chns = nvdev->num_chn;
 
-	ndevctx->start_remove = true;
 	rndis_filter_device_remove(hdev, nvdev);
 
 	/* 'nvdev' has been freed in rndis_filter_device_remove() ->

@@ -888,8 +884,6 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 
 	rndis_filter_device_add(hdev, &device_info);
 
-	ndevctx->start_remove = false;
-
 	if (was_running)
 		ret = netvsc_open(ndev);
 

@@ -1245,10 +1239,10 @@ static void netvsc_link_change(struct work_struct *w)
 	unsigned long flags, next_reconfig, delay;
 
 	rtnl_lock();
-	if (ndev_ctx->start_remove)
+	net_device = rtnl_dereference(ndev_ctx->nvdev);
+	if (!net_device)
 		goto out_unlock;
 
-	net_device = rtnl_dereference(ndev_ctx->nvdev);
 	rdev = net_device->extension;
 
 	next_reconfig = ndev_ctx->last_reconfig + LINKCHANGE_INT;

@@ -1509,8 +1503,6 @@ static int netvsc_probe(struct hv_device *dev,
 
 	hv_set_drvdata(dev, net);
 
-	net_device_ctx->start_remove = false;
-
 	INIT_DELAYED_WORK(&net_device_ctx->dwork, netvsc_link_change);
 	INIT_WORK(&net_device_ctx->work, do_set_multicast);
 

@@ -1579,26 +1571,20 @@ static int netvsc_remove(struct hv_device *dev)
 
 	ndev_ctx = netdev_priv(net);
 
-	/* Avoid racing with netvsc_change_mtu()/netvsc_set_channels()
-	 * removing the device.
-	 */
-	rtnl_lock();
-	ndev_ctx->start_remove = true;
-	rtnl_unlock();
+	netif_device_detach(net);
 
 	cancel_delayed_work_sync(&ndev_ctx->dwork);
 	cancel_work_sync(&ndev_ctx->work);
 
-	/* Stop outbound asap */
-	netif_tx_disable(net);
-
-	unregister_netdev(net);
-
 	/*
 	 * Call to the vsc driver to let it know that the device is being
-	 * removed
+	 * removed. Also blocks mtu and channel changes.
 	 */
+	rtnl_lock();
 	rndis_filter_device_remove(dev, ndev_ctx->nvdev);
+	rtnl_unlock();
+
+	unregister_netdev(net);
 
 	hv_set_drvdata(dev, NULL);
Signature · 1 line
2.11.0
author
#6 lore raw

PATCH 6/9 netvsc: use refcount_t for keeping track of sub channels

In reply to Stephen Hemminger

Rather than a lock and variable, use a refcount_t to keep track of the number of sub channels. Don't need to wait for subchannels on device removal since wait was already done in device_add.

Also fix the error handling; don't wait forever in case of an error on request to create sub channels.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/hyperv/hyperv_net.h   |  4 ++--
 drivers/net/hyperv/rndis_filter.c | 41 ++++++++++-----------------------------
 2 files changed, 12 insertions(+), 33 deletions(-)
drivers/net/hyperv/hyperv_net.h +2 -2
index 907f55960ba8..4747ad48b3cc 100644
@@ -761,8 +761,8 @@ struct netvsc_device {
 
 	u32 max_chn;
 	u32 num_chn;
-	spinlock_t sc_lock; /* Protects num_sc_offered variable */
-	u32 num_sc_offered;
+
+	refcount_t sc_offered;
 
 	/* Holds rndis device info */
 	void *extension;
drivers/net/hyperv/rndis_filter.c +10 -31

96 lines not on this page · raw message

Signature · 1 line
2.11.0
author
#7 lore raw

PATCH 7/9 netvsc: remove unnecessary lock on shutdown

In reply to Stephen Hemminger

The channel inbound lock was not being used at all by the netvsc device, but the spin_lock was helpful by providing necessary barrier before waiting.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/hyperv/rndis_filter.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/net/hyperv/rndis_filter.c +3 -4
index 3bd5447277ad..cd7b83707e04 100644
@@ -926,8 +926,6 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
 	struct rndis_halt_request *halt;
 	struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
 	struct netvsc_device *nvdev = net_device_ctx->nvdev;
-	struct hv_device *hdev = net_device_ctx->device_ctx;
-	ulong flags;
 
 	/* Attempt to do a rndis device halt */
 	request = get_rndis_request(dev, RNDIS_MSG_HALT,

@@ -945,9 +943,10 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
 	dev->state = RNDIS_DEV_UNINITIALIZED;
 
 cleanup:
-	spin_lock_irqsave(&hdev->channel->inbound_lock, flags);
 	nvdev->destroy = true;
-	spin_unlock_irqrestore(&hdev->channel->inbound_lock, flags);
+
+	/* Force flag to be ordered before waiting */
+	wmb();
 
 	/* Wait for all send completions */
 	wait_event(nvdev->wait_drain, netvsc_device_idle(nvdev));
Signature · 1 line
2.11.0
author
#8 lore raw

PATCH 8/9 netvsc: eliminate unnecessary skb == NULL checks

In reply to Stephen Hemminger

Since there already is a special case goto for control messages (skb == NULL) in netvsc_send, there is no need for later checks in same code path.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/hyperv/netvsc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
drivers/net/hyperv/netvsc.c +3 -6
index 1f17d948f9b0..e998e2f7a619 100644
@@ -706,8 +706,7 @@ static u32 netvsc_copy_to_send_buf(struct netvsc_device *net_device,
 		packet->page_buf_cnt;
 
 	/* Add padding */
-	if (skb && skb->xmit_more && remain &&
-	    !packet->cp_partial) {
+	if (skb->xmit_more && remain && !packet->cp_partial) {
 		padding = net_device->pkt_align - remain;
 		rndis_msg->msg_len += padding;
 		packet->total_data_buflen += padding;

@@ -865,9 +864,7 @@ int netvsc_send(struct hv_device *device,
 	if (msdp->pkt)
 		msd_len = msdp->pkt->total_data_buflen;
 
-	try_batch = (skb != NULL) && msd_len > 0 && msdp->count <
-		    net_device->max_pkt;
-
+	try_batch =  msd_len > 0 && msdp->count < net_device->max_pkt;
 	if (try_batch && msd_len + pktlen + net_device->pkt_align <
 	    net_device->send_section_size) {
 		section_index = msdp->pkt->send_buf_index;

@@ -877,7 +874,7 @@ int netvsc_send(struct hv_device *device,
 		section_index = msdp->pkt->send_buf_index;
 		packet->cp_partial = true;
 
-	} else if ((skb != NULL) && pktlen + net_device->pkt_align <
+	} else if (pktlen + net_device->pkt_align <
 		   net_device->send_section_size) {
 		section_index = netvsc_get_next_send_section(net_device);
 		if (section_index != NETVSC_INVALID_INDEX) {
Signature · 1 line
2.11.0
author
#9 lore raw

PATCH 9/9 netvsc: fix and cleanup rndis_filter_set_packet_filter

In reply to Stephen Hemminger

Fix warning from unused set_complete variable. And rearrange code to eliminate unnecessary goto's.

Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/hyperv/rndis_filter.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)
drivers/net/hyperv/rndis_filter.c +6 -12
index cd7b83707e04..91b3bcfd9acb 100644
@@ -819,16 +819,14 @@ int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter)
 {
 	struct rndis_request *request;
 	struct rndis_set_request *set;
-	struct rndis_set_complete *set_complete;
 	int ret;
 
 	request = get_rndis_request(dev, RNDIS_MSG_SET,
 			RNDIS_MESSAGE_SIZE(struct rndis_set_request) +
 			sizeof(u32));
-	if (!request) {
-		ret = -ENOMEM;
-		goto cleanup;
-	}
+	if (!request)
+		return -ENOMEM;
+
 
 	/* Setup the rndis set */
 	set = &request->request_msg.msg.set_req;

@@ -840,15 +838,11 @@ int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter)
 	       &new_filter, sizeof(u32));
 
 	ret = rndis_filter_send_request(dev, request);
-	if (ret != 0)
-		goto cleanup;
+	if (ret == 0)
+		wait_for_completion(&request->wait_event);
 
-	wait_for_completion(&request->wait_event);
+	put_rndis_request(dev, request);
 
-	set_complete = &request->response_msg.msg.set_complete;
-cleanup:
-	if (request)
-		put_rndis_request(dev, request);
 	return ret;
 }
Signature · 1 line
2.11.0