* Re: [PATCH net] net: phy: fix race in genphy_update_link
From: David Miller @ 2019-08-03 1:15 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev, liuyonglong
In-Reply-To: <19122a98-cfcd-424c-a598-e034c1a9349d@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 31 Jul 2019 23:05:10 +0200
> In phy_start_aneg() autoneg is started, and immediately after that
> link and autoneg status are read. As reported in [0] it can happen that
> at time of this read the PHY has reset the "aneg complete" bit but not
> yet the "link up" bit, what can result in a false link-up detection.
> To fix this don't report link as up if we're in aneg mode and PHY
> doesn't signal "aneg complete".
>
> [0] https://marc.info/?t=156413509900003&r=1&w=2
>
> Fixes: 4950c2ba49cc ("net: phy: fix autoneg mismatch case in genphy_read_status")
> Reported-by: liuyonglong <liuyonglong@huawei.com>
> Tested-by: liuyonglong <liuyonglong@huawei.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: mdio-octeon: Fix build error and Kconfig warning
From: David Miller @ 2019-08-03 1:11 UTC (permalink / raw)
To: natechancellor
Cc: andrew, broonie, devel, f.fainelli, gregkh, hkallweit1,
kernel-build-reports, linux-arm-kernel, linux-next, netdev, willy,
lkp, rdunlap
In-Reply-To: <20190731185023.20954-1-natechancellor@gmail.com>
From: Nathan Chancellor <natechancellor@gmail.com>
Date: Wed, 31 Jul 2019 11:50:24 -0700
> arm allyesconfig warns:
>
> WARNING: unmet direct dependencies detected for MDIO_OCTEON
> Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y]
> && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=y]
> Selected by [y]:
> - OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC &&
> NETDEVICES [=y] || COMPILE_TEST [=y])
>
> and errors:
>
> In file included from ../drivers/net/phy/mdio-octeon.c:14:
> ../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_probe':
> ../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of
> function 'writeq'; did you mean 'writeb'?
The proper way to fix this is to include either
linux/io-64-nonatomic-hi-lo.h
or
linux/io-64-nonatomic-lo-hi.h
whichever is appropriate.
^ permalink raw reply
* Re: [PATCH] atm: iphase: Fix Spectre v1 vulnerability
From: Gustavo A. R. Silva @ 2019-08-03 0:42 UTC (permalink / raw)
To: David Miller; +Cc: 3chas3, linux-atm-general, netdev, linux-kernel
In-Reply-To: <20190802.173125.399771294364845553.davem@davemloft.net>
Hi Dave,
On 8/2/19 7:31 PM, David Miller wrote:
> From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
> Date: Tue, 30 Jul 2019 22:21:41 -0500
>
>> board is controlled by user-space, hence leading to a potential
>> exploitation of the Spectre variant 1 vulnerability.
>>
>> This issue was detected with the help of Smatch:
>
> Applied and queued up for -stable.
>
> Do not CC: -stable for networking fixes, we take care of the stable
> submissions manually for this subsystem.
>
Yeah. I'm aware of that. The thing is that you don't appear
as a maintainer of this file:
$ scripts/get_maintainer.pl --nokeywords --nogit --nogit-fallback -f drivers/atm/iphase.c
Chas Williams <3chas3@gmail.com> (maintainer:ATM)
linux-atm-general@lists.sourceforge.net (moderated list:ATM)
netdev@vger.kernel.org (open list:ATM)
linux-kernel@vger.kernel.org (open list)
so, I didn't know this patch would be applied to net.
Thanks
--
Gustavo
^ permalink raw reply
* Re: [PATCH net-next v2 0/6] net: dsa: mv88e6xxx: add support for MV88E6220
From: David Miller @ 2019-08-03 0:59 UTC (permalink / raw)
To: h.feurstein
Cc: netdev, linux-kernel, andrew, vivien.didelot, f.fainelli,
rasmus.villemoes
In-Reply-To: <20190731082351.3157-1-h.feurstein@gmail.com>
From: Hubert Feurstein <h.feurstein@gmail.com>
Date: Wed, 31 Jul 2019 10:23:45 +0200
> This patch series adds support for the MV88E6220 chip to the mv88e6xxx driver.
> The MV88E6220 is almost the same as MV88E6250 except that the ports 2-4 are
> not routed to pins.
>
> Furthermore, PTP support is added to the MV88E6250 family.
>
> v2:
> - insert all 6220 entries in correct numerical order
> - introduce invalid_port_mask
> - move ptp_cc_mult* to ptp_ops and restored original ptp_adjfine code
> - added Andrews Reviewed-By to patch 2 and 4
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH] net/ethernet/qlogic/qed: force the string buffer NULL-terminated
From: David Miller @ 2019-08-03 0:57 UTC (permalink / raw)
To: xywang.sjtu; +Cc: aelior, GR-everest-linux-l2, netdev
In-Reply-To: <20190731081542.14178-1-xywang.sjtu@sjtu.edu.cn>
From: Wang Xiayang <xywang.sjtu@sjtu.edu.cn>
Date: Wed, 31 Jul 2019 16:15:42 +0800
> strncpy() does not ensure NULL-termination when the input string
> size equals to the destination buffer size 30.
> The output string is passed to qed_int_deassertion_aeu_bit()
> which calls DP_INFO() and relies NULL-termination.
>
> Use strlcpy instead. The other conditional branch above strncpy()
> needs no fix as snprintf() ensures NULL-termination.
>
> This issue is identified by a Coccinelle script.
>
> Signed-off-by: Wang Xiayang <xywang.sjtu@sjtu.edu.cn>
Applied.
^ permalink raw reply
* RE: [PATCH v2 net] hv_sock: Fix hang when a connection is closed
From: Dexuan Cui @ 2019-08-03 0:49 UTC (permalink / raw)
To: David Miller
Cc: Sunil Muthuswamy, netdev@vger.kernel.org, KY Srinivasan,
Haiyang Zhang, Stephen Hemminger, sashal@kernel.org,
Michael Kelley, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, olaf@aepfle.de, apw@canonical.com,
jasowang@redhat.com, vkuznets, marcelo.cerri@canonical.com
In-Reply-To: <20190802.172729.1656276508211556851.davem@davemloft.net>
> From: linux-hyperv-owner@vger.kernel.org
> Sent: Friday, August 2, 2019 5:27 PM
> ...
> Applied and queued up for -stable.
>
> Do not ever CC: stable for networking patches, we submit to -stable manually.
Thanks, David!
I'll remember to not add the stable tag for network patches.
Thanks,
-- Dexuan
^ permalink raw reply
* Re: [PATCH net-next v2 0/4] net: phy: Add AST2600 MDIO support
From: David Miller @ 2019-08-03 0:33 UTC (permalink / raw)
To: andrew
Cc: netdev, robh+dt, mark.rutland, joel, andrew, f.fainelli,
hkallweit1, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel
In-Reply-To: <20190731053959.16293-1-andrew@aj.id.au>
From: Andrew Jeffery <andrew@aj.id.au>
Date: Wed, 31 Jul 2019 15:09:55 +0930
> v2 of the ASPEED MDIO series addresses comments from Rob on the devicetree
> bindings and Andrew on the driver itself.
>
> v1 of the series can be found here:
>
> http://patchwork.ozlabs.org/cover/1138140/
>
> Please review!
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH] atm: iphase: Fix Spectre v1 vulnerability
From: David Miller @ 2019-08-03 0:31 UTC (permalink / raw)
To: gustavo; +Cc: 3chas3, linux-atm-general, netdev, linux-kernel
In-Reply-To: <20190731032141.GA30246@embeddedor>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Tue, 30 Jul 2019 22:21:41 -0500
> board is controlled by user-space, hence leading to a potential
> exploitation of the Spectre variant 1 vulnerability.
>
> This issue was detected with the help of Smatch:
Applied and queued up for -stable.
Do not CC: -stable for networking fixes, we take care of the stable
submissions manually for this subsystem.
Thank you.
^ permalink raw reply
* Re: [PATCH net v3] net: ipv6: Fix a bug in ndisc_send_ns when netdev only has a global address
From: David Miller @ 2019-08-03 0:29 UTC (permalink / raw)
To: suyj.fnst; +Cc: kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <1564537972-76503-1-git-send-email-suyj.fnst@cn.fujitsu.com>
From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
Date: Wed, 31 Jul 2019 09:52:52 +0800
> When the egress interface does not have a link local address, it can
> not communicate with other hosts.
>
> In RFC4861, 7.2.2 says
> "If the source address of the packet prompting the solicitation is the
> same as one of the addresses assigned to the outgoing interface, that
> address SHOULD be placed in the IP Source Address of the outgoing
> solicitation. Otherwise, any one of the addresses assigned to the
> interface should be used."
>
> In this patch we try get a global address if we get ll address failed.
>
> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
> ---
> Changes since V2:
> - Let banned_flags under the scope of its use.
I do not want to apply this.
The only situation where this can occur is when userland is managing the
interface addresses and has failed to properly add a link local address.
That is a failure by userspace to uphold it's responsibilites when it
has taken over management of these issues, not a situation the kernel
should handle.
Sorry.
^ permalink raw reply
* Re: [PATCH v2 net] hv_sock: Fix hang when a connection is closed
From: David Miller @ 2019-08-03 0:27 UTC (permalink / raw)
To: decui
Cc: sunilmut, netdev, kys, haiyangz, sthemmin, sashal, mikelley,
linux-hyperv, linux-kernel, olaf, apw, jasowang, vkuznets,
marcelo.cerri
In-Reply-To: <PU1P153MB01696DDD3A3F601370701DD2BFDF0@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM>
From: Dexuan Cui <decui@microsoft.com>
Date: Wed, 31 Jul 2019 01:25:45 +0000
>
> There is a race condition for an established connection that is being closed
> by the guest: the refcnt is 4 at the end of hvs_release() (Note: here the
> 'remove_sock' is false):
>
> 1 for the initial value;
> 1 for the sk being in the bound list;
> 1 for the sk being in the connected list;
> 1 for the delayed close_work.
>
> After hvs_release() finishes, __vsock_release() -> sock_put(sk) *may*
> decrease the refcnt to 3.
>
> Concurrently, hvs_close_connection() runs in another thread:
> calls vsock_remove_sock() to decrease the refcnt by 2;
> call sock_put() to decrease the refcnt to 0, and free the sk;
> next, the "release_sock(sk)" may hang due to use-after-free.
>
> In the above, after hvs_release() finishes, if hvs_close_connection() runs
> faster than "__vsock_release() -> sock_put(sk)", then there is not any issue,
> because at the beginning of hvs_close_connection(), the refcnt is still 4.
>
> The issue can be resolved if an extra reference is taken when the
> connection is established.
>
> Fixes: a9eeb998c28d ("hv_sock: Add support for delayed close")
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
Applied and queued up for -stable.
Do not ever CC: stable for networking patches, we submit to -stable manually.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next] net/tls: prevent skb_orphan() from leaking TLS plain text with offload
From: David Miller @ 2019-08-03 0:24 UTC (permalink / raw)
To: jakub.kicinski
Cc: netdev, oss-drivers, edumazet, borisp, aviadye, davejwatson,
john.fastabend, daniel, willemb, xiyou.wangcong, fw,
alexei.starovoitov
In-Reply-To: <20190730211258.13748-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Tue, 30 Jul 2019 14:12:58 -0700
> I'm sending this for net-next because of lack of confidence
> in my own abilities. It should apply cleanly to net... :)
It looks like there will be changes to this.
^ permalink raw reply
* Re: [PATCH net-next v5 5/6] flow_offload: support get flow_block immediately
From: Jakub Kicinski @ 2019-08-03 0:21 UTC (permalink / raw)
To: wenxu; +Cc: jiri, pablo, fw, netfilter-devel, netdev, John Hurley
In-Reply-To: <45660f1e-b6a8-1bcb-0d57-7c1790d3fbaf@ucloud.cn>
On Sat, 3 Aug 2019 07:19:31 +0800, wenxu wrote:
> > Or:
> >
> > device unregister:
> > - nft block destroy
> > - UNBIND cb
> > - free driver's block state
> > - driver notifier callback
> > - free driver's state
> >
> > No?
>
> For the second case maybe can't unbind cb? because the nft block is
> destroied. There is no way to find the block(chain) in nft.
But before the block is destroyed doesn't nft send an UNBIND event to
the drivers, always?
^ permalink raw reply
* RE: [PATCH net 1/2] net/tls: partially revert fix transition through disconnect with close
From: John Fastabend @ 2019-08-02 23:57 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, oss-drivers, edumazet, davejwatson, borisp, aviadye,
john.fastabend, daniel, Jakub Kicinski
In-Reply-To: <20190801213602.19634-1-jakub.kicinski@netronome.com>
Jakub Kicinski wrote:
> Looks like we were slightly overzealous with the shutdown()
> cleanup. Even though the sock->sk_state can reach CLOSED again,
> socket->state will not got back to SS_UNCONNECTED once
> connections is ESTABLISHED. Meaning we will see EISCONN if
> we try to reconnect, and EINVAL if we try to listen.
>
> Only listen sockets can be shutdown() and reused, but since
> ESTABLISHED sockets can never be re-connected() or used for
> listen() we don't need to try to clean up the ULP state early.
>
> Fixes: 32857cf57f92 ("net/tls: fix transition through disconnect with close")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
Thanks Jakub this looks reasonable to me I'm going to run some tests
with sockmap + ktls on it this weekend to be sure the two work
together with some of our applications. I believe the original
series should be enough so that BPF can be safely unloaded out
from underneath ktls now.
I guess we could do something similar on the sockmap side but I
do want to loosen the restrictions there at some point so might
be best just to keep it as is.
Thanks,
John
^ permalink raw reply
* [PATCH bpf-next 2/2] selftests/bpf: add loop test 5
From: Alexei Starovoitov @ 2019-08-02 23:33 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, bpf, kernel-team
In-Reply-To: <20190802233344.863418-1-ast@kernel.org>
Add a test with multiple exit conditions.
It's not an infinite loop only when the verifier can properly track
all math on variable 'i' through all possible ways of executing this loop.
barrier()s are needed to disable llvm optimization that combines multiple
branches into fewer branches.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
.../bpf/prog_tests/bpf_verif_scale.c | 1 +
tools/testing/selftests/bpf/progs/loop5.c | 37 +++++++++++++++++++
2 files changed, 38 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/loop5.c
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index 757e39540eda..29615a4a9362 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -72,6 +72,7 @@ void test_bpf_verif_scale(void)
{ "loop1.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
{ "loop2.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
{ "loop4.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+ { "loop5.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
/* partial unroll. 19k insn in a loop.
* Total program size 20.8k insn.
diff --git a/tools/testing/selftests/bpf/progs/loop5.c b/tools/testing/selftests/bpf/progs/loop5.c
new file mode 100644
index 000000000000..9d9817efe208
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/loop5.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Facebook
+#include <linux/sched.h>
+#include <linux/ptrace.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <stdbool.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+#define barrier() __asm__ __volatile__("": : :"memory")
+
+char _license[] SEC("license") = "GPL";
+
+SEC("socket")
+int while_true(volatile struct __sk_buff* skb)
+{
+ int i = 0;
+
+ while (true) {
+ if (skb->len)
+ i += 3;
+ else
+ i += 7;
+ if (i == 9)
+ break;
+ barrier();
+ if (i == 10)
+ break;
+ barrier();
+ if (i == 13)
+ break;
+ barrier();
+ if (i == 14)
+ break;
+ }
+ return i;
+}
--
2.20.0
^ permalink raw reply related
* [PATCH bpf-next 0/2] selftests/bpf: more loop tests
From: Alexei Starovoitov @ 2019-08-02 23:33 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, bpf, kernel-team
Add two bounded loop tests.
Alexei Starovoitov (2):
selftests/bpf: add loop test 4
selftests/bpf: add loop test 5
.../bpf/prog_tests/bpf_verif_scale.c | 2 +
tools/testing/selftests/bpf/progs/loop4.c | 23 ++++++++++++
tools/testing/selftests/bpf/progs/loop5.c | 37 +++++++++++++++++++
3 files changed, 62 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/loop4.c
create mode 100644 tools/testing/selftests/bpf/progs/loop5.c
--
2.20.0
^ permalink raw reply
* [PATCH bpf-next 1/2] selftests/bpf: add loop test 4
From: Alexei Starovoitov @ 2019-08-02 23:33 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, bpf, kernel-team
In-Reply-To: <20190802233344.863418-1-ast@kernel.org>
Add a test that returns a 'random' number between [0, 2^20)
If state pruning is not working correctly for loop body the number of
processed insns will be 2^20 * num_of_insns_in_loop_body and the program
will be rejected.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
.../bpf/prog_tests/bpf_verif_scale.c | 1 +
tools/testing/selftests/bpf/progs/loop4.c | 23 +++++++++++++++++++
2 files changed, 24 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/loop4.c
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index b4be96162ff4..757e39540eda 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -71,6 +71,7 @@ void test_bpf_verif_scale(void)
{ "loop1.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
{ "loop2.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+ { "loop4.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
/* partial unroll. 19k insn in a loop.
* Total program size 20.8k insn.
diff --git a/tools/testing/selftests/bpf/progs/loop4.c b/tools/testing/selftests/bpf/progs/loop4.c
new file mode 100644
index 000000000000..3e7ee14fddbd
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/loop4.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Facebook
+#include <linux/sched.h>
+#include <linux/ptrace.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <stdbool.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("socket")
+int combinations(volatile struct __sk_buff* skb)
+{
+ int ret = 0, i;
+
+#pragma nounroll
+ for (i = 0; i < 20; i++)
+ if (skb->len)
+ ret |= 1 << i;
+ return ret;
+}
--
2.20.0
^ permalink raw reply related
* Re: [PATCH] net: sctp: Rename fallthrough label to unhandled
From: Joe Perches @ 2019-08-02 23:26 UTC (permalink / raw)
To: David Miller
Cc: nhorman, vyasevich, marcelo.leitner, linux-sctp, netdev,
linux-kernel
In-Reply-To: <20190802.161932.1776993765494484851.davem@davemloft.net>
On Fri, 2019-08-02 at 16:19 -0700, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Fri, 02 Aug 2019 10:47:34 -0700
>
> > On Wed, 2019-07-31 at 08:16 -0400, Neil Horman wrote:
> >> On Wed, Jul 31, 2019 at 04:32:43AM -0700, Joe Perches wrote:
> >> > On Wed, 2019-07-31 at 07:19 -0400, Neil Horman wrote:
> >> > > On Tue, Jul 30, 2019 at 10:04:37PM -0700, Joe Perches wrote:
> >> > > > fallthrough may become a pseudo reserved keyword so this only use of
> >> > > > fallthrough is better renamed to allow it.
> >
> > Can you or any other maintainer apply this patch
> > or ack it so David Miller can apply it?
>
> I, like others, don't like the lack of __ in the keyword. It's kind of
> rediculous the problems it creates to pollute the global namespace like
> that and yes also inconsistent with other shorthands for builtins.
Please add that to the conversation on the RFC thread.
https://lore.kernel.org/patchwork/patch/1108577/
In any case, fallthrough is not really a good label
name for this use.
^ permalink raw reply
* Re: [PATCH net-next v5 5/6] flow_offload: support get flow_block immediately
From: wenxu @ 2019-08-02 23:19 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: jiri, pablo, fw, netfilter-devel, netdev, John Hurley
In-Reply-To: <20190802110216.5e1fd938@cakuba.netronome.com>
在 2019/8/3 2:02, Jakub Kicinski 写道:
> On Fri, 2 Aug 2019 21:09:03 +0800, wenxu wrote:
>>>> We'd have something like the loop in flow_get_default_block():
>>>>
>>>> for each (subsystem)
>>>> subsystem->handle_new_indir_cb(indr_dev, cb);
>>>>
>>>> And then per-subsystem logic would actually call the cb. Or:
>>>>
>>>> for each (subsystem)
>>>> block = get_default_block(indir_dev)
>>>> indr_dev->ing_cmd_cb(...)
>>> nft dev chian is also based on register_netdevice_notifier, So for unregister case,
>>>
>>> the basechian(block) of nft maybe delete before the __tc_indr_block_cb_unregister. is right?
>>>
>>> So maybe we can cache the block as a list of all the subsystem in indr_dev ?
>>
>> when the device is unregister the nft netdev chain related to this device will also be delete through netdevice_notifier
>>
>> . So for unregister case,the basechian(block) of nft maybe delete before the __tc_indr_block_cb_unregister.
> Hm, but I don't think that should be an issue. The ordering should be
> like one of the following two:
>
> device unregister:
> - driver notifier callback
> - unregister flow cb
> - UNBIND cb
> - free driver's block state
> - free driver's device state
> - nft block destroy
> # doesn't see driver's CB any more
>
> Or:
>
> device unregister:
> - nft block destroy
> - UNBIND cb
> - free driver's block state
> - driver notifier callback
> - free driver's state
>
> No?
For the second case maybe can't unbind cb? because the nft block is destroied. There is no way
to find the block(chain) in nft.
>
>> cache for the block is not work because the chain already be delete and free. Maybe it improve the prio of
>>
>> rep_netdev_event can help this?
> In theory the cache should work in a similar way as drivers, because
> once the indr_dev is created and the initial block is found, the cached
> value is just recorded in BIND/UNBIND calls. So if BIND/UNBIND works for
> drivers it will also put the right info in the cache.
>
^ permalink raw reply
* Re: [PATCH] net: sctp: Rename fallthrough label to unhandled
From: David Miller @ 2019-08-02 23:19 UTC (permalink / raw)
To: joe; +Cc: nhorman, vyasevich, marcelo.leitner, linux-sctp, netdev,
linux-kernel
In-Reply-To: <a03a23728d3b468942a20b55f70babceaec587ee.camel@perches.com>
From: Joe Perches <joe@perches.com>
Date: Fri, 02 Aug 2019 10:47:34 -0700
> On Wed, 2019-07-31 at 08:16 -0400, Neil Horman wrote:
>> On Wed, Jul 31, 2019 at 04:32:43AM -0700, Joe Perches wrote:
>> > On Wed, 2019-07-31 at 07:19 -0400, Neil Horman wrote:
>> > > On Tue, Jul 30, 2019 at 10:04:37PM -0700, Joe Perches wrote:
>> > > > fallthrough may become a pseudo reserved keyword so this only use of
>> > > > fallthrough is better renamed to allow it.
>
> Can you or any other maintainer apply this patch
> or ack it so David Miller can apply it?
I, like others, don't like the lack of __ in the keyword. It's kind of
rediculous the problems it creates to pollute the global namespace like
that and yes also inconsistent with other shorthands for builtins.
^ permalink raw reply
* Re: [PATCH 03/34] net/ceph: convert put_page() to put_user_page*()
From: Jeff Layton @ 2019-08-02 22:32 UTC (permalink / raw)
To: john.hubbard, Andrew Morton
Cc: Christoph Hellwig, Dan Williams, Dave Chinner, Dave Hansen,
Ira Weiny, Jan Kara, Jason Gunthorpe, Jérôme Glisse,
LKML, amd-gfx, ceph-devel, devel, devel, dri-devel, intel-gfx,
kvm, linux-arm-kernel, linux-block, linux-crypto, linux-fbdev,
linux-fsdevel, linux-media, linux-mm, linux-nfs, linux-rdma,
linux-rpi-kernel, linux-xfs, netdev, rds-devel, sparclinux, x86,
xen-devel, John Hubbard, Ilya Dryomov, Sage Weil,
David S . Miller
In-Reply-To: <20190802022005.5117-4-jhubbard@nvidia.com>
On Thu, 2019-08-01 at 19:19 -0700, john.hubbard@gmail.com wrote:
> From: John Hubbard <jhubbard@nvidia.com>
>
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
>
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
>
> Cc: Ilya Dryomov <idryomov@gmail.com>
> Cc: Sage Weil <sage@redhat.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: ceph-devel@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
> net/ceph/pagevec.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/net/ceph/pagevec.c b/net/ceph/pagevec.c
> index 64305e7056a1..c88fff2ab9bd 100644
> --- a/net/ceph/pagevec.c
> +++ b/net/ceph/pagevec.c
> @@ -12,13 +12,7 @@
>
> void ceph_put_page_vector(struct page **pages, int num_pages, bool dirty)
> {
> - int i;
> -
> - for (i = 0; i < num_pages; i++) {
> - if (dirty)
> - set_page_dirty_lock(pages[i]);
> - put_page(pages[i]);
> - }
> + put_user_pages_dirty_lock(pages, num_pages, dirty);
> kvfree(pages);
> }
> EXPORT_SYMBOL(ceph_put_page_vector);
This patch looks sane enough. Assuming that the earlier patches are OK:
Acked-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH net 0/2] flow_offload hardware priority fixes
From: Jakub Kicinski @ 2019-08-02 22:25 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netfilter-devel, davem, netdev, marcelo.leitner, jiri, wenxu,
saeedm, paulb, gerlitz.or
In-Reply-To: <20190802220409.klwdkcvjgegz6hj2@salvia>
On Sat, 3 Aug 2019 00:04:09 +0200, Pablo Neira Ayuso wrote:
> That patch removed the reference to tcf_auto_prio() already, please
> let me know if you have any more specific update you would like to see
> on that patch.
Please explain why the artificial priorities are needed at all.
Hardware should order tables based on table type - ethtool, TC, nft.
As I mentioned in the first email, and unless you can make a strong
case against that.
Within those tables we should follow the same ordering rules as we
do in software (modulo ethtool but ordering is pretty clear there).
^ permalink raw reply
* Re: [PATCH] isdn: hysdn: Fix error spaces around '*'
From: Joe Perches @ 2019-08-02 22:05 UTC (permalink / raw)
To: Stephen Hemminger, Jose Carlos Cazarin Filho
Cc: isdn, gregkh, devel, netdev, linux-kernel
In-Reply-To: <20190802145506.168b576b@hermes.lan>
On Fri, 2019-08-02 at 14:55 -0700, Stephen Hemminger wrote:
> On Fri, 2 Aug 2019 19:56:02 +0000
> Jose Carlos Cazarin Filho <joseespiriki@gmail.com> wrote:
>
> > Fix checkpath error:
> > CHECK: spaces preferred around that '*' (ctx:WxV)
> > +extern hysdn_card *card_root; /* pointer to first card */
> >
> > Signed-off-by: Jose Carlos Cazarin Filho <joseespiriki@gmail.com>
>
> Read the TODO, these drivers are scheduled for removal, so changes
> are not helpful at this time.
Maybe better to mark the MAINTAINERS entry obsolete so
checkpatch bleats a message about unnecessary changes.
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 30bf852e6d6b..b5df91032574 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8628,7 +8628,7 @@ M: Karsten Keil <isdn@linux-pingi.de>
L: isdn4linux@listserv.isdn4linux.de (subscribers-only)
L: netdev@vger.kernel.org
W: http://www.isdn4linux.de
-S: Odd Fixes
+S: Obsolete
F: Documentation/isdn/
F: drivers/isdn/capi/
F: drivers/staging/isdn/
^ permalink raw reply related
* Re: [PATCH net 0/2] flow_offload hardware priority fixes
From: Pablo Neira Ayuso @ 2019-08-02 22:04 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netfilter-devel, davem, netdev, marcelo.leitner, jiri, wenxu,
saeedm, paulb, gerlitz.or
In-Reply-To: <20190802134738.328691b4@cakuba.netronome.com>
Hi Jakub,
On Fri, Aug 02, 2019 at 01:47:38PM -0700, Jakub Kicinski wrote:
> On Fri, 2 Aug 2019 13:00:23 +0200, Pablo Neira Ayuso wrote:
> > Hi Jakub,
> >
> > If the user specifies 'pref' in the new rule, then tc checks if there
> > is a tcf_proto object that matches this priority. If the tcf_proto
> > object does not exist, tc creates a tcf_proto object and it adds the
> > new rule to this tcf_proto.
> >
> > In cls_flower, each tcf_proto only stores one single rule, so if the
> > user tries to add another rule with the same 'pref', cls_flower
> > returns EEXIST.
>
> So you're saying this doesn't work?
>
> ip link add type dummy
> tc qdisc add dev dummy0 clsact
> tc filter add dev dummy0 ingress protocol ipv6 prio 123 flower src_ip 1111::1 action drop
> tc filter add dev dummy0 ingress protocol ipv6 prio 123 flower src_ip 1111::2 action drop
This works indeed as you describe.
I'll go back to the original netfilter basechain priority patch then:
https://patchwork.ozlabs.org/patch/1140412/
That patch removed the reference to tcf_auto_prio() already, please
let me know if you have any more specific update you would like to see
on that patch.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 bpf-next 02/12] libbpf: implement BPF CO-RE offset relocation algorithm
From: Alexei Starovoitov @ 2019-08-02 21:56 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Yonghong Song, Song Liu, Kernel Team
In-Reply-To: <CAEf4BzarjODxo5c-UKtCL_dGGNb1m-3QPAGGR0eq_0tcZVMt8g@mail.gmail.com>
On Fri, Aug 02, 2019 at 12:16:52AM -0700, Andrii Nakryiko wrote:
> On Thu, Aug 1, 2019 at 4:50 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Jul 31, 2019 at 11:47:53PM -0700, Andrii Nakryiko wrote:
> > > This patch implements the core logic for BPF CO-RE offsets relocations.
> > > Every instruction that needs to be relocated has corresponding
> > > bpf_offset_reloc as part of BTF.ext. Relocations are performed by trying
> > > to match recorded "local" relocation spec against potentially many
> > > compatible "target" types, creating corresponding spec. Details of the
> > > algorithm are noted in corresponding comments in the code.
> > >
> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > > Acked-by: Song Liu <songliubraving@fb.com>
> > ...
> > > + if (btf_is_composite(t)) {
> > > + const struct btf_member *m = (void *)(t + 1);
> > > + __u32 offset;
> > > +
> > > + if (access_idx >= BTF_INFO_VLEN(t->info))
> > > + return -EINVAL;
> > > +
> > > + m = &m[access_idx];
> > > +
> > > + if (BTF_INFO_KFLAG(t->info)) {
> > > + if (BTF_MEMBER_BITFIELD_SIZE(m->offset))
> > > + return -EINVAL;
> > > + offset = BTF_MEMBER_BIT_OFFSET(m->offset);
> > > + } else {
> > > + offset = m->offset;
> > > + }
> >
> > very similar logic exists in btf_dump.c
> > probably makes sense to make a common helper at some point.
>
> Will add btf_member_bit_offset(type, member) and
> btf_member_bit_size(type, member).
>
> >
> > > +static size_t bpf_core_essential_name_len(const char *name)
> > > +{
> > > + size_t n = strlen(name);
> > > + int i = n - 3;
> > > +
> > > + while (i > 0) {
> > > + if (name[i] == '_' && name[i + 1] == '_' && name[i + 2] == '_')
> > > + return i;
> > > + i--;
> > > + }
> > > + return n;
> > > +}
> >
> > that's a bit of an eye irritant. How about?
> > size_t n = strlen(name);
> > int i, cnt = 0;
> >
> > for (i = n - 1; i >= 0; i--) {
> > if (name[i] == '_') {
> > cnt++;
> > } else {
> > if (cnt == 3)
> > return i + 1;
> > cnt = 0;
> > }
> > }
> > return n;
>
> I find this one much harder to read and understand. What's
> eye-irritating about that loop?
>
> Your loop will also handle `a____b` differently. My version will
> return "a_" as essential name, yours "a____b". Was this intentional on
> your part?
hmm. I think both will return sizeof("a") == 1
> I'd rather use this instead, if you hate the first one:
>
> size_t n = strlen(name);
> int i;
>
> for (i = n - 3; i > 0; i--) {
> if (strncmp(name + i, "___", 3) == 0)
> return i;
> }
>
> Is this better?
that is worse.
What I don't like about it is that every byte is
compared N=sizeof(string-to-found) times.
I guess it's not such a big performance criticial path,
but libbpf has to keep the bar high.
> >
> > > + case BTF_KIND_ARRAY: {
> > > + const struct btf_array *loc_a, *targ_a;
> > > +
> > > + loc_a = (void *)(local_type + 1);
> > > + targ_a = (void *)(targ_type + 1);
> > > + local_id = loc_a->type;
> > > + targ_id = targ_a->type;
> >
> > can we add a helper like:
>
> Yes, we can. I was thinking about that, but decided to not expand
> patch set. But we do need to extract all those small, but nice
> helpers. I'll put them in libbpf_internal.h for now, but I think it
> might be good idea to expose them as part of btf.h. Thoughts?
part of btf.h make sense to me.
>
> > const struct btf_array *btf_array(cosnt struct btf_type *t)
> > {
> > return (const struct btf_array *)(t + 1);
> > }
> >
> > then above will be:
> > case BTF_KIND_ARRAY: {
> > local_id = btf_array(local_type)->type;
> > targ_id = btf_array(targ_type)->type;
> >
> > and a bunch of code in btf.c and btf_dump.c would be cleaner as well?
>
> Yep, some of those are already scattered around btf.c and btf_dump.c,
> will clean up and add patch to this patch set.
>
> >
> > > + goto recur;
> > > + }
> > > + default:
> > > + pr_warning("unexpected kind %d relocated, local [%d], target [%d]\n",
> > > + kind, local_id, targ_id);
> > > + return 0;
> > > + }
> > > +}
> > > +
> > > +/*
> > > + * Given single high-level named field accessor in local type, find
> > > + * corresponding high-level accessor for a target type. Along the way,
> > > + * maintain low-level spec for target as well. Also keep updating target
> > > + * offset.
> > > + *
> > > + * Searching is performed through recursive exhaustive enumeration of all
> > > + * fields of a struct/union. If there are any anonymous (embedded)
> > > + * structs/unions, they are recursively searched as well. If field with
> > > + * desired name is found, check compatibility between local and target types,
> > > + * before returning result.
> > > + *
> > > + * 1 is returned, if field is found.
> > > + * 0 is returned if no compatible field is found.
> > > + * <0 is returned on error.
> > > + */
> > > +static int bpf_core_match_member(const struct btf *local_btf,
> > > + const struct bpf_core_accessor *local_acc,
> > > + const struct btf *targ_btf,
> > > + __u32 targ_id,
> > > + struct bpf_core_spec *spec,
> > > + __u32 *next_targ_id)
> > > +{
> > > + const struct btf_type *local_type, *targ_type;
> > > + const struct btf_member *local_member, *m;
> > > + const char *local_name, *targ_name;
> > > + __u32 local_id;
> > > + int i, n, found;
> > > +
> > > + targ_type = skip_mods_and_typedefs(targ_btf, targ_id, &targ_id);
> > > + if (!targ_type)
> > > + return -EINVAL;
> > > + if (!btf_is_composite(targ_type))
> > > + return 0;
> > > +
> > > + local_id = local_acc->type_id;
> > > + local_type = btf__type_by_id(local_btf, local_id);
> > > + local_member = (void *)(local_type + 1);
> > > + local_member += local_acc->idx;
> > > + local_name = btf__name_by_offset(local_btf, local_member->name_off);
> > > +
> > > + n = BTF_INFO_VLEN(targ_type->info);
> > > + m = (void *)(targ_type + 1);
> >
> > new btf_member() helper?
> >
> > > + for (i = 0; i < n; i++, m++) {
> > > + __u32 offset;
> > > +
> > > + /* bitfield relocations not supported */
> > > + if (BTF_INFO_KFLAG(targ_type->info)) {
> > > + if (BTF_MEMBER_BITFIELD_SIZE(m->offset))
> > > + continue;
> > > + offset = BTF_MEMBER_BIT_OFFSET(m->offset);
> > > + } else {
> > > + offset = m->offset;
> > > + }
> > > + if (offset % 8)
> > > + continue;
> >
> > same bit of code again?
> > definitely could use a helper.
>
> Different handling (continue here, return error above), but can use
> those helpers I mentioned above.
>
> >
> > > + for (i = 0; i < local_spec->len; i++, local_acc++, targ_acc++) {
> > > + targ_type = skip_mods_and_typedefs(targ_spec->btf, targ_id,
> > > + &targ_id);
> > > + if (!targ_type)
> > > + return -EINVAL;
> > > +
> > > + if (local_acc->name) {
> > > + matched = bpf_core_match_member(local_spec->btf,
> > > + local_acc,
> > > + targ_btf, targ_id,
> > > + targ_spec, &targ_id);
> > > + if (matched <= 0)
> > > + return matched;
> > > + } else {
> > > + /* for i=0, targ_id is already treated as array element
> > > + * type (because it's the original struct), for others
> > > + * we should find array element type first
> > > + */
> > > + if (i > 0) {
> > > + const struct btf_array *a;
> > > +
> > > + if (!btf_is_array(targ_type))
> > > + return 0;
> > > +
> > > + a = (void *)(targ_type + 1);
> > > + if (local_acc->idx >= a->nelems)
> > > + return 0;
> >
> > am I reading it correctly that the local spec requested out-of-bounds
> > index in the target array type?
> > Why this is 'ignore' instead of -EINVAL?
>
> Similar to any other mismatch (e.g., int in local type vs int64 in
> target type). It just makes candidate not matching. Why would that be
> error that will stop the whole relocation and subsequently object
> loading process?
Did the field name match or this is for anon types?
I've read it as names matched and type miscompared.
>
> >
> > > +/*
> > > + * Probe few well-known locations for vmlinux kernel image and try to load BTF
> > > + * data out of it to use for target BTF.
> > > + */
> > > +static struct btf *bpf_core_find_kernel_btf(void)
> > > +{
> > > + const char *locations[] = {
> > > + "/lib/modules/%1$s/vmlinux-%1$s",
> > > + "/usr/lib/modules/%1$s/kernel/vmlinux",
> > > + };
> > > + char path[PATH_MAX + 1];
> > > + struct utsname buf;
> > > + struct btf *btf;
> > > + int i, err;
> > > +
> > > + err = uname(&buf);
> > > + if (err) {
> > > + pr_warning("failed to uname(): %d\n", err);
> >
> > defensive programming ?
> > I think uname() can fail only if &buf points to non-existing page like null.
>
> I haven't checked source for this syscall, but man page specified that
> it might return -1 on error.
man page says that it can only return EFAULT.
>
> >
> > > + return ERR_PTR(err);
> > > + }
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(locations); i++) {
> > > + snprintf(path, PATH_MAX, locations[i], buf.release);
> > > + pr_debug("attempting to load kernel BTF from '%s'\n", path);
> >
> > I think this debug message would have been more useful after access().
>
> Sure, will move.
>
> >
> > > +
> > > + if (access(path, R_OK))
> > > + continue;
> > > +
> > > + btf = btf__parse_elf(path, NULL);
> > > + if (IS_ERR(btf))
> > > + continue;
> > > +
> > > + pr_debug("successfully loaded kernel BTF from '%s'\n", path);
> > > + return btf;
> > > + }
> > > +
> > > + pr_warning("failed to find valid kernel BTF\n");
> > > + return ERR_PTR(-ESRCH);
> > > +}
> > > +
> > > +/* Output spec definition in the format:
> > > + * [<type-id>] (<type-name>) + <raw-spec> => <offset>@<spec>,
> > > + * where <spec> is a C-syntax view of recorded field access, e.g.: x.a[3].b
> > > + */
> > > +static void bpf_core_dump_spec(int level, const struct bpf_core_spec *spec)
> > > +{
> > > + const struct btf_type *t;
> > > + const char *s;
> > > + __u32 type_id;
> > > + int i;
> > > +
> > > + type_id = spec->spec[0].type_id;
> > > + t = btf__type_by_id(spec->btf, type_id);
> > > + s = btf__name_by_offset(spec->btf, t->name_off);
> > > + libbpf_print(level, "[%u] (%s) + ", type_id, s);
> >
> > imo extra []() don't improve readability of the dump.
>
> [<num>] is the general convention I've been using throughout libbpf to
> specify type ID, so I'd rather keep it for consistency. I can drop
> parens, though, no problem.
>
> >
> > > +
> > > + for (i = 0; i < spec->raw_len; i++)
> > > + libbpf_print(level, "%d%s", spec->raw_spec[i],
> > > + i == spec->raw_len - 1 ? " => " : ":");
> > > +
> > > + libbpf_print(level, "%u @ &x", spec->offset);
> > > +
> > > + for (i = 0; i < spec->len; i++) {
> > > + if (spec->spec[i].name)
> > > + libbpf_print(level, ".%s", spec->spec[i].name);
> > > + else
> > > + libbpf_print(level, "[%u]", spec->spec[i].idx);
> > > + }
> > > +
> > > +}
> > > +
> > > +static size_t bpf_core_hash_fn(const void *key, void *ctx)
> > > +{
> > > + return (size_t)key;
> > > +}
> > > +
> > > +static bool bpf_core_equal_fn(const void *k1, const void *k2, void *ctx)
> > > +{
> > > + return k1 == k2;
> > > +}
> > > +
> > > +static void *u32_to_ptr(__u32 x)
> > > +{
> > > + return (void *)(uintptr_t)x;
> > > +}
> >
> > u32 to pointer on 64-bit arch?!
> > That surely needs a comment.
>
> I should probably call it u32_to_hash_key() to make it obvious it's
> conversion to hashmap's generic `void *` key type.
>
> >
> > > +
> > > +/*
> > > + * CO-RE relocate single instruction.
> > > + *
> > > + * The outline and important points of the algorithm:
> > > + * 1. For given local type, find corresponding candidate target types.
> > > + * Candidate type is a type with the same "essential" name, ignoring
> > > + * everything after last triple underscore (___). E.g., `sample`,
> > > + * `sample___flavor_one`, `sample___flavor_another_one`, are all candidates
> > > + * for each other. Names with triple underscore are referred to as
> > > + * "flavors" and are useful, among other things, to allow to
> > > + * specify/support incompatible variations of the same kernel struct, which
> > > + * might differ between different kernel versions and/or build
> > > + * configurations.
> > > + *
> > > + * N.B. Struct "flavors" could be generated by bpftool's BTF-to-C
> > > + * converter, when deduplicated BTF of a kernel still contains more than
> > > + * one different types with the same name. In that case, ___2, ___3, etc
> > > + * are appended starting from second name conflict. But start flavors are
> > > + * also useful to be defined "locally", in BPF program, to extract same
> > > + * data from incompatible changes between different kernel
> > > + * versions/configurations. For instance, to handle field renames between
> > > + * kernel versions, one can use two flavors of the struct name with the
> > > + * same common name and use conditional relocations to extract that field,
> > > + * depending on target kernel version.
> >
> > there are actual kernel types that have ___ in the name.
> > Ex: struct lmc___media
> > We probably need to revisit this 'flavor' convention.
>
> There are only these:
> - lmc___softc
> - lmc___media
> - lmc___ctl (all three in drivers/net/wan/lmc/lmc_var.h)
> - ____ftrace_##name set of structs
>
> I couldn't come up with anything cleaner-looking. I think we can still
> keep ___ convention, but:
>
> 1. Match only exactly 3 underscores, delimited by non-underscore from
> both sides (so similar to your proposed loop above);
> 2. We can also try matching candidates assuming full name without
> ___xxx part removed, in addition to current logic. This seems like an
> overkill at this point and unlikely to be useful in practice, so I'd
> postpone implementing this until we really need it.
>
> What do you think? Which other convention did you have in mind?
may be match ___[0-9]+ instead for now?
Not as flexible, but user supplied "flavors" is not an immediate task.
>
> >
> > > + for (i = 0, j = 0; i < cand_ids->len; i++) {
> > > + cand_id = cand_ids->data[i];
> > > + cand_type = btf__type_by_id(targ_btf, cand_id);
> > > + cand_name = btf__name_by_offset(targ_btf, cand_type->name_off);
> > > +
> > > + err = bpf_core_spec_match(&local_spec, targ_btf,
> > > + cand_id, &cand_spec);
> > > + if (err < 0) {
> > > + pr_warning("prog '%s': relo #%d: failed to match spec ",
> > > + prog_name, relo_idx);
> > > + bpf_core_dump_spec(LIBBPF_WARN, &local_spec);
> > > + libbpf_print(LIBBPF_WARN,
> > > + " to candidate #%d [%d] (%s): %d\n",
> > > + i, cand_id, cand_name, err);
> > > + return err;
> > > + }
> > > + if (err == 0) {
> > > + pr_debug("prog '%s': relo #%d: candidate #%d [%d] (%s) doesn't match spec ",
> > > + prog_name, relo_idx, i, cand_id, cand_name);
> > > + bpf_core_dump_spec(LIBBPF_DEBUG, &local_spec);
> > > + libbpf_print(LIBBPF_DEBUG, "\n");
> > > + continue;
> > > + }
> > > +
> > > + pr_debug("prog '%s': relo #%d: candidate #%d matched as spec ",
> > > + prog_name, relo_idx, i);
> >
> > did you mention that you're going to make a helper for this debug dumps?
>
> yeah, I added bpf_core_dump_spec(), but I don't know how to shorten
> this further... This output is extremely useful to understand what's
> happening and will be invaluable when users will inevitably report
> confusing behavior in some cases, so I still want to keep it.
not sure yet. Just pointing out that this function has more debug printfs
than actual code which doesn't look right.
We have complex algorithms in the kernel (like verifier).
Yet we don't sprinkle printfs in there to this degree.
^ permalink raw reply
* Re: [PATCH] isdn: hysdn: Fix error spaces around '*'
From: Stephen Hemminger @ 2019-08-02 21:55 UTC (permalink / raw)
To: Jose Carlos Cazarin Filho; +Cc: isdn, gregkh, devel, netdev, linux-kernel
In-Reply-To: <20190802195602.28414-1-joseespiriki@gmail.com>
On Fri, 2 Aug 2019 19:56:02 +0000
Jose Carlos Cazarin Filho <joseespiriki@gmail.com> wrote:
> Fix checkpath error:
> CHECK: spaces preferred around that '*' (ctx:WxV)
> +extern hysdn_card *card_root; /* pointer to first card */
>
> Signed-off-by: Jose Carlos Cazarin Filho <joseespiriki@gmail.com>
Read the TODO, these drivers are scheduled for removal, so changes
are not helpful at this time.
^ 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