re: Some guidance/suggestion please

2022-01-15 Thread Paul Goyette

On Sat, 15 Jan 2022, matthew green wrote:


it seems to me that if some driver depends upon altq, then
altq should simply always refuse to unload if a driver is
loaded that depends upon it.  this should be an explicit
dependency, and probably implicit via symbols.

if, say there's a fully modular system with two NICs, and
only one of them supports altq.  only one of the NIC drivers
will declare a dep on altq, blocking the unload of altq
while the driver remains loaded.


if this isn't already the case, can we arrange it to be?


I see the problem a little differently.  The driver doesn't
actually _depend_ on altq, but it does different (perhaps
additional) things based on whether altq is present or not.
Currently the present-or-not decision is done at compile-
time based on ``options ALTQ'', but it should be possible to
make the present-or-not decision at run-time.

The sample pseudo-code in my original post could use the
existing module_hooks, so it becomes more like:

...
MODULE_HOOK_CALL(altq-part-A-hook);
{common code}
MODULE_HOOK_CALL(altq-part-B-hook);
...

but with an additional mechanism to prevent a load/unload
in between the two hook calls.

I'm going to dig in a little bit deeper into the ALTQ code;
perhaps in some cases the part-A/part-B code can be coalesed
into a single hook and the problem goes away.  That would
address the immediate concern for ALTQ but I suspect that it
will resurface later on for some other option/module.


++--+--+
| Paul Goyette   | PGP Key fingerprint: | E-mail addresses:|
| (Retired)  | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com|
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org  |
| & Network Engineer |  | pgoyett...@gmail.com |
++--+--+


Re: Some guidance/suggestion please

2022-01-14 Thread Michael van Elst
m...@eterna.com.au (matthew green) writes:

>it seems to me that if some driver depends upon altq, then
>altq should simply always refuse to unload if a driver is
>loaded that depends upon it.  this should be an explicit
>dependency, and probably implicit via symbols.

>if, say there's a fully modular system with two NICs, and
>only one of them supports altq.  only one of the NIC drivers
>will declare a dep on altq, blocking the unload of altq
>while the driver remains loaded.

>if this isn't already the case, can we arrange it to be?


Isn't altq going to be redesigned anyway for NET_MPSAFE ?
Efforts to move it into a module might be a bit premature then.

N.B. I'd just make the different queuing mechanisms loadable
but keep hooks for the network drivers in the kernel itself,
similar to bufq for disk drivers.



re: Some guidance/suggestion please

2022-01-14 Thread matthew green
it seems to me that if some driver depends upon altq, then
altq should simply always refuse to unload if a driver is
loaded that depends upon it.  this should be an explicit
dependency, and probably implicit via symbols.

if, say there's a fully modular system with two NICs, and
only one of them supports altq.  only one of the NIC drivers
will declare a dep on altq, blocking the unload of altq
while the driver remains loaded.


if this isn't already the case, can we arrange it to be?


.mrg.


Re: Some guidance/suggestion please

2022-01-14 Thread John Nemeth
On Jan 14,  9:11, Paul Goyette wrote:
}
} I'm looking into more modularization of the kernel, and my next
} "target" is the ALTQ stuff.  Right now, there are several network
} device drivers that are built as loadable modules, yet they still
} depend on conditional compilation.  In particular, there seems to
} be some number of code fragments similar to
} 
}   ...
}   #ifdef ALTQ
}   altq-code-part-A
}   #endif
}   (common code)
}   #ifdef ALTQ
}   altq-code-part-B
}   #endif
}   ...
} 
} The existing module_hook mechanism doesn't help us here.  We can
} make the two pieces of altq code into module hooks, but that
} doesn't handle the case where the module gets loaded or unloaded
} between the two parts of the altq code.
} 
} We have the module_hold()/module_rele() mechanism but they're not
} really appropriate here.  Both routines require us to already have
} access to the module's data in its struct module.
} 
} So, I'm pretty sure we need a new mechanism, one that prevents a
} module from being unloaded during critical sequences.  This new

 Unload is a request that goes to the module.  It is the modules
responsibility to deny the request if it can't be safely unloaded.
This has always been the case.  I don't think a new mechanism is
requried here.

} mechanism needs to also prevent a module from being newly loaded
} (or at least, from installing any new hooks);  otherwise we could
} potentially execute the part-B code without have run any part-A
} code on which part-B might depend.  Therefore the mechanism cannot
} live within the module itself.

 This one is more tricky.  The module can certainly error out
in the load routine and refuse to load.  The problem is that the
calls to "part A" and "part B" are external to the module.  How
does it know that the dummy "part A" has been called, but not the
dummy "part B"?

 There are a couple of possibilities here, all bad.  The dummy
"part A" could set a flag which is cleared by the dummy "part B".
The module could then refuse to load if the flag is set.  The other
possibility is that the module could simply clear the flag and
return when "part B" is called.

 The problem here is what happens if multiple devices are
calling AltQ at the same time?  The best situatiion I can see is
that the module be written in such a way that if "part B" is called
without "part A" being called that it not cause problems.  This
should be the case anyways to have resiliency against a broken
driver calls "part B" without calling "part A".

} FWIW, I vaguely remember having one or two instances of this issue
} during my work on the compat modules (some years ago).  I think I
} addressed these by making explicit checks on the hook->hooked
} member, but that's not really correct.  (Unfortunately I can't
} remember any details on this...)
} 
}-- End of excerpt from Paul Goyette


Re: Some guidance/suggestion please

2022-01-14 Thread Martin Husemann
On Fri, Jan 14, 2022 at 09:11:08AM -0800, Paul Goyette wrote:
>   #ifdef ALTQ
>   altq-code-part-A
>   #endif
>   (common code)
>   #ifdef ALTQ
>   altq-code-part-B
>   #endif
>   ...
> 
> The existing module_hook mechanism doesn't help us here.  We can
> make the two pieces of altq code into module hooks, but that
> doesn't handle the case where the module gets loaded or unloaded
> between the two parts of the altq code.

I guess the same thing will happen when we make more networking things
loadable: each interface needs to "attach to altq", and if altq is loaded
later, it needs a way to iterate all interfaces "at some convenient
time" and do the necessary fixup. Same for unloading altq.

That way the interface driver can set a "altq present" flag at attach time
and only do the part A and part B calls if that flag is set.

Martin