Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 09/12] mlxsw: core: Extend hwmon interface with fan fault attribute
From: Andrew Lunn @ 2019-02-13 13:53 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev@vger.kernel.org, davem@davemloft.net, Jiri Pirko, mlxsw,
	Vadim Pasternak, Guenter Roeck
In-Reply-To: <20190213112814.32334-10-idosch@mellanox.com>

On Wed, Feb 13, 2019 at 11:28:53AM +0000, Ido Schimmel wrote:
> From: Vadim Pasternak <vadimp@mellanox.com>
> 
> Add new fan hwmon attribute for exposing fan faults (fault indication is
> read from Fan Out of Range Event Register).
> 
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>

Hi Ido

You should include the HWMON maintainer in the Cc: list.

I would not be too surprised if he says to use
hwmon_device_register_with_info().

	Andrew

^ permalink raw reply

* Re: KASAN: use-after-free Read in sctp_outq_tail
From: Marcelo Ricardo Leitner @ 2019-02-13 13:51 UTC (permalink / raw)
  To: Xin Long
  Cc: syzbot, davem, LKML, linux-sctp, network dev, Neil Horman,
	syzkaller-bugs, Vlad Yasevich
In-Reply-To: <CADvbK_dndFMxyUZk=k8qEy9=PNbLc81F++2L0fbdtUdbupq4Xg@mail.gmail.com>

On Wed, Feb 13, 2019 at 12:35:56PM +0800, Xin Long wrote:
> On Wed, Feb 13, 2019 at 4:00 AM syzbot
> <syzbot+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com> wrote:
> >
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit:    d4104460aec1 Add linux-next specific files for 20190211
> > git tree:       linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=14140124c00000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=c8a112d3b0d6719b
> > dashboard link: https://syzkaller.appspot.com/bug?extid=7823fa3f3e2d69341ea8
> > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> >
> > Unfortunately, I don't have any reproducer for this crash yet.
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com
> >
> > ==================================================================
> > BUG: KASAN: use-after-free in list_add_tail include/linux/list.h:93 [inline]
> > BUG: KASAN: use-after-free in sctp_outq_tail_data net/sctp/outqueue.c:105
> > [inline]
> > BUG: KASAN: use-after-free in sctp_outq_tail+0x816/0x930
> > net/sctp/outqueue.c:313
> > Read of size 8 at addr ffff88807b19a7b8 by task syz-executor.0/30745
> I think https://patchwork.ozlabs.org/patch/1040500/ will fix this.

I don't think so. Seems it will switch from use-after-free to NULL deref
instead with that patch.

> > CPU: 1 PID: 30745 Comm: syz-executor.0 Not tainted 5.0.0-rc5-next-20190211
> > #32
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > Call Trace:
> >   __dump_stack lib/dump_stack.c:77 [inline]
> >   dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> >   print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187
> >   kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
> >   __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132
> >   list_add_tail include/linux/list.h:93 [inline]
> >   sctp_outq_tail_data net/sctp/outqueue.c:105 [inline]
> >   sctp_outq_tail+0x816/0x930 net/sctp/outqueue.c:313
> >   sctp_cmd_send_msg net/sctp/sm_sideeffect.c:1109 [inline]
> >   sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1784 [inline]
> >   sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> >   sctp_do_sm+0x68e/0x5380 net/sctp/sm_sideeffect.c:1191
> >   sctp_primitive_SEND+0xa0/0xd0 net/sctp/primitive.c:178
> >   sctp_sendmsg_to_asoc+0xa63/0x17b0 net/sctp/socket.c:1955

sctp_sendmsg_to_asoc()
...
        if (sinfo->sinfo_stream >= asoc->stream.outcnt) {
                err = -EINVAL;
                goto err;
        }

        if (unlikely(!SCTP_SO(&asoc->stream, sinfo->sinfo_stream)->ext)) {
...

It should have aborted even if an old ->ext was still there because
outcnt is correctly updated. So somehow outcnt was wrong here.

sctp_stream_init()
...
        /* Filter out chunks queued on streams that won't exist anymore */
        sched->unsched_all(stream);
        sctp_stream_outq_migrate(stream, NULL, outcnt);   <--- [A]
        sched->sched_all(stream);

        ret = sctp_stream_alloc_out(stream, outcnt, gfp); <--- [B]
        if (ret)
                goto out;

        stream->outcnt = outcnt;                          <--- [C]
...

We have a problem here because [A] is freeing ->ext, but [B] can fail (ENOMEM),
which would lead it to not update outcnt in [C] even after the
changes already performed in [A].

It should handle the freeing of ->ext in sctp_stream_alloc_out()
instead, or allocate the flexarray earlier (so it can bail out before
freeing stuff).

> >   sctp_sendmsg+0x10a9/0x17e0 net/sctp/socket.c:2113
> >   inet_sendmsg+0x147/0x5d0 net/ipv4/af_inet.c:798
> >   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:0x457e39
> > Code: ad b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 7b b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> > RSP: 002b:00007fa9b8630c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> > RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000457e39
> > RDX: 0000000000000000 RSI: 0000000020000140 RDI: 0000000000000003
> > RBP: 000000000073bf00 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 00007fa9b86316d4
> > R13: 00000000004c4e2b R14: 00000000004d8ab8 R15: 00000000ffffffff
> >
> > Allocated by task 30745:
> >   save_stack+0x45/0xd0 mm/kasan/common.c:75
> >   set_track mm/kasan/common.c:87 [inline]
> >   __kasan_kmalloc mm/kasan/common.c:498 [inline]
> >   __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:471
> >   kasan_kmalloc+0x9/0x10 mm/kasan/common.c:506
> >   kmem_cache_alloc_trace+0x151/0x760 mm/slab.c:3615
> >   kmalloc include/linux/slab.h:548 [inline]
> >   kzalloc include/linux/slab.h:743 [inline]
> >   sctp_stream_init_ext+0x51/0x110 net/sctp/stream.c:172
> >   sctp_sendmsg_to_asoc+0x1273/0x17b0 net/sctp/socket.c:1896
> >   sctp_sendmsg+0x10a9/0x17e0 net/sctp/socket.c:2113
> >   inet_sendmsg+0x147/0x5d0 net/ipv4/af_inet.c:798
> >   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
> >
> > Freed by task 30745:
> >   save_stack+0x45/0xd0 mm/kasan/common.c:75
> >   set_track mm/kasan/common.c:87 [inline]
> >   __kasan_slab_free+0x102/0x150 mm/kasan/common.c:460
> >   kasan_slab_free+0xe/0x10 mm/kasan/common.c:468
> >   __cache_free mm/slab.c:3491 [inline]
> >   kfree+0xcf/0x230 mm/slab.c:3816
> >   sctp_stream_outq_migrate+0x3e6/0x540 net/sctp/stream.c:88
> >   sctp_stream_init+0xbc/0x410 net/sctp/stream.c:139
> >   sctp_process_init+0x21c3/0x2b20 net/sctp/sm_make_chunk.c:2466
> >   sctp_cmd_process_init net/sctp/sm_sideeffect.c:682 [inline]
> >   sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1410 [inline]
> >   sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> >   sctp_do_sm+0x3145/0x5380 net/sctp/sm_sideeffect.c:1191
> >   sctp_assoc_bh_rcv+0x343/0x660 net/sctp/associola.c:1074
> >   sctp_inq_push+0x1ea/0x290 net/sctp/inqueue.c:95
> >   sctp_backlog_rcv+0x196/0xbe0 net/sctp/input.c:354
> >   sk_backlog_rcv include/net/sock.h:937 [inline]
> >   __release_sock+0x12e/0x3a0 net/core/sock.c:2379
> >   release_sock+0x59/0x1c0 net/core/sock.c:2895
> >   sctp_wait_for_connect+0x316/0x540 net/sctp/socket.c:8998
> >   sctp_sendmsg_to_asoc+0x13e3/0x17b0 net/sctp/socket.c:1967
> >   sctp_sendmsg+0x10a9/0x17e0 net/sctp/socket.c:2113
> >   inet_sendmsg+0x147/0x5d0 net/ipv4/af_inet.c:798
> >   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
> >
> > The buggy address belongs to the object at ffff88807b19a780
> >   which belongs to the cache kmalloc-96 of size 96
> > The buggy address is located 56 bytes inside of
> >   96-byte region [ffff88807b19a780, ffff88807b19a7e0)
> > The buggy address belongs to the page:
> > page:ffffea0001ec6680 count:1 mapcount:0 mapping:ffff88812c3f04c0
> > index:0xffff88807b19a800
> > flags: 0x1fffc0000000200(slab)
> > raw: 01fffc0000000200 ffffea000262acc8 ffffea0001448348 ffff88812c3f04c0
> > raw: ffff88807b19a800 ffff88807b19a000 000000010000001d 0000000000000000
> > page dumped because: kasan: bad access detected
> >
> > Memory state around the buggy address:
> >   ffff88807b19a680: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> >   ffff88807b19a700: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > > ffff88807b19a780: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> >                                          ^
> >   ffff88807b19a800: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> >   ffff88807b19a880: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > ==================================================================
> >
> >
> > ---
> > 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.
> 

^ permalink raw reply

* Re: [PATCH 1/2] mac80211: Use linked list instead of rhashtable walk for mesh tables
From: Herbert Xu @ 2019-02-13 13:47 UTC (permalink / raw)
  To: David Miller, johannes, linux-wireless, netdev, j, tgraf,
	johannes.berg
In-Reply-To: <E1gtmu5-0001Vb-LI@gondobar>

On Wed, Feb 13, 2019 at 01:16:13PM +0800, Herbert Xu wrote:
> The mesh table code walks over hash tables for two purposes.  First of
> all it's used as part of a netlink dump process, but it is also used
> for looking up entries to delete using criteria other than the hash
> key.
> 
> The second purpose is directly contrary to the design specification
> of rhashtable walks.  It is only meant for use by netlink dumps.
> 
> This is because rhashtable is resizable and you cannot obtain a
> stable walk over it during a resize process.
> 
> In fact mesh's use of rhashtable for dumping is bogus too.  Rather
> than using rhashtable walk's iterator to keep track of the current
> position, it always converts the current position to an integer
> which defeats the purpose of the iterator.
> 
> Therefore this patch converts all uses of rhashtable walk into a
> simple linked list.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

OK this patch is broken because there is no locking on the linked
list.  I'll repost the series.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 2/2] mac80211: Free mpath object when rhashtable insertion fails (fwd)
From: Herbert Xu @ 2019-02-13 13:43 UTC (permalink / raw)
  To: Julia Lawall
  Cc: David Miller, johannes, linux-wireless, netdev, j, tgraf,
	johannes.berg, kbuild-all
In-Reply-To: <alpine.DEB.2.20.1902131349440.3561@hadrien>

On Wed, Feb 13, 2019 at 01:50:20PM +0100, Julia Lawall wrote:
> Perhaps a return is needed after line 436.

I don't think so.  The kfree only occurs in the case where the
hash table insertion fails.  While we should only return an error
if both the hash table insertion failed and the error is not due
to an existing entry (EEXIST).

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] rhashtable: Remove obsolete rhashtable_walk_init function
From: Herbert Xu @ 2019-02-13 13:41 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, David Miller, johannes, linux-wireless, netdev, j,
	tgraf, johannes.berg
