I find this quite a bit simpler to reason about. Also, future patches
require xlate_actions() not to modify the packet.
Signed-off-by: Ethan Jackson <et...@nicira.com>
---
ofproto/ofproto-dpif-upcall.c | 25 ++------------
ofproto/ofproto-dpif-xlate.c | 80 +++++++++++++++++++++++++------------------
ofproto/ofproto-dpif-xlate.h | 9 +++--
ofproto/ofproto-dpif.c | 6 ++--
tests/vlan-splinters.at | 4 +--
5 files changed, 58 insertions(+), 66 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 48393b0..b0fd203 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -858,7 +858,7 @@ convert_upcall(struct udpif *udpif, struct upcall *upcall)
odp_port_t odp_in_port;
int error;
- error = xlate_receive(udpif->backer, packet, dupcall->key,
+ error = xlate_receive(udpif->backer, dupcall->key,
dupcall->key_len, &flow,
&ofproto, &ipfix, &sflow, NULL, &odp_in_port);
@@ -967,25 +967,6 @@ handle_upcalls(struct udpif *udpif, struct upcall *upcalls,
fail_open = fail_open || upcall->xout.fail_open;
- if (upcall->flow.in_port.ofp_port
- != vsp_realdev_to_vlandev(upcall->ofproto,
- upcall->flow.in_port.ofp_port,
- upcall->flow.vlan_tci)) {
- /* This packet was received on a VLAN splinter port. We
- * added a VLAN to the packet to make the packet resemble
- * the flow, but the actions were composed assuming that
- * the packet contained no VLAN. So, we must remove the
- * VLAN header from the packet before trying to execute the
- * actions. */
- if (ofpbuf_size(upcall->xout.odp_actions)) {
- eth_pop_vlan(packet);
- }
-
- /* Remove the flow vlan tags inserted by vlan splinter logic
- * to ensure megaflow masks generated match the data path flow. */
- upcall->flow.vlan_tci = 0;
- }
-
/* Do not install a flow into the datapath if:
*
* - The datapath already has too many flows.
@@ -1257,7 +1238,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
goto exit;
}
- error = xlate_receive(udpif->backer, NULL, ukey->key, ukey->key_len, &flow,
+ error = xlate_receive(udpif->backer, ukey->key, ukey->key_len, &flow,
&ofproto, NULL, NULL, &netflow, &odp_in_port);
if (error) {
goto exit;
@@ -1385,7 +1366,7 @@ push_dump_ops__(struct udpif *udpif, struct dump_op *ops,
size_t n_ops)
}
ovs_mutex_unlock(&op->ukey->mutex);
- error = xlate_receive(udpif->backer, NULL, op->op.u.flow_del.key,
+ error = xlate_receive(udpif->backer, op->op.u.flow_del.key,
op->op.u.flow_del.key_len, &flow, &ofproto,
NULL, NULL, &netflow, NULL);
if (!error) {
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index c816135..f115849 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -934,23 +934,16 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
xlate_xport_remove(new_xcfg, xport);
}
-/* Given a datpath, packet, and flow metadata ('backer', 'packet', and 'key'
- * respectively), populates 'flow' with the result of odp_flow_key_to_flow().
- * Optionally populates 'ofproto' with the ofproto_dpif, 'odp_in_port' with
- * the datapath in_port, that 'packet' ingressed, and 'ipfix', 'sflow', and
- * 'netflow' with the appropriate handles for those protocols if they're
- * enabled. Caller is responsible for unrefing them.
+/* Given a datpath and flow metadata ('backer', and 'key' respectively),
+ * populates 'flow' with the result of odp_flow_key_to_flow(). Optionally
+ * populates 'ofproto' with the ofproto_dpif, 'odp_in_port' with the datapath
+ * in_port, and 'ipfix', 'sflow', and 'netflow' with the appropriate handles
+ * for those protocols if they're enabled. Caller is responsible for unrefing
+ * them.
*
* If 'ofproto' is nonnull, requires 'flow''s in_port to exist. Otherwise sets
* 'flow''s in_port to OFPP_NONE.
*
- * This function does post-processing on data returned from
- * odp_flow_key_to_flow() to help make VLAN splinters transparent to the rest
- * of the upcall processing logic. In particular, if the extracted in_port is
- * a VLAN splinter port, it replaces flow->in_port by the "real" port, sets
- * flow->vlan_tci correctly for the VLAN of the VLAN splinter port, and pushes
- * a VLAN header onto 'packet' (if it is nonnull).
- *
* Similarly, this function also includes some logic to help with tunnels. It
* may modify 'flow' as necessary to make the tunneling implementation
* transparent to the upcall processing logic.
@@ -958,19 +951,16 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
* Returns 0 if successful, ENODEV if the parsed flow has no associated ofport,
* or some other positive errno if there are other problems. */
int
-xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet,
- const struct nlattr *key, size_t key_len, struct flow *flow,
- struct ofproto_dpif **ofproto, struct dpif_ipfix **ipfix,
- struct dpif_sflow **sflow, struct netflow **netflow,
- odp_port_t *odp_in_port)
+xlate_receive(const struct dpif_backer *backer, const struct nlattr *key,
+ size_t key_len, struct flow *flow, struct ofproto_dpif **ofproto,
+ struct dpif_ipfix **ipfix, struct dpif_sflow **sflow,
+ struct netflow **netflow, odp_port_t *odp_in_port)
{
struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
- int error = ENODEV;
const struct xport *xport;
if (odp_flow_key_to_flow(key, key_len, flow) == ODP_FIT_ERROR) {
- error = EINVAL;
- return error;
+ return EINVAL;
}
if (odp_in_port) {
@@ -983,19 +973,8 @@ xlate_receive(const struct dpif_backer *backer, struct
ofpbuf *packet,
flow->in_port.ofp_port = xport ? xport->ofp_port : OFPP_NONE;
if (!xport) {
- return error;
- }
-
- if (vsp_adjust_flow(xport->xbridge->ofproto, flow)) {
- if (packet) {
- /* Make the packet resemble the flow, so that it gets sent to
- * an OpenFlow controller properly, so that it looks correct
- * for sFlow, and so that flow_extract() will get the correct
- * vlan_tci if it is called on 'packet'. */
- eth_push_vlan(packet, htons(ETH_TYPE_VLAN), flow->vlan_tci);
- }
+ return ENODEV;
}
- error = 0;
if (ofproto) {
*ofproto = xport->xbridge->ofproto;
@@ -1012,7 +991,7 @@ xlate_receive(const struct dpif_backer *backer, struct
ofpbuf *packet,
if (netflow) {
*netflow = netflow_ref(xport->xbridge->netflow);
}
- return error;
+ return 0;
}
static struct xbridge *
@@ -3918,6 +3897,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
struct flow orig_flow;
struct xlate_ctx ctx;
size_t ofpacts_len;
+ bool vsp_adjusted;
bool tnl_may_send;
bool is_icmp;
@@ -3995,6 +3975,27 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
ctx.use_recirc = false;
ctx.was_mpls = false;
+ vsp_adjusted = vsp_adjust_flow(ctx.xbridge->ofproto, flow);
+ if (vsp_adjusted) {
+ if (xin->report_hook) {
+ struct ds ds = DS_EMPTY_INITIALIZER;
+
+ ds_put_cstr(&ds, "Adjusted for VLAN splinters: ");
+ flow_format(&ds, flow);
+ xlate_report(&ctx, ds_cstr(&ds));
+ ds_destroy(&ds);
+ }
+
+ if (xin->packet) {
+ /* Make the packet resemble the flow, so that it gets sent to
+ * an OpenFlow controller properly, so that it looks correct
+ * for sFlow, and so that flow_extract() will get the correct
+ * vlan_tci if it is called on 'packet'. */
+ eth_push_vlan(CONST_CAST(struct ofpbuf *, xin->packet),
+ htons(ETH_TYPE_VLAN), flow->vlan_tci);
+ }
+ }
+
if (!xin->ofpacts && !ctx.rule) {
ctx.table_id = rule_dpif_lookup(ctx.xbridge->ofproto, flow,
!xin->skip_wildcards ? wc : NULL,
@@ -4187,6 +4188,17 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
wc->masks.tp_src &= htons(UINT8_MAX);
wc->masks.tp_dst &= htons(UINT8_MAX);
}
+
+ if (vsp_adjusted) {
+ /* This packet was received on a VLAN splinter port. We added a VLAN
+ * to the packet to make the packet resemble the flow, but the actions
+ * were composed assuming that the packet contained no VLAN. So, we
+ * must remove the VLAN header from the packet before trying to execute
+ * the actions. */
+ if (xin->packet) {
+ eth_pop_vlan(CONST_CAST(struct ofpbuf *, xin->packet));
+ }
+ }
}
/* Sends 'packet' out 'ofport'.
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 3f6807d..3522c0b 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -173,11 +173,10 @@ void xlate_ofport_set(struct ofproto_dpif *, struct
ofbundle *,
bool may_enable);
void xlate_ofport_remove(struct ofport_dpif *);
-int xlate_receive(const struct dpif_backer *, struct ofpbuf *packet,
- const struct nlattr *key, size_t key_len,
- struct flow *, struct ofproto_dpif **, struct dpif_ipfix **,
- struct dpif_sflow **, struct netflow **,
- odp_port_t *odp_in_port);
+int xlate_receive(const struct dpif_backer *, const struct nlattr *key,
+ size_t key_len, struct flow *, struct ofproto_dpif **,
+ struct dpif_ipfix **, struct dpif_sflow **,
+ struct netflow **, odp_port_t *odp_in_port);
void xlate_actions(struct xlate_in *, struct xlate_out *);
void xlate_in_init(struct xlate_in *, struct ofproto_dpif *,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 161b26a..c49f1fb 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4205,7 +4205,7 @@ parse_flow_and_packet(int argc, const char *argv[],
goto exit;
}
- if (xlate_receive(backer, NULL, ofpbuf_data(&odp_key),
+ if (xlate_receive(backer, ofpbuf_data(&odp_key),
ofpbuf_size(&odp_key), flow,
ofprotop, NULL, NULL, NULL, NULL)) {
error = "Invalid datapath flow";
@@ -4592,8 +4592,8 @@ ofproto_dpif_contains_flow(const struct ofproto_dpif
*ofproto,
struct ofproto_dpif *ofp;
struct flow flow;
- xlate_receive(ofproto->backer, NULL, key, key_len, &flow, &ofp,
- NULL, NULL, NULL, NULL);
+ xlate_receive(ofproto->backer, key, key_len, &flow, &ofp, NULL, NULL, NULL,
+ NULL);
return ofp == ofproto;
}
diff --git a/tests/vlan-splinters.at b/tests/vlan-splinters.at
index b38ab52..6406029 100644
--- a/tests/vlan-splinters.at
+++ b/tests/vlan-splinters.at
@@ -30,8 +30,8 @@ for args in '9 p2' '11 p3' '15 p4'; do
# treated as if it had been received on p1 in the correct VLAN.
AT_CHECK([ovs-appctl ofproto/trace ovs-dummy "in_port($splinter_port)"],
[0], [stdout])
- AT_CHECK_UNQUOTED([sed -n '/^Flow/p; /^Datapath/p' stdout], [0], [dnl
-Flow:
metadata=0,in_port=$p1,dl_vlan=$vlan,dl_vlan_pcp=0,dl_src=00:00:00:00:00:00,dl_dst=00:00:00:00:00:00,dl_type=0x05ff
+ AT_CHECK_UNQUOTED([sed -n '/^Adjusted/p; /^Datapath/p' stdout], [0], [dnl
+Adjusted for VLAN splinters:
metadata=0,in_port=$p1,dl_vlan=$vlan,dl_vlan_pcp=0,dl_src=00:00:00:00:00:00,dl_dst=00:00:00:00:00:00,dl_type=0x05ff
Datapath actions: $access_port
])
--
1.8.1.2
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
