* Re: [PATCH net-next 5/6] bridge: a netlink notification should be sent when those attributes are changed by br_sysfs_if
From: Xin Long @ 2016-03-16 14:49 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: network dev, davem, Hannes Frederic Sowa
In-Reply-To: <CADvbK_eZUKV21x241heK45D9j62Fg7fZqd5rgG-S_LPed2WGjw@mail.gmail.com>
On Wed, Mar 16, 2016 at 10:45 PM, Xin Long <lucien.xin@gmail.com> wrote:
> yeah, because port fdb_flush is called by brport_store(), in the
> common function.
> do you think it''s redundant if we add a notification in bridge
> fdb_flush to keep
> consistence with port fdb_flush?
just change it on patch 1/6.
^ permalink raw reply
* Re: [PATCH net-next 5/6] bridge: a netlink notification should be sent when those attributes are changed by br_sysfs_if
From: Xin Long @ 2016-03-16 14:45 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: network dev, davem, Hannes Frederic Sowa
In-Reply-To: <56E96C4C.8060407@cumulusnetworks.com>
On Wed, Mar 16, 2016 at 10:23 PM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On 03/16/2016 02:34 PM, Xin Long wrote:
>> Now when we change the attributes of bridge or br_port by netlink,
>> a relevant netlink notification will be sent, but if we change them
>> by ioctl or sysfs, no notification will be sent.
>>
>> We should ensure that whenever those attributes change internally or from
>> sysfs/ioctl, that a netlink notification is sent out to listeners.
>>
>> Also, NetworkManager will use this in the future to listen for out-of-band
>> bridge master attribute updates and incorporate them into the runtime
>> configuration.
>>
>> This patch is used for br_sysfs_if, and we also move br_ifinfo_notify out
>> of store_flag.
>>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>> net/bridge/br_sysfs_if.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>
> Generally looks good, but it creates an inconsistency between bridge fdb_flush
> and port fdb_flush since the latter will generate a notification while the
> bridge flush will not.
>
yeah, because port fdb_flush is called by brport_store(), in the
common function.
do you think it''s redundant if we add a notification in bridge
fdb_flush to keep
consistence with port fdb_flush?
^ permalink raw reply
* net/bluetooth: use-after-free in hci_event_packet
From: Baozeng Ding @ 2016-03-16 14:44 UTC (permalink / raw)
To: linux-kernel
Cc: linux-bluetooth, netdev, marcel, gustavo, johan.hedberg, davem
Dear all,
I've hit the following use-after-free in hci_event_packet while
fuzzying kernel(4.4, on commit
9638685e32af961943b679fcb72d4ddd458eb18f) using syzkaller. I
cannot reproduce it with a standalone C program. But it reproduces
easily by replaying the fuzzer log using Go toolchain:
$ go get github.com/google/syzkaller
$ cd $GOPATH/src/github.com/google/syzkaller
$ make executor execprog
$ scp bin/syz-executor bin/syz-execprog (your@testmachine)
$ scp poc_file your@testmachine
on your test machine:
$ ./bin/syz-execprog -executor ./bin/syz-executor -cover=0 -repeat=0
-procs=16 poc_file
The content of the poc_file is as the following:
mmap(&(0x7f0000000000)=nil, (0xd77000), 0x3, 0x32, 0xffffffffffffffff,
0x0)
r0 = syz_open_dev$vhci(&(0x7f000078a000-0x2)="2f6465762f7668636900",
0x0, 0x2081)
writev(r0, &(0x7f0000d72000+0xce4)=[{&(0x7f0000d6d000)="ff00", 0x2}],
0x1)
write(r0,
&(0x7f0000d77000-0x56)="0422e1e37a57f86c13ecf1267dbc33d62693e36b1518dee20b325c6c99f61c416e7dc6dd0452224180f8197ba570311b02cf04e1875f9a9a70c9393c9d42175b341af060368bafea5e028b50be8afea2f53a9564d00b",
0x56)
After running about a few seconds, we will get the following reports:
(in /var/log/kern.log)
BUG: KASAN: use-after-free in hci_event_packet+0x8d45/0x9f90 at addr
ffff88043ef6e310
Read of size 1 by task kworker/u17:11/9348
=============================================================================
BUG kmalloc-512 (Tainted: G B ): kasan: bad access
detected
-----------------------------------------------------------------------------
INFO: Allocated in __alloc_workqueue_key+0xf7/0xe50 age=2844 cpu=2
pid=9403
[< none >] ___slab_alloc+0x4c7/0x500 kernel/mm/slub.c:2440
[< none >] __slab_alloc+0x4c/0x90 kernel/mm/slub.c:2469
[< inline >] slab_alloc_node kernel/mm/slub.c:2532
[< inline >] slab_alloc kernel/mm/slub.c:2574
[< none >] __kmalloc+0x28f/0x320 kernel/mm/slub.c:3534
[< inline >] kmalloc kernel/include/linux/slab.h:468
[< inline >] kzalloc kernel/include/linux/slab.h:607
[< none >] __alloc_workqueue_key+0xf7/0xe50 kernel/kernel/workqueue.c:3853
[< none >] hci_register_dev+0x21b/0x870 kernel/net/bluetooth/hci_core.c:3053
[< none >] vhci_create_device+0x275/0x520 kernel/drivers/bluetooth/hci_vhci.c:135
[< inline >] vhci_get_user kernel/drivers/bluetooth/hci_vhci.c:209
[< none >] vhci_write+0x2ad/0x430 kernel/drivers/bluetooth/hci_vhci.c:289
[< none >] do_iter_readv_writev+0x18b/0x250 kernel/fs/read_write.c:703
[< none >] do_readv_writev+0x3b9/0x6e0 kernel/fs/read_write.c:847
[< none >] vfs_writev+0x86/0xc0 kernel/fs/read_write.c:886
[< inline >] SYSC_writev kernel/fs/read_write.c:919
[< none >] SyS_writev+0x111/0x2b0 kernel/fs/read_write.c:911
[< none >] entry_SYSCALL_64_fastpath+0x16/0x7a
kernel/arch/x86/entry/entry_64.S:185
INFO: Freed in rcu_free_wq+0xb6/0x110 age=353 cpu=5 pid=4134
[< none >] __slab_free+0x1fc/0x320 kernel/mm/slub.c:2650
[< inline >] slab_free kernel/mm/slub.c:2805
[< none >] kfree+0x279/0x2a0 kernel/mm/slub.c:3634
[< none >] rcu_free_wq+0xb6/0x110 kernel/kernel/workqueue.c:3159
[< inline >] __rcu_reclaim kernel/kernel/rcu/rcu.h:118
[< inline >] rcu_do_batch kernel/kernel/rcu/tree.c:2704
[< inline >] invoke_rcu_callbacks kernel/kernel/rcu/tree.c:2970
[< inline >] __rcu_process_callbacks kernel/kernel/rcu/tree.c:2937
[< none >] rcu_process_callbacks+0xb08/0x1230 kernel/kernel/rcu/tree.c:2954
[< none >] __do_softirq+0x23b/0x8a0 kernel/kernel/softirq.c:273
[< inline >] invoke_softirq kernel/kernel/softirq.c:350
[< none >] irq_exit+0x15d/0x190 kernel/kernel/softirq.c:391
[< inline >] exiting_irq kernel/./arch/x86/include/asm/apic.h:659
[< none >] smp_apic_timer_interrupt+0x7b/0xa0 kernel/arch/x86/kernel/apic/apic.c:932
[< none >] apic_timer_interrupt+0x8c/0xa0 kernel/arch/x86/entry/entry_64.S:520
[< inline >] zero_user_segments kernel/include/linux/highmem.h:202
[< none >] ext4_block_write_begin+0xb2e/0xd20 kernel/fs/ext4/inode.c:938
[< none >] ext4_da_write_begin+0x3ec/0xa30 kernel/fs/ext4/inode.c:2724
[< none >] generic_perform_write+0x297/0x540 kernel/mm/filemap.c:2537
[< none >] __generic_file_write_iter+0x351/0x5a0 kernel/mm/filemap.c:2662
[< none >] ext4_file_write_iter+0x2e7/0xc80 kernel/fs/ext4/file.c:171
[< inline >] new_sync_write kernel/fs/read_write.c:517
[< none >] __vfs_write+0x300/0x470 kernel/fs/read_write.c:530
[< none >] vfs_write+0x167/0x4a0 kernel/fs/read_write.c:577
[< inline >] SYSC_write kernel/fs/read_write.c:624
[< none >] SyS_write+0x111/0x220 kernel/fs/read_write.c:616
INFO: Slab 0xffffea0010fbdb00 objects=20 used=19 fp=0xffff88043ef6e310
flags=0x2fffc0000004080
INFO: Object 0xffff88043ef6e310 @offset=8976 fp=0x (null)
CPU: 1 PID: 9348 Comm: kworker/u17:11 Tainted: G B 4.4.0+
#5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
Workqueue: hci4 hci_rx_work
00000000ffffffff ffff880433b8f6b0 ffffffff8292049d ffff88048a004b40
ffff88043ef6e310 ffff88043ef6c000 ffff880433b8f6e0 ffffffff816f2054
ffff88048a004b40 ffffea0010fbdb00 ffff88043ef6e310 ffff88043ef6e318
Call Trace:
[< inline >] __dump_stack kernel/lib/dump_stack.c:15
[<ffffffff8292049d>] dump_stack+0x6f/0xa2 kernel/lib/dump_stack.c:50
[<ffffffff816f2054>] print_trailer+0xf4/0x150 kernel/mm/slub.c:654
[<ffffffff816f875f>] object_err+0x2f/0x40 kernel/mm/slub.c:661
[< inline >] print_address_description kernel/mm/kasan/report.c:138
[<ffffffff816fb0c5>] kasan_report_error+0x215/0x530 kernel/mm/kasan/report.c:236
[< inline >] kasan_report kernel/mm/kasan/report.c:259
[<ffffffff816fb41e>] __asan_report_load1_noabort+0x3e/0x40 kernel/mm/kasan/report.c:277
[< inline >] ? hci_inquiry_result_with_rssi_evt kernel/net/bluetooth/hci_event.c:3616
[<ffffffff854db5f5>] ? hci_event_packet+0x8d45/0x9f90 kernel/net/bluetooth/hci_event.c:5323
[< inline >] hci_inquiry_result_with_rssi_evt kernel/net/bluetooth/hci_event.c:3616
[<ffffffff854db5f5>] hci_event_packet+0x8d45/0x9f90 kernel/net/bluetooth/hci_event.c:5323
[< inline >] ? spin_lock kernel/include/linux/spinlock.h:302
[<ffffffff816f3d32>] ? deactivate_slab+0x212/0x710 kernel/mm/slub.c:1949
[< inline >] ? hci_cc_read_local_amp_info kernel/net/bluetooth/hci_event.c:833
[<ffffffff854d28b0>] ? hci_cmd_complete_evt+0xcfb0/0xcfb0 kernel/net/bluetooth/hci_event.c:2905
[< inline >] ? spin_unlock kernel/include/linux/spinlock.h:347
[<ffffffff816f3f28>] ? deactivate_slab+0x408/0x710 kernel/mm/slub.c:1995
[<ffffffff813fdd20>] ? debug_check_no_locks_freed+0x290/0x290 kernel/kernel/locking/lockdep.c:4123
[<ffffffff813fdd20>] ? debug_check_no_locks_freed+0x290/0x290 kernel/kernel/locking/lockdep.c:4123
[< inline >] ? rcu_read_unlock kernel/include/linux/rcupdate.h:926
[<ffffffff813f1df7>] ? cpuacct_charge+0x1a7/0x380 kernel/kernel/sched/cpuacct.c:255
[< inline >] ? rcu_lock_release kernel/include/linux/rcupdate.h:495
[< inline >] ? rcu_read_unlock kernel/include/linux/rcupdate.h:930
[<ffffffff813f1e16>] ? cpuacct_charge+0x1c6/0x380 kernel/kernel/sched/cpuacct.c:255
[< inline >] ? task_cpu kernel/include/linux/sched.h:3111
[<ffffffff813f1cb0>] ? cpuacct_charge+0x60/0x380 kernel/kernel/sched/cpuacct.c:240
[<ffffffff8139e056>] ? rcu_read_unlock+0x16/0x70 kernel/include/linux/rcupdate.h:926
[<ffffffff813fdd20>] ? debug_check_no_locks_freed+0x290/0x290 kernel/kernel/locking/lockdep.c:4123
[<ffffffff813a0124>] ? __compute_runnable_contrib+0x54/0x70 kernel/kernel/sched/fair.c:2549
[< inline >] ? __update_load_avg kernel/kernel/sched/fair.c:2668
[<ffffffff813a0653>] ? update_cfs_rq_load_avg+0x513/0x1160 kernel/kernel/sched/fair.c:2795
[<ffffffff84c34792>] ? skb_dequeue+0x22/0x180 kernel/net/core/skbuff.c:2333
[<ffffffff813fd7ad>] ? trace_hardirqs_on+0xd/0x10 kernel/kernel/locking/lockdep.c:2619
[<ffffffff85509956>] ? hci_send_to_monitor+0x296/0x3e0 kernel/net/bluetooth/hci_sock.c:305
[<ffffffff8549ad12>] hci_rx_work+0x6f2/0xc00 kernel/net/bluetooth/hci_core.c:4157
[<ffffffff8134acaa>] ? process_one_work+0x6ca/0x1440 kernel/kernel/workqueue.c:2033
[<ffffffff8134ad74>] process_one_work+0x794/0x1440 kernel/kernel/workqueue.c:2036
[<ffffffff8134acaa>] ? process_one_work+0x6ca/0x1440 kernel/kernel/workqueue.c:2033
[<ffffffff8134a5e0>] ? pwq_dec_nr_in_flight+0x2e0/0x2e0 kernel/include/linux/compiler.h:218
[<ffffffff8134bafb>] worker_thread+0xdb/0xfc0 kernel/kernel/workqueue.c:2170
[< inline >] ? context_switch kernel/kernel/sched/core.c:2807
[<ffffffff85d794e9>] ? __schedule+0x919/0x1bd0 kernel/kernel/sched/core.c:3283
[<ffffffff8135e4ff>] kthread+0x23f/0x2d0 kernel/drivers/block/aoe/aoecmd.c:1303
[<ffffffff8134ba20>] ? process_one_work+0x1440/0x1440 kernel/include/linux/list.h:655
[<ffffffff8135e2c0>] ? kthread_create_on_node+0x3b0/0x3b0 kernel/kernel/kthread.c:285
[<ffffffff8135e2c0>] ? kthread_create_on_node+0x3b0/0x3b0 kernel/kernel/kthread.c:285
[<ffffffff85d8826f>] ret_from_fork+0x3f/0x70 kernel/arch/x86/entry/entry_64.S:468
[<ffffffff8135e2c0>] ? kthread_create_on_node+0x3b0/0x3b0 kernel/kernel/kthread.c:285
Memory state around the buggy address:
ffff88043ef6e200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88043ef6e280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff88043ef6e300: fc fc fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88043ef6e380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88043ef6e400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
============================================================
Best Regards,
Baozeng Ding
^ permalink raw reply
* Re: [PATCH net-next 4/6] bridge: a netlink notification should be sent when those attributes are changed by br_sysfs_br
From: Nikolay Aleksandrov @ 2016-03-16 14:33 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, davem, Hannes Frederic Sowa
In-Reply-To: <CADvbK_fApZ76HLaVKHKukkfNMvg3b_Htz+n-vzni6H3eJPEf2Q@mail.gmail.com>
On 03/16/2016 03:29 PM, Xin Long wrote:
> On Wed, Mar 16, 2016 at 10:14 PM, Nikolay Aleksandrov
> <nikolay@cumulusnetworks.com> wrote:
>> On 03/16/2016 02:34 PM, Xin Long wrote:
>>> Now when we change the attributes of bridge or br_port by netlink,
>>> a relevant netlink notification will be sent, but if we change them
>>> by ioctl or sysfs, no notification will be sent.
>>>
>>> We should ensure that whenever those attributes change internally or from
>>> sysfs/ioctl, that a netlink notification is sent out to listeners.
>>>
>>> Also, NetworkManager will use this in the future to listen for out-of-band
>>> bridge master attribute updates and incorporate them into the runtime
>>> configuration.
>>>
>>> This patch is used for br_sysfs_br. and we also need to remove some
>>> rtnl_trylock in old functions so that we can call it in a common one.
>>>
>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>> ---
>>> net/bridge/br_sysfs_br.c | 17 ++++++++---------
>>> net/bridge/br_vlan.c | 30 +++++-------------------------
>>> 2 files changed, 13 insertions(+), 34 deletions(-)
>>>
>>
>> What about the group_addr option ? Changing it will not generate a notification.
>>
>>
>
> group_addr is not a string-to-long convert in sysfs. so it's hard to use
> store_bridge_parm, that's why I didn't modify it.
>
> in group_addr_store():
> it also tries to hold rtnl_lock. maybe we can send rtnl msg there.
> what do you think?
Sounds good.
>
> when I cooked this patch, I was wondering why br_recalculate_fwd_mask
> "Must be protected by RTNL."
>
vlan_enabled and vlan_proto are changed under rtnl, also this can race with
changing via netlink
^ permalink raw reply
* Re: [PATCH net-next 0/6] bridge: support sending rntl info when we set attributes through sysfs/ioctl
From: Nikolay Aleksandrov @ 2016-03-16 14:30 UTC (permalink / raw)
To: Xin Long, network dev
Cc: davem, Hannes Frederic Sowa, Stephen Hemminger,
bridge@lists.linux-foundation.org
In-Reply-To: <cover.1458134414.git.lucien.xin@gmail.com>
On 03/16/2016 02:34 PM, Xin Long wrote:
> This patchset is used to support sending rntl info to user in some places,
> and ensure that whenever those attributes change internally or from sysfs,
> that a netlink notification is sent out to listeners.
>
> It also make some adjustment in bridge sysfs so that we can implement this
> easily.
>
> I've done some tests on this patchset, like:
> [br_sysfs]
> 1. change all the attribute values of br or brif:
> $ echo $value > /sys/class/net/br0/bridge/{*}
> $ echo $value > /sys/class/net/br0/brif/eth1/{*}
>
> 2. meanwhile, on another terminal to observe the msg:
> $ bridge monitor
>
> [br_ioctl]
> 1. in bridge-utils package, do some changes in br_set, let brctl command
> use ioctl to set attribute:
> if ((ret = set_sysfs(path, value)) < 0) { -->
> if (1) {
>
> $ brctl set*
>
> 2. meanwhile, on another terminal to observe the msg:
> $ bridge monitor
>
> This test covers all the attributes that brctl and sysfs support to set.
>
Please also include the bridge maintainers (CCed).
^ permalink raw reply
* Re: [PATCH net-next 4/6] bridge: a netlink notification should be sent when those attributes are changed by br_sysfs_br
From: Xin Long @ 2016-03-16 14:29 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: network dev, davem, Hannes Frederic Sowa
In-Reply-To: <56E96A3B.1030907@cumulusnetworks.com>
On Wed, Mar 16, 2016 at 10:14 PM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On 03/16/2016 02:34 PM, Xin Long wrote:
>> Now when we change the attributes of bridge or br_port by netlink,
>> a relevant netlink notification will be sent, but if we change them
>> by ioctl or sysfs, no notification will be sent.
>>
>> We should ensure that whenever those attributes change internally or from
>> sysfs/ioctl, that a netlink notification is sent out to listeners.
>>
>> Also, NetworkManager will use this in the future to listen for out-of-band
>> bridge master attribute updates and incorporate them into the runtime
>> configuration.
>>
>> This patch is used for br_sysfs_br. and we also need to remove some
>> rtnl_trylock in old functions so that we can call it in a common one.
>>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>> net/bridge/br_sysfs_br.c | 17 ++++++++---------
>> net/bridge/br_vlan.c | 30 +++++-------------------------
>> 2 files changed, 13 insertions(+), 34 deletions(-)
>>
>
> What about the group_addr option ? Changing it will not generate a notification.
>
>
group_addr is not a string-to-long convert in sysfs. so it's hard to use
store_bridge_parm, that's why I didn't modify it.
in group_addr_store():
it also tries to hold rtnl_lock. maybe we can send rtnl msg there.
what do you think?
when I cooked this patch, I was wondering why br_recalculate_fwd_mask
"Must be protected by RTNL."
^ permalink raw reply
* Re: [PATCH net-next 6/6] bridge: a netlink notification should be sent when those attributes are changed by ioctl
From: Nikolay Aleksandrov @ 2016-03-16 14:28 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: davem, Hannes Frederic Sowa
In-Reply-To: <9cadf4925a11b60a5420406b4eb164e9a2d13b50.1458134414.git.lucien.xin@gmail.com>
On 03/16/2016 02:34 PM, Xin Long wrote:
> Now when we change the attributes of bridge or br_port by netlink,
> a relevant netlink notification will be sent, but if we change them
> by ioctl or sysfs, no notification will be sent.
>
> We should ensure that whenever those attributes change internally or from
> sysfs/ioctl, that a netlink notification is sent out to listeners.
>
> Also, NetworkManager will use this in the future to listen for out-of-band
> bridge master attribute updates and incorporate them into the runtime
> configuration.
>
> This patch is used for ioctl.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/bridge/br_ioctl.c | 40 ++++++++++++++++++++++++----------------
> 1 file changed, 24 insertions(+), 16 deletions(-)
>
LGTM,
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* Re: linux-next: manual merge of the rdma tree with the net-next tree
From: Maor Gottlieb @ 2016-03-16 14:27 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Doug Ledford, David Miller, netdev, linux-next, linux-kernel,
Amir Vadai, Maor Gottlieb
In-Reply-To: <20160316115809.61b4478e@canb.auug.org.au>
2016-03-16 2:58 GMT+02:00 Stephen Rothwell <sfr@canb.auug.org.au>:
> Hi all,
>
> Today's linux-next merge of the rdma tree got a conflict in:
>
> drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>
> between commit:
>
> 60ab4584f5bf ("net/mlx5_core: Set flow steering dest only for forward rules")
>
> from the net-next tree and commit:
>
> b3638e1a7664 ("net/mlx5_core: Introduce forward to next priority action")
>
> from the rdma tree.
>
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> index e848d708d2b7,bf3446794bd5..000000000000
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
> @@@ -73,10 -73,13 +73,13 @@@
> #define BY_PASS_MIN_LEVEL (KENREL_MIN_LEVEL + MLX5_BY_PASS_NUM_PRIOS +\
> LEFTOVERS_MAX_FT)
>
> -#define KERNEL_MAX_FT 2
> -#define KERNEL_NUM_PRIOS 1
> +#define KERNEL_MAX_FT 3
> +#define KERNEL_NUM_PRIOS 2
> #define KENREL_MIN_LEVEL 2
>
> + #define ANCHOR_MAX_FT 1
> + #define ANCHOR_NUM_PRIOS 1
> + #define ANCHOR_MIN_LEVEL (BY_PASS_MIN_LEVEL + 1)
> struct node_caps {
> size_t arr_sz;
> long *caps;
> @@@ -360,8 -367,13 +367,13 @@@ static void del_rule(struct fs_node *no
> memcpy(match_value, fte->val, sizeof(fte->val));
> fs_get_obj(ft, fg->node.parent);
> list_del(&rule->node.list);
> + if (rule->sw_action == MLX5_FLOW_CONTEXT_ACTION_FWD_NEXT_PRIO) {
> + mutex_lock(&rule->dest_attr.ft->lock);
> + list_del(&rule->next_ft);
> + mutex_unlock(&rule->dest_attr.ft->lock);
> + }
> - fte->dests_size--;
> - if (fte->dests_size) {
> + if ((fte->action & MLX5_FLOW_CONTEXT_ACTION_FWD_DEST) &&
> + --fte->dests_size) {
> err = mlx5_cmd_update_fte(dev, ft,
> fg->id, fte);
> if (err)
> @@@ -762,9 -835,9 +835,10 @@@ static struct mlx5_flow_rule *alloc_rul
> if (!rule)
> return NULL;
>
> + INIT_LIST_HEAD(&rule->next_ft);
> rule->node.type = FS_TYPE_FLOW_DEST;
> - memcpy(&rule->dest_attr, dest, sizeof(*dest));
> + if (dest)
> + memcpy(&rule->dest_attr, dest, sizeof(*dest));
>
> return rule;
> }
> @@@ -783,12 -856,16 +857,17 @@@ static struct mlx5_flow_rule *add_rule_
> return ERR_PTR(-ENOMEM);
>
> fs_get_obj(ft, fg->node.parent);
> - /* Add dest to dests list- added as first element after the head */
> + /* Add dest to dests list- we need flow tables to be in the
> + * end of the list for forward to next prio rules.
> + */
> tree_init_node(&rule->node, 1, del_rule);
> - list_add_tail(&rule->node.list, &fte->node.children);
> + if (dest && dest->type != MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE)
> + list_add(&rule->node.list, &fte->node.children);
> + else
> + list_add_tail(&rule->node.list, &fte->node.children);
> - fte->dests_size++;
> - if (fte->dests_size == 1)
> + if (dest)
> + fte->dests_size++;
> + if (fte->dests_size == 1 || !dest)
> err = mlx5_cmd_create_fte(get_dev(&ft->node),
> ft, fg->id, fte);
> else
Hi Stephen,
I reveiwed your merge and it's fine.
Thanks,
Maor
^ permalink raw reply
* Re: [RFC v2 -next 2/2] virtio_net: Read the advised MTU
From: Michael S. Tsirkin @ 2016-03-16 14:24 UTC (permalink / raw)
To: Aaron Conole; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <1458075853-14789-3-git-send-email-aconole@redhat.com>
On Tue, Mar 15, 2016 at 05:04:13PM -0400, Aaron Conole wrote:
> This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
> exists, read the advised MTU and use it.
>
> No proper error handling is provided for the case where a user changes the
> negotiated MTU. A future commit will add proper error handling. Instead, a
> warning is emitted if the guest changes the device MTU after previously
> being given advice.
I don't see this as an error. Device might at best give a hint,
user/network admin always knows best.
>
> Signed-off-by: Aaron Conole <aconole@bytheb.org>
> ---
> v2:
> * Whitespace cleanup in the last hunk
> * Code style change around the pr_warn
> * Additional test for mtu change before printing warning
>
> drivers/net/virtio_net.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 767ab11..429fe01 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -146,6 +146,7 @@ struct virtnet_info {
> virtio_net_ctrl_ack ctrl_status;
> u8 ctrl_promisc;
> u8 ctrl_allmulti;
> + bool negotiated_mtu;
> };
>
> struct padded_vnet_hdr {
> @@ -1390,8 +1391,11 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>
> static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
> {
> + struct virtnet_info *vi = netdev_priv(dev);
> if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
> return -EINVAL;
> + if ((vi->negotiated_mtu) && (dev->mtu != new_mtu))
> + pr_warn("changing mtu while the advised mtu bit exists.");
I don't really see why are we warning here. Just drop this chunk,
as well as the flag in struct virtnet_info.
> dev->mtu = new_mtu;
> return 0;
> }
> @@ -1836,6 +1840,13 @@ static int virtnet_probe(struct virtio_device *vdev)
> if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
> vi->has_cvq = true;
>
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
> + vi->negotiated_mtu = true;
> + dev->mtu = virtio_cread16(vdev,
> + offsetof(struct virtio_net_config,
> + mtu));
> + }
> +
> if (vi->any_header_sg)
> dev->needed_headroom = vi->hdr_len;
>
> @@ -2019,6 +2030,7 @@ static unsigned int features[] = {
> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> VIRTIO_NET_F_CTRL_MAC_ADDR,
> VIRTIO_F_ANY_LAYOUT,
> + VIRTIO_NET_F_MTU,
> };
>
> static struct virtio_driver virtio_net_driver = {
> --
> 2.5.0
^ permalink raw reply
* Re: [PATCH net-next 5/6] bridge: a netlink notification should be sent when those attributes are changed by br_sysfs_if
From: Nikolay Aleksandrov @ 2016-03-16 14:23 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: davem, Hannes Frederic Sowa
In-Reply-To: <7c788b65786582209a3ba4bc4d4ed279f6f77568.1458134414.git.lucien.xin@gmail.com>
On 03/16/2016 02:34 PM, Xin Long wrote:
> Now when we change the attributes of bridge or br_port by netlink,
> a relevant netlink notification will be sent, but if we change them
> by ioctl or sysfs, no notification will be sent.
>
> We should ensure that whenever those attributes change internally or from
> sysfs/ioctl, that a netlink notification is sent out to listeners.
>
> Also, NetworkManager will use this in the future to listen for out-of-band
> bridge master attribute updates and incorporate them into the runtime
> configuration.
>
> This patch is used for br_sysfs_if, and we also move br_ifinfo_notify out
> of store_flag.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/bridge/br_sysfs_if.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
Generally looks good, but it creates an inconsistency between bridge fdb_flush
and port fdb_flush since the latter will generate a notification while the
bridge flush will not.
^ permalink raw reply
* [BUG] bcmgenet tx path
From: Eric Dumazet @ 2016-03-16 14:18 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev
Hi Florian
I was looking at drivers/net/ethernet/broadcom/genet/bcmgenet.c
and found TX completion (__bcmgenet_tx_reclaim()) was freeing skb before
frags were actually dma unmapped.
Are you sure this is OK ? bnx2 and bnx2x actually do the reverse (unmap
all frags before freeing skb)
A second problem is the dma_unmap_single() uses tx_cb_ptr->skb->len for
the length, while it really should be the same thing that was used in
bcmgenet_xmit_single()
skb_len = skb_headlen(skb) < ETH_ZLEN ? ETH_ZLEN : skb_headlen(skb);
So maybe something like :
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index d7e01a7..9211b9c7 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1197,7 +1197,7 @@ static unsigned int __bcmgenet_tx_reclaim(struct net_device *dev,
dev->stats.tx_bytes += tx_cb_ptr->skb->len;
dma_unmap_single(&dev->dev,
dma_unmap_addr(tx_cb_ptr, dma_addr),
- tx_cb_ptr->skb->len,
+ dma_unmap_len(tx_cb_ptr, dma_len),
DMA_TO_DEVICE);
bcmgenet_free_cb(tx_cb_ptr);
} else if (dma_unmap_addr(tx_cb_ptr, dma_addr)) {
@@ -1308,7 +1308,7 @@ static int bcmgenet_xmit_single(struct net_device *dev,
}
dma_unmap_addr_set(tx_cb_ptr, dma_addr, mapping);
- dma_unmap_len_set(tx_cb_ptr, dma_len, skb->len);
+ dma_unmap_len_set(tx_cb_ptr, dma_len, skb_len);
length_status = (skb_len << DMA_BUFLENGTH_SHIFT) | dma_desc_flags |
(priv->hw_params->qtag_mask << DMA_TX_QTAG_SHIFT) |
DMA_TX_APPEND_CRC;
^ permalink raw reply related
* Re: [PATCH 2/2] lan78xx: add ndo_get_stats64
From: Sergei Shtylyov @ 2016-03-16 14:16 UTC (permalink / raw)
To: Woojung.Huh, davem, netdev; +Cc: UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D404C906A@CHN-SV-EXMX02.mchp-main.com>
Hello.
On 3/16/2016 1:52 AM, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <woojung.huh@microchip.com>
>
> Add lan78xx_get_stats64 of ndo_get_stats64 to report
> all statistics counters including errors from HW statistics.
>
> Read from HW when auto suspend is disabled, use saved counter when
> auto suspend is enabled because periodic call to ndo_get_stats64
> prevents USB auto suspend.
>
> Signed-off-by: Woojung Huh <woojung.huh@microchip.com>
> ---
> drivers/net/usb/lan78xx.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index f20890e..f4a9708f 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -3261,6 +3261,54 @@ void lan78xx_tx_timeout(struct net_device *net)
> tasklet_schedule(&dev->bh);
> }
>
> +struct rtnl_link_stats64 *lan78xx_get_stats64(struct net_device *netdev,
> + struct rtnl_link_stats64 *storage)
> +{
[...]
> + storage->rx_length_errors = (stats.rx_undersize_frame_errors +
> + stats.rx_oversize_frame_errors);
Parens not needed.
> + storage->rx_crc_errors = stats.rx_fcs_errors;
> + storage->rx_frame_errors = stats.rx_alignment_errors;
> + storage->rx_fifo_errors = stats.rx_dropped_frames;
> + storage->rx_over_errors = stats.rx_oversize_frame_errors;
> + storage->rx_errors = (stats.rx_fcs_errors +
> + stats.rx_alignment_errors +
> + stats.rx_fragment_errors +
> + stats.rx_jabber_errors +
> + stats.rx_undersize_frame_errors +
> + stats.rx_oversize_frame_errors +
> + stats.rx_dropped_frames);
Neither here.
> +
> + storage->tx_carrier_errors = stats.tx_carrier_errors;
> + storage->tx_errors = (stats.tx_fcs_errors +
> + stats.tx_excess_deferral_errors +
> + stats.tx_carrier_errors);
And here.
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next 4/6] bridge: a netlink notification should be sent when those attributes are changed by br_sysfs_br
From: Nikolay Aleksandrov @ 2016-03-16 14:14 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: davem, Hannes Frederic Sowa
In-Reply-To: <38a10eecc6347a3c47b55c86b4783c49b6dbbc99.1458134414.git.lucien.xin@gmail.com>
On 03/16/2016 02:34 PM, Xin Long wrote:
> Now when we change the attributes of bridge or br_port by netlink,
> a relevant netlink notification will be sent, but if we change them
> by ioctl or sysfs, no notification will be sent.
>
> We should ensure that whenever those attributes change internally or from
> sysfs/ioctl, that a netlink notification is sent out to listeners.
>
> Also, NetworkManager will use this in the future to listen for out-of-band
> bridge master attribute updates and incorporate them into the runtime
> configuration.
>
> This patch is used for br_sysfs_br. and we also need to remove some
> rtnl_trylock in old functions so that we can call it in a common one.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/bridge/br_sysfs_br.c | 17 ++++++++---------
> net/bridge/br_vlan.c | 30 +++++-------------------------
> 2 files changed, 13 insertions(+), 34 deletions(-)
>
What about the group_addr option ? Changing it will not generate a notification.
^ permalink raw reply
* Re: [RFC v2 -next 2/2] virtio_net: Read the advised MTU
From: Sergei Shtylyov @ 2016-03-16 14:11 UTC (permalink / raw)
To: Aaron Conole, netdev, Michael S. Tsirkin, virtualization,
linux-kernel
In-Reply-To: <1458075853-14789-3-git-send-email-aconole@redhat.com>
Hello.
On 3/16/2016 12:04 AM, Aaron Conole wrote:
> This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
> exists, read the advised MTU and use it.
>
> No proper error handling is provided for the case where a user changes the
> negotiated MTU. A future commit will add proper error handling. Instead, a
> warning is emitted if the guest changes the device MTU after previously
> being given advice.
>
> Signed-off-by: Aaron Conole <aconole@bytheb.org>
> ---
> v2:
> * Whitespace cleanup in the last hunk
> * Code style change around the pr_warn
> * Additional test for mtu change before printing warning
>
> drivers/net/virtio_net.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 767ab11..429fe01 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
[...]
> @@ -1390,8 +1391,11 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>
> static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
> {
> + struct virtnet_info *vi = netdev_priv(dev);
> if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
> return -EINVAL;
> + if ((vi->negotiated_mtu) && (dev->mtu != new_mtu))
Inner parens not needed, please be consistent with the code above.
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next 3/6] bridge: simplify the stp_state_store by calling store_bridge_parm
From: Nikolay Aleksandrov @ 2016-03-16 14:06 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: davem, Hannes Frederic Sowa
In-Reply-To: <45f430c026ce8a2fb358c650a41452aebff9ec36.1458134414.git.lucien.xin@gmail.com>
On 03/16/2016 02:34 PM, Xin Long wrote:
> There are some repetitive codes in stp_state_store, we can remove
> them by calling store_bridge_parm.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/bridge/br_sysfs_br.c | 24 +++++++-----------------
> 1 file changed, 7 insertions(+), 17 deletions(-)
>
LGTM. Note: it introduces a bug (missing rtnl) until patch 04 is applied.
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* Re: [net-next 1/2] bonding: remove duplicate set of flag IFF_MULTICAST
From: Andy Gospodarek @ 2016-03-16 14:01 UTC (permalink / raw)
To: Zhang Shengju; +Cc: davem, j.vosburgh, jiri, netdev
In-Reply-To: <1458122356-25021-2-git-send-email-zhangshengju@cmss.chinamobile.com>
On Wed, Mar 16, 2016 at 09:59:15AM +0000, Zhang Shengju wrote:
> Remove unnecessary set of flag IFF_MULTICAST, since ether_setup
> already does this.
>
> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com>
> ---
> drivers/net/bonding/bond_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index b6236ff..270b39c 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4175,7 +4175,7 @@ void bond_setup(struct net_device *bond_dev)
> SET_NETDEV_DEVTYPE(bond_dev, &bond_type);
>
> /* Initialize the device options */
> - bond_dev->flags |= IFF_MASTER|IFF_MULTICAST;
> + bond_dev->flags |= IFF_MASTER;
> bond_dev->priv_flags |= IFF_BONDING | IFF_UNICAST_FLT | IFF_NO_QUEUE;
> bond_dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE | IFF_TX_SKB_SHARING);
>
> --
> 1.8.3.1
>
>
>
^ permalink raw reply
* Re: [PATCH net-next 2/6] bridge: simplify the forward_delay_store by calling store_bridge_parm
From: Nikolay Aleksandrov @ 2016-03-16 13:58 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: davem, Hannes Frederic Sowa
In-Reply-To: <f379a033c5b7798906c57dd1bb67df89b7de02cb.1458134414.git.lucien.xin@gmail.com>
On 03/16/2016 02:34 PM, Xin Long wrote:
> There are some repetitive codes in forward_delay_store, we can remove
> them by calling store_bridge_parm.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/bridge/br_sysfs_br.c | 27 ++++++++++-----------------
> 1 file changed, 10 insertions(+), 17 deletions(-)
>
I actually have a similar patch in my tree. :-)
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* Re: [PATCH v3 2/9] net: arc_emac: add phy reset is optional for device tree
From: Sergei Shtylyov @ 2016-03-16 13:57 UTC (permalink / raw)
To: Caesar Wang, Heiko Stuebner, David S. Miller, Rob Herring
Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
keescook-hpIqsD4AKlfQT0dZR+AlfA, leozwang-hpIqsD4AKlfQT0dZR+AlfA,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1457942520-12859-3-git-send-email-wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Hello.
On 3/14/2016 11:01 AM, Caesar Wang wrote:
> This patch adds the following property for arc_emac.
>
> 1) phy-reset-gpios:
> The phy-reset-gpio is an optional property for arc emac device tree boot.
> Change the binding document to match the driver code.
>
> 2) phy-reset-duration:
> Different boards may require different phy reset duration. Add property
> phy-reset-duration for device tree probe, so that the boards that need
> a longer reset duration can specify it in their device tree.
>
> Anyway, we can add the above property for arc emac.
>
> Signed-off-by: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Could you have a look at drivers/net/ethernet/cadence/macb/? It seems to be
the only driver which places the PHY's "reset-gpios" prop correctly, into the
PHY subnode? I'm currently working on adding support of this prop into phylib...
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] openvswitch: call only into reachable nf-nat code
From: Arnd Bergmann @ 2016-03-16 13:56 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Joe Stringer, Paolo Abeni,
David S. Miller
In-Reply-To: <20160316132536.GA29550@salvia>
On Wednesday 16 March 2016 14:25:36 Pablo Neira Ayuso wrote:
> Not related with this patch, just a side note/recommendation.
>
> I understand this code just got into tree, and that this needs a bit
> work/iterations but this thing above is ugly, I wonder if there is a
> better way to avoid this.
>
> Probably with some modularization of the openvswitch code this will
> look better, I mean:
>
> 1) adding Kconfig switches to enable conntrack and NAT support to
> net/openvswitch/Kconfig.
>
> 2) Move the NAT code to the corresponding openvswitch/nat.c file.
>
> Just my two cents.
Yes, I think that would be good too. I also found that the driver
used to look like that but it was changed as part of f88f69dd17f1
("openvswitch: Remove conntrack Kconfig option.").
Arnd
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH net-next 1/6] bridge: add rtnl_lock in fdb_flush in br_sysfs_br.c
From: Nikolay Aleksandrov @ 2016-03-16 13:55 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: davem, Hannes Frederic Sowa
In-Reply-To: <059d608901fd86407fed374e567c46447aa8c88f.1458134414.git.lucien.xin@gmail.com>
On 03/16/2016 02:34 PM, Xin Long wrote:
> In fdb_delete, it will send rtnl msg, so before that, we should
> hold rtnl_lock in the function that call it in sysfs.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/bridge/br_sysfs_br.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
IIRC rtnl_notify() doesn't require rtnl lock to be held so this patch is not
needed
^ permalink raw reply
* Re: [PATCH 5/5] net: macb: Fix simple typo.
From: Nicolas Ferre @ 2016-03-16 13:51 UTC (permalink / raw)
To: Michal Simek, Moritz Fischer
Cc: joe, davem, netdev, linux-kernel, moritz.fischer.private
In-Reply-To: <56E7237A.10401@xilinx.com>
Le 14/03/2016 21:47, Michal Simek a écrit :
> On 13.3.2016 20:10, Moritz Fischer wrote:
>> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
>> ---
>> drivers/net/ethernet/cadence/macb.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>> index a0c01e5..681e5bf 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -127,7 +127,7 @@ static void hw_writel(struct macb *bp, int offset, u32 value)
>> }
>>
>> /* Find the CPU endianness by using the loopback bit of NCR register. When the
>> - * CPU is in big endian we need to program swaped mode for management
>> + * CPU is in big endian we need to program swapped mode for management
>> * descriptor access.
>> */
>> static bool hw_is_native_io(void __iomem *addr)
>>
>
> Remove dot at the end of subject and feel free to add my:
> Acked-by: Michal Simek <michal.simek@xilinx.com>
Yes, same for me, no dot. But anyway, here is my:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Thanks Moritz for the patches and Michal for the reviews.
Bye,
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH 4/5] net: macb: Use ether_addr_copy over memcpy
From: Nicolas Ferre @ 2016-03-16 13:48 UTC (permalink / raw)
To: Moritz Fischer
Cc: michal.simek, joe, davem, netdev, linux-kernel,
moritz.fischer.private
In-Reply-To: <1457896247-25934-5-git-send-email-moritz.fischer@ettus.com>
Le 13/03/2016 20:10, Moritz Fischer a écrit :
> Checkpatch suggests using ether_addr_copy over memcpy
> to copy the mac address.
>
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
Yes:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 53400f6..a0c01e5 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2891,7 +2891,7 @@ static int macb_probe(struct platform_device *pdev)
>
> mac = of_get_mac_address(np);
> if (mac)
> - memcpy(bp->dev->dev_addr, mac, ETH_ALEN);
> + ether_addr_copy(bp->dev->dev_addr, mac);
> else
> macb_get_hwaddr(bp);
>
>
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH 3/5] net: macb: Address checkpatch 'check' suggestions
From: Nicolas Ferre @ 2016-03-16 13:46 UTC (permalink / raw)
To: Moritz Fischer
Cc: michal.simek, joe, davem, netdev, linux-kernel,
moritz.fischer.private
In-Reply-To: <1457896247-25934-4-git-send-email-moritz.fischer@ettus.com>
Le 13/03/2016 20:10, Moritz Fischer a écrit :
> This commit deals with a bunch of checkpatch suggestions
> that without changing behavior make checkpatch happier.
>
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 46 +++++++++++++++++++------------------
> 1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index c2d31c5..53400f6 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -184,7 +184,7 @@ static void macb_get_hwaddr(struct macb *bp)
>
> pdata = dev_get_platdata(&bp->pdev->dev);
>
> - /* Check all 4 address register for vaild address */
> + /* Check all 4 address register for valid address */
> for (i = 0; i < 4; i++) {
> bottom = macb_or_gem_readl(bp, SA1B + i * 8);
> top = macb_or_gem_readl(bp, SA1T + i * 8);
> @@ -292,7 +292,7 @@ static void macb_set_tx_clk(struct clk *clk, int speed, struct net_device *dev)
> ferr = DIV_ROUND_UP(ferr, rate / 100000);
> if (ferr > 5)
> netdev_warn(dev, "unable to generate target frequency: %ld Hz\n",
> - rate);
> + rate);
>
> if (clk_set_rate(clk, rate_rounded))
> netdev_err(dev, "adjusting tx_clk failed.\n");
> @@ -426,7 +426,7 @@ static int macb_mii_init(struct macb *bp)
> macb_writel(bp, NCR, MACB_BIT(MPE));
>
> bp->mii_bus = mdiobus_alloc();
> - if (bp->mii_bus == NULL) {
> + if (!bp->mii_bus) {
> err = -ENOMEM;
> goto err_out;
> }
> @@ -435,7 +435,7 @@ static int macb_mii_init(struct macb *bp)
> bp->mii_bus->read = &macb_mdio_read;
> bp->mii_bus->write = &macb_mdio_write;
> snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
> - bp->pdev->name, bp->pdev->id);
> + bp->pdev->name, bp->pdev->id);
> bp->mii_bus->priv = bp;
> bp->mii_bus->parent = &bp->dev->dev;
> pdata = dev_get_platdata(&bp->pdev->dev);
> @@ -656,7 +656,7 @@ static void macb_tx_interrupt(struct macb_queue *queue)
> queue_writel(queue, ISR, MACB_BIT(TCOMP));
>
> netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n",
> - (unsigned long)status);
> + (unsigned long)status);
>
> head = queue->tx_head;
> for (tail = queue->tx_tail; tail != head; tail++) {
> @@ -725,10 +725,10 @@ static void gem_rx_refill(struct macb *bp)
>
> bp->rx_prepared_head++;
>
> - if (bp->rx_skbuff[entry] == NULL) {
> + if (!bp->rx_skbuff[entry]) {
> /* allocate sk_buff for this free entry in ring */
> skb = netdev_alloc_skb(bp->dev, bp->rx_buffer_size);
> - if (unlikely(skb == NULL)) {
> + if (unlikely(!skb)) {
> netdev_err(bp->dev,
> "Unable to allocate sk_buff\n");
> break;
> @@ -762,7 +762,7 @@ static void gem_rx_refill(struct macb *bp)
> wmb();
>
> netdev_vdbg(bp->dev, "rx ring: prepared head %d, tail %d\n",
> - bp->rx_prepared_head, bp->rx_tail);
> + bp->rx_prepared_head, bp->rx_tail);
> }
>
> /* Mark DMA descriptors from begin up to and not including end as unused */
> @@ -876,8 +876,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
> len = desc->ctrl & bp->rx_frm_len_mask;
>
> netdev_vdbg(bp->dev, "macb_rx_frame frags %u - %u (len %u)\n",
> - macb_rx_ring_wrap(first_frag),
> - macb_rx_ring_wrap(last_frag), len);
> + macb_rx_ring_wrap(first_frag),
> + macb_rx_ring_wrap(last_frag), len);
>
> /* The ethernet header starts NET_IP_ALIGN bytes into the
> * first buffer. Since the header is 14 bytes, this makes the
> @@ -916,7 +916,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
> frag_len = len - offset;
> }
> skb_copy_to_linear_data_offset(skb, offset,
> - macb_rx_buffer(bp, frag), frag_len);
> + macb_rx_buffer(bp, frag),
> + frag_len);
> offset += bp->rx_buffer_size;
> desc = macb_rx_desc(bp, frag);
> desc->addr &= ~MACB_BIT(RX_USED);
> @@ -934,7 +935,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
> bp->stats.rx_packets++;
> bp->stats.rx_bytes += skb->len;
> netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n",
> - skb->len, skb->csum);
> + skb->len, skb->csum);
> netif_receive_skb(skb);
>
> return 0;
> @@ -999,7 +1000,7 @@ static int macb_poll(struct napi_struct *napi, int budget)
> work_done = 0;
>
> netdev_vdbg(bp->dev, "poll: status = %08lx, budget = %d\n",
> - (unsigned long)status, budget);
> + (unsigned long)status, budget);
>
> work_done = bp->macbgem_ops.mog_rx(bp, budget);
> if (work_done < budget) {
> @@ -1214,7 +1215,7 @@ static unsigned int macb_tx_map(struct macb *bp,
> }
>
> /* Should never happen */
> - if (unlikely(tx_skb == NULL)) {
> + if (unlikely(!tx_skb)) {
> netdev_err(bp->dev, "BUG! empty skb!\n");
> return 0;
> }
> @@ -1284,16 +1285,16 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>
> #if defined(DEBUG) && defined(VERBOSE_DEBUG)
> netdev_vdbg(bp->dev,
> - "start_xmit: queue %hu len %u head %p data %p tail %p end %p\n",
> - queue_index, skb->len, skb->head, skb->data,
> - skb_tail_pointer(skb), skb_end_pointer(skb));
> + "start_xmit: queue %hu len %u head %p data %p tail %p end %p\n",
> + queue_index, skb->len, skb->head, skb->data,
> + skb_tail_pointer(skb), skb_end_pointer(skb));
> print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_OFFSET, 16, 1,
> skb->data, 16, true);
> #endif
>
> /* Count how many TX buffer descriptors are needed to send this
> * socket buffer: skb fragments of jumbo frames may need to be
> - * splitted into many buffer descriptors.
> + * split into many buffer descriptors.
> */
> count = DIV_ROUND_UP(skb_headlen(skb), bp->max_tx_length);
> nr_frags = skb_shinfo(skb)->nr_frags;
> @@ -1344,8 +1345,8 @@ static void macb_init_rx_buffer_size(struct macb *bp, size_t size)
>
> if (bp->rx_buffer_size % RX_BUFFER_MULTIPLE) {
> netdev_dbg(bp->dev,
> - "RX buffer must be multiple of %d bytes, expanding\n",
> - RX_BUFFER_MULTIPLE);
> + "RX buffer must be multiple of %d bytes, expanding\n",
> + RX_BUFFER_MULTIPLE);
> bp->rx_buffer_size =
> roundup(bp->rx_buffer_size, RX_BUFFER_MULTIPLE);
> }
> @@ -1368,7 +1369,7 @@ static void gem_free_rx_buffers(struct macb *bp)
> for (i = 0; i < RX_RING_SIZE; i++) {
> skb = bp->rx_skbuff[i];
>
> - if (skb == NULL)
> + if (!skb)
> continue;
>
> desc = &bp->rx_ring[i];
> @@ -1776,7 +1777,8 @@ static void macb_sethashtable(struct net_device *dev)
> unsigned int bitnr;
> struct macb *bp = netdev_priv(dev);
>
> - mc_filter[0] = mc_filter[1] = 0;
> + mc_filter[0] = 0;
> + mc_filter[1] = 0;
>
> netdev_for_each_mc_addr(ha, dev) {
> bitnr = hash_get_index(ha->addr);
>
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH 2/5] net: macb: Fix coding style warnings
From: Nicolas Ferre @ 2016-03-16 13:46 UTC (permalink / raw)
To: Michal Simek, Moritz Fischer
Cc: joe, davem, netdev, linux-kernel, moritz.fischer.private
In-Reply-To: <56E724BB.5030600@xilinx.com>
Le 14/03/2016 21:53, Michal Simek a écrit :
> On 13.3.2016 20:10, Moritz Fischer wrote:
>> This commit takes care of the coding style warnings
>> that are mostly due to a different comment style and
>> lines over 80 chars, as well as a dangling else.
>>
>> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
>> ---
>> drivers/net/ethernet/cadence/macb.c | 101 +++++++++++++++---------------------
>> 1 file changed, 43 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>> index 4370f37..c2d31c5 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -58,8 +58,7 @@
>>
>> #define GEM_MTU_MIN_SIZE 68
>>
>> -/*
>> - * Graceful stop timeouts in us. We should allow up to
>> +/* Graceful stop timeouts in us. We should allow up to
>> * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions)
>> */
>> #define MACB_HALT_TIMEOUT 1230
>> @@ -127,8 +126,7 @@ static void hw_writel(struct macb *bp, int offset, u32 value)
>> writel_relaxed(value, bp->regs + offset);
>> }
>>
>> -/*
>> - * Find the CPU endianness by using the loopback bit of NCR register. When the
>> +/* Find the CPU endianness by using the loopback bit of NCR register. When the
>
> TBH: I would rather see this converting to kernel-doc format instead of
> using this networking block.
As there is hardly any kernel-doc comments in this driver, I won't force
Moritz to move this one to it.
I would advice, if someone want to move to kernel-doc for some function
comments, to do it in a separate patch (series).
> Also splitting this to more patches will be better. Just by categories
> but that's just my opinion.
Well, yes... but I won't be too picky for such a patch. So here is my:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Thank for your feedback, bye,
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH 0/5] net: macb: Checkpatch cleanups
From: Nicolas Ferre @ 2016-03-16 13:39 UTC (permalink / raw)
To: Moritz Fischer
Cc: michal.simek, joe, davem, netdev, linux-kernel,
moritz.fischer.private
In-Reply-To: <1457896247-25934-1-git-send-email-moritz.fischer@ettus.com>
Le 13/03/2016 20:10, Moritz Fischer a écrit :
> Hi all,
>
> I backed out the variable scope changes and made a separate
> patch for the ether_addr_copy change.
>
> Changes from v1:
As it's v2, it's better to add it in each subject of the patch series like:
"[PATCH v2 0/5] net: macb: Checkpatch cleanups"
> * Backed out variable scope changes
> * Separated out ether_addr_copy into it's own commit
> * Fixed typo in comments as suggested by Joe
>
> Cheers,
>
> Moritz
>
> Moritz Fischer (5):
> net: macb: Fix coding style error message
> net: macb: Fix coding style warnings
> net: macb: Address checkpatch 'check' suggestions
> net: macb: Use ether_addr_copy over memcpy
> net: macb: Fix simple typo.
>
> drivers/net/ethernet/cadence/macb.c | 153 +++++++++++++++++-------------------
> 1 file changed, 70 insertions(+), 83 deletions(-)
>
--
Nicolas Ferre
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox