* Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY
From: Andrew Lunn @ 2017-09-14 19:19 UTC (permalink / raw)
To: Corentin Labbe
Cc: Rob Herring, Maxime Ripard, mark.rutland, wens, linux,
catalin.marinas, will.deacon, peppe.cavallaro, alexandre.torgue,
f.fainelli, netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20170914185301.GB4021@Red>
> > Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"?
> > If the latter, then I think the node is fine, but then the mux should be
> > a child node of it. IOW, the child of an MDIO controller should either
> > be a mux node or slave devices.
Hi Rob
Up until now, children of an MDIO bus have been MDIO devices. Those
MDIO devices are either Ethernet PHYs, Ethernet Switches, or the
oddball devices that Broadcom iProc has, like generic PHYs.
We have never had MDIO-muxes as MDIO children. A Mux is not an MDIO
device, and does not have the properties of an MDIO device. It is not
addressable on the MDIO bus. The current MUXes are addressed via GPIOs
or MMIO.
There other similar cases. i2c-mux-gpio is not a child of an i2c bus,
nor i2c-mux-reg or gpio-mux. nxp,pca9548 is however a child of the i2c
bus, because it is an i2c device itself...
If the MDIO mux was an MDIO device, i would agree with you. Bit it is
not, so lets not make it a child.
Andrew
^ permalink raw reply
* Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY
From: Corentin Labbe @ 2017-09-14 18:53 UTC (permalink / raw)
To: Rob Herring
Cc: Maxime Ripard, mark.rutland-5wv7dgnIgG8, wens-jdAy2FN1RRM,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, peppe.cavallaro-qxv4g6HH51o,
alexandre.torgue-qxv4g6HH51o, andrew-g2DYL2Zd6BY,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170913182004.uniyo5opeilcfk7r@rob-hp-laptop>
On Wed, Sep 13, 2017 at 01:20:04PM -0500, Rob Herring wrote:
> On Fri, Sep 08, 2017 at 09:43:25AM +0200, Corentin Labbe wrote:
> > On Fri, Sep 08, 2017 at 09:25:38AM +0200, Maxime Ripard wrote:
> > > On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> > > > This patch add documentation about the MDIO switch used on sun8i-h3-emac
> > > > for integrated PHY.
> > > >
> > > > Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > ---
> > > > .../devicetree/bindings/net/dwmac-sun8i.txt | 127 +++++++++++++++++++--
> > > > 1 file changed, 120 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > > index 725f3b187886..3fa0e54825ea 100644
> > > > --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > > +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > > @@ -39,7 +39,7 @@ Optional properties for the following compatibles:
> > > > - allwinner,leds-active-low: EPHY LEDs are active low
> > > >
> > > > Required child node of emac:
> > > > -- mdio bus node: should be named mdio
> > > > +- mdio bus node: should be labelled mdio
> > >
> > > labels do not end up in the final DT (while the names do) so why are
> > > you making this change?
> > >
> >
> > I misunderstood label/name.
> > Anyway, this contrainst should leave due to "snps,dwmac-mdio MDIOs are automatically registered"
> >
> > > >
> > > > Required properties of the mdio node:
> > > > - #address-cells: shall be 1
> > > > @@ -48,14 +48,28 @@ Required properties of the mdio node:
> > > > The device node referenced by "phy" or "phy-handle" should be a child node
> > > > of the mdio node. See phy.txt for the generic PHY bindings.
> > > >
> > > > -Required properties of the phy node with the following compatibles:
> > > > +The following compatibles require an mdio-mux node called "mdio-mux":
> > > > + - "allwinner,sun8i-h3-emac"
> > > > + - "allwinner,sun8i-v3s-emac":
> > > > +Required properties for the mdio-mux node:
> > > > + - compatible = "mdio-mux"
> > > > + - one child mdio for the integrated mdio
> > > > + - one child mdio for the external mdio if present (V3s have none)
> > > > +Required properties for the mdio-mux children node:
> > > > + - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> > > > +
> > > > +The following compatibles require a PHY node representing the integrated
> > > > +PHY, under the integrated MDIO bus node if an mdio-mux node is used:
> > > > - "allwinner,sun8i-h3-emac",
> > > > - "allwinner,sun8i-v3s-emac":
> > > > +
> > > > +Required properties of the integrated phy node:
> > > > - clocks: a phandle to the reference clock for the EPHY
> > > > - resets: a phandle to the reset control for the EPHY
> > > > +- phy-is-integrated
> > > > +- Should be a child of the integrated mdio
> > >
> > > I'm not sure what you mean by that, you ask that it should (so not
> > > required?) be a child of the integrated mdio...
> > >
> >
> > I will change words to "must"
> >
> > > >
> > > > -Example:
> > > > -
> > > > +Example with integrated PHY:
> > > > emac: ethernet@1c0b000 {
> > > > compatible = "allwinner,sun8i-h3-emac";
> > > > syscon = <&syscon>;
> > > > @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
> > > > phy-handle = <&int_mii_phy>;
> > > > phy-mode = "mii";
> > > > allwinner,leds-active-low;
> > > > +
> > > > + mdio0: mdio {
> > >
> > > (You don't label it mdio here, unlike what was asked before)
> > >
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + compatible = "snps,dwmac-mdio";
> > > > + };
> > >
> > > I think Rob wanted that node gone?
> > >
> >
> > MDIO mux does not work without a parent MDIO, either gived by "parent-bus" or directly via mdio_mux_init() (like it is the case in dwmac-sun8i)
>
> Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"?
> If the latter, then I think the node is fine, but then the mux should be
> a child node of it. IOW, the child of an MDIO controller should either
> be a mux node or slave devices.
>
It will be snps,dwmac-mdio but putting mdio-mux as a child of it (the mdio node) give me:
[ 18.175338] libphy: stmmac: probed
[ 18.175379] mdio_bus stmmac-0: /soc/ethernet@1c30000/mdio/mdio-mux has invalid PHY address
[ 18.175408] mdio_bus stmmac-0: scan phy mdio-mux at address 0
[ 18.175450] mdio_bus stmmac-0: scan phy mdio-mux at address 1
[ 18.175482] mdio_bus stmmac-0: scan phy mdio-mux at address 2
[ 18.175513] mdio_bus stmmac-0: scan phy mdio-mux at address 3
[ 18.175544] mdio_bus stmmac-0: scan phy mdio-mux at address 4
[ 18.175575] mdio_bus stmmac-0: scan phy mdio-mux at address 5
[ 18.175607] mdio_bus stmmac-0: scan phy mdio-mux at address 6
[ 18.175638] mdio_bus stmmac-0: scan phy mdio-mux at address 7
[ 18.175669] mdio_bus stmmac-0: scan phy mdio-mux at address 8
[ 18.175700] mdio_bus stmmac-0: scan phy mdio-mux at address 9
[ 18.175731] mdio_bus stmmac-0: scan phy mdio-mux at address 10
[ 18.175762] mdio_bus stmmac-0: scan phy mdio-mux at address 11
[ 18.175795] mdio_bus stmmac-0: scan phy mdio-mux at address 12
[ 18.175827] mdio_bus stmmac-0: scan phy mdio-mux at address 13
[ 18.175858] mdio_bus stmmac-0: scan phy mdio-mux at address 14
[ 18.175889] mdio_bus stmmac-0: scan phy mdio-mux at address 15
[ 18.175919] mdio_bus stmmac-0: scan phy mdio-mux at address 16
[ 18.175951] mdio_bus stmmac-0: scan phy mdio-mux at address 17
[ 18.175982] mdio_bus stmmac-0: scan phy mdio-mux at address 18
[ 18.176014] mdio_bus stmmac-0: scan phy mdio-mux at address 19
[ 18.176045] mdio_bus stmmac-0: scan phy mdio-mux at address 20
[ 18.176076] mdio_bus stmmac-0: scan phy mdio-mux at address 21
[ 18.176107] mdio_bus stmmac-0: scan phy mdio-mux at address 22
[ 18.176139] mdio_bus stmmac-0: scan phy mdio-mux at address 23
[ 18.176170] mdio_bus stmmac-0: scan phy mdio-mux at address 24
[ 18.176202] mdio_bus stmmac-0: scan phy mdio-mux at address 25
[ 18.176233] mdio_bus stmmac-0: scan phy mdio-mux at address 26
[ 18.176271] mdio_bus stmmac-0: scan phy mdio-mux at address 27
[ 18.176320] mdio_bus stmmac-0: scan phy mdio-mux at address 28
[ 18.176371] mdio_bus stmmac-0: scan phy mdio-mux at address 29
[ 18.176420] mdio_bus stmmac-0: scan phy mdio-mux at address 30
[ 18.176452] mdio_bus stmmac-0: scan phy mdio-mux at address 31
Adding a fake <reg> to mdio-mux remove it.
Does it is acceptable ? or perhaps patching of_mdiobus_register() to not scan node with compatible "mdio-mux".
> >
> > > > + mdio-mux {
> > > > + compatible = "mdio-mux";
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + int_mdio: mdio@1 {
> > > > + reg = <0>;
>
> unit address of 1 and reg prop of 0 don't match. Build your dtb with
> W=2.
>
reg are arbitrary value (like mdio-mux-mmioreg), but in our case it is easy to fix this warning.
Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] net/mlx5: fpga: avoid uninitialized return codes
From: Saeed Mahameed @ 2017-09-14 18:39 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Arnd Bergmann, Ilan Tayari, Saeed Mahameed, Matan Barak,
Boris Pismenny, Linux Netdev List, linux-rdma, linux-kernel
In-Reply-To: <20170914125429.GZ3405@mtr-leonro.local>
On Thu, Sep 14, 2017 at 5:54 AM, Leon Romanovsky <leonro@mellanox.com> wrote:
> On Thu, Sep 14, 2017 at 01:06:18PM +0200, Arnd Bergmann wrote:
>> calling mlx5_fpga_mem_{read,write}_i2c() with a zero length on
>> older compiler version such as gcc-4.6 results in a warning that
>> the return code is not initialized:
>>
>> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c:147:6: error: ‘err’ may be used uninitialized in this function [-Werror=uninitialized]
>> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c:126:6: error: ‘err’ may be used uninitialized in this function [-Werror=uninitialized]
>>
>> On newer compilers, the 'err' variable is optimized away in this
>> code path and assumed to be zero when the loop completes, so we
>> don't get this warning.
>>
>> I'm changing the function here to instead return -EINVAL for the
>> case, under the assumption that it was never meant to be called
>> with a zero length argument.
Thanks Arnd, Looks good.
>
> I agree with you that size can't be zero and this patch will fix the
> warning, but if it is possible, I will prefer to have this check is
> written explicitly and not implicitly.
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> index 3c11d6e2160a..0e85bebe1dad 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> @@ -69,6 +69,9 @@ static int mlx5_fpga_mem_read_i2c(struct mlx5_fpga_device *fdev, size_t size,
> if (!fdev->mdev)
> return -ENOTCONN;
>
> + if (!size)
> + return -EINVAL;
> +
Or simply trust the caller :), and avoid redundant code.
> while (bytes_done < size) {
> actual_size = min(max_size, (size - bytes_done));
>
> Thanks
>
>>
>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
>> index 3c11d6e2160a..914fb9d77a1a 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
>> @@ -64,7 +64,7 @@ static int mlx5_fpga_mem_read_i2c(struct mlx5_fpga_device *fdev, size_t size,
>> size_t max_size = MLX5_FPGA_ACCESS_REG_SIZE_MAX;
>> size_t bytes_done = 0;
>> u8 actual_size;
>> - int err;
>> + int err = -EINVAL;
>>
>> if (!fdev->mdev)
>> return -ENOTCONN;
>> @@ -93,7 +93,7 @@ static int mlx5_fpga_mem_write_i2c(struct mlx5_fpga_device *fdev, size_t size,
>> size_t max_size = MLX5_FPGA_ACCESS_REG_SIZE_MAX;
>> size_t bytes_done = 0;
>> u8 actual_size;
>> - int err;
>> + int err = -EINVAL;
>>
>> if (!fdev->mdev)
>> return -ENOTCONN;
>> --
>> 2.9.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 02/10] dt-bindings: net: Restore sun8i dwmac binding
From: Corentin Labbe @ 2017-09-14 18:31 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland, maxime.ripard, wens, linux, catalin.marinas,
will.deacon, peppe.cavallaro, alexandre.torgue, andrew,
f.fainelli, netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20170913180734.xvazvf7thpqtssgz@rob-hp-laptop>
On Wed, Sep 13, 2017 at 01:07:34PM -0500, Rob Herring wrote:
> On Fri, Sep 08, 2017 at 09:11:48AM +0200, Corentin Labbe wrote:
> > This patch restore dt-bindings documentation about dwmac-sun8i
> > This reverts commit 8aa33ec2f481 ("dt-bindings: net: Revert sun8i dwmac binding")
>
> Why?
>
I have put the reason in cover-letter and forget to put it in here also.
I fix that in next series.
Thanks
^ permalink raw reply
* Re: Bug with BPF_ALU64 | BPF_END?
From: David Miller @ 2017-09-14 18:14 UTC (permalink / raw)
To: ecree; +Cc: daniel, ast, netdev
In-Reply-To: <52c7df55-84d9-f6d2-ed84-51ac90eb6bcc@solarflare.com>
From: Edward Cree <ecree@solarflare.com>
Date: Thu, 14 Sep 2017 18:53:17 +0100
> Is BPF_END supposed to only be used with BPF_ALU, never with BPF_ALU64?
> In kernel/bpf/core.c:___bpf_prog_run(), there are only jump table targets
> for the BPF_ALU case, not for the BPF_ALU64 case (opcodes 0xd7 and 0xdf).
> But the verifier doesn't enforce this; by crafting a program that uses
> these opcodes I can get a WARN when they're run (without JIT; it looks
> like the x86 JIT, at least, won't like it either).
> Proposed patch below the cut; build-tested only.
Good catch.
A really neat test would be a program that uploads random BPF programs
into the kernel, in a syzkaller'ish way. It might have triggered this
(eventually).
^ permalink raw reply
* Re: RFC: Audit Kernel Container IDs
From: Richard Guy Briggs @ 2017-09-14 18:07 UTC (permalink / raw)
To: Eric W. Biederman
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, Linux Containers, Linux API,
Linux Audit, Linux FS Devel, Linux Kernel,
Linux Network Development, Aristeu Rozanski, David Howells,
Eric Paris, jlayton-H+wXaHxf7aLQT0dZR+AlfA, Andy Lutomirski,
mszeredi-H+wXaHxf7aLQT0dZR+AlfA, Paul Moore, Serge E. Hallyn,
Steve Grubb, trondmy-7I+n7zu2hftEKMMhf/gKZA, Al Viro
In-Reply-To: <87d16tb2y5.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
On 2017-09-14 12:33, Eric W. Biederman wrote:
> Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:
>
> > The trigger is a pseudo filesystem (proc, since PID tree already exists)
> > write of a u64 representing the container ID to a file representing a
> > process that will become the first process in a new container.
> > This might place restrictions on mount namespaces required to define a
> > container, or at least careful checking of namespaces in the kernel to
> > verify permissions of the orchestrator so it can't change its own
> > container ID.
>
> Why a u64?
u32 will roll too quickly. UUID is large enough that it adds
significantly to audit record bandwidth. I'd prefer u64, but can look
at the difference of accommodating a UUID...
> Why a proc filesystem write and not a magic audit message?
A magic audit message requires CAP_AUDIT_WRITE, which we'd like to use
sparingly. Given that orchestrators will already require it to send
the mandatory AUDIT_VIRT_*, this doesn't seem like an unreasonable burden.
I was originally leaning towards an audit message trigger or a syscall.
> I don't like the fact that the proc filesystem entry is likely going to
> be readable and abusable by non-audit contexts?
This proposal wasn't going to start with that link being readable, but
its filesystem structure and link names would be, perhaps giving away
too much already.
I think we will need to find a way for the orchestrator or one of its
authorized agents to read this information while blocking reads from
unauthorized agents, otherwise this would be of very limited use.
> Why the ability to change the containerid? What is the use case you are
> thinking of there?
This was covered in the end of the conversation with Paul Moore (that
maybe you got tired reading?) I'd originally proposed having it write
once, but Paul figured there was no good reason to restrict it and leave
that decision up to the orchestrator. The use case would be adding
other processes to a container, but it could be argued all additional
processes should be spawned by the first process in a container.
> Eric
- RGB
--
Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: RTL8192EE PCIe Wireless Network Adapter crashed with linux-4.13
From: Larry Finger @ 2017-09-14 18:05 UTC (permalink / raw)
To: Zwindl, linux-wireless@vger.kernel.org
Cc: chaoming_li@realsil.com.cn, kvalo@codeaurora.org,
pkshih@realtek.com, johannes.berg@intel.com,
gregkh@linuxfoundation.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <2WhCEttl4IJtrTB2AkJYmhbsu-kiR_5fU0A_Z7eWW05rbVE5tB86A_ZS8ek_FQFdgaZw1bfx0wFbxOa0Ydjv1T6BzBkIHtaWTbSptcpB_kg=@protonmail.com>
On 09/14/2017 08:30 AM, Zwindl wrote:
> Dear developers:
> I'm using Arch Linux with testing enabled, the current kernel version and
> details are
> `Linux zwindl 4.13.2-1-ARCH #1 SMP PREEMPT Thu Sep 14 02:57:34 UTC 2017 x86_64
> GNU/Linux`.
> The wireless card can't work properly from the kernel 4.13. Here's the log(in
> attachment) when NetworkManager trying to connect my wifi which is named as
> 'TP', my mac addr hided as xx:xx:xx:xx:xx.
> What should I provide to help to debug?
> ZWindL.
The BUG-ON arises in __intel_map_single() due to dir (for direction of DMA)
equal to DMA_NONE (3). When rtl8192ee calls pci_map_single(), it uses
PCI_DMA_TODEVICE (1). I followed the calling sequence through the entire chain,
and none of the routines made any changes to 'dir', other that changing the type
from int to enum dma_data_direction. That would not have changed a 1 to a 3.
I built a 4.13.2 system. The problem does not happen here. At this point, the
system has been up for about two hours. I did discover a small memory leak
associated with firmware loading, but that should not have caused the problem.
Nonetheless, I will be sending a patch to fix that problem.
I will continue testing, although I doubt that the problem will happen here.
How long had your system been up when the problem occurred? Your dmesg fragment
did not show any times. What kernels have you tried besides 4.13.2?
Larry
^ permalink raw reply
* Bug with BPF_ALU64 | BPF_END?
From: Edward Cree @ 2017-09-14 17:53 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov; +Cc: netdev
Is BPF_END supposed to only be used with BPF_ALU, never with BPF_ALU64?
In kernel/bpf/core.c:___bpf_prog_run(), there are only jump table targets
for the BPF_ALU case, not for the BPF_ALU64 case (opcodes 0xd7 and 0xdf).
But the verifier doesn't enforce this; by crafting a program that uses
these opcodes I can get a WARN when they're run (without JIT; it looks
like the x86 JIT, at least, won't like it either).
Proposed patch below the cut; build-tested only.
-Ed
---
[PATCH net] bpf/verifier: reject BPF_ALU64|BPF_END
Neither ___bpf_prog_run nor the JITs accept it.
Fixes: 17a5267067f3 ("bpf: verifier (add verifier core)")
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
kernel/bpf/verifier.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 477b693..799b245 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2292,7 +2292,8 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
}
} else {
if (insn->src_reg != BPF_REG_0 || insn->off != 0 ||
- (insn->imm != 16 && insn->imm != 32 && insn->imm != 64)) {
+ (insn->imm != 16 && insn->imm != 32 && insn->imm != 64) ||
+ BPF_CLASS(insn->code) == BPF_ALU64) {
verbose("BPF_END uses reserved fields\n");
return -EINVAL;
}
^ permalink raw reply related
* Re: [PATCH net] netvsc: increase default receive buffer size
From: Stephen Hemminger @ 2017-09-14 17:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev, haiyangz, sthemmin, devel
In-Reply-To: <20170914.100203.1656028222884493229.davem@davemloft.net>
On Thu, 14 Sep 2017 10:02:03 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Thu, 14 Sep 2017 09:31:07 -0700
>
> > The default receive buffer size was reduced by recent change
> > to a value which was appropriate for 10G and Windows Server 2016.
> > But the value is too small for full performance with 40G on Azure.
> > Increase the default back to maximum supported by host.
> >
> > Fixes: 8b5327975ae1 ("netvsc: allow controlling send/recv buffer size")
> > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
>
> What other side effects are there to making this buffer so large?
>
> Just curious...
It increase latency and exercises bufferbloat avoidance on TCP.
The problem was the smaller buffer caused regressions in UDP
benchmarks on 40G Azure. One could argue that this is not a reasonable
benchmark but people run it. Apparently, Windows already went
the same thing and uses an even bigger buffer.
Longer term there will be more internal discussion with different
teams about what the receive latency and buffering needs to be.
Also, the issue goes away when doing accelerated networking (SR-IOV)
is more widely used.
^ permalink raw reply
* Re: RFC: Audit Kernel Container IDs
From: Eric W. Biederman @ 2017-09-14 17:33 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: cgroups, Linux Containers, Linux API, Linux Audit, Linux FS Devel,
Linux Kernel, Linux Network Development, Aristeu Rozanski,
David Howells, Eric Paris, jlayton, Andy Lutomirski, mszeredi,
Paul Moore, Serge E. Hallyn, Steve Grubb, trondmy, Al Viro
In-Reply-To: <20170913171328.GP3405@madcap2.tricolour.ca>
Richard Guy Briggs <rgb@redhat.com> writes:
> The trigger is a pseudo filesystem (proc, since PID tree already exists)
> write of a u64 representing the container ID to a file representing a
> process that will become the first process in a new container.
> This might place restrictions on mount namespaces required to define a
> container, or at least careful checking of namespaces in the kernel to
> verify permissions of the orchestrator so it can't change its own
> container ID.
Why a u64?
Why a proc filesystem write and not a magic audit message?
I don't like the fact that the proc filesystem entry is likely going to
be readable and abusable by non-audit contexts?
Why the ability to change the containerid? What is the use case you are
thinking of there?
Eric
^ permalink raw reply
* Re: [PATCH net] netvsc: increase default receive buffer size
From: David Miller @ 2017-09-14 17:02 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, netdev, devel, sthemmin
In-Reply-To: <20170914163107.8404-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 14 Sep 2017 09:31:07 -0700
> The default receive buffer size was reduced by recent change
> to a value which was appropriate for 10G and Windows Server 2016.
> But the value is too small for full performance with 40G on Azure.
> Increase the default back to maximum supported by host.
>
> Fixes: 8b5327975ae1 ("netvsc: allow controlling send/recv buffer size")
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
What other side effects are there to making this buffer so large?
Just curious...
^ permalink raw reply
* Re: [PATCH] tg3: clean up redundant initialization of tnapi
From: David Miller @ 2017-09-14 17:00 UTC (permalink / raw)
To: colin.king
Cc: siva.kallam, prashant, mchan, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20170914160125.11007-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Thu, 14 Sep 2017 17:01:25 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> tnapi is being initialized and then immediately updated and
> hence the initialiation is redundant. Clean up the warning
> by moving the declaration and initialization to the inside
> of the for-loop.
>
> Cleans up clang scan-build warning:
> warning: Value stored to 'tnapi' during its initialization is never read
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH] tls: make tls_sw_free_resources static
From: David Miller @ 2017-09-14 16:56 UTC (permalink / raw)
To: tklauser; +Cc: ilyal, aviadye, davejwatson, netdev
In-Reply-To: <20170914112225.4296-1-tklauser@distanz.ch>
From: Tobias Klauser <tklauser@distanz.ch>
Date: Thu, 14 Sep 2017 13:22:25 +0200
> Make the needlessly global function tls_sw_free_resources static to fix
> a gcc/sparse warning.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Applied.
^ permalink raw reply
* Page allocator bottleneck
From: Tariq Toukan @ 2017-09-14 16:49 UTC (permalink / raw)
To: David Miller, Jesper Dangaard Brouer, Mel Gorman, Eric Dumazet,
Alexei Starovoitov, Saeed Mahameed, Eran Ben Elisha,
Linux Kernel Network Developers, Andrew Morton, Michal Hocko,
linux-mm
Hi all,
As part of the efforts to support increasing next-generation NIC speeds,
I am investigating SW bottlenecks in network stack receive flow.
Here I share some numbers I got for a simple experiment, in which I
simulate the page allocation rate needed in 200Gpbs NICs.
I ran the test below over 3 different (modified) mlx5 driver versions,
loaded on server side (RX):
1) RX page cache disabled, 2 packets per page.
2) RX page cache disabled, one packet per page.
3) Huge RX page cache, one packet per page.
All page allocations are of order 0.
NIC: Connectx-5 100 Gbps.
CPU: Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz
Test:
128 TCP streams (using super_netperf).
Changing num of RX queues.
HW LRO OFF, GRO ON, MTU 1500.
Observe: BW as a function of num RX queues.
Results:
Driver #1:
#rings BW (Mbps)
1 23,813
2 44,086
3 62,128
4 78,058
6 94,210 (linerate)
8 94,205 (linerate)
12 94,202 (linerate)
16 94,191 (linerate)
Driver #2:
#rings BW (Mbps)
1 18,835
2 36,716
3 50,521
4 61,746
6 63,637
8 60,299
12 51,048
16 43,337
Driver #3:
#rings BW (Mbps)
1 19,316
2 44,850
3 69,549
4 87,434
6 94,342 (linerate)
8 94,350 (linerate)
12 94,327 (linerate)
16 94,327 (linerate)
Insights:
Major degradation between #1 and #2, not getting any close to linerate!
Degradation is fixed between #2 and #3.
This is because page allocator cannot stand the higher allocation rate.
In #2, we also see that the addition of rings (cores) reduces BW (!!),
as result of increasing congestion over shared resources.
Congestion in this case is very clear.
When monitored in perf top:
85.58% [kernel] [k] queued_spin_lock_slowpath
I think that page allocator issues should be discussed separately:
1) Rate: Increase the allocation rate on a single core.
2) Scalability: Reduce congestion and sync overhead between cores.
This is clearly the current bottleneck in the network stack receive flow.
I know about some efforts that were made in the past two years.
For example the ones from Jesper et al.:
- Page-pool (not accepted AFAIK).
- Page-allocation bulking.
- Optimize order-0 allocations in Per-Cpu-Pages.
I am not an mm expert, but wanted to raise the issue again, to combine
the efforts and hear from you guys about status and possible directions.
Best regards,
Tariq Toukan
^ permalink raw reply
* Re: [LPC] 2nd RDMA Mini-Summit Schedule
From: Leon Romanovsky @ 2017-09-14 16:48 UTC (permalink / raw)
To: RDMA mailing list
Cc: linux-netdev, NVME mailing list, Jason Gunthorpe, Bart Van Assche,
Knut Omang, Christoph Lameter, Don Dutile, Liran Liss, Tzahi Oved,
Matan Barak, Dennis Dalessandro, Yuval Shaia, Marcel Apfelbaum,
lwn-T1hC0tSOHrs
In-Reply-To: <20170913130216.GT3405-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3327 bytes --]
On Wed, Sep 13, 2017 at 04:02:16PM +0300, Leon Romanovsky wrote:
> On Thu, Sep 07, 2017 at 08:18:05AM +0300, Leon Romanovsky wrote:
> > Hi,
> >
> > We're happy to announce schedule of the 2nd RDMA mini-summit, which will be
> > held as part of coming Linux Plumbers Conference 2017.
> >
> > During the conference, we will have two sessions: main track session and
> > round table session.
> >
> > First session will be held on Thursday, September 14, 2017 from 9:30am – 12:30pm
> > with the topics relevant for the wider audience.
> >
> > In the second session, we will focus on more face to face discussions in round
> > table format and it will be scheduled a little bit later once we will get meeting
> > room.
>
> UPDATE:
> The round table will take place at 2:00pm on Thursday, September 14,
> 2017 in one of the speakers's suites (3rd floor, near Georgia I/II).
>
> Thanks
UPDATE:
RDMA round table will be held at Platinum Ballroom J. from 2:00pm till 6:30pm.
Thanks
>
> >
> > ---------------------------------------------------------------------------------------
> > FIRST SESSION:
> > ---------------------------------------------------------------------------------------
> > * Backporting issues with multi-subsystem device (RDMA + netdev + more) by Don Dutile
> > - More and more devices in RDMA are dependent on netdev as well as other
> > subsystems -- target, NFS, scsi, block, cgroups, SELinux.... Can we make
> > backporting task for distro people easier?
> > * kABI Update by Matan Barak
> > - Going forward with a flexible and secure RDMA kABI
> > * System Boot and RDMA by Jason Gunthrope
> > - Discuss boot-time issues with the RDMA subsystem
> > * Paravirtual RDMA device by Marcel Apfelbaum and Yuval Shaia
> > - QEMU's limited RDMA support leaves it behind other modern hypervisors.
> > Marcel and Yuval will present the implementation of an emulated RDMA device,
> > analyze its performance and usability, and finally talk about future plans
> > for a possible virtio-rdma device.
> > * TX Flow Steering for IPsec by Liran Liss (pending time slot)
> > - The IPsec protocol provides both encryption and authentication for IP
> > protocol packets. Users of the raw ethernet QP that send/receive IPsec packets
> > could benefit from the IPsec crypto offloads capabilities that are
> > available in recent NICs.
> > * OpenFabrics Alliance - Status and New Directions by Jason Gunthrope
> > * BOF - Open Discussion
> > - Open discussion with the community lead by Leon Romanovsky, Jason Gunthorpe,
> > Christoph Lameter and Dennis Dalessandro
> >
> > ---------------------------------------------------------------------------------------
> > ROUND TABLE SESSION:
> > ---------------------------------------------------------------------------------------
> > Possible list, bring your own topics:
> > * RDMA and ULPs
> > * Verbs testing suite
> > * RDMA netlink and RDMAtool future plans
> > * RDMA and the Linux IP stack
> > * Infrastructure issues with Travis CI
> > * Infrastructure gaps with rdma-core
> > * Packaging as part of CI for rdma-core.
> > * ....
> >
> > See you next week at Linux Plumber Conference 2017.
> >
> > Thanks
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net] tcp: update skb->skb_mstamp more carefully
From: Soheil Hassas Yeganeh @ 2017-09-14 16:41 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Neal Cardwell, Eric Dumazet, liujian, David Miller, Eric Dumazet,
Jerry Chu, Netdev, weiyongjun (A), wangkefeng 00227729
In-Reply-To: <CAK6E8=dzYTaXnrXGeANX+wOtE+jfzX7M1ZokToe=ukm+tRK+Jw@mail.gmail.com>
On Thu, Sep 14, 2017 at 12:32 PM, Yuchung Cheng <ycheng@google.com> wrote:
> On Thu, Sep 14, 2017 at 6:57 AM, Neal Cardwell <ncardwell@google.com> wrote:
>> On Wed, Sep 13, 2017 at 11:30 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>
>>> From: Eric Dumazet <edumazet@googl.com>
>>>
>>> liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
>>> in tcp_probe_timer() :
>>> https://www.spinics.net/lists/netdev/msg454496.html
>>>
>>> After investigations, the root cause of the problem is that we update
>>> skb->skb_mstamp of skbs in write queue, even if the attempt to send a
>>> clone or copy of it failed. One reason being a routing problem.
>>>
>>> This patch prevents this, solving liujian issue.
>>>
>>> It also removes a potential RTT miscalculation, since
>>> __tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
>>> TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
>>> been changed.
>>>
>>> A future ACK would then lead to a very small RTT sample and min_rtt
>>> would then be lowered to this too small value.
>> ...
>>>
>>> Signed-off-by: Eric Dumazet <edumazet@googl.com>
>>> Reported-by: liujian <liujian56@huawei.com>
>>> ---
>>> net/ipv4/tcp_output.c | 19 ++++++++++++-------
>>> 1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> Acked-by: Neal Cardwell <ncardwell@google.com>
> Acked-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Very nice! Thank you, Eric!
^ permalink raw reply
* Re: [PATCH net] tcp: update skb->skb_mstamp more carefully
From: Yuchung Cheng @ 2017-09-14 16:32 UTC (permalink / raw)
To: Neal Cardwell
Cc: Eric Dumazet, liujian, David Miller, Eric Dumazet, Jerry Chu,
Netdev, weiyongjun (A), wangkefeng 00227729
In-Reply-To: <CADVnQy=OeD1C4xg6pdYY98djsGCdnKLv6PD5vC7j6-GSkfr8Yw@mail.gmail.com>
On Thu, Sep 14, 2017 at 6:57 AM, Neal Cardwell <ncardwell@google.com> wrote:
> On Wed, Sep 13, 2017 at 11:30 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> From: Eric Dumazet <edumazet@googl.com>
>>
>> liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
>> in tcp_probe_timer() :
>> https://www.spinics.net/lists/netdev/msg454496.html
>>
>> After investigations, the root cause of the problem is that we update
>> skb->skb_mstamp of skbs in write queue, even if the attempt to send a
>> clone or copy of it failed. One reason being a routing problem.
>>
>> This patch prevents this, solving liujian issue.
>>
>> It also removes a potential RTT miscalculation, since
>> __tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
>> TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
>> been changed.
>>
>> A future ACK would then lead to a very small RTT sample and min_rtt
>> would then be lowered to this too small value.
> ...
>>
>> Signed-off-by: Eric Dumazet <edumazet@googl.com>
>> Reported-by: liujian <liujian56@huawei.com>
>> ---
>> net/ipv4/tcp_output.c | 19 ++++++++++++-------
>> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
nicely done!
>
> Thanks, Eric!
>
> neal
^ permalink raw reply
* [PATCH net] netvsc: increase default receive buffer size
From: Stephen Hemminger @ 2017-09-14 16:31 UTC (permalink / raw)
To: kys, haiyangz, davem; +Cc: netdev, Stephen Hemminger, devel
The default receive buffer size was reduced by recent change
to a value which was appropriate for 10G and Windows Server 2016.
But the value is too small for full performance with 40G on Azure.
Increase the default back to maximum supported by host.
Fixes: 8b5327975ae1 ("netvsc: allow controlling send/recv buffer size")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index c538a4f15f3b..d4902ee5f260 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -49,7 +49,7 @@
#define NETVSC_MIN_TX_SECTIONS 10
#define NETVSC_DEFAULT_TX 192 /* ~1M */
#define NETVSC_MIN_RX_SECTIONS 10 /* ~64K */
-#define NETVSC_DEFAULT_RX 2048 /* ~4M */
+#define NETVSC_DEFAULT_RX 10485 /* Max ~16M */
#define LINKCHANGE_INT (2 * HZ)
#define VF_TAKEOVER_INT (HZ / 10)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2] vxlan: only reduce known arp broadcast request to support virtual IP
From: Jiri Benc @ 2017-09-14 16:14 UTC (permalink / raw)
To: Chen Haiquan; +Cc: davem, netdev
In-Reply-To: <20170914151440.GA30675@samsung>
On Thu, 14 Sep 2017 23:14:40 +0800, Chen Haiquan wrote:
> +static bool arp_reduce_ignore_unknown_ip;
> +module_param(arp_reduce_ignore_unknown_ip, bool, 0644);
> +MODULE_PARM_DESC(arp_reduce_ignore_unknown_ip,
> + "Only reduce known arp broaddcast request to support virtual IP");
Oh, no. This is not the way. It needs to be a runtime configuration
option (a netlink attribute). Generally, every time you find yourself
adding a module parameter, you're doing something wrong.
Also, net-next is currently closed. You should not submit new features
during this period, it's bugfixes only now.
http://vger.kernel.org/~davem/net-next.html
Jiri
^ permalink raw reply
* [PATCH] tg3: clean up redundant initialization of tnapi
From: Colin King @ 2017-09-14 16:01 UTC (permalink / raw)
To: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
tnapi is being initialized and then immediately updated and
hence the initialiation is redundant. Clean up the warning
by moving the declaration and initialization to the inside
of the for-loop.
Cleans up clang scan-build warning:
warning: Value stored to 'tnapi' during its initialization is never read
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/broadcom/tg3.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index af33dc15c55f..54588809b867 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -11536,11 +11536,11 @@ static int tg3_start(struct tg3 *tp, bool reset_phy, bool test_irq,
tg3_napi_enable(tp);
for (i = 0; i < tp->irq_cnt; i++) {
- struct tg3_napi *tnapi = &tp->napi[i];
err = tg3_request_irq(tp, i);
if (err) {
for (i--; i >= 0; i--) {
- tnapi = &tp->napi[i];
+ struct tg3_napi *tnapi = &tp->napi[i];
+
free_irq(tnapi->irq_vec, tnapi);
}
goto out_napi_fini;
--
2.14.1
^ permalink raw reply related
* selftests/bpf doesn't compile
From: Shuah Khan @ 2017-09-14 15:33 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Thomas Meyer
Cc: linux-kernel, linux-kselftest, Shuah Khan, Networking
In-Reply-To: <ed20875b-7a01-fbc8-e8d1-3929ba35c8ac@kernel.org>
Hi Alexei and Daniel,
bpf test depends on clang and fails to compile when
------------------------------------------------------
make -C tools/testing/selftests/bpf run_tests
make: clang: Command not found
Makefile:39: recipe for target '.linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o' failed
make: *** [./linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o] Error 127
make: Leaving directory '.linux-kselftest/tools/testing/selftests/bpf'
With "make TARGETS=bpf kselftest" it fails earlier:
make[3]: Entering directory './linux-kselftest/tools/lib/bpf'
Makefile:40: tools/scripts/Makefile.arch: No such file or directory
Makefile:84: tools/build/Makefile.feature: No such file or directory
Makefile:143: tools/build/Makefile.include: No such file or directory
make[3]: *** No rule to make target 'tools/build/Makefile.include'. Stop.
make[3]: Leaving directory '.linux-kselftest/tools/lib/bpf'
Makefile:34: recipe for target './linux-kselftest/tools/testing/selftests/bpf/libbpf.a' failed
make[2]: *** [./linux-kselftest/tools/testing/selftests/bpf/libbpf.a] Error 2
make[2]: Leaving directory './linux-kselftest/tools/testing/selftests/bpf'
Makefile:69: recipe for target 'all' failed
make[1]: *** [all] Error 2
Makefile:1190: recipe for target 'kselftest' failed
make: *** [kselftest] Error 2
--------------------------------------------------------------
Is bpf test intended to be run in kselftest run? The clang dependency might
not be met on majority of the systems. Is this a hard dependency??
Would it make sense to create a special target for bpf test? We do have a few
tests that do that now.
TARGETS_HOTPLUG = cpu-hotplug
TARGETS_HOTPLUG += memory-hotplug
I could add a special target for bpf TARGET_BPF perhaps and exclude it from
the run_tests.
Please let me know your thoughts on this.
thanks,
-- Shuah
^ permalink raw reply
* [PATCH v2] vxlan: only reduce known arp broadcast request to support virtual IP
From: Chen Haiquan @ 2017-09-14 15:14 UTC (permalink / raw)
To: jbenc; +Cc: davem, netdev, oc
The purpose of vxlan arp reduce feature is to reply the broadcast
arp request in vtep instead of sending it out to save traffic.
The current implementation drops arp packet, if the ip cannot be
found in neigh table. In the case of virtual IP address, user
defines IP address without management from SDN controller. The IP
address does not exist in neigh table, so the arp broadcast request
from a client can not be sent to the server who owns the virtual IP
address.
This patch allow the arp request to be sent out if:
1. not arp broadcast request
2. cannot be found in neigh table
3. arp record status is not NUD_CONNECTED
The user defined of virtual IP address works while arp reduce still
suppress the arp broadcast for IP address managed by SDN controller
with this patch.
Signed-off-by: Chen Haiquan <oc@yunify.com>
---
Changes in v2:
- Add config option, arp_reduce_ignore_unknown_ip, to enable
the behavior.
---
drivers/net/vxlan.c | 42 +++++++++++++++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index d7c49cf1d5e9..cc9ee28f3481 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -53,6 +53,11 @@ static bool log_ecn_error = true;
module_param(log_ecn_error, bool, 0644);
MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN");
+static bool arp_reduce_ignore_unknown_ip;
+module_param(arp_reduce_ignore_unknown_ip, bool, 0644);
+MODULE_PARM_DESC(arp_reduce_ignore_unknown_ip,
+ "Only reduce known arp broaddcast request to support virtual IP");
+
static unsigned int vxlan_net_id;
static struct rtnl_link_ops vxlan_link_ops;
@@ -1473,7 +1478,7 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
parp->ar_op != htons(ARPOP_REQUEST) ||
parp->ar_hln != dev->addr_len ||
parp->ar_pln != 4)
- goto out;
+ goto ignore;
arpptr = (u8 *)parp + sizeof(struct arphdr);
sha = arpptr;
arpptr += dev->addr_len; /* sha */
@@ -1494,7 +1499,7 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
if (!(n->nud_state & NUD_CONNECTED)) {
neigh_release(n);
- goto out;
+ goto ignore;
}
f = vxlan_find_mac(vxlan, n->ha, vni);
@@ -1526,10 +1531,20 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
};
vxlan_ip_miss(dev, &ipa);
+ goto ignore;
+ } else {
+ /* broadcast unknown arp */
+ goto ignore;
}
+
out:
consume_skb(skb);
return NETDEV_TX_OK;
+
+ignore:
+ if (arp_reduce_ignore_unknown_ip)
+ return 1;
+ goto out;
}
#if IS_ENABLED(CONFIG_IPV6)
@@ -1642,7 +1657,7 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
msg = (struct nd_msg *)(iphdr + 1);
if (msg->icmph.icmp6_code != 0 ||
msg->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION)
- goto out;
+ goto ignore;
if (ipv6_addr_loopback(daddr) ||
ipv6_addr_is_multicast(&msg->target))
@@ -1656,7 +1671,7 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
if (!(n->nud_state & NUD_CONNECTED)) {
neigh_release(n);
- goto out;
+ goto ignore;
}
f = vxlan_find_mac(vxlan, n->ha, vni);
@@ -1684,11 +1699,20 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
};
vxlan_ip_miss(dev, &ipa);
+ goto ignore;
+ } else {
+ /* broadcast unknown neigh */
+ goto ignore;
}
out:
consume_skb(skb);
return NETDEV_TX_OK;
+
+ignore:
+ if (arp_reduce_ignore_unknown_ip)
+ return 1;
+ goto out;
}
#endif
@@ -2266,8 +2290,10 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
if (vxlan->cfg.flags & VXLAN_F_PROXY) {
eth = eth_hdr(skb);
- if (ntohs(eth->h_proto) == ETH_P_ARP)
- return arp_reduce(dev, skb, vni);
+ if (ntohs(eth->h_proto) == ETH_P_ARP) {
+ if (arp_reduce(dev, skb, vni) == NETDEV_TX_OK)
+ return NETDEV_TX_OK;
+ }
#if IS_ENABLED(CONFIG_IPV6)
else if (ntohs(eth->h_proto) == ETH_P_IPV6) {
struct ipv6hdr *hdr, _hdr;
@@ -2275,7 +2301,9 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
skb_network_offset(skb),
sizeof(_hdr), &_hdr)) &&
hdr->nexthdr == IPPROTO_ICMPV6)
- return neigh_reduce(dev, skb, vni);
+ if (neigh_reduce(dev,
+ skb, vni) == NETDEV_TX_OK)
+ return NETDEV_TX_OK;
}
#endif
}
--
2.7.4
^ permalink raw reply related
* [PATCH iproute2] tc: fix typo in tc-tcindex man page
From: Davide Caratti @ 2017-09-14 15:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
fix mis-typed 'pass_on' keyword.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
man/man8/tc-tcindex.8 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man/man8/tc-tcindex.8 b/man/man8/tc-tcindex.8
index 7fcf8254..9a4e5ffc 100644
--- a/man/man8/tc-tcindex.8
+++ b/man/man8/tc-tcindex.8
@@ -11,7 +11,7 @@ tcindex \- traffic control index filter
.IR MASK " ] [ "
.B shift
.IR SHIFT " ] [ "
-.BR pas_on " | " fall_through " ] [ " classid
+.BR pass_on " | " fall_through " ] [ " classid
.IR CLASSID " ] [ "
.B action
.BR ACTION_SPEC " ]"
--
2.13.5
^ permalink raw reply related
* Re: [Intel-wired-lan] [PATCH] igb: check memory allocation failure
From: Waskiewicz Jr, Peter @ 2017-09-14 14:47 UTC (permalink / raw)
To: Brown, Aaron F, Christophe JAILLET, Kirsher, Jeffrey T
Cc: netdev@vger.kernel.org, kernel-janitors@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org
In-Reply-To: <309B89C4C689E141A5FF6A0C5FB2118B8C6A028B@ORSMSX101.amr.corp.intel.com>
On 9/13/17 7:24 PM, Brown, Aaron F wrote:
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
>> Of Christophe JAILLET
>> Sent: Monday, August 28, 2017 10:13 AM
>> To: Waskiewicz Jr, Peter <peter.waskiewicz.jr@intel.com>; Kirsher, Jeffrey T
>> <jeffrey.t.kirsher@intel.com>
>> Cc: netdev@vger.kernel.org; kernel-janitors@vger.kernel.org; intel-wired-
>> lan@lists.osuosl.org; linux-kernel@vger.kernel.org
>> Subject: Re: [Intel-wired-lan] [PATCH] igb: check memory allocation failure
>>
>> Le 28/08/2017 à 01:09, Waskiewicz Jr, Peter a écrit :
>>> On 8/27/17 2:42 AM, Christophe JAILLET wrote:
>>>> Check memory allocation failures and return -ENOMEM in such cases, as
>>>> already done for other memory allocations in this function.
>>>>
>>>> This avoids NULL pointers dereference.
>>>>
>>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>>>> ---
>>>> drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>
> This seems to be fine from a "it does not break in testing" perspective, so...
>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com
>
>>> -PJ
>>>
>> Hi,
>>
>> in fact, there is no leak because the only caller of 'igb_sw_init()'
>> (i.e. 'igb_probe()'), already frees these resources in case of error,
>> see [1]
>>
>> These resources are also freed in 'igb_remove()'.
>>
>> Best reagrds,
>> CJ
>>
>> [1]:
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
>> next.git/tree/drivers/net/ethernet/intel/igb/igb_main.c#n2775
>
> But is PJ's comment saying that it is not really necessary? If so I tend to lean towards the don't touch it if it's not broken perspective.
I guess I didn't respond after Christophe replied, sorry about that.
The patch is good to me. It's definitely catching an issue where we're
not checking for a memory failure, then just follows the same
de-allocation path on unwind.
If you want it:
Acked-by: PJ Waskiewicz <peter.waskiewicz.jr@intel.com>
^ permalink raw reply
* Re: [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: Willem de Bruijn @ 2017-09-14 14:35 UTC (permalink / raw)
To: nixiaoming
Cc: David Miller, Eric Dumazet, waltje, gw4pts, Andrey Konovalov,
Tobias Klauser, Philip Pettersson, Alexander Potapenko,
Network Development, LKML, dede.wu
In-Reply-To: <20170914140722.44292-1-nixiaoming@huawei.com>
On Thu, Sep 14, 2017 at 10:07 AM, nixiaoming <nixiaoming@huawei.com> wrote:
> From: l00219569 <lisimin@huawei.com>
>
> If fanout_add is preempted after running po-> fanout = match
> and before running __fanout_link,
> it will cause BUG_ON when __unregister_prot_hook call __fanout_unlink
>
> so, we need add mutex_lock(&fanout_mutex) to __unregister_prot_hook
The packet socket code has no shortage of locks, so there are many
ways to avoid the race condition between fanout_add and packet_set_ring.
Another option would be to lock the socket when calling fanout_add:
- return fanout_add(sk, val & 0xffff, val >> 16);
+ lock_sock(sk);
+ ret = fanout_add(sk, val & 0xffff, val >> 16);
+ release_sock(sk);
+ return ret;
But, for consistency, and to be able to continue to make sense of the
locking policy, we should use the most appropriate lock. This
is po->bind_lock, as it ensures atomicity between testing whether
a protocol hook is active through po->running and the actual existence
of that hook on the protocol hook list.
fanout_mutex protects the fanout object's list. Taking that on
__unregister_prot_hook even in the case where fanout is not
used (and __dev_remove_pack is called) complicates locking
in this already complicated code.
> or add spin_lock(&po->bind_lock) before po-> fanout = match
>
> this is a patch for add po->bind_lock in fanout_add
>
> test on linux 4.1.12:
> ./trinity -c setsockopt -C 2 -X &
Thanks for testing!
>
> BUG: failure at net/packet/af_packet.c:1414/__fanout_unlink()!
> Kernel panic - not syncing: BUG!
> CPU: 2 PID: 2271 Comm: trinity-c0 Tainted: G W O 4.1.12 #1
> Hardware name: Hisilicon PhosphorHi1382 FPGA (DT)
> Call trace:
> [<ffffffc000209414>] dump_backtrace+0x0/0xf8
> [<ffffffc00020952c>] show_stack+0x20/0x28
> [<ffffffc000635574>] dump_stack+0xac/0xe4
> [<ffffffc000633fb8>] panic+0xf8/0x268
> [<ffffffc0005fa778>] __unregister_prot_hook+0xa0/0x144
> [<ffffffc0005fba48>] packet_set_ring+0x280/0x5b4
> [<ffffffc0005fc33c>] packet_setsockopt+0x320/0x950
> [<ffffffc000554a04>] SyS_setsockopt+0xa4/0xd4
>
> Signed-off-by: nixiaoming <nixiaoming@huawei.com>
> Tested-by: wudesheng <dede.wu@huawei.com>
> ---
> net/packet/af_packet.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 54a18a8..7a52a3b 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1446,12 +1446,16 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
> default:
> return -EINVAL;
> }
> -
> - if (!po->running)
> + spin_lock(&po->bind_lock);
> + if (!po->running) {
> + spin_unlock(&po->bind_lock);
> return -EINVAL;
> + }
>
> - if (po->fanout)
> + if (po->fanout) {
> + spin_unlock(&po->bind_lock);
> return -EALREADY;
> + }
>
> mutex_lock(&fanout_mutex);
> match = NULL;
> @@ -1501,6 +1505,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
> }
> out:
> mutex_unlock(&fanout_mutex);
> + spin_unlock(&po->bind_lock);
This function can call kzalloc with GFP_KERNEL, which may sleep. It is
not correct to sleep while holding a spinlock. Which is why I take the lock
later and test po->running again.
I will clean up that patch and send it for review.
^ 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