2000-11-09 17:28:33

by Shmuel Hen

[permalink] [raw]
Subject: catch 22 - porting net driver from 2.2 to 2.4

Hello,

This is a bit long and I apologize (since there are kdb captures in it).

We are developing an advanced networking services driver (loadable module)
and are having problems porting it to work on 2.4.x kernel.
The driver is supposed to provide services such as fault tolerance, load
balancing, link aggregation and VLAN. It does that by creating a group of
"virtual" adapters that are bound on top of a team of network adapters. This
works great on 2.2 kernels but demonstrated a few problems on 2.4.0-test9

The only problem we have left now has to do with insmod/rmmod. for good
reasons, we cant just call init_etherdev() like base drivers do, so we
created our own version that handles memory and name allocations and calls
register_netdevice() on it's own the same as init_etherdev(). since we've
got several "virtual" adapters that are part of a topology being built
progressively, we can't perform their registrations during module_init() but
rather through an IOCTL and here is our problem:

if we call register_netdevice(), we get the following message:

RTNL: assertion failed at devinet.c(775):inetdev_event

we figured this is because we neglected rtnl_lock() so instead we try using
register_netdev() to handle this for us but then we get:

Scheduling in interrupt
kernel BUG at sched.c:696!
Entering kdb (current=0xc51a8000, pid 1075) on processor 1 Panic:
invalid operand
due to panic @ 0xc011aa71

eax = 0x0000001b ebx = 0x00000020 ecx = 0xc030d80c edx = 0xffffffff
esi = 0xc030b5b4 edi = 0xffffffff esp = 0xc51a9d34 eip = 0xc011aa71
ebp = 0xc51a9d8c ss = 0x00000018 cs = 0x00000010 eflags =
0x00010246
ds = 0x00000018 es = 0x00000018 origeax = 0xffffffff &regs =
0xc51a9d00

[1]kdb> bt
EBP EIP Function(args)
0xc51a9d8c 0xc011aa71 schedule+0x935 (0xc2c9e000, 0xc4482520,
0xc51a9e10)
kernel .text 0xc0100000 0xc011a13c
0xc011aa80
0xc51a9db8 0xc0107b8d __down+0xf5
kernel .text 0xc0100000 0xc0107a98
0xc0107c68
0xc51a9dcc 0xc0107f43 __down_failed+0xb (0xc51a9de4, 0xd082de5c,
0xc2c9e000, 0xc60a8320, 0xc51a9dfc)
kernel .text 0xc0100000 0xc0107f38
0xc0107f4c
0xc023a7a9 stext_lock+0x4919
kernel .text.lock 0xc0235e90
0xc0235e90 0xc023bd80
0xc51a9dd4 0xc01f2e81 rtnl_lock+0x11 (0xc2c9e000)
kernel .text 0xc0100000 0xc01f2e70
0xc01f2e88
0xc51a9de4 0xd082de5c [ians]iansInitEtherdev+0x20 (0xc4482520)
ians .text 0xd082d060 0xd082de3c
0xd082de78
.
. (boring chain of calls)
.
0xc51a9ec4 0xd082dbcd [ians]doControlIoctl+0x15d (0xc2c9e200,
0xc51a9f20, 0x89f0)
ians .text 0xd082d060 0xd082da70
0xd082dc40
0xc51a9ee4 0xc01ef09f dev_ifsioc+0x33f (0xc51a9f20, 0x89f0,
0xc51a9f20)
kernel .text 0xc0100000 0xc01eed60
0xc01ef0b0
0xc51a9f40 0xc01ef29d dev_ioctl+0x1ed (0x89f0, 0xbffffa58)
kernel .text 0xc0100000 0xc01ef0b0
0xc01ef300
0xc51a9f64 0xc021a70c inet_ioctl+0x18c (0xc339d13c, 0x89f0,
0xbffffa58)
kernel .text 0xc0100000 0xc021a580
0xc021a720
0xc51a9f84 0xc01e8f06 sock_ioctl+0x9e (0xc339d040, 0xc38e0900,
0x89f0, 0xbffffa58)
kernel .text 0xc0100000 0xc01e8e68
0xc01e8f6c
0xc51a9fbc 0xc014f5fd sys_ioctl+0x26d (0x3, 0x89f0, 0xbffffa58,
0x4000ae60, 0xbffffba4)
kernel .text 0xc0100000 0xc014f390
0xc014f6a0
0xc010965f system_call+0x33
kernel .text 0xc0100000 0xc010962c
0xc0109664

