On Tue, Sep 24, 2013 at 04:45:14PM -0700, Ethan Jackson wrote:
> Both the IPFIX and SFLOW modules are thread safe, so there's no
> particular reason to pass them up to the main thread.  Eliminating
> this step significantly simplifies the code.
> 
> Signed-off-by: Ethan Jackson <et...@nicira.com>

The comments on enum upcall_type are wrong, since none of the upcalls
still require main thread involvement.

The comments on udpif_dispatcher() and udpif_upcall_handler() are
inaccurate now.

Some comments on pre-existing code:

    * I spotted another misspelling of "receiving" (as "receving").

    * I think that the VLOG_WARN in recv_upcalls should be rate-limited.
      If it happens once it'll happen a lot.

    * Part of this code:

            if (type == OVS_KEY_ATTR_IN_PORT
                || type == OVS_KEY_ATTR_TCP
                || type == OVS_KEY_ATTR_UDP) {
                if (nl_attr_get_size(nla) == 4) {
                    ovs_be32 attr = nl_attr_get_be32(nla);
                    hash = mhash_add(hash, (OVS_FORCE uint32_t) attr);

      could be simplified:

                    hash = mhash_add(hash, nl_attr_get_u32(nla));

      It's "incorrect" for TCP and UDP but the pre-existing code is
      "incorrect" in the same way for in_port.

In ofproto/ofproto-dpif-xlate.h, I would put a blank line before the
#endif ending the file.  Also the space after " *" below is not our
usual style:
    struct dpif_sflow * xlate_get_sflow(const struct ofproto_dpif *)

Acked-by: Ben Pfaff <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to