Netdev List
 help / color / mirror / Atom feed
* Re: [net-next 00/13][pull request] Intel Wired LAN Doc Updates 2018-10-18
From: David Miller @ 2018-10-18 22:46 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann
In-Reply-To: <20181018203259.4175-1-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 18 Oct 2018 13:32:46 -0700

> This series contains documentation fixes and updates for Intel wired LAN
> drivers.
> 
> The following was done:
>  - Updated incorrect URLs
>  - removed document references which did not apply to the current
>    in-kernel drivers
>  - added documentation for fm10k driver
>  - added missing documentation on existing or new features
>  - added SPDX headers to all the documentation files
> 
> Lastly, the documentation was converted over to the RST (reStructured
> Text) format, so that 'make htmldocs' produces pretty html driver
> documentation for our drivers.
> 
> The following are changes since commit 3a3295bfa6f484bba91de0a804c1d0bf7d31dbd3:
>   Merge branch 'sctp-fix-sk_wmem_queued-and-use-it-to-check-for-writable-space'
> and are available in the git repository at:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 10GbE

Pulled, thanks Jeff.

^ permalink raw reply

* [PATCH v4.14-stable] sch_netem: restore skb->dev after dequeuing from the rbtree
From: Christoph Paasch @ 2018-10-18 20:38 UTC (permalink / raw)
  To: netdev, stable, David Miller, gregkh
  Cc: Stephen Hemminger, Eric Dumazet, Soheil Hassas Yeganeh, Wei Wang,
	Willem de Bruijn

Upstream commit bffa72cf7f9d ("net: sk_buff rbnode reorg") got
backported as commit 6b921536f170 ("net: sk_buff rbnode reorg") into the
v4.14.x-tree.

However, the backport does not include the changes in sch_netem.c

We need these, as otherwise the skb->dev pointer is not set when
dequeueing from the netem rbtree, resulting in a panic:

[   15.427748] BUG: unable to handle kernel NULL pointer dereference at 00000000000000d0
[   15.428863] IP: netif_skb_features+0x24/0x230
[   15.429402] PGD 0 P4D 0
[   15.429733] Oops: 0000 [#1] SMP PTI
[   15.430169] Modules linked in:
[   15.430614] CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.14.77.mptcp #77
[   15.431497] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.5.1 01/01/2011
[   15.432568] task: ffff88042db19680 task.stack: ffffc90000070000
[   15.433356] RIP: 0010:netif_skb_features+0x24/0x230
[   15.433977] RSP: 0018:ffff88043fd83e70 EFLAGS: 00010286
[   15.434665] RAX: ffff880429ad80c0 RBX: ffff88042bd0e400 RCX: ffff880429ad8000
[   15.435585] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88042bd0e400
[   15.436551] RBP: ffff88042bd0e400 R08: ffff88042a4b6c9c R09: 0000000000000001
[   15.437485] R10: 0000000000000004 R11: 0000000000000000 R12: ffff88042c700000
[   15.438393] R13: ffff88042c700000 R14: ffff88042a4b6c00 R15: ffff88042c6bb000
[   15.439315] FS:  0000000000000000(0000) GS:ffff88043fd80000(0000) knlGS:0000000000000000
[   15.440314] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   15.441084] CR2: 00000000000000d0 CR3: 000000042c374000 CR4: 00000000000006e0
[   15.442016] Call Trace:
[   15.442333]  <IRQ>
[   15.442596]  validate_xmit_skb+0x17/0x270
[   15.443134]  validate_xmit_skb_list+0x38/0x60
[   15.443698]  sch_direct_xmit+0x102/0x190
[   15.444198]  __qdisc_run+0xe3/0x240
[   15.444671]  net_tx_action+0x121/0x140
[   15.445177]  __do_softirq+0xe2/0x224
[   15.445654]  irq_exit+0xbf/0xd0
[   15.446072]  smp_apic_timer_interrupt+0x5d/0x90
[   15.446654]  apic_timer_interrupt+0x7d/0x90
[   15.447185]  </IRQ>
[   15.447460] RIP: 0010:native_safe_halt+0x2/0x10
[   15.447992] RSP: 0018:ffffc90000073f10 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff10
[   15.449008] RAX: ffffffff816667d0 RBX: ffffffff820946b0 RCX: 0000000000000000
[   15.449895] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[   15.450768] RBP: ffffffff82026940 R08: 00000004e858e5e1 R09: ffff88042a4b6d58
[   15.451643] R10: 0000000000000000 R11: 000000d0d56879bb R12: 0000000000000000
[   15.452478] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[   15.453340]  ? __sched_text_end+0x2/0x2
[   15.453835]  default_idle+0xf/0x20
[   15.454259]  do_idle+0x170/0x200
[   15.454653]  cpu_startup_entry+0x14/0x20
[   15.455142]  secondary_startup_64+0xa5/0xb0
[   15.455715] Code: 1f 84 00 00 00 00 00 55 53 48 89 fd 48 83 ec 08 8b 87 bc 00 00 00 48 8b 8f c0 00 00 00 0f b6 97 81 00 00 00 48 8b 77 10 48 01 c8 <48> 8b 9
[   15.458138] RIP: netif_skb_features+0x24/0x230 RSP: ffff88043fd83e70
[   15.458933] CR2: 00000000000000d0
[   15.459352] ---[ end trace 083925903ae60570 ]---

Fixes: 6b921536f170 ("net: sk_buff rbnode reorg")
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Cc: Wei Wang <weiwan@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Signed-off-by: Christoph Paasch <cpaasch@apple.com>
---

Notes:
    This patch should only make it into v4.14-stable as that's the only branch where
    the offending commit has been backported to.

 net/sched/sch_netem.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 2a2ab6bfe5d8..3d325b840802 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -624,6 +624,10 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)
 			skb->next = NULL;
 			skb->prev = NULL;
 			skb->tstamp = netem_skb_cb(skb)->tstamp_save;
