Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next repost] openvswitch: allow output of MPLS packets on tunnel vports
From: David Miller @ 2016-03-18 22:27 UTC (permalink / raw)
  To: simon.horman; +Cc: pshelar, jesse, netdev, dev
In-Reply-To: <1458010835-18555-1-git-send-email-simon.horman@netronome.com>

From: Simon Horman <simon.horman@netronome.com>
Date: Tue, 15 Mar 2016 12:00:35 +0900

> Currently output of MPLS packets on tunnel vports is not allowed by Open
> vSwitch. This is because historically encapsulation was done in such a way
> that the inner_protocol field of the skb needed to hold the inner protocol
> for both MPLS and tunnel encapsulation in order for GSO segmentation to be
> performed correctly.
> 
> Since b2acd1dc3949 ("openvswitch: Use regular GRE net_device instead of
> vport") Open vSwitch makes use of lwt to output to tunnel netdevs which
> perform encapsulation. As no drivers expose support for MPLS offloads this
> means that GSO packets are segmented in software by validate_xmit_skb(),
> which is called from __dev_queue_xmit(), before tunnel encapsulation occurs.
> This means that the inner protocol of MPLS is no longer needed by the time
> encapsulation occurs and the contention on the inner_protocol field of the
> skb no longer occurs.
> 
> Thus it is now safe to output MPLS to tunnel vports.
> 
> Signed-off-by: Simon Horman <simon.horman@netronome.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] netdev: Move octeon/octeon_mgmt driver to cavium directory.
From: David Miller @ 2016-03-18 22:25 UTC (permalink / raw)
  To: ddaney.cavm; +Cc: netdev, linux-kernel, linux-mips, david.daney
In-Reply-To: <1458003428-27632-1-git-send-email-ddaney.cavm@gmail.com>

From: David Daney <ddaney.cavm@gmail.com>
Date: Mon, 14 Mar 2016 17:57:08 -0700

> From: David Daney <david.daney@cavium.com>
> 
> No code changes.  Since OCTEON is a Cavium product, move the driver to
> the vendor directory to unclutter things a bit.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] openvswitch: reduce padding in struct sw_flow_key
From: Jesse Gross @ 2016-03-18 22:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: ovs dev, Linux Kernel Network Developers,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Jiri Benc, Joe Stringer,
	Justin Pettit, David S. Miller
In-Reply-To: <1458308084-268272-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>

On Fri, Mar 18, 2016 at 6:34 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> This means it's still too large really, we just don't warn about it any more,
> and will get the warning again once another member is added. My patch is a
> band-aid at best, but more work is needed here. One problem is that
> ovs_flow_cmd_new and ovs_flow_cmd_set have two copies of struct sw_flow_key on
> the stack, one of them nested within sw_flow_mask. If we could reduce that to
> a single instance, the stack usage would be cut in half here.

I think it should be possible to reduce those two functions to only
use a single instance of the structure.

For new flows, we're already allocating the key structure on the heap,
it seems like we could just translate the key into that rather than to
intermediate location on the stack. And when setting flows, I'm not
sure that we really need the mask at all - we don't do anything with
it other than check it against the actions but we really should be
using the original mask for that rather than the new one.

It seems like it would be preferable to go down that route rather than
this patch, since as you say, this patch is really only suppressing
the warning and doesn't have a significant impact on the actual stack
consumption. Plus, the ordering of the flow layout affects how much
data needs to be compared during packet lookup, so this change will
likely be bad for forwarding performance.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

^ permalink raw reply

* Re: [PATCH] ipv6: Fix the pmtu path for connected UDP socket
From: Cong Wang @ 2016-03-18 22:09 UTC (permalink / raw)
  To: Wei Wang
  Cc: Wei Wang, Martin KaFai Lau, Eric Dumazet, David Miller,
	Eric Dumazet, Linux Kernel Network Developers
In-Reply-To: <CAC15z3hhXxTQ+_Y6DaZcd-XVfCdDO9ZcQD6doNNKt-wHjTXAQw@mail.gmail.com>

On Fri, Mar 18, 2016 at 2:26 PM, Wei Wang <tracywwnj@gmail.com> wrote:
> 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.
>

Well, your patch touches all the callers of ip6_update_pmtu() , if you just
fix ip6_sk_update_pmtu() as I suggested, you only need to change one
function, ideally. And the ipv4 code is there, although I am not sure, it
looks like we can just mimic the logic here:

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index ed44663..b88c2ff 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1417,8 +1417,28 @@ EXPORT_SYMBOL_GPL(ip6_update_pmtu);

 void ip6_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, __be32 mtu)
 {
-       ip6_update_pmtu(skb, sock_net(sk), mtu,
-                       sk->sk_bound_dev_if, sk->sk_mark);
+       const struct ipv6hdr *iph = (struct ipv6hdr *) skb->data;
+       struct net *net = sock_net(sk);
+       struct dst_entry *dst;
+       struct flowi6 fl6;
+
+       bh_lock_sock(sk);
+
+       memset(&fl6, 0, sizeof(fl6));
+       fl6.flowi6_oif = sk->sk_bound_dev_if;
+       fl6.flowi6_mark = sk->sk_mark ? : IP6_REPLY_MARK(net, skb->mark);
+       fl6.daddr = iph->daddr;
+       fl6.saddr = iph->saddr;
+       fl6.flowlabel = ip6_flowinfo(iph);
+
+       dst = ip6_route_output(net, NULL, &fl6);
+       if (!dst->error)
+               __ip6_rt_update_pmtu(dst, NULL, iph, ntohl(mtu));
+
+       sk_dst_set(sk, &rt->dst);
+       bh_unlock_sock(sk);
+
+       dst_release(dst);
 }


Please don't judge me on the code, it could still miss a lot of things,
but it can show my idea...

^ permalink raw reply related

* Re: [PATCH] sctp: align MTU to a word
From: Eric Dumazet @ 2016-03-18 22:09 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: netdev, Neil Horman, Vlad Yasevich, linux-sctp
In-Reply-To: <1458337159-22792-1-git-send-email-marcelo.leitner@gmail.com>

On Fri, 2016-03-18 at 18:39 -0300, Marcelo Ricardo Leitner wrote:

> -		transport->pathmtu = dst_mtu(transport->dst);
> +		transport->pathmtu = dst_mtu(transport->dst) & ~3;


Well, surely a helper doing this would be better than spreading & ~3 all
over the places ;)

^ permalink raw reply

* 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


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