In-Reply-To: <201902131934.29Pw8ywP%fengguang.wu@intel.com>

On Wed, Feb 13, 2019 at 07:20:51PM +0800, kbuild test robot wrote:
> Hi Herbert,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on v5.0-rc4 next-20190212]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Herbert-Xu/rhashtable-Remove-obsolete-rhashtable_walk_init-function/20190213-182336
> config: x86_64-lkp (attached as .config)
> compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    net/mac80211/mesh_pathtbl.c: In function '__mesh_path_lookup_by_idx':
> >> net/mac80211/mesh_pathtbl.c:256:8: error: implicit declaration of function 'rhashtable_walk_init'; did you mean 'rhashtable_walk_exit'? [-Werror=implicit-function-declaration]
>      ret = rhashtable_walk_init(&tbl->rhead, &iter, GFP_ATOMIC);
>            ^~~~~~~~~~~~~~~~~~~~
>            rhashtable_walk_exit
>    cc1: some warnings being treated as errors

This patch depends on a prior patch to mac80211.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode
From: Marc Gonzalez @ 2019-02-13 13:40 UTC (permalink / raw)
  To: Andrew Lunn, Niklas Cassel, Florian Fainelli
  Cc: Vinod Koul, David S Miller, linux-arm-msm, Bjorn Andersson,
	netdev, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi
In-Reply-To: <20190213132900.GA24589@lunn.ch>

On 13/02/2019 14:29, Andrew Lunn wrote:

>> So we have these modes:
>>
>> PHY_INTERFACE_MODE_RGMII: TX and RX delays disabled
>> PHY_INTERFACE_MODE_RGMII_ID: TX and RX delays enabled
>> PHY_INTERFACE_MODE_RGMII_RXID: RX delay enabled, TX delay disabled
>> PHY_INTERFACE_MODE_RGMII_TXID: TX delay enabled, RX delay disabled
>>
>> What I don't like with this patch, is that if we specify phy-mode
>> PHY_INTERFACE_MODE_RGMII_TXID, this patch will enable TX delay,
>> but RX delay will not be explicitly set.
> 
> That is not the behaviour we want. It is best to assume the device is
> in a random state, and correctly enable/disable all delays as
> requested. Only leave the hardware alone if PHY_INTERFACE_MODE_NA is
> used.

That's what my patch did:
https://www.spinics.net/lists/netdev/msg445053.html

But see Florian's remarks:
https://www.spinics.net/lists/netdev/msg445133.html

Regards.

^ permalink raw reply

* Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode
From: Andrew Lunn @ 2019-02-13 13:29 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Vinod Koul, David S Miller, linux-arm-msm, Bjorn Andersson,
	netdev, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
	Marc Gonzalez
In-Reply-To: <20190213131206.GA460@centauri.lan>

> So we have these modes:
> 
> PHY_INTERFACE_MODE_RGMII: TX and RX delays disabled
> PHY_INTERFACE_MODE_RGMII_ID: TX and RX delays enabled
> PHY_INTERFACE_MODE_RGMII_RXID: RX delay enabled, TX delay disabled
> PHY_INTERFACE_MODE_RGMII_TXID: TX delay enabled, RX delay disabled
> 
> What I don't like with this patch, is that if we specify phy-mode
> PHY_INTERFACE_MODE_RGMII_TXID, this patch will enable TX delay,
> but RX delay will not be explicitly set.

That is not the behaviour we want. It is best to assume the device is
in a random state, and correctly enable/disable all delays as
requested. Only leave the hardware alone if PHY_INTERFACE_MODE_NA is
used.

     Andrew

^ permalink raw reply

* Re: [PATCH 3/3 net-next] test_objagg: Uninitialized variable in error handling
From: Jiri Pirko @ 2019-02-13 13:16 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Jiri Pirko, netdev, kernel-janitors
In-Reply-To: <20190213085931.GD14113@kadam>

Wed, Feb 13, 2019 at 09:59:31AM CET, dan.carpenter@oracle.com wrote:
>We need to set the error message on this path otherwise some of the
>callers, such as test_hints_case(), print from an uninitialized pointer.
>
>We had a similar bug earlier and set "errmsg" to NULL in the caller,
>test_delta_action_item().  That code is no longer required so I have
>removed it.
>
>Fixes: 9069a3817d82 ("lib: objagg: implement optimization hints assembly and use hints for object creation")
>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH 2/3 net-next] test_objagg: Test the correct variable
From: Jiri Pirko @ 2019-02-13 13:16 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Jiri Pirko, netdev, kernel-janitors
In-Reply-To: <20190213085820.GC14113@kadam>

Wed, Feb 13, 2019 at 09:58:20AM CET, dan.carpenter@oracle.com wrote:
>There is a typo here.  We intended to check "objagg2" but we instead
>test "objagg" which is not an error pointer.
>
>Fixes: 9069a3817d82 ("lib: objagg: implement optimization hints assembly and use hints for object creation")
>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

Thanks.

^ permalink raw reply

* Re: [PATCH 1/3 net-next] lib: objagg: Fix an error code in objagg_hints_get()
From: Jiri Pirko @ 2019-02-13 13:15 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Jiri Pirko, netdev, Ido Schimmel, kernel-janitors
In-Reply-To: <20190213085650.GB14113@kadam>

