Netdev List
 help / color / mirror / Atom feed
* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Josef Bacik @ 2017-09-13 19:44 UTC (permalink / raw)
  To: Cole Robinson, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <a4807126-1f17-20bd-dace-d9c6b6c4787b@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

Alright thanks, this should fix it.

Josef

On 9/13/17, 12:14 PM, "Cole Robinson" <crobinso@redhat.com> wrote:

On 09/13/2017 01:40 PM, Cole Robinson wrote:
> On 09/13/2017 01:28 PM, Josef Bacik wrote:
>> Sorry I thought I had made this other fix, can you apply this on top of the other one and try that?  I have more things to try if this doesn’t work, sorry you are playing go between, but I want to make sure I know _which_ fix actually fixes the problem, and then clean up in followup patches.  Thanks,
>>
> 
> I'm the bug reporter. I'll combine the two patches and report back
> 

Nope, issue is still present with both patches applied. Tried my own build and
a package Laura provided

Thanks,
Cole




[-- Attachment #2: 0001-net-don-t-fast-patch-mismatched-sockets-in-STRICT-mo.patch --]
[-- Type: application/octet-stream, Size: 1182 bytes --]

From 8d3da6d30a7c63034d4ff7d4fc33ea9e3f23cf41 Mon Sep 17 00:00:00 2001
From: Josef Bacik <josef@toxicpanda.com>
Date: Wed, 13 Sep 2017 12:42:13 -0700
Subject: [PATCH] net: don't fast patch mismatched sockets in STRICT mode

With FASTREUSE_STRICT we may have sockets on the tb that don't match our
fast_sk information, so if our new socket don't strictly match the
fast_sk info we need to not go down the fast path and do the
inet_csk_bind_conflict so we can look at the other sockets on the tb.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 net/ipv4/inet_connection_sock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index ff8b15a99e42..fe9cf4862de2 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -263,6 +263,8 @@ static inline int sk_reuseport_match(struct inet_bind_bucket *tb,
 	 */
 	if (tb->fastreuseport == FASTREUSEPORT_ANY)
 		return 1;
+	if (tb->fast_ipv6_only && tb->fast_sk_family != sk->sk_family)
+		return 0;
 #if IS_ENABLED(CONFIG_IPV6)
 	if (tb->fast_sk_family == AF_INET6)
 		return ipv6_rcv_saddr_equal(&tb->fast_v6_rcv_saddr,
-- 
2.13.5


^ permalink raw reply related

* Re: RFC: Audit Kernel Container IDs
From: Carlos O'Donell @ 2017-09-13 19:33 UTC (permalink / raw)
  To: Richard Guy Briggs, cgroups, Linux Containers, Linux API,
	Linux Audit, Linux FS Devel, Linux Kernel,
	Linux Network Development
  Cc: Aristeu Rozanski, David Howells, Eric W. Biederman, Eric Paris,
	jlayton, Andy Lutomirski, mszeredi, Paul Moore, Serge E. Hallyn,
	Steve Grubb, trondmy, Al Viro
In-Reply-To: <20170913171328.GP3405@madcap2.tricolour.ca>

On 09/13/2017 12:13 PM, Richard Guy Briggs wrote:
> Containers are a userspace concept.  The kernel knows nothing of them.

I am looking at this RFC from a userspace perspective, particularly from
the loader's point of view and the unshare syscall and the semantics that
arise from the use of it.

At a high level what you are doing is providing a way to group, without
hierarchy, processes and namespaces. The processes can move between
container's if they have CAP_CONTAINER_ADMIN and can open and write to
a special proc file.

* With unshare a thread may dissociate part of its execution context and
  therefore see a distinct mount namespace. When you say "process" in this
  particular RFC do you exclude the fact that a thread might be in a
  distinct container from the rest of the threads in the process?

> The Linux audit system needs a way to be able to track the container
> provenance of events and actions.  Audit needs the kernel's help to do
> this.

* Why does the Linux audit system need to tracker container provenance?

  - How does it help to provide better audit messages?

  - Is it be enough to list the namespace that a process occupies?

* Why does it need the kernel's help?

  - Is there a race condition that is only fixable with kernel support?

  - Or is it easier with kernel help but not required?

Providing background on these questions would help clarify the
design requirements.

> Since the concept of a container is entirely a userspace concept, a
> trigger signal from the userspace container orchestration system
> initiates this.  This will define a point in time and a set of resources
> associated with a particular container with an audit container ID.

Please don't use the word 'signal', I suggest 'register' since you are
writing to a filesystem.

> 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.
> A bind mount of nsfs may be necessary in the container orchestrator's
> mntNS.
> 
> Require a new CAP_CONTAINER_ADMIN to be able to write to the pseudo
> filesystem to have this action permitted.  At that time, record the
> child container's user-supplied 64-bit container identifier along with

What is a "child container?" Containers don't have any hierarchy.

I assume that if you don't have CAP_CONTAINER_ADMIN, that nothing prevents
your continued operation as we have today?

> the child container's first process (which may become the container's
> "init" process) process ID (referenced from the initial PID namespace),
> all namespace IDs (in the form of a nsfs device number and inode number
> tuple) in a new auxilliary record AUDIT_CONTAINER with a qualifying
> op=$action field.

What kind of requirement is there on the first tid/pid registering
the container ID? What if the 8th tid/pid does the registration?
Would that mean that the first process of the container did not
register? It seems like you are suggesting that the registration
by the 8th tid/pid causes a cascading registration progress,
registering all tid/pids in the same grouping? Is that true?

> Issue a new auxilliary record AUDIT_CONTAINER_INFO for each valid
> container ID present on an auditable action or event.
> 
> Forked and cloned processes inherit their parent's container ID,
> referenced in the process' audit_context struct.

So a cloned process with CLONE_NEWNS has the came container ID
as the parent process that called clone, at least until the clone
has time to change to a new container ID?

Do you forsee any case where someone might need a semantic that is
slightly different? For example wanting to set the container ID on
clone?

> Log the creation of every namespace, inheriting/adding its spawning
> process' containerID(s), if applicable.  Include the spawning and
> spawned namespace IDs (device and inode number tuples).
> [AUDIT_NS_CREATE, AUDIT_NS_DESTROY] [clone(2), unshare(2), setns(2)]
> Note: At this point it appears only network namespaces may need to track
> container IDs apart from processes since incoming packets may cause an
> auditable event before being associated with a process.

OK.

> Log the destruction of every namespace when it is no longer used by any
> process, include the namespace IDs (device and inode number tuples).
> [AUDIT_NS_DESTROY] [process exit, unshare(2), setns(2)]
> 
> Issue a new auxilliary record AUDIT_NS_CHANGE listing (opt: op=$action)
> the parent and child namespace IDs for any changes to a process'
> namespaces. [setns(2)]
> Note: It may be possible to combine AUDIT_NS_* record formats and
> distinguish them with an op=$action field depending on the fields
> required for each message type.
> 
> A process can be moved from one container to another by using the
> container assignment method outlined above a second time.

OK.

> When a container ceases to exist because the last process in that
> container has exited and hence the last namespace has been destroyed and
> its refcount dropping to zero, log the fact.
> (This latter is likely needed for certification accountability.)  A
> container object may need a list of processes and/or namespaces.

OK.

> A namespace cannot directly migrate from one container to another but
> could be assigned to a newly spawned container.  A namespace can be
> moved from one container to another indirectly by having that namespace
> used in a second process in another container and then ending all the
> processes in the first container.

OK.

> Feedback please.

-- 
Cheers,
Carlos.

^ permalink raw reply

* [PATCH] MAINTAINERS: review Renesas DT bindings as well
From: Sergei Shtylyov @ 2017-09-13 19:28 UTC (permalink / raw)
  To: netdev; +Cc: linux-renesas-soc, linux-kernel, Sergei Shtylyov

[-- Attachment #1: MAINTAINERS-review-Renesas-DT-bindings-as-well.patch --]
[-- Type: text/plain, Size: 880 bytes --]

When adding myself  as a reviewer for  the Renesas  Ethernet drivers
I somehow forgot about the bindings -- I want to review them as well.

Fixes: 8e6569af3a1b ("MAINTAINERS: add myself as Renesas Ethernet drivers reviewer")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
The patch is against DaveM's 'net.git' repo.

 MAINTAINERS |    2 ++
 1 file changed, 2 insertions(+)

Index: net/MAINTAINERS
===================================================================
--- net.orig/MAINTAINERS
+++ net/MAINTAINERS
@@ -11379,6 +11379,8 @@ RENESAS ETHERNET DRIVERS
 R:	Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
 L:	netdev@vger.kernel.org
 L:	linux-renesas-soc@vger.kernel.org
+F:	Documentation/devicetree/bindings/net/renesas,*.txt
+F:	Documentation/devicetree/bindings/net/sh_eth.txt
 F:	drivers/net/ethernet/renesas/
 F:	include/linux/sh_eth.h
 

^ permalink raw reply

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Cole Robinson @ 2017-09-13 19:13 UTC (permalink / raw)
  To: Josef Bacik, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <370b38d5-346c-91df-4ad4-79886ae3a9ba@redhat.com>

On 09/13/2017 01:40 PM, Cole Robinson wrote:
> On 09/13/2017 01:28 PM, Josef Bacik wrote:
>> Sorry I thought I had made this other fix, can you apply this on top of the other one and try that?  I have more things to try if this doesn’t work, sorry you are playing go between, but I want to make sure I know _which_ fix actually fixes the problem, and then clean up in followup patches.  Thanks,
>>
> 
> I'm the bug reporter. I'll combine the two patches and report back
> 

Nope, issue is still present with both patches applied. Tried my own build and
a package Laura provided

Thanks,
Cole

^ permalink raw reply

* [PATCH] ath10k: make ath10k_hw_ce_regs const
From: Bhumika Goyal @ 2017-09-13 19:01 UTC (permalink / raw)
  To: julia.lawall, kvalo, ath10k, linux-wireless, netdev, linux-kernel
  Cc: Bhumika Goyal

Make them const as they are not modified in the file referencing
them. They are only stored in the const field 'hw_ce_reg' of an ath10k
structure. Also, make the declarations in the header const.

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/net/wireless/ath/ath10k/hw.c | 4 ++--
 drivers/net/wireless/ath/ath10k/hw.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/hw.c b/drivers/net/wireless/ath/ath10k/hw.c
index a860691..07df7c6 100644
--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c
@@ -310,7 +310,7 @@
 	.wm_high	= &wcn3990_dst_wm_high,
 };
 
-struct ath10k_hw_ce_regs wcn3990_ce_regs = {
+const struct ath10k_hw_ce_regs wcn3990_ce_regs = {
 	.sr_base_addr		= 0x00000000,
 	.sr_size_addr		= 0x00000008,
 	.dr_base_addr		= 0x0000000c,
@@ -457,7 +457,7 @@ struct ath10k_hw_ce_regs wcn3990_ce_regs = {
 	.wm_high	= &qcax_dst_wm_high,
 };
 
-struct ath10k_hw_ce_regs qcax_ce_regs = {
+const struct ath10k_hw_ce_regs qcax_ce_regs = {
 	.sr_base_addr		= 0x00000000,
 	.sr_size_addr		= 0x00000004,
 	.dr_base_addr		= 0x00000008,
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 0c089f6..f80840f 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -369,8 +369,8 @@ struct ath10k_hw_values {
 extern const struct ath10k_hw_values qca9888_values;
 extern const struct ath10k_hw_values qca4019_values;
 extern const struct ath10k_hw_values wcn3990_values;
-extern struct ath10k_hw_ce_regs wcn3990_ce_regs;
-extern struct ath10k_hw_ce_regs qcax_ce_regs;
+extern const struct ath10k_hw_ce_regs wcn3990_ce_regs;
+extern const struct ath10k_hw_ce_regs qcax_ce_regs;
 
 void ath10k_hw_fill_survey_time(struct ath10k *ar, struct survey_info *survey,
 				u32 cc, u32 rcc, u32 cc_prev, u32 rcc_prev);
-- 
1.9.1

^ permalink raw reply related

* Re: scheduling while atomic from vmci_transport_recv_stream_cb in 3.16 kernels
From: Jorgen S. Hansen @ 2017-09-13 18:58 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Aditya Sarwade, Thomas Hellstrom, LKML, netdev@vger.kernel.org,
	Masik Petr, Ben Hutchings, Sasha Levin, Stable tree
In-Reply-To: <20170913151939.gf7n6rvvjtz47tz7@dhcp22.suse.cz>


> On Sep 13, 2017, at 5:19 PM, Michal Hocko <mhocko@kernel.org> wrote:
> 
> On Wed 13-09-17 15:07:26, Jorgen S. Hansen wrote:
>> 
>>> On Sep 12, 2017, at 11:08 AM, Michal Hocko <mhocko@kernel.org> wrote:
>>> 
>>> Hi,
>>> we are seeing the following splat with Debian 3.16 stable kernel
>>> 
>>> BUG: scheduling while atomic: MATLAB/26771/0x00000100
>>> Modules linked in: veeamsnap(O) hmac cbc cts nfsv4 dns_resolver rpcsec_gss_krb5 nfsd auth_rpcgss oid_registry nfs_acl nfs lockd fscache sunrpc vmw_vso$
>>> CPU: 0 PID: 26771 Comm: MATLAB Tainted: G           O  3.16.0-4-amd64 #1 Debian 3.16.7-ckt20-1+deb8u3
>>> Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 09/21/2015
>>> ffff88315c1e4c20 ffffffff8150db3f ffff88193f803dc8 ffffffff8150acdf
>>> ffffffff815103a2 0000000000012f00 ffff8819423dbfd8 0000000000012f00
>>> ffff88315c1e4c20 ffff88193f803dc8 ffff88193f803d50 ffff88193f803dc0
>>> Call Trace:
>>> <IRQ>  [<ffffffff8150db3f>] ? dump_stack+0x41/0x51
>>> [<ffffffff8150acdf>] ? __schedule_bug+0x48/0x55
>>> [<ffffffff815103a2>] ? __schedule+0x5d2/0x700
>>> [<ffffffff8150f9b9>] ? schedule_timeout+0x229/0x2a0
>>> [<ffffffff8109ba70>] ? select_task_rq_fair+0x390/0x700
>>> [<ffffffff8109f780>] ? check_preempt_wakeup+0x120/0x1d0
>>> [<ffffffff81510eb8>] ? wait_for_completion+0xa8/0x120
>>> [<ffffffff81096de0>] ? wake_up_state+0x10/0x10
>>> [<ffffffff810c3da0>] ? call_rcu_bh+0x20/0x20
>>> [<ffffffff810c180b>] ? wait_rcu_gp+0x4b/0x60
>>> [<ffffffff810c17b0>] ? ftrace_raw_output_rcu_utilization+0x40/0x40
>>> [<ffffffffa02ca6f5>] ? vmci_event_unsubscribe+0x75/0xb0 [vmw_vmci]
>>> [<ffffffffa031f5cd>] ? vmci_transport_destruct+0x1d/0xe0 [vmw_vsock_vmci_transport]
>>> [<ffffffffa03167e3>] ? vsock_sk_destruct+0x13/0x60 [vsock]
>>> [<ffffffff81409f7a>] ? __sk_free+0x1a/0x130
>>> [<ffffffffa0320218>] ? vmci_transport_recv_stream_cb+0x1e8/0x2d0 [vmw_vsock_vmci_transport]
>>> [<ffffffffa02c9cba>] ? vmci_datagram_invoke_guest_handler+0xaa/0xd0 [vmw_vmci]
>>> [<ffffffffa02cab51>] ? vmci_dispatch_dgs+0xc1/0x200 [vmw_vmci]
>>> [<ffffffff8106c294>] ? tasklet_action+0xf4/0x100
>>> [<ffffffff8106c681>] ? __do_softirq+0xf1/0x290
>>> [<ffffffff8106ca55>] ? irq_exit+0x95/0xa0
>>> [<ffffffff81516b22>] ? do_IRQ+0x52/0xe0
>>> [<ffffffff8151496d>] ? common_interrupt+0x6d/0x6d
>>> 
>>> AFAICS this has been fixed by 4ef7ea9195ea ("VSOCK: sock_put wasn't safe
>>> to call in interrupt context") but this patch hasn't been backported to
>>> stable trees. It applies cleanly on top of 3.16 stable tree but I am not
>>> familiar with the code to send the backport to the stable maintainer
>>> directly.
>>> 
>>> Could you double check that the patch below (just a blind cherry-pick)
>>> is correct and it doesn't need additional patches on top?
>> 
>> Hi,
>> 
>> The patch below has been used to fix the above issue by other distros
>> - among them Redhat for the 3.10 kernel, so it should work for 3.16 as
>> well.
> 
> Thanks for the confirmation. I do not see 4ef7ea9195ea ("VSOCK: sock_put
> wasn't safe to call in interrupt context") in 3.10 stable branch
> though.
> 
>> In addition to the patch above, there are two other patches that
>> need to be applied on top for the fix to be correct:
>> 
>> 8566b86ab9f0f45bc6f7dd422b21de9d0cf5415a "VSOCK: Fix lockdep issue."
>> 
>> and
>> 
>> 8ab18d71de8b07d2c4d6f984b718418c09ea45c5 "VSOCK: Detach QP check should filter out non matching QPs."
> 
> Good to know. I will send all three patches cherry-picked on top of the
> current 3.16 stable branch. Could you have a look please?

The patch series look good to me.

Thanks for taking care of this,
Jorgen

^ permalink raw reply

* Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY
From: Rob Herring @ 2017-09-13 18:20 UTC (permalink / raw)
  To: Corentin Labbe
  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: <20170908074325.GB29999@Red>

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.

> 
> > > +	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.

> > > +			#address-cells = <1>;
> > > +			#size-cells = <0>;
> > > +			int_mii_phy: ethernet-phy@1 {
> > > +				reg = <1>;
> > > +				clocks = <&ccu CLK_BUS_EPHY>;
> > > +				resets = <&ccu RST_BUS_EPHY>;
> > > +				phy-is-integrated

Missing ;

> > > +			};
> > > +		};
> > 
> > ... And in your example it's a child of the mdio mux?
> > 
> 
> So I confirm, integrated PHY must be a child of integrated MDIO (that must be a child of mdio-mux).
> The example is good.
> 
> > > +		ext_mdio: mdio@0 {
> > > +			reg = <1>;

Another unit address mismatch.

Rob
--
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

* [PATCH net] net_sched: gen_estimator: fix scaling error in bytes/packets samples
From: Eric Dumazet @ 2017-09-13 18:16 UTC (permalink / raw)
  To: Denys Fedoryshchenko
  Cc: David Miller, netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko
In-Reply-To: <1505320949.15310.173.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <edumazet@google.com>

Denys reported wrong rate estimations with HTB classes.

It appears the bug was added in linux-4.10, since my tests
where using intervals of one second only.

HTB using 4 sec default rate estimators, reported rates
were 4x higher.

We need to properly scale the bytes/packets samples before
integrating them in EWMA.

Tested:
 echo 1 >/sys/module/sch_htb/parameters/htb_rate_est

 Setup HTB with one class with a rate/cail of 5Gbit

 Generate traffic on this class

 tc -s -d cl sh dev eth0 classid 7002:11 
class htb 7002:11 parent 7002:1 prio 5 quantum 200000 rate 5Gbit ceil
5Gbit linklayer ethernet burst 80000b/1 mpu 0b cburst 80000b/1 mpu 0b
level 0 rate_handle 1 
 Sent 1488215421648 bytes 982969243 pkt (dropped 0, overlimits 0
requeues 0) 
 rate 5Gbit 412814pps backlog 136260b 2p requeues 0 
 TCP pkts/rtx 982969327/45 bytes 1488215557414/68130
 lended: 22732826 borrowed: 0 giants: 0
 tokens: -1684 ctokens: -1684
 
Fixes: 1c0d32fde5bd ("net_sched: gen_estimator: complete rewrite of rate estimators")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Denys Fedoryshchenko <nuclearcat@nuclearcat.com>
---
 net/core/gen_estimator.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
index 0385dece1f6fe5e26df1ce5f40956a79a2eebbf4..7c1ffd6f950172c1915d8e5fa2b5e3f77e4f4c78 100644
--- a/net/core/gen_estimator.c
+++ b/net/core/gen_estimator.c
@@ -83,10 +83,10 @@ static void est_timer(unsigned long arg)
 	u64 rate, brate;
 
 	est_fetch_counters(est, &b);
-	brate = (b.bytes - est->last_bytes) << (8 - est->ewma_log);
+	brate = (b.bytes - est->last_bytes) << (10 - est->ewma_log - est->intvl_log);
 	brate -= (est->avbps >> est->ewma_log);
 
-	rate = (u64)(b.packets - est->last_packets) << (8 - est->ewma_log);
+	rate = (u64)(b.packets - est->last_packets) << (10 - est->ewma_log - est->intvl_log);
 	rate -= (est->avpps >> est->ewma_log);
 
 	write_seqcount_begin(&est->seq);

^ permalink raw reply related

* Re: [PATCH v5 02/10] dt-bindings: net: Restore sun8i dwmac binding
From: Rob Herring @ 2017-09-13 18:07 UTC (permalink / raw)
  To: Corentin Labbe
  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: <20170908071156.5115-3-clabbe.montjoie@gmail.com>

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?

> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>  .../devicetree/bindings/net/dwmac-sun8i.txt        | 84 ++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/dwmac-sun8i.txt

Otherwise,

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: HTB going crazy over ~5Gbit/s (4.12.9, but problem present in older kernels as well)
From: Eric Dumazet @ 2017-09-13 18:03 UTC (permalink / raw)
  To: Denys Fedoryshchenko; +Cc: Linux Kernel Network Developers, netdev-owner
In-Reply-To: <1505325534.15310.182.camel@edumazet-glaptop3.roam.corp.google.com>

On Wed, 2017-09-13 at 10:58 -0700, Eric Dumazet wrote:
> On Wed, 2017-09-13 at 20:35 +0300, Denys Fedoryshchenko wrote:
> 
> > Overlimits never appear in HTB as i know, here is simulation on this 
> > class that have constant "at least" 1G traffic, i throttled it to 1Kbit 
> > to simulate forced drops:
> > 
> > shapernew ~ # sh /etc/shaper.cfg;sleep 1;tc -s -d class show dev 
> > eth3.777 classid 1:111;tc qdisc del dev eth3.777 root
> > class htb 1:111 parent 1:1 leaf 111: prio 0 quantum 50000 rate 1Kbit 
> > ceil 1Kbit linklayer ethernet burst 31280b/1 mpu 0b cburst 31280b/1 mpu 
> > 0b level 0
> >   Sent 134350019 bytes 117520 pkt (dropped 7819, overlimits 0 requeues 0)
> >   backlog 7902126b 4976p requeues 0
> >   lended: 86694 borrowed: 0 giants: 0
> >   tokens: -937500000 ctokens: -937500000
> > 
> 
> Oh right, I am using a local patch for this. Time to upstream :/

Please try :

diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 7e148376ba528efabe5a53a09653f9161c264be7..c6d7ae81b41f4e277afb93a3003fefcd3f27de35 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -142,6 +142,7 @@ struct htb_class {
        struct rb_node          node[TC_HTB_NUMPRIO];   /* node for self or feed tree */
 
        unsigned int drops ____cacheline_aligned_in_smp;
+       unsigned int            overlimits;
 };
 
 struct htb_level {
@@ -533,6 +534,9 @@ htb_change_class_mode(struct htb_sched *q, struct htb_class *cl, s64 *diff)
        if (new_mode == cl->cmode)
                return;
 
+       if (new_mode == HTB_CANT_SEND)
+               cl->overlimits++;
+
        if (cl->prio_activity) {        /* not necessary: speed optimization */
                if (cl->cmode != HTB_CANT_SEND)
                        htb_deactivate_prios(q, cl);
@@ -1143,6 +1147,7 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d)
        struct htb_class *cl = (struct htb_class *)arg;
        struct gnet_stats_queue qs = {
                .drops = cl->drops,
+               .overlimits = cl->overlimits,
        };
        __u32 qlen = 0;
 

^ permalink raw reply related

* Re: HTB going crazy over ~5Gbit/s (4.12.9, but problem present in older kernels as well)
From: Eric Dumazet @ 2017-09-13 17:58 UTC (permalink / raw)
  To: Denys Fedoryshchenko; +Cc: Linux Kernel Network Developers, netdev-owner
In-Reply-To: <5a95e6550a304545fbde95360985c018@nuclearcat.com>

On Wed, 2017-09-13 at 20:35 +0300, Denys Fedoryshchenko wrote:

> Overlimits never appear in HTB as i know, here is simulation on this 
> class that have constant "at least" 1G traffic, i throttled it to 1Kbit 
> to simulate forced drops:
> 
> shapernew ~ # sh /etc/shaper.cfg;sleep 1;tc -s -d class show dev 
> eth3.777 classid 1:111;tc qdisc del dev eth3.777 root
> class htb 1:111 parent 1:1 leaf 111: prio 0 quantum 50000 rate 1Kbit 
> ceil 1Kbit linklayer ethernet burst 31280b/1 mpu 0b cburst 31280b/1 mpu 
> 0b level 0
>   Sent 134350019 bytes 117520 pkt (dropped 7819, overlimits 0 requeues 0)
>   backlog 7902126b 4976p requeues 0
>   lended: 86694 borrowed: 0 giants: 0
>   tokens: -937500000 ctokens: -937500000
> 

Oh right, I am using a local patch for this. Time to upstream :/

^ permalink raw reply

* Re: Memory leaks in conntrack
From: Cong Wang @ 2017-09-13 17:58 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpVk1X+ZV2Nzi28G0HJu+RZWJ-_=0AO+EF4HVZnqSjVQ8w@mail.gmail.com>

On Wed, Sep 13, 2017 at 9:45 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Wed, Sep 13, 2017 at 1:05 AM, Florian Westphal <fw@strlen.de> wrote:
>> Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> While testing my TC filter patches (so not related to conntrack), the
>>> following memory leaks are shown up:
>>>
>>> unreferenced object 0xffff9b19ba551228 (size 128):
>>>   comm "chronyd", pid 338, jiffies 4294910829 (age 53.188s)
>>>   hex dump (first 32 bytes):
>>>     6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
>>>     00 00 00 00 18 00 00 30 00 00 00 00 00 00 00 00  .......0........
>>>   backtrace:
>>>     [<ffffffff9f1e1175>] create_object+0x169/0x2aa
>>>     [<ffffffff9fb77fb2>] kmemleak_alloc+0x25/0x41
>>>     [<ffffffff9f1c47ed>] slab_post_alloc_hook+0x44/0x65
>>>     [<ffffffff9f1ca2db>] __kmalloc_track_caller+0x113/0x146
>>>     [<ffffffff9f193c3b>] __krealloc+0x4a/0x69
>>>     [<ffffffff9f948dbd>] nf_ct_ext_add+0xe1/0x145
>>>     [<ffffffff9f942395>] init_conntrack+0x1f7/0x36e
>>>     [<ffffffff9f942762>] nf_conntrack_in+0x1d3/0x326
>>>     [<ffffffff9fa1ea69>] ipv4_conntrack_local+0x4d/0x50
>>>     [<ffffffff9f93ad70>] nf_hook_slow+0x3c/0x9b
>>>     [<ffffffff9f9c7999>] nf_hook.constprop.40+0xbe/0xd8
>>>     [<ffffffff9f9c7ba2>] __ip_local_out+0xb3/0xbf
>>>     [<ffffffff9f9c7bca>] ip_local_out+0x1c/0x36
>>>     [<ffffffff9f9c9216>] ip_send_skb+0x19/0x3d
>>>     [<ffffffff9f9ee3de>] udp_send_skb+0x17e/0x1df
>>>     [<ffffffff9f9eea37>] udp_sendmsg+0x5a2/0x77c
>>> unreferenced object 0xffff9b19a69b3340 (size 336):
>>>   comm "chronyd", pid 338, jiffies 4294910868 (age 53.032s)
>>>   hex dump (first 32 bytes):
>>>     01 00 00 00 5a 5a 5a 5a 00 00 00 00 ad 4e ad de  ....ZZZZ.....N..
>>>     ff ff ff ff 5a 5a 5a 5a ff ff ff ff ff ff ff ff  ....ZZZZ........
>>>   backtrace:
>>>     [<ffffffff9f1e1175>] create_object+0x169/0x2aa
>>>     [<ffffffff9fb77fb2>] kmemleak_alloc+0x25/0x41
>>>     [<ffffffff9f1c47ed>] slab_post_alloc_hook+0x44/0x65
>>>     [<ffffffff9f1c7a7d>] kmem_cache_alloc+0xd7/0x1f1
>>>     [<ffffffff9f941b78>] __nf_conntrack_alloc+0xa2/0x146
>>>     [<ffffffff9f942250>] init_conntrack+0xb2/0x36e
>>>     [<ffffffff9f942762>] nf_conntrack_in+0x1d3/0x326
>>>     [<ffffffff9fa1ea69>] ipv4_conntrack_local+0x4d/0x50
>>>     [<ffffffff9f93ad70>] nf_hook_slow+0x3c/0x9b
>>>     [<ffffffff9f9c7999>] nf_hook.constprop.40+0xbe/0xd8
>>>     [<ffffffff9f9c7ba2>] __ip_local_out+0xb3/0xbf
>>>     [<ffffffff9f9c7bca>] ip_local_out+0x1c/0x36
>>>     [<ffffffff9f9c9216>] ip_send_skb+0x19/0x3d
>>>     [<ffffffff9f9ee3de>] udp_send_skb+0x17e/0x1df
>>>     [<ffffffff9f9eea37>] udp_sendmsg+0x5a2/0x77c
>>>     [<ffffffff9f9f8cb8>] inet_sendmsg+0x37/0x5e
>>>
>>> I don't touch chronyd in my VM, so I have no idea why it sends out UDP
>>> packets, my guess is it is some periodical packet.
>>>
>>> I don't think I use conntrack either, since /proc/net/ip_conntrack
>>> does not exist.
>>
>> You probably do, can you try "cat /proc/net/nf_conntrack" instead?
>>
>> (otherwise there should be no ipv4_conntrack_local() invocation
>>  since we would not register this hook at all).
>
> Yeah it is very weird but it is true:
>
> [root@localhost ~]# echo scan > /sys/kernel/debug/kmemleak
> [  133.450823] kmemleak: 18 new suspected memory leaks (see
> /sys/kernel/debug/kmemleak)
> [root@localhost ~]# cat /proc/net/ip_conntrack
> cat: /proc/net/ip_conntrack: No such file or directory
> [root@localhost ~]# cat /sys/kernel/debug/kmemleak
> unreferenced object 0xffff95c1e0b24040 (size 336):
> ...

Oops, you mean nf_conntrack... Here we go:

[root@localhost ~]# cat /proc/net/nf_conntrack
ipv4     2 udp      17 116 src=192.168.124.6 dst=204.2.134.162
sport=123 dport=123 src=204.2.134.162 dst=192.168.124.6 sport=123
dport=123 [ASSURED] mark=0 zone=0 use=2
ipv4     2 udp      17 117 src=192.168.124.6 dst=45.79.187.10
sport=123 dport=123 src=45.79.187.10 dst=192.168.124.6 sport=123
dport=123 [ASSURED] mark=0 zone=0 use=2
ipv4     2 udp      17 110 src=192.168.124.6 dst=192.168.124.1
sport=35486 dport=53 src=192.168.124.1 dst=192.168.124.6 sport=53
dport=35486 [ASSURED] mark=0 zone=0 use=2
ipv4     2 udp      17 110 src=192.168.124.6 dst=192.168.124.1
sport=52373 dport=53 src=192.168.124.1 dst=192.168.124.6 sport=53
dport=52373 [ASSURED] mark=0 zone=0 use=2
ipv4     2 unknown  2 518 src=192.168.124.6 dst=224.0.0.22 [UNREPLIED]
src=224.0.0.22 dst=192.168.124.6 mark=0 zone=0 use=2
ipv4     2 udp      17 110 src=192.168.124.6 dst=192.168.124.1
sport=43242 dport=53 src=192.168.124.1 dst=192.168.124.6 sport=53
dport=43242 [ASSURED] mark=0 zone=0 use=2
ipv4     2 udp      17 116 src=192.168.124.6 dst=96.226.123.196
sport=123 dport=123 src=96.226.123.196 dst=192.168.124.6 sport=123
dport=123 [ASSURED] mark=0 zone=0 use=2
ipv4     2 udp      17 110 src=192.168.124.6 dst=192.168.124.1
sport=42838 dport=53 src=192.168.124.1 dst=192.168.124.6 sport=53
dport=42838 [ASSURED] mark=0 zone=0 use=2
ipv4     2 udp      17 117 src=192.168.124.6 dst=97.127.104.4
sport=123 dport=123 src=97.127.104.4 dst=192.168.124.6 sport=123
dport=123 [ASSURED] mark=0 zone=0 use=2

^ permalink raw reply

* Re: [PATCH/RFC net-next] ravb: RX checksum offload
From: Sergei Shtylyov @ 2017-09-13 17:54 UTC (permalink / raw)
  To: Simon Horman, David Miller; +Cc: Magnus Damm, netdev, linux-renesas-soc
In-Reply-To: <1505221489-12870-1-git-send-email-horms+renesas@verge.net.au>

Hello!

On 09/12/2017 04:04 PM, Simon Horman wrote:

> Add support for RX checksum offload. This is enabled by default and
> may be disabled and re-enabled using ethtool:
> 
>   # ethtool -K eth0 rx off
>   # ethtool -K eth0 rx on
> 
> The RAVB provides a simple checksumming scheme which appears to be
> completely compatible with CHECKSUM_COMPLETE: a 1's complement sum of

    Hm, the gen2/3 manuals say calculation doesn't involve bit inversion...

> all packet data after the L2 header is appended to packet data; this may
> be trivially read by the driver and used to update the skb accordingly.
 >
> In terms of performance throughput is close to gigabit line-rate both with
> and without RX checksum offload enabled. Perf output, however, appears to
> indicate that significantly less time is spent in do_csum(). This is as
> expected.

[...]

> By inspection this also appears to be compatible with the ravb found
> on R-Car Gen 2 SoCs, however, this patch is currently untested on such
> hardware.

    I probably won't be able to test it on gen2 too...

> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

    I'm generally OK with the patch but have some questions/comments below...

> ---
>   drivers/net/ethernet/renesas/ravb_main.c | 58 +++++++++++++++++++++++++++++++-
>   1 file changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index fdf30bfa403b..7c6438cd7de7 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
> @@ -1842,6 +1859,41 @@ static int ravb_do_ioctl(struct net_device *ndev, struct ifreq *req, int cmd)
>   	return phy_mii_ioctl(phydev, req, cmd);
>   }
>   
> +static void ravb_set_rx_csum(struct net_device *ndev, bool enable)
> +{
> +	struct ravb_private *priv = netdev_priv(ndev);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +
> +	/* Disable TX and RX */
> +	ravb_rcv_snd_disable(ndev);
> +
> +	/* Modify RX Checksum setting */
> +	if (enable)
> +		ravb_modify(ndev, ECMR, 0, ECMR_RCSC);

    Please use ECMR_RCSC as the 3rd argument too to conform the common driver 
style.

> +	else
> +		ravb_modify(ndev, ECMR, ECMR_RCSC, 0);

    This *if* can easily be folded into a single ravb_modify() call...

[...]
> @@ -2004,6 +2057,9 @@ static int ravb_probe(struct platform_device *pdev)
>   	if (!ndev)
>   		return -ENOMEM;
>   
> +	ndev->features |= NETIF_F_RXCSUM;
> +	ndev->hw_features |= ndev->features;

    Hum, both fields are 0 before this? Then why not use '=' instead of '|='?
Even if not, why not just use the same value as both the rvalues?

[...]

MBR, Sergei

^ permalink raw reply

* [PATCH v2] net: smsc911x: Quieten netif during suspend
From: Geert Uytterhoeven @ 2017-09-13 17:42 UTC (permalink / raw)
  To: David S . Miller, Steve Glendinning
  Cc: Andrew Lunn, Florian Fainelli, netdev, linux-pm,
	linux-renesas-soc, linux-kernel, Geert Uytterhoeven

If the network interface is kept running during suspend, the net core
may call net_device_ops.ndo_start_xmit() while the Ethernet device is
still suspended, which may lead to a system crash.

E.g. on sh73a0/kzm9g and r8a73a4/ape6evm, the external Ethernet chip is
driven by a PM controlled clock.  If the Ethernet registers are accessed
while the clock is not running, the system will crash with an imprecise
external abort.

As this is a race condition with a small time window, it is not so easy
to trigger at will.  Using pm_test may increase your chances:

    # echo 0 > /sys/module/printk/parameters/console_suspend
    # echo platform > /sys/power/pm_test
    # echo mem > /sys/power/state

To fix this, make sure the network interface is quietened during
suspend.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This is v2 of the series "[PATCH 0/2] net: Fix crashes due to activity
during suspend", which degenerated into a single patch after commit
ebc8254aeae34226 ("Revert "net: phy: Correctly process PHY_HALTED in
phy_stop_machine()"") made "[PATCH 1/2] net: phy: Freeze PHY polling before
suspending devices" no longer needed.

v2:
  - Spelling s/quit/quiet/g.

No stacktrace is provided, as the imprecise external abort is usually
reported from an innocent looking and unrelated function like
__loop_delay(), cpu_idle_poll(), or arch_timer_read_counter_long().
---
 drivers/net/ethernet/smsc/smsc911x.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index 0b6a39b003a4e188..012fb66eed8dd618 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -2595,6 +2595,11 @@ static int smsc911x_suspend(struct device *dev)
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct smsc911x_data *pdata = netdev_priv(ndev);
 
+	if (netif_running(ndev)) {
+		netif_stop_queue(ndev);
+		netif_device_detach(ndev);
+	}
+
 	/* enable wake on LAN, energy detection and the external PME
 	 * signal. */
 	smsc911x_reg_write(pdata, PMT_CTRL,
@@ -2628,7 +2633,15 @@ static int smsc911x_resume(struct device *dev)
 	while (!(smsc911x_reg_read(pdata, PMT_CTRL) & PMT_CTRL_READY_) && --to)
 		udelay(1000);
 
-	return (to == 0) ? -EIO : 0;
+	if (to == 0)
+		return -EIO;
+
+	if (netif_running(ndev)) {
+		netif_device_attach(ndev);
+		netif_start_queue(ndev);
+	}
+
+	return 0;
 }
 
 static const struct dev_pm_ops smsc911x_pm_ops = {
-- 
2.7.4

^ permalink raw reply related

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Cole Robinson @ 2017-09-13 17:40 UTC (permalink / raw)
  To: Josef Bacik, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <EF55EF02-AD40-4F06-8C07-4F9F4221BFE6@fb.com>

On 09/13/2017 01:28 PM, Josef Bacik wrote:
> Sorry I thought I had made this other fix, can you apply this on top of the other one and try that?  I have more things to try if this doesn’t work, sorry you are playing go between, but I want to make sure I know _which_ fix actually fixes the problem, and then clean up in followup patches.  Thanks,
> 

I'm the bug reporter. I'll combine the two patches and report back

Thanks,
Cole

^ permalink raw reply

* Re: HTB going crazy over ~5Gbit/s (4.12.9, but problem present in older kernels as well)
From: Denys Fedoryshchenko @ 2017-09-13 17:35 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Linux Kernel Network Developers, netdev-owner
In-Reply-To: <1505323222.15310.177.camel@edumazet-glaptop3.roam.corp.google.com>

On 2017-09-13 20:20, Eric Dumazet wrote:
> On Wed, 2017-09-13 at 20:12 +0300, Denys Fedoryshchenko wrote:
> 
>> For my case, as load increased now, i am hitting same issue (i tried 
>> to
>> play with quantum / bursts as well, didnt helped):
>> 
>> tc -s -d class show dev eth3.777 classid 1:111;sleep 5;tc -s -d class
>> show dev eth3.777 classid 1:111
>> class htb 1:111 parent 1:1 leaf 111: prio 0 quantum 50000 rate 20Gbit
>> ceil 100Gbit linklayer ethernet burst 100000b/1 mpu 0b cburst 
>> 100000b/1
>> mpu 0b level 0
>>   Sent 864151559 bytes 730566 pkt (dropped 15111, overlimits 0 
>> requeues
>> 0)
>>   backlog 73968000b 39934p requeues 0
>>   lended: 499867 borrowed: 0 giants: 0
>>   tokens: 608 ctokens: 121
>> 
> 
> You have drops (and ~40,000 packets in backlog)
> 
> 
>> class htb 1:111 parent 1:1 leaf 111: prio 0 quantum 50000 rate 20Gbit
>> ceil 100Gbit linklayer ethernet burst 100000b/1 mpu 0b cburst 
>> 100000b/1
>> mpu 0b level 0
>>   Sent 1469352160 bytes 1243649 pkt (dropped 42933, overlimits 0 
>> requeues
>> 0)
>>   backlog 82536047b 39963p requeues 0
>>   lended: 810475 borrowed: 0 giants: 0
>>   tokens: 612 ctokens: 122
>> 
>> (1469352160-864151559)/5*8
>> 968320961.60000000000000000000
>> Less than 1Gbit and it's being throttled
> 
> It is not : "overlimits 0"  means this class was not throttled.
Overlimits never appear in HTB as i know, here is simulation on this 
class that have constant "at least" 1G traffic, i throttled it to 1Kbit 
to simulate forced drops:

shapernew ~ # sh /etc/shaper.cfg;sleep 1;tc -s -d class show dev 
eth3.777 classid 1:111;tc qdisc del dev eth3.777 root
class htb 1:111 parent 1:1 leaf 111: prio 0 quantum 50000 rate 1Kbit 
ceil 1Kbit linklayer ethernet burst 31280b/1 mpu 0b cburst 31280b/1 mpu 
0b level 0
  Sent 134350019 bytes 117520 pkt (dropped 7819, overlimits 0 requeues 0)
  backlog 7902126b 4976p requeues 0
  lended: 86694 borrowed: 0 giants: 0
  tokens: -937500000 ctokens: -937500000

^ permalink raw reply

* Re: [PATCH 0/2] net: Fix crashes due to activity during suspend
From: Geert Uytterhoeven @ 2017-09-13 17:33 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrew Lunn, marc_gonzalez, Mason, Geert Uytterhoeven,
	David S . Miller, Steve Glendinning, Lukas Wunner,
	Rafael J . Wysocki, netdev@vger.kernel.org, Linux PM list,
	Linux-Renesas, linux-kernel@vger.kernel.org
In-Reply-To: <af7d0675-0cf2-1345-f137-59346e6f962c@gmail.com>

Hi Florian,

On Thu, Sep 7, 2017 at 3:09 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 08/23/2017 10:13 AM, Florian Fainelli wrote:
>> On 08/23/2017 04:45 AM, Geert Uytterhoeven wrote:
>>> On Tue, Aug 22, 2017 at 8:49 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>> On 08/22/2017 11:37 AM, Geert Uytterhoeven wrote:
>>>>> If an Ethernet device is used while the device is suspended, the system may
>>>>> crash.
>>>>>
>>>>> E.g. on sh73a0/kzm9g and r8a73a4/ape6evm, the external Ethernet chip is
>>>>> driven by a PM controlled clock.  If the Ethernet registers are accessed
>>>>> while the clock is not running, the system will crash with an imprecise
>>>>> external abort.
>>>>>
>>>>> This patch series fixes two of such crashes:
>>>>>   1. The first patch prevents the PHY polling state machine from accessing
>>>>>      PHY registers while a device is suspended,
>>>>>   2. The second patch prevents the net core from trying to transmit packets
>>>>>      when an smsc911x device is suspended.
>>>>>
>>>>> Both crashes can be reproduced on sh73a0/kzm9g and r8a73a4/ape6evm during
>>>>> s2ram (rarely), or by using pm_test (more likely to trigger):
>>>>>
>>>>>     # echo 0 > /sys/module/printk/parameters/console_suspend
>>>>>     # echo platform > /sys/power/pm_test
>>>>>     # echo mem > /sys/power/state
>>>>>
>>>>> With this series applied, my test systems survive a loop of 100 test
>>>>> suspends.
>>>>
>>>> It seems to me like part, if not the entire problem is that smsc91xx's
>>>> suspend and resume functions are way too simplistic and absolutely do
>>>> not manage the PHY during suspend/resume, the PHY state machine is not
>>>> even stopped, so of course, this will cause bus errors if you access
>>>> those registers.
>>>>
>>>> You are addressing this as part of patch 2, but this seems to me like
>>>> this is still a bit incomplete and you'd need at least phy_stop() and/or
>>>> phy_suspend() (does a power down of the PHY) and phy_start() and/or
>>>> phy_resume() calls to complete the PHY state machine shutdown during
>>>> suspend.
>>>>
>>>> Have you tried that?
>>>
>>> Unfortunately that doesn't help.
>>> In state PHY_HALTED, the PHY state machine still calls the .adjust_link()
>>> callback while the device is suspended.
>>
>> Humm that is correct yes.
>>
>>> Do you have a clue? This is too far beyond my phy-foo...
>>
>> I was initially contemplating a revert of
>> 7ad813f208533cebfcc32d3d7474dc1677d1b09a ("net: phy: Correctly process
>> PHY_HALTED in phy_stop_machine()") but this is not the root of the
>> problem. The problem really is that phy_stop() does not wait for the PHY
>> state machine to be stopped so you cannot rely on that and past the
>> function return be offered any guarantees that adjust_link is not called.
>>
>> We seem to be getting away with that in most drivers because when we see
>> phydev->link = 0, we either do nothing or actually turn of the HW block.
>>
>> How about we export phy_stop_machine() to drivers which would provide a
>> synchronization point that would ensure that no HW accesses are done
>> past this point?
>>
>> I am absolutely not clear on the implications of using a freezable
>> workqueue with respect to the PHY state machine and how devices are
>> going to wind-up being powered down or not...
>
> Geert, as you may have notice a revert of the change was sent so 4.13
> should be fine, but ultimately I would like to put the non-reverted code
> back in after we add a few safeguards:

With the revert, I no longer need "[PATCH 1/2] net: phy: Freeze PHY polling
before suspending devices".
I just did more than 50 successful suspend/resume cycles to verify that.

I still need "[PATCH 2/2] net: smsc911x: Quiten netif during suspend", so
I'll submit a v2 for that.

> - and you reported the bus errors on smsc911x when we call adjust_link
> during suspend, and due to a lack of hard synchronization so phy_stop()
> here does not give you enough guarantees to let you turn off power to
> the smsc911x block
>
> If that seems accurate then we can work on something that should be
> working again (famous last words).

Sounds accurate to me.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
From: Josef Bacik @ 2017-09-13 17:28 UTC (permalink / raw)
  To: Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Cole Robinson
In-Reply-To: <588f3795-931e-7779-4ec7-5fe7d4437927@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

Sorry I thought I had made this other fix, can you apply this on top of the other one and try that?  I have more things to try if this doesn’t work, sorry you are playing go between, but I want to make sure I know _which_ fix actually fixes the problem, and then clean up in followup patches.  Thanks,

Josef

On 9/13/17, 8:45 AM, "Laura Abbott" <labbott@redhat.com> wrote:

On 09/12/2017 04:12 PM, Josef Bacik wrote:
> First I’m super sorry for the top post, I’m at plumbers and I forgot to upload my muttrc to my new cloud instance, so I’m screwed using outlook.
> 
> I have a completely untested, uncompiled patch that I think will fix the problem, would you mind giving it a go?  Thanks,
> 
> Josef

Thanks for the quick turnaround. Unfortunately, the problem is still
reproducible according to the reporter.

Thanks,
Laura



[-- Attachment #2: 0001-net-use-inet6_rcv_saddr-to-compare-sockets.patch --]
[-- Type: application/octet-stream, Size: 1085 bytes --]

From 380968f58d543ac9ec0cb1aa11db3979f3aee69d Mon Sep 17 00:00:00 2001
From: Josef Bacik <josef@toxicpanda.com>
Date: Wed, 13 Sep 2017 10:24:22 -0700
Subject: [PATCH] net: use inet6_rcv_saddr to compare sockets

In ipv6_rcv_saddr_equal() we need to use inet6_rcv_saddr(sk) for the
ipv6 compare with the fast socket information to make sure we're doing
the proper comparisons.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 net/ipv4/inet_connection_sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 3cff95f10995..ff8b15a99e42 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -266,7 +266,7 @@ static inline int sk_reuseport_match(struct inet_bind_bucket *tb,
 #if IS_ENABLED(CONFIG_IPV6)
 	if (tb->fast_sk_family == AF_INET6)
 		return ipv6_rcv_saddr_equal(&tb->fast_v6_rcv_saddr,
-					    &sk->sk_v6_rcv_saddr,
+					    inet6_rcv_saddr(sk),
 					    tb->fast_rcv_saddr,
 					    sk->sk_rcv_saddr,
 					    tb->fast_ipv6_only,
-- 
2.13.5


^ permalink raw reply related

* Re: [PATCH net] perf/bpf: fix a clang compilation issue
From: Nick Desaulniers @ 2017-09-13 17:26 UTC (permalink / raw)
  To: David Miller; +Cc: yhs, peterz, rostedt, ast, daniel, netdev, kernel-team
In-Reply-To: <20170911.142857.1205848546110018404.davem@davemloft.net>

great, thanks!

On Mon, Sep 11, 2017 at 2:28 PM, David Miller <davem@davemloft.net> wrote:
> From: Yonghong Song <yhs@fb.com>
> Date: Thu, 7 Sep 2017 18:36:15 -0700
>
>> clang does not support variable length array for structure member.
>> It has the following error during compilation:
>>
>> kernel/trace/trace_syscalls.c:568:17: error: fields must have a constant size:
>> 'variable length array in structure' extension will never be supported
>>                 unsigned long args[sys_data->nb_args];
>>                               ^
>>
>> The fix is to use a fixed array length instead.
>>
>> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>
> Applied.



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* Re: HTB going crazy over ~5Gbit/s (4.12.9, but problem present in older kernels as well)
From: Eric Dumazet @ 2017-09-13 17:20 UTC (permalink / raw)
  To: Denys Fedoryshchenko; +Cc: Linux Kernel Network Developers, netdev-owner
In-Reply-To: <57baf3c3ea2544ed4d53967b7a2d0e36@nuclearcat.com>

On Wed, 2017-09-13 at 20:12 +0300, Denys Fedoryshchenko wrote:

> For my case, as load increased now, i am hitting same issue (i tried to 
> play with quantum / bursts as well, didnt helped):
> 
> tc -s -d class show dev eth3.777 classid 1:111;sleep 5;tc -s -d class 
> show dev eth3.777 classid 1:111
> class htb 1:111 parent 1:1 leaf 111: prio 0 quantum 50000 rate 20Gbit 
> ceil 100Gbit linklayer ethernet burst 100000b/1 mpu 0b cburst 100000b/1 
> mpu 0b level 0
>   Sent 864151559 bytes 730566 pkt (dropped 15111, overlimits 0 requeues 
> 0)
>   backlog 73968000b 39934p requeues 0
>   lended: 499867 borrowed: 0 giants: 0
>   tokens: 608 ctokens: 121
> 

You have drops (and ~40,000 packets in backlog)


> class htb 1:111 parent 1:1 leaf 111: prio 0 quantum 50000 rate 20Gbit 
> ceil 100Gbit linklayer ethernet burst 100000b/1 mpu 0b cburst 100000b/1 
> mpu 0b level 0
>   Sent 1469352160 bytes 1243649 pkt (dropped 42933, overlimits 0 requeues 
> 0)
>   backlog 82536047b 39963p requeues 0
>   lended: 810475 borrowed: 0 giants: 0
>   tokens: 612 ctokens: 122
> 
> (1469352160-864151559)/5*8
> 968320961.60000000000000000000
> Less than 1Gbit and it's being throttled

It is not : "overlimits 0"  means this class was not throttled.

^ permalink raw reply

* [PATCH net v2 3/3] nfp: wait for the NSP resource to appear on boot
From: Jakub Kicinski @ 2017-09-13 17:16 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Jakub Kicinski
In-Reply-To: <20170913171600.31049-1-jakub.kicinski@netronome.com>

The control process (NSP) may take some time to complete its
initialization.  This is not a problem on most servers, but
on very fast-booting machines it may not be ready for operation
when driver probes the device.  There is also a version of the
flash in the wild where NSP tries to train the links as part
of init.  To wait for NSP initialization we should make sure
its resource has already been added to the resource table.
NSP adds itself there as last step of init.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_main.c      |  4 ++
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h   |  2 +
 .../ethernet/netronome/nfp/nfpcore/nfp_resource.c  | 45 ++++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index 424707d41fbd..f8fa63b66739 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -351,6 +351,10 @@ static int nfp_nsp_init(struct pci_dev *pdev, struct nfp_pf *pf)
 	struct nfp_nsp *nsp;
 	int err;
 
+	err = nfp_resource_wait(pf->cpp, NFP_RESOURCE_NSP, 30);
+	if (err)
+		return err;
+
 	nsp = nfp_nsp_open(pf->cpp);
 	if (IS_ERR(nsp)) {
 		err = PTR_ERR(nsp);
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
index 1a8d04a1e113..3ce51f03126f 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
@@ -97,6 +97,8 @@ nfp_resource_acquire(struct nfp_cpp *cpp, const char *name);
 
 void nfp_resource_release(struct nfp_resource *res);
 
+int nfp_resource_wait(struct nfp_cpp *cpp, const char *name, unsigned int secs);
+
 u32 nfp_resource_cpp_id(struct nfp_resource *res);
 
 const char *nfp_resource_name(struct nfp_resource *res);
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c
index 072612263dab..b1dd13ff282b 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_resource.c
@@ -249,6 +249,51 @@ void nfp_resource_release(struct nfp_resource *res)
 	kfree(res);
 }
 
+/**
+ * nfp_resource_wait() - Wait for resource to appear
+ * @cpp:	NFP CPP handle
+ * @name:	Name of the resource
+ * @secs:	Number of seconds to wait
+ *
+ * Wait for resource to appear in the resource table, grab and release
+ * its lock.  The wait is jiffies-based, don't expect fine granularity.
+ *
+ * Return: 0 on success, errno otherwise.
+ */
+int nfp_resource_wait(struct nfp_cpp *cpp, const char *name, unsigned int secs)
+{
+	unsigned long warn_at = jiffies + NFP_MUTEX_WAIT_FIRST_WARN * HZ;
+	unsigned long err_at = jiffies + secs * HZ;
+	struct nfp_resource *res;
+
+	while (true) {
+		res = nfp_resource_acquire(cpp, name);
+		if (!IS_ERR(res)) {
+			nfp_resource_release(res);
+			return 0;
+		}
+
+		if (PTR_ERR(res) != -ENOENT) {
+			nfp_err(cpp, "error waiting for resource %s: %ld\n",
+				name, PTR_ERR(res));
+			return PTR_ERR(res);
+		}
+		if (time_is_before_eq_jiffies(err_at)) {
+			nfp_err(cpp, "timeout waiting for resource %s\n", name);
+			return -ETIMEDOUT;
+		}
+		if (time_is_before_eq_jiffies(warn_at)) {
+			warn_at = jiffies + NFP_MUTEX_WAIT_NEXT_WARN * HZ;
+			nfp_info(cpp, "waiting for NFP resource %s\n", name);
+		}
+		if (msleep_interruptible(10)) {
+			nfp_err(cpp, "wait for resource %s interrupted\n",
+				name);
+			return -ERESTARTSYS;
+		}
+	}
+}
+
 /**
  * nfp_resource_cpp_id() - Return the cpp_id of a resource handle
  * @res:	NFP Resource handle
-- 
2.14.1

^ permalink raw reply related

* [PATCH net v2 2/3] nfp: wait for board state before talking to the NSP
From: Jakub Kicinski @ 2017-09-13 17:15 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Jakub Kicinski
In-Reply-To: <20170913171600.31049-1-jakub.kicinski@netronome.com>

Board state informs us which low-level initialization stages the card
has completed.  We should wait for the card to be fully initialized
before trying to communicate with it, not only before we configure
passing traffic.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_main.c     | 43 +++++++++++++++++++++++
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c | 23 ------------
 2 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index f055b1774d65..424707d41fbd 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -74,6 +74,45 @@ static const struct pci_device_id nfp_pci_device_ids[] = {
 };
 MODULE_DEVICE_TABLE(pci, nfp_pci_device_ids);
 
+static bool nfp_board_ready(struct nfp_pf *pf)
+{
+	const char *cp;
+	long state;
+	int err;
+
+	cp = nfp_hwinfo_lookup(pf->hwinfo, "board.state");
+	if (!cp)
+		return false;
+
+	err = kstrtol(cp, 0, &state);
+	if (err < 0)
+		return false;
+
+	return state == 15;
+}
+
+static int nfp_pf_board_state_wait(struct nfp_pf *pf)
+{
+	const unsigned long wait_until = jiffies + 10 * HZ;
+
+	while (!nfp_board_ready(pf)) {
+		if (time_is_before_eq_jiffies(wait_until)) {
+			nfp_err(pf->cpp, "NFP board initialization timeout\n");
+			return -EINVAL;
+		}
+
+		nfp_info(pf->cpp, "waiting for board initialization\n");
+		if (msleep_interruptible(500))
+			return -ERESTARTSYS;
+
+		/* Refresh cached information */
+		kfree(pf->hwinfo);
+		pf->hwinfo = nfp_hwinfo_read(pf->cpp);
+	}
+
+	return 0;
+}
+
 static int nfp_pcie_sriov_read_nfd_limit(struct nfp_pf *pf)
 {
 	int err;
@@ -425,6 +464,10 @@ static int nfp_pci_probe(struct pci_dev *pdev,
 		 nfp_hwinfo_lookup(pf->hwinfo, "assembly.revision"),
 		 nfp_hwinfo_lookup(pf->hwinfo, "cpld.version"));
 
+	err = nfp_pf_board_state_wait(pf);
+	if (err)
+		goto err_hwinfo_free;
+
 	err = devlink_register(devlink, &pdev->dev);
 	if (err)
 		goto err_hwinfo_free;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index 5abb9ba31e7d..ff373acd28f3 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -64,23 +64,6 @@
 
 #define NFP_PF_CSR_SLICE_SIZE	(32 * 1024)
 
-static int nfp_is_ready(struct nfp_pf *pf)
-{
-	const char *cp;
-	long state;
-	int err;
-
-	cp = nfp_hwinfo_lookup(pf->hwinfo, "board.state");
-	if (!cp)
-		return 0;
-
-	err = kstrtol(cp, 0, &state);
-	if (err < 0)
-		return 0;
-
-	return state == 15;
-}
-
 /**
  * nfp_net_get_mac_addr() - Get the MAC address.
  * @pf:       NFP PF handle
@@ -725,12 +708,6 @@ int nfp_net_pci_probe(struct nfp_pf *pf)
 
 	INIT_WORK(&pf->port_refresh_work, nfp_net_refresh_vnics);
 
-	/* Verify that the board has completed initialization */
-	if (!nfp_is_ready(pf)) {
-		nfp_err(pf->cpp, "NFP is not ready for NIC operation.\n");
-		return -EINVAL;
-	}
-
 	if (!pf->rtbl) {
 		nfp_err(pf->cpp, "No %s, giving up.\n",
 			pf->fw_loaded ? "symbol table" : "firmware found");
-- 
2.14.1

^ permalink raw reply related

* [PATCH net v2 1/3] nfp: add whitelist of supported flow dissector
From: Jakub Kicinski @ 2017-09-13 17:15 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Pieter Jansen van Vuuren
In-Reply-To: <20170913171600.31049-1-jakub.kicinski@netronome.com>

From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>

Previously we did not check the flow dissector against a list of allowed
and supported flow key dissectors. This patch introduces such a list and
correctly rejects unsupported flow keys.

Fixes: 43f84b72c50d ("nfp: add metadata to each flow offload")
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/flower/offload.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index d396183108f7..a18b4d2b1d3e 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -44,6 +44,16 @@
 #include "../nfp_net.h"
 #include "../nfp_port.h"
 
+#define NFP_FLOWER_WHITELIST_DISSECTOR \
+	(BIT(FLOW_DISSECTOR_KEY_CONTROL) | \
+	 BIT(FLOW_DISSECTOR_KEY_BASIC) | \
+	 BIT(FLOW_DISSECTOR_KEY_IPV4_ADDRS) | \
+	 BIT(FLOW_DISSECTOR_KEY_IPV6_ADDRS) | \
+	 BIT(FLOW_DISSECTOR_KEY_PORTS) | \
+	 BIT(FLOW_DISSECTOR_KEY_ETH_ADDRS) | \
+	 BIT(FLOW_DISSECTOR_KEY_VLAN) | \
+	 BIT(FLOW_DISSECTOR_KEY_IP))
+
 static int
 nfp_flower_xmit_flow(struct net_device *netdev,
 		     struct nfp_fl_payload *nfp_flow, u8 mtype)
@@ -112,6 +122,9 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls,
 	u8 key_layer;
 	int key_size;
 
+	if (flow->dissector->used_keys & ~NFP_FLOWER_WHITELIST_DISSECTOR)
+		return -EOPNOTSUPP;
+
 	if (dissector_uses_key(flow->dissector,
 			       FLOW_DISSECTOR_KEY_ENC_CONTROL)) {
 		struct flow_dissector_key_control *mask_enc_ctl =
-- 
2.14.1

^ permalink raw reply related

* [PATCH net v2 0/3] nfp: wait more carefully for card init
From: Jakub Kicinski @ 2017-09-13 17:15 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, Jakub Kicinski

Hi!

The first patch is a small fix for flower offload, we need a whitelist
of supported matches, otherwise the unsupported ones will be ignored.

The second and the third patch are adding wait/polling to the probe path.
We had reports of driver failing probe because it couldn't find the 
control process (NSP) on the card.  Turns out the NSP will only announce
its existence after it's fully initialized.  Until now we assumed it 
will be reachable, just not processing commands (hence we wait for
a NOOP command to execute successfully).

v2:
 - fix a bad merge which resulted in a build warning and retest.

Jakub Kicinski (2):
  nfp: wait for board state before talking to the NSP
  nfp: wait for the NSP resource to appear on boot

Pieter Jansen van Vuuren (1):
  nfp: add whitelist of supported flow dissector

 .../net/ethernet/netronome/nfp/flower/offload.c    | 13 ++++++
 drivers/net/ethernet/netronome/nfp/nfp_main.c      | 47 ++++++++++++++++++++++
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c  | 23 -----------
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h   |  2 +
 .../ethernet/netronome/nfp/nfpcore/nfp_resource.c  | 45 +++++++++++++++++++++
 5 files changed, 107 insertions(+), 23 deletions(-)

-- 
2.14.1

^ permalink raw reply

* RFC: Audit Kernel Container IDs
From: Richard Guy Briggs @ 2017-09-13 17:13 UTC (permalink / raw)
  To: cgroups-u79uwXL29TY76Z2rM5mHXA, Linux Containers, Linux API,
	Linux Audit, Linux FS Devel, Linux Kernel,
	Linux Network Development
  Cc: Aristeu Rozanski, David Howells, Eric W. Biederman, 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

Containers are a userspace concept.  The kernel knows nothing of them.

The Linux audit system needs a way to be able to track the container
provenance of events and actions.  Audit needs the kernel's help to do
this.

Since the concept of a container is entirely a userspace concept, a
trigger signal from the userspace container orchestration system
initiates this.  This will define a point in time and a set of resources
associated with a particular container with an audit container ID.

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.
A bind mount of nsfs may be necessary in the container orchestrator's
mntNS.

Require a new CAP_CONTAINER_ADMIN to be able to write to the pseudo
filesystem to have this action permitted.  At that time, record the
child container's user-supplied 64-bit container identifier along with
the child container's first process (which may become the container's
"init" process) process ID (referenced from the initial PID namespace),
all namespace IDs (in the form of a nsfs device number and inode number
tuple) in a new auxilliary record AUDIT_CONTAINER with a qualifying
op=$action field.

Issue a new auxilliary record AUDIT_CONTAINER_INFO for each valid
container ID present on an auditable action or event.

Forked and cloned processes inherit their parent's container ID,
referenced in the process' audit_context struct.

Log the creation of every namespace, inheriting/adding its spawning
process' containerID(s), if applicable.  Include the spawning and
spawned namespace IDs (device and inode number tuples).
[AUDIT_NS_CREATE, AUDIT_NS_DESTROY] [clone(2), unshare(2), setns(2)]
Note: At this point it appears only network namespaces may need to track
container IDs apart from processes since incoming packets may cause an
auditable event before being associated with a process.

Log the destruction of every namespace when it is no longer used by any
process, include the namespace IDs (device and inode number tuples).
[AUDIT_NS_DESTROY] [process exit, unshare(2), setns(2)]

Issue a new auxilliary record AUDIT_NS_CHANGE listing (opt: op=$action)
the parent and child namespace IDs for any changes to a process'
namespaces. [setns(2)]
Note: It may be possible to combine AUDIT_NS_* record formats and
distinguish them with an op=$action field depending on the fields
required for each message type.

A process can be moved from one container to another by using the
container assignment method outlined above a second time.

When a container ceases to exist because the last process in that
container has exited and hence the last namespace has been destroyed and
its refcount dropping to zero, log the fact.
(This latter is likely needed for certification accountability.)  A
container object may need a list of processes and/or namespaces.

A namespace cannot directly migrate from one container to another but
could be assigned to a newly spawned container.  A namespace can be
moved from one container to another indirectly by having that namespace
used in a second process in another container and then ending all the
processes in the first container.

Feedback please.

- 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


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