Hi Ankur,

Few comments below.

Thanks
Numan


On Thu, Feb 21, 2019 at 4:08 AM Ankur Sharma <ankur.sha...@nutanix.com>
wrote:

> Background:
> [1]
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
> [2]
> https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing
>
> This Series:
> Layer 2, Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan
> backed distributed logical router.
>
> This Patch:
> a. ovn-northd changes to get network_type from northbound database
> and populate the same in ovn_datapath struct.
>
> b. ovn-northd changes to set the network_type in datapath_binding
> table of southbound database, in external_id column as following
> key value pair.
>
> network-type = [overlay|vlan]
>
> c. This value will help with logical flow changes for L3.
>
> d. Unit tests to validate the content in southbound database.
>
> Signed-off-by: Ankur Sharma <ankur.sha...@nutanix.com>
> ---
>  ovn/northd/ovn-northd.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  ovn/ovn-sb.xml          |  7 +++++++
>  tests/ovn-northd.at     | 22 ++++++++++++++++++++++
>  3 files changed, 69 insertions(+)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 3569ea2..279840c 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -86,6 +86,12 @@ enum ovn_datapath_type {
>      DP_ROUTER                   /* OVN logical router. */
>  };
>
> +/* Network type of a datapath */
> +typedef enum ovn_datapath_nw_type {
> +   DP_NETWORK_OVERLAY,
> +   DP_NETWORK_VLAN
> +} ovn_datapath_nw_type;
>

In the OVS code base, I haven't seen usage of "typedef enum"  much.
May be you can just have "enum ovn_datapath_nw_type" ?


We could also have flat network type right ? i.e a logical_switch with a
localnet port without any
vlan tag.



+
>  /* Returns an "enum ovn_stage" built from the arguments.
>   *
>   * (It's better to use ovn_stage_build() for type-safety reasons, but
> inline
> @@ -438,6 +444,8 @@ struct ovn_datapath {
>
>      bool has_unknown;
>
> +    ovn_datapath_nw_type network_type;
> +
>      /* IPAM data. */
>      struct ipam_info ipam_info;
>
> @@ -469,6 +477,30 @@ cleanup_macam(struct hmap *macam_)
>      }
>  }
>
> +static void
> +ovn_datapath_update_nw_type(struct ovn_datapath *od)
> +{
> +      if (!od->nbs) {
> +         return;
> +      }
> +
> +      if (!strcmp(od->nbs->network_type, "overlay") ||
> +          !strlen(od->nbs->network_type)) {
> +
> +         // No value in network_type is taken as OVERLAY.
> +         od->network_type = DP_NETWORK_OVERLAY;
> +
> +      } else if (!strcmp(od->nbs->network_type, "vlan")) {
> +         od->network_type = DP_NETWORK_VLAN;
> +
> +      } else {
> +         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +         VLOG_WARN_RL(&rl, "bad network type %s, for %s",
> +                      od->nbs->network_type, od->nbs->name);
> +      }
> +
> +}
> +
>  static struct ovn_datapath *
>  ovn_datapath_create(struct hmap *datapaths, const struct uuid *key,
>                      const struct nbrec_logical_switch *nbs,
> @@ -659,6 +691,12 @@ ovn_datapath_update_external_ids(struct ovn_datapath
> *od)
>      if (name2 && name2[0]) {
>          smap_add(&ids, "name2", name2);
>      }
> +
> +    if (od->nbs) {
> +       smap_add(&ids, "network-type", strlen(od->nbs->network_type) ?
> +                od->nbs->network_type : "overlay");
> +    }
> +
>      sbrec_datapath_binding_set_external_ids(od->sb, &ids);
>      smap_destroy(&ids);
>  }
> @@ -712,9 +750,11 @@ join_datapaths(struct northd_context *ctx, struct
> hmap *datapaths,
>              ovs_list_remove(&od->list);
>              ovs_list_push_back(both, &od->list);
>              ovn_datapath_update_external_ids(od);
> +            ovn_datapath_update_nw_type(od);
>          } else {
>              od = ovn_datapath_create(datapaths, &nbs->header_.uuid,
>                                       nbs, NULL, NULL);
> +            ovn_datapath_update_nw_type(od);
>              ovs_list_push_back(nb_only, &od->list);
>          }
>
> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> index 8ffef40..01ef892 100644
> --- a/ovn/ovn-sb.xml
> +++ b/ovn/ovn-sb.xml
> @@ -2090,6 +2090,13 @@ tcp.flags = RST;
>          the <ref db="OVN_Northbound"/> database.
>        </column>
>
> +      <column name="external_ids" key="logical-switch-type">
> +        For a logical datapath that represents a logical switch,
> +        <code>ovn-northd</code> stores in this key the network type from
> +        corresponding <ref table="Logical_Switch" db="OVN_Northbound"/>
> row in
> +        the <ref db="OVN_Northbound"/> database.
> +      </column>
> +
>        <group title="Naming">
>          <p>
>            <code>ovn-northd</code> copies these from the name fields in
> the <ref
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 1878eb2..818109f 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -301,3 +301,25 @@ as northd
>  OVS_APP_EXIT_AND_WAIT([ovn-northd])
>
>  AT_CLEANUP
> +
> +AT_SETUP([ovn -- check logical switch type propagation from NBDB to SBDB])
> +AT_SKIP_IF([test $HAVE_PYTHON = no])
> +ovn_start
> +
> +ovn-nbctl ls-add ls0
> +
> +uuid=`ovn-sbctl --bare --columns=_uuid list Datapath`
> +echo "LS UUID is: " $uuid
> +
> +type=`ovn-sbctl get Datapath_Binding ${uuid} external_ids:network-type`
> +echo "LS TYPE is: " $type
> +AT_CHECK([ovn-sbctl get Datapath_Binding ${uuid}
> external_ids:network-type], [0], [overlay
> +])
> +
> +ovn-nbctl ls-set-network-type ls0 vlan
> +type=`ovn-sbctl get Datapath_Binding ${uuid} external_ids:network-type`
> +echo "LS TYPE is: " $type
> +AT_CHECK([ovn-sbctl get Datapath_Binding ${uuid}
> external_ids:network-type], [0], [vlan
> +])
> +
> +AT_CLEANUP
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to