[ovs-dev] [PATCH V2] feature: Create specific types for ofp and odp port

Ben Pfaff blp at nicira.com
Tue Jun 11 22:59:24 UTC 2013


On Fri, Jun 07, 2013 at 11:11:43AM -0700, Alex Wang wrote:
> Currently datapath ports and openflow ports are both represented by unsigned
> integers of various sizes. With implicit casts, etc. it is easy to mix them
> up and use one where the other is expected. This commit creates two typedefs
> ofp_port_t and odp_port_t. Both of these two types are marked by
> "__attribute__((bitwise))" so that Sparse can be used to detect any misuse.
> 
> Signed-off-by: Alex Wang <alexw at nicira.com>

I am really glad to see this implemented.  Thank you.

> REMAINING ISSUE
> 
> When doing comparisions like '<', '<=', etc, both ofp_port_t and odp_port_t
> will be converted to int, even though the L- and R- operands have same type.
> This seems to be caused by "usual arithmetic conversion" and I have not found
> a satisfying solution yet.
> 
> As a workaround, I defined a macro function "PORT_COMPARE()" in "lib/flow.h",
> which uses OVS_FORCE to cast both operands to uint32_t and conducts the
> comparision. It also makes it easy to check all such comparisons. Just
> grep "PORT_COMPARE".

How about, instead, implementing a pair of functions something like
this in appropriate headers:

        static inline uint32_t
        odp_to_u32(odp_port_t odp_port)
        {
            return (OVS_FORCE uint32_t) odp_port;
        }

        static inline uint16_t
        ofp_to_u16(ofp_port_t ofp_port)
        {
            return (OVS_FORCE uint16_t) ofp_port:
        }

Then comparisons can be written inline in a straightforward way,
e.g. in dpif_netdev_flow_from_nlattrs() in dpif-netdev.c:
    if (odp_to_u32(flow->in_port.odp_port) >= MAX_PORTS) {

Does that make sense?

Thanks,

Ben.



More information about the dev mailing list