From 625b07205a37dd8c7a8b53b2e53e5f0b55203b38 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sat, 5 May 2012 11:07:42 -0700 Subject: [PATCH] ofproto-dpif: Segregate CFM, LACP, and STP traffic into separate queues. Until now, packets for these special protocols have been mixed with general traffic in the kernel-to-userspace queues. This means that a big-enough storm of new flows in these queues can cause packets for these special protocols to be dropped at this interface, fooling userspace into believing that, say, no CFM packets have been received even though they are arriving at the expected rate. This commit moves special protocols to a dedicated kernel-to-userspace queue to avoid the problem. Bug #7550. Signed-off-by: Ben Pfaff --- lib/dpif-linux.c | 11 ++++++++--- lib/dpif-provider.h | 4 ++++ lib/dpif.c | 3 +++ ofproto/ofproto-dpif.c | 19 +++++++++++++++++-- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index 3b2fba3c..256c9d6c 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -61,8 +61,9 @@ VLOG_DEFINE_THIS_MODULE(dpif_linux); enum { MAX_PORTS = USHRT_MAX }; -enum { N_UPCALL_SOCKS = 16 }; -BUILD_ASSERT_DECL(IS_POW2(N_UPCALL_SOCKS)); +enum { N_UPCALL_SOCKS = 17 }; +BUILD_ASSERT_DECL(IS_POW2(N_UPCALL_SOCKS - 1)); +BUILD_ASSERT_DECL(N_UPCALL_SOCKS > 1); BUILD_ASSERT_DECL(N_UPCALL_SOCKS <= 32); /* We use a 32-bit word as a mask. */ /* This ethtool flag was introduced in Linux 2.6.24, so it might be @@ -460,7 +461,11 @@ dpif_linux_port_get_pid(const struct dpif *dpif_, uint16_t port_no) if (dpif->epoll_fd < 0) { return 0; } else { - int idx = port_no & (N_UPCALL_SOCKS - 1); + int idx; + + idx = (port_no != UINT16_MAX + ? 1 + (port_no & (N_UPCALL_SOCKS - 2)) + : 0); return nl_sock_pid(dpif->upcall_socks[idx]); } } diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index 594ce592..0641d306 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -136,6 +136,10 @@ struct dpif_class { * actions as the OVS_USERSPACE_ATTR_PID attribute's value, for use in * flows whose packets arrived on port 'port_no'. * + * A 'port_no' of UINT16_MAX should be treated as a special case. The + * implementation should return a reserved PID, not allocated to any port, + * that the client may use for special purposes. + * * The return value only needs to be meaningful when DPIF_UC_ACTION has * been enabled in the 'dpif''s listen mask, and it is allowed to change * when DPIF_UC_ACTION is disabled and then re-enabled. diff --git a/lib/dpif.c b/lib/dpif.c index 4d7e8b39..ce7c5801 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -541,6 +541,9 @@ dpif_get_max_ports(const struct dpif *dpif) * as the OVS_USERSPACE_ATTR_PID attribute's value, for use in flows whose * packets arrived on port 'port_no'. * + * A 'port_no' of UINT16_MAX is a special case: it returns a reserved PID, not + * allocated to any port, that the client may use for special purposes. + * * The return value is only meaningful when DPIF_UC_ACTION has been enabled in * the 'dpif''s listen mask. It is allowed to change when DPIF_UC_ACTION is * disabled and then re-enabled, so a client that does that must be prepared to diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index ff6dac18..094cbd00 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -3469,7 +3469,12 @@ expire_batch(struct ofproto_dpif *ofproto, struct subfacet **subfacets, int n) static void expire_subfacets(struct ofproto_dpif *ofproto, int dp_max_idle) { - long long int cutoff = time_msec() - dp_max_idle; + /* Cutoff time for most flows. */ + long long int normal_cutoff = time_msec() - dp_max_idle; + + /* We really want to keep flows for special protocols around, so use a more + * conservative cutoff. */ + long long int special_cutoff = time_msec() - 10000; struct subfacet *subfacet, *next_subfacet; struct subfacet *batch[EXPIRE_MAX_BATCH]; @@ -3478,6 +3483,11 @@ expire_subfacets(struct ofproto_dpif *ofproto, int dp_max_idle) n_batch = 0; HMAP_FOR_EACH_SAFE (subfacet, next_subfacet, hmap_node, &ofproto->subfacets) { + long long int cutoff; + + cutoff = (subfacet->slow & (SLOW_CFM | SLOW_LACP | SLOW_STP) + ? special_cutoff + : normal_cutoff); if (subfacet->used < cutoff) { if (subfacet->path != SF_NOT_INSTALLED) { batch[n_batch++] = subfacet; @@ -4714,7 +4724,12 @@ compose_slow_path(const struct ofproto_dpif *ofproto, const struct flow *flow, cookie.slow_path.reason = slow; ofpbuf_use_stack(&buf, stub, stub_size); - put_userspace_action(ofproto, &buf, flow, &cookie); + if (slow & (SLOW_CFM | SLOW_LACP | SLOW_STP)) { + uint32_t pid = dpif_port_get_pid(ofproto->dpif, UINT16_MAX); + odp_put_userspace_action(pid, &cookie, &buf); + } else { + put_userspace_action(ofproto, &buf, flow, &cookie); + } *actionsp = buf.data; *actions_lenp = buf.size; } -- 2.30.2