Netdev List
 help / color / mirror / Atom feed
* [PATCH 5/8] netfilter: ip6t_SYNPROXY: remove magic number for hop_limit
From: Pablo Neira Ayuso @ 2016-04-12 23:02 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1460502166-20340-1-git-send-email-pablo@netfilter.org>

From: Liping Zhang <liping.zhang@spreadtrum.com>

Replace '64' with the per-net ipv6_devconf_all's hop_limit when
building the ipv6 header.

Signed-off-by: Liping Zhang <liping.zhang@spreadtrum.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv6/netfilter/ip6t_SYNPROXY.c | 56 ++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/net/ipv6/netfilter/ip6t_SYNPROXY.c b/net/ipv6/netfilter/ip6t_SYNPROXY.c
index 3deed58..5d778dd 100644
--- a/net/ipv6/netfilter/ip6t_SYNPROXY.c
+++ b/net/ipv6/netfilter/ip6t_SYNPROXY.c
@@ -20,15 +20,16 @@
 #include <net/netfilter/nf_conntrack_synproxy.h>
 
 static struct ipv6hdr *
-synproxy_build_ip(struct sk_buff *skb, const struct in6_addr *saddr,
-				       const struct in6_addr *daddr)
+synproxy_build_ip(struct net *net, struct sk_buff *skb,
+		  const struct in6_addr *saddr,
+		  const struct in6_addr *daddr)
 {
 	struct ipv6hdr *iph;
 
 	skb_reset_network_header(skb);
 	iph = (struct ipv6hdr *)skb_put(skb, sizeof(*iph));
 	ip6_flow_hdr(iph, 0, 0);
-	iph->hop_limit	= 64;	//XXX
+	iph->hop_limit	= net->ipv6.devconf_all->hop_limit;
 	iph->nexthdr	= IPPROTO_TCP;
 	iph->saddr	= *saddr;
 	iph->daddr	= *daddr;
@@ -37,13 +38,12 @@ synproxy_build_ip(struct sk_buff *skb, const struct in6_addr *saddr,
 }
 
 static void
-synproxy_send_tcp(const struct synproxy_net *snet,
+synproxy_send_tcp(struct net *net,
 		  const struct sk_buff *skb, struct sk_buff *nskb,
 		  struct nf_conntrack *nfct, enum ip_conntrack_info ctinfo,
 		  struct ipv6hdr *niph, struct tcphdr *nth,
 		  unsigned int tcp_hdr_size)
 {
-	struct net *net = nf_ct_net(snet->tmpl);
 	struct dst_entry *dst;
 	struct flowi6 fl6;
 
@@ -84,7 +84,7 @@ free_nskb:
 }
 
 static void
-synproxy_send_client_synack(const struct synproxy_net *snet,
+synproxy_send_client_synack(struct net *net,
 			    const struct sk_buff *skb, const struct tcphdr *th,
 			    const struct synproxy_options *opts)
 {
@@ -103,7 +103,7 @@ synproxy_send_client_synack(const struct synproxy_net *snet,
 		return;
 	skb_reserve(nskb, MAX_TCP_HEADER);
 
-	niph = synproxy_build_ip(nskb, &iph->daddr, &iph->saddr);
+	niph = synproxy_build_ip(net, nskb, &iph->daddr, &iph->saddr);
 
 	skb_reset_transport_header(nskb);
 	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
@@ -121,15 +121,16 @@ synproxy_send_client_synack(const struct synproxy_net *snet,
 
 	synproxy_build_options(nth, opts);
 
-	synproxy_send_tcp(snet, skb, nskb, skb->nfct, IP_CT_ESTABLISHED_REPLY,
+	synproxy_send_tcp(net, skb, nskb, skb->nfct, IP_CT_ESTABLISHED_REPLY,
 			  niph, nth, tcp_hdr_size);
 }
 
 static void
-synproxy_send_server_syn(const struct synproxy_net *snet,
+synproxy_send_server_syn(struct net *net,
 			 const struct sk_buff *skb, const struct tcphdr *th,
 			 const struct synproxy_options *opts, u32 recv_seq)
 {
+	struct synproxy_net *snet = synproxy_pernet(net);
 	struct sk_buff *nskb;
 	struct ipv6hdr *iph, *niph;
 	struct tcphdr *nth;
@@ -144,7 +145,7 @@ synproxy_send_server_syn(const struct synproxy_net *snet,
 		return;
 	skb_reserve(nskb, MAX_TCP_HEADER);
 
-	niph = synproxy_build_ip(nskb, &iph->saddr, &iph->daddr);
+	niph = synproxy_build_ip(net, nskb, &iph->saddr, &iph->daddr);
 
 	skb_reset_transport_header(nskb);
 	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
@@ -165,12 +166,12 @@ synproxy_send_server_syn(const struct synproxy_net *snet,
 
 	synproxy_build_options(nth, opts);
 
-	synproxy_send_tcp(snet, skb, nskb, &snet->tmpl->ct_general, IP_CT_NEW,
+	synproxy_send_tcp(net, skb, nskb, &snet->tmpl->ct_general, IP_CT_NEW,
 			  niph, nth, tcp_hdr_size);
 }
 
 static void
-synproxy_send_server_ack(const struct synproxy_net *snet,
+synproxy_send_server_ack(struct net *net,
 			 const struct ip_ct_tcp *state,
 			 const struct sk_buff *skb, const struct tcphdr *th,
 			 const struct synproxy_options *opts)
@@ -189,7 +190,7 @@ synproxy_send_server_ack(const struct synproxy_net *snet,
 		return;
 	skb_reserve(nskb, MAX_TCP_HEADER);
 
-	niph = synproxy_build_ip(nskb, &iph->daddr, &iph->saddr);
+	niph = synproxy_build_ip(net, nskb, &iph->daddr, &iph->saddr);
 
 	skb_reset_transport_header(nskb);
 	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
@@ -205,11 +206,11 @@ synproxy_send_server_ack(const struct synproxy_net *snet,
 
 	synproxy_build_options(nth, opts);
 
-	synproxy_send_tcp(snet, skb, nskb, NULL, 0, niph, nth, tcp_hdr_size);
+	synproxy_send_tcp(net, skb, nskb, NULL, 0, niph, nth, tcp_hdr_size);
 }
 
 static void
-synproxy_send_client_ack(const struct synproxy_net *snet,
+synproxy_send_client_ack(struct net *net,
 			 const struct sk_buff *skb, const struct tcphdr *th,
 			 const struct synproxy_options *opts)
 {
@@ -227,7 +228,7 @@ synproxy_send_client_ack(const struct synproxy_net *snet,
 		return;
 	skb_reserve(nskb, MAX_TCP_HEADER);
 
-	niph = synproxy_build_ip(nskb, &iph->saddr, &iph->daddr);
+	niph = synproxy_build_ip(net, nskb, &iph->saddr, &iph->daddr);
 
 	skb_reset_transport_header(nskb);
 	nth = (struct tcphdr *)skb_put(nskb, tcp_hdr_size);
@@ -243,15 +244,16 @@ synproxy_send_client_ack(const struct synproxy_net *snet,
 
 	synproxy_build_options(nth, opts);
 
-	synproxy_send_tcp(snet, skb, nskb, skb->nfct, IP_CT_ESTABLISHED_REPLY,
+	synproxy_send_tcp(net, skb, nskb, skb->nfct, IP_CT_ESTABLISHED_REPLY,
 			  niph, nth, tcp_hdr_size);
 }
 
 static bool
-synproxy_recv_client_ack(const struct synproxy_net *snet,
+synproxy_recv_client_ack(struct net *net,
 			 const struct sk_buff *skb, const struct tcphdr *th,
 			 struct synproxy_options *opts, u32 recv_seq)
 {
+	struct synproxy_net *snet = synproxy_pernet(net);
 	int mss;
 
 	mss = __cookie_v6_check(ipv6_hdr(skb), th, ntohl(th->ack_seq) - 1);
@@ -267,7 +269,7 @@ synproxy_recv_client_ack(const struct synproxy_net *snet,
 	if (opts->options & XT_SYNPROXY_OPT_TIMESTAMP)
 		synproxy_check_timestamp_cookie(opts);
 
-	synproxy_send_server_syn(snet, skb, th, opts, recv_seq);
+	synproxy_send_server_syn(net, skb, th, opts, recv_seq);
 	return true;
 }
 
@@ -275,7 +277,8 @@ static unsigned int
 synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
 {
 	const struct xt_synproxy_info *info = par->targinfo;
-	struct synproxy_net *snet = synproxy_pernet(par->net);
+	struct net *net = par->net;
+	struct synproxy_net *snet = synproxy_pernet(net);
 	struct synproxy_options opts = {};
 	struct tcphdr *th, _th;
 
@@ -304,12 +307,12 @@ synproxy_tg6(struct sk_buff *skb, const struct xt_action_param *par)
 					  XT_SYNPROXY_OPT_SACK_PERM |
 					  XT_SYNPROXY_OPT_ECN);
 
-		synproxy_send_client_synack(snet, skb, th, &opts);
+		synproxy_send_client_synack(net, skb, th, &opts);
 		return NF_DROP;
 
 	} else if (th->ack && !(th->fin || th->rst || th->syn)) {
 		/* ACK from client */
-		synproxy_recv_client_ack(snet, skb, th, &opts, ntohl(th->seq));
+		synproxy_recv_client_ack(net, skb, th, &opts, ntohl(th->seq));
 		return NF_DROP;
 	}
 
@@ -320,7 +323,8 @@ static unsigned int ipv6_synproxy_hook(void *priv,
 				       struct sk_buff *skb,
 				       const struct nf_hook_state *nhs)
 {
-	struct synproxy_net *snet = synproxy_pernet(nhs->net);
+	struct net *net = nhs->net;
+	struct synproxy_net *snet = synproxy_pernet(net);
 	enum ip_conntrack_info ctinfo;
 	struct nf_conn *ct;
 	struct nf_conn_synproxy *synproxy;
@@ -384,7 +388,7 @@ static unsigned int ipv6_synproxy_hook(void *priv,
 			 * therefore we need to add 1 to make the SYN sequence
 			 * number match the one of first SYN.
 			 */
-			if (synproxy_recv_client_ack(snet, skb, th, &opts,
+			if (synproxy_recv_client_ack(net, skb, th, &opts,
 						     ntohl(th->seq) + 1))
 				this_cpu_inc(snet->stats->cookie_retrans);
 
@@ -410,12 +414,12 @@ static unsigned int ipv6_synproxy_hook(void *priv,
 				  XT_SYNPROXY_OPT_SACK_PERM);
 
 		swap(opts.tsval, opts.tsecr);
-		synproxy_send_server_ack(snet, state, skb, th, &opts);
+		synproxy_send_server_ack(net, state, skb, th, &opts);
 
 		nf_ct_seqadj_init(ct, ctinfo, synproxy->isn - ntohl(th->seq));
 
 		swap(opts.tsval, opts.tsecr);
-		synproxy_send_client_ack(snet, skb, th, &opts);
+		synproxy_send_client_ack(net, skb, th, &opts);
 
 		consume_skb(skb);
 		return NF_STOLEN;
-- 
2.1.4

^ permalink raw reply related

* [PATCH 7/8] netfilter: conntrack: de-inline nf_conntrack_eventmask_report
From: Pablo Neira Ayuso @ 2016-04-12 23:02 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1460502166-20340-1-git-send-email-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Way too large; move it to nf_conntrack_ecache.c.
Reduces total object size by 1216 byte on my machine.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_ecache.h | 66 ++++++-----------------------
 net/netfilter/nf_conntrack_ecache.c         | 54 +++++++++++++++++++++++
 2 files changed, 66 insertions(+), 54 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_ecache.h b/include/net/netfilter/nf_conntrack_ecache.h
index 57c8803..019a5b8 100644
--- a/include/net/netfilter/nf_conntrack_ecache.h
+++ b/include/net/netfilter/nf_conntrack_ecache.h
@@ -73,6 +73,8 @@ void nf_conntrack_unregister_notifier(struct net *net,
 				      struct nf_ct_event_notifier *nb);
 
 void nf_ct_deliver_cached_events(struct nf_conn *ct);
+int nf_conntrack_eventmask_report(unsigned int eventmask, struct nf_conn *ct,
+				  u32 portid, int report);
 
 static inline void
 nf_conntrack_event_cache(enum ip_conntrack_events event, struct nf_conn *ct)
@@ -91,69 +93,25 @@ nf_conntrack_event_cache(enum ip_conntrack_events event, struct nf_conn *ct)
 }
 
 static inline int
-nf_conntrack_eventmask_report(unsigned int eventmask,
-			      struct nf_conn *ct,
-			      u32 portid,
-			      int report)
-{
-	int ret = 0;
-	struct net *net = nf_ct_net(ct);
-	struct nf_ct_event_notifier *notify;
-	struct nf_conntrack_ecache *e;
-
-	rcu_read_lock();
-	notify = rcu_dereference(net->ct.nf_conntrack_event_cb);
-	if (notify == NULL)
-		goto out_unlock;
-
-	e = nf_ct_ecache_find(ct);
-	if (e == NULL)
-		goto out_unlock;
-
-	if (nf_ct_is_confirmed(ct) && !nf_ct_is_dying(ct)) {
-		struct nf_ct_event item = {
-			.ct 	= ct,
-			.portid	= e->portid ? e->portid : portid,
-			.report = report
-		};
-		/* This is a resent of a destroy event? If so, skip missed */
-		unsigned long missed = e->portid ? 0 : e->missed;
-
-		if (!((eventmask | missed) & e->ctmask))
-			goto out_unlock;
-
-		ret = notify->fcn(eventmask | missed, &item);
-		if (unlikely(ret < 0 || missed)) {
-			spin_lock_bh(&ct->lock);
-			if (ret < 0) {
-				/* This is a destroy event that has been
-				 * triggered by a process, we store the PORTID
-				 * to include it in the retransmission. */
-				if (eventmask & (1 << IPCT_DESTROY) &&
-				    e->portid == 0 && portid != 0)
-					e->portid = portid;
-				else
-					e->missed |= eventmask;
-			} else
-				e->missed &= ~missed;
-			spin_unlock_bh(&ct->lock);
-		}
-	}
-out_unlock:
-	rcu_read_unlock();
-	return ret;
-}
-
-static inline int
 nf_conntrack_event_report(enum ip_conntrack_events event, struct nf_conn *ct,
 			  u32 portid, int report)
 {
+	const struct net *net = nf_ct_net(ct);
+
+	if (!rcu_access_pointer(net->ct.nf_conntrack_event_cb))
+		return 0;
+
 	return nf_conntrack_eventmask_report(1 << event, ct, portid, report);
 }
 
 static inline int
 nf_conntrack_event(enum ip_conntrack_events event, struct nf_conn *ct)
 {
+	const struct net *net = nf_ct_net(ct);
+
+	if (!rcu_access_pointer(net->ct.nf_conntrack_event_cb))
+		return 0;
+
 	return nf_conntrack_eventmask_report(1 << event, ct, 0, 0);
 }
 
diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
index 4e78c57..a0ebab9 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -113,6 +113,60 @@ static void ecache_work(struct work_struct *work)
 		schedule_delayed_work(&ctnet->ecache_dwork, delay);
 }
 
+int nf_conntrack_eventmask_report(unsigned int eventmask, struct nf_conn *ct,
+				  u32 portid, int report)
+{
+	int ret = 0;
+	struct net *net = nf_ct_net(ct);
+	struct nf_ct_event_notifier *notify;
+	struct nf_conntrack_ecache *e;
+
+	rcu_read_lock();
+	notify = rcu_dereference(net->ct.nf_conntrack_event_cb);
+	if (!notify)
+		goto out_unlock;
+
+	e = nf_ct_ecache_find(ct);
+	if (!e)
+		goto out_unlock;
+
+	if (nf_ct_is_confirmed(ct) && !nf_ct_is_dying(ct)) {
+		struct nf_ct_event item = {
+			.ct	= ct,
+			.portid	= e->portid ? e->portid : portid,
+			.report = report
+		};
+		/* This is a resent of a destroy event? If so, skip missed */
+		unsigned long missed = e->portid ? 0 : e->missed;
+
+		if (!((eventmask | missed) & e->ctmask))
+			goto out_unlock;
+
+		ret = notify->fcn(eventmask | missed, &item);
+		if (unlikely(ret < 0 || missed)) {
+			spin_lock_bh(&ct->lock);
+			if (ret < 0) {
+				/* This is a destroy event that has been
+				 * triggered by a process, we store the PORTID
+				 * to include it in the retransmission.
+				 */
+				if (eventmask & (1 << IPCT_DESTROY) &&
+				    e->portid == 0 && portid != 0)
+					e->portid = portid;
+				else
+					e->missed |= eventmask;
+			} else {
+				e->missed &= ~missed;
+			}
+			spin_unlock_bh(&ct->lock);
+		}
+	}
+out_unlock:
+	rcu_read_unlock();
+	return ret;
+}
+EXPORT_SYMBOL_GPL(nf_conntrack_eventmask_report);
+
 /* deliver cached events and clear cache entry - must be called with locally
  * disabled softirqs */
 void nf_ct_deliver_cached_events(struct nf_conn *ct)
-- 
2.1.4

^ permalink raw reply related

* [PATCH 8/8] netfilter: conntrack: move expectation event helper to ecache.c
From: Pablo Neira Ayuso @ 2016-04-12 23:02 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1460502166-20340-1-git-send-email-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Not performance critical, it is only invoked when an expectation is
added/destroyed.

While at it, kill unused nf_ct_expect_event() wrapper.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_ecache.h | 42 +++--------------------------
 net/netfilter/nf_conntrack_ecache.c         | 30 +++++++++++++++++++++
 2 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_ecache.h b/include/net/netfilter/nf_conntrack_ecache.h
index 019a5b8..fa36447 100644
--- a/include/net/netfilter/nf_conntrack_ecache.h
+++ b/include/net/netfilter/nf_conntrack_ecache.h
@@ -130,43 +130,9 @@ int nf_ct_expect_register_notifier(struct net *net,
 void nf_ct_expect_unregister_notifier(struct net *net,
 				      struct nf_exp_event_notifier *nb);
 
-static inline void
-nf_ct_expect_event_report(enum ip_conntrack_expect_events event,
-			  struct nf_conntrack_expect *exp,
-			  u32 portid,
-			  int report)
-{
-	struct net *net = nf_ct_exp_net(exp);
-	struct nf_exp_event_notifier *notify;
-	struct nf_conntrack_ecache *e;
-
-	rcu_read_lock();
-	notify = rcu_dereference(net->ct.nf_expect_event_cb);
-	if (notify == NULL)
-		goto out_unlock;
-
-	e = nf_ct_ecache_find(exp->master);
-	if (e == NULL)
-		goto out_unlock;
-
-	if (e->expmask & (1 << event)) {
-		struct nf_exp_event item = {
-			.exp	= exp,
-			.portid	= portid,
-			.report = report
-		};
-		notify->fcn(1 << event, &item);
-	}
-out_unlock:
-	rcu_read_unlock();
-}
-
-static inline void
-nf_ct_expect_event(enum ip_conntrack_expect_events event,
-		   struct nf_conntrack_expect *exp)
-{
-	nf_ct_expect_event_report(event, exp, 0, 0);
-}
+void nf_ct_expect_event_report(enum ip_conntrack_expect_events event,
+			       struct nf_conntrack_expect *exp,
+			       u32 portid, int report);
 
 int nf_conntrack_ecache_pernet_init(struct net *net);
 void nf_conntrack_ecache_pernet_fini(struct net *net);
@@ -203,8 +169,6 @@ static inline int nf_conntrack_event_report(enum ip_conntrack_events event,
 					    u32 portid,
 					    int report) { return 0; }
 static inline void nf_ct_deliver_cached_events(const struct nf_conn *ct) {}
-static inline void nf_ct_expect_event(enum ip_conntrack_expect_events event,
-				      struct nf_conntrack_expect *exp) {}
 static inline void nf_ct_expect_event_report(enum ip_conntrack_expect_events e,
 					     struct nf_conntrack_expect *exp,
  					     u32 portid,
diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
index a0ebab9..d28011b 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -221,6 +221,36 @@ out_unlock:
 }
 EXPORT_SYMBOL_GPL(nf_ct_deliver_cached_events);
 
+void nf_ct_expect_event_report(enum ip_conntrack_expect_events event,
+			       struct nf_conntrack_expect *exp,
+			       u32 portid, int report)
+
+{
+	struct net *net = nf_ct_exp_net(exp);
+	struct nf_exp_event_notifier *notify;
+	struct nf_conntrack_ecache *e;
+
+	rcu_read_lock();
+	notify = rcu_dereference(net->ct.nf_expect_event_cb);
+	if (!notify)
+		goto out_unlock;
+
+	e = nf_ct_ecache_find(exp->master);
+	if (!e)
+		goto out_unlock;
+
+	if (e->expmask & (1 << event)) {
+		struct nf_exp_event item = {
+			.exp	= exp,
+			.portid	= portid,
+			.report = report
+		};
+		notify->fcn(1 << event, &item);
+	}
+out_unlock:
+	rcu_read_unlock();
+}
+
 int nf_conntrack_register_notifier(struct net *net,
 				   struct nf_ct_event_notifier *new)
 {
-- 
2.1.4

^ permalink raw reply related

* [PATCH v2 net-next 5/7] net: dsa: Rename DSA probe function.
From: Andrew Lunn @ 2016-04-13  0:40 UTC (permalink / raw)
  To: David Miller; +Cc: Florian Fainelli, netdev, Vivien Didelot, Andrew Lunn
In-Reply-To: <1460508045-20254-1-git-send-email-andrew@lunn.ch>

Rename the function called from the DSA to perform a probe for the
switch. This makes the normal _probe() name available for a standard
Linux device driver probe function.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/bcm_sf2.c   | 7 ++++---
 drivers/net/dsa/mv88e6060.c | 7 ++++---
 drivers/net/dsa/mv88e6123.c | 7 ++++---
 drivers/net/dsa/mv88e6131.c | 7 ++++---
 drivers/net/dsa/mv88e6171.c | 7 ++++---
 drivers/net/dsa/mv88e6352.c | 7 ++++---
 6 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 7d62802a66d5..50caa525cda3 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -135,8 +135,9 @@ static int bcm_sf2_sw_get_sset_count(struct dsa_switch *ds)
 	return BCM_SF2_STATS_SIZE;
 }
 
-static char *bcm_sf2_sw_probe(struct device *dsa_dev, struct device *host_dev,
-			      int sw_addr, void **_priv)
+static char *bcm_sf2_sw_drv_probe(struct device *dsa_dev,
+				  struct device *host_dev,
+				  int sw_addr, void **_priv)
 {
 	struct bcm_sf2_priv *priv;
 
@@ -1370,7 +1371,7 @@ static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
 
 static struct dsa_switch_driver bcm_sf2_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_BRCM,
-	.probe			= bcm_sf2_sw_probe,
+	.probe			= bcm_sf2_sw_drv_probe,
 	.setup			= bcm_sf2_sw_setup,
 	.set_addr		= bcm_sf2_sw_set_addr,
 	.get_phy_flags		= bcm_sf2_sw_get_phy_flags,
diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index 46fe5dc65a99..adb608ccd9aa 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -69,8 +69,9 @@ static char *mv88e6060_get_name(struct mii_bus *bus, int sw_addr)
 	return NULL;
 }
 
-static char *mv88e6060_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr, void **_priv)
+static char *mv88e6060_drv_probe(struct device *dsa_dev,
+				 struct device *host_dev,
+				 int sw_addr, void **_priv)
 {
 	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
 	struct mv88e6060_priv *priv;
@@ -248,7 +249,7 @@ mv88e6060_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
 
 static struct dsa_switch_driver mv88e6060_switch_driver = {
 	.tag_protocol	= DSA_TAG_PROTO_TRAILER,
-	.probe		= mv88e6060_probe,
+	.probe		= mv88e6060_drv_probe,
 	.setup		= mv88e6060_setup,
 	.set_addr	= mv88e6060_set_addr,
 	.phy_read	= mv88e6060_phy_read,
diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 8aaac0894752..c34283d929c4 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -29,8 +29,9 @@ static const struct mv88e6xxx_switch_id mv88e6123_table[] = {
 	{ PORT_SWITCH_ID_6165_A2, "Marvell 88e6165 (A2)" },
 };
 
-static char *mv88e6123_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr, void **priv)
+static char *mv88e6123_drv_probe(struct device *dsa_dev,
+				 struct device *host_dev,
+				 int sw_addr, void **priv)
 {
 	return mv88e6xxx_drv_probe(dsa_dev, host_dev, sw_addr, priv,
 				   mv88e6123_table,
@@ -106,7 +107,7 @@ static int mv88e6123_setup(struct dsa_switch *ds)
 
 struct dsa_switch_driver mv88e6123_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_EDSA,
-	.probe			= mv88e6123_probe,
+	.probe			= mv88e6123_drv_probe,
 	.setup			= mv88e6123_setup,
 	.set_addr		= mv88e6xxx_set_addr_indirect,
 	.phy_read		= mv88e6xxx_phy_read,
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 9e6edf94a855..f5d75fce1e96 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -25,8 +25,9 @@ static const struct mv88e6xxx_switch_id mv88e6131_table[] = {
 	{ PORT_SWITCH_ID_6185, "Marvell 88E6185" },
 };
 
-static char *mv88e6131_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr, void **priv)
+static char *mv88e6131_drv_probe(struct device *dsa_dev,
+				 struct device *host_dev,
+				 int sw_addr, void **priv)
 {
 	return mv88e6xxx_drv_probe(dsa_dev, host_dev, sw_addr, priv,
 				   mv88e6131_table,
@@ -163,7 +164,7 @@ mv88e6131_phy_write(struct dsa_switch *ds,
 
 struct dsa_switch_driver mv88e6131_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_DSA,
-	.probe			= mv88e6131_probe,
+	.probe			= mv88e6131_drv_probe,
 	.setup			= mv88e6131_setup,
 	.set_addr		= mv88e6xxx_set_addr_direct,
 	.phy_read		= mv88e6131_phy_read,
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 6ab86071391f..f5622506cdfa 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -24,8 +24,9 @@ static const struct mv88e6xxx_switch_id mv88e6171_table[] = {
 	{ PORT_SWITCH_ID_6351, "Marvell 88E6351" },
 };
 
-static char *mv88e6171_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr, void **priv)
+static char *mv88e6171_drv_probe(struct device *dsa_dev,
+				 struct device *host_dev,
+				 int sw_addr, void **priv)
 {
 	return mv88e6xxx_drv_probe(dsa_dev, host_dev, sw_addr, priv,
 				   mv88e6171_table,
@@ -92,7 +93,7 @@ static int mv88e6171_setup(struct dsa_switch *ds)
 
 struct dsa_switch_driver mv88e6171_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_EDSA,
-	.probe			= mv88e6171_probe,
+	.probe			= mv88e6171_drv_probe,
 	.setup			= mv88e6171_setup,
 	.set_addr		= mv88e6xxx_set_addr_indirect,
 	.phy_read		= mv88e6xxx_phy_read_indirect,
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 764b10ffb631..e54ee27db129 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -37,8 +37,9 @@ static const struct mv88e6xxx_switch_id mv88e6352_table[] = {
 	{ PORT_SWITCH_ID_6352_A1, "Marvell 88E6352 (A1)" },
 };
 
-static char *mv88e6352_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr, void **priv)
+static char *mv88e6352_drv_probe(struct device *dsa_dev,
+				 struct device *host_dev,
+				 int sw_addr, void **priv)
 {
 	return mv88e6xxx_drv_probe(dsa_dev, host_dev, sw_addr, priv,
 				   mv88e6352_table,
@@ -306,7 +307,7 @@ static int mv88e6352_set_eeprom(struct dsa_switch *ds,
 
 struct dsa_switch_driver mv88e6352_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_EDSA,
-	.probe			= mv88e6352_probe,
+	.probe			= mv88e6352_drv_probe,
 	.setup			= mv88e6352_setup,
 	.set_addr		= mv88e6xxx_set_addr_indirect,
 	.phy_read		= mv88e6xxx_phy_read_indirect,
-- 
2.7.0

^ permalink raw reply related

* [PATCH v2 net-next 7/7] dsa: mv88e6xxx: Use bus in mv88e6xxx_lookup_name()
From: Andrew Lunn @ 2016-04-13  0:40 UTC (permalink / raw)
  To: David Miller; +Cc: Florian Fainelli, netdev, Vivien Didelot, Andrew Lunn
In-Reply-To: <1460508045-20254-1-git-send-email-andrew@lunn.ch>

mv88e6xxx_lookup_name() returns the model name of a switch at a given
address on an MII bus. Using mii_bus to identify the bus rather than
the host device is more logical, so change the parameter.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index c242ffd8eb09..9985a0cf31f1 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3068,11 +3068,10 @@ int mv88e6xxx_get_temp_alarm(struct dsa_switch *ds, bool *alarm)
 }
 #endif /* CONFIG_NET_DSA_HWMON */
 
-static char *mv88e6xxx_lookup_name(struct device *host_dev, int sw_addr,
+static char *mv88e6xxx_lookup_name(struct mii_bus *bus, int sw_addr,
 				   const struct mv88e6xxx_switch_id *table,
 				   unsigned int num)
 {
-	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
 	int i, ret;
 
 	if (!bus)
@@ -3090,7 +3089,8 @@ static char *mv88e6xxx_lookup_name(struct device *host_dev, int sw_addr,
 	/* Look up only the product number */
 	for (i = 0; i < num; ++i) {
 		if (table[i].id == (ret & PORT_SWITCH_ID_PROD_NUM_MASK)) {
-			dev_warn(host_dev, "unknown revision %d, using base switch 0x%x\n",
+			dev_warn(&bus->dev,
+				 "unknown revision %d, using base switch 0x%x\n",
 				 ret & PORT_SWITCH_ID_REV_MASK,
 				 ret & PORT_SWITCH_ID_PROD_NUM_MASK);
 			return table[i].name;
@@ -3106,9 +3106,13 @@ char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
 			  unsigned int num)
 {
 	struct mv88e6xxx_priv_state *ps;
+	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
 	char *name;
 
-	name = mv88e6xxx_lookup_name(host_dev, sw_addr, table, num);
+	if (!bus)
+		return NULL;
+
+	name = mv88e6xxx_lookup_name(bus, sw_addr, table, num);
 	if (name) {
 		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
 		if (!ps)
-- 
2.7.0

^ permalink raw reply related

* [PATCH v2 net-next 3/7] net: dsa: Remove allocation of driver private memory
From: Andrew Lunn @ 2016-04-13  0:40 UTC (permalink / raw)
  To: David Miller; +Cc: Florian Fainelli, netdev, Vivien Didelot, Andrew Lunn
In-Reply-To: <1460508045-20254-1-git-send-email-andrew@lunn.ch>

The drivers now allocate their own memory for private usage. Remove
the allocation from the core code.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/net/dsa.h | 1 -
 net/dsa/dsa.c     | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7bc7bd9b5ded..165c2e10615c 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -213,7 +213,6 @@ struct dsa_switch_driver {
 	struct list_head	list;
 
 	enum dsa_tag_protocol	tag_protocol;
-	int			priv_size;
 
 	/*
 	 * Probing and setup.
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 7ef8a92a9e39..14bf12f637d2 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -402,7 +402,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 	/*
 	 * Allocate and initialise switch state.
 	 */
-	ds = devm_kzalloc(parent, sizeof(*ds) + drv->priv_size, GFP_KERNEL);
+	ds = devm_kzalloc(parent, sizeof(*ds), GFP_KERNEL);
 	if (ds == NULL)
 		return ERR_PTR(-ENOMEM);
 
-- 
2.7.0

^ permalink raw reply related

* [PATCH v2 net-next 2/7] net: dsa: Have the switch driver allocate there own private memory
From: Andrew Lunn @ 2016-04-13  0:40 UTC (permalink / raw)
  To: David Miller; +Cc: Florian Fainelli, netdev, Vivien Didelot, Andrew Lunn
In-Reply-To: <1460508045-20254-1-git-send-email-andrew@lunn.ch>

Now the switch devices have a dev pointer, make use of it for allocating
the drivers private data structures using a devm_kzalloc().

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/bcm_sf2.c   | 10 ++++++++--
 drivers/net/dsa/mv88e6060.c |  3 ++-
 drivers/net/dsa/mv88e6123.c | 17 ++++++++++++++---
 drivers/net/dsa/mv88e6131.c | 17 ++++++++++++++---
 drivers/net/dsa/mv88e6171.c | 17 ++++++++++++++---
 drivers/net/dsa/mv88e6352.c | 17 ++++++++++++++---
 drivers/net/dsa/mv88e6xxx.c |  6 ++++--
 drivers/net/dsa/mv88e6xxx.h |  3 +++
 include/net/dsa.h           | 10 ++++++++--
 net/dsa/dsa.c               |  8 +++++---
 10 files changed, 86 insertions(+), 22 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 18a79579141f..7d62802a66d5 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -136,8 +136,15 @@ static int bcm_sf2_sw_get_sset_count(struct dsa_switch *ds)
 }
 
 static char *bcm_sf2_sw_probe(struct device *dsa_dev, struct device *host_dev,
-			      int sw_addr)
+			      int sw_addr, void **_priv)
 {
+	struct bcm_sf2_priv *priv;
+
+	priv = devm_kzalloc(dsa_dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return NULL;
+	*_priv = priv;
+
 	return "Broadcom Starfighter 2";
 }
 
@@ -1363,7 +1370,6 @@ static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
 
 static struct dsa_switch_driver bcm_sf2_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_BRCM,
-	.priv_size		= sizeof(struct bcm_sf2_priv),
 	.probe			= bcm_sf2_sw_probe,
 	.setup			= bcm_sf2_sw_setup,
 	.set_addr		= bcm_sf2_sw_set_addr,
diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index 34d0f9fe19db..41195f1417f1 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -58,11 +58,12 @@ static int reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
 	})
 
 static char *mv88e6060_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr)
+			     int sw_addr, void **priv)
 {
 	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
 	int ret;
 
+	*priv = NULL;
 	if (bus == NULL)
 		return NULL;
 
diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index ede4c6f0b31a..bcab3ef22448 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -30,10 +30,20 @@ static const struct mv88e6xxx_switch_id mv88e6123_table[] = {
 };
 
 static char *mv88e6123_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr)
+			     int sw_addr, void **priv)
 {
-	return mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6123_table,
+	struct mv88e6xxx_priv_state *ps;
+	char *name;
+
+	name = mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6123_table,
 				     ARRAY_SIZE(mv88e6123_table));
+	if (name) {
+		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
+		if (!ps)
+			return NULL;
+		*priv = ps;
+	}
+	return name;
 }
 
 static int mv88e6123_setup_global(struct dsa_switch *ds)
@@ -74,6 +84,8 @@ static int mv88e6123_setup(struct dsa_switch *ds)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
+	ps->ds = ds;
+
 	ret = mv88e6xxx_setup_common(ds);
 	if (ret < 0)
 		return ret;
@@ -103,7 +115,6 @@ static int mv88e6123_setup(struct dsa_switch *ds)
 
 struct dsa_switch_driver mv88e6123_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_EDSA,
-	.priv_size		= sizeof(struct mv88e6xxx_priv_state),
 	.probe			= mv88e6123_probe,
 	.setup			= mv88e6123_setup,
 	.set_addr		= mv88e6xxx_set_addr_indirect,
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index bfadfd2cbb8d..b9f9b009f65a 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -26,10 +26,20 @@ static const struct mv88e6xxx_switch_id mv88e6131_table[] = {
 };
 
 static char *mv88e6131_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr)
+			     int sw_addr, void **priv)
 {
-	return mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6131_table,
+	struct mv88e6xxx_priv_state *ps;
+	char *name;
+
+	name = mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6131_table,
 				     ARRAY_SIZE(mv88e6131_table));
+	if (name) {
+		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
+		if (!ps)
+			return NULL;
+		*priv = ps;
+	}
+	return name;
 }
 
 static int mv88e6131_setup_global(struct dsa_switch *ds)
@@ -92,6 +102,8 @@ static int mv88e6131_setup(struct dsa_switch *ds)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
+	ps->ds = ds;
+
 	ret = mv88e6xxx_setup_common(ds);
 	if (ret < 0)
 		return ret;
@@ -160,7 +172,6 @@ mv88e6131_phy_write(struct dsa_switch *ds,
 
 struct dsa_switch_driver mv88e6131_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_DSA,
-	.priv_size		= sizeof(struct mv88e6xxx_priv_state),
 	.probe			= mv88e6131_probe,
 	.setup			= mv88e6131_setup,
 	.set_addr		= mv88e6xxx_set_addr_direct,
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index fb35d3ac1644..15200928cecc 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -25,10 +25,20 @@ static const struct mv88e6xxx_switch_id mv88e6171_table[] = {
 };
 
 static char *mv88e6171_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr)
+			     int sw_addr, void **priv)
 {
-	return mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6171_table,
+	struct mv88e6xxx_priv_state *ps;
+	char *name;
+
+	name = mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6171_table,
 				     ARRAY_SIZE(mv88e6171_table));
+		if (name) {
+		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
+		if (!ps)
+			return NULL;
+		*priv = ps;
+	}
+	return name;
 }
 
 static int mv88e6171_setup_global(struct dsa_switch *ds)
@@ -70,6 +80,8 @@ static int mv88e6171_setup(struct dsa_switch *ds)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
+	ps->ds = ds;
+
 	ret = mv88e6xxx_setup_common(ds);
 	if (ret < 0)
 		return ret;
@@ -89,7 +101,6 @@ static int mv88e6171_setup(struct dsa_switch *ds)
 
 struct dsa_switch_driver mv88e6171_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_EDSA,
-	.priv_size		= sizeof(struct mv88e6xxx_priv_state),
 	.probe			= mv88e6171_probe,
 	.setup			= mv88e6171_setup,
 	.set_addr		= mv88e6xxx_set_addr_indirect,
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 577ab2cfa944..7081a78a67e1 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -38,10 +38,20 @@ static const struct mv88e6xxx_switch_id mv88e6352_table[] = {
 };
 
 static char *mv88e6352_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr)
+			     int sw_addr, void **priv)
 {
-	return mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6352_table,
+	struct mv88e6xxx_priv_state *ps;
+	char *name;
+
+	name = mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6352_table,
 				     ARRAY_SIZE(mv88e6352_table));
+	if (name) {
+		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
+		if (!ps)
+			return NULL;
+		*priv = ps;
+	}
+	return name;
 }
 
 static int mv88e6352_setup_global(struct dsa_switch *ds)
@@ -82,6 +92,8 @@ static int mv88e6352_setup(struct dsa_switch *ds)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
+	ps->ds = ds;
+
 	ret = mv88e6xxx_setup_common(ds);
 	if (ret < 0)
 		return ret;
@@ -303,7 +315,6 @@ static int mv88e6352_set_eeprom(struct dsa_switch *ds,
 
 struct dsa_switch_driver mv88e6352_switch_driver = {
 	.tag_protocol		= DSA_TAG_PROTO_EDSA,
-	.priv_size		= sizeof(struct mv88e6xxx_priv_state),
 	.probe			= mv88e6352_probe,
 	.setup			= mv88e6352_setup,
 	.set_addr		= mv88e6xxx_set_addr_indirect,
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 62320fca6712..085fc4a49eb2 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -281,7 +281,7 @@ static void mv88e6xxx_ppu_reenable_work(struct work_struct *ugly)
 
 	ps = container_of(ugly, struct mv88e6xxx_priv_state, ppu_work);
 	if (mutex_trylock(&ps->ppu_mutex)) {
-		struct dsa_switch *ds = ((struct dsa_switch *)ps) - 1;
+		struct dsa_switch *ds = ps->ds;
 
 		if (mv88e6xxx_ppu_enable(ds) == 0)
 			ps->ppu_disabled = 0;
@@ -2322,7 +2322,7 @@ static void mv88e6xxx_bridge_work(struct work_struct *work)
 	int port;
 
 	ps = container_of(work, struct mv88e6xxx_priv_state, bridge_work);
-	ds = ((struct dsa_switch *)ps) - 1;
+	ds = ps->ds;
 
 	mutex_lock(&ps->smi_mutex);
 
@@ -2670,6 +2670,8 @@ int mv88e6xxx_setup_common(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
+	ps->ds = ds;
+
 	mutex_init(&ps->smi_mutex);
 
 	ps->id = REG_READ(REG_PORT(0), PORT_SWITCH_ID) & 0xfff0;
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 236bcaa606e7..0322e3e0e7d9 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -397,6 +397,9 @@ struct mv88e6xxx_priv_port {
 };
 
 struct mv88e6xxx_priv_state {
+	/* The dsa_switch this private structure is related to */
+	struct dsa_switch *ds;
+
 	/* When using multi-chip addressing, this mutex protects
 	 * access to the indirect access registers.  (In single-chip
 	 * mode, this mutex is effectively useless.)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 0f9f6f38f255..7bc7bd9b5ded 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -130,6 +130,12 @@ struct dsa_switch {
 	int			index;
 
 	/*
+	 * Give the switch driver somewhere to hang its private data
+	 * structure.
+	 */
+	void *priv;
+
+	/*
 	 * Tagging protocol understood by this switch
 	 */
 	enum dsa_tag_protocol	tag_protocol;
@@ -213,7 +219,7 @@ struct dsa_switch_driver {
 	 * Probing and setup.
 	 */
 	char	*(*probe)(struct device *dsa_dev, struct device *host_dev,
-			  int sw_addr);
+			  int sw_addr, void **priv);
 	int	(*setup)(struct dsa_switch *ds);
 	int	(*set_addr)(struct dsa_switch *ds, u8 *addr);
 	u32	(*get_phy_flags)(struct dsa_switch *ds, int port);
@@ -342,7 +348,7 @@ struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev);
 
 static inline void *ds_to_priv(struct dsa_switch *ds)
 {
-	return (void *)(ds + 1);
+	return ds->priv;
 }
 
 static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index c06275311cb2..7ef8a92a9e39 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -52,7 +52,7 @@ EXPORT_SYMBOL_GPL(unregister_switch_driver);
 
 static struct dsa_switch_driver *
 dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
-		 char **_name)
+		 char **_name, void **priv)
 {
 	struct dsa_switch_driver *ret;
 	struct list_head *list;
@@ -67,7 +67,7 @@ dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
 
 		drv = list_entry(list, struct dsa_switch_driver, list);
 
-		name = drv->probe(parent, host_dev, sw_addr);
+		name = drv->probe(parent, host_dev, sw_addr, priv);
 		if (name != NULL) {
 			ret = drv;
 			break;
@@ -384,11 +384,12 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 	struct dsa_switch *ds;
 	int ret;
 	char *name;
+	void *priv;
 
 	/*
 	 * Probe for switch model.
 	 */
-	drv = dsa_switch_probe(parent, host_dev, pd->sw_addr, &name);
+	drv = dsa_switch_probe(parent, host_dev, pd->sw_addr, &name, &priv);
 	if (drv == NULL) {
 		netdev_err(dst->master_netdev, "[%d]: could not detect attached switch\n",
 			   index);
@@ -409,6 +410,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 	ds->index = index;
 	ds->pd = pd;
 	ds->drv = drv;
+	ds->priv = priv;
 	ds->tag_protocol = drv->tag_protocol;
 	ds->master_dev = host_dev;
 
-- 
2.7.0

^ permalink raw reply related

* [PATCH v2 net-next 4/7] net: dsa: Keep the mii bus and address in the private structure
From: Andrew Lunn @ 2016-04-13  0:40 UTC (permalink / raw)
  To: David Miller; +Cc: Florian Fainelli, netdev, Vivien Didelot, Andrew Lunn
In-Reply-To: <1460508045-20254-1-git-send-email-andrew@lunn.ch>

Rather than looking up the mii bus and address every time, do it once
at probe, and keep it in the private structure. Centralise this probe
code in mv88e6xxx.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6060.c | 44 ++++++++++++++++++++++++++------------------
 drivers/net/dsa/mv88e6060.h | 11 +++++++++++
 drivers/net/dsa/mv88e6123.c | 15 +++------------
 drivers/net/dsa/mv88e6131.c | 15 +++------------
 drivers/net/dsa/mv88e6171.c | 15 +++------------
 drivers/net/dsa/mv88e6352.c | 15 +++------------
 drivers/net/dsa/mv88e6xxx.c | 43 +++++++++++++++++++++++++++++--------------
 drivers/net/dsa/mv88e6xxx.h | 14 +++++++++++---
 8 files changed, 89 insertions(+), 83 deletions(-)

diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index 41195f1417f1..46fe5dc65a99 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -19,12 +19,9 @@
 
 static int reg_read(struct dsa_switch *ds, int addr, int reg)
 {
-	struct mii_bus *bus = dsa_host_dev_to_mii_bus(ds->master_dev);
+	struct mv88e6060_priv *priv = ds_to_priv(ds);
 
-	if (bus == NULL)
-		return -EINVAL;
-
-	return mdiobus_read_nested(bus, ds->pd->sw_addr + addr, reg);
+	return mdiobus_read_nested(priv->bus, priv->sw_addr + addr, reg);
 }
 
 #define REG_READ(addr, reg)					\
@@ -40,12 +37,9 @@ static int reg_read(struct dsa_switch *ds, int addr, int reg)
 
 static int reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
 {
-	struct mii_bus *bus = dsa_host_dev_to_mii_bus(ds->master_dev);
-
-	if (bus == NULL)
-		return -EINVAL;
+	struct mv88e6060_priv *priv = ds_to_priv(ds);
 
-	return mdiobus_write_nested(bus, ds->pd->sw_addr + addr, reg, val);
+	return mdiobus_write_nested(priv->bus, priv->sw_addr + addr, reg, val);
 }
 
 #define REG_WRITE(addr, reg, val)				\
@@ -57,16 +51,10 @@ static int reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
 			return __ret;				\
 	})
 
-static char *mv88e6060_probe(struct device *dsa_dev, struct device *host_dev,
-			     int sw_addr, void **priv)
+static char *mv88e6060_get_name(struct mii_bus *bus, int sw_addr)
 {
-	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
 	int ret;
 
-	*priv = NULL;
-	if (bus == NULL)
-		return NULL;
-
 	ret = mdiobus_read(bus, sw_addr + REG_PORT(0), PORT_SWITCH_ID);
 	if (ret >= 0) {
 		if (ret == PORT_SWITCH_ID_6060)
@@ -81,6 +69,26 @@ static char *mv88e6060_probe(struct device *dsa_dev, struct device *host_dev,
 	return NULL;
 }
 
+static char *mv88e6060_probe(struct device *dsa_dev, struct device *host_dev,
+			     int sw_addr, void **_priv)
+{
+	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
+	struct mv88e6060_priv *priv;
+	char *name;
+
+	name = mv88e6060_get_name(bus, sw_addr);
+	if (name) {
+		priv = devm_kzalloc(dsa_dev, sizeof(*priv), GFP_KERNEL);
+		if (!priv)
+			return NULL;
+		*_priv = priv;
+		priv->bus = bus;
+		priv->sw_addr = sw_addr;
+	}
+
+	return name;
+}
+
 static int mv88e6060_switch_reset(struct dsa_switch *ds)
 {
 	int i;
@@ -176,8 +184,8 @@ static int mv88e6060_setup_port(struct dsa_switch *ds, int p)
 
 static int mv88e6060_setup(struct dsa_switch *ds)
 {
-	int i;
 	int ret;
+	int i;
 
 	ret = mv88e6060_switch_reset(ds);
 	if (ret < 0)
diff --git a/drivers/net/dsa/mv88e6060.h b/drivers/net/dsa/mv88e6060.h
index cc9b2ed4aff4..10249bd16292 100644
--- a/drivers/net/dsa/mv88e6060.h
+++ b/drivers/net/dsa/mv88e6060.h
@@ -108,4 +108,15 @@
 #define GLOBAL_ATU_MAC_23	0x0e
 #define GLOBAL_ATU_MAC_45	0x0f
 
+struct mv88e6060_priv {
+	/* MDIO bus and address on bus to use. When in single chip
+	 * mode, address is 0, and the switch uses multiple addresses
+	 * on the bus.  When in multi-chip mode, the switch uses a
+	 * single address which contains two registers used for
+	 * indirect access to more registers.
+	 */
+	struct mii_bus *bus;
+	int sw_addr;
+};
+
 #endif
diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index bcab3ef22448..8aaac0894752 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -32,18 +32,9 @@ static const struct mv88e6xxx_switch_id mv88e6123_table[] = {
 static char *mv88e6123_probe(struct device *dsa_dev, struct device *host_dev,
 			     int sw_addr, void **priv)
 {
-	struct mv88e6xxx_priv_state *ps;
-	char *name;
-
-	name = mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6123_table,
-				     ARRAY_SIZE(mv88e6123_table));
-	if (name) {
-		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
-		if (!ps)
-			return NULL;
-		*priv = ps;
-	}
-	return name;
+	return mv88e6xxx_drv_probe(dsa_dev, host_dev, sw_addr, priv,
+				   mv88e6123_table,
+				   ARRAY_SIZE(mv88e6123_table));
 }
 
 static int mv88e6123_setup_global(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index b9f9b009f65a..9e6edf94a855 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -28,18 +28,9 @@ static const struct mv88e6xxx_switch_id mv88e6131_table[] = {
 static char *mv88e6131_probe(struct device *dsa_dev, struct device *host_dev,
 			     int sw_addr, void **priv)
 {
-	struct mv88e6xxx_priv_state *ps;
-	char *name;
-
-	name = mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6131_table,
-				     ARRAY_SIZE(mv88e6131_table));
-	if (name) {
-		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
-		if (!ps)
-			return NULL;
-		*priv = ps;
-	}
-	return name;
+	return mv88e6xxx_drv_probe(dsa_dev, host_dev, sw_addr, priv,
+				   mv88e6131_table,
+				   ARRAY_SIZE(mv88e6131_table));
 }
 
 static int mv88e6131_setup_global(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 15200928cecc..6ab86071391f 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -27,18 +27,9 @@ static const struct mv88e6xxx_switch_id mv88e6171_table[] = {
 static char *mv88e6171_probe(struct device *dsa_dev, struct device *host_dev,
 			     int sw_addr, void **priv)
 {
-	struct mv88e6xxx_priv_state *ps;
-	char *name;
-
-	name = mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6171_table,
-				     ARRAY_SIZE(mv88e6171_table));
-		if (name) {
-		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
-		if (!ps)
-			return NULL;
-		*priv = ps;
-	}
-	return name;
+	return mv88e6xxx_drv_probe(dsa_dev, host_dev, sw_addr, priv,
+				   mv88e6171_table,
+				   ARRAY_SIZE(mv88e6171_table));
 }
 
 static int mv88e6171_setup_global(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 7081a78a67e1..764b10ffb631 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -40,18 +40,9 @@ static const struct mv88e6xxx_switch_id mv88e6352_table[] = {
 static char *mv88e6352_probe(struct device *dsa_dev, struct device *host_dev,
 			     int sw_addr, void **priv)
 {
-	struct mv88e6xxx_priv_state *ps;
-	char *name;
-
-	name = mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6352_table,
-				     ARRAY_SIZE(mv88e6352_table));
-	if (name) {
-		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
-		if (!ps)
-			return NULL;
-		*priv = ps;
-	}
-	return name;
+	return mv88e6xxx_drv_probe(dsa_dev, host_dev, sw_addr, priv,
+				   mv88e6352_table,
+				   ARRAY_SIZE(mv88e6352_table));
 }
 
 static int mv88e6352_setup_global(struct dsa_switch *ds)
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 085fc4a49eb2..c242ffd8eb09 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -94,15 +94,12 @@ static int __mv88e6xxx_reg_read(struct mii_bus *bus, int sw_addr, int addr,
 
 static int _mv88e6xxx_reg_read(struct dsa_switch *ds, int addr, int reg)
 {
-	struct mii_bus *bus = dsa_host_dev_to_mii_bus(ds->master_dev);
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
 	assert_smi_lock(ds);
 
-	if (bus == NULL)
-		return -EINVAL;
-
-	ret = __mv88e6xxx_reg_read(bus, ds->pd->sw_addr, addr, reg);
+	ret = __mv88e6xxx_reg_read(ps->bus, ps->sw_addr, addr, reg);
 	if (ret < 0)
 		return ret;
 
@@ -159,17 +156,14 @@ static int __mv88e6xxx_reg_write(struct mii_bus *bus, int sw_addr, int addr,
 static int _mv88e6xxx_reg_write(struct dsa_switch *ds, int addr, int reg,
 				u16 val)
 {
-	struct mii_bus *bus = dsa_host_dev_to_mii_bus(ds->master_dev);
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
 	assert_smi_lock(ds);
 
-	if (bus == NULL)
-		return -EINVAL;
-
 	dev_dbg(ds->master_dev, "-> addr: 0x%.2x reg: 0x%.2x val: 0x%.4x\n",
 		addr, reg, val);
 
-	return __mv88e6xxx_reg_write(bus, ds->pd->sw_addr, addr, reg, val);
+	return __mv88e6xxx_reg_write(ps->bus, ps->sw_addr, addr, reg, val);
 }
 
 int mv88e6xxx_reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
@@ -2671,7 +2665,6 @@ int mv88e6xxx_setup_common(struct dsa_switch *ds)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
 	ps->ds = ds;
-
 	mutex_init(&ps->smi_mutex);
 
 	ps->id = REG_READ(REG_PORT(0), PORT_SWITCH_ID) & 0xfff0;
@@ -3075,9 +3068,9 @@ int mv88e6xxx_get_temp_alarm(struct dsa_switch *ds, bool *alarm)
 }
 #endif /* CONFIG_NET_DSA_HWMON */
 
-char *mv88e6xxx_lookup_name(struct device *host_dev, int sw_addr,
-			    const struct mv88e6xxx_switch_id *table,
-			    unsigned int num)
+static char *mv88e6xxx_lookup_name(struct device *host_dev, int sw_addr,
+				   const struct mv88e6xxx_switch_id *table,
+				   unsigned int num)
 {
 	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
 	int i, ret;
@@ -3107,6 +3100,28 @@ char *mv88e6xxx_lookup_name(struct device *host_dev, int sw_addr,
 	return NULL;
 }
 
+char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
+			  int sw_addr, void **priv,
+			  const struct mv88e6xxx_switch_id *table,
+			  unsigned int num)
+{
+	struct mv88e6xxx_priv_state *ps;
+	char *name;
+
+	name = mv88e6xxx_lookup_name(host_dev, sw_addr, table, num);
+	if (name) {
+		ps = devm_kzalloc(dsa_dev, sizeof(*ps), GFP_KERNEL);
+		if (!ps)
+			return NULL;
+		*priv = ps;
+		ps->bus = dsa_host_dev_to_mii_bus(host_dev);
+		if (!ps->bus)
+			return NULL;
+		ps->sw_addr = sw_addr;
+	}
+	return name;
+}
+
 static int __init mv88e6xxx_init(void)
 {
 #if IS_ENABLED(CONFIG_NET_DSA_MV88E6131)
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 0322e3e0e7d9..5d27decc85cb 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -406,6 +406,12 @@ struct mv88e6xxx_priv_state {
 	 */
 	struct mutex	smi_mutex;
 
+	/* The MII bus and the address on the bus that is used to
+	 * communication with the switch
+	 */
+	struct mii_bus *bus;
+	int sw_addr;
+
 #ifdef CONFIG_NET_DSA_MV88E6XXX_NEED_PPU
 	/* Handles automatic disabling and re-enabling of the PHY
 	 * polling unit.
@@ -456,9 +462,11 @@ struct mv88e6xxx_hw_stat {
 };
 
 int mv88e6xxx_switch_reset(struct dsa_switch *ds, bool ppu_active);
-char *mv88e6xxx_lookup_name(struct device *host_dev, int sw_addr,
-			    const struct mv88e6xxx_switch_id *table,
-			    unsigned int num);
+char *mv88e6xxx_drv_probe(struct device *dsa_dev, struct device *host_dev,
+			  int sw_addr, void **priv,
+			  const struct mv88e6xxx_switch_id *table,
+			  unsigned int num);
+
 int mv88e6xxx_setup_ports(struct dsa_switch *ds);
 int mv88e6xxx_setup_common(struct dsa_switch *ds);
 int mv88e6xxx_setup_global(struct dsa_switch *ds);
-- 
2.7.0

^ permalink raw reply related

* [PATCH v2 net-next 1/7] net: dsa: Pass the dsa device to the switch drivers
From: Andrew Lunn @ 2016-04-13  0:40 UTC (permalink / raw)
  To: David Miller; +Cc: Florian Fainelli, netdev, Vivien Didelot, Andrew Lunn
In-Reply-To: <1460508045-20254-1-git-send-email-andrew@lunn.ch>

By passing a device structure to the switch devices, it allows them
to use devm_* methods for resource management.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/bcm_sf2.c   | 3 ++-
 drivers/net/dsa/mv88e6060.c | 3 ++-
 drivers/net/dsa/mv88e6123.c | 3 ++-
 drivers/net/dsa/mv88e6131.c | 3 ++-
 drivers/net/dsa/mv88e6171.c | 3 ++-
 drivers/net/dsa/mv88e6352.c | 3 ++-
 include/net/dsa.h           | 3 ++-
 net/dsa/dsa.c               | 7 ++++---
 8 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 780f22876538..18a79579141f 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -135,7 +135,8 @@ static int bcm_sf2_sw_get_sset_count(struct dsa_switch *ds)
 	return BCM_SF2_STATS_SIZE;
 }
 
-static char *bcm_sf2_sw_probe(struct device *host_dev, int sw_addr)
+static char *bcm_sf2_sw_probe(struct device *dsa_dev, struct device *host_dev,
+			      int sw_addr)
 {
 	return "Broadcom Starfighter 2";
 }
diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index 0527f485c3dc..34d0f9fe19db 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -57,7 +57,8 @@ static int reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
 			return __ret;				\
 	})
 
-static char *mv88e6060_probe(struct device *host_dev, int sw_addr)
+static char *mv88e6060_probe(struct device *dsa_dev, struct device *host_dev,
+			     int sw_addr)
 {
 	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
 	int ret;
diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
index 69a6f79dcb10..ede4c6f0b31a 100644
--- a/drivers/net/dsa/mv88e6123.c
+++ b/drivers/net/dsa/mv88e6123.c
@@ -29,7 +29,8 @@ static const struct mv88e6xxx_switch_id mv88e6123_table[] = {
 	{ PORT_SWITCH_ID_6165_A2, "Marvell 88e6165 (A2)" },
 };
 
-static char *mv88e6123_probe(struct device *host_dev, int sw_addr)
+static char *mv88e6123_probe(struct device *dsa_dev, struct device *host_dev,
+			     int sw_addr)
 {
 	return mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6123_table,
 				     ARRAY_SIZE(mv88e6123_table));
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 24070287c2bc..bfadfd2cbb8d 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -25,7 +25,8 @@ static const struct mv88e6xxx_switch_id mv88e6131_table[] = {
 	{ PORT_SWITCH_ID_6185, "Marvell 88E6185" },
 };
 
-static char *mv88e6131_probe(struct device *host_dev, int sw_addr)
+static char *mv88e6131_probe(struct device *dsa_dev, struct device *host_dev,
+			     int sw_addr)
 {
 	return mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6131_table,
 				     ARRAY_SIZE(mv88e6131_table));
diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index 0e62f3b5bc81..fb35d3ac1644 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -24,7 +24,8 @@ static const struct mv88e6xxx_switch_id mv88e6171_table[] = {
 	{ PORT_SWITCH_ID_6351, "Marvell 88E6351" },
 };
 
-static char *mv88e6171_probe(struct device *host_dev, int sw_addr)
+static char *mv88e6171_probe(struct device *dsa_dev, struct device *host_dev,
+			     int sw_addr)
 {
 	return mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6171_table,
 				     ARRAY_SIZE(mv88e6171_table));
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 7f452e4a04a5..577ab2cfa944 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -37,7 +37,8 @@ static const struct mv88e6xxx_switch_id mv88e6352_table[] = {
 	{ PORT_SWITCH_ID_6352_A1, "Marvell 88E6352 (A1)" },
 };
 
-static char *mv88e6352_probe(struct device *host_dev, int sw_addr)
+static char *mv88e6352_probe(struct device *dsa_dev, struct device *host_dev,
+			     int sw_addr)
 {
 	return mv88e6xxx_lookup_name(host_dev, sw_addr, mv88e6352_table,
 				     ARRAY_SIZE(mv88e6352_table));
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 18d1be3ad62d..0f9f6f38f255 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -212,7 +212,8 @@ struct dsa_switch_driver {
 	/*
 	 * Probing and setup.
 	 */
-	char	*(*probe)(struct device *host_dev, int sw_addr);
+	char	*(*probe)(struct device *dsa_dev, struct device *host_dev,
+			  int sw_addr);
 	int	(*setup)(struct dsa_switch *ds);
 	int	(*set_addr)(struct dsa_switch *ds, u8 *addr);
 	u32	(*get_phy_flags)(struct dsa_switch *ds, int port);
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index c28c47463b7e..c06275311cb2 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -51,7 +51,8 @@ void unregister_switch_driver(struct dsa_switch_driver *drv)
 EXPORT_SYMBOL_GPL(unregister_switch_driver);
 
 static struct dsa_switch_driver *
-dsa_switch_probe(struct device *host_dev, int sw_addr, char **_name)
+dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
+		 char **_name)
 {
 	struct dsa_switch_driver *ret;
 	struct list_head *list;
@@ -66,7 +67,7 @@ dsa_switch_probe(struct device *host_dev, int sw_addr, char **_name)
 
 		drv = list_entry(list, struct dsa_switch_driver, list);
 
-		name = drv->probe(host_dev, sw_addr);
+		name = drv->probe(parent, host_dev, sw_addr);
 		if (name != NULL) {
 			ret = drv;
 			break;
@@ -387,7 +388,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
 	/*
 	 * Probe for switch model.
 	 */
-	drv = dsa_switch_probe(host_dev, pd->sw_addr, &name);
+	drv = dsa_switch_probe(parent, host_dev, pd->sw_addr, &name);
 	if (drv == NULL) {
 		netdev_err(dst->master_netdev, "[%d]: could not detect attached switch\n",
 			   index);
-- 
2.7.0

^ permalink raw reply related

* [PATCH v2 net-next 6/7] dsa: Rename phys_port_mask to enabled_port_mask
From: Andrew Lunn @ 2016-04-13  0:40 UTC (permalink / raw)
  To: David Miller; +Cc: Florian Fainelli, netdev, Vivien Didelot, Andrew Lunn
In-Reply-To: <1460508045-20254-1-git-send-email-andrew@lunn.ch>

The phys in phys_port_mask suggests this mask is about PHYs. In fact,
it means physical ports. Rename to enabled_port_mask, indicating
external enabled ports of the switch, which is hopefully less
confusing.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
v2:
    s/user_port_mask/enabled_port_mask/g
---
 drivers/net/dsa/bcm_sf2.c   | 19 ++++++++++---------
 drivers/net/dsa/mv88e6060.c |  2 +-
 include/net/dsa.h           |  4 ++--
 net/dsa/dsa.c               |  8 ++++----
 4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 50caa525cda3..7a5f0ef46bd6 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -160,7 +160,7 @@ static void bcm_sf2_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
 	 * the same VLAN.
 	 */
 	for (i = 0; i < priv->hw_params.num_ports; i++) {
-		if (!((1 << i) & ds->phys_port_mask))
+		if (!((1 << i) & ds->enabled_port_mask))
 			continue;
 
 		reg = core_readl(priv, CORE_PORT_VLAN_CTL_PORT(i));
@@ -1009,7 +1009,7 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
 	/* Enable all valid ports and disable those unused */
 	for (port = 0; port < priv->hw_params.num_ports; port++) {
 		/* IMP port receives special treatment */
-		if ((1 << port) & ds->phys_port_mask)
+		if ((1 << port) & ds->enabled_port_mask)
 			bcm_sf2_port_setup(ds, port, NULL);
 		else if (dsa_is_cpu_port(ds, port))
 			bcm_sf2_imp_setup(ds, port);
@@ -1022,11 +1022,12 @@ static int bcm_sf2_sw_setup(struct dsa_switch *ds)
 	 * 7445D0, since 7445E0 disconnects the internal switch pseudo-PHY such
 	 * that we can use the regular SWITCH_MDIO master controller instead.
 	 *
-	 * By default, DSA initializes ds->phys_mii_mask to ds->phys_port_mask
-	 * to have a 1:1 mapping between Port address and PHY address in order
-	 * to utilize the slave_mii_bus instance to read from Port PHYs. This is
-	 * not what we want here, so we initialize phys_mii_mask 0 to always
-	 * utilize the "master" MDIO bus backed by the "mdio-unimac" driver.
+	 * By default, DSA initializes ds->phys_mii_mask to
+	 * ds->enabled_port_mask to have a 1:1 mapping between Port address
+	 * and PHY address in order to utilize the slave_mii_bus instance to
+	 * read from Port PHYs. This is not what we want here, so we
+	 * initialize phys_mii_mask 0 to always utilize the "master" MDIO
+	 * bus backed by the "mdio-unimac" driver.
 	 */
 	if (of_machine_is_compatible("brcm,bcm7445d0"))
 		ds->phys_mii_mask |= ((1 << BRCM_PSEUDO_PHY_ADDR) | (1 << 0));
@@ -1284,7 +1285,7 @@ static int bcm_sf2_sw_suspend(struct dsa_switch *ds)
 	 * bcm_sf2_sw_setup
 	 */
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
-		if ((1 << port) & ds->phys_port_mask ||
+		if ((1 << port) & ds->enabled_port_mask ||
 		    dsa_is_cpu_port(ds, port))
 			bcm_sf2_port_disable(ds, port, NULL);
 	}
@@ -1308,7 +1309,7 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds)
 		bcm_sf2_gphy_enable_set(ds, true);
 
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
-		if ((1 << port) & ds->phys_port_mask)
+		if ((1 << port) & ds->enabled_port_mask)
 			bcm_sf2_port_setup(ds, port, NULL);
 		else if (dsa_is_cpu_port(ds, port))
 			bcm_sf2_imp_setup(ds, port);
diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index adb608ccd9aa..92cebab9383e 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -170,7 +170,7 @@ static int mv88e6060_setup_port(struct dsa_switch *ds, int p)
 	REG_WRITE(addr, PORT_VLAN_MAP,
 		  ((p & 0xf) << PORT_VLAN_MAP_DBNUM_SHIFT) |
 		   (dsa_is_cpu_port(ds, p) ?
-			ds->phys_port_mask :
+			ds->enabled_port_mask :
 			BIT(ds->dst->cpu_port)));
 
 	/* Port Association Vector: when learning source addresses
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 165c2e10615c..689ebd3542ba 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -167,7 +167,7 @@ struct dsa_switch {
 	 * Slave mii_bus and devices for the individual ports.
 	 */
 	u32			dsa_port_mask;
-	u32			phys_port_mask;
+	u32			enabled_port_mask;
 	u32			phys_mii_mask;
 	struct mii_bus		*slave_mii_bus;
 	struct net_device	*ports[DSA_MAX_PORTS];
@@ -185,7 +185,7 @@ static inline bool dsa_is_dsa_port(struct dsa_switch *ds, int p)
 
 static inline bool dsa_is_port_initialized(struct dsa_switch *ds, int p)
 {
-	return ds->phys_port_mask & (1 << p) && ds->ports[p];
+	return ds->enabled_port_mask & (1 << p) && ds->ports[p];
 }
 
 static inline u8 dsa_upstream_port(struct dsa_switch *ds)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 14bf12f637d2..60ea98481806 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -246,7 +246,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 		} else if (!strcmp(name, "dsa")) {
 			ds->dsa_port_mask |= 1 << i;
 		} else {
-			ds->phys_port_mask |= 1 << i;
+			ds->enabled_port_mask |= 1 << i;
 		}
 		valid_name_found = true;
 	}
@@ -259,7 +259,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 	/* Make the built-in MII bus mask match the number of ports,
 	 * switch drivers can override this later
 	 */
-	ds->phys_mii_mask = ds->phys_port_mask;
+	ds->phys_mii_mask = ds->enabled_port_mask;
 
 	/*
 	 * If the CPU connects to this switch, set the switch tree
@@ -325,7 +325,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 	 * Create network devices for physical switch ports.
 	 */
 	for (i = 0; i < DSA_MAX_PORTS; i++) {
-		if (!(ds->phys_port_mask & (1 << i)))
+		if (!(ds->enabled_port_mask & (1 << i)))
 			continue;
 
 		ret = dsa_slave_create(ds, parent, i, pd->port_names[i]);
@@ -435,7 +435,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
 
 	/* Destroy network devices for physical switch ports. */
 	for (port = 0; port < DSA_MAX_PORTS; port++) {
-		if (!(ds->phys_port_mask & (1 << port)))
+		if (!(ds->enabled_port_mask & (1 << port)))
 			continue;
 
 		if (!ds->ports[port])
-- 
2.7.0

^ permalink raw reply related

* [PATCH v2 net-next 0/7] DSA refactoring: set 1
From: Andrew Lunn @ 2016-04-13  0:40 UTC (permalink / raw)
  To: David Miller; +Cc: Florian Fainelli, netdev, Vivien Didelot, Andrew Lunn

There has been a long running effort to refractor DSA probing to make
the switches true linux devices. Here are a small collection of
patches moving in this direction. Most have been seen before.

We take a little step forward by passing the dsa device point to the
driver, thus allowing it to perform resource allocations using the
normal mechanisms. This device structure will later be replaced by the
devices own device structure.

Future patches will add a true driver probe function, so we rename the
current probe function, cleaning up the namespace.

phys_port_mask continually confuses me, thinking it is about PHYs. But
it is actually about ports enabled to the outside world. So rename it to
enabled_port_mask.

Lots more patches yet to follow, this is just doing some ground work.

v2:
  enabled_port_mask instread of user_port_masks
  Added Tested-by's and Reviewed-by.

Andrew Lunn (7):
  net: dsa: Pass the dsa device to the switch drivers
  net: dsa: Have the switch driver allocate there own private memory
  net: dsa: Remove allocation of driver private memory
  net: dsa: Keep the mii bus and address in the private structure
  net: dsa: Rename DSA probe function.
  dsa: Rename phys_port_mask to user_port_mask
  dsa: mv88e6xxx: Use bus in mv88e6xxx_lookup_name()

 drivers/net/dsa/bcm_sf2.c   | 24 +++++++++++++-------
 drivers/net/dsa/mv88e6060.c | 47 +++++++++++++++++++++++---------------
 drivers/net/dsa/mv88e6060.h | 11 +++++++++
 drivers/net/dsa/mv88e6123.c | 14 +++++++-----
 drivers/net/dsa/mv88e6131.c | 14 +++++++-----
 drivers/net/dsa/mv88e6171.c | 14 +++++++-----
 drivers/net/dsa/mv88e6352.c | 14 +++++++-----
 drivers/net/dsa/mv88e6xxx.c | 55 +++++++++++++++++++++++++++++++--------------
 drivers/net/dsa/mv88e6xxx.h | 17 +++++++++++---
 include/net/dsa.h           | 16 ++++++++-----
 net/dsa/dsa.c               | 19 +++++++++-------
 11 files changed, 166 insertions(+), 79 deletions(-)

-- 
2.7.0

^ permalink raw reply

* Re: [PATCH net-next] ibmvnic: Defer tx completion processing using a wait queue
From: David Miller @ 2016-04-13  1:12 UTC (permalink / raw)
  To: jallen; +Cc: eric.dumazet, tlfalcon, netdev, linuxppc-dev
In-Reply-To: <570D61E7.2090203@linux.vnet.ibm.com>

From: John Allen <jallen@linux.vnet.ibm.com>
Date: Tue, 12 Apr 2016 16:00:23 -0500

> On 04/12/2016 03:12 PM, Eric Dumazet wrote:
>> On Tue, 2016-04-12 at 14:38 -0500, John Allen wrote:
>>> Moves tx completion processing out of interrupt context, deferring work
>>> using a wait queue. With this work now deferred, we must account for the
>>> possibility that skbs can be sent faster than we can process completion
>>> requests in which case the tx buffer will overflow. If the tx buffer is
>>> full, ibmvnic_xmit will return NETDEV_TX_BUSY and stop the current tx
>>> queue. Subsequently, the queue will be restarted in ibmvnic_complete_tx
>>> when all pending tx completion requests have been cleared.
>> 
>> 1) Why is this needed ?
> 
> In the current ibmvnic implementation, tx completion processing is done in
> interrupt context. Depending on the load, this can block further
> interrupts for a long time. This patch just creates a bottom half so that
> when a tx completion interrupt comes in, we can defer the majority of the
> work and exit interrupt context quickly.

You should use NAPI polling for this, not your own invented
mechanism.

^ permalink raw reply

* Re: Configuring ethernet link fails with No such device
From: Stefan Agner @ 2016-04-13  1:29 UTC (permalink / raw)
  To: David Miller, teg, f.fainelli
  Cc: fabio.estevam, systemd-devel, netdev, u.kleine-koenig, l.stach
In-Reply-To: <20160412.114435.458888996741613334.davem@davemloft.net>

[added Tom Gundersen as he seemed to have worked on udev's net link
built-in]

On 2016-04-12 08:44, David Miller wrote:
> From: Bob Ham <bob.ham@collabora.com>
> Date: Tue, 12 Apr 2016 09:58:12 +0100
> 
>> On Mon, 2016-04-11 at 15:46 -0700, Stefan Agner wrote:
>>
>>> Or in other words: Is this a Kernel or systemd issue?
>>
>> From what I recall, both; an issue with the FEC driver, and issues in
>> systemd/udevd's handling of link-level settings.
> 
> This is my impression of the situation as well.

The execution order looks as follows for the FEC driver (taken from a
recent kernel with some additional printks, serial console):

...
[    1.157086] fec_probe
...
[    1.190350] fec_enet_mii_init
[    1.205446] libphy: fec_enet_mii_bus: probed
[    1.217849] fec 400d1000.ethernet eth0: registered PHC device 0
...
[   10.751060] dev_ethtool, rc -19 => (ETHTOOL_GSET) Too early...
[   11.262333] fec_enet_open
[   11.479928] fec_enet_mii_probe => Or/and too late...

Looking at the journal:
...
Apr 13 01:02:49 colibri-vf systemd[1]: Started udev Coldplug all
Devices.
...
Apr 13 01:02:50 colibri-vf kernel: mdev_ethtool, rc -19
...
Apr 13 01:02:50 colibri-vf systemd-udevd[197]: Could not set speed or
duplex of eth0 to 0 Mbps (half): No such device
...
Apr 13 01:02:51 colibri-vf kernel: fec_enet_open
...
Apr 13 01:02:51 colibri-vf kernel: fec_enet_mii_probe

The service "udev Coldplug all Devices" essentially executes:
udevadm trigger --type=subsystems --action=add ; udevadm trigger
--type=devices --action=add

I guess the "add" udev event is generated due to the fact that the
device has successfully been probed. If the kernel is allowed to
initialize the PHY as late as at open time, systemd should configure the
link at a point after link up... Do you agree on that?

Afact there is no udev event on link up, hence this would not be a mere
udev rule change...?

So I think the issue is either a systemd/udev issue or a FEC driver
issue. And the solution is one of this two options:
- Move (all the) PHY probes at driver probe time, then the PHY will be
probed when udev processes the add event
- Or move systemd/networkd/udev link processing, set the link settings
only after the device has been opened (link up).

Florian brought up a good point in another answer:
> Part of the reason for doing that, is that probing for the PHY during
> the driver's probe function could leave you with unused HW that is
> powered on for no good reason. Until we get into ndo_open, we have no
> clue if the interface is going to be used or not, so it is better to
> move part of the HW initialization as late as possible in the process.

Therefor I think only option 2 is actually viable.

--
Stefan






_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

^ permalink raw reply

* Re: [PATCH 0/8] Netfilter updates for net-next
From: David Miller @ 2016-04-13  2:35 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1460502166-20340-1-git-send-email-pablo@netfilter.org>

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 13 Apr 2016 01:02:38 +0200

> The following patchset contains the first batch of Netfilter updates for
> your net-next tree.
 ...

Pulled, thanks Pablo.

^ permalink raw reply

* Re: [PATCH RFC] net: decrease the length of backlog queue immediately after it's detached from sk
From: Eric Dumazet @ 2016-04-13  2:42 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: netdev, davem, Ding Tianhong
In-Reply-To: <570CEA9C.1070803@huawei.com>

On Tue, 2016-04-12 at 20:31 +0800, Yang Yingliang wrote:

> I traced the cost cycles of handling backlog packets in
> __release_sock().
> 16.97 ms to handling about 12MB backlog packets, of which 13.66ms to do
> sk_data_ready.
> The speed of handling packets in TCP is 5.65Gb/s which is smaller than
> the NIC's bandwidth. So the packets will be dropped.
> 
> If the cost of sk_data_read cannot be reduced, do we have other choice
> exclude dropping packets ?

Normally, TCP stack sends ACK packets with appropriate RWIN.

Sender should not send more packets than allowed in RWIN, even if there
are 128 threads using one TCP socket, it does not matter.

Imagine you do not have a backlog problem (nothing does the sendmsg()
while you receive data), and nothing reads the socket. Then the receiver
should eventually send WIN 0 back to the sender and sender should stop,
before any drop can possibly happen.

I have no problem receiving one TCP flow at 34Gbit, so it must be
something related to the huge windows you seem to use.

One possibility could be to tweak in ACK packets a reduced rwin so that
the sender is not allowed to continue the flood while we are painfully
processing a huge backlog.

^ permalink raw reply

* Re: [PATCH V3 0/8] net: mediatek: make the driver pass stress tests
From: David Miller @ 2016-04-13  2:42 UTC (permalink / raw)
  To: blogic; +Cc: nbd, matthias.bgg, sean.wang, netdev, linux-mediatek,
	linux-kernel
In-Reply-To: <1460069651-1234-1-git-send-email-blogic@openwrt.org>

From: John Crispin <blogic@openwrt.org>
Date: Fri,  8 Apr 2016 00:54:03 +0200

> While testing the driver we managed to get the TX path to stall and fail
> to recover. When dual MAC support was added to the driver, the whole queue
> stop/wake code was not properly adapted. There was also a regression in the
> locking of the xmit function. The fact that watchdog_timeo was not set and
> that the tx_timeout code failed to properly reset the dma, irq and queue
> just made the mess complete.
> 
> This series make the driver pass stress testing. With this series applied
> the testbed has been running for several days and still has not locked up.
> We have a second setup that has a small hack patch applied to randomly stop
> irqs and/or one of the queues and successfully manages to recover from these
> simulated tx stalls.

Series applied, thanks.

^ permalink raw reply

* Re: TCP reaching to maximum throughput after a long time
From: Yuchung Cheng @ 2016-04-13  3:08 UTC (permalink / raw)
  To: Ben Greear
  Cc: Eric Dumazet, Machani, Yaniv, netdev, David S. Miller,
	Eric Dumazet, Neal Cardwell, Nandita Dukkipati, open list,
	Kama, Meirav
In-Reply-To: <570D6B39.3050605@candelatech.com>

On Tue, Apr 12, 2016 at 2:40 PM, Ben Greear <greearb@candelatech.com> wrote:
> On 04/12/2016 01:29 PM, Eric Dumazet wrote:
>>
>> On Tue, 2016-04-12 at 13:23 -0700, Ben Greear wrote:
>>
>>> It worked well enough for years that I didn't even know other algorithms
>>> were
>>> available.  It was broken around 4.0 time, and I reported it to the list,
>>> and no one seemed to really care enough to do anything about it.  I
>>> changed
>>> to reno and ignored the problem as well.
>>>
>>> It is trivially easy to see the regression when using ath10k NIC, and
>>> from this email
>>> thread, I guess other NICs have similar issues.
>>
>>
>> Since it is so trivial, why don't you start a bisection ?
>
>
> I vaguely remember doing a bisect, but I can't find any email about
> that, so maybe I didn't.  At any rate, it is somewhere between 3.17 and 4.0.
> From memory, it was between 3.19 and 4.0, but I am not certain of that.
>
> Neil's suggestion, from the thread below, is that it was likely:  "605ad7f
> tcp: refine TSO autosizing"
>
> Here is previous email thread:
>
> https://www.mail-archive.com/netdev@vger.kernel.org/msg80803.html
>
> This one has a link to a pcap I made at the time:
>
> https://www.mail-archive.com/netdev@vger.kernel.org/msg80890.html
based on the prev thread I propose we disable hystart ack-train. It is
brittle under various circumstances. We've disabled that at Google for
years.

>
>>
>> I asked a capture, I did not say ' switch to Reno or whatever ', right ?
>>
>> Guessing is nice, but investigating and fixing is better.
>>
>> Do not assume that nothing can be done, please ?
>
>
> Thanks,
> Ben
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>

^ permalink raw reply

* Re: TCP reaching to maximum throughput after a long time
From: Eric Dumazet @ 2016-04-13  3:32 UTC (permalink / raw)
  To: Yuchung Cheng
  Cc: Ben Greear, Machani, Yaniv, netdev, David S. Miller, Eric Dumazet,
	Neal Cardwell, Nandita Dukkipati, open list, Kama, Meirav
In-Reply-To: <CAK6E8=f+4BagGi5BzLER=Hn2dDJ5FHWkEqLq=04tnWResk58GQ@mail.gmail.com>

On Tue, 2016-04-12 at 20:08 -0700, Yuchung Cheng wrote:

> based on the prev thread I propose we disable hystart ack-train. It is
> brittle under various circumstances. We've disabled that at Google for
> years.

Right, but because we also use sch_fq packet scheduler and pacing ;)

^ permalink raw reply

* [PATCH net-next] net: validate_xmit_skb() changes
From: Eric Dumazet @ 2016-04-13  4:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

skbs given to validate_xmit_skb() should not have a next
pointer anymore.

Also if a packet is dropped, increment dev->tx_dropped
__dev_queue_xmit() no longer has to change tx_dropped in this case.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/dev.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index d51343a821ed..85e808f977f5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2915,9 +2915,6 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
 {
 	netdev_features_t features;
 
-	if (skb->next)
-		return skb;
-
 	features = netif_skb_features(skb);
 	skb = validate_xmit_vlan(skb, features);
 	if (unlikely(!skb))
@@ -2960,6 +2957,7 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
 out_kfree_skb:
 	kfree_skb(skb);
 out_null:
+	atomic_long_inc(&dev->tx_dropped);
 	return NULL;
 }
 
@@ -3349,7 +3347,7 @@ static int __dev_queue_xmit(struct sk_buff *skb, void *accel_priv)
 
 			skb = validate_xmit_skb(skb, dev);
 			if (!skb)
-				goto drop;
+				goto out;
 
 			HARD_TX_LOCK(dev, txq, cpu);
 
@@ -3376,7 +3374,6 @@ recursion_alert:
 	}
 
 	rc = -ENETDOWN;
-drop:
 	rcu_read_unlock_bh();
 
 	atomic_long_inc(&dev->tx_dropped);

^ permalink raw reply related

* [PATCH v3] prism54: isl_38xx: Replace 'struct timeval'
From: Tina Ruchandani @ 2016-04-13  6:09 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: y2038-cunTk1MwBs8s++Sfvej+rw, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA

'struct timeval' uses a 32-bit seconds field which will overflow in
year 2038 and beyond. This patch is part of a larger effort to remove
all instances of 'struct timeval' from the kernel and replace them
with 64-bit timekeeping variables.
The patch also fixes the debug printf specifier to avoid the
seconds value being truncated.
The patch was build-tested / debugged by removing the
"if VERBOSE > SHOW_ERROR_MESSAGES" guards.

Signed-off-by: Tina Ruchandani <ruchandani.tina-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Suggested-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
--
Changes in v3:
 Fix commit message
Changes in v2:
 Changed printf specifier as suggested by Arnd Bergmann to
avoid truncation.
---
 drivers/net/wireless/intersil/prism54/isl_38xx.c | 35 ++++++++++++------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/intersil/prism54/isl_38xx.c b/drivers/net/wireless/intersil/prism54/isl_38xx.c
index 333c1a2..6700387 100644
--- a/drivers/net/wireless/intersil/prism54/isl_38xx.c
+++ b/drivers/net/wireless/intersil/prism54/isl_38xx.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/delay.h>
+#include <linux/ktime.h>

 #include <asm/uaccess.h>
 #include <asm/io.h>
@@ -113,7 +114,7 @@ isl38xx_trigger_device(int asleep, void __iomem *device_base)

 #if VERBOSE > SHOW_ERROR_MESSAGES
 	u32 counter = 0;
-	struct timeval current_time;
+	struct timespec64 current_ts64;
 	DEBUG(SHOW_FUNCTION_CALLS, "isl38xx trigger device\n");
 #endif

@@ -121,22 +122,22 @@ isl38xx_trigger_device(int asleep, void __iomem *device_base)
 	if (asleep) {
 		/* device is in powersave, trigger the device for wakeup */
 #if VERBOSE > SHOW_ERROR_MESSAGES
-		do_gettimeofday(&current_time);
-		DEBUG(SHOW_TRACING, "%08li.%08li Device wakeup triggered\n",
-		      current_time.tv_sec, (long)current_time.tv_usec);
+		ktime_get_real_ts64(&current_ts64);
+		DEBUG(SHOW_TRACING, "%lld.%09ld Device wakeup triggered\n",
+		      (s64)current_ts64.tv_sec, current_ts64.tv_nsec);

-		DEBUG(SHOW_TRACING, "%08li.%08li Device register read %08x\n",
-		      current_time.tv_sec, (long)current_time.tv_usec,
+		DEBUG(SHOW_TRACING, "%lld.%09ld Device register read %08x\n",
+		      (s64)current_ts64.tv_sec, current_ts64.tv_nsec,
 		      readl(device_base + ISL38XX_CTRL_STAT_REG));
 #endif

 		reg = readl(device_base + ISL38XX_INT_IDENT_REG);
 		if (reg == 0xabadface) {
 #if VERBOSE > SHOW_ERROR_MESSAGES
-			do_gettimeofday(&current_time);
+			ktime_get_real_ts64(&current_ts64);
 			DEBUG(SHOW_TRACING,
-			      "%08li.%08li Device register abadface\n",
-			      current_time.tv_sec, (long)current_time.tv_usec);
+			      "%lld.%09ld Device register abadface\n",
+			      (s64)current_ts64.tv_sec, current_ts64.tv_nsec);
 #endif
 			/* read the Device Status Register until Sleepmode bit is set */
 			while (reg = readl(device_base + ISL38XX_CTRL_STAT_REG),
@@ -149,13 +150,13 @@ isl38xx_trigger_device(int asleep, void __iomem *device_base)

 #if VERBOSE > SHOW_ERROR_MESSAGES
 			DEBUG(SHOW_TRACING,
-			      "%08li.%08li Device register read %08x\n",
-			      current_time.tv_sec, (long)current_time.tv_usec,
+			      "%lld.%09ld Device register read %08x\n",
+			      (s64)current_ts64.tv_sec, current_ts64.tv_nsec,
 			      readl(device_base + ISL38XX_CTRL_STAT_REG));
-			do_gettimeofday(&current_time);
+			ktime_get_real_ts64(&current_ts64);
 			DEBUG(SHOW_TRACING,
-			      "%08li.%08li Device asleep counter %i\n",
-			      current_time.tv_sec, (long)current_time.tv_usec,
+			      "%lld.%09ld Device asleep counter %i\n",
+			      (s64)current_ts64.tv_sec, current_ts64.tv_nsec,
 			      counter);
 #endif
 		}
@@ -168,9 +169,9 @@ isl38xx_trigger_device(int asleep, void __iomem *device_base)

 		/* perform another read on the Device Status Register */
 		reg = readl(device_base + ISL38XX_CTRL_STAT_REG);
-		do_gettimeofday(&current_time);
-		DEBUG(SHOW_TRACING, "%08li.%08li Device register read %08x\n",
-		      current_time.tv_sec, (long)current_time.tv_usec, reg);
+		ktime_get_real_ts64(&current_ts64);
+		DEBUG(SHOW_TRACING, "%lld.%00ld Device register read %08x\n",
+		      (s64)current_ts64.tv_sec, current_ts64.tv_nsec, reg);
 #endif
 	} else {
 		/* device is (still) awake  */
--
2.8.0.rc3.226.g39d4020

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

* RE: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver
From: Ramesh Shanmugasundaram @ 2016-04-13  6:25 UTC (permalink / raw)
  To: wg@grandegger.com, robh+dt@kernel.org, pawel.moll@arm.com,
	mark.rutland@arm.com, ijc+devicetree@hellion.org.uk,
	galak@codeaurora.org, corbet@lwn.net, mkl@pengutronix.de
  Cc: linux-renesas-soc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-can@vger.kernel.org, netdev@vger.kernel.org,
	linux-doc@vger.kernel.org, geert+renesas@glider.be,
	Chris Paterson
In-Reply-To: <SG2PR06MB10389E1E68089FF900746B44C39A0@SG2PR06MB1038.apcprd06.prod.outlook.com>

HI Marc,

Gentle reminder!
Are you happy with the open comment's disposition? I can send a next version of patch if we have a closure on current set of comments.

Thanks,
Ramesh

> -----Original Message-----
> From: Ramesh Shanmugasundaram
> Sent: 01 April 2016 13:49
> To: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>;
> wg@grandegger.com; robh+dt@kernel.org; pawel.moll@arm.com;
> mark.rutland@arm.com; ijc+devicetree@hellion.org.uk; galak@codeaurora.org;
> corbet@lwn.net
> Cc: linux-renesas-soc@vger.kernel.org; devicetree@vger.kernel.org; linux-
> can@vger.kernel.org; netdev@vger.kernel.org; linux-doc@vger.kernel.org;
> geert+renesas@glider.be; Chris Paterson <Chris.Paterson2@renesas.com>
> Subject: RE: [PATCH v2] can: rcar_canfd: Add Renesas R-Car CAN FD driver
> 
> Hi Marc,
> 
> Thanks for your time & review comments.
> 
> > -----Original Message-----
> (...)
> > > +#define RCANFD_NAPI_WEIGHT		8	/* Rx poll quota */
> > > +
> > > +#define RCANFD_NUM_CHANNELS		2
> > > +#define RCANFD_CHANNELS_MASK		0x3	/* Two channels max */
> >
> > (BIT(RCANFD_NUM_CHANNELS) - 1
> 
> OK
> 
> >
> > > +
> > > +/* Rx FIFO is a global resource of the controller. There are 8 such
> > FIFOs
> > > + * available. Each channel gets a dedicated Rx FIFO (i.e.) the
> > > + channel
> (...)
> > > +#define RCANFD_CMFIFO_CFDLC(x)		(((x) & 0xf) << 28)
> > > +#define RCANFD_CMFIFO_CFPTR(x)		(((x) & 0xfff) << 16)
> > > +#define RCANFD_CMFIFO_CFTS(x)		(((x) & 0xff) << 0)
> > > +
> > > +/* Global Test Config register */
> > > +#define RCANFD_GTSTCFG_C0CBCE		BIT(0)
> > > +#define RCANFD_GTSTCFG_C1CBCE		BIT(1)
> > > +
> > > +#define RCANFD_GTSTCTR_ICBCTME		BIT(0)
> > > +
> > > +/* AFL Rx rules registers */
> > > +#define RCANFD_AFLCFG_SETRNC0(x)	(((x) & 0xff) << 24)
> > > +#define RCANFD_AFLCFG_SETRNC1(x)	(((x) & 0xff) << 16)
> >
> > What about something like:
> >
> > #define RCANFD_AFLCFG_SETRNC(n, x)	(((x) & 0xff) << (24 - n * 8))
> >
> > This will save some if()s in the code
> 
> Nice :-). Will update.
> 
> >
> > > +#define RCANFD_AFLCFG_GETRNC0(x)	(((x) >> 24) & 0xff)
> > > +#define RCANFD_AFLCFG_GETRNC1(x)	(((x) >> 16) & 0xff)
> > > +
> > > +#define RCANFD_AFL_PAGENUM(entry)	((entry) / 16)
> (...)
> > > +#define rcar_canfd_read(priv, offset)			\
> > > +	readl(priv->base + (offset))
> > > +#define rcar_canfd_write(priv, offset, val)		\
> > > +	writel(val, priv->base + (offset))
> > > +#define rcar_canfd_set_bit(priv, reg, val)		\
> > > +	rcar_canfd_update(val, val, priv->base + (reg))
> > > +#define rcar_canfd_clear_bit(priv, reg, val)		\
> > > +	rcar_canfd_update(val, 0, priv->base + (reg))
> > > +#define rcar_canfd_update_bit(priv, reg, mask, val)	\
> > > +	rcar_canfd_update(mask, val, priv->base + (reg))
> >
> > please use static inline functions instead of defines.
> 
> OK.
> 
> >
> > > +
> > > +static void rcar_canfd_get_data(struct canfd_frame *cf,
> > > +				struct rcar_canfd_channel *priv, u32 off)
> >
> > Please use "struct rcar_canfd_channel *priv" as first argument, struct
> > canfd_frame *cf as second. Remove off, as the offset is already
> > defined by the channel.
> 
> I'll re-order priv, cf as you mentioned. I'll leave "off" arg as it is
> because it is based on FIFO number of channel + mode (CAN only or CANFD
> only). Otherwise, I will have to add another check inside this function
> for mode.
> 
> > > +{
> > > +	u32 i, lwords;
> > > +
> > > +	lwords = cf->len / sizeof(u32);
> > > +	if (cf->len % sizeof(u32))
> > > +		lwords++;
> >
> > Use DIV_ROUND_UP() instead of open coding it.
> 
> Agreed. Thanks.
> 
> >
> > > +	for (i = 0; i < lwords; i++)
> > > +		*((u32 *)cf->data + i) =
> > > +			rcar_canfd_read(priv, off + (i * sizeof(u32))); }
> > > +
> > > +static void rcar_canfd_put_data(struct canfd_frame *cf,
> > > +				struct rcar_canfd_channel *priv, u32 off)
> >
> > same here
> 
> Yes (same as _get_data)
> 
> >
> > > +{
> > > +	u32 i, j, lwords, leftover;
> > > +	u32 data = 0;
> > > +
> > > +	lwords = cf->len / sizeof(u32);
> > > +	leftover = cf->len % sizeof(u32);
> > > +	for (i = 0; i < lwords; i++)
> > > +		rcar_canfd_write(priv, off + (i * sizeof(u32)),
> > > +				 *((u32 *)cf->data + i));
> >
> > Here you don't convert the endianess...
> 
> Yes
> 
> > > +
> > > +	if (leftover) {
> > > +		u8 *p = (u8 *)((u32 *)cf->data + i);
> > > +
> > > +		for (j = 0; j < leftover; j++)
> > > +			data |= p[j] << (j * 8);
> >
> > ...here you do an implicit endianess conversion. "data" is little
> > endian, while p[j] is big endian.
> 
> Not sure I got the question correctly.
> Controller expectation of data bytes in 32bit register is bits[7:0] =
> byte0, bits[15:8] = byte1 and so on - little endian.
> For e.g. if cf->data points to byte stream H'112233445566 (cf->data[0] =
> 0x11), first rcar_canfd_write will write 0x44332211 value to register. Yes
> the host cpu is assumed little endian. In leftover case, data will be
> 0x00006655 - again little endian.
> I think I should remove this leftover logic and just mask the unused bits
> to zero as cf->data is pre-allocated for max length anyway.
> 
> p[j] is a byte?? Am I missing something?
> 
> >
> > > +		rcar_canfd_write(priv, off + (i * sizeof(u32)), data);
> > > +	}
> >
> > Have you tested to send CAN frames with len != n*4 against a different
> > controller?
> 
> Yes, with Vector VN1630A.
> 
> > > +}
> > > +
> > > +static void rcar_canfd_tx_failure_cleanup(struct net_device *ndev)
> > > +{
> > > +	u32 i;
> > > +
> > > +	for (i = 0; i < RCANFD_FIFO_DEPTH; i++)
> > > +		can_free_echo_skb(ndev, i);
> > > +}
> > > +
> (...)
> > > +static void rcar_canfd_tx_done(struct net_device *ndev) {
> > > +	struct rcar_canfd_channel *priv = netdev_priv(ndev);
> > > +	struct net_device_stats *stats = &ndev->stats;
> > > +	u32 sts;
> > > +	unsigned long flags;
> > > +	u32 ch = priv->channel;
> > > +
> > > +	do {
> >
> > You should iterare over all pending CAN frames:
> >
> > > 	for (/* nix */; (priv->tx_head - priv->tx_tail) > 0; priv-
> > >tx_tail++) {
> >
> 
> Yes, current code does iterate over pending tx'ed frames. If we use this
> for loop semantics, we may have to protect the whole loop with no real
> benefit.
> 
> >
> > > +		u8 unsent, sent;
> > > +
> > > +		sent = priv->tx_tail % RCANFD_FIFO_DEPTH;
> >
> > and check here, if that packet has really been tramsitted. Exit the
> > loop otherweise.
> 
> We are here because of tx_done and hence no need to check tx done again
> for the first iteration. Hence the do-while loop. Checks further down
> takes care of the need for more iterations/pending tx'ed frames.
> 
> >
> > > +		stats->tx_packets++;
> > > +		stats->tx_bytes += priv->tx_len[sent];
> > > +		priv->tx_len[sent] = 0;
> > > +		can_get_echo_skb(ndev, sent);
> > > +
> > > +		spin_lock_irqsave(&priv->tx_lock, flags);
> >
> > What does the tx_lock protect? The tx path is per channel, isn't it?
> 
> You are right - tx path is per channel. In tx path, head & tail are also
> used to determine when to stop/wakeup netif queue. They are incremented &
> compared in different contexts to make this decision. Per channel tx_lock
> protects this critical section.
> 
> >
> > > +		priv->tx_tail++;
> > > +		sts = rcar_canfd_read(priv,
> > > +				      RCANFD_CFSTS(ch, RCANFD_CFFIFO_IDX));
> > > +		unsent = RCANFD_CMFIFO_CFMC(sts);
> > > +
> > > +		/* Wake producer only when there is room */
> > > +		if (unsent != RCANFD_FIFO_DEPTH)
> > > +			netif_wake_queue(ndev);
> >
> > Move the netif_wake_queue() out of the loop.
> 
> With the tx logic mentioned above, I think keeping it in the loop seems
> better. For e.g. say cpu1 pumps 8 frames from an app loop and cpu0 handles
> tx_done interrupt handling: cpu1 would have stopped the queue because FIFO
> is full -> cpu0 gets tx_done interrupt and by the time it checks "unsent"
> there could be one or more frames transmitted by device (i.e.) there would
> be more space in fifo. It is better to wakeup the netif queue then and
> there so that app running on cpu1 can pump more. If we move it out of the
> loop we wake up the queue only in the end. Have I missed anything?
> 
> >
> > > +
> > > +		if (priv->tx_head - priv->tx_tail <= unsent) {
> > > +			spin_unlock_irqrestore(&priv->tx_lock, flags);
> > > +			break;
> > > +		}
> > > +		spin_unlock_irqrestore(&priv->tx_lock, flags);
> > > +
> > > +	} while (1);
> > > +
> > > +	/* Clear interrupt */
> > > +	rcar_canfd_write(priv, RCANFD_CFSTS(ch, RCANFD_CFFIFO_IDX),
> > > +			 sts & ~RCANFD_CMFIFO_CFTXIF);
> > > +	can_led_event(ndev, CAN_LED_EVENT_TX); }
> > > +
> > > +	if (cf->can_id & CAN_RTR_FLAG)
> > > +		id |= RCANFD_CMFIFO_CFRTR;
> > > +
> > > +	rcar_canfd_write(priv, RCANFD_F_CFID(ch, RCANFD_CFFIFO_IDX),
> > > +			 id);
> > > +	ptr = RCANFD_CMFIFO_CFDLC(can_len2dlc(cf->len));
> >
> > ptr usually means pointer, better call it dlc.
> 
> I used the register name "ptr". OK, will change it do dlc.
> 
> >
> > > +	rcar_canfd_write(priv, RCANFD_F_CFPTR(ch, RCANFD_CFFIFO_IDX),
> > > +			 ptr);
> > > +
> (...)
> > > +	can_put_echo_skb(skb, ndev, priv->tx_head % RCANFD_FIFO_DEPTH);
> > > +
> > > +	spin_lock_irqsave(&priv->tx_lock, flags);
> > > +	priv->tx_head++;
> > > +
> > > +	/* Start Tx: Write 0xff to CFPC to increment the CPU-side
> > > +	 * pointer for the Common FIFO
> > > +	 */
> > > +	rcar_canfd_write(priv, RCANFD_CFPCTR(ch, RCANFD_CFFIFO_IDX),
> > > +0xff);
> > > +
> > > +	/* Stop the queue if we've filled all FIFO entries */
> > > +	if (priv->tx_head - priv->tx_tail >= RCANFD_FIFO_DEPTH)
> > > +		netif_stop_queue(ndev);
> >
> > Please move the check of stop_queue, before starting the send.
> 
> OK.
> 
> >
> > > +
> > > +	spin_unlock_irqrestore(&priv->tx_lock, flags);
> > > +	return NETDEV_TX_OK;
> > > +}
> > > +
> (...)
> > > +{
> > > +	struct rcar_canfd_channel *priv =
> > > +		container_of(napi, struct rcar_canfd_channel, napi);
> > > +	int num_pkts;
> > > +	u32 sts;
> > > +	u32 ch = priv->channel;
> > > +	u32 ridx = ch + RCANFD_RFFIFO_IDX;
> > > +
> > > +	for (num_pkts = 0; num_pkts < quota; num_pkts++) {
> > > +		sts = rcar_canfd_read(priv, RCANFD_RFSTS(ridx));
> > > +		/* Clear interrupt bit */
> > > +		if (sts & RCANFD_RFFIFO_RFIF)
> > > +			rcar_canfd_write(priv, RCANFD_RFSTS(ridx),
> > > +					 sts & ~RCANFD_RFFIFO_RFIF);
> > > +
> > > +		/* Check FIFO empty condition */
> > > +		if (sts & RCANFD_RFFIFO_RFEMP)
> > > +			break;
> > > +
> > > +		rcar_canfd_rx_pkt(priv);
> >
> > This sequence looks strange. You first conditionally ack the interrupt
> > then you check for empty fifo, then read the CAN frame. I would assume
> > that you first check if there's a CAN frame, read it and then clear
> > the interrupt.
> 
> Yes, I shall re-arrange the sequence as you mentioned.
> 
> >
> > > +	}
> > > +
> > > +	/* All packets processed */
> > > +	if (num_pkts < quota) {
> > > +		napi_complete(napi);
> > > +		/* Enable Rx FIFO interrupts */
> > > +		rcar_canfd_set_bit(priv, RCANFD_RFCC(ridx),
> > RCANFD_RFFIFO_RFIE);
> (...)
> > > +	for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
> > > +		err = rcar_canfd_channel_probe(gpriv, ch);
> > > +		if (err)
> > > +			goto fail_channel;
> > > +	}
> >
> > Should the CAN IP core be clocked the whole time? What about shuting
> > down the clock and enabling it on ifup?
> 
> The fCAN clock is enabled only on ifup of one of the channels. However,
> the peripheral clock is enabled during probe to bring the controller to
> Global Operational mode. This clock cannot be turned off with the register
> values & mode retained.
> 
> > > +
> > > +	platform_set_drvdata(pdev, gpriv);
> > > +	dev_info(&pdev->dev, "global operational state (clk %d)\n",
> > > +		 gpriv->clock_select);
> (...)
> > > +	rcar_canfd_reset_controller(gpriv);
> > > +	rcar_canfd_disable_global_interrupts(gpriv);
> > > +
> > > +	for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
> > > +		priv = gpriv->ch[ch];
> > > +		if (priv) {
> >
> > This should always be true.
> 
> I agree. I thought I cleaned this up but it's in my local commits :-(
> 
> >
> > > +			rcar_canfd_disable_channel_interrupts(priv);
> > > +			unregister_candev(priv->ndev);
> > > +			netif_napi_del(&priv->napi);
> > > +			free_candev(priv->ndev);
> >
> > Please make use of rcar_canfd_channel_remove(), as you already have
> > the function.
> 
> Yes.
> 
> Thanks,
> Ramesh

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: exit busy loop when another process is runnable
From: Ingo Molnar @ 2016-04-13  7:20 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Mike Galbraith, Jason Wang, davem, netdev, linux-kernel,
	Peter Zijlstra, Ingo Molnar
In-Reply-To: <20160411182111-mutt-send-email-mst@redhat.com>


* Michael S. Tsirkin <mst@redhat.com> wrote:

> On Fri, Aug 22, 2014 at 09:36:53AM +0200, Ingo Molnar wrote:
> > 
> > > > diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
> > > > index 1d67fb6..8a33fb2 100644
> > > > --- a/include/net/busy_poll.h
> > > > +++ b/include/net/busy_poll.h
> > > > @@ -109,7 +109,8 @@ static inline bool sk_busy_loop(struct sock *sk, int nonblock)
> > > >  		cpu_relax();
> > > >  
> > > >  	} while (!nonblock && skb_queue_empty(&sk->sk_receive_queue) &&
> > > > -		 !need_resched() && !busy_loop_timeout(end_time));
> > > > +		 !need_resched() && !busy_loop_timeout(end_time) &&
> > > > +		 nr_running_this_cpu() < 2);
> > 
> > So it's generally a bad idea to couple to the scheduler through 
> > such a low level, implementation dependent value like 
> > 'nr_running', causing various problems:
> > 
> >  - It misses important work that might be pending on this CPU,
> >    like RCU callbacks.
> > 
> >  - It will also over-credit task contexts that might be
> >    runnable, but which are less important than the currently
> >    running one: such as a SCHED_IDLE task
> > 
> >  - It will also over-credit even regular SCHED_NORMAL tasks, if
> >    this current task is more important than them: say
> >    SCHED_FIFO. A SCHED_FIFO workload should run just as fast 
> >    with SCHED_NORMAL tasks around, as a SCHED_NORMAL workload 
> >    on an otherwise idle system.
> > 
> > So what you want is a more sophisticated query to the 
> > scheduler, a sched_expected_runtime() method that returns the 
> > number of nsecs this task is expected to run in the future, 
> > which returns 0 if you will be scheduled away on the next 
> > schedule(), and returns infinity for a high prio SCHED_FIFO 
> > task, or if this SCHED_NORMAL task is on an otherwise idle CPU.
> > 
> > It will return a regular time slice value in other cases, when 
> > there's some load on the CPU.
> > 
> > The polling logic can then do its decision based on that time 
> > value.
> > 
> > All this can be done reasonably fast and lockless in most 
> > cases, so that it can be called from busy-polling code.
> >
> > An added advantage would be that this approach consolidates the 
> > somewhat random need_resched() checks into this method as well.
> > 
> > In any case I don't agree with the nr_running_this_cpu() 
> > method.
> > 
> > (Please Cc: me and lkml to future iterations of this patchset.)
> > 
> > Thanks,
> > 
> > 	Ingo
> 
> I tried to look into this: it might be even nicer to add
> sched_expected_to_run(time) which tells us whether we expect the current
> task to keep running for the next XX nsecs.
> 
> For the fair scheduler, it seems that it could be as simple as
> 
> +static bool expected_to_run_fair(struct cfs_rq *cfs_rq, s64 t)
> +{
> +       struct sched_entity *left;
> +       struct sched_entity *curr = cfs_rq->curr;
> +
> +       if (!curr || !curr->on_rq)
> +               return false;
> +
> +       left = __pick_first_entity(cfs_rq);
> +       if (!left)
> +               return true;
> +
> +       return (s64)(curr->vruntime + calc_delta_fair(t, curr) -
> +                    left->vruntime) < 0;
> +}
> 
> The reason it seems easier is because that way we can reuse
> calc_delta_fair and don't have to do the reverse translation
> from vruntime to nsec.
> 
> And I guess if we do this with interrupts disabled, and only poke
> at the current CPU's rq, we know first entity
> won't go away so we don't need locks? 
> 
> Is this close to what you had in mind?

Yeah, fair enough ;-)

I'm not 100% convinced about the interface, but the model looks good to me.
Let's try it - I don't have fundamental objections anymore.

I also agree that it could be done lockless - although I'd suggest two steps: 
first do the dumb thing with the proper scheduler lock(s) held, then another patch 
which removes the locks for a bit more performance. That will make any subtle 
crashes/races bisectable.

Thanks,

	Ingo

^ permalink raw reply

* linux-next: build failure after merge of the net-next tree
From: Stephen Rothwell @ 2016-04-13  7:50 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: linux-next, linux-kernel, Mark Rustad, Jeff Kirsher,
	Andrew Bowers, Build bot for Mark Brown, kernel-build-reports,
	linaro-kernel

Hi all,

[A report from Mark's buildbot]

After merging the net-next tree, today's linux-next build (arm
allmodconfig) failed like thisi (this has actually been failing for a
few days, now):

ERROR: "__bad_udelay" [drivers/net/ethernet/intel/ixgbe/ixgbe.ko] undefined!

Caused by commit

  49425dfc7451 ("ixgbe: Add support for x550em_a 10G MAC type")

arm only allows udelay()s up to 2 milliseconds.  This commit
adds a 5 ms udelay in ixgbe_acquire_swfw_sync_x550em_a() in
drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c.

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply

* RESPOND BACK TO MY EMAIL PLS
From: Mrs Elena @ 2016-04-13  8:09 UTC (permalink / raw)


Contact me now for $6.2 Million offer

^ permalink raw reply

* Re: [PATCH v3] prism54: isl_38xx: Replace 'struct timeval'
From: Johannes Berg @ 2016-04-13  8:38 UTC (permalink / raw)
  To: Tina Ruchandani, Arnd Bergmann
  Cc: y2038, netdev, linux-wireless, linux-kernel
In-Reply-To: <20160413060916.GA21184@localhost>


> The patch was build-tested / debugged by removing the
> "if VERBOSE > SHOW_ERROR_MESSAGES" guards.

Stands to reason that we should just remove the (more or less) dead
code, since I don't think anyone really ever touches this driver any
more or will ever again ...

johannes
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ 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