Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v2 5/7] rhashtable: avoid unnecessary wakeup for worker queue
From: Ying Xue @ 2015-01-07 10:28 UTC (permalink / raw)
  To: Thomas Graf; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion, davem
In-Reply-To: <20150107095013.GJ21820@casper.infradead.org>

On 01/07/2015 05:50 PM, Thomas Graf wrote:
> On 01/07/15 at 01:41pm, Ying Xue wrote:
>> Move condition statements of verifying whether hash table size exceeds
>> its maximum threshold or reaches its minimum threshold from resizing
>> functions to resizing decision functions, avoiding unnecessary wakeup
>> for worker queue thread.
>>
>> Signed-off-by: Ying Xue <ying.xue@windriver.com>
>> Cc: Thomas Graf <tgraf@suug.ch>
> 
> Good optimization, thanks!
> 
> Acked-by: Thomas Graf <tgraf@suug.ch>
> 
> Can you do a follow-up patch and add a note in rhashtable.h to
> indicate the implementation of the grow and shrink decision function
> must enforce min/max shift?
> 
> 

Thanks for the reminder, and I will do this later.

Regards,
Ying


------------------------------------------------------------------------------
Dive into the World of Parallel Programming! The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net

^ permalink raw reply

* Re: IPsec workshop at netdev01?
From: Steffen Klassert @ 2015-01-07 10:31 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev, Jamal Hadi Salim, Herbert Xu, David Miller
In-Reply-To: <20150106170026.GD11324@breakpoint.cc>

On Tue, Jan 06, 2015 at 06:00:26PM +0100, Florian Westphal wrote:
> Steffen Klassert <steffen.klassert@secunet.com> wrote:
> > - We still lack a 32/64 bit compatibiltiy layer for IPsec, this issue
> >   comes up from time to time. Some solutions were proposed in the past
> >   but all had problems. The current behaviour is broken if someone tries
> >   to configure IPsec with 32 bit tools on a 64 bit machine. Can we get
> >   this right somehow or is it better to just return an error in this case?
> 
> FWIW I think
> http://patchwork.ozlabs.org/patch/49465/
> 
> came closest to achieving full CONFIG_COMPAT support; since netlink is
> no longer async now I'm not sure we'd still need additonal 32-compat syscalls
> to make compat work for all cases.
> 
> So "its ugly as hell" is probably the only problem that is hard to avoid ;-)

Yeah, and it will be no fun to maintain it...

So the question is still, do we really need/want it or should we
tell that this is not supported. We just can't leave it as it is.
We allow to configure with 32 bit tools, but the result is crap.

^ permalink raw reply

* Re: [PATCH iproute2 -next] ip: route: add congestion control metric
From: Vadim Kochan @ 2015-01-07 10:22 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Stephen Hemminger, fw, netdev
In-Reply-To: <54AD08A9.9000205@redhat.com>

