From 33158a18daadcb4c7eaddb226fcfbdcca54539b7 Mon Sep 17 00:00:00 2001 From: Justin Pettit Date: Tue, 3 Jan 2012 13:34:20 -0800 Subject: [PATCH] ofproto-dpif: Fake-up OFPP_NONE input bundle for mirroring and normal. 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 Signed-off-by: Justin Pettit --- ofproto/ofproto-dpif.c | 32 ++++++++++++++++++++++++++++++-- tests/ofproto-dpif.at | 30 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 1f14d12c..baa191e7 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. */ +static 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); @@ -4858,6 +4868,11 @@ input_vid_to_vlan(const struct ofbundle *in_bundle, uint16_t vid) static bool input_vid_is_valid(uint16_t vid, struct ofbundle *in_bundle, bool warn) { + /* Allow any VID on the OFPP_NONE port. */ + if (in_bundle == &ofpp_none_bundle) { + return true; + } + switch (in_bundle->vlan_mode) { case PORT_VLAN_ACCESS: if (vid) { @@ -5171,6 +5186,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; } @@ -5206,6 +5226,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) { @@ -5299,7 +5325,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. */ @@ -5333,7 +5360,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 55d1500d..3ce10293 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( -- 2.30.2