diff mbox series

[ovs-dev] ovn-northd: Support logical router port with config CIDR, such as 192.168.1.0/24.

Message ID 20180207042135.11036-1-ligs@dtdream.com
State Changes Requested
Delegated to: Guru Shetty
Headers show
Series [ovs-dev] ovn-northd: Support logical router port with config CIDR, such as 192.168.1.0/24. | expand

Commit Message

Guoshuai Li Feb. 7, 2018, 4:21 a.m. UTC
Support logical router port who is gateway can configure CIDR whitout IP address,
so as to save the public network IP when connecting to the external network.

With this configuration, the gateway's default snat functionality will not be available and
VMs accessing the external network can only pass floatingip(snat_and_dnat).

The modify:

1. Logical switch Ingress table 11: ARP reply for logical router port IPs.
   - Do not reply ARP when logical router port without IP.

2. Logical router ingress table 5: IP Routing.
   - Use floatingIp (sNat IP) for routeing source IP in static routing.
   - Use floatingIp (sNat IP) for routeing source IP in direct routing.

3. Local router ingress table 6: ARP Resolution.
   - Do not reply ARP when logical router port without IP.

4. Logical router ingress table 1: IP Input for IPv4.
   - Do not reply ARP for the router own when logical router port without IP.

Signed-off-by: Guoshuai Li <ligs@dtdream.com>
---
 ovn/northd/ovn-northd.c | 114 +++++++++++++++++++++++++++++++++++++++++++-----
 ovn/ovn-nb.xml          |   7 +++
 tests/system-ovn.at     |   8 ++++
 3 files changed, 118 insertions(+), 11 deletions(-)

Comments

Ben Pfaff Feb. 15, 2018, 12:29 a.m. UTC | #1
Guru, does it make sense for you to look at this?  You understand
gateways better than me.

