* Re: [PATCH 09/11] bpf: Add d_path helper
From: Andrii Nakryiko @ 2020-06-19 4:35 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Networking, bpf, Song Liu,
Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
Wenbo Zhang, KP Singh, Andrii Nakryiko, Brendan Gregg,
Florent Revest, Al Viro
In-Reply-To: <20200616100512.2168860-10-jolsa@kernel.org>
On Tue, Jun 16, 2020 at 3:07 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding d_path helper function that returns full path
> for give 'struct path' object, which needs to be the
> kernel BTF 'path' object.
>
> The helper calls directly d_path function.
>
> Updating also bpf.h tools uapi header and adding
> 'path' to bpf_helpers_doc.py script.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> include/linux/bpf.h | 4 ++++
> include/uapi/linux/bpf.h | 14 ++++++++++++-
> kernel/bpf/btf_ids.c | 11 ++++++++++
> kernel/trace/bpf_trace.c | 38 ++++++++++++++++++++++++++++++++++
> scripts/bpf_helpers_doc.py | 2 ++
> tools/include/uapi/linux/bpf.h | 14 ++++++++++++-
> 6 files changed, 81 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index a94e85c2ec50..d35265b6c574 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1752,5 +1752,9 @@ extern int bpf_skb_output_btf_ids[];
> extern int bpf_seq_printf_btf_ids[];
> extern int bpf_seq_write_btf_ids[];
> extern int bpf_xdp_output_btf_ids[];
> +extern int bpf_d_path_btf_ids[];
> +
> +extern int btf_whitelist_d_path[];
> +extern int btf_whitelist_d_path_cnt;
So with suggestion from previous patch, this would be declared as:
extern const struct btf_id_set btf_whitelist_d_path;
>
> #endif /* _LINUX_BPF_H */
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index c65b374a5090..e308746b9344 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3252,6 +3252,17 @@ union bpf_attr {
> * case of **BPF_CSUM_LEVEL_QUERY**, the current skb->csum_level
> * is returned or the error code -EACCES in case the skb is not
> * subject to CHECKSUM_UNNECESSARY.
> + *
> + * int bpf_d_path(struct path *path, char *buf, u32 sz)
> + * Description
> + * Return full path for given 'struct path' object, which
> + * needs to be the kernel BTF 'path' object. The path is
> + * returned in buffer provided 'buf' of size 'sz'.
> + *
> + * Return
> + * length of returned string on success, or a negative
> + * error in case of failure
> + *
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -3389,7 +3400,8 @@ union bpf_attr {
> FN(ringbuf_submit), \
> FN(ringbuf_discard), \
> FN(ringbuf_query), \
> - FN(csum_level),
> + FN(csum_level), \
> + FN(d_path),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> diff --git a/kernel/bpf/btf_ids.c b/kernel/bpf/btf_ids.c
> index d8d0df162f04..853c8fd59b06 100644
> --- a/kernel/bpf/btf_ids.c
> +++ b/kernel/bpf/btf_ids.c
> @@ -13,3 +13,14 @@ BTF_ID(struct, seq_file)
>
> BTF_ID_LIST(bpf_xdp_output_btf_ids)
> BTF_ID(struct, xdp_buff)
> +
> +BTF_ID_LIST(bpf_d_path_btf_ids)
> +BTF_ID(struct, path)
> +
> +BTF_WHITELIST_ENTRY(btf_whitelist_d_path)
> +BTF_ID(func, vfs_truncate)
> +BTF_ID(func, vfs_fallocate)
> +BTF_ID(func, dentry_open)
> +BTF_ID(func, vfs_getattr)
> +BTF_ID(func, filp_close)
> +BTF_WHITELIST_END(btf_whitelist_d_path)
Oh, so that's why you added btf_ids.c. Do you think centralizing all
those BTF ID lists in one file is going to be more convenient? I lean
towards keeping them closer to where they are used, as it was with all
those helper BTF IDS. But I wonder what others think...
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index c1866d76041f..0ff5d8434d40 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1016,6 +1016,42 @@ static const struct bpf_func_proto bpf_send_signal_thread_proto = {
> .arg1_type = ARG_ANYTHING,
> };
>
[...]
^ permalink raw reply
* Re: [PATCH 10/11] selftests/bpf: Add verifier test for d_path helper
From: Andrii Nakryiko @ 2020-06-19 4:38 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Networking, bpf, Song Liu,
Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
Wenbo Zhang, KP Singh, Andrii Nakryiko, Brendan Gregg,
Florent Revest, Al Viro
In-Reply-To: <20200616100512.2168860-11-jolsa@kernel.org>
On Tue, Jun 16, 2020 at 3:06 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding verifier test for attaching tracing program and
> calling d_path helper from within and testing that it's
> allowed for dentry_open function and denied for 'd_path'
> function with appropriate error.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/testing/selftests/bpf/test_verifier.c | 13 ++++++-
> tools/testing/selftests/bpf/verifier/d_path.c | 38 +++++++++++++++++++
> 2 files changed, 50 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/verifier/d_path.c
>
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 78a6bae56ea6..3cce3dc766a2 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -114,6 +114,7 @@ struct bpf_test {
> bpf_testdata_struct_t retvals[MAX_TEST_RUNS];
> };
> enum bpf_attach_type expected_attach_type;
> + const char *kfunc;
> };
>
> /* Note we want this to be 64 bit aligned so that the end of our array is
> @@ -984,8 +985,18 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
> attr.log_level = 4;
> attr.prog_flags = pflags;
>
> + if (prog_type == BPF_PROG_TYPE_TRACING && test->kfunc) {
> + attr.attach_btf_id = libbpf_find_vmlinux_btf_id(test->kfunc,
> + attr.expected_attach_type);
if (!attr.attach_btf_id)
emit more meaningful error, than later during load?
> + }
> +
> fd_prog = bpf_load_program_xattr(&attr, bpf_vlog, sizeof(bpf_vlog));
> - if (fd_prog < 0 && !bpf_probe_prog_type(prog_type, 0)) {
> +
> + /* BPF_PROG_TYPE_TRACING requires more setup and
> + * bpf_probe_prog_type won't give correct answer
> + */
> + if (fd_prog < 0 && (prog_type != BPF_PROG_TYPE_TRACING) &&
nit: () are redundant
> + !bpf_probe_prog_type(prog_type, 0)) {
> printf("SKIP (unsupported program type %d)\n", prog_type);
> skips++;
> goto close_fds;
> diff --git a/tools/testing/selftests/bpf/verifier/d_path.c b/tools/testing/selftests/bpf/verifier/d_path.c
> new file mode 100644
> index 000000000000..e08181abc056
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/verifier/d_path.c
> @@ -0,0 +1,38 @@
> +{
> + "d_path accept",
> + .insns = {
> + BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_1, 0),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> + BPF_MOV64_IMM(BPF_REG_6, 0),
> + BPF_STX_MEM(BPF_DW, BPF_REG_2, BPF_REG_6, 0),
> + BPF_LD_IMM64(BPF_REG_3, 8),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_d_path),
> + BPF_MOV64_IMM(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + },
> + .errstr = "R0 max value is outside of the array range",
> + .result = ACCEPT,
accept with error string expected?
> + .prog_type = BPF_PROG_TYPE_TRACING,
> + .expected_attach_type = BPF_TRACE_FENTRY,
> + .kfunc = "dentry_open",
> +},
> +{
> + "d_path reject",
> + .insns = {
> + BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_1, 0),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> + BPF_MOV64_IMM(BPF_REG_6, 0),
> + BPF_STX_MEM(BPF_DW, BPF_REG_2, BPF_REG_6, 0),
> + BPF_LD_IMM64(BPF_REG_3, 8),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_d_path),
> + BPF_MOV64_IMM(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + },
> + .errstr = "helper call is not allowed in probe",
> + .result = REJECT,
> + .prog_type = BPF_PROG_TYPE_TRACING,
> + .expected_attach_type = BPF_TRACE_FENTRY,
> + .kfunc = "d_path",
> +},
> --
> 2.25.4
>
^ permalink raw reply
* Re: [PATCH net] net: increment xmit_recursion level in dev_direct_xmit()
From: Eric Dumazet @ 2020-06-19 4:42 UTC (permalink / raw)
To: David Miller, edumazet; +Cc: netdev, eric.dumazet, kuba, syzkaller
In-Reply-To: <20200618.204830.1094276610079682944.davem@davemloft.net>
On 6/18/20 8:48 PM, David Miller wrote:
> From: Eric Dumazet <edumazet@google.com>
> Date: Wed, 17 Jun 2020 22:23:25 -0700
>
>> Back in commit f60e5990d9c1 ("ipv6: protect skb->sk accesses
>> from recursive dereference inside the stack") Hannes added code
>> so that IPv6 stack would not trust skb->sk for typical cases
>> where packet goes through 'standard' xmit path (__dev_queue_xmit())
>>
>> Alas af_packet had a dev_direct_xmit() path that was not
>> dealing yet with xmit_recursion level.
>>
>> Also change sk_mc_loop() to dump a stack once only.
>>
>> Without this patch, syzbot was able to trigger :
> ...
>> f60e5990d9c1 ("ipv6: protect skb->sk accesses from recursive dereference inside the stack")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Reported-by: syzbot <syzkaller@googlegroups.com>
>
> Applied and queued up for -stable.
>
> I only noticed after pushing this out the missing "Fixes: " prefix, but not
> much I can do about this now sorry :-/
Oh right, sorry for this.
^ permalink raw reply
* Re: [PATCH 11/11] selftests/bpf: Add test for d_path helper
From: Andrii Nakryiko @ 2020-06-19 4:44 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Wenbo Zhang, Networking, bpf,
Song Liu, Yonghong Song, Martin KaFai Lau, David Miller,
John Fastabend, KP Singh, Andrii Nakryiko, Brendan Gregg,
Florent Revest, Al Viro
In-Reply-To: <20200616100512.2168860-12-jolsa@kernel.org>
On Tue, Jun 16, 2020 at 3:07 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding test for d_path helper which is pretty much
> copied from Wenbo Zhang's test for bpf_get_fd_path,
> which never made it in.
>
> I've failed so far to compile the test with <linux/fs.h>
> kernel header, so for now adding 'struct file' with f_path
> member that has same offset as kernel's file object.
>
> Original-patch-by: Wenbo Zhang <ethercflow@gmail.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> .../testing/selftests/bpf/prog_tests/d_path.c | 153 ++++++++++++++++++
> .../testing/selftests/bpf/progs/test_d_path.c | 55 +++++++
> 2 files changed, 208 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/d_path.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_d_path.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
> new file mode 100644
> index 000000000000..e2b7dfeb506f
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> @@ -0,0 +1,153 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <test_progs.h>
> +#include <sys/stat.h>
> +#include <linux/sched.h>
> +#include <sys/syscall.h>
> +
> +#define MAX_PATH_LEN 128
> +#define MAX_FILES 7
> +#define MAX_EVENT_NUM 16
> +
> +struct d_path_test_data {
> + pid_t pid;
> + __u32 cnt_stat;
> + __u32 cnt_close;
> + char paths_stat[MAX_EVENT_NUM][MAX_PATH_LEN];
> + char paths_close[MAX_EVENT_NUM][MAX_PATH_LEN];
> +};
with skeleton there is no point in defining this container struct, and
especially duplicating it between BPF code and user-space code. Just
declare all those fields as global variables and access them from
skeleton directly.
[...]
> diff --git a/tools/testing/selftests/bpf/progs/test_d_path.c b/tools/testing/selftests/bpf/progs/test_d_path.c
> new file mode 100644
> index 000000000000..1b478c00ee7a
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_d_path.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +#define MAX_PATH_LEN 128
> +#define MAX_EVENT_NUM 16
> +
> +static struct d_path_test_data {
> + pid_t pid;
> + __u32 cnt_stat;
> + __u32 cnt_close;
> + char paths_stat[MAX_EVENT_NUM][MAX_PATH_LEN];
> + char paths_close[MAX_EVENT_NUM][MAX_PATH_LEN];
> +} data;
> +
> +struct path;
> +struct kstat;
both structs are in vmlinux.h, you shouldn't need this.
[...]
>
^ permalink raw reply
* [PATCH net 0/2] net: phy: MDIO bus scanning fixes
From: Florian Fainelli @ 2020-06-19 4:47 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Jakub Kicinski, Rob Herring, Frank Rowand,
Dajun Jin, Alexandre Belloni, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
Hi all,
This patch series fixes two problems with the current MDIO bus scanning
logic which was identified while moving from 4.9 to 5.4 on devices that
do rely on scanning the MDIO bus at runtime because they use pluggable
cards.
Florian Fainelli (2):
of: of_mdio: Correct loop scanning logic
net: phy: Check harder for errors in get_phy_id()
drivers/net/phy/phy_device.c | 6 ++++--
drivers/of/of_mdio.c | 5 +++--
2 files changed, 7 insertions(+), 4 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net 1/2] of: of_mdio: Correct loop scanning logic
From: Florian Fainelli @ 2020-06-19 4:47 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Jakub Kicinski, Rob Herring, Frank Rowand,
Dajun Jin, Alexandre Belloni, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
In-Reply-To: <20200619044759.11387-1-f.fainelli@gmail.com>
Commit 209c65b61d94 ("drivers/of/of_mdio.c:fix of_mdiobus_register()")
introduced a break of the loop on the premise that a successful
registration should exit the loop. The premise is correct but not to
code, because rc && rc != -ENODEV is just a special error condition,
that means we would exit the loop even with rc == -ENODEV which is
absolutely not correct since this is the error code to indicate to the
MDIO bus layer that scanning should continue.
Fix this by explicitly checking for rc = 0 as the only valid condition
to break out of the loop.
Fixes: 209c65b61d94 ("drivers/of/of_mdio.c:fix of_mdiobus_register()")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/of/of_mdio.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index a04afe79529c..7496dc64d6b5 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -315,9 +315,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
if (of_mdiobus_child_is_phy(child)) {
rc = of_mdiobus_register_phy(mdio, child, addr);
- if (rc && rc != -ENODEV)
+ if (!rc)
+ break;
+ if (rc != -ENODEV)
goto unregister;
- break;
}
}
}
--
2.17.1
^ permalink raw reply related
* [PATCH net 2/2] net: phy: Check harder for errors in get_phy_id()
From: Florian Fainelli @ 2020-06-19 4:47 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Jakub Kicinski, Rob Herring, Frank Rowand,
Dajun Jin, Alexandre Belloni, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
In-Reply-To: <20200619044759.11387-1-f.fainelli@gmail.com>
Commit 02a6efcab675 ("net: phy: allow scanning busses with missing
phys") added a special condition to return -ENODEV in case -ENODEV or
-EIO was returned from the first read of the MII_PHYSID1 register.
In case the MDIO bus data line pull-up is not strong enough, the MDIO
bus controller will not flag this as a read error. This can happen when
a pluggable daughter card is not connected and weak internal pull-ups
are used (since that is the only option, otherwise the pins are
floating).
The second read of MII_PHYSID2 will be correctly flagged an error
though, but now we will return -EIO which will be treated as a hard
error, thus preventing MDIO bus scanning loops to continue succesfully.
Apply the same logic to both register reads, thus allowing the scanning
logic to proceed.
Fixes: 02a6efcab675 ("net: phy: allow scanning busses with missing phys")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/phy_device.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 04946de74fa0..85ba95b598b5 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -794,8 +794,10 @@ static int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id,
/* Grab the bits from PHYIR2, and put them in the lower half */
phy_reg = mdiobus_read(bus, addr, MII_PHYSID2);
- if (phy_reg < 0)
- return -EIO;
+ if (phy_reg < 0) {
+ /* returning -ENODEV doesn't stop bus scanning */
+ return (phy_reg == -EIO || phy_reg == -ENODEV) ? -ENODEV : -EIO;
+ }
*phy_id |= phy_reg;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v5 2/3] dt-bindings: net: mscc-vsc8531: add optional clock properties
From: Florian Fainelli @ 2020-06-19 5:01 UTC (permalink / raw)
To: Heiko Stuebner, davem, kuba
Cc: robh+dt, andrew, hkallweit1, linux, netdev, devicetree,
linux-kernel, christoph.muellner, Heiko Stuebner
In-Reply-To: <20200618121139.1703762-3-heiko@sntech.de>
On 6/18/2020 5:11 AM, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>
> Some mscc ethernet phys have a configurable clock output, so describe the
> generic properties to access them in devicetrees.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
> Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> index 5ff37c68c941..67625ba27f53 100644
> --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> @@ -1,6 +1,8 @@
> * Microsemi - vsc8531 Giga bit ethernet phy
>
> Optional properties:
> +- clock-output-names : Name for the exposed clock output
> +- #clock-cells : should be 0
> - vsc8531,vddmac : The vddmac in mV. Allowed values is listed
> in the first row of Table 1 (below).
> This property is only used in combination
>
With that approach, you also need to be careful as a driver writer to
ensure that you have at least probed the MDIO bus to ensure that the PHY
device has been created (and therefore it is available as a clock
provider) if that same Ethernet MAC is a consumer of that clock (which
it appears to be). Otherwise you may just never probe and be trapped in
a circular dependency.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v8 4/5] net: dp83869: Add RGMII internal delay configuration
From: kernel test robot @ 2020-06-19 5:14 UTC (permalink / raw)
To: Dan Murphy, andrew, f.fainelli, hkallweit1, davem, robh
Cc: kbuild-all, clang-built-linux, netdev, linux-kernel, devicetree,
Dan Murphy
In-Reply-To: <20200618211011.28837-5-dmurphy@ti.com>
[-- Attachment #1: Type: text/plain, Size: 7249 bytes --]
Hi Dan,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Dan-Murphy/RGMII-Internal-delay-common-property/20200619-051238
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git cb8e59cc87201af93dfbb6c3dccc8fcad72a09c2
config: s390-randconfig-r014-20200618 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 487ca07fcc75d52755c9fe2ee05bcb3b6eeeec44)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : ^
include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x0000ff00UL) << 8) | ^
In file included from drivers/net/phy/dp83869.c:6:
In file included from include/linux/ethtool.h:18:
In file included from include/uapi/linux/ethtool.h:19:
In file included from include/linux/if_ether.h:19:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:492:45: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu(__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : ^
include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | ^
In file included from drivers/net/phy/dp83869.c:6:
In file included from include/linux/ethtool.h:18:
In file included from include/uapi/linux/ethtool.h:19:
In file included from include/linux/if_ether.h:19:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:492:45: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu(__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : ^
include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0xff000000UL) >> 24)))
^
In file included from drivers/net/phy/dp83869.c:6:
In file included from include/linux/ethtool.h:18:
In file included from include/uapi/linux/ethtool.h:19:
In file included from include/linux/if_ether.h:19:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:492:45: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu(__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
__fswab32(x))
^
In file included from drivers/net/phy/dp83869.c:6:
In file included from include/linux/ethtool.h:18:
In file included from include/uapi/linux/ethtool.h:19:
In file included from include/linux/if_ether.h:19:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/s390/include/asm/io.h:72:
include/asm-generic/io.h:503:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:513:46: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew(cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:523:46: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel(cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:585:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:593:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:601:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:610:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:619:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:628:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/net/phy/dp83869.c:103:18: warning: unused variable 'dp83869_internal_delay' [-Wunused-const-variable]
static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
^
21 warnings generated.
vim +/dp83869_internal_delay +103 drivers/net/phy/dp83869.c
102
> 103 static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
104 1750, 2000, 2250, 2500, 2750, 3000,
105 3250, 3500, 3750, 4000};
106
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23458 bytes --]
^ permalink raw reply
* Re: [PATCH] bpf: Allow small structs to be type of function argument
From: Yonghong Song @ 2020-06-19 5:39 UTC (permalink / raw)
To: Alexei Starovoitov, John Fastabend
Cc: Andrii Nakryiko, Jiri Olsa, Andrii Nakryiko, Jiri Olsa,
Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
Martin KaFai Lau, Jakub Kicinski, David Miller,
Jesper Dangaard Brouer, KP Singh, Masanori Misono
In-Reply-To: <CAADnVQLGNUcDWmrgUBmdcgLMfUNf=-3yroA8a+b7s+Ki5Tb4Jg@mail.gmail.com>
On 6/18/20 7:04 PM, Alexei Starovoitov wrote:
> On Thu, Jun 18, 2020 at 5:26 PM John Fastabend <john.fastabend@gmail.com> wrote:
>>
>> foo(int a, __int128 b)
>>
>> would put a in r0 and b in r2 and r3 leaving a hole in r1. But that
>> was some old reference manual and might no longer be the case
This should not happen if clang compilation with -target bpf.
This MAY happen if they compile with 'clang -target riscv' as the IR
could change before coming to bpf backend.
>> in reality. Perhaps just spreading hearsay, but the point is we
>> should say something about what the BPF backend convention
>> is and write it down. We've started to bump into these things
>> lately.
>
> calling convention for int128 in bpf is _undefined_.
> calling convention for struct by value in bpf is also _undefined_.
Just to clarify a little bit. bpf backend did not do anything
special about int128 and struct type. It is using llvm default.
That is, int128 using two argument registers and struct passed
by address. But I do see some other architectures having their
own ways to handle these parameters like X86, AARCH64, AMDGPU, MIPS.
int128 is not widely used. passing struct as the argument is not
a good practice. So Agree with Alexei is not really worthwhile to
handle them in the place of arguments.
>
> In many cases the compiler has to have the backend code
> so other parts of the compiler can function.
> I didn't bother explicitly disabling every undefined case.
> Please don't read too much into llvm generated code.
>
^ permalink raw reply
* bpf test error: KASAN: use-after-free Write in afs_wake_up_async_call
From: syzbot @ 2020-06-19 5:46 UTC (permalink / raw)
To: ast, daniel, dhowells, linux-afs, linux-kernel, netdev,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: ef7232da ionic: export features for vlans to use
git tree: bpf
console output: https://syzkaller.appspot.com/x/log.txt?x=173214d1100000
kernel config: https://syzkaller.appspot.com/x/.config?x=b8ad29058cb749bc
dashboard link: https://syzkaller.appspot.com/bug?extid=0710b20f5412c027fefb
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+0710b20f5412c027fefb@syzkaller.appspotmail.com
tipc: TX() has been purged, node left!
==================================================================
BUG: KASAN: use-after-free in afs_wake_up_async_call+0x6aa/0x770 fs/afs/rxrpc.c:707
Write of size 1 at addr ffff88809a0449e4 by task kworker/u4:0/7
CPU: 0 PID: 7 Comm: kworker/u4:0 Not tainted 5.8.0-rc1-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: netns cleanup_net
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x18f/0x20d lib/dump_stack.c:118
print_address_description.constprop.0.cold+0xd3/0x413 mm/kasan/report.c:383
__kasan_report mm/kasan/report.c:513 [inline]
kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
afs_wake_up_async_call+0x6aa/0x770 fs/afs/rxrpc.c:707
rxrpc_notify_socket+0x1db/0x5d0 net/rxrpc/recvmsg.c:40
__rxrpc_set_call_completion.part.0+0x172/0x410 net/rxrpc/recvmsg.c:76
__rxrpc_call_completed net/rxrpc/recvmsg.c:112 [inline]
rxrpc_call_completed+0xca/0xf0 net/rxrpc/recvmsg.c:111
rxrpc_discard_prealloc+0x781/0xab0 net/rxrpc/call_accept.c:233
rxrpc_listen+0x147/0x360 net/rxrpc/af_rxrpc.c:245
afs_close_socket+0x95/0x320 fs/afs/rxrpc.c:110
afs_net_exit+0x1bc/0x310 fs/afs/main.c:155
ops_exit_list.isra.0+0xa8/0x150 net/core/net_namespace.c:186
cleanup_net+0x511/0xa50 net/core/net_namespace.c:603
process_one_work+0x965/0x1690 kernel/workqueue.c:2269
worker_thread+0x96/0xe10 kernel/workqueue.c:2415
kthread+0x3b5/0x4a0 kernel/kthread.c:291
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
Allocated by task 6807:
save_stack+0x1b/0x40 mm/kasan/common.c:48
set_track mm/kasan/common.c:56 [inline]
__kasan_kmalloc mm/kasan/common.c:494 [inline]
__kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:467
kmem_cache_alloc_trace+0x153/0x7d0 mm/slab.c:3551
kmalloc include/linux/slab.h:555 [inline]
kzalloc include/linux/slab.h:669 [inline]
afs_alloc_call+0x55/0x630 fs/afs/rxrpc.c:141
afs_charge_preallocation+0xe9/0x2d0 fs/afs/rxrpc.c:757
afs_open_socket+0x292/0x360 fs/afs/rxrpc.c:92
afs_net_init+0xa6c/0xe30 fs/afs/main.c:125
ops_init+0xaf/0x420 net/core/net_namespace.c:151
setup_net+0x2de/0x860 net/core/net_namespace.c:341
copy_net_ns+0x293/0x590 net/core/net_namespace.c:482
create_new_namespaces+0x3fb/0xb30 kernel/nsproxy.c:110
unshare_nsproxy_namespaces+0xbd/0x1f0 kernel/nsproxy.c:231
ksys_unshare+0x43d/0x8e0 kernel/fork.c:2983
__do_sys_unshare kernel/fork.c:3051 [inline]
__se_sys_unshare kernel/fork.c:3049 [inline]
__x64_sys_unshare+0x2d/0x40 kernel/fork.c:3049
do_syscall_64+0x60/0xe0 arch/x86/entry/common.c:359
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Freed by task 7:
save_stack+0x1b/0x40 mm/kasan/common.c:48
set_track mm/kasan/common.c:56 [inline]
kasan_set_free_info mm/kasan/common.c:316 [inline]
__kasan_slab_free+0xf7/0x140 mm/kasan/common.c:455
__cache_free mm/slab.c:3426 [inline]
kfree+0x109/0x2b0 mm/slab.c:3757
afs_put_call+0x585/0xa40 fs/afs/rxrpc.c:190
rxrpc_discard_prealloc+0x764/0xab0 net/rxrpc/call_accept.c:230
rxrpc_listen+0x147/0x360 net/rxrpc/af_rxrpc.c:245
afs_close_socket+0x95/0x320 fs/afs/rxrpc.c:110
afs_net_exit+0x1bc/0x310 fs/afs/main.c:155
ops_exit_list.isra.0+0xa8/0x150 net/core/net_namespace.c:186
cleanup_net+0x511/0xa50 net/core/net_namespace.c:603
process_one_work+0x965/0x1690 kernel/workqueue.c:2269
worker_thread+0x96/0xe10 kernel/workqueue.c:2415
kthread+0x3b5/0x4a0 kernel/kthread.c:291
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
The buggy address belongs to the object at ffff88809a044800
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 484 bytes inside of
1024-byte region [ffff88809a044800, ffff88809a044c00)
The buggy address belongs to the page:
page:ffffea0002681100 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0
flags: 0xfffe0000000200(slab)
raw: 00fffe0000000200 ffffea000291db08 ffffea00028c27c8 ffff8880aa000c40
raw: 0000000000000000 ffff88809a044000 0000000100000002 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff88809a044880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88809a044900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88809a044980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88809a044a00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88809a044a80: 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#status for how to communicate with syzbot.
^ permalink raw reply
* RE: [PATCH/RFC] net: ethernet: ravb: Try to wake subqueue instead of stop on timeout
From: Yoshihiro Shimoda @ 2020-06-19 5:46 UTC (permalink / raw)
To: Sergei Shtylyov, sergei.shtylyov@cogentembedded.com,
davem@davemloft.net, kuba@kernel.org
Cc: REE dirk.behme@de.bosch.com, Shashikant.Suguni@in.bosch.com,
netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <9f71873b-ee5e-7ae3-8a5a-caf7bf38a68e@gmail.com>
Hello!
> From: Sergei Shtylyov, Sent: Monday, June 15, 2020 5:13 PM
>
> Hello!
>
> On 15.06.2020 8:58, Yoshihiro Shimoda wrote:
>
> >> From: Yoshihiro Shimoda, Sent: Tuesday, May 26, 2020 6:47 PM
> >>
> >> According to the report of [1], this driver is possible to cause
> >> the following error in ravb_tx_timeout_work().
> >>
> >> ravb e6800000.ethernet ethernet: failed to switch device to config mode
> >>
> >> This error means that the hardware could not change the state
> >> from "Operation" to "Configuration" while some tx queue is operating.
> >> After that, ravb_config() in ravb_dmac_init() will fail, and then
> >> any descriptors will be not allocaled anymore so that NULL porinter
> >> dereference happens after that on ravb_start_xmit().
> >>
> >> Such a case is possible to be caused because this driver supports
> >> two queues (NC and BE) and the ravb_stop_dma() is possible to return
> >> without any stopping process if TCCR or CSR register indicates
> >> the hardware is operating for TX.
> >>
> >> To fix the issue, just try to wake the subqueue on
> >> ravb_tx_timeout_work() if the descriptors are not full instead
> >> of stop all transfers (all queues of TX and RX).
> >>
> >> [1]
> >> https://lore.kernel.org/linux-renesas-soc/20200518045452.2390-1-dirk.behme@de.bosch.com/
> >>
> >> Reported-by: Dirk Behme <dirk.behme@de.bosch.com>
> >> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >> ---
> >> I'm guessing that this issue is possible to happen if:
> >> - ravb_start_xmit() calls netif_stop_subqueue(), and
> >> - ravb_poll() will not be called with some reason, and
> >> - netif_wake_subqueue() will be not called, and then
> >> - dev_watchdog() in net/sched/sch_generic.c calls ndo_tx_timeout().
> >>
> >> However, unfortunately, I didn't reproduce the issue yet.
> >> To be honest, I'm also guessing other queues (SR) of this hardware
> >> which out-of tree driver manages are possible to reproduce this issue,
> >> but I didn't try such environment for now...
> >>
> >> So, I marked RFC on this patch now.
> >
> > I'm afraid, but do you have any comments about this patch?
>
> I agree that we should now reset only the stuck queue, not both but I
> doubt your solution is good enough. Let me have another look...
Thank you for your comment! I hope this solution is good enough...
By the way, there is other topic though, I'm thinking we should not call
ravb_{close,open}() in ravb_{suspend,resume}() because this is possible to
a race condition between ifconfig up/down and system suspend/resume.
In other words, I'm thinking this should not free/reallocate
dma memory while netif_running is true. But, what do you think?
Best regards,
Yoshihiro Shimoda
> [...]
>
> MBR, Sergei
^ permalink raw reply
* [v2,net-next] net: qos offload add flow status with dropped count
From: Po Liu @ 2020-06-19 6:01 UTC (permalink / raw)
To: davem, linux-kernel, netdev, jiri
Cc: vinicius.gomes, vlad, claudiu.manoil, vladimir.oltean,
alexandru.marginean, michael.chan, vishal, saeedm, leon, jiri,
idosch, alexandre.belloni, UNGLinuxDriver, kuba, jhs,
xiyou.wangcong, simon.horman, pablo, moshe, m-karicheri2,
andre.guedes, stephen, Po Liu
In-Reply-To: <20200324034745.30979-1-Po.Liu@nxp.com>
From: Po Liu <Po.Liu@nxp.com>
This patch adds a drop frames counter to tc flower offloading.
Reporting h/w dropped frames is necessary for some actions.
Some actions like police action and the coming introduced stream gate
action would produce dropped frames which is necessary for user. Status
update shows how many filtered packets increasing and how many dropped
in those packets.
v2: Changes
- Update commit comments suggest by Jiri Pirko.
Signed-off-by: Po Liu <Po.Liu@nxp.com>
---
This patch is continue the thread 20200324034745.30979-1-Po.Liu@nxp.com
drivers/net/dsa/sja1105/sja1105_vl.c | 2 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 2 +-
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 2 +-
.../net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c | 2 +-
drivers/net/ethernet/freescale/enetc/enetc_qos.c | 7 +++++--
drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++--
drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c | 2 +-
drivers/net/ethernet/mscc/ocelot_flower.c | 2 +-
drivers/net/ethernet/netronome/nfp/flower/offload.c | 2 +-
drivers/net/ethernet/netronome/nfp/flower/qos_conf.c | 2 +-
include/net/act_api.h | 11 ++++++-----
include/net/flow_offload.h | 5 ++++-
include/net/pkt_cls.h | 5 +++--
net/sched/act_api.c | 10 ++++------
net/sched/act_ct.c | 6 +++---
net/sched/act_gact.c | 7 ++++---
net/sched/act_gate.c | 6 +++---
net/sched/act_mirred.c | 6 +++---
net/sched/act_pedit.c | 6 +++---
net/sched/act_police.c | 4 ++--
net/sched/act_skbedit.c | 5 +++--
net/sched/act_vlan.c | 6 +++---
net/sched/cls_flower.c | 1 +
net/sched/cls_matchall.c | 3 ++-
25 files changed, 60 insertions(+), 50 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index bdfd6c4e190d..9ddc49b7eb8f 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -771,7 +771,7 @@ int sja1105_vl_stats(struct sja1105_private *priv, int port,
pkts = timingerr + unreleased + lengtherr;
- flow_stats_update(stats, 0, pkts - rule->vl.stats.pkts,
+ flow_stats_update(stats, 0, pkts - rule->vl.stats.pkts, 0,
jiffies - rule->vl.stats.lastused,
FLOW_ACTION_HW_STATS_IMMEDIATE);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index 0eef4f5e4a46..4d482d75a20b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -1638,7 +1638,7 @@ static int bnxt_tc_get_flow_stats(struct bnxt *bp,
lastused = flow->lastused;
spin_unlock(&flow->stats_lock);
- flow_stats_update(&tc_flow_cmd->stats, stats.bytes, stats.packets,
+ flow_stats_update(&tc_flow_cmd->stats, stats.bytes, stats.packets, 0,
lastused, FLOW_ACTION_HW_STATS_DELAYED);
return 0;
}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
index 4a5fa9eba0b6..030de20a5d27 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
@@ -902,7 +902,7 @@ int cxgb4_tc_flower_stats(struct net_device *dev,
if (ofld_stats->prev_packet_count != packets)
ofld_stats->last_used = jiffies;
flow_stats_update(&cls->stats, bytes - ofld_stats->byte_count,
- packets - ofld_stats->packet_count,
+ packets - ofld_stats->packet_count, 0,
ofld_stats->last_used,
FLOW_ACTION_HW_STATS_IMMEDIATE);
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
index c88c47a14fbb..c439b5bce9c9 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c
@@ -346,7 +346,7 @@ int cxgb4_tc_matchall_stats(struct net_device *dev,
flow_stats_update(&cls_matchall->stats,
bytes - tc_port_matchall->ingress.bytes,
packets - tc_port_matchall->ingress.packets,
- tc_port_matchall->ingress.last_used,
+ 0, tc_port_matchall->ingress.last_used,
FLOW_ACTION_HW_STATS_IMMEDIATE);
tc_port_matchall->ingress.packets = packets;
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index fd3df19eaa32..fb76903eca90 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -1291,12 +1291,15 @@ static int enetc_psfp_get_stats(struct enetc_ndev_priv *priv,
spin_lock(&epsfp.psfp_lock);
stats.pkts = counters.matching_frames_count - filter->stats.pkts;
+ stats.drops = counters.not_passing_frames_count -
+ filter->stats.drops;
stats.lastused = filter->stats.lastused;
filter->stats.pkts += stats.pkts;
+ filter->stats.drops += stats.drops;
spin_unlock(&epsfp.psfp_lock);
- flow_stats_update(&f->stats, 0x0, stats.pkts, stats.lastused,
- FLOW_ACTION_HW_STATS_DELAYED);
+ flow_stats_update(&f->stats, 0x0, stats.pkts, stats.drops,
+ stats.lastused, FLOW_ACTION_HW_STATS_DELAYED);
return 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
index 430025550fad..c7107da03212 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
@@ -672,7 +672,7 @@ mlx5_tc_ct_block_flow_offload_stats(struct mlx5_ct_ft *ft,
return -ENOENT;
mlx5_fc_query_cached(entry->counter, &bytes, &packets, &lastuse);
- flow_stats_update(&f->stats, bytes, packets, lastuse,
+ flow_stats_update(&f->stats, bytes, packets, 0, lastuse,
FLOW_ACTION_HW_STATS_DELAYED);
return 0;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 7fc84f58e28a..bc9c0ac15f99 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -4828,7 +4828,7 @@ int mlx5e_stats_flower(struct net_device *dev, struct mlx5e_priv *priv,
no_peer_counter:
mlx5_devcom_release_peer_data(devcom, MLX5_DEVCOM_ESW_OFFLOADS);
out:
- flow_stats_update(&f->stats, bytes, packets, lastuse,
+ flow_stats_update(&f->stats, bytes, packets, 0, lastuse,
FLOW_ACTION_HW_STATS_DELAYED);
trace_mlx5e_stats_flower(f);
errout:
@@ -4946,7 +4946,7 @@ void mlx5e_tc_stats_matchall(struct mlx5e_priv *priv,
dpkts = cur_stats.rx_packets - rpriv->prev_vf_vport_stats.rx_packets;
dbytes = cur_stats.rx_bytes - rpriv->prev_vf_vport_stats.rx_bytes;
rpriv->prev_vf_vport_stats = cur_stats;
- flow_stats_update(&ma->stats, dbytes, dpkts, jiffies,
+ flow_stats_update(&ma->stats, dbytes, dpkts, 0, jiffies,
FLOW_ACTION_HW_STATS_DELAYED);
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 51e1b3930c56..61d21043d83a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -633,7 +633,7 @@ int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
if (err)
goto err_rule_get_stats;
- flow_stats_update(&f->stats, bytes, packets, lastuse, used_hw_stats);
+ flow_stats_update(&f->stats, bytes, packets, 0, lastuse, used_hw_stats);
mlxsw_sp_acl_ruleset_put(mlxsw_sp, ruleset);
return 0;
diff --git a/drivers/net/ethernet/mscc/ocelot_flower.c b/drivers/net/ethernet/mscc/ocelot_flower.c
index 5ce172e22b43..c90bafbd651f 100644
--- a/drivers/net/ethernet/mscc/ocelot_flower.c
+++ b/drivers/net/ethernet/mscc/ocelot_flower.c
@@ -244,7 +244,7 @@ int ocelot_cls_flower_stats(struct ocelot *ocelot, int port,
if (ret)
return ret;
- flow_stats_update(&f->stats, 0x0, ace.stats.pkts, 0x0,
+ flow_stats_update(&f->stats, 0x0, ace.stats.pkts, 0, 0x0,
FLOW_ACTION_HW_STATS_IMMEDIATE);
return 0;
}
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 695d24b9dd92..234c652700e1 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -1491,7 +1491,7 @@ nfp_flower_get_stats(struct nfp_app *app, struct net_device *netdev,
nfp_flower_update_merge_stats(app, nfp_flow);
flow_stats_update(&flow->stats, priv->stats[ctx_id].bytes,
- priv->stats[ctx_id].pkts, priv->stats[ctx_id].used,
+ priv->stats[ctx_id].pkts, 0, priv->stats[ctx_id].used,
FLOW_ACTION_HW_STATS_DELAYED);
priv->stats[ctx_id].pkts = 0;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c b/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
index d18a830e4264..bb327d48d1ab 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
@@ -319,7 +319,7 @@ nfp_flower_stats_rate_limiter(struct nfp_app *app, struct net_device *netdev,
prev_stats->bytes = curr_stats->bytes;
spin_unlock_bh(&fl_priv->qos_stats_lock);
- flow_stats_update(&flow->stats, diff_bytes, diff_pkts,
+ flow_stats_update(&flow->stats, diff_bytes, diff_pkts, 0,
repr_priv->qos_table.last_update,
FLOW_ACTION_HW_STATS_DELAYED);
return 0;
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 8c3934880670..cb382a89ea58 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -106,7 +106,7 @@ struct tc_action_ops {
struct netlink_callback *, int,
const struct tc_action_ops *,
struct netlink_ext_ack *);
- void (*stats_update)(struct tc_action *, u64, u32, u64, bool);
+ void (*stats_update)(struct tc_action *, u64, u64, u64, u64, bool);
size_t (*get_fill_size)(const struct tc_action *act);
struct net_device *(*get_dev)(const struct tc_action *a,
tc_action_priv_destructor *destructor);
@@ -232,8 +232,8 @@ static inline void tcf_action_inc_overlimit_qstats(struct tc_action *a)
spin_unlock(&a->tcfa_lock);
}
-void tcf_action_update_stats(struct tc_action *a, u64 bytes, u32 packets,
- bool drop, bool hw);
+void tcf_action_update_stats(struct tc_action *a, u64 bytes, u64 packets,
+ u64 drops, bool hw);
int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int);
int tcf_action_check_ctrlact(int action, struct tcf_proto *tp,
@@ -244,13 +244,14 @@ struct tcf_chain *tcf_action_set_ctrlact(struct tc_action *a, int action,
#endif /* CONFIG_NET_CLS_ACT */
static inline void tcf_action_stats_update(struct tc_action *a, u64 bytes,
- u64 packets, u64 lastuse, bool hw)
+ u64 packets, u64 drops,
+ u64 lastuse, bool hw)
{
#ifdef CONFIG_NET_CLS_ACT
if (!a->ops->stats_update)
return;
- a->ops->stats_update(a, bytes, packets, lastuse, hw);
+ a->ops->stats_update(a, bytes, packets, drops, lastuse, hw);
#endif
}
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index f2c8311a0433..00c15f14c434 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -389,17 +389,20 @@ static inline bool flow_rule_match_key(const struct flow_rule *rule,
struct flow_stats {
u64 pkts;
u64 bytes;
+ u64 drops;
u64 lastused;
enum flow_action_hw_stats used_hw_stats;
bool used_hw_stats_valid;
};
static inline void flow_stats_update(struct flow_stats *flow_stats,
- u64 bytes, u64 pkts, u64 lastused,
+ u64 bytes, u64 pkts,
+ u64 drops, u64 lastused,
enum flow_action_hw_stats used_hw_stats)
{
flow_stats->pkts += pkts;
flow_stats->bytes += bytes;
+ flow_stats->drops += drops;
flow_stats->lastused = max_t(u64, flow_stats->lastused, lastused);
/* The driver should pass value with a maximum of one bit set.
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index ed65619cbc47..ff017e5b3ea2 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -262,7 +262,7 @@ static inline void tcf_exts_put_net(struct tcf_exts *exts)
static inline void
tcf_exts_stats_update(const struct tcf_exts *exts,
- u64 bytes, u64 packets, u64 lastuse,
+ u64 bytes, u64 packets, u64 drops, u64 lastuse,
u8 used_hw_stats, bool used_hw_stats_valid)
{
#ifdef CONFIG_NET_CLS_ACT
@@ -273,7 +273,8 @@ tcf_exts_stats_update(const struct tcf_exts *exts,
for (i = 0; i < exts->nr_actions; i++) {
struct tc_action *a = exts->actions[i];
- tcf_action_stats_update(a, bytes, packets, lastuse, true);
+ tcf_action_stats_update(a, bytes, packets, drops,
+ lastuse, true);
a->used_hw_stats = used_hw_stats;
a->used_hw_stats_valid = used_hw_stats_valid;
}
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 8ac7eb0a8309..4c4466f18801 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1059,14 +1059,13 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
return err;
}
-void tcf_action_update_stats(struct tc_action *a, u64 bytes, u32 packets,
- bool drop, bool hw)
+void tcf_action_update_stats(struct tc_action *a, u64 bytes, u64 packets,
+ u64 drops, bool hw)
{
if (a->cpu_bstats) {
_bstats_cpu_update(this_cpu_ptr(a->cpu_bstats), bytes, packets);
- if (drop)
- this_cpu_ptr(a->cpu_qstats)->drops += packets;
+ this_cpu_ptr(a->cpu_qstats)->drops += drops;
if (hw)
_bstats_cpu_update(this_cpu_ptr(a->cpu_bstats_hw),
@@ -1075,8 +1074,7 @@ void tcf_action_update_stats(struct tc_action *a, u64 bytes, u32 packets,
}
_bstats_update(&a->tcfa_bstats, bytes, packets);
- if (drop)
- a->tcfa_qstats.drops += packets;
+ a->tcfa_qstats.drops += drops;
if (hw)
_bstats_update(&a->tcfa_bstats_hw, bytes, packets);
}
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index e9f3576cbf71..1b9c6d4a1b6b 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -1450,12 +1450,12 @@ static int tcf_ct_search(struct net *net, struct tc_action **a, u32 index)
return tcf_idr_search(tn, a, index);
}
-static void tcf_stats_update(struct tc_action *a, u64 bytes, u32 packets,
- u64 lastuse, bool hw)
+static void tcf_stats_update(struct tc_action *a, u64 bytes, u64 packets,
+ u64 drops, u64 lastuse, bool hw)
{
struct tcf_ct *c = to_ct(a);
- tcf_action_update_stats(a, bytes, packets, false, hw);
+ tcf_action_update_stats(a, bytes, packets, drops, hw);
c->tcf_tm.lastuse = max_t(u64, c->tcf_tm.lastuse, lastuse);
}
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 416065772719..410e3bbfb9ca 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -171,14 +171,15 @@ static int tcf_gact_act(struct sk_buff *skb, const struct tc_action *a,
return action;
}
-static void tcf_gact_stats_update(struct tc_action *a, u64 bytes, u32 packets,
- u64 lastuse, bool hw)
+static void tcf_gact_stats_update(struct tc_action *a, u64 bytes, u64 packets,
+ u64 drops, u64 lastuse, bool hw)
{
struct tcf_gact *gact = to_gact(a);
int action = READ_ONCE(gact->tcf_action);
struct tcf_t *tm = &gact->tcf_tm;
- tcf_action_update_stats(a, bytes, packets, action == TC_ACT_SHOT, hw);
+ tcf_action_update_stats(a, bytes, packets,
+ action == TC_ACT_SHOT ? packets : drops, hw);
tm->lastuse = max_t(u64, tm->lastuse, lastuse);
}
diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index 9c628591f452..c818844846b1 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -568,13 +568,13 @@ static int tcf_gate_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
-static void tcf_gate_stats_update(struct tc_action *a, u64 bytes, u32 packets,
- u64 lastuse, bool hw)
+static void tcf_gate_stats_update(struct tc_action *a, u64 bytes, u64 packets,
+ u64 drops, u64 lastuse, bool hw)
{
struct tcf_gate *gact = to_gate(a);
struct tcf_t *tm = &gact->tcf_tm;
- tcf_action_update_stats(a, bytes, packets, false, hw);
+ tcf_action_update_stats(a, bytes, packets, drops, hw);
tm->lastuse = max_t(u64, tm->lastuse, lastuse);
}
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 83dd82fc9f40..b2705318993b 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -312,13 +312,13 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
return retval;
}
-static void tcf_stats_update(struct tc_action *a, u64 bytes, u32 packets,
- u64 lastuse, bool hw)
+static void tcf_stats_update(struct tc_action *a, u64 bytes, u64 packets,
+ u64 drops, u64 lastuse, bool hw)
{
struct tcf_mirred *m = to_mirred(a);
struct tcf_t *tm = &m->tcf_tm;
- tcf_action_update_stats(a, bytes, packets, false, hw);
+ tcf_action_update_stats(a, bytes, packets, drops, hw);
tm->lastuse = max_t(u64, tm->lastuse, lastuse);
}
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index d41d6200d9de..66986db062ed 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -409,13 +409,13 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a,
return p->tcf_action;
}
-static void tcf_pedit_stats_update(struct tc_action *a, u64 bytes, u32 packets,
- u64 lastuse, bool hw)
+static void tcf_pedit_stats_update(struct tc_action *a, u64 bytes, u64 packets,
+ u64 drops, u64 lastuse, bool hw)
{
struct tcf_pedit *d = to_pedit(a);
struct tcf_t *tm = &d->tcf_tm;
- tcf_action_update_stats(a, bytes, packets, false, hw);
+ tcf_action_update_stats(a, bytes, packets, drops, hw);
tm->lastuse = max_t(u64, tm->lastuse, lastuse);
}
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 8b7a0ac96c51..0b431d493768 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -288,13 +288,13 @@ static void tcf_police_cleanup(struct tc_action *a)
}
static void tcf_police_stats_update(struct tc_action *a,
- u64 bytes, u32 packets,
+ u64 bytes, u64 packets, u64 drops,
u64 lastuse, bool hw)
{
struct tcf_police *police = to_police(a);
struct tcf_t *tm = &police->tcf_tm;
- tcf_action_update_stats(a, bytes, packets, false, hw);
+ tcf_action_update_stats(a, bytes, packets, drops, hw);
tm->lastuse = max_t(u64, tm->lastuse, lastuse);
}
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index b125b2be4467..361b863e0634 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -74,12 +74,13 @@ static int tcf_skbedit_act(struct sk_buff *skb, const struct tc_action *a,
}
static void tcf_skbedit_stats_update(struct tc_action *a, u64 bytes,
- u32 packets, u64 lastuse, bool hw)
+ u64 packets, u64 drops,
+ u64 lastuse, bool hw)
{
struct tcf_skbedit *d = to_skbedit(a);
struct tcf_t *tm = &d->tcf_tm;
- tcf_action_update_stats(a, bytes, packets, false, hw);
+ tcf_action_update_stats(a, bytes, packets, drops, hw);
tm->lastuse = max_t(u64, tm->lastuse, lastuse);
}
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index c91d3958fcbb..a5ff9f68ab02 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -302,13 +302,13 @@ static int tcf_vlan_walker(struct net *net, struct sk_buff *skb,
return tcf_generic_walker(tn, skb, cb, type, ops, extack);
}
-static void tcf_vlan_stats_update(struct tc_action *a, u64 bytes, u32 packets,
- u64 lastuse, bool hw)
+static void tcf_vlan_stats_update(struct tc_action *a, u64 bytes, u64 packets,
+ u64 drops, u64 lastuse, bool hw)
{
struct tcf_vlan *v = to_vlan(a);
struct tcf_t *tm = &v->tcf_tm;
- tcf_action_update_stats(a, bytes, packets, false, hw);
+ tcf_action_update_stats(a, bytes, packets, drops, hw);
tm->lastuse = max_t(u64, tm->lastuse, lastuse);
}
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index b2da37286082..391971672d54 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -491,6 +491,7 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f,
tcf_exts_stats_update(&f->exts, cls_flower.stats.bytes,
cls_flower.stats.pkts,
+ cls_flower.stats.drops,
cls_flower.stats.lastused,
cls_flower.stats.used_hw_stats,
cls_flower.stats.used_hw_stats_valid);
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 8d39dbcf1746..cafb84480bab 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -338,7 +338,8 @@ static void mall_stats_hw_filter(struct tcf_proto *tp,
tc_setup_cb_call(block, TC_SETUP_CLSMATCHALL, &cls_mall, false, true);
tcf_exts_stats_update(&head->exts, cls_mall.stats.bytes,
- cls_mall.stats.pkts, cls_mall.stats.lastused,
+ cls_mall.stats.pkts, cls_mall.stats.drops,
+ cls_mall.stats.lastused,
cls_mall.stats.used_hw_stats,
cls_mall.stats.used_hw_stats_valid);
}
--
2.17.1
^ permalink raw reply related
* [net 0/4][pull request] Intel Wired LAN Driver Updates 2020-06-18
From: Jeff Kirsher @ 2020-06-19 6:22 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann
This series contains fixes to ixgbe, i40e and ice driver.
Ciara fixes up the ixgbe, i40e and ice drivers to protect access when
allocating and freeing the rings. In addition, made use of READ_ONCE
when reading the rings prior to accessing the statistics pointer.
Björn fixes a crash where the receive descriptor ring allocation was
moved to a different function, which broke the ethtool set_ringparam()
hook.
The following are changes since commit 0ad6f6e767ec2f613418cbc7ebe5ec4c35af540c:
net: increment xmit_recursion level in dev_direct_xmit()
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue 40GbE
Björn Töpel (1):
i40e: fix crash when Rx descriptor count is changed
Ciara Loftus (3):
ixgbe: protect ring accesses with READ- and WRITE_ONCE
i40e: protect ring accesses with READ- and WRITE_ONCE
ice: protect ring accesses with WRITE_ONCE
.../net/ethernet/intel/i40e/i40e_ethtool.c | 3 ++
drivers/net/ethernet/intel/i40e/i40e_main.c | 29 ++++++++++++-------
drivers/net/ethernet/intel/ice/ice_lib.c | 8 ++---
drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 12 ++++----
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 14 +++++++--
6 files changed, 44 insertions(+), 24 deletions(-)
--
2.26.2
^ permalink raw reply
* [net 4/4] i40e: fix crash when Rx descriptor count is changed
From: Jeff Kirsher @ 2020-06-19 6:22 UTC (permalink / raw)
To: davem
Cc: Björn Töpel, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20200619062210.3159291-1-jeffrey.t.kirsher@intel.com>
From: Björn Töpel <bjorn.topel@intel.com>
When the AF_XDP buffer allocator was introduced, the Rx SW ring
"rx_bi" allocation was moved from i40e_setup_rx_descriptors()
function, and was instead done in the i40e_configure_rx_ring()
function.
This broke the ethtool set_ringparam() hook for changing the Rx
descriptor count, which was relying on i40e_setup_rx_descriptors() to
handle the allocation.
Fix this by adding an explicit i40e_alloc_rx_bi() call to
i40e_set_ringparam().
Fixes: be1222b585fd ("i40e: Separate kernel allocated rx_bi rings from AF_XDP rings")
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index aa8026b1eb81..67806b7b2f49 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -2070,6 +2070,9 @@ static int i40e_set_ringparam(struct net_device *netdev,
*/
rx_rings[i].tail = hw->hw_addr + I40E_PRTGEN_STATUS;
err = i40e_setup_rx_descriptors(&rx_rings[i]);
+ if (err)
+ goto rx_unwind;
+ err = i40e_alloc_rx_bi(&rx_rings[i]);
if (err)
goto rx_unwind;
--
2.26.2
^ permalink raw reply related
* [net 3/4] ice: protect ring accesses with WRITE_ONCE
From: Jeff Kirsher @ 2020-06-19 6:22 UTC (permalink / raw)
To: davem; +Cc: Ciara Loftus, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20200619062210.3159291-1-jeffrey.t.kirsher@intel.com>
From: Ciara Loftus <ciara.loftus@intel.com>
The READ_ONCE macro is used when reading rings prior to accessing the
statistics pointer. The corresponding WRITE_ONCE usage when allocating and
freeing the rings to ensure protected access was not in place. Introduce
this.
Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ice/ice_lib.c | 8 ++++----
drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 28b46cc9f5cb..2e3a39cea2c0 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -1194,7 +1194,7 @@ static void ice_vsi_clear_rings(struct ice_vsi *vsi)
for (i = 0; i < vsi->alloc_txq; i++) {
if (vsi->tx_rings[i]) {
kfree_rcu(vsi->tx_rings[i], rcu);
- vsi->tx_rings[i] = NULL;
+ WRITE_ONCE(vsi->tx_rings[i], NULL);
}
}
}
@@ -1202,7 +1202,7 @@ static void ice_vsi_clear_rings(struct ice_vsi *vsi)
for (i = 0; i < vsi->alloc_rxq; i++) {
if (vsi->rx_rings[i]) {
kfree_rcu(vsi->rx_rings[i], rcu);
- vsi->rx_rings[i] = NULL;
+ WRITE_ONCE(vsi->rx_rings[i], NULL);
}
}
}
@@ -1235,7 +1235,7 @@ static int ice_vsi_alloc_rings(struct ice_vsi *vsi)
ring->vsi = vsi;
ring->dev = dev;
ring->count = vsi->num_tx_desc;
- vsi->tx_rings[i] = ring;
+ WRITE_ONCE(vsi->tx_rings[i], ring);
}
/* Allocate Rx rings */
@@ -1254,7 +1254,7 @@ static int ice_vsi_alloc_rings(struct ice_vsi *vsi)
ring->netdev = vsi->netdev;
ring->dev = dev;
ring->count = vsi->num_rx_desc;
- vsi->rx_rings[i] = ring;
+ WRITE_ONCE(vsi->rx_rings[i], ring);
}
return 0;
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 082825e3cb39..4cbd49c87568 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1702,7 +1702,7 @@ static int ice_xdp_alloc_setup_rings(struct ice_vsi *vsi)
xdp_ring->netdev = NULL;
xdp_ring->dev = dev;
xdp_ring->count = vsi->num_tx_desc;
- vsi->xdp_rings[i] = xdp_ring;
+ WRITE_ONCE(vsi->xdp_rings[i], xdp_ring);
if (ice_setup_tx_ring(xdp_ring))
goto free_xdp_rings;
ice_set_ring_xdp(xdp_ring);
--
2.26.2
^ permalink raw reply related
* [net 1/4] ixgbe: protect ring accesses with READ- and WRITE_ONCE
From: Jeff Kirsher @ 2020-06-19 6:22 UTC (permalink / raw)
To: davem; +Cc: Ciara Loftus, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20200619062210.3159291-1-jeffrey.t.kirsher@intel.com>
From: Ciara Loftus <ciara.loftus@intel.com>
READ_ONCE should be used when reading rings prior to accessing the
statistics pointer. Introduce this as well as the corresponding WRITE_ONCE
usage when allocating and freeing the rings, to ensure protected access.
Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 12 ++++++------
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 14 +++++++++++---
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index fd9f5d41b594..2e35c5706cf1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -921,7 +921,7 @@ static int ixgbe_alloc_q_vector(struct ixgbe_adapter *adapter,
ring->queue_index = txr_idx;
/* assign ring to adapter */
- adapter->tx_ring[txr_idx] = ring;
+ WRITE_ONCE(adapter->tx_ring[txr_idx], ring);
/* update count and index */
txr_count--;
@@ -948,7 +948,7 @@ static int ixgbe_alloc_q_vector(struct ixgbe_adapter *adapter,
set_ring_xdp(ring);
/* assign ring to adapter */
- adapter->xdp_ring[xdp_idx] = ring;
+ WRITE_ONCE(adapter->xdp_ring[xdp_idx], ring);
/* update count and index */
xdp_count--;
@@ -991,7 +991,7 @@ static int ixgbe_alloc_q_vector(struct ixgbe_adapter *adapter,
ring->queue_index = rxr_idx;
/* assign ring to adapter */
- adapter->rx_ring[rxr_idx] = ring;
+ WRITE_ONCE(adapter->rx_ring[rxr_idx], ring);
/* update count and index */
rxr_count--;
@@ -1020,13 +1020,13 @@ static void ixgbe_free_q_vector(struct ixgbe_adapter *adapter, int v_idx)
ixgbe_for_each_ring(ring, q_vector->tx) {
if (ring_is_xdp(ring))
- adapter->xdp_ring[ring->queue_index] = NULL;
+ WRITE_ONCE(adapter->xdp_ring[ring->queue_index], NULL);
else
- adapter->tx_ring[ring->queue_index] = NULL;
+ WRITE_ONCE(adapter->tx_ring[ring->queue_index], NULL);
}
ixgbe_for_each_ring(ring, q_vector->rx)
- adapter->rx_ring[ring->queue_index] = NULL;
+ WRITE_ONCE(adapter->rx_ring[ring->queue_index], NULL);
adapter->q_vector[v_idx] = NULL;
napi_hash_del(&q_vector->napi);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f162b8b8f345..97a423ecf808 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7051,7 +7051,10 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
}
for (i = 0; i < adapter->num_rx_queues; i++) {
- struct ixgbe_ring *rx_ring = adapter->rx_ring[i];
+ struct ixgbe_ring *rx_ring = READ_ONCE(adapter->rx_ring[i]);
+
+ if (!rx_ring)
+ continue;
non_eop_descs += rx_ring->rx_stats.non_eop_descs;
alloc_rx_page += rx_ring->rx_stats.alloc_rx_page;
alloc_rx_page_failed += rx_ring->rx_stats.alloc_rx_page_failed;
@@ -7072,15 +7075,20 @@ void ixgbe_update_stats(struct ixgbe_adapter *adapter)
packets = 0;
/* gather some stats to the adapter struct that are per queue */
for (i = 0; i < adapter->num_tx_queues; i++) {
- struct ixgbe_ring *tx_ring = adapter->tx_ring[i];
+ struct ixgbe_ring *tx_ring = READ_ONCE(adapter->tx_ring[i]);
+
+ if (!tx_ring)
+ continue;
restart_queue += tx_ring->tx_stats.restart_queue;
tx_busy += tx_ring->tx_stats.tx_busy;
bytes += tx_ring->stats.bytes;
packets += tx_ring->stats.packets;
}
for (i = 0; i < adapter->num_xdp_queues; i++) {
- struct ixgbe_ring *xdp_ring = adapter->xdp_ring[i];
+ struct ixgbe_ring *xdp_ring = READ_ONCE(adapter->xdp_ring[i]);
+ if (!xdp_ring)
+ continue;
restart_queue += xdp_ring->tx_stats.restart_queue;
tx_busy += xdp_ring->tx_stats.tx_busy;
bytes += xdp_ring->stats.bytes;
--
2.26.2
^ permalink raw reply related
* [net 2/4] i40e: protect ring accesses with READ- and WRITE_ONCE
From: Jeff Kirsher @ 2020-06-19 6:22 UTC (permalink / raw)
To: davem; +Cc: Ciara Loftus, netdev, nhorman, sassmann, Andrew Bowers,
Jeff Kirsher
In-Reply-To: <20200619062210.3159291-1-jeffrey.t.kirsher@intel.com>
From: Ciara Loftus <ciara.loftus@intel.com>
READ_ONCE should be used when reading rings prior to accessing the
statistics pointer. Introduce this as well as the corresponding WRITE_ONCE
usage when allocating and freeing the rings, to ensure protected access.
Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 29 ++++++++++++++-------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 5d807c8004f8..56ecd6c3f236 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -439,11 +439,15 @@ static void i40e_get_netdev_stats_struct(struct net_device *netdev,
i40e_get_netdev_stats_struct_tx(ring, stats);
if (i40e_enabled_xdp_vsi(vsi)) {
- ring++;
+ ring = READ_ONCE(vsi->xdp_rings[i]);
+ if (!ring)
+ continue;
i40e_get_netdev_stats_struct_tx(ring, stats);
}
- ring++;
+ ring = READ_ONCE(vsi->rx_rings[i]);
+ if (!ring)
+ continue;
do {
start = u64_stats_fetch_begin_irq(&ring->syncp);
packets = ring->stats.packets;
@@ -787,6 +791,8 @@ static void i40e_update_vsi_stats(struct i40e_vsi *vsi)
for (q = 0; q < vsi->num_queue_pairs; q++) {
/* locate Tx ring */
p = READ_ONCE(vsi->tx_rings[q]);
+ if (!p)
+ continue;
do {
start = u64_stats_fetch_begin_irq(&p->syncp);
@@ -800,8 +806,11 @@ static void i40e_update_vsi_stats(struct i40e_vsi *vsi)
tx_linearize += p->tx_stats.tx_linearize;
tx_force_wb += p->tx_stats.tx_force_wb;
- /* Rx queue is part of the same block as Tx queue */
- p = &p[1];
+ /* locate Rx ring */
+ p = READ_ONCE(vsi->rx_rings[q]);
+ if (!p)
+ continue;
+
do {
start = u64_stats_fetch_begin_irq(&p->syncp);
packets = p->stats.packets;
@@ -10824,10 +10833,10 @@ static void i40e_vsi_clear_rings(struct i40e_vsi *vsi)
if (vsi->tx_rings && vsi->tx_rings[0]) {
for (i = 0; i < vsi->alloc_queue_pairs; i++) {
kfree_rcu(vsi->tx_rings[i], rcu);
- vsi->tx_rings[i] = NULL;
- vsi->rx_rings[i] = NULL;
+ WRITE_ONCE(vsi->tx_rings[i], NULL);
+ WRITE_ONCE(vsi->rx_rings[i], NULL);
if (vsi->xdp_rings)
- vsi->xdp_rings[i] = NULL;
+ WRITE_ONCE(vsi->xdp_rings[i], NULL);
}
}
}
@@ -10861,7 +10870,7 @@ static int i40e_alloc_rings(struct i40e_vsi *vsi)
if (vsi->back->hw_features & I40E_HW_WB_ON_ITR_CAPABLE)
ring->flags = I40E_TXR_FLAGS_WB_ON_ITR;
ring->itr_setting = pf->tx_itr_default;
- vsi->tx_rings[i] = ring++;
+ WRITE_ONCE(vsi->tx_rings[i], ring++);
if (!i40e_enabled_xdp_vsi(vsi))
goto setup_rx;
@@ -10879,7 +10888,7 @@ static int i40e_alloc_rings(struct i40e_vsi *vsi)
ring->flags = I40E_TXR_FLAGS_WB_ON_ITR;
set_ring_xdp(ring);
ring->itr_setting = pf->tx_itr_default;
- vsi->xdp_rings[i] = ring++;
+ WRITE_ONCE(vsi->xdp_rings[i], ring++);
setup_rx:
ring->queue_index = i;
@@ -10892,7 +10901,7 @@ static int i40e_alloc_rings(struct i40e_vsi *vsi)
ring->size = 0;
ring->dcb_tc = 0;
ring->itr_setting = pf->rx_itr_default;
- vsi->rx_rings[i] = ring;
+ WRITE_ONCE(vsi->rx_rings[i], ring);
}
return 0;
--
2.26.2
^ permalink raw reply related
* Re: [Patch net] cgroup: fix cgroup_sk_alloc() for sk_clone_lock()
From: Zefan Li @ 2020-06-19 6:40 UTC (permalink / raw)
To: Cong Wang, Roman Gushchin
Cc: Linux Kernel Network Developers, Cameron Berkenpas, Peter Geis,
Lu Fengqi, Daniël Sonck, Daniel Borkmann, Tejun Heo
In-Reply-To: <CAM_iQpWuNnHqNHKz5FMgAXoqQ5qGDEtNbBKDXpmpeNSadCZ-1w@mail.gmail.com>
On 2020/6/19 5:09, Cong Wang wrote:
> On Thu, Jun 18, 2020 at 12:36 PM Roman Gushchin <guro@fb.com> wrote:
>>
>> On Thu, Jun 18, 2020 at 12:19:13PM -0700, Cong Wang wrote:
>>> On Wed, Jun 17, 2020 at 6:44 PM Zefan Li <lizefan@huawei.com> wrote:
>>>>
>>>> Cc: Roman Gushchin <guro@fb.com>
>>>>
>>>> Thanks for fixing this.
>>>>
>>>> On 2020/6/17 2:03, Cong Wang wrote:
>>>>> When we clone a socket in sk_clone_lock(), its sk_cgrp_data is
>>>>> copied, so the cgroup refcnt must be taken too. And, unlike the
>>>>> sk_alloc() path, sock_update_netprioidx() is not called here.
>>>>> Therefore, it is safe and necessary to grab the cgroup refcnt
>>>>> even when cgroup_sk_alloc is disabled.
>>>>>
>>>>> sk_clone_lock() is in BH context anyway, the in_interrupt()
>>>>> would terminate this function if called there. And for sk_alloc()
>>>>> skcd->val is always zero. So it's safe to factor out the code
>>>>> to make it more readable.
>>>>>
>>>>> Fixes: 090e28b229af92dc5b ("netprio_cgroup: Fix unlimited memory leak of v2 cgroups")
>>>>
>>>> but I don't think the bug was introduced by this commit, because there
>>>> are already calls to cgroup_sk_alloc_disable() in write_priomap() and
>>>> write_classid(), which can be triggered by writing to ifpriomap or
>>>> classid in cgroupfs. This commit just made it much easier to happen
>>>> with systemd invovled.
>>>>
>>>> I think it's 4bfc0bb2c60e2f4c ("bpf: decouple the lifetime of cgroup_bpf from cgroup itself"),
>>>> which added cgroup_bpf_get() in cgroup_sk_alloc().
>>>
>>> Good point.
>>>
>>> I take a deeper look, it looks like commit d979a39d7242e06
>>> is the one to blame, because it is the first commit that began to
>>> hold cgroup refcnt in cgroup_sk_alloc().
>>
>> I agree, ut seems that the issue is not related to bpf and probably
>> can be reproduced without CONFIG_CGROUP_BPF. d979a39d7242e06 indeed
>> seems closer to the origin.
>
> Yeah, I will update the Fixes tag and send V2.
>
Commit d979a39d7242e06 looks innocent to me. With this commit when cgroup_sk_alloc
is disabled and then a socket is cloned the cgroup refcnt will not be incremented,
but this is fine, because when the socket is to be freed:
sk_prot_free()
cgroup_sk_free()
cgroup_put(sock_cgroup_ptr(skcd)) == cgroup_put(&cgrp_dfl_root.cgrp)
cgroup_put() does nothing for the default root cgroup, so nothing bad will happen.
but cgroup_bpf_put() will decrement the bpf refcnt while this refcnt were not incremented
as cgroup_sk_alloc has already been disabled. That's why I think it's 4bfc0bb2c60e2f4c
that needs to be fixed.
>>
>> Btw, based on the number of reported-by tags it seems that there was
>> a real issue which the patch is fixing. Maybe you'll a couple of words
>> about how it reveals itself in the real life?
>
> I still have no idea how exactly this is triggered. According to the
> people who reported this bug, they just need to wait for some hours
> to trigger. So I am not sure what to add here, just the stack trace?
>
> Thanks.
> .
>
^ permalink raw reply
* Re: [PATCH v5 2/3] dt-bindings: net: mscc-vsc8531: add optional clock properties
From: Heiko Stuebner @ 2020-06-19 6:46 UTC (permalink / raw)
To: Florian Fainelli
Cc: davem, kuba, robh+dt, andrew, hkallweit1, linux, netdev,
devicetree, linux-kernel, christoph.muellner
In-Reply-To: <a877e41d-4c3c-c4c2-1875-71e1e08cf977@gmail.com>
Am Freitag, 19. Juni 2020, 07:01:58 CEST schrieb Florian Fainelli:
>
> On 6/18/2020 5:11 AM, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> >
> > Some mscc ethernet phys have a configurable clock output, so describe the
> > generic properties to access them in devicetrees.
> >
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > ---
> > Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> > index 5ff37c68c941..67625ba27f53 100644
> > --- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> > +++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
> > @@ -1,6 +1,8 @@
> > * Microsemi - vsc8531 Giga bit ethernet phy
> >
> > Optional properties:
> > +- clock-output-names : Name for the exposed clock output
> > +- #clock-cells : should be 0
> > - vsc8531,vddmac : The vddmac in mV. Allowed values is listed
> > in the first row of Table 1 (below).
> > This property is only used in combination
> >
>
> With that approach, you also need to be careful as a driver writer to
> ensure that you have at least probed the MDIO bus to ensure that the PHY
> device has been created (and therefore it is available as a clock
> provider) if that same Ethernet MAC is a consumer of that clock (which
> it appears to be). Otherwise you may just never probe and be trapped in
> a circular dependency.
Yep - although without anything like this, the phy won't emit any clock
at all. Even when enabling the clock output in u-boot already, when the
kernel starts that config is lost, so no existing board should break.
As you can see in the discussion about patch 3/3 the wanted solution
is not so clear cut as well. With Rob suggesting this clock-provider way
and Russell strongly encouraging taking a second look.
[My first iteration (till v4) was doing it like other phys by specifying
a property to just tell the phy what frequency to output]
I don't really have a preference for one or the other, so
maybe you can also give a vote over there ;-)
Heiko
^ permalink raw reply
* Re: [PATCHv2 net] tc-testing: update geneve options match in tunnel_key unit tests
From: Simon Horman @ 2020-06-19 6:56 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Lucas Bates, David Miller, Davide Caratti
In-Reply-To: <20200619032445.664868-1-liuhangbin@gmail.com>
On Fri, Jun 19, 2020 at 11:24:45AM +0800, Hangbin Liu wrote:
> Since iproute2 commit f72c3ad00f3b ("tc: m_tunnel_key: add options
> support for vxlan"), the geneve opt output use key word "geneve_opts"
> instead of "geneve_opt". To make compatibility for both old and new
> iproute2, let's accept both "geneve_opt" and "geneve_opts".
>
> Suggested-by: Davide Caratti <dcaratti@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
^ permalink raw reply
* Re: [PATCH iproute2] tc: m_tunnel_key: fix geneve opt output
From: Simon Horman @ 2020-06-19 6:56 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Davide Caratti, lucien.xin, Stephen Hemminger
In-Reply-To: <20200619022524.GX102436@dhcp-12-153.nay.redhat.com>
On Fri, Jun 19, 2020 at 10:25:24AM +0800, Hangbin Liu wrote:
> On Thu, Jun 18, 2020 at 12:51:08PM +0200, Simon Horman wrote:
> > On Thu, Jun 18, 2020 at 06:44:20PM +0800, Hangbin Liu wrote:
> > > Commit f72c3ad00f3b changed the geneve option output from "geneve_opt"
> > > to "geneve_opts", which may break the program compatibility. Reset
> > > it back to geneve_opt.
> > >
> > > Fixes: f72c3ad00f3b ("tc: m_tunnel_key: add options support for vxlan")
> > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> >
> > Thanks Hangbin.
> >
> > I agree that the patch in question did change the name of the option
> > as you describe, perhaps inadvertently. But I wonder if perhaps this fix
> > is too simple as the patch mentioned also:
> >
> > 1. Documents the option as geneve_opts
> > 2. Adds vxlan_opts
> >
> > So this patch invalidates the documentation and creates asymmetry between
> > the VXLAN and Geneve variants of this feature.
>
> Not sure if I understand you comment correctly. This patch only fix the cmd
> output(revert to previous output format), The cmd option is not changed. e.g.
>
> # tc actions add action tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 \
> dst_port 6081 geneve_opts 0102:80:00880022 index 1
> # tc actions get action tunnel_key index 1
> total acts 0
>
> action order 1: tunnel_key set
> src_ip 1.1.1.1
> dst_ip 2.2.2.2
> key_id 42
> dst_port 6081
> geneve_opt 0102:80:00880022
> csum pipe
> index 1 ref 1 bind 0
>
> But this do make a asymmetry for vxlan and geneve output. I prefer
> to let them consistent personally. Also it looks more reasonable
> to output "geneve_opts" when we have parameter geneve_opts.
>
> So I'm not going to fix it in iproute, but do as Davide mentioned, make
> tdc test case accept both 'geneve_opts' and 'geneve_opt'.
Thanks. I agree this seems to be the best way forward.
^ permalink raw reply
* Re: [v2,net-next] net: qos offload add flow status with dropped count
From: Simon Horman @ 2020-06-19 7:03 UTC (permalink / raw)
To: Po Liu
Cc: davem, linux-kernel, netdev, jiri, vinicius.gomes, vlad,
claudiu.manoil, vladimir.oltean, alexandru.marginean,
michael.chan, vishal, saeedm, leon, jiri, idosch,
alexandre.belloni, UNGLinuxDriver, kuba, jhs, xiyou.wangcong,
pablo, moshe, m-karicheri2, andre.guedes, stephen
In-Reply-To: <20200619060107.6325-1-po.liu@nxp.com>
On Fri, Jun 19, 2020 at 02:01:07PM +0800, Po Liu wrote:
> From: Po Liu <Po.Liu@nxp.com>
>
> This patch adds a drop frames counter to tc flower offloading.
> Reporting h/w dropped frames is necessary for some actions.
> Some actions like police action and the coming introduced stream gate
> action would produce dropped frames which is necessary for user. Status
> update shows how many filtered packets increasing and how many dropped
> in those packets.
>
> v2: Changes
> - Update commit comments suggest by Jiri Pirko.
>
> Signed-off-by: Po Liu <Po.Liu@nxp.com>
> ---
> This patch is continue the thread 20200324034745.30979-1-Po.Liu@nxp.com
>
> drivers/net/dsa/sja1105/sja1105_vl.c | 2 +-
> drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 2 +-
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 2 +-
> .../net/ethernet/chelsio/cxgb4/cxgb4_tc_matchall.c | 2 +-
> drivers/net/ethernet/freescale/enetc/enetc_qos.c | 7 +++++--
> drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c | 2 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++--
> drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c | 2 +-
> drivers/net/ethernet/mscc/ocelot_flower.c | 2 +-
> drivers/net/ethernet/netronome/nfp/flower/offload.c | 2 +-
> drivers/net/ethernet/netronome/nfp/flower/qos_conf.c | 2 +-
> include/net/act_api.h | 11 ++++++-----
> include/net/flow_offload.h | 5 ++++-
> include/net/pkt_cls.h | 5 +++--
> net/sched/act_api.c | 10 ++++------
> net/sched/act_ct.c | 6 +++---
> net/sched/act_gact.c | 7 ++++---
> net/sched/act_gate.c | 6 +++---
> net/sched/act_mirred.c | 6 +++---
> net/sched/act_pedit.c | 6 +++---
> net/sched/act_police.c | 4 ++--
> net/sched/act_skbedit.c | 5 +++--
> net/sched/act_vlan.c | 6 +++---
> net/sched/cls_flower.c | 1 +
> net/sched/cls_matchall.c | 3 ++-
> 25 files changed, 60 insertions(+), 50 deletions(-)
Netronome portion:
Reviewed-by: Simon Horman <simon.horman@netronome.com>
^ permalink raw reply
* Re: [PATCH net v5 0/4] several fixes for indirect flow_blocks offload
From: Simon Horman @ 2020-06-19 7:10 UTC (permalink / raw)
To: wenxu; +Cc: netdev, davem, pablo, vladbu
In-Reply-To: <1592484551-16188-1-git-send-email-wenxu@ucloud.cn>
On Thu, Jun 18, 2020 at 08:49:07PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> v2:
> patch2: store the cb_priv of representor to the flow_block_cb->indr.cb_priv
> in the driver. And make the correct check with the statments
> this->indr.cb_priv == cb_priv
>
> patch4: del the driver list only in the indriect cleanup callbacks
>
> v3:
> add the cover letter and changlogs.
>
> v4:
> collapsed 1/4, 2/4, 4/4 in v3 to one fix
> Add the prepare patch 1 and 2
>
> v5:
> patch1: place flow_indr_block_cb_alloc() right before
> flow_indr_dev_setup_offload() to avoid moving flow_block_indr_init()
>
> This series fixes commit 1fac52da5942 ("net: flow_offload: consolidate
> indirect flow_block infrastructure") that revists the flow_block
> infrastructure.
>
> patch #1 #2: prepare for fix patch #3
> add and use flow_indr_block_cb_alloc/remove function
>
> patch #3: fix flow_indr_dev_unregister path
> If the representor is removed, then identify the indirect flow_blocks
> that need to be removed by the release callback and the port representor
> structure. To identify the port representor structure, a new
> indr.cb_priv field needs to be introduced. The flow_block also needs to
> be removed from the driver list from the cleanup path
>
>
> patch#4 fix block->nooffloaddevcnt warning dmesg log.
> When a indr device add in offload success. The block->nooffloaddevcnt
> should be 0. After the representor go away. When the dir device go away
> the flow_block UNBIND operation with -EOPNOTSUPP which lead the warning
> demesg log.
> The block->nooffloaddevcnt should always count for indr block.
> even the indr block offload successful. The representor maybe
> gone away and the ingress qdisc can work in software mode.
>
>
> wenxu (4):
> flow_offload: add flow_indr_block_cb_alloc/remove function
> flow_offload: use flow_indr_block_cb_alloc/remove function
> net: flow_offload: fix flow_indr_dev_unregister path
> net/sched: cls_api: fix nooffloaddevcnt warning dmesg log
Reviewed-by: Simon Horman <simon.horman@netronome.com>
^ permalink raw reply
* Re: [PATCH 1/2] docs: net: ieee802154: change link to new project URL
From: Stefan Schmidt @ 2020-06-19 7:14 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-wpan, alex.aring
In-Reply-To: <20200616065814.816248-1-stefan@datenfreihafen.org>
Hello Dave.
On 16.06.20 08:58, Stefan Schmidt wrote:
> We finally came around to setup a new project website.
> Update the reference here.
>
> Signed-off-by: Stefan Schmidt <stefan@datenfreihafen.org>
> ---
> Documentation/networking/ieee802154.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/networking/ieee802154.rst b/Documentation/networking/ieee802154.rst
> index 36ca823a1122..6f4bf8447a21 100644
> --- a/Documentation/networking/ieee802154.rst
> +++ b/Documentation/networking/ieee802154.rst
> @@ -30,8 +30,8 @@ Socket API
>
> The address family, socket addresses etc. are defined in the
> include/net/af_ieee802154.h header or in the special header
> -in the userspace package (see either http://wpan.cakelab.org/ or the
> -git tree at https://github.com/linux-wpan/wpan-tools).
> +in the userspace package (see either https://linux-wpan.org/wpan-tools.html
> +or the git tree at https://github.com/linux-wpan/wpan-tools).
>
> 6LoWPAN Linux implementation
> ============================
>
I see you marked both patches here as awaiting upstream in patchwork. I
am not really sure what to do best now. Am I supposed to pick them up
myself and send them in my usual ieee802154 pull request?
Before you had been picking up docs and MAINTAINERS patches directly. I
am fine with either way. Just want to check what you expect.
regards
Stefan Schmidt
^ 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