On 9 September 2016 at 13:48, Ben Pfaff <b...@ovn.org> wrote:

> On Tue, Sep 06, 2016 at 01:25:50AM -0700, Gurucharan Shetty wrote:
> > When there are hundreds of nodes controlled by OVN, the workflow
> > to track and allocate unique tags across multiple hosts becomes
> > complicated.  It is much easier to let ovn-northd do the allocation.
> >
> > Signed-off-by: Gurucharan Shetty <g...@ovn.org>
>
> I think that there's an off-by-one in the call to bitmap_scan().  Here's
> an incremental for this and other minor stuff.
>
You are right. Thanks for the fix.


>
> Acked-by: Ben Pfaff <b...@ovn.org>
>
> --8<--------------------------cut here-------------------------->8--
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index a2af36d..32ee1f4 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -1036,9 +1036,8 @@ tag_alloc_create_new_tag(struct hmap
> *tag_alloc_table,
>          int64_t tag;
>          tag_alloc_node = tag_alloc_get_node(tag_alloc_table,
>                                              nbsp->parent_name);
> -        tag = bitmap_scan(tag_alloc_node->allocated_tags, 0, 1,
> -                          MAX_OVN_TAGS + 1);
> -        if (tag == MAX_OVN_TAGS + 1) {
> +        tag = bitmap_scan(tag_alloc_node->allocated_tags, 0, 1,
> MAX_OVN_TAGS);
> +        if (tag == MAX_OVN_TAGS) {
>              static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
>              VLOG_ERR_RL(&rl, "out of vlans for logical switch ports with "
>                          "parent %s", nbsp->parent_name);
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index 077e4c5..d6f55e6 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -331,13 +331,13 @@
>          <p>
>            The VLAN tag in the network traffic associated with a
> container's
>            network interface.  The client can request
> <code>ovn-northd</code>
> -          to allocate a uniqe tag for the logical switch port with a
> specific
> +          to allocate a tag that is unique within the scope of a specific
>            parent (specified in <ref column="parent_name"/>) by setting a
> value
>            of <code>0</code> in this column.  The allocated value is
> written
>            by <code>ovn-northd</code> in the <ref column="tag"/> column
>            (Note that these tags are allocated and managed locally in
>            <code>ovn-northd</code>, so they cannot be reconstructed in the
> event
> -          that the database is lost).  The client can also request a
> specific
> +          that the database is lost.)  The client can also request a
> specific
>
Was the full stop move above intentional?



           non-zero tag and <code>ovn-northd</code> will honor it and copy
> that
>            value to the <ref column="tag"/> column.
>          </p>
> diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
> index 28e9572..015b416 100644
> --- a/ovn/utilities/ovn-nbctl.8.xml
> +++ b/ovn/utilities/ovn-nbctl.8.xml
> @@ -125,9 +125,10 @@
>          <p>
>            Creates on <var>switch</var> a logical switch port named
>            <var>port</var> that is a child of <var>parent</var> that is
> -          identifed with VLAN ID <var>tag_request</var>.  For a given
> -          <var>parent</var>, if <var>tag_request</var> is <code>0</code>,
> -          <code>ovn-northd</code> generates a unique tag.  This is useful
> in
> +          identified with VLAN ID <var>tag_request</var>.  If
> +          <var>tag_request</var> is <code>0</code>,
> <code>ovn-northd</code>
> +          generates a tag that is unique in the scope of
> <var>parent</var>.
> +          This is useful in
>            cases such as virtualized container environments where Open
>            vSwitch does not have a direct connection to the container's
>            port and it must be shared with the virtual machine's port.
> diff --git a/tests/ovn.at b/tests/ovn.at
> index b62f58f..2cca6cf 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -5093,7 +5093,7 @@ AT_CHECK([ovn-sbctl --data=bare --no-heading
> --columns=tag find port_binding \
>  logical_port="c3"], [0], [4
>  ])
>
> -dnl A differnet parent.
> +dnl A different parent.
>  AT_CHECK([ovn-nbctl --wait=sb lsp-add ls1 c4 parent2 0])
>  AT_CHECK([ovn-nbctl lsp-get-tag c4], [0], [1
>  ])
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to