Skip to content

Commit 3d541ac

Browse files
vittyvkdavem330
authored andcommitted
hv_netvsc: untangle the pointer mess
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 <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 1bdcec8 commit 3d541ac

File tree

4 files changed

+99
-122
lines changed

4 files changed

+99
-122
lines changed

drivers/net/hyperv/hyperv_net.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ enum rndis_device_state {
158158
};
159159

160160
struct rndis_device {
161-
struct netvsc_device *net_dev;
161+
struct net_device *ndev;
162162

163163
enum rndis_device_state state;
164164
bool link_state;
@@ -173,6 +173,7 @@ struct rndis_device {
173173

174174
/* Interface */
175175
struct rndis_message;
176+
struct netvsc_device;
176177
int netvsc_device_add(struct hv_device *device, void *additional_info);
177178
int netvsc_device_remove(struct hv_device *device);
178179
int netvsc_send(struct hv_device *device,
@@ -653,6 +654,8 @@ struct garp_wrk {
653654
struct net_device_context {
654655
/* point back to our device context */
655656
struct hv_device *device_ctx;
657+
/* netvsc_device */
658+
struct netvsc_device *nvdev;
656659
/* reconfigure work */
657660
struct delayed_work dwork;
658661
/* last reconfig time */
@@ -679,8 +682,6 @@ struct net_device_context {
679682

680683
/* Per netvsc device */
681684
struct netvsc_device {
682-
struct hv_device *dev;
683-
684685
u32 nvsp_version;
685686

686687
atomic_t num_outstanding_sends;
@@ -734,9 +735,6 @@ struct netvsc_device {
734735
u32 max_pkt; /* max number of pkt in one send, e.g. 8 */
735736
u32 pkt_align; /* alignment bytes, e.g. 8 */
736737

737-
/* The net device context */
738-
struct net_device_context *nd_ctx;
739-
740738
/* 1: allocated, serial number is valid. 0: not allocated */
741739
u32 vf_alloc;
742740
/* Serial number of the VF to team with */

drivers/net/hyperv/netvsc.c

Lines changed: 33 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@
4040
void netvsc_switch_datapath(struct netvsc_device *nv_dev, bool vf)
4141
{
4242
struct nvsp_message *init_pkt = &nv_dev->channel_init_pkt;
43-
struct hv_device *dev = nv_dev->dev;
43+
struct net_device *ndev = nv_dev->ndev;
44+
struct net_device_context *net_device_ctx = netdev_priv(ndev);
45+
struct hv_device *dev = net_device_ctx->device_ctx;
4446

4547
memset(init_pkt, 0, sizeof(struct nvsp_message));
4648
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)
6264
{
6365
struct netvsc_device *net_device;
6466
struct net_device *ndev = hv_get_drvdata(device);
67+
struct net_device_context *net_device_ctx = netdev_priv(ndev);
6568

6669
net_device = kzalloc(sizeof(struct netvsc_device), GFP_KERNEL);
6770
if (!net_device)
@@ -77,15 +80,15 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
7780
net_device->destroy = false;
7881
atomic_set(&net_device->open_cnt, 0);
7982
atomic_set(&net_device->vf_use_cnt, 0);
80-
net_device->dev = device;
8183
net_device->ndev = ndev;
8284
net_device->max_pkt = RNDIS_MAX_PKT_DEFAULT;
8385
net_device->pkt_align = RNDIS_PKT_ALIGN_DEFAULT;
8486

8587
net_device->vf_netdev = NULL;
8688
net_device->vf_inject = false;
8789

88-
hv_set_drvdata(device, net_device);
90+
net_device_ctx->nvdev = net_device;
91+
8992
return net_device;
9093
}
9194

@@ -97,9 +100,10 @@ static void free_netvsc_device(struct netvsc_device *nvdev)
97100

98101
static struct netvsc_device *get_outbound_net_device(struct hv_device *device)
99102
{
100-
struct netvsc_device *net_device;
103+
struct net_device *ndev = hv_get_drvdata(device);
104+
struct net_device_context *net_device_ctx = netdev_priv(ndev);
105+
struct netvsc_device *net_device = net_device_ctx->nvdev;
101106

102-
net_device = hv_get_drvdata(device);
103107
if (net_device && net_device->destroy)
104108
net_device = NULL;
105109

@@ -108,9 +112,9 @@ static struct netvsc_device *get_outbound_net_device(struct hv_device *device)
108112

109113
static struct netvsc_device *get_inbound_net_device(struct hv_device *device)
110114
{
111-
struct netvsc_device *net_device;
112-
113-
net_device = hv_get_drvdata(device);
115+
struct net_device *ndev = hv_get_drvdata(device);
116+
struct net_device_context *net_device_ctx = netdev_priv(ndev);
117+
struct netvsc_device *net_device = net_device_ctx->nvdev;
114118

115119
if (!net_device)
116120
goto get_in_err;
@@ -124,11 +128,13 @@ static struct netvsc_device *get_inbound_net_device(struct hv_device *device)
124128
}
125129

126130

127-
static int netvsc_destroy_buf(struct netvsc_device *net_device)
131+
static int netvsc_destroy_buf(struct hv_device *device)
128132
{
129133
struct nvsp_message *revoke_packet;
130134
int ret = 0;
131-
struct net_device *ndev = net_device->ndev;
135+
struct net_device *ndev = hv_get_drvdata(device);
136+
struct net_device_context *net_device_ctx = netdev_priv(ndev);
137+
struct netvsc_device *net_device = net_device_ctx->nvdev;
132138

133139
/*
134140
* 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)
146152
revoke_packet->msg.v1_msg.
147153
revoke_recv_buf.id = NETVSC_RECEIVE_BUFFER_ID;
148154

149-
ret = vmbus_sendpacket(net_device->dev->channel,
155+
ret = vmbus_sendpacket(device->channel,
150156
revoke_packet,
151157
sizeof(struct nvsp_message),
152158
(unsigned long)revoke_packet,
@@ -164,8 +170,8 @@ static int netvsc_destroy_buf(struct netvsc_device *net_device)
164170

165171
/* Teardown the gpadl on the vsp end */
166172
if (net_device->recv_buf_gpadl_handle) {
167-
ret = vmbus_teardown_gpadl(net_device->dev->channel,
168-
net_device->recv_buf_gpadl_handle);
173+
ret = vmbus_teardown_gpadl(device->channel,
174+
net_device->recv_buf_gpadl_handle);
169175

170176
/* If we failed here, we might as well return and have a leak
171177
* rather than continue and a bugchk
@@ -206,7 +212,7 @@ static int netvsc_destroy_buf(struct netvsc_device *net_device)
206212
revoke_packet->msg.v1_msg.revoke_send_buf.id =
207213
NETVSC_SEND_BUFFER_ID;
208214

209-
ret = vmbus_sendpacket(net_device->dev->channel,
215+
ret = vmbus_sendpacket(device->channel,
210216
revoke_packet,
211217
sizeof(struct nvsp_message),
212218
(unsigned long)revoke_packet,
@@ -222,7 +228,7 @@ static int netvsc_destroy_buf(struct netvsc_device *net_device)
222228
}
223229
/* Teardown the gpadl on the vsp end */
224230
if (net_device->send_buf_gpadl_handle) {
225-
ret = vmbus_teardown_gpadl(net_device->dev->channel,
231+
ret = vmbus_teardown_gpadl(device->channel,
226232
net_device->send_buf_gpadl_handle);
227233

228234
/* 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)
434440
goto exit;
435441

436442
cleanup:
437-
netvsc_destroy_buf(net_device);
443+
netvsc_destroy_buf(device);
438444

439445
exit:
440446
return ret;
@@ -565,34 +571,23 @@ static int netvsc_connect_vsp(struct hv_device *device)
565571
return ret;
566572
}
567573

568-
static void netvsc_disconnect_vsp(struct netvsc_device *net_device)
574+
static void netvsc_disconnect_vsp(struct hv_device *device)
569575
{
570-
netvsc_destroy_buf(net_device);
576+
netvsc_destroy_buf(device);
571577
}
572578

573579
/*
574580
* netvsc_device_remove - Callback when the root bus device is removed
575581
*/
576582
int netvsc_device_remove(struct hv_device *device)
577583
{
578-
struct netvsc_device *net_device;
579-
unsigned long flags;
580-
581-
net_device = hv_get_drvdata(device);
582-
583-
netvsc_disconnect_vsp(net_device);
584+
struct net_device *ndev = hv_get_drvdata(device);
585+
struct net_device_context *net_device_ctx = netdev_priv(ndev);
586+
struct netvsc_device *net_device = net_device_ctx->nvdev;
584587

585-
/*
586-
* Since we have already drained, we don't need to busy wait
587-
* as was done in final_release_stor_device()
588-
* Note that we cannot set the ext pointer to NULL until
589-
* we have drained - to drain the outgoing packets, we need to
590-
* allow incoming packets.
591-
*/
588+
netvsc_disconnect_vsp(device);
592589

593-
spin_lock_irqsave(&device->channel->inbound_lock, flags);
594-
hv_set_drvdata(device, NULL);
595-
spin_unlock_irqrestore(&device->channel->inbound_lock, flags);
590+
net_device_ctx->nvdev = NULL;
596591

597592
/*
598593
* At this point, no one should be accessing net_device
@@ -640,12 +635,11 @@ static void netvsc_send_completion(struct netvsc_device *net_device,
640635
{
641636
struct nvsp_message *nvsp_packet;
642637
struct hv_netvsc_packet *nvsc_packet;
643-
struct net_device *ndev;
638+
struct net_device *ndev = hv_get_drvdata(device);
639+
struct net_device_context *net_device_ctx = netdev_priv(ndev);
644640
u32 send_index;
645641
struct sk_buff *skb;
646642

647-
ndev = net_device->ndev;
648-
649643
nvsp_packet = (struct nvsp_message *)((unsigned long)packet +
650644
(packet->offset8 << 3));
651645

@@ -690,7 +684,7 @@ static void netvsc_send_completion(struct netvsc_device *net_device,
690684
wake_up(&net_device->wait_drain);
691685

692686
if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx)) &&
693-
!net_device->nd_ctx->start_remove &&
687+
!net_device_ctx->start_remove &&
694688
(hv_ringbuf_avail_percent(&channel->outbound) >
695689
RING_AVAIL_PERCENT_HIWATER || queue_sends < 1))
696690
netif_tx_wake_queue(netdev_get_tx_queue(
@@ -1264,17 +1258,7 @@ int netvsc_device_add(struct hv_device *device, void *additional_info)
12641258

12651259
net_device->ring_size = ring_size;
12661260

1267-
/*
1268-
* Coming into this function, struct net_device * is
1269-
* registered as the driver private data.
1270-
* In alloc_net_device(), we register struct netvsc_device *
1271-
* as the driver private data and stash away struct net_device *
1272-
* in struct netvsc_device *.
1273-
*/
1274-
ndev = net_device->ndev;
1275-
1276-
/* Add netvsc_device context to netvsc_device */
1277-
net_device->nd_ctx = netdev_priv(ndev);
1261+
ndev = hv_get_drvdata(device);
12781262

12791263
/* Initialize the NetVSC channel extension */
12801264
init_completion(&net_device->channel_init_wait);

0 commit comments

Comments
 (0)