DHCP server working in production now

Stephen R. van den Berg srb at cuci.nl
Tue Aug 23 15:43:33 CEST 2011


Alan DeKok wrote:
>Stephen R. van den Berg wrote:
>> Alan DeKok wrote:
>>>  Except that the DHCP server already works, and is used in production
>>> in multiple environments.

>> Is it?  The existing code suggests otherwise.

>  Stop being rude.

I was not being rude.  I just expressed my genuine startledness.

>  Go read the list archives.  MULTIPLE people are using it an a
>production environment.  Are they all lying?

I admit that I didn't read all the archives.  I read the code first
and found numerous spots where it was marked "Does not work" and
a lot of the code broke due to different macros, and I found places
which simply said "Fill in code here".  So I assumed that it's not
in production.

My sincere apologies for that assumption, if I'd known, I'd *not* have
deleted the DHCP-relay support.

>> b. Convoluted and non-transparent broadcast/unicast/IP decision logic.
>>    I'm not saying it was/is wrong, but these matters are rather subtle,
>>    and it is difficult to audit the existing code.

>  Maybe that's true.  But replacing it with incomprehensible code isn't
>a step forward.

I am going to make it comprehensible before resubmitting.

>> c. Too much tapdancing to avoid accessing the struct of the DHCP message,
>>    which results in more convoluted code than necessary.

>  While that might be true, you *removed* a lot of useful code.  That
>code allowed the administrator to over-ride portions of the DHCP message.

It might not seem like that, but I don't think I corrupted this ability.
I want/need that flexibility myself, so I was very careful to preserve it.

>> Point a and b were important enough to start replacing parts, point c
>> was more or less collateral damage (fixed it as I went along).

>  Then *please* submit multiple patches.  Submitting a jumbo patch that
>changes everything makes it difficult to separate the good changes from
>the bad changes.

I normally always do, just not in this preview case.

>> I warned, that this is preliminary; this is not intended to be merged
>> in the existing codebase; I'm giving a preview, that's all.

>  Ugly code means I don't review it.  I just delete the message.  I don'
>have time to try to figure out what the code is doing.  Well formatted
>code means I can review it and provide productive feedback.

I understand that.  I'm not offended by it.  Feedback that it is
too timeconsuming to parse for review is good enough too.

>> It only breaks DHCP relaying, if that is necessary, I'll add it back.
>> I'm still puzzled why freeradius would want to relay itself, and not
>> delegate that functionality to a small program that does this quite
>> well already.

>  *Policy* based relays.

>	MAC A -> DHCP server B
>	MAC X -> DHCP server Y

Interesting.  Haven't seen those "in the wild" yet.  But yes, I didn't
know people did this.
-- 
Stephen.

"What do I do when I see someone *extremely gorgeous*?
 I stare, I smile, and when I get tired... I put the mirror down."



More information about the Freeradius-Devel mailing list