On Wed, Jan 07, 2015 at 11:21:29AM +0100, Daniel Borkmann wrote:
> On 01/07/2015 02:09 AM, Stephen Hemminger wrote:
> >On Wed,  7 Jan 2015 00:52:37 +0100
> >Daniel Borkmann <dborkman@redhat.com> wrote:
> >
> >>+		} else if (matches(*argv, "congctl") == 0) {
> >>+			char cc[16];
> >>+			NEXT_ARG();
> >>+			memset(cc, 0, sizeof(cc));
> >>+			if (strcmp(*argv, "lock") == 0) {
> >>+				mxlock |= (1<<RTAX_CC_ALGO);
> >
> >Unneeded paren
> 
 And what about spaces arount "<<" ?

^ permalink raw reply

* Re: [PATCH net-next v2 6/7] rhashtable: initialize atomic nelems variable
From: Thomas Graf @ 2015-01-07 10:36 UTC (permalink / raw)
  To: Ying Xue
  Cc: davem, jon.maloy, Paul.Gortmaker, erik.hugne, netdev,
	tipc-discussion
In-Reply-To: <54AD09CD.3010100@windriver.com>

On 01/07/15 at 06:26pm, Ying Xue wrote:
> I am not sure whether we really need to reinitialize atomic variable
> again although we have reset it with memset() or something else. But I
> see many places in kernel where we do this, for example:
> 
> Although we use kmem_cache_zalloc() to allocate "net" structure instance
> in net_alloc(), there are still several places where to reinitialize its
> atomic variables again:
> 
> setup_net()
>   atomic_set(&net->use_count, 0);
> 
> rt_genid_init()
>   atomic_set(&net->ipv4.rt_genid, 0);
>   atomic_set(&net->fnhe_genid, 0);
> 
> Can you please definitely confirm that the reinitialisation is redundant
> for us?

I see examples for both, explicit initialization and dependence on
a previous memset. I'm not sure what is the preferred way.

I'll provide my ACK since this obviously doesn't break anything and
leave it up to Dave.

Acked-by: Thomas Graf <tgraf@suug.ch>

^ permalink raw reply

* Re: [PATCH] wireless extensions should default to Y
From: Johannes Berg @ 2015-01-07 10:41 UTC (permalink / raw)
  To: Pavel Machek
  Cc: jikos-AlSwsSmVLrQ, kernel list, Linus Torvalds,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150107101903.GA6563@amd>

On Wed, 2015-01-07 at 11:19 +0100, Pavel Machek wrote:
> Do we need following patch on top of
> 24a0aa212ee2dbe44360288684478d76a8e20a0a revert?
> 
> I updated kernel today, and (probably because extensions were not
> selectable before), the default choice was "N", which is wrong:
> oldconfig should result in compatible choices being made, for example
> to help bisect.

I don't believe we need this. It has been defaulting to N for a long
time, it's just that it got thrown out of your config due to building
inbetween the patch and its revert. Had you built only before and after
that wouldn't be an issue.

If we default to Y now we send the wrong signal. If you really need to
bisect something wext related you have far bigger issues I'd think, the
code hasn't changed *forever*.

johannes

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

^ permalink raw reply

* Re: [PATCH net] ipv6: Prevent ipv6_find_hdr() from returning ENOENT for valid non-first fragments
From: Hannes Frederic Sowa @ 2015-01-07 10:43 UTC (permalink / raw)
  To: Rahul Sharma; +Cc: Pablo Neira Ayuso, netdev, linux-kernel, netfilter-devel
In-Reply-To: <CAFB3abwKpR1VqZXUrsexqveAZrA2LTSmTe8a=-H7z2HjAptjwA@mail.gmail.com>

Hi,

On Mi, 2015-01-07 at 11:11 +0530, Rahul Sharma wrote:
> On Wed, Jan 7, 2015 at 4:17 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Wed, Jan 07, 2015 at 03:03:20AM +0530, Rahul Sharma wrote:
> >> ipv6_find_hdr() currently assumes that the next-header field in the
> >> fragment header of the non-first fragment is the "protocol number of
> >> the last header" (here last header excludes any extension header
> >> protocol numbers ) which is incorrect as per RFC2460. The next-header
> >> value is the first header of the fragmentable part of the original
> >> packet (which can be extension header as well).
> >> This can create reassembly problems. For example: Fragmented
> >> authenticated OSPFv3 packets (where AH header is inserted before the
> >> protocol header). For the second fragment, the next header value in
> >> the fragment header will be NEXTHDR_AUTH which is correct but
> >> ipv6_find_hdr will return ENOENT since AH is an extension header
> >> resulting in second fragment getting dropped. This check for the
> >> presence of non-extension header needs to be removed.
> >>
> >> Signed-off-by: Rahul Sharma <rsharma@arista.com>
> >> ---
> >> --- linux-3.18.1/net/ipv6/exthdrs_core.c.orig   2015-01-06
> >> 10:25:36.411419863 -0800
> >> +++ linux-3.18.1/net/ipv6/exthdrs_core.c        2015-01-06
> >> 10:51:45.819364986 -0800
> >> @@ -171,10 +171,11 @@ EXPORT_SYMBOL_GPL(ipv6_find_tlv);
> >>   * If the first fragment doesn't contain the final protocol header or
> >>   * NEXTHDR_NONE it is considered invalid.
> >>   *
> >> - * Note that non-1st fragment is special case that "the protocol number
> >> - * of last header" is "next header" field in Fragment header. In this case,
> >> - * *offset is meaningless and fragment offset is stored in *fragoff if fragoff
> >> - * isn't NULL.
> >> + * Note that non-1st fragment is special case that "the protocol number of the
> >> + * first header of the fragmentable part of the original packet" is
> >> + * "next header" field in the Fragment header. In this case, *offset is
> >> + * meaningless and fragment offset is stored in *fragoff if fragoff isn't
> >> + * NULL.
> >>   *
> >>   * if flags is not NULL and it's a fragment, then the frag flag
> >>   * IP6_FH_F_FRAG will be set. If it's an AH header, the
> >> @@ -250,9 +251,7 @@ int ipv6_find_hdr(const struct sk_buff *
> >>
> >>                         _frag_off = ntohs(*fp) & ~0x7;
> >>                         if (_frag_off) {
> >> -                               if (target < 0 &&
> >> -                                   ((!ipv6_ext_hdr(hp->nexthdr)) ||
> >
> > This check assumes that the following headers cannot show up in the
> > fragmented part of the IPv6 packet:
> >
> >  12 bool ipv6_ext_hdr(u8 nexthdr)
> >  13 {
> >  14         /*
> >  15          * find out if nexthdr is an extension header or a protocol
> >  16          */
> >  17         return   (nexthdr == NEXTHDR_HOP)       ||
> >  18                  (nexthdr == NEXTHDR_ROUTING)   ||
> >  19                  (nexthdr == NEXTHDR_FRAGMENT)  ||
> >  20                  (nexthdr == NEXTHDR_AUTH)      ||
> >  21                  (nexthdr == NEXTHDR_NONE)      ||
> >  22                  (nexthdr == NEXTHDR_DEST);
> >
> >> -                                    hp->nexthdr == NEXTHDR_NONE)) {
> >> +                               if (target < 0) {
> >>                                         if (fragoff)
> >>                                                 *fragoff = _frag_off;
> >>                                         return hp->nexthdr;
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> I think this is incorrect. Authentication header shows up in the
> fragmentable part of the original IPv6 packet. So, for the non-first
> fragments the next-header field value can be NEXTHDR_AUTH.

Pablo's mail got me thinking again.

In general, IPv6 extension headers can appear in any order and stacks
must be process them. Fragmentation adds a limitation, that some
extension headers do not make sense and don't have any effect if they
appear after a fragmentation header (HbH and ROUTING).

Looking at the rest of the function we don't check for HBHHDR or RTHDR
following a fragmentation header either if we process the first fragment
(core stack only processes HBH if directly following the ipv6 header
anyway).

So, in my opinion, it is safe to completely remove this check and it
would align if the rest of the extension processing logic. The callers
all seem fine with that.

Pablo, what do you think?

Anyway, the patch does not apply cleanly, the patch header is mangled.
Could you check and send again?

Thanks,
Hannes

^ permalink raw reply

* Re: ipv6: oops in datagram.c line 260
From: Hannes Frederic Sowa @ 2015-01-07 10:45 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: Chris Ruehl, netdev, davem
In-Reply-To: <20150107072254.GA13046@secunet.com>

On Mi, 2015-01-07 at 08:22 +0100, Steffen Klassert wrote:
> On Tue, Jan 06, 2015 at 05:01:13PM +0100, Hannes Frederic Sowa wrote:
> > 
> > xfrm6_output_finish unconditionally resets skb->protocol so we try to
> > dispatch to the IPv6 handler, even though tcp just sends an IPv4 packet.
> 
> Maybe we better dispatch based on sk->sk_family, this should give
> always the right address family of the socket.

The original problem was dealing with IPv4/v6 mapped traffic. Processing
local errors from unconnected UDP sockets which are emitting both IPv4
and IPv6 frames won't play nicely with sk->sk_family I fear.

Bye,
Hannes

^ permalink raw reply

* Re: [PATCH 2/6] vxlan: Group Policy extension
From: Thomas Graf @ 2015-01-07 11:01 UTC (permalink / raw)
  To: David Laight
  Cc: 'Alexei Starovoitov', David S. Miller, Jesse Gross,
	Stephen Hemminger, Pravin Shelar, netdev@vger.kernel.org,
	dev@openvswitch.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAC2740@AcuExch.aculab.com>

On 01/07/15 at 10:03am, David Laight wrote:
> From: Alexei Starovoitov
> > On Tue, Jan 6, 2015 at 6:05 PM, Thomas Graf <tgraf@suug.ch> wrote:
> > > +struct vxlan_gbp {
> > > +#ifdef __LITTLE_ENDIAN_BITFIELD
> > > +       __u8    reserved_flags1:3,
> > ...
> > > +       __be16 policy_id;
> > > +} __packed;
> > 
> > are you sure that compiler will be smart enough
> > to do single short load when you pack
> > u8 + struct vxlan_gbp inside struct vxlanhdr ?
> > I suspect compiler will use two byte loads
> > with shifts and ors every time to access policy_id.
> > Even it works ok, I think this struct layout is ugly.
> > imo would be much easier to read if you replace
> > the whole vxlanhdr with vxlanhdr_gbp
> > or split vxlanhdr into two 32-bit structs.
> > then __packed hacks won't be needed.

If I read objdump output correctly, gcc seems fine with it:

        /* For backwards compatibility, only allow reserved fields to be
         * used by VXLAN extensions if explicitly requested.
         */
        if (vs->exts) {
                if (!vxh->vni_present)
    2640:       41 0f b6 55 08          movzbl 0x8(%r13),%edx
    2645:       f6 c2 08                test   $0x8,%dl
    2648:       74 c2                   je     260c <vxlan_udp_encap_recv+0x9c>
    [...]
                        md.gbp = ntohs(vxh->gbp.policy_id);
    2652:       41 0f b7 55 0a          movzwl 0xa(%r13),%edx

Let me know what I have to do/provide to validate this properly.

> Also, if you are writing the values then you need to write
> all the members of the bitfield in order to get a single write.
> 
> Basically you are much better off using explicit masks.

I went back and forth on this and chose to use individual bit fields
and map them to a static bit definition which is exported via Netlink.
That way the user space Netlink interface remains stable should the
wire protocol ever change. Yes, this implies some branching which could
be avoided right now as long as user and wire protocol are identical. I
did not observe any performance differences in benchmarks though.

^ permalink raw reply

* Re: [PATCH] wireless extensions should default to Y
From: Pavel Machek @ 2015-01-07 11:03 UTC (permalink / raw)
  To: Johannes Berg
  Cc: jikos, kernel list, Linus Torvalds, davem, linux-wireless, netdev
In-Reply-To: <1420627315.3407.2.camel@sipsolutions.net>

On Wed 2015-01-07 11:41:55, Johannes Berg wrote:
> On Wed, 2015-01-07 at 11:19 +0100, Pavel Machek wrote:
> > Do we need following patch on top of
> > 24a0aa212ee2dbe44360288684478d76a8e20a0a revert?
> > 
> > I updated kernel today, and (probably because extensions were not
> > selectable before), the default choice was "N", which is wrong:
> > oldconfig should result in compatible choices being made, for example
> > to help bisect.
> 
> I don't believe we need this. It has been defaulting to N for a long
> time, it's just that it got thrown out of your config due to building
> inbetween the patch and its revert. Had you built only before and after
> that wouldn't be an issue.

Well, I clearly hit the issue. If someone had _not_ build in between,
the "default y" does not change anything for him, as he will not be
asked thequestion. If someone starts config from scratch, Y is the
safe answer.

> If we default to Y now we send the wrong signal. If you really need to
> bisect something wext related you have far bigger issues I'd think, the
> code hasn't changed *forever*.

I am woried about bisecting something unrelated, and then wext coming
and breaking the bisect. But you are right that it will break the
bisect, anyway...

                                                                        Pavel
									
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH] wireless extensions should default to Y
From: Johannes Berg @ 2015-01-07 11:06 UTC (permalink / raw)
  To: Pavel Machek
  Cc: jikos-AlSwsSmVLrQ, kernel list, Linus Torvalds,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150107110323.GB7368@amd>

On Wed, 2015-01-07 at 12:03 +0100, Pavel Machek wrote:
> On Wed 2015-01-07 11:41:55, Johannes Berg wrote:
> > On Wed, 2015-01-07 at 11:19 +0100, Pavel Machek wrote:
> > > Do we need following patch on top of
> > > 24a0aa212ee2dbe44360288684478d76a8e20a0a revert?
> > > 
> > > I updated kernel today, and (probably because extensions were not
> > > selectable before), the default choice was "N", which is wrong:
> > > oldconfig should result in compatible choices being made, for example
> > > to help bisect.
> > 
> > I don't believe we need this. It has been defaulting to N for a long
> > time, it's just that it got thrown out of your config due to building
> > inbetween the patch and its revert. Had you built only before and after
> > that wouldn't be an issue.
> 
> Well, I clearly hit the issue. If someone had _not_ build in between,
> the "default y" does not change anything for him, as he will not be
> asked thequestion. If someone starts config from scratch, Y is the
> safe answer.

We've long stated that if really starting from scratch it's the wrong
thing to do and had a default N, so I don't really know why we'd revert
that only to make a short period of bisect slightly happier?

I really don't want to merge this patch and encourage more people to
build with wext enabled.

johannes

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

^ permalink raw reply

* Re: [PATCH 2/6] vxlan: Group Policy extension
From: Thomas Graf @ 2015-01-07 11:10 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Jesse Gross, Stephen Hemminger, Pravin Shelar,
	netdev@vger.kernel.org, dev@openvswitch.org
In-Reply-To: <CAADnVQ+EO34qYbK+sji-Da3CMkn+-V-9Uvdar_WgxRQ4_+JFXA@mail.gmail.com>

On 01/06/15 at 07:37pm, Alexei Starovoitov wrote:
> Even it works ok, I think this struct layout is ugly.
> imo would be much easier to read if you replace
> the whole vxlanhdr with vxlanhdr_gbp
> or split vxlanhdr into two 32-bit structs.
> then __packed hacks won't be needed.

The main reason why I merged it into vxlanhdr is for documentation
purposes and to avoid duplicating the generic VXLAN header for every
extension. The RCO and GPE extensions would need to duplicate this
over and over. It gets messy in particular when multiple extensions
can be used in combination (such as GBP and RCO) which then each
have their own conflicting header definitions. This way, it is clear
which extensions are compatible by just looking at the definition
of the structure.

That said, I'm not married to this idea.

^ permalink raw reply

* [PATCH iproute2 0/3] ip netns: Run over all netns
From: Vadim Kochan @ 2015-01-07 11:04 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan

From: Vadim Kochan <vadim4j@gmail.com>

Allow 'ip netns del' and 'ip netns exec' run over each network namespace names.

'ip netns exec' executes command forcely on eacn nsname.

Vadim Kochan (3):
  lib: Exec func on each netns
  ip netns: Allow exec on each netns
  ip netns: Delete all netns

 include/namespace.h |  6 ++++
 include/utils.h     |  5 +++
 ip/ipnetns.c        | 96 ++++++++++++++++++++++++++++++++---------------------
 lib/namespace.c     | 22 ++++++++++++
 lib/utils.c         | 28 ++++++++++++++++
 man/man8/ip-netns.8 | 26 ++++++++++++---
 6 files changed, 141 insertions(+), 42 deletions(-)

-- 
2.1.3

^ permalink raw reply

* [PATCH iproute2 1/3] lib: Exec func on each netns
From: Vadim Kochan @ 2015-01-07 11:04 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan
In-Reply-To: <1420628662-9930-1-git-send-email-vadim4j@gmail.com>

From: Vadim Kochan <vadim4j@gmail.com>

Added possibility to run some func on each netns.

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 include/namespace.h |  6 ++++++
 include/utils.h     |  5 +++++
 lib/namespace.c     | 22 ++++++++++++++++++++++
 lib/utils.c         | 28 ++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+)

diff --git a/include/namespace.h b/include/namespace.h
index 2f13e65..1a7e066 100644
--- a/include/namespace.h
+++ b/include/namespace.h
@@ -42,5 +42,11 @@ static int setns(int fd, int nstype)
 #endif /* HAVE_SETNS */
 
 extern int netns_switch(char *netns);
+extern int netns_foreach(int (*func)(char *nsname, void *arg), void *arg);
+
+struct netns_func {
+	int (*func)(char *nsname, void *arg);
+	void *arg;
+};
 
 #endif /* __NAMESPACE_H__ */
diff --git a/include/utils.h b/include/utils.h
index eecbc39..3196561 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -5,6 +5,7 @@
 #include <asm/types.h>
 #include <resolv.h>
 #include <stdlib.h>
+#include <stdbool.h>
 
 #include "libnetlink.h"
 #include "ll_map.h"
@@ -160,4 +161,8 @@ struct iplink_req;
 int iplink_parse(int argc, char **argv, struct iplink_req *req,
 		char **name, char **type, char **link, char **dev,
 		int *group, int *index);
+
+extern int do_each_netns(int (*func)(char *nsname, void *arg), void *arg,
+		bool show_label);
+
 #endif /* __UTILS_H__ */
diff --git a/lib/namespace.c b/lib/namespace.c
index 1554ce0..a0f7c6d 100644
--- a/lib/namespace.c
+++ b/lib/namespace.c
@@ -84,3 +84,25 @@ int netns_switch(char *name)
 	bind_etc(name);
 	return 0;
 }
+
+int netns_foreach(int (*func)(char *nsname, void *arg), void *arg)
+{
+	DIR *dir;
+	struct dirent *entry;
+
+	dir = opendir(NETNS_RUN_DIR);
+	if (!dir)
+		return -1;
+
+	while ((entry = readdir(dir)) != NULL) {
+		if (strcmp(entry->d_name, ".") == 0)
+			continue;
+		if (strcmp(entry->d_name, "..") == 0)
+			continue;
+		if (func(entry->d_name, arg))
+			break;
+	}
+
+	closedir(dir);
+	return 0;
+}
diff --git a/lib/utils.c b/lib/utils.c
index 64915f3..e3ca11b 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -31,6 +31,7 @@
 
 
 #include "utils.h"
+#include "namespace.h"
 
 int timestamp_short = 0;
 
@@ -868,3 +869,30 @@ int inet_get_addr(const char *src, __u32 *dst, struct in6_addr *dst6)
 	else
 		return inet_pton(AF_INET, src, dst);
 }
+
+static int on_netns(char *nsname, void *arg)
+{
+	struct netns_func *f = arg;
+
+	if (netns_switch(nsname))
+		return -1;
+
+	return f->func(nsname, f->arg);
+}
+
+static int on_netns_label(char *nsname, void *arg)
+{
+	printf("\nnetns: %s\n", nsname);
+	return on_netns(nsname, arg);
+}
+
+int do_each_netns(int (*func)(char *nsname, void *arg), void *arg,
+		bool show_label)
+{
+	struct netns_func nsf = { .func = func, .arg = arg };
+
+	if (show_label)
+		return netns_foreach(on_netns_label, &nsf);
+
+	return netns_foreach(on_netns, &nsf);
+}
-- 
2.1.3

^ permalink raw reply related

* [PATCH iproute2 2/3] ip netns: Allow exec on each netns
From: Vadim Kochan @ 2015-01-07 11:04 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan
In-Reply-To: <1420628662-9930-1-git-send-email-vadim4j@gmail.com>

From: Vadim Kochan <vadim4j@gmail.com>

This change allows to exec some cmd on each
netns (except default) by specifying 'all' as netns name:

    # ip netns exec all ip link

Each command executes synchronously.

Exit status is not considered, so there might be a case
that some CMD can fail on some netns but success on the other.

EXAMPLES:

1) Show link info on all netns:

$ ip netns exec all ip link

netns: test_net
1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group default
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
4: tap0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast state DOWN mode DEFAULT group default qlen 500
    link/ether 1a:19:6f:25:eb:85 brd ff:ff:ff:ff:ff:ff

netns: home0
1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group default
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
4: tap0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast state DOWN mode DEFAULT group default qlen 500
    link/ether ea:1a:59:40:d3:29 brd ff:ff:ff:ff:ff:ff

netns: lan0
1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group default
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
4: tap0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast state DOWN mode DEFAULT group default qlen 500
    link/ether ce:49:d5:46:81:ea brd ff:ff:ff:ff:ff:ff

2) Set UP tap0 device for the all netns:

$ ip netns exec all ip link set dev tap0 up

netns: test_net

netns: home0

netns: lan0

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 ip/ipnetns.c        | 72 ++++++++++++++++++++++++++++++++---------------------
 man/man8/ip-netns.8 | 14 +++++++++--
 2 files changed, 55 insertions(+), 31 deletions(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 519d518..b5a6f57 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -26,7 +26,7 @@ static int usage(void)
 	fprintf(stderr, "       ip netns delete NAME\n");
 	fprintf(stderr, "       ip netns identify [PID]\n");
 	fprintf(stderr, "       ip netns pids NAME\n");
-	fprintf(stderr, "       ip netns exec NAME cmd ...\n");
+	fprintf(stderr, "       ip netns exec { NAME | all } cmd ...\n");
 	fprintf(stderr, "       ip netns monitor\n");
 	exit(-1);
 }
@@ -66,29 +66,10 @@ static int netns_list(int argc, char **argv)
 	return 0;
 }
 
