[kea-dev] I'd appreciate some feedback on trac ticket 3963 :)

[Marcin Siodelski]

On 05.08.2015 22:08, Angelo Failla wrote:
>  
> On August 5, 2015 at 8:07:53 PM, Marcin Siodelski (marcin at isc.org(mailto:marcin at isc.org)) wrote:
> 
>> Angelo,  
>>  
>> We have accepted this work for 1.0 assuming that you will write the code  
>> for it.  
> 
> \o/
> Yeah, I will work on it, how does your review process work? I red the dev guide, I don’t have access to your internal git repo, I guess you are ok with a PR
> on github?
> 

We maintain the clone of the internal repository on github. In fact, it
is not so much internal, but for external people it is read only.
Nevertheless, we want people to use the github repo for contributions:

http://github.com/isc-projects/kea

You should fork this repo (if you haven't done that already) and issue
pull request when you're ready for a review of your code.

The ticket is here: http://kea.isc.org/ticket/3964

Please assign this ticket to yourself (assuming you have a account in
our trac system). If you don't, I'll just assign it to a generic person
called "contributor". When it is assigned to you or contributor it is an
information for others to not take this ticket because someone else is
working on it.

Did you read this in the developer's guide:
http://git.kea.isc.org/~tester/kea/doxygen/d7/df4/contributorGuide.html ?

It is important to make sure that you follow the coding style and make
sure that proper unit tests are implemented. We follow the test driven
development, so the test should be created first, then the actual code.
To enable compilation of tests (google tests) you need to specify
--with-gtest as a configuration parameter when building Kea.

When you fork the repo please create the branch trac3964. It will be
easier for me to find this code in your fork, as this is the naming
convention for branches dedicated for specific tickets.

>> I do think it is quite a straight forward change. One thing I  
>> haven't pointed out in the ticket, and which may be worth some  
>> clarification, is that we would like to have a 'runtime' configuration  
>> parameter, rather than the compilation time parameter to enable  
>> stateless mode.  
> 
> yeah I’d like some clarification on this part, do you have an example
> in the code of a ‘runtime’ configuration parameter?
> I guess I can look it out myself but asking here is quicker :)
> For ‘runtime’ configuration param I guess you mean configuration that doesn't
> require a complete restart of the daemons right?
> 

By 'runtime' parameter I mean the parameter that is specified in the Kea
configuration (json) file.

I think you could take the 'echo-client-id' as an example. It is a
boolean flag in the global scope of the configuration. Your parameter
will be defined in the similar way. Please make sure you include a
proper declaration for your new parameter in the
src/bin/dhcp4/dhcp4.spec file.

It may be a good idea to grep for 'echo-client-id' in the src/bin/dhcp4
and you will find out how to implement the logic required for your new
parameter to be interpreted by the server. You'll also find how unit
tests for it can be implemented.



> Did you already create a TRAC ticket for this proposed change?
> I can’t find it.
> 

See above.

>> One question I had is this. Do you require sending different sets of  
>> options depending on the location of the client in the network? So for  
>> example: for the clients from which the packet is received on this  
>> interface you send these options, and for the clients connected to  
>> another interface other options? Or accordingly for different relays  
>> when dealing with the relayed traffic?  
> 
> What do you mean by different set of options? Do you mean different DHCP
> sent to the clients? Or you are referring to KEA config options?
> But no, we really just need a single set of options: we don’t care on which
> interface a client is hitting us on.
> 

Oh, I meant different DHCP options sent by the server to the client. So
you don't really need to do any subnet specific configurations.

What will be needed on the server side is to make sure that in the
stateless mode:

The server shouldn't require that the subnet is selected, which is
currently required by many functions like appendRequestedOptions.
They currently use the subnet->getCfgOption() to retrieve subnet
specific options, but in your case they'd need to use
CfgMgr::instance().getCurrentCfg()->getCfgOption() which holds top level
options (because we don't use subnets in statless mode). Whether it uses
top level options or subnet options should depend on whether you're in
stateless or stateful mode.

Perhaps you could just reuse the solution from the
"appendRequestedOptions" implemented in the DHCPv6 server. Note that it
also does the stateless configuration when Information-request is sent.

> Maybe explaining our setup can help responding to your question:
> 
> - we only serve relayed traffic
> - all rack switches in our DCs runs a DHCP relayer
> - each relayer is configured to forward traffic to a global BGP anycast VIP
> - KEA servers across the fleet receive the traffic on that VIP
> 


Hope that helps a bit,

Marcin