* addrconf: refcnt with IPV6_PRIVACY enabled
From: Sergey Senozhatsky @ 2010-11-23 13:40 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, linux-kernel, Pekka Savola (ipv6), Hideaki YOSHIFUJI
Hello,
I've sent a patch http://lkml.org/lkml/2010/11/21/124
"[PATCH] ipv6: fix inet6_dev refcnt with IPV6_PRIVACY enabled" related
to wrong in6_dev refcnt value when IPV6_PRIVACY enabled.
I've took a second look, and question arised -
Is it actually necessary to in6_dev_hold/in6_dev_put in ipv6_regen_rndid?
In ipv6_regen_rndid we call in6_dev_hold only when mod_timer == 0,
and always in6_dev_put.
I've removed check for < 0 and goto from __ipv6_regen_rndid call, since it
always return 0.
Please, kindly review V2.
---
net/ipv6/addrconf.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2fc35b3..8187d14 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -425,7 +425,6 @@ static struct inet6_dev * ipv6_add_dev(struct net_device *dev)
dev->name);
ndev->cnf.use_tempaddr = -1;
} else {
- in6_dev_hold(ndev);
ipv6_regen_rndid((unsigned long) ndev);
}
#endif
@@ -1653,8 +1652,7 @@ static void ipv6_regen_rndid(unsigned long data)
if (idev->dead)
goto out;
- if (__ipv6_regen_rndid(idev) < 0)
- goto out;
+ __ipv6_regen_rndid(idev);
expires = jiffies +
idev->cnf.temp_prefered_lft * HZ -
@@ -1667,13 +1665,11 @@ static void ipv6_regen_rndid(unsigned long data)
goto out;
}
- if (!mod_timer(&idev->regen_timer, expires))
- in6_dev_hold(idev);
-
+ mod_timer(&idev->regen_timer, expires);
+
out:
write_unlock_bh(&idev->lock);
rcu_read_unlock_bh();
- in6_dev_put(idev);
}
static int __ipv6_try_regen_rndid(struct inet6_dev *idev, struct in6_addr *tmpaddr) {
^ permalink raw reply related
* Re: [PATCH] xfrm: use gre key as flow upper protocol info
From: Timo Teräs @ 2010-11-23 14:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev, herbert
In-Reply-To: <20101115.104322.212675424.davem@davemloft.net>
On 11/15/2010 08:43 PM, David Miller wrote:
> From: Timo Teräs <timo.teras@iki.fi>
> Date: Wed, 3 Nov 2010 16:41:38 +0200
>
>> The GRE Key field is intended to be used for identifying an individual
>> traffic flow within a tunnel. It is useful to be able to have XFRM
>> policy selector matches to have different policies for different
>> GRE tunnels.
>>
>> Signed-off-by: Timo Teräs <timo.teras@iki.fi>
>
> I'll apply this to net-next-2.6, thanks.
Hmm.. I tested this with using the "ip xfrm" sport and dport manually
(without doing the actual userland support for this), and checking it in
kernel with printk's in various places that the stuff matches. In these
tests I checked the sport/dport by hand and apparently messed up the
byte order.
Now that I'm writing the GRE support for "ip xfrm" I think that missed
two htons() calls.
I was confused if xfrm_flowi_{s|d}port was supposed to return host or
net byte order for non-TCP/UDP packets.
I was under the assumption that host byte order since case IPPROTO_ICMP
swaps the byte order. But it would appear that the fl->fl_icmp_* is
actually host order and it's turned to network order; this is also
implied by using htons instead of ntohs. Since I decided to keep
fl_gre_key in network order, the return value would now be inconsistent,
and make userland abi endianess dependent.
I'll follow up with iproute2 patch soon.
So we probably would need to do:
xfrm: fix gre key endianess
fl->fl_gre_key is network byte order contrary to fl->fl_icmp_*.
Make xfrm_flowi_{s|d}port return network byte order values for gre
key too.
Signed-off-by: Timo Teräs <timo.teras@iki.fi>
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 1a57ff9..916ac47 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -806,7 +806,7 @@ __be16 xfrm_flowi_sport(struct flowi *fl)
port = htons(fl->fl_mh_type);
break;
case IPPROTO_GRE:
- port = htonl(fl->fl_gre_key) >> 16;
+ port = htons(ntohl(fl->fl_gre_key) >> 16);
break;
default:
port = 0; /*XXX*/
@@ -830,7 +830,7 @@ __be16 xfrm_flowi_dport(struct flowi *fl)
port = htons(fl->fl_icmp_code);
break;
case IPPROTO_GRE:
- port = htonl(fl->fl_gre_key) & 0xffff;
+ port = htons(ntohl(fl->fl_gre_key) & 0xffff);
break;
default:
port = 0; /*XXX*/
^ permalink raw reply related
* Re: addrconf: refcnt with IPV6_PRIVACY enabled
From: Eric Dumazet @ 2010-11-23 14:33 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: David S. Miller, netdev, linux-kernel, Pekka Savola (ipv6),
Hideaki YOSHIFUJI
In-Reply-To: <20101123134003.GB4142@swordfish.minsk.epam.com>
Le mardi 23 novembre 2010 à 15:40 +0200, Sergey Senozhatsky a écrit :
> Hello,
>
> I've sent a patch http://lkml.org/lkml/2010/11/21/124
> "[PATCH] ipv6: fix inet6_dev refcnt with IPV6_PRIVACY enabled" related
> to wrong in6_dev refcnt value when IPV6_PRIVACY enabled.
>
> I've took a second look, and question arised -
> Is it actually necessary to in6_dev_hold/in6_dev_put in ipv6_regen_rndid?
> In ipv6_regen_rndid we call in6_dev_hold only when mod_timer == 0,
> and always in6_dev_put.
>
>
> I've removed check for < 0 and goto from __ipv6_regen_rndid call, since it
> always return 0.
>
>
> Please, kindly review V2.
>
> ---
>
> net/ipv6/addrconf.c | 10 +++-------
> 1 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 2fc35b3..8187d14 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -425,7 +425,6 @@ static struct inet6_dev * ipv6_add_dev(struct net_device *dev)
> dev->name);
> ndev->cnf.use_tempaddr = -1;
> } else {
> - in6_dev_hold(ndev);
> ipv6_regen_rndid((unsigned long) ndev);
> }
> #endif
> @@ -1653,8 +1652,7 @@ static void ipv6_regen_rndid(unsigned long data)
> if (idev->dead)
> goto out;
>
> - if (__ipv6_regen_rndid(idev) < 0)
> - goto out;
> + __ipv6_regen_rndid(idev);
>
> expires = jiffies +
> idev->cnf.temp_prefered_lft * HZ -
> @@ -1667,13 +1665,11 @@ static void ipv6_regen_rndid(unsigned long data)
> goto out;
> }
>
> - if (!mod_timer(&idev->regen_timer, expires))
> - in6_dev_hold(idev);
> -
> + mod_timer(&idev->regen_timer, expires);
> +
> out:
> write_unlock_bh(&idev->lock);
> rcu_read_unlock_bh();
> - in6_dev_put(idev);
> }
>
> static int __ipv6_try_regen_rndid(struct inet6_dev *idev, struct in6_addr *tmpaddr) {
>
Sergey, I dont think your patches (V1 or V2) are correct.
You leak a refcount if (idev->dead) is true
ipv6_regen_rndid() is the timer handler function, it must call
in6_dev_put() at the end, unless we rearm the timer.
So this code is correct.
if (!mod_timer(&idev->regen_timer, expires))
in6_dev_hold(idev);
...
in6_dev_put(idev);
And we must call in6_dev_hold() before calling the handler, either
directly from ipv6_add_dev() [Where your first patch tried to remove the
dev_hold() call], or when arming the timer.
Are you sure the problem you try to solve is not already solved in
commit 88b2a9a3d98a19496d64aadda7158c0ad51cbe7d in net-2.6 tree ?
http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commitdiff;h=88b2a9a3d98a19496d64aadda7158c0ad51cbe7d
^ permalink raw reply
* [PATCH] flexcan: fix NAPI for bus errors
From: John Ogness @ 2010-11-23 14:34 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: socketcan-core, netdev, kernel
If bus error reporting is disabled and bus errors occur, the flexcan
driver will hog the system because it continually re-enables the IRQs
(work_done = 0). This patch changes the driver to only re-enable the
IRQs if some work was actually done. This allows the features of NAPI to
be used for bus errors as well (when bus error reporting is disabled).
This patch is against Linux next-20101123.
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
drivers/net/can/flexcan.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
--- next-20101123-a/drivers/net/can/flexcan.c
+++ next-20101123-b/drivers/net/can/flexcan.c
@@ -535,9 +535,12 @@ static int flexcan_poll(struct napi_stru
if (work_done < quota) {
napi_complete(napi);
- /* enable IRQs */
- writel(FLEXCAN_IFLAG_DEFAULT, ®s->imask1);
- writel(priv->reg_ctrl_default, ®s->ctrl);
+
+ if (work_done > 0) {
+ /* enable IRQs */
+ writel(FLEXCAN_IFLAG_DEFAULT, ®s->imask1);
+ writel(priv->reg_ctrl_default, ®s->ctrl);
+ }
}
return work_done;
^ permalink raw reply
* Re: pc300too on a modern kernel?
From: Ward Vandewege @ 2010-11-23 14:44 UTC (permalink / raw)
To: Krzysztof Halasa; +Cc: Bernie Innocenti, lkml, Jan Seiffert, netdev
In-Reply-To: <m38w0l2gzh.fsf@intrepid.localdomain>
On Mon, Nov 22, 2010 at 10:20:02PM +0100, Krzysztof Halasa wrote:
> > With this workaround applied, we're st seeing occasional clusters of
> > packet loss. We're working to graph the ping loss alongside traffic to
> > see if there's any correlation.
>
> That's interesting. I remember seeing some TX underruns at higher
> speeds, though nothing alike at 2 Mb/s. What bit rate are you using?
> Does "ifconfig hdlc0" show any errors?
This turned out to be caused by line saturation. We were not seeing this
before we upgraded to the latest kernel because we have a set of qos rules
that we forgot to install on our new box. Mea culpa...
Thanks,
Ward.
--
Ward Vandewege <ward@gnu.org>
^ permalink raw reply
* Re: addrconf: refcnt with IPV6_PRIVACY enabled
From: Sergey Senozhatsky @ 2010-11-23 14:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, netdev, linux-kernel, Pekka Savola (ipv6),
Hideaki YOSHIFUJI
In-Reply-To: <1290522803.3046.11.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]
Hello Eric,
On (11/23/10 15:33), Eric Dumazet wrote:
> Sergey, I dont think your patches (V1 or V2) are correct.
>
> You leak a refcount if (idev->dead) is true
>
Hm, that's true. Thank you.
> ipv6_regen_rndid() is the timer handler function, it must call
> in6_dev_put() at the end, unless we rearm the timer.
>
> So this code is correct.
>
>
> if (!mod_timer(&idev->regen_timer, expires))
> in6_dev_hold(idev);
> ...
> in6_dev_put(idev);
>
Even if in6_dev_hold hasn't been called from ipv6_regen_rndid?
>
> And we must call in6_dev_hold() before calling the handler, either
> directly from ipv6_add_dev() [Where your first patch tried to remove the
> dev_hold() call], or when arming the timer.
>
> Are you sure the problem you try to solve is not already solved in
> commit 88b2a9a3d98a19496d64aadda7158c0ad51cbe7d in net-2.6 tree ?
>
> http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commitdiff;h=88b2a9a3d98a19496d64aadda7158c0ad51cbe7d
>
Oh, haven't seen this one. I'll try, thank you.
Sergey
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
* Re: alchemy/gpr: au1000_eth regression with v2.6.37rc2
From: Wolfgang Grandegger @ 2010-11-23 15:01 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Linux-MIPS, Netdev
In-Reply-To: <201011191146.01454.florian@openwrt.org>
[-- Attachment #1: Type: text/plain, Size: 2849 bytes --]
Hi Florian,
On 11/19/2010 11:46 AM, Florian Fainelli wrote:
> Hello Wolfgang,
>
> On Friday 19 November 2010 11:29:45 Wolfgang Grandegger wrote:
>> Hello Florian,
>>
>> On 11/18/2010 11:30 PM, Florian Fainelli wrote:
>>> Hello Wolfgang,
>>>
>>> Le Thursday 18 November 2010 20:59:15, Wolfgang Grandegger a écrit :
>>>> Hello,
>>>>
>>>> I just realized that the v2.6.37-rc2 kernel does not boot any more on
>>>> the Alchemy GPR board. It works fine with v2.6.36. It hangs in the
>>>> probe function of the au1000_eth driver when probing the second
>>>>
>>>> ethernet port (eth1):
>>>> au1000_eth_mii: probed
>>>> au1000-eth au1000-eth.0: (unregistered net_device): attached PHY
>>>> driver
>>>>
>>>> [Generic PHY] (mii_bus:phy_addr=0:00, irq=-1) au1000-eth au1000-eth.0:
>>>> eth0: Au1xx0 Ethernet found at 0x10500000, irq 35 au1000_eth: au1000_eth
>>>> version 1.7 Pete Popov <ppopov@embeddedalley.com> ... hangs ...
>>>>
>>>> Similar messages should follow for eth1. I narrowed down (bisect'ed) the
>>>>
>>>> problem to commit:
>>>> commit d0e7cb5d401695809ba8c980124ab1d8c66efc8b
>>>> Author: Florian Fainelli <florian@openwrt.org>
>>>> Date: Wed Sep 8 11:15:13 2010 +0000
>>>>
>>>> au1000-eth: remove volatiles, switch to I/O accessors
>>>>
>>>> Remove all the volatile keywords where they were used, switch to
>>>> using
>>>>
>>>> the proper readl/writel accessors.
>>>>
>>>> Signed-off-by: Florian Fainelli <florian@openwrt.org>
>>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>>>
>>>> The kernel actually hangs when accessing "&aup->mac->mii_control" in
>>>> au1000_mdio_read(), but only for eth1. Any idea what does go wrong?
>>>
>>> I do not understand so far while it hangs only for eth1. My device only
>>> has one ethernet MAC, so I could not notice the problem. Looking at this
>>> close, there are a couple of u32 const* usages in
>>> au1000_mdio_{read,write} which are looking wrong to me now. Can you try
>>> to remove these?
>>
>> That did not help.
>
> I suspected it, but thanks for the confirmation.
>
>>
>>>> In principle, I do not want to access the MII regs of the MAC because
>>>> eth0 and eth1 are connected to switches. But that's not possible, even
>>>> with "aup->phy_static_config=1" and "aup->phy_addr=0".
>>>
>>> If you think this is another issue, I will fix it in another patch.
>>
>> Accessing the MII registers of the MAC should not hang the system even
>> if I do not need to. First I want to understand why. Looks like a wired
>> optimizer issue.
>
> I definitively agree, furthermore since there is a timeout for read and write
> operations. I will look at the assembly and see if I can see anything
> different.
The attached patch fixes the issue. It's caused by a simple porting
error. I'm going to prepare a proper patch later today.
Wolfgang.
[-- Attachment #2: au1000-eth-mac-enable.patch --]
[-- Type: text/x-diff, Size: 1261 bytes --]
diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
index 43489f8..53eff9b 100644
--- a/drivers/net/au1000_eth.c
+++ b/drivers/net/au1000_eth.c
@@ -155,10 +155,10 @@ static void au1000_enable_mac(struct net_device *dev, int force_reset)
spin_lock_irqsave(&aup->lock, flags);
if (force_reset || (!aup->mac_enabled)) {
- writel(MAC_EN_CLOCK_ENABLE, &aup->enable);
+ writel(MAC_EN_CLOCK_ENABLE, aup->enable);
au_sync_delay(2);
writel((MAC_EN_RESET0 | MAC_EN_RESET1 | MAC_EN_RESET2
- | MAC_EN_CLOCK_ENABLE), &aup->enable);
+ | MAC_EN_CLOCK_ENABLE), aup->enable);
au_sync_delay(2);
aup->mac_enabled = 1;
@@ -503,9 +503,9 @@ static void au1000_reset_mac_unlocked(struct net_device *dev)
au1000_hard_stop(dev);
- writel(MAC_EN_CLOCK_ENABLE, &aup->enable);
+ writel(MAC_EN_CLOCK_ENABLE, aup->enable);
au_sync_delay(2);
- writel(0, &aup->enable);
+ writel(0, aup->enable);
au_sync_delay(2);
aup->tx_full = 0;
@@ -1119,7 +1119,7 @@ static int __devinit au1000_probe(struct platform_device *pdev)
/* set a random MAC now in case platform_data doesn't provide one */
random_ether_addr(dev->dev_addr);
- writel(0, &aup->enable);
+ writel(0, aup->enable);
aup->mac_enabled = 0;
pd = pdev->dev.platform_data;
^ permalink raw reply related
* [PATCH] iproute2: support xfrm upper protocol gre key
From: Timo Teräs @ 2010-11-23 15:02 UTC (permalink / raw)
To: shemminger, netdev; +Cc: Timo Teräs
The gre key handling is consistent with ip tunnel side: both
dotted-quad and number are accepted, but dotted-quad is used
for printing.
Signed-off-by: Timo Teräs <timo.teras@iki.fi>
---
This is the userland part for:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commitdiff;h=cc9ff19da9bf76a2f70bcb80225a1c587c162e52
However, that commit is flawed, and for this patch to work
properly, the following patch is needed:
http://patchwork.ozlabs.org/patch/72668/
ip/ipxfrm.c | 42 ++++++++++++++++++++++++++++++++++++++++++
ip/xfrm_policy.c | 3 ++-
man/man8/ip.8 | 25 ++++++++++++++++---------
3 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index 99a6756..cf4b7c7 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -35,6 +35,7 @@
#include <linux/netlink.h>
#include <linux/rtnetlink.h>
#include <linux/xfrm.h>
+#include <arpa/inet.h>
#include "utils.h"
#include "xfrm.h"
@@ -483,6 +484,14 @@ void xfrm_selector_print(struct xfrm_selector *sel, __u16 family,
if (sel->dport_mask)
fprintf(fp, "code %u ", ntohs(sel->dport));
break;
+ case IPPROTO_GRE:
+ if (sel->sport_mask || sel->dport_mask) {
+ struct in_addr key;
+ key.s_addr = htonl((ntohs(sel->sport) << 16) + ntohs(sel->dport));
+ inet_ntop(AF_INET, &key, abuf, sizeof(abuf));
+ fprintf(fp, "key %s ", abuf);
+ }
+ break;
case IPPROTO_MH:
if (sel->sport_mask)
fprintf(fp, "type %u ", ntohs(sel->sport));
@@ -1086,6 +1095,7 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
char *dportp = NULL;
char *typep = NULL;
char *codep = NULL;
+ char *grekey = NULL;
while (1) {
if (strcmp(*argv, "proto") == 0) {
@@ -1162,6 +1172,29 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
filter.upspec_dport_mask = XFRM_FILTER_MASK_FULL;
+ } else if (strcmp(*argv, "key") == 0) {
+ unsigned key;
+
+ grekey = *argv;
+
+ NEXT_ARG();
+
+ if (strchr(*argv, '.'))
+ key = htonl(get_addr32(*argv));
+ else {
+ if (get_unsigned(&key, *argv, 0)<0) {
+ fprintf(stderr, "invalid value of \"key\"\n");
+ exit(-1);
+ }
+ }
+
+ sel->sport = htons(key >> 16);
+ sel->dport = htons(key & 0xffff);
+ sel->sport_mask = ~((__u16)0);
+ sel->dport_mask = ~((__u16)0);
+
+ filter.upspec_dport_mask = XFRM_FILTER_MASK_FULL;
+
} else {
PREV_ARG(); /* back track */
break;
@@ -1196,6 +1229,15 @@ static int xfrm_selector_upspec_parse(struct xfrm_selector *sel,
exit(1);
}
}
+ if (grekey) {
+ switch (sel->proto) {
+ case IPPROTO_GRE:
+ break;
+ default:
+ fprintf(stderr, "\"key\" is invalid with proto=%s\n", strxf_proto(sel->proto));
+ exit(1);
+ }
+ }
*argcp = argc;
*argvp = argv;
diff --git a/ip/xfrm_policy.c b/ip/xfrm_policy.c
index 121afa1..dcb3da4 100644
--- a/ip/xfrm_policy.c
+++ b/ip/xfrm_policy.c
@@ -66,7 +66,8 @@ static void usage(void)
fprintf(stderr, "SELECTOR := src ADDR[/PLEN] dst ADDR[/PLEN] [ UPSPEC ] [ dev DEV ]\n");
fprintf(stderr, "UPSPEC := proto PROTO [ [ sport PORT ] [ dport PORT ] |\n");
- fprintf(stderr, " [ type NUMBER ] [ code NUMBER ] ]\n");
+ fprintf(stderr, " [ type NUMBER ] [ code NUMBER ] |\n");
+ fprintf(stderr, " [ key { DOTTED_QUAD | NUMBER } ] ]\n");
//fprintf(stderr, "DEV - device name(default=none)\n");
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 1a73efa..c1e03f3 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -547,7 +547,10 @@ throw " | " unreachable " | " prohibit " | " blackhole " | " nat " ]"
.RB " [ " type
.IR NUMBER " ] "
.RB " [ " code
-.IR NUMBER " ]] "
+.IR NUMBER " ] | "
+.br
+.RB " [ " key
+.IR KEY " ]] "
.ti -8
.IR LIMIT-LIST " := [ " LIMIT-LIST " ] |"
@@ -642,7 +645,10 @@ throw " | " unreachable " | " prohibit " | " blackhole " | " nat " ]"
.RB " [ " type
.IR NUMBER " ] "
.RB " [ " code
-.IR NUMBER " ] ] "
+.IR NUMBER " ] | "
+.br
+.RB " [ " key
+.IR KEY " ] ] "
.ti -8
.IR ACTION " := "
@@ -2487,9 +2493,11 @@ is defined by source port
.BR sport ", "
destination port
.BR dport ", " type
-as number and
+as number,
.B code
-also number.
+also number and
+.BR key
+as dotted-quad or number.
.TP
.BI dev " DEV "
@@ -2556,11 +2564,10 @@ and the other choice is
.TP
.IR UPSPEC
is specified by
-.BR sport ", "
-.BR dport ", " type
-and
-.B code
-(NUMBER).
+.BR sport " and " dport " (for UDP/TCP), "
+.BR type " and " code " (for ICMP; as number) or "
+.BR key " (for GRE; as dotted-quad or number)."
+.
.SS ip xfrm monitor - is used for listing all objects or defined group of them.
The
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 4/9] AF_UNIX: find the recipients for multicast messages
From: Alban Crequy @ 2010-11-23 15:03 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, shemminger, gorcunov, adobriyan, lennart,
kay.sievers, ian.molton, netdev, linux-kernel
In-Reply-To: <20101122.110519.39205345.davem@davemloft.net>
Le Mon, 22 Nov 2010 11:05:19 -0800 (PST),
David Miller <davem@davemloft.net> a écrit :
> From: Alban Crequy <alban.crequy@collabora.co.uk>
> Date: Mon, 22 Nov 2010 18:36:17 +0000
>
> > unix_find_multicast_recipients() builds an array of recipients. It
> > can either find the peers of a specific multicast address, or find
> > all the peers of all multicast group the sender is part of.
> >
> > Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
>
> You really should use RCU to lock this stuff, this way sends run
> lockless and have less worries wrt. the memory allocation. You'll
> also only take a spinlock in the write paths which change the
> multicast groups, which ought to be rare.
I understand the benefit to use RCU in order to have lockless sends.
But with RCU I will still have worries about the memory allocation:
- I cannot allocate inside a rcu_read_lock()-rcu_read_unlock() block.
- If I iterate locklessly over the multicast group members with
hlist_for_each_entry_rcu(), new members can be added, so the
array can be allocated with the wrong size and I have to try again
("goto try_again") when this rare case occurs.
- Another idea would be to avoid completely the allocation by inlining
unix_find_multicast_recipients() inside unix_dgram_sendmsg() and
delivering the messages to the recipients as long as the list is
being iterated locklessly. But I want to provide atomicity of
delivery: the message must be delivered with skb_queue_tail() either
to all the recipients or to none of them in case of interruption or
memory pressure. I don't see how I can achieve that without
iterating several times on the list of recipients, hence the
allocation and the copy in the array. I also want to guarantee the
order of delivery as described in multicast-unix-sockets.txt and for
this, I am taking lots of spinlocks anyway. I don't see how to avoid
that, but I would be happy to be wrong and have a better solution.
> Although to be honest you should optimize the case of small numbers of
> recipients, in the same way we optimize small numbers of iovecs on
> sends. Have an on-stack array that holds a small number of entries
> and use that if the set fits, otherwise dynamic allocation.
To give an idea of the number of members in a multicast group for the
D-Bus use case, I have 90 D-Bus connections on my session bus:
$ dbus-send --print-reply --dest=org.freedesktop.DBus \
/org/freedesktop/DBus org.freedesktop.DBus.ListNames | grep '":'|wc -l
90
In common cases, there should be only a few real recipients (1 or 2?)
after the socket filters eliminate most of them, but
unix_find_multicast_recipients() will still allocate an array of
about that size.
^ permalink raw reply
* Re: addrconf: refcnt with IPV6_PRIVACY enabled
From: Eric Dumazet @ 2010-11-23 15:08 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: David S. Miller, netdev, linux-kernel, Pekka Savola (ipv6),
Hideaki YOSHIFUJI
In-Reply-To: <20101123145331.GA4102@swordfish.minsk.epam.com>
Le mardi 23 novembre 2010 à 16:53 +0200, Sergey Senozhatsky a écrit :
>
> > ipv6_regen_rndid() is the timer handler function, it must call
> > in6_dev_put() at the end, unless we rearm the timer.
> >
> > So this code is correct.
> >
> >
> > if (!mod_timer(&idev->regen_timer, expires))
> > in6_dev_hold(idev);
> > ...
> > in6_dev_put(idev);
> >
>
> Even if in6_dev_hold hasn't been called from ipv6_regen_rndid?
>
Yes.
Rules are :
in6_dev_hold() when arming timer, so that device doesnt disappear while
timer handler might run and need it.
in6_dev_put() when timer handler finishes
^ permalink raw reply
* Re: addrconf: refcnt with IPV6_PRIVACY enabled
From: Sergey Senozhatsky @ 2010-11-23 15:17 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, netdev, linux-kernel, Pekka Savola (ipv6),
Hideaki YOSHIFUJI
In-Reply-To: <1290522803.3046.11.camel@edumazet-laptop>
[-- Attachment #1: Type: text/plain, Size: 440 bytes --]
On (11/23/10 15:33), Eric Dumazet wrote:
> Are you sure the problem you try to solve is not already solved in
> commit 88b2a9a3d98a19496d64aadda7158c0ad51cbe7d in net-2.6 tree ?
>
> http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commitdiff;h=88b2a9a3d98a19496d64aadda7158c0ad51cbe7d
>
Seems to work for me.
Sorry for noise, thank you.
Tested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Sergey
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
* Re: alchemy/gpr: au1000_eth regression with v2.6.37rc2
From: Florian Fainelli @ 2010-11-23 15:33 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Linux-MIPS, Netdev
In-Reply-To: <4CEBD751.7090807@grandegger.com>
Hello Wolfgang,
On Tuesday 23 November 2010 16:01:37 Wolfgang Grandegger wrote:
> Hi Florian,
>
> On 11/19/2010 11:46 AM, Florian Fainelli wrote:
> > Hello Wolfgang,
> >
> > On Friday 19 November 2010 11:29:45 Wolfgang Grandegger wrote:
> >> Hello Florian,
> >>
> >> On 11/18/2010 11:30 PM, Florian Fainelli wrote:
> >>> Hello Wolfgang,
> >>>
> >>> Le Thursday 18 November 2010 20:59:15, Wolfgang Grandegger a écrit :
> >>>> Hello,
> >>>>
> >>>> I just realized that the v2.6.37-rc2 kernel does not boot any more on
> >>>> the Alchemy GPR board. It works fine with v2.6.36. It hangs in the
> >>>> probe function of the au1000_eth driver when probing the second
> >>>>
> >>>> ethernet port (eth1):
> >>>> au1000_eth_mii: probed
> >>>> au1000-eth au1000-eth.0: (unregistered net_device): attached PHY
> >>>> driver
> >>>>
> >>>> [Generic PHY] (mii_bus:phy_addr=0:00, irq=-1) au1000-eth au1000-eth.0:
> >>>> eth0: Au1xx0 Ethernet found at 0x10500000, irq 35 au1000_eth:
> >>>> au1000_eth version 1.7 Pete Popov <ppopov@embeddedalley.com> ...
> >>>> hangs ...
> >>>>
> >>>> Similar messages should follow for eth1. I narrowed down (bisect'ed)
> >>>> the
> >>>>
> >>>> problem to commit:
> >>>> commit d0e7cb5d401695809ba8c980124ab1d8c66efc8b
> >>>> Author: Florian Fainelli <florian@openwrt.org>
> >>>> Date: Wed Sep 8 11:15:13 2010 +0000
> >>>>
> >>>> au1000-eth: remove volatiles, switch to I/O accessors
> >>>>
> >>>> Remove all the volatile keywords where they were used, switch to
> >>>> using
> >>>>
> >>>> the proper readl/writel accessors.
> >>>>
> >>>> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> >>>> Signed-off-by: David S. Miller <davem@davemloft.net>
> >>>>
> >>>> The kernel actually hangs when accessing "&aup->mac->mii_control" in
> >>>> au1000_mdio_read(), but only for eth1. Any idea what does go wrong?
> >>>
> >>> I do not understand so far while it hangs only for eth1. My device only
> >>> has one ethernet MAC, so I could not notice the problem. Looking at
> >>> this close, there are a couple of u32 const* usages in
> >>> au1000_mdio_{read,write} which are looking wrong to me now. Can you try
> >>> to remove these?
> >>
> >> That did not help.
> >
> > I suspected it, but thanks for the confirmation.
> >
> >>>> In principle, I do not want to access the MII regs of the MAC because
> >>>> eth0 and eth1 are connected to switches. But that's not possible, even
> >>>> with "aup->phy_static_config=1" and "aup->phy_addr=0".
> >>>
> >>> If you think this is another issue, I will fix it in another patch.
> >>
> >> Accessing the MII registers of the MAC should not hang the system even
> >> if I do not need to. First I want to understand why. Looks like a wired
> >> optimizer issue.
> >
> > I definitively agree, furthermore since there is a timeout for read and
> > write operations. I will look at the assembly and see if I can see
> > anything different.
>
> The attached patch fixes the issue. It's caused by a simple porting
> error. I'm going to prepare a proper patch later today.
Nasty and simple enough not to be noticed at compile time. Thanks for
debugging this. Feel free to add my:
Acked-by: Florian Fainelli <florian@openwrt.org>
--
Florian
^ permalink raw reply
* Re: [PATCH] flexcan: fix NAPI for bus errors
From: Marc Kleine-Budde @ 2010-11-23 15:39 UTC (permalink / raw)
To: John Ogness
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
Wolfgang Grandegger
In-Reply-To: <80k4k45csy.fsf-l77OnrVvfFAyMciVaGeJ0d53zsg1cpMQ@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 3280 bytes --]
Hello,
On 11/23/2010 03:34 PM, John Ogness wrote:
> If bus error reporting is disabled and bus errors occur, the flexcan
> driver will hog the system because it continually re-enables the IRQs
> (work_done = 0). This patch changes the driver to only re-enable the
> IRQs if some work was actually done. This allows the features of NAPI to
> be used for bus errors as well (when bus error reporting is disabled).
Good idea, however the chip has IMHO a bug:
The problem with the error interrupt is, when disabled the can core
doesn't issue any can bus warning or bus passive interrupts.
Can you ensure that you get both error warning and error passive can
error messages with disabled BERR and you patch applied?
> This patch is against Linux next-20101123.
>
> Signed-off-by: John Ogness <john.ogness-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> ---
> drivers/net/can/flexcan.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> --- next-20101123-a/drivers/net/can/flexcan.c
> +++ next-20101123-b/drivers/net/can/flexcan.c
> @@ -535,9 +535,12 @@ static int flexcan_poll(struct napi_stru
>
> if (work_done < quota) {
> napi_complete(napi);
> - /* enable IRQs */
> - writel(FLEXCAN_IFLAG_DEFAULT, ®s->imask1);
> - writel(priv->reg_ctrl_default, ®s->ctrl);
> +
> + if (work_done > 0) {
> + /* enable IRQs */
> + writel(FLEXCAN_IFLAG_DEFAULT, ®s->imask1);
> + writel(priv->reg_ctrl_default, ®s->ctrl);
> + }
> }
>
> return work_done;
If a bus error occurs and bus error reporting is disabled, work_done
will stay 0, so both the RX and the ERR interrupt stay disabled (which
is done in flexcan_irq). I think we should always enable the RX
interrupt. (However, if the bus is working again the chip can probably
send messages again, so we get a TX-complete IRQ, but this depends on
the application.)
Having a look at flexcan_irq:
> /*
> * schedule NAPI in case of:
> * - rx IRQ
> * - state change IRQ
> * - bus error IRQ and bus error reporting is activated
> */
This mean with disabled BERR NAPI should only be scheduled in case of a
RX or a state change interrupt. Both of these interrupts should generate
a can_frame in flexcan_poll and this the work_done shoud be > 0, modulo
out of memory situations.
> if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
> (reg_esr & FLEXCAN_ESR_ERR_STATE) ||
> flexcan_has_and_handle_berr(priv, reg_esr)) {
> /*
> * The error bits are cleared on read,
> * save them for later use.
> */
> priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
> writel(FLEXCAN_IFLAG_DEFAULT & ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE,
> ®s->imask1);
> writel(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
> ®s->ctrl);
> napi_schedule(&priv->napi);
> }
So thinking about the problem I don't see how your patch works. But
there might be a bug in the driver or my logic.
Cheers, 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: [PATCH 4/9] AF_UNIX: find the recipients for multicast messages
From: Eric Dumazet @ 2010-11-23 16:08 UTC (permalink / raw)
To: Alban Crequy
Cc: David Miller, shemminger, gorcunov, adobriyan, lennart,
kay.sievers, ian.molton, netdev, linux-kernel
In-Reply-To: <20101123150315.4e67a139@chocolatine.cbg.collabora.co.uk>
Le mardi 23 novembre 2010 à 15:03 +0000, Alban Crequy a écrit :
> Le Mon, 22 Nov 2010 11:05:19 -0800 (PST),
> David Miller <davem@davemloft.net> a écrit :
>
> > From: Alban Crequy <alban.crequy@collabora.co.uk>
> > Date: Mon, 22 Nov 2010 18:36:17 +0000
> >
> > > unix_find_multicast_recipients() builds an array of recipients. It
> > > can either find the peers of a specific multicast address, or find
> > > all the peers of all multicast group the sender is part of.
> > >
> > > Signed-off-by: Alban Crequy <alban.crequy@collabora.co.uk>
> >
> > You really should use RCU to lock this stuff, this way sends run
> > lockless and have less worries wrt. the memory allocation. You'll
> > also only take a spinlock in the write paths which change the
> > multicast groups, which ought to be rare.
>
> I understand the benefit to use RCU in order to have lockless sends.
>
> But with RCU I will still have worries about the memory allocation:
>
> - I cannot allocate inside a rcu_read_lock()-rcu_read_unlock() block.
>
Thats not true.
Sames rules than inside a spin_lock() or write_lock() apply.
We already allocate memory inside rcu_read_lock() in network stack.
> - If I iterate locklessly over the multicast group members with
> hlist_for_each_entry_rcu(), new members can be added, so the
> array can be allocated with the wrong size and I have to try again
> ("goto try_again") when this rare case occurs.
You are allowed to allocate memory to add stuff while doing your loop
iteration.
Nothing prevents you to use a chain of items, each item holding up to
128 sockets for example. If full, allocate a new item.
We have such schem in poll()/select() for example
fs/select.c function poll_get_entry()
Use a small embedded struct on stack, and allocate extra items if number
of fd is too big.
(If you cant allocate memory to hold pointers, chance is you wont be
able to clone skbs anyway. One skb is about 400 bytes.)
If new members are added to the group while you are iterating the list,
they wont receive a copy of the message.
Or just chain skbs while you clone them, store in skb->sk the socket...
no need for extra memory allocations.
>
> - Another idea would be to avoid completely the allocation by inlining
> unix_find_multicast_recipients() inside unix_dgram_sendmsg() and
> delivering the messages to the recipients as long as the list is
> being iterated locklessly. But I want to provide atomicity of
> delivery: the message must be delivered with skb_queue_tail() either
> to all the recipients or to none of them in case of interruption or
> memory pressure. I don't see how I can achieve that without
> iterating several times on the list of recipients, hence the
> allocation and the copy in the array. I also want to guarantee the
> order of delivery as described in multicast-unix-sockets.txt and for
> this, I am taking lots of spinlocks anyway. I don't see how to avoid
> that, but I would be happy to be wrong and have a better solution.
>
So if one destination has a full receive queue, you want nobody receive
the message ? That seems a bit risky to me, if someone sends SIGSTOP to
one of your process...
>
> To give an idea of the number of members in a multicast group for the
> D-Bus use case, I have 90 D-Bus connections on my session bus:
>
> $ dbus-send --print-reply --dest=org.freedesktop.DBus \
> /org/freedesktop/DBus org.freedesktop.DBus.ListNames | grep '":'|wc -l
> 90
>
> In common cases, there should be only a few real recipients (1 or 2?)
> after the socket filters eliminate most of them, but
> unix_find_multicast_recipients() will still allocate an array of
> about that size.
>
I am not sure if doing 90 clones of skb and filtering them one by one is
going to be fast :-(
^ permalink raw reply
* [PATCH net-next] bnx2x: Resolving a possible dead-lock situation
From: Vladislav Zolotarov @ 2010-11-23 16:15 UTC (permalink / raw)
To: Dave Miller; +Cc: Eilon Greenstein, netdev list
There is a possible dead-lock situation between sch_direct_xmit()
(called from soft_IRQ context) and bnx2x_tx_int() when called from
an ethtool self-test flow (syscall context).
To prevent a dead-lock, disable bottom-halves on a local CPU when taking
a tx_lock from bnx2x_tx_int() (use __netif_tx_lock_bh(txq)).
The flow in the bnx2x_tx_int() where tx_lock is taken should be hit
very rarely thus performance penalty of this change should be minimal.
Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/bnx2x/bnx2x_cmn.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
index 94d5f59..5189788 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/bnx2x/bnx2x_cmn.c
@@ -144,14 +144,14 @@ int bnx2x_tx_int(struct bnx2x_fastpath *fp)
* stops the queue
*/
- __netif_tx_lock(txq, smp_processor_id());
+ __netif_tx_lock_bh(txq);
if ((netif_tx_queue_stopped(txq)) &&
(bp->state == BNX2X_STATE_OPEN) &&
(bnx2x_tx_avail(fp) >= MAX_SKB_FRAGS + 3))
netif_tx_wake_queue(txq);
- __netif_tx_unlock(txq);
+ __netif_tx_unlock_bh(txq);
}
return 0;
}
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] iproute2: support xfrm upper protocol gre key
From: Stephen Hemminger @ 2010-11-23 16:24 UTC (permalink / raw)
To: Timo Teräs; +Cc: netdev
In-Reply-To: <1290524559-22086-1-git-send-email-timo.teras@iki.fi>
On Tue, 23 Nov 2010 17:02:39 +0200
Timo Teräs <timo.teras@iki.fi> wrote:
> + case IPPROTO_GRE:
> + if (sel->sport_mask || sel->dport_mask) {
> + struct in_addr key;
> + key.s_addr = htonl((ntohs(sel->sport) << 16) + ntohs(sel->dport));
> + inet_ntop(AF_INET, &key, abuf, sizeof(abuf));
> + fprintf(fp, "key %s ", abuf);
> + }
The GRE key is not really an IPv4 address. Why should the utilities
use IPv4 address manipulation to format/scan it. It makes more sense
to me to just use u32 an do the necessary ntohl.
--
^ permalink raw reply
* Re: [PATCH 00/62] drivers/net: Use static const
From: David Miller @ 2010-11-23 16:28 UTC (permalink / raw)
To: joe-6d6DIl74uiNBDgjK7y7TUQ
Cc: ath5k-devel-xDcbHBWguxEUs3QNXV6qNA,
libertas-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
users-poMEt7QlJxcwIE2E9O76wjtx2kNaKg5H,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
ath9k-devel-xDcbHBWguxHbcTqmT+pZeQ
In-Reply-To: <1290465605.27683.52.camel@Joe-Laptop>
From: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
Date: Mon, 22 Nov 2010 14:40:05 -0800
> I'll see who picks up or acks what and send a pull request to
> you for the remainder of these patches mid December or so.
Thanks.
> Do you want micro patches or a single patch?
You can group them by directory or similar if you like.
F.e.:
Patch 1: drivers/net/*.c
Patch 2: drivers/net/*/*.c (wired drivers)
Patch 3: drivers/net/wireless/*
Thanks again.
^ permalink raw reply
* [PATCH 1/2] SELinux: Only return netlink error when we know the return is fatal
From: Eric Paris @ 2010-11-23 16:28 UTC (permalink / raw)
To: netdev, selinux; +Cc: sds, paul.moore, davem
Some of the SELinux netlink code returns a fatal error when the error might
actually be transient. This patch just silently drops packets on
potentially transient errors but continues to return a permanant error
indicator when the denial was because of policy.
Based-on-comments-by: Paul Moore <paul.moore@hp.com>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
security/selinux/hooks.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a0bc5c0..bd6dc16 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4594,7 +4594,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
}
if (secmark_perm == PACKET__FORWARD_OUT) {
if (selinux_skb_peerlbl_sid(skb, family, &peer_sid))
- return NF_DROP_ERR(-ECONNREFUSED);
+ return NF_DROP;
} else
peer_sid = SECINITSID_KERNEL;
} else {
@@ -4607,7 +4607,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
ad.u.net.netif = ifindex;
ad.u.net.family = family;
if (selinux_parse_skb(skb, &ad, &addrp, 0, NULL))
- return NF_DROP_ERR(-ECONNREFUSED);
+ return NF_DROP;
if (secmark_active)
if (avc_has_perm(peer_sid, skb->secmark,
@@ -4619,13 +4619,13 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
u32 node_sid;
if (sel_netif_sid(ifindex, &if_sid))
- return NF_DROP_ERR(-ECONNREFUSED);
+ return NF_DROP;
if (avc_has_perm(peer_sid, if_sid,
SECCLASS_NETIF, NETIF__EGRESS, &ad))
return NF_DROP_ERR(-ECONNREFUSED);
if (sel_netnode_sid(addrp, family, &node_sid))
- return NF_DROP_ERR(-ECONNREFUSED);
+ return NF_DROP;
if (avc_has_perm(peer_sid, node_sid,
SECCLASS_NODE, NODE__SENDTO, &ad))
return NF_DROP_ERR(-ECONNREFUSED);
^ permalink raw reply related
* [PATCH 2/2] SELinux: indicate fatal error in compat netfilter code
From: Eric Paris @ 2010-11-23 16:28 UTC (permalink / raw)
To: netdev, selinux; +Cc: sds, paul.moore, davem
In-Reply-To: <20101123162802.3588.74894.stgit@paris.rdu.redhat.com>
The SELinux ip postroute code indicates when policy rejected a packet and
passes the error back up the stack. The compat code does not. This patch
sends the same kind of error back up the stack in the compat code.
Based-on-patch-by: Paul Moore <paul.moore@hp.com>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
security/selinux/hooks.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index bd6dc16..dd1690f 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4529,11 +4529,11 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
if (selinux_secmark_enabled())
if (avc_has_perm(sksec->sid, skb->secmark,
SECCLASS_PACKET, PACKET__SEND, &ad))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
if (selinux_policycap_netpeer)
if (selinux_xfrm_postroute_last(sksec->sid, skb, &ad, proto))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
return NF_ACCEPT;
}
^ permalink raw reply related
* Re: [PATCH net-next] bnx2x: Resolving a possible dead-lock situation
From: Eric Dumazet @ 2010-11-23 16:29 UTC (permalink / raw)
To: Vladislav Zolotarov; +Cc: Dave Miller, Eilon Greenstein, netdev list
In-Reply-To: <1290528912.15453.3.camel@lb-tlvb-vladz>
Le mardi 23 novembre 2010 à 18:15 +0200, Vladislav Zolotarov a écrit :
> There is a possible dead-lock situation between sch_direct_xmit()
> (called from soft_IRQ context) and bnx2x_tx_int() when called from
> an ethtool self-test flow (syscall context).
>
> To prevent a dead-lock, disable bottom-halves on a local CPU when taking
> a tx_lock from bnx2x_tx_int() (use __netif_tx_lock_bh(txq)).
>
> The flow in the bnx2x_tx_int() where tx_lock is taken should be hit
> very rarely thus performance penalty of this change should be minimal.
>
> Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> ---
> drivers/net/bnx2x/bnx2x_cmn.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
> index 94d5f59..5189788 100644
> --- a/drivers/net/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/bnx2x/bnx2x_cmn.c
> @@ -144,14 +144,14 @@ int bnx2x_tx_int(struct bnx2x_fastpath *fp)
> * stops the queue
> */
>
> - __netif_tx_lock(txq, smp_processor_id());
> + __netif_tx_lock_bh(txq);
>
> if ((netif_tx_queue_stopped(txq)) &&
> (bp->state == BNX2X_STATE_OPEN) &&
> (bnx2x_tx_avail(fp) >= MAX_SKB_FRAGS + 3))
> netif_tx_wake_queue(txq);
>
> - __netif_tx_unlock(txq);
> + __netif_tx_unlock_bh(txq);
> }
> return 0;
> }
That seems strange. Even if performance penalty is not minimal, it
should be avoided.
If problem comes from ethtool, why not preventing BH in ethtool itself ?
diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
index d02ffbd..1f7d4e6 100644
--- a/drivers/net/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/bnx2x/bnx2x_ethtool.c
@@ -1499,9 +1499,11 @@ static int bnx2x_run_loopback(struct bnx2x *bp, int loopback_mode, u8 link_up)
* updates that have been performed while interrupts were
* disabled.
*/
- if (bp->common.int_block == INT_BLOCK_IGU)
+ if (bp->common.int_block == INT_BLOCK_IGU) {
+ local_bh_disable();
bnx2x_tx_int(fp_tx);
-
+ local_bh_enable();
+ }
rx_idx = le16_to_cpu(*fp_rx->rx_cons_sb);
if (rx_idx != rx_start_idx + num_pkts)
goto test_loopback_exit;
^ permalink raw reply related
* Re: [PATCH 2/2] SELinux: indicate fatal error in compat netfilter code
From: Paul Moore @ 2010-11-23 16:32 UTC (permalink / raw)
To: Eric Paris; +Cc: netdev, selinux, sds, davem
In-Reply-To: <20101123162808.3588.18495.stgit@paris.rdu.redhat.com>
On Tue, 2010-11-23 at 11:28 -0500, Eric Paris wrote:
> The SELinux ip postroute code indicates when policy rejected a packet and
> passes the error back up the stack. The compat code does not. This patch
> sends the same kind of error back up the stack in the compat code.
>
> Based-on-patch-by: Paul Moore <paul.moore@hp.com>
> Signed-off-by: Eric Paris <eparis@redhat.com>
Reviewed-by: Paul Moore <paul.moore@hp.com>
> ---
>
> security/selinux/hooks.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index bd6dc16..dd1690f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4529,11 +4529,11 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
> if (selinux_secmark_enabled())
> if (avc_has_perm(sksec->sid, skb->secmark,
> SECCLASS_PACKET, PACKET__SEND, &ad))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
>
> if (selinux_policycap_netpeer)
> if (selinux_xfrm_postroute_last(sksec->sid, skb, &ad, proto))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
>
> return NF_ACCEPT;
> }
>
--
paul moore
linux @ hp
^ permalink raw reply
* Re: [PATCH 1/2] SELinux: Only return netlink error when we know the return is fatal
From: Paul Moore @ 2010-11-23 16:32 UTC (permalink / raw)
To: Eric Paris; +Cc: netdev, selinux, sds, davem
In-Reply-To: <20101123162802.3588.74894.stgit@paris.rdu.redhat.com>
On Tue, 2010-11-23 at 11:28 -0500, Eric Paris wrote:
> Some of the SELinux netlink code returns a fatal error when the error might
> actually be transient. This patch just silently drops packets on
> potentially transient errors but continues to return a permanant error
> indicator when the denial was because of policy.
>
> Based-on-comments-by: Paul Moore <paul.moore@hp.com>
> Signed-off-by: Eric Paris <eparis@redhat.com>
Thanks for fixing this up.
Reviewed-by: Paul Moore <paul.moore@hp.com>
> ---
>
> security/selinux/hooks.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index a0bc5c0..bd6dc16 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4594,7 +4594,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
> }
> if (secmark_perm == PACKET__FORWARD_OUT) {
> if (selinux_skb_peerlbl_sid(skb, family, &peer_sid))
> - return NF_DROP_ERR(-ECONNREFUSED);
> + return NF_DROP;
> } else
> peer_sid = SECINITSID_KERNEL;
> } else {
> @@ -4607,7 +4607,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
> ad.u.net.netif = ifindex;
> ad.u.net.family = family;
> if (selinux_parse_skb(skb, &ad, &addrp, 0, NULL))
> - return NF_DROP_ERR(-ECONNREFUSED);
> + return NF_DROP;
>
> if (secmark_active)
> if (avc_has_perm(peer_sid, skb->secmark,
> @@ -4619,13 +4619,13 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
> u32 node_sid;
>
> if (sel_netif_sid(ifindex, &if_sid))
> - return NF_DROP_ERR(-ECONNREFUSED);
> + return NF_DROP;
> if (avc_has_perm(peer_sid, if_sid,
> SECCLASS_NETIF, NETIF__EGRESS, &ad))
> return NF_DROP_ERR(-ECONNREFUSED);
>
> if (sel_netnode_sid(addrp, family, &node_sid))
> - return NF_DROP_ERR(-ECONNREFUSED);
> + return NF_DROP;
> if (avc_has_perm(peer_sid, node_sid,
> SECCLASS_NODE, NODE__SENDTO, &ad))
> return NF_DROP_ERR(-ECONNREFUSED);
>
--
paul moore
linux @ hp
^ permalink raw reply
* RE: [PATCH net-next] bnx2x: Resolving a possible dead-lock situation
From: Vladislav Zolotarov @ 2010-11-23 16:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Dave Miller, Eilon Greenstein, netdev list
In-Reply-To: <1290529777.3046.99.camel@edumazet-laptop>
>
> That seems strange. Even if performance penalty is not minimal, it
> should be avoided.
>
> If problem comes from ethtool, why not preventing BH in ethtool itself
> ?
Looks good. Let me run a few checks before u submit a patch to Dave. Or do u prefer
me to push it on your behalf?
Thanks,
vlad
>
> diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c
> b/drivers/net/bnx2x/bnx2x_ethtool.c
> index d02ffbd..1f7d4e6 100644
> --- a/drivers/net/bnx2x/bnx2x_ethtool.c
> +++ b/drivers/net/bnx2x/bnx2x_ethtool.c
> @@ -1499,9 +1499,11 @@ static int bnx2x_run_loopback(struct bnx2x *bp,
> int loopback_mode, u8 link_up)
> * updates that have been performed while interrupts were
> * disabled.
> */
> - if (bp->common.int_block == INT_BLOCK_IGU)
> + if (bp->common.int_block == INT_BLOCK_IGU) {
> + local_bh_disable();
> bnx2x_tx_int(fp_tx);
> -
> + local_bh_enable();
> + }
> rx_idx = le16_to_cpu(*fp_rx->rx_cons_sb);
> if (rx_idx != rx_start_idx + num_pkts)
> goto test_loopback_exit;
>
>
^ permalink raw reply
* [PATCH] au1000_eth: fix invalid address accessing the MAC enable register
From: Wolfgang Grandegger @ 2010-11-23 16:40 UTC (permalink / raw)
To: Netdev; +Cc: Linux-MIPS, Florian Fainelli
"aup->enable" holds already the address pointing to the MAC enable
register. The bug was introduced by commit d0e7cb:
"au1000-eth: remove volatiles, switch to I/O accessors".
CC: Florian Fainelli <florian@openwrt.org>
Signed-off-by: Wolfgang Grandegger <wg@denx.de>
Acked-by: Florian Fainelli <florian@openwrt.org>
---
drivers/net/au1000_eth.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
index 43489f8..53eff9b 100644
--- a/drivers/net/au1000_eth.c
+++ b/drivers/net/au1000_eth.c
@@ -155,10 +155,10 @@ static void au1000_enable_mac(struct net_device *dev, int force_reset)
spin_lock_irqsave(&aup->lock, flags);
if (force_reset || (!aup->mac_enabled)) {
- writel(MAC_EN_CLOCK_ENABLE, &aup->enable);
+ writel(MAC_EN_CLOCK_ENABLE, aup->enable);
au_sync_delay(2);
writel((MAC_EN_RESET0 | MAC_EN_RESET1 | MAC_EN_RESET2
- | MAC_EN_CLOCK_ENABLE), &aup->enable);
+ | MAC_EN_CLOCK_ENABLE), aup->enable);
au_sync_delay(2);
aup->mac_enabled = 1;
@@ -503,9 +503,9 @@ static void au1000_reset_mac_unlocked(struct net_device *dev)
au1000_hard_stop(dev);
- writel(MAC_EN_CLOCK_ENABLE, &aup->enable);
+ writel(MAC_EN_CLOCK_ENABLE, aup->enable);
au_sync_delay(2);
- writel(0, &aup->enable);
+ writel(0, aup->enable);
au_sync_delay(2);
aup->tx_full = 0;
@@ -1119,7 +1119,7 @@ static int __devinit au1000_probe(struct platform_device *pdev)
/* set a random MAC now in case platform_data doesn't provide one */
random_ether_addr(dev->dev_addr);
- writel(0, &aup->enable);
+ writel(0, aup->enable);
aup->mac_enabled = 0;
pd = pdev->dev.platform_data;
--
1.7.2.3
^ permalink raw reply related
* Re: [PATCH] iproute2: support xfrm upper protocol gre key
From: Timo Teräs @ 2010-11-23 16:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20101123082404.5b8ad019@nehalam>
On 11/23/2010 06:24 PM, Stephen Hemminger wrote:
> On Tue, 23 Nov 2010 17:02:39 +0200
> Timo Teräs <timo.teras@iki.fi> wrote:
>
>> + case IPPROTO_GRE:
>> + if (sel->sport_mask || sel->dport_mask) {
>> + struct in_addr key;
>> + key.s_addr = htonl((ntohs(sel->sport) << 16) + ntohs(sel->dport));
>> + inet_ntop(AF_INET, &key, abuf, sizeof(abuf));
>> + fprintf(fp, "key %s ", abuf);
>> + }
>
> The GRE key is not really an IPv4 address. Why should the utilities
> use IPv4 address manipulation to format/scan it. It makes more sense
> to me to just use u32 an do the necessary ntohl.
This is pretty much how iptunnel.c does it, so I copied the code. Would
you prefer to format it as single u32 number? Or use something else for
formatting it similar to IPv4?
In either case, we should change iptunnel.c to match ipxfrm.c. It'll be
easier if both parts handling the gre key treat it equivalently.
I think Cisco does indeed treat it as u32 number in the configurations.
So I'm okay updating this patch, and fixing iptunnel.c side too. We
might still want to keep the parsing of ipv4 format to keep backwards
compatibility.
^ 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