mir/openvswitch
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-10-11 13:57:52 +00:00
Code Issues Releases Wiki Activity

ovn: Implement logical patch ports.

Browse Source 
This implementation is suboptimal for several reasons.  First, it
creates an OVS port for every OVN logical patch port, not just for the
ones that are actually useful on this hypervisor.  Second, it's
wasteful to create an OVS patch port per OVN logical patch port, when
really there's no benefit to them beyond a way to identify how a
packet ingressed into a logical datapath.

There are two obvious ways to improve the situation here, by modifying
OVS:

    1. Add a way to configure in OVS which fields are preserved on a
       hop across an OVS patch port.  If MFF_LOG_DATAPATH and
       MFF_LOG_INPORT were preserved, then only a single pair of OVS
       patch ports would be required regardless of the number of OVN
       logical patch ports.

    2. Add a new OpenFlow extension action modeled on "resubmit" that
       also saves and restores the packet data and metadata (the
       inability to do this is the only reason that "resubmit" can't
       be used already).  Or add OpenFlow extension actions to
       otherwise save and restore packet data and metadata.

We should probably choose one of those in the medium to long term, but
I don't know which one.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Justin Pettit <jpettit@nicira.com>
This commit is contained in:
Ben Pfaff
2015-10-16 13:32:03 -07:00
parent 14f82efde4
commit d387d24dca
7 changed files with 177 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
ovn/TODO
View File

@@ -18,31 +18,22 @@ one router to another, this doesn't seem to matter (just put more than
one connection between them), but for connections between a router and
a switch it might matter because a switch has only one router port.
*** Logical router port names in ACLs
Currently the ACL table documents that the logical router port is
always named "ROUTER". This can't work directly using logical patch
ports to connect a logical switch to its logical router, because every
row in the Logical_Port table must have a unique name. This probably
means that we should change the convention for the ACL table so that
the logical router port name is unique; for example, we could change
the Logical_Router_Port table to require the 'name' column to be
unique, and then use that name in the ACL table.
Another alternative would be to add a way to have aliases for logical
ports, but I'm not sure that's a rathole we really want to go down.
** OVN_SB schema
*** Logical datapath interconnection
There needs to be a way in the OVN_Southbound database to express
connections between logical datapaths, so that packets can pass from a
logical switch to its logical router (and vice versa) and from one
logical router to another.
One way to do this would be to introduce logical patch ports, closely
modeled on the "physical" patch ports that OVS has had for ages. Each
logical patch port would consist of two rows in the Port_Binding table
(one in each logical datapath), with type "patch" and an option "peer"
that names the other logical port in the pair.
If we do it this way then we'll need to figure out one odd special
case. Currently the ACL table documents that the logical router port
is always named "ROUTER". This can't be done directly with this patch
port technique, because every row in the Logical_Port table must have
a unique name. This probably means that we should change the
convention for the ACL table so that the logical router port name is
unique; for example, we could change the Logical_Router_Port table to
require the 'name' column to be unique, and then use that name in the
ACL table.
*** Allow output to ingress port
Sometimes when a packet ingresses into a router, it has to egress the

14
ovn/controller/ovn-controller.8.xml
View File

@@ -179,6 +179,20 @@
value.
</p>
</dd>
<dt>
<code>external-ids:ovn-logical-patch-port</code> in the
<code>Port</code> table
</dt>
<dd>
<p>
This key identifies a patch port as one created by
<code>ovn-controller</code> to implement an OVN logical patch port
within the integration bridge. Its value is the name of the OVN
logical patch port that it implements.
</p>
</dd>
</dl>
<h1>Runtime Management Commands</h1>

70
ovn/controller/patch.c
View File

@@ -48,13 +48,13 @@ match_patch_port(const struct ovsrec_port *port, const char *peer)
}
/* Creates a patch port in bridge 'src' named 'src_name', whose peer is
* 'dst_name' in bridge 'dst'. Initializes the patch port's
* external-ids:ovn-localnet-port to 'network'.
* 'dst_name' in bridge 'dst'. Initializes the patch port's external-ids:'key'
* to 'key'.
*
* If such a patch port already exists, removes it from 'existing_ports'. */
static void
create_patch_port(struct controller_ctx *ctx,
const char *network,
const char *key, const char *value,
const struct ovsrec_bridge *src, const char *src_name,
const struct ovsrec_bridge *dst, const char *dst_name,
struct shash *existing_ports)
@@ -82,7 +82,7 @@ create_patch_port(struct controller_ctx *ctx,
port = ovsrec_port_insert(ctx->ovs_idl_txn);
ovsrec_port_set_name(port, src_name);
ovsrec_port_set_interfaces(port, &iface, 1);
const struct smap ids = SMAP_CONST1(&ids, "ovn-localnet-port", network);
const struct smap ids = SMAP_CONST1(&ids, key, value);
ovsrec_port_set_external_ids(port, &ids);
struct ovsrec_port **ports;
@@ -97,21 +97,21 @@ create_patch_port(struct controller_ctx *ctx,
/* Creates a pair of patch ports that connect bridges 'b1' and 'b2', using a
* port named 'name1' and 'name2' in each respective bridge.
* external-ids:ovn-localnet-port in each port is initialized to 'network'.
* external-ids:'key' in each port is initialized to 'value'.
*
* If one or both of the ports already exists, leaves it there and removes it
* from 'existing_ports'. */
static void
create_patch_ports(struct controller_ctx *ctx,
const char *network,
const char *key, const char *value,
const struct ovsrec_bridge *b1,
const struct ovsrec_bridge *b2,
struct shash *existing_ports)
{
char *name1 = patch_port_name(b1->name, b2->name);
char *name2 = patch_port_name(b2->name, b1->name);
create_patch_port(ctx, network, b1, name1, b2, name2, existing_ports);
create_patch_port(ctx, network, b2, name2, b1, name1, existing_ports);
create_patch_port(ctx, key, value, b1, name1, b2, name2, existing_ports);
create_patch_port(ctx, key, value, b2, name2, b1, name1, existing_ports);
free(name2);
free(name1);
}
@@ -187,11 +187,59 @@ add_bridge_mappings(struct controller_ctx *ctx,
continue;
}
create_patch_ports(ctx, network, br_int, ovs_bridge, existing_ports);
create_patch_ports(ctx, "ovn-localnet-port", network,
br_int, ovs_bridge, existing_ports);
}
free(start);
}
/* Add one OVS patch port for each OVN logical patch port.
*
* This is suboptimal for several reasons. First, it creates an OVS port for
* every OVN logical patch port, not just for the ones that are actually useful
* on this hypervisor. Second, it's wasteful to create an OVS patch port per
* OVN logical patch port, when really there's no benefit to them beyond a way
* to identify how a packet ingressed into a logical datapath.
*
* There are two obvious ways to improve the situation here, by modifying
* OVS:
*
* 1. Add a way to configure in OVS which fields are preserved on a hop
* across an OVS patch port. If MFF_LOG_DATAPATH and MFF_LOG_INPORT
* were preserved, then only a single pair of OVS patch ports would be
* required regardless of the number of OVN logical patch ports.
*
* 2. Add a new OpenFlow extension action modeled on "resubmit" that also
* saves and restores the packet data and metadata (the inability to do
* this is the only reason that "resubmit" can't be used already). Or
* add OpenFlow extension actions to otherwise save and restore packet
* data and metadata.
*/
static void
add_logical_patch_ports(struct controller_ctx *ctx,
const struct ovsrec_bridge *br_int,
struct shash *existing_ports)
{
const struct sbrec_port_binding *binding;
SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
if (!strcmp(binding->type, "patch")) {
const char *local = binding->logical_port;
const char *peer = smap_get(&binding->options, "peer");
if (!peer) {
continue;
}
char *src_name = patch_port_name(local, peer);
char *dst_name = patch_port_name(peer, local);
create_patch_port(ctx, "ovn-logical-patch-port", local,
br_int, src_name, br_int, dst_name,
existing_ports);
free(dst_name);
free(src_name);
}
}
}
void
patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
{
@@ -203,7 +251,8 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
struct shash existing_ports = SHASH_INITIALIZER(&existing_ports);
const struct ovsrec_port *port;
OVSREC_PORT_FOR_EACH (port, ctx->ovs_idl) {
if (smap_get(&port->external_ids, "ovn-localnet-port")) {
if (smap_get(&port->external_ids, "ovn-localnet-port") ||
smap_get(&port->external_ids, "ovn-logical-patch-port")) {
shash_add(&existing_ports, port->name, port);
}
}
@@ -212,6 +261,7 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int)
* 'existing_ports' any patch ports that do exist in the database and
* should be there. */
add_bridge_mappings(ctx, br_int, &existing_ports);
add_logical_patch_ports(ctx, br_int, &existing_ports);
/* Now 'existing_ports' only still contains patch ports that exist in the
* database but shouldn't. Delete them from the database. */

14
ovn/controller/physical.c
View File

@@ -158,6 +158,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
const char *localnet = smap_get(&port_rec->external_ids,
"ovn-localnet-port");
const char *logpatch = smap_get(&port_rec->external_ids,
"ovn-logical-patch-port");
for (int j = 0; j < port_rec->n_interfaces; j++) {
const struct ovsrec_interface *iface_rec = port_rec->interfaces[j];
@@ -171,10 +173,16 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
continue;
}
/* Record as patch to local net, chassis, or local logical port. */
if (!strcmp(iface_rec->type, "patch") && localnet) {
/* Record as patch to local net, logical patch port, chassis, or
* local logical port. */
bool is_patch = !strcmp(iface_rec->type, "patch");
if (is_patch && localnet) {
simap_put(&localnet_to_ofport, localnet, ofport);
break;
} else if (is_patch && logpatch) {
/* Logical patch ports can be handled just like VIFs. */
simap_put(&localvif_to_ofport, logpatch, ofport);
break;
} else if (chassis_id) {
enum chassis_tunnel_type tunnel_type;
if (!strcmp(iface_rec->type, "geneve")) {
@@ -246,6 +254,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
* and accessible via a VLAN, 'tag' is the VLAN ID; otherwise
* 'tag' is 0.
*
* The same logic handles logical patch ports.
*
* - If the port is on a remote chassis, the OpenFlow port for a
* tunnel to the VIF's remote chassis. 'tun' identifies that
* tunnel.

12
ovn/ovn-architecture.7.xml
View File

@@ -796,7 +796,9 @@
<p>
Flows in table 33 resemble those in table 32 but for logical ports that
reside locally rather than remotely. For unicast logical output ports
reside locally rather than remotely. (This includes logical patch
ports, which do not have a physical location and effectively reside on
every hypervisor.) For unicast logical output ports
on the local hypervisor, the actions just resubmit to table 34. For
multicast output ports that include one or more logical ports on the
local hypervisor, for each such logical port <var>P</var>, the actions
@@ -837,6 +839,14 @@
is a container nested with a VM, then before sending the packet the
actions push on a VLAN header with an appropriate VLAN ID.
</p>
<p>
If the logical egress port is a logical patch port, then table 64
outputs to an OVS patch port that represents the logical patch port.
The packet re-enters the OpenFlow flow table from the OVS patch port's
peer in table 0, which identifies the logical datapath and logical
input port based on the OVS patch port's OpenFlow port number.
</p>
</li>
</ol>

29
ovn/ovn-sb.xml
View File

@@ -1092,8 +1092,9 @@ tcp.flags = RST;
<table name="Port_Binding" title="Physical-Logical Port Bindings">
<p>
Each row in this table identifies the physical location of a logical
port.
Most rows in this table identify the physical location of a logical port.
(The exceptions are logical patch ports, which do not have any physical
location.)
</p>
<p>
@@ -1180,6 +1181,14 @@ tcp.flags = RST;
<dl>
<dt>(empty string)</dt>
<dd>VM (or VIF) interface.</dd>
<dt><code>patch</code></dt>
<dd>
One of a pair of logical ports that act as if connected by a patch
cable. Useful for connecting two logical datapaths, e.g. to connect
a logical router to a logical switch or to another logical router.
</dd>
<dt><code>localnet</code></dt>
<dd>
A connection to a locally accessible network from each
@@ -1203,6 +1212,22 @@ tcp.flags = RST;
</column>
</group>
<group title="Patch Options">
<p>
These options apply to logical ports with <ref column="type"/> of
<code>patch</code>.
</p>
<column name="options" key="peer">
The <ref column="logical_port"/> in the <ref table="Port_Binding"/>
record for the other side of the patch. The named <ref
column="logical_port"/> must specify this <ref column="logical_port"/>
in its own <code>peer</code> option. That is, the two patch logical
ports must have reversed <ref column="logical_port"/> and
<code>peer</code> values.
</column>
</group>
<group title="Localnet Options">
<p>
These options apply to logical ports with <ref column="type"/> of

40
tests/ovn-controller.at
View File

@@ -64,8 +64,46 @@ check_patches \
'br-eth1 patch-br-eth1-to-br-int patch-br-int-to-br-eth1' \
'br-int patch-br-int-to-br-eth1 patch-br-eth1-to-br-int'
# Delete the mapping and the patch ports should go away.
# Add logical patch ports.
AT_CHECK([ovn-sbctl \
-- --id=@dp1 create Datapath_Binding tunnel_key=1 \
-- --id=@dp2 create Datapath_Binding tunnel_key=2 \
-- create Port_Binding datapath=@dp1 logical_port=foo tunnel_key=1 type=patch options:peer=bar \
-- create Port_Binding datapath=@dp2 logical_port=bar tunnel_key=2 type=patch options:peer=foo \
| ${PERL} $srcdir/uuidfilt.pl], [0], [<0>
<1>
<2>
<3>
])
check_patches \
'br-eth2 patch-br-eth2-to-br-int patch-br-int-to-br-eth2' \
'br-int patch-br-int-to-br-eth2 patch-br-eth2-to-br-int' \
'br-eth1 patch-br-eth1-to-br-int patch-br-int-to-br-eth1' \
'br-int patch-br-int-to-br-eth1 patch-br-eth1-to-br-int' \
'br-int patch-foo-to-bar patch-bar-to-foo' \
'br-int patch-bar-to-foo patch-foo-to-bar'
# Delete the mapping and the ovn-bridge-mapping patch ports should go away;
# the ones from the logical patch port remain.
AT_CHECK([ovs-vsctl remove Open_vSwitch . external-ids ovn-bridge-mappings])
check_patches \
'br-int patch-foo-to-bar patch-bar-to-foo' \
'br-int patch-bar-to-foo patch-foo-to-bar'
# Change name of logical patch port, check that the OVS patch ports
# get updated.
AT_CHECK([ovn-sbctl \
-- set Port_Binding foo logical_port=quux options:peer=baz \
-- set Port_Binding bar logical_port=baz options:peer=quux])
check_patches \
'br-int patch-quux-to-baz patch-baz-to-quux' \
'br-int patch-baz-to-quux patch-quux-to-baz'
# Change the logical patch ports to VIFs and verify that the OVS patch
# ports disappear.
AT_CHECK([ovn-sbctl \
-- set Port_Binding quux type='""' options='{}' \
-- set Port_Binding baz type='""' options='{}'])
check_patches
AT_CLEANUP