Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v2 2/9] net: macsec: move some definitions in a dedicated header
From: Antoine Tenart @ 2019-08-12  8:17 UTC (permalink / raw)
  To: Igor Russkikh
  Cc: Antoine Tenart, davem@davemloft.net, sd@queasysnail.net,
	andrew@lunn.ch, f.fainelli@gmail.com, hkallweit1@gmail.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	thomas.petazzoni@bootlin.com, alexandre.belloni@bootlin.com,
	allan.nielsen@microchip.com, camelia.groza@nxp.com,
	Simon Edelhaus, Pavel Belous
In-Reply-To: <9f65de8e-bf62-f9b0-5aba-69c0f92df1ca@aquantia.com>

Hi Igor,

On Sat, Aug 10, 2019 at 12:19:36PM +0000, Igor Russkikh wrote:
> > +/**
> > + * struct macsec_tx_sa - transmit secure association
> > + * @active:
> > + * @next_pn: packet number to use for the next packet
> > + * @lock: protects next_pn manipulations
> > + * @key: key structure
> > + * @stats: per-SA stats
> > + */
> > +struct macsec_tx_sa {
> > +	struct macsec_key key;
> > +	spinlock_t lock;
> > +	u32 next_pn;
> > +	refcount_t refcnt;
> > +	bool active;
> > +	bool offloaded;
> 
> I don't see this `offloaded` field being used anywhere. Is this needed?

You're right it's not and was only used in previous versions of this
patchset. I'll remove it.

Thanks for spotting this!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH net-next v2 6/9] net: macsec: hardware offloading infrastructure
From: Antoine Tenart @ 2019-08-12  8:15 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Antoine Tenart, davem, sd, f.fainelli, hkallweit1, netdev,
	linux-kernel, thomas.petazzoni, alexandre.belloni, allan.nielsen,
	camelia.groza, Simon.Edelhaus
In-Reply-To: <20190810163423.GA30120@lunn.ch>

Hi Andrew,

On Sat, Aug 10, 2019 at 06:34:23PM +0200, Andrew Lunn wrote:
> On Thu, Aug 08, 2019 at 04:05:57PM +0200, Antoine Tenart wrote:
> > This patch introduces the MACsec hardware offloading infrastructure.
> > 
> > The main idea here is to re-use the logic and data structures of the
> > software MACsec implementation. This allows not to duplicate definitions
> > and structure storing the same kind of information. It also allows to
> > use a unified genlink interface for both MACsec implementations (so that
> > the same userspace tool, `ip macsec`, is used with the same arguments).
> > The MACsec offloading support cannot be disabled if an interface
> > supports it at the moment.
> > 
> > The MACsec configuration is passed to device drivers supporting it
> > through macsec_hw_offload() which is called from the MACsec genl
> > helpers. This function calls the macsec ops of PHY and Ethernet
> > drivers in two steps
> 
> It is great that you are thinking how a MAC driver would make use of
> this. But on the flip side, we don't usual add an API unless there is
> a user. And as far as i see, you only add a PHY level implementation,
> not a MAC level.

That's right, and the only modification here is a simple patch adding
the MACsec ops within struct net_device. I can remove it as we do not
have providers as of now and it can be added easily later on.

Thanks,
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH net-next v2 8/9] net: phy: mscc: macsec initialization
From: Antoine Tenart @ 2019-08-12  8:12 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Antoine Tenart, davem, sd, f.fainelli, hkallweit1, netdev,
	linux-kernel, thomas.petazzoni, alexandre.belloni, allan.nielsen,
	camelia.groza, Simon.Edelhaus
In-Reply-To: <20190810165317.GB30120@lunn.ch>

Hi Andrew,

On Sat, Aug 10, 2019 at 06:53:17PM +0200, Andrew Lunn wrote:
> > The MACsec read and write functions are wrapped into two versions: one
> > called during the init phase, and the other one later on. This is
> > because the init functions in the Microsemi Ocelot PHY driver are called
> > while the MDIO bus lock is taken.
> 
> It is nice you have wrapped it all up, but it is still messy. Sometime
> in the future, we should maybe take another look at adding the concept
> of initialisation of a package, before the initialization of the PHYs
> in the package.

I agree, it's still a hack to have those read/write functions acting
differently based on an 'init' flag.

> > +static u32 __vsc8584_macsec_phy_read(struct phy_device *phydev,
> > +				     enum macsec_bank bank, u32 reg, bool init)
> > +{
> > +	u32 val, val_l = 0, val_h = 0;
> > +	unsigned long deadline;
> > +	int rc;
> > +
> > +	if (!init) {
> > +		rc = phy_select_page(phydev, MSCC_PHY_PAGE_MACSEC);
> > +		if (rc < 0)
> > +			goto failed;
> > +	} else {
> > +		__phy_write_page(phydev, MSCC_PHY_PAGE_MACSEC);
> > +	}
> 
> ...
> 
> > +	if (!init) {
> > +failed:
> > +		phy_restore_page(phydev, rc, rc);
> > +	} else {
> > +		__phy_write_page(phydev, MSCC_PHY_PAGE_STANDARD);
> > +	}
> 
> Having the failed label inside the if is correct, but i think it is
> potentially dangerous for future modifications to this function. I
> would move the label before the if. I doubt it makes any difference to
> the generated code, but it might prevent future bugs.

Right, having readable code is always better. I'll fix that.

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH net-next] net: can: Fix compiling warning
From: Oliver Hartkopp @ 2019-08-12  8:11 UTC (permalink / raw)
  To: maowenan; +Cc: netdev, linux-kernel, kernel-janitors
In-Reply-To: <5018f6ca-53b5-c712-a012-a0fcda5c10c2@huawei.com>

On 12/08/2019 07.48, maowenan wrote:
> On 2019/8/7 0:41, Oliver Hartkopp wrote:

>> I compiled the code (the original version), but I do not get that "Should it be static?" warning:

> here are my steps for net/can/bcm.c,
> make allmodconfig ARCH=mips CROSS_COMPILE=mips-linux-gnu-
> make C=2 net/can/bcm.o ARCH=mips CROSS_COMPILE=mips-linux-gnu-

There were some sparse _errors_ in my setup that hide that "static" 
warning. I will use sparse by default now.

Many thanks,
Oliver


^ permalink raw reply

* Re: [PATCH v2 net-next 2/2] net: mvpp2: support multiple comphy lanes
From: Antoine Tenart @ 2019-08-12  7:55 UTC (permalink / raw)
  To: Matt Pelland; +Cc: Antoine Tenart, netdev, maxime.chevallier
In-Reply-To: <20190809222028.GB1320@cohiba>

Hello Matt,