+			/* skb->dev shares skb->rbnode area,
+			 * we need to restore its value.
+			 */
+			skb->dev = qdisc_dev(sch);
 
 #ifdef CONFIG_NET_CLS_ACT
 			/*
-- 
2.16.2

^ permalink raw reply related

* Re: [PATCH 0/4] Add SOCFPGA System Manager
From: Lee Jones @ 2018-10-19  6:16 UTC (permalink / raw)
  To: Thor Thayer
  Cc: peppe.cavallaro, dinguyen, linux, alexandre.torgue, joabreu,
	davem, mchehab+samsung, catalin.marinas, akpm, arnd, aisheng.dong,
	linux-kernel, netdev, linux-arm-kernel
In-Reply-To: <a095071e-bd46-e440-fc01-7e2bbaf5964a@linux.intel.com>

On Wed, 17 Oct 2018, Thor Thayer wrote:


> On 10/10/2018 09:42 AM, Thor Thayer wrote:
> > Hi
> > On 09/24/2018 05:09 PM, thor.thayer@linux.intel.com wrote:
> > > From: Thor Thayer <thor.thayer@linux.intel.com>
> > > 
> > > Add MFD driver for ARM64 SOCFPGA System Manager to steer
> > > System Manager calls appropriately.
> > > The SOCFPGA System Manager includes registers from several
> > > SOC peripherals.
> > > 
> > > On ARM32, syscon handles this aggregated register grouping.
> > > Redirect System Manager calls to syscon for ARM32 SOCFPGA
> > > systems.
> > > 
> > > The ARM64 System Manager can only be accessed from priority
> > > level EL3 so this new MFD driver handles the calls to EL3.
> > > 
> > > Thor Thayer (4):
> > >    mfd: altera-sysmgr: Add SOCFPGA System Manager abstraction
> > >    ARM: socfpga_defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
> > >    arm64: defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
> > >    net: stmmac: socfpga: Convert to shared System Manager driver
> > > 
> > >   MAINTAINERS                                        |   6 +
> > >   arch/arm/configs/socfpga_defconfig                 |   1 +
> > >   arch/arm64/configs/defconfig                       |   1 +
> > >   drivers/mfd/Kconfig                                |   9 +
> > >   drivers/mfd/Makefile                               |   1 +
> > >   drivers/mfd/altera-sysmgr.c                        | 310
> > > +++++++++++++++++++++
> > >   .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c    |   4 +-
> > >   include/linux/mfd/altera-sysmgr.h                  | 113 ++++++++
> > >   8 files changed, 444 insertions(+), 1 deletion(-)
> > >   create mode 100644 drivers/mfd/altera-sysmgr.c
> > >   create mode 100644 include/linux/mfd/altera-sysmgr.h
> > > 
> > Gentle ping.
> 
> Gentle ping again...
> 
> Any comments on this patch series?

"Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.
Sending content free pings just adds to the mail volume (if they are
seen at all) and if something has gone wrong you'll have to resend the
patches anyway so resending with any comments addressed is generally a
much better approach."

In this case, the we are too late in the series to have these
applied.  Maintainers are generally preparing their submissions for
the merge-window.  The MFD component of this set is marked as "To
Review" and I will get around to it when time is more abundant.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
From: Steve Grubb @ 2018-10-18 22:09 UTC (permalink / raw)
  To: linux-audit
  Cc: Richard Guy Briggs, Jesper Dangaard Brouer, Daniel Borkmann,
	netdev, ast, acme, Jiri Olsa, Jiri Olsa, Alexei Starovoitov
In-Reply-To: <20181018195306.td5ijww3lk3knswx@madcap2.tricolour.ca>

On Thursday, October 18, 2018 3:53:06 PM EDT Richard Guy Briggs wrote:
> On 2018-10-07 18:19, Jesper Dangaard Brouer wrote:
> > On Sat, 6 Oct 2018 00:05:22 +0200
> > 
> > Jiri Olsa <jolsa@redhat.com> wrote:
> > > On Fri, Oct 05, 2018 at 11:44:35AM -0700, Alexei Starovoitov wrote:
> > > > On Fri, Oct 05, 2018 at 08:14:09AM +0200, Jiri Olsa wrote:
> > > > > On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> > > > > > On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer 
wrote:
> > > > > > > On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann 
<daniel@iogearbox.net> wrote:
> > > > > > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > > > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
<alexei.starovoitov@gmail.com> wrote:
> > > > > > > > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann 
wrote:
> > > > > > > [...]
> > > > > > > 
> > > > > > > > >> If the purpose of the patch is to give user space
> > > > > > > > >> visibility into
> > > > > > > > >> bpf prog load/unload as a notification, then I completely
> > > > > > > > >> agree that
> > > > > > > > >> some notification mechanism is necessary.
> > > > > > > > 
> > > > > > > > Yeah, I did only regard it as only that, nothing more. Some
> > > > > > > > means
> > > > > > > > of timeline and notification that can be kept in a record in
> > > > > > > > user
> > > > > > > > space and later retrieved e.g. for introspection on what has
> > > > > > > > been
> > > > > > > > loaded.
> > > > > > > > 
> > > > > > > > >> I've started working on such mechanism via perf ring
> > > > > > > > >> buffer which is
> > > > > > > > >> the fastest mechanism we have in the kernel so far.
> > > > > > > > >> See long discussion here:
> > > > > > > > >> https://patchwork.ozlabs.org/patch/971970/
> > 
> > [...]
> > 
> > > > > > > > That one is definitely needed in any case to resolve the
> > > > > > > > kallsyms
> > > > > > > > limitations, and it does have overlap in that in either case
> > > > > > > > we
> > > > > > > > want to look at past BPF programs that have been unloaded in
> > > > > > > > the
> > > > > > > > meantime, so I don't have a strong preference either way, and
> > > > > > > > the
> > > > > > > > former is needed in any case. Though thought was that audit
> > > > > > > > might
> > > > > > > > be an option for those not running profiling daemons 24/7,
> > > > > > > > but
> > > > > > > > presumably bpftool could be extended to record these events
> > > > > > > > as
> > > > > > > > well if we don't want to reuse audit infra.
> > > > > > > 
> > > > > > > Yes, exactly, I don't want to run a profiling daemon 24/7 to
> > > > > > > record
> > > > > > > these events.  I do acknowledge that this perf event is
> > > > > > > relevant,
> > > > > > > especially for catching the kernel symbols (I need that
> > > > > > > myself), but it
> > > > > > > does not cover my use-case.
> > > > > > > 
> > > > > > > My use-case is to 24/7 collect and keep records in userspace,
> > > > > > > and have a
> > > > > > > timeline of these notifications, for later retrieval.  The idea
> > > > > > > is that
> > > > > > > our support engineers can look at these records when
> > > > > > > troubleshooting
> > > > > > > the system.  And the plan is also to collect these records as
> > > > > > > part of
> > > > > > > our sosreport tool, which is part of the support case.
> > > > > > 
> > > > > > I don't think you're implying that prog load/unload should be
> > > > > > spamming dmesg and auditd not even running...
> > > > > 
> > > > > I think the problem Jesper implied is that in order to collect
> > > > > those logs you'll need perf tool running all the time.. which
> > > > > it's not equipped for yet
> > > > 
> > > > I'm not proposing to run 'perf' binary all the time.
> > > > Setting up perf ring buffer just for these new bpf prog load/unload
> > > > events and epolling it is simple enough to do from any application
> > > > including auditd. selftests/bpf/ do it for bpf output events.
> > > 
> > > ok, did not think about the possibility to teach auditd talk to perf,
> > > time to get that tool evsel/evlist/rb library ready ;-)
> > 
> > Interesting, I also didn't consider teaching auditd to gets its 'bpf'
> > events from a separate perf ring-buffer, that might work.  I do wonder
> > how the audit people will take this suggestion.

I'm not sure exactly what the issue is. You can audit for specific syscall 
and argument. So, if you want to see loads, then you can make a rule like:

-a always,exit -F arch=b64 -S bpf -F a0=5

-Steve

^ permalink raw reply

* RE: Issues in error queue polling
From: Keller, Jacob E @ 2018-10-18 21:54 UTC (permalink / raw)
  To: pasqualirb@gmail.com, netdev@vger.kernel.org
  Cc: Gomes, Vinicius, davem@davemloft.net
In-Reply-To: <20181018182644.GA11634@localhost.localdomain>

Hi,

> -----Original Message-----
> From: Ricardo Biehl Pasquali [mailto:pasqualirb@gmail.com]
> Sent: Thursday, October 18, 2018 11:27 AM
> To: netdev@vger.kernel.org
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; Gomes, Vinicius
> <vinicius.gomes@intel.com>; davem@davemloft.net
> Subject: Issues in error queue polling
> 
> The commit 7d4c04fc170087119727 ("net: add option to enable
> error queue packets waking select") (2013-03-28) introduced
> SO_SELECT_ERR_QUEUE, which masks POLLPRI with POLLERR event
> return in some socket poll callbacks.
> 
> POLLERR event issued with sock_queue_err_skb() did not wake
> up a poll when POLLERR is the only requested event because
> sk_data_ready() (sock_def_readable()) was used and it
> doesn't mask POLLERR in poll wake up:
> 

Right.

> wake_up_interruptible_sync_poll(&wq->wait,
>                                 EPOLLIN | EPOLLPRI |
>                                 EPOLLRDNORM | EPOLLRDBAND);
> 
> If POLLIN or POLLPRI are requested, for example, poll does
> wake up.
> 
> POLLERR wakeup by requesting POLLPRI is possible without
> set SO_SELECT_ERR_QUEUE. All the option does is masking
> POLLPRI as a returned event before poll returns. poll
> would return anyway because of POLLERR.
> 

Yes. The problem being that the application thread not being ready to handle POLLPRI, so they want to avoid the application waking up to an event.

> Also, the sentence "[...] enable software to wait on error
> queue packets without waking up for regular data on the
> socket." from the above commit is not true.
> 

Not entirely true but...

> A POLLIN event issued via sock_def_readable() wakes up
> threads waiting for POLLPRI, and vice versa. However,
> poll() does not return, sleeping again, as the requested
> events do not match events.
> 

The thread wakes up, but the application handling the events doesn't because the thread goes right back to sleep.

> The commit 6e5d58fdc9bedd0255a8 ("skbuff: Fix not waking
> applications when errors are enqueued") (2018-03-14) make
> POLLERR alone wake up poll. It replaces sk_data_ready()
> (sock_def_readable()) with sk_error_report()
> (sock_def_error_report()). This makes "POLLERR wake up by
> requesting POLLPRI" obsolete.
> 

Yep, this is a better solution, and I wish it had been thought of before we introduced SO_SELECT_ERR_QUEUE.

> Rationale:
> 
> POLLIN-only and POLLERR-only wake up are useful when there
> is a receiving thread, a sending thread, and a thread that
> get transmit timestamps. The thread polling on POLLERR will
> not wake up when regular data arrives (POLLIN). The thread
> polling on POLLIN will not wake up when tx timestamps are
> ready (POLLERR).

Right. This is the goal for applications like ptp4l.

> 
> One solution is adding an option that disable POLLERR as
> requested event. This is in the Virtual File System
> subsystem, not in the network, though.
> 
> This solves the problem of waking up other threads that
> not interested in error queue. Thus allowing a separate
> thread take care of error queue (useful for receiving
> transmit timestamps).

Yes, this makes sense to me.

Thanks,
Jake

^ permalink raw reply

* Re: Kernel oops with mlx5 and dual XDP redirect programs
From: Toke Høiland-Jørgensen @ 2018-10-18 21:53 UTC (permalink / raw)
  To: Saeed Mahameed, netdev@vger.kernel.org
  Cc: Eran Ben Elisha, Tariq Toukan, brouer@redhat.com
In-Reply-To: <4e2cfdc3db244f4b9483a0c3dfc62fae55238bb3.camel@mellanox.com>

Saeed Mahameed <saeedm@mellanox.com> writes:

> I think that the mlx5 driver doesn't know how to tell the other device
> to stop transmitting to it while it is resetting.. Maybe tariq or
> Jesper know more about this ?
> I will look at this tomorrow after noon and will try to repro...

Hi Saeed

Did you have a chance to poke at this? :)

-Toke

^ permalink raw reply

* [bpf-next PATCH 3/3] bpf: test_sockmap add options to use msg_push_data
From: John Fastabend @ 2018-10-18 21:52 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, john.fastabend
In-Reply-To: <20181018215041.22362.36585.stgit@john-Precision-Tower-5810>

Add options to run msg_push_data, this patch creates two more flags
in test_sockmap that can be used to specify the offset and length
of bytes to be added. The new options are --txmsg_start_push to
specify where bytes should be inserted and --txmsg_end_push to
specify how many bytes. This is analagous to the options that are
used to pull data, --txmsg_start and --txmsg_end.

In addition to adding the options tests are added to the test
suit to run the tests similar to what was done for msg_pull_data.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 tools/testing/selftests/bpf/test_sockmap.c      |   58 +++++++++++++-
 tools/testing/selftests/bpf/test_sockmap_kern.h |   97 ++++++++++++++++++-----
 2 files changed, 129 insertions(+), 26 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_sockmap.c b/tools/testing/selftests/bpf/test_sockmap.c
index cbd1c0b..622ade0 100644
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -77,6 +77,8 @@
 int txmsg_cork;
 int txmsg_start;
 int txmsg_end;
+int txmsg_start_push;
+int txmsg_end_push;
 int txmsg_ingress;
 int txmsg_skb;
 int ktls;
@@ -100,6 +102,8 @@
 	{"txmsg_cork",	required_argument,	NULL, 'k'},
 	{"txmsg_start", required_argument,	NULL, 's'},
 	{"txmsg_end",	required_argument,	NULL, 'e'},
+	{"txmsg_start_push", required_argument,	NULL, 'p'},
+	{"txmsg_end_push",   required_argument,	NULL, 'q'},
 	{"txmsg_ingress", no_argument,		&txmsg_ingress, 1 },
 	{"txmsg_skb", no_argument,		&txmsg_skb, 1 },
 	{"ktls", no_argument,			&ktls, 1 },
@@ -903,6 +907,30 @@ static int run_options(struct sockmap_options *options, int cg_fd,  int test)
 			}
 		}
 
+		if (txmsg_start_push) {
+			i = 2;
+			err = bpf_map_update_elem(map_fd[5],
+						  &i, &txmsg_start_push, BPF_ANY);
+			if (err) {
+				fprintf(stderr,
+					"ERROR: bpf_map_update_elem (txmsg_start_push):  %d (%s)\n",
+					err, strerror(errno));
+				goto out;
+			}
+		}
+
+		if (txmsg_end_push) {
+			i = 3;
+			err = bpf_map_update_elem(map_fd[5],
+						  &i, &txmsg_end_push, BPF_ANY);
+			if (err) {
+				fprintf(stderr,
+					"ERROR: bpf_map_update_elem %i@%i (txmsg_end_push):  %d (%s)\n",
+					txmsg_end_push, i, err, strerror(errno));
+				goto out;
+			}
+		}
+
 		if (txmsg_ingress) {
 			int in = BPF_F_INGRESS;
 
@@ -1235,6 +1263,8 @@ static int test_mixed(int cgrp)
 	txmsg_pass = txmsg_noisy = txmsg_redir_noisy = txmsg_drop = 0;
 	txmsg_apply = txmsg_cork = 0;
 	txmsg_start = txmsg_end = 0;
+	txmsg_start_push = txmsg_end_push = 0;
+
 	/* Test small and large iov_count values with pass/redir/apply/cork */
 	txmsg_pass = 1;
 	txmsg_redir = 0;
@@ -1351,6 +1381,8 @@ static int test_start_end(int cgrp)
 	/* Test basic start/end with lots of iov_count and iov_lengths */
 	txmsg_start = 1;
 	txmsg_end = 2;
+	txmsg_start_push = 1;
+	txmsg_end_push = 2;
 	err = test_txmsg(cgrp);
 	if (err)
 		goto out;
@@ -1364,6 +1396,8 @@ static int test_start_end(int cgrp)
 	for (i = 99; i <= 1600; i += 500) {
 		txmsg_start = 0;
 		txmsg_end = i;
+		txmsg_start_push = 0;
+		txmsg_end_push = i;
 		err = test_exec(cgrp, &opt);
 		if (err)
 			goto out;
@@ -1373,6 +1407,8 @@ static int test_start_end(int cgrp)
 	for (i = 199; i <= 1600; i += 500) {
 		txmsg_start = 100;
 		txmsg_end = i;
+		txmsg_start_push = 100;
+		txmsg_end_push = i;
 		err = test_exec(cgrp, &opt);
 		if (err)
 			goto out;
@@ -1381,6 +1417,8 @@ static int test_start_end(int cgrp)
 	/* Test start/end with cork pulling last sg entry */
 	txmsg_start = 1500;
 	txmsg_end = 1600;
+	txmsg_start_push = 1500;
+	txmsg_end_push = 1600;
 	err = test_exec(cgrp, &opt);
 	if (err)
 		goto out;
@@ -1388,6 +1426,8 @@ static int test_start_end(int cgrp)
 	/* Test start/end pull of single byte in last page */
 	txmsg_start = 1111;
 	txmsg_end = 1112;
+	txmsg_start_push = 1111;
+	txmsg_end_push = 1112;
 	err = test_exec(cgrp, &opt);
 	if (err)
 		goto out;
@@ -1395,6 +1435,8 @@ static int test_start_end(int cgrp)
 	/* Test start/end with end < start */
 	txmsg_start = 1111;
 	txmsg_end = 0;
+	txmsg_start_push = 1111;
+	txmsg_end_push = 0;
 	err = test_exec(cgrp, &opt);
 	if (err)
 		goto out;
@@ -1402,6 +1444,8 @@ static int test_start_end(int cgrp)
 	/* Test start/end with end > data */
 	txmsg_start = 0;
 	txmsg_end = 1601;
+	txmsg_start_push = 0;
+	txmsg_end_push = 1601;
 	err = test_exec(cgrp, &opt);
 	if (err)
 		goto out;
@@ -1409,6 +1453,8 @@ static int test_start_end(int cgrp)
 	/* Test start/end with start > data */
 	txmsg_start = 1601;
 	txmsg_end = 1600;
+	txmsg_start_push = 1601;
+	txmsg_end_push = 1600;
 	err = test_exec(cgrp, &opt);
 
 out:
@@ -1424,7 +1470,7 @@ static int test_start_end(int cgrp)
 	"sock_map_redir",
 	"sock_apply_bytes",
 	"sock_cork_bytes",
-	"sock_pull_bytes",
+	"sock_bytes",
 	"sock_redir_flags",
 	"sock_skb_opts",
 };
@@ -1531,7 +1577,7 @@ static int __test_suite(int cg_fd, char *bpf_file)
 	}
 
 	/* Tests basic commands and APIs with range of iov values */
-	txmsg_start = txmsg_end = 0;
+	txmsg_start = txmsg_end = txmsg_start_push = txmsg_end_push = 0;
 	err = test_txmsg(cg_fd);
 	if (err)
 		goto out;
@@ -1580,7 +1626,7 @@ int main(int argc, char **argv)
 	if (argc < 2)
 		return test_suite(-1);
 
