Netdev List
 help / color / mirror / Atom feed
* Job Offer In USA
From: Valero Energy Recruitment @ 2017-09-15  8:12 UTC (permalink / raw)
  To: Recipients

Valero Energy Oil & Gas Company USA
Employment Opportunity in Texas, USA.
Oil & Gas Sector ( NEW PLANT RIG )
Salary Range $7,000 USD - $11,700 Per Month
Kindly send your CV/RESUME for more details regarding our Job Program, Employment offers for all Offices.

CC: Forward All Documents/Certificates & Resume to: jobs.valerousa@gmail.com

^ permalink raw reply

* Re: [PATCH 0/3] Fix y2038 issues for security/keys subsystem
From: Baolin Wang @ 2017-09-15  8:38 UTC (permalink / raw)
  To: David Howells
  Cc: David Miller, James Morris, Serge E. Hallyn, marc.dionne,
	Dan Carpenter, Jason A. Donenfeld, Arnd Bergmann, Mark Brown,
	keyrings, LKML, linux-security-module, Networking
In-Reply-To: <CAMz4kuL-mriUk3YV6Mz9xgC7geNe5a05vhjfXaxt58x=7BUo0w@mail.gmail.com>

Hi James,

On 21 August 2017 at 20:12, Baolin Wang <baolin.wang@linaro.org> wrote:
> Hi David and James,
>
> On 9 August 2017 at 16:28, David Howells <dhowells@redhat.com> wrote:
>> The rxrpc patch isn't part of the security/keys subsystem.  I'll push it
>> to the network tree.  The other two I'll push to James.
>
> Could you apply this patch serials if there are no other comments? Thanks.

Could you apply patch1 and patch2? I have seen patch3 was merged into
4.14 by David. Thanks.

-- 
Baolin.wang
Best Regards

^ permalink raw reply

* Re: [PATCH] dt-bindings: net: renesas-ravb: Add support for R8A77995 RAVB
From: Simon Horman @ 2017-09-15  8:40 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Yoshihiro Shimoda, davem, robh+dt, mark.rutland, netdev,
	linux-renesas-soc, devicetree
In-Reply-To: <de2287b5-f208-1ffd-e8a4-3779a11695f7@cogentembedded.com>

On Thu, Sep 14, 2017 at 12:44:58PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 9/14/2017 3:06 AM, Yoshihiro Shimoda wrote:
> 
> >Add a new compatible string for the R8A77995 (R-Car D3) RAVB.
> >
> >Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
>    Usually those are added after sign-off.
> 
> >Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> >---
> >  Changes from v1:
> >   - Based on r8a77970 patch.
> >    https://marc.info/?l=linux-renesas-soc&m=150524655515476
> 
>    So this is against net-next, right? The net.git repo now has a patch
> making me a reviewer for the Renesas bindings too.

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

^ permalink raw reply

* Re: Regression in throughput between kvm guests over virtual bridge
From: Jason Wang @ 2017-09-15  8:55 UTC (permalink / raw)
  To: Matthew Rosato, netdev; +Cc: davem, mst
In-Reply-To: <e42083d9-ae7b-7207-e5e6-06483bf6293e@linux.vnet.ibm.com>



On 2017年09月15日 11:36, Matthew Rosato wrote:
>> Is the issue gone if you reduce VHOST_RX_BATCH to 1? And it would be
>> also helpful to collect perf diff to see if anything interesting.
>> (Consider 4.4 shows more obvious regression, please use 4.4).
>>
> Issue still exists when I force VHOST_RX_BATCH = 1

Interesting, so this looks more like an issue of the changes in 
vhost_net instead of batch dequeuing itself. I try this on Intel but 
still can't meet it.

>
> Collected perf data, with 4.12 as the baseline, 4.13 as delta1 and
> 4.13+VHOST_RX_BATCH=1 as delta2. All guests running 4.4.  Same scenario,
> 2 uperf client guests, 2 uperf slave guests - I collected perf data
> against 1 uperf client process and 1 uperf slave process.  Here are the
> significant diffs:
>
> uperf client:
>
> 75.09%   +9.32%   +8.52%  [kernel.kallsyms]   [k] enabled_wait
>   9.04%   -4.11%   -3.79%  [kernel.kallsyms]   [k] __copy_from_user
>   2.30%   -0.79%   -0.71%  [kernel.kallsyms]   [k] arch_free_page
>   2.17%   -0.65%   -0.58%  [kernel.kallsyms]   [k] arch_alloc_page
>   0.69%   -0.25%   -0.24%  [kernel.kallsyms]   [k] get_page_from_freelist
>   0.56%   +0.08%   +0.14%  [kernel.kallsyms]   [k] virtio_ccw_kvm_notify
>   0.42%   -0.11%   -0.09%  [kernel.kallsyms]   [k] tcp_sendmsg
>   0.31%   -0.15%   -0.14%  [kernel.kallsyms]   [k] tcp_write_xmit
>
> uperf slave:
>
> 72.44%   +8.99%   +8.85%  [kernel.kallsyms]   [k] enabled_wait
>   8.99%   -3.67%   -3.51%  [kernel.kallsyms]   [k] __copy_to_user
>   2.31%   -0.71%   -0.67%  [kernel.kallsyms]   [k] arch_free_page
>   2.16%   -0.67%   -0.63%  [kernel.kallsyms]   [k] arch_alloc_page
>   0.89%   -0.14%   -0.11%  [kernel.kallsyms]   [k] virtio_ccw_kvm_notify
>   0.71%   -0.30%   -0.30%  [kernel.kallsyms]   [k] get_page_from_freelist
>   0.70%   -0.25%   -0.29%  [kernel.kallsyms]   [k] __wake_up_sync_key
>   0.61%   -0.22%   -0.22%  [kernel.kallsyms]   [k] virtqueue_add_inbuf

It looks like vhost is slowed down for some reason which leads to more 
idle time on 4.13+VHOST_RX_BATCH=1. Appreciated if you can collect the 
perf.diff on host, one for rx and one for tx.

>
>
>> May worth to try disable zerocopy or do the test form host to guest
>> instead of guest to guest to exclude the possible issue of sender.
>>
> With zerocopy disabled, still seeing the regression.  The provided perf
> #s have zerocopy enabled.
>
> I replaced 1 uperf guest and instead ran that uperf client as a host
> process, pointing at a guest.  All traffic still over the virtual
> bridge.  In this setup, it's still easy to see the regression for the
> remaining guest1<->guest2 uperf run, but the host<->guest3 run does NOT
> exhibit a reliable regression pattern.  The significant perf diffs from
> the host uperf process (baseline=4.12, delta=4.13):
>
>
> 59.96%   +5.03%  [kernel.kallsyms]           [k] enabled_wait
>   6.47%   -2.27%  [kernel.kallsyms]           [k] raw_copy_to_user
>   5.52%   -1.63%  [kernel.kallsyms]           [k] raw_copy_from_user
>   0.87%   -0.30%  [kernel.kallsyms]           [k] get_page_from_freelist
>   0.69%   +0.30%  [kernel.kallsyms]           [k] finish_task_switch
>   0.66%   -0.15%  [kernel.kallsyms]           [k] swake_up
>   0.58%   -0.00%  [vhost]                     [k] vhost_get_vq_desc
>     ...
>   0.42%   +0.50%  [kernel.kallsyms]           [k] ckc_irq_pending

Another hint to perf vhost threads.

>
> I also tried flipping the uperf stream around (a guest uperf client is
> communicating to a slave uperf process on the host) and also cannot see
> the regression pattern.  So it seems to require a guest on both ends of
> the connection.
>

Yes. Will try to get a s390 environment.

Thanks

^ permalink raw reply

* [PATCH net] l2tp: fix race condition in l2tp_tunnel_delete
From: Sabrina Dubroca @ 2017-09-15  9:08 UTC (permalink / raw)
  To: netdev; +Cc: Sabrina Dubroca, Guillaume Nault, Xin Long

The tunnel is currently removed from the list during destruction. This
can lead to a double-free of the struct sock if we try to delete the tunnel
twice fast enough.

