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

Guru Shetty guru at ovn.org
Thu Feb 15 19:02:36 UTC 2018


On 6 February 2018 at 20:21, Guoshuai Li <ligs at 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 at 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 at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list