We figured that since we are in user context (do_ioctl) and use
spin_lock_bh() to protect us from other concurrent threads, it might
interfere with rtnl_lock() so we remove our lock just before calling
register_netdev() and lock again upon return but then the whole process just
stopped and didn't return to the prompt. from within kdb, we could see that
all CPU's are running in idle but if we try to return to the prompt the
whole system hangs. sometimes it hangs if we try to run ifconfig -a to see
if the virtual adapters appear.

I can't use it without locks, I can't use it with locks and I can't complete
the operation if I remove my own locks - catch 22.


The other problem has to do with rmmod - here we get called in our
cleanup_module function and from it we try to call unregister_netdev() for
each registered virtual adapter.
in this case, we get:

Scheduling in interrupt
kernel BUG at sched.c:696!

Entering kdb (current=0xc38c8000, pid 1602) on processor 0 Panic:
invalid operand
due to panic @ 0xc011aa71
eax = 0x0000001b ebx = 0x00000000 ecx = 0xc030d80c edx = 0x00000000
esi = 0x007ae686 edi = 0xffffffff esp = 0xc38c9e54 eip = 0xc011aa71
ebp = 0xc38c9eac ss = 0x00000018 cs = 0x00000010 eflags =
0x00010246
ds = 0x00000018 es = 0x00000018 origeax = 0xffffffff &regs =
0xc38c9e20

[0]kdb> bt
EBP EIP Function(args)
0xc38c9eac 0xc011aa71 schedule+0x935 (0xc38c9ec0)
kernel .text 0xc0100000 0xc011a13c
0xc011aa80
0xc38c9ed4 0xc011a02a schedule_timeout+0x76
kernel .text 0xc0100000 0xc0119fb4
0xc011a04c
0xc38c9eec 0xc01ef80f unregister_netdevice+0x1c7 (0xc38f9600)
kernel .text 0xc0100000 0xc01ef648
0xc01ef880
0xc38c9efc 0xc01b834e unregister_netdev+0x12 (0xc38f9600)
kernel .text 0xc0100000 0xc01b833c
0xc01b8360
0xc38c9f18 0xd082f3b9 [ians]iansUnregisterDev+0xcd (0xc154a4a0, 0x0)
ians .text 0xd082d060 0xd082f2ec
0xd082f458
.
. (boring chain of calls)
.
0xc38c9f7c 0xd082e29d [ians]cleanup_module+0x69
ians .text 0xd082d060 0xd082e234
0xd082e3cc
0xc38c9f90 0xc0120620 free_module+0x1c (0xd082d000, 0x0, 0xc38c8000,
0x80563cc)
kernel .text 0xc0100000 0xc0120604
0xc01206a0
0xc38c9fbc 0xc011f70f sys_delete_module+0x1fb (0xbffffcad, 0x28,
0x6, 0x80563cc, 0x1)
kernel .text 0xc0100000 0xc011f514
0xc011f9e8
0xc010965f system_call+0x33
kernel .text 0xc0100000 0xc010962c
0xc0109664

If we replace it with a direct call to unregister_netdevice(), we get stuck
in an infinite loop that says something like:

waiting for <device> to become free. usage count = 2
waiting for <device> to become free. usage count = 2
waiting for <device> to become free. usage count = 2

Trying to debug this I could see that dev->refcnt is involved, so I print
it's value before and after register_netdevice() and again before and after
unregister_netdevice(). oddly I get 0 first and then 1, but before
unregister_netdevice() I get 2 !!!

So again, if I use locks I get stuck and if I don't use locks I get stuck -
catch 22.

Please advise.

Thanks in advance.

Shmulik Hen,
Software Engineer
Linux Advanced Networking Services
Network Communications Group, Israel (NCGj)
Intel Corporation Ltd.




2000-11-09 17:37:53

by Jeff Garzik

[permalink] [raw]
Subject: Re: catch 22 - porting net driver from 2.2 to 2.4

do_ioctl is inside rtnl_lock...

Remember if you need to alter the rules, you can always queue work in
the current context, and have a kernel thread handle the work. The nice
thing about a kernel thread is that you start with a [almost] clean
state, when it comes to locks.

Jeff


--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |

2000-11-10 00:33:43

by Olaf Titz

[permalink] [raw]
Subject: Re: catch 22 - porting net driver from 2.2 to 2.4

