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

Ben Pfaff blp at nicira.com
Tue Oct 1 04:45:25 UTC 2013


On Tue, Oct 01, 2013 at 01:23:20PM +0900, Simon Horman wrote:
> 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

I don't understand that: no __be32 is involved in the code I suggested.
Maybe you accidentally tried nl_attr_get_be32(nla) instead of
nl_attr_get_u32(nla)?



More information about the dev mailing list