* 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
* [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
* [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] 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
* [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, 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
* 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® Ethernet, visit http://communities.intel.com/community/wired
^ 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 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 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] 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] 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 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: 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: tun issue after e0b46d0ee9c: tun: Use iovec iterators
From: Marcelo Ricardo Leitner @ 2014-11-29 4:49 UTC (permalink / raw)
To: Al Viro; +Cc: herbert, netdev
In-Reply-To: <20141128223503.GE29748@ZenIV.linux.org.uk>
On 28-11-2014 20:35, Al Viro wrote:
> On Fri, Nov 28, 2014 at 08:10:03PM -0200, Marcelo Ricardo Leitner wrote:
>>> Could you print vnet_hdr_sz and sizeof(gso) right after that
>>> copy_from_iter(&gso, ...)?
>>
>> Did a:
>> else {
>> err = skb_copy_datagram_from_iter(skb, 0, from, len);
>> + pr_err("vnet_hdr_sz=%d sizeof(gso)=%lu\n",
>> tun->vnet_hdr_sz, sizeof(gso));
>> if (!err && msg_control) {
>>
>> Got, for tun:
>> [ 50.514165] tun: vnet_hdr_sz=12 sizeof(gso)=10
>>
>> for tap:
>> [ 82.911840] tun: vnet_hdr_sz=10 sizeof(gso)=10
>>
>> other values were just as before.
>
> Hmm... Do you have
> commit 8c847d254146d32c86574a1b16923ff91bb784dd
> Author: Jason Wang <jasowang@redhat.com>
> Date: Thu Nov 13 16:54:14 2014 +0800
>
> tun: fix issues of iovec iterators using in tun_put_user()
> in your tree?
>
Yes I have. I'm using based on net-next's
799d2fff1858004526ad75d66a5dd8a5cce6ad40.
Marcelo
^ permalink raw reply
* Re: [PATCH] drivers:net:wireless: Add mutex locking for function, b43_op_beacon_set_time in main.c
From: nick @ 2014-11-29 3:32 UTC (permalink / raw)
To: Michael Büsch, Rafał Miłecki
Cc: Network Development,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Kernel Mailing List, Stefano Brivio, b43-dev
In-Reply-To: <20141129002148.1beb21d9@wiggum>
Michael,
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.
Cheers Nick
On 2014-11-28 06:21 PM, Michael Büsch wrote:
> On Fri, 28 Nov 2014 23:40:46 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>>> @@ -5094,8 +5094,9 @@ static int b43_op_beacon_set_tim(struct ieee80211_hw *hw,
>>> {
>>> struct b43_wl *wl = hw_to_b43_wl(hw);
>>>
>>> - /* FIXME: add locking */
>>> + mutex_lock(&wl->mutex);
>>> b43_update_templates(wl);
>>> + mutex_unlock(&wl->mutex);
>>>
>>> return 0;
>>> }
>>
>> Does anyone remember why this simple solution wasn't implemented
>> earlier? Michael?
>
> I think the callback used to be (is?) in atomic context.
>
>> Nicholas: did you test it anyhow?
>
_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev
^ permalink raw reply
* Re: tun issue after e0b46d0ee9c: tun: Use iovec iterators
From: Herbert Xu @ 2014-11-28 23:59 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: netdev
In-Reply-To: <5478CC27.9040705@redhat.com>
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?
-- >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,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [PATCH] e1000: remove unused variables
From: Florian Fainelli @ 2014-11-28 23:28 UTC (permalink / raw)
To: Sudip Mukherjee, Hisashi T Fujinaka
Cc: Linux NICS, e1000-devel, Bruce Allan, Jesse Brandeburg,
linux-kernel, John Ronciak, netdev
In-Reply-To: <20141127130725.GA12406@sudip-PC>
Le 27/11/2014 05:07, Sudip Mukherjee a écrit :
> On Wed, Nov 26, 2014 at 09:59:28PM -0800, Hisashi T Fujinaka wrote:
>> I'm pretty sure those double reads are there for a reason, so most of
>> this I'm going to have to check on Monday. We have a long holiday
>> weekend here in the US.
>
> if the double reads are there for some reason, can you please let me know what that reason might be..
Could be latching, especially in the context of reading from Ethernet
PHYs, some registers are latched, so you may have to do a double read to
ensure the value you get is consistent.
Also, if you do a read that is not stored in any return value, the
compiler is now free to remove that actual read, and that may have other
side effects for registers which are e.g: read to clear, or any of the like.
>
>>
>> I'm not sure why you're bothering with an old driver like this, but if
>> you haven't actually tried this on all the hardware it pertains to, I'm
>> going want to NAK this.
>
> no it has not been tested on hardware. :(
>
> i am still in the learning process, NAK is also part of learning.
>
> infact there is another part of the code, which, theoretically, will never get executed. but i didnot dare to send that removal patch without testing on the hardware.
>
> thanks
> sudip
>
>>
>> I should do this from my todd.fujinaka@intel.com account but it's 10PM
>> on the first day of a long holiday weekend.
>>
>> On Thu, 27 Nov 2014, Sudip Mukherjee wrote:
>>
>>> these variables were only being assigned some values, but were never
>>> used.
>>>
>>> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
>>> ---
>>> drivers/net/ethernet/intel/e1000/e1000_hw.c | 142 ++++++++++++--------------
> <snip>
>>> case SPEED_100:
>>> - txb2b = false;
>>> /* maybe add some timeout factor ? */
>>> break;
>>> }
>>>
>>
>> --
>> Hisashi T Fujinaka - htodd@twofifty.com
>> BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
------------------------------------------------------------------------------
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® Ethernet, visit http://communities.intel.com/community/wired
^ 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-28 23:21 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Nicholas Krause, Stefano Brivio, Network Development,
linux-wireless@vger.kernel.org, Linux Kernel Mailing List,
b43-dev
In-Reply-To: <CACna6rxmOC0h9hOpa=5iWFM1THQ8Q4hCi8MnmqkZJbB_U0PJ0A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 627 bytes --]
On Fri, 28 Nov 2014 23:40:46 +0100
Rafał Miłecki <zajec5@gmail.com> wrote:
> > @@ -5094,8 +5094,9 @@ static int b43_op_beacon_set_tim(struct ieee80211_hw *hw,
> > {
> > struct b43_wl *wl = hw_to_b43_wl(hw);
> >
> > - /* FIXME: add locking */
> > + mutex_lock(&wl->mutex);
> > b43_update_templates(wl);
> > + mutex_unlock(&wl->mutex);
> >
> > return 0;
> > }
>
> Does anyone remember why this simple solution wasn't implemented
> earlier? Michael?
I think the callback used to be (is?) in atomic context.
> Nicholas: did you test it anyhow?
--
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-28 22:40 UTC (permalink / raw)
To: Nicholas Krause, Michael Büsch
Cc: Stefano Brivio, Network Development,
linux-wireless@vger.kernel.org, Linux Kernel Mailing List,
b43-dev
In-Reply-To: <1417213013-19758-1-git-send-email-xerofoify@gmail.com>
On 28 November 2014 at 23:16, Nicholas Krause <xerofoify@gmail.com> wrote:
> Adds needed mutex lockng of wl->mutex in order to prevent issues with separate threads executing on
> the b43_update_templates function call in the function, b43_op_beacon_set_time at the same time.
For all kind of kernel documentation / descriptions we try to fit 72
chars limit and we really try to avoid exceeding 80 chars.
> @@ -5094,8 +5094,9 @@ static int b43_op_beacon_set_tim(struct ieee80211_hw *hw,
> {
> struct b43_wl *wl = hw_to_b43_wl(hw);
>
> - /* FIXME: add locking */
> + mutex_lock(&wl->mutex);
> b43_update_templates(wl);
> + mutex_unlock(&wl->mutex);
>
> return 0;
> }
Does anyone remember why this simple solution wasn't implemented
earlier? Michael?
Nicholas: did you test it anyhow?
^ permalink raw reply
* Re: tun issue after e0b46d0ee9c: tun: Use iovec iterators
From: Al Viro @ 2014-11-28 22:35 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: herbert, netdev
In-Reply-To: <5478F2BB.9050409@redhat.com>
On Fri, Nov 28, 2014 at 08:10:03PM -0200, Marcelo Ricardo Leitner wrote:
> >Could you print vnet_hdr_sz and sizeof(gso) right after that
> >copy_from_iter(&gso, ...)?
>
> Did a:
> else {
> err = skb_copy_datagram_from_iter(skb, 0, from, len);
> + pr_err("vnet_hdr_sz=%d sizeof(gso)=%lu\n",
> tun->vnet_hdr_sz, sizeof(gso));
> if (!err && msg_control) {
>
> Got, for tun:
> [ 50.514165] tun: vnet_hdr_sz=12 sizeof(gso)=10
>
> for tap:
> [ 82.911840] tun: vnet_hdr_sz=10 sizeof(gso)=10
>
> other values were just as before.
Hmm... Do you have
commit 8c847d254146d32c86574a1b16923ff91bb784dd
Author: Jason Wang <jasowang@redhat.com>
Date: Thu Nov 13 16:54:14 2014 +0800
tun: fix issues of iovec iterators using in tun_put_user()
in your tree?
^ permalink raw reply
* [PATCH] drivers:net:wireless: Add mutex locking for function, b43_op_beacon_set_time in main.c
From: Nicholas Krause @ 2014-11-28 22:16 UTC (permalink / raw)
To: stefano.brivio-hl5o88x/ua9eoWH0uzbU5w
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Adds needed mutex lockng of wl->mutex in order to prevent issues with separate threads executing on
the b43_update_templates function call in the function, b43_op_beacon_set_time at the same time.
Signed-off-by: Nicholas Krause <xerofoify-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/net/wireless/b43/main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index 5d4173e..19ddc72 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -5094,8 +5094,9 @@ static int b43_op_beacon_set_tim(struct ieee80211_hw *hw,
{
struct b43_wl *wl = hw_to_b43_wl(hw);
- /* FIXME: add locking */
+ mutex_lock(&wl->mutex);
b43_update_templates(wl);
+ mutex_unlock(&wl->mutex);
return 0;
}
--
2.1.0
^ permalink raw reply related
* Re: tun issue after e0b46d0ee9c: tun: Use iovec iterators
From: Marcelo Ricardo Leitner @ 2014-11-28 22:10 UTC (permalink / raw)
To: Al Viro; +Cc: herbert, netdev
In-Reply-To: <20141128203733.GD29748@ZenIV.linux.org.uk>
On 28-11-2014 18:37, Al Viro wrote:
> On Fri, Nov 28, 2014 at 05:25:27PM -0200, Marcelo Ricardo Leitner wrote:
>> Hi,
>>
>> 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:
>
> Umm... In host, presumably?
>
Exactly
>> And net/core/datagram.c, skb_copy_datagram_from_iter():
>> if (copy > 0) {
>> int ret;
>> if (copy > len)
>> copy = len;
>> ret = copy_from_iter(skb->data + offset, copy, from);
>> if (ret != copy) {
>> pr_err("%d ret=%d copy=%d offset=%d
>> len=%d\n", __LINE__, ret, copy, offset, len);
>> goto fault;
>> }
>>
>>
>> I get, for tun interfaces:
>> [ 75.435552] 506 ret=80 copy=90 offset=0 len=90
>> [ 75.435563] tun: 1124 0 -14 (null)
>> [ 75.499528] 506 ret=80 copy=90 offset=0 len=90
>> [ 75.499540] tun: 1124 0 -14 (null)
>>
>> These were 1 drop on 1 interface each
>>
>> And for tap interfaces:
>> [ 301.982639] 506 ret=80 copy=90 offset=0 len=90
>> [ 301.982649] tun: 1124 0 -14 (null)
>> [ 301.988625] 506 ret=80 copy=90 offset=0 len=90
>> [ 301.988635] tun: 1124 0 -14 (null)
>> [ 301.994762] 506 ret=80 copy=90 offset=0 len=90
>> [ 301.994773] tun: 1124 0 -14 (null)
>> [ 302.229962] 506 ret=332 copy=342 offset=0 len=342
>> [ 302.229972] tun: 1124 0 -14 (null)
>> [ 302.230621] 506 ret=332 copy=342 offset=0 len=342
>> [ 302.230627] tun: 1124 0 -14 (null)
>> [ 302.239065] 506 ret=332 copy=342 offset=0 len=342
>> [ 302.239071] tun: 1124 0 -14 (null)
>>
>> It's returning 10 bytes less than the expected... ideas?
>
> Could you print vnet_hdr_sz and sizeof(gso) right after that
> copy_from_iter(&gso, ...)?
Did a:
else {
err = skb_copy_datagram_from_iter(skb, 0, from, len);
+ pr_err("vnet_hdr_sz=%d sizeof(gso)=%lu\n", tun->vnet_hdr_sz,
sizeof(gso));
if (!err && msg_control) {
Got, for tun:
[ 50.514165] tun: vnet_hdr_sz=12 sizeof(gso)=10
for tap:
[ 82.911840] tun: vnet_hdr_sz=10 sizeof(gso)=10
other values were just as before.
Marcelo
^ permalink raw reply
* Fix me in htc.c relating to skb_reserve
From: nick @ 2014-11-28 21:43 UTC (permalink / raw)
To: kvalo; +Cc: netdev, linux-wireless, linville, ath10k, linux-kernel
Greetings Kalle, John and others,
I am wondering why this file is complaining about the code below, if someone can explain to me the issue I would be
very glad :).
static struct sk_buff *ath10k_htc_build_tx_ctrl_skb(void *ar)
{
struct sk_buff *skb;
struct ath10k_skb_cb *skb_cb;
skb = dev_alloc_skb(ATH10K_HTC_CONTROL_BUFFER_SIZE);
if (!skb)
return NULL;
skb_reserve(skb, 20); /* FIXME: why 20 bytes? */
WARN_ONCE((unsigned long)skb->data & 3, "unaligned skb");
skb_cb = ATH10K_SKB_CB(skb);
memset(skb_cb, 0, sizeof(*skb_cb));
ath10k_dbg(ar, ATH10K_DBG_HTC, "%s: skb %p\n", __func__, skb);
return skb;
}
^ permalink raw reply
* Re: tun issue after e0b46d0ee9c: tun: Use iovec iterators
From: Al Viro @ 2014-11-28 20:37 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: herbert, netdev
In-Reply-To: <5478CC27.9040705@redhat.com>
On Fri, Nov 28, 2014 at 05:25:27PM -0200, Marcelo Ricardo Leitner wrote:
> Hi,
>
> 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:
Umm... In host, presumably?
> And net/core/datagram.c, skb_copy_datagram_from_iter():
> if (copy > 0) {
> int ret;
> if (copy > len)
> copy = len;
> ret = copy_from_iter(skb->data + offset, copy, from);
> if (ret != copy) {
> pr_err("%d ret=%d copy=%d offset=%d
> len=%d\n", __LINE__, ret, copy, offset, len);
> goto fault;
> }
>
>
> I get, for tun interfaces:
> [ 75.435552] 506 ret=80 copy=90 offset=0 len=90
> [ 75.435563] tun: 1124 0 -14 (null)
> [ 75.499528] 506 ret=80 copy=90 offset=0 len=90
> [ 75.499540] tun: 1124 0 -14 (null)
>
> These were 1 drop on 1 interface each
>
> And for tap interfaces:
> [ 301.982639] 506 ret=80 copy=90 offset=0 len=90
> [ 301.982649] tun: 1124 0 -14 (null)
> [ 301.988625] 506 ret=80 copy=90 offset=0 len=90
> [ 301.988635] tun: 1124 0 -14 (null)
> [ 301.994762] 506 ret=80 copy=90 offset=0 len=90
> [ 301.994773] tun: 1124 0 -14 (null)
> [ 302.229962] 506 ret=332 copy=342 offset=0 len=342
> [ 302.229972] tun: 1124 0 -14 (null)
> [ 302.230621] 506 ret=332 copy=342 offset=0 len=342
> [ 302.230627] tun: 1124 0 -14 (null)
> [ 302.239065] 506 ret=332 copy=342 offset=0 len=342
> [ 302.239071] tun: 1124 0 -14 (null)
>
> It's returning 10 bytes less than the expected... ideas?
Could you print vnet_hdr_sz and sizeof(gso) right after that
copy_from_iter(&gso, ...)?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox