* Re: stmmac / meson8b-dwmac
From: Jose Abreu @ 2019-02-19 8:47 UTC (permalink / raw)
To: Simon Huelck, Jose Abreu, Martin Blumenstingl
Cc: Emiliano Ingrassia, Gpeppe.cavallaro, alexandre.torgue,
linux-amlogic, netdev
In-Reply-To: <cd0b3dec-af3f-af69-50b7-6ca6f7256900@gmx.de>
Hi Simon,
On 2/18/2019 6:05 PM, Simon Huelck wrote:
> disabling EEE doesnt help ( did it via the entry in the .dtb / .dts ),
> the results are the same. I can confirm the LPI counters are zero or one
> only after the test.....
It's interesting to see that you have a lot of RX packets but few
RX interrupts. This can either be due to mis-configuration of
interrupt flags or RX Watchdog.
1. For interrupt flags you should be using LEVEL triggered IRQ.
2. For RX Watchdog you can try disabling it by adding the
following in your platform wrapper driver (which will be
"dwmac-meson8b.c", at probe):
"plat_data->force_sf_dma_mode = 1;" and
"plat_data->riwt_off = 1;"
Thanks,
Jose Miguel Abreu
^ permalink raw reply
* Re: [PATCH v3 perf,bpf 05/11] perf, bpf: save bpf_prog_info in a rbtree in perf_env
From: Jiri Olsa @ 2019-02-19 8:51 UTC (permalink / raw)
To: Song Liu
Cc: Netdev, linux-kernel, ast@kernel.org, daniel@iogearbox.net,
Kernel Team, peterz@infradead.org, acme@redhat.com,
jolsa@kernel.org, namhyung@kernel.org
In-Reply-To: <4784383E-1BFA-4387-8864-B1814FFEC937@fb.com>
On Tue, Feb 19, 2019 at 05:52:20AM +0000, Song Liu wrote:
>
>
> > On Feb 17, 2019, at 3:05 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Fri, Feb 15, 2019 at 01:53:48PM -0800, Song Liu wrote:
> >
> > SNIP
> >
> >> info_linear = bpf_program__get_prog_info_linear(fd, arrays);
> >> if (IS_ERR_OR_NULL(info_linear)) {
> >> @@ -151,8 +165,8 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
> >> machine, process);
> >> }
> >>
> >> - /* Synthesize PERF_RECORD_BPF_EVENT */
> >> if (opts->bpf_event) {
> >> + /* Synthesize PERF_RECORD_BPF_EVENT */
> >> *bpf_event = (struct bpf_event){
> >> .header = {
> >> .type = PERF_RECORD_BPF_EVENT,
> >> @@ -165,6 +179,19 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
> >> memcpy(bpf_event->tag, info->tag, BPF_TAG_SIZE);
> >> memset((void *)event + event->header.size, 0, machine->id_hdr_size);
> >> event->header.size += machine->id_hdr_size;
> >> +
> >> + /* save bpf_prog_info to env */
> >> + info_node = malloc(sizeof(struct bpf_prog_info_node));
> >> + if (info_node) {
> >> + info_node->info_linear = info_linear;
> >> + perf_env__insert_bpf_prog_info(env, info_node);
> >> + info_linear = NULL;
> >> + }
> >
> > what if the allocation fails? we don't care?
> >
> > jirka
>
> My original plan is to just ignore it and accept that this program
> doesn't have annotation. Any suggestion on what would be a better
> approach?
there's an error path in the function, I'd bail out if the malloc fails
jirka
^ permalink raw reply
* [PATCH v3] net: ns83820: code cleanup for ns83820_probe_phy()
From: Mao Wenan @ 2019-02-19 9:06 UTC (permalink / raw)
To: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
daniel, ast, julia.lawall
This patch is to do code cleanup for ns83820_probe_phy().
It deletes unused variable 'first' and commented out code.
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
v2->v3: delte unused variable 'first'; change subject from
"net: ns83820: drop pointless static qualifier in ns83820_probe_phy()" to
"net: ns83820: code cleanup for ns83820_probe_phy()".
drivers/net/ethernet/natsemi/ns83820.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
index 958fced4dacf..955d34a6f0d8 100644
--- a/drivers/net/ethernet/natsemi/ns83820.c
+++ b/drivers/net/ethernet/natsemi/ns83820.c
@@ -1869,34 +1869,16 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
static void ns83820_probe_phy(struct net_device *ndev)
{
struct ns83820 *dev = PRIV(ndev);
- static int first;
int i;
#define MII_PHYIDR1 0x02
#define MII_PHYIDR2 0x03
-#if 0
- if (!first) {
- unsigned tmp;
- ns83820_mii_read_reg(dev, 1, 0x09);
- ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
-
- tmp = ns83820_mii_read_reg(dev, 1, 0x00);
- ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
- udelay(1300);
- ns83820_mii_read_reg(dev, 1, 0x09);
- }
-#endif
- first = 1;
-
for (i=1; i<2; i++) {
int j;
unsigned a, b;
a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
- //printk("%s: phy %d: 0x%04x 0x%04x\n",
- // ndev->name, i, a, b);
-
for (j=0; j<0x16; j+=4) {
dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
ndev->name, j,
--
2.20.1
^ permalink raw reply related
* Re: [RFC v2] net: bridge: don't flood known multicast traffic when snooping is enabled
From: Ido Schimmel @ 2019-02-19 8:53 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, roopa, linus.luessing, f.fainelli, bridge, rmk+kernel,
ilyav
In-Reply-To: <20190218122107.10097-1-nikolay@cumulusnetworks.com>
Hi Nik,
On Mon, Feb 18, 2019 at 02:21:07PM +0200, Nikolay Aleksandrov wrote:
> This is v2 of the RFC patch which aims to forward packets to known
> mdsts' ports only (the no querier case). After v1 I've kept
> the previous behaviour when it comes to unregistered traffic or when
> a querier is present. All of this is of course only with snooping
> enabled. So with this patch the following changes should occur:
> - No querier: forward known mdst traffic to its registered ports,
> no change about unknown mcast (flood)
> - Querier present: no change
>
> The reason to do this is simple - we want to respect the user's mdb
> configuration in both cases, that is if the user adds static mdb entries
> manually then we should use that information about forwarding traffic.
>
> What do you think ?
>
> * Notes
> Traffic that is currently marked as mrouters_only:
> - IPv4: non-local mcast traffic, igmp reports
> - IPv6: non-all-nodes-dst mcast traffic, mldv1 reports
>
> Simple use case:
> $ echo 1 > /sys/class/net/bridge/bridge/multicast_snooping
> $ bridge mdb add dev bridge port swp1 grp 239.0.0.1
> - without a querier currently traffic for 239.0.0.1 will still be flooded,
> with this change it will be forwarded only to swp1
Looks good to me.
>
> Ido, I know this doesn't solve the issue you brought up, maybe we should
> have a separate discussion about acting on querier changes in the switch
> driver or alternative solutions (e.g. always-flood-unknown-mcast knob).
> Perhaps the bridge can notify the drivers on querier state changes.
I think we need to reflect querier state to drivers. That's the only way
to sync the two data paths. We can flood different packet types using
different tables. The IPv6 packet types are like in the Linux bridge:
* IPv6 all-hosts (ff02::1)
* IPv6 unregistered multicast
Currently we treat both as broadcast, but in case querier is present, we
can switch the second to use the multicast flood table where we only
flood to mrouter ports.
But I wonder if we can simplify it. I believe that the main reason we
take querier state into account in the data path is because multicast is
enabled by default on the bridge. Users that would otherwise explicitly
enable it would probably make sure they also have a querier in the
network.
The workaround I did in commit 9d45deb04c59 ("mlxsw: spectrum: Treat
IPv6 unregistered multicast as broadcast") was in response to blocked
IPv6 neighbour solicitation packets sent to the solicited-node multicast
address. I now see that Russel (Cc-ed) bumped into the same problem [1].
If we trap such packets at L2 (we need it for ND suppression anyway) and
let the Linux bridge take care of flooding them correctly, will it fix
the problem? I'm basically asking if after the proposed fix we will
still have basic scenarios broken by not taking querier state into
account.
Thanks
[1] https://patchwork.ozlabs.org/patch/1043694/
>
> This patch is meant about discussing the best way to solve the issue,
> it's not thoroughly tested, in case we settle about the details I'll run
> more tests.
>
> Thanks,
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> net/bridge/br_device.c | 5 +++--
> net/bridge/br_input.c | 5 +++--
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index 013323b6dbe4..e8c01409a7e7 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -96,8 +96,9 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
> }
>
> mdst = br_mdb_get(br, skb, vid);
> - if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
> - br_multicast_querier_exists(br, eth_hdr(skb)))
> + if (mdst ||
> + (BR_INPUT_SKB_CB_MROUTERS_ONLY(skb) &&
> + br_multicast_querier_exists(br, eth_hdr(skb))))
> br_multicast_flood(mdst, skb, false, true);
> else
> br_flood(br, skb, BR_PKT_MULTICAST, false, true);
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 5ea7e56119c1..8777566f7b6d 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -136,8 +136,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
> switch (pkt_type) {
> case BR_PKT_MULTICAST:
> mdst = br_mdb_get(br, skb, vid);
> - if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
> - br_multicast_querier_exists(br, eth_hdr(skb))) {
> + if (mdst ||
> + (BR_INPUT_SKB_CB_MROUTERS_ONLY(skb) &&
> + br_multicast_querier_exists(br, eth_hdr(skb)))) {
> if ((mdst && mdst->host_joined) ||
> br_multicast_is_router(br)) {
> local_rcv = true;
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH 3/8] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
From: Stefan Wahren @ 2019-02-19 8:56 UTC (permalink / raw)
To: Vasily Khoruzhick, Rob Herring
Cc: Mark Rutland, devicetree, Johan Hedberg, Maxime Ripard, netdev,
Marcel Holtmann, linux-bluetooth, Chen-Yu Tsai, David S. Miller,
arm-linux
In-Reply-To: <CA+E=qVdq5GORg-t-vVXM3zBxy3Aq93iCE+zmcGgLFBMcnTDgfw@mail.gmail.com>
Hi Vasily,
Am 18.02.19 um 22:24 schrieb Vasily Khoruzhick:
> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <robh@kernel.org> wrote:
>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
>>>
>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>> ---
>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
>>> 1 file changed, 35 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>> new file mode 100644
>>> index 000000000000..8357f242ae4c
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>> @@ -0,0 +1,35 @@
>>> +RTL8723BS/RTL8723CS Bluetooth
>>> +---------------------
>>> +
>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
>>> +to upload firmware and change module speed.
>>> +
>>> +Required properties:
>>> +
>>> + - compatible: should be one of the following:
>>> + * "realtek,rtl8723bs-bt"
>>> + * "realtek,rtl8723cs-bt"
>>> +Optional properties:
>>> +
>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
>>> + - firmware-postfix: firmware postfix to be used for firmware config
>> How is this used?
> rtl8723bs-bt needs 2 firmware binaries -- one is actual firmware,
> another is firmware config which is specific to the board. If
> firmware-postfix is specified, driver appends it to the name of config
> and requests board-specific config while loading firmware. I.e. if
> 'pine64' is specified as firmware-postfix driver will load
> rtl8723bs_config-pine64.bin.
this is a common problem (e.g. brcmfmac has the same problem [1]).
Why not using the "model" property instead of inventing a new software
specific property?
[1] - https://patchwork.kernel.org/patch/10764463/
>
>> Rob
>>
> _
^ permalink raw reply
* Re: [RFC v2] net: bridge: don't flood known multicast traffic when snooping is enabled
From: Linus Lüssing @ 2019-02-19 8:57 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, roopa, idosch, f.fainelli, bridge
In-Reply-To: <20190218122107.10097-1-nikolay@cumulusnetworks.com>
On Mon, Feb 18, 2019 at 02:21:07PM +0200, Nikolay Aleksandrov wrote:
> This is v2 of the RFC patch which aims to forward packets to known
> mdsts' ports only (the no querier case). After v1 I've kept
> the previous behaviour when it comes to unregistered traffic or when
> a querier is present. All of this is of course only with snooping
> enabled. So with this patch the following changes should occur:
> - No querier: forward known mdst traffic to its registered ports,
> no change about unknown mcast (flood)
> - Querier present: no change
>
> The reason to do this is simple - we want to respect the user's mdb
> configuration in both cases, that is if the user adds static mdb entries
> manually then we should use that information about forwarding traffic.
>
> What do you think ?
>
> * Notes
> Traffic that is currently marked as mrouters_only:
> - IPv4: non-local mcast traffic, igmp reports
> - IPv6: non-all-nodes-dst mcast traffic, mldv1 reports
>
> Simple use case:
> $ echo 1 > /sys/class/net/bridge/bridge/multicast_snooping
> $ bridge mdb add dev bridge port swp1 grp 239.0.0.1
> - without a querier currently traffic for 239.0.0.1 will still be flooded,
> with this change it will be forwarded only to swp1
There is still the issue with unsolicited reports adding mdst
entries here, too. Leading to unwanted packet loss and connectivity issues.
If I understand correctly, then the goal is to give the user
slightly more control over specific, dedicated multicast addresses, right?
Could you achieve the same via netfilter? Or is that rather unfeasible
with switchdev drivers? (sorry, don't have much
knowledge/experience with switchdev yet)
Regards, Linus
^ permalink raw reply
* Re: [PATCH v3] net: ns83820: code cleanup for ns83820_probe_phy()
From: Julia Lawall @ 2019-02-19 9:04 UTC (permalink / raw)
To: Mao Wenan
Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
daniel, ast, Julia Lawall
In-Reply-To: <20190219090635.134457-1-maowenan@huawei.com>
On Tue, 19 Feb 2019, Mao Wenan wrote:
> This patch is to do code cleanup for ns83820_probe_phy().
> It deletes unused variable 'first' and commented out code.
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> ---
> v2->v3: delte unused variable 'first'; change subject from
> "net: ns83820: drop pointless static qualifier in ns83820_probe_phy()" to
> "net: ns83820: code cleanup for ns83820_probe_phy()".
> drivers/net/ethernet/natsemi/ns83820.c | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
> index 958fced4dacf..955d34a6f0d8 100644
> --- a/drivers/net/ethernet/natsemi/ns83820.c
> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> @@ -1869,34 +1869,16 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
> static void ns83820_probe_phy(struct net_device *ndev)
> {
> struct ns83820 *dev = PRIV(ndev);
> - static int first;
> int i;
> #define MII_PHYIDR1 0x02
> #define MII_PHYIDR2 0x03
>
> -#if 0
> - if (!first) {
> - unsigned tmp;
> - ns83820_mii_read_reg(dev, 1, 0x09);
> - ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
> -
> - tmp = ns83820_mii_read_reg(dev, 1, 0x00);
> - ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
> - udelay(1300);
> - ns83820_mii_read_reg(dev, 1, 0x09);
> - }
> -#endif
> - first = 1;
> -
> for (i=1; i<2; i++) {
> int j;
> unsigned a, b;
> a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
> b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
>
> - //printk("%s: phy %d: 0x%04x 0x%04x\n",
> - // ndev->name, i, a, b);
> -
> for (j=0; j<0x16; j+=4) {
> dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
> ndev->name, j,
> --
> 2.20.1
>
>
^ permalink raw reply
* Re: general protection fault in tc_ctl_chain
From: Vlad Buslov @ 2019-02-19 9:10 UTC (permalink / raw)
To: Cong Wang
Cc: syzbot, David Miller, Jamal Hadi Salim, Jiri Pirko, LKML,
Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <CAM_iQpW656Aafj860Gen-KOg8Pojg=q9+hDTgPvoaSK8qsg79A@mail.gmail.com>
This is fixed by Dan Carpenter's patch "net: sched: potential NULL
dereference in tcf_block_find()" that was submitted yesterday.
On Mon 18 Feb 2019 at 20:02, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> (Cc'ing Vlad, please fix it)
>
> On Wed, Feb 13, 2019 at 9:56 AM syzbot
> <syzbot+eff9cae063e4b633c6c1@syzkaller.appspotmail.com> wrote:
>>
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: bd3606c29fcc rocker: Remove port_attr_bridge_flags_get ass..
>> git tree: net-next
>> console output: https://syzkaller.appspot.com/x/log.txt?x=121bbf87400000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=8572a6e4661225f4
>> dashboard link: https://syzkaller.appspot.com/bug?extid=eff9cae063e4b633c6c1
>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11cbd404c00000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17fc80d4c00000
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+eff9cae063e4b633c6c1@syzkaller.appspotmail.com
>>
>> audit: type=1800 audit(1550028783.638:30): pid=7517 uid=0 auid=4294967295
>> ses=4294967295 subj==unconfined op=collect_data cause=failed(directio)
>> comm="startpar" name="rmnologin" dev="sda1" ino=2423 res=0
>> kasan: CONFIG_KASAN_INLINE enabled
>> kasan: GPF could be caused by NULL-ptr deref or user memory access
>> general protection fault: 0000 [#1] PREEMPT SMP KASAN
>> CPU: 0 PID: 7669 Comm: syz-executor789 Not tainted 5.0.0-rc5+ #60
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>> Google 01/01/2011
>> RIP: 0010:__lock_acquire+0x8df/0x4700 kernel/locking/lockdep.c:3215
>> Code: 28 00 00 00 0f 85 35 27 00 00 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f
>> 5d c3 48 b8 00 00 00 00 00 fc ff df 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 0f
>> 85 dc 27 00 00 49 81 3c 24 20 45 9a 89 0f 84 03 f8
>> RSP: 0018:ffff88808b44f180 EFLAGS: 00010006
>> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
>> RDX: 000000000000000c RSI: 0000000000000000 RDI: 0000000000000060
>> RBP: ffff88808b44f350 R08: 0000000000000001 R09: 0000000000000001
>> R10: ffff88808b44f570 R11: 0000000000000001 R12: 0000000000000060
>> R13: 0000000000000000 R14: 0000000000000000 R15: ffff8880915d4680
>> FS: 0000000001ef6880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 0000000020000080 CR3: 0000000093549000 CR4: 00000000001406f0
>> Call Trace:
>> lock_acquire+0x16f/0x3f0 kernel/locking/lockdep.c:3841
>> __mutex_lock_common kernel/locking/mutex.c:925 [inline]
>> __mutex_lock+0xf7/0x1310 kernel/locking/mutex.c:1072
>> mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
>> tc_ctl_chain+0x42f/0x11a0 net/sched/cls_api.c:2812
>> rtnetlink_rcv_msg+0x465/0xb00 net/core/rtnetlink.c:5192
>> netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2485
>> rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5210
>> netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
>> netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
>> netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1925
>> sock_sendmsg_nosec net/socket.c:621 [inline]
>> sock_sendmsg+0xdd/0x130 net/socket.c:631
>> ___sys_sendmsg+0x806/0x930 net/socket.c:2136
>> __sys_sendmsg+0x105/0x1d0 net/socket.c:2174
>> __do_sys_sendmsg net/socket.c:2183 [inline]
>> __se_sys_sendmsg net/socket.c:2181 [inline]
>> __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2181
>> do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
>> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>> RIP: 0033:0x4400d9
>> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
>> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
>> ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
>> RSP: 002b:00007ffd09281608 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004400d9
>> RDX: 0000000000000000 RSI: 0000000020000080 RDI: 0000000000000003
>> RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
>> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401960
>> R13: 00000000004019f0 R14: 0000000000000000 R15: 0000000000000000
>> Modules linked in:
>> ---[ end trace 25ab48d993ef9249 ]---
>> RIP: 0010:__lock_acquire+0x8df/0x4700 kernel/locking/lockdep.c:3215
>> Code: 28 00 00 00 0f 85 35 27 00 00 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f
>> 5d c3 48 b8 00 00 00 00 00 fc ff df 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 0f
>> 85 dc 27 00 00 49 81 3c 24 20 45 9a 89 0f 84 03 f8
>> RSP: 0018:ffff88808b44f180 EFLAGS: 00010006
>> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
>> RDX: 000000000000000c RSI: 0000000000000000 RDI: 0000000000000060
>> RBP: ffff88808b44f350 R08: 0000000000000001 R09: 0000000000000001
>> R10: ffff88808b44f570 R11: 0000000000000001 R12: 0000000000000060
>> R13: 0000000000000000 R14: 0000000000000000 R15: ffff8880915d4680
>> FS: 0000000001ef6880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 0000000020000080 CR3: 0000000093549000 CR4: 00000000001406f0
>>
>>
>> ---
>> This bug is generated by a bot. It may contain errors.
>> See https://goo.gl/tpsmEJ for more information about syzbot.
>> syzbot engineers can be reached at syzkaller@googlegroups.com.
>>
>> syzbot will keep track of this bug report. See:
>> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
>> syzbot.
>> syzbot can test patches for this bug, for details see:
>> https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Handling an Extra Signal at PHY Reset
From: Paul Kocialkowski @ 2019-02-19 9:14 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Heiner Kallweit
Cc: netdev, Thomas Petazzoni, Mylène Josserand
Hi,
We are dealing with an Ethernet PHY (Marvell 88E1512) that comes with a
CONFIG pin that must be connected to one of the other pins of the PHY
to configure the LSB of the PHY address as well as I/O voltages (see
section 2.18.1 Hardware Configuration of the datasheet). It must be
connected "soon after reset" for the PHY to be correctly configured.
We have a switch for connecting the CONFIG pin to the other pin (LED0),
which needs to be controlled by Linux. The CONFIG pin seems to be used
for a PTP clock the rest of the time.
So we are wondering how to properly represent this case, especially on
the device-tree side.
The trick here is that this step is necessary before the PHY can be
discovered on the MDIO bus (and thus the PHY driver selected) so we
can't rely on the PHY driver to do this. Basically, it looks like we
need to handle this like the reset pin and describe it at the MDIO bus
level.
Here are some ideas for potential solutions:
- Allowing more than a single GPIO to be passed to the MDIO bus' reset-
gpios via device-tree and toggling all the passed GPIOs at once;
- Adding a new optional GPIO for the MDIO bus dedicated to controlling
switches for such config switching, perhaps called "config-gpios"
(quite a narrow solution);
- Adding a broader power sequence description to the MDIO bus (a bit
like it's done with the mmc pwrseq descriptions) which would allow
specifying the toggle order/delays of various GPIOs (would probably be
the most extensive solution);
- Adding the extra GPIO control to the MAC description and toggling it
through bus->reset (probably the less invasive solution for the core
but not very satisfying from the description perspective, since this is
definitely not MAC-specific).
What do you think about how we could solve this issue?
Do you see other options that I missed here?
Cheers and thanks in advance,
Paul
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v2] bonding: fix PACKET_ORIGDEV regression
From: Maciej Żenczykowski @ 2019-02-19 9:14 UTC (permalink / raw)
To: Michal Soltys
Cc: David Miller, Jay Vosburgh, Vincent Bernat, Mahesh Bandewar,
Chonggang Li, Linux NetDev
In-Reply-To: <20190218165528.15575-1-soltys@ziu.info>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
It certainly seems like this should do the trick, but I do agree some
sort of tests would be nice.
^ permalink raw reply
* Re: general protection fault in tc_ctl_chain
From: Dmitry Vyukov @ 2019-02-19 9:15 UTC (permalink / raw)
To: Vlad Buslov, Dan Carpenter
Cc: Cong Wang, syzbot, David Miller, Jamal Hadi Salim, Jiri Pirko,
LKML, Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <vbfa7isqh3r.fsf@mellanox.com>
On Tue, Feb 19, 2019 at 10:10 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> This is fixed by Dan Carpenter's patch "net: sched: potential NULL
> dereference in tcf_block_find()" that was submitted yesterday.
+Dan
Let's tell syzbot that this is fixed:
#syz fix: net: sched: potential NULL dereference in tcf_block_find()
> On Mon 18 Feb 2019 at 20:02, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > (Cc'ing Vlad, please fix it)
> >
> > On Wed, Feb 13, 2019 at 9:56 AM syzbot
> > <syzbot+eff9cae063e4b633c6c1@syzkaller.appspotmail.com> wrote:
> >>
> >> Hello,
> >>
> >> syzbot found the following crash on:
> >>
> >> HEAD commit: bd3606c29fcc rocker: Remove port_attr_bridge_flags_get ass..
> >> git tree: net-next
> >> console output: https://syzkaller.appspot.com/x/log.txt?x=121bbf87400000
> >> kernel config: https://syzkaller.appspot.com/x/.config?x=8572a6e4661225f4
> >> dashboard link: https://syzkaller.appspot.com/bug?extid=eff9cae063e4b633c6c1
> >> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> >> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11cbd404c00000
> >> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17fc80d4c00000
> >>
> >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> >> Reported-by: syzbot+eff9cae063e4b633c6c1@syzkaller.appspotmail.com
> >>
> >> audit: type=1800 audit(1550028783.638:30): pid=7517 uid=0 auid=4294967295
> >> ses=4294967295 subj==unconfined op=collect_data cause=failed(directio)
> >> comm="startpar" name="rmnologin" dev="sda1" ino=2423 res=0
> >> kasan: CONFIG_KASAN_INLINE enabled
> >> kasan: GPF could be caused by NULL-ptr deref or user memory access
> >> general protection fault: 0000 [#1] PREEMPT SMP KASAN
> >> CPU: 0 PID: 7669 Comm: syz-executor789 Not tainted 5.0.0-rc5+ #60
> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> >> Google 01/01/2011
> >> RIP: 0010:__lock_acquire+0x8df/0x4700 kernel/locking/lockdep.c:3215
> >> Code: 28 00 00 00 0f 85 35 27 00 00 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f
> >> 5d c3 48 b8 00 00 00 00 00 fc ff df 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 0f
> >> 85 dc 27 00 00 49 81 3c 24 20 45 9a 89 0f 84 03 f8
> >> RSP: 0018:ffff88808b44f180 EFLAGS: 00010006
> >> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
> >> RDX: 000000000000000c RSI: 0000000000000000 RDI: 0000000000000060
> >> RBP: ffff88808b44f350 R08: 0000000000000001 R09: 0000000000000001
> >> R10: ffff88808b44f570 R11: 0000000000000001 R12: 0000000000000060
> >> R13: 0000000000000000 R14: 0000000000000000 R15: ffff8880915d4680
> >> FS: 0000000001ef6880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >> CR2: 0000000020000080 CR3: 0000000093549000 CR4: 00000000001406f0
> >> Call Trace:
> >> lock_acquire+0x16f/0x3f0 kernel/locking/lockdep.c:3841
> >> __mutex_lock_common kernel/locking/mutex.c:925 [inline]
> >> __mutex_lock+0xf7/0x1310 kernel/locking/mutex.c:1072
> >> mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
> >> tc_ctl_chain+0x42f/0x11a0 net/sched/cls_api.c:2812
> >> rtnetlink_rcv_msg+0x465/0xb00 net/core/rtnetlink.c:5192
> >> netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2485
> >> rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5210
> >> netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
> >> netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
> >> netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1925
> >> sock_sendmsg_nosec net/socket.c:621 [inline]
> >> sock_sendmsg+0xdd/0x130 net/socket.c:631
> >> ___sys_sendmsg+0x806/0x930 net/socket.c:2136
> >> __sys_sendmsg+0x105/0x1d0 net/socket.c:2174
> >> __do_sys_sendmsg net/socket.c:2183 [inline]
> >> __se_sys_sendmsg net/socket.c:2181 [inline]
> >> __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2181
> >> do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
> >> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> >> RIP: 0033:0x4400d9
> >> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
> >> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> >> ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> >> RSP: 002b:00007ffd09281608 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> >> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004400d9
> >> RDX: 0000000000000000 RSI: 0000000020000080 RDI: 0000000000000003
> >> RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
> >> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401960
> >> R13: 00000000004019f0 R14: 0000000000000000 R15: 0000000000000000
> >> Modules linked in:
> >> ---[ end trace 25ab48d993ef9249 ]---
> >> RIP: 0010:__lock_acquire+0x8df/0x4700 kernel/locking/lockdep.c:3215
> >> Code: 28 00 00 00 0f 85 35 27 00 00 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f
> >> 5d c3 48 b8 00 00 00 00 00 fc ff df 4c 89 e2 48 c1 ea 03 <80> 3c 02 00 0f
> >> 85 dc 27 00 00 49 81 3c 24 20 45 9a 89 0f 84 03 f8
> >> RSP: 0018:ffff88808b44f180 EFLAGS: 00010006
> >> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
> >> RDX: 000000000000000c RSI: 0000000000000000 RDI: 0000000000000060
> >> RBP: ffff88808b44f350 R08: 0000000000000001 R09: 0000000000000001
> >> R10: ffff88808b44f570 R11: 0000000000000001 R12: 0000000000000060
> >> R13: 0000000000000000 R14: 0000000000000000 R15: ffff8880915d4680
> >> FS: 0000000001ef6880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >> CR2: 0000000020000080 CR3: 0000000093549000 CR4: 00000000001406f0
> >>
> >>
> >> ---
> >> This bug is generated by a bot. It may contain errors.
> >> See https://goo.gl/tpsmEJ for more information about syzbot.
> >> syzbot engineers can be reached at syzkaller@googlegroups.com.
> >>
> >> syzbot will keep track of this bug report. See:
> >> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> >> syzbot.
> >> syzbot can test patches for this bug, for details see:
> >> https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [Bridge] [RFC v2] net: bridge: don't flood known multicast traffic when snooping is enabled
From: Linus Lüssing @ 2019-02-19 9:21 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: bridge, netdev, roopa, f.fainelli, idosch
In-Reply-To: <20190219085716.GD10191@otheros>
On Tue, Feb 19, 2019 at 09:57:16AM +0100, Linus Lüssing wrote:
> On Mon, Feb 18, 2019 at 02:21:07PM +0200, Nikolay Aleksandrov wrote:
> > This is v2 of the RFC patch which aims to forward packets to known
> > mdsts' ports only (the no querier case). After v1 I've kept
> > the previous behaviour when it comes to unregistered traffic or when
> > a querier is present. All of this is of course only with snooping
> > enabled. So with this patch the following changes should occur:
> > - No querier: forward known mdst traffic to its registered ports,
> > no change about unknown mcast (flood)
> > - Querier present: no change
> >
> > The reason to do this is simple - we want to respect the user's mdb
> > configuration in both cases, that is if the user adds static mdb entries
> > manually then we should use that information about forwarding traffic.
> >
> > What do you think ?
> >
> > * Notes
> > Traffic that is currently marked as mrouters_only:
> > - IPv4: non-local mcast traffic, igmp reports
> > - IPv6: non-all-nodes-dst mcast traffic, mldv1 reports
> >
> > Simple use case:
> > $ echo 1 > /sys/class/net/bridge/bridge/multicast_snooping
> > $ bridge mdb add dev bridge port swp1 grp 239.0.0.1
> > - without a querier currently traffic for 239.0.0.1 will still be flooded,
> > with this change it will be forwarded only to swp1
>
> There is still the issue with unsolicited reports adding mdst
> entries here, too. Leading to unwanted packet loss and connectivity issues.
Or in other words, an unsolicited report will turn a previously
unregistered multicast group into a registered one. However in the
absence of a querier the knowledge about this newly registered multicast group
will be incomplete. And therefore still needs to be flooded to avoid packet
loss.
^ permalink raw reply
* Re: [PATCH AUTOSEL 3.18 12/16] sfc: suppress duplicate nvmem partition types in efx_ef10_mtd_probe
From: Bert Kenward @ 2019-02-19 9:22 UTC (permalink / raw)
To: Sasha Levin, linux-kernel, stable; +Cc: Edward Cree, David S . Miller, netdev
In-Reply-To: <20190215021546.179605-12-sashal@kernel.org>
On 15/02/2019 02:15, Sasha Levin wrote:
> From: Edward Cree <ecree@solarflare.com>
>
> [ Upstream commit 3366463513f544c12c6b88c13da4462ee9e7a1a1 ]
>
> Use a bitmap to keep track of which partition types we've already seen;
> for duplicates, return -EEXIST from efx_ef10_mtd_probe_partition() and
> thus skip adding that partition.
> Duplicate partitions occur because of the A/B backup scheme used by newer
> sfc NICs. Prior to this patch they cause sysfs_warn_dup errors because
> they have the same name, causing us not to expose any MTDs at all.
>
> Signed-off-by: Edward Cree <ecree@solarflare.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
I don't think this particularly needs to go to stable, but if it does it
should be accompanied by:
c65285428b6e ("sfc: initialise found bitmap in efx_ef10_mtd_probe")
Bert.
> ---
> drivers/net/ethernet/sfc/ef10.c | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
> index 010009d64017..84a17b41313c 100644
> --- a/drivers/net/ethernet/sfc/ef10.c
> +++ b/drivers/net/ethernet/sfc/ef10.c
> @@ -3407,22 +3407,25 @@ static const struct efx_ef10_nvram_type_info efx_ef10_nvram_types[] = {
> { NVRAM_PARTITION_TYPE_LICENSE, 0, 0, "sfc_license" },
> { NVRAM_PARTITION_TYPE_PHY_MIN, 0xff, 0, "sfc_phy_fw" },
> };
> +#define EF10_NVRAM_PARTITION_COUNT ARRAY_SIZE(efx_ef10_nvram_types)
>
> static int efx_ef10_mtd_probe_partition(struct efx_nic *efx,
> struct efx_mcdi_mtd_partition *part,
> - unsigned int type)
> + unsigned int type,
> + unsigned long *found)
> {
> MCDI_DECLARE_BUF(inbuf, MC_CMD_NVRAM_METADATA_IN_LEN);
> MCDI_DECLARE_BUF(outbuf, MC_CMD_NVRAM_METADATA_OUT_LENMAX);
> const struct efx_ef10_nvram_type_info *info;
> size_t size, erase_size, outlen;
> + int type_idx = 0;
> bool protected;
> int rc;
>
> - for (info = efx_ef10_nvram_types; ; info++) {
> - if (info ==
> - efx_ef10_nvram_types + ARRAY_SIZE(efx_ef10_nvram_types))
> + for (type_idx = 0; ; type_idx++) {
> + if (type_idx == EF10_NVRAM_PARTITION_COUNT)
> return -ENODEV;
> + info = efx_ef10_nvram_types + type_idx;
> if ((type & ~info->type_mask) == info->type)
> break;
> }
> @@ -3435,6 +3438,13 @@ static int efx_ef10_mtd_probe_partition(struct efx_nic *efx,
> if (protected)
> return -ENODEV; /* hide it */
>
> + /* If we've already exposed a partition of this type, hide this
> + * duplicate. All operations on MTDs are keyed by the type anyway,
> + * so we can't act on the duplicate.
> + */
> + if (__test_and_set_bit(type_idx, found))
> + return -EEXIST;
> +
> part->nvram_type = type;
>
> MCDI_SET_DWORD(inbuf, NVRAM_METADATA_IN_TYPE, type);
> @@ -3463,6 +3473,7 @@ static int efx_ef10_mtd_probe_partition(struct efx_nic *efx,
> static int efx_ef10_mtd_probe(struct efx_nic *efx)
> {
> MCDI_DECLARE_BUF(outbuf, MC_CMD_NVRAM_PARTITIONS_OUT_LENMAX);
> + DECLARE_BITMAP(found, EF10_NVRAM_PARTITION_COUNT);
> struct efx_mcdi_mtd_partition *parts;
> size_t outlen, n_parts_total, i, n_parts;
> unsigned int type;
> @@ -3491,11 +3502,13 @@ static int efx_ef10_mtd_probe(struct efx_nic *efx)
> for (i = 0; i < n_parts_total; i++) {
> type = MCDI_ARRAY_DWORD(outbuf, NVRAM_PARTITIONS_OUT_TYPE_ID,
> i);
> - rc = efx_ef10_mtd_probe_partition(efx, &parts[n_parts], type);
> - if (rc == 0)
> - n_parts++;
> - else if (rc != -ENODEV)
> + rc = efx_ef10_mtd_probe_partition(efx, &parts[n_parts], type,
> + found);
> + if (rc == -EEXIST || rc == -ENODEV)
> + continue;
> + if (rc)
> goto fail;
> + n_parts++;
> }
>
> rc = efx_mtd_add(efx, &parts[0].common, n_parts, sizeof(*parts));
>
^ permalink raw reply
* Re: [PATCH] brcmfmac: remove set but not used variable 'old_state'
From: Arend Van Spriel @ 2019-02-19 9:23 UTC (permalink / raw)
To: YueHaibing, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, Kalle Valo
Cc: linux-wireless, netdev, brcm80211-dev-list.pdl,
brcm80211-dev-list, kernel-janitors
In-Reply-To: <20190218080846.187942-1-yuehaibing@huawei.com>
On 2/18/2019 9:08 AM, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c: In function 'brcmf_usb_state_change':
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c:578:6: warning:
> variable 'old_state' set but not used [-Wunused-but-set-variable]
>
> It's never used and can be removed.
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 2 --
> 1 file changed, 2 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next 3/3] nfp: devlink: allow flashing the device via devlink
From: Jiri Pirko @ 2019-02-19 9:19 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, netdev, oss-drivers, mkubecek, andrew
In-Reply-To: <20190215074429.6890e47e@cakuba.netronome.com>
Fri, Feb 15, 2019 at 04:44:29PM CET, jakub.kicinski@netronome.com wrote:
>On Fri, 15 Feb 2019 11:15:14 +0100, Jiri Pirko wrote:
>> > static const struct ethtool_ops nfp_net_ethtool_ops = {
>>
>> Why don't you use the compat fallback? I think you should.
>
>You and Michal both asked the same so let me answer the first to ask :)
>- if devlink is built as a module the fallback is not reachable.
So the fallback is not really good as you can't use it for real drivers
anyway. Odd. Maybe we should compile devlink in without possibility to
have it as module.
^ permalink raw reply
* Re: [PATCH net] mlxsw: __mlxsw_sp_port_headroom_set(): Fix a use of local variable
From: Ido Schimmel @ 2019-02-19 9:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org, Jiri Pirko, Petr Machata, mlxsw
In-Reply-To: <20190217.101440.744217887687576351.davem@davemloft.net>
On Sun, Feb 17, 2019 at 10:14:40AM -0800, David Miller wrote:
> From: Ido Schimmel <idosch@mellanox.com>
> Date: Sun, 17 Feb 2019 07:18:41 +0000
>
> > From: Petr Machata <petrm@mellanox.com>
> >
> > The function-local variable "delay" enters the loop interpreted as delay
> > in bits. However, inside the loop it gets overwritten by the result of
> > mlxsw_sp_pg_buf_delay_get(), and thus leaves the loop as quantity in
> > cells. Thus on second and further loop iterations, the headroom for a
> > given priority is configured with a wrong size.
> >
> > Fix by introducing a loop-local variable, delay_cells. Rename thres to
> > thres_cells for consistency.
> >
> > Fixes: f417f04da589 ("mlxsw: spectrum: Refactor port buffer configuration")
> > Signed-off-by: Petr Machata <petrm@mellanox.com>
> > Acked-by: Jiri Pirko <jiri@mellanox.com>
> > Signed-off-by: Ido Schimmel <idosch@mellanox.com>
>
> Applied and queued up for -stable, thanks Ido.
Hi David,
We have a series for net-next that adds support for Spectrum-2 shared
buffers and it depends on this patch. I was wondering if you could merge
net into net-next today or later this week, so that we could submit it.
Normally I would just wait for the merge to happen, but it looks like
this is going to be the last week to submit changes and I prefer not to
miss the window while waiting for the inevitable merge.
Thanks and sorry about the noise.
^ permalink raw reply
* Re: Handling an Extra Signal at PHY Reset
From: Thomas Petazzoni @ 2019-02-19 9:36 UTC (permalink / raw)
To: Paul Kocialkowski
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, netdev,
Mylène Josserand
In-Reply-To: <e647050a0fbadb8445cf6a7a5a2ccfbfd0865592.camel@bootlin.com>
Hello Paul,
On Tue, 19 Feb 2019 10:14:20 +0100
Paul Kocialkowski <paul.kocialkowski@bootlin.com> wrote:
> We are dealing with an Ethernet PHY (Marvell 88E1512) that comes with a
> CONFIG pin that must be connected to one of the other pins of the PHY
> to configure the LSB of the PHY address as well as I/O voltages (see
> section 2.18.1 Hardware Configuration of the datasheet). It must be
> connected "soon after reset" for the PHY to be correctly configured.
>
> We have a switch for connecting the CONFIG pin to the other pin (LED0),
> which needs to be controlled by Linux. The CONFIG pin seems to be used
> for a PTP clock the rest of the time.
>
> So we are wondering how to properly represent this case, especially on
> the device-tree side.
>
> The trick here is that this step is necessary before the PHY can be
> discovered on the MDIO bus (and thus the PHY driver selected) so we
> can't rely on the PHY driver to do this. Basically, it looks like we
> need to handle this like the reset pin and describe it at the MDIO bus
> level.
>
> Here are some ideas for potential solutions:
> - Allowing more than a single GPIO to be passed to the MDIO bus' reset-
> gpios via device-tree and toggling all the passed GPIOs at once;
>
> - Adding a new optional GPIO for the MDIO bus dedicated to controlling
> switches for such config switching, perhaps called "config-gpios"
> (quite a narrow solution);
>
> - Adding a broader power sequence description to the MDIO bus (a bit
> like it's done with the mmc pwrseq descriptions) which would allow
> specifying the toggle order/delays of various GPIOs (would probably be
> the most extensive solution);
>
> - Adding the extra GPIO control to the MAC description and toggling it
> through bus->reset (probably the less invasive solution for the core
> but not very satisfying from the description perspective, since this is
> definitely not MAC-specific).
>
> What do you think about how we could solve this issue?
> Do you see other options that I missed here?
I think it's important to mention the sequence that is needed:
1. assert reset
2. wait 10 us
3. switch config signal
4. deassert reset
5. wait 100us
6. de-switch config signal
I.e, the config signal needs to be switched properly before deasserting
reset, and then switched back to its original state so that the config
pin can be used for its normal (non-reset) purpose.
So the manipulation of the config signal is intertwined with the
assert/de-assert of the reset. So I don't see how your fourth option
would work for example. Am I missing something here ?
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH net-next v3 3/3] net: stmmac: dwxgmac2: Also use TBU interrupt to clean TX path
From: Jose Abreu @ 2019-02-19 9:38 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1550569066.git.joabreu@synopsys.com>
TBU interrupt is a normal interrupt and can be used to trigger the
cleaning of TX path. Lets check if it's active in DMA interrupt handler.
While at it, refactor a little bit the function:
- Don't check if RI is enabled because at function exit we will
only clear the interrupts that are enabled so, no event will
be missed.
In my tests withe XGMAC2 this increased performance.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 4 +++-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 8 +++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index d6bb953685fa..37d5e6fe7473 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -193,9 +193,10 @@
#define XGMAC_AIE BIT(14)
#define XGMAC_RBUE BIT(7)
#define XGMAC_RIE BIT(6)
+#define XGMAC_TBUE BIT(2)
#define XGMAC_TIE BIT(0)
#define XGMAC_DMA_INT_DEFAULT_EN (XGMAC_NIE | XGMAC_AIE | XGMAC_RBUE | \
- XGMAC_RIE | XGMAC_TIE)
+ XGMAC_RIE | XGMAC_TBUE | XGMAC_TIE)
#define XGMAC_DMA_CH_Rx_WATCHDOG(x) (0x0000313c + (0x80 * (x)))
#define XGMAC_RWT GENMASK(7, 0)
#define XGMAC_DMA_CH_STATUS(x) (0x00003160 + (0x80 * (x)))
@@ -204,6 +205,7 @@
#define XGMAC_FBE BIT(12)
#define XGMAC_RBU BIT(7)
#define XGMAC_RI BIT(6)
+#define XGMAC_TBU BIT(2)
#define XGMAC_TPS BIT(1)
#define XGMAC_TI BIT(0)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index c5e25580a43f..2ba712b48a89 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -283,12 +283,10 @@ static int dwxgmac2_dma_interrupt(void __iomem *ioaddr,
x->normal_irq_n++;
if (likely(intr_status & XGMAC_RI)) {
- if (likely(intr_en & XGMAC_RIE)) {
- x->rx_normal_irq_n++;
- ret |= handle_rx;
- }
+ x->rx_normal_irq_n++;
+ ret |= handle_rx;
}
- if (likely(intr_status & XGMAC_TI)) {
+ if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
x->tx_normal_irq_n++;
ret |= handle_tx;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v3 1/3] net: stmmac: Fix NAPI poll in TX path when in multi-queue
From: Jose Abreu @ 2019-02-19 9:38 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Jose Abreu, Florian Fainelli, Joao Pinto, David S . Miller,
Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <cover.1550569066.git.joabreu@synopsys.com>
Commit 8fce33317023 introduced the concept of NAPI per-channel and
independent cleaning of TX path.
This is currently breaking performance in some cases. The scenario
happens when all packets are being received in Queue 0 but the TX is
performed in Queue != 0.
Fix this by using different NAPI instances per each TX and RX queue, as
suggested by Florian.
Changes from v2:
- Only force restart transmission if there are pending packets
Changes from v1:
- Pass entire ring size to TX clean path (Florian)
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 5 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 115 +++++++++++++---------
2 files changed, 68 insertions(+), 52 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 63e1064b27a2..e697ecd9b0a6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -78,11 +78,10 @@ struct stmmac_rx_queue {
};
struct stmmac_channel {
- struct napi_struct napi ____cacheline_aligned_in_smp;
+ struct napi_struct rx_napi ____cacheline_aligned_in_smp;
+ struct napi_struct tx_napi ____cacheline_aligned_in_smp;
struct stmmac_priv *priv_data;
u32 index;
- int has_rx;
- int has_tx;
};
struct stmmac_tc_entry {
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 685d20472358..9d515b86e278 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -155,7 +155,10 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
for (queue = 0; queue < maxq; queue++) {
struct stmmac_channel *ch = &priv->channel[queue];
- napi_disable(&ch->napi);
+ if (queue < rx_queues_cnt)
+ napi_disable(&ch->rx_napi);
+ if (queue < tx_queues_cnt)
+ napi_disable(&ch->tx_napi);
}
}
@@ -173,7 +176,10 @@ static void stmmac_enable_all_queues(struct stmmac_priv *priv)
for (queue = 0; queue < maxq; queue++) {
struct stmmac_channel *ch = &priv->channel[queue];
- napi_enable(&ch->napi);
+ if (queue < rx_queues_cnt)
+ napi_enable(&ch->rx_napi);
+ if (queue < tx_queues_cnt)
+ napi_enable(&ch->tx_napi);
}
}
@@ -1939,6 +1945,10 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue)
mod_timer(&priv->eee_ctrl_timer, STMMAC_LPI_T(eee_timer));
}
+ /* We still have pending packets, let's call for a new scheduling */
+ if (tx_q->dirty_tx != tx_q->cur_tx)
+ mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10));
+
__netif_tx_unlock_bh(netdev_get_tx_queue(priv->dev, queue));
return count;
@@ -2029,23 +2039,15 @@ static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan)
int status = stmmac_dma_interrupt_status(priv, priv->ioaddr,
&priv->xstats, chan);
struct stmmac_channel *ch = &priv->channel[chan];
- bool needs_work = false;
-
- if ((status & handle_rx) && ch->has_rx) {
- needs_work = true;
- } else {
- status &= ~handle_rx;
- }
- if ((status & handle_tx) && ch->has_tx) {
- needs_work = true;
- } else {
- status &= ~handle_tx;
+ if ((status & handle_rx) && (chan < priv->plat->rx_queues_to_use)) {
+ stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
+ napi_schedule_irqoff(&ch->rx_napi);
}
- if (needs_work && napi_schedule_prep(&ch->napi)) {
+ if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use)) {
stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
- __napi_schedule(&ch->napi);
+ napi_schedule_irqoff(&ch->tx_napi);
}
return status;
@@ -2241,8 +2243,14 @@ static void stmmac_tx_timer(struct timer_list *t)
ch = &priv->channel[tx_q->queue_index];
- if (likely(napi_schedule_prep(&ch->napi)))
- __napi_schedule(&ch->napi);
+ /*
+ * If NAPI is already running we can miss some events. Let's rearm
+ * the timer and try again.
+ */
+ if (likely(napi_schedule_prep(&ch->tx_napi)))
+ __napi_schedule(&ch->tx_napi);
+ else
+ mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10));
}
/**
@@ -3498,7 +3506,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
else
skb->ip_summed = CHECKSUM_UNNECESSARY;
- napi_gro_receive(&ch->napi, skb);
+ napi_gro_receive(&ch->rx_napi, skb);
priv->dev->stats.rx_packets++;
priv->dev->stats.rx_bytes += frame_len;
@@ -3513,40 +3521,45 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
return count;
}
-/**
- * stmmac_poll - stmmac poll method (NAPI)
- * @napi : pointer to the napi structure.
- * @budget : maximum number of packets that the current CPU can receive from
- * all interfaces.
- * Description :
- * To look at the incoming frames and clear the tx resources.
- */
-static int stmmac_napi_poll(struct napi_struct *napi, int budget)
+static int stmmac_napi_poll_rx(struct napi_struct *napi, int budget)
{
struct stmmac_channel *ch =
- container_of(napi, struct stmmac_channel, napi);
+ container_of(napi, struct stmmac_channel, rx_napi);
struct stmmac_priv *priv = ch->priv_data;
- int work_done, rx_done = 0, tx_done = 0;
u32 chan = ch->index;
+ int work_done;
priv->xstats.napi_poll++;
- if (ch->has_tx)
- tx_done = stmmac_tx_clean(priv, budget, chan);
- if (ch->has_rx)
- rx_done = stmmac_rx(priv, budget, chan);
+ work_done = stmmac_rx(priv, budget, chan);
+ if (work_done < budget && napi_complete_done(napi, work_done))
+ stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
+ return work_done;
+}
- work_done = max(rx_done, tx_done);
- work_done = min(work_done, budget);
+static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget)
+{
+ struct stmmac_channel *ch =
+ container_of(napi, struct stmmac_channel, tx_napi);
+ struct stmmac_priv *priv = ch->priv_data;
+ struct stmmac_tx_queue *tx_q;
+ u32 chan = ch->index;
+ int work_done;
- if (work_done < budget && napi_complete_done(napi, work_done)) {
- int stat;
+ priv->xstats.napi_poll++;
+
+ work_done = stmmac_tx_clean(priv, DMA_TX_SIZE, chan);
+ work_done = min(work_done, budget);
+ if (work_done < budget && napi_complete_done(napi, work_done))
stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
- stat = stmmac_dma_interrupt_status(priv, priv->ioaddr,
- &priv->xstats, chan);
- if (stat && napi_reschedule(napi))
- stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
+
+ /* Force transmission restart */
+ tx_q = &priv->tx_queue[chan];
+ if (tx_q->cur_tx != tx_q->dirty_tx) {
+ stmmac_enable_dma_transmission(priv, priv->ioaddr);
+ stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr,
+ chan);
}
return work_done;
@@ -4323,13 +4336,14 @@ int stmmac_dvr_probe(struct device *device,
ch->priv_data = priv;
ch->index = queue;
- if (queue < priv->plat->rx_queues_to_use)
- ch->has_rx = true;
- if (queue < priv->plat->tx_queues_to_use)
- ch->has_tx = true;
-
- netif_napi_add(ndev, &ch->napi, stmmac_napi_poll,
- NAPI_POLL_WEIGHT);
+ if (queue < priv->plat->rx_queues_to_use) {
+ netif_napi_add(ndev, &ch->rx_napi, stmmac_napi_poll_rx,
+ NAPI_POLL_WEIGHT);
+ }
+ if (queue < priv->plat->tx_queues_to_use) {
+ netif_napi_add(ndev, &ch->tx_napi, stmmac_napi_poll_tx,
+ NAPI_POLL_WEIGHT);
+ }
}
mutex_init(&priv->lock);
@@ -4385,7 +4399,10 @@ int stmmac_dvr_probe(struct device *device,
for (queue = 0; queue < maxq; queue++) {
struct stmmac_channel *ch = &priv->channel[queue];
- netif_napi_del(&ch->napi);
+ if (queue < priv->plat->rx_queues_to_use)
+ netif_napi_del(&ch->rx_napi);
+ if (queue < priv->plat->tx_queues_to_use)
+ netif_napi_del(&ch->tx_napi);
}
error_hw_init:
destroy_workqueue(priv->wq);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v3 2/3] net: stmmac: dwmac4: Also use TBU interrupt to clean TX path
From: Jose Abreu @ 2019-02-19 9:38 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1550569066.git.joabreu@synopsys.com>
TBU interrupt is a normal interrupt and can be used to trigger the
cleaning of TX path. Lets check if it's active in DMA interrupt handler.
While at it, refactor a little bit the function:
- Don't check if RI is enabled because at function exit we will
only clear the interrupts that are enabled so, no event will be
missed.
In my tests with GMAC5 this increased performance.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
index 49f5687879df..545cb9c47433 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
@@ -124,9 +124,9 @@ void dwmac4_disable_dma_irq(void __iomem *ioaddr, u32 chan)
int dwmac4_dma_interrupt(void __iomem *ioaddr,
struct stmmac_extra_stats *x, u32 chan)
{
- int ret = 0;
-
u32 intr_status = readl(ioaddr + DMA_CHAN_STATUS(chan));
+ u32 intr_en = readl(ioaddr + DMA_CHAN_INTR_ENA(chan));
+ int ret = 0;
/* ABNORMAL interrupts */
if (unlikely(intr_status & DMA_CHAN_STATUS_AIS)) {
@@ -151,16 +151,11 @@ int dwmac4_dma_interrupt(void __iomem *ioaddr,
if (likely(intr_status & DMA_CHAN_STATUS_NIS)) {
x->normal_irq_n++;
if (likely(intr_status & DMA_CHAN_STATUS_RI)) {
- u32 value;
-
- value = readl(ioaddr + DMA_CHAN_INTR_ENA(chan));
- /* to schedule NAPI on real RIE event. */
- if (likely(value & DMA_CHAN_INTR_ENA_RIE)) {
- x->rx_normal_irq_n++;
- ret |= handle_rx;
- }
+ x->rx_normal_irq_n++;
+ ret |= handle_rx;
}
- if (likely(intr_status & DMA_CHAN_STATUS_TI)) {
+ if (likely(intr_status & (DMA_CHAN_STATUS_TI |
+ DMA_CHAN_STATUS_TBU))) {
x->tx_normal_irq_n++;
ret |= handle_tx;
}
@@ -168,12 +163,7 @@ int dwmac4_dma_interrupt(void __iomem *ioaddr,
x->rx_early_irq++;
}
- /* Clear the interrupt by writing a logic 1 to the chanX interrupt
- * status [21-0] expect reserved bits [5-3]
- */
- writel((intr_status & 0x3fffc7),
- ioaddr + DMA_CHAN_STATUS(chan));
-
+ writel(intr_status & intr_en, ioaddr + DMA_CHAN_STATUS(chan));
return ret;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v3 0/3] net: stmmac: Performance improvements in Multi-Queue
From: Jose Abreu @ 2019-02-19 9:38 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Jose Abreu, Florian Fainelli, Joao Pinto, David S . Miller,
Giuseppe Cavallaro, Alexandre Torgue
Tested in XGMAC2 and GMAC5.
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Jose Abreu (3):
net: stmmac: Fix NAPI poll in TX path when in multi-queue
net: stmmac: dwmac4: Also use TBU interrupt to clean TX path
net: stmmac: dwxgmac2: Also use TBU interrupt to clean TX path
drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 24 ++---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 4 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 8 +-
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 5 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 115 ++++++++++++---------
5 files changed, 81 insertions(+), 75 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH net-next 01/12] net: sched: flower: don't check for rtnl on head dereference
From: Vlad Buslov @ 2019-02-19 9:45 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <CAM_iQpXHLkwGC9T4231hQHeS8_8AyiA2+Pt-LqyjyeckKeUSzA@mail.gmail.com>
On Mon 18 Feb 2019 at 19:08, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Wed, Feb 13, 2019 at 11:47 PM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>> Flower classifier only changes root pointer during init and destroy. Cls
>> API implements reference counting for tcf_proto, so there is no danger of
>> concurrent access to tp when it is being destroyed, even without protection
>> provided by rtnl lock.
>
> How about atomicity? Refcnt doesn't guarantee atomicity, how do
> you make sure two concurrent modifications are atomic?
In order to guarantee atomicity I lock shared flower classifier data
structures with tp->lock in following patches.
>
>
>>
>> Implement new function fl_head_dereference() to dereference tp->root
>> without checking for rtnl lock. Use it in all flower function that obtain
>> head pointer instead of rtnl_dereference().
>>
>
> So what lock protects RCU writers after this patch?
I explained it in comment for fl_head_dereference(), but should have
copied this information to changelog as well:
Flower classifier only changes root pointer during init and destroy.
Cls API implements reference counting for tcf_proto, so there is no
danger of concurrent access to tp when it is being destroyed, even
without protection provided by rtnl lock.
In initial version of this change I used tp->lock to protect tp->root
access and verified it with lockdep, but during internal review Jiri
noted that this is not needed in current flower implementation.
^ permalink raw reply
* Re: [PATCH RESEND net] net: phy: xgmiitorgmii: Support generic PHY status read
From: Paul Kocialkowski @ 2019-02-19 9:56 UTC (permalink / raw)
To: Florian Fainelli, netdev, linux-arm-kernel, linux-kernel
Cc: Andrew Lunn, Heiner Kallweit, David S . Miller, Michal Simek,
Thomas Petazzoni
In-Reply-To: <958bb823-3dc8-607f-3c38-3d902acb85a8@gmail.com>
Hi,
On Fri, 2019-02-15 at 10:53 -0800, Florian Fainelli wrote:
> On 2/15/19 10:34 AM, Paul Kocialkowski wrote:
> > As I was mentionning to Andrew in the initial submission of this patch,
> > this driver is a bit unusual since it represents a GMII to RGMII
> > bridge, so it's not actually a PHY driver on its own -- it just sticks
> > itself in between the actual PHY and the MAC.
>
> Yes, my bad, you should still consider checking priv->phy_drv though, if
> someone unbinds the PHY driver on either side, you are toast.
Thanks for the suggestion! So I had a closer look at that driver to try
and see what could go wrong and it looks like I found a few things
there.
First, we have:
> priv->phy_dev->priv = priv;
Which basically overwrites that target PHY driver's priv with the
gmii2rgmii driver's. It looks like most PHY drivers don't use their
priv data so much so it kind of works in practice. But that's still a
receipe for disaster. I don't really see an immediate easy fix for that
one.
It might help to do things the other way round and bind the gmii2rgmii
PHY to the MAC, which itself would bind the actual PHY. That way we can
just have the gmii2rgmii redirect all ops to the actual PHY, except for
read_status. Maybe there was a reason I'm not seeing for doing things
the way they are done now though?
Then, it looks like there is no way for the gmii2rgmii driver to know
whether the PHY driver is still alive. It gets a pointer to the initial
priv->phy_dev->drv and then overwrites it. So when the target driver is
removed, the pointer will still be alive. Perhaps the memory backing it
will have been freed too.
How realistic does this scneario sound? I guess there are not many
cases where the PHY driver will be unregistered once it was picked up
by the gmii2rgmii driver, but I'm pretty new to the subsystem.
Cheers,
Paul
> > > > drivers/net/phy/xilinx_gmii2rgmii.c | 5 ++++-
> > > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
> > > > index 74a8782313cf..bd6084e315de 100644
> > > > --- a/drivers/net/phy/xilinx_gmii2rgmii.c
> > > > +++ b/drivers/net/phy/xilinx_gmii2rgmii.c
> > > > @@ -44,7 +44,10 @@ static int xgmiitorgmii_read_status(struct phy_device *phydev)
> > > > u16 val = 0;
> > > > int err;
> > > >
> > > > - err = priv->phy_drv->read_status(phydev);
> > > > + if (priv->phy_drv->read_status)
> > > > + err = priv->phy_drv->read_status(phydev);
> > > > + else
> > > > + err = genphy_read_status(phydev);
> > > > if (err < 0)
> > > > return err;
> > > >
> > > >
>
>
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH net-next 00/12] Refactor flower classifier to remove dependency on rtnl lock
From: Vlad Buslov @ 2019-02-19 10:00 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <CAM_iQpWpr_148Lnd-2Y9eybu7--bQEcQfUQC6GP-ViKUyX_xpA@mail.gmail.com>
On Mon 18 Feb 2019 at 19:15, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> Hi,
>
>> net/sched/cls_flower.c | 424 +++++++++++++++++++++++++++++++++++++------------
>> 1 file changed, 321 insertions(+), 103 deletions(-)
>>
>
> Given you change cls_flower so much, please also add a test case for
> verifying your changes, especially focusing on the atomicity of concurrent
> modifications.
>
> Thanks.
Will do.
^ permalink raw reply
* Re: [PATCH bpf-next 0/9] bpf: Network Resource Manager (NRM)
From: Daniel Borkmann @ 2019-02-19 10:13 UTC (permalink / raw)
To: brakmo
Cc: netdev, Martin Lau, Alexei Starovoitov, Daniel Borkmann, edumazet,
ycheng, ncardwell
In-Reply-To: <20190219053829.2086512-1-brakmo@fb.com>
On 02/19/2019 06:38 AM, brakmo wrote:
> Network Resource Manager is a framework for limiting the bandwidth used
> by v2 cgroups. It consists of 4 BPF helpers and a sample BPF program to
> limit egress bandwdith as well as a sample user program and script to
> simplify NRM testing.
>
> The sample NRM BPF program is not meant to be production quality, it is
> provided as proof of concept. A lot more information, including sample
> runs in some cases, are provided in the commit messages of the individual
> patches.
>
> Two more BPF programs, one to limit ingress and one that limits egress
> and uses fq's Earliest Departure Time feature, will be provided in an
> upcomming patchset.
>
> brakmo (9):
> bpf: Add bpf helper bpf_tcp_enter_cwr
> bpf: Test bpf_tcp_enter_cwr in test_verifier
> bpf: add bpf helper bpf_skb_set_ecn
> bpf: Add bpf helper bpf_tcp_check_probe_timer
> bpf: sync bpf.h to tools and update bpf_helpers.h
> bpf: Sample program to load cg skb BPF programs
> bpf: Sample NRM BPF program to limit egress bw
> bpf: User program for testing NRM
> bpf: NRM test script
>
> include/linux/bpf.h | 1 +
> include/uapi/linux/bpf.h | 27 +-
> kernel/bpf/verifier.c | 4 +
> net/core/filter.c | 70 ++++
> samples/bpf/Makefile | 7 +
> samples/bpf/do_nrm_test.sh | 429 +++++++++++++++++++
> samples/bpf/load_cg_skb.c | 109 +++++
> samples/bpf/nrm.c | 439 ++++++++++++++++++++
> samples/bpf/nrm.h | 31 ++
> samples/bpf/nrm_kern.h | 109 +++++
> samples/bpf/nrm_out_kern.c | 213 ++++++++++
> tools/include/uapi/linux/bpf.h | 27 +-
> tools/testing/selftests/bpf/bpf_helpers.h | 6 +
> tools/testing/selftests/bpf/verifier/sock.c | 33 ++
> 14 files changed, 1503 insertions(+), 2 deletions(-)
> create mode 100755 samples/bpf/do_nrm_test.sh
> create mode 100644 samples/bpf/load_cg_skb.c
> create mode 100644 samples/bpf/nrm.c
> create mode 100644 samples/bpf/nrm.h
> create mode 100644 samples/bpf/nrm_kern.h
> create mode 100644 samples/bpf/nrm_out_kern.c
Looks okay to me modulo few minor comments, but lets also Cc TCP maintainers
to given them a chance to comment/ACK helper functions.
Thanks,
Daniel
^ 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