-static int netns_exec(int argc, char **argv)
+static int cmd_exec(const char *cmd, char **argv, bool do_fork)
 {
-	/* Setup the proper environment for apps that are not netns
-	 * aware, and execute a program in that environment.
-	 */
-	const char *cmd;
-
-	if (argc < 1) {
-		fprintf(stderr, "No netns name specified\n");
-		return -1;
-	}
-	if (argc < 2) {
-		fprintf(stderr, "No command specified\n");
-		return -1;
-	}
-	cmd = argv[1];
-
-	if (netns_switch(argv[0]))
-		return -1;
-
 	fflush(stdout);
-
-	if (batch_mode) {
+	if (do_fork) {
 		int status;
 		pid_t pid;
 
@@ -106,23 +87,56 @@ static int netns_exec(int argc, char **argv)
 			}
 
 			if (WIFEXITED(status)) {
-				/* ip must return the status of the child,
-				 * but do_cmd() will add a minus to this,
-				 * so let's add another one here to cancel it.
-				 */
-				return -WEXITSTATUS(status);
+				return WEXITSTATUS(status);
 			}
 
 			exit(1);
 		}
 	}
 
-	if (execvp(cmd, argv + 1)  < 0)
+	if (execvp(cmd, argv)  < 0)
 		fprintf(stderr, "exec of \"%s\" failed: %s\n",
-			cmd, strerror(errno));
+				cmd, strerror(errno));
 	_exit(1);
 }
 
