On Mon, Oct 1, 2018 at 12:49 PM Ben Pfaff <b...@ovn.org> wrote: > > On Fri, Sep 28, 2018 at 05:57:28PM -0700, Han Zhou wrote: > > Justin, please find the ofproto/trace result with the new wildcards here: > > https://gist.github.com/hzhou8/ff719b6565b9264304251054ce447b5d > > I also tried using different ports, but result is same - no wildcard > > happening for the dst-port field. > > I don't actually see tp_dst mentioned on any of the "new wildcards" > lines there. That is weird.
Hi Ben, I figured out the issue, and sent out a patch. But it seems the mailing list has been failing since the weekend (and still not working right now). I am pasting my earlier replies here. (I also sent a pull request since ml doesn't work: https://github.com/openvswitch/ovs/pull/254) ============================== Checking the xlate code, the problem is that bit-wise un-wildcarding happens only for general rule classification. For BFD, which is handled in process_special() -> bfd_should_process_flow(), it is set to full mask for the whole tp_dst field directly in the code. I submitted a patch to fix this by un-wildcarding only the necessary bits (which didn't yet show up on mailing-list after quite a while). Since it is small, I paste here: ------8><--------------------------------------------------------------------------------------------><8-------- Subject: [PATCH] bfd: Make the tp_dst masking megaflow-friendly. From: Han Zhou <hzh...@ebay.com> When there are tunnel ports with BFD enabled, all UDP flows will have dst port as match condition in datapath, which causes unnecessarily high flow miss for all UDP traffic, and results in latency increase. For more details, see [1]. This patch solves the problem by masking tp_dst only for the leading bits that is enough to tell the mismatch when it is not BFD traffic. [1] https://mail.openvswitch.org/pipermail/ovs-discuss/2018-September/047360.html Signed-off-by: Han Zhou <hzh...@ebay.com> --- lib/bfd.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/bfd.c b/lib/bfd.c index 5308262..1697d02 100644 --- a/lib/bfd.c +++ b/lib/bfd.c @@ -656,6 +656,19 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p, ovs_mutex_unlock(&mutex); } +static void +unwildcard_tp_dst(const struct flow *flow, struct flow_wildcards *wc) +{ + uint64_t diff = BFD_DEST_PORT ^ ntohs(flow->tp_dst); + if (diff) { + unsigned int eqbits = raw_clz64(diff << 48 | UINT64_C(1) << 47); + /* Set mask including the first mismatching bit. */ + wc->masks.tp_dst |= htons((uint16_t)~0u << (16 - eqbits -1)); + } else { + memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst); + } +} + bool bfd_should_process_flow(const struct bfd *bfd_, const struct flow *flow, struct flow_wildcards *wc) @@ -673,7 +686,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 && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) { - memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst); + unwildcard_tp_dst(flow, wc); if (flow->tp_dst == htons(BFD_DEST_PORT)) { bool check_tnl_key; -- 2.1.0 ------8><--------------------------------------------------------------------------------------------><8-------- With this patch the problem is solved - UDP dst ports are now getting wildcarded. E.g.: recirc_id(0),in_port(7),eth(src=22:49:4e:1e:5b:63,dst=aa:aa:aa:cc:cc:01),eth_type(0x0800),ipv4(src= 192.168.64.0/255.255.192.0,dst=10.0.0.11,proto=17,tos=0/0x3,ttl=63,frag=no),udp(dst=3788/0xfffc), packets:3, bytes:126, used:9.612s, actions:ct_clear,ct_clear,set(tunnel(tun_id=0x30001000001,dst=10.169.153.7,ttl=64,tp_dst=7471,flags(df|csum|key))),set(eth(src=aa:aa:aa:bb:bb:01,dst=aa:aa:aa:aa:aa:01)),set(ipv4(src= 192.168.64.0/255.255.192.0,dst=10.0.0.11,ttl=62)),1 recirc_id(0),in_port(7),eth(src=22:49:4e:1e:5b:63,dst=aa:aa:aa:cc:cc:01),eth_type(0x0800),ipv4(src= 192.168.64.0/255.255.192.0,dst=10.0.0.11,proto=17,tos=0/0x3,ttl=63,frag=no),udp(dst=3792/0xfff0), packets:8, bytes:336, used:0.612s, actions:ct_clear,ct_clear,set(tunnel(tun_id=0x30001000001,dst=10.169.153.7,ttl=64,tp_dst=7471,flags(df|csum|key))),set(eth(src=aa:aa:aa:bb:bb:01,dst=aa:aa:aa:aa:aa:01)),set(ipv4(src= 192.168.64.0/255.255.192.0,dst=10.0.0.11,ttl=62)),1 recirc_id(0),in_port(7),eth(src=22:49:4e:1e:5b:63,dst=aa:aa:aa:cc:cc:01),eth_type(0x0800),ipv4(src= 192.168.64.0/255.255.192.0,dst=10.0.0.11,proto=17,tos=0/0x3,ttl=63,frag=no),udp(dst=4096/0xf000), packets:3, bytes:126, used:0.476s, actions:ct_clear,ct_clear,set(tunnel(tun_id=0x30001000001,dst=10.169.153.7,ttl=64,tp_dst=7471,flags(df|csum|key))),set(eth(src=aa:aa:aa:bb:bb:01,dst=aa:aa:aa:aa:aa:01)),set(ipv4(src= 192.168.64.0/255.255.192.0,dst=10.0.0.11,ttl=62)),1 Please let me know if it is a reasonable fix. Thanks, Han
_______________________________________________ discuss mailing list disc...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
