[libvirt] [PATCH] Add V6LOCAL parameter to nwfilter rules.

[Brian Rak]

On 4/2/2014 3:56 PM, Eric Blake wrote:
> On 04/02/2014 01:40 PM, Brian Rak wrote:
>> Currently, adding any sort of IPv6 nwfilter rules is rather difficult.  There are no standard rules,
> Long lines; we tend to keep commit messages wrapped around 72 columns or
> so ('git log' adds indentation, and commits start to look stupid in the
> terminal if they wrap while reading 'git log').
>
>> and you end up doing a lot of things by hand.  This patch makes the $V6LOCAL variable available within
>> rules.  This is the generated from the interface's mac address using the modified EUI-64 format, which
>> matches what the guest should be using.
> An example in the commit message of what the variable expands to would
> be nice.
>
>> This is part of what information is needed to correctly filter guest IPv6 traffic.  Since this changes
>> with the MAC address, it is significantly easier if libvirt populates it (rather then requring the
> s/requring/requiring/
>
>> user to enter it)
>>
>> ---
>>   docs/formatnwfilter.html.in            |    9 ++++++---
>>   src/conf/nwfilter_params.h             |    1 +
>>   src/nwfilter/nwfilter_gentech_driver.c |   23 +++++++++++++++++++++++
>>   3 files changed, 30 insertions(+), 3 deletions(-)
>>   mode change 100644 => 100755 src/nwfilter/nwfilter_gentech_driver.c
>>
>> @@ -251,6 +251,9 @@
>>         parameter similar to the IP parameter above, it is discouraged
>>         since libvirt knows what MAC address an interface will be using.
>>         <br/><br/>
>> +      <code>V6LOCAL</code> is the computed IPv6 link-local address.
>> +      This is based on the MAC variable
> Also worth an example of what this will contain (such as
> fe80::5254:00ff:fe1a:0a6d).  And definitely needs a "Since" tag (in the
> appropriate <div> markup) mentioning this was added in 1.2.4.
>
>
>> +
>> +        virMacAddr parsedMac;
>> +        if (virMacAddrParse(macaddr, &parsedMac) == 0)
>> +        {
> Style - this { belongs on the same line as the if.
Thanks, will make those changes

>> +            parsedMac.addr[0] ^= 2;
>> +
>> +            char euiMacAddr[26];
>> +            snprintf(euiMacAddr, sizeof(euiMacAddr), "fe80::%x%x:%xff:fe%x:%x%x", parsedMac.addr[0], parsedMac.addr[1], parsedMac.addr[2],
>> +                parsedMac.addr[3], parsedMac.addr[4], parsedMac.addr[5]);
> Long line; please wrap to 80 columns.  You MEANT to use %02x; your code
> misbehaves on zero bytes.  Why do you need to open-code the snprintf;
> would it be any cleaner to just use functions from util/virsocketaddr.h
> for formatting an IPv6 value that you construct from the MAC address?
In my opinion, manually formatting the address here is a lot simpler to 
understand then constructing a virSockAddr and using virSocketAddrFormat 
on it.  It's definitely shorter code this way. I'm not sure which way 
makes more sense.