Netdev List
 help / color / mirror / Atom feed
* 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 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: [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: 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: [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

* 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: 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

* 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: [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 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: 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

* [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: [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

* 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 v2] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: maowenan @ 2019-02-19  8:39 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <alpine.DEB.2.21.1902190929250.2570@hadrien>



On 2019/2/19 16:31, Julia Lawall wrote:
> 
> 
> On Tue, 19 Feb 2019, Mao Wenan wrote:
> 
>> There is no need to have the 'int first' static
>> since new value always be assigned before use it.
>>
>> This patch cleans up the codes in ns83820_probe_phy() as well.
> 
> As mentioned previously, there is no use of first in the function at all,
> other than setting it to 1.  So it can be just dropped completely.
> 
OK, thank you for your comments.

> "codes" is also a word that I am particularly not fond of.  Just "code" is
> fine.
> 
> "cleans up" is not very specific about what is going on, and why.
> 

> julia
> 
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  v1->v2: delete unused codes.
>>  drivers/net/ethernet/natsemi/ns83820.c | 17 +----------------
>>  1 file changed, 1 insertion(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
>> index 958fced4dacf..37694f6843fe 100644
>> --- a/drivers/net/ethernet/natsemi/ns83820.c
>> +++ b/drivers/net/ethernet/natsemi/ns83820.c
>> @@ -1869,23 +1869,11 @@ 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 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++) {
>> @@ -1894,9 +1882,6 @@ static void ns83820_probe_phy(struct net_device *ndev)
>>  		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: [PATCH v2] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Julia Lawall @ 2019-02-19  8:31 UTC (permalink / raw)
  To: Mao Wenan
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <20190219084214.96202-1-maowenan@huawei.com>



On Tue, 19 Feb 2019, Mao Wenan wrote:

> There is no need to have the 'int first' static
> since new value always be assigned before use it.
>
> This patch cleans up the codes in ns83820_probe_phy() as well.

As mentioned previously, there is no use of first in the function at all,
other than setting it to 1.  So it can be just dropped completely.

"codes" is also a word that I am particularly not fond of.  Just "code" is
fine.

"cleans up" is not very specific about what is going on, and why.

julia

>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  v1->v2: delete unused codes.
>  drivers/net/ethernet/natsemi/ns83820.c | 17 +----------------
>  1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
> index 958fced4dacf..37694f6843fe 100644
> --- a/drivers/net/ethernet/natsemi/ns83820.c
> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> @@ -1869,23 +1869,11 @@ 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 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++) {
> @@ -1894,9 +1882,6 @@ static void ns83820_probe_phy(struct net_device *ndev)
>  		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

* [PATCH v2] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Mao Wenan @ 2019-02-19  8:42 UTC (permalink / raw)
  To: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast

There is no need to have the 'int first' static
since new value always be assigned before use it.

This patch cleans up the codes in ns83820_probe_phy() as well.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 v1->v2: delete unused codes.
 drivers/net/ethernet/natsemi/ns83820.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
index 958fced4dacf..37694f6843fe 100644
--- a/drivers/net/ethernet/natsemi/ns83820.c
+++ b/drivers/net/ethernet/natsemi/ns83820.c
@@ -1869,23 +1869,11 @@ 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 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++) {
@@ -1894,9 +1882,6 @@ static void ns83820_probe_phy(struct net_device *ndev)
 		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

* Regarding CLNP driver
From: Shreeya Patel @ 2019-02-19  8:22 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Moustafa Ali, Mohamed El-Dessouki

Hi All,

My goal is to have the CLNP ( Connectionless network protocol ) driver
working on the latest Ubuntu version / any Linux distros.
But I see that CLNP driver was developed by a group of people during
2.6 kernel and now it doesn't exist anymore.
Here is the source code...
https://github.com/crazoes/clnp

I tried to build the driver on latest kernel of latest Ubuntu version
by having 2.6 kernel's header files but as we all know it is not
possible to have so old module working on the latest version.

I also tried to downgrade the kernel on Ubuntu 18.04 to 2.6 kernel but
again I knew this was not going to work and it showed me an error of
KERNEL TOO OLD during boot time.

Is there any way possible to have CLNP driver working on the latest
Ubuntu release or any other Linux distro's release?

Thanks




^ permalink raw reply

* Re: [PATCH] net/mlx4_en: fix spelling mistake: "quiting" -> "quitting"
From: Tariq Toukan @ 2019-02-19  8:20 UTC (permalink / raw)
  To: David Miller, colin.king@canonical.com
  Cc: Tariq Toukan, netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190218.120836.1419708231583986821.davem@davemloft.net>



On 2/18/2019 10:08 PM, David Miller wrote:
> From: Colin King <colin.king@canonical.com>
> Date: Sun, 17 Feb 2019 23:03:31 +0000
> 
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> There is a spelling mistake in a en_err error message. Fix it.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> 
> Applied, thanks Colin.
> 
> And I agree that this doesn't really deserve a Fixes: tag.
> 
> Fixes: tags should really be for changes that introduce truly
> functional bugs.
> 
> And that could even be applied in this case _iff_ the string
> was essential in some way for userland tools which parse the
> output or similar.  But that is not the case here.
> 

Thanks for the clarification.

> Anyways, thanks.
> 

^ permalink raw reply

* Re: [PATCH] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Julia Lawall @ 2019-02-19  8:15 UTC (permalink / raw)
  To: maowenan
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <578c4208-956d-b905-6a50-5ad04620a349@huawei.com>



On Tue, 19 Feb 2019, maowenan wrote:

>
>
> On 2019/2/19 16:01, Julia Lawall wrote:
> >
> >
> > On Tue, 19 Feb 2019, Mao Wenan wrote:
> >
> >> There is no need to have the 'int first' static
> >> since new value always be assigned before use it.
> >> The old codes of first dereferencing this variable have
> >> been commented out. So it is useless with 'static int first'.
> >> /*
> >> if (!first) {
> >>  ...
> >> }
> >> */
> >> first = 1;
> >
> > Do you need the variable at all?
> >
> > In the code, the commenting out is actually done with #if 0.  It could be
> > good for the commit log to reflect the code more accurately.
> >
> OK, the original codes are using #if 0 to comment out, but when I generate patch
> the "#if 0" has been removed, so I use "/*...*/" to reflect that.

I don't understand the above comment.  The proposed patch doesn't have any
impact on the #if 0 lines.

>
> > Can the #if 0 and code be removed?  Apparently it predates git.
> >
> > A commented out printk below also predates git.
> >
> I think all the "#if 0" comment out codes and printk codes can be removed, shall I send
> v2 to do this?

It seems reasonable to me.

julia

> > julia
> >
> >>
> >> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >> ---
> >>  drivers/net/ethernet/natsemi/ns83820.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
> >> index 958fced4dacf..fb064df3a1a6 100644
> >> --- a/drivers/net/ethernet/natsemi/ns83820.c
> >> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> >> @@ -1869,7 +1869,7 @@ 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 first;
> >>  	int i;
> >>  #define MII_PHYIDR1	0x02
> >>  #define MII_PHYIDR2	0x03
> >> --
> >> 2.20.1
> >>
> >>
> >
> > .
> >
>
>

^ permalink raw reply

* Re: [PATCH] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: maowenan @ 2019-02-19  8:09 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <alpine.DEB.2.21.1902190857300.2570@hadrien>



On 2019/2/19 16:01, Julia Lawall wrote:
> 
> 
> On Tue, 19 Feb 2019, Mao Wenan wrote:
> 
>> There is no need to have the 'int first' static
>> since new value always be assigned before use it.
>> The old codes of first dereferencing this variable have
>> been commented out. So it is useless with 'static int first'.
>> /*
>> if (!first) {
>>  ...
>> }
>> */
>> first = 1;
> 
> Do you need the variable at all?
> 
> In the code, the commenting out is actually done with #if 0.  It could be
> good for the commit log to reflect the code more accurately.
> 
OK, the original codes are using #if 0 to comment out, but when I generate patch
the "#if 0" has been removed, so I use "/*...*/" to reflect that.

> Can the #if 0 and code be removed?  Apparently it predates git.
> 
> A commented out printk below also predates git.
> 
I think all the "#if 0" comment out codes and printk codes can be removed, shall I send
v2 to do this?

> julia
> 
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  drivers/net/ethernet/natsemi/ns83820.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
>> index 958fced4dacf..fb064df3a1a6 100644
>> --- a/drivers/net/ethernet/natsemi/ns83820.c
>> +++ b/drivers/net/ethernet/natsemi/ns83820.c
>> @@ -1869,7 +1869,7 @@ 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 first;
>>  	int i;
>>  #define MII_PHYIDR1	0x02
>>  #define MII_PHYIDR2	0x03
>> --
>> 2.20.1
>>
>>
> 
> .
> 


^ permalink raw reply

* Re: [PATCH net-next v3 0/2] net: phy: at803x: Update delays for RGMII modes
From: Peter Ujfalusi @ 2019-02-19  8:09 UTC (permalink / raw)
  To: Vinod Koul, David S Miller
  Cc: linux-arm-msm, Bjorn Andersson, netdev, Niklas Cassel,
	Andrew Lunn, Florian Fainelli, Nori, Sekhar, Marc Gonzalez
In-Reply-To: <20190219061800.31025-1-vkoul@kernel.org>

Hi Vinod,

On 19/02/2019 8.17, Vinod Koul wrote:
> Peter[1] reported that patch cd28d1d6e52e: ("net: phy: at803x: Disable
> phy delay for RGMII mode") caused regression on am335x-evmsk board.
> This board expects the Phy delay to be enabled but specified RGMII mode
> which refers to delays being disabled. So fix this by disabling delay only
> for RGMII mode and enable for RGMII_ID and RGMII_TXID/RXID modes.
> 
> While at it, as pointed by Dave, don't inline the helpers.
> 
> [1]: https://www.spinics.net/lists/netdev/msg550749.html

Not sure what was changed since v2, but with
https://patchwork.ozlabs.org/project/netdev/list/?series=92672
ethernet is working on am335x-evmsk. thank you:

Tested-by: Peter Ujfalusi <peter.ujflausi@ti.com>

> Vinod Koul (2):
>   net: phy: at803x: don't inline helpers
>   net: phy: at803x: disable delay only for RGMII mode
> 
>  drivers/net/phy/at803x.c | 51 ++++++++++++++++++++++++++++++----------
>  1 file changed, 38 insertions(+), 13 deletions(-)
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* [PATCH net-next] net: rose: add missing dev_put() on error in rose_bind
From: YueHaibing @ 2019-02-19  8:06 UTC (permalink / raw)
  To: ralf, davem; +Cc: linux-kernel, netdev, linux-hams, YueHaibing

when capable check failed, dev_put should
be call before return -EACCES.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 net/rose/af_rose.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index d00a0ef..c96f63f 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -689,8 +689,10 @@ static int rose_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 		rose->source_call = user->call;
 		ax25_uid_put(user);
 	} else {
-		if (ax25_uid_policy && !capable(CAP_NET_BIND_SERVICE))
+		if (ax25_uid_policy && !capable(CAP_NET_BIND_SERVICE)) {
+			dev_put(dev);
 			return -EACCES;
+		}
 		rose->source_call   = *source;
 	}
 
-- 
2.7.0



^ permalink raw reply related

* Re: [PATCH 0/2] ARM: dts: am335x-evm/evmsk: Fix PHY mode for ethernet
From: Peter Ujfalusi @ 2019-02-19  8:02 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: bcousson, linux-omap, devicetree, linux-arm-kernel, nsekhar,
	grygorii.strashko, vkoul, netdev, f.fainelli, marc.w.gonzalez,
	niklas.cassel
In-Reply-To: <20190218162613.GH15711@atomide.com>

Hi Tony,

On 18/02/2019 18.26, Tony Lindgren wrote:
> * Peter Ujfalusi <peter.ujfalusi@ti.com> [190218 16:22]:
>>
>>
>> On 18/02/2019 16.44, Tony Lindgren wrote:
>>> * Peter Ujfalusi <peter.ujfalusi@ti.com> [190218 14:36]:
>>>> Hi,
>>>>
>>>> cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode") broke the
>>>> ethernet networking on evmsk (and most likely on the evm as well):
>>>> https://patchwork.ozlabs.org/patch/1028527/
>>>>
>>>> v1 patch to fix the situation:
>>>> https://patchwork.ozlabs.org/patch/1040617/
>>>>
>>>> It turned out that the at803x driver is actually broken and need to be fixed
>>>> along with the DT data.
>>>>
>>>> The following series is proposed to fix the driver:
>>>> https://patchwork.ozlabs.org/project/netdev/list/?series=92611
>>>>
>>>> but the PHT mode needs to be switched to rgmii-id from rgmii-txid:
>>>> The rx delay is enabled by default and the driver never disabled it so when
>>>> asking rgmii-txid it actually got rgmii-id.
>>>>
>>>> The patch can be backported to stable, I have tested that it is not causing
>>>> regression with the old, broken driver.
>>>
>>> Can the dts changes be merged before the driver changes or
>>> does it cause the phy to stop working?
>>
>> The phy is not working atm, but this change will not cause regression
>> even if it is merged first.
> 
> OK so sounds like these are OK to wait for v5.1 merge window
> then as the dts changes alone won't fix anything?

I think it would be better to send these to 5.0 to avoid the regression
siting in linux-next.

The patches are actually doing to the correct thing (we need rgmii-id,
not rmgii-txid for these boards).
It was just a matter of luck that it worked with rgmii-txid as the
driver was broken and did not disabled the rxid when it should have.

Tested the patch on top of mainline
(b5372fe5dc84235dbe04998efdede3c4daa866a9) was the topmost commit after
the rc7 tag.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* Re: [PATCH] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Julia Lawall @ 2019-02-19  8:01 UTC (permalink / raw)
  To: Mao Wenan
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <20190219080659.89732-1-maowenan@huawei.com>



On Tue, 19 Feb 2019, Mao Wenan wrote:

> There is no need to have the 'int first' static
> since new value always be assigned before use it.
> The old codes of first dereferencing this variable have
> been commented out. So it is useless with 'static int first'.
> /*
> if (!first) {
>  ...
> }
> */
> first = 1;

Do you need the variable at all?

In the code, the commenting out is actually done with #if 0.  It could be
good for the commit log to reflect the code more accurately.

Can the #if 0 and code be removed?  Apparently it predates git.

A commented out printk below also predates git.

julia

>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  drivers/net/ethernet/natsemi/ns83820.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
> index 958fced4dacf..fb064df3a1a6 100644
> --- a/drivers/net/ethernet/natsemi/ns83820.c
> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> @@ -1869,7 +1869,7 @@ 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 first;
>  	int i;
>  #define MII_PHYIDR1	0x02
>  #define MII_PHYIDR2	0x03
> --
> 2.20.1
>
>

^ permalink raw reply


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