From 4a7ab326dccec8df3494833420433051e2b56be5 Mon Sep 17 00:00:00 2001 From: Daniele Di Proietto Date: Thu, 25 Aug 2016 09:48:56 -0700 Subject: [PATCH] ofproto-dpif-xlate: Adjust generated mask for fragments. It's possible to install an OpenFlow flow that matches on udp source and destination ports without matching on fragments. If the subtable where such flow stays is visited during translation of a later fragment, the generated mask will have incorrect prerequisited for the datapath and it would be revalidated away at the first chance. This commit fixes it by adjusting the mask for later fragments after translation. Other prerequisites of the mask are also prerequisites in OpenFlow, but not the ip fragment bit, that's why we need a special case here. For completeness, this commits also fixes a related problem in bfd, where we check the udp destination port without checking if the frame is an ip fragment. It's not really necessary to address this separately, given the adjustment that we perform. VMware-BZ: #1651589 Signed-off-by: Daniele Di Proietto Acked-by: Jarno Rajahalme --- lib/bfd.c | 2 +- ofproto/ofproto-dpif-xlate.c | 11 +++++++++++ tests/ofproto-dpif.at | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/lib/bfd.c b/lib/bfd.c index fcb6f6404..87f3322ee 100644 --- a/lib/bfd.c +++ b/lib/bfd.c @@ -658,7 +658,7 @@ bfd_should_process_flow(const struct bfd *bfd_, const struct flow *flow, if (flow->dl_type == htons(ETH_TYPE_IP)) { memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); - if (flow->nw_proto == IPPROTO_UDP) { + if (flow->nw_proto == IPPROTO_UDP && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) { memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst); if (flow->tp_dst == htons(BFD_DEST_PORT)) { bool check_tnl_key; diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 71ffca0e8..4c287124f 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -5298,6 +5298,17 @@ xlate_wc_finish(struct xlate_ctx *ctx) if (ctx->wc->masks.vlan_tci) { ctx->wc->masks.vlan_tci |= htons(VLAN_CFI); } + + /* The classifier might return masks that match on tp_src and tp_dst even + * for later fragments. This happens because there might be flows that + * match on tp_src or tp_dst without matching on the frag bits, because + * it is not a prerequisite for OpenFlow. Since it is a prerequisite for + * datapath flows and since tp_src and tp_dst are always going to be 0, + * wildcard the fields here. */ + if (ctx->xin->flow.nw_frag & FLOW_NW_FRAG_LATER) { + ctx->wc->masks.tp_src = 0; + ctx->wc->masks.tp_dst = 0; + } } /* Translates the flow, actions, or rule in 'xin' into datapath actions in diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index de57efd9f..e2b983f69 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -8961,3 +8961,38 @@ AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1400]) OVS_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([ofproto - fragment prerequisites]) +OVS_VSWITCHD_START + +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) + +add_of_ports br0 1 + +AT_DATA([flows.txt], [dnl +priority=10,in_port=1,udp,tp_src=67,tp_dst=68,action=drop +priority=1,in_port=1,udp,action=drop +]) + +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) + +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:max-idle=10000]) + +ovs-appctl time/stop +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'recirc_id(0),in_port(1),eth_type(0x0800),ipv4(proto=17,frag=later)']) +ovs-appctl time/warp 5000 + +AT_CHECK([strip_ufid < ovs-vswitchd.log | filter_flow_install | strip_used], [0], [dnl +recirc_id(0),in_port(1),eth_type(0x0800),ipv4(proto=17,frag=later), actions:drop +]) + +dnl Change the flow table. This will trigger revalidation of all the flows. +AT_CHECK([ovs-ofctl add-flow br0 priority=5,in_port=1,action=drop]) +AT_CHECK([ovs-appctl revalidator/wait], [0]) + +dnl We don't want revalidators to delete any flow. If the flow has been +dnl deleted it means that there's some inconsistency with the revalidation. +AT_CHECK([grep flow_del ovs-vswitchd.log], [1]) + +OVS_VSWITCHD_STOP +AT_CLEANUP -- 2.39.2