On Wed, Aug 15, 2012 at 01:16:58PM -0400, Ed Maste wrote:
> >>> There may be some confusion about numbering.  OpenFlow says that the
> >>> port number of the "local port" is 65534 (OFPP_LOCAL).  The datapaths,
> >>> on the other hand, use port number 0 for the "local port".  There is
> >>> supposed to be translation going on at the interface between the
> >>> datapath and the OpenFlow implementation, but mistakes can creep in,
> >>> especially in new code.
> >>
> >> Indeed this is the case here - I compared the key for the packet being
> >> looked up and the otherwise-matching entry in the hash, and see 0 vs
> >> 65534 for the port.
> >>
> >> -Ed
> >
> > This fixes it for me, but I'm not certain this is the right place to
> > do the translation:
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 60fae5f..7112149 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -1011,7 +1011,7 @@ dp_netdev_port_input(struct dp_netdev *dp,
> > struct dp_netdev_port *port,
> >      if (packet->size < ETH_HEADER_LEN) {
> >          return;
> >      }
> > -    flow_extract(packet, 0, 0, port->port_no, &key);
> > +    flow_extract(packet, 0, 0, ofp_port_to_odp_port(port->port_no), &key);
> >      flow = dp_netdev_lookup_flow(dp, &key);
> >      if (flow) {
> >          dp_netdev_flow_used(flow, &key, packet);
> 
> Oops, I had it backwards in the version I posted - thanks Giuseppe for
> pointing it out off-list.
> 
> This is the version that I tested and pushed to my threaded patch
> repo, and Giuseppe confirmed it fixes it for him as well.  It looks
> like this bug affects the userspace datapath in general and isn't
> specific to the threaded patch or the FreeBSD netdev implementation.
> 
> +++ b/lib/dpif-netdev.c
> @@ -1011,7 +1011,7 @@ dp_netdev_port_input(struct dp_netdev *dp,
> struct dp_netdev_port *port,
>      if (packet->size < ETH_HEADER_LEN) {
>          return;
>      }
> -    flow_extract(packet, 0, 0, port->port_no, &key);
> +    flow_extract(packet, 0, 0, odp_port_to_ofp_port(port->port_no), &key);
>      flow = dp_netdev_lookup_flow(dp, &key);
>      if (flow) {
>          dp_netdev_flow_used(flow, &key, packet);

I agree that this is correct.

If you will send me a signed-off version, I'll apply the fix.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to