> We figured that since we are in user context (do_ioctl) and use
> spin_lock_bh() to protect us from other concurrent threads, it might
> interfere with rtnl_lock() so we remove our lock just before calling
> register_netdev() and lock again upon return but then the whole process just
> stopped and didn't return to the prompt. from within kdb, we could see that

Can't you just do this:

#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,3,0) /* not sure about the 0 */
#define rtnl_LOCK() rtnl_lock()
#define rtnl_UNLOCK() rtnl_unlock()
#else
#define rtnl_LOCK() /* nop */
#define rtnl_UNLOCK() /* nop */
#endif

rtnl_LOCK();
register_netdevice(...);
rtnl_UNLOCK();

that works for me (yes, from do_ioctl, but without the bh lock - I
don't know if that's absolutely needed in your case).

Olaf

2000-11-12 11:41:04

by Shmuel Hen

[permalink] [raw]
Subject: RE: catch 22 - porting net driver from 2.2 to 2.4

and you don't get the "RTNL: assertion failed at
devinet.c(775):inetdev_event" in 2.4.x ?

the thing is I need to prevent Tx/Rx when a topology change is initiated
from the ioctl (registering a virtual adapter is just one example), so they
all share a single lock and I must use spin_lock_bh from the ioctl.

Shmulik.

-----Original Message-----
From: Olaf Titz [mailto:[email protected]]
Sent: Friday, November 10, 2000 2:09 AM
To: [email protected]
Subject: Re: catch 22 - porting net driver from 2.2 to 2.4


> We figured that since we are in user context (do_ioctl) and use
> spin_lock_bh() to protect us from other concurrent threads, it might
> interfere with rtnl_lock() so we remove our lock just before calling
> register_netdev() and lock again upon return but then the whole process
just
> stopped and didn't return to the prompt. from within kdb, we could see
that

Can't you just do this:

#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,3,0) /* not sure about the 0 */
#define rtnl_LOCK() rtnl_lock()
#define rtnl_UNLOCK() rtnl_unlock()
#else
#define rtnl_LOCK() /* nop */
#define rtnl_UNLOCK() /* nop */
#endif

rtnl_LOCK();
register_netdevice(...);
rtnl_UNLOCK();

that works for me (yes, from do_ioctl, but without the bh lock - I
don't know if that's absolutely needed in your case).

Olaf

2000-11-12 11:48:58

by Shmuel Hen

[permalink] [raw]
Subject: RE: catch 22 - porting net driver from 2.2 to 2.4

So how come I get the "RTNL: assertion failed at
devinet.c(775):inetdev_event" when I call register_netdevice without
rtnl_lock/unlock ?
could it be a 2.4.0-test9 thing ? (haven't used test10 or 11 yet).

and what about rmmod causing the panic when I use unregister_netdev or never
completing the operation when I use unregister_netdevice ?
does module_exit run inside rtnl_lock too ?


Shmulik.

-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Thursday, November 09, 2000 7:37 PM
To: Hen, Shmulik
Cc: 'LNML'; 'LKML'; [email protected]
Subject: Re: catch 22 - porting net driver from 2.2 to 2.4


do_ioctl is inside rtnl_lock...

Remember if you need to alter the rules, you can always queue work in
the current context, and have a kernel thread handle the work. The nice
thing about a kernel thread is that you start with a [almost] clean
state, when it comes to locks.

Jeff


--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |

2000-11-12 12:18:18

by Jeff Garzik

[permalink] [raw]
Subject: Re: catch 22 - porting net driver from 2.2 to 2.4

"Hen, Shmulik" wrote:
>
> So how come I get the "RTNL: assertion failed at
> devinet.c(775):inetdev_event" when I call register_netdevice without
> rtnl_lock/unlock ?

Uh. Don't do that. You MUST call register_netdevice with rtnl_lock
held.


> and what about rmmod causing the panic when I use unregister_netdev or never
> completing the operation when I use unregister_netdevice ?
> does module_exit run inside rtnl_lock too ?

module_exit does not run inside rtnl_lock.

Theoretically, if you call unregister_netdev from rmmon, it should grab
rtnl_lock and then complete the operation for you. If that doesn't work
for you, it sounds like you are not setting up, or cleaning up,
something correctly.

Basically... it sounds like there are still bugs in your driver that
need working out :)

Jeff


--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |

2000-11-12 12:21:18

by Jeff Garzik

[permalink] [raw]
Subject: Re: catch 22 - porting net driver from 2.2 to 2.4

