[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH V2 2/5] Add support for STP filtering



On 11/22/2011 08:51 AM, Stefan Berger wrote:
> This patch adds support for filtering of STP (spanning tree protocol) traffic
> to the parser and makes us of the ebtables support for STP filtering. This code
> now enables the filtering of traffic in chains with prefix 'stp'.
> 
> Signed-off-by: Stefan Berger <stefanb linux vnet ibm com>
> 
> ---
> 
> v2:
>  - addressing Eric Blake's comments
>  - replacing occurrences of number[20] with number[INT_BUFFER_BOUND(uint32)]; this
>    also works for IP masks which are expressed as "%d".

The maximum %d is one byte longer than %u, thanks to a negative sign; so
you've got a potential off-by-one issue (although a negative IP mask
doesn't make sense, so you may be safe for now).

>  
> +static const virXMLAttr2Struct stpAttributes[] = {
> +    /* spanning tree uses a special destination MAC address */
> +    {

I'd feel a bit better if this comment appears near here:

/* STP is documented by IEEE 802.1D; for a synopsis,
 * see http://www.javvin.com/protocolSTP.html */

> @@ -937,7 +950,7 @@ iptablesHandleIpHdr(virBufferPtr buf,
>                      virBufferPtr prefix)
>  {
>      char ipaddr[INET6_ADDRSTRLEN],
> -         number[20];
> +         number[INT_BUFSIZE_BOUND(uint32_t)];

Maybe it's best to rely on util.h giving us MAX, and doing:

char number[MAX(INT_BUFSIZE_BOUND(uint32_t),
                INT_BUFSIZE_BOUND(int))];

so that we can use both %u for uint32_t, and %d for int, without
worrying about any other weird type promotions going on.

ACK with those nits addressed.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]