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: procfs files vs symlink

2022-01-14 Thread RVP

On Fri, 14 Jan 2022, Christos Zoulas wrote:


That looks nice, committed and forgot to give you credit, apologies.



Thanks, but, the 1st part of the patch with the corrections to the table
need to go in as well, otherwise there's a mismatch between what the
table says and what the code does: exe is a link:procfs_readlink(); emul
is a reg. file:procfs_doemul():

{ DT_LNK, N("emul"),PFSemul,NULL },
{ DT_REG, N("exe"), PFSexe, procfs_validfile },

$ ls -l /proc/self/emul /proc/self/exe
-r--r--r--  1 rvp   rvp6 Jan 14 20:23 /proc/self/emul
lr-xr-xr-x  1 root  wheel  7 Jan 14 20:23 /proc/self/exe -> /bin/ls

$ ./a.out /proc/self# print d_type vs. st_mode mismatches
DT_LNK: S_ISREG emul
DT_REG: S_ISLNK exe

-RVP



Re: procfs files vs symlink

2022-01-14 Thread Christos Zoulas
In article <8ad9feaf-a513-d33d-c887-3ca8407c...@sdf.org>,
RVP   wrote:
>On Fri, 14 Jan 2022, Robert Elz wrote:
>
>> If your patch makes any difference to the way ls /proc/self/fd
>> works, that is just a fluke (relates to the way ls happens to
>> sequence its operations) and in no way any kind of general fix.
>>
>
>It does not, and wasn't meant to. I noticed that d_type was being
>set to VREG and attached a patch for that to my reply.
>
>> If procfs is not setting d_type correctly, that should be fixed.
>>
>
>I think this does it:

That looks nice, committed and forgot to give you credit, apologies.

christos



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


Some guidance/suggestion please

2022-01-14 Thread Paul Goyette

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
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.

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...)

Anyone got any suggestions?


++--+--+
| 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 |
++--+--+