When vlan-splinter is enabled, ovs receives non-vlan flows from the kernel vlan ports, vlan tag is then added to the incoming flow before xlating, so that they look like those received from a trunk port.
In case megaflow is enabled, xlating may set vlan masks during rule processing as usual. If those vlan masks were serialized and downloaded to the kernel (this bug), those mega flows will be rejected due to unexpected vlan mask encapsulation, since the original kernel flows do not have vlan tags. This bug does not break connectivity, but impacts performance since all traffic received on vlan splinter ports will now be handled by vswitchd, as no datapath flows can be successfully installed. This fix is to make sure no vlan mask encapsulation is generated for the datapath flow if its in_port was re-written by vlan-splinter receiving logic. Bug #22567 Signed-off-by: Andy Zhou <[email protected]> --- v1->v2: format fixes. drop compose_output_action__() changes avoid calling eth_pop_vlan() if action.size is zero v2->v3: Save and restore miss flow's vlan_tci in case multiple upcalls share the same miss flow. v3->v4: use type ovs_be16 to save flow's vlan_tci instead of type int. --- ofproto/ofproto-dpif-upcall.c | 57 +++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index b57afdc..d0de27c 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -1076,9 +1076,12 @@ handle_upcalls(struct handler *handler, struct list *upcalls) LIST_FOR_EACH (upcall, list_node, upcalls) { struct flow_miss *miss = upcall->flow_miss; struct ofpbuf *packet = &upcall->dpif_upcall.packet; - struct ofpbuf mask; struct dpif_op *op; - bool megaflow; + ovs_be16 flow_vlan_tci; + + /* Save a copy of flow.vlan_tci in case it is changed to + * generate proper mega flow masks for VLAN splinter flows. */ + flow_vlan_tci = miss->flow.vlan_tci; if (miss->xout.slow) { struct xlate_in xin; @@ -1087,14 +1090,36 @@ handle_upcalls(struct handler *handler, struct list *upcalls) xlate_actions_for_side_effects(&xin); } - atomic_read(&enable_megaflows, &megaflow); - ofpbuf_use_stack(&mask, &miss->mask_buf, sizeof miss->mask_buf); - if (megaflow) { - odp_flow_key_from_mask(&mask, &miss->xout.wc.masks, &miss->flow, - UINT32_MAX); + if (miss->flow.in_port.ofp_port + != vsp_realdev_to_vlandev(miss->ofproto, + miss->flow.in_port.ofp_port, + miss->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 (miss->xout.odp_actions.size) { + eth_pop_vlan(packet); + } + + /* Remove the flow vlan tags inserted by vlan splinter logic + * to ensure megaflow masks generated match the data path flow. */ + miss->flow.vlan_tci = 0; } if (may_put) { + struct ofpbuf mask; + bool megaflow; + + atomic_read(&enable_megaflows, &megaflow); + ofpbuf_use_stack(&mask, &miss->mask_buf, sizeof miss->mask_buf); + if (megaflow) { + odp_flow_key_from_mask(&mask, &miss->xout.wc.masks, + &miss->flow, UINT32_MAX); + } + op = &ops[n_ops++]; op->type = DPIF_OP_FLOW_PUT; op->u.flow_put.flags = DPIF_FP_CREATE | DPIF_FP_MODIFY; @@ -1118,19 +1143,13 @@ handle_upcalls(struct handler *handler, struct list *upcalls) } } + /* + * The 'miss' may be shared by multiple upcalls. Restore + * the saved flow vlan_tci field before processing the next + * upcall. */ + miss->flow.vlan_tci = flow_vlan_tci; + if (miss->xout.odp_actions.size) { - if (miss->flow.in_port.ofp_port - != vsp_realdev_to_vlandev(miss->ofproto, - miss->flow.in_port.ofp_port, - miss->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. */ - eth_pop_vlan(packet); - } op = &ops[n_ops++]; op->type = DPIF_OP_EXECUTE; -- 1.7.9.5 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
