* Re: [Patch][RFC] Disabling per-tgid stats on task exit in taskstats
From: Shailabh Nagar @ 2006-06-30 19:19 UTC (permalink / raw)
To: Shailabh Nagar
Cc: Andrew Morton, Paul Jackson, Valdis.Kletnieks, jlan, balbir,
csturtiv, linux-kernel, Jamal, netdev
In-Reply-To: <44A5770F.3080206@watson.ibm.com>
Shailabh Nagar wrote:
> Shailabh Nagar wrote:
>
>
<snip>
> Index: linux-2.6.17-mm3equiv/kernel/taskstats.c
> ===================================================================
> --- linux-2.6.17-mm3equiv.orig/kernel/taskstats.c 2006-06-30 11:57:14.000000000 -0400
> +++ linux-2.6.17-mm3equiv/kernel/taskstats.c 2006-06-30 13:58:36.000000000 -0400
> @@ -266,7 +266,7 @@ void taskstats_exit_send(struct task_str
> struct sk_buff *rep_skb;
> void *reply;
> size_t size;
> - int is_thread_group;
> + int is_thread_group, setid;
> struct nlattr *na;
>
> if (!family_registered || !tidstats)
> @@ -320,7 +320,8 @@ void taskstats_exit_send(struct task_str
> nla_nest_end(rep_skb, na);
>
> send:
> - send_reply(rep_skb, 0, TASKSTATS_MSG_MULTICAST);
> + setid = (smp_processor_id()%TASKSTATS_CPUS_PER_SET)+1;
> + send_reply(rep_skb, setid, TASKSTATS_MSG_MULTICAST);
This should be
send_reply(rep_skb, setid, TASKSTATS_MSG_UNICAST);
> return;
>
> nla_put_failure:
> Index: linux-2.6.17-mm3equiv/Documentation/accounting/getdelays.c
> ===================================================================
> --- linux-2.6.17-mm3equiv.orig/Documentation/accounting/getdelays.c 2006-06-28 16:08:56.000000000 -0400
> +++ linux-2.6.17-mm3equiv/Documentation/accounting/getdelays.c 2006-06-30 14:09:28.000000000 -0400
> @@ -40,7 +40,7 @@ int done = 0;
> /*
> * Create a raw netlink socket and bind
> */
> -static int create_nl_socket(int protocol, int groups)
> +static int create_nl_socket(int protocol, int cpugroup)
> {
> socklen_t addr_len;
> int fd;
> @@ -52,7 +52,8 @@ static int create_nl_socket(int protocol
>
> memset(&local, 0, sizeof(local));
> local.nl_family = AF_NETLINK;
> - local.nl_groups = groups;
> + local.nl_groups = TASKSTATS_LISTEN_GROUP;
> + local.nl_pid = cpugroup;
>
> if (bind(fd, (struct sockaddr *) &local, sizeof(local)) < 0)
> goto error;
> @@ -203,7 +204,7 @@ int main(int argc, char *argv[])
> pid_t rtid = 0;
> int cmd_type = TASKSTATS_TYPE_TGID;
> int c, status;
> - int forking = 0;
> + int forking = 0, cpugroup = 0;
> struct sigaction act = {
> .sa_handler = SIG_IGN,
> .sa_mask = SA_NOMASK,
> @@ -222,7 +223,7 @@ int main(int argc, char *argv[])
>
> while (1) {
>
> - c = getopt(argc, argv, "t:p:c:");
> + c = getopt(argc, argv, "t:p:c:g:l");
> if (c < 0)
> break;
>
> @@ -252,8 +253,14 @@ int main(int argc, char *argv[])
> }
> forking = 1;
> break;
> + case 'g':
> + cpugroup = atoi(optarg);
> + break;
> + case 'l':
> + loop = 1;
> + break;
> default:
> - printf("usage %s [-t tgid][-p pid][-c cmd]\n", argv[0]);
> + printf("usage %s [-t tgid][-p pid][-c cmd][-g cpugroup][-l]\n", argv[0]);
> exit(-1);
> break;
> }
> @@ -266,7 +273,7 @@ int main(int argc, char *argv[])
> /* Send Netlink request message & get reply */
>
> if ((nl_sd =
> - create_nl_socket(NETLINK_GENERIC, TASKSTATS_LISTEN_GROUP)) < 0)
> + create_nl_socket(NETLINK_GENERIC, cpugroup)) < 0)
> err(1, "error creating Netlink socket\n");
>
>
> @@ -287,10 +294,10 @@ int main(int argc, char *argv[])
>
>
> if (!forking && sendto_fd(nl_sd, (char *) &req, req.n.nlmsg_len) < 0)
> + if ((!forking && !loop) &&
> + sendto_fd(nl_sd, (char *) &req, req.n.nlmsg_len) < 0)
> err(1, "error sending message via Netlink\n");
>
> - act.sa_handler = SIG_IGN;
> - sigemptyset(&act.sa_mask);
> if (sigaction(SIGINT, &act, NULL) < 0)
> err(1, "sigaction failed for SIGINT\n");
>
> @@ -349,10 +356,11 @@ int main(int argc, char *argv[])
> rtid = *(int *) NLA_DATA(na);
> break;
> case TASKSTATS_TYPE_STATS:
> - if (rtid == tid) {
> + if (rtid == tid || loop) {
> print_taskstats((struct taskstats *)
> NLA_DATA(na));
> - done = 1;
> + if (!loop)
> + done = 1;
> }
> break;
> }
> @@ -369,7 +377,7 @@ int main(int argc, char *argv[])
> if (done)
> break;
> }
> - while (1);
> + while (loop);
>
> close(nl_sd);
> return 0;
>
^ permalink raw reply
* Re: [Patch][RFC] Disabling per-tgid stats on task exit in taskstats
From: Shailabh Nagar @ 2006-06-30 19:10 UTC (permalink / raw)
To: Shailabh Nagar
Cc: Andrew Morton, Paul Jackson, Valdis.Kletnieks, jlan, balbir,
csturtiv, linux-kernel, Jamal, netdev
In-Reply-To: <44A57310.3010208@watson.ibm.com>
Shailabh Nagar wrote:
> Andrew,
>
> Based on previous discussions, the above solutions can be expanded/modified to:
>
> a) allow userspace to listen to a group of cpus instead of all. Multiple
> collection daemons can distribute the load as you pointed out. Doing collection
> by cpu groups rather than individual cpus reduces the aggregation burden on
> userspace (and scales better with NR_CPUS)
> I'm sending a patch that demonstrates how a) can be done quite simply
> and a patch for b) is in progress.
>
Here's the patch.
Testing etc. need to be done (an earlier version that did per-cpu queues
has worked) but the main point is to show how small a change is needed
in the interface (on both the kernel and user side)
and current codebase to achieve the a) solution.
Also to get feedback on this kind of usage of the nl_pid field, the
approach etc.
Thanks,
Shailabh
=======================================================================
On systems with a large number of cpus, with even a modest rate of tasks
exiting per cpu, the volume of taskstats data sent on thread exit can overflow
a userspace listener's buffers.
One approach to avoiding overflow is to allow listeners to get data for a
limited number of cpus. By scaling the number of listening programs, each
listening to a different set of cpus, userspace can avoid more overflow
situations.
This patch implements this idea by creating simple grouping of cpus and
allowing userspace to listen to any cpu group it chooses.
Alternative designs considered and rejected were:
- creating a separate netlink group for each group of cpus. Since only 32
netlink groups can be specified by a user, this option will not scale with
number of cpus.
- aligning the grouping of cpus with cpusets. The unnecessary tying together
of the two functionalities was not merited.
Thanks to Balbir Singh for discovering the potential use of the pid field of
sockaddr_nl as a communication subchannel in the same socket, Paul Jackson and
Vivek Kashyap for suggesting cpus be grouped together for data send purposes.
Signed-Off-By: Shailabh Nagar <nagar@watson.ibm.com>
Signed-Off-By: Balbir Singh <balbir@in.ibm.com>
Documentation/accounting/getdelays.c | 30 +++++++++++++++++++-----------
include/linux/taskstats.h | 22 ++++++++++++++++++++++
kernel/taskstats.c | 5 +++--
3 files changed, 44 insertions(+), 13 deletions(-)
Index: linux-2.6.17-mm3equiv/include/linux/taskstats.h
===================================================================
--- linux-2.6.17-mm3equiv.orig/include/linux/taskstats.h 2006-06-30 11:57:14.000000000 -0400
+++ linux-2.6.17-mm3equiv/include/linux/taskstats.h 2006-06-30 14:24:49.000000000 -0400
@@ -89,6 +89,28 @@ struct taskstats {
#define TASKSTATS_LISTEN_GROUP 0x1
+
+/*
+ * Per-task exit data sent from the kernel to user space
+ * is tagged by an id based on grouping of cpus.
+ *
+ * If userspace specifies a non-zero P as the nl_pid field of
+ * the sockaddr_nl structure while binding to a netlink socket,
+ * it will receive exit data from threads that exited on cpus in the range
+ *
+ * [(P-1)*Y, P*Y-1]
+ *
+ * where Y = TASKSTATS_CPUS_PER_SET
+ * i.e. if TASKSTATS_CPUS_PER_SET is 16,
+ * to listen to data from cpus 0..15, specify P=1
+ * for cpus 16..32, specify P=2 etc.
+ *
+ * To listen to data from all cpus, userspace should use P=0
+ */
+
+#define TASKSTATS_CPUS_PER_SET 16
+
+
/*
* Commands sent from userspace
* Not versioned. New commands should only be inserted at the enum's end
Index: linux-2.6.17-mm3equiv/kernel/taskstats.c
===================================================================
--- linux-2.6.17-mm3equiv.orig/kernel/taskstats.c 2006-06-30 11:57:14.000000000 -0400
+++ linux-2.6.17-mm3equiv/kernel/taskstats.c 2006-06-30 13:58:36.000000000 -0400
@@ -266,7 +266,7 @@ void taskstats_exit_send(struct task_str
struct sk_buff *rep_skb;
void *reply;
size_t size;
- int is_thread_group;
+ int is_thread_group, setid;
struct nlattr *na;
if (!family_registered || !tidstats)
@@ -320,7 +320,8 @@ void taskstats_exit_send(struct task_str
nla_nest_end(rep_skb, na);
send:
- send_reply(rep_skb, 0, TASKSTATS_MSG_MULTICAST);
+ setid = (smp_processor_id()%TASKSTATS_CPUS_PER_SET)+1;
+ send_reply(rep_skb, setid, TASKSTATS_MSG_MULTICAST);
return;
nla_put_failure:
Index: linux-2.6.17-mm3equiv/Documentation/accounting/getdelays.c
===================================================================
--- linux-2.6.17-mm3equiv.orig/Documentation/accounting/getdelays.c 2006-06-28 16:08:56.000000000 -0400
+++ linux-2.6.17-mm3equiv/Documentation/accounting/getdelays.c 2006-06-30 14:09:28.000000000 -0400
@@ -40,7 +40,7 @@ int done = 0;
/*
* Create a raw netlink socket and bind
*/
-static int create_nl_socket(int protocol, int groups)
+static int create_nl_socket(int protocol, int cpugroup)
{
socklen_t addr_len;
int fd;
@@ -52,7 +52,8 @@ static int create_nl_socket(int protocol
memset(&local, 0, sizeof(local));
local.nl_family = AF_NETLINK;
- local.nl_groups = groups;
+ local.nl_groups = TASKSTATS_LISTEN_GROUP;
+ local.nl_pid = cpugroup;
if (bind(fd, (struct sockaddr *) &local, sizeof(local)) < 0)
goto error;
@@ -203,7 +204,7 @@ int main(int argc, char *argv[])
pid_t rtid = 0;
int cmd_type = TASKSTATS_TYPE_TGID;
int c, status;
- int forking = 0;
+ int forking = 0, cpugroup = 0;
struct sigaction act = {
.sa_handler = SIG_IGN,
.sa_mask = SA_NOMASK,
@@ -222,7 +223,7 @@ int main(int argc, char *argv[])
while (1) {
- c = getopt(argc, argv, "t:p:c:");
+ c = getopt(argc, argv, "t:p:c:g:l");
if (c < 0)
break;
@@ -252,8 +253,14 @@ int main(int argc, char *argv[])
}
forking = 1;
break;
+ case 'g':
+ cpugroup = atoi(optarg);
+ break;
+ case 'l':
+ loop = 1;
+ break;
default:
- printf("usage %s [-t tgid][-p pid][-c cmd]\n", argv[0]);
+ printf("usage %s [-t tgid][-p pid][-c cmd][-g cpugroup][-l]\n", argv[0]);
exit(-1);
break;
}
@@ -266,7 +273,7 @@ int main(int argc, char *argv[])
/* Send Netlink request message & get reply */
if ((nl_sd =
- create_nl_socket(NETLINK_GENERIC, TASKSTATS_LISTEN_GROUP)) < 0)
+ create_nl_socket(NETLINK_GENERIC, cpugroup)) < 0)
err(1, "error creating Netlink socket\n");
@@ -287,10 +294,10 @@ int main(int argc, char *argv[])
if (!forking && sendto_fd(nl_sd, (char *) &req, req.n.nlmsg_len) < 0)
+ if ((!forking && !loop) &&
+ sendto_fd(nl_sd, (char *) &req, req.n.nlmsg_len) < 0)
err(1, "error sending message via Netlink\n");
- act.sa_handler = SIG_IGN;
- sigemptyset(&act.sa_mask);
if (sigaction(SIGINT, &act, NULL) < 0)
err(1, "sigaction failed for SIGINT\n");
@@ -349,10 +356,11 @@ int main(int argc, char *argv[])
rtid = *(int *) NLA_DATA(na);
break;
case TASKSTATS_TYPE_STATS:
- if (rtid == tid) {
+ if (rtid == tid || loop) {
print_taskstats((struct taskstats *)
NLA_DATA(na));
- done = 1;
+ if (!loop)
+ done = 1;
}
break;
}
@@ -369,7 +377,7 @@ int main(int argc, char *argv[])
if (done)
break;
}
- while (1);
+ while (loop);
close(nl_sd);
return 0;
^ permalink raw reply
* Re: [Patch][RFC] Disabling per-tgid stats on task exit in taskstats
From: Shailabh Nagar @ 2006-06-30 18:53 UTC (permalink / raw)
To: Andrew Morton
Cc: Paul Jackson, Valdis.Kletnieks, jlan, balbir, csturtiv,
linux-kernel, Jamal, netdev
In-Reply-To: <20060629110107.2e56310b.akpm@osdl.org>
Andrew Morton wrote:
> On Thu, 29 Jun 2006 09:44:08 -0700
> Paul Jackson <pj@sgi.com> wrote:
>
>
>>>You're probably correct on that model. However, it all depends on the actual
>>>workload. Are people who actually have large-CPU (>256) systems actually
>>>running fork()-heavy things like webservers on them, or are they running things
>>>like database servers and computations, which tend to have persistent
>>>processes?
>>
>>It may well be mostly as you say - the large-CPU systems not running
>>the fork() heavy jobs.
>>
>>Sooner or later, someone will want to run a fork()-heavy job on a
>>large-CPU system. On a 1024 CPU system, it would apparently take
>>just 14 exits/sec/CPU to hit this bottleneck, if Jay's number of
>>14000 applied.
>>
>>Chris Sturdivant's reply is reasonable -- we'll hit it sooner or later,
>>and deal with it then.
>>
>
>
> I agree, and I'm viewing this as blocking the taskstats merge. Because if
> this _is_ a problem then it's a big one because fixing it will be
> intrusive, and might well involve userspace-visible changes.
>
> The only ways I can see of fixing the problem generally are to either
>
> a) throw more CPU(s) at stats collection: allow userspace to register for
> "stats generated by CPU N", then run a stats collection daemon on each
> CPU or
>
> b) make the kernel recognise when it's getting overloaded and switch to
> some degraded mode where it stops trying to send all the data to
> userspace - just send a summary, or a "we goofed" message or something.
Andrew,
Based on previous discussions, the above solutions can be expanded/modified to:
a) allow userspace to listen to a group of cpus instead of all. Multiple
collection daemons can distribute the load as you pointed out. Doing collection
by cpu groups rather than individual cpus reduces the aggregation burden on
userspace (and scales better with NR_CPUS)
b) do flow control on the kernel send side. This can involve buffering and sending
later (to handle bursty case) or dropping (to handle sustained load) as pointed out
by you, Jamal in other threads.
c) increase receiver's socket buffer. This can and should always be done but no
involvement needed.
With regards to taskstats changes to handle the problem and its impact on userspace
visible changes,
a) will change userspace
b) will be transparent.
c) is immaterial going forward (except perhaps as a change in Documentation)
I'm sending a patch that demonstrates how a) can be done quite simply
and a patch for b) is in progress.
If the approach suggested in patch a) is acceptable (and I'll provide the testing, stability
results once comments on it are largely over), could taskstats acceptance in 2.6.18 go ahead
and patch b) be added later (solution outline has already been provided and a prelim patch should
be out by eod)
--Shailabh
^ permalink raw reply
* Re: jumbo frames and memory fragmentation
From: Evgeniy Polyakov @ 2006-06-30 18:48 UTC (permalink / raw)
To: Chris Friesen; +Cc: Herbert Xu, netdev
In-Reply-To: <44A56520.5010703@nortel.com>
On Fri, Jun 30, 2006 at 11:53:36AM -0600, Chris Friesen (cfriesen@nortel.com) wrote:
> >Either upgrade your kernel or backport the page-splitting code in the
> >current tree. That's really the only sane solution for jumbo packets.
>
> Looking at the page-splitting code, it says "82571 and greater support
> packet-split...". We're running the 82546GB device. Looks like it
> won't help me.
It definitely will.
Packet split in hardware means separating data and headers into
different pages in different reads, while software page split means that
skb has a list of fragments where part of the packet will be DMAed, so
jumbo frame will be converted into several pages.
--
Evgeniy Polyakov
^ permalink raw reply
* Re: [patch 2/6] [Network namespace] Network device sharing by view
From: Eric W. Biederman @ 2006-06-30 18:22 UTC (permalink / raw)
To: hadi
Cc: Andrey Savochkin, Alexey Kuznetsov, Dave Hansen, Ben Greear,
Daniel Lezcano, linux-kernel, netdev, serue, clg, Andrew Morton,
dev, devel, viro, Herbert Poetzl, Sam Vilain
In-Reply-To: <1151675452.5270.10.camel@jzny2>
jamal <hadi@cyberus.ca> writes:
>> > > Then the pragmatic question becomes how to correlate what you see from
>> > > `ip addr list' to guests.
>> >
>> > on the host ip addr and the one seen on the guest side are the same.
>> > Except one is seen (on the host) on guest0-eth0 and another is seen
>> > on eth0 (on guest).
>>
>> Then what to do if the host system has 10.0.0.1 as a private address on eth3,
>> and then interfaces guest1-tun0 and guest2-tun0 both get address 10.0.0.1
>> when each guest has added 10.0.0.1 to their tun0 device?
>
> Yes, that would be a conflict that needs to be resolved. If you look at
> ip addresses as also belonging to namespaces, then it should work, no?
> i am assuming a tag at the ifa table level.
Yes. The conception is that everything belongs to the namespace,
so it looks like you have multiple instances of the network stack.
Which means through existing interfaces it would be a real problem
if a network device showed up in more than one network stack as
that would confuse things.
Basically the reading and configuration through existing interfaces
is expected to be in the namespace as well which is where the difficulty
shows up.
When you get serious about splitting up roots powers this becomes a real
advantage. Because you might want to have one person responsible for
what would normally be eth0 and another person responsible for eth1.
Anyway Jamal can you see the problem the aliases present to the implementation?
Eric
^ permalink raw reply
* Re: strict isolation of net interfaces
From: Eric W. Biederman @ 2006-06-30 18:09 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Serge E. Hallyn, Cedric Le Goater, Sam Vilain, hadi,
Herbert Poetzl, Alexey Kuznetsov, viro, devel, dev, Andrew Morton,
netdev, linux-kernel, Andrey Savochkin, Ben Greear, Dave Hansen,
Alexey Kuznetsov, Eric W. Biederman
In-Reply-To: <44A517B4.4010500@fr.ibm.com>
Daniel Lezcano <dlezcano@fr.ibm.com> writes:
> Serge E. Hallyn wrote:
>> Quoting Cedric Le Goater (clg@fr.ibm.com):
>>
>>>we could work on virtualizing the net interfaces in the host, map them to
>>>eth0 or something in the guest and let the guest handle upper network layers ?
>>>
>>>lo0 would just be exposed relying on skbuff tagging to discriminate traffic
>>>between guests.
>> This seems to me the preferable way. We create a full virtual net
>> device for each new container, and fully virtualize the device
>> namespace.
>
Answers with respect to how I see layer 2 isolation,
with network devices and sockets as well as the associated routing
information given per namespace.
> I have a few questions about all the network isolation stuff:
>
> * What level of isolation is wanted for the network ? network devices ?
> IPv4/IPv6 ? TCP/UDP ?
>
> * How is handled the incoming packets from the network ? I mean what will be
> mecanism to dispatch the packet to the right virtual device ?
Wrong question. A better question is to ask how do you know which namespace
a packet is in.
Answer: By looking at which device or socket it just came from.
How do you get a packet into a non-default namespace?
Either you move a real network interface into that namespace.
Or you use a tunnel device that shows up as two network interfaces in
two different namespaces.
Then you route, or bridge packets between the two. Trivial.
> * How to handle the SO_BINDTODEVICE socket option ?
Just like we do now.
> * Has the virtual device a different MAC address ?
All network devices are abstractions of the hardware so they are all
sort of virtual. My implementation of a tunnel device has a mac
address so I can use it with ethernet bridging but that isn't a hard
requirement. And yes the mac address is different because you can't
do layer 2 switching if everyone has the same mac address.
But there is no special ``virtual'' device.
> How to manage it with the real MAC address on the system ?
Manage?
> How to manage ARP, ICMP, multicasting and IP ?
Like you always do. It would be a terrible implementation if
we had to change that logic. There is a little bit of that
where we need to detect which network namespace we are going to because
the answers can differ but that is pretty straight forward.
> It seems for me, IMHO that will require a lot of translation and browsing
> table. It will probably add a very significant overhead.
Then look at:
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/linux-2.6-ns.git#proof-of-concept
or the OpenVZ implementation.
It isn't serious overhead.
> * How to handle NFS access mounted outside of the container ?
The socket should remember it's network namespace.
It works fine.
> * How to handle ICMP_REDIRECT ?
Just like we always do?
Eric
^ permalink raw reply
* Re: strict isolation of net interfaces
From: Eric W. Biederman @ 2006-06-30 17:58 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Serge E. Hallyn, Cedric Le Goater, Sam Vilain, hadi,
Herbert Poetzl, Alexey Kuznetsov, viro, devel, dev, Andrew Morton,
netdev, linux-kernel, Andrey Savochkin, Ben Greear, Dave Hansen,
Alexey Kuznetsov
In-Reply-To: <44A541CB.9080102@fr.ibm.com>
Daniel Lezcano <dlezcano@fr.ibm.com> writes:
> Eric W. Biederman wrote:
>> Daniel Lezcano <dlezcano@fr.ibm.com> writes:
>>
>>>Serge E. Hallyn wrote:
>>>
>>>>Quoting Cedric Le Goater (clg@fr.ibm.com):
>>>>
>>>>
>>>>>we could work on virtualizing the net interfaces in the host, map them to
>>>>>eth0 or something in the guest and let the guest handle upper network layers
> ?
>>>>>
>>>>>lo0 would just be exposed relying on skbuff tagging to discriminate traffic
>>>>>between guests.
>>>>
>>>>This seems to me the preferable way. We create a full virtual net
>>>>device for each new container, and fully virtualize the device
>>>>namespace.
>>>
>>>I have a few questions about all the network isolation stuff:
>>
>
> It seems these questions are not important.
I'm just trying to get us back to a productive topic.
>> So far I have seen two viable possibilities on the table,
>> neither of them involve multiple names for a network device.
>> layer 3 (filtering the allowed ip addresses at bind time roughly the current
>> vserver).
>> - implementable as a security hook.
>> - Benefit no measurable performance impact.
>> - Downside not many things we can do.
>
> What things ? Can you develop please ? Can you give some examples ?
DHCP, tcpdump,.. Probably a bad way of phrasing it. But there
is a lot more that we can do using a pure layer 2 approach.
>> layer 2 (What appears to applications a separate instance of the network
>> stack).
>> - Implementable as a namespace.
>
> what about accessing a NFS mounted outside the container ?
As I replied earlier it isn't a problem. If you get to it through the
filesystem namespace it uses the network namespace it was mounted with
for it's connection.
>> - Each network namespace would have dedicated network devices.
>> - Benefit extremely flexible.
>
> For what ? For who ? Do you have examples ?
See above.
>> - Downside since at least the slow path must examine the packet
>> it has the possibility of slowing down the networking stack.
>
> What is/are the slow path(s) you identified ?
Grr. I put that badly. Basically at least on the slow path you need to
look at a per network namespace data structure. The extra pointer
indirection could slow things down. The point is that we may be
able to have a fast path that is exactly the same as the rest
of the network stack.
If the obvious approach does not work my gut the feeling the
network stack fast path will give us an implementation without overhead.
>> For me the important characteristics.
>> - Allows for application migration, when we take our ip address with us.
>> In particular it allows for importation of addresses assignments
>> mad on other machines.
>
> Ok for the two methods no ?
So far.
>> - No measurable impact on the existing networking when the code
>> is compiled in.
>
> You contradict ...
How so? As far as I can tell this is a basic requirement to get
merged.
>> - Clean predictable semantics.
>
> What that means ? Can you explain, please ?
>> This whole debate on network devices show up in multiple network namespaces
>> is just silly.
>
> The debate is not on the network device show up. The debate is can we have a
> network isolation ___usable for everybody___ not only for the beauty of having
> namespaces and for a system container like.
This subthread talking about devices showing up in multiple namespaces seemed
very much exactly on how network devices show up.
> I am not against the network device virtualization or against the namespaces. I
> am just asking if the namespace is the solution for all the network
> isolation. Should we nest layer 2 and layer 3 vitualization into namespaces or
> separate them in order to have the flexibility to choose isolation/performance.
I believe I addressed Herbert Poetzl's concerns earlier. To me the question
is can we implement an acceptable layer 2 solution, that distrubutions and
other people who do not need isolation would have no problem compiling in
by default.
The joy of namespaces is that if you don't want it you don't have to use it.
Layer 2 can do everything and is likely usable by everyone iff the performance
is acceptable.
>> The only reason for wanting that appears to be better management.
>> We have deeper issues like can we do a reasonable implementation without a
>> network device showing up in multiple namespaces.
>
> Again, I am not against having the network device virtualization. It is a good
> idea.
>
>> I think the reason the debate exists at all is that it is a very approachable
>> topic, as opposed to the fundamentals here.
>> If we can get layer 2 level isolation working without measurable overhead
>> with one namespace per device it may be worth revisiting things. Until
>> then it is a side issue at best.
>
> I agree, so where are the answers of the questions I asked in my previous email
> ? You said you did some implementation of network isolation with and without
> namespaces, so you should be able to answer...
Sorry. More than anything those questions looked retorical and aimed
at disarming some of the silliness. I will go back and try and
answer those. Fundamentally when we have one namespace that includes
network devices, network sockets, and all of the data structures necessary
to use them (routing tables and the like) and we have a tunnel device
that can connect namespaces the answers are trivial and I though obvious.
Eric
^ permalink raw reply
* Re: jumbo frames and memory fragmentation
From: Chris Friesen @ 2006-06-30 17:53 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <E1FwIcw-00068x-00@gondolin.me.apana.org.au>
Herbert Xu wrote:
> Either upgrade your kernel or backport the page-splitting code in the
> current tree. That's really the only sane solution for jumbo packets.
Looking at the page-splitting code, it says "82571 and greater support
packet-split...". We're running the 82546GB device. Looks like it
won't help me.
Chris
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: Thomas Graf @ 2006-06-30 17:44 UTC (permalink / raw)
To: Patrick McHardy; +Cc: hadi, netdev, David Miller
In-Reply-To: <44A56294.7030309@trash.net>
* Patrick McHardy <kaber@trash.net> 2006-06-30 19:42
> I think Thomas was more complaining about the values it is set to
> and the fact that only a single redirection is possible for each
> packet, no?
I have no interest in resolving this anymore, it seems to be
a "feature" to avoid tx lock deadlocks.
^ permalink raw reply
* Re: strict isolation of net interfaces
From: Eric W. Biederman @ 2006-06-30 17:41 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Daniel Lezcano, Cedric Le Goater, Sam Vilain, hadi,
Herbert Poetzl, Alexey Kuznetsov, viro, devel, dev, Andrew Morton,
netdev, linux-kernel, Andrey Savochkin, Ben Greear, Dave Hansen,
Alexey Kuznetsov
In-Reply-To: <20060630161442.GA27210@sergelap.austin.ibm.com>
"Serge E. Hallyn" <serue@us.ibm.com> writes:
> Quoting Eric W. Biederman (ebiederm@xmission.com):
>> This whole debate on network devices show up in multiple network namespaces
>> is just silly. The only reason for wanting that appears to be better
> management.
>
> A damned good reason.
Better management is a good reason. But constructing the management in
a way that hampers the implementation and confuses existing applications is
a problem.
Things are much easier if namespaces are completely independent.
Among other things the semantics are clear and obvious.
> Clearly we want the parent namespace to be able
> to control what the child can do. So whatever interface a child gets,
> the parent should be able to somehow address. Simple iptables rules
> controlling traffic between it's own netdevice and the one it hands it's
> children seem a good option.
That or we setup the child and then drop CAP_NET_ADMIN.
>> We have deeper issues like can we do a reasonable implementation without a
>> network device showing up in multiple namespaces.
>
> Isn't that the same issue?
I guess I was thinking from the performance and cleanliness point of
view.
>> If we can get layer 2 level isolation working without measurable overhead
>> with one namespace per device it may be worth revisiting things. Until
>> then it is a side issue at best.
>
> Ok, and in the meantime we can all use the network part of the bsdjail
> lsm? :)
If necessary. But mostly we concentrate on the fundamentals and figure
out what it takes to take the level 2 stuff working.
Eric
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: Patrick McHardy @ 2006-06-30 17:42 UTC (permalink / raw)
To: hadi; +Cc: netdev, David Miller, Thomas Graf
In-Reply-To: <1151688732.5270.101.camel@jzny2>
jamal wrote:
> On Fri, 2006-30-06 at 19:18 +0200, Patrick McHardy wrote:
>
>>Thomas Graf wrote:
>>
>>>* Thomas Graf <tgraf@suug.ch> 2006-06-30 18:32
>>>
>>>
>>>>Anyways, I give up. Last time I've been running after you trying
>>>>to fix the many bugs you leave behind. Ever noticed that whenever
>>>>you add some new code it's someone else following up with tons of
>>>>small bugfix patches having a hard time trying to figure out the
>>>>actual intent. I'll just duplicate the code for my purpose, so
>>>>much easier.
>>>
>>>
>>>There you go, leaves ifb broken as-is, at least prevents it
>>>from crashing randomly when the input_dev disappears.
>>
>>That would be a pity. After this patch has been ACKed, could we start
>>over with the other bugs one at a time? I wasn't really able to gather
>>the problems from this thread, but some of the behaviour you mentioned
>>does seem questionable.
>>
>
>
> I will summarize what the outstanding issues are, the rest of the "bugs"
> just ignore otherwise the discussion is a waste of time and may
> get out of control.
>
> 1) ifb references skb->input_dev
That should be fixed by this patch.
> 2) mirred sets the skb->input_dev which is used in #1
I think Thomas was more complaining about the values it is set to
and the fact that only a single redirection is possible for each
packet, no?
> It is possible that when #1 happens infact input_dev is gone because no
> ref count is incremented. Ok, Thomas is that sufficient to discuss the
> crux of the matter?
> At one point many months ago, the logic was for now the likelihood this
> will happen is low but we need to cover for by at least figuring the
> existence of input_dev when referencing it.
>
> Thomas makes the claim, this can be achieved only by using an ifindex.
> And i havent been able to see how. I have a small performance problem if
> i just use ifindex. Using ifindex will eventually save 32 bits on the
> 64 bit machines. I posed the question as to which was more beneficial
> as a solution that hasnt been addressed.
Are we still talking about this? Its easy: a pointer without taking
a reference can become stale, with ifindex you do the lookup right
when using it, so you either get an result or you don't. It also
saves atomic operations for anyone _not_ using it, which is that
vast majority of users. So the patch clearly makes sense to me.
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: Thomas Graf @ 2006-06-30 17:42 UTC (permalink / raw)
To: jamal; +Cc: Patrick McHardy, netdev, David Miller
In-Reply-To: <1151688732.5270.101.camel@jzny2>
* jamal <hadi@cyberus.ca> 2006-06-30 13:32
> I will summarize what the outstanding issues are, the rest of the "bugs"
> just ignore otherwise the discussion is a waste of time and may
> get out of control.
>
> 1) ifb references skb->input_dev
> 2) mirred sets the skb->input_dev which is used in #1
>
> It is possible that when #1 happens infact input_dev is gone because no
> ref count is incremented. Ok, Thomas is that sufficient to discuss the
> crux of the matter?
Sure, that's the most obvious bug.
> At one point many months ago, the logic was for now the likelihood this
> will happen is low but we need to cover for by at least figuring the
> existence of input_dev when referencing it.
>
> Thomas makes the claim, this can be achieved only by using an ifindex.
> And i havent been able to see how. I have a small performance problem if
> i just use ifindex. Using ifindex will eventually save 32 bits on the
> 64 bit machines. I posed the question as to which was more beneficial
> as a solution that hasnt been addressed.
I'd appreciate if you'd stop sperading lies. I claimed it to be the
best solution, not the only one. Everyone agrees that it's possible
to use a pointer and take a reference in netif_receive_skb() while
it also seems obvious to most that it's not a good idea to take two
additional atomic operations in the fast path for this purpose.
^ permalink raw reply
* Please re-apply the pwork macsuspend patch
From: Michael Buesch @ 2006-06-30 17:38 UTC (permalink / raw)
To: linville; +Cc: Jeff Garzik, netdev, bcm43xx-dev
Hi John,
Please re-apply the "Suspend MAC in periodic work" now.
I neither remember the exact name, nor do I still have
the patch. Well, I deleted it since it was applied... hrm...
But I assume you know which patch I mean.
Please re-apply it because:
* It does _NO_ harm.
* It _fixes_ a bug.
* I am doing development which depends on this patch
My development is stalled without the patch going
into the tree. (Or at least it is a _lot_ more painful).
* It does _NO_ harm.
* Every "bug" or inconvenience (udelay), that Jeff saw, was
not introduced by this patch. It was only in the _context_.
In fact, that udelay loop has been there _forever_.
Thanks.
--
Greetings Michael.
^ permalink raw reply
* Re: 2.6.17-mm4
From: Alan Cox @ 2006-06-30 17:52 UTC (permalink / raw)
To: Dave Jones
Cc: Arjan van de Ven, Ingo Molnar, Michal Piotrowski, Andrew Morton,
linux-kernel, netdev
In-Reply-To: <20060630172713.GH32729@redhat.com>
Ar Gwe, 2006-06-30 am 13:27 -0400, ysgrifennodd Dave Jones:
> surely no-one made an acpi aware vlb machine :)
>
> There are probably other creative ways.
And the not-so-creative simple one which is how old IDE addresses much
of this:
Date: Fri Jun 30 16:39:20 2006 +0100
Subject: [PATCH 13/20] My name is Ingo Molnar, you killed my make allyesconfig, prepare to die
Teach the qdi driver to be more polite about probing when compiled in
so that people who make allyesconfig don't get burned.
Alan
Signed-off-by: Alan Cox <alan@redhat.com>
---
drivers/scsi/pata_qdi.c | 15 +++++++++++++--
1 files changed, 13 insertions(+), 2 deletions(-)
12b633d45a8600147314c2ce33b28f92f52c92bd
diff --git a/drivers/scsi/pata_qdi.c b/drivers/scsi/pata_qdi.c
index ca6fef0..f8fc0ef 100644
--- a/drivers/scsi/pata_qdi.c
+++ b/drivers/scsi/pata_qdi.c
@@ -26,7 +26,7 @@
#include <linux/platform_device.h>
#define DRV_NAME "pata_qdi"
-#define DRV_VERSION "0.2.3"
+#define DRV_VERSION "0.2.4"
#define NR_HOST 4 /* Two 6580s */
@@ -41,7 +41,13 @@ struct qdi_data {
static struct ata_host_set *qdi_host[NR_HOST];
static struct qdi_data qdi_data[NR_HOST];
-static int nr_qdi_host = 0;
+static int nr_qdi_host;
+
+#ifdef MODULE
+static int probe_qdi = 1;
+#else
+static int probe_qdi;
+#endif
static void qdi6500_set_piomode(struct ata_port *ap, struct ata_device *adev)
{
@@ -302,6 +308,9 @@ static __init int qdi_init(void)
int ct = 0;
int i;
+ if (probe_qdi == 0)
+ return;
+
/*
* Check each possible QD65xx base address
*/
@@ -390,3 +399,5 @@ MODULE_VERSION(DRV_VERSION);
module_init(qdi_init);
module_exit(qdi_exit);
+module_param(probe_qdi, int, 0);
+
--
1.2.GIT
^ permalink raw reply related
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: Thomas Graf @ 2006-06-30 17:34 UTC (permalink / raw)
To: jamal; +Cc: netdev, David Miller, Patrick McHardy
In-Reply-To: <1151687977.5270.91.camel@jzny2>
* jamal <hadi@cyberus.ca> 2006-06-30 13:19
> On Fri, 2006-30-06 at 18:32 +0200, Thomas Graf wrote:
> > * jamal <hadi@cyberus.ca> 2006-06-30 10:35
>
> > > Did you actually try to run this before you reached this conclusion?
> >
> > I did, fortunately some other bug prevents this from happening,
> > packets are simply dropped somewhere.
> >
>
> It is not a bug, Thomas! I am getting a little frustrated now.
> The packets will be dropped because we set the at field to zero which is
> invalid. That is done on purpose. It is only meaningful for ifb. The
> challenge is much bigger than it appears. You could end up deadlocking
> on the tx lock. So this was the choice i had to make.
Please explain, tc_verd is reset in the tasklet after dequeueing and set
again in dev_queue_xmit(). ifb_xmit will always see a valid tc_verd
at egress.
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: Patrick McHardy @ 2006-06-30 17:33 UTC (permalink / raw)
To: hadi; +Cc: Thomas Graf, netdev, David Miller
In-Reply-To: <1151687977.5270.91.camel@jzny2>
jamal wrote:
> You are attempting to change architecture (which works just fine) in the way you
> think it should work - and then point to something as a bug because it
> doesnt work the way you think it should work.
> This is a problem not just with you BTW, but with Patrick as well (although he has
> gotten better lately). There is a huge difference for example when dealing with
> Herbert. My approach in situations like this, which you dont have to follow, is
> to ask first what the intent was then if i dont like the intent try to convince the
> owner that there maybe better ways. Or why they are wrong.
Well, I thought I stay out of this, but since you mention me ..
I also had the feeling it has gotten easier working with you lately,
but I can understand Thomas's pain, I had the same thoughts more than
once. Your code often does have an enormous amount of bugs and
whitespace and other stylistic problems and working with you can be
challenging for multiple reasons, for example having to go to endless,
often entirely pointless discussions for obvious fixes, especially if
you're already pissed by noticing 50 bugs at once. The reason why you
might be able to work better with Herbert is IMO that he usually
doesn't touch what you seem to feel is "your area". Besides that, I
never changed anything in your architecture, only fixed massive
amounts of bugs.
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: jamal @ 2006-06-30 17:32 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, David Miller, Thomas Graf
In-Reply-To: <44A55CF8.2040509@trash.net>
On Fri, 2006-30-06 at 19:18 +0200, Patrick McHardy wrote:
> Thomas Graf wrote:
> > * Thomas Graf <tgraf@suug.ch> 2006-06-30 18:32
> >
> >>Anyways, I give up. Last time I've been running after you trying
> >>to fix the many bugs you leave behind. Ever noticed that whenever
> >>you add some new code it's someone else following up with tons of
> >>small bugfix patches having a hard time trying to figure out the
> >>actual intent. I'll just duplicate the code for my purpose, so
> >>much easier.
> >
> >
> > There you go, leaves ifb broken as-is, at least prevents it
> > from crashing randomly when the input_dev disappears.
>
> That would be a pity. After this patch has been ACKed, could we start
> over with the other bugs one at a time? I wasn't really able to gather
> the problems from this thread, but some of the behaviour you mentioned
> does seem questionable.
>
I will summarize what the outstanding issues are, the rest of the "bugs"
just ignore otherwise the discussion is a waste of time and may
get out of control.
1) ifb references skb->input_dev
2) mirred sets the skb->input_dev which is used in #1
It is possible that when #1 happens infact input_dev is gone because no
ref count is incremented. Ok, Thomas is that sufficient to discuss the
crux of the matter?
At one point many months ago, the logic was for now the likelihood this
will happen is low but we need to cover for by at least figuring the
existence of input_dev when referencing it.
Thomas makes the claim, this can be achieved only by using an ifindex.
And i havent been able to see how. I have a small performance problem if
i just use ifindex. Using ifindex will eventually save 32 bits on the
64 bit machines. I posed the question as to which was more beneficial
as a solution that hasnt been addressed.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: Ben Greear @ 2006-06-30 17:27 UTC (permalink / raw)
To: Thomas Graf; +Cc: jamal, Patrick McHardy, David Miller, netdev
In-Reply-To: <20060630171348.GI14627@postel.suug.ch>
Thomas Graf wrote:
> * Thomas Graf <tgraf@suug.ch> 2006-06-30 18:32
>
>>Anyways, I give up. Last time I've been running after you trying
>>to fix the many bugs you leave behind. Ever noticed that whenever
>>you add some new code it's someone else following up with tons of
>>small bugfix patches having a hard time trying to figure out the
>>actual intent. I'll just duplicate the code for my purpose, so
>>much easier.
>
>
> There you go, leaves ifb broken as-is, at least prevents it
> from crashing randomly when the input_dev disappears.
>
> [NET]: Use interface index to keep input device information
>
> Using the interface index instead of a direct reference
> allows a safe usage beyond the scope where an interface
> could disappear.
>
> The old input_dev field was incorrectly made dependant
> on CONFIG_NET_CLS_ACT in skb_copy().
>
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> ===================================================================
> --- net-2.6.git.orig/drivers/net/ifb.c
> +++ net-2.6.git/drivers/net/ifb.c
> @@ -158,19 +158,23 @@ static int ifb_xmit(struct sk_buff *skb,
> stats->tx_packets++;
> stats->tx_bytes+=skb->len;
>
> - if (!from || !skb->input_dev) {
> + if (!from || !skb->iif) {
Since iif of 0 is valid (afaik), this check is now bogus, eh?
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: 2.6.17-mm4
From: Dave Jones @ 2006-06-30 17:27 UTC (permalink / raw)
To: Alan Cox
Cc: Arjan van de Ven, Ingo Molnar, Michal Piotrowski, Andrew Morton,
linux-kernel, netdev
In-Reply-To: <1151669670.31392.16.camel@localhost.localdomain>
On Fri, Jun 30, 2006 at 01:14:30PM +0100, Alan Cox wrote:
> Ar Gwe, 2006-06-30 am 11:54 +0200, ysgrifennodd Arjan van de Ven:
> > another quick hack is to check for vesa lb... eg if pci is present, skip
> > this thing entirely :)
>
> Not really, many people made VLB/PCI combo boards.
- check the pci version (I'm pretty sure these were pre PCI 2.0 ?)
- check for dmi existance
DMI came after VLB didn't it? Even if not, the BIOS date may
give clues. I don't recall VLB after 1996 or so.
- check for acpi existance.
surely no-one made an acpi aware vlb machine :)
There are probably other creative ways.
Dave
--
http://www.codemonkey.org.uk
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: jamal @ 2006-06-30 17:19 UTC (permalink / raw)
To: Thomas Graf; +Cc: netdev, David Miller, Patrick McHardy
In-Reply-To: <20060630163229.GH14627@postel.suug.ch>
On Fri, 2006-30-06 at 18:32 +0200, Thomas Graf wrote:
> * jamal <hadi@cyberus.ca> 2006-06-30 10:35
> > Did you actually try to run this before you reached this conclusion?
>
> I did, fortunately some other bug prevents this from happening,
> packets are simply dropped somewhere.
>
It is not a bug, Thomas! I am getting a little frustrated now.
The packets will be dropped because we set the at field to zero which is
invalid. That is done on purpose. It is only meaningful for ifb. The
challenge is much bigger than it appears. You could end up deadlocking
on the tx lock. So this was the choice i had to make.
> > With all due respect, the architecture works. I have invested many many
> > hours testing and verifying. There may be coding bugs - and those need
> > fixing. Kill the bugs.
>
> Right, just run this
>
> tc filter add dev eth0 parent 1: protocol ip prio 10
> u32 match ip tos 0 0 flowid 1:1
> action mirred egress redirect dev ifb1
> tc filter add dev ifb1 parent 1: protocol ip prio 10
> u32 match ip tos 0 0 flowid 1:1
> action mirred egress redirect dev ifb0
>
>
> Anyways, I give up.
I understood you when you first posted. This is part of my testing.
Try also to redirect from the same device eth0 to eth0 etc and see some
more interesting things.
> Last time I've been running after you trying
> to fix the many bugs you leave behind. Ever noticed that whenever
> you add some new code it's someone else following up with tons of
> small bugfix patches having a hard time trying to figure out the
> actual intent.
You could always ask instead of making assumptions. And dont get me wrong, I
honestly do appreciate you fixing bugs. You have been great and i have
always thanked you - except in situations like this where you just
stress the hell out of me. Bugs will always be there.
Even God has bugs - but you are not fixing bugs in this case.
You are attempting to change architecture (which works just fine) in the way you
think it should work - and then point to something as a bug because it
doesnt work the way you think it should work.
This is a problem not just with you BTW, but with Patrick as well (although he has
gotten better lately). There is a huge difference for example when dealing with
Herbert. My approach in situations like this, which you dont have to follow, is
to ask first what the intent was then if i dont like the intent try to convince the
owner that there maybe better ways. Or why they are wrong.
To make my case, look at what you just did above in just the last 2 emails:
You made a claim there is a bug.
I asked you if you had really tested what you are pointing to (because i
know i test for that). You come back and make claims the bug is
elsewhere. This could go on forever typically - and infact it is throughout this
thread.
Didnt ask if this is perhaps the way it is supposed to work or what the
intent was. It has to be a bug and by golly you are fixing it. Put yourself in
my shoes.
> I'll just duplicate the code for my purpose, so
> much easier.
>
Well, I am sorry you feel that way. I dont even remember where it is that
we started.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: Patrick McHardy @ 2006-06-30 17:18 UTC (permalink / raw)
To: Thomas Graf; +Cc: jamal, David Miller, netdev
In-Reply-To: <20060630171348.GI14627@postel.suug.ch>
Thomas Graf wrote:
> * Thomas Graf <tgraf@suug.ch> 2006-06-30 18:32
>
>>Anyways, I give up. Last time I've been running after you trying
>>to fix the many bugs you leave behind. Ever noticed that whenever
>>you add some new code it's someone else following up with tons of
>>small bugfix patches having a hard time trying to figure out the
>>actual intent. I'll just duplicate the code for my purpose, so
>>much easier.
>
>
> There you go, leaves ifb broken as-is, at least prevents it
> from crashing randomly when the input_dev disappears.
That would be a pity. After this patch has been ACKed, could we start
over with the other bugs one at a time? I wasn't really able to gather
the problems from this thread, but some of the behaviour you mentioned
does seem questionable.
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: Thomas Graf @ 2006-06-30 17:13 UTC (permalink / raw)
To: jamal; +Cc: Patrick McHardy, David Miller, netdev
In-Reply-To: <20060630163229.GH14627@postel.suug.ch>
* Thomas Graf <tgraf@suug.ch> 2006-06-30 18:32
> Anyways, I give up. Last time I've been running after you trying
> to fix the many bugs you leave behind. Ever noticed that whenever
> you add some new code it's someone else following up with tons of
> small bugfix patches having a hard time trying to figure out the
> actual intent. I'll just duplicate the code for my purpose, so
> much easier.
There you go, leaves ifb broken as-is, at least prevents it
from crashing randomly when the input_dev disappears.
[NET]: Use interface index to keep input device information
Using the interface index instead of a direct reference
allows a safe usage beyond the scope where an interface
could disappear.
The old input_dev field was incorrectly made dependant
on CONFIG_NET_CLS_ACT in skb_copy().
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Index: net-2.6.git/include/linux/skbuff.h
===================================================================
--- net-2.6.git.orig/include/linux/skbuff.h
+++ net-2.6.git/include/linux/skbuff.h
@@ -181,7 +181,6 @@ enum {
* @sk: Socket we are owned by
* @tstamp: Time we arrived
* @dev: Device we arrived on/are leaving by
- * @input_dev: Device we arrived on
* @h: Transport layer header
* @nh: Network layer header
* @mac: Link layer header
@@ -192,6 +191,7 @@ enum {
* @data_len: Data length
* @mac_len: Length of link layer header
* @csum: Checksum
+ * @iif: Device we arrived on
* @local_df: allow local fragmentation
* @cloned: Head may be cloned (check refcnt to be sure)
* @nohdr: Payload reference only, must not modify header
@@ -228,7 +228,6 @@ struct sk_buff {
struct sock *sk;
struct skb_timeval tstamp;
struct net_device *dev;
- struct net_device *input_dev;
union {
struct tcphdr *th;
@@ -266,6 +265,7 @@ struct sk_buff {
data_len,
mac_len,
csum;
+ int iif;
__u32 priority;
__u8 local_df:1,
cloned:1,
Index: net-2.6.git/include/net/pkt_cls.h
===================================================================
--- net-2.6.git.orig/include/net/pkt_cls.h
+++ net-2.6.git/include/net/pkt_cls.h
@@ -352,14 +352,19 @@ tcf_change_indev(struct tcf_proto *tp, c
static inline int
tcf_match_indev(struct sk_buff *skb, char *indev)
{
+ int ret = 1;
+
if (indev[0]) {
- if (!skb->input_dev)
- return 0;
- if (strcmp(indev, skb->input_dev->name))
+ struct net_device *dev;
+
+ dev = dev_get_by_index(skb->iif);
+ if (!dev)
return 0;
+ ret = !strcmp(indev, dev->name);
+ dev_put(dev);
}
- return 1;
+ return ret;
}
#endif /* CONFIG_NET_CLS_IND */
Index: net-2.6.git/net/core/dev.c
===================================================================
--- net-2.6.git.orig/net/core/dev.c
+++ net-2.6.git/net/core/dev.c
@@ -1715,8 +1715,8 @@ static int ing_filter(struct sk_buff *sk
if (dev->qdisc_ingress) {
__u32 ttl = (__u32) G_TC_RTTL(skb->tc_verd);
if (MAX_RED_LOOP < ttl++) {
- printk("Redir loop detected Dropping packet (%s->%s)\n",
- skb->input_dev->name, skb->dev->name);
+ printk("Redir loop detected Dropping packet (%d->%s)\n",
+ skb->iif, skb->dev->name);
return TC_ACT_SHOT;
}
@@ -1749,8 +1749,8 @@ int netif_receive_skb(struct sk_buff *sk
if (!skb->tstamp.off_sec)
net_timestamp(skb);
- if (!skb->input_dev)
- skb->input_dev = skb->dev;
+ if (!skb->iif)
+ skb->iif = skb->dev->ifindex;
orig_dev = skb_bond(skb);
Index: net-2.6.git/net/core/skbuff.c
===================================================================
--- net-2.6.git.orig/net/core/skbuff.c
+++ net-2.6.git/net/core/skbuff.c
@@ -463,10 +463,10 @@ struct sk_buff *skb_clone(struct sk_buff
n->tc_verd = SET_TC_VERD(skb->tc_verd,0);
n->tc_verd = CLR_TC_OK2MUNGE(n->tc_verd);
n->tc_verd = CLR_TC_MUNGED(n->tc_verd);
- C(input_dev);
#endif
skb_copy_secmark(n, skb);
#endif
+ C(iif);
C(truesize);
atomic_set(&n->users, 1);
C(head);
Index: net-2.6.git/net/sched/act_api.c
===================================================================
--- net-2.6.git.orig/net/sched/act_api.c
+++ net-2.6.git/net/sched/act_api.c
@@ -156,9 +156,8 @@ int tcf_action_exec(struct sk_buff *skb,
if (skb->tc_verd & TC_NCLS) {
skb->tc_verd = CLR_TC_NCLS(skb->tc_verd);
- D2PRINTK("(%p)tcf_action_exec: cleared TC_NCLS in %s out %s\n",
- skb, skb->input_dev ? skb->input_dev->name : "xxx",
- skb->dev->name);
+ D2PRINTK("(%p)tcf_action_exec: cleared TC_NCLS in %d out %s\n",
+ skb, skb->iif, skb->dev->name);
ret = TC_ACT_OK;
goto exec_done;
}
Index: net-2.6.git/net/sched/act_mirred.c
===================================================================
--- net-2.6.git.orig/net/sched/act_mirred.c
+++ net-2.6.git/net/sched/act_mirred.c
@@ -207,7 +207,7 @@ bad_mirred:
skb2->tc_verd = SET_TC_FROM(skb2->tc_verd, at);
skb2->dev = dev;
- skb2->input_dev = skb->dev;
+ skb2->iif = skb->dev->ifindex;
dev_queue_xmit(skb2);
spin_unlock(&p->lock);
return p->action;
Index: net-2.6.git/drivers/net/ifb.c
===================================================================
--- net-2.6.git.orig/drivers/net/ifb.c
+++ net-2.6.git/drivers/net/ifb.c
@@ -158,19 +158,23 @@ static int ifb_xmit(struct sk_buff *skb,
stats->tx_packets++;
stats->tx_bytes+=skb->len;
- if (!from || !skb->input_dev) {
+ if (!from || !skb->iif) {
dropped:
dev_kfree_skb(skb);
stats->rx_dropped++;
return ret;
} else {
+ struct net_device *iif;
/*
* note we could be going
* ingress -> egress or
* egress -> ingress
*/
- skb->dev = skb->input_dev;
- skb->input_dev = dev;
+ iif = __dev_get_by_index(skb->iif);
+ if (!iif)
+ goto dropped;
+ skb->dev = iif;
+ skb->iif = dev->ifindex;
if (from & AT_INGRESS) {
skb_pull(skb, skb->dev->hard_header_len);
} else {
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: Patrick McHardy @ 2006-06-30 17:02 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: Thomas Graf, jamal, David Miller, netdev
In-Reply-To: <44A558CD.4020600@trash.net>
Patrick McHardy wrote:
> Nicolas Dichtel wrote:
>
>>Bit 8 of skb->tc_verd is set by IFB, so packet isn't reclassify.
>>This bit avoid the loop.
>
>
> It would, if something would actually set it.
>
> ~/src/kernel/linux-2.6$ grep NCLS -r net/
> net/core/dev.c: if (skb->tc_verd & TC_NCLS) {
> net/core/dev.c: skb->tc_verd = CLR_TC_NCLS(skb->tc_verd);
> net/sched/act_api.c: if (skb->tc_verd & TC_NCLS) {
> net/sched/act_api.c: skb->tc_verd = CLR_TC_NCLS(skb->tc_verd);
> net/sched/act_api.c: D2PRINTK("(%p)tcf_action_exec: cleared
> TC_NCLS in %s out %s\n",
> ~/src/kernel/linux-2.6$
>
> Am I missing something? Jamal, where is this bit supposed to be set?
OK, I'm apparently missing the ability to read :) Please disregard ..
^ permalink raw reply
* Re: [PATCH 2/3] [VLAN]: Update iif when receiving via VLAN device
From: Patrick McHardy @ 2006-06-30 17:01 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: Thomas Graf, jamal, David Miller, netdev
In-Reply-To: <44A52435.20909@6wind.com>
Nicolas Dichtel wrote:
> Bit 8 of skb->tc_verd is set by IFB, so packet isn't reclassify.
> This bit avoid the loop.
It would, if something would actually set it.
~/src/kernel/linux-2.6$ grep NCLS -r net/
net/core/dev.c: if (skb->tc_verd & TC_NCLS) {
net/core/dev.c: skb->tc_verd = CLR_TC_NCLS(skb->tc_verd);
net/sched/act_api.c: if (skb->tc_verd & TC_NCLS) {
net/sched/act_api.c: skb->tc_verd = CLR_TC_NCLS(skb->tc_verd);
net/sched/act_api.c: D2PRINTK("(%p)tcf_action_exec: cleared
TC_NCLS in %s out %s\n",
~/src/kernel/linux-2.6$
Am I missing something? Jamal, where is this bit supposed to be set?
^ permalink raw reply
* Re: [PATCH 0 of 39] ipath - bug fixes, performance enhancements,and portability improvements
From: Bryan O'Sullivan @ 2006-06-30 17:00 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: akpm, Roland Dreier, openib-general, linux-kernel, netdev
In-Reply-To: <20060630163108.GA24882@mellanox.co.il>
On Fri, 2006-06-30 at 19:31 +0300, Michael S. Tsirkin wrote:
> OK, next week I'll put these into my tree, too.
Thanks. The first 37 are in -mm; the last two you can drop until I sort
them out.
<b
^ 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