[ovs-dev] ofport_open and alloc_ofp_port

[Ben Pfaff]

On Thu, Nov 20, 2014 at 12:38:47PM -0800, Prashanth Mundkur wrote:
> I'm able to resolve the assert with the following patch to
> ofproto.c:ofport_open():
> 
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 653b2a8..48ceff7 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -2177,6 +2177,8 @@ ofport_open(struct ofproto *ofproto,
>              ofproto_port->ofp_port = alloc_ofp_port(ofproto,
>                                                      ofproto_port->name);
>          }
> +    } else {
> +        ofport_set_usage(ofproto, ofproto_port->ofp_port, LLONG_MAX);
>      }
>      pp->port_no = ofproto_port->ofp_port;
>      netdev_get_etheraddr(netdev, pp->hw_addr);
> 
> 
> The port is created in the following sequence in ofproto.c:update_port():
> 
>     netdev = (!ofproto_port_query_by_name(ofproto, name, &ofproto_port)
>               ? ofport_open(ofproto, &ofproto_port, &pp)
>               : NULL);
> 
> My ofproto implementation of port_query_by_name returns a ofport of 1, which
> causes ofport_open to skip the alloc_ofp_port() to update the usage hmap.  When
> I do the update by adding the line in the above patch, the assert goes away.
> 
> Is this a bug, or am I not using the api correctly?

The ofproto layer (not the implementation of it in e.g. ofproto-dpif)
expects to assign OpenFlow port numbers.  It sounds like your
implementation is trying to assign OpenFlow port numbers.  That may be
the problem.  That said, since ofproto-dpif is the main implementation
of the ofproto layer, there could easily be bugs in the layering.