Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v2 0/3] ethtool: support for forward error correction mode setting on a link
From: Jakub Kicinski @ 2017-07-28 16:46 UTC (permalink / raw)
  To: Roopa Prabhu
  Cc: davem@davemloft.net, John W. Linville, netdev@vger.kernel.org,
	Vidya Sagar Ravipati, Dustin Byford, Dave Olson, Casey Leedom,
	Gal Pressman, Andrew Lunn, Manoj Malviya, Santosh Rastapur,
	yuval.mintz, odedw, Ariel Almog, Jeff Kirsher
In-Reply-To: <CAJieiUiXuN9s0ndJ4K6UVE2pomRi0aD9PH6uYHe12qCzL0+wSg@mail.gmail.com>

On Fri, 28 Jul 2017 07:53:01 -0700, Roopa Prabhu wrote:
> On Thu, Jul 27, 2017 at 7:33 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> > On Thu, 27 Jul 2017 16:47:25 -0700, Roopa Prabhu wrote:  
> >> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> >>
> >> Forward Error Correction (FEC) modes i.e Base-R
> >> and Reed-Solomon modes are introduced in 25G/40G/100G standards
> >> for providing good BER at high speeds. Various networking devices
> >> which support 25G/40G/100G provides ability to manage supported FEC
> >> modes and the lack of FEC encoding control and reporting today is a
> >> source for interoperability issues for many vendors.
> >> FEC capability as well as specific FEC mode i.e. Base-R
> >> or RS modes can be requested or advertised through bits D44:47 of base link
> >> codeword.
> >>
> >> This patch set intends to provide option under ethtool to manage and
> >> report FEC encoding settings for networking devices as per IEEE 802.3
> >> bj, bm and by specs.
> >>
> >> v2 :
> >>         - minor patch format fixes and typos pointed out by Andrew
> >>         - there was a pending discussion on the use of 'auto' vs
> >>           'automatic' for fec settings. I have left it as 'auto'
> >>           because in most cases today auto is used in place of
> >>           automatic to represent automatically generated values.
> >>           We use it in other networking config too. I would prefer
> >>           leaving it as auto.  
> >
> > On the subject of resetting the values when module is replugged I
> > assume what was previously described remains:
> >  - we always allow users to set the FEC regardless of the module type;
> >  - if user set an incorrect FEC for the module type (or module gets
> >    swapped) the link will be administratively taken down by either
> >    the driver or FW.
> >
> > Is that correct?  Am I misremembering?  
> 
> yes, correct. And possible future sfp hotplug events can give user-space
> more info to react to module type changes etc.

OK, if nobody else objects and we go with that - lets make sure we
document clearly those are expected :)  My concern is that if there is
ever 10G + RS FEC standard we don't want to end up in a situation where
some drivers silently ignore FEC settings in 10G and other apply it.
So let's make it clear what the intended Linux behaviour is.  It could
be in the ethtool man page, or the kernel somewhere.

^ permalink raw reply

* Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
From: Marc Gonzalez @ 2017-07-28 16:43 UTC (permalink / raw)
  To: Mans Rullgard; +Cc: Florian Fainelli, David S. Miller, netdev, Linux ARM, Mason
In-Reply-To: <yw1xk22slemx.fsf@mansr.com>

On 28/07/2017 18:17, Måns Rullgård wrote:

> Marc Gonzalez wrote:
> 
>> ndo_stop breaks RX in a way that ndo_open is unable to undo.
> 
> Please elaborate.  Why can't it be fixed in a less heavy-handed way?

I'm not sure what "elaborate" means. After we've been through
ndo_stop once, the board can send packets, but it doesn't see
any replies from remote systems. RX is wedged.

I think ndo_stop is rare enough an event that doing a full
reset is not an issue, in terms of performance. Also I will
need this infrastructure anyway for suspend/resume support.

It might also make sense to put the HW in reset at close
(to save power). I will try measuring the power savings,
if any.


>> Work around the issue by resetting the HW in ndo_open.
>> This will provide the basis for suspend/resume support.
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>>  drivers/net/ethernet/aurora/nb8800.c | 40 +++++++++++++++++-------------------
>>  drivers/net/ethernet/aurora/nb8800.h |  1 +
>>  2 files changed, 20 insertions(+), 21 deletions(-)
> 
> I'm pretty sure this doesn't preserve everything it should.

Hmmm, we're supposed to start fresh ("full reset").
What could there be to preserve?
You mentioned flow control and multicast elsewhere.
I will take a closer look. Thanks for the heads up.

Regards.

^ permalink raw reply

* Re: [v4.12 regression] netns: NULL deref in fib_sync_down_dev()
From: Ido Schimmel @ 2017-07-28 16:43 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev
In-Reply-To: <20170728160047.7tzapklj7zrgxktq@qmqm.qmqm.pl>

On Fri, Jul 28, 2017 at 06:00:47PM +0200, Michał Mirosław wrote:
> Dear NetDevs,
> 
> Before I go to bisecting, have you seen a following NULL dereference,
> yet?  Where should I start looking?  It is triggered by deleting netns
> (cut-down script attached - triggers every time).  This was working
> correctly under v4.11.x.

Thanks for the report. I just reproduced this on my system. I believe
the problem is a missing NULL check for 'in_dev' in
call_fib_nh_notifiers(). I'll test a fix.

^ permalink raw reply

* Re: [PATCH net] xgene: Don't fail probe, if there is no clk resource for SGMII interfaces
From: Laura Abbott @ 2017-07-28 16:42 UTC (permalink / raw)
  To: Tom Bogendoerfer; +Cc: isubramanian, kchudgar, qnguyen, netdev, linux-kernel
In-Reply-To: <20170728142318.GA20368@alpha.franken.de>

