Netdev List
 help / color / mirror / Atom feed
* Re: wl1251 & mac address & calibration data
From: Pali Rohár @ 2016-12-16 10:26 UTC (permalink / raw)
  To: Arend Van Spriel
  Cc: Kalle Valo, Sebastian Reichel, Pavel Machek, Michal Kazior,
	Ivaylo Dimitrov, Aaro Koskinen, Tony Lindgren, linux-wireless,
	Network Development, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Luis R. Rodriguez
In-Reply-To: <76365770-f5ba-9565-3fca-710518f64d81-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

[-- Attachment #1: Type: Text/Plain, Size: 3594 bytes --]

On Thursday 15 December 2016 21:12:47 Arend Van Spriel wrote:
> On 15-12-2016 16:33, Pali Rohár wrote:
> > On Thu Dec 15 09:18:44 2016 Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> >> (Adding Luis because he has been working on request_firmware()
> >> lately)
> >> 
> >> Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> >>>>> So no, there is no argument against... request_firmware() in
> >>>>> fallback mode with userspace helper is by design blocking and
> >>>>> waiting for userspace. But waiting for some change in DTS in
> >>>>> kernel is just nonsense.
> >>>> 
> >>>> I would just mark the wlan device with status = "disabled" and
> >>>> enable it in the overlay together with adding the NVS & MAC
> >>>> info.
> >>> 
> >>> So if you think that this solution make sense, we can wait what
> >>> net wireless maintainers say about it...
> >>> 
> >>> For me it looks like that solution can be:
> >>> 
> >>> extending request_firmware() to use only userspace helper
> >> 
> >> I haven't followed the discussion very closely but this is my
> >> preference what drivers should do:
> >> 
> >> 1) First the driver should do try to get the calibration data and
> >> mac
> >> 
> >>        address from the device tree.
> > 
> > Ok, but there is no (dynamic, device specific) data in DTS for
> > N900. So 1) is noop.
> 
> Uhm. What do you mean? You can propose a patch to the DT bindings [1]
> to get it in there and create your N900 DTB or am I missing
> something here. Are there hardware restrictions that do not allow
> you to boot with your own DTB.

What is [1]?

N900's bootloader does not support DTB and it does not pass any DTB. It 
boots kernel in ATAGs mode. What we are doing is appending DTB compiled 
from kernel sources to end of zImage.

But that appended DTB cannot contains device specific nodes (e.g. 
calibration data for device) as zImage is there for any N900 device, not 
just only one.

> >> 2) If they are not in DT the driver should retrieve the
> >> calibration data
> >> 
> >>        with request_firmware(). BUT with an option for user space
> >>        to implement that with a helper script so that the data
> >>        can be created dynamically, which I believe openwrt does
> >>        with ath10k calibration data right now.
> > 
> > Currently there is flag for request_firmware() that it should
> > fallback to user helper if direct VFS access not find needed
> > firmware.
> > 
> > But this flag is not suitable as /lib/firmware already provides
> > default (not device specific) calibration data.
> > 
> > So I would suggest to add another flag/function which will primary
> > use user helper.
> 
> I recall Luis saying that user-mode helper (fallback) should be
> discouraged, because there is no assurance that there is a user-mode
> helper so you might just be pissing in the wind. The idea was to have
> a dedicated API call that explicitly does the request towards
> user-space.
> 
> By the way, are we talking here about wl1251 device or driver as you
> also mentioned wl12xx? I did not read the entire thread.

Yes, we are talking about wl1251 device, which is in Nokia N900 and 
wl1251.ko and wl1251_spi.ko drivers.

I mentioned wl12xx as it already uses similar approach with mac address 
via request_firmware(). And as those drivers are very similar plus from 
one manufactor and in same kernel folder I mentioned similar API for 
consistency...

-- 
Pali Rohár
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PULL] virtio, vhost: new device, fixes, speedups
From: Paolo Bonzini @ 2016-12-16 10:12 UTC (permalink / raw)
  To: Linus Torvalds, Michael S. Tsirkin
  Cc: Network Development, arend.vanspriel, Linux Kernel Mailing List,
	KVM list, virtualization
In-Reply-To: <CA+55aFxwG=jj6=2fKjNkHCOnbKr-mNvNucbA=CSdPvb4kzW3FA@mail.gmail.com>



On 16/12/2016 03:20, Linus Torvalds wrote:
> On Thu, Dec 15, 2016 at 3:05 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> 
> Pulled, but I wonder...
> 
>>  Documentation/translations/zh_CN/sparse.txt        |   7 +-
>>  arch/arm/plat-samsung/include/plat/gpio-cfg.h      |   2 +-
>>  drivers/crypto/virtio/virtio_crypto_common.h       | 128 +++++
> [...]
> 
> what are you generating these diffstats with? Because they are pretty bogus..
> 
> The end result is correct:
> 
>>  86 files changed, 2106 insertions(+), 280 deletions(-)
> 
> but the file order in the diffstat is completely random, which makes
> it very hard to compare with what I get. It also makes it hard to see
> what you changed, because it's not alphabetical like it should be
> (strictly speaking the git pathname ordering isnt' really
> alphabetical, since the '/' sorts as the NUL character, but close
> enough).
> 
> I can't see the logic to the re-ordering of the lines, so I'm
> intrigued how you even generated it.

Looks like a diff.orderFile that places .h and .txt first, then .c, then
Makefile.  I've seen others propose it.

Paolo

^ permalink raw reply

* Re: [PATCH net-next 1/3] net:dsa:mv88e6xxx: use hashtable to store multicast entries
From: Andrew Lunn @ 2016-12-16 10:08 UTC (permalink / raw)
  To: Volodymyr Bendiuga
  Cc: Vivien Didelot, Florian Fainelli, Volodymyr Bendiuga, netdev,
	Volodymyr Bendiuga, John Crispin
In-Reply-To: <CABHmqqCOYMPA-Py7Uu4gsQ=s8CrQd-tT4-Db+j8eJH9=2s4L2g@mail.gmail.com>

On Fri, Dec 16, 2016 at 10:24:28AM +0100, Volodymyr Bendiuga wrote:
> Hi all,
> 
> Does this mean we all agree on implementing caching
> mechanism in net/dsa layer? If yes, then I can start
> working on it immediately.

Volodymyr

Could you provide us with your test script, which adds multicast MAC
addresses to ports. I would like to do some profiling with it.

	  Thanks
		Andrew

^ permalink raw reply

* Re: [PATCH net-next 1/3] net:dsa:mv88e6xxx: use hashtable to store multicast entries
From: John Crispin @ 2016-12-16  9:31 UTC (permalink / raw)
  To: Volodymyr Bendiuga, Vivien Didelot
  Cc: Florian Fainelli, Andrew Lunn, Volodymyr Bendiuga, netdev,
	Volodymyr Bendiuga
In-Reply-To: <CABHmqqCOYMPA-Py7Uu4gsQ=s8CrQd-tT4-Db+j8eJH9=2s4L2g@mail.gmail.com>



On 16/12/2016 10:24, Volodymyr Bendiuga wrote:
> Hi all,
> 
> Does this mean we all agree on implementing caching
> mechanism in net/dsa layer? If yes, then I can start
> working on it immediately.
> 
> Regards,
> Volodymyr

i think that the brcm approach is better as at least on QCA you can also
read auto learned values from the table which would get broken by a
caching mechanism. filtering inside the dump function or the code
calling the dump function seams a lot simpler and lightweight to me.

	John

> 
> On Thu, Dec 15, 2016 at 6:50 PM, Vivien Didelot
> <vivien.didelot@savoirfairelinux.com
> <mailto:vivien.didelot@savoirfairelinux.com>> wrote:
> 
>     Florian Fainelli <f.fainelli@gmail.com
>     <mailto:f.fainelli@gmail.com>> writes:
> 
>     >> In all cases *if caching is really needed*, I think it won't hurt to do
>     >> it in DSA core even if a switch support FDB dump operations on a
>     >> per-port basis, as Andrew mentioned.
>     >
>     > Agreed, and there does not appear to be any need to new dsa_switch_ops
>     > operations to be introduced?
> 
>     Nope.
> 
> 

^ permalink raw reply

* RE: [PATCH v5 3/4] secure_seq: use SipHash in place of MD5
From: David Laight @ 2016-12-16  9:59 UTC (permalink / raw)
  To: 'Jason A. Donenfeld', Netdev,
	kernel-hardening@lists.openwall.com, LKML,
	linux-crypto@vger.kernel.org, Ted Tso, Hannes Frederic Sowa,
	Linus Torvalds, Eric Biggers, Tom Herbert, George Spelvin,
	Vegard Nossum, ak@linux.intel.com, davem@davemloft.net,
	luto@amacapital.net
In-Reply-To: <20161215203003.31989-4-Jason@zx2c4.com>

From: Jason A. Donenfeld
> Sent: 15 December 2016 20:30
> This gives a clear speed and security improvement. Siphash is both
> faster and is more solid crypto than the aging MD5.
> 
> Rather than manually filling MD5 buffers, for IPv6, we simply create
> a layout by a simple anonymous struct, for which gcc generates
> rather efficient code. For IPv4, we pass the values directly to the
> short input convenience functions.
...
> diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
> index 88a8e429fc3e..c80583bf3213 100644
...
> +	const struct {
> +		struct in6_addr saddr;
> +		struct in6_addr daddr;
> +		__be16 sport;
> +		__be16 dport;
> +		u32 padding;
> +	} __aligned(SIPHASH_ALIGNMENT) combined = {
> +		.saddr = *(struct in6_addr *)saddr,
> +		.daddr = *(struct in6_addr *)daddr,
> +		.sport = sport,
> +		.dport = dport
> +	};

I think you should explicitly initialise the 'padding'.
It can do no harm and makes it obvious that it is necessary.

You are still putting over-aligned data on stack.
You only need to align it to the alignment of u64 (not the size of u64).
If an on-stack item has a stronger alignment requirement than the stack
the gcc has to generate two stack frames for the function.

If you assign to each field (instead of using initialisers) then you
can get the alignment by making the first member an anonymous union
of in6_addr and u64.

Oh - and wait a bit longer between revisions.

	David

^ permalink raw reply

* Re: [PATCH net 2/3] cpsw/netcp: davinci_cpdma: sanitize inter-module API
From: Ivan Khoronzhuk @ 2016-12-16  9:59 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mugunthan V N, Grygorii Strashko, David S. Miller,
	Uwe Kleine-König, linux-omap, netdev, linux-kernel
