[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