The first delete operation does a lookup (l2tp_tunnel_get), finds the
tunnel, calls l2tp_tunnel_delete, which queues it for deletion by
l2tp_tunnel_del_work.

The second delete operation also finds the tunnel and calls
l2tp_tunnel_delete. If the workqueue has already fired and started
running l2tp_tunnel_del_work, then l2tp_tunnel_delete will queue the
same tunnel a second time, and try to free the socket again.

Add a dead flag and remove tunnel from its list earlier. Then we can
remove the check of queue_work's result that was meant to prevent that
race but doesn't.

Reproducer:

    ip l2tp add tunnel tunnel_id 3000 peer_tunnel_id 4000 local 192.168.0.2 remote 192.168.0.1 encap udp udp_sport 5000 udp_dport 6000
    ip l2tp add session name l2tp1 tunnel_id 3000 session_id 1000 peer_session_id 2000
    ip link set l2tp1 up
    ip l2tp del tunnel tunnel_id 3000
    ip l2tp del tunnel tunnel_id 3000

Fixes: f8ccac0e4493 ("l2tp: put tunnel socket release on a workqueue")
Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/l2tp/l2tp_core.c | 30 ++++++++++++++++--------------
 net/l2tp/l2tp_core.h |  4 +++-
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index ee485df73ccd..26553d3c4f8f 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1245,7 +1245,6 @@ EXPORT_SYMBOL_GPL(l2tp_xmit_skb);
 static void l2tp_tunnel_destruct(struct sock *sk)
 {
 	struct l2tp_tunnel *tunnel = l2tp_tunnel(sk);
-	struct l2tp_net *pn;
 
 	if (tunnel == NULL)
 		goto end;
@@ -1269,13 +1268,6 @@ static void l2tp_tunnel_destruct(struct sock *sk)
 	sk->sk_destruct = tunnel->old_sk_destruct;
 	sk->sk_user_data = NULL;
 
-	/* Remove the tunnel struct from the tunnel list */
-	pn = l2tp_pernet(tunnel->l2tp_net);
-	spin_lock_bh(&pn->l2tp_tunnel_list_lock);
-	list_del_rcu(&tunnel->list);
-	spin_unlock_bh(&pn->l2tp_tunnel_list_lock);
-	atomic_dec(&l2tp_tunnel_count);
-
 	l2tp_tunnel_closeall(tunnel);
 
 	tunnel->sock = NULL;
@@ -1685,14 +1677,24 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_create);
 
 /* This function is used by the netlink TUNNEL_DELETE command.
  */