Wed, Feb 13, 2019 at 09:56:50AM CET, dan.carpenter@oracle.com wrote:
>We need to set the error code on this path otherwise we return
>ERR_PTR(0) which would result in a NULL dereference in the caller.
>
>Fixes: 9069a3817d82 ("lib: objagg: implement optimization hints assembly and use hints for object creation")
>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

I have the same patch in my queue.

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net] net: dsa: bcm_sf2: potential array overflow in bcm_sf2_sw_suspend()
From: Vivien Didelot @ 2019-02-13 13:13 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Andrew Lunn, Florian Fainelli, David S. Miller, netdev,
	kernel-janitors
In-Reply-To: <20190213082304.GA14113@kadam>

On Wed, 13 Feb 2019 11:23:04 +0300, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> The value of ->num_ports comes from bcm_sf2_sw_probe() and it is less
> than or equal to DSA_MAX_PORTS.  The ds->ports[] array is used inside
> the dsa_is_user_port() and dsa_is_cpu_port() functions.  The ds->ports[]
> array is allocated in dsa_switch_alloc() and it has ds->num_ports
> elements so this leads to a static checker warning about a potential out
> of bounds read.
> 
> Fixes: 8cfa94984c9c ("net: dsa: bcm_sf2: add suspend/resume callbacks")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>

^ permalink raw reply

* Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode
From: Niklas Cassel @ 2019-02-13 13:12 UTC (permalink / raw)
  To: Vinod Koul
  Cc: David S Miller, linux-arm-msm, Bjorn Andersson, netdev,
	Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
	Marc Gonzalez
In-Reply-To: <20190212141922.12849-1-vkoul@kernel.org>

On Tue, Feb 12, 2019 at 07:49:22PM +0530, Vinod Koul wrote:
> Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
> should not have delay in PHY whereas RGMII_ID and RGMII_RXID/RGMII_TXID
> can have delay in phy.
> 
> So disable the delay only for RGMII mode and disable for other modes
> 
> Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
> Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  drivers/net/phy/at803x.c | 33 ++++++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 8ff12938ab47..7b54b54e3316 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -110,6 +110,18 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
>  	return phy_write(phydev, AT803X_DEBUG_DATA, val);
>  }
>  
> +static inline int at803x_enable_rx_delay(struct phy_device *phydev)
> +{
> +	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
> +				     AT803X_DEBUG_RX_CLK_DLY_EN);
> +}
> +
> +static inline int at803x_enable_tx_delay(struct phy_device *phydev)
> +{
> +	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
> +				     AT803X_DEBUG_TX_CLK_DLY_EN);
> +}
> +
>  static inline int at803x_disable_rx_delay(struct phy_device *phydev)
>  {
>  	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
> @@ -255,18 +267,25 @@ static int at803x_config_init(struct phy_device *phydev)
>  	if (ret < 0)
>  		return ret;
>  
> -	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
> -			phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> -			phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII) {
>  		ret = at803x_disable_rx_delay(phydev);
>  		if (ret < 0)
>  			return ret;
> +		ret = at803x_disable_tx_delay(phydev);
> +		if (ret < 0)
> +			return ret;
> +	};
> +
> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> +			phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
> +		ret = at803x_enable_rx_delay(phydev);
> +		if (ret < 0)
> +			return ret;
>  	}
>  
> -	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> -			phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> -			phydev->interface == PHY_INTERFACE_MODE_RGMII) {
> -		ret = at803x_disable_tx_delay(phydev);
> +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
> +			phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
> +		ret = at803x_enable_tx_delay(phydev);
>  		if (ret < 0)
>  			return ret;
>  	}

So we have these modes:

PHY_INTERFACE_MODE_RGMII: TX and RX delays disabled
PHY_INTERFACE_MODE_RGMII_ID: TX and RX delays enabled
PHY_INTERFACE_MODE_RGMII_RXID: RX delay enabled, TX delay disabled
PHY_INTERFACE_MODE_RGMII_TXID: TX delay enabled, RX delay disabled

What I don't like with this patch, is that if we specify phy-mode
PHY_INTERFACE_MODE_RGMII_TXID, this patch will enable TX delay,
but RX delay will not be explicitly set.

Thus it will use the default value of the PHY, and for this PHY
both TX and RX delays are enabled by default.

This means that someone specifying PHY_INTERFACE_MODE_RGMII_TXID
will actually be getting PHY_INTERFACE_MODE_RGMII_ID.


Marc's patch:
https://www.spinics.net/lists/netdev/msg445053.html

does explicitly either enable or disable each delay
(so it does not depend on default values).


However, Marc's patch was never merged, because someone reported
a regression on am335x-evm.


On Vinod's V1 submission Andrew replied that if this change
breaks some existing DT (because that DT specifies a phy-mode
that does not match with reality), then we should fix so that
DT specifies the correct phy-mode:
https://www.spinics.net/lists/netdev/msg542638.html

IMHO, this makes most sense, since if a DT specifies an
incorrect phy-mode, then that is what the user should get.


Kind regards,
Niklas

^ permalink raw reply

* Re: [PATCH 2/2] mac80211: Free mpath object when rhashtable insertion fails (fwd)
From: Julia Lawall @ 2019-02-13 12:50 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, johannes, linux-wireless, netdev, j, tgraf,
	johannes.berg, kbuild-all

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

Perhaps a return is needed after line 436.

julia

---------- Forwarded message ----------
Date: Wed, 13 Feb 2019 20:29:38 +0800
From: kbuild test robot <lkp@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH 2/2] mac80211: Free mpath object when rhashtable insertion
    fails

CC: kbuild-all@01.org
In-Reply-To: <E1gtmu6-0001Vl-O7@gondobar>
References: <E1gtmu6-0001Vl-O7@gondobar>
TO: Herbert Xu <herbert@gondor.apana.org.au>
CC: David Miller <davem@davemloft.net>, johannes@sipsolutions.net, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, j@w1.fi, tgraf@suug.ch, johannes.berg@intel.com
CC:

