[libvirt] [PATCH V11 4/7] nwfilter: add DHCP snooping

Stefan Berger stefanb at linux.vnet.ibm.com
Thu Apr 19 10:46:22 UTC 2012


On 04/19/2012 05:54 AM, Daniel Veillard wrote:
> On Tue, Apr 17, 2012 at 10:44:05AM -0400, Stefan Berger wrote:
>
>> Index: libvirt-acl/src/conf/nwfilter_params.h
>> ===================================================================
>> --- libvirt-acl.orig/src/conf/nwfilter_params.h
>> +++ libvirt-acl/src/conf/nwfilter_params.h
>> @@ -91,6 +91,11 @@ int virNWFilterHashTablePutAll(virNWFilt
>>   # define VALID_VARVALUE \
>>     "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:"
>>
>> +# define NWFILTER_VARNAME_IP "IP"
>> +# define NWFILTER_VARNAME_MAC "MAC"
>> +# define NWFILTER_VARNAME_IP_LEARNING "ip_learning"
>> +# define NWFILTER_VARNAME_DHCPSERVER "DHCPSERVER"
>> +
>>   enum virNWFilterVarAccessType {
>>       VIR_NWFILTER_VAR_ACCESS_ELEMENT = 0,
>>       VIR_NWFILTER_VAR_ACCESS_ITERATOR = 1,
>    A fairly big patch ! I have tried to really read everything, but
> finding locking issues, especially when there is multiple layers
> of locks is nearly impossible by review.
>    I think the best is to get this running and tested as much as possible
> before the next release,
>
>    ACK
>

Thanks for the ACK.

However, there are some things that I would like to adapt before 
checking in:

- rename the 'ip_learning' variable to CTRL_IP_LEARNING to open up a 
'namespace' for future extensions providing control to users over the 
behavior of the algorithm; someone may be interested in having IP 
addresses detected by the DHCP snooper but have no filters instantiated 
and for that there could be a variable CTRL_NO_INSTANTIATE (=true)

- some small optimization to not have packets submitted to the worker 
that are shorter than the minimal size a valid DHCP packet must have

- forgot about the existence of virSocketAddr and so I needed to replace 
the usage of uint32_t's with virSocketAddr

So I would check in patches 1-3 soon and post the rest of the series 
again later.

    Stefan





More information about the libvir-list mailing list