+static int on_netns_exec(char *nsname, void *arg)
+{
+	char **argv = arg;
+	cmd_exec(argv[1], argv + 1, 1);
+	return 0;
+}
+
+static int netns_exec(int argc, char **argv)
+{
+	/* Setup the proper environment for apps that are not netns
+	 * aware, and execute a program in that environment.
+	 */
+	const char *cmd;
+
+	if (argc < 1) {
+		fprintf(stderr, "No netns name specified\n");
+		return -1;
+	}
+	if (argc < 2) {
+		fprintf(stderr, "No command specified\n");
+		return -1;
+	}
+	cmd = argv[1];
+
+	if (strcmp(argv[0], "all") == 0)
+		return do_each_netns(on_netns_exec, argv, 1);
+
+	if (netns_switch(argv[0]))
+		return -1;
+
+	/* ip must return the status of the child,
+	 * but do_cmd() will add a minus to this,
+	 * so let's add another one here to cancel it.
+	 */
+	return -cmd_exec(cmd, argv + 1, batch_mode);
+}
+
 static int is_pid(const char *str)
 {
 	int ch;
diff --git a/man/man8/ip-netns.8 b/man/man8/ip-netns.8
index 74343ed..70ea4f0 100644
--- a/man/man8/ip-netns.8
+++ b/man/man8/ip-netns.8
@@ -29,7 +29,7 @@ ip-netns \- process network namespace management
 
 .ti -8
 .BR "ip netns exec "
-.I NETNSNAME command ...
+.RI "{ " NETNSNAME " | " all " } " command ...
 
 .ti -8
 .BR "ip netns monitor"
@@ -98,7 +98,7 @@ This command walks through proc and finds all of the process who have
 the named network namespace as their primary network namespace.
 
 .TP
-.B ip netns exec NAME cmd ... - Run cmd in the named network namespace
+.B ip netns exec { NAME | all } cmd ... - Run cmd in the named network namespace
 .sp
 This command allows applications that are network namespace unaware
 to be run in something other than the default network namespace with
@@ -107,6 +107,16 @@ in the customary global locations.  A network namespace and bind mounts
 are used to move files from their network namespace specific location
 to their default locations without affecting other processes.
 
+If
+.B all
+was specified then
+.B cmd
+will be executed synchronously on the each named network namespace even if
+.B cmd
+fails on some of them. Network namespace name is printed on each
+.B cmd
+executing.
+
 .TP
 .B ip netns monitor - Report as network namespace names are added and deleted
 .sp
-- 
2.1.3

^ permalink raw reply related

* [PATCH iproute2 3/3] ip netns: Delete all netns
From: Vadim Kochan @ 2015-01-07 11:04 UTC (permalink / raw)
  To: netdev; +Cc: Vadim Kochan
In-Reply-To: <1420628662-9930-1-git-send-email-vadim4j@gmail.com>

From: Vadim Kochan <vadim4j@gmail.com>

Allow delete all namespace names by:

    $ ip netns del all

Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
 ip/ipnetns.c        | 24 +++++++++++++++---------
 man/man8/ip-netns.8 | 12 ++++++++++--
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index b5a6f57..20707b8 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -274,18 +274,11 @@ static int netns_identify(int argc, char **argv)
 
 }
 
-static int netns_delete(int argc, char **argv)
+static int on_netns_del(char *nsname, void *arg)
 {
-	const char *name;
 	char netns_path[MAXPATHLEN];
 
-	if (argc < 1) {
-		fprintf(stderr, "No netns name specified\n");
-		return -1;
-	}
-
-	name = argv[0];
-	snprintf(netns_path, sizeof(netns_path), "%s/%s", NETNS_RUN_DIR, name);
+	snprintf(netns_path, sizeof(netns_path), "%s/%s", NETNS_RUN_DIR, nsname);
 	umount2(netns_path, MNT_DETACH);
 	if (unlink(netns_path) < 0) {
 		fprintf(stderr, "Cannot remove namespace file \"%s\": %s\n",
@@ -295,6 +288,19 @@ static int netns_delete(int argc, char **argv)
 	return 0;
 }
 
+static int netns_delete(int argc, char **argv)
+{
+	if (argc < 1) {
+		fprintf(stderr, "No netns name specified\n");
+		return -1;
+	}
+
+	if (strcmp(argv[0], "all") == 0)
+		return netns_foreach(on_netns_del, NULL);
+
+	return on_netns_del(argv[0], NULL);
+}
+
 static int create_netns_dir(void)
 {
 	/* Create the base netns directory if it doesn't exist */
diff --git a/man/man8/ip-netns.8 b/man/man8/ip-netns.8
index 70ea4f0..e56068e 100644
--- a/man/man8/ip-netns.8
+++ b/man/man8/ip-netns.8
@@ -16,10 +16,14 @@ ip-netns \- process network namespace management
 .BR "ip netns" " { " list " } "
 
 .ti -8
-.BR "ip netns" " { " add " | " delete " } "
+.B ip netns add
 .I NETNSNAME
 
 .ti -8
+.B ip netns del
+.RI "{ " NETNSNAME " | " all " }"
+
+.ti -8
 .BR "ip netns identify"
 .RI "[ " PID " ]"
 
@@ -76,7 +80,7 @@ If NAME is available in /var/run/netns/ this command creates a new
 network namespace and assigns NAME.
 
 .TP
-.B ip netns delete NAME - delete the name of a network namespace
+.B ip netns delete { NAME | all } - delete the name of a network namespace(s)
 .sp
 If NAME is present in /var/run/netns it is umounted and the mount
 point is removed.  If this is the last user of the network namespace the
@@ -84,6 +88,10 @@ network namespace will be freed, otherwise the network namespace
 persists until it has no more users.  ip netns delete may fail if
 the mount point is in use in another mount namespace.
 
+If
+.B all
+was specified then all the network namespace names will be removed.
+
 .TP
 .B ip netns identify [PID] - Report network namespaces names for process
 .sp
-- 
2.1.3

^ permalink raw reply related

* Re: [PATCH net-next 1/3] net: add IPv4 routing FIB support for swdev
From: Hannes Frederic Sowa @ 2015-01-07 11:23 UTC (permalink / raw)
  To: Shrijeet Mukherjee
  Cc: Scott Feldman, Netdev, Jiří Pírko, john fastabend,
	Thomas Graf, Jamal Hadi Salim, Andy Gospodarek, Roopa Prabhu
In-Reply-To: <98433f41e1876041471cdb513ca53fe5@mail.gmail.com>

On Di, 2015-01-06 at 18:08 -0800, Shrijeet Mukherjee wrote:
> >For the first idea, I'll try to make an example:
> >
> >Initial setup:
> ># ip rule ls
> >0:	from all lookup local
> >32766:	from all lookup main
> >32767:	from all lookup default
> >
> ># ip rule add pref 100 iif swdev0 table 5 # ip rule ls
> >0:	from all lookup local
> >100:	from all iif swdev0 [detached] lookup 5
> >> maybe we can show which rules are being able to get offloaded here
> >32766:	from all lookup main
> >32767:	from all lookup default
> >
> >table 5 should be the table we can insert routes into which are offloaded
> >to
> >hardware.
> >
> >During table modifications we linearly scan the rules if we find selectors
> >which
> >cannot be represented by hardware.
> >
> >In case we have a iif selector, we simply can use this table and just
> >synthesize it
> >into the particular interface.
> >
> >A ip-rule-from would need all the hardware being capable of matching source
> >addresses, otherwise we cannot offload all routing tables with higher
> >preference,
> >same for a to/tos rule. If we encounter a fwmark rule, we certainly cannot
> >represent it in hardware, so skip it (here we can think about entangling
> >those with
> >ACLs, but it feels hard to do).
> >
> >If rules are inserted or changed we must again validate the complete list
> >of rules
> >and decide if we need to flush all the routes and install a slow path via
> >kernel.
> >
> >What do you think? Does that make sense? I could try to come up with an API
> >for
> >that. ;)
> >
> 
> This sounds really good, but I suspect the real problem is the case where
> the rule evaluation is in the hardware path right. If it is purely IF based
> there is no issue .. but any other policy like missed in table 1, then use
> table 2 will not work with this model .. or did I miss something ?

I could come up with several ways how to model hardware. Depending on
that the integration with rules is easy or nearly impossible:

1) it simply cannot deal with ip rules, so there is no way an ACL can
influence the outcome of a routing table lookup - if the feature should
be used, it has to use the slow-path in the kernel.

2) ACLs can influence which routing table will get queried - this sounds
very much like the ip rule model and it seems not too hard to model
that.

