Both mirroring and "normal" processing make use of the input bundle to
perform various sanity checks. Controller-generated traffic typically
uses an ingress port of OFPP_NONE, which doesn't have a corresponding
input bundle. This commit fakes one up well enough that mirroring and
"normal" processing succeed.
We looked at creating an actual bundle based on the "real" OFPP_NONE.
This was even uglier, since there were even more special-cases that
needed to be handled, including having to hide it from port queries.
Reported-by: Jesse Gross <je...@nicira.com>
Signed-off-by: Justin Pettit <jpet...@nicira.com>
---
ofproto/ofproto-dpif.c | 27 +++++++++++++++++++++++++--
tests/ofproto-dpif.at | 30 ++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 84eab05..713d935 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -179,6 +179,16 @@ static void bundle_wait(struct ofbundle *);
static struct ofbundle *lookup_input_bundle(struct ofproto_dpif *,
uint16_t in_port, bool warn);
+/* A controller may use OFPP_NONE as the ingress port to indicate that
+ * it did not arrive on a "real" port. 'ofpp_none_bundle' exists for
+ * when an input bundle is needed for validation (e.g., mirroring or
+ * OFPP_NORMAL processing). It is not connected to an 'ofproto' or have
+ * any 'port' structs, so care must be taken when dealing with it. */
+struct ofbundle ofpp_none_bundle = {
+ .name = "OFPP_NONE",
+ .vlan_mode = PORT_VLAN_TRUNK
+};
+
static void stp_run(struct ofproto_dpif *ofproto);
static void stp_wait(struct ofproto_dpif *ofproto);
@@ -5166,6 +5176,11 @@ update_learning_table(struct ofproto_dpif *ofproto,
{
struct mac_entry *mac;
+ /* Don't learn the OFPP_NONE port. */
+ if (in_bundle == &ofpp_none_bundle) {
+ return;
+ }
+
if (!mac_learning_may_learn(ofproto->ml, flow->dl_src, vlan)) {
return;
}
@@ -5201,6 +5216,12 @@ lookup_input_bundle(struct ofproto_dpif *ofproto,
uint16_t in_port, bool warn)
{
struct ofport_dpif *ofport;
+ /* Special-case OFPP_NONE, which a controller may use as the ingress
+ * port for traffic that it is sourcing. */
+ if (in_port == OFPP_NONE) {
+ return &ofpp_none_bundle;
+ }
+
/* Find the port and bundle for the received packet. */
ofport = get_ofp_port(ofproto, in_port);
if (ofport && ofport->bundle) {
@@ -5294,7 +5315,8 @@ xlate_normal(struct action_xlate_ctx *ctx)
return;
}
- /* We know 'in_port' exists, since lookup_input_bundle() succeeded. */
+ /* We know 'in_port' exists unless it is "ofpp_none_bundle",
+ * since lookup_input_bundle() succeeded. */
in_port = get_ofp_port(ctx->ofproto, ctx->flow.in_port);
/* Drop malformed frames. */
@@ -5328,7 +5350,8 @@ xlate_normal(struct action_xlate_ctx *ctx)
vlan = input_vid_to_vlan(in_bundle, vid);
/* Check other admissibility requirements. */
- if (!is_admissible(ctx->ofproto, &ctx->flow, in_port, vlan, &ctx->tags)) {
+ if (in_port &&
+ !is_admissible(ctx->ofproto, &ctx->flow, in_port, vlan, &ctx->tags)) {
return;
}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 55d1500..3ce1029 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -460,6 +460,36 @@ AT_CHECK_UNQUOTED([tail -1 stdout], [0],
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - mirroring, OFPP_NONE ingress port])
+OVS_VSWITCHD_START(
+ [add-port br0 p1 -- set Interface p1 type=dummy --\
+ add-port br0 p2 -- set Interface p2 type=dummy --\
+ set Bridge br0 mirrors=@m --\
+ --id=@p2 get Port p2 --\
+ --id=@m create Mirror name=mymirror \
+ select_all=true output_port=@p2], [<0>
+])
+
+AT_CHECK(
+ [ovs-vsctl \
+ -- get Interface p1 ofport \
+ -- get Interface p2 ofport],
+ [0], [stdout])
+set `cat stdout`
+p1=$1 p2=$2
+
+AT_CHECK([ovs-ofctl add-flow br0 action=output:1])
+
+# "in_port" defaults to OFPP_NONE if it's not specified.
+flow="eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)"
+AT_CHECK([ovs-appctl ofproto/trace br0 "$flow"], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0],
+ [Datapath actions: $p1,$p2
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - mirroring, select_dst])
OVS_VSWITCHD_START(
--
1.7.4.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
