Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR
Hi Numan, Thanks for going through the commit plan. I submitted a V10 now, which has just the E-W changes. It has all the review comments handled. You wanted to have periodic GARP advertisement patch together as well, however as per our discussion on another email thread, since we want your garp advertisement patch to go in first, hence I could not accommodate those changes in this patch. Please take a look and looking forward toy our feedback. Thanks Regards, Ankur From: Numan Siddique Sent: Monday, June 10, 2019 10:18 AM To: Ankur Sharma Cc: ovs-dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR On Fri, Jun 7, 2019 at 5:15 AM Ankur Sharma mailto:ankur.sha...@nutanix.com>> wrote: Hi Numan, Thanks for trying out the patch and providing feedback. I am planning to change this series to reflect only the E-W and would send out separate patches for N-S improvements. Following is the reasoning: == a. I agree that the network_type construct is adding to the confusion and may be we should rely on optional/external-id based key-value config. b. N-S patch has 3- changes (on a high level) which are distinct from each other, having them in a single patch is causing the confusion and is holding rest of the reviewed changes. c. Last but not the least, there were some gaps in the patch as well. Here is what I am planning to do: == a. Keep this series for E-W only and remove all the network_type related changes from here (including showing type as bridged/vlan). Hi Ankur. I agree with the approach you are planning to take. b. For N-S Changes, this series has following changes. I will send them out in separate patches, especially the ones which are more of a bug fix. i. Do not allow ARP resolution from physical network unless gateway chassis is configured ==> More of a bug fix, will be sent as a separate standalone patch. ii. GARP advertisement during failover in the absence of NAT configuration ==> More of a bug fix, will be sent as a separate standalone patch. iii. Periodic GARP advertisement with/without NAT configuration ==> New feature, will be added along with SNAT changes. This periodic GARP adv will be required irrespective of network_type of the logical switches connected to a router. So I would request you to handle this as well when you submit the patch. iv. Avoid redirection ==> New feature, will come as a separate patchset, we will make it as optional feature, i.e by default even non NATed traffic will go via gateway chassis, but config knob can override it. Agree. This makes sense and easier. v. No chassis mac replace on gateway chassis ==> More of a addendum to E-W, I am thinking about clubbing it some of N-S changes as this is where it will be relevant. c. Will send out separate patch for showing network type as overlay or bridged (based on localnet port’s presence), I believe it is good to have . i.e we will not have any new column in logical switch table, but the output of relevant ovn-nbctl show command will show type as “overlay” or “bridged”. Above will allow us to make progress on the changes we are in agreement on, while having thorough discussion on the remaining. Let me know, if you are fine with the plan, I should be able to send E-W only changes in a couple of days and should be able to individual bug fixes soon after as well. For rest of the comments, please find my replies inline. Appreciate your feedback. Regards, Ankur From: Numan Siddique mailto:nusid...@redhat.com>> Sent: Monday, June 3, 2019 3:06 AM To: Ankur Sharma mailto:ankur.sha...@nutanix.com>> Cc: ovs-dev@openvswitch.org<mailto:ovs-dev@openvswitch.org> Subject: Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR Hi Ankur, Please see some comments inline. Please note that I haven't got the chance to look into the code in detail. I am first trying to test out the patches. (I am in PTO. Expect some delay in my replies). On Thu, May 30, 2019 at 5:58 AM Ankur Sharma mailto:ankur.sha...@nutanix.com>> wrote: Background: [1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html [mail.openvswitch.org]<https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2018-2DOctober_353066.html=DwMFaQ=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=PIGJNMisAQ9iokicyVS4lKZ7fLKTOjQYSIV6R83EdO8=xQRPm8R90ygR4nx7uyRGOYHzW5NFiroiyZqi9JSYb-A=> [2] https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing [docs.google.com]<https://urldefense.proofpoint.com/v2/url?u=https-3A__docs.google.com_document_d_1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU_edit-3Fusp-3Dsharing=DwMFaQ=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=PIGJNMisAQ9iokicyVS4lKZ7fLKTOjQYSIV6R83EdO8=myrKaOI2LsuZQ
Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR
On Fri, Jun 7, 2019 at 5:15 AM Ankur Sharma wrote: > Hi Numan, > > Thanks for trying out the patch and providing feedback. > I am planning to change this series to reflect only the E-W and would send > out separate patches for N-S improvements. > > Following is the reasoning: > > == > a. I agree that the network_type construct is adding to the confusion and > may be we should rely on optional/external-id based key-value config. > b. N-S patch has 3- changes (on a high level) which are distinct from each > other, having them in a single patch is causing the confusion and is > holding rest of the reviewed changes. > c. Last but not the least, there were some gaps in the patch as well. > > Here is what I am planning to do: > > == > a. Keep this series for E-W only and remove all the network_type related > changes from here (including showing type as bridged/vlan). > Hi Ankur. I agree with the approach you are planning to take. > > b. For N-S Changes, this series has following changes. I will send them > out in separate patches, especially the ones which are more of a bug fix. > i. Do not allow ARP resolution from physical network unless gateway > chassis is configured è More of a bug fix, will be sent as a separate > standalone patch. > >ii. GARP advertisement during failover in the absence of NAT > configuration è More of a bug fix, will be sent as a separate standalone > patch. > > iii. Periodic GARP advertisement with/without NAT configuration è New > feature, will be added along with SNAT changes. > This periodic GARP adv will be required irrespective of network_type of the logical switches connected to a router. So I would request you to handle this as well when you submit the patch. > iv. Avoid redirection è New feature, will come as a separate patchset, > we will make it as optional feature, i.e by default even non NATed traffic > will go via gateway chassis, but config knob can override it. > Agree. This makes sense and easier. >v. No chassis mac replace on gateway chassis è More of a addendum to > E-W, I am thinking about clubbing it some of N-S changes as this is where > it will be relevant. > > > > c. Will send out separate patch for showing network type as overlay or > bridged (based on localnet port’s presence), I believe it is good to have > . > i.e we will not have any new column in logical switch table, but the > output of relevant ovn-nbctl show command will show type as “overlay” or > “bridged”. > > > Above will allow us to make progress on the changes we are in agreement > on, while having thorough discussion on the remaining. > > Let me know, if you are fine with the plan, I should be able to send E-W > only changes in a couple of days and should be able to individual bug fixes > soon after as well. > > > For rest of the comments, please find my replies inline. > > Appreciate your feedback. > > Regards, > Ankur > > > > > *From:* Numan Siddique > *Sent:* Monday, June 3, 2019 3:06 AM > *To:* Ankur Sharma > *Cc:* ovs-dev@openvswitch.org > *Subject:* Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan > backed DVR > > > > > > Hi Ankur, > > > > Please see some comments inline. Please note that I haven't got the chance > to look into the code > > in detail. I am first trying to test out the patches. (I am in PTO. Expect > some delay in my replies). > > > > > > > > On Thu, May 30, 2019 at 5:58 AM Ankur Sharma > wrote: > > Background: > [1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html > [mail.openvswitch.org] > <https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2018-2DOctober_353066.html=DwMFaQ=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=PIGJNMisAQ9iokicyVS4lKZ7fLKTOjQYSIV6R83EdO8=xQRPm8R90ygR4nx7uyRGOYHzW5NFiroiyZqi9JSYb-A=> > [2] > https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing > [docs.google.com] > <https://urldefense.proofpoint.com/v2/url?u=https-3A__docs.google.com_document_d_1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU_edit-3Fusp-3Dsharing=DwMFaQ=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=PIGJNMisAQ9iokicyVS4lKZ7fLKTOjQYSIV6R83EdO8=myrKaOI2LsuZQQOZhhhNw1zwDgat77e5CmPmpTFpllw=> > > This Series: > Layer 2, Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan > backed distributed logical router. > > This patch: > For North-South traffic, we need a chassis which will respond to > ARP requests for router port coming from outside. For this purpose, > we will reply upon gateway
Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR
Hi Numan, Thanks for trying out the patch and providing feedback. I am planning to change this series to reflect only the E-W and would send out separate patches for N-S improvements. Following is the reasoning: == a. I agree that the network_type construct is adding to the confusion and may be we should rely on optional/external-id based key-value config. b. N-S patch has 3- changes (on a high level) which are distinct from each other, having them in a single patch is causing the confusion and is holding rest of the reviewed changes. c. Last but not the least, there were some gaps in the patch as well. Here is what I am planning to do: == a. Keep this series for E-W only and remove all the network_type related changes from here (including showing type as bridged/vlan). b. For N-S Changes, this series has following changes. I will send them out in separate patches, especially the ones which are more of a bug fix. i. Do not allow ARP resolution from physical network unless gateway chassis is configured ==> More of a bug fix, will be sent as a separate standalone patch. ii. GARP advertisement during failover in the absence of NAT configuration ==> More of a bug fix, will be sent as a separate standalone patch. iii. Periodic GARP advertisement with/without NAT configuration ==> New feature, will be added along with SNAT changes. iv. Avoid redirection ==> New feature, will come as a separate patchset, we will make it as optional feature, i.e by default even non NATed traffic will go via gateway chassis, but config knob can override it. v. No chassis mac replace on gateway chassis ==> More of a addendum to E-W, I am thinking about clubbing it some of N-S changes as this is where it will be relevant. c. Will send out separate patch for showing network type as overlay or bridged (based on localnet port’s presence), I believe it is good to have . i.e we will not have any new column in logical switch table, but the output of relevant ovn-nbctl show command will show type as “overlay” or “bridged”. Above will allow us to make progress on the changes we are in agreement on, while having thorough discussion on the remaining. Let me know, if you are fine with the plan, I should be able to send E-W only changes in a couple of days and should be able to individual bug fixes soon after as well. For rest of the comments, please find my replies inline. Appreciate your feedback. Regards, Ankur From: Numan Siddique Sent: Monday, June 3, 2019 3:06 AM To: Ankur Sharma Cc: ovs-dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR Hi Ankur, Please see some comments inline. Please note that I haven't got the chance to look into the code in detail. I am first trying to test out the patches. (I am in PTO. Expect some delay in my replies). On Thu, May 30, 2019 at 5:58 AM Ankur Sharma mailto:ankur.sha...@nutanix.com>> wrote: Background: [1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html [mail.openvswitch.org]<https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2018-2DOctober_353066.html=DwMFaQ=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=PIGJNMisAQ9iokicyVS4lKZ7fLKTOjQYSIV6R83EdO8=xQRPm8R90ygR4nx7uyRGOYHzW5NFiroiyZqi9JSYb-A=> [2] https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing [docs.google.com]<https://urldefense.proofpoint.com/v2/url?u=https-3A__docs.google.com_document_d_1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU_edit-3Fusp-3Dsharing=DwMFaQ=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=PIGJNMisAQ9iokicyVS4lKZ7fLKTOjQYSIV6R83EdO8=myrKaOI2LsuZQQOZhhhNw1zwDgat77e5CmPmpTFpllw=> This Series: Layer 2, Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan backed distributed logical router. This patch: For North-South traffic, we need a chassis which will respond to ARP requests for router port coming from outside. For this purpose, we will reply upon gateway-chassis construct in OVN, on a logical router port, we will associate one or more chassis as gateway chassis. One of these chassis would be active at a point and will become entry point to traffic, bound for end points behind logical router coming from outside network (North to South). This patch make some enhancements to gateway chassis implementation to manage above used case. A. Do not replace router port mac with chassis mac on gateway chassis. This is done, because: i. Chassisredirect port is NOT a distributed port, hence we need not replace its mac address (which same as router port mac). ii. ARP cache will be consistent everywhere, i.e just like endpoints on OVN chassis will see configured router port mac as resolved mac for router port ip, outside endpoints will see that as well. iii.
Re: [ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR
Hi Ankur,
Please see some comments inline. Please note that I haven't got the chance
to look into the code
in detail. I am first trying to test out the patches. (I am in PTO. Expect
some delay in my replies).
On Thu, May 30, 2019 at 5:58 AM Ankur Sharma
wrote:
> Background:
> [1]
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
> [2]
> https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing
>
> This Series:
> Layer 2, Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan
> backed distributed logical router.
>
> This patch:
> For North-South traffic, we need a chassis which will respond to
> ARP requests for router port coming from outside. For this purpose,
> we will reply upon gateway-chassis construct in OVN, on a logical
> router port, we will associate one or more chassis as gateway chassis.
>
> One of these chassis would be active at a point and will become
> entry point to traffic, bound for end points behind logical router
> coming from outside network (North to South).
>
> This patch make some enhancements to gateway chassis implementation
> to manage above used case.
>
> A.
> Do not replace router port mac with chassis mac on gateway
> chassis.
> This is done, because:
> i. Chassisredirect port is NOT a distributed port, hence
>we need not replace its mac address
> (which same as router port mac).
>
>ii. ARP cache will be consistent everywhere, i.e just like
>endpoints on OVN chassis will see configured router port
>mac as resolved mac for router port ip, outside endpoints
>will see that as well.
>
> iii. For implementing Network Address Translation. Although
>not a part of this series. But, follow up series would
>be having this feature and approach would rely upon
>sending packets to redirect chassis using chassis redirect
>router port mac as dest mac.
>
> B.
> Advertise router port GARP on gateway chassis.
> This is needed, especially if a failover happens and
> chassisredirect port moves to a new gateway chassis.
> Otherwise, there would be packet drops till outside
> router ARPs for router port ip again.
>
> Intention of this GARP is to update top of the rack (TOR)
> to direct router port mac to new hypervisor.
>
> Hence, we could have done the same using RARP as well, but
> because ovn-controller has implementation for GARP already,
> hence it did not look like worthy to add a RARP implementation
> just for this.
>
> C.
> For South to North traffic, we need not pass through gateway
> chassis, if there is no address transalation needed.
>
> For overlay networks, NATing is a must to talk to outside networks.
> However, for vlan backed networks, NATing is not a must, and hence
> in the absence of NATing configuration we need redirect the packet
> to gateway chassis.
>
> Signed-off-by: Ankur Sharma
> ---
> ovn/controller/physical.c | 24 +-
> ovn/controller/pinctrl.c | 205 +++--
> ovn/controller/pinctrl.h | 6 +
> ovn/lib/ovn-util.c | 31 ++
> ovn/lib/ovn-util.h | 6 +
> ovn/northd/ovn-northd.c| 43 ++-
> ovn/ovn-architecture.7.xml | 87 +-
> tests/ovn.at | 732
> -
> 8 files changed, 1090 insertions(+), 44 deletions(-)
>
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index af587a5..1ab5968 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -21,6 +21,7 @@
> #include "lflow.h"
> #include "lport.h"
> #include "chassis.h"
> +#include "pinctrl.h"
> #include "lib/bundle.h"
> #include "openvswitch/poll-loop.h"
> #include "lib/uuid.h"
> @@ -238,9 +239,12 @@ get_zone_ids(const struct sbrec_port_binding *binding,
> }
>
> static void
> -put_replace_router_port_mac_flows(const struct
> +put_replace_router_port_mac_flows(struct ovsdb_idl_index
> + *sbrec_port_binding_by_name,
> + const struct
>sbrec_port_binding *localnet_port,
>const struct sbrec_chassis *chassis,
> + const struct sset *active_tunnels,
>const struct hmap *local_datapaths,
>struct ofpbuf *ofpacts_p,
>ofp_port_t ofport,
> @@ -281,8 +285,21 @@ put_replace_router_port_mac_flows(const struct
> char *err_str = NULL;
> struct match match;
> struct ofpact_mac *replace_mac;
> +char *cr_peer_name = xasprintf("cr-%s",
> rport_binding->logical_port);
>
> -/* Table 65, priority 150.
> +
> +if (pinctrl_is_chassis_resident(sbrec_port_binding_by_name,
> +chassis, active_tunnels,
> +cr_peer_name)) {
> +/* If a router port's
[ovs-dev] [PATCH v9 2/2] OVN: Enable N-S Traffic, Vlan backed DVR
Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2]
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing
This Series:
Layer 2, Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan
backed distributed logical router.
This patch:
For North-South traffic, we need a chassis which will respond to
ARP requests for router port coming from outside. For this purpose,
we will reply upon gateway-chassis construct in OVN, on a logical
router port, we will associate one or more chassis as gateway chassis.
One of these chassis would be active at a point and will become
entry point to traffic, bound for end points behind logical router
coming from outside network (North to South).
This patch make some enhancements to gateway chassis implementation
to manage above used case.
A.
Do not replace router port mac with chassis mac on gateway
chassis.
This is done, because:
i. Chassisredirect port is NOT a distributed port, hence
we need not replace its mac address
(which same as router port mac).
ii. ARP cache will be consistent everywhere, i.e just like
endpoints on OVN chassis will see configured router port
mac as resolved mac for router port ip, outside endpoints
will see that as well.
iii. For implementing Network Address Translation. Although
not a part of this series. But, follow up series would
be having this feature and approach would rely upon
sending packets to redirect chassis using chassis redirect
router port mac as dest mac.
B.
Advertise router port GARP on gateway chassis.
This is needed, especially if a failover happens and
chassisredirect port moves to a new gateway chassis.
Otherwise, there would be packet drops till outside
router ARPs for router port ip again.
Intention of this GARP is to update top of the rack (TOR)
to direct router port mac to new hypervisor.
Hence, we could have done the same using RARP as well, but
because ovn-controller has implementation for GARP already,
hence it did not look like worthy to add a RARP implementation
just for this.
C.
For South to North traffic, we need not pass through gateway
chassis, if there is no address transalation needed.
For overlay networks, NATing is a must to talk to outside networks.
However, for vlan backed networks, NATing is not a must, and hence
in the absence of NATing configuration we need redirect the packet
to gateway chassis.
Signed-off-by: Ankur Sharma
---
ovn/controller/physical.c | 24 +-
ovn/controller/pinctrl.c | 205 +++--
ovn/controller/pinctrl.h | 6 +
ovn/lib/ovn-util.c | 31 ++
ovn/lib/ovn-util.h | 6 +
ovn/northd/ovn-northd.c| 43 ++-
ovn/ovn-architecture.7.xml | 87 +-
tests/ovn.at | 732 -
8 files changed, 1090 insertions(+), 44 deletions(-)
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index af587a5..1ab5968 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -21,6 +21,7 @@
#include "lflow.h"
#include "lport.h"
#include "chassis.h"
+#include "pinctrl.h"
#include "lib/bundle.h"
#include "openvswitch/poll-loop.h"
#include "lib/uuid.h"
@@ -238,9 +239,12 @@ get_zone_ids(const struct sbrec_port_binding *binding,
}
static void
-put_replace_router_port_mac_flows(const struct
+put_replace_router_port_mac_flows(struct ovsdb_idl_index
+ *sbrec_port_binding_by_name,
+ const struct
sbrec_port_binding *localnet_port,
const struct sbrec_chassis *chassis,
+ const struct sset *active_tunnels,
const struct hmap *local_datapaths,
struct ofpbuf *ofpacts_p,
ofp_port_t ofport,
@@ -281,8 +285,21 @@ put_replace_router_port_mac_flows(const struct
char *err_str = NULL;
struct match match;
struct ofpact_mac *replace_mac;
+char *cr_peer_name = xasprintf("cr-%s", rport_binding->logical_port);
-/* Table 65, priority 150.
+
+if (pinctrl_is_chassis_resident(sbrec_port_binding_by_name,
+chassis, active_tunnels,
+cr_peer_name)) {
+/* If a router port's chassisredirect port is
+ * resident on this chassis, then we need not do mac replace. */
+free(cr_peer_name);
+continue;
+}
+
+free(cr_peer_name);
+
+ /* Table 65, priority 150.
* ===
*
* Implements output to localnet port.
@@ -797,7 +814,8 @@ consider_port_binding(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
, ofpacts_p, >header_.uuid);
