> You are right in regards to dynamic memory allocation. You're using
> static array allocation, defined by MAX_IPS_PER_HOSTNAME. This value is
> set to 100. Where did you take this number from? IMHO, that sounds to
> be fairly high.
Actually, I don't use static allocation but stack allocation. The value
of 100 comes straight out of nowhere. Basically I though "hmm, I've
never seen more than about 10 IPs for a given FQDN, so 100 should cover
most cases". This is based on the premise that 400bytes of stack space
is pretty much negligible on the one hand, and that more than 100 IPs is
so unlikely that dynamic allocation is not worth the trouble.
> Correct! Thanks for the nitpick! Stack memory is anyway precious, on
> some systems more than others depending on how stack allocation is done.
What's the smallest system on which OpenVPN is expected to work?
My OpenVPN server has 32MB of RAM (a linksys home router), and I assumed
it was on the lower scale already. A system where 1KB of stack space is
significant would have to be a lot smaller.
> As I said initially, in the current situation this might not be the
> case. But this part of OpenVPN will most likely see a change in the
> future. Not forked nor threaded applications scales very badly on
> today's multi core systems. So this comment was really meant as a
> hands-up of a potential issue in the future.
Depending on the style of threading, I could imagine 1KB of stack space
per thread to become more significant, indeed. I'll let others decide
whether it's likely to be a real problem or not.
> Reasonable argument. On the other hand, a hackerish approach it might
> be considered to use a global variable in this case. This setting
> should only be read only, not changed and is a global setting. Even
> though, I'm not a big fan of global variables, I'm willing to consider
> it in this setting.
If it's a config var, it could indeed just be a global var, so I don't
think it would be very complex. But that's really not something the
user should have to configure.
>> Reading through this, my suggestion would be "go for a reasonable but
>> lower value", something like "20". Still arbitrary, but brings down the
>> (transient!) memory used to 80 bytes, which is hard to beat with any sort
>> of dynamic allocation scheme.
> Good point! But that's only valid as long as openvpn stays single
> threaded. On the other hand, this might be good enough for now.
A normal function's activation frame can already be within the same
order of magnitude, so worrying about 80bytes sounds pretty extreme to
me (as a compiler guy). OTOH I don't know if 20 entries is enough
(it's plenty for my use case, tho, so I'd be fine with this choice).
Stefan
