[ovs-dev] [PATCH 1/1] Add Static route to logical router

Guru Shetty guru at ovn.org
Thu Mar 31 17:39:42 UTC 2016


On 30 March 2016 at 19:36, Shi Xin Ruan <Steve.ruan at cn.ibm.com> wrote:

>
> This patch add static route to logical router which are required in many
> scenarios, such as VPNaas service, l3 gateway.
> For VPNaas, OVN logical router SHOULD be able to route the VPN remote
> subnet to the
> VPN VM, static route is the best choice.
>
> This patach will add a new pointer array in "Logical_Router" table, the
> pointers will point to a new table "Logical_Router_Static_Route" to record
> the static routes.
>
> Current logical router already has default route. This patach will add
> static route will ovn northd update the router logical flow. Static route
> will add before the default router.
>
> ovn/northd/ovn-northd.c     Add static route logical flow
> ovn/ovn-nb.ovsschema        Add staic route table
> ovn/utilities/ovn-nbctl.c   Add static route to ovn-nbctl command
>
> Test case updates:
> Add static route test in case: 3 HVs, 3 LS, 3 lports/LS, 1 LR
>
> Signed-off-by: Steve Ruan <ruansx at cn.ibm.com>,
> Reported-by: Na Zhu <nazhu at cn.ibm.com>
> Reported-by: Dustin Lundquist <dlundquist at linux.vnet.ibm.com>
>


I pretty much had a similar patch in my queue. But I do not use the
additional table but rather just use the static_routes as a column which is
a map of "subnet:next_hop". e.g "192.168.1.0/24:20.0.0.2". You could
potentially use the similar model except that you have introduced a new
"valid" column in your new table. Why do you need that column?

Also, you will have to add a unit test wherein you connect multiple
routers.

I haven't looked at the code itself very closely.

