* [PATCH] sh_eth: fix napi_{en|dis}able() calls racing against interrupts
From: Sergei Shtylyov @ 2013-09-03 22:41 UTC (permalink / raw)
To: netdev; +Cc: nobuhiro.iwamatsu.yj, linux-sh
While implementing NAPI for the driver, I overlooked the race conditions where
interrupt handler might have called napi_schedule_prep() before napi_enable()
was called or after napi_disable() was called. If RX interrupt happens, this
would cause the endless interrupts and messages like:
sh-eth eth0: ignoring interrupt, status 0x00040000, mask 0x01ff009f.
The interrupt wouldn't even be masked by the kernel eventually since the handler
would return IRQ_HANDLED all the time.
As a fix, move napi_enable() call before request_irq() call and napi_disable()
call after free_irq() call.
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
The patch is against Dave's 'net.git' repo. It should be applied to 3.11.y too.
drivers/net/ethernet/renesas/sh_eth.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
Index: net/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- net.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net/drivers/net/ethernet/renesas/sh_eth.c
@@ -1857,11 +1857,13 @@ static int sh_eth_open(struct net_device
pm_runtime_get_sync(&mdp->pdev->dev);
+ napi_enable(&mdp->napi);
+
ret = request_irq(ndev->irq, sh_eth_interrupt,
mdp->cd->irq_flags, ndev->name, ndev);
if (ret) {
dev_err(&ndev->dev, "Can not assign IRQ number\n");
- return ret;
+ goto out_napi_off;
}
/* Descriptor set */
@@ -1879,12 +1881,12 @@ static int sh_eth_open(struct net_device
if (ret)
goto out_free_irq;
- napi_enable(&mdp->napi);
-
return ret;
out_free_irq:
free_irq(ndev->irq, ndev);
+out_napi_off:
+ napi_disable(&mdp->napi);
pm_runtime_put_sync(&mdp->pdev->dev);
return ret;
}
@@ -1976,8 +1978,6 @@ static int sh_eth_close(struct net_devic
{
struct sh_eth_private *mdp = netdev_priv(ndev);
- napi_disable(&mdp->napi);
-
netif_stop_queue(ndev);
/* Disable interrupts by clearing the interrupt mask. */
@@ -1995,6 +1995,8 @@ static int sh_eth_close(struct net_devic
free_irq(ndev->irq, ndev);
+ napi_disable(&mdp->napi);
+
/* Free all the skbuffs in the Rx queue. */
sh_eth_ring_free(ndev);
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Hannes Frederic Sowa @ 2013-09-03 22:43 UTC (permalink / raw)
To: Thomas Graf; +Cc: Stephen Warren, Eric Dumazet, davem, netdev, Fabio Estevam
In-Reply-To: <20130903182348.GB4166@casper.infradead.org>
On Tue, Sep 03, 2013 at 07:23:48PM +0100, Thomas Graf wrote:
> I'm confused why lockdep would only trigger after my patch, the
> deadlock is unrelated.
Either we had patch caf92bc ("ipv6: do not call ndisc_send_rs() with write
lock") already applied or both our testing was flawed. I reverted caf92bc and
tested v1 again and could also reproduce this problem.
Because of the sk_mc_loop logic I do think it is better to set skb->sk, so:
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
If this should still go to stable, the above mentioned commit should also hit
the queue.
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH net-next v2 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
From: Hannes Frederic Sowa @ 2013-09-03 23:01 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378246784-21067-5-git-send-email-dborkman@redhat.com>
On Wed, Sep 04, 2013 at 12:19:40AM +0200, Daniel Borkmann wrote:
> RFC3810, 10. Security Considerations says under subsection 10.1.
> Query Message:
>
> A forged Version 1 Query message will put MLDv2 listeners on that
> link in MLDv1 Host Compatibility Mode. This scenario can be avoided
> by providing MLDv2 hosts with a configuration option to ignore
> Version 1 messages completely.
>
> Hence, implement a MLDv2-only mode that will ignore MLDv1 traffic:
>
> echo 2 > /proc/sys/net/ipv6/conf/ethX/force_mld_version or
> echo 2 > /proc/sys/net/ipv6/conf/all/force_mld_version
>
> Note that <all> device has a higher precedence as it was previously
> also the case in the macro MLD_V1_SEEN() that would "short-circuit"
> if condition on <all> case.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Even better...
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Thanks!
^ permalink raw reply
* bride: IPv6 multicast snooping enhancements
From: Linus Lüssing @ 2013-09-04 0:13 UTC (permalink / raw)
To: netdev
Cc: Herbert Xu, bridge, linux-kernel, Adam Baker, Stephen Hemminger,
Linus Lüssing, David S. Miller, Cong Wang
Hi,
Here are two, small feature changes I would like to submit to increase
the usefulness of the multicast snooping of the bridge code.
The first patch is an unaltered one I had submitted before, but since it
got no feedback I'm resubmitting it here for net-next. With the recently
added patch to disable snooping if there is no querier (b00589af + 248ba8ec05
+ 8d50af4fb), it should be a safe choice now (without these, patch 1/2 would
have introduced another potential for lost IPv6 multicast packets).
Both conceptually and also with some testing and fuzzing, I couldn't spot
any more causes for potential packet loss. And since the multicast snooping
code has now been tried by various people, I think it should be a safe
choice to apply the multicast snooping not only for IPv6 multicast packets
with a scope greater than link-local, but also for packets of exactly this
scope. The IPv6 standard mandates MLD reports for link-local multicast, too,
so we can safely snoop them as well (in contrast to IPv4 link-local).
Cheers, Linus
^ permalink raw reply
* [PATCH net-next 1/2] bridge: prevent flooding IPv6 packets that do not have a listener
From: Linus Lüssing @ 2013-09-04 0:13 UTC (permalink / raw)
To: netdev
Cc: bridge, Stephen Hemminger, David S. Miller, linux-kernel,
Herbert Xu, Cong Wang, Adam Baker, Linus Lüssing
In-Reply-To: <1378253619-23918-1-git-send-email-linus.luessing@web.de>
Currently if there is no listener for a certain group then IPv6 packets
for that group are flooded on all ports, even though there might be no
host and router interested in it on a port.
With this commit they are only forwarded to ports with a multicast
router.
Just like commit bd4265fe36 ("bridge: Only flood unregistered groups
to routers") did for IPv4, let's do the same for IPv6 with the same
reasoning.
Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
net/bridge/br_multicast.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index bbcb435..662ba7b 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1547,8 +1547,14 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
* - MLD has always Router Alert hop-by-hop option
* - But we do not support jumbrograms.
*/
- if (ip6h->version != 6 ||
- ip6h->nexthdr != IPPROTO_HOPOPTS ||
+ if (ip6h->version != 6)
+ return 0;
+
+ /* Prevent flooding this packet if there is no listener present */
+ if (ipv6_is_transient_multicast(&ip6h->daddr))
+ BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
+
+ if (ip6h->nexthdr != IPPROTO_HOPOPTS ||
ip6h->payload_len == 0)
return 0;
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next 2/2] bridge: apply multicast snooping to IPv6 link-local, too
From: Linus Lüssing @ 2013-09-04 0:13 UTC (permalink / raw)
To: netdev
Cc: bridge, Stephen Hemminger, David S. Miller, linux-kernel,
Herbert Xu, Cong Wang, Adam Baker, Linus Lüssing
In-Reply-To: <1378253619-23918-1-git-send-email-linus.luessing@web.de>
The multicast snooping code should have matured enough to be safely
applicable to IPv6 link-local multicast addresses (excluding the
link-local all nodes address, ff02::1), too.
Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
net/bridge/br_mdb.c | 3 ++-
net/bridge/br_multicast.c | 7 ++++---
net/bridge/br_private.h | 10 ----------
3 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index a7c6cd0..85a09bb 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -9,6 +9,7 @@
#include <net/netlink.h>
#if IS_ENABLED(CONFIG_IPV6)
#include <net/ipv6.h>
+#include <net/addrconf.h>
#endif
#include "br_private.h"
@@ -254,7 +255,7 @@ static bool is_valid_mdb_entry(struct br_mdb_entry *entry)
return false;
#if IS_ENABLED(CONFIG_IPV6)
} else if (entry->addr.proto == htons(ETH_P_IPV6)) {
- if (!ipv6_is_transient_multicast(&entry->addr.u.ip6))
+ if (ipv6_addr_is_ll_all_nodes(&entry->addr.u.ip6))
return false;
#endif
} else
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 662ba7b..3b0ed99 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -29,6 +29,7 @@
#include <net/ipv6.h>
#include <net/mld.h>
#include <net/ip6_checksum.h>
+#include <net/addrconf.h>
#endif
#include "br_private.h"
@@ -724,7 +725,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
{
struct br_ip br_group;
- if (!ipv6_is_transient_multicast(group))
+ if (ipv6_addr_is_ll_all_nodes(group))
return 0;
br_group.u.ip6 = *group;
@@ -1410,7 +1411,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
&br->ip6_query;
- if (!ipv6_is_transient_multicast(group))
+ if (ipv6_addr_is_ll_all_nodes(group))
return;
br_group.u.ip6 = *group;
@@ -1551,7 +1552,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
return 0;
/* Prevent flooding this packet if there is no listener present */
- if (ipv6_is_transient_multicast(&ip6h->daddr))
+ if (!ipv6_addr_is_ll_all_nodes(&ip6h->daddr))
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
if (ip6h->nexthdr != IPPROTO_HOPOPTS ||
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f225fb6..598cb0b 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -494,16 +494,6 @@ extern void br_mdb_notify(struct net_device *dev, struct net_bridge_port *port,
#define mlock_dereference(X, br) \
rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock))
-#if IS_ENABLED(CONFIG_IPV6)
-#include <net/addrconf.h>
-static inline int ipv6_is_transient_multicast(const struct in6_addr *addr)
-{
- if (ipv6_addr_is_multicast(addr) && IPV6_ADDR_MC_FLAG_TRANSIENT(addr))
- return 1;
- return 0;
-}
-#endif
-
static inline bool br_multicast_is_router(struct net_bridge *br)
{
return br->multicast_router == 2 ||
--
1.7.10.4
^ permalink raw reply related
* Re: ipv4: warnings on sk_wmem_queued
From: Eric Wong @ 2013-09-04 0:21 UTC (permalink / raw)
To: dormando; +Cc: netdev, linux-kernel
In-Reply-To: <alpine.DEB.2.02.1308311229280.3532@dtop>
dormando <dormando@rydia.net> wrote:
> > I noticed these warnings on stock 3.10.9 running stress tests on
> > cmogstored.git (git://bogomips.org/cmogstored.git) doing standard
> > HTTP server stuff between lo and tmpfs:
> >
> [...]
> > I was going to reboot into 3.10.10 before I looked at dmesg. These
> > warnings happened after ~8 hours of stress tests, and those stress tests
> > are still running.
>
> I had a kernel panic this morning on a production machine, also running
> 3.10.9. I only got a small part of the end of the trace, but it matches:
I haven't managed to hit any kernel panic nor my original warning again
after all these days of constant 20+ GB/s lo traffic. I'll finally
reboot into 3.10.10 now. dmesg has been completely quiet since the warning
and the machine is still running well.
> (also: hi Eric!)
Hi dormando!
^ permalink raw reply
* Re: [PATCH net] net: ipv6: tcp: fix potential use after free in tcp_v6_do_rcv
From: Jean Sacren @ 2013-09-04 0:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Daniel Borkmann, davem, netdev, yoshfuji, Jiri Benc
In-Reply-To: <1378247352.7360.50.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 03 Sep 2013 15:29:12 -0700
>
> On Tue, 2013-09-03 at 15:59 -0600, Jean Sacren wrote:
> > From: Eric Dumazet <eric.dumazet@gmail.com>
> > Date: Tue, 03 Sep 2013 13:35:17 -0700
> > >
> > > How did you get your conclusion ?
> >
> > If one changes one line of code, doesn't one own that line?
> >
>
> 'Owning' like he is the guy who can sell it ? OK I understand now
> so many people send 'cleanups' ;)
'Owning' like he is the guy who is responsible for the change. Whether
you understand it or not, that's your own business.
The idea was by taking one example to determine who is really to blame
for: Is the one passing on the bug responsible? Thanks to Daniel
Borkmann for the answer.
--
Jean Sacren
^ permalink raw reply
* RE: [net-next v3 1/8] i40e: main driver core
From: Nelson, Shannon @ 2013-09-04 0:59 UTC (permalink / raw)
To: Joe Perches, Kirsher, Jeffrey T
Cc: davem@davemloft.net, Brandeburg, Jesse, netdev@vger.kernel.org,
gospo@redhat.com, sassmann@redhat.com, Waskiewicz Jr, Peter P,
e1000-devel@lists.sourceforge.net
In-Reply-To: <1377880368.2070.26.camel@joe-AO722>
> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Friday, August 30, 2013 9:33 AM
>
Hi Joe, thanks for your comments, and we're working through another version to be posted Real Soon. In the meantime I thought I'd respond to a couple specific comments.
> > +#define DRV_KERN "-k"
> > +
> > +#define DRV_VERSION_MAJOR 0
> > +#define DRV_VERSION_MINOR 3
> > +#define DRV_VERSION_BUILD 8
> > +#define DRV_VERSION __stringify(DRV_VERSION_MAJOR) "." \
> > + __stringify(DRV_VERSION_MINOR) "." \
> > + __stringify(DRV_VERSION_BUILD) DRV_KERN
>
> does "-k" add/signify anything useful?
The "-k" helps us to quickly spot that a customer is asking us about the upstream version of our driver and not an out-of-tree (e.g. SourceForge) version.
[...]
>
> > +/**
> > + * i40e_get_netdev_stats_struct - Get statistics for netdev interface
> > + * @netdev: network interface device structure
> > + *
> > + * Returns the address of the device statistics structure.
> > + * The statistics are actually updated from the service task.
> > + **/
> > +static struct rtnl_link_stats64 *i40e_get_netdev_stats_struct(
> > + struct net_device *netdev,
> > + struct rtnl_link_stats64 *storage)
>
> An alternative bsd declaration style might be
> more readable for these very long types and names.
>
> static struct rtnl_link_stats64 *
> i40e_get_netdev_stats_struct(struct net_device *netdev,
> struct rtnl_link_stats64 *storage)
We're trying to stay consistent in kernel function styles, stay within standard checkpatch restrictions, and keep the code readable and maintainable, which all together make for some difficult choices. No matter which way we go on some of these decisions, we run into the painfully gray opinion areas of how someone else might have solved the problems.
sln
^ permalink raw reply
* RE: [E1000-devel] [PATCH 0/4] i40e: Neatening and object size reductions
From: Nelson, Shannon @ 2013-09-04 1:00 UTC (permalink / raw)
To: Joe Perches, Kirsher, Jeffrey T
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <cover.1377903831.git.joe@perches.com>
> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Friday, August 30, 2013 4:06 PM
>
> Just some potential cleanings...
> i40e: Whitespace cleaning
Hmmm, we hadn't noticed the new experimental "--fix" option before. There are a lot of good suggestions there, but obviously it needs a lot of reading and tweaking before it can be used. There are cases here where function call parameters are adjusted to line up with the opening '(' but that pushes the parameter(s) beyond 80 columns - we're trying to stay within the 80 column line and checkpatch clean. Also, there are several where the first continued parameter line indent is changed but the next line or two are not.
We'll spend time going through these and try to take care of what makes sense.
> i40e: Add and use pf_<level>
We had considered this kind of macro awhile ago, but nixed it for a few different reasons, but primarily because it seems like yet-another-print-macro and not necessarily worth the effort.
> i40e: pf_<level> remove "%s: " ... __func__
We're beginning to remove many of the __func__ uses, so these prints are no longer all doing the __func__ thing. We originally had them there for early development and debugging and are currently removing them from the normal path messages.
> i40e: Convert pf_<level> macros to functions
Doesn't this create a problem with polluting the kernel namespace? These don't apply to any other driver. I suppose we could lessen the namespace problem with i40e_ prefix, but I'm still not sold on it. I suspect we can still get much of the text savings replacing the __func__ with __builtin_return_address(0) where needed, and remove them where no longer needed. Does that work for you?
> i40e: Fix 32 bit shift compilation warnings
Sure.
sln
^ permalink raw reply
* Re: [E1000-devel] [PATCH 0/4] i40e: Neatening and object size reductions
From: Joe Perches @ 2013-09-04 1:30 UTC (permalink / raw)
To: Nelson, Shannon
Cc: Kirsher, Jeffrey T, e1000-devel@lists.sourceforge.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <FC41C24E35F18A40888AACA1A36F3E416C61C0E5@FMSMSX102.amr.corp.intel.com>
On Wed, 2013-09-04 at 01:00 +0000, Nelson, Shannon wrote:
Hi Shannon.
> > -----Original Message----- > From: Joe Perches
> [mailto:joe@perches.com] > Sent: Friday, August 30, 2013 4:06 PM > >
> Just some potential cleanings...
>
> > i40e: Whitespace cleaning
>
> Hmmm, we hadn't noticed the new experimental "--fix" option before.
> There are a lot of good suggestions there, but obviously it needs a lot
> of reading and tweaking before it can be used. There are cases here
> where function call parameters are adjusted to line up with the opening
> '(' but that pushes the parameter(s) beyond 80 columns - we're trying
> to stay within the 80 column line and checkpatch clean. Also, there
> are several where the first continued parameter line indent is changed
> but the next line or two are not.
>
> We'll spend time going through these and try to take care of what makes
> sense.
Swell. All these are your choice to fix as you want.
Exceeding 80 columns doesn't bother me much.
Keeping alignment appropriate for multi-line statements
needs work inside checkpatch. I played with it a bit
but it's unfortunately complicated by intermixed
insertions and deletions.
> > i40e: Add and use pf_<level>
>
> We had considered this kind of macro awhile ago, but nixed it for a few
> different reasons, but primarily because it seems like
> yet-another-print-macro and not necessarily worth the effort.
>
> > i40e: pf_<level> remove "%s: " ... __func__
>
> We're beginning to remove many of the __func__ uses, so these prints
> are no longer all doing the __func__ thing. We originally had them
> there for early development and debugging and are currently removing
> them from the normal path messages.
Fine by me. I think __func__ is nearly always pretty
useless myself.
> > i40e: Convert pf_<level> macros to functions
>
> Doesn't this create a problem with polluting the kernel namespace?
> These don't apply to any other driver. I suppose we could lessen the
> namespace problem with i40e_ prefix, but I'm still not sold on it. I
> suspect we can still get much of the text savings replacing the
> __func__ with __builtin_return_address(0) where needed, and remove them
> where no longer needed. Does that work for you?
I think you could just as soon whatever combinations of the
other standard logging mechanisms without using pf_<level>
wiphy_<level>
netif_<level>
netdev_<level>
dev_<level>
pr_<level>
as appropriate. I did that only because there was ~10K
of what I think of as not too useful function names out
of a defconfig size of 140k.
> > i40e: Fix 32 bit shift compilation warnings
>
> Sure.
I think you should use the kernel.h standard macros
for lower_32_bits and upper_32_bits instead.
cheers, Joe
^ permalink raw reply
* Re: [net-next v3 1/8] i40e: main driver core
From: Joe Perches @ 2013-09-04 1:35 UTC (permalink / raw)
To: Nelson, Shannon
Cc: Kirsher, Jeffrey T, davem@davemloft.net, Brandeburg, Jesse,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com,
Waskiewicz Jr, Peter P, e1000-devel@lists.sourceforge.net
In-Reply-To: <FC41C24E35F18A40888AACA1A36F3E416C61C0CE@FMSMSX102.amr.corp.intel.com>
On Wed, 2013-09-04 at 00:59 +0000, Nelson, Shannon wrote:
> > -----Original Message-----
> > From: Joe Perches [mailto:joe@perches.com]
[]
> Hi Joe,
Hi again.
> > > +/**
> > > + * i40e_get_netdev_stats_struct - Get statistics for netdev interface
> > > + * @netdev: network interface device structure
> > > + *
> > > + * Returns the address of the device statistics structure.
> > > + * The statistics are actually updated from the service task.
> > > + **/
> > > +static struct rtnl_link_stats64 *i40e_get_netdev_stats_struct(
> > > + struct net_device *netdev,
> > > + struct rtnl_link_stats64 *storage)
> >
> > An alternative bsd declaration style might be
> > more readable for these very long types and names.
> >
> > static struct rtnl_link_stats64 *
> > i40e_get_netdev_stats_struct(struct net_device *netdev,
> > struct rtnl_link_stats64 *storage)
>
> We're trying to stay consistent in kernel function styles, stay within
> standard checkpatch restrictions, and keep the code readable and
> maintainable, which all together make for some difficult choices. No
> matter which way we go on some of these decisions, we run into the
> painfully gray opinion areas of how someone else might have solved the
> problems.
No worries. Whatever you choose to do will be OK.
The BSD declaration style is pretty common in the
kernel tree too though.
Another thing would be to mark various arguments const struct
in the prototypes and declarations instead of just struct.
cheers, Joe
^ permalink raw reply
* RE: [E1000-devel] [PATCH 0/4] i40e: Neatening and object size reductions
From: Nelson, Shannon @ 2013-09-04 1:38 UTC (permalink / raw)
To: Joe Perches
Cc: Kirsher, Jeffrey T, e1000-devel@lists.sourceforge.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1378258258.7347.48.camel@joe-AO722>
> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Tuesday, September 03, 2013 6:31 PM
>
> On Wed, 2013-09-04 at 01:00 +0000, Nelson, Shannon wrote:
>
> Hi Shannon.
>
> > > -----Original Message----- > From: Joe Perches
> > [mailto:joe@perches.com] > Sent: Friday, August 30, 2013 4:06 PM > >
> > Just some potential cleanings...
> >
> > > i40e: Whitespace cleaning
> >
> > Hmmm, we hadn't noticed the new experimental "--fix" option before.
> > There are a lot of good suggestions there, but obviously it needs a
> lot
> > of reading and tweaking before it can be used. There are cases here
> > where function call parameters are adjusted to line up with the
> opening
> > '(' but that pushes the parameter(s) beyond 80 columns - we're trying
> > to stay within the 80 column line and checkpatch clean. Also, there
> > are several where the first continued parameter line indent is changed
> > but the next line or two are not.
> >
> > We'll spend time going through these and try to take care of what
> makes
> > sense.
>
> Swell. All these are your choice to fix as you want.
>
> Exceeding 80 columns doesn't bother me much.
We should perhaps become a little more flexible ourselves, but we've finally got a good process going internally, including this as a check. I don't dare disturb the machine now that it is working :-).
> Keeping alignment appropriate for multi-line statements
> needs work inside checkpatch. I played with it a bit
> but it's unfortunately complicated by intermixed
> insertions and deletions.
Yeah, it all gets a little funky after a while.
>
> > > i40e: Add and use pf_<level>
> >
> > We had considered this kind of macro awhile ago, but nixed it for a
> few
> > different reasons, but primarily because it seems like
> > yet-another-print-macro and not necessarily worth the effort.
> >
> > > i40e: pf_<level> remove "%s: " ... __func__
> >
> > We're beginning to remove many of the __func__ uses, so these prints
> > are no longer all doing the __func__ thing. We originally had them
> > there for early development and debugging and are currently removing
> > them from the normal path messages.
>
> Fine by me. I think __func__ is nearly always pretty
> useless myself.
It was useful for a while, but it is time to be pulling it out.
>
> > > i40e: Convert pf_<level> macros to functions
> >
> > Doesn't this create a problem with polluting the kernel namespace?
> > These don't apply to any other driver. I suppose we could lessen the
> > namespace problem with i40e_ prefix, but I'm still not sold on it. I
> > suspect we can still get much of the text savings replacing the
> > __func__ with __builtin_return_address(0) where needed, and remove
> them
> > where no longer needed. Does that work for you?
>
> I think you could just as soon whatever combinations of the
> other standard logging mechanisms without using pf_<level>
>
> wiphy_<level>
> netif_<level>
> netdev_<level>
> dev_<level>
> pr_<level>
>
> as appropriate. I did that only because there was ~10K
> of what I think of as not too useful function names out
> of a defconfig size of 140k.
Yes, and I think removing much of the __func__ or using __builtin_return_address(0) will help.
>
> > > i40e: Fix 32 bit shift compilation warnings
> >
> > Sure.
>
> I think you should use the kernel.h standard macros
> for lower_32_bits and upper_32_bits instead.
Yep.
>
> cheers, Joe
^ permalink raw reply
* Re: [PATCH v2 net-next 0/9] set addr_assign_type when inheriting a dev_addr
From: David Miller @ 2013-09-04 1:41 UTC (permalink / raw)
To: bjorn
Cc: devel, jiri, libertas-dev, j, netdev, linux-wireless, linville,
stephen, forest, gregkh, kaber, dan.carpenter
In-Reply-To: <87mwnuqrrx.fsf@nemi.mork.no>
From: Bjørn Mork <bjorn@mork.no>
Date: Tue, 03 Sep 2013 09:38:10 +0200
> But it doesn't look like it ended up in net-next? Or am I missing
> something (again)?
I forgot to push it out from my workstation before going away for the
holiday weekend, this has now been fixed :-)
^ permalink raw reply
* Re: [PATCH net-next v2] net: neighbour: Remove CONFIG_ARPD
From: David Miller @ 2013-09-04 1:42 UTC (permalink / raw)
To: tim.gardner
Cc: netdev, linux-kernel, kuznet, jmorris, yoshfuji, kaber, ebiederm,
gaofeng, joe, vfalico
In-Reply-To: <1377779927-28500-1-git-send-email-tim.gardner@canonical.com>
From: Tim Gardner <tim.gardner@canonical.com>
Date: Thu, 29 Aug 2013 06:38:47 -0600
> This config option is superfluous in that it only guards a call
> to neigh_app_ns(). Enabling CONFIG_ARPD by default has no
> change in behavior. There will now be call to __neigh_notify()
> for each ARP resolution, which has no impact unless there is a
> user space daemon waiting to receive the notification, i.e.,
> the case for which CONFIG_ARPD was designed anyways.
>
> Suggested-by: Eric W. Biederman <ebiederm@xmission.com>
...
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: sctp: probe: allow more advanced ingress filtering by mark
From: David Miller @ 2013-09-04 1:44 UTC (permalink / raw)
To: nhorman; +Cc: dborkman, netdev, linux-sctp
In-Reply-To: <20130830113630.GB1142@hmsreliant.think-freely.org>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Fri, 30 Aug 2013 07:36:30 -0400
> On Thu, Aug 29, 2013 at 06:54:49PM +0200, Daniel Borkmann wrote:
>> This is a follow-up commit for commit b1dcdc68b1f4 ("net: tcp_probe:
>> allow more advanced ingress filtering by mark") that allows for
>> advanced SCTP probe module filtering based on skb mark (for a more
>> detailed description and advantages using mark, refer to b1dcdc68b1f4).
>> The current option to filter by a given port is still being preserved.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
Applied.
^ permalink raw reply
* Re: pull request: wireless-next 2013-08-29
From: David Miller @ 2013-09-04 1:46 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev
In-Reply-To: <20130829185352.GB2300@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Thu, 29 Aug 2013 14:53:53 -0400
> Please accept this batch of updates intended for the 3.12 stream.
Pulled, thanks John.
^ permalink raw reply
* Re: [PATCH v2 0/8] Drop support for Renesas H8/300 architecture
From: Chen Gang F T @ 2013-09-04 1:53 UTC (permalink / raw)
To: Guenter Roeck
Cc: Al Viro, linux-kernel, linux-ide, netdev, linux-watchdog,
Wim Van Sebroeck, David S. Miller, Yoshinori Sato,
Geert Uytterhoeven, Eric Paris, Greg Kroah-Hartman, Jiang Liu,
David Howells, Thomas Gleixner, Stephen Rothwell, Linus Torvalds,
Andrew Morton, Rusty Russell, Linus Walleij
In-Reply-To: <20130903205922.GA3372@roeck-us.net>
Thank you for your valuable information: it will let kernel waste mails
less, and also can save my time resources.
On 09/04/2013 04:59 AM, Guenter Roeck wrote:
> On Tue, Sep 03, 2013 at 08:39:38PM +0100, Al Viro wrote:
>> On Tue, Sep 03, 2013 at 11:52:17AM +0800, Chen Gang F T wrote:
>>
>>> extreme sample: let 'kernel code style' and 'gcc code style' in one file, that will make the code very ugly.
>>
>> gcc style will make any code very ugly, no matter what (if anything) else is
>> in the same file...
>>
Hmm... for me, I don't check/judge the 'coding style' of different
products, what I focus on is to follow the original product 'coding
style'.
e.g. Windows, gcc, Linux kernel, their 'coding styles' are quite different with each other.
Originally I worked under Windows, I followed Windows coding style.
Now I worked under Linux kernel, I follow Linux kernel coding style.
I plan to make patch for gcc, I will follow gcc coding style.
(hope this month I can, but I am not sure, I have no experience for gcc development).
And excuse me, I will be silent during 2013-09-05 - 2013-09-20 (but can
response mail). During these days, I will focus on gcc issues (wish can
fix one), and also do some company's internal things.
Thanks.
>> [digs out the ports history table]
>> x86: 0.01 [alive]
>> i386: 0.01..2.6.24-rc1 [folded into x86]
>> x86_64: 2.5.5-pre1..2.6.24-rc1 [folded into x86]
>> x86: 2.6.24-rc1 [alive]
>> alpha: 1.1.67 [alive]
>> sparc: 1.1.77 [alive]
>> sparc64: 2.1.19..2.6.28 [folded into sparc]
>> mips: 1.1.82 [alive]
>> mips64: 2.3.48-pre2..2.6.0-test2 [folded into mips]
>> powerpc: 1.3.45 [alive]
>> ppc: 1.3.45..2.6.26 [folded into powerpc]
>> ppc64: 2.5.5..2.6.15-rc1 [folded into powerpc]
>> powerpc: 2.6.15-rc1 [alive]
>> m68k: 1.3.94 [alive]
>> m68knommu: 2.5.46..2.6.38 [folded into m68k]
>> arm: 2.1.80 [alive]
>> arm26: 2.5.71..2.6.23-rc2 [gone]
>> arm64: 3.7-rc1 [alive][might eventually fold]
>> sh: 2.3.16 [alive]
>> sh64: 2.6.8-rc1..2.6.24 [folded into sh, nearly dead there]
>> ia64: 2.3.43-pre1 [alive]
>> s390: 2.3.99pre8 [alive]
>> s390x: 2.5.0..2.5.67 [folded into s390]
>> parisc: 2.4.0-test12 [alive]
>> cris: 2.5.0 [alive]
>> um: 2.5.35 [alive]
>> v850: 2.5.46..2.6.26 [gone]
>> h8300: 2.5.68 [moderately responsive]
>> m32r: 2.6.9-rc3 [alive]
>> frv: 2.6.11-rc1 [alive]
>> xtensa: 2.6.13-rc1 [alive]
>> avr32: 2.6.19-rc1 [alive]
>> blackfin: 2.6.22-rc1 [alive]
>> mn10300: 2.6.25-rc1 [alive]
>> microblaze: 2.6.30-rc2 [alive]
>> score: 2.6.32-rc1 [abandoned][cloned off mips]
>> tile: 2.6.36-rc1 [alive]
>> unicore32: 2.6.39-rc1 [alive][cloned off arm]
>> openrisc: 3.1-rc1 [alive]
>> hexagon: 3.2-rc1 [alive]
>> c6x: 3.3-rc1 [alive]
>> arc: 3.9-rc1 [alive]
>> metag: 3.9-rc1 [alive]
>>
>> Frankly, I would've expected score and lefotvers of sh64 (aka sh5) to be
>> the first against the wall - h8300 was a bit surprising...
>>
>
> Great summary.
>
> There seemed to be a consensus to remove h8300, at least so far and sufficiently
> enough for me to ask Stephen to add the removal branch to linux-next.
> We'll see if that triggers any further responses.
>
> With score, I am not entirely sure. I got one Ack for the removal, but
> on the other side the score maintainers came back and claimed they would
> still support it. We'll see if anything changes in practice. I am still
> not sure if I should ask for the removal branch to be added to linux-next.
> Frankly I thought I might jump the gun here more than with h8300.
>
> Either case, what to ultimately do with those two architectures will be
> up to the community to decide.
>
> Guenter
>
Thanks again.
--
Chen Gang
^ permalink raw reply
* Re: pull-request: can-next 2013-08-29
From: David Miller @ 2013-09-04 1:56 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <5224E1A2.1010302@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Mon, 02 Sep 2013 21:06:10 +0200
> Hello David,
>
> On 08/29/2013 11:13 PM, Marc Kleine-Budde wrote:
>> this is a pull request for net-next. There are two patches from Gerhard
>> Sittig, which improves the clock handling on mpc5121. Oliver Hartkopp
>> provides a patch that adds a per rule limitation of frame hops.
>
> have you forgotten this pull request?
If you actually checked the patchwork queue, you're see that this pull
request was still "under review" and there are people who have been
waiting much longer than you have.
I just pulled this in and will push it out once my build tests complete.
There is no reason for you to ever ask me the above question unless
your pull request doesn't show up in patchwork in an appropriate
state. By default assume I am backlogged and I will get to it
whenever time and my resources allow.
^ permalink raw reply
* Re: [PATCH] r8169: add ethtool eeprom change/dump feature
From: David Miller @ 2013-09-04 1:59 UTC (permalink / raw)
To: lekensteyn; +Cc: netdev, romieu, hayeswang, bhutchings
In-Reply-To: <1377787491-2932-1-git-send-email-lekensteyn@gmail.com>
From: Peter Wu <lekensteyn@gmail.com>
Date: Thu, 29 Aug 2013 16:44:51 +0200
> This adds the ability to read and change EEPROM for 93C46/93C56 serial
> EEPROM. Two-Wire serial interface and SPI are not supported. (Do not
> even try this for SPI or other chips, it may break your hardware.)
Please block the operation on configurations, such as the aforementioned
SPI, where it won't work.
^ permalink raw reply
* Re: [PATCH net-next 0/2] bonding: clean rlb vlan use
From: David Miller @ 2013-09-04 2:02 UTC (permalink / raw)
To: vfalico; +Cc: netdev, fubar, andy
In-Reply-To: <1377812337-19551-1-git-send-email-vfalico@redhat.com>
From: Veaceslav Falico <vfalico@redhat.com>
Date: Thu, 29 Aug 2013 23:38:55 +0200
> Finish the vlan cleanup, only rlb left.
>
> Remove the bond_vlan_used() function, it's useless anyway, and
> rlb_client_info->tag, cause we can use only ->vlan_id.
>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 0/2] sh_eth Updates
From: David Miller @ 2013-09-04 2:02 UTC (permalink / raw)
To: horms+renesas
Cc: netdev, linux-sh, magnus.damm, sergei.shtylyov, kouei.abe.cp
In-Reply-To: <1377834068-6978-1-git-send-email-horms+renesas@verge.net.au>
From: Simon Horman <horms+renesas@verge.net.au>
Date: Fri, 30 Aug 2013 12:41:06 +0900
> please consider the following updates to the sh_eth driver
> by Abe-san. They were developed during his testing of the
> driver on the r8a7790/lager board.
>
> Changes between v1 and v2:
Both applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] netconsole: avoid a crash with multiple sysfs writers
From: David Miller @ 2013-09-04 2:11 UTC (permalink / raw)
To: alonid; +Cc: linux-kernel, netdev, nhorman
In-Reply-To: <1377860386-8494-1-git-send-email-alonid@postram.com>
From: Dan Aloni <alonid@postram.com>
Date: Fri, 30 Aug 2013 13:59:46 +0300
> When my 'ifup eth' script was fired multiple times and ran concurrent on
> my laptop, for some obscure /etc scripting reason, it was revealed
> that the store_enabled() function in netconsole doesn't handle it nicely,
> as recorded by the Oops below (a syslog paste, but not mangled too much
> to prevent from discerning the traceback).
>
> On Linux 3.10.4, this patch seeks to remedy the problem, and it has been
> running stable on my laptop for a few days.
...
> Signed-off-by: Dan Aloni <alonid@postram.com>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks.
^ permalink raw reply
* Re: ICMPv6: treat dest unreachable codes 5 and 6 as EACCES, not EPROTO
From: David Miller @ 2013-09-04 2:12 UTC (permalink / raw)
To: jbohac; +Cc: yoshfuji, netdev
In-Reply-To: <20130830091844.GA2706@midget.suse.cz>
From: Jiri Bohac <jbohac@suse.cz>
Date: Fri, 30 Aug 2013 11:18:45 +0200
> RFC 4443 has defined two additional codes for ICMPv6 type 1 (destination
> unreachable) messages:
> 5 - Source address failed ingress/egress policy
> 6 - Reject route to destination
>
> Now they are treated as protocol error and icmpv6_err_convert() converts them
> to EPROTO.
>
> RFC 4443 says:
> "Codes 5 and 6 are more informative subsets of code 1."
>
> Treat codes 5 and 6 as code 1 (EACCES)
>
> Btw, connect() returning -EPROTO confuses firefox, so that fallback to
> other/IPv4 addresses does not work:
> https://bugzilla.mozilla.org/show_bug.cgi?id=910773
>
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH][net-next] gianfar: Fix reported number of sent bytes to BQL
From: David Miller @ 2013-09-04 2:14 UTC (permalink / raw)
To: claudiu.manoil; +Cc: netdev, eric.dumazet
In-Reply-To: <1377864075-19491-1-git-send-email-claudiu.manoil@freescale.com>
From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Fri, 30 Aug 2013 15:01:15 +0300
> Fix the amount of sent bytes reported to BQL by reporting the
> number of bytes on wire in the xmit routine, and recording that
> value for each skb in order to be correctly confirmed on Tx
> confirmation cleanup.
>
> Reporting skb->len to BQL just before exiting xmit is not correct
> due to possible insertions of TOE block and alignment bytes in the
> skb->data, which are being stripped off by the controller before
> transmission on wire. This led to mismatch of (incorrectly)
> reported bytes to BQL b/w xmit and Tx confirmation, resulting in
> Tx timeout firing, for the h/w tx timestamping acceleration case.
>
> There's no easy way to obtain the number of bytes on wire in the Tx
> confirmation routine, so skb->cb is used to convey that information
> from xmit to Tx confirmation, for now (as proposed by Eric). Revived
> the currently unused GFAR_CB() construct for that purpose.
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
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