Re: [libvirt] [PATCH v3 2/4] util/viriptables: add/remove rules that short-circuit masquerading

On 09/23/2013 08:03 PM, Laszlo Ersek wrote:

> diff --git a/src/util/viriptables.h b/src/util/viriptables.h > index 447f4a8..fcff5f8 100644 > --- a/src/util/viriptables.h > +++ b/src/util/viriptables.h > @@ -26,6 +26,11 @@ > > # include "virsocketaddr.h" > > +typedef struct { > + virSocketAddr addr; > + unsigned int prefix; > +} virPfxSocketAddr; > + I'm conflicted about whether or not it's really beneficial to have this struct (is the setup really worth it just to save one arg?),

but I suppose it doesn't hurt anything, so I'll go along with it if nobody else objects. It *does* make these functions' APIs inconsistent with the other iptables functions that take a separate addr and prefix, though - if we keep this then we should probably update the rest of the API to be consistent (not in this patch though).

> +/* Don't masquerade traffic coming from the network associated with the bridge > + * if said traffic targets @destaddr/@destprefix. > + */ > +static int > +iptablesDontMasquerade(const virPfxSocketAddr *src, > + const char *physdev, > + const virPfxSocketAddr *dst, > + int action) > +{ > + int ret = -1; > + char *srcStr = NULL; > + char *dstStr = NULL; > + virCommandPtr cmd = NULL; > + > + if (!(srcStr = iptablesFormatNetwork(&src->addr, src->prefix))) > + return -1; > + > + if (!(dstStr = iptablesFormatNetwork(&dst->addr, dst->prefix))) > + goto cleanup; > + > + if (!VIR_SOCKET_ADDR_IS_FAMILY(&src->addr, AF_INET)) { > + /* Higher level code *should* guaranteee it's impossible to get here. */ > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Attempted to NAT '%s'. NAT is only supported for IPv4."), > + srcStr); > + goto cleanup; > + } Don't you need to do the same check for dst?