* Re: [PATCH v2 04/21] octeontx2-af: Relax resource lock into mutex
From: David Miller @ 2018-11-17 19:54 UTC (permalink / raw)
To: sunil.kovvuri; +Cc: netdev, arnd, linux-soc, skardach, sgoutham
In-Reply-To: <CA+sq2Cfoi50d-yo7YXWZ2CX7EEcYfBoY+DpXek9KLQnjE3z5YQ@mail.gmail.com>
From: Sunil Kovvuri <sunil.kovvuri@gmail.com>
Date: Sat, 17 Nov 2018 21:42:41 +0530
> On Sat, Nov 17, 2018 at 12:50 PM David Miller <davem@davemloft.net> wrote:
>>
>> From: sunil.kovvuri@gmail.com
>> Date: Thu, 15 Nov 2018 16:29:29 +0530
>>
>> > From: Stanislaw Kardach <skardach@marvell.com>
>> >
>> > The resource locks does not need to be a spinlock as they are not
>> > used in any interrupt handling routines (only in bottom halves).
>> > Therefore relax them into a mutex so that later on we may use them
>> > in routines that might sleep.
>> >
>> > Signed-off-by: Stanislaw Kardach <skardach@marvell.com>
>> > Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
>>
>> This is confusing because software interrupts are often called bottom
>> halves, and in which sleeping and thus mutexes are not allowed.
>
> Mailbox message interrupt handler schedules workqueue and exits.
> Message handling happens in workqueue context, hence moving from
> spinlock to mutex.
> Hope this clarifies.
Fix the commit message like this then.
^ permalink raw reply
* Re: [PATCH mlx5-next 11/12] {net,IB}/mlx5: Move Page fault EQ and ODP logic to RDMA
From: Jason Gunthorpe @ 2018-11-17 20:13 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Leon Romanovsky, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org
In-Reply-To: <20181116215901.5874-12-saeedm@mellanox.com>
On Fri, Nov 16, 2018 at 01:59:00PM -0800, Saeed Mahameed wrote:
> Use the new generic EQ API to move all ODP RDMA data structures and logic
> form mlx5 core driver into mlx5_ib driver.
>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
> Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
> drivers/infiniband/hw/mlx5/main.c | 10 +-
> drivers/infiniband/hw/mlx5/mlx5_ib.h | 15 +-
> drivers/infiniband/hw/mlx5/odp.c | 281 +++++++++++++++++-
> drivers/net/ethernet/mellanox/mlx5/core/dev.c | 34 ---
> drivers/net/ethernet/mellanox/mlx5/core/eq.c | 252 ----------------
> .../net/ethernet/mellanox/mlx5/core/lib/eq.h | 8 -
> .../net/ethernet/mellanox/mlx5/core/main.c | 17 +-
> .../ethernet/mellanox/mlx5/core/mlx5_core.h | 2 -
> include/linux/mlx5/driver.h | 49 ---
> include/linux/mlx5/eq.h | 21 ++
> 10 files changed, 308 insertions(+), 381 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index 6fbc0cba1bac..fcf4a0328a90 100644
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -6040,6 +6040,11 @@ static int mlx5_ib_stage_odp_init(struct mlx5_ib_dev *dev)
> return mlx5_ib_odp_init_one(dev);
> }
>
> +void mlx5_ib_stage_odp_cleanup(struct mlx5_ib_dev *dev)
> +{
> + mlx5_ib_odp_cleanup_one(dev);
> +}
> +
> int mlx5_ib_stage_counters_init(struct mlx5_ib_dev *dev)
> {
> if (MLX5_CAP_GEN(dev->mdev, max_qp_cnt)) {
> @@ -6225,7 +6230,7 @@ static const struct mlx5_ib_profile pf_profile = {
> mlx5_ib_stage_dev_res_cleanup),
> STAGE_CREATE(MLX5_IB_STAGE_ODP,
> mlx5_ib_stage_odp_init,
> - NULL),
> + mlx5_ib_stage_odp_cleanup),
> STAGE_CREATE(MLX5_IB_STAGE_COUNTERS,
> mlx5_ib_stage_counters_init,
> mlx5_ib_stage_counters_cleanup),
> @@ -6395,9 +6400,6 @@ static struct mlx5_interface mlx5_ib_interface = {
> .add = mlx5_ib_add,
> .remove = mlx5_ib_remove,
> .event = mlx5_ib_event,
> -#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> - .pfault = mlx5_ib_pfault,
> -#endif
> .protocol = MLX5_INTERFACE_PROTOCOL_IB,
> };
>
> diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> index b651a7a6fde9..d01af2d829b8 100644
> +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
> @@ -880,6 +880,15 @@ struct mlx5_ib_lb_state {
> bool enabled;
> };
>
> +struct mlx5_ib_pf_eq {
> + struct mlx5_ib_dev *dev;
> + struct mlx5_eq *core;
> + struct work_struct work;
> + spinlock_t lock; /* Pagefaults spinlock */
> + struct workqueue_struct *wq;
> + mempool_t *pool;
> +};
I know this is being copied, but can we please not do this vertical
alignment madness in RDMA? It is such a nightmare to maintain this..
> +/* mempool_refill() was proposed but unfortunately wasn't accepted
> + * http://lkml.iu.edu/hypermail/linux/kernel/1512.1/05073.html
> + * Chip workaround.
'cheap workaround'
Jason
^ permalink raw reply
* Re: [PATCH 10/11] nvmet-tcp: add NVMe over TCP target driver
From: David Miller @ 2018-11-17 20:15 UTC (permalink / raw)
To: sagi; +Cc: linux-nvme, linux-block, netdev, hch, keith.busch
In-Reply-To: <20181115171626.9306-11-sagi@lightbitslabs.com>
From: Sagi Grimberg <sagi@lightbitslabs.com>
Date: Thu, 15 Nov 2018 09:16:22 -0800
> +static unsigned nvmet_tcp_recv_budget = 8;
> +module_param_named(recv_budget, nvmet_tcp_recv_budget, int, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(recv_budget, "recvs budget");
> +
> +static unsigned nvmet_tcp_send_budget = 8;
> +module_param_named(send_budget, nvmet_tcp_send_budget, int, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(send_budget, "sends budget");
> +
> +static unsigned nvmet_tcp_io_work_budget = 64;
> +module_param_named(io_work_budget, nvmet_tcp_io_work_budget, int, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(io_work_budget, "io work budget");
I strongly suggest moving away from module parameters for this stuff.
Create a genetlink socket family and allow run time configuration of these knobs
by the user.
Thanks.
^ permalink raw reply
* Re: [PATCH] net/ibmnvic: Fix deadlock problem in reset
From: David Miller @ 2018-11-17 20:41 UTC (permalink / raw)
To: julietk
Cc: linuxppc-dev, netdev, linux-kernel, benh, paulus, mpe, tlfalcon,
mwb, tyreld
In-Reply-To: <25faa351-51d3-3aeb-a1c9-e8d3259604dd@linux.vnet.ibm.com>
Your patch is completely corrupted by your email client.
Please fix this.
Then, email a test patch privately to yourself.
Please do not post your patch to this mailing list again until you can
successfully apply the test patch you email to yourself in the step
above.
Thank you.
^ permalink raw reply
* Re: Linux kernel hangs if using RV1108 with MSZ8863 switch with two ports connected
From: Otavio Salvador @ 2018-11-17 21:04 UTC (permalink / raw)
To: andrew; +Cc: netdev, Heiko Stuebner, david.choi, Andy Yan
In-Reply-To: <20181116234044.GA752@lunn.ch>
Hi Andrew,
On Fri, Nov 16, 2018 at 9:40 PM Andrew Lunn <andrew@lunn.ch> wrote:
> On Fri, Nov 16, 2018 at 04:28:29PM -0200, Otavio Salvador wrote:
> > RV1108 GMAC is connected to KSZ8863 port 3 and after kernel boots, I
> > can put an Ethernet cable from my router to the uplink port of
> > KSZ8863, which makes the RV1108 board and a Linux PC connected to the
> > other KSZ8863 port to both get IP addresses.
> >
> > So in this usecase the setup is working fine.
> >
> > However, if the RV1108 board boots with both Ethernet cables to the
> > KSZ8863 switch connected, then the kernel silently hangs.
>
> By silently, you mean it prints nothing at all?
Yes, it prints but suddenly hangs.
> I would try building the kernel with all the lock debugging turned
> on. That might find something even with your working case, if there is
> a potential deadlock.
>
> If the kernel dies very early, you might need to enable "kernel
> low-level debugping print and EARLY_PRINTK, in order to see anything.
The kernel starts booting normally and then hangs like this when two
Ethernet cables are connected to the KSZ8863 switch:
http://dark-code.bulix.org/3xexu5-507563
This has the lock detection, inside kernel hacking, enabled.
Any hint?
--
Otavio Salvador O.S. Systems
http://www.ossystems.com.br http://code.ossystems.com.br
Mobile: +55 (53) 9 9981-7854 Mobile: +1 (347) 903-9750
^ permalink raw reply
* Re: [PATCH 2/4] net/bpf: refactor freeing of executable allocations
From: Y Song @ 2018-11-18 7:47 UTC (permalink / raw)
To: ard.biesheuvel
Cc: LKML, Daniel Borkmann, Alexei Starovoitov, rick.p.edgecombe,
eric.dumazet, jannh, Kees Cook, jeyu, arnd, catalin.marinas,
will.deacon, mark.rutland, ralf, paul.burton, jhogan, benh,
paulus, mpe, David Miller, linux-arm-kernel, linux-mips,
linuxppc-dev, sparclinux, netdev
In-Reply-To: <20181117185715.25198-3-ard.biesheuvel@linaro.org>
On Sat, Nov 17, 2018 at 6:58 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> All arch overrides of the __weak bpf_jit_free() amount to the same
> thing: the allocated memory was never mapped read-only, and so
> it does not have to be remapped to read-write before being freed.
>
> So in preparation of permitting arches to serve allocations for BPF
> JIT programs from other regions than the module region, refactor
> the existing bpf_jit_free() implementations to use the shared code
> where possible, and only specialize the remap and free operations.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/mips/net/bpf_jit.c | 7 ++-----
> arch/powerpc/net/bpf_jit_comp.c | 7 ++-----
> arch/powerpc/net/bpf_jit_comp64.c | 9 +++------
> arch/sparc/net/bpf_jit_comp_32.c | 7 ++-----
> kernel/bpf/core.c | 15 +++++----------
> 5 files changed, 14 insertions(+), 31 deletions(-)
>
> diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c
> index 1b69897274a1..5696bd7dccc7 100644
> --- a/arch/mips/net/bpf_jit.c
> +++ b/arch/mips/net/bpf_jit.c
> @@ -1261,10 +1261,7 @@ void bpf_jit_compile(struct bpf_prog *fp)
> kfree(ctx.offsets);
> }
>
> -void bpf_jit_free(struct bpf_prog *fp)
> +void bpf_jit_binary_free(struct bpf_binary_header *hdr)
> {
> - if (fp->jited)
> - bpf_jit_binary_free(bpf_jit_binary_hdr(fp));
> -
> - bpf_prog_unlock_free(fp);
> + module_memfree(hdr);
> }
> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> index a1ea1ea6b40d..5b5ce4a1b44b 100644
> --- a/arch/powerpc/net/bpf_jit_comp.c
> +++ b/arch/powerpc/net/bpf_jit_comp.c
> @@ -680,10 +680,7 @@ void bpf_jit_compile(struct bpf_prog *fp)
> return;
> }
>
> -void bpf_jit_free(struct bpf_prog *fp)
> +void bpf_jit_binary_free(struct bpf_binary_header *hdr)
> {
> - if (fp->jited)
> - bpf_jit_binary_free(bpf_jit_binary_hdr(fp));
> -
> - bpf_prog_unlock_free(fp);
> + module_memfree(hdr);
> }
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index 84c8f013a6c6..f64f1294bd62 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -1021,11 +1021,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> return fp;
> }
>
> -/* Overriding bpf_jit_free() as we don't set images read-only. */
> -void bpf_jit_free(struct bpf_prog *fp)
> +/* Overriding bpf_jit_binary_free() as we don't set images read-only. */
> +void bpf_jit_binary_free(struct bpf_binary_header *hdr)
> {
> - if (fp->jited)
> - bpf_jit_binary_free(bpf_jit_binary_hdr(fp));
> -
> - bpf_prog_unlock_free(fp);
> + module_memfree(hdr);
> }
> diff --git a/arch/sparc/net/bpf_jit_comp_32.c b/arch/sparc/net/bpf_jit_comp_32.c
> index 01bda6bc9e7f..589950d152cc 100644
> --- a/arch/sparc/net/bpf_jit_comp_32.c
> +++ b/arch/sparc/net/bpf_jit_comp_32.c
> @@ -756,10 +756,7 @@ cond_branch: f_offset = addrs[i + filter[i].jf];
> return;
> }
>
> -void bpf_jit_free(struct bpf_prog *fp)
> +void bpf_jit_binary_free(struct bpf_binary_header *hdr)
> {
> - if (fp->jited)
> - bpf_jit_binary_free(bpf_jit_binary_hdr(fp));
> -
> - bpf_prog_unlock_free(fp);
> + module_memfree(hdr);
> }
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 1a796e0799ec..29f766dac203 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -646,25 +646,20 @@ bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
> return hdr;
> }
>
> -void bpf_jit_binary_free(struct bpf_binary_header *hdr)
> +void __weak bpf_jit_binary_free(struct bpf_binary_header *hdr)
> {
> - u32 pages = hdr->pages;
> -
> + bpf_jit_binary_unlock_ro(hdr);
> module_memfree(hdr);
> - bpf_jit_uncharge_modmem(pages);
> }
>
> -/* This symbol is only overridden by archs that have different
> - * requirements than the usual eBPF JITs, f.e. when they only
> - * implement cBPF JIT, do not set images read-only, etc.
> - */
Do you want to move the above comments to
new weak function bpf_jit_binary_free?
> -void __weak bpf_jit_free(struct bpf_prog *fp)
> +void bpf_jit_free(struct bpf_prog *fp)
> {
> if (fp->jited) {
> struct bpf_binary_header *hdr = bpf_jit_binary_hdr(fp);
> + u32 pages = hdr->pages;
>
> - bpf_jit_binary_unlock_ro(hdr);
> bpf_jit_binary_free(hdr);
> + bpf_jit_uncharge_modmem(pages);
>
> WARN_ON_ONCE(!bpf_prog_kallsyms_verify_off(fp));
> }
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH 0/4] bpf: permit JIT allocations to be served outside the module region
From: Y Song @ 2018-11-18 7:48 UTC (permalink / raw)
To: ard.biesheuvel
Cc: LKML, Daniel Borkmann, Alexei Starovoitov, rick.p.edgecombe,
eric.dumazet, jannh, Kees Cook, jeyu, arnd, catalin.marinas,
will.deacon, mark.rutland, ralf, paul.burton, jhogan, benh,
paulus, mpe, David Miller, linux-arm-kernel, linux-mips,
linuxppc-dev, sparclinux, netdev
In-Reply-To: <20181117185715.25198-1-ard.biesheuvel@linaro.org>
On Sat, Nov 17, 2018 at 6:58 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On arm64, modules are allocated from a 128 MB window which is close to
> the core kernel, so that relative direct branches are guaranteed to be
> in range (except in some KASLR configurations). Also, module_alloc()
> is in charge of allocating KASAN shadow memory when running with KASAN
> enabled.
>
> This means that the way BPF reuses module_alloc()/module_memfree() is
> undesirable on arm64 (and potentially other architectures as well),
> and so this series refactors BPF's use of those functions to permit
> architectures to change this behavior.
>
> Patch #1 fixes a bug introduced during the merge window, where the new
> alloc/free tracking does not account for memory that is freed by some
> arch code.
>
> Patch #2 refactors the freeing path so that architectures can switch to
> something other than module_memfree().
>
> Patch #3 does the same for module_alloc().
>
> Patch #4 implements the new alloc/free overrides for arm64
Except a minor comment, the whole patch set looks good to me.
Acked-by: Yonghong Song <yhs@fb.com>
>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Jann Horn <jannh@google.com>
> Cc: Kees Cook <keescook@chromium.org>
>
> Cc: Jessica Yu <jeyu@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Paul Burton <paul.burton@mips.com>
> Cc: James Hogan <jhogan@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mips@linux-mips.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: sparclinux@vger.kernel.org
> Cc: netdev@vger.kernel.org
>
> Ard Biesheuvel (4):
> bpf: account for freed JIT allocations in arch code
> net/bpf: refactor freeing of executable allocations
> bpf: add __weak hook for allocating executable memory
> arm64/bpf: don't allocate BPF JIT programs in module memory
>
> arch/arm64/net/bpf_jit_comp.c | 11 ++++++++++
> arch/mips/net/bpf_jit.c | 7 ++-----
> arch/powerpc/net/bpf_jit_comp.c | 7 ++-----
> arch/powerpc/net/bpf_jit_comp64.c | 12 +++--------
> arch/sparc/net/bpf_jit_comp_32.c | 7 ++-----
> kernel/bpf/core.c | 22 ++++++++++----------
> 6 files changed, 31 insertions(+), 35 deletions(-)
>
> --
> 2.17.1
>
^ permalink raw reply
* Re: [RFC] DTrace based on eBPF and other tracing facilities
From: Aleksa Sarai @ 2018-11-18 8:32 UTC (permalink / raw)
To: Kris Van Hees; +Cc: Brendan Gregg, linux-kernel, netdev
In-Reply-To: <201811160602.wAG62uAu005007@aserv0121.oracle.com>
[-- Attachment #1: Type: text/plain, Size: 3422 bytes --]
On 2018-11-15, Kris Van Hees <kris.van.hees@oracle.com> wrote:
> A lot of work has been done on various aspects of the tracing infrastructure
> in Linux in the past years and with the further development of BPF a quite
> powerful execution engine has become available as well.
>
> One of the difficulties we have experienced in furthering DTrace on Linux is
> that we have to duplicate functionality already available in the kernel
> because that functionality is not easy to make use of.
>
> In the past year or so we have been working towards changing that. There is
> no point in having multiple projects reinvent the same wheel a couple of times
> over, especially when there are ways where everyone can benefit from actually
> cooperating. Our current (lofty) goal is to rework the DTrace implementation
> that we currently have to make it more modular and less self-sufficient. We
> are envisioning a future for DTrace where we can leverage its strengths in the
> areas where it matters most (e.g. very efficient handling of large amounts of
> kernel probes, well defined and understood D language, user familiarity with
> existing providers, ...) while building on the existing tracing infrastructure
> in Linux. That also means that we can contribute better to existing pieces
> in the infrastructure and work together with other tracing projects to continue
> to improve tracing on Linux.
>
> Ideally we would like to see an infrastructure where any tracers can attach
> actions to any kind of probe source, and have data generated according to the
> actions the tracer associated with the probe source when a specific probe
> fires. The execution of those actions would be done using BPF.
>
> We believe that this proposal would be a benefit to all because it allows us
> to pool resources in the areas that really need it. E.g. if we all depend on
> BPF as execution engine we invariably work together to make it as solid as can
> be.
>
> Obviously we cannot do this work on our own, and we cannot do it behind closed
> doors. We've created a github repository for the kernel with DTrace added in
> at:
>
> https://github.com/ezannoni/dtrace-linux-kernel/tree/master
>
> We also have a branch there with the most recent BPF-based work:
>
> https://github.com/ezannoni/dtrace-linux-kernel/tree/nix/bpf/4.19/helpers
>
> Since most (if not all) tracing tools have similar requirements for what may
> need to be done when a probe fires, we really want to join forces.
Have you taken a look at bpftrace[1]? It uses LLVM to compile to BPF
bytecode and has a DTrace-like syntax. Yes, it's a from-scratch
reimplementation, but as a user of tracing I don't really mind either
way (I think most people like DTrace because of how seamlessly all
aspects of probing work -- but I believe that most of this is a result
of the DTrace kernel infrastructure more than the user-space tooling).
I'm not entirely sure how USDT interacts with the current Linux tracing
tools, but it would be nice if USDT worked on Linux the same way it
worked on Solaris (though I will admit I haven't played with it much on
Linux, I'm sure the other tracing folks will be able to comment on this
far more than myself).
[1]: https://github.com/iovisor/bpftrace
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 04/11] nvme-core: add work elements to struct nvme_ctrl
From: Max Gurtovoy @ 2018-11-17 22:18 UTC (permalink / raw)
To: Sagi Grimberg, linux-nvme
Cc: linux-block, netdev, Keith Busch, Christoph Hellwig
In-Reply-To: <20181115171626.9306-5-sagi@lightbitslabs.com>
On 11/15/2018 7:16 PM, Sagi Grimberg wrote:
> connect_work and err_work will be reused by nvme-tcp so
> share those in nvme_ctrl for rdma and fc to share.
>
> Signed-off-by: Sagi Grimberg <sagi@lightbitslabs.com>
looks good (and I see that more sharing should be made in the future as
well :) )
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
^ permalink raw reply
* Re: [PATCH 05/11] nvmet: Add install_queue callout
From: Max Gurtovoy @ 2018-11-17 22:36 UTC (permalink / raw)
To: Sagi Grimberg, linux-nvme
Cc: linux-block, netdev, Keith Busch, Christoph Hellwig
In-Reply-To: <20181115171626.9306-6-sagi@lightbitslabs.com>
On 11/15/2018 7:16 PM, Sagi Grimberg wrote:
> nvmet-tcp will implement it to allocate queue commands which
> are only known at nvmf connect time (sq size).
>
> Signed-off-by: Sagi Grimberg <sagi@lightbitslabs.com>
> ---
> drivers/nvme/target/fabrics-cmd.c | 9 +++++++++
> drivers/nvme/target/nvmet.h | 1 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
> index 1f05d8507e35..a84668e8939c 100644
> --- a/drivers/nvme/target/fabrics-cmd.c
> +++ b/drivers/nvme/target/fabrics-cmd.c
> @@ -117,6 +117,15 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req)
> nvmet_sq_setup(ctrl, req->sq, qid, sqsize + 1,
> !!(c->cattr & NVME_CONNECT_DISABLE_SQFLOW));
what is your code base ? does SQFLOW must for TCP ?
>
> + if (ctrl->ops->install_queue) {
> + int ret = ctrl->ops->install_queue(req->sq);
> + if (ret) {
> + pr_err("failed to install queue %d cntlid %d ret %d\n",
> + qid, ret, ctrl->cntlid);
> + return NVME_SC_CONNECT_INVALID_PARAM | NVME_SC_DNR;
I'm not sure regarding the INVALID_PARAM rc.
maybe use NVME_SC_INTERNAL ?
otherwise,
look fine,
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
^ permalink raw reply
* Re: [PATCH][net-next] net: slightly optimize eth_type_trans
From: Eric Dumazet @ 2018-11-17 22:37 UTC (permalink / raw)
To: David Miller, lirongqing; +Cc: netdev
In-Reply-To: <20181115.151131.596213640354724307.davem@davemloft.net>
On 11/15/2018 03:11 PM, David Miller wrote:
>
> Applied.
>
While reviewing this stuff, I found we have a bug.
If napi_reuse_skb() is called, we might inherit from prior skb->pkt_type value.
It seems that GRO could aggregate packets with pkt_type != PACKET_HOST, right ?
David, any objection if I submit the following fix ?
diff --git a/net/core/dev.c b/net/core/dev.c
index 5927f6a7c301ed90af21a4b82b443f30f00bb483..f2bfd2eda7b2734d29d30f0e82c1a48c1b5b166a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5664,6 +5664,10 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
__vlan_hwaccel_clear_tag(skb);
skb->dev = napi->dev;
skb->skb_iif = 0;
+
+ /* eth_type_trans() assumes initial pkt_type is PACKET_HOST */
+ skb->pkt_type = PACKET_HOST;
+
skb->encapsulation = 0;
skb_shinfo(skb)->gso_type = 0;
skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
^ permalink raw reply related
* Re: [PATCH 06/11] nvmet: allow configfs tcp trtype configuration
From: Max Gurtovoy @ 2018-11-17 22:38 UTC (permalink / raw)
To: Sagi Grimberg, linux-nvme
Cc: linux-block, netdev, Christoph Hellwig, Keith Busch
In-Reply-To: <20181115171626.9306-7-sagi@lightbitslabs.com>
On 11/15/2018 7:16 PM, Sagi Grimberg wrote:
> Signed-off-by: Sagi Grimberg <sagi@lightbitslabs.com>
> ---
> drivers/nvme/target/configfs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index c2df205096ec..4d28c4d78850 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -34,6 +34,7 @@ static const struct nvmet_transport_name {
> } nvmet_transport_names[] = {
> { NVMF_TRTYPE_RDMA, "rdma" },
> { NVMF_TRTYPE_FC, "fc" },
> + { NVMF_TRTYPE_TCP, "tcp" },
> { NVMF_TRTYPE_LOOP, "loop" },
> };
>
looks good,
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
^ permalink raw reply
* Re: [PATCH 10/11] nvmet-tcp: add NVMe over TCP target driver
From: Max Gurtovoy @ 2018-11-17 22:48 UTC (permalink / raw)
To: David Miller, sagi; +Cc: linux-nvme, linux-block, netdev, hch, keith.busch
In-Reply-To: <20181117.121510.1754018619653365187.davem@davemloft.net>
On 11/17/2018 10:15 PM, David Miller wrote:
> From: Sagi Grimberg <sagi@lightbitslabs.com>
> Date: Thu, 15 Nov 2018 09:16:22 -0800
>
>> +static unsigned nvmet_tcp_recv_budget = 8;
>> +module_param_named(recv_budget, nvmet_tcp_recv_budget, int, S_IRUGO | S_IWUSR);
>> +MODULE_PARM_DESC(recv_budget, "recvs budget");
>> +
>> +static unsigned nvmet_tcp_send_budget = 8;
>> +module_param_named(send_budget, nvmet_tcp_send_budget, int, S_IRUGO | S_IWUSR);
>> +MODULE_PARM_DESC(send_budget, "sends budget");
>> +
>> +static unsigned nvmet_tcp_io_work_budget = 64;
>> +module_param_named(io_work_budget, nvmet_tcp_io_work_budget, int, S_IRUGO | S_IWUSR);
>> +MODULE_PARM_DESC(io_work_budget, "io work budget");
> I strongly suggest moving away from module parameters for this stuff.
agree here.
also, Sagi, can you explain about the performance trade-offs seen during
your development for these values ?
are they HCA/NIC dependent ?
should send/recv ratio be 1:1 ?
should total/send/recv ratio be 8:1:1 ?
>
> Create a genetlink socket family and allow run time configuration of these knobs
> by the user.
>
> Thanks.
^ permalink raw reply
* [net-next:master 114/114] drivers/net/tun.c:2503:42: warning: Using plain integer as NULL pointer
From: kbuild test robot @ 2018-11-17 23:13 UTC (permalink / raw)
To: Jason Wang; +Cc: kbuild-all, netdev
[-- Attachment #1: Type: text/plain, Size: 3062 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head: f9e06c45cb28beb30a6a474952ead7da2b8940f3
commit: f9e06c45cb28beb30a6a474952ead7da2b8940f3 [114/114] tuntap: free XDP dropped packets in a batch
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout f9e06c45cb28beb30a6a474952ead7da2b8940f3
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
include/linux/slab.h:332:43: warning: dubious: x & !y
include/linux/slab.h:332:43: warning: dubious: x & !y
>> drivers/net/tun.c:2503:42: warning: Using plain integer as NULL pointer
drivers/net/tun.c:2989:36: warning: incorrect type in argument 2 (different address spaces)
drivers/net/tun.c:2989:36: expected struct tun_prog [noderef] <asn:4>**prog_p
drivers/net/tun.c:2989:36: got struct tun_prog **prog_p
drivers/net/tun.c:3281:42: warning: incorrect type in argument 2 (different address spaces)
drivers/net/tun.c:3281:42: expected struct tun_prog **prog_p
drivers/net/tun.c:3281:42: got struct tun_prog [noderef] <asn:4>**<noident>
drivers/net/tun.c:3285:42: warning: incorrect type in argument 2 (different address spaces)
drivers/net/tun.c:3285:42: expected struct tun_prog **prog_p
drivers/net/tun.c:3285:42: got struct tun_prog [noderef] <asn:4>**<noident>
drivers/net/tun.c:3378:9: error: incompatible types in comparison expression (different address spaces)
include/linux/slab.h:332:43: warning: dubious: x & !y
include/linux/slab.h:332:43: warning: dubious: x & !y
vim +2503 drivers/net/tun.c
2490
2491 static int tun_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
2492 {
2493 int ret, i;
2494 struct tun_file *tfile = container_of(sock, struct tun_file, socket);
2495 struct tun_struct *tun = tun_get(tfile);
2496 struct tun_msg_ctl *ctl = m->msg_control;
2497 struct xdp_buff *xdp;
2498
2499 if (!tun)
2500 return -EBADFD;
2501
2502 if (ctl && (ctl->type == TUN_MSG_PTR)) {
> 2503 struct tun_page tpage = {0};
2504 int n = ctl->num;
2505 int flush = 0;
2506
2507 local_bh_disable();
2508 rcu_read_lock();
2509
2510 for (i = 0; i < n; i++) {
2511 xdp = &((struct xdp_buff *)ctl->ptr)[i];
2512 tun_xdp_one(tun, tfile, xdp, &flush, &tpage);
2513 }
2514
2515 if (flush)
2516 xdp_do_flush_map();
2517
2518 rcu_read_unlock();
2519 local_bh_enable();
2520
2521 tun_put_page(&tpage);
2522
2523 ret = total_len;
2524 goto out;
2525 }
2526
2527 ret = tun_get_user(tun, tfile, ctl ? ctl->ptr : NULL, &m->msg_iter,
2528 m->msg_flags & MSG_DONTWAIT,
2529 m->msg_flags & MSG_MORE);
2530 out:
2531 tun_put(tun);
2532 return ret;
2533 }
2534
---
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: 66595 bytes --]
^ permalink raw reply
* hw csum failure + conntrack with more debugging information
From: Andre Tomt @ 2018-11-17 23:18 UTC (permalink / raw)
To: netdev, Cong Wang; +Cc: Eric Dumazet
I added Cong Wang's hw csum failure debug patch to my 4.19.2 tree and
got a splat with a bit more information.
> [47273.905616] p0xe0: hw csum failure
> [47273.905642] dev features: 0x000860c000114bb3
> [47273.905663] skb len=44 data_len=0 gso_size=0 gso_type=0 ip_summed=2 csum=0, csum_complete_sw=0, csum_valid=0
> [47273.905706] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.19.0-1 #1
> [47273.905707] Hardware name: Supermicro Super Server/X10SDV-4C-TLN2F, BIOS 2.0 06/13/2018
> [47273.905707] Call Trace:
> [47273.905710] <IRQ>
> [47273.905717] dump_stack+0x5c/0x80
> [47273.905721] __skb_checksum_complete+0xaf/0xc0
> [47273.905731] icmp_error+0x1c8/0x1f0 [nf_conntrack]
> [47273.905734] ? skb_copy_bits+0x13d/0x220
> [47273.905740] nf_conntrack_in+0xd8/0x390 [nf_conntrack]
> [47273.905743] ? ___pskb_trim+0x192/0x330
> [47273.905746] nf_hook_slow+0x43/0xc0
> [47273.905749] ip_rcv+0x90/0xb0
> [47273.905752] ? ip_rcv_finish_core.isra.0+0x310/0x310
> [47273.905754] __netif_receive_skb_one_core+0x42/0x50
> [47273.905756] netif_receive_skb_internal+0x24/0xb0
> [47273.905758] napi_gro_frags+0x177/0x210
> [47273.905762] mlx4_en_process_rx_cq+0x8df/0xb50 [mlx4_en]
> [47273.905773] ? mlx4_eq_int+0x38f/0xcb0 [mlx4_core]
> [47273.905776] mlx4_en_poll_rx_cq+0x55/0xf0 [mlx4_en]
> [47273.905778] net_rx_action+0xe1/0x2c0
> [47273.905781] __do_softirq+0xe7/0x2d3
> [47273.905784] irq_exit+0x96/0xd0
> [47273.905786] do_IRQ+0x85/0xd0
> [47273.905790] common_interrupt+0xf/0xf
> [47273.905791] </IRQ>
> [47273.905794] RIP: 0010:cpuidle_enter_state+0xb9/0x320
> [47273.905796] Code: e8 3c 15 bc ff 80 7c 24 0b 00 74 17 9c 58 0f 1f 44 00 00 f6 c4 02 0f 85 3b 02 00 00 31 ff e8 6e fa c0 ff fb 66 0f 1f 44 00 00 <48> b8 ff ff ff ff f3 01 00 00 48 2b 1c 24 ba ff ff ff 7f 48 39 c3
> [47273.905798] RSP: 0018:ffffb75601943ea8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffdb
> [47273.905801] RAX: ffff9d636fa60fc0 RBX: 00002afed059e821 RCX: 000000000000001f
> [47273.905802] RDX: 00002afed059e821 RSI: 000000003a2ea91a RDI: 0000000000000000
> [47273.905803] RBP: ffff9d636fa698c8 R08: 0000000000000002 R09: 0000000000020840
> [47273.905804] R10: 000e97ef158d1e39 R11: ffff9d636fa601e8 R12: 0000000000000001
> [47273.905805] R13: ffffffffab0ac698 R14: 0000000000000001 R15: 0000000000000000
> [47273.905808] ? cpuidle_enter_state+0x94/0x320
> [47273.905812] do_idle+0x1e4/0x220
> [47273.905815] cpu_startup_entry+0x5f/0x70
> [47273.905818] start_secondary+0x185/0x1a0
> [47273.905821] secondary_startup_64+0xa4/0xb0
All instances stripped of the identical stack traces:
> [13778.531040] dev features: 0x000860c000114bb3
> [13778.531056] skb len=40 data_len=0 gso_size=0 gso_type=0 ip_summed=2 csum=0, csum_complete_sw=0, csum_valid=0
> [13778.531176] dev features: 0x000860c000114bb3
> [13778.531204] skb len=40 data_len=0 gso_size=0 gso_type=0 ip_summed=2 csum=0, csum_complete_sw=0, csum_valid=0
> [13778.531256] dev features: 0x000860c000114bb3
> [13778.531285] skb len=40 data_len=0 gso_size=0 gso_type=0 ip_summed=2 csum=0, csum_complete_sw=0, csum_valid=0 > [47273.905642] dev features: 0x000860c000114bb3
> [47273.905663] skb len=44 data_len=0 gso_size=0 gso_type=0 ip_summed=2 csum=0, csum_complete_sw=0, csum_valid=0
The setup has also further been simplified by also removing vlans and
6to4 tunnels, It's now only conntrack and nat (configured with nftables)
on bare ethernet netdevs.
offloads, ring sizes etc is left at defaults, net.ipv4.ip_early_demux is
off, fq_codel as net.core.default_qdisc
Hardware is ConnectX-3 VPI 2xQSFP+ (firmware 2.42.5000) on a quad core
Xeon D-1521, passing traffic from port 1 to port 2 on the same card.
Last switch to touch the packets is an Arista DCS-7050QX-32 running EOS
4.20.2.1F
This kernel build contains some other bits and pieces from net.git
(mostly things queued for stable) and a couple of backports from
net-next (Aaron Lu's pcp page recycling fix, Eric's BQL+mlx4
optimizations), but the stack traces are identical to before so they
dont seem involved in this.
Workload remains nearly exclusively TCP and UDP torrent junk traffic to
two machines behind it.
^ permalink raw reply
* Re: [PATCH iproute2 00/22] misc cleanups
From: David Ahern @ 2018-11-17 23:19 UTC (permalink / raw)
To: Stephen Hemminger, netdev
In-Reply-To: <20181115223640.411-1-stephen@networkplumber.org>
On 11/15/18 3:36 PM, Stephen Hemminger wrote:
> Code cleanup including:
> * make local functions static
> * drop dead code
> * whitespace code style cleanup
>
Hi Stephen:
You did not mark these for -next but that is typically where cleanups
go. What is your intention?
^ permalink raw reply
* Re: selftests: net: udpgro.sh hangs on DUT devices running Linux -next
From: Paolo Abeni @ 2018-11-17 23:31 UTC (permalink / raw)
To: Naresh Kamboju, netdev
Cc: David S. Miller, Willem de Bruijn, Anders Roxell, Rafael Tinoco
In-Reply-To: <CA+G9fYsFcOnL2hU=4J+avbGug7Qty6D7GKi-6rVii4aNh0X0Cw@mail.gmail.com>
Hi,
On Fri, 2018-11-16 at 14:55 +0530, Naresh Kamboju wrote:
> Kernel selftests: net: udpgro.sh hangs / waits forever on x86_64 and
> arm32 devices running Linux -next. Test getting PASS on arm64 devices.
>
> Do you see this problem ?
>
> Short error log:
> ---------------------
> ip6tables v1.6.1: can't initialize ip6tables table `nat': Table does
> not exist (do you need to insmod?)
Thank you for the report.
It looks like your kernel config has
# CONFIG_NF_NAT_IPV6 is not set
Can you please confirm ?
net selftests do not explicitly ask for that, despiting using such
functionality (my bad).
I'll be travelling up to Monday (included). I'll have a better look
after that.
Cheers,
Paolo
^ permalink raw reply
* Re: [Patch net] net: invert the check of detecting hardware RX checksum fault
From: David Miller @ 2018-11-18 0:40 UTC (permalink / raw)
To: herbert; +Cc: xiyou.wangcong, netdev, tom, edumazet
In-Reply-To: <20181116015242.wmm6mooubjnum7qv@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 16 Nov 2018 09:52:42 +0800
> netdev_rx_csum_fault is meant to warn about the situation where
> a packet with a valid checksum (i.e., sum == 0) was given to us
> by the hardware with a partial checksum that was invalid.
Thanks for reminding us how this code is supposed to work :-)
Definitely we need to better document this and it looks like Cong is
going to add a comment in order to do so.
As for dumping out a packet, we need to rate limit that more strongly
than the existing net_ratelimit() there. But only dumping once per
bootup seems to extreme in the other direction. If we get like 2
or 3 of these in an hour, dumping all of them would be reasonable
and useful.
^ permalink raw reply
* Re: [PATCH][net-next] net: slightly optimize eth_type_trans
From: David Miller @ 2018-11-18 0:51 UTC (permalink / raw)
To: eric.dumazet; +Cc: lirongqing, netdev
In-Reply-To: <d5201107-08b3-8681-993d-2ba3480707e7@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 17 Nov 2018 14:37:12 -0800
>
>
> On 11/15/2018 03:11 PM, David Miller wrote:
>
>>
>> Applied.
>>
>
> While reviewing this stuff, I found we have a bug.
>
> If napi_reuse_skb() is called, we might inherit from prior skb->pkt_type value.
>
> It seems that GRO could aggregate packets with pkt_type != PACKET_HOST, right ?
>
> David, any objection if I submit the following fix ?
Oh weird, so we do GRO frags accumulation using SKB which never goes through
eth_type_trans()?
I don't understand how we can, in this circumstance, assume PACKET_HOST?
Because that is what your suggested patch does.
Frame could be UDP multicast, and we could legitimately GRO accumulate it.
In that situations setting PACKET_HOST doesn't seem correct.
^ permalink raw reply
* Re: [net-next:master 114/114] drivers/net/tun.c:2503:42: warning: Using plain integer as NULL pointer
From: David Miller @ 2018-11-18 0:54 UTC (permalink / raw)
To: lkp; +Cc: jasowang, kbuild-all, netdev
In-Reply-To: <201811180738.uNZrNoZv%fengguang.wu@intel.com>
From: kbuild test robot <lkp@intel.com>
Date: Sun, 18 Nov 2018 07:13:44 +0800
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
> head: f9e06c45cb28beb30a6a474952ead7da2b8940f3
> commit: f9e06c45cb28beb30a6a474952ead7da2b8940f3 [114/114] tuntap: free XDP dropped packets in a batch
> config: x86_64-allmodconfig (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
> git checkout f9e06c45cb28beb30a6a474952ead7da2b8940f3
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
> include/linux/slab.h:332:43: warning: dubious: x & !y
> include/linux/slab.h:332:43: warning: dubious: x & !y
>>> drivers/net/tun.c:2503:42: warning: Using plain integer as NULL pointer
Fixed as follows:
====================
>From 6f0271d92963bc8588ad7058622ec7b1e94852de Mon Sep 17 00:00:00 2001
From: "David S. Miller" <davem@davemloft.net>
Date: Sat, 17 Nov 2018 16:53:46 -0800
Subject: [PATCH] tun: Adjust on-stack tun_page initialization.
Instead of constantly playing with the struct initializer
syntax trying to make gcc and CLang both happy, just clear
it out using memset().
>> drivers/net/tun.c:2503:42: warning: Using plain integer as NULL pointer
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/tun.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e90a7923a5f6..36163a147d39 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2500,10 +2500,12 @@ static int tun_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
return -EBADFD;
if (ctl && (ctl->type == TUN_MSG_PTR)) {
- struct tun_page tpage = {0};
+ struct tun_page tpage;
int n = ctl->num;
int flush = 0;
+ memset(&tpage, 0, sizeof(tpage));
+
local_bh_disable();
rcu_read_lock();
--
2.19.1
^ permalink raw reply related
* Re: [PATCH][net-next] net: slightly optimize eth_type_trans
From: Eric Dumazet @ 2018-11-18 1:19 UTC (permalink / raw)
To: David Miller, eric.dumazet; +Cc: lirongqing, netdev
In-Reply-To: <20181117.165138.801331179496411533.davem@davemloft.net>
On 11/17/2018 04:51 PM, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sat, 17 Nov 2018 14:37:12 -0800
>
>>
>>
>> On 11/15/2018 03:11 PM, David Miller wrote:
>>
>>>
>>> Applied.
>>>
>>
>> While reviewing this stuff, I found we have a bug.
>>
>> If napi_reuse_skb() is called, we might inherit from prior skb->pkt_type value.
>>
>> It seems that GRO could aggregate packets with pkt_type != PACKET_HOST, right ?
>>
>> David, any objection if I submit the following fix ?
>
> Oh weird, so we do GRO frags accumulation using SKB which never goes through
> eth_type_trans()?
>
> I don't understand how we can, in this circumstance, assume PACKET_HOST?
>
> Because that is what your suggested patch does.
>
> Frame could be UDP multicast, and we could legitimately GRO accumulate it.
> In that situations setting PACKET_HOST doesn't seem correct.
>
I might have been not very clear.
Issue is that with macvlan and GRO-friend traffic we can receive the following packets :
P1-P2 with PACKET_OTHERHOST because eth_type_trans() detected the dst MAC is not the eth0 device mac address.
P2 has been aggregated to P1, so the sk_buff has been put to napi->skb via napi_reuse_skb()
Then we receive on same NAPI packet P3, for this host, reusing napi->skb that was saved (old P2 sk_buff).
skb->pkt_type is PACKET_OTHERHOST.
eth_type_trans() does not change skb->pkt_type because ethernet dst mac address is our ethernet mac address.
-> We feed the upper stack with P3, with incorrect pkt_type.
-> packet is dropped because pkt_type != PACKET_HOST, for example in tcp_v4_rcv()
^ permalink raw reply
* Re: [PATCH][net-next] net: slightly optimize eth_type_trans
From: David Miller @ 2018-11-18 4:06 UTC (permalink / raw)
To: eric.dumazet; +Cc: lirongqing, netdev
In-Reply-To: <4c39a882-2d58-f258-9bd2-df0b2540be41@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 17 Nov 2018 17:19:34 -0800
> I might have been not very clear.
>
> Issue is that with macvlan and GRO-friend traffic we can receive the following packets :
>
> P1-P2 with PACKET_OTHERHOST because eth_type_trans() detected the dst MAC is not the eth0 device mac address.
>
> P2 has been aggregated to P1, so the sk_buff has been put to napi->skb via napi_reuse_skb()
>
> Then we receive on same NAPI packet P3, for this host, reusing napi->skb that was saved (old P2 sk_buff).
> skb->pkt_type is PACKET_OTHERHOST.
>
> eth_type_trans() does not change skb->pkt_type because ethernet dst mac address is our ethernet mac address.
>
> -> We feed the upper stack with P3, with incorrect pkt_type.
>
> -> packet is dropped because pkt_type != PACKET_HOST, for example in tcp_v4_rcv()
Oh I see, it is about defaults when using skb freshly from cache for another new packet.
Yes, I completely agree with your suggested patch.
Thanks for explaining.
^ permalink raw reply
* Re: [PATCH 01/10] flow_dissector: add flow_rule and flow_match structures and use them
From: David Miller @ 2018-11-18 4:32 UTC (permalink / raw)
To: pablo
Cc: netdev, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr
In-Reply-To: <20181116014139.25438-2-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri, 16 Nov 2018 02:41:30 +0100
> static int bnxt_tc_parse_flow(struct bnxt *bp,
> struct tc_cls_flower_offload *tc_flow_cmd,
> struct bnxt_tc_flow *flow)
> {
> - struct flow_dissector *dissector = tc_flow_cmd->dissector;
> + struct flow_rule *rule = &tc_flow_cmd->rule;
> + struct flow_dissector *dissector = rule->match.dissector;
This adds a reverse christmas tree violation. Please fix this up.
> @@ -255,10 +224,12 @@ static void cxgb4_process_flow_match(struct net_device *dev,
> static int cxgb4_validate_flow_match(struct net_device *dev,
> struct tc_cls_flower_offload *cls)
> {
> + struct flow_rule *rule = &cls->rule;
> + struct flow_dissector *dissector = rule->match.dissector;
> u16 ethtype_mask = 0;
> u16 ethtype_key = 0;
Likewise.
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 21c2688d6308..287cb8665bb3 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -7166,11 +7166,13 @@ static int i40e_parse_cls_flower(struct i40e_vsi *vsi,
> struct tc_cls_flower_offload *f,
> struct i40e_cloud_filter *filter)
> {
> + struct flow_rule *rule = &f->rule;
> + struct flow_dissector *dissector = rule->match.dissector;
> u16 n_proto_mask = 0, n_proto_key = 0, addr_type = 0;
> struct i40e_pf *pf = vsi->back;
> u8 field_flags = 0;
Likewise.
And so on and so forth for your entire submission.
^ permalink raw reply
* Re: [PATCH 10/10] dsa: bcm_sf2: use flow_rule infrastructure
From: David Miller @ 2018-11-18 4:32 UTC (permalink / raw)
To: pablo
Cc: netdev, thomas.lendacky, f.fainelli, ariel.elior, michael.chan,
santosh, madalin.bucur, yisen.zhuang, salil.mehta,
jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch, jakub.kicinski,
peppe.cavallaro, grygorii.strashko, andrew, vivien.didelot,
alexandre.torgue, joabreu, linux-net-drivers, ganeshgr
In-Reply-To: <20181116014139.25438-11-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri, 16 Nov 2018 02:41:39 +0100
> @@ -335,6 +336,11 @@ static int bcm_sf2_cfp_ipv4_rule_set(struct bcm_sf2_priv *priv, int port,
> struct ethtool_tcpip4_spec *v4_spec, *v4_m_spec;
Local variables v4_spec and v4_m_spec are now set but not used.
^ permalink raw reply
* Re: [PATCH net-next] uapi/ethtool: fix spelling errors
From: David Miller @ 2018-11-18 4:34 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin
In-Reply-To: <20181115232651.15948-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 15 Nov 2018 15:26:51 -0800
> Trivial spelling errors found by codespell.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied.
^ 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