Netdev List
 help / color / mirror / Atom feed
* Re: [patch net-next] nfp: flower: fix error path during representor creation
From: Jakub Kicinski @ 2018-05-17 17:56 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, simon.horman, dirk.vandermerwe, john.hurley,
	pieter.jansenvanvuuren, oss-drivers
In-Reply-To: <20180517100643.24044-1-jiri@resnulli.us>

On Thu, 17 May 2018 12:06:43 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Don't store repr pointer to reprs array until the representor is
> successfully created. This avoids message about "representor
> destruction" even when it was never created. Also it cleans-up the flow.
> Also, check return value after port alloc.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Thank you!

^ permalink raw reply

* KASAN: use-after-free Read in timer_is_static_object
From: syzbot @ 2018-05-17 17:57 UTC (permalink / raw)
  To: alexey.kodanev, davem, dccp, edumazet, gerrit, keescook,
	linux-kernel, netdev, soheil, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    e6506eb24187 Merge tag 'trace-v4.17-rc4-2' of git://git.ke..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=177fe477800000
kernel config:  https://syzkaller.appspot.com/x/.config?x=f3b4e30da84ec1ed
dashboard link: https://syzkaller.appspot.com/bug?extid=5d47e9ec91a6f15dbd6f
compiler:       gcc (GCC) 8.0.1 20180413 (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+5d47e9ec91a6f15dbd6f@syzkaller.appspotmail.com

RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000016
RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000017
R13: 0000000000000053 R14: 00000000006f4868 R15: 0000000000000001
==================================================================
BUG: KASAN: use-after-free in timer_is_static_object+0x80/0x90  
kernel/time/timer.c:607
Read of size 8 at addr ffff8801bebb5118 by task syz-executor2/25299

CPU: 1 PID: 25299 Comm: syz-executor2 Not tainted 4.17.0-rc5+ #54
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  <IRQ>
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
  print_address_description+0x6c/0x20b mm/kasan/report.c:256
  kasan_report_error mm/kasan/report.c:354 [inline]
  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
  __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
  timer_is_static_object+0x80/0x90 kernel/time/timer.c:607
  debug_object_activate+0x2d9/0x670 lib/debugobjects.c:508
  debug_timer_activate kernel/time/timer.c:709 [inline]
  debug_activate kernel/time/timer.c:764 [inline]
  __mod_timer kernel/time/timer.c:1041 [inline]
  mod_timer+0x4d3/0x13b0 kernel/time/timer.c:1102
  sk_reset_timer+0x22/0x60 net/core/sock.c:2742
  ccid2_hc_tx_rto_expire+0x587/0x680 net/dccp/ccids/ccid2.c:147
  call_timer_fn+0x230/0x940 kernel/time/timer.c:1326
  expire_timers kernel/time/timer.c:1363 [inline]
  __run_timers+0x79e/0xc50 kernel/time/timer.c:1666
  run_timer_softirq+0x4c/0x70 kernel/time/timer.c:1692
  __do_softirq+0x2e0/0xaf5 kernel/softirq.c:285
  invoke_softirq kernel/softirq.c:365 [inline]
  irq_exit+0x1d1/0x200 kernel/softirq.c:405
  exiting_irq arch/x86/include/asm/apic.h:525 [inline]
  smp_apic_timer_interrupt+0x17e/0x710 arch/x86/kernel/apic/apic.c:1052
  apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:863
  </IRQ>
RIP: 0010:cap_capable+0x3f/0x260 security/commoncap.c:82
RSP: 0018:ffff8801ac75f8c8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: dffffc0000000000 RBX: ffffffff88d5be00 RCX: 0000000000000000
RDX: 1ffff100386dc571 RSI: ffffffff830ee893 RDI: ffff8801c36e2b88
RBP: ffff8801ac75f910 R08: ffff8801ac126440 R09: ffff8801ac75fcb8
R10: ffff8801ac126c78 R11: ffff8801ac75fc78 R12: dffffc0000000000
R13: dffffc0000000000 R14: ffff8801c36e2b00 R15: 0000000000000021
  cap_vm_enough_memory+0x50/0x70 security/commoncap.c:1307
  security_vm_enough_memory_mm+0x71/0xc0 security/security.c:327
  mmap_region+0x37b/0x1870 mm/mmap.c:1714
  do_mmap+0xde2/0x1360 mm/mmap.c:1535
  do_mmap_pgoff include/linux/mm.h:2237 [inline]
  vm_mmap_pgoff+0x1fb/0x2a0 mm/util.c:357
  ksys_mmap_pgoff+0x26e/0x640 mm/mmap.c:1585
  __do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
  __se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
  __x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x455a5a
RSP: 002b:0000000000a3e778 EFLAGS: 00000246 ORIG_RAX: 0000000000000009
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000455a5a
RDX: 0000000000000003 RSI: 0000000000021000 RDI: 0000000000000000
RBP: ffffffffffffffff R08: ffffffffffffffff R09: 0000000000000000
R10: 0000000000020022 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000021000 R14: 0000000000020022 R15: 0000000000000000

Allocated by task 25374:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
  kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
  kmem_cache_alloc+0x12e/0x760 mm/slab.c:3554
  ccid_new+0x25b/0x3e0 net/dccp/ccid.c:151
  dccp_hdlr_ccid+0x27/0x150 net/dccp/feat.c:44
  __dccp_feat_activate+0x184/0x270 net/dccp/feat.c:344
  dccp_feat_activate_values+0x3a7/0x819 net/dccp/feat.c:1538
  dccp_create_openreq_child+0x472/0x610 net/dccp/minisocks.c:128
  dccp_v4_request_recv_sock+0x12c/0xca0 net/dccp/ipv4.c:408
  dccp_v6_request_recv_sock+0x125d/0x1f10 net/dccp/ipv6.c:415
  dccp_check_req+0x455/0x6a0 net/dccp/minisocks.c:197
  dccp_v4_rcv+0x7b8/0x1f3f net/dccp/ipv4.c:841
  ip_local_deliver_finish+0x2e3/0xd80 net/ipv4/ip_input.c:215
  NF_HOOK include/linux/netfilter.h:288 [inline]
  ip_local_deliver+0x1e1/0x720 net/ipv4/ip_input.c:256
  dst_input include/net/dst.h:450 [inline]
  ip_rcv_finish+0x81b/0x2200 net/ipv4/ip_input.c:396
  NF_HOOK include/linux/netfilter.h:288 [inline]
  ip_rcv+0xb70/0x143d net/ipv4/ip_input.c:492
  __netif_receive_skb_core+0x26f5/0x3630 net/core/dev.c:4592
  __netif_receive_skb+0x2c/0x1e0 net/core/dev.c:4657
  process_backlog+0x219/0x760 net/core/dev.c:5337
  napi_poll net/core/dev.c:5735 [inline]
  net_rx_action+0x7b7/0x1930 net/core/dev.c:5801
  __do_softirq+0x2e0/0xaf5 kernel/softirq.c:285

Freed by task 25374:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
  __cache_free mm/slab.c:3498 [inline]
  kmem_cache_free+0x86/0x2d0 mm/slab.c:3756
  ccid_hc_tx_delete+0xc3/0x100 net/dccp/ccid.c:190
  dccp_disconnect+0x130/0xc66 net/dccp/proto.c:286
  dccp_close+0x3bc/0xe60 net/dccp/proto.c:1045
  inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
  inet6_release+0x50/0x70 net/ipv6/af_inet6.c:460
  sock_release+0x96/0x1b0 net/socket.c:594
  sock_close+0x16/0x20 net/socket.c:1149
  __fput+0x34d/0x890 fs/file_table.c:209
  ____fput+0x15/0x20 fs/file_table.c:243
  task_work_run+0x1e4/0x290 kernel/task_work.c:113
  tracehook_notify_resume include/linux/tracehook.h:191 [inline]
  exit_to_usermode_loop+0x2bd/0x310 arch/x86/entry/common.c:166
  prepare_exit_to_usermode arch/x86/entry/common.c:196 [inline]
  syscall_return_slowpath arch/x86/entry/common.c:265 [inline]
  do_syscall_64+0x6ac/0x800 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff8801bebb4cc0
  which belongs to the cache ccid2_hc_tx_sock of size 1240
The buggy address is located 1112 bytes inside of
  1240-byte region [ffff8801bebb4cc0, ffff8801bebb5198)
The buggy address belongs to the page:
page:ffffea0006faed00 count:1 mapcount:0 mapping:ffff8801bebb41c0  
index:0xffff8801bebb5240 compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffff8801bebb41c0 ffff8801bebb5240 0000000100000003
raw: ffff8801cdba3138 ffffea0007634120 ffff8801cdbaab40 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8801bebb5000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8801bebb5080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801bebb5100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                             ^
  ffff8801bebb5180: fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc fc
  ffff8801bebb5200: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.

^ permalink raw reply

* Re: Request for -stable inclusion: time stamping fix for nfp
From: David Miller @ 2018-05-17 18:09 UTC (permalink / raw)
  To: g.nault; +Cc: jakub.kicinski, netdev
In-Reply-To: <20180517174147.GC1534@alphalink.fr>

From: Guillaume Nault <g.nault@alphalink.fr>
Date: Thu, 17 May 2018 19:41:47 +0200

> On Thu, Nov 16, 2017 at 10:13:28AM +0900, David Miller wrote:
>> From: Guillaume Nault <g.nault@alphalink.fr>
>> Date: Wed, 15 Nov 2017 17:20:46 +0100
>> 
>> > Can you please queue commit 46f1c52e66db
>> > ("nfp: TX time stamp packets before HW doorbell is rung") for -stable?
>> > We got hit but this bug in the late summer. We run this fix internally
>> > since a couple of months, but that'd be better to have it officially
>> > backported so everyone can benefit of it.
>> 
>> Queued up.
> 
> I guess this one got lost somewhere as it doesn't appear in linux-4.9.y
> (other trees aren't relevant).
> If that's unintentional, than can you please re-queue
> 46f1c52e66db ("nfp: TX time stamp packets before HW doorbell is rung")
> to -stable?

I only submit patches to -stable for the two most recent active branches
which right now consists of 4.16 and 4.14 as per www.kernel.org

^ permalink raw reply

* Re: [bpf-next PATCH 1/2] bpf: allow sk_msg programs to read sock fields
From: Martin KaFai Lau @ 2018-05-17 18:17 UTC (permalink / raw)
  To: John Fastabend; +Cc: ast, daniel, netdev
In-Reply-To: <20180517155404.21250.87046.stgit@john-Precision-Tower-5810>

On Thu, May 17, 2018 at 08:54:04AM -0700, John Fastabend wrote:
> Currently sk_msg programs only have access to the raw data. However,
> it is often useful when building policies to have the policies specific
> to the socket endpoint. This allows using the socket tuple as input
> into filters, etc.
> 
> This patch adds ctx access to the sock fields.
> 
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  include/linux/filter.h   |    1 
>  include/uapi/linux/bpf.h |    8 +++
>  kernel/bpf/sockmap.c     |    1 
>  net/core/filter.c        |  114 +++++++++++++++++++++++++++++++++++++++++++++-
It is indeed a lot of dup lines with sock_ops_convert_ctx_access()
as you mentioned in the cover.

Other than that, LGTM.

Acked-by: Martin KaFai Lau <kafafi@fb.com>

>  4 files changed, 121 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 9dbcb9d..d358d18 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -517,6 +517,7 @@ struct sk_msg_buff {
>  	bool sg_copy[MAX_SKB_FRAGS];
>  	__u32 flags;
>  	struct sock *sk_redir;
> +	struct sock *sk;
>  	struct sk_buff *skb;
>  	struct list_head list;
>  };
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index d94d333..97446bb 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2176,6 +2176,14 @@ enum sk_action {
>  struct sk_msg_md {
>  	void *data;
>  	void *data_end;
> +
> +	__u32 family;
> +	__u32 remote_ip4;	/* Stored in network byte order */
> +	__u32 local_ip4;	/* Stored in network byte order */
> +	__u32 remote_ip6[4];	/* Stored in network byte order */
> +	__u32 local_ip6[4];	/* Stored in network byte order */
> +	__u32 remote_port;	/* Stored in network byte order */
> +	__u32 local_port;	/* stored in host byte order */
This ordering inconsistency could be a trap to write bpf_prog
but I guess it is too late to change now considering
bpf_sock_ops is also using this convention.

Just curious, we cannot always assume inet_sk and then uses
its inet_sport?

^ permalink raw reply

* Re: Request for -stable inclusion: time stamping fix for nfp
From: Willy Tarreau @ 2018-05-17 18:32 UTC (permalink / raw)
  To: Greg KH; +Cc: David Miller, g.nault, jakub.kicinski, netdev
In-Reply-To: <20180517.140903.1581758299927078395.davem@davemloft.net>

Adding Greg here.

Greg, apparently a backport of 46f1c52e66db is needed in 4.9 according
to the thread below. It was merged in 4.13 so 4.14 already has it.

Willy

On Thu, May 17, 2018 at 02:09:03PM -0400, David Miller wrote:
> From: Guillaume Nault <g.nault@alphalink.fr>
> Date: Thu, 17 May 2018 19:41:47 +0200
> 
> > On Thu, Nov 16, 2017 at 10:13:28AM +0900, David Miller wrote:
> >> From: Guillaume Nault <g.nault@alphalink.fr>
> >> Date: Wed, 15 Nov 2017 17:20:46 +0100
> >> 
> >> > Can you please queue commit 46f1c52e66db
> >> > ("nfp: TX time stamp packets before HW doorbell is rung") for -stable?
> >> > We got hit but this bug in the late summer. We run this fix internally
> >> > since a couple of months, but that'd be better to have it officially
> >> > backported so everyone can benefit of it.
> >> 
> >> Queued up.
> > 
> > I guess this one got lost somewhere as it doesn't appear in linux-4.9.y
> > (other trees aren't relevant).
> > If that's unintentional, than can you please re-queue
> > 46f1c52e66db ("nfp: TX time stamp packets before HW doorbell is rung")
> > to -stable?
> 
> I only submit patches to -stable for the two most recent active branches
> which right now consists of 4.16 and 4.14 as per www.kernel.org

^ permalink raw reply

* Re: [PATCH 1/2] bpf: sockmap, fix uninitialized variable
From: Gustavo A. R. Silva @ 2018-05-17 18:12 UTC (permalink / raw)
  To: John Fastabend, Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, linux-kernel
In-Reply-To: <dd511eb3-abbf-1822-8bcc-7bcc6ca68b6c@gmail.com>

Hi John,

On 05/17/2018 12:27 PM, John Fastabend wrote:
> On 05/17/2018 07:08 AM, Gustavo A. R. Silva wrote:
>> There is a potential execution path in which variable err is
>> returned without being properly initialized previously.
>>
>> Fix this by initializing variable err to 0.
>>
>> Addresses-Coverity-ID: 1468964 ("Uninitialized scalar variable")
>> Fixes: e5cd3abcb31a ("bpf: sockmap, refactor sockmap routines to work
>> with hashmap")
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>>   kernel/bpf/sockmap.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
>> index c6de139..41b41fc 100644
>> --- a/kernel/bpf/sockmap.c
>> +++ b/kernel/bpf/sockmap.c
>> @@ -1713,7 +1713,7 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
>>   	struct smap_psock_map_entry *e = NULL;
>>   	struct smap_psock *psock;
>>   	bool new = false;
>> -	int err;
>> +	int err = 0;
>>   
>>   	/* 1. If sock map has BPF programs those will be inherited by the
>>   	 * sock being added. If the sock is already attached to BPF programs
>>
> 
> Thanks for catching this and the quick fix. The path to hit this case
> is to add a sock to a map (without a BPF program) where the sock already
> has been added to another map. I don't have any tests for the case with
> socks in multiple maps so I'll add some to the selftests so I remember
> this case.
> 

Glad to help. :)

> The alternative fix would be to always 'return 0' at the end of the
> function, but I think its probably better to init err here like above.
> 

Yeah. I think initializing err is better in this case.

> Acked-by: John Fastabend <john.fastabend@gmail.com>
> 

Thank you

^ permalink raw reply

* Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)
From: Jeff Johnson @ 2018-05-17 18:35 UTC (permalink / raw)
  To: Ramon Fried
  Cc: kvalo, linux-kernel, wcn36xx, linux-wireless, netdev, Eyal Ilsar,
	linux-wireless-owner
In-Reply-To: <20180517113250.16517-1-ramon.fried@linaro.org>

On 2018-05-17 04:32, Ramon Fried wrote:
> From: Eyal Ilsar <eilsar@codeaurora.org>
...
> +static int wcn36xx_smd_process_ptt_msg_rsp(void *buf, size_t len,
> +					   void **p_ptt_rsp_msg)
> +{
> +	struct wcn36xx_hal_process_ptt_msg_rsp_msg *rsp;
> +	int ret = 0;

why initialize 'ret' when you immediately overwrite?

> +	ret = wcn36xx_smd_rsp_status_check(buf, len);
...
> +	if (rsp->header.len > 0) {
> +		*p_ptt_rsp_msg = kmalloc(rsp->header.len, GFP_ATOMIC);

NULL check required?

> +		memcpy(*p_ptt_rsp_msg, rsp->ptt_msg, rsp->header.len);
> +	}
> +	return ret;
> +}
> +
> +int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
> +				struct ieee80211_vif *vif, void *ptt_msg, size_t len,
> +		void **ptt_rsp_msg)
> +{
> +	struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
> +	int ret = 0;

why initialize 'ret' when it is always overwritten before use?

> +	ret = wcn36xx_smd_send_and_wait(wcn, p_msg_body->header.len);

^ permalink raw reply

* Re: [PATCH] wcn36xx: Add support for Factory Test Mode (FTM)
From: Jeff Johnson @ 2018-05-17 18:37 UTC (permalink / raw)
  To: Ramon Fried
  Cc: kvalo, linux-kernel, wcn36xx, linux-wireless, netdev, Eyal Ilsar,
	linux-wireless-owner
In-Reply-To: <20180517113250.16517-1-ramon.fried@linaro.org>

On 2018-05-17 04:32, Ramon Fried wrote:
> From: Eyal Ilsar <eilsar@codeaurora.org>
...
> +int wcn36xx_smd_process_ptt_msg(struct wcn36xx *wcn,
> +				struct ieee80211_vif *vif, void *ptt_msg, size_t len,
> +		void **ptt_rsp_msg)
> +{
> +	struct wcn36xx_hal_process_ptt_msg_req_msg *p_msg_body;
> +	int ret = 0;
> +
> +	mutex_lock(&wcn->hal_mutex);
> +	p_msg_body = kmalloc(
> +		sizeof(struct wcn36xx_hal_process_ptt_msg_req_msg) + len,
> +		GFP_ATOMIC);

NULL check required?

> +	INIT_HAL_PTT_MSG(p_msg_body, len);
> +

^ permalink raw reply

* Re: [net-next PATCH v2 1/4] net: Refactor XPS for CPUs and Rx queues
From: David Miller @ 2018-05-17 18:38 UTC (permalink / raw)
  To: amritha.nambiar
  Cc: netdev, alexander.h.duyck, sridhar.samudrala, edumazet, hannes,
	tom
In-Reply-To: <152643400370.4991.2044471541271189575.stgit@anamdev.jf.intel.com>

From: Amritha Nambiar <amritha.nambiar@intel.com>
Date: Tue, 15 May 2018 18:26:43 -0700

> @@ -2125,7 +2125,7 @@ static bool remove_xps_queue_cpu(struct net_device *dev,
>  		int i, j;
>  
>  		for (i = count, j = offset; i--; j++) {
> -			if (!remove_xps_queue(dev_maps, cpu, j))
> +			if (!remove_xps_queue(dev_maps, tci, j))
>  				break;
>  		}
>  

This looks like a bug fix, completely unrelated to the feature being added
by this patch set.

Please submit this targetting the 'net' tree, then when that fix propagates
into 'net-next' you can rebase this series on top of that.

Thank you.

^ permalink raw reply

* Re: [PATCH V2] mlx4_core: allocate ICM memory in page size chunks
From: kbuild test robot @ 2018-05-17 18:39 UTC (permalink / raw)
  To: Qing Huang
  Cc: kbuild-all, tariqt, davem, haakon.bugge, yanjun.zhu, netdev,
	linux-rdma, linux-kernel, Qing Huang
In-Reply-To: <20180511192318.22342-1-qing.huang@oracle.com>

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

Hi Qing,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Qing-Huang/mlx4_core-allocate-ICM-memory-in-page-size-chunks/20180512-090438
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All error/warnings (new ones prefixed by >>):

   drivers/net//ethernet/mellanox/mlx4/icm.c: In function 'mlx4_init_icm_table':
>> drivers/net//ethernet/mellanox/mlx4/icm.c:403:20: error: implicit declaration of function 'vzalloc'; did you mean 'kzalloc'? [-Werror=implicit-function-declaration]
     table->icm      = vzalloc(num_icm * sizeof(*table->icm));
                       ^~~~~~~
                       kzalloc
>> drivers/net//ethernet/mellanox/mlx4/icm.c:403:18: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     table->icm      = vzalloc(num_icm * sizeof(*table->icm));
                     ^
>> drivers/net//ethernet/mellanox/mlx4/icm.c:449:2: error: implicit declaration of function 'vfree'; did you mean 'kfree'? [-Werror=implicit-function-declaration]
     vfree(table->icm);
     ^~~~~
     kfree
   cc1: some warnings being treated as errors

vim +403 drivers/net//ethernet/mellanox/mlx4/icm.c

   389	
   390	int mlx4_init_icm_table(struct mlx4_dev *dev, struct mlx4_icm_table *table,
   391				u64 virt, int obj_size,	u32 nobj, int reserved,
   392				int use_lowmem, int use_coherent)
   393	{
   394		int obj_per_chunk;
   395		int num_icm;
   396		unsigned chunk_size;
   397		int i;
   398		u64 size;
   399	
   400		obj_per_chunk = MLX4_TABLE_CHUNK_SIZE / obj_size;
   401		num_icm = (nobj + obj_per_chunk - 1) / obj_per_chunk;
   402	
 > 403		table->icm      = vzalloc(num_icm * sizeof(*table->icm));
   404		if (!table->icm)
   405			return -ENOMEM;
   406		table->virt     = virt;
   407		table->num_icm  = num_icm;
   408		table->num_obj  = nobj;
   409		table->obj_size = obj_size;
   410		table->lowmem   = use_lowmem;
   411		table->coherent = use_coherent;
   412		mutex_init(&table->mutex);
   413	
   414		size = (u64) nobj * obj_size;
   415		for (i = 0; i * MLX4_TABLE_CHUNK_SIZE < reserved * obj_size; ++i) {
   416			chunk_size = MLX4_TABLE_CHUNK_SIZE;
   417			if ((i + 1) * MLX4_TABLE_CHUNK_SIZE > size)
   418				chunk_size = PAGE_ALIGN(size -
   419						i * MLX4_TABLE_CHUNK_SIZE);
   420	
   421			table->icm[i] = mlx4_alloc_icm(dev, chunk_size >> PAGE_SHIFT,
   422						       (use_lowmem ? GFP_KERNEL : GFP_HIGHUSER) |
   423						       __GFP_NOWARN, use_coherent);
   424			if (!table->icm[i])
   425				goto err;
   426			if (mlx4_MAP_ICM(dev, table->icm[i], virt + i * MLX4_TABLE_CHUNK_SIZE)) {
   427				mlx4_free_icm(dev, table->icm[i], use_coherent);
   428				table->icm[i] = NULL;
   429				goto err;
   430			}
   431	
   432			/*
   433			 * Add a reference to this ICM chunk so that it never
   434			 * gets freed (since it contains reserved firmware objects).
   435			 */
   436			++table->icm[i]->refcount;
   437		}
   438	
   439		return 0;
   440	
   441	err:
   442		for (i = 0; i < num_icm; ++i)
   443			if (table->icm[i]) {
   444				mlx4_UNMAP_ICM(dev, virt + i * MLX4_TABLE_CHUNK_SIZE,
   445					       MLX4_TABLE_CHUNK_SIZE / MLX4_ICM_PAGE_SIZE);
   446				mlx4_free_icm(dev, table->icm[i], use_coherent);
   447			}
   448	
 > 449		vfree(table->icm);
   450	
   451		return -ENOMEM;
   452	}
   453	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53307 bytes --]

^ permalink raw reply

* Re: [PATCH v2 net-next 00/12] net: stmmac: Clean-up and tune-up
From: David Miller @ 2018-05-17 18:41 UTC (permalink / raw)
  To: Jose.Abreu
  Cc: f.fainelli, netdev, Joao.Pinto, Vitor.Soares, peppe.cavallaro,
	alexandre.torgue
In-Reply-To: <a36d9afa-6939-2695-5f31-bc8777b5f27c@synopsys.com>

From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Thu, 17 May 2018 14:24:42 +0100

> Given that the difference between better/worst is < 1%, I think
> we can conclude patches 3-13 don't affect the overall
> performance. I didn't profile the cache hits/miss though ...

Ok, thanks for making an effort to look into this more thoroughly.

I'll apply this series to net-next, thank you.

^ permalink raw reply

* Re: [PATCH v2 net-next 00/12] net: stmmac: Clean-up and tune-up
From: David Miller @ 2018-05-17 18:47 UTC (permalink / raw)
  To: Jose.Abreu
  Cc: f.fainelli, netdev, Joao.Pinto, Vitor.Soares, peppe.cavallaro,
	alexandre.torgue
In-Reply-To: <20180517.144117.1704300189822624802.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Thu, 17 May 2018 14:41:17 -0400 (EDT)

> From: Jose Abreu <Jose.Abreu@synopsys.com>
> Date: Thu, 17 May 2018 14:24:42 +0100
> 
>> Given that the difference between better/worst is < 1%, I think
>> we can conclude patches 3-13 don't affect the overall
>> performance. I didn't profile the cache hits/miss though ...
> 
> Ok, thanks for making an effort to look into this more thoroughly.
> 
> I'll apply this series to net-next, thank you.

Sorry, I had to revert.

It is one thing to say that you lack the hardware to physically test
the changes on all chip types.

It is yet another to not even test the build properly:

drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c:494:10: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
  .init = sun8i_dwmac_dma_init,
          ^~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c:494:10: note: (near initialization for ‘sun8i_dwmac_dma_ops.init’)
cc1: some warnings being treated as errors

^ permalink raw reply

* Re: [net-next PATCH v2 1/4] net: Refactor XPS for CPUs and Rx queues
From: Nambiar, Amritha @ 2018-05-17 18:47 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, alexander.h.duyck, sridhar.samudrala, edumazet, hannes,
	tom
In-Reply-To: <20180517.143853.84077547534626009.davem@davemloft.net>

On 5/17/2018 11:38 AM, David Miller wrote:
> From: Amritha Nambiar <amritha.nambiar@intel.com>
> Date: Tue, 15 May 2018 18:26:43 -0700
> 
>> @@ -2125,7 +2125,7 @@ static bool remove_xps_queue_cpu(struct net_device *dev,
>>  		int i, j;
>>  
>>  		for (i = count, j = offset; i--; j++) {
>> -			if (!remove_xps_queue(dev_maps, cpu, j))
>> +			if (!remove_xps_queue(dev_maps, tci, j))
>>  				break;
>>  		}
>>  
> 
> This looks like a bug fix, completely unrelated to the feature being added
> by this patch set.
> 
> Please submit this targetting the 'net' tree, then when that fix propagates
> into 'net-next' you can rebase this series on top of that.
> 
> Thank you.
> 

Sure, will do that.
Thanks.

^ permalink raw reply

* Re: [PATCH net] tls: don't use stack memory in a scatterlist
From: David Miller @ 2018-05-17 18:50 UTC (permalink / raw)
  To: mmullins; +Cc: ilyal, aviadye, davejwatson, netdev, linux-kernel
In-Reply-To: <20180516174841.2119-1-mmullins@fb.com>

From: Matt Mullins <mmullins@fb.com>
Date: Wed, 16 May 2018 10:48:40 -0700

> scatterlist code expects virt_to_page() to work, which fails with
> CONFIG_VMAP_STACK=y.
> 
> Fixes: c46234ebb4d1e ("tls: RX path for ktls")
> Signed-off-by: Matt Mullins <mmullins@fb.com>

Applied and queued up for -stable, thanks.

I'm surprised this problem wasn't discovered sooner.  How exactly did you
discover it?  Did you actually see it trigger or is this purely from code
inspection?

^ permalink raw reply

* Re: [PATCH bpf 5/6] tools: bpftool: resolve calls without using imm field
From: Jakub Kicinski @ 2018-05-17 18:51 UTC (permalink / raw)
  To: Sandipan Das; +Cc: ast, daniel, netdev, linuxppc-dev, naveen.n.rao
In-Reply-To: <20180517063548.6373-6-sandipan@linux.vnet.ibm.com>

On Thu, 17 May 2018 12:05:47 +0530, Sandipan Das wrote:
> Currently, we resolve the callee's address for a JITed function
> call by using the imm field of the call instruction as an offset
> from __bpf_call_base. If bpf_jit_kallsyms is enabled, we further
> use this address to get the callee's kernel symbol's name.
> 
> For some architectures, such as powerpc64, the imm field is not
> large enough to hold this offset. So, instead of assigning this
> offset to the imm field, the verifier now assigns the subprog
> id. Also, a list of kernel symbol addresses for all the JITed
> functions is provided in the program info. We now use the imm
> field as an index for this list to lookup a callee's symbol's
> address and resolve its name.
> 
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>

A few nit-picks below, thank you for the patch!

>  tools/bpf/bpftool/prog.c          | 31 +++++++++++++++++++++++++++++++
>  tools/bpf/bpftool/xlated_dumper.c | 24 +++++++++++++++++-------
>  tools/bpf/bpftool/xlated_dumper.h |  2 ++
>  3 files changed, 50 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 9bdfdf2d3fbe..ac2f62a97e84 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -430,6 +430,10 @@ static int do_dump(int argc, char **argv)
>  	unsigned char *buf;
>  	__u32 *member_len;
>  	__u64 *member_ptr;
> +	unsigned int nr_addrs;
> +	unsigned long *addrs = NULL;
> +	__u32 *ksyms_len;
> +	__u64 *ksyms_ptr;

nit: please try to keep the variables ordered longest to shortest like
we do in networking code (please do it in all functions).

>  	ssize_t n;
>  	int err;
>  	int fd;
> @@ -437,6 +441,8 @@ static int do_dump(int argc, char **argv)
>  	if (is_prefix(*argv, "jited")) {
>  		member_len = &info.jited_prog_len;
>  		member_ptr = &info.jited_prog_insns;
> +		ksyms_len = &info.nr_jited_ksyms;
> +		ksyms_ptr = &info.jited_ksyms;
>  	} else if (is_prefix(*argv, "xlated")) {
>  		member_len = &info.xlated_prog_len;
>  		member_ptr = &info.xlated_prog_insns;
> @@ -496,10 +502,23 @@ static int do_dump(int argc, char **argv)
>  		return -1;
>  	}
>  
> +	nr_addrs = *ksyms_len;

Here and ...

> +	if (nr_addrs) {
> +		addrs = malloc(nr_addrs * sizeof(__u64));
> +		if (!addrs) {
> +			p_err("mem alloc failed");
> +			free(buf);
> +			close(fd);
> +			return -1;

You can just jump to err_free here.

> +		}
> +	}
> +
>  	memset(&info, 0, sizeof(info));
>  
>  	*member_ptr = ptr_to_u64(buf);
>  	*member_len = buf_size;
> +	*ksyms_ptr = ptr_to_u64(addrs);
> +	*ksyms_len = nr_addrs;

... here - this function is getting long, so maybe I'm not seeing
something, but are ksyms_ptr and ksyms_len guaranteed to be initialized?

>  	err = bpf_obj_get_info_by_fd(fd, &info, &len);
>  	close(fd);
> @@ -513,6 +532,11 @@ static int do_dump(int argc, char **argv)
>  		goto err_free;
>  	}
>  
> +	if (*ksyms_len > nr_addrs) {
> +		p_err("too many addresses returned");
> +		goto err_free;
> +	}
> +
>  	if ((member_len == &info.jited_prog_len &&
>  	     info.jited_prog_insns == 0) ||
>  	    (member_len == &info.xlated_prog_len &&
> @@ -558,6 +582,9 @@ static int do_dump(int argc, char **argv)
>  			dump_xlated_cfg(buf, *member_len);
>  	} else {
>  		kernel_syms_load(&dd);
> +		dd.jited_ksyms = ksyms_ptr;
> +		dd.nr_jited_ksyms = *ksyms_len;
> +
>  		if (json_output)
>  			dump_xlated_json(&dd, buf, *member_len, opcodes);
>  		else
> @@ -566,10 +593,14 @@ static int do_dump(int argc, char **argv)
>  	}
>  
>  	free(buf);
> +	if (addrs)
> +		free(addrs);

Free can deal with NULL pointers, no need for an if.

>  	return 0;
>  
>  err_free:
>  	free(buf);
> +	if (addrs)
> +		free(addrs);
>  	return -1;
>  }
>  
> diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
> index 7a3173b76c16..dc8e4eca0387 100644
> --- a/tools/bpf/bpftool/xlated_dumper.c
> +++ b/tools/bpf/bpftool/xlated_dumper.c
> @@ -178,8 +178,12 @@ static const char *print_call_pcrel(struct dump_data *dd,
>  		snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
>  			 "%+d#%s", insn->off, sym->name);
>  	else

else if (address)

saves us the indentation.

> -		snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
> -			 "%+d#0x%lx", insn->off, address);
> +		if (address)
> +			snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
> +				 "%+d#0x%lx", insn->off, address);
> +		else
> +			snprintf(dd->scratch_buff, sizeof(dd->scratch_buff),
> +				 "%+d", insn->off);
>  	return dd->scratch_buff;
>  }
>  
> @@ -200,14 +204,20 @@ static const char *print_call(void *private_data,
>  			      const struct bpf_insn *insn)
>  {
>  	struct dump_data *dd = private_data;
> -	unsigned long address = dd->address_call_base + insn->imm;
> -	struct kernel_sym *sym;
> +	unsigned long address = 0;
> +	struct kernel_sym *sym = NULL;
>  

Hm.  Quite a bit of churn.  Why not just add these three lines here:

if (insn->src_reg == BPF_PSEUDO_CALL && 
    insn->imm < dd->nr_jited_ksyms)
	address = dd->jited_ksyms[insn->imm];

> -	sym = kernel_syms_search(dd, address);
> -	if (insn->src_reg == BPF_PSEUDO_CALL)
> +	if (insn->src_reg == BPF_PSEUDO_CALL) {
> +		if (dd->nr_jited_ksyms) {
> +			address = dd->jited_ksyms[insn->imm];

Perhaps it's paranoid, but it'd please do to bound check insn->imm
against dd->nr_jited_ksyms.

> +			sym = kernel_syms_search(dd, address);
> +		}
>  		return print_call_pcrel(dd, sym, address, insn);
> -	else
> +	} else {
> +		address = dd->address_call_base + insn->imm;
> +		sym = kernel_syms_search(dd, address);
>  		return print_call_helper(dd, sym, address);
> +	}
>  }
>  
>  static const char *print_imm(void *private_data,

^ permalink raw reply

* Re: [PATCH 0/3] Add R8A77980 GEther support
From: David Miller @ 2018-05-17 18:53 UTC (permalink / raw)
  To: sergei.shtylyov
  Cc: netdev, devicetree, robh+dt, mark.rutland, linux-renesas-soc
In-Reply-To: <087c91a3-a451-6de7-5e0f-a835f8cc98f1@cogentembedded.com>

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Wed, 16 May 2018 22:52:40 +0300

> Here's a set of 3 patches against DaveM's 'net-next.git' repo. They (gradually)
> add R8A77980 GEther support to the 'sh_eth' driver, starting with couple new
> register bits/values introduced with this chip, and ending with adding a new
> 'struct sh_eth_cpu_data' instance connected to the new DT "compatible" prop
> value...
> 
> [1/1] sh_eth: add RGMII support
> [2/3] sh_eth: add EDMR.NBST support
> [3/3] sh_eth: add R8A77980 support

Waiting for a respin of this, correcting the RGMII check in patch #1.

^ permalink raw reply

* Re: [PATCH v2 net] net/ipv4: Initialize proto and ports in flow struct
From: David Miller @ 2018-05-17 18:56 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, roopa
In-Reply-To: <20180516203640.12568-1-dsahern@gmail.com>

From: David Ahern <dsahern@gmail.com>
Date: Wed, 16 May 2018 13:36:40 -0700

> Updating the FIB tracepoint for the recent change to allow rules using
> the protocol and ports exposed a few places where the entries in the flow
> struct are not initialized.
> 
> For __fib_validate_source add the call to fib4_rules_early_flow_dissect
> since it is invoked for the input path. For netfilter, add the memset on
> the flow struct to avoid future problems like this. In ip_route_input_slow
> need to set the fields if the skb dissection does not happen.
> 
> Fixes: bfff4862653b ("net: fib_rules: support for match on ip_proto, sport and dport")
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> Have not seen any problems with the IPv6 version
> 
> v2
> - do not remove tracepoint in __fib_validate_source (sent the net-next
>   version of this patch)
> - add set of ports and proto to ip_route_input_slow if skb dissect
>   is not done

Applied, thanks David.

^ permalink raw reply

* Re: [bpf-next PATCH 2/2] bpf: add sk_msg prog sk access tests to test_verifier
From: Martin KaFai Lau @ 2018-05-17 18:57 UTC (permalink / raw)
  To: John Fastabend; +Cc: ast, daniel, netdev
In-Reply-To: <20180517155409.21250.77305.stgit@john-Precision-Tower-5810>

On Thu, May 17, 2018 at 08:54:10AM -0700, John Fastabend wrote:
> Add tests for BPF_PROG_TYPE_SK_MSG to test_verifier for read access
> to new sk fields.
> 
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
>  tools/include/uapi/linux/bpf.h              |    8 ++
>  tools/testing/selftests/bpf/test_verifier.c |  115 +++++++++++++++++++++++++++
>  2 files changed, 123 insertions(+)
> 
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index d94d333..97446bb 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -2176,6 +2176,14 @@ enum sk_action {
>  struct sk_msg_md {
>  	void *data;
>  	void *data_end;
> +
> +	__u32 family;
> +	__u32 remote_ip4;	/* Stored in network byte order */
> +	__u32 local_ip4;	/* Stored in network byte order */
> +	__u32 remote_ip6[4];	/* Stored in network byte order */
> +	__u32 local_ip6[4];	/* Stored in network byte order */
> +	__u32 remote_port;	/* Stored in network byte order */
> +	__u32 local_port;	/* stored in host byte order */
>  };
>  
>  #define BPF_TAG_SIZE	8
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index a877af0..1ac7630 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -1686,6 +1686,121 @@ static void bpf_fill_rand_ld_dw(struct bpf_test *self)
>  		.prog_type = BPF_PROG_TYPE_SK_SKB,
>  	},
>  	{
> +		"valid access family in SK_MSG",
> +		.insns = {
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, family)),
> +			BPF_EXIT_INSN(),
> +		},
> +		.result = ACCEPT,
> +		.prog_type = BPF_PROG_TYPE_SK_MSG,
> +	},
> +	{
> +		"valid access remote_ip4 in SK_MSG",
> +		.insns = {
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, remote_ip4)),
> +			BPF_EXIT_INSN(),
> +		},
> +		.result = ACCEPT,
> +		.prog_type = BPF_PROG_TYPE_SK_MSG,
> +	},
> +	{
> +		"valid access local_ip4 in SK_MSG",
> +		.insns = {
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, local_ip4)),
> +			BPF_EXIT_INSN(),
> +		},
> +		.result = ACCEPT,
> +		.prog_type = BPF_PROG_TYPE_SK_MSG,
> +	},
> +	{
> +		"valid access remote_port in SK_MSG",
> +		.insns = {
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, remote_port)),
> +			BPF_EXIT_INSN(),
> +		},
> +		.result = ACCEPT,
> +		.prog_type = BPF_PROG_TYPE_SK_MSG,
> +	},
> +	{
> +		"valid access local_port in SK_MSG",
> +		.insns = {
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, local_port)),
> +			BPF_EXIT_INSN(),
> +		},
> +		.result = ACCEPT,
> +		.prog_type = BPF_PROG_TYPE_SK_MSG,
> +	},
> +	{
> +		"valid access remote_ip6 in SK_MSG",
> +		.insns = {
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, remote_ip6[0])),
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, remote_ip6[1])),
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, remote_ip6[2])),
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, remote_ip6[3])),
> +			BPF_EXIT_INSN(),
> +		},
> +		.result = ACCEPT,
> +		.prog_type = BPF_PROG_TYPE_SK_SKB,
> +	},
> +	{
> +		"valid access local_ip6 in SK_MSG",
> +		.insns = {
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, local_ip6[0])),
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, local_ip6[1])),
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, local_ip6[2])),
> +			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, local_ip6[3])),
> +			BPF_EXIT_INSN(),
> +		},
> +		.result = ACCEPT,
> +		.prog_type = BPF_PROG_TYPE_SK_SKB,
> +	},
> +	{
> +		"invalid 64B read of family in SK_MSG",
> +		.insns = {
> +			BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, family)),
> +			BPF_EXIT_INSN(),
> +		},
> +		.errstr = "invalid bpf_context access",
> +		.result = REJECT,
> +		.prog_type = BPF_PROG_TYPE_SK_MSG,
> +	},
> +	{
> +		"invalid read past end of SK_MSG",
> +		.insns = {
> +			BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, local_port) + 4),
> +			BPF_EXIT_INSN(),
> +		},
> +		.errstr = "",
no errstr in this case?

