Commit 860d7ef6 authored by David S. Miller's avatar David S. Miller

Merge branch 'hv_netvsc-races'

Vitaly Kuznetsov says:

====================
hv_netvsc: avoid races on mtu change/set channels

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.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents da47b457 88098834
...@@ -158,7 +158,7 @@ enum rndis_device_state { ...@@ -158,7 +158,7 @@ enum rndis_device_state {
}; };
struct rndis_device { struct rndis_device {
struct netvsc_device *net_dev; struct net_device *ndev;
enum rndis_device_state state; enum rndis_device_state state;
bool link_state; bool link_state;
...@@ -202,7 +202,7 @@ int rndis_filter_receive(struct hv_device *dev, ...@@ -202,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_packet_filter(struct rndis_device *dev, u32 new_filter);
int rndis_filter_set_device_mac(struct hv_device *hdev, char *mac); 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) #define NVSP_INVALID_PROTOCOL_VERSION ((u32)0xFFFFFFFF)
...@@ -653,6 +653,8 @@ struct garp_wrk { ...@@ -653,6 +653,8 @@ struct garp_wrk {
struct net_device_context { struct net_device_context {
/* point back to our device context */ /* point back to our device context */
struct hv_device *device_ctx; struct hv_device *device_ctx;
/* netvsc_device */
struct netvsc_device *nvdev;
/* reconfigure work */ /* reconfigure work */
struct delayed_work dwork; struct delayed_work dwork;
/* last reconfig time */ /* last reconfig time */
...@@ -672,17 +674,17 @@ struct net_device_context { ...@@ -672,17 +674,17 @@ struct net_device_context {
/* Ethtool settings */ /* Ethtool settings */
u8 duplex; u8 duplex;
u32 speed; u32 speed;
/* the device is going away */
bool start_remove;
}; };
/* Per netvsc device */ /* Per netvsc device */
struct netvsc_device { struct netvsc_device {
struct hv_device *dev;
u32 nvsp_version; u32 nvsp_version;
atomic_t num_outstanding_sends; atomic_t num_outstanding_sends;
wait_queue_head_t wait_drain; wait_queue_head_t wait_drain;
bool start_remove;
bool destroy; bool destroy;
/* Receive buffer allocated by us but manages by NetVSP */ /* Receive buffer allocated by us but manages by NetVSP */
...@@ -708,8 +710,6 @@ struct netvsc_device { ...@@ -708,8 +710,6 @@ struct netvsc_device {
struct nvsp_message revoke_packet; struct nvsp_message revoke_packet;
/* unsigned char HwMacAddr[HW_MACADDR_LEN]; */ /* unsigned char HwMacAddr[HW_MACADDR_LEN]; */
struct net_device *ndev;
struct vmbus_channel *chn_table[VRSS_CHANNEL_MAX]; struct vmbus_channel *chn_table[VRSS_CHANNEL_MAX];
u32 send_table[VRSS_SEND_TAB_SIZE]; u32 send_table[VRSS_SEND_TAB_SIZE];
u32 max_chn; u32 max_chn;
...@@ -732,9 +732,6 @@ struct netvsc_device { ...@@ -732,9 +732,6 @@ struct netvsc_device {
u32 max_pkt; /* max number of pkt in one send, e.g. 8 */ u32 max_pkt; /* max number of pkt in one send, e.g. 8 */
u32 pkt_align; /* alignment bytes, 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 */ /* 1: allocated, serial number is valid. 0: not allocated */
u32 vf_alloc; u32 vf_alloc;
/* Serial number of the VF to team with */ /* Serial number of the VF to team with */
......
This diff is collapsed.
This diff is collapsed.
...@@ -126,11 +126,7 @@ static void put_rndis_request(struct rndis_device *dev, ...@@ -126,11 +126,7 @@ static void put_rndis_request(struct rndis_device *dev,
static void dump_rndis_message(struct hv_device *hv_dev, static void dump_rndis_message(struct hv_device *hv_dev,
struct rndis_message *rndis_msg) struct rndis_message *rndis_msg)
{ {
struct net_device *netdev; struct net_device *netdev = hv_get_drvdata(hv_dev);
struct netvsc_device *net_device;
net_device = hv_get_drvdata(hv_dev);
netdev = net_device->ndev;
switch (rndis_msg->ndis_msg_type) { switch (rndis_msg->ndis_msg_type) {
case RNDIS_MSG_PACKET: case RNDIS_MSG_PACKET:
...@@ -211,6 +207,7 @@ static int rndis_filter_send_request(struct rndis_device *dev, ...@@ -211,6 +207,7 @@ static int rndis_filter_send_request(struct rndis_device *dev,
struct hv_netvsc_packet *packet; struct hv_netvsc_packet *packet;
struct hv_page_buffer page_buf[2]; struct hv_page_buffer page_buf[2];
struct hv_page_buffer *pb = page_buf; struct hv_page_buffer *pb = page_buf;
struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
/* Setup the packet to send it */ /* Setup the packet to send it */
packet = &req->pkt; packet = &req->pkt;
...@@ -236,7 +233,7 @@ static int rndis_filter_send_request(struct rndis_device *dev, ...@@ -236,7 +233,7 @@ static int rndis_filter_send_request(struct rndis_device *dev,
pb[0].len; 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; return ret;
} }
...@@ -262,9 +259,7 @@ static void rndis_filter_receive_response(struct rndis_device *dev, ...@@ -262,9 +259,7 @@ static void rndis_filter_receive_response(struct rndis_device *dev,
struct rndis_request *request = NULL; struct rndis_request *request = NULL;
bool found = false; bool found = false;
unsigned long flags; unsigned long flags;
struct net_device *ndev; struct net_device *ndev = dev->ndev;
ndev = dev->net_dev->ndev;
spin_lock_irqsave(&dev->request_lock, flags); spin_lock_irqsave(&dev->request_lock, flags);
list_for_each_entry(request, &dev->req_list, list_ent) { list_for_each_entry(request, &dev->req_list, list_ent) {
...@@ -355,6 +350,7 @@ static int rndis_filter_receive_data(struct rndis_device *dev, ...@@ -355,6 +350,7 @@ static int rndis_filter_receive_data(struct rndis_device *dev,
struct ndis_pkt_8021q_info *vlan; struct ndis_pkt_8021q_info *vlan;
struct ndis_tcp_ip_checksum_info *csum_info; struct ndis_tcp_ip_checksum_info *csum_info;
u16 vlan_tci = 0; u16 vlan_tci = 0;
struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
rndis_pkt = &msg->msg.pkt; rndis_pkt = &msg->msg.pkt;
...@@ -368,7 +364,7 @@ static int rndis_filter_receive_data(struct rndis_device *dev, ...@@ -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 * should be the data packet size plus the trailer padding size
*/ */
if (pkt->total_data_buflen < rndis_pkt->data_len) { 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)" "overflow detected (got %u, min %u)"
"...dropping this message!\n", "...dropping this message!\n",
pkt->total_data_buflen, rndis_pkt->data_len); pkt->total_data_buflen, rndis_pkt->data_len);
...@@ -390,7 +386,7 @@ static int rndis_filter_receive_data(struct rndis_device *dev, ...@@ -390,7 +386,7 @@ static int rndis_filter_receive_data(struct rndis_device *dev,
} }
csum_info = rndis_get_ppi(rndis_pkt, TCPIP_CHKSUM_PKTINFO); 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); csum_info, channel, vlan_tci);
} }
...@@ -399,10 +395,11 @@ int rndis_filter_receive(struct hv_device *dev, ...@@ -399,10 +395,11 @@ int rndis_filter_receive(struct hv_device *dev,
void **data, void **data,
struct vmbus_channel *channel) 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_device *rndis_dev;
struct rndis_message *rndis_msg; struct rndis_message *rndis_msg;
struct net_device *ndev;
int ret = 0; int ret = 0;
if (!net_dev) { if (!net_dev) {
...@@ -410,8 +407,6 @@ int rndis_filter_receive(struct hv_device *dev, ...@@ -410,8 +407,6 @@ int rndis_filter_receive(struct hv_device *dev,
goto exit; goto exit;
} }
ndev = net_dev->ndev;
/* Make sure the rndis device state is initialized */ /* Make sure the rndis device state is initialized */
if (!net_dev->extension) { if (!net_dev->extension) {
netdev_err(ndev, "got rndis message but no rndis device - " netdev_err(ndev, "got rndis message but no rndis device - "
...@@ -430,7 +425,7 @@ int rndis_filter_receive(struct hv_device *dev, ...@@ -430,7 +425,7 @@ int rndis_filter_receive(struct hv_device *dev,
rndis_msg = *data; 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); dump_rndis_message(dev, rndis_msg);
switch (rndis_msg->ndis_msg_type) { switch (rndis_msg->ndis_msg_type) {
...@@ -550,9 +545,10 @@ static int rndis_filter_query_device_mac(struct rndis_device *dev) ...@@ -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) 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 rndis_device *rdev = nvdev->extension;
struct net_device *ndev = nvdev->ndev;
struct rndis_request *request; struct rndis_request *request;
struct rndis_set_request *set; struct rndis_set_request *set;
struct rndis_config_parameter_info *cpi; struct rndis_config_parameter_info *cpi;
...@@ -629,9 +625,10 @@ static int ...@@ -629,9 +625,10 @@ static int
rndis_filter_set_offload_params(struct hv_device *hdev, rndis_filter_set_offload_params(struct hv_device *hdev,
struct ndis_offload_params *req_offloads) 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 rndis_device *rdev = nvdev->extension;
struct net_device *ndev = nvdev->ndev;
struct rndis_request *request; struct rndis_request *request;
struct rndis_set_request *set; struct rndis_set_request *set;
struct ndis_offload_params *offload_params; struct ndis_offload_params *offload_params;
...@@ -703,7 +700,7 @@ u8 netvsc_hash_key[HASH_KEYLEN] = { ...@@ -703,7 +700,7 @@ u8 netvsc_hash_key[HASH_KEYLEN] = {
static int rndis_filter_set_rss_param(struct rndis_device *rdev, int num_queue) 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_request *request;
struct rndis_set_request *set; struct rndis_set_request *set;
struct rndis_set_complete *set_complete; struct rndis_set_complete *set_complete;
...@@ -799,9 +796,7 @@ int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter) ...@@ -799,9 +796,7 @@ int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter)
u32 status; u32 status;
int ret; int ret;
unsigned long t; unsigned long t;
struct net_device *ndev; struct net_device *ndev = dev->ndev;
ndev = dev->net_dev->ndev;
request = get_rndis_request(dev, RNDIS_MSG_SET, request = get_rndis_request(dev, RNDIS_MSG_SET,
RNDIS_MESSAGE_SIZE(struct rndis_set_request) + RNDIS_MESSAGE_SIZE(struct rndis_set_request) +
...@@ -856,7 +851,8 @@ static int rndis_filter_init_device(struct rndis_device *dev) ...@@ -856,7 +851,8 @@ static int rndis_filter_init_device(struct rndis_device *dev)
u32 status; u32 status;
int ret; int ret;
unsigned long t; 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, request = get_rndis_request(dev, RNDIS_MSG_INIT,
RNDIS_MESSAGE_SIZE(struct rndis_initialize_request)); RNDIS_MESSAGE_SIZE(struct rndis_initialize_request));
...@@ -879,7 +875,6 @@ static int rndis_filter_init_device(struct rndis_device *dev) ...@@ -879,7 +875,6 @@ static int rndis_filter_init_device(struct rndis_device *dev)
goto cleanup; goto cleanup;
} }
t = wait_for_completion_timeout(&request->wait_event, 5*HZ); t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
if (t == 0) { if (t == 0) {
...@@ -910,8 +905,9 @@ static void rndis_filter_halt_device(struct rndis_device *dev) ...@@ -910,8 +905,9 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
{ {
struct rndis_request *request; struct rndis_request *request;
struct rndis_halt_request *halt; struct rndis_halt_request *halt;
struct netvsc_device *nvdev = dev->net_dev; struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
struct hv_device *hdev = nvdev->dev; struct netvsc_device *nvdev = net_device_ctx->nvdev;
struct hv_device *hdev = net_device_ctx->device_ctx;
ulong flags; ulong flags;
/* Attempt to do a rndis device halt */ /* Attempt to do a rndis device halt */
...@@ -979,13 +975,14 @@ static int rndis_filter_close_device(struct rndis_device *dev) ...@@ -979,13 +975,14 @@ static int rndis_filter_close_device(struct rndis_device *dev)
static void netvsc_sc_open(struct vmbus_channel *new_sc) 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; u16 chn_index = new_sc->offermsg.offer.sub_channel_index;
int ret; int ret;
unsigned long flags; unsigned long flags;
nvscdev = hv_get_drvdata(new_sc->primary_channel->device_obj);
if (chn_index >= nvscdev->num_chn) if (chn_index >= nvscdev->num_chn)
return; return;
...@@ -1010,6 +1007,8 @@ int rndis_filter_device_add(struct hv_device *dev, ...@@ -1010,6 +1007,8 @@ int rndis_filter_device_add(struct hv_device *dev,
void *additional_info) void *additional_info)
{ {
int ret; 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 netvsc_device *net_device;
struct rndis_device *rndis_device; struct rndis_device *rndis_device;
struct netvsc_device_info *device_info = additional_info; struct netvsc_device_info *device_info = additional_info;
...@@ -1040,16 +1039,15 @@ int rndis_filter_device_add(struct hv_device *dev, ...@@ -1040,16 +1039,15 @@ int rndis_filter_device_add(struct hv_device *dev,
return ret; return ret;
} }
/* Initialize the rndis device */ /* Initialize the rndis device */
net_device = hv_get_drvdata(dev); net_device = net_device_ctx->nvdev;
net_device->max_chn = 1; net_device->max_chn = 1;
net_device->num_chn = 1; net_device->num_chn = 1;
spin_lock_init(&net_device->sc_lock); spin_lock_init(&net_device->sc_lock);
net_device->extension = rndis_device; net_device->extension = rndis_device;
rndis_device->net_dev = net_device; rndis_device->ndev = net;
/* Send the rndis initialization message */ /* Send the rndis initialization message */
ret = rndis_filter_init_device(rndis_device); ret = rndis_filter_init_device(rndis_device);
...@@ -1063,8 +1061,8 @@ int rndis_filter_device_add(struct hv_device *dev, ...@@ -1063,8 +1061,8 @@ int rndis_filter_device_add(struct hv_device *dev,
ret = rndis_filter_query_device(rndis_device, ret = rndis_filter_query_device(rndis_device,
RNDIS_OID_GEN_MAXIMUM_FRAME_SIZE, RNDIS_OID_GEN_MAXIMUM_FRAME_SIZE,
&mtu, &size); &mtu, &size);
if (ret == 0 && size == sizeof(u32) && mtu < net_device->ndev->mtu) if (ret == 0 && size == sizeof(u32) && mtu < net->mtu)
net_device->ndev->mtu = mtu; net->mtu = mtu;
/* Get the mac address */ /* Get the mac address */
ret = rndis_filter_query_device_mac(rndis_device); ret = rndis_filter_query_device_mac(rndis_device);
...@@ -1198,7 +1196,9 @@ int rndis_filter_device_add(struct hv_device *dev, ...@@ -1198,7 +1196,9 @@ int rndis_filter_device_add(struct hv_device *dev,
void rndis_filter_device_remove(struct hv_device *dev) 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; struct rndis_device *rndis_dev = net_dev->extension;
unsigned long t; unsigned long t;
...@@ -1224,7 +1224,9 @@ void rndis_filter_device_remove(struct hv_device *dev) ...@@ -1224,7 +1224,9 @@ void rndis_filter_device_remove(struct hv_device *dev)
int rndis_filter_open(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) if (!net_device)
return -EINVAL; return -EINVAL;
...@@ -1237,7 +1239,9 @@ int rndis_filter_open(struct hv_device *dev) ...@@ -1237,7 +1239,9 @@ int rndis_filter_open(struct hv_device *dev)
int rndis_filter_close(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) if (!nvdev)
return -EINVAL; return -EINVAL;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment