* [PATCH 1/1 net-next] esp4: remove assignment in if condition
From: Fabian Frederick @ 2014-11-04 19:28 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Steffen Klassert, Herbert Xu, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/ipv4/esp4.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 360b565..9dd66ee 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -392,8 +392,11 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
if (elen <= 0)
goto out;
- if ((err = skb_cow_data(skb, 0, &trailer)) < 0)
+ err = skb_cow_data(skb, 0, &trailer);
+
+ if (err < 0)
goto out;
+
nfrags = err;
assoclen = sizeof(*esph);
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] ipv4: use seq_puts instead of seq_printf where possible
From: Fabian Frederick @ 2014-11-04 19:31 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/ipv4/proc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 8e3eb39..f0d4eb8 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -296,12 +296,12 @@ static void icmpmsg_put_line(struct seq_file *seq, unsigned long *vals,
int j;
if (count) {
- seq_printf(seq, "\nIcmpMsg:");
+ seq_puts(seq, "\nIcmpMsg:");
for (j = 0; j < count; ++j)
seq_printf(seq, " %sType%u",
type[j] & 0x100 ? "Out" : "In",
type[j] & 0xff);
- seq_printf(seq, "\nIcmpMsg:");
+ seq_puts(seq, "\nIcmpMsg:");
for (j = 0; j < count; ++j)
seq_printf(seq, " %lu", vals[j]);
}
@@ -342,7 +342,7 @@ static void icmp_put(struct seq_file *seq)
seq_puts(seq, "\nIcmp: InMsgs InErrors InCsumErrors");
for (i = 0; icmpmibmap[i].name != NULL; i++)
seq_printf(seq, " In%s", icmpmibmap[i].name);
- seq_printf(seq, " OutMsgs OutErrors");
+ seq_puts(seq, " OutMsgs OutErrors");
for (i = 0; icmpmibmap[i].name != NULL; i++)
seq_printf(seq, " Out%s", icmpmibmap[i].name);
seq_printf(seq, "\nIcmp: %lu %lu %lu",
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 1/1 net-next] esp4: remove assignment in if condition
From: Joe Perches @ 2014-11-04 19:33 UTC (permalink / raw)
To: Fabian Frederick
Cc: linux-kernel, Steffen Klassert, Herbert Xu, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev
In-Reply-To: <1415129311-12480-1-git-send-email-fabf@skynet.be>
On Tue, 2014-11-04 at 20:28 +0100, Fabian Frederick wrote:
trivia:
> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
[]
> @@ -392,8 +392,11 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
> if (elen <= 0)
> goto out;
>
> - if ((err = skb_cow_data(skb, 0, &trailer)) < 0)
> + err = skb_cow_data(skb, 0, &trailer);
> +
> + if (err < 0)
> goto out;
Generally it's better/more common to use
foo = bar();
if (!foo)
[error_handler...]
without the blank line between the set
and the test.
^ permalink raw reply
* [PATCH 1/1 net-next] ipv4: remove 0/NULL assignment on static
From: Fabian Frederick @ 2014-11-04 19:37 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
static values are automatically initialized to 0
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/ipv4/ipconfig.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index a896da5..7fa18bc 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -115,7 +115,7 @@
*/
int ic_set_manually __initdata = 0; /* IPconfig parameters set manually */
-static int ic_enable __initdata = 0; /* IP config enabled? */
+static int ic_enable __initdata; /* IP config enabled? */
/* Protocol choice */
int ic_proto_enabled __initdata = 0
@@ -130,7 +130,7 @@ int ic_proto_enabled __initdata = 0
#endif
;
-static int ic_host_name_set __initdata = 0; /* Host name set by us? */
+static int ic_host_name_set __initdata; /* Host name set by us? */
__be32 ic_myaddr = NONE; /* My IP address */
static __be32 ic_netmask = NONE; /* Netmask for local subnet */
@@ -160,17 +160,17 @@ static u8 ic_domain[64]; /* DNS (not NIS) domain name */
static char user_dev_name[IFNAMSIZ] __initdata = { 0, };
/* Protocols supported by available interfaces */
-static int ic_proto_have_if __initdata = 0;
+static int ic_proto_have_if __initdata;
/* MTU for boot device */
-static int ic_dev_mtu __initdata = 0;
+static int ic_dev_mtu __initdata;
#ifdef IPCONFIG_DYNAMIC
static DEFINE_SPINLOCK(ic_recv_lock);
-static volatile int ic_got_reply __initdata = 0; /* Proto(s) that replied */
+static volatile int ic_got_reply __initdata; /* Proto(s) that replied */
#endif
#ifdef IPCONFIG_DHCP
-static int ic_dhcp_msgtype __initdata = 0; /* DHCP msg type received */
+static int ic_dhcp_msgtype __initdata; /* DHCP msg type received */
#endif
@@ -186,8 +186,8 @@ struct ic_device {
__be32 xid;
};
-static struct ic_device *ic_first_dev __initdata = NULL;/* List of open device */
-static struct net_device *ic_dev __initdata = NULL; /* Selected device */
+static struct ic_device *ic_first_dev __initdata; /* List of open device */
+static struct net_device *ic_dev __initdata; /* Selected device */
static bool __init ic_is_init_dev(struct net_device *dev)
{
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] ipv4: ip_frag_queue function clean-up
From: Fabian Frederick @ 2014-11-04 19:41 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
-remove unnecessary else after break
-declare free_it sk_buff * only once (like prev and next)
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/ipv4/ip_fragment.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 2811cc1..153c402 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -317,7 +317,7 @@ static int ip_frag_reinit(struct ipq *qp)
/* Add new segment to existing queue. */
static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
{
- struct sk_buff *prev, *next;
+ struct sk_buff *prev, *next, *free_it;
struct net_device *dev;
int flags, offset;
int ihl, end;
@@ -432,23 +432,23 @@ found:
if (next->ip_summed != CHECKSUM_UNNECESSARY)
next->ip_summed = CHECKSUM_NONE;
break;
- } else {
- struct sk_buff *free_it = next;
+ }
- /* Old fragment is completely overridden with
- * new one drop it.
- */
- next = next->next;
+ free_it = next;
- if (prev)
- prev->next = next;
- else
- qp->q.fragments = next;
+ /* Old fragment is completely overridden with
+ * new one drop it.
+ */
+ next = next->next;
- qp->q.meat -= free_it->len;
- sub_frag_mem_limit(&qp->q, free_it->truesize);
- kfree_skb(free_it);
- }
+ if (prev)
+ prev->next = next;
+ else
+ qp->q.fragments = next;
+
+ qp->q.meat -= free_it->len;
+ sub_frag_mem_limit(&qp->q, free_it->truesize);
+ kfree_skb(free_it);
}
FRAG_CB(skb)->offset = offset;
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 1/1 net-next] esp4: remove assignment in if condition
From: Daniel Borkmann @ 2014-11-04 19:42 UTC (permalink / raw)
To: Fabian Frederick
Cc: linux-kernel, Steffen Klassert, Herbert Xu, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev
In-Reply-To: <1415129311-12480-1-git-send-email-fabf@skynet.be>
On 11/04/2014 08:28 PM, Fabian Frederick wrote:
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> net/ipv4/esp4.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index 360b565..9dd66ee 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -392,8 +392,11 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
> if (elen <= 0)
> goto out;
>
> - if ((err = skb_cow_data(skb, 0, &trailer)) < 0)
> + err = skb_cow_data(skb, 0, &trailer);
> +
If you already feel the need to change this (?), then please don't
add an extra newline here ...
> + if (err < 0)
> goto out;
> +
> nfrags = err;
>
> assoclen = sizeof(*esph);
>
^ permalink raw reply
* [PATCH 1/1 net-next] inet: frags: remove inline on static in c file
From: Fabian Frederick @ 2014-11-04 19:44 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
remove __inline__ / inline and let compiler decide what to do
with static functions
Inspired-by: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/ipv4/ip_fragment.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 2811cc1..4d964da 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -80,7 +80,7 @@ struct ipq {
struct inet_peer *peer;
};
-static inline u8 ip4_frag_ecn(u8 tos)
+static u8 ip4_frag_ecn(u8 tos)
{
return 1 << (tos & INET_ECN_MASK);
}
@@ -148,7 +148,7 @@ static void ip4_frag_init(struct inet_frag_queue *q, const void *a)
inet_getpeer_v4(net->ipv4.peers, arg->iph->saddr, 1) : NULL;
}
-static __inline__ void ip4_frag_free(struct inet_frag_queue *q)
+static void ip4_frag_free(struct inet_frag_queue *q)
{
struct ipq *qp;
@@ -160,7 +160,7 @@ static __inline__ void ip4_frag_free(struct inet_frag_queue *q)
/* Destruction primitives. */
-static __inline__ void ipq_put(struct ipq *ipq)
+static void ipq_put(struct ipq *ipq)
{
inet_frag_put(&ipq->q, &ip4_frags);
}
@@ -236,7 +236,7 @@ out:
/* Find the correct entry in the "incomplete datagrams" queue for
* this IP datagram, and create new one, if nothing is found.
*/
-static inline struct ipq *ip_find(struct net *net, struct iphdr *iph, u32 user)
+static struct ipq *ip_find(struct net *net, struct iphdr *iph, u32 user)
{
struct inet_frag_queue *q;
struct ip4_create_arg arg;
@@ -256,7 +256,7 @@ static inline struct ipq *ip_find(struct net *net, struct iphdr *iph, u32 user)
}
/* Is the fragment too far ahead to be part of ipq? */
-static inline int ip_frag_too_far(struct ipq *qp)
+static int ip_frag_too_far(struct ipq *qp)
{
struct inet_peer *peer = qp->peer;
unsigned int max = sysctl_ipfrag_max_dist;
@@ -795,16 +795,16 @@ static void __init ip4_frags_ctl_register(void)
register_net_sysctl(&init_net, "net/ipv4", ip4_frags_ctl_table);
}
#else
-static inline int ip4_frags_ns_ctl_register(struct net *net)
+static int ip4_frags_ns_ctl_register(struct net *net)
{
return 0;
}
-static inline void ip4_frags_ns_ctl_unregister(struct net *net)
+static void ip4_frags_ns_ctl_unregister(struct net *net)
{
}
-static inline void __init ip4_frags_ctl_register(void)
+static void __init ip4_frags_ctl_register(void)
{
}
#endif
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] udp: remove else after return
From: Fabian Frederick @ 2014-11-04 19:48 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
else is unnecessary after return 0 in __udp4_lib_rcv()
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/ipv4/udp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index cd0db54..cf0cece 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1777,14 +1777,14 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
if (ret > 0)
return -ret;
return 0;
- } else {
- if (rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST))
- return __udp4_lib_mcast_deliver(net, skb, uh,
- saddr, daddr, udptable);
-
- sk = __udp4_lib_lookup_skb(skb, uh->source, uh->dest, udptable);
}
+ if (rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST))
+ return __udp4_lib_mcast_deliver(net, skb, uh,
+ saddr, daddr, udptable);
+
+ sk = __udp4_lib_lookup_skb(skb, uh->source, uh->dest, udptable);
+
if (sk != NULL) {
int ret;
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] igmp: remove camel case definitions
From: Fabian Frederick @ 2014-11-04 19:52 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
use standard uppercase for definitions
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/ipv4/igmp.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index fb70e3e..3f80513 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -112,17 +112,17 @@
#ifdef CONFIG_IP_MULTICAST
/* Parameter names and values are taken from igmp-v2-06 draft */
-#define IGMP_V1_Router_Present_Timeout (400*HZ)
-#define IGMP_V2_Router_Present_Timeout (400*HZ)
-#define IGMP_V2_Unsolicited_Report_Interval (10*HZ)
-#define IGMP_V3_Unsolicited_Report_Interval (1*HZ)
-#define IGMP_Query_Response_Interval (10*HZ)
-#define IGMP_Query_Robustness_Variable 2
+#define IGMP_V1_ROUTER_PRESENT_TIMEOUT (400*HZ)
+#define IGMP_V2_ROUTER_PRESENT_TIMEOUT (400*HZ)
+#define IGMP_V2_UNSOLICITED_REPORT_INTERVAL (10*HZ)
+#define IGMP_V3_UNSOLICITED_REPORT_INTERVAL (1*HZ)
+#define IGMP_QUERY_RESPONSE_INTERVAL (10*HZ)
+#define IGMP_QUERY_ROBUSTNESS_VARIABLE 2
-#define IGMP_Initial_Report_Delay (1)
+#define IGMP_INITIAL_REPORT_DELAY (1)
-/* IGMP_Initial_Report_Delay is not from IGMP specs!
+/* IGMP_INITIAL_REPORT_DELAY is not from IGMP specs!
* IGMP specs require to report membership immediately after
* joining a group, but we delay the first report by a
* small interval. It seems more natural and still does not
@@ -879,15 +879,15 @@ static bool igmp_heard_query(struct in_device *in_dev, struct sk_buff *skb,
if (ih->code == 0) {
/* Alas, old v1 router presents here. */
- max_delay = IGMP_Query_Response_Interval;
+ max_delay = IGMP_QUERY_RESPONSE_INTERVAL;
in_dev->mr_v1_seen = jiffies +
- IGMP_V1_Router_Present_Timeout;
+ IGMP_V1_ROUTER_PRESENT_TIMEOUT;
group = 0;
} else {
/* v2 router present */
max_delay = ih->code*(HZ/IGMP_TIMER_SCALE);
in_dev->mr_v2_seen = jiffies +
- IGMP_V2_Router_Present_Timeout;
+ IGMP_V2_ROUTER_PRESENT_TIMEOUT;
}
/* cancel the interface change timer */
in_dev->mr_ifc_count = 0;
@@ -899,7 +899,7 @@ static bool igmp_heard_query(struct in_device *in_dev, struct sk_buff *skb,
return true; /* ignore bogus packet; freed by caller */
} else if (IGMP_V1_SEEN(in_dev)) {
/* This is a v3 query with v1 queriers present */
- max_delay = IGMP_Query_Response_Interval;
+ max_delay = IGMP_QUERY_RESPONSE_INTERVAL;
group = 0;
} else if (IGMP_V2_SEEN(in_dev)) {
/* this is a v3 query with v2 queriers present;
@@ -1218,7 +1218,7 @@ static void igmp_group_added(struct ip_mc_list *im)
return;
if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev)) {
spin_lock_bh(&im->lock);
- igmp_start_timer(im, IGMP_Initial_Report_Delay);
+ igmp_start_timer(im, IGMP_INITIAL_REPORT_DELAY);
spin_unlock_bh(&im->lock);
return;
}
@@ -1541,7 +1541,7 @@ static struct in_device *ip_mc_find_dev(struct net *net, struct ip_mreqn *imr)
int sysctl_igmp_max_memberships __read_mostly = IP_MAX_MEMBERSHIPS;
int sysctl_igmp_max_msf __read_mostly = IP_MAX_MSF;
#ifdef CONFIG_IP_MULTICAST
-int sysctl_igmp_qrv __read_mostly = IGMP_Query_Robustness_Variable;
+int sysctl_igmp_qrv __read_mostly = IGMP_QUERY_ROBUSTNESS_VARIABLE;
#endif
static int ip_mc_del1_src(struct ip_mc_list *pmc, int sfmode,
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 02/20] selftests/net: move test out of Makefile into a shell script
From: David Miller @ 2014-11-04 19:53 UTC (permalink / raw)
To: shuahkh
Cc: gregkh, akpm, mmarek, keescook, tranmanphong, dh.herrmann, hughd,
bobby.prani, ebiederm, serge.hallyn, linux-kbuild, linux-kernel,
linux-api, netdev
In-Reply-To: <51d57725e89809806bd8af5555af341296cc4dd7.1415117102.git.shuahkh@osg.samsung.com>
From: Shuah Khan <shuahkh@osg.samsung.com>
Date: Tue, 4 Nov 2014 10:10:58 -0700
> Currently bpf test run from the Makefile. Move it out of the
> Makefile to be run from a shell script to allow the test to
> be run as stand-alone test, in addition to allowing the test
> run from a make target.
>
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH 00/13] net_sched: misc cleanups and improvements
From: David Miller @ 2014-11-04 19:56 UTC (permalink / raw)
To: eric.dumazet; +Cc: xiyou.wangcong, netdev, jhs
In-Reply-To: <1415125250.25370.2.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 04 Nov 2014 10:20:50 -0800
> On Tue, 2014-11-04 at 09:56 -0800, Cong Wang wrote:
>> Minor code cleanups for TC qdiscs and filters, each patch has
>> more details.
>>
>> Cong Wang (13):
>> net_sched: refactor out tcf_exts
>> net_sched: introduce qdisc_peek() helper function
>> net_sched: rename ->gso_skb to ->dequeued_skb
>> net_sched: rename qdisc_drop() to qdisc_drop_skb()
>> net_sched: introduce qdisc_drop() helper function
>> net_sched: move some qdisc flag into qdisc ops
>> net_sched: move TCQ_F_MQROOT into qdisc ops
>> net_sched: use a flag to indicate fifo qdiscs instead of the name
>> net_sched: redefine qdisc_create_dflt()
>> net_sched: forbid setting default qdisc to inappropriate ones
>> net_sched: remove hashmask from Qdisc_class_hash
>> net_sched: remove useless qdisc_stab_lock
>> net_sched: return NULL instead of ERR_PTR for qdisc_alloc()
>>
>
> NACK for the whole serie.
>
> I am tired of your inexistent changelogs and code churn in qdisc.
>
> If you do not care of writing good changelogs and explain why you want
> all this, why should I care spending time to review all this stuff ?
>
> What is the plan, beyond all this code churn ?
+1
^ permalink raw reply
* Re: [PATCH 1/1 net-next] esp4: remove assignment in if condition
From: David Miller @ 2014-11-04 20:07 UTC (permalink / raw)
To: joe
Cc: fabf, linux-kernel, steffen.klassert, herbert, kuznet, jmorris,
yoshfuji, kaber, netdev
In-Reply-To: <1415129631.23168.3.camel@perches.com>
From: Joe Perches <joe@perches.com>
Date: Tue, 04 Nov 2014 11:33:51 -0800
> On Tue, 2014-11-04 at 20:28 +0100, Fabian Frederick wrote:
>
> trivia:
>
>> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> []
>> @@ -392,8 +392,11 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
>> if (elen <= 0)
>> goto out;
>>
>> - if ((err = skb_cow_data(skb, 0, &trailer)) < 0)
>> + err = skb_cow_data(skb, 0, &trailer);
>> +
>> + if (err < 0)
>> goto out;
>
> Generally it's better/more common to use
>
> foo = bar();
> if (!foo)
> [error_handler...]
>
> without the blank line between the set
> and the test.
+1
^ permalink raw reply
* Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net on arndale platform
From: Charles Keepax @ 2014-11-04 20:09 UTC (permalink / raw)
To: Stam, Michel [FINT]
Cc: Riku Voipio, davem, linux-usb, netdev, linux-kernel,
linux-samsung-soc
In-Reply-To: <C89EFD3CD56F64468D3D206D683A8D2203985FA9@ldam-msx2.fugro-nl.local>
On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> Hello Riku,
>
> >Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> this case on arndale). This is now a regression of functionality from
> 3.17.
> >
> >I think it would better to revert the change now and with less hurry
> introduce a ethtool fix that doesn't break arndale.
>
> I don't fully agree here;
> I would like to point out that this commit is a revert itself. Fixing
> the armdale will then cause breakage in other implementations, such as
> ours. Blankly reverting breaks other peoples' implementations.
>
> The PHY reset is the thing that breaks ethtool support, so any fix that
> appeases all would have to take existing PHY state into account.
>
> I'm not an expert on the ASIX driver, nor the MII, but I think this is
> the cause;
> drivers/net/usb/asix_devices.c:
> 361 asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
> 362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> 363 ADVERTISE_ALL | ADVERTISE_CSMA);
> 364 mii_nway_restart(&dev->mii);
>
> I would think that the ADVERTISE_ALL is the cause here, as it will reset
> the MII back to default, thus overriding ethtool settings.
> Would an:
> Int reg;
> reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
>
> prior to the offending lines, and then;
>
> 362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> 363 reg);
>
> solve the problem for you guys?
If I revert the patch in question and add this in:
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev)
{
struct asix_data *data = (struct asix_data *)&dev->data;
int ret, embd_phy;
+ int reg;
u16 rx_ctl;
ret = asix_write_gpio(dev,
@@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
msleep(150);
asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
- asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
- ADVERTISE_ALL | ADVERTISE_CSMA);
+ reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
+ if (!reg)
+ reg = ADVERTISE_ALL | ADVERTISE_CSMA;
+ asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, reg);
mii_nway_restart(&dev->mii);
ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
Then things work on Arndale for me. Does that work for you?
Whether that is a sensible fix I don't know however.
>
> Mind, maybe the read function should take into account the reset value
> of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't have
> any documention here at the moment.
Yeah I also have no documentation.
Thanks,
Charles
>
> Is anyone able to confirm my suspicions?
>
> Kind regards,
>
> Michel Stam
> -----Original Message-----
> From: Riku Voipio [mailto:riku.voipio@iki.fi]
> Sent: Tuesday, November 04, 2014 10:44 AM
> To: Stam, Michel [FINT]
> Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-samsung-soc@vger.kernel.org; ckeepax@opensource.wolfsonmicro.com
> Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
> on arndale platform
>
> On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > Interesting, as the commit itself is a revert from a kernel back to
> > 2.6 somewhere. The problem I had is related to the PHY being reset on
> > interface-up, can you confirm that you require this?
>
> I can't confirm what exactly is needed on arndale. I'm neither expert in
> USB or ethernet. However, I can confirm that without the PHY reset,
> networking doesn't work on arndale.
>
> I now see someone else has the same problem, adding Charles to CC.
>
> http://www.spinics.net/lists/linux-usb/msg116656.html
>
> > Reverting this
> > breaks ethtool support in turn.
>
> Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> this case on arndale). This is now a regression of functionality from
> 3.17.
>
> I think it would better to revert the change now and with less hurry
> introduce a ethtool fix that doesn't break arndale.
>
> > Kind regards,
> >
> > Michel Stam
> >
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 8:23 AM
> > To: davem@davemloft.net; Stam, Michel [FINT]
> > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org;
> > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net on
>
> > arndale platform
> >
> > Hi,
> >
> > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board),
> > fails to work. Interface is initialized but network traffic seem not
> > to pass through. With kernel IP config the result looks like:
> >
> > [ 3.323275] usb 3-3.2.4: new high-speed USB device number 4 using
> > exynos-ehci
> > [ 3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > idProduct=772a
> > [ 3.424735] usb 3-3.2.4: New USB device strings: Mfr=1, Product=2,
> > SerialNumber=3
> > [ 3.432196] usb 3-3.2.4: Product: AX88772
> > [ 3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > [ 3.441486] usb 3-3.2.4: SerialNumber: 000001
> > [ 3.447530] asix 3-3.2.4:1.0 (unnamed net_device) (uninitialized):
> > invalid hw address, using random
> > [ 3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> de:a2:66:bf:ca:4f
> > [ 4.488773] asix 3-3.2.4:1.0 eth0: link down
> > [ 5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [ 5.712947] Sending DHCP requests ...... timed out!
> > [ 83.165303] IP-Config: Retrying forever (NFS root)...
> > [ 83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [ 83.192944] Sending DHCP requests .....
> >
> > Similar results also with dhclient. Git bisect identified the breaking
>
> > commit as:
> >
> > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > Author: Michel Stam <m.stam@fugro.nl>
> > Date: Thu Oct 2 10:22:02 2014 +0200
> >
> > asix: Don't reset PHY on if_up for ASIX 88772
> >
> > Taking 3.18-rc3 and that commit reverted, network works again:
> >
> > [ 3.303500] usb 3-3.2.4: new high-speed USB device number 4 using
> > exynos-ehci
> > [ 3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > idProduct=772a
> > [ 3.404963] usb 3-3.2.4: New USB device strings: Mfr=1, Product=2,
> > SerialNumber=3
> > [ 3.412424] usb 3-3.2.4: Product: AX88772
> > [ 3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > [ 3.421715] usb 3-3.2.4: SerialNumber: 000001
> > [ 3.427755] asix 3-3.2.4:1.0 (unnamed net_device) (uninitialized):
> > invalid hw address, using random
> > [ 3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> 12:59:f1:a8:43:90
> > [ 7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [ 7.118258] Sending DHCP requests ., OK
> > [ 9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my address
> > is 192.168.1.111
> >
> > There might something wrong on the samsung platform code (I understand
>
> > the USB on arndale is "funny"), but this is still an regression from
> > 3.17.
> >
> > Riku
^ permalink raw reply
* Re: [PATCH v2 5/5] stmmac: pci: remove FSF address
From: Xose Vazquez Perez @ 2014-11-04 20:09 UTC (permalink / raw)
To: netdev, davem, andriy.shevchenko
David Miller wrote:
> Andy Shevchenko wrote:
>> The FSF address is subject to change, thus remove it from the file.
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> I cound 90t instances of this under drivers/net, therefore if this change is
^^^^^^^^^^^^^
> appropriate I'd rather someone script this and kill it across entire
> subdirectories.
It's a bit more, 555:
134 * 51 Franklin Street, Fifth Floor, Boston, MA 02110, USA
96 * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
89 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
86 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110,
44 * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA or on the
43 Boston, MA 02110-1301, USA.
19 * Temple Place - Suite 330, Boston, MA 02111-1307, USA.
10 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
8 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
5 * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
5 * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
5 # 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
3 * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
2 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA or on the
1 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
1 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110,
1 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
1 * Boston, MA 02110-1301, USA.
1 * Boston, MA 02110-1301, USA.
1 * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] cipso: kerneldoc warning fix
From: David Miller @ 2014-11-04 20:10 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415128201-8246-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 4 Nov 2014 20:10:01 +0100
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] ipv4: include linux/bug.h instead of asm/bug.h
From: David Miller @ 2014-11-04 20:10 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415128430-10674-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 4 Nov 2014 20:13:50 +0100
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH v2 net] tcp: zero retrans_stamp if all retrans were acked
From: Neal Cardwell @ 2014-11-04 20:10 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: Netdev, Yuchung Cheng, Eric Dumazet
In-Reply-To: <f58c4b02028e0750e583518608891d20742c0f38.1415128437.git.mleitner@redhat.com>
On Tue, Nov 4, 2014 at 2:15 PM, Marcelo Ricardo Leitner
<mleitner@redhat.com> wrote:
...
> Therefore, now we clear retrans_stamp as soon as all data during the
> loss window is fully acked.
>
> Reported-by: Ueki Kohei
> Cc: Neal Cardwell <ncardwell@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
> ---
>
> Notes:
> v1->v2: fixed compilation issue noticed by Neal
>
> net/ipv4/tcp_input.c | 60 +++++++++++++++++++++++++++-------------------------
> 1 file changed, 31 insertions(+), 29 deletions(-)
Acked-by: Neal Cardwell <ncardwell@google.com>
Tested-by: Neal Cardwell <ncardwell@google.com>
Code looks fine, and it passes Yuchung's packetdrill test case for this.
Thanks for finding and fixing this, Marcelo.
neal
^ permalink raw reply
* Re: [PATCH 1/1 net-next] cipso: include linux/bug.h instead of asm/bug.h
From: David Miller @ 2014-11-04 20:10 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415128541-10891-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 4 Nov 2014 20:15:41 +0100
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
This is already the case in the net-next tree, therefore this patch is
not applicable.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] cipso: remove NULL assignment on static
From: David Miller @ 2014-11-04 20:10 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415128759-11264-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 4 Nov 2014 20:19:19 +0100
> Also add blank line after structure declarations
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] tcp: spelling s/plugable/pluggable
From: David Miller @ 2014-11-04 20:10 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415129138-11901-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 4 Nov 2014 20:25:38 +0100
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] ipv4: use seq_puts instead of seq_printf where possible
From: David Miller @ 2014-11-04 20:11 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415129486-13185-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 4 Nov 2014 20:31:26 +0100
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] ipv4: remove 0/NULL assignment on static
From: David Miller @ 2014-11-04 20:11 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415129866-13852-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 4 Nov 2014 20:37:46 +0100
> static values are automatically initialized to 0
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] ipv4: ip_frag_queue function clean-up
From: David Miller @ 2014-11-04 20:12 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415130097-14490-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 4 Nov 2014 20:41:37 +0100
> -remove unnecessary else after break
> -declare free_it sk_buff * only once (like prev and next)
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
I disagree.
Declaring a local variable in it's inner-most scope is preferable
to moving it to the top level and making the job of the compiler
and the code auditor more difficult.
I'm not applying this, sorry.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] inet: frags: remove inline on static in c file
From: David Miller @ 2014-11-04 20:12 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415130244-15003-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 4 Nov 2014 20:44:04 +0100
> remove __inline__ / inline and let compiler decide what to do
> with static functions
>
> Inspired-by: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] udp: remove else after return
From: David Miller @ 2014-11-04 20:12 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415130521-16479-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 4 Nov 2014 20:48:41 +0100
> else is unnecessary after return 0 in __udp4_lib_rcv()
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox