Netdev List
 help / color / mirror / Atom feed
* Re: tun issue after e0b46d0ee9c: tun: Use iovec iterators
From: Marcelo Ricardo Leitner @ 2014-11-29  5:08 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev
In-Reply-To: <20141128235935.GA2178@gondor.apana.org.au>

On 28-11-2014 21:59, Herbert Xu wrote:
> On Fri, Nov 28, 2014 at 05:25:27PM -0200, Marcelo Ricardo Leitner wrote:
>>
>> I saw there are tun updates on Dave's queue but none seemed to handle this.
>>
>> I can't use current net-next
>> (799d2fff1858004526ad75d66a5dd8a5cce6ad40) on a kvm hypervisor
>> because tun got clogged somehow. Bisected down to:
>>
>> commit e0b46d0ee9c240c7430a47e9b0365674d4a04522
>> Author: Herbert Xu <herbert@gondor.apana.org.au>
>> Date:   Fri Nov 7 21:22:23 2014 +0800
>
> Does this patch help?

Yay, it does! Works for me, thanks Herbert.
I didn't test performance, but dhcp could get through.

Are you sure about the Fixes tag? Because bisect really pointed to e0b46d0ee9c.

Cheers,
Marcelo

> -- >8 --
> Subject: tun: Fix GSO meta-data handling in tun_get_user
>
> When we write the GSO meta-data in tun_get_user we end up advancing
> the IO vector twice, thus exhausting the user buffer before we can
> finish writing the packet.
>
> Fixes: f5ff53b4d97c ("{macvtap,tun}_get_user(): switch to iov_iter")
> Reported-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 4b743c6..9357871 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1052,7 +1052,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>
>   		if (gso.hdr_len > len)
>   			return -EINVAL;
> -		iov_iter_advance(from, tun->vnet_hdr_sz);
> +		iov_iter_advance(iter, tun->vnet_hdr_sz - sizeof(gso));
>   	}
>
>   	if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) {
>
> Cheers,
>

^ permalink raw reply

* Re: [PATCH] drivers:net:wireless: Add mutex locking for function, b43_op_beacon_set_time in main.c
From: Rafał Miłecki @ 2014-11-29  8:54 UTC (permalink / raw)
  To: nick
  Cc: Michael Büsch, Stefano Brivio, Network Development,
	linux-wireless@vger.kernel.org, Linux Kernel Mailing List,
	b43-dev
In-Reply-To: <54793E4E.7050602@gmail.com>

On 29 November 2014 at 04:32, nick <xerofoify@gmail.com> wrote:
> I don't have hardware for this driver on me, so I didn't test it. However this seems to
> be correct from my reading of the code around this function and other locking related
> to this driver.

So do you say it's not executed in an atomic?

>From *early* look it seems b43_update_templates is called from
b43_op_beacon_set_tim and b43_op_bss_info_changed, both or them are
mac80211 callbacks. That also should be safe, however I didn't check
if it may conflict with some in-kernel code (still, assuming it's not
atomic code, which I don't know!).

^ permalink raw reply

* Re: [PATCH] drivers:net:wireless: Add mutex locking for function, b43_op_beacon_set_time in main.c
From: Michael Büsch @ 2014-11-29  8:56 UTC (permalink / raw)
  To: nick
  Cc: Rafał Miłecki, Stefano Brivio, Network Development,
	linux-wireless@vger.kernel.org, Linux Kernel Mailing List,
	b43-dev
In-Reply-To: <54793E4E.7050602@gmail.com>

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

On Fri, 28 Nov 2014 22:32:30 -0500
nick <xerofoify@gmail.com> wrote:

> I don't have hardware for this driver on me, so I didn't test it. However this seems to 
> be correct from my reading of the code around this function and other locking related
> to this driver.

From the current docs:

>  * @set_tim: Set TIM bit. mac80211 calls this function when a TIM bit
>  *      must be set or cleared for a given STA. Must be atomic.

So it is not allowed to lock a mutex here.

-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH] drivers:net:wireless: Add mutex locking for function, b43_op_beacon_set_time in main.c
From: Rafał Miłecki @ 2014-11-29  9:04 UTC (permalink / raw)
  To: Michael Büsch
  Cc: nick, Stefano Brivio, Network Development,
	linux-wireless@vger.kernel.org, Linux Kernel Mailing List,
	b43-dev
In-Reply-To: <20141129095632.69361457@wiggum>

On 29 November 2014 at 09:56, Michael Büsch <m@bues.ch> wrote:
> On Fri, 28 Nov 2014 22:32:30 -0500
> nick <xerofoify@gmail.com> wrote:
>
>> I don't have hardware for this driver on me, so I didn't test it. However this seems to
>> be correct from my reading of the code around this function and other locking related
>> to this driver.
>
> From the current docs:
>
>>  * @set_tim: Set TIM bit. mac80211 calls this function when a TIM bit
>>  *      must be set or cleared for a given STA. Must be atomic.
>
> So it is not allowed to lock a mutex here.

Nicholas please be more careful with your patches. It seems your patch
is wrong, you didn't check the docs and you didn't even say in the
first place that it wasn't tested. You could at least submit with with
RFT tag or sth.

-- 
Rafał

^ permalink raw reply

* Re: [patch net-next v5 06/21] net: introduce generic switch devices support
From: Lino Sanfilippo @ 2014-11-29  9:31 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse, pshelar,
	azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
	john.r.fastabend, edumazet, jhs, sfeldma, f.fainelli, roopa,
	linville, jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a,
	buytenh, aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye,
	simon.horman, alexander.h.duyck, john.ronciak, mleitner, shrijeet,
	gospo, bcrl, hemal
In-Reply-To: <1417181672-11531-7-git-send-email-jiri@resnulli.us>


Hi,

On 28.11.2014 14:34, Jiri Pirko wrote:
> +
> +  ndo_switch_parent_id_get - This returns the same ID for two port netdevices
> +			     of the same physical switch chip. This is
> +			     mandatory to be implemented by all switch drivers
> +			     and serves the caller for recognition of a port
> +			     netdevice.
> +  ndo_switch_parent_* - Functions that serve for a manipulation of the switch
> +			chip itself (it can be though of as a "parent" of the
> +			port, therefore the name). They are not port-specific.
> +			Caller might use arbitrary port netdevice of the same
> +			switch and it will make no difference.

I doubt that this is a good solution. If you want to access some kind of
parent device why dont you provide a device structure for it along with
register and unregister functions? Also each device should IMHO show up
in sysfs somehow, which should also be part of a register function.
There is already an existing driver (see net/dsa) that addresses the
representation of switch devices. What is wrong with that?

Regards,
Lino

^ permalink raw reply

* Re: [patch net-next v5 06/21] net: introduce generic switch devices support
From: Jiri Pirko @ 2014-11-29  9:59 UTC (permalink / raw)
  To: Lino Sanfilippo
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
	f.fainelli, roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5479927B.4020807@gmx.de>

Sat, Nov 29, 2014 at 10:31:39AM CET, LinoSanfilippo@gmx.de wrote:
>
>Hi,
>
>On 28.11.2014 14:34, Jiri Pirko wrote:
>> +
>> +  ndo_switch_parent_id_get - This returns the same ID for two port netdevices
>> +			     of the same physical switch chip. This is
>> +			     mandatory to be implemented by all switch drivers
>> +			     and serves the caller for recognition of a port
>> +			     netdevice.
>> +  ndo_switch_parent_* - Functions that serve for a manipulation of the switch
>> +			chip itself (it can be though of as a "parent" of the
>> +			port, therefore the name). They are not port-specific.
>> +			Caller might use arbitrary port netdevice of the same
>> +			switch and it will make no difference.
>
>I doubt that this is a good solution. If you want to access some kind of
>parent device why dont you provide a device structure for it along with
>register and unregister functions? Also each device should IMHO show up
>in sysfs somehow, which should also be part of a register function.

Lino, this was already discussed. So far there is no need to introduce
such new infrastructure. But we have deal that once the need appears, it
will be added (it easily can be).

>There is already an existing driver (see net/dsa) that addresses the
>representation of switch devices. What is wrong with that?

This was already discussed as well. dsa is more of a driver for specific
devices (has for example highly integrated mdio interface). There is
also a deal to keep these 2 thing separated for now and try to merge
that as much as we can in the future.

>
>Regards,
>Lino
>
>
>

^ permalink raw reply

* Re: [patch net-next v5 06/21] net: introduce generic switch devices support
From: Lino Sanfilippo @ 2014-11-29 10:44 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
	f.fainelli, roopa, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141129095902.GA1856@nanopsycho.orion>

On 29.11.2014 10:59, Jiri Pirko wrote:

> 
> Lino, this was already discussed. So far there is no need to introduce
> such new infrastructure. But we have deal that once the need appears, it
> will be added (it easily can be).
> 
>>There is already an existing driver (see net/dsa) that addresses the
>>representation of switch devices. What is wrong with that?
> 
> This was already discussed as well. dsa is more of a driver for specific
> devices (has for example highly integrated mdio interface). There is
> also a deal to keep these 2 thing separated for now and try to merge
> that as much as we can in the future.
> 

I admit i did not follow the whole discussion. I was just wondering why
the existing dsa driver is not extended. But as you said it is planned
to do this in future. So thanks for your explanation.

Regards,
Lino

^ permalink raw reply

* Re: [PATCH] e1000: remove unused variables
From: Lino Sanfilippo @ 2014-11-29 11:01 UTC (permalink / raw)
  To: Florian Fainelli, Sudip Mukherjee, Hisashi T Fujinaka
  Cc: Linux NICS, e1000-devel, Bruce Allan, Jesse Brandeburg,
	linux-kernel, John Ronciak, netdev
In-Reply-To: <54790537.809@gmail.com>

On 29.11.2014 00:28, Florian Fainelli wrote:

> Also, if you do a read that is not stored in any return value, the
> compiler is now free to remove that actual read, 

This does not apply to reads from iomem (see "volatile" specifier in
readl()).

Regards,
Lino


------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* [PATCH, regression against -rc6] net/stmmac: fix one more regression from filter bins setting
From: Arnd Bergmann @ 2014-11-29 12:03 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, khilman, Huacai Chen, Giuseppe Cavallaro, olof,
	linux-arm-kernel

Commit 571dcfde2371 ("stmmac: platform: fix default values of the filter
bins setting") tries to fix a regression in the stmmac driver, but instead
it also causes a new regression and makes all DT-based users of this
driver fail, as found by the automated ARM boot testing:

[    0.881667] Unable to handle kernel NULL pointer dereference at virtual address 00000048
[    0.889784] pgd = c0204000
[    0.892502] [00000048] *pgd=00000000
[    0.896090] Internal error: Oops: 805 [#1] SMP ARM
[    0.900874] Modules linked in:
[    0.903943] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.18.0-rc6-00150-g8891063 #1
[    0.911502] task: ee06c000 ti: ee070000 task.ti: ee070000
[    0.916899] PC is at stmmac_pltfr_probe+0x40/0x5d0
[    0.921689] LR is at devm_ioremap_nocache+0x54/0x74
[    0.926561] pc : [<c0642df8>]    lr : [<c0430508>]    psr: 80000153
[    0.926561] sp : ee071e50  ip : 00000000  fp : 00000000
[    0.938020] r10: c0c14f74  r9 : ee3ca2c0  r8 : ee101c10
[    0.943237] r7 : f00c0000  r6 : ee101c00  r5 : ee101c10  r4 : 00000000
[    0.949755] r3 : 00000001  r2 : 00000040  r1 : a0000153  r0 : f00c0000
[    0.956274] Flags: Nzcv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
[    0.963658] Control: 10c5387d  Table: 4020406a  DAC: 00000015
[    0.969395] Process swapper/0 (pid: 1, stack limit = 0xee070248)
[    0.975384] Stack: (0xee071e50 to 0xee072000)
[    0.979738] 1e40:                                     c0d83aa0 ee101c10 c0cfc030 fffffdfb
[    0.987906] 1e60: 00000000 ee3ca2c0 c0c14f74 c056c6d0 c056c68c c0d83aa0 ee101c10 00000000
[    0.996075] 1e80: c0cfc030 c056af54 ee101c10 c0cfc030 ee101c44 c0cf15f8 c0c616a0 c056b158
[    1.004243] 1ea0: 00000000 c0cfc030 c056b0cc c0569514 ee01675c ee0d86b4 c0cfc030 ee3c9300
[    1.012412] 1ec0: 00000000 c056a77c c0ad7be0 ee071ef8 c0cfc030 c0cfc030 ee070000 c0be0ca8
[    1.020580] 1ee0: 00000000 c056b77c 00000000 c0c616a0 ee070000 c0be0cbc c0c616a0 c0208cb0
[    1.028750] 1f00: ee011900 c0d6c324 ee0b6600 c08a6748 00000000 00000000 00006a40 c034782c
[    1.036918] 1f20: 00000000 c0ca1aac 60000153 00000001 c0b9f598 ef7fcd9c ef7fcd99 c025ffa0
[    1.045087] 1f40: c0ae650c ef7fcda0 00000006 00000006 c0ca1a94 c0c50d44 00000006 c0c14f6c
[    1.053255] 1f60: c0d46a40 c0d46a40 000000ff c0c14f74 00000000 c0b9fda4 00000006 00000006
[    1.061423] 1f80: c0b9f598 9c041042 00000000 c089460c 00000000 00000000 00000000 00000000
[    1.069591] 1fa0: 00000000 c0894614 00000000 c020e878 00000000 00000000 00000000 00000000
[    1.077759] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    1.085926] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 18044434 8a444414
[    1.094113] [<c0642df8>] (stmmac_pltfr_probe) from [<c056c6d0>] (platform_drv_probe+0x44/0xa4)
[    1.102719] [<c056c6d0>] (platform_drv_probe) from [<c056af54>] (driver_probe_device+0x108/0x23c)
[    1.111582] [<c056af54>] (driver_probe_device) from [<c056b158>] (__driver_attach+0x8c/0x90)
[    1.120011] [<c056b158>] (__driver_attach) from [<c0569514>] (bus_for_each_dev+0x6c/0xa0)
[    1.128182] [<c0569514>] (bus_for_each_dev) from [<c056a77c>] (bus_add_driver+0x148/0x1f0)
[    1.136438] [<c056a77c>] (bus_add_driver) from [<c056b77c>] (driver_register+0x78/0xf8)
[    1.144434] [<c056b77c>] (driver_register) from [<c0be0cbc>] (stmmac_init+0x14/0x3c)
[    1.152176] [<c0be0cbc>] (stmmac_init) from [<c0208cb0>] (do_one_initcall+0x8c/0x1c4)
[    1.160004] [<c0208cb0>] (do_one_initcall) from [<c0b9fda4>] (kernel_init_freeable+0x13c/0x1dc)
[    1.168699] [<c0b9fda4>] (kernel_init_freeable) from [<c0894614>] (kernel_init+0x8/0xe8)
[    1.176785] [<c0894614>] (kernel_init) from [<c020e878>] (ret_from_fork+0x14/0x3c)
[    1.184348] Code: 8a0000fe e5964064 e3a02040 e3a03001 (e5842048)
[    1.190501] ---[ end trace 724411f6ae142767 ]---
[    1.195158] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

Where the original patch left out the non-DT probe method, the fix that
was merged for -rc7 instead broke the DT path by dereferencing a NULL
pointer.

This patch reverts the broken fix and tries to put the initialization
for multicast_filter_bins/unicast_filter_entries in the right place,
which prevents non-DT users from overriding the defaults as a side
effect, as before. Alternatively, one could change all instances of
the platform_data definition to set the values themselves, but it's
probably safer to do that as a follow-up.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 571dcfde2371 ("stmmac: platform: fix default values of the filter bins setting")
Link: http://arm-soc.lixom.net/bootlogs/mainline/v3.18-rc6-150-g8891063/cubie2-arm-multi_v7_defconfig.html
---
I've sent this off to the build bots, but it will probably take some more
time before it's been tested on all the ARM machines and I might not be
there to see the results, so I'm sending it out now for review. I will
reply once I am sure the problem is fixed.

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 5b0da3986216..94c3cefaa2c3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -177,6 +177,12 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
 	 */
 	plat->maxmtu = JUMBO_LEN;
 
+	/* Set default value for multicast hash bins */
+	plat->multicast_filter_bins = HASH_TABLE_SIZE;
+
+	/* Set default value for unicast filter entries */
+	plat->unicast_filter_entries = 1;
+
 	/*
 	 * Currently only the properties needed on SPEAr600
 	 * are provided. All other properties should be added
@@ -264,13 +270,6 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
 		return PTR_ERR(addr);
 
 	plat_dat = dev_get_platdata(&pdev->dev);
-
-	/* Set default value for multicast hash bins */
-	plat_dat->multicast_filter_bins = HASH_TABLE_SIZE;
-
-	/* Set default value for unicast filter entries */
-	plat_dat->unicast_filter_entries = 1;
-
 	if (pdev->dev.of_node) {
 		if (!plat_dat)
 			plat_dat = devm_kzalloc(&pdev->dev,
@@ -286,6 +285,12 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
 			pr_err("%s: main dt probe failed", __func__);
 			return ret;
 		}
+	} else {
+		/* Set default value for multicast hash bins */
+		plat_dat->multicast_filter_bins = HASH_TABLE_SIZE;
+
+		/* Set default value for unicast filter entries */
+		plat_dat->unicast_filter_entries = 1;
 	}
 
 	/* Custom setup (if needed) */

^ permalink raw reply related

* [bisected] xfrm: TCP connection initiating PMTU discovery stalls on v3.12+
From: Thomas Jarosch @ 2014-11-29 11:44 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet

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

Hello,

we're in the process of updating production level machines
from kernel 3.4.101 to kernel 3.14.25. On one mail server
we noticed that emails destined for an IPSec tunnel sometimes
get stuck in the mail queue with TCP timeouts.

To make a long story short: When the VPN connection is initially
set up or re-newed, the path MTU for the xfrm tunnel is undetermined.

As soon as a TCP client starts to send large packets,
it triggers path MTU detection. Some middlebox on the
way to the final server has a lower MTU and sends back
an "ICMP fragmentation needed" packet as normal.

With the old kernel, the packet size for the TCP connection inside
the xfrm tunnel gets adjusted and all is fine. With kernel v3.12+,
the connection stalls completely. Same thing with kernel v3.18-rc6.

We wrote a small tool to mimic postfix's TCP behavior (see attached file).
In the end it's a normal TCP client sending large packets.
The server side is just "socat - tcp4-listen:667".

If you run "socket_client" a second time, the path MTU
for the xfrm tunnel is already known and packets flow normal, too.


The "evil" commit in question is this one:
---------------------------------------------------------------------
commit 8f26fb1c1ed81c33f5d87c5936f4d9d1b4118918
Author: Eric Dumazet <edumazet@google.com>
Date:   Tue Oct 15 12:24:54 2013 -0700

    tcp: remove the sk_can_gso() check from tcp_set_skb_tso_segs()

    sk_can_gso() should only be used as a hint in tcp_sendmsg() to build GSO
    packets in the first place. (As a performance hint)

    Once we have GSO packets in write queue, we can not decide they are no
    longer GSO only because flow now uses a route which doesn't handle
    TSO/GSO.

    Core networking stack handles the case very well for us, all we need
    is keeping track of packet counts in MSS terms, regardless of
    segmentation done later (in GSO or hardware)

    Right now, if  tcp_fragment() splits a GSO packet in two parts,
    @left and @right, and route changed through a non GSO device,
    both @left and @right have pcount set to 1, which is wrong,
    and leads to incorrect packet_count tracking.

    This problem was added in commit d5ac99a648 ("[TCP]: skb pcount with MTU
    discovery")

    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Signed-off-by: Neal Cardwell <ncardwell@google.com>
    Signed-off-by: Yuchung Cheng <ycheng@google.com>
    Reported-by: Maciej Żenczykowski <maze@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 8fad1c1..d46f214 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -989,8 +989,7 @@ static void tcp_set_skb_tso_segs(const struct sock *sk, struct sk_buff *skb,
        /* Make sure we own this skb before messing gso_size/gso_segs */
        WARN_ON_ONCE(skb_cloned(skb));
 
-       if (skb->len <= mss_now || !sk_can_gso(sk) ||
-           skb->ip_summed == CHECKSUM_NONE) {
+       if (skb->len <= mss_now || skb->ip_summed == CHECKSUM_NONE) {
                /* Avoid the costly divide in the normal
                 * non-TSO case.
                 */
---------------------------------------------------------------------

When I revert it, even kernel v3.18-rc6 starts working.
But I doubt this is the root problem, may be just hiding another issue.

--- Sample output of socket_client using vanilla v3.12 kernel ---
[1417258063 SEND result: 4096, strerror: Success]
tcp max seg: res: 0, max_seg: 1370
[1417258063 SEND result: 4096, strerror: Success]
tcp max seg: res: 0, max_seg: 1370
[1417258063 SEND result: 4096, strerror: Success]
tcp max seg: res: 0, max_seg: 1370
[1417258063 SEND result: 4096, strerror: Success]
tcp max seg: res: 0, max_seg: 1370
[1417258063 SEND result: 4096, strerror: Success]
tcp max seg: res: 0, max_seg: 1338
[1417258063 SEND result: 4096, strerror: Success]
tcp max seg: res: 0, max_seg: 1338
*STUCK*
--------------------------------------------------------

The "machine" is running on KVM and using "virtio_net" as NIC driver.
I've played with the ethtool offload settings:

*** eth1 defaults ***
Offload parameters for eth1:
rx-checksumming: on
tx-checksumming: on
scatter-gather: on
tcp-segmentation-offload: on
udp-fragmentation-offload: on
generic-segmentation-offload: on
generic-receive-offload: on
large-receive-offload: off

*** eth1 working (no stalls) using vanilla kernel ***
Offload parameters for eth1:
rx-checksumming: on
tx-checksumming: off  <-- the magic switch
scatter-gather: off
tcp-segmentation-offload: off
udp-fragmentation-offload: off
generic-segmentation-offload: off
generic-receive-offload: off
large-receive-offload: off

When I turn "tx-checksumming" back on, it fails again.
Though that is probably also just a side effect.

I can provide tcpdumps if needed but they are no real help
since you can just see the kernel stops sending TCP packets.
(and the outgoing TCP packets are encrypted in ESP packets)


Any vague idea what might be the root cause?

I also tried reverting commit 4d53eff48b5f03ce67f4f301d6acca1d2145cb7a
("xfrm: Don't queue retransmitted packets if the original is still on the host")
but that didn't change the situation. In fact it wasn't even triggered.

Please CC: comments. Thanks.

Best regards,
Thomas

[-- Attachment #2: socket_client.c --]
[-- Type: text/x-csrc, Size: 1327 bytes --]

#include <sys/types.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <netinet/in.h>
#include <netinet/tcp.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/un.h>
#include <unistd.h>
#include <errno.h>
#include <time.h>

/*
    Remote server: socat - tcp4-listen:667
*/

int main()
{
    int sockfd = socket(AF_INET, SOCK_STREAM, 0);

    struct sockaddr_in servaddr;
    bzero(&servaddr,sizeof(servaddr));
    servaddr.sin_family = AF_INET;
    servaddr.sin_addr.s_addr=inet_addr("192.168.12.254");
    servaddr.sin_port=htons(667);

    int result = connect(sockfd, (struct sockaddr *)&servaddr, sizeof(servaddr));
    if(result != 0)
    {
        perror("failed to connect");
        exit(1);
    }

    char sendbuf[4096];
    memset(sendbuf, 0, sizeof(sendbuf));
    strcpy(sendbuf, "NOOP\n");

    int max_seg = 0, max_seg_len = sizeof(max_seg), get_res = 0;

    for (int i = 0; i < 10; ++i)
    {
        errno = 0;
        int send_res = send(sockfd, sendbuf, sizeof(sendbuf), 0);
        printf("[%d SEND result: %d, strerror: %s]\n", time(NULL), send_res, strerror(errno));

        get_res = getsockopt(sockfd, SOL_TCP, TCP_MAXSEG, &max_seg, &max_seg_len);
        printf("tcp max seg: res: %d, max_seg: %d\n", get_res, max_seg);
    }

    printf("All sent.\n");

    close(sockfd);
    exit(0);
}

^ permalink raw reply related

* [PATCH 1/1] HID: Wacom: Deletion of an unnecessary check before the function call "vfree"
From: SF Markus Elfring @ 2014-11-29 13:42 UTC (permalink / raw)
  To: netdev; +Cc: LKML, kernel-janitors, Coccinelle
In-Reply-To: <5317A59D.4@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 29 Nov 2014 14:34:59 +0100

The vfree() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/ethernet/sun/cassini.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index 37f87ff..d745808 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -5179,8 +5179,7 @@ static void cas_remove_one(struct pci_dev *pdev)
 	cp = netdev_priv(dev);
 	unregister_netdev(dev);
 
-	if (cp->fw_data)
-		vfree(cp->fw_data);
+	vfree(cp->fw_data);
 
 	mutex_lock(&cp->pm_mutex);
 	cancel_work_sync(&cp->reset_task);
-- 
2.1.3


^ permalink raw reply related

* [PATCH 1/1] net: cassini: Deletion of an unnecessary check before the function call "vfree"
From: SF Markus Elfring @ 2014-11-29 14:05 UTC (permalink / raw)
  To: netdev; +Cc: LKML, kernel-janitors, Coccinelle
In-Reply-To: <5479CD2A.9030009@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 29 Nov 2014 14:34:59 +0100

The vfree() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/ethernet/sun/cassini.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index 37f87ff..d745808 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -5179,8 +5179,7 @@ static void cas_remove_one(struct pci_dev *pdev)
 	cp = netdev_priv(dev);
 	unregister_netdev(dev);
 
-	if (cp->fw_data)
-		vfree(cp->fw_data);
+	vfree(cp->fw_data);
 
 	mutex_lock(&cp->pm_mutex);
 	cancel_work_sync(&cp->reset_task);
-- 
2.1.3


^ permalink raw reply related

* [PATCH 1/1] net-ipvlan: Deletion of an unnecessary check before the function call "free_percpu"
From: SF Markus Elfring @ 2014-11-29 15:30 UTC (permalink / raw)
  To: netdev; +Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 29 Nov 2014 16:23:20 +0100

The free_percpu() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/ipvlan/ipvlan_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 96b71b0..feb1853 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -125,8 +125,7 @@ static void ipvlan_uninit(struct net_device *dev)
 	struct ipvl_dev *ipvlan = netdev_priv(dev);
 	struct ipvl_port *port = ipvlan->port;
 
-	if (ipvlan->pcpu_stats)
-		free_percpu(ipvlan->pcpu_stats);
+	free_percpu(ipvlan->pcpu_stats);
 
 	port->count -= 1;
 	if (!port->count)
-- 
2.1.3

^ permalink raw reply related

* Re:[PATCH, regression against -rc6] net/stmmac: fix one more regression from filter bins setting
From: 陈华才 @ 2014-11-29 15:44 UTC (permalink / raw)
  To: Arnd Bergmann, David Miller
  Cc: olof, netdev, Giuseppe Cavallaro, linux-arm-kernel, khilman
In-Reply-To: <5388366.Y15J6TLmlA@wuerfel>

Hi, Arnd,

Maybe this patch is better?
http://www.spinics.net/lists/netdev/msg306413.html

Huacai
 
------------------ Original ------------------
From:  "Arnd Bergmann"<arnd@arndb.de>;
Date:  Sat, Nov 29, 2014 08:03 PM
To:  "David Miller"<davem@davemloft.net>;
Cc:  "netdev"<netdev@vger.kernel.org>; "khilman"<khilman@kernel.org>; "Huacai Chen"<chenhc@lemote.com>; "Giuseppe Cavallaro"<peppe.cavallaro@st.com>; "olof"<olof@lixom.net>; "linux-arm-kernel"<linux-arm-kernel@lists.infradead.org>;
Subject:  [PATCH, regression against -rc6] net/stmmac: fix one more regression from filter bins setting
 
Commit 571dcfde2371 ("stmmac: platform: fix default values of the filter
bins setting") tries to fix a regression in the stmmac driver, but instead
it also causes a new regression and makes all DT-based users of this
driver fail, as found by the automated ARM boot testing:

[    0.881667] Unable to handle kernel NULL pointer dereference at virtual address 00000048
[    0.889784] pgd = c0204000
[    0.892502] [00000048] *pgd=00000000
[    0.896090] Internal error: Oops: 805 [#1] SMP ARM
[    0.900874] Modules linked in:
[    0.903943] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.18.0-rc6-00150-g8891063 #1
[    0.911502] task: ee06c000 ti: ee070000 task.ti: ee070000
[    0.916899] PC is at stmmac_pltfr_probe+0x40/0x5d0
[    0.921689] LR is at devm_ioremap_nocache+0x54/0x74
[    0.926561] pc : [<c0642df8>]    lr : [<c0430508>]    psr: 80000153
[    0.926561] sp : ee071e50  ip : 00000000  fp : 00000000
[    0.938020] r10: c0c14f74  r9 : ee3ca2c0  r8 : ee101c10
[    0.943237] r7 : f00c0000  r6 : ee101c00  r5 : ee101c10  r4 : 00000000
[    0.949755] r3 : 00000001  r2 : 00000040  r1 : a0000153  r0 : f00c0000
[    0.956274] Flags: Nzcv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
[    0.963658] Control: 10c5387d  Table: 4020406a  DAC: 00000015
[    0.969395] Process swapper/0 (pid: 1, stack limit = 0xee070248)
[    0.975384] Stack: (0xee071e50 to 0xee072000)
[    0.979738] 1e40:                                     c0d83aa0 ee101c10 c0cfc030 fffffdfb
[    0.987906] 1e60: 00000000 ee3ca2c0 c0c14f74 c056c6d0 c056c68c c0d83aa0 ee101c10 00000000
[    0.996075] 1e80: c0cfc030 c056af54 ee101c10 c0cfc030 ee101c44 c0cf15f8 c0c616a0 c056b158
[    1.004243] 1ea0: 00000000 c0cfc030 c056b0cc c0569514 ee01675c ee0d86b4 c0cfc030 ee3c9300
[    1.012412] 1ec0: 00000000 c056a77c c0ad7be0 ee071ef8 c0cfc030 c0cfc030 ee070000 c0be0ca8
[    1.020580] 1ee0: 00000000 c056b77c 00000000 c0c616a0 ee070000 c0be0cbc c0c616a0 c0208cb0
[    1.028750] 1f00: ee011900 c0d6c324 ee0b6600 c08a6748 00000000 00000000 00006a40 c034782c
[    1.036918] 1f20: 00000000 c0ca1aac 60000153 00000001 c0b9f598 ef7fcd9c ef7fcd99 c025ffa0
[    1.045087] 1f40: c0ae650c ef7fcda0 00000006 00000006 c0ca1a94 c0c50d44 00000006 c0c14f6c
[    1.053255] 1f60: c0d46a40 c0d46a40 000000ff c0c14f74 00000000 c0b9fda4 00000006 00000006
[    1.061423] 1f80: c0b9f598 9c041042 00000000 c089460c 00000000 00000000 00000000 00000000
[    1.069591] 1fa0: 00000000 c0894614 00000000 c020e878 00000000 00000000 00000000 00000000
[    1.077759] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    1.085926] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 18044434 8a444414
[    1.094113] [<c0642df8>] (stmmac_pltfr_probe) from [<c056c6d0>] (platform_drv_probe+0x44/0xa4)
[    1.102719] [<c056c6d0>] (platform_drv_probe) from [<c056af54>] (driver_probe_device+0x108/0x23c)
[    1.111582] [<c056af54>] (driver_probe_device) from [<c056b158>] (__driver_attach+0x8c/0x90)
[    1.120011] [<c056b158>] (__driver_attach) from [<c0569514>] (bus_for_each_dev+0x6c/0xa0)
[    1.128182] [<c0569514>] (bus_for_each_dev) from [<c056a77c>] (bus_add_driver+0x148/0x1f0)
[    1.136438] [<c056a77c>] (bus_add_driver) from [<c056b77c>] (driver_register+0x78/0xf8)
[    1.144434] [<c056b77c>] (driver_register) from [<c0be0cbc>] (stmmac_init+0x14/0x3c)
[    1.152176] [<c0be0cbc>] (stmmac_init) from [<c0208cb0>] (do_one_initcall+0x8c/0x1c4)
[    1.160004] [<c0208cb0>] (do_one_initcall) from [<c0b9fda4>] (kernel_init_freeable+0x13c/0x1dc)
[    1.168699] [<c0b9fda4>] (kernel_init_freeable) from [<c0894614>] (kernel_init+0x8/0xe8)
[    1.176785] [<c0894614>] (kernel_init) from [<c020e878>] (ret_from_fork+0x14/0x3c)
[    1.184348] Code: 8a0000fe e5964064 e3a02040 e3a03001 (e5842048)
[    1.190501] ---[ end trace 724411f6ae142767 ]---
[    1.195158] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

Where the original patch left out the non-DT probe method, the fix that
was merged for -rc7 instead broke the DT path by dereferencing a NULL
pointer.

This patch reverts the broken fix and tries to put the initialization
for multicast_filter_bins/unicast_filter_entries in the right place,
which prevents non-DT users from overriding the defaults as a side
effect, as before. Alternatively, one could change all instances of
the platform_data definition to set the values themselves, but it's
probably safer to do that as a follow-up.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 571dcfde2371 ("stmmac: platform: fix default values of the filter bins setting")
Link: http://arm-soc.lixom.net/bootlogs/mainline/v3.18-rc6-150-g8891063/cubie2-arm-multi_v7_defconfig.html
---
I've sent this off to the build bots, but it will probably take some more
time before it's been tested on all the ARM machines and I might not be
there to see the results, so I'm sending it out now for review. I will
reply once I am sure the problem is fixed.

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 5b0da3986216..94c3cefaa2c3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -177,6 +177,12 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
 	 */
 	plat->maxmtu = JUMBO_LEN;
 
+	/* Set default value for multicast hash bins */
+	plat->multicast_filter_bins = HASH_TABLE_SIZE;
+
+	/* Set default value for unicast filter entries */
+	plat->unicast_filter_entries = 1;
+
 	/*
 	 * Currently only the properties needed on SPEAr600
 	 * are provided. All other properties should be added
@@ -264,13 +270,6 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
 	 return PTR_ERR(addr);
 
 	plat_dat = dev_get_platdata(&pdev->dev);
-
-	/* Set default value for multicast hash bins */
-	plat_dat->multicast_filter_bins = HASH_TABLE_SIZE;
-
-	/* Set default value for unicast filter entries */
-	plat_dat->unicast_filter_entries = 1;
-
 	if (pdev->dev.of_node) {
 	 if (!plat_dat)
 	 plat_dat = devm_kzalloc(&pdev->dev,
@@ -286,6 +285,12 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
 	 pr_err("%s: main dt probe failed", __func__);
 	 return ret;
 	 }
+	} else {
+	 /* Set default value for multicast hash bins */
+	 plat_dat->multicast_filter_bins = HASH_TABLE_SIZE;
+
+	 /* Set default value for unicast filter entries */
+	 plat_dat->unicast_filter_entries = 1;
 	}
 
 	/* Custom setup (if needed) */

^ permalink raw reply related

* Re: [PATCH, regression against -rc6] net/stmmac: fix one more regression from filter bins setting
From: Arnd Bergmann @ 2014-11-29 16:26 UTC (permalink / raw)
  To: 陈华才
  Cc: David Miller, netdev, khilman, Giuseppe Cavallaro, olof,
	linux-arm-kernel
In-Reply-To: <tencent_0FE25D6968E3D939662D9E00@qq.com>

On Saturday 29 November 2014 23:44:23 陈华才 wrote:
> Hi, Arnd,
> 
> Maybe this patch is better?
> http://www.spinics.net/lists/netdev/msg306413.html

Yes, that would work too. I also checked that my version works now.

I'm fine with either one, as long as a fix makes it into 3.18.

	Arnd

> ------------------ Original ------------------
> From:  "Arnd Bergmann"<arnd@arndb.de>;
> Date:  Sat, Nov 29, 2014 08:03 PM
> To:  "David Miller"<davem@davemloft.net>;
> Cc:  "netdev"<netdev@vger.kernel.org>; "khilman"<khilman@kernel.org>; "Huacai Chen"<chenhc@lemote.com>; "Giuseppe Cavallaro"<peppe.cavallaro@st.com>; "olof"<olof@lixom.net>; "linux-arm-kernel"<linux-arm-kernel@lists.infradead.org>;
> Subject:  [PATCH, regression against -rc6] net/stmmac: fix one more regression from filter bins setting
>  
> Commit 571dcfde2371 ("stmmac: platform: fix default values of the filter
> bins setting") tries to fix a regression in the stmmac driver, but instead
> it also causes a new regression and makes all DT-based users of this
> driver fail, as found by the automated ARM boot testing:
> 
> [    0.881667] Unable to handle kernel NULL pointer dereference at virtual address 00000048
> [    0.889784] pgd = c0204000
> [    0.892502] [00000048] *pgd=00000000
> [    0.896090] Internal error: Oops: 805 [#1] SMP ARM
> [    0.900874] Modules linked in:
> [    0.903943] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.18.0-rc6-00150-g8891063 #1
> [    0.911502] task: ee06c000 ti: ee070000 task.ti: ee070000
> [    0.916899] PC is at stmmac_pltfr_probe+0x40/0x5d0
> [    0.921689] LR is at devm_ioremap_nocache+0x54/0x74
> [    0.926561] pc : [<c0642df8>]    lr : [<c0430508>]    psr: 80000153
> [    0.926561] sp : ee071e50  ip : 00000000  fp : 00000000
> [    0.938020] r10: c0c14f74  r9 : ee3ca2c0  r8 : ee101c10
> [    0.943237] r7 : f00c0000  r6 : ee101c00  r5 : ee101c10  r4 : 00000000
> [    0.949755] r3 : 00000001  r2 : 00000040  r1 : a0000153  r0 : f00c0000
> [    0.956274] Flags: Nzcv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
> [    0.963658] Control: 10c5387d  Table: 4020406a  DAC: 00000015
> [    0.969395] Process swapper/0 (pid: 1, stack limit = 0xee070248)
> [    0.975384] Stack: (0xee071e50 to 0xee072000)
> [    0.979738] 1e40:                                     c0d83aa0 ee101c10 c0cfc030 fffffdfb
> [    0.987906] 1e60: 00000000 ee3ca2c0 c0c14f74 c056c6d0 c056c68c c0d83aa0 ee101c10 00000000
> [    0.996075] 1e80: c0cfc030 c056af54 ee101c10 c0cfc030 ee101c44 c0cf15f8 c0c616a0 c056b158
> [    1.004243] 1ea0: 00000000 c0cfc030 c056b0cc c0569514 ee01675c ee0d86b4 c0cfc030 ee3c9300
> [    1.012412] 1ec0: 00000000 c056a77c c0ad7be0 ee071ef8 c0cfc030 c0cfc030 ee070000 c0be0ca8
> [    1.020580] 1ee0: 00000000 c056b77c 00000000 c0c616a0 ee070000 c0be0cbc c0c616a0 c0208cb0
> [    1.028750] 1f00: ee011900 c0d6c324 ee0b6600 c08a6748 00000000 00000000 00006a40 c034782c
> [    1.036918] 1f20: 00000000 c0ca1aac 60000153 00000001 c0b9f598 ef7fcd9c ef7fcd99 c025ffa0
> [    1.045087] 1f40: c0ae650c ef7fcda0 00000006 00000006 c0ca1a94 c0c50d44 00000006 c0c14f6c
> [    1.053255] 1f60: c0d46a40 c0d46a40 000000ff c0c14f74 00000000 c0b9fda4 00000006 00000006
> [    1.061423] 1f80: c0b9f598 9c041042 00000000 c089460c 00000000 00000000 00000000 00000000
> [    1.069591] 1fa0: 00000000 c0894614 00000000 c020e878 00000000 00000000 00000000 00000000
> [    1.077759] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    1.085926] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 18044434 8a444414
> [    1.094113] [<c0642df8>] (stmmac_pltfr_probe) from [<c056c6d0>] (platform_drv_probe+0x44/0xa4)
> [    1.102719] [<c056c6d0>] (platform_drv_probe) from [<c056af54>] (driver_probe_device+0x108/0x23c)
> [    1.111582] [<c056af54>] (driver_probe_device) from [<c056b158>] (__driver_attach+0x8c/0x90)
> [    1.120011] [<c056b158>] (__driver_attach) from [<c0569514>] (bus_for_each_dev+0x6c/0xa0)
> [    1.128182] [<c0569514>] (bus_for_each_dev) from [<c056a77c>] (bus_add_driver+0x148/0x1f0)
> [    1.136438] [<c056a77c>] (bus_add_driver) from [<c056b77c>] (driver_register+0x78/0xf8)
> [    1.144434] [<c056b77c>] (driver_register) from [<c0be0cbc>] (stmmac_init+0x14/0x3c)
> [    1.152176] [<c0be0cbc>] (stmmac_init) from [<c0208cb0>] (do_one_initcall+0x8c/0x1c4)
> [    1.160004] [<c0208cb0>] (do_one_initcall) from [<c0b9fda4>] (kernel_init_freeable+0x13c/0x1dc)
> [    1.168699] [<c0b9fda4>] (kernel_init_freeable) from [<c0894614>] (kernel_init+0x8/0xe8)
> [    1.176785] [<c0894614>] (kernel_init) from [<c020e878>] (ret_from_fork+0x14/0x3c)
> [    1.184348] Code: 8a0000fe e5964064 e3a02040 e3a03001 (e5842048)
> [    1.190501] ---[ end trace 724411f6ae142767 ]---
> [    1.195158] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> 
> Where the original patch left out the non-DT probe method, the fix that
> was merged for -rc7 instead broke the DT path by dereferencing a NULL
> pointer.
> 
> This patch reverts the broken fix and tries to put the initialization
> for multicast_filter_bins/unicast_filter_entries in the right place,
> which prevents non-DT users from overriding the defaults as a side
> effect, as before. Alternatively, one could change all instances of
> the platform_data definition to set the values themselves, but it's
> probably safer to do that as a follow-up.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 571dcfde2371 ("stmmac: platform: fix default values of the filter bins setting")
> Link: http://arm-soc.lixom.net/bootlogs/mainline/v3.18-rc6-150-g8891063/cubie2-arm-multi_v7_defconfig.html
> ---
> I've sent this off to the build bots, but it will probably take some more
> time before it's been tested on all the ARM machines and I might not be
> there to see the results, so I'm sending it out now for review. I will
> reply once I am sure the problem is fixed.
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 5b0da3986216..94c3cefaa2c3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -177,6 +177,12 @@ static int stmmac_probe_config_dt(struct platform_device *pdev,
>  	 */
>  	plat->maxmtu = JUMBO_LEN;
>  
> +	/* Set default value for multicast hash bins */
> +	plat->multicast_filter_bins = HASH_TABLE_SIZE;
> +
> +	/* Set default value for unicast filter entries */
> +	plat->unicast_filter_entries = 1;
> +
>  	/*
>  	 * Currently only the properties needed on SPEAr600
>  	 * are provided. All other properties should be added
> @@ -264,13 +270,6 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
>  	 return PTR_ERR(addr);
>  
>  	plat_dat = dev_get_platdata(&pdev->dev);
> -
> -	/* Set default value for multicast hash bins */
> -	plat_dat->multicast_filter_bins = HASH_TABLE_SIZE;
> -
> -	/* Set default value for unicast filter entries */
> -	plat_dat->unicast_filter_entries = 1;
> -
>  	if (pdev->dev.of_node) {
>  	 if (!plat_dat)
>  	 plat_dat = devm_kzalloc(&pdev->dev,
> @@ -286,6 +285,12 @@ static int stmmac_pltfr_probe(struct platform_device *pdev)
>  	 pr_err("%s: main dt probe failed", __func__);
>  	 return ret;
>  	 }
> +	} else {
> +	 /* Set default value for multicast hash bins */
> +	 plat_dat->multicast_filter_bins = HASH_TABLE_SIZE;
> +
> +	 /* Set default value for unicast filter entries */
> +	 plat_dat->unicast_filter_entries = 1;
>  	}
>  
>  	/* Custom setup (if needed) */

^ permalink raw reply

* Re: [PATCH, regression against -rc6] net/stmmac: fix one more regression from filter bins setting
From: Arnd Bergmann @ 2014-11-29 16:29 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, khilman, Huacai Chen, Giuseppe Cavallaro, olof,
	linux-arm-kernel
In-Reply-To: <5388366.Y15J6TLmlA@wuerfel>

On Saturday 29 November 2014 13:03:19 Arnd Bergmann wrote:
> I've sent this off to the build bots, but it will probably take some more
> time before it's been tested on all the ARM machines and I might not be
> there to see the results, so I'm sending it out now for review. I will
> reply once I am sure the problem is fixed.

With this patch applied, both boot bots are back to 100% success now,
all eight failing machines are working again.

	Arnd

^ permalink raw reply

* Re: [PATCH v6 37/46] tun: move internal flag defines out of uapi
From: Michael S. Tsirkin @ 2014-11-29 17:17 UTC (permalink / raw)
  To: Jason Wang
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Miller,
	cornelia.huck-tA70FqPdS9bQT0dZR+AlfA,
	rusty-8fk3Idey6ehBDgjK7y7TUQ, nab-IzHhD5pYlfBP7FQvKIMDCQ,
	pbonzini-H+wXaHxf7aLQT0dZR+AlfA,
	thuth-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	dahi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Zhi Yong Wu, Ben Hutchings,
	Herbert Xu, Tom Herbert, Masatake YAMATO, Xi Wang,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1417162052.5822.0-LLcwxoN42L4NLKR9yMNcA1aTQe2KTcn/@public.gmane.org>

On Fri, Nov 28, 2014 at 08:15:32AM +0008, Jason Wang wrote:
> 
> 
> On Fri, Nov 28, 2014 at 4:10 AM, Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >TUN_ flags are internal and never exposed
> >to userspace. Any application using it is almost
> >certainly buggy.
> >
> >Move them out to tun.c, we'll remove them in follow-up patches.
> >
> >Signed-off-by: Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >---
> > include/uapi/linux/if_tun.h | 16 ++--------
> > drivers/net/tun.c           | 74
> >++++++++++++++-------------------------------
> > 2 files changed, 26 insertions(+), 64 deletions(-)
> >
> >diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> >index e9502dd..277a260 100644
> >--- a/include/uapi/linux/if_tun.h
> >+++ b/include/uapi/linux/if_tun.h
> >@@ -22,21 +22,11 @@
> > /* Read queue size */
> > #define TUN_READQ_SIZE	500
> >-
> >-/* TUN device flags */
> >-#define TUN_TUN_DEV 	0x0001	
> >-#define TUN_TAP_DEV	0x0002
> >+/* TUN device type flags: deprecated. Use IFF_TUN/IFF_TAP instead. */
> >+#define TUN_TUN_DEV 	IFF_TUN
> >+#define TUN_TAP_DEV	IFF_TAP
> > #define TUN_TYPE_MASK   0x000f
> >-#define TUN_FASYNC	0x0010
> >-#define TUN_NOCHECKSUM	0x0020
> >-#define TUN_NO_PI	0x0040
> >-/* This flag has no real effect */
> >-#define TUN_ONE_QUEUE	0x0080
> >-#define TUN_PERSIST 	0x0100	
> >-#define TUN_VNET_HDR 	0x0200
> >-#define TUN_TAP_MQ      0x0400
> >-
> > /* Ioctl defines */
> > #define TUNSETNOCSUM  _IOW('T', 200, int)  #define TUNSETDEBUG
> >_IOW('T', 201, int) diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >index 9dd3746..bc89d07 100644
> >--- a/drivers/net/tun.c
> >+++ b/drivers/net/tun.c
> >@@ -103,6 +103,21 @@ do {								\
> > } while (0)
> > #endif
> >+/* TUN device flags */
> >+
> >+/* IFF_ATTACH_QUEUE is never stored in device flags,
> >+ * overload it to mean fasync when stored there.
> >+ */
> >+#define TUN_FASYNC	IFF_ATTACH_QUEUE
> >+#define TUN_NO_PI	IFF_NO_PI
> >+/* This flag has no real effect */
> >+#define TUN_ONE_QUEUE	IFF_ONE_QUEUE
> >+#define TUN_PERSIST 	IFF_PERSIST
> >+#define TUN_VNET_HDR 	IFF_VNET_HDR
> >+#define TUN_TAP_MQ      IFF_MULTI_QUEUE
> >+
> >+#define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
> >+		      IFF_MULTI_QUEUE)
> > #define GOODCOPY_LEN 128
> > #define FLT_EXACT_COUNT 8
> >@@ -1521,32 +1536,7 @@ static struct proto tun_proto = {
> > static int tun_flags(struct tun_struct *tun)
> > {
> >-	int flags = 0;
> >-
> >-	if (tun->flags & TUN_TUN_DEV)
> >-		flags |= IFF_TUN;
> >-	else
> >-		flags |= IFF_TAP;
> >-
> >-	if (tun->flags & TUN_NO_PI)
> >-		flags |= IFF_NO_PI;
> >-
> >-	/* This flag has no real effect.  We track the value for backwards
> >-	 * compatibility.
> >-	 */
> >-	if (tun->flags & TUN_ONE_QUEUE)
> >-		flags |= IFF_ONE_QUEUE;
> >-
> >-	if (tun->flags & TUN_VNET_HDR)
> >-		flags |= IFF_VNET_HDR;
> >-
> >-	if (tun->flags & TUN_TAP_MQ)
> >-		flags |= IFF_MULTI_QUEUE;
> >-
> >-	if (tun->flags & TUN_PERSIST)
> >-		flags |= IFF_PERSIST;
> >-
> >-	return flags;
> >+	return tun->flags & (TUN_FEATURES | IFF_PERSIST | IFF_TUN | IFF_TAP);
> > }
> >   static ssize_t tun_show_flags(struct device *dev, struct
> >device_attribute *attr,
> >@@ -1706,28 +1696,8 @@ static int tun_set_iff(struct net *net, struct file
> >*file, struct ifreq *ifr)
> > 	tun_debug(KERN_INFO, tun, "tun_set_iff\n");
> >-	if (ifr->ifr_flags & IFF_NO_PI)
> >-		tun->flags |= TUN_NO_PI;
> >-	else
> >-		tun->flags &= ~TUN_NO_PI;
> >-
> >-	/* This flag has no real effect.  We track the value for backwards
> >-	 * compatibility.
> >-	 */
> >-	if (ifr->ifr_flags & IFF_ONE_QUEUE)
> >-		tun->flags |= TUN_ONE_QUEUE;
> >-	else
> >-		tun->flags &= ~TUN_ONE_QUEUE;
> >-
> >-	if (ifr->ifr_flags & IFF_VNET_HDR)
> >-		tun->flags |= TUN_VNET_HDR;
> >-	else
> >-		tun->flags &= ~TUN_VNET_HDR;
> >-
> >-	if (ifr->ifr_flags & IFF_MULTI_QUEUE)
> >-		tun->flags |= TUN_TAP_MQ;
> >-	else
> >-		tun->flags &= ~TUN_TAP_MQ;
> >+	tun->flags = (tun->flags & ~TUN_FEATURES) |
> >+		(ifr->ifr_flags & TUN_FEATURES);
> > 	/* Make sure persistent devices do not get stuck in
> > 	 * xoff state.
> >@@ -1890,9 +1860,11 @@ static long __tun_chr_ioctl(struct file *file,
> >unsigned int cmd,
> > 	if (cmd == TUNGETFEATURES) {
> > 		/* Currently this just means: "what IFF flags are valid?".
> > 		 * This is needed because we never checked for invalid flags on
> >-		 * TUNSETIFF. */
> >-		return put_user(IFF_TUN | IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE |
> >-				IFF_VNET_HDR | IFF_MULTI_QUEUE,
> >+		 * TUNSETIFF.  Why do we report IFF_TUN and IFF_TAP which are
> >+		 * not legal for TUNSETIFF here?  It's probably a bug, but it
> >+		 * doesn't seem to be worth fixing.
> >+		 */
> 
> The question is not very clear. IFF_TUN and IFF_TAP are legal for
> ifr.ifr_flags during TUNSETIFF. No?


Absolutely. I'm not sure what I meant anymore. I will drop this chunk -
either by replacing this patch if I have to post another version, or
by a separate patch if I don't.

> >
> >+		return put_user(IFF_TUN | IFF_TAP | TUN_FEATURES,
> > 				(unsigned int __user*)argp);
> > 	} else if (cmd == TUNSETQUEUE)
> > 		return tun_set_queue(file, &ifr);
> >-- 
> >MST
> >
> >--
> >To unsubscribe from this list: send the line "unsubscribe netdev" 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: [PATCH v6 38/46] tun: drop most type defines
From: Michael S. Tsirkin @ 2014-11-29 17:23 UTC (permalink / raw)
  To: Jason Wang
  Cc: linux-kernel, David Miller, cornelia.huck, rusty, nab, pbonzini,
	thuth, dahi, Zhi Yong Wu, Tom Herbert, Ben Hutchings,
	Masatake YAMATO, Herbert Xu, Xi Wang, netdev
In-Reply-To: <1417162188.5822.1@smtp.corp.redhat.com>

On Fri, Nov 28, 2014 at 08:17:48AM +0008, Jason Wang wrote:
> 
> 
> On Fri, Nov 28, 2014 at 4:11 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >It's just as easy to use IFF_ flags directly,
> >there's no point in adding our own defines.
> >
> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >---
> > drivers/net/tun.c | 62
> >+++++++++++++++++++++++++------------------------------
> > 1 file changed, 28 insertions(+), 34 deletions(-)
> >
> >diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> 
> Patch looks fine, but I don't see the reason that why not squash patch 37
> into this.
> 
> [..]

squashing is always easy, I thought review is easier if at each step
identity to original code can be verified directly.

^ permalink raw reply

* Re: [PATCH v6 34/46] virtio_net: disable mac write for virtio 1.0
From: Michael S. Tsirkin @ 2014-11-29 17:28 UTC (permalink / raw)
  To: Jason Wang
  Cc: thuth, rusty, netdev, linux-kernel, virtualization, dahi,
	pbonzini, David Miller
In-Reply-To: <1417164024.5822.11@smtp.corp.redhat.com>

On Fri, Nov 28, 2014 at 08:48:24AM +0008, Jason Wang wrote:
> 
> 
> On Fri, Nov 28, 2014 at 4:10 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >The spec states that mac in config space is only driver-writable in the
> >legacy case.  Fence writing it in virtnet_set_mac_address() in the
> >virtio 1.0 case.
> >
> >Suggested-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >---
> > drivers/net/virtio_net.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >index c6a72d3..9ab3c50 100644
> >--- a/drivers/net/virtio_net.c
> >+++ b/drivers/net/virtio_net.c
> >@@ -1030,7 +1030,8 @@ static int virtnet_set_mac_address(struct net_device
> >*dev, void *p)
> > 				 "Failed to set mac address by vq command.\n");
> > 			return -EINVAL;
> > 		}
> >-	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
> >+	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC) &&
> >+		   !virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> > 		unsigned int i;
> > 		/* Naturally, this has an atomicity problem. */
> >-- 
> >MST
> 
> Does this mean there's no way to setting mac addres
> if ctrl mac addr is disabled in virtio-1?

More exactly: no way for host to know the guest mac address.

> If yes, we'd better keep this compatibility somehow.

If host wants to be notified about guest mac updates,
it must enable the appropriate ctrl command.
Or if it's an old host, it just doesn't enable virtio-1.

There's no way around this - it's in the spec.


-- 
MST

^ permalink raw reply

* Re: [PATCH 0/5 net] bridge: Fix missing Netlink message validations
From: John Fastabend @ 2014-11-29 17:51 UTC (permalink / raw)
  To: Thomas Graf; +Cc: Jiri Pirko, davem, stephen, netdev
In-Reply-To: <20141126231436.GF32174@casper.infradead.org>

On 11/26/2014 03:14 PM, Thomas Graf wrote:
> On 11/26/14 at 09:25am, John Fastabend wrote:
>>> --- a/net/ipv4/devinet.c
>>> +++ b/net/ipv4/devinet.c
>>> @@ -1687,8 +1687,11 @@ static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla)
>>>                 BUG();
>>>
>>>         if (tb[IFLA_INET_CONF]) {
>>> -               nla_for_each_nested(a, tb[IFLA_INET_CONF], rem)
>>> +               nla_for_each_nested(a, tb[IFLA_INET_CONF], rem) {
>>> +                       if (nla_len(a) < sizeof(u32))
>>> +                               return -EINVAL;
>>>                         ipv4_devconf_set(in_dev, nla_type(a), nla_get_u32(a));
>>> +               }
>
> Looked into this and found a validation function
> inet_validate_link_af(). It's split to keep the updates atomic.
>

Ah great thanks.

-- 
John Fastabend         Intel Corporation

^ permalink raw reply

* Re: [PATCH net] bond: Check length of IFLA_BOND_ARP_IP_TARGET attributes
From: John Fastabend @ 2014-11-29 17:53 UTC (permalink / raw)
  To: Thomas Graf; +Cc: davem, netdev, Scott Feldman
In-Reply-To: <a7c193909c30dbc19b4f279a3f7ba757107641d3.1417044113.git.tgraf@suug.ch>

On 11/26/2014 03:22 PM, Thomas Graf wrote:
> Fixes: 7f28fa10 ("bonding: add arp_ip_target netlink support")
> Reported-by: John Fastabend <john.fastabend@gmail.com>
> Cc: Scott Feldman <sfeldma@cumulusnetworks.com>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> ---
>   drivers/net/bonding/bond_netlink.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>

Looks good thanks! For what its worth...

Acked-by: John Fastabend <john.r.fastabend@intel.com>

> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> index c13d83e..45f09a6 100644
> --- a/drivers/net/bonding/bond_netlink.c
> +++ b/drivers/net/bonding/bond_netlink.c
> @@ -225,7 +225,12 @@ static int bond_changelink(struct net_device *bond_dev,
>
>   		bond_option_arp_ip_targets_clear(bond);
>   		nla_for_each_nested(attr, data[IFLA_BOND_ARP_IP_TARGET], rem) {
> -			__be32 target = nla_get_be32(attr);
> +			__be32 target;
> +
> +			if (nla_len(attr) < sizeof(target))
> +				return -EINVAL;
> +
> +			target = nla_get_be32(attr);
>
>   			bond_opt_initval(&newval, (__force u64)target);
>   			err = __bond_opt_set(bond, BOND_OPT_ARP_TARGETS,
>


-- 
John Fastabend         Intel Corporation

^ permalink raw reply

* [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call "pci_dev_put"
From: SF Markus Elfring @ 2014-11-29 18:00 UTC (permalink / raw)
  To: Olof Johansson, netdev; +Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 29 Nov 2014 18:55:40 +0100

The pci_dev_put() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call
is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/ethernet/pasemi/pasemi_mac.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.c b/drivers/net/ethernet/pasemi/pasemi_mac.c
index 30d934d..44e8d7d 100644
--- a/drivers/net/ethernet/pasemi/pasemi_mac.c
+++ b/drivers/net/ethernet/pasemi/pasemi_mac.c
@@ -1837,10 +1837,8 @@ pasemi_mac_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return err;
 
 out:
-	if (mac->iob_pdev)
-		pci_dev_put(mac->iob_pdev);
-	if (mac->dma_pdev)
-		pci_dev_put(mac->dma_pdev);
+	pci_dev_put(mac->iob_pdev);
+	pci_dev_put(mac->dma_pdev);
 
 	free_netdev(dev);
 out_disable_device:
-- 
2.1.3

^ permalink raw reply related

* [PATCH net] gre: Set inner mac header in gro complete
From: Tom Herbert @ 2014-11-29 17:59 UTC (permalink / raw)
  To: davem, linux, netdev

Set the inner mac header to point to the GRE payload when
doing GRO. This is needed if we proceed to send the packet
through GRE GSO which now uses the inner mac header instead
of inner network header to determine the length of encapsulation
headers.

Fixes: 14051f0452a2 ("gre: Use inner mac length when computing tunnel length")
Reported-by: Wolfgang Walter <linux@stwm.de>
Signed-off-by: Tom Herbert <therbert@google.com>
---
 net/ipv4/gre_offload.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index bb5947b..51973dd 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -247,6 +247,9 @@ static int gre_gro_complete(struct sk_buff *skb, int nhoff)
 		err = ptype->callbacks.gro_complete(skb, nhoff + grehlen);
 
 	rcu_read_unlock();
+
+	skb_set_inner_mac_header(skb, nhoff + grehlen);
+
 	return err;
 }
 
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* Re: [PATCH] drivers:net:wireless: Add mutex locking for function, b43_op_beacon_set_time in main.c
From: nick @ 2014-11-29 18:09 UTC (permalink / raw)
  To: Rafał Miłecki, Michael Büsch
  Cc: Network Development,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Kernel Mailing List, Stefano Brivio, b43-dev
In-Reply-To: <CACna6ryjZ-LY06wAMzoL77Wca3McUUT+VYsO4EPO87NqjBnaNw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Sorry about that, next time I will be more careful.
Nick

On 2014-11-29 04:04 AM, Rafał Miłecki wrote:
> On 29 November 2014 at 09:56, Michael Büsch <m@bues.ch> wrote:
>> On Fri, 28 Nov 2014 22:32:30 -0500
>> nick <xerofoify@gmail.com> wrote:
>>
>>> I don't have hardware for this driver on me, so I didn't test it. However this seems to
>>> be correct from my reading of the code around this function and other locking related
>>> to this driver.
>>
>> From the current docs:
>>
>>>  * @set_tim: Set TIM bit. mac80211 calls this function when a TIM bit
>>>  *      must be set or cleared for a given STA. Must be atomic.
>>
>> So it is not allowed to lock a mutex here.
> 
> Nicholas please be more careful with your patches. It seems your patch
> is wrong, you didn't check the docs and you didn't even say in the
> first place that it wasn't tested. You could at least submit with with
> RFT tag or sth.
> 

_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply

* Re: [PATCH] drivers:net:wireless: Add mutex locking for function, b43_op_beacon_set_time in main.c
From: Larry Finger @ 2014-11-29 18:54 UTC (permalink / raw)
  To: nick, Rafał Miłecki, Michael Büsch
  Cc: Network Development,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Kernel Mailing List, Stefano Brivio, b43-dev
In-Reply-To: <547A0BC5.2080005-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 11/29/2014 12:09 PM, nick wrote:
> Sorry about that, next time I will be more careful.

One other thing would be to change the initial part of the subject. Yes, the 
device is in drivers/net/wireless/, but it is much more common to not include 
those in patches that are sent to Linville. I would have used "b43: Add ...". 
That catches the eye more quickly. As they say in the newspaper business, "Don't 
bury the lead."

Larry


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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


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