3) Routing implementations in the hardware have a single routing table
and the leafs carry different actions with priorities: making this kind
of model working with the ip rule concept will become very difficult and
it might require lots of algorithmic code by every driver to adapt to a
single API provided by Linux. It might be possible, if the hardware
provides actions like backtrack and retrack and can keep state of
priorities during walking the tree, I really doubt that.

Implementations of type 3) would look naturally to do in hardware (see
different Cisco policy routing configurations or ipv6 subtree feature),
so it seems it won't be possible to find a simple way to fuse rules and
offloading in case of point 3).

Rocker sounds a lot like model 2) and this seems possible and should be
a matter of API design. It should merely be a matter of nicely model the
data structures. ;)

Also, @Scott: if you build drivers with l3 offloading as modules, don't
you need to push the full routing tables to the hw once? Maybe we should
think about the drivers pulling routing information from the kernel, the
kernel only notifying something changed?

Bye,
Hannes

^ permalink raw reply

* [PATCH iproute2 -next v2] ip: route: add congestion control metric
From: Daniel Borkmann @ 2015-01-07 11:28 UTC (permalink / raw)
  To: stephen; +Cc: fw, netdev

This patch adds configuration and dumping of congestion control metric
for ip route, for example:

  ip route add <dst> dev foo congctl [lock] dctcp

Reference: http://thread.gmane.org/gmane.linux.network/344733
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 v1->v2:
  - Adapted mxlock setting to kernel style
  - Use arg directly in rta_addattr_l

 include/linux/rtnetlink.h |  2 ++
 ip/iproute.c              | 20 +++++++++++++++++---
 man/man8/ip-route.8.in    | 19 ++++++++++++++++++-
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 9aa5c2f..ac4af97 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -389,6 +389,8 @@ enum {
 #define RTAX_INITRWND RTAX_INITRWND
 	RTAX_QUICKACK,
 #define RTAX_QUICKACK RTAX_QUICKACK
+	RTAX_CC_ALGO,
+#define RTAX_CC_ALGO RTAX_CC_ALGO
 	__RTAX_MAX
 };
 
diff --git a/ip/iproute.c b/ip/iproute.c
index 5a496a9..732029e 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -53,6 +53,7 @@ static const char *mx_names[RTAX_MAX+1] = {
 	[RTAX_RTO_MIN]	= "rto_min",
 	[RTAX_INITRWND]	= "initrwnd",
 	[RTAX_QUICKACK]	= "quickack",
+	[RTAX_CC_ALGO]	= "congctl",
 };
 static void usage(void) __attribute__((noreturn));
 
@@ -80,8 +81,7 @@ static void usage(void)
 	fprintf(stderr, "           [ window NUMBER] [ cwnd NUMBER ] [ initcwnd NUMBER ]\n");
 	fprintf(stderr, "           [ ssthresh NUMBER ] [ realms REALM ] [ src ADDRESS ]\n");
 	fprintf(stderr, "           [ rto_min TIME ] [ hoplimit NUMBER ] [ initrwnd NUMBER ]\n");
-	fprintf(stderr, "           [ features FEATURES ]\n");
-	fprintf(stderr, "           [ quickack BOOL ]\n");
+	fprintf(stderr, "           [ features FEATURES ] [ quickack BOOL ] [ congctl NAME ]\n");
 	fprintf(stderr, "TYPE := [ unicast | local | broadcast | multicast | throw |\n");
 	fprintf(stderr, "          unreachable | prohibit | blackhole | nat ]\n");
 	fprintf(stderr, "TABLE_ID := [ local | main | default | all | NUMBER ]\n");
@@ -545,10 +545,12 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 				fprintf(fp, " %s", mx_names[i]);
 			else
 				fprintf(fp, " metric %d", i);
+
 			if (mxlock & (1<<i))
 				fprintf(fp, " lock");
+			if (i != RTAX_CC_ALGO)
+				val = *(unsigned *)RTA_DATA(mxrta[i]);
 
-			val = *(unsigned*)RTA_DATA(mxrta[i]);
 			switch (i) {
 			case RTAX_FEATURES:
 				print_rtax_features(fp, val);
@@ -573,6 +575,10 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 					fprintf(fp, " %gs", val/1e3);
 				else
 					fprintf(fp, " %ums", val);
+				break;
+			case RTAX_CC_ALGO:
+				fprintf(fp, " %s", (char *)RTA_DATA(mxrta[i]));
+				break;
 			}
 		}
 	}
@@ -925,6 +931,14 @@ static int iproute_modify(int cmd, unsigned flags, int argc, char **argv)
 			if (quickack != 1 && quickack != 0)
 				invarg("\"quickack\" value should be 0 or 1\n", *argv);
 			rta_addattr32(mxrta, sizeof(mxbuf), RTAX_QUICKACK, quickack);