On 07/28/2017 07:23 AM, Tom Bogendoerfer wrote:
> On Thu, Jul 27, 2017 at 03:39:58PM -0700, Laura Abbott wrote:
>> I don't know the intricacies of the Mustang hardware but external
>> aborts have been a symptom of missing clocks on other hardware.
> 
> you are right, it's a missing clock. For SGMII ports the driver
> doesn't really use the clock source other then doing the one
> devm_clk_get(), but this is enough to get the clock going.
> 
> Below patch fixes the crash and brings back the second SGMII port
> (even without a clock source in device tree, I guess it uses clock
> from first port).  From a correctness standpoint there should probably 
> be a clk_prepare_enable(), if port is SGMII.
> 
> Could you test the patch ? If it works, I'll submit it.
> 
> Thomas.
> 
> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
> index 86058a9f3417..1d307f2def2d 100644
> --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
> @@ -1785,9 +1785,9 @@ static int xgene_enet_get_resources(struct xgene_enet_pdata *pdata)
>  
>  	xgene_enet_gpiod_get(pdata);
>  
> -	if (pdata->phy_mode != PHY_INTERFACE_MODE_SGMII) {
> -		pdata->clk = devm_clk_get(&pdev->dev, NULL);
> -		if (IS_ERR(pdata->clk)) {
> +	pdata->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(pdata->clk)) {
> +		if (pdata->phy_mode != PHY_INTERFACE_MODE_SGMII) {
>  			/* Abort if the clock is defined but couldn't be
>  			 * retrived. Always abort if the clock is missing on
>  			 * DT system as the driver can't cope with this case.
> 


Yes, this works. You can add my Tested-by.

Thanks,
Laura

^ permalink raw reply

* Re: [PATCH net-next 0/4] net: dsa: lan9303: Fix MDIO issues.
From: Egil Hjelmeland @ 2017-07-28 16:33 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: vivien.didelot, f.fainelli, netdev, kernel
In-Reply-To: <20170728152455.GI2132@lunn.ch>

Den 28. juli 2017 17:24, skrev Andrew Lunn:
> Thanks for breaking the patch up. It was much easier to understand and
> review.
> 
I do see for my self too.

Egil

^ permalink raw reply

* Re: [PATCH net-next 4/4] net: dsa: lan9303: MDIO access phy registers directly
From: Egil Hjelmeland @ 2017-07-28 16:24 UTC (permalink / raw)
  To: Vivien Didelot, andrew, f.fainelli, netdev, linux-kernel, kernel
In-Reply-To: <87fudglget.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>

Den 28. juli 2017 17:39, skrev Vivien Didelot:
> Hi Egil,
> 
> Egil Hjelmeland <privat@egil-hjelmeland.no> writes:
> 
>> +const struct lan9303_phy_ops lan9303_indirect_phy_ops = {
>> +	.phy_read = lan9303_indirect_phy_read,
>> +	.phy_write = lan9303_indirect_phy_write,
>> +};
>> +EXPORT_SYMBOL(lan9303_indirect_phy_ops);
> 
> Isn't EXPORT_SYMBOL_GPL prefered over EXPORT_SYMBOL?
> 
> 
> Thanks,
> 
>          Vivien
> 

I have no opinion. I just used the same variant as the other EXPORTS in
the file.

Egil

^ permalink raw reply

* Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
From: Måns Rullgård @ 2017-07-28 16:17 UTC (permalink / raw)
  To: Marc Gonzalez; +Cc: Florian Fainelli, David S. Miller, netdev, Linux ARM, Mason
In-Reply-To: <823b1540-b528-bbd7-7f99-5dc39a08868a@sigmadesigns.com>

Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes:

> ndo_stop breaks RX in a way that ndo_open is unable to undo.

Please elaborate.  Why can't it be fixed in a less heavy-handed way?

> Work around the issue by resetting the HW in ndo_open.
> This will provide the basis for suspend/resume support.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
>  drivers/net/ethernet/aurora/nb8800.c | 40 +++++++++++++++++-------------------
>  drivers/net/ethernet/aurora/nb8800.h |  1 +
>  2 files changed, 20 insertions(+), 21 deletions(-)

I'm pretty sure this doesn't preserve everything it should.

> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
> index e94159507847..954a28542c3b 100644
> --- a/drivers/net/ethernet/aurora/nb8800.c
> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -39,6 +39,7 @@
>
>  #include "nb8800.h"
>
> +static void nb8800_init(struct net_device *dev);
>  static void nb8800_tx_done(struct net_device *dev);
>  static int nb8800_dma_stop(struct net_device *dev);
>
> @@ -957,6 +958,8 @@ static int nb8800_open(struct net_device *dev)
>  	struct phy_device *phydev;
>  	int err;
>
> +	nb8800_init(dev);
> +
>  	/* clear any pending interrupts */
>  	nb8800_writel(priv, NB8800_RXC_SR, 0xf);
>  	nb8800_writel(priv, NB8800_TXC_SR, 0xf);
> @@ -1350,6 +1353,20 @@ static const struct of_device_id nb8800_dt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, nb8800_dt_ids);
>
> +static void nb8800_init(struct net_device *dev)
> +{
> +	struct nb8800_priv *priv = netdev_priv(dev);
> +	const struct nb8800_ops *ops = priv->ops;
> +
> +	if (ops && ops->reset)
> +		ops->reset(dev);
> +	nb8800_hw_init(dev);
> +	if (ops && ops->init)
> +		ops->init(dev);
> +	nb8800_update_mac_addr(dev);
> +	priv->speed = 0;
> +}
> +
>  static int nb8800_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *match;
> @@ -1389,6 +1406,7 @@ static int nb8800_probe(struct platform_device *pdev)
>
>  	priv = netdev_priv(dev);
>  	priv->base = base;
> +	priv->ops = ops;
>
>  	priv->phy_mode = of_get_phy_mode(pdev->dev.of_node);
>  	if (priv->phy_mode < 0)
> @@ -1407,12 +1425,6 @@ static int nb8800_probe(struct platform_device *pdev)
>
>  	spin_lock_init(&priv->tx_lock);
>
> -	if (ops && ops->reset) {
> -		ret = ops->reset(dev);
> -		if (ret)
> -			goto err_disable_clk;
> -	}
> -
>  	bus = devm_mdiobus_alloc(&pdev->dev);
>  	if (!bus) {
>  		ret = -ENOMEM;
> @@ -1454,16 +1466,6 @@ static int nb8800_probe(struct platform_device *pdev)
>
>  	priv->mii_bus = bus;
>
> -	ret = nb8800_hw_init(dev);
> -	if (ret)
> -		goto err_deregister_fixed_link;
> -
> -	if (ops && ops->init) {
> -		ret = ops->init(dev);
> -		if (ret)
> -			goto err_deregister_fixed_link;
> -	}
> -
>  	dev->netdev_ops = &nb8800_netdev_ops;
>  	dev->ethtool_ops = &nb8800_ethtool_ops;
>  	dev->flags |= IFF_MULTICAST;
> @@ -1476,14 +1478,12 @@ static int nb8800_probe(struct platform_device *pdev)
>  	if (!is_valid_ether_addr(dev->dev_addr))
>  		eth_hw_addr_random(dev);
>
> -	nb8800_update_mac_addr(dev);
> -
>  	netif_carrier_off(dev);
>
>  	ret = register_netdev(dev);
>  	if (ret) {
>  		netdev_err(dev, "failed to register netdev\n");
> -		goto err_free_dma;
> +		goto err_deregister_fixed_link;
>  	}
>
>  	netif_napi_add(dev, &priv->napi, nb8800_poll, NAPI_POLL_WEIGHT);
> @@ -1492,8 +1492,6 @@ static int nb8800_probe(struct platform_device *pdev)
>
>  	return 0;
>
> -err_free_dma:
> -	nb8800_dma_free(dev);
>  err_deregister_fixed_link:
>  	if (of_phy_is_fixed_link(pdev->dev.of_node))
>  		of_phy_deregister_fixed_link(pdev->dev.of_node);
> diff --git a/drivers/net/ethernet/aurora/nb8800.h b/drivers/net/ethernet/aurora/nb8800.h
> index 6ec4a956e1e5..d5f4481a2c7b 100644
> --- a/drivers/net/ethernet/aurora/nb8800.h
> +++ b/drivers/net/ethernet/aurora/nb8800.h
> @@ -305,6 +305,7 @@ struct nb8800_priv {
>  	dma_addr_t			tx_desc_dma;
>
>  	struct clk			*clk;
> +	const struct nb8800_ops		*ops;
>  };
>
>  struct nb8800_ops {
> -- 
> 2.11.0
>

-- 
Måns Rullgård

^ permalink raw reply

* [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
From: Marc Gonzalez @ 2017-07-28 16:13 UTC (permalink / raw)
  To: Florian Fainelli, Mans Rullgard, David S. Miller; +Cc: netdev, Linux ARM, Mason

ndo_stop breaks RX in a way that ndo_open is unable to undo.
Work around the issue by resetting the HW in ndo_open.
This will provide the basis for suspend/resume support.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
 drivers/net/ethernet/aurora/nb8800.c | 40 +++++++++++++++++-------------------
 drivers/net/ethernet/aurora/nb8800.h |  1 +
 2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index e94159507847..954a28542c3b 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -39,6 +39,7 @@
 
 #include "nb8800.h"
 
+static void nb8800_init(struct net_device *dev);
 static void nb8800_tx_done(struct net_device *dev);
 static int nb8800_dma_stop(struct net_device *dev);
 
@@ -957,6 +958,8 @@ static int nb8800_open(struct net_device *dev)
 	struct phy_device *phydev;
 	int err;
 
+	nb8800_init(dev);
+
 	/* clear any pending interrupts */
 	nb8800_writel(priv, NB8800_RXC_SR, 0xf);
 	nb8800_writel(priv, NB8800_TXC_SR, 0xf);
@@ -1350,6 +1353,20 @@ static const struct of_device_id nb8800_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, nb8800_dt_ids);
 
+static void nb8800_init(struct net_device *dev)
+{
+	struct nb8800_priv *priv = netdev_priv(dev);
+	const struct nb8800_ops *ops = priv->ops;
+
+	if (ops && ops->reset)
+		ops->reset(dev);
+	nb8800_hw_init(dev);
+	if (ops && ops->init)
+		ops->init(dev);
+	nb8800_update_mac_addr(dev);
+	priv->speed = 0;
+}
+
 static int nb8800_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *match;
@@ -1389,6 +1406,7 @@ static int nb8800_probe(struct platform_device *pdev)
 
 	priv = netdev_priv(dev);
 	priv->base = base;
+	priv->ops = ops;
 
 	priv->phy_mode = of_get_phy_mode(pdev->dev.of_node);
 	if (priv->phy_mode < 0)
@@ -1407,12 +1425,6 @@ static int nb8800_probe(struct platform_device *pdev)
 
 	spin_lock_init(&priv->tx_lock);
 
-	if (ops && ops->reset) {
-		ret = ops->reset(dev);
-		if (ret)
-			goto err_disable_clk;
-	}
-
 	bus = devm_mdiobus_alloc(&pdev->dev);
 	if (!bus) {
 		ret = -ENOMEM;
@@ -1454,16 +1466,6 @@ static int nb8800_probe(struct platform_device *pdev)
 
 	priv->mii_bus = bus;
 
-	ret = nb8800_hw_init(dev);
-	if (ret)
-		goto err_deregister_fixed_link;
-
-	if (ops && ops->init) {
-		ret = ops->init(dev);
-		if (ret)
-			goto err_deregister_fixed_link;
-	}
-
 	dev->netdev_ops = &nb8800_netdev_ops;
 	dev->ethtool_ops = &nb8800_ethtool_ops;
 	dev->flags |= IFF_MULTICAST;
@@ -1476,14 +1478,12 @@ static int nb8800_probe(struct platform_device *pdev)
 	if (!is_valid_ether_addr(dev->dev_addr))
 		eth_hw_addr_random(dev);
 
-	nb8800_update_mac_addr(dev);
-
 	netif_carrier_off(dev);
 
 	ret = register_netdev(dev);
 	if (ret) {
 		netdev_err(dev, "failed to register netdev\n");
-		goto err_free_dma;
+		goto err_deregister_fixed_link;
 	}
 
 	netif_napi_add(dev, &priv->napi, nb8800_poll, NAPI_POLL_WEIGHT);
@@ -1492,8 +1492,6 @@ static int nb8800_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_free_dma:
-	nb8800_dma_free(dev);
 err_deregister_fixed_link:
 	if (of_phy_is_fixed_link(pdev->dev.of_node))
 		of_phy_deregister_fixed_link(pdev->dev.of_node);
diff --git a/drivers/net/ethernet/aurora/nb8800.h b/drivers/net/ethernet/aurora/nb8800.h
index 6ec4a956e1e5..d5f4481a2c7b 100644
--- a/drivers/net/ethernet/aurora/nb8800.h
+++ b/drivers/net/ethernet/aurora/nb8800.h
@@ -305,6 +305,7 @@ struct nb8800_priv {
 	dma_addr_t			tx_desc_dma;
 
 	struct clk			*clk;
+	const struct nb8800_ops		*ops;
 };
 
 struct nb8800_ops {
-- 
2.11.0

^ permalink raw reply related

* [v4.12 regression] netns: NULL deref in fib_sync_down_dev()
From: Michał Mirosław @ 2017-07-28 16:00 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: Type: text/plain, Size: 280 bytes --]

Dear NetDevs,

Before I go to bisecting, have you seen a following NULL dereference,
yet?  Where should I start looking?  It is triggered by deleting netns
(cut-down script attached - triggers every time).  This was working
correctly under v4.11.x.

Best Regards,
Michał Mirosław

[-- Attachment #2: XXX-BUG-linux-netns --]
[-- Type: text/plain, Size: 4373 bytes --]

[1097925.958758] eth0: renamed from tve0x
[1097926.012266] IPv6: ADDRCONF(NETDEV_UP): tve0: link is not ready
[1097926.035396] IPv6: ADDRCONF(NETDEV_CHANGE): tve0: link becomes ready
[1097926.709371] ip6_tables: (C) 2000-2006 Netfilter Core Team
[1097929.961977] BUG: unable to handle kernel NULL pointer dereference at 0000000000000138
[1097929.961983] IP: fib_sync_down_dev+0x19a/0x230
[1097929.961983] PGD 0
[1097929.961984] P4D 0
[1097929.961986] Oops: 0000 [#1] PREEMPT SMP
[1097929.961987] Modules linked in: ip6table_mangle ip6_tables xt_tcpudp iptable_mangle xt_TPROXY nf_defrag_ipv6 veth nvidia_uvm(PO) cpuid cdc_ether cdc_subset usbnet mii pci_stub vboxpci(O) vboxnetadp(O) vboxnetflt(O) vboxdrv(O) tun xt_REDIRECT nf_nat_redirect cdc_acm ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack bridge stp llc rfcomm snd_emu10k1_synth snd_emux_synth snd_seq_midi_emul snd_seq_virmidi snd_hrtimer snd_seq_midi snd_seq_midi_event snd_seq xfrm_user iptable_filter xfrm4_tunnel tunnel4 ipcomp xfrm_ipcomp esp4 ah4 af_key xfrm_algo cpufreq_powersave cpufreq_userspace cpufreq_conservative nfc fuse bnep binfmt_misc xfs snd_hda_codec_hdmi mxm_wmi btusb btrtl btbcm btintel bluetooth intel_powerclamp snd_hda_codec_re
 altek snd_hda_codec_generic
[1097929.962009]  kvm_intel snd_hda_intel pl2303 snd_emu10k1 usbserial input_leds ecdh_generic snd_hda_codec kvm snd_util_mem snd_hda_core snd_ac97_codec ac97_bus snd_rawmidi snd_seq_device snd_hwdep snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd irqbypass sg pcspkr soundcore emu10k1_gp gameport iTCO_wdt wmi video nvidia_drm(PO) drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm nvidia_modeset(PO) nvidia(PO) nfsd auth_rpcgss oid_registry nfs_acl lockd grace sunrpc loop firewire_sbp2 firewire_core crc_itu_t ecryptfs ip_tables x_tables autofs4 algif_skcipher af_alg sr_mod cdrom raid10 raid0 multipath linear raid456 async_raid6_recov async_memcpy async_pq uas usb_storage raid6_pq async_xor xor async_tx libcrc32c crc32c_intel ghash_clmulni_intel i2c_i801 i2c_core xhci_pci xhci_h
 cd e1000e
[1097929.962034] CPU: 2 PID: 28976 Comm: kworker/u16:2 Tainted: P           O    4.12.2mq+ #204
[1097929.962035] Hardware name: System manufacturer System Product Name/P8Z68-V PRO, BIOS 3603 11/09/2012
[1097929.962037] Workqueue: netns cleanup_net
[1097929.962038] task: ffff8804078ae600 task.stack: ffffc90003b0c000
[1097929.962039] RIP: 0010:fib_sync_down_dev+0x19a/0x230
[1097929.962040] RSP: 0018:ffffc90003b0fc80 EFLAGS: 00010206
[1097929.962041] RAX: 0000000000000011 RBX: ffff880005b70570 RCX: 0000000000000000
[1097929.962042] RDX: 0000000000000000 RSI: 00000000000000ff RDI: 0000000000000000
[1097929.962043] RBP: 0000000000000000 R08: 00000000ffffffff R09: 0000000000000000
[1097929.962044] R10: ffffc90003b0fd10 R11: 0000000000000c79 R12: ffff880005b70500
[1097929.962044] R13: 0000000000000006 R14: ffff880005b70570 R15: ffff880405bc5000
[1097929.962045] FS:  0000000000000000(0000) GS:ffff88041ec80000(0000) knlGS:0000000000000000
[1097929.962046] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[1097929.962047] CR2: 0000000000000138 CR3: 0000000001c09000 CR4: 00000000000406e0
[1097929.962047] Call Trace:
[1097929.962050]  ? fib_disable_ip+0xc/0x40
[1097929.962051]  ? fib_netdev_event+0xe8/0x110
[1097929.962053]  ? notifier_call_chain+0x40/0x60
[1097929.962054]  ? rollback_registered_many+0x2b6/0x400
[1097929.962056]  ? unregister_netdevice_many+0x15/0xa0
[1097929.962057]  ? default_device_exit_batch+0x13c/0x170
[1097929.962059]  ? do_wait_intr_irq+0x80/0x80
[1097929.962061]  ? cleanup_net+0x1d0/0x2c0
[1097929.962062]  ? process_one_work+0x1d2/0x3e0
[1097929.962064]  ? worker_thread+0x42/0x3d0
[1097929.962065]  ? kthread+0xf7/0x130
[1097929.962067]  ? trace_event_raw_event_workqueue_work+0xa0/0xa0
[1097929.962068]  ? kthread_create_on_node+0x60/0x60
[1097929.962069]  ? do_group_exit+0x35/0xa0
[1097929.962070]  ? ret_from_fork+0x22/0x30
[1097929.962071] Code: 39 f0 74 77 49 83 fd 04 74 69 49 83 fd 06 74 60 49 83 fd 02 74 5a 49 8b 97 d8 02 00 00 48 89 5c 24 18 48 c7 44 24 10 00 00 00 00 <8b> 92 38 01 00 00 85 d2 74 04 a8 10 75 08 a8 01 0f 84 12 ff ff
[1097929.962085] RIP: fib_sync_down_dev+0x19a/0x230 RSP: ffffc90003b0fc80
[1097929.962086] CR2: 0000000000000138
[1097929.962087] ---[ end trace 07d937abbfe4921d ]---

[-- Attachment #3: net-test --]
[-- Type: text/plain, Size: 663 bytes --]

#!/bin/bash

trap cleanup EXIT
cleanup() {
	ip netns del test
	ip link del tve0
}

ip netns add test || exit 1
ip link add name tve0 type veth peer name tve0x
ip link set tve0x netns test
ip netns exec test ip link set tve0x name eth0

ip link set tve0 up
ip addr add 10.22.0.1/24 dev tve0
ip route add 10.23.0.0/16 via 10.22.0.2

ip netns exec test ip link set eth0 up
ip netns exec test ip addr add 10.22.0.2/24 dev eth0
ip netns exec test ip route add default via 10.22.0.1 dev eth0
ip netns exec test sysctl net.ipv4.conf.all.forwarding=1

ip netns exec test ip rule add fwmark 1/1 table 1
ip netns exec test ip route add local default dev lo table 1
sleep 1

^ permalink raw reply

* [PATCH net-next v3 6/7] netvsc: fix error unwind on device setup failure
From: Stephen Hemminger @ 2017-07-28 15:59 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev
In-Reply-To: <20170728155947.10062-1-sthemmin@microsoft.com>

If setting receive buffer fails, the error unwind would cause
kernel panic because it was not correctly doing RCU and NAPI
unwind.  RCU'd pointer needs to be reset to NULL, and NAPI needs
to be disabled not deleted.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/netvsc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 4c709b454d34..db95487807fd 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1307,7 +1307,8 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
 	return net_device;
 
 close:
-	netif_napi_del(&net_device->chan_table[0].napi);
+	RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
+	napi_disable(&net_device->chan_table[0].napi);
 
 	/* Now, we can close the channel safely */
 	vmbus_close(device->channel);
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next v3 5/7] netvsc: optimize receive completions
From: Stephen Hemminger @ 2017-07-28 15:59 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev
In-Reply-To: <20170728155947.10062-1-sthemmin@microsoft.com>

Optimize how receive completion ring are managed.
   * Allocate only as many slots as needed for all buffers from host
   * Allocate before setting up sub channel for better error detection
   * Don't need to keep copy of initial receive section message
   * Precompute the watermark for when receive flushing is needed
   * Replace division with conditional test
   * Replace atomic per-device variable with per-channel check.
   * Handle corner case where receive completion send
     fails if ring buffer to host is full.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |  14 +-
 drivers/net/hyperv/netvsc.c       | 267 ++++++++++++++++----------------------
 drivers/net/hyperv/rndis_filter.c |  20 +--
 3 files changed, 126 insertions(+), 175 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 9ca3ed692d73..f2cef5aaed1f 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -186,6 +186,7 @@ struct net_device_context;
 
 struct netvsc_device *netvsc_device_add(struct hv_device *device,
 					const struct netvsc_device_info *info);
+int netvsc_alloc_recv_comp_ring(struct netvsc_device *net_device, u32 q_idx);
 void netvsc_device_remove(struct hv_device *device);
 int netvsc_send(struct net_device_context *ndc,
 		struct hv_netvsc_packet *packet,
@@ -657,13 +658,10 @@ struct recv_comp_data {
 	u32 status;
 };
 
-/* Netvsc Receive Slots Max */
-#define NETVSC_RECVSLOT_MAX (NETVSC_RECEIVE_BUFFER_SIZE / ETH_DATA_LEN + 1)
-
 struct multi_recv_comp {
-	void *buf; /* queued receive completions */
-	u32 first; /* first data entry */
-	u32 next; /* next entry for writing */
+	struct recv_comp_data *slots;
+	u32 first;	/* first data entry */
+	u32 next;	/* next entry for writing */
 };
 
 struct netvsc_stats {
@@ -750,7 +748,7 @@ struct netvsc_device {
 	u32 recv_buf_size;
 	u32 recv_buf_gpadl_handle;
 	u32 recv_section_cnt;
-	struct nvsp_1_receive_buffer_section *recv_section;
+	u32 recv_completion_cnt;
 
 	/* Send buffer allocated by us */
 	void *send_buf;
@@ -778,8 +776,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 */
 
-	atomic_t num_outstanding_recvs;
-
 	atomic_t open_cnt;
 
 	struct netvsc_channel chan_table[VRSS_CHANNEL_MAX];
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index d3c0b19f6d34..4c709b454d34 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -72,9 +72,6 @@ static struct netvsc_device *alloc_net_device(void)
 	if (!net_device)
 		return NULL;
 
-	net_device->chan_table[0].mrc.buf
-		= vzalloc(NETVSC_RECVSLOT_MAX * sizeof(struct recv_comp_data));
-
 	init_waitqueue_head(&net_device->wait_drain);
 	net_device->destroy = false;
 	atomic_set(&net_device->open_cnt, 0);
@@ -92,7 +89,7 @@ static void free_netvsc_device(struct rcu_head *head)
 	int i;
 
 	for (i = 0; i < VRSS_CHANNEL_MAX; i++)
-		vfree(nvdev->chan_table[i].mrc.buf);
+		vfree(nvdev->chan_table[i].mrc.slots);
 
 	kfree(nvdev);
 }
@@ -171,12 +168,6 @@ static void netvsc_destroy_buf(struct hv_device *device)
 		net_device->recv_buf = NULL;
 	}
 
-	if (net_device->recv_section) {
-		net_device->recv_section_cnt = 0;
-		kfree(net_device->recv_section);
-		net_device->recv_section = NULL;
-	}
-
 	/* Deal with the send buffer we may have setup.
 	 * If we got a  send section size, it means we received a
 	 * NVSP_MSG1_TYPE_SEND_SEND_BUF_COMPLETE msg (ie sent
@@ -239,11 +230,26 @@ static void netvsc_destroy_buf(struct hv_device *device)
 	kfree(net_device->send_section_map);
 }
 
+int netvsc_alloc_recv_comp_ring(struct netvsc_device *net_device, u32 q_idx)
+{
+	struct netvsc_channel *nvchan = &net_device->chan_table[q_idx];
+	int node = cpu_to_node(nvchan->channel->target_cpu);
+	size_t size;
+
+	size = net_device->recv_completion_cnt * sizeof(struct recv_comp_data);
+	nvchan->mrc.slots = vzalloc_node(size, node);
+	if (!nvchan->mrc.slots)
+		nvchan->mrc.slots = vzalloc(size);
+
+	return nvchan->mrc.slots ? 0 : -ENOMEM;
+}
+
 static int netvsc_init_buf(struct hv_device *device,
 			   struct netvsc_device *net_device)
 {
 	int ret = 0;
 	struct nvsp_message *init_packet;
+	struct nvsp_1_message_send_receive_buffer_complete *resp;
 	struct net_device *ndev;
 	size_t map_words;
 	int node;
@@ -300,43 +306,41 @@ static int netvsc_init_buf(struct hv_device *device,
 	wait_for_completion(&net_device->channel_init_wait);
 
 	/* Check the response */
-	if (init_packet->msg.v1_msg.
-	    send_recv_buf_complete.status != NVSP_STAT_SUCCESS) {
-		netdev_err(ndev, "Unable to complete receive buffer "
-			   "initialization with NetVsp - status %d\n",
-			   init_packet->msg.v1_msg.
-			   send_recv_buf_complete.status);
+	resp = &init_packet->msg.v1_msg.send_recv_buf_complete;
+	if (resp->status != NVSP_STAT_SUCCESS) {
+		netdev_err(ndev,
+			   "Unable to complete receive buffer initialization with NetVsp - status %d\n",
+			   resp->status);
 		ret = -EINVAL;
 		goto cleanup;
 	}
 
 	/* Parse the response */
+	netdev_dbg(ndev, "Receive sections: %u sub_allocs: size %u count: %u\n",
+		   resp->num_sections, resp->sections[0].sub_alloc_size,
+		   resp->sections[0].num_sub_allocs);
 
-	net_device->recv_section_cnt = init_packet->msg.
-		v1_msg.send_recv_buf_complete.num_sections;
-
-	net_device->recv_section = kmemdup(
-		init_packet->msg.v1_msg.send_recv_buf_complete.sections,
-		net_device->recv_section_cnt *
-		sizeof(struct nvsp_1_receive_buffer_section),
-		GFP_KERNEL);
-	if (net_device->recv_section == NULL) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	net_device->recv_section_cnt = resp->num_sections;
 
 	/*
 	 * For 1st release, there should only be 1 section that represents the
 	 * entire receive buffer
 	 */
 	if (net_device->recv_section_cnt != 1 ||
-	    net_device->recv_section->offset != 0) {
+	    resp->sections[0].offset != 0) {
 		ret = -EINVAL;
 		goto cleanup;
 	}
 
-	/* Now setup the send buffer.
-	 */
+	/* Setup receive completion ring */
+	net_device->recv_completion_cnt
+		= round_up(resp->sections[0].num_sub_allocs + 1,
+			   PAGE_SIZE / sizeof(u64));
+	ret = netvsc_alloc_recv_comp_ring(net_device, 0);
+	if (ret)
+		goto cleanup;
+
+	/* Now setup the send buffer. */
 	net_device->send_buf = vzalloc_node(net_device->send_buf_size, node);
 	if (!net_device->send_buf)
 		net_device->send_buf = vzalloc(net_device->send_buf_size);
@@ -951,130 +955,94 @@ int netvsc_send(struct net_device_context *ndev_ctx,
 	return ret;
 }
 
-static int netvsc_send_recv_completion(struct vmbus_channel *channel,
-				       u64 transaction_id, u32 status)
+/* Send pending recv completions */
+static int send_recv_completions(struct netvsc_channel *nvchan)
 {
-	struct nvsp_message recvcompMessage;
+	struct netvsc_device *nvdev = nvchan->net_device;
+	struct multi_recv_comp *mrc = &nvchan->mrc;
+	struct recv_comp_msg {
+		struct nvsp_message_header hdr;
+		u32 status;
+	}  __packed;
+	struct recv_comp_msg msg = {
+		.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT_COMPLETE,
+	};
 	int ret;
 
-	recvcompMessage.hdr.msg_type =
-				NVSP_MSG1_TYPE_SEND_RNDIS_PKT_COMPLETE;
-
-	recvcompMessage.msg.v1_msg.send_rndis_pkt_complete.status = status;
-
-	/* Send the completion */
-	ret = vmbus_sendpacket(channel, &recvcompMessage,
-			       sizeof(struct nvsp_message_header) + sizeof(u32),
-			       transaction_id, VM_PKT_COMP, 0);
+	while (mrc->first != mrc->next) {
+		const struct recv_comp_data *rcd
+			= mrc->slots + mrc->first;
 
-	return ret;
-}
-
-static inline void count_recv_comp_slot(struct netvsc_device *nvdev, u16 q_idx,
-					u32 *filled, u32 *avail)
-{
-	struct multi_recv_comp *mrc = &nvdev->chan_table[q_idx].mrc;
-	u32 first = mrc->first;
-	u32 next = mrc->next;
+		msg.status = rcd->status;
+		ret = vmbus_sendpacket(nvchan->channel, &msg, sizeof(msg),
+				       rcd->tid, VM_PKT_COMP, 0);
+		if (unlikely(ret))
+			return ret;
 
-	*filled = (first > next) ? NETVSC_RECVSLOT_MAX - first + next :
-		  next - first;
-
-	*avail = NETVSC_RECVSLOT_MAX - *filled - 1;
-}
-
-/* Read the first filled slot, no change to index */
-static inline struct recv_comp_data *read_recv_comp_slot(struct netvsc_device
-							 *nvdev, u16 q_idx)
-{
-	struct multi_recv_comp *mrc = &nvdev->chan_table[q_idx].mrc;
-	u32 filled, avail;
-
-	if (unlikely(!mrc->buf))
-		return NULL;
+		if (++mrc->first == nvdev->recv_completion_cnt)
+			mrc->first = 0;
+	}
 
-	count_recv_comp_slot(nvdev, q_idx, &filled, &avail);
-	if (!filled)
-		return NULL;
+	/* receive completion ring has been emptied */
+	if (unlikely(nvdev->destroy))
+		wake_up(&nvdev->wait_drain);
 
-	return mrc->buf + mrc->first * sizeof(struct recv_comp_data);
+	return 0;
 }
 
-/* Put the first filled slot back to available pool */
-static inline void put_recv_comp_slot(struct netvsc_device *nvdev, u16 q_idx)
+/* Count how many receive completions are outstanding */
+static void recv_comp_slot_avail(const struct netvsc_device *nvdev,
+				 const struct multi_recv_comp *mrc,
+				 u32 *filled, u32 *avail)
 {
-	struct multi_recv_comp *mrc = &nvdev->chan_table[q_idx].mrc;
-	int num_recv;
+	u32 count = nvdev->recv_completion_cnt;
 
-	mrc->first = (mrc->first + 1) % NETVSC_RECVSLOT_MAX;
-
-	num_recv = atomic_dec_return(&nvdev->num_outstanding_recvs);
+	if (mrc->next >= mrc->first)
+		*filled = mrc->next - mrc->first;
+	else
+		*filled = (count - mrc->first) + mrc->next;
 
-	if (nvdev->destroy && num_recv == 0)
-		wake_up(&nvdev->wait_drain);
+	*avail = count - *filled - 1;
 }
 
-/* Check and send pending recv completions */
-static void netvsc_chk_recv_comp(struct netvsc_device *nvdev,
-				 struct vmbus_channel *channel, u16 q_idx)
+/* Add receive complete to ring to send to host. */
+static void enq_receive_complete(struct net_device *ndev,
+				 struct netvsc_device *nvdev, u16 q_idx,
+				 u64 tid, u32 status)
 {
+	struct netvsc_channel *nvchan = &nvdev->chan_table[q_idx];
+	struct multi_recv_comp *mrc = &nvchan->mrc;
 	struct recv_comp_data *rcd;
-	int ret;
-
-	while (true) {
-		rcd = read_recv_comp_slot(nvdev, q_idx);
-		if (!rcd)
-			break;
+	u32 filled, avail;
 
-		ret = netvsc_send_recv_completion(channel, rcd->tid,
-						  rcd->status);
-		if (ret)
-			break;
+	recv_comp_slot_avail(nvdev, mrc, &filled, &avail);
 
-		put_recv_comp_slot(nvdev, q_idx);
+	if (unlikely(filled > NAPI_POLL_WEIGHT)) {
+		send_recv_completions(nvchan);
+		recv_comp_slot_avail(nvdev, mrc, &filled, &avail);
 	}
-}
-
-#define NETVSC_RCD_WATERMARK 80
 
-/* Get next available slot */
-static inline struct recv_comp_data *get_recv_comp_slot(
-	struct netvsc_device *nvdev, struct vmbus_channel *channel, u16 q_idx)
-{
-	struct multi_recv_comp *mrc = &nvdev->chan_table[q_idx].mrc;
-	u32 filled, avail, next;
-	struct recv_comp_data *rcd;
-
-	if (unlikely(!nvdev->recv_section))
-		return NULL;
-
-	if (unlikely(!mrc->buf))
-		return NULL;
-
-	if (atomic_read(&nvdev->num_outstanding_recvs) >
-	    nvdev->recv_section->num_sub_allocs * NETVSC_RCD_WATERMARK / 100)
-		netvsc_chk_recv_comp(nvdev, channel, q_idx);
-
-	count_recv_comp_slot(nvdev, q_idx, &filled, &avail);
-	if (!avail)
-		return NULL;
-
-	next = mrc->next;
-	rcd = mrc->buf + next * sizeof(struct recv_comp_data);
-	mrc->next = (next + 1) % NETVSC_RECVSLOT_MAX;
+	if (unlikely(!avail)) {
+		netdev_err(ndev, "Recv_comp full buf q:%hd, tid:%llx\n",
+			   q_idx, tid);
+		return;
+	}
 
-	atomic_inc(&nvdev->num_outstanding_recvs);
+	rcd = mrc->slots + mrc->next;
+	rcd->tid = tid;
+	rcd->status = status;
 
-	return rcd;
+	if (++mrc->next == nvdev->recv_completion_cnt)
+		mrc->next = 0;
 }
 
 static int netvsc_receive(struct net_device *ndev,
-		   struct netvsc_device *net_device,
-		   struct net_device_context *net_device_ctx,
-		   struct hv_device *device,
-		   struct vmbus_channel *channel,
-		   const struct vmpacket_descriptor *desc,
-		   struct nvsp_message *nvsp)
+			  struct netvsc_device *net_device,
+			  struct net_device_context *net_device_ctx,
+			  struct hv_device *device,
+			  struct vmbus_channel *channel,
+			  const struct vmpacket_descriptor *desc,
+			  struct nvsp_message *nvsp)
 {
 	const struct vmtransfer_page_packet_header *vmxferpage_packet
 		= container_of(desc, const struct vmtransfer_page_packet_header, d);
@@ -1083,7 +1051,6 @@ static int netvsc_receive(struct net_device *ndev,
 	u32 status = NVSP_STAT_SUCCESS;
 	int i;
 	int count = 0;
-	int ret;
 
 	/* Make sure this is a valid nvsp packet */
 	if (unlikely(nvsp->hdr.msg_type != NVSP_MSG1_TYPE_SEND_RNDIS_PKT)) {
@@ -1114,25 +1081,9 @@ static int netvsc_receive(struct net_device *ndev,
 					      channel, data, buflen);
 	}
 
-	if (net_device->chan_table[q_idx].mrc.buf) {
-		struct recv_comp_data *rcd;
+	enq_receive_complete(ndev, net_device, q_idx,
+			     vmxferpage_packet->d.trans_id, status);
 
-		rcd = get_recv_comp_slot(net_device, channel, q_idx);
-		if (rcd) {
-			rcd->tid = vmxferpage_packet->d.trans_id;
-			rcd->status = status;
-		} else {
-			netdev_err(ndev, "Recv_comp full buf q:%hd, tid:%llx\n",
-				   q_idx, vmxferpage_packet->d.trans_id);
-		}
-	} else {
-		ret = netvsc_send_recv_completion(channel,
-						  vmxferpage_packet->d.trans_id,
-						  status);
-		if (ret)
-			netdev_err(ndev, "Recv_comp q:%hd, tid:%llx, err:%d\n",
-				   q_idx, vmxferpage_packet->d.trans_id, ret);
-	}
 	return count;
 }
 
@@ -1231,7 +1182,6 @@ int netvsc_poll(struct napi_struct *napi, int budget)
 	struct netvsc_device *net_device = nvchan->net_device;
 	struct vmbus_channel *channel = nvchan->channel;
 	struct hv_device *device = netvsc_channel_to_device(channel);
-	u16 q_idx = channel->offermsg.offer.sub_channel_index;
 	struct net_device *ndev = hv_get_drvdata(device);
 	int work_done = 0;
 
@@ -1245,17 +1195,18 @@ int netvsc_poll(struct napi_struct *napi, int budget)
 		nvchan->desc = hv_pkt_iter_next(channel, nvchan->desc);
 	}
 
-	/* If receive ring was exhausted
-	 * and not doing busy poll
-	 * then re-enable host interrupts
-	 *  and reschedule if ring is not empty.
+	/* If send of  pending receive completions suceeded
+	 *   and did not exhaust NAPI budget
+	 *   and not doing busy poll
+	 * then reschedule if more data has arrived from host
 	 */
-	if (work_done < budget &&
+	if (send_recv_completions(nvchan) == 0 &&
+	    work_done < budget &&
 	    napi_complete_done(napi, work_done) &&
-	    hv_end_read(&channel->inbound) != 0)
+	    hv_end_read(&channel->inbound)) {
+		hv_begin_read(&channel->inbound);
 		napi_reschedule(napi);
-
-	netvsc_chk_recv_comp(net_device, channel, q_idx);
+	}
 
 	/* Driver may overshoot since multiple packets per descriptor */
 	return min(work_done, budget);
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index d80e9e3f433e..44165fe328a4 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -928,12 +928,12 @@ static bool netvsc_device_idle(const struct netvsc_device *nvdev)
 {
 	int i;
 
-	if (atomic_read(&nvdev->num_outstanding_recvs) > 0)
-		return false;
-
 	for (i = 0; i < nvdev->num_chn; i++) {
 		const struct netvsc_channel *nvchan = &nvdev->chan_table[i];
 
+		if (nvchan->mrc.first != nvchan->mrc.next)
+			return false;
+
 		if (atomic_read(&nvchan->queue_sends) > 0)
 			return false;
 	}
@@ -1031,11 +1031,6 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
 		return;
 
 	nvchan = nvscdev->chan_table + chn_index;
-	nvchan->mrc.buf
-		= vzalloc(NETVSC_RECVSLOT_MAX * sizeof(struct recv_comp_data));
-
-	if (!nvchan->mrc.buf)
-		return;
 
 	/* Because the device uses NAPI, all the interrupt batching and
 	 * control is done via Net softirq, not the channel handling
@@ -1225,6 +1220,15 @@ struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
 	if (num_rss_qs == 0)
 		return net_device;
 
+	for (i = 1; i < net_device->num_chn; i++) {
+		ret = netvsc_alloc_recv_comp_ring(net_device, i);
+		if (ret) {
+			while (--i != 0)
+				vfree(net_device->chan_table[i].mrc.slots);
+			goto out;
+		}
+	}
+
 	refcount_set(&net_device->sc_offered, num_rss_qs);
 	vmbus_set_sc_create_callback(dev->channel, netvsc_sc_open);
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next v3 4/7] netvsc: remove unnecessary indirection of page_buffer
From: Stephen Hemminger @ 2017-07-28 15:59 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev
In-Reply-To: <20170728155947.10062-1-sthemmin@microsoft.com>

The internal API was passing struct hv_page_buffer **
when only simple struct hv_page_buffer * was necessary
for passing an array.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h   |  2 +-
 drivers/net/hyperv/netvsc.c       | 21 ++++++++++-----------
 drivers/net/hyperv/netvsc_drv.c   | 10 ++++------
 drivers/net/hyperv/rndis_filter.c |  4 ++--
 4 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index fb62ea632914..9ca3ed692d73 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -190,7 +190,7 @@ void netvsc_device_remove(struct hv_device *device);
 int netvsc_send(struct net_device_context *ndc,
 		struct hv_netvsc_packet *packet,
 		struct rndis_message *rndis_msg,
-		struct hv_page_buffer **page_buffer,
+		struct hv_page_buffer *page_buffer,
 		struct sk_buff *skb);
 void netvsc_linkstatus_callback(struct hv_device *device_obj,
 				struct rndis_message *resp);
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index f0c15e782ce0..d3c0b19f6d34 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -697,7 +697,7 @@ static u32 netvsc_copy_to_send_buf(struct netvsc_device *net_device,
 				   u32 pend_size,
 				   struct hv_netvsc_packet *packet,
 				   struct rndis_message *rndis_msg,
-				   struct hv_page_buffer **pb,
+				   struct hv_page_buffer *pb,
 				   struct sk_buff *skb)
 {
 	char *start = net_device->send_buf;
@@ -718,9 +718,9 @@ static u32 netvsc_copy_to_send_buf(struct netvsc_device *net_device,
 	}
 
 	for (i = 0; i < page_count; i++) {
-		char *src = phys_to_virt((*pb)[i].pfn << PAGE_SHIFT);
-		u32 offset = (*pb)[i].offset;
-		u32 len = (*pb)[i].len;
+		char *src = phys_to_virt(pb[i].pfn << PAGE_SHIFT);
+		u32 offset = pb[i].offset;
+		u32 len = pb[i].len;
 
 		memcpy(dest, (src + offset), len);
 		msg_size += len;
@@ -739,7 +739,7 @@ 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,
+	struct hv_page_buffer *pb,
 	struct sk_buff *skb)
 {
 	struct nvsp_message nvmsg;
@@ -750,7 +750,6 @@ static inline int netvsc_send_pkt(
 	struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet->q_idx);
 	u64 req_id;
 	int ret;
-	struct hv_page_buffer *pgbuf;
 	u32 ring_avail = hv_ringbuf_avail_percent(&out_channel->outbound);
 
 	nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
@@ -776,11 +775,11 @@ static inline int netvsc_send_pkt(
 		return -ENODEV;
 
 	if (packet->page_buf_cnt) {
-		pgbuf = packet->cp_partial ? (*pb) +
-			packet->rmsg_pgcnt : (*pb);
+		if (packet->cp_partial)
+			pb += packet->rmsg_pgcnt;
+
 		ret = vmbus_sendpacket_pagebuffer_ctl(out_channel,
-						      pgbuf,
-						      packet->page_buf_cnt,
+						      pb, packet->page_buf_cnt,
 						      &nvmsg,
 						      sizeof(struct nvsp_message),
 						      req_id,
@@ -830,7 +829,7 @@ static inline void move_pkt_msd(struct hv_netvsc_packet **msd_send,
 int netvsc_send(struct net_device_context *ndev_ctx,
 		struct hv_netvsc_packet *packet,
 		struct rndis_message *rndis_msg,
-		struct hv_page_buffer **pb,
+		struct hv_page_buffer *pb,
 		struct sk_buff *skb)
 {
 	struct netvsc_device *net_device
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index a04f2efbbc25..8ff4cbf582cc 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -282,9 +282,8 @@ static u32 fill_pg_buf(struct page *page, u32 offset, u32 len,
 
 static u32 init_page_array(void *hdr, u32 len, struct sk_buff *skb,
 			   struct hv_netvsc_packet *packet,
-			   struct hv_page_buffer **page_buf)
+			   struct hv_page_buffer *pb)
 {
-	struct hv_page_buffer *pb = *page_buf;
 	u32 slots_used = 0;
 	char *data = skb->data;
 	int frags = skb_shinfo(skb)->nr_frags;
@@ -359,8 +358,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	u32 rndis_msg_size;
 	struct rndis_per_packet_info *ppi;
 	u32 hash;
-	struct hv_page_buffer page_buf[MAX_PAGE_BUFFER_COUNT];
-	struct hv_page_buffer *pb = page_buf;
+	struct hv_page_buffer pb[MAX_PAGE_BUFFER_COUNT];
 
 	/* We can only transmit MAX_PAGE_BUFFER_COUNT number
 	 * of pages in a single packet. If skb is scattered around
@@ -503,12 +501,12 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
 	rndis_msg->msg_len += rndis_msg_size;
 	packet->total_data_buflen = rndis_msg->msg_len;
 	packet->page_buf_cnt = init_page_array(rndis_msg, rndis_msg_size,
-					       skb, packet, &pb);
+					       skb, packet, pb);
 
 	/* timestamp packet in software */
 	skb_tx_timestamp(skb);
 
-	ret = netvsc_send(net_device_ctx, packet, rndis_msg, &pb, skb);
+	ret = netvsc_send(net_device_ctx, packet, rndis_msg, pb, skb);
 	if (likely(ret == 0))
 		return NETDEV_TX_OK;
 
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index bf21ea92c743..d80e9e3f433e 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -214,11 +214,11 @@ static void dump_rndis_message(struct hv_device *hv_dev,
 static int rndis_filter_send_request(struct rndis_device *dev,
 				  struct rndis_request *req)
 {
-	int ret;
 	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);
+	int ret;
 
 	/* Setup the packet to send it */
 	packet = &req->pkt;
@@ -245,7 +245,7 @@ static int rndis_filter_send_request(struct rndis_device *dev,
 	}
 
 	rcu_read_lock_bh();
-	ret = netvsc_send(net_device_ctx, packet, NULL, &pb, NULL);
+	ret = netvsc_send(net_device_ctx, packet, NULL, pb, NULL);
 	rcu_read_unlock_bh();
 
 	return ret;
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next v3 3/7] netvsc: don't print pointer value in error message
From: Stephen Hemminger @ 2017-07-28 15:59 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev
In-Reply-To: <20170728155947.10062-1-sthemmin@microsoft.com>

Using %p to print pointer to packet meta-data doesn't give any
good info, and exposes kernel memory offsets.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/netvsc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 94c00acac58a..f0c15e782ce0 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -805,8 +805,10 @@ static inline int netvsc_send_pkt(
 			ret = -ENOSPC;
 		}
 	} else {
-		netdev_err(ndev, "Unable to send packet %p ret %d\n",
-			   packet, ret);
+		netdev_err(ndev,
+			   "Unable to send packet pages %u len %u, ret %d\n",
+			   packet->page_buf_cnt, packet->total_data_buflen,
+			   ret);
 	}
 
 	return ret;
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next v3 2/7] netvsc: fix warnings reported by lockdep
From: Stephen Hemminger @ 2017-07-28 15:59 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev
In-Reply-To: <20170728155947.10062-1-sthemmin@microsoft.com>

This includes a bunch of fixups for issues reported by
lockdep.
   * ethtool routines can assume RTNL
   * send is done with RCU lock (and BH disable)
   * avoid refetching internal device struct (netvsc)
     instead pass it as a parameter.

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

^ permalink raw reply related

* [PATCH net-next v3 0/7] netvsc: minor fixes and optimization
From: Stephen Hemminger @ 2017-07-28 15:59 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev

This is a subset of earlier submission with a few more fixes
found during testing. The are two small optimizations, one is to
better manage the receive completion ring, and the other is removing
one unneeded level of indirection.

Will submit the improved VF support and buffer sizing in a later
patch so they get more review.

Stephen Hemminger (7):
  netvsc: fix return value for set_channels
  netvsc: fix warnings reported by lockdep
  netvsc: don't print pointer value in error message
  netvsc: remove unnecessary indirection of page_buffer
  netvsc: optimize receive completions
  netvsc: fix error unwind on device setup failure
  netvsc: signal host if receive ring is emptied

 drivers/net/hyperv/hyperv_net.h   |  19 +--
 drivers/net/hyperv/netvsc.c       | 302 ++++++++++++++++----------------------
 drivers/net/hyperv/netvsc_drv.c   |  27 ++--
 drivers/net/hyperv/rndis_filter.c | 108 +++++++-------
 4 files changed, 208 insertions(+), 248 deletions(-)

-- 
2.11.0

^ permalink raw reply

* [PATCH net-next v3 7/7] netvsc: signal host if receive ring is emptied
From: Stephen Hemminger @ 2017-07-28 15:59 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev
In-Reply-To: <20170728155947.10062-1-sthemmin@microsoft.com>

Latency improvement related to NAPI conversion.
If all packets are processed from receive ring then need
to signal host.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/netvsc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index db95487807fd..c64934c64dca 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1195,10 +1195,15 @@ int netvsc_poll(struct napi_struct *napi, int budget)
 		nvchan->desc = hv_pkt_iter_next(channel, nvchan->desc);
 	}
 
-	/* If send of  pending receive completions suceeded
-	 *   and did not exhaust NAPI budget
+	/* if ring is empty, signal host */
+	if (!nvchan->desc)
+		hv_pkt_iter_close(channel);
+
+	/* If send of pending receive completions suceeded
+	 *   and did not exhaust NAPI budget this time
 	 *   and not doing busy poll
-	 * then reschedule if more data has arrived from host
+	 * then re-enable host interrupts
+	 *     and reschedule if ring is not empty.
 	 */
 	if (send_recv_completions(nvchan) == 0 &&
 	    work_done < budget &&
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next v3 1/7] netvsc: fix return value for set_channels
From: Stephen Hemminger @ 2017-07-28 15:59 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin; +Cc: devel, netdev
In-Reply-To: <20170728155947.10062-1-sthemmin@microsoft.com>

The error and normal case got swapped.

Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
 drivers/net/hyperv/netvsc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 262486ce8e2a..f1eaf675d2e9 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -758,8 +758,8 @@ static int netvsc_set_channels(struct net_device *net,
 	if (!IS_ERR(nvdev)) {
 		netif_set_real_num_tx_queues(net, nvdev->num_chn);
 		netif_set_real_num_rx_queues(net, nvdev->num_chn);
-		ret = PTR_ERR(nvdev);
 	} else {
+		ret = PTR_ERR(nvdev);
 		device_info.num_chn = orig;
 		rndis_filter_device_add(dev, &device_info);
 	}
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH net-next v11 3/4] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jiri Pirko @ 2017-07-28 15:45 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: davem, netdev, xiyou.wangcong, dsahern, eric.dumazet, mrv,
	simon.horman, alex.aring
In-Reply-To: <fc91ce4d-b7d1-7e95-7e9a-9265aed68424@mojatatu.com>

Fri, Jul 28, 2017 at 05:08:10PM CEST, jhs@mojatatu.com wrote:
>On 17-07-28 10:52 AM, Jamal Hadi Salim wrote:
>> On 17-07-28 10:12 AM, Jiri Pirko wrote:
>> > Fri, Jul 28, 2017 at 03:41:44PM CEST, jhs@mojatatu.com wrote:
>> [..]
>> > 
>> > Looks like a big mess to be honest. Mixing up u32* u32 void*. I don't
>> > understand ****. Would be probably good to first apply my review comment
>> > on the function itselt, then to add the checks :)
>> > 
>> 
>> I havent even compiled/test that Jiri.
>> Just ignore the void * and assume it is a u32 *.
>> 
>
>This compiled - but dont have much time right now to test.
>===
>static int validate_nla_bitfield32(const struct nlattr *nla,
>                                   u32 *valid_flags_allowed)
>{
>        const struct nla_bitfield32 *bf = nla_data(nla);
>        u32 *valid_flags_mask = valid_flags_allowed;

good one :D


>
>        if (!valid_flags_allowed)
>                return -EINVAL;
>
>        /*disallow invalid selector */
>        if ((bf->selector & *valid_flags_allowed) > *valid_flags_allowed)

I don't get the ">"....
Just (bf->selector & ~*valid_flags_allowed) should be enought, no?


>                return -EINVAL;
>
>        /*disallow invalid bit values */
>        if (bf->value & ~*valid_flags_mask)
>                return -EINVAL;
>
>        /*disallow valid bit values that are not selected*/
>        if (bf->value & ~bf->selector)
>                return -EINVAL;
>
>        return 0;
>}
>========
>
>cheers,
>jamal

^ permalink raw reply

* RE: [PATCH net] tcp: avoid bogus gcc-7 array-bounds warning
From: David Laight @ 2017-07-28 15:39 UTC (permalink / raw)
  To: 'Arnd Bergmann', David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: Eric Dumazet, Neal Cardwell, Soheil Hassas Yeganeh, Yuchung Cheng,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170728144153.2024234-1-arnd@arndb.de>

From: Arnd Bergmann
> Sent: 28 July 2017 15:42
...
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2202,9 +2202,10 @@ static bool tcp_small_queue_check(struct sock *sk, const struct sk_buff *skb,
>  static void tcp_chrono_set(struct tcp_sock *tp, const enum tcp_chrono new)
>  {
>  	const u32 now = tcp_jiffies32;
> +	enum tcp_chrono old = tp->chrono_type;
> 
> -	if (tp->chrono_type > TCP_CHRONO_UNSPEC)
> -		tp->chrono_stat[tp->chrono_type - 1] += now - tp->chrono_start;
> +	if (old > TCP_CHRONO_UNSPEC)
> +		tp->chrono_stat[old - 1] += now - tp->chrono_start;
>  	tp->chrono_start = now;
>  	tp->chrono_type = new;

What a horrid combination of enum and integers.
Also have u32 chrono_stat[3]; - should probably be [__TCP_CHRONO_MAX - 1]
(or - CHRONO_FIRST which is defined to be 1).

Checking if (old != 0) would make the code more readable.

	David

^ permalink raw reply

* Re: [PATCH net-next 4/4] net: dsa: lan9303: MDIO access phy registers directly
From: Vivien Didelot @ 2017-07-28 15:39 UTC (permalink / raw)
  To: Egil Hjelmeland, andrew, f.fainelli, netdev, linux-kernel, kernel
  Cc: Egil Hjelmeland
In-Reply-To: <20170728151157.28983-5-privat@egil-hjelmeland.no>

Hi Egil,

Egil Hjelmeland <privat@egil-hjelmeland.no> writes:

> +const struct lan9303_phy_ops lan9303_indirect_phy_ops = {
> +	.phy_read = lan9303_indirect_phy_read,
> +	.phy_write = lan9303_indirect_phy_write,
> +};
> +EXPORT_SYMBOL(lan9303_indirect_phy_ops);

Isn't EXPORT_SYMBOL_GPL prefered over EXPORT_SYMBOL?


Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH net-next 3/4] net: dsa: lan9303: Renamed indirect phy access functions
From: Vivien Didelot @ 2017-07-28 15:37 UTC (permalink / raw)
  To: Egil Hjelmeland, andrew, f.fainelli, netdev, linux-kernel, kernel
  Cc: Egil Hjelmeland
In-Reply-To: <20170728151157.28983-4-privat@egil-hjelmeland.no>

Egil Hjelmeland <privat@egil-hjelmeland.no> writes:

> Preparing for the following fix of MDIO phy access:
>
> Renamed functions that access PHY 1 and 2 indirectly through PMI
> registers.
>
>  lan9303_port_phy_reg_wait_for_completion() to
>  lan9303_indirect_phy_wait_for_completion()
>
>  lan9303_port_phy_reg_read() to
>  lan9303_indirect_phy_read()
>
>  lan9303_port_phy_reg_write() to
>  lan9303_indirect_phy_write()
>
> Also changed "val" parameter of lan9303_indirect_phy_write() to u16,
> for clarity.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

^ permalink raw reply

* Re: [PATCH net-next 2/4] net: dsa: lan9303: Multiply by 4 to get MDIO register
From: Vivien Didelot @ 2017-07-28 15:37 UTC (permalink / raw)
  To: Egil Hjelmeland, andrew, f.fainelli, netdev, linux-kernel, kernel
  Cc: Egil Hjelmeland
In-Reply-To: <20170728151157.28983-3-privat@egil-hjelmeland.no>

Egil Hjelmeland <privat@egil-hjelmeland.no> writes:

> lan9303_mdio_write()/_read() must multiply register number by 4 to get
> offset.
>
> Added some commments to the register definitions.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

^ permalink raw reply

* Re: [PATCH net-next 1/4] net: dsa: lan9303: Fix lan9303_detect_phy_setup() for MDIO
From: Vivien Didelot @ 2017-07-28 15:36 UTC (permalink / raw)
  To: Egil Hjelmeland, andrew, f.fainelli, netdev, linux-kernel, kernel
  Cc: Egil Hjelmeland
In-Reply-To: <20170728151157.28983-2-privat@egil-hjelmeland.no>

Egil Hjelmeland <privat@egil-hjelmeland.no> writes:

> Handle that MDIO read with no response return 0xffff.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

^ permalink raw reply

* Re: [PATCH net-next 0/4] net: dsa: lan9303: Fix MDIO issues.
From: Andrew Lunn @ 2017-07-28 15:24 UTC (permalink / raw)
  To: Egil Hjelmeland; +Cc: vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
In-Reply-To: <20170728151157.28983-1-privat@egil-hjelmeland.no>

On Fri, Jul 28, 2017 at 05:11:53PM +0200, Egil Hjelmeland wrote:
> This series fix the MDIO interface for the lan9303 DSA driver.
> Bugs found after testing on actual HW.
> 
> This series is extracted from the first patch of my first large
> series.

Hi Egil

Thanks for breaking the patch up. It was much easier to understand and
review.

	Andrew

^ permalink raw reply

* Re: [PATCH net-next 4/4] net: dsa: lan9303: MDIO access phy registers directly
From: Andrew Lunn @ 2017-07-28 15:24 UTC (permalink / raw)
  To: Egil Hjelmeland; +Cc: vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
In-Reply-To: <20170728151157.28983-5-privat@egil-hjelmeland.no>

On Fri, Jul 28, 2017 at 05:11:57PM +0200, Egil Hjelmeland wrote:
> Indirect access (PMI) to phy register only work in I2C mode. In
> MDIO mode phy registers must be accessed directly. Introduced
> struct lan9303_phy_ops to handle the two modes.
> 
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox