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

Re: [libvirt] [PATCH V2 1/5] Add a mac chain



On 11/22/2011 01:47 PM, Eric Blake wrote:
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 us ibm com>
Signed-off-by: Stefan Berger<stefanb linux vnet ibm com>

---
  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;
My further testing also pointed that out to me in the meantime... :-/
default:
     virAsprintf(&protostr, "-p 0x%04x ", l3_protocols[protoidx].attr);
I removed the trailing whitespace above...
     break;
...
    CMD_DEF("%s -t %s -%%c %s %%s %s-j %s")

ACK with those tweaks.

   Stefan


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