+		} else if (matches(*argv, "congctl") == 0) {
+			NEXT_ARG();
+			if (strcmp(*argv, "lock") == 0) {
+				mxlock |= 1 << RTAX_CC_ALGO;
+				NEXT_ARG();
+			}
+			rta_addattr_l(mxrta, sizeof(mxbuf), RTAX_CC_ALGO, *argv,
+				      strlen(*argv));
 		} else if (matches(*argv, "rttvar") == 0) {
 			unsigned win;
 			NEXT_ARG();
diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
index 89960c1..2b1583d 100644
--- a/man/man8/ip-route.8.in
+++ b/man/man8/ip-route.8.in
@@ -116,7 +116,9 @@ replace " } "
 .B  features
 .IR FEATURES " ] [ "
 .B  quickack
-.IR BOOL " ]"
+.IR BOOL " ] [ "
+.B  congctl
+.IR NAME " ]"
 
 .ti -8
 .IR TYPE " := [ "
@@ -433,6 +435,21 @@ sysctl is set to 0.
 Enable or disable quick ack for connections to this destination.
 
 .TP
+.BI congctl " NAME " "(3.20+ only)"
+.TP
+.BI "congctl lock" " NAME " "(3.20+ only)"
+Sets a specific TCP congestion control algorithm only for a given destination.
+If not specified, Linux keeps the current global default TCP congestion control
+algorithm, or the one set from the application. If the modifier
+.B lock
+is not used, an application may nevertheless overwrite the suggested congestion
+control algorithm for that destination. If the modifier
+.B lock
+is used, then an application is not allowed to overwrite the specified congestion
+control algorithm for that destination, thus it will be enforced/guaranteed to
+use the proposed algorithm.
+
+.TP
 .BI advmss " NUMBER " "(2.3.15+ only)"
 the MSS ('Maximal Segment Size') to advertise to these
 destinations when establishing TCP connections.  If it is not given,
-- 
1.9.0

^ permalink raw reply related

* Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function
From: David Miller @ 2015-01-07 12:02 UTC (permalink / raw)
  To: shawn.guo; +Cc: fugang.duan, netdev, bhutchings, stephen
In-Reply-To: <20150107031658.GB4928@dragon>

From: Shawn Guo <shawn.guo@linaro.org>
Date: Wed, 7 Jan 2015 11:17:00 +0800

> On Wed, Jan 07, 2015 at 02:40:33AM +0000, fugang.duan@freescale.com wrote:
>> Yes, the platform code will become quite messy and tremendous.
>> I agree with your thinking.
>> 
>> But David applied the patches, how can I do it now ?
> 
> David,
> 
> Can you please drop the last two arch/arm/ patches, or revert them if
> your tree is non-rebasable?

My tree is non-rebasable, someone will need to send me a revert patch
which has a commit message explains why the revert is happening.

^ permalink raw reply

* Re: [PATCH] Fix NUL (\0 or \x00) specification in string
From: David Sterba @ 2015-01-07 12:22 UTC (permalink / raw)
  To: Giel van Schijndel
  Cc: linux-kernel, Armin Schindler, Karsten Keil,
	open list:ISDN SUBSYSTEM
In-Reply-To: <20150106192804.GL4806@salidar.dom.custoft.eu>

On Tue, Jan 06, 2015 at 08:28:04PM +0100, Giel van Schijndel wrote:
> On Mon, Jan 05, 2015 at 16:00:26 +0100, David Sterba wrote:
> > I'm replying to all your recent patches here as they are fixing things
> > reported in http://www.viva64.com/en/b/0299/ . I'ts a good practice to
> > give the credit the reporter.
> > 
> > The blogpost also contains analyses of the issues so it could help
> > reviewing the patches.
> 
> I guess you suggest I use a 'Reported-(at|by)' line?

A link in the changelog would be enough IMHO.

> Would something like the below suffice? I found similar log entries in the
> commit log, e.g. bf3204cb, except that those add an e-mail address for
> the reporters, which I don't think is necessary in this case.

If you don't have the emails, then rather do not use reported-by.

^ permalink raw reply

* Re: ipv6: oops in datagram.c line 260
From: Steffen Klassert @ 2015-01-07 12:26 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: Chris Ruehl, netdev, davem
In-Reply-To: <1420627502.26870.38.camel@redhat.com>

On Wed, Jan 07, 2015 at 11:45:02AM +0100, Hannes Frederic Sowa wrote:
> On Mi, 2015-01-07 at 08:22 +0100, Steffen Klassert wrote:
> > On Tue, Jan 06, 2015 at 05:01:13PM +0100, Hannes Frederic Sowa wrote:
> > > 
> > > xfrm6_output_finish unconditionally resets skb->protocol so we try to
> > > dispatch to the IPv6 handler, even though tcp just sends an IPv4 packet.
> > 
> > Maybe we better dispatch based on sk->sk_family, this should give
> > always the right address family of the socket.
> 
> The original problem was dealing with IPv4/v6 mapped traffic. Processing
> local errors from unconnected UDP sockets which are emitting both IPv4
> and IPv6 frames won't play nicely with sk->sk_family I fear.

Good point, unfortunately it is not so easy to fix as I thought.

^ permalink raw reply

* Re: [PATCH v4] can: Convert to runtime_pm
From: Marc Kleine-Budde @ 2015-01-07 12:28 UTC (permalink / raw)
  To: Kedareswara rao Appana, wg, michal.simek, grant.likely
  Cc: linux-can, netdev, linux-kernel, Kedareswara rao Appana,
	Soren Brinkmann
In-Reply-To: <58f37b6fd9104ce185c413c473fe047b@BY2FFO11FD050.protection.gbl>

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

On 12/23/2014 01:25 PM, Kedareswara rao Appana wrote:
> Instead of enabling/disabling clocks at several locations in the driver,
> use the runtime_pm framework. This consolidates the actions for
> runtime PM in the appropriate callbacks and makes the driver more
> readable and mantainable.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> Chnages for v4:
>  - Updated with the review comments.
> Changes for v3:
>   - Converted the driver to use runtime_pm.
> Changes for v2:
>   - Removed the struct platform_device* from suspend/resume
>     as suggest by Lothar.
> 
>  drivers/net/can/xilinx_can.c |  123 +++++++++++++++++++++++++-----------------
>  1 files changed, 74 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index 6c67643..c71f683 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -32,6 +32,7 @@
>  #include <linux/can/dev.h>
>  #include <linux/can/error.h>
>  #include <linux/can/led.h>
> +#include <linux/pm_runtime.h>
>  
>  #define DRIVER_NAME	"xilinx_can"
>  
> @@ -138,7 +139,7 @@ struct xcan_priv {
>  	u32 (*read_reg)(const struct xcan_priv *priv, enum xcan_reg reg);
>  	void (*write_reg)(const struct xcan_priv *priv, enum xcan_reg reg,
>  			u32 val);
> -	struct net_device *dev;
> +	struct device *dev;
>  	void __iomem *reg_base;
>  	unsigned long irq_flags;
>  	struct clk *bus_clk;
> @@ -842,6 +843,13 @@ static int xcan_open(struct net_device *ndev)
>  	struct xcan_priv *priv = netdev_priv(ndev);
>  	int ret;
>  
> +	ret = pm_runtime_get_sync(priv->dev);
> +	if (ret < 0) {
> +		netdev_err(ndev, "%s: pm_runtime_get failed\r(%d)\n\r",
> +				__func__, ret);
> +		return ret;
> +	}
> +
>  	ret = request_irq(ndev->irq, xcan_interrupt, priv->irq_flags,
>  			ndev->name, ndev);
>  	if (ret < 0) {
> @@ -849,29 +857,17 @@ static int xcan_open(struct net_device *ndev)
>  		goto err;
>  	}
>  
> -	ret = clk_prepare_enable(priv->can_clk);
> -	if (ret) {
> -		netdev_err(ndev, "unable to enable device clock\n");
> -		goto err_irq;
> -	}
> -
> -	ret = clk_prepare_enable(priv->bus_clk);
> -	if (ret) {
> -		netdev_err(ndev, "unable to enable bus clock\n");
> -		goto err_can_clk;
> -	}
> -
>  	/* Set chip into reset mode */
>  	ret = set_reset_mode(ndev);
>  	if (ret < 0) {
>  		netdev_err(ndev, "mode resetting failed!\n");
> -		goto err_bus_clk;
> +		goto err_irq;
>  	}
>  
>  	/* Common open */
>  	ret = open_candev(ndev);
>  	if (ret)
> -		goto err_bus_clk;
> +		goto err_irq;
>  
>  	ret = xcan_chip_start(ndev);
>  	if (ret < 0) {
> @@ -887,13 +883,11 @@ static int xcan_open(struct net_device *ndev)
>  
>  err_candev:
>  	close_candev(ndev);
> -err_bus_clk:
> -	clk_disable_unprepare(priv->bus_clk);
> -err_can_clk:
> -	clk_disable_unprepare(priv->can_clk);
>  err_irq:
>  	free_irq(ndev->irq, ndev);
>  err:
> +	pm_runtime_put(priv->dev);
> +
>  	return ret;
>  }
>  
> @@ -910,12 +904,11 @@ static int xcan_close(struct net_device *ndev)
>  	netif_stop_queue(ndev);
>  	napi_disable(&priv->napi);
>  	xcan_chip_stop(ndev);
> -	clk_disable_unprepare(priv->bus_clk);
> -	clk_disable_unprepare(priv->can_clk);
>  	free_irq(ndev->irq, ndev);
>  	close_candev(ndev);
>  
>  	can_led_event(ndev, CAN_LED_EVENT_STOP);
> +	pm_runtime_put(priv->dev);
>  
>  	return 0;
>  }
> @@ -934,27 +927,20 @@ static int xcan_get_berr_counter(const struct net_device *ndev,
>  	struct xcan_priv *priv = netdev_priv(ndev);
>  	int ret;
>  
> -	ret = clk_prepare_enable(priv->can_clk);
> -	if (ret)
> -		goto err;
> -
> -	ret = clk_prepare_enable(priv->bus_clk);
> -	if (ret)
> -		goto err_clk;
> +	ret = pm_runtime_get_sync(priv->dev);
> +	if (ret < 0) {
> +		netdev_err(ndev, "%s: pm_runtime_get failed\r(%d)\n\r",
> +				__func__, ret);

Please remove the \r from the error messages.

> +		return ret;
> +	}
>  
>  	bec->txerr = priv->read_reg(priv, XCAN_ECR_OFFSET) & XCAN_ECR_TEC_MASK;
>  	bec->rxerr = ((priv->read_reg(priv, XCAN_ECR_OFFSET) &
>  			XCAN_ECR_REC_MASK) >> XCAN_ESR_REC_SHIFT);
>  
> -	clk_disable_unprepare(priv->bus_clk);
> -	clk_disable_unprepare(priv->can_clk);
> +	pm_runtime_put(priv->dev);
>  
>  	return 0;
> -
> -err_clk:
> -	clk_disable_unprepare(priv->can_clk);
> -err:
> -	return ret;
>  }
>  
>  
> @@ -967,15 +953,45 @@ static const struct net_device_ops xcan_netdev_ops = {
>  
>  /**
>   * xcan_suspend - Suspend method for the driver
> - * @dev:	Address of the platform_device structure
> + * @dev:	Address of the device structure
>   *
>   * Put the driver into low power mode.
> - * Return: 0 always
> + * Return: 0 on success and failure value on error
>   */
>  static int __maybe_unused xcan_suspend(struct device *dev)
>  {
> -	struct platform_device *pdev = dev_get_drvdata(dev);
> -	struct net_device *ndev = platform_get_drvdata(pdev);
> +	if (!device_may_wakeup(dev))
> +		return pm_runtime_force_suspend(dev);
> +
> +	return 0;
> +}
> +
> +/**
> + * xcan_resume - Resume from suspend
> + * @dev:	Address of the device structure
> + *
> + * Resume operation after suspend.
> + * Return: 0 on success and failure value on error
> + */
> +static int __maybe_unused xcan_resume(struct device *dev)
> +{
> +	if (!device_may_wakeup(dev))
> +		return pm_runtime_force_resume(dev);
> +
> +	return 0;
> +
> +}
> +
> +/**
> + * xcan_runtime_suspend - Runtime suspend method for the driver
> + * @dev:	Address of the device structure
> + *
> + * Put the driver into low power mode.
> + * Return: 0 always
> + */
> +static int __maybe_unused xcan_runtime_suspend(struct device *dev)
> +{
> +	struct net_device *ndev = dev_get_drvdata(dev);
>  	struct xcan_priv *priv = netdev_priv(ndev);
>  
>  	if (netif_running(ndev)) {
> @@ -993,16 +1009,15 @@ static int __maybe_unused xcan_suspend(struct device *dev)
>  }
>  
>  /**
> - * xcan_resume - Resume from suspend
> - * @dev:	Address of the platformdevice structure
> + * xcan_runtime_resume - Runtime resume from suspend
> + * @dev:	Address of the device structure
>   *
>   * Resume operation after suspend.
>   * Return: 0 on success and failure value on error
>   */
> -static int __maybe_unused xcan_resume(struct device *dev)
> +static int __maybe_unused xcan_runtime_resume(struct device *dev)
>  {
> -	struct platform_device *pdev = dev_get_drvdata(dev);
> -	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct net_device *ndev = dev_get_drvdata(dev);
>  	struct xcan_priv *priv = netdev_priv(ndev);
>  	int ret;

Some more context:

> 	ret = clk_enable(priv->bus_clk);
> 	if (ret) {
> 		dev_err(dev, "Cannot enable clock.\n");
> 		return ret;
> 	}
> 	ret = clk_enable(priv->can_clk);
> 	if (ret) {
> 		dev_err(dev, "Cannot enable clock.\n");
> 		clk_disable_unprepare(priv->bus_clk);

This disable_unprepare looks wrong, should be a disable only.

> 		return ret;
> 	}
> 
> 	priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
> 	priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
> 
> 	if (netif_running(ndev)) {
> 		priv->can.state = CAN_STATE_ERROR_ACTIVE;

What happens if the device was not in ACTIVE state prior to the
runtime_suspend?

> 		netif_device_attach(ndev);
> 		netif_start_queue(ndev);
> 	}
> 
> 	return 0;
> }


>  
> @@ -1020,9 +1035,9 @@ static int __maybe_unused xcan_resume(struct device *dev)
>  
>  	priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
>  	priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
> -	priv->can.state = CAN_STATE_ERROR_ACTIVE;
>  
>  	if (netif_running(ndev)) {
> +		priv->can.state = CAN_STATE_ERROR_ACTIVE;
>  		netif_device_attach(ndev);
>  		netif_start_queue(ndev);
>  	}
> @@ -1030,7 +1045,10 @@ static int __maybe_unused xcan_resume(struct device *dev)
>  	return 0;
>  }
>  
> -static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend, xcan_resume);
> +static const struct dev_pm_ops xcan_dev_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(xcan_suspend, xcan_resume)
> +	SET_PM_RUNTIME_PM_OPS(xcan_runtime_suspend, xcan_runtime_resume, NULL)
> +};
>  
>  /**
>   * xcan_probe - Platform registration call
> @@ -1071,7 +1089,7 @@ static int xcan_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	priv = netdev_priv(ndev);
> -	priv->dev = ndev;
> +	priv->dev = &pdev->dev;
>  	priv->can.bittiming_const = &xcan_bittiming_const;
>  	priv->can.do_set_mode = xcan_do_set_mode;
>  	priv->can.do_get_berr_counter = xcan_get_berr_counter;
> @@ -1137,15 +1155,22 @@ static int xcan_probe(struct platform_device *pdev)
>  
>  	netif_napi_add(ndev, &priv->napi, xcan_rx_poll, rx_max);
>  
> +	pm_runtime_set_active(&pdev->dev);
> +	pm_runtime_irq_safe(&pdev->dev);
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_get_sync(&pdev->dev);
Check error values?
> +
>  	ret = register_candev(ndev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "fail to register failed (err=%d)\n", ret);
> +		pm_runtime_put(priv->dev);

Please move the pm_runtime_put into the common error exit path.

>  		goto err_unprepare_disable_busclk;
>  	}
>  
>  	devm_can_led_init(ndev);
> -	clk_disable_unprepare(priv->bus_clk);
> -	clk_disable_unprepare(priv->can_clk);
> +
> +	pm_runtime_put(&pdev->dev);
> +
>  	netdev_dbg(ndev, "reg_base=0x%p irq=%d clock=%d, tx fifo depth:%d\n",
>  			priv->reg_base, ndev->irq, priv->can.clock.freq,
>  			priv->tx_max);
> 

I think you have to convert the _remove() function, too. Have a look at
the gpio-zynq.c driver:

> static int zynq_gpio_remove(struct platform_device *pdev)
> {
> 	struct zynq_gpio *gpio = platform_get_drvdata(pdev);
> 
> 	pm_runtime_get_sync(&pdev->dev);

However I don't understand why the get_sync() is here. Maybe Sören can help?
	
> 	gpiochip_remove(&gpio->chip);
> 	clk_disable_unprepare(gpio->clk);
> 	device_set_wakeup_capable(&pdev->dev, 0);
> 	return 0;
> }

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH net] netlink: fix wrong subscription bitmask to group mapping in binding callbacks
From: Pablo Neira Ayuso @ 2015-01-07 12:31 UTC (permalink / raw)
  To: netdev; +Cc: davem

The subscription bitmask passed via struct sockaddr_nl is converted to
the group number when calling the netlink_bind() and netlink_unbind()
callbacks.

The conversion is however incorrect since bitmask (1 << 0) needs to be
mapped to group number 1. Note that you cannot specify the group number 0
(usually known as _NONE) from setsockopt() using NETLINK_ADD_MEMBERSHIP
since this is rejected through -EINVAL.

This problem became noticeable since 97840cb ("netfilter: nfnetlink:
fix insufficient validation in nfnetlink_bind") when binding to bitmask
(1 << 0) in ctnetlink.

Reported-by: Andre Tomt <andre@tomt.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netlink/af_netlink.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 074cf3e..cbcf73b 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1420,7 +1420,7 @@ static void netlink_unbind(int group, long unsigned int groups,
 
 	for (undo = 0; undo < group; undo++)
 		if (test_bit(undo, &groups))
-			nlk->netlink_unbind(undo);
+			nlk->netlink_unbind(undo + 1);
 }
 
 static int netlink_bind(struct socket *sock, struct sockaddr *addr,
@@ -1458,7 +1458,7 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr,
 		for (group = 0; group < nlk->ngroups; group++) {
 			if (!test_bit(group, &groups))
 				continue;
-			err = nlk->netlink_bind(group);
+			err = nlk->netlink_bind(group + 1);
 			if (!err)
 				continue;
 			netlink_unbind(group, groups, nlk);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH net-next 1/2] Revert "ARM: imx: add FEC sleep mode callback function"
From: Fabio Estevam @ 2015-01-07 12:39 UTC (permalink / raw)
  To: davem; +Cc: shawn.guo, fugang.duan, netdev, Fabio Estevam

From: Fabio Estevam <fabio.estevam@freescale.com>

i.MX platform maintainer Shawn Guo is not happy with the such commit as
explained below [1]:

"The GPR difference between SoCs can be encoded in device tree as well.
It's pointless to repeat the same code pattern for every single
platform, that need to set up GPR bits for enabling magic packet wake
up, while the only difference is the register and bit offset.

The platform code will become quite messy and unmaintainable if every
device driver dump their GPR register setup code into platform.

Sorry, but it's NACK from me."

This reverts commit 456062b3ec6f5b9 ("ARM: imx: add FEC sleep mode callback 
function").

[1] http://www.spinics.net/lists/netdev/msg310922.html

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/mach-imx/mach-imx6q.c  | 41 +--------------------------------
 arch/arm/mach-imx/mach-imx6sx.c | 50 -----------------------------------------
 2 files changed, 1 insertion(+), 90 deletions(-)

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 2f76168..5057d61 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -31,8 +31,6 @@
 #include <linux/micrel_phy.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
-#include <linux/fec.h>
-#include <linux/netdevice.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 #include <asm/system_misc.h>
@@ -41,35 +39,6 @@
 #include "cpuidle.h"
 #include "hardware.h"
 
-static struct fec_platform_data fec_pdata;
-
-static void imx6q_fec_sleep_enable(int enabled)
-{
-	struct regmap *gpr;
-
-	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
-	if (!IS_ERR(gpr)) {
-		if (enabled)
-			regmap_update_bits(gpr, IOMUXC_GPR13,
-					   IMX6Q_GPR13_ENET_STOP_REQ,
-					   IMX6Q_GPR13_ENET_STOP_REQ);
-
-		else
-			regmap_update_bits(gpr, IOMUXC_GPR13,
-					   IMX6Q_GPR13_ENET_STOP_REQ, 0);
-	} else
-		pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");
-}
-
-static void __init imx6q_enet_plt_init(void)
-{
-	struct device_node *np;
-
-	np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@02188000");
-	if (np && of_get_property(np, "fsl,magic-packet", NULL))
-		fec_pdata.sleep_mode_enable = imx6q_fec_sleep_enable;
-}
-
 /* For imx6q sabrelite board: set KSZ9021RN RGMII pad skew */
 static int ksz9021rn_phy_fixup(struct phy_device *phydev)
 {
@@ -292,12 +261,6 @@ static void __init imx6q_axi_init(void)
 	}
 }
 
-/* Add auxdata to pass platform data */
-static const struct of_dev_auxdata imx6q_auxdata_lookup[] __initconst = {
-	OF_DEV_AUXDATA("fsl,imx6q-fec", 0x02188000, NULL, &fec_pdata),
-	{ /* sentinel */ }
-};
-
 static void __init imx6q_init_machine(void)
 {
 	struct device *parent;
@@ -311,13 +274,11 @@ static void __init imx6q_init_machine(void)
 
 	imx6q_enet_phy_init();
 
-	of_platform_populate(NULL, of_default_bus_match_table,
-			     imx6q_auxdata_lookup, parent);
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, parent);
 
 	imx_anatop_init();
 	cpu_is_imx6q() ?  imx6q_pm_init() : imx6dl_pm_init();
 	imx6q_1588_init();
-	imx6q_enet_plt_init();
 	imx6q_axi_init();
 }
 
diff --git a/arch/arm/mach-imx/mach-imx6sx.c b/arch/arm/mach-imx/mach-imx6sx.c
index 68fad21..66988eb 100644
--- a/arch/arm/mach-imx/mach-imx6sx.c
+++ b/arch/arm/mach-imx/mach-imx6sx.c
@@ -12,62 +12,12 @@
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
-#include <linux/fec.h>
-#include <linux/netdevice.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 
 #include "common.h"
 #include "cpuidle.h"
 
-static struct fec_platform_data fec_pdata[2];
-
-static void imx6sx_fec1_sleep_enable(int enabled)
-{
-	struct regmap *gpr;
-
-	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
-	if (!IS_ERR(gpr)) {
-		if (enabled)
-			regmap_update_bits(gpr, IOMUXC_GPR4,
-					   IMX6SX_GPR4_FEC_ENET1_STOP_REQ,
-					   IMX6SX_GPR4_FEC_ENET1_STOP_REQ);
-		else
-			regmap_update_bits(gpr, IOMUXC_GPR4,
-					   IMX6SX_GPR4_FEC_ENET1_STOP_REQ, 0);
-	} else
-		pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
-}
-
-static void imx6sx_fec2_sleep_enable(int enabled)
-{
-	struct regmap *gpr;
-
-	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
-	if (!IS_ERR(gpr)) {
-		if (enabled)
-			regmap_update_bits(gpr, IOMUXC_GPR4,
-					   IMX6SX_GPR4_FEC_ENET2_STOP_REQ,
-					   IMX6SX_GPR4_FEC_ENET2_STOP_REQ);
-		else
-			regmap_update_bits(gpr, IOMUXC_GPR4,
-					   IMX6SX_GPR4_FEC_ENET2_STOP_REQ, 0);
-	} else
-		pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
-}
-
-static void __init imx6sx_enet_plt_init(void)
-{
-	struct device_node *np;
-
-	np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@02188000");
-	if (np && of_get_property(np, "fsl,magic-packet", NULL))
-		fec_pdata[0].sleep_mode_enable = imx6sx_fec1_sleep_enable;
-	np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@021b4000");
-	if (np && of_get_property(np, "fsl,magic-packet", NULL))
-		fec_pdata[1].sleep_mode_enable = imx6sx_fec2_sleep_enable;
-}
-
 static int ar8031_phy_fixup(struct phy_device *dev)
 {
 	u16 val;
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 2/2] Revert "ARM: dts: imx6qdl: enable FEC magic-packet feature"
From: Fabio Estevam @ 2015-01-07 12:39 UTC (permalink / raw)
  To: davem; +Cc: shawn.guo, fugang.duan, netdev, Fabio Estevam
In-Reply-To: <1420634393-30027-1-git-send-email-festevam@gmail.com>

From: Fabio Estevam <fabio.estevam@freescale.com>

As 456062b3ec6f ("ARM: imx: add FEC sleep mode callback function") has been
reverted, also revert the dts part.

This reverts commit 07b4d2dda0c00f56248 ("ARM: dts: imx6qdl: enable FEC 
magic-packet feature").

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 1 -
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 327d362f..009abd6 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -67,7 +67,6 @@
 	phy-mode = "rgmii";
 	interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
 			      <&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
-	fsl,magic-packet;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index 6bfd0bc..f1cd214 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -160,7 +160,6 @@
 	pinctrl-0 = <&pinctrl_enet>;
 	phy-mode = "rgmii";
 	phy-reset-gpios = <&gpio1 25 0>;
-	fsl,magic-packet;
 	status = "okay";
 };
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH] e1000: fix time comparison
From: Asaf Vertz @ 2015-01-07 12:41 UTC (permalink / raw)
  To: jeffrey.t.kirsher
  Cc: jesse.brandeburg, bruce.w.allan, carolyn.wyborny,
	donald.c.skidmore, gregory.v.rose, matthew.vick, john.ronciak,
	mitch.a.williams, linux.nics, e1000-devel, netdev, linux-kernel

To be future-proof and for better readability the time comparisons are
modified to use time_after_eq() instead of plain, error-prone math.

Signed-off-by: Asaf Vertz <asaf.vertz@tandemg.com>
---
 drivers/net/ethernet/intel/e1000/e1000_ethtool.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index b691eb4..4270ad2 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -24,6 +24,7 @@
 /* ethtool support for e1000 */
 
 #include "e1000.h"
+#include <linux/jiffies.h>
 #include <linux/uaccess.h>
 
 enum {NETDEV_STATS, E1000_STATS};
@@ -1460,7 +1461,7 @@ static int e1000_run_loopback_test(struct e1000_adapter *adapter)
 			ret_val = 13; /* ret_val is the same as mis-compare */
 			break;
 		}
-		if (jiffies >= (time + 2)) {
+		if (time_after_eq(jiffies, time + 2)) {
 			ret_val = 14; /* error code for time out error */
 			break;
 		}
-- 
1.7.0.4

^ permalink raw reply related


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