IP layer bug?
Hello!
2.4.x kernel. have not tried 2.2
I just found somethig, I believe is kernel bug.
I am working with usbnet.c driver, which stores some of its internal state in sk_buff.cb area. But once such skb passed to upper layer with netif_rx, net/ipv4/ip_input.c reuses content of cb (line #345), and all packets that should go outside of beyond hosts we have direct routes to, fails, because we think, they have source routing enabled.
For now I workarounded it with filling skb->cb with zeroes before netif_rx(), but I believe it is a kludge and networking layer should be fixed instead.
Thank you.
Bye,
Oleg
Hi
Quoted 8 lines · Oleg Drokin
Hello!
2.4.x kernel. have not tried 2.2
I just found somethig, I believe is kernel bug.
I am working withusbnet.cdriver, which stores some of its internal state in sk_buff.cb area. But once such skb passed to upper layer withnetif_rx,net/ipv4/ip_input.creuses content of cb (line #345),
ip_options_compile() when called with first argument NULL resets cb to 0. This is probably because the cb is supposed to be used IP and above. The underlying layer(link and phy) could be anything so where from the ip_options should start will depend upon the underlying layer.
Quoted 17 lines
and all packets that should go outside of beyond hosts
we have direct routes to, fails, because we think, they have source routing enabled.
For now I workarounded it with fillingskb->cbwith zeroes beforenetif_rx(), but I believe it is a kludge and networking layer should be fixed instead.Thank you.
Bye,
Oleg
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Signature · 2 lines
Regards, Manoj Sontakke
Hello!
Quoted 7 lines · Manoj Sontakke
Quoted 6 lines
2.4.x kernel. have not tried 2.2
I just found somethig, I believe is kernel bug.
I am working withusbnet.cdriver, which stores some of its internal state in sk_buff.cb area. But once such skb passed to upper layer withnetif_rx,net/ipv4/ip_input.creuses content of cb (line #345),
ip_options_compile()when called with first argument NULL resets cb to 0.
I have found that already.
Quoted 1 line
This is probably because the cb is supposed to be used IP and above. The
Sure.
Quoted 2 lines
underlying layer(link and phy) could be anything so where from the
ip_optionsshould start will depend upon the underlying layer.
But here's the problem!
If I won't zero cb in my driver before netif_rx() call, IP layer thinks that all my packets have various ip options set (source routing most notable)
Bye,
Oleg
Hi,
Quoted 13 lines · Oleg Drokin
Hello!
Quoted 7 lines · Manoj Sontakke
Quoted 6 lines
2.4.x kernel. have not tried 2.2
I just found somethig, I believe is kernel bug.
I am working withusbnet.cdriver, which stores some of its internal state in sk_buff.cb area. But once such skb passed to upper layer withnetif_rx,net/ipv4/ip_input.creuses content of cb (line #345),
ip_options_compile()when called with first argument NULL resets cb to 0.I have found that already.
Quoted 1 line
This is probably because the cb is supposed to be used IP and above. The
Quoted 8 lines
Sure.
Quoted 2 lines
underlying layer(link and phy) could be anything so where from the
ip_optionsshould start will depend upon the underlying layer.But here's the problem!
If I won't zero cb in my driver beforenetif_rx()call, IP layer thinks that all my packets have various ip options set (source routing most notable)
U can set cb to zero, but u also plan to use cb for storing ur data. If that happens then u need to modify the way the macro IPCB (probably in net/ip.h) is defined. Also make sure to increase the size of cb to accomodate ur data. This will solve ur problem but making this general (part of the standard kernel code) needs a lot of work in the ip layer. The cb is also used by TCP. The same needs to be done in IP layer but this will again largly depend on the layers below and hence the complexity.
The alternative to this is that u can add another buffer like cb in sk_buff.
Manoj Sontakke
Hello!
Quoted 9 lines · Manoj Sontakke
Quoted 8 lines
Quoted 1 line
ip_options_compile()when called with first argument NULL resets cb to 0.I have found that already.
Quoted 2 lines
underlying layer(link and phy) could be anything so where from the
ip_optionsshould start will depend upon the underlying layer.But here's the problem!
If I won't zero cb in my driver beforenetif_rx()call, IP layer thinks that all my packets have various ip options set (source routing most notable)U can set cb to zero, but u also plan to use cb for storing ur data. If
I use it for storing data, but when I do netif_rx() on skb, I cannot touch it anymore, so no data there is belong to me, and if I read that code in ip_input.c, it gets zeroed.
BTW, I found that it's not ip_input.c that sends me 'source routing rejected' message, because no message is logged. It might be some other place. But anyway something is wrong in IP layer.
Quoted 3 lines
that happens then u need to modify the way the macro IPCB (probably in
net/ip.h) is defined. Also make sure to increase the size of cb to accomodate ur data. This will solve ur problem but making this
My data perfectly fits in cb of current size.
Quoted 4 lines
general (part of the standard kernel code) needs a lot of work in the ip layer. The cb is also used by TCP. The same needs to be done in IP layer but this will again largly depend on the layers below and hence the complexity.
But comment in skbuff.h claims that any owner of skb can use cb for its own needs, till it passes ownership to whoever else.
Quoted 2 lines
The alternative to this is that u can add another buffer like cb in
sk_buff.
What for?
Bye,
Oleg
Hello!
Quoted 2 lines
For now I workarounded it with filling
skb->cbwith zeroes beforenetif_rx(),
This is right. For another examples look into tunnels.
Quoted 1 line
but I believe it is a kludge and networking layer should be fixed instead.
No.
alloc_skb() creates skb with clean cb. ip_rcv() and other protocol handlers do not redo this work. If device uses cb internally, it must clear it before handing skb to netif_rx().
Alexey
Hello!
Quoted 3 lines · On Fri, Mar 30, 2001 at 09:13:40PM +0400, [email protected] wrote:
Quoted 2 lines
For now I workarounded it with filling
skb->cbwith zeroes beforenetif_rx(),This is right. For another examples look into tunnels.
Hm. But comment in linux/skbuff.h says:
/*
* This is the control buffer. It is free to use for every
* layer. Please put your private variables there. If you
* want to keep them across layers you have to do a skb_clone()
* first. This is owned by whoever has the skb queued ATM. */
Which does not imply I should clear buffer after I am passing ownership.
Quoted 6 lines
Quoted 1 line
but I believe it is a kludge and networking layer should be fixed instead.
No.
alloc_skb()creates skb with clean cb.ip_rcv()and other protocol handlers do not redo this work. If device uses cb internally, it must clear it before handing skb tonetif_rx().
Why not document it somewhere, so that others will not fall into the same trap?
Bye,
Oleg
Hello!
Quoted 1 line
Hm. But comment in
linux/skbuff.hsays:
The comment is about more difficult case: transmit path, where cb is used both by top level protocol and lower layers:
f.e. TCP -> IP -> device. cb is dirty from the moment of skb creation in this case.
Also, note that the second sentence in the comment is obsolete. Passing not cloned skbs between layers is strongly deprecated practice (I hope it is not used in any place) and cb of skb entering to lower layer is property of the layer.
RX path is simpler: cb must be kept clean, that's all.
General rule is minimization redundant clearings of the area.
Quoted 1 line
Why not document it somewhere, so that others will not fall into the same trap?
Indeed. 8) You got the experience, which you expect to be useful for people, it is time to prepare some note recording this. 8)
Alexey
Hello!
Quoted 4 lines · On Sat, Mar 31, 2001 at 07:32:48PM +0400, [email protected] wrote:
General rule is minimization redundant clearings of the area.
Quoted 1 line
Why not document it somewhere, so that others will not fall into the same trap?
Indeed. 8) You got the experience, which you expect to be useful for people, it is time to prepare some note recording this. 8)
I cannot think of something better that piece below, so I think you may want to change it, anyway ;)
linux/skbuff.h renamed from linux/skbuff.h.orig +4 -1
@@ -102,7 +102,10 @@* This is the control buffer. It is free to use for every * layer. Please put your private variables there. If you * want to keep them across layers you have to do a skb_clone() -* first. This is owned by whoever has the skb queued ATM. +* first (which is a must, anyway). This is owned by whoever +* has the skb queued ATM. +* Driver writers: notice you should zero cb before netif_rx() +* if you used it. */ char cb[48 ];
Bye,
Oleg