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