diff options
| author | Vitaly Kuznetsov <vkuznets@redhat.com> | 2016-06-09 06:44:03 -0400 |
|---|---|---|
| committer | David S. Miller <davem@davemloft.net> | 2016-06-09 14:40:05 -0400 |
| commit | 5362855aba7159aab8f7c6573eb675d9da317914 (patch) | |
| tree | b8d3a5a3e72278ec239c5d7eba9dcab3ae66608d /drivers/net/hyperv | |
| parent | adba931fbc825efca7c821f0d76baed0a8dc9189 (diff) | |
netvsc: get rid of completion timeouts
I'm hitting 5 second timeout in rndis_filter_set_rss_param() while setting
RSS parameters for the device. When this happens we end up returning
-ETIMEDOUT from the function and rndis_filter_device_add() falls back to
setting
net_device->max_chn = 1;
net_device->num_chn = 1;
net_device->num_sc_offered = 0;
but after a moment the rndis request succeeds and subchannels start to
appear. netvsc_sc_open() does unconditional nvscdev->num_sc_offered-- and
it becomes U32_MAX-1. Consequent rndis_filter_device_remove() will hang
while waiting for all U32_MAX-1 subchannels to appear and this is not
going to happen.
The immediate issue could be solved by adding num_sc_offered > 0 check to
netvsc_sc_open() but we're getting out of sync with the host and it's not
easy to adjust things later, e.g. in this particular case we'll be creating
queues without a user request for it and races are expected. Same applies
to other parts of the driver which have the same completion timeout.
Following the trend in drivers/hv/* code I suggest we remove all these
timeouts completely. As a guest we can always trust the host we're running
on and if the host screws things up there is no easy way to recover anyway.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Acked-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net/hyperv')
| -rw-r--r-- | drivers/net/hyperv/netvsc.c | 14 | ||||
| -rw-r--r-- | drivers/net/hyperv/rndis_filter.c | 117 |
2 files changed, 31 insertions, 100 deletions
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index 96f00c0261fa..6909c322de4e 100644 --- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c | |||
| @@ -244,7 +244,6 @@ static int netvsc_destroy_buf(struct hv_device *device) | |||
| 244 | static int netvsc_init_buf(struct hv_device *device) | 244 | static int netvsc_init_buf(struct hv_device *device) |
| 245 | { | 245 | { |
| 246 | int ret = 0; | 246 | int ret = 0; |
| 247 | unsigned long t; | ||
| 248 | struct netvsc_device *net_device; | 247 | struct netvsc_device *net_device; |
| 249 | struct nvsp_message *init_packet; | 248 | struct nvsp_message *init_packet; |
| 250 | struct net_device *ndev; | 249 | struct net_device *ndev; |
| @@ -305,9 +304,7 @@ static int netvsc_init_buf(struct hv_device *device) | |||
| 305 | goto cleanup; | 304 | goto cleanup; |
| 306 | } | 305 | } |
| 307 | 306 | ||
| 308 | t = wait_for_completion_timeout(&net_device->channel_init_wait, 5*HZ); | 307 | wait_for_completion(&net_device->channel_init_wait); |
| 309 | BUG_ON(t == 0); | ||
| 310 | |||
| 311 | 308 | ||
| 312 | /* Check the response */ | 309 | /* Check the response */ |
| 313 | if (init_packet->msg.v1_msg. | 310 | if (init_packet->msg.v1_msg. |
| @@ -390,8 +387,7 @@ static int netvsc_init_buf(struct hv_device *device) | |||
| 390 | goto cleanup; | 387 | goto cleanup; |
| 391 | } | 388 | } |
| 392 | 389 | ||
| 393 | t = wait_for_completion_timeout(&net_device->channel_init_wait, 5*HZ); | 390 | wait_for_completion(&net_device->channel_init_wait); |
| 394 | BUG_ON(t == 0); | ||
| 395 | 391 | ||
| 396 | /* Check the response */ | 392 | /* Check the response */ |
| 397 | if (init_packet->msg.v1_msg. | 393 | if (init_packet->msg.v1_msg. |
| @@ -445,7 +441,6 @@ static int negotiate_nvsp_ver(struct hv_device *device, | |||
| 445 | { | 441 | { |
| 446 | struct net_device *ndev = hv_get_drvdata(device); | 442 | struct net_device *ndev = hv_get_drvdata(device); |
| 447 | int ret; | 443 | int ret; |
| 448 | unsigned long t; | ||
| 449 | 444 | ||
| 450 | memset(init_packet, 0, sizeof(struct nvsp_message)); | 445 | memset(init_packet, 0, sizeof(struct nvsp_message)); |
| 451 | init_packet->hdr.msg_type = NVSP_MSG_TYPE_INIT; | 446 | init_packet->hdr.msg_type = NVSP_MSG_TYPE_INIT; |
| @@ -462,10 +457,7 @@ static int negotiate_nvsp_ver(struct hv_device *device, | |||
| 462 | if (ret != 0) | 457 | if (ret != 0) |
| 463 | return ret; | 458 | return ret; |
| 464 | 459 | ||
| 465 | t = wait_for_completion_timeout(&net_device->channel_init_wait, 5*HZ); | 460 | wait_for_completion(&net_device->channel_init_wait); |
| 466 | |||
| 467 | if (t == 0) | ||
| 468 | return -ETIMEDOUT; | ||
| 469 | 461 | ||
| 470 | if (init_packet->msg.init_msg.init_complete.status != | 462 | if (init_packet->msg.init_msg.init_complete.status != |
| 471 | NVSP_STAT_SUCCESS) | 463 | NVSP_STAT_SUCCESS) |
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c index 979fa4455f72..8e830f741d47 100644 --- a/drivers/net/hyperv/rndis_filter.c +++ b/drivers/net/hyperv/rndis_filter.c | |||
| @@ -466,7 +466,6 @@ static int rndis_filter_query_device(struct rndis_device *dev, u32 oid, | |||
| 466 | struct rndis_query_request *query; | 466 | struct rndis_query_request *query; |
| 467 | struct rndis_query_complete *query_complete; | 467 | struct rndis_query_complete *query_complete; |
| 468 | int ret = 0; | 468 | int ret = 0; |
| 469 | unsigned long t; | ||
| 470 | 469 | ||
| 471 | if (!result) | 470 | if (!result) |
| 472 | return -EINVAL; | 471 | return -EINVAL; |
| @@ -503,11 +502,7 @@ static int rndis_filter_query_device(struct rndis_device *dev, u32 oid, | |||
| 503 | if (ret != 0) | 502 | if (ret != 0) |
| 504 | goto cleanup; | 503 | goto cleanup; |
| 505 | 504 | ||
| 506 | t = wait_for_completion_timeout(&request->wait_event, 5*HZ); | 505 | wait_for_completion(&request->wait_event); |
| 507 | if (t == 0) { | ||
| 508 | ret = -ETIMEDOUT; | ||
| 509 | goto cleanup; | ||
| 510 | } | ||
| 511 | 506 | ||
| 512 | /* Copy the response back */ | 507 | /* Copy the response back */ |
| 513 | query_complete = &request->response_msg.msg.query_complete; | 508 | query_complete = &request->response_msg.msg.query_complete; |
| @@ -556,7 +551,6 @@ int rndis_filter_set_device_mac(struct net_device *ndev, char *mac) | |||
| 556 | u32 extlen = sizeof(struct rndis_config_parameter_info) + | 551 | u32 extlen = sizeof(struct rndis_config_parameter_info) + |
| 557 | 2*NWADR_STRLEN + 4*ETH_ALEN; | 552 | 2*NWADR_STRLEN + 4*ETH_ALEN; |
| 558 | int ret; | 553 | int ret; |
| 559 | unsigned long t; | ||
| 560 | 554 | ||
| 561 | request = get_rndis_request(rdev, RNDIS_MSG_SET, | 555 | request = get_rndis_request(rdev, RNDIS_MSG_SET, |
| 562 | RNDIS_MESSAGE_SIZE(struct rndis_set_request) + extlen); | 556 | RNDIS_MESSAGE_SIZE(struct rndis_set_request) + extlen); |
| @@ -597,21 +591,13 @@ int rndis_filter_set_device_mac(struct net_device *ndev, char *mac) | |||
| 597 | if (ret != 0) | 591 | if (ret != 0) |
| 598 | goto cleanup; | 592 | goto cleanup; |
| 599 | 593 | ||
| 600 | t = wait_for_completion_timeout(&request->wait_event, 5*HZ); | 594 | wait_for_completion(&request->wait_event); |
| 601 | if (t == 0) { | 595 | |
| 602 | netdev_err(ndev, "timeout before we got a set response...\n"); | 596 | set_complete = &request->response_msg.msg.set_complete; |
| 603 | /* | 597 | if (set_complete->status != RNDIS_STATUS_SUCCESS) { |
| 604 | * can't put_rndis_request, since we may still receive a | 598 | netdev_err(ndev, "Fail to set MAC on host side:0x%x\n", |
| 605 | * send-completion. | 599 | set_complete->status); |
| 606 | */ | 600 | ret = -EINVAL; |
| 607 | return -EBUSY; | ||
| 608 | } else { | ||
| 609 | set_complete = &request->response_msg.msg.set_complete; | ||
| 610 | if (set_complete->status != RNDIS_STATUS_SUCCESS) { | ||
| 611 | netdev_err(ndev, "Fail to set MAC on host side:0x%x\n", | ||
| 612 | set_complete->status); | ||
| 613 | ret = -EINVAL; | ||
| 614 | } | ||
| 615 | } | 601 | } |
| 616 | 602 | ||
| 617 | cleanup: | 603 | cleanup: |
| @@ -631,7 +617,6 @@ rndis_filter_set_offload_params(struct net_device *ndev, | |||
| 631 | struct rndis_set_complete *set_complete; | 617 | struct rndis_set_complete *set_complete; |
| 632 | u32 extlen = sizeof(struct ndis_offload_params); | 618 | u32 extlen = sizeof(struct ndis_offload_params); |
| 633 | int ret; | 619 | int ret; |
| 634 | unsigned long t; | ||
| 635 | u32 vsp_version = nvdev->nvsp_version; | 620 | u32 vsp_version = nvdev->nvsp_version; |
| 636 | 621 | ||
| 637 | if (vsp_version <= NVSP_PROTOCOL_VERSION_4) { | 622 | if (vsp_version <= NVSP_PROTOCOL_VERSION_4) { |
| @@ -665,20 +650,12 @@ rndis_filter_set_offload_params(struct net_device *ndev, | |||
| 665 | if (ret != 0) | 650 | if (ret != 0) |
| 666 | goto cleanup; | 651 | goto cleanup; |
| 667 | 652 | ||
| 668 | t = wait_for_completion_timeout(&request->wait_event, 5*HZ); | 653 | wait_for_completion(&request->wait_event); |
| 669 | if (t == 0) { | 654 | set_complete = &request->response_msg.msg.set_complete; |
| 670 | netdev_err(ndev, "timeout before we got aOFFLOAD set response...\n"); | 655 | if (set_complete->status != RNDIS_STATUS_SUCCESS) { |
| 671 | /* can't put_rndis_request, since we may still receive a | 656 | netdev_err(ndev, "Fail to set offload on host side:0x%x\n", |
| 672 | * send-completion. | 657 | set_complete->status); |
| 673 | */ | 658 | ret = -EINVAL; |
| 674 | return -EBUSY; | ||
| 675 | } else { | ||
| 676 | set_complete = &request->response_msg.msg.set_complete; | ||
| 677 | if (set_complete->status != RNDIS_STATUS_SUCCESS) { | ||
| 678 | netdev_err(ndev, "Fail to set offload on host side:0x%x\n", | ||
| 679 | set_complete->status); | ||
| 680 | ret = -EINVAL; | ||
| 681 | } | ||
| 682 | } | 659 | } |
| 683 | 660 | ||
| 684 | cleanup: | 661 | cleanup: |
| @@ -706,7 +683,6 @@ static int rndis_filter_set_rss_param(struct rndis_device *rdev, int num_queue) | |||
| 706 | u32 *itab; | 683 | u32 *itab; |
| 707 | u8 *keyp; | 684 | u8 *keyp; |
| 708 | int i, ret; | 685 | int i, ret; |
| 709 | unsigned long t; | ||
| 710 | 686 | ||
| 711 | request = get_rndis_request( | 687 | request = get_rndis_request( |
| 712 | rdev, RNDIS_MSG_SET, | 688 | rdev, RNDIS_MSG_SET, |
| @@ -749,20 +725,12 @@ static int rndis_filter_set_rss_param(struct rndis_device *rdev, int num_queue) | |||
| 749 | if (ret != 0) | 725 | if (ret != 0) |
| 750 | goto cleanup; | 726 | goto cleanup; |
| 751 | 727 | ||
| 752 | t = wait_for_completion_timeout(&request->wait_event, 5*HZ); | 728 | wait_for_completion(&request->wait_event); |
| 753 | if (t == 0) { | 729 | set_complete = &request->response_msg.msg.set_complete; |
| 754 | netdev_err(ndev, "timeout before we got a set response...\n"); | 730 | if (set_complete->status != RNDIS_STATUS_SUCCESS) { |
| 755 | /* can't put_rndis_request, since we may still receive a | 731 | netdev_err(ndev, "Fail to set RSS parameters:0x%x\n", |
| 756 | * send-completion. | 732 | set_complete->status); |
| 757 | */ | 733 | ret = -EINVAL; |
| 758 | return -ETIMEDOUT; | ||
| 759 | } else { | ||
| 760 | set_complete = &request->response_msg.msg.set_complete; | ||
| 761 | if (set_complete->status != RNDIS_STATUS_SUCCESS) { | ||
| 762 | netdev_err(ndev, "Fail to set RSS parameters:0x%x\n", | ||
| 763 | set_complete->status); | ||
| 764 | ret = -EINVAL; | ||
| 765 | } | ||
| 766 | } | 734 | } |
| 767 | 735 | ||
| 768 | cleanup: | 736 | cleanup: |
| @@ -791,8 +759,6 @@ int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter) | |||
| 791 | struct rndis_set_complete *set_complete; | 759 | struct rndis_set_complete *set_complete; |
| 792 | u32 status; | 760 | u32 status; |
| 793 | int ret; | 761 | int ret; |
| 794 | unsigned long t; | ||
| 795 | struct net_device *ndev = dev->ndev; | ||
| 796 | 762 | ||
| 797 | request = get_rndis_request(dev, RNDIS_MSG_SET, | 763 | request = get_rndis_request(dev, RNDIS_MSG_SET, |
| 798 | RNDIS_MESSAGE_SIZE(struct rndis_set_request) + | 764 | RNDIS_MESSAGE_SIZE(struct rndis_set_request) + |
| @@ -815,26 +781,14 @@ int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter) | |||
| 815 | if (ret != 0) | 781 | if (ret != 0) |
| 816 | goto cleanup; | 782 | goto cleanup; |
| 817 | 783 | ||
| 818 | t = wait_for_completion_timeout(&request->wait_event, 5*HZ); | 784 | wait_for_completion(&request->wait_event); |
| 819 | 785 | ||
| 820 | if (t == 0) { | 786 | set_complete = &request->response_msg.msg.set_complete; |
| 821 | netdev_err(ndev, | 787 | status = set_complete->status; |
| 822 | "timeout before we got a set response...\n"); | ||
| 823 | ret = -ETIMEDOUT; | ||
| 824 | /* | ||
| 825 | * We can't deallocate the request since we may still receive a | ||
| 826 | * send completion for it. | ||
| 827 | */ | ||
| 828 | goto exit; | ||
| 829 | } else { | ||
| 830 | set_complete = &request->response_msg.msg.set_complete; | ||
| 831 | status = set_complete->status; | ||
| 832 | } | ||
| 833 | 788 | ||
| 834 | cleanup: | 789 | cleanup: |
| 835 | if (request) | 790 | if (request) |
| 836 | put_rndis_request(dev, request); | 791 | put_rndis_request(dev, request); |
| 837 | exit: | ||
| 838 | return ret; | 792 | return ret; |
| 839 | } | 793 | } |
| 840 | 794 | ||
| @@ -846,7 +800,6 @@ static int rndis_filter_init_device(struct rndis_device *dev) | |||
| 846 | struct rndis_initialize_complete *init_complete; | 800 | struct rndis_initialize_complete *init_complete; |
| 847 | u32 status; | 801 | u32 status; |
| 848 | int ret; | 802 | int ret; |
| 849 | unsigned long t; | ||
| 850 | struct netvsc_device *nvdev = net_device_to_netvsc_device(dev->ndev); | 803 | struct netvsc_device *nvdev = net_device_to_netvsc_device(dev->ndev); |
| 851 | 804 | ||
| 852 | request = get_rndis_request(dev, RNDIS_MSG_INIT, | 805 | request = get_rndis_request(dev, RNDIS_MSG_INIT, |
| @@ -870,12 +823,7 @@ static int rndis_filter_init_device(struct rndis_device *dev) | |||
| 870 | goto cleanup; | 823 | goto cleanup; |
| 871 | } | 824 | } |
| 872 | 825 | ||
| 873 | t = wait_for_completion_timeout(&request->wait_event, 5*HZ); | 826 | wait_for_completion(&request->wait_event); |
| 874 | |||
| 875 | if (t == 0) { | ||
| 876 | ret = -ETIMEDOUT; | ||
| 877 | goto cleanup; | ||
| 878 | } | ||
| 879 | 827 | ||
| 880 | init_complete = &request->response_msg.msg.init_complete; | 828 | init_complete = &request->response_msg.msg.init_complete; |
| 881 | status = init_complete->status; | 829 | status = init_complete->status; |
| @@ -1008,7 +956,6 @@ int rndis_filter_device_add(struct hv_device *dev, | |||
| 1008 | struct netvsc_device_info *device_info = additional_info; | 956 | struct netvsc_device_info *device_info = additional_info; |
| 1009 | struct ndis_offload_params offloads; | 957 | struct ndis_offload_params offloads; |
| 1010 | struct nvsp_message *init_packet; | 958 | struct nvsp_message *init_packet; |
| 1011 | unsigned long t; | ||
| 1012 | struct ndis_recv_scale_cap rsscap; | 959 | struct ndis_recv_scale_cap rsscap; |
| 1013 | u32 rsscap_size = sizeof(struct ndis_recv_scale_cap); | 960 | u32 rsscap_size = sizeof(struct ndis_recv_scale_cap); |
| 1014 | u32 mtu, size; | 961 | u32 mtu, size; |
| @@ -1151,11 +1098,8 @@ int rndis_filter_device_add(struct hv_device *dev, | |||
| 1151 | VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); | 1098 | VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); |
| 1152 | if (ret) | 1099 | if (ret) |
| 1153 | goto out; | 1100 | goto out; |
| 1154 | t = wait_for_completion_timeout(&net_device->channel_init_wait, 5*HZ); | 1101 | wait_for_completion(&net_device->channel_init_wait); |
| 1155 | if (t == 0) { | 1102 | |
| 1156 | ret = -ETIMEDOUT; | ||
| 1157 | goto out; | ||
| 1158 | } | ||
| 1159 | if (init_packet->msg.v5_msg.subchn_comp.status != | 1103 | if (init_packet->msg.v5_msg.subchn_comp.status != |
| 1160 | NVSP_STAT_SUCCESS) { | 1104 | NVSP_STAT_SUCCESS) { |
| 1161 | ret = -ENODEV; | 1105 | ret = -ENODEV; |
| @@ -1192,17 +1136,12 @@ void rndis_filter_device_remove(struct hv_device *dev) | |||
| 1192 | { | 1136 | { |
| 1193 | struct netvsc_device *net_dev = hv_device_to_netvsc_device(dev); | 1137 | struct netvsc_device *net_dev = hv_device_to_netvsc_device(dev); |
| 1194 | struct rndis_device *rndis_dev = net_dev->extension; | 1138 | struct rndis_device *rndis_dev = net_dev->extension; |
| 1195 | unsigned long t; | ||
| 1196 | 1139 | ||
| 1197 | /* If not all subchannel offers are complete, wait for them until | 1140 | /* If not all subchannel offers are complete, wait for them until |
| 1198 | * completion to avoid race. | 1141 | * completion to avoid race. |
| 1199 | */ | 1142 | */ |
| 1200 | while (net_dev->num_sc_offered > 0) { | 1143 | if (net_dev->num_sc_offered > 0) |
| 1201 | t = wait_for_completion_timeout(&net_dev->channel_init_wait, | 1144 | wait_for_completion(&net_dev->channel_init_wait); |
| 1202 | 10 * HZ); | ||
| 1203 | if (t == 0) | ||
| 1204 | WARN(1, "Netvsc: Waiting for sub-channel processing"); | ||
| 1205 | } | ||
| 1206 | 1145 | ||
| 1207 | /* Halt and release the rndis device */ | 1146 | /* Halt and release the rndis device */ |
| 1208 | rndis_filter_halt_device(rndis_dev); | 1147 | rndis_filter_halt_device(rndis_dev); |
