| Message ID | 1550673468-84872-3-git-send-email-ankur.sharma@nutanix.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | OVN: Distributed Virtual Router for Vlan Backed Networks | expand |
On Wed, Feb 20, 2019 at 10:34:54PM +0000, Ankur Sharma 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. I suspect that at least patches 1 and 2, and possibly 3, should be squashed together. Patch 1 adds a setting with no implementation, which is not what we normally do. Patches 2 and 3 then implement the feature.
Hi Ankur, Few comments below. Thanks Numan On Thu, Feb 21, 2019 at 4:08 AM Ankur Sharma <ankur.sharma@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.sharma@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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Hi Numan, Thank you so much for review and initial comments. My apologies for delay in reply. Please find my response inline. Thanks Regards, Ankur From: Numan Siddique <nusiddiq@redhat.com> Sent: Wednesday, February 27, 2019 4:49 AM To: Ankur Sharma <ankur.sharma@nutanix.com> Cc: ovs-dev@openvswitch.org Subject: Re: [ovs-dev] [RFC PATCH v1 2/6] Vlan Support in ovn, northd changes to read logical switch network type. Hi Ankur, Few comments below. Thanks Numan On Thu, Feb 21, 2019 at 4:08 AM Ankur Sharma <ankur.sharma@nutanix.com<mailto:ankur.sharma@nutanix.com>> wrote: Background: [1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html [mail.openvswitch.org]<https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2018-2DOctober_353066.html&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=GDMl8HFF4Ya4H6QlK8PTJMvIrDw1OQ7wbLk9WVOUq-U&s=-zNki_k79T4KVfuRb-PcFVbf3nBc6n6dfRZKGWfv_dc&e=> [2] https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing [docs.google.com]<https://urldefense.proofpoint.com/v2/url?u=https-3A__docs.google.com_document_d_1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU_edit-3Fusp-3Dsharing&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=GDMl8HFF4Ya4H6QlK8PTJMvIrDw1OQ7wbLk9WVOUq-U&s=WkooftnZxx-FPucrY6I_QF0mlsBowEmoVOydrSrgLJA&e=> 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.sharma@nutanix.com<mailto:ankur.sharma@nutanix.com>> --- ovn/northd/ovn-northd.c | 40 ++++++++++++++++++++++++++++++++++++++++ ovn/ovn-sb.xml | 7 +++++++ tests/ovn-northd.at [ovn-northd.at]<https://urldefense.proofpoint.com/v2/url?u=http-3A__ovn-2Dnorthd.at&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=GDMl8HFF4Ya4H6QlK8PTJMvIrDw1OQ7wbLk9WVOUq-U&s=sNkKHpq29fCSVtI2Jo3kSmdqhaFOcbr4bZF1vfZajyg&e=> | 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" ? [ANKUR]: Sure, v2 will have this change. We could also have flat network type right ? i.e a logical_switch with a localnet port without any vlan tag. [ANKUR]: That’s correct. + /* 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 [ovn-northd.at]<https://urldefense.proofpoint.com/v2/url?u=http-3A__ovn-2Dnorthd.at&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=GDMl8HFF4Ya4H6QlK8PTJMvIrDw1OQ7wbLk9WVOUq-U&s=sNkKHpq29fCSVtI2Jo3kSmdqhaFOcbr4bZF1vfZajyg&e=> b/tests/ovn-northd.at [ovn-northd.at]<https://urldefense.proofpoint.com/v2/url?u=http-3A__ovn-2Dnorthd.at&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=GDMl8HFF4Ya4H6QlK8PTJMvIrDw1OQ7wbLk9WVOUq-U&s=sNkKHpq29fCSVtI2Jo3kSmdqhaFOcbr4bZF1vfZajyg&e=> index 1878eb2..818109f 100644 --- a/tests/ovn-northd.at [ovn-northd.at]<https://urldefense.proofpoint.com/v2/url?u=http-3A__ovn-2Dnorthd.at&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=GDMl8HFF4Ya4H6QlK8PTJMvIrDw1OQ7wbLk9WVOUq-U&s=sNkKHpq29fCSVtI2Jo3kSmdqhaFOcbr4bZF1vfZajyg&e=> +++ b/tests/ovn-northd.at [ovn-northd.at]<https://urldefense.proofpoint.com/v2/url?u=http-3A__ovn-2Dnorthd.at&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=GDMl8HFF4Ya4H6QlK8PTJMvIrDw1OQ7wbLk9WVOUq-U&s=sNkKHpq29fCSVtI2Jo3kSmdqhaFOcbr4bZF1vfZajyg&e=> @@ -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
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; + /* 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
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.sharma@nutanix.com> --- ovn/northd/ovn-northd.c | 40 ++++++++++++++++++++++++++++++++++++++++ ovn/ovn-sb.xml | 7 +++++++ tests/ovn-northd.at | 22 ++++++++++++++++++++++ 3 files changed, 69 insertions(+)