> +		.result = REJECT,
> +		.prog_type = BPF_PROG_TYPE_SK_MSG,
> +	},
> +	{
> +		"invalid read offset in SK_MSG",
> +		.insns = {
> +			BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
> +				    offsetof(struct sk_msg_md, family) + 1),
> +			BPF_EXIT_INSN(),
> +		},
> +		.errstr = "",
same here.

> +		.result = REJECT,
> +		.prog_type = BPF_PROG_TYPE_SK_MSG,
> +	},
> +	{
>  		"direct packet read for SK_MSG",
>  		.insns = {
>  			BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1,
> 
Other than the above,

Acked-by: Martin KaFai Lau <kafai@fb.com>

^ permalink raw reply

* Re: [PATCH 0/3] ibmvnic: Fix bugs and memory leaks
From: David Miller @ 2018-05-17 18:57 UTC (permalink / raw)
  To: tlfalcon; +Cc: netdev, jallen, nfont
In-Reply-To: <1526503745-14421-1-git-send-email-tlfalcon@linux.vnet.ibm.com>

From: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
Date: Wed, 16 May 2018 15:49:02 -0500

> This is a small patch series fixing up some bugs and memory leaks
> in the ibmvnic driver. The first fix frees up previously allocated
> memory that should be freed in case of an error. The second fixes
> a reset case that was failing due to TX/RX queue IRQ's being
> erroneously disabled without being enabled again. The final patch
> fixes incorrect reallocated of statistics buffers during a device
> reset, resulting in loss of statistics information and a memory leak.

Series applied to 'net', thanks Thomas.

^ permalink raw reply

* [PATCH net-next 1/1] qede: Add build_skb() support.
From: Manish Chopra @ 2018-05-17 19:05 UTC (permalink / raw)
  To: davem; +Cc: netdev, ariel.elior, michal.kalderon

This patch makes use of build_skb() throughout in driver's receieve
data path [HW gro flow and non HW gro flow]. With this, driver can
build skb directly from the page segments which are already mapped
to the hardware instead of allocating new SKB via netdev_alloc_skb()
and memcpy the data which is quite costly.

This really improves performance (keeping same or slight gain in rx
throughput) in terms of CPU utilization which is significantly reduced
[almost half] in non HW gro flow where for every incoming MTU sized
packet driver had to allocate skb, memcpy headers etc. Additionally
in that flow, it also gets rid of bunch of additional overheads
[eth_get_headlen() etc.] to split headers and data in the skb.

Tested with:
system: 2 sockets, 4 cores per socket, hyperthreading, 2x4x2=16 cores
iperf [server]: iperf -s
iperf [client]: iperf -c <server_ip> -t 500 -i 10 -P 32

HW GRO off – w/o build_skb(), throughput: 36.8 Gbits/sec

Average:     CPU    %usr   %nice    %sys %iowait    %irq   %soft  %steal  %guest   %idle
Average:     all    0.59    0.00   32.93    0.00    0.00   43.07    0.00    0.00   23.42

HW GRO off - with build_skb(), throughput: 36.9 Gbits/sec

Average:     CPU    %usr   %nice    %sys %iowait    %irq   %soft  %steal  %guest   %idle
Average:     all    0.70    0.00   31.70    0.00    0.00   25.68    0.00    0.00   41.92

HW GRO on - w/o build_skb(), throughput: 36.9 Gbits/sec

Average:     CPU    %usr   %nice    %sys %iowait    %irq   %soft  %steal  %guest   %idle
Average:     all    0.86    0.00   24.14    0.00    0.00    6.59    0.00    0.00   68.41

HW GRO on - with build_skb(), throughput: 37.5 Gbits/sec

Average:     CPU    %usr   %nice    %sys %iowait    %irq   %soft  %steal  %guest   %idle
Average:     all    0.87    0.00   23.75    0.00    0.00    6.19    0.00    0.00   69.19

Signed-off-by: Ariel Elior <ariel.elior@cavium.com>
Signed-off-by: Manish Chopra <manish.chopra@cavium.com>
---
 drivers/net/ethernet/qlogic/qede/qede.h         |   5 +-
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c |   3 +-
 drivers/net/ethernet/qlogic/qede/qede_fp.c      | 227 +++++++++++++-----------
 drivers/net/ethernet/qlogic/qede/qede_main.c    |  76 ++------
 4 files changed, 137 insertions(+), 174 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index 9935978c..2d3f09e 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -290,15 +290,12 @@ struct qede_agg_info {
 	 * aggregation.
 	 */
 	struct sw_rx_data buffer;
-	dma_addr_t buffer_mapping;
-
 	struct sk_buff *skb;
 
 	/* We need some structs from the start cookie until termination */
 	u16 vlan_tag;
-	u16 start_cqe_bd_len;
-	u8 start_cqe_placement_offset;
 
+	bool tpa_start_fail;
 	u8 state;
 	u8 frag_id;
 
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index ecbf1de..8c6fdad 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -1508,7 +1508,8 @@ static int qede_selftest_receive_traffic(struct qede_dev *edev)
 		len =  le16_to_cpu(fp_cqe->len_on_first_bd);
 		data_ptr = (u8 *)(page_address(sw_rx_data->data) +
 				  fp_cqe->placement_offset +
-				  sw_rx_data->page_offset);
+				  sw_rx_data->page_offset +
+				  rxq->rx_headroom);
 		if (ether_addr_equal(data_ptr,  edev->ndev->dev_addr) &&
 		    ether_addr_equal(data_ptr + ETH_ALEN,
 				     edev->ndev->dev_addr)) {
diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c b/drivers/net/ethernet/qlogic/qede/qede_fp.c
index 1494130..6c70239 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
@@ -660,7 +660,8 @@ static int qede_fill_frag_skb(struct qede_dev *edev,
 
 	/* Add one frag and update the appropriate fields in the skb */
 	skb_fill_page_desc(skb, tpa_info->frag_id++,
-			   current_bd->data, current_bd->page_offset,
+			   current_bd->data,
+			   current_bd->page_offset + rxq->rx_headroom,
 			   len_on_bd);
 
 	if (unlikely(qede_realloc_rx_buffer(rxq, current_bd))) {
@@ -671,8 +672,7 @@ static int qede_fill_frag_skb(struct qede_dev *edev,
 		goto out;
 	}
 
-	qed_chain_consume(&rxq->rx_bd_ring);
-	rxq->sw_rx_cons++;
+	qede_rx_bd_ring_consume(rxq);
 
 	skb->data_len += len_on_bd;
 	skb->truesize += rxq->rx_buf_seg_size;
@@ -721,64 +721,129 @@ static u8 qede_check_tunn_csum(u16 flag)
 	return QEDE_CSUM_UNNECESSARY | tcsum;
 }
 
+static inline struct sk_buff *
+qede_build_skb(struct qede_rx_queue *rxq,
+	       struct sw_rx_data *bd, u16 len, u16 pad)
+{
+	struct sk_buff *skb;
+	void *buf;
+
+	buf = page_address(bd->data) + bd->page_offset;
+	skb = build_skb(buf, rxq->rx_buf_seg_size);
+
+	skb_reserve(skb, pad);
+	skb_put(skb, len);
+
+	return skb;
+}
+
+static struct sk_buff *
+qede_tpa_rx_build_skb(struct qede_dev *edev,
+		      struct qede_rx_queue *rxq,
+		      struct sw_rx_data *bd, u16 len, u16 pad,
+		      bool alloc_skb)
+{
+	struct sk_buff *skb;
+
+	skb = qede_build_skb(rxq, bd, len, pad);
+	bd->page_offset += rxq->rx_buf_seg_size;
+
+	if (bd->page_offset == PAGE_SIZE) {
+		if (unlikely(qede_alloc_rx_buffer(rxq, true))) {
+			DP_NOTICE(edev,
+				  "Failed to allocate RX buffer for tpa start\n");
+			bd->page_offset -= rxq->rx_buf_seg_size;
+			page_ref_inc(bd->data);
+			dev_kfree_skb_any(skb);
+			return NULL;
+		}
+	} else {
+		page_ref_inc(bd->data);
+		qede_reuse_page(rxq, bd);
+	}
+
+	/* We've consumed the first BD and prepared an SKB */
+	qede_rx_bd_ring_consume(rxq);
+
+	return skb;
+}
+
+static struct sk_buff *
+qede_rx_build_skb(struct qede_dev *edev,
+		  struct qede_rx_queue *rxq,
+		  struct sw_rx_data *bd, u16 len, u16 pad)
+{
+	struct sk_buff *skb = NULL;
+
+	/* For smaller frames still need to allocate skb, memcpy
+	 * data and benefit in reusing the page segment instead of
+	 * un-mapping it.
+	 */
+	if ((len + pad <= edev->rx_copybreak)) {
+		unsigned int offset = bd->page_offset + pad;
+
+		skb = netdev_alloc_skb(edev->ndev, QEDE_RX_HDR_SIZE);
+		if (unlikely(!skb))
+			return NULL;
+
+		skb_reserve(skb, pad);
+		memcpy(skb_put(skb, len),
+		       page_address(bd->data) + offset, len);
+		qede_reuse_page(rxq, bd);
+		goto out;
+	}
+
+	skb = qede_build_skb(rxq, bd, len, pad);
+
+	if (unlikely(qede_realloc_rx_buffer(rxq, bd))) {
+		/* Incr page ref count to reuse on allocation failure so
+		 * that it doesn't get freed while freeing SKB [as its
+		 * already mapped there].
+		 */
+		page_ref_inc(bd->data);
+		dev_kfree_skb_any(skb);
+		return NULL;
+	}
+out:
+	/* We've consumed the first BD and prepared an SKB */
+	qede_rx_bd_ring_consume(rxq);
+
+	return skb;
+}
+
 static void qede_tpa_start(struct qede_dev *edev,
 			   struct qede_rx_queue *rxq,
 			   struct eth_fast_path_rx_tpa_start_cqe *cqe)
 {
 	struct qede_agg_info *tpa_info = &rxq->tpa_info[cqe->tpa_agg_index];
-	struct eth_rx_bd *rx_bd_cons = qed_chain_consume(&rxq->rx_bd_ring);
-	struct eth_rx_bd *rx_bd_prod = qed_chain_produce(&rxq->rx_bd_ring);
-	struct sw_rx_data *replace_buf = &tpa_info->buffer;
-	dma_addr_t mapping = tpa_info->buffer_mapping;
 	struct sw_rx_data *sw_rx_data_cons;
-	struct sw_rx_data *sw_rx_data_prod;
+	u16 pad;
 
 	sw_rx_data_cons = &rxq->sw_rx_ring[rxq->sw_rx_cons & NUM_RX_BDS_MAX];
-	sw_rx_data_prod = &rxq->sw_rx_ring[rxq->sw_rx_prod & NUM_RX_BDS_MAX];
+	pad = cqe->placement_offset + rxq->rx_headroom;
 
-	/* Use pre-allocated replacement buffer - we can't release the agg.
-	 * start until its over and we don't want to risk allocation failing
-	 * here, so re-allocate when aggregation will be over.
-	 */
-	sw_rx_data_prod->mapping = replace_buf->mapping;
-
-	sw_rx_data_prod->data = replace_buf->data;
-	rx_bd_prod->addr.hi = cpu_to_le32(upper_32_bits(mapping));
-	rx_bd_prod->addr.lo = cpu_to_le32(lower_32_bits(mapping));
-	sw_rx_data_prod->page_offset = replace_buf->page_offset;
-
-	rxq->sw_rx_prod++;
+	tpa_info->skb = qede_tpa_rx_build_skb(edev, rxq, sw_rx_data_cons,
+					      le16_to_cpu(cqe->len_on_first_bd),
+					      pad, false);
+	tpa_info->buffer.page_offset = sw_rx_data_cons->page_offset;
+	tpa_info->buffer.mapping = sw_rx_data_cons->mapping;
 
-	/* move partial skb from cons to pool (don't unmap yet)
-	 * save mapping, incase we drop the packet later on.
-	 */
-	tpa_info->buffer = *sw_rx_data_cons;
-	mapping = HILO_U64(le32_to_cpu(rx_bd_cons->addr.hi),
-			   le32_to_cpu(rx_bd_cons->addr.lo));
-
-	tpa_info->buffer_mapping = mapping;
-	rxq->sw_rx_cons++;
-
-	/* set tpa state to start only if we are able to allocate skb
-	 * for this aggregation, otherwise mark as error and aggregation will
-	 * be dropped
-	 */
-	tpa_info->skb = netdev_alloc_skb(edev->ndev,
-					 le16_to_cpu(cqe->len_on_first_bd));
 	if (unlikely(!tpa_info->skb)) {
 		DP_NOTICE(edev, "Failed to allocate SKB for gro\n");
+
+		/* Consume from ring but do not produce since
+		 * this might be used by FW still, it will be re-used
+		 * at TPA end.
+		 */
+		tpa_info->tpa_start_fail = true;
+		qede_rx_bd_ring_consume(rxq);
 		tpa_info->state = QEDE_AGG_STATE_ERROR;
 		goto cons_buf;
 	}
 
-	/* Start filling in the aggregation info */
-	skb_put(tpa_info->skb, le16_to_cpu(cqe->len_on_first_bd));
 	tpa_info->frag_id = 0;
 	tpa_info->state = QEDE_AGG_STATE_START;
 
-	/* Store some information from first CQE */
-	tpa_info->start_cqe_placement_offset = cqe->placement_offset;
-	tpa_info->start_cqe_bd_len = le16_to_cpu(cqe->len_on_first_bd);
 	if ((le16_to_cpu(cqe->pars_flags.flags) >>
 	     PARSING_AND_ERR_FLAGS_TAG8021QEXIST_SHIFT) &
 	    PARSING_AND_ERR_FLAGS_TAG8021QEXIST_MASK)
@@ -899,6 +964,10 @@ static int qede_tpa_end(struct qede_dev *edev,
 	tpa_info = &rxq->tpa_info[cqe->tpa_agg_index];
 	skb = tpa_info->skb;
 
+	if (tpa_info->buffer.page_offset == PAGE_SIZE)
+		dma_unmap_page(rxq->dev, tpa_info->buffer.mapping,
+			       PAGE_SIZE, rxq->data_direction);
+
 	for (i = 0; cqe->len_list[i]; i++)
 		qede_fill_frag_skb(edev, rxq, cqe->tpa_agg_index,
 				   le16_to_cpu(cqe->len_list[i]));
@@ -919,11 +988,6 @@ static int qede_tpa_end(struct qede_dev *edev,
 		       "Strange - total packet len [cqe] is %4x but SKB has len %04x\n",
 		       le16_to_cpu(cqe->total_packet_len), skb->len);
 
-	memcpy(skb->data,
-	       page_address(tpa_info->buffer.data) +
-	       tpa_info->start_cqe_placement_offset +
-	       tpa_info->buffer.page_offset, tpa_info->start_cqe_bd_len);
-
 	/* Finalize the SKB */
 	skb->protocol = eth_type_trans(skb, edev->ndev);
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -940,6 +1004,12 @@ static int qede_tpa_end(struct qede_dev *edev,
 	return 1;
 err:
 	tpa_info->state = QEDE_AGG_STATE_NONE;
+
+	if (tpa_info->tpa_start_fail) {
+		qede_reuse_page(rxq, &tpa_info->buffer);
+		tpa_info->tpa_start_fail = false;
+	}
+
 	dev_kfree_skb_any(tpa_info->skb);
 	tpa_info->skb = NULL;
 	return 0;
@@ -1058,65 +1128,6 @@ static bool qede_rx_xdp(struct qede_dev *edev,
 	return false;
 }
 
-static struct sk_buff *qede_rx_allocate_skb(struct qede_dev *edev,
-					    struct qede_rx_queue *rxq,
-					    struct sw_rx_data *bd, u16 len,
-					    u16 pad)
-{
-	unsigned int offset = bd->page_offset + pad;
-	struct skb_frag_struct *frag;
-	struct page *page = bd->data;
-	unsigned int pull_len;
-	struct sk_buff *skb;
-	unsigned char *va;
-
-	/* Allocate a new SKB with a sufficient large header len */
-	skb = netdev_alloc_skb(edev->ndev, QEDE_RX_HDR_SIZE);
-	if (unlikely(!skb))
-		return NULL;
-
-	/* Copy data into SKB - if it's small, we can simply copy it and
-	 * re-use the already allcoated & mapped memory.
-	 */
-	if (len + pad <= edev->rx_copybreak) {
-		skb_put_data(skb, page_address(page) + offset, len);
-		qede_reuse_page(rxq, bd);
-		goto out;
-	}
-
-	frag = &skb_shinfo(skb)->frags[0];
-
-	skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
-			page, offset, len, rxq->rx_buf_seg_size);
-
-	va = skb_frag_address(frag);
-	pull_len = eth_get_headlen(va, QEDE_RX_HDR_SIZE);
-
-	/* Align the pull_len to optimize memcpy */
-	memcpy(skb->data, va, ALIGN(pull_len, sizeof(long)));
-
-	/* Correct the skb & frag sizes offset after the pull */
-	skb_frag_size_sub(frag, pull_len);
-	frag->page_offset += pull_len;
-	skb->data_len -= pull_len;
-	skb->tail += pull_len;
-
-	if (unlikely(qede_realloc_rx_buffer(rxq, bd))) {
-		/* Incr page ref count to reuse on allocation failure so
-		 * that it doesn't get freed while freeing SKB [as its
-		 * already mapped there].
-		 */
-		page_ref_inc(page);
-		dev_kfree_skb_any(skb);
-		return NULL;
-	}
-
-out:
-	/* We've consumed the first BD and prepared an SKB */
-	qede_rx_bd_ring_consume(rxq);
-	return skb;
-}
-
 static int qede_rx_build_jumbo(struct qede_dev *edev,
 			       struct qede_rx_queue *rxq,
 			       struct sk_buff *skb,
@@ -1157,7 +1168,7 @@ static int qede_rx_build_jumbo(struct qede_dev *edev,
 			       PAGE_SIZE, DMA_FROM_DEVICE);
 
 		skb_fill_page_desc(skb, skb_shinfo(skb)->nr_frags++,
-				   bd->data, 0, cur_size);
+				   bd->data, rxq->rx_headroom, cur_size);
 
 		skb->truesize += PAGE_SIZE;
 		skb->data_len += cur_size;
@@ -1256,7 +1267,7 @@ static int qede_rx_process_cqe(struct qede_dev *edev,
 	/* Basic validation passed; Need to prepare an SKB. This would also
 	 * guarantee to finally consume the first BD upon success.
 	 */
-	skb = qede_rx_allocate_skb(edev, rxq, bd, len, pad);
+	skb = qede_rx_build_skb(edev, rxq, bd, len, pad);
 	if (!skb) {
 		rxq->rx_alloc_errors++;
 		qede_recycle_rx_bd_ring(rxq, fp_cqe->bd_num);
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 89c581c..40e2b92 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -1197,30 +1197,8 @@ static void qede_free_rx_buffers(struct qede_dev *edev,
 	}
 }
 
-static void qede_free_sge_mem(struct qede_dev *edev, struct qede_rx_queue *rxq)
-{
-	int i;
-
-	if (edev->gro_disable)
-		return;
-
-	for (i = 0; i < ETH_TPA_MAX_AGGS_NUM; i++) {
-		struct qede_agg_info *tpa_info = &rxq->tpa_info[i];
-		struct sw_rx_data *replace_buf = &tpa_info->buffer;
-
-		if (replace_buf->data) {
-			dma_unmap_page(&edev->pdev->dev,
-				       replace_buf->mapping,
-				       PAGE_SIZE, DMA_FROM_DEVICE);
-			__free_page(replace_buf->data);
-		}
-	}
-}
-
 static void qede_free_mem_rxq(struct qede_dev *edev, struct qede_rx_queue *rxq)
 {
-	qede_free_sge_mem(edev, rxq);
-
 	/* Free rx buffers */
 	qede_free_rx_buffers(edev, rxq);
 
@@ -1232,45 +1210,15 @@ static void qede_free_mem_rxq(struct qede_dev *edev, struct qede_rx_queue *rxq)
 	edev->ops->common->chain_free(edev->cdev, &rxq->rx_comp_ring);
 }
 
-static int qede_alloc_sge_mem(struct qede_dev *edev, struct qede_rx_queue *rxq)
+static void qede_set_tpa_param(struct qede_rx_queue *rxq)
 {
-	dma_addr_t mapping;
 	int i;
 
-	if (edev->gro_disable)
-		return 0;
-
 	for (i = 0; i < ETH_TPA_MAX_AGGS_NUM; i++) {
 		struct qede_agg_info *tpa_info = &rxq->tpa_info[i];
-		struct sw_rx_data *replace_buf = &tpa_info->buffer;
-
-		replace_buf->data = alloc_pages(GFP_ATOMIC, 0);
-		if (unlikely(!replace_buf->data)) {
-			DP_NOTICE(edev,
-				  "Failed to allocate TPA skb pool [replacement buffer]\n");
-			goto err;
-		}
-
-		mapping = dma_map_page(&edev->pdev->dev, replace_buf->data, 0,
-				       PAGE_SIZE, DMA_FROM_DEVICE);
-		if (unlikely(dma_mapping_error(&edev->pdev->dev, mapping))) {
-			DP_NOTICE(edev,
-				  "Failed to map TPA replacement buffer\n");
-			goto err;
-		}
 
-		replace_buf->mapping = mapping;
-		tpa_info->buffer.page_offset = 0;
-		tpa_info->buffer_mapping = mapping;
 		tpa_info->state = QEDE_AGG_STATE_NONE;
 	}
-
-	return 0;
-err:
-	qede_free_sge_mem(edev, rxq);
-	edev->gro_disable = 1;
-	edev->ndev->features &= ~NETIF_F_GRO_HW;
-	return -ENOMEM;
 }
 
 /* This function allocates all memory needed per Rx queue */
@@ -1281,19 +1229,24 @@ static int qede_alloc_mem_rxq(struct qede_dev *edev, struct qede_rx_queue *rxq)
 	rxq->num_rx_buffers = edev->q_num_rx_buffers;
 
 	rxq->rx_buf_size = NET_IP_ALIGN + ETH_OVERHEAD + edev->ndev->mtu;
-	rxq->rx_headroom = edev->xdp_prog ? XDP_PACKET_HEADROOM : 0;
+
+	rxq->rx_headroom = edev->xdp_prog ? XDP_PACKET_HEADROOM : NET_SKB_PAD;
+	size = rxq->rx_headroom +
+	       SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 
 	/* Make sure that the headroom and  payload fit in a single page */
-	if (rxq->rx_buf_size + rxq->rx_headroom > PAGE_SIZE)
-		rxq->rx_buf_size = PAGE_SIZE - rxq->rx_headroom;
+	if (rxq->rx_buf_size + size > PAGE_SIZE)
+		rxq->rx_buf_size = PAGE_SIZE - size;
 
-	/* Segment size to spilt a page in multiple equal parts,
+	/* Segment size to spilt a page in multiple equal parts ,
 	 * unless XDP is used in which case we'd use the entire page.
 	 */
-	if (!edev->xdp_prog)
-		rxq->rx_buf_seg_size = roundup_pow_of_two(rxq->rx_buf_size);
-	else
+	if (!edev->xdp_prog) {
+		size = size + rxq->rx_buf_size;
+		rxq->rx_buf_seg_size = roundup_pow_of_two(size);
+	} else {
 		rxq->rx_buf_seg_size = PAGE_SIZE;
+	}
 
 	/* Allocate the parallel driver ring for Rx buffers */
 	size = sizeof(*rxq->sw_rx_ring) * RX_RING_SIZE;
@@ -1337,7 +1290,8 @@ static int qede_alloc_mem_rxq(struct qede_dev *edev, struct qede_rx_queue *rxq)
 		}
 	}
 
-	rc = qede_alloc_sge_mem(edev, rxq);
+	if (!edev->gro_disable)
+		qede_set_tpa_param(rxq);
 err:
 	return rc;
 }
-- 
1.8.3.1

^ permalink raw reply related

* Re: [net-next PATCH v2 3/4] net-sysfs: Add interface for Rx queue map per Tx queue
From: Florian Fainelli @ 2018-05-17 19:05 UTC (permalink / raw)
  To: Amritha Nambiar, netdev, davem
  Cc: alexander.h.duyck, sridhar.samudrala, edumazet, hannes, tom
In-Reply-To: <152643401477.4991.16716689991777771771.stgit@anamdev.jf.intel.com>

On 05/15/2018 06:26 PM, Amritha Nambiar wrote:
> Extend transmit queue sysfs attribute to configure Rx queue map
> per Tx queue. By default no receive queues are configured for the
> Tx queue.
> 
> - /sys/class/net/eth0/queues/tx-*/xps_rxqs

Please include an update to Documentation/ABI/testing/sysfs-class-net
with your new attribute.
-- 
Florian

^ permalink raw reply

* Re: [PATCH] net: ethernet: ti: cpsw: fix packet leaking in dual_mac mode
From: Greg Kroah-Hartman @ 2018-05-17 19:10 UTC (permalink / raw)
  To: Naresh Kamboju
  Cc: David Miller, Grygorii Strashko, netdev, nsekhar, open list,
	linux-omap
In-Reply-To: <CA+G9fYv5yXD4_hyUSxWra+gxux1NFv+sD2HgB7LyJ4E78bgh+A@mail.gmail.com>

On Thu, May 17, 2018 at 11:18:16PM +0530, Naresh Kamboju wrote:
> On 2 May 2018 at 20:38, David Miller <davem@davemloft.net> wrote:
> > From: Grygorii Strashko <grygorii.strashko@ti.com>
> > Date: Tue, 1 May 2018 12:41:22 -0500
> <trim>
> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> >
> > Applied and queued up for -stable, thank you.
> 
> 4.4 stable-rc build failed for arm32.
> MACHINE=am57xx-evm
> 
> Build error log:
> --------------------
> drivers/net/ethernet/ti/cpsw.c:
>  In function 'cpsw_add_dual_emac_def_ale_entries':
> drivers/net/ethernet/ti/cpsw.c:1112:23:
>  error: 'cpsw' undeclared (first use in this function)
>    cpsw_ale_control_set(cpsw->ale, slave_port,
>                         ^~~~
> drivers/net/ethernet/ti/cpsw.c:1112:23: note:
>  each undeclared identifier is reported only once for each function it appears
>  in
> scripts/Makefile.build:269: recipe for target 'drivers/net/ethernet/ti/cpsw.o'
>  failed
>  make[6]: *** [drivers/net/ethernet/ti/cpsw.o] Error 1
> scripts/Makefile.build:476: recipe for target 'drivers/net/ethernet/ti' failed
>  make[5]: *** [drivers/net/ethernet/ti] Error 2
> 

Now dropped, it's nice to see I got 3 reports about this :)

greg k-h

^ permalink raw reply

* Re: [PATCH] net: ethernet: ti: cpsw: disable mq feature for "AM33xx ES1.0" devices
From: David Miller @ 2018-05-17 19:12 UTC (permalink / raw)
  To: ivan.khoronzhuk; +Cc: grygorii.strashko, netdev, linux-kernel, linux-omap
In-Reply-To: <20180516222145.26877-1-ivan.khoronzhuk@linaro.org>

From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Thu, 17 May 2018 01:21:45 +0300

> The early versions of am33xx devices, related to ES1.0 SoC revision
> have errata limiting mq support. That's the same errata as
> commit 7da1160002f1 ("drivers: net: cpsw: add am335x errata workarround for
> interrutps")
> 
> AM33xx Errata [1] Advisory 1.0.9
> http://www.ti.com/lit/er/sprz360f/sprz360f.pdf
> 
> After additional investigation were found that drivers w/a is
> propagated on all AM33xx SoCs and on DM814x. But the errata exists
> only for ES1.0 of AM33xx family, limiting mq support for revisions
> after ES1.0. So, disable mq support only for related SoCs and use
> separate polls for revisions allowing mq.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
> 
> Based on net-next/master

Patch applied to net-next, thank you.

^ permalink raw reply

* Re: [PATCH 11/40] ipv6/flowlabel: simplify pid namespace lookup
From: Eric W. Biederman @ 2018-05-17 19:14 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Andrew Morton, Alexander Viro, Alexey Dobriyan,
	Greg Kroah-Hartman, Jiri Slaby, Alessandro Zummo,
	Alexandre Belloni, linux-acpi, drbd-dev, linux-ide, netdev,
	linux-rtc, megaraidlinux.pdl, linux-scsi, devel, linux-afs,
	linux-ext4, jfs-discussion, netfilter-devel, linux-kernel
In-Reply-To: <20180517064220.GA24300@lst.de>

Christoph Hellwig <hch@lst.de> writes:

> On Thu, May 17, 2018 at 12:28:01AM -0500, Eric W. Biederman wrote:
>> > struct pid_namespace *proc_pid_namespace(struct inode *inode)
>> > {
>> > 	// maybe warn on for s_magic not on procfs??
>> > 	return inode->i_sb->s_fs_info;
>> > }
>> 
>> That should work.  Ideally out of line for the proc_fs.h version.
>> Basically it should be a cousin of PDE_DATA.
>
> The version in Al's tree is inline and without the warning as
> I didn't want to drag in the magic.h include.  Please look at it for
> additional comments, I can send incremental fixups if needed.

Sounds good.

Eric

^ permalink raw reply

* Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
From: Florian Fainelli @ 2018-05-17 19:14 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Andrew Lunn, netdev, davem, idosch, jakub.kicinski, mlxsw,
	vivien.didelot, michael.chan, ganeshgr, saeedm, simon.horman,
	pieter.jansenvanvuuren, john.hurley, dirk.vandermerwe,
	alexander.h.duyck, ogerlitz, dsahern, vijaya.guvva,
	satananda.burla, raghu.vatsavayi, felix.manlunas, gospo,
	sathya.perla, vasundhara-v.volam, tariqt, eranbe,
	jeffrey.t.kirsher
In-Reply-To: <20180517173907.GW1972@nanopsycho>

On 05/17/2018 10:39 AM, Jiri Pirko wrote:
>>> That is compiled inside "fixed_phy", isn't it?
>>
>> It matches what CONFIG_FIXED_PHY is, so if it's built-in it also becomes
>> built-in, if is modular, it is also modular, this was fixed with
>> 40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
>> dsa-loop dependency on FIXED_PHY")
> 
> Now I have it compiled as module, and after modprobe dsa_loop I see:
> [ 1168.129202] libphy: Fixed MDIO Bus: probed
> [ 1168.222716] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
> 
> This messages I did not see when I had fixed_phy compiled as buildin.
> 
> But I still see no netdevs :/

The platform data assumes there is a network device named "eth0" as the
parent device, yes I know this is terrible, but unfortunately we don't
have anything better at this point, though that could certainly change
that to take a proper struct device reference in the future.

I am assuming that you don't have such a network device named "eth0" in
your system? You can also look at the less than 360 LOCs of the driver
and find out where your problem is, this is not mlxsw :)
-- 
Florian

^ 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