>
> Reported-at:
> https://bugs.launchpad.net/networking-ovn/+bug/1545140
> https://bugs.launchpad.net/networking-ovn/+bug/1539347
>
>  ovn/northd/ovn-northd.8.xml   |  2 +-
>  ovn/northd/ovn-northd.c       | 70 +++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++
>  ovn/ovn-nb.ovsschema          | 13 ++++++++++++-
>  ovn/ovn-nb.xml                | 35 +++++++++++++++++++++++++++++++++++
>  ovn/utilities/ovn-nbctl.8.xml |  5 +++++
>  ovn/utilities/ovn-nbctl.c     |  5 +++++
>  tests/ovn.at                  | 10 ++++++++++
>  7 files changed, 138 insertions(+), 2 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index 743c939..7eb75df 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -680,7 +680,7 @@ next;
>          </p>
>
>          <p>
> -          If the route has a gateway, <var>G</var> is the gateway IP
> address,
> +          If the route has a nexthop, <var>G</var> is the nexthop address,
>            otherwise it is <code>ip4.dst</code>.
>          </p>
>        </li>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 0c869f4..bd25c49 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1780,6 +1780,68 @@ add_route(struct hmap *lflows, const struct ovn_port
> *op,
>  }
>
>  static void
> +build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od,
> struct hmap *ports,
> +        const struct nbrec_logical_router_static_route *route)
> +{
> +    struct ovn_port *out_port, *op;
> +    ovs_be32 prefix, nexthop, mask;
> +    int len;
> +    bool enabled;
> +
> +    /* verify nexthop */
> +    char *error = ip_parse_masked(route->nexthop, &nexthop, &mask);
> +    if (error || mask != OVS_BE32_MAX) {
> +        static struct vlog_rate_limit rl
> +                        = VLOG_RATE_LIMIT_INIT(5, 10);
> +        VLOG_WARN_RL(&rl, "bad 'nexthop' %s error %s", route->nexthop,
> error);
> +        free(error);
> +        return;
> +    }
> +
> +    /* verify prefix */
> +    error = ip_parse_masked(route->prefix, &prefix, &mask);
> +    if (error || !ip_is_cidr(mask)) {
> +        static struct vlog_rate_limit rl
> +                        = VLOG_RATE_LIMIT_INIT(5, 10);
> +        VLOG_WARN_RL(&rl, "bad 'prefix' %s error %s", route->prefix,
> error);
> +        free(error);
> +        return;
> +    }
> +
> +    /* find the outgoing port */
> +    out_port = NULL;
> +    len = 0;
> +    for (int i = 0; i < od->nbr->n_ports; i++){
> +
> +        struct nbrec_logical_router_port *lrp = od->nbr->ports[i];
> +        op = ovn_port_find(ports, lrp->name);
> +        if (!op){  /* this should not happen */
> +            continue;
> +        }
> +
> +        if (op->network && !((op->network ^ nexthop) & op->mask)){
> +            if (ip_count_cidr_bits(op->mask) > len){
> +                len = ip_count_cidr_bits(op->mask);
> +                out_port = op;
> +            }
> +        }
> +    }
> +
> +    if (!out_port){
> +        /*set route invalid flag*/
> +        enabled = false;
> +        nbrec_logical_router_static_route_set_valid(route, &enabled, 1);
> +        return;
> +    }
> +
> +    /* set valid flag */
> +    enabled = true;
> +    nbrec_logical_router_static_route_set_valid(route, &enabled, 1);
> +
> +    add_route(lflows, out_port, prefix, mask, nexthop);
> +}
> +
> +static void
>  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>                      struct hmap *lflows)
>  {
> @@ -1948,6 +2010,14 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>              continue;
>          }
>
> +        /* convert the routing table to flow */
> +        for (int i = 0; i < od->nbr->n_routes; i++){
> +            const struct nbrec_logical_router_static_route *route;
> +
> +            route = od->nbr->routes[i];
> +            build_static_route_flow(lflows, od, ports, route);
> +        }
> +
>          if (od->gateway && od->gateway_port) {
>              add_route(lflows, od->gateway_port, 0, 0, od->gateway);
>          }
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index 9fb8cd1..f1894b1 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>      "name": "OVN_Northbound",
>      "version": "2.0.1",
> -    "cksum": "660370796 4618",
> +    "cksum": "3768071525 5196",
>      "tables": {
>          "Logical_Switch": {
>              "columns": {
> @@ -72,6 +72,11 @@
>                                     "min": 0,
>                                     "max": "unlimited"}},
>                  "default_gw": {"type": {"key": "string", "min": 0, "max":
> 1}},
> +                "routes": {"type": {"key": {"type": "uuid",
> +                                           "refTable":
> "Logical_Router_Static_Route",
> +                                           "refType": "strong"},
> +                                   "min": 0,
> +                                   "max": "unlimited"}},
>                  "external_ids": {
>                      "type": {"key": "string", "value": "string",
>                               "min": 0, "max": "unlimited"}}},
> @@ -90,6 +95,12 @@
>                      "type": {"key": "string", "value": "string",
>                               "min": 0, "max": "unlimited"}}},
>              "indexes": [["name"]],
> +            "isRoot": false},
> +        "Logical_Router_Static_Route": {
> +            "columns": {
> +                "prefix": {"type": "string"},
> +                "nexthop": {"type": "string"},
> +                "valid": {"type": {"key": "boolean", "min": 0, "max":
> 1}}},
>              "isRoot": false}
>      }
>  }
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index e65bc3a..cf044d0 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -627,6 +627,10 @@
>        IP address to use as default gateway, if any.
>      </column>
>
> +    <column name="routes">
> +      Routes belonging to this router.
> +    </column>
> +
>      <group title="Common Columns">
>        <column name="external_ids">
>          See <em>External IDs</em> at the beginning of this document.
> @@ -717,4 +721,35 @@
>        </column>
>      </group>
>    </table>
> +
> +  <table name="Logical_Router_Static_Route" title="L3 logical router
> static routes">
> +    <p>
> +      Each route represents a static route.
> +    </p>
> +
> +    <p>
> +      Exactly one <ref table="Logical_Router"/> row must reference a given
> +      logical router static routes.
> +    </p>
> +
> +    <column name="prefix">
> +      <p>
> +        Prefix of this route, example 192.168.100.0/24
> +      </p>
> +    </column>
> +
> +    <column name="nexthop">
> +      <p>
> +        Nexthop of this route, nexthop can be a IP address of neutron
> port, or
> +        IP address which has been learnt by dynamic ARP
> +      </p>
> +    </column>
> +
> +    <column name="valid">
> +      <p>
> +        It will set to valid when its nexthop can be resolved.
> +      </p>
> +    </column>
> +  </table>
> +
>  </database>
> diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
> index 3b337dd..2a0bfd2 100644
> --- a/ovn/utilities/ovn-nbctl.8.xml
> +++ b/ovn/utilities/ovn-nbctl.8.xml
> @@ -231,6 +231,11 @@
>          A port within an L3 logical router.  Records may be identified by
> name.
>        </dd>
>
> +      <dt><code>Logical_Router_Static_Route</code></dt>
> +      <dd>
> +        A static route belonging to an L3 logical router.
> +      </dd>
> +
>      </dl>
>
>      <xi:include href="lib/db-ctl-base.xml" xmlns:xi="
> http://www.w3.org/2003/XInclude"/>
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index bdad368..bb3ce3e 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -1101,6 +1101,11 @@ static const struct ctl_table_class tables[] = {
>         NULL},
>        {NULL, NULL, NULL}}},
>
> +    {&nbrec_table_logical_router_static_route,
> +     {{&nbrec_table_logical_router_static_route, NULL,
> +       NULL},
> +      {NULL, NULL, NULL}}},
> +
>      {NULL, {{NULL, NULL, NULL}, {NULL, NULL, NULL}}}
>  };
>  ^L
> diff --git a/tests/ovn.at b/tests/ovn.at
> index f2ceba3..9b5c199 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1385,6 +1385,16 @@ as hv1 ovn-sbctl list datapath_binding
>  as hv1 ovn-sbctl dump-flows
>  as hv1 ovs-ofctl dump-flows br-int
>
> +### add a static route to router
> +ovn-nbctl -- --id=@lrt create Logical_Router_Static_Route
> prefix=172.16.1.0/24 nexthop=192.168.12.1 -- add Logical_Router
> test_router
> routes @lrt
> +
> +# Allow some time for ovn-northd and ovn-controller to catch up.
> +sleep 1
> +
> +as hv1 ovs-ofctl dump-flows br-int
> +as hv2 ovs-ofctl dump-flows br-int
> +as hv3 ovs-ofctl dump-flows br-int
> +
>  # Send IP packets between all pairs of source and destination ports:
>  #
>  # 1. Unicast IP packets are delivered to exactly one lport (except
>
>
> Best Regards
> Steve Ruan(阮诗新)
>
> Tel: (86-021)60928749,
> Address: 5/F, Building 10, 399 Ke Yuan Road, Zhangjiang High-Tech Park,
> Shanghai 201203, PRC
> 上海浦东新区张江高科园区科苑路399号10号楼5楼
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list