On Fri, May 3, 2019 at 3:04 AM Ankur Sharma <ankur.sha...@nutanix.com> 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 <mac,vlan> 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, I applied both the patches in the series and I tested them. For now I have only tested E/W scenario. I have one questions. In my setup, I created 2 bridged logical switches - sw0 and sw1 and attached a router to it. I have 2 chassis - c1 and c2. On c1 mac-bindings are configured as - sw0: c1-sw0-MAC, sw1:c1-sw1-MAC On c2 mac-bindings are configured as - sw0: c2-sw0-MAC, sw1:c2-sw1-MAC I have a logical port - sw0-port1 (which belongs to sw0 logical switch) with IP - 10.0.0.4 and another logical port - sw1-port1 (which belongs to sw1) with IP - 20.0.0.4. sw0-port1 is bound on chassis c1 and sw1-port1 on chassis c2. When sw0-port1 (10.0.0.4) pings sw1-port1 (20.0.0.4), the router port (20.0.0.1) MAC is replaced by c1-sw1-MAC in table 65 as expected. When sw1-port1 receives the packet it will receive the packet as - --------------------------------------------------------------------------------------------------------------------------- | SRC MAC (c1-sw1-MAC) | SRC IP (10.0.0.4) | DST MAC (sw1-port1 mac) | DST IP (20.0.0.4) | --------------------------------------------------------------------------------------------------------------------------- when sw1-port1 replies for the ping, it sends the packet as - -------------------------------------------------------------------------------------------------------------------------------------------------------- | SRC MAC (sw1-port1 MAC) | SRC IP (20.0.0.4) | DST MAC ( he router port (20.0.0.1) MAC ) | DST IP (10.0.0.4) | -------------------------------------------------------------------------------------------------------------------------------------------------------- I observed for the packets on the sw1-port1 interface on c2. When ovn-controller delivers the packet to sw1-port1, shouldn't it replace the c1-sw1-MAC with the router port (20.0.0.1) MAC ? Or it's fine ? Few comments inline Thanks Numan > Signed-off-by: Ankur Sharma <ankur.sha...@nutanix.com> > --- > ovn/controller/binding.c | 12 +-- > ovn/controller/chassis.c | 66 +++++++++++- > 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 | 12 +++ > 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, 572 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..617a7e1 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(&cfg->external_ids); > + const char *chassis_macs = > get_chassis_mac_mappings(&cfg->external_ids); > > struct ds iface_types = DS_EMPTY_INITIALIZER; > ds_put_cstr(&iface_types, ""); > @@ -190,18 +198,22 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > = smap_get_def(&chassis_rec->external_ids, "iface-types", ""); > const char *chassis_cms_options > = get_cms_options(&chassis_rec->external_ids); > + const char *chassis_mac_mappings > + = get_chassis_mac_mappings(&chassis_rec->external_ids); > > /* If any of the external-ids should change, update them. */ > if (strcmp(bridge_mappings, chassis_bridge_mappings) || > strcmp(datapath_type, chassis_datapath_type) || > strcmp(iface_types_str, chassis_iface_types) || > - strcmp(cms_options, chassis_cms_options)) { > + strcmp(cms_options, chassis_cms_options) || > + strcmp(chassis_macs, chassis_mac_mappings)) { > struct smap new_ids; > smap_clone(&new_ids, &chassis_rec->external_ids); > smap_replace(&new_ids, "ovn-bridge-mappings", > bridge_mappings); > smap_replace(&new_ids, "datapath-type", datapath_type); > smap_replace(&new_ids, "iface-types", iface_types_str); > smap_replace(&new_ids, "ovn-cms-options", cms_options); > + smap_replace(&new_ids, "ovn-chassis-mac-mappings", > chassis_macs); > sbrec_chassis_verify_external_ids(chassis_rec); > sbrec_chassis_set_external_ids(chassis_rec, &new_ids); > smap_destroy(&new_ids); > @@ -319,6 +331,58 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > return chassis_rec; > } > > +bool > +chassis_get_mac(const struct sbrec_chassis *chassis_rec, > + const char *bridge_mapping, > + struct eth_addr *chassis_mac) > +{ > + const char *tokens > + = get_chassis_mac_mappings(&chassis_rec->external_ids); > + char *save_ptr = NULL; > + char *token; > + bool ret = false; > + > + if (!strlen(tokens)) { > + return false; > + } > + > + char *tokstr = xstrdup(tokens); > + > + /* Format for a chassis mac configuration is: > + * ovn-chassis-mac-mappings="bridge-name1:MAC1,bridge-name2:MAC2" > + */ > + for (token = strtok_r(tokstr, ",", &save_ptr); > + token != NULL; > + token = strtok_r(NULL, ",", &save_ptr)) { > + char *save_ptr2 = NULL; > + char *chassis_mac_bridge = strtok_r(token, ":", &save_ptr2); > + char *chassis_mac_str = strtok_r(NULL, "", &save_ptr2); > + > + if (!strcmp(chassis_mac_bridge, bridge_mapping)) { > + struct eth_addr temp_mac; > + char *err_str = NULL; > + > + ret = true; > + > + /* Return the first chassis mac. */ > + if ((err_str = str_to_mac(chassis_mac_str, &temp_mac))) { > + free(err_str); > + ret = false; > + continue; > + } > + > + memcpy(chassis_mac, &temp_mac, sizeof(struct eth_addr)); > + > + goto done; > I would suggest to use "break" instead of "goto done" and remove the goto "done" label. > + } > + } > + > +done: > + free(tokstr); > + > + return ret; > +} > + > /* Returns true if the database is all cleaned up, false if more work is > * required. */ > bool > diff --git a/ovn/controller/chassis.h b/ovn/controller/chassis.h > index 9847e19..5bc5ff3 100644 > --- a/ovn/controller/chassis.h > +++ b/ovn/controller/chassis.h > @@ -17,6 +17,7 @@ > #define OVN_CHASSIS_H 1 > > #include <stdbool.h> > +#include "openvswitch/types.h" > There is no need to include this header file. You can just do forward declaration of "struct eth_addr" > > struct ovsdb_idl; > struct ovsdb_idl_index; > @@ -36,5 +37,8 @@ const struct sbrec_chassis *chassis_run( > const struct sset *transport_zones); > bool chassis_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, > const struct sbrec_chassis *); > +bool chassis_get_mac(const struct sbrec_chassis *chassis, > + const char *bridge_mapping, > + struct eth_addr *chassis_mac); > > #endif /* ovn/chassis.h */ > diff --git a/ovn/controller/ovn-controller.8.xml > b/ovn/controller/ovn-controller.8.xml > index 9721d9a..18f66fe 100644 > --- a/ovn/controller/ovn-controller.8.xml > +++ b/ovn/controller/ovn-controller.8.xml > @@ -182,6 +182,16 @@ > transport zone. > </p> > </dd> > + <dt><code>external_ids:ovn-chassis-mac-mappings</code></dt> > + <dd> > + A list of key-value pairs that map a chassis specific mac to > + a physical network name. An example > + value mapping two chassis macs to two physical network names > would be: > + > <code>physnet1:aa:bb:cc:dd:ee:ff,physnet2:a1:b2:c3:d4:e5:f6</code>. > + These are the macs that ovn-controller will replace a router port > + mac with, if packet is going from a distributed router port on > + vlan type logical switch. > + </dd> > </dl> > > <p> > diff --git a/ovn/controller/ovn-controller.c > b/ovn/controller/ovn-controller.c > index 69eeee5..90b2d57 100644 > --- a/ovn/controller/ovn-controller.c > +++ b/ovn/controller/ovn-controller.c > @@ -867,7 +867,7 @@ main(int argc, char *argv[]) > struct local_datapath *cur_node, *next_node; > HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, > &local_datapaths) { > - free(cur_node->peer_dps); > + free(cur_node->peer_ports); > hmap_remove(&local_datapaths, &cur_node->hmap_node); > free(cur_node); > } > diff --git a/ovn/controller/ovn-controller.h > b/ovn/controller/ovn-controller.h > index 6afd727..a4c1309 100644 > --- a/ovn/controller/ovn-controller.h > +++ b/ovn/controller/ovn-controller.h > @@ -59,8 +59,9 @@ struct local_datapath { > /* True if this datapath contains an l3gateway port located on this > * hypervisor. */ > bool has_local_l3gateway; > - const struct sbrec_datapath_binding **peer_dps; > - size_t n_peer_dps; > + > + const struct sbrec_port_binding **peer_ports; > + size_t n_peer_ports; > }; > > struct local_datapath *get_local_datapath(const struct hmap *, > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c > index 7386404..d689e89 100644 > --- a/ovn/controller/physical.c > +++ b/ovn/controller/physical.c > @@ -20,6 +20,7 @@ > #include "ha-chassis.h" > #include "lflow.h" > #include "lport.h" > +#include "chassis.h" > #include "lib/bundle.h" > #include "openvswitch/poll-loop.h" > #include "lib/uuid.h" > @@ -30,6 +31,7 @@ > #include "openvswitch/ofp-actions.h" > #include "openvswitch/ofpbuf.h" > #include "openvswitch/vlog.h" > +#include "openvswitch/ofp-parse.h" > #include "ovn-controller.h" > #include "ovn/lib/chassis-index.h" > #include "ovn/lib/ovn-sb-idl.h" > @@ -233,6 +235,93 @@ get_zone_ids(const struct sbrec_port_binding *binding, > } > > static void > +put_replace_router_port_mac_flows(const struct > + sbrec_port_binding *localnet_port, > + const struct sbrec_chassis *chassis, > + const struct hmap *local_datapaths, > + struct ofpbuf *ofpacts_p, > + ofp_port_t ofport, > + struct hmap *flow_table) > +{ > + struct local_datapath *ld = get_local_datapath(local_datapaths, > + > localnet_port->datapath-> > + tunnel_key); > + uint32_t dp_key = localnet_port->datapath->tunnel_key; > + uint32_t port_key = localnet_port->tunnel_key; > + int tag = localnet_port->tag ? *localnet_port->tag : 0; > + const char *network = smap_get(&localnet_port->options, > "network_name"); > + struct eth_addr chassis_mac; > + > + ovs_assert(ld); > + > + if (!network) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, "Physical network not configured for datapath: > %ld " > + "with localnet port", > + localnet_port->datapath->tunnel_key); > + return; > + } > + > + /* Get chassis mac */ > + if (chassis_get_mac(chassis, network, &chassis_mac) == false) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > + /* Keeping the log level low for backward compatibility. > + * Chassis mac is a new configuration. > + */ > + VLOG_DBG_RL(&rl, "Could not get chassis mac for network: %s", > network); > + return; > + } > + > + for (int i = 0; i < ld->n_peer_ports; i++) { > + const struct sbrec_port_binding *rport_binding = > ld->peer_ports[i]; > + struct eth_addr router_port_mac; > + char *err_str = NULL; > + struct match match; > + struct ofpact_mac *replace_mac; > + > + /* Table 65, priority 150. > + * ======================= > + * > + * Implements output to localnet port. > + * a. Flow replaces ingress router port mac with a chassis mac. > + * b. Flow appends the vlan id localnet port is configured with. > + */ > + match_init_catchall(&match); > + ofpbuf_clear(ofpacts_p); > + > + ovs_assert(rport_binding->n_mac == 1); > + if ((err_str = str_to_mac(rport_binding->mac[0], > &router_port_mac))) { > + /* Parsing of mac failed. */ > + VLOG_WARN("Parsing or router port mac failed for router port: > %s, " > + "with error: %s", rport_binding->logical_port, > err_str); > + free(err_str); > + return; > + } > + > + /* Replace Router mac flow */ > + > + replace_mac = ofpact_put_SET_ETH_SRC(ofpacts_p); > + replace_mac->mac = chassis_mac; > + > + match_set_metadata(&match, htonll(dp_key)); > + match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, port_key); > + match_set_dl_src(&match, router_port_mac); > + > + if (tag) { > + struct ofpact_vlan_vid *vlan_vid; > + vlan_vid = ofpact_put_SET_VLAN_VID(ofpacts_p); > + vlan_vid->vlan_vid = tag; > + vlan_vid->push_vlan_if_needed = true; > + } > + > + ofpact_put_OUTPUT(ofpacts_p)->port = ofport; > + > + ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 150, 0, > + &match, ofpacts_p); > + } > +} > + > +static void > put_local_common_flows(uint32_t dp_key, uint32_t port_key, > uint32_t parent_port_key, > const struct zone_ids *zone_ids, > @@ -701,6 +790,13 @@ consider_port_binding(struct ovsdb_idl_index > *sbrec_port_binding_by_name, > } > ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100, 0, > &match, ofpacts_p); > + > + if (!strcmp(binding->type, "localnet")) { > + put_replace_router_port_mac_flows(binding, chassis, > + local_datapaths, ofpacts_p, > + ofport, flow_table); > + } > + > } else if (!tun && !is_ha_remote) { > /* Remote port connected by localnet port */ > /* Table 33, priority 100. > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index de0c06d..e4c0da0 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -86,6 +86,12 @@ enum ovn_datapath_type { > DP_ROUTER /* OVN logical router. */ > }; > > +/* Network type of a datapath */ > +enum ovn_datapath_nw_type { > + DP_NETWORK_OVERLAY, > + DP_NETWORK_BRIDGED > +}; > + > /* Returns an "enum ovn_stage" built from the arguments. > * > * (It's better to use ovn_stage_build() for type-safety reasons, but > inline > @@ -445,6 +451,8 @@ struct ovn_datapath { > > bool has_unknown; > > + enum ovn_datapath_nw_type network_type; > + > /* IPAM data. */ > struct ipam_info ipam_info; > > @@ -491,6 +499,27 @@ cleanup_macam(struct hmap *macam_) > } > } > > +static void > +ovn_datapath_update_nw_type(struct ovn_datapath *od) > +{ > + if (!od->nbs) { > + return; > + } > + > + if (!od->nbs->network_type || > + !strlen(od->nbs->network_type) || > + !strcmp(od->nbs->network_type, "overlay")) { > + /* No value in network_type is taken as OVERLAY. */ > + od->network_type = DP_NETWORK_OVERLAY; > + } else if (!strcmp(od->nbs->network_type, "bridged")) { > + od->network_type = DP_NETWORK_BRIDGED; > + } else { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "bad network type %s, for %s", > + od->nbs->network_type, od->nbs->name); > + } > +} > + > static struct ovn_datapath * > ovn_datapath_create(struct hmap *datapaths, const struct uuid *key, > const struct nbrec_logical_switch *nbs, > @@ -682,6 +711,13 @@ ovn_datapath_update_external_ids(struct ovn_datapath > *od) > if (name2 && name2[0]) { > smap_add(&ids, "name2", name2); > } > + > + if (od->nbs) { > + smap_add(&ids, "network-type", > + (od->nbs->network_type && strlen(od->nbs->network_type)) > ? > + od->nbs->network_type : "overlay"); > + } > + > sbrec_datapath_binding_set_external_ids(od->sb, &ids); > smap_destroy(&ids); > } > @@ -734,9 +770,11 @@ join_datapaths(struct northd_context *ctx, struct > hmap *datapaths, > ovs_list_remove(&od->list); > ovs_list_push_back(both, &od->list); > ovn_datapath_update_external_ids(od); > + ovn_datapath_update_nw_type(od); > } else { > od = ovn_datapath_create(datapaths, &nbs->header_.uuid, > nbs, NULL, NULL); > + ovn_datapath_update_nw_type(od); > ovs_list_push_back(nb_only, &od->list); > } > > diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml > index 8c9e106..4025a11 100644 > --- a/ovn/ovn-architecture.7.xml > +++ b/ovn/ovn-architecture.7.xml > @@ -1407,6 +1407,18 @@ > egress pipeline of the destination localnet logical switch datapath > and goes out of the integration bridge to the provider bridge ( > belonging to the destination logical switch) via the localnet port. > + > + For a distributed router port, we cannot use the router port mac > + while sending the packet to provider port. This is because, since > + router port is distributed, hence same router port mac would appear > + from different ports in the provider network. > + > + To mitigate this, we replace router port mac with a chassis unique > + mac whenever a routed packet is being sent to provider network. > + > + For example, CMS should program chassis mac through following > + ovs mapping: > + ovn-bridge-mappings="physnet1:br-eth1" > </li> > > <li> > diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema > index 2c87cbb..09df854 100644 > --- a/ovn/ovn-nb.ovsschema > +++ b/ovn/ovn-nb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Northbound", > - "version": "5.16.0", > - "cksum": "923459061 23095", > + "version": "5.17.0", > + "cksum": "1251608309 23487", > "tables": { > "NB_Global": { > "columns": { > @@ -29,6 +29,12 @@ > "Logical_Switch": { > "columns": { > "name": {"type": "string"}, > + "network_type": {"type": {"key": {"type": "string", > + "enum": ["set", > ["overlay", > + > "bridged"] > + ]}, > + "min": 0, > + "max": 1}}, > "ports": {"type": {"key": {"type": "uuid", > "refTable": > "Logical_Switch_Port", > "refType": "strong"}, > diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml > index cbaa949..96c3645 100644 > --- a/ovn/ovn-nb.xml > +++ b/ovn/ovn-nb.xml > @@ -158,6 +158,24 @@ > </p> > </column> > > + <column name="network_type"> > + <p> > + Whether logical switch will fully virtualize the network (i.e > overlay) > + or it simply connects to the physical network (i.e bridged). > + This field will take either of the following values: "overlay" or > + "bridged". > + </p> > + > + <p> > + An "overlay" type logical switch means that 24 bit virtual network > + identifier defines its broadcast domain and hence packets leaving > + the chassis will be encapsulated. A "bridged" logical switch > means that > + it uses 12 bit vlan id as broadcast domain and packets leaving the > + chassis would not be encapsulated, but would have a vlan header > + instead (logical switches with vlan zero are also to be assigned > the > + type as "bridged"). > + </p> > + </column> > <column name="load_balancer"> > Load balance a virtual ip address to a set of logical port endpoint > ip addresses. > diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml > index 1a2bc1d..74498c6 100644 > --- a/ovn/ovn-sb.xml > +++ b/ovn/ovn-sb.xml > @@ -301,6 +301,14 @@ > See <code>ovn-controller</code>(8) for more information. > </column> > > + <column name="external_ids" key="ovn-chassis-mac-mappings"> > + <code>ovn-controller</code> populates this key with the set of > options > + configured in the <ref table="Open_vSwitch" > + column="external_ids:ovn-chassis-mac-mappings"/> column of the > + Open_vSwitch database's <ref table="Open_vSwitch" > db="Open_vSwitch"/> > + table. See <code>ovn-controller</code>(8) for more information. > + </column> > + > <group title="Common Columns"> > The overall purpose of these columns is described under <code>Common > Columns</code> at the beginning of this document. > @@ -2162,6 +2170,13 @@ tcp.flags = RST; > the <ref db="OVN_Northbound"/> database. > </column> > > + <column name="external_ids" key="logical-switch-type"> > + For a logical datapath that represents a logical switch, > + <code>ovn-northd</code> stores in this key the network type from > + corresponding <ref table="Logical_Switch" db="OVN_Northbound"/> > row in > + the <ref db="OVN_Northbound"/> database. > + </column> > + > <group title="Naming"> > <p> > <code>ovn-northd</code> copies these from the name fields in > the <ref > diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c > index e86ab7f..3f4f215 100644 > --- a/ovn/utilities/ovn-nbctl.c > +++ b/ovn/utilities/ovn-nbctl.c > @@ -572,7 +572,8 @@ General commands:\n\ > show ROUTER print overview of database contents for > ROUTER\n\ > \n\ > Logical switch commands:\n\ > - ls-add [SWITCH] create a logical switch named SWITCH\n\ > + ls-add [SWITCH] [TYPE] create a logical switch named SWITCH of TYPE > \n\ > + bridged or overlay\n\ > ls-del SWITCH delete SWITCH and all its ports\n\ > ls-list print the names of all logical switches\n\ > \n\ > @@ -1013,8 +1014,10 @@ print_lr(const struct nbrec_logical_router *lr, > struct ds *s) > static void > print_ls(const struct nbrec_logical_switch *ls, struct ds *s) > { > - ds_put_format(s, "switch "UUID_FMT" (%s)", > - UUID_ARGS(&ls->header_.uuid), ls->name); > + ds_put_format(s, "switch "UUID_FMT" (%s) (type: %s)", > + UUID_ARGS(&ls->header_.uuid), ls->name, > + ls->network_type && strlen(ls->network_type) ? > + ls->network_type : "overlay"); > print_alias(&ls->external_ids, "neutron:network_name", s); > ds_put_char(s, '\n'); > > @@ -1116,7 +1119,8 @@ nbctl_show(struct ctl_context *ctx) > static void > nbctl_ls_add(struct ctl_context *ctx) > { > - const char *ls_name = ctx->argc == 2 ? ctx->argv[1] : NULL; > + const char *ls_name = ctx->argc >= 2 ? ctx->argv[1] : NULL; > + const char *nw_type = ctx->argc == 3 ? ctx->argv[2] : NULL; > > bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL; > bool add_duplicate = shash_find(&ctx->options, "--add-duplicate") != > NULL; > @@ -1153,6 +1157,33 @@ nbctl_ls_add(struct ctl_context *ctx) > if (ls_name) { > nbrec_logical_switch_set_name(ls, ls_name); > } > + > + if (nw_type) { > + nbrec_logical_switch_set_network_type(ls, nw_type); > + } > +} > + > +static void > +nbctl_ls_set_network_type(struct ctl_context *ctx) > +{ > + const char *ls_name = ctx->argv[1]; > + const char *ls_type = ctx->argv[2]; > + const struct nbrec_logical_switch *ls = NULL; > + > + char *error = ls_by_name_or_uuid(ctx, ls_name, true, &ls); > + > + if (!ls || error) { > + ctx->error = error; > + return; > + } > + > + if (strcmp(ls_type, "bridged") && strcmp(ls_type, "overlay")) { > + ctl_error(ctx, "Invalid type: \"%s\", supported types are > \"bridged\" " > + "and \"overlay\"", ls_type); > + return; > + } > + > + nbrec_logical_switch_set_network_type(ls, ls_type); > } > > static void > @@ -1182,8 +1213,10 @@ nbctl_ls_list(struct ctl_context *ctx) > > smap_init(&switches); > NBREC_LOGICAL_SWITCH_FOR_EACH(ls, ctx->idl) { > - smap_add_format(&switches, ls->name, UUID_FMT " (%s)", > - UUID_ARGS(&ls->header_.uuid), ls->name); > + smap_add_format(&switches, ls->name, UUID_FMT " (%s) (type: %s)", > + UUID_ARGS(&ls->header_.uuid), ls->name, > + ls->network_type && strlen(ls->network_type) ? > + ls->network_type : "overlay"); > } > const struct smap_node **nodes = smap_sort(&switches); > for (size_t i = 0; i < smap_count(&switches); i++) { > @@ -5504,10 +5537,12 @@ static const struct ctl_command_syntax > nbctl_commands[] = { > { "show", 0, 1, "[SWITCH]", NULL, nbctl_show, NULL, "", RO }, > > /* logical switch commands. */ > - { "ls-add", 0, 1, "[SWITCH]", NULL, nbctl_ls_add, NULL, > + { "ls-add", 0, 2, "[SWITCH] [TYPE]", NULL, nbctl_ls_add, NULL, > "--may-exist,--add-duplicate", RW }, > { "ls-del", 1, 1, "SWITCH", NULL, nbctl_ls_del, NULL, "--if-exists", > RW }, > { "ls-list", 0, 0, "", NULL, nbctl_ls_list, NULL, "", RO }, > + { "ls-set-network-type", 2, 2, "SWITCH TYPE", NULL, > + nbctl_ls_set_network_type, NULL, "", RW }, > > /* acl commands. */ > { "acl-add", 5, 6, "{SWITCH | PORTGROUP} DIRECTION PRIORITY MATCH > ACTION", > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at > index 18c5c1d..457d6fd 100644 > --- a/tests/ovn-nbctl.at > +++ b/tests/ovn-nbctl.at > @@ -56,31 +56,39 @@ m4_define([OVN_NBCTL_TEST], > OVN_NBCTL_TEST([ovn_nbctl_basic_switch], [basic switch commands], [ > AT_CHECK([ovn-nbctl ls-add ls0]) > AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl > -<0> (ls0) > +<0> (ls0) (type: overlay) > ]) > > AT_CHECK([ovn-nbctl ls-add ls1]) > AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl > -<0> (ls0) > -<1> (ls1) > +<0> (ls0) (type: overlay) > +<1> (ls1) (type: overlay) > +]) > + > +AT_CHECK([ovn-nbctl ls-add ls2 bridged]) > +AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl > +<0> (ls0) (type: overlay) > +<1> (ls1) (type: overlay) > +<2> (ls2) (type: bridged) > ]) > > +AT_CHECK([ovn-nbctl ls-del ls2]) > AT_CHECK([ovn-nbctl ls-del ls0]) > AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl > -<0> (ls1) > +<0> (ls1) (type: overlay) > ]) > > AT_CHECK([ovn-nbctl show ls0]) > AT_CHECK([ovn-nbctl ls-add ls0]) > AT_CHECK([ovn-nbctl show ls0 | uuidfilt], [0], > - [switch <0> (ls0) > + [switch <0> (ls0) (type: overlay) > ]) > AT_CHECK([ovn-nbctl ls-add ls0], [1], [], > [ovn-nbctl: ls0: a switch with this name already exists > ]) > AT_CHECK([ovn-nbctl --may-exist ls-add ls0]) > AT_CHECK([ovn-nbctl show ls0 | uuidfilt], [0], > - [switch <0> (ls0) > + [switch <0> (ls0) (type: overlay) > ]) > AT_CHECK([ovn-nbctl --add-duplicate ls-add ls0]) > AT_CHECK([ovn-nbctl --may-exist --add-duplicate ls-add ls0], [1], [], > @@ -102,7 +110,23 @@ AT_CHECK([ovn-nbctl --add-duplicate ls-add], [1], [], > ]) > AT_CHECK([ovn-nbctl --may-exist ls-add], [1], [], > [ovn-nbctl: --may-exist requires specifying a name > -])]) > +]) > + > +AT_CHECK([ovn-nbctl ls-set-network-type ls1 bridged]) > +AT_CHECK([ovn-nbctl show ls1 | uuidfilt], [0], > + [switch <0> (ls1) (type: bridged) > +]) > + > +AT_CHECK([ovn-nbctl ls-set-network-type ls1 overlay]) > +AT_CHECK([ovn-nbctl show ls1 | uuidfilt], [0], > + [switch <0> (ls1) (type: overlay) > +]) > + > +AT_CHECK([ovn-nbctl ls-set-network-type ls1 temp], [1], [], > + [ovn-nbctl: Invalid type: "temp", supported types are "bridged" and > "overlay" > +]) > + > +]) > > dnl --------------------------------------------------------------------- > > @@ -1490,7 +1514,7 @@ dnl > --------------------------------------------------------------------- > OVN_NBCTL_TEST([ovn_nbctl_dry_run_mode], [dry run mode], [ > dnl Check that dry run has no permanent effect. > AT_CHECK([ovn-nbctl --dry-run ls-add ls0 -- ls-list | uuidfilt], [0], [dnl > -<0> (ls0) > +<0> (ls0) (type: overlay) > ]) > AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl > ]) > @@ -1498,7 +1522,7 @@ AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl > dnl Check that dry-run mode is not sticky. > AT_CHECK([ovn-nbctl ls-add ls0]) > AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl > -<0> (ls0) > +<0> (ls0) (type: overlay) > ])]) > > dnl --------------------------------------------------------------------- > @@ -1508,13 +1532,13 @@ AT_CHECK([ovn-nbctl ls-add ls0 -- ls-add ls1]) > > dnl Expect one line for one command. > AT_CHECK([ovn-nbctl --oneline ls-list | uuidfilt], [0], [dnl > -<0> (ls0)\n<1> (ls1) > +<0> (ls0) (type: overlay)\n<1> (ls1) (type: overlay) > ]) > > dnl Expect lines for two commands. > AT_CHECK([ovn-nbctl --oneline ls-list -- ls-list | uuidfilt], [0], [dnl > -<0> (ls0)\n<1> (ls1) > -<0> (ls0)\n<1> (ls1) > +<0> (ls0) (type: overlay)\n<1> (ls1) (type: overlay) > +<0> (ls0) (type: overlay)\n<1> (ls1) (type: overlay) > ])]) > > dnl --------------------------------------------------------------------- > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 9588c76..8473d85 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -898,3 +898,25 @@ as northd > OVS_APP_EXIT_AND_WAIT([ovn-northd]) > > AT_CLEANUP > + > +AT_SETUP([ovn -- check logical switch type propagation from NBDB to SBDB]) > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > +ovn_start > + > +ovn-nbctl ls-add ls0 > + > +uuid=`ovn-sbctl --bare --columns=_uuid list Datapath` > +echo "LS UUID is: " $uuid > + > +type=`ovn-sbctl get Datapath_Binding ${uuid} external_ids:network-type` > +echo "LS TYPE is: " $type > +AT_CHECK([ovn-sbctl get Datapath_Binding ${uuid} > external_ids:network-type], [0], [overlay > +]) > + > +ovn-nbctl ls-set-network-type ls0 bridged > +type=`ovn-sbctl get Datapath_Binding ${uuid} external_ids:network-type` > +echo "LS TYPE is: " $type > +AT_CHECK([ovn-sbctl get Datapath_Binding ${uuid} > external_ids:network-type], [0], [bridged > +]) > + > +AT_CLEANUP > diff --git a/tests/ovn.at b/tests/ovn.at > index 592f491..a39d056 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -14020,3 +14020,200 @@ ovn-hv4-0 > > OVN_CLEANUP([hv1], [hv2], [hv3]) > AT_CLEANUP > + > +AT_SETUP([ovn -- 2 HVs, 2 lports/HV, localnet ports, DVR chassis mac]) > +ovn_start > + > + > +# In this test cases we create 2 switches, all connected to same > +# physical network (through br-phys on each HV). Each switch has > +# 1 VIF. Each HV has 1 VIF port. The first digit > +# of VIF port name indicates the hypervisor it is bound to, e.g. > +# lp23 means VIF 3 on hv2. > +# > +# Each switch's VLAN tag and their logical switch ports are: > +# - ls1: > +# - tagged with VLAN 101 > +# - ports: lp11 > +# - ls2: > +# - tagged with VLAN 201 > +# - ports: lp22 > +# > +# Note: a localnet port is created for each switch to connect to > +# physical network. > + > +for i in 1 2; do > + ls_name=ls$i > + ovn-nbctl ls-add $ls_name bridged > + ln_port_name=ln$i > + if test $i -eq 1; then > + ovn-nbctl lsp-add $ls_name $ln_port_name "" 101 > + elif test $i -eq 2; then > + ovn-nbctl lsp-add $ls_name $ln_port_name "" 201 > + fi > + ovn-nbctl lsp-set-addresses $ln_port_name unknown > + ovn-nbctl lsp-set-type $ln_port_name localnet > + ovn-nbctl lsp-set-options $ln_port_name network_name=phys > +done > + > +# lsp_to_ls LSP > +# > +# Prints the name of the logical switch that contains LSP. > +lsp_to_ls () { > + case $1 in dnl ( > + lp?[[11]]) echo ls1 ;; dnl ( > + lp?[[12]]) echo ls2 ;; dnl ( > + *) AT_FAIL_IF([:]) ;; > + esac > +} > + > +vif_to_ls () { > + case $1 in dnl ( > + vif?[[11]]) echo ls1 ;; dnl ( > + vif?[[12]]) echo ls2 ;; dnl ( > + *) AT_FAIL_IF([:]) ;; > + esac > +} > + > +hv_to_num () { > + case $1 in dnl ( > + hv1) echo 1 ;; dnl ( > + hv2) echo 2 ;; dnl ( > + *) AT_FAIL_IF([:]) ;; > + esac > +} > + > +vif_to_num () { > + case $1 in dnl ( > + vif22) echo 22 ;; dnl ( > + vif21) echo 21 ;; dnl ( > + *) AT_FAIL_IF([:]) ;; > + esac > +} > + > +vif_to_hv () { > + case $1 in dnl ( > + vif[[1]]?) echo hv1 ;; dnl ( > + vif[[2]]?) echo hv2 ;; dnl ( > + *) AT_FAIL_IF([:]) ;; > + esac > +} > + > +vif_to_lrp () { > + echo router-to-`vif_to_ls $1` > +} > + > +hv_to_chassis_mac () { > + case $1 in dnl ( > + hv[[1]]) echo aa:bb:cc:dd:ee:11 ;; dnl ( > + hv[[2]]) echo aa:bb:cc:dd:ee:22 ;; dnl ( > + *) AT_FAIL_IF([:]) ;; > + esac > +} > + > +ip_to_hex() { > + printf "%02x%02x%02x%02x" "$@" > +} > + > +net_add n1 > +for i in 1 2; do > + sim_add hv$i > + as hv$i > + ovs-vsctl add-br br-phys > + ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys > + ovs-vsctl set open . > external-ids:ovn-chassis-mac-mappings="phys:aa:bb:cc:dd:ee:$i$i" > + ovn_attach n1 br-phys 192.168.0.$i > + > + ovs-vsctl add-port br-int vif$i$i -- \ > + set Interface vif$i$i external-ids:iface-id=lp$i$i \ > + options:tx_pcap=hv$i/vif$i$i-tx.pcap \ > + options:rxq_pcap=hv$i/vif$i$i-rx.pcap \ > + ofport-request=$i$i > + > + lsp_name=lp$i$i > + ls_name=$(lsp_to_ls $lsp_name) > + > + ovn-nbctl lsp-add $ls_name $lsp_name > + ovn-nbctl lsp-set-addresses $lsp_name "f0:00:00:00:00:$i$i > 192.168.$i.$i" > + ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:00:$i$i > + > + OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup]) > + > +done > + > +ovn-nbctl lr-add router > +ovn-nbctl lrp-add router router-to-ls1 00:00:01:01:02:03 192.168.1.3/24 > +ovn-nbctl <http://192.168.1.3/24+ovn-nbctl> lrp-add router router-to-ls2 > 00:00:01:01:02:05 192.168.2.3/24 > + > +ovn-nbctl lsp-add ls1 ls1-to-router -- set Logical_Switch_Port > ls1-to-router type=router options:router-port=router-to-ls1 -- > lsp-set-addresses ls1-to-router router > +ovn-nbctl lsp-add ls2 ls2-to-router -- set Logical_Switch_Port > ls2-to-router type=router options:router-port=router-to-ls2 -- > lsp-set-addresses ls2-to-router router > + > +ovn-nbctl --wait=sb sync > +#ovn-sbctl dump-flows > + > +ovn-nbctl show > +ovn-sbctl show > + > +OVN_POPULATE_ARP > + > +test_ip() { > + # This packet has bad checksums but logical L3 routing doesn't check. > + local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 > + local > packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}${dst_ip}0035111100080000 > + shift; shift; shift; shift; shift > + hv=`vif_to_hv $inport` > + hv_num=`hv_to_num $hv` > + chassis_mac=`hv_to_chassis_mac $hv` > + as $hv ovs-appctl netdev-dummy/receive $inport $packet > + #as $hv ovs-appctl ofproto/trace br-int in_port=$inport $packet > + in_ls=`vif_to_ls $inport` > + in_lrp=`vif_to_lrp $inport` > + for outport; do > + out_ls=`vif_to_ls $outport` > + if test $in_ls = $out_ls; then > + # Ports on the same logical switch receive exactly the same > packet. > + echo $packet > + else > + # Routing decrements TTL and updates source and dest MAC > + # (and checksum). > + outport_num=`vif_to_num $outport` > + out_lrp=`vif_to_lrp $outport` > + echo > f000000000${outport_num}aabbccddee${hv_num}${hv_num}08004500001c00000000"3f1101"00${src_ip}${dst_ip}0035111100080000 > + fi >> $outport.expected > + done > +} > + > +# Dump a bunch of info helpful for debugging if there's a failure. > + > +echo "------ OVN dump ------" > +ovn-nbctl show > +ovn-sbctl show > + > +echo "------ hv1 dump ------" > +as hv1 ovs-vsctl show > +as hv1 ovs-vsctl list Open_Vswitch > + > +echo "------ hv2 dump ------" > +as hv2 ovs-vsctl show > +as hv2 ovs-vsctl list Open_Vswitch > + > +echo "Send traffic" > +sip=`ip_to_hex 192 168 1 1` > +dip=`ip_to_hex 192 168 2 2` > +test_ip vif11 f00000000011 000001010203 $sip $dip vif22 > + > +sleep 1 > + > +echo "----------- Post Traffic hv1 dump -----------" > +as hv1 ovs-ofctl -O OpenFlow13 dump-flows br-int > +as hv1 ovs-appctl fdb/show br-phys > + > +echo "----------- Post Traffic hv2 dump -----------" > +as hv2 ovs-ofctl -O OpenFlow13 dump-flows br-int > +as hv2 ovs-appctl fdb/show br-phys > + > +OVN_CHECK_PACKETS([hv2/vif22-tx.pcap], [vif22.expected]) > + > +OVN_CLEANUP([hv1],[hv2]) > + > +AT_CLEANUP > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev