* [PATCH net-next v2 1/6] hv_netvsc: move start_remove flag to net_device_context
2016-05-13 11:55 [PATCH net-next v2 0/6] hv_netvsc: avoid races on mtu change/set channels Vitaly Kuznetsov
@ 2016-05-13 11:55 ` Vitaly Kuznetsov
2016-05-13 11:55 ` [PATCH net-next v2 2/6] hv_netvsc: use start_remove flag to protect netvsc_link_change() Vitaly Kuznetsov
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Vitaly Kuznetsov @ 2016-05-13 11:55 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, devel, Haiyang Zhang, K. Y. Srinivasan, Lino Sanfilippo
struct netvsc_device is destroyed on mtu change so keeping the
protection flag there is not a good idea. Move it to struct
net_device_context which is preserved.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/net/hyperv/hyperv_net.h | 4 +++-
drivers/net/hyperv/netvsc.c | 3 +--
drivers/net/hyperv/netvsc_drv.c | 12 +++++++++---
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 6700a4d..18e9cc8 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -672,6 +672,9 @@ struct net_device_context {
/* Ethtool settings */
u8 duplex;
u32 speed;
+
+ /* the device is going away */
+ bool start_remove;
};
/* Per netvsc device */
@@ -682,7 +685,6 @@ struct netvsc_device {
atomic_t num_outstanding_sends;
wait_queue_head_t wait_drain;
- bool start_remove;
bool destroy;
/* Receive buffer allocated by us but manages by NetVSP */
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index eddce3c..5e2017b 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -74,7 +74,6 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
}
init_waitqueue_head(&net_device->wait_drain);
- net_device->start_remove = false;
net_device->destroy = false;
atomic_set(&net_device->open_cnt, 0);
atomic_set(&net_device->vf_use_cnt, 0);
@@ -691,7 +690,7 @@ static void netvsc_send_completion(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->start_remove &&
+ !net_device->nd_ctx->start_remove &&
(hv_ringbuf_avail_percent(&channel->outbound) >
RING_AVAIL_PERCENT_HIWATER || queue_sends < 1))
netif_tx_wake_queue(netdev_get_tx_queue(
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index ba3f3f3..b3fa2cd 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -793,7 +793,7 @@ static int netvsc_set_channels(struct net_device *net,
goto out;
do_set:
- nvdev->start_remove = true;
+ net_device_ctx->start_remove = true;
rndis_filter_device_remove(dev);
nvdev->num_chn = channels->combined_count;
@@ -837,6 +837,7 @@ static int netvsc_set_channels(struct net_device *net,
out:
netvsc_open(net);
+ net_device_ctx->start_remove = false;
return ret;
@@ -927,7 +928,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
num_chn = nvdev->num_chn;
- nvdev->start_remove = true;
+ ndevctx->start_remove = true;
rndis_filter_device_remove(hdev);
ndev->mtu = mtu;
@@ -943,6 +944,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
out:
netvsc_open(ndev);
+ ndevctx->start_remove = false;
return ret;
}
@@ -1358,6 +1360,9 @@ 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);
INIT_WORK(&net_device_ctx->gwrk.dwrk, netvsc_notify_peers);
@@ -1419,9 +1424,10 @@ static int netvsc_remove(struct hv_device *dev)
return 0;
}
- net_device->start_remove = true;
ndev_ctx = netdev_priv(net);
+ ndev_ctx->start_remove = true;
+
cancel_delayed_work_sync(&ndev_ctx->dwork);
cancel_work_sync(&ndev_ctx->work);
--
2.5.5
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net-next v2 2/6] hv_netvsc: use start_remove flag to protect netvsc_link_change()
2016-05-13 11:55 [PATCH net-next v2 0/6] hv_netvsc: avoid races on mtu change/set channels Vitaly Kuznetsov
2016-05-13 11:55 ` [PATCH net-next v2 1/6] hv_netvsc: move start_remove flag to net_device_context Vitaly Kuznetsov
@ 2016-05-13 11:55 ` Vitaly Kuznetsov
2016-05-13 11:55 ` [PATCH net-next v2 3/6] hv_netvsc: untangle the pointer mess Vitaly Kuznetsov
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Vitaly Kuznetsov @ 2016-05-13 11:55 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, devel, Haiyang Zhang, K. Y. Srinivasan, Lino Sanfilippo
netvsc_link_change() can race with netvsc_change_mtu() or
netvsc_set_channels() as these functions destroy struct netvsc_device and
rndis filter. Use start_remove flag for syncronization. As
netvsc_change_mtu()/netvsc_set_channels() are called with rtnl lock held
we need to take it before checking start_remove value in
netvsc_link_change().
Reported-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/net/hyperv/netvsc_drv.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index b3fa2cd..01de2dc 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -838,6 +838,8 @@ static int netvsc_set_channels(struct net_device *net,
out:
netvsc_open(net);
net_device_ctx->start_remove = false;
+ /* We may have missed link change notifications */
+ schedule_delayed_work(&net_device_ctx->dwork, 0);
return ret;
@@ -946,6 +948,9 @@ out:
netvsc_open(ndev);
ndevctx->start_remove = false;
+ /* We may have missed link change notifications */
+ schedule_delayed_work(&ndevctx->dwork, 0);
+
return ret;
}
@@ -1066,6 +1071,11 @@ static void netvsc_link_change(struct work_struct *w)
unsigned long flags, next_reconfig, delay;
ndev_ctx = container_of(w, struct net_device_context, dwork.work);
+
+ rtnl_lock();
+ if (ndev_ctx->start_remove)
+ goto out_unlock;
+
net_device = hv_get_drvdata(ndev_ctx->device_ctx);
rdev = net_device->extension;
net = net_device->ndev;
@@ -1079,7 +1089,7 @@ static void netvsc_link_change(struct work_struct *w)
delay = next_reconfig - jiffies;
delay = delay < LINKCHANGE_INT ? delay : LINKCHANGE_INT;
schedule_delayed_work(&ndev_ctx->dwork, delay);
- return;
+ goto out_unlock;
}
ndev_ctx->last_reconfig = jiffies;
@@ -1093,9 +1103,7 @@ static void netvsc_link_change(struct work_struct *w)
spin_unlock_irqrestore(&ndev_ctx->lock, flags);
if (!event)
- return;
-
- rtnl_lock();
+ goto out_unlock;
switch (event->event) {
/* Only the following events are possible due to the check in
@@ -1144,6 +1152,11 @@ static void netvsc_link_change(struct work_struct *w)
*/
if (reschedule)
schedule_delayed_work(&ndev_ctx->dwork, LINKCHANGE_INT);
+
+ return;
+
+out_unlock:
+ rtnl_unlock();
}
static void netvsc_free_netdev(struct net_device *netdev)
--
2.5.5
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net-next v2 3/6] hv_netvsc: untangle the pointer mess
2016-05-13 11:55 [PATCH net-next v2 0/6] hv_netvsc: avoid races on mtu change/set channels Vitaly Kuznetsov
2016-05-13 11:55 ` [PATCH net-next v2 1/6] hv_netvsc: move start_remove flag to net_device_context Vitaly Kuznetsov
2016-05-13 11:55 ` [PATCH net-next v2 2/6] hv_netvsc: use start_remove flag to protect netvsc_link_change() Vitaly Kuznetsov
@ 2016-05-13 11:55 ` Vitaly Kuznetsov
2016-05-13 11:55 ` [PATCH net-next v2 4/6] hv_netvsc: get rid of struct net_device pointer in struct netvsc_device Vitaly Kuznetsov
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Vitaly Kuznetsov @ 2016-05-13 11:55 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, devel, Haiyang Zhang, K. Y. Srinivasan, Lino Sanfilippo
We have the following structures keeping netvsc adapter state:
- struct net_device
- struct net_device_context
- struct netvsc_device
- struct rndis_device
- struct hv_device
and there are pointers/dependencies between them:
- struct net_device_context is contained in struct net_device
- struct hv_device has driver_data pointer which points to
'struct net_device' OR 'struct netvsc_device' depending on driver's
state (!).
- struct net_device_context has a pointer to 'struct hv_device'.
- struct netvsc_device has pointers to 'struct hv_device' and
'struct net_device_context'.
- struct rndis_device has a pointer to 'struct netvsc_device'.
Different functions get different structures as parameters and use these
pointers for traveling. The problem is (in addition to keeping in mind
this complex graph) that some of these structures (struct netvsc_device
and struct rndis_device) are being removed and re-created on mtu change
(as we implement it as re-creation of hyper-v device) so our travel using
these pointers is dangerous.
Simplify this to a the following:
- add struct netvsc_device pointer to struct net_device_context (which is
a part of struct net_device and thus never disappears)
- remove struct hv_device and struct net_device_context pointers from
struct netvsc_device
- replace pointer to 'struct netvsc_device' with pointer to
'struct net_device'.
- always keep 'struct net_device' in hv_device driver_data.
We'll end up with the following 'circular' structure:
net_device:
[net_device_context] -> netvsc_device -> rndis_device -> net_device
-> hv_device -> net_device
On MTU change we'll be removing the 'netvsc_device -> rndis_device'
branch and re-creating it making the synchronization easier.
There is one additional redundant pointer left, it is struct net_device
link in struct netvsc_device, it is going to be removed in a separate
commit.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/net/hyperv/hyperv_net.h | 10 ++---
drivers/net/hyperv/netvsc.c | 82 ++++++++++++++++-----------------------
drivers/net/hyperv/netvsc_drv.c | 51 ++++++++++--------------
drivers/net/hyperv/rndis_filter.c | 78 +++++++++++++++++++------------------
4 files changed, 99 insertions(+), 122 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 18e9cc8..0f38743 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -158,7 +158,7 @@ enum rndis_device_state {
};
struct rndis_device {
- struct netvsc_device *net_dev;
+ struct net_device *ndev;
enum rndis_device_state state;
bool link_state;
@@ -173,6 +173,7 @@ struct rndis_device {
/* Interface */
struct rndis_message;
+struct netvsc_device;
int netvsc_device_add(struct hv_device *device, void *additional_info);
int netvsc_device_remove(struct hv_device *device);
int netvsc_send(struct hv_device *device,
@@ -653,6 +654,8 @@ struct garp_wrk {
struct net_device_context {
/* point back to our device context */
struct hv_device *device_ctx;
+ /* netvsc_device */
+ struct netvsc_device *nvdev;
/* reconfigure work */
struct delayed_work dwork;
/* last reconfig time */
@@ -679,8 +682,6 @@ struct net_device_context {
/* Per netvsc device */
struct netvsc_device {
- struct hv_device *dev;
-
u32 nvsp_version;
atomic_t num_outstanding_sends;
@@ -734,9 +735,6 @@ struct netvsc_device {
u32 max_pkt; /* max number of pkt in one send, e.g. 8 */
u32 pkt_align; /* alignment bytes, e.g. 8 */
- /* The net device context */
- struct net_device_context *nd_ctx;
-
/* 1: allocated, serial number is valid. 0: not allocated */
u32 vf_alloc;
/* Serial number of the VF to team with */
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 5e2017b..1cd01ad 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -40,7 +40,9 @@
void netvsc_switch_datapath(struct netvsc_device *nv_dev, bool vf)
{
struct nvsp_message *init_pkt = &nv_dev->channel_init_pkt;
- struct hv_device *dev = nv_dev->dev;
+ struct net_device *ndev = nv_dev->ndev;
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct hv_device *dev = net_device_ctx->device_ctx;
memset(init_pkt, 0, sizeof(struct nvsp_message));
init_pkt->hdr.msg_type = NVSP_MSG4_TYPE_SWITCH_DATA_PATH;
@@ -62,6 +64,7 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
{
struct netvsc_device *net_device;
struct net_device *ndev = hv_get_drvdata(device);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
net_device = kzalloc(sizeof(struct netvsc_device), GFP_KERNEL);
if (!net_device)
@@ -77,7 +80,6 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
net_device->destroy = false;
atomic_set(&net_device->open_cnt, 0);
atomic_set(&net_device->vf_use_cnt, 0);
- net_device->dev = device;
net_device->ndev = ndev;
net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT;
net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT;
@@ -85,7 +87,8 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
net_device->vf_netdev = NULL;
net_device->vf_inject = false;
- hv_set_drvdata(device, net_device);
+ net_device_ctx->nvdev = net_device;
+
return net_device;
}
@@ -97,9 +100,10 @@ static void free_netvsc_device(struct netvsc_device *nvdev)
static struct netvsc_device *get_outbound_net_device(struct hv_device *device)
{
- struct netvsc_device *net_device;
+ struct net_device *ndev = hv_get_drvdata(device);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *net_device = net_device_ctx->nvdev;
- net_device = hv_get_drvdata(device);
if (net_device && net_device->destroy)
net_device = NULL;
@@ -108,9 +112,9 @@ static struct netvsc_device *get_outbound_net_device(struct hv_device *device)
static struct netvsc_device *get_inbound_net_device(struct hv_device *device)
{
- struct netvsc_device *net_device;
-
- net_device = hv_get_drvdata(device);
+ struct net_device *ndev = hv_get_drvdata(device);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *net_device = net_device_ctx->nvdev;
if (!net_device)
goto get_in_err;
@@ -124,11 +128,13 @@ get_in_err:
}
-static int netvsc_destroy_buf(struct netvsc_device *net_device)
+static int netvsc_destroy_buf(struct hv_device *device)
{
struct nvsp_message *revoke_packet;
int ret = 0;
- struct net_device *ndev = net_device->ndev;
+ struct net_device *ndev = hv_get_drvdata(device);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *net_device = net_device_ctx->nvdev;
/*
* If we got a section count, it means we received a
@@ -146,7 +152,7 @@ static int netvsc_destroy_buf(struct netvsc_device *net_device)
revoke_packet->msg.v1_msg.
revoke_recv_buf.id = NETVSC_RECEIVE_BUFFER_ID;
- ret = vmbus_sendpacket(net_device->dev->channel,
+ ret = vmbus_sendpacket(device->channel,
revoke_packet,
sizeof(struct nvsp_message),
(unsigned long)revoke_packet,
@@ -164,8 +170,8 @@ static int netvsc_destroy_buf(struct netvsc_device *net_device)
/* Teardown the gpadl on the vsp end */
if (net_device->recv_buf_gpadl_handle) {
- ret = vmbus_teardown_gpadl(net_device->dev->channel,
- net_device->recv_buf_gpadl_handle);
+ ret = vmbus_teardown_gpadl(device->channel,
+ net_device->recv_buf_gpadl_handle);
/* If we failed here, we might as well return and have a leak
* rather than continue and a bugchk
@@ -206,7 +212,7 @@ static int netvsc_destroy_buf(struct netvsc_device *net_device)
revoke_packet->msg.v1_msg.revoke_send_buf.id =
NETVSC_SEND_BUFFER_ID;
- ret = vmbus_sendpacket(net_device->dev->channel,
+ ret = vmbus_sendpacket(device->channel,
revoke_packet,
sizeof(struct nvsp_message),
(unsigned long)revoke_packet,
@@ -222,7 +228,7 @@ static int netvsc_destroy_buf(struct netvsc_device *net_device)
}
/* Teardown the gpadl on the vsp end */
if (net_device->send_buf_gpadl_handle) {
- ret = vmbus_teardown_gpadl(net_device->dev->channel,
+ ret = vmbus_teardown_gpadl(device->channel,
net_device->send_buf_gpadl_handle);
/* If we failed here, we might as well return and have a leak
@@ -434,7 +440,7 @@ static int netvsc_init_buf(struct hv_device *device)
goto exit;
cleanup:
- netvsc_destroy_buf(net_device);
+ netvsc_destroy_buf(device);
exit:
return ret;
@@ -565,9 +571,9 @@ cleanup:
return ret;
}
-static void netvsc_disconnect_vsp(struct netvsc_device *net_device)
+static void netvsc_disconnect_vsp(struct hv_device *device)
{
- netvsc_destroy_buf(net_device);
+ netvsc_destroy_buf(device);
}
/*
@@ -575,24 +581,13 @@ static void netvsc_disconnect_vsp(struct netvsc_device *net_device)
*/
int netvsc_device_remove(struct hv_device *device)
{
- struct netvsc_device *net_device;
- unsigned long flags;
-
- net_device = hv_get_drvdata(device);
-
- netvsc_disconnect_vsp(net_device);
+ struct net_device *ndev = hv_get_drvdata(device);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *net_device = net_device_ctx->nvdev;
- /*
- * Since we have already drained, we don't need to busy wait
- * as was done in final_release_stor_device()
- * Note that we cannot set the ext pointer to NULL until
- * we have drained - to drain the outgoing packets, we need to
- * allow incoming packets.
- */
+ netvsc_disconnect_vsp(device);
- spin_lock_irqsave(&device->channel->inbound_lock, flags);
- hv_set_drvdata(device, NULL);
- spin_unlock_irqrestore(&device->channel->inbound_lock, flags);
+ net_device_ctx->nvdev = NULL;
/*
* At this point, no one should be accessing net_device
@@ -640,12 +635,11 @@ static void netvsc_send_completion(struct netvsc_device *net_device,
{
struct nvsp_message *nvsp_packet;
struct hv_netvsc_packet *nvsc_packet;
- struct net_device *ndev;
+ struct net_device *ndev = hv_get_drvdata(device);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
u32 send_index;
struct sk_buff *skb;
- ndev = net_device->ndev;
-
nvsp_packet = (struct nvsp_message *)((unsigned long)packet +
(packet->offset8 << 3));
@@ -690,7 +684,7 @@ static void netvsc_send_completion(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->nd_ctx->start_remove &&
+ !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(
@@ -1264,17 +1258,7 @@ int netvsc_device_add(struct hv_device *device, void *additional_info)
net_device->ring_size = ring_size;
- /*
- * Coming into this function, struct net_device * is
- * registered as the driver private data.
- * In alloc_net_device(), we register struct netvsc_device *
- * as the driver private data and stash away struct net_device *
- * in struct netvsc_device *.
- */
- ndev = net_device->ndev;
-
- /* Add netvsc_device context to netvsc_device */
- net_device->nd_ctx = netdev_priv(ndev);
+ ndev = hv_get_drvdata(device);
/* Initialize the NetVSC channel extension */
init_completion(&net_device->channel_init_wait);
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 01de2dc..a33a1c9 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -70,7 +70,7 @@ static void do_set_multicast(struct work_struct *w)
struct netvsc_device *nvdev;
struct rndis_device *rdev;
- nvdev = hv_get_drvdata(ndevctx->device_ctx);
+ nvdev = ndevctx->nvdev;
if (nvdev == NULL || nvdev->ndev == NULL)
return;
@@ -99,7 +99,7 @@ static int netvsc_open(struct net_device *net)
{
struct net_device_context *net_device_ctx = netdev_priv(net);
struct hv_device *device_obj = net_device_ctx->device_ctx;
- struct netvsc_device *nvdev;
+ struct netvsc_device *nvdev = net_device_ctx->nvdev;
struct rndis_device *rdev;
int ret = 0;
@@ -114,7 +114,6 @@ static int netvsc_open(struct net_device *net)
netif_tx_wake_all_queues(net);
- nvdev = hv_get_drvdata(device_obj);
rdev = nvdev->extension;
if (!rdev->link_state)
netif_carrier_on(net);
@@ -126,7 +125,7 @@ static int netvsc_close(struct net_device *net)
{
struct net_device_context *net_device_ctx = netdev_priv(net);
struct hv_device *device_obj = net_device_ctx->device_ctx;
- struct netvsc_device *nvdev = hv_get_drvdata(device_obj);
+ struct netvsc_device *nvdev = net_device_ctx->nvdev;
int ret;
u32 aread, awrite, i, msec = 10, retry = 0, retry_max = 20;
struct vmbus_channel *chn;
@@ -205,8 +204,7 @@ static u16 netvsc_select_queue(struct net_device *ndev, struct sk_buff *skb,
void *accel_priv, select_queue_fallback_t fallback)
{
struct net_device_context *net_device_ctx = netdev_priv(ndev);
- struct hv_device *hdev = net_device_ctx->device_ctx;
- struct netvsc_device *nvsc_dev = hv_get_drvdata(hdev);
+ struct netvsc_device *nvsc_dev = net_device_ctx->nvdev;
u32 hash;
u16 q_idx = 0;
@@ -580,7 +578,6 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,
struct rndis_indicate_status *indicate = &resp->msg.indicate_status;
struct net_device *net;
struct net_device_context *ndev_ctx;
- struct netvsc_device *net_device;
struct netvsc_reconfig *event;
unsigned long flags;
@@ -590,8 +587,7 @@ void netvsc_linkstatus_callback(struct hv_device *device_obj,
indicate->status != RNDIS_STATUS_MEDIA_DISCONNECT)
return;
- net_device = hv_get_drvdata(device_obj);
- net = net_device->ndev;
+ net = hv_get_drvdata(device_obj);
if (!net || net->reg_state != NETREG_REGISTERED)
return;
@@ -659,16 +655,15 @@ int netvsc_recv_callback(struct hv_device *device_obj,
struct vmbus_channel *channel,
u16 vlan_tci)
{
- struct net_device *net;
- struct net_device_context *net_device_ctx;
+ struct net_device *net = hv_get_drvdata(device_obj);
+ struct net_device_context *net_device_ctx = netdev_priv(net);
struct sk_buff *skb;
struct sk_buff *vf_skb;
struct netvsc_stats *rx_stats;
- struct netvsc_device *netvsc_dev = hv_get_drvdata(device_obj);
+ struct netvsc_device *netvsc_dev = net_device_ctx->nvdev;
u32 bytes_recvd = packet->total_data_buflen;
int ret = 0;
- net = netvsc_dev->ndev;
if (!net || net->reg_state != NETREG_REGISTERED)
return NVSP_STAT_FAIL;
@@ -743,8 +738,7 @@ static void netvsc_get_channels(struct net_device *net,
struct ethtool_channels *channel)
{
struct net_device_context *net_device_ctx = netdev_priv(net);
- struct hv_device *dev = net_device_ctx->device_ctx;
- struct netvsc_device *nvdev = hv_get_drvdata(dev);
+ struct netvsc_device *nvdev = net_device_ctx->nvdev;
if (nvdev) {
channel->max_combined = nvdev->max_chn;
@@ -757,7 +751,7 @@ static int netvsc_set_channels(struct net_device *net,
{
struct net_device_context *net_device_ctx = netdev_priv(net);
struct hv_device *dev = net_device_ctx->device_ctx;
- struct netvsc_device *nvdev = hv_get_drvdata(dev);
+ struct netvsc_device *nvdev = net_device_ctx->nvdev;
struct netvsc_device_info device_info;
u32 num_chn;
u32 max_chn;
@@ -798,9 +792,6 @@ static int netvsc_set_channels(struct net_device *net,
nvdev->num_chn = channels->combined_count;
- net_device_ctx->device_ctx = dev;
- hv_set_drvdata(dev, net);
-
memset(&device_info, 0, sizeof(device_info));
device_info.num_chn = nvdev->num_chn; /* passed to RNDIS */
device_info.ring_size = ring_size;
@@ -815,7 +806,7 @@ static int netvsc_set_channels(struct net_device *net,
goto recover;
}
- nvdev = hv_get_drvdata(dev);
+ nvdev = net_device_ctx->nvdev;
ret = netif_set_real_num_tx_queues(net, nvdev->num_chn);
if (ret) {
@@ -908,8 +899,8 @@ static int netvsc_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
static int netvsc_change_mtu(struct net_device *ndev, int mtu)
{
struct net_device_context *ndevctx = netdev_priv(ndev);
- struct hv_device *hdev = ndevctx->device_ctx;
- struct netvsc_device *nvdev = hv_get_drvdata(hdev);
+ struct netvsc_device *nvdev = ndevctx->nvdev;
+ struct hv_device *hdev = ndevctx->device_ctx;
struct netvsc_device_info device_info;
int limit = ETH_DATA_LEN;
u32 num_chn;
@@ -935,9 +926,6 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
ndev->mtu = mtu;
- ndevctx->device_ctx = hdev;
- hv_set_drvdata(hdev, ndev);
-
memset(&device_info, 0, sizeof(device_info));
device_info.ring_size = ring_size;
device_info.num_chn = num_chn;
@@ -1076,7 +1064,7 @@ static void netvsc_link_change(struct work_struct *w)
if (ndev_ctx->start_remove)
goto out_unlock;
- net_device = hv_get_drvdata(ndev_ctx->device_ctx);
+ net_device = ndev_ctx->nvdev;
rdev = net_device->extension;
net = net_device->ndev;
@@ -1201,7 +1189,7 @@ static struct netvsc_device *get_netvsc_device(char *mac)
if (netvsc_ctx == NULL)
return NULL;
- return hv_get_drvdata(netvsc_ctx->device_ctx);
+ return netvsc_ctx->nvdev;
}
static int netvsc_register_vf(struct net_device *vf_netdev)
@@ -1407,7 +1395,7 @@ static int netvsc_probe(struct hv_device *dev,
}
memcpy(net->dev_addr, device_info.mac_adr, ETH_ALEN);
- nvdev = hv_get_drvdata(dev);
+ nvdev = net_device_ctx->nvdev;
netif_set_real_num_tx_queues(net, nvdev->num_chn);
netif_set_real_num_rx_queues(net, nvdev->num_chn);
@@ -1429,8 +1417,7 @@ static int netvsc_remove(struct hv_device *dev)
struct net_device_context *ndev_ctx;
struct netvsc_device *net_device;
- net_device = hv_get_drvdata(dev);
- net = net_device->ndev;
+ net = hv_get_drvdata(dev);
if (net == NULL) {
dev_err(&dev->device, "No net device to remove\n");
@@ -1439,6 +1426,8 @@ static int netvsc_remove(struct hv_device *dev)
ndev_ctx = netdev_priv(net);
+ net_device = ndev_ctx->nvdev;
+
ndev_ctx->start_remove = true;
cancel_delayed_work_sync(&ndev_ctx->dwork);
@@ -1455,6 +1444,8 @@ static int netvsc_remove(struct hv_device *dev)
*/
rndis_filter_device_remove(dev);
+ hv_set_drvdata(dev, NULL);
+
netvsc_free_netdev(net);
return 0;
}
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index a59cdeb..6caba51 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -126,11 +126,7 @@ static void put_rndis_request(struct rndis_device *dev,
static void dump_rndis_message(struct hv_device *hv_dev,
struct rndis_message *rndis_msg)
{
- struct net_device *netdev;
- struct netvsc_device *net_device;
-
- net_device = hv_get_drvdata(hv_dev);
- netdev = net_device->ndev;
+ struct net_device *netdev = hv_get_drvdata(hv_dev);
switch (rndis_msg->ndis_msg_type) {
case RNDIS_MSG_PACKET:
@@ -211,6 +207,7 @@ static int rndis_filter_send_request(struct rndis_device *dev,
struct hv_netvsc_packet *packet;
struct hv_page_buffer page_buf[2];
struct hv_page_buffer *pb = page_buf;
+ struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
/* Setup the packet to send it */
packet = &req->pkt;
@@ -236,7 +233,7 @@ static int rndis_filter_send_request(struct rndis_device *dev,
pb[0].len;
}
- ret = netvsc_send(dev->net_dev->dev, packet, NULL, &pb, NULL);
+ ret = netvsc_send(net_device_ctx->device_ctx, packet, NULL, &pb, NULL);
return ret;
}
@@ -262,9 +259,7 @@ static void rndis_filter_receive_response(struct rndis_device *dev,
struct rndis_request *request = NULL;
bool found = false;
unsigned long flags;
- struct net_device *ndev;
-
- ndev = dev->net_dev->ndev;
+ struct net_device *ndev = dev->ndev;
spin_lock_irqsave(&dev->request_lock, flags);
list_for_each_entry(request, &dev->req_list, list_ent) {
@@ -355,6 +350,7 @@ static int rndis_filter_receive_data(struct rndis_device *dev,
struct ndis_pkt_8021q_info *vlan;
struct ndis_tcp_ip_checksum_info *csum_info;
u16 vlan_tci = 0;
+ struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
rndis_pkt = &msg->msg.pkt;
@@ -368,7 +364,7 @@ static int rndis_filter_receive_data(struct rndis_device *dev,
* should be the data packet size plus the trailer padding size
*/
if (pkt->total_data_buflen < rndis_pkt->data_len) {
- netdev_err(dev->net_dev->ndev, "rndis message buffer "
+ netdev_err(dev->ndev, "rndis message buffer "
"overflow detected (got %u, min %u)"
"...dropping this message!\n",
pkt->total_data_buflen, rndis_pkt->data_len);
@@ -390,7 +386,7 @@ static int rndis_filter_receive_data(struct rndis_device *dev,
}
csum_info = rndis_get_ppi(rndis_pkt, TCPIP_CHKSUM_PKTINFO);
- return netvsc_recv_callback(dev->net_dev->dev, pkt, data,
+ return netvsc_recv_callback(net_device_ctx->device_ctx, pkt, data,
csum_info, channel, vlan_tci);
}
@@ -399,10 +395,11 @@ int rndis_filter_receive(struct hv_device *dev,
void **data,
struct vmbus_channel *channel)
{
- struct netvsc_device *net_dev = hv_get_drvdata(dev);
+ struct net_device *ndev = hv_get_drvdata(dev);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *net_dev = net_device_ctx->nvdev;
struct rndis_device *rndis_dev;
struct rndis_message *rndis_msg;
- struct net_device *ndev;
int ret = 0;
if (!net_dev) {
@@ -410,8 +407,6 @@ int rndis_filter_receive(struct hv_device *dev,
goto exit;
}
- ndev = net_dev->ndev;
-
/* Make sure the rndis device state is initialized */
if (!net_dev->extension) {
netdev_err(ndev, "got rndis message but no rndis device - "
@@ -430,7 +425,7 @@ int rndis_filter_receive(struct hv_device *dev,
rndis_msg = *data;
- if (netif_msg_rx_err(net_dev->nd_ctx))
+ if (netif_msg_rx_err(net_device_ctx))
dump_rndis_message(dev, rndis_msg);
switch (rndis_msg->ndis_msg_type) {
@@ -550,9 +545,10 @@ static int rndis_filter_query_device_mac(struct rndis_device *dev)
int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac)
{
- struct netvsc_device *nvdev = hv_get_drvdata(hdev);
+ struct net_device *ndev = hv_get_drvdata(hdev);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *nvdev = net_device_ctx->nvdev;
struct rndis_device *rdev = nvdev->extension;
- struct net_device *ndev = nvdev->ndev;
struct rndis_request *request;
struct rndis_set_request *set;
struct rndis_config_parameter_info *cpi;
@@ -629,9 +625,10 @@ static int
rndis_filter_set_offload_params(struct hv_device *hdev,
struct ndis_offload_params *req_offloads)
{
- struct netvsc_device *nvdev = hv_get_drvdata(hdev);
+ struct net_device *ndev = hv_get_drvdata(hdev);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *nvdev = net_device_ctx->nvdev;
struct rndis_device *rdev = nvdev->extension;
- struct net_device *ndev = nvdev->ndev;
struct rndis_request *request;
struct rndis_set_request *set;
struct ndis_offload_params *offload_params;
@@ -703,7 +700,7 @@ u8 netvsc_hash_key[HASH_KEYLEN] = {
static int rndis_filter_set_rss_param(struct rndis_device *rdev, int num_queue)
{
- struct net_device *ndev = rdev->net_dev->ndev;
+ struct net_device *ndev = rdev->ndev;
struct rndis_request *request;
struct rndis_set_request *set;
struct rndis_set_complete *set_complete;
@@ -799,9 +796,7 @@ int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter)
u32 status;
int ret;
unsigned long t;
- struct net_device *ndev;
-
- ndev = dev->net_dev->ndev;
+ struct net_device *ndev = dev->ndev;
request = get_rndis_request(dev, RNDIS_MSG_SET,
RNDIS_MESSAGE_SIZE(struct rndis_set_request) +
@@ -856,7 +851,8 @@ static int rndis_filter_init_device(struct rndis_device *dev)
u32 status;
int ret;
unsigned long t;
- struct netvsc_device *nvdev = dev->net_dev;
+ struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
+ struct netvsc_device *nvdev = net_device_ctx->nvdev;
request = get_rndis_request(dev, RNDIS_MSG_INIT,
RNDIS_MESSAGE_SIZE(struct rndis_initialize_request));
@@ -879,7 +875,6 @@ static int rndis_filter_init_device(struct rndis_device *dev)
goto cleanup;
}
-
t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
if (t == 0) {
@@ -910,8 +905,9 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
{
struct rndis_request *request;
struct rndis_halt_request *halt;
- struct netvsc_device *nvdev = dev->net_dev;
- struct hv_device *hdev = nvdev->dev;
+ 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 */
@@ -979,13 +975,14 @@ static int rndis_filter_close_device(struct rndis_device *dev)
static void netvsc_sc_open(struct vmbus_channel *new_sc)
{
- struct netvsc_device *nvscdev;
+ struct net_device *ndev =
+ hv_get_drvdata(new_sc->primary_channel->device_obj);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *nvscdev = net_device_ctx->nvdev;
u16 chn_index = new_sc->offermsg.offer.sub_channel_index;
int ret;
unsigned long flags;
- nvscdev = hv_get_drvdata(new_sc->primary_channel->device_obj);
-
if (chn_index >= nvscdev->num_chn)
return;
@@ -1010,6 +1007,8 @@ int rndis_filter_device_add(struct hv_device *dev,
void *additional_info)
{
int ret;
+ struct net_device *net = hv_get_drvdata(dev);
+ struct net_device_context *net_device_ctx = netdev_priv(net);
struct netvsc_device *net_device;
struct rndis_device *rndis_device;
struct netvsc_device_info *device_info = additional_info;
@@ -1040,16 +1039,15 @@ int rndis_filter_device_add(struct hv_device *dev,
return ret;
}
-
/* Initialize the rndis device */
- net_device = hv_get_drvdata(dev);
+ net_device = net_device_ctx->nvdev;
net_device->max_chn = 1;
net_device->num_chn = 1;
spin_lock_init(&net_device->sc_lock);
net_device->extension = rndis_device;
- rndis_device->net_dev = net_device;
+ rndis_device->ndev = net;
/* Send the rndis initialization message */
ret = rndis_filter_init_device(rndis_device);
@@ -1198,7 +1196,9 @@ err_dev_remv:
void rndis_filter_device_remove(struct hv_device *dev)
{
- struct netvsc_device *net_dev = hv_get_drvdata(dev);
+ struct net_device *ndev = hv_get_drvdata(dev);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *net_dev = net_device_ctx->nvdev;
struct rndis_device *rndis_dev = net_dev->extension;
unsigned long t;
@@ -1224,7 +1224,9 @@ void rndis_filter_device_remove(struct hv_device *dev)
int rndis_filter_open(struct hv_device *dev)
{
- struct netvsc_device *net_device = hv_get_drvdata(dev);
+ struct net_device *ndev = hv_get_drvdata(dev);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *net_device = net_device_ctx->nvdev;
if (!net_device)
return -EINVAL;
@@ -1237,7 +1239,9 @@ int rndis_filter_open(struct hv_device *dev)
int rndis_filter_close(struct hv_device *dev)
{
- struct netvsc_device *nvdev = hv_get_drvdata(dev);
+ struct net_device *ndev = hv_get_drvdata(dev);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
+ struct netvsc_device *nvdev = net_device_ctx->nvdev;
if (!nvdev)
return -EINVAL;
--
2.5.5
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net-next v2 4/6] hv_netvsc: get rid of struct net_device pointer in struct netvsc_device
2016-05-13 11:55 [PATCH net-next v2 0/6] hv_netvsc: avoid races on mtu change/set channels Vitaly Kuznetsov
` (2 preceding siblings ...)
2016-05-13 11:55 ` [PATCH net-next v2 3/6] hv_netvsc: untangle the pointer mess Vitaly Kuznetsov
@ 2016-05-13 11:55 ` Vitaly Kuznetsov
2016-05-13 11:55 ` [PATCH net-next v2 5/6] hv_netvsc: synchronize netvsc_change_mtu()/netvsc_set_channels() with netvsc_remove() Vitaly Kuznetsov
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Vitaly Kuznetsov @ 2016-05-13 11:55 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, devel, Haiyang Zhang, K. Y. Srinivasan, Lino Sanfilippo
Simplify netvsvc pointer graph by getting rid of the redundant ndev
pointer. We can always get a pointer to struct net_device from somewhere
else.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/net/hyperv/hyperv_net.h | 5 +--
drivers/net/hyperv/netvsc.c | 36 +++++++---------
drivers/net/hyperv/netvsc_drv.c | 91 +++++++++++++++++++++++----------------
drivers/net/hyperv/rndis_filter.c | 4 +-
4 files changed, 72 insertions(+), 64 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 0f38743..c270c5a 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -173,7 +173,6 @@ struct rndis_device {
/* Interface */
struct rndis_message;
-struct netvsc_device;
int netvsc_device_add(struct hv_device *device, void *additional_info);
int netvsc_device_remove(struct hv_device *device);
int netvsc_send(struct hv_device *device,
@@ -203,7 +202,7 @@ int rndis_filter_receive(struct hv_device *dev,
int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter);
int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac);
-void netvsc_switch_datapath(struct netvsc_device *nv_dev, bool vf);
+void netvsc_switch_datapath(struct net_device *nv_dev, bool vf);
#define NVSP_INVALID_PROTOCOL_VERSION ((u32)0xFFFFFFFF)
@@ -711,8 +710,6 @@ struct netvsc_device {
struct nvsp_message revoke_packet;
/* unsigned char HwMacAddr[HW_MACADDR_LEN]; */
- struct net_device *ndev;
-
struct vmbus_channel *chn_table[VRSS_CHANNEL_MAX];
u32 send_table[VRSS_SEND_TAB_SIZE];
u32 max_chn;
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 1cd01ad..96b3c32 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -37,12 +37,12 @@
* Switch the data path from the synthetic interface to the VF
* interface.
*/
-void netvsc_switch_datapath(struct netvsc_device *nv_dev, bool vf)
+void netvsc_switch_datapath(struct net_device *ndev, bool vf)
{
- struct nvsp_message *init_pkt = &nv_dev->channel_init_pkt;
- struct net_device *ndev = nv_dev->ndev;
struct net_device_context *net_device_ctx = netdev_priv(ndev);
struct hv_device *dev = net_device_ctx->device_ctx;
+ struct netvsc_device *nv_dev = net_device_ctx->nvdev;
+ struct nvsp_message *init_pkt = &nv_dev->channel_init_pkt;
memset(init_pkt, 0, sizeof(struct nvsp_message));
init_pkt->hdr.msg_type = NVSP_MSG4_TYPE_SWITCH_DATA_PATH;
@@ -80,7 +80,6 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
net_device->destroy = false;
atomic_set(&net_device->open_cnt, 0);
atomic_set(&net_device->vf_use_cnt, 0);
- net_device->ndev = ndev;
net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT;
net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT;
@@ -263,7 +262,7 @@ static int netvsc_init_buf(struct hv_device *device)
net_device = get_outbound_net_device(device);
if (!net_device)
return -ENODEV;
- ndev = net_device->ndev;
+ ndev = hv_get_drvdata(device);
node = cpu_to_node(device->channel->target_cpu);
net_device->recv_buf = vzalloc_node(net_device->recv_buf_size, node);
@@ -453,6 +452,7 @@ static int negotiate_nvsp_ver(struct hv_device *device,
struct nvsp_message *init_packet,
u32 nvsp_ver)
{
+ struct net_device *ndev = hv_get_drvdata(device);
int ret;
unsigned long t;
@@ -486,8 +486,7 @@ static int negotiate_nvsp_ver(struct hv_device *device,
/* NVSPv2 or later: Send NDIS config */
memset(init_packet, 0, sizeof(struct nvsp_message));
init_packet->hdr.msg_type = NVSP_MSG2_TYPE_SEND_NDIS_CONFIG;
- init_packet->msg.v2_msg.send_ndis_config.mtu = net_device->ndev->mtu +
- ETH_HLEN;
+ init_packet->msg.v2_msg.send_ndis_config.mtu = ndev->mtu + ETH_HLEN;
init_packet->msg.v2_msg.send_ndis_config.capability.ieee8021q = 1;
if (nvsp_ver >= NVSP_PROTOCOL_VERSION_5)
@@ -507,7 +506,6 @@ static int netvsc_connect_vsp(struct hv_device *device)
struct netvsc_device *net_device;
struct nvsp_message *init_packet;
int ndis_version;
- struct net_device *ndev;
u32 ver_list[] = { NVSP_PROTOCOL_VERSION_1, NVSP_PROTOCOL_VERSION_2,
NVSP_PROTOCOL_VERSION_4, NVSP_PROTOCOL_VERSION_5 };
int i, num_ver = 4; /* number of different NVSP versions */
@@ -515,7 +513,6 @@ static int netvsc_connect_vsp(struct hv_device *device)
net_device = get_outbound_net_device(device);
if (!net_device)
return -ENODEV;
- ndev = net_device->ndev;
init_packet = &net_device->channel_init_pkt;
@@ -768,6 +765,7 @@ static u32 netvsc_copy_to_send_buf(struct netvsc_device *net_device,
}
static inline int netvsc_send_pkt(
+ struct hv_device *device,
struct hv_netvsc_packet *packet,
struct netvsc_device *net_device,
struct hv_page_buffer **pb,
@@ -776,7 +774,7 @@ static inline int netvsc_send_pkt(
struct nvsp_message nvmsg;
u16 q_idx = packet->q_idx;
struct vmbus_channel *out_channel = net_device->chn_table[q_idx];
- struct net_device *ndev = net_device->ndev;
+ struct net_device *ndev = hv_get_drvdata(device);
u64 req_id;
int ret;
struct hv_page_buffer *pgbuf;
@@ -971,7 +969,8 @@ int netvsc_send(struct hv_device *device,
}
if (msd_send) {
- m_ret = netvsc_send_pkt(msd_send, net_device, NULL, msd_skb);
+ m_ret = netvsc_send_pkt(device, msd_send, net_device,
+ NULL, msd_skb);
if (m_ret != 0) {
netvsc_free_send_slot(net_device,
@@ -982,7 +981,7 @@ int netvsc_send(struct hv_device *device,
send_now:
if (cur_send)
- ret = netvsc_send_pkt(cur_send, net_device, pb, skb);
+ ret = netvsc_send_pkt(device, cur_send, net_device, pb, skb);
if (ret != 0 && section_index != NETVSC_INVALID_INDEX)
netvsc_free_send_slot(net_device, section_index);
@@ -998,9 +997,7 @@ static void netvsc_send_recv_completion(struct hv_device *device,
struct nvsp_message recvcompMessage;
int retries = 0;
int ret;
- struct net_device *ndev;
-
- ndev = net_device->ndev;
+ struct net_device *ndev = hv_get_drvdata(device);
recvcompMessage.hdr.msg_type =
NVSP_MSG1_TYPE_SEND_RNDIS_PKT_COMPLETE;
@@ -1047,11 +1044,9 @@ static void netvsc_receive(struct netvsc_device *net_device,
u32 status = NVSP_STAT_SUCCESS;
int i;
int count = 0;
- struct net_device *ndev;
+ struct net_device *ndev = hv_get_drvdata(device);
void *data;
- ndev = net_device->ndev;
-
/*
* All inbound packets other than send completion should be xfer page
* packet
@@ -1107,14 +1102,13 @@ static void netvsc_send_table(struct hv_device *hdev,
struct nvsp_message *nvmsg)
{
struct netvsc_device *nvscdev;
- struct net_device *ndev;
+ struct net_device *ndev = hv_get_drvdata(hdev);
int i;
u32 count, *tab;
nvscdev = get_outbound_net_device(hdev);
if (!nvscdev)
return;
- ndev = nvscdev->ndev;
count = nvmsg->msg.v5_msg.send_table.count;
if (count != VRSS_SEND_TAB_SIZE) {
@@ -1173,7 +1167,7 @@ void netvsc_channel_cb(void *context)
net_device = get_inbound_net_device(device);
if (!net_device)
return;
- ndev = net_device->ndev;
+ ndev = hv_get_drvdata(device);
buffer = get_per_channel_state(channel);
do {
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index a33a1c9..7325d69 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -67,18 +67,19 @@ static void do_set_multicast(struct work_struct *w)
{
struct net_device_context *ndevctx =
container_of(w, struct net_device_context, work);
- struct netvsc_device *nvdev;
+ struct hv_device *device_obj = ndevctx->device_ctx;
+ struct net_device *ndev = hv_get_drvdata(device_obj);
+ struct netvsc_device *nvdev = ndevctx->nvdev;
struct rndis_device *rdev;
- nvdev = ndevctx->nvdev;
- if (nvdev == NULL || nvdev->ndev == NULL)
+ if (!nvdev)
return;
rdev = nvdev->extension;
if (rdev == NULL)
return;
- if (nvdev->ndev->flags & IFF_PROMISC)
+ if (ndev->flags & IFF_PROMISC)
rndis_filter_set_packet_filter(rdev,
NDIS_PACKET_TYPE_PROMISCUOUS);
else
@@ -1050,23 +1051,22 @@ static const struct net_device_ops device_ops = {
*/
static void netvsc_link_change(struct work_struct *w)
{
- struct net_device_context *ndev_ctx;
- struct net_device *net;
+ struct net_device_context *ndev_ctx =
+ container_of(w, struct net_device_context, dwork.work);
+ struct hv_device *device_obj = ndev_ctx->device_ctx;
+ struct net_device *net = hv_get_drvdata(device_obj);
struct netvsc_device *net_device;
struct rndis_device *rdev;
struct netvsc_reconfig *event = NULL;
bool notify = false, reschedule = false;
unsigned long flags, next_reconfig, delay;
- ndev_ctx = container_of(w, struct net_device_context, dwork.work);
-
rtnl_lock();
if (ndev_ctx->start_remove)
goto out_unlock;
net_device = ndev_ctx->nvdev;
rdev = net_device->extension;
- net = net_device->ndev;
next_reconfig = ndev_ctx->last_reconfig + LINKCHANGE_INT;
if (time_is_after_jiffies(next_reconfig)) {
@@ -1167,10 +1167,9 @@ static void netvsc_notify_peers(struct work_struct *wrk)
atomic_dec(&gwrk->netvsc_dev->vf_use_cnt);
}
-static struct netvsc_device *get_netvsc_device(char *mac)
+static struct net_device *get_netvsc_net_device(char *mac)
{
- struct net_device *dev;
- struct net_device_context *netvsc_ctx = NULL;
+ struct net_device *dev, *found = NULL;
int rtnl_locked;
rtnl_locked = rtnl_trylock();
@@ -1179,21 +1178,20 @@ static struct netvsc_device *get_netvsc_device(char *mac)
if (memcmp(dev->dev_addr, mac, ETH_ALEN) == 0) {
if (dev->netdev_ops != &device_ops)
continue;
- netvsc_ctx = netdev_priv(dev);
+ found = dev;
break;
}
}
if (rtnl_locked)
rtnl_unlock();
- if (netvsc_ctx == NULL)
- return NULL;
-
- return netvsc_ctx->nvdev;
+ return found;
}
static int netvsc_register_vf(struct net_device *vf_netdev)
{
+ struct net_device *ndev;
+ struct net_device_context *net_device_ctx;
struct netvsc_device *netvsc_dev;
const struct ethtool_ops *eth_ops = vf_netdev->ethtool_ops;
@@ -1205,11 +1203,16 @@ static int netvsc_register_vf(struct net_device *vf_netdev)
* associate with the VF interface. If we don't find a matching
* synthetic interface, move on.
*/
- netvsc_dev = get_netvsc_device(vf_netdev->dev_addr);
+ ndev = get_netvsc_net_device(vf_netdev->dev_addr);
+ if (!ndev)
+ return NOTIFY_DONE;
+
+ net_device_ctx = netdev_priv(ndev);
+ netvsc_dev = net_device_ctx->nvdev;
if (netvsc_dev == NULL)
return NOTIFY_DONE;
- netdev_info(netvsc_dev->ndev, "VF registering: %s\n", vf_netdev->name);
+ netdev_info(ndev, "VF registering: %s\n", vf_netdev->name);
/*
* Take a reference on the module.
*/
@@ -1221,6 +1224,7 @@ static int netvsc_register_vf(struct net_device *vf_netdev)
static int netvsc_vf_up(struct net_device *vf_netdev)
{
+ struct net_device *ndev;
struct netvsc_device *netvsc_dev;
const struct ethtool_ops *eth_ops = vf_netdev->ethtool_ops;
struct net_device_context *net_device_ctx;
@@ -1228,13 +1232,17 @@ static int netvsc_vf_up(struct net_device *vf_netdev)
if (eth_ops == ðtool_ops)
return NOTIFY_DONE;
- netvsc_dev = get_netvsc_device(vf_netdev->dev_addr);
+ ndev = get_netvsc_net_device(vf_netdev->dev_addr);
+ if (!ndev)
+ return NOTIFY_DONE;
+
+ net_device_ctx = netdev_priv(ndev);
+ netvsc_dev = net_device_ctx->nvdev;
if ((netvsc_dev == NULL) || (netvsc_dev->vf_netdev == NULL))
return NOTIFY_DONE;
- netdev_info(netvsc_dev->ndev, "VF up: %s\n", vf_netdev->name);
- net_device_ctx = netdev_priv(netvsc_dev->ndev);
+ netdev_info(ndev, "VF up: %s\n", vf_netdev->name);
netvsc_dev->vf_inject = true;
/*
@@ -1245,11 +1253,10 @@ static int netvsc_vf_up(struct net_device *vf_netdev)
/*
* notify the host to switch the data path.
*/
- netvsc_switch_datapath(netvsc_dev, true);
- netdev_info(netvsc_dev->ndev, "Data path switched to VF: %s\n",
- vf_netdev->name);
+ netvsc_switch_datapath(ndev, true);
+ netdev_info(ndev, "Data path switched to VF: %s\n", vf_netdev->name);
- netif_carrier_off(netvsc_dev->ndev);
+ netif_carrier_off(ndev);
/*
* Now notify peers. We are scheduling work to
@@ -1267,6 +1274,7 @@ static int netvsc_vf_up(struct net_device *vf_netdev)
static int netvsc_vf_down(struct net_device *vf_netdev)
{
+ struct net_device *ndev;
struct netvsc_device *netvsc_dev;
struct net_device_context *net_device_ctx;
const struct ethtool_ops *eth_ops = vf_netdev->ethtool_ops;
@@ -1274,13 +1282,17 @@ static int netvsc_vf_down(struct net_device *vf_netdev)
if (eth_ops == ðtool_ops)
return NOTIFY_DONE;
- netvsc_dev = get_netvsc_device(vf_netdev->dev_addr);
+ ndev = get_netvsc_net_device(vf_netdev->dev_addr);
+ if (!ndev)
+ return NOTIFY_DONE;
+
+ net_device_ctx = netdev_priv(ndev);
+ netvsc_dev = net_device_ctx->nvdev;
if ((netvsc_dev == NULL) || (netvsc_dev->vf_netdev == NULL))
return NOTIFY_DONE;
- netdev_info(netvsc_dev->ndev, "VF down: %s\n", vf_netdev->name);
- net_device_ctx = netdev_priv(netvsc_dev->ndev);
+ netdev_info(ndev, "VF down: %s\n", vf_netdev->name);
netvsc_dev->vf_inject = false;
/*
* Wait for currently active users to
@@ -1289,16 +1301,15 @@ static int netvsc_vf_down(struct net_device *vf_netdev)
while (atomic_read(&netvsc_dev->vf_use_cnt) != 0)
udelay(50);
- netvsc_switch_datapath(netvsc_dev, false);
- netdev_info(netvsc_dev->ndev, "Data path switched from VF: %s\n",
- vf_netdev->name);
+ netvsc_switch_datapath(ndev, false);
+ netdev_info(ndev, "Data path switched from VF: %s\n", vf_netdev->name);
rndis_filter_close(net_device_ctx->device_ctx);
- netif_carrier_on(netvsc_dev->ndev);
+ netif_carrier_on(ndev);
/*
* Notify peers.
*/
atomic_inc(&netvsc_dev->vf_use_cnt);
- net_device_ctx->gwrk.netdev = netvsc_dev->ndev;
+ net_device_ctx->gwrk.netdev = ndev;
net_device_ctx->gwrk.netvsc_dev = netvsc_dev;
schedule_work(&net_device_ctx->gwrk.dwrk);
@@ -1308,17 +1319,23 @@ static int netvsc_vf_down(struct net_device *vf_netdev)
static int netvsc_unregister_vf(struct net_device *vf_netdev)
{
+ struct net_device *ndev;
struct netvsc_device *netvsc_dev;
const struct ethtool_ops *eth_ops = vf_netdev->ethtool_ops;
+ struct net_device_context *net_device_ctx;
if (eth_ops == ðtool_ops)
return NOTIFY_DONE;
- netvsc_dev = get_netvsc_device(vf_netdev->dev_addr);
+ ndev = get_netvsc_net_device(vf_netdev->dev_addr);
+ if (!ndev)
+ return NOTIFY_DONE;
+
+ net_device_ctx = netdev_priv(ndev);
+ netvsc_dev = net_device_ctx->nvdev;
if (netvsc_dev == NULL)
return NOTIFY_DONE;
- netdev_info(netvsc_dev->ndev, "VF unregistering: %s\n",
- vf_netdev->name);
+ netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
netvsc_dev->vf_netdev = NULL;
module_put(THIS_MODULE);
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 6caba51..97c292b 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1061,8 +1061,8 @@ int rndis_filter_device_add(struct hv_device *dev,
ret = rndis_filter_query_device(rndis_device,
RNDIS_OID_GEN_MAXIMUM_FRAME_SIZE,
&mtu, &size);
- if (ret == 0 && size == sizeof(u32) && mtu < net_device->ndev->mtu)
- net_device->ndev->mtu = mtu;
+ if (ret == 0 && size == sizeof(u32) && mtu < net->mtu)
+ net->mtu = mtu;
/* Get the mac address */
ret = rndis_filter_query_device_mac(rndis_device);
--
2.5.5
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net-next v2 5/6] hv_netvsc: synchronize netvsc_change_mtu()/netvsc_set_channels() with netvsc_remove()
2016-05-13 11:55 [PATCH net-next v2 0/6] hv_netvsc: avoid races on mtu change/set channels Vitaly Kuznetsov
` (3 preceding siblings ...)
2016-05-13 11:55 ` [PATCH net-next v2 4/6] hv_netvsc: get rid of struct net_device pointer in struct netvsc_device Vitaly Kuznetsov
@ 2016-05-13 11:55 ` Vitaly Kuznetsov
2016-05-13 11:55 ` [PATCH net-next v2 6/6] hv_netvsc: set nvdev link after populating chn_table Vitaly Kuznetsov
2016-05-16 17:27 ` [PATCH net-next v2 0/6] hv_netvsc: avoid races on mtu change/set channels David Miller
6 siblings, 0 replies; 8+ messages in thread
From: Vitaly Kuznetsov @ 2016-05-13 11:55 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, devel, Haiyang Zhang, K. Y. Srinivasan, Lino Sanfilippo
When netvsc device is removed during mtu change or channels setup we get
into troubles as both paths are trying to remove the device. Synchronize
them with start_remove flag and rtnl lock.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/net/hyperv/netvsc_drv.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 7325d69..6a69b5c 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -759,7 +759,7 @@ static int netvsc_set_channels(struct net_device *net,
int ret = 0;
bool recovering = false;
- if (!nvdev || nvdev->destroy)
+ if (net_device_ctx->start_remove || !nvdev || nvdev->destroy)
return -ENODEV;
num_chn = nvdev->num_chn;
@@ -907,7 +907,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
u32 num_chn;
int ret = 0;
- if (nvdev == NULL || nvdev->destroy)
+ if (ndevctx->start_remove || !nvdev || nvdev->destroy)
return -ENODEV;
if (nvdev->nvsp_version >= NVSP_PROTOCOL_VERSION_2)
@@ -1445,7 +1445,12 @@ static int netvsc_remove(struct hv_device *dev)
ndev_ctx = netdev_priv(net);
net_device = ndev_ctx->nvdev;
+ /* Avoid racing with netvsc_change_mtu()/netvsc_set_channels()
+ * removing the device.
+ */
+ rtnl_lock();
ndev_ctx->start_remove = true;
+ rtnl_unlock();
cancel_delayed_work_sync(&ndev_ctx->dwork);
cancel_work_sync(&ndev_ctx->work);
--
2.5.5
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net-next v2 6/6] hv_netvsc: set nvdev link after populating chn_table
2016-05-13 11:55 [PATCH net-next v2 0/6] hv_netvsc: avoid races on mtu change/set channels Vitaly Kuznetsov
` (4 preceding siblings ...)
2016-05-13 11:55 ` [PATCH net-next v2 5/6] hv_netvsc: synchronize netvsc_change_mtu()/netvsc_set_channels() with netvsc_remove() Vitaly Kuznetsov
@ 2016-05-13 11:55 ` Vitaly Kuznetsov
2016-05-16 17:27 ` [PATCH net-next v2 0/6] hv_netvsc: avoid races on mtu change/set channels David Miller
6 siblings, 0 replies; 8+ messages in thread
From: Vitaly Kuznetsov @ 2016-05-13 11:55 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, devel, Haiyang Zhang, K. Y. Srinivasan, Lino Sanfilippo
Crash in netvsc_send() is observed when netvsc device is re-created on
mtu change/set channels. The crash is caused by dereferencing of NULL
channel pointer which comes from chn_table. The root cause is a mixture
of two facts:
- we set nvdev pointer in net_device_context in alloc_net_device()
before we populate chn_table.
- we populate chn_table[0] only.
The issue could be papered over by checking channel != NULL in
netvsc_send() but populating the whole chn_table and writing the
nvdev pointer afterwards seems more appropriate.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
drivers/net/hyperv/netvsc.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 96b3c32..719cb35 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -60,11 +60,9 @@ void netvsc_switch_datapath(struct net_device *ndev, bool vf)
}
-static struct netvsc_device *alloc_net_device(struct hv_device *device)
+static struct netvsc_device *alloc_net_device(void)
{
struct netvsc_device *net_device;
- struct net_device *ndev = hv_get_drvdata(device);
- struct net_device_context *net_device_ctx = netdev_priv(ndev);
net_device = kzalloc(sizeof(struct netvsc_device), GFP_KERNEL);
if (!net_device)
@@ -86,8 +84,6 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
net_device->vf_netdev = NULL;
net_device->vf_inject = false;
- net_device_ctx->nvdev = net_device;
-
return net_device;
}
@@ -1240,20 +1236,19 @@ void netvsc_channel_cb(void *context)
*/
int netvsc_device_add(struct hv_device *device, void *additional_info)
{
- int ret = 0;
+ int i, ret = 0;
int ring_size =
((struct netvsc_device_info *)additional_info)->ring_size;
struct netvsc_device *net_device;
- struct net_device *ndev;
+ struct net_device *ndev = hv_get_drvdata(device);
+ struct net_device_context *net_device_ctx = netdev_priv(ndev);
- net_device = alloc_net_device(device);
+ net_device = alloc_net_device();
if (!net_device)
return -ENOMEM;
net_device->ring_size = ring_size;
- ndev = hv_get_drvdata(device);
-
/* Initialize the NetVSC channel extension */
init_completion(&net_device->channel_init_wait);
@@ -1272,7 +1267,19 @@ int netvsc_device_add(struct hv_device *device, void *additional_info)
/* Channel is opened */
pr_info("hv_netvsc channel opened successfully\n");
- net_device->chn_table[0] = device->channel;
+ /* If we're reopening the device we may have multiple queues, fill the
+ * chn_table with the default channel to use it before subchannels are
+ * opened.
+ */
+ for (i = 0; i < VRSS_CHANNEL_MAX; i++)
+ net_device->chn_table[i] = device->channel;
+
+ /* Writing nvdev pointer unlocks netvsc_send(), make sure chn_table is
+ * populated.
+ */
+ wmb();
+
+ net_device_ctx->nvdev = net_device;
/* Connect with the NetVsp */
ret = netvsc_connect_vsp(device);
--
2.5.5
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net-next v2 0/6] hv_netvsc: avoid races on mtu change/set channels
2016-05-13 11:55 [PATCH net-next v2 0/6] hv_netvsc: avoid races on mtu change/set channels Vitaly Kuznetsov
` (5 preceding siblings ...)
2016-05-13 11:55 ` [PATCH net-next v2 6/6] hv_netvsc: set nvdev link after populating chn_table Vitaly Kuznetsov
@ 2016-05-16 17:27 ` David Miller
6 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2016-05-16 17:27 UTC (permalink / raw)
To: vkuznets; +Cc: netdev, linux-kernel, devel, haiyangz, kys, LinoSanfilippo
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Fri, 13 May 2016 13:55:19 +0200
> Changes since v1:
> - Rebased to net-next [Haiyang Zhang]
>
> Original description:
>
> MTU change and set channels operations are implemented as netvsc device
> re-creation destroying internal structures (struct net_device stays). This
> is really unfortunate but there is no support from Hyper-V host to do it
> in a different way. Such re-creation is unsurprisingly racy, Haiyang
> reported a crash when netvsc_change_mtu() is racing with
> netvsc_link_change() but I was able to identify additional races upon
> investigation. Both netvsc_set_channels() and netvsc_change_mtu() race
> against:
> 1) netvsc_link_change()
> 2) netvsc_remove()
> 3) netvsc_send()
>
> To solve these issues without introducing new locks some refactoring is
> required. We need to get rid of very complex link graph in all the
> internal structures and avoid traveling through structures which are being
> removed.
Series applied, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread