* Re: [PATCH 1/3] net: lpc_eth: Replace WARN() trace with simple pr_warn()
From: Roland Stigge @ 2012-06-11 8:36 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, netdev, linux-kernel, kevin.wells, srinivas.bakki,
aletes.xgr, linux-arm-kernel
In-Reply-To: <1339403108.6001.1697.camel@edumazet-glaptop>
Hi Dave and Eric,
thanks for your feedback!
On 06/11/2012 10:25 AM, Eric Dumazet wrote:
> On Mon, 2012-06-11 at 10:03 +0200, Roland Stigge wrote:
>> A WARN() trace indicating a "BUG!" was identified as a "normal" case in the
>> xmit function in case all TX descriptors are occupied already. In this case,
>> NETDEV_TX_BUSY is returned, nothing buggy at all.
>>
>> Signed-off-by: Roland Stigge <stigge@antcom.de>
>> Tested-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
>>
>> ---
>> drivers/net/ethernet/nxp/lpc_eth.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- linux-2.6.orig/drivers/net/ethernet/nxp/lpc_eth.c
>> +++ linux-2.6/drivers/net/ethernet/nxp/lpc_eth.c
>> @@ -1114,7 +1114,7 @@ static int lpc_eth_hard_start_xmit(struc
>> buffers */
>> netif_stop_queue(ndev);
>> spin_unlock_irq(&pldat->lock);
>> - WARN(1, "BUG! TX request when no free TX buffers!\n");
>> + pr_warn("Note: TX request when no free TX buffers.\n");
>> return NETDEV_TX_BUSY;
>> }
>>
>
> Entering this path is a bug, don't hide it...
>
> Please share with us how this bug was identified as a "normal case" ?
I encountered cases where this happened for me on a custom board under
heavy load.
I discussed this with Kevin Wells, the original driver author. We
identified the case of xmit()'s TX request (from .ndo_start_xmit) with
full TX driver buffers as valid when ethernet is busy.
But maybe this is wrong. Can you please give me a hint how the net
subsystem makes sure that this doesn't happen under normal circumstances?
Thanks in advance!
Roland
^ permalink raw reply
* Re: [PATCH 1/3] net: lpc_eth: Replace WARN() trace with simple pr_warn()
From: Eric Dumazet @ 2012-06-11 8:25 UTC (permalink / raw)
To: Roland Stigge
Cc: davem, netdev, linux-kernel, kevin.wells, srinivas.bakki,
aletes.xgr, linux-arm-kernel
In-Reply-To: <1339401793-12258-1-git-send-email-stigge@antcom.de>
On Mon, 2012-06-11 at 10:03 +0200, Roland Stigge wrote:
> A WARN() trace indicating a "BUG!" was identified as a "normal" case in the
> xmit function in case all TX descriptors are occupied already. In this case,
> NETDEV_TX_BUSY is returned, nothing buggy at all.
>
> Signed-off-by: Roland Stigge <stigge@antcom.de>
> Tested-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
>
> ---
> drivers/net/ethernet/nxp/lpc_eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-2.6.orig/drivers/net/ethernet/nxp/lpc_eth.c
> +++ linux-2.6/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -1114,7 +1114,7 @@ static int lpc_eth_hard_start_xmit(struc
> buffers */
> netif_stop_queue(ndev);
> spin_unlock_irq(&pldat->lock);
> - WARN(1, "BUG! TX request when no free TX buffers!\n");
> + pr_warn("Note: TX request when no free TX buffers.\n");
> return NETDEV_TX_BUSY;
> }
>
Entering this path is a bug, don't hide it...
Please share with us how this bug was identified as a "normal case" ?
^ permalink raw reply
* Re: [PATCH 2/3] net: lpc_eth: Increase number of TX descriptors
From: Eric Dumazet @ 2012-06-11 8:21 UTC (permalink / raw)
To: Roland Stigge
Cc: davem, netdev, linux-kernel, kevin.wells, srinivas.bakki,
aletes.xgr, linux-arm-kernel
In-Reply-To: <1339401793-12258-2-git-send-email-stigge@antcom.de>
On Mon, 2012-06-11 at 10:03 +0200, Roland Stigge wrote:
> Since we have enough SRAM, we can increase the number of TX descriptors, so the
> "BUSY" warning about occupied TX descriptors doesn't need to show up as often.
>
> Signed-off-by: Roland Stigge <stigge@antcom.de>
> Tested-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
>
> ---
> drivers/net/ethernet/nxp/lpc_eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-2.6.orig/drivers/net/ethernet/nxp/lpc_eth.c
> +++ linux-2.6/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -56,7 +56,7 @@
>
> #define ENET_MAXF_SIZE 1536
> #define ENET_RX_DESC 48
> -#define ENET_TX_DESC 16
> +#define ENET_TX_DESC 32
>
> #define NAPI_WEIGHT 16
>
Sorry, this is not the right fix.
You only lower probability of the bug.
What is this BUSY warning mentioned in changelog ?
^ permalink raw reply
* Re: [PATCH] ieee802154: verify packet size before trying to allocate it
From: Sasha Levin @ 2012-06-11 8:18 UTC (permalink / raw)
To: David Miller; +Cc: dbaryshkov, slapin, linux-zigbee-devel, netdev, linux-kernel
In-Reply-To: <20120610.200443.971015025499077057.davem@davemloft.net>
On Sun, 2012-06-10 at 20:04 -0700, David Miller wrote:
> From: Sasha Levin <levinsasha928@gmail.com>
> Date: Sun, 10 Jun 2012 13:10:19 +0200
>
> > Currently when sending data over datagram, the send function will attempt to
> > allocate any size passed on from the userspace.
> >
> > We should make sure that this size is checked and limited. The maximum size
> > of an IP packet seemed like the safest limit here.
> >
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
>
> Why not limit to the device MTU? That's exactly what I suggested
> to you.
That's what I ended up doing in the reply to this mail.
^ permalink raw reply
* Re: [PATCH 2/3] net: lpc_eth: Increase number of TX descriptors
From: David Miller @ 2012-06-11 8:11 UTC (permalink / raw)
To: stigge
Cc: eric.dumazet, netdev, linux-kernel, kevin.wells, srinivas.bakki,
aletes.xgr, linux-arm-kernel
In-Reply-To: <1339401793-12258-2-git-send-email-stigge@antcom.de>
From: Roland Stigge <stigge@antcom.de>
Date: Mon, 11 Jun 2012 10:03:12 +0200
> Since we have enough SRAM, we can increase the number of TX descriptors, so the
> "BUSY" warning about occupied TX descriptors doesn't need to show up as often.
>
> Signed-off-by: Roland Stigge <stigge@antcom.de>
> Tested-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
This is way too terse, and as I described in my reply to your first
patch it is not normal for the transmit function to be invoked when
there are no TX descriptors available. That's a bug if it is happening.
^ permalink raw reply
* Re: [PATCH 1/3] net: lpc_eth: Replace WARN() trace with simple pr_warn()
From: David Miller @ 2012-06-11 8:10 UTC (permalink / raw)
To: stigge
Cc: eric.dumazet, netdev, linux-kernel, kevin.wells, srinivas.bakki,
aletes.xgr, linux-arm-kernel
In-Reply-To: <1339401793-12258-1-git-send-email-stigge@antcom.de>
From: Roland Stigge <stigge@antcom.de>
Date: Mon, 11 Jun 2012 10:03:11 +0200
> A WARN() trace indicating a "BUG!" was identified as a "normal" case in the
> xmit function in case all TX descriptors are occupied already. In this case,
> NETDEV_TX_BUSY is returned, nothing buggy at all.
>
> Signed-off-by: Roland Stigge <stigge@antcom.de>
> Tested-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
This is not normal.
Read the comment above this code you are changing. If we are
out of TX descriptors, the queue must be stopped, and therefore
if the queue is stopped this transmit method should not be
invoked.
It is a hard error condition, should never occur, and indicates
a very serious error condition in the driver.
^ permalink raw reply
* Re: Generic user-space routing library -- need collaborator
From: Thomas Graf @ 2012-06-11 8:09 UTC (permalink / raw)
To: Philip Prindeville; +Cc: Netdev
In-Reply-To: <4FCBBD6C.8020904@redfish-solutions.com>
On Sun, Jun 03, 2012 at 01:39:24PM -0600, Philip Prindeville wrote:
> Hi.
>
> I'm working on adding a few more portability classes to Poco (a multi-platform C++ toolkit) and wanted to add a Net::Routing class for examining and manipulating the routing tables.
>
> The C++ would just be convenience wrappers around a core C library that handles the netlink semantics. I've looked at libmnl and it's handy, but I need a higher level of abstraction (for instance, parsing an RTA_NETMASK for IPv6 is anything but well-documented).
You want to look at libnl. It's similiar to libmnl but provides a higher
level of abstraction. It implements routing, netfilter and generic
netlink parsing and message construction.
http://www.infradead.org/~tgr/libnl/
You should be able to easily construct C++ wrappers around the lib.
~Thomas
^ permalink raw reply
* [PATCH 3/3] net: lpc_eth: Driver cleanup
From: Roland Stigge @ 2012-06-11 8:03 UTC (permalink / raw)
To: davem, eric.dumazet, netdev, linux-kernel, kevin.wells,
srinivas.bakki, aletes.xgr, linux-arm-kernel
Cc: Roland Stigge
In-Reply-To: <1339401793-12258-1-git-send-email-stigge@antcom.de>
This patch removes some nowadays superfluous definitions (one unused define and
an obsolete function forward declaration) and corrects a netdev_err() to
netdev_dbg().
Signed-off-by: Roland Stigge <stigge@antcom.de>
Signed-off-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
---
drivers/net/ethernet/nxp/lpc_eth.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
--- linux-2.6.orig/drivers/net/ethernet/nxp/lpc_eth.c
+++ linux-2.6/drivers/net/ethernet/nxp/lpc_eth.c
@@ -52,7 +52,6 @@
#define MODNAME "lpc-eth"
#define DRV_VERSION "1.00"
-#define PHYDEF_ADDR 0x00
#define ENET_MAXF_SIZE 1536
#define ENET_RX_DESC 48
@@ -416,9 +415,6 @@ static bool use_iram_for_net(struct devi
#define TXDESC_CONTROL_LAST (1 << 30)
#define TXDESC_CONTROL_INT (1 << 31)
-static int lpc_eth_hard_start_xmit(struct sk_buff *skb,
- struct net_device *ndev);
-
/*
* Structure of a TX/RX descriptors and RX status
*/
@@ -1441,7 +1437,7 @@ static int lpc_eth_drv_probe(struct plat
res->start);
netdev_dbg(ndev, "IO address size :%d\n",
res->end - res->start + 1);
- netdev_err(ndev, "IO address (mapped) :0x%p\n",
+ netdev_dbg(ndev, "IO address (mapped) :0x%p\n",
pldat->net_base);
netdev_dbg(ndev, "IRQ number :%d\n", ndev->irq);
netdev_dbg(ndev, "DMA buffer size :%d\n", pldat->dma_buff_size);
^ permalink raw reply
* [PATCH 2/3] net: lpc_eth: Increase number of TX descriptors
From: Roland Stigge @ 2012-06-11 8:03 UTC (permalink / raw)
To: davem, eric.dumazet, netdev, linux-kernel, kevin.wells,
srinivas.bakki, aletes.xgr, linux-arm-kernel
Cc: Roland Stigge
In-Reply-To: <1339401793-12258-1-git-send-email-stigge@antcom.de>
Since we have enough SRAM, we can increase the number of TX descriptors, so the
"BUSY" warning about occupied TX descriptors doesn't need to show up as often.
Signed-off-by: Roland Stigge <stigge@antcom.de>
Tested-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
---
drivers/net/ethernet/nxp/lpc_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-2.6.orig/drivers/net/ethernet/nxp/lpc_eth.c
+++ linux-2.6/drivers/net/ethernet/nxp/lpc_eth.c
@@ -56,7 +56,7 @@
#define ENET_MAXF_SIZE 1536
#define ENET_RX_DESC 48
-#define ENET_TX_DESC 16
+#define ENET_TX_DESC 32
#define NAPI_WEIGHT 16
^ permalink raw reply
* [PATCH 1/3] net: lpc_eth: Replace WARN() trace with simple pr_warn()
From: Roland Stigge @ 2012-06-11 8:03 UTC (permalink / raw)
To: davem, eric.dumazet, netdev, linux-kernel, kevin.wells,
srinivas.bakki, aletes.xgr, linux-arm-kernel
Cc: Roland Stigge
A WARN() trace indicating a "BUG!" was identified as a "normal" case in the
xmit function in case all TX descriptors are occupied already. In this case,
NETDEV_TX_BUSY is returned, nothing buggy at all.
Signed-off-by: Roland Stigge <stigge@antcom.de>
Tested-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
---
drivers/net/ethernet/nxp/lpc_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-2.6.orig/drivers/net/ethernet/nxp/lpc_eth.c
+++ linux-2.6/drivers/net/ethernet/nxp/lpc_eth.c
@@ -1114,7 +1114,7 @@ static int lpc_eth_hard_start_xmit(struc
buffers */
netif_stop_queue(ndev);
spin_unlock_irq(&pldat->lock);
- WARN(1, "BUG! TX request when no free TX buffers!\n");
+ pr_warn("Note: TX request when no free TX buffers.\n");
return NETDEV_TX_BUSY;
}
^ permalink raw reply
* [PATCH v2] dummy: fix rcu_sched self-detected stalls
From: Eric Dumazet @ 2012-06-11 7:11 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20120610.224800.602807319671005890.davem@davemloft.net>
From: Eric Dumazet <edumazet@google.com>
Trying to "modprobe dummy numdummies=30000" triggers :
INFO: rcu_sched self-detected stall on CPU { 8} (t=60000 jiffies)
After this splat, RTNL is locked and reboot is needed.
We must call cond_resched() to avoid this, even holding RTNL.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
drivers/net/dummy.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index 442d91a..bab0158 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -187,8 +187,10 @@ static int __init dummy_init_module(void)
rtnl_lock();
err = __rtnl_link_register(&dummy_link_ops);
- for (i = 0; i < numdummies && !err; i++)
+ for (i = 0; i < numdummies && !err; i++) {
err = dummy_init_one();
+ cond_resched();
+ }
if (err < 0)
__rtnl_link_unregister(&dummy_link_ops);
rtnl_unlock();
^ permalink raw reply related
* Re: [PATCH] net: Reorder initialization in ip_route_output to fix gcc warning
From: David Miller @ 2012-06-11 7:05 UTC (permalink / raw)
To: roland; +Cc: netdev
In-Reply-To: <CAG4TOxNpw0BAWPK3Zde_mThTvY-ho+Ce_aau3DoLc-0TRFq3PQ@mail.gmail.com>
From: Roland Dreier <roland@kernel.org>
Date: Mon, 11 Jun 2012 00:00:51 -0700
>> I can't figure out what it is actually warning about, can you?
>
> I think gcc thinks it already initialized the __fl_common struct
> once when it hits the .flowi4_oif initializer (which expands to
> __fl_common.flowic_oif), and then having the .daddr / .saddr
> initializers makes it thinks its done with that structure.
>
> So it thinks it's initializing __fl_common.flowic_tos to 0, and
> then the .flowi4_tos initializer comes along as a surprise.
>
> Hmm, looks like http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52880
> which is fixed after gcc 4.7.0. I think it's probably worth working
> around this gcc issue, since this makes W=1 way noisier in
> my build.
I suspected it was a compiler bug :-)
Anyways, agreed, applied.
^ permalink raw reply
* Re: [PATCH] net: Reorder initialization in ip_route_output to fix gcc warning
From: Roland Dreier @ 2012-06-11 7:00 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20120610.234411.850948486629299757.davem@davemloft.net>
> I can't figure out what it is actually warning about, can you?
I think gcc thinks it already initialized the __fl_common struct
once when it hits the .flowi4_oif initializer (which expands to
__fl_common.flowic_oif), and then having the .daddr / .saddr
initializers makes it thinks its done with that structure.
So it thinks it's initializing __fl_common.flowic_tos to 0, and
then the .flowi4_tos initializer comes along as a surprise.
Hmm, looks like http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52880
which is fixed after gcc 4.7.0. I think it's probably worth working
around this gcc issue, since this makes W=1 way noisier in
my build.
- R.
^ permalink raw reply
* Re: [PATCH] net: Reorder initialization in ip_route_output to fix gcc warning
From: David Miller @ 2012-06-11 6:44 UTC (permalink / raw)
To: roland; +Cc: netdev, roland
In-Reply-To: <1339394724-28296-1-git-send-email-roland@kernel.org>
From: Roland Dreier <roland@kernel.org>
Date: Sun, 10 Jun 2012 23:05:24 -0700
> From: Roland Dreier <roland@purestorage.com>
>
> If I build with W=1, for every file that includes <net/route.h>, I get the warning
>
> include/net/route.h: In function 'ip_route_output':
> include/net/route.h:135:3: warning: initialized field overwritten [-Woverride-init]
> include/net/route.h:135:3: warning: (near initialization for 'fl4') [-Woverride-init]
>
> (This is with "gcc (Debian 4.6.3-1) 4.6.3")
>
> A fix seems pretty trivial: move the initialization of .flowi4_tos
> earlier. As far as I can tell, this has no effect on code generation.
>
> Signed-off-by: Roland Dreier <roland@purestorage.com>
I can't figure out what it is actually warning about, can you?
^ permalink raw reply
* [PATCH] net: Reorder initialization in ip_route_output to fix gcc warning
From: Roland Dreier @ 2012-06-11 6:05 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Roland Dreier
From: Roland Dreier <roland@purestorage.com>
If I build with W=1, for every file that includes <net/route.h>, I get the warning
include/net/route.h: In function 'ip_route_output':
include/net/route.h:135:3: warning: initialized field overwritten [-Woverride-init]
include/net/route.h:135:3: warning: (near initialization for 'fl4') [-Woverride-init]
(This is with "gcc (Debian 4.6.3-1) 4.6.3")
A fix seems pretty trivial: move the initialization of .flowi4_tos
earlier. As far as I can tell, this has no effect on code generation.
Signed-off-by: Roland Dreier <roland@purestorage.com>
---
include/net/route.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/route.h b/include/net/route.h
index ed2b78e..9870546 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -130,9 +130,9 @@ static inline struct rtable *ip_route_output(struct net *net, __be32 daddr,
{
struct flowi4 fl4 = {
.flowi4_oif = oif,
+ .flowi4_tos = tos,
.daddr = daddr,
.saddr = saddr,
- .flowi4_tos = tos,
};
return ip_route_output_key(net, &fl4);
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] dummy: fix rcu_sched self-detected stalls
From: David Miller @ 2012-06-11 5:48 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1339239546.6001.177.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 09 Jun 2012 12:59:06 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> Trying to "modprobe dummy numdummies=30000" triggers :
>
> INFO: rcu_sched self-detected stall on CPU { 8} (t=60000 jiffies)
>
> After this splat, RTNL is locked and reboot is needed.
>
> We must call cond_resched() to avoid this, even holding RTNL.
>
> Also remove the ~32767 limit on number of dummies (PAGE_SIZE*8)
>
> Tested with "modprobe dummy numdummies=128000"
> (it took ~12 minutes, because of sysfs)
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
I'm all for the cond_resched() change, but the name optimization
belongs generically in net/core/dev.c not in specific drivers.
So, please submit this cond_resched() fix first then we can work
on the naming performance issue.
Thanks.
^ permalink raw reply
* [PATCH net-next 4/4] 6lowpan: len field is not stored and accessed properly
From: Tony Cheneau @ 2012-06-11 4:40 UTC (permalink / raw)
To: netdev, linux-zigbee-devel; +Cc: alex.bluesman.smirnov
Lenght field should be encoded (and accessed) the other way around.
As it is currently written, it could lead to interroperability issues.
Also, I rewrote the code so that iphc0 argument of
lowpan_alloc_new_frame could be removed.
---
net/ieee802154/6lowpan.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index af2f12e..b400156 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -654,7 +654,7 @@ static void lowpan_fragment_timer_expired(unsigned
long entry_addr) }
static struct lowpan_fragment *
-lowpan_alloc_new_frame(struct sk_buff *skb, u8 iphc0, u8 len, u16 tag)
+lowpan_alloc_new_frame(struct sk_buff *skb, u16 len, u16 tag)
{
struct lowpan_fragment *frame;
@@ -665,7 +665,7 @@ lowpan_alloc_new_frame(struct sk_buff *skb, u8
iphc0, u8 len, u16 tag)
INIT_LIST_HEAD(&frame->list);
- frame->length = (iphc0 & 7) | (len << 3);
+ frame->length = len;
frame->tag = tag;
/* allocate buffer for frame assembling */
@@ -721,13 +721,17 @@ lowpan_process_data(struct sk_buff *skb)
case LOWPAN_DISPATCH_FRAGN:
{
struct lowpan_fragment *frame;
- u8 len, offset;
- u16 tag;
+ /* slen stores the rightmost 8 bits of the 11 bits
length */
+ u8 slen, offset;
+ u16 len, tag;
bool found = false;
- len = lowpan_fetch_skb_u8(skb); /* frame length */
+ slen = lowpan_fetch_skb_u8(skb); /* frame length */
tag = lowpan_fetch_skb_u16(skb);
+ /* adds the 3 MSB to the 8 LSB to retrieve the 11 bits
length */
+ len = ((iphc0 & 7) << 8) | slen;
+
/*
* check if frame assembling with the same tag is
* already in progress
@@ -742,7 +746,7 @@ lowpan_process_data(struct sk_buff *skb)
/* alloc new frame structure */
if (!found) {
- frame = lowpan_alloc_new_frame(skb, iphc0,
len, tag);
+ frame = lowpan_alloc_new_frame(skb, len, tag);
if (!frame)
goto unlock_and_drop;
}
@@ -1000,8 +1004,8 @@ lowpan_skb_fragmentation(struct sk_buff *skb)
tag = fragment_tag++;
/* first fragment header */
- head[0] = LOWPAN_DISPATCH_FRAG1 | (payload_length & 0x7);
- head[1] = (payload_length >> 3) & 0xff;
+ head[0] = LOWPAN_DISPATCH_FRAG1 | ((payload_length >> 8) &
0x7);
+ head[1] = payload_length & 0xff;
head[2] = tag >> 8;
head[3] = tag & 0xff;
--
1.7.3.4
^ permalink raw reply related
* [PATCH net-next 1/4] 6lowpan: Fix in UDP uncompression function when a null pointer gets dereferenced
From: Tony Cheneau @ 2012-06-11 4:38 UTC (permalink / raw)
To: netdev, linux-zigbee-devel; +Cc: alex.bluesman.smirnov
When a UDP packet gets fragmented, a crash will occur during
reassembly.
skb->transport_header is not set during earlier period of fragment
reassembly. As a consequence, calll to udp_hdr() return NULL and uh
(which is NULL) gets dereferenced without much test.
I will post a patch later that will set skb->transport_header
correctly in lowpan_process_data(), so that
lowpan_uncompress_udp_header() behave as intended.
---
net/ieee802154/6lowpan.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 32eb417..a52e795 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -317,6 +317,9 @@ lowpan_uncompress_udp_header(struct sk_buff *skb)
{
struct udphdr *uh = udp_hdr(skb);
u8 tmp;
+
+ if (!uh)
+ goto err;
tmp = lowpan_fetch_skb_u8(skb);
--
1.7.3.4
^ permalink raw reply related
* [PATCH net-next 3/4] 6lowpan: the two bytes of u16 tag needs to be stored/accessed the other way around
From: Tony Cheneau @ 2012-06-11 4:39 UTC (permalink / raw)
To: netdev, linux-zigbee-devel; +Cc: alex.bluesman.smirnov
Or else, tag values, as displayed with a trafic analyser, such a
Wireshark, are not properly displayed (e.g. 0x01 00 insted of 0x00 01,
and so on). ---
net/ieee802154/6lowpan.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index af4f29b..af2f12e 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -307,7 +307,7 @@ static u16 lowpan_fetch_skb_u16(struct sk_buff *skb)
BUG_ON(!pskb_may_pull(skb, 2));
- ret = skb->data[0] | (skb->data[1] << 8);
+ ret = (skb->data[0] << 8) | skb->data[1];
skb_pull(skb, 2);
return ret;
}
@@ -1002,8 +1002,8 @@ lowpan_skb_fragmentation(struct sk_buff *skb)
/* first fragment header */
head[0] = LOWPAN_DISPATCH_FRAG1 | (payload_length & 0x7);
head[1] = (payload_length >> 3) & 0xff;
- head[2] = tag & 0xff;
- head[3] = tag >> 8;
+ head[2] = tag >> 8;
+ head[3] = tag & 0xff;
err = lowpan_fragment_xmit(skb, head, header_length, 0, 0);
--
1.7.3.4
^ permalink raw reply related
* [PATCH net-next 2/4] 6lowpan: Incorrect type in lowpan_alloc_new_frame() definition : tag is u8 but should be u16
From: Tony Cheneau @ 2012-06-11 4:39 UTC (permalink / raw)
To: netdev, linux-zigbee-devel; +Cc: alex.bluesman.smirnov
lowpan_alloc_new_frame() takes u8 tag as an argument. Howerer,
its only caller, lowpan_process_data() passes down a u16. Hence,
the tag value got corrupted.
This prevent 6lowpan fragment reassembly after 256 fragmented packets
have been reassembled by the same recipient.
---
net/ieee802154/6lowpan.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index a52e795..af4f29b 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -654,7 +654,7 @@ static void lowpan_fragment_timer_expired(unsigned
long entry_addr) }
static struct lowpan_fragment *
-lowpan_alloc_new_frame(struct sk_buff *skb, u8 iphc0, u8 len, u8 tag)
+lowpan_alloc_new_frame(struct sk_buff *skb, u8 iphc0, u8 len, u16 tag)
{
struct lowpan_fragment *frame;
--
1.7.3.4
^ permalink raw reply related
* [PATCH net-next 0/4] 6lowpan: set of bug fixes
From: Tony Cheneau @ 2012-06-11 4:37 UTC (permalink / raw)
To: netdev, linux-zigbee-devel; +Cc: alex.bluesman.smirnov
Hello,
(This is my first time submitting patches. If I fail to apply to
some rules in here, please let me know)
After reading the 6lowpan code, I found a few issues. This patchset
fixes them. This patchset should apply cleanly against the current
net-next. It contains only bug fixes, I'll send later on a few other
patches that will offer new functionalities.
This is a set of 4 small patches that correct bugs in 6lowpan:
- patch 1 fixes a potential crash when reassembling UDP fragments
- patch 2 fixes a type issues that prevent the fragmentation reassembly
to operate properly.
- patch 3 and 4 corrects field encoding issues
Hope it helps.
Regards,
Tony Cheneau
^ permalink raw reply
* Re: [PATCH rfc net] Allow the autoconfigured network interface to be renamed.
From: David Miller @ 2012-06-11 3:25 UTC (permalink / raw)
To: scott; +Cc: netdev, scott.parlane
In-Reply-To: <1339228087-14870-1-git-send-email-scott@scottnz.com>
From: Scott Parlane <scott@scottnz.com>
Date: Sat, 9 Jun 2012 19:48:07 +1200
> From: Scott Parlane <scott.parlane@alliedtelesis.co.nz>
>
> if IP_PNP_RENAME_DEV is set, the first interface to be configured
> automatically by the kernel during boot will be renamed.
>
> IP_PNP_DEV_NEWNAME is the name to give the autoconfigured device.
>
> No changes will be made to any interface that is not autoconfigured.
>
> This allows the assurance of the boot device name, without the need
> for an initramfs.
>
> Signed-off-by: Scott Parlane <scott.parlane@alliedtelesis.co.nz>
Making this a compile time option makes absolutely no sense at all.
Assuming this feature is desirable at all (which is a big IF), it
should be a kernel command line option.
^ permalink raw reply
* Re: [PATCH] virtio-net: fix a race on 32bit arches
From: David Miller @ 2012-06-11 3:23 UTC (permalink / raw)
To: mst; +Cc: eric.dumazet, netdev, linux-kernel, virtualization, shemminger
In-Reply-To: <20120610102512.GB6793@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Sun, 10 Jun 2012 13:25:12 +0300
> On Wed, Jun 06, 2012 at 10:35:24AM +0200, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> commit 3fa2a1df909 (virtio-net: per cpu 64 bit stats (v2)) added a race
>> on 32bit arches.
>>
>> We must use separate syncp for rx and tx path as they can be run at the
>> same time on different cpus. Thus one sequence increment can be lost and
>> readers spin forever.
>>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Cc: Stephen Hemminger <shemminger@vyatta.com>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>
> I'm still thinking about moving tx to take a xmit lock long term,
> meanwhile this fix appears appropriate for 3.5.
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> Dave, can you pick this up pls?
Done, thanks.
^ permalink raw reply
* Re: [PATCH] r8169: avoid NAPI scheduling delay.
From: David Miller @ 2012-06-11 3:21 UTC (permalink / raw)
To: romieu; +Cc: netdev, davej, marc.c.dionne, tglx, js, realnc, hayeswang
In-Reply-To: <20120609205316.GA5915@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sat, 9 Jun 2012 22:53:16 +0200
> While reworking the r8169 driver a few months ago to perform the
> smallest amount of work in the irq handler, I took care of avoiding
> any irq mask register operation in the slow work dedicated user
> context thread. The slow work thread scheduled an extra round of NAPI
> work which would ultimately set the irq mask register as required,
> thus keeping such irq mask operations in the NAPI handler.
> It would eventually race with the irq handler and delay NAPI execution
> for - assuming no further irq - a whole ksoftirqd period. Mildly a
> problem for rare link changes or corner case PCI events.
>
> The race was always lost after the last bh disabling lock had been
> removed from the work thread and people started wondering where those
> pesky "NOHZ: local_softirq_pending 08" messages came from.
>
> Actually the irq mask register _can_ be set up directly in the slow
> work thread.
>
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Reported-by: Dave Jones <davej@redhat.com>
> Tested-by: Marc Dionne <marc.c.dionne@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] af_packet: check loop for greater than zero in tpacket_fill_skb
From: David Miller @ 2012-06-11 3:10 UTC (permalink / raw)
To: danborkmann; +Cc: netdev
In-Reply-To: <4FD4F494.6020102@iogearbox.net>
From: Daniel Borkmann <danborkmann@iogearbox.net>
Date: Sun, 10 Jun 2012 21:25:08 +0200
> It could be more safe to check the 'to_write' for 'greater than zero'
> instead for 'not zero'. 'to_write' is of type int and subtraction operations
> are performed on it, so in the case of malformed values that are
> subtracted from 'to_write', it could become less than zero, which is then
> interpreted as 'not zero' in the while condition, thus the loop won't
> return as expected.
>
> Signed-off-by: Daniel Borkmann <daniel.borkmann@tik.ee.ethz.ch>
I hate this kind of change.
The implication is that there are bugs in the loop that can cause
the value to go negative.
Unless you can show that this actually happens, leave this clean
test alone.
^ 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