* Re: [PATCH ghak90 V6 02/10] audit: add container id
From: Richard Guy Briggs @ 2019-07-08 17:51 UTC (permalink / raw)
To: Paul Moore
Cc: Tycho Andersen, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, netfilter-devel, sgrubb, omosnace,
dhowells, simo, Eric Paris, Serge Hallyn, ebiederm, nhorman
In-Reply-To: <CAHC9VhR4fudQanvZGYWMvCf7k2CU3q7e7n1Pi7hzC3v_zpVEdw@mail.gmail.com>
On 2019-05-29 11:29, Paul Moore wrote:
> On Wed, May 29, 2019 at 10:57 AM Tycho Andersen <tycho@tycho.ws> wrote:
> >
> > On Mon, Apr 08, 2019 at 11:39:09PM -0400, Richard Guy Briggs wrote:
> > > It is not permitted to unset the audit container identifier.
> > > A child inherits its parent's audit container identifier.
> >
> > ...
> >
> > > /**
> > > + * audit_set_contid - set current task's audit contid
> > > + * @contid: contid value
> > > + *
> > > + * Returns 0 on success, -EPERM on permission failure.
> > > + *
> > > + * Called (set) from fs/proc/base.c::proc_contid_write().
> > > + */
> > > +int audit_set_contid(struct task_struct *task, u64 contid)
> > > +{
> > > + u64 oldcontid;
> > > + int rc = 0;
> > > + struct audit_buffer *ab;
> > > + uid_t uid;
> > > + struct tty_struct *tty;
> > > + char comm[sizeof(current->comm)];
> > > +
> > > + task_lock(task);
> > > + /* Can't set if audit disabled */
> > > + if (!task->audit) {
> > > + task_unlock(task);
> > > + return -ENOPROTOOPT;
> > > + }
> > > + oldcontid = audit_get_contid(task);
> > > + read_lock(&tasklist_lock);
> > > + /* Don't allow the audit containerid to be unset */
> > > + if (!audit_contid_valid(contid))
> > > + rc = -EINVAL;
> > > + /* if we don't have caps, reject */
> > > + else if (!capable(CAP_AUDIT_CONTROL))
> > > + rc = -EPERM;
> > > + /* if task has children or is not single-threaded, deny */
> > > + else if (!list_empty(&task->children))
> > > + rc = -EBUSY;
> > > + else if (!(thread_group_leader(task) && thread_group_empty(task)))
> > > + rc = -EALREADY;
> > > + read_unlock(&tasklist_lock);
> > > + if (!rc)
> > > + task->audit->contid = contid;
> > > + task_unlock(task);
> > > +
> > > + if (!audit_enabled)
> > > + return rc;
> >
> > ...but it is allowed to change it (assuming
> > capable(CAP_AUDIT_CONTROL), of course)? Seems like this might be more
> > immediately useful since we still live in the world of majority
> > privileged containers if we didn't allow changing it, in addition to
> > un-setting it.
>
> The idea is that only container orchestrators should be able to
> set/modify the audit container ID, and since setting the audit
> container ID can have a significant effect on the records captured
> (and their routing to multiple daemons when we get there) modifying
> the audit container ID is akin to modifying the audit configuration
> which is why it is gated by CAP_AUDIT_CONTROL. The current thinking
> is that you would only change the audit container ID from one
> set/inherited value to another if you were nesting containers, in
> which case the nested container orchestrator would need to be granted
> CAP_AUDIT_CONTROL (which everyone to date seems to agree is a workable
> compromise). We did consider allowing for a chain of nested audit
> container IDs, but the implications of doing so are significant
> (implementation mess, runtime cost, etc.) so we are leaving that out
> of this effort.
We had previously discussed the idea of restricting
orchestrators/engines from only being able to set the audit container
identifier on their own descendants, but it was discarded. I've added a
check to ensure this is now enforced.
I've also added a check to ensure that a process can't set its own audit
container identifier and that if the identifier is already set, then the
orchestrator/engine must be in a descendant user namespace from the
orchestrator that set the previously inherited audit container
identifier.
> From a practical perspective, un-setting the audit container ID is
> pretty much the same as changing it from one set value to another so
> most of the above applies to that case as well.
>
> --
> paul moore
> www.paul-moore.com
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH net-next,v3 07/11] net: sched: use flow block API
From: Jiri Pirko @ 2019-07-08 17:42 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ogerlitz,
Manish.Chopra, marcelo.leitner, mkubecek, venkatkumar.duvvuru,
maxime.chevallier, cphealy, netfilter-devel
In-Reply-To: <20190708160614.2226-8-pablo@netfilter.org>
Mon, Jul 08, 2019 at 06:06:09PM CEST, pablo@netfilter.org wrote:
>This patch adds tcf_block_setup() which uses the flow block API.
>
>This infrastructure takes the flow block callbacks coming from the
>driver and register/unregister to/from the cls_api core.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
[...]
>+static int tcf_block_bind(struct tcf_block *block,
>+ struct flow_block_offload *bo)
>+{
>+ struct flow_block_cb *block_cb, *next;
>+ int err, i = 0;
>+
>+ list_for_each_entry(block_cb, &bo->cb_list, driver_list) {
>+ err = tcf_block_playback_offloads(block, block_cb->cb,
>+ block_cb->cb_priv, true,
>+ tcf_block_offload_in_use(block),
>+ bo->extack);
>+ if (err)
>+ goto err_unroll;
>+
>+ list_add(&block_cb->list, &block->cb_list);
>+ i++;
>+ }
>+ list_splice(&bo->cb_list, bo->driver_block_list);
This cl/driver_block list magic is really very hard to follow. Could you
please make it more clear?
>+
>+ return 0;
>+
>+err_unroll:
>+ list_for_each_entry_safe(block_cb, next, &bo->cb_list, driver_list) {
>+ if (i-- > 0) {
>+ list_del(&block_cb->list);
>+ tcf_block_playback_offloads(block, block_cb->cb,
>+ block_cb->cb_priv, false,
>+ tcf_block_offload_in_use(block),
>+ NULL);
>+ }
>+ flow_block_cb_free(block_cb);
>+ }
>+
>+ return err;
>+}
>+
>+static void tcf_block_unbind(struct tcf_block *block,
>+ struct flow_block_offload *bo)
>+{
>+ struct flow_block_cb *block_cb, *next;
>+
>+ list_for_each_entry_safe(block_cb, next, &bo->cb_list, driver_list) {
>+ list_del(&block_cb->driver_list);
>+ tcf_block_playback_offloads(block, block_cb->cb,
>+ block_cb->cb_priv, false,
>+ tcf_block_offload_in_use(block),
>+ NULL);
>+ list_del(&block_cb->list);
>+ flow_block_cb_free(block_cb);
>+ }
>+}
[...]
^ permalink raw reply
* Re: [PATCH v3] perf cs-etm: Improve completeness for kernel address space
From: Mathieu Poirier @ 2019-07-08 17:33 UTC (permalink / raw)
To: Leo Yan
Cc: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Suzuki K Poulose, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
Linux Kernel Mailing List, linux-arm-kernel, netdev, bpf,
Peter Zijlstra, Coresight ML
In-Reply-To: <20190620034446.25561-1-leo.yan@linaro.org>
On Wed, 19 Jun 2019 at 21:45, Leo Yan <leo.yan@linaro.org> wrote:
>
> Arm and arm64 architecture reserve some memory regions prior to the
> symbol '_stext' and these memory regions later will be used by device
> module and BPF jit. The current code misses to consider these memory
> regions thus any address in the regions will be taken as user space
> mode, but perf cannot find the corresponding dso with the wrong CPU
> mode so we misses to generate samples for device module and BPF
> related trace data.
>
> This patch parse the link scripts to get the memory size prior to start
> address and reduce this size from 'etmq->etm->kernel_start', then can
> get a fixed up kernel start address which contain memory regions for
> device module and BPF. Finally, cs_etm__cpu_mode() can return right
> mode for these memory regions and perf can successfully generate
> samples.
>
> The reason for parsing the link scripts is Arm architecture changes text
> offset dependent on different platforms, which define multiple text
> offsets in $kernel/arch/arm/Makefile. This offset is decided when build
> kernel and the final value is extended in the link script, so we can
> extract the used value from the link script. We use the same way to
> parse arm64 link script as well. If fail to find the link script, the
> pre start memory size is assumed as zero, in this case it has no any
> change caused with this patch.
>
> Below is detailed info for testing this patch:
>
> - Build LLVM/Clang 8.0 or later version;
>
> - Configure perf with ~/.perfconfig:
>
> root@debian:~# cat ~/.perfconfig
> # this file is auto-generated.
> [llvm]
> clang-path = /mnt/build/llvm-build/build/install/bin/clang
> kbuild-dir = /mnt/linux-kernel/linux-cs-dev/
> clang-opt = "-g"
> dump-obj = true
>
> [trace]
> show_zeros = yes
> show_duration = no
> no_inherit = yes
> show_timestamp = no
> show_arg_names = no
> args_alignment = 40
> show_prefix = yes
>
> - Run 'perf trace' command with eBPF event:
>
> root@debian:~# perf trace -e string \
> -e $kernel/tools/perf/examples/bpf/augmented_raw_syscalls.c
>
> - Read eBPF program memory mapping in kernel:
>
> root@debian:~# echo 1 > /proc/sys/net/core/bpf_jit_kallsyms
> root@debian:~# cat /proc/kallsyms | grep -E "bpf_prog_.+_sys_[enter|exit]"
> ffff000000086a84 t bpf_prog_f173133dc38ccf87_sys_enter [bpf]
> ffff000000088618 t bpf_prog_c1bd85c092d6e4aa_sys_exit [bpf]
>
> - Launch any program which accesses file system frequently so can hit
> the system calls trace flow with eBPF event;
>
> - Capture CoreSight trace data with filtering eBPF program:
>
> root@debian:~# perf record -e cs_etm/@20070000.etr/ \
> --filter 'filter 0xffff000000086a84/0x800' -a sleep 5s
>
> - Annotate for symbol 'bpf_prog_f173133dc38ccf87_sys_enter':
>
> root@debian:~# perf report
> Then select 'branches' samples and press 'a' to annotate symbol
> 'bpf_prog_f173133dc38ccf87_sys_enter', press 'P' to print to the
> bpf_prog_f173133dc38ccf87_sys_enter.annotation file:
>
> root@debian:~# cat bpf_prog_f173133dc38ccf87_sys_enter.annotation
>
> bpf_prog_f173133dc38ccf87_sys_enter() bpf_prog_f173133dc38ccf87_sys_enter
> Event: branches
>
> Percent int sys_enter(struct syscall_enter_args *args)
> stp x29, x30, [sp, #-16]!
>
> int key = 0;
> mov x29, sp
>
> augmented_args = bpf_map_lookup_elem(&augmented_filename_map, &key);
> stp x19, x20, [sp, #-16]!
>
> augmented_args = bpf_map_lookup_elem(&augmented_filename_map, &key);
> stp x21, x22, [sp, #-16]!
>
> stp x25, x26, [sp, #-16]!
>
> return bpf_get_current_pid_tgid();
> mov x25, sp
>
> return bpf_get_current_pid_tgid();
> mov x26, #0x0 // #0
>
> sub sp, sp, #0x10
>
> return bpf_map_lookup_elem(pids, &pid) != NULL;
> add x19, x0, #0x0
>
> mov x0, #0x0 // #0
>
> mov x10, #0xfffffffffffffff8 // #-8
>
> if (pid_filter__has(&pids_filtered, getpid()))
> str w0, [x25, x10]
>
> probe_read(&augmented_args->args, sizeof(augmented_args->args), args);
> add x1, x25, #0x0
>
> probe_read(&augmented_args->args, sizeof(augmented_args->args), args);
> mov x10, #0xfffffffffffffff8 // #-8
>
> syscall = bpf_map_lookup_elem(&syscalls, &augmented_args->args.syscall_nr);
> add x1, x1, x10
>
> syscall = bpf_map_lookup_elem(&syscalls, &augmented_args->args.syscall_nr);
> mov x0, #0xffff8009ffffffff // #-140694538682369
>
> movk x0, #0x6698, lsl #16
>
> movk x0, #0x3e00
>
> mov x10, #0xffffffffffff1040 // #-61376
>
> if (syscall == NULL || !syscall->enabled)
> movk x10, #0x1023, lsl #16
>
> if (syscall == NULL || !syscall->enabled)
> movk x10, #0x0, lsl #32
>
> loop_iter_first()
> 3.69 → blr bpf_prog_f173133dc38ccf87_sys_enter
> loop_iter_first()
> add x7, x0, #0x0
>
> loop_iter_first()
> add x20, x7, #0x0
>
> int size = probe_read_str(&augmented_filename->value, filename_len, filename_arg);
> mov x0, #0x1 // #1
I'm not sure all this information about annotation should be in the
changelog. This patch is about being able to decode traces that
executed outside the current kernel addresse range and as such simply
using "perf report" or "perf script" successfully is enough to test
this set. Any information that goes beyond that muddies the water.
>
> [...]
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/Makefile.config | 22 ++++++++++++++++++++++
> tools/perf/util/cs-etm.c | 19 ++++++++++++++++++-
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 51dd00f65709..a58cd5a43a98 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -418,6 +418,28 @@ ifdef CORESIGHT
> endif
> LDFLAGS += $(LIBOPENCSD_LDFLAGS)
> EXTLIBS += $(OPENCSDLIBS)
> + PRE_START_SIZE := 0
> + ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds),)
> + ifeq ($(SRCARCH),arm64)
> + # Extract info from lds:
> + # . = ((((((((0xffffffffffffffff)) - (((1)) << (48)) + 1) + (0)) + (0x08000000))) + (0x08000000))) + 0x00080000;
> + # PRE_START_SIZE := (0x08000000 + 0x08000000 + 0x00080000) = 0x10080000
> + PRE_START_SIZE := $(shell egrep ' \. \= \({8}0x[0-9a-fA-F]+\){2}' \
> + $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds | \
> + sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> + awk -F' ' '{printf "0x%x", $$6+$$7+$$8}' 2>/dev/null)
> + endif
> + ifeq ($(SRCARCH),arm)
> + # Extract info from lds:
> + # . = ((0xC0000000)) + 0x00208000;
> + # PRE_START_SIZE := 0x00208000
> + PRE_START_SIZE := $(shell egrep ' \. \= \({2}0x[0-9a-fA-F]+\){2}' \
> + $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds | \
> + sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> + awk -F' ' '{printf "0x%x", $$2}' 2>/dev/null)
> + endif
> + endif
> + CFLAGS += -DARM_PRE_START_SIZE=$(PRE_START_SIZE)
It might be useful to do this for arm and arm64 regardless of
CoreSight but I'll let Arnaldo decide on this.
> $(call detected,CONFIG_LIBOPENCSD)
> ifdef CSTRACE_RAW
> CFLAGS += -DCS_DEBUG_RAW
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 0c7776b51045..5fa0be3a3904 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -613,10 +613,27 @@ static void cs_etm__free(struct perf_session *session)
> static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address)
> {
> struct machine *machine;
> + u64 fixup_kernel_start = 0;
>
> machine = etmq->etm->machine;
>
> - if (address >= etmq->etm->kernel_start) {
> + /*
> + * Since arm and arm64 specify some memory regions prior to
> + * 'kernel_start', kernel addresses can be less than 'kernel_start'.
> + *
> + * For arm architecture, the 16MB virtual memory space prior to
> + * 'kernel_start' is allocated to device modules, a PMD table if
> + * CONFIG_HIGHMEM is enabled and a PGD table.
> + *
> + * For arm64 architecture, the root PGD table, device module memory
> + * region and BPF jit region are prior to 'kernel_start'.
> + *
> + * To reflect the complete kernel address space, compensate these
> + * pre-defined regions for kernel start address.
> + */
> + fixup_kernel_start = etmq->etm->kernel_start - ARM_PRE_START_SIZE;
> +
> + if (address >= fixup_kernel_start) {
> if (machine__is_host(machine))
> return PERF_RECORD_MISC_KERNEL;
> else
Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net-next,v3 06/11] net: flow_offload: add flow_block_cb_{priv,incref,decref}()
From: Jiri Pirko @ 2019-07-08 17:28 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ogerlitz,
Manish.Chopra, marcelo.leitner, mkubecek, venkatkumar.duvvuru,
maxime.chevallier, cphealy, netfilter-devel
In-Reply-To: <20190708160614.2226-7-pablo@netfilter.org>
Mon, Jul 08, 2019 at 06:06:08PM CEST, pablo@netfilter.org wrote:
>This patch completes the flow block API to introduce:
>
>* flow_block_cb_priv() to accept callback private data.
"accept"? That's sounds odd to me.
>* flow_block_cb_incref() to bump reference counter on this flow block.
>* flow_block_cb_decref() to decrement the reference counter.
>
>These function are taken from the existing tcf_block_cb_priv(),
"functions"
>tcf_block_cb_incref() and tcf_block_cb_decref().
>
[...]
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: vsc73xx: Fix Kconfig warning and build errors
From: Andrew Lunn @ 2019-07-08 17:28 UTC (permalink / raw)
To: YueHaibing
Cc: vivien.didelot, f.fainelli, davem, paweldembicki, linux-kernel,
netdev
In-Reply-To: <20190708144224.33376-1-yuehaibing@huawei.com>
On Mon, Jul 08, 2019 at 10:42:24PM +0800, YueHaibing wrote:
> Fix Kconfig dependency warning and subsequent build errors
> caused by OF is not set:
>
> WARNING: unmet direct dependencies detected for NET_DSA_VITESSE_VSC73XX
> Depends on [n]: NETDEVICES [=y] && HAVE_NET_DSA [=y] && OF [=n] && NET_DSA [=m]
> Selected by [m]:
> - NET_DSA_VITESSE_VSC73XX_PLATFORM [=m] && NETDEVICES [=y] && HAVE_NET_DSA [=y] && HAS_IOMEM [=y]
>
> Move OF and NET_DSA dependencies to NET_DSA_VITESSE_VSC73XX/
> NET_DSA_VITESSE_VSC73XX_PLATFORM to fix this.
Hi YueHaibing
I might be better to make NET_DSA_VITESSE_VSC73XX_SPI and NET_DSA_VITESSE_VSC73XX_PLATFORM
depend on NET_DSA_VITESSE_VSC73XX rather than select it.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v6 04/15] ethtool: introduce ethtool netlink interface
From: Michal Kubecek @ 2019-07-08 17:27 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190703084151.GR2250@nanopsycho>
On Wed, Jul 03, 2019 at 10:41:51AM +0200, Jiri Pirko wrote:
> Tue, Jul 02, 2019 at 04:52:41PM CEST, mkubecek@suse.cz wrote:
> >On Tue, Jul 02, 2019 at 02:25:21PM +0200, Jiri Pirko wrote:
> >> Tue, Jul 02, 2019 at 01:49:59PM CEST, mkubecek@suse.cz wrote:
> >> >+
> >> >+ ETHTOOL_A_HEADER_DEV_INDEX (u32) device ifindex
> >> >+ ETHTOOL_A_HEADER_DEV_NAME (string) device name
> >> >+ ETHTOOL_A_HEADER_INFOMASK (u32) info mask
> >> >+ ETHTOOL_A_HEADER_GFLAGS (u32) flags common for all requests
> >> >+ ETHTOOL_A_HEADER_RFLAGS (u32) request specific flags
> >> >+
> >> >+ETHTOOL_A_HEADER_DEV_INDEX and ETHTOOL_A_HEADER_DEV_NAME identify the device
> >> >+message relates to. One of them is sufficient in requests, if both are used,
> >> >+they must identify the same device. Some requests, e.g. global string sets, do
> >> >+not require device identification. Most GET requests also allow dump requests
> >> >+without device identification to query the same information for all devices
> >> >+providing it (each device in a separate message).
> >> >+
> >> >+Optional info mask allows to ask only for a part of data provided by GET
> >>
> >> How this "infomask" works? What are the bits related to? Is that request
> >> specific?
> >
> >The interpretation is request specific, the information returned for
> >a GET request is divided into multiple parts and client can choose to
> >request one of them (usually one). In the code so far, infomask bits
> >correspond to top level (nest) attributes but I would rather not make it
> >a strict rule.
>
> Wait, so it is a matter of verbosity? If you have multiple parts and the
> user is able to chose one of them, why don't you rather have multiple
> get commands, one per bit. This infomask construct seems redundant to me.
I thought it was a matter of verbosity because it is a very basic
element of the design, it was even advertised in the cover letter among
the basic ideas, it has been there since the very beginning and in five
previous versions through year and a half, noone did question it. That's
why I thought you objected against unclear description, not against the
concept as such.
There are two reasons for this design. First is to reduce the number of
requests needed to get the information. This is not so much a problem of
ethtool itself; the only existing commands that would result in multiple
request messages would be "ethtool <dev>" and "ethtool -s <dev>". Maybe
also "ethtool -x/-X <dev>" but even if the indirection table and hash
key have different bits assigned now, they don't have to be split even
if we split other commands. It may be bigger problem for daemons wanting
to keep track of system configuration which would have to issue many
requests whenever a new device appears.
Second reason is that with 8-bit genetlink command/message id, the space
is not as infinite as it might seem. I counted quickly, right now the
full series uses 14 ids for kernel messages, with split you propose it
would most likely grow to 44. For full implementation of all ethtool
functionality, we could get to ~60 ids. It's still only 1/4 of the
available space but it's not clear what the future development will look
like. We would certainly need to be careful not to start allocating new
commands for single parameters and try to be foreseeing about what can
be grouped together. But we will need to do that in any case.
On kernel side, splitting existing messages would make some things a bit
easier. It would also reduce the number of scenarios where only part of
requested information is available or only part of a SET request fails.
Michal
^ permalink raw reply
* Re: [PATCH net-next,v3 05/11] net: flow_offload: add list handling functions
From: Jiri Pirko @ 2019-07-08 17:26 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, davem, thomas.lendacky, f.fainelli, ariel.elior,
michael.chan, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ogerlitz,
Manish.Chopra, marcelo.leitner, mkubecek, venkatkumar.duvvuru,
maxime.chevallier, cphealy, netfilter-devel
In-Reply-To: <20190708160614.2226-6-pablo@netfilter.org>
Mon, Jul 08, 2019 at 06:06:07PM CEST, pablo@netfilter.org wrote:
>This patch adds the list handling functions for the flow block API:
>
>* flow_block_cb_lookup() allows drivers to look up for existing flow blocks.
>* flow_block_cb_add() adds a flow block to the list to be registered by the
> core.
Per driver? You say "per driver" in the "remove" part.
>* flow_block_cb_remove() to remove a flow block from the list of existing
> flow blocks per driver and to request the core to unregister this.
>
>The flow block API also annotates the netns this flow block belongs to.
>
>Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>---
>v3: extracted from former patch "net: flow_offload: add flow_block_cb API".
>
> include/net/flow_offload.h | 20 ++++++++++++++++++++
> net/core/flow_offload.c | 18 ++++++++++++++++++
> net/sched/cls_api.c | 3 +++
> 3 files changed, 41 insertions(+)
>
>diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
>index bcc4e2fef6ba..06acde2960fa 100644
>--- a/include/net/flow_offload.h
>+++ b/include/net/flow_offload.h
>@@ -251,12 +251,16 @@ struct flow_block_offload {
> enum flow_block_command command;
> enum flow_block_binder_type binder_type;
> struct tcf_block *block;
>+ struct net *net;
>+ struct list_head cb_list;
> struct list_head *driver_block_list;
> struct netlink_ext_ack *extack;
> };
>
> struct flow_block_cb {
>+ struct list_head driver_list;
> struct list_head list;
>+ struct net *net;
> tc_setup_cb_t *cb;
> void *cb_ident;
> void *cb_priv;
>@@ -269,6 +273,22 @@ struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
> void (*release)(void *cb_priv));
> void flow_block_cb_free(struct flow_block_cb *block_cb);
>
>+struct flow_block_cb *flow_block_cb_lookup(struct net *net,
>+ struct list_head *driver_flow_block_list,
>+ tc_setup_cb_t *cb, void *cb_ident);
>+
>+static inline void flow_block_cb_add(struct flow_block_cb *block_cb,
>+ struct flow_block_offload *offload)
>+{
>+ list_add_tail(&block_cb->driver_list, &offload->cb_list);
>+}
>+
>+static inline void flow_block_cb_remove(struct flow_block_cb *block_cb,
>+ struct flow_block_offload *offload)
>+{
>+ list_move(&block_cb->driver_list, &offload->cb_list);
>+}
>+
> int flow_block_cb_setup_simple(struct flow_block_offload *f,
> struct list_head *driver_list, tc_setup_cb_t *cb,
> void *cb_ident, void *cb_priv, bool ingress_only);
>diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
>index d08148cb6953..85fd5f4a1e0f 100644
>--- a/net/core/flow_offload.c
>+++ b/net/core/flow_offload.c
>@@ -176,6 +176,7 @@ struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
> if (!block_cb)
> return ERR_PTR(-ENOMEM);
>
>+ block_cb->net = net;
> block_cb->cb = cb;
> block_cb->cb_ident = cb_ident;
> block_cb->cb_priv = cb_priv;
>@@ -194,6 +195,23 @@ void flow_block_cb_free(struct flow_block_cb *block_cb)
> }
> EXPORT_SYMBOL(flow_block_cb_free);
>
>+struct flow_block_cb *flow_block_cb_lookup(struct net *net,
>+ struct list_head *driver_block_list,
In the header, you call this "driver_flow_block_list".
Where is this list coming from? In general, I don't think it is good to
have struct list_head as an arg of exported symbol. Should be contained
in some struct. Looks like this might be the "struct
flow_block_offload"?
Does this have anything to do with "struct list_head
*driver_block_list"? This is very confusing...
>+ tc_setup_cb_t *cb, void *cb_ident)
>+{
>+ struct flow_block_cb *block_cb;
>+
>+ list_for_each_entry(block_cb, driver_block_list, driver_list) {
>+ if (block_cb->net == net &&
>+ block_cb->cb == cb &&
>+ block_cb->cb_ident == cb_ident)
>+ return block_cb;
>+ }
>+
>+ return NULL;
>+}
>+EXPORT_SYMBOL(flow_block_cb_lookup);
>+
> int flow_block_cb_setup_simple(struct flow_block_offload *f,
> struct list_head *driver_block_list,
> tc_setup_cb_t *cb, void *cb_ident, void *cb_priv,
>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>index fa0c451aca59..72761b43ae41 100644
>--- a/net/sched/cls_api.c
>+++ b/net/sched/cls_api.c
>@@ -679,6 +679,7 @@ static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
> struct tc_block_offload bo = {
> .command = command,
> .binder_type = FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS,
>+ .net = dev_net(indr_dev->dev),
> .block = indr_dev->block,
> };
>
>@@ -767,6 +768,7 @@ static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
> struct tc_block_offload bo = {
> .command = command,
> .binder_type = ei->binder_type,
>+ .net = dev_net(dev),
> .block = block,
> .extack = extack,
> };
>@@ -795,6 +797,7 @@ static int tcf_block_offload_cmd(struct tcf_block *block,
> {
> struct tc_block_offload bo = {};
>
>+ bo.net = dev_net(dev);
> bo.command = command;
> bo.binder_type = ei->binder_type;
> bo.block = block;
>--
>2.11.0
>
^ permalink raw reply
* Re: [PATCH v2 net-next 1/3] tc-testing: Add JSON verification to tdc
From: Alexander Aring @ 2019-07-08 17:24 UTC (permalink / raw)
To: Lucas Bates
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner, Vlad Buslov,
Davide Caratti, kernel
In-Reply-To: <CAMDBHY+Mg9W0wJRQWeUBHCk=G0Qp4nij8B4Oz77XA6AK2Dt7Gw@mail.gmail.com>
Hi,
On Mon, Jul 08, 2019 at 12:48:12PM -0400, Lucas Bates wrote:
> On Thu, Jul 4, 2019 at 4:21 PM Alexander Aring <aring@mojatatu.com> wrote:
>
> > why you just use eval() as pattern matching operation and let the user
> > define how to declare a matching mechanism instead you introduce another
> > static matching scheme based on a json description?
> >
> > Whereas in eval() you could directly use the python bool expression
> > parser to make whatever you want.
> >
> > I don't know, I see at some points you will hit limitations what you can
> > express with this matchFOO and we need to introduce another matchBAR,
> > whereas in providing the code it should be no problem expression
> > anything. If you want smaller shortcuts writing matching patterns you
> > can implement them and using in your eval() operation.
>
> Regarding hitting limitations: quite possibly, yes.
>
> Using eval() to provide code for matching is going to put more of a
> dependency on the test writer knowing Python. I know it's not a
> terribly difficult language to pick up, but it's still setting a
> higher barrier to entry. This is the primary reason I scrapped the
> work I had presented at Netdev 1.2 in Tokyo, where all the tests were
> coded using Python's unittest framework - I want to be sure it's as
> easy as possible for people to use tdc and write tests for it.
>
> Unless I'm off-base here?
yes you need to know some python, complex code can be hidden by some
helper functionality I guess.
I have no problem to let this patch in, it will not harm anything...
Maybe I work on a matchEval and show some examples... in a human
readable way you can even concatenate bool expressions in combinations
with helpers.
I just was curious, so I might add the matchEval or something to show
this approach.
add the and it shows like:
"x == 5 or x == '5'"
Whereas you could introduce helpers to do:
"str_or_num(x, 5)"
even
"str_or_num_any_base(x, 5)"
to also catch if somebody change the base.
In this case "or" could be also concatenate with python bool
expression... depends on how lowlvl your helpers be.
Pretty sure the x as inputstring to match can also be hidden by user or
transformed with split, regex, etc before. At the end it will work like
TC with actions just provide the code to run... or is it more like
"act_bpf"?, where act is the hook and bpf the eval(). :-)
- Alex
^ permalink raw reply
* Re: [PATCH v2 net-next] net: stmmac: enable clause 45 mdio support
From: Andrew Lunn @ 2019-07-08 17:24 UTC (permalink / raw)
To: Voon Weifeng
Cc: David S. Miller, Maxime Coquelin, netdev, linux-kernel,
Jose Abreu, Giuseppe Cavallaro, Florian Fainelli,
Alexandre Torgue, biao huang, Ong Boon Leong, Kweh Hock Leong
In-Reply-To: <1562348007-12263-1-git-send-email-weifeng.voon@intel.com>
On Sat, Jul 06, 2019 at 01:33:27AM +0800, Voon Weifeng wrote:
> From: Kweh Hock Leong <hock.leong.kweh@intel.com>
>
> DWMAC4 is capable to support clause 45 mdio communication.
> This patch enable the feature on stmmac_mdio_write() and
> stmmac_mdio_read() by following phy_write_mmd() and
> phy_read_mmd() mdiobus read write implementation format.
>
> Reviewed-by: Li, Yifan <yifan2.li@intel.com>
> Signed-off-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH 1/1] tools/dtrace: initial implementation of DTrace
From: Arnaldo Carvalho de Melo @ 2019-07-08 17:15 UTC (permalink / raw)
To: Kris Van Hees
Cc: netdev, bpf, dtrace-devel, linux-kernel, rostedt, mhiramat, ast,
daniel, Peter Zijlstra, Chris Mason
In-Reply-To: <201907040314.x643EUoA017906@aserv0122.oracle.com>
Em Wed, Jul 03, 2019 at 08:14:30PM -0700, Kris Van Hees escreveu:
> This initial implementation of a tiny subset of DTrace functionality
> provides the following options:
>
> dtrace [-lvV] [-b bufsz] -s script
> -b set trace buffer size
> -l list probes (only works with '-s script' for now)
> -s enable or list probes for the specified BPF program
> -V report DTrace API version
>
> The patch comprises quite a bit of code due to DTrace requiring a few
> crucial components, even in its most basic form.
>
> The code is structured around the command line interface implemented in
> dtrace.c. It provides option parsing and drives the three modes of
> operation that are currently implemented:
>
> 1. Report DTrace API version information.
> Report the version information and terminate.
>
> 2. List probes in BPF programs.
> Initialize the list of probes that DTrace recognizes, load BPF
> programs, parse all BPF ELF section names, resolve them into
> known probes, and emit the probe names. Then terminate.
>
> 3. Load BPF programs and collect tracing data.
> Initialize the list of probes that DTrace recognizes, load BPF
> programs and attach them to their corresponding probes, set up
> perf event output buffers, and start processing tracing data.
>
> This implementation makes extensive use of BPF (handled by dt_bpf.c) and
> the perf event output ring buffer (handled by dt_buffer.c). DTrace-style
> probe handling (dt_probe.c) offers an interface to probes that hides the
> implementation details of the individual probe types by provider (dt_fbt.c
> and dt_syscall.c). Probe lookup by name uses a hashtable implementation
> (dt_hash.c). The dt_utils.c code populates a list of online CPU ids, so
> we know what CPUs we can obtain tracing data from.
>
> Building the tool is trivial because its only dependency (libbpf) is in
> the kernel tree under tools/lib/bpf. A simple 'make' in the tools/dtrace
> directory suffices.
>
> The 'dtrace' executable needs to run as root because BPF programs cannot
> be loaded by non-root users.
>
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> Reviewed-by: David Mc Lean <david.mclean@oracle.com>
> Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
> ---
> MAINTAINERS | 6 +
> tools/dtrace/Makefile | 88 ++++++++++
> tools/dtrace/bpf_sample.c | 145 ++++++++++++++++
> tools/dtrace/dt_bpf.c | 188 +++++++++++++++++++++
> tools/dtrace/dt_buffer.c | 331 +++++++++++++++++++++++++++++++++++++
> tools/dtrace/dt_fbt.c | 201 ++++++++++++++++++++++
> tools/dtrace/dt_hash.c | 211 +++++++++++++++++++++++
> tools/dtrace/dt_probe.c | 230 ++++++++++++++++++++++++++
> tools/dtrace/dt_syscall.c | 179 ++++++++++++++++++++
> tools/dtrace/dt_utils.c | 132 +++++++++++++++
> tools/dtrace/dtrace.c | 249 ++++++++++++++++++++++++++++
> tools/dtrace/dtrace.h | 13 ++
> tools/dtrace/dtrace_impl.h | 101 +++++++++++
> 13 files changed, 2074 insertions(+)
> create mode 100644 tools/dtrace/Makefile
> create mode 100644 tools/dtrace/bpf_sample.c
> create mode 100644 tools/dtrace/dt_bpf.c
> create mode 100644 tools/dtrace/dt_buffer.c
> create mode 100644 tools/dtrace/dt_fbt.c
> create mode 100644 tools/dtrace/dt_hash.c
> create mode 100644 tools/dtrace/dt_probe.c
> create mode 100644 tools/dtrace/dt_syscall.c
> create mode 100644 tools/dtrace/dt_utils.c
> create mode 100644 tools/dtrace/dtrace.c
> create mode 100644 tools/dtrace/dtrace.h
> create mode 100644 tools/dtrace/dtrace_impl.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 606d1f80bc49..668468834865 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5474,6 +5474,12 @@ W: https://linuxtv.org
> S: Odd Fixes
> F: drivers/media/pci/dt3155/
>
> +DTRACE
> +M: Kris Van Hees <kris.van.hees@oracle.com>
> +L: dtrace-devel@oss.oracle.com
> +S: Maintained
> +F: tools/dtrace/
> +
> DVB_USB_AF9015 MEDIA DRIVER
> M: Antti Palosaari <crope@iki.fi>
> L: linux-media@vger.kernel.org
> diff --git a/tools/dtrace/Makefile b/tools/dtrace/Makefile
> new file mode 100644
> index 000000000000..99fd0f9dd1d6
> --- /dev/null
> +++ b/tools/dtrace/Makefile
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# This Makefile is based on samples/bpf.
> +#
> +# Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
> +
> +DT_VERSION := 2.0.0
> +DT_GIT_VERSION := $(shell git rev-parse HEAD 2>/dev/null || \
> + echo Unknown)
> +
> +DTRACE_PATH ?= $(abspath $(srctree)/$(src))
> +TOOLS_PATH := $(DTRACE_PATH)/..
> +SAMPLES_PATH := $(DTRACE_PATH)/../../samples
> +
> +hostprogs-y := dtrace
> +
> +LIBBPF := $(TOOLS_PATH)/lib/bpf/libbpf.a
> +OBJS := dt_bpf.o dt_buffer.o dt_utils.o dt_probe.o \
> + dt_hash.o \
> + dt_fbt.o dt_syscall.o
> +
> +dtrace-objs := $(OBJS) dtrace.o
> +
> +always := $(hostprogs-y)
> +always += bpf_sample.o
> +
> +KBUILD_HOSTCFLAGS += -DDT_VERSION=\"$(DT_VERSION)\"
> +KBUILD_HOSTCFLAGS += -DDT_GIT_VERSION=\"$(DT_GIT_VERSION)\"
> +KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib
> +KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf
Interesting, what are you using from tools/perf/? So that we can move to
tools/{include,lib,arch}.
> +KBUILD_HOSTCFLAGS += -I$(srctree)/tools/include/uapi
> +KBUILD_HOSTCFLAGS += -I$(srctree)/tools/include/
> +KBUILD_HOSTCFLAGS += -I$(srctree)/usr/include
> +
> +KBUILD_HOSTLDLIBS := $(LIBBPF) -lelf
> +
> +LLC ?= llc
> +CLANG ?= clang
> +LLVM_OBJCOPY ?= llvm-objcopy
> +
> +ifdef CROSS_COMPILE
> +HOSTCC = $(CROSS_COMPILE)gcc
> +CLANG_ARCH_ARGS = -target $(ARCH)
> +endif
> +
> +all:
> + $(MAKE) -C ../../ $(CURDIR)/ DTRACE_PATH=$(CURDIR)
> +
> +clean:
> + $(MAKE) -C ../../ M=$(CURDIR) clean
> + @rm -f *~
> +
> +$(LIBBPF): FORCE
> + $(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(DTRACE_PATH)/../../ O=
> +
> +FORCE:
> +
> +.PHONY: verify_cmds verify_target_bpf $(CLANG) $(LLC)
> +
> +verify_cmds: $(CLANG) $(LLC)
> + @for TOOL in $^ ; do \
> + if ! (which -- "$${TOOL}" > /dev/null 2>&1); then \
> + echo "*** ERROR: Cannot find LLVM tool $${TOOL}" ;\
> + exit 1; \
> + else true; fi; \
> + done
> +
> +verify_target_bpf: verify_cmds
> + @if ! (${LLC} -march=bpf -mattr=help > /dev/null 2>&1); then \
> + echo "*** ERROR: LLVM (${LLC}) does not support 'bpf' target" ;\
> + echo " NOTICE: LLVM version >= 3.7.1 required" ;\
> + exit 2; \
> + else true; fi
> +
> +$(DTRACE_PATH)/*.c: verify_target_bpf $(LIBBPF)
> +$(src)/*.c: verify_target_bpf $(LIBBPF)
> +
> +$(obj)/%.o: $(src)/%.c
> + @echo " CLANG-bpf " $@
> + $(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
> + -I$(srctree)/tools/testing/selftests/bpf/ \
> + -D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \
> + -D__TARGET_ARCH_$(ARCH) -Wno-compare-distinct-pointer-types \
> + -Wno-gnu-variable-sized-type-not-at-end \
> + -Wno-address-of-packed-member -Wno-tautological-compare \
> + -Wno-unknown-warning-option $(CLANG_ARCH_ARGS) \
> + -I$(srctree)/samples/bpf/ -include asm_goto_workaround.h \
> + -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf $(LLC_FLAGS) -filetype=obj -o $@
We have the above in tools/perf/util/llvm-utils.c, perhaps we need to
move it to some place in lib/ to share?
> diff --git a/tools/dtrace/bpf_sample.c b/tools/dtrace/bpf_sample.c
> new file mode 100644
> index 000000000000..49f350390b5f
> --- /dev/null
> +++ b/tools/dtrace/bpf_sample.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * This sample DTrace BPF tracing program demonstrates how actions can be
> + * associated with different probe types.
> + *
> + * The kprobe/ksys_write probe is a Function Boundary Tracing (FBT) entry probe
> + * on the ksys_write(fd, buf, count) function in the kernel. Arguments to the
> + * function can be retrieved from the CPU registers (struct pt_regs).
> + *
> + * The tracepoint/syscalls/sys_enter_write probe is a System Call entry probe
> + * for the write(d, buf, count) system call. Arguments to the system call can
> + * be retrieved from the tracepoint data passed to the BPF program as context
> + * struct syscall_data) when the probe fires.
> + *
> + * The BPF program associated with each probe prepares a DTrace BPF context
> + * (struct dt_bpf_context) that stores the probe ID and up to 10 arguments.
> + * Only 3 arguments are used in this sample. Then the prorgams call a shared
> + * BPF function (bpf_action) that implements the actual action to be taken when
> + * a probe fires. It prepares a data record to be stored in the tracing buffer
> + * and submits it to the buffer. The data in the data record is obtained from
> + * the DTrace BPF context.
> + *
> + * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
> + */
> +#include <uapi/linux/bpf.h>
> +#include <linux/ptrace.h>
> +#include <linux/version.h>
> +#include <uapi/linux/unistd.h>
> +#include "bpf_helpers.h"
> +
> +#include "dtrace.h"
> +
> +struct syscall_data {
> + struct pt_regs *regs;
> + long syscall_nr;
> + long arg[6];
> +};
> +
> +struct bpf_map_def SEC("maps") buffers = {
> + .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> + .key_size = sizeof(u32),
> + .value_size = sizeof(u32),
> + .max_entries = NR_CPUS,
> +};
> +
> +#if defined(__amd64)
> +# define GET_REGS_ARG0(regs) ((regs)->di)
> +# define GET_REGS_ARG1(regs) ((regs)->si)
> +# define GET_REGS_ARG2(regs) ((regs)->dx)
> +# define GET_REGS_ARG3(regs) ((regs)->cx)
> +# define GET_REGS_ARG4(regs) ((regs)->r8)
> +# define GET_REGS_ARG5(regs) ((regs)->r9)
> +#else
> +# warning Argument retrieval from pt_regs is not supported yet on this arch.
> +# define GET_REGS_ARG0(regs) 0
> +# define GET_REGS_ARG1(regs) 0
> +# define GET_REGS_ARG2(regs) 0
> +# define GET_REGS_ARG3(regs) 0
> +# define GET_REGS_ARG4(regs) 0
> +# define GET_REGS_ARG5(regs) 0
> +#endif
We have this in tools/testing/selftests/bpf/bpf_helpers.h, probably need
to move to some other place in tools/include/ where this can be shared.
- Arnaldo
^ permalink raw reply
* [PATCH net-next] sctp: remove rcu_read_lock from sctp_bind_addr_state
From: Xin Long @ 2019-07-08 16:59 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
sctp_bind_addr_state() is called either in packet rcv path or
by sctp_copy_local_addr_list(), which are under rcu_read_lock.
So there's no need to call it again in sctp_bind_addr_state().
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/bind_addr.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
index f54333c..53bc615 100644
--- a/net/sctp/bind_addr.c
+++ b/net/sctp/bind_addr.c
@@ -393,24 +393,19 @@ int sctp_bind_addr_state(const struct sctp_bind_addr *bp,
{
struct sctp_sockaddr_entry *laddr;
struct sctp_af *af;
- int state = -1;
af = sctp_get_af_specific(addr->sa.sa_family);
if (unlikely(!af))
- return state;
+ return -1;
- rcu_read_lock();
list_for_each_entry_rcu(laddr, &bp->address_list, list) {
if (!laddr->valid)
continue;
- if (af->cmp_addr(&laddr->a, addr)) {
- state = laddr->state;
- break;
- }
+ if (af->cmp_addr(&laddr->a, addr))
+ return laddr->state;
}
- rcu_read_unlock();
- return state;
+ return -1;
}
/* Find the first address in the bind address list that is not present in
--
2.1.0
^ permalink raw reply related
* Re: [PATCH rdma-next 0/2] DEVX VHCA tunnel support
From: Jason Gunthorpe @ 2019-07-08 16:58 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Max Gurtovoy,
Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <20190701181402.25286-1-leon@kernel.org>
On Mon, Jul 01, 2019 at 09:14:00PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Hi,
>
> Those two patches introduce VHCA tunnel mechanism to DEVX interface
> needed for Bluefield SOC. See extensive commit messages for more
> information.
>
> Thanks
>
> Max Gurtovoy (2):
> net/mlx5: Introduce VHCA tunnel device capability
> IB/mlx5: Implement VHCA tunnel mechanism in DEVX
Thanks, applied to for-next
Jason
^ permalink raw reply
* [PATCH net-next 4/4] sctp: rename sp strm_interleave to ep intl_enable
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1562604972.git.lucien.xin@gmail.com>
Like other endpoint features, strm_interleave should be moved to
sctp_endpoint and renamed to intl_enable.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 2 +-
net/sctp/sm_make_chunk.c | 4 ++--
net/sctp/socket.c | 8 ++++----
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index c41b57b..ba5c4f6 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -219,7 +219,6 @@ struct sctp_sock {
disable_fragments:1,
v4mapped:1,
frag_interleave:1,
- strm_interleave:1,
recvrcvinfo:1,
recvnxtinfo:1,
data_ready_signalled:1;
@@ -1324,6 +1323,7 @@ struct sctp_endpoint {
struct list_head endpoint_shared_keys;
__u16 active_key_id;
__u8 auth_enable:1,
+ intl_enable:1,
prsctp_enable:1,
reconf_enable:1;
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 31ab2c6..ed39396 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -269,7 +269,7 @@ struct sctp_chunk *sctp_make_init(const struct sctp_association *asoc,
if (sp->adaptation_ind)
chunksize += sizeof(aiparam);
- if (sp->strm_interleave) {
+ if (asoc->ep->intl_enable) {
extensions[num_ext] = SCTP_CID_I_DATA;
num_ext += 1;
}
@@ -2027,7 +2027,7 @@ static void sctp_process_ext_param(struct sctp_association *asoc,
asoc->peer.asconf_capable = 1;
break;
case SCTP_CID_I_DATA:
- if (sctp_sk(asoc->base.sk)->strm_interleave)
+ if (asoc->ep->intl_enable)
asoc->peer.intl_capable = 1;
break;
default:
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index b679b61..0fc5b90 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1913,7 +1913,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
if (err)
goto err;
- if (sp->strm_interleave) {
+ if (asoc->ep->intl_enable) {
timeo = sock_sndtimeo(sk, 0);
err = sctp_wait_for_connect(asoc, &timeo);
if (err) {
@@ -3581,7 +3581,7 @@ static int sctp_setsockopt_fragment_interleave(struct sock *sk,
sctp_sk(sk)->frag_interleave = !!val;
if (!sctp_sk(sk)->frag_interleave)
- sctp_sk(sk)->strm_interleave = 0;
+ sctp_sk(sk)->ep->intl_enable = 0;
return 0;
}
@@ -4484,7 +4484,7 @@ static int sctp_setsockopt_interleaving_supported(struct sock *sk,
goto out;
}
- sp->strm_interleave = !!params.assoc_value;
+ sp->ep->intl_enable = !!params.assoc_value;
retval = 0;
@@ -7711,7 +7711,7 @@ static int sctp_getsockopt_interleaving_supported(struct sock *sk, int len,
}
params.assoc_value = asoc ? asoc->peer.intl_capable
- : sctp_sk(sk)->strm_interleave;
+ : sctp_sk(sk)->ep->intl_enable;
if (put_user(len, optlen))
goto out;
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 3/4] sctp: rename asoc intl_enable to asoc peer.intl_capable
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1562604972.git.lucien.xin@gmail.com>
To keep consistent with other asoc features, we move intl_enable
to peer.intl_capable in asoc.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 33 +++++++++++++++++----------------
net/sctp/sm_make_chunk.c | 4 ++--
net/sctp/socket.c | 2 +-
net/sctp/stream_interleave.c | 4 ++--
net/sctp/stream_sched.c | 2 +-
5 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 7f35b8e..c41b57b 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1679,28 +1679,30 @@ struct sctp_association {
__be16 addip_disabled_mask;
/* These are capabilities which our peer advertised. */
- __u8 ecn_capable:1, /* Can peer do ECN? */
+ __u16 ecn_capable:1, /* Can peer do ECN? */
ipv4_address:1, /* Peer understands IPv4 addresses? */
ipv6_address:1, /* Peer understands IPv6 addresses? */
hostname_address:1, /* Peer understands DNS addresses? */
asconf_capable:1, /* Does peer support ADDIP? */
prsctp_capable:1, /* Can peer do PR-SCTP? */
reconf_capable:1, /* Can peer do RE-CONFIG? */
- auth_capable:1; /* Is peer doing SCTP-AUTH? */
-
- /* sack_needed : This flag indicates if the next received
- * : packet is to be responded to with a
- * : SACK. This is initialized to 0. When a packet
- * : is received sack_cnt is incremented. If this value
- * : reaches 2 or more, a SACK is sent and the
- * : value is reset to 0. Note: This is used only
- * : when no DATA chunks are received out of
- * : order. When DATA chunks are out of order,
- * : SACK's are not delayed (see Section 6).
- */
- __u8 sack_needed:1, /* Do we need to sack the peer? */
+ intl_capable:1, /* Can peer do INTERLEAVE */
+ auth_capable:1, /* Is peer doing SCTP-AUTH? */
+ /* sack_needed:
+ * This flag indicates if the next received
+ * packet is to be responded to with a
+ * SACK. This is initialized to 0. When a packet
+ * is received sack_cnt is incremented. If this value
+ * reaches 2 or more, a SACK is sent and the
+ * value is reset to 0. Note: This is used only
+ * when no DATA chunks are received out of
+ * order. When DATA chunks are out of order,
+ * SACK's are not delayed (see Section 6).
+ */
+ sack_needed:1, /* Do we need to sack the peer? */
sack_generation:1,
zero_window_announced:1;
+
__u32 sack_cnt;
__u32 adaptation_ind; /* Adaptation Code point. */
@@ -2049,8 +2051,7 @@ struct sctp_association {
__u8 need_ecne:1, /* Need to send an ECNE Chunk? */
temp:1, /* Is it a temporary association? */
- force_delay:1,
- intl_enable:1;
+ force_delay:1;
__u8 strreset_enable;
__u8 strreset_outstanding; /* request param count on the fly */
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 227bbac..31ab2c6 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -438,7 +438,7 @@ struct sctp_chunk *sctp_make_init_ack(const struct sctp_association *asoc,
if (sp->adaptation_ind)
chunksize += sizeof(aiparam);
- if (asoc->intl_enable) {
+ if (asoc->peer.intl_capable) {
extensions[num_ext] = SCTP_CID_I_DATA;
num_ext += 1;
}
@@ -2028,7 +2028,7 @@ static void sctp_process_ext_param(struct sctp_association *asoc,
break;
case SCTP_CID_I_DATA:
if (sctp_sk(asoc->base.sk)->strm_interleave)
- asoc->intl_enable = 1;
+ asoc->peer.intl_capable = 1;
break;
default:
break;
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index da2a3c2..b679b61 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -7710,7 +7710,7 @@ static int sctp_getsockopt_interleaving_supported(struct sock *sk, int len,
goto out;
}
- params.assoc_value = asoc ? asoc->intl_enable
+ params.assoc_value = asoc ? asoc->peer.intl_capable
: sctp_sk(sk)->strm_interleave;
if (put_user(len, optlen))
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index afbf122..40c40be 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -1358,6 +1358,6 @@ void sctp_stream_interleave_init(struct sctp_stream *stream)
struct sctp_association *asoc;
asoc = container_of(stream, struct sctp_association, stream);
- stream->si = asoc->intl_enable ? &sctp_stream_interleave_1
- : &sctp_stream_interleave_0;
+ stream->si = asoc->peer.intl_capable ? &sctp_stream_interleave_1
+ : &sctp_stream_interleave_0;
}
diff --git a/net/sctp/stream_sched.c b/net/sctp/stream_sched.c
index b8fa7ab..99e5f69 100644
--- a/net/sctp/stream_sched.c
+++ b/net/sctp/stream_sched.c
@@ -228,7 +228,7 @@ int sctp_sched_get_value(struct sctp_association *asoc, __u16 sid,
void sctp_sched_dequeue_done(struct sctp_outq *q, struct sctp_chunk *ch)
{
if (!list_is_last(&ch->frag_list, &ch->msg->chunks) &&
- !q->asoc->intl_enable) {
+ !q->asoc->peer.intl_capable) {
struct sctp_stream_out *sout;
__u16 sid;
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 2/4] sctp: remove prsctp_enable from asoc
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1562604972.git.lucien.xin@gmail.com>
Like reconf_enable, prsctp_enable should also be removed from asoc,
as asoc->peer.prsctp_capable has taken its job.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 3 +--
net/sctp/associola.c | 1 -
net/sctp/sm_make_chunk.c | 8 ++++----
net/sctp/socket.c | 2 +-
4 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index d9e0e1a..7f35b8e 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2050,8 +2050,7 @@ struct sctp_association {
__u8 need_ecne:1, /* Need to send an ECNE Chunk? */
temp:1, /* Is it a temporary association? */
force_delay:1,
- intl_enable:1,
- prsctp_enable:1;
+ intl_enable:1;
__u8 strreset_enable;
__u8 strreset_outstanding; /* request param count on the fly */
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 321c199..5010cce 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -261,7 +261,6 @@ static struct sctp_association *sctp_association_init(
goto stream_free;
asoc->active_key_id = ep->active_key_id;
- asoc->prsctp_enable = ep->prsctp_enable;
asoc->strreset_enable = ep->strreset_enable;
/* Save the hmacs and chunks list into this association */
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index d784dc1..227bbac 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -247,7 +247,7 @@ struct sctp_chunk *sctp_make_init(const struct sctp_association *asoc,
chunksize += SCTP_PAD4(SCTP_SAT_LEN(num_types));
chunksize += sizeof(ecap_param);
- if (asoc->prsctp_enable)
+ if (asoc->ep->prsctp_enable)
chunksize += sizeof(prsctp_param);
/* ADDIP: Section 4.2.7:
@@ -348,7 +348,7 @@ struct sctp_chunk *sctp_make_init(const struct sctp_association *asoc,
sctp_addto_param(retval, num_ext, extensions);
}
- if (asoc->prsctp_enable)
+ if (asoc->ep->prsctp_enable)
sctp_addto_chunk(retval, sizeof(prsctp_param), &prsctp_param);
if (sp->adaptation_ind) {
@@ -2011,7 +2011,7 @@ static void sctp_process_ext_param(struct sctp_association *asoc,
asoc->peer.reconf_capable = 1;
break;
case SCTP_CID_FWD_TSN:
- if (asoc->prsctp_enable && !asoc->peer.prsctp_capable)
+ if (asoc->ep->prsctp_enable)
asoc->peer.prsctp_capable = 1;
break;
case SCTP_CID_AUTH:
@@ -2636,7 +2636,7 @@ static int sctp_process_param(struct sctp_association *asoc,
break;
case SCTP_PARAM_FWD_TSN_SUPPORT:
- if (asoc->prsctp_enable) {
+ if (asoc->ep->prsctp_enable) {
asoc->peer.prsctp_capable = 1;
break;
}
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 0424876..da2a3c2 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -7343,7 +7343,7 @@ static int sctp_getsockopt_pr_supported(struct sock *sk, int len,
goto out;
}
- params.assoc_value = asoc ? asoc->prsctp_enable
+ params.assoc_value = asoc ? asoc->peer.prsctp_capable
: sctp_sk(sk)->ep->prsctp_enable;
if (put_user(len, optlen))
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 1/4] sctp: remove reconf_enable from asoc
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1562604972.git.lucien.xin@gmail.com>
asoc's reconf support is actually decided by the 4-shakehand negotiation,
not something that users can set by sockopt. asoc->peer.reconf_capable is
working for this. So remove it from asoc.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 3 +--
net/sctp/associola.c | 1 -
net/sctp/sm_make_chunk.c | 5 ++---
net/sctp/socket.c | 7 ++-----
4 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 0767701..d9e0e1a 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2051,8 +2051,7 @@ struct sctp_association {
temp:1, /* Is it a temporary association? */
force_delay:1,
intl_enable:1,
- prsctp_enable:1,
- reconf_enable:1;
+ prsctp_enable:1;
__u8 strreset_enable;
__u8 strreset_outstanding; /* request param count on the fly */
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 1999237..321c199 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -262,7 +262,6 @@ static struct sctp_association *sctp_association_init(
asoc->active_key_id = ep->active_key_id;
asoc->prsctp_enable = ep->prsctp_enable;
- asoc->reconf_enable = ep->reconf_enable;
asoc->strreset_enable = ep->strreset_enable;
/* Save the hmacs and chunks list into this association */
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 9b0e5b0..d784dc1 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -261,7 +261,7 @@ struct sctp_chunk *sctp_make_init(const struct sctp_association *asoc,
num_ext += 2;
}
- if (asoc->reconf_enable) {
+ if (asoc->ep->reconf_enable) {
extensions[num_ext] = SCTP_CID_RECONF;
num_ext += 1;
}
@@ -2007,8 +2007,7 @@ static void sctp_process_ext_param(struct sctp_association *asoc,
for (i = 0; i < num_ext; i++) {
switch (param.ext->chunks[i]) {
case SCTP_CID_RECONF:
- if (asoc->reconf_enable &&
- !asoc->peer.reconf_capable)
+ if (asoc->ep->reconf_enable)
asoc->peer.reconf_capable = 1;
break;
case SCTP_CID_FWD_TSN:
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 39ea0a3..0424876 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4226,10 +4226,7 @@ static int sctp_setsockopt_reconfig_supported(struct sock *sk,
sctp_style(sk, UDP))
goto out;
- if (asoc)
- asoc->reconf_enable = !!params.assoc_value;
- else
- sctp_sk(sk)->ep->reconf_enable = !!params.assoc_value;
+ sctp_sk(sk)->ep->reconf_enable = !!params.assoc_value;
retval = 0;
@@ -7554,7 +7551,7 @@ static int sctp_getsockopt_reconfig_supported(struct sock *sk, int len,
goto out;
}
- params.assoc_value = asoc ? asoc->reconf_enable
+ params.assoc_value = asoc ? asoc->peer.reconf_capable
: sctp_sk(sk)->ep->reconf_enable;
if (put_user(len, optlen))
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 0/4] sctp: tidy up some ep and asoc feature flags
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
This patchset is to remove some unnecessary feature flags from
sctp_assocation and move some others to the right places.
Xin Long (4):
sctp: remove reconf_enable from asoc
sctp: remove prsctp_enable from asoc
sctp: rename asoc intl_enable to asoc peer.intl_capable
sctp: rename sp strm_interleave to ep intl_enable
include/net/sctp/structs.h | 37 ++++++++++++++++++-------------------
net/sctp/associola.c | 2 --
net/sctp/sm_make_chunk.c | 21 ++++++++++-----------
net/sctp/socket.c | 19 ++++++++-----------
net/sctp/stream_interleave.c | 4 ++--
net/sctp/stream_sched.c | 2 +-
6 files changed, 39 insertions(+), 46 deletions(-)
--
2.1.0
^ permalink raw reply
* Re: [bpf-next v2 08/10] bpf: Implement bpf_prog_test_run for perf event programs
From: Krzesimir Nowak @ 2019-07-08 16:51 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, Alban Crequy, Iago López Galeiras,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, linux-kernel, bpf
In-Reply-To: <20190626161231.GA4866@mini-arch>
On Wed, Jun 26, 2019 at 6:12 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/26, Krzesimir Nowak wrote:
> > On Tue, Jun 25, 2019 at 10:12 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> > >
> > > On 06/25, Krzesimir Nowak wrote:
> > > > As an input, test run for perf event program takes struct
> > > > bpf_perf_event_data as ctx_in and struct bpf_perf_event_value as
> > > > data_in. For an output, it basically ignores ctx_out and data_out.
> > > >
> > > > The implementation sets an instance of struct bpf_perf_event_data_kern
> > > > in such a way that the BPF program reading data from context will
> > > > receive what we passed to the bpf prog test run in ctx_in. Also BPF
> > > > program can call bpf_perf_prog_read_value to receive what was passed
> > > > in data_in.
> > > >
> > > > Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
> > > > ---
> > > > kernel/trace/bpf_trace.c | 107 ++++++++++++++++++
> > > > .../bpf/verifier/perf_event_sample_period.c | 8 ++
> > > > 2 files changed, 115 insertions(+)
> > > >
> > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > > > index c102c240bb0b..2fa49ea8a475 100644
> > > > --- a/kernel/trace/bpf_trace.c
> > > > +++ b/kernel/trace/bpf_trace.c
> > > > @@ -16,6 +16,8 @@
> > > >
> > > > #include <asm/tlb.h>
> > > >
> > > > +#include <trace/events/bpf_test_run.h>
> > > > +
> > > > #include "trace_probe.h"
> > > > #include "trace.h"
> > > >
> > > > @@ -1160,7 +1162,112 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
> > > > .convert_ctx_access = pe_prog_convert_ctx_access,
> > > > };
> > > >
> > > > +static int pe_prog_test_run(struct bpf_prog *prog,
> > > > + const union bpf_attr *kattr,
> > > > + union bpf_attr __user *uattr)
> > > > +{
> > > > + void __user *ctx_in = u64_to_user_ptr(kattr->test.ctx_in);
> > > > + void __user *data_in = u64_to_user_ptr(kattr->test.data_in);
> > > > + u32 data_size_in = kattr->test.data_size_in;
> > > > + u32 ctx_size_in = kattr->test.ctx_size_in;
> > > > + u32 repeat = kattr->test.repeat;
> > > > + u32 retval = 0, duration = 0;
> > > > + int err = -EINVAL;
> > > > + u64 time_start, time_spent = 0;
> > > > + int i;
> > > > + struct perf_sample_data sample_data = {0, };
> > > > + struct perf_event event = {0, };
> > > > + struct bpf_perf_event_data_kern real_ctx = {0, };
> > > > + struct bpf_perf_event_data fake_ctx = {0, };
> > > > + struct bpf_perf_event_value value = {0, };
> > > > +
> > > > + if (ctx_size_in != sizeof(fake_ctx))
> > > > + goto out;
> > > > + if (data_size_in != sizeof(value))
> > > > + goto out;
> > > > +
> > > > + if (copy_from_user(&fake_ctx, ctx_in, ctx_size_in)) {
> > > > + err = -EFAULT;
> > > > + goto out;
> > > > + }
> > > Move this to net/bpf/test_run.c? I have a bpf_ctx_init helper to deal
> > > with ctx input, might save you some code above wrt ctx size/etc.
> >
> > My impression about net/bpf/test_run.c was that it was a collection of
> > helpers for test runs of the network-related BPF programs, because
> > they are so similar to each other. So kernel/trace/bpf_trace.c looked
> > like an obvious place for the test_run implementation since other perf
> > trace BPF stuff was already there.
> Maybe net/bpf/test_run.c should be renamed to kernel/bpf/test_run.c?
Just sent another version of this patch series. I went with slightly
different approach - moved some functions to kernel/bpf/test_run.c and
left the network specific stuff in net/bpf/test_run.c.
>
> > And about bpf_ctx_init - looks useful as it seems to me that it
> > handles the scenario where the size of the ctx struct grows, but still
> > allows passing older version of the struct (thus smaller) from
> > userspace for compatibility. Maybe that checking and copying part of
> > the function could be moved into some non-static helper function, so I
> > could use it and still skip the need for allocating memory for the
> > context?
> You can always make bpf_ctx_init non-static and export it.
> But, again, consider adding your stuff to the net/bpf/test_run.c
> and exporting only pe_prog_test_run. That way you can reuse
> bpf_ctx_init and bpf_test_run.
>
> Why do you care about memory allocation though? It's a one time
> operation and doesn't affect the performance measurements.
>
> > > > + if (copy_from_user(&value, data_in, data_size_in)) {
> > > > + err = -EFAULT;
> > > > + goto out;
> > > > + }
> > > > +
> > > > + real_ctx.regs = &fake_ctx.regs;
> > > > + real_ctx.data = &sample_data;
> > > > + real_ctx.event = &event;
> > > > + perf_sample_data_init(&sample_data, fake_ctx.addr,
> > > > + fake_ctx.sample_period);
> > > > + event.cpu = smp_processor_id();
> > > > + event.oncpu = -1;
> > > > + event.state = PERF_EVENT_STATE_OFF;
> > > > + local64_set(&event.count, value.counter);
> > > > + event.total_time_enabled = value.enabled;
> > > > + event.total_time_running = value.running;
> > > > + /* make self as a leader - it is used only for checking the
> > > > + * state field
> > > > + */
> > > > + event.group_leader = &event;
> > > > +
> > > > + /* slightly changed copy pasta from bpf_test_run() in
> > > > + * net/bpf/test_run.c
> > > > + */
> > > > + if (!repeat)
> > > > + repeat = 1;
> > > > +
> > > > + rcu_read_lock();
> > > > + preempt_disable();
> > > > + time_start = ktime_get_ns();
> > > > + for (i = 0; i < repeat; i++) {
> > > Any reason for not using bpf_test_run?
> >
> > Two, mostly. One was that it is a static function and my code was
> > elsewhere. Second was that it does some cgroup storage setup and I'm
> > not sure if the perf event BPF program needs that.
> You can always make it non-static.
>
> Regarding cgroup storage: do we care? If you can see it affecting
> your performance numbers, then yes, but you can try to measure to see
> if it gives you any noticeable overhead. Maybe add an argument to
> bpf_test_run to skip cgroup storage stuff?
>
> > > > + retval = BPF_PROG_RUN(prog, &real_ctx);
> > > > +
> > > > + if (signal_pending(current)) {
> > > > + err = -EINTR;
> > > > + preempt_enable();
> > > > + rcu_read_unlock();
> > > > + goto out;
> > > > + }
> > > > +
> > > > + if (need_resched()) {
> > > > + time_spent += ktime_get_ns() - time_start;
> > > > + preempt_enable();
> > > > + rcu_read_unlock();
> > > > +
> > > > + cond_resched();
> > > > +
> > > > + rcu_read_lock();
> > > > + preempt_disable();
> > > > + time_start = ktime_get_ns();
> > > > + }
> > > > + }
> > > > + time_spent += ktime_get_ns() - time_start;
> > > > + preempt_enable();
> > > > + rcu_read_unlock();
> > > > +
> > > > + do_div(time_spent, repeat);
> > > > + duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> > > > + /* end of slightly changed copy pasta from bpf_test_run() in
> > > > + * net/bpf/test_run.c
> > > > + */
> > > > +
> > > > + if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval))) {
> > > > + err = -EFAULT;
> > > > + goto out;
> > > > + }
> > > > + if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration))) {
> > > > + err = -EFAULT;
> > > > + goto out;
> > > > + }
> > > Can BPF program modify fake_ctx? Do we need/want to copy it back?
> >
> > Reading the pe_prog_is_valid_access function tells me that it's not
> > possible - the only type of valid access is read. So maybe I should be
> > stricter about the requirements for the data_out and ctx_out sizes
> > (should be zero or return -EINVAL).
> Yes, better to explicitly prohibit anything that we don't support.
>
> > > > + err = 0;
> > > > +out:
> > > > + trace_bpf_test_finish(&err);
> > > > + return err;
> > > > +}
> > > > +
> > > > const struct bpf_prog_ops perf_event_prog_ops = {
> > > > + .test_run = pe_prog_test_run,
> > > > };
> > > >
> > > > static DEFINE_MUTEX(bpf_event_mutex);
> > > > diff --git a/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c b/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
> > > > index 471c1a5950d8..16e9e5824d14 100644
> > > > --- a/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
> > > > +++ b/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
> > > This should probably go in another patch.
> >
> > Yeah, I was wondering about it. These changes are here to avoid
> > breaking the tests, since perf event program can actually be run now
> > and the test_run for perf event required certain sizes for ctx and
> > data.
> You need to make sure the context is optional, that way you don't break
> any existing tests out in the wild and can move those changes to
> another patch.
>
> > So, I will either move them to a separate patch or rework the test_run
> > for perf event to accept the size between 0 and sizeof(struct
> > something), so the changes in tests maybe will not be necessary.
> >
> > >
> > > > @@ -13,6 +13,8 @@
> > > > },
> > > > .result = ACCEPT,
> > > > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > > > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > > > + .data_len = sizeof(struct bpf_perf_event_value),
> > > > },
> > > > {
> > > > "check bpf_perf_event_data->sample_period half load permitted",
> > > > @@ -29,6 +31,8 @@
> > > > },
> > > > .result = ACCEPT,
> > > > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > > > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > > > + .data_len = sizeof(struct bpf_perf_event_value),
> > > > },
> > > > {
> > > > "check bpf_perf_event_data->sample_period word load permitted",
> > > > @@ -45,6 +49,8 @@
> > > > },
> > > > .result = ACCEPT,
> > > > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > > > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > > > + .data_len = sizeof(struct bpf_perf_event_value),
> > > > },
> > > > {
> > > > "check bpf_perf_event_data->sample_period dword load permitted",
> > > > @@ -56,4 +62,6 @@
> > > > },
> > > > .result = ACCEPT,
> > > > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > > > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > > > + .data_len = sizeof(struct bpf_perf_event_value),
> > > > },
> > > > --
> > > > 2.20.1
> > > >
> >
> >
> >
> > --
> > Kinvolk GmbH | Adalbertstr.6a, 10999 Berlin | tel: +491755589364
> > Geschäftsführer/Directors: Alban Crequy, Chris Kühl, Iago López Galeiras
> > Registergericht/Court of registration: Amtsgericht Charlottenburg
> > Registernummer/Registration number: HRB 171414 B
> > Ust-ID-Nummer/VAT ID number: DE302207000
--
Kinvolk GmbH | Adalbertstr.6a, 10999 Berlin | tel: +491755589364
Geschäftsführer/Directors: Alban Crequy, Chris Kühl, Iago López Galeiras
Registergericht/Court of registration: Amtsgericht Charlottenburg
Registernummer/Registration number: HRB 171414 B
Ust-ID-Nummer/VAT ID number: DE302207000
^ permalink raw reply
* Re: [PATCH bpf-next 0/3] xdp: Add devmap_hash map type
From: Jonathan Lemon @ 2019-07-08 16:50 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <87bly4zg8n.fsf@toke.dk>
On 8 Jul 2019, at 8:40, Toke Høiland-Jørgensen wrote:
> "Jonathan Lemon" <jlemon@flugsvamp.com> writes:
>
>> On 5 Jul 2019, at 10:56, Toke Høiland-Jørgensen wrote:
>>
>>> This series adds a new map type, devmap_hash, that works like the
>>> existing
>>> devmap type, but using a hash-based indexing scheme. This is useful
>>> for the use
>>> case where a devmap is indexed by ifindex (for instance for use with
>>> the routing
>>> table lookup helper). For this use case, the regular devmap needs to
>>> be sized
>>> after the maximum ifindex number, not the number of devices in it. A
>>> hash-based
>>> indexing scheme makes it possible to size the map after the number of
>>> devices it
>>> should contain instead.
>>
>> This device hash map is sized at NETDEV_HASHENTRIES == 2^8 == 256. Is
>> this actually smaller than an array? What ifindex values are you
>> seeing?
>
> Well, not in all cases, certainly. But machines with lots of virtual
> interfaces (e.g., container hosts) can easily exceed that. Also, for a
> devmap we charge the full size of max_entries * struct bpf_dtab_netdev
> towards the locked memory cost on map creation. And since sizeof(struct
> bpf_dtab_netdev) is 64, the size of the hashmap only corresponds to 32
> entries...
>
> But more importantly, it's a UI issue: Say you want to create a simple
> program that uses the fib_lookup helper (something like the xdp_fwd
> example under samples/bpf/). You know that you only want to route
> between a couple of interfaces, so you naturally create a devmap that
> can hold, say, 8 entries (just to be sure). This works fine on your
> initial test, where the machine only has a couple of physical interfaces
> brought up at boot. But then you try to run the same program on your
> production server, where the interfaces you need to use just happen to
> have ifindexes higher than 8, and now it breaks for no discernible
> reason. Or even worse, if you remove and re-add an interface, you may no
> longer be able to insert it into your map because the ifindex changed...
Thanks for the explanation, that makes sense.
--
Jonathan
^ permalink raw reply
* Re: [PATCH 1/1] tools/dtrace: initial implementation of DTrace
From: Kris Van Hees @ 2019-07-08 16:48 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Kris Van Hees, netdev, bpf, dtrace-devel, linux-kernel, rostedt,
mhiramat, acme, ast, daniel, Chris Mason
In-Reply-To: <20190704130336.GN3402@hirez.programming.kicks-ass.net>
On Thu, Jul 04, 2019 at 03:03:36PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 03, 2019 at 08:14:30PM -0700, Kris Van Hees wrote:
> > +/*
> > + * Read the data_head offset from the header page of the ring buffer. The
> > + * argument is declared 'volatile' because it references a memory mapped page
> > + * that the kernel may be writing to while we access it here.
> > + */
> > +static u64 read_rb_head(volatile struct perf_event_mmap_page *rb_page)
> > +{
> > + u64 head = rb_page->data_head;
> > +
> > + asm volatile("" ::: "memory");
> > +
> > + return head;
> > +}
> > +
> > +/*
> > + * Write the data_tail offset in the header page of the ring buffer. The
> > + * argument is declared 'volatile' because it references a memory mapped page
> > + * that the kernel may be writing to while we access it here.
>
> s/writing/reading/
Thanks!
> > + */
> > +static void write_rb_tail(volatile struct perf_event_mmap_page *rb_page,
> > + u64 tail)
> > +{
> > + asm volatile("" ::: "memory");
> > +
> > + rb_page->data_tail = tail;
> > +}
>
> That volatile usage is atrocious (kernel style would have you use
> {READ,WRITE}_ONCE()). Also your comments fail to mark these as
> load_acquire and store_release. And by only using a compiler barrier
> you're hard assuming TSO, which is somewhat fragile at best.
>
> Alternatively, you can use the C11 bits and write:
>
> return __atomic_load_n(&rb_page->data_head, __ATOMIC_ACQUIRE);
>
> __atomic_store_n(&rb_page->data_tail, tail, __ATOMIC_RELEASE);
Perhaps I should just use ring_buffer_read_head() and ring_buffer_write_tail()
since they are provided in tools/include/linux/ring_buffer.h? I expect that
would be even more preferable over __atomic_load_n() and __atomic_store_n()?
> > +/*
> > + * Process and output the probe data at the supplied address.
> > + */
> > +static int output_event(int cpu, u64 *buf)
> > +{
> > + u8 *data = (u8 *)buf;
> > + struct perf_event_header *hdr;
> > +
> > + hdr = (struct perf_event_header *)data;
> > + data += sizeof(struct perf_event_header);
> > +
> > + if (hdr->type == PERF_RECORD_SAMPLE) {
> > + u8 *ptr = data;
> > + u32 i, size, probe_id;
> > +
> > + /*
> > + * struct {
> > + * struct perf_event_header header;
> > + * u32 size;
> > + * u32 probe_id;
> > + * u32 gap;
> > + * u64 data[n];
> > + * }
> > + * and data points to the 'size' member at this point.
> > + */
> > + if (ptr > (u8 *)buf + hdr->size) {
> > + fprintf(stderr, "BAD: corrupted sample header\n");
> > + goto out;
> > + }
> > +
> > + size = *(u32 *)data;
> > + data += sizeof(size);
> > + ptr += sizeof(size) + size;
> > + if (ptr != (u8 *)buf + hdr->size) {
> > + fprintf(stderr, "BAD: invalid sample size\n");
> > + goto out;
> > + }
> > +
> > + probe_id = *(u32 *)data;
> > + data += sizeof(probe_id);
> > + size -= sizeof(probe_id);
> > + data += sizeof(u32); /* skip 32-bit gap */
> > + size -= sizeof(u32);
> > + buf = (u64 *)data;
> > +
> > + printf("%3d %6d ", cpu, probe_id);
> > + for (i = 0, size /= sizeof(u64); i < size; i++)
> > + printf("%#016lx ", buf[i]);
> > + printf("\n");
> > + } else if (hdr->type == PERF_RECORD_LOST) {
> > + u64 lost;
> > +
> > + /*
> > + * struct {
> > + * struct perf_event_header header;
> > + * u64 id;
> > + * u64 lost;
> > + * }
> > + * and data points to the 'id' member at this point.
> > + */
> > + lost = *(u64 *)(data + sizeof(u64));
> > +
> > + printf("[%ld probes dropped]\n", lost);
> > + } else
> > + fprintf(stderr, "UNKNOWN: record type %d\n", hdr->type);
> > +
> > +out:
> > + return hdr->size;
> > +}
>
> I see a distinct lack of wrapping support. AFAICT when buf+hdr->size
> wraps you're doing out-of-bounds accesses.
Yes, that is correct. I'm actually trying to figure out why it didn't actually
cause a SEGV when I tested this because I'm clearly reading past the end of
the mmap'd memory. Thank you for noticing this - I was trying to be too
minimal in the code I was putting out and really didn't pay attention to this.
Fixed in the V2 I am preparing.
> > +/*
> > + * Process the available probe data in the given buffer.
> > + */
> > +static void process_data(struct dtrace_buffer *buf)
> > +{
> > + /* This is volatile because the kernel may be updating the content. */
> > + volatile struct perf_event_mmap_page *rb_page = buf->base;
> > + u8 *base = (u8 *)buf->base +
> > + buf->page_size;
> > + u64 head = read_rb_head(rb_page);
> > +
> > + while (rb_page->data_tail != head) {
> > + u64 tail = rb_page->data_tail;
> > + u64 *ptr = (u64 *)(base + tail % buf->data_size);
> > + int len;
> > +
> > + len = output_event(buf->cpu, ptr);
> > +
> > + write_rb_tail(rb_page, tail + len);
> > + head = read_rb_head(rb_page);
> > + }
> > +}
>
> more volatile yuck.
>
> Also:
>
> for (;;) {
> head = __atomic_load_n(&rb_page->data_head, __ATOMIC_ACQUIRE);
> tail = __atomic_load_n(&rb_page->data_tail, __ATOMIC_RELAXED);
>
> if (head == tail)
> break;
>
> do {
> hdr = buf->base + (tail & ((1UL << buf->data_shift) - 1));
> if ((tail >> buf->data_shift) !=
> ((tail + hdr->size) >> buf->data_shift))
> /* handle wrap case */
> else
> /* normal case */
>
> tail += hdr->size;
> } while (tail != head);
>
> __atomic_store_n(&rb_page->data_tail, tail, __ATOMIC_RELEASE);
> }
>
> Or something.
Thank you for this suggestion. As mentioned above, I lean towards using the
provided ring_buffer_(read_head,write_tail) implementations since that is the
'other end' of the ring buffer head/tail mechanism that is going to be kept
in sync with any changes that might happen on the kernel side, right?
> > +/*
> > + * Wait for data to become available in any of the buffers.
> > + */
> > +int dt_buffer_poll(int epoll_fd, int timeout)
> > +{
> > + struct epoll_event events[dt_numcpus];
> > + int i, cnt;
> > +
> > + cnt = epoll_wait(epoll_fd, events, dt_numcpus, timeout);
> > + if (cnt < 0)
> > + return -errno;
> > +
> > + for (i = 0; i < cnt; i++)
> > + process_data((struct dtrace_buffer *)events[i].data.ptr);
> > +
> > + return cnt;
> > +}
>
> Or make sure to read on the CPU by having a poll thread per CPU, then
> you can do away with the memory barriers.
That is definitely something for the todo list for future optimizations.
Thanks for your review and code suggestions.
Kris
^ permalink raw reply
* Re: [PATCH bpf-next v3 5/6] tools/libbpf_probes: Add new devmap_hash type
From: Yonghong Song @ 2019-07-08 16:48 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, Daniel Borkmann
Cc: Alexei Starovoitov, netdev@vger.kernel.org, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156258334745.1664.1686759894096070590.stgit@alrua-x1>
On 7/8/19 3:55 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> This adds the definition for BPF_MAP_TYPE_DEVMAP_HASH to libbpf_probes.c in
> tools/lib/bpf.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> tools/lib/bpf/libbpf_probes.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index ace1a0708d99..4b0b0364f5fc 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -244,6 +244,7 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
> case BPF_MAP_TYPE_ARRAY_OF_MAPS:
> case BPF_MAP_TYPE_HASH_OF_MAPS:
> case BPF_MAP_TYPE_DEVMAP:
> + case BPF_MAP_TYPE_DEVMAP_HASH:
> case BPF_MAP_TYPE_SOCKMAP:
> case BPF_MAP_TYPE_CPUMAP:
> case BPF_MAP_TYPE_XSKMAP:
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 4/6] tools/include/uapi: Add devmap_hash BPF map type
From: Yonghong Song @ 2019-07-08 16:47 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, Daniel Borkmann
Cc: Alexei Starovoitov, netdev@vger.kernel.org, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156258334740.1664.18295003114988159871.stgit@alrua-x1>
On 7/8/19 3:55 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> This adds the devmap_hash BPF map type to the uapi headers in tools/.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> tools/include/uapi/linux/bpf.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index cecf42c871d4..8afaa0a19c67 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -134,6 +134,7 @@ enum bpf_map_type {
> BPF_MAP_TYPE_QUEUE,
> BPF_MAP_TYPE_STACK,
> BPF_MAP_TYPE_SK_STORAGE,
> + BPF_MAP_TYPE_DEVMAP_HASH,
> };
>
> /* Note that tracing related programs such as
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/6] xdp: Add devmap_hash map type for looking up devices by hashed index
From: Yonghong Song @ 2019-07-08 16:47 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, Daniel Borkmann
Cc: Alexei Starovoitov, netdev@vger.kernel.org, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156258334732.1664.10214955962271992722.stgit@alrua-x1>
On 7/8/19 3:55 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> A common pattern when using xdp_redirect_map() is to create a device map
> where the lookup key is simply ifindex. Because device maps are arrays,
> this leaves holes in the map, and the map has to be sized to fit the
> largest ifindex, regardless of how many devices actually are actually
> needed in the map.
>
> This patch adds a second type of device map where the key is looked up
> using a hashmap, instead of being used as an array index. This allows maps
> to be densely packed, so they can be smaller.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> include/linux/bpf.h | 7 ++
> include/linux/bpf_types.h | 1
> include/trace/events/xdp.h | 3 -
> include/uapi/linux/bpf.h | 1
> kernel/bpf/devmap.c | 194 ++++++++++++++++++++++++++++++++++++++++++++
> kernel/bpf/verifier.c | 2
> net/core/filter.c | 9 ++
> 7 files changed, 214 insertions(+), 3 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 net-next 1/3] tc-testing: Add JSON verification to tdc
From: Lucas Bates @ 2019-07-08 16:48 UTC (permalink / raw)
To: Alexander Aring
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner, Vlad Buslov,
Davide Caratti, kernel
In-Reply-To: <20190704202130.tv2ivy5tjj7pjasj@x220t>
On Thu, Jul 4, 2019 at 4:21 PM Alexander Aring <aring@mojatatu.com> wrote:
> why you just use eval() as pattern matching operation and let the user
> define how to declare a matching mechanism instead you introduce another
> static matching scheme based on a json description?
>
> Whereas in eval() you could directly use the python bool expression
> parser to make whatever you want.
>
> I don't know, I see at some points you will hit limitations what you can
> express with this matchFOO and we need to introduce another matchBAR,
> whereas in providing the code it should be no problem expression
> anything. If you want smaller shortcuts writing matching patterns you
> can implement them and using in your eval() operation.
Regarding hitting limitations: quite possibly, yes.
Using eval() to provide code for matching is going to put more of a
dependency on the test writer knowing Python. I know it's not a
terribly difficult language to pick up, but it's still setting a
higher barrier to entry. This is the primary reason I scrapped the
work I had presented at Netdev 1.2 in Tokyo, where all the tests were
coded using Python's unittest framework - I want to be sure it's as
easy as possible for people to use tdc and write tests for it.
Unless I'm off-base here?
Lucas
^ permalink raw reply
* Re: [bpf-next v3 09/12] bpf: Split out some helper functions
From: Krzesimir Nowak @ 2019-07-08 16:40 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies
In-Reply-To: <20190708163121.18477-10-krzesimir@kinvolk.io>
On Mon, Jul 8, 2019 at 6:31 PM Krzesimir Nowak <krzesimir@kinvolk.io> wrote:
>
> The moved functions are generally useful for implementing
> bpf_prog_test_run for other types of BPF programs - they don't have
> any network-specific stuff in them, so I can use them in a test run
> implementation for perf event BPF program too.
>
> Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
This patch has some whitespace issues (indentation with spaces instead
of tabs in some places), will fix them for v4. Sorry about that.
> ---
> include/linux/bpf.h | 28 +++++
> kernel/bpf/Makefile | 1 +
> kernel/bpf/test_run.c | 212 ++++++++++++++++++++++++++++++++++
> net/bpf/test_run.c | 263 +++++++++++-------------------------------
> 4 files changed, 308 insertions(+), 196 deletions(-)
> create mode 100644 kernel/bpf/test_run.c
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 18f4cc2c6acd..28db8ba57bc3 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1143,4 +1143,32 @@ static inline u32 bpf_xdp_sock_convert_ctx_access(enum bpf_access_type type,
> }
> #endif /* CONFIG_INET */
>
> +/* Helper functions for bpf_prog_test_run implementations */
> +typedef u32 bpf_prog_run_helper_t(struct bpf_prog *prog, void *ctx,
> + void *private_data);
> +
> +enum bpf_test_run_flags {
> + BPF_TEST_RUN_PLAIN = 0,
> + BPF_TEST_RUN_SETUP_CGROUP_STORAGE = 1 << 0,
> +};
> +
> +int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
> + u32 *retval, u32 *duration);
> +
> +int bpf_test_run_cb(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
> + bpf_prog_run_helper_t run_prog, void *private_data,
> + u32 *retval, u32 *duration);
> +
> +int bpf_test_finish(union bpf_attr __user *uattr, u32 retval, u32 duration);
> +
> +void *bpf_receive_ctx(const union bpf_attr *kattr, u32 max_size);
> +
> +int bpf_send_ctx(const union bpf_attr *kattr, union bpf_attr __user *uattr,
> + const void *data, u32 size);
> +
> +void *bpf_receive_data(const union bpf_attr *kattr, u32 max_size);
> +
> +int bpf_send_data(const union bpf_attr *kattr, union bpf_attr __user *uattr,
> + const void *data, u32 size);
> +
> #endif /* _LINUX_BPF_H */
> diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
> index 29d781061cd5..570fd40288f4 100644
> --- a/kernel/bpf/Makefile
> +++ b/kernel/bpf/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_CGROUP_BPF) += cgroup.o
> ifeq ($(CONFIG_INET),y)
> obj-$(CONFIG_BPF_SYSCALL) += reuseport_array.o
> endif
> +obj-$(CONFIG_BPF_SYSCALL) += test_run.o
> diff --git a/kernel/bpf/test_run.c b/kernel/bpf/test_run.c
> new file mode 100644
> index 000000000000..0481373da8be
> --- /dev/null
> +++ b/kernel/bpf/test_run.c
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2017 Facebook
> + * Copyright (c) 2019 Tigera, Inc
> + */
> +
> +#include <asm/div64.h>
> +
> +#include <linux/bpf-cgroup.h>
> +#include <linux/bpf.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/filter.h>
> +#include <linux/gfp.h>
> +#include <linux/kernel.h>
> +#include <linux/limits.h>
> +#include <linux/preempt.h>
> +#include <linux/rcupdate.h>
> +#include <linux/sched.h>
> +#include <linux/sched/signal.h>
> +#include <linux/slab.h>
> +#include <linux/timekeeping.h>
> +#include <linux/uaccess.h>
> +
> +static void teardown_cgroup_storage(struct bpf_cgroup_storage **storage)
> +{
> + enum bpf_cgroup_storage_type stype;
> +
> + if (!storage)
> + return;
> + for_each_cgroup_storage_type(stype)
> + bpf_cgroup_storage_free(storage[stype]);
> + kfree(storage);
> +}
> +
> +static struct bpf_cgroup_storage **setup_cgroup_storage(struct bpf_prog *prog)
> +{
> + enum bpf_cgroup_storage_type stype;
> + struct bpf_cgroup_storage **storage;
> + size_t size = MAX_BPF_CGROUP_STORAGE_TYPE;
> +
> + size *= sizeof(struct bpf_cgroup_storage *);
> + storage = kzalloc(size, GFP_KERNEL);
> + for_each_cgroup_storage_type(stype) {
> + storage[stype] = bpf_cgroup_storage_alloc(prog, stype);
> + if (IS_ERR(storage[stype])) {
> + storage[stype] = NULL;
> + teardown_cgroup_storage(storage);
> + return ERR_PTR(-ENOMEM);
> + }
> + }
> + return storage;
> +}
> +
> +static u32 run_bpf_prog(struct bpf_prog *prog, void *ctx, void *private_data)
> +{
> + return BPF_PROG_RUN(prog, ctx);
> +}
> +
> +int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
> + u32 *retval, u32 *duration)
> +{
> + return bpf_test_run_cb(prog, ctx, repeat, flags, run_bpf_prog, NULL,
> + retval, duration);
> +}
> +
> +int bpf_test_run_cb(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
> + bpf_prog_run_helper_t run_prog, void *private_data,
> + u32 *retval, u32 *duration)
> +{
> + struct bpf_cgroup_storage **storage = NULL;
> + u64 time_start, time_spent = 0;
> + int ret = 0;
> + u32 i;
> +
> + if (flags & BPF_TEST_RUN_SETUP_CGROUP_STORAGE) {
> + storage = setup_cgroup_storage(prog);
> + if (IS_ERR(storage))
> + return PTR_ERR(storage);
> + }
> +
> + if (!repeat)
> + repeat = 1;
> +
> + rcu_read_lock();
> + preempt_disable();
> + time_start = ktime_get_ns();
> + for (i = 0; i < repeat; i++) {
> + if (storage)
> + bpf_cgroup_storage_set(storage);
> + *retval = run_prog(prog, ctx, private_data);
> +
> + if (signal_pending(current)) {
> + preempt_enable();
> + rcu_read_unlock();
> + teardown_cgroup_storage(storage);
> + return -EINTR;
> + }
> +
> + if (need_resched()) {
> + time_spent += ktime_get_ns() - time_start;
> + preempt_enable();
> + rcu_read_unlock();
> +
> + cond_resched();
> +
> + rcu_read_lock();
> + preempt_disable();
> + time_start = ktime_get_ns();
> + }
> + }
> + time_spent += ktime_get_ns() - time_start;
> + preempt_enable();
> + rcu_read_unlock();
> +
> + do_div(time_spent, repeat);
> + *duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> +
> + teardown_cgroup_storage(storage);
> +
> + return ret;
> +}
> +
> +int bpf_test_finish(union bpf_attr __user *uattr, u32 retval, u32 duration)
> +{
> + if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval)))
> + return -EFAULT;
> + if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration)))
> + return -EFAULT;
> + return 0;
> +}
> +
> +static void *bpf_receive_mem(u64 in, u32 in_size, u32 max_size)
> +{
> + void __user *data_in = u64_to_user_ptr(in);
> + void *data;
> + int err;
> +
> + if (!data_in && in_size)
> + return ERR_PTR(-EINVAL);
> + data = kzalloc(max_size, GFP_USER);
> + if (!data)
> + return ERR_PTR(-ENOMEM);
> +
> + if (data_in) {
> + err = bpf_check_uarg_tail_zero(data_in, max_size, in_size);
> + if (err) {
> + kfree(data);
> + return ERR_PTR(err);
> + }
> +
> + in_size = min_t(u32, max_size, in_size);
> + if (copy_from_user(data, data_in, in_size)) {
> + kfree(data);
> + return ERR_PTR(-EFAULT);
> + }
> + }
> + return data;
> +}
> +
> +static int bpf_send_mem(u64 out, u32 out_size, u32 *out_size_write,
> + const void *data, u32 data_size)
> +{
> + void __user *data_out = u64_to_user_ptr(out);
> + int err = -EFAULT;
> + u32 copy_size = data_size;
> +
> + if (!data_out && out_size)
> + return -EINVAL;
> +
> + if (!data || !data_out)
> + return 0;
> +
> + if (copy_size > out_size) {
> + copy_size = out_size;
> + err = -ENOSPC;
> + }
> +
> + if (copy_to_user(data_out, data, copy_size))
> + goto out;
> + if (copy_to_user(out_size_write, &data_size, sizeof(data_size)))
> + goto out;
> + if (err != -ENOSPC)
> + err = 0;
> +out:
> + return err;
> +}
> +
> +void *bpf_receive_data(const union bpf_attr *kattr, u32 max_size)
> +{
> + return bpf_receive_mem(kattr->test.data_in, kattr->test.data_size_in,
> + max_size);
> +}
> +
> +int bpf_send_data(const union bpf_attr *kattr, union bpf_attr __user *uattr,
> + const void *data, u32 size)
> +{
> + return bpf_send_mem(kattr->test.data_out, kattr->test.data_size_out,
> + &uattr->test.data_size_out, data, size);
> +}
> +
> +void *bpf_receive_ctx(const union bpf_attr *kattr, u32 max_size)
> +{
> + return bpf_receive_mem(kattr->test.ctx_in, kattr->test.ctx_size_in,
> + max_size);
> +}
> +
> +int bpf_send_ctx(const union bpf_attr *kattr, union bpf_attr __user *uattr,
> + const void *data, u32 size)
> +{
> + return bpf_send_mem(kattr->test.ctx_out, kattr->test.ctx_size_out,
> + &uattr->test.ctx_size_out, data, size);
> +}
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 80e6f3a6864d..fe6b7b1af0cc 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -14,97 +14,6 @@
> #define CREATE_TRACE_POINTS
> #include <trace/events/bpf_test_run.h>
>
> -static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
> - u32 *retval, u32 *time)
> -{
> - struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { NULL };
> - enum bpf_cgroup_storage_type stype;
> - u64 time_start, time_spent = 0;
> - int ret = 0;
> - u32 i;
> -
> - for_each_cgroup_storage_type(stype) {
> - storage[stype] = bpf_cgroup_storage_alloc(prog, stype);
> - if (IS_ERR(storage[stype])) {
> - storage[stype] = NULL;
> - for_each_cgroup_storage_type(stype)
> - bpf_cgroup_storage_free(storage[stype]);
> - return -ENOMEM;
> - }
> - }
> -
> - if (!repeat)
> - repeat = 1;
> -
> - rcu_read_lock();
> - preempt_disable();
> - time_start = ktime_get_ns();
> - for (i = 0; i < repeat; i++) {
> - bpf_cgroup_storage_set(storage);
> - *retval = BPF_PROG_RUN(prog, ctx);
> -
> - if (signal_pending(current)) {
> - ret = -EINTR;
> - break;
> - }
> -
> - if (need_resched()) {
> - time_spent += ktime_get_ns() - time_start;
> - preempt_enable();
> - rcu_read_unlock();
> -
> - cond_resched();
> -
> - rcu_read_lock();
> - preempt_disable();
> - time_start = ktime_get_ns();
> - }
> - }
> - time_spent += ktime_get_ns() - time_start;
> - preempt_enable();
> - rcu_read_unlock();
> -
> - do_div(time_spent, repeat);
> - *time = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> -
> - for_each_cgroup_storage_type(stype)
> - bpf_cgroup_storage_free(storage[stype]);
> -
> - return ret;
> -}
> -
> -static int bpf_test_finish(const union bpf_attr *kattr,
> - union bpf_attr __user *uattr, const void *data,
> - u32 size, u32 retval, u32 duration)
> -{
> - void __user *data_out = u64_to_user_ptr(kattr->test.data_out);
> - int err = -EFAULT;
> - u32 copy_size = size;
> -
> - /* Clamp copy if the user has provided a size hint, but copy the full
> - * buffer if not to retain old behaviour.
> - */
> - if (kattr->test.data_size_out &&
> - copy_size > kattr->test.data_size_out) {
> - copy_size = kattr->test.data_size_out;
> - err = -ENOSPC;
> - }
> -
> - if (data_out && copy_to_user(data_out, data, copy_size))
> - goto out;
> - if (copy_to_user(&uattr->test.data_size_out, &size, sizeof(size)))
> - goto out;
> - if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval)))
> - goto out;
> - if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration)))
> - goto out;
> - if (err != -ENOSPC)
> - err = 0;
> -out:
> - trace_bpf_test_finish(&err);
> - return err;
> -}
> -
> static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
> u32 headroom, u32 tailroom)
> {
> @@ -125,63 +34,6 @@ static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
> return data;
> }
>
> -static void *bpf_ctx_init(const union bpf_attr *kattr, u32 max_size)
> -{
> - void __user *data_in = u64_to_user_ptr(kattr->test.ctx_in);
> - void __user *data_out = u64_to_user_ptr(kattr->test.ctx_out);
> - u32 size = kattr->test.ctx_size_in;
> - void *data;
> - int err;
> -
> - if (!data_in && !data_out)
> - return NULL;
> -
> - data = kzalloc(max_size, GFP_USER);
> - if (!data)
> - return ERR_PTR(-ENOMEM);
> -
> - if (data_in) {
> - err = bpf_check_uarg_tail_zero(data_in, max_size, size);
> - if (err) {
> - kfree(data);
> - return ERR_PTR(err);
> - }
> -
> - size = min_t(u32, max_size, size);
> - if (copy_from_user(data, data_in, size)) {
> - kfree(data);
> - return ERR_PTR(-EFAULT);
> - }
> - }
> - return data;
> -}
> -
> -static int bpf_ctx_finish(const union bpf_attr *kattr,
> - union bpf_attr __user *uattr, const void *data,
> - u32 size)
> -{
> - void __user *data_out = u64_to_user_ptr(kattr->test.ctx_out);
> - int err = -EFAULT;
> - u32 copy_size = size;
> -
> - if (!data || !data_out)
> - return 0;
> -
> - if (copy_size > kattr->test.ctx_size_out) {
> - copy_size = kattr->test.ctx_size_out;
> - err = -ENOSPC;
> - }
> -
> - if (copy_to_user(data_out, data, copy_size))
> - goto out;
> - if (copy_to_user(&uattr->test.ctx_size_out, &size, sizeof(size)))
> - goto out;
> - if (err != -ENOSPC)
> - err = 0;
> -out:
> - return err;
> -}
> -
> /**
> * range_is_zero - test whether buffer is initialized
> * @buf: buffer to check
> @@ -238,6 +90,36 @@ static void convert_skb_to___skb(struct sk_buff *skb, struct __sk_buff *__skb)
> memcpy(__skb->cb, &cb->data, QDISC_CB_PRIV_LEN);
> }
>
> +static int bpf_net_prog_test_run_finish(const union bpf_attr *kattr,
> + union bpf_attr __user *uattr,
> + const void *data, u32 data_size,
> + const void *ctx, u32 ctx_size,
> + u32 retval, u32 duration)
> +{
> + int ret;
> + union bpf_attr fixed_kattr;
> + const union bpf_attr *kattr_ptr = kattr;
> +
> + /* Clamp copy (in bpf_send_mem) if the user has provided a
> + * size hint, but copy the full buffer if not to retain old
> + * behaviour.
> + */
> + if (!kattr->test.data_size_out && kattr->test.data_out) {
> + fixed_kattr = *kattr;
> + fixed_kattr.test.data_size_out = U32_MAX;
> + kattr_ptr = &fixed_kattr;
> + }
> +
> + ret = bpf_send_data(kattr_ptr, uattr, data, data_size);
> + if (!ret) {
> + ret = bpf_test_finish(uattr, retval, duration);
> + if (!ret && ctx)
> + ret = bpf_send_ctx(kattr_ptr, uattr, ctx, ctx_size);
> + }
> + trace_bpf_test_finish(&ret);
> + return ret;
> +}
> +
> int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> union bpf_attr __user *uattr)
> {
> @@ -257,7 +139,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> if (IS_ERR(data))
> return PTR_ERR(data);
>
> - ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff));
> + ctx = bpf_receive_ctx(kattr, sizeof(struct __sk_buff));
> if (IS_ERR(ctx)) {
> kfree(data);
> return PTR_ERR(ctx);
> @@ -307,7 +189,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> ret = convert___skb_to_skb(skb, ctx);
> if (ret)
> goto out;
> - ret = bpf_test_run(prog, skb, repeat, &retval, &duration);
> + ret = bpf_test_run(prog, skb, repeat, BPF_TEST_RUN_SETUP_CGROUP_STORAGE,
> + &retval, &duration);
> if (ret)
> goto out;
> if (!is_l2) {
> @@ -327,10 +210,9 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> /* bpf program can never convert linear skb to non-linear */
> if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
> size = skb_headlen(skb);
> - ret = bpf_test_finish(kattr, uattr, skb->data, size, retval, duration);
> - if (!ret)
> - ret = bpf_ctx_finish(kattr, uattr, ctx,
> - sizeof(struct __sk_buff));
> + ret = bpf_net_prog_test_run_finish(kattr, uattr, skb->data, size,
> + ctx, sizeof(struct __sk_buff),
> + retval, duration);
> out:
> kfree_skb(skb);
> bpf_sk_storage_free(sk);
> @@ -365,32 +247,48 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
> rxqueue = __netif_get_rx_queue(current->nsproxy->net_ns->loopback_dev, 0);
> xdp.rxq = &rxqueue->xdp_rxq;
>
> - ret = bpf_test_run(prog, &xdp, repeat, &retval, &duration);
> + ret = bpf_test_run(prog, &xdp, repeat,
> + BPF_TEST_RUN_SETUP_CGROUP_STORAGE,
> + &retval, &duration);
> if (ret)
> goto out;
> if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN ||
> xdp.data_end != xdp.data + size)
> size = xdp.data_end - xdp.data;
> - ret = bpf_test_finish(kattr, uattr, xdp.data, size, retval, duration);
> + ret = bpf_net_prog_test_run_finish(kattr, uattr, xdp.data, size,
> + NULL, 0, retval, duration);
> out:
> kfree(data);
> return ret;
> }
>
> +struct bpf_flow_dissect_run_data {
> + __be16 proto;
> + int nhoff;
> + int hlen;
> +};
> +
> +static u32 bpf_flow_dissect_run(struct bpf_prog *prog, void *ctx,
> + void *private_data)
> +{
> + struct bpf_flow_dissect_run_data *data = private_data;
> +
> + return bpf_flow_dissect(prog, ctx, data->proto, data->nhoff, data->hlen);
> +}
> +
> int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> const union bpf_attr *kattr,
> union bpf_attr __user *uattr)
> {
> + struct bpf_flow_dissect_run_data run_data = {};
> u32 size = kattr->test.data_size_in;
> struct bpf_flow_dissector ctx = {};
> u32 repeat = kattr->test.repeat;
> struct bpf_flow_keys flow_keys;
> - u64 time_start, time_spent = 0;
> const struct ethhdr *eth;
> u32 retval, duration;
> void *data;
> int ret;
> - u32 i;
>
> if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR)
> return -EINVAL;
> @@ -407,49 +305,22 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
>
> eth = (struct ethhdr *)data;
>
> - if (!repeat)
> - repeat = 1;
> -
> ctx.flow_keys = &flow_keys;
> ctx.data = data;
> ctx.data_end = (__u8 *)data + size;
>
> - rcu_read_lock();
> - preempt_disable();
> - time_start = ktime_get_ns();
> - for (i = 0; i < repeat; i++) {
> - retval = bpf_flow_dissect(prog, &ctx, eth->h_proto, ETH_HLEN,
> - size);
> -
> - if (signal_pending(current)) {
> - preempt_enable();
> - rcu_read_unlock();
> -
> - ret = -EINTR;
> - goto out;
> - }
> -
> - if (need_resched()) {
> - time_spent += ktime_get_ns() - time_start;
> - preempt_enable();
> - rcu_read_unlock();
> -
> - cond_resched();
> -
> - rcu_read_lock();
> - preempt_disable();
> - time_start = ktime_get_ns();
> - }
> - }
> - time_spent += ktime_get_ns() - time_start;
> - preempt_enable();
> - rcu_read_unlock();
> -
> - do_div(time_spent, repeat);
> - duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> + run_data.proto = eth->h_proto;
> + run_data.nhoff = ETH_HLEN;
> + run_data.hlen = size;
> + ret = bpf_test_run_cb(prog, &ctx, repeat, BPF_TEST_RUN_PLAIN,
> + bpf_flow_dissect_run, &run_data,
> + &retval, &duration);
> + if (!ret)
> + goto out;
>
> - ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys),
> - retval, duration);
> + ret = bpf_net_prog_test_run_finish(kattr, uattr, &flow_keys,
> + sizeof(flow_keys), NULL, 0,
> + retval, duration);
>
> out:
> kfree(data);
> --
> 2.20.1
>
--
Kinvolk GmbH | Adalbertstr.6a, 10999 Berlin | tel: +491755589364
Geschäftsführer/Directors: Alban Crequy, Chris Kühl, Iago López Galeiras
Registergericht/Court of registration: Amtsgericht Charlottenburg
Registernummer/Registration number: HRB 171414 B
Ust-ID-Nummer/VAT ID number: DE302207000
^ 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