Hi Herbert,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mac80211-next/master]
[also build test WARNING on v5.0-rc4 next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Herbert-Xu/mac80211-Fix-incorrect-usage-of-rhashtable-walk-API/20190213-181325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago

>> net/mac80211/mesh_pathtbl.c:448:8-17: ERROR: reference preceded by free on line 436

# https://github.com/0day-ci/linux/commit/a0886e834aacf883ceaf0c34c842c4cdb4d318fd
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a0886e834aacf883ceaf0c34c842c4cdb4d318fd
vim +448 net/mac80211/mesh_pathtbl.c

b15dc38b9 Bob Copeland         2016-02-28  390
eb2b9311f Luis Carlos Cobo     2008-02-23  391  /**
eb2b9311f Luis Carlos Cobo     2008-02-23  392   * mesh_path_add - allocate and add a new path to the mesh path table
bf7cd94dc Johannes Berg        2013-02-15  393   * @dst: destination address of the path (ETH_ALEN length)
f698d856f Jasper Bryant-Greene 2008-08-03  394   * @sdata: local subif
eb2b9311f Luis Carlos Cobo     2008-02-23  395   *
af901ca18 André Goddard Rosa   2009-11-14  396   * Returns: 0 on success
eb2b9311f Luis Carlos Cobo     2008-02-23  397   *
eb2b9311f Luis Carlos Cobo     2008-02-23  398   * State: the initial state of the new path is set to 0
eb2b9311f Luis Carlos Cobo     2008-02-23  399   */
ae76eef02 Bob Copeland         2013-03-29  400  struct mesh_path *mesh_path_add(struct ieee80211_sub_if_data *sdata,
ae76eef02 Bob Copeland         2013-03-29  401  				const u8 *dst)
eb2b9311f Luis Carlos Cobo     2008-02-23  402  {
349eb8cf4 Johannes Berg        2011-05-14  403  	struct mesh_table *tbl;
eb2b9311f Luis Carlos Cobo     2008-02-23  404  	struct mesh_path *mpath, *new_mpath;
60854fd94 Bob Copeland         2016-03-02  405  	int ret;
eb2b9311f Luis Carlos Cobo     2008-02-23  406
b203ca391 Joe Perches          2012-05-08  407  	if (ether_addr_equal(dst, sdata->vif.addr))
eb2b9311f Luis Carlos Cobo     2008-02-23  408  		/* never add ourselves as neighbours */
ae76eef02 Bob Copeland         2013-03-29  409  		return ERR_PTR(-ENOTSUPP);
eb2b9311f Luis Carlos Cobo     2008-02-23  410
eb2b9311f Luis Carlos Cobo     2008-02-23  411  	if (is_multicast_ether_addr(dst))
ae76eef02 Bob Copeland         2013-03-29  412  		return ERR_PTR(-ENOTSUPP);
eb2b9311f Luis Carlos Cobo     2008-02-23  413
472dbc45d Johannes Berg        2008-09-11  414  	if (atomic_add_unless(&sdata->u.mesh.mpaths, 1, MESH_MAX_MPATHS) == 0)
ae76eef02 Bob Copeland         2013-03-29  415  		return ERR_PTR(-ENOSPC);
ae76eef02 Bob Copeland         2013-03-29  416
b15dc38b9 Bob Copeland         2016-02-28  417  	new_mpath = mesh_path_new(sdata, dst, GFP_ATOMIC);
402d7752e Pavel Emelyanov      2008-05-06  418  	if (!new_mpath)
60854fd94 Bob Copeland         2016-03-02  419  		return ERR_PTR(-ENOMEM);
402d7752e Pavel Emelyanov      2008-05-06  420
60854fd94 Bob Copeland         2016-03-02  421  	tbl = sdata->u.mesh.mesh_paths;
60854fd94 Bob Copeland         2016-03-02  422  	do {
60854fd94 Bob Copeland         2016-03-02  423  		ret = rhashtable_lookup_insert_fast(&tbl->rhead,
60854fd94 Bob Copeland         2016-03-02  424  						    &new_mpath->rhash,
60854fd94 Bob Copeland         2016-03-02  425  						    mesh_rht_params);
f84e71a94 Pavel Emelyanov      2008-05-06  426
60854fd94 Bob Copeland         2016-03-02  427  		if (ret == -EEXIST)
60854fd94 Bob Copeland         2016-03-02  428  			mpath = rhashtable_lookup_fast(&tbl->rhead,
60854fd94 Bob Copeland         2016-03-02  429  						       dst,
60854fd94 Bob Copeland         2016-03-02  430  						       mesh_rht_params);
05b0152f1 Herbert Xu           2019-02-13  431  		else if (!ret)
05b0152f1 Herbert Xu           2019-02-13  432  			hlist_add_head(&new_mpath->walk_list, &tbl->walk_head);
60854fd94 Bob Copeland         2016-03-02  433  	} while (unlikely(ret == -EEXIST && !mpath));
f5ea9120b Johannes Berg        2009-08-07  434
a0886e834 Herbert Xu           2019-02-13  435  	if (ret)
a0886e834 Herbert Xu           2019-02-13 @436  		kfree(new_mpath);
a0886e834 Herbert Xu           2019-02-13  437
a0886e834 Herbert Xu           2019-02-13  438  	if (ret != -EEXIST)
60854fd94 Bob Copeland         2016-03-02  439  		return ERR_PTR(ret);
ae76eef02 Bob Copeland         2013-03-29  440
60854fd94 Bob Copeland         2016-03-02  441  	/* At this point either new_mpath was added, or we found a
60854fd94 Bob Copeland         2016-03-02  442  	 * matching entry already in the table; in the latter case
60854fd94 Bob Copeland         2016-03-02  443  	 * free the unnecessary new entry.
60854fd94 Bob Copeland         2016-03-02  444  	 */
a0886e834 Herbert Xu           2019-02-13  445  	if (ret == -EEXIST)
60854fd94 Bob Copeland         2016-03-02  446  		new_mpath = mpath;
60854fd94 Bob Copeland         2016-03-02  447  	sdata->u.mesh.mesh_paths_generation++;
60854fd94 Bob Copeland         2016-03-02 @448  	return new_mpath;
18889231e Javier Cardona       2009-08-10  449  }
eb2b9311f Luis Carlos Cobo     2008-02-23  450

:::::: The code at line 448 was first introduced by commit
:::::: 60854fd94573f0d3b80b55b40cf0140a0430f3ab mac80211: mesh: convert path table to rhashtable

:::::: TO: Bob Copeland <me@bobcopeland.com>
:::::: CC: Johannes Berg <johannes.berg@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply

* [PATCH iproute2] ss: add option --tos for requesting ipv4 tos and ipv6 tclass
From: Konstantin Khlebnikov @ 2019-02-13 12:39 UTC (permalink / raw)
  To: Stephen Hemminger, netdev; +Cc: Eric Dumazet

Also show socket class_id/priority used by classful qdisc.
Kernel report this together with tclass since commit
("inet_diag: fix reporting cgroup classid and fallback to priority")

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 man/man8/ss.8 |   17 +++++++++++++++++
 misc/ss.c     |   27 +++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/man/man8/ss.8 b/man/man8/ss.8
index 553a6cf46f0e..9f21202de424 100644
--- a/man/man8/ss.8
+++ b/man/man8/ss.8
@@ -244,6 +244,23 @@ the pacing rate and max pacing rate
 a helper variable for TCP internal auto tuning socket receive buffer
 .RE
 .TP
+.B \-\-tos
+Show ToS and priority information. Below fields may appear:
+.RS
+.P
+.TP
+.B tos
+IPv4 Type-of-Service byte
+.P
+.TP
+.B tclass
+IPv6 Traffic Class byte
+.P
+.TP
+.B class_id
+Class id set by net_cls cgroup. If class is zero this shows priority set by SO_PRIORITY.
+.RE
+.TP
 .B \-K, \-\-kill
 Attempts to forcibly close sockets. This option displays sockets that are
 successfully closed and silently skips sockets that the kernel does not support
diff --git a/misc/ss.c b/misc/ss.c
index 3589ebedc5a0..9e821faf0d31 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -110,6 +110,7 @@ static int show_header = 1;
 static int follow_events;
 static int sctp_ino;
 static int show_tipcinfo;
+static int show_tos;
 
 enum col_id {
 	COL_NETID,
@@ -3008,6 +3009,15 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 		}
 	}
 
+	if (show_tos) {
+		if (tb[INET_DIAG_TOS])
+			out(" tos:%#x", rta_getattr_u8(tb[INET_DIAG_TOS]));
+		if (tb[INET_DIAG_TCLASS])
+			out(" tclass:%#x", rta_getattr_u8(tb[INET_DIAG_TCLASS]));
+		if (tb[INET_DIAG_CLASS_ID])
+			out(" class_id:%#x", rta_getattr_u32(tb[INET_DIAG_CLASS_ID]));
+	}
+
 	if (show_mem || (show_tcpinfo && s->type != IPPROTO_UDP)) {
 		out("\n\t");
 		if (s->type == IPPROTO_SCTP)
@@ -3058,6 +3068,11 @@ static int tcpdiag_send(int fd, int protocol, struct filter *f)
 		req.r.idiag_ext |= (1<<(INET_DIAG_CONG-1));
 	}
 
+	if (show_tos) {
+		req.r.idiag_ext |= (1<<(INET_DIAG_TOS-1));
+		req.r.idiag_ext |= (1<<(INET_DIAG_TCLASS-1));
+	}
+
 	iov[0] = (struct iovec){
 		.iov_base = &req,
 		.iov_len = sizeof(req)
@@ -3118,6 +3133,11 @@ static int sockdiag_send(int family, int fd, int protocol, struct filter *f)
 		req.r.idiag_ext |= (1<<(INET_DIAG_CONG-1));
 	}
 
