On 30 March 2016 at 19:36, Shi Xin Ruan <steve.r...@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 <rua...@cn.ibm.com>, > Reported-by: Na Zhu <na...@cn.ibm.com> > Reported-by: Dustin Lundquist <dlundqu...@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@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