-	while ((opt = getopt_long(argc, argv, ":dhvc:r:i:l:t:",
+	while ((opt = getopt_long(argc, argv, ":dhvc:r:i:l:t:p:q:",
 				  long_options, &longindex)) != -1) {
 		switch (opt) {
 		case 's':
@@ -1589,6 +1635,12 @@ int main(int argc, char **argv)
 		case 'e':
 			txmsg_end = atoi(optarg);
 			break;
+		case 'p':
+			txmsg_start_push = atoi(optarg);
+			break;
+		case 'q':
+			txmsg_end_push = atoi(optarg);
+			break;
 		case 'a':
 			txmsg_apply = atoi(optarg);
 			break;
diff --git a/tools/testing/selftests/bpf/test_sockmap_kern.h b/tools/testing/selftests/bpf/test_sockmap_kern.h
index 8e8e417..14b8bba 100644
--- a/tools/testing/selftests/bpf/test_sockmap_kern.h
+++ b/tools/testing/selftests/bpf/test_sockmap_kern.h
@@ -70,11 +70,11 @@ struct bpf_map_def SEC("maps") sock_cork_bytes = {
 	.max_entries = 1
 };
 
-struct bpf_map_def SEC("maps") sock_pull_bytes = {
+struct bpf_map_def SEC("maps") sock_bytes = {
 	.type = BPF_MAP_TYPE_ARRAY,
 	.key_size = sizeof(int),
 	.value_size = sizeof(int),
-	.max_entries = 2
+	.max_entries = 4
 };
 
 struct bpf_map_def SEC("maps") sock_redir_flags = {
@@ -181,8 +181,8 @@ int bpf_sockmap(struct bpf_sock_ops *skops)
 SEC("sk_msg1")
 int bpf_prog4(struct sk_msg_md *msg)
 {
-	int *bytes, zero = 0, one = 1;
-	int *start, *end;
+	int *bytes, zero = 0, one = 1, two = 2, three = 3;
+	int *start, *end, *start_push, *end_push;
 
 	bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
 	if (bytes)
@@ -190,18 +190,24 @@ int bpf_prog4(struct sk_msg_md *msg)
 	bytes = bpf_map_lookup_elem(&sock_cork_bytes, &zero);
 	if (bytes)
 		bpf_msg_cork_bytes(msg, *bytes);
-	start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
-	end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
+	start = bpf_map_lookup_elem(&sock_bytes, &zero);
+	end = bpf_map_lookup_elem(&sock_bytes, &one);
 	if (start && end)
 		bpf_msg_pull_data(msg, *start, *end, 0);
+	start_push = bpf_map_lookup_elem(&sock_bytes, &two);
+	end_push = bpf_map_lookup_elem(&sock_bytes, &three);
+	if (start_push && end_push)
+		bpf_msg_push_data(msg, *start_push, *end_push, 0);
 	return SK_PASS;
 }
 
 SEC("sk_msg2")
 int bpf_prog5(struct sk_msg_md *msg)
 {
-	int err1 = -1, err2 = -1, zero = 0, one = 1;
-	int *bytes, *start, *end, len1, len2;
+	int zero = 0, one = 1, two = 2, three = 3;
+	int *start, *end, *start_push, *end_push;
+	int *bytes, len1, len2 = 0, len3;
+	int err1 = -1, err2 = -1;
 
 	bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
 	if (bytes)
@@ -210,8 +216,8 @@ int bpf_prog5(struct sk_msg_md *msg)
 	if (bytes)
 		err2 = bpf_msg_cork_bytes(msg, *bytes);
 	len1 = (__u64)msg->data_end - (__u64)msg->data;
-	start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
-	end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
+	start = bpf_map_lookup_elem(&sock_bytes, &zero);
+	end = bpf_map_lookup_elem(&sock_bytes, &one);
 	if (start && end) {
 		int err;
 
@@ -225,6 +231,23 @@ int bpf_prog5(struct sk_msg_md *msg)
 		bpf_printk("sk_msg2: length update %i->%i\n",
 			   len1, len2);
 	}
+
+	start_push = bpf_map_lookup_elem(&sock_bytes, &two);
+	end_push = bpf_map_lookup_elem(&sock_bytes, &three);
+	if (start_push && end_push) {
+		int err;
+
+		bpf_printk("sk_msg2: push(%i:%i)\n",
+			   start_push ? *start_push : 0,
+			   end_push ? *end_push : 0);
+		err = bpf_msg_push_data(msg, *start_push, *end_push, 0);
+		if (err)
+			bpf_printk("sk_msg2: push_data err %i\n", err);
+		len3 = (__u64)msg->data_end - (__u64)msg->data;
+		bpf_printk("sk_msg2: length push_update %i->%i\n",
+			   len2 ? len2 : len1, len3);
+	}
+
 	bpf_printk("sk_msg2: data length %i err1 %i err2 %i\n",
 		   len1, err1, err2);
 	return SK_PASS;
@@ -233,8 +256,8 @@ int bpf_prog5(struct sk_msg_md *msg)
 SEC("sk_msg3")
 int bpf_prog6(struct sk_msg_md *msg)
 {
-	int *bytes, zero = 0, one = 1, key = 0;
-	int *start, *end, *f;
+	int *bytes, *start, *end, *start_push, *end_push, *f;
+	int zero = 0, one = 1, two = 2, three = 3, key = 0;
 	__u64 flags = 0;
 
 	bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
@@ -243,10 +266,17 @@ int bpf_prog6(struct sk_msg_md *msg)
 	bytes = bpf_map_lookup_elem(&sock_cork_bytes, &zero);
 	if (bytes)
 		bpf_msg_cork_bytes(msg, *bytes);
-	start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
-	end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
+
+	start = bpf_map_lookup_elem(&sock_bytes, &zero);
+	end = bpf_map_lookup_elem(&sock_bytes, &one);
 	if (start && end)
 		bpf_msg_pull_data(msg, *start, *end, 0);
+
+	start_push = bpf_map_lookup_elem(&sock_bytes, &two);
+	end_push = bpf_map_lookup_elem(&sock_bytes, &three);
+	if (start_push && end_push)
+		bpf_msg_push_data(msg, *start_push, *end_push, 0);
+
 	f = bpf_map_lookup_elem(&sock_redir_flags, &zero);
 	if (f && *f) {
 		key = 2;
@@ -262,8 +292,9 @@ int bpf_prog6(struct sk_msg_md *msg)
 SEC("sk_msg4")
 int bpf_prog7(struct sk_msg_md *msg)
 {
-	int err1 = 0, err2 = 0, zero = 0, one = 1, key = 0;
-	int *f, *bytes, *start, *end, len1, len2;
+	int zero = 0, one = 1, two = 2, three = 3, len1, len2 = 0, len3;
+	int *bytes, *start, *end, *start_push, *end_push, *f;
+	int err1 = 0, err2 = 0, key = 0;
 	__u64 flags = 0;
 
 		int err;
@@ -274,10 +305,10 @@ int bpf_prog7(struct sk_msg_md *msg)
 	if (bytes)
 		err2 = bpf_msg_cork_bytes(msg, *bytes);
 	len1 = (__u64)msg->data_end - (__u64)msg->data;
-	start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
-	end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
-	if (start && end) {
 
+	start = bpf_map_lookup_elem(&sock_bytes, &zero);
+	end = bpf_map_lookup_elem(&sock_bytes, &one);
+	if (start && end) {
 		bpf_printk("sk_msg2: pull(%i:%i)\n",
 			   start ? *start : 0, end ? *end : 0);
 		err = bpf_msg_pull_data(msg, *start, *end, 0);
@@ -288,6 +319,22 @@ int bpf_prog7(struct sk_msg_md *msg)
 		bpf_printk("sk_msg2: length update %i->%i\n",
 			   len1, len2);
 	}
+
+	start_push = bpf_map_lookup_elem(&sock_bytes, &two);
+	end_push = bpf_map_lookup_elem(&sock_bytes, &three);
+	if (start_push && end_push) {
+		bpf_printk("sk_msg4: push(%i:%i)\n",
+			   start_push ? *start_push : 0,
+			   end_push ? *end_push : 0);
+		err = bpf_msg_push_data(msg, *start_push, *end_push, 0);
+		if (err)
+			bpf_printk("sk_msg4: push_data err %i\n",
+				   err);
+		len3 = (__u64)msg->data_end - (__u64)msg->data;
+		bpf_printk("sk_msg4: length push_update %i->%i\n",
+			   len2 ? len2 : len1, len3);
+	}
+
 	f = bpf_map_lookup_elem(&sock_redir_flags, &zero);
 	if (f && *f) {
 		key = 2;
@@ -342,8 +389,8 @@ int bpf_prog9(struct sk_msg_md *msg)
 SEC("sk_msg7")
 int bpf_prog10(struct sk_msg_md *msg)
 {
-	int *bytes, zero = 0, one = 1;
-	int *start, *end;
+	int *bytes, *start, *end, *start_push, *end_push;
+	int zero = 0, one = 1, two = 2, three = 3;
 
 	bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
 	if (bytes)
@@ -351,10 +398,14 @@ int bpf_prog10(struct sk_msg_md *msg)
 	bytes = bpf_map_lookup_elem(&sock_cork_bytes, &zero);
 	if (bytes)
 		bpf_msg_cork_bytes(msg, *bytes);
-	start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
-	end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
+	start = bpf_map_lookup_elem(&sock_bytes, &zero);
+	end = bpf_map_lookup_elem(&sock_bytes, &one);
 	if (start && end)
 		bpf_msg_pull_data(msg, *start, *end, 0);
+	start_push = bpf_map_lookup_elem(&sock_bytes, &two);
+	end_push = bpf_map_lookup_elem(&sock_bytes, &three);
+	if (start_push && end_push)
+		bpf_msg_push_data(msg, *start_push, *end_push, 0);
 
 	return SK_DROP;
 }

^ permalink raw reply related

* [bpf-next PATCH 2/3] bpf: libbpf support for msg_push_data
From: John Fastabend @ 2018-10-18 21:52 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, john.fastabend
In-Reply-To: <20181018215041.22362.36585.stgit@john-Precision-Tower-5810>

Add support for new bpf_msg_push_data in libbpf.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 tools/include/uapi/linux/bpf.h            |   20 +++++++++++++++++++-
 tools/testing/selftests/bpf/bpf_helpers.h |    2 ++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 5e46f67..1e9fbc5 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2215,6 +2215,23 @@ struct bpf_stack_build_id {
  *		pointer that was returned from bpf_sk_lookup_xxx\ ().
  *	Return
  *		0 on success, or a negative error in case of failure.
+ *
+ * int bpf_msg_push_data(struct sk_buff *skb, u32 start, u32 len, u64 flags)
+ *	Description
+ *		For socket policies, insert *len* bytes into msg at offset
+ *		*start*.
+ *
+ *		If a program of type **BPF_PROG_TYPE_SK_MSG** is run on a
+ *		*msg* it may want to insert metadata or options into the msg.
+ *		This can later be read and used by any of the lower layer BPF
+ *		hooks.
+ *
+ *		This helper may fail if under memory pressure (a malloc
+ *		fails) in these cases BPF programs will get an appropriate
+ *		error and BPF programs will need to handle them.
+ *
+ *	Return
+ *		0 on success, or a negative error in case of failure.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2303,7 +2320,8 @@ struct bpf_stack_build_id {
 	FN(skb_ancestor_cgroup_id),	\
 	FN(sk_lookup_tcp),		\
 	FN(sk_lookup_udp),		\
-	FN(sk_release),
+	FN(sk_release),			\
+	FN(msg_push_data),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index fda8c16..4e33511 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -104,6 +104,8 @@ static int (*bpf_msg_cork_bytes)(void *ctx, int len) =
 	(void *) BPF_FUNC_msg_cork_bytes;
 static int (*bpf_msg_pull_data)(void *ctx, int start, int end, int flags) =
 	(void *) BPF_FUNC_msg_pull_data;
+static int (*bpf_msg_push_data)(void *ctx, int start, int end, int flags) =
+	(void *) BPF_FUNC_msg_push_data;
 static int (*bpf_bind)(void *ctx, void *addr, int addr_len) =
 	(void *) BPF_FUNC_bind;
 static int (*bpf_xdp_adjust_tail)(void *ctx, int offset) =

^ permalink raw reply related

* [bpf-next PATCH 1/3] bpf: sk_msg program helper bpf_msg_push_data
From: John Fastabend @ 2018-10-18 21:52 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, john.fastabend
In-Reply-To: <20181018215041.22362.36585.stgit@john-Precision-Tower-5810>

This allows user to push data into a msg using sk_msg program types.
The format is as follows,

	bpf_msg_push_data(msg, offset, len, flags)

this will insert 'len' bytes at offset 'offset'. For example to
prepend 10 bytes at the front of the message the user can,

	bpf_msg_push_data(msg, 0, 10, 0);

This will invalidate data bounds so BPF user will have to then recheck
data bounds after calling this. After this the msg size will have been
updated and the user is free to write into the added bytes. We allow
any offset/len as long as it is within the (data, data_end) range.
However, a copy will be required if the ring is full and its possible
for the helper to fail with ENOMEM or EINVAL errors which need to be
handled by the BPF program.

This can be used similar to XDP metadata to pass data between sk_msg
layer and lower layers.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 include/linux/skmsg.h    |    5 ++
 include/uapi/linux/bpf.h |   20 +++++++
 net/core/filter.c        |  134 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 158 insertions(+), 1 deletion(-)

diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index 22347b0..677b673 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -207,6 +207,11 @@ static inline struct scatterlist *sk_msg_elem(struct sk_msg *msg, int which)
 	return &msg->sg.data[which];
 }
 
+static inline struct scatterlist sk_msg_elem_cpy(struct sk_msg *msg, int which)
+{
+	return msg->sg.data[which];
+}
+
 static inline struct page *sk_msg_page(struct sk_msg *msg, int which)
 {
 	return sg_page(sk_msg_elem(msg, which));
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 5e46f67..1e9fbc5 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2215,6 +2215,23 @@ struct bpf_stack_build_id {
  *		pointer that was returned from bpf_sk_lookup_xxx\ ().
  *	Return
  *		0 on success, or a negative error in case of failure.
+ *
+ * int bpf_msg_push_data(struct sk_buff *skb, u32 start, u32 len, u64 flags)
+ *	Description
+ *		For socket policies, insert *len* bytes into msg at offset
+ *		*start*.
+ *
+ *		If a program of type **BPF_PROG_TYPE_SK_MSG** is run on a
+ *		*msg* it may want to insert metadata or options into the msg.
+ *		This can later be read and used by any of the lower layer BPF
+ *		hooks.
+ *
+ *		This helper may fail if under memory pressure (a malloc
+ *		fails) in these cases BPF programs will get an appropriate
+ *		error and BPF programs will need to handle them.
+ *
+ *	Return
+ *		0 on success, or a negative error in case of failure.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2303,7 +2320,8 @@ struct bpf_stack_build_id {
 	FN(skb_ancestor_cgroup_id),	\
 	FN(sk_lookup_tcp),		\
 	FN(sk_lookup_udp),		\
-	FN(sk_release),
+	FN(sk_release),			\
+	FN(msg_push_data),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/net/core/filter.c b/net/core/filter.c
index 1a3ac6c..4bcf238 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2297,6 +2297,137 @@ int skb_do_redirect(struct sk_buff *skb)
 	.arg4_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
+	   u32, len, u64, flags)
+{
+	struct scatterlist sge, nsge, nnsge, rsge = {0}, *psge;
+	int new, i = 0, l, space, copy = 0, offset = 0;
+	u8 *raw, *to, *from;
+	struct page *page;
+
+	if (unlikely(flags))
+		return -EINVAL;
+
+	/* First find the starting scatterlist element */
+	i = msg->sg.start;
+	do {
+		l = sk_msg_elem(msg, i)->length;
+
+		if (start < offset + l)
+			break;
+		offset += l;
+		sk_msg_iter_var_next(i);
+	} while (i != msg->sg.end);
+
+	if (start >= offset + l)
+		return -EINVAL;
+
+	space = MAX_MSG_FRAGS - sk_msg_elem_used(msg);
+
+	/* If no space available will fallback to copy, we need at
+	 * least one scatterlist elem available to push data into
+	 * when start aligns to the beginning of an element or two
+	 * when it falls inside an element. We handle the start equals
+	 * offset case because its the common case for inserting a
+	 * header.
+	 */
+	if (!space || (space == 1 && start != offset))
+		copy = msg->sg.data[i].length;
+
+	page = alloc_pages(__GFP_NOWARN | GFP_ATOMIC | __GFP_COMP,
+			   get_order(copy + len));
+	if (unlikely(!page))
+		return -ENOMEM;
+
+	if (copy) {
+		int front, back;
+
+		raw = page_address(page);
+
+		psge = sk_msg_elem(msg, i);
+		front = start - offset;
+		back = psge->length - front;
+		from = sg_virt(psge);
+
+		if (front)
+			memcpy(raw, from, front);
+
+		if (back) {
+			from += front;
+			to = raw + front + len;
+
+			memcpy(to, from, back);
+		}
+
+		put_page(sg_page(psge));
+	} else if (start - offset) {
+		psge = sk_msg_elem(msg, i);
+		rsge = sk_msg_elem_cpy(msg, i);
+
+		psge->length = start - offset;
+		rsge.length -= psge->length;
+		rsge.offset += start;
+
+		sk_msg_iter_var_next(i);
+		sg_unmark_end(psge);
+		sk_msg_iter_next(msg, end);
+	}
+
+	/* Slot(s) to place newly allocated data */
+	new = i;
+
+	/* Shift one or two slots as needed */
+	if (!copy) {
+		sge = sk_msg_elem_cpy(msg, i);
+
+		sk_msg_iter_var_next(i);
+		sg_unmark_end(&sge);
+		sk_msg_iter_next(msg, end);
+
+		nsge = sk_msg_elem_cpy(msg, i);
+		if (rsge.length) {
+			sk_msg_iter_var_next(i);
+			nnsge = sk_msg_elem_cpy(msg, i);
+		}
+
+		while (i != msg->sg.end) {
+			msg->sg.data[i] = sge;
+			sge = nsge;
+			sk_msg_iter_var_next(i);
+			if (rsge.length) {
+				nsge = nnsge;
+				nnsge = sk_msg_elem_cpy(msg, i);
+			} else {
+				nsge = sk_msg_elem_cpy(msg, i);
+			}
+		}
+	}
+
+	/* Place newly allocated data buffer */
+	sk_mem_charge(msg->sk, len);
+	msg->sg.size += len;
+	msg->sg.copy[new] = false;
+	sg_set_page(&msg->sg.data[new], page, len + copy, 0);
+	if (rsge.length) {
+		get_page(sg_page(&rsge));
+		sk_msg_iter_var_next(new);
+		msg->sg.data[new] = rsge;
+	}
+
+	sk_msg_compute_data_pointers(msg);
+	return 0;
+}
+
+static const struct bpf_func_proto bpf_msg_push_data_proto = {
+	.func		= bpf_msg_push_data,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_ANYTHING,
+	.arg3_type	= ARG_ANYTHING,
+	.arg4_type	= ARG_ANYTHING,
+};
+
 BPF_CALL_1(bpf_get_cgroup_classid, const struct sk_buff *, skb)
 {
 	return task_get_classid(skb);
@@ -4854,6 +4985,7 @@ bool bpf_helper_changes_pkt_data(void *func)
 	    func == bpf_xdp_adjust_head ||
 	    func == bpf_xdp_adjust_meta ||
 	    func == bpf_msg_pull_data ||
+	    func == bpf_msg_push_data ||
 	    func == bpf_xdp_adjust_tail ||
 #if IS_ENABLED(CONFIG_IPV6_SEG6_BPF)
 	    func == bpf_lwt_seg6_store_bytes ||
@@ -5124,6 +5256,8 @@ bool bpf_helper_changes_pkt_data(void *func)
 		return &bpf_msg_cork_bytes_proto;
 	case BPF_FUNC_msg_pull_data:
 		return &bpf_msg_pull_data_proto;
+	case BPF_FUNC_msg_push_data:
+		return &bpf_msg_push_data_proto;
 	case BPF_FUNC_get_local_storage:
 		return &bpf_get_local_storage_proto;
 	default:

^ permalink raw reply related

* [bpf-next PATCH 0/3] sockmap, bpf_msg_push_data helper
From: John Fastabend @ 2018-10-18 21:51 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, john.fastabend

This series adds a new helper bpf_msg_push_data to be used by
sk_msg programs. The helper can be used to insert extra bytes into
the message that can then be used by the program as metadata tags
among other things.

The first patch adds the helper, second patch the libbpf support,
and last patch updates test_sockmap to run msg_push_data tests.

---

John Fastabend (3):
      bpf: sk_msg program helper bpf_msg_push_data
      bpf: libbpf support for msg_push_data
      bpf: test_sockmap add options to use msg_push_data


 include/linux/skmsg.h                           |    5 +
 include/uapi/linux/bpf.h                        |   20 +++
 net/core/filter.c                               |  134 +++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h                  |   20 +++
 tools/testing/selftests/bpf/bpf_helpers.h       |    2 
 tools/testing/selftests/bpf/test_sockmap.c      |   58 +++++++++-
 tools/testing/selftests/bpf/test_sockmap_kern.h |   97 +++++++++++++----
 7 files changed, 308 insertions(+), 28 deletions(-)

^ permalink raw reply

* Re: linux-next: build failure after merge of the net-next tree
From: Or Gerlitz @ 2018-10-19  5:48 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Miller, Linux Netdev List, Doug Ledford, Jason Gunthorpe,
	linux-next, Linux Kernel, Mark Bloch, Leon Romanovsky,
	Paul Blakey, Saeed Mahameed
In-Reply-To: <20181019111805.28860f73@canb.auug.org.au>

On Fri, Oct 19, 2018 at 3:19 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> After merging the net-next tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> drivers/infiniband/hw/mlx5/flow.c: In function 'mlx5_ib_handler_MLX5_IB_METHOD_CREATE_FLOW':
> drivers/infiniband/hw/mlx5/flow.c:163:12: error: 'struct mlx5_flow_act' has no member named 'has_flow_tag'; did you mean 'flow_tag'?
>    flow_act.has_flow_tag = true;
>             ^~~~~~~~~~~~
>             flow_tag
>
> Caused by commit
>
>   d5634fee245f ("net/mlx5: Add a no-append flow insertion mode")
>
> interacting with commit
>
>   ba4a41198324 ("RDMA/mlx5: Add support for flow tag to raw create flow")
>
> from the rdma tree.
>
> I have applied the following merge fix patch for today:
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Fri, 19 Oct 2018 11:10:39 +1100
> Subject: [PATCH] net/mlx5: fix up for has_flow_tag changing to a flag
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  drivers/infiniband/hw/mlx5/flow.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/flow.c b/drivers/infiniband/hw/mlx5/flow.c
> index e57435cb6d96..f86cdcafdafc 100644
> --- a/drivers/infiniband/hw/mlx5/flow.c
> +++ b/drivers/infiniband/hw/mlx5/flow.c
> @@ -160,7 +160,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)(
>                         ret = -EINVAL;
>                         goto err_out;
>                 }
> -               flow_act.has_flow_tag = true;
> +               flow_act.flags |= FLOW_ACT_HAS_TAG;
>         }
>
>         flow_handler = mlx5_ib_raw_fs_rule_add(dev, fs_matcher, &flow_act,


yeah, this is the correct resolution, thanks for addressing!

^ permalink raw reply

* Re: [PATCH net-next] MAINTAINERS: Update contact info for VRF entry
From: David Miller @ 2018-10-18 21:30 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, shrijeet, dsahern
In-Reply-To: <20181018191549.64041-1-dsahern@kernel.org>

From: David Ahern <dsahern@kernel.org>
Date: Thu, 18 Oct 2018 13:15:49 -0600

> From: David Ahern <dsahern@gmail.com>
> 
> Update Shrijeet's email address for the VRF entry.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH bpf-next 2/2] samples: bpf: get ifindex from ifname
From: John Fastabend @ 2018-10-18 21:30 UTC (permalink / raw)
  To: Matteo Croce, netdev; +Cc: Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20181018204709.29547-3-mcroce@redhat.com>

On 10/18/2018 01:47 PM, Matteo Croce wrote:
> Find the ifindex via ioctl(SIOCGIFINDEX) instead of requiring the
> numeric ifindex.
> 
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
> ---

I don't think there are any expectation that samples have to be
stable as far as inputs over versions. And because I consistently
run this with the ifname before realizing its the ifindex not
string name I'll Ack it.

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply

* Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Martin Lau @ 2018-10-18 21:19 UTC (permalink / raw)
  To: Edward Cree
  Cc: Yonghong Song, Alexei Starovoitov, daniel@iogearbox.net,
	netdev@vger.kernel.org, Kernel Team
In-Reply-To: <6645d026-f909-3f9d-2816-02bea6dced97@solarflare.com>

On Thu, Oct 18, 2018 at 08:41:52PM +0100, Edward Cree wrote:
> On 18/10/18 19:12, Martin Lau wrote:
> > The BTF verification and bpf_prog_load() has to treat
> > them differently.
> >
> > Are you suggesting they can be treated the same for
> > the kernel verification purpose?
> >
> > or
> >
> > The concern is, having two kind, BTF_KIND_FUNC_PROTO and
> > BTF_KIND_FUNC, is confusing because they have almost the
> > same BTF metadata?
> >
> > From the BTF perspective, they are different because
> > they are allowed to contain different information.
> > For example, only "foo" can have func_info in patch 5
> > (the to-be-added line_info can only belong to "foo" also).
> Perhaps I'm not properly understanding how BTF is to be used.
> But from my perspective, 'foo' is not a type; it's an instance
>  of a type; i.e. it's a value of type int ()(int).  Then by a
>  kind of metonymy, we are including a BTF record that defines
>  the type 'foo' as int()(int), in order to state that this is
>  the type _of_ foo.  Similarly, we are including a BTF record
>  defining 'bar' as the type int (*)(int), thus stating that
>  the type _of_ bar is int (*)(int).
> So everything that's special about 'foo' relates to the fact
>  that it is a named, rather than anonymous, type, which is
>  being used to indicate that there is an actual object 'foo'
>  of this type.
> And this is *identical* to how the BTF record "name 'a', type
>  int" declares an object a, the BTF record "name 'b', type
>  int *" declares an object b, and the anonymous int type
>  referenced by b's btf_type.type does not declare an object.
> 
> So either types and objects are represented the same way in
>  BTF, or not; it's inconsistent to make a distinction between
>  them for functions but not for other types.
> 
> I know C thinks functions are special, but that's a wart in C
>  that maybe makes sense in its context but not elsewhere.
As I have mentioned earlier, it is also special to
the kernel because the BTF verifier and bpf_prog_load()
need to do different checks for FUNC and FUNC_PROTO to
ensure they are sane.

First, we need to agree that the kernel needs to verify
them differently.

and then we can proceed to discuss how to distinguish them.
We picked the current way to avoid adding a
new BTF function section and keep it
strict forward to distinguish them w/o relying
on other hints from 'struct btf_type'.

Are you suggesting another way of doing it?

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: Extend the sk_lookup() helper to XDP hookpoint.
From: Daniel Borkmann @ 2018-10-18 21:20 UTC (permalink / raw)
  To: Joe Stringer, Nitin Hande; +Cc: netdev, ast, Jesper Brouer, john fastabend
In-Reply-To: <CAOftzPgpz=1TUHfFAOvrr03YUtTLY+jJYBB8bygnHZZEgt2eVw@mail.gmail.com>

On 10/18/2018 11:06 PM, Joe Stringer wrote:
> On Thu, 18 Oct 2018 at 11:54, Nitin Hande <nitin.hande@gmail.com> wrote:
[...]
>> Open Issue
>> * The underlying code relies on presence of an skb to find out the
>> right sk for the case of REUSEPORT socket option. Since there is
>> no skb available at XDP hookpoint, the helper function will return
>> the first available sk based off the 5 tuple hash. If the desire
>> is to return a particular sk matching reuseport_cb function, please
>> suggest way to tackle it, which can be addressed in a future commit.

>> Signed-off-by: Nitin Hande <Nitin.Hande@gmail.com>
> 
> Thanks Nitin, LGTM overall.
> 
> The REUSEPORT thing suggests that the usage of this helper from XDP
> layer may lead to a different socket being selected vs. the equivalent
> call at TC hook, or other places where the selection may occur. This
> could be a bit counter-intuitive.
> 
> One thought I had to work around this was to introduce a flag,
> something like BPF_F_FIND_REUSEPORT_SK_BY_HASH. This flag would
> effectively communicate in the API that the bpf_sk_lookup_xxx()
> functions will only select a REUSEPORT socket based on the hash and
> not by, for example BPF_PROG_TYPE_SK_REUSEPORT programs. The absence
> of the flag would support finding REUSEPORT sockets by other
> mechanisms (which would be allowed for now from TC hooks but would be
> disallowed from XDP, since there's no specific plan to support this).

Hmm, given skb is NULL here the only way to lookup the socket in such
scenario is based on hash, that is, inet_ehashfn() / inet6_ehashfn(),
perhaps alternative is to pass this hash in from XDP itself to the
helper so it could be custom selector. Do you have a specific use case
on this for XDP (just curious)?

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: Extend the sk_lookup() helper to XDP hookpoint.
From: Joe Stringer @ 2018-10-18 21:06 UTC (permalink / raw)
  To: Nitin Hande
  Cc: daniel, netdev, ast, Joe Stringer, Jesper Brouer, john fastabend
In-Reply-To: <20181018115029.77ff8e71@ubun>

On Thu, 18 Oct 2018 at 11:54, Nitin Hande <nitin.hande@gmail.com> wrote:
>
>
> This patch proposes to extend the sk_lookup() BPF API to the
> XDP hookpoint. The sk_lookup() helper supports a lookup
> on incoming packet to find the corresponding socket that will
> receive this packet. Current support for this BPF API is
> at the tc hookpoint. This patch will extend this API at XDP
> hookpoint. A XDP program can map the incoming packet to the
> 5-tuple parameter and invoke the API to find the corresponding
> socket structure.
>
> Open Issue
> * The underlying code relies on presence of an skb to find out the
> right sk for the case of REUSEPORT socket option. Since there is
> no skb available at XDP hookpoint, the helper function will return
> the first available sk based off the 5 tuple hash. If the desire
> is to return a particular sk matching reuseport_cb function, please
> suggest way to tackle it, which can be addressed in a future commit.
>
> Signed-off-by: Nitin Hande <Nitin.Hande@gmail.com>

Thanks Nitin, LGTM overall.

The REUSEPORT thing suggests that the usage of this helper from XDP
layer may lead to a different socket being selected vs. the equivalent
call at TC hook, or other places where the selection may occur. This
could be a bit counter-intuitive.

One thought I had to work around this was to introduce a flag,
something like BPF_F_FIND_REUSEPORT_SK_BY_HASH. This flag would
effectively communicate in the API that the bpf_sk_lookup_xxx()
functions will only select a REUSEPORT socket based on the hash and
not by, for example BPF_PROG_TYPE_SK_REUSEPORT programs. The absence
of the flag would support finding REUSEPORT sockets by other
mechanisms (which would be allowed for now from TC hooks but would be
disallowed from XDP, since there's no specific plan to support this).

^ permalink raw reply

* Re: [PATCH bpf-next v3 7/7] selftests/bpf: add test cases for queue and stack maps
From: Song Liu @ 2018-10-18 21:02 UTC (permalink / raw)
  To: mauricio.vasquez; +Cc: Alexei Starovoitov, Daniel Borkmann, Networking
In-Reply-To: <4e01c6b0-2f9a-f8e5-d746-c9448fb78717@polito.it>

On Thu, Oct 18, 2018 at 10:33 AM Mauricio Vasquez
<mauricio.vasquez@polito.it> wrote:
>
>
> On 10/18/18 11:36 AM, Song Liu wrote:
> > On Thu, Oct 18, 2018 at 6:16 AM Mauricio Vasquez B
> > <mauricio.vasquez@polito.it> wrote:
> >> test_maps:
> >> Tests that queue/stack maps are behaving correctly even in corner cases
> >>
> >> test_progs:
> >> Tests new ebpf helpers
> >>
> >> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
> >> ---
> >>   tools/lib/bpf/bpf.c                                |   12 ++
> >>   tools/lib/bpf/bpf.h                                |    2
> >>   tools/testing/selftests/bpf/Makefile               |    5 +
> >>   tools/testing/selftests/bpf/bpf_helpers.h          |    7 +
> >>   tools/testing/selftests/bpf/test_maps.c            |  122 ++++++++++++++++++++
> >>   tools/testing/selftests/bpf/test_progs.c           |   99 ++++++++++++++++
> >>   tools/testing/selftests/bpf/test_queue_map.c       |    4 +
> >>   tools/testing/selftests/bpf/test_queue_stack_map.h |   59 ++++++++++
> >>   tools/testing/selftests/bpf/test_stack_map.c       |    4 +
> >>   9 files changed, 313 insertions(+), 1 deletion(-)
> >>   create mode 100644 tools/testing/selftests/bpf/test_queue_map.c
> >>   create mode 100644 tools/testing/selftests/bpf/test_queue_stack_map.h
> >>   create mode 100644 tools/testing/selftests/bpf/test_stack_map.c
> >>
> >> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> >> index d70a255cb05e..03f9bcc4ef50 100644
> >> --- a/tools/lib/bpf/bpf.c
> >> +++ b/tools/lib/bpf/bpf.c
> >> @@ -278,6 +278,18 @@ int bpf_map_lookup_elem(int fd, const void *key, void *value)
> >>          return sys_bpf(BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr));
> >>   }
> >>
> >> +int bpf_map_lookup_and_delete_elem(int fd, const void *key, void *value)
> >> +{
> >> +       union bpf_attr attr;
> >> +
> >> +       bzero(&attr, sizeof(attr));
> >> +       attr.map_fd = fd;
> >> +       attr.key = ptr_to_u64(key);
> >> +       attr.value = ptr_to_u64(value);
> >> +
> >> +       return sys_bpf(BPF_MAP_LOOKUP_AND_DELETE_ELEM, &attr, sizeof(attr));
> >> +}
> >> +
> >>   int bpf_map_delete_elem(int fd, const void *key)
> >>   {
> >>          union bpf_attr attr;
> >> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> >> index 258c3c178333..26a51538213c 100644
> >> --- a/tools/lib/bpf/bpf.h
> >> +++ b/tools/lib/bpf/bpf.h
> >> @@ -99,6 +99,8 @@ LIBBPF_API int bpf_map_update_elem(int fd, const void *key, const void *value,
> >>                                     __u64 flags);
> >>
> >>   LIBBPF_API int bpf_map_lookup_elem(int fd, const void *key, void *value);
> >> +LIBBPF_API int bpf_map_lookup_and_delete_elem(int fd, const void *key,
> >> +                                             void *value);
> >>   LIBBPF_API int bpf_map_delete_elem(int fd, const void *key);
> >>   LIBBPF_API int bpf_map_get_next_key(int fd, const void *key, void *next_key);
> >>   LIBBPF_API int bpf_obj_pin(int fd, const char *pathname);
> >> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> >> index d99dd6fc3fbe..e39dfb4e7970 100644
> >> --- a/tools/testing/selftests/bpf/Makefile
> >> +++ b/tools/testing/selftests/bpf/Makefile
> >> @@ -37,7 +37,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
> >>          test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
> >>          get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
> >>          test_skb_cgroup_id_kern.o bpf_flow.o netcnt_prog.o \
> >> -       test_sk_lookup_kern.o test_xdp_vlan.o
> >> +       test_sk_lookup_kern.o test_xdp_vlan.o test_queue_map.o test_stack_map.o
> >>
> >>   # Order correspond to 'make run_tests' order
> >>   TEST_PROGS := test_kmod.sh \
> >> @@ -118,6 +118,9 @@ CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> >>   $(OUTPUT)/test_l4lb_noinline.o: CLANG_FLAGS += -fno-inline
> >>   $(OUTPUT)/test_xdp_noinline.o: CLANG_FLAGS += -fno-inline
> >>
> >> +$(OUTPUT)/test_queue_map.o: test_queue_stack_map.h
> >> +$(OUTPUT)/test_stack_map.o: test_queue_stack_map.h
> > This looks weird. You meant the .c files, right?
>
> Queue and stack tests are very similar, in order to avoid a lot of code
> duplication I created a single test_queue_stack_map.h file where all the
> logic is placed.
>
> There are two .c files (test_queue_map.c and test_stack_map.c) they
> define the map type and include test_queue_stack_map.h.
>
> test_queue_map.o and test_stack_map.o create an implicit dependency on
> test_queue_map.c and test_stack_map.c, but the dependency on the header
> is also needed, so I added those two lines.

Thanks for the explanation.
Acked-by: Song Liu <songliubraving@fb.com>

>
> >
> >> +
> >>   BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris)
> >>   BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
> >>   BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm')
> >> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> >> index fda8c162d0df..6407a3df0f3b 100644
> >> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> >> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> >> @@ -16,6 +16,13 @@ static int (*bpf_map_update_elem)(void *map, void *key, void *value,
> >>          (void *) BPF_FUNC_map_update_elem;
> >>   static int (*bpf_map_delete_elem)(void *map, void *key) =
> >>          (void *) BPF_FUNC_map_delete_elem;
> >> +static int (*bpf_map_push_elem)(void *map, void *value,
> >> +                               unsigned long long flags) =
> >> +       (void *) BPF_FUNC_map_push_elem;
> >> +static int (*bpf_map_pop_elem)(void *map, void *value) =
> >> +       (void *) BPF_FUNC_map_pop_elem;
> >> +static int (*bpf_map_peek_elem)(void *map, void *value) =
> >> +       (void *) BPF_FUNC_map_peek_elem;
> >>   static int (*bpf_probe_read)(void *dst, int size, void *unsafe_ptr) =
> >>          (void *) BPF_FUNC_probe_read;
> >>   static unsigned long long (*bpf_ktime_get_ns)(void) =
> >> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> >> index 9b552c0fc47d..4db2116e52be 100644
> >> --- a/tools/testing/selftests/bpf/test_maps.c
> >> +++ b/tools/testing/selftests/bpf/test_maps.c
> >> @@ -15,6 +15,7 @@
> >>   #include <string.h>
> >>   #include <assert.h>
> >>   #include <stdlib.h>
> >> +#include <time.h>
> >>
> >>   #include <sys/wait.h>
> >>   #include <sys/socket.h>
> >> @@ -471,6 +472,122 @@ static void test_devmap(int task, void *data)
> >>          close(fd);
> >>   }
> >>
> >> +static void test_queuemap(int task, void *data)
> >> +{
> >> +       const int MAP_SIZE = 32;
> >> +       __u32 vals[MAP_SIZE + MAP_SIZE/2], val;
> >> +       int fd, i;
> >> +
> >> +       /* Fill test values to be used */
> >> +       for (i = 0; i < MAP_SIZE + MAP_SIZE/2; i++)
> >> +               vals[i] = rand();
> >> +
> >> +       /* Invalid key size */
> >> +       fd = bpf_create_map(BPF_MAP_TYPE_QUEUE, 4, sizeof(val), MAP_SIZE,
> >> +                           map_flags);
> >> +       assert(fd < 0 && errno == EINVAL);
> >> +
> >> +       fd = bpf_create_map(BPF_MAP_TYPE_QUEUE, 0, sizeof(val), MAP_SIZE,
> >> +                           map_flags);
> >> +       /* Queue map does not support BPF_F_NO_PREALLOC */
> >> +       if (map_flags & BPF_F_NO_PREALLOC) {
> >> +               assert(fd < 0 && errno == EINVAL);
> >> +               return;
> >> +       }
> >> +       if (fd < 0) {
> >> +               printf("Failed to create queuemap '%s'!\n", strerror(errno));
> >> +               exit(1);
> >> +       }
> >> +
> >> +       /* Push MAP_SIZE elements */
> >> +       for (i = 0; i < MAP_SIZE; i++)
> >> +               assert(bpf_map_update_elem(fd, NULL, &vals[i], 0) == 0);
> >> +
> >> +       /* Check that element cannot be pushed due to max_entries limit */
> >> +       assert(bpf_map_update_elem(fd, NULL, &val, 0) == -1 &&
> >> +              errno == E2BIG);
> >> +
> >> +       /* Peek element */
> >> +       assert(bpf_map_lookup_elem(fd, NULL, &val) == 0 && val == vals[0]);
> >> +
> >> +       /* Replace half elements */
> >> +       for (i = MAP_SIZE; i < MAP_SIZE + MAP_SIZE/2; i++)
> >> +               assert(bpf_map_update_elem(fd, NULL, &vals[i], BPF_EXIST) == 0);
> >> +
> >> +       /* Pop all elements */
> >> +       for (i = MAP_SIZE/2; i < MAP_SIZE + MAP_SIZE/2; i++)
> >> +               assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == 0 &&
> >> +                      val == vals[i]);
> >> +
> >> +       /* Check that there are not elements left */
> >> +       assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == -1 &&
> >> +              errno == ENOENT);
> >> +
> >> +       /* Check that non supported functions set errno to EINVAL */
> >> +       assert(bpf_map_delete_elem(fd, NULL) == -1 && errno == EINVAL);
> >> +       assert(bpf_map_get_next_key(fd, NULL, NULL) == -1 && errno == EINVAL);
> >> +
> >> +       close(fd);
> >> +}
> >> +
> >> +static void test_stackmap(int task, void *data)
> >> +{
> >> +       const int MAP_SIZE = 32;
> >> +       __u32 vals[MAP_SIZE + MAP_SIZE/2], val;
> >> +       int fd, i;
> >> +
> >> +       /* Fill test values to be used */
> >> +       for (i = 0; i < MAP_SIZE + MAP_SIZE/2; i++)
> >> +               vals[i] = rand();
> >> +
> >> +       /* Invalid key size */
> >> +       fd = bpf_create_map(BPF_MAP_TYPE_STACK, 4, sizeof(val), MAP_SIZE,
> >> +                           map_flags);
> >> +       assert(fd < 0 && errno == EINVAL);
> >> +
> >> +       fd = bpf_create_map(BPF_MAP_TYPE_STACK, 0, sizeof(val), MAP_SIZE,
> >> +                           map_flags);
> >> +       /* Stack map does not support BPF_F_NO_PREALLOC */
> >> +       if (map_flags & BPF_F_NO_PREALLOC) {
> >> +               assert(fd < 0 && errno == EINVAL);
> >> +               return;
> >> +       }
> >> +       if (fd < 0) {
> >> +               printf("Failed to create stackmap '%s'!\n", strerror(errno));
> >> +               exit(1);
> >> +       }
> >> +
> >> +       /* Push MAP_SIZE elements */
> >> +       for (i = 0; i < MAP_SIZE; i++)
> >> +               assert(bpf_map_update_elem(fd, NULL, &vals[i], 0) == 0);
> >> +
> >> +       /* Check that element cannot be pushed due to max_entries limit */
> >> +       assert(bpf_map_update_elem(fd, NULL, &val, 0) == -1 &&
> >> +              errno == E2BIG);
> >> +
> >> +       /* Peek element */
> >> +       assert(bpf_map_lookup_elem(fd, NULL, &val) == 0 && val == vals[i - 1]);
> >> +
> >> +       /* Replace half elements */
> >> +       for (i = MAP_SIZE; i < MAP_SIZE + MAP_SIZE/2; i++)
> >> +               assert(bpf_map_update_elem(fd, NULL, &vals[i], BPF_EXIST) == 0);
> >> +
> >> +       /* Pop all elements */
> >> +       for (i = MAP_SIZE + MAP_SIZE/2 - 1; i >= MAP_SIZE/2; i--)
> >> +               assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == 0 &&
> >> +                      val == vals[i]);
> >> +
> >> +       /* Check that there are not elements left */
> >> +       assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == -1 &&
> >> +              errno == ENOENT);
> >> +
> >> +       /* Check that non supported functions set errno to EINVAL */
> >> +       assert(bpf_map_delete_elem(fd, NULL) == -1 && errno == EINVAL);
> >> +       assert(bpf_map_get_next_key(fd, NULL, NULL) == -1 && errno == EINVAL);
> >> +
> >> +       close(fd);
> >> +}
> >> +
> >>   #include <sys/socket.h>
> >>   #include <sys/ioctl.h>
> >>   #include <arpa/inet.h>
> >> @@ -1434,10 +1551,15 @@ static void run_all_tests(void)
> >>          test_map_wronly();
> >>
> >>          test_reuseport_array();
> >> +
> >> +       test_queuemap(0, NULL);
> >> +       test_stackmap(0, NULL);
> >>   }
> >>
> >>   int main(void)
> >>   {
> >> +       srand(time(NULL));
> >> +
> >>          map_flags = 0;
> >>          run_all_tests();
> >>
> >> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> >> index e8becca9c521..2d3c04f45530 100644
> >> --- a/tools/testing/selftests/bpf/test_progs.c
> >> +++ b/tools/testing/selftests/bpf/test_progs.c
> >> @@ -1735,8 +1735,105 @@ static void test_reference_tracking()
> >>          bpf_object__close(obj);
> >>   }
> >>
> >> +enum {
> >> +       QUEUE,
> >> +       STACK,
> >> +};
> >> +
> >> +static void test_queue_stack_map(int type)
> >> +{
> >> +       const int MAP_SIZE = 32;
> >> +       __u32 vals[MAP_SIZE], duration, retval, size, val;
> >> +       int i, err, prog_fd, map_in_fd, map_out_fd;
> >> +       char file[32], buf[128];
> >> +       struct bpf_object *obj;
> >> +       struct iphdr *iph = (void *)buf + sizeof(struct ethhdr);
> >> +
> >> +       /* Fill test values to be used */
> >> +       for (i = 0; i < MAP_SIZE; i++)
> >> +               vals[i] = rand();
> >> +
> >> +       if (type == QUEUE)
> >> +               strncpy(file, "./test_queue_map.o", sizeof(file));
> >> +       else if (type == STACK)
> >> +               strncpy(file, "./test_stack_map.o", sizeof(file));
> >> +       else
> >> +               return;
> >> +
> >> +       err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
> >> +       if (err) {
> >> +               error_cnt++;
> >> +               return;
> >> +       }
> >> +
> >> +       map_in_fd = bpf_find_map(__func__, obj, "map_in");
> >> +       if (map_in_fd < 0)
> >> +               goto out;
> >> +
> >> +       map_out_fd = bpf_find_map(__func__, obj, "map_out");
> >> +       if (map_out_fd < 0)
> >> +               goto out;
> >> +
> >> +       /* Push 32 elements to the input map */
> >> +       for (i = 0; i < MAP_SIZE; i++) {
> >> +               err = bpf_map_update_elem(map_in_fd, NULL, &vals[i], 0);
> >> +               if (err) {
> >> +                       error_cnt++;
> >> +                       goto out;
> >> +               }
> >> +       }
> >> +
> >> +       /* The eBPF program pushes iph.saddr in the output map,
> >> +        * pops the input map and saves this value in iph.daddr
> >> +        */
> >> +       for (i = 0; i < MAP_SIZE; i++) {
> >> +               if (type == QUEUE) {
> >> +                       val = vals[i];
> >> +                       pkt_v4.iph.saddr = vals[i] * 5;
> >> +               } else if (type == STACK) {
> >> +                       val = vals[MAP_SIZE - 1 - i];
> >> +                       pkt_v4.iph.saddr = vals[MAP_SIZE - 1 - i] * 5;
> >> +               }
> >> +
> >> +               err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
> >> +                                       buf, &size, &retval, &duration);
> >> +               if (err || retval || size != sizeof(pkt_v4) ||
> >> +                   iph->daddr != val)
> >> +                       break;
> >> +       }
> >> +
> >> +       CHECK(err || retval || size != sizeof(pkt_v4) || iph->daddr != val,
> >> +             "bpf_map_pop_elem",
> >> +             "err %d errno %d retval %d size %d iph->daddr %u\n",
> >> +             err, errno, retval, size, iph->daddr);
> >> +
> >> +       /* Queue is empty, program should return TC_ACT_SHOT */
> >> +       err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
> >> +                               buf, &size, &retval, &duration);
> >> +       CHECK(err || retval != 2 /* TC_ACT_SHOT */|| size != sizeof(pkt_v4),
> >> +             "check-queue-stack-map-empty",
> >> +             "err %d errno %d retval %d size %d\n",
> >> +             err, errno, retval, size);
> >> +
> >> +       /* Check that the program pushed elements correctly */
> >> +       for (i = 0; i < MAP_SIZE; i++) {
> >> +               err = bpf_map_lookup_and_delete_elem(map_out_fd, NULL, &val);
> >> +               if (err || val != vals[i] * 5)
> >> +                       break;
> >> +       }
> >> +
> >> +       CHECK(i != MAP_SIZE && (err || val != vals[i] * 5),
> >> +             "bpf_map_push_elem", "err %d value %u\n", err, val);
> >> +
> >> +out:
> >> +       pkt_v4.iph.saddr = 0;
> >> +       bpf_object__close(obj);
> >> +}
> >> +
> >>   int main(void)
> >>   {
> >> +       srand(time(NULL));
> >> +
> >>          jit_enabled = is_jit_enabled();
> >>
> >>          test_pkt_access();
> >> @@ -1757,6 +1854,8 @@ int main(void)
> >>          test_task_fd_query_rawtp();
> >>          test_task_fd_query_tp();
> >>          test_reference_tracking();
> >> +       test_queue_stack_map(QUEUE);
> >> +       test_queue_stack_map(STACK);
> >>
> >>          printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
> >>          return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
> >> diff --git a/tools/testing/selftests/bpf/test_queue_map.c b/tools/testing/selftests/bpf/test_queue_map.c
> >> new file mode 100644
> >> index 000000000000..87db1f9da33d
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/bpf/test_queue_map.c
> >> @@ -0,0 +1,4 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +// Copyright (c) 2018 Politecnico di Torino
> >> +#define MAP_TYPE BPF_MAP_TYPE_QUEUE
> >> +#include "test_queue_stack_map.h"
> >> diff --git a/tools/testing/selftests/bpf/test_queue_stack_map.h b/tools/testing/selftests/bpf/test_queue_stack_map.h
> >> new file mode 100644
> >> index 000000000000..295b9b3bc5c7
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/bpf/test_queue_stack_map.h
> >> @@ -0,0 +1,59 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +// Copyright (c) 2018 Politecnico di Torino
> >> +#include <stddef.h>
> >> +#include <string.h>
> >> +#include <linux/bpf.h>
> >> +#include <linux/if_ether.h>
> >> +#include <linux/ip.h>
> >> +#include <linux/pkt_cls.h>
> >> +#include "bpf_helpers.h"
> >> +
> >> +int _version SEC("version") = 1;
> >> +
> >> +struct bpf_map_def __attribute__ ((section("maps"), used)) map_in = {
> >> +       .type = MAP_TYPE,
> >> +       .key_size = 0,
> >> +       .value_size = sizeof(__u32),
> >> +       .max_entries = 32,
> >> +       .map_flags = 0,
> >> +};
> >> +
> >> +struct bpf_map_def __attribute__ ((section("maps"), used)) map_out = {
> >> +       .type = MAP_TYPE,
> >> +       .key_size = 0,
> >> +       .value_size = sizeof(__u32),
> >> +       .max_entries = 32,
> >> +       .map_flags = 0,
> >> +};
> >> +
> >> +SEC("test")
> >> +int _test(struct __sk_buff *skb)
> >> +{
> >> +       void *data_end = (void *)(long)skb->data_end;
> >> +       void *data = (void *)(long)skb->data;
> >> +       struct ethhdr *eth = (struct ethhdr *)(data);
> >> +       __u32 value;
> >> +       int err;
> >> +
> >> +       if (eth + 1 > data_end)
> >> +               return TC_ACT_SHOT;
> >> +
> >> +       struct iphdr *iph = (struct iphdr *)(eth + 1);
> >> +
> >> +       if (iph + 1 > data_end)
> >> +               return TC_ACT_SHOT;
> >> +
> >> +       err = bpf_map_pop_elem(&map_in, &value);
> >> +       if (err)
> >> +               return TC_ACT_SHOT;
> >> +
> >> +       iph->daddr = value;
> >> +
> >> +       err = bpf_map_push_elem(&map_out, &iph->saddr, 0);
> >> +       if (err)
> >> +               return TC_ACT_SHOT;
> >> +
> >> +       return TC_ACT_OK;
> >> +}
> >> +
> >> +char _license[] SEC("license") = "GPL";
> >> diff --git a/tools/testing/selftests/bpf/test_stack_map.c b/tools/testing/selftests/bpf/test_stack_map.c
> >> new file mode 100644
> >> index 000000000000..31c3880e6da0
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/bpf/test_stack_map.c
> >> @@ -0,0 +1,4 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +// Copyright (c) 2018 Politecnico di Torino
> >> +#define MAP_TYPE BPF_MAP_TYPE_STACK
> >> +#include "test_queue_stack_map.h"
> >>

^ permalink raw reply

* [bpf-next v3 2/2] bpf: test_maps add a test to catch kcm + sockmap
From: John Fastabend @ 2018-10-18 20:58 UTC (permalink / raw)
  To: ast, daniel, eric.dumazet; +Cc: john.fastabend, netdev
In-Reply-To: <1539896316-13403-1-git-send-email-john.fastabend@gmail.com>

Adding a socket to both sockmap and kcm is not supported due to
collision on sk_user_data usage.

If selftests is run without KCM support we will issue a warning
and continue with the tests.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 tools/testing/selftests/bpf/Makefile      |  2 +-
 tools/testing/selftests/bpf/sockmap_kcm.c | 14 +++++++
 tools/testing/selftests/bpf/test_maps.c   | 64 ++++++++++++++++++++++++++++++-
 3 files changed, 77 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/sockmap_kcm.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index d99dd6f..f290554 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -28,7 +28,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
 
 TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test_obj_id.o \
 	test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o     \
-	sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o test_tracepoint.o \
+	sockmap_verdict_prog.o sockmap_kcm.o dev_cgroup.o sample_ret0.o test_tracepoint.o \
 	test_l4lb_noinline.o test_xdp_noinline.o test_stacktrace_map.o \
 	sample_map_ret0.o test_tcpbpf_kern.o test_stacktrace_build_id.o \
 	sockmap_tcp_msg_prog.o connect4_prog.o connect6_prog.o test_adjust_tail.o \
diff --git a/tools/testing/selftests/bpf/sockmap_kcm.c b/tools/testing/selftests/bpf/sockmap_kcm.c
new file mode 100644
index 0000000..4377adc
--- /dev/null
+++ b/tools/testing/selftests/bpf/sockmap_kcm.c
@@ -0,0 +1,14 @@
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+#include "bpf_util.h"
+#include "bpf_endian.h"
+
+int _version SEC("version") = 1;
+
+SEC("socket_kcm")
+int bpf_prog1(struct __sk_buff *skb)
+{
+	return skb->len;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 9b552c0..be20f1d 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -20,6 +20,7 @@
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <linux/bpf.h>
+#include <linux/kcm.h>
 
 #include <bpf/bpf.h>
 #include <bpf/libbpf.h>
@@ -479,14 +480,16 @@ static void test_devmap(int task, void *data)
 #define SOCKMAP_PARSE_PROG "./sockmap_parse_prog.o"
 #define SOCKMAP_VERDICT_PROG "./sockmap_verdict_prog.o"
 #define SOCKMAP_TCP_MSG_PROG "./sockmap_tcp_msg_prog.o"
+#define KCM_PROG "./sockmap_kcm.o"
 static void test_sockmap(int tasks, void *data)
 {
 	struct bpf_map *bpf_map_rx, *bpf_map_tx, *bpf_map_msg, *bpf_map_break;
-	int map_fd_msg = 0, map_fd_rx = 0, map_fd_tx = 0, map_fd_break;
+	int map_fd_msg = 0, map_fd_rx = 0, map_fd_tx = 0, map_fd_break, kcm;
 	int ports[] = {50200, 50201, 50202, 50204};
 	int err, i, fd, udp, sfd[6] = {0xdeadbeef};
 	u8 buf[20] = {0x0, 0x5, 0x3, 0x2, 0x1, 0x0};
-	int parse_prog, verdict_prog, msg_prog;
+	int parse_prog, verdict_prog, msg_prog, kcm_prog;
+	struct kcm_attach attach_info;
 	struct sockaddr_in addr;
 	int one = 1, s, sc, rc;
 	struct bpf_object *obj;
@@ -744,6 +747,62 @@ static void test_sockmap(int tasks, void *data)
 		goto out_sockmap;
 	}
 
+	/* Test adding a KCM socket into map */
+#define AF_KCM 41
+	kcm = socket(AF_KCM, SOCK_DGRAM, KCMPROTO_CONNECTED);
+	if (kcm == -1) {
+		printf("Warning, KCM+Sockmap could not be tested.\n");
+		goto skip_kcm;
+	}
+
+	err = bpf_prog_load(KCM_PROG,
+			    BPF_PROG_TYPE_SOCKET_FILTER,
+			    &obj, &kcm_prog);
+	if (err) {
+		printf("Failed to load SK_SKB parse prog\n");
+		goto out_sockmap;
+	}
+
+	i = 2;
+	memset(&attach_info, 0, sizeof(attach_info));
+	attach_info.fd = sfd[i];
+	attach_info.bpf_fd = kcm_prog;
+	err = ioctl(kcm, SIOCKCMATTACH, &attach_info);
+	if (!err) {
+		perror("Failed KCM attached to sockmap fd: ");
+		goto out_sockmap;
+	}
+
+	err = bpf_map_delete_elem(fd, &i);
+	if (err) {
+		printf("Failed delete sockmap from empty map %i %i\n", err, errno);
+		goto out_sockmap;
+	}
+
+	err = ioctl(kcm, SIOCKCMATTACH, &attach_info);
+	if (err) {
+		perror("Failed KCM attach");
+		goto out_sockmap;
+	}
+
+	err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
+	if (!err) {
+		printf("Failed sockmap attached KCM sock!\n");
+		goto out_sockmap;
+	}
+	err = ioctl(kcm, SIOCKCMUNATTACH, &attach_info);
+	if (err) {
+		printf("Failed detach KCM sock!\n");
+		goto out_sockmap;
+	}
+
+	err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
+	if (err) {
+		printf("Failed post-kcm update sockmap '%i:%i'\n",
+		       i, sfd[i]);
+		goto out_sockmap;
+	}
+
 	/* Test map update elem afterwards fd lives in fd and map_fd */
 	for (i = 2; i < 6; i++) {
 		err = bpf_map_update_elem(map_fd_rx, &i, &sfd[i], BPF_ANY);
@@ -776,6 +835,7 @@ static void test_sockmap(int tasks, void *data)
 		}
 	}
 
+skip_kcm:
 	/* Put sfd[2] (sending fd below) into msg map to test sendmsg bpf */
 	i = 0;
 	err = bpf_map_update_elem(map_fd_msg, &i, &sfd[2], BPF_ANY);
-- 
1.9.1

^ permalink raw reply related

* [bpf-next v3 1/2] bpf: skmsg, fix psock create on existing kcm/tls port
From: John Fastabend @ 2018-10-18 20:58 UTC (permalink / raw)
  To: ast, daniel, eric.dumazet; +Cc: john.fastabend, netdev
In-Reply-To: <1539896316-13403-1-git-send-email-john.fastabend@gmail.com>

Before using the psock returned by sk_psock_get() when adding it to a
sockmap we need to ensure it is actually a sockmap based psock.
Previously we were only checking this after incrementing the reference
counter which was an error. This resulted in a slab-out-of-bounds
error when the psock was not actually a sockmap type.

This moves the check up so the reference counter is only used
if it is a sockmap psock.

Eric reported the following KASAN BUG,

BUG: KASAN: slab-out-of-bounds in atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
BUG: KASAN: slab-out-of-bounds in refcount_inc_not_zero_checked+0x97/0x2f0 lib/refcount.c:120
Read of size 4 at addr ffff88019548be58 by task syz-executor4/22387

CPU: 1 PID: 22387 Comm: syz-executor4 Not tainted 4.19.0-rc7+ #264
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
 print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
 kasan_report_error mm/kasan/report.c:354 [inline]
 kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
 check_memory_region_inline mm/kasan/kasan.c:260 [inline]
 check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267
 kasan_check_read+0x11/0x20 mm/kasan/kasan.c:272
 atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
 refcount_inc_not_zero_checked+0x97/0x2f0 lib/refcount.c:120
 sk_psock_get include/linux/skmsg.h:379 [inline]
 sock_map_link.isra.6+0x41f/0xe30 net/core/sock_map.c:178
 sock_hash_update_common+0x19b/0x11e0 net/core/sock_map.c:669
 sock_hash_update_elem+0x306/0x470 net/core/sock_map.c:738
 map_update_elem+0x819/0xdf0 kernel/bpf/syscall.c:818

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
---
 include/linux/skmsg.h | 25 ++++++++++++++++++++-----
 net/core/sock_map.c   | 11 ++++++-----
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index 22347b0..84e1886 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -270,11 +270,6 @@ static inline struct sk_psock *sk_psock(const struct sock *sk)
 	return rcu_dereference_sk_user_data(sk);
 }
 
-static inline bool sk_has_psock(struct sock *sk)
-{
-	return sk_psock(sk) != NULL && sk->sk_prot->recvmsg == tcp_bpf_recvmsg;
-}
-
 static inline void sk_psock_queue_msg(struct sk_psock *psock,
 				      struct sk_msg *msg)
 {
@@ -374,6 +369,26 @@ static inline bool sk_psock_test_state(const struct sk_psock *psock,
 	return test_bit(bit, &psock->state);
 }
 
+static inline struct sk_psock *sk_psock_get_checked(struct sock *sk)
+{
+	struct sk_psock *psock;
+
+	rcu_read_lock();
+	psock = sk_psock(sk);
+	if (psock) {
+		if (sk->sk_prot->recvmsg != tcp_bpf_recvmsg) {
+			psock = ERR_PTR(-EBUSY);
+			goto out;
+		}
+
+		if (!refcount_inc_not_zero(&psock->refcnt))
+			psock = ERR_PTR(-EBUSY);
+	}
+out:
+	rcu_read_unlock();
+	return psock;
+}
+
 static inline struct sk_psock *sk_psock_get(struct sock *sk)
 {
 	struct sk_psock *psock;
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 3c0e44c..be6092a 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -175,12 +175,13 @@ static int sock_map_link(struct bpf_map *map, struct sk_psock_progs *progs,
 		}
 	}
 
-	psock = sk_psock_get(sk);
+	psock = sk_psock_get_checked(sk);
+	if (IS_ERR(psock)) {
+		ret = PTR_ERR(psock);
+		goto out_progs;
+	}
+
 	if (psock) {
-		if (!sk_has_psock(sk)) {
-			ret = -EBUSY;
-			goto out_progs;
-		}
 		if ((msg_parser && READ_ONCE(psock->progs.msg_parser)) ||
 		    (skb_progs  && READ_ONCE(psock->progs.skb_parser))) {
 			sk_psock_put(sk, psock);
-- 
1.9.1

^ permalink raw reply related

* [bpf-next v3 0/2] Fix kcm + sockmap by checking psock type
From: John Fastabend @ 2018-10-18 20:58 UTC (permalink / raw)
  To: ast, daniel, eric.dumazet; +Cc: john.fastabend, netdev

We check if the sk_user_data (the psock in skmsg) is in fact a sockmap
type to late, after we read the refcnt which is an error. This
series moves the check up before reading refcnt and also adds a test
to test_maps to test trying to add a KCM socket into a sockmap.

While reviewig this code I also found an issue with KCM and kTLS
where each uses sk_data_ready hooks and associated stream parser
breaking expectations in kcm, ktls or both. But that fix will need
to go to net.

Thanks to Eric for reporting.

v2: Fix up file +/- my scripts lost track of them
v3: return EBUSY if refcnt is zero

John Fastabend (2):
  bpf: skmsg, fix psock create on existing kcm/tls port
  bpf: test_maps add a test to catch kcm + sockmap

 include/linux/skmsg.h                     | 25 +++++++++---
 net/core/sock_map.c                       | 11 +++---
 tools/testing/selftests/bpf/Makefile      |  2 +-
 tools/testing/selftests/bpf/sockmap_kcm.c | 14 +++++++
 tools/testing/selftests/bpf/test_maps.c   | 64 ++++++++++++++++++++++++++++++-
 5 files changed, 103 insertions(+), 13 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/sockmap_kcm.c

-- 
1.9.1

^ permalink raw reply

* [PATCH bpf-next 2/2] samples: bpf: get ifindex from ifname
From: Matteo Croce @ 2018-10-18 20:47 UTC (permalink / raw)
  To: netdev; +Cc: Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20181018204709.29547-2-mcroce@redhat.com>

Find the ifindex via ioctl(SIOCGIFINDEX) instead of requiring the
numeric ifindex.

Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
 samples/bpf/xdp1_user.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index 4f3d824fc044..a1d0c5dcee9c 100644
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -15,6 +15,9 @@
 #include <unistd.h>
 #include <libgen.h>
 #include <sys/resource.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <linux/if.h>
 
 #include "bpf_util.h"
 #include "bpf/bpf.h"
@@ -59,7 +62,7 @@ static void poll_stats(int map_fd, int interval)
 static void usage(const char *prog)
 {
 	fprintf(stderr,
-		"usage: %s [OPTS] IFINDEX\n\n"
+		"usage: %s [OPTS] IFACE\n\n"
 		"OPTS:\n"
 		"    -S    use skb-mode\n"
 		"    -N    enforce native mode\n",
@@ -74,9 +77,11 @@ int main(int argc, char **argv)
 	};
 	const char *optstr = "SN";
 	int prog_fd, map_fd, opt;
+	struct ifreq ifr = { 0 };
 	struct bpf_object *obj;
 	struct bpf_map *map;
 	char filename[256];
+	int sock;
 
 	while ((opt = getopt(argc, argv, optstr)) != -1) {
 		switch (opt) {
@@ -102,7 +107,24 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
-	ifindex = strtoul(argv[optind], NULL, 0);
+	sock = socket(AF_UNIX, SOCK_DGRAM, 0);
+	if (sock == -1) {
+		perror("socket");
+		return 1;
+	}
+
+	if (strlen(argv[optind]) >= IFNAMSIZ) {
+		printf("invalid ifname '%s'\n", argv[optind]);
+		return 1;
+	}
+
+	strcpy(ifr.ifr_name, argv[optind]);
+	if (ioctl(sock, SIOCGIFINDEX, &ifr) < 0) {
+		perror("SIOCGIFINDEX");
+		return 1;
+	}
+	close(sock);
+	ifindex = ifr.ifr_ifindex;
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
 	prog_load_attr.file = filename;
-- 
2.19.1

^ permalink raw reply related

* [PATCH bpf-next 1/2] samples: bpf: improve xdp1 example
From: Matteo Croce @ 2018-10-18 20:47 UTC (permalink / raw)
  To: netdev; +Cc: Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20181018204709.29547-1-mcroce@redhat.com>

Store only the total packet count for every protocol, instead of the
whole per-cpu array.
Use bpf_map_get_next_key() to iterate the map, instead of looking up
all the protocols.

Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
 samples/bpf/xdp1_user.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index b02c531510ed..4f3d824fc044 100644
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -34,26 +34,24 @@ static void int_exit(int sig)
 static void poll_stats(int map_fd, int interval)
 {
 	unsigned int nr_cpus = bpf_num_possible_cpus();
-	const unsigned int nr_keys = 256;
-	__u64 values[nr_cpus], prev[nr_keys][nr_cpus];
-	__u32 key;
+	__u64 values[nr_cpus], prev[UINT8_MAX] = { 0 };
 	int i;
 
-	memset(prev, 0, sizeof(prev));
-
 	while (1) {
+		__u32 key = UINT32_MAX;
+
 		sleep(interval);
 
-		for (key = 0; key < nr_keys; key++) {
+		while (bpf_map_get_next_key(map_fd, &key, &key) != -1) {
 			__u64 sum = 0;
 
 			assert(bpf_map_lookup_elem(map_fd, &key, values) == 0);
 			for (i = 0; i < nr_cpus; i++)
-				sum += (values[i] - prev[key][i]);
-			if (sum)
+				sum += values[i];
+			if (sum > prev[key])
 				printf("proto %u: %10llu pkt/s\n",
-				       key, sum / interval);
-			memcpy(prev[key], values, sizeof(values));
+				       key, (sum - prev[key]) / interval);
+			prev[key] = sum;
 		}
 	}
 }
-- 
2.19.1

^ permalink raw reply related

* [PATCH bpf-next 0/2] sample: xdp1 improvements
From: Matteo Croce @ 2018-10-18 20:47 UTC (permalink / raw)
  To: netdev; +Cc: Alexei Starovoitov, Daniel Borkmann

Small improvements to improve the readability and easiness
to use of the xdp1 sample.

Matteo Croce (2):
  samples: bpf: improve xdp1 example
  samples: bpf: get ifindex from ifname

 samples/bpf/xdp1_user.c | 44 ++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 12 deletions(-)

-- 
2.19.1

^ permalink raw reply

* [net-next 7/8] net/mlx5: Added "per_lane_error_counters" cap bit to PCAM
From: Saeed Mahameed @ 2018-10-18 20:39 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Shay Agroskin, Saeed Mahameed
In-Reply-To: <20181018203907.25149-1-saeedm@mellanox.com>

From: Shay Agroskin <shayag@mellanox.com>

Added "Per lane raw errors" capability bit in
Ports Capabilities Mask (PCAM) enhanced features
layout.

This bit determines if the fields "phy_raw_errors_laneX"
in "Physical Layer statistical" counters group are supported.

Signed-off-by: Shay Agroskin <shayag@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 include/linux/mlx5/mlx5_ifc.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index 47b09a742ae5..dbff9ff28f2c 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -8140,7 +8140,8 @@ struct mlx5_ifc_pcam_enhanced_features_bits {
 	u8         rx_icrc_encapsulated_counter[0x1];
 	u8	   reserved_at_6e[0x8];
 	u8         pfcc_mask[0x1];
-	u8         reserved_at_77[0x4];
+	u8         reserved_at_77[0x3];
+	u8         per_lane_error_counters[0x1];
 	u8         rx_buffer_fullness_counters[0x1];
 	u8         ptys_connector_type[0x1];
 	u8         reserved_at_7d[0x1];
-- 
2.17.2

^ permalink raw reply related

* [net-next 8/8] net/mlx5e: Added 'raw_errors_laneX' fields to ethtool statistics
From: Saeed Mahameed @ 2018-10-18 20:39 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Shay Agroskin, Saeed Mahameed
In-Reply-To: <20181018203907.25149-1-saeedm@mellanox.com>

From: Shay Agroskin <shayag@mellanox.com>

These are counters for errors received on rx side, such as
FEC errors.

Signed-off-by: Shay Agroskin <shayag@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 .../ethernet/mellanox/mlx5/core/en_stats.c    | 46 +++++++++++++++++--
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index 1c006869a642..1e55b9c27ffc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -614,46 +614,82 @@ static const struct counter_desc pport_phy_statistical_stats_desc[] = {
 	{ "rx_corrected_bits_phy", PPORT_PHY_STATISTICAL_OFF(phy_corrected_bits) },
 };
 
-#define NUM_PPORT_PHY_STATISTICAL_COUNTERS ARRAY_SIZE(pport_phy_statistical_stats_desc)
+static const struct counter_desc
+pport_phy_statistical_err_lanes_stats_desc[] = {
+	{ "rx_err_lane_0_phy", PPORT_PHY_STATISTICAL_OFF(phy_corrected_bits_lane0) },
+	{ "rx_err_lane_1_phy", PPORT_PHY_STATISTICAL_OFF(phy_corrected_bits_lane1) },
+	{ "rx_err_lane_2_phy", PPORT_PHY_STATISTICAL_OFF(phy_corrected_bits_lane2) },
+	{ "rx_err_lane_3_phy", PPORT_PHY_STATISTICAL_OFF(phy_corrected_bits_lane3) },
+};
+
+#define NUM_PPORT_PHY_STATISTICAL_COUNTERS \
+	ARRAY_SIZE(pport_phy_statistical_stats_desc)
+#define NUM_PPORT_PHY_STATISTICAL_PER_LANE_COUNTERS \
+	ARRAY_SIZE(pport_phy_statistical_err_lanes_stats_desc)
 
 static int mlx5e_grp_phy_get_num_stats(struct mlx5e_priv *priv)
 {
+	struct mlx5_core_dev *mdev = priv->mdev;
+	int num_stats;
+
 	/* "1" for link_down_events special counter */
-	return MLX5_CAP_PCAM_FEATURE((priv)->mdev, ppcnt_statistical_group) ?
-		NUM_PPORT_PHY_STATISTICAL_COUNTERS + 1 : 1;
+	num_stats = 1;
+
+	num_stats += MLX5_CAP_PCAM_FEATURE(mdev, ppcnt_statistical_group) ?
+		     NUM_PPORT_PHY_STATISTICAL_COUNTERS : 0;
+
+	num_stats += MLX5_CAP_PCAM_FEATURE(mdev, per_lane_error_counters) ?
+		     NUM_PPORT_PHY_STATISTICAL_PER_LANE_COUNTERS : 0;
+
+	return num_stats;
 }
 
 static int mlx5e_grp_phy_fill_strings(struct mlx5e_priv *priv, u8 *data,
 				      int idx)
 {
+	struct mlx5_core_dev *mdev = priv->mdev;
 	int i;
 
 	strcpy(data + (idx++) * ETH_GSTRING_LEN, "link_down_events_phy");
 
-	if (!MLX5_CAP_PCAM_FEATURE((priv)->mdev, ppcnt_statistical_group))
+	if (!MLX5_CAP_PCAM_FEATURE(mdev, ppcnt_statistical_group))
 		return idx;
 
 	for (i = 0; i < NUM_PPORT_PHY_STATISTICAL_COUNTERS; i++)
 		strcpy(data + (idx++) * ETH_GSTRING_LEN,
 		       pport_phy_statistical_stats_desc[i].format);
+
+	if (MLX5_CAP_PCAM_FEATURE(mdev, per_lane_error_counters))
+		for (i = 0; i < NUM_PPORT_PHY_STATISTICAL_PER_LANE_COUNTERS; i++)
+			strcpy(data + (idx++) * ETH_GSTRING_LEN,
+			       pport_phy_statistical_err_lanes_stats_desc[i].format);
+
 	return idx;
 }
 
 static int mlx5e_grp_phy_fill_stats(struct mlx5e_priv *priv, u64 *data, int idx)
 {
+	struct mlx5_core_dev *mdev = priv->mdev;
 	int i;
 
 	/* link_down_events_phy has special handling since it is not stored in __be64 format */
 	data[idx++] = MLX5_GET(ppcnt_reg, priv->stats.pport.phy_counters,
 			       counter_set.phys_layer_cntrs.link_down_events);
 
-	if (!MLX5_CAP_PCAM_FEATURE((priv)->mdev, ppcnt_statistical_group))
+	if (!MLX5_CAP_PCAM_FEATURE(mdev, ppcnt_statistical_group))
 		return idx;
 
 	for (i = 0; i < NUM_PPORT_PHY_STATISTICAL_COUNTERS; i++)
 		data[idx++] =
 			MLX5E_READ_CTR64_BE(&priv->stats.pport.phy_statistical_counters,
 					    pport_phy_statistical_stats_desc, i);
+
+	if (MLX5_CAP_PCAM_FEATURE(mdev, per_lane_error_counters))
+		for (i = 0; i < NUM_PPORT_PHY_STATISTICAL_PER_LANE_COUNTERS; i++)
+			data[idx++] =
+				MLX5E_READ_CTR64_BE(&priv->stats.pport.phy_statistical_counters,
+						    pport_phy_statistical_err_lanes_stats_desc,
+						    i);
 	return idx;
 }
 
-- 
2.17.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox