Re: [ovs-dev] [PATCH v7 1/2] OVN: Enable E-W Traffic, Vlan backed DVR
Hi Numan, Glad to see your comments. I have handled all the comments in the code, v8 will have the same. Regarding your comments on backward compatibility: “ Hi Ankur, For most part the patch looks fine to me. Please see some additional comments. I still have few questions about the type added to the logical switch. How would you handle the case for upgrades ? i.e if the deployment already have a logical switch with a localnet port. What if the user sets the type as - bridged, but doesn't create the localnet port ? “ Changes are backward compatible, i.e upgrades would not be an issue. a. Flow for replacing router port mac with chassis mac happens based on localnet port configuration. b. If chassis_mac is not configured, then router port mac will be used as is. If localnet port is not created, then connectivity to vlan backed physical network cannot be established, like for a regular logical switch. Adding a validation for this looked tricky, because a single transaction will not have both network_types and localnet port creation. I added a line in documentation to indicate that bridged types MUST have a localnet port configured. I will be happy to make more changes in this regard, let me know if i can take it as a follow up activity, we can have a more thorough discussion on it. Thanks Regards, Ankur From: Numan Siddique Sent: Friday, May 10, 2019 5:39 AM To: Ankur Sharma Cc: ovs-dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v7 1/2] OVN: Enable E-W Traffic, Vlan backed DVR On Thu, May 9, 2019 at 4:28 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=5OonhTZQnqsFU6vSzAfWyNIje-pwDwWMg90Vq2GIY8k=z1uVHJVy9izGN3VF9_Y8wXc4MrEux_EKbfUVnKEYXu0=> [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=5OonhTZQnqsFU6vSzAfWyNIje-pwDwWMg90Vq2GIY8k=MBc4zJ-tba0b480nSFAvHAKeJMSeWfEw39O4Pz-Az3M=> This Series: Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan backed distributed logical router. This patch: A. Key difference between an overlay logical switch and vlan backed logical switch is that for vlan logical switches packets are not encapsulated. Hence, if a distributed router port is connected to vlan type logical switch, then router port mac as source mac could be seen from multiple hypervisors. Same pairs coming from multiple ports from a top of the rack switch (TOR) perspective could be seen as a security threat and it could send alarms, drop the packets or block the ports etc. This patch addresses the same by introducing the concept of chassis mac. A chassis mac is CMS provisioned unique mac per chassis. For any routed packet (i.e source mac is router port mac) going on the wire on a vlan type logical switch, we will replace its source mac with chassis mac. This replacing of source mac with chassis mac will happen in table=65 of the logical switch datapath. A flow is added at priority 150, which matches the source mac and replaces it with chassis mac if the value is a router port mac. Example flow: cookie=0x0, duration=67765.830s, table=65, n_packets=0, n_bytes=0, idle_age=65534, hard_age=65534, priority=150,reg15=0x1,metadata=0x4, dl_src=00:00:01:01:02:03 actions=mod_dl_src:aa:bb:cc:dd:ee:ff, mod_vlan_vid:1000,output:16 Here, 00:00:01:01:02:03 is router port mac and aa:bb:cc:dd:ee:ff is chassis mac. B. This patch adds one more change of associating "types" with logical switches. i.e a logical switch could be of type "overlay" or "bridged". This is done to explicitly call out that on a bridged logical switch there will no encapsulation. Just a localnet port's presence is not sufficient, as we do encap while redirecting the packet to gateway chassis. By marking the logical switch as bridged, we can either avoid redirection totally (if there is no NAT) or do redirection based on router port mac, rather than encap over a tunnel. Hi Ankur, For most part the patch looks fine to me. Please see some additional comments. I still have few questions about the type added to the logical switch. How would you handle the case for upgrades ? i.e if the deployment already have a logical switch with a localnet port. What if the user sets the type as - bridged, but doesn't create the localnet port ? Thanks Numan Signed-off-by: Ankur Sharma mailto:ankur.sha...@nutanix.com>> --- ovn/controller/binding.c| 12 +-- ovn/c
Re: [ovs-dev] [PATCH v7 1/2] OVN: Enable E-W Traffic, Vlan backed DVR
On Thu, May 9, 2019 at 4:28 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 3 E-W and Layer 3 N-S (NO NAT) changes for vlan > backed distributed logical router. > > This patch: > > A. > Key difference between an overlay logical switch and > vlan backed logical switch is that for vlan logical switches > packets are not encapsulated. > > Hence, if a distributed router port is connected to vlan type > logical switch, then router port mac as source mac could be > seen from multiple hypervisors. Same pairs coming > from multiple ports from a top of the rack switch (TOR) perspective > could be seen as a security threat and it could send alarms, drop > the packets or block the ports etc. > > This patch addresses the same by introducing the concept of chassis mac. > A chassis mac is CMS provisioned unique mac per chassis. For any routed > packet > (i.e source mac is router port mac) going on the wire on a vlan type > logical switch, we will replace its source mac with chassis mac. > > This replacing of source mac with chassis mac will happen in table=65 > of the logical switch datapath. A flow is added at priority 150, which > matches the source mac and replaces it with chassis mac if the value > is a router port mac. > > Example flow: > cookie=0x0, duration=67765.830s, table=65, n_packets=0, n_bytes=0, > idle_age=65534, hard_age=65534, priority=150,reg15=0x1,metadata=0x4, > dl_src=00:00:01:01:02:03 actions=mod_dl_src:aa:bb:cc:dd:ee:ff, > mod_vlan_vid:1000,output:16 > > Here, 00:00:01:01:02:03 is router port mac and aa:bb:cc:dd:ee:ff > is chassis mac. > > B. > This patch adds one more change of associating "types" with logical > switches. i.e a logical switch could be of type "overlay" or "bridged". > This is done to explicitly call out that on a bridged logical > switch there will no encapsulation. > Just a localnet port's presence is not sufficient, as we do > encap while redirecting the packet to gateway chassis. > By marking the logical switch as bridged, we can either > avoid redirection totally (if there is no NAT) or do redirection > based on router port mac, rather than encap over a tunnel. > > Hi Ankur, For most part the patch looks fine to me. Please see some additional comments. I still have few questions about the type added to the logical switch. How would you handle the case for upgrades ? i.e if the deployment already have a logical switch with a localnet port. What if the user sets the type as - bridged, but doesn't create the localnet port ? Thanks Numan > Signed-off-by: Ankur Sharma > --- > ovn/controller/binding.c| 12 +-- > ovn/controller/chassis.c| 64 +++- > ovn/controller/chassis.h| 4 + > ovn/controller/ovn-controller.8.xml | 10 ++ > ovn/controller/ovn-controller.c | 2 +- > ovn/controller/ovn-controller.h | 5 +- > ovn/controller/physical.c | 96 ++ > ovn/northd/ovn-northd.c | 38 +++ > ovn/ovn-architecture.7.xml | 24 + > ovn/ovn-nb.ovsschema| 10 +- > ovn/ovn-nb.xml | 18 > ovn/ovn-sb.xml | 15 +++ > ovn/utilities/ovn-nbctl.c | 49 +++-- > tests/ovn-nbctl.at | 48 ++--- > tests/ovn-northd.at | 22 > tests/ovn.at| 197 > > 16 files changed, 582 insertions(+), 32 deletions(-) > > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c > index 404f0e7..e9461ef 100644 > --- a/ovn/controller/binding.c > +++ b/ovn/controller/binding.c > @@ -159,13 +159,11 @@ add_local_datapath__(struct ovsdb_idl_index > *sbrec_datapath_binding_by_key, > sbrec_port_binding_by_name, > peer->datapath, false, > depth + 1, local_datapaths); > -ld->n_peer_dps++; > -ld->peer_dps = xrealloc( > -ld->peer_dps, > -ld->n_peer_dps * sizeof *ld->peer_dps); > -ld->peer_dps[ld->n_peer_dps - 1] = > datapath_lookup_by_key( > -sbrec_datapath_binding_by_key, > -peer->datapath->tunnel_key); > +ld->n_peer_ports++; > +ld->peer_ports = xrealloc(ld->peer_ports, > + ld->n_peer_ports * > + sizeof *ld->peer_ports); > +ld->peer_ports[ld->n_peer_ports - 1] = peer; > } > } > } > diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c > index
[ovs-dev] [PATCH v7 1/2] OVN: Enable E-W 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 3 E-W and Layer 3 N-S (NO NAT) changes for vlan
backed distributed logical router.
This patch:
A.
Key difference between an overlay logical switch and
vlan backed logical switch is that for vlan logical switches
packets are not encapsulated.
Hence, if a distributed router port is connected to vlan type
logical switch, then router port mac as source mac could be
seen from multiple hypervisors. Same pairs coming
from multiple ports from a top of the rack switch (TOR) perspective
could be seen as a security threat and it could send alarms, drop
the packets or block the ports etc.
This patch addresses the same by introducing the concept of chassis mac.
A chassis mac is CMS provisioned unique mac per chassis. For any routed packet
(i.e source mac is router port mac) going on the wire on a vlan type
logical switch, we will replace its source mac with chassis mac.
This replacing of source mac with chassis mac will happen in table=65
of the logical switch datapath. A flow is added at priority 150, which
matches the source mac and replaces it with chassis mac if the value
is a router port mac.
Example flow:
cookie=0x0, duration=67765.830s, table=65, n_packets=0, n_bytes=0,
idle_age=65534, hard_age=65534, priority=150,reg15=0x1,metadata=0x4,
dl_src=00:00:01:01:02:03 actions=mod_dl_src:aa:bb:cc:dd:ee:ff,
mod_vlan_vid:1000,output:16
Here, 00:00:01:01:02:03 is router port mac and aa:bb:cc:dd:ee:ff
is chassis mac.
B.
This patch adds one more change of associating "types" with logical
switches. i.e a logical switch could be of type "overlay" or "bridged".
This is done to explicitly call out that on a bridged logical
switch there will no encapsulation.
Just a localnet port's presence is not sufficient, as we do
encap while redirecting the packet to gateway chassis.
By marking the logical switch as bridged, we can either
avoid redirection totally (if there is no NAT) or do redirection
based on router port mac, rather than encap over a tunnel.
Signed-off-by: Ankur Sharma
---
ovn/controller/binding.c| 12 +--
ovn/controller/chassis.c| 64 +++-
ovn/controller/chassis.h| 4 +
ovn/controller/ovn-controller.8.xml | 10 ++
ovn/controller/ovn-controller.c | 2 +-
ovn/controller/ovn-controller.h | 5 +-
ovn/controller/physical.c | 96 ++
ovn/northd/ovn-northd.c | 38 +++
ovn/ovn-architecture.7.xml | 24 +
ovn/ovn-nb.ovsschema| 10 +-
ovn/ovn-nb.xml | 18
ovn/ovn-sb.xml | 15 +++
ovn/utilities/ovn-nbctl.c | 49 +++--
tests/ovn-nbctl.at | 48 ++---
tests/ovn-northd.at | 22
tests/ovn.at| 197
16 files changed, 582 insertions(+), 32 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 404f0e7..e9461ef 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -159,13 +159,11 @@ add_local_datapath__(struct ovsdb_idl_index
*sbrec_datapath_binding_by_key,
sbrec_port_binding_by_name,
peer->datapath, false,
depth + 1, local_datapaths);
-ld->n_peer_dps++;
-ld->peer_dps = xrealloc(
-ld->peer_dps,
-ld->n_peer_dps * sizeof *ld->peer_dps);
-ld->peer_dps[ld->n_peer_dps - 1] = datapath_lookup_by_key(
-sbrec_datapath_binding_by_key,
-peer->datapath->tunnel_key);
+ld->n_peer_ports++;
+ld->peer_ports = xrealloc(ld->peer_ports,
+ ld->n_peer_ports *
+ sizeof *ld->peer_ports);
+ld->peer_ports[ld->n_peer_ports - 1] = peer;
}
}
}
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 0f537f1..d029091 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -23,6 +23,7 @@
#include "lib/vswitch-idl.h"
#include "openvswitch/dynamic-string.h"
#include "openvswitch/vlog.h"
+#include "openvswitch/ofp-parse.h"
#include "ovn/lib/chassis-index.h"
#include "ovn/lib/ovn-sb-idl.h"
#include "ovn-controller.h"
@@ -69,6 +70,12 @@ get_bridge_mappings(const struct smap *ext_ids)
}
static const char *
+get_chassis_mac_mappings(const struct smap *ext_ids)
+{
+return smap_get_def(ext_ids, "ovn-chassis-mac-mappings", "");
+}
+
+static const char *
get_cms_options(const struct smap *ext_ids)
{
