* Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer
From: Linus Torvalds @ 2018-06-28 21:16 UTC (permalink / raw)
To: Al Viro; +Cc: Christoph Hellwig, linux-fsdevel, Network Development, LKP
In-Reply-To: <20180628203746.GJ30522@ZenIV.linux.org.uk>
On Thu, Jun 28, 2018 at 1:37 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
>
> Speaking of obvious bogosities (a lot more so than a blocking allocation
> several calls down into helper):
>
> static __poll_t ca8210_test_int_poll(
> struct file *filp,
> struct poll_table_struct *ptable
> )
Ok, that's just garbage.
Again, this is not an excuse for changing "->poll()". This is just a
bogus driver that does stupid things.
And again, don't use this as an example of "poll must not block" The
fact is, poll() *can* block for locking and other such things, and
it's perfectly normal and ok. It just shouldn't block for IO.
I will not take any misguided patches to try to "fix" poll functions
that might block.
But I will take patches to fix bugs in drivers. In this case, I think
the fix is to just remove that crazy "wait_event_interruptible()"
entirely.
Not that anybody cares, obviously. Apparently nobody has ever noticed
how broken that poll function is.
Linus
^ permalink raw reply
* Re: [PATCH v12 03/10] netdev: cavium: octeon: Add Octeon III BGX Ethernet Nexus
From: Carlos Munoz @ 2018-06-28 21:20 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Steven J. Hill, netdev, Chandrakala Chavva
In-Reply-To: <20180628084156.GF16727@lunn.ch>
On 06/28/2018 01:41 AM, Andrew Lunn wrote:
> External Email
>
>> +static char *mix_port;
>> +module_param(mix_port, charp, 0444);
>> +MODULE_PARM_DESC(mix_port, "Specifies which ports connect to MIX interfaces.");
>> +
>> +static char *pki_port;
>> +module_param(pki_port, charp, 0444);
>> +MODULE_PARM_DESC(pki_port, "Specifies which ports connect to the PKI.");
> Module parameters are generally not liked. Can you do without them?
These parameters change the kernel port assignment required by user space applications. We rather keep them as they simplify the process.
>
>> + /* One time request driver module */
>> + if (is_mix) {
>> + if (atomic_cmpxchg(&request_mgmt_once, 0, 1) == 0)
>> + request_module_nowait("octeon_mgmt");
> Why is this needed? So long as the driver has the needed properties,
> udev should load the module.
>
> Andrew
The thing is the management module is only loaded when a port is assigned to it (determined by the above module parameter "mix_port").
Best regards,
Carlos
^ permalink raw reply
* Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer
From: Al Viro @ 2018-06-28 21:30 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Christoph Hellwig, linux-fsdevel, Network Development, LKP
In-Reply-To: <CA+55aFxfkD9ozw77j-bshReLJN4dN2GEdW+R-LkxztceCW6OTg@mail.gmail.com>
On Thu, Jun 28, 2018 at 02:11:17PM -0700, Linus Torvalds wrote:
> On Thu, Jun 28, 2018 at 1:28 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> >
> > Sure, but...
> >
> > static __poll_t binder_poll(struct file *filp,
> > struct poll_table_struct *wait)
> > {
> > struct binder_proc *proc = filp->private_data;
> > struct binder_thread *thread = NULL;
> > bool wait_for_proc_work;
> >
> > thread = binder_get_thread(proc);
> > if (!thread)
> > return POLLERR;
>
> That's actually fine.
>
> In particular, it's ok to *not* add yourself to the wait-queues if you
> already return the value that you will always return.
Sure (and that's one of the problems I mentioned with ->get_poll_head() model).
But in this case I was refering to GFP_KERNEL allocation down there.
> > And that's hardly unique - we have instances playing with timers,
> > allocations, whatnot. Even straight mutex_lock(), as in
>
> So?
>
> Again, locking is permitted. It's not great, but it's not against the rules.
Me: a *LOT* of ->poll() instances only block in __pollwait() called (indirectly)
on the first pass.
You: They are *all* supposed to do it.
Me: <examples of instances that block elsewhere>
I'm not saying that blocking on other things is a bug; some of such *are* bogus,
but a lot aren't really broken. What I said is that in a lot of cases we really
have hard "no blocking other than in callback" (and on subsequent passes there's
no callback at all). Which is just about perfect for AIO purposes, so *IF* we
go for "new method just for AIO, those who don't have it can take a hike", we might
as well indicate that "can take a hike" in some way (be it opt-in or opt-out) and
use straight unchanged ->poll(), with alternative callback.
Looks like we were talking past each other for the last couple of rounds...
> So none of the things you point to are excuses for changing interfaces
> or adding any flags.
> Anybody who thinks "select cannot block" or "->poll() musn't block" is
> just confused. It has *never* been about that. It waits asynchronously
> for IO, but it may well wait synchronously for locks or memory or just
> "lazy implementation".
Obviously. I *do* understand how poll() works, really.
> The fact is, those interface changes were just broken shit. They were
> confused. I don't actually believe that AIO even needed them.
>
> Christoph, do you have a test program for IOCB_CMD_POLL and what it's
> actually supposed to do?
>
> Because I think that what it can do is simply to do the ->poll() calls
> outside the iocb locks, and then just attach the poll table to the
> kioctx afterwards.
I'd do a bit more - embed the first poll_table_entry into poll iocb itself,
so that the instances that use only one queue wouldn't need any allocations
at all.
^ permalink raw reply
* Re: [PATCH 2/3] MIPS: AR7: Normalize clk API
From: Paul Burton @ 2018-06-28 21:31 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Greg Ungerer, Ralf Baechle, James Hogan, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Corentin Labbe, David S . Miller,
Arnd Bergmann, linux-m68k, linux-mips, netdev, linux-kernel
In-Reply-To: <1528706663-20670-3-git-send-email-geert@linux-m68k.org>
Hi Geert,
On Mon, Jun 11, 2018 at 10:44:22AM +0200, Geert Uytterhoeven wrote:
> Coldfire still provides its own variant of the clk API rather than using
> the generic COMMON_CLK API. This generally works, but it causes some
> link errors with drivers using the clk_round_rate(), clk_set_rate(),
> clk_set_parent(), or clk_get_parent() functions when a platform lacks
> those interfaces.
>
> This adds empty stub implementations for each of them, and I don't even
> try to do something useful here but instead just print a WARN() message
> to make it obvious what is going on if they ever end up being called.
>
> The drivers that call these won't be used on these platforms (otherwise
> we'd get a link error today), so the added code is harmless bloat and
> will warn about accidental use.
>
> Based on commit bd7fefe1f06ca6cc ("ARM: w90x900: normalize clk API").
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> arch/mips/ar7/clock.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
Applied to mips-next for 4.19.
Thanks,
Paul
^ permalink raw reply
* Re: [PATCH bpf-next 08/14] bpf: introduce the bpf_get_local_storage() helper function
From: kbuild test robot @ 2018-06-28 21:34 UTC (permalink / raw)
To: Roman Gushchin
Cc: kbuild-all, netdev, linux-kernel, kernel-team, tj, Roman Gushchin,
Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20180628164719.28215-9-guro@fb.com>
[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]
Hi Roman,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bpf-next/master]
url: https://github.com/0day-ci/linux/commits/Roman-Gushchin/bpf-cgroup-local-storage/20180629-031527
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: parisc-c3000_defconfig (attached as .config)
compiler: hppa-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=parisc
All errors (new ones prefixed by >>):
net/core/filter.o: In function `cg_skb_func_proto':
>> (.text.cg_skb_func_proto+0x18): undefined reference to `bpf_get_local_storage_proto'
(.text.cg_skb_func_proto+0x20): undefined reference to `bpf_get_local_storage_proto'
net/core/filter.o: In function `sock_filter_func_proto':
>> (.text.sock_filter_func_proto+0x1c): undefined reference to `bpf_get_local_storage_proto'
(.text.sock_filter_func_proto+0x3c): undefined reference to `bpf_get_local_storage_proto'
net/core/filter.o: In function `.L1715':
>> (.text.sock_ops_func_proto+0x144): undefined reference to `bpf_get_local_storage_proto'
net/core/filter.o:(.text.sock_ops_func_proto+0x14c): more undefined references to `bpf_get_local_storage_proto' follow
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 14533 bytes --]
^ permalink raw reply
* [PATCH bpf 0/3] Three BPF fixes
From: Daniel Borkmann @ 2018-06-28 21:34 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
This set contains three fixes that are mostly JIT and set_memory_*()
related. The third in the series in particular fixes the syzkaller
bugs that were still pending; aside from local reproduction & testing,
also 'syz test' wasn't able to trigger them anymore. I've tested this
series on x86_64, arm64 and s390x, and kbuild bot wasn't yelling either
for the rest. For details, please see patches as usual, thanks!
Daniel Borkmann (3):
bpf, arm32: fix to use bpf_jit_binary_lock_ro api
bpf, s390: fix potential memleak when later bpf_jit_prog fails
bpf: undo prog rejection on read-only lock failure
arch/arm/net/bpf_jit_32.c | 2 +-
arch/s390/net/bpf_jit_comp.c | 1 +
include/linux/filter.h | 56 +++++++-------------------------------------
kernel/bpf/core.c | 30 +-----------------------
4 files changed, 11 insertions(+), 78 deletions(-)
--
2.9.5
^ permalink raw reply
* [PATCH bpf 1/3] bpf, arm32: fix to use bpf_jit_binary_lock_ro api
From: Daniel Borkmann @ 2018-06-28 21:34 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180628213459.28631-1-daniel@iogearbox.net>
Any eBPF JIT that where its underlying arch supports ARCH_HAS_SET_MEMORY
would need to use bpf_jit_binary_{un,}lock_ro() pair instead of the
set_memory_{ro,rw}() pair directly as otherwise changes to the former
might break. arm32's eBPF conversion missed to change it, so fix this
up here.
Fixes: 39c13c204bb1 ("arm: eBPF JIT compiler")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
arch/arm/net/bpf_jit_32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index 6e8b716..f6a62ae 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1844,7 +1844,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
/* there are 2 passes here */
bpf_jit_dump(prog->len, image_size, 2, ctx.target);
- set_memory_ro((unsigned long)header, header->pages);
+ bpf_jit_binary_lock_ro(header);
prog->bpf_func = (void *)ctx.target;
prog->jited = 1;
prog->jited_len = image_size;
--
2.9.5
^ permalink raw reply related
* [PATCH bpf 2/3] bpf, s390: fix potential memleak when later bpf_jit_prog fails
From: Daniel Borkmann @ 2018-06-28 21:34 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann, Martin Schwidefsky
In-Reply-To: <20180628213459.28631-1-daniel@iogearbox.net>
If we would ever fail in the bpf_jit_prog() pass that writes the
actual insns to the image after we got header via bpf_jit_binary_alloc()
then we also need to make sure to free it through bpf_jit_binary_free()
again when bailing out. Given we had prior bpf_jit_prog() passes to
initially probe for clobbered registers, program size and to fill in
addrs arrray for jump targets, this is more of a theoretical one,
but at least make sure this doesn't break with future changes.
Fixes: 054623105728 ("s390/bpf: Add s390x eBPF JIT compiler backend")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
arch/s390/net/bpf_jit_comp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index d2db8ac..5f0234e 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -1286,6 +1286,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
goto free_addrs;
}
if (bpf_jit_prog(&jit, fp)) {
+ bpf_jit_binary_free(header);
fp = orig_fp;
goto free_addrs;
}
--
2.9.5
^ permalink raw reply related
* [PATCH bpf 3/3] bpf: undo prog rejection on read-only lock failure
From: Daniel Borkmann @ 2018-06-28 21:34 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann, Laura Abbott, Kees Cook
In-Reply-To: <20180628213459.28631-1-daniel@iogearbox.net>
Partially undo commit 9facc336876f ("bpf: reject any prog that failed
read-only lock") since it caused a regression, that is, syzkaller was
able to manage to cause a panic via fault injection deep in set_memory_ro()
path by letting an allocation fail: In x86's __change_page_attr_set_clr()
it was able to change the attributes of the primary mapping but not in
the alias mapping via cpa_process_alias(), so the second, inner call
to the __change_page_attr() via __change_page_attr_set_clr() had to split
a larger page and failed in the alloc_pages() with the artifically triggered
allocation error which is then propagated down to the call site.
Thus, for set_memory_ro() this means that it returned with an error, but
from debugging a probe_kernel_write() revealed EFAULT on that memory since
the primary mapping succeeded to get changed. Therefore the subsequent
hdr->locked = 0 reset triggered the panic as it was performed on read-only
memory, so call-site assumptions were infact wrong to assume that it would
either succeed /or/ not succeed at all since there's no such rollback in
set_memory_*() calls from partial change of mappings, in other words, we're
left in a state that is "half done". A later undo via set_memory_rw() is
succeeding though due to matching permissions on that part (aka due to the
try_preserve_large_page() succeeding). While reproducing locally with
explicitly triggering this error, the initial splitting only happens on
rare occasions and in real world it would additionally need oom conditions,
but that said, it could partially fail. Therefore, it is definitely wrong
to bail out on set_memory_ro() error and reject the program with the
set_memory_*() semantics we have today. Shouldn't have gone the extra mile
since no other user in tree today infact checks for any set_memory_*()
errors, e.g. neither module_enable_ro() / module_disable_ro() for module
RO/NX handling which is mostly default these days nor kprobes core with
alloc_insn_page() / free_insn_page() as examples that could be invoked long
after bootup and original 314beb9bcabf ("x86: bpf_jit_comp: secure bpf jit
against spraying attacks") did neither when it got first introduced to BPF
so "improving" with bailing out was clearly not right when set_memory_*()
cannot handle it today.
Kees suggested that if set_memory_*() can fail, we should annotate it with
__must_check, and all callers need to deal with it gracefully given those
set_memory_*() markings aren't "advisory", but they're expected to actually
do what they say. This might be an option worth to move forward in future
but would at the same time require that set_memory_*() calls from supporting
archs are guaranteed to be "atomic" in that they provide rollback if part
of the range fails, once that happened, the transition from RW -> RO could
be made more robust that way, while subsequent RO -> RW transition /must/
continue guaranteeing to always succeed the undo part.
Reported-by: syzbot+a4eb8c7766952a1ca872@syzkaller.appspotmail.com
Reported-by: syzbot+d866d1925855328eac3b@syzkaller.appspotmail.com
Fixes: 9facc336876f ("bpf: reject any prog that failed read-only lock")
Cc: Laura Abbott <labbott@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/filter.h | 56 ++++++++------------------------------------------
kernel/bpf/core.c | 30 +--------------------------
2 files changed, 9 insertions(+), 77 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 20f2659..300baad 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -470,9 +470,7 @@ struct sock_fprog_kern {
};
struct bpf_binary_header {
- u16 pages;
- u16 locked:1;
-
+ u32 pages;
/* Some arches need word alignment for their instructions */
u8 image[] __aligned(4);
};
@@ -481,7 +479,7 @@ struct bpf_prog {
u16 pages; /* Number of allocated pages */
u16 jited:1, /* Is our filter JIT'ed? */
jit_requested:1,/* archs need to JIT the prog */
- locked:1, /* Program image locked? */
+ undo_set_mem:1, /* Passed set_memory_ro() checkpoint */
gpl_compatible:1, /* Is filter GPL compatible? */
cb_access:1, /* Is control block accessed? */
dst_needed:1, /* Do we need dst entry? */
@@ -677,46 +675,24 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
{
-#ifdef CONFIG_ARCH_HAS_SET_MEMORY
- fp->locked = 1;
- if (set_memory_ro((unsigned long)fp, fp->pages))
- fp->locked = 0;
-#endif
+ fp->undo_set_mem = 1;
+ set_memory_ro((unsigned long)fp, fp->pages);
}
static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
{
-#ifdef CONFIG_ARCH_HAS_SET_MEMORY
- if (fp->locked) {
- WARN_ON_ONCE(set_memory_rw((unsigned long)fp, fp->pages));
- /* In case set_memory_rw() fails, we want to be the first
- * to crash here instead of some random place later on.
- */
- fp->locked = 0;
- }
-#endif
+ if (fp->undo_set_mem)
+ set_memory_rw((unsigned long)fp, fp->pages);
}
static inline void bpf_jit_binary_lock_ro(struct bpf_binary_header *hdr)
{
-#ifdef CONFIG_ARCH_HAS_SET_MEMORY
- hdr->locked = 1;
- if (set_memory_ro((unsigned long)hdr, hdr->pages))
- hdr->locked = 0;
-#endif
+ set_memory_ro((unsigned long)hdr, hdr->pages);
}
static inline void bpf_jit_binary_unlock_ro(struct bpf_binary_header *hdr)
{
-#ifdef CONFIG_ARCH_HAS_SET_MEMORY
- if (hdr->locked) {
- WARN_ON_ONCE(set_memory_rw((unsigned long)hdr, hdr->pages));
- /* In case set_memory_rw() fails, we want to be the first
- * to crash here instead of some random place later on.
- */
- hdr->locked = 0;
- }
-#endif
+ set_memory_rw((unsigned long)hdr, hdr->pages);
}
static inline struct bpf_binary_header *
@@ -728,22 +704,6 @@ bpf_jit_binary_hdr(const struct bpf_prog *fp)
return (void *)addr;
}
-#ifdef CONFIG_ARCH_HAS_SET_MEMORY
-static inline int bpf_prog_check_pages_ro_single(const struct bpf_prog *fp)
-{
- if (!fp->locked)
- return -ENOLCK;
- if (fp->jited) {
- const struct bpf_binary_header *hdr = bpf_jit_binary_hdr(fp);
-
- if (!hdr->locked)
- return -ENOLCK;
- }
-
- return 0;
-}
-#endif
-
int sk_filter_trim_cap(struct sock *sk, struct sk_buff *skb, unsigned int cap);
static inline int sk_filter(struct sock *sk, struct sk_buff *skb)
{
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index a9e6c04..1e5625d 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -598,8 +598,6 @@ bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
bpf_fill_ill_insns(hdr, size);
hdr->pages = size / PAGE_SIZE;
- hdr->locked = 0;
-
hole = min_t(unsigned int, size - (proglen + sizeof(*hdr)),
PAGE_SIZE - sizeof(*hdr));
start = (get_random_int() % hole) & ~(alignment - 1);
@@ -1450,22 +1448,6 @@ static int bpf_check_tail_call(const struct bpf_prog *fp)
return 0;
}
-static int bpf_prog_check_pages_ro_locked(const struct bpf_prog *fp)
-{
-#ifdef CONFIG_ARCH_HAS_SET_MEMORY
- int i, err;
-
- for (i = 0; i < fp->aux->func_cnt; i++) {
- err = bpf_prog_check_pages_ro_single(fp->aux->func[i]);
- if (err)
- return err;
- }
-
- return bpf_prog_check_pages_ro_single(fp);
-#endif
- return 0;
-}
-
static void bpf_prog_select_func(struct bpf_prog *fp)
{
#ifndef CONFIG_BPF_JIT_ALWAYS_ON
@@ -1524,17 +1506,7 @@ struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
* all eBPF JITs might immediately support all features.
*/
*err = bpf_check_tail_call(fp);
- if (*err)
- return fp;
-
- /* Checkpoint: at this point onwards any cBPF -> eBPF or
- * native eBPF program is read-only. If we failed to change
- * the page attributes (e.g. allocation failure from
- * splitting large pages), then reject the whole program
- * in order to guarantee not ending up with any W+X pages
- * from BPF side in kernel.
- */
- *err = bpf_prog_check_pages_ro_locked(fp);
+
return fp;
}
EXPORT_SYMBOL_GPL(bpf_prog_select_runtime);
--
2.9.5
^ permalink raw reply related
* Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer
From: Linus Torvalds @ 2018-06-28 21:39 UTC (permalink / raw)
To: Al Viro; +Cc: Christoph Hellwig, linux-fsdevel, Network Development, LKP
In-Reply-To: <20180628213027.GK30522@ZenIV.linux.org.uk>
On Thu, Jun 28, 2018 at 2:30 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> > Again, locking is permitted. It's not great, but it's not against the rules.
>
> Me: a *LOT* of ->poll() instances only block in __pollwait() called (indirectly)
> on the first pass.
>
> You: They are *all* supposed to do it.
>
> Me: <examples of instances that block elsewhere>
Oh, I thought you were talking about the whole "first pass" adding to
wait queues, as opposed to doing it on the second pass.
The *blocking* is entirely immaterial. I didn't even react to it,
because it's simply not an issue.
I don't understand why you're even hung up about it.
The only reason "blocking" seems to be an issu eis because AIO has
shit-for-brains and wanted to do poll() under the spinlock.
But that's literally just AIO being confused garbage. It has zero
relevance for anything else.
Linus
^ permalink raw reply
* Re: [PATCH v2 net-next 0/2] net: preserve sock reference when scrubbing the skb.
From: Cong Wang @ 2018-06-28 21:41 UTC (permalink / raw)
To: David Miller
Cc: Flavio Leitner, Linux Kernel Network Developers, Eric Dumazet,
Paolo Abeni, Florian Westphal, NetFilter
In-Reply-To: <20180628.222040.2117056805629988850.davem@davemloft.net>
On Thu, Jun 28, 2018 at 6:20 AM David Miller <davem@davemloft.net> wrote:
>
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Wed, 27 Jun 2018 12:39:01 -0700
>
> > Let me rephrase why I don't like this patchset:
>
> Cong, I don't think you are seeing the situation clearly and
> I am certainly going to apply this patch series even in the
> face of your objections.
>
> Suggesting that solving the lack of back pressure on a UDP
> socket caused by this problem by using cgroups or cpu
> usage controllers is just complete and utter madness.
Pretty sure you didn't even read the rest of my reply,
I can't help you if you just stop at where you quoted.
^ permalink raw reply
* [PATCH bpf-next 0/8] tools: bpf: updates to bpftool and libbpf
From: Jakub Kicinski @ 2018-06-28 21:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
Hi!
Set of random updates to bpftool and libbpf. I'm preparing for
extending bpftool prog load, but there is a good number of
improvements that can be made before bpf -> bpf-next merge
helping to keep the later patch set to a manageable size as well.
First patch is a bpftool build speed improvement. Next missing
program types are added to libbpf program type detection by section
name. The ability to load programs from '.text' section is restored
when ELF file doesn't contain any pseudo calls.
In bpftool I remove my Author comments as unnecessary sign of vanity.
Last but not least missing option is added to bash completions and
processing of options in bash completions is improved.
Jakub Kicinski (8):
tools: bpftool: use correct make variable type to improve compilation
time
tools: libbpf: add section names for missing program types
tools: libbpf: allow setting ifindex for programs and maps
tools: libbpf: restore the ability to load programs from .text section
tools: libbpf: don't return '.text' as a program for multi-function
programs
tools: bpftool: drop unnecessary Author comments
tools: bpftool: add missing --bpffs to completions
tools: bpftool: deal with options upfront
tools/bpf/bpftool/Makefile | 2 +-
tools/bpf/bpftool/bash-completion/bpftool | 32 ++++++++++-----
tools/bpf/bpftool/common.c | 2 -
tools/bpf/bpftool/main.c | 4 +-
tools/bpf/bpftool/main.h | 2 -
tools/bpf/bpftool/map.c | 2 -
tools/bpf/bpftool/prog.c | 4 +-
tools/lib/bpf/libbpf.c | 49 ++++++++++++++++++-----
tools/lib/bpf/libbpf.h | 2 +
9 files changed, 66 insertions(+), 33 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH bpf-next 1/8] tools: bpftool: use correct make variable type to improve compilation time
From: Jakub Kicinski @ 2018-06-28 21:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20180628214142.11268-1-jakub.kicinski@netronome.com>
Commit 4bfe3bd3cc35 ("tools/bpftool: use version from the kernel
source tree") added version to bpftool. The version used is
equal to the kernel version and obtained by running make kernelversion
against kernel source tree. Version is then communicated
to the sources with a command line define set in CFLAGS.
Use a simply expanded variable for the version, otherwise the
recursive make will run every time CFLAGS are used.
This brings the single-job compilation time for me from almost
16 sec down to less than 4 sec.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/bpf/bpftool/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 892dbf095bff..0911b00b25cc 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -23,7 +23,7 @@ endif
LIBBPF = $(BPF_PATH)libbpf.a
-BPFTOOL_VERSION=$(shell make --no-print-directory -sC ../../.. kernelversion)
+BPFTOOL_VERSION := $(shell make --no-print-directory -sC ../../.. kernelversion)
$(LIBBPF): FORCE
$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) $(OUTPUT)libbpf.a FEATURES_DUMP=$(FEATURE_DUMP_EXPORT)
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 2/8] tools: libbpf: add section names for missing program types
From: Jakub Kicinski @ 2018-06-28 21:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20180628214142.11268-1-jakub.kicinski@netronome.com>
Specify default section names for BPF_PROG_TYPE_LIRC_MODE2
and BPF_PROG_TYPE_LWT_SEG6LOCAL, these are the only two
missing right now.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/lib/bpf/libbpf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a1e96b5de5ff..a1491e95edd0 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2037,9 +2037,11 @@ static const struct {
BPF_PROG_SEC("lwt_in", BPF_PROG_TYPE_LWT_IN),
BPF_PROG_SEC("lwt_out", BPF_PROG_TYPE_LWT_OUT),
BPF_PROG_SEC("lwt_xmit", BPF_PROG_TYPE_LWT_XMIT),
+ BPF_PROG_SEC("lwt_seg6local", BPF_PROG_TYPE_LWT_SEG6LOCAL),
BPF_PROG_SEC("sockops", BPF_PROG_TYPE_SOCK_OPS),
BPF_PROG_SEC("sk_skb", BPF_PROG_TYPE_SK_SKB),
BPF_PROG_SEC("sk_msg", BPF_PROG_TYPE_SK_MSG),
+ BPF_PROG_SEC("lirc_mode2", BPF_PROG_TYPE_LIRC_MODE2),
BPF_SA_PROG_SEC("cgroup/bind4", BPF_CGROUP_INET4_BIND),
BPF_SA_PROG_SEC("cgroup/bind6", BPF_CGROUP_INET6_BIND),
BPF_SA_PROG_SEC("cgroup/connect4", BPF_CGROUP_INET4_CONNECT),
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 3/8] tools: libbpf: allow setting ifindex for programs and maps
From: Jakub Kicinski @ 2018-06-28 21:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20180628214142.11268-1-jakub.kicinski@netronome.com>
Users of bpf_object__open()/bpf_object__load() APIs may want to
load the programs and maps onto a device for offload. Allow
setting ifindex on those sub-objects.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/lib/bpf/libbpf.c | 10 ++++++++++
tools/lib/bpf/libbpf.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a1491e95edd0..7bc02d93e277 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1896,6 +1896,11 @@ void *bpf_program__priv(struct bpf_program *prog)
return prog ? prog->priv : ERR_PTR(-EINVAL);
}
+void bpf_program__set_ifindex(struct bpf_program *prog, __u32 ifindex)
+{
+ prog->prog_ifindex = ifindex;
+}
+
const char *bpf_program__title(struct bpf_program *prog, bool needs_copy)
{
const char *title;
@@ -2122,6 +2127,11 @@ void *bpf_map__priv(struct bpf_map *map)
return map ? map->priv : ERR_PTR(-EINVAL);
}
+void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex)
+{
+ map->map_ifindex = ifindex;
+}
+
struct bpf_map *
bpf_map__next(struct bpf_map *prev, struct bpf_object *obj)
{
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 09976531aa74..564f4be9bae0 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -109,6 +109,7 @@ int bpf_program__set_priv(struct bpf_program *prog, void *priv,
bpf_program_clear_priv_t clear_priv);
void *bpf_program__priv(struct bpf_program *prog);
+void bpf_program__set_ifindex(struct bpf_program *prog, __u32 ifindex);
const char *bpf_program__title(struct bpf_program *prog, bool needs_copy);
@@ -251,6 +252,7 @@ typedef void (*bpf_map_clear_priv_t)(struct bpf_map *, void *);
int bpf_map__set_priv(struct bpf_map *map, void *priv,
bpf_map_clear_priv_t clear_priv);
void *bpf_map__priv(struct bpf_map *map);
+void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex);
int bpf_map__pin(struct bpf_map *map, const char *path);
long libbpf_get_error(const void *ptr);
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 4/8] tools: libbpf: restore the ability to load programs from .text section
From: Jakub Kicinski @ 2018-06-28 21:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20180628214142.11268-1-jakub.kicinski@netronome.com>
libbpf used to be able to load programs from the default section
called '.text'. It's not very common to leave sections unnamed,
but if it happens libbpf will fail to load the programs reporting
-EINVAL from the kernel. The -EINVAL comes from bpf_obj_name_cpy()
because since 48cca7e44f9f ("libbpf: add support for bpf_call")
libbpf does not resolve program names for programs in '.text',
defaulting to '.text'. '.text', however, does not pass the
(isalnum(*src) || *src == '_') check in bpf_obj_name_cpy().
With few extra lines of code we can limit the pseudo call
assumptions only to objects which actually contain code relocations.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/lib/bpf/libbpf.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7bc02d93e277..e2401b95f08d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -234,6 +234,7 @@ struct bpf_object {
size_t nr_maps;
bool loaded;
+ bool has_pseudo_calls;
/*
* Information when doing elf related work. Only valid if fd
@@ -400,10 +401,6 @@ bpf_object__init_prog_names(struct bpf_object *obj)
const char *name = NULL;
prog = &obj->programs[pi];
- if (prog->idx == obj->efile.text_shndx) {
- name = ".text";
- goto skip_search;
- }
for (si = 0; si < symbols->d_size / sizeof(GElf_Sym) && !name;
si++) {
@@ -426,12 +423,15 @@ bpf_object__init_prog_names(struct bpf_object *obj)
}
}
+ if (!name && prog->idx == obj->efile.text_shndx)
+ name = ".text";
+
if (!name) {
pr_warning("failed to find sym for prog %s\n",
prog->section_name);
return -EINVAL;
}
-skip_search:
+
prog->name = strdup(name);
if (!prog->name) {
pr_warning("failed to allocate memory for prog sym %s\n",
@@ -981,6 +981,7 @@ bpf_program__collect_reloc(struct bpf_program *prog, GElf_Shdr *shdr,
prog->reloc_desc[i].type = RELO_CALL;
prog->reloc_desc[i].insn_idx = insn_idx;
prog->reloc_desc[i].text_off = sym.st_value;
+ obj->has_pseudo_calls = true;
continue;
}
@@ -1426,6 +1427,12 @@ bpf_program__load(struct bpf_program *prog,
return err;
}
+static bool bpf_program__is_function_storage(struct bpf_program *prog,
+ struct bpf_object *obj)
+{
+ return prog->idx == obj->efile.text_shndx && obj->has_pseudo_calls;
+}
+
static int
bpf_object__load_progs(struct bpf_object *obj)
{
@@ -1433,7 +1440,7 @@ bpf_object__load_progs(struct bpf_object *obj)
int err;
for (i = 0; i < obj->nr_programs; i++) {
- if (obj->programs[i].idx == obj->efile.text_shndx)
+ if (bpf_program__is_function_storage(&obj->programs[i], obj))
continue;
err = bpf_program__load(&obj->programs[i],
obj->license,
@@ -2247,7 +2254,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
bpf_program__set_expected_attach_type(prog,
expected_attach_type);
- if (prog->idx != obj->efile.text_shndx && !first_prog)
+ if (!bpf_program__is_function_storage(prog, obj) && !first_prog)
first_prog = prog;
}
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 5/8] tools: libbpf: don't return '.text' as a program for multi-function programs
From: Jakub Kicinski @ 2018-06-28 21:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20180628214142.11268-1-jakub.kicinski@netronome.com>
Make bpf_program__next() skip over '.text' section if object file
has pseudo calls. The '.text' section is hardly a program in that
case, it's more of a storage for code of functions other than main.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/lib/bpf/libbpf.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e2401b95f08d..38ed3e92e393 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1865,8 +1865,8 @@ void *bpf_object__priv(struct bpf_object *obj)
return obj ? obj->priv : ERR_PTR(-EINVAL);
}
-struct bpf_program *
-bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
+static struct bpf_program *
+__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
size_t idx;
@@ -1887,6 +1887,18 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
return &obj->programs[idx];
}
+struct bpf_program *
+bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
+{
+ struct bpf_program *prog = prev;
+
+ do {
+ prog = __bpf_program__next(prog, obj);
+ } while (prog && bpf_program__is_function_storage(prog, obj));
+
+ return prog;
+}
+
int bpf_program__set_priv(struct bpf_program *prog, void *priv,
bpf_program_clear_priv_t clear_priv)
{
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 6/8] tools: bpftool: drop unnecessary Author comments
From: Jakub Kicinski @ 2018-06-28 21:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20180628214142.11268-1-jakub.kicinski@netronome.com>
Drop my author comments, those are from the early days of
bpftool and make little sense in tree, where we have quite
a few people contributing and git to attribute the work.
While at it bump some copyrights.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/bpf/bpftool/common.c | 2 --
tools/bpf/bpftool/main.c | 4 +---
tools/bpf/bpftool/main.h | 2 --
tools/bpf/bpftool/map.c | 2 --
tools/bpf/bpftool/prog.c | 4 +---
5 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 32f9e397a6c0..b432daea4520 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -31,8 +31,6 @@
* SOFTWARE.
*/
-/* Author: Jakub Kicinski <kubakici@wp.pl> */
-
#include <ctype.h>
#include <errno.h>
#include <fcntl.h>
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index eea7f14355f3..d15a62be6cf0 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2017 Netronome Systems, Inc.
+ * Copyright (C) 2017-2018 Netronome Systems, Inc.
*
* This software is dual licensed under the GNU General License Version 2,
* June 1991 as shown in the file COPYING in the top-level directory of this
@@ -31,8 +31,6 @@
* SOFTWARE.
*/
-/* Author: Jakub Kicinski <kubakici@wp.pl> */
-
#include <bfd.h>
#include <ctype.h>
#include <errno.h>
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 63fdb310b9a4..d39f7ef01d23 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -31,8 +31,6 @@
* SOFTWARE.
*/
-/* Author: Jakub Kicinski <kubakici@wp.pl> */
-
#ifndef __BPF_TOOL_H
#define __BPF_TOOL_H
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 097b1a5e046b..5989e1575ae4 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -31,8 +31,6 @@
* SOFTWARE.
*/
-/* Author: Jakub Kicinski <kubakici@wp.pl> */
-
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 05f42a46d6ed..fd8cd9b51621 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2017 Netronome Systems, Inc.
+ * Copyright (C) 2017-2018 Netronome Systems, Inc.
*
* This software is dual licensed under the GNU General License Version 2,
* June 1991 as shown in the file COPYING in the top-level directory of this
@@ -31,8 +31,6 @@
* SOFTWARE.
*/
-/* Author: Jakub Kicinski <kubakici@wp.pl> */
-
#include <errno.h>
#include <fcntl.h>
#include <stdarg.h>
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 7/8] tools: bpftool: add missing --bpffs to completions
From: Jakub Kicinski @ 2018-06-28 21:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20180628214142.11268-1-jakub.kicinski@netronome.com>
--bpffs is not suggested by bash completions.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/bpf/bpftool/bash-completion/bpftool | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 1e1083321643..b0b8022d3570 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -182,7 +182,7 @@ _bpftool()
if [[ -z $object ]]; then
case $cur in
-*)
- local c='--version --json --pretty'
+ local c='--version --json --pretty --bpffs'
COMPREPLY=( $( compgen -W "$c" -- "$cur" ) )
return 0
;;
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 8/8] tools: bpftool: deal with options upfront
From: Jakub Kicinski @ 2018-06-28 21:41 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20180628214142.11268-1-jakub.kicinski@netronome.com>
Remove options (in getopt() sense, i.e. starting with a dash like
-n or --NAME) while parsing arguments for bash completions. This
allows us to refer to position-dependent parameters better, and
complete options at any point.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
tools/bpf/bpftool/bash-completion/bpftool | 32 +++++++++++++++--------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index b0b8022d3570..fffd76f4998b 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -153,6 +153,13 @@ _bpftool()
local cur prev words objword
_init_completion || return
+ # Deal with options
+ if [[ ${words[cword]} == -* ]]; then
+ local c='--version --json --pretty --bpffs'
+ COMPREPLY=( $( compgen -W "$c" -- "$cur" ) )
+ return 0
+ fi
+
# Deal with simplest keywords
case $prev in
help|hex|opcodes|visual)
@@ -172,20 +179,23 @@ _bpftool()
;;
esac
- # Search for object and command
- local object command cmdword
- for (( cmdword=1; cmdword < ${#words[@]}-1; cmdword++ )); do
- [[ -n $object ]] && command=${words[cmdword]} && break
- [[ ${words[cmdword]} != -* ]] && object=${words[cmdword]}
+ # Remove all options so completions don't have to deal with them.
+ local i
+ for (( i=1; i < ${#words[@]}; )); do
+ if [[ ${words[i]::1} == - ]]; then
+ words=( "${words[@]:0:i}" "${words[@]:i+1}" )
+ [[ $i -le $cword ]] && cword=$(( cword - 1 ))
+ else
+ i=$(( ++i ))
+ fi
done
+ cur=${words[cword]}
+ prev=${words[cword - 1]}
- if [[ -z $object ]]; then
+ local object=${words[1]} command=${words[2]}
+
+ if [[ -z $object || $cword -eq 1 ]]; then
case $cur in
- -*)
- local c='--version --json --pretty --bpffs'
- COMPREPLY=( $( compgen -W "$c" -- "$cur" ) )
- return 0
- ;;
*)
COMPREPLY=( $( compgen -W "$( bpftool help 2>&1 | \
command sed \
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next] net: preserve sock reference when scrubbing the skb.
From: Cong Wang @ 2018-06-28 21:51 UTC (permalink / raw)
To: Flavio Leitner
Cc: Eric Dumazet, Linux Kernel Network Developers, Paolo Abeni,
David Miller, Florian Westphal, NetFilter
In-Reply-To: <20180627201937.GZ19565@plex.lan>
On Wed, Jun 27, 2018 at 1:19 PM Flavio Leitner <fbl@redhat.com> wrote:
>
> On Wed, Jun 27, 2018 at 12:06:16PM -0700, Cong Wang wrote:
> > On Wed, Jun 27, 2018 at 5:32 AM Flavio Leitner <fbl@redhat.com> wrote:
> > >
> > > On Tue, Jun 26, 2018 at 06:28:27PM -0700, Cong Wang wrote:
> > > > On Tue, Jun 26, 2018 at 5:39 PM Flavio Leitner <fbl@redhat.com> wrote:
> > > > >
> > > > > On Tue, Jun 26, 2018 at 05:29:51PM -0700, Cong Wang wrote:
> > > > > > On Tue, Jun 26, 2018 at 4:33 PM Flavio Leitner <fbl@redhat.com> wrote:
> > > > > > >
> > > > > > > It is still isolated, the sk carries the netns info and it is
> > > > > > > orphaned when it re-enters the stack.
> > > > > >
> > > > > > Then what difference does your patch make?
> > > > >
> > > > > Don't forget it is fixing two issues.
> > > >
> > > > Sure. I am only talking about TSQ from the very beginning.
> > > > Let me rephrase my above question:
> > > > What difference does your patch make to TSQ?
> > >
> > > It avoids burstiness.
> >
> > Never even mentioned in changelog or in your patch. :-/
>
> It's part of queueing and helping the bufferbloat problem in the
> commit message.
Please don't add all queues in this scope. Are you really
going to put all queues in networking into your "bufferbloat" claim?
Seriously? Please get it defined, seriously. You really need to
read into the other reply from me, none of you or David even
seriously finish reading it.
>
> > > > > > Before your patch:
> > > > > > veth orphans skb in its xmit
> > > > > >
> > > > > > After your patch:
> > > > > > RX orphans it when re-entering stack (as you claimed, I don't know)
> > > > >
> > > > > ip_rcv, and equivalents.
> > > >
> > > > ip_rcv() is L3, we enter a stack from L1. So your above claim is incorrect. :)
> > >
> > > Maybe you found a problem, could you please point me to where in
> > > between L1 to L3 the socket is relevant?
> >
> > Of course, ingress qdisc is in L2. Do I need to say more? This
> > is where we can re-route the packets, for example, redirecting it to
> > yet another netns. This is in fact what we use in production, not anything
> > that only in my imagination.
> >
> > You really have to think about why you allow a different netns influence
> > another netns by holding the skb to throttle the source TCP socket.
>
> Maybe I wasn't clear and you didn't understand the question. Please find
> a spot where the preserved socket is used incorrectly.
It's sad you still don't get what I mean, I never complain you leak skb->sk,
I complain you break TSQ. Dragging discussion into skb->sk doesn't
even help.
>
> > > > which means you have to update Documentation/networking/ip-sysctl.txt
> > > > too.
> > >
> > > How it is never targeted? Whole point is to avoid queueing traffic.
> >
> > What queues? You really need to define it, seriously.
> >
> >
> > > Would you be okay if I include this chunk?
> >
> > No, still lack of an explanation why it comes across netns for
> > a good reason.
>
> Because it doesn't. Since you talk more about veth, let's pick it
> as an example. The TX is nothing more than add to the CPU backlog,
That's RX, assume "CPU backlog" here still means softnet_data.
> right? That is netns agnostic. The same for processing that queue
> which will push the skb anyways and will call skb_orphan().
Once it leaves TX, it leaves the stack. skb_orphan() called
in L3 (as you claimed) is already in yet another stack.
>
> How can one netns avoid/delay the skb_orphan()? And even if does
> that, what gain will you have to allow queuing of more and more
> packets in the CPU backlog? It is stalled.
Please read the other reply from me, you don't even understand
what a boundary of a stack is.
^ permalink raw reply
* [pull request][net-next 00/12] Mellanox, mlx5e updates 2018-06-28
From: Saeed Mahameed @ 2018-06-28 21:50 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Or Gerlitz, Saeed Mahameed
Hi Dave,
The following pull request includes updates for mlx5e netdevice driver.
For more information please see tag log below.
Please pull and let me know if there's any problem.
Thanks,
Saeed.
---
The following changes since commit 7861552cedd81a164c0d5d1c89fe2cb45a3ed41b:
netlink: Return extack message if attribute validation fails (2018-06-28 16:18:04 +0900)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git tags/mlx5e-updates-2018-06-28
for you to fetch changes up to ed56c5193ad89d1097cdbdc87abeb062e03a06eb:
net/mlx5e: Update NIC HW stats on demand only (2018-06-28 14:44:38 -0700)
----------------------------------------------------------------
mlx5e-updates-2018-06-28
mlx5e netdevice driver updates:
- Boris Pismenny added the support for UDP GSO in the first two patches.
Impressive performance numbers are included in the commit message,
@Line rate with ~half of the cpu utilization compared to non offload
or no GSO at all.
- From Tariq Toukan:
- Convert large order kzalloc allocations to kvzalloc.
- Added performance diagnostic statistics to several places in data path.
>From Saeed and Eran,
- Update NIC HW stats on demand only, this is to eliminate the background
thread needed to update some HW statistics in the driver cache in
order to report error and drop counters from HW in ndo_get_stats.
----------------------------------------------------------------
Boris Pismenny (2):
net/mlx5e: Add UDP GSO support
net/mlx5e: Add UDP GSO remaining counter
Saeed Mahameed (1):
net/mlx5e: Update NIC HW stats on demand only
Tariq Toukan (9):
net/mlx5e: Convert large order kzalloc allocations to kvzalloc
net/mlx5e: RX, Use existing WQ local variable
net/mlx5e: Add TX completions statistics
net/mlx5e: Add XDP_TX completions statistics
net/mlx5e: Add NAPI statistics
net/mlx5e: Add a counter for congested UMRs
net/mlx5e: Add channel events counter
net/mlx5e: Add counter for MPWQE filler strides
net/mlx5e: Add counter for total num of NOP operations
drivers/net/ethernet/mellanox/mlx5/core/Makefile | 4 +-
drivers/net/ethernet/mellanox/mlx5/core/en.h | 1 -
.../mellanox/mlx5/core/en_accel/en_accel.h | 11 ++-
.../ethernet/mellanox/mlx5/core/en_accel/rxtx.c | 109 +++++++++++++++++++++
.../ethernet/mellanox/mlx5/core/en_accel/rxtx.h | 14 +++
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 57 ++++++-----
drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 3 +
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 11 ++-
drivers/net/ethernet/mellanox/mlx5/core/en_stats.c | 34 ++++++-
drivers/net/ethernet/mellanox/mlx5/core/en_stats.h | 25 ++++-
drivers/net/ethernet/mellanox/mlx5/core/en_tx.c | 17 +++-
drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 9 +-
drivers/net/ethernet/mellanox/mlx5/core/wq.h | 5 +
13 files changed, 252 insertions(+), 48 deletions(-)
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.h
^ permalink raw reply
* [net-next 02/12] net/mlx5e: Add UDP GSO remaining counter
From: Saeed Mahameed @ 2018-06-28 21:50 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Or Gerlitz, Boris Pismenny, Saeed Mahameed
In-Reply-To: <20180628215103.9141-1-saeedm@mellanox.com>
From: Boris Pismenny <borisp@mellanox.com>
This patch adds a counter for tx UDP GSO packets that contain a segment
that is not aligned to MSS - remaining segment.
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c | 1 +
drivers/net/ethernet/mellanox/mlx5/core/en_stats.c | 2 ++
drivers/net/ethernet/mellanox/mlx5/core/en_stats.h | 2 ++
3 files changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c
index 4bb1f3b12b96..7b7ec3998e84 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c
@@ -92,6 +92,7 @@ struct sk_buff *mlx5e_udp_gso_handle_tx_skb(struct net_device *netdev,
if (!remaining)
return skb;
+ sq->stats->udp_seg_rem++;
nskb = alloc_skb(max_t(int, headlen, headlen + remaining - skb->data_len), GFP_ATOMIC);
if (unlikely(!nskb)) {
sq->stats->dropped++;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index 1646859974ce..7e7155b4e0f0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -68,6 +68,7 @@ static const struct counter_desc sw_stats_desc[] = {
{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_xmit_more) },
{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_recover) },
{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_queue_wake) },
+ { MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_udp_seg_rem) },
{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_cqe_err) },
{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_wqe_err) },
{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, rx_mpwqe_filler) },
@@ -159,6 +160,7 @@ void mlx5e_grp_sw_update_stats(struct mlx5e_priv *priv)
s->tx_added_vlan_packets += sq_stats->added_vlan_packets;
s->tx_queue_stopped += sq_stats->stopped;
s->tx_queue_wake += sq_stats->wake;
+ s->tx_udp_seg_rem += sq_stats->udp_seg_rem;
s->tx_queue_dropped += sq_stats->dropped;
s->tx_cqe_err += sq_stats->cqe_err;
s->tx_recover += sq_stats->recover;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
index 643153bb3607..d416bb86e747 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
@@ -79,6 +79,7 @@ struct mlx5e_sw_stats {
u64 tx_xmit_more;
u64 tx_recover;
u64 tx_queue_wake;
+ u64 tx_udp_seg_rem;
u64 tx_cqe_err;
u64 rx_wqe_err;
u64 rx_mpwqe_filler;
@@ -196,6 +197,7 @@ struct mlx5e_sq_stats {
u64 csum_partial_inner;
u64 added_vlan_packets;
u64 nop;
+ u64 udp_seg_rem;
#ifdef CONFIG_MLX5_EN_TLS
u64 tls_ooo;
u64 tls_resync_bytes;
--
2.17.0
^ permalink raw reply related
* [net-next 01/12] net/mlx5e: Add UDP GSO support
From: Saeed Mahameed @ 2018-06-28 21:50 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Or Gerlitz, Boris Pismenny, Yossi Kuperman,
Saeed Mahameed
In-Reply-To: <20180628215103.9141-1-saeedm@mellanox.com>
From: Boris Pismenny <borisp@mellanox.com>
This patch enables UDP GSO support. We enable this by using two WQEs
the first is a UDP LSO WQE for all segments with equal length, and the
second is for the last segment in case it has different length.
Due to HW limitation, before sending, we must adjust the packet length fields.
We measure performance between two Intel(R) Xeon(R) CPU E5-2643 v2 @3.50GHz
machines connected back-to-back with Connectx4-Lx (40Gbps) NICs.
We compare single stream UDP, UDP GSO and UDP GSO with offload.
Performance:
| MSS (bytes) | Throughput (Gbps) | CPU utilization (%)
UDP GSO offload | 1472 | 35.6 | 8%
UDP GSO | 1472 | 25.5 | 17%
UDP | 1472 | 10.2 | 17%
UDP GSO offload | 1024 | 35.6 | 8%
UDP GSO | 1024 | 19.2 | 17%
UDP | 1024 | 5.7 | 17%
UDP GSO offload | 512 | 33.8 | 16%
UDP GSO | 512 | 10.4 | 17%
UDP | 512 | 3.5 | 17%
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Yossi Kuperman <yossiku@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
.../net/ethernet/mellanox/mlx5/core/Makefile | 4 +-
.../mellanox/mlx5/core/en_accel/en_accel.h | 11 +-
.../mellanox/mlx5/core/en_accel/rxtx.c | 108 ++++++++++++++++++
.../mellanox/mlx5/core/en_accel/rxtx.h | 14 +++
.../net/ethernet/mellanox/mlx5/core/en_main.c | 3 +
.../net/ethernet/mellanox/mlx5/core/en_tx.c | 8 +-
6 files changed, 139 insertions(+), 9 deletions(-)
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.h
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
index 9efbf193ad5a..d923f2f58608 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
@@ -14,8 +14,8 @@ mlx5_core-$(CONFIG_MLX5_FPGA) += fpga/cmd.o fpga/core.o fpga/conn.o fpga/sdk.o \
fpga/ipsec.o fpga/tls.o
mlx5_core-$(CONFIG_MLX5_CORE_EN) += en_main.o en_common.o en_fs.o en_ethtool.o \
- en_tx.o en_rx.o en_dim.o en_txrx.o en_stats.o vxlan.o \
- en_arfs.o en_fs_ethtool.o en_selftest.o en/port.o
+ en_tx.o en_rx.o en_dim.o en_txrx.o en_accel/rxtx.o en_stats.o \
+ vxlan.o en_arfs.o en_fs_ethtool.o en_selftest.o en/port.o
mlx5_core-$(CONFIG_MLX5_MPFS) += lib/mpfs.o
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
index f20074dbef32..39a5d13ba459 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
@@ -34,12 +34,11 @@
#ifndef __MLX5E_EN_ACCEL_H__
#define __MLX5E_EN_ACCEL_H__
-#ifdef CONFIG_MLX5_ACCEL
-
#include <linux/skbuff.h>
#include <linux/netdevice.h>
#include "en_accel/ipsec_rxtx.h"
#include "en_accel/tls_rxtx.h"
+#include "en_accel/rxtx.h"
#include "en.h"
static inline struct sk_buff *mlx5e_accel_handle_tx(struct sk_buff *skb,
@@ -64,9 +63,13 @@ static inline struct sk_buff *mlx5e_accel_handle_tx(struct sk_buff *skb,
}
#endif
+ if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) {
+ skb = mlx5e_udp_gso_handle_tx_skb(dev, sq, skb, wqe, pi);
+ if (unlikely(!skb))
+ return NULL;
+ }
+
return skb;
}
-#endif /* CONFIG_MLX5_ACCEL */
-
#endif /* __MLX5E_EN_ACCEL_H__ */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c
new file mode 100644
index 000000000000..4bb1f3b12b96
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.c
@@ -0,0 +1,108 @@
+#include "en_accel/rxtx.h"
+
+static void mlx5e_udp_gso_prepare_last_skb(struct sk_buff *skb,
+ struct sk_buff *nskb,
+ int remaining)
+{
+ int bytes_needed = remaining, remaining_headlen, remaining_page_offset;
+ int headlen = skb_transport_offset(skb) + sizeof(struct udphdr);
+ int payload_len = remaining + sizeof(struct udphdr);
+ int k = 0, i, j;
+
+ skb_copy_bits(skb, 0, nskb->data, headlen);
+ nskb->dev = skb->dev;
+ skb_reset_mac_header(nskb);
+ skb_set_network_header(nskb, skb_network_offset(skb));
+ skb_set_transport_header(nskb, skb_transport_offset(skb));
+ skb_set_tail_pointer(nskb, headlen);
+
+ /* How many frags do we need? */
+ for (i = skb_shinfo(skb)->nr_frags - 1; i >= 0; i--) {
+ bytes_needed -= skb_frag_size(&skb_shinfo(skb)->frags[i]);
+ k++;
+ if (bytes_needed <= 0)
+ break;
+ }
+
+ /* Fill the first frag and split it if necessary */
+ j = skb_shinfo(skb)->nr_frags - k;
+ remaining_page_offset = -bytes_needed;
+ skb_fill_page_desc(nskb, 0,
+ skb_shinfo(skb)->frags[j].page.p,
+ skb_shinfo(skb)->frags[j].page_offset + remaining_page_offset,
+ skb_shinfo(skb)->frags[j].size - remaining_page_offset);
+
+ skb_frag_ref(skb, j);
+
+ /* Fill the rest of the frags */
+ for (i = 1; i < k; i++) {
+ j = skb_shinfo(skb)->nr_frags - k + i;
+
+ skb_fill_page_desc(nskb, i,
+ skb_shinfo(skb)->frags[j].page.p,
+ skb_shinfo(skb)->frags[j].page_offset,
+ skb_shinfo(skb)->frags[j].size);
+ skb_frag_ref(skb, j);
+ }
+ skb_shinfo(nskb)->nr_frags = k;
+
+ remaining_headlen = remaining - skb->data_len;
+
+ /* headlen contains remaining data? */
+ if (remaining_headlen > 0)
+ skb_copy_bits(skb, skb->len - remaining, nskb->data + headlen,
+ remaining_headlen);
+ nskb->len = remaining + headlen;
+ nskb->data_len = payload_len - sizeof(struct udphdr) +
+ max_t(int, 0, remaining_headlen);
+ nskb->protocol = skb->protocol;
+ if (nskb->protocol == htons(ETH_P_IP)) {
+ ip_hdr(nskb)->id = htons(ntohs(ip_hdr(nskb)->id) +
+ skb_shinfo(skb)->gso_segs);
+ ip_hdr(nskb)->tot_len =
+ htons(payload_len + sizeof(struct iphdr));
+ } else {
+ ipv6_hdr(nskb)->payload_len = htons(payload_len);
+ }
+ udp_hdr(nskb)->len = htons(payload_len);
+ skb_shinfo(nskb)->gso_size = 0;
+ nskb->ip_summed = skb->ip_summed;
+ nskb->csum_start = skb->csum_start;
+ nskb->csum_offset = skb->csum_offset;
+ nskb->queue_mapping = skb->queue_mapping;
+}
+
+/* might send skbs and update wqe and pi */
+struct sk_buff *mlx5e_udp_gso_handle_tx_skb(struct net_device *netdev,
+ struct mlx5e_txqsq *sq,
+ struct sk_buff *skb,
+ struct mlx5e_tx_wqe **wqe,
+ u16 *pi)
+{
+ int payload_len = skb_shinfo(skb)->gso_size + sizeof(struct udphdr);
+ int headlen = skb_transport_offset(skb) + sizeof(struct udphdr);
+ int remaining = (skb->len - headlen) % skb_shinfo(skb)->gso_size;
+ struct sk_buff *nskb;
+
+ if (skb->protocol == htons(ETH_P_IP))
+ ip_hdr(skb)->tot_len = htons(payload_len + sizeof(struct iphdr));
+ else
+ ipv6_hdr(skb)->payload_len = htons(payload_len);
+ udp_hdr(skb)->len = htons(payload_len);
+ if (!remaining)
+ return skb;
+
+ nskb = alloc_skb(max_t(int, headlen, headlen + remaining - skb->data_len), GFP_ATOMIC);
+ if (unlikely(!nskb)) {
+ sq->stats->dropped++;
+ return NULL;
+ }
+
+ mlx5e_udp_gso_prepare_last_skb(skb, nskb, remaining);
+
+ skb_shinfo(skb)->gso_segs--;
+ pskb_trim(skb, skb->len - remaining);
+ mlx5e_sq_xmit(sq, skb, *wqe, *pi);
+ mlx5e_sq_fetch_wqe(sq, wqe, pi);
+ return nskb;
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.h
new file mode 100644
index 000000000000..ed42699a78b3
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/rxtx.h
@@ -0,0 +1,14 @@
+
+#ifndef __MLX5E_EN_ACCEL_RX_TX_H__
+#define __MLX5E_EN_ACCEL_RX_TX_H__
+
+#include <linux/skbuff.h>
+#include "en.h"
+
+struct sk_buff *mlx5e_udp_gso_handle_tx_skb(struct net_device *netdev,
+ struct mlx5e_txqsq *sq,
+ struct sk_buff *skb,
+ struct mlx5e_tx_wqe **wqe,
+ u16 *pi);
+
+#endif
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 134f20a182b5..e2ef68b1daa2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -4592,6 +4592,9 @@ static void mlx5e_build_nic_netdev(struct net_device *netdev)
netdev->features |= NETIF_F_HIGHDMA;
netdev->features |= NETIF_F_HW_VLAN_STAG_FILTER;
+ netdev->features |= NETIF_F_GSO_UDP_L4;
+ netdev->hw_features |= NETIF_F_GSO_UDP_L4;
+
netdev->priv_flags |= IFF_UNICAST_FLT;
mlx5e_set_netdev_dev_addr(netdev);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index f29deb44bf3b..f450d9ca31fb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -228,7 +228,10 @@ mlx5e_tx_get_gso_ihs(struct mlx5e_txqsq *sq, struct sk_buff *skb)
stats->tso_inner_packets++;
stats->tso_inner_bytes += skb->len - ihs;
} else {
- ihs = skb_transport_offset(skb) + tcp_hdrlen(skb);
+ if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
+ ihs = skb_transport_offset(skb) + sizeof(struct udphdr);
+ else
+ ihs = skb_transport_offset(skb) + tcp_hdrlen(skb);
stats->tso_packets++;
stats->tso_bytes += skb->len - ihs;
}
@@ -443,12 +446,11 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev)
sq = priv->txq2sq[skb_get_queue_mapping(skb)];
mlx5e_sq_fetch_wqe(sq, &wqe, &pi);
-#ifdef CONFIG_MLX5_ACCEL
/* might send skbs and update wqe and pi */
skb = mlx5e_accel_handle_tx(skb, sq, dev, &wqe, &pi);
if (unlikely(!skb))
return NETDEV_TX_OK;
-#endif
+
return mlx5e_sq_xmit(sq, skb, wqe, pi);
}
--
2.17.0
^ permalink raw reply related
* [net-next 04/12] net/mlx5e: RX, Use existing WQ local variable
From: Saeed Mahameed @ 2018-06-28 21:50 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Or Gerlitz, Tariq Toukan, Saeed Mahameed
In-Reply-To: <20180628215103.9141-1-saeedm@mellanox.com>
From: Tariq Toukan <tariqt@mellanox.com>
Local variable 'wq' already points to &sq->wq, use it.
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index d3a1dd20e41d..a2d91eaa99c4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -487,7 +487,7 @@ static int mlx5e_alloc_rx_mpwqe(struct mlx5e_rq *rq, u16 ix)
sq->db.ico_wqe[pi].opcode = MLX5_OPCODE_UMR;
sq->pc += MLX5E_UMR_WQEBBS;
- mlx5e_notify_hw(&sq->wq, sq->pc, sq->uar_map, &umr_wqe->ctrl);
+ mlx5e_notify_hw(wq, sq->pc, sq->uar_map, &umr_wqe->ctrl);
return 0;
--
2.17.0
^ permalink raw reply related
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