* Re: [PATCH] sysfs: Don't allow the creation of symlinks we can't remove
From: Johannes Berg @ 2010-07-22 11:30 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Greg KH, Andrew Morton, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Greg KH, netdev
In-Reply-To: <m1sk3bycxl.fsf@fess.ebiederm.org>
On Thu, 2010-07-22 at 04:27 -0700, Eric W. Biederman wrote:
> >> Do we have a convenient command line tool to do this?
> >> I remember there being a different netlink message from
> >> normal network devices.
> >
> > iw phy0 set netns <pid>
> >
> > http://git.sipsolutions.net/iw.git
> >
> >> > root@kvm:~# ip link
> >> > 3: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
> >> > link/ether 02:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
> >> > 7: lo: <LOOPBACK> mtu 16436 qdisc noop state DOWN
> >> > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> >> > root@kvm:~# ls /sys/class/net/
> >> > eth0 hwsim0 lo wlan1 wlan2
> >>
> >> I think this is actually the output of something working.
> >>
> >> I expect after you created a new netns you didn't mount
> >> a new instance of /sys. /sys remembers which netns you
> >> had when you mounted it. So you have to mount /sys again
> >> so you can see the /sys/class/net for the network namespace
> >> you are in.
> >
> > Ohh, oops! I saw all the "current->" references in the code and somehow
> > expected the same instance of sysfs to show the right thing.
> >
> > Yes, it works now. But the patch below doesn't seem to work, am I
> > missing something?
>
> You are trying to move the phy devices as well?
Yes. The intent is that each wireless phy lives in a netns along with
all of its child devices.
> My guess is that at least part of the problem is that you don't have a
> ieee80211 directory under hwsim.
But I should have? 'ieee80211' is a class just like 'net', no?
> My apologies for not thinking about the peculiarities of the wireless
> drivers.
No worries.
johannes
^ permalink raw reply
* Re: [PATCH] sysfs: Don't allow the creation of symlinks we can't remove
From: Eric W. Biederman @ 2010-07-22 11:27 UTC (permalink / raw)
To: Johannes Berg
Cc: Greg KH, Andrew Morton, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Greg KH, netdev
In-Reply-To: <1279795286.12439.8.camel@jlt3.sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> On Thu, 2010-07-22 at 03:35 -0700, Eric W. Biederman wrote:
>
>> >> The warning patch just makes things fail faster. Although I get some of the
>> >> wireless interfaces for hwsim when I use this one.
>> >
>> > Hmm, I didn't.
>>
>> To be clear I just get hwsim0. Not wlan0 or wlan1.
>
> Ah, yes, but that's just a regular netdev, you can pretty much ignore
> it. It just shows all hwsim traffic as it is on the "air" for sniffing.
>
>> > Right, it actually starts working again with that patch you sent.
>> > However, netns support is really broken:
>> >
>> > <create net namespace, put phy0/wlan0 into it>
>>
>> Do we have a convenient command line tool to do this?
>> I remember there being a different netlink message from
>> normal network devices.
>
> iw phy0 set netns <pid>
>
> http://git.sipsolutions.net/iw.git
>
>> > root@kvm:~# ip link
>> > 3: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
>> > link/ether 02:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
>> > 7: lo: <LOOPBACK> mtu 16436 qdisc noop state DOWN
>> > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> > root@kvm:~# ls /sys/class/net/
>> > eth0 hwsim0 lo wlan1 wlan2
>>
>> I think this is actually the output of something working.
>>
>> I expect after you created a new netns you didn't mount
>> a new instance of /sys. /sys remembers which netns you
>> had when you mounted it. So you have to mount /sys again
>> so you can see the /sys/class/net for the network namespace
>> you are in.
>
> Ohh, oops! I saw all the "current->" references in the code and somehow
> expected the same instance of sysfs to show the right thing.
>
> Yes, it works now. But the patch below doesn't seem to work, am I
> missing something?
You are trying to move the phy devices as well?
My guess is that at least part of the problem is that you don't have a
ieee80211 directory under hwsim.
My apologies for not thinking about the peculiarities of the wireless
drivers.
Eric
> ---
> include/linux/netdevice.h | 2 ++
> net/core/net-sysfs.c | 3 ++-
> net/wireless/sysfs.c | 9 +++++++++
> 3 files changed, 13 insertions(+), 1 deletion(-)
>
> --- wireless-testing.orig/include/linux/netdevice.h 2010-07-22 10:01:22.000000000 +0200
> +++ wireless-testing/include/linux/netdevice.h 2010-07-22 10:11:00.000000000 +0200
> @@ -2148,6 +2148,8 @@ extern void dev_seq_stop(struct seq_file
> extern int netdev_class_create_file(struct class_attribute *class_attr);
> extern void netdev_class_remove_file(struct class_attribute *class_attr);
>
> +extern struct kobj_ns_type_operations net_ns_type_operations;
> +
> extern char *netdev_drivername(const struct net_device *dev, char *buffer, int len);
>
> extern void linkwatch_run_queue(void);
> --- wireless-testing.orig/net/core/net-sysfs.c 2010-07-22 10:01:22.000000000 +0200
> +++ wireless-testing/net/core/net-sysfs.c 2010-07-22 10:11:51.000000000 +0200
> @@ -785,12 +785,13 @@ static const void *net_netlink_ns(struct
> return sock_net(sk);
> }
>
> -static struct kobj_ns_type_operations net_ns_type_operations = {
> +struct kobj_ns_type_operations net_ns_type_operations = {
> .type = KOBJ_NS_TYPE_NET,
> .current_ns = net_current_ns,
> .netlink_ns = net_netlink_ns,
> .initial_ns = net_initial_ns,
> };
> +EXPORT_SYMBOL_GPL(net_ns_type_operations);
>
> static void net_kobj_ns_exit(struct net *net)
> {
> --- wireless-testing.orig/net/wireless/sysfs.c 2010-07-22 10:01:22.000000000 +0200
> +++ wireless-testing/net/wireless/sysfs.c 2010-07-22 10:13:08.000000000 +0200
> @@ -110,6 +110,13 @@ static int wiphy_resume(struct device *d
> return ret;
> }
>
> +static const void *wiphy_namespace(struct device *d)
> +{
> + struct wiphy *wiphy = container_of(d, struct wiphy, dev);
> +
> + return wiphy_net(wiphy);
> +}
> +
> struct class ieee80211_class = {
> .name = "ieee80211",
> .owner = THIS_MODULE,
> @@ -120,6 +127,8 @@ struct class ieee80211_class = {
> #endif
> .suspend = wiphy_suspend,
> .resume = wiphy_resume,
> + .ns_type = &net_ns_type_operations,
> + .namespace = wiphy_namespace,
> };
>
> int wiphy_sysfs_init(void)
^ permalink raw reply
* [patch -next] stmmac: handle allocation errors in setup functions
From: Dan Carpenter @ 2010-07-22 11:16 UTC (permalink / raw)
To: Giuseppe Cavallaro; +Cc: David S. Miller, netdev, kernel-janitors
If the allocations fail in either dwmac1000_setup() or dwmac100_setup()
then return NULL. These are called from stmmac_mac_device_setup(). The
check for NULL returns in stmmac_mac_device_setup() needed to be moved
forward a couple lines.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/net/stmmac/dwmac1000_core.c b/drivers/net/stmmac/dwmac1000_core.c
index 917b4e1..2b2f5c8 100644
--- a/drivers/net/stmmac/dwmac1000_core.c
+++ b/drivers/net/stmmac/dwmac1000_core.c
@@ -220,6 +220,8 @@ struct mac_device_info *dwmac1000_setup(unsigned long ioaddr)
((uid & 0x0000ff00) >> 8), (uid & 0x000000ff));
mac = kzalloc(sizeof(const struct mac_device_info), GFP_KERNEL);
+ if (!mac)
+ return NULL;
mac->mac = &dwmac1000_ops;
mac->dma = &dwmac1000_dma_ops;
diff --git a/drivers/net/stmmac/dwmac100_core.c b/drivers/net/stmmac/dwmac100_core.c
index 6f270a0..2fb165f 100644
--- a/drivers/net/stmmac/dwmac100_core.c
+++ b/drivers/net/stmmac/dwmac100_core.c
@@ -179,6 +179,8 @@ struct mac_device_info *dwmac100_setup(unsigned long ioaddr)
struct mac_device_info *mac;
mac = kzalloc(sizeof(const struct mac_device_info), GFP_KERNEL);
+ if (!mac)
+ return NULL;
pr_info("\tDWMAC100\n");
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index acf0616..0bdd332 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -1558,15 +1558,15 @@ static int stmmac_mac_device_setup(struct net_device *dev)
else
device = dwmac100_setup(ioaddr);
+ if (!device)
+ return -ENOMEM;
+
if (priv->enh_desc) {
device->desc = &enh_desc_ops;
pr_info("\tEnhanced descriptor structure\n");
} else
device->desc = &ndesc_ops;
- if (!device)
- return -ENOMEM;
-
priv->hw = device;
priv->wolenabled = priv->hw->pmt; /* PMT supported */
^ permalink raw reply related
* [patch -next] caif: precedence bug
From: Dan Carpenter @ 2010-07-22 11:11 UTC (permalink / raw)
To: SjurBraendeland
Cc: David S. Miller, Sjur Braendeland, netdev, kernel-janitors
Negate has precedence over comparison so the original assert only
checked that "rfml->fragment_size" was larger than 1 or 0.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/net/caif/cfrfml.c b/net/caif/cfrfml.c
index 4b04d25..eb16020 100644
--- a/net/caif/cfrfml.c
+++ b/net/caif/cfrfml.c
@@ -193,7 +193,7 @@ out:
static int cfrfml_transmit_segment(struct cfrfml *rfml, struct cfpkt *pkt)
{
- caif_assert(!cfpkt_getlen(pkt) < rfml->fragment_size);
+ caif_assert(cfpkt_getlen(pkt) >= rfml->fragment_size);
/* Add info for MUX-layer to route the packet out. */
cfpkt_info(pkt)->channel_id = rfml->serv.layer.id;
^ permalink raw reply related
* Re: [PATCH] sysfs: Don't allow the creation of symlinks we can't remove
From: Johannes Berg @ 2010-07-22 10:41 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Greg KH, Andrew Morton, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Greg KH, netdev
In-Reply-To: <m1mxtjztvy.fsf@fess.ebiederm.org>
On Thu, 2010-07-22 at 03:35 -0700, Eric W. Biederman wrote:
> >> The warning patch just makes things fail faster. Although I get some of the
> >> wireless interfaces for hwsim when I use this one.
> >
> > Hmm, I didn't.
>
> To be clear I just get hwsim0. Not wlan0 or wlan1.
Ah, yes, but that's just a regular netdev, you can pretty much ignore
it. It just shows all hwsim traffic as it is on the "air" for sniffing.
> > Right, it actually starts working again with that patch you sent.
> > However, netns support is really broken:
> >
> > <create net namespace, put phy0/wlan0 into it>
>
> Do we have a convenient command line tool to do this?
> I remember there being a different netlink message from
> normal network devices.
iw phy0 set netns <pid>
http://git.sipsolutions.net/iw.git
> > root@kvm:~# ip link
> > 3: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
> > link/ether 02:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
> > 7: lo: <LOOPBACK> mtu 16436 qdisc noop state DOWN
> > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> > root@kvm:~# ls /sys/class/net/
> > eth0 hwsim0 lo wlan1 wlan2
>
> I think this is actually the output of something working.
>
> I expect after you created a new netns you didn't mount
> a new instance of /sys. /sys remembers which netns you
> had when you mounted it. So you have to mount /sys again
> so you can see the /sys/class/net for the network namespace
> you are in.
Ohh, oops! I saw all the "current->" references in the code and somehow
expected the same instance of sysfs to show the right thing.
Yes, it works now. But the patch below doesn't seem to work, am I
missing something?
johannes
---
include/linux/netdevice.h | 2 ++
net/core/net-sysfs.c | 3 ++-
net/wireless/sysfs.c | 9 +++++++++
3 files changed, 13 insertions(+), 1 deletion(-)
--- wireless-testing.orig/include/linux/netdevice.h 2010-07-22 10:01:22.000000000 +0200
+++ wireless-testing/include/linux/netdevice.h 2010-07-22 10:11:00.000000000 +0200
@@ -2148,6 +2148,8 @@ extern void dev_seq_stop(struct seq_file
extern int netdev_class_create_file(struct class_attribute *class_attr);
extern void netdev_class_remove_file(struct class_attribute *class_attr);
+extern struct kobj_ns_type_operations net_ns_type_operations;
+
extern char *netdev_drivername(const struct net_device *dev, char *buffer, int len);
extern void linkwatch_run_queue(void);
--- wireless-testing.orig/net/core/net-sysfs.c 2010-07-22 10:01:22.000000000 +0200
+++ wireless-testing/net/core/net-sysfs.c 2010-07-22 10:11:51.000000000 +0200
@@ -785,12 +785,13 @@ static const void *net_netlink_ns(struct
return sock_net(sk);
}
-static struct kobj_ns_type_operations net_ns_type_operations = {
+struct kobj_ns_type_operations net_ns_type_operations = {
.type = KOBJ_NS_TYPE_NET,
.current_ns = net_current_ns,
.netlink_ns = net_netlink_ns,
.initial_ns = net_initial_ns,
};
+EXPORT_SYMBOL_GPL(net_ns_type_operations);
static void net_kobj_ns_exit(struct net *net)
{
--- wireless-testing.orig/net/wireless/sysfs.c 2010-07-22 10:01:22.000000000 +0200
+++ wireless-testing/net/wireless/sysfs.c 2010-07-22 10:13:08.000000000 +0200
@@ -110,6 +110,13 @@ static int wiphy_resume(struct device *d
return ret;
}
+static const void *wiphy_namespace(struct device *d)
+{
+ struct wiphy *wiphy = container_of(d, struct wiphy, dev);
+
+ return wiphy_net(wiphy);
+}
+
struct class ieee80211_class = {
.name = "ieee80211",
.owner = THIS_MODULE,
@@ -120,6 +127,8 @@ struct class ieee80211_class = {
#endif
.suspend = wiphy_suspend,
.resume = wiphy_resume,
+ .ns_type = &net_ns_type_operations,
+ .namespace = wiphy_namespace,
};
int wiphy_sysfs_init(void)
^ permalink raw reply
* Re: [PATCH] sysfs: Don't allow the creation of symlinks we can't remove
From: Eric W. Biederman @ 2010-07-22 10:35 UTC (permalink / raw)
To: Johannes Berg
Cc: Greg KH, Andrew Morton, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Greg KH, netdev
In-Reply-To: <1279793435.12439.3.camel@jlt3.sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> On Thu, 2010-07-22 at 03:05 -0700, Eric W. Biederman wrote:
>
>> >> Detect this problem up front and simply don't create symlinks we won't
>> >> be able to remove later. This prevents symlink leakage and fails in
>> >> a much clearer and more understandable way.
>> >
>> > Eric, I was looking into sysfs netns support for wireless, and with this
>> > patch applied I just get the warning and no network interfaces.
>>
>> The warning patch just makes things fail faster. Although I get some of the
>> wireless interfaces for hwsim when I use this one.
>
> Hmm, I didn't.
To be clear I just get hwsim0. Not wlan0 or wlan1.
>> > Was there any patch that was supposed to fix hwsim?
>>
>> - If you have my patches that fix CONFIG_SYSFS_DEPRECATED,
>> you should find everything works there.
>
> But then I was carrying those two patches too.
>
>> As for a proper fix I have just resent my one liner to
>> drives/base/core.c I can't think of a better option right now.
>>
>> For hwsim it is arguable, but the behaviour of sysfs for the
>> bluetooth bnep driver is very clearly a 3 year old regression,
>> and the cause is exactly the same.
>
> Right, it actually starts working again with that patch you sent.
> However, netns support is really broken:
>
> <create net namespace, put phy0/wlan0 into it>
Do we have a convenient command line tool to do this?
I remember there being a different netlink message from
normal network devices.
> root@kvm:~# ip link
> 3: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
> link/ether 02:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
> 7: lo: <LOOPBACK> mtu 16436 qdisc noop state DOWN
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> root@kvm:~# ls /sys/class/net/
> eth0 hwsim0 lo wlan1 wlan2
I think this is actually the output of something working.
I expect after you created a new netns you didn't mount
a new instance of /sys. /sys remembers which netns you
had when you mounted it. So you have to mount /sys again
so you can see the /sys/class/net for the network namespace
you are in.
Eric
^ permalink raw reply
* Re: [PATCH] sysfs: Don't allow the creation of symlinks we can't remove
From: Johannes Berg @ 2010-07-22 10:10 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Greg KH, Andrew Morton, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Greg KH, netdev
In-Reply-To: <m139vb25nz.fsf@fess.ebiederm.org>
On Thu, 2010-07-22 at 03:05 -0700, Eric W. Biederman wrote:
> >> Detect this problem up front and simply don't create symlinks we won't
> >> be able to remove later. This prevents symlink leakage and fails in
> >> a much clearer and more understandable way.
> >
> > Eric, I was looking into sysfs netns support for wireless, and with this
> > patch applied I just get the warning and no network interfaces.
>
> The warning patch just makes things fail faster. Although I get some of the
> wireless interfaces for hwsim when I use this one.
Hmm, I didn't.
> > Was there any patch that was supposed to fix hwsim?
>
> - If you have my patches that fix CONFIG_SYSFS_DEPRECATED,
> you should find everything works there.
But then I was carrying those two patches too.
> As for a proper fix I have just resent my one liner to
> drives/base/core.c I can't think of a better option right now.
>
> For hwsim it is arguable, but the behaviour of sysfs for the
> bluetooth bnep driver is very clearly a 3 year old regression,
> and the cause is exactly the same.
Right, it actually starts working again with that patch you sent.
However, netns support is really broken:
<create net namespace, put phy0/wlan0 into it>
root@kvm:~# ip link
3: wlan0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
link/ether 02:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
7: lo: <LOOPBACK> mtu 16436 qdisc noop state DOWN
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
root@kvm:~# ls /sys/class/net/
eth0 hwsim0 lo wlan1 wlan2
johannes
^ permalink raw reply
* Re: mirred, redirect action vs. dev refcount issue
From: jamal @ 2010-07-22 10:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20100721170024.60cd9ef4@nehalam>
On Wed, 2010-07-21 at 17:00 -0700, Stephen Hemminger wrote:
> On Wed, 21 Jul 2010 16:58:02 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
>
> > Whether the ifindex or the global list + delete scheme is better is a
> > topic for discussion. Since from the user's perspective it is unclear
> > which semantic is less surprising, entries disappearing or suddenly
> > stop working (or start applying to a different device which has taken
> > a previous one's ifindex!).
>
> ifindex is unique (until integer wraps) so that soft reference
> works.
The proper way to do it is via a notifier since we point to the
netdev - and yes it is a little more complex thats why i just
let the admin suffer (IMO) the well deserved consequences[1].
I am in travel mode - but i will do some background thinking and
come up with a good way to resolve it when i get back. Unless you
have a patch you want me to look at.
cheers,
jamal
[1] least element of suprise principle:
Admin adds a rule which says
"you see a packet matching blah incoming on eth0,
do action1 then action2 ... then actionN"
Say action2 is "mirror to ifb0".
And then this same admin goes and rmmods ifb0 - it is easier to
just reject this rmmod operation as we do todau. Maybe we could be
kinder and be more informative and syslog something along the lines of
"rejected to unregister device you rat-bastard because you have a rule
which says we should mirror to ifb0".
Thoughts?
^ permalink raw reply
* Re: Fwd: LVS on local node
From: Changli Gao @ 2010-07-22 10:06 UTC (permalink / raw)
To: Eric Dumazet
Cc: Patrick McHardy, Jan Engelhardt, Franchoze Eric, wensong,
lvs-devel, netdev, netfilter-devel
In-Reply-To: <1279792792.2467.15.camel@edumazet-laptop>
On Thu, Jul 22, 2010 at 5:59 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 22 juillet 2010 à 17:52 +0800, Changli Gao a écrit :
>
>>
>> FYI: the random option is documented in the manual page of iptables.
>>
>> REDIRECT
>> This target is only valid in the nat table, in the PREROUTING and OUT-
>> PUT chains, and user-defined chains which are only called from those
>> chains. It redirects the packet to the machine itself by changing the
>> destination IP to the primary address of the incoming interface
>> (locally-generated packets are mapped to the 127.0.0.1 address).
>>
>> --to-ports port[-port]
>> This specifies a destination port or range of ports to use:
>> without this, the destination port is never altered. This is
>> only valid if the rule also specifies -p tcp or -p udp.
>>
>> --random
>> If option --random is used then port mapping will be randomized
>> (kernel >= 2.6.22).
>>
>>
>
> Note my patch has nothing to do with the man page, its already up2date.
>
> I usually dont read the Fine manuals, do you ?
Yea. And I don't object your patch. so I add FYI. Thanks.
>
> Try :
>
> iptables -t nat -A PREROUTING -p tcp --dport 1234 -j REDIRECT --help
>
> REDIRECT target options:
> --to-ports <port>[-<port>]
> Port (range) to map to.
>
>
> You see [--random] is missing.
>
>
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] sysfs: Don't allow the creation of symlinks we can't remove
From: Eric W. Biederman @ 2010-07-22 10:05 UTC (permalink / raw)
To: Johannes Berg
Cc: Greg KH, Andrew Morton, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Greg KH, netdev
In-Reply-To: <1279792459.12439.0.camel@jlt3.sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
> On Thu, 2010-07-08 at 09:31 -0700, Eric W. Biederman wrote:
>> Recently my tagged sysfs support revealed a flaw in the device core
>> that a few rare drivers are running into such that we don't always put
>> network devices in a class subdirectory named net/.
>>
>> Since we are not creating the class directory the network devices wind
>> up in a non-tagged directory, but the symlinks to the network devices
>> from /sys/class/net are in a tagged directory. All of which works
>> until we go to remove or rename the symlink. When we remove or rename
>> a symlink we look in the namespace of the target of the symlink.
>> Since the target of the symlink is in a non-tagged sysfs directory we
>> don't have a namespace to look in, and we fail to remove the symlink.
>>
>> Detect this problem up front and simply don't create symlinks we won't
>> be able to remove later. This prevents symlink leakage and fails in
>> a much clearer and more understandable way.
>
> Eric, I was looking into sysfs netns support for wireless, and with this
> patch applied I just get the warning and no network interfaces.
The warning patch just makes things fail faster. Although I get some of the
wireless interfaces for hwsim when I use this one.
> Was there any patch that was supposed to fix hwsim?
- If you have my patches that fix CONFIG_SYSFS_DEPRECATED,
you should find everything works there.
As for a proper fix I have just resent my one liner to
drives/base/core.c I can't think of a better option right now.
For hwsim it is arguable, but the behaviour of sysfs for the
bluetooth bnep driver is very clearly a 3 year old regression,
and the cause is exactly the same.
Eric
^ permalink raw reply
* Re: Fwd: LVS on local node
From: Eric Dumazet @ 2010-07-22 9:59 UTC (permalink / raw)
To: Changli Gao
Cc: Patrick McHardy, Jan Engelhardt, Franchoze Eric, wensong,
lvs-devel, netdev, netfilter-devel
In-Reply-To: <AANLkTimLZG_-4wbKy8nrajHVKQvvGtRKEd8qHNoU3swJ@mail.gmail.com>
Le jeudi 22 juillet 2010 à 17:52 +0800, Changli Gao a écrit :
>
> FYI: the random option is documented in the manual page of iptables.
>
> REDIRECT
> This target is only valid in the nat table, in the PREROUTING and OUT-
> PUT chains, and user-defined chains which are only called from those
> chains. It redirects the packet to the machine itself by changing the
> destination IP to the primary address of the incoming interface
> (locally-generated packets are mapped to the 127.0.0.1 address).
>
> --to-ports port[-port]
> This specifies a destination port or range of ports to use:
> without this, the destination port is never altered. This is
> only valid if the rule also specifies -p tcp or -p udp.
>
> --random
> If option --random is used then port mapping will be randomized
> (kernel >= 2.6.22).
>
>
Note my patch has nothing to do with the man page, its already up2date.
I usually dont read the Fine manuals, do you ?
Try :
iptables -t nat -A PREROUTING -p tcp --dport 1234 -j REDIRECT --help
REDIRECT target options:
--to-ports <port>[-<port>]
Port (range) to map to.
You see [--random] is missing.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] sysfs: Don't allow the creation of symlinks we can't remove
From: Johannes Berg @ 2010-07-22 9:54 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Greg KH, Andrew Morton, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Greg KH, netdev
In-Reply-To: <m1630q7x5v.fsf_-_@fess.ebiederm.org>
On Thu, 2010-07-08 at 09:31 -0700, Eric W. Biederman wrote:
> Recently my tagged sysfs support revealed a flaw in the device core
> that a few rare drivers are running into such that we don't always put
> network devices in a class subdirectory named net/.
>
> Since we are not creating the class directory the network devices wind
> up in a non-tagged directory, but the symlinks to the network devices
> from /sys/class/net are in a tagged directory. All of which works
> until we go to remove or rename the symlink. When we remove or rename
> a symlink we look in the namespace of the target of the symlink.
> Since the target of the symlink is in a non-tagged sysfs directory we
> don't have a namespace to look in, and we fail to remove the symlink.
>
> Detect this problem up front and simply don't create symlinks we won't
> be able to remove later. This prevents symlink leakage and fails in
> a much clearer and more understandable way.
Eric, I was looking into sysfs netns support for wireless, and with this
patch applied I just get the warning and no network interfaces.
Was there any patch that was supposed to fix hwsim?
johannes
^ permalink raw reply
* Re: Fwd: LVS on local node
From: Changli Gao @ 2010-07-22 9:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: Patrick McHardy, Jan Engelhardt, Franchoze Eric, wensong,
lvs-devel, netdev, netfilter-devel
In-Reply-To: <1279791964.2467.12.camel@edumazet-laptop>
On Thu, Jul 22, 2010 at 5:46 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 22 juillet 2010 à 17:10 +0800, Changli Gao a écrit :
>
>>
>> I think maybe REDIRECT is enough. If the public port is one of the
>> real ports, you need to append "random" option to iptables target
>> REDIRECT. If not, "REDIRECT --to-ports 1000-1007" is good enough, and
>> the destination port will be selected in the round-robin manner.
>>
>
> Yes, on 2.6.32, no RPS, so undocumented --random option is probably the
> best we can offer. (random option was added in 2.6.22)
>
> iptables -t nat -A PREROUTING -p tcp --dport 1234 -j REDIRECT --random --to-port 1000-1007
>
> Here is a patch to add "random" help to REDIRECT iptables target
>
FYI: the random option is documented in the manual page of iptables.
REDIRECT
This target is only valid in the nat table, in the PREROUTING and OUT-
PUT chains, and user-defined chains which are only called from those
chains. It redirects the packet to the machine itself by changing the
destination IP to the primary address of the incoming interface
(locally-generated packets are mapped to the 127.0.0.1 address).
--to-ports port[-port]
This specifies a destination port or range of ports to use:
without this, the destination port is never altered. This is
only valid if the rule also specifies -p tcp or -p udp.
--random
If option --random is used then port mapping will be randomized
(kernel >= 2.6.22).
--
Regards,
Changli Gao(xiaosuo@gmail.com)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] nf_nat_core: merge the same lines
From: Changli Gao @ 2010-07-22 9:49 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David S. Miller, netfilter-devel, netdev, Changli Gao
nf_nat_core: merge the same lines
proto->unique_tuple() will be called finally, if the previous calls fail. This
patch checks the false condition of (range->flags & IP_NAT_RANGE_PROTO_RANDOM)
instead to avoid duplicate line of code: proto->unique_tuple().
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/ipv4/netfilter/nf_nat_core.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index c7719b2..037a3a6 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -261,14 +261,9 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple,
rcu_read_lock();
proto = __nf_nat_proto_find(orig_tuple->dst.protonum);
- /* Change protocol info to have some randomization */
- if (range->flags & IP_NAT_RANGE_PROTO_RANDOM) {
- proto->unique_tuple(tuple, range, maniptype, ct);
- goto out;
- }
-
/* Only bother mapping if it's not already in range and unique */
- if ((!(range->flags & IP_NAT_RANGE_PROTO_SPECIFIED) ||
+ if (!(range->flags & IP_NAT_RANGE_PROTO_RANDOM) &&
+ (!(range->flags & IP_NAT_RANGE_PROTO_SPECIFIED) ||
proto->in_range(tuple, maniptype, &range->min, &range->max)) &&
!nf_nat_used_tuple(tuple, ct))
goto out;
^ permalink raw reply related
* Re: Fwd: LVS on local node
From: Eric Dumazet @ 2010-07-22 9:46 UTC (permalink / raw)
To: Changli Gao, Patrick McHardy, Jan Engelhardt
Cc: Franchoze Eric, wensong, lvs-devel, netdev, netfilter-devel
In-Reply-To: <AANLkTimneLE2xKg6fie5u3Cvjzcrq4VnK6wMT3pno0JK@mail.gmail.com>
Le jeudi 22 juillet 2010 à 17:10 +0800, Changli Gao a écrit :
>
> I think maybe REDIRECT is enough. If the public port is one of the
> real ports, you need to append "random" option to iptables target
> REDIRECT. If not, "REDIRECT --to-ports 1000-1007" is good enough, and
> the destination port will be selected in the round-robin manner.
>
Yes, on 2.6.32, no RPS, so undocumented --random option is probably the
best we can offer. (random option was added in 2.6.22)
iptables -t nat -A PREROUTING -p tcp --dport 1234 -j REDIRECT --random --to-port 1000-1007
Here is a patch to add "random" help to REDIRECT iptables target
Thanks
[PATCH] extensions: REDIRECT: add random help
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/extensions/libipt_REDIRECT.c b/extensions/libipt_REDIRECT.c
index 3dfcadf..324d0eb 100644
--- a/extensions/libipt_REDIRECT.c
+++ b/extensions/libipt_REDIRECT.c
@@ -17,7 +17,8 @@ static void REDIRECT_help(void)
printf(
"REDIRECT target options:\n"
" --to-ports <port>[-<port>]\n"
-" Port (range) to map to.\n");
+" Port (range) to map to.\n"
+" [--random]\n");
}
static const struct option REDIRECT_opts[] = {
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] CAN: Add Flexcan CAN controller driver
From: Wolfgang Grandegger @ 2010-07-22 9:44 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C481064.8090809-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 07/22/2010 11:33 AM, Marc Kleine-Budde wrote:
> Wolfgang Grandegger wrote:
>> On 07/21/2010 10:42 PM, Marc Kleine-Budde wrote:
>>> Marc Kleine-Budde wrote:
>>>> Wolfgang Grandegger wrote:
>>>>> I realized a few issues. You can add my "acked-by" when they are fixed.
>>>> thanks for the review.
>>> [...]
>>>
>>>>>> +static void flexcan_poll_err_frame(struct net_device *dev,
>>>>>> + struct can_frame *cf, u32 reg_esr)
>>>>>> +{
>>>>>> + struct flexcan_priv *priv = netdev_priv(dev);
>>>>>> + int error_warning = 0, rx_errors = 0, tx_errors = 0;
>>>>>> +
>>>>>> + if (reg_esr & FLEXCAN_ESR_BIT1_ERR) {
>>>>>> + rx_errors = 1;
>>>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>>> + cf->data[2] |= CAN_ERR_PROT_BIT1;
>>>>>> + }
>>>>>> +
>>>>>> + if (reg_esr & FLEXCAN_ESR_BIT0_ERR) {
>>>>>> + rx_errors = 1;
>>>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>>> + cf->data[2] |= CAN_ERR_PROT_BIT0;
>>>>>> + }
>>>>>> +
>>>>>> + if (reg_esr & FLEXCAN_ESR_FRM_ERR) {
>>>>>> + rx_errors = 1;
>>>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>>> + cf->data[2] |= CAN_ERR_PROT_FORM;
>>>>>> + }
>>>>>> +
>>>>>> + if (reg_esr & FLEXCAN_ESR_STF_ERR) {
>>>>>> + rx_errors = 1;
>>>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>>>>>> + }
>>>>>> +
>>>>>> +
>>>>>> + if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
>>>>>> + tx_errors = 1;
>>>>>> + cf->can_id |= CAN_ERR_ACK;
>>>>> This is a bus-error as well. Therefore I think it should be:
>>>>>
>>>>> if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
>>>>> tx_errors = 1;
>>>>> cf->can_id |= CAN_ERR_ACK;
>>>>> cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>> cf->data[3] |= CAN_ERR_PROT_LOC_ACK; /* ACK slot */
>>>>> }
>>>>>
>>>>> I need to check what CAN_ERR_ACK is intended for. Then, cf->can_id could
>>>>> be preset with "CAN_ERR_PROT | CAN_ERR_BUSERROR" at the beginning.
>>> This controller issues an ACK error if there are no other nodes on the
>>> CAN bus to send a ACK that the message has been received. Or all
>>> remaining Nodes are in bus off state.
>>
>> Mainly this bus error can cause the infamous IRQ and message flooding
>> when no cable is connected.
>
> No cable connected can (if your node doesn't have an activated on baord
> termination) result in no termination at all, and this may result in a
> different error.
>
> At least it does on the at91. I haven't checked with the flexcan.
>
> The subtile difference is that the CAN controller isn't allowed to go
> into bus-off with a proper terminated bus when it recevies no ACKs, but
> going to bus off on a not terminated bus is okay.
>
>>> From the datasheet:
>>> "This bit indicates that an acknowledge (ACK) error has been detected by
>>> the transmitter node; that is, a dominant bit has not been detected
>>> during the ACK SLOT."
>>
>> That's what the above error describes, like on the SJA1000, apart from
>> setting CAN_ERR_ACK. Setting CAN_ERR_ACK is somehow bogus, but just
>> leave it for the time being. I will fix it globally when time permits.
>
> Now I'm confused. What's the meaning of CAN_ERR_ACK? When should it be used?
The type of the error is already defined via "CAN_ERR_PROT |
CAN_ERR_BUSERROR". The details of "CAN_ERR_PROT" are described in
data[2..3]. Just for the ACK errors we have CAN_ERR_ACK, but not for the
other bus errors and I ask myself why CAN_ERR_ACK was introduced.
If it does not have another meaning, I tend to remove it (only the AT91
driver actually uses it).
Wolfgang.
^ permalink raw reply
* Re: [PATCH] CAN: Add Flexcan CAN controller driver
From: Marc Kleine-Budde @ 2010-07-22 9:33 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C480E7E.70607-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 3381 bytes --]
Wolfgang Grandegger wrote:
> On 07/21/2010 10:42 PM, Marc Kleine-Budde wrote:
>> Marc Kleine-Budde wrote:
>>> Wolfgang Grandegger wrote:
>>>> I realized a few issues. You can add my "acked-by" when they are fixed.
>>> thanks for the review.
>> [...]
>>
>>>>> +static void flexcan_poll_err_frame(struct net_device *dev,
>>>>> + struct can_frame *cf, u32 reg_esr)
>>>>> +{
>>>>> + struct flexcan_priv *priv = netdev_priv(dev);
>>>>> + int error_warning = 0, rx_errors = 0, tx_errors = 0;
>>>>> +
>>>>> + if (reg_esr & FLEXCAN_ESR_BIT1_ERR) {
>>>>> + rx_errors = 1;
>>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>> + cf->data[2] |= CAN_ERR_PROT_BIT1;
>>>>> + }
>>>>> +
>>>>> + if (reg_esr & FLEXCAN_ESR_BIT0_ERR) {
>>>>> + rx_errors = 1;
>>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>> + cf->data[2] |= CAN_ERR_PROT_BIT0;
>>>>> + }
>>>>> +
>>>>> + if (reg_esr & FLEXCAN_ESR_FRM_ERR) {
>>>>> + rx_errors = 1;
>>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>> + cf->data[2] |= CAN_ERR_PROT_FORM;
>>>>> + }
>>>>> +
>>>>> + if (reg_esr & FLEXCAN_ESR_STF_ERR) {
>>>>> + rx_errors = 1;
>>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>>>>> + }
>>>>> +
>>>>> +
>>>>> + if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
>>>>> + tx_errors = 1;
>>>>> + cf->can_id |= CAN_ERR_ACK;
>>>> This is a bus-error as well. Therefore I think it should be:
>>>>
>>>> if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
>>>> tx_errors = 1;
>>>> cf->can_id |= CAN_ERR_ACK;
>>>> cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>> cf->data[3] |= CAN_ERR_PROT_LOC_ACK; /* ACK slot */
>>>> }
>>>>
>>>> I need to check what CAN_ERR_ACK is intended for. Then, cf->can_id could
>>>> be preset with "CAN_ERR_PROT | CAN_ERR_BUSERROR" at the beginning.
>> This controller issues an ACK error if there are no other nodes on the
>> CAN bus to send a ACK that the message has been received. Or all
>> remaining Nodes are in bus off state.
>
> Mainly this bus error can cause the infamous IRQ and message flooding
> when no cable is connected.
No cable connected can (if your node doesn't have an activated on baord
termination) result in no termination at all, and this may result in a
different error.
At least it does on the at91. I haven't checked with the flexcan.
The subtile difference is that the CAN controller isn't allowed to go
into bus-off with a proper terminated bus when it recevies no ACKs, but
going to bus off on a not terminated bus is okay.
>> From the datasheet:
>> "This bit indicates that an acknowledge (ACK) error has been detected by
>> the transmitter node; that is, a dominant bit has not been detected
>> during the ACK SLOT."
>
> That's what the above error describes, like on the SJA1000, apart from
> setting CAN_ERR_ACK. Setting CAN_ERR_ACK is somehow bogus, but just
> leave it for the time being. I will fix it globally when time permits.
Now I'm confused. What's the meaning of CAN_ERR_ACK? When should it be used?
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: 260 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: macvtap: Limit packet queue length
From: Arnd Bergmann @ 2010-07-22 9:30 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev, Mark Wagner, Chris Wright
In-Reply-To: <20100722074431.GA26744@gondor.apana.org.au>
On Thursday 22 July 2010, Herbert Xu wrote:
> On Thu, Jul 22, 2010 at 02:41:57PM +0800, Herbert Xu wrote:
> > Hi:
> >
> > macvtap: Limit packet queue length
>
> Chris has informed me that he's already tried a similar patch
> and it only makes the problem worse :)
>
> The issue is that the macvtap TX queue length defaults to zero.
>
> So here is an updated patch which addresses this:
Thanks for debugging this and coming up with a solution.
I'm currently travelling, so I can't easily work on it myself.
> Please note that macvtap currently has no way of giving congestion
> notification, that means the software device TX queue cannot be
> used and packets will always be dropped once the macvtap driver
> queue fills up.
This is something I was planning to look into for doing it right,
and then I forgot about it. I'll investigate what could be done
to get proper flow control once I get back to the office.
> Chris Wright noticed that for this patch to work, we need a
> non-zero TX queue length. This patch includes his work to change
> the default macvtap TX queue length to 500.
The only problem I can see with this patch is making it depend on
the *TX* queue length. The point is that unlike tun/tap, the
macvtap network interface's point of view is that this is the
receive queue, not the transmit queue.
In the TX direction, we really don't queue, since we simply forward
to the lowerdev tx queue, so exposing the tunable to user space
as the tx queue length is a little bit awkward, as well as inconsistent
between macvtap and macvlan.
> Reported-by: Mark Wagner <mwagner@redhat.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
As long as we're missing a better solution,
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* Re: [PATCH] CAN: Add Flexcan CAN controller driver
From: Wolfgang Grandegger @ 2010-07-22 9:25 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C475BA6.3030505-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 07/21/2010 10:42 PM, Marc Kleine-Budde wrote:
> Marc Kleine-Budde wrote:
>> Wolfgang Grandegger wrote:
>>> I realized a few issues. You can add my "acked-by" when they are fixed.
>>
>> thanks for the review.
>
> [...]
>
>>>> +static void flexcan_poll_err_frame(struct net_device *dev,
>>>> + struct can_frame *cf, u32 reg_esr)
>>>> +{
>>>> + struct flexcan_priv *priv = netdev_priv(dev);
>>>> + int error_warning = 0, rx_errors = 0, tx_errors = 0;
>>>> +
>>>> + if (reg_esr & FLEXCAN_ESR_BIT1_ERR) {
>>>> + rx_errors = 1;
>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>> + cf->data[2] |= CAN_ERR_PROT_BIT1;
>>>> + }
>>>> +
>>>> + if (reg_esr & FLEXCAN_ESR_BIT0_ERR) {
>>>> + rx_errors = 1;
>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>> + cf->data[2] |= CAN_ERR_PROT_BIT0;
>>>> + }
>>>> +
>>>> + if (reg_esr & FLEXCAN_ESR_FRM_ERR) {
>>>> + rx_errors = 1;
>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>> + cf->data[2] |= CAN_ERR_PROT_FORM;
>>>> + }
>>>> +
>>>> + if (reg_esr & FLEXCAN_ESR_STF_ERR) {
>>>> + rx_errors = 1;
>>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>>>> + }
>>>> +
>>>> +
>>>> + if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
>>>> + tx_errors = 1;
>>>> + cf->can_id |= CAN_ERR_ACK;
>>> This is a bus-error as well. Therefore I think it should be:
>>>
>>> if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
>>> tx_errors = 1;
>>> cf->can_id |= CAN_ERR_ACK;
>>> cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>> cf->data[3] |= CAN_ERR_PROT_LOC_ACK; /* ACK slot */
>>> }
>>>
>>> I need to check what CAN_ERR_ACK is intended for. Then, cf->can_id could
>>> be preset with "CAN_ERR_PROT | CAN_ERR_BUSERROR" at the beginning.
>
> This controller issues an ACK error if there are no other nodes on the
> CAN bus to send a ACK that the message has been received. Or all
> remaining Nodes are in bus off state.
Mainly this bus error can cause the infamous IRQ and message flooding
when no cable is connected.
> From the datasheet:
> "This bit indicates that an acknowledge (ACK) error has been detected by
> the transmitter node; that is, a dominant bit has not been detected
> during the ACK SLOT."
That's what the above error describes, like on the SJA1000, apart from
setting CAN_ERR_ACK. Setting CAN_ERR_ACK is somehow bogus, but just
leave it for the time being. I will fix it globally when time permits.
Wolfgang.
^ permalink raw reply
* [PATCH] Driver-core: Fix bluetooth network device rename regression
From: Eric W. Biederman @ 2010-07-22 9:16 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Morton, Greg KH, Rafael J. Wysocki, Maciej W. Rozycki,
Kay Sievers, Johannes Berg, netdev
In-Reply-To: <m139vd5sms.fsf_-_@fess.ebiederm.org>
With CONFIG_SYSFS_DEPRECATED_V2 enabled I can rename any network device
anything as long as the new name does not conflict with another network
device.
With CONFIG_SYSFS_DEPRECATED_V2 disabled without this fix bluetooth benp
devices, and the mac80211_hwsim driver can not be renamed to any arbitrary
name that happens to conflict with any other name that is used in their
parent devices directory.
The device model usage of the bluetooth bnep driver has not changed since
the current device model sysfs laytout was introduced. Making this a latent
and very annoying regression.
This regression was reported at the time it was introduced and apprently a
few cases were missed by:
commit 864062457a2e444227bd368ca5f2a2b740de604f
Author: Kay Sievers <kay.sievers@vrfy.org>
Date: Wed Mar 14 03:25:56 2007 +0100
driver core: fix namespace issue with devices assigned to classes
- uses a kset in "struct class" to keep track of all directories
belonging to this class
- merges with the /sys/devices/virtual logic.
- removes the namespace-dir if the last member of that class
leaves the directory.
There may be locking or refcounting fixes left, I stopped when it seemed
to work with network and sound modules. :)
From: Kay Sievers <kay.sievers@vrfy.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
That bug fix had a completely undocumented and apparently deliberate omission
where it does not apply in some cases. Those omitted cases cover the
bluetooth network driver.
What makes this regression a serious issue now is the introduction of network
namespace support in sysfs took this from a mild bug to complete driver
non-function.
Several reasons have been put forward not to use this one line bug fix in
the driver core.
- We don't have special cases in the driver core.
This is non-sense we currently have special cases for block devices
scattered all throughout the driver core.
- The driver is at fault.
The bluetooth driver's driver core usage predates the introduction of the
current sysfs layout. It has been 3 years and no one has bothered
to change the driver in all of this time.
If this was really a driver issue that someone cared about and not simply
an academic issue the driver should have been fixed long since.
I offer these reasons to make the change.
- There is not enough information available for sysfs to support rename
or delete of symlinks when only the source directory but not the destination
directory is tagged.
- All of the proposals put forth will change the sysfs layout slightly in
one way or another.
- A network device special case makes sense as network devices are unique
in placing a user choosen name sysfs.
- The driver that are affected are different in sysfs from all other
network devices which likely already consuses any userspace software
that cares about the exact device layout.
- The change is one line that is obviously correct and has no chance
of affecting anything that it is not a network device.
- Real users have hit this problem and reported this bug and those users
deserve drivers that work.
The fix is a trivial change to get_device_parent to always create
the network class directory in any parent of a network device,
Ignoring the incorrect, non-obvious and esoteric rules that the
driver core is trying to impose on the creation of class directories.
Ideally I would remove those crazy special cases get_device_parent for all
devices but in testing it was observed there are other devices such as the
input layer that don't create the class directories today, and applying the
change broadly is likely to break something in userspace and there is no
need to take that risk.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
drivers/base/core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 9630fbd..ffb8443 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -673,7 +673,7 @@ static struct kobject *get_device_parent(struct device *dev,
*/
if (parent == NULL)
parent_kobj = virtual_device_parent(dev);
- else if (parent->class)
+ else if (parent->class && (strcmp(dev->class->name, "net") != 0))
return &parent->kobj;
else
parent_kobj = &parent->kobj;
--
1.6.5.2.143.g8cc62
^ permalink raw reply related
* Re: Fwd: LVS on local node
From: Changli Gao @ 2010-07-22 9:10 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Franchoze Eric, wensong, lvs-devel, netdev, netfilter-devel
In-Reply-To: <1279781811.2405.15.camel@edumazet-laptop>
On Thu, Jul 22, 2010 at 2:56 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> lvs seems not very SMP friendly and a bit complex.
>
> I would use an iptables setup and a slighly modified REDIRECT target
> (and/or a nf_nat_setup_info() change)
>
> Say you have 8 daemons listening on different ports (1000 to 1007)
>
> iptables -t nat -A PREROUTING -p tcp --dport 1234 -j REDIRECT --rxhash-dist --to-port 1000-1007
>
> rxhash would be provided by RPS on recent kernels or locally computed if
> not already provided by core network (or old kernel)
>
> This rule would be triggered only at connection establishment.
> conntracking take care of following packets and is SMP friendly.
>
>
I think maybe REDIRECT is enough. If the public port is one of the
real ports, you need to append "random" option to iptables target
REDIRECT. If not, "REDIRECT --to-ports 1000-1007" is good enough, and
the destination port will be selected in the round-robin manner.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] irq: add tracepoint to softirq_raise
From: Koki Sanagi @ 2010-07-22 8:41 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, linux-kernel, davem, kaneshige.kenji, izumi.taku,
kosaki.motohiro, laijs, scott.a.mcmillan, rostedt, eric.dumazet,
fweisbec, mathieu.desnoyers
In-Reply-To: <20100721111419.GB21259@hmsreliant.think-freely.org>
(2010/07/21 20:14), Neil Horman wrote:
> On Wed, Jul 21, 2010 at 03:57:05PM +0900, Koki Sanagi wrote:
>> (2010/07/20 20:04), Neil Horman wrote:
>>> On Tue, Jul 20, 2010 at 09:45:31AM +0900, Koki Sanagi wrote:
>>>> From: Lai Jiangshan <laijs@cn.fujitsu.com>
>>>>
>>>> Add a tracepoint for tracing when softirq action is raised.
>>>>
>>>> It and the existed tracepoints complete softirq's tracepoints:
>>>> softirq_raise, softirq_entry and softirq_exit.
>>>>
>>>> And when this tracepoint is used in combination with
>>>> the softirq_entry tracepoint we can determine
>>>> the softirq raise latency.
>>>>
>>>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>>>> Acked-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
>>>> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
>>>>
>>>> [ factorize softirq events with DECLARE_EVENT_CLASS ]
>>>> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
>>>> ---
>>>> include/linux/interrupt.h | 8 +++++-
>>>> include/trace/events/irq.h | 57 ++++++++++++++++++++++++++-----------------
>>>> kernel/softirq.c | 4 +-
>>>> 3 files changed, 43 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
>>>> index c233113..1cb5726 100644
>>>> --- a/include/linux/interrupt.h
>>>> +++ b/include/linux/interrupt.h
>>>> @@ -18,6 +18,7 @@
>>>> #include <asm/atomic.h>
>>>> #include <asm/ptrace.h>
>>>> #include <asm/system.h>
>>>> +#include <trace/events/irq.h>
>>>>
>>>> /*
>>>> * These correspond to the IORESOURCE_IRQ_* defines in
>>>> @@ -402,7 +403,12 @@ asmlinkage void do_softirq(void);
>>>> asmlinkage void __do_softirq(void);
>>>> extern void open_softirq(int nr, void (*action)(struct softirq_action *));
>>>> extern void softirq_init(void);
>>>> -#define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
>>>> +static inline void __raise_softirq_irqoff(unsigned int nr)
>>>> +{
>>>> + trace_softirq_raise(nr);
>>>> + or_softirq_pending(1UL << nr);
>>>> +}
>>>> +
>>> We already have tracepoints in irq_enter and irq_exit. If the goal here is to
>>> detect latency during packet processing, cant the delta in time between those
>>> two points be used to determine interrupt handling latency?
>>
>> Certainly, the time between irq_entry and irq_exit is not directly related to
>> latency during packet processing. But it's indirectly related it.
>> Because softirq_entry isn't passed until irq exits and softirq_entry time is
>> related to packet processing latency. So I show it as a reference.
>>
> Its not directly related no, but look at it, the amount of processing between
> irq_exit and softirq_entry is minimal. The information you are trying to
> extract by computing the delta from irq_entry to softirq_entry is almost exactly
> the same as that from irq_entry to irq_exit. For that matter, since you're
> trying to guage lantency for packet processing, I expect you could get the same
> delta by measuring irq_entry to napi_poll tracepoint time, and save the hassle
> of needing to filter on softirq processing that doesn't relate to packet
> processing.
Yeah, to determine interrput latency, we need either one irq_exit or
softirq_entry, not both.
And I think softirq_entry should be left because there is a possibility that
softirq isn't triggered immidiately after irq_exit.
softirq_exit isn't needed because it is not related to packet processing.
softirq_raise is needed because it connects irq_entry and softirq_entry but
there is no need to show it. Currently, my idea is like the following.
irq_entry(+0.000000msec,irq=77:eth3)
|
softirq_entry(+0.003562msec)
|
|---netif_receive_skb(+0.006279msec,len=100)
| |
| skb_copy_datagram_iovec(+0.038778msec, 2285:sshd)
|
napi_poll_exit(+0.017160msec, eth3)
>
>>>
>>>
>>>> extern void raise_softirq_irqoff(unsigned int nr);
>>>> extern void raise_softirq(unsigned int nr);
>>>> extern void wakeup_softirqd(void);
>>>> diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
>>>> index 0e4cfb6..717744c 100644
>>>> --- a/include/trace/events/irq.h
>>>> +++ b/include/trace/events/irq.h
>>>> @@ -5,7 +5,9 @@
>>>> #define _TRACE_IRQ_H
>>>>
>>>> #include <linux/tracepoint.h>
>>>> -#include <linux/interrupt.h>
>>>> +
>>>> +struct irqaction;
>>>> +struct softirq_action;
>>>>
>>>> #define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq }
>>>> #define show_softirq_name(val) \
>>>> @@ -84,56 +86,65 @@ TRACE_EVENT(irq_handler_exit,
>>>>
>>>> DECLARE_EVENT_CLASS(softirq,
>>>>
>>>> - TP_PROTO(struct softirq_action *h, struct softirq_action *vec),
>>>> + TP_PROTO(unsigned int nr),
>>>>
>>>> - TP_ARGS(h, vec),
>>>> + TP_ARGS(nr),
>>>>
>>>> TP_STRUCT__entry(
>>>> - __field( int, vec )
>>>> + __field( unsigned int, vec )
>>>> ),
>>>>
>>>> TP_fast_assign(
>>>> - __entry->vec = (int)(h - vec);
>>>> + __entry->vec = nr;
>>>> ),
>>>>
>>>> TP_printk("vec=%d [action=%s]", __entry->vec,
>>>> - show_softirq_name(__entry->vec))
>>>> + show_softirq_name(__entry->vec))
>>>> +);
>>>> +
>>>> +/**
>>>> + * softirq_raise - called immediately when a softirq is raised
>>>> + * @nr: softirq vector number
>>>> + *
>>>> + * Tracepoint for tracing when softirq action is raised.
>>>> + * Also, when used in combination with the softirq_entry tracepoint
>>>> + * we can determine the softirq raise latency.
>>>> + */
>>>> +DEFINE_EVENT(softirq, softirq_raise,
>>>> +
>>>> + TP_PROTO(unsigned int nr),
>>>> +
>>>> + TP_ARGS(nr)
>>>> );
>>>>
>>>> /**
>>>> * softirq_entry - called immediately before the softirq handler
>>>> - * @h: pointer to struct softirq_action
>>>> - * @vec: pointer to first struct softirq_action in softirq_vec array
>>>> + * @nr: softirq vector number
>>>> *
>>>> - * The @h parameter, contains a pointer to the struct softirq_action
>>>> - * which has a pointer to the action handler that is called. By subtracting
>>>> - * the @vec pointer from the @h pointer, we can determine the softirq
>>>> - * number. Also, when used in combination with the softirq_exit tracepoint
>>>> + * Tracepoint for tracing when softirq action starts.
>>>> + * Also, when used in combination with the softirq_exit tracepoint
>>>> * we can determine the softirq latency.
>>>> */
>>>> DEFINE_EVENT(softirq, softirq_entry,
>>>>
>>>> - TP_PROTO(struct softirq_action *h, struct softirq_action *vec),
>>>> + TP_PROTO(unsigned int nr),
>>>>
>>>> - TP_ARGS(h, vec)
>>>> + TP_ARGS(nr)
>>>> );
>>>>
>>>> /**
>>>> * softirq_exit - called immediately after the softirq handler returns
>>>> - * @h: pointer to struct softirq_action
>>>> - * @vec: pointer to first struct softirq_action in softirq_vec array
>>>> + * @nr: softirq vector number
>>>> *
>>>> - * The @h parameter contains a pointer to the struct softirq_action
>>>> - * that has handled the softirq. By subtracting the @vec pointer from
>>>> - * the @h pointer, we can determine the softirq number. Also, when used in
>>>> - * combination with the softirq_entry tracepoint we can determine the softirq
>>>> - * latency.
>>>> + * Tracepoint for tracing when softirq action ends.
>>>> + * Also, when used in combination with the softirq_entry tracepoint
>>>> + * we can determine the softirq latency.
>>>> */
>>>> DEFINE_EVENT(softirq, softirq_exit,
>>>>
>>>> - TP_PROTO(struct softirq_action *h, struct softirq_action *vec),
>>>> + TP_PROTO(unsigned int nr),
>>>>
>>>> - TP_ARGS(h, vec)
>>>> + TP_ARGS(nr)
>>>> );
>>>>
>>>> #endif /* _TRACE_IRQ_H */
>>>> diff --git a/kernel/softirq.c b/kernel/softirq.c
>>>> index 825e112..6790599 100644
>>>> --- a/kernel/softirq.c
>>>> +++ b/kernel/softirq.c
>>>> @@ -215,9 +215,9 @@ restart:
>>>> int prev_count = preempt_count();
>>>> kstat_incr_softirqs_this_cpu(h - softirq_vec);
>>>>
>>>> - trace_softirq_entry(h, softirq_vec);
>>>> + trace_softirq_entry(h - softirq_vec);
>>>> h->action(h);
>>>> - trace_softirq_exit(h, softirq_vec);
>>>> + trace_softirq_exit(h - softirq_vec);
>>>
>>> You're loosing information here by reducing the numbers of parameters in this
>>> tracepoint. How many other tracepoint scripts rely on having both pointers
>>> handy? Why not just do the pointer math inside your tracehook instead?
>>
>> In __raise_softirq_irqoff macro there is no method to refer softirq_vec, so it
>> can't use softirq DECLARE_EVENT_CLASS as is.
>> Currently, there is no script using softirq_entry or softirq_exit.
>>
> That shouldn't matter, just pass in NULL for softirq_vec in
> __raise_softirq_irqoff as the second argument to the trace function. You may
> need to fix up the class definition so that the assignment or printk doesn't try
> to dereference that pointer when its NULL, but thats easy enough, and it avoids
> breaking any other perf scripts floating out there.
> Neil
>
>> Thanks,
>> Koki Sanagi.
>>
>>>
>>>> if (unlikely(prev_count != preempt_count())) {
>>>> printk(KERN_ERR "huh, entered softirq %td %s %p"
>>>> "with preempt_count %08x,"
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>>
>>
>>
>>
>
>
^ permalink raw reply
* Re: [RFC PATCH v3 4/5] skb: add tracepoints to freeing skb
From: Koki Sanagi @ 2010-07-22 8:39 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, linux-kernel, davem, kaneshige.kenji, izumi.taku,
kosaki.motohiro, laijs, scott.a.mcmillan, rostedt, eric.dumazet,
fweisbec, mathieu.desnoyers
In-Reply-To: <20100721105645.GA21259@hmsreliant.think-freely.org>
(2010/07/21 19:56), Neil Horman wrote:
> On Wed, Jul 21, 2010 at 04:02:57PM +0900, Koki Sanagi wrote:
>> (2010/07/20 20:50), Neil Horman wrote:
>>> On Tue, Jul 20, 2010 at 09:49:10AM +0900, Koki Sanagi wrote:
>>>> [RFC PATCH v3 4/5] skb: add tracepoints to freeing skb
>>>> This patch adds tracepoint to consume_skb, dev_kfree_skb_irq and
>>>> skb_free_datagram_locked. Combinating with tracepoint on dev_hard_start_xmit,
>>>> we can check how long it takes to free transmited packets. And using it, we can
>>>> calculate how many packets driver had at that time. It is useful when a drop of
>>>> transmited packet is a problem.
>>>>
>>>> <idle>-0 [001] 241409.218333: consume_skb: skbaddr=dd6b2fb8
>>>> <idle>-0 [001] 241409.490555: dev_kfree_skb_irq: skbaddr=f5e29840
>>>>
>>>> udp-recv-302 [001] 515031.206008: skb_free_datagram_locked: skbaddr=f5b1d900
>>>>
>>>>
>>>> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
>>>> ---
>>>> include/trace/events/skb.h | 42 ++++++++++++++++++++++++++++++++++++++++++
>>>> net/core/datagram.c | 1 +
>>>> net/core/dev.c | 2 ++
>>>> net/core/skbuff.c | 1 +
>>>> 4 files changed, 46 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
>>>> index 4b2be6d..84c9041 100644
>>>> --- a/include/trace/events/skb.h
>>>> +++ b/include/trace/events/skb.h
>>>> @@ -35,6 +35,48 @@ TRACE_EVENT(kfree_skb,
>>>> __entry->skbaddr, __entry->protocol, __entry->location)
>>>> );
>>>>
>>>> +DECLARE_EVENT_CLASS(free_skb,
>>>> +
>>>> + TP_PROTO(struct sk_buff *skb),
>>>> +
>>>> + TP_ARGS(skb),
>>>> +
>>>> + TP_STRUCT__entry(
>>>> + __field( void *, skbaddr )
>>>> + ),
>>>> +
>>>> + TP_fast_assign(
>>>> + __entry->skbaddr = skb;
>>>> + ),
>>>> +
>>>> + TP_printk("skbaddr=%p", __entry->skbaddr)
>>>> +
>>>> +);
>>>> +
>>>> +DEFINE_EVENT(free_skb, consume_skb,
>>>> +
>>>> + TP_PROTO(struct sk_buff *skb),
>>>> +
>>>> + TP_ARGS(skb)
>>>> +
>>>> +);
>>>> +
>>>> +DEFINE_EVENT(free_skb, dev_kfree_skb_irq,
>>>> +
>>>> + TP_PROTO(struct sk_buff *skb),
>>>> +
>>>> + TP_ARGS(skb)
>>>> +
>>>> +);
>>>> +
>>>> +DEFINE_EVENT(free_skb, skb_free_datagram_locked,
>>>> +
>>>> + TP_PROTO(struct sk_buff *skb),
>>>> +
>>>> + TP_ARGS(skb)
>>>> +
>>>> +);
>>>> +
>>>
>>> Why create these last two tracepoints at all? dev_kfree_skb_irq will eventually
>>> pass through kfree_skb anyway, getting picked up by the tracepoint there, the
>>> while the latter won't (since it uses __kfree_skb instead), I think that could
>>> be fixed up by add a call to trace_kfree_skb there directly, saving you two
>>> tracepoints.
>>>
>>> Neil
>>>
>> I think dev_kfree_skb_irq isn't chased by trace_kfree_skb or trace_consume_skb
>> completely. Because net_tx_action frees skb by __kfree_skb. So it is better to
>> add trace_kfree_skb before it. skb_free_datagram_locked is same.
>>
> It isn't, you're right, but that was the point I made above. Those missed areas
> could be easily handled by adding calls to trace_kfree_skb which already exists,
> to the missed areas. Then you don't need to create those new tracepoints. The
> way your doing this, if someone wants to trace all skb frees in debugfs, they
> would have to enable three tracepoints, not just one. Not that thats the point
> of your patch, but its something to consider, and it simplifies your code.
> Neil
>
O.K. I've re-made a patch to use trace_kfree_skb instead of
trace_dev_kfree_skb_irq and trace_skb_free_datagram_locked.
But I've got a problem.
I should use not __builtin_return_address, but macro or function which returns
current address. But I don't know any macro like that. Do you know any solution ?
Koki Sanagi.
---
include/trace/events/skb.h | 17 +++++++++++++++++
net/core/datagram.c | 1 +
net/core/dev.c | 2 ++
net/core/skbuff.c | 1 +
4 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 4b2be6d..75ce9d5 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -35,6 +35,23 @@ TRACE_EVENT(kfree_skb,
__entry->skbaddr, __entry->protocol, __entry->location)
);
+TRACE_EVENT(consume_skb,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb),
+
+ TP_STRUCT__entry(
+ __field( void *, skbaddr )
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ ),
+
+ TP_printk("skbaddr=%p", __entry->skbaddr)
+);
+
TRACE_EVENT(skb_copy_datagram_iovec,
TP_PROTO(const struct sk_buff *skb, int len),
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 251997a..96dab4f 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -243,6 +243,7 @@ void skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb)
unlock_sock_fast(sk, slow);
/* skb is now orphaned, can be freed outside of locked section */
+ trace_kfree_skb(skb, __builtin_return_address(0));
__kfree_skb(skb);
}
EXPORT_SYMBOL(skb_free_datagram_locked);
diff --git a/net/core/dev.c b/net/core/dev.c
index e6a911f..faded6f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -131,6 +131,7 @@
#include <linux/random.h>
#include <trace/events/napi.h>
#include <trace/events/net.h>
+#include <trace/events/skb.h>
#include <linux/pci.h>
#include "net-sysfs.h"
@@ -2577,6 +2578,7 @@ static void net_tx_action(struct softirq_action *h)
clist = clist->next;
WARN_ON(atomic_read(&skb->users));
+ trace_kfree_skb(skb, __builtin_return_address(0));
__kfree_skb(skb);
}
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 76d33ca..ce0bc36 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -466,6 +466,7 @@ void consume_skb(struct sk_buff *skb)
smp_rmb();
else if (likely(!atomic_dec_and_test(&skb->users)))
return;
+ trace_consume_skb(skb);
__kfree_skb(skb);
}
EXPORT_SYMBOL(consume_skb);
^ permalink raw reply related
* [patch v2.8 1/4] netfilter: xt_ipvs (netfilter matcher for IPVS)
From: Simon Horman @ 2010-07-22 7:35 UTC (permalink / raw)
To: lvs-devel, netdev, linux-kernel, netfilter, netfilter-devel
Cc: Malcolm Turnbull, Mark Brooks, Wensong Zhang, Julius Volz,
Patrick McHardy, David S. Miller, Hannes Eder, Jan Engelhardt
In-Reply-To: <20100722073547.504156161@vergenet.net>
[-- Attachment #1: netfilter-xt_ipvs-netfilter-matcher-for-IPVS.patch --]
[-- Type: text/plain, Size: 8937 bytes --]
From: Hannes Eder <heder@google.com>
This implements the kernel-space side of the netfilter matcher xt_ipvs.
[ minor fixes by Simon Horman <horms@verge.net.au> ]
Signed-off-by: Hannes Eder <heder@google.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/linux/netfilter/xt_ipvs.h | 27 ++++
net/netfilter/Kconfig | 10 +
net/netfilter/Makefile | 1
net/netfilter/ipvs/ip_vs_proto.c | 1
net/netfilter/xt_ipvs.c | 189 +++++++++++++++++++++++++++++++++++++
5 files changed, 228 insertions(+), 0 deletions(-)
create mode 100644 include/linux/netfilter/xt_ipvs.h
create mode 100644 net/netfilter/xt_ipvs.c
v2.8
* Trivial rediff
As suggested by Jan Engelhardt
* Use an enum instead of #ifdefs for flags and masks
v2.5
* Use nf_ct_is_untracked(ct) instead of ct == nf_ct_is_untracked(ct),
the later is blatantly incorrect.
v2.4
As per advice from Patrick McHardy
* Reduce size of l4proto and fwd_method members of struct xt_ipvs_mtinf
from __u16 to __u8
* Use nf_conntrack_untracked() instead of &nf_conntrack_untracked
v2.3
As per advice from Patrick McHardy
* Don't define a value for _XT_IPVS_H in xt_ipvs.h
* Depend on NF_CONNTRACK
* Update to new API
- ipvs_mt_check() should return an int rather than a bool
- Change type of ipvs_mt()'s par parameter from
struct xt_action_param to struct xt_match_param
- Make ipvs_mt()'s par parameter non-const
v2.1, v2.2
No Change
Index: nf-next-2.6/include/linux/netfilter/xt_ipvs.h
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ nf-next-2.6/include/linux/netfilter/xt_ipvs.h 2010-07-22 10:39:44.000000000 +0900
@@ -0,0 +1,27 @@
+#ifndef _XT_IPVS_H
+#define _XT_IPVS_H
+
+enum {
+ XT_IPVS_IPVS_PROPERTY = 1 << 0, /* all other options imply this one */
+ XT_IPVS_PROTO = 1 << 1,
+ XT_IPVS_VADDR = 1 << 2,
+ XT_IPVS_VPORT = 1 << 3,
+ XT_IPVS_DIR = 1 << 4,
+ XT_IPVS_METHOD = 1 << 5,
+ XT_IPVS_VPORTCTL = 1 << 6,
+ XT_IPVS_MASK = (1 << 7) - 1,
+ XT_IPVS_ONCE_MASK = XT_IPVS_MASK & ~XT_IPVS_IPVS_PROPERTY
+};
+
+struct xt_ipvs_mtinfo {
+ union nf_inet_addr vaddr, vmask;
+ __be16 vport;
+ __u8 l4proto;
+ __u8 fwd_method;
+ __be16 vportctl;
+
+ __u8 invert;
+ __u8 bitmask;
+};
+
+#endif /* _XT_IPVS_H */
Index: nf-next-2.6/net/netfilter/Kconfig
===================================================================
--- nf-next-2.6.orig/net/netfilter/Kconfig 2010-07-22 10:13:21.000000000 +0900
+++ nf-next-2.6/net/netfilter/Kconfig 2010-07-22 10:13:42.000000000 +0900
@@ -742,6 +742,16 @@ config NETFILTER_XT_MATCH_IPRANGE
If unsure, say M.
+config NETFILTER_XT_MATCH_IPVS
+ tristate '"ipvs" match support'
+ depends on IP_VS
+ depends on NETFILTER_ADVANCED
+ depends on NF_CONNTRACK
+ help
+ This option allows you to match against IPVS properties of a packet.
+
+ If unsure, say N.
+
config NETFILTER_XT_MATCH_LENGTH
tristate '"length" match support'
depends on NETFILTER_ADVANCED
Index: nf-next-2.6/net/netfilter/Makefile
===================================================================
--- nf-next-2.6.orig/net/netfilter/Makefile 2010-07-22 10:13:21.000000000 +0900
+++ nf-next-2.6/net/netfilter/Makefile 2010-07-22 10:13:42.000000000 +0900
@@ -77,6 +77,7 @@ obj-$(CONFIG_NETFILTER_XT_MATCH_HASHLIMI
obj-$(CONFIG_NETFILTER_XT_MATCH_HELPER) += xt_helper.o
obj-$(CONFIG_NETFILTER_XT_MATCH_HL) += xt_hl.o
obj-$(CONFIG_NETFILTER_XT_MATCH_IPRANGE) += xt_iprange.o
+obj-$(CONFIG_NETFILTER_XT_MATCH_IPVS) += xt_ipvs.o
obj-$(CONFIG_NETFILTER_XT_MATCH_LENGTH) += xt_length.o
obj-$(CONFIG_NETFILTER_XT_MATCH_LIMIT) += xt_limit.o
obj-$(CONFIG_NETFILTER_XT_MATCH_MAC) += xt_mac.o
Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_proto.c
===================================================================
--- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_proto.c 2010-07-22 10:13:21.000000000 +0900
+++ nf-next-2.6/net/netfilter/ipvs/ip_vs_proto.c 2010-07-22 10:13:42.000000000 +0900
@@ -98,6 +98,7 @@ struct ip_vs_protocol * ip_vs_proto_get(
return NULL;
}
+EXPORT_SYMBOL(ip_vs_proto_get);
/*
Index: nf-next-2.6/net/netfilter/xt_ipvs.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ nf-next-2.6/net/netfilter/xt_ipvs.c 2010-07-22 10:13:42.000000000 +0900
@@ -0,0 +1,189 @@
+/*
+ * xt_ipvs - kernel module to match IPVS connection properties
+ *
+ * Author: Hannes Eder <heder@google.com>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/spinlock.h>
+#include <linux/skbuff.h>
+#ifdef CONFIG_IP_VS_IPV6
+#include <net/ipv6.h>
+#endif
+#include <linux/ip_vs.h>
+#include <linux/types.h>
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter/xt_ipvs.h>
+#include <net/netfilter/nf_conntrack.h>
+
+#include <net/ip_vs.h>
+
+MODULE_AUTHOR("Hannes Eder <heder@google.com>");
+MODULE_DESCRIPTION("Xtables: match IPVS connection properties");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("ipt_ipvs");
+MODULE_ALIAS("ip6t_ipvs");
+
+/* borrowed from xt_conntrack */
+static bool ipvs_mt_addrcmp(const union nf_inet_addr *kaddr,
+ const union nf_inet_addr *uaddr,
+ const union nf_inet_addr *umask,
+ unsigned int l3proto)
+{
+ if (l3proto == NFPROTO_IPV4)
+ return ((kaddr->ip ^ uaddr->ip) & umask->ip) == 0;
+#ifdef CONFIG_IP_VS_IPV6
+ else if (l3proto == NFPROTO_IPV6)
+ return ipv6_masked_addr_cmp(&kaddr->in6, &umask->in6,
+ &uaddr->in6) == 0;
+#endif
+ else
+ return false;
+}
+
+static bool
+ipvs_mt(const struct sk_buff *skb, struct xt_action_param *par)
+{
+ const struct xt_ipvs_mtinfo *data = par->matchinfo;
+ /* ipvs_mt_check ensures that family is only NFPROTO_IPV[46]. */
+ const u_int8_t family = par->family;
+ struct ip_vs_iphdr iph;
+ struct ip_vs_protocol *pp;
+ struct ip_vs_conn *cp;
+ bool match = true;
+
+ if (data->bitmask == XT_IPVS_IPVS_PROPERTY) {
+ match = skb->ipvs_property ^
+ !!(data->invert & XT_IPVS_IPVS_PROPERTY);
+ goto out;
+ }
+
+ /* other flags than XT_IPVS_IPVS_PROPERTY are set */
+ if (!skb->ipvs_property) {
+ match = false;
+ goto out;
+ }
+
+ ip_vs_fill_iphdr(family, skb_network_header(skb), &iph);
+
+ if (data->bitmask & XT_IPVS_PROTO)
+ if ((iph.protocol == data->l4proto) ^
+ !(data->invert & XT_IPVS_PROTO)) {
+ match = false;
+ goto out;
+ }
+
+ pp = ip_vs_proto_get(iph.protocol);
+ if (unlikely(!pp)) {
+ match = false;
+ goto out;
+ }
+
+ /*
+ * Check if the packet belongs to an existing entry
+ */
+ cp = pp->conn_out_get(family, skb, pp, &iph, iph.len, 1 /* inverse */);
+ if (unlikely(cp == NULL)) {
+ match = false;
+ goto out;
+ }
+
+ /*
+ * We found a connection, i.e. ct != 0, make sure to call
+ * __ip_vs_conn_put before returning. In our case jump to out_put_con.
+ */
+
+ if (data->bitmask & XT_IPVS_VPORT)
+ if ((cp->vport == data->vport) ^
+ !(data->invert & XT_IPVS_VPORT)) {
+ match = false;
+ goto out_put_cp;
+ }
+
+ if (data->bitmask & XT_IPVS_VPORTCTL)
+ if ((cp->control != NULL &&
+ cp->control->vport == data->vportctl) ^
+ !(data->invert & XT_IPVS_VPORTCTL)) {
+ match = false;
+ goto out_put_cp;
+ }
+
+ if (data->bitmask & XT_IPVS_DIR) {
+ enum ip_conntrack_info ctinfo;
+ struct nf_conn *ct = nf_ct_get(skb, &ctinfo);
+
+ if (ct == NULL || nf_ct_is_untracked(ct)) {
+ match = false;
+ goto out_put_cp;
+ }
+
+ if ((ctinfo >= IP_CT_IS_REPLY) ^
+ !!(data->invert & XT_IPVS_DIR)) {
+ match = false;
+ goto out_put_cp;
+ }
+ }
+
+ if (data->bitmask & XT_IPVS_METHOD)
+ if (((cp->flags & IP_VS_CONN_F_FWD_MASK) == data->fwd_method) ^
+ !(data->invert & XT_IPVS_METHOD)) {
+ match = false;
+ goto out_put_cp;
+ }
+
+ if (data->bitmask & XT_IPVS_VADDR) {
+ if (ipvs_mt_addrcmp(&cp->vaddr, &data->vaddr,
+ &data->vmask, family) ^
+ !(data->invert & XT_IPVS_VADDR)) {
+ match = false;
+ goto out_put_cp;
+ }
+ }
+
+out_put_cp:
+ __ip_vs_conn_put(cp);
+out:
+ pr_debug("match=%d\n", match);
+ return match;
+}
+
+static int ipvs_mt_check(const struct xt_mtchk_param *par)
+{
+ if (par->family != NFPROTO_IPV4
+#ifdef CONFIG_IP_VS_IPV6
+ && par->family != NFPROTO_IPV6
+#endif
+ ) {
+ pr_info("protocol family %u not supported\n", par->family);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static struct xt_match xt_ipvs_mt_reg __read_mostly = {
+ .name = "ipvs",
+ .revision = 0,
+ .family = NFPROTO_UNSPEC,
+ .match = ipvs_mt,
+ .checkentry = ipvs_mt_check,
+ .matchsize = XT_ALIGN(sizeof(struct xt_ipvs_mtinfo)),
+ .me = THIS_MODULE,
+};
+
+static int __init ipvs_mt_init(void)
+{
+ return xt_register_match(&xt_ipvs_mt_reg);
+}
+
+static void __exit ipvs_mt_exit(void)
+{
+ xt_unregister_match(&xt_ipvs_mt_reg);
+}
+
+module_init(ipvs_mt_init);
+module_exit(ipvs_mt_exit);
^ permalink raw reply
* Re: macvtap: Limit packet queue length
From: Chris Wright @ 2010-07-22 7:47 UTC (permalink / raw)
To: Herbert Xu
Cc: David S. Miller, netdev, Arnd Bergmann, Mark Wagner, Chris Wright
In-Reply-To: <20100722074431.GA26744@gondor.apana.org.au>
* Herbert Xu (herbert@gondor.hengli.com.au) wrote:
> On Thu, Jul 22, 2010 at 02:41:57PM +0800, Herbert Xu wrote:
> > Hi:
> >
> > macvtap: Limit packet queue length
>
> Chris has informed me that he's already tried a similar patch
> and it only makes the problem worse :)
>
> The issue is that the macvtap TX queue length defaults to zero.
>
> So here is an updated patch which addresses this:
>
> macvtap: Limit packet queue length
>
> Mark Wagner reported OOM symptoms when sending UDP traffic over
> a macvtap link to a kvm receiver.
>
> This appears to be caused by the fact that macvtap packet queues
> are unlimited in length. This means that if the receiver can't
> keep up with the rate of flow, then we will hit OOM. Of course
> it gets worse if the OOM killer then decides to kill the receiver.
>
> This patch imposes a cap on the packet queue length, in the same
> way as the tuntap driver, using the device TX queue length.
>
> Please note that macvtap currently has no way of giving congestion
> notification, that means the software device TX queue cannot be
> used and packets will always be dropped once the macvtap driver
> queue fills up.
>
> This shouldn't be a great problem for the scenario where macvtap
> is used to feed a kvm receiver, as the traffic is most likely
> external in origin so congestion notification can't be applied
> anyway.
>
> Of course, if anybody decides to complain about guest-to-guest
> UDP packet loss down the track, then we may have to revisit this.
>
> Incidentally, this patch also fixes a real memory leak when
> macvtap_get_queue fails.
>
> Chris Wright noticed that for this patch to work, we need a
> non-zero TX queue length. This patch includes his work to change
> the default macvtap TX queue length to 500.
>
> Reported-by: Mark Wagner <mwagner@redhat.com>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Acked-by: Chris Wright <chrisw@sous-sol.org>
Thanks Herbert.
-chris
^ 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