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

Alex Wang alexw at nicira.com
Tue Jun 11 23:10:34 UTC 2013


On Tue, Jun 11, 2013 at 3:59 PM, Ben Pfaff <blp at nicira.com> wrote:

> 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,

This makes sense. But one thing is that if we compare to "ofp_port_t"
variables, e.g. "flow->in_port.ofp_port >= OFPP_MAX", we must call this
function for both operands. this seems to make coding harder, really want
to know what do you think?

Also want to ask for your comments on "ofp_is_less_than()" solution, I
provided in previous email.

Kind Regards,
Alex Wang,



> Thanks,
>
> Ben.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130611/da47bbc3/attachment-0003.html>


More information about the dev mailing list