On Wed, Feb 07, 2018 at 12:21:35PM +0800, Guoshuai Li wrote:
> Support logical router port who is gateway can configure CIDR whitout IP address,
> so as to save the public network IP when connecting to the external network.
> 
> With this configuration, the gateway's default snat functionality will not be available and
> VMs accessing the external network can only pass floatingip(snat_and_dnat).
> 
> The modify:
> 
> 1. Logical switch Ingress table 11: ARP reply for logical router port IPs.
>    - Do not reply ARP when logical router port without IP.
> 
> 2. Logical router ingress table 5: IP Routing.
>    - Use floatingIp (sNat IP) for routeing source IP in static routing.
>    - Use floatingIp (sNat IP) for routeing source IP in direct routing.
> 
> 3. Local router ingress table 6: ARP Resolution.
>    - Do not reply ARP when logical router port without IP.
> 
> 4. Logical router ingress table 1: IP Input for IPv4.
>    - Do not reply ARP for the router own when logical router port without IP.
> 
> Signed-off-by: Guoshuai Li <ligs@dtdream.com>
> ---
>  ovn/northd/ovn-northd.c | 114 +++++++++++++++++++++++++++++++++++++++++++-----
>  ovn/ovn-nb.xml          |   7 +++
>  tests/system-ovn.at     |   8 ++++
>  3 files changed, 118 insertions(+), 11 deletions(-)
> 
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 4d95a3d9d..c98bf2ab6 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -3548,6 +3548,17 @@ build_stateful(struct ovn_datapath *od, struct hmap *lflows)
>      }
>  }
>  
> +/* Return to whether Port is only configured cidr. */
> +static bool
> +op_is_v4_network_cidr(const struct ipv4_netaddr *ipv4_addrs)
> +{
> +    if ((ipv4_addrs->plen != 32) &&
> +        (ipv4_addrs->addr == ipv4_addrs->network)) {
> +        return true;
> +    }
> +    return false;
> +}
> +
>  static void
>  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
>                      struct hmap *lflows, struct hmap *mcgroups)
> @@ -3679,6 +3690,10 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
>  
>          for (size_t i = 0; i < op->n_lsp_addrs; i++) {
>              for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) {
> +                if (!strcmp(op->nbsp->type, "router") &&
> +                    op_is_v4_network_cidr(&op->lsp_addrs[i].ipv4_addrs[j])) {
> +                        continue;
> +                }
>                  ds_clear(&match);
>                  ds_put_format(&match, "arp.tpa == %s && arp.op == 1",
>                                op->lsp_addrs[i].ipv4_addrs[j].addr_s);
> @@ -4100,8 +4115,32 @@ lrport_is_enabled(const struct nbrec_logical_router_port *lrport)
>      return !lrport->enabled || *lrport->enabled;
>  }
>  
> +/* Returns a string of the net external_ip of the router port 'op'.
> + * If one is not found, returns NULL.
> + *
> + * The caller must not free the returned string. */
> +static const char *
> +find_lrp_external_ip(const struct ovn_port *op)
> +{
> +    for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> +        const struct ipv4_netaddr *na = &op->lrp_networks.ipv4_addrs[i];
> +        for (size_t j = 0; j < op->od->nbr->n_nat; j++) {
> +            const struct nbrec_nat *nat = op->od->nbr->nat[j];
> +            ovs_be32 external_ip;
> +            if (!ip_parse(nat->external_ip, &external_ip)) {
> +                continue;
> +            }
> +            if (!((na->network ^ external_ip) & na->mask)) {
> +                return nat->external_ip;
> +            }
> +        }
> +    }
> +    return NULL;
> +}
> +
>  /* Returns a string of the IP address of the router port 'op' that
> - * overlaps with 'ip_s".  If one is not found, returns NULL.
> + * overlaps with 'ip_s". Or Returns a string of the nat external_ip
> + * address of the router port 'op', If one is not found, returns NULL.
>   *
>   * The caller must not free the returned string. */
>  static const char *
> @@ -4118,15 +4157,27 @@ find_lrp_member_ip(const struct ovn_port *op, const char *ip_s)
>              return NULL;
>          }
>  
> +        bool find_port = false;
>          for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
>              const struct ipv4_netaddr *na = &op->lrp_networks.ipv4_addrs[i];
>  
>              if (!((na->network ^ ip) & na->mask)) {
> +                find_port = true;
>                  /* There should be only 1 interface that matches the
>                   * supplied IP.  Otherwise, it's a configuration error,
>                   * because subnets of a router's interfaces should NOT
>                   * overlap. */
> -                return na->addr_s;
> +                if (na->addr != na->network) {
> +                    return na->addr_s;
> +                }
> +            }
> +        }
> +
> +        if (find_port) {
> +            /* Get NAT external IP addresses. */
> +            const char *external_ip = find_lrp_external_ip(op);
> +            if (external_ip) {
> +                return external_ip;
>              }
>          }
>      } else {
> @@ -4156,6 +4207,20 @@ find_lrp_member_ip(const struct ovn_port *op, const char *ip_s)
>      return NULL;
>  }
>  
> +static int
> +op_get_v4_ip_number(const struct ovn_port *op)
> +{
> +    int n = 0;
> +    if (op->lrp_networks.n_ipv4_addrs) {
> +        for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> +            if (!op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
> +                n++;
> +            }
> +        }
> +    }
> +    return n;
> +}
> +
>  static void
>  add_route(struct hmap *lflows, const struct ovn_port *op,
>            const char *lrp_addr_s, const char *network_s, int plen,
> @@ -4296,12 +4361,19 @@ build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od,
>              /* There are no IP networks configured on the router's port via
>               * which 'route->nexthop' is theoretically reachable.  But since
>               * 'out_port' has been specified, we honor it by trying to reach
> -             * 'route->nexthop' via the first IP address of 'out_port'.
> +             * 'route->nexthop' via the first IP address  of 'out_port' or
> +             * nat external IP addresses of 'out_port'.
>               * (There are cases, e.g in GCE, where each VM gets a /32 IP
>               * address and the default gateway is still reachable from it.) */
>              if (is_ipv4) {
>                  if (out_port->lrp_networks.n_ipv4_addrs) {
> -                    lrp_addr_s = out_port->lrp_networks.ipv4_addrs[0].addr_s;
> +                    struct lport_addresses *address = &out_port->lrp_networks;
> +                    for (int i = 0; i < address->n_ipv4_addrs; i++) {
> +                        if (!op_is_v4_network_cidr(&address->ipv4_addrs[i])) {
> +                            lrp_addr_s = address->ipv4_addrs[i].addr_s;
> +                            break;
> +                        }
> +                    }
>                  }
>              } else {
>                  if (out_port->lrp_networks.n_ipv6_addrs) {
> @@ -4328,7 +4400,7 @@ build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od,
>          }
>      }
>  
> -    if (!out_port || !lrp_addr_s) {
> +    if (!out_port) {
>          /* There is no matched out port. */
>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>          VLOG_WARN_RL(&rl, "No path for static route %s; next hop %s",
> @@ -4336,6 +4408,14 @@ build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od,
>          goto free_prefix_s;
>      }
>  
> +    if (!lrp_addr_s) {
> +        /* There is no matched lrp_addr_s. */
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +        VLOG_DBG_RL(&rl, "No source ip for static route %s; next hop %s",
> +                     route->ip_prefix, route->nexthop);
> +        goto free_prefix_s;
> +    }
> +
>      char *policy = route->policy ? route->policy : "dst-ip";
>      add_route(lflows, out_port, lrp_addr_s, prefix_s, plen, route->nexthop,
>                policy);
> @@ -4347,14 +4427,16 @@ free_prefix_s:
>  static void
>  op_put_v4_networks(struct ds *ds, const struct ovn_port *op, bool add_bcast)
>  {
> -    if (!add_bcast && op->lrp_networks.n_ipv4_addrs == 1) {
> +    if (!add_bcast && op_get_v4_ip_number(op) == 1) {
>          ds_put_format(ds, "%s", op->lrp_networks.ipv4_addrs[0].addr_s);
>          return;
>      }
>  
>      ds_put_cstr(ds, "{");
>      for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> -        ds_put_format(ds, "%s, ", op->lrp_networks.ipv4_addrs[i].addr_s);
> +        if (!op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
> +            ds_put_format(ds, "%s, ", op->lrp_networks.ipv4_addrs[i].addr_s);
> +        }
>          if (add_bcast) {
>              ds_put_format(ds, "%s, ", op->lrp_networks.ipv4_addrs[i].bcast_s);
>          }
> @@ -4685,7 +4767,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>              continue;
>          }
>  
> -        if (op->lrp_networks.n_ipv4_addrs) {
> +        if (op_get_v4_ip_number(op)) {
>              /* L3 admission control: drop packets that originate from an
>               * IPv4 address owned by the router or a broadcast address
>               * known to the router (priority 100). */
> @@ -4720,6 +4802,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>          /* ARP reply.  These flows reply to ARP requests for the router's own
>           * IP address. */
>          for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> +            if (op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
> +                continue;
> +            }
>              ds_clear(&match);
>              ds_put_format(&match,
>                            "inport == %s && arp.tpa == %s && arp.op == 1",
> @@ -5660,7 +5745,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>          }
>  
>          for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> -            add_route(lflows, op, op->lrp_networks.ipv4_addrs[i].addr_s,
> +            const char *lrp_addr_s = op->lrp_networks.ipv4_addrs[i].addr_s;
> +            if (op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
> +                lrp_addr_s = find_lrp_member_ip(op, lrp_addr_s);
> +                if (!lrp_addr_s) {
> +                    continue;
> +                }
> +            }
> +            add_route(lflows, op, lrp_addr_s,
>                        op->lrp_networks.ipv4_addrs[i].network_s,
>                        op->lrp_networks.ipv4_addrs[i].plen, NULL, NULL);
>          }
> @@ -5709,7 +5801,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>               * The packet is still in peer's logical pipeline. So the match
>               * should be on peer's outport. */
>              if (op->peer && op->nbrp->peer) {
> -                if (op->lrp_networks.n_ipv4_addrs) {
> +                if (op_get_v4_ip_number(op)) {
>                      ds_clear(&match);
>                      ds_put_format(&match, "outport == %s && reg0 == ",
>                                    op->peer->json_key);
> @@ -5846,7 +5938,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>                     continue;
>                  }
>  
> -                if (router_port->lrp_networks.n_ipv4_addrs) {
> +                if (op_get_v4_ip_number(router_port)) {
>                      ds_clear(&match);
>                      ds_put_format(&match, "outport == %s && reg0 == ",
>                                    peer->json_key);
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index b7a5b6bf2..fc9fd42ff 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -1345,6 +1345,13 @@
>        </p>
>  
>        <p>
> +        A logical router port who is gateway port also can configure CIDR,
> +        For example, <code>192.168.0.0/24</code>. This means that traffic
> +        to the external network can only pass NAT's external_ip. This
> +        does not make sense for normal logical router port.
> +      </p>
> +
> +      <p>
>          A logical router port always adds a link-local IPv6 address
>          (fe80::/64) automatically generated from the interface's MAC
>          address using the modified EUI-64 format.
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 638c0b661..5bca5e3d2 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -1347,6 +1347,14 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
>  icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared>
>  ])
>  
> +ovn-nbctl set Logical_Router_Port alice networks=172.16.1.0/24
> +
> +# North-South DNAT without gatewayIp: 'alice1' pings 'foo1' using 172.16.1.3.
> +NS_CHECK_EXEC([alice1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.3 | FORMAT_PING], \
> +[0], [dnl
> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> +])
> +
>  OVS_APP_EXIT_AND_WAIT([ovn-controller])
>  
>  as ovn-sb
> -- 
> 2.13.2.windows.1
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Gurucharan Shetty Feb. 15, 2018, 7:02 p.m. UTC | #2
On 6 February 2018 at 20:21, Guoshuai Li <ligs@dtdream.com> wrote:

> Support logical router port who is gateway can configure CIDR whitout IP
> address,
> so as to save the public network IP when connecting to the external
> network.
>
> With this configuration, the gateway's default snat functionality will not
> be available and
> VMs accessing the external network can only pass floatingip(snat_and_dnat).
>
> The modify:
>
> 1. Logical switch Ingress table 11: ARP reply for logical router port IPs.
>    - Do not reply ARP when logical router port without IP.
>
> 2. Logical router ingress table 5: IP Routing.
>    - Use floatingIp (sNat IP) for routeing source IP in static routing.
>    - Use floatingIp (sNat IP) for routeing source IP in direct routing.
>
> 3. Local router ingress table 6: ARP Resolution.
>    - Do not reply ARP when logical router port without IP.
>
> 4. Logical router ingress table 1: IP Input for IPv4.
>    - Do not reply ARP for the router own when logical router port without
> IP.
>
> Signed-off-by: Guoshuai Li <ligs@dtdream.com>
>

Are there real routers which accept a configuration like this? It makes me
nervous when we change things away from real networking for small gains
that adds code complexity. In this case, it looks like there is really no
gain. If you are going to have atleast one snat_and_dnat IP, why not use it
for router IP? If you don't have any snat_and_dnat IP, then why configure
the gateway router in the first place?


> ---
>  ovn/northd/ovn-northd.c | 114 ++++++++++++++++++++++++++++++
> +++++++++++++-----
>  ovn/ovn-nb.xml          |   7 +++
>  tests/system-ovn.at     |   8 ++++
>  3 files changed, 118 insertions(+), 11 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 4d95a3d9d..c98bf2ab6 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -3548,6 +3548,17 @@ build_stateful(struct ovn_datapath *od, struct hmap
> *lflows)
>      }
>  }
>
> +/* Return to whether Port is only configured cidr. */
> +static bool
> +op_is_v4_network_cidr(const struct ipv4_netaddr *ipv4_addrs)
> +{
> +    if ((ipv4_addrs->plen != 32) &&
> +        (ipv4_addrs->addr == ipv4_addrs->network)) {
> +        return true;
> +    }
> +    return false;
> +}
> +
>  static void
>  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
>                      struct hmap *lflows, struct hmap *mcgroups)
> @@ -3679,6 +3690,10 @@ build_lswitch_flows(struct hmap *datapaths, struct
> hmap *ports,
>
>          for (size_t i = 0; i < op->n_lsp_addrs; i++) {
>              for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) {
> +                if (!strcmp(op->nbsp->type, "router") &&
> +                    op_is_v4_network_cidr(&op->lsp_addrs[i].ipv4_addrs[j]))
> {
> +                        continue;
> +                }
>                  ds_clear(&match);
>                  ds_put_format(&match, "arp.tpa == %s && arp.op == 1",
>                                op->lsp_addrs[i].ipv4_addrs[j].addr_s);
> @@ -4100,8 +4115,32 @@ lrport_is_enabled(const struct
> nbrec_logical_router_port *lrport)
>      return !lrport->enabled || *lrport->enabled;
>  }
>
> +/* Returns a string of the net external_ip of the router port 'op'.
> + * If one is not found, returns NULL.
> + *
> + * The caller must not free the returned string. */
> +static const char *
> +find_lrp_external_ip(const struct ovn_port *op)
> +{
> +    for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> +        const struct ipv4_netaddr *na = &op->lrp_networks.ipv4_addrs[i];
> +        for (size_t j = 0; j < op->od->nbr->n_nat; j++) {
> +            const struct nbrec_nat *nat = op->od->nbr->nat[j];
> +            ovs_be32 external_ip;
> +            if (!ip_parse(nat->external_ip, &external_ip)) {
> +                continue;
> +            }
> +            if (!((na->network ^ external_ip) & na->mask)) {
> +                return nat->external_ip;
> +            }
> +        }
> +    }
> +    return NULL;
> +}
> +
>  /* Returns a string of the IP address of the router port 'op' that
> - * overlaps with 'ip_s".  If one is not found, returns NULL.
> + * overlaps with 'ip_s". Or Returns a string of the nat external_ip
> + * address of the router port 'op', If one is not found, returns NULL.
>   *
>   * The caller must not free the returned string. */
>  static const char *
> @@ -4118,15 +4157,27 @@ find_lrp_member_ip(const struct ovn_port *op,
> const char *ip_s)
>              return NULL;
>          }
>
> +        bool find_port = false;
>          for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
>              const struct ipv4_netaddr *na = &op->lrp_networks.ipv4_addrs[
> i];
>
>              if (!((na->network ^ ip) & na->mask)) {
> +                find_port = true;
>                  /* There should be only 1 interface that matches the
>                   * supplied IP.  Otherwise, it's a configuration error,
>                   * because subnets of a router's interfaces should NOT
>                   * overlap. */
> -                return na->addr_s;
> +                if (na->addr != na->network) {
> +                    return na->addr_s;
> +                }
> +            }
> +        }
> +
> +        if (find_port) {
> +            /* Get NAT external IP addresses. */
> +            const char *external_ip = find_lrp_external_ip(op);
> +            if (external_ip) {
> +                return external_ip;
>              }
>          }
>      } else {
> @@ -4156,6 +4207,20 @@ find_lrp_member_ip(const struct ovn_port *op, const
> char *ip_s)
>      return NULL;
>  }
>
> +static int
> +op_get_v4_ip_number(const struct ovn_port *op)
> +{
> +    int n = 0;
> +    if (op->lrp_networks.n_ipv4_addrs) {
> +        for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> +            if (!op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i]))
> {
> +                n++;
> +            }
> +        }
> +    }
> +    return n;
> +}
> +
>  static void
>  add_route(struct hmap *lflows, const struct ovn_port *op,
>            const char *lrp_addr_s, const char *network_s, int plen,
> @@ -4296,12 +4361,19 @@ build_static_route_flow(struct hmap *lflows,
> struct ovn_datapath *od,
>              /* There are no IP networks configured on the router's port
> via
>               * which 'route->nexthop' is theoretically reachable.  But
> since
>               * 'out_port' has been specified, we honor it by trying to
> reach
> -             * 'route->nexthop' via the first IP address of 'out_port'.
> +             * 'route->nexthop' via the first IP address  of 'out_port' or
> +             * nat external IP addresses of 'out_port'.
>               * (There are cases, e.g in GCE, where each VM gets a /32 IP
>               * address and the default gateway is still reachable from
> it.) */
>              if (is_ipv4) {
>                  if (out_port->lrp_networks.n_ipv4_addrs) {
> -                    lrp_addr_s = out_port->lrp_networks.ipv4_
> addrs[0].addr_s;
> +                    struct lport_addresses *address =
> &out_port->lrp_networks;
> +                    for (int i = 0; i < address->n_ipv4_addrs; i++) {
> +                        if (!op_is_v4_network_cidr(&address->ipv4_addrs[i]))
> {
> +                            lrp_addr_s = address->ipv4_addrs[i].addr_s;
> +                            break;
> +                        }
> +                    }
>                  }
>              } else {
>                  if (out_port->lrp_networks.n_ipv6_addrs) {
> @@ -4328,7 +4400,7 @@ build_static_route_flow(struct hmap *lflows, struct
> ovn_datapath *od,
>          }
>      }
>
> -    if (!out_port || !lrp_addr_s) {
> +    if (!out_port) {
>          /* There is no matched out port. */
>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>          VLOG_WARN_RL(&rl, "No path for static route %s; next hop %s",
> @@ -4336,6 +4408,14 @@ build_static_route_flow(struct hmap *lflows, struct
> ovn_datapath *od,
>          goto free_prefix_s;
>      }
>
> +    if (!lrp_addr_s) {
> +        /* There is no matched lrp_addr_s. */
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +        VLOG_DBG_RL(&rl, "No source ip for static route %s; next hop %s",
> +                     route->ip_prefix, route->nexthop);
> +        goto free_prefix_s;
> +    }
> +
>      char *policy = route->policy ? route->policy : "dst-ip";
>      add_route(lflows, out_port, lrp_addr_s, prefix_s, plen,
> route->nexthop,
>                policy);
> @@ -4347,14 +4427,16 @@ free_prefix_s:
>  static void
>  op_put_v4_networks(struct ds *ds, const struct ovn_port *op, bool
> add_bcast)
>  {
> -    if (!add_bcast && op->lrp_networks.n_ipv4_addrs == 1) {
> +    if (!add_bcast && op_get_v4_ip_number(op) == 1) {
>          ds_put_format(ds, "%s", op->lrp_networks.ipv4_addrs[0].addr_s);
>          return;
>      }
>
>      ds_put_cstr(ds, "{");
>      for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> -        ds_put_format(ds, "%s, ", op->lrp_networks.ipv4_addrs[i].addr_s);
> +        if (!op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
> +            ds_put_format(ds, "%s, ", op->lrp_networks.ipv4_addrs[i]
> .addr_s);
> +        }
>          if (add_bcast) {
>              ds_put_format(ds, "%s, ", op->lrp_networks.ipv4_addrs[i]
> .bcast_s);
>          }
> @@ -4685,7 +4767,7 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>              continue;
>          }
>
> -        if (op->lrp_networks.n_ipv4_addrs) {
> +        if (op_get_v4_ip_number(op)) {
>              /* L3 admission control: drop packets that originate from an
>               * IPv4 address owned by the router or a broadcast address
>               * known to the router (priority 100). */
> @@ -4720,6 +4802,9 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>          /* ARP reply.  These flows reply to ARP requests for the router's
> own
>           * IP address. */
>          for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> +            if (op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
> +                continue;
> +            }
>              ds_clear(&match);
>              ds_put_format(&match,
>                            "inport == %s && arp.tpa == %s && arp.op == 1",
> @@ -5660,7 +5745,14 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>          }
>
>          for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> -            add_route(lflows, op, op->lrp_networks.ipv4_addrs[i].addr_s,
> +            const char *lrp_addr_s = op->lrp_networks.ipv4_addrs[i]
> .addr_s;
> +            if (op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
> +                lrp_addr_s = find_lrp_member_ip(op, lrp_addr_s);
> +                if (!lrp_addr_s) {
> +                    continue;
> +                }
> +            }
> +            add_route(lflows, op, lrp_addr_s,
>                        op->lrp_networks.ipv4_addrs[i].network_s,
>                        op->lrp_networks.ipv4_addrs[i].plen, NULL, NULL);
>          }
> @@ -5709,7 +5801,7 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>               * The packet is still in peer's logical pipeline. So the
> match
>               * should be on peer's outport. */
>              if (op->peer && op->nbrp->peer) {
> -                if (op->lrp_networks.n_ipv4_addrs) {
> +                if (op_get_v4_ip_number(op)) {
>                      ds_clear(&match);
>                      ds_put_format(&match, "outport == %s && reg0 == ",
>                                    op->peer->json_key);
> @@ -5846,7 +5938,7 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>                     continue;
>                  }
>
> -                if (router_port->lrp_networks.n_ipv4_addrs) {
> +                if (op_get_v4_ip_number(router_port)) {
>                      ds_clear(&match);
>                      ds_put_format(&match, "outport == %s && reg0 == ",
>                                    peer->json_key);
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index b7a5b6bf2..fc9fd42ff 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -1345,6 +1345,13 @@
>        </p>
>
>        <p>
> +        A logical router port who is gateway port also can configure CIDR,
> +        For example, <code>192.168.0.0/24</code>. This means that traffic
> +        to the external network can only pass NAT's external_ip. This
> +        does not make sense for normal logical router port.
> +      </p>
> +
> +      <p>
>          A logical router port always adds a link-local IPv6 address
>          (fe80::/64) automatically generated from the interface's MAC
>          address using the modified EUI-64 format.
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 638c0b661..5bca5e3d2 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -1347,6 +1347,14 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0],
> [dnl
>  icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,
> type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<
> cleared>,type=0,code=0),zone=<cleared>
>  ])
>
> +ovn-nbctl set Logical_Router_Port alice networks=172.16.1.0/24
> +
> +# North-South DNAT without gatewayIp: 'alice1' pings 'foo1' using
> 172.16.1.3.
> +NS_CHECK_EXEC([alice1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.3 |
> FORMAT_PING], \
> +[0], [dnl
> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> +])
> +
>  OVS_APP_EXIT_AND_WAIT([ovn-controller])
>
>  as ovn-sb
> --
> 2.13.2.windows.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 4d95a3d9d..c98bf2ab6 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -3548,6 +3548,17 @@  build_stateful(struct ovn_datapath *od, struct hmap *lflows)
     }
 }
 
+/* Return to whether Port is only configured cidr. */
+static bool
+op_is_v4_network_cidr(const struct ipv4_netaddr *ipv4_addrs)
+{
+    if ((ipv4_addrs->plen != 32) &&
+        (ipv4_addrs->addr == ipv4_addrs->network)) {
+        return true;
+    }
+    return false;
+}
+
 static void
 build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
                     struct hmap *lflows, struct hmap *mcgroups)
@@ -3679,6 +3690,10 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
 
         for (size_t i = 0; i < op->n_lsp_addrs; i++) {
             for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) {
+                if (!strcmp(op->nbsp->type, "router") &&
+                    op_is_v4_network_cidr(&op->lsp_addrs[i].ipv4_addrs[j])) {
+                        continue;
+                }
                 ds_clear(&match);
                 ds_put_format(&match, "arp.tpa == %s && arp.op == 1",
                               op->lsp_addrs[i].ipv4_addrs[j].addr_s);
@@ -4100,8 +4115,32 @@  lrport_is_enabled(const struct nbrec_logical_router_port *lrport)
     return !lrport->enabled || *lrport->enabled;
 }
 
+/* Returns a string of the net external_ip of the router port 'op'.
+ * If one is not found, returns NULL.
+ *
+ * The caller must not free the returned string. */
+static const char *
+find_lrp_external_ip(const struct ovn_port *op)
+{
+    for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
+        const struct ipv4_netaddr *na = &op->lrp_networks.ipv4_addrs[i];
+        for (size_t j = 0; j < op->od->nbr->n_nat; j++) {
+            const struct nbrec_nat *nat = op->od->nbr->nat[j];
+            ovs_be32 external_ip;
+            if (!ip_parse(nat->external_ip, &external_ip)) {
+                continue;
+            }
+            if (!((na->network ^ external_ip) & na->mask)) {
+                return nat->external_ip;
+            }
+        }
+    }
+    return NULL;
+}
+
 /* Returns a string of the IP address of the router port 'op' that
- * overlaps with 'ip_s".  If one is not found, returns NULL.
+ * overlaps with 'ip_s". Or Returns a string of the nat external_ip
+ * address of the router port 'op', If one is not found, returns NULL.
  *
  * The caller must not free the returned string. */
 static const char *
@@ -4118,15 +4157,27 @@  find_lrp_member_ip(const struct ovn_port *op, const char *ip_s)
             return NULL;
         }
 
+        bool find_port = false;
         for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
             const struct ipv4_netaddr *na = &op->lrp_networks.ipv4_addrs[i];
 
             if (!((na->network ^ ip) & na->mask)) {
+                find_port = true;
                 /* There should be only 1 interface that matches the
                  * supplied IP.  Otherwise, it's a configuration error,
                  * because subnets of a router's interfaces should NOT
                  * overlap. */
-                return na->addr_s;
+                if (na->addr != na->network) {
+                    return na->addr_s;
+                }
+            }
+        }
+
+        if (find_port) {
+            /* Get NAT external IP addresses. */
+            const char *external_ip = find_lrp_external_ip(op);
+            if (external_ip) {
+                return external_ip;
             }
         }
     } else {
@@ -4156,6 +4207,20 @@  find_lrp_member_ip(const struct ovn_port *op, const char *ip_s)
     return NULL;
 }
 
+static int
+op_get_v4_ip_number(const struct ovn_port *op)
+{
+    int n = 0;
+    if (op->lrp_networks.n_ipv4_addrs) {
+        for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
+            if (!op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
+                n++;
+            }
+        }
+    }
+    return n;
+}
+
 static void
 add_route(struct hmap *lflows, const struct ovn_port *op,
           const char *lrp_addr_s, const char *network_s, int plen,
@@ -4296,12 +4361,19 @@  build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od,
             /* There are no IP networks configured on the router's port via
              * which 'route->nexthop' is theoretically reachable.  But since
              * 'out_port' has been specified, we honor it by trying to reach
-             * 'route->nexthop' via the first IP address of 'out_port'.
+             * 'route->nexthop' via the first IP address  of 'out_port' or
+             * nat external IP addresses of 'out_port'.
              * (There are cases, e.g in GCE, where each VM gets a /32 IP
              * address and the default gateway is still reachable from it.) */
             if (is_ipv4) {
                 if (out_port->lrp_networks.n_ipv4_addrs) {
-                    lrp_addr_s = out_port->lrp_networks.ipv4_addrs[0].addr_s;
+                    struct lport_addresses *address = &out_port->lrp_networks;
+                    for (int i = 0; i < address->n_ipv4_addrs; i++) {
+                        if (!op_is_v4_network_cidr(&address->ipv4_addrs[i])) {
+                            lrp_addr_s = address->ipv4_addrs[i].addr_s;
+                            break;
+                        }
+                    }
                 }
             } else {
                 if (out_port->lrp_networks.n_ipv6_addrs) {
@@ -4328,7 +4400,7 @@  build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od,
         }
     }
 
-    if (!out_port || !lrp_addr_s) {
+    if (!out_port) {
         /* There is no matched out port. */
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
         VLOG_WARN_RL(&rl, "No path for static route %s; next hop %s",
@@ -4336,6 +4408,14 @@  build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od,
         goto free_prefix_s;
     }
 
+    if (!lrp_addr_s) {
+        /* There is no matched lrp_addr_s. */
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+        VLOG_DBG_RL(&rl, "No source ip for static route %s; next hop %s",
+                     route->ip_prefix, route->nexthop);
+        goto free_prefix_s;
+    }
+
     char *policy = route->policy ? route->policy : "dst-ip";
     add_route(lflows, out_port, lrp_addr_s, prefix_s, plen, route->nexthop,
               policy);
@@ -4347,14 +4427,16 @@  free_prefix_s:
 static void
 op_put_v4_networks(struct ds *ds, const struct ovn_port *op, bool add_bcast)
 {
-    if (!add_bcast && op->lrp_networks.n_ipv4_addrs == 1) {
+    if (!add_bcast && op_get_v4_ip_number(op) == 1) {
         ds_put_format(ds, "%s", op->lrp_networks.ipv4_addrs[0].addr_s);
         return;
     }
 
     ds_put_cstr(ds, "{");
     for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
-        ds_put_format(ds, "%s, ", op->lrp_networks.ipv4_addrs[i].addr_s);
+        if (!op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
+            ds_put_format(ds, "%s, ", op->lrp_networks.ipv4_addrs[i].addr_s);
+        }
         if (add_bcast) {
             ds_put_format(ds, "%s, ", op->lrp_networks.ipv4_addrs[i].bcast_s);
         }
@@ -4685,7 +4767,7 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
             continue;
         }
 
-        if (op->lrp_networks.n_ipv4_addrs) {
+        if (op_get_v4_ip_number(op)) {
             /* L3 admission control: drop packets that originate from an
              * IPv4 address owned by the router or a broadcast address
              * known to the router (priority 100). */
@@ -4720,6 +4802,9 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
         /* ARP reply.  These flows reply to ARP requests for the router's own
          * IP address. */
         for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
+            if (op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
+                continue;
+            }
             ds_clear(&match);
             ds_put_format(&match,
                           "inport == %s && arp.tpa == %s && arp.op == 1",
@@ -5660,7 +5745,14 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
         }
 
         for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
-            add_route(lflows, op, op->lrp_networks.ipv4_addrs[i].addr_s,
+            const char *lrp_addr_s = op->lrp_networks.ipv4_addrs[i].addr_s;
+            if (op_is_v4_network_cidr(&op->lrp_networks.ipv4_addrs[i])) {
+                lrp_addr_s = find_lrp_member_ip(op, lrp_addr_s);
+                if (!lrp_addr_s) {
+                    continue;
+                }
+            }
+            add_route(lflows, op, lrp_addr_s,
                       op->lrp_networks.ipv4_addrs[i].network_s,
                       op->lrp_networks.ipv4_addrs[i].plen, NULL, NULL);
         }
@@ -5709,7 +5801,7 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
              * The packet is still in peer's logical pipeline. So the match
              * should be on peer's outport. */
             if (op->peer && op->nbrp->peer) {
-                if (op->lrp_networks.n_ipv4_addrs) {
+                if (op_get_v4_ip_number(op)) {
                     ds_clear(&match);
                     ds_put_format(&match, "outport == %s && reg0 == ",
                                   op->peer->json_key);
@@ -5846,7 +5938,7 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                    continue;
                 }
 
-                if (router_port->lrp_networks.n_ipv4_addrs) {
+                if (op_get_v4_ip_number(router_port)) {
                     ds_clear(&match);
                     ds_put_format(&match, "outport == %s && reg0 == ",
                                   peer->json_key);
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index b7a5b6bf2..fc9fd42ff 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -1345,6 +1345,13 @@ 
       </p>
 
       <p>
+        A logical router port who is gateway port also can configure CIDR,
+        For example, <code>192.168.0.0/24</code>. This means that traffic
+        to the external network can only pass NAT's external_ip. This
+        does not make sense for normal logical router port.
+      </p>
+
+      <p>
         A logical router port always adds a link-local IPv6 address
         (fe80::/64) automatically generated from the interface's MAC
         address using the modified EUI-64 format.
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 638c0b661..5bca5e3d2 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -1347,6 +1347,14 @@  sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
 icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>,type=0,code=0),zone=<cleared>
 ])
 
+ovn-nbctl set Logical_Router_Port alice networks=172.16.1.0/24
+
+# North-South DNAT without gatewayIp: 'alice1' pings 'foo1' using 172.16.1.3.
+NS_CHECK_EXEC([alice1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.3 | FORMAT_PING], \
+[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
 OVS_APP_EXIT_AND_WAIT([ovn-controller])
 
 as ovn-sb