Netdev List
 help / color / mirror / Atom feed
* Re: [RFC PATCH linux-next] ovs: internal_set_rx_headroom() can be static
From: David Miller @ 2016-03-18 21:50 UTC (permalink / raw)
  To: fengguang.wu-ral2JQCrhuEAvxtiuMwx3w
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, kbuild-all-JC7UmRfGjtg,
	pabeni-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <20160318165450.GA16372@cairo>

From: kbuild test robot <fengguang.wu@intel.com>
Date: Sat, 19 Mar 2016 00:54:50 +0800

> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>

Looks good, applied, thanks.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

^ permalink raw reply

* Re: [RFC PATCH linux-next] net: dst_cache_per_cpu_dst_set() can be static
From: David Miller @ 2016-03-18 21:45 UTC (permalink / raw)
  To: fengguang.wu; +Cc: pabeni, kbuild-all, netdev, linux-kernel
In-Reply-To: <20160318152728.GA64259@cairo>

From: kbuild test robot <fengguang.wu@intel.com>
Date: Fri, 18 Mar 2016 23:27:28 +0800

> 
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>

Looks good, applied, thanks.

^ permalink raw reply

* Re: [PATCH v2] openvswitch: call only into reachable nf-nat code
From: David Miller @ 2016-03-18 21:41 UTC (permalink / raw)
  To: arnd
  Cc: pshelar, tgraf, joestringer, pabeni, jarno, pablo, ebiederm, fw,
	netdev, dev, linux-kernel
In-Reply-To: <1458308044-246105-1-git-send-email-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 18 Mar 2016 14:33:45 +0100

