* [PATCH net] ipv4: Use return value of inet_iif() for __raw_v4_lookup in the while loop
From: Stephen Suryaputra @ 2019-06-25 0:14 UTC (permalink / raw)
To: netdev; +Cc: dsahern, Stephen Suryaputra
In commit 19e4e768064a8 ("ipv4: Fix raw socket lookup for local
traffic"), the dif argument to __raw_v4_lookup() is coming from the
returned value of inet_iif() but the change was done only for the first
lookup. Subsequent lookups in the while loop still use skb->dev->ifIndex.
Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
---
net/ipv4/raw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 0b8e06ca75d6..40a6abbc9cf6 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -197,7 +197,7 @@ static int raw_v4_input(struct sk_buff *skb, const struct iphdr *iph, int hash)
}
sk = __raw_v4_lookup(net, sk_next(sk), iph->protocol,
iph->saddr, iph->daddr,
- skb->dev->ifindex, sdif);
+ dif, sdif);
}
out:
read_unlock(&raw_v4_hashinfo.lock);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH bpf-next] bpftool: Add BPF_F_QUERY_EFFECTIVE support in bpftool cgroup [show|tree]
From: Alexei Starovoitov @ 2019-06-25 1:22 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrey Ignatov, Daniel Borkmann, Takshak Chahande,
netdev@vger.kernel.org, ast@kernel.org, Kernel Team,
Stanislav Fomichev
In-Reply-To: <20190624174726.2dda122b@cakuba.netronome.com>
On 6/24/19 5:47 PM, Jakub Kicinski wrote:
> On Tue, 25 Jun 2019 00:40:09 +0000, Alexei Starovoitov wrote:
>> On 6/24/19 5:30 PM, Jakub Kicinski wrote:
>>> On Tue, 25 Jun 2019 00:21:57 +0000, Alexei Starovoitov wrote:
>>>> On 6/24/19 5:16 PM, Jakub Kicinski wrote:
>>>>> On Mon, 24 Jun 2019 23:38:11 +0000, Alexei Starovoitov wrote:
>>>>>> I don't think this patch should be penalized.
>>>>>> I'd rather see we fix them all.
>>>>>
>>>>> So we are going to add this broken option just to remove it?
>>>>> I don't understand.
>>>>> I'm happy to spend the 15 minutes rewriting this if you don't
>>>>> want to penalize Takshak.
>>>>
>>>> hmm. I don't understand the 'broken' part.
>>>> The only issue I see that it could have been local vs global,
>>>> but they all should have been local.
>>>
>>> I don't think all of them. Only --mapcompat and --bpffs. bpffs could
>>> be argued. On mapcompat I must have not read the patch fully, I was
>>> under the impression its a global libbpf flag :(
>>>
>>> --json, --pretty, --nomount, --debug are global because they affect
>>> global behaviour of bpftool. The difference here is that we basically
>>> add a syscall parameter as a global option.
>>
>> sure. I only disagreed about not touching older flags.
>> --effective should be local.
>> If follow up patch means 90% rewrite then revert is better.
>> If it's 10% fixup then it's different story.
>
> I see. The local flag would not an option in getopt_long() sense, what
> I was thinking was about adding an "effective" keyword:
>
> # bpftool -e cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/
>
> becomes:
>
> # bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/ effective
>
> That's how we handle flags for update calls for instance..
>
> So I think a revert :(
fair enough.
removed it and force pushed bpf-next.
^ permalink raw reply
* Re: [PATCH bpf-next] bpftool: Add BPF_F_QUERY_EFFECTIVE support in bpftool cgroup [show|tree]
From: Alexei Starovoitov @ 2019-06-25 1:23 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrey Ignatov, Daniel Borkmann, Takshak Chahande,
netdev@vger.kernel.org, ast@kernel.org, Kernel Team,
Stanislav Fomichev
In-Reply-To: <20190624175740.5cccea9b@cakuba.netronome.com>
On 6/24/19 5:57 PM, Jakub Kicinski wrote:
> On Mon, 24 Jun 2019 17:47:26 -0700, Jakub Kicinski wrote:
>> I see. The local flag would not an option in getopt_long() sense, what
>> I was thinking was about adding an "effective" keyword:
>
> Something like this, untested:
>
> --->8------------
>
> The BPF_F_QUERY_EFFECTIVE is a syscall flag, and fits nicely
> as a subcommand option. We want to move away from global
> options, anyway.
>
> We need a global variable because of nftw limitations.
> Clean this flag on every invocations in case we run in
> batch mode.
>
> NOTE the argv[1] use on the error path in do_show() looks
> like a bug on it's own.
> ---
> .../bpftool/Documentation/bpftool-cgroup.rst | 24 +++----
> tools/bpf/bpftool/Documentation/bpftool.rst | 6 +-
> tools/bpf/bpftool/bash-completion/bpftool | 17 ++---
> tools/bpf/bpftool/cgroup.c | 62 ++++++++++++-------
> tools/bpf/bpftool/main.c | 7 +--
> tools/bpf/bpftool/main.h | 3 +-
> 6 files changed, 66 insertions(+), 53 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> index 324df15bf4cc..4fde3dfad395 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> @@ -12,8 +12,7 @@ SYNOPSIS
>
> **bpftool** [*OPTIONS*] **cgroup** *COMMAND*
>
> - *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** }
> - | { **-e** | **--effective** } }
> + *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
>
> *COMMANDS* :=
> { **show** | **list** | **tree** | **attach** | **detach** | **help** }
> @@ -21,8 +20,8 @@ SYNOPSIS
> CGROUP COMMANDS
> ===============
>
> -| **bpftool** **cgroup { show | list }** *CGROUP*
> -| **bpftool** **cgroup tree** [*CGROUP_ROOT*]
> +| **bpftool** **cgroup { show | list }** *CGROUP* [**effective**]
> +| **bpftool** **cgroup tree** [*CGROUP_ROOT*] [**effective**]
> | **bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
> | **bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
> | **bpftool** **cgroup help**
> @@ -35,13 +34,17 @@ CGROUP COMMANDS
>
> DESCRIPTION
> ===========
> - **bpftool cgroup { show | list }** *CGROUP*
> + **bpftool cgroup { show | list }** *CGROUP* [**effective**]
> List all programs attached to the cgroup *CGROUP*.
>
> Output will start with program ID followed by attach type,
> attach flags and program name.
>
> - **bpftool cgroup tree** [*CGROUP_ROOT*]
> + If **effective** is specified retrieve effective programs that
> + will execute for events within a cgroup. This includes
> + inherited along with attached ones.
> +
> + **bpftool cgroup tree** [*CGROUP_ROOT*] [**effective**]
> Iterate over all cgroups in *CGROUP_ROOT* and list all
> attached programs. If *CGROUP_ROOT* is not specified,
> bpftool uses cgroup v2 mountpoint.
> @@ -50,6 +53,10 @@ DESCRIPTION
> commands: it starts with absolute cgroup path, followed by
> program ID, attach type, attach flags and program name.
>
> + If **effective** is specified retrieve effective programs that
> + will execute for events within a cgroup. This includes
> + inherited along with attached ones.
> +
> **bpftool cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
> Attach program *PROG* to the cgroup *CGROUP* with attach type
> *ATTACH_TYPE* and optional *ATTACH_FLAGS*.
> @@ -122,11 +129,6 @@ OPTIONS
> Print all logs available from libbpf, including debug-level
> information.
>
> - -e, --effective
> - Retrieve effective programs that will execute for events
> - within a cgroup. This includes inherited along with attached
> - ones.
> -
> EXAMPLES
> ========
> |
> diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
> index d2f76b55988d..6a9c52ef84a9 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool.rst
> @@ -19,7 +19,7 @@ SYNOPSIS
> *OBJECT* := { **map** | **program** | **cgroup** | **perf** | **net** | **feature** }
>
> *OPTIONS* := { { **-V** | **--version** } | { **-h** | **--help** }
> - | { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-e** | **--effective** } }
> + | { **-j** | **--json** } [{ **-p** | **--pretty** }] }
>
> *MAP-COMMANDS* :=
> { **show** | **list** | **create** | **dump** | **update** | **lookup** | **getnext**
> @@ -71,10 +71,6 @@ OPTIONS
> includes logs from libbpf as well as from the verifier, when
> attempting to load programs.
>
> - -e, --effective
> - Retrieve effective programs that will execute for events
> - within a cgroup. This includes inherited along with attached ones.
> -
> SEE ALSO
> ========
> **bpf**\ (2),
> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index c98cb99867f6..de84ae06ae4e 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -187,7 +187,7 @@ _bpftool()
>
> # Deal with options
> if [[ ${words[cword]} == -* ]]; then
> - local c='--version --json --pretty --bpffs --mapcompat --debug --effective'
> + local c='--version --json --pretty --bpffs --mapcompat --debug'
> COMPREPLY=( $( compgen -W "$c" -- "$cur" ) )
> return 0
> fi
> @@ -678,12 +678,15 @@ _bpftool()
> ;;
> cgroup)
> case $command in
> - show|list)
> - _filedir
> - return 0
> - ;;
> - tree)
> - _filedir
> + show|list|tree)
> + case $cword in
> + 3)
> + _filedir
> + ;;
> + 4)
> + COMPREPLY=( $( compgen -W 'effective' -- "$cur" ) )
> + ;;
> + esac
> return 0
> ;;
> attach|detach)
> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index 1bb2a751107a..88b80616d47b 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -28,6 +28,8 @@
> " connect6 | sendmsg4 | sendmsg6 |\n" \
> " recvmsg4 | recvmsg6 | sysctl }"
>
> +static unsigned int query_flags;
> +
> static const char * const attach_type_strings[] = {
> [BPF_CGROUP_INET_INGRESS] = "ingress",
> [BPF_CGROUP_INET_EGRESS] = "egress",
> @@ -104,8 +106,8 @@ static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
> __u32 prog_cnt = 0;
> int ret;
>
> - ret = bpf_prog_query(cgroup_fd, type, query_flags, NULL, NULL,
> - &prog_cnt);
> + ret = bpf_prog_query(cgroup_fd, type, query_flags, NULL,
> + NULL, &prog_cnt);
> if (ret)
> return -1;
>
> @@ -156,20 +158,30 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
> static int do_show(int argc, char **argv)
> {
> enum bpf_attach_type type;
> + const char *path;
> int cgroup_fd;
> int ret = -1;
>
> - if (argc < 1) {
> - p_err("too few parameters for cgroup show");
> - goto exit;
> - } else if (argc > 1) {
> - p_err("too many parameters for cgroup show");
> - goto exit;
> + query_flags = 0;
> +
> + if (!REQ_ARGS(1))
> + return -1;
> + path = GET_ARG();
> +
> + while (argc) {
> + if (is_prefix(*argv, "effective")) {
> + query_flags |= BPF_F_QUERY_EFFECTIVE;
> + NEXT_ARG();
> + } else {
> + p_err("expected no more arguments, 'effective', got: '%s'?",
> + *argv);
> + return -1;
> + }
> }
>
> - cgroup_fd = open(argv[0], O_RDONLY);
> + cgroup_fd = open(path, O_RDONLY);
> if (cgroup_fd < 0) {
> - p_err("can't open cgroup %s", argv[1]);
> + p_err("can't open cgroup %s", path);
> goto exit;
> }
>
> @@ -295,23 +307,29 @@ static int do_show_tree(int argc, char **argv)
> char *cgroup_root;
> int ret;
>
> - switch (argc) {
> - case 0:
> + query_flags = 0;
> +
> + if (!argc) {
> cgroup_root = find_cgroup_root();
> if (!cgroup_root) {
> p_err("cgroup v2 isn't mounted");
> return -1;
> }
> - break;
> - case 1:
> - cgroup_root = argv[0];
> - break;
> - default:
> - p_err("too many parameters for cgroup tree");
> - return -1;
> + } else {
> + cgroup_root = GET_ARG();
> +
> + while (argc) {
> + if (is_prefix(*argv, "effective")) {
> + query_flags |= BPF_F_QUERY_EFFECTIVE;
> + NEXT_ARG();
> + } else {
> + p_err("expected no more arguments, 'effective', got: '%s'?",
> + *argv);
> + return -1;
> + }
> + }
> }
>
> -
> if (json_output)
> jsonw_start_array(json_wtr);
> else
> @@ -457,8 +475,8 @@ static int do_help(int argc, char **argv)
> }
>
> fprintf(stderr,
> - "Usage: %s %s { show | list } CGROUP\n"
> - " %s %s tree [CGROUP_ROOT]\n"
> + "Usage: %s %s { show | list } CGROUP [**effective**]\n"
> + " %s %s tree [CGROUP_ROOT] [**effective**]\n"
lgtm.
Takshak, Andrey, wdyt?
^ permalink raw reply
* Re: [PATCH v2 0/3] fix bugs when enable route_localnet
From: Zhiqiang Liu @ 2019-06-25 1:25 UTC (permalink / raw)
To: David Miller, luoshijie1
Cc: tgraf, dsahern, netdev, wangxiaogang3, mingfangsen, zhoukang7
In-Reply-To: <20190624.090325.163853495970223718.davem@davemloft.net>
On 2019/6/25 0:03, David Miller wrote:
> From: luoshijie <luoshijie1@huawei.com>
> Date: Tue, 18 Jun 2019 15:14:02 +0000
>
>> From: Shijie Luo <luoshijie1@huawei.com>
>>
>> When enable route_localnet, route of the 127/8 address is enabled.
>> But in some situations like arp_announce=2, ARP requests or reply
>> work abnormally.
>>
>> This patchset fix some bugs when enable route_localnet.
>>
>> Change History:
>> V2:
>> - Change a single patch to a patchset.
>> - Add bug fix for arp_ignore = 3.
>> - Add a couple of test for enabling route_localnet in selftests.
>
> Series applied to net-next, thanks.
>
Thanks again for you and David Ahern.
^ permalink raw reply
* Re: [PATCH bpf-next] bpftool: Add BPF_F_QUERY_EFFECTIVE support in bpftool cgroup [show|tree]
From: Takshak Chahande @ 2019-06-25 1:39 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Jakub Kicinski, Andrey Ignatov, Daniel Borkmann,
netdev@vger.kernel.org, ast@kernel.org, Kernel Team,
Stanislav Fomichev
In-Reply-To: <99a92dd0-4914-aeb5-709b-ed4615820aa0@fb.com>
Alexei Starovoitov <ast@fb.com> wrote on Mon [2019-Jun-24 18:23:28 -0700]:
> On 6/24/19 5:57 PM, Jakub Kicinski wrote:
> > On Mon, 24 Jun 2019 17:47:26 -0700, Jakub Kicinski wrote:
> >> I see. The local flag would not an option in getopt_long() sense, what
> >> I was thinking was about adding an "effective" keyword:
> >
> > Something like this, untested:
> >
> > --->8------------
> >
> > The BPF_F_QUERY_EFFECTIVE is a syscall flag, and fits nicely
> > as a subcommand option. We want to move away from global
> > options, anyway.
> >
> > We need a global variable because of nftw limitations.
> > Clean this flag on every invocations in case we run in
> > batch mode.
> >
> > NOTE the argv[1] use on the error path in do_show() looks
> > like a bug on it's own.
> > ---
> > .../bpftool/Documentation/bpftool-cgroup.rst | 24 +++----
> > tools/bpf/bpftool/Documentation/bpftool.rst | 6 +-
> > tools/bpf/bpftool/bash-completion/bpftool | 17 ++---
> > tools/bpf/bpftool/cgroup.c | 62 ++++++++++++-------
> > tools/bpf/bpftool/main.c | 7 +--
> > tools/bpf/bpftool/main.h | 3 +-
> > 6 files changed, 66 insertions(+), 53 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> > index 324df15bf4cc..4fde3dfad395 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
> > @@ -12,8 +12,7 @@ SYNOPSIS
> >
> > **bpftool** [*OPTIONS*] **cgroup** *COMMAND*
> >
> > - *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** }
> > - | { **-e** | **--effective** } }
> > + *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
> >
> > *COMMANDS* :=
> > { **show** | **list** | **tree** | **attach** | **detach** | **help** }
> > @@ -21,8 +20,8 @@ SYNOPSIS
> > CGROUP COMMANDS
> > ===============
> >
> > -| **bpftool** **cgroup { show | list }** *CGROUP*
> > -| **bpftool** **cgroup tree** [*CGROUP_ROOT*]
> > +| **bpftool** **cgroup { show | list }** *CGROUP* [**effective**]
> > +| **bpftool** **cgroup tree** [*CGROUP_ROOT*] [**effective**]
> > | **bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
> > | **bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
> > | **bpftool** **cgroup help**
> > @@ -35,13 +34,17 @@ CGROUP COMMANDS
> >
> > DESCRIPTION
> > ===========
> > - **bpftool cgroup { show | list }** *CGROUP*
> > + **bpftool cgroup { show | list }** *CGROUP* [**effective**]
> > List all programs attached to the cgroup *CGROUP*.
> >
> > Output will start with program ID followed by attach type,
> > attach flags and program name.
> >
> > - **bpftool cgroup tree** [*CGROUP_ROOT*]
> > + If **effective** is specified retrieve effective programs that
> > + will execute for events within a cgroup. This includes
> > + inherited along with attached ones.
> > +
> > + **bpftool cgroup tree** [*CGROUP_ROOT*] [**effective**]
> > Iterate over all cgroups in *CGROUP_ROOT* and list all
> > attached programs. If *CGROUP_ROOT* is not specified,
> > bpftool uses cgroup v2 mountpoint.
> > @@ -50,6 +53,10 @@ DESCRIPTION
> > commands: it starts with absolute cgroup path, followed by
> > program ID, attach type, attach flags and program name.
> >
> > + If **effective** is specified retrieve effective programs that
> > + will execute for events within a cgroup. This includes
> > + inherited along with attached ones.
> > +
> > **bpftool cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
> > Attach program *PROG* to the cgroup *CGROUP* with attach type
> > *ATTACH_TYPE* and optional *ATTACH_FLAGS*.
> > @@ -122,11 +129,6 @@ OPTIONS
> > Print all logs available from libbpf, including debug-level
> > information.
> >
> > - -e, --effective
> > - Retrieve effective programs that will execute for events
> > - within a cgroup. This includes inherited along with attached
> > - ones.
> > -
> > EXAMPLES
> > ========
> > |
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
> > index d2f76b55988d..6a9c52ef84a9 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool.rst
> > @@ -19,7 +19,7 @@ SYNOPSIS
> > *OBJECT* := { **map** | **program** | **cgroup** | **perf** | **net** | **feature** }
> >
> > *OPTIONS* := { { **-V** | **--version** } | { **-h** | **--help** }
> > - | { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-e** | **--effective** } }
> > + | { **-j** | **--json** } [{ **-p** | **--pretty** }] }
> >
> > *MAP-COMMANDS* :=
> > { **show** | **list** | **create** | **dump** | **update** | **lookup** | **getnext**
> > @@ -71,10 +71,6 @@ OPTIONS
> > includes logs from libbpf as well as from the verifier, when
> > attempting to load programs.
> >
> > - -e, --effective
> > - Retrieve effective programs that will execute for events
> > - within a cgroup. This includes inherited along with attached ones.
> > -
> > SEE ALSO
> > ========
> > **bpf**\ (2),
> > diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> > index c98cb99867f6..de84ae06ae4e 100644
> > --- a/tools/bpf/bpftool/bash-completion/bpftool
> > +++ b/tools/bpf/bpftool/bash-completion/bpftool
> > @@ -187,7 +187,7 @@ _bpftool()
> >
> > # Deal with options
> > if [[ ${words[cword]} == -* ]]; then
> > - local c='--version --json --pretty --bpffs --mapcompat --debug --effective'
> > + local c='--version --json --pretty --bpffs --mapcompat --debug'
> > COMPREPLY=( $( compgen -W "$c" -- "$cur" ) )
> > return 0
> > fi
> > @@ -678,12 +678,15 @@ _bpftool()
> > ;;
> > cgroup)
> > case $command in
> > - show|list)
> > - _filedir
> > - return 0
> > - ;;
> > - tree)
> > - _filedir
> > + show|list|tree)
> > + case $cword in
> > + 3)
> > + _filedir
> > + ;;
> > + 4)
> > + COMPREPLY=( $( compgen -W 'effective' -- "$cur" ) )
> > + ;;
> > + esac
> > return 0
> > ;;
> > attach|detach)
> > diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> > index 1bb2a751107a..88b80616d47b 100644
> > --- a/tools/bpf/bpftool/cgroup.c
> > +++ b/tools/bpf/bpftool/cgroup.c
> > @@ -28,6 +28,8 @@
> > " connect6 | sendmsg4 | sendmsg6 |\n" \
> > " recvmsg4 | recvmsg6 | sysctl }"
> >
> > +static unsigned int query_flags;
> > +
> > static const char * const attach_type_strings[] = {
> > [BPF_CGROUP_INET_INGRESS] = "ingress",
> > [BPF_CGROUP_INET_EGRESS] = "egress",
> > @@ -104,8 +106,8 @@ static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
> > __u32 prog_cnt = 0;
> > int ret;
> >
> > - ret = bpf_prog_query(cgroup_fd, type, query_flags, NULL, NULL,
> > - &prog_cnt);
> > + ret = bpf_prog_query(cgroup_fd, type, query_flags, NULL,
> > + NULL, &prog_cnt);
> > if (ret)
> > return -1;
> >
> > @@ -156,20 +158,30 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
> > static int do_show(int argc, char **argv)
> > {
> > enum bpf_attach_type type;
> > + const char *path;
> > int cgroup_fd;
> > int ret = -1;
> >
> > - if (argc < 1) {
> > - p_err("too few parameters for cgroup show");
> > - goto exit;
> > - } else if (argc > 1) {
> > - p_err("too many parameters for cgroup show");
> > - goto exit;
> > + query_flags = 0;
> > +
> > + if (!REQ_ARGS(1))
> > + return -1;
> > + path = GET_ARG();
> > +
> > + while (argc) {
> > + if (is_prefix(*argv, "effective")) {
> > + query_flags |= BPF_F_QUERY_EFFECTIVE;
> > + NEXT_ARG();
> > + } else {
> > + p_err("expected no more arguments, 'effective', got: '%s'?",
> > + *argv);
> > + return -1;
> > + }
> > }
> >
> > - cgroup_fd = open(argv[0], O_RDONLY);
> > + cgroup_fd = open(path, O_RDONLY);
> > if (cgroup_fd < 0) {
> > - p_err("can't open cgroup %s", argv[1]);
> > + p_err("can't open cgroup %s", path);
> > goto exit;
> > }
> >
> > @@ -295,23 +307,29 @@ static int do_show_tree(int argc, char **argv)
> > char *cgroup_root;
> > int ret;
> >
> > - switch (argc) {
> > - case 0:
> > + query_flags = 0;
> > +
> > + if (!argc) {
> > cgroup_root = find_cgroup_root();
> > if (!cgroup_root) {
> > p_err("cgroup v2 isn't mounted");
> > return -1;
> > }
> > - break;
> > - case 1:
> > - cgroup_root = argv[0];
> > - break;
> > - default:
> > - p_err("too many parameters for cgroup tree");
> > - return -1;
> > + } else {
> > + cgroup_root = GET_ARG();
> > +
> > + while (argc) {
> > + if (is_prefix(*argv, "effective")) {
> > + query_flags |= BPF_F_QUERY_EFFECTIVE;
> > + NEXT_ARG();
> > + } else {
> > + p_err("expected no more arguments, 'effective', got: '%s'?",
> > + *argv);
> > + return -1;
> > + }
> > + }
> > }
> >
> > -
> > if (json_output)
> > jsonw_start_array(json_wtr);
> > else
> > @@ -457,8 +475,8 @@ static int do_help(int argc, char **argv)
> > }
> >
> > fprintf(stderr,
> > - "Usage: %s %s { show | list } CGROUP\n"
> > - " %s %s tree [CGROUP_ROOT]\n"
> > + "Usage: %s %s { show | list } CGROUP [**effective**]\n"
> > + " %s %s tree [CGROUP_ROOT] [**effective**]\n"
>
> lgtm.
> Takshak, Andrey, wdyt?
ya, looks fine to me.
--Takshak
^ permalink raw reply
* Re: [PATCH v2 0/3] fix bugs when enable route_localnet
From: Luoshijie (Poincare Lab) @ 2019-06-25 1:50 UTC (permalink / raw)
To: David Miller
Cc: tgraf, dsahern, netdev, liuzhiqiang26, wangxiaogang3, mingfangsen,
zhoukang7
In-Reply-To: <20190624.090325.163853495970223718.davem@davemloft.net>
On 2019/6/25 0:03, David Miller wrote:
> From: luoshijie <luoshijie1@huawei.com>
> Date: Tue, 18 Jun 2019 15:14:02 +0000
>
>> From: Shijie Luo <luoshijie1@huawei.com>
>>
>> When enable route_localnet, route of the 127/8 address is enabled.
>> But in some situations like arp_announce=2, ARP requests or reply
>> work abnormally.
>>
>> This patchset fix some bugs when enable route_localnet.
>>
>> Change History:
>> V2:
>> - Change a single patch to a patchset.
>> - Add bug fix for arp_ignore = 3.
>> - Add a couple of test for enabling route_localnet in selftests.
>
> Series applied to net-next, thanks.
>
Thanks a lot, and I'm truly grateful for advice and help of David Ahern.
^ permalink raw reply
* Re: [PATCH net-next] net: link_watch: prevent starvation when processing linkwatch wq
From: Yunsheng Lin @ 2019-06-25 2:28 UTC (permalink / raw)
To: David Miller
Cc: hkallweit1, f.fainelli, netdev, linux-kernel, linuxarm, pbonzini,
rkrcmar, kvm, xuwei (O)
In-Reply-To: <6e9b41c9-6edb-be7f-07ee-5480162a227e@huawei.com>
On 2019/5/29 16:59, Yunsheng Lin wrote:
> On 2019/5/29 14:58, David Miller wrote:
>> From: Yunsheng Lin <linyunsheng@huawei.com>
>> Date: Mon, 27 May 2019 09:47:54 +0800
>>
>>> When user has configured a large number of virtual netdev, such
>>> as 4K vlans, the carrier on/off operation of the real netdev
>>> will also cause it's virtual netdev's link state to be processed
>>> in linkwatch. Currently, the processing is done in a work queue,
>>> which may cause worker starvation problem for other work queue.
>>>
>>> This patch releases the cpu when link watch worker has processed
>>> a fixed number of netdev' link watch event, and schedule the
>>> work queue again when there is still link watch event remaining.
>>>
>>> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
>>
>> Why not rtnl_unlock(); yield(); rtnl_lock(); every "100" events
>> processed?
>>
>> That seems better than adding all of this overhead to reschedule the
>> workqueue every 100 items.
>
> One minor concern, the above solution does not seem to solve the cpu
> starvation for other normal workqueue which was scheduled on the same
> cpu as linkwatch. Maybe I misunderstand the workqueue or there is other
> consideration here? :)
>
> Anyway, I will implemet it as you suggested and test it before posting V2.
> Thanks.
Hi, David
I stress tested the above solution with a lot of vlan dev and qemu-kvm with
vf passthrongh mode, the linkwatch wq sometimes block the irqfd_inject wq
when they are scheduled on the same cpu, which may cause interrupt delay
problem for vm.
Rescheduling workqueue every 100 items does give irqfd_inject wq to run sooner,
which alleviate the interrupt delay problems for vm.
So It is ok for me to fall back to reschedule the link watch wq every 100 items,
or is there a better way to fix it properly?
>
>>
>> .
>>
>
>
> .
>
^ permalink raw reply
* [PATCH net-next] xdp: Make __mem_id_disconnect static
From: YueHaibing @ 2019-06-25 2:31 UTC (permalink / raw)
To: davem, ast, daniel, jakub.kicinski, hawk, john.fastabend
Cc: linux-kernel, netdev, xdp-newbies, bpf, YueHaibing
Fix sparse warning:
net/core/xdp.c:88:6: warning:
symbol '__mem_id_disconnect' was not declared. Should it be static?
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
net/core/xdp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/xdp.c b/net/core/xdp.c
index b29d7b5..829377c 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -85,7 +85,7 @@ static void __xdp_mem_allocator_rcu_free(struct rcu_head *rcu)
kfree(xa);
}
-bool __mem_id_disconnect(int id, bool force)
+static bool __mem_id_disconnect(int id, bool force)
{
struct xdp_mem_allocator *xa;
bool safe_to_remove = true;
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] tty: ldisc: Fix misuse of proc_dointvec "ldisc_autoload"
From: Eiichi Tsukata @ 2019-06-25 3:08 UTC (permalink / raw)
To: gregkh, jslaby, davem, kuznet, yoshfuji, linux-kernel, netdev
Cc: Eiichi Tsukata
/proc/sys/dev/tty/ldisc_autoload assumes given value to be 0 or 1. Use
proc_dointvec_minmax instead of proc_dointvec.
Fixes: 7c0cca7c847e "(tty: ldisc: add sysctl to prevent autoloading of ldiscs)"
Signed-off-by: Eiichi Tsukata <devel@etsukata.com>
---
drivers/tty/tty_ldisc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index e38f104db174..a8ea7a35c94e 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -863,7 +863,7 @@ static struct ctl_table tty_table[] = {
.data = &tty_ldisc_autoload,
.maxlen = sizeof(tty_ldisc_autoload),
.mode = 0644,
- .proc_handler = proc_dointvec,
+ .proc_handler = proc_dointvec_minmax,
.extra1 = &zero,
.extra2 = &one,
},
--
2.21.0
^ permalink raw reply related
* [PATCH 2/2] net/ipv6: Fix misuse of proc_dointvec "skip_notify_on_dev_down"
From: Eiichi Tsukata @ 2019-06-25 3:08 UTC (permalink / raw)
To: gregkh, jslaby, davem, kuznet, yoshfuji, linux-kernel, netdev
Cc: Eiichi Tsukata
In-Reply-To: <20190625030801.24538-1-devel@etsukata.com>
/proc/sys/net/ipv6/route/skip_notify_on_dev_down assumes given value to be
0 or 1. Use proc_dointvec_minmax instead of proc_dointvec.
Fixes: 7c6bb7d2faaf ("net/ipv6: Add knob to skip DELROUTE message ondevice down")
Signed-off-by: Eiichi Tsukata <devel@etsukata.com>
---
net/ipv6/route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 11ad62effd56..aade636c6be6 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -5281,7 +5281,7 @@ static struct ctl_table ipv6_route_table_template[] = {
.data = &init_net.ipv6.sysctl.skip_notify_on_dev_down,
.maxlen = sizeof(int),
.mode = 0644,
- .proc_handler = proc_dointvec,
+ .proc_handler = proc_dointvec_minmax,
.extra1 = &zero,
.extra2 = &one,
},
--
2.21.0
^ permalink raw reply related
* Re: Removing skb_orphan() from ip_rcv_core()
From: Joe Stringer @ 2019-06-25 3:17 UTC (permalink / raw)
To: Florian Westphal
Cc: Joe Stringer, Eric Dumazet, netdev, john fastabend,
Daniel Borkmann, Lorenz Bauer, Jakub Sitnicki, Paolo Abeni
In-Reply-To: <20190621205935.og7ajx57j7usgycq@breakpoint.cc>
On Fri, Jun 21, 2019 at 1:59 PM Florian Westphal <fw@strlen.de> wrote:
>
> Joe Stringer <joe@wand.net.nz> wrote:
> > As discussed during LSFMM, I've been looking at adding something like
> > an `skb_sk_assign()` helper to BPF so that logic similar to TPROXY can
> > be implemented with integration into other BPF logic, however
> > currently any attempts to do so are blocked by the skb_orphan() call
> > in ip_rcv_core() (which will effectively ignore any socket assign
> > decision made by the TC BPF program).
> >
> > Recently I was attempting to remove the skb_orphan() call, and I've
> > been trying different things but there seems to be some context I'm
> > missing. Here's the core of the patch:
> >
> > diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
> > index ed97724c5e33..16aea980318a 100644
> > --- a/net/ipv4/ip_input.c
> > +++ b/net/ipv4/ip_input.c
> > @@ -500,8 +500,6 @@ static struct sk_buff *ip_rcv_core(struct sk_buff
> > *skb, struct net *net)
> > memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
> > IPCB(skb)->iif = skb->skb_iif;
> >
> > - /* Must drop socket now because of tproxy. */
> > - skb_orphan(skb);
> >
> > return skb;
> >
> > The statement that the socket must be dropped because of tproxy
> > doesn't make sense to me, because the PRE_ROUTING hook is hit after
> > this, which will call into the tproxy logic and eventually
> > nf_tproxy_assign_sock() which already does the skb_orphan() itself.
>
> in comment: s/tproxy/skb_steal_sock/
For reference, I was following the path like this:
ip_rcv()
( -> ip_rcv_core() for skb_orphan)
-> NF_INET_PRE_ROUTING hook
(... invoke iptables hooks)
-> iptable_mangle_hook()
-> ipt_do_table()
... -> tproxy_tg4()
... -> nf_tproxy_assign_sock()
-> skb_orphan()
(... finish iptables processing)
( -> ip_rcv_finish())
( ... -> ip_rcv_finish_core() for early demux / route lookup )
(... -> dst_input())
(... -> tcp_v4_rcv())
( -> __inet_lookup_skb())
( -> skb_steal_sock() )
> at least thats what I concluded a few years ago when I looked into
> the skb_oprhan() need.
>
> IIRC some device drivers use skb->sk for backpressure, so without this
> non-tcp socket would be stolen by skb_steal_sock.
Do you happen to recall which device drivers? Or have some idea of a
list I could try to go through? Are you referring to virtual drivers
like veth or something else?
> We also recently removed skb orphan when crossing netns:
>
> commit 9c4c325252c54b34d53b3d0ffd535182b744e03d
> Author: Flavio Leitner <fbl@redhat.com>
> skbuff: preserve sock reference when scrubbing the skb.
>
> So thats another case where this orphan is needed.
Presumably the orphan is only needed in this case if the packet
crosses a namespace and then is subsequently passed back into the
stack?
> What could be done is adding some way to delay/defer the orphaning
> further, but we would need at the very least some annotation for
> skb_steal_sock to know when the skb->sk is really from TPROXY or
> if it has to orphan.
Eric mentions in another response to this thread that skb_orphan()
should be called from any ndo_start_xmit() which sends traffic back
into the stack. With that, presumably we would be pushing the
orphaning earlier such that the only way that the skb->sk ref can be
non-NULL around this point in receive would be because it was
specifically set by some kind of tproxy logic?
> Same for the safety check in the forwarding path.
> Netfilter modules need o be audited as well, they might make assumptions
> wrt. skb->sk being inet sockets (set by local stack or early demux).
>
> > However, if I drop these lines then I end up causing sockets to
> > release references too many times. Seems like if we don't orphan the
> > skb here, then later logic assumes that we have one more reference
> > than we actually have, and decrements the count when it shouldn't
> > (perhaps the skb_steal_sock() call in __inet_lookup_skb() which seems
> > to assume we always have a reference to the socket?)
>
> We might be calling the wrong destructor (i.e., the one set by tcp
> receive instead of the one set at tx time)?
Hmm, interesting thought. Sure enough, with a bit of bpftrace
debugging we find it's tcp_wfree():
$ cat ip_rcv.bt
#include <linux/skbuff.h>
kprobe:ip_rcv {
$sk = ((struct sk_buff *)arg0)->sk;
$des = ((struct sk_buff *)arg0)->destructor;
if ($sk) {
if ($des) {
printf("received %s on %s with sk destructor %s
set\n", str(arg0), str(arg1), ksym($des));
@ip4_stacks[kstack] = count();
}
}
}
$ sudo bpftrace ip_rcv.bt
Attaching 1 probe...
received on eth0 with sk destructor tcp_wfree set
^C
@ip4_stacks[
ip_rcv+1
__netif_receive_skb+24
process_backlog+179
net_rx_action+304
__do_softirq+220
do_softirq_own_stack+42
do_softirq.part.17+70
__local_bh_enable_ip+101
ip_finish_output2+421
__ip_finish_output+187
ip_finish_output+44
ip_output+109
ip_local_out+59
__ip_queue_xmit+368
ip_queue_xmit+16
__tcp_transmit_skb+1303
tcp_connect+2758
tcp_v4_connect+1135
__inet_stream_connect+214
inet_stream_connect+59
__sys_connect+237
__x64_sys_connect+26
do_syscall_64+90
entry_SYSCALL_64_after_hwframe+68
]: 1
Is there a solution here where we call the destructor if it's not
sock_efree()? When the socket is later stolen, it will only return the
reference via a call to sock_put(), so presumably at that point in the
stack we already assume that the skb->destructor is not one of these
other destructors (otherwise we wouldn't release the resources
correctly).
^ permalink raw reply
* Re: [RFC V1 net-next 1/1] net: ena: implement XDP drop support
From: Machulsky, Zorik @ 2019-06-25 3:19 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jubran, Samih
Cc: davem@davemloft.net, netdev@vger.kernel.org, Woodhouse, David,
Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt,
Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel,
Saidi, Ali, Herrenschmidt, Benjamin, Kiyanovski, Arthur,
Daniel Borkmann
In-Reply-To: <20190623162133.6b7f24e1@carbon>
On 6/23/19, 7:21 AM, "Jesper Dangaard Brouer" <brouer@redhat.com> wrote:
On Sun, 23 Jun 2019 10:06:49 +0300 <sameehj@amazon.com> wrote:
> This commit implements the basic functionality of drop/pass logic in the
> ena driver.
Usually we require a driver to implement all the XDP return codes,
before we accept it. But as Daniel and I discussed with Zorik during
NetConf[1], we are going to make an exception and accept the driver
if you also implement XDP_TX.
As we trust that Zorik/Amazon will follow and implement XDP_REDIRECT
later, given he/you wants AF_XDP support which requires XDP_REDIRECT.
Jesper, thanks for your comments and very helpful discussion during NetConf!
That's the plan, as we agreed. From our side I would like to reiterate again the
importance of multi-buffer support by xdp frame. We would really prefer not
to see our MTU shrinking because of xdp support.
[1] http://vger.kernel.org/netconf2019.html
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH v4 5/7] lib/hexdump.c: Allow multiple groups to be separated by lines '|'
From: Alastair D'Silva @ 2019-06-25 3:17 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <20190625031726.12173-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
With the wider display format, it can become hard to identify how many
bytes into the line you are looking at.
The patch adds new flags to hex_dump_to_buffer() and print_hex_dump() to
print vertical lines to separate every N groups of bytes.
eg.
buf:00000000: 454d414e 43415053|4e495f45 00584544 NAMESPAC|E_INDEX.
buf:00000010: 00000000 00000002|00000000 00000000 ........|........
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
include/linux/printk.h | 3 +++
lib/hexdump.c | 59 ++++++++++++++++++++++++++++++++++++------
2 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index f0761b3a2d5d..ae80d7af82ab 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -485,6 +485,9 @@ enum {
#define HEXDUMP_ASCII BIT(0)
#define HEXDUMP_SUPPRESS_REPEATED BIT(1)
+#define HEXDUMP_2_GRP_LINES BIT(2)
+#define HEXDUMP_4_GRP_LINES BIT(3)
+#define HEXDUMP_8_GRP_LINES BIT(4)
extern int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
int groupsize, char *linebuf, size_t linebuflen,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 1bf838c1a568..4dcf759fe048 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -77,6 +77,23 @@ char *bin2hex(char *dst, const void *src, size_t count)
}
EXPORT_SYMBOL(bin2hex);
+static const char *group_separator(int group, u64 flags)
+{
+ if (group == 0)
+ return " ";
+
+ if ((flags & HEXDUMP_8_GRP_LINES) && !((group) % 8))
+ return "|";
+
+ if ((flags & HEXDUMP_4_GRP_LINES) && !((group) % 4))
+ return "|";
+
+ if ((flags & HEXDUMP_2_GRP_LINES) && !((group) % 2))
+ return "|";
+
+ return " ";
+}
+
/**
* hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
* @buf: data blob to dump
@@ -87,6 +104,9 @@ EXPORT_SYMBOL(bin2hex);
* @linebuflen: total size of @linebuf, including space for terminating NUL
* @flags: A bitwise OR of the following flags:
* HEXDUMP_ASCII: include ASCII after the hex output
+ * HEXDUMP_2_GRP_LINES: insert a '|' after every 2 groups
+ * HEXDUMP_4_GRP_LINES: insert a '|' after every 4 groups
+ * HEXDUMP_8_GRP_LINES: insert a '|' after every 8 groups
*
* hex_dump_to_buffer() works on one "line" of output at a time, converting
* <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
@@ -119,6 +139,7 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
int j, lx = 0;
int ascii_column;
int ret;
+ int line_chars = 0;
if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
@@ -145,7 +166,8 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
for (j = 0; j < ngroups; j++) {
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%16.16llx", j ? " " : "",
+ "%s%16.16llx",
+ j ? group_separator(j, flags) : "",
get_unaligned(ptr8 + j));
if (ret >= linebuflen - lx)
goto overflow1;
@@ -156,7 +178,8 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
for (j = 0; j < ngroups; j++) {
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%8.8x", j ? " " : "",
+ "%s%8.8x",
+ j ? group_separator(j, flags) : "",
get_unaligned(ptr4 + j));
if (ret >= linebuflen - lx)
goto overflow1;
@@ -167,7 +190,8 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
for (j = 0; j < ngroups; j++) {
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%4.4x", j ? " " : "",
+ "%s%4.4x",
+ j ? group_separator(j, flags) : "",
get_unaligned(ptr2 + j));
if (ret >= linebuflen - lx)
goto overflow1;
@@ -197,11 +221,26 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
goto overflow2;
linebuf[lx++] = ' ';
}
+
+ if (flags & HEXDUMP_2_GRP_LINES)
+ line_chars = groupsize * 2;
+ if (flags & HEXDUMP_4_GRP_LINES)
+ line_chars = groupsize * 4;
+ if (flags & HEXDUMP_8_GRP_LINES)
+ line_chars = groupsize * 8;
+
for (j = 0; j < len; j++) {
if (linebuflen < lx + 2)
goto overflow2;
ch = ptr[j];
linebuf[lx++] = (isascii(ch) && isprint(ch)) ? ch : '.';
+
+ if (line_chars && ((j + 1) < len) &&
+ ((j + 1) % line_chars == 0)) {
+ if (linebuflen < lx + 2)
+ goto overflow2;
+ linebuf[lx++] = '|';
+ }
}
nil:
linebuf[lx] = '\0';
@@ -209,7 +248,8 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
overflow2:
linebuf[lx++] = '\0';
overflow1:
- return (flags & HEXDUMP_ASCII) ? ascii_column + len :
+ return (flags & HEXDUMP_ASCII) ? ascii_column + len +
+ (len - 1) / line_chars :
(groupsize * 2 + 1) * ngroups - 1;
}
EXPORT_SYMBOL(hex_dump_to_buffer_ext);
@@ -247,7 +287,7 @@ static void announce_skipped(const char *level, const char *prefix_str,
if (count == 0)
return;
- printk("%s%s ** Skipped %lu bytes of value 0x%x **\n",
+ printk("%s%s ** Skipped %lu bytes of value 0x%02x **\n",
level, prefix_str, count, val);
}
@@ -267,6 +307,9 @@ static void announce_skipped(const char *level, const char *prefix_str,
* HEXDUMP_ASCII: include ASCII after the hex output
* HEXDUMP_SUPPRESS_REPEATED: suppress repeated lines of identical
* bytes
+ * HEXDUMP_2_GRP_LINES: insert a '|' after every 2 groups
+ * HEXDUMP_4_GRP_LINES: insert a '|' after every 4 groups
+ * HEXDUMP_8_GRP_LINES: insert a '|' after every 8 groups
*
* Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump
* to the kernel log at the specified kernel log level, with an optional
@@ -296,15 +339,15 @@ void print_hex_dump_ext(const char *level, const char *prefix_str,
u8 skipped_val = 0;
size_t skipped = 0;
-
if (rowsize % groupsize)
rowsize -= rowsize % groupsize;
/*
* Worst case line length:
- * 2 hex chars + space per byte in, 2 spaces, 1 char per byte in, NULL
+ * 2 hex chars + space per byte in, 2 spaces, 1 char per byte in,
+ * 1 char per N groups, NULL
*/
- linebuf_len = rowsize * 3 + 2 + rowsize + 1;
+ linebuf_len = rowsize * 3 + 2 + rowsize + rowsize / groupsize + 1;
linebuf = kzalloc(linebuf_len, GFP_KERNEL);
if (!linebuf) {
printk("%s%shexdump: Could not alloc %u bytes for buffer\n",
--
2.21.0
^ permalink raw reply related
* [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Alastair D'Silva @ 2019-06-25 3:17 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <20190625031726.12173-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
In order to support additional features, rename hex_dump_to_buffer to
hex_dump_to_buffer_ext, and replace the ascii bool parameter with flags.
A wrapper is provided for callers that do not need anything but a basic
dump.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
drivers/gpu/drm/i915/intel_engine_cs.c | 5 +-
drivers/isdn/hardware/mISDN/mISDNisar.c | 10 ++--
drivers/mailbox/mailbox-test.c | 8 ++--
drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +-
.../net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +-
drivers/net/wireless/ath/ath10k/debug.c | 7 +--
.../net/wireless/intel/iwlegacy/3945-mac.c | 4 +-
drivers/platform/chrome/wilco_ec/debugfs.c | 10 ++--
drivers/scsi/scsi_logging.c | 8 ++--
drivers/staging/fbtft/fbtft-core.c | 2 +-
fs/seq_file.c | 6 ++-
include/linux/printk.h | 46 +++++++++++++++++--
lib/hexdump.c | 24 +++++-----
lib/test_hexdump.c | 10 ++--
14 files changed, 94 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index eea9bec04f1b..64189a0e5ec9 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len)
}
WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
- rowsize, sizeof(u32),
- line, sizeof(line),
- false) >= sizeof(line));
+ rowsize, sizeof(u32), line,
+ sizeof(line)) >= sizeof(line));
drm_printf(m, "[%04zx] %s\n", pos, line);
prev = buf + pos;
diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c b/drivers/isdn/hardware/mISDN/mISDNisar.c
index fd5c52f37802..84455b521246 100644
--- a/drivers/isdn/hardware/mISDN/mISDNisar.c
+++ b/drivers/isdn/hardware/mISDN/mISDNisar.c
@@ -70,8 +70,9 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, u8 len, u8 *msg)
int l = 0;
while (l < (int)len) {
- hex_dump_to_buffer(msg + l, len - l, 32, 1,
- isar->log, 256, 1);
+ hex_dump_to_buffer_ext(msg + l, len - l, 32, 1,
+ isar->log, 256,
+ HEXDUMP_ASCII);
pr_debug("%s: %s %02x: %s\n", isar->name,
__func__, l, isar->log);
l += 32;
@@ -99,8 +100,9 @@ rcv_mbox(struct isar_hw *isar, u8 *msg)
int l = 0;
while (l < (int)isar->clsb) {
- hex_dump_to_buffer(msg + l, isar->clsb - l, 32,
- 1, isar->log, 256, 1);
+ hex_dump_to_buffer_ext(msg + l, isar->clsb - l,
+ 32, 1, isar->log, 256,
+ HEXDUMP_ASCII);
pr_debug("%s: %s %02x: %s\n", isar->name,
__func__, l, isar->log);
l += 32;
diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index 4555d678fadd..ce334f88a3ee 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c
@@ -206,10 +206,10 @@ static ssize_t mbox_test_message_read(struct file *filp, char __user *userbuf,
ptr = tdev->rx_buffer;
while (l < MBOX_HEXDUMP_MAX_LEN) {
- hex_dump_to_buffer(ptr,
- MBOX_BYTES_PER_LINE,
- MBOX_BYTES_PER_LINE, 1, touser + l,
- MBOX_HEXDUMP_LINE_LEN, true);
+ hex_dump_to_buffer_ext(ptr,
+ MBOX_BYTES_PER_LINE,
+ MBOX_BYTES_PER_LINE, 1, touser + l,
+ MBOX_HEXDUMP_LINE_LEN, HEXDUMP_ASCII);
ptr += MBOX_BYTES_PER_LINE;
l += MBOX_HEXDUMP_LINE_LEN;
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 3dd0cecddba8..f0118fe35c41 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -2992,7 +2992,7 @@ void xgbe_print_pkt(struct net_device *netdev, struct sk_buff *skb, bool tx_rx)
unsigned int len = min(skb->len - i, 32U);
hex_dump_to_buffer(&skb->data[i], len, 32, 1,
- buffer, sizeof(buffer), false);
+ buffer, sizeof(buffer));
netdev_dbg(netdev, " %#06x: %s\n", i, buffer);
}
diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c b/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c
index eb1c6b03c329..53991c123c4d 100644
--- a/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c
+++ b/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c
@@ -349,7 +349,7 @@ void xlgmac_print_pkt(struct net_device *netdev,
unsigned int len = min(skb->len - i, 32U);
hex_dump_to_buffer(&skb->data[i], len, 32, 1,
- buffer, sizeof(buffer), false);
+ buffer, sizeof(buffer));
netdev_dbg(netdev, " %#06x: %s\n", i, buffer);
}
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 32d967a31c65..8d79f8fc694d 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2660,9 +2660,10 @@ void ath10k_dbg_dump(struct ath10k *ar,
"%s%08x: ",
(prefix ? prefix : ""),
(unsigned int)(ptr - buf));
- hex_dump_to_buffer(ptr, len - (ptr - buf), 16, 1,
- linebuf + linebuflen,
- sizeof(linebuf) - linebuflen, true);
+ hex_dump_to_buffer_ext(ptr, len - (ptr - buf), 16, 1,
+ linebuf + linebuflen,
+ sizeof(linebuf) - linebuflen,
+ HEXDUMP_ASCII);
dev_printk(KERN_DEBUG, ar->dev, "%s\n", linebuf);
}
}
diff --git a/drivers/net/wireless/intel/iwlegacy/3945-mac.c b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
index b82da75a9ae3..92d030109395 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
@@ -3231,8 +3231,8 @@ il3945_show_measurement(struct device *d, struct device_attribute *attr,
spin_unlock_irqrestore(&il->lock, flags);
while (size && PAGE_SIZE - len) {
- hex_dump_to_buffer(data + ofs, size, 16, 1, buf + len,
- PAGE_SIZE - len, true);
+ hex_dump_to_buffer_ext(data + ofs, size, 16, 1, buf + len,
+ PAGE_SIZE - len, HEXDUMP_ASCII);
len = strlen(buf);
if (PAGE_SIZE - len)
buf[len++] = '\n';
diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
index f163476d080d..6c0a443bdb1b 100644
--- a/drivers/platform/chrome/wilco_ec/debugfs.c
+++ b/drivers/platform/chrome/wilco_ec/debugfs.c
@@ -144,11 +144,11 @@ static ssize_t raw_read(struct file *file, char __user *user_buf, size_t count,
int fmt_len = 0;
if (debug_info->response_size) {
- fmt_len = hex_dump_to_buffer(debug_info->raw_data,
- debug_info->response_size,
- 16, 1, debug_info->formatted_data,
- sizeof(debug_info->formatted_data),
- true);
+ fmt_len = hex_dump_to_buffer_ext(debug_info->raw_data,
+ debug_info->response_size,
+ 16, 1, debug_info->formatted_data,
+ sizeof(debug_info->formatted_data),
+ HEXDUMP_ASCII);
/* Only return response the first time it is read */
debug_info->response_size = 0;
}
diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c
index 39b8cc4574b4..aead28e5c4cd 100644
--- a/drivers/scsi/scsi_logging.c
+++ b/drivers/scsi/scsi_logging.c
@@ -262,7 +262,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
"CDB[%02x]: ", k);
hex_dump_to_buffer(&cmd->cmnd[k], linelen,
16, 1, logbuf + off,
- logbuf_len - off, false);
+ logbuf_len - off);
}
dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s",
logbuf);
@@ -273,8 +273,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
if (!WARN_ON(off > logbuf_len - 49)) {
off += scnprintf(logbuf + off, logbuf_len - off, " ");
hex_dump_to_buffer(cmd->cmnd, cmd->cmd_len, 16, 1,
- logbuf + off, logbuf_len - off,
- false);
+ logbuf + off, logbuf_len - off);
}
out_printk:
dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf);
@@ -353,8 +352,7 @@ scsi_log_dump_sense(const struct scsi_device *sdev, const char *name, int tag,
off = sdev_format_header(logbuf, logbuf_len,
name, tag);
hex_dump_to_buffer(&sense_buffer[i], len, 16, 1,
- logbuf + off, logbuf_len - off,
- false);
+ logbuf + off, logbuf_len - off);
dev_printk(KERN_INFO, &sdev->sdev_gendev, "%s", logbuf);
}
scsi_log_release_buffer(logbuf);
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 9b07badf4c6c..9765a7962f0c 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -61,7 +61,7 @@ void fbtft_dbg_hex(const struct device *dev, int groupsize,
va_end(args);
hex_dump_to_buffer(buf, len, 32, groupsize, text + text_len,
- 512 - text_len, false);
+ 512 - text_len);
if (len > 32)
dev_info(dev, "%s ...\n", text);
diff --git a/fs/seq_file.c b/fs/seq_file.c
index abe27ec43176..5f3d656c5ed6 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -873,8 +873,10 @@ void seq_hex_dump(struct seq_file *m, const char *prefix_str, int prefix_type,
}
size = seq_get_buf(m, &buffer);
- ret = hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
- buffer, size, ascii);
+ ret = hex_dump_to_buffer_ext(ptr + i, linelen,
+ rowsize, groupsize,
+ buffer, size,
+ ascii ? HEXDUMP_ASCII : 0);
seq_commit(m, ret < size ? ret : -1);
seq_putc(m, '\n');
diff --git a/include/linux/printk.h b/include/linux/printk.h
index c0416d0eb9e2..f0761b3a2d5d 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -483,13 +483,51 @@ enum {
DUMP_PREFIX_OFFSET
};
-extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
- int groupsize, char *linebuf, size_t linebuflen,
- bool ascii);
-
#define HEXDUMP_ASCII BIT(0)
#define HEXDUMP_SUPPRESS_REPEATED BIT(1)
+extern int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
+ int groupsize, char *linebuf, size_t linebuflen,
+ u32 flags);
+
+/**
+ * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
+ * @buf: data blob to dump
+ * @len: number of bytes in the @buf
+ * @rowsize: number of bytes to print per line; must be a multiple of groupsize
+ * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
+ * @linebuf: where to put the converted data
+ * @linebuflen: total size of @linebuf, including space for terminating NUL
+ *
+ * hex_dump_to_buffer() works on one "line" of output at a time, converting
+ * <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
+ * until <rowsize> bytes have been emitted.
+ *
+ * Given a buffer of u8 data, hex_dump_to_buffer() converts the input data
+ * to a hex dump at the supplied memory location.
+ * The converted output is always NUL-terminated.
+ *
+ * E.g.:
+ * hex_dump_to_buffer(frame->data, frame->len, 16, 1,
+ * linebuf, sizeof(linebuf));
+ *
+ * example output buffer:
+ * 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f
+ *
+ * Return:
+ * The amount of bytes placed in the buffer without terminating NUL. If the
+ * output was truncated, then the return value is the number of bytes
+ * (excluding the terminating NUL) which would have been written to the final
+ * string if enough space had been available.
+ */
+static inline int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
+ int groupsize, char *linebuf, size_t linebuflen)
+{
+ return hex_dump_to_buffer_ext(buf, len, rowsize, groupsize,
+ linebuf, linebuflen, 0);
+}
+
+
#ifdef CONFIG_PRINTK
extern void print_hex_dump_ext(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 61dc625c32f5..1bf838c1a568 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -85,7 +85,8 @@ EXPORT_SYMBOL(bin2hex);
* @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
* @linebuf: where to put the converted data
* @linebuflen: total size of @linebuf, including space for terminating NUL
- * @ascii: include ASCII after the hex output
+ * @flags: A bitwise OR of the following flags:
+ * HEXDUMP_ASCII: include ASCII after the hex output
*
* hex_dump_to_buffer() works on one "line" of output at a time, converting
* <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
@@ -96,8 +97,8 @@ EXPORT_SYMBOL(bin2hex);
* The converted output is always NUL-terminated.
*
* E.g.:
- * hex_dump_to_buffer(frame->data, frame->len, 16, 1,
- * linebuf, sizeof(linebuf), true);
+ * hex_dump_to_buffer_ext(frame->data, frame->len, 16, 1,
+ * linebuf, sizeof(linebuf), HEXDUMP_ASCII);
*
* example output buffer:
* 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f @ABCDEFGHIJKLMNO
@@ -108,8 +109,9 @@ EXPORT_SYMBOL(bin2hex);
* (excluding the terminating NUL) which would have been written to the final
* string if enough space had been available.
*/
-int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
- char *linebuf, size_t linebuflen, bool ascii)
+int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
+ int groupsize, char *linebuf, size_t linebuflen,
+ u32 flags)
{
const u8 *ptr = buf;
int ngroups;
@@ -187,7 +189,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
if (j)
lx--;
}
- if (!ascii)
+ if (!(flags & HEXDUMP_ASCII))
goto nil;
while (lx < ascii_column) {
@@ -207,9 +209,10 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
overflow2:
linebuf[lx++] = '\0';
overflow1:
- return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1;
+ return (flags & HEXDUMP_ASCII) ? ascii_column + len :
+ (groupsize * 2 + 1) * ngroups - 1;
}
-EXPORT_SYMBOL(hex_dump_to_buffer);
+EXPORT_SYMBOL(hex_dump_to_buffer_ext);
#ifdef CONFIG_PRINTK
@@ -336,9 +339,8 @@ void print_hex_dump_ext(const char *level, const char *prefix_str,
skipped = 0;
}
- hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
- linebuf, linebuf_len,
- flags & HEXDUMP_ASCII);
+ hex_dump_to_buffer_ext(ptr + i, linelen, rowsize, groupsize,
+ linebuf, linebuf_len, flags);
switch (prefix_type) {
case DUMP_PREFIX_ADDRESS:
diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index 5e54525a937c..ad43218437f1 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -165,8 +165,9 @@ static void __init test_hexdump(size_t len, int rowsize, int groupsize,
total_tests++;
memset(real, FILL_CHAR, sizeof(real));
- hex_dump_to_buffer(data_b, len, rowsize, groupsize, real, sizeof(real),
- ascii);
+ hex_dump_to_buffer_ext(data_b, len, rowsize, groupsize,
+ real, sizeof(real),
+ ascii ? HEXDUMP_ASCII : 0);
memset(test, FILL_CHAR, sizeof(test));
test_hexdump_prepare_test(len, rowsize, groupsize, test, sizeof(test),
@@ -204,8 +205,9 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
memset(buf, FILL_CHAR, sizeof(buf));
- rc = hex_dump_to_buffer(data_b, len, rowsize, groupsize, buf, buflen,
- ascii);
+ rc = hex_dump_to_buffer_ext(data_b, len, rowsize, groupsize,
+ buf, buflen,
+ ascii ? HEXDUMP_ASCII : 0);
/*
* Caller must provide the data length multiple of groupsize. The
--
2.21.0
^ permalink raw reply related
* [PATCH v4 6/7] lib/hexdump.c: Allow multiple groups to be separated by spaces
From: Alastair D'Silva @ 2019-06-25 3:17 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <20190625031726.12173-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Similar to the previous patch, this patch separates groups by 2 spaces for
the hex fields, and 1 space for the ASCII field.
eg.
buf:00000000: 454d414e 43415053 4e495f45 00584544 NAMESPAC E_INDEX.
buf:00000010: 00000000 00000002 00000000 00000000 ........ ........
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
include/linux/printk.h | 3 ++
lib/hexdump.c | 65 +++++++++++++++++++++++++++++++-----------
2 files changed, 52 insertions(+), 16 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index ae80d7af82ab..1d082291facf 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -488,6 +488,9 @@ enum {
#define HEXDUMP_2_GRP_LINES BIT(2)
#define HEXDUMP_4_GRP_LINES BIT(3)
#define HEXDUMP_8_GRP_LINES BIT(4)
+#define HEXDUMP_2_GRP_SPACES BIT(5)
+#define HEXDUMP_4_GRP_SPACES BIT(6)
+#define HEXDUMP_8_GRP_SPACES BIT(7)
extern int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
int groupsize, char *linebuf, size_t linebuflen,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 4dcf759fe048..e09e3cf8e595 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -91,9 +91,37 @@ static const char *group_separator(int group, u64 flags)
if ((flags & HEXDUMP_2_GRP_LINES) && !((group) % 2))
return "|";
+ if ((flags & HEXDUMP_8_GRP_SPACES) && !((group) % 8))
+ return " ";
+
+ if ((flags & HEXDUMP_4_GRP_SPACES) && !((group) % 4))
+ return " ";
+
+ if ((flags & HEXDUMP_2_GRP_SPACES) && !((group) % 2))
+ return " ";
+
return " ";
}
+static void separator_parameters(u64 flags, int groupsize, int *sep_chars,
+ char *sep)
+{
+ if (flags & (HEXDUMP_2_GRP_LINES | HEXDUMP_2_GRP_SPACES))
+ *sep_chars = groupsize * 2;
+ if (flags & (HEXDUMP_4_GRP_LINES | HEXDUMP_4_GRP_SPACES))
+ *sep_chars = groupsize * 4;
+ if (flags & (HEXDUMP_8_GRP_LINES | HEXDUMP_8_GRP_SPACES))
+ *sep_chars = groupsize * 8;
+
+ if (flags & (HEXDUMP_2_GRP_LINES | HEXDUMP_4_GRP_LINES |
+ HEXDUMP_8_GRP_LINES))
+ *sep = '|';
+
+ if (flags & (HEXDUMP_2_GRP_SPACES | HEXDUMP_4_GRP_SPACES |
+ HEXDUMP_8_GRP_SPACES))
+ *sep = ' ';
+}
+
/**
* hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
* @buf: data blob to dump
@@ -107,6 +135,9 @@ static const char *group_separator(int group, u64 flags)
* HEXDUMP_2_GRP_LINES: insert a '|' after every 2 groups
* HEXDUMP_4_GRP_LINES: insert a '|' after every 4 groups
* HEXDUMP_8_GRP_LINES: insert a '|' after every 8 groups
+ * HEXDUMP_2_GRP_SPACES: insert a ' ' after every 2 groups
+ * HEXDUMP_4_GRP_SPACES: insert a ' ' after every 4 groups
+ * HEXDUMP_8_GRP_SPACES: insert a ' ' after every 8 groups
*
* hex_dump_to_buffer() works on one "line" of output at a time, converting
* <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
@@ -139,7 +170,8 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
int j, lx = 0;
int ascii_column;
int ret;
- int line_chars = 0;
+ int sep_chars = 0;
+ char sep = 0;
if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
@@ -153,8 +185,14 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
len = rowsize;
ngroups = len / groupsize;
+
ascii_column = rowsize * 2 + rowsize / groupsize + 1;
+ // space separators use 2 spaces in the hex output
+ separator_parameters(flags, groupsize, &sep_chars, &sep);
+ if (sep == ' ')
+ ascii_column += rowsize / sep_chars;
+
if (!linebuflen)
goto overflow1;
@@ -222,24 +260,17 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
linebuf[lx++] = ' ';
}
- if (flags & HEXDUMP_2_GRP_LINES)
- line_chars = groupsize * 2;
- if (flags & HEXDUMP_4_GRP_LINES)
- line_chars = groupsize * 4;
- if (flags & HEXDUMP_8_GRP_LINES)
- line_chars = groupsize * 8;
-
for (j = 0; j < len; j++) {
if (linebuflen < lx + 2)
goto overflow2;
ch = ptr[j];
linebuf[lx++] = (isascii(ch) && isprint(ch)) ? ch : '.';
- if (line_chars && ((j + 1) < len) &&
- ((j + 1) % line_chars == 0)) {
+ if (sep_chars && ((j + 1) < len) &&
+ ((j + 1) % sep_chars == 0)) {
if (linebuflen < lx + 2)
goto overflow2;
- linebuf[lx++] = '|';
+ linebuf[lx++] = sep;
}
}
nil:
@@ -248,9 +279,11 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
overflow2:
linebuf[lx++] = '\0';
overflow1:
- return (flags & HEXDUMP_ASCII) ? ascii_column + len +
- (len - 1) / line_chars :
- (groupsize * 2 + 1) * ngroups - 1;
+ if (flags & HEXDUMP_ASCII)
+ return ascii_column + len + (len - 1) / sep_chars;
+
+ return groupsize * 2 * ngroups +
+ ((sep == ' ') ? 2 : 1) * (ngroups - 1);
}
EXPORT_SYMBOL(hex_dump_to_buffer_ext);
@@ -345,9 +378,9 @@ void print_hex_dump_ext(const char *level, const char *prefix_str,
/*
* Worst case line length:
* 2 hex chars + space per byte in, 2 spaces, 1 char per byte in,
- * 1 char per N groups, NULL
+ * 2 char per N groups, NULL
*/
- linebuf_len = rowsize * 3 + 2 + rowsize + rowsize / groupsize + 1;
+ linebuf_len = rowsize * 3 + 2 + rowsize + 2 * rowsize / groupsize + 1;
linebuf = kzalloc(linebuf_len, GFP_KERNEL);
if (!linebuf) {
printk("%s%shexdump: Could not alloc %u bytes for buffer\n",
--
2.21.0
^ permalink raw reply related
* [PATCH v4 7/7] lib/hexdump.c: Optionally retain byte ordering
From: Alastair D'Silva @ 2019-06-25 3:17 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <20190625031726.12173-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
The behaviour of hexdump groups is to print the data out as if
it was a native-endian number.
This patch tweaks the documentation to make this clear, and also
adds the HEXDUMP_RETAIN_BYTE_ORDER flag to allow groups of
multiple bytes to be printed without affecting the ordering
of the printed bytes.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
include/linux/printk.h | 1 +
lib/hexdump.c | 30 ++++++++++++++++----
lib/test_hexdump.c | 62 ++++++++++++++++++++++++++++--------------
3 files changed, 68 insertions(+), 25 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 1d082291facf..ed1a79aa9695 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -491,6 +491,7 @@ enum {
#define HEXDUMP_2_GRP_SPACES BIT(5)
#define HEXDUMP_4_GRP_SPACES BIT(6)
#define HEXDUMP_8_GRP_SPACES BIT(7)
+#define HEXDUMP_RETAIN_BYTE_ORDER BIT(8)
extern int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
int groupsize, char *linebuf, size_t linebuflen,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index e09e3cf8e595..29024eccf5da 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -127,7 +127,8 @@ static void separator_parameters(u64 flags, int groupsize, int *sep_chars,
* @buf: data blob to dump
* @len: number of bytes in the @buf
* @rowsize: number of bytes to print per line; must be a multiple of groupsize
- * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
+ * @groupsize: number of bytes to convert to a native endian number and print:
+ * 1, 2, 4, 8; default = 1
* @linebuf: where to put the converted data
* @linebuflen: total size of @linebuf, including space for terminating NUL
* @flags: A bitwise OR of the following flags:
@@ -138,6 +139,9 @@ static void separator_parameters(u64 flags, int groupsize, int *sep_chars,
* HEXDUMP_2_GRP_SPACES: insert a ' ' after every 2 groups
* HEXDUMP_4_GRP_SPACES: insert a ' ' after every 4 groups
* HEXDUMP_8_GRP_SPACES: insert a ' ' after every 8 groups
+ * HEXDUMP_RETAIN_BYTE_ORDER: Retain the byte ordering of groups
+ * instead of treating each group as a
+ * native-endian number
*
* hex_dump_to_buffer() works on one "line" of output at a time, converting
* <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
@@ -172,6 +176,7 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
int ret;
int sep_chars = 0;
char sep = 0;
+ bool big_endian = (flags & HEXDUMP_RETAIN_BYTE_ORDER) ? 1 : 0;
if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
@@ -203,10 +208,13 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
const u64 *ptr8 = buf;
for (j = 0; j < ngroups; j++) {
+ u64 val = big_endian ?
+ be64_to_cpu(get_unaligned(ptr8 + j)) :
+ get_unaligned(ptr8 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
"%s%16.16llx",
j ? group_separator(j, flags) : "",
- get_unaligned(ptr8 + j));
+ val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -215,10 +223,14 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
const u32 *ptr4 = buf;
for (j = 0; j < ngroups; j++) {
+ u32 val = big_endian ?
+ be32_to_cpu(get_unaligned(ptr4 + j)) :
+ get_unaligned(ptr4 + j);
+
ret = snprintf(linebuf + lx, linebuflen - lx,
"%s%8.8x",
j ? group_separator(j, flags) : "",
- get_unaligned(ptr4 + j));
+ val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -227,10 +239,14 @@ int hex_dump_to_buffer_ext(const void *buf, size_t len, int rowsize,
const u16 *ptr2 = buf;
for (j = 0; j < ngroups; j++) {
+ u16 val = big_endian ?
+ be16_to_cpu(get_unaligned(ptr2 + j)) :
+ get_unaligned(ptr2 + j);
+
ret = snprintf(linebuf + lx, linebuflen - lx,
"%s%4.4x",
j ? group_separator(j, flags) : "",
- get_unaligned(ptr2 + j));
+ val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -332,7 +348,8 @@ static void announce_skipped(const char *level, const char *prefix_str,
* @prefix_type: controls whether prefix of an offset, address, or none
* is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
* @rowsize: number of bytes to print per line; must be a multiple of groupsize
- * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
+ * @groupsize: number of bytes to convert to a native endian number and print:
+ * 1, 2, 4, 8; default = 1
* @buf: data blob to dump
* @len: number of bytes in the @buf
* @ascii: include ASCII after the hex output
@@ -343,6 +360,9 @@ static void announce_skipped(const char *level, const char *prefix_str,
* HEXDUMP_2_GRP_LINES: insert a '|' after every 2 groups
* HEXDUMP_4_GRP_LINES: insert a '|' after every 4 groups
* HEXDUMP_8_GRP_LINES: insert a '|' after every 8 groups
+ * HEXDUMP_RETAIN_BYTE_ORDER: Retain the byte ordering of groups
+ * instead of treating each group as a
+ * native-endian number
*
* Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump
* to the kernel log at the specified kernel log level, with an optional
diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index ad43218437f1..d2cfcb3e2d2b 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -98,14 +98,15 @@ static unsigned failed_tests __initdata;
static void __init test_hexdump_prepare_test(size_t len, int rowsize,
int groupsize, char *test,
- size_t testlen, bool ascii)
+ size_t testlen, u64 flags)
{
char *p;
const char * const *result;
size_t l = len;
int gs = groupsize, rs = rowsize;
unsigned int i;
- const bool is_be = IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
+ const bool is_be = IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) ||
+ (flags & HEXDUMP_RETAIN_BYTE_ORDER);
if (l > rs)
l = rs;
@@ -142,7 +143,7 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
p--;
/* ASCII part */
- if (ascii) {
+ if (flags & HEXDUMP_ASCII) {
do {
*p++ = ' ';
} while (p < test + rs * 2 + rs / gs + 1);
@@ -157,7 +158,7 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
#define TEST_HEXDUMP_BUF_SIZE (64 * 3 + 2 + 64 + 1)
static void __init test_hexdump(size_t len, int rowsize, int groupsize,
- bool ascii)
+ u64 flags)
{
char test[TEST_HEXDUMP_BUF_SIZE];
char real[TEST_HEXDUMP_BUF_SIZE];
@@ -166,12 +167,11 @@ static void __init test_hexdump(size_t len, int rowsize, int groupsize,
memset(real, FILL_CHAR, sizeof(real));
hex_dump_to_buffer_ext(data_b, len, rowsize, groupsize,
- real, sizeof(real),
- ascii ? HEXDUMP_ASCII : 0);
+ real, sizeof(real), flags);
memset(test, FILL_CHAR, sizeof(test));
test_hexdump_prepare_test(len, rowsize, groupsize, test, sizeof(test),
- ascii);
+ flags);
if (memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)) {
pr_err("Len: %zu row: %d group: %d\n", len, rowsize, groupsize);
@@ -194,7 +194,7 @@ static void __init test_hexdump_set(int rowsize, bool ascii)
static void __init test_hexdump_overflow(size_t buflen, size_t len,
int rowsize, int groupsize,
- bool ascii)
+ u64 flags)
{
char test[TEST_HEXDUMP_BUF_SIZE];
char buf[TEST_HEXDUMP_BUF_SIZE];
@@ -206,8 +206,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
memset(buf, FILL_CHAR, sizeof(buf));
rc = hex_dump_to_buffer_ext(data_b, len, rowsize, groupsize,
- buf, buflen,
- ascii ? HEXDUMP_ASCII : 0);
+ buf, buflen, flags);
/*
* Caller must provide the data length multiple of groupsize. The
@@ -224,12 +223,12 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
- 1 /* no trailing space */;
}
- expected_len = (ascii) ? ascii_len : hex_len;
+ expected_len = (flags & HEXDUMP_ASCII) ? ascii_len : hex_len;
fill_point = min_t(int, expected_len + 1, buflen);
if (buflen) {
test_hexdump_prepare_test(len, rowsize, groupsize, test,
- sizeof(test), ascii);
+ sizeof(test), flags);
test[fill_point - 1] = '\0';
}
memset(test + fill_point, FILL_CHAR, sizeof(test) - fill_point);
@@ -239,8 +238,8 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
buf[sizeof(buf) - 1] = '\0';
if (!match) {
- pr_err("rowsize: %u groupsize: %u ascii: %d Len: %zu buflen: %zu strlen: %zu\n",
- rowsize, groupsize, ascii, len, buflen,
+ pr_err("rowsize: %u groupsize: %u flags: %llx Len: %zu buflen: %zu strlen: %zu\n",
+ rowsize, groupsize, flags, len, buflen,
strnlen(buf, sizeof(buf)));
pr_err("Result: %d '%-.*s'\n", rc, (int)buflen, buf);
pr_err("Expect: %d '%-.*s'\n", expected_len, (int)buflen, test);
@@ -249,7 +248,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
}
}
-static void __init test_hexdump_overflow_set(size_t buflen, bool ascii)
+static void __init test_hexdump_overflow_set(size_t buflen, u64 flags)
{
unsigned int i = 0;
int rs = (get_random_int() % 4 + 1) * 16;
@@ -258,7 +257,7 @@ static void __init test_hexdump_overflow_set(size_t buflen, bool ascii)
int gs = 1 << i;
size_t len = get_random_int() % rs + gs;
- test_hexdump_overflow(buflen, rounddown(len, gs), rs, gs, ascii);
+ test_hexdump_overflow(buflen, rounddown(len, gs), rs, gs, flags);
} while (i++ < 3);
}
@@ -266,20 +265,43 @@ static int __init test_hexdump_init(void)
{
unsigned int i;
int rowsize;
+ u64 flags;
+ flags = 0;
rowsize = (get_random_int() % 4 + 1) * 16;
for (i = 0; i < 16; i++)
- test_hexdump_set(rowsize, false);
+ test_hexdump_set(rowsize, flags);
+ flags = HEXDUMP_ASCII;
rowsize = (get_random_int() % 4 + 1) * 16;
for (i = 0; i < 16; i++)
- test_hexdump_set(rowsize, true);
+ test_hexdump_set(rowsize, flags);
+ flags = HEXDUMP_RETAIN_BYTE_ORDER;
+ rowsize = (get_random_int() % 2 + 1) * 16;
+ for (i = 0; i < 16; i++)
+ test_hexdump_set(rowsize, flags);
+
+ flags = HEXDUMP_ASCII | HEXDUMP_RETAIN_BYTE_ORDER;
+ rowsize = (get_random_int() % 2 + 1) * 16;
+ for (i = 0; i < 16; i++)
+ test_hexdump_set(rowsize, flags);
+
+ flags = 0;
+ for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
+ test_hexdump_overflow_set(i, flags);
+
+ flags = HEXDUMP_ASCII;
+ for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
+ test_hexdump_overflow_set(i, flags);
+
+ flags = HEXDUMP_RETAIN_BYTE_ORDER;
for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
- test_hexdump_overflow_set(i, false);
+ test_hexdump_overflow_set(i, flags);
+ flags = HEXDUMP_ASCII | HEXDUMP_RETAIN_BYTE_ORDER;
for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
- test_hexdump_overflow_set(i, true);
+ test_hexdump_overflow_set(i, flags);
if (failed_tests == 0)
pr_info("all %u tests passed\n", total_tests);
--
2.21.0
^ permalink raw reply related
* [PATCH v4 0/7] Hexdump Enhancements
From: Alastair D'Silva @ 2019-06-25 3:17 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
From: Alastair D'Silva <alastair@d-silva.org>
Apologies for the large CC list, it's a heads up for those responsible
for subsystems where a prototype change in generic code causes a change
in those subsystems.
This series enhances hexdump.
These improve the readability of the dumped data in certain situations
(eg. wide terminals are available, many lines of empty bytes exist, etc).
The default behaviour of hexdump is unchanged, however, the prototype
for hex_dump_to_buffer() has changed, and print_hex_dump() has been
renamed to print_hex_dump_ext(), with a wrapper replacing it for
compatibility with existing code, which would have been too invasive to
change.
Hexdump selftests have be run & confirmed passed.
Changelog:
V4:
- Add missing header (linux/bits.h)
- Fix comment formatting
- Create hex_dump_to_buffer_ext & make hex_dump_to_buffer a wrapper
V3:
- Fix inline documention
- use BIT macros
- use u32 rather than u64 for flags
V2:
- Fix failing selftests
- Fix precedence bug in 'Replace ascii bool in hex_dump_to_buffer...'
- Remove hardcoded new lengths & instead relax the checks in
hex_dump_to_buffer, allocating the buffer from the heap instead of the
stack.
- Replace the skipping of lines of 0x00/0xff with skipping lines of
repeated characters, announcing what has been skipped.
- Add spaces as an optional N-group separator
- Allow byte ordering to be maintained when HEXDUMP_RETAIN_BYTE_ORDERING
is set.
- Updated selftests to cover 'Relax rowsize checks' &
'Optionally retain byte ordering'
Alastair D'Silva (7):
lib/hexdump.c: Fix selftests
lib/hexdump.c: Relax rowsize checks in hex_dump_to_buffer
lib/hexdump.c: Optionally suppress lines of repeated bytes
lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
lib/hexdump.c: Allow multiple groups to be separated by lines '|'
lib/hexdump.c: Allow multiple groups to be separated by spaces
lib/hexdump.c: Optionally retain byte ordering
drivers/gpu/drm/i915/intel_engine_cs.c | 5 +-
drivers/isdn/hardware/mISDN/mISDNisar.c | 10 +-
drivers/mailbox/mailbox-test.c | 8 +-
drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +-
.../net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +-
drivers/net/wireless/ath/ath10k/debug.c | 7 +-
.../net/wireless/intel/iwlegacy/3945-mac.c | 4 +-
drivers/platform/chrome/wilco_ec/debugfs.c | 10 +-
drivers/scsi/scsi_logging.c | 8 +-
drivers/staging/fbtft/fbtft-core.c | 2 +-
fs/seq_file.c | 6 +-
include/linux/printk.h | 75 ++++-
lib/hexdump.c | 267 +++++++++++++++---
lib/test_hexdump.c | 154 +++++++---
14 files changed, 438 insertions(+), 122 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH v4 2/7] lib/hexdump.c: Relax rowsize checks in hex_dump_to_buffer
From: Alastair D'Silva @ 2019-06-25 3:17 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <20190625031726.12173-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
This patch removes the hardcoded row limits and allows for
other lengths. These lengths must still be a multiple of
groupsize.
This allows structs that are not 16/32 bytes to display on
a single line.
This patch also expands the self-tests to test row sizes
up to 64 bytes (though they can now be arbitrarily long).
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
lib/hexdump.c | 49 +++++++++++++++++++++++++++++--------------
lib/test_hexdump.c | 52 ++++++++++++++++++++++++++++++++++++++--------
2 files changed, 76 insertions(+), 25 deletions(-)
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 81b70ed37209..870c8cbff1e1 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -12,6 +12,7 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/export.h>
+#include <linux/slab.h>
#include <asm/unaligned.h>
const char hex_asc[] = "0123456789abcdef";
@@ -80,14 +81,15 @@ EXPORT_SYMBOL(bin2hex);
* hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
* @buf: data blob to dump
* @len: number of bytes in the @buf
- * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @rowsize: number of bytes to print per line; must be a multiple of groupsize
* @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
* @linebuf: where to put the converted data
* @linebuflen: total size of @linebuf, including space for terminating NUL
* @ascii: include ASCII after the hex output
*
- * hex_dump_to_buffer() works on one "line" of output at a time, i.e.,
- * 16 or 32 bytes of input data converted to hex + ASCII output.
+ * hex_dump_to_buffer() works on one "line" of output at a time, converting
+ * <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
+ * until <rowsize> bytes have been emitted.
*
* Given a buffer of u8 data, hex_dump_to_buffer() converts the input data
* to a hex + ASCII dump at the supplied memory location.
@@ -116,16 +118,17 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
int ascii_column;
int ret;
- if (rowsize != 16 && rowsize != 32)
- rowsize = 16;
-
- if (len > rowsize) /* limit to one line at a time */
- len = rowsize;
if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
if ((len % groupsize) != 0) /* no mixed size output */
groupsize = 1;
+ if (rowsize % groupsize)
+ rowsize -= rowsize % groupsize;
+
+ if (len > rowsize) /* limit to one line at a time */
+ len = rowsize;
+
ngroups = len / groupsize;
ascii_column = rowsize * 2 + rowsize / groupsize + 1;
@@ -216,7 +219,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
* caller supplies trailing spaces for alignment if desired
* @prefix_type: controls whether prefix of an offset, address, or none
* is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
- * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @rowsize: number of bytes to print per line; must be a multiple of groupsize
* @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
* @buf: data blob to dump
* @len: number of bytes in the @buf
@@ -226,10 +229,9 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
* to the kernel log at the specified kernel log level, with an optional
* leading prefix.
*
- * print_hex_dump() works on one "line" of output at a time, i.e.,
- * 16 or 32 bytes of input data converted to hex + ASCII output.
* print_hex_dump() iterates over the entire input @buf, breaking it into
- * "line size" chunks to format and print.
+ * lines of rowsize/groupsize groups of input data converted to hex +
+ * (optionally) ASCII output.
*
* E.g.:
* print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
@@ -246,17 +248,30 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
{
const u8 *ptr = buf;
int i, linelen, remaining = len;
- unsigned char linebuf[32 * 3 + 2 + 32 + 1];
+ unsigned char *linebuf;
+ unsigned int linebuf_len;
- if (rowsize != 16 && rowsize != 32)
- rowsize = 16;
+ if (rowsize % groupsize)
+ rowsize -= rowsize % groupsize;
+
+ /*
+ * Worst case line length:
+ * 2 hex chars + space per byte in, 2 spaces, 1 char per byte in, NULL
+ */
+ linebuf_len = rowsize * 3 + 2 + rowsize + 1;
+ linebuf = kzalloc(linebuf_len, GFP_KERNEL);
+ if (!linebuf) {
+ printk("%s%shexdump: Could not alloc %u bytes for buffer\n",
+ level, prefix_str, linebuf_len);
+ return;
+ }
for (i = 0; i < len; i += rowsize) {
linelen = min(remaining, rowsize);
remaining -= rowsize;
hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
- linebuf, sizeof(linebuf), ascii);
+ linebuf, linebuf_len, ascii);
switch (prefix_type) {
case DUMP_PREFIX_ADDRESS:
@@ -271,6 +286,8 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
break;
}
}
+
+ kfree(linebuf);
}
EXPORT_SYMBOL(print_hex_dump);
diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index bef97a964582..5e54525a937c 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -14,15 +14,25 @@ static const unsigned char data_b[] = {
'\x70', '\xba', '\xc4', '\x24', '\x7d', '\x83', '\x34', '\x9b', /* 08 - 0f */
'\xa6', '\x9c', '\x31', '\xad', '\x9c', '\x0f', '\xac', '\xe9', /* 10 - 17 */
'\x4c', '\xd1', '\x19', '\x99', '\x43', '\xb1', '\xaf', '\x0c', /* 18 - 1f */
+ '\x00', '\x01', '\x02', '\x03', '\x04', '\x05', '\x06', '\x07', /* 20 - 27 */
+ '\x0f', '\x0e', '\x0d', '\x0c', '\x0b', '\x0a', '\x09', '\x08', /* 28 - 2f */
+ '\x10', '\x11', '\x12', '\x13', '\x14', '\x15', '\x16', '\x17', /* 30 - 37 */
+ '\x1f', '\x1e', '\x1d', '\x1c', '\x1b', '\x1a', '\x19', '\x18', /* 38 - 3f */
};
-static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C...";
+static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C..."
+ "................................";
static const char * const test_data_1[] __initconst = {
"be", "32", "db", "7b", "0a", "18", "93", "b2",
"70", "ba", "c4", "24", "7d", "83", "34", "9b",
"a6", "9c", "31", "ad", "9c", "0f", "ac", "e9",
"4c", "d1", "19", "99", "43", "b1", "af", "0c",
+ "00", "01", "02", "03", "04", "05", "06", "07",
+ "0f", "0e", "0d", "0c", "0b", "0a", "09", "08",
+ "10", "11", "12", "13", "14", "15", "16", "17",
+ "1f", "1e", "1d", "1c", "1b", "1a", "19", "18",
+ NULL
};
static const char * const test_data_2_le[] __initconst = {
@@ -30,6 +40,11 @@ static const char * const test_data_2_le[] __initconst = {
"ba70", "24c4", "837d", "9b34",
"9ca6", "ad31", "0f9c", "e9ac",
"d14c", "9919", "b143", "0caf",
+ "0100", "0302", "0504", "0706",
+ "0e0f", "0c0d", "0a0b", "0809",
+ "1110", "1312", "1514", "1716",
+ "1e1f", "1c1d", "1a1b", "1819",
+ NULL
};
static const char * const test_data_2_be[] __initconst = {
@@ -37,26 +52,43 @@ static const char * const test_data_2_be[] __initconst = {
"70ba", "c424", "7d83", "349b",
"a69c", "31ad", "9c0f", "ace9",
"4cd1", "1999", "43b1", "af0c",
+ "0001", "0203", "0405", "0607",
+ "0f0e", "0d0c", "0b0a", "0908",
+ "1011", "1213", "1415", "1617",
+ "1f1e", "1d1c", "1b1a", "1918",
+ NULL
};
static const char * const test_data_4_le[] __initconst = {
"7bdb32be", "b293180a", "24c4ba70", "9b34837d",
"ad319ca6", "e9ac0f9c", "9919d14c", "0cafb143",
+ "03020100", "07060504", "0c0d0e0f", "08090a0b",
+ "13121110", "17161514", "1c1d1e1f", "18191a1b",
+ NULL
};
static const char * const test_data_4_be[] __initconst = {
"be32db7b", "0a1893b2", "70bac424", "7d83349b",
"a69c31ad", "9c0face9", "4cd11999", "43b1af0c",
+ "00010203", "04050607", "0f0e0d0c", "0b0a0908",
+ "10111213", "14151617", "1f1e1d1c", "1b1a1918",
+ NULL
};
static const char * const test_data_8_le[] __initconst = {
"b293180a7bdb32be", "9b34837d24c4ba70",
"e9ac0f9cad319ca6", "0cafb1439919d14c",
+ "0706050403020100", "08090a0b0c0d0e0f",
+ "1716151413121110", "18191a1b1c1d1e1f",
+ NULL
};
static const char * const test_data_8_be[] __initconst = {
"be32db7b0a1893b2", "70bac4247d83349b",
"a69c31ad9c0face9", "4cd1199943b1af0c",
+ "0001020304050607", "0f0e0d0c0b0a0908",
+ "1011121314151617", "1f1e1d1c1b1a1918",
+ NULL
};
#define FILL_CHAR '#'
@@ -75,9 +107,6 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
unsigned int i;
const bool is_be = IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
- if (rs != 16 && rs != 32)
- rs = 16;
-
if (l > rs)
l = rs;
@@ -97,7 +126,12 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
p = test;
for (i = 0; i < l / gs; i++) {
const char *q = *result++;
- size_t amount = strlen(q);
+ size_t amount;
+
+ if (!q)
+ break;
+
+ amount = strlen(q);
memcpy(p, q, amount);
p += amount;
@@ -120,7 +154,7 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
*p = '\0';
}
-#define TEST_HEXDUMP_BUF_SIZE (32 * 3 + 2 + 32 + 1)
+#define TEST_HEXDUMP_BUF_SIZE (64 * 3 + 2 + 64 + 1)
static void __init test_hexdump(size_t len, int rowsize, int groupsize,
bool ascii)
@@ -216,7 +250,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
static void __init test_hexdump_overflow_set(size_t buflen, bool ascii)
{
unsigned int i = 0;
- int rs = (get_random_int() % 2 + 1) * 16;
+ int rs = (get_random_int() % 4 + 1) * 16;
do {
int gs = 1 << i;
@@ -231,11 +265,11 @@ static int __init test_hexdump_init(void)
unsigned int i;
int rowsize;
- rowsize = (get_random_int() % 2 + 1) * 16;
+ rowsize = (get_random_int() % 4 + 1) * 16;
for (i = 0; i < 16; i++)
test_hexdump_set(rowsize, false);
- rowsize = (get_random_int() % 2 + 1) * 16;
+ rowsize = (get_random_int() % 4 + 1) * 16;
for (i = 0; i < 16; i++)
test_hexdump_set(rowsize, true);
--
2.21.0
^ permalink raw reply related
* [PATCH v4 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes
From: Alastair D'Silva @ 2019-06-25 3:17 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <20190625031726.12173-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
Some buffers may only be partially filled with useful data, while the rest
is padded (typically with 0x00 or 0xff).
This patch introduces a flag to allow the supression of lines of repeated
bytes, which are replaced with '** Skipped %u bytes of value 0x%x **'
An inline wrapper function is provided for backwards compatibility with
existing code, which maintains the original behaviour.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
include/linux/printk.h | 26 +++++++++---
lib/hexdump.c | 91 ++++++++++++++++++++++++++++++++++++------
2 files changed, 100 insertions(+), 17 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index cefd374c47b1..c0416d0eb9e2 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -7,6 +7,7 @@
#include <linux/kern_levels.h>
#include <linux/linkage.h>
#include <linux/cache.h>
+#include <linux/bits.h>
extern const char linux_banner[];
extern const char linux_proc_banner[];
@@ -481,13 +482,18 @@ enum {
DUMP_PREFIX_ADDRESS,
DUMP_PREFIX_OFFSET
};
+
extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
int groupsize, char *linebuf, size_t linebuflen,
bool ascii);
+
+#define HEXDUMP_ASCII BIT(0)
+#define HEXDUMP_SUPPRESS_REPEATED BIT(1)
+
#ifdef CONFIG_PRINTK
-extern void print_hex_dump(const char *level, const char *prefix_str,
+extern void print_hex_dump_ext(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
- const void *buf, size_t len, bool ascii);
+ const void *buf, size_t len, u32 flags);
#if defined(CONFIG_DYNAMIC_DEBUG)
#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \
dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
@@ -496,18 +502,28 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len);
#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
#else
-static inline void print_hex_dump(const char *level, const char *prefix_str,
+static inline void print_hex_dump_ext(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
- const void *buf, size_t len, bool ascii)
+ const void *buf, size_t len, u32 flags)
{
}
static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len)
{
}
-
#endif
+static __always_inline void print_hex_dump(const char *level,
+ const char *prefix_str,
+ int prefix_type, int rowsize,
+ int groupsize, const void *buf,
+ size_t len, bool ascii)
+{
+ print_hex_dump_ext(level, prefix_str, prefix_type, rowsize, groupsize,
+ buf, len, ascii ? HEXDUMP_ASCII : 0);
+}
+
+
#if defined(CONFIG_DYNAMIC_DEBUG)
#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
groupsize, buf, len, ascii) \
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 870c8cbff1e1..61dc625c32f5 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
EXPORT_SYMBOL(hex_dump_to_buffer);
#ifdef CONFIG_PRINTK
+
+/**
+ * buf_is_all - Check if a buffer contains only a single byte value
+ * @buf: pointer to the buffer
+ * @len: the size of the buffer in bytes
+ * @val: outputs the value if if the bytes are identical
+ */
+static bool buf_is_all(const u8 *buf, size_t len, u8 *val_out)
+{
+ size_t i;
+ u8 val;
+
+ if (len <= 1)
+ return false;
+
+ val = buf[0];
+
+ for (i = 1; i < len; i++) {
+ if (buf[i] != val)
+ return false;
+ }
+
+ *val_out = val;
+ return true;
+}
+
+static void announce_skipped(const char *level, const char *prefix_str,
+ u8 val, size_t count)
+{
+ if (count == 0)
+ return;
+
+ printk("%s%s ** Skipped %lu bytes of value 0x%x **\n",
+ level, prefix_str, count, val);
+}
+
/**
- * print_hex_dump - print a text hex dump to syslog for a binary blob of data
+ * print_hex_dump_ext - dump a binary blob of data to syslog in hexadecimal
* @level: kernel log level (e.g. KERN_DEBUG)
* @prefix_str: string to prefix each line with;
* caller supplies trailing spaces for alignment if desired
@@ -224,6 +260,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
* @buf: data blob to dump
* @len: number of bytes in the @buf
* @ascii: include ASCII after the hex output
+ * @flags: A bitwise OR of the following flags:
+ * HEXDUMP_ASCII: include ASCII after the hex output
+ * HEXDUMP_SUPPRESS_REPEATED: suppress repeated lines of identical
+ * bytes
*
* Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump
* to the kernel log at the specified kernel log level, with an optional
@@ -234,22 +274,25 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
* (optionally) ASCII output.
*
* E.g.:
- * print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
- * 16, 1, frame->data, frame->len, true);
+ * print_hex_dump_ext(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
+ * 16, 1, frame->data, frame->len, HEXDUMP_ASCII);
*
* Example output using %DUMP_PREFIX_OFFSET and 1-byte mode:
* 0009ab42: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f @ABCDEFGHIJKLMNO
* Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode:
* ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~.
*/
-void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
- int rowsize, int groupsize,
- const void *buf, size_t len, bool ascii)
+void print_hex_dump_ext(const char *level, const char *prefix_str,
+ int prefix_type, int rowsize, int groupsize,
+ const void *buf, size_t len, u32 flags)
{
const u8 *ptr = buf;
- int i, linelen, remaining = len;
+ int i, remaining = len;
unsigned char *linebuf;
unsigned int linebuf_len;
+ u8 skipped_val = 0;
+ size_t skipped = 0;
+
if (rowsize % groupsize)
rowsize -= rowsize % groupsize;
@@ -267,11 +310,35 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
}
for (i = 0; i < len; i += rowsize) {
- linelen = min(remaining, rowsize);
+ int linelen = min(remaining, rowsize);
remaining -= rowsize;
+ if (flags & HEXDUMP_SUPPRESS_REPEATED) {
+ u8 new_val;
+
+ if (buf_is_all(ptr + i, linelen, &new_val)) {
+ if (new_val == skipped_val) {
+ skipped += linelen;
+ continue;
+ } else {
+ announce_skipped(level, prefix_str,
+ skipped_val, skipped);
+ skipped_val = new_val;
+ skipped = linelen;
+ continue;
+ }
+ }
+ }
+
+ if (skipped) {
+ announce_skipped(level, prefix_str, skipped_val,
+ skipped);
+ skipped = 0;
+ }
+
hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
- linebuf, linebuf_len, ascii);
+ linebuf, linebuf_len,
+ flags & HEXDUMP_ASCII);
switch (prefix_type) {
case DUMP_PREFIX_ADDRESS:
@@ -289,7 +356,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
kfree(linebuf);
}
-EXPORT_SYMBOL(print_hex_dump);
+EXPORT_SYMBOL(print_hex_dump_ext);
#if !defined(CONFIG_DYNAMIC_DEBUG)
/**
@@ -307,8 +374,8 @@ EXPORT_SYMBOL(print_hex_dump);
void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len)
{
- print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
- buf, len, true);
+ print_hex_dump_ext(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
+ buf, len, HEXDUMP_ASCII);
}
EXPORT_SYMBOL(print_hex_dump_bytes);
#endif /* !defined(CONFIG_DYNAMIC_DEBUG) */
--
2.21.0
^ permalink raw reply related
* [PATCH v4 1/7] lib/hexdump.c: Fix selftests
From: Alastair D'Silva @ 2019-06-25 3:17 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <20190625031726.12173-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
The overflow tests did not account for the situation where no
overflow occurs and len < rowsize.
This patch renames the cryptic variables and accounts for the
above case.
The selftests now pass.
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
lib/test_hexdump.c | 48 +++++++++++++++++++++++++++-------------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index 5144899d3c6b..bef97a964582 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -163,45 +163,53 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
{
char test[TEST_HEXDUMP_BUF_SIZE];
char buf[TEST_HEXDUMP_BUF_SIZE];
- int rs = rowsize, gs = groupsize;
- int ae, he, e, f, r;
- bool a;
+ int ascii_len, hex_len, expected_len, fill_point, ngroups, rc;
+ bool match;
total_tests++;
memset(buf, FILL_CHAR, sizeof(buf));
- r = hex_dump_to_buffer(data_b, len, rs, gs, buf, buflen, ascii);
+ rc = hex_dump_to_buffer(data_b, len, rowsize, groupsize, buf, buflen,
+ ascii);
/*
* Caller must provide the data length multiple of groupsize. The
* calculations below are made with that assumption in mind.
*/
- ae = rs * 2 /* hex */ + rs / gs /* spaces */ + 1 /* space */ + len /* ascii */;
- he = (gs * 2 /* hex */ + 1 /* space */) * len / gs - 1 /* no trailing space */;
+ ngroups = rowsize / groupsize;
+ hex_len = (groupsize * 2 /* hex */ + 1 /* spaces */) * ngroups
+ - 1 /* no trailing space */;
+ ascii_len = hex_len + 2 /* space */ + len /* ascii */;
+
+ if (len < rowsize) {
+ ngroups = len / groupsize;
+ hex_len = (groupsize * 2 /* hex */ + 1 /* spaces */) * ngroups
+ - 1 /* no trailing space */;
+ }
- if (ascii)
- e = ae;
- else
- e = he;
+ expected_len = (ascii) ? ascii_len : hex_len;
- f = min_t(int, e + 1, buflen);
+ fill_point = min_t(int, expected_len + 1, buflen);
if (buflen) {
- test_hexdump_prepare_test(len, rs, gs, test, sizeof(test), ascii);
- test[f - 1] = '\0';
+ test_hexdump_prepare_test(len, rowsize, groupsize, test,
+ sizeof(test), ascii);
+ test[fill_point - 1] = '\0';
}
- memset(test + f, FILL_CHAR, sizeof(test) - f);
+ memset(test + fill_point, FILL_CHAR, sizeof(test) - fill_point);
- a = r == e && !memcmp(test, buf, TEST_HEXDUMP_BUF_SIZE);
+ match = rc == expected_len && !memcmp(test, buf, TEST_HEXDUMP_BUF_SIZE);
buf[sizeof(buf) - 1] = '\0';
- if (!a) {
- pr_err("Len: %zu buflen: %zu strlen: %zu\n",
- len, buflen, strnlen(buf, sizeof(buf)));
- pr_err("Result: %d '%s'\n", r, buf);
- pr_err("Expect: %d '%s'\n", e, test);
+ if (!match) {
+ pr_err("rowsize: %u groupsize: %u ascii: %d Len: %zu buflen: %zu strlen: %zu\n",
+ rowsize, groupsize, ascii, len, buflen,
+ strnlen(buf, sizeof(buf)));
+ pr_err("Result: %d '%-.*s'\n", rc, (int)buflen, buf);
+ pr_err("Expect: %d '%-.*s'\n", expected_len, (int)buflen, test);
failed_tests++;
+
}
}
--
2.21.0
^ permalink raw reply related
* Re: Removing skb_orphan() from ip_rcv_core()
From: Joe Stringer @ 2019-06-25 3:26 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Joe Stringer, Eric Dumazet, Florian Westphal, netdev,
john fastabend, Daniel Borkmann, Lorenz Bauer, Jakub Sitnicki,
Paolo Abeni
In-Reply-To: <ab745372-35eb-8bb8-30a4-0e861af27ac2@mojatatu.com>
On Mon, Jun 24, 2019 at 7:47 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On 2019-06-21 1:58 p.m., Joe Stringer wrote:
> > Hi folks, picking this up again..
> [..]
> > During LSFMM, it seemed like no-one knew quite why the skb_orphan() is
> > necessary in that path in the current version of the code, and that we
> > may be able to remove it. Florian, I know you weren't in the room for
> > that discussion, so raising it again now with a stack trace, Do you
> > have some sense what's going on here and whether there's a path
> > towards removing it from this path or allowing the skb->sk to be
> > retained during ip_rcv() in some conditions?
>
>
> Sorry - I havent followed the discussion but saw your email over
> the weekend and wanted to be at work to refresh my memory on some
> code. For maybe 2-3 years we have deployed the tproxy
> equivalent as a tc action on ingress (with no netfilter dependency).
>
> And, of course, we had to work around that specific code you are
> referring to - we didnt remove it. The tc action code increments
> the sk refcount and sets the tc index. The net core doesnt orphan
> the skb if a speacial tc index value is set (see attached patch)
>
> I never bothered up streaming the patch because the hack is a bit
> embarrassing (but worked ;->); and never posted the action code
> either because i thought this was just us that had this requirement.
> I am glad other people see the need for this feature. Is there effort
> to make this _not_ depend on iptables/netfilter? I am guessing if you
> want to do this from ebpf (tc or xdp) that is a requirement.
> Our need was with tcp at the time; so left udp dependency on netfilter
> alone.
I haven't got as far as UDP yet, but I didn't see any need for a
dependency on netfilter.
^ permalink raw reply
* Re: [PATCH 1/2] tty: ldisc: Fix misuse of proc_dointvec "ldisc_autoload"
From: Greg KH @ 2019-06-25 3:32 UTC (permalink / raw)
To: Eiichi Tsukata; +Cc: jslaby, davem, kuznet, yoshfuji, linux-kernel, netdev
In-Reply-To: <20190625030801.24538-1-devel@etsukata.com>
On Tue, Jun 25, 2019 at 12:08:00PM +0900, Eiichi Tsukata wrote:
> /proc/sys/dev/tty/ldisc_autoload assumes given value to be 0 or 1. Use
> proc_dointvec_minmax instead of proc_dointvec.
>
> Fixes: 7c0cca7c847e "(tty: ldisc: add sysctl to prevent autoloading of ldiscs)"
> Signed-off-by: Eiichi Tsukata <devel@etsukata.com>
> ---
> drivers/tty/tty_ldisc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
> index e38f104db174..a8ea7a35c94e 100644
> --- a/drivers/tty/tty_ldisc.c
> +++ b/drivers/tty/tty_ldisc.c
> @@ -863,7 +863,7 @@ static struct ctl_table tty_table[] = {
> .data = &tty_ldisc_autoload,
> .maxlen = sizeof(tty_ldisc_autoload),
> .mode = 0644,
> - .proc_handler = proc_dointvec,
> + .proc_handler = proc_dointvec_minmax,
> .extra1 = &zero,
> .extra2 = &one,
Ah, nice catch. But this really isn't an issue as if you use a bigger
value, things will not "break", right?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH next 0/3] blackhole device to invalidate dst
From: Michael Chan @ 2019-06-25 4:00 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Netdev, Eric Dumazet, David Miller, Daniel Axtens,
Mahesh Bandewar
In-Reply-To: <20190622004519.89335-1-maheshb@google.com>
On Fri, Jun 21, 2019 at 5:45 PM Mahesh Bandewar <maheshb@google.com> wrote:
> Well, I'm not a TCP expert and though we have experienced
> these corner cases in our environment, I could not reproduce
> this case reliably in my test setup to try this fix myself.
> However, Michael Chan <michael.chan@broadcom.com> had a setup
> where these fixes helped him mitigate the issue and not cause
> the crash.
>
I will ask the lab to test these patches tomorrow. Thanks.
^ permalink raw reply
* RE: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled indefinitely
From: Sudarsana Reddy Kalluru @ 2019-06-25 4:01 UTC (permalink / raw)
To: Guilherme G. Piccoli, GR-everest-linux-l2, netdev@vger.kernel.org
Cc: Ariel Elior, jay.vosburgh@canonical.com
In-Reply-To: <20190624222356.17037-1-gpiccoli@canonical.com>
> -----Original Message-----
> From: Guilherme G. Piccoli <gpiccoli@canonical.com>
> Sent: Tuesday, June 25, 2019 3:54 AM
> To: GR-everest-linux-l2 <GR-everest-linux-l2@marvell.com>;
> netdev@vger.kernel.org; Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: Ariel Elior <aelior@marvell.com>; gpiccoli@canonical.com;
> jay.vosburgh@canonical.com
> Subject: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled
> indefinitely
>
> External Email
>
> ----------------------------------------------------------------------
> Currently bnx2x ptp worker tries to read a register with timestamp
> information in case of TX packet timestamping and in case it fails, the routine
> reschedules itself indefinitely. This was reported as a kworker always at 100%
> of CPU usage, which was narrowed down to be bnx2x ptp_task.
>
> By following the ioctl handler, we could narrow down the problem to an NTP
> tool (chrony) requesting HW timestamping from bnx2x NIC with RX filter
> zeroed; this isn't reproducible for example with ptp4l (from linuxptp) since
> this tool requests a supported RX filter.
> It seems NIC FW timestamp mechanism cannot work well with
> RX_FILTER_NONE - driver's PTP filter init routine skips a register write to the
> adapter if there's not a supported filter request.
>
> This patch addresses the problem of bnx2x ptp thread's everlasting
> reschedule by retrying the register read 10 times; between the read
> attempts the thread sleeps for an increasing amount of time starting in 50ms
> to give FW some time to perform the timestamping. If it still fails after all
> retries, we bail out in order to prevent an unbound resource consumption
> from bnx2x.
Thanks for your changes and time on this. In general time-latching happens in couple or more milliseconds (even in some 100s of usec) under the normal traffic conditions. With this approach, there's a possibility that every packet has to wait for atleast 50ms for the timestamping. This in turn affects the wait-queue (of packets to be timestamped) at hardware as next TS recording happens only after the register is freed/read. And also, it incurs some latency for the ptp applications.
PTP thread is consuming time may be due to the debug messages in this error path, which you are planning address already (thanks!!).
"Also, I've dropped the PTP "outstanding, etc" messages to debug-level, they're quite flooding my log.
Do you see cpu hog even after removing this message? In such case we may need to think of other alternatives such as sleep for 1 ms.
Just for the info, the approach continuous-poll-for-timestamp() is used ixgbe driver (ixgbe_ptp_tx_hwtstamp_work()) as well.
>
> The patch also adds an ethtool statistic for accounting the skipped TX
> timestamp packets and it reduces the priority of timestamping error
> messages to prevent log flooding. The code was tested using both linuxptp
> and chrony.
>
> Reported-and-tested-by: Przemyslaw Hausman
> <przemyslaw.hausman@canonical.com>
> Suggested-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
> ---
>
> Sudarsana, thanks for the suggestion! I've tried to follow an identical
> approach from [0], but still the ptp thread was consuming a lot of CPU due to
> the good amount of reschedules.
>
> I decided then to use the loop approach with small increasing delays, in order
> to respect the time FW takes eventually to complete timestamping.
>
> Also, I've dropped the PTP "outstanding, etc" messages to debug-level,
> they're quite flooding my log. Cheers!
>
> [0] git.kernel.org/pub/scm/linux/kernel/git/davem/net-
> next.git/commit/?id=9adebac37e7d
>
> .../net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 12 +++++--
> .../ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 4 ++-
> .../net/ethernet/broadcom/bnx2x/bnx2x_main.c | 36 ++++++++++++++----
> - .../net/ethernet/broadcom/bnx2x/bnx2x_stats.h | 3 ++
> 4 files changed, 43 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index 008ad0ca89ba..6751cd04e8d8 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -3857,9 +3857,17 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff
> *skb, struct net_device *dev)
>
> if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
> if (!(bp->flags & TX_TIMESTAMPING_EN)) {
> - BNX2X_ERR("Tx timestamping was not enabled, this
> packet will not be timestamped\n");
> + bp->eth_stats.ptp_skip_tx_ts++;
> + netdev_err_once(bp->dev,
> + "Tx timestamping isn't enabled, this
> packet won't be timestamped\n");
> + DP(BNX2X_MSG_PTP,
> + "Tx timestamping isn't enabled, this packet won't
> be
> +timestamped\n");
> } else if (bp->ptp_tx_skb) {
> - BNX2X_ERR("The device supports only a single
> outstanding packet to timestamp, this packet will not be timestamped\n");
> + bp->eth_stats.ptp_skip_tx_ts++;
> + netdev_err_once(bp->dev,
> + "Device supports only a single
> outstanding packet to timestamp, this packet won't be timestamped\n");
> + DP(BNX2X_MSG_PTP,
> + "Device supports only a single outstanding packet to
> timestamp,
> +this packet won't be timestamped\n");
> } else {
> skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> /* schedule check for Tx timestamp */ diff --git
> a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> index 51fc845de31a..4a0ba6801c9e 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> @@ -182,7 +182,9 @@ static const struct {
> { STATS_OFFSET32(driver_filtered_tx_pkt),
> 4, false, "driver_filtered_tx_pkt" },
> { STATS_OFFSET32(eee_tx_lpi),
> - 4, true, "Tx LPI entry count"}
> + 4, true, "Tx LPI entry count"},
> + { STATS_OFFSET32(ptp_skip_tx_ts),
> + 4, false, "ptp_skipped_tx_tstamp" },
> };
>
> #define BNX2X_NUM_STATS ARRAY_SIZE(bnx2x_stats_arr)
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index 03ac10b1cd1e..066b24611890 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -15214,11 +15214,27 @@ static void bnx2x_ptp_task(struct work_struct
> *work)
> u32 val_seq;
> u64 timestamp, ns;
> struct skb_shared_hwtstamps shhwtstamps;
> + bool bail = true;
> + int i;
>
> - /* Read Tx timestamp registers */
> - val_seq = REG_RD(bp, port ? NIG_REG_P1_TLLH_PTP_BUF_SEQID :
> - NIG_REG_P0_TLLH_PTP_BUF_SEQID);
> - if (val_seq & 0x10000) {
> + /* FW may take a while to complete timestamping; try a bit and if it's
> + * still not complete, may indicate an error state - bail out then.
> + */
> + for (i = 0; i <= 10; i++) {
> + /* Read Tx timestamp registers */
> + val_seq = REG_RD(bp, port ?
> NIG_REG_P1_TLLH_PTP_BUF_SEQID :
> + NIG_REG_P0_TLLH_PTP_BUF_SEQID);
> + if (val_seq & 0x10000) {
> + bail = false;
> + break;
> + }
> +
> + if (!(i % 5)) /* Avoid log flood */
> + DP(BNX2X_MSG_PTP, "There's no valid Tx timestamp
> yet\n");
> + msleep(50 + 25 * i);
> + }
> +
> + if (!bail) {
> /* There is a valid timestamp value */
> timestamp = REG_RD(bp, port ?
> NIG_REG_P1_TLLH_PTP_BUF_TS_MSB :
> NIG_REG_P0_TLLH_PTP_BUF_TS_MSB);
> @@ -15233,16 +15249,18 @@ static void bnx2x_ptp_task(struct work_struct
> *work)
> memset(&shhwtstamps, 0, sizeof(shhwtstamps));
> shhwtstamps.hwtstamp = ns_to_ktime(ns);
> skb_tstamp_tx(bp->ptp_tx_skb, &shhwtstamps);
> - dev_kfree_skb_any(bp->ptp_tx_skb);
> - bp->ptp_tx_skb = NULL;
>
> DP(BNX2X_MSG_PTP, "Tx timestamp, timestamp cycles =
> %llu, ns = %llu\n",
> timestamp, ns);
> } else {
> - DP(BNX2X_MSG_PTP, "There is no valid Tx timestamp
> yet\n");
> - /* Reschedule to keep checking for a valid timestamp value
> */
> - schedule_work(&bp->ptp_task);
> + DP(BNX2X_MSG_PTP,
> + "Tx timestamp is not recorded (register read=%u)\n",
> + val_seq);
> + bp->eth_stats.ptp_skip_tx_ts++;
> }
> +
> + dev_kfree_skb_any(bp->ptp_tx_skb);
> + bp->ptp_tx_skb = NULL;
> }
>
> void bnx2x_set_rx_ts(struct bnx2x *bp, struct sk_buff *skb) diff --git
> a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> index b2644ed13d06..d55e63692cf3 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> @@ -207,6 +207,9 @@ struct bnx2x_eth_stats {
> u32 driver_filtered_tx_pkt;
> /* src: Clear-on-Read register; Will not survive PMF Migration */
> u32 eee_tx_lpi;
> +
> + /* PTP */
> + u32 ptp_skip_tx_ts;
> };
>
> struct bnx2x_eth_q_stats {
> --
> 2.22.0
^ permalink raw reply
* [PATCH V3 00/15] cleanup cppcheck signed shifting errors
From: Phong Tran @ 2019-06-25 4:03 UTC (permalink / raw)
To: tranmanphong
Cc: acme, alexander.shishkin, alexander.sverdlin, allison, andrew,
ast, bgolaszewski, bpf, daniel, daniel, dmg, festevam, gerg,
gregkh, gregory.clement, haojian.zhuang, hsweeten,
illusionist.neo, info, jason, jolsa, kafai, kernel, kgene, krzk,
kstewart, linux-arm-kernel, linux-imx, linux-kernel, linux-omap,
linux-samsung-soc, linux, liviu.dudau, lkundrak,
lorenzo.pieralisi, mark.rutland, mingo, namhyung, netdev, nsekhar,
peterz, robert.jarzmik, s.hauer, sebastian.hesselbarth, shawnguo,
songliubraving, sudeep.holla, swinslow, tglx, tony, will, yhs
In-Reply-To: <20190624135105.15579-1-tranmanphong@gmail.com>
This is also do as the suggestion of "Linux Kernel Mentorship Task List"
https://wiki.linuxfoundation.org/lkmp/lkmp_task_list#cleanup_cppcheck_errors
"Shifting signed 32-bit value by 31 bits is undefined behaviour errors"
Change Log:
V2: Using BIT() macro instead of (1UL << nr)
V3:
* Update the comments from Russell King.
* Update commit message and cover letter for clearly the reason as request
Peter Zijlstra
* For avoiding the broken only change (1<<nr) pattern to BIT(nr)
Phong Tran (15):
arm: perf: cleanup cppcheck shifting error
ARM: davinci: cleanup cppcheck shifting errors
ARM: ep93xx: cleanup cppcheck shifting errors
ARM: exynos: cleanup cppcheck shifting error
ARM: footbridge: cleanup cppcheck shifting error
ARM: imx: cleanup cppcheck shifting errors
ARM: ks8695: cleanup cppcheck shifting error
ARM: mmp: cleanup cppcheck shifting errors
ARM: omap2: cleanup cppcheck shifting error
ARM: orion5x: cleanup cppcheck shifting errors
ARM: pxa: cleanup cppcheck shifting errors
ARM: vexpress: cleanup cppcheck shifting error
ARM: mm: cleanup cppcheck shifting errors
ARM: bpf: cleanup cppcheck shifting error
ARM: vfp: cleanup cppcheck shifting errors
arch/arm/kernel/perf_event_v7.c | 6 +-
arch/arm/mach-davinci/ddr2.h | 6 +-
arch/arm/mach-ep93xx/soc.h | 134 ++++++++++++++++++-------------------
arch/arm/mach-exynos/suspend.c | 2 +-
arch/arm/mach-footbridge/dc21285.c | 2 +-
arch/arm/mach-imx/iomux-mx3.h | 64 +++++++++---------
arch/arm/mach-ks8695/regs-pci.h | 4 +-
arch/arm/mach-mmp/pm-mmp2.h | 40 +++++------
arch/arm/mach-mmp/pm-pxa910.h | 74 ++++++++++----------
arch/arm/mach-omap2/powerdomain.c | 2 +-
arch/arm/mach-orion5x/pci.c | 8 +--
arch/arm/mach-pxa/irq.c | 4 +-
arch/arm/mach-vexpress/spc.c | 4 +-
arch/arm/mm/fault.h | 6 +-
arch/arm/net/bpf_jit_32.c | 2 +-
arch/arm/vfp/vfpinstr.h | 8 +--
16 files changed, 183 insertions(+), 183 deletions(-)
--
2.11.0
^ 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