On Tue, Jun 11, 2013 at 3:59 PM, Ben Pfaff <b...@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 <al...@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.
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
