All three patches look good to me. Guolin, please feel free to do additional review or testing.
Acked-by: Andy Zhou <[email protected]> On Fri, Jan 10, 2014 at 3:18 PM, Ben Pfaff <[email protected]> wrote: > handle_upcalls() always installed a flow for every packet, as long as > the datapath didn't already have too many flows, but there are cases where > we don't want to do this: > > - If we get multiple packets in a single microflow all in one batch > (perhaps due to GSO breaking up a large TCP packet for sending to > userspace, or for another reason), then we only need to install the > datapath flow once. > > - For a slow-pathed flow received via a slow-path action in the kernel, > we know that the kernel flow is already there (because otherwise it > would have been received as "no match" instead of an action), so > there is no benefit to reinstalling it. > > Noticed because a CFM slow-pathed flow was getting reinstalled every time > a CFM packet was received. > > Reported-by: Guolin Yang <[email protected]> > Signed-off-by: Ben Pfaff <[email protected]> > --- > ofproto/ofproto-dpif-upcall.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index dd24f5c..c3a1a03 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -1,4 +1,4 @@ > -/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc. > +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -208,6 +208,8 @@ struct flow_miss { > struct odputil_keybuf mask_buf; > > struct xlate_out xout; > + > + bool put; > }; > > static void upcall_destroy(struct upcall *); > @@ -971,6 +973,7 @@ handle_upcalls(struct handler *handler, struct list > *upcalls) > miss->stats.used = time_msec(); > miss->stats.tcp_flags = 0; > miss->odp_in_port = odp_in_port; > + miss->put = false; > > n_misses++; > } else { > @@ -1107,10 +1110,23 @@ handle_upcalls(struct handler *handler, struct > list *upcalls) > miss->flow.vlan_tci = 0; > } > > - if (may_put) { > + /* Do not install a flow into the datapath if: > + * > + * - The datapath already has too many flows. > + * > + * - An earlier iteration of this loop already put the same > flow. > + * > + * - Slow-pathed flows only: we received this packet via some > flow > + * installed in the kernel already. */ > + if (may_put > + && !miss->put > + && (!miss->xout.slow > + || upcall->dpif_upcall.type == DPIF_UC_MISS)) { > struct ofpbuf mask; > bool megaflow; > > + miss->put = true; > + > atomic_read(&enable_megaflows, &megaflow); > ofpbuf_use_stack(&mask, &miss->mask_buf, sizeof > miss->mask_buf); > if (megaflow) { > -- > 1.7.10.4 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev >
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