-int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel)
+void l2tp_tunnel_delete(struct l2tp_tunnel *tunnel)
 {
+	struct l2tp_net *pn;
+
+	if (tunnel->dead)
+		return;
+
 	l2tp_tunnel_inc_refcount(tunnel);
-	if (false == queue_work(l2tp_wq, &tunnel->del_work)) {
-		l2tp_tunnel_dec_refcount(tunnel);
-		return 1;
-	}
-	return 0;
+
+	/* Remove the tunnel struct from the tunnel list */
+	pn = l2tp_pernet(tunnel->l2tp_net);
+	spin_lock_bh(&pn->l2tp_tunnel_list_lock);
+	tunnel->dead = 1;
+	list_del_rcu(&tunnel->list);
+	spin_unlock_bh(&pn->l2tp_tunnel_list_lock);
+	atomic_dec(&l2tp_tunnel_count);
+
+	queue_work(l2tp_wq, &tunnel->del_work);
 }
 EXPORT_SYMBOL_GPL(l2tp_tunnel_delete);
 
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index a305e0c5925a..173e68bb8119 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -160,6 +160,8 @@ struct l2tp_tunnel_cfg {
 
 struct l2tp_tunnel {
 	int			magic;		/* Should be L2TP_TUNNEL_MAGIC */
+	int			dead;
+
 	struct rcu_head rcu;
 	rwlock_t		hlist_lock;	/* protect session_hlist */
 	bool			acpt_newsess;	/* Indicates whether this
@@ -254,7 +256,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id,
 		       u32 peer_tunnel_id, struct l2tp_tunnel_cfg *cfg,
 		       struct l2tp_tunnel **tunnelp);
 void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel);
-int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel);
+void l2tp_tunnel_delete(struct l2tp_tunnel *tunnel);
 struct l2tp_session *l2tp_session_create(int priv_size,
 					 struct l2tp_tunnel *tunnel,
 					 u32 session_id, u32 peer_session_id,
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH net] l2tp: fix race condition in l2tp_tunnel_delete
From: Tom Parkin @ 2017-09-15  9:42 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: netdev, Guillaume Nault, Xin Long
In-Reply-To: <f07e7ba88b9b4651900a888796e3f8816d06a8a8.1505396134.git.sd@queasysnail.net>

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

On Fri, Sep 15, 2017 at 11:08:07AM +0200, Sabrina Dubroca wrote:
> The tunnel is currently removed from the list during destruction. This
> can lead to a double-free of the struct sock if we try to delete the tunnel
> twice fast enough.
> 
> The first delete operation does a lookup (l2tp_tunnel_get), finds the
> tunnel, calls l2tp_tunnel_delete, which queues it for deletion by
> l2tp_tunnel_del_work.
>
> The second delete operation also finds the tunnel and calls
> l2tp_tunnel_delete. If the workqueue has already fired and started
> running l2tp_tunnel_del_work, then l2tp_tunnel_delete will queue the
> same tunnel a second time, and try to free the socket again.
> 
> Add a dead flag and remove tunnel from its list earlier. Then we can
> remove the check of queue_work's result that was meant to prevent that
> race but doesn't.

How do we avoid leaving stale information on the tunnel list for
use-cases which don't delete tunnels using netlink?  For example the
L2TPv2 ppp/socket API depends on sk_destruct to clean up the kernel
context on socket destruction.  Similarly, userspace may just close
the tunnel socket without first making netlink calls to delete the
tunnel.

By moving the tunnel list removal from l2tp_tunnel_destruct to
l2tp_tunnel_delete I can't see how codepaths which don't involve
l2tp_tunnel_delete don't end up with a corrupted tunnel list.

Tom
-- 
Tom Parkin
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* Re: RFC: Audit Kernel Container IDs
From: Richard Guy Briggs @ 2017-09-15 10:19 UTC (permalink / raw)
  To: Carlos O'Donell
  Cc: mszeredi-H+wXaHxf7aLQT0dZR+AlfA, jlayton-H+wXaHxf7aLQT0dZR+AlfA,
	Linux API, Linux Containers, Linux Kernel, Eric Paris, Al Viro,
	David Howells, Linux Audit, Andy Lutomirski,
	Linux Network Development, Linux FS Devel,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Serge E. Hallyn,
	trondmy-7I+n7zu2hftEKMMhf/gKZA, Eric W. Biederman
In-Reply-To: <20170914053007.GR3405-bcJWsdo4jJjeVoXN4CMphl7TgLCtbB0G@public.gmane.org>

On 2017-09-14 01:30, Richard Guy Briggs wrote:
> On 2017-09-13 14:33, Carlos O'Donell wrote:
> > 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.

I should clarify: It wasn't intended that a process can see or modify
its own or a peer's special proc container file to be able to set it or
discover its value.  This was only meant for its orchestrator or
delegated agents to do.  This can't be left only to CAP_CONTAINER_ADMIN.
This may require a container to have its own mount namespace if the 
trigger mechanism is a proc file write.  Other methods (additional
namespaces?) may be needed to restrict it for other trigger methods
(syscall?).

> > * 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?
> 
> - ability to filter unwanted, irrelevant or unimportant messages before
>   they fill queue so important messages don't get lost.  This is a
>   certification requirement.
> 
> - ability to make security claims about containers, require tracking of
>   actions within those containers to ensure compliance with established
>   security policies.
> 
> - ability to route messages from events to relevant audit daemon
>   instance or host audit daemon instance or both, as required or
>   determined by user-initiated rules
> 
> >   - How does it help to provide better audit messages?
> > 
> >   - Is it be enough to list the namespace that a process occupies?
> 
> We started with that approach back more than 4 years ago and found it
> helped, but didn't go far enough in terms of quick and inexpensive
> record filtering and left some doubt about provenance of events in the
> case of non-user context events (incoming network packets).
> 
> > * Why does it need the kernel's help?
> > 
> >   - Is there a race condition that is only fixable with kernel support?
> 
> This was a concern, but relatively minor compared with the other benefits.
> 
> >   - Or is it easier with kernel help but not required?
> 
> It is much easier and much less expensive.
> 
> > Providing background on these questions would help clarify the
> > design requirements.
> 
> Here are some references that should help provide some background:
> 	https://github.com/linux-audit/audit-kernel/issues/32
> 	RFE: add namespace IDs to audit records
> 
> 	https://github.com/linux-audit/audit-documentation/wiki/SPEC-Virtualization-Manager-Guest-Lifecycle-Events
> 	SPEC Virtualization Manager Guest Lifecycle Events
> 
> 	https://lwn.net/Articles/699819/
> 	Audit, namespaces, and containers
> 
> 	https://lwn.net/Articles/723561/
> 	Containers as kernel objects
> 	(my reply, with references: https://lkml.org/lkml/2017/8/14/15 )
> 
> 	https://bugzilla.redhat.com/show_bug.cgi?id=1045666
> 	audit: add namespace IDs to log records
> 
> > > 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.
> 
> Ok, that's a very reasonable request.  'signal' has a previous meaning.
> 
> > > 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.
> 
> Maybe some don't, but that's not likely to last long given the
> abstraction and nesting of orchestration tools.  This must be nestable.

This is why we can't rely only on CAP_CONTAINER_ADMIN to restrict the
ability for self-modification or self-discovery.

> > I assume that if you don't have CAP_CONTAINER_ADMIN, that nothing prevents
> > your continued operation as we have today?
> 
> Correct.  It won't prevent processes that otherwise have permissions
> today from creating all the namespaces it wishes.
> 
> > > 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?
> 
> Ah, good question, I forgot to address that fact.  The intent is that
> either threaded processes after initiating threading will not have
> permission to execute this, or all the processes in the thread group
> will be forced into the same container.  I don't have a strong opinion
> on whether or not it must be the lead thread process that must be the
> one to receive that registration, but I suspect that would be wise.
> 
> > > 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?
> 
> Yes.

And as pointed to above, it isn't the process itself that is able to
change to a new container, but its orchestrator to move/assign it.

> > 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?
> 
> I could envision that situation and I think that might be workable but
> for the synchronicity of having one initiated by a specific syscall and
> the other initiated by a /proc write.

The ability to clone while providing a containerID would work really
well, but I'm hesitant to extend or duplicate the clone call.  This
actually sounds like a potentially sane way of approaching it.

> > > 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.
> 
> Thank you sir!
> 
> > Carlos.
> 
> - 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
> 
> --
> Linux-audit mailing list
> Linux-audit-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> https://www.redhat.com/mailman/listinfo/linux-audit

- 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: Page allocator bottleneck
From: Mel Gorman @ 2017-09-15 10:23 UTC (permalink / raw)
  To: Tariq Toukan
  Cc: David Miller, Jesper Dangaard Brouer, Eric Dumazet,
	Alexei Starovoitov, Saeed Mahameed, Eran Ben Elisha,
	Linux Kernel Network Developers, Andrew Morton, Michal Hocko,
	linux-mm
In-Reply-To: <cef85936-10b2-5d76-9f97-cb03b418fd94@mellanox.com>

On Thu, Sep 14, 2017 at 07:49:31PM +0300, Tariq Toukan wrote:
> 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.
> 

Unfortunately, no surprises there. 

> Congestion in this case is very clear.
> When monitored in perf top:
> 85.58% [kernel] [k] queued_spin_lock_slowpath
> 

While it's not proven, the most likely candidate is the zone lock and
that should be confirmed using a call-graph profile. If so, then the
suggestion to tune to the size of the per-cpu allocator would mitigate
the problem.

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

Indeed not and it would also need driver conversion.

> - Page-allocation bulking.

Prototypes exist but it's pointless without the pool or driver
conversion so it's in the back burner for the moment.

> - Optimize order-0 allocations in Per-Cpu-Pages.
> 

This had a prototype that was reverted as it must be able to cope with
both irq and noirq contexts. Unfortunately I never found the time to
revisit it but a split there to handle both would mitigate the problem.
Probably not enough to actually reach line speed though so tuning of the
per-cpu allocator sizes would still be needed. I don't know when I'll
get the chance to revisit it. I'm travelling all next week and am mostly
occupied with other work at the moment that is consuming all my
concentration.

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

The recent effort to reduce overhead from stats will help mitigate the
problem. Finishing the page pool, the bulk allocator and converting drivers
would be the most likely successful path forward but it's currently stalled
as everyone that was previously involved is too busy.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH net] sctp: fix an use-after-free issue in sctp_sock_dump
From: Marcelo Ricardo Leitner @ 2017-09-15 12:55 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman, pabeni
In-Reply-To: <9de7bef8f4f8c0f369361118f6d2daee5d188467.1505444541.git.lucien.xin@gmail.com>

On Fri, Sep 15, 2017 at 11:02:21AM +0800, Xin Long wrote:
> Commit 86fdb3448cc1 ("sctp: ensure ep is not destroyed before doing the
> dump") tried to fix an use-after-free issue by checking !sctp_sk(sk)->ep
> with holding sock and sock lock.
> 
> But Paolo noticed that endpoint could be destroyed in sctp_rcv without
> sock lock protection. It means the use-after-free issue still could be
> triggered when sctp_rcv put and destroy ep after sctp_sock_dump checks
> !ep, although it's pretty hard to reproduce.
> 
> I could reproduce it by mdelay in sctp_rcv while msleep in sctp_close
> and sctp_sock_dump long time.
> 
> This patch is to add another param cb_done to sctp_for_each_transport
> and dump ep->assocs with holding tsp after jumping out of transport's
> traversal in it to avoid this issue.
> 
> It can also improve sctp diag dump to make it run faster, as no need
> to save sk into cb->args[5] and keep calling sctp_for_each_transport
> any more.
> 
> This patch is also to use int * instead of int for the pos argument
> in sctp_for_each_transport, which could make postion increment only
> in sctp_for_each_transport and no need to keep changing cb->args[2]
> in sctp_sock_filter and sctp_sock_dump any more.
> 
> Fixes: 86fdb3448cc1 ("sctp: ensure ep is not destroyed before doing the dump")
> Reported-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  include/net/sctp/sctp.h |  3 ++-
>  net/sctp/sctp_diag.c    | 32 +++++++++-----------------------
>  net/sctp/socket.c       | 40 +++++++++++++++++++++++++---------------
>  3 files changed, 36 insertions(+), 39 deletions(-)
> 
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index 06b4f51..d7d8cba 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -127,7 +127,8 @@ int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
>  				  const union sctp_addr *laddr,
>  				  const union sctp_addr *paddr, void *p);
>  int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
> -			    struct net *net, int pos, void *p);
> +			    int (*cb_done)(struct sctp_transport *, void *),
> +			    struct net *net, int *pos, void *p);
>  int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *), void *p);
>  int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
>  		       struct sctp_info *info);
> diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
> index e99518e..7008a99 100644
> --- a/net/sctp/sctp_diag.c
> +++ b/net/sctp/sctp_diag.c
> @@ -279,9 +279,11 @@ static int sctp_tsp_dump_one(struct sctp_transport *tsp, void *p)
>  	return err;
>  }
>  
> -static int sctp_sock_dump(struct sock *sk, void *p)
> +static int sctp_sock_dump(struct sctp_transport *tsp, void *p)
>  {
> +	struct sctp_endpoint *ep = tsp->asoc->ep;
>  	struct sctp_comm_param *commp = p;
> +	struct sock *sk = ep->base.sk;
>  	struct sk_buff *skb = commp->skb;
>  	struct netlink_callback *cb = commp->cb;
>  	const struct inet_diag_req_v2 *r = commp->r;
> @@ -289,9 +291,7 @@ static int sctp_sock_dump(struct sock *sk, void *p)
>  	int err = 0;
>  
>  	lock_sock(sk);
> -	if (!sctp_sk(sk)->ep)
> -		goto release;
> -	list_for_each_entry(assoc, &sctp_sk(sk)->ep->asocs, asocs) {
> +	list_for_each_entry(assoc, &ep->asocs, asocs) {
>  		if (cb->args[4] < cb->args[1])
>  			goto next;
>  
> @@ -327,40 +327,30 @@ static int sctp_sock_dump(struct sock *sk, void *p)
>  		cb->args[4]++;
>  	}
>  	cb->args[1] = 0;
> -	cb->args[2]++;
>  	cb->args[3] = 0;
>  	cb->args[4] = 0;
>  release:
>  	release_sock(sk);
> -	sock_put(sk);
>  	return err;
>  }
>  
> -static int sctp_get_sock(struct sctp_transport *tsp, void *p)
> +static int sctp_sock_filter(struct sctp_transport *tsp, void *p)
>  {
>  	struct sctp_endpoint *ep = tsp->asoc->ep;
>  	struct sctp_comm_param *commp = p;
>  	struct sock *sk = ep->base.sk;
> -	struct netlink_callback *cb = commp->cb;
>  	const struct inet_diag_req_v2 *r = commp->r;
>  	struct sctp_association *assoc =
>  		list_entry(ep->asocs.next, struct sctp_association, asocs);
>  
>  	/* find the ep only once through the transports by this condition */
>  	if (tsp->asoc != assoc)
> -		goto out;
> +		return 0;
>  
>  	if (r->sdiag_family != AF_UNSPEC && sk->sk_family != r->sdiag_family)
> -		goto out;
> -
> -	sock_hold(sk);
> -	cb->args[5] = (long)sk;
> +		return 0;
>  
>  	return 1;
> -
> -out:
> -	cb->args[2]++;
> -	return 0;
>  }
>  
>  static int sctp_ep_dump(struct sctp_endpoint *ep, void *p)
> @@ -503,12 +493,8 @@ static void sctp_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
>  	if (!(idiag_states & ~(TCPF_LISTEN | TCPF_CLOSE)))
>  		goto done;
>  
> -next:
> -	cb->args[5] = 0;
> -	sctp_for_each_transport(sctp_get_sock, net, cb->args[2], &commp);
> -
> -	if (cb->args[5] && !sctp_sock_dump((struct sock *)cb->args[5], &commp))
> -		goto next;
> +	sctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,
> +				net, (int *)&cb->args[2], &commp);
>  
>  done:
>  	cb->args[1] = cb->args[4];
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 1b00a1e..d4730ad 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4658,29 +4658,39 @@ int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
>  EXPORT_SYMBOL_GPL(sctp_transport_lookup_process);
>  
>  int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
> -			    struct net *net, int pos, void *p) {
> +			    int (*cb_done)(struct sctp_transport *, void *),
> +			    struct net *net, int *pos, void *p) {
>  	struct rhashtable_iter hti;
> -	void *obj;
> -	int err;
> -
> -	err = sctp_transport_walk_start(&hti);
> -	if (err)
> -		return err;
> +	struct sctp_transport *tsp;
> +	int ret;
>  
> -	obj = sctp_transport_get_idx(net, &hti, pos + 1);
> -	for (; !IS_ERR_OR_NULL(obj); obj = sctp_transport_get_next(net, &hti)) {
> -		struct sctp_transport *transport = obj;
> +again:
> +	ret = sctp_transport_walk_start(&hti);
> +	if (ret)
> +		return ret;
>  
> -		if (!sctp_transport_hold(transport))
> +	tsp = sctp_transport_get_idx(net, &hti, *pos + 1);
> +	for (; !IS_ERR_OR_NULL(tsp); tsp = sctp_transport_get_next(net, &hti)) {
> +		if (!sctp_transport_hold(tsp))
>  			continue;
> -		err = cb(transport, p);
> -		sctp_transport_put(transport);
> -		if (err)
> +		ret = cb(tsp, p);
> +		if (ret)
>  			break;
> +		(*pos)++;
> +		sctp_transport_put(tsp);
>  	}
>  	sctp_transport_walk_stop(&hti);
>  
> -	return err;
> +	if (ret) {
> +		if (cb_done && !cb_done(tsp, p)) {
> +			(*pos)++;
> +			sctp_transport_put(tsp);
> +			goto again;
> +		}
> +		sctp_transport_put(tsp);
> +	}
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(sctp_for_each_transport);
>  
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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 net] sctp: do not mark sk dumped when inet_sctp_diag_fill returns err
From: Marcelo Ricardo Leitner @ 2017-09-15 12:55 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman
In-Reply-To: <c5398945d4e80ed8b8497675dd0449763fec4eec.1505444568.git.lucien.xin@gmail.com>

On Fri, Sep 15, 2017 at 11:02:48AM +0800, Xin Long wrote:
> sctp_diag would not actually dump out sk/asoc if inet_sctp_diag_fill
> returns err, in which case it shouldn't mark sk dumped by setting
> cb->args[3] as 1 in sctp_sock_dump().
> 
> Otherwise, it could cause some asocs to have no parent's sk dumped
> in 'ss --sctp'.
> 
> So this patch is to not set cb->args[3] when inet_sctp_diag_fill()
> returns err in sctp_sock_dump().
> 
> Fixes: 8f840e47f190 ("sctp: add the sctp_diag.c file")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/sctp_diag.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
> index 7008a99..22ed01a 100644
> --- a/net/sctp/sctp_diag.c
> +++ b/net/sctp/sctp_diag.c
> @@ -309,7 +309,6 @@ static int sctp_sock_dump(struct sctp_transport *tsp, void *p)
>  					cb->nlh->nlmsg_seq,
>  					NLM_F_MULTI, cb->nlh,
>  					commp->net_admin) < 0) {
> -			cb->args[3] = 1;
>  			err = 1;
>  			goto release;
>  		}
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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 net] sctp: fix an use-after-free issue in sctp_sock_dump
From: Neil Horman @ 2017-09-15 12:57 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner, pabeni
In-Reply-To: <9de7bef8f4f8c0f369361118f6d2daee5d188467.1505444541.git.lucien.xin@gmail.com>