+	if (show_tos) {
+		req.r.idiag_ext |= (1<<(INET_DIAG_TOS-1));
+		req.r.idiag_ext |= (1<<(INET_DIAG_TCLASS-1));
+	}
+
 	iov[0] = (struct iovec){
 		.iov_base = &req,
 		.iov_len = sizeof(req)
@@ -4661,6 +4681,7 @@ static void _usage(FILE *dest)
 "   -i, --info          show internal TCP information\n"
 "       --tipcinfo      show internal tipc socket information\n"
 "   -s, --summary       show socket usage summary\n"
+"       --tos           show tos and priority information\n"
 "   -b, --bpf           show bpf filter socket information\n"
 "   -E, --events        continually display sockets as they are destroyed\n"
 "   -Z, --context       display process SELinux security contexts\n"
@@ -4765,6 +4786,8 @@ static int scan_state(const char *state)
 #define OPT_TIPCSOCK 257
 #define OPT_TIPCINFO 258
 
+#define OPT_TOS 259
+
 static const struct option long_opts[] = {
 	{ "numeric", 0, 0, 'n' },
 	{ "resolve", 0, 0, 'r' },
@@ -4800,6 +4823,7 @@ static const struct option long_opts[] = {
 	{ "contexts", 0, 0, 'z' },
 	{ "net", 1, 0, 'N' },
 	{ "tipcinfo", 0, 0, OPT_TIPCINFO},
+	{ "tos", 0, 0, OPT_TOS },
 	{ "kill", 0, 0, 'K' },
 	{ "no-header", 0, 0, 'H' },
 	{ 0 }
@@ -4977,6 +5001,9 @@ int main(int argc, char *argv[])
 		case OPT_TIPCINFO:
 			show_tipcinfo = 1;
 			break;
+		case OPT_TOS:
+			show_tos = 1;
+			break;
 		case 'K':
 			current_filter.kill = 1;
 			break;


^ permalink raw reply related

* Re: [PATCH] inet_diag: fix reporting cgroup classid and fallback to priority
From: Konstantin Khlebnikov @ 2019-02-13 12:28 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, sashal, linux-sctp
In-Reply-To: <20190212.103700.893095897141469287.davem@davemloft.net>

On 12.02.2019 21:37, David Miller wrote:
> From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Date: Sat, 09 Feb 2019 13:35:52 +0300
> 
>> Field idiag_ext in struct inet_diag_req_v2 used as bitmap of requested
>> extensions has only 8 bits. Thus extensions starting from DCTCPINFO
>> cannot be requested directly. Some of them included into response
>> unconditionally or hook into some of lower 8 bits.
>>
>> Extension INET_DIAG_CLASS_ID has not way to request from the beginning.
>>
>> This patch bundle it with INET_DIAG_TCLASS (ipv6 tos), fixes space
>> reservation, and documents behavior for other extensions.
>>
>> Also this patch adds fallback to reporting socket priority. This filed
>> is more widely used for traffic classification because ipv4 sockets
>> automatically maps TOS to priority and default qdisc pfifo_fast knows
>> about that. But priority could be changed via setsockopt SO_PRIORITY so
>> INET_DIAG_TOS isn't enough for predicting class.
>>
>> Also cgroup2 obsoletes net_cls classid (it always zero), but we cannot
>> reuse this field for reporting cgroup2 id because it is 64-bit (ino+gen).
>>
>> So, after this patch INET_DIAG_CLASS_ID will report socket priority
>> for most common setup when net_cls isn't set and/or cgroup2 in use.
>>
>> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
>> Fixes: 0888e372c37f ("net: inet: diag: expose sockets cgroup classid")
> 
> Applied, and queued up for -stable.
> 
> Please always put the Fixes: tag first in the list of tags.  I fixed
> it up for you this time.

Ok. Never heard about that rule, checkpatch.pl doesn't complain about that too.

> 
> Thanks.
> 

^ permalink raw reply

* Re: [ISSUE][4.20.6] mlx5 and checksum failures
From: Ian Kumlien @ 2019-02-13 12:04 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: Cong Wang, David Miller, Saeed Mahameed,
	Linux Kernel Network Developers
In-Reply-To: <CAA85sZvw9DouvMLT8+TgS5Y-0kG-XSVbNmBweJVBjOSNQPovRw@mail.gmail.com>

One last update on this, 4.20.8 compiled with the same compiler works
- I still suspect that it was fixed by:
net/mlx5e: Force CHECKSUM_UNNECESSARY for short ethernet frames

Anyway, we can forget about it now ;)

On Sat, Feb 9, 2019 at 4:54 PM Ian Kumlien <ian.kumlien@gmail.com> wrote:
>
> On Fri, Feb 8, 2019 at 5:29 PM Ian Kumlien <ian.kumlien@gmail.com> wrote
> > On Thu, Feb 7, 2019 at 11:01 PM Ian Kumlien <ian.kumlien@gmail.com> wrote:
> > > On Thu, Feb 7, 2019 at 7:43 PM Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote:
> > > > On Thu, Feb 7, 2019 at 2:17 AM Ian Kumlien <ian.kumlien@gmail.com> wrote:
> > > > > On Thu, Feb 7, 2019 at 2:01 AM Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote:
> > > > > > On Wed, Feb 6, 2019 at 3:00 PM Ian Kumlien <ian.kumlien@gmail.com> wrote:
> > > > > > > It changes directly after the first hw checksum failure, I don't know why =/
> > > > > >
> > > > > > weird, Maybe a real check-summing issue/corruption on the PCI ?!
> > > > >
> > > > > Actually, it seems to have been introduced in 4.20.6 - 4.20.5 works just fine
> >
> > > > Great, the difference is only 120 patches.
> > > > that is bisect-able, it will only take 5 iterations to find the
> > > > offending commit.
> > >
> > > I just wish it wasn't a server that takes, what feels like 5 minutes to boot...
> > >
> > > All of these seas of sensors 2d and 3d... =P
> > >
> > > But, yep, that's the plan
> >
> > Huh, spent most of the day with two bisects and none of them yielded
> > any results....
> >
> > Looks like I'll have to start investigating the elrepo kernel-ml build =(
>
> Just realized that it's not an entirely fair comparison - since
> retpolines wasn't enabled, damned old compilers...

^ permalink raw reply

* Re: [PATCH bpf-next v4 0/2] libbpf: adding AF_XDP support
From: Jesper Dangaard Brouer @ 2019-02-13 11:55 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: Jonathan Lemon, Magnus Karlsson, Björn Töpel, ast,
	Daniel Borkmann, Network Development, Jakub Kicinski,
	Björn Töpel, Zhang, Qi Z, xiaolong.ye, brouer,
	xdp-newbies@vger.kernel.org
In-Reply-To: <CAJ8uoz19UjmEHTc28Qd_9KdY9D-ojXSBRTbmffRhUTX49mnWvg@mail.gmail.com>

On Wed, 13 Feb 2019 12:32:47 +0100
Magnus Karlsson <magnus.karlsson@gmail.com> wrote:

> On Mon, Feb 11, 2019 at 9:44 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> >
> > On 8 Feb 2019, at 5:05, Magnus Karlsson wrote:
> >  
> > > This patch proposes to add AF_XDP support to libbpf. The main reason
> > > for this is to facilitate writing applications that use AF_XDP by
> > > offering higher-level APIs that hide many of the details of the AF_XDP
> > > uapi. This is in the same vein as libbpf facilitates XDP adoption by
> > > offering easy-to-use higher level interfaces of XDP
> > > functionality. Hopefully this will facilitate adoption of AF_XDP, make
> > > applications using it simpler and smaller, and finally also make it
> > > possible for applications to benefit from optimizations in the AF_XDP
> > > user space access code. Previously, people just copied and pasted the
> > > code from the sample application into their application, which is not
> > > desirable.  
> >
> > I like the idea of encapsulating the boilerplate logic in a library.
> >
> > I do think there is an important missing piece though - there should be
> > some code which queries the netdev for how many queues are attached, and
> > create the appropriate number of umem/AF_XDP sockets.
> >
> > I ran into this issue when testing the current AF_XDP code - on my test
> > boxes, the mlx5 card has 55 channels (aka queues), so when the test program
> > binds only to channel 0, nothing works as expected, since not all traffic
> > is being intercepted.  While obvious in hindsight, this took a while to
> > track down.  
> 
> Yes, agreed. You are not the first one to stumble upon this problem
> :-). Let me think a little bit on how to solve this in a good way. We
> need this to be simple and intuitive, as you say.

I see people hitting this with AF_XDP all the time... I had some
backup-slides[2] in our FOSDEM presentation[1] that describe the issue,
give the performance reason why and propose a workaround.

[1] https://github.com/xdp-project/xdp-project/tree/master/conference/FOSDEM2019
[2] https://github.com/xdp-project/xdp-project/blob/master/conference/FOSDEM2019/xdp_building_block.org#backup-slides

Alternative work-around
  * Create as many AF_XDP sockets as RXQs
  * Have userspace poll()/select on all sockets

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

* Backup Slides                                                      :export:

** Slide: Where does AF_XDP performance come from?                  :export:

/Lock-free [[https://lwn.net/Articles/169961/][channel]] directly from driver RX-queue into AF_XDP socket/
- Single-Producer/Single-Consumer (SPSC) descriptor ring queues
- *Single*-/Producer/ (SP) via bind to specific RX-*/queue id/*
  * NAPI-softirq assures only 1-CPU process 1-RX-queue id (per sched)
- *Single*-/Consumer/ (SC) via 1-Application
- *Bounded* buffer pool (UMEM) allocated by userspace (register with kernel)
  * Descriptor(s) in ring(s) point into UMEM
  * /No memory allocation/, but return frames to UMEM in timely manner
- [[http://www.lemis.com/grog/Documentation/vj/lca06vj.pdf][Transport signature]] Van Jacobson talked about
  * Replaced by XDP/eBPF program choosing to XDP_REDIRECT

** Slide: Details: Actually *four* SPSC ring queues                 :export:

AF_XDP /socket/: Has /two rings/: *RX* and *TX*
 - Descriptor(s) in ring points into UMEM
/UMEM/ consists of a number of equally sized chunks
 - Has /two rings/: *FILL* ring and *COMPLETION* ring
 - FILL ring: application gives kernel area to RX fill
 - COMPLETION ring: kernel tells app TX is done for area (can be reused)

** Slide: Gotcha by RX-queue id binding                             :export:

AF_XDP bound to */single RX-queue id/* (for SPSC performance reasons)
- NIC by default spreads flows with RSS-hashing over RX-queues
  * Traffic likely not hitting queue you expect
- You *MUST* configure NIC *HW filters* to /steer to RX-queue id/
  * Out of scope for XDP setup
  * Use ethtool or TC HW offloading for filter setup
- *Alternative* work-around
  * /Create as many AF_XDP sockets as RXQs/
  * Have userspace poll()/select on all sockets


^ permalink raw reply

* Re: [PATCH RFC v2 2/3] net: Support GRO/GSO fraglist chaining.
From: Steffen Klassert @ 2019-02-13 11:49 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, Willem de Bruijn, Paolo Abeni,
	Jason A. Donenfeld
In-Reply-To: <CAF=yD-+Tvyz8x+F+VnZXYDToW-kC2MuPG5Lcna2W+CQwTOMybQ@mail.gmail.com>

On Mon, Jan 28, 2019 at 02:50:34PM -0600, Willem de Bruijn wrote:
> On Mon, Jan 28, 2019 at 2:53 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> > +
> > +       if (skb_needs_linearize(skb, features) &&
> > +           __skb_linearize(skb)) {
> > +               skb->next = NULL;
> > +               kfree_skb_list(skb->next);
> 
> inverse order

Oh yes, apparently.

> 
> also, I would probably deduplicate with the same branch above in a new
> err_linearize: block

Will do that.

Thanks for the review!

^ permalink raw reply

* Re: [PATCH RFC v2 3/3] udp: Support UDP fraglist GRO/GSO.
From: Steffen Klassert @ 2019-02-13 11:48 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, Willem de Bruijn, Paolo Abeni,
	Jason A. Donenfeld
In-Reply-To: <CAF=yD-+HfQ8Q=KCTq3UKqK2UOHaKFt7ztnbnyKiUutt_-Cr0EQ@mail.gmail.com>

On Mon, Jan 28, 2019 at 02:49:32PM -0600, Willem de Bruijn wrote:
> On Mon, Jan 28, 2019 at 2:51 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > This patch extends UDP GRO to support fraglist GRO/GSO
> > by using the previously introduced infrastructure.
> > All UDP packets that are not targeted to a GRO capable
> > UDP sockets are going to fraglist GRO now (local input
> > and forward).
> > ---
> >  net/ipv4/udp_offload.c | 45 ++++++++++++++++++++++++++++++++++++++----
> >  net/ipv6/udp_offload.c |  9 +++++++++
> >  2 files changed, 50 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index 584635db9231..c0be33216750 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -188,6 +188,22 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
> >  }
> >  EXPORT_SYMBOL(skb_udp_tunnel_segment);
> >
> > +static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
> > +                                             netdev_features_t features)
> > +{
> > +       unsigned int mss = skb_shinfo(skb)->gso_size;
> > +
> > +       skb = skb_segment_list(skb, features, skb_mac_header_len(skb));
> > +       if (IS_ERR(skb))
> > +               return skb;
> > +
> > +       udp_hdr(skb)->len = htons(sizeof(struct udphdr) + mss);
> > +       skb->ip_summed = CHECKSUM_NONE;
> > +       skb->csum_valid = 1;
> 
> csum_valid is only used on ingress.
> 
> Hardcoding CHECKSUM_NONE is probably fine as long as this function is
> only used for forwarding, assuming we don't care about verifiying
> checksums in the forwarding case.
> 
> But this is fragile if we ever add local list segment output. Should
> convert the checksum field in skb_forward_csum, instead of at the GSO
> layer, just as for forwarding of non list skbs? Though that would
> require traversing the list yet another time. Other option is to
> already do this conversion when building the list in GRO.
> 
> The comment also applies to the same logic in skb_segment_list. As a
> matter or fact, even if this belongs in GSO instead of core forwarding
> or GRO, then probably both the list head and frag_list skbs should be
> set in the same function, so skb_segment_list.

I had a deeper look into this now, it seems that the head skb has
already the correct conversion (either for local input or forwarding).
So we probaply just need to adjust the frag_list skbs to the
head skb checksum conversion.

> >         /* pull encapsulating udp header */
> >         skb_gro_pull(skb, sizeof(struct udphdr));
> > -       skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> >
> >         list_for_each_entry(p, head, list) {
> >                 if (!NAPI_GRO_CB(p)->same_flow)
> > @@ -379,8 +397,17 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
> >                  * Under small packet flood GRO count could elsewhere grow a lot
> >                  * leading to execessive truesize values
> >                  */
> > -               if (!skb_gro_receive(p, skb) &&
> > -                   NAPI_GRO_CB(p)->count >= UDP_GRO_CNT_MAX)
> > +               if (NAPI_GRO_CB(skb)->is_flist) {
> > +                       if (!pskb_may_pull(skb, skb_gro_offset(skb)))
> > +                               return NULL;
> > +                       ret = skb_gro_receive_list(p, skb);
> > +               } else {
> > +                       skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> > +
> > +                       ret = skb_gro_receive(p, skb);
> > +               }
> > +
> > +               if (!ret && NAPI_GRO_CB(p)->count > UDP_GRO_CNT_MAX)
> >                         pp = p;
> >                 else if (uh->len != uh2->len)
> >                         pp = p;
> > @@ -402,6 +429,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> >         int flush = 1;
> >
> >         if (!sk || !udp_sk(sk)->gro_receive) {
> > +               NAPI_GRO_CB(skb)->is_flist = sk ? !udp_sk(sk)->gro_enabled: 1;
> 
> This updates the choice of whether to use a list on each received skb.
> Which is problematic as a socket can call the setsockopt in between
> packets.
> 
> Actually, there no longer is a need for a route lookup for each skb at
> all. We always apply GRO, which was the previous reason for the
> lookup. And if a matching flow is found in the GRO table, we already
> the choice to use a list is already stored.

Yes, that's true. I'll change that.


^ permalink raw reply

* Re: [PATCH bpf-next v4 0/2] libbpf: adding AF_XDP support
From: Magnus Karlsson @ 2019-02-13 11:32 UTC (permalink / raw)
  To: Jonathan Lemon
  Cc: Magnus Karlsson, Björn Töpel, ast, Daniel Borkmann,
	Network Development, Jakub Kicinski, Björn Töpel,
	Zhang, Qi Z, Jesper Dangaard Brouer, xiaolong.ye
In-Reply-To: <36557463-D23A-432E-AA18-7731F43CEBA6@gmail.com>

On Mon, Feb 11, 2019 at 9:44 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
>
> On 8 Feb 2019, at 5:05, Magnus Karlsson wrote:
>
> > This patch proposes to add AF_XDP support to libbpf. The main reason
> > for this is to facilitate writing applications that use AF_XDP by
> > offering higher-level APIs that hide many of the details of the AF_XDP
> > uapi. This is in the same vein as libbpf facilitates XDP adoption by
> > offering easy-to-use higher level interfaces of XDP
> > functionality. Hopefully this will facilitate adoption of AF_XDP, make
> > applications using it simpler and smaller, and finally also make it
> > possible for applications to benefit from optimizations in the AF_XDP
> > user space access code. Previously, people just copied and pasted the
> > code from the sample application into their application, which is not
> > desirable.
>
> I like the idea of encapsulating the boilerplate logic in a library.
>
> I do think there is an important missing piece though - there should be
> some code which queries the netdev for how many queues are attached, and
> create the appropriate number of umem/AF_XDP sockets.
>
> I ran into this issue when testing the current AF_XDP code - on my test
> boxes, the mlx5 card has 55 channels (aka queues), so when the test program
> binds only to channel 0, nothing works as expected, since not all traffic
> is being intercepted.  While obvious in hindsight, this took a while to
> track down.

Yes, agreed. You are not the first one to stumble upon this problem
:-). Let me think a little bit on how to solve this in a good way. We
need this to be simple and intuitive, as you say.

/Magnus

> --
> Jonathan

^ permalink raw reply

* [PATCH net-next 09/12] mlxsw: core: Extend hwmon interface with fan fault attribute
From: Ido Schimmel @ 2019-02-13 11:28 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: davem@davemloft.net, Jiri Pirko, andrew@lunn.ch, mlxsw,
	Vadim Pasternak, Ido Schimmel
In-Reply-To: <20190213112814.32334-1-idosch@mellanox.com>

From: Vadim Pasternak <vadimp@mellanox.com>

Add new fan hwmon attribute for exposing fan faults (fault indication is
read from Fan Out of Range Event Register).

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/core_hwmon.c  | 34 ++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c b/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c
index e04e8162aa14..40e13093b62d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c
@@ -121,6 +121,27 @@ static ssize_t mlxsw_hwmon_fan_rpm_show(struct device *dev,
 	return sprintf(buf, "%u\n", mlxsw_reg_mfsm_rpm_get(mfsm_pl));
 }
 
+static ssize_t mlxsw_hwmon_fan_fault_show(struct device *dev,
+					  struct device_attribute *attr,
+					  char *buf)
+{
+	struct mlxsw_hwmon_attr *mlwsw_hwmon_attr =
+			container_of(attr, struct mlxsw_hwmon_attr, dev_attr);
+	struct mlxsw_hwmon *mlxsw_hwmon = mlwsw_hwmon_attr->hwmon;
+	char fore_pl[MLXSW_REG_FORE_LEN];
+	bool fault;
+	int err;
+
+	err = mlxsw_reg_query(mlxsw_hwmon->core, MLXSW_REG(fore), fore_pl);
+	if (err) {
+		dev_err(mlxsw_hwmon->bus_info->dev, "Failed to query fan\n");
+		return err;
+	}
+	mlxsw_reg_fore_unpack(fore_pl, mlwsw_hwmon_attr->type_index, &fault);
+
+	return sprintf(buf, "%u\n", fault);
+}
+
 static ssize_t mlxsw_hwmon_pwm_show(struct device *dev,
 				    struct device_attribute *attr,
 				    char *buf)
@@ -172,6 +193,7 @@ enum mlxsw_hwmon_attr_type {
 	MLXSW_HWMON_ATTR_TYPE_TEMP_MAX,
 	MLXSW_HWMON_ATTR_TYPE_TEMP_RST,
 	MLXSW_HWMON_ATTR_TYPE_FAN_RPM,
+	MLXSW_HWMON_ATTR_TYPE_FAN_FAULT,
 	MLXSW_HWMON_ATTR_TYPE_PWM,
 };
 
@@ -209,6 +231,12 @@ static void mlxsw_hwmon_attr_add(struct mlxsw_hwmon *mlxsw_hwmon,
 		snprintf(mlxsw_hwmon_attr->name, sizeof(mlxsw_hwmon_attr->name),
 			 "fan%u_input", num + 1);
 		break;
+	case MLXSW_HWMON_ATTR_TYPE_FAN_FAULT:
+		mlxsw_hwmon_attr->dev_attr.show = mlxsw_hwmon_fan_fault_show;
+		mlxsw_hwmon_attr->dev_attr.attr.mode = 0444;
+		snprintf(mlxsw_hwmon_attr->name, sizeof(mlxsw_hwmon_attr->name),
+			 "fan%u_fault", num + 1);
+		break;
 	case MLXSW_HWMON_ATTR_TYPE_PWM:
 		mlxsw_hwmon_attr->dev_attr.show = mlxsw_hwmon_pwm_show;
 		mlxsw_hwmon_attr->dev_attr.store = mlxsw_hwmon_pwm_store;
@@ -280,10 +308,14 @@ static int mlxsw_hwmon_fans_init(struct mlxsw_hwmon *mlxsw_hwmon)
 	mlxsw_reg_mfcr_unpack(mfcr_pl, &freq, &tacho_active, &pwm_active);
 	num = 0;
 	for (type_index = 0; type_index < MLXSW_MFCR_TACHOS_MAX; type_index++) {
-		if (tacho_active & BIT(type_index))
+		if (tacho_active & BIT(type_index)) {
 			mlxsw_hwmon_attr_add(mlxsw_hwmon,
 					     MLXSW_HWMON_ATTR_TYPE_FAN_RPM,
+					     type_index, num);
+			mlxsw_hwmon_attr_add(mlxsw_hwmon,
+					     MLXSW_HWMON_ATTR_TYPE_FAN_FAULT,
 					     type_index, num++);
+		}
 	}
 	num = 0;
 	for (type_index = 0; type_index < MLXSW_MFCR_PWMS_MAX; type_index++) {
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next 08/12] mlxsw: core: Rename cooling device
From: Ido Schimmel @ 2019-02-13 11:28 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: davem@davemloft.net, Jiri Pirko, andrew@lunn.ch, mlxsw,
	Vadim Pasternak, Ido Schimmel
In-Reply-To: <20190213112814.32334-1-idosch@mellanox.com>

From: Vadim Pasternak <vadimp@mellanox.com>

Rename cooling device from "Fan" to "mlxsw_fan".  Name "Fan" is too
common name, and such name is misleading, while it's interpreted by
user. For example name "Fan" could be used by ACPI.

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core_thermal.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index b9fb33417a82..8b71706f7a27 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -431,8 +431,9 @@ int mlxsw_thermal_init(struct mlxsw_core *core,
 		if (pwm_active & BIT(i)) {
 			struct thermal_cooling_device *cdev;
 
-			cdev = thermal_cooling_device_register("Fan", thermal,
-							&mlxsw_cooling_ops);
+			cdev = thermal_cooling_device_register("mlxsw_fan",
+							       thermal,
+							       &mlxsw_cooling_ops);
 			if (IS_ERR(cdev)) {
 				err = PTR_ERR(cdev);
 				dev_err(dev, "Failed to register cooling device\n");
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next 12/12] mlxsw: core: Allow thermal zone binding to an external cooling device
From: Ido Schimmel @ 2019-02-13 11:28 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: davem@davemloft.net, Jiri Pirko, andrew@lunn.ch, mlxsw,
	Vadim Pasternak, Ido Schimmel
In-Reply-To: <20190213112814.32334-1-idosch@mellanox.com>

From: Vadim Pasternak <vadimp@mellanox.com>

Allow thermal zone binding to an external cooling device from the
cooling devices white list.

It provides support for Mellanox next generation systems on which
cooling device logic is not controlled through the switch registers.

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/core_thermal.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index 8b71706f7a27..821fef2e2230 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -31,6 +31,11 @@
 #define MLXSW_THERMAL_SPEED_MAX		(MLXSW_THERMAL_MAX_STATE * 2)
 #define MLXSW_THERMAL_SPEED_MIN_LEVEL	2		/* 20% */
 
+/* External cooling devices, allowed for binding to mlxsw thermal zones. */
+static char * const mlxsw_thermal_external_allowed_cdev[] = {
+	"mlxreg_fan",
+};
+
 struct mlxsw_thermal_trip {
 	int	type;
 	int	temp;
@@ -107,6 +112,13 @@ static int mlxsw_get_cooling_device_idx(struct mlxsw_thermal *thermal,
 		if (thermal->cdevs[i] == cdev)
 			return i;
 
+	/* Allow mlxsw thermal zone binding to an external cooling device */
+	for (i = 0; i < ARRAY_SIZE(mlxsw_thermal_external_allowed_cdev); i++) {
+		if (strnstr(cdev->type, mlxsw_thermal_external_allowed_cdev[i],
+			    sizeof(cdev->type)))
+			return 0;
+	}
+
 	return -ENODEV;
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next 11/12] mlxsw: core: Add QSFP module temperature label attribute to hwmon
From: Ido Schimmel @ 2019-02-13 11:28 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: davem@davemloft.net, Jiri Pirko, andrew@lunn.ch, mlxsw,
	Vadim Pasternak, Ido Schimmel
In-Reply-To: <20190213112814.32334-1-idosch@mellanox.com>

From: Vadim Pasternak <vadimp@mellanox.com>

Add label attribute to hwmon object for exposing QSFP module's
temperature sensor name. Modules are labeled as "front panel xxx". The
label is used by utilities such as "sensors":

front panel 001:   +0.0C  (crit =  +0.0C, emerg =  +0.0C)
..
front panel 020:  +31.0C  (crit = +70.0C, emerg = +80.0C)
..
front panel 056:  +41.0C  (crit = +70.0C, emerg = +80.0C)

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/core_hwmon.c  | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c b/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c
index 504f6bba8874..f1ada4cdbd6b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c
@@ -321,6 +321,18 @@ mlxsw_hwmon_module_temp_emergency_show(struct device *dev,
 	return sprintf(buf, "%u\n", temp);
 }
 
+static ssize_t
+mlxsw_hwmon_module_temp_label_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct mlxsw_hwmon_attr *mlwsw_hwmon_attr =
+			container_of(attr, struct mlxsw_hwmon_attr, dev_attr);
+
+	return sprintf(buf, "front panel %03u\n",
+		       mlwsw_hwmon_attr->type_index);
+}
+
 enum mlxsw_hwmon_attr_type {
 	MLXSW_HWMON_ATTR_TYPE_TEMP,
 	MLXSW_HWMON_ATTR_TYPE_TEMP_MAX,
@@ -332,6 +344,7 @@ enum mlxsw_hwmon_attr_type {
 	MLXSW_HWMON_ATTR_TYPE_TEMP_MODULE_FAULT,
 	MLXSW_HWMON_ATTR_TYPE_TEMP_MODULE_CRIT,
 	MLXSW_HWMON_ATTR_TYPE_TEMP_MODULE_EMERG,
+	MLXSW_HWMON_ATTR_TYPE_TEMP_MODULE_LABEL,
 };
 
 static void mlxsw_hwmon_attr_add(struct mlxsw_hwmon *mlxsw_hwmon,
@@ -408,6 +421,13 @@ static void mlxsw_hwmon_attr_add(struct mlxsw_hwmon *mlxsw_hwmon,
 		snprintf(mlxsw_hwmon_attr->name, sizeof(mlxsw_hwmon_attr->name),
 			 "temp%u_emergency", num + 1);
 		break;
+	case MLXSW_HWMON_ATTR_TYPE_TEMP_MODULE_LABEL:
+		mlxsw_hwmon_attr->dev_attr.show =
+			mlxsw_hwmon_module_temp_label_show;
+		mlxsw_hwmon_attr->dev_attr.attr.mode = 0444;
+		snprintf(mlxsw_hwmon_attr->name, sizeof(mlxsw_hwmon_attr->name),
+			 "temp%u_label", num + 1);
+		break;
 	default:
 		WARN_ON(1);
 	}
@@ -528,6 +548,9 @@ static int mlxsw_hwmon_module_init(struct mlxsw_hwmon *mlxsw_hwmon)
 		mlxsw_hwmon_attr_add(mlxsw_hwmon,
 				     MLXSW_HWMON_ATTR_TYPE_TEMP_MODULE_EMERG,
 				     index, index);
+		mlxsw_hwmon_attr_add(mlxsw_hwmon,
+				     MLXSW_HWMON_ATTR_TYPE_TEMP_MODULE_LABEL,
+				     index, index);
 		index++;
 	}
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next 07/12] mlxsw: core: Replace thermal temperature trips with defines
From: Ido Schimmel @ 2019-02-13 11:28 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: davem@davemloft.net, Jiri Pirko, andrew@lunn.ch, mlxsw,
	Vadim Pasternak, Ido Schimmel
In-Reply-To: <20190213112814.32334-1-idosch@mellanox.com>

From: Vadim Pasternak <vadimp@mellanox.com>

Replace thermal hardcoded temperature trip values with defines.

Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/core_thermal.c    | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index dbf9a845084a..b9fb33417a82 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -14,7 +14,10 @@
 
 #define MLXSW_THERMAL_POLL_INT	1000	/* ms */
 #define MLXSW_THERMAL_SLOW_POLL_INT	20000	/* ms */
-#define MLXSW_THERMAL_MAX_TEMP	110000	/* 110C */
+#define MLXSW_THERMAL_ASIC_TEMP_NORM	75000	/* 75C */
+#define MLXSW_THERMAL_ASIC_TEMP_HIGH	85000	/* 85C */
+#define MLXSW_THERMAL_ASIC_TEMP_HOT	105000	/* 105C */
+#define MLXSW_THERMAL_ASIC_TEMP_CRIT	110000	/* 110C */
 #define MLXSW_THERMAL_HYSTERESIS_TEMP	5000	/* 5C */
 #define MLXSW_THERMAL_MAX_STATE	10
 #define MLXSW_THERMAL_MAX_DUTY	255
@@ -39,7 +42,7 @@ struct mlxsw_thermal_trip {
 static const struct mlxsw_thermal_trip default_thermal_trips[] = {
 	{	/* In range - 0-40% PWM */
 		.type		= THERMAL_TRIP_ACTIVE,
-		.temp		= 75000,
+		.temp		= MLXSW_THERMAL_ASIC_TEMP_NORM,
 		.hyst		= MLXSW_THERMAL_HYSTERESIS_TEMP,
 		.min_state	= 0,
 		.max_state	= (4 * MLXSW_THERMAL_MAX_STATE) / 10,
@@ -47,21 +50,21 @@ static const struct mlxsw_thermal_trip default_thermal_trips[] = {
 	{
 		/* In range - 40-100% PWM */
 		.type		= THERMAL_TRIP_ACTIVE,
-		.temp		= 80000,
+		.temp		= MLXSW_THERMAL_ASIC_TEMP_HIGH,
 		.hyst		= MLXSW_THERMAL_HYSTERESIS_TEMP,
 		.min_state	= (4 * MLXSW_THERMAL_MAX_STATE) / 10,
 		.max_state	= MLXSW_THERMAL_MAX_STATE,
 	},
 	{	/* Warning */
 		.type		= THERMAL_TRIP_HOT,
-		.temp		= 85000,
+		.temp		= MLXSW_THERMAL_ASIC_TEMP_HOT,
 		.hyst		= MLXSW_THERMAL_HYSTERESIS_TEMP,
 		.min_state	= MLXSW_THERMAL_MAX_STATE,
 		.max_state	= MLXSW_THERMAL_MAX_STATE,
 	},
 	{	/* Critical - soft poweroff */
 		.type		= THERMAL_TRIP_CRITICAL,
-		.temp		= MLXSW_THERMAL_MAX_TEMP,
+		.temp		= MLXSW_THERMAL_ASIC_TEMP_CRIT,
 		.min_state	= MLXSW_THERMAL_MAX_STATE,
 		.max_state	= MLXSW_THERMAL_MAX_STATE,
 	}
@@ -238,7 +241,7 @@ static int mlxsw_thermal_set_trip_temp(struct thermal_zone_device *tzdev,
 	struct mlxsw_thermal *thermal = tzdev->devdata;
 
 	if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS ||
-	    temp > MLXSW_THERMAL_MAX_TEMP)
+	    temp > MLXSW_THERMAL_ASIC_TEMP_CRIT)
 		return -EINVAL;
 
 	thermal->trips[trip].temp = temp;
-- 
2.20.1


^ permalink raw reply related


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