Small typo on the first line of the new comment. Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
On Sep 19, 2013, at 10:59 AM, Ben Pfaff <b...@nicira.com> wrote: > On Tue, Sep 17, 2013 at 05:24:02PM -0700, Jarno Rajahalme wrote: >> I like this! Much clearer, especially when comparing to the upcall >> processing in the past monolithic ofproto-dpif. >> >> One small comment about a comment below, >> >> Jarno >> >> Acked-by: Jarno Rajahalme <jrajaha...@nicira.com> > > Thanks for the review, see below also. > >> On Sep 17, 2013, at 4:08 PM, Ben Pfaff <b...@nicira.com> wrote: >> >>> >>> - /* Process each element in the to-do list, constructing the set of >>> - * operations to batch. */ >>> + /* Now handle each packet individually: >>> + * >>> + * - In the common case, the packet is a member of a flow that >>> doesn't >>> + * need per-packet translation. We already did the translation in >>> the >>> + * previous loop, so reuse it. >>> + * >>> + * - Otherwise, we need to do another translation just for this >>> + * packet. >> >> I would find it a bit clearer, if the above comment was replaced with >> something like this: >> >> /* Now handle each packet in the order they were received: >> * >> * - In the common case each packet of a miss can share the same actions >> * >> * - Slow-pathed packets, however, need to be translated individually. (it >> would be nice to know why?) >> * > > How about this: > > /* Now handle the packets individuallys in order of arrival. In the common > * case each packet of a miss can share the same actions, but slow-pathed > * packets need to be translated individually: > * > * - For SLOW_CFM, SLOW_LACP, SLOW_STP, and SLOW_BFD, translation is what > * processes received packets for these protocols. > * > * - For SLOW_CONTROLLER, translation sends the packet to the OpenFlow > * controller. > * > * The loop fills 'ops' with an array of operations to execute in the > * datapath. */ _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
