From: Stephen Hemminger <stephen@networkplumber.org>
To: kys@microsoft.com, haiyangz@microsoft.com, sthemmin@microsoft.com
Cc: devel@linuxdriverproject.org, netdev@vger.kernel.org
Subject: [PATCH net-next 2/5] netvsc: fix warnings reported by lockdep
Date: Tue, 25 Jul 2017 13:04:19 -0700 [thread overview]
Message-ID: <20170725200422.13795-3-sthemmin@microsoft.com> (raw)
In-Reply-To: <20170725200422.13795-1-sthemmin@microsoft.com>
This includes a bunch of fixups for issues reported by sparse and
lockdep.
* ethtool routines can assume RTNL
* send is done with RCU lock (and BH disable)
Most of the changes involve passing internal device struct (netvsc)
as parameter, so that code called in both initialization and changes
to MTU and channels doesn't have to worry about whether the value
is protected by RTNL, RCU, or the fact it isn't registered yet.
One special case is the callback that happens when a new VMBUS
channel is added. The setup code is waiting for completion with
RTNL held, so it is already safe.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/net/hyperv/hyperv_net.h | 3 +-
drivers/net/hyperv/netvsc.c | 2 +-
drivers/net/hyperv/netvsc_drv.c | 15 ++++---
drivers/net/hyperv/rndis_filter.c | 84 +++++++++++++++++++--------------------
4 files changed, 54 insertions(+), 50 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 4e7ff348327e..fb62ea632914 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -217,7 +217,8 @@ int rndis_filter_receive(struct net_device *ndev,
struct vmbus_channel *channel,
void *data, u32 buflen);
-int rndis_filter_set_device_mac(struct net_device *ndev, char *mac);
+int rndis_filter_set_device_mac(struct netvsc_device *ndev,
+ const char *mac);
void netvsc_switch_datapath(struct net_device *nv_dev, bool vf);
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 06f39a99da7c..94c00acac58a 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -832,7 +832,7 @@ int netvsc_send(struct net_device_context *ndev_ctx,
struct sk_buff *skb)
{
struct netvsc_device *net_device
- = rcu_dereference_rtnl(ndev_ctx->nvdev);
+ = rcu_dereference_bh(ndev_ctx->nvdev);
struct hv_device *device = ndev_ctx->device_ctx;
int ret = 0;
struct netvsc_channel *nvchan;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f1eaf675d2e9..a04f2efbbc25 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -923,6 +923,8 @@ static void netvsc_get_stats64(struct net_device *net,
static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
{
+ struct net_device_context *ndc = netdev_priv(ndev);
+ struct netvsc_device *nvdev = rtnl_dereference(ndc->nvdev);
struct sockaddr *addr = p;
char save_adr[ETH_ALEN];
unsigned char save_aatype;
@@ -935,7 +937,10 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
if (err != 0)
return err;
- err = rndis_filter_set_device_mac(ndev, addr->sa_data);
+ if (!nvdev)
+ return -ENODEV;
+
+ err = rndis_filter_set_device_mac(nvdev, addr->sa_data);
if (err != 0) {
/* roll back to saved MAC */
memcpy(ndev->dev_addr, save_adr, ETH_ALEN);
@@ -981,7 +986,7 @@ static void netvsc_get_ethtool_stats(struct net_device *dev,
struct ethtool_stats *stats, u64 *data)
{
struct net_device_context *ndc = netdev_priv(dev);
- struct netvsc_device *nvdev = rcu_dereference(ndc->nvdev);
+ struct netvsc_device *nvdev = rtnl_dereference(ndc->nvdev);
const void *nds = &ndc->eth_stats;
const struct netvsc_stats *qstats;
unsigned int start;
@@ -1019,7 +1024,7 @@ static void netvsc_get_ethtool_stats(struct net_device *dev,
static void netvsc_get_strings(struct net_device *dev, u32 stringset, u8 *data)
{
struct net_device_context *ndc = netdev_priv(dev);
- struct netvsc_device *nvdev = rcu_dereference(ndc->nvdev);
+ struct netvsc_device *nvdev = rtnl_dereference(ndc->nvdev);
u8 *p = data;
int i;
@@ -1077,7 +1082,7 @@ netvsc_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
u32 *rules)
{
struct net_device_context *ndc = netdev_priv(dev);
- struct netvsc_device *nvdev = rcu_dereference(ndc->nvdev);
+ struct netvsc_device *nvdev = rtnl_dereference(ndc->nvdev);
if (!nvdev)
return -ENODEV;
@@ -1127,7 +1132,7 @@ static int netvsc_get_rxfh(struct net_device *dev, u32 *indir, u8 *key,
u8 *hfunc)
{
struct net_device_context *ndc = netdev_priv(dev);
- struct netvsc_device *ndev = rcu_dereference(ndc->nvdev);
+ struct netvsc_device *ndev = rtnl_dereference(ndc->nvdev);
struct rndis_device *rndis_dev;
int i;
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index eaa3f0d5682a..bf21ea92c743 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -85,14 +85,6 @@ static struct rndis_device *get_rndis_device(void)
return device;
}
-static struct netvsc_device *
-net_device_to_netvsc_device(struct net_device *ndev)
-{
- struct net_device_context *net_device_ctx = netdev_priv(ndev);
-
- return rtnl_dereference(net_device_ctx->nvdev);
-}
-
static struct rndis_request *get_rndis_request(struct rndis_device *dev,
u32 msg_type,
u32 msg_len)
@@ -252,7 +244,10 @@ static int rndis_filter_send_request(struct rndis_device *dev,
pb[0].len;
}
+ rcu_read_lock_bh();
ret = netvsc_send(net_device_ctx, packet, NULL, &pb, NULL);
+ rcu_read_unlock_bh();
+
return ret;
}
@@ -452,8 +447,9 @@ int rndis_filter_receive(struct net_device *ndev,
return 0;
}
-static int rndis_filter_query_device(struct rndis_device *dev, u32 oid,
- void *result, u32 *result_size)
+static int rndis_filter_query_device(struct rndis_device *dev,
+ struct netvsc_device *nvdev,
+ u32 oid, void *result, u32 *result_size)
{
struct rndis_request *request;
u32 inresult_size = *result_size;
@@ -480,8 +476,6 @@ static int rndis_filter_query_device(struct rndis_device *dev, u32 oid,
query->dev_vc_handle = 0;
if (oid == OID_TCP_OFFLOAD_HARDWARE_CAPABILITIES) {
- struct net_device_context *ndevctx = netdev_priv(dev->ndev);
- struct netvsc_device *nvdev = rtnl_dereference(ndevctx->nvdev);
struct ndis_offload *hwcaps;
u32 nvsp_version = nvdev->nvsp_version;
u8 ndis_rev;
@@ -550,14 +544,15 @@ static int rndis_filter_query_device(struct rndis_device *dev, u32 oid,
/* Get the hardware offload capabilities */
static int
-rndis_query_hwcaps(struct rndis_device *dev, struct ndis_offload *caps)
+rndis_query_hwcaps(struct rndis_device *dev, struct netvsc_device *net_device,
+ struct ndis_offload *caps)
{
u32 caps_len = sizeof(*caps);
int ret;
memset(caps, 0, sizeof(*caps));
- ret = rndis_filter_query_device(dev,
+ ret = rndis_filter_query_device(dev, net_device,
OID_TCP_OFFLOAD_HARDWARE_CAPABILITIES,
caps, &caps_len);
if (ret)
@@ -586,11 +581,12 @@ rndis_query_hwcaps(struct rndis_device *dev, struct ndis_offload *caps)
return 0;
}
-static int rndis_filter_query_device_mac(struct rndis_device *dev)
+static int rndis_filter_query_device_mac(struct rndis_device *dev,
+ struct netvsc_device *net_device)
{
u32 size = ETH_ALEN;
- return rndis_filter_query_device(dev,
+ return rndis_filter_query_device(dev, net_device,
RNDIS_OID_802_3_PERMANENT_ADDRESS,
dev->hw_mac_adr, &size);
}
@@ -598,9 +594,9 @@ static int rndis_filter_query_device_mac(struct rndis_device *dev)
#define NWADR_STR "NetworkAddress"
#define NWADR_STRLEN 14
-int rndis_filter_set_device_mac(struct net_device *ndev, char *mac)
+int rndis_filter_set_device_mac(struct netvsc_device *nvdev,
+ const char *mac)
{
- struct netvsc_device *nvdev = net_device_to_netvsc_device(ndev);
struct rndis_device *rdev = nvdev->extension;
struct rndis_request *request;
struct rndis_set_request *set;
@@ -654,11 +650,8 @@ int rndis_filter_set_device_mac(struct net_device *ndev, char *mac)
wait_for_completion(&request->wait_event);
set_complete = &request->response_msg.msg.set_complete;
- if (set_complete->status != RNDIS_STATUS_SUCCESS) {
- netdev_err(ndev, "Fail to set MAC on host side:0x%x\n",
- set_complete->status);
- ret = -EINVAL;
- }
+ if (set_complete->status != RNDIS_STATUS_SUCCESS)
+ ret = -EIO;
cleanup:
put_rndis_request(rdev, request);
@@ -791,27 +784,27 @@ int rndis_filter_set_rss_param(struct rndis_device *rdev,
return ret;
}
-static int rndis_filter_query_device_link_status(struct rndis_device *dev)
+static int rndis_filter_query_device_link_status(struct rndis_device *dev,
+ struct netvsc_device *net_device)
{
u32 size = sizeof(u32);
u32 link_status;
- int ret;
- ret = rndis_filter_query_device(dev,
- RNDIS_OID_GEN_MEDIA_CONNECT_STATUS,
- &link_status, &size);
-
- return ret;
+ return rndis_filter_query_device(dev, net_device,
+ RNDIS_OID_GEN_MEDIA_CONNECT_STATUS,
+ &link_status, &size);
}
-static int rndis_filter_query_link_speed(struct rndis_device *dev)
+static int rndis_filter_query_link_speed(struct rndis_device *dev,
+ struct netvsc_device *net_device)
{
u32 size = sizeof(u32);
u32 link_speed;
struct net_device_context *ndc;
int ret;
- ret = rndis_filter_query_device(dev, RNDIS_OID_GEN_LINK_SPEED,
+ ret = rndis_filter_query_device(dev, net_device,
+ RNDIS_OID_GEN_LINK_SPEED,
&link_speed, &size);
if (!ret) {
@@ -880,14 +873,14 @@ void rndis_filter_update(struct netvsc_device *nvdev)
schedule_work(&rdev->mcast_work);
}
-static int rndis_filter_init_device(struct rndis_device *dev)
+static int rndis_filter_init_device(struct rndis_device *dev,
+ struct netvsc_device *nvdev)
{
struct rndis_request *request;
struct rndis_initialize_request *init;
struct rndis_initialize_complete *init_complete;
u32 status;
int ret;
- struct netvsc_device *nvdev = net_device_to_netvsc_device(dev->ndev);
request = get_rndis_request(dev, RNDIS_MSG_INIT,
RNDIS_MESSAGE_SIZE(struct rndis_initialize_request));
@@ -1024,12 +1017,17 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
{
struct net_device *ndev =
hv_get_drvdata(new_sc->primary_channel->device_obj);
- struct netvsc_device *nvscdev = net_device_to_netvsc_device(ndev);
+ struct net_device_context *ndev_ctx = netdev_priv(ndev);
+ struct netvsc_device *nvscdev;
u16 chn_index = new_sc->offermsg.offer.sub_channel_index;
struct netvsc_channel *nvchan;
int ret;
- if (chn_index >= nvscdev->num_chn)
+ /* This is safe because this callback only happens when
+ * new device is being setup and waiting on the channel_init_wait.
+ */
+ nvscdev = rcu_dereference_raw(ndev_ctx->nvdev);
+ if (!nvscdev || chn_index >= nvscdev->num_chn)
return;
nvchan = nvscdev->chan_table + chn_index;
@@ -1104,27 +1102,27 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
rndis_device->ndev = net;
/* Send the rndis initialization message */
- ret = rndis_filter_init_device(rndis_device);
+ ret = rndis_filter_init_device(rndis_device, net_device);
if (ret != 0)
goto err_dev_remv;
/* Get the MTU from the host */
size = sizeof(u32);
- ret = rndis_filter_query_device(rndis_device,
+ ret = rndis_filter_query_device(rndis_device, net_device,
RNDIS_OID_GEN_MAXIMUM_FRAME_SIZE,
&mtu, &size);
if (ret == 0 && size == sizeof(u32) && mtu < net->mtu)
net->mtu = mtu;
/* Get the mac address */
- ret = rndis_filter_query_device_mac(rndis_device);
+ ret = rndis_filter_query_device_mac(rndis_device, net_device);
if (ret != 0)
goto err_dev_remv;
memcpy(device_info->mac_adr, rndis_device->hw_mac_adr, ETH_ALEN);
/* Find HW offload capabilities */
- ret = rndis_query_hwcaps(rndis_device, &hwcaps);
+ ret = rndis_query_hwcaps(rndis_device, net_device, &hwcaps);
if (ret != 0)
goto err_dev_remv;
@@ -1185,7 +1183,7 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
if (ret)
goto err_dev_remv;
- rndis_filter_query_device_link_status(rndis_device);
+ rndis_filter_query_device_link_status(rndis_device, net_device);
netdev_dbg(net, "Device MAC %pM link state %s\n",
rndis_device->hw_mac_adr,
@@ -1194,11 +1192,11 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_5)
return net_device;
- rndis_filter_query_link_speed(rndis_device);
+ rndis_filter_query_link_speed(rndis_device, net_device);
/* vRSS setup */
memset(&rsscap, 0, rsscap_size);
- ret = rndis_filter_query_device(rndis_device,
+ ret = rndis_filter_query_device(rndis_device, net_device,
OID_GEN_RECEIVE_SCALE_CAPABILITIES,
&rsscap, &rsscap_size);
if (ret || rsscap.num_recv_que < 2)
--
2.11.0
next prev parent reply other threads:[~2017-07-25 20:04 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-25 20:04 [PATCH net-next 0/5] netvsc: fixes and performance related changes Stephen Hemminger
2017-07-25 20:04 ` [PATCH net-next 1/5] netvsc: fix return value for set_channels Stephen Hemminger
2017-07-25 20:04 ` Stephen Hemminger [this message]
2017-07-25 20:04 ` [PATCH net-next 3/5] netvsc: optimize receive completions Stephen Hemminger
2017-07-25 21:07 ` Stephen Hemminger
2017-07-25 20:04 ` [PATCH net-next 4/5] netvsc: signal host if receive ring is emptied Stephen Hemminger
2017-07-25 20:04 ` [PATCH net-next 5/5] netvsc: allow smaller send/recv buffer size Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170725200422.13795-3-sthemmin@microsoft.com \
--to=stephen@networkplumber.org \
--cc=devel@linuxdriverproject.org \
--cc=haiyangz@microsoft.com \
--cc=kys@microsoft.com \
--cc=netdev@vger.kernel.org \
--cc=sthemmin@microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).