On 11/22/2011 08:51 AM, Stefan Berger wrote:
> With hunks borrowed from one of David Steven's previous patches, we now
> add the capability of having a 'mac' chain which is useful to filter
> for multiple valid MAC addresses.
>
> Signed-off-by: David L Stevens <dlstevens@xxxxxxxxxx>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
>
> ---
> docs/schemas/nwfilter.rng | 3 +++
> src/conf/nwfilter_conf.c | 2 ++
> src/conf/nwfilter_conf.h | 2 ++
> src/nwfilter/nwfilter_ebiptables_driver.c | 22 ++++++++++++++++++++--
> 4 files changed, 27 insertions(+), 2 deletions(-)
Almost - in addressing my v1 comments, you introduced some other problems.
> @@ -2806,11 +2808,25 @@ ebtablesCreateTmpSubChain(ebiptablesRule
> char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH];
> char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
> : CHAINPREFIX_HOST_OUT_TEMP;
> + char *protostr = NULL;
>
> PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
> PRINT_CHAIN(chain, chainPrefix, ifname,
> (filtername) ? filtername : l3_protocols[protoidx].val);
>
> + switch (protoidx) {
> + case L2_PROTO_MAC_IDX:
> + break;
> + default:
> + virAsprintf(&protostr, "-p 0x%04x", l3_protocols[protoidx].attr);
> + break;
> + }
> +
> + if (!protostr) {
> + virReportOOMError();
Oops. This gives a spurious OOM failure if protoidx is L2_PROTO_MAC_IDX.
> @@ -2819,7 +2835,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
> CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR
> CMD_EXEC
> "%s"
> - CMD_DEF("%s -t %s -%%c %s %%s -p 0x%x -j %s")
> + CMD_DEF("%s -t %s -%%c %s %%s %s -j %s")
^^
While you fixed the double space problem for a non-empty protostr, you
still have the double space for L2_PROTO_MAC_IDX. To completely avoid a
double space, as well as the spurious OOM, you'd need:
case L2_PROTO_MAC_IDX:
protostr = strdup("");
break;
default:
virAsprintf(&protostr, "-p 0x%04x ", l3_protocols[protoidx].attr);
break;
...
CMD_DEF("%s -t %s -%%c %s %%s %s-j %s")
ACK with those tweaks.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list