Re: Proposed interface for per-packet mesh-ttl
On Fri, 27 Jul 2007 18:22:22 -0400 Dan Williams [EMAIL PROTECTED] wrote: On Fri, 2007-07-27 at 20:56 +0100, Christoph Hellwig wrote: On Tue, Jul 03, 2007 at 12:29:24PM -0700, Javier Cardona wrote: I'm currently working on per-packet mesh ttl. My plan is to register new mesh sockopts through netfilter. The user interface will be: NACK. Drivers should never add sockopt, and drivers should not abuse netfilter hooks. Do you have a suggestion for a better way to approach per-packet options that userspace programs can set? Thanks, Dan In this case perhaps you can have a table that maps skb-priority to mesh ttl? priorty can already by handled by existing setsockopt calls, and modified by netfilter and QoS managements. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC/Progress update: e1000e pci-express -only e1000 driver
Kok, Auke [EMAIL PROTECTED] writes: * Removed all multi-queue code Why? With David's new multi NAPI work it will finally make sense, won't it? -Andi (who would finally like to see a driver which is able to process incoming packets and TX completion interrupts on multiple CPUs) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFX]: napi_struct V3
I've been on vacation so I missed most of this thread. I'm just catching up now... Ok I converted everything with Rusty's suggestion to move napi_struct out of net_device, this was mostly mechanical but some devices took some unanticipated amount of work. Actually you missed drivers/infiniband/ulp/ipoib, which has used NAPI since 2.6.22. I can take a stab at the conversion soon if you want... Another area of consternation are drivers that were using netif_rx_reschedule(), as that interface was removed because it doesn't fit well with the caller managing the dev-quota et al. I left race conditions in the drivers that were using that interface, but they should still basically work nonetheless. ...but this is a problem for IPoIB. The underlying IB stuff only allows us to register what is essentially a one-shot edge-triggered interrupt. So there is a race between calling netif_rx_complete() and calling ib_req_notify_cq() (which enables the interrupt), since the interrupt might never happen if a packet arrives between the two calls. In the current driver we are OK because ib_req_notify_cq() can return a hint if an event was missed, but to use this hint we need a way to restart the NAPI poll without getting into trouble if the interrupt handler calls netif_rx_schedule() too. Thanks, Roland - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC]: napi_struct V4
Most drivers are in good shape, although some still have very questionable netif_rx_complete() handling, in that racy area that Rusty and myself were discussing today. My inclination is to wrap those sequences around with an IRQ safe spinlock to fix the race provably, and then if driver authors want to optimize that away with techniques like those that tg3, bnx2, sky2, skge et al. use, that's fine but can be done later. Ouch... that extra lock seems pretty expensive. Also I'm having a hard time understanding how the techniques you're alluding to apply to devices that may miss events when enabling interrupts; the drivers you mention all seem to be for devices that didn't have the race and didn't use netif_rx_reschedule() in the old NAPI world. Can you provide a little more detail on how the lock could be optimized away? Thanks, Roland - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
Hi, I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ). ... net/core/netpoll.c: In function 'netpoll_poll': net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller' net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller' net/core/netpoll.c: In function 'netpoll_setup': net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller' make[2]: *** [net/core/netpoll.o] Error 1 make[1]: *** [net/core] Error 2 make: *** [net] Error 2 make: *** Waiting for unfinished jobs ... I think is because KGDBOE selects just NETPOLL. Regards, Gabriel - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
small bug in tcp
When application closes socket with unread data in receive buffer, tcp stack sends rst packet from the wrong source port, not the source port of the socket being closed. This is the same problem that was described in my first post, witch unfortunately nobody cared to look into. This problem appeared in 2.4.0-test9-pre3 and is still present in kernel. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
pcnet32 79C972 doesn't survive suspend to RAM
Hallo, I tried suspend to RAM on my desktop. Surprisingly near everything worked after the wakeup, except for the pcnet32 PCI card. Kernel is 2.6.23-rc1-git4 on x86_64. Bootup: pcnet32.c:v1.33-NAPI 27.Jun.2006 [EMAIL PROTECTED] IOAPIC[0]: Set routing entry (2-21 - 0x81 - IRQ 21 Mode:1 Active:1) ACPI: PCI Interrupt :05:00.0[A] - GSI 21 (level, low) - IRQ 21 pcnet32: PCnet/FAST+ 79C972 at 0x1100, 00 a0 d2 18 9d 21 tx_start_pt(0x0c00):~220 bytes, BCR18(9861):BurstWrEn BurstRdEn NoUFlow SRAMSIZE=0x1700, SRAM_BND=0x0800, assigned IRQ 21. pcnet32: Found PHY 7810:0003 at address 1. eth1: registered as PCnet/FAST+ 79C972 pcnet32: 1 cards_found. After the S3 wakeup I get: NETDEV WATCHDOG: eth4: transmit timed out eth4: transmit timed out, status 0053, resetting. NETDEV WATCHDOG: eth4: transmit timed out eth4: transmit timed out, status 0053, resetting. NETDEV WATCHDOG: eth4: transmit timed out eth4: transmit timed out, status 0053, resetting. NETDEV WATCHDOG: eth4: transmit timed out eth4: transmit timed out, status 0053, resetting. NETDEV WATCHDOG: eth4: transmit timed out eth4: transmit timed out, status 0053, resetting. NETDEV WATCHDOG: eth4: transmit timed out eth4: transmit timed out, status 0053, resetting. ... going on ... And no packets are getting received either. lspci -vvvxxx in the broken state: 05:00.0 Ethernet controller: Advanced Micro Devices [AMD] 79c970 [PCnet32 LANCE] (rev 36) Subsystem: Allied Telesyn International AT-2700TX 10/100 Fast Ethernet Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium TAbort- TAbort- MAbort- SERR- PERR- Latency: 32 (6000ns min, 6000ns max) Interrupt: pin A routed to IRQ 21 Region 0: I/O ports at 1100 [size=32] Region 1: Memory at e8004900 (32-bit, non-prefetchable) [size=32] Expansion ROM at fff0 [disabled] [size=1M] Capabilities: [40] Power Management version 1 Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+) Status: D0 PME-Enable- DSel=0 DScale=1 PME- 00: 22 10 00 20 07 00 90 02 36 00 00 02 00 20 00 00 10: 01 11 00 00 00 49 00 e8 00 00 00 00 00 00 00 00 20: 00 00 00 00 00 00 00 00 00 00 00 00 59 12 00 27 30: 00 00 f0 ff 40 00 00 00 00 00 00 00 0b 01 18 18 40: 01 00 11 fe 00 20 00 14 00 00 00 00 00 00 00 00 50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Interrupt is not shared: 21: 13292 0 IO-APIC-fasteoi eth4 -Andi - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C [EMAIL PROTECTED] wrote: Hi, I got this compile error with a randconfig ( http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ). ... net/core/netpoll.c: In function 'netpoll_poll': net/core/netpoll.c:155: error: 'struct net_device' has no member named 'poll_controller' net/core/netpoll.c:159: error: 'struct net_device' has no member named 'poll_controller' net/core/netpoll.c: In function 'netpoll_setup': net/core/netpoll.c:670: error: 'struct net_device' has no member named 'poll_controller' make[2]: *** [net/core/netpoll.o] Error 1 make[1]: *** [net/core] Error 2 make: *** [net] Error 2 make: *** Waiting for unfinished jobs ... I think is because KGDBOE selects just NETPOLL. Looks like it. Select went and selected NETPOLL and NETPOLL_TRAP but things like CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset. `select' remains evil. Something like this.. --- a/lib/Kconfig.kgdb~kgdb-kconfig-fix +++ a/lib/Kconfig.kgdb @@ -175,8 +175,7 @@ endchoice config KGDBOE tristate KGDB: On ethernet if !KGDBOE_NOMODULE depends on m KGDB - select NETPOLL - select NETPOLL_TRAP + depends on NETPOLL_TRAP NET_POLL_CONTROLLER help Uses the NETPOLL API to communicate with the host GDB via UDP. In order for this to work, the ethernet interface specified must _ - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC]: napi_struct V4
David Miller wrote: From: Jeff Garzik [EMAIL PROTECTED] Date: Wed, 25 Jul 2007 22:00:31 -0400 David Miller wrote: From: Jeff Garzik [EMAIL PROTECTED] Date: Wed, 25 Jul 2007 21:55:08 -0400 I don't see any logic to your request, only added overhead for no reason. There may be some flawed logic in what Stephen stated, but the change really is needed. It must be atomic to execute the: enable_interrupts(); netif_rx_complete(); sequence wrt. the same code path in the interrupt handler. Sure. And how did the existing code fail to achieve that? The interrupt handler can run on another cpu in betwen those two statements, running the NAPI test-and-do-something operations in parallel with the netif_rx_complete() which causes problems as Rusty and I discussed yesterday. That's a performance/parallelization regression from current NAPI :( Jeff - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC/Progress update: e1000e pci-express -only e1000 driver
Jeff Garzik wrote: Andi Kleen wrote: Kok, Auke [EMAIL PROTECTED] writes: * Removed all multi-queue code Why? With David's new multi NAPI work it will finally make sense, won't it? It will come back, most definitely. Not speaking for Auke, but I'm pretty sure this falls under the category of start small and simple, get it into the tree rapidly. on top of that it's trivial to add it back, is currently not even used (!) and just a remnant of earlier efforts. PJ is working on getting a consistent multiqueue patch out and he will (once this driver is ready) write a proper patch to add it back after the driver gets merged. Auke - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: NETPOLL=y , NETDEVICES=n compile error ( Re: 2.6.23-rc1-mm1 )
Andrew Morton wrote:
On Sat, 28 Jul 2007 17:44:45 +0200 Gabriel C [EMAIL PROTECTED] wrote:
Hi,
I got this compile error with a randconfig (
http://194.231.229.228/MM/randconfig-auto-82.broken.netpoll.c ).
...
net/core/netpoll.c: In function 'netpoll_poll':
net/core/netpoll.c:155: error: 'struct net_device' has no member named
'poll_controller'
net/core/netpoll.c:159: error: 'struct net_device' has no member named
'poll_controller'
net/core/netpoll.c: In function 'netpoll_setup':
net/core/netpoll.c:670: error: 'struct net_device' has no member named
'poll_controller'
make[2]: *** [net/core/netpoll.o] Error 1
make[1]: *** [net/core] Error 2
make: *** [net] Error 2
make: *** Waiting for unfinished jobs
...
I think is because KGDBOE selects just NETPOLL.
Looks like it.
Select went and selected NETPOLL and NETPOLL_TRAP but things like
CONFIG_NETDEVICES and CONFIG_NET_POLL_CONTROLLER remain unset. `select'
remains evil.
Something like this..
--- a/lib/Kconfig.kgdb~kgdb-kconfig-fix
+++ a/lib/Kconfig.kgdb
@@ -175,8 +175,7 @@ endchoice
config KGDBOE
tristate KGDB: On ethernet if !KGDBOE_NOMODULE
depends on m KGDB
- select NETPOLL
- select NETPOLL_TRAP
+ depends on NETPOLL_TRAP NET_POLL_CONTROLLER
help
Uses the NETPOLL API to communicate with the host GDB via UDP.
In order for this to work, the ethernet interface specified must
_
That doesn't fix it. With that patch an 'make oldconfig' all NETPOLL stuff gone
and we end up with :
...
drivers/built-in.o: In function `option_setup':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:160: undefined
reference to `netpoll_parse_options'
drivers/built-in.o: In function `configure_kgdboe':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:183: undefined
reference to `netpoll_setup'
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:189: undefined
reference to `netpoll_cleanup'
drivers/built-in.o: In function `eth_post_exception_handler':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:119: undefined
reference to `netpoll_set_trap'
drivers/built-in.o: In function `eth_pre_exception_handler':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:111: undefined
reference to `netpoll_set_trap'
drivers/built-in.o: In function `eth_flush_buf':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:138: undefined
reference to `netpoll_send_udp'
drivers/built-in.o: In function `eth_get_char':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:127: undefined
reference to `netpoll_poll'
drivers/built-in.o: In function `cleanup_kgdboe':
/work/crazy/linux-git/MM/linux-2.6.23-rc1/drivers/net/kgdboe.c:217: undefined
reference to `netpoll_cleanup'
make: *** [.tmp_vmlinux1] Error 1
...
If I get that right select is needed here because all NETPOLL{_*} depends on
if NETDEVICES if NET_ETHERNET.
Also doing
...
select NETPOLL_TRAP
select NETPOLL
select NET_POLL_CONTROLLER
...
makes the driver happy and everything compiles fine.
I think there may be a logical issue ( again if I got it right ).
We need some ethernet card to work with kgdboe right ? but we don't have any if
!NETDEVICES !NET_ETHERNET.
So maybe some ' depends on ... NETDEVICES!=n NET_ETHERNET!=n ' is needed
too ?
( really sory if I said something stupid these Kconfig depends are not really
easy to figure for me )
Gabriel
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB Pegasus driver - avoid a potential NULL pointer dereference.
Hello,
This patch makes sure we don't dereference a NULL pointer in
drivers/net/usb/pegasus.c::write_bulk_callback() in the initial
struct net_device *net = pegasus-net; assignment.
The existing code checks if 'pegasus' is NULL and bails out if
it is, so we better not touch that pointer until after that check.
Please consider merging.
Signed-off-by: Jesper Juhl [EMAIL PROTECTED]
---
drivers/net/usb/pegasus.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index a05fd97..04cba6b 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -768,11 +768,13 @@ done:
static void write_bulk_callback(struct urb *urb)
{
pegasus_t *pegasus = urb-context;
- struct net_device *net = pegasus-net;
+ struct net_device *net;
if (!pegasus)
return;
+ net = pegasus-net;
+
if (!netif_device_present(net) || !netif_running(net))
return;
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
netdevice queueing / sendmsg issue?
Hi,
I have noticed an unexpected behaviour of a userland program sending
packets with AF_PACKET through a network device driver. The problem
is that the userland program waits on sock_wait_for_wmem() for a long
time even if the transmitter is ready and all skb packets have been
transmitted and freed by the driver. Perhaps some clues?
Does it work as designed?
The driver is actually ARM Intel IXP425 Ethernet doing bus mastering
TX, it basically does:
xmit()
{
send_skb_to_hw(skb);
if (no_more_tx_skb_slots) /* there are 16 TX skb slots total */
netif_stop_queue(dev);
return NETDEV_TX_OK;
}
xmit_ready_irq()
{
count = free_tx_skb_slots;
while (packets_transmitted) {
dev_kfree_skb_irq(get_skb_from_hw());
free_tx_skb_slot();
}
if (count == 0)
netif_start_queue(dev);
}
Now the userland program does something like:
struct sockaddr_ll tx_addr;
ip_sock = socket(PF_INET, SOCK_DGRAM, IPPROTO_IP);
strcpy(ifr.ifr_name, eth0);
ioctl(ip_sock, SIOCGIFINDEX, ifr);
memset(tx_addr, 0, sizeof(tx_addr));
tx_addr.sll_family = AF_PACKET;
tx_addr.sll_protocol = htons(ETH_P_ALL);
tx_addr.sll_ifindex = ifr-ifr_ifindex;
tx_sock = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL))
while (1) {
sendto(tx_sock, valid_packet_data, 1514, 0, tx_addr,
sizeof(tx_addr));
print('X');
}
The userland program sends multiple packets and then stops for a period
of several seconds.
What does it wait for?
It seems it's waiting in sock_wait_for_wmem(), at the end of
sock_alloc_send_pskb():
(schedule+0x0/0x6a0) from (schedule_timeout+0x90/0xd0)
(schedule_timeout+0x0/0xd0) from (sock_alloc_send_skb+0x178/0x268)
r7:c6d01d2c r6:7fff r5:c6d0 r4:c6c13800
(sock_alloc_send_skb+0x0/0x268) from (packet_sendmsg+0x100/0x28c)
(packet_sendmsg+0x0/0x28c) from (sock_sendmsg+0xb4/0xe4)
(sock_sendmsg+0x0/0xe4) from (sys_sendto+0xc8/0xf0)
r9:c7b5c500 r8:beeb6dac r7:05ea r6:c73a5580 r5:c6d01e9c r4:
(sys_sendto+0x0/0xf0) from (sys_socketcall+0x154/0x1f4)
(sys_socketcall+0x0/0x1f4) from (ret_fast_syscall+0x0/0x2c)
r4:0014
The sequence of events from the device driver POV is:
...
xmit entering and using last skb slot
xmit queue full, netif_stop_queue(dev);
xmit exiting
(now the userland program waits)
xmit_ready_irq entering
xmit_ready_irq dev_kfree_skb_irq()
xmit_ready_irq xmit ready, netif_start_queue(dev);
xmit_ready_irq exiting
(now the TX restarts and the userland program sends another packets)
The above is repeated multiple times, then:
xmit entering and using last skb slot
xmit queue full, netif_stop_queue(dev);
xmit_ready_irq entering
xmit_ready_irq dev_kfree_skb_irq() (1 slot empty and ready for TX)
xmit_ready_irq xmit ready, netif_start_queue(dev);
xmit_ready_irq
xmit_ready_irq dev_kfree_skb_irq() (2 slots empty)
...
xmit_ready_irq dev_kfree_skb_irq() (15 slots empty)
xmit_ready_irq
xmit_ready_irq dev_kfree_skb_irq() (all 16 slots empty)
xmit_ready_irq exiting
(transmitter idle, but the userland program doesn't wake up)
The xmit() is not called again for several seconds, despite
netif_start_queue(dev) called from IRQ handler, all TX skb slots are
ready to be used for transmit.
I wonder if it's dev_kfree_skb_irq() which should but fails to wake
the thing up?
Doing echo 197665 /proc/sys/net/core/wmem_default or
echo 52824 /proc/sys/net/core/wmem_default apparently
fixes the problem, anything 197665 and = 52825 doesn't.
197665 = 65 * 3041, 52825 = 25 * 2113.
Doing echo 25560 /proc/sys/net/core/wmem_default causes the driver
to never become TX queue full (IOW max 15 skb being transmitted),
25561 allows for TX queue full.
25560 = 16 * 1597.5.
--
Krzysztof Halasa
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB Pegasus driver - avoid a potential NULL pointer dereference.
Hi,
On 7/29/07, Jesper Juhl [EMAIL PROTECTED] wrote:
Hello,
This patch makes sure we don't dereference a NULL pointer in
drivers/net/usb/pegasus.c::write_bulk_callback() in the initial
struct net_device *net = pegasus-net; assignment.
The existing code checks if 'pegasus' is NULL and bails out if
it is, so we better not touch that pointer until after that check.
[...]
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index a05fd97..04cba6b 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -768,11 +768,13 @@ done:
static void write_bulk_callback(struct urb *urb)
{
pegasus_t *pegasus = urb-context;
- struct net_device *net = pegasus-net;
+ struct net_device *net;
if (!pegasus)
return;
+ net = pegasus-net;
+
if (!netif_device_present(net) || !netif_running(net))
return;
Is it really possible that we're called into this function with
urb-context == NULL? If not, I'd suggest let's just get rid of
the check itself, instead.
Satyam
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB Pegasus driver - avoid a potential NULL pointer dereference.
On 29/07/07, Satyam Sharma [EMAIL PROTECTED] wrote:
Hi,
On 7/29/07, Jesper Juhl [EMAIL PROTECTED] wrote:
Hello,
This patch makes sure we don't dereference a NULL pointer in
drivers/net/usb/pegasus.c::write_bulk_callback() in the initial
struct net_device *net = pegasus-net; assignment.
The existing code checks if 'pegasus' is NULL and bails out if
it is, so we better not touch that pointer until after that check.
[...]
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index a05fd97..04cba6b 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -768,11 +768,13 @@ done:
static void write_bulk_callback(struct urb *urb)
{
pegasus_t *pegasus = urb-context;
- struct net_device *net = pegasus-net;
+ struct net_device *net;
if (!pegasus)
return;
+ net = pegasus-net;
+
if (!netif_device_present(net) || !netif_running(net))
return;
Is it really possible that we're called into this function with
urb-context == NULL? If not, I'd suggest let's just get rid of
the check itself, instead.
I'm not sure. I am not very familiar with this code. I just figured
that moving the assignment is potentially a little safer and it is
certainly no worse than the current code, so that's a safe and
potentially benneficial change. Removing the check may be safe but I'm
not certain enough to make that call...
--
Jesper Juhl [EMAIL PROTECTED]
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2/3] 2.6.23-rc1: known regressions v2
On Friday 27 July 2007, Michal Piotrowski wrote: IDE Subject : ide problems: 2.6.22-git17 working, 2.6.23-rc1* is not References : http://lkml.org/lkml/2007/7/27/298 Last known good : ? Submitter : dth [EMAIL PROTECTED] Caused-By : ? Handled-By : ? Status : unknown No IDE changes after 2.6.22-git17. Despite this I will try to follow this bugreport. Thanks, Bart - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] amso1100: QP init bug in amso driver
thanks, applied. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Merge the Sonics Silicon Backplane subsystem
On Friday 27 July 2007 16:12, Andrew Morton wrote: On Fri, 27 Jul 2007 21:43:59 +0200 Michael Buesch [EMAIL PROTECTED] wrote: Sure, but why is the locking interruptible rather than plain old mutex_lock()? Hm, well. We hold this mutex for several seconds, as writing takes this long. So I simply thought it was worth allowing the waiter to interrupt here. If you say that's not an issue, I'll be happy to use mutex_lock() and reduce code complexity in this area. So.. is that what the _interruptible() is for? To allow an impatient user to ^c a read? If so, that sounds reasonable. It's worth a comment explaining these decisions to future readers, because it is hard to work out this sort of thinking just from the bare C code. I think most of sysfs -show() and -store() implementations use _interruptible() variant to allow user to interrupt and return early. -- Dmitry - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sb1000: prevent a potential NULL pointer dereference in sb1000_dev_ioctl()
On 29/07/07 00:02 +0200, Jesper Juhl wrote: Hi, Here's a small patch, prompted by a find by the Coverity checker, that removes a potential NULL pointer dereference from drivers/net/sb1000.c::sb1000_dev_ioctl(). The checker spotted that we do a NULL test of 'dev', yet we dereference the pointer prior to that check. This patch simply moves the dereference after the NULL test. But... it can't be called without a valid 'dev', no? A quick 'grep do_ioctl net/' confirms that all calls are in the form of 'dev-do_ioctl(dev, ...'. Domen @@ -991,11 +991,13 @@ static int sb1000_dev_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) short PID[4]; int ioaddr[2], status, frequency; unsigned int stats[5]; - struct sb1000_private *lp = netdev_priv(dev); + struct sb1000_private *lp; if (!(dev dev-flags IFF_UP)) return -ENODEV; + lp = netdev_priv(dev); + ioaddr[0] = dev-base_addr; /* mem_start holds the second I/O address */ ioaddr[1] = dev-mem_start; - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFX]: napi_struct V3
From: Roland Dreier [EMAIL PROTECTED] Date: Sat, 28 Jul 2007 08:21:58 -0700 Another area of consternation are drivers that were using netif_rx_reschedule(), as that interface was removed because it doesn't fit well with the caller managing the dev-quota et al. I left race conditions in the drivers that were using that interface, but they should still basically work nonetheless. ...but this is a problem for IPoIB. The underlying IB stuff only allows us to register what is essentially a one-shot edge-triggered interrupt. So there is a race between calling netif_rx_complete() and calling ib_req_notify_cq() (which enables the interrupt), since the interrupt might never happen if a packet arrives between the two calls. You will see further along in the thread that we have found a locking sequence by which this issue can be resolved. Please read the full thread and the most recent patch before coming to conclusions :-) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC]: napi_struct V4
From: Roland Dreier [EMAIL PROTECTED] Date: Sat, 28 Jul 2007 08:27:18 -0700 Most drivers are in good shape, although some still have very questionable netif_rx_complete() handling, in that racy area that Rusty and myself were discussing today. My inclination is to wrap those sequences around with an IRQ safe spinlock to fix the race provably, and then if driver authors want to optimize that away with techniques like those that tg3, bnx2, sky2, skge et al. use, that's fine but can be done later. Ouch... that extra lock seems pretty expensive. Also I'm having a hard time understanding how the techniques you're alluding to apply to devices that may miss events when enabling interrupts; the drivers you mention all seem to be for devices that didn't have the race and didn't use netif_rx_reschedule() in the old NAPI world. Can you provide a little more detail on how the lock could be optimized away? If you have a means in the device (like tg3, bnx2, e1000, and a score of others do) to force the device to trigger a HW interrupt, that's what you do if you detect that events are pending after re-enabling interrupt in the -poll() handler. Frankly I don't think the lock is a big deal and you need something like it anyways typically. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC]: napi_struct V4
From: Jeff Garzik [EMAIL PROTECTED] Date: Sat, 28 Jul 2007 14:08:44 -0400 That's a performance/parallelization regression from current NAPI :( That's not true since current NAPI will only run on one cpu, the one that the interrupt triggers on. The existing cases that are not guarding the sequence to be atomic are racy, and in a way which is very hard to debug when it triggers and the device just appears dead. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sb1000: prevent a potential NULL pointer dereference in sb1000_dev_ioctl()
On Sun, 29 Jul 2007, Domen Puncer wrote: On 29/07/07 00:02 +0200, Jesper Juhl wrote: Hi, Here's a small patch, prompted by a find by the Coverity checker, that removes a potential NULL pointer dereference from drivers/net/sb1000.c::sb1000_dev_ioctl(). The checker spotted that we do a NULL test of 'dev', yet we dereference the pointer prior to that check. This patch simply moves the dereference after the NULL test. But... it can't be called without a valid 'dev', no? A quick 'grep do_ioctl net/' confirms that all calls are in the form of 'dev-do_ioctl(dev, ...'. Yup, I think so too ... @@ -991,11 +991,13 @@ static int sb1000_dev_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) short PID[4]; int ioaddr[2], status, frequency; unsigned int stats[5]; - struct sb1000_private *lp = netdev_priv(dev); + struct sb1000_private *lp; if (!(dev dev-flags IFF_UP)) return -ENODEV; I think we could get rid of the !dev check itself. Actually, the IFF_UP check /also/ looks suspect to me for two reasons: (1) I remember Stephen Hemminger once telling me dev-flags is legacy and unsafe, and one of the netif_xxx() functions be used instead, and, (2) I wonder if we really require the interface to be up and *running* when we do this ioctl. Satyam - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
