[ovs-dev] [PATCH v3] ofproto-dpif: Move special upcall handling into ofproto-dpif-upcall.

Simon Horman horms at verge.net.au
Tue Oct 1 04:23:20 UTC 2013


On Wed, Sep 25, 2013 at 08:54:31AM -0700, Ben Pfaff wrote:
> 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 <ethan at 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));

Sparse seems unhappy about that:

# MAKE=make make C=2 CF="-D__CHECK_ENDIAN__ -Wsparse-all" all
ofproto/ofproto-dpif-upcall.c:518:60: warning: incorrect type in argument 2 (different base types)
ofproto/ofproto-dpif-upcall.c:518:60:    expected unsigned int [unsigned] [usertype] data
ofproto/ofproto-dpif-upcall.c:518:60:    got restricted __be32

The sparse version I have is the following checkout from
git://git.kernel.org/pub/scm/devel/sparse/sparse.git.
I believe that you guys were using this version at some point.

30cb3a104d4fb8b70409e8f1e8a0a08363236f58
("Get rid of gcc warning about enum values")

> 
>       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 <blp at nicira.com>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
> 



More information about the dev mailing list