* Re: [PATCH v5 bpf-next 06/10] tracepoint: compute num_args at build time
From: Steven Rostedt @ 2018-03-26 18:11 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Mathieu Desnoyers, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api,
Frank Ch. Eigler
In-Reply-To: <17073efa-d833-7348-bef1-79376ad43bc6@fb.com>
On Mon, 26 Mar 2018 10:55:51 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> An email ago you were ok to s/return/return NULL/ in your out-of-tree
> module, but now flip flop to add new function approach just to
> reduce the work you need to do in lttng?
> We're not talking about changing __kmalloc signature here.
> My patch extends for_each_kernel_tracepoint() api similar to other
> for_each_*() iterators and improves possible uses of it.
Alexei, do you have another use case for using
for_each_kernel_tracepoint() other than the find_tp? If so, then I'm
sure Mathieu can handle the change.
But I think it's cleaner to add a tracepoint_find_by_name() function.
If you come up with another use case for using the for_each* function
then we'll consider changing it then.
> One thing is to be nice to out-of-tree and do not break them
> for no reason, but arguing that kernel shouldn't add a minor extension
> to for_each_kernel_tracepoint() api is really taking the whole thing
> to next level.
That's not the point. I disagree with the reason for the change, and
believe that it would be cleaner to add a find_by_name() function.
Which would make your patch set even cleaner.
Instead of having in the bpf code:
static void *__find_tp(struct tracepoint *tp, void *priv)
{
char *name = priv;
if (!strcmp(tp->name, name))
return tp;
return NULL;
}
[..]
tp = for_each_kernel_tracepoint(__find_tp, tp_name);
if (!tp)
return -ENOENT;
You would simply have:
tp = tracepoint_find_by_name(tp_name);
if (!tp)
return -ENOENT;
That would make the code more obvious to what it is doing. And this
does not impede your patch set at all.
-- Steve
^ permalink raw reply
* Re: [PATCH] Documentation/isdn: check and fix dead links ...
From: Paul Bolle @ 2018-03-26 18:10 UTC (permalink / raw)
To: Sanjeev Gupta; +Cc: David Miller, corbet, isdn, netdev
In-Reply-To: <1522087557.2883.12.camel@tiscali.nl>
On Mon, 2018-03-26 at 20:05 +0200, Paul Bolle wrote:
> Your patch never hit my inbox. The joy of email! Anyhow, the diff on spinic's
> netdev archive suggests you left (at least) one reference to http://gigaset307
> x.sourceforge.net/ in the documentation.
>
> If you'd be so kind to send a follow up patch to clean up that reference too
> that would be appreciated.
Never mind. That doesn't actually work over https.
Meh!
Paul Bolle
^ permalink raw reply
* Re: [PATCH] Documentation/isdn: check and fix dead links ...
From: Paul Bolle @ 2018-03-26 18:05 UTC (permalink / raw)
To: Sanjeev Gupta; +Cc: David Miller, corbet, isdn, netdev
In-Reply-To: <20180326.123125.1782554768511538474.davem@davemloft.net>
Sanjeev,
On Mon, 2018-03-26 at 12:31 -0400, David Miller wrote:
> > and switch to https where possible.
> >
> > All links have been eyeballed to verify that the domains have
> > not changed, etc.
> >
> > Signed-off-by: Sanjeev Gupta <ghane0@gmail.com>
>
> Applied, thank you.
Your patch never hit my inbox. The joy of email! Anyhow, the diff on spinic's
netdev archive suggests you left (at least) one reference to http://gigaset307
x.sourceforge.net/ in the documentation.
If you'd be so kind to send a follow up patch to clean up that reference too
that would be appreciated.
Thanks!
Paul Bolle
^ permalink raw reply
* Re: NETDEV WATCHDOG: eth0 (tg3): transmit queue 0 timed out
From: Borislav Petkov @ 2018-03-26 18:05 UTC (permalink / raw)
To: Satish Baddipadige
Cc: Siva Reddy Kallam, Linux Netdev List, Prashant Sreedharan,
Michael Chan, Linux Kernel list
In-Reply-To: <CA+oDAx8CCW8rUfdqSZJpKFzHUBocY_Q1xbJXB0y8qa6qP0iUAg@mail.gmail.com>
On Tue, Mar 20, 2018 at 11:41:06AM +0530, Satish Baddipadige wrote:
> Can you please test the attached patch?
Well, the network connection just died with it. It didn't fire the
netdev watchdog but I still had to down and up eth0 in order to continue
using it. ssh connection into the box survived so I didn't have to login
again but it still died intermittently.
I'll keep playing with it to see if I'll catch some sort of splat...
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* Re: [PATCH v5 bpf-next 06/10] tracepoint: compute num_args at build time
From: Alexei Starovoitov @ 2018-03-26 17:55 UTC (permalink / raw)
To: Mathieu Desnoyers, rostedt
Cc: David S. Miller, Daniel Borkmann, Linus Torvalds, Peter Zijlstra,
netdev, kernel-team, linux-api, Frank Ch. Eigler
In-Reply-To: <1218234422.259.1522083422808.JavaMail.zimbra@efficios.com>
On 3/26/18 9:57 AM, Mathieu Desnoyers wrote:
> ----- On Mar 26, 2018, at 12:35 PM, rostedt rostedt@goodmis.org wrote:
>
>> On Mon, 26 Mar 2018 09:25:07 -0700
>> Alexei Starovoitov <ast@fb.com> wrote:
>>
>>> commit log of patch 6 states:
>>>
>>> "for_each_tracepoint_range() api has no users inside the kernel.
>>> Make it more useful with ability to stop for_each() loop depending
>>> via callback return value.
>>> In such form it's used in subsequent patch."
>>>
>>> and in patch 7:
>>>
>>> +static void *__find_tp(struct tracepoint *tp, void *priv)
>>> +{
>>> + char *name = priv;
>>> +
>>> + if (!strcmp(tp->name, name))
>>> + return tp;
>>> + return NULL;
>>> +}
>>> ...
>>> + struct tracepoint *tp;
>>> ...
>>> + tp = for_each_kernel_tracepoint(__find_tp, tp_name);
>>> + if (!tp)
>>> + return -ENOENT;
>>>
>>> still not obvious?
>>
>> Please just create a new function called tracepoint_find_by_name(), and
>> use that. I don't see any benefit in using a for_each* function for
>> such a simple routine. Not to mention, you then don't need to know the
>> internals of a tracepoint in kernel/bpf/syscall.c.
>
> Steven's approach is fine by me, considering there should never be duplicated
> tracepoint definitions (it emits a __tracepoint_##name symbol which would cause
> multiple symbols definition errors at link time if there are more than
> a single definition per tracepoint name in the core kernel). The exported
> API should probably be named "kernel_tracepoint_find_by_name()" or something
> similar, thus indicating that it only lookup tracepoints in the core kernel.
An email ago you were ok to s/return/return NULL/ in your out-of-tree
module, but now flip flop to add new function approach just to
reduce the work you need to do in lttng?
We're not talking about changing __kmalloc signature here.
My patch extends for_each_kernel_tracepoint() api similar to other
for_each_*() iterators and improves possible uses of it.
It can event help lltng.
lttng-tracepoint.c is doing:
for_each_kernel_tracepoint(lttng_kernel_tracepoint_add, &ret);
to copy kernel tracepoints into its own hash table.
Only to later do:
/*
* Get tracepoint if the tracepoint is present in the tracepoint hash
table.
* Must be called with lttng_tracepoint_mutex held.
* Returns NULL if not present.
*/
static
struct tracepoint_entry *get_tracepoint(const char *name)
{
struct hlist_head *head;
struct tracepoint_entry *e;
u32 hash = jhash(name, strlen(name), 0);
head = &tracepoint_table[hash & (TRACEPOINT_TABLE_SIZE - 1)];
lttng_hlist_for_each_entry(e, head, hlist) {
if (!strcmp(name, e->name))
return e;
}
return NULL;
}
this use case potentially can be reimplemented in lttng with
this extended for_each_kernel_tracepoint() api.
Like I do on bpf side with very similar callback:
+static void *__find_tp(struct tracepoint *tp, void *priv)
+{
+ char *name = priv;
+
+ if (!strcmp(tp->name, name))
+ return tp;
+ return NULL;
+}
> Which brings the next question: what are Alexei's plan to handle tracepoints
> in modules, considering module load/unload scenarios ? The tracepoint API
> has module notifiers for this, but it does not appear to be used in this
> patch series.
correct. this set deals with in-kernel tracepoints only.
No attempt to do anything with tracepoints inside modules.
but your question brings another question:
why kernel/module.c have this code:
mod->tracepoints_ptrs = section_objs(info, "__tracepoints_ptrs"
sizeof(*mod->tracepoints_ptrs),
&mod->num_tracepoints);
and the whole module tracepoint notifier logic
that is only used by out-of-tree ?
I didn't realize so much of kernel code was taken hostage by lttng.
One thing is to be nice to out-of-tree and do not break them
for no reason, but arguing that kernel shouldn't add a minor extension
to for_each_kernel_tracepoint() api is really taking the whole thing
to next level.
Also I hope you noticed that the patch is doing:
+++ b/include/linux/tracepoint-defs.h
@@ -33,6 +33,7 @@ struct tracepoint {
int (*regfunc)(void);
void (*unregfunc)(void);
struct tracepoint_func __rcu *funcs;
+ u32 num_args;
};
To make sure that bpf programs are safe I need to do a static check
in the verifier that programs don't access arguments beyond
those specified by the tracepoint.
That was mentioned in the commit log of patch 6 too:
"
compute number of arguments passed into tracepoint
at compile time and store it as part of 'struct tracepoint'.
The number is necessary to check safety of bpf program access that
is coming in subsequent patch.
"
I suspect you will have the same objection?
It breaks out-of-tree modules?!
^ permalink raw reply
* Re: syzbot rcu/debugobjects warning
From: Guillaume Nault @ 2018-03-26 17:38 UTC (permalink / raw)
To: Joel Fernandes
Cc: Thomas Gleixner, Paul McKenney, LKML, Todd Poynor,
open list:BPF (Safe dynamic programs and tools), Ben Hutchings,
Greg Kroah-Hartman
In-Reply-To: <CAJWu+or1YJEZX8R1fwJrb=cUc05_uzwZHJhD5-SMEycUO7W1mQ@mail.gmail.com>
On Sat, Mar 24, 2018 at 11:29:42PM -0700, Joel Fernandes wrote:
> On Fri, Mar 23, 2018 at 1:41 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Fri, 23 Mar 2018, Joel Fernandes wrote:
> >> On Fri, Mar 23, 2018 at 2:11 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > On Thu, 22 Mar 2018, Joel Fernandes wrote:
> >> Sorry. Here is the raw crash log: https://pastebin.com/raw/puvh0cXE
> >> (The kernel logs are toward the end with the above).
> >
> > And that is interesting:
> >
> > [ 150.629667] <IRQ> [ 150.631700] [<ffffffff81d96069>] dump_stack+0xc1/0x128
> > [ 150.637051] [<ffffffff81dfefb6>] ? __debug_object_init+0x526/0xc40
> > [ 150.643431] [<ffffffff8142fbd1>] panic+0x1bc/0x3a8
> > [ 150.648416] [<ffffffff8142fa15>] ? percpu_up_read_preempt_enable.constprop.53+0xd7/0xd7
> > [ 150.656611] [<ffffffff81430835>] ? load_image_and_restore+0xf9/0xf9
> > [ 150.663070] [<ffffffff81269efd>] ? vprintk_default+0x1d/0x30
> > [ 150.668925] [<ffffffff81131879>] ? __warn+0x1a9/0x1e0
> > [ 150.674170] [<ffffffff81dfefb6>] ? __debug_object_init+0x526/0xc40
> > [ 150.680543] [<ffffffff81131894>] __warn+0x1c4/0x1e0
> > [ 150.685614] [<ffffffff81131afc>] warn_slowpath_null+0x2c/0x40
> > [ 150.691972] [<ffffffff81dfefb6>] __debug_object_init+0x526/0xc40
> > [ 150.698174] [<ffffffff81dfea90>] ? debug_object_fixup+0x30/0x30
> > [ 150.704283] [<ffffffff81dff709>] debug_object_init_on_stack+0x19/0x20
> > [ 150.710917] [<ffffffff81287a93>] __wait_rcu_gp+0x93/0x1b0
> > [ 150.716508] [<ffffffff81290251>] synchronize_rcu.part.65+0x101/0x110
> > [ 150.723054] [<ffffffff81290150>] ? rcu_pm_notify+0xc0/0xc0
> > [ 150.728735] [<ffffffff81292bc0>] ? __call_rcu.constprop.72+0x910/0x910
> > [ 150.735459] [<ffffffff81235221>] ? __lock_is_held+0xa1/0xf0
> > [ 150.741223] [<ffffffff81290287>] synchronize_rcu+0x27/0x90
> >
> > So this calls synchronize_rcu from a rcu callback. That's a nono. This is
> > on the back of an interrupt in softirq context and __wait_rcu_gp() can
> > sleep, which is obviously a bad idea in softirq context....
> >
> > Cc'ed netdev ....
> >
> > And that also explains the debug object splat because this is not running
> > on the task stack. It's running on the softirq stack ....
> >
> > [ 150.746908] [<ffffffff83588b35>] __l2tp_session_unhash+0x3d5/0x550
> > [ 150.753281] [<ffffffff8358891f>] ? __l2tp_session_unhash+0x1bf/0x550
> > [ 150.759828] [<ffffffff8114596a>] ? __local_bh_enable_ip+0x6a/0xd0
> > [ 150.766123] [<ffffffff8358ddb0>] ? l2tp_udp_encap_recv+0xd90/0xd90
> > [ 150.772497] [<ffffffff83588e97>] l2tp_tunnel_closeall+0x1e7/0x3a0
> > [ 150.778782] [<ffffffff835897be>] l2tp_tunnel_destruct+0x30e/0x5a0
> > [ 150.785067] [<ffffffff8358965a>] ? l2tp_tunnel_destruct+0x1aa/0x5a0
> > [ 150.791537] [<ffffffff835894b0>] ? l2tp_tunnel_del_work+0x460/0x460
> > [ 150.797997] [<ffffffff82ee8053>] __sk_destruct+0x53/0x570
> > [ 150.803588] [<ffffffff81293918>] rcu_process_callbacks+0x898/0x1300
> > [ 150.810048] [<ffffffff812939f7>] ? rcu_process_callbacks+0x977/0x1300
> > [ 150.816684] [<ffffffff82ee8000>] ? __sk_dst_check+0x240/0x240
> > [ 150.822625] [<ffffffff838be5d6>] __do_softirq+0x206/0x951
> > [ 150.828223] [<ffffffff81147315>] irq_exit+0x165/0x190
> > [ 150.833557] [<ffffffff838bd1eb>] smp_apic_timer_interrupt+0x7b/0xa0
> > [ 150.840018] [<ffffffff838b9470>] apic_timer_interrupt+0xa0/0xb0
> > [ 150.846132] <EOI> [ 150.848166] [<ffffffff838b6756>] ? native_safe_halt+0x6/0x10
> > [ 150.854036] [<ffffffff8123bf2d>] ? trace_hardirqs_on+0xd/0x10
> > [ 150.859973] [<ffffffff838b5d85>] default_idle+0x55/0x360
> > [ 150.865478] [<ffffffff8106be0a>] arch_cpu_idle+0xa/0x10
> > [ 150.870896] [<ffffffff838b6b96>] default_idle_call+0x36/0x60
> > [ 150.876751] [<ffffffff81226cb0>] cpu_startup_entry+0x2b0/0x380
> > [ 150.882787] [<ffffffff81226a00>] ? cpu_in_idle+0x20/0x20
> > [ 150.888291] [<ffffffff812d2343>] ? clockevents_register_device+0x123/0x200
> > [ 150.895358] [<ffffffff810b0693>] start_secondary+0x303/0x3e0
> > [ 150.901209] [<ffffffff810b0390>] ? set_cpu_sibling_map+0x11f0/0x11f0
>
> Thomas, thanks a lot. It appears this issue will not happen on
> mainline since from commit 765924e362d1 (subject "l2tp: don't close
> sessions in l2tp_tunnel_destruct()"), l2tp_tunnel_closeall is no
> longer called from l2tp_tunnel_destruct. From that commit message it
> seems one of the motivations is to solve scheduling from atomic issue.
>
I agree that this patch should fix the above splat.
> However for this change to be applied to android-4.9 and/or 4.9
> stable, it depends on several other l2p patches and they aren't
> straight forward cherry-picks from mainline (and I don't have much
> background with this driver).
>
> v3.16.56 stable seems to be further along with l2tp than v4.9.89, in
> that it atleast has more of the upstream patches adapted for it, that
> the above patch depends on. Since this also related to stable, I am
> CC'ing Greg kh and Ben.
>
I generally review l2tp patches proposed for -stable trees (although
not in time). If a patch has been ported to 3.16.y and is missing in
another tree, then it should be safe to port it there too.
> Here are some of the commits in 3.16 stable that I couldn't find
> applied to v4.9 stable. The above fix quotes the below patches as
> dependencies so they would need to be stable backported. Also CC'ing
> Guillaume since he authored the above mentioned fix.
>
> 0c15ddabbcf l2tp: don't register sessions in l2tp_session_create()
> a3c5d5b70f4e l2tp: fix race condition in l2tp_tunnel_delete
> 5b216e8dcda2 l2tp: prevent creation of sessions on terminated tunnels
> 76ff5e22f1e0 l2tp: hold tunnel while looking up sessions in l2tp_netlink
> ceb8f6b23a38 l2tp: define parameters of l2tp_session_get*() as "const"
> 0295d020b63f l2tp: initialise session's refcount before making it reachable
> 29a77518927e l2tp: take reference on sessions being dumped
> b301c9b7782f l2tp: take a reference on sessions used in genetlink handlers
>
Yes, I think it'd make sense to port these patches.
^ permalink raw reply
* Re: [net PATCH v2] net: sched, fix OOO packets with pfifo_fast
From: Cong Wang @ 2018-03-26 17:30 UTC (permalink / raw)
To: John Fastabend
Cc: Eric Dumazet, Jiri Pirko, David Miller,
Linux Kernel Network Developers
In-Reply-To: <20180325052505.4098.36912.stgit@john-Precision-Tower-5810>
On Sat, Mar 24, 2018 at 10:25 PM, John Fastabend
<john.fastabend@gmail.com> wrote:
> After the qdisc lock was dropped in pfifo_fast we allow multiple
> enqueue threads and dequeue threads to run in parallel. On the
> enqueue side the skb bit ooo_okay is used to ensure all related
> skbs are enqueued in-order. On the dequeue side though there is
> no similar logic. What we observe is with fewer queues than CPUs
> it is possible to re-order packets when two instances of
> __qdisc_run() are running in parallel. Each thread will dequeue
> a skb and then whichever thread calls the ndo op first will
> be sent on the wire. This doesn't typically happen because
> qdisc_run() is usually triggered by the same core that did the
> enqueue. However, drivers will trigger __netif_schedule()
> when queues are transitioning from stopped to awake using the
> netif_tx_wake_* APIs. When this happens netif_schedule() calls
> qdisc_run() on the same CPU that did the netif_tx_wake_* which
> is usually done in the interrupt completion context. This CPU
> is selected with the irq affinity which is unrelated to the
> enqueue operations.
Interesting. Why this is unique to pfifo_fast? For me it could
happen to other qdisc's too, when we release the qdisc root
lock in sch_direct_xmit(), another CPU could dequeue from
the same qdisc and transmit the skb in parallel too?
...
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 7e3fbe9..39c144b 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -373,24 +373,33 @@ bool sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
> */
> static inline bool qdisc_restart(struct Qdisc *q, int *packets)
> {
> + bool more, validate, nolock = q->flags & TCQ_F_NOLOCK;
> spinlock_t *root_lock = NULL;
> struct netdev_queue *txq;
> struct net_device *dev;
> struct sk_buff *skb;
> - bool validate;
>
> /* Dequeue packet */
> + if (nolock && test_and_set_bit(__QDISC_STATE_RUNNING, &q->state))
> + return false;
> +
Nit: you probably want to move the comment below this if check,
or simply remove it since it is useless...
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Leon Romanovsky @ 2018-03-26 17:27 UTC (permalink / raw)
To: Steve Wise; +Cc: David Ahern, stephen, netdev, linux-rdma
In-Reply-To: <6c376ad8-7b42-8b13-1eba-328fecd0e3bd@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 2644 bytes --]
On Mon, Mar 26, 2018 at 12:13:53PM -0500, Steve Wise wrote:
>
>
> On 3/26/2018 12:06 PM, Leon Romanovsky wrote:
> > On Mon, Mar 26, 2018 at 10:24:25AM -0500, Steve Wise wrote:
> >>
> >> On 3/26/2018 10:08 AM, Leon Romanovsky wrote:
> >>> On Mon, Mar 26, 2018 at 09:55:46AM -0500, Steve Wise wrote:
> >>>> On 3/26/2018 9:44 AM, David Ahern wrote:
> >>>>> On 3/26/18 8:30 AM, Steve Wise wrote:
> >>>>>> On 3/26/2018 9:17 AM, David Ahern wrote:
> >>>>>>> On 2/27/18 9:07 AM, Steve Wise wrote:
> >>>>>>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> >>>>>>>> index 5809f70..e55205b 100644
> >>>>>>>> --- a/rdma/rdma.h
> >>>>>>>> +++ b/rdma/rdma.h
> >>>>>>>> @@ -18,10 +18,12 @@
> >>>>>>>> #include <libmnl/libmnl.h>
> >>>>>>>> #include <rdma/rdma_netlink.h>
> >>>>>>>> #include <time.h>
> >>>>>>>> +#include <net/if_arp.h>
> >>>>>>>>
> >>>>>>>> #include "list.h"
> >>>>>>>> #include "utils.h"
> >>>>>>>> #include "json_writer.h"
> >>>>>>>> +#include <rdma/rdma_cma.h>
> >>>>>>>>
> >>>>>>> did you forget to add rdma_cma.h? I don't see that file in my repo.
> >>>>>> It is provided by the rdma-core package, upon which rdma tool now
> >>>>>> depends for the rdma_port_space enum.
> >>>>>>
> >>>>> You need to add a check for the package, and only build rdma if that
> >>>>> package is installed. See check_mnl in configure for an example.
> >>>> Ok, that makes sense.
> >>> IMHO, better solution will be to copy those files to iproute2.
> >> Hey Leon,
> >>
> >> Why is it better in your opinion? My gut tells me adding rdma_cma.h to
> >> iproute2 means more uabi type syncing.
> > Making rdmatool be dependant on rdma-core will require that distributions
> > will update their specs to install rdma-core as a dependency for every
> > iprotue2 install.
> >
> > The rdma-core dependency makes sense for RDMA users, but doesn't for most of
> > the iproute2 users.
>
> I'm fuzzy on the details of distro packaging, but David's suggestion is
> that rdmatool wouldn't get built if rdma-core isn't present. But
> everything else would. Just like it does not get built if libmnl is not
> installed. For pre-built rpms, the rdma-core would have to be present.
>
> I'm ok pulling it in, I'm just trying to understand. :)
>
Distros supply pre-built packages, for example Fedora's RPM:
https://rpmfind.net/linux/RPM/fedora/27/x86_64/i/iproute-4.12.0-3.fc27.x86_64.html
It requires that libmnl will be installed. Once rdmatool will need
rdma-core, it will pulled in too.
BTW, don't forget to change header's guards (ifdef/defne ..), see
rdma_netlink.h as an example.
> Steve.
>
>
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [bpf-next PATCH 4/4] bpf: sockmap, more BPF_SK_SKB_STREAM_VERDICT tests
From: John Fastabend @ 2018-03-26 17:22 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, davem
In-Reply-To: <20180326172022.20504.1790.stgit@john-Precision-Tower-5810>
Add BPF_SK_SKB_STREAM_VERDICT tests for ingress hook. While
we do this also bring stream tests in-line with MSG based
testing.
A map for skb options is added for userland to push options
at BPF programs.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
samples/sockmap/sockmap_kern.c | 21 ++++++++++++++++++---
samples/sockmap/sockmap_test.sh | 20 +++++++++++++++++++-
samples/sockmap/sockmap_user.c | 23 +++++++++++++++++++++++
3 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/samples/sockmap/sockmap_kern.c b/samples/sockmap/sockmap_kern.c
index ca28722..9ff8bc5 100644
--- a/samples/sockmap/sockmap_kern.c
+++ b/samples/sockmap/sockmap_kern.c
@@ -85,6 +85,12 @@ struct bpf_map_def SEC("maps") sock_redir_flags = {
.max_entries = 1
};
+struct bpf_map_def SEC("maps") sock_skb_opts = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .max_entries = 1
+};
SEC("sk_skb1")
int bpf_prog1(struct __sk_buff *skb)
@@ -97,15 +103,24 @@ int bpf_prog2(struct __sk_buff *skb)
{
__u32 lport = skb->local_port;
__u32 rport = skb->remote_port;
- int ret = 0;
+ int len, *f, ret, zero = 0;
+ __u64 flags = 0;
if (lport == 10000)
ret = 10;
else
ret = 1;
- bpf_printk("sockmap: %d -> %d @ %d\n", lport, bpf_ntohl(rport), ret);
- return bpf_sk_redirect_map(skb, &sock_map, ret, 0);
+ len = (__u32)skb->data_end - (__u32)skb->data;
+ f = bpf_map_lookup_elem(&sock_skb_opts, &zero);
+ if (f && *f) {
+ ret = 3;
+ flags = *f;
+ }
+
+ bpf_printk("sk_skb2: redirect(%iB) flags=%i\n",
+ len, flags);
+ return bpf_sk_redirect_map(skb, &sock_map, ret, flags);
}
SEC("sockops")
diff --git a/samples/sockmap/sockmap_test.sh b/samples/sockmap/sockmap_test.sh
index 13b205f..ace75f0 100755
--- a/samples/sockmap/sockmap_test.sh
+++ b/samples/sockmap/sockmap_test.sh
@@ -1,5 +1,5 @@
#Test a bunch of positive cases to verify basic functionality
-for prog in "--txmsg_redir --txmsg_ingress" "--txmsg" "--txmsg_redir" "--txmsg_redir --txmsg_ingress" "--txmsg_drop"; do
+for prog in "--txmsg_redir --txmsg_skb" "--txmsg_redir --txmsg_ingress" "--txmsg" "--txmsg_redir" "--txmsg_redir --txmsg_ingress" "--txmsg_drop"; do
for t in "sendmsg" "sendpage"; do
for r in 1 10 100; do
for i in 1 10 100; do
@@ -109,6 +109,15 @@ for t in "sendmsg" "sendpage"; do
sleep 2
done
+prog="--txmsg_redir --txmsg_apply 1 --txmsg_skb"
+
+for t in "sendmsg" "sendpage"; do
+ TEST="./sockmap --cgroup /mnt/cgroup2/ -t $t -r $r -i $i -l $l $prog"
+ echo $TEST
+ $TEST
+ sleep 2
+done
+
# Test apply and redirect with larger value than send
r=1
@@ -132,6 +141,15 @@ for t in "sendmsg" "sendpage"; do
sleep 2
done
+prog="--txmsg_redir --txmsg_apply 2048 --txmsg_skb"
+
+for t in "sendmsg" "sendpage"; do
+ TEST="./sockmap --cgroup /mnt/cgroup2/ -t $t -r $r -i $i -l $l $prog"
+ echo $TEST
+ $TEST
+ sleep 2
+done
+
# Test apply and redirect with apply that never reaches limit
r=1024
diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index f7503f4..6f23349 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -65,6 +65,7 @@
int txmsg_start;
int txmsg_end;
int txmsg_ingress;
+int txmsg_skb;
static const struct option long_options[] = {
{"help", no_argument, NULL, 'h' },
@@ -85,6 +86,7 @@
{"txmsg_start", required_argument, NULL, 's'},
{"txmsg_end", required_argument, NULL, 'e'},
{"txmsg_ingress", no_argument, &txmsg_ingress, 1 },
+ {"txmsg_skb", no_argument, &txmsg_skb, 1 },
{0, 0, NULL, 0 }
};
@@ -828,6 +830,27 @@ int main(int argc, char **argv)
err, strerror(errno));
}
}
+
+ if (txmsg_skb) {
+ int skb_fd = (test == SENDMSG || test == SENDPAGE) ? p2 : p1;
+ int ingress = BPF_F_INGRESS;
+
+ i = 0;
+ err = bpf_map_update_elem(map_fd[7], &i, &ingress, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (txmsg_ingress): %d (%s)\n",
+ err, strerror(errno));
+ }
+
+ i = 3;
+ err = bpf_map_update_elem(map_fd[0], &i, &skb_fd, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (c1 sockmap): %d (%s)\n",
+ err, strerror(errno));
+ }
+ }
}
if (txmsg_drop)
^ permalink raw reply related
* [bpf-next PATCH 3/4] bpf: sockmap, BPF_F_INGRESS flag for BPF_SK_SKB_STREAM_VERDICT:
From: John Fastabend @ 2018-03-26 17:22 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, davem
In-Reply-To: <20180326172022.20504.1790.stgit@john-Precision-Tower-5810>
Add support for the BPF_F_INGRESS flag in skb redirect helper. To
do this convert skb into a scatterlist and push into ingress queue.
This is the same logic that is used in the sk_msg redirect helper
so it should feel familiar.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/linux/filter.h | 1 +
kernel/bpf/sockmap.c | 94 +++++++++++++++++++++++++++++++++++++++---------
net/core/filter.c | 2 +
3 files changed, 78 insertions(+), 19 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index d0e207f..7de778a 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -521,6 +521,7 @@ struct sk_msg_buff {
__u32 key;
__u32 flags;
struct bpf_map *map;
+ struct sk_buff *skb;
struct list_head list;
};
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 7f32b03..92b4e4b 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -785,7 +785,8 @@ int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
i++;
if (i == MAX_SKB_FRAGS)
i = 0;
- put_page(page);
+ if (!md->skb)
+ put_page(page);
}
if (copied == len)
break;
@@ -794,6 +795,8 @@ int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
if (!sg->length && md->sg_start == md->sg_end) {
list_del(&md->list);
+ if (md->skb)
+ consume_skb(md->skb);
kfree(md);
}
}
@@ -1045,27 +1048,72 @@ static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb)
__SK_DROP;
}
+static int smap_do_ingress(struct smap_psock *psock, struct sk_buff *skb)
+{
+ struct sock *sk = psock->sock;
+ int copied = 0, num_sg;
+ struct sk_msg_buff *r;
+
+ r = kzalloc(sizeof(struct sk_msg_buff), __GFP_NOWARN | GFP_ATOMIC);
+ if (unlikely(!r))
+ return -EAGAIN;
+
+ if (!sk_rmem_schedule(sk, skb, skb->len)) {
+ kfree(r);
+ return -EAGAIN;
+ }
+ sk_mem_charge(sk, skb->len);
+
+ sg_init_table(r->sg_data, MAX_SKB_FRAGS);
+ num_sg = skb_to_sgvec(skb, r->sg_data, 0, skb->len);
+ if (unlikely(num_sg < 0)) {
+ kfree(r);
+ return num_sg;
+ }
+ copied = skb->len;
+ r->sg_start = 0;
+ r->sg_end = num_sg == MAX_SKB_FRAGS ? 0 : num_sg;
+ r->skb = skb;
+ list_add_tail(&r->list, &psock->ingress);
+ sk->sk_data_ready(sk);
+ return copied;
+}
+
static void smap_do_verdict(struct smap_psock *psock, struct sk_buff *skb)
{
+ struct smap_psock *peer;
struct sock *sk;
+ __u32 in;
int rc;
rc = smap_verdict_func(psock, skb);
switch (rc) {
case __SK_REDIRECT:
sk = do_sk_redirect_map(skb);
- if (likely(sk)) {
- struct smap_psock *peer = smap_psock_sk(sk);
-
- if (likely(peer &&
- test_bit(SMAP_TX_RUNNING, &peer->state) &&
- !sock_flag(sk, SOCK_DEAD) &&
- sock_writeable(sk))) {
- skb_set_owner_w(skb, sk);
- skb_queue_tail(&peer->rxqueue, skb);
- schedule_work(&peer->tx_work);
- break;
- }
+ if (!sk) {
+ kfree_skb(skb);
+ break;
+ }
+
+ peer = smap_psock_sk(sk);
+ in = (TCP_SKB_CB(skb)->bpf.flags) & BPF_F_INGRESS;
+
+ if (unlikely(!peer || sock_flag(sk, SOCK_DEAD) ||
+ !test_bit(SMAP_TX_RUNNING, &peer->state))) {
+ kfree_skb(skb);
+ break;
+ }
+
+ if (!in && sock_writeable(sk)) {
+ skb_set_owner_w(skb, sk);
+ skb_queue_tail(&peer->rxqueue, skb);
+ schedule_work(&peer->tx_work);
+ break;
+ } else if (in &&
+ atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf) {
+ skb_queue_tail(&peer->rxqueue, skb);
+ schedule_work(&peer->tx_work);
+ break;
}
/* Fall through and free skb otherwise */
case __SK_DROP:
@@ -1127,15 +1175,23 @@ static void smap_tx_work(struct work_struct *w)
}
while ((skb = skb_dequeue(&psock->rxqueue))) {
+ __u32 flags;
+
rem = skb->len;
off = 0;
start:
+ flags = (TCP_SKB_CB(skb)->bpf.flags) & BPF_F_INGRESS;
do {
- if (likely(psock->sock->sk_socket))
- n = skb_send_sock_locked(psock->sock,
- skb, off, rem);
- else
+ if (likely(psock->sock->sk_socket)) {
+ if (flags)
+ n = smap_do_ingress(psock, skb);
+ else
+ n = skb_send_sock_locked(psock->sock,
+ skb, off, rem);
+ } else {
n = -EINVAL;
+ }
+
if (n <= 0) {
if (n == -EAGAIN) {
/* Retry when space is available */
@@ -1153,7 +1209,9 @@ static void smap_tx_work(struct work_struct *w)
rem -= n;
off += n;
} while (rem);
- kfree_skb(skb);
+
+ if (!flags)
+ kfree_skb(skb);
}
out:
release_sock(psock->sock);
diff --git a/net/core/filter.c b/net/core/filter.c
index 11b1f16..b46916d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1855,7 +1855,7 @@ int skb_do_redirect(struct sk_buff *skb)
struct tcp_skb_cb *tcb = TCP_SKB_CB(skb);
/* If user passes invalid input drop the packet. */
- if (unlikely(flags))
+ if (unlikely(flags & ~(BPF_F_INGRESS)))
return SK_DROP;
tcb->bpf.key = key;
^ permalink raw reply related
* [bpf-next PATCH 2/4] bpf: sockmap, add BPF_F_INGRESS tests
From: John Fastabend @ 2018-03-26 17:22 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, davem
In-Reply-To: <20180326172022.20504.1790.stgit@john-Precision-Tower-5810>
Add a set of tests to verify ingress flag in redirect helpers
works correctly with various msg sizes.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
samples/sockmap/sockmap_kern.c | 41 +++++++++++++++++++++++++++++----------
samples/sockmap/sockmap_test.sh | 22 ++++++++++++++++++++-
samples/sockmap/sockmap_user.c | 35 +++++++++++++++++++++++++++++++++
3 files changed, 87 insertions(+), 11 deletions(-)
diff --git a/samples/sockmap/sockmap_kern.c b/samples/sockmap/sockmap_kern.c
index 9ad5ba7..ca28722 100644
--- a/samples/sockmap/sockmap_kern.c
+++ b/samples/sockmap/sockmap_kern.c
@@ -54,7 +54,7 @@ struct bpf_map_def SEC("maps") sock_map_redir = {
.type = BPF_MAP_TYPE_SOCKMAP,
.key_size = sizeof(int),
.value_size = sizeof(int),
- .max_entries = 1,
+ .max_entries = 20,
};
struct bpf_map_def SEC("maps") sock_apply_bytes = {
@@ -78,6 +78,13 @@ struct bpf_map_def SEC("maps") sock_pull_bytes = {
.max_entries = 2
};
+struct bpf_map_def SEC("maps") sock_redir_flags = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .max_entries = 1
+};
+
SEC("sk_skb1")
int bpf_prog1(struct __sk_buff *skb)
@@ -197,8 +204,9 @@ 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;
- int *start, *end;
+ int *bytes, zero = 0, one = 1, key = 0;
+ int *start, *end, *f;
+ __u64 flags = 0;
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
if (bytes)
@@ -210,15 +218,22 @@ int bpf_prog6(struct sk_msg_md *msg)
end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
if (start && end)
bpf_msg_pull_data(msg, *start, *end, 0);
- return bpf_msg_redirect_map(msg, &sock_map_redir, zero, 0);
+ f = bpf_map_lookup_elem(&sock_redir_flags, &zero);
+ if (f && *f) {
+ key = 2;
+ flags = *f;
+ }
+ return bpf_msg_redirect_map(msg, &sock_map_redir, key, flags);
}
SEC("sk_msg4")
int bpf_prog7(struct sk_msg_md *msg)
{
- int err1 = 0, err2 = 0, zero = 0, one = 1;
- int *bytes, *start, *end, len1, len2;
+ int err1 = 0, err2 = 0, zero = 0, one = 1, key = 0;
+ int *f, *bytes, *start, *end, len1, len2;
+ __u64 flags = 0;
+ int err;
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
if (bytes)
err1 = bpf_msg_apply_bytes(msg, *bytes);
@@ -229,7 +244,6 @@ int bpf_prog7(struct sk_msg_md *msg)
start = bpf_map_lookup_elem(&sock_pull_bytes, &zero);
end = bpf_map_lookup_elem(&sock_pull_bytes, &one);
if (start && end) {
- int err;
bpf_printk("sk_msg2: pull(%i:%i)\n",
start ? *start : 0, end ? *end : 0);
@@ -241,9 +255,16 @@ int bpf_prog7(struct sk_msg_md *msg)
bpf_printk("sk_msg2: length update %i->%i\n",
len1, len2);
}
- bpf_printk("sk_msg3: redirect(%iB) err1=%i err2=%i\n",
- len1, err1, err2);
- return bpf_msg_redirect_map(msg, &sock_map_redir, zero, 0);
+ f = bpf_map_lookup_elem(&sock_redir_flags, &zero);
+ if (f && *f) {
+ key = 2;
+ flags = *f;
+ }
+ bpf_printk("sk_msg3: redirect(%iB) flags=%i err=%i\n",
+ len1, flags, err1 ? err1 : err2);
+ err = bpf_msg_redirect_map(msg, &sock_map_redir, key, flags);
+ bpf_printk("sk_msg3: err %i\n", err);
+ return err;
}
SEC("sk_msg5")
diff --git a/samples/sockmap/sockmap_test.sh b/samples/sockmap/sockmap_test.sh
index 6d8cc40..13b205f 100755
--- a/samples/sockmap/sockmap_test.sh
+++ b/samples/sockmap/sockmap_test.sh
@@ -1,5 +1,5 @@
#Test a bunch of positive cases to verify basic functionality
-for prog in "--txmsg" "--txmsg_redir" "--txmsg_drop"; do
+for prog in "--txmsg_redir --txmsg_ingress" "--txmsg" "--txmsg_redir" "--txmsg_redir --txmsg_ingress" "--txmsg_drop"; do
for t in "sendmsg" "sendpage"; do
for r in 1 10 100; do
for i in 1 10 100; do
@@ -100,6 +100,16 @@ for t in "sendmsg" "sendpage"; do
sleep 2
done
+prog="--txmsg_redir --txmsg_apply 1 --txmsg_ingress"
+
+for t in "sendmsg" "sendpage"; do
+ TEST="./sockmap --cgroup /mnt/cgroup2/ -t $t -r $r -i $i -l $l $prog"
+ echo $TEST
+ $TEST
+ sleep 2
+done
+
+
# Test apply and redirect with larger value than send
r=1
i=8
@@ -113,6 +123,16 @@ for t in "sendmsg" "sendpage"; do
sleep 2
done
+prog="--txmsg_redir --txmsg_apply 2048 --txmsg_ingress"
+
+for t in "sendmsg" "sendpage"; do
+ TEST="./sockmap --cgroup /mnt/cgroup2/ -t $t -r $r -i $i -l $l $prog"
+ echo $TEST
+ $TEST
+ sleep 2
+done
+
+
# Test apply and redirect with apply that never reaches limit
r=1024
i=1
diff --git a/samples/sockmap/sockmap_user.c b/samples/sockmap/sockmap_user.c
index 07aa237..f7503f4 100644
--- a/samples/sockmap/sockmap_user.c
+++ b/samples/sockmap/sockmap_user.c
@@ -64,6 +64,7 @@
int txmsg_cork;
int txmsg_start;
int txmsg_end;
+int txmsg_ingress;
static const struct option long_options[] = {
{"help", no_argument, NULL, 'h' },
@@ -83,6 +84,7 @@
{"txmsg_cork", required_argument, NULL, 'k'},
{"txmsg_start", required_argument, NULL, 's'},
{"txmsg_end", required_argument, NULL, 'e'},
+ {"txmsg_ingress", no_argument, &txmsg_ingress, 1 },
{0, 0, NULL, 0 }
};
@@ -793,6 +795,39 @@ int main(int argc, char **argv)
return err;
}
}
+
+ if (txmsg_ingress) {
+ int in = BPF_F_INGRESS;
+
+ i = 0;
+ err = bpf_map_update_elem(map_fd[6], &i, &in, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (txmsg_ingress): %d (%s)\n",
+ err, strerror(errno));
+ }
+ i = 1;
+ err = bpf_map_update_elem(map_fd[1], &i, &p1, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (p1 txmsg): %d (%s)\n",
+ err, strerror(errno));
+ }
+ err = bpf_map_update_elem(map_fd[2], &i, &p1, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (p1 redir): %d (%s)\n",
+ err, strerror(errno));
+ }
+
+ i = 2;
+ err = bpf_map_update_elem(map_fd[2], &i, &p2, BPF_ANY);
+ if (err) {
+ fprintf(stderr,
+ "ERROR: bpf_map_update_elem (p2 txmsg): %d (%s)\n",
+ err, strerror(errno));
+ }
+ }
}
if (txmsg_drop)
^ permalink raw reply related
* [bpf-next PATCH 1/4] bpf: sockmap redirect ingress support
From: John Fastabend @ 2018-03-26 17:22 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, davem
In-Reply-To: <20180326172022.20504.1790.stgit@john-Precision-Tower-5810>
Add support for the BPF_F_INGRESS flag in sk_msg redirect helper.
To do this add a scatterlist ring for receiving socks to check
before calling into regular recvmsg call path. Additionally, because
the poll wakeup logic only checked the skb recv queue we need to
add a hook in TCP stack (similar to write side) so that we have
a way to wake up polling socks when a scatterlist is redirected
to that sock.
After this all that is needed is for the redirect helper to
push the scatterlist into the psock receive queue.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/linux/filter.h | 1
include/net/sock.h | 1
kernel/bpf/sockmap.c | 198 +++++++++++++++++++++++++++++++++++++++++++++++-
net/core/filter.c | 2
net/ipv4/tcp.c | 10 ++
5 files changed, 207 insertions(+), 5 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 109d05c..d0e207f 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -521,6 +521,7 @@ struct sk_msg_buff {
__u32 key;
__u32 flags;
struct bpf_map *map;
+ struct list_head list;
};
/* Compute the linear packet data range [data, data_end) which
diff --git a/include/net/sock.h b/include/net/sock.h
index 7093111..b8ff435 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1085,6 +1085,7 @@ struct proto {
#endif
bool (*stream_memory_free)(const struct sock *sk);
+ bool (*stream_memory_read)(const struct sock *sk);
/* Memory pressure */
void (*enter_memory_pressure)(struct sock *sk);
void (*leave_memory_pressure)(struct sock *sk);
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 69c5bcc..7f32b03 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -41,6 +41,8 @@
#include <linux/mm.h>
#include <net/strparser.h>
#include <net/tcp.h>
+#include <linux/ptr_ring.h>
+#include <net/inet_common.h>
#define SOCK_CREATE_FLAG_MASK \
(BPF_F_NUMA_NODE | BPF_F_RDONLY | BPF_F_WRONLY)
@@ -82,6 +84,7 @@ struct smap_psock {
int sg_size;
int eval;
struct sk_msg_buff *cork;
+ struct list_head ingress;
struct strparser strp;
struct bpf_prog *bpf_tx_msg;
@@ -103,6 +106,8 @@ struct smap_psock {
};
static void smap_release_sock(struct smap_psock *psock, struct sock *sock);
+static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
+ int nonblock, int flags, int *addr_len);
static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
int offset, size_t size, int flags);
@@ -112,6 +117,21 @@ static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
return rcu_dereference_sk_user_data(sk);
}
+bool bpf_tcp_stream_read(const struct sock *sk)
+{
+ struct smap_psock *psock;
+ bool empty = true;
+
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock))
+ goto out;
+ empty = list_empty(&psock->ingress);
+out:
+ rcu_read_unlock();
+ return !empty;
+}
+
static struct proto tcp_bpf_proto;
static int bpf_tcp_init(struct sock *sk)
{
@@ -135,6 +155,8 @@ static int bpf_tcp_init(struct sock *sk)
if (psock->bpf_tx_msg) {
tcp_bpf_proto.sendmsg = bpf_tcp_sendmsg;
tcp_bpf_proto.sendpage = bpf_tcp_sendpage;
+ tcp_bpf_proto.recvmsg = bpf_tcp_recvmsg;
+ tcp_bpf_proto.stream_memory_read = bpf_tcp_stream_read;
}
sk->sk_prot = &tcp_bpf_proto;
@@ -170,6 +192,7 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
{
void (*close_fun)(struct sock *sk, long timeout);
struct smap_psock_map_entry *e, *tmp;
+ struct sk_msg_buff *md, *mtmp;
struct smap_psock *psock;
struct sock *osk;
@@ -188,6 +211,12 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
close_fun = psock->save_close;
write_lock_bh(&sk->sk_callback_lock);
+ list_for_each_entry_safe(md, mtmp, &psock->ingress, list) {
+ list_del(&md->list);
+ free_start_sg(psock->sock, md);
+ kfree(md);
+ }
+
list_for_each_entry_safe(e, tmp, &psock->maps, list) {
osk = cmpxchg(e->entry, sk, NULL);
if (osk == sk) {
@@ -468,6 +497,72 @@ static unsigned int smap_do_tx_msg(struct sock *sk,
return _rc;
}
+static int bpf_tcp_ingress(struct sock *sk, int apply_bytes,
+ struct smap_psock *psock,
+ struct sk_msg_buff *md, int flags)
+{
+ bool apply = apply_bytes;
+ size_t size, copied = 0;
+ struct sk_msg_buff *r;
+ int err = 0, i;
+
+ r = kzalloc(sizeof(struct sk_msg_buff), __GFP_NOWARN | GFP_KERNEL);
+ if (unlikely(!r))
+ return -ENOMEM;
+
+ lock_sock(sk);
+ r->sg_start = md->sg_start;
+ i = md->sg_start;
+
+ do {
+ r->sg_data[i] = md->sg_data[i];
+
+ size = (apply && apply_bytes < md->sg_data[i].length) ?
+ apply_bytes : md->sg_data[i].length;
+
+ if (!sk_wmem_schedule(sk, size)) {
+ if (!copied)
+ err = -ENOMEM;
+ break;
+ }
+
+ sk_mem_charge(sk, size);
+ r->sg_data[i].length = size;
+ md->sg_data[i].length -= size;
+ md->sg_data[i].offset += size;
+ copied += size;
+
+ if (md->sg_data[i].length) {
+ get_page(sg_page(&r->sg_data[i]));
+ r->sg_end = (i + 1) == MAX_SKB_FRAGS ? 0 : i + 1;
+ } else {
+ i++;
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ r->sg_end = i;
+ }
+
+ if (apply) {
+ apply_bytes -= size;
+ if (!apply_bytes)
+ break;
+ }
+ } while (i != md->sg_end);
+
+ md->sg_start = i;
+
+ if (!err) {
+ list_add_tail(&r->list, &psock->ingress);
+ sk->sk_data_ready(sk);
+ } else {
+ free_start_sg(sk, r);
+ kfree(r);
+ }
+
+ release_sock(sk);
+ return err;
+}
+
static int bpf_tcp_sendmsg_do_redirect(struct sock *sk, int send,
struct sk_msg_buff *md,
int flags)
@@ -475,6 +570,7 @@ static int bpf_tcp_sendmsg_do_redirect(struct sock *sk, int send,
struct smap_psock *psock;
struct scatterlist *sg;
int i, err, free = 0;
+ bool ingress = !!(md->flags & BPF_F_INGRESS);
sg = md->sg_data;
@@ -487,9 +583,14 @@ static int bpf_tcp_sendmsg_do_redirect(struct sock *sk, int send,
goto out_rcu;
rcu_read_unlock();
- lock_sock(sk);
- err = bpf_tcp_push(sk, send, md, flags, false);
- release_sock(sk);
+
+ if (ingress) {
+ err = bpf_tcp_ingress(sk, send, psock, md, flags);
+ } else {
+ lock_sock(sk);
+ err = bpf_tcp_push(sk, send, md, flags, false);
+ release_sock(sk);
+ }
smap_release_sock(psock, sk);
if (unlikely(err))
goto out;
@@ -623,6 +724,89 @@ static int bpf_exec_tx_verdict(struct smap_psock *psock,
return err;
}
+int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
+ int nonblock, int flags, int *addr_len)
+{
+ struct iov_iter *iter = &msg->msg_iter;
+ struct smap_psock *psock;
+ int copied = 0;
+
+ if (unlikely(flags & MSG_ERRQUEUE))
+ return inet_recv_error(sk, msg, len, addr_len);
+
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock))
+ goto out;
+
+ if (unlikely(!refcount_inc_not_zero(&psock->refcnt)))
+ goto out;
+
+ if (!skb_queue_empty(&sk->sk_receive_queue))
+ goto out;
+ rcu_read_unlock();
+
+ lock_sock(sk);
+ while (copied != len) {
+ struct scatterlist *sg;
+ struct sk_msg_buff *md;
+ int i;
+
+ md = list_first_entry_or_null(&psock->ingress,
+ struct sk_msg_buff, list);
+ if (unlikely(!md))
+ break;
+ i = md->sg_start;
+ do {
+ struct page *page;
+ int n, copy;
+
+ sg = &md->sg_data[i];
+ copy = sg->length;
+ page = sg_page(sg);
+
+ if (copied + copy > len)
+ copy = len - copied;
+
+ n = copy_page_to_iter(page, sg->offset, copy, iter);
+ if (n != copy) {
+ md->sg_start = i;
+ release_sock(sk);
+ smap_release_sock(psock, sk);
+ return -EFAULT;
+ }
+
+ copied += copy;
+ sg->offset += copy;
+ sg->length -= copy;
+ sk_mem_uncharge(sk, copy);
+
+ if (!sg->length) {
+ i++;
+ if (i == MAX_SKB_FRAGS)
+ i = 0;
+ put_page(page);
+ }
+ if (copied == len)
+ break;
+ } while (i != md->sg_end);
+ md->sg_start = i;
+
+ if (!sg->length && md->sg_start == md->sg_end) {
+ list_del(&md->list);
+ kfree(md);
+ }
+ }
+
+ release_sock(sk);
+ smap_release_sock(psock, sk);
+ return copied;
+out:
+ rcu_read_unlock();
+ return tcp_recvmsg(sk, msg, len, nonblock, flags, addr_len);
+}
+
+
static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
{
int flags = msg->msg_flags | MSG_NO_SHARED_FRAGS;
@@ -1107,6 +1291,7 @@ static void sock_map_remove_complete(struct bpf_stab *stab)
static void smap_gc_work(struct work_struct *w)
{
struct smap_psock_map_entry *e, *tmp;
+ struct sk_msg_buff *md, *mtmp;
struct smap_psock *psock;
psock = container_of(w, struct smap_psock, gc_work);
@@ -1131,6 +1316,12 @@ static void smap_gc_work(struct work_struct *w)
kfree(psock->cork);
}
+ list_for_each_entry_safe(md, mtmp, &psock->ingress, list) {
+ list_del(&md->list);
+ free_start_sg(psock->sock, md);
+ kfree(md);
+ }
+
list_for_each_entry_safe(e, tmp, &psock->maps, list) {
list_del(&e->list);
kfree(e);
@@ -1160,6 +1351,7 @@ static struct smap_psock *smap_init_psock(struct sock *sock,
INIT_WORK(&psock->tx_work, smap_tx_work);
INIT_WORK(&psock->gc_work, smap_gc_work);
INIT_LIST_HEAD(&psock->maps);
+ INIT_LIST_HEAD(&psock->ingress);
refcount_set(&psock->refcnt, 1);
rcu_assign_sk_user_data(sock, psock);
diff --git a/net/core/filter.c b/net/core/filter.c
index 00c711c..11b1f16 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1894,7 +1894,7 @@ struct sock *do_sk_redirect_map(struct sk_buff *skb)
struct bpf_map *, map, u32, key, u64, flags)
{
/* If user passes invalid input drop the packet. */
- if (unlikely(flags))
+ if (unlikely(flags & ~(BPF_F_INGRESS)))
return SK_DROP;
msg->key = key;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 0c31be3..bccc4c2 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -485,6 +485,14 @@ static void tcp_tx_timestamp(struct sock *sk, u16 tsflags)
}
}
+static inline bool tcp_stream_is_readable(const struct tcp_sock *tp,
+ int target, struct sock *sk)
+{
+ return (tp->rcv_nxt - tp->copied_seq >= target) ||
+ (sk->sk_prot->stream_memory_read ?
+ sk->sk_prot->stream_memory_read(sk) : false);
+}
+
/*
* Wait for a TCP event.
*
@@ -554,7 +562,7 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
tp->urg_data)
target++;
- if (tp->rcv_nxt - tp->copied_seq >= target)
+ if (tcp_stream_is_readable(tp, target, sk))
mask |= EPOLLIN | EPOLLRDNORM;
if (!(sk->sk_shutdown & SEND_SHUTDOWN)) {
^ permalink raw reply related
* [bpf-next PATCH 0/4] bpf, sockmap BPF_F_INGRESS support
From: John Fastabend @ 2018-03-26 17:22 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, davem
This series adds the BPF_F_INGRESS flag support to the reidrect APIs.
Bringing the sockmap API in-line with the cls_bpf redirect APIs.
We add it to both variants of sockmap programs, the first patch adds
support for tx ulp hooks and the third patch adds supoprt for the recv
skb hooks. Patches two and four add tests for the corresponding
ingress redirect hooks.
Follow on patches can address busy polling support, but next series
from me will move the sockmap sample program into selftests.
Thanks,
John
---
John Fastabend (4):
bpf: sockmap redirect ingress support
bpf: sockmap, add BPF_F_INGRESS tests
bpf: sockmap, BPF_F_INGRESS flag for BPF_SK_SKB_STREAM_VERDICT:
bpf: sockmap, more BPF_SK_SKB_STREAM_VERDICT tests
include/linux/filter.h | 2
include/net/sock.h | 1
kernel/bpf/sockmap.c | 290 ++++++++++++++++++++++++++++++++++++---
net/core/filter.c | 4 -
net/ipv4/tcp.c | 10 +
samples/sockmap/sockmap_kern.c | 62 +++++++-
samples/sockmap/sockmap_test.sh | 40 +++++
samples/sockmap/sockmap_user.c | 58 ++++++++
8 files changed, 430 insertions(+), 37 deletions(-)
^ permalink raw reply
* [PATCH v2 net] r8169: fix setting driver_data after register_netdev
From: Heiner Kallweit @ 2018-03-26 17:19 UTC (permalink / raw)
To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org
pci_set_drvdata() is called only after registering the net_device,
therefore we could run into a NPE if one of the functions using
driver_data is called before it's set.
Fix this by calling pci_set_drvdata() before registering the
net_device.
This fix is a candidate for stable. As far as I can see the
bug has been there in kernel version 3.2 already, therefore
I can't provide a reference which commit is fixed by it.
The fix may need small adjustments per kernel version because
due to other changes the label which is jumped to if
register_netdev() fails has changed over time.
Reported-by: David Miller <davem@davemloft.net>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- rebased based on net instead of net-next
---
drivers/net/ethernet/realtek/r8169.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 0bf7d1759..b4779acb6 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -8660,12 +8660,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (!tp->counters)
return -ENOMEM;
+ pci_set_drvdata(pdev, dev);
+
rc = register_netdev(dev);
if (rc < 0)
return rc;
- pci_set_drvdata(pdev, dev);
-
netif_info(tp, probe, dev, "%s at 0x%p, %pM, XID %08x IRQ %d\n",
rtl_chip_infos[chipset].name, ioaddr, dev->dev_addr,
(u32)(RTL_R32(TxConfig) & 0x9cf0f8ff), pdev->irq);
--
2.16.2
^ permalink raw reply related
* Re: [PATCH net-next 00/15] mlxsw: Offload IPv6 multicast routes
From: David Miller @ 2018-03-26 17:15 UTC (permalink / raw)
To: idosch; +Cc: netdev, yuvalm, jiri, nikolay
In-Reply-To: <20180326120145.11752-1-idosch@mellanox.com>
From: Ido Schimmel <idosch@mellanox.com>
Date: Mon, 26 Mar 2018 15:01:30 +0300
> The series is intended to allow offloading IPv6 multicast routes
> and is split into two parts:
>
> - First half of the patches continue extending ip6mr [& refactor ipmr]
> with missing bits necessary for the offloading - fib-notifications,
> mfc refcounting and default rule identification.
>
> - Second half of the patches extend functionality inside mlxsw,
> beginning with extending lower-parts to support IPv6 mroutes
> to host and later extending the router/mr internal APIs within
> the driver to accommodate support in ipv6 configurations.
> Lastly it adds support in the RTNL_FAMILY_IP6MR notifications,
> allowing driver to react and offload related routes.
I like the consolidations and the rest of the series looks fine to me
as well, so series applied.
Any adjustments people want can be done as follow-on patches.
Thank you.
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Steve Wise @ 2018-03-26 17:13 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: David Ahern, stephen, netdev, linux-rdma
In-Reply-To: <20180326170647.GL1877@mtr-leonro.local>
On 3/26/2018 12:06 PM, Leon Romanovsky wrote:
> On Mon, Mar 26, 2018 at 10:24:25AM -0500, Steve Wise wrote:
>>
>> On 3/26/2018 10:08 AM, Leon Romanovsky wrote:
>>> On Mon, Mar 26, 2018 at 09:55:46AM -0500, Steve Wise wrote:
>>>> On 3/26/2018 9:44 AM, David Ahern wrote:
>>>>> On 3/26/18 8:30 AM, Steve Wise wrote:
>>>>>> On 3/26/2018 9:17 AM, David Ahern wrote:
>>>>>>> On 2/27/18 9:07 AM, Steve Wise wrote:
>>>>>>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
>>>>>>>> index 5809f70..e55205b 100644
>>>>>>>> --- a/rdma/rdma.h
>>>>>>>> +++ b/rdma/rdma.h
>>>>>>>> @@ -18,10 +18,12 @@
>>>>>>>> #include <libmnl/libmnl.h>
>>>>>>>> #include <rdma/rdma_netlink.h>
>>>>>>>> #include <time.h>
>>>>>>>> +#include <net/if_arp.h>
>>>>>>>>
>>>>>>>> #include "list.h"
>>>>>>>> #include "utils.h"
>>>>>>>> #include "json_writer.h"
>>>>>>>> +#include <rdma/rdma_cma.h>
>>>>>>>>
>>>>>>> did you forget to add rdma_cma.h? I don't see that file in my repo.
>>>>>> It is provided by the rdma-core package, upon which rdma tool now
>>>>>> depends for the rdma_port_space enum.
>>>>>>
>>>>> You need to add a check for the package, and only build rdma if that
>>>>> package is installed. See check_mnl in configure for an example.
>>>> Ok, that makes sense.
>>> IMHO, better solution will be to copy those files to iproute2.
>> Hey Leon,
>>
>> Why is it better in your opinion? My gut tells me adding rdma_cma.h to
>> iproute2 means more uabi type syncing.
> Making rdmatool be dependant on rdma-core will require that distributions
> will update their specs to install rdma-core as a dependency for every
> iprotue2 install.
>
> The rdma-core dependency makes sense for RDMA users, but doesn't for most of
> the iproute2 users.
I'm fuzzy on the details of distro packaging, but David's suggestion is
that rdmatool wouldn't get built if rdma-core isn't present. But
everything else would. Just like it does not get built if libmnl is not
installed. For pre-built rpms, the rdma-core would have to be present.
I'm ok pulling it in, I'm just trying to understand. :)
Steve.
^ permalink raw reply
* Re: [net PATCH v2] net: sched, fix OOO packets with pfifo_fast
From: John Fastabend @ 2018-03-26 17:10 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, xiyou.wangcong, jiri, netdev, Jakob Unterwurzacher
In-Reply-To: <20180326.123643.803872307508307757.davem@davemloft.net>
On 03/26/2018 09:36 AM, David Miller wrote:
> From: John Fastabend <john.fastabend@gmail.com>
> Date: Sat, 24 Mar 2018 22:25:06 -0700
>
>> After the qdisc lock was dropped in pfifo_fast we allow multiple
>> enqueue threads and dequeue threads to run in parallel. On the
>> enqueue side the skb bit ooo_okay is used to ensure all related
>> skbs are enqueued in-order. On the dequeue side though there is
>> no similar logic. What we observe is with fewer queues than CPUs
>> it is possible to re-order packets when two instances of
>> __qdisc_run() are running in parallel. Each thread will dequeue
>> a skb and then whichever thread calls the ndo op first will
>> be sent on the wire. This doesn't typically happen because
>> qdisc_run() is usually triggered by the same core that did the
>> enqueue. However, drivers will trigger __netif_schedule()
>> when queues are transitioning from stopped to awake using the
>> netif_tx_wake_* APIs. When this happens netif_schedule() calls
>> qdisc_run() on the same CPU that did the netif_tx_wake_* which
>> is usually done in the interrupt completion context. This CPU
>> is selected with the irq affinity which is unrelated to the
>> enqueue operations.
>>
>> To resolve this we add a RUNNING bit to the qdisc to ensure
>> only a single dequeue per qdisc is running. Enqueue and dequeue
>> operations can still run in parallel and also on multi queue
>> NICs we can still have a dequeue in-flight per qdisc, which
>> is typically per CPU.
>>
>> Fixes: c5ad119fb6c0 ("net: sched: pfifo_fast use skb_array")
>> Reported-by: Jakob Unterwurzacher <jakob.unterwurzacher@theobroma-systems.com>
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>
> Applied, thanks John.
>
Great, also off-list email from Jakob (I forgot to add him to the
CC list here, oops) told me to add,
Tested-by: Jakob Unterwurzacher <jakob.unterwurzacher@theobroma-systems.com>
Also in net-next I'll look to see if we can avoid doing the extra
atomics especially in cases where they are not actually needed. For
example the 1:1 qdisc to txq mappings. It seems a bit evasive
though for net.
Finally just an FYI but I think I'll look at a distributed counter
soon so we can get a lockless token bucket. I need the counter for
BPF as well so coming soon.
Thanks,
John
^ permalink raw reply
* Re: [PATCH net 2/2] net: dsa: mt7530: fix module autoloading for OF platform drivers
From: David Miller @ 2018-03-26 17:10 UTC (permalink / raw)
To: sean.wang
Cc: andrew, f.fainelli, vivien.didelot, netdev, linux-kernel,
linux-mediatek
In-Reply-To: <cb3bdedb65fca8e206ba081b0dfd69667c3f7804.1522057659.git.sean.wang@mediatek.com>
From: <sean.wang@mediatek.com>
Date: Mon, 26 Mar 2018 18:07:10 +0800
> From: Sean Wang <sean.wang@mediatek.com>
>
> It's required to create a modules.alias via MODULE_DEVICE_TABLE helper
> for the OF platform driver. Otherwise, module autoloading cannot work.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Applied.
^ permalink raw reply
* Re: [PATCH net 1/2] net: dsa: mt7530: remove redundant MODULE_ALIAS entries
From: David Miller @ 2018-03-26 17:09 UTC (permalink / raw)
To: sean.wang
Cc: andrew, f.fainelli, vivien.didelot, netdev, linux-kernel,
linux-mediatek
In-Reply-To: <2497321afc9156f7954e813721f60a0a95c03bd2.1522057659.git.sean.wang@mediatek.com>
From: <sean.wang@mediatek.com>
Date: Mon, 26 Mar 2018 18:07:09 +0800
> From: Sean Wang <sean.wang@mediatek.com>
>
> MODULE_ALIAS exports information to allow the module to be auto-loaded at
> boot for the drivers registered using legacy platform registration.
>
> However, currently the driver is always used by DT-only platform,
> MODULE_ALIAS is redundant and should be removed properly.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Applied.
^ permalink raw reply
* Re: [PATCH iproute2-next] rdma: Move RDMA UAPI header file to be under RDMA responsibility
From: Leon Romanovsky @ 2018-03-26 17:07 UTC (permalink / raw)
To: Steve Wise; +Cc: David Ahern, netdev, RDMA mailing list, Stephen Hemminger
In-Reply-To: <6a187694-654d-02f8-49fe-98704839de1f@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 1262 bytes --]
On Mon, Mar 26, 2018 at 09:23:46AM -0500, Steve Wise wrote:
>
>
> On 3/26/2018 9:03 AM, David Ahern wrote:
> > On 3/25/18 12:38 AM, Leon Romanovsky wrote:
> >> From: Leon Romanovsky <leonro@mellanox.com>
> >>
> >> In iproute2 package, the updates of UAPIs files are performed
> >> after the needed feature lands in kernel's net-next tree.
> >>
> >> Such development flow created delays to the rdma tool developers,
> >> who uses rdma-next tree as a basis for their work.
> >>
> >> Move RDMA UAPI file to be under rdma/ folder, so whole responsibility
> >> of syncing this file will be on them.
> >>
> >> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> >> ---
> >> rdma/Makefile | 1 +
> >> {include => rdma/include}/uapi/rdma/rdma_netlink.h | 0
> >> 2 files changed, 1 insertion(+)
> >> rename {include => rdma/include}/uapi/rdma/rdma_netlink.h (100%)
> > applied to iproute2-next
> >
>
> Hey Leon, so how do I change rdma/include/uapi/rdma_netlink.h now for my
> series [1]? Do I just make the changes? Or is there some other process?
I don't think that we need "process" yet, simply copy from kernel.
>
> [1] https://www.spinics.net/lists/linux-rdma/msg61419.html
>
> Steve.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Leon Romanovsky @ 2018-03-26 17:06 UTC (permalink / raw)
To: Steve Wise; +Cc: David Ahern, stephen, netdev, linux-rdma
In-Reply-To: <09f5080f-04d2-3b5e-74fd-7aba85516097@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 1719 bytes --]
On Mon, Mar 26, 2018 at 10:24:25AM -0500, Steve Wise wrote:
>
>
> On 3/26/2018 10:08 AM, Leon Romanovsky wrote:
> > On Mon, Mar 26, 2018 at 09:55:46AM -0500, Steve Wise wrote:
> >>
> >> On 3/26/2018 9:44 AM, David Ahern wrote:
> >>> On 3/26/18 8:30 AM, Steve Wise wrote:
> >>>> On 3/26/2018 9:17 AM, David Ahern wrote:
> >>>>> On 2/27/18 9:07 AM, Steve Wise wrote:
> >>>>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> >>>>>> index 5809f70..e55205b 100644
> >>>>>> --- a/rdma/rdma.h
> >>>>>> +++ b/rdma/rdma.h
> >>>>>> @@ -18,10 +18,12 @@
> >>>>>> #include <libmnl/libmnl.h>
> >>>>>> #include <rdma/rdma_netlink.h>
> >>>>>> #include <time.h>
> >>>>>> +#include <net/if_arp.h>
> >>>>>>
> >>>>>> #include "list.h"
> >>>>>> #include "utils.h"
> >>>>>> #include "json_writer.h"
> >>>>>> +#include <rdma/rdma_cma.h>
> >>>>>>
> >>>>> did you forget to add rdma_cma.h? I don't see that file in my repo.
> >>>> It is provided by the rdma-core package, upon which rdma tool now
> >>>> depends for the rdma_port_space enum.
> >>>>
> >>> You need to add a check for the package, and only build rdma if that
> >>> package is installed. See check_mnl in configure for an example.
> >> Ok, that makes sense.
> > IMHO, better solution will be to copy those files to iproute2.
>
> Hey Leon,
>
> Why is it better in your opinion? My gut tells me adding rdma_cma.h to
> iproute2 means more uabi type syncing.
Making rdmatool be dependant on rdma-core will require that distributions
will update their specs to install rdma-core as a dependency for every
iprotue2 install.
The rdma-core dependency makes sense for RDMA users, but doesn't for most of
the iproute2 users.
Thanks
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2] of_net: Implement of_get_nvmem_mac_address helper
From: Florian Fainelli @ 2018-03-26 17:05 UTC (permalink / raw)
To: David Miller, mike.looijmans
Cc: netdev, linux-kernel, devicetree, andrew, robh+dt, frowand.list
In-Reply-To: <20180326.125816.1498152294570222508.davem@davemloft.net>
On 03/26/2018 09:58 AM, David Miller wrote:
> From: Mike Looijmans <mike.looijmans@topic.nl>
> Date: Mon, 26 Mar 2018 08:41:29 +0200
>
>> It's common practice to store MAC addresses for network interfaces into
>> nvmem devices. However the code to actually do this in the kernel lacks,
>> so this patch adds of_get_nvmem_mac_address() for drivers to obtain the
>> address from an nvmem cell provider.
>>
>> This is particulary useful on devices where the ethernet interface cannot
>> be configured by the bootloader, for example because it's in an FPGA.
>>
>> Tested by adapting the cadence macb driver to call this instead of
>> of_get_mac_address().
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> ---
>> v2: Use of_nvmem_cell_get to avoid needing the assiciated device
>> Use void* instead of char*
>> Add devicetree binding doc
>
> Like Andrew, I think you should add a new interface for getting the MAC
> address from nvmem.
>
> And drivers can call both of them if they want OF device tree and
> NVMEM probing for MAC addresses.
>
> Later you can add a consolidated interface, if necessary, which does
> both and also take a reference to the MAC address buffer of the driver
> in order to deal with the resource allocation issues.
Agreed, also, how does this fit with Alban's patch series here:
https://lkml.org/lkml/2018/3/24/312
do you depend on those changes at all?
--
Florian
^ permalink raw reply
* Re: [PATCH v5 bpf-next 06/10] tracepoint: compute num_args at build time
From: Steven Rostedt @ 2018-03-26 17:04 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Mathieu Desnoyers, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api,
Frank Ch. Eigler
In-Reply-To: <b07ea9b4-6b73-60aa-9391-002db42c571b@fb.com>
On Mon, 26 Mar 2018 09:47:21 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> I don't mind to _rename_ for_each_kernel_tracepoint() into
> tracepoint_find_by_name(), but keeping exported function
> just to be used by out-of-tree modules would be wrong message for
> the kernel community in general.
> With my patch the for_each_kernel_tracepoint() will be used by bpf side
> and out-of-tree can trivially hack their callbacks to keep working.
> imo that's a better approach then renaming it.
Look, the tracepoint code was written by Mathieu for LTTng, and perf
and ftrace were able to benefit because of it, as well as your bpf code.
For this, we agreed to keep this function around for his use, as its the
only thing he requires. Everyone has been fine with that. Not all out
of tree code is evil. In fact, some out of tree modules help the kernel
community. You ask why I care. Because PREEMPT_RT has been one of those
out of tree modules that has helped the kernel community a lot. Have
you noticed that there are "raw_spin_lock()" and "spin_lock()"? There's
no difference between the two in the kernel. Why have them? Because
they are used by PREEMPT_RT.
Having that function for LTTng does not hurt us. And I will NACK
removing it.
-- Stevwe
^ permalink raw reply
* Re: [PATCH net-next v1 0/4] Converting pernet_operations (part #7.1)
From: David Miller @ 2018-03-26 17:03 UTC (permalink / raw)
To: ktkhai; +Cc: anna.schumaker, trond.myklebust, netdev
In-Reply-To: <152205594525.6058.12471686205983419325.stgit@localhost.localdomain>
From: Kirill Tkhai <ktkhai@virtuozzo.com>
Date: Mon, 26 Mar 2018 12:28:39 +0300
> this is a resending of the 4 patches from path #7.
>
> Anna kindly reviewed them and suggested to take the patches
> through net tree, since there is pernet_operations::async only
> in net-next.git.
>
> There is Anna's acks on every header, the rest of patch
> has no changes.
Thanks for respinning this, it helps me a lot.
Series applied, thanks again.
^ permalink raw reply
* Re: [PATCH net v2] udp6: set dst cache for a connected sk before udp_v6_send_skb
From: Martin KaFai Lau @ 2018-03-26 17:02 UTC (permalink / raw)
To: Alexey Kodanev; +Cc: netdev, Eric Dumazet, David Miller
In-Reply-To: <1522075727-19860-1-git-send-email-alexey.kodanev@oracle.com>
On Mon, Mar 26, 2018 at 05:48:47PM +0300, Alexey Kodanev wrote:
> After commit 33c162a980fe ("ipv6: datagram: Update dst cache of a
> connected datagram sk during pmtu update"), when the error occurs on
> sending datagram in udpv6_sendmsg() due to ICMPV6_PKT_TOOBIG type,
> error handler can trigger the following path and call ip6_dst_store():
>
> udpv6_err()
> ip6_sk_update_pmtu()
> ip6_datagram_dst_update()
> ip6_dst_lookup_flow(), can create a RTF_CACHE clone
Instead of ip6_dst_lookup_flow(),
you meant the RTF_CACHE route created in ip6_update_pmtu()
> ...
> ip6_dst_store()
>
> It can happen before a connected UDP socket invokes ip6_dst_store()
> in the end of udpv6_sendmsg(), on destination release, as a result,
> the last one changes dst to the old one, preventing getting updated
> dst cache on the next udpv6_sendmsg() call.
>
> This patch moves ip6_dst_store() in udpv6_sendmsg(), so that it is
> invoked after ip6_sk_dst_lookup_flow() and before udp_v6_send_skb().
After this patch, the above udpv6_err() path could not happen after
ip6_sk_dst_lookup_flow() and before the ip6_dst_store() in udpv6_sendmsg()?
>
> Also, increase refcnt for dst, when passing it to ip6_dst_store()
> because after that the dst cache can be released by other calls
> to ip6_dst_store() with the same socket.
>
> Fixes: 33c162a980fe ("ipv6: datagram: Update dst cache of a connected datagram sk during pmtu update")
> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
> ---
>
> v2: * remove 'release_dst:' label
>
> * move ip6_dst_store() below MSG_CONFIRM check as
> suggested by Eric and add dst_clone()
>
> * add 'Fixes' commit.
>
>
> net/ipv6/udp.c | 29 +++++++++++------------------
> 1 file changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 52e3ea0..4508e5a 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -1303,6 +1303,16 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> goto do_confirm;
> back_from_confirm:
>
> + if (connected)
> + ip6_dst_store(sk, dst_clone(dst),
> + ipv6_addr_equal(&fl6.daddr, &sk->sk_v6_daddr) ?
> + &sk->sk_v6_daddr : NULL,
> +#ifdef CONFIG_IPV6_SUBTREES
> + ipv6_addr_equal(&fl6.saddr, &np->saddr) ?
> + &np->saddr :
> +#endif
> + NULL);
> +
> /* Lockless fast path for the non-corking case */
> if (!corkreq) {
> struct sk_buff *skb;
> @@ -1314,7 +1324,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> err = PTR_ERR(skb);
> if (!IS_ERR_OR_NULL(skb))
> err = udp_v6_send_skb(skb, &fl6);
> - goto release_dst;
> + goto out;
> }
>
> lock_sock(sk);
> @@ -1348,23 +1358,6 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> err = np->recverr ? net_xmit_errno(err) : 0;
> release_sock(sk);
>
> -release_dst:
> - if (dst) {
> - if (connected) {
> - ip6_dst_store(sk, dst,
> - ipv6_addr_equal(&fl6.daddr, &sk->sk_v6_daddr) ?
> - &sk->sk_v6_daddr : NULL,
> -#ifdef CONFIG_IPV6_SUBTREES
> - ipv6_addr_equal(&fl6.saddr, &np->saddr) ?
> - &np->saddr :
> -#endif
> - NULL);
> - } else {
> - dst_release(dst);
> - }
> - dst = NULL;
> - }
> -
> out:
> dst_release(dst);
> fl6_sock_release(flowlabel);
> --
> 1.8.3.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox