* Re: [PATCH RFC 3/6] The file ksz_common.c contains common code shared by all KSZ switch drivers
From: Andrew Lunn @ 2017-09-08 1:12 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A0D@CHN-SV-EXMX02.mchp-main.com>
Hi Tristram
You patches are so badly mangled that they won't apply. So i'm
guessing a bit here...
What i think is you can do this in three patches:
1) Straight rename of ksz_common.c to ksz9477.c. Makefile changes to
fit, no code changes.
2) Move common code back into ksz_common.c. Makefile changes to fit.
3) ksz9477.c prefix name changes
It looks like there is not too much common code in ksz_common.c. So
doing it this way makes the changes minimal, and things compile at
each stage.
Remember to use -M with git format-patch.
Andrew
^ permalink raw reply
* Re: Use after free in __dst_destroy_metrics_generic
From: Stefano Brivio @ 2017-09-08 0:56 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan; +Cc: netdev, eric.dumazet, lorenzo
In-Reply-To: <1234e09b75197d43ed84bdb1b154b4b6@codeaurora.org>
On Thu, 07 Sep 2017 18:52:02 -0600
Subash Abhinov Kasiviswanathan <subashab@codeaurora.org> wrote:
> We are seeing a possible use after free in ip6_dst_destroy.
>
> It appears as if memory of the __DST_METRICS_PTR(old) was freed in some
> path and allocated
> to ion driver. ion driver has also freed it. Finally the memory is freed
> by the
> fib gc and crashes since it is already deallocated.
>
> Target is running an ARM64 Android based 4.9 kernel.
> Issue was seen once on a regression rack (sorry, no reproducer).
> Any pointers to debug this is highly appreciated.
>
> [ 3489.470581] [<ffffff83c0a289c0>] object_err+0x4c/0x5c
> [ 3489.470586] [<ffffff83c0a2b284>] free_debug_processing+0x2e0/0x398
> [ 3489.470589] [<ffffff83c0a2b63c>] __slab_free+0x300/0x3e0
> [ 3489.470593] [<ffffff83c0a2bfc8>] kfree+0x28c/0x290
> [ 3489.470601] [<ffffff83c16b9580>]
> __dst_destroy_metrics_generic+0x6c/0x78
> [ 3489.470609] [<ffffff83c17d3408>] ip6_dst_destroy+0xb0/0xb4
Should be fixed by:
commit ad65a2f05695aced349e308193c6e2a6b1d87112
Author: Wei Wang <weiwan@google.com>
Date: Sat Jun 17 10:42:35 2017 -0700
ipv6: call dst_hold_safe() properly
^ permalink raw reply
* Use after free in __dst_destroy_metrics_generic
From: Subash Abhinov Kasiviswanathan @ 2017-09-08 0:52 UTC (permalink / raw)
To: netdev; +Cc: eric.dumazet, lorenzo
We are seeing a possible use after free in ip6_dst_destroy.
It appears as if memory of the __DST_METRICS_PTR(old) was freed in some
path and allocated
to ion driver. ion driver has also freed it. Finally the memory is freed
by the
fib gc and crashes since it is already deallocated.
Target is running an ARM64 Android based 4.9 kernel.
Issue was seen once on a regression rack (sorry, no reproducer).
Any pointers to debug this is highly appreciated.
[ 3489.470581] [<ffffff83c0a289c0>] object_err+0x4c/0x5c
[ 3489.470586] [<ffffff83c0a2b284>] free_debug_processing+0x2e0/0x398
[ 3489.470589] [<ffffff83c0a2b63c>] __slab_free+0x300/0x3e0
[ 3489.470593] [<ffffff83c0a2bfc8>] kfree+0x28c/0x290
[ 3489.470601] [<ffffff83c16b9580>]
__dst_destroy_metrics_generic+0x6c/0x78
[ 3489.470609] [<ffffff83c17d3408>] ip6_dst_destroy+0xb0/0xb4
[ 3489.470612] [<ffffff83c16b9714>] dst_destroy+0x88/0x174
[ 3489.470616] [<ffffff83c17d7f64>] icmp6_dst_gc+0x90/0xc0
[ 3489.470621] [<ffffff83c17db52c>] fib6_gc_timer_cb+0x40/0xc0
[ 3489.470630] [<ffffff83c093aef4>] call_timer_fn+0x58/0x1d0
[ 3489.470635] [<ffffff83c093b198>] expire_timers+0x100/0x18c
[ 3489.470638] [<ffffff83c093b2bc>] run_timer_softirq+0x98/0x270
[ 3489.470642] [<ffffff83c0881a00>] __do_softirq+0x150/0x438
[ 3489.470649] [<ffffff83c08af59c>] irq_exit+0xe0/0x138
[ 3489.127227]
=============================================================================
[ 3489.135489] BUG kmalloc-128 (Tainted: G W O ): Object
already free
[ 3489.142591]
-----------------------------------------------------------------------------
[ 3489.142591]
[ 3489.152313] Disabling lock debugging due to kernel taint
[ 3489.157688] INFO: Allocated in alloc_largest_available+0x58/0x1f0
age=17 cpu=4 pid=649
[ 3489.165667] alloc_debug_processing+0x114/0x1a0
[ 3489.170233] ___slab_alloc.constprop.72+0x654/0x690
[ 3489.175150] __slab_alloc.isra.68.constprop.71+0x48/0x80
[ 3489.180505] kmem_cache_alloc_trace+0x198/0x2c4
[ 3489.185073] alloc_largest_available+0x58/0x1f0
[ 3489.189643] ion_system_heap_allocate+0x1b0/0x6e8
[ 3489.194392] __ion_alloc+0x180/0x988
[ 3489.198004] ion_ioctl+0x26c/0x590
[ 3489.201437] do_vfs_ioctl+0xcc/0x888
[ 3489.205037] SyS_ioctl+0x90/0xa4
[ 3489.208298] el0_svc_naked+0x24/0x28
[ 3489.211909] INFO: Freed in process_info+0x188/0x1bc age=21 cpu=4
pid=649
[ 3489.218661] free_debug_processing+0x180/0x398
[ 3489.223137] __slab_free+0x300/0x3e0
[ 3489.226745] kfree+0x28c/0x290
[ 3489.229827] process_info+0x188/0x1bc
[ 3489.233526] ion_system_heap_allocate+0x4b4/0x6e8
[ 3489.238266] __ion_alloc+0x180/0x988
[ 3489.241875] ion_ioctl+0x26c/0x590
[ 3489.245308] do_vfs_ioctl+0xcc/0x888
[ 3489.248917] SyS_ioctl+0x90/0xa4
[ 3489.252171] el0_svc_naked+0x24/0x28
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-09-08 0:47 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Reinis Rozitis
In-Reply-To: <CAF2d9jhxrpmTEqHtf6NES4LmoT8A5O1v==8NJZcDGhS-PS5AFQ@mail.gmail.com>
On Thu, Sep 7, 2017 at 5:39 PM, Mahesh Bandewar (महेश बंडेवार)
<maheshb@google.com> wrote:
> On Thu, Sep 7, 2017 at 4:09 PM, Nikolay Aleksandrov
> <nikolay@cumulusnetworks.com> wrote:
>> On 7.09.2017 01:47, Kosuke Tatsukawa wrote:
>>> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
>>> balance-alb mode") tried to fix transmit dynamic load balancing in
>>> balance-alb mode, which wasn't working after commit 8b426dc54cf4
>>> ("bonding: remove hardcoded value").
>>>
>>> It turned out that my previous patch only fixed the case when
>>> balance-alb was specified as bonding module parameter, and not when
>>> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
>>> common usage). In the latter case, tlb_dynamic_lb was set up according
>>> to the default mode of the bonding interface, which happens to be
>>> balance-rr.
>>>
>>> This additional patch addresses this issue by setting up tlb_dynamic_lb
>>> to 1 if "mode" is set to balance-alb through the sysfs interface.
>>>
>>> I didn't add code to change tlb_balance_lb back to the default value for
>>> other modes, because "mode" is usually set up only once during
>>> initialization, and it's not worthwhile to change the static variable
>>> bonding_defaults in bond_main.c to a global variable just for this
>>> purpose.
>>>
>>> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
>>> balance-tlb mode if it is set up using the sysfs interface. I didn't
>>> change that behavior, because the value of tlb_balance_lb can be changed
>>> using the sysfs interface for balance-tlb, and I didn't like changing
>>> the default value back and forth for balance-tlb.
>>>
>>> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
>>> written to. However, I think balance-alb with tlb_dynamic_lb set to 0
>>> is not an intended usage, so there is little use making it writable at
>>> this moment.
>>>
>>> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
>
> This is wrong! I think you are confusing various things here. ALB is
> different mode from TLB and TLB-dynamic-lb is *only* a special case of
> TLB. Your earlier patch is also wrong for the same reasons. However,
> since the default value of tlb_dynamic_lb is set to 0 it's not
> causing issues for ALB mode otherwise it would break ALB mode.
I take this back. The default value is 1 so ALB is broken because of
the referenced change.
> tlb_dynamic_lb has absolutely nothing to do with ALB mode. Please
> revert the earlier change (cbf5ecb30560).
>
> It's not clear to me what you saw as broken, so can't really suggest
> what really need to be done.
^ permalink raw reply
* Re: [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in ksz_common.c
From: Andrew Lunn @ 2017-09-08 0:46 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A06@CHN-SV-EXMX02.mchp-main.com>
> +static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
> + bool set)
Hi Tristram
As you move these functions around, it would be good to change the
prefix to ksz9477_
You have similar functions in the KSZ8795 driver. It is good to keep
the names different. This is useful when looking at stack dumps, etc.
Andrew
^ permalink raw reply
* Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.
From: Andrew Lunn @ 2017-09-08 0:40 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411219FF@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 07, 2017 at 09:08:58PM +0000, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
Hi Tristram
Another process thing you are missing. Patch subject should follow a
pattern:
git log --oneline drivers/net/dsa/mv88e6xxx
bb0a2675f72b net: dsa: mv88e6xxx: Enable CMODE config support for 6390X
b3e05aa12319 net: dsa: mv88e6xxx: add a multi_chip info flag
68b8f60cf70d net: dsa: mv88e6xxx: add Energy Detect ops
9069c13a4867 net: dsa: mv88e6xxx: add a global2_addr info flag
9e907d739cc3 net: dsa: mv88e6xxx: add POT operation
a2a05db8a5ed net: dsa: mv88e6xxx: add POT flag to 88E6390
51c901a77562 net: dsa: mv88e6xxx: distinguish Global 2 Rsvd2CPU
d6c5e6aff50c net: dsa: mv88e6xxx: add number of Global 2 IRQs
74e60241ce14 net: dsa: mv88e6xxx: remove 88E6185 G2 interrupt
2466f64ae4e9 net: dsa: mv88e6xxx: remove unused capabilities
Please set the subject line in your patches similarly.
Andrew
^ permalink raw reply
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-09-08 0:39 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Reinis Rozitis
In-Reply-To: <c9b6ae59-741b-3ecd-47ff-1d5b41710607@cumulusnetworks.com>
On Thu, Sep 7, 2017 at 4:09 PM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On 7.09.2017 01:47, Kosuke Tatsukawa wrote:
>> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
>> balance-alb mode") tried to fix transmit dynamic load balancing in
>> balance-alb mode, which wasn't working after commit 8b426dc54cf4
>> ("bonding: remove hardcoded value").
>>
>> It turned out that my previous patch only fixed the case when
>> balance-alb was specified as bonding module parameter, and not when
>> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
>> common usage). In the latter case, tlb_dynamic_lb was set up according
>> to the default mode of the bonding interface, which happens to be
>> balance-rr.
>>
>> This additional patch addresses this issue by setting up tlb_dynamic_lb
>> to 1 if "mode" is set to balance-alb through the sysfs interface.
>>
>> I didn't add code to change tlb_balance_lb back to the default value for
>> other modes, because "mode" is usually set up only once during
>> initialization, and it's not worthwhile to change the static variable
>> bonding_defaults in bond_main.c to a global variable just for this
>> purpose.
>>
>> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
>> balance-tlb mode if it is set up using the sysfs interface. I didn't
>> change that behavior, because the value of tlb_balance_lb can be changed
>> using the sysfs interface for balance-tlb, and I didn't like changing
>> the default value back and forth for balance-tlb.
>>
>> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
>> written to. However, I think balance-alb with tlb_dynamic_lb set to 0
>> is not an intended usage, so there is little use making it writable at
>> this moment.
>>
>> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
This is wrong! I think you are confusing various things here. ALB is
different mode from TLB and TLB-dynamic-lb is *only* a special case of
TLB. Your earlier patch is also wrong for the same reasons. However,
since the default value of tlb_dynamic_lb is set to 0 it's not
causing issues for ALB mode otherwise it would break ALB mode.
tlb_dynamic_lb has absolutely nothing to do with ALB mode. Please
revert the earlier change (cbf5ecb30560).
It's not clear to me what you saw as broken, so can't really suggest
what really need to be done.
^ permalink raw reply
* ATENCIÓN;
From: administrador @ 2017-09-07 11:11 UTC (permalink / raw)
To: Recipients
ATENCIÓN;
Su buzón ha superado el límite de almacenamiento, que es de 5 GB definidos por el administrador, quien actualmente está ejecutando en 10.9GB, no puede ser capaz de enviar o recibir correo nuevo hasta que vuelva a validar su buzón de correo electrónico. Para revalidar su buzón de correo, envíe la siguiente información a continuación:
nombre:
Nombre de usuario:
contraseña:
Confirmar contraseña:
E-mail:
teléfono:
Si usted no puede revalidar su buzón, el buzón se deshabilitará!
Disculpa las molestias.
Código de verificación: es: 006524
Correo Soporte Técnico © 2016
¡gracias
Sistemas administrador
^ permalink raw reply
* Re: WARN_ON() in __nf_hook_entries_try_shrink()
From: Stefano Brivio @ 2017-09-08 0:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: Aaron Conole, Florian Westphal, Pablo Neira Ayuso,
Jozsef Kadlecsik, David Miller, NetFilter, coreteam,
Network Development
In-Reply-To: <CA+55aFw64ihu1bai0Em8pXD4dZ-wFVtV7BRjWbDLLb2MPWWXkw@mail.gmail.com>
On Thu, 7 Sep 2017 17:01:12 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On shutdown I get this (edited down a bit to be more legible):
>
> Stopping firewalld - dynamic firewall daemon...
> NETFILTER_CFG table=nat family=2 entries=55
> NETFILTER_CFG table=mangle family=2 entries=40
> NETFILTER_CFG table=raw family=2 entries=28
> NETFILTER_CFG table=security family=2 entries=13
> NETFILTER_CFG table=filter family=2 entries=93
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 21512 at net/netfilter/core.c:218
> __nf_hook_entries_try_shrink+0x106/0x130
Should be fixed by:
http://patchwork.ozlabs.org/patch/810570/
^ permalink raw reply
* WARN_ON() in __nf_hook_entries_try_shrink()
From: Linus Torvalds @ 2017-09-08 0:01 UTC (permalink / raw)
To: Aaron Conole, Florian Westphal, Pablo Neira Ayuso
Cc: Jozsef Kadlecsik, David Miller, NetFilter, coreteam,
Network Development
On shutdown I get this (edited down a bit to be more legible):
Stopping firewalld - dynamic firewall daemon...
NETFILTER_CFG table=nat family=2 entries=55
NETFILTER_CFG table=mangle family=2 entries=40
NETFILTER_CFG table=raw family=2 entries=28
NETFILTER_CFG table=security family=2 entries=13
NETFILTER_CFG table=filter family=2 entries=93
------------[ cut here ]------------
WARNING: CPU: 1 PID: 21512 at net/netfilter/core.c:218
__nf_hook_entries_try_shrink+0x106/0x130
CPU: 1 PID: 21512 Comm: iptables-restor Not tainted
4.13.0-08555-gc0da4fa0d1a5 #7
Hardware name: Dell Inc. XPS 13 9350/09JHRY, BIOS 1.4.17 05/10/2017
RIP: 0010:__nf_hook_entries_try_shrink+0x106/0x130
Call Trace:
nf_unregister_net_hooks+0x117/0x240
ipv4_hooks_unregister+0x60/0x70 [nf_conntrack_ipv4]
nf_ct_netns_put+0x48/0x80 [nf_conntrack]
conntrack_mt_destroy+0x15/0x20 [xt_conntrack]
cleanup_match+0x43/0x70
cleanup_entry+0x42/0xc0
__do_replace+0x17a/0x1f0
do_ipt_set_ctl+0x146/0x1b0
nf_setsockopt+0x46/0x80
ip_setsockopt+0x82/0xb0
raw_setsockopt+0x34/0x40
sock_common_setsockopt+0x14/0x20
SyS_setsockopt+0x80/0xe0
entry_SYSCALL_64_fastpath+0x13/0x94
[ .. warning repeats a few times .. ]
---[ end trace 56a6f5b20d97161d ]---
NETFILTER_CFG table=broute family=7 entries=0
NETFILTER_CFG table=nat family=7 entries=0
NETFILTER_CFG table=filter family=7 entries=0
NETFILTER_CFG table=mangle family=2 entries=6
and some searching notes that the kernel test robot already reported
this a few days ago but nobody reacted.
The kernel test robot seems to blame commit d3ad2c17b404 ("netfilter:
core: batch nf_unregister_net_hooks synchronize_net calls").
Hmm?
Linus
^ permalink raw reply
* [PATCH] rtl8xxxu: Don't printk raw binary if serial number is not burned in.
From: Adam Borowski @ 2017-09-07 23:51 UTC (permalink / raw)
To: Jes Sorensen, Kalle Valo, linux-wireless, netdev; +Cc: Adam Borowski
I assume that a blank efuse comes with all ones, thus I did not bother
recognizing other possible junk values. This matches 100% of dongles
I've seen (a single Gembird 8192eu).
Signed-off-by: Adam Borowski <kilobyte@angband.pl>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index 80fee699f58a..bdc37e7272ca 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -614,7 +614,11 @@ static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv *priv)
dev_info(&priv->udev->dev, "Vendor: %.7s\n", efuse->vendor_name);
dev_info(&priv->udev->dev, "Product: %.11s\n", efuse->device_name);
- dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
+ if (strncmp(efuse->serial,
+ "\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff", 11))
+ dev_info(&priv->udev->dev, "Serial: %.11s\n", efuse->serial);
+ else
+ dev_info(&priv->udev->dev, "Serial not available.\n");
if (rtl8xxxu_debug & RTL8XXXU_DEBUG_EFUSE) {
unsigned char *raw = priv->efuse_wifi.raw;
--
2.14.1
^ permalink raw reply related
* Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs
From: Nikolay Aleksandrov @ 2017-09-07 23:09 UTC (permalink / raw)
To: Kosuke Tatsukawa, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Reinis Rozitis
In-Reply-To: <17EC94B0A072C34B8DCF0D30AD16044A02985CB5@BPXM09GP.gisp.nec.co.jp>
On 7.09.2017 01:47, Kosuke Tatsukawa wrote:
> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
> balance-alb mode") tried to fix transmit dynamic load balancing in
> balance-alb mode, which wasn't working after commit 8b426dc54cf4
> ("bonding: remove hardcoded value").
>
> It turned out that my previous patch only fixed the case when
> balance-alb was specified as bonding module parameter, and not when
> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
> common usage). In the latter case, tlb_dynamic_lb was set up according
> to the default mode of the bonding interface, which happens to be
> balance-rr.
>
> This additional patch addresses this issue by setting up tlb_dynamic_lb
> to 1 if "mode" is set to balance-alb through the sysfs interface.
>
> I didn't add code to change tlb_balance_lb back to the default value for
> other modes, because "mode" is usually set up only once during
> initialization, and it's not worthwhile to change the static variable
> bonding_defaults in bond_main.c to a global variable just for this
> purpose.
>
> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
> balance-tlb mode if it is set up using the sysfs interface. I didn't
> change that behavior, because the value of tlb_balance_lb can be changed
> using the sysfs interface for balance-tlb, and I didn't like changing
> the default value back and forth for balance-tlb.
>
> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
> written to. However, I think balance-alb with tlb_dynamic_lb set to 0
> is not an intended usage, so there is little use making it writable at
> this moment.
>
> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
> Reported-by: Reinis Rozitis <r@roze.lv>
> Signed-off-by: Kosuke Tatsukawa <tatsu@ab.jp.nec.com>
> Cc: stable@vger.kernel.org # v4.12+
> ---
> drivers/net/bonding/bond_options.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
I don't believe this to be the right solution, hardcoding it like this
changes user-visible behaviour. The issue is that if someone configured
it to be 0 in tlb mode, suddenly it will become 1 and will silently
override their config if they switch the mode to alb. Also it robs users
from their choice.
If you think this should be settable in ALB mode, then IMO you should
edit tlb_dynamic_lb option's unsuppmodes and allow it to be set in ALB.
That would also be consistent with how it's handled in TLB mode.
Going back and looking at your previous fix I'd argue that it is also
wrong, you should've removed the mode check altogether to return the
original behaviour where the dynamic_lb is set to 1 (enabled) by
default and then ALB mode would've had it, of course that would've left
the case of setting it to 0 in TLB mode and switching to ALB, but that
is a different issue.
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
From: Andrew Lunn @ 2017-09-07 22:43 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121B0A@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 07, 2017 at 10:29:34PM +0000, Tristram.Ha@microchip.com wrote:
> > -----Original Message-----
> > From: Andrew Lunn [mailto:andrew@lunn.ch]
> > Sent: Thursday, September 07, 2017 2:56 PM
> > To: Tristram Ha - C24268
> > Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> > vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> > Subject: Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
> >
> > On Thu, Sep 07, 2017 at 09:17:10PM +0000, Tristram.Ha@microchip.com wrote:
> > > From: Tristram Ha <Tristram.Ha@microchip.com>
> > >
> > > Add KSZ8795 switch support with SPI access.
> > >
> > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > > ---
> > > diff --git a/drivers/net/dsa/microchip/Makefile
> > b/drivers/net/dsa/microchip/Makefile
> > > index 0961c30..0d8ba48 100644
> > > --- a/drivers/net/dsa/microchip/Makefile
> > > +++ b/drivers/net/dsa/microchip/Makefile
> > > @@ -1,2 +1,4 @@
> > > obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> > > obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ8795) += ksz8795.o ksz_common.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
> >
> > I've not tried it, but i think this breaks the build
> >
> > Andrew
>
> So you would like to have all 5 patches in 1 patch file?
Or maybe this one last? Would that stop the build from breaking?
Andrew
^ permalink raw reply
* Re: [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in ksz_common.c
From: Andrew Lunn @ 2017-09-07 22:41 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121AEB@CHN-SV-EXMX02.mchp-main.com>
> Sorry about that. It seems my e-mail system wraps the line too soon.
git send-email should solve your problems.
Andrew
^ permalink raw reply
* Re: latest netdev net-next kernel panic
From: Paweł Staszewski @ 2017-09-07 22:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Network Developers
In-Reply-To: <91ae50fc-300a-2b92-7de0-ae512f364280@itcare.pl>
What is weird - changed in bios and enabled p-states and no panic from
30 minutes
w
00:38:07 up 33 min, 2 users, load average: 0.10, 0.13, 0.17
USER TTY LOGIN@ IDLE JCPU PCPU WHAT
So the problem can be related to some cpu weird stuff with c/p states or
it is random.
W dniu 2017-09-08 o 00:08, Paweł Staszewski pisze:
> Also this panic occured about 3 minutes after traffic rise to 40G
>
> before host was working without trafic for about 30minutes - after
> traffic start to flow this bug start.
>
> Now i have this host without traffic and it is working for almost 1
> hour :)
>
>
>
> W dniu 2017-09-07 o 23:47, Paweł Staszewski pisze:
>> Hi Eric
>>
>>
>> today upgraded some host from 4.13.0-rc7+ to latest net-next from git
>>
>> and run some tests simulate clients traffic 40Gbit / 5Mpps mixed tcp
>> + udp
>>
>> There is also bgpd running that feeds FIB with full bgp feed - about
>> 700k prefixes (so routing tables is about 700k routes with about 300
>> nexthops)
>>
>>
>> There is also simulated about 1000 ip neigh hosts (arp) with about
>> 300 ip interfaces assigned on vlans
>>
>>
>>
>> With kernel 4.13.0-rc7+ (git from 05.09.2017) all tests are good
>> (besides that there is panic after about 10 hours - every time same
>> like in this link:
>>
>> https://bugzilla.kernel.org/attachment.cgi?id=258257
>>
>> But looks like acpi related or c/p states with new intel cpus... dono
>> now.
>>
>>
>>
>>
>> W dniu 2017-09-07 o 23:39, Eric Dumazet pisze:
>>> On Thu, 2017-09-07 at 23:30 +0200, Paweł Staszewski wrote:
>>>> I want to mention also here cause last trace for RIP was dst_dev_put
>>>>
>>>> https://bugzilla.kernel.org/attachment.cgi?id=258259
>>> Can you provide a bit more context ?
>>>
>>> Thanks !
>>>
>>>
>>>
>>
>>
>
>
^ permalink raw reply
* Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.
From: Andrew Lunn @ 2017-09-07 22:40 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121AFB@CHN-SV-EXMX02.mchp-main.com>
> > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > > ---
> > > diff --git a/drivers/net/dsa/microchip/Makefile
> > > b/drivers/net/dsa/microchip/Makefile
> > > index ed335e2..0961c30 100644
> > > --- a/drivers/net/dsa/microchip/Makefile
> > > +++ b/drivers/net/dsa/microchip/Makefile
> > > @@ -1,2 +1,2 @@
> > > -obj-$(CONFIG_MICROCHIP_KSZ) += ksz_common.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> > > obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> >
> > Hi Tristram
> >
> > I would of thought this would break the build. You don't add ksz9477.c until the
> > next patch.
> >
> > Each patch needs to compile, otherwise you break git bisect.
> >
> > Andrew
>
> Eventually the file will need to be broken in two, so you would like
> to see all 3 changes (Makefile, ksz_common.c, and ksz9477.c) in 1
> patch file?
You cannot break the build. Each patch must compile on its own.
Breaking changes up into smaller chunks is good. Makes it easier to
review. So think about how you can do it without breaking the build,
but have smaller changes. For example, move a group of functions at a
time?
Andrew
^ permalink raw reply
* Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
From: Alexei Starovoitov @ 2017-09-07 22:37 UTC (permalink / raw)
To: Daniel Borkmann, davem; +Cc: john.fastabend, brouer, andy, netdev
In-Reply-To: <01d69254802986ad3a8b18a8650c45df3df95def.1504821825.git.daniel@iogearbox.net>
On 9/7/17 3:14 PM, Daniel Borkmann wrote:
> Fixes: 97f91a7cf04f ("bpf: add bpf_redirect_map helper routine")
> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> kernel/bpf/verifier.c | 16 ++++++++++++++++
> net/core/filter.c | 21 +++++++++++++++++++--
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d690c7d..477b693 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4203,6 +4203,22 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
> continue;
> }
>
> + if (insn->imm == BPF_FUNC_redirect_map) {
> + u64 addr = (unsigned long)prog;
> + struct bpf_insn r4_ld[] = {
> + BPF_LD_IMM64(BPF_REG_4, addr),
> + *insn,
> + };
> + cnt = ARRAY_SIZE(r4_ld);
> +
> + new_prog = bpf_patch_insn_data(env, i + delta, r4_ld, cnt);
that's a neat trick.
I think we'll be seeing more of such pattern in the future.
Definitely less intrusive fix than asking drivers or net/core
to clear it.
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
From: Andrew Lunn @ 2017-09-07 22:36 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41121A87@CHN-SV-EXMX02.mchp-main.com>
On Thu, Sep 07, 2017 at 09:17:16PM +0000, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add KSZ8795 switch support with function code.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> new file mode 100644
> index 0000000..e4d4e6a
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -0,0 +1,2066 @@
> +/*
> + * Microchip KSZ8795 switch driver
> + *
> + * Copyright (C) 2017 Microchip Technology Inc.
> + * Tristram Ha <Tristram.Ha@microchip.com>
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/export.h>
> +#include <linux/gpio.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/microchip-ksz.h>
> +#include <linux/phy.h>
> +#include <linux/etherdevice.h>
> +#include <linux/if_bridge.h>
> +#include <net/dsa.h>
> +#include <net/switchdev.h>
> +
> +#include "ksz_priv.h"
> +#include "ksz8795.h"
> +
> +/**
> + * Some counters do not need to be read too often because they are less likely
> + * to increase much.
> + */
What does comment mean? Are you caching statistics, and updating
different values at different rates?
> +static const struct {
> + int interval;
> + char string[ETH_GSTRING_LEN];
> +} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
> + { 1, "rx_hi" },
> + { 4, "rx_undersize" },
> + { 4, "rx_fragments" },
> + { 4, "rx_oversize" },
> + { 4, "rx_jabbers" },
> + { 4, "rx_symbol_err" },
> + { 4, "rx_crc_err" },
> + { 4, "rx_align_err" },
> + { 4, "rx_mac_ctrl" },
> + { 1, "rx_pause" },
> + { 1, "rx_bcast" },
> + { 1, "rx_mcast" },
> + { 1, "rx_ucast" },
> + { 2, "rx_64_or_less" },
> + { 2, "rx_65_127" },
> + { 2, "rx_128_255" },
> + { 2, "rx_256_511" },
> + { 2, "rx_512_1023" },
> + { 2, "rx_1024_1522" },
> + { 2, "rx_1523_2000" },
> + { 2, "rx_2001" },
> + { 1, "tx_hi" },
> + { 4, "tx_late_col" },
> + { 1, "tx_pause" },
> + { 1, "tx_bcast" },
> + { 1, "tx_mcast" },
> + { 1, "tx_ucast" },
> + { 4, "tx_deferred" },
> + { 4, "tx_total_col" },
> + { 4, "tx_exc_col" },
> + { 4, "tx_single_col" },
> + { 4, "tx_mult_col" },
> + { 1, "rx_total" },
> + { 1, "tx_total" },
> + { 2, "rx_discards" },
> + { 2, "tx_discards" },
> +};
> +
> +static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
> +{
> + u8 data;
> +
> + ksz_read8(dev, addr, &data);
> + if (set)
> + data |= bits;
> + else
> + data &= ~bits;
> + ksz_write8(dev, addr, data);
> +}
Shouldn't this function be in the shared code? There is an exact copy
currently in ksz_common.c.
> +static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
> + bool set)
> +{
> + u32 addr;
> + u8 data;
> +
> + addr = PORT_CTRL_ADDR(port, offset);
> + ksz_read8(dev, addr, &data);
> +
> + if (set)
> + data |= bits;
> + else
> + data &= ~bits;
> +
> + ksz_write8(dev, addr, data);
> +}
And this also appears to be shared.
> +static void port_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, u64 *cnt)
It would be good to use the ksz8795_ prefix for functions specific to
the KSZ8795.
> +{
> + u32 data;
> + u16 ctrl_addr;
> + u8 check;
> + int timeout;
> +
> + ctrl_addr = addr + SWITCH_COUNTER_NUM * port;
> +
> + ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
> + mutex_lock(&dev->stats_mutex);
> + ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
> +
> + for (timeout = 1; timeout > 0; timeout--) {
A rather short timeount!
> + ksz_read8(dev, REG_IND_MIB_CHECK, &check);
> +
> + if (check & MIB_COUNTER_VALID) {
> + ksz_read32(dev, REG_IND_DATA_LO, &data);
> + if (check & MIB_COUNTER_OVERFLOW)
> + *cnt += MIB_COUNTER_VALUE + 1;
> + *cnt += data & MIB_COUNTER_VALUE;
> + break;
> + }
Should there be a dev_err() here about timing out?
> + }
> + mutex_unlock(&dev->stats_mutex);
> +}
> +
> +static int valid_dyn_entry(struct ksz_device *dev, u8 *data)
> +{
> + int timeout = 100;
> +
> + do {
> + ksz_read8(dev, REG_IND_DATA_CHECK, data);
> + timeout--;
> + } while ((*data & DYNAMIC_MAC_TABLE_NOT_READY) && timeout);
> +
> + /* Entry is not ready for accessing. */
> + if (*data & DYNAMIC_MAC_TABLE_NOT_READY) {
> + return 1;
Use standard error code. -ETIMEOUT
> + /* Entry is ready for accessing. */
> + } else {
> + ksz_read8(dev, REG_IND_DATA_8, data);
> +
> + /* There is no valid entry in the table. */
> + if (*data & DYNAMIC_MAC_TABLE_MAC_EMPTY)
> + return 2;
-EINVAL?
> + }
> + return 0;
> +}
It also looks like these return values get propagated further. So you
really should be using error code.
> +static int ksz_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
> + u8 *fid, u8 *src_port, u8 *timestamp,
> + u16 *entries)
> +{
> + u32 data_hi;
> + u32 data_lo;
> + u16 ctrl_addr;
> + int rc;
> + u8 data;
> +
> + ctrl_addr = IND_ACC_TABLE(TABLE_DYNAMIC_MAC | TABLE_READ) | addr;
> +
> + ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
> +
> + rc = valid_dyn_entry(dev, &data);
> + if (rc == 1) {
> + if (addr == 0)
> + *entries = 0;
> + } else if (rc == 2) {
> + *entries = 0;
> + /* At least one valid entry in the table. */
> + } else {
> + u64 buf;
> +
> + dev->ops->get(dev, REG_IND_DATA_HI, &buf, sizeof(buf));
> + buf = be64_to_cpu(buf);
> + data_hi = (u32)(buf >> 32);
> + data_lo = (u32)buf;
> +
> + /* Check out how many valid entry in the table. */
> + *entries = (u16)(((((u16)
> + data & DYNAMIC_MAC_TABLE_ENTRIES_H) <<
> + DYNAMIC_MAC_ENTRIES_H_S) |
> + (((data_hi & DYNAMIC_MAC_TABLE_ENTRIES) >>
> + DYNAMIC_MAC_ENTRIES_S))) + 1);
When i see so many ((((( i think this needs splitting up to make it
readable.
> +
> + *fid = (u8)((data_hi & DYNAMIC_MAC_TABLE_FID) >>
> + DYNAMIC_MAC_FID_S);
> + *src_port = (u8)((data_hi & DYNAMIC_MAC_TABLE_SRC_PORT) >>
> + DYNAMIC_MAC_SRC_PORT_S);
> + *timestamp = (u8)((
> + data_hi & DYNAMIC_MAC_TABLE_TIMESTAMP) >>
> + DYNAMIC_MAC_TIMESTAMP_S);
Are all the casts needed? Please go through all the code and see about
removing all the unneeded casts.
> +
> + return rc;
> +}
> +
> +struct alu_struct {
> + u8 is_static:1;
> + u8 prio_age:3;
> + u8 is_override:1;
> + u8 is_use_fid:1;
> + u8 port_forward:5;
> + u8 fid:7;
> + u8 mac[ETH_ALEN];
> +};
> +
> +static int ksz_r_sta_mac_table(struct ksz_device *dev, u16 addr,
> + struct alu_struct *alu)
> +{
> + u64 data;
> + u32 data_hi;
> + u32 data_lo;
> +
> + ksz_r_table_64(dev, TABLE_STATIC_MAC, addr, &data);
> + data_hi = data >> 32;
> + data_lo = (u32)data;
> + if (data_hi & (STATIC_MAC_TABLE_VALID | STATIC_MAC_TABLE_OVERRIDE)) {
> + alu->mac[5] = (u8)data_lo;
> + alu->mac[4] = (u8)(data_lo >> 8);
> + alu->mac[3] = (u8)(data_lo >> 16);
> + alu->mac[2] = (u8)(data_lo >> 24);
> + alu->mac[1] = (u8)data_hi;
> + alu->mac[0] = (u8)(data_hi >> 8);
> + alu->port_forward =
> + (u8)((data_hi & STATIC_MAC_TABLE_FWD_PORTS) >>
> + STATIC_MAC_FWD_PORTS_S);
> + alu->is_override =
> + (data_hi & STATIC_MAC_TABLE_OVERRIDE) ? 1 : 0;
> + data_hi >>= 1;
> + alu->is_use_fid = (data_hi & STATIC_MAC_TABLE_USE_FID) ? 1 : 0;
> + alu->fid = (u8)((data_hi & STATIC_MAC_TABLE_FID) >>
> + STATIC_MAC_FID_S);
> + return 0;
> + }
> + return -1;
And what does -1 mean?
> +static void ksz_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
> +{
> + struct ksz_port *port;
> + u8 ctrl;
> + u8 restart;
> + u8 link;
> + u8 speed;
> + u8 force;
> + u8 p = phy;
> + u16 data = 0;
> + int processed = true;
> +
> + port = &dev->ports[p];
> + switch (reg) {
> + case PHY_REG_CTRL:
> + ksz_pread8(dev, p, P_LOCAL_CTRL, &ctrl);
> + ksz_pread8(dev, p, P_NEG_RESTART_CTRL, &restart);
> + ksz_pread8(dev, p, P_SPEED_STATUS, &speed);
> + ksz_pread8(dev, p, P_FORCE_CTRL, &force);
> + if (restart & PORT_PHY_LOOPBACK)
> + data |= PHY_LOOPBACK;
> + if (force & PORT_FORCE_100_MBIT)
> + data |= PHY_SPEED_100MBIT;
> + if (!(force & PORT_AUTO_NEG_DISABLE))
> + data |= PHY_AUTO_NEG_ENABLE;
> + if (restart & PORT_POWER_DOWN)
> + data |= PHY_POWER_DOWN;
Same questions i asked Pavel
Is there a way to access the PHY registers over SPI? The datasheet
suggests there is, but it does not say how, as far as i can tell.
Ideally, we want to use just a normal PHY driver.
> +static int ksz_sset_count(struct dsa_switch *ds)
> +{
> + return TOTAL_SWITCH_COUNTER_NUM;
> +}
> +
> +static void ksz_get_strings(struct dsa_switch *ds, int port, uint8_t *buf)
> +{
> + int i;
> +
> + for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> + memcpy(buf + i * ETH_GSTRING_LEN, mib_names[i].string,
> + ETH_GSTRING_LEN);
> + }
> +}
> +
> +static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
> + uint64_t *buf)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct ksz_port_mib *mib;
> + struct ksz_port_mib_info *info;
> + int i;
> +
> + mib = &dev->ports[port].mib;
> +
> + spin_lock(&dev->mib_read_lock);
> + for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> + info = &mib->info[i];
> + buf[i] = info->counter;
> + info->read_cnt += info->read_max;
> + }
> + spin_unlock(&dev->mib_read_lock);
> +
> + mib->ctrl = 1;
> + schedule_work(&dev->mib_read);
Humm. I want to take a closer look at this caching code...
> +}
> +
> +static u8 STP_MULTICAST_ADDR[] = {
> + 0x01, 0x80, 0xC2, 0x00, 0x00, 0x00
> +};
This could be const. And this is not python. Global variables don't
need to be all capitals.
Andrew
^ permalink raw reply
* RE: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
From: Tristram.Ha @ 2017-09-07 22:29 UTC (permalink / raw)
To: andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170907215551.GV11248@lunn.ch>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 07, 2017 2:56 PM
> To: Tristram Ha - C24268
> Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
>
> On Thu, Sep 07, 2017 at 09:17:10PM +0000, Tristram.Ha@microchip.com wrote:
> > From: Tristram Ha <Tristram.Ha@microchip.com>
> >
> > Add KSZ8795 switch support with SPI access.
> >
> > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > ---
> > diff --git a/drivers/net/dsa/microchip/Makefile
> b/drivers/net/dsa/microchip/Makefile
> > index 0961c30..0d8ba48 100644
> > --- a/drivers/net/dsa/microchip/Makefile
> > +++ b/drivers/net/dsa/microchip/Makefile
> > @@ -1,2 +1,4 @@
> > obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> > obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> > +obj-$(CONFIG_MICROCHIP_KSZ8795) += ksz8795.o ksz_common.o
> > +obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
>
> I've not tried it, but i think this breaks the build
>
> Andrew
So you would like to have all 5 patches in 1 patch file?
^ permalink raw reply
* RE: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.
From: Tristram.Ha @ 2017-09-07 22:26 UTC (permalink / raw)
To: andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170907212449.GQ11248@lunn.ch>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 07, 2017 2:25 PM
> To: Tristram Ha - C24268
> Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ
> switch drivers.
>
> On Thu, Sep 07, 2017 at 09:08:58PM +0000, Tristram.Ha@microchip.com wrote:
> > From: Tristram Ha <Tristram.Ha@microchip.com>
> >
> > Break ksz_common.c into 2 files so that the common code can be used by other
> KSZ switch drivers.
> >
> > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > ---
> > diff --git a/drivers/net/dsa/microchip/Makefile
> > b/drivers/net/dsa/microchip/Makefile
> > index ed335e2..0961c30 100644
> > --- a/drivers/net/dsa/microchip/Makefile
> > +++ b/drivers/net/dsa/microchip/Makefile
> > @@ -1,2 +1,2 @@
> > -obj-$(CONFIG_MICROCHIP_KSZ) += ksz_common.o
> > +obj-$(CONFIG_MICROCHIP_KSZ) += ksz9477.o ksz_common.o
> > obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
>
> Hi Tristram
>
> I would of thought this would break the build. You don't add ksz9477.c until the
> next patch.
>
> Each patch needs to compile, otherwise you break git bisect.
>
> Andrew
Eventually the file will need to be broken in two, so you would like to see all 3 changes (Makefile, ksz_common.c, and ksz9477.c) in 1 patch file?
^ permalink raw reply
* RE: [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in ksz_common.c
From: Tristram.Ha @ 2017-09-07 22:20 UTC (permalink / raw)
To: andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170907213242.GR11248@lunn.ch>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 07, 2017 2:33 PM
> To: Tristram Ha - C24268
> Cc: muvarov@gmail.com; pavel@ucw.cz; nathan.leigh.conrad@gmail.com;
> vivien.didelot@savoirfairelinux.com; f.fainelli@gmail.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in
> ksz_common.c
>
> On Thu, Sep 07, 2017 at 09:09:04PM +0000, Tristram.Ha@microchip.com wrote:
> > From: Tristram Ha <Tristram.Ha@microchip.com>
> >
> > Create new ksz9477.c file from original ksz_common.c.
> >
> > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> > ---
> > diff --git a/drivers/net/dsa/microchip/ksz9477.c
> > b/drivers/net/dsa/microchip/ksz9477.c
> > new file mode 100644
> > index 0000000..bc722b4
> > --- /dev/null
> > +++ b/drivers/net/dsa/microchip/ksz9477.c
> > @@ -0,0 +1,1317 @@
> > +/*
> > + * Microchip switch driver main logic
> > + *
> > + * Copyright (C) 2017
> > + *
> > + * Permission to use, copy, modify, and/or distribute this software
> > +for any
> > + * purpose with or without fee is hereby granted, provided that the
> > +above
>
> Tristram
>
> It looks like something hand mangled this comment. "any" and "above"
> appear to be on a line on there own.
>
> > + * copyright notice and this permission notice appear in all copies.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> > +WARRANTIES
> > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES
> OF
> > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE
> > +LIABLE FOR
> > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
> > +DAMAGES
> > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER
> IN
> > +AN
> > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> ARISING
> > +OUT OF
> > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/export.h>
> > +#include <linux/gpio.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_data/microchip-ksz.h>
> > +#include <linux/phy.h>
> > +#include <linux/etherdevice.h>
> > +#include <linux/if_bridge.h>
> > +#include <net/dsa.h>
> > +#include <net/switchdev.h>
> > +
> > +#include "ksz_priv.h"
> > +#include "ksz_9477_reg.h"
> > +
> > +static const struct {
> > + int index;
> > + char string[ETH_GSTRING_LEN];
> > +} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
> > + { 0x00, "rx_hi" },
> > + { 0x01, "rx_undersize" },
> > + { 0x02, "rx_fragments" },
> > + { 0x03, "rx_oversize" },
> > + { 0x04, "rx_jabbers" },
> > + { 0x05, "rx_symbol_err" },
> > + { 0x06, "rx_crc_err" },
> > + { 0x07, "rx_align_err" },
> > + { 0x08, "rx_mac_ctrl" },
> > + { 0x09, "rx_pause" },
> > + { 0x0A, "rx_bcast" },
> > + { 0x0B, "rx_mcast" },
> > + { 0x0C, "rx_ucast" },
> > + { 0x0D, "rx_64_or_less" },
> > + { 0x0E, "rx_65_127" },
> > + { 0x0F, "rx_128_255" },
> > + { 0x10, "rx_256_511" },
> > + { 0x11, "rx_512_1023" },
> > + { 0x12, "rx_1024_1522" },
> > + { 0x13, "rx_1523_2000" },
> > + { 0x14, "rx_2001" },
> > + { 0x15, "tx_hi" },
> > + { 0x16, "tx_late_col" },
> > + { 0x17, "tx_pause" },
> > + { 0x18, "tx_bcast" },
> > + { 0x19, "tx_mcast" },
> > + { 0x1A, "tx_ucast" },
> > + { 0x1B, "tx_deferred" },
> > + { 0x1C, "tx_total_col" },
> > + { 0x1D, "tx_exc_col" },
> > + { 0x1E, "tx_single_col" },
> > + { 0x1F, "tx_mult_col" },
> > + { 0x80, "rx_total" },
> > + { 0x81, "tx_total" },
> > + { 0x82, "rx_discards" },
> > + { 0x83, "tx_discards" },
> > +};
> > +
> > +static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool
> > +set) {
>
> More mangling? Should set be on the end of the previous line?
>
> > +static void read_table(struct dsa_switch *ds, u32 *table) {
> > + struct ksz_device *dev = ds->priv;
> > +
> > + ksz_read32(dev, REG_SW_ALU_VAL_A, &table[0]);
> > + ksz_read32(dev, REG_SW_ALU_VAL_B, &table[1]);
> > + ksz_read32(dev, REG_SW_ALU_VAL_C, &table[2]);
> > + ksz_read32(dev, REG_SW_ALU_VAL_D, &table[3]); }
> > +
> > +static void write_table(struct dsa_switch *ds, u32 *table) {
> > + struct ksz_device *dev = ds->priv;
> > +
> > + ksz_write32(dev, REG_SW_ALU_VAL_A, table[0]);
> > + ksz_write32(dev, REG_SW_ALU_VAL_B, table[1]);
> > + ksz_write32(dev, REG_SW_ALU_VAL_C, table[2]);
> > + ksz_write32(dev, REG_SW_ALU_VAL_D, table[3]); }
>
> More mangling? } at the end of a line?
>
> I will stop reading now and wait for a v2 this is not corrupt.
>
> Andrew
Sorry about that. It seems my e-mail system wraps the line too soon.
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls of udelay to usleep_range
From: Brown, Aaron F @ 2017-09-07 22:19 UTC (permalink / raw)
To: Pavel Machek, Matthew Tan, intel-wired-lan@lists.osuosl.org
Cc: michael.kardonik@nxp.com, Williams, Mitch A,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20170904162555.GB21849@amd>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Pavel Machek
> Sent: Monday, September 4, 2017 9:26 AM
> To: Matthew Tan <matthew.tan_1@nxp.com>
> Cc: michael.kardonik@nxp.com; Williams, Mitch A
> <mitch.a.williams@intel.com>; linux-kernel@vger.kernel.org;
> john.ronciak@intel.com; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls
> of udelay to usleep_range
>
> Hi!
>
> > @@ -183,7 +183,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw
> *hw, u32 offset, u16 *data)
> > * reading duplicate data in the next MDIC transaction.
> > */
> > if (hw->mac.type == e1000_pch2lan)
> > - udelay(100);
> > + usleep_range(90, 100);
> >
> > return 0;
> > }
>
> Can you explain why shortening the delay is acceptable here?
Maybe it's not.
This patch is causing speed / duplex tests to fail on several of my test systems. Specifically a Lenova laptop with an 82577 and a NUC with an i218 (though that does not mean it is limited to those or that it's not related to the individual link partner.)
In both cases they "blow up" when attempting to link at 10 Mb with a Call Trace on the console / log and a watchdog: BUG: soft lockup - CPU#X stuck appearing in the current login session. The simplest way to produce the crash is simply load the interface and attempt to bring it up with the link partner configured to 10 Mb half (force or autoneg) or forced to 10 Mb full (autoneg to 10 Mb full did not blow up.) The problem is also appearing at some, but not all 100 Mb speeds and duplexes. 100 Mb also crashes, apparently with the same pattern. With the link partner set to 100 Mb half Autoneg on, 100 Mb half forced and 100 Mb full forced the system crashes, 100 Full autoneg seems to work fine.
The call trace repeats every 30 seconds, is captured in the log and looks a lot like this:
----------------------------------------
Sep 7 14:17:10 u1459 kernel: watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:23]
Sep 7 14:17:10 u1459 kernel: Modules linked in: e1000e ptp pps_core ip6table_filter ip6_tables ebtable_nat ebtables nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack libcrc32c ipt_REJECT nf_reject_ipv4 xt_CHECKSUM iptable_mangle iptable_filter ip_tables bridge stp llc nfsd lockd grace nfs_acl auth_rpcgss sunrpc autofs4 ipv6 crc_ccitt dm_mirror dm_region_hash dm_log vhost_net vhost tap tun kvm_intel kvm irqbypass uinput joydev ax88179_178a usbnet mii iTCO_wdt iTCO_vendor_support sg i2c_i801 lpc_ich mfd_core xhci_pci snd_hda_codec_realtek xhci_hcd snd_hda_codec_hdmi snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore dm_mod(E) dax(E) ext4(E) jbd2(E) mbcache(E) sd_mod(E) ahci(E) libahci(E) i915(E) drm_kms_helper
(E) drm(E) fb_sys_fops(E)
Sep 7 14:17:10 u1459 kernel: sysimgblt(E) sysfillrect(E) syscopyarea(E) i2c_algo_bit(E) i2c_core(E) iosf_mbi(E) video(E)
Sep 7 14:17:10 u1459 kernel: CPU: 2 PID: 23 Comm: kworker/2:0 Tainted: G EL 4.13.0-rc6_next-queue_dev-queue_regress #38
Sep 7 14:17:10 u1459 kernel: Hardware name: /NUC5i5MYBE, BIOS MYBDWi5v.86A.0030.2016.0527.1657 05/27/2016
Sep 7 14:17:10 u1459 kernel: Workqueue: events linkwatch_event
Sep 7 14:17:10 u1459 kernel: task: ffff88024cf84680 task.stack: ffffc90000d44000
Sep 7 14:17:10 u1459 kernel: RIP: 0010:queued_spin_lock_slowpath+0x56/0x1d0
Sep 7 14:17:10 u1459 kernel: RSP: 0018:ffffc90000d478c8 EFLAGS: 00000202 ORIG_RAX: ffffffffffffff10
Sep 7 14:17:10 u1459 kernel: RAX: 0000000000000101 RBX: ffff880239fe8000 RCX: 0000000000000101
Sep 7 14:17:10 u1459 kernel: RDX: ffffc90000d47948 RSI: 0000000000000001 RDI: ffff880239feb1a0
Sep 7 14:17:10 u1459 kernel: RBP: ffffc90000d47968 R08: 0000000000000001 R09: ffff88024ce181a4
Sep 7 14:17:10 u1459 kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff880239fe8840
Sep 7 14:17:10 u1459 kernel: R13: ffff88024ce180e4 R14: ffff88024ce180e4 R15: ffffc90000d47a48
Sep 7 14:17:10 u1459 kernel: FS: 0000000000000000(0000) GS:ffff880255d00000(0000) knlGS:0000000000000000
Sep 7 14:17:10 u1459 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Sep 7 14:17:10 u1459 kernel: CR2: 00007ffd4c3579b8 CR3: 0000000001c09000 CR4: 00000000003406e0
Sep 7 14:17:10 u1459 kernel: DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Sep 7 14:17:10 u1459 kernel: DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Sep 7 14:17:10 u1459 kernel: Call Trace:
Sep 7 14:17:10 u1459 kernel: ? netlink_broadcast_filtered+0x15f/0x1a0
Sep 7 14:17:10 u1459 kernel: _raw_spin_lock+0x21/0x30
Sep 7 14:17:10 u1459 kernel: e1000e_get_stats64+0x2b/0x140 [e1000e]
Sep 7 14:17:10 u1459 kernel: dev_get_stats+0x3d/0xc0
Sep 7 14:17:10 u1459 kernel: ? __nla_reserve+0x53/0x70
Sep 7 14:17:10 u1459 kernel: rtnl_fill_stats+0x6b/0x150
Sep 7 14:17:10 u1459 kernel: rtnl_fill_ifinfo+0x542/0x920
Sep 7 14:17:10 u1459 kernel: ? if_nlmsg_size+0xae/0x180
Sep 7 14:17:10 u1459 kernel: rtmsg_ifinfo_build_skb+0x90/0xe0
Sep 7 14:17:10 u1459 kernel: rtmsg_ifinfo_event+0x35/0x60
Sep 7 14:17:10 u1459 kernel: rtmsg_ifinfo+0x13/0x20
Sep 7 14:17:10 u1459 kernel: netdev_state_change+0x46/0x50
Sep 7 14:17:10 u1459 kernel: linkwatch_do_dev+0x3c/0x60
Sep 7 14:17:10 u1459 kernel: __linkwatch_run_queue+0x13a/0x210
Sep 7 14:17:10 u1459 kernel: ? set_task_cpu+0xbb/0x1a0
Sep 7 14:17:10 u1459 kernel: ? lock_timer_base+0x8a/0xb0
Sep 7 14:17:10 u1459 kernel: linkwatch_event+0x25/0x30
Sep 7 14:17:10 u1459 kernel: process_one_work+0x193/0x410
Sep 7 14:17:10 u1459 kernel: ? try_to_del_timer_sync+0x53/0x70
Sep 7 14:17:10 u1459 kernel: ? del_timer_sync+0x3d/0x50
Sep 7 14:17:10 u1459 kernel: ? maybe_create_worker+0x8b/0x110
Sep 7 14:17:10 u1459 kernel: ? schedule+0x3a/0xa0
Sep 7 14:17:10 u1459 kernel: worker_thread+0x7c/0x420
Sep 7 14:17:10 u1459 kernel: ? default_wake_function+0x12/0x20
Sep 7 14:17:10 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 7 14:17:10 u1459 kernel: ? schedule+0x3a/0xa0
Sep 7 14:17:10 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 7 14:17:10 u1459 kernel: kthread+0x111/0x150
Sep 7 14:17:10 u1459 kernel: ? __kthread_init_worker+0x40/0x40
Sep 7 14:17:10 u1459 kernel: ret_from_fork+0x22/0x30
Sep 7 14:17:10 u1459 kernel: Code: ff ff 75 57 83 fe 01 89 ca 89 f0 41 0f 45 d0 f0 0f b1 17 39 f0 75 e3 83 fa 01 74 1c 8b 07 48 8d 55 e0 84 c0 89 45 e0 74 0a f3 90 <8b> 07 84 c0 89 02 75 f6 66 c7 07 01 00 c9 c3 8b 37 48 8d 45 a0
Sep 7 14:17:38 u1459 kernel: watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:23]
Sep 7 14:17:38 u1459 kernel: Modules linked in: e1000e ptp pps_core ip6table_filter ip6_tables ebtable_nat ebtables nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack libcrc32c ipt_REJECT nf_reject_ipv4 xt_CHECKSUM iptable_mangle iptable_filter ip_tables bridge stp llc nfsd lockd grace nfs_acl auth_rpcgss sunrpc autofs4 ipv6 crc_ccitt dm_mirror dm_region_hash dm_log vhost_net vhost tap tun kvm_intel kvm irqbypass uinput joydev ax88179_178a usbnet mii iTCO_wdt iTCO_vendor_support sg i2c_i801 lpc_ich mfd_core xhci_pci snd_hda_codec_realtek xhci_hcd snd_hda_codec_hdmi snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore dm_mod(E) dax(E) ext4(E) jbd2(E) mbcache(E) sd_mod(E) ahci(E) libahci(E) i915(E) drm_kms_helper
(E) drm(E) fb_sys_fops(E)
Sep 7 14:17:38 u1459 kernel: sysimgblt(E) sysfillrect(E) syscopyarea(E) i2c_algo_bit(E) i2c_core(E) iosf_mbi(E) video(E)
Sep 7 14:17:38 u1459 kernel: CPU: 2 PID: 23 Comm: kworker/2:0 Tainted: G EL 4.13.0-rc6_next-queue_dev-queue_regress #38
Sep 7 14:17:38 u1459 kernel: Hardware name: /NUC5i5MYBE, BIOS MYBDWi5v.86A.0030.2016.0527.1657 05/27/2016
Sep 7 14:17:38 u1459 kernel: Workqueue: events linkwatch_event
Sep 7 14:17:38 u1459 kernel: task: ffff88024cf84680 task.stack: ffffc90000d44000
Sep 7 14:17:38 u1459 kernel: RIP: 0010:queued_spin_lock_slowpath+0x56/0x1d0
Sep 7 14:17:38 u1459 kernel: RSP: 0018:ffffc90000d478c8 EFLAGS: 00000202 ORIG_RAX: ffffffffffffff10
Sep 7 14:17:38 u1459 kernel: RAX: 0000000000000101 RBX: ffff880239fe8000 RCX: 0000000000000101
Sep 7 14:17:38 u1459 kernel: RDX: ffffc90000d47948 RSI: 0000000000000001 RDI: ffff880239feb1a0
Sep 7 14:17:38 u1459 kernel: RBP: ffffc90000d47968 R08: 0000000000000001 R09: ffff88024ce181a4
Sep 7 14:17:38 u1459 kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff880239fe8840
Sep 7 14:17:38 u1459 kernel: R13: ffff88024ce180e4 R14: ffff88024ce180e4 R15: ffffc90000d47a48
Sep 7 14:17:38 u1459 kernel: FS: 0000000000000000(0000) GS:ffff880255d00000(0000) knlGS:0000000000000000
Sep 7 14:17:38 u1459 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Sep 7 14:17:38 u1459 kernel: CR2: 00007ffd4c3579b8 CR3: 0000000001c09000 CR4: 00000000003406e0
Sep 7 14:17:38 u1459 kernel: DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Sep 7 14:17:38 u1459 kernel: DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Sep 7 14:17:38 u1459 kernel: Call Trace:
Sep 7 14:17:38 u1459 kernel: ? netlink_broadcast_filtered+0x15f/0x1a0
Sep 7 14:17:38 u1459 kernel: _raw_spin_lock+0x21/0x30
Sep 7 14:17:38 u1459 kernel: e1000e_get_stats64+0x2b/0x140 [e1000e]
Sep 7 14:17:38 u1459 kernel: dev_get_stats+0x3d/0xc0
Sep 7 14:17:38 u1459 kernel: ? __nla_reserve+0x53/0x70
Sep 7 14:17:38 u1459 kernel: rtnl_fill_stats+0x6b/0x150
Sep 7 14:17:38 u1459 kernel: rtnl_fill_ifinfo+0x542/0x920
Sep 7 14:17:38 u1459 kernel: ? if_nlmsg_size+0xae/0x180
Sep 7 14:17:38 u1459 kernel: rtmsg_ifinfo_build_skb+0x90/0xe0
Sep 7 14:17:38 u1459 kernel: rtmsg_ifinfo_event+0x35/0x60
Sep 7 14:17:38 u1459 kernel: rtmsg_ifinfo+0x13/0x20
Sep 7 14:17:38 u1459 kernel: netdev_state_change+0x46/0x50
Sep 7 14:17:38 u1459 kernel: linkwatch_do_dev+0x3c/0x60
Sep 7 14:17:38 u1459 kernel: __linkwatch_run_queue+0x13a/0x210
Sep 7 14:17:38 u1459 kernel: ? set_task_cpu+0xbb/0x1a0
Sep 7 14:17:38 u1459 kernel: ? lock_timer_base+0x8a/0xb0
Sep 7 14:17:38 u1459 kernel: linkwatch_event+0x25/0x30
Sep 7 14:17:38 u1459 kernel: process_one_work+0x193/0x410
Sep 7 14:17:38 u1459 kernel: ? try_to_del_timer_sync+0x53/0x70
Sep 7 14:17:38 u1459 kernel: ? del_timer_sync+0x3d/0x50
Sep 7 14:17:38 u1459 kernel: ? maybe_create_worker+0x8b/0x110
Sep 7 14:17:38 u1459 kernel: ? schedule+0x3a/0xa0
Sep 7 14:17:38 u1459 kernel: worker_thread+0x7c/0x420
Sep 7 14:17:38 u1459 kernel: ? default_wake_function+0x12/0x20
Sep 7 14:17:38 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 7 14:17:38 u1459 kernel: ? schedule+0x3a/0xa0
Sep 7 14:17:38 u1459 kernel: ? maybe_create_worker+0x110/0x110
Sep 7 14:17:38 u1459 kernel: kthread+0x111/0x150
Sep 7 14:17:38 u1459 kernel: ? __kthread_init_worker+0x40/0x40
Sep 7 14:17:38 u1459 kernel: ret_from_fork+0x22/0x30
Sep 7 14:17:38 u1459 kernel: Code: ff ff 75 57 83 fe 01 89 ca 89 f0 41 0f 45 d0 f0 0f b1 17 39 f0 75 e3 83 fa 01 74 1c 8b 07 48 8d 55 e0 84 c0 89 45 e0 74 0a f3 90 <8b> 07 84 c0 89 02 75 f6 66 c7 07 01 00 c9 c3 8b 37 48 8d 45 a0
----------------------------------------
Here is the contents of an ssh session to the box (using a usb ethernet port for a control network as the LOM goes dead.) :
----------------------
u1459:[0]/root> modprobe e1000e
u1459:[0]/root> nwk
Bringing up eth0... -> success
Bringing up eth1... -> success
Bringing up eth1 IPv6... -> success
u1459:[0]/root> lnks
Message from syslogd@u1459 at Sep 7 14:04:42 ...
kernel:watchdog: BUG: soft lockup - CPU#0 stuck for 23s! [ifconfig:3980]
Message from syslogd@u1459 at Sep 7 14:04:42 ...
kernel:watchdog: BUG: soft lockup - CPU#3 stuck for 22s! [ifconfig:3983]
Write failed: Broken pipe
[root@s252 ~]# ssh u1459
Warning: Permanently added 'u1459,190.0.14.59' (RSA) to the list of known hosts.
Last login: Thu Sep 7 14:12:06 2017
You are running LINUX 4.13.0-rc6_next-queue_dev-queue_regress.
This system was (post)installed: Fri Jan 27 16:38:47 PST 2017
It has been more than 7 days since you ran 'sgetall' on this system!
Your root drive is 11% used.
You have 4 CPUs on-line.
The current ethtool is 4.11
Your ethool is 4.8
To get the current version, run 'ethtool.inst'
u1459:[0]/root> dmesg -c > /dev/null
u1459:[0]/root> modprobe e1000e
u1459:[0]/root> dmesg -c
pps_core: LinuxPPS API ver. 1 registered
pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <giometti@linux.it>
PTP clock support registered
e1000e: Intel(R) PRO/1000 Network Driver - 3.2.6-k
e1000e: Copyright(c) 1999 - 2015 Intel Corporation.
e1000e 0000:00:19.0: Interrupt Throttling Rate (ints/sec) set to dynamic conservative mode
e1000e 0000:00:19.0 0000:00:19.0 (uninitialized): registered PHC clock
e1000e 0000:00:19.0 eth1: (PCI Express:2.5GT/s:Width x1) f4:4d:30:62:c7:14
e1000e 0000:00:19.0 eth1: Intel(R) PRO/1000 Network Connection
e1000e 0000:00:19.0 eth1: MAC: 11, PHY: 12, PBA No: FFFFFF-0FF
u1459:[0]/root> ifconfig eth1 190.1.14.59
u1459:[0]/root>
Message from syslogd@u1459 at Sep 7 14:15:06 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 23s! [kworker/2:0:23]
Message from syslogd@u1459 at Sep 7 14:15:34 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 23s! [kworker/2:0:23]
Message from syslogd@u1459 at Sep 7 14:16:06 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 23s! [kworker/2:0:23]
Message from syslogd@u1459 at Sep 7 14:16:34 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 23s! [kworker/2:0:23]
Message from syslogd@u1459 at Sep 7 14:17:10 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:23]
Message from syslogd@u1459 at Sep 7 14:17:38 ...
kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:23]
^ permalink raw reply
* [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs
From: Daniel Borkmann @ 2017-09-07 22:14 UTC (permalink / raw)
To: davem; +Cc: ast, john.fastabend, brouer, andy, netdev, Daniel Borkmann
We can potentially run into a couple of issues with the XDP
bpf_redirect_map() helper. The ri->map in the per CPU storage
can become stale in several ways, mostly due to misuse, where
we can then trigger a use after free on the map:
i) prog A is calling bpf_redirect_map(), returning XDP_REDIRECT
and running on a driver not supporting XDP_REDIRECT yet. The
ri->map on that CPU becomes stale when the XDP program is unloaded
on the driver, and a prog B loaded on a different driver which
supports XDP_REDIRECT return code. prog B would have to omit
calling to bpf_redirect_map() and just return XDP_REDIRECT, which
would then access the freed map in xdp_do_redirect() since not
cleared for that CPU.
ii) prog A is calling bpf_redirect_map(), returning a code other
than XDP_REDIRECT. prog A is then detached, which triggers release
of the map. prog B is attached which, similarly as in i), would
just return XDP_REDIRECT without having called bpf_redirect_map()
and thus be accessing the freed map in xdp_do_redirect() since
not cleared for that CPU.
iii) prog A is attached to generic XDP, calling the bpf_redirect_map()
helper and returning XDP_REDIRECT. xdp_do_generic_redirect() is
currently not handling ri->map (will be fixed by Jesper), so it's
not being reset. Later loading a e.g. native prog B which would,
say, call bpf_xdp_redirect() and then returns XDP_REDIRECT would
find in xdp_do_redirect() that a map was set and uses that causing
use after free on map access.
Fix thus needs to avoid accessing stale ri->map pointers, naive
way would be to call a BPF function from drivers that just resets
it to NULL for all XDP return codes but XDP_REDIRECT and including
XDP_REDIRECT for drivers not supporting it yet (and let ri->map
being handled in xdp_do_generic_redirect()). There is a less
intrusive way w/o letting drivers call a reset for each BPF run.
The verifier knows we're calling into bpf_xdp_redirect_map()
helper, so it can do a small insn rewrite transparent to the prog
itself in the sense that it fills R4 with a pointer to the own
bpf_prog. We have that pointer at verification time anyway and
R4 is allowed to be used as per calling convention we scratch
R0 to R5 anyway, so they become inaccessible and program cannot
read them prior to a write. Then, the helper would store the prog
pointer in the current CPUs struct redirect_info. Later in
xdp_do_*_redirect() we check whether the redirect_info's prog
pointer is the same as passed xdp_prog pointer, and if that's
the case then all good, since the prog holds a ref on the map
anyway, so it is always valid at that point in time and must
have a reference count of at least 1. If in the unlikely case
they are not equal, it means we got a stale pointer, so we clear
and bail out right there. Also do reset map and the owning prog
in bpf_xdp_redirect(), so that bpf_xdp_redirect_map() and
bpf_xdp_redirect() won't get mixed up, only the last call should
take precedence. A tc bpf_redirect() doesn't use map anywhere
yet, so no need to clear it there since never accessed in that
layer.
Note that in case the prog is released, and thus the map as
well we're still under RCU read critical section at that time
and have preemption disabled as well. Once we commit with the
__dev_map_insert_ctx() from xdp_do_redirect_map() and set the
map to ri->map_to_flush, we still wait for a xdp_do_flush_map()
to finish in devmap dismantle time once flush_needed bit is set,
so that is fine.
Fixes: 97f91a7cf04f ("bpf: add bpf_redirect_map helper routine")
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/verifier.c | 16 ++++++++++++++++
net/core/filter.c | 21 +++++++++++++++++++--
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d690c7d..477b693 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4203,6 +4203,22 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
continue;
}
+ if (insn->imm == BPF_FUNC_redirect_map) {
+ u64 addr = (unsigned long)prog;
+ struct bpf_insn r4_ld[] = {
+ BPF_LD_IMM64(BPF_REG_4, addr),
+ *insn,
+ };
+ cnt = ARRAY_SIZE(r4_ld);
+
+ new_prog = bpf_patch_insn_data(env, i + delta, r4_ld, cnt);
+ if (!new_prog)
+ return -ENOMEM;
+
+ delta += cnt - 1;
+ env->prog = prog = new_prog;
+ insn = new_prog->insnsi + i + delta;
+ }
patch_call_imm:
fn = prog->aux->ops->get_func_proto(insn->imm);
/* all functions that have prototype and verifier allowed
diff --git a/net/core/filter.c b/net/core/filter.c
index 5912c73..0848df2 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1794,6 +1794,7 @@ struct redirect_info {
u32 flags;
struct bpf_map *map;
struct bpf_map *map_to_flush;
+ const struct bpf_prog *map_owner;
};
static DEFINE_PER_CPU(struct redirect_info, redirect_info);
@@ -1807,7 +1808,6 @@ struct redirect_info {
ri->ifindex = ifindex;
ri->flags = flags;
- ri->map = NULL;
return TC_ACT_REDIRECT;
}
@@ -2504,6 +2504,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ const struct bpf_prog *map_owner = ri->map_owner;
struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
struct net_device *fwd;
@@ -2511,6 +2512,15 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
ri->ifindex = 0;
ri->map = NULL;
+ ri->map_owner = NULL;
+
+ /* This is really only caused by a deliberately crappy
+ * BPF program, normally we would never hit that case,
+ * so no need to inform someone via tracepoints either,
+ * just bail out.
+ */
+ if (unlikely(map_owner != xdp_prog))
+ return -EINVAL;
fwd = __dev_map_lookup_elem(map, index);
if (!fwd) {
@@ -2607,6 +2617,8 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
ri->ifindex = ifindex;
ri->flags = flags;
+ ri->map = NULL;
+ ri->map_owner = NULL;
return XDP_REDIRECT;
}
@@ -2619,7 +2631,8 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
.arg2_type = ARG_ANYTHING,
};
-BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, u64, flags)
+BPF_CALL_4(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, u64, flags,
+ const struct bpf_prog *, map_owner)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
@@ -2629,10 +2642,14 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
ri->ifindex = ifindex;
ri->flags = flags;
ri->map = map;
+ ri->map_owner = map_owner;
return XDP_REDIRECT;
}
+/* Note, arg4 is hidden from users and populated by the verifier
+ * with the right pointer.
+ */
static const struct bpf_func_proto bpf_xdp_redirect_map_proto = {
.func = bpf_xdp_redirect_map,
.gpl_only = false,
--
1.9.3
^ permalink raw reply related
* Re: [PATCH iproute2 0/4] Add support for dpipe's global header formatting
From: Stephen Hemminger @ 2017-09-07 22:13 UTC (permalink / raw)
To: Arkadi Sharshevsky; +Cc: netdev, davem, jiri, mlxsw, andrew
In-Reply-To: <1504794403-45690-1-git-send-email-arkadis@mellanox.com>
On Thu, 7 Sep 2017 17:26:39 +0300
Arkadi Sharshevsky <arkadis@mellanox.com> wrote:
> Some dpipe's global header values need special formatting, for example
> Ethernet and IP addresses. This patchset adds support for IPv4/6 and
> Ethernet's special format.
>
> Arkadi Sharshevsky (4):
> devlink: Make match/action parsing more flexible
> devlink: Add support for special format protocol headers
> devlink: Update devlink UAPI file
> devlink: Add support for protocol IPv4/IPv6/Ethernet special formats
>
> devlink/devlink.c | 319 +++++++++++++++++++++++++++++++++++++-----------
> include/linux/devlink.h | 18 +++
> 2 files changed, 265 insertions(+), 72 deletions(-)
>
Applied 1,2 and 4. #3 was not needed.
^ permalink raw reply
* Re: [PATCH iproute2 3/4] devlink: Update devlink UAPI file
From: Stephen Hemminger @ 2017-09-07 22:12 UTC (permalink / raw)
To: Arkadi Sharshevsky; +Cc: netdev, davem, jiri, mlxsw, andrew, Jiri Pirko
In-Reply-To: <1504794403-45690-4-git-send-email-arkadis@mellanox.com>
On Thu, 7 Sep 2017 17:26:42 +0300
Arkadi Sharshevsky <arkadis@mellanox.com> wrote:
> Update devlink UAPI file.
>
> Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> include/linux/devlink.h | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/include/linux/devlink.h b/include/linux/devlink.h
> index 7644005..a62695e 100644
> --- a/include/linux/devlink.h
> +++ b/include/linux/devlink.h
> @@ -226,4 +226,22 @@ enum devlink_dpipe_action_type {
> DEVLINK_DPIPE_ACTION_TYPE_FIELD_MODIFY,
> };
>
> +enum devlink_dpipe_field_ethernet_id {
> + DEVLINK_DPIPE_FIELD_ETHERNET_DST_MAC,
> +};
> +
> +enum devlink_dpipe_field_ipv4_id {
> + DEVLINK_DPIPE_FIELD_IPV4_DST_IP,
> +};
> +
> +enum devlink_dpipe_field_ipv6_id {
> + DEVLINK_DPIPE_FIELD_IPV6_DST_IP,
> +};
> +
> +enum devlink_dpipe_header_id {
> + DEVLINK_DPIPE_HEADER_ETHERNET,
> + DEVLINK_DPIPE_HEADER_IPV4,
> + DEVLINK_DPIPE_HEADER_IPV6,
> +};
> +
> #endif /* _LINUX_DEVLINK_H_ */
This patch was unnecessary. Already had updated file present.
^ 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