On Thu, Jul 28, 2005 at 04:23:18PM +0400, Alexey Kuznetsov wrote:
>
> > You're right. That makes it safe with preemption. However, netfilter
> > could still queue the packet which means the neigh entry and dev can
> > go away under it, right?
>
> Netfilter is very clean about this. Each hook ge
Hello!
> Just double checking (I think we are saying the same thing),
> that using ifindices and requiring refcounting for input_dev means you
> have to use __dev_get_by_index() on a per-packet basis.
The third check, I do not follow...
You just compare ifindex stored in skb (skb->iif) with ifin
Hello!
> You're right. That makes it safe with preemption. However, netfilter
> could still queue the packet which means the neigh entry and dev can
> go away under it, right?
Netfilter is very clean about this. Each hook gets two devices as _arguments_
(input and output, skb->dev is one of the
So 20 emails or so later ...
On Wed, 2005-27-07 at 08:43 -0700, Ben Greear wrote:
> I don't see a good reason for the feature, or at least nothing that
> justifies the work of trying to implement it.
You are the one who pointed the ifindex issue. What a waste of time.
> What benefits do you e
jamal wrote:
On Tue, 2005-26-07 at 09:54 -0700, Ben Greear wrote:
[..]
You will need to enforce that nothing else gets the index 34 while eth7 is
removed.
How do you do that?
Thats trivial if you assume there's one management app which most of the
router vendors implementing it have. It
On Tue, 2005-26-07 at 13:00 -0700, David S. Miller wrote:
>
> Calling __dev_get_by_index() at every classification check is quite
> silly and potentially expensive, so let's call using ifindex a last
> resort, yet correct, fix.
Just double checking (I think we are saying the same thing),
that usi
On Tue, 2005-26-07 at 09:54 -0700, Ben Greear wrote:
[..]
> You will need to enforce that nothing else gets the index 34 while eth7 is
> removed.
> How do you do that?
Thats trivial if you assume there's one management app which most of the
router vendors implementing it have. It will get tric
On Tue, Jul 26, 2005 at 10:09:19AM -0400, jamal wrote:
>
> > I think the answer is that it works by accident.
>
> But does it matter really? If say we close down the device,
> do we want to be stopped from closing it until all the packets have been
> transmitted? We purge the qdiscs iirc already
From: jamal <[EMAIL PROTECTED]>
Date: Tue, 26 Jul 2005 10:09:19 -0400
> PS:- Dave, I think all this holding refcount thing on the input_dev is
> complicating things. Lets just use the ifindex and forget about
> refcounting; worst is we will end up failing some classification
> somewhere - which if
From: jamal <[EMAIL PROTECTED]>
Date: Tue, 26 Jul 2005 10:00:08 -0400
> As an example, if eth7 appeared again, greearb-snmp by looking at lspci
> can validate it is still the same device type, some serial number output
> etc; it can also determine it was exactly the same card as before even
That'
jamal wrote:
On Mon, 2005-25-07 at 16:25 -0700, Ben Greear wrote:
jamal wrote:
Why not easily? It should be pretty trivial to tell user space "the
ifindex you want is in use"; return -EEXIST
If you have to deal with finding a free ifindex, how does it help you?
If you are going to use
Hello!
> But how can this possibly work for skb->dst'less packets (such as IPV4
> ARP generated frames)?
Consider this as an implicit argument to functions passing skbs
(that's why skb->dev can be killed btw). Caller is obliged to hold
reference to the device, when doing dev_queue_xmit(). The way
On Tue, 2005-26-07 at 21:54 +1000, Herbert Xu wrote:
> David S. Miller <[EMAIL PROTECTED]> wrote:
> >
> > But how can this possibly work for skb->dst'less packets (such as IPV4
> > ARP generated frames)?
>
> I think the answer is that it works by accident.
But does it matter really? If say we cl
On Mon, 2005-25-07 at 16:25 -0700, Ben Greear wrote:
> jamal wrote:
> I'm not suggesting to change current behaviour. I am suggesting that
> anyone who depends on ifindex for a primary key without checking the
> name is asking for trouble, since ifindex is transient with regard
> to device names.
David S. Miller <[EMAIL PROTECTED]> wrote:
>
> But how can this possibly work for skb->dst'less packets (such as IPV4
> ARP generated frames)?
I think the answer is that it works by accident.
Without preemption or weird netfilter targets that queue packets,
the packet will drop right through to
From: Thomas Graf <[EMAIL PROTECTED]>
Date: Tue, 26 Jul 2005 01:38:08 +0200
> Therefore I'd say stick to the device pointers for now to be on the
> safe side, in case we're missing a 4 bytes saving for cacheline
> reasons we can think about this again and defuse the worst case a
> bit by increasin
* David S. Miller <[EMAIL PROTECTED]> 2005-07-25 16:18
> From: Thomas Graf <[EMAIL PROTECTED]>
> Date: Tue, 26 Jul 2005 01:11:13 +0200
>
> > Not sure but as I see it:
> ...
> > The __dev_get_by_index() can be expensive with
> > more than a 1k interfaces but the hash generally
> > does well so I d
jamal wrote:
On Mon, 2005-25-07 at 15:49 -0700, Ben Greear wrote:
jamal wrote:
Name is not totaly sane either:
I think one of the fscked s/ware i have seen is pppd that tries
to recycle names immediately with the same name but different ifindex.
It should probably use ppp for name.
That w
From: Thomas Graf <[EMAIL PROTECTED]>
Date: Tue, 26 Jul 2005 01:11:13 +0200
> Not sure but as I see it:
...
> The __dev_get_by_index() can be expensive with
> more than a 1k interfaces but the hash generally
> does well so I don't see much of a problem.
Whilst this does save us 4 bytes on 64-bit
On Tue, 2005-26-07 at 00:56 +0200, Thomas Graf wrote:
> There is no code to set this flag at the moment so my question
> simply is what are your plans regarding this flag? It's dead code
> at the moment.
Ok, sorry, i misunderstood you then.
Shouldnt be dead for long. The dummy device will use it
* David S. Miller <[EMAIL PROTECTED]> 2005-07-25 15:56
> From: Thomas Graf <[EMAIL PROTECTED]>
> Date: Tue, 26 Jul 2005 00:35:05 +0200
>
> > We can also use ifindex and hold a reference. We won't access input_dev
> > too often so a __dev_get_by_ifindex() won't be too expensive given the
> > number
On Mon, 2005-25-07 at 15:54 -0700, David S. Miller wrote:
> We should probably just bite the bullet and do something like:
>
> static inline void skb_set_input_dev(struct sk_buff *skb, struct net_device
> *dev)
> {
> struct net_device *orig_dev = skb->input_dev;
>
> if (orig_dev)
>
On Mon, 2005-25-07 at 15:49 -0700, Ben Greear wrote:
> jamal wrote:
> > Name is not totaly sane either:
> > I think one of the fscked s/ware i have seen is pppd that tries
> > to recycle names immediately with the same name but different ifindex.
> > It should probably use ppp for name.
>
> That
* jamal <[EMAIL PROTECTED]> 2005-07-25 18:40
> On Tue, 2005-26-07 at 00:32 +0200, Thomas Graf wrote:
> > I was referring to the tc_verd & TC_NCLS check in tcf_action_exec()
> > which uses input_dev in a printk. I assume dave was referring to the
> > same code part. What is intended usage of TC_NCLS
From: Thomas Graf <[EMAIL PROTECTED]>
Date: Tue, 26 Jul 2005 00:32:32 +0200
> I was referring to the tc_verd & TC_NCLS check in tcf_action_exec()
> which uses input_dev in a printk. I assume dave was referring to the
> same code part.
Yes, I was.
-
To unsubscribe from this list: send the line "un
From: Thomas Graf <[EMAIL PROTECTED]>
Date: Tue, 26 Jul 2005 00:35:05 +0200
> We can also use ifindex and hold a reference. We won't access input_dev
> too often so a __dev_get_by_ifindex() won't be too expensive given the
> number of interfaces is not too big but that's another issue.
Since the
From: jamal <[EMAIL PROTECTED]>
Date: Mon, 25 Jul 2005 18:42:55 -0400
> Having centralized it like Dave is intending to makes it tricky on where
> to actually issue the hold/put.
We should probably just bite the bullet and do something like:
static inline void skb_set_input_dev(struct sk_buff *
jamal wrote:
If a device goes down, and comes back up with the same name
but a different ifindex, we should still match it with existing
rules based upon name.
The ifindex will change if you unload a module;
If you try hard enough you will hit a point where things will wrap
around; you ju
On Tue, 2005-26-07 at 00:35 +0200, Thomas Graf wrote:
> * jamal <[EMAIL PROTECTED]> 2005-07-25 18:28
> > So ifindex maybe the simplest way to go. If someone unloads a module
> > for the netdevice then loads it back or pops out a hotplug net card
> > and then pops it in - could we safely assume they
On Tue, 2005-26-07 at 00:32 +0200, Thomas Graf wrote:
> * jamal <[EMAIL PROTECTED]> 2005-07-25 18:22
> > > The code block which uses input_dev is dead, TC_NCLS is never
> > > set as of now. Jamal might have use for it though? Generally,
> > > tcf_action_exec() may be called on both ingress and egre
David S. Miller wrote:
From: Ben Greear <[EMAIL PROTECTED]>
Date: Mon, 25 Jul 2005 14:13:20 -0700
From what I remember, a new message could be added with a 32-bit
or so routing table identifier. It should be backwards compatible since
and old version of IP would never create or access the rou
* jamal <[EMAIL PROTECTED]> 2005-07-25 18:28
> So ifindex maybe the simplest way to go. If someone unloads a module
> for the netdevice then loads it back or pops out a hotplug net card
> and then pops it in - could we safely assume they are talking about
> another device? ;->
We can also use ifin
* jamal <[EMAIL PROTECTED]> 2005-07-25 18:22
> > The code block which uses input_dev is dead, TC_NCLS is never
> > set as of now. Jamal might have use for it though? Generally,
> > tcf_action_exec() may be called on both ingress and egress.
>
> I mentioned the meta action already; the dummy device
On Mon, 2005-25-07 at 15:19 -0700, David S. Miller wrote:
> The idea is to do the changes as a multi-step process :-)
>
> First, we do the input_dev initialization in a centralized
> location, then we investigate the legality of using ifindex.
>
> So we've done the first part, and are now discus
On Mon, 2005-25-07 at 22:09 +0200, Thomas Graf wrote:
> * David S. Miller <[EMAIL PROTECTED]> 2005-07-25 12:09
> > Ok, so can we agree on the following patch? ing_filter() should
> > never see a NULL ->input_dev, and we could add a BUG() check
> > there for that invariant if you want.
>
> This lo
From: jamal <[EMAIL PROTECTED]>
Date: Mon, 25 Jul 2005 18:14:45 -0400
> BTW, I just glanced at your suggested patch. If we are not going to use
> ifindex we aint saving the 4 bytes on 64 bit machines. And we have to
> keep refcounts and release devices.
The idea is to do the changes as a multi-st
On Mon, 2005-25-07 at 13:54 -0700, David S. Miller wrote:
> From: Thomas Graf <[EMAIL PROTECTED]>
> Date: Mon, 25 Jul 2005 22:16:21 +0200
>
> > * David S. Miller <[EMAIL PROTECTED]> 2005-07-25 13:14
> > > We removed INPUT_DEV from the meta ematch last week remember?
> >
> > Sure, but we can readd
From: Ben Greear <[EMAIL PROTECTED]>
Date: Mon, 25 Jul 2005 14:13:20 -0700
> From what I remember, a new message could be added with a 32-bit
> or so routing table identifier. It should be backwards compatible since
> and old version of IP would never create or access the routing tables > 0xFF,
>
David S. Miller wrote:
From: Ben Greear <[EMAIL PROTECTED]>
Date: Sun, 24 Jul 2005 23:25:53 -0700
When these other changes go in, is there any chance that
we can get support for more than 256 routing tables? That
can help make all of these virtual interfaces more useful
for some of us with st
From: Thomas Graf <[EMAIL PROTECTED]>
Date: Mon, 25 Jul 2005 22:16:21 +0200
> * David S. Miller <[EMAIL PROTECTED]> 2005-07-25 13:14
> > We removed INPUT_DEV from the meta ematch last week remember?
>
> Sure, but we can readd it once its clear how it will be used.
> What I meant is that the meta
From: Thomas Graf <[EMAIL PROTECTED]>
Date: Mon, 25 Jul 2005 22:05:21 +0200
> The meta ematch which will the primary user of input_dev
> provides both, filtering by ifindex and by name so I'm fine
> with input_dev being an ifindex.
We removed INPUT_DEV from the meta ematch last week remember?
-
T
* David S. Miller <[EMAIL PROTECTED]> 2005-07-25 13:14
> From: Thomas Graf <[EMAIL PROTECTED]>
> Date: Mon, 25 Jul 2005 22:05:21 +0200
>
> > The meta ematch which will the primary user of input_dev
> > provides both, filtering by ifindex and by name so I'm fine
> > with input_dev being an ifindex.
* David S. Miller <[EMAIL PROTECTED]> 2005-07-25 11:46
> There is, of course, still the issue of refcounting. If we go
> the ifindex route for skb->input_dev, it might make sense to
> translate device specifications in action rules into ifindex.
> But things could break if you down then up a devic
* David S. Miller <[EMAIL PROTECTED]> 2005-07-25 12:09
> Ok, so can we agree on the following patch? ing_filter() should
> never see a NULL ->input_dev, and we could add a BUG() check
> there for that invariant if you want.
This looks good to me, a input_dev==NULL means the packets
origin is loca
From: Ben Greear <[EMAIL PROTECTED]>
Date: Sun, 24 Jul 2005 23:25:53 -0700
> When these other changes go in, is there any chance that
> we can get support for more than 256 routing tables? That
> can help make all of these virtual interfaces more useful
> for some of us with strange routing fetis
From: Jamal Hadi Salim <[EMAIL PROTECTED]>
Date: Mon, 25 Jul 2005 09:50:33 -0400
>
> if (!skb->input_dev)
> skb->input_dev = skb->dev->ifindex;
> -
>
> We get rid of the printk and all other places that set input_dev except
> for mirred.
> Any new actions can continue to
From: Jamal Hadi Salim <[EMAIL PROTECTED]>
Date: Mon, 25 Jul 2005 09:50:33 -0400
> how about we set it in only ing_filter() if it is zero. i.e,
>
>
> if (!skb->input_dev)
> skb->input_dev = skb->dev->ifindex;
> -
Ok... how about we take this one step further? The idea
b
* Jamal Hadi Salim <[EMAIL PROTECTED]> 2005-07-25 09:50
> Thomas:
> bit 0 of tc_verd can be moved into cb;
Any reason why we don't see OK2MUNGE in the action itself but
have it triggered by setting MUNGED first?
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a m
On Sun, 2005-24-07 at 23:18 -0400, Jamal Hadi Salim wrote:
> > Posting a bunch of patches that explicitly set input_dev to dev
> > right before netif_rx() sort of further proves my point :-)
> >
>
> ;->
> Let me sleep it over and think about it - I am not thinking straight
> right now. I am back
David S. Miller wrote:
From: Thomas Graf <[EMAIL PROTECTED]>
Date: Sat, 23 Jul 2005 16:29:42 +0200
I think we should head for a 16bit ifindex at some point
or is there any reason why 65K interfaces wouldn't be
sufficient?
I think we are locked into using "s32" for this.
I can definitely im
On Sun, 2005-24-07 at 20:14 -0700, David S. Miller wrote:
> I see one place where input_dev could be something different,
> the mirred stuff. And in fact this ends up being the one
> place where an input_dev reference can escape the singular
> execution of the input packet path in softirq context
From: Jamal Hadi Salim <[EMAIL PROTECTED]>
Date: Sun, 24 Jul 2005 23:02:14 -0400
> On Sun, 2005-24-07 at 22:58 -0400, Jamal Hadi Salim wrote:
> > On Sun, 2005-24-07 at 19:02 -0700, David S. Miller wrote:
> >
> > I agree they will mean the same thing.
>
> I meant to say they will mean the same t
On Sun, 2005-24-07 at 22:58 -0400, Jamal Hadi Salim wrote:
> On Sun, 2005-24-07 at 19:02 -0700, David S. Miller wrote:
>
> I agree they will mean the same thing.
I meant to say they will mean the same thing in the simple case.
And this line below extrapolates that statement further
> The example
On Sun, 2005-24-07 at 19:02 -0700, David S. Miller wrote:
I agree they will mean the same thing.
The example i gave was to show where one meaning contradicts the other.
> And the current specific settings of input_dev has the following
> problems:
>
> 1) you cannot ingress classify on tunneling
From: Thomas Graf <[EMAIL PROTECTED]>
Date: Sat, 23 Jul 2005 16:29:42 +0200
> I think we should head for a 16bit ifindex at some point
> or is there any reason why 65K interfaces wouldn't be
> sufficient?
I think we are locked into using "s32" for this.
I can definitely imagine virtual interface
From: Jamal Hadi Salim <[EMAIL PROTECTED]>
Date: Sun, 24 Jul 2005 21:49:34 -0400
> The semantics of the two are very different. real_dev is in regards to
> stacked interfaces and input_dev is in reference to track what the last
> ingress interface was. So it is ambigous to have them to be the same
On Sun, 2005-24-07 at 18:00 -0700, David S. Miller wrote:
> > Signed-off-by: Jamal Hadi Salim <[EMAIL PROTECTED]>
>
> I have a better change in the wings that totally eliminates
> real_dev _AND_ input_dev completely, and passes them as
> parameters into pt->func() and ->enqueue() as it should hav
From: Patrick McHardy <[EMAIL PROTECTED]>
Date: Sat, 23 Jul 2005 18:54:13 +0200
> Let me propose again to just set it here for all cases. So far there
> hasn't been a single exception where indev is not the input device
> as seen by netif_rx(), and I don't expect any to come up. In any case
> you
From: Jamal Hadi Salim <[EMAIL PROTECTED]>
Date: Sat, 23 Jul 2005 09:32:07 -0400
> This is part of mission skb diet.
> Against git/davem/2.6.14 that was on vger 30 minutes back: It changes
> input_dev to be an ifindex so we dont bother holding devices.
> Would only cut a 4 byte fat on 64 bit ma
Thomas Graf wrote:
I think we should head for a 16bit ifindex at some point
or is there any reason why 65K interfaces wouldn't be
sufficient? We waste one bit at the moment to have simpler
error handling for all the functions returning indices.
Although I don't know anyone doing so, it would
Jamal Hadi Salim wrote:
On Sat, 2005-23-07 at 16:50 +0200, Thomas Graf wrote:
No, we use s32 at the moment with the limitation that negative indices
are illegal. We don't have to break the interfaces just because of this,
but since dev_new_ifindex() will be reusing gone indices if needed it
m
Jamal Hadi Salim wrote:
This is part of mission skb diet.
Against git/davem/2.6.14 that was on vger 30 minutes back: It changes
input_dev to be an ifindex so we dont bother holding devices.
Would only cut a 4 byte fat on 64 bit machines.
Signed-off-by: Jamal Hadi Salim <[EMAIL PROTECTED]>
L
On Sat, 2005-23-07 at 16:50 +0200, Thomas Graf wrote:
> No, we use s32 at the moment with the limitation that negative indices
> are illegal. We don't have to break the interfaces just because of this,
> but since dev_new_ifindex() will be reusing gone indices if needed it
> might be possible to
* Jamal Hadi Salim <[EMAIL PROTECTED]> 2005-07-23 10:37
> ifindex has to be 32 bit because most management apps (SNMP etc) expect
> it to be 32 bit. I havent scanned the code but making int actually
> doesnt seem right - unless some code is expecting it to be a -ve number
> somewhere; it should be
On Sat, 2005-23-07 at 16:29 +0200, Thomas Graf wrote:
> I think we should head for a 16bit ifindex at some point
> or is there any reason why 65K interfaces wouldn't be
> sufficient? We waste one bit at the moment to have simpler
> error handling for all the functions returning indices.
Strange,
* Jamal Hadi Salim <[EMAIL PROTECTED]> 2005-07-23 09:32
> @@ -205,7 +205,6 @@ struct sk_buff {
> struct sock *sk;
> struct timeval stamp;
> struct net_device *dev;
> - struct net_device *input_dev;
> struct net_device *real_dev;
>
This is part of mission skb diet.
Against git/davem/2.6.14 that was on vger 30 minutes back: It changes
input_dev to be an ifindex so we dont bother holding devices.
Would only cut a 4 byte fat on 64 bit machines.
Signed-off-by: Jamal Hadi Salim <[EMAIL PROTECTED]>
cheers,
jamal
diff --git a
67 matches
Mail list logo
