Re: [ovs-dev] [PATCH] Support multiple logical routing port configuration "redirect-chassis" on a distributed router

2017-02-27 Thread Guoshuai Li 

Hi Mickey, Thanks for review.

This is a quick preliminary review. I will review this in more detail 
tomorrow afternoon.


On Mon, Feb 27, 2017 at 5:12 AM, Guoshuai Li > wrote:


The main application scenario of this patch is that the user flow
wants to
different destination addresses through different external networks.
This scenario requires a distributed route to be associated with
multiple external network logical switches.

In a distributed router, the NAT logical flow table is generated
based on
the external IP lookup distributed router port, otherwise not
generated.


I see your problem that you need some way to figure out which router 
gateway port this NAT rule should be associated with, now that you 
have multiple distributed gateway ports on the same logical router.


However, there is currently no restriction that NAT external IP 
addresses need to match an existing subnet on a router port. I am 
uncomfortable with the addition of such a restriction in this patch, 
since it will not support scenarios that are valid in OVN and in 
OpenStack today.
I think the general usage is NAT external IP addresses in the 
existing subnet segment.
Do you accept the NAT*l**o**g**i**c**a**l**_**p**o**r**t *field to 
specify the gateway port ？*e**x**t**e**r**n**a**l**_**m**a**c *field may 
used to distinguish between centralized and distributed.


When the destination address of the packet is an external IP of
the NAT rule,
and the ingress port is not a gateway,
it is necessary to route the actual outgoing port.


Are you suggesting that static routes need to be programmed to NAT 
external addresses?
This is Base on NAT external addresses in the router port subnet, 
not need static routes, because matching subnet segment routing.

If break this limit, your idea is good.
That would significantly complicate what the user needs to do in order 
to make NAT on a distributed router work.

Doesn't this break existing tests?
My suggestion would be to set outport when you set 
REGBIT_NAT_REDIRECT, in the DNAT and UNSNAT stages.


I would also like to hear back from Russell or others about the 
"routed external networks" discussion at the OpenStack PTG in Atlanta.

Does a router still have only one external network?
Does a router still have only one external IPv4 address?
Is it limited to one segment?
If there can be multiple external IPv4 addresses, then there might be 
some overlap with this proposal.


Mickey


Signed-off-by: Guoshuai Li >
Co-authored-by: Dong Jun >
---
 ovn/northd/ovn-northd.8.xml |  22 ++---
 ovn/northd/ovn-northd.c | 232
+++-
 ovn/ovn-nb.xml  |  12 ++-
 3 files changed, 137 insertions(+), 129 deletions(-)



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Support multiple logical routing port configuration "redirect-chassis" on a distributed router

2017-02-27 Thread Mickey Spiegel 
This is a quick preliminary review. I will review this in more detail
tomorrow afternoon.

On Mon, Feb 27, 2017 at 5:12 AM, Guoshuai Li  wrote:

> The main application scenario of this patch is that the user flow wants to
> different destination addresses through different external networks.
> This scenario requires a distributed route to be associated with
> multiple external network logical switches.
>
> In a distributed router, the NAT logical flow table is generated based on
> the external IP lookup distributed router port, otherwise not generated.
>

I see your problem that you need some way to figure out which router
gateway port this NAT rule should be associated with, now that you have
multiple distributed gateway ports on the same logical router.

However, there is currently no restriction that NAT external IP addresses
need to match an existing subnet on a router port. I am uncomfortable with
the addition of such a restriction in this patch, since it will not support
scenarios that are valid in OVN and in OpenStack today.


> When the destination address of the packet is an external IP of the NAT
> rule,
> and the ingress port is not a gateway,
> it is necessary to route the actual outgoing port.
>

Are you suggesting that static routes need to be programmed to NAT external
addresses?
That would significantly complicate what the user needs to do in order to
make NAT on a distributed router work.
Doesn't this break existing tests?
My suggestion would be to set outport when you set REGBIT_NAT_REDIRECT, in
the DNAT and UNSNAT stages.

I would also like to hear back from Russell or others about the "routed
external networks" discussion at the OpenStack PTG in Atlanta.
Does a router still have only one external network?
Does a router still have only one external IPv4 address?
Is it limited to one segment?
If there can be multiple external IPv4 addresses, then there might be some
overlap with this proposal.

Mickey


>
> Signed-off-by: Guoshuai Li 
> Co-authored-by: Dong Jun 
> ---
>  ovn/northd/ovn-northd.8.xml |  22 ++---
>  ovn/northd/ovn-northd.c | 232 +++---
> --
>  ovn/ovn-nb.xml  |  12 ++-
>  3 files changed, 137 insertions(+), 129 deletions(-)
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Support multiple logical routing port configuration "redirect-chassis" on a distributed router

2017-02-27 Thread Guoshuai Li 
The main application scenario of this patch is that the user flow wants to
different destination addresses through different external networks.
This scenario requires a distributed route to be associated with
multiple external network logical switches.

In a distributed router, the NAT logical flow table is generated based on
the external IP lookup distributed router port, otherwise not generated.

When the destination address of the packet is an external IP of the NAT rule,
and the ingress port is not a gateway,
it is necessary to route the actual outgoing port.

Signed-off-by: Guoshuai Li 
Co-authored-by: Dong Jun 
---
 ovn/northd/ovn-northd.8.xml |  22 ++---
 ovn/northd/ovn-northd.c | 232 +++-
 ovn/ovn-nb.xml  |  12 ++-
 3 files changed, 137 insertions(+), 129 deletions(-)

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index ab8fd88..d9056ef 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -1549,16 +1549,6 @@ icmp4 {
 
   
 
-  For distributed logical routers where one of the logical router
-  ports specifies a redirect-chassis, a priority-300
-  logical flow with match REGBIT_NAT_REDIRECT == 1 has
-  actions ip.ttl--; next;.  The outport
-  will be set later in the Gateway Redirect table.
-
-  
-
-  
-
   IPv4 routing table.  For each route to IPv4 network N with
   netmask M, on router port P with IP address
   A and Ethernet
@@ -1644,12 +1634,12 @@ next;
 
   
 
-  For distributed logical routers where one of the logical router
-  ports specifies a redirect-chassis, a priority-200
-  logical flow with match REGBIT_NAT_REDIRECT == 1 has
-  actions eth.dst = E; next;, where
-  E is the ethernet address of the router's distributed
-  gateway port.
+  For distributed logical routers where router port P
+  specifies a redirect-chassis, a priority-200
+  logical flow with match REGBIT_NAT_REDIRECT == 1
+  and outport == P has actions
+  eth.dst = E; next;, where E
+  is the ethernet address of the router's distributed gateway port.
 
   
 
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 03dc850..8477e23 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -398,12 +398,10 @@ struct ovn_datapath {
 
 /* OVN northd only needs to know about the logical router gateway port for
  * NAT on a distributed router.  This "distributed gateway port" is
- * populated only when there is a "redirect-chassis" specified for one of
- * the ports on the logical router.  Otherwise this will be NULL. */
-struct ovn_port *l3dgw_port;
-/* The "derived" OVN port representing the instance of l3dgw_port on
- * the "redirect-chassis". */
-struct ovn_port *l3redirect_port;
+ * populated only when there is a "redirect-chassis" specified for the 
+ * ports on the logical router.  Otherwise this will be NULL. */
+struct ovn_port **l3dgw_ports;
+size_t n_l3dgw_ports;
 };
 
 struct macam_node {
@@ -683,6 +681,10 @@ struct ovn_port {
 bool derived; /* Indicates whether this is an additional port
* derived from nbsp or nbrp. */
 
+/* The "derived" OVN port representing the instance of l3dgw_port on
+ * the "redirect-chassis". Otherwise this will be NULL. */
+struct ovn_port *l3redirect_port;
+
 /* The port's peer:
  *
  * - A switch port S of type "router" has a router port R as a peer,
@@ -1402,16 +1404,11 @@ join_logical_ports(struct northd_context *ctx,
 
 /* Set l3dgw_port and l3redirect_port in od, for later
  * use during flow creation. */
-if (od->l3dgw_port || od->l3redirect_port) {
-static struct vlog_rate_limit rl
-= VLOG_RATE_LIMIT_INIT(1, 1);
-VLOG_WARN_RL(, "Bad configuration: multiple ports "
- "with redirect-chassis on same logical "
- "router %s", od->nbr->name);
-continue;
-} else {
-od->l3dgw_port = op;
-od->l3redirect_port = crp;
+if (!op->l3redirect_port) {
+op->l3redirect_port = crp;
+od->l3dgw_ports = xrealloc(od->l3dgw_ports,
+sizeof *od->l3dgw_ports * (od->n_l3dgw_ports + 1));
+od->l3dgw_ports[od->n_l3dgw_ports++] = op;
 }
 }
 }
@@ -3300,14 +3297,12 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 ds_clear();