* [iproute PATCH] tc: htb: Print default value in hex
From: Phil Sutter @ 2018-10-19 15:42 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jiri Pirko, netdev
Value of 'default' is assumed to be hexadecimal when parsing, so
consequently it should be printed in hex as well. This is a regression
introduced when adding JSON output.
Fixes: f354fa6aa5ff0 ("tc: jsonify htb qdisc")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tc/q_htb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/q_htb.c b/tc/q_htb.c
index c8b2941d945b7..c69485db8ef7d 100644
--- a/tc/q_htb.c
+++ b/tc/q_htb.c
@@ -332,7 +332,7 @@ static int htb_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
if (RTA_PAYLOAD(tb[TCA_HTB_INIT]) < sizeof(*gopt)) return -1;
print_int(PRINT_ANY, "r2q", "r2q %d", gopt->rate2quantum);
- print_uint(PRINT_ANY, "default", " default %u", gopt->defcls);
+ print_uint(PRINT_ANY, "default", " default %x", gopt->defcls);
print_uint(PRINT_ANY, "direct_packets_stat",
" direct_packets_stat %u", gopt->direct_pkts);
if (show_details) {
--
2.19.0
^ permalink raw reply related
* KASAN: use-after-free Read in sk_psock_link_pop
From: syzbot @ 2018-10-19 15:54 UTC (permalink / raw)
To: daniel, davem, john.fastabend, linux-kernel, netdev,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 3a3295bfa6f4 Merge branch 'sctp-fix-sk_wmem_queued-and-use..
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=10a09791400000
kernel config: https://syzkaller.appspot.com/x/.config?x=133950703f7759f9
dashboard link: https://syzkaller.appspot.com/bug?extid=1651eee005f9de26ec35
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+1651eee005f9de26ec35@syzkaller.appspotmail.com
__nla_parse: 2 callbacks suppressed
netlink: 20 bytes leftover after parsing attributes in process
`syz-executor0'.
==================================================================
BUG: KASAN: use-after-free in __lock_acquire+0x37c2/0x4ec0
kernel/locking/lockdep.c:3290
Read of size 8 at addr ffff8801bcae2ff8 by task syz-executor3/30186
CPU: 0 PID: 30186 Comm: syz-executor3 Not tainted 4.19.0-rc7+ #266
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
__lock_acquire+0x37c2/0x4ec0 kernel/locking/lockdep.c:3290
lock_acquire+0x1ed/0x520 kernel/locking/lockdep.c:3900
__raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
_raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:168
spin_lock_bh include/linux/spinlock.h:334 [inline]
sk_psock_link_pop+0x93/0x3b0 net/core/skmsg.c:504
tcp_bpf_remove+0xe5/0x130 net/ipv4/tcp_bpf.c:497
tcp_bpf_close+0x1c6/0x4a0 net/ipv4/tcp_bpf.c:538
inet_release+0x104/0x1f0 net/ipv4/af_inet.c:428
__sock_release+0xd7/0x250 net/socket.c:579
sock_close+0x19/0x20 net/socket.c:1141
__fput+0x385/0xa30 fs/file_table.c:278
____fput+0x15/0x20 fs/file_table.c:309
task_work_run+0x1e8/0x2a0 kernel/task_work.c:113
get_signal+0x155e/0x1980 kernel/signal.c:2343
do_signal+0x9c/0x21e0 arch/x86/kernel/signal.c:816
exit_to_usermode_loop+0x2e5/0x380 arch/x86/entry/common.c:162
prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline]
syscall_return_slowpath arch/x86/entry/common.c:268 [inline]
do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457569
Code: fd b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f386588dc78 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
RAX: 0000000000280000 RBX: 0000000000000006 RCX: 0000000000457569
RDX: fffffffffffffe6e RSI: 0000000020a88f88 RDI: 0000000000000004
RBP: 000000000072bf00 R08: 0000000020e68000 R09: 0000000000000010
R10: 0000000020000000 R11: 0000000000000246 R12: 00007f386588e6d4
R13: 00000000004c3915 R14: 00000000004d57c0 R15: 00000000ffffffff
Allocated by task 30195:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
kmem_cache_alloc_node_trace+0x14c/0x740 mm/slab.c:3663
kmalloc_node include/linux/slab.h:551 [inline]
kzalloc_node include/linux/slab.h:718 [inline]
sk_psock_init+0xe4/0x630 net/core/skmsg.c:474
sock_map_link.isra.6+0xa6b/0xe30 net/core/sock_map.c:191
sock_hash_update_common+0x19b/0x11e0 net/core/sock_map.c:669
sock_hash_update_elem+0x306/0x470 net/core/sock_map.c:738
map_update_elem+0x819/0xdf0 kernel/bpf/syscall.c:818
__do_sys_bpf kernel/bpf/syscall.c:2400 [inline]
__se_sys_bpf kernel/bpf/syscall.c:2371 [inline]
__x64_sys_bpf+0x32d/0x510 kernel/bpf/syscall.c:2371
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 14:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xcf/0x230 mm/slab.c:3813
sk_psock_destroy_deferred+0x5f0/0x850 net/core/skmsg.c:559
process_one_work+0xc90/0x1b90 kernel/workqueue.c:2153
worker_thread+0x17f/0x1390 kernel/workqueue.c:2296
kthread+0x35a/0x420 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413
The buggy address belongs to the object at ffff8801bcae2dc0
which belongs to the cache kmalloc-1024 of size 1024
The buggy address is located 568 bytes inside of
1024-byte region [ffff8801bcae2dc0, ffff8801bcae31c0)
The buggy address belongs to the page:
page:ffffea0006f2b880 count:1 mapcount:0 mapping:ffff8801da800ac0 index:0x0
compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffffea0007238b08 ffffea0006deca08 ffff8801da800ac0
raw: 0000000000000000 ffff8801bcae2040 0000000100000007 0000000000000000
page dumped because: kasan: bad access detected
netlink: 20 bytes leftover after parsing attributes in process
`syz-executor0'.
Memory state around the buggy address:
ffff8801bcae2e80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801bcae2f00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801bcae2f80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8801bcae3000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801bcae3080: fb fb fb fb fb fb fb fb 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: [PATCH v3 net-next 0/2] Add support for Microchip Technology KSZ9131 10/100/1000 Ethernet PHY
From: David Miller @ 2018-10-20 0:02 UTC (permalink / raw)
To: yuiko.oshino
Cc: robh+dt, devicetree, f.fainelli, andrew, linux-kernel,
mark.rutland, m.felsch, Markus.Niebel, netdev, UNGLinuxDriver
In-Reply-To: <1539889562-21458-1-git-send-email-yuiko.oshino@microchip.com>
From: Yuiko Oshino <yuiko.oshino@microchip.com>
Date: Thu, 18 Oct 2018 15:06:00 -0400
> This is the initial driver for Microchip KSZ9131 10/100/1000 Ethernet PHY
>
> v3:
> - KSZ9131 uses picosecond units for values of devicetree properties.
> - rewrite micrel.c and micrel-ksz90x1.txt to use the picosecond values.
> v2:
> - Creating a series from two related patches.
Series applied.
^ permalink raw reply
* Re: [PATCH 1/2] net: emac: implement 802.1Q VLAN TX tagging support
From: Christian Lamparter @ 2018-10-19 15:56 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, David S . Miller
In-Reply-To: <0b0b153c-b5fa-cd7d-62ff-3dd67eb8fe97@gmail.com>
On Wednesday, October 17, 2018 10:09:10 PM CEST Florian Fainelli wrote:
> On 10/17/2018 01:08 PM, Florian Fainelli wrote:
> > On 10/17/2018 12:53 PM, Christian Lamparter wrote:
> >> As per' APM82181 Embedded Processor User Manual 26.1 EMAC Features:
> >> VLAN:
> >> - Support for VLAN tag ID in compliance with IEEE 802.3ac.
> >> - VLAN tag insertion or replacement for transmit packets
> >>
> >> This patch completes the missing code for the VLAN tx tagging
> >> support, as the the EMAC_MR1_VLE was already enabled.
> >>
> >> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> >> ---
> >> drivers/net/ethernet/ibm/emac/core.c | 32 ++++++++++++++++++++++++----
> >> drivers/net/ethernet/ibm/emac/core.h | 6 +++++-
> >> 2 files changed, 33 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
> >> index 760b2ad8e295..be560f9031f4 100644
> >> --- a/drivers/net/ethernet/ibm/emac/core.c
> >> +++ b/drivers/net/ethernet/ibm/emac/core.c
> >> @@ -37,6 +37,7 @@
> >> #include <linux/ethtool.h>
> >> #include <linux/mii.h>
> >> #include <linux/bitops.h>
> >> +#include <linux/if_vlan.h>
> >> #include <linux/workqueue.h>
> >> #include <linux/of.h>
> >> #include <linux/of_address.h>
> >> @@ -674,7 +675,7 @@ static int emac_configure(struct emac_instance *dev)
> >> ndev->dev_addr[5]);
> >>
> >> /* VLAN Tag Protocol ID */
> >> - out_be32(&p->vtpid, 0x8100);
> >> + out_be32(&p->vtpid, ETH_P_8021Q);
> >>
> >> /* Receive mode register */
> >> r = emac_iff2rmr(ndev);
> >> @@ -1435,6 +1436,22 @@ static inline netdev_tx_t emac_xmit_finish(struct emac_instance *dev, int len)
> >> return NETDEV_TX_OK;
> >> }
> >>
> >> +static inline u16 emac_tx_vlan(struct emac_instance *dev, struct sk_buff *skb)
> >> +{
> >> + /* Handle VLAN TPID and TCI insert if this is a VLAN skb */
> >> + if (emac_has_feature(dev, EMAC_FTR_HAS_VLAN_CTAG_TX) &&
> >> + skb_vlan_tag_present(skb)) {
> >> + struct emac_regs __iomem *p = dev->emacp;
> >> +
> >> + /* update the VLAN TCI */
> >> + out_be32(&p->vtci, (u32)skb_vlan_tag_get(skb));
> >
> > The only case where this is likely not going to be 0x8100/ETH_P_8021Q is
> > if you do 802.1ad (QinQ) and you decided to somehow offload the S-Tag
> > instead of the C-Tag.
>
> Sorry, looks like I mixed up TCI and TPID here, this looks obviously
> correct ;)
Ok, I wasn't really sure what to write anyway ;).
The hardware documentation mentions that:
"Support for VLAN tag ID in compliance with IEEE Draft 802.3ac/D1.0 standard".
It's too old for offloading any fancy QinQ stuff :(.
^ permalink raw reply
* Re: [danielwa@cisco.com: Re: gianfar: Implement MAC reset and reconfig procedure]
From: Daniel Walker @ 2018-10-19 16:21 UTC (permalink / raw)
To: Claudiu Manoil
Cc: Hemant Ramdasi, netdev@vger.kernel.org,
Sunil Kumar -X (sunilk8 - MONTA VISTA SOFTWARE INC at Cisco)
In-Reply-To: <HE1PR04MB1145DAA9F214275D6489B06B96F80@HE1PR04MB1145.eurprd04.prod.outlook.com>
On Thu, Oct 18, 2018 at 04:49:26PM +0000, Claudiu Manoil wrote:
>
> I can only advise you to check whether the MACCFG2 register settings are consistent
> at this point, when ping fails. You should check the I/F Mode bits (22-23) and the
> Full Duplex bit (31), in big-endian format. If these do not match the 100Mbps full
> duplex link mode, then it might be that another thread (probably doing reset_gfar)
> changes MACCFG2 concurrently. I think MACCFG2 may be dumped with ethtool -d.
> I can get my hands on a board no sooner than maybe next week.
What does the MACCFG2 register actually do ? Is that connected to the phy
somehow ? I'm wondering because it seems like the gianfar driver is doing the
right things, and adjust_link() is getting called etc.. Something seems not to
tolerate the change from GMII to MII.
Daniel
^ permalink raw reply
* [PATCH v7 bpf-next 2/2] bpf: add tests for direct packet access from CGROUP_SKB
From: Song Liu @ 2018-10-19 16:27 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, kernel-team, edumazet, Song Liu
In-Reply-To: <20181019162748.1285423-1-songliubraving@fb.com>
Tests are added to make sure CGROUP_SKB cannot access:
tc_classid, data_meta, flow_keys
and can read and write:
mark, prority, and cb[0-4]
and can read other fields.
To make selftest with skb->sk work, a dummy sk is added in
bpf_prog_test_run_skb().
Signed-off-by: Song Liu <songliubraving@fb.com>
---
net/bpf/test_run.c | 8 +
tools/testing/selftests/bpf/test_verifier.c | 171 ++++++++++++++++++++
2 files changed, 179 insertions(+)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 0c423b8cd75c..ae2ab89a9291 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -10,6 +10,8 @@
#include <linux/etherdevice.h>
#include <linux/filter.h>
#include <linux/sched/signal.h>
+#include <net/sock.h>
+#include <net/tcp.h>
static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx,
struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
@@ -106,6 +108,8 @@ static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
return data;
}
+static struct sock test_run_sk = {0};
+
int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
union bpf_attr __user *uattr)
{
@@ -137,11 +141,15 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
break;
}
+ sock_net_set(&test_run_sk, current->nsproxy->net_ns);
+ sock_init_data(NULL, &test_run_sk);
+
skb = build_skb(data, 0);
if (!skb) {
kfree(data);
return -ENOMEM;
}
+ skb->sk = &test_run_sk;
skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
__skb_put(skb, size);
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index cf4cd32b6772..f1ae8d09770f 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -4862,6 +4862,177 @@ static struct bpf_test tests[] = {
.result = REJECT,
.flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
+ {
+ "direct packet read test#1 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+ offsetof(struct __sk_buff, data)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+ offsetof(struct __sk_buff, data_end)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, len)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, pkt_type)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, mark)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+ offsetof(struct __sk_buff, mark)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, queue_mapping)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, protocol)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_present)),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+ BPF_JMP_REG(BPF_JGT, BPF_REG_0, BPF_REG_3, 1),
+ BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "direct packet read test#2 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_tci)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_proto)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, priority)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+ offsetof(struct __sk_buff, priority)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff,
+ ingress_ifindex)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, tc_index)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, hash)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "direct packet read test#3 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[3])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[4])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, napi_id)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_4,
+ offsetof(struct __sk_buff, cb[0])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_5,
+ offsetof(struct __sk_buff, cb[1])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+ offsetof(struct __sk_buff, cb[2])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_7,
+ offsetof(struct __sk_buff, cb[3])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_8,
+ offsetof(struct __sk_buff, cb[4])),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "direct packet read test#4 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+ offsetof(struct __sk_buff, family)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip4)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip4)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[3])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[3])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_port)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, local_port)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid access of tc_classid for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, tc_classid)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid access of data_meta for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, data_meta)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid access of flow_keys for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, flow_keys)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid write access to napi_id for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, napi_id)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_9,
+ offsetof(struct __sk_buff, napi_id)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
{
"valid cgroup storage access",
.insns = {
--
2.17.1
^ permalink raw reply related
* [PATCH v7 bpf-next 1/2] bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
From: Song Liu @ 2018-10-19 16:27 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, kernel-team, edumazet, Song Liu
In-Reply-To: <20181019162748.1285423-1-songliubraving@fb.com>
BPF programs of BPF_PROG_TYPE_CGROUP_SKB need to access headers in the
skb. This patch enables direct access of skb for these programs.
Two helper functions bpf_compute_and_save_data_end() and
bpf_restore_data_end() are introduced. There are used in
__cgroup_bpf_run_filter_skb(), to compute proper data_end for the
BPF program, and restore original data afterwards.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
include/linux/filter.h | 21 +++++++++++++++++++++
kernel/bpf/cgroup.c | 6 ++++++
net/core/filter.c | 36 +++++++++++++++++++++++++++++++++++-
3 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 5771874bc01e..91b4c934f02e 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -548,6 +548,27 @@ static inline void bpf_compute_data_pointers(struct sk_buff *skb)
cb->data_end = skb->data + skb_headlen(skb);
}
+/* Similar to bpf_compute_data_pointers(), except that save orginal
+ * data in cb->data and cb->meta_data for restore.
+ */
+static inline void bpf_compute_and_save_data_end(
+ struct sk_buff *skb, void **saved_data_end)
+{
+ struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+ *saved_data_end = cb->data_end;
+ cb->data_end = skb->data + skb_headlen(skb);
+}
+
+/* Restore data saved by bpf_compute_data_pointers(). */
+static inline void bpf_restore_data_end(
+ struct sk_buff *skb, void *saved_data_end)
+{
+ struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+ cb->data_end = saved_data_end;
+}
+
static inline u8 *bpf_skb_cb(struct sk_buff *skb)
{
/* eBPF programs may read/write skb->cb[] area to transfer meta
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 00f6ed2e4f9a..9425c2fb872f 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -553,6 +553,7 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
{
unsigned int offset = skb->data - skb_network_header(skb);
struct sock *save_sk;
+ void *saved_data_end;
struct cgroup *cgrp;
int ret;
@@ -566,8 +567,13 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
save_sk = skb->sk;
skb->sk = sk;
__skb_push(skb, offset);
+
+ /* compute pointers for the bpf prog */
+ bpf_compute_and_save_data_end(skb, &saved_data_end);
+
ret = BPF_PROG_RUN_ARRAY(cgrp->bpf.effective[type], skb,
bpf_prog_run_save_cb);
+ bpf_restore_data_end(skb, saved_data_end);
__skb_pull(skb, offset);
skb->sk = save_sk;
return ret == 1 ? 0 : -EPERM;
diff --git a/net/core/filter.c b/net/core/filter.c
index 1a3ac6c46873..e3ca30bd6840 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5346,6 +5346,40 @@ static bool sk_filter_is_valid_access(int off, int size,
return bpf_skb_is_valid_access(off, size, type, prog, info);
}
+static bool cg_skb_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ const struct bpf_prog *prog,
+ struct bpf_insn_access_aux *info)
+{
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, tc_classid):
+ case bpf_ctx_range(struct __sk_buff, data_meta):
+ case bpf_ctx_range(struct __sk_buff, flow_keys):
+ return false;
+ }
+ if (type == BPF_WRITE) {
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, mark):
+ case bpf_ctx_range(struct __sk_buff, priority):
+ case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
+ break;
+ default:
+ return false;
+ }
+ }
+
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, data):
+ info->reg_type = PTR_TO_PACKET;
+ break;
+ case bpf_ctx_range(struct __sk_buff, data_end):
+ info->reg_type = PTR_TO_PACKET_END;
+ break;
+ }
+
+ return bpf_skb_is_valid_access(off, size, type, prog, info);
+}
+
static bool lwt_is_valid_access(int off, int size,
enum bpf_access_type type,
const struct bpf_prog *prog,
@@ -7038,7 +7072,7 @@ const struct bpf_prog_ops xdp_prog_ops = {
const struct bpf_verifier_ops cg_skb_verifier_ops = {
.get_func_proto = cg_skb_func_proto,
- .is_valid_access = sk_filter_is_valid_access,
+ .is_valid_access = cg_skb_is_valid_access,
.convert_ctx_access = bpf_convert_ctx_access,
};
--
2.17.1
^ permalink raw reply related
* [PATCH v7 bpf-next 0/2] bpf: add cg_skb_is_valid_access
From: Song Liu @ 2018-10-19 16:27 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, kernel-team, edumazet, Song Liu
Changes v6 -> v7:
1. Make dummy sk a global variable (test_run_sk).
Changes v5 -> v6:
1. Fixed dummy sk in bpf_prog_test_run_skb() as suggested by Eric Dumazet.
Changes v4 -> v5:
1. Replaced bpf_compute_and_save_data_pointers() with
bpf_compute_and_save_data_end();
Replaced bpf_restore_data_pointers() with bpf_restore_data_end().
2. Fixed indentation in test_verifier.c
Changes v3 -> v4:
1. Fixed crash issue reported by Alexei.
Changes v2 -> v3:
1. Added helper function bpf_compute_and_save_data_pointers() and
bpf_restore_data_pointers().
Changes v1 -> v2:
1. Updated the list of read-only fields, and read-write fields.
2. Added dummy sk to bpf_prog_test_run_skb().
This set enables BPF program of type BPF_PROG_TYPE_CGROUP_SKB to access
some __skb_buff data directly.
Song Liu (2):
bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
bpf: add tests for direct packet access from CGROUP_SKB
include/linux/filter.h | 21 +++
kernel/bpf/cgroup.c | 6 +
net/bpf/test_run.c | 8 +
net/core/filter.c | 36 ++++-
tools/testing/selftests/bpf/test_verifier.c | 171 ++++++++++++++++++++
5 files changed, 241 insertions(+), 1 deletion(-)
^ permalink raw reply
* [PATCH] can: janz-ican3: fix a missing-check bug
From: Wenwen Wang @ 2018-10-19 16:38 UTC (permalink / raw)
To: Wenwen Wang
Cc: Kangjie Lu, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, open list:CAN NETWORK DRIVERS,
open list:NETWORKING DRIVERS, open list
In ican3_old_recv_msg(), the values in the MSYNC control registers are
firstly read to 'peer' and 'locl' from the IO memory region 'mod->dpm'
through ioread8(). Then the result of the bitwise XOR of 'locl' and 'peer'
is saved to 'xord'. After that, 'xord' is checked to see whether the flag
MSYNC_RB_MASK is set. If not, an error code ENOMEM will be returned to
indicate that there is no mbox for reading. Later on, the whole message,
including the control registers, is read from 'mod->dpm' to 'msg' through
memcpy_fromio(). However, after this read, there is no re-check on the
values of the control registers. Given that the device also has the
permission to access the IO memory region, it is possible that a malicious
device controlled by an attacker modify the values in the control registers
between these two reads. By doing so, the attacker can bypass the check on
the control registers and supply unexpected values, which can cause
undefined behavior of the kernel and introduce potential security risk.
This patch rewrites the values of the control registers in 'msg' after
memcpy_fromio(), using the values acquired from ioread8(). Through this
way, the above issue can be avoided.
Signed-off-by: Wenwen Wang <wang6495@umn.edu>
---
drivers/net/can/janz-ican3.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
index 02042cb..45c6760 100644
--- a/drivers/net/can/janz-ican3.c
+++ b/drivers/net/can/janz-ican3.c
@@ -335,6 +335,8 @@ static int ican3_old_recv_msg(struct ican3_dev *mod, struct ican3_msg *msg)
mbox_page = (mbox == MSYNC_RB0) ? QUEUE_OLD_RB0 : QUEUE_OLD_RB1;
ican3_set_page(mod, mbox_page);
memcpy_fromio(msg, mod->dpm, sizeof(*msg));
+ msg->control = peer;
+ msg->spec = locl;
/*
* notify the firmware that the read buffer is available
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v7 bpf-next 2/2] bpf: add tests for direct packet access from CGROUP_SKB
From: Eric Dumazet @ 2018-10-19 16:38 UTC (permalink / raw)
To: Song Liu, netdev; +Cc: ast, daniel, kernel-team, edumazet
In-Reply-To: <20181019162748.1285423-3-songliubraving@fb.com>
On 10/19/2018 09:27 AM, Song Liu wrote:
> Tests are added to make sure CGROUP_SKB cannot access:
> tc_classid, data_meta, flow_keys
>
> and can read and write:
> mark, prority, and cb[0-4]
>
> and can read other fields.
>
> To make selftest with skb->sk work, a dummy sk is added in
> bpf_prog_test_run_skb().
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
> net/bpf/test_run.c | 8 +
> tools/testing/selftests/bpf/test_verifier.c | 171 ++++++++++++++++++++
> 2 files changed, 179 insertions(+)
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 0c423b8cd75c..ae2ab89a9291 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -10,6 +10,8 @@
> #include <linux/etherdevice.h>
> #include <linux/filter.h>
> #include <linux/sched/signal.h>
> +#include <net/sock.h>
> +#include <net/tcp.h>
>
> static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx,
> struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
> @@ -106,6 +108,8 @@ static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
> return data;
> }
>
> +static struct sock test_run_sk = {0};
No need for the {0} : bss is guaranteed to be zero.
> +
> int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> union bpf_attr __user *uattr)
> {
> @@ -137,11 +141,15 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> break;
> }
>
> + sock_net_set(&test_run_sk, current->nsproxy->net_ns);
> + sock_init_data(NULL, &test_run_sk);
> +
>
Can bpf_prog_test_run_skb() be used in parallel from different CPUS/threads ?
If yes, this looks racy, and I would suggest to use a kzalloc()ed socket just to be safe.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: Extend the sk_lookup() helper to XDP hookpoint.
From: Joe Stringer @ 2018-10-19 16:47 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Joe Stringer, daniel, Nitin Hande, netdev, ast, Jesper Brouer,
john fastabend
In-Reply-To: <20181019050620.yomnfxdide3v7k6v@kafai-mbp.dhcp.thefacebook.com>
On Thu, 18 Oct 2018 at 22:07, Martin Lau <kafai@fb.com> wrote:
>
> On Thu, Oct 18, 2018 at 04:52:40PM -0700, Joe Stringer wrote:
> > On Thu, 18 Oct 2018 at 14:20, Daniel Borkmann <daniel@iogearbox.net> wrote:
> > >
> > > On 10/18/2018 11:06 PM, Joe Stringer wrote:
> > > > On Thu, 18 Oct 2018 at 11:54, Nitin Hande <nitin.hande@gmail.com> wrote:
> > > [...]
> > > >> Open Issue
> > > >> * The underlying code relies on presence of an skb to find out the
> > > >> right sk for the case of REUSEPORT socket option. Since there is
> > > >> no skb available at XDP hookpoint, the helper function will return
> > > >> the first available sk based off the 5 tuple hash. If the desire
> > > >> is to return a particular sk matching reuseport_cb function, please
> > > >> suggest way to tackle it, which can be addressed in a future commit.
> > >
> > > >> Signed-off-by: Nitin Hande <Nitin.Hande@gmail.com>
> > > >
> > > > Thanks Nitin, LGTM overall.
> > > >
> > > > The REUSEPORT thing suggests that the usage of this helper from XDP
> > > > layer may lead to a different socket being selected vs. the equivalent
> > > > call at TC hook, or other places where the selection may occur. This
> > > > could be a bit counter-intuitive.
> > > >
> > > > One thought I had to work around this was to introduce a flag,
> > > > something like BPF_F_FIND_REUSEPORT_SK_BY_HASH. This flag would
> > > > effectively communicate in the API that the bpf_sk_lookup_xxx()
> > > > functions will only select a REUSEPORT socket based on the hash and
> > > > not by, for example BPF_PROG_TYPE_SK_REUSEPORT programs. The absence
> > > > of the flag would support finding REUSEPORT sockets by other
> > > > mechanisms (which would be allowed for now from TC hooks but would be
> > > > disallowed from XDP, since there's no specific plan to support this).
> > >
> > > Hmm, given skb is NULL here the only way to lookup the socket in such
> > > scenario is based on hash, that is, inet_ehashfn() / inet6_ehashfn(),
> > > perhaps alternative is to pass this hash in from XDP itself to the
> > > helper so it could be custom selector. Do you have a specific use case
> > > on this for XDP (just curious)?
> >
> > I don't have a use case for SO_REUSEPORT introspection from XDP, so
> > I'm primarily thinking from the perspective of making the behaviour
> > clear in the API in a way that leaves open the possibility for a
> > reasonable implementation in future. From that perspective, my main
> > concern is that it may surprise some BPF writers that the same
> > "bpf_sk_lookup_tcp()" call (with identical parameters) may have
> > different behaviour at TC vs. XDP layers, as the BPF selection of
> > sockets is respected at TC but not at XDP.
> >
> > FWIW we're already out of parameters for the actual call, so if we
> > wanted to allow passing a hash in, we'd need to either dedicate half
> > the 'flags' field for this configurable hash, or consider adding the
> > new hash parameter to 'struct bpf_sock_tuple'.
> >
> > +Martin for any thoughts on SO_REUSEPORT and XDP here.
> The XDP/TC prog has read access to the sk fields through
> 'struct bpf_sock'?
>
> A quick thought...
> Considering all sk in the same reuse->socks[] share
> many things (e.g. family,type,protocol,ip,port..etc are the same),
> I wonder returning which particular sk from reuse->socks[] will
> matter too much since most of the fields from 'struct bpf_sock' will
> be the same. Some of fields in 'struct bpf_sock' could be different
> though, like priority? Hence, another possibility is to limit the
> accessible fields for the XDP prog. Only allow accessing the fields
> that must be the same among the sk in the same reuse->socks[].
This sounds pretty reasonable to me.
^ permalink raw reply
* RE: Improving accuracy of PHC readings
From: Keller, Jacob E @ 2018-10-19 16:52 UTC (permalink / raw)
To: Miroslav Lichvar, netdev@vger.kernel.org; +Cc: Richard Cochran
In-Reply-To: <20181019095137.GG4407@localhost>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of Miroslav Lichvar
> Sent: Friday, October 19, 2018 2:52 AM
> To: netdev@vger.kernel.org
> Cc: Richard Cochran <richardcochran@gmail.com>; Keller, Jacob E
> <jacob.e.keller@intel.com>
> Subject: Improving accuracy of PHC readings
>
> I think there might be a way how we could significantly improve
> accuracy of synchronization between the system clock and a PTP
> hardware clock, at least with some network drivers.
>
> Currently, the PTP_SYS_OFFSET ioctl reads the system clock, reads the
> PHC using the gettime64 function of the driver, and reads the system
> clock again. The ioctl can repeat this to provide multiple readings to
> the user space.
>
> phc2sys (or another program synchronizing the system clock to the PHC)
> assumes the PHC timestamps were captured in the middle between the two
> closest system clock timestamps.
>
> The trouble is that gettime64 typically reads multiple (2-3) registers
> and the timestamp is latched on the first one, so the assumption about
> middle point is wrong. There is an asymmetry, even if the delays on
> the PCIe bus are perfectly symmetric.
>
Right! I feel like this is obvious now that you said it, so I'm surprised no one thought of it before...
> A solution to this would be a new driver function that wraps the
> latching register read with readings of the system clock and return
> three timestamps instead of one. For example:
>
> ktime_get_real_ts64(&sys_ts1);
> IXGBE_READ_REG(hw, IXGBE_SYSTIMR);
> ktime_get_real_ts64(&sys_ts2);
> phc_ts.tv_nsec = IXGBE_READ_REG(hw, IXGBE_SYSTIML);
> phc_ts.tv_sec = IXGBE_READ_REG(hw, IXGBE_SYSTIMH);
>
> The extra timestamp doesn't fit the API of the PTP_SYS_OFFSET ioctl,
> so it would need to shift the timestamp it returns by the missing
> intervals (assuming the frequency offset between the PHC and system
> clock is small), or a new ioctl could be introduced that would return
> all timestamps in an array looking like this:
>
> [sys, phc, sys, sys, phc, sys, ...]
>
I think the new ioctl is probably the better solution.
> This should significantly improve the accuracy of the synchronization,
> reduce the uncertainty in the readings to less than a half or third,
> and also reduce the jitter as there are fewer register reads sensitive
> to the PCIe delay.
>
> What do you think?
>
Nice! I think this is good. I'd love to see some data to back it up, but it makes sense to me.
Thanks,
Jake
> --
> Miroslav Lichvar
^ permalink raw reply
* [PATCH v8 bpf-next 2/2] bpf: add tests for direct packet access from CGROUP_SKB
From: Song Liu @ 2018-10-19 16:57 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, kernel-team, edumazet, Song Liu
In-Reply-To: <20181019165758.1410213-1-songliubraving@fb.com>
Tests are added to make sure CGROUP_SKB cannot access:
tc_classid, data_meta, flow_keys
and can read and write:
mark, prority, and cb[0-4]
and can read other fields.
To make selftest with skb->sk work, a dummy sk is added in
bpf_prog_test_run_skb().
Signed-off-by: Song Liu <songliubraving@fb.com>
---
net/bpf/test_run.c | 15 ++
tools/testing/selftests/bpf/test_verifier.c | 171 ++++++++++++++++++++
2 files changed, 186 insertions(+)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 0c423b8cd75c..c89c22c49015 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -10,6 +10,8 @@
#include <linux/etherdevice.h>
#include <linux/filter.h>
#include <linux/sched/signal.h>
+#include <net/sock.h>
+#include <net/tcp.h>
static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx,
struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
@@ -115,6 +117,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
u32 retval, duration;
int hh_len = ETH_HLEN;
struct sk_buff *skb;
+ struct sock *sk;
void *data;
int ret;
@@ -137,11 +140,21 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
break;
}
+ sk = kzalloc(sizeof(struct sock), GFP_USER);
+ if (!sk) {
+ kfree(data);
+ return -ENOMEM;
+ }
+ sock_net_set(sk, current->nsproxy->net_ns);
+ sock_init_data(NULL, sk);
+
skb = build_skb(data, 0);
if (!skb) {
kfree(data);
+ kfree(sk);
return -ENOMEM;
}
+ skb->sk = sk;
skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
__skb_put(skb, size);
@@ -159,6 +172,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
if (pskb_expand_head(skb, nhead, 0, GFP_USER)) {
kfree_skb(skb);
+ kfree(sk);
return -ENOMEM;
}
}
@@ -171,6 +185,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
size = skb_headlen(skb);
ret = bpf_test_finish(kattr, uattr, skb->data, size, retval, duration);
kfree_skb(skb);
+ kfree(sk);
return ret;
}
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index cf4cd32b6772..f1ae8d09770f 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -4862,6 +4862,177 @@ static struct bpf_test tests[] = {
.result = REJECT,
.flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
+ {
+ "direct packet read test#1 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+ offsetof(struct __sk_buff, data)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+ offsetof(struct __sk_buff, data_end)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, len)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, pkt_type)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, mark)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+ offsetof(struct __sk_buff, mark)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, queue_mapping)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, protocol)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_present)),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+ BPF_JMP_REG(BPF_JGT, BPF_REG_0, BPF_REG_3, 1),
+ BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "direct packet read test#2 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_tci)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_proto)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, priority)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+ offsetof(struct __sk_buff, priority)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff,
+ ingress_ifindex)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, tc_index)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, hash)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "direct packet read test#3 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[3])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[4])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, napi_id)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_4,
+ offsetof(struct __sk_buff, cb[0])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_5,
+ offsetof(struct __sk_buff, cb[1])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+ offsetof(struct __sk_buff, cb[2])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_7,
+ offsetof(struct __sk_buff, cb[3])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_8,
+ offsetof(struct __sk_buff, cb[4])),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "direct packet read test#4 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+ offsetof(struct __sk_buff, family)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip4)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip4)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[3])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[3])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_port)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, local_port)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid access of tc_classid for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, tc_classid)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid access of data_meta for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, data_meta)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid access of flow_keys for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, flow_keys)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid write access to napi_id for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, napi_id)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_9,
+ offsetof(struct __sk_buff, napi_id)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
{
"valid cgroup storage access",
.insns = {
--
2.17.1
^ permalink raw reply related
* [PATCH v8 bpf-next 0/2] bpf: add cg_skb_is_valid_access
From: Song Liu @ 2018-10-19 16:57 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, kernel-team, edumazet, Song Liu
Changes v7 -> v8:
1. Dynamically allocate the dummy sk to avoid race conditions.
Changes v6 -> v7:
1. Make dummy sk a global variable (test_run_sk).
Changes v5 -> v6:
1. Fixed dummy sk in bpf_prog_test_run_skb() as suggested by Eric Dumazet.
Changes v4 -> v5:
1. Replaced bpf_compute_and_save_data_pointers() with
bpf_compute_and_save_data_end();
Replaced bpf_restore_data_pointers() with bpf_restore_data_end().
2. Fixed indentation in test_verifier.c
Changes v3 -> v4:
1. Fixed crash issue reported by Alexei.
Changes v2 -> v3:
1. Added helper function bpf_compute_and_save_data_pointers() and
bpf_restore_data_pointers().
Changes v1 -> v2:
1. Updated the list of read-only fields, and read-write fields.
2. Added dummy sk to bpf_prog_test_run_skb().
This set enables BPF program of type BPF_PROG_TYPE_CGROUP_SKB to access
some __skb_buff data directly.
Song Liu (2):
bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
bpf: add tests for direct packet access from CGROUP_SKB
include/linux/filter.h | 21 +++
kernel/bpf/cgroup.c | 6 +
net/bpf/test_run.c | 15 ++
net/core/filter.c | 36 ++++-
tools/testing/selftests/bpf/test_verifier.c | 171 ++++++++++++++++++++
5 files changed, 248 insertions(+), 1 deletion(-)
^ permalink raw reply
* [PATCH net] net/ipv6: Fix index counter for unicast addresses in in6_dump_addrs
From: David Ahern @ 2018-10-19 17:00 UTC (permalink / raw)
To: netdev; +Cc: davem, stephen, David Ahern
From: David Ahern <dsahern@gmail.com>
The loop wants to skip previously dumped addresses, so loops until
current index >= saved index. If the message fills it wants to save
the index for the next address to dump - ie., the one that did not
fit in the current message.
Currently, it is incrementing the index counter before comparing to the
saved index, and then the saved index is off by 1 - it assumes the
current address is going to fit in the message.
Change the index handling to increment only after a succesful dump.
Fixes: 502a2ffd7376a ("ipv6: convert idev_list to list macros")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/addrconf.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c63ccce6425f..4e81ff2f4588 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4928,8 +4928,8 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
/* unicast address incl. temp addr */
list_for_each_entry(ifa, &idev->addr_list, if_list) {
- if (++ip_idx < s_ip_idx)
- continue;
+ if (ip_idx < s_ip_idx)
+ goto next;
err = inet6_fill_ifaddr(skb, ifa,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
@@ -4938,6 +4938,8 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
if (err < 0)
break;
nl_dump_check_consistent(cb, nlmsg_hdr(skb));
+next:
+ ip_idx++;
}
break;
}
--
2.11.0
^ permalink raw reply related
* [PATCH v8 bpf-next 1/2] bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
From: Song Liu @ 2018-10-19 16:57 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, kernel-team, edumazet, Song Liu
In-Reply-To: <20181019165758.1410213-1-songliubraving@fb.com>
BPF programs of BPF_PROG_TYPE_CGROUP_SKB need to access headers in the
skb. This patch enables direct access of skb for these programs.
Two helper functions bpf_compute_and_save_data_end() and
bpf_restore_data_end() are introduced. There are used in
__cgroup_bpf_run_filter_skb(), to compute proper data_end for the
BPF program, and restore original data afterwards.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
include/linux/filter.h | 21 +++++++++++++++++++++
kernel/bpf/cgroup.c | 6 ++++++
net/core/filter.c | 36 +++++++++++++++++++++++++++++++++++-
3 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 5771874bc01e..91b4c934f02e 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -548,6 +548,27 @@ static inline void bpf_compute_data_pointers(struct sk_buff *skb)
cb->data_end = skb->data + skb_headlen(skb);
}
+/* Similar to bpf_compute_data_pointers(), except that save orginal
+ * data in cb->data and cb->meta_data for restore.
+ */
+static inline void bpf_compute_and_save_data_end(
+ struct sk_buff *skb, void **saved_data_end)
+{
+ struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+ *saved_data_end = cb->data_end;
+ cb->data_end = skb->data + skb_headlen(skb);
+}
+
+/* Restore data saved by bpf_compute_data_pointers(). */
+static inline void bpf_restore_data_end(
+ struct sk_buff *skb, void *saved_data_end)
+{
+ struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+ cb->data_end = saved_data_end;
+}
+
static inline u8 *bpf_skb_cb(struct sk_buff *skb)
{
/* eBPF programs may read/write skb->cb[] area to transfer meta
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 00f6ed2e4f9a..9425c2fb872f 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -553,6 +553,7 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
{
unsigned int offset = skb->data - skb_network_header(skb);
struct sock *save_sk;
+ void *saved_data_end;
struct cgroup *cgrp;
int ret;
@@ -566,8 +567,13 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
save_sk = skb->sk;
skb->sk = sk;
__skb_push(skb, offset);
+
+ /* compute pointers for the bpf prog */
+ bpf_compute_and_save_data_end(skb, &saved_data_end);
+
ret = BPF_PROG_RUN_ARRAY(cgrp->bpf.effective[type], skb,
bpf_prog_run_save_cb);
+ bpf_restore_data_end(skb, saved_data_end);
__skb_pull(skb, offset);
skb->sk = save_sk;
return ret == 1 ? 0 : -EPERM;
diff --git a/net/core/filter.c b/net/core/filter.c
index 1a3ac6c46873..e3ca30bd6840 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5346,6 +5346,40 @@ static bool sk_filter_is_valid_access(int off, int size,
return bpf_skb_is_valid_access(off, size, type, prog, info);
}
+static bool cg_skb_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ const struct bpf_prog *prog,
+ struct bpf_insn_access_aux *info)
+{
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, tc_classid):
+ case bpf_ctx_range(struct __sk_buff, data_meta):
+ case bpf_ctx_range(struct __sk_buff, flow_keys):
+ return false;
+ }
+ if (type == BPF_WRITE) {
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, mark):
+ case bpf_ctx_range(struct __sk_buff, priority):
+ case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
+ break;
+ default:
+ return false;
+ }
+ }
+
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, data):
+ info->reg_type = PTR_TO_PACKET;
+ break;
+ case bpf_ctx_range(struct __sk_buff, data_end):
+ info->reg_type = PTR_TO_PACKET_END;
+ break;
+ }
+
+ return bpf_skb_is_valid_access(off, size, type, prog, info);
+}
+
static bool lwt_is_valid_access(int off, int size,
enum bpf_access_type type,
const struct bpf_prog *prog,
@@ -7038,7 +7072,7 @@ const struct bpf_prog_ops xdp_prog_ops = {
const struct bpf_verifier_ops cg_skb_verifier_ops = {
.get_func_proto = cg_skb_func_proto,
- .is_valid_access = sk_filter_is_valid_access,
+ .is_valid_access = cg_skb_is_valid_access,
.convert_ctx_access = bpf_convert_ctx_access,
};
--
2.17.1
^ permalink raw reply related
* Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Edward Cree @ 2018-10-19 17:04 UTC (permalink / raw)
To: Martin Lau
Cc: Yonghong Song, Alexei Starovoitov, daniel@iogearbox.net,
netdev@vger.kernel.org, Kernel Team
In-Reply-To: <20181018211954.egdolymm3zqgphmd@kafai-mbp.dhcp.thefacebook.com>
On 18/10/18 22:19, Martin Lau wrote:
> As I have mentioned earlier, it is also special to
> the kernel because the BTF verifier and bpf_prog_load()
> need to do different checks for FUNC and FUNC_PROTO to
> ensure they are sane.
>
> First, we need to agree that the kernel needs to verify
> them differently.
>
> and then we can proceed to discuss how to distinguish them.
> We picked the current way to avoid adding a
> new BTF function section and keep it
> strict forward to distinguish them w/o relying
> on other hints from 'struct btf_type'.
>
> Are you suggesting another way of doing it?
But you *do* have such a new section.
The patch comment talks about a 'FuncInfo Table' which appears to
map (section, insn_idx) to type_id. (I think this gets added in
.BTF.ext per patch 9?) So when you're looking at a FUNC type
because you looked up a type_id from that table, you know it's
the signature of a subprogram, and you're checking it as such.
Whereas, if you were doing something with some other type and it
referenced a FUNC type (e.g., in the patch comment's example,
you're checking foo's first argument against the type bar) in
its type_id, you know you're using it as a formal type (a FUNC_
PROTO in your parlance) and not as a subprogram.
The context in which you are using a type entry tells you which
kind it is. And the verifier can and should be smart enough to
know what it's doing in this way.
And it's entirely reasonable for the same type entry to get used
for both those cases; in my example, you'd have a FUNC type for
int foo(int), referenced both by the func_info entry for foo
and by the PTR type for bar. And if you had another subprogram
int baz(int), its func_info entry could reference the same
type_id, because the (reference to the) name of the function
should live in the func_info, not in the type.
What you are proposing seems to be saying "if we have this
particular special btf_kind, then this BTF entry doesn't just
define a type, it declares a subprogram of that type". Oh,
and with the name of the type as the subprogram name. Which
just creates blurry confusion as to whether BTF entries define
types or declare objects; IMNSHO the correct approach is for
objects to be declared elsewhere and to reference BTF types by
their type_id.
Which is what the func_info table in patch 9 appears to do.
(It also rather bothers me the way we are using special type
names to associate maps with their k/v types, rather than
extending the records in the maps section to include type_ids
referencing them. It's the same kind of weird implicitness,
and if I'd spotted it when it was going in I'd've nacked it,
but I suppose it's ABI now and too late to change.)
-Ed
^ permalink raw reply
* Re: [PATCH net-next] rocker: Drop pointless static qualifier
From: David Miller @ 2018-10-19 17:42 UTC (permalink / raw)
To: yuehaibing; +Cc: jiri, netdev, kernel-janitors
In-Reply-To: <1539950579-182423-1-git-send-email-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Fri, 19 Oct 2018 12:02:59 +0000
> There is no need to have the 'struct rocker_desc_info *desc_info'
> variable static since new value always be assigned before use it.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thank you.
^ permalink raw reply
* (unknown)
From: David Miller @ 2018-10-19 17:46 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs
In-Reply-To: <11408.1539960053@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Fri, 19 Oct 2018 15:40:53 +0100
> Is there going to be a merge of net into net-next before the merge
> window opens? Or do you have a sample merge that I can rebase my
> afs-next branch on?
I'll be doing a net to net-next merge some time today.
^ permalink raw reply
* [net 1/1] tipc: eliminate message disordering during binding table update
From: Jon Maloy @ 2018-10-19 17:55 UTC (permalink / raw)
To: davem, netdev
Cc: gordan.mihaljevic, tung.q.nguyen, hoang.h.le, jon.maloy,
canh.d.luu, ying.xue, tipc-discussion
We have seen the following race scenario:
1) named_distribute() builds a "bulk" message, containing a PUBLISH
item for a certain publication. This is based on the contents of
the binding tables's 'cluster_scope' list.
2) tipc_named_withdraw() removes the same publication from the list,
bulds a WITHDRAW message and distributes it to all cluster nodes.
3) tipc_named_node_up(), which was calling named_distribute(), sends
out the bulk message built under 1)
4) The WITHDRAW message arrives at the just detected node, finds
no corresponding publication, and is dropped.
5) The PUBLISH item arrives at the same node, is added to its binding
table, and remains there forever.
This arrival disordering was earlier taken care of by the backlog queue,
originally added for a different purpose, which was removed in the
commit referred to below, but we now need a different solution.
In this commit, we replace the rcu lock protecting the 'cluster_scope'
list with a regular RW lock which comprises even the sending of the
bulk message. This both guarantees both the list integrity and the
message sending order. We will later add a commit which cleans up
this code further.
Note that this commit needs recently added commit d3092b2efca1 ("tipc:
fix unsafe rcu locking when accessing publication list") to apply
cleanly.
Fixes: 37922ea4a310 ("tipc: permit overlapping service ranges in name table")
Reported-by: Tuong Lien Tong <tuong.t.lien@dektech.com.au>
Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/name_distr.c | 18 ++++++++++--------
net/tipc/name_table.c | 1 +
net/tipc/name_table.h | 1 +
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 3cfeb9d..61219f0 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -94,8 +94,9 @@ struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ)
list_add_tail_rcu(&publ->binding_node, &nt->node_scope);
return NULL;
}
- list_add_tail_rcu(&publ->binding_node, &nt->cluster_scope);
-
+ write_lock_bh(&nt->cluster_scope_lock);
+ list_add_tail(&publ->binding_node, &nt->cluster_scope);
+ write_unlock_bh(&nt->cluster_scope_lock);
skb = named_prepare_buf(net, PUBLICATION, ITEM_SIZE, 0);
if (!skb) {
pr_warn("Publication distribution failure\n");
@@ -112,11 +113,13 @@ struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ)
*/
struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ)
{
+ struct name_table *nt = tipc_name_table(net);
struct sk_buff *buf;
struct distr_item *item;
- list_del_rcu(&publ->binding_node);
-
+ write_lock_bh(&nt->cluster_scope_lock);
+ list_del(&publ->binding_node);
+ write_unlock_bh(&nt->cluster_scope_lock);
if (publ->scope == TIPC_NODE_SCOPE)
return NULL;
@@ -147,7 +150,7 @@ static void named_distribute(struct net *net, struct sk_buff_head *list,
ITEM_SIZE) * ITEM_SIZE;
u32 msg_rem = msg_dsz;
- list_for_each_entry_rcu(publ, pls, binding_node) {
+ list_for_each_entry(publ, pls, binding_node) {
/* Prepare next buffer: */
if (!skb) {
skb = named_prepare_buf(net, PUBLICATION, msg_rem,
@@ -189,11 +192,10 @@ void tipc_named_node_up(struct net *net, u32 dnode)
__skb_queue_head_init(&head);
- rcu_read_lock();
+ read_lock_bh(&nt->cluster_scope_lock);
named_distribute(net, &head, dnode, &nt->cluster_scope);
- rcu_read_unlock();
-
tipc_node_xmit(net, &head, dnode, 0);
+ read_unlock_bh(&nt->cluster_scope_lock);
}
/**
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 66d5b2c..bff241f 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -744,6 +744,7 @@ int tipc_nametbl_init(struct net *net)
INIT_LIST_HEAD(&nt->node_scope);
INIT_LIST_HEAD(&nt->cluster_scope);
+ rwlock_init(&nt->cluster_scope_lock);
tn->nametbl = nt;
spin_lock_init(&tn->nametbl_lock);
return 0;
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 892bd75..f790663 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -100,6 +100,7 @@ struct name_table {
struct hlist_head services[TIPC_NAMETBL_SIZE];
struct list_head node_scope;
struct list_head cluster_scope;
+ rwlock_t cluster_scope_lock;
u32 local_publ_count;
};
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v2 net-next 0/8] net: dsa: microchip: Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers
From: Florian Fainelli @ 2018-10-19 17:56 UTC (permalink / raw)
To: Tristram.Ha, marek.vasut
Cc: arkadis, UNGLinuxDriver, netdev, andrew, pavel, ruediger.schmitt
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD411AC863@CHN-SV-EXMX02.mchp-main.com>
On 08/16/2018 02:34 PM, Tristram.Ha@microchip.com wrote:
>> -----Original Message-----
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Sent: Wednesday, August 15, 2018 5:29 PM
>> To: Tristram Ha - C24268 <Tristram.Ha@microchip.com>; Andrew Lunn
>> <andrew@lunn.ch>; Pavel Machek <pavel@ucw.cz>; Ruediger Schmitt
>> <ruediger.schmitt@philips.com>
>> Cc: Arkadi Sharshevsky <arkadis@mellanox.com>; UNGLinuxDriver
>> <UNGLinuxDriver@microchip.com>; netdev@vger.kernel.org
>> Subject: Re: [PATCH v2 net-next 0/8] net: dsa: microchip: Modify KSZ9477
>> DSA driver in preparation to add other KSZ switch drivers
>>
>> On 12/05/2017 05:46 PM, Tristram.Ha@microchip.com wrote:
>>> From: Tristram Ha <Tristram.Ha@microchip.com>
>>>
>>> This series of patches is to modify the original KSZ9477 DSA driver so
>>> that other KSZ switch drivers can be added and use the common code.
>>>
>>> There are several steps to accomplish this achievement. First is to
>>> rename some function names with a prefix to indicate chip specific
>>> function. Second is to move common code into header that can be shared.
>>> Last is to modify tag_ksz.c so that it can handle many tail tag formats
>>> used by different KSZ switch drivers.
>>>
>>> ksz_common.c will contain the common code used by all KSZ switch drivers.
>>> ksz9477.c will contain KSZ9477 code from the original ksz_common.c.
>>> ksz9477_spi.c is renamed from ksz_spi.c.
>>> ksz9477_reg.h is renamed from ksz_9477_reg.h.
>>> ksz_common.h is added to provide common code access to KSZ switch
>>> drivers.
>>> ksz_spi.h is added to provide common SPI access functions to KSZ SPI
>>> drivers.
>>
>> Is something gating this series from getting included? It's been nearly
>> 8 months now and this has not been include nor resubmitted, any plans to
>> rebase that patch series and work towards inclusion in net-next when it
>> opens back again?
>>
>> Thank you!
>
> Sorry for the long delay. I will restart my kernel submission effort next month
> after finishing the work on current development project.
>
Tristram, any chance of resubmitting this or should someone with access
to those switches take up your series and submit it?
--
Florian
^ permalink raw reply
* [PATCH][next] igc: fix error return handling from call to netif_set_real_num_tx_queues
From: Colin King @ 2018-10-19 18:16 UTC (permalink / raw)
To: Sasha Neftin, Jeff Kirsher, David S . Miller, intel-wired-lan
Cc: kernel-janitors, netdev
From: Colin Ian King <colin.king@canonical.com>
The call to netif_set_real_num_tx_queues is not assigning the error
return to variable err even though the next line checks err for an
error. Fix this by adding the missing err assignment.
Detected by CoverityScan, CID#1474551 ("Logically dead code")
Fixes: 3df25e4c1e66 ("igc: Add interrupt support")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 9d85707e8a81..80ddbd987764 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -3358,7 +3358,7 @@ static int __igc_open(struct net_device *netdev, bool resuming)
goto err_req_irq;
/* Notify the stack of the actual queue counts. */
- netif_set_real_num_tx_queues(netdev, adapter->num_tx_queues);
+ err = netif_set_real_num_tx_queues(netdev, adapter->num_tx_queues);
if (err)
goto err_set_queues;
--
2.19.1
^ permalink raw reply related
* Re: C45 Phys and PHY_FORCING state
From: Florian Fainelli @ 2018-10-19 18:43 UTC (permalink / raw)
To: Jose Abreu, Andrew Lunn
Cc: David S. Miller, netdev@vger.kernel.org, Joao Pinto
In-Reply-To: <cd0c9fe6-8b13-041b-1f80-caf3c6250df5@synopsys.com>
On 10/19/2018 05:02 AM, Jose Abreu wrote:
> Hello Andrew and Florian,
>
> Currently I have a 10G C45 phy that is fixed at 10G link. This
> version does not support auto negotiation so I'm turning off the
> feature in phydev struct field. I found out that when I do this
> phylib is not composing C45 frames and is instead using C22. This
> is due to call to genphy_udpate_link() which doesn't work on my
> phy because it doesn't support C22.
>
> If I apply attached patch then things work perfectly fine. Can
> you please review it ?
Looks reasonable, I could not find other functions in the state machine
that were not already abstracting the clause type, or letting a driver
callback be called. Can you submit this as a formal patch against
net-next (and not attached, but inline)?
I would suggest creating a helper, e.g: phy_update_link() that way
everything is well namespaced and clear within the state machine itself.
--
Florian
^ permalink raw reply
* [PATCH net-next 7/7] net: hns3: Add enable and process hw errors of TM scheduler
From: Salil Mehta @ 2018-10-19 19:15 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linuxarm, Shiju Jose
In-Reply-To: <20181019191532.10088-1-salil.mehta@huawei.com>
From: Shiju Jose <shiju.jose@huawei.com>
This patch enables and process hw errors of TM scheduler and
QCN(Quantized Congestion Control).
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h | 8 +
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c | 286 +++++++++++++++++++++
.../net/ethernet/hisilicon/hns3/hns3pf/hclge_err.h | 3 +
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 6 +
4 files changed, 303 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index f23cf78..872cd4b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -211,6 +211,12 @@ enum hclge_opcode_type {
HCLGE_OPC_LED_STATUS_CFG = 0xB000,
/* Error INT commands */
+ HCLGE_TM_SCH_ECC_INT_EN = 0x0829,
+ HCLGE_TM_SCH_ECC_ERR_RINT_CMD = 0x082d,
+ HCLGE_TM_SCH_ECC_ERR_RINT_CE = 0x082f,
+ HCLGE_TM_SCH_ECC_ERR_RINT_NFE = 0x0830,
+ HCLGE_TM_SCH_ECC_ERR_RINT_FE = 0x0831,
+ HCLGE_TM_SCH_MBIT_ECC_INFO_CMD = 0x0833,
HCLGE_COMMON_ECC_INT_CFG = 0x1505,
HCLGE_IGU_EGU_TNL_INT_QUERY = 0x1802,
HCLGE_IGU_EGU_TNL_INT_EN = 0x1803,
@@ -218,6 +224,8 @@ enum hclge_opcode_type {
HCLGE_IGU_COMMON_INT_QUERY = 0x1805,
HCLGE_IGU_COMMON_INT_EN = 0x1806,
HCLGE_IGU_COMMON_INT_CLR = 0x1807,
+ HCLGE_TM_QCN_MEM_INT_CFG = 0x1A14,
+ HCLGE_TM_QCN_MEM_INT_INFO_CMD = 0x1A17,
HCLGE_PPP_CMD0_INT_CMD = 0x2100,
HCLGE_PPP_CMD1_INT_CMD = 0x2101,
HCLGE_NCSI_INT_QUERY = 0x2400,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
index ea73def..f7e363b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
@@ -213,6 +213,129 @@ static const struct hclge_hw_error hclge_ppp_mpf_int3[] = {
{ /* sentinel */ }
};
+struct hclge_tm_sch_ecc_info {
+ const char *name;
+};
+
+static const struct hclge_tm_sch_ecc_info hclge_tm_sch_ecc_err[7][15] = {
+ {
+ { .name = "QSET_QUEUE_CTRL:PRI_LEN TAB" },
+ { .name = "QSET_QUEUE_CTRL:SPA_LEN TAB" },
+ { .name = "QSET_QUEUE_CTRL:SPB_LEN TAB" },
+ { .name = "QSET_QUEUE_CTRL:WRRA_LEN TAB" },
+ { .name = "QSET_QUEUE_CTRL:WRRB_LEN TAB" },
+ { .name = "QSET_QUEUE_CTRL:SPA_HPTR TAB" },
+ { .name = "QSET_QUEUE_CTRL:SPB_HPTR TAB" },
+ { .name = "QSET_QUEUE_CTRL:WRRA_HPTR TAB" },
+ { .name = "QSET_QUEUE_CTRL:WRRB_HPTR TAB" },
+ { .name = "QSET_QUEUE_CTRL:QS_LINKLIST TAB" },
+ { .name = "QSET_QUEUE_CTRL:SPA_TPTR TAB" },
+ { .name = "QSET_QUEUE_CTRL:SPB_TPTR TAB" },
+ { .name = "QSET_QUEUE_CTRL:WRRA_TPTR TAB" },
+ { .name = "QSET_QUEUE_CTRL:WRRB_TPTR TAB" },
+ { .name = "QSET_QUEUE_CTRL:QS_DEFICITCNT TAB" },
+ },
+ {
+ { .name = "ROCE_QUEUE_CTRL:QS_LEN TAB" },
+ { .name = "ROCE_QUEUE_CTRL:QS_TPTR TAB" },
+ { .name = "ROCE_QUEUE_CTRL:QS_HPTR TAB" },
+ { .name = "ROCE_QUEUE_CTRL:QLINKLIST TAB" },
+ { .name = "ROCE_QUEUE_CTRL:QCLEN TAB" },
+ },
+ {
+ { .name = "NIC_QUEUE_CTRL:QS_LEN TAB" },
+ { .name = "NIC_QUEUE_CTRL:QS_TPTR TAB" },
+ { .name = "NIC_QUEUE_CTRL:QS_HPTR TAB" },
+ { .name = "NIC_QUEUE_CTRL:QLINKLIST TAB" },
+ { .name = "NIC_QUEUE_CTRL:QCLEN TAB" },
+ },
+ {
+ { .name = "RAM_CFG_CTRL:CSHAP TAB" },
+ { .name = "RAM_CFG_CTRL:PSHAP TAB" },
+ },
+ {
+ { .name = "SHAPER_CTRL:PSHAP TAB" },
+ },
+ {
+ { .name = "MSCH_CTRL" },
+ },
+ {
+ { .name = "TOP_CTRL" },
+ },
+};
+
+static const struct hclge_hw_error hclge_tm_sch_err_int[] = {
+ { .int_msk = BIT(0), .msg = "tm_sch_ecc_1bit_err" },
+ { .int_msk = BIT(1), .msg = "tm_sch_ecc_mbit_err" },
+ { .int_msk = BIT(2), .msg = "tm_sch_port_shap_sub_fifo_wr_full_err" },
+ { .int_msk = BIT(3), .msg = "tm_sch_port_shap_sub_fifo_rd_empty_err" },
+ { .int_msk = BIT(4), .msg = "tm_sch_pg_pshap_sub_fifo_wr_full_err" },
+ { .int_msk = BIT(5), .msg = "tm_sch_pg_pshap_sub_fifo_rd_empty_err" },
+ { .int_msk = BIT(6), .msg = "tm_sch_pg_cshap_sub_fifo_wr_full_err" },
+ { .int_msk = BIT(7), .msg = "tm_sch_pg_cshap_sub_fifo_rd_empty_err" },
+ { .int_msk = BIT(8), .msg = "tm_sch_pri_pshap_sub_fifo_wr_full_err" },
+ { .int_msk = BIT(9), .msg = "tm_sch_pri_pshap_sub_fifo_rd_empty_err" },
+ { .int_msk = BIT(10), .msg = "tm_sch_pri_cshap_sub_fifo_wr_full_err" },
+ { .int_msk = BIT(11), .msg = "tm_sch_pri_cshap_sub_fifo_rd_empty_err" },
+ { .int_msk = BIT(12),
+ .msg = "tm_sch_port_shap_offset_fifo_wr_full_err" },
+ { .int_msk = BIT(13),
+ .msg = "tm_sch_port_shap_offset_fifo_rd_empty_err" },
+ { .int_msk = BIT(14),
+ .msg = "tm_sch_pg_pshap_offset_fifo_wr_full_err" },
+ { .int_msk = BIT(15),
+ .msg = "tm_sch_pg_pshap_offset_fifo_rd_empty_err" },
+ { .int_msk = BIT(16),
+ .msg = "tm_sch_pg_cshap_offset_fifo_wr_full_err" },
+ { .int_msk = BIT(17),
+ .msg = "tm_sch_pg_cshap_offset_fifo_rd_empty_err" },
+ { .int_msk = BIT(18),
+ .msg = "tm_sch_pri_pshap_offset_fifo_wr_full_err" },
+ { .int_msk = BIT(19),
+ .msg = "tm_sch_pri_pshap_offset_fifo_rd_empty_err" },
+ { .int_msk = BIT(20),
+ .msg = "tm_sch_pri_cshap_offset_fifo_wr_full_err" },
+ { .int_msk = BIT(21),
+ .msg = "tm_sch_pri_cshap_offset_fifo_rd_empty_err" },
+ { .int_msk = BIT(22), .msg = "tm_sch_rq_fifo_wr_full_err" },
+ { .int_msk = BIT(23), .msg = "tm_sch_rq_fifo_rd_empty_err" },
+ { .int_msk = BIT(24), .msg = "tm_sch_nq_fifo_wr_full_err" },
+ { .int_msk = BIT(25), .msg = "tm_sch_nq_fifo_rd_empty_err" },
+ { .int_msk = BIT(26), .msg = "tm_sch_roce_up_fifo_wr_full_err" },
+ { .int_msk = BIT(27), .msg = "tm_sch_roce_up_fifo_rd_empty_err" },
+ { .int_msk = BIT(28), .msg = "tm_sch_rcb_byte_fifo_wr_full_err" },
+ { .int_msk = BIT(29), .msg = "tm_sch_rcb_byte_fifo_rd_empty_err" },
+ { .int_msk = BIT(30), .msg = "tm_sch_ssu_byte_fifo_wr_full_err" },
+ { .int_msk = BIT(31), .msg = "tm_sch_ssu_byte_fifo_rd_empty_err" },
+ { /* sentinel */ }
+};
+
+static const struct hclge_hw_error hclge_qcn_ecc_err_int[] = {
+ { .int_msk = BIT(0), .msg = "qcn_byte_mem_ecc_1bit_err" },
+ { .int_msk = BIT(1), .msg = "qcn_byte_mem_ecc_mbit_err" },
+ { .int_msk = BIT(2), .msg = "qcn_time_mem_ecc_1bit_err" },
+ { .int_msk = BIT(3), .msg = "qcn_time_mem_ecc_mbit_err" },
+ { .int_msk = BIT(4), .msg = "qcn_fb_mem_ecc_1bit_err" },
+ { .int_msk = BIT(5), .msg = "qcn_fb_mem_ecc_mbit_err" },
+ { .int_msk = BIT(6), .msg = "qcn_link_mem_ecc_1bit_err" },
+ { .int_msk = BIT(7), .msg = "qcn_link_mem_ecc_mbit_err" },
+ { .int_msk = BIT(8), .msg = "qcn_rate_mem_ecc_1bit_err" },
+ { .int_msk = BIT(9), .msg = "qcn_rate_mem_ecc_mbit_err" },
+ { .int_msk = BIT(10), .msg = "qcn_tmplt_mem_ecc_1bit_err" },
+ { .int_msk = BIT(11), .msg = "qcn_tmplt_mem_ecc_mbit_err" },
+ { .int_msk = BIT(12), .msg = "qcn_shap_cfg_mem_ecc_1bit_err" },
+ { .int_msk = BIT(13), .msg = "qcn_shap_cfg_mem_ecc_mbit_err" },
+ { .int_msk = BIT(14), .msg = "qcn_gp0_barrel_mem_ecc_1bit_err" },
+ { .int_msk = BIT(15), .msg = "qcn_gp0_barrel_mem_ecc_mbit_err" },
+ { .int_msk = BIT(16), .msg = "qcn_gp1_barrel_mem_ecc_1bit_err" },
+ { .int_msk = BIT(17), .msg = "qcn_gp1_barrel_mem_ecc_mbit_err" },
+ { .int_msk = BIT(18), .msg = "qcn_gp2_barrel_mem_ecc_1bit_err" },
+ { .int_msk = BIT(19), .msg = "qcn_gp2_barrel_mem_ecc_mbit_err" },
+ { .int_msk = BIT(20), .msg = "qcn_gp3_barral_mem_ecc_1bit_err" },
+ { .int_msk = BIT(21), .msg = "qcn_gp3_barral_mem_ecc_mbit_err" },
+ { /* sentinel */ }
+};
+
static void hclge_log_error(struct device *dev,
const struct hclge_hw_error *err_list,
u32 err_sts)
@@ -501,6 +624,47 @@ static int hclge_enable_ppp_error(struct hclge_dev *hdev, bool en)
return ret;
}
+int hclge_enable_tm_hw_error(struct hclge_dev *hdev, bool en)
+{
+ struct device *dev = &hdev->pdev->dev;
+ struct hclge_desc desc;
+ int ret;
+
+ /* enable TM SCH hw errors */
+ hclge_cmd_setup_basic_desc(&desc, HCLGE_TM_SCH_ECC_INT_EN, false);
+ if (en)
+ desc.data[0] = cpu_to_le32(HCLGE_TM_SCH_ECC_ERR_INT_EN);
+ else
+ desc.data[0] = 0;
+
+ ret = hclge_cmd_send(&hdev->hw, &desc, 1);
+ if (ret) {
+ dev_err(dev, "failed(%d) to configure TM SCH errors\n", ret);
+ return ret;
+ }
+
+ /* enable TM QCN hw errors */
+ ret = hclge_cmd_query_error(hdev, &desc, HCLGE_TM_QCN_MEM_INT_CFG,
+ 0, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed(%d) to read TM QCN CFG status\n", ret);
+ return ret;
+ }
+
+ hclge_cmd_reuse_desc(&desc, false);
+ if (en)
+ desc.data[1] = cpu_to_le32(HCLGE_TM_QCN_MEM_ERR_INT_EN);
+ else
+ desc.data[1] = 0;
+
+ ret = hclge_cmd_send(&hdev->hw, &desc, 1);
+ if (ret)
+ dev_err(dev,
+ "failed(%d) to configure TM QCN mem errors\n", ret);
+
+ return ret;
+}
+
static void hclge_process_common_error(struct hclge_dev *hdev,
enum hclge_err_int_type type)
{
@@ -736,6 +900,125 @@ static void hclge_process_ppp_error(struct hclge_dev *hdev,
ret);
}
+static void hclge_process_tm_sch_error(struct hclge_dev *hdev)
+{
+ struct device *dev = &hdev->pdev->dev;
+ const struct hclge_tm_sch_ecc_info *tm_sch_ecc_info;
+ struct hclge_desc desc;
+ u32 ecc_info;
+ u8 module_no;
+ u8 ram_no;
+ int ret;
+
+ /* read TM scheduler errors */
+ ret = hclge_cmd_query_error(hdev, &desc,
+ HCLGE_TM_SCH_MBIT_ECC_INFO_CMD, 0, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed(%d) to read SCH mbit ECC err info\n", ret);
+ return;
+ }
+ ecc_info = le32_to_cpu(desc.data[0]);
+
+ ret = hclge_cmd_query_error(hdev, &desc,
+ HCLGE_TM_SCH_ECC_ERR_RINT_CMD, 0, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed(%d) to read SCH ECC err status\n", ret);
+ return;
+ }
+
+ /* log TM scheduler errors */
+ if (le32_to_cpu(desc.data[0])) {
+ hclge_log_error(dev, &hclge_tm_sch_err_int[0],
+ le32_to_cpu(desc.data[0]));
+ if (le32_to_cpu(desc.data[0]) & 0x2) {
+ module_no = (ecc_info >> 20) & 0xF;
+ ram_no = (ecc_info >> 16) & 0xF;
+ tm_sch_ecc_info =
+ &hclge_tm_sch_ecc_err[module_no][ram_no];
+ dev_warn(dev, "ecc err module:ram=%s\n",
+ tm_sch_ecc_info->name);
+ dev_warn(dev, "ecc memory address = 0x%x\n",
+ ecc_info & 0xFFFF);
+ }
+ }
+
+ /* clear TM scheduler errors */
+ ret = hclge_cmd_clear_error(hdev, &desc, NULL, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed(%d) to clear TM SCH error status\n", ret);
+ return;
+ }
+
+ ret = hclge_cmd_query_error(hdev, &desc,
+ HCLGE_TM_SCH_ECC_ERR_RINT_CE, 0, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed(%d) to read SCH CE status\n", ret);
+ return;
+ }
+
+ ret = hclge_cmd_clear_error(hdev, &desc, NULL, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed(%d) to clear TM SCH CE status\n", ret);
+ return;
+ }
+
+ ret = hclge_cmd_query_error(hdev, &desc,
+ HCLGE_TM_SCH_ECC_ERR_RINT_NFE, 0, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed(%d) to read SCH NFE status\n", ret);
+ return;
+ }
+
+ ret = hclge_cmd_clear_error(hdev, &desc, NULL, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed(%d) to clear TM SCH NFE status\n", ret);
+ return;
+ }
+
+ ret = hclge_cmd_query_error(hdev, &desc,
+ HCLGE_TM_SCH_ECC_ERR_RINT_FE, 0, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed(%d) to read SCH FE status\n", ret);
+ return;
+ }
+
+ ret = hclge_cmd_clear_error(hdev, &desc, NULL, 0, 0);
+ if (ret)
+ dev_err(dev, "failed(%d) to clear TM SCH FE status\n", ret);
+}
+
+static void hclge_process_tm_qcn_error(struct hclge_dev *hdev)
+{
+ struct device *dev = &hdev->pdev->dev;
+ struct hclge_desc desc;
+ int ret;
+
+ /* read QCN errors */
+ ret = hclge_cmd_query_error(hdev, &desc,
+ HCLGE_TM_QCN_MEM_INT_INFO_CMD, 0, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed(%d) to read QCN ECC err status\n", ret);
+ return;
+ }
+
+ /* log QCN errors */
+ if (le32_to_cpu(desc.data[0]))
+ hclge_log_error(dev, &hclge_qcn_ecc_err_int[0],
+ le32_to_cpu(desc.data[0]));
+
+ /* clear QCN errors */
+ ret = hclge_cmd_clear_error(hdev, &desc, NULL, 0, 0);
+ if (ret)
+ dev_err(dev, "failed(%d) to clear QCN error status\n", ret);
+}
+
+static void hclge_process_tm_error(struct hclge_dev *hdev,
+ enum hclge_err_int_type type)
+{
+ hclge_process_tm_sch_error(hdev);
+ hclge_process_tm_qcn_error(hdev);
+}
+
static const struct hclge_hw_blk hw_blk[] = {
{ .msk = BIT(0), .name = "IGU_EGU",
.enable_error = hclge_enable_igu_egu_error,
@@ -743,6 +1026,9 @@ static const struct hclge_hw_blk hw_blk[] = {
{ .msk = BIT(5), .name = "COMMON",
.enable_error = hclge_enable_common_error,
.process_error = hclge_process_common_error, },
+ { .msk = BIT(4), .name = "TM",
+ .enable_error = hclge_enable_tm_hw_error,
+ .process_error = hclge_process_tm_error, },
{ .msk = BIT(1), .name = "PPP",
.enable_error = hclge_enable_ppp_error,
.process_error = hclge_process_ppp_error, },
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.h
index c6d3739..e0e3b58 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.h
@@ -37,6 +37,8 @@
#define HCLGE_PPP_MPF_ECC_ERR_INT2_EN_MASK 0x003F
#define HCLGE_PPP_MPF_ECC_ERR_INT3_EN 0x003F
#define HCLGE_PPP_MPF_ECC_ERR_INT3_EN_MASK 0x003F
+#define HCLGE_TM_SCH_ECC_ERR_INT_EN 0x3
+#define HCLGE_TM_QCN_MEM_ERR_INT_EN 0xFFFFFF
#define HCLGE_NCSI_ERR_INT_EN 0x3
#define HCLGE_NCSI_ERR_INT_TYPE 0x9
@@ -76,5 +78,6 @@ struct hclge_hw_error {
};
int hclge_hw_error_set_state(struct hclge_dev *hdev, bool state);
+int hclge_enable_tm_hw_error(struct hclge_dev *hdev, bool en);
pci_ers_result_t hclge_process_ras_hw_error(struct hnae3_ae_dev *ae_dev);
#endif
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 082ea97..5234b53 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -6881,6 +6881,12 @@ static int hclge_reset_ae_dev(struct hnae3_ae_dev *ae_dev)
return ret;
}
+ /* Re-enable the TM hw error interrupts because
+ * they get disabled on core/global reset.
+ */
+ if (hclge_enable_tm_hw_error(hdev, true))
+ dev_err(&pdev->dev, "failed to enable TM hw error interrupts\n");
+
dev_info(&pdev->dev, "Reset done, %s driver initialization finished.\n",
HCLGE_DRIVER_NAME);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] net: ethernet: ti: cpsw: don't flush mcast entries while switch promisc mode
From: Ivan Khoronzhuk @ 2018-10-19 19:24 UTC (permalink / raw)
To: Grygorii Strashko; +Cc: davem, linux-omap, netdev, linux-kernel
In-Reply-To: <36da9bb2-38b7-cc70-9569-8895e20c6d1f@ti.com>
On Fri, Oct 19, 2018 at 12:23:28PM -0500, Grygorii Strashko wrote:
>
>
>On 10/19/18 7:04 AM, Ivan Khoronzhuk wrote:
>>On Thu, Oct 18, 2018 at 07:03:06PM -0500, Grygorii Strashko wrote:
>>>
>>>
>>>On 10/18/18 1:00 PM, Ivan Khoronzhuk wrote:
>>>>No need now to flush mcast entries in switch mode while toggling to
>>>>promiscuous mode. It's not needed as vlan reg_mcast = ALL_PORTS
>>>>and mcast/vlan ports = ALL_PORTS, the same happening for vlan
>>>>unreg_mcast, it's set to ALL_PORT_MASK just after calling promisc
>>>>mode routine by calling set allmulti. I suppose main reason to flush
>>>>them is to use unreg_mcast to receive all to host port. Thus, now, all
>>>>mcast packets are received anyway and no reason to flush mcast entries
>>>>unsafely, as they were synced with __dev_mc_sync() previously and are
>>>>not restored. Another way is to _dev_mc_unsync() them, but no need.
>>>
>>>User have possibility to add additional mcast entries or edit
>>>existing in switch-mode, which is now done using custom tool. So,
>>>Host in promisc
>>>mode will not receive packets for mcast address X if port mask for this
>>>addr set to (ALL_PORTS - HOST_PORT). Am I missing smth?
>>
>>I didn't take into account the custom tool changing entries directly,
>>but even in this case there is at least a couple of interesting
>>questions:
>>
>>1) Before the patch applied only several days ago -
>> 5da1948969bc1991920987ce4361ea56046e5a98
>> "ti: cpsw: fix lost of mcast packets while rx_mode update"
>> It was impossible to do correctly anyway, as all mcast entries not
>> in the mc list were flushed (after rx_mode cb), by:
>> cpsw_ale_flush_multicast(cpsw->ale, ALE_ALL_PORTS, vid);
>> and those in mc, rewritten by adding them back in corrected form.
>> ... or this cb was not supposed to be called at all ...
>
>It's not allowed to manipulate ALE table in dual_mac mode, so your
>patches are safe in dual_mac mode. For switch-mode (unless we move
>forward with switch dev) standard linux interfaces allows create
>default mcast entries which then (if required) corrected using custom
>tool now.
It's not related to my patches at all, this as it was from very beginning.
My proposition with __dev_mc_unsync(priv->ndev, NULL) is safe as for
dual_mac mode as for switch mode.
>
>>
>>2) What is the reason to add mcast switch entires
>> (ALL_PORTS - HOST_PORT) if its function is added anyway by
>> unreq_mcast & (ALL_PORTS - HOST_PORT) ?
>> So, doesn't matter it's added or not - it will work :-|.
>
>because in switch mode not all traffic directed to the Host port -
No objections, that's exactly I'm talking about here, only p1&p2.
And looks like you forgot about single port board reusing switch
implementation, which by fact is not a switch but reuse this code.
Proposition to flush all its mcast table w/o restore - softly
speaking isn't best choice.
>only in promisc mode. Reason safety and performance - Host should not
>receive traffic which is not designated for it.
Any objections.
>
>promiscuous in switch mode:
>- disables learning
>- enables unicast flooding to Host port
>- enables unregistered multi-cast flooding to the Host port
>In other words, CPSW will continue forwarding packets between P1&P2,
>but also will "duplicate" packets to Host port. This will work only
>for
>vlans which have host port as member.
Sorry, but what part was not clearly described in the cover letter?
If you mean vlan entries added not by linux (linux vlans have host
port), and having no host port as a memeber - flush() hasn't been
helping in this case also.
>
>>
>>3) Even so, toggling promisc mode will clear all these changes anyway,
>> even I will call _dev_mc_unsync() after flushing them.
>
>there can be records which are not under control of Linux now.
yes, and they are flushed w/o restore (along with linux ones).
that's why fix is needed.
>
>>
>>4) If user can tune ALE table by hand, what stops him do it after moving
>> to promisc mode, seems he knows what he's doing?
>>
>>5) It could be possible only for not default vlan entries, but mcast
>> vlan support is not supported yet. Who is gona restore those
>> entries after promisc off?
>>
>>This behaviour is arguable, and flushing mcast entries can bring more
>>issues then leaving. For me it doesn't matter, I can archive the same
>>by adding after flush one line, it's even shorter:
>>__dev_mc_unsync(priv->ndev, NULL);
>
>Again, unless we move forward with switch dev you can't assume that
>Linux stack has full control over ALE table.
and that's why Linux stack is flushing all not created by it mcast entries...
but this what it was doing w/o my changes ).
after my patch it won't, Look:
- it adds/removes entries under control of linux
- it doesn't touch entries added whoever but not a linux
- it restores only entries added linux
(it was before also, but with flushing all not related entries..
..deleting what you don't want to delete)
>Sry, hence this patch is
>not a fix and can introduce changes in current behavior and cause
>regression reports - NACK.
Sorry but what regression you are talking about, what the problem with
__dev_mc_unsync(priv->ndev, NULL)? after flush? This is more than fix.
Restoring entries that were under control of linux is not a fix?
It doesn't touch anything added by custom tool and restores entries
in mc list like it was before. According to your comments, it seems
absolutely one fix you need, ...and don't forget about BBB with one port.
>
>--
>regards,
>-grygorii
--
Regards,
Ivan Khoronzhuk
^ 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