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

Re: [libvirt] [PATCH v5 2/3] net: add support for specifying port range for forward mode nat



On 02/19/2013 05:44 AM, Natanael Copa wrote:
> Let users set the port range to be used for forward mode NAT:
>
> ...
>   <forward mode='nat'>
>     <nat>
>       <port start='1024' end='65535'/>
>     </nat>
>   </forward>
> ...
>
> Signed-off-by: Natanael Copa <ncopa alpinelinux org>
> ---
>  docs/formatnetwork.html.in  | 21 ++++++++++++++---
>  src/conf/network_conf.c     | 56 +++++++++++++++++++++++++++++++++++++++------
>  src/conf/network_conf.h     |  3 ++-
>  src/network/bridge_driver.c | 16 +++++++++++++
>  src/util/viriptables.c      | 49 +++++++++++++++++++++++++++++++--------
>  src/util/viriptables.h      |  4 ++++
>  6 files changed, 128 insertions(+), 21 deletions(-)
>
> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
> index 5fbd0a9..adb5bb9 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in
> @@ -138,9 +138,11 @@
>              0.4.2</span>
>  
>              <p><span class="since">Since 1.0.3</span> it is possible to
> -            specify a public IPv4 address range to be used for the NAT by
> -            using the <code>&lt;nat&gt;</code> and
> -            <code>&lt;address&gt;</code> subelements.
> +            specify a public IPv4 address and port range to be used for
> +            the NAT by using the <code>&lt;nat&gt;</code> subelement.
> +            The address range is set with the <code>&lt;address&gt;</code>
> +            subelements and <code>start</code> and <code>stop</code>
> +            attributes:
>              <pre>
>  ...
>    &lt;forward mode='nat'&gt;
> @@ -154,6 +156,19 @@
>              <code>start</code> and <code>end</code> attributes to
>              the same value.
>              </p>
> +            <p>
> +            The port range to be used for the <code>&lt;nat&gt;</code> can
> +            be set via the subelement <code>&lt;port&gt;</code>:
> +            <pre>
> +...
> +  &lt;forward mode='nat'&gt;
> +    &lt;nat&gt;
> +      &lt;port start='500' end='1000'/&gt;
> +    &lt;/nat&gt;
> +  &lt;/forward&gt;
> +...
> +            </pre>
> +            </p>
>            </dd>
>  
>            <dt><code>route</code></dt>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 620110c..bfdbeef 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -1332,7 +1332,8 @@ virNetworkForwardNatDefParseXML(const char *networkName,
>  {
>      int ret = -1;
>      xmlNodePtr *natAddrNodes = NULL;
> -    int nNatAddrs;
> +    xmlNodePtr *natPortNodes = NULL;
> +    int nNatAddrs, nNatPorts;
>      char *addrStart = NULL;
>      char *addrEnd = NULL;
>      xmlNodePtr save = ctxt->node;
> @@ -1389,6 +1390,36 @@ virNetworkForwardNatDefParseXML(const char *networkName,
>          goto cleanup;
>      }
>  
> +    /* ports for SNAT and MASQUERADE */
> +    nNatPorts = virXPathNodeSet("./port", ctxt, &natPortNodes);
> +    if (nNatPorts < 0) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("invalid <port> element found in <forward> of "
> +                         "network %s"), networkName);
> +        goto cleanup;
> +    } else if (nNatPorts > 1) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("Only one <port> element is allowed in <nat> in "
> +                         "<forward> in network %s"), networkName);
> +        goto cleanup;
> +    } else if (nNatPorts == 1) {
> +        if (virXPathUInt("string(./port[1]/@start)", ctxt, &def->portStart) < 0
> +            || def->portStart > 65535) {
> +
> +            virReportError(VIR_ERR_XML_DETAIL,
> +                           _("Missing or invalid 'start' attribute in <port> "
> +                             "in <nat> in <forward> in network %s"),
> +                             networkName);
> +            goto cleanup;
> +        }
> +        if (virXPathUInt("string(./port[1]/@end)", ctxt, &def->portEnd) < 0
> +            || def->portEnd > 65535 || def->portEnd < def->portStart) {
> +            virReportError(VIR_ERR_XML_DETAIL,
> +                           _("Missing or invalid 'end' attribute in <port> in "
> +                             "<nat> in <forward> in network %s"), networkName);
> +            goto cleanup;
> +        }
> +    }
>      ret = 0;
>  
>  cleanup:
> @@ -2192,16 +2223,25 @@ virNatDefFormat(virBufferPtr buf,
>              goto cleanup;
>      }
>  
> -    if (!addrEnd && !addrStart)
> +    if (!addrEnd && !addrStart && !fwd->portStart && !fwd->portEnd)
>          return 0;
>  
>      virBufferAddLit(buf, "<nat>\n");
>      virBufferAdjustIndent(buf, 2);
>  
> -    virBufferAsprintf(buf, "<address start='%s'", addrStart);
> -    if (addrEnd)
> -        virBufferAsprintf(buf, " end='%s'", addrEnd);
> -    virBufferAsprintf(buf, "/>\n");
> +    if (addrStart) {
> +        virBufferAsprintf(buf, "<address start='%s'", addrStart);
> +        if (addrEnd)
> +            virBufferAsprintf(buf, " end='%s'", addrEnd);
> +        virBufferAsprintf(buf, "/>\n");
> +    }
> +
> +    if (fwd->portStart || fwd->portEnd) {
> +        virBufferAsprintf(buf, "<port start='%d'", fwd->portStart);
> +        if (fwd->portEnd)
> +            virBufferAsprintf(buf, " end='%d'", fwd->portEnd);
> +        virBufferAsprintf(buf, "/>\n");

Again - virBufferAddLit()

> +    }
>  
>      virBufferAdjustIndent(buf, -2);
>      virBufferAsprintf(buf, "</nat>\n");
> @@ -2259,7 +2299,9 @@ virNetworkDefFormatInternal(virBufferPtr buf,
>          }
>          shortforward = !(def->forward.nifs || def->forward.npfs
>                           || VIR_SOCKET_ADDR_VALID(&def->forward.addrStart)
> -                         || VIR_SOCKET_ADDR_VALID(&def->forward.addrEnd));
> +                         || VIR_SOCKET_ADDR_VALID(&def->forward.addrEnd)
> +                         || def->forward.portStart
> +                         || def->forward.portEnd);
>          virBufferAsprintf(buf, "%s>\n", shortforward ? "/" : "");
>          virBufferAdjustIndent(buf, 2);
>  
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index 11d6c9c..515115b 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -175,8 +175,9 @@ struct _virNetworkForwardDef {
>      size_t nifs;
>      virNetworkForwardIfDefPtr ifs;
>  
> -    /* adresses for SNAT */
> +    /* ranges for NAT */
>      virSocketAddr addrStart, addrEnd;
> +    unsigned int portStart, portEnd;
>  };
>  
>  typedef struct _virPortGroupDef virPortGroupDef;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 9f502a5..cf47ec4 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -1589,6 +1589,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
>                                       forwardIf,
>                                       &network->def->forward.addrStart,
>                                       &network->def->forward.addrEnd,
> +                                     network->def->forward.portStart,
> +                                     network->def->forward.portEnd,
>                                       NULL) < 0) {
>          virReportError(VIR_ERR_SYSTEM_ERROR,
>                         forwardIf ?
> @@ -1605,6 +1607,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
>                                       forwardIf,
>                                       &network->def->forward.addrStart,
>                                       &network->def->forward.addrEnd,
> +                                     network->def->forward.portStart,
> +                                     network->def->forward.portEnd,
>                                       "udp") < 0) {
>          virReportError(VIR_ERR_SYSTEM_ERROR,
>                         forwardIf ?
> @@ -1621,6 +1625,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
>                                       forwardIf,
>                                       &network->def->forward.addrStart,
>                                       &network->def->forward.addrEnd,
> +                                     network->def->forward.portStart,
> +                                     network->def->forward.portEnd,
>                                       "tcp") < 0) {
>          virReportError(VIR_ERR_SYSTEM_ERROR,
>                         forwardIf ?
> @@ -1639,6 +1645,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
>                                      forwardIf,
>                                      &network->def->forward.addrStart,
>                                      &network->def->forward.addrEnd,
> +                                    network->def->forward.portStart,
> +                                    network->def->forward.portEnd,
>                                      "udp");
>   masqerr4:
>      iptablesRemoveForwardMasquerade(driver->iptables,
> @@ -1647,6 +1655,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
>                                      forwardIf,
>                                      &network->def->forward.addrStart,
>                                      &network->def->forward.addrEnd,
> +                                    network->def->forward.portStart,
> +                                    network->def->forward.portEnd,
>                                      NULL);
>   masqerr3:
>      iptablesRemoveForwardAllowRelatedIn(driver->iptables,
> @@ -1679,6 +1689,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver,
>                                          forwardIf,
>                                          &network->def->forward.addrStart,
>                                          &network->def->forward.addrEnd,
> +                                        network->def->forward.portStart,
> +                                        network->def->forward.portEnd,
>                                          "tcp");
>          iptablesRemoveForwardMasquerade(driver->iptables,
>                                          &ipdef->address,
> @@ -1686,6 +1698,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver,
>                                          forwardIf,
>                                          &network->def->forward.addrStart,
>                                          &network->def->forward.addrEnd,
> +                                        network->def->forward.portStart,
> +                                        network->def->forward.portEnd,
>                                          "udp");
>          iptablesRemoveForwardMasquerade(driver->iptables,
>                                          &ipdef->address,
> @@ -1693,6 +1707,8 @@ networkRemoveMasqueradingIptablesRules(struct network_driver *driver,
>                                          forwardIf,
>                                          &network->def->forward.addrStart,
>                                          &network->def->forward.addrEnd,
> +                                        network->def->forward.portStart,
> +                                        network->def->forward.portEnd,
>                                          NULL);
>  
>          iptablesRemoveForwardAllowRelatedIn(driver->iptables,
> diff --git a/src/util/viriptables.c b/src/util/viriptables.c
> index f44236a..f9f28b4 100644
> --- a/src/util/viriptables.c
> +++ b/src/util/viriptables.c
> @@ -807,6 +807,8 @@ iptablesForwardMasquerade(iptablesContext *ctx,
>                            const char *physdev,
>                            virSocketAddr *addrStart,
>                            virSocketAddr *addrEnd,
> +                          unsigned int portStart,
> +                          unsigned int portEnd,
>                            const char *protocol,
>                            int action)
>  {
> @@ -815,6 +817,7 @@ iptablesForwardMasquerade(iptablesContext *ctx,
>      char *addrStartStr = NULL;
>      char *addrEndStr = NULL;
>      char *tmpStr = NULL;
> +    char *portRangeStr = NULL;
>      virCommandPtr cmd = NULL;
>  
>      if (!(networkstr = iptablesFormatNetwork(netaddr, prefix)))
> @@ -850,19 +853,34 @@ iptablesForwardMasquerade(iptablesContext *ctx,
>      if (physdev && physdev[0])
>          virCommandAddArgList(cmd, "--out-interface", physdev, NULL);
>  
> +    if (protocol && protocol[0]) {
> +        if (portStart == 0 && portEnd == 0) {
> +            portStart = 1024;
> +            portEnd = 65535;
> +        }
> +
> +        if (portStart < portEnd && portEnd < 65536) {
> +            if (virAsprintf(&portRangeStr, ":%u-%u", portStart, portEnd) < 0) {
> +                virReportOOMError();
> +                goto cleanup;
> +            }
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Invalid port range '%u-%u'."),
> +                           portStart, portEnd);
> +        }
> +    }
> +
>      /* Use --jump SNAT if public addr is specified */
>      if (addrStartStr && addrStartStr[0]) {
> -        const char *portStr = "";
>          int r = 0;
>  
> -        if (protocol && protocol[0])
> -            portStr = ":1024-65535";
> -
>          if (addrEndStr && addrEndStr[0]) {
>              r = virAsprintf(&tmpStr, "%s-%s%s", addrStartStr, addrEndStr,
> -                            portStr);
> +                            portRangeStr ? portRangeStr : "");
>          } else {
> -            r = virAsprintf(&tmpStr, "%s%s", addrStartStr, portStr);
> +            r = virAsprintf(&tmpStr, "%s%s", addrStartStr,
> +                            portRangeStr ? portRangeStr : "");
>          }
>  
>          if (r < 0) {
> @@ -875,14 +893,15 @@ iptablesForwardMasquerade(iptablesContext *ctx,
>       } else {
>           virCommandAddArgList(cmd, "--jump", "MASQUERADE", NULL);
>  
> -         if (protocol && protocol[0])
> -             virCommandAddArgList(cmd, "--to-ports", "1024-65535", NULL);
> +         if (portRangeStr && portRangeStr[0])
> +             virCommandAddArgList(cmd, "--to-ports", &portRangeStr[1], NULL);
>       }
>  
>      ret = iptablesCommandRunAndFree(cmd);
>  cleanup:
>      VIR_FREE(networkstr);
>      VIR_FREE(tmpStr);
> +    VIR_FREE(portRangeStr);
>      return ret;
>  }
>  
> @@ -906,9 +925,14 @@ iptablesAddForwardMasquerade(iptablesContext *ctx,
>                               const char *physdev,
>                               virSocketAddr *addrStart,
>                               virSocketAddr *addrEnd,
> +                             unsigned int portStart,
> +                             unsigned int portEnd,
>                               const char *protocol)
>  {
> -    return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addrStart, addrEnd, protocol, ADD);
> +    return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev,
> +                                     addrStart, addrEnd,
> +                                     portStart, portEnd,
> +                                     protocol, ADD);
>  }
>  
>  /**
> @@ -931,9 +955,14 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx,
>                                  const char *physdev,
>                                  virSocketAddr *addrStart,
>                                  virSocketAddr *addrEnd,
> +                                unsigned int portStart,
> +                                unsigned int portEnd,
>                                  const char *protocol)
>  {
> -    return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev, addrStart, addrEnd, protocol, REMOVE);
> +    return iptablesForwardMasquerade(ctx, netaddr, prefix, physdev,
> +                                     addrStart, addrEnd,
> +                                     portStart, portEnd,
> +                                     protocol, REMOVE);
>  }
>  
>  
> diff --git a/src/util/viriptables.h b/src/util/viriptables.h
> index 05362da..ca6adcc 100644
> --- a/src/util/viriptables.h
> +++ b/src/util/viriptables.h
> @@ -109,6 +109,8 @@ int              iptablesAddForwardMasquerade    (iptablesContext *ctx,
>                                                    const char *physdev,
>                                                    virSocketAddr *addrStart,
>                                                    virSocketAddr *addrEnd,
> +                                                  unsigned int portStart,
> +                                                  unsigned int portEnd,
>                                                    const char *protocol);
>  int              iptablesRemoveForwardMasquerade (iptablesContext *ctx,
>                                                    virSocketAddr *netaddr,
> @@ -116,6 +118,8 @@ int              iptablesRemoveForwardMasquerade (iptablesContext *ctx,
>                                                    const char *physdev,
>                                                    virSocketAddr *addrStart,
>                                                    virSocketAddr *addrEnd,
> +                                                  unsigned int portStart,
> +                                                  unsigned int portEnd,
>                                                    const char *protocol);
>  int              iptablesAddOutputFixUdpChecksum (iptablesContext *ctx,
>                                                    const char *iface,

ACK with the one small change I hadn't noticed before. I made that
change and pushed.


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