On Fri, Aug 09, 2019 at 06:20:28PM -0400, Matt Pelland wrote:
> On Fri, Aug 09, 2019 at 10:32:50AM +0200, Antoine Tenart wrote:
> > On Thu, Aug 08, 2019 at 07:06:06PM -0400, Matt Pelland wrote:
> > > @@ -5084,14 +5107,38 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> > >  		goto err_free_netdev;
> > >  	}
> > >  
> > > +	port = netdev_priv(dev);
> > > +
> > >  	if (port_node) {
> > > -		comphy = devm_of_phy_get(&pdev->dev, port_node, NULL);
> > > -		if (IS_ERR(comphy)) {
> > > -			if (PTR_ERR(comphy) == -EPROBE_DEFER) {
> > > -				err = -EPROBE_DEFER;
> > > -				goto err_free_netdev;
> > > +		for (i = 0, ncomphys = 0; i < ARRAY_SIZE(port->comphys); i++) {
> > > +			port->comphys[i] = devm_of_phy_get_by_index(&pdev->dev,
> > > +								    port_node,
> > > +								    i);
> > > +			if (IS_ERR(port->comphys[i])) {
> > > +				err = PTR_ERR(port->comphys[i]);
> > > +				port->comphys[i] = NULL;
> > > +				if (err == -EPROBE_DEFER)
> > > +					goto err_free_netdev;
> > > +				err = 0;
> > > +				break;
> > >  			}
> > > -			comphy = NULL;
> > > +
> > > +			++ncomphys;
> > > +		}
> > > +
> > > +		if (phy_mode == PHY_INTERFACE_MODE_XAUI)
> > > +			nrequired_comphys = 4;
> > > +		else if (phy_mode == PHY_INTERFACE_MODE_RXAUI)
> > > +			nrequired_comphys = 2;
> > > +		else
> > > +			nrequired_comphys = 1;
> > > +
> > > +		if (ncomphys < nrequired_comphys) {
> > > +			dev_err(&pdev->dev,
> > > +				"not enough comphys to support %s\n",
> > > +				phy_modes(phy_mode));
> > > +			err = -EINVAL;
> > > +			goto err_free_netdev;
> > 
> > The comphy is optional and could not be described (some SoC do not have
> > a driver for their comphy, and some aren't described at all). In such
> > cases we do rely on the bootloader/firmware configuration. Also, I'm not
> > sure how that would work with dynamic reconfiguration of the mode if the
> > n# of lanes used changes (I'm not sure that is possible though).
> > 
> 
> I'm new to this space, but, from my limited experience it seems unlikely that
> some hardware configuration would require dynamically reconfiguring the number
> of comphy lanes used depending on the phy mode. Unless you disagree, instead of
> removing this check or making things really complicated to support this
> scenario, I propose extending the conditional above to disable sanity checking
> if no comphys were parsed out of the device tree. Something like:
> 
> if (ncomphys != 0 && ncomphys < nrequired_comphys)
> 
> This would cover Maxime's request for sanity checking, which I think is
> valuable, while also maintaining compatibility with platforms that have no
> comphy drivers or some other issue that prevents properly defining comphy nodes
> in the device tree. Does that sound reasonable?

That sounds good.

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH v2 1/3] tipc: fix memory leak issue
From: Eric Dumazet @ 2019-08-12  7:54 UTC (permalink / raw)
  To: Ying Xue, davem, netdev
  Cc: jon.maloy, hdanton, tipc-discussion, syzkaller-bugs,
	jakub.kicinski
In-Reply-To: <1565595162-1383-2-git-send-email-ying.xue@windriver.com>



On 8/12/19 9:32 AM, Ying Xue wrote:
> syzbot found the following memory leak:
> 
> [   68.602482][ T7130] kmemleak: 2 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
> BUG: memory leak
> unreferenced object 0xffff88810df83c00 (size 512):
>   comm "softirq", pid 0, jiffies 4294942354 (age 19.830s)
>   hex dump (first 32 bytes):
>     38 1a 0d 0f 81 88 ff ff 38 1a 0d 0f 81 88 ff ff  8.......8.......
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<000000009375ee42>] kmem_cache_alloc_node+0x153/0x2a0
>     [<000000004c563922>] __alloc_skb+0x6e/0x210
>     [<00000000ec87bfa1>] tipc_buf_acquire+0x2f/0x80
>     [<00000000d151ef84>] tipc_msg_create+0x37/0xe0
>     [<000000008bb437b0>] tipc_group_create_event+0xb3/0x1b0
>     [<00000000947b1d0f>] tipc_group_proto_rcv+0x569/0x640
>     [<00000000b75ab039>] tipc_sk_filter_rcv+0x9ac/0xf20
>     [<000000000dab7a6c>] tipc_sk_rcv+0x494/0x8a0
>     [<00000000023a7ddd>] tipc_node_xmit+0x196/0x1f0
>     [<00000000337dd9eb>] tipc_node_distr_xmit+0x7d/0x120
>     [<00000000b6375182>] tipc_group_delete+0xe6/0x130
>     [<000000000361ba2b>] tipc_sk_leave+0x57/0xb0
>     [<000000009df90505>] tipc_release+0x7b/0x5e0
>     [<000000009f3189da>] __sock_release+0x4b/0xe0
>     [<00000000d3568ee0>] sock_close+0x1b/0x30
>     [<00000000266a6215>] __fput+0xed/0x300
> 
> Reported-by: syzbot+78fbe679c8ca8d264a8d@syzkaller.appspotmail.com
> Signed-off-by: Hillf Danton <hdanton@sina.com>
> Signed-off-by: Ying Xue <ying.xue@windriver.com>
> ---
>  net/tipc/node.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index 7ca0190..d1852fc 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -1469,10 +1469,13 @@ int tipc_node_xmit(struct net *net, struct sk_buff_head *list,
>  	spin_unlock_bh(&le->lock);
>  	tipc_node_read_unlock(n);
>  
> -	if (unlikely(rc == -ENOBUFS))
> +	if (unlikely(rc == -ENOBUFS)) {
>  		tipc_node_link_down(n, bearer_id, false);
> -	else
> +		skb_queue_purge(list);
> +		skb_queue_purge(&xmitq);

This will crash if you enable LOCKDEP

> +	} else {
>  		tipc_bearer_xmit(net, bearer_id, &xmitq, &le->maddr);
> +	}
>  
>  	tipc_node_put(n);
>  
> 

^ permalink raw reply

* [PATCH v2 2/3] tipc: fix memory leak issue
From: Ying Xue @ 2019-08-12  7:32 UTC (permalink / raw)
  To: davem, netdev
  Cc: jon.maloy, hdanton, tipc-discussion, syzkaller-bugs,
	jakub.kicinski
In-Reply-To: <1565595162-1383-1-git-send-email-ying.xue@windriver.com>

syzbot found the following memory leak issue:

[   72.286706][ T7064] kmemleak: 1 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
BUG: memory leak
unreferenced object 0xffff888122bca200 (size 128):
  comm "syz-executor232", pid 7065, jiffies 4294943817 (age 8.880s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 18 a2 bc 22 81 88 ff ff  ..........."....
  backtrace:
    [<000000005bada299>] kmem_cache_alloc_trace+0x145/0x2c0
    [<00000000e7bcdc9f>] tipc_group_create_member+0x3c/0x190
    [<0000000005f56f40>] tipc_group_add_member+0x34/0x40
    [<0000000044406683>] tipc_nametbl_build_group+0x9b/0xf0
    [<000000009f71e803>] tipc_setsockopt+0x170/0x490
    [<000000007f61cbc2>] __sys_setsockopt+0x10f/0x220
    [<00000000cc630372>] __x64_sys_setsockopt+0x26/0x30
    [<00000000ec30be33>] do_syscall_64+0x76/0x1a0
    [<00000000271be3e6>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reported-by: syzbot+f95d90c454864b3b5bc9@syzkaller.appspotmail.com
Signed-off-by: Hillf Danton <hdanton@sina.com>
Signed-off-by: Ying Xue <ying.xue@windriver.com>
---
 net/tipc/group.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/net/tipc/group.c b/net/tipc/group.c
index 5f98d38..cbc540a 100644
--- a/net/tipc/group.c
+++ b/net/tipc/group.c
@@ -273,8 +273,8 @@ static struct tipc_member *tipc_group_find_node(struct tipc_group *grp,
 	return NULL;
 }
 
-static void tipc_group_add_to_tree(struct tipc_group *grp,
-				   struct tipc_member *m)
+struct tipc_member *tipc_group_add_to_tree(struct tipc_group *grp,
+					   struct tipc_member *m)
 {
 	u64 nkey, key = (u64)m->node << 32 | m->port;
 	struct rb_node **n, *parent = NULL;
@@ -282,7 +282,6 @@ static void tipc_group_add_to_tree(struct tipc_group *grp,
 
 	n = &grp->members.rb_node;
 	while (*n) {
-		tmp = container_of(*n, struct tipc_member, tree_node);
 		parent = *n;
 		tmp = container_of(parent, struct tipc_member, tree_node);
 		nkey = (u64)tmp->node << 32 | tmp->port;
@@ -291,17 +290,18 @@ static void tipc_group_add_to_tree(struct tipc_group *grp,
 		else if (key > nkey)
 			n = &(*n)->rb_right;
 		else
-			return;
+			return tmp;
 	}
 	rb_link_node(&m->tree_node, parent, n);
 	rb_insert_color(&m->tree_node, &grp->members);
+	return m;
 }
 
 static struct tipc_member *tipc_group_create_member(struct tipc_group *grp,
 						    u32 node, u32 port,
 						    u32 instance, int state)
 {
-	struct tipc_member *m;
+	struct tipc_member *m, *n;
 
 	m = kzalloc(sizeof(*m), GFP_ATOMIC);
 	if (!m)
@@ -315,10 +315,14 @@ static struct tipc_member *tipc_group_create_member(struct tipc_group *grp,
 	m->instance = instance;
 	m->bc_acked = grp->bc_snd_nxt - 1;
 	grp->member_cnt++;
-	tipc_group_add_to_tree(grp, m);
-	tipc_nlist_add(&grp->dests, m->node);
-	m->state = state;
-	return m;
+	n = tipc_group_add_to_tree(grp, m);
+	if (n == m) {
+		tipc_nlist_add(&grp->dests, m->node);
+		m->state = state;
+	} else {
+		kfree(m);
+	}
+	return n;
 }
 
 void tipc_group_add_member(struct tipc_group *grp, u32 node,
-- 
2.7.4


^ permalink raw reply related

* BUG: unable to handle kernel NULL pointer dereference in rxrpc_unuse_local
From: syzbot @ 2019-08-12  7:52 UTC (permalink / raw)
  To: davem, dhowells, linux-afs, linux-kernel, netdev, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    57c722e9 net/tls: swap sk_write_space on close
git tree:       net
console output: https://syzkaller.appspot.com/x/log.txt?x=13e6c6ee600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=a4c9e9f08e9e8960
dashboard link: https://syzkaller.appspot.com/bug?extid=ae09baad492cce05644a
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+ae09baad492cce05644a@syzkaller.appspotmail.com

BUG: kernel NULL pointer dereference, address: 0000000000000010
#PF: supervisor write access in kernel mode
#PF: error_code(0x0002) - not-present page
PGD 8bbd0067 P4D 8bbd0067 PUD 907a2067 PMD 0
Oops: 0002 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 11895 Comm: syz-executor.3 Not tainted 5.3.0-rc3+ #157
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:arch_atomic_add_return arch/x86/include/asm/atomic.h:167 [inline]
RIP: 0010:arch_atomic_sub_return arch/x86/include/asm/atomic.h:179 [inline]
RIP: 0010:atomic_sub_return include/asm-generic/atomic-instrumented.h:160  
[inline]
RIP: 0010:atomic_dec_return include/linux/atomic-fallback.h:455 [inline]
RIP: 0010:rxrpc_unuse_local+0x23/0x70 net/rxrpc/local_object.c:405
Code: 1f 84 00 00 00 00 00 55 48 89 e5 41 54 49 89 fc 53 bb ff ff ff ff e8  
4c 1a d7 fa 49 8d 7c 24 10 be 04 00 00 00 e8 8d 09 11 fb <f0> 41 0f c1 5c  
24 10 bf 01 00 00 00 89 de e8 aa 1b d7 fa 83 fb 01
RSP: 0018:ffff88806ab1fb28 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 00000000ffffffff RCX: ffffffff869b6f43
RDX: 0000000000000001 RSI: 0000000000000004 RDI: 0000000000000010
RBP: ffff88806ab1fb38 R08: ffff88805f40a480 R09: ffffed1010e9310f
R10: ffffed1010e9310e R11: ffff888087498877 R12: 0000000000000000
R13: ffff88805f48cd92 R14: ffff888087498680 R15: ffff88805f48d208
FS:  00007fe68c1f5700(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000010 CR3: 00000000a1353000 CR4: 00000000001406f0
Call Trace:
  rxrpc_release_sock net/rxrpc/af_rxrpc.c:904 [inline]
  rxrpc_release+0x47d/0x840 net/rxrpc/af_rxrpc.c:930
  __sock_release+0xce/0x280 net/socket.c:590
  sock_close+0x1e/0x30 net/socket.c:1268
  __fput+0x2ff/0x890 fs/file_table.c:280
  ____fput+0x16/0x20 fs/file_table.c:313
  task_work_run+0x145/0x1c0 kernel/task_work.c:113
  get_signal+0x2078/0x2500 kernel/signal.c:2523
  do_signal+0x87/0x1700 arch/x86/kernel/signal.c:815
  exit_to_usermode_loop+0x286/0x380 arch/x86/entry/common.c:159
  prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
  syscall_return_slowpath arch/x86/entry/common.c:274 [inline]
  do_syscall_64+0x5a9/0x6a0 arch/x86/entry/common.c:299
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459829
Code: fd b7 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 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fe68c1f4c78 EFLAGS: 00000246 ORIG_RAX: 000000000000010f
RAX: 0000000000000003 RBX: 0000000000000005 RCX: 0000000000459829
RDX: 0000000020003440 RSI: 0000000000000005 RDI: 00000000200033c0
RBP: 000000000075c070 R08: 0000000000000008 R09: 0000000000000000
R10: 0000000020003480 R11: 0000000000000246 R12: 00007fe68c1f56d4
R13: 00000000004c6706 R14: 00000000004db7b8 R15: 00000000ffffffff
Modules linked in:
CR2: 0000000000000010


---
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#status for how to communicate with syzbot.

^ permalink raw reply

* [PATCH v2 0/3] Fix three issues found by syzbot
From: Ying Xue @ 2019-08-12  7:32 UTC (permalink / raw)
  To: davem, netdev
  Cc: jon.maloy, hdanton, tipc-discussion, syzkaller-bugs,
	jakub.kicinski

In this series, try to fix two memory leak issues and another issue of
calling smp_processor_id() in preemptible context.

Changes since v1:
 - Fix "Reported-by:" missing in patch #3, which was reported by Jakub
   Kicinski

Ying Xue (3):
  tipc: fix memory leak issue
  tipc: fix memory leak issue
  tipc: fix issue of calling smp_processor_id() in preemptible

 net/tipc/group.c     | 22 +++++++++++++---------
 net/tipc/node.c      |  7 +++++--
 net/tipc/udp_media.c | 12 +++++++++---
 3 files changed, 27 insertions(+), 14 deletions(-)

-- 
2.7.4


^ permalink raw reply

* [PATCH v2 3/3] tipc: fix issue of calling smp_processor_id() in preemptible
From: Ying Xue @ 2019-08-12  7:32 UTC (permalink / raw)
  To: davem, netdev
  Cc: jon.maloy, hdanton, tipc-discussion, syzkaller-bugs,
	jakub.kicinski
In-Reply-To: <1565595162-1383-1-git-send-email-ying.xue@windriver.com>

syzbot found the following issue:

[   81.119772][ T8612] BUG: using smp_processor_id() in preemptible [00000000] code: syz-executor834/8612
[   81.136212][ T8612] caller is dst_cache_get+0x3d/0xb0
[   81.141450][ T8612] CPU: 0 PID: 8612 Comm: syz-executor834 Not tainted 5.2.0-rc6+ #48
[   81.149435][ T8612] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
[   81.159480][ T8612] Call Trace:
[   81.162789][ T8612]  dump_stack+0x172/0x1f0
[   81.167123][ T8612]  debug_smp_processor_id+0x251/0x280
[   81.172479][ T8612]  dst_cache_get+0x3d/0xb0
[   81.176928][ T8612]  tipc_udp_xmit.isra.0+0xc4/0xb80
[   81.182046][ T8612]  ? kasan_kmalloc+0x9/0x10
[   81.186531][ T8612]  ? tipc_udp_addr2str+0x170/0x170
[   81.191641][ T8612]  ? __copy_skb_header+0x2e8/0x560
[   81.196750][ T8612]  ? __skb_checksum_complete+0x3f0/0x3f0
[   81.202364][ T8612]  ? netdev_alloc_frag+0x1b0/0x1b0
[   81.207452][ T8612]  ? skb_copy_header+0x21/0x2b0
[   81.212282][ T8612]  ? __pskb_copy_fclone+0x516/0xc90
[   81.217470][ T8612]  tipc_udp_send_msg+0x29a/0x4b0
[   81.222400][ T8612]  tipc_bearer_xmit_skb+0x16c/0x360
[   81.227585][ T8612]  tipc_enable_bearer+0xabe/0xd20
[   81.232606][ T8612]  ? __nla_validate_parse+0x2d0/0x1ee0
[   81.238048][ T8612]  ? tipc_bearer_xmit_skb+0x360/0x360
[   81.243401][ T8612]  ? nla_memcpy+0xb0/0xb0
[   81.247710][ T8612]  ? nla_memcpy+0xb0/0xb0
[   81.252020][ T8612]  ? __nla_parse+0x43/0x60
[   81.256417][ T8612]  __tipc_nl_bearer_enable+0x2de/0x3a0
[   81.261856][ T8612]  ? __tipc_nl_bearer_enable+0x2de/0x3a0
[   81.267467][ T8612]  ? tipc_nl_bearer_disable+0x40/0x40
[   81.272848][ T8612]  ? unwind_get_return_address+0x58/0xa0
[   81.278501][ T8612]  ? lock_acquire+0x16f/0x3f0
[   81.283190][ T8612]  tipc_nl_bearer_enable+0x23/0x40
[   81.288300][ T8612]  genl_family_rcv_msg+0x74b/0xf90
[   81.293404][ T8612]  ? genl_unregister_family+0x790/0x790
[   81.298935][ T8612]  ? __lock_acquire+0x54f/0x5490
[   81.303852][ T8612]  ? __netlink_lookup+0x3fa/0x7b0
[   81.308865][ T8612]  genl_rcv_msg+0xca/0x16c
[   81.313266][ T8612]  netlink_rcv_skb+0x177/0x450
[   81.318043][ T8612]  ? genl_family_rcv_msg+0xf90/0xf90
[   81.323311][ T8612]  ? netlink_ack+0xb50/0xb50
[   81.327906][ T8612]  ? lock_acquire+0x16f/0x3f0
[   81.332589][ T8612]  ? kasan_check_write+0x14/0x20
[   81.337511][ T8612]  genl_rcv+0x29/0x40
[   81.341485][ T8612]  netlink_unicast+0x531/0x710
[   81.346268][ T8612]  ? netlink_attachskb+0x770/0x770
[   81.351374][ T8612]  ? _copy_from_iter_full+0x25d/0x8c0
[   81.356765][ T8612]  ? __sanitizer_cov_trace_cmp8+0x18/0x20
[   81.362479][ T8612]  ? __check_object_size+0x3d/0x42f
[   81.367667][ T8612]  netlink_sendmsg+0x8ae/0xd70
[   81.372415][ T8612]  ? netlink_unicast+0x710/0x710
[   81.377520][ T8612]  ? aa_sock_msg_perm.isra.0+0xba/0x170
[   81.383051][ T8612]  ? apparmor_socket_sendmsg+0x2a/0x30
[   81.388530][ T8612]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
[   81.394775][ T8612]  ? security_socket_sendmsg+0x8d/0xc0
[   81.400240][ T8612]  ? netlink_unicast+0x710/0x710
[   81.405161][ T8612]  sock_sendmsg+0xd7/0x130
[   81.409561][ T8612]  ___sys_sendmsg+0x803/0x920
[   81.414220][ T8612]  ? copy_msghdr_from_user+0x430/0x430
[   81.419667][ T8612]  ? _raw_spin_unlock_irqrestore+0x6b/0xe0
[   81.425461][ T8612]  ? debug_object_active_state+0x25d/0x380
[   81.431255][ T8612]  ? __lock_acquire+0x54f/0x5490
[   81.436174][ T8612]  ? kasan_check_read+0x11/0x20
[   81.441208][ T8612]  ? _raw_spin_unlock_irqrestore+0xa4/0xe0
[   81.447008][ T8612]  ? mark_held_locks+0xf0/0xf0
[   81.451768][ T8612]  ? __call_rcu.constprop.0+0x28b/0x720
[   81.457298][ T8612]  ? call_rcu+0xb/0x10
[   81.461353][ T8612]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
[   81.467589][ T8612]  ? __fget_light+0x1a9/0x230
[   81.472249][ T8612]  ? __fdget+0x1b/0x20
[   81.476301][ T8612]  ? __sanitizer_cov_trace_const_cmp8+0x18/0x20
[   81.482545][ T8612]  __sys_sendmsg+0x105/0x1d0
[   81.487115][ T8612]  ? __ia32_sys_shutdown+0x80/0x80
[   81.492208][ T8612]  ? blkcg_maybe_throttle_current+0x5e2/0xfb0
[   81.498272][ T8612]  ? trace_hardirqs_on_thunk+0x1a/0x1c
[   81.503726][ T8612]  ? do_syscall_64+0x26/0x680
[   81.508385][ T8612]  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
[   81.514444][ T8612]  ? do_syscall_64+0x26/0x680
[   81.519110][ T8612]  __x64_sys_sendmsg+0x78/0xb0
[   81.523862][ T8612]  do_syscall_64+0xfd/0x680
[   81.528352][ T8612]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[   81.534234][ T8612] RIP: 0033:0x444679
[   81.538114][ T8612] 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 1b d8 fb ff c3 66 2e 0f 1f 84 00 00 00 00
[   81.557709][ T8612] RSP: 002b:00007fff0201a8b8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[   81.566147][ T8612] RAX: ffffffffffffffda RBX: 00000000004002e0 RCX: 0000000000444679
[   81.574108][ T8612] RDX: 0000000000000000 RSI: 0000000020000580 RDI: 0000000000000003
[   81.582152][ T8612] RBP: 00000000006cf018 R08: 0000000000000001 R09: 00000000004002e0
[   81.590113][ T8612] R10: 0000000000000008 R11: 0000000000000246 R12: 0000000000402320
[   81.598089][ T8612] R13: 00000000004023b0 R14: 0000000000000000 R15: 0000000000

In commit e9c1a793210f ("tipc: add dst_cache support for udp media")
dst_cache_get() was introduced to be called in tipc_udp_xmit(). But
smp_processor_id() called by dst_cache_get() cannot be invoked in
preemptible context, as a result, the complaint above was reported.

Fixes: e9c1a793210f ("tipc: add dst_cache support for udp media")
Reported-by: syzbot+1a68504d96cd17b33a05@syzkaller.appspotmail.com
Signed-off-by: Hillf Danton <hdanton@sina.com>
Signed-off-by: Ying Xue <ying.xue@windriver.com>
---
 net/tipc/udp_media.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index 287df687..ca3ae2e 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -224,6 +224,8 @@ static int tipc_udp_send_msg(struct net *net, struct sk_buff *skb,
 	struct udp_bearer *ub;
 	int err = 0;
 
+	local_bh_disable();
+
 	if (skb_headroom(skb) < UDP_MIN_HEADROOM) {
 		err = pskb_expand_head(skb, UDP_MIN_HEADROOM, 0, GFP_ATOMIC);
 		if (err)
@@ -237,9 +239,12 @@ static int tipc_udp_send_msg(struct net *net, struct sk_buff *skb,
 		goto out;
 	}
 
-	if (addr->broadcast != TIPC_REPLICAST_SUPPORT)
-		return tipc_udp_xmit(net, skb, ub, src, dst,
-				     &ub->rcast.dst_cache);
+	if (addr->broadcast != TIPC_REPLICAST_SUPPORT) {
+		err = tipc_udp_xmit(net, skb, ub, src, dst,
+				    &ub->rcast.dst_cache);
+		local_bh_enable();
+		return err;
+	}
 
 	/* Replicast, send an skb to each configured IP address */
 	list_for_each_entry_rcu(rcast, &ub->rcast.list, list) {
@@ -259,6 +264,7 @@ static int tipc_udp_send_msg(struct net *net, struct sk_buff *skb,
 	err = 0;
 out:
 	kfree_skb(skb);
+	local_bh_enable();
 	return err;
 }
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH v2 1/3] tipc: fix memory leak issue
From: Ying Xue @ 2019-08-12  7:32 UTC (permalink / raw)
  To: davem, netdev
  Cc: jon.maloy, hdanton, tipc-discussion, syzkaller-bugs,
	jakub.kicinski
In-Reply-To: <1565595162-1383-1-git-send-email-ying.xue@windriver.com>

syzbot found the following memory leak:

[   68.602482][ T7130] kmemleak: 2 new suspected memory leaks (see /sys/kernel/debug/kmemleak)
BUG: memory leak
unreferenced object 0xffff88810df83c00 (size 512):
  comm "softirq", pid 0, jiffies 4294942354 (age 19.830s)
  hex dump (first 32 bytes):
    38 1a 0d 0f 81 88 ff ff 38 1a 0d 0f 81 88 ff ff  8.......8.......
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<000000009375ee42>] kmem_cache_alloc_node+0x153/0x2a0
    [<000000004c563922>] __alloc_skb+0x6e/0x210
    [<00000000ec87bfa1>] tipc_buf_acquire+0x2f/0x80
    [<00000000d151ef84>] tipc_msg_create+0x37/0xe0
    [<000000008bb437b0>] tipc_group_create_event+0xb3/0x1b0
    [<00000000947b1d0f>] tipc_group_proto_rcv+0x569/0x640
    [<00000000b75ab039>] tipc_sk_filter_rcv+0x9ac/0xf20
    [<000000000dab7a6c>] tipc_sk_rcv+0x494/0x8a0
    [<00000000023a7ddd>] tipc_node_xmit+0x196/0x1f0
    [<00000000337dd9eb>] tipc_node_distr_xmit+0x7d/0x120
    [<00000000b6375182>] tipc_group_delete+0xe6/0x130
    [<000000000361ba2b>] tipc_sk_leave+0x57/0xb0
    [<000000009df90505>] tipc_release+0x7b/0x5e0
    [<000000009f3189da>] __sock_release+0x4b/0xe0
    [<00000000d3568ee0>] sock_close+0x1b/0x30
    [<00000000266a6215>] __fput+0xed/0x300

Reported-by: syzbot+78fbe679c8ca8d264a8d@syzkaller.appspotmail.com
Signed-off-by: Hillf Danton <hdanton@sina.com>
Signed-off-by: Ying Xue <ying.xue@windriver.com>
---
 net/tipc/node.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 7ca0190..d1852fc 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1469,10 +1469,13 @@ int tipc_node_xmit(struct net *net, struct sk_buff_head *list,
 	spin_unlock_bh(&le->lock);
 	tipc_node_read_unlock(n);
 
-	if (unlikely(rc == -ENOBUFS))
+	if (unlikely(rc == -ENOBUFS)) {
 		tipc_node_link_down(n, bearer_id, false);
-	else
+		skb_queue_purge(list);
+		skb_queue_purge(&xmitq);
+	} else {
 		tipc_bearer_xmit(net, bearer_id, &xmitq, &le->maddr);
+	}
 
 	tipc_node_put(n);
 
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH v4 1/2] perf machine: Support arch's specific kernel start address
From: Leo Yan @ 2019-08-12  7:39 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
	Milian Wolff, Donald Yandt, Davidlohr Bueso, Wei Li, Mark Drayton,
	Tzvetomir Stoyanov (VMware), linux-kernel, netdev, bpf,
	clang-built-linux, Mathieu Poirier
In-Reply-To: <250165c6-908a-c57e-8d83-03da4272f568@intel.com>

On Mon, Aug 12, 2019 at 10:23:21AM +0300, Adrian Hunter wrote:
> On 12/08/19 10:02 AM, Leo Yan wrote:
> > On Mon, Aug 12, 2019 at 09:37:33AM +0300, Adrian Hunter wrote:
> >> On 10/08/19 10:21 AM, Leo Yan wrote:
> >>> machine__get_kernel_start() gives out the kernel start address; some
> >>> architectures need to tweak the start address so that can reflect the
> >>> kernel start address correctly.  This is not only for x86_64 arch, but
> >>> it is also required by other architectures, e.g. arm/arm64 needs to
> >>> tweak the kernel start address so can include the kernel memory regions
> >>> which are used before the '_stext' symbol.
> >>>
> >>> This patch refactors machine__get_kernel_start() by adding a weak
> >>> arch__fix_kernel_text_start(), any architecture can implement it to
> >>> tweak its specific start address; this also allows the arch specific
> >>> code to be placed into 'arch' folder.
> >>>
> >>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> >>> ---
> >>>  tools/perf/arch/x86/util/machine.c | 10 ++++++++++
> >>>  tools/perf/util/machine.c          | 13 +++++++------
> >>>  tools/perf/util/machine.h          |  2 ++
> >>>  3 files changed, 19 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/tools/perf/arch/x86/util/machine.c b/tools/perf/arch/x86/util/machine.c
> >>> index 1e9ec783b9a1..9f012131534a 100644
> >>> --- a/tools/perf/arch/x86/util/machine.c
> >>> +++ b/tools/perf/arch/x86/util/machine.c
> >>> @@ -101,4 +101,14 @@ int machine__create_extra_kernel_maps(struct machine *machine,
> >>>  	return ret;
> >>>  }
> >>>  
> >>> +void arch__fix_kernel_text_start(u64 *start)
> >>> +{
> >>> +	/*
> >>> +	 * On x86_64, PTI entry trampolines are less than the
> >>> +	 * start of kernel text, but still above 2^63. So leave
> >>> +	 * kernel_start = 1ULL << 63 for x86_64.
> >>> +	 */
> >>> +	*start = 1ULL << 63;
> >>> +}
> >>
> >> That is needed for reporting x86 data on any arch i.e. it is not specific to
> >> the compile-time architecture, it is specific to the perf.data file
> >> architecture, which is what machine__is() compares. So, this looks wrong.
> > 
> > Thanks for reviewing, Adrian.
> > 
> > If so, I think we should extend the function machine__get_kernel_start()
> > as below; for building successfully, will always define the macro
> > ARM_PRE_START_SIZE in Makefile.config.
> > 
> > @Arnaldo, @Adrian, Please let me know if this works for you?
> 
> I don't know how you intend to calculate ARM_PRE_START_SIZE, but below is OK
> for x86.
> 
> > 
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index f6ee7fbad3e4..30a0ff627263 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -2687,13 +2687,26 @@ int machine__get_kernel_start(struct machine *machine)
> >         machine->kernel_start = 1ULL << 63;
> >         if (map) {
> >                 err = map__load(map);
> > +               if (err)
> > +                       return err;
> > +
> >                 /*
> >                  * On x86_64, PTI entry trampolines are less than the
> >                  * start of kernel text, but still above 2^63. So leave
> >                  * kernel_start = 1ULL << 63 for x86_64.
> >                  */
> > -               if (!err && !machine__is(machine, "x86_64"))
> > +               if (!machine__is(machine, "x86_64"))
> >                         machine->kernel_start = map->start;
> > +
> > +               /*
> > +                * On arm/arm64, some memory regions are prior to '_stext'
> > +                * symbol; to reflect the complete kernel address space,
> > +                * compensate these pre-defined regions for kernel start
> > +                * address.
> > +                */
> > +               if (machine__is(machine, "arm64") ||
> > +                   machine__is(machine, "arm"))
> 
> machine__is() does not normalize the architecture, so you may want to use
> perf_env__arch() instead.

You are right, thanks for suggestion.  Will use perf_env__arch() in
next spin.

Thanks,
Leo Yan

> 
> > +                       machine->kernel_start -= ARM_PRE_START_SIZE;
> >         }
> >         return err;
> >  }

[...]

^ permalink raw reply

* Re: [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush
From: Hangbin Liu @ 2019-08-12  7:37 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, joe, tlfalcon
In-Reply-To: <20190811.210820.1168889173898610979.davem@davemloft.net>

On Sun, Aug 11, 2019 at 09:08:20PM -0700, David Miller wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Fri,  9 Aug 2019 08:29:39 +0800
> 
> > ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table

> > error when add more thann 256 memberships in one multicast group. I haven't
> > found this issue on other driver. It looks like an ibm driver issue and need
> > to be fixed separately.
> 
> You need to root cause and fix the reason this message appears so much.
> 
> Once I let you rate limit the message you will have zero incentive to
> fix the real problem and fix it.

Sorry, I'm not familiar with ibmveth driver...

Hi Thomas,

Would you please help check why this issue happens

Thanks
Hangbbin

^ permalink raw reply

* Re: [PING 2] [PATCH v5 1/7] nfc: pn533: i2c: "pn532" as dt compatible string
From: Lars Poeschel @ 2019-08-12  7:40 UTC (permalink / raw)
  To: Johan Hovold
  Cc: devicetree, Samuel Ortiz, open list:NFC SUBSYSTEM, open list,
	netdev
In-Reply-To: <20190805124236.GG3574@localhost>

On Mon, Aug 05, 2019 at 02:42:36PM +0200, Johan Hovold wrote:
> You may want to resend this series to netdev now. David Miller will be
> picking up NFC patches directly from there.

Thank you very much for this information. Johannes Berg did reach out to
me already.
Rebase, test and resend is queued up for one of my next free timeslots.

Thanks again,
Lars

^ permalink raw reply

* [PATCH 1/5] can: xilinx_can: defer the probe if clock is not found
From: Appana Durga Kedareswara rao @ 2019-08-12  7:28 UTC (permalink / raw)
  To: wg, mkl, davem, michal.simek
  Cc: linux-can, netdev, linux-arm-kernel, linux-kernel,
	Venkatesh Yadav Abbarapu
In-Reply-To: <1565594914-18999-1-git-send-email-appana.durga.rao@xilinx.com>

From: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@xilinx.com>

It's not always the case that clock is already available when can
driver get probed at the first time, e.g. the clock is provided by
clock wizard which may be probed after can driver. So let's defer
the probe when devm_clk_get() call fails and give it chance to
try later.

Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/can/xilinx_can.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index bd95cfa..ac175ab 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -1791,7 +1791,8 @@ static int xcan_probe(struct platform_device *pdev)
 	/* Getting the CAN can_clk info */
 	priv->can_clk = devm_clk_get(&pdev->dev, "can_clk");
 	if (IS_ERR(priv->can_clk)) {
-		dev_err(&pdev->dev, "Device clock not found.\n");
+		if (PTR_ERR(priv->can_clk) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Device clock not found.\n");
 		ret = PTR_ERR(priv->can_clk);
 		goto err_free;
 	}
-- 
2.7.4


^ permalink raw reply related

* [PATCH 3/5] can: xilinx_can: Fix the data updation logic for CANFD FD frames
From: Appana Durga Kedareswara rao @ 2019-08-12  7:28 UTC (permalink / raw)
  To: wg, mkl, davem, michal.simek
  Cc: linux-can, netdev, linux-arm-kernel, linux-kernel,
	Appana Durga Kedareswara rao
In-Reply-To: <1565594914-18999-1-git-send-email-appana.durga.rao@xilinx.com>

commit c223da689324 ("can: xilinx_can: Add support for CANFD FD frames")
is writing data to a wrong offset for FD frames.

This patch fixes this issue.

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Reviewed-by: Shubhrajyoti Datta <Shubhrajyoti.datta@xilinx.com>
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/can/xilinx_can.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 2d3399e..c9b951b 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -66,8 +66,7 @@ enum xcan_reg {
 #define XCAN_FRAME_DLC_OFFSET(frame_base)	((frame_base) + 0x04)
 #define XCAN_FRAME_DW1_OFFSET(frame_base)	((frame_base) + 0x08)
 #define XCAN_FRAME_DW2_OFFSET(frame_base)	((frame_base) + 0x0C)
-#define XCANFD_FRAME_DW_OFFSET(frame_base, n)	(((frame_base) + 0x08) + \
-						 ((n) * XCAN_CANFD_FRAME_SIZE))
+#define XCANFD_FRAME_DW_OFFSET(frame_base)	((frame_base) + 0x08)
 
 #define XCAN_CANFD_FRAME_SIZE		0x48
 #define XCAN_TXMSG_FRAME_OFFSET(n)	(XCAN_TXMSG_BASE_OFFSET + \
@@ -600,7 +599,7 @@ static void xcan_write_frame(struct xcan_priv *priv, struct sk_buff *skb,
 	if (priv->devtype.cantype == XAXI_CANFD ||
 	    priv->devtype.cantype == XAXI_CANFD_2_0) {
 		for (i = 0; i < cf->len; i += 4) {
-			ramoff = XCANFD_FRAME_DW_OFFSET(frame_offset, dwindex) +
+			ramoff = XCANFD_FRAME_DW_OFFSET(frame_offset) +
 					(dwindex * XCANFD_DW_BYTES);
 			priv->write_reg(priv, ramoff,
 					be32_to_cpup((__be32 *)(cf->data + i)));
@@ -816,10 +815,8 @@ static int xcanfd_rx(struct net_device *ndev, int frame_base)
 	struct net_device_stats *stats = &ndev->stats;
 	struct canfd_frame *cf;
 	struct sk_buff *skb;
-	u32 id_xcan, dlc, data[2] = {0, 0}, dwindex = 0, i, fsr, readindex;
+	u32 id_xcan, dlc, data[2] = {0, 0}, dwindex = 0, i, dw_offset;
 
-	fsr = priv->read_reg(priv, XCAN_FSR_OFFSET);
-	readindex = fsr & XCAN_FSR_RI_MASK;
 	id_xcan = priv->read_reg(priv, XCAN_FRAME_ID_OFFSET(frame_base));
 	dlc = priv->read_reg(priv, XCAN_FRAME_DLC_OFFSET(frame_base));
 	if (dlc & XCAN_DLCR_EDL_MASK)
@@ -863,26 +860,16 @@ static int xcanfd_rx(struct net_device *ndev, int frame_base)
 	/* Check the frame received is FD or not*/
 	if (dlc & XCAN_DLCR_EDL_MASK) {
 		for (i = 0; i < cf->len; i += 4) {
-			if (priv->devtype.flags & XCAN_FLAG_CANFD_2)
-				data[0] = priv->read_reg(priv,
-					(XCAN_RXMSG_2_FRAME_OFFSET(readindex) +
-					(dwindex * XCANFD_DW_BYTES)));
-			else
-				data[0] = priv->read_reg(priv,
-					(XCAN_RXMSG_FRAME_OFFSET(readindex) +
-					(dwindex * XCANFD_DW_BYTES)));
+			dw_offset = XCANFD_FRAME_DW_OFFSET(frame_base) +
+					(dwindex * XCANFD_DW_BYTES);
+			data[0] = priv->read_reg(priv, dw_offset);
 			*(__be32 *)(cf->data + i) = cpu_to_be32(data[0]);
 			dwindex++;
 		}
 	} else {
 		for (i = 0; i < cf->len; i += 4) {
-			if (priv->devtype.flags & XCAN_FLAG_CANFD_2)
-				data[0] = priv->read_reg(priv,
-					XCAN_RXMSG_2_FRAME_OFFSET(readindex) +
-								  i);
-			else
-				data[0] = priv->read_reg(priv,
-					XCAN_RXMSG_FRAME_OFFSET(readindex) + i);
+			dw_offset = XCANFD_FRAME_DW_OFFSET(frame_base);
+			data[0] = priv->read_reg(priv, dw_offset + i);
 			*(__be32 *)(cf->data + i) = cpu_to_be32(data[0]);
 		}
 	}
-- 
2.7.4


^ permalink raw reply related

* [PATCH 5/5] can: xilinx_can: Fix the data phase btr1 calculation
From: Appana Durga Kedareswara rao @ 2019-08-12  7:28 UTC (permalink / raw)
  To: wg, mkl, davem, michal.simek
  Cc: linux-can, netdev, linux-arm-kernel, linux-kernel, Srinivas Neeli
In-Reply-To: <1565594914-18999-1-git-send-email-appana.durga.rao@xilinx.com>

From: Srinivas Neeli <srinivas.neeli@xilinx.com>

While calculating bitrate for the data phase, the driver is using phase
segment 1 of the arbitration phase instead of the data phase.

Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
Acked-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/can/xilinx_can.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 4cb8c1c9..ab26691 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -425,7 +425,7 @@ static int xcan_set_bittiming(struct net_device *ndev)
 		btr0 = dbt->brp - 1;
 
 		/* Setting Time Segment 1 in BTR Register */
-		btr1 = dbt->prop_seg + bt->phase_seg1 - 1;
+		btr1 = dbt->prop_seg + dbt->phase_seg1 - 1;
 
 		/* Setting Time Segment 2 in BTR Register */
 		btr1 |= (dbt->phase_seg2 - 1) << priv->devtype.btr_ts2_shift;
-- 
2.7.4


^ permalink raw reply related

* [PATCH 2/5] can: xilinx_can: Fix FSR register handling in the rx path
From: Appana Durga Kedareswara rao @ 2019-08-12  7:28 UTC (permalink / raw)
  To: wg, mkl, davem, michal.simek
  Cc: linux-can, netdev, linux-arm-kernel, linux-kernel,
	Appana Durga Kedareswara rao
In-Reply-To: <1565594914-18999-1-git-send-email-appana.durga.rao@xilinx.com>

After commit c223da689324 ("can: xilinx_can: Add support for
CANFD FD frames") Driver is updating the FSR IRI index multiple
times(i.e in xcanfd_rx() and xcan_rx_fifo_get_next_frame()),
It should be updated once per rx packet this patch fixes this issue,
also this patch removes the unnecessary fsr register checks in
xcanfd_rx() API.

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Reviewed-by: Shubhrajyoti Datta <Shubhrajyoti.datta@xilinx.com>
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/can/xilinx_can.c | 139 ++++++++++++++++++++-----------------------
 1 file changed, 63 insertions(+), 76 deletions(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index ac175ab..2d3399e 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -819,91 +819,78 @@ static int xcanfd_rx(struct net_device *ndev, int frame_base)
 	u32 id_xcan, dlc, data[2] = {0, 0}, dwindex = 0, i, fsr, readindex;
 
 	fsr = priv->read_reg(priv, XCAN_FSR_OFFSET);
-	if (fsr & XCAN_FSR_FL_MASK) {
-		readindex = fsr & XCAN_FSR_RI_MASK;
-		id_xcan = priv->read_reg(priv,
-					 XCAN_FRAME_ID_OFFSET(frame_base));
-		dlc = priv->read_reg(priv, XCAN_FRAME_DLC_OFFSET(frame_base));
-		if (dlc & XCAN_DLCR_EDL_MASK)
-			skb = alloc_canfd_skb(ndev, &cf);
-		else
-			skb = alloc_can_skb(ndev, (struct can_frame **)&cf);
+	readindex = fsr & XCAN_FSR_RI_MASK;
+	id_xcan = priv->read_reg(priv, XCAN_FRAME_ID_OFFSET(frame_base));
+	dlc = priv->read_reg(priv, XCAN_FRAME_DLC_OFFSET(frame_base));
+	if (dlc & XCAN_DLCR_EDL_MASK)
+		skb = alloc_canfd_skb(ndev, &cf);
+	else
+		skb = alloc_can_skb(ndev, (struct can_frame **)&cf);
 
-		if (unlikely(!skb)) {
-			stats->rx_dropped++;
-			return 0;
-		}
+	if (unlikely(!skb)) {
+		stats->rx_dropped++;
+		return 0;
+	}
 
-		/* Change Xilinx CANFD data length format to socketCAN data
-		 * format
-		 */
-		if (dlc & XCAN_DLCR_EDL_MASK)
-			cf->len = can_dlc2len((dlc & XCAN_DLCR_DLC_MASK) >>
+	/* Change Xilinx CANFD data length format to socketCAN data
+	 * format
+	 */
+	if (dlc & XCAN_DLCR_EDL_MASK)
+		cf->len = can_dlc2len((dlc & XCAN_DLCR_DLC_MASK) >>
+				  XCAN_DLCR_DLC_SHIFT);
+	else
+		cf->len = get_can_dlc((dlc & XCAN_DLCR_DLC_MASK) >>
 					  XCAN_DLCR_DLC_SHIFT);
-		else
-			cf->len = get_can_dlc((dlc & XCAN_DLCR_DLC_MASK) >>
-						  XCAN_DLCR_DLC_SHIFT);
-
-		/* Change Xilinx CAN ID format to socketCAN ID format */
-		if (id_xcan & XCAN_IDR_IDE_MASK) {
-			/* The received frame is an Extended format frame */
-			cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >> 3;
-			cf->can_id |= (id_xcan & XCAN_IDR_ID2_MASK) >>
-					XCAN_IDR_ID2_SHIFT;
-			cf->can_id |= CAN_EFF_FLAG;
-			if (id_xcan & XCAN_IDR_RTR_MASK)
-				cf->can_id |= CAN_RTR_FLAG;
-		} else {
-			/* The received frame is a standard format frame */
-			cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >>
-					XCAN_IDR_ID1_SHIFT;
-			if (!(dlc & XCAN_DLCR_EDL_MASK) && (id_xcan &
-						XCAN_IDR_SRR_MASK))
-				cf->can_id |= CAN_RTR_FLAG;
-		}
 
-		/* Check the frame received is FD or not*/
-		if (dlc & XCAN_DLCR_EDL_MASK) {
-			for (i = 0; i < cf->len; i += 4) {
-				if (priv->devtype.flags & XCAN_FLAG_CANFD_2)
-					data[0] = priv->read_reg(priv,
+	/* Change Xilinx CAN ID format to socketCAN ID format */
+	if (id_xcan & XCAN_IDR_IDE_MASK) {
+		/* The received frame is an Extended format frame */
+		cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >> 3;
+		cf->can_id |= (id_xcan & XCAN_IDR_ID2_MASK) >>
+				XCAN_IDR_ID2_SHIFT;
+		cf->can_id |= CAN_EFF_FLAG;
+		if (id_xcan & XCAN_IDR_RTR_MASK)
+			cf->can_id |= CAN_RTR_FLAG;
+	} else {
+		/* The received frame is a standard format frame */
+		cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >>
+				XCAN_IDR_ID1_SHIFT;
+		if (!(dlc & XCAN_DLCR_EDL_MASK) && (id_xcan &
+					XCAN_IDR_SRR_MASK))
+			cf->can_id |= CAN_RTR_FLAG;
+	}
+
+	/* Check the frame received is FD or not*/
+	if (dlc & XCAN_DLCR_EDL_MASK) {
+		for (i = 0; i < cf->len; i += 4) {
+			if (priv->devtype.flags & XCAN_FLAG_CANFD_2)
+				data[0] = priv->read_reg(priv,
 					(XCAN_RXMSG_2_FRAME_OFFSET(readindex) +
 					(dwindex * XCANFD_DW_BYTES)));
-				else
-					data[0] = priv->read_reg(priv,
+			else
+				data[0] = priv->read_reg(priv,
 					(XCAN_RXMSG_FRAME_OFFSET(readindex) +
-						(dwindex * XCANFD_DW_BYTES)));
-				*(__be32 *)(cf->data + i) =
-						cpu_to_be32(data[0]);
-				dwindex++;
-			}
-		} else {
-			for (i = 0; i < cf->len; i += 4) {
-				if (priv->devtype.flags & XCAN_FLAG_CANFD_2)
-					data[0] = priv->read_reg(priv,
-						XCAN_RXMSG_2_FRAME_OFFSET(readindex) + i);
-				else
-					data[0] = priv->read_reg(priv,
-						XCAN_RXMSG_FRAME_OFFSET(readindex) + i);
-				*(__be32 *)(cf->data + i) =
-						cpu_to_be32(data[0]);
-			}
+					(dwindex * XCANFD_DW_BYTES)));
+			*(__be32 *)(cf->data + i) = cpu_to_be32(data[0]);
+			dwindex++;
+		}
+	} else {
+		for (i = 0; i < cf->len; i += 4) {
+			if (priv->devtype.flags & XCAN_FLAG_CANFD_2)
+				data[0] = priv->read_reg(priv,
+					XCAN_RXMSG_2_FRAME_OFFSET(readindex) +
+								  i);
+			else
+				data[0] = priv->read_reg(priv,
+					XCAN_RXMSG_FRAME_OFFSET(readindex) + i);
+			*(__be32 *)(cf->data + i) = cpu_to_be32(data[0]);
 		}
-		/* Update FSR Register so that next packet will save to
-		 * buffer
-		 */
-		fsr = priv->read_reg(priv, XCAN_FSR_OFFSET);
-		fsr |= XCAN_FSR_IRI_MASK;
-		priv->write_reg(priv, XCAN_FSR_OFFSET, fsr);
-		fsr = priv->read_reg(priv, XCAN_FSR_OFFSET);
-		stats->rx_bytes += cf->len;
-		stats->rx_packets++;
-		netif_receive_skb(skb);
-
-		return 1;
 	}
-	/* If FSR Register is not updated with fill level */
-	return 0;
+	stats->rx_bytes += cf->len;
+	stats->rx_packets++;
+	netif_receive_skb(skb);
+
+	return 1;
 }
 
 /**
-- 
2.7.4


^ permalink raw reply related

* [PATCH 4/5] can: xilinx_can: Fix FSR register FL and RI mask values for canfd 2.0
From: Appana Durga Kedareswara rao @ 2019-08-12  7:28 UTC (permalink / raw)
  To: wg, mkl, davem, michal.simek
  Cc: linux-can, netdev, linux-arm-kernel, linux-kernel,
	Appana Durga Kedareswara rao
In-Reply-To: <1565594914-18999-1-git-send-email-appana.durga.rao@xilinx.com>

For CANFD 2.0 IP configuration existing driver is using incorrect mask
values for FSR register FL and RI fields.

Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Acked-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/can/xilinx_can.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index c9b951b..4cb8c1c9 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -123,8 +123,10 @@ enum xcan_reg {
 #define XCAN_IDR_RTR_MASK		0x00000001 /* Remote TX request */
 #define XCAN_DLCR_DLC_MASK		0xF0000000 /* Data length code */
 #define XCAN_FSR_FL_MASK		0x00003F00 /* RX Fill Level */
+#define XCAN_2_FSR_FL_MASK		0x00007F00 /* RX Fill Level */
 #define XCAN_FSR_IRI_MASK		0x00000080 /* RX Increment Read Index */
 #define XCAN_FSR_RI_MASK		0x0000001F /* RX Read Index */
+#define XCAN_2_FSR_RI_MASK		0x0000003F /* RX Read Index */
 #define XCAN_DLCR_EDL_MASK		0x08000000 /* EDL Mask in DLC */
 #define XCAN_DLCR_BRS_MASK		0x04000000 /* BRS Mask in DLC */
 
@@ -1138,7 +1140,7 @@ static int xcan_rx_fifo_get_next_frame(struct xcan_priv *priv)
 	int offset;
 
 	if (priv->devtype.flags & XCAN_FLAG_RX_FIFO_MULTI) {
-		u32 fsr;
+		u32 fsr, mask;
 
 		/* clear RXOK before the is-empty check so that any newly
 		 * received frame will reassert it without a race
@@ -1148,12 +1150,17 @@ static int xcan_rx_fifo_get_next_frame(struct xcan_priv *priv)
 		fsr = priv->read_reg(priv, XCAN_FSR_OFFSET);
 
 		/* check if RX FIFO is empty */
-		if (!(fsr & XCAN_FSR_FL_MASK))
+		if (priv->devtype.flags & XCAN_FLAG_CANFD_2)
+			mask = XCAN_2_FSR_FL_MASK;
+		else
+			mask = XCAN_FSR_FL_MASK;
+
+		if (!(fsr & mask))
 			return -ENOENT;
 
 		if (priv->devtype.flags & XCAN_FLAG_CANFD_2)
 			offset =
-			  XCAN_RXMSG_2_FRAME_OFFSET(fsr & XCAN_FSR_RI_MASK);
+			  XCAN_RXMSG_2_FRAME_OFFSET(fsr & XCAN_2_FSR_RI_MASK);
 		else
 			offset =
 			  XCAN_RXMSG_FRAME_OFFSET(fsr & XCAN_FSR_RI_MASK);
-- 
2.7.4


^ permalink raw reply related

* [PATCH 0/5] can: xilinx_can: Bug fixes
From: Appana Durga Kedareswara rao @ 2019-08-12  7:28 UTC (permalink / raw)
  To: wg, mkl, davem, michal.simek
  Cc: linux-can, netdev, linux-arm-kernel, linux-kernel,
	Appana Durga Kedareswara rao

This patch series fixes below issues
--> Bugs in the driver w.r.to CANFD 2.0 IP support
--> Defer the probe if clock is not found

Appana Durga Kedareswara rao (3):
  can: xilinx_can: Fix FSR register handling in the rx path
  can: xilinx_can: Fix the data updation logic for CANFD FD frames
  can: xilinx_can: Fix FSR register FL and RI mask values for canfd 2.0

Srinivas Neeli (1):
  can: xilinx_can: Fix the data phase btr1 calculation

Venkatesh Yadav Abbarapu (1):
  can: xilinx_can: defer the probe if clock is not found

 drivers/net/can/xilinx_can.c | 162 +++++++++++++++++++------------------------
 1 file changed, 72 insertions(+), 90 deletions(-)

-- 
2.7.4


^ permalink raw reply

* Re: [PATCH v4 1/2] perf machine: Support arch's specific kernel start address
From: Adrian Hunter @ 2019-08-12  7:23 UTC (permalink / raw)
  To: Leo Yan
  Cc: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
	Milian Wolff, Donald Yandt, Davidlohr Bueso, Wei Li, Mark Drayton,
	Tzvetomir Stoyanov (VMware), linux-kernel, netdev, bpf,
	clang-built-linux, Mathieu Poirier
In-Reply-To: <20190812070236.GA8062@leoy-ThinkPad-X240s>

On 12/08/19 10:02 AM, Leo Yan wrote:
> On Mon, Aug 12, 2019 at 09:37:33AM +0300, Adrian Hunter wrote:
>> On 10/08/19 10:21 AM, Leo Yan wrote:
>>> machine__get_kernel_start() gives out the kernel start address; some
>>> architectures need to tweak the start address so that can reflect the
>>> kernel start address correctly.  This is not only for x86_64 arch, but
>>> it is also required by other architectures, e.g. arm/arm64 needs to
>>> tweak the kernel start address so can include the kernel memory regions
>>> which are used before the '_stext' symbol.
>>>
>>> This patch refactors machine__get_kernel_start() by adding a weak
>>> arch__fix_kernel_text_start(), any architecture can implement it to
>>> tweak its specific start address; this also allows the arch specific
>>> code to be placed into 'arch' folder.
>>>
>>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
>>> ---
>>>  tools/perf/arch/x86/util/machine.c | 10 ++++++++++
>>>  tools/perf/util/machine.c          | 13 +++++++------
>>>  tools/perf/util/machine.h          |  2 ++
>>>  3 files changed, 19 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tools/perf/arch/x86/util/machine.c b/tools/perf/arch/x86/util/machine.c
>>> index 1e9ec783b9a1..9f012131534a 100644
>>> --- a/tools/perf/arch/x86/util/machine.c
>>> +++ b/tools/perf/arch/x86/util/machine.c
>>> @@ -101,4 +101,14 @@ int machine__create_extra_kernel_maps(struct machine *machine,
>>>  	return ret;
>>>  }
>>>  
>>> +void arch__fix_kernel_text_start(u64 *start)
>>> +{
>>> +	/*
>>> +	 * On x86_64, PTI entry trampolines are less than the
>>> +	 * start of kernel text, but still above 2^63. So leave
>>> +	 * kernel_start = 1ULL << 63 for x86_64.
>>> +	 */
>>> +	*start = 1ULL << 63;
>>> +}
>>
>> That is needed for reporting x86 data on any arch i.e. it is not specific to
>> the compile-time architecture, it is specific to the perf.data file
>> architecture, which is what machine__is() compares. So, this looks wrong.
> 
> Thanks for reviewing, Adrian.
> 
> If so, I think we should extend the function machine__get_kernel_start()
> as below; for building successfully, will always define the macro
> ARM_PRE_START_SIZE in Makefile.config.
> 
> @Arnaldo, @Adrian, Please let me know if this works for you?

I don't know how you intend to calculate ARM_PRE_START_SIZE, but below is OK
for x86.

> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index f6ee7fbad3e4..30a0ff627263 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -2687,13 +2687,26 @@ int machine__get_kernel_start(struct machine *machine)
>         machine->kernel_start = 1ULL << 63;
>         if (map) {
>                 err = map__load(map);
> +               if (err)
> +                       return err;
> +
>                 /*
>                  * On x86_64, PTI entry trampolines are less than the
>                  * start of kernel text, but still above 2^63. So leave
>                  * kernel_start = 1ULL << 63 for x86_64.
>                  */
> -               if (!err && !machine__is(machine, "x86_64"))
> +               if (!machine__is(machine, "x86_64"))
>                         machine->kernel_start = map->start;
> +
> +               /*
> +                * On arm/arm64, some memory regions are prior to '_stext'
> +                * symbol; to reflect the complete kernel address space,
> +                * compensate these pre-defined regions for kernel start
> +                * address.
> +                */
> +               if (machine__is(machine, "arm64") ||
> +                   machine__is(machine, "arm"))

machine__is() does not normalize the architecture, so you may want to use
perf_env__arch() instead.

> +                       machine->kernel_start -= ARM_PRE_START_SIZE;
>         }
>         return err;
>  }
> 
> Thanks,
> Leo Yan
> 
>>> +
>>>  #endif
>>> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
>>> index f6ee7fbad3e4..603518835692 100644
>>> --- a/tools/perf/util/machine.c
>>> +++ b/tools/perf/util/machine.c
>>> @@ -2671,6 +2671,10 @@ int machine__nr_cpus_avail(struct machine *machine)
>>>  	return machine ? perf_env__nr_cpus_avail(machine->env) : 0;
>>>  }
>>>  
>>> +void __weak arch__fix_kernel_text_start(u64 *start __maybe_unused)
>>> +{
>>> +}
>>> +
>>>  int machine__get_kernel_start(struct machine *machine)
>>>  {
>>>  	struct map *map = machine__kernel_map(machine);
>>> @@ -2687,14 +2691,11 @@ int machine__get_kernel_start(struct machine *machine)
>>>  	machine->kernel_start = 1ULL << 63;
>>>  	if (map) {
>>>  		err = map__load(map);
>>> -		/*
>>> -		 * On x86_64, PTI entry trampolines are less than the
>>> -		 * start of kernel text, but still above 2^63. So leave
>>> -		 * kernel_start = 1ULL << 63 for x86_64.
>>> -		 */
>>> -		if (!err && !machine__is(machine, "x86_64"))
>>> +		if (!err)
>>>  			machine->kernel_start = map->start;
>>>  	}
>>> +
>>> +	arch__fix_kernel_text_start(&machine->kernel_start);
>>>  	return err;
>>>  }
>>>  
>>> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
>>> index ef803f08ae12..9cb459f4bfbc 100644
>>> --- a/tools/perf/util/machine.h
>>> +++ b/tools/perf/util/machine.h
>>> @@ -278,6 +278,8 @@ void machine__get_kallsyms_filename(struct machine *machine, char *buf,
>>>  int machine__create_extra_kernel_maps(struct machine *machine,
>>>  				      struct dso *kernel);
>>>  
>>> +void arch__fix_kernel_text_start(u64 *start);
>>> +
>>>  /* Kernel-space maps for symbols that are outside the main kernel map and module maps */
>>>  struct extra_kernel_map {
>>>  	u64 start;
>>>
>>
> 


^ permalink raw reply

* Re: [PATCH v3 bpf-next] btf: expose BTF info through sysfs
From: Greg KH @ 2019-08-12  7:08 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, netdev, ast, daniel, yhs, andrii.nakryiko, kernel-team,
	Masahiro Yamada, Arnaldo Carvalho de Melo, Jiri Olsa,
	Sam Ravnborg
In-Reply-To: <20190812061405.2290824-1-andriin@fb.com>

On Sun, Aug 11, 2019 at 11:14:05PM -0700, Andrii Nakryiko wrote:
> Make .BTF section allocated and expose its contents through sysfs.
> 
> /sys/kernel/btf directory is created to contain all the BTFs present
> inside kernel. Currently there is only kernel's main BTF, represented as
> /sys/kernel/btf/kernel file. Once kernel modules' BTFs are supported,
> each module will expose its BTF as /sys/kernel/btf/<module-name> file.
> 
> Current approach relies on a few pieces coming together:
> 1. pahole is used to take almost final vmlinux image (modulo .BTF and
>    kallsyms) and generate .BTF section by converting DWARF info into
>    BTF. This section is not allocated and not mapped to any segment,
>    though, so is not yet accessible from inside kernel at runtime.
> 2. objcopy dumps .BTF contents into binary file and subsequently
>    convert binary file into linkable object file with automatically
>    generated symbols _binary__btf_kernel_bin_start and
>    _binary__btf_kernel_bin_end, pointing to start and end, respectively,
>    of BTF raw data.
> 3. final vmlinux image is generated by linking this object file (and
>    kallsyms, if necessary). sysfs_btf.c then creates
>    /sys/kernel/btf/kernel file and exposes embedded BTF contents through
>    it. This allows, e.g., libbpf and bpftool access BTF info at
>    well-known location, without resorting to searching for vmlinux image
>    on disk (location of which is not standardized and vmlinux image
>    might not be even available in some scenarios, e.g., inside qemu
>    during testing).
> 
> Alternative approach using .incbin assembler directive to embed BTF
> contents directly was attempted but didn't work, because sysfs_proc.o is
> not re-compiled during link-vmlinux.sh stage. This is required, though,
> to update embedded BTF data (initially empty data is embedded, then
> pahole generates BTF info and we need to regenerate sysfs_btf.o with
> updated contents, but it's too late at that point).
> 
> If BTF couldn't be generated due to missing or too old pahole,
> sysfs_btf.c handles that gracefully by detecting that
> _binary__btf_kernel_bin_start (weak symbol) is 0 and not creating
> /sys/kernel/btf at all.
> 
> v2->v3:
> - added Documentation/ABI/testing/sysfs-kernel-btf (Greg K-H);
> - created proper kobject (btf_kobj) for btf directory (Greg K-H);
> - undo v2 change of reusing vmlinux, as it causes extra kallsyms pass
>   due to initially missing  __binary__btf_kernel_bin_{start/end} symbols;
> 
> v1->v2:
> - allow kallsyms stage to re-use vmlinux generated by gen_btf();
> 
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  Documentation/ABI/testing/sysfs-kernel-btf | 17 +++++++
>  kernel/bpf/Makefile                        |  3 ++
>  kernel/bpf/sysfs_btf.c                     | 51 +++++++++++++++++++++
>  scripts/link-vmlinux.sh                    | 52 ++++++++++++++--------
>  4 files changed, 104 insertions(+), 19 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-kernel-btf
>  create mode 100644 kernel/bpf/sysfs_btf.c

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply

* Re: [PATCH v4 1/2] perf machine: Support arch's specific kernel start address
From: Leo Yan @ 2019-08-12  7:02 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
	Milian Wolff, Donald Yandt, Davidlohr Bueso, Wei Li, Mark Drayton,
	Tzvetomir Stoyanov (VMware), linux-kernel, netdev, bpf,
	clang-built-linux, Mathieu Poirier
In-Reply-To: <c1818f6f-37df-6971-fddc-6663e5b6ff95@intel.com>

On Mon, Aug 12, 2019 at 09:37:33AM +0300, Adrian Hunter wrote:
> On 10/08/19 10:21 AM, Leo Yan wrote:
> > machine__get_kernel_start() gives out the kernel start address; some
> > architectures need to tweak the start address so that can reflect the
> > kernel start address correctly.  This is not only for x86_64 arch, but
> > it is also required by other architectures, e.g. arm/arm64 needs to
> > tweak the kernel start address so can include the kernel memory regions
> > which are used before the '_stext' symbol.
> > 
> > This patch refactors machine__get_kernel_start() by adding a weak
> > arch__fix_kernel_text_start(), any architecture can implement it to
> > tweak its specific start address; this also allows the arch specific
> > code to be placed into 'arch' folder.
> > 
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  tools/perf/arch/x86/util/machine.c | 10 ++++++++++
> >  tools/perf/util/machine.c          | 13 +++++++------
> >  tools/perf/util/machine.h          |  2 ++
> >  3 files changed, 19 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/perf/arch/x86/util/machine.c b/tools/perf/arch/x86/util/machine.c
> > index 1e9ec783b9a1..9f012131534a 100644
> > --- a/tools/perf/arch/x86/util/machine.c
> > +++ b/tools/perf/arch/x86/util/machine.c
> > @@ -101,4 +101,14 @@ int machine__create_extra_kernel_maps(struct machine *machine,
> >  	return ret;
> >  }
> >  
> > +void arch__fix_kernel_text_start(u64 *start)
> > +{
> > +	/*
> > +	 * On x86_64, PTI entry trampolines are less than the
> > +	 * start of kernel text, but still above 2^63. So leave
> > +	 * kernel_start = 1ULL << 63 for x86_64.
> > +	 */
> > +	*start = 1ULL << 63;
> > +}
> 
> That is needed for reporting x86 data on any arch i.e. it is not specific to
> the compile-time architecture, it is specific to the perf.data file
> architecture, which is what machine__is() compares. So, this looks wrong.

Thanks for reviewing, Adrian.

If so, I think we should extend the function machine__get_kernel_start()
as below; for building successfully, will always define the macro
ARM_PRE_START_SIZE in Makefile.config.

@Arnaldo, @Adrian, Please let me know if this works for you?

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index f6ee7fbad3e4..30a0ff627263 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2687,13 +2687,26 @@ int machine__get_kernel_start(struct machine *machine)
        machine->kernel_start = 1ULL << 63;
        if (map) {
                err = map__load(map);
+               if (err)
+                       return err;
+
                /*
                 * On x86_64, PTI entry trampolines are less than the
                 * start of kernel text, but still above 2^63. So leave
                 * kernel_start = 1ULL << 63 for x86_64.
                 */
-               if (!err && !machine__is(machine, "x86_64"))
+               if (!machine__is(machine, "x86_64"))
                        machine->kernel_start = map->start;
+
+               /*
+                * On arm/arm64, some memory regions are prior to '_stext'
+                * symbol; to reflect the complete kernel address space,
+                * compensate these pre-defined regions for kernel start
+                * address.
+                */
+               if (machine__is(machine, "arm64") ||
+                   machine__is(machine, "arm"))
+                       machine->kernel_start -= ARM_PRE_START_SIZE;
        }
        return err;
 }

Thanks,
Leo Yan

> > +
> >  #endif
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index f6ee7fbad3e4..603518835692 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -2671,6 +2671,10 @@ int machine__nr_cpus_avail(struct machine *machine)
> >  	return machine ? perf_env__nr_cpus_avail(machine->env) : 0;
> >  }
> >  
> > +void __weak arch__fix_kernel_text_start(u64 *start __maybe_unused)
> > +{
> > +}
> > +
> >  int machine__get_kernel_start(struct machine *machine)
> >  {
> >  	struct map *map = machine__kernel_map(machine);
> > @@ -2687,14 +2691,11 @@ int machine__get_kernel_start(struct machine *machine)
> >  	machine->kernel_start = 1ULL << 63;
> >  	if (map) {
> >  		err = map__load(map);
> > -		/*
> > -		 * On x86_64, PTI entry trampolines are less than the
> > -		 * start of kernel text, but still above 2^63. So leave
> > -		 * kernel_start = 1ULL << 63 for x86_64.
> > -		 */
> > -		if (!err && !machine__is(machine, "x86_64"))
> > +		if (!err)
> >  			machine->kernel_start = map->start;
> >  	}
> > +
> > +	arch__fix_kernel_text_start(&machine->kernel_start);
> >  	return err;
> >  }
> >  
> > diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> > index ef803f08ae12..9cb459f4bfbc 100644
> > --- a/tools/perf/util/machine.h
> > +++ b/tools/perf/util/machine.h
> > @@ -278,6 +278,8 @@ void machine__get_kallsyms_filename(struct machine *machine, char *buf,
> >  int machine__create_extra_kernel_maps(struct machine *machine,
> >  				      struct dso *kernel);
> >  
> > +void arch__fix_kernel_text_start(u64 *start);
> > +
> >  /* Kernel-space maps for symbols that are outside the main kernel map and module maps */
> >  struct extra_kernel_map {
> >  	u64 start;
> > 
> 

^ permalink raw reply related

* Re: [PATCH v4 1/2] perf machine: Support arch's specific kernel start address
From: Adrian Hunter @ 2019-08-12  6:37 UTC (permalink / raw)
  To: Leo Yan, Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
	Milian Wolff, Donald Yandt, Davidlohr Bueso, Wei Li, Mark Drayton,
	Tzvetomir Stoyanov (VMware), linux-kernel, netdev, bpf,
	clang-built-linux, Mathieu Poirier
In-Reply-To: <20190810072135.27072-2-leo.yan@linaro.org>

On 10/08/19 10:21 AM, Leo Yan wrote:
> machine__get_kernel_start() gives out the kernel start address; some
> architectures need to tweak the start address so that can reflect the
> kernel start address correctly.  This is not only for x86_64 arch, but
> it is also required by other architectures, e.g. arm/arm64 needs to
> tweak the kernel start address so can include the kernel memory regions
> which are used before the '_stext' symbol.
> 
> This patch refactors machine__get_kernel_start() by adding a weak
> arch__fix_kernel_text_start(), any architecture can implement it to
> tweak its specific start address; this also allows the arch specific
> code to be placed into 'arch' folder.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/arch/x86/util/machine.c | 10 ++++++++++
>  tools/perf/util/machine.c          | 13 +++++++------
>  tools/perf/util/machine.h          |  2 ++
>  3 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/arch/x86/util/machine.c b/tools/perf/arch/x86/util/machine.c
> index 1e9ec783b9a1..9f012131534a 100644
> --- a/tools/perf/arch/x86/util/machine.c
> +++ b/tools/perf/arch/x86/util/machine.c
> @@ -101,4 +101,14 @@ int machine__create_extra_kernel_maps(struct machine *machine,
>  	return ret;
>  }
>  
> +void arch__fix_kernel_text_start(u64 *start)
> +{
> +	/*
> +	 * On x86_64, PTI entry trampolines are less than the
> +	 * start of kernel text, but still above 2^63. So leave
> +	 * kernel_start = 1ULL << 63 for x86_64.
> +	 */
> +	*start = 1ULL << 63;
> +}

That is needed for reporting x86 data on any arch i.e. it is not specific to
the compile-time architecture, it is specific to the perf.data file
architecture, which is what machine__is() compares. So, this looks wrong.

> +
>  #endif
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index f6ee7fbad3e4..603518835692 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -2671,6 +2671,10 @@ int machine__nr_cpus_avail(struct machine *machine)
>  	return machine ? perf_env__nr_cpus_avail(machine->env) : 0;
>  }
>  
> +void __weak arch__fix_kernel_text_start(u64 *start __maybe_unused)
> +{
> +}
> +
>  int machine__get_kernel_start(struct machine *machine)
>  {
>  	struct map *map = machine__kernel_map(machine);
> @@ -2687,14 +2691,11 @@ int machine__get_kernel_start(struct machine *machine)
>  	machine->kernel_start = 1ULL << 63;
>  	if (map) {
>  		err = map__load(map);
> -		/*
> -		 * On x86_64, PTI entry trampolines are less than the
> -		 * start of kernel text, but still above 2^63. So leave
> -		 * kernel_start = 1ULL << 63 for x86_64.
> -		 */
> -		if (!err && !machine__is(machine, "x86_64"))
> +		if (!err)
>  			machine->kernel_start = map->start;
>  	}
> +
> +	arch__fix_kernel_text_start(&machine->kernel_start);
>  	return err;
>  }
>  
> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> index ef803f08ae12..9cb459f4bfbc 100644
> --- a/tools/perf/util/machine.h
> +++ b/tools/perf/util/machine.h
> @@ -278,6 +278,8 @@ void machine__get_kallsyms_filename(struct machine *machine, char *buf,
>  int machine__create_extra_kernel_maps(struct machine *machine,
>  				      struct dso *kernel);
>  
> +void arch__fix_kernel_text_start(u64 *start);
> +
>  /* Kernel-space maps for symbols that are outside the main kernel map and module maps */
>  struct extra_kernel_map {
>  	u64 start;
> 


^ permalink raw reply

* [PATCH] net/mlx4_en: fix a memory leak bug
From: Wenwen Wang @ 2019-08-12  6:36 UTC (permalink / raw)
  To: Wenwen Wang
  Cc: Tariq Toukan, David S. Miller,
	open list:MELLANOX ETHERNET DRIVER (mlx4_en),
	open list:MELLANOX MLX4 core VPI driver, open list

In mlx4_en_config_rss_steer(), 'rss_map->indir_qp' is allocated through
kzalloc(). After that, mlx4_qp_alloc() is invoked to configure RSS
indirection. However, if mlx4_qp_alloc() fails, the allocated
'rss_map->indir_qp' is not deallocated, leading to a memory leak bug.

To fix the above issue, add the 'mlx4_err' label to free
'rss_map->indir_qp'.

Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
---
 drivers/net/ethernet/mellanox/mlx4/en_rx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 6c01314..9476dbd 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -1187,7 +1187,7 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
 	err = mlx4_qp_alloc(mdev->dev, priv->base_qpn, rss_map->indir_qp);
 	if (err) {
 		en_err(priv, "Failed to allocate RSS indirection QP\n");
-		goto rss_err;
+		goto mlx4_err;
 	}
 
 	rss_map->indir_qp->event = mlx4_en_sqp_event;
@@ -1241,6 +1241,7 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
 		       MLX4_QP_STATE_RST, NULL, 0, 0, rss_map->indir_qp);
 	mlx4_qp_remove(mdev->dev, rss_map->indir_qp);
 	mlx4_qp_free(mdev->dev, rss_map->indir_qp);
+mlx4_err:
 	kfree(rss_map->indir_qp);
 	rss_map->indir_qp = NULL;
 rss_err:
-- 
2.7.4


^ 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