I'm not super familiar with the VLAN splinter code at the moment, but the changes here look reasonable to me.
--Justin On Dec 27, 2011, at 3:01 PM, Ben Pfaff wrote: > Bug #8671. > Signed-off-by: Ben Pfaff <[email protected]> > --- > ofproto/ofproto-dpif.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--- > 1 files changed, 48 insertions(+), 4 deletions(-) > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index bce2c87..d7683be 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -2579,6 +2579,15 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct > flow_miss *miss, > struct flow_miss_op *op = &ops[(*n_ops)++]; > struct dpif_execute *execute = &op->dpif_op.execute; > > + if (flow->vlan_tci != subfacet->initial_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. */ > + eth_pop_vlan(packet); > + } > + > op->subfacet = subfacet; > execute->type = DPIF_OP_EXECUTE; > execute->key = miss->key; > @@ -2607,10 +2616,27 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct > flow_miss *miss, > } > } > > +/* Like odp_flow_key_to_flow(), this function converts the 'key_len' bytes of > + * OVS_KEY_ATTR_* attributes in 'key' to a flow structure in 'flow' and > returns > + * an ODP_FIT_* value that indicates how well 'key' fits our expectations for > + * what a flow key should contain. > + * > + * This function also includes some logic 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). > + * > + * Sets '*initial_tci' to the VLAN TCI with which the packet was really > + * received, that is, the actual VLAN TCI extracted by > odp_flow_key_to_flow(). > + * (This differs from the value returned in flow->vlan_tci only for packets > + * received on VLAN splinters.) > + */ > static enum odp_key_fitness > ofproto_dpif_extract_flow_key(const struct ofproto_dpif *ofproto, > const struct nlattr *key, size_t key_len, > - struct flow *flow, ovs_be16 *initial_tci) > + struct flow *flow, ovs_be16 *initial_tci, > + struct ofpbuf *packet) > { > enum odp_key_fitness fitness; > uint16_t realdev; > @@ -2628,6 +2654,23 @@ ofproto_dpif_extract_flow_key(const struct > ofproto_dpif *ofproto, > * with the VLAN device's VLAN ID. */ > flow->in_port = realdev; > flow->vlan_tci = htons((vid & VLAN_VID_MASK) | VLAN_CFI); > + 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'. > + * > + * The allocated space inside 'packet' probably also contains > + * 'key', that is, both 'packet' and 'key' are probably part of a > + * struct dpif_upcall (see the large comment on that structure > + * definition), so pushing data on 'packet' is in general not a > + * good idea since it could overwrite 'key' or free it as a side > + * effect. However, it's OK in this special case because we know > + * that 'packet' is inside a Netlink attribute: pushing 4 bytes > + * will just overwrite the 4-byte "struct nlattr", which is fine > + * since we don't need that header anymore. */ > + eth_push_vlan(packet, flow->vlan_tci); > + } > > /* Let the caller know that we can't reproduce 'key' from 'flow'. */ > if (fitness == ODP_FIT_PERFECT) { > @@ -2670,7 +2713,8 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto, > struct dpif_upcall *upcalls, > * then set 'flow''s header pointers. */ > fitness = ofproto_dpif_extract_flow_key(ofproto, > upcall->key, upcall->key_len, > - &flow, &initial_tci); > + &flow, &initial_tci, > + upcall->packet); > if (fitness == ODP_FIT_ERROR) { > ofpbuf_delete(upcall->packet); > continue; > @@ -2749,7 +2793,7 @@ handle_userspace_upcall(struct ofproto_dpif *ofproto, > > fitness = ofproto_dpif_extract_flow_key(ofproto, upcall->key, > upcall->key_len, &flow, > - &initial_tci); > + &initial_tci, upcall->packet); > if (fitness == ODP_FIT_ERROR) { > ofpbuf_delete(upcall->packet); > return; > @@ -5720,7 +5764,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int > argc, const char *argv[], > /* Convert odp_key to flow. */ > error = ofproto_dpif_extract_flow_key(ofproto, odp_key.data, > odp_key.size, &flow, > - &initial_tci); > + &initial_tci, NULL); > if (error == ODP_FIT_ERROR) { > unixctl_command_reply(conn, 501, "Invalid flow"); > goto exit; > -- > 1.7.2.5 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
