On Tue, May 31, 2016 at 08:20:24PM -0700, Ben Pfaff wrote:
> On Wed, May 04, 2016 at 04:34:25PM +0900, Simon Horman wrote:
> > Add support for layer 3 GRE vports (non-tap aka non-VTEP).
> >
> > This makes use of a vport mode configuration for the existing (tap/VTEP)
> > GRE vports.
> >
> > In order to differentiate packets for two different types of GRE vports a
> > new flow key attribute, OVS_KEY_ATTR_NEXT_BASE_LAYER, is used. It is
> > intended that this attribute is only used in userspace as there appears to
> > be no need for it to be used in the kernel datapath.
>
> Should the OVS_KEY_ATTR_NEXT_BASE_LAYER declaration in an #ifndef
> __KERNEL__ block or similar? At least a comment on the declaration
> would be helpful.
Good point, I think its reasonable to so something like this:
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -358,6 +358,9 @@ enum ovs_key_attr {
#ifdef __KERNEL__
/* Only used within kernel data path. */
OVS_KEY_ATTR_TUNNEL_INFO, /* struct ovs_tunnel_info */
+#else
+ /* Only used within user-space data path. */
+ OVS_KEY_ATTR_NEXT_BASE_LAYER, /* base layer of encapsulated packet */
#endif
__OVS_KEY_ATTR_MAX
};
> > It is envisaged that this attribute may be used for other encapsulation
> > protocols that support both layer3 and layer2 inner-packets.
> >
> > Signed-off-by: Simon Horman <simon.hor...@netronome.com>
>
> miniflow_extract() has some tabs that should be spaces for indentation.
Thanks, I'll fix that.
> There's a change to tnl_port_show() to "Skip ports with duplicate 'port'
> field". I don't understand this change. Can you explain it? (It's
> O(n**2) in the number of ports, too.)
The problem that this tries to address is that it is now possible
to have an l3 and non-l3 tunnel which share the same port and when
the ports are dumped this shows up as a duplicate.
For example in the tunnel-push-pop.at test updated by this patch the
following duplicate gre port appears without this portion of the change:
# ovs-appctl tnl/ports/show
Listening ports:
genev_sys_6081 (6081)
gre_sys (3)
gre_sys (3)
vxlan_sys_4789 (4789)
Perhaps rather than hiding this duplication it would be best just to
provide a bit more information to the users like this.
# ovs-appctl tnl/ports/show
Listening ports:
genev_sys_6081 (6081)
gre_sys (3)
gre_sys (3) (layer3)
vxlan_sys_4789 (4789)
Which can be achieved as follows without altering the O() of the code.
diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
@@ -354,7 +371,8 @@ tnl_port_show(struct unixctl_conn *conn, int argc OVS_UNUSED
,
}
LIST_FOR_EACH(p, node, &port_list) {
- ds_put_format(&ds, "%s (%"PRIu32")\n", p->dev_name, p->port);
+ ds_put_format(&ds, "%s (%"PRIu32")%s\n", p->dev_name, p->port,
+ p->is_layer3 ? " (layer3)" : "");
}
out:
> In vswitch.xml, there's a couple of typos: s/recieved/received/ and
> s/ethernet/Ethernet/.
Thanks, I'll fix that too.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