On Fri, Sep 15, 2017 at 11:02:21AM +0800, Xin Long wrote:
> Commit 86fdb3448cc1 ("sctp: ensure ep is not destroyed before doing the
> dump") tried to fix an use-after-free issue by checking !sctp_sk(sk)->ep
> with holding sock and sock lock.
> 
> But Paolo noticed that endpoint could be destroyed in sctp_rcv without
> sock lock protection. It means the use-after-free issue still could be
> triggered when sctp_rcv put and destroy ep after sctp_sock_dump checks
> !ep, although it's pretty hard to reproduce.
> 
> I could reproduce it by mdelay in sctp_rcv while msleep in sctp_close
> and sctp_sock_dump long time.
> 
> This patch is to add another param cb_done to sctp_for_each_transport
> and dump ep->assocs with holding tsp after jumping out of transport's
> traversal in it to avoid this issue.
> 
> It can also improve sctp diag dump to make it run faster, as no need
> to save sk into cb->args[5] and keep calling sctp_for_each_transport
> any more.
> 
> This patch is also to use int * instead of int for the pos argument
> in sctp_for_each_transport, which could make postion increment only
> in sctp_for_each_transport and no need to keep changing cb->args[2]
> in sctp_sock_filter and sctp_sock_dump any more.
> 
> Fixes: 86fdb3448cc1 ("sctp: ensure ep is not destroyed before doing the dump")
> Reported-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/net/sctp/sctp.h |  3 ++-
>  net/sctp/sctp_diag.c    | 32 +++++++++-----------------------
>  net/sctp/socket.c       | 40 +++++++++++++++++++++++++---------------
>  3 files changed, 36 insertions(+), 39 deletions(-)
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

^ permalink raw reply

* Re: [PATCH net] sctp: do not mark sk dumped when inet_sctp_diag_fill returns err
From: Neil Horman @ 2017-09-15 12:59 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner
In-Reply-To: <c5398945d4e80ed8b8497675dd0449763fec4eec.1505444568.git.lucien.xin@gmail.com>

On Fri, Sep 15, 2017 at 11:02:48AM +0800, Xin Long wrote:
> sctp_diag would not actually dump out sk/asoc if inet_sctp_diag_fill
> returns err, in which case it shouldn't mark sk dumped by setting
> cb->args[3] as 1 in sctp_sock_dump().
> 
> Otherwise, it could cause some asocs to have no parent's sk dumped
> in 'ss --sctp'.
> 
> So this patch is to not set cb->args[3] when inet_sctp_diag_fill()
> returns err in sctp_sock_dump().
> 
> Fixes: 8f840e47f190 ("sctp: add the sctp_diag.c file")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/sctp/sctp_diag.c | 1 -
>  1 file changed, 1 deletion(-)
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

^ permalink raw reply

* Re: feature request for kernel module 8021q
From: Pierre Colombier @ 2017-09-15 13:02 UTC (permalink / raw)
  To: Florian Fainelli, netdev
In-Reply-To: <A4D84B7F-5D4A-44C8-875C-C113D7C5C688@gmail.com>


>> One interesting feature would be to have a special vlan number
>>
>> (let's say 0 or -1 or 4097 ) designed so that it gets all the untagged
>> trafic and only the untagged trafic.
> 4097 is not a valid number nor is -1 since that closely follows what the VLAN ID extracted from the tag would be. VLAN 0 is more or less the untagged VLAN.
I know -1 or 4097 are invalid numbers
This is precicely why i suggested them as "special"
I wasn't aware of the existing Vlan 0 feature
I just tried again and it seems to be exactly what I was requesting.

What do you mean by "more or less" ?

^ permalink raw reply

* [patch net] mlxsw: spectrum_router: Only handle IPv4 and IPv6 events
From: Jiri Pirko @ 2017-09-15 13:31 UTC (permalink / raw)
  To: netdev; +Cc: davem, idosch, flokli, andreas, mlxsw

From: Ido Schimmel <idosch@mellanox.com>

The driver doesn't support events from address families other than IPv4
and IPv6, so ignore them. Otherwise, we risk queueing a work item before
it's initialized.

This can happen in case a VRF is configured when MROUTE_MULTIPLE_TABLES
is enabled, as the VRF driver will try to add an l3mdev rule for the
IPMR family.

Fixes: 65e65ec137f4 ("mlxsw: spectrum_router: Don't ignore IPv6 notifications")
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reported-by: Andreas Rammhold <andreas@rammhold.de>
Reported-by: Florian Klink <flokli@flokli.de>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index f0fb898..2cfb3f5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -4868,7 +4868,8 @@ static int mlxsw_sp_router_fib_event(struct notifier_block *nb,
 	struct fib_notifier_info *info = ptr;
 	struct mlxsw_sp_router *router;
 
-	if (!net_eq(info->net, &init_net))
+	if (!net_eq(info->net, &init_net) ||
+	    (info->family != AF_INET && info->family != AF_INET6))
 		return NOTIFY_DONE;
 
 	fib_work = kzalloc(sizeof(*fib_work), GFP_ATOMIC);
-- 
2.9.3

^ permalink raw reply related

* [PATCH net] bpf/verifier: reject BPF_ALU64|BPF_END
From: Edward Cree @ 2017-09-15 13:37 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Daniel Borkmann

Neither ___bpf_prog_run nor the JITs accept it.
Also adds a new test case.

Fixes: 17a5267067f3 ("bpf: verifier (add verifier core)")
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 kernel/bpf/verifier.c                       |  3 ++-
 tools/testing/selftests/bpf/test_verifier.c | 16 ++++++++++++++++
 2 files changed, 18 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;
 			}
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 8eb0995..26f3250 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -6629,6 +6629,22 @@ static struct bpf_test tests[] = {
 		.result = REJECT,
 		.flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
 	},
+	{
+		"invalid 64-bit BPF_END",
+		.insns = {
+			BPF_MOV32_IMM(BPF_REG_0, 0),
+			{
+				.code  = BPF_ALU64 | BPF_END | BPF_TO_LE,
+				.dst_reg = BPF_REG_0,
+				.src_reg = 0,
+				.off   = 0,
+				.imm   = 32,
+			},
+			BPF_EXIT_INSN(),
+		},
+		.errstr = "BPF_END uses reserved fields",
+		.result = REJECT,
+	},
 };
 
 static int probe_filter_length(const struct bpf_insn *fp)

^ permalink raw reply related

* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Neal Cardwell @ 2017-09-15 14:03 UTC (permalink / raw)
  To: Oleksandr Natalenko
  Cc: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Netdev,
	Yuchung Cheng
In-Reply-To: <1760119.uO98D2ft6f@natalenko.name>

On Fri, Sep 15, 2017 at 1:03 AM, Oleksandr Natalenko
<oleksandr@natalenko.name> wrote:
> Hi.
>
> I've applied your test patch but it doesn't fix the issue for me since the
> warning is still there.
>
> Were you able to reproduce it?

Hi,

Thanks for testing that. That is a very useful data point.

I was able to cook up a packetdrill test that could put the connection
in CA_Disorder with retransmitted packets out, but not in CA_Open. So
we do not yet have a test case to reproduce this.

We do not see this warning on our fleet at Google. One significant
difference I see between our environment and yours is that it seems
you run with FACK enabled:

  net.ipv4.tcp_fack = 1

Note that FACK was disabled by default (since it was replaced by RACK)
between kernel v4.10 and v4.11. And this is exactly the time when this
bug started manifesting itself for you and some others, but not our
fleet. So my new working hypothesis would be that this warning is due
to a behavior that only shows up in kernels >=4.11 when FACK is
enabled.

Would you be able to disable FACK ("sysctl net.ipv4.tcp_fack=0" at
boot, or net.ipv4.tcp_fack=0 in /etc/sysctl.conf, or equivalent),
reboot, and test the kernel for a few days to see if the warning still
pops up?

thanks,
neal

[ps: apologies for the previous, mis-formatted post...]

^ permalink raw reply

* Re: [PATCH net] packet: hold bind lock when rebinding to fanout hook
From: Willem de Bruijn @ 2017-09-15 14:07 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: Network Development, David Miller, nixiaoming
In-Reply-To: <20170914211441.67326-1-willemb@google.com>

On Thu, Sep 14, 2017 at 5:14 PM, Willem de Bruijn <willemb@google.com> wrote:
> Packet socket bind operations must hold the po->bind_lock. This keeps
> po->running consistent with whether the socket is actually on a ptype
> list to receive packets.
>
> fanout_add unbinds a socket and its packet_rcv/tpacket_rcv call, then
> binds the fanout object to receive through packet_rcv_fanout.
>
> Make it hold the po->bind_lock when testing po->running and rebinding.
> Else, it can race with other rebind operations, such as that in
> packet_set_ring from packet_rcv to tpacket_rcv. Concurrent updates
> can result in a socket being added to a fanout group twice, causing
> use-after-free KASAN bug reports, among others.
>
> Reported independently by both trinity and syzkaller.
> Verified that the syzkaller reproducer passes after this patch.
>

I forgot to add the Fixes tag, sorry.

Fixes: dc99f600698d ("packet: Add fanout support.")

> Reported-by: nixioaming <nixiaoming@huawei.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>

^ permalink raw reply

* Re: [PATCH net-next v2 01/10] net: dsa: add debugfs interface
From: Andrew Lunn @ 2017-09-15 14:08 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Alexander Duyck, Maxim Uvarov, Vivien Didelot, netdev, kernel,
	Florian Fainelli, Egil Hjelmeland, John Crispin, Woojung Huh,
	Sean Wang, Nikita Yushchenko, Chris Healy
In-Reply-To: <20170915055107.GA1927@nanopsycho.orion>

> Could you put together your requirements so we can work it out to extend
> devlink to support them?

As i've said multiple times, generic two dimensional tables. Examples
could look like:

Stats                 cpu     lan0   lan1   lan2   lan3   lan4    dsa
---------------------------------------------------------------------
tx_packets:              2       0      0      0      0      0      0
tx_bytes:             1180    6666      0      0      0      0      0
rx_packets:              0       0      0      0      0      0      0
rx_bytes:                0    1180      0      0      0      0      0
in_good_octets:       6666    1188      0      0      0      0      0
in_bad_octets:           0       0      0      0      0      0      0
in_unicast:              0       0      0      0      0      0      0
in_broadcasts:           0       0      0      0      0      0      0
in_multicasts:          89       0      0      0      0      0      0
in_pause:                0       0      0      0      0      0      0
in_undersize:            0       0      0      0      0      0      0
in_fragments:            0       0      0      0      0      0      0
in_oversize:             0       0      0      0      0      0      0
in_jabber:               0       0      0      0      0      0      0
in_rx_error:             0       0      0      0      0      0      0
in_fcs_error:            0       0      0      0      0      0      0
out_octets:           1188    6666      0      0      0      0      0
out_unicast:             0       0      0      0      0      0      0
out_broadcasts:          2       2      0      0      0      0      0
out_multicasts:          0      89      0      0      0      0      0
out_pause:               0       0      0      0      0      0      0
excessive:               0       0      0      0      0      0      0
collisions:              0       0      0      0      0      0      0
deferred:                0       0      0      0      0      0      0
single:                  0       0      0      0      0      0      0
multiple:                0       0      0      0      0      0      0
out_fcs_error:           0       0      0      0      0      0      0
late:                    0       0      0      0      0      0      0
hist_64bytes:            0       0      0      0      0      0      0
hist_65_127bytes:        0       0      0      0      0      0      0
hist_128_255bytes:       0       0      0      0      0      0      0
hist_256_511bytes:       0       0      0      0      0      0      0
hist_512_1023bytes:      0       0      0      0      0      0      0
hist_1024_max_bytes:     0       0      0      0      0      0      0
sw_in_discards:          0       0      0      0      0      0      0
sw_in_filtered:          0       0      0      0      0      0      0
sw_out_filtered:        89      89      0      0      0      0      0


   Reg  cpu     lan0    lan1    lan2    lan3    lan4    lan5  global0 global1
-----------------------------------------------------------------------------
    00: 4e07	4d04	4d04	4d04	4d04	4d04	4d04     0000   00000
    01: 403e	003d	003d	003d	003d	003d	003d     0000   00000
    02: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    03: 3521	3521	3521	3521	3521	3521	3521     0000   00000
    04: 0533	373f	373f	373f	373f	373f	373f     0000   00000
    05: 8000	0000	0000	0000	0000	0000	0000     0000   00000
    06: 005f	003f	003f	003f	003f	003f	003f     0000   00000
    07: 002a	002a	002a	002a	002a	002a	002a     0000   00000
    08: 2080	2080	2080	2080	2080	2080	2080     0000   00000
    09: 0001	0001	0001	0001	0001	0001	0001     0000   00000
    0a: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    0b: 0020	0000	0000	0000	0000	0000	0000     0000   00000
    0c: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    0d: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    0e: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    0f: 9100	dada	dada	dada	dada	dada	dada     0000   00000
    10: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    11: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    12: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    13: 0000	00d8	00d8	00d8	00d8	00d8	00d8     0000   00000
    14: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    15: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    16: 0022	0000	0000	0000	0000	0000	0000     0000   00000
    17: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    18: 3210	2210	2210	2210	2210	2210	2210     0000   00000
    19: 7654	7654	7654	7654	7654	7654	7654     0000   00000
    1a: 0000	0000	0000	0000	0000	0000	0000     0000   00000
    1b: 8000	8000	8000	8000	8000	8000	8000     0000   00000
    1c: 0000    0000    0000    0000    0000    0000    0000     0000   00000
    1d: 0000    0000    0000    0000    0000    0000    0000     0000   00000
    1e: 0000    0000    0000    0000    0000    0000    0000     0000   00000
    1f: 0000    0000    0000    0000    0000    0000    0000     0000   00000

So a table would have an optional header row. Then a number of data
rows. The number of columns is the same for each row. The number of
columns is determined at run time, but is known at the beginning of
enumerating the table. The number of data rows is not known until the
last one is enumerated.

Each cell in the row is typed. Can be a string, bool, u8, u16, u32, u64,
ifindex, MAC address, IP address, devlink port, ....

Each cell in a row can have a different type. The types of the header
row cells can be different to the types of the data cells. All data
rows have the same type information. So you can enumerate the types
once, and use them for the whole table.

The userspace tool should make its best effort to print the table,
using the type info. It might need hits from the command line, like -x
to print in hex not decimal. The second example shows this. Registers
make more sense in hex, where as statistics make more sense in
decimal. But let the user choose, so keeping the kAPI simple.  This is
intended as a debug tool. The output does not need to be highly
polished. But it needs to be a lot more readable than the current
dpipe output which prints a table as a list, not a table.

The tables can be per port, or per switch. Remember that DSA allows a
cluster of switches within one DSA instance.

Vivien, Florian, did i miss anything?

	 Andrew

^ permalink raw reply

* Re: [PATCH] net: phy: Fix mask value write on gmii2rgmii converter speed register.
From: Andrew Lunn @ 2017-09-15 14:10 UTC (permalink / raw)
  To: Michal Simek
  Cc: Fahad Kunnathadi, f.fainelli, netdev, linux-kernel,
	soren.brinkmann, linux-arm-kernel
In-Reply-To: <5f4bd003-d251-5a14-497a-9cc72dd0f160@xilinx.com>

> Can you please point me to that email?

I assume you can search the email lists just as well as i can.

  Andrew

^ permalink raw reply

* Re: [PATCH net-next v2 01/10] net: dsa: add debugfs interface
From: Jiri Pirko @ 2017-09-15 14:26 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Alexander Duyck, Maxim Uvarov, Vivien Didelot, netdev, kernel,
	Florian Fainelli, Egil Hjelmeland, John Crispin, Woojung Huh,
	Sean Wang, Nikita Yushchenko, Chris Healy
In-Reply-To: <20170915140839.GD3084@lunn.ch>

Fri, Sep 15, 2017 at 04:08:39PM CEST, andrew@lunn.ch wrote:
>> Could you put together your requirements so we can work it out to extend
>> devlink to support them?
>
>As i've said multiple times, generic two dimensional tables. Examples
>could look like:
>
>Stats                 cpu     lan0   lan1   lan2   lan3   lan4    dsa
>---------------------------------------------------------------------
>tx_packets:              2       0      0      0      0      0      0
>tx_bytes:             1180    6666      0      0      0      0      0
>rx_packets:              0       0      0      0      0      0      0
>rx_bytes:                0    1180      0      0      0      0      0
>in_good_octets:       6666    1188      0      0      0      0      0
>in_bad_octets:           0       0      0      0      0      0      0
>in_unicast:              0       0      0      0      0      0      0
>in_broadcasts:           0       0      0      0      0      0      0
>in_multicasts:          89       0      0      0      0      0      0
>in_pause:                0       0      0      0      0      0      0
>in_undersize:            0       0      0      0      0      0      0
>in_fragments:            0       0      0      0      0      0      0
>in_oversize:             0       0      0      0      0      0      0
>in_jabber:               0       0      0      0      0      0      0
>in_rx_error:             0       0      0      0      0      0      0
>in_fcs_error:            0       0      0      0      0      0      0
>out_octets:           1188    6666      0      0      0      0      0
>out_unicast:             0       0      0      0      0      0      0
>out_broadcasts:          2       2      0      0      0      0      0
>out_multicasts:          0      89      0      0      0      0      0
>out_pause:               0       0      0      0      0      0      0
>excessive:               0       0      0      0      0      0      0
>collisions:              0       0      0      0      0      0      0
>deferred:                0       0      0      0      0      0      0
>single:                  0       0      0      0      0      0      0
>multiple:                0       0      0      0      0      0      0
>out_fcs_error:           0       0      0      0      0      0      0
>late:                    0       0      0      0      0      0      0
>hist_64bytes:            0       0      0      0      0      0      0
>hist_65_127bytes:        0       0      0      0      0      0      0
>hist_128_255bytes:       0       0      0      0      0      0      0
>hist_256_511bytes:       0       0      0      0      0      0      0
>hist_512_1023bytes:      0       0      0      0      0      0      0
>hist_1024_max_bytes:     0       0      0      0      0      0      0
>sw_in_discards:          0       0      0      0      0      0      0
>sw_in_filtered:          0       0      0      0      0      0      0
>sw_out_filtered:        89      89      0      0      0      0      0

I believe we discussed this already. You can have devlink_port instance
for each port, then you just have stats-per-devlink_port. Looks quite
easy to implement actually.

Would be mistake to have this as just 2 dim array.


>
>
>   Reg  cpu     lan0    lan1    lan2    lan3    lan4    lan5  global0 global1
>-----------------------------------------------------------------------------
>    00: 4e07	4d04	4d04	4d04	4d04	4d04	4d04     0000   00000
>    01: 403e	003d	003d	003d	003d	003d	003d     0000   00000
>    02: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    03: 3521	3521	3521	3521	3521	3521	3521     0000   00000
>    04: 0533	373f	373f	373f	373f	373f	373f     0000   00000
>    05: 8000	0000	0000	0000	0000	0000	0000     0000   00000
>    06: 005f	003f	003f	003f	003f	003f	003f     0000   00000
>    07: 002a	002a	002a	002a	002a	002a	002a     0000   00000
>    08: 2080	2080	2080	2080	2080	2080	2080     0000   00000
>    09: 0001	0001	0001	0001	0001	0001	0001     0000   00000
>    0a: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    0b: 0020	0000	0000	0000	0000	0000	0000     0000   00000
>    0c: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    0d: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    0e: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    0f: 9100	dada	dada	dada	dada	dada	dada     0000   00000
>    10: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    11: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    12: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    13: 0000	00d8	00d8	00d8	00d8	00d8	00d8     0000   00000
>    14: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    15: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    16: 0022	0000	0000	0000	0000	0000	0000     0000   00000
>    17: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    18: 3210	2210	2210	2210	2210	2210	2210     0000   00000
>    19: 7654	7654	7654	7654	7654	7654	7654     0000   00000
>    1a: 0000	0000	0000	0000	0000	0000	0000     0000   00000
>    1b: 8000	8000	8000	8000	8000	8000	8000     0000   00000
>    1c: 0000    0000    0000    0000    0000    0000    0000     0000   00000
>    1d: 0000    0000    0000    0000    0000    0000    0000     0000   00000
>    1e: 0000    0000    0000    0000    0000    0000    0000     0000   00000
>    1f: 0000    0000    0000    0000    0000    0000    0000     0000   00000
>

Is this a reg dump per-port? Also, with per-port devlink_port instance,
you can have reg array for each. How the values can change? Is this
change result of driver<->hw communication? If yes, you might consider
using devlink hwmsg trace to expose the communication to userspace.


>So a table would have an optional header row. Then a number of data
>rows. The number of columns is the same for each row. The number of
>columns is determined at run time, but is known at the beginning of
>enumerating the table. The number of data rows is not known until the
>last one is enumerated.

I would rather focus on what exactly you need to expose to userspace,
then we can figure out how to do it. Generic multipurpose arrays should
be considered as last-resort solution in my opinion.


>
>Each cell in the row is typed. Can be a string, bool, u8, u16, u32, u64,
>ifindex, MAC address, IP address, devlink port, ....
>
>Each cell in a row can have a different type. The types of the header
>row cells can be different to the types of the data cells. All data
>rows have the same type information. So you can enumerate the types
>once, and use them for the whole table.
>
>The userspace tool should make its best effort to print the table,
>using the type info. It might need hits from the command line, like -x
>to print in hex not decimal. The second example shows this. Registers
>make more sense in hex, where as statistics make more sense in
>decimal. But let the user choose, so keeping the kAPI simple.  This is
>intended as a debug tool. The output does not need to be highly
>polished. But it needs to be a lot more readable than the current
>dpipe output which prints a table as a list, not a table.
>
>The tables can be per port, or per switch. Remember that DSA allows a
>cluster of switches within one DSA instance.
>
>Vivien, Florian, did i miss anything?
>
>	 Andrew

^ permalink raw reply

* Re: [PATCH net] l2tp: fix race condition in l2tp_tunnel_delete
From: Sabrina Dubroca @ 2017-09-15 14:55 UTC (permalink / raw)
  To: Tom Parkin; +Cc: netdev, Guillaume Nault, Xin Long
In-Reply-To: <20170915094259.GA3209@jackdaw>

2017-09-15, 10:42:59 +0100, Tom Parkin wrote:
> On Fri, Sep 15, 2017 at 11:08:07AM +0200, Sabrina Dubroca wrote:
> > The tunnel is currently removed from the list during destruction. This
> > can lead to a double-free of the struct sock if we try to delete the tunnel
> > twice fast enough.
> > 
> > The first delete operation does a lookup (l2tp_tunnel_get), finds the
> > tunnel, calls l2tp_tunnel_delete, which queues it for deletion by
> > l2tp_tunnel_del_work.
> >
> > The second delete operation also finds the tunnel and calls
> > l2tp_tunnel_delete. If the workqueue has already fired and started
> > running l2tp_tunnel_del_work, then l2tp_tunnel_delete will queue the
> > same tunnel a second time, and try to free the socket again.
> > 
> > Add a dead flag and remove tunnel from its list earlier. Then we can
> > remove the check of queue_work's result that was meant to prevent that
> > race but doesn't.
> 
> How do we avoid leaving stale information on the tunnel list for
> use-cases which don't delete tunnels using netlink?  For example the
> L2TPv2 ppp/socket API depends on sk_destruct to clean up the kernel
> context on socket destruction.  Similarly, userspace may just close
> the tunnel socket without first making netlink calls to delete the
> tunnel.
> 
> By moving the tunnel list removal from l2tp_tunnel_destruct to
> l2tp_tunnel_delete I can't see how codepaths which don't involve
> l2tp_tunnel_delete don't end up with a corrupted tunnel list.

Ok, thanks for pointing that out. We could go with just the ->dead
flag then. I'm not sure whether we need to set it in
l2tp_tunnel_destruct as well.

-------- 8< --------

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index ee485df73ccd..e74596418169 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1685,14 +1685,12 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_create);
 
 /* This function is used by the netlink TUNNEL_DELETE command.
  */
-int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel)
+void l2tp_tunnel_delete(struct l2tp_tunnel *tunnel)
 {
-	l2tp_tunnel_inc_refcount(tunnel);
-	if (false == queue_work(l2tp_wq, &tunnel->del_work)) {
-		l2tp_tunnel_dec_refcount(tunnel);
-		return 1;
+	if (!test_and_set_bit(1, &tunnel->dead)) {
+		l2tp_tunnel_inc_refcount(tunnel);
+		queue_work(l2tp_wq, &tunnel->del_work);
 	}
-	return 0;
 }
 EXPORT_SYMBOL_GPL(l2tp_tunnel_delete);
 
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index a305e0c5925a..deda869504d0 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -160,6 +160,9 @@ struct l2tp_tunnel_cfg {
 
 struct l2tp_tunnel {
 	int			magic;		/* Should be L2TP_TUNNEL_MAGIC */
+
+	unsigned long		dead;
+
 	struct rcu_head rcu;
 	rwlock_t		hlist_lock;	/* protect session_hlist */
 	bool			acpt_newsess;	/* Indicates whether this
@@ -254,7 +257,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id,
 		       u32 peer_tunnel_id, struct l2tp_tunnel_cfg *cfg,
 		       struct l2tp_tunnel **tunnelp);
 void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel);
-int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel);
+void l2tp_tunnel_delete(struct l2tp_tunnel *tunnel);
 struct l2tp_session *l2tp_session_create(int priv_size,
 					 struct l2tp_tunnel *tunnel,
 					 u32 session_id, u32 peer_session_id,


-- 
Sabrina

^ permalink raw reply related

* Re: [PATCH net-next v2 01/10] net: dsa: add debugfs interface
From: Andrew Lunn @ 2017-09-15 15:19 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Alexander Duyck, Maxim Uvarov, Vivien Didelot, netdev, kernel,
	Florian Fainelli, Egil Hjelmeland, John Crispin, Woojung Huh,
	Sean Wang, Nikita Yushchenko, Chris Healy
In-Reply-To: <20170915142617.GA2060@nanopsycho.orion>

> >   Reg  cpu     lan0    lan1    lan2    lan3    lan4    lan5  global0 global1
> >-----------------------------------------------------------------------------
> >    00: 4e07	4d04	4d04	4d04	4d04	4d04	4d04     0000   00000
> >    01: 403e	003d	003d	003d	003d	003d	003d     0000   00000
> >    02: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    03: 3521	3521	3521	3521	3521	3521	3521     0000   00000
> >    04: 0533	373f	373f	373f	373f	373f	373f     0000   00000
> >    05: 8000	0000	0000	0000	0000	0000	0000     0000   00000
> >    06: 005f	003f	003f	003f	003f	003f	003f     0000   00000
> >    07: 002a	002a	002a	002a	002a	002a	002a     0000   00000
> >    08: 2080	2080	2080	2080	2080	2080	2080     0000   00000
> >    09: 0001	0001	0001	0001	0001	0001	0001     0000   00000
> >    0a: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    0b: 0020	0000	0000	0000	0000	0000	0000     0000   00000
> >    0c: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    0d: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    0e: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    0f: 9100	dada	dada	dada	dada	dada	dada     0000   00000
> >    10: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    11: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    12: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    13: 0000	00d8	00d8	00d8	00d8	00d8	00d8     0000   00000
> >    14: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    15: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    16: 0022	0000	0000	0000	0000	0000	0000     0000   00000
> >    17: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    18: 3210	2210	2210	2210	2210	2210	2210     0000   00000
> >    19: 7654	7654	7654	7654	7654	7654	7654     0000   00000
> >    1a: 0000	0000	0000	0000	0000	0000	0000     0000   00000
> >    1b: 8000	8000	8000	8000	8000	8000	8000     0000   00000
> >    1c: 0000    0000    0000    0000    0000    0000    0000     0000   00000
> >    1d: 0000    0000    0000    0000    0000    0000    0000     0000   00000
> >    1e: 0000    0000    0000    0000    0000    0000    0000     0000   00000
> >    1f: 0000    0000    0000    0000    0000    0000    0000     0000   00000
> >
> 
> Is this a reg dump per-port?

No. Look at the global0 and global1 columns. These are not port
registers, but switch global registers. The other columns are per
port. We could have a two column table per port for these
registers. However for debugging, it is useful to see the port
registers side-by-side. The difference between ports often gives you
clues as to what is wrong.

The global registers however are scoped to a switch, not a port.

> you can have reg array for each. How the values can change? Is this
> change result of driver<->hw communication? If yes, you might consider
> using devlink hwmsg trace to expose the communication to userspace.

The reason a value changes varies per bit. Some are status, set by the
switch. Some are configuration, set by the driver. They are all mixed
up together. And some registers are actually just multiplexors onto
other tables of registers. You set an opcode in the lower nibble,
address in the middle, set the top bit and busy loop waiting for it to
clear. You can then read a result out of the register, or a near by
register.  So the register dump gives you an idea what the last access
to such a table was.

> I would rather focus on what exactly you need to expose to userspace,
> then we can figure out how to do it. Generic multipurpose arrays should
> be considered as last-resort solution in my opinion.

So go look at Vivien's patches. That is what we want to expose. For a
start. Viviens patches are at the DSA level. Once we get something
accepted at that level, i can imaging drivers want to augment it with
driver specific tables. But we can handle that later.

       Andrew

^ permalink raw reply

* Re: RTL8192EE PCIe Wireless Network Adapter crashed with linux-4.13
From: Larry Finger @ 2017-09-15 15:19 UTC (permalink / raw)
  To: Zwindl
  Cc: linux-wireless@vger.kernel.org, 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: <jB0WJNG17mSfKo9m7T26r-b2Gr7mDL4lJ-1cWxeCJ6UCsA4TsxXz4znCLhYY5EASPQciwp1rf_xtwvDR-xzwD-OTApTUFoaVmIvahleDcJk=@protonmail.com>

On 09/15/2017 05:10 AM, Zwindl wrote:
> 
>> -------- Original Message --------
>> Subject: Re: RTL8192EE PCIe Wireless Network Adapter crashed with linux-4.13
>> Local Time: 14 September 2017 6:05 PM
>> UTC Time: 14 September 2017 18:05
>> From: Larry.Finger@lwfinger.net
>> To: Zwindl <zwindl@protonmail.com>, linux-wireless@vger.kernel.org 
>> <linux-wireless@vger.kernel.org>
>> chaoming_li@realsil.com.cn <chaoming_li@realsil.com.cn>, kvalo@codeaurora.org 
>> <kvalo@codeaurora.org>, pkshih@realtek.com <pkshih@realtek.com>, 
>> johannes.berg@intel.com <johannes.berg@intel.com>, gregkh@linuxfoundation.org 
>> <gregkh@linuxfoundation.org>, netdev@vger.kernel.org <netdev@vger.kernel.org>, 
>> linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
>>
>> 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
> Oh, sorry, the original log is from `journalctl`.
> Here's the `dmesg` prints(error.txt). I can't determine which part is related, 
> so I paste all of it. I've tried 4.12.X(no issue), 4.13.1(issue), 4.13.2(issue).
> ZWindL

The output of dmesg is a lot more instructive than that of journalctl. I now 
know exactly the location that triggered the WARNING. I still do not understand 
it. In fact, it is likely a regression in kernel 4.13 that does not affect my 
Toshiba laptop, nor a Lenovo machine I have, but does affect your Lenovo laptop.

Is it possible for you to install the mainline source from vger.kernel.org using 
git and bisect the issue? It will take quite a bit of time, but it is likely the 
only way to find the offending change. If you are willing to try this, I will 
send you reasonably complete instructions.

By the way, it is usually better to load the dmesg output into a pastebin site 
and post the link. Sending the entire file to a list makes a lot of people 
receive a lot of data for which they have no interest.

Larry

^ permalink raw reply

* Re: [PATCH net] bpf/verifier: reject BPF_ALU64|BPF_END
From: Alexei Starovoitov @ 2017-09-15 15:50 UTC (permalink / raw)
  To: Edward Cree; +Cc: David Miller, netdev, Daniel Borkmann
In-Reply-To: <8c3d5513-6171-3e68-56df-1efd0e87f071@solarflare.com>

On Fri, Sep 15, 2017 at 02:37:38PM +0100, Edward Cree wrote:
> Neither ___bpf_prog_run nor the JITs accept it.
> Also adds a new test case.
> 
> Fixes: 17a5267067f3 ("bpf: verifier (add verifier core)")
> Signed-off-by: Edward Cree <ecree@solarflare.com>

Good catch. Thanks!
Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: selftests/bpf doesn't compile
From: Alexei Starovoitov @ 2017-09-15 16:02 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Daniel Borkmann, Thomas Meyer, linux-kernel, linux-kselftest,
	Shuah Khan, Networking
In-Reply-To: <295553a4-aadc-e5d1-229e-22d1966bc9f5@kernel.org>

On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote:
> 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??

It is a hard dependency and clang should be present on majority of the systems.
More details are in samples/bpf/README.rst
which was written long ago. Nowadays apt-get/yum will install clang
with bpf support builtin.

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

I'm not sure what was the motivation to exclude hotplug from default testing,
since I think it diminishes the value of selftests overall.
Not running all tests all the time risks breaking them.
selftest makefile refactoring broke selftests/bpf in the past,
so I strongly suggest to install clang and make sure the tests are passing
on the test servers otherwise we'd need to move selftests/bpf out of
selftests to avoid further headaches for us when selftests infra keeps
changing.

^ 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