> The openvswitch code has gained support for calling into the
> nf-nat-ipv4/ipv6 modules, however those can be loadable modules
> in a configuration in which openvswitch is built-in, leading
> to link errors:
> 
> net/built-in.o: In function `__ovs_ct_lookup':
> :(.text+0x2cc2c8): undefined reference to `nf_nat_icmp_reply_translation'
> :(.text+0x2cc66c): undefined reference to `nf_nat_icmpv6_reply_translation'
> 
> The dependency on (!NF_NAT || NF_NAT) prevents similar issues,
> but NF_NAT is set to 'y' if any of the symbols selecting
> it are built-in, but the link error happens when any of them
> are modular.
> 
> A second issue is that even if CONFIG_NF_NAT_IPV6 is built-in,
> CONFIG_NF_NAT_IPV4 might be completely disabled. This is unlikely
> to be useful in practice, but the driver currently only handles
> IPv6 being optional.
> 
> This patch improves the Kconfig dependency so that openvswitch
> cannot be built-in if either of the two other symbols are set
> to 'm', and it replaces the incorrect #ifdef in ovs_ct_nat_execute()
> with two "if (IS_ENABLED())" checks that should catch all corner
> cases also make the code more readable.
> 
> The same #ifdef exists ovs_ct_nat_to_attr(), where it does not
> cause a link error, but for consistency I'm changing it the same
> way.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 05752523e565 ("openvswitch: Interface with NAT.")
> Acked-by: Joe Stringer <joe@ovn.org>
> ---
> v2: leave (!NF_NAT || NF_NAT) dependency in there, we also need that

I'll let Pablo pick this up.

^ permalink raw reply

* [PATCH] sctp: do not leak chunks that are sent to unconfirmed paths
From: Marcelo Ricardo Leitner @ 2016-03-18 21:39 UTC (permalink / raw)
  To: netdev; +Cc: Neil Horman, Vlad Yasevich, linux-sctp

Currently, if a chunk is scheduled to be sent through a transport that
is currently unconfirmed, it will be leaked as it is dequeued from outq
and is not re-queued nor freed.

As I'm not aware of any situation that may lead to this situation, I'm
fixing this by freeing the chunk and also logging a trace so that we can
fix the other bug if it ever happens.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/outqueue.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index f03541d0f12d052d1f58901d57852be46f18a15a..8d3d3625130ee0fd294998554a9290d57eae56e7 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -978,8 +978,12 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout, gfp_t gfp)
 			     (new_transport->state == SCTP_UNCONFIRMED) ||
 			     (new_transport->state == SCTP_PF)))
 				new_transport = asoc->peer.active_path;
-			if (new_transport->state == SCTP_UNCONFIRMED)
+			if (new_transport->state == SCTP_UNCONFIRMED) {
+				WARN_ONCE(1, "Atempt to send packet on unconfirmed path.");
+				sctp_chunk_fail(chunk, 0);
+				sctp_chunk_free(chunk);
 				continue;
+			}
 
 			/* Change packets if necessary.  */
 			if (new_transport != transport) {
-- 
2.5.0

^ permalink raw reply related

* [PATCH] sctp: do not update a_rwnd if we are not issuing a sack
From: Marcelo Ricardo Leitner @ 2016-03-18 21:39 UTC (permalink / raw)
  To: netdev; +Cc: Neil Horman, Vlad Yasevich, linux-sctp

The SACK can be lost pretty much elsewhere, but if its allocation fail,
we know we are not sending it, so it is better to revert a_rwnd to its
previous value as this may give it a chance to issue a window update
later.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/sm_sideeffect.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 3c22c41a2bc2dc128d651f64b1a81036be22b9b3..7fe56d0acabf66cfd8fe29dfdb45f7620b470ac7 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -215,10 +215,14 @@ static int sctp_gen_sack(struct sctp_association *asoc, int force,
 		sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_RESTART,
 				SCTP_TO(SCTP_EVENT_TIMEOUT_SACK));
 	} else {
+		__u32 old_a_rwnd = asoc->a_rwnd;
+
 		asoc->a_rwnd = asoc->rwnd;
 		sack = sctp_make_sack(asoc);
-		if (!sack)
+		if (!sack) {
+			asoc->a_rwnd = old_a_rwnd;
 			goto nomem;
+		}
 
 		asoc->peer.sack_needed = 0;
 		asoc->peer.sack_cnt = 0;
-- 
2.5.0

^ permalink raw reply related

* [PATCH] sctp: keep fragmentation point aligned to word size
From: Marcelo Ricardo Leitner @ 2016-03-18 21:39 UTC (permalink / raw)
  To: netdev; +Cc: Neil Horman, Vlad Yasevich, linux-sctp

If the user supply a different fragmentation point or if there is a
network header that cause it to not be aligned, force it to be aligned.

Fragmentation point at a value that is not aligned is not optimal.  It
causes extra padding to be used and has just no pros.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 835aa2ed987092634a4242314e9eabb51d1e4e35..f27d38113c7926783a9bf2863e14e963ed0ad5d6 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -426,7 +426,7 @@ static inline int sctp_frag_point(const struct sctp_association *asoc, int pmtu)
 	if (asoc->user_frag)
 		frag = min_t(int, frag, asoc->user_frag);
 
-	frag = min_t(int, frag, SCTP_MAX_CHUNK_LEN);
+	frag = min_t(int, frag, SCTP_MAX_CHUNK_LEN) & ~3;
 
 	return frag;
 }
-- 
2.5.0

^ permalink raw reply related

* [PATCH] sctp: align MTU to a word
From: Marcelo Ricardo Leitner @ 2016-03-18 21:39 UTC (permalink / raw)
  To: netdev; +Cc: Neil Horman, Vlad Yasevich, linux-sctp

SCTP is a protocol that is aligned to a word (4 bytes). Thus using bare
MTU can sometimes return values that are not aligned, like for loopback,
which is 65536 but ipv4_mtu() limits that to 65535. This mis-alignment
will cause the last non-aligned bytes to never be used and can cause
issues with congestion control.

So it's better to just consider a lower MTU and keep congestion control
calcs saner as they are based on PMTU.

Same applies to icmp frag needed messages, which is also fixed by this
patch.

One other effect of this is the inability to send MTU-sized packet
without queueing or fragmentation and without hitting Nagle. As the
check performed at sctp_packet_can_append_data():

if (chunk->skb->len + q->out_qlen >= transport->pathmtu - packet->overhead)
	/* Enough data queued to fill a packet */
	return SCTP_XMIT_OK;

with the above example of MTU, if there are no other messages queued,
one cannot send a packet that just fits one packet (65532 bytes) and
without causing DATA chunk fragmentation or a delay.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/associola.c | 2 +-
 net/sctp/input.c     | 2 +-
 net/sctp/transport.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index a19b3e60770382507050a56a172ffc5adb2f497a..f89862194093f160c2801c22050690789d956b44 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1406,7 +1406,7 @@ void sctp_assoc_sync_pmtu(struct sock *sk, struct sctp_association *asoc)
 	list_for_each_entry(t, &asoc->peer.transport_addr_list,
 				transports) {
 		if (t->pmtu_pending && t->dst) {
-			sctp_transport_update_pmtu(sk, t, dst_mtu(t->dst));
+			sctp_transport_update_pmtu(sk, t, dst_mtu(t->dst) & ~3);
 			t->pmtu_pending = 0;
 		}
 		if (!pmtu || (t->pathmtu < pmtu))
diff --git a/net/sctp/input.c b/net/sctp/input.c
index db76f1ab4ac2cb16b4568f0d9fbe4e3b90deaa1c..fda32738986971648ff73b05318b8a306d496446 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -606,7 +606,7 @@ void sctp_v4_err(struct sk_buff *skb, __u32 info)
 
 		/* PMTU discovery (RFC1191) */
 		if (ICMP_FRAG_NEEDED == code) {
-			sctp_icmp_frag_needed(sk, asoc, transport, info);
+			sctp_icmp_frag_needed(sk, asoc, transport, info & ~3);
 			goto out_unlock;
 		} else {
 			if (ICMP_PROT_UNREACH == code) {
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index d517153891a6efca6242ce1a4b3d86afb9026542..22219e8e6e510469d9d5868245bd7e2a9e2faf78 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -226,7 +226,7 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
 	}
 
 	if (transport->dst) {
-		transport->pathmtu = dst_mtu(transport->dst);
+		transport->pathmtu = dst_mtu(transport->dst) & ~3;
 	} else
 		transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
 }
@@ -280,7 +280,7 @@ void sctp_transport_route(struct sctp_transport *transport,
 		return;
 	}
 	if (transport->dst) {
-		transport->pathmtu = dst_mtu(transport->dst);
+		transport->pathmtu = dst_mtu(transport->dst) & ~3;
 
 		/* Initialize sk->sk_rcv_saddr, if the transport is the
 		 * association's active path for getsockname().
-- 
2.5.0

^ permalink raw reply related

* Re: [PATCH] net: consolidate lock/unlock into unlock_wait
From: David Miller @ 2016-03-18 21:37 UTC (permalink / raw)
  To: hofrat; +Cc: joe, netdev, linux-kernel
In-Reply-To: <1458293525-16842-1-git-send-email-hofrat@osadl.org>

From: Nicholas Mc Guire <hofrat@osadl.org>
Date: Fri, 18 Mar 2016 10:32:05 +0100

> The spin_lock()/spin_unlock() is synchronizing on the adapter->work_lock
> as the comment also suggests, which is equivalent to spin_unlock_wait()
> but the later should be more efficient.
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>

There really is no justification for this change.

This is an optimization in a slow-path of the driver.

The device is a rarely used older piece of hardware.

The amount of testers of this driver is probably approximating zero.

So there is only risk of breakage from this change, and absolutely
zero upside.

Therefore, I'm not applying this patch, and I'd kindly like to ask
you to please consider such issues in the future for these kinds of
transformations.

Thanks.

^ permalink raw reply

* Re: [PATCH] ipv6: Fix the pmtu path for connected UDP socket
From: Wei Wang @ 2016-03-18 21:26 UTC (permalink / raw)
  To: Cong Wang
  Cc: Wei Wang, Martin KaFai Lau, Eric Dumazet, David Miller,
	Eric Dumazet, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpXgHHr=h=3VeMdYfOnzTdeBeODGVKmwBi7AFW485_WRAg@mail.gmail.com>

I don't think ip6_sk_update_pmtu() is a good place to put it as all it
does is to call ip6_update_pmtu(). And ip6_update_pmtu() does the
route lookup and call __ip6_rt_update_pmtu.
We can put it in ip6_update_pmtu(). But that still means we need to
pass sk to ip6_update_pmtu() and I don't think it makes any difference
compared to the current fix.

Thanks.
Wei

On Fri, Mar 18, 2016 at 11:10 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Mar 18, 2016 at 10:45 AM, Wei Wang <weiwan@google.com> wrote:
>> Thanks all for the comments.
>>
>> Cong, you are right that for ipv6, the code does not take care of updating
>> sk_dst_cache entry like Ipv4. And this patch is updating the entry by
>> calling ip6_dst_store() to achieve similar functionality as Ipv4.
>
> My question is why not updating ip6_sk_update_pmtu() to sync with ipv4?

^ permalink raw reply

* Re: [RFD] workqueue: WQ_MEM_RECLAIM usage in network drivers
From: J. Bruce Fields @ 2016-03-18 21:24 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Jeff Layton, David S. Miller, Trond Myklebust, Anna Schumaker,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, Amitoj Kaur Chawla,
	kernel-team-b10kYP2dOMg, Johannes Weiner, Johannes Berg,
	Eva Rachel Retuya, Bhaktipriya Shridhar,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160318204623.GM20028-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>

On Fri, Mar 18, 2016 at 04:46:23PM -0400, Tejun Heo wrote:
> Hello, Jeff.
> 
> On Thu, Mar 17, 2016 at 09:32:16PM -0400, Jeff Layton wrote:
> > > * Are network devices expected to be able to serve as a part of
> > >   storage stack which is depended upon for memory reclamation?
> > 
> > I think they should be. Cached NFS pages can consume a lot of memory,
> > and flushing them generally takes network device access.
> 
> But does that actually work?  It's pointless to add WQ_MEM_RECLAIM to
> workqueues unless all other things are also guaranteed to make forward
> progress regardless of memory pressure.

It's supposed to work.

Also note there was a bunch of work done to allow swap on NFS: see
a564b8f0 "nfs: enable swap on NFS".

> 
> > > * If so, are all the pieces in place for that to work for all (or at
> > >   least most) network devices?  If it's only for a subset of NICs, how
> > >   can one tell whether a given driver needs forward progress guarantee
> > >   or not?
> > > 
> > > * I assume that wireless drivers aren't and can't be used in this
> > >   fashion.  Is that a correction assumption?
> > > 
> > 
> > People do mount NFS over wireless interfaces. It's not terribly common
> > though, in my experience.
> 
> Ditto, I'm very skeptical that this actually works in practice and
> people expect and depend on it.  I don't follow wireless development
> closely but haven't heard anyone talking about reserving memory pools
> or people complaining about wireless being the cause of OOM.
> 
> So, I really want to avoid spraying WQ_MEM_RECLAIM if it doesn't serve
> actual purposes.  It's wasteful, sets bad precedences and confuses
> future readers.

I use NFS mounts over wifi at home.  I may just be odd.  I seem to
recall some bug reports about suspend vs. NFS--were those also on
laptops using NFS?

I wonder if home media centers might do writes over wifi to network
storage?

Googling for "nfs wifi" turns up lots of individuals doing this.

My first impulse is to say that it's probably not perfect but that we
shouldn't make it worse.

But, I don't claim to understand the WQ_MEM_RECLAIM-proliferation issue
you're seeing....

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [net] openvswitch: Allow deferred action fifo to expand during run time
From: David Miller @ 2016-03-18 21:19 UTC (permalink / raw)
  To: azhou-LZ6Gd1LRuIk
  Cc: dev-yBygre7rU0SM8Zsap4Y0gw, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1458275533-2896-1-git-send-email-azhou-LZ6Gd1LRuIk@public.gmane.org>

From: Andy Zhou <azhou@ovn.org>
Date: Thu, 17 Mar 2016 21:32:13 -0700

> Current openvswitch implementation allows up to 10 recirculation actions
> for each packet. This limit was sufficient for most use cases in the
> past, but with more new features, such as supporting connection
> tracking, and testing in larger scale network environment,
> This limit may be too restrictive.
 ...

Actions that need to recirculate that many times are extremely poorly
designed, and will have significant performance problems.

I think the way rules are put together and processed should be redone
before we do insane stuff like this.

There is no way I'm applying a patch like this, sorry.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

^ permalink raw reply

* Re: [PATCH v3] can: rcar_canfd: Add Renesas R-Car CAN FD driver
From: Rob Herring @ 2016-03-18 21:07 UTC (permalink / raw)
  To: Ramesh Shanmugasundaram
  Cc: mkl, wg, pawel.moll, mark.rutland, ijc+devicetree, galak, corbet,
	linux-renesas-soc, devicetree, linux-can, netdev, linux-doc,
	geert+renesas, chris.paterson2
In-Reply-To: <1458035294-8150-1-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>

On Tue, Mar 15, 2016 at 09:48:14AM +0000, Ramesh Shanmugasundaram wrote:
> This patch adds support for the CAN FD controller found in Renesas R-Car
> SoCs. The controller operates in CAN FD only mode by default.
> 
> CAN FD mode supports both Classical CAN & CAN FD frame formats. The
> controller supports ISO 11898-1:2015 CAN FD format only.
> 
> This controller supports two channels and the driver can enable either
> or both of the channels.
> 
> Driver uses Rx FIFOs (one per channel) for reception & Common FIFOs (one
> per channel) for transmission. Rx filter rules are configured to the
> minimum (one per channel) and it accepts Standard, Extended, Data &
> Remote Frame combinations.
> 
> Note: There are few documentation errors in R-Car Gen3 Hardware User
> Manual v0.5E with respect to CAN FD controller. They are listed below:
> 
> 1. CAN FD interrupt numbers 29 & 30 are listed as per channel
> interrupts. However, they are common to both channels (i.e.) they are
> global and channel interrupts respectively.
> 
> 2. CANFD clock is derived from PLL1. This is not documented.
> 
> 3. CANFD clock is further divided by (1/2) within the CAN FD controller.
> This is not documented.
> 
> 4. The minimum value of NTSEG1 in RSCFDnCFDCmNCFG register is 2 Tq. It
> is specified 4 Tq in the manual.
> 
> 5. The maximum number of message RAM area the controller can use is 3584
> bytes. It is specified 10752 bytes in the manual.
> 
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> ---
> Hi All,
> 
>    Thanks for the review comments.
> 
>    This updated patch is based on linux-can-next tag (linux-can-next-for-4.6-20160310).
> 
>    This patch depends on
> 
>    [RFC] [PATCH v3] can: fix handling of unmodifiable configuration options
>    (http://comments.gmane.org/gmane.linux.can/9126)
> 
> Changes since v2:
> 	* Rebased to latest tag (linux-can-next-for-4.6-20160310)
> 	* Cleaned up leftover debugfs code (Thanks Oliver H)
> 	* Revised devicetree documentation text (as suggested by Rob H)
> 	  (https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg01597.html)
> 	* Used new can subsystem api to set static configuration & removed check in rcar_canfd_start (as suggested by Oliver H)
> 	  (Refer: http://comments.gmane.org/gmane.linux.can/9126 &
> 	   https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg01867.html)
> 	* Clubbed Renesas controller drivers to driver/net/can/rcar dir (as suggested by Oliver H)
> 	* Updated commit message
> 
> Changes since v1:
> 	* Removed testmodes & debugfs code (suggested by Oliver H)
> 	* Fixed tx path race issue by introducing lock (suggested by Marc K)
> 	* Removed __maybe_unused attribute of rcar_canfd_of_table
> ---
>  .../devicetree/bindings/net/can/rcar_canfd.txt     |   89 ++

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

>  drivers/net/can/Kconfig                            |   11 +-
>  drivers/net/can/Makefile                           |    2 +-
>  drivers/net/can/rcar/Kconfig                       |   19 +
>  drivers/net/can/rcar/Makefile                      |    6 +
>  drivers/net/can/{ => rcar}/rcar_can.c              |    0
>  drivers/net/can/rcar/rcar_canfd.c                  | 1614 ++++++++++++++++++++
>  7 files changed, 1730 insertions(+), 11 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/can/rcar_canfd.txt
>  create mode 100644 drivers/net/can/rcar/Kconfig
>  create mode 100644 drivers/net/can/rcar/Makefile
>  rename drivers/net/can/{ => rcar}/rcar_can.c (100%)
>  create mode 100644 drivers/net/can/rcar/rcar_canfd.c

^ permalink raw reply

* Re: net/bluetooth: workqueue destruction WARNING in hci_unregister_dev
From: Tejun Heo @ 2016-03-18 20:52 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Dmitry Vyukov, Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev,
	LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Sasha Levin, Eric Dumazet, Takashi Iwai
In-Reply-To: <56EA9C4D.2080803-AlSwsSmVLrQ@public.gmane.org>

Hello, Jiri.

On Thu, Mar 17, 2016 at 01:00:13PM +0100, Jiri Slaby wrote:
> >> I have not done that yet, but today, I see:
> >> destroy_workqueue: name='req_hci0' pwq=ffff88002f590300
> >> wq->dfl_pwq=ffff88002f591e00 pwq->refcnt=2 pwq->nr_active=0 delayed_works:
> >>    pwq 12: cpus=0-1 node=0 flags=0x4 nice=-20 active=0/1
> >>      in-flight: 18568:wq_barrier_func
> > 
> > So, this means that there's flush_work() racing against workqueue
> > destruction, which can't be safe. :(
> 
> But I cannot trigger the WARN_ONs in the attached patch, so I am
> confused how this can happen :(. (While I am still seeing the destroy
> WARNINGs.)

So, no operations should be in progress when destroy_workqueue() is
called.  If somebody was flushing a work item, the flush call must
have returned before destroy_workqueue() was invoked, which doesn't
seem to be the case here.  Can you trigger BUG_ON() or sysrq-t when
the above triggers?  There must be a task which is flushing a work
item there and it shouldn't be difficult to pinpoint what's going on
from it.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [Outreachy kernel] [PATCH v2] iwlwifi: dvm: use alloc_ordered_workqueue()
From: Tejun Heo @ 2016-03-18 20:47 UTC (permalink / raw)
  To: Eva Rachel Retuya
  Cc: outreachy-kernel-/JYPxA39Uh5TLH3MbocFFw,
	johannes.berg-ral2JQCrhuEAvxtiuMwx3w,
	emmanuel.grumbach-ral2JQCrhuEAvxtiuMwx3w,
	linuxwifi-ral2JQCrhuEAvxtiuMwx3w, kvalo-sgV2jX0FEOL9JmXXK+q4OQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1458231561-13184-1-git-send-email-eraretuya-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Fri, Mar 18, 2016 at 12:19:21AM +0800, Eva Rachel Retuya wrote:
> Use alloc_ordered_workqueue() to allocate the workqueue instead of
> create_singlethread_workqueue() since the latter is deprecated and is scheduled
> for removal.
> 
> There are work items doing related operations that shouldn't be swapped when
> queued in a certain order hence preserve the strict execution ordering of a
> single threaded (ST) workqueue by switching to alloc_ordered_workqueue().
> 
> WQ_MEM_RECLAIM flag is not needed since the worker is not supposed to free
> memory.

I think "not depended during memory reclaim" probalby is a better way
to describe it.

> Signed-off-by: Eva Rachel Retuya <eraretuya-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

But other than that,

 Acked-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFD] workqueue: WQ_MEM_RECLAIM usage in network drivers
From: Tejun Heo @ 2016-03-18 20:46 UTC (permalink / raw)
  To: Jeff Layton
  Cc: David S. Miller, Trond Myklebust, J. Bruce Fields, Anna Schumaker,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, Amitoj Kaur Chawla,
	kernel-team-b10kYP2dOMg, Johannes Weiner, Johannes Berg,
	Eva Rachel Retuya, Bhaktipriya Shridhar,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160317213216.731d1fcc-08S845evdOaAjSkqwZiSMmfYqLom42DlXqFh9Ls21Oc@public.gmane.org>

Hello, Jeff.

On Thu, Mar 17, 2016 at 09:32:16PM -0400, Jeff Layton wrote:
> > * Are network devices expected to be able to serve as a part of
> >   storage stack which is depended upon for memory reclamation?
> 
> I think they should be. Cached NFS pages can consume a lot of memory,
> and flushing them generally takes network device access.

But does that actually work?  It's pointless to add WQ_MEM_RECLAIM to
workqueues unless all other things are also guaranteed to make forward
progress regardless of memory pressure.

> > * If so, are all the pieces in place for that to work for all (or at
> >   least most) network devices?  If it's only for a subset of NICs, how
> >   can one tell whether a given driver needs forward progress guarantee
> >   or not?
> > 
> > * I assume that wireless drivers aren't and can't be used in this
> >   fashion.  Is that a correction assumption?
> > 
> 
> People do mount NFS over wireless interfaces. It's not terribly common
> though, in my experience.

Ditto, I'm very skeptical that this actually works in practice and
people expect and depend on it.  I don't follow wireless development
closely but haven't heard anyone talking about reserving memory pools
or people complaining about wireless being the cause of OOM.

So, I really want to avoid spraying WQ_MEM_RECLAIM if it doesn't serve
actual purposes.  It's wasteful, sets bad precedences and confuses
future readers.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Mason @ 2016-03-18 20:44 UTC (permalink / raw)
  To: Uwe Kleine-Koenig
  Cc: Sebastian Frias, Daniel Mack, David S. Miller, netdev, lkml,
	Florian Fainelli, Mans Rullgard, Fabio Estevam,
	Martin Blumenstingl, Linus Walleij
In-Reply-To: <20160318201153.GR4292@pengutronix.de>

On 18/03/2016 21:11, Uwe Kleine-König wrote:

> Hello,
> 
> On Fri, Mar 18, 2016 at 08:31:20PM +0100, Mason wrote:
>
>> On 18/03/2016 20:12, Uwe Kleine-König wrote:
>>
>>> On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote:
>>>
>>>> What would you think of making at803x_link_change_notify() print a
>>>> message every time it should do a reset but does not has a way to do it?
>>>
>>> Then this question is obsolete because the device doesn't probe.
>>
>> I don't understand this statement.
>>
>> What does it mean for a question to be obsolete?
> 
> If the driver doesn't probe because it cannot control the reset line,
> you don't need to think about how it should behave in
> at803x_link_change_notify without control of the reset line, because
> this code isn't reached then.

If I understand correctly, it is possible to soft-reset the PHY
by writing to a specific register. The GPIO pin is useful only to
force a hardware-reset when the PHY is wedged by some random event.

(Or am I completely off the mark?)

Regards.

^ permalink raw reply

* Re: [PATCH v3 1/9] net: arc_emac: make the rockchip emac document more compatible
From: Rob Herring @ 2016-03-18 20:22 UTC (permalink / raw)
  To: Caesar Wang
  Cc: Heiko Stuebner, David S. Miller,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	keescook-hpIqsD4AKlfQT0dZR+AlfA, leozwang-hpIqsD4AKlfQT0dZR+AlfA,
	sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Alexander Kochetkov, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1457942520-12859-2-git-send-email-wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On Mon, Mar 14, 2016 at 04:01:52PM +0800, Caesar Wang wrote:
> Add the rk3036 SoCs to match driver for document since the emac driver
> has supported the rk3036 SoCs.
> 
> This patch adds the rk3036/rk3066/rk3188 SoCS to compatible for rockchip
> emac ducument. Also, that will suit for other SoCs in the future.
> 
> Signed-off-by: Caesar Wang <wxt-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> Cc: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> ---
> 
> Changes in v3:
> - %s/he/the
> - Add the Cc people
> 
> Changes in v2:
> - change the commit and remove the repeat the name 'rockchip'.
> 
>  Documentation/devicetree/bindings/net/emac_rockchip.txt | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Uwe Kleine-König @ 2016-03-18 20:11 UTC (permalink / raw)
  To: Mason
  Cc: Sebastian Frias, Daniel Mack, David S. Miller, netdev, lkml,
	Florian Fainelli, Mans Rullgard, Fabio Estevam,
	Martin Blumenstingl, Linus Walleij
In-Reply-To: <56EC5788.3030509@free.fr>

Hello,

On Fri, Mar 18, 2016 at 08:31:20PM +0100, Mason wrote:
> On 18/03/2016 20:12, Uwe Kleine-König wrote:
> 
> > On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote:
> > 
> >> What would you think of making at803x_link_change_notify() print a
> >> message every time it should do a reset but does not has a way to do it?
> > 
> > Then this question is obsolete because the device doesn't probe.
> 
> I don't understand this statement.
> 
> What does it mean for a question to be obsolete?

If the driver doesn't probe because it cannot control the reset line,
you don't need to think about how it should behave in
at803x_link_change_notify without control of the reset line, because
this code isn't reached then.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Mason @ 2016-03-18 19:31 UTC (permalink / raw)
  To: Uwe Kleine-Koenig, Sebastian Frias
  Cc: Daniel Mack, David S. Miller, netdev, lkml, Florian Fainelli,
	Mans Rullgard, Fabio Estevam, Martin Blumenstingl, Linus Walleij
In-Reply-To: <20160318191242.GQ4292@pengutronix.de>

On 18/03/2016 20:12, Uwe Kleine-König wrote:

> On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote:
> 
>> What would you think of making at803x_link_change_notify() print a
>> message every time it should do a reset but does not has a way to do it?
> 
> Then this question is obsolete because the device doesn't probe.

I don't understand this statement.

What does it mean for a question to be obsolete?

Regards.

^ permalink raw reply

* [PATCH] mac80211: remove description of dropped member
From: Luis de Bethencourt @ 2016-03-18 19:23 UTC (permalink / raw)
  To: linux-kernel; +Cc: johannes, davem, linux-wireless, netdev, Luis de Bethencourt

Commit 976bd9efdae6 ("mac80211: move beacon_loss_count into ifmgd")
removed the member from the sta_info struct but the description stayed
lingering. Remove it.

Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
---
Hi,

A second patch fixing a warning in make htmldocs.

Thanks :)
Luis

 net/mac80211/sta_info.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 053f5c4..62193f4 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -377,7 +377,6 @@ DECLARE_EWMA(signal, 1024, 8)
  * @uploaded: set to true when sta is uploaded to the driver
  * @sta: station information we share with the driver
  * @sta_state: duplicates information about station state (for debug)
- * @beacon_loss_count: number of times beacon loss has triggered
  * @rcu_head: RCU head used for freeing this station struct
  * @cur_max_bandwidth: maximum bandwidth to use for TX to the station,
  *	taken from HT/VHT capabilities or VHT operating mode notification
-- 
2.5.1

^ permalink raw reply related

* Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
From: Uwe Kleine-König @ 2016-03-18 19:12 UTC (permalink / raw)
  To: Sebastian Frias
  Cc: Daniel Mack, David S. Miller, netdev, lkml, mason,
	Florian Fainelli, Mans Rullgard, Fabio Estevam,
	Martin Blumenstingl, Linus Walleij
In-Reply-To: <56EC2525.8000706@laposte.net>

Hello Sebastian,

On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote:
> On 03/18/2016 01:54 PM, Uwe Kleine-König wrote:
> > From a driver perspecitive, it would be nice if devm_gpiod_get_optional
> > returned NULL iff the respective gpio isn't specified even with
> > GPIOLIB=n, but this isn't sensible either because it would result in
> > quite some gpiolib code to not being conditionally compiled on
> > CONFIG_GPIOLIB any more.
> 
> Let's say that was the case, what would the PHY code do?

With reset gpios it might not be that critical, but consider an optional
enable gpio. (Optional in the sense, that some device have it and others
don't, e.g. because the pin is pulled into active level by hardware.)

Now you do:

	gpiod = gpiod_get_optional("enable");

and if gpiod now is an error pointer, you must assume that you cannot
operate the device. And even with GPIOLIB=n (and gpiod = ERR_PTR(-ENOSYS))
you cannot ignore the error. For consistency I'd recommend to do the
same for reset even though there is a chance to get a working device.

> What would you think of making at803x_link_change_notify() print a
> message every time it should do a reset but does not has a way to do it?

Then this question is obsolete because the device doesn't probe.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH] mac80211: add doc for RX_FLAG_DUP_VALIDATED flag
From: Luis de Bethencourt @ 2016-03-18 18:54 UTC (permalink / raw)
  To: linux-kernel; +Cc: johannes, davem, linux-wireless, netdev
In-Reply-To: <1458317369-28185-1-git-send-email-luisbg@osg.samsung.com>

On 18/03/16 16:09, Luis de Bethencourt wrote:
> Add documentation for the flag for duplication check.
> 
> Fixes the following warning when running make htmldocs:
> warning: Enum value 'RX_FLAG_DUP_VALIDATED' not described in enum 'mac80211_rx_flags'
> 
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> ---
> Hi,
> 
> While running make htmldocs I noticed a warning about RX_FLAG_DUP_VALIDATED
> missing a description. This patch fixes this.
> 
> I have noticed the order of the descriptions of the mac80211_rx_flags doesn't
> match the order in the enum. I am happy to fix this if it would be of interest.
> 
> Thanks,
> Luis
> 
>  include/net/mac80211.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index a53333c..42c8cde 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1001,6 +1001,8 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info)
>   *	flag indicates that the PN was verified for replay protection.
>   *	Note that this flag is also currently only supported when a frame
>   *	is also decrypted (ie. @RX_FLAG_DECRYPTED must be set)
> + * @RX_FLAG_DUP_VALIDATED: Set this flag for the driver to check for packet
> + *	duplication by itself.
>   * @RX_FLAG_FAILED_FCS_CRC: Set this flag if the FCS check failed on
>   *	the frame.
>   * @RX_FLAG_FAILED_PLCP_CRC: Set this flag if the PCLP check failed on
> 

Please ignore the second patch I replied to this one. It is wrong due to my
misunderstanding.

This above patch adding RX_FLAG_DUP_VALIDATED is correct and I believe it to be
valid.

Thanks for the reviews,
Luis

^ permalink raw reply

* Re: [v6, 5/5] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0
From: Scott Wood @ 2016-03-18 18:28 UTC (permalink / raw)
  To: Arnd Bergmann, Rob Herring
  Cc: Yangbo Lu, linuxppc-dev@lists.ozlabs.org,
	devicetree@vger.kernel.org, ulf.hansson@linaro.org, Zhao Qiang,
	Russell King, Bhupesh Sharma, netdev@vger.kernel.org,
	Joerg Roedel, Kumar Gala, linux-mmc@vger.kernel.org,
	linux-kernel@vger.kernel.org, Yang-Leo Li,
	iommu@lists.linux-foundation.org, linux-i2c@vger.kernel.org,
	Claudiu Manoil, Santosh Shilimkar, Xiaobo Xie,
	"linux-clk@vger.kernel.org
In-Reply-To: <3706958.XeRFmKM0K5@wuerfel>

On 03/17/2016 12:06 PM, Arnd Bergmann wrote:
> On Thursday 17 March 2016 12:01:01 Rob Herring wrote:
>> On Mon, Mar 14, 2016 at 05:45:43PM +0000, Scott Wood wrote:
> 
>>>>> This makes the driver non-portable. Better identify the specific
>>>>> workarounds based on the compatible string for this device, or add a
>>>>> boolean DT property for the quirk.
>>>>>
>>>>>    Arnd
>>>>
>>>> [Lu Yangbo-B47093] Hi Arnd, we did have a discussion about using DTS in v1 before.
>>>> https://patchwork.kernel.org/patch/6834221/
>>>>
>>>> We don’t have a separate DTS file for each revision of an SOC and if we did, we'd constantly have people using the wrong one.
>>>> In addition, the device tree is stable ABI and errata are often discovered after device tree are deployed.
>>>> See the link for details.
>>>>
>>>> So we decide to read SVR from the device-config/guts MMIO block other than using DTS.
>>>> Thanks.
>>>
>>> Also note that this driver is already only for fsl-specific hardware,
>>> and it will still work even if fsl_guts doesn't find anything to bind to
>>> -- it just wouldn't be able to detect errata based on SVR in that case.
>>
>> IIRC, it is the same IP block as i.MX and Arnd's point is this won't 
>> even compile on !PPC. It is things like this that prevent sharing the 
>> driver.

The whole point of using the MMIO SVR instead of the PPC SPR is so that
it will work on ARM...  The guts driver should build on any platform as
long as OF is enabled, and if it doesn't find a node to bind to it will
return 0 for SVR, and the eSDHC driver will continue (after printing an
error that should be removed) without the ability to test for errata
based on SVR.

> I think the first four patches take care of building for ARM,
> but the problem remains if you want to enable COMPILE_TEST as
> we need for certain automated checking.

What specific problem is there with COMPILE_TEST?

>> Dealing with Si revs is a common problem. We should have a 
>> common solution. There is soc_device for this purpose.
> 
> Exactly. The last time this came up, I think we agreed to implement a
> helper using glob_match() on the soc_device strings. Unfortunately
> this hasn't happened then, but I'd still prefer that over yet another
> vendor-specific way of dealing with the generic issue.

soc_device would require encoding the SVR as a string and then decoding
the string, which is more complicated and error prone than having
platform-specific code test a platform-specific number.  And when would
it get registered on arm64, which doesn't have platform code?

-Scott


^ permalink raw reply

* 我在这里等你
From: 我在这里等你 @ 2016-03-18 18:05 UTC (permalink / raw)
  To: netdev

你的老朋友邀你来Q群:343257759

^ permalink raw reply

* Re: [PATCH] mac80211: fix order of flag descriptions
From: Luis de Bethencourt @ 2016-03-18 18:20 UTC (permalink / raw)
  To: Johannes Berg, linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <56EC3EF1.50805-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

On 18/03/16 17:46, Luis de Bethencourt wrote:
> On 18/03/16 16:49, Johannes Berg wrote:
>> On Fri, 2016-03-18 at 16:35 +0000, Luis de Bethencourt wrote:
>>> Fix order of mac80211_rx_flags description to match the enum.
>>>
>>> Signed-off-by: Luis de Bethencourt <luisbg-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>>> ---
>>> Hi,
>>>
>>> I want ahead and fixed the order of the descriptions. checkpatch.pl
>>> was giving
>>> a warning to my previous patch and I had a hunch it was because the
>>> wrong order
>>> breaks the parser. Indeed it does and with this patch below
>>> checkpatch.pl does
>>> not complain about this flag descriptions anymore.
>>>
>> Huh. I think we should fix checkpatch.pl instead. While the current
>> order isn't likely really good, I believe kernel-doc will output the
>> documentation in the order it's listed, and that can be useful for the
>> documentation output to group related things, even if their bits may be
>> further apart.
>>
>> johannes
>>
> 
> I agree checkpatch.pl should be fixed. I can look into it. No promises
> though, it has been a long time since I look at Perl code.
> 
> I understand the logic of grouping the documentation in logical blocks.
> It is unfortunate though that this means the enum and documentation won't
> match, which makes reading the code harder.
> 
> In this particular case I don't see the order of the documentation broken
> because flags are being grouped.
> 
> Thanks for the review :)
> Luis
> 

Sorry Johannes,

Update. checkpatch doesn't need fixing and it does see the documentation
independently of the order. It was my mistake.

Joe Perches pointed it out in this email branch:
https://lkml.org/lkml/2016/3/18/532


Thanks for looking into this, sorry for my blunder.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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


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