Pushed to master, branch-1.7.
On Wed, May 09, 2012 at 12:30:04PM -0700, Ben Pfaff wrote: > Thanks. > > I'll push this series soon, let me know quickly if you want another > look beforehand. > > On Tue, May 08, 2012 at 05:07:35PM -0700, Ethan Jackson wrote: > > Looks good, thanks. > > > > Ethan > > > > On Sat, May 5, 2012 at 11:10 AM, Ben Pfaff <[email protected]> wrote: > > > 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 <[email protected]> > > > --- > > > 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 3b2fba3..256c9d6 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 594ce59..0641d30 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 4d7e8b3..ce7c580 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 486630c..97d3426 100644 > > > --- a/ofproto/ofproto-dpif.c > > > +++ b/ofproto/ofproto-dpif.c > > > @@ -3474,7 +3474,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]; > > > @@ -3483,6 +3488,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; > > > @@ -4719,7 +4729,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; > > > } > > > -- > > > 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
