Re: [ovs-dev] [PATCH] ovn-northd: Fix peering of routers.
On 30 March 2016 at 16:18, Ben Pfaff wrote:
> On Mon, Mar 28, 2016 at 02:31:41PM -0700, Gurucharan Shetty wrote:
> > 1. Currently, the ovn-nb man page says that the 'peer'
> > in a logical_router_port table should point to the name
> > of the peer's logical router port. But the schema had declared
> > this column as a uuid. This looks not to be the intention as peers
> > for logical switches connected to routers is a name (and not a uuid).
> > So this patch changes the schema to be name.
> >
> > 2. In the southbound database, in the port_binding table, for a
> > logical_router_port, the peer was pointing back to itself. This
> > was causing ovn-controller to create patch ports where the peer
> > was wrongly pointing back to the source itself. This clearly looks
> > to be an error. So this patch fixes the peer in southbound database
> > to correclty point to the real peer.
> >
> > 3. ovn-northd.c currently skips generating logical flows to transfer
> > packets between two peers with comment about needing 'ARP for
> > neighboring routers'. It looked to me that since the router peer
> > is a logical object that has to be created in OVN-NB database, we
> > always need to statically assign the mac address. So this patch
> > picks the mac address from the database.
> >
> > Signed-off-by: Gurucharan Shetty
>
> Thank you. The lack of this feature was due to my laziness.
>
> I suspect that you should update ovn-northd.8.xml also.
>
True. I will add the following incremental
diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 7954e22..a89b88e 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -746,6 +746,15 @@ icmp4 {
A has actions eth.dst = E;
next;.
+
+
+ For each logical router port with an IP address A and
+ a mac address of E that is reachable via a different
+ logical router port P, a priority-100 flow with
+ match outport === P && reg0 ==
+ A has actions eth.dst = E;
+ next;.
+
>
> Acked-by: Ben Pfaff
>
Thanks, I will apply this in a while.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn-northd: Fix peering of routers.
On Mon, Mar 28, 2016 at 02:31:41PM -0700, Gurucharan Shetty wrote: > 1. Currently, the ovn-nb man page says that the 'peer' > in a logical_router_port table should point to the name > of the peer's logical router port. But the schema had declared > this column as a uuid. This looks not to be the intention as peers > for logical switches connected to routers is a name (and not a uuid). > So this patch changes the schema to be name. > > 2. In the southbound database, in the port_binding table, for a > logical_router_port, the peer was pointing back to itself. This > was causing ovn-controller to create patch ports where the peer > was wrongly pointing back to the source itself. This clearly looks > to be an error. So this patch fixes the peer in southbound database > to correclty point to the real peer. > > 3. ovn-northd.c currently skips generating logical flows to transfer > packets between two peers with comment about needing 'ARP for > neighboring routers'. It looked to me that since the router peer > is a logical object that has to be created in OVN-NB database, we > always need to statically assign the mac address. So this patch > picks the mac address from the database. > > Signed-off-by: Gurucharan Shetty Thank you. The lack of this feature was due to my laziness. I suspect that you should update ovn-northd.8.xml also. Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn-northd: Fix peering of routers.
Acked-by: Ryan Moats "dev" wrote on 03/28/2016 04:31:41 PM: > From: Gurucharan Shetty > To: dev@openvswitch.org > Cc: Gurucharan Shetty > Date: 03/28/2016 04:48 PM > Subject: [ovs-dev] [PATCH] ovn-northd: Fix peering of routers. > Sent by: "dev" > > 1. Currently, the ovn-nb man page says that the 'peer' > in a logical_router_port table should point to the name > of the peer's logical router port. But the schema had declared > this column as a uuid. This looks not to be the intention as peers > for logical switches connected to routers is a name (and not a uuid). > So this patch changes the schema to be name. > > 2. In the southbound database, in the port_binding table, for a > logical_router_port, the peer was pointing back to itself. This > was causing ovn-controller to create patch ports where the peer > was wrongly pointing back to the source itself. This clearly looks > to be an error. So this patch fixes the peer in southbound database > to correclty point to the real peer. > > 3. ovn-northd.c currently skips generating logical flows to transfer > packets between two peers with comment about needing 'ARP for > neighboring routers'. It looked to me that since the router peer > is a logical object that has to be created in OVN-NB database, we > always need to statically assign the mac address. So this patch > picks the mac address from the database. > > Signed-off-by: Gurucharan Shetty > --- > ovn/northd/ovn-northd.c | 28 - > ovn/ovn-nb.ovsschema|9 +-- > tests/ovn.at| 155 > +++ > 3 files changed, 184 insertions(+), 8 deletions(-) ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovn-northd: Fix peering of routers.
1. Currently, the ovn-nb man page says that the 'peer'
in a logical_router_port table should point to the name
of the peer's logical router port. But the schema had declared
this column as a uuid. This looks not to be the intention as peers
for logical switches connected to routers is a name (and not a uuid).
So this patch changes the schema to be name.
2. In the southbound database, in the port_binding table, for a
logical_router_port, the peer was pointing back to itself. This
was causing ovn-controller to create patch ports where the peer
was wrongly pointing back to the source itself. This clearly looks
to be an error. So this patch fixes the peer in southbound database
to correclty point to the real peer.
3. ovn-northd.c currently skips generating logical flows to transfer
packets between two peers with comment about needing 'ARP for
neighboring routers'. It looked to me that since the router peer
is a logical object that has to be created in OVN-NB database, we
always need to statically assign the mac address. So this patch
picks the mac address from the database.
Signed-off-by: Gurucharan Shetty
---
ovn/northd/ovn-northd.c | 28 -
ovn/ovn-nb.ovsschema|9 +--
tests/ovn.at| 155 +++
3 files changed, 184 insertions(+), 8 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 6db5ed6..50ac509 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -669,7 +669,7 @@ join_logical_ports(struct northd_context *ctx,
sizeof *op->od->router_ports * (op->od->n_router_ports + 1));
op->od->router_ports[op->od->n_router_ports++] = op;
} else if (op->nbr && op->nbr->peer) {
-op->peer = ovn_port_find(ports, op->nbr->name);
+op->peer = ovn_port_find(ports, op->nbr->peer);
}
}
}
@@ -1962,7 +1962,31 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap
*ports,
* Ethernet address in eth.dst. */
HMAP_FOR_EACH (op, key_node, ports) {
if (op->nbr) {
-/* XXX ARP for neighboring router */
+/* This is a logical router port. If next-hop IP address in 'reg0'
+ * matches ip address of this router port, then the packet is
+ * intended to eventually be sent to this logical port. Set the
+ * destination mac address using this port's mac address.
+ *
+ * The packet is still in peer's logical pipeline. So the match
+ * should be on peer's outport. */
+if (op->nbr->peer) {
+struct ovn_port *peer = ovn_port_find(ports, op->nbr->peer);
+if (!peer) {
+continue;
+}
+
+if (!peer->ip || !op->ip) {
+continue;
+}
+char *match = xasprintf("outport == %s && reg0 == "IP_FMT,
+peer->json_key, IP_ARGS(op->ip));
+char *actions = xasprintf("eth.dst = "ETH_ADDR_FMT"; "
+ "next;", ETH_ADDR_ARGS(op->mac));
+ovn_lflow_add(lflows, peer->od, S_ROUTER_IN_ARP_RESOLVE,
+ 100, match, actions);
+free(actions);
+free(match);
+}
} else if (op->od->n_router_ports) {
for (size_t i = 0; i < op->nbs->n_addresses; i++) {
struct lport_addresses laddrs;
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 9fb8cd1..40a7a97 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",
+"version": "2.0.2",
+"cksum": "4289495412 4436",
"tables": {
"Logical_Switch": {
"columns": {
@@ -81,10 +81,7 @@
"name": {"type": "string"},
"network": {"type": "string"},
"mac": {"type": "string"},
-"peer": {"type": {"key": {"type": "uuid",
- "refTable": "Logical_Router_Port",
- "refType": "strong"},
- "min": 0, "max": 1}},
+"peer": {"type": {"key": "string", "min": 0, "max": 1}},
"enabled": {"type": {"key": "boolean", "min": 0, "max": 1}},
"external_ids": {
"type": {"key": "string", "value": "string",
diff --git a/tests/ovn.at b/tests/ovn.at
index f2ceba3..22121e1 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1986,3 +1986,158 @@ OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
OVS_APP_EXIT_AND_WAIT([ovsdb-server])
AT_CLEANUP
+
+AT_SETUP([ovn -- 2 HVs, 2 LS, 1 lport/LS, 2 peer LRs])
+AT_KEYWORDS([ovnpeer])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+
+# Logical network:
+# Two LRs - R1 and R2 that are connecte