In-Reply-To: <20161216092017.2560717-2-arnd@arndb.de>

On Fri, Dec 16, 2016 at 10:19:58AM +0100, Arnd Bergmann wrote:
> The davinci_cpdma module is a helper library that is used by the
> actual device drivers and does nothing by itself, so all its API
> functions need to be exported.
> 
> Four new functions were added recently without an export, so now
> we get a build error:
> 
> ERROR: "cpdma_chan_set_weight" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
> ERROR: "cpdma_chan_get_rate" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
> ERROR: "cpdma_chan_get_min_rate" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
> ERROR: "cpdma_chan_set_rate" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
/\
||
A patch correcting this has been applied.
(397c5ad153f0ea62023accb67b3d2b07e5039948)

> 
> This exports those symbols. After taking a closer look, I found two
> more global functions in this file that are not exported and not used
> anywhere, so they can obviously be removed. There is also one function
> that is only used internally in this module, and should be 'static'.
> 
> Fixes: 8f32b90981dc ("net: ethernet: ti: davinci_cpdma: add set rate for a channel")
> Fixes: 0fc6432cc78d ("net: ethernet: ti: davinci_cpdma: add weight function for channels")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ethernet/ti/davinci_cpdma.c | 59 ++++++++++-----------------------
>  drivers/net/ethernet/ti/davinci_cpdma.h |  4 ---
>  2 files changed, 17 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
> index 36518fc5c7cc..b9d40f0cdf6c 100644
> --- a/drivers/net/ethernet/ti/davinci_cpdma.c
> +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
> @@ -628,6 +628,23 @@ int cpdma_ctlr_destroy(struct cpdma_ctlr *ctlr)
>  }
>  EXPORT_SYMBOL_GPL(cpdma_ctlr_destroy);
>  
> +static int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&chan->lock, flags);
> +	if (chan->state != CPDMA_STATE_ACTIVE) {
> +		spin_unlock_irqrestore(&chan->lock, flags);
> +		return -EINVAL;
> +	}
> +
> +	dma_reg_write(chan->ctlr, enable ? chan->int_set : chan->int_clear,
> +		      chan->mask);
> +	spin_unlock_irqrestore(&chan->lock, flags);
> +
> +	return 0;
> +}
> +
>  int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable)
>  {
>  	unsigned long flags;
> @@ -1274,46 +1291,4 @@ int cpdma_chan_stop(struct cpdma_chan *chan)
>  }
>  EXPORT_SYMBOL_GPL(cpdma_chan_stop);
>  
> -int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable)
> -{
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&chan->lock, flags);
> -	if (chan->state != CPDMA_STATE_ACTIVE) {
> -		spin_unlock_irqrestore(&chan->lock, flags);
> -		return -EINVAL;
> -	}
> -
> -	dma_reg_write(chan->ctlr, enable ? chan->int_set : chan->int_clear,
> -		      chan->mask);
> -	spin_unlock_irqrestore(&chan->lock, flags);
> -
> -	return 0;
> -}
> -
> -int cpdma_control_get(struct cpdma_ctlr *ctlr, int control)
> -{
> -	unsigned long flags;
> -	int ret;
> -
> -	spin_lock_irqsave(&ctlr->lock, flags);
> -	ret = _cpdma_control_get(ctlr, control);
> -	spin_unlock_irqrestore(&ctlr->lock, flags);
> -
> -	return ret;
> -}
> -
> -int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value)
> -{
> -	unsigned long flags;
> -	int ret;
> -
> -	spin_lock_irqsave(&ctlr->lock, flags);
> -	ret = _cpdma_control_set(ctlr, control, value);
> -	spin_unlock_irqrestore(&ctlr->lock, flags);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(cpdma_control_set);
> -
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.h b/drivers/net/ethernet/ti/davinci_cpdma.h
> index 4a167db2abab..36d0a09a3d44 100644
> --- a/drivers/net/ethernet/ti/davinci_cpdma.h
> +++ b/drivers/net/ethernet/ti/davinci_cpdma.h
> @@ -87,7 +87,6 @@ int cpdma_chan_process(struct cpdma_chan *chan, int quota);
>  
>  int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable);
>  void cpdma_ctlr_eoi(struct cpdma_ctlr *ctlr, u32 value);
> -int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable);
>  u32 cpdma_ctrl_rxchs_state(struct cpdma_ctlr *ctlr);
>  u32 cpdma_ctrl_txchs_state(struct cpdma_ctlr *ctlr);
>  bool cpdma_check_free_tx_desc(struct cpdma_chan *chan);
> @@ -111,7 +110,4 @@ enum cpdma_control {
>  	CPDMA_RX_BUFFER_OFFSET,		/* read-write */
>  };
>  
> -int cpdma_control_get(struct cpdma_ctlr *ctlr, int control);
> -int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value);
> -
>  #endif
> -- 
> 2.9.0
> 

^ permalink raw reply

* [PATCH net 3/3] cpsw/netcp: work around reverse cpts dependency
From: Arnd Bergmann @ 2016-12-16  9:19 UTC (permalink / raw)
  To: David S. Miller
  Cc: Arnd Bergmann, Grygorii Strashko, Thomas Gleixner, Nicolas Pitre,
	Uwe Kleine-König, WingMan Kwok, netdev, linux-kernel
In-Reply-To: <20161216092017.2560717-1-arnd@arndb.de>

The dependency is reversed: cpsw and netcp call into cpts,
but cpts depends on the other two in Kconfig. This can lead
to cpts being a loadable module and its callers built-in:

