Re: [ovs-dev] [PATCH v10] OVN: Enable E-W Traffic, Vlan backed DVR

2019-06-17 Thread Ankur Sharma 
Hi Numan,

Thank for the Ack.
Sent out v11, addressing your comment.

Thanks again.

Regards,
Ankur

From: Numan Siddique 
Sent: Monday, June 17, 2019 3:53 AM
To: Ankur Sharma 
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v10] OVN: Enable E-W Traffic, Vlan backed DVR



On Wed, Jun 12, 2019 at 4:47 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=zaT-JvuNmQv0Q5YQoPtHigGpZlX0UC0QZHKN-a8VzfQ=wxz7gTPh2rjmCdqqfwx-1bR-TmO4cH5vUWwounmM7bI=>
[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=zaT-JvuNmQv0Q5YQoPtHigGpZlX0UC0QZHKN-a8VzfQ=3viJBXBU_4-d5yneJW8CgdfdmpDFL_vbjyydTdEZzrI=>

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 backed
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.

Signed-off-by: Ankur Sharma 
mailto:ankur.sha...@nutanix.com>>

Thanks Ankur for the patch.

Acked-by: Numan Siddique mailto:nusid...@redhat.com>>

There is just one small minor comment. It would be nice if you can address it,

Thanks
Numan


---
 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 |   4 +-
 ovn/controller/ovn-controller.h |   5 +-
 ovn/controller/physical.c   |  95 +
 ovn/ovn-architecture.7.xml  |  24 +
 ovn/ovn-sb.xml  |   8 ++
 tests/ovn.at 
[ovn.at]<https://urldefense.proofpoint.com/v2/url?u=http-3A__ovn.at=DwMFaQ=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=zaT-JvuNmQv0Q5YQoPtHigGpZlX0UC0QZHKN-a8VzfQ=3liFn9Ja4awYUt3zaPN5Wsw1PO0SLdzD9kiAPJ09Oco=>
| 197 
 10 files changed, 411 insertions(+), 12 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index b62b3da..c73d1aa 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..8403212 100644
--- a/ovn/contr

Re: [ovs-dev] [PATCH v10] OVN: Enable E-W Traffic, Vlan backed DVR

2019-06-17 Thread Numan Siddique 
On Wed, Jun 12, 2019 at 4:47 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
>
> 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 backed
> 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.
>
> Signed-off-by: Ankur Sharma 
>

Thanks Ankur for the patch.

Acked-by: Numan Siddique 

There is just one small minor comment. It would be nice if you can address
it,

Thanks
Numan



> ---
>  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 |   4 +-
>  ovn/controller/ovn-controller.h |   5 +-
>  ovn/controller/physical.c   |  95 +
>  ovn/ovn-architecture.7.xml  |  24 +
>  ovn/ovn-sb.xml  |   8 ++
>  tests/ovn.at| 197
> 
>  10 files changed, 411 insertions(+), 12 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index b62b3da..c73d1aa 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..8403212 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)
>  {
>  return smap_get_def(ext_ids, "ovn-cms-options", "");
> @@ -162,6 +169,7 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>  const char *datapath_type =
>  br_int && br_int->datapath_type ? br_int->datapath_type : "";
>  const char *cms_options = get_cms_options(>external_ids);
> +const char *chassis_macs =
> get_chassis_mac_mappings(>external_ids);
>
>  struct ds iface_types = DS_EMPTY_INITIALIZER;
>  ds_put_cstr(_types, "");
> @@ -190,18 +198,22 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>  =

[ovs-dev] [PATCH v10] OVN: Enable E-W Traffic, Vlan backed DVR

2019-06-11 Thread Ankur Sharma 
Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing

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 backed
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.

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 |   4 +-
 ovn/controller/ovn-controller.h |   5 +-
 ovn/controller/physical.c   |  95 +
 ovn/ovn-architecture.7.xml  |  24 +
 ovn/ovn-sb.xml  |   8 ++
 tests/ovn.at| 197 
 10 files changed, 411 insertions(+), 12 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index b62b3da..c73d1aa 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..8403212 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)
 {
 return smap_get_def(ext_ids, "ovn-cms-options", "");
@@ -162,6 +169,7 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 const char *datapath_type =
 br_int && br_int->datapath_type ? br_int->datapath_type : "";
 const char *cms_options = get_cms_options(>external_ids);
+const char *chassis_macs = get_chassis_mac_mappings(>external_ids);
 
 struct ds iface_types = DS_EMPTY_INITIALIZER;
 ds_put_cstr(_types, "");
@@ -190,18 +198,22 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
 = smap_get_def(_rec->external_ids, "iface-types", "");
 const char *chassis_cms_options
 = get_cms_options(_rec->external_ids);
+const char *chassis_mac_mappings
+= get_chassis_mac_mappings(_rec->external_ids);
 
 /* If any of the external-ids should change, update them. */
 if (strcmp(bridge_mappings, chassis_bridge_mappings) ||
 strcmp(datapath_type,