"Hen, Shmulik" wrote:
> the thing is I need to prevent Tx/Rx when a topology change is initiated
> from the ioctl (registering a virtual adapter is just one example), so they
> all share a single lock and I must use spin_lock_bh from the ioctl.

I do not think that they all need to shared a single lock. And we don't
have your code, but spin_lock_bh may be an incorrect choice too.

Note that when topology changes, that is an operation which might take
more than a few milliseconds. Therefore, your solution should do
something OTHER than spinning on a lock, while topology is changing.

dev->open and dev->do_ioctl are called with rtnl_lock already held. You
can sleep in them. Use that to your advantage...

--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |

2000-11-13 08:18:47

by Shmuel Hen

[permalink] [raw]
Subject: RE: catch 22 - porting net driver from 2.2 to 2.4

Where can I find info about that ?
My first idea was to fire a timer and let the callback routine do the work,
but I worry about synchronization and about passing the list of items for it
to handle.
What is the accepted way of starting a kernel thread and how do I handle
parameters and sync. ?


Thanks,
Shmulik.

-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Thursday, November 09, 2000 7:37 PM
To: Hen, Shmulik
Cc: 'LNML'; 'LKML'; [email protected]
Subject: Re: catch 22 - porting net driver from 2.2 to 2.4


do_ioctl is inside rtnl_lock...

Remember if you need to alter the rules, you can always queue work in
the current context, and have a kernel thread handle the work. The nice
thing about a kernel thread is that you start with a [almost] clean
state, when it comes to locks.

Jeff


--
Jeff Garzik |
Building 1024 | Would you like a Twinkie?
MandrakeSoft |

2000-11-13 17:10:45

by Shmuel Hen

[permalink] [raw]
Subject: RE: catch 22 - porting net driver from 2.2 to 2.4


"Jeff Garzik" wrote:
> Theoretically, if you call unregister_netdev from rmmon, it should grab
> rtnl_lock and then complete the operation for you. If that doesn't work
> for you, it sounds like you are not setting up, or cleaning up,
> something correctly.
>
> Basically... it sounds like there are still bugs in your driver that
> need working out :)

I followed the value of dev->refcnt and there is something strange. before
the call to register_netdev it is set to 0 and after that it is increased to
1. but before the call to unregister_netdev it is somehow 2.

How can I tell who is modifying it and when ?

I tried using kdb to find out but it keeps hanging the machine. I wanted to
place a breakpoint that will pop if a certain memory address is accessed so
I did the following:

static void *p; (global - at the top of my source file)
EXPORT_SYMBOL (p);

in my probe function:
p = (void*) dev->refcnt;

> insmod my_module
> ksyms

this showed that p is at address 0xd081ee90

then from within kdb:
kdb> md 0xd081ee90
0xd081ee90 c470bedc ........ ........

kdb> bpha 0xc470bedc
Forced Global Breakpoint at...

kdb> go

system hung


Is there a way to place a breakpoint on a memory address access ?

2000-11-20 16:44:36

by Shmuel Hen

[permalink] [raw]
Subject: RE: catch 22 - porting net driver from 2.2 to 2.4

I tried using the kernel thread as demonstrated in your example and again it
failed (panic - scheduling in interrupt).
The difference is that your code executes the thread from within dev->open,
while my code tries to do that from dev->do_ioctl that has spinlocks around
the entire operation (which apparently sleeps).
If I comment out the spin_lock/unlock it will succeed, but then I can't be
sure I don't get any concurrent Tx/Rx/timer which is a bad idea while the
topology is still being created.

is there any way to do something like firing threads/timers atomically ?


Thanks,
Shmulik.

-----Original Message-----
From: Jeff Garzik [mailto:[email protected]]
Sent: Monday, November 13, 2000 4:26 PM
To: Hen, Shmulik
Subject: Re: catch 22 - porting net driver from 2.2 to 2.4


"Hen, Shmulik" wrote:
>
> Where can I find info about that ?
> My first idea was to fire a timer and let the callback routine do the
work,
> but I worry about synchronization and about passing the list of items for
it
> to handle.
> What is the accepted way of starting a kernel thread and how do I handle
> parameters and sync. ?

Attached is an example. My "8139too" ethernet driver uses a kernel
thread instead of a timer to perform media checking. It illustrates how
to start and stop a kernel thread.

--
Jeff Garzik |
Building 1024 | The chief enemy of creativity is "good" sense
MandrakeSoft | -- Picasso