drivers/net/ethernet/ti/cpsw.o: In function `cpsw_remove':
cpsw.c:(.text.cpsw_remove+0xd0): undefined reference to `cpts_release'
drivers/net/ethernet/ti/cpsw.o: In function `cpsw_rx_handler':
cpsw.c:(.text.cpsw_rx_handler+0x2dc): undefined reference to `cpts_rx_timestamp'
drivers/net/ethernet/ti/cpsw.o: In function `cpsw_tx_handler':
cpsw.c:(.text.cpsw_tx_handler+0x7c): undefined reference to `cpts_tx_timestamp'
drivers/net/ethernet/ti/cpsw.o: In function `cpsw_ndo_stop':

As a workaround, I'm introducing another Kconfig symbol to
control the compilation of cpts, while making the actual
module controlled by a silent symbol that is =y when necessary.

Fixes: 6246168b4a38 ("net: ethernet: ti: netcp: add support of cpts")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/ti/Kconfig | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 366e29ff8605..c114efcd1575 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -73,8 +73,8 @@ config TI_CPSW
 	  To compile this driver as a module, choose M here: the module
 	  will be called cpsw.
 
-config TI_CPTS
-	tristate "TI Common Platform Time Sync (CPTS) Support"
+config TI_CPTS_ENABLE
+	bool "TI Common Platform Time Sync (CPTS) Support"
 	depends on TI_CPSW || TI_KEYSTONE_NETCP
 	depends on POSIX_TIMERS
 	select PTP_1588_CLOCK
@@ -84,6 +84,12 @@ config TI_CPTS
 	  The unit can time stamp PTP UDP/IPv4 and Layer 2 packets, and the
 	  driver offers a PTP Hardware Clock.
 
+config TI_CPTS
+	tristate
+	depends on TI_CPTS_ENABLE
+	default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y
+	default m
+
 config TI_KEYSTONE_NETCP
 	tristate "TI Keystone NETCP Core Support"
 	select TI_CPSW_ALE
-- 
2.9.0

^ permalink raw reply related

* [PATCH net 1/3] cpsw/netcp: cpts depends on posix_timers
From: Arnd Bergmann @ 2016-12-16  9:19 UTC (permalink / raw)
  To: David S. Miller
  Cc: Arnd Bergmann, Grygorii Strashko, Thomas Gleixner, John Stultz,
	Uwe Kleine-König, WingMan Kwok, Nicolas Pitre, netdev,
	linux-kernel

With posix timers having become optional, we get a build error with
the cpts time sync option of the CPSW driver:

drivers/net/ethernet/ti/cpts.c: In function 'cpts_find_ts':
drivers/net/ethernet/ti/cpts.c:291:23: error: implicit declaration of function 'ptp_classify_raw';did you mean 'ptp_classifier_init'? [-Werror=implicit-function-declaration]

It really makes no sense to build this driver if we can't use PTP,
so it's better to go back to 'select PTP_1588_CLOCK' but instead
add a dependency on POSIX_TIMERS.

Fixes: baa73d9e478f ("posix-timers: Make them configurable")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/ti/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 296c8efd0038..366e29ff8605 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -76,7 +76,8 @@ config TI_CPSW
 config TI_CPTS
 	tristate "TI Common Platform Time Sync (CPTS) Support"
 	depends on TI_CPSW || TI_KEYSTONE_NETCP
-	imply PTP_1588_CLOCK
+	depends on POSIX_TIMERS
+	select PTP_1588_CLOCK
 	---help---
 	  This driver supports the Common Platform Time Sync unit of
 	  the CPSW Ethernet Switch and Keystone 2 1g/10g Switch Subsystem.
-- 
2.9.0

^ permalink raw reply related

* [PATCH net 2/3] cpsw/netcp: davinci_cpdma: sanitize inter-module API
From: Arnd Bergmann @ 2016-12-16  9:19 UTC (permalink / raw)
  To: Mugunthan V N
  Cc: Arnd Bergmann, Grygorii Strashko, David S. Miller,
	Ivan Khoronzhuk, Uwe Kleine-König, linux-omap, netdev,
	linux-kernel
In-Reply-To: <20161216092017.2560717-1-arnd@arndb.de>

The davinci_cpdma module is a helper library that is used by the
actual device drivers and does nothing by itself, so all its API
functions need to be exported.

Four new functions were added recently without an export, so now
we get a build error:

ERROR: "cpdma_chan_set_weight" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
ERROR: "cpdma_chan_get_rate" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
ERROR: "cpdma_chan_get_min_rate" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
ERROR: "cpdma_chan_set_rate" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!

This exports those symbols. After taking a closer look, I found two
more global functions in this file that are not exported and not used
anywhere, so they can obviously be removed. There is also one function
that is only used internally in this module, and should be 'static'.

Fixes: 8f32b90981dc ("net: ethernet: ti: davinci_cpdma: add set rate for a channel")
Fixes: 0fc6432cc78d ("net: ethernet: ti: davinci_cpdma: add weight function for channels")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/ti/davinci_cpdma.c | 59 ++++++++++-----------------------
 drivers/net/ethernet/ti/davinci_cpdma.h |  4 ---
 2 files changed, 17 insertions(+), 46 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index 36518fc5c7cc..b9d40f0cdf6c 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -628,6 +628,23 @@ int cpdma_ctlr_destroy(struct cpdma_ctlr *ctlr)
 }
 EXPORT_SYMBOL_GPL(cpdma_ctlr_destroy);
 
+static int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&chan->lock, flags);
+	if (chan->state != CPDMA_STATE_ACTIVE) {
+		spin_unlock_irqrestore(&chan->lock, flags);
+		return -EINVAL;
+	}
+
+	dma_reg_write(chan->ctlr, enable ? chan->int_set : chan->int_clear,
+		      chan->mask);
+	spin_unlock_irqrestore(&chan->lock, flags);
+
+	return 0;
+}
+
 int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable)
 {
 	unsigned long flags;
@@ -1274,46 +1291,4 @@ int cpdma_chan_stop(struct cpdma_chan *chan)
 }
 EXPORT_SYMBOL_GPL(cpdma_chan_stop);
 
-int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&chan->lock, flags);
-	if (chan->state != CPDMA_STATE_ACTIVE) {
-		spin_unlock_irqrestore(&chan->lock, flags);
-		return -EINVAL;
-	}
-
-	dma_reg_write(chan->ctlr, enable ? chan->int_set : chan->int_clear,
-		      chan->mask);
-	spin_unlock_irqrestore(&chan->lock, flags);
-
-	return 0;
-}
-
-int cpdma_control_get(struct cpdma_ctlr *ctlr, int control)
-{
-	unsigned long flags;
-	int ret;
-
-	spin_lock_irqsave(&ctlr->lock, flags);
-	ret = _cpdma_control_get(ctlr, control);
-	spin_unlock_irqrestore(&ctlr->lock, flags);
-
-	return ret;
-}
-
-int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value)
-{
-	unsigned long flags;
-	int ret;
-
-	spin_lock_irqsave(&ctlr->lock, flags);
-	ret = _cpdma_control_set(ctlr, control, value);
-	spin_unlock_irqrestore(&ctlr->lock, flags);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(cpdma_control_set);
-
 MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.h b/drivers/net/ethernet/ti/davinci_cpdma.h
index 4a167db2abab..36d0a09a3d44 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.h
+++ b/drivers/net/ethernet/ti/davinci_cpdma.h
@@ -87,7 +87,6 @@ int cpdma_chan_process(struct cpdma_chan *chan, int quota);
 
 int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable);
 void cpdma_ctlr_eoi(struct cpdma_ctlr *ctlr, u32 value);
-int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable);
 u32 cpdma_ctrl_rxchs_state(struct cpdma_ctlr *ctlr);
 u32 cpdma_ctrl_txchs_state(struct cpdma_ctlr *ctlr);
 bool cpdma_check_free_tx_desc(struct cpdma_chan *chan);
@@ -111,7 +110,4 @@ enum cpdma_control {
 	CPDMA_RX_BUFFER_OFFSET,		/* read-write */
 };
 
-int cpdma_control_get(struct cpdma_ctlr *ctlr, int control);
-int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value);
-
 #endif
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH net-next 1/2] net: batman-adv: Treat NET_XMIT_CN as transmit successfully
From: Sven Eckelmann @ 2016-12-16  9:19 UTC (permalink / raw)
  To: b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
  Cc: mareklindner-rVWd3aGhH2z5bpWLKbzFeg,
	netdev-u79uwXL29TY76Z2rM5mHXA, a, fgao-KlmEoCYek3zQT0dZR+AlfA,
	gfree.wind-Re5JQEeQqe8AvxtiuMwx3w, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1479740432-3721-1-git-send-email-fgao-KlmEoCYek3zQT0dZR+AlfA@public.gmane.org>

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

On Montag, 21. November 2016 23:00:32 CET fgao-KlmEoCYek3zQT0dZR+AlfA@public.gmane.org wrote:
> From: Gao Feng <gfree.wind-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> The tc could return NET_XMIT_CN as one congestion notification, but
> it does not mean the packet is lost. Other modules like ipvlan,
> macvlan, and others treat NET_XMIT_CN as success too.
> 
> So batman-adv should add the NET_XMIT_CN check.
> 
> Signed-off-by: Gao Feng <gfree.wind-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  net/batman-adv/distributed-arp-table.c |  2 +-
>  net/batman-adv/fragmentation.c         |  2 +-
>  net/batman-adv/routing.c               | 10 +++++-----
>  net/batman-adv/soft-interface.c        |  2 +-
>  net/batman-adv/tp_meter.c              |  2 +-
>  5 files changed, 9 insertions(+), 9 deletions(-)

David marked your patches as "derefered" after "under review" and did not
apply them directly. Also Florian Westphal didn't continue the discussion
about the direction you should choose.

The patches were therefore queued up in the in batman-adv
671630d6aad0..eab7617142d2. They will be submitted later(tm) in a pull
request to David.

Thanks,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH mac80211-next] rfkill: hide unused goto label
From: Johannes Berg @ 2016-12-16  9:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: David S. Miller, João Paulo Rechi Vita,
	Michał Kępień, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <20161216084425.1692828-1-arnd@arndb.de>

On Fri, 2016-12-16 at 09:44 +0100, Arnd Bergmann wrote:
> A cleanup introduced a harmless warning in some configurations:
> 
> net/rfkill/core.c: In function 'rfkill_init':
> net/rfkill/core.c:1350:1: warning: label 'error_input' defined but
> not used [-Wunused-label]
> 
> This adds another #ifdef around the label to match that around the
> caller.

Applied, thanks.

johannes

^ permalink raw reply

* Re: [PATCH net] qed: fix old-style function definition
From: Johannes Thumshirn @ 2016-12-16  8:55 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Yuval Mintz, Ariel Elior, everest-linux-l2, Arun Easi,
	Hannes Reinecke, David S. Miller, netdev, linux-kernel
In-Reply-To: <20161216084808.1815139-1-arnd@arndb.de>

On Fri, Dec 16, 2016 at 09:47:41AM +0100, Arnd Bergmann wrote:
> The newly added file causes a harmless warning, with "make W=1":
> 
> drivers/net/ethernet/qlogic/qed/qed_iscsi.c: In function 'qed_get_iscsi_ops':
> drivers/net/ethernet/qlogic/qed/qed_iscsi.c:1268:29: warning: old-style function definition [-Wold-style-definition]
> 
> This makes it a proper prototype.
> 
> Fixes: fc831825f99e ("qed: Add support for hardware offloaded iSCSI.")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply

* [PATCH net] qed: fix old-style function definition
From: Arnd Bergmann @ 2016-12-16  8:47 UTC (permalink / raw)
  To: Yuval Mintz, Ariel Elior, everest-linux-l2
  Cc: Arnd Bergmann, Arun Easi, Hannes Reinecke, David S. Miller,
	Johannes Thumshirn, netdev, linux-kernel

The newly added file causes a harmless warning, with "make W=1":

drivers/net/ethernet/qlogic/qed/qed_iscsi.c: In function 'qed_get_iscsi_ops':
drivers/net/ethernet/qlogic/qed/qed_iscsi.c:1268:29: warning: old-style function definition [-Wold-style-definition]

This makes it a proper prototype.

Fixes: fc831825f99e ("qed: Add support for hardware offloaded iSCSI.")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I only saw this after it was already merged for v4.10

 drivers/net/ethernet/qlogic/qed/qed_iscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
index 00efb1c4c57e..17a70122df05 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
@@ -1265,7 +1265,7 @@ static const struct qed_iscsi_ops qed_iscsi_ops_pass = {
 	.get_stats = &qed_iscsi_stats,
 };
 
-const struct qed_iscsi_ops *qed_get_iscsi_ops()
+const struct qed_iscsi_ops *qed_get_iscsi_ops(void)
 {
 	return &qed_iscsi_ops_pass;
 }
-- 
2.9.0

^ permalink raw reply related

* [PATCH mac80211-next] rfkill: hide unused goto label
From: Arnd Bergmann @ 2016-12-16  8:44 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Arnd Bergmann, David S. Miller, João Paulo Rechi Vita,
	Michał Kępień, linux-wireless, netdev,
	linux-kernel

A cleanup introduced a harmless warning in some configurations:

net/rfkill/core.c: In function 'rfkill_init':
net/rfkill/core.c:1350:1: warning: label 'error_input' defined but not used [-Wunused-label]

This adds another #ifdef around the label to match that around the
caller.

Fixes: 6124c53edeea ("rfkill: Cleanup error handling in rfkill_init()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/rfkill/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index f6b01f3616e5..b7adaee69756 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -1347,8 +1347,10 @@ static int __init rfkill_init(void)
 
 	return 0;
 
+#ifdef CONFIG_RFKILL_INPUT
 error_input:
 	rfkill_any_led_trigger_unregister();
+#endif
 error_led_trigger:
 	misc_deregister(&rfkill_miscdev);
 error_misc:
-- 
2.9.0

^ permalink raw reply related

* [PATCH] net: ipv6: check route protocol when deleting routes
From: Mantas Mikulėnas @ 2016-12-16  8:30 UTC (permalink / raw)
  To: netdev, linux-kernel, davem; +Cc: Mantas Mikulėnas

The protocol field is checked when deleting IPv4 routes, but ignored for
IPv6, which causes problems with routing daemons accidentally deleting
externally set routes (observed by multiple bird6 users).

This can be verified using `ip -6 route del <prefix> proto something`.

Signed-off-by: Mantas Mikulėnas <grawity@gmail.com>
---
 net/ipv6/route.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 947ed1ded026..ef6de8f9e5a2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2163,6 +2163,8 @@ static int ip6_route_del(struct fib6_config *cfg)
 				continue;
 			if (cfg->fc_metric && cfg->fc_metric != rt->rt6i_metric)
 				continue;
+			if (cfg->fc_protocol && cfg->fc_protocol != rt->rt6i_protocol)
+				continue;
 			dst_hold(&rt->dst);
 			read_unlock_bh(&table->tb6_lock);
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH] liquidio CN23XX: make timeout HZ independent
From: Nicholas Mc Guire @ 2016-12-16  8:10 UTC (permalink / raw)
  To: Derek Chickles
  Cc: Satanand Burla, Felix Manlunas, Raghu Vatsavayi, netdev,
	linux-kernel, Nicholas Mc Guire

schedule_timeout_* takes a timeout in jiffies but the code currently is
passing in a constant which makes this timeout HZ dependent, so pass it
through msecs_to_jiffies() to fix this up.

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

Problem found by coccinelle spatch

The current delay can vary by a factor 10 depending on the HZ
setting chose, which does not seem reasonable here.

The below patch sets the timeout to 10ms - it is though not clear
if this is the intent or if it should be longer/shorter as it is not
clear what HZ setting was assumed during design and used for testing.

This needs an ack by someone who knows the device and can confirm that
10ms is reasonable to wait for completion of queuing.

Patch was compile tested with: x86_64_defconfig + CONFIG_LIQUIDIO_VF=m
Note that this driver has a large number of sparse warnings !

Patch is against 4.9.0 (localversion-next is -next-20161216)

 drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c | 2 ++-
 1 file changed, 2 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
index 73696b42..1a0fcbe 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_mailbox.c
@@ -171,7 +171,8 @@ int octeon_mbox_write(struct octeon_device *oct,
 			count = 0;
 			while (readq(mbox->mbox_write_reg) !=
 			       OCTEON_PFVFACK) {
-				schedule_timeout_uninterruptible(10);
+				schedule_timeout_uninterruptible(
+							msecs_to_jiffies(10));
 				if (count++ == LIO_MBOX_WRITE_WAIT_CNT) {
 					ret = OCTEON_MBOX_STATUS_FAILED;
 					break;
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH v5 1/4] siphash: add cryptographically secure PRF
From: Jean-Philippe Aumasson @ 2016-12-16  8:08 UTC (permalink / raw)
  To: George Spelvin, ak, davem, David.Laight, ebiggers3, hannes, Jason,
	kernel-hardening, linux-crypto, linux-kernel, luto, netdev, tom,
	torvalds, tytso, vegard.nossum
  Cc: djb
In-Reply-To: <20161216034618.28276.qmail@ns.sciencehorizons.net>

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

Here's a tentative HalfSipHash:
https://github.com/veorq/SipHash/blob/halfsiphash/halfsiphash.c

Haven't computed the cycle count nor measured its speed.

On Fri, Dec 16, 2016 at 4:46 AM George Spelvin <linux@sciencehorizons.net>
wrote:

> Jean-Philippe Aumasson wrote:
> > If a halved version of SipHash can bring significant performance boost
> > (with 32b words instead of 64b words) with an acceptable security level
> > (64-bit enough?) then we may design such a version.
>
> It would be fairly significant, a 2x speed benefit on a lot of 32-bit
> machines.
>
> First is the fact that a 64-bit SipHash round on a generic 32-bit machine
> requires not twice as many instructions, but more than three.
>
> Consider the core SipHash quarter-round operation:
>         a += b;
>         b = rotate_left(b, k)
>         b ^= a
>
> The add and xor are equivalent between 32- and 64-bit rounds; twice the
> instructions do twice the work.  (There's a dependency via the carry
> bit between the two halves of the add, but it ends up not being on the
> critical path even in a superscalar implementation.)
>
> The problem is the rotates.  Although some particularly nice code is
> possible on 32-bit ARM due to its support for shift-and-xor operations,
> on a generic 32-bit CPU the rotate grows to 6 instructions with a 2-cycle
> dependency chain (more in practice because barrel shifters are large and
> even quad-issue CPUs can't do 4 shifts per cycle):
>
>         temp_lo = b_lo >> (32-k)
>         temp_hi = b_hi >> (32-k)
>         b_lo <<= k
>         b_hi <<= k
>         b_lo ^= temp_hi
>         b_hi ^= temp_lo
>
> The resultant instruction counts and (assuming wide issue)
> latencies are:
>
>         64-bit SipHash  "Half" SipHash
>         Inst.   Latency Inst.   Latency
>          10      3        3      2      Quarter round
>          40      6       12      4      Full round
>          80     12       24      8      Two rounds
>          82     13       26      9      Mix in one word
>          82     13       52     18      Mix in 64 bits
>         166     26       61     18      Four round finalization + final XOR
>         248     39      113     36      Hash 64 bits
>         330     52      165     54      Hash 128 bits
>         412     65      217     72      Hash 192 bits
>
> While the ideal latencies are actually better for the 64-bit algorithm,
> that requires an unrealistic 6+-wide superscalar implementation that's
> more than twice as wide as the 64-bit code requires (which is already
> optimized for quad-issue).  For a 1- or 2-wide processor, the instruction
> counts dominate, and not only does the 64-bit algorithm take 60% more
> time to mix in the same number of bytes, but the finalization rounds
> bring the ratio to 2:1 for small inputs.
>
> (And I haven't included the possible savings if the input size is an odd
> number of 32-bit words, such as networking applications which include
> the source/dest port numbers.)
>
>
> Notes on particular processors:
> - x86 can do a 64-bit rotate in 3 instructions and 2 cycles using
>   the SHLD/SHRD instructions instead:
>         movl    %b_hi, %temp
>         shldl   $k, %b_lo, %b_hi
>         shldl   $k, %temp, %b_lo
>   ... but as I mentioned the problem is registers.  SipHash needs 8 32-bit
>   words plus at least one temporary, and 32-bit x86 has only 7 available.
>   (And compilers can rarely manage to keep more than 6 of them busy.)
> - 64-bit SipHash is particularly efficient on 32-bit ARM due to its
>   support for shift-and-op instructions.  The 64-bit shift and following
>   xor can be done in 4 instructions.  So the only benefit is from the
>   reduced finalization.
> - Double-width adds cost a little more on CPUs like MIPS and RISC-V without
>   condition codes.
> - Certain particularly crappy uClinux processors with slow shifts
>   (68000, anyone?) really suffer from extra shifts.
>
> One *weakly* requested feature: It might simplify some programming
> interfaces if we could use the same key for multiple hash tables with a
> 1-word "tweak" (e.g. pointer to the hash table, so it could be assumed
> non-zero if that helped) to make distinct functions.  That would let us
> more safely use a global key for multiple small hash tables without the
> need to add code to generate and store key material for each place that
> an unkeyed hash is replaced.
>

[-- Attachment #2: Type: text/html, Size: 6883 bytes --]

^ permalink raw reply

* Re: wl1251 & mac address & calibration data
From: Daniel Wagner @ 2016-12-16  7:25 UTC (permalink / raw)
  To: Luis R. Rodriguez, Arend Van Spriel, Tom Gundersen, Johannes Berg,
	Ming Lei, Mimi Zohar, Bjorn Andersson, Rafał Miłecki
  Cc: Pali Rohár, Kalle Valo, Sebastian Reichel, Pavel Machek,
	Michal Kazior, Ivaylo Dimitrov, Aaro Koskinen, Tony Lindgren,
	linux-wireless, Network Development,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Woodhouse, Takashi Iwai, Josh Boyer, Dmitry Torokhov
In-Reply-To: <CAB=NE6VHF+U7DzapENvLMKtN4Dgi9J0qMoeUJ8o4vSwc2-rRFQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 12/16/2016 03:03 AM, Luis R. Rodriguez wrote:
> For the new API a solution for "fallback mechanisms" should be clean
> though and I am looking to stay as far as possible from the existing
> mess. A solution to help both the old API and new API is possible for
> the "fallback mechanism" though -- but for that I can only refer you
> at this point to some of Daniel Wagner and Tom Gunderson's firmwared
> deamon prospect. It should help pave the way for a clean solution and
> help address other stupid issues.

The firmwared project is hosted here

https://github.com/teg/firmwared

As Luis pointed out, firmwared relies on FW_LOADER_USER_HELPER_FALLBACK, 
which is not enabled by default. I don't see any reason why firmwared 
should not also support loading calibration data. If we find a sound way 
to do this.

As you can see from the commit history it is a pretty young project and 
more ore less reanimation of the old udev firmware loader feature.  We 
are getting int into shape, adding integration tests etc.

The main motivation for this project is the get movement back in stuck 
discussion on the firmware loader API. Luis was very busy writing up all 
the details on the current situation and purely from the amount of 
documentation need to describe the API you can tell something is awry.

Thanks,
Daniel

^ permalink raw reply

* [PATCH RFC] liquidio: make timeout HZ independent
From: Nicholas Mc Guire @ 2016-12-16  6:57 UTC (permalink / raw)
  To: Derek Chickles
  Cc: Satanand Burla, Felix Manlunas, Raghu Vatsavayi, netdev,
	linux-kernel, Nicholas Mc Guire

schedule_timeout_* takes a timeout in jiffies but the code currently is
passing in a constant which makes this timeout HZ dependent, so pass it
through msecs_to_jiffies() to fix this up.

Fixes: commit b0d66369edcd ("liquidio VF error handling")
Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

Problem found by coccinelle spatch

The current wait time can vary by a factor 10 depending on the HZ
setting chose, which does not seem reasonable here.

The below patch sets the timeout to 100ms - it is though not clear
if this is the intent or if it should be longer/shorter as it is not
clear what HZ setting was assumed during design and used for testing.

This needs an ack by someone who knows the device and can confirm that
100ms is reasonable to wait for completion of in-flight requests.

Patch was compile tested with: x86_64_defconfig + CONFIG_LIQUIDIO_VF=m

Patch is against 4.9.0 (localversion-next is -next-20161216)

 drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index 70d96c1..eca469e 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -189,7 +189,7 @@ static void pcierror_quiesce_device(struct octeon_device *oct)
 	 */
 
 	/* To allow for in-flight requests */
-	schedule_timeout_uninterruptible(100);
+	schedule_timeout_uninterruptible(msecs_to_jiffies(100));
 
 	if (wait_for_pending_requests(oct))
 		dev_err(&oct->pci_dev->dev, "There were pending requests\n");
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH] net: wan: Use dma_pool_zalloc
From: Souptick Joarder @ 2016-12-16  6:03 UTC (permalink / raw)
  To: Joe Perches; +Cc: Krzysztof Hałasa, netdev, Rameshwar Sahu
In-Reply-To: <1481820487.29291.66.camel@perches.com>

On Thu, Dec 15, 2016 at 10:18 PM, Joe Perches <joe@perches.com> wrote:
> On Thu, 2016-12-15 at 10:41 +0530, Souptick Joarder wrote:
>> On Mon, Dec 12, 2016 at 10:12 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
>> > On Fri, Dec 9, 2016 at 6:33 PM, Krzysztof Hałasa <khalasa@piap.pl> wrote:
>> > > Souptick Joarder <jrdr.linux@gmail.com> writes:
>> > >
>> > > > We should use dma_pool_zalloc instead of dma_pool_alloc/memset
> []
>> > > > diff --git a/drivers/net/wan/ixp4xx_hss.c b/drivers/net/wan/ixp4xx_hss.c
> []
>> > > > @@ -976,10 +976,9 @@ static int init_hdlc_queues(struct port *port)
>> > > >                       return -ENOMEM;
>> > > >       }
>> > > >
>> > > > -     if (!(port->desc_tab = dma_pool_alloc(dma_pool, GFP_KERNEL,
>> > > > -                                           &port->desc_tab_phys)))
>> > > > +     if (!(port->desc_tab = dma_pool_zalloc(dma_pool, GFP_KERNEL,
>> > > > +                                            &port->desc_tab_phys)))
>> > > >               return -ENOMEM;
>> > > > -     memset(port->desc_tab, 0, POOL_ALLOC_SIZE);
>> > > >       memset(port->rx_buff_tab, 0, sizeof(port->rx_buff_tab)); /* tables */
>> > > >       memset(port->tx_buff_tab, 0, sizeof(port->tx_buff_tab));
>> > >
>> > > This look fine, feel free to send it to the netdev mailing list for
>> > > inclusion.
>> >
>> > Including netdev mailing list based as requested.
>> > > Acked-by: Krzysztof Halasa <khalasa@piap.pl>
> []
>> Any comment on this patch ?
>
> Shouldn't the one in drivers/net/ethernet/xscale/ixp4xx_eth.c
> also be changed?

Yes, you are right.   Do you want me to include it in same patch?

Regards
Souptick

^ permalink raw reply

* Re: [PATCH] net: wan: Use dma_pool_zalloc
From: Joe Perches @ 2016-12-16  6:10 UTC (permalink / raw)
  To: Souptick Joarder; +Cc: Krzysztof Hałasa, netdev, Rameshwar Sahu
In-Reply-To: <CAFqt6zaPWuc+gZTMqk-gh8weZvAjvVwm0kyDT5ub=iah=y9skA@mail.gmail.com>

On Fri, 2016-12-16 at 11:33 +0530, Souptick Joarder wrote:
> On Thu, Dec 15, 2016 at 10:18 PM, Joe Perches <joe@perches.com> wrote:
> > On Thu, 2016-12-15 at 10:41 +0530, Souptick Joarder wrote:
> > > On Mon, Dec 12, 2016 at 10:12 AM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
> > > > On Fri, Dec 9, 2016 at 6:33 PM, Krzysztof Hałasa <khalasa@piap.pl> wrote:
> > > > > Souptick Joarder <jrdr.linux@gmail.com> writes:
> > > > > 
> > > > > > We should use dma_pool_zalloc instead of dma_pool_alloc/memset
> > 
> > []
> > > > > > diff --git a/drivers/net/wan/ixp4xx_hss.c b/drivers/net/wan/ixp4xx_hss.c
> > 
> > []
> > > > > > @@ -976,10 +976,9 @@ static int init_hdlc_queues(struct port *port)
> > > > > >                       return -ENOMEM;
> > > > > >       }
> > > > > > 
> > > > > > -     if (!(port->desc_tab = dma_pool_alloc(dma_pool, GFP_KERNEL,
> > > > > > -                                           &port->desc_tab_phys)))
> > > > > > +     if (!(port->desc_tab = dma_pool_zalloc(dma_pool, GFP_KERNEL,
> > > > > > +                                            &port->desc_tab_phys)))
> > > > > >               return -ENOMEM;
> > > > > > -     memset(port->desc_tab, 0, POOL_ALLOC_SIZE);
> > > > > >       memset(port->rx_buff_tab, 0, sizeof(port->rx_buff_tab)); /* tables */
> > > > > >       memset(port->tx_buff_tab, 0, sizeof(port->tx_buff_tab));
> > > > > 
> > > > > This look fine, feel free to send it to the netdev mailing list for
> > > > > inclusion.
> > > > 
> > > > Including netdev mailing list based as requested.
> > > > > Acked-by: Krzysztof Halasa <khalasa@piap.pl>
> > 
> > []
> > > Any comment on this patch ?
> > 
> > Shouldn't the one in drivers/net/ethernet/xscale/ixp4xx_eth.c
> > also be changed?
> 
> Yes, you are right.   Do you want me to include it in same patch?

Your choice.  I would use a single patch.

^ permalink raw reply

* Re: [RFC v2 03/10] IB/hfi-vnic: Virtual Network Interface Controller (VNIC) netdev
From: Jason Gunthorpe @ 2016-12-16  4:24 UTC (permalink / raw)
  To: Vishwanathapura, Niranjana
  Cc: dledford, linux-rdma, netdev, dennis.dalessandro, ira.weiny,
	Sadanand Warrier, Sudeep Dutt, Tanya K Jajodia,
	Andrzej Kacprowski
In-Reply-To: <20161216025947.GC90951@knc-06.sc.intel.com>

On Thu, Dec 15, 2016 at 06:59:47PM -0800, Vishwanathapura, Niranjana wrote:
> We have made the hfi_vnic driver dependent on CONFIG_X86_64.

Er, don't do that either?

> >>+struct __hfi_vesw_info {
> >>+	u16  fabric_id;
> >>+	u16  vesw_id;
> >>+
> >>+	u8   rsvd0[6];
> >>+	u16  def_port_mask;
> >>+
> >>+	u8   rsvd1[2];
> >>+	u16  pkey;
> >>+
> >>+	u8   rsvd2[4];
> >>+	u32  u_mcast_dlid;
> >>+	u32  u_ucast_dlid[HFI_VESW_MAX_NUM_DEF_PORT];
> >>+
> >>+	u8   rsvd3[44];
> >>+	u16  eth_mtu[HFI_VNIC_MAX_NUM_PCP];
> >>+	u16  eth_mtu_non_vlan;
> >>+	u8   rsvd4[2];
> >>+} __packed;
> >
> >This goes on the network too? Also looks like it has endian problems.
> >
> >Ditto for all the __packed structures.
> >
> 
> This is in CPU format. There is a separate big endian version of
> this

Why are CPU handled structures packed and full of reserved fields?
Don't pack them if they are not pushed out to the network..

There were lots of __packed structures, any that go on the network
need be/le annoations.

Jason

^ permalink raw reply

* Re: [RFC v2 00/10] HFI Virtual Network Interface Controller (VNIC)
From: Jason Gunthorpe @ 2016-12-16  4:17 UTC (permalink / raw)
  To: ira.weiny
  Cc: Doug Ledford, Leon Romanovsky, Jeff Kirsher, David S. Miller,
	Vishwanathapura, Niranjana, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <20161216012404.GD3785-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>

On Thu, Dec 15, 2016 at 08:24:05PM -0500, ira.weiny wrote:
> > The main share is the 'skb send' part, we've talked about hoisting
> > that out of ipoib in the past anyhow. A generic verb along those lines
> > would probably allow the sdma optimization for hfi for both this new
> > ulp and ipoib without creating such an ugly HFI1 specific interface.
> 
> I'm not sure what you mean about "skb send" being used by ipoib.  Right now
> IPoIB already supplies a "generic skb send" for _Verbs_ in ipoib_send.

Sending a skb is very hard, the boring standard verbs implementation
is slow. Mellanox extended that with some simple offloads, but it is not
enough to get the full performance out of the hardware.

The suggestion is to add a 'skb qp', for lack of a better name, that is
perfectly optimized to delegate working with skbs to the driver. The
driver will then optmize with all possible offloads and bypass the
verbs qp API between netdev and driver.

Look at what is already in patch #2:

+struct hfi_vnic_ops {
+	int (*open)(struct hfi_vnic_port *vport, hfi_vnic_evt_cb_fn cb);
+	void (*close)(struct hfi_vnic_port *vport);
+	int (*put_skb)(struct hfi_vnic_port *vport,
+	           u8 q_idx, struct sk_buff *skb);
+       struct sk_buff *(*get_skb)(struct hfi_vnic_port *vport, u8 q_idx);
+	      u16 (*get_read_avail)(struct hfi_vnic_port *vport, u8 q_idx);
+	bool (*get_write_avail)(struct hfi_vnic_port *vport, u8 q_idx);
+	u8 (*select_queue)(struct hfi_vnic_port *vport, u8 vl, u8 entropy);
+	void (*config_notify)(struct hfi_vnic_port *vport,
+	     			           u8 evt, bool enable);

That is almost what I'm talking about.

A 'vport' is a 'skb qp' that has been made overly specific.

So, clean it up to get rid of all the hfi specific stuff, stop calling
it a port. Get feedback from Mellanox. Refactor ipoib to use it to
show that it works sanely with both drivers.

> I don't know what other devices would do to implement ipoib_send?  To me, it
> seems like the abstraction for IPoIB is at the proper layer now.

An example is multi queue tx with QPN sharing. We don't have anything
like that in verbs. At some point it just doesn't make sense to twist
verbs into knots to do this stuff - a skb qp is much cleaner and
powerful.

> For OPA, te hfi driver supports both IPoIB and VNIC.  So expecting IPoIB and
> VNIC to use a generic "skb send" in ib_device is going to make hfi1 do a lot of
> work to determine which ULP is calling it or make the interface kind
> of ugly.

I don' think it will be that bad at all, the tx path is actually the
same execpt for the header construction. Handling that should not be
ugly, IMHO.

Think of it this way, if you do this you can probably boost the ipoib
performance on hfi as well.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v2 00/10] HFI Virtual Network Interface Controller (VNIC)
From: Vishwanathapura, Niranjana @ 2016-12-16  4:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w,
	ira.weiny-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <20161215165611.GB3264-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

On Thu, Dec 15, 2016 at 09:56:11AM -0700, Jason Gunthorpe wrote:
>On Wed, Dec 14, 2016 at 11:59:32PM -0800, Vishwanathapura, Niranjana wrote:
>>  create mode 100644 drivers/infiniband/sw/intel/hfi_vnic/Kconfig
>>  create mode 100644 drivers/infiniband/sw/intel/hfi_vnic/Makefile
>
>Stil NAK on these paths, I already explained why 'sw' is totally
>unsuitable. Put it in drivers/net or drivers/infiniband/ulp
>

I understand. I did not want to change dirver location until we concenses
on where it belongs.
In next revision, I will move it under drivers/infiniband/ulp/hfi_vnic.
If anybody thinks it should be in a different folder, let me know.

Niranjana

>Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v3 net] r6040: move spinlock in r6040_close as SOFTIRQ-unsafe lock order detected
From: Manuel Bessler @ 2016-12-16  3:55 UTC (permalink / raw)
  To: netdev; +Cc: f.fainelli, Manuel Bessler
In-Reply-To: <1481826099-12840-1-git-send-email-manuel.bessler@sensus.com>

'ifconfig eth0 down' makes r6040_close() trigger:
 INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected

Fixed by moving calls to phy_stop(), napi_disable(), netif_stop_queue()
to outside of the module's private spin_lock_irq block.

Found on a Versalogic Tomcat SBC with a Vortex86 SoC

s1660e_5150:~# sudo ifconfig eth0 down
[   61.306415] ======================================================
[   61.306415] [ INFO: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected ]
[   61.306415] 4.9.0-gb898d2d-manuel #1 Not tainted
[   61.306415] ------------------------------------------------------
[   61.306415] ifconfig/449 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
[   61.306415]  (&dev->lock){+.+...}, at: [<c1336276>] phy_stop+0x16/0x80

[   61.306415] and this task is already holding:
[   61.306415]  (&(&lp->lock)->rlock){+.-...}, at: [<d0934c84>] r6040_close+0x24/0x230 [r6040]
which would create a new lock dependency:
[   61.306415]  (&(&lp->lock)->rlock){+.-...} -> (&dev->lock){+.+...}

[   61.306415] but this new dependency connects a SOFTIRQ-irq-safe lock:
[   61.306415]  (&(&lp->lock)->rlock){+.-...}
[   61.306415] ... which became SOFTIRQ-irq-safe at:
[   61.306415]   [   61.306415] [<c1075bc5>] __lock_acquire+0x555/0x1770
[   61.306415]   [   61.306415] [<c107717c>] lock_acquire+0x7c/0x150
[   61.306415]   [   61.306415] [<c14bb334>] _raw_spin_lock_irqsave+0x24/0x40
[   61.306415]   [   61.306415] [<d0934ac0>] r6040_start_xmit+0x30/0x1d0 [r6040]
[   61.306415]   [   61.306415] [<c13a7d4d>] dev_hard_start_xmit+0x9d/0x2d0
[   61.306415]   [   61.306415] [<c13c8a38>] sch_direct_xmit+0xa8/0x140
[   61.306415]   [   61.306415] [<c13a8436>] __dev_queue_xmit+0x416/0x780
[   61.306415]   [   61.306415] [<c13a87aa>] dev_queue_xmit+0xa/0x10
[   61.306415]   [   61.306415] [<c13b4837>] neigh_resolve_output+0x147/0x220
[   61.306415]   [   61.306415] [<c144541b>] ip6_finish_output2+0x2fb/0x910
[   61.306415]   [   61.306415] [<c14494e6>] ip6_finish_output+0xa6/0x1a0
[   61.306415]   [   61.306415] [<c1449635>] ip6_output+0x55/0x320
[   61.306415]   [   61.306415] [<c146f4d2>] mld_sendpack+0x352/0x560
[   61.306415]   [   61.306415] [<c146fe55>] mld_ifc_timer_expire+0x155/0x280
[   61.306415]   [   61.306415] [<c108b081>] call_timer_fn+0x81/0x270
[   61.306415]   [   61.306415] [<c108b331>] expire_timers+0xc1/0x180
[   61.306415]   [   61.306415] [<c108b4f7>] run_timer_softirq+0x77/0x150
[   61.306415]   [   61.306415] [<c1043d04>] __do_softirq+0xb4/0x3d0
[   61.306415]   [   61.306415] [<c101a15c>] do_softirq_own_stack+0x1c/0x30
[   61.306415]   [   61.306415] [<c104416e>] irq_exit+0x8e/0xa0
[   61.306415]   [   61.306415] [<c1019d31>] do_IRQ+0x51/0x100
[   61.306415]   [   61.306415] [<c14bc176>] common_interrupt+0x36/0x40
[   61.306415]   [   61.306415] [<c1134928>] set_root+0x68/0xf0
[   61.306415]   [   61.306415] [<c1136120>] path_init+0x400/0x640
[   61.306415]   [   61.306415] [<c11386bf>] path_lookupat+0xf/0xe0
[   61.306415]   [   61.306415] [<c1139ebc>] filename_lookup+0x6c/0x100
[   61.306415]   [   61.306415] [<c1139fd5>] user_path_at_empty+0x25/0x30
[   61.306415]   [   61.306415] [<c11298c6>] SyS_faccessat+0x86/0x1e0
[   61.306415]   [   61.306415] [<c1129a30>] SyS_access+0x10/0x20
[   61.306415]   [   61.306415] [<c100179f>] do_int80_syscall_32+0x3f/0x110
[   61.306415]   [   61.306415] [<c14bba3f>] restore_all+0x0/0x61
[   61.306415]
[   61.306415] to a SOFTIRQ-irq-unsafe lock:
[   61.306415]  (&dev->lock){+.+...}
[   61.306415] ... which became SOFTIRQ-irq-unsafe at:
[   61.306415] ...[   61.306415]
[   61.306415] [<c1075c0c>] __lock_acquire+0x59c/0x1770
[   61.306415]   [   61.306415] [<c107717c>] lock_acquire+0x7c/0x150
[   61.306415]   [   61.306415] [<c14b7add>] mutex_lock_nested+0x2d/0x4a0
[   61.306415]   [   61.306415] [<c133747d>] phy_probe+0x4d/0xc0
[   61.306415]   [   61.306415] [<c1338afe>] phy_attach_direct+0xbe/0x190
[   61.306415]   [   61.306415] [<c1338ca7>] phy_connect_direct+0x17/0x60
[   61.306415]   [   61.306415] [<c1338d23>] phy_connect+0x33/0x70
[   61.306415]   [   61.306415] [<d09357a0>] r6040_init_one+0x3a0/0x500 [r6040]
[   61.306415]   [   61.306415] [<c12a78c7>] pci_device_probe+0x77/0xd0
[   61.306415]   [   61.306415] [<c12f5e15>] driver_probe_device+0x145/0x280
[   61.306415]   [   61.306415] [<c12f5fd9>] __driver_attach+0x89/0x90
[   61.306415]   [   61.306415] [<c12f43ef>] bus_for_each_dev+0x4f/0x80
[   61.306415]   [   61.306415] [<c12f5954>] driver_attach+0x14/0x20
[   61.306415]   [   61.306415] [<c12f55b7>] bus_add_driver+0x197/0x210
[   61.306415]   [   61.306415] [<c12f6a21>] driver_register+0x51/0xd0
[   61.306415]   [   61.306415] [<c12a6955>] __pci_register_driver+0x45/0x50
[   61.306415]   [   61.306415] [<d0938017>] 0xd0938017
[   61.306415]   [   61.306415] [<c100043f>] do_one_initcall+0x2f/0x140
[   61.306415]   [   61.306415] [<c10e48c0>] do_init_module+0x4a/0x19b
[   61.306415]   [   61.306415] [<c10a680e>] load_module+0x1b2e/0x2070
[   61.306415]   [   61.306415] [<c10a6eb9>] SyS_finit_module+0x69/0x80
[   61.306415]   [   61.306415] [<c100179f>] do_int80_syscall_32+0x3f/0x110
[   61.306415]   [   61.306415] [<c14bba3f>] restore_all+0x0/0x61
[   61.306415]
[   61.306415] other info that might help us debug this:
[   61.306415]
[   61.306415]  Possible interrupt unsafe locking scenario:
[   61.306415]
[   61.306415]        CPU0                    CPU1
[   61.306415]        ----                    ----
[   61.306415]   lock(&dev->lock);
[   61.306415]                                local_irq_disable();
[   61.306415]                                lock(&(&lp->lock)->rlock);
[   61.306415]                                lock(&dev->lock);
[   61.306415]   <Interrupt>
[   61.306415]     lock(&(&lp->lock)->rlock);
[   61.306415]
[   61.306415]  *** DEADLOCK ***
[   61.306415]
[   61.306415] 2 locks held by ifconfig/449:
[   61.306415]  #0:  (rtnl_mutex){+.+.+.}, at: [<c13b68ef>] rtnl_lock+0xf/0x20
[   61.306415]  #1:  (&(&lp->lock)->rlock){+.-...}, at: [<d0934c84>] r6040_close+0x24/0x230 [r6040]
[   61.306415]
[   61.306415] the dependencies between SOFTIRQ-irq-safe lock and the holding lock:
[   61.306415] -> (&(&lp->lock)->rlock){+.-...} ops: 3049 {
[   61.306415]    HARDIRQ-ON-W at:
[   61.306415]                     [   61.306415] [<c1075be7>] __lock_acquire+0x577/0x1770
[   61.306415]                     [   61.306415] [<c107717c>] lock_acquire+0x7c/0x150
[   61.306415]                     [   61.306415] [<c14bb21b>] _raw_spin_lock+0x1b/0x30
[   61.306415]                     [   61.306415] [<d09343cc>] r6040_poll+0x2c/0x330 [r6040]
[   61.306415]                     [   61.306415] [<c13a5577>] net_rx_action+0x197/0x340
[   61.306415]                     [   61.306415] [<c1043d04>] __do_softirq+0xb4/0x3d0
[   61.306415]                     [   61.306415] [<c1044037>] run_ksoftirqd+0x17/0x40
[   61.306415]                     [   61.306415] [<c105fe91>] smpboot_thread_fn+0x141/0x180
[   61.306415]                     [   61.306415] [<c105c84e>] kthread+0xde/0x110
[   61.306415]                     [   61.306415] [<c14bb949>] ret_from_fork+0x19/0x30
[   61.306415]    IN-SOFTIRQ-W at:
[   61.306415]                     [   61.306415] [<c1075bc5>] __lock_acquire+0x555/0x1770
[   61.306415]                     [   61.306415] [<c107717c>] lock_acquire+0x7c/0x150
[   61.306415]                     [   61.306415] [<c14bb334>] _raw_spin_lock_irqsave+0x24/0x40
[   61.306415]                     [   61.306415] [<d0934ac0>] r6040_start_xmit+0x30/0x1d0 [r6040]
[   61.306415]                     [   61.306415] [<c13a7d4d>] dev_hard_start_xmit+0x9d/0x2d0
[   61.306415]                     [   61.306415] [<c13c8a38>] sch_direct_xmit+0xa8/0x140
[   61.306415]                     [   61.306415] [<c13a8436>] __dev_queue_xmit+0x416/0x780
[   61.306415]                     [   61.306415] [<c13a87aa>] dev_queue_xmit+0xa/0x10
[   61.306415]                     [   61.306415] [<c13b4837>] neigh_resolve_output+0x147/0x220
[   61.306415]                     [   61.306415] [<c144541b>] ip6_finish_output2+0x2fb/0x910
[   61.306415]                     [   61.306415] [<c14494e6>] ip6_finish_output+0xa6/0x1a0
[   61.306415]                     [   61.306415] [<c1449635>] ip6_output+0x55/0x320
[   61.306415]                     [   61.306415] [<c146f4d2>] mld_sendpack+0x352/0x560
[   61.306415]                     [   61.306415] [<c146fe55>] mld_ifc_timer_expire+0x155/0x280
[   61.306415]                     [   61.306415] [<c108b081>] call_timer_fn+0x81/0x270
[   61.306415]                     [   61.306415] [<c108b331>] expire_timers+0xc1/0x180
[   61.306415]                     [   61.306415] [<c108b4f7>] run_timer_softirq+0x77/0x150
[   61.306415]                     [   61.306415] [<c1043d04>] __do_softirq+0xb4/0x3d0
[   61.306415]                     [   61.306415] [<c101a15c>] do_softirq_own_stack+0x1c/0x30
[   61.306415]                     [   61.306415] [<c104416e>] irq_exit+0x8e/0xa0
[   61.306415]                     [   61.306415] [<c1019d31>] do_IRQ+0x51/0x100
[   61.306415]                     [   61.306415] [<c14bc176>] common_interrupt+0x36/0x40
[   61.306415]                     [   61.306415] [<c1134928>] set_root+0x68/0xf0
[   61.306415]                     [   61.306415] [<c1136120>] path_init+0x400/0x640
[   61.306415]                     [   61.306415] [<c11386bf>] path_lookupat+0xf/0xe0
[   61.306415]                     [   61.306415] [<c1139ebc>] filename_lookup+0x6c/0x100
[   61.306415]                     [   61.306415] [<c1139fd5>] user_path_at_empty+0x25/0x30
[   61.306415]                     [   61.306415] [<c11298c6>] SyS_faccessat+0x86/0x1e0
[   61.306415]                     [   61.306415] [<c1129a30>] SyS_access+0x10/0x20
[   61.306415]                     [   61.306415] [<c100179f>] do_int80_syscall_32+0x3f/0x110
[   61.306415]                     [   61.306415] [<c14bba3f>] restore_all+0x0/0x61
[   61.306415]    INITIAL USE at:
[   61.306415]                    [   61.306415] [<c107586e>] __lock_acquire+0x1fe/0x1770
[   61.306415]                    [   61.306415] [<c107717c>] lock_acquire+0x7c/0x150
[   61.306415]                    [   61.306415] [<c14bb334>] _raw_spin_lock_irqsave+0x24/0x40
[   61.306415]                    [   61.306415] [<d093474e>] r6040_get_stats+0x1e/0x60 [r6040]
[   61.306415]                    [   61.306415] [<c139fb16>] dev_get_stats+0x96/0xc0
[   61.306415]                    [   61.306415] [<c14b416e>] rtnl_fill_stats+0x36/0xfd
[   61.306415]                    [   61.306415] [<c13b7b3c>] rtnl_fill_ifinfo+0x47c/0xce0
[   61.306415]                    [   61.306415] [<c13bc08e>] rtmsg_ifinfo_build_skb+0x4e/0xd0
[   61.306415]                    [   61.306415] [<c13bc120>] rtmsg_ifinfo.part.20+0x10/0x40
[   61.306415]                    [   61.306415] [<c13bc16b>] rtmsg_ifinfo+0x1b/0x20
[   61.306415]                    [   61.306415] [<c13a9d19>] register_netdevice+0x409/0x550
[   61.306415]                    [   61.306415] [<c13a9e72>] register_netdev+0x12/0x20
[   61.306415]                    [   61.306415] [<d09357e8>] r6040_init_one+0x3e8/0x500 [r6040]
[   61.306415]                    [   61.306415] [<c12a78c7>] pci_device_probe+0x77/0xd0
[   61.306415]                    [   61.306415] [<c12f5e15>] driver_probe_device+0x145/0x280
[   61.306415]                    [   61.306415] [<c12f5fd9>] __driver_attach+0x89/0x90
[   61.306415]                    [   61.306415] [<c12f43ef>] bus_for_each_dev+0x4f/0x80
[   61.306415]                    [   61.306415] [<c12f5954>] driver_attach+0x14/0x20
[   61.306415]                    [   61.306415] [<c12f55b7>] bus_add_driver+0x197/0x210
[   61.306415]                    [   61.306415] [<c12f6a21>] driver_register+0x51/0xd0
[   61.306415]                    [   61.306415] [<c12a6955>] __pci_register_driver+0x45/0x50
[   61.306415]                    [   61.306415] [<d0938017>] 0xd0938017
[   61.306415]                    [   61.306415] [<c100043f>] do_one_initcall+0x2f/0x140
[   61.306415]                    [   61.306415] [<c10e48c0>] do_init_module+0x4a/0x19b
[   61.306415]                    [   61.306415] [<c10a680e>] load_module+0x1b2e/0x2070
[   61.306415]                    [   61.306415] [<c10a6eb9>] SyS_finit_module+0x69/0x80
[   61.306415]                    [   61.306415] [<c100179f>] do_int80_syscall_32+0x3f/0x110
[   61.306415]                    [   61.306415] [<c14bba3f>] restore_all+0x0/0x61
[   61.306415]  }
[   61.306415]  ... key      at: [<d0936280>] __key.45893+0x0/0xfffff739 [r6040]
[   61.306415]  ... acquired at:
[   61.306415]    [   61.306415] [<c1074a32>] check_irq_usage+0x42/0xb0
[   61.306415]    [   61.306415] [<c107677c>] __lock_acquire+0x110c/0x1770
[   61.306415]    [   61.306415] [<c107717c>] lock_acquire+0x7c/0x150
[   61.306415]    [   61.306415] [<c14b7add>] mutex_lock_nested+0x2d/0x4a0
[   61.306415]    [   61.306415] [<c1336276>] phy_stop+0x16/0x80
[   61.306415]    [   61.306415] [<d0934ce9>] r6040_close+0x89/0x230 [r6040]
[   61.306415]    [   61.306415] [<c13a0a91>] __dev_close_many+0x61/0xa0
[   61.306415]    [   61.306415] [<c13a0bbf>] __dev_close+0x1f/0x30
[   61.306415]    [   61.306415] [<c13a9127>] __dev_change_flags+0x87/0x150
[   61.306415]    [   61.306415] [<c13a9213>] dev_change_flags+0x23/0x60
[   61.306415]    [   61.306415] [<c1416238>] devinet_ioctl+0x5f8/0x6f0
[   61.306415]    [   61.306415] [<c1417f75>] inet_ioctl+0x65/0x90
[   61.306415]    [   61.306415] [<c1389b54>] sock_ioctl+0x124/0x2b0
[   61.306415]    [   61.306415] [<c113cf7c>] do_vfs_ioctl+0x7c/0x790
[   61.306415]    [   61.306415] [<c113d6b8>] SyS_ioctl+0x28/0x50
[   61.306415]    [   61.306415] [<c100179f>] do_int80_syscall_32+0x3f/0x110
[   61.306415]    [   61.306415] [<c14bba3f>] restore_all+0x0/0x61
[   61.306415]
[   61.306415]
the dependencies between the lock to be acquired[   61.306415]  and SOFTIRQ-irq-unsafe lock:
[   61.306415] -> (&dev->lock){+.+...} ops: 56 {
[   61.306415]    HARDIRQ-ON-W at:
[   61.306415]                     [   61.306415] [<c1075be7>] __lock_acquire+0x577/0x1770
[   61.306415]                     [   61.306415] [<c107717c>] lock_acquire+0x7c/0x150
[   61.306415]                     [   61.306415] [<c14b7add>] mutex_lock_nested+0x2d/0x4a0
[   61.306415]                     [   61.306415] [<c133747d>] phy_probe+0x4d/0xc0
[   61.306415]                     [   61.306415] [<c1338afe>] phy_attach_direct+0xbe/0x190
[   61.306415]                     [   61.306415] [<c1338ca7>] phy_connect_direct+0x17/0x60
[   61.306415]                     [   61.306415] [<c1338d23>] phy_connect+0x33/0x70
[   61.306415]                     [   61.306415] [<d09357a0>] r6040_init_one+0x3a0/0x500 [r6040]
[   61.306415]                     [   61.306415] [<c12a78c7>] pci_device_probe+0x77/0xd0
[   61.306415]                     [   61.306415] [<c12f5e15>] driver_probe_device+0x145/0x280
[   61.306415]                     [   61.306415] [<c12f5fd9>] __driver_attach+0x89/0x90
[   61.306415]                     [   61.306415] [<c12f43ef>] bus_for_each_dev+0x4f/0x80
[   61.306415]                     [   61.306415] [<c12f5954>] driver_attach+0x14/0x20
[   61.306415]                     [   61.306415] [<c12f55b7>] bus_add_driver+0x197/0x210
[   61.306415]                     [   61.306415] [<c12f6a21>] driver_register+0x51/0xd0
[   61.306415]                     [   61.306415] [<c12a6955>] __pci_register_driver+0x45/0x50
[   61.306415]                     [   61.306415] [<d0938017>] 0xd0938017
[   61.306415]                     [   61.306415] [<c100043f>] do_one_initcall+0x2f/0x140
[   61.306415]                     [   61.306415] [<c10e48c0>] do_init_module+0x4a/0x19b
[   61.306415]                     [   61.306415] [<c10a680e>] load_module+0x1b2e/0x2070
[   61.306415]                     [   61.306415] [<c10a6eb9>] SyS_finit_module+0x69/0x80
[   61.306415]                     [   61.306415] [<c100179f>] do_int80_syscall_32+0x3f/0x110
[   61.306415]                     [   61.306415] [<c14bba3f>] restore_all+0x0/0x61
[   61.306415]    SOFTIRQ-ON-W at:
[   61.306415]                     [   61.306415] [<c1075c0c>] __lock_acquire+0x59c/0x1770
[   61.306415]                     [   61.306415] [<c107717c>] lock_acquire+0x7c/0x150
[   61.306415]                     [   61.306415] [<c14b7add>] mutex_lock_nested+0x2d/0x4a0
[   61.306415]                     [   61.306415] [<c133747d>] phy_probe+0x4d/0xc0
[   61.306415]                     [   61.306415] [<c1338afe>] phy_attach_direct+0xbe/0x190
[   61.306415]                     [   61.306415] [<c1338ca7>] phy_connect_direct+0x17/0x60
[   61.306415]                     [   61.306415] [<c1338d23>] phy_connect+0x33/0x70
[   61.306415]                     [   61.306415] [<d09357a0>] r6040_init_one+0x3a0/0x500 [r6040]
[   61.306415]                     [   61.306415] [<c12a78c7>] pci_device_probe+0x77/0xd0
[   61.306415]                     [   61.306415] [<c12f5e15>] driver_probe_device+0x145/0x280
[   61.306415]                     [   61.306415] [<c12f5fd9>] __driver_attach+0x89/0x90
[   61.306415]                     [   61.306415] [<c12f43ef>] bus_for_each_dev+0x4f/0x80
[   61.306415]                     [   61.306415] [<c12f5954>] driver_attach+0x14/0x20
[   61.306415]                     [   61.306415] [<c12f55b7>] bus_add_driver+0x197/0x210
[   61.306415]                     [   61.306415] [<c12f6a21>] driver_register+0x51/0xd0
[   61.306415]                     [   61.306415] [<c12a6955>] __pci_register_driver+0x45/0x50
[   61.306415]                     [   61.306415] [<d0938017>] 0xd0938017
[   61.306415]                     [   61.306415] [<c100043f>] do_one_initcall+0x2f/0x140
[   61.306415]                     [   61.306415] [<c10e48c0>] do_init_module+0x4a/0x19b
[   61.306415]                     [   61.306415] [<c10a680e>] load_module+0x1b2e/0x2070
[   61.306415]                     [   61.306415] [<c10a6eb9>] SyS_finit_module+0x69/0x80
[   61.306415]                     [   61.306415] [<c100179f>] do_int80_syscall_32+0x3f/0x110
[   61.306415]                     [   61.306415] [<c14bba3f>] restore_all+0x0/0x61
[   61.306415]    INITIAL USE at:
[   61.306415]                    [   61.306415] [<c107586e>] __lock_acquire+0x1fe/0x1770
[   61.306415]                    [   61.306415] [<c107717c>] lock_acquire+0x7c/0x150
[   61.306415]                    [   61.306415] [<c14b7add>] mutex_lock_nested+0x2d/0x4a0
[   61.306415]                    [   61.306415] [<c133747d>] phy_probe+0x4d/0xc0
[   61.306415]                    [   61.306415] [<c1338afe>] phy_attach_direct+0xbe/0x190
[   61.306415]                    [   61.306415] [<c1338ca7>] phy_connect_direct+0x17/0x60
[   61.306415]                    [   61.306415] [<c1338d23>] phy_connect+0x33/0x70
[   61.306415]                    [   61.306415] [<d09357a0>] r6040_init_one+0x3a0/0x500 [r6040]
[   61.306415]                    [   61.306415] [<c12a78c7>] pci_device_probe+0x77/0xd0
[   61.306415]                    [   61.306415] [<c12f5e15>] driver_probe_device+0x145/0x280
[   61.306415]                    [   61.306415] [<c12f5fd9>] __driver_attach+0x89/0x90
[   61.306415]                    [   61.306415] [<c12f43ef>] bus_for_each_dev+0x4f/0x80
[   61.306415]                    [   61.306415] [<c12f5954>] driver_attach+0x14/0x20
[   61.306415]                    [   61.306415] [<c12f55b7>] bus_add_driver+0x197/0x210
[   61.306415]                    [   61.306415] [<c12f6a21>] driver_register+0x51/0xd0
[   61.306415]                    [   61.306415] [<c12a6955>] __pci_register_driver+0x45/0x50
[   61.306415]                    [   61.306415] [<d0938017>] 0xd0938017
[   61.306415]                    [   61.306415] [<c100043f>] do_one_initcall+0x2f/0x140
[   61.306415]                    [   61.306415] [<c10e48c0>] do_init_module+0x4a/0x19b
[   61.306415]                    [   61.306415] [<c10a680e>] load_module+0x1b2e/0x2070
[   61.306415]                    [   61.306415] [<c10a6eb9>] SyS_finit_module+0x69/0x80
[   61.306415]                    [   61.306415] [<c100179f>] do_int80_syscall_32+0x3f/0x110
[   61.306415]                    [   61.306415] [<c14bba3f>] restore_all+0x0/0x61
[   61.306415]  }
[   61.306415]  ... key      at: [<c1f28f39>] __key.43998+0x0/0x8
[   61.306415]  ... acquired at:
[   61.306415]    [   61.306415] [<c1074a32>] check_irq_usage+0x42/0xb0
[   61.306415]    [   61.306415] [<c107677c>] __lock_acquire+0x110c/0x1770
[   61.306415]    [   61.306415] [<c107717c>] lock_acquire+0x7c/0x150
[   61.306415]    [   61.306415] [<c14b7add>] mutex_lock_nested+0x2d/0x4a0
[   61.306415]    [   61.306415] [<c1336276>] phy_stop+0x16/0x80
[   61.306415]    [   61.306415] [<d0934ce9>] r6040_close+0x89/0x230 [r6040]
[   61.306415]    [   61.306415] [<c13a0a91>] __dev_close_many+0x61/0xa0
[   61.306415]    [   61.306415] [<c13a0bbf>] __dev_close+0x1f/0x30
[   61.306415]    [   61.306415] [<c13a9127>] __dev_change_flags+0x87/0x150
[   61.306415]    [   61.306415] [<c13a9213>] dev_change_flags+0x23/0x60
[   61.306415]    [   61.306415] [<c1416238>] devinet_ioctl+0x5f8/0x6f0
[   61.306415]    [   61.306415] [<c1417f75>] inet_ioctl+0x65/0x90
[   61.306415]    [   61.306415] [<c1389b54>] sock_ioctl+0x124/0x2b0
[   61.306415]    [   61.306415] [<c113cf7c>] do_vfs_ioctl+0x7c/0x790
[   61.306415]    [   61.306415] [<c113d6b8>] SyS_ioctl+0x28/0x50
[   61.306415]    [   61.306415] [<c100179f>] do_int80_syscall_32+0x3f/0x110
[   61.306415]    [   61.306415] [<c14bba3f>] restore_all+0x0/0x61
[   61.306415]
[   61.306415]
[   61.306415] stack backtrace:
[   61.306415] CPU: 0 PID: 449 Comm: ifconfig Not tainted 4.9.0-gb898d2d-manuel #1
[   61.306415] Call Trace:
[   61.306415]  dump_stack+0x16/0x19
[   61.306415]  check_usage+0x3f6/0x550
[   61.306415]  ? check_usage+0x4d/0x550
[   61.306415]  check_irq_usage+0x42/0xb0
[   61.306415]  __lock_acquire+0x110c/0x1770
[   61.306415]  lock_acquire+0x7c/0x150
[   61.306415]  ? phy_stop+0x16/0x80
[   61.306415]  mutex_lock_nested+0x2d/0x4a0
[   61.306415]  ? phy_stop+0x16/0x80
[   61.306415]  ? r6040_close+0x24/0x230 [r6040]
[   61.306415]  ? __delay+0x9/0x10
[   61.306415]  phy_stop+0x16/0x80
[   61.306415]  r6040_close+0x89/0x230 [r6040]
[   61.306415]  __dev_close_many+0x61/0xa0
[   61.306415]  __dev_close+0x1f/0x30
[   61.306415]  __dev_change_flags+0x87/0x150
[   61.306415]  dev_change_flags+0x23/0x60
[   61.306415]  devinet_ioctl+0x5f8/0x6f0
[   61.306415]  inet_ioctl+0x65/0x90
[   61.306415]  sock_ioctl+0x124/0x2b0
[   61.306415]  ? dlci_ioctl_set+0x30/0x30
[   61.306415]  do_vfs_ioctl+0x7c/0x790
[   61.306415]  ? trace_hardirqs_on+0xb/0x10
[   61.306415]  ? call_rcu_sched+0xd/0x10
[   61.306415]  ? __put_cred+0x32/0x50
[   61.306415]  ? SyS_faccessat+0x178/0x1e0
[   61.306415]  SyS_ioctl+0x28/0x50
[   61.306415]  do_int80_syscall_32+0x3f/0x110
[   61.306415]  entry_INT80_32+0x2f/0x2f
[   61.306415] EIP: 0xb764d364
[   61.306415] EFLAGS: 00000286 CPU: 0
[   61.306415] EAX: ffffffda EBX: 00000004 ECX: 00008914 EDX: bfa99d7c
[   61.306415] ESI: bfa99e4c EDI: fffffffe EBP: 00000004 ESP: bfa99d58
[   61.306415]  DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b
[   63.836607] r6040 0000:00:08.0 eth0: Link is Down

Signed-off-by: Manuel Bessler <manuel.bessler@sensus.com>
---
 drivers/net/ethernet/rdc/r6040.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index 4ff4e04..aa11b70 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -472,8 +472,6 @@ static void r6040_down(struct net_device *dev)
 	iowrite16(adrp[0], ioaddr + MID_0L);
 	iowrite16(adrp[1], ioaddr + MID_0M);
 	iowrite16(adrp[2], ioaddr + MID_0H);
-
-	phy_stop(dev->phydev);
 }
 
 static int r6040_close(struct net_device *dev)
@@ -481,12 +479,12 @@ static int r6040_close(struct net_device *dev)
 	struct r6040_private *lp = netdev_priv(dev);
 	struct pci_dev *pdev = lp->pdev;
 
-	spin_lock_irq(&lp->lock);
+	phy_stop(dev->phydev);
 	napi_disable(&lp->napi);
 	netif_stop_queue(dev);
-	r6040_down(dev);
 
-	free_irq(dev->irq, dev);
+	spin_lock_irq(&lp->lock);
+	r6040_down(dev);
 
 	/* Free RX buffer */
 	r6040_free_rxbufs(dev);
@@ -496,6 +494,8 @@ static int r6040_close(struct net_device *dev)
 
 	spin_unlock_irq(&lp->lock);
 
+	free_irq(dev->irq, dev);
+
 	/* Free Descriptor memory */
 	if (lp->rx_ring) {
 		pci_free_consistent(pdev,
-- 
2.7.4

^ permalink raw reply related


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