Re: [ovs-dev] [PATCH ovn v4 4/9] controller: Move OVS port functions to new module.
On Fri, Sep 17, 2021 at 6:17 PM Mark Michelson wrote: > > Hi Frode, > > The changes I list below are all minor and could be taken care of by > whoever may merge this series. I don't think it's worth spinning a new > version of the series for these changes alone. However, I do see that > there were other findings by Numan for later patches. Thank you for taking the time to review, Mark, most appreciated! I'll make sure to incorporate your feedback into the next roll of the series. Yes, Numan did provide (great) feedback on patch 8 which requires more work than can physically fit before the 21.09 hard freeze, which is unfortunate for me but ok. It will make the result all the better. Cheers! -- Frode Nordahl > On 9/3/21 3:27 PM, Frode Nordahl wrote: > > Up until now the controller patch module has been the only > > consumer of functions to maintain OVS ports and interfaces. > > > > With the introduction of infrastructure for plugging providers > > these functions will also be consumed by the controller binding > > module. > > > > As such we introduce a new module called ovsport where these > > shared utility functions can live and be consumed by multiple > > modules. > > > > Signed-off-by: Frode Nordahl > > --- > > controller/automake.mk | 4 +- > > controller/ovsport.c | 256 + > > controller/ovsport.h | 60 ++ > > 3 files changed, 319 insertions(+), 1 deletion(-) > > create mode 100644 controller/ovsport.c > > create mode 100644 controller/ovsport.h > > > > diff --git a/controller/automake.mk b/controller/automake.mk > > index 41f907d6e..ad2d68af2 100644 > > --- a/controller/automake.mk > > +++ b/controller/automake.mk > > @@ -35,7 +35,9 @@ controller_ovn_controller_SOURCES = \ > > controller/mac-learn.c \ > > controller/mac-learn.h \ > > controller/local_data.c \ > > - controller/local_data.h > > + controller/local_data.h \ > > + controller/ovsport.h \ > > + controller/ovsport.c > > > > controller_ovn_controller_LDADD = lib/libovn.la > > $(OVS_LIBDIR)/libopenvswitch.la > > man_MANS += controller/ovn-controller.8 > > diff --git a/controller/ovsport.c b/controller/ovsport.c > > new file mode 100644 > > index 0..b1183e9ed > > --- /dev/null > > +++ b/controller/ovsport.c > > @@ -0,0 +1,256 @@ > > +/* Copyright (c) 2021 Canonical > > + * > > + * Licensed under the Apache License, Version 2.0 (the "License"); > > + * you may not use this file except in compliance with the License. > > + * You may obtain a copy of the License at: > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, software > > + * distributed under the License is distributed on an "AS IS" BASIS, > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > > + * See the License for the specific language governing permissions and > > + * limitations under the License. > > + */ > > + > > +#include > > +#include "ovsport.h" > > + > > +#include "lib/vswitch-idl.h" > > +#include "openvswitch/vlog.h" > > + > > +VLOG_DEFINE_THIS_MODULE(ovsport); > > + > > +/* Create a port and interface record and add it to 'bridge' in the Open > > + * vSwitch database represented by 'ovs_idl_txn'. > > + * > > + * 'name' is required and is used both for the name of the port and > > interface > > + * records. Depending on the contents of the optional 'iface_type' > > parameter > > + * the name may need to refer to an existing interface in the system. It > > is > > + * the callers responsibility to ensure that no other port with the desired > > s/callers/caller's/ > > > + * name already exist. > > s/exist/exists/ > > > + * > > + * 'iface_type' optionally specifies the type of intercace, otherwise set > > it to > > s/intercace/interface/ > > > + * NULL. > > + * > > + * 'port_external_ids' - the contents of the map will be used to fill the > > + * external_ids column of the created port record, otherwise set it to > > NULL. > > + * > > + * 'iface_external_ids' - the contents of the map will be used to fill the > > + * external_ids column of the created interface record, otherwise set it to > > + * NULL. > > + * > > + * 'iface_options' - the contents of the map will be used to fill the > > options > > + * column of the created interface record, otherwise set it to NULL. > > + * > > + * 'iface_mtu_request' - if a value > 0 is provided it will be filled into > > the > > + * mtu_request column of the created interface record. */ > > +void > > +ovsport_create(struct ovsdb_idl_txn *ovs_idl_txn, > > + const struct ovsrec_bridge *bridge, > > + const char *name, > > + const char *iface_type, > > + const struct smap *port_external_ids, > > + const struct smap *iface_external_ids, > > + const struct smap *iface_options, > > + const int64_t
Re: [ovs-dev] [PATCH ovn v4 4/9] controller: Move OVS port functions to new module.
Hi Frode,
The changes I list below are all minor and could be taken care of by
whoever may merge this series. I don't think it's worth spinning a new
version of the series for these changes alone. However, I do see that
there were other findings by Numan for later patches.
On 9/3/21 3:27 PM, Frode Nordahl wrote:
Up until now the controller patch module has been the only
consumer of functions to maintain OVS ports and interfaces.
With the introduction of infrastructure for plugging providers
these functions will also be consumed by the controller binding
module.
As such we introduce a new module called ovsport where these
shared utility functions can live and be consumed by multiple
modules.
Signed-off-by: Frode Nordahl
---
controller/automake.mk | 4 +-
controller/ovsport.c | 256 +
controller/ovsport.h | 60 ++
3 files changed, 319 insertions(+), 1 deletion(-)
create mode 100644 controller/ovsport.c
create mode 100644 controller/ovsport.h
diff --git a/controller/automake.mk b/controller/automake.mk
index 41f907d6e..ad2d68af2 100644
--- a/controller/automake.mk
+++ b/controller/automake.mk
@@ -35,7 +35,9 @@ controller_ovn_controller_SOURCES = \
controller/mac-learn.c \
controller/mac-learn.h \
controller/local_data.c \
- controller/local_data.h
+ controller/local_data.h \
+ controller/ovsport.h \
+ controller/ovsport.c
controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
man_MANS += controller/ovn-controller.8
diff --git a/controller/ovsport.c b/controller/ovsport.c
new file mode 100644
index 0..b1183e9ed
--- /dev/null
+++ b/controller/ovsport.c
@@ -0,0 +1,256 @@
+/* Copyright (c) 2021 Canonical
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+#include "ovsport.h"
+
+#include "lib/vswitch-idl.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(ovsport);
+
+/* Create a port and interface record and add it to 'bridge' in the Open
+ * vSwitch database represented by 'ovs_idl_txn'.
+ *
+ * 'name' is required and is used both for the name of the port and interface
+ * records. Depending on the contents of the optional 'iface_type' parameter
+ * the name may need to refer to an existing interface in the system. It is
+ * the callers responsibility to ensure that no other port with the desired
s/callers/caller's/
+ * name already exist.
s/exist/exists/
+ *
+ * 'iface_type' optionally specifies the type of intercace, otherwise set it to
s/intercace/interface/
+ * NULL.
+ *
+ * 'port_external_ids' - the contents of the map will be used to fill the
+ * external_ids column of the created port record, otherwise set it to NULL.
+ *
+ * 'iface_external_ids' - the contents of the map will be used to fill the
+ * external_ids column of the created interface record, otherwise set it to
+ * NULL.
+ *
+ * 'iface_options' - the contents of the map will be used to fill the options
+ * column of the created interface record, otherwise set it to NULL.
+ *
+ * 'iface_mtu_request' - if a value > 0 is provided it will be filled into the
+ * mtu_request column of the created interface record. */
+void
+ovsport_create(struct ovsdb_idl_txn *ovs_idl_txn,
+ const struct ovsrec_bridge *bridge,
+ const char *name,
+ const char *iface_type,
+ const struct smap *port_external_ids,
+ const struct smap *iface_external_ids,
+ const struct smap *iface_options,
+ const int64_t iface_mtu_request)
+{
+struct ovsrec_interface *iface;
+iface = ovsrec_interface_insert(ovs_idl_txn);
+ovsrec_interface_set_name(iface, name);
+if (iface_type) {
+ovsrec_interface_set_type(iface, iface_type);
+}
+ovsrec_interface_set_external_ids(iface, iface_external_ids);
+ovsrec_interface_set_options(iface, iface_options);
+ovsrec_interface_set_mtu_request(
+iface, _mtu_request, iface_mtu_request > 0);
+
+struct ovsrec_port *port;
+port = ovsrec_port_insert(ovs_idl_txn);
+ovsrec_port_set_name(port, name);
+ovsrec_port_set_interfaces(port, , 1);
+ovsrec_port_set_external_ids(port, port_external_ids);
+
+ovsrec_bridge_update_ports_addvalue(bridge, port);
+ovsrec_bridge_verify_ports(bridge);
+}
+
+/* Remove 'port' from 'bridge' and delete the 'port' recrd and any records
[ovs-dev] [PATCH ovn v4 4/9] controller: Move OVS port functions to new module.
Up until now the controller patch module has been the only
consumer of functions to maintain OVS ports and interfaces.
With the introduction of infrastructure for plugging providers
these functions will also be consumed by the controller binding
module.
As such we introduce a new module called ovsport where these
shared utility functions can live and be consumed by multiple
modules.
Signed-off-by: Frode Nordahl
---
controller/automake.mk | 4 +-
controller/ovsport.c | 256 +
controller/ovsport.h | 60 ++
3 files changed, 319 insertions(+), 1 deletion(-)
create mode 100644 controller/ovsport.c
create mode 100644 controller/ovsport.h
diff --git a/controller/automake.mk b/controller/automake.mk
index 41f907d6e..ad2d68af2 100644
--- a/controller/automake.mk
+++ b/controller/automake.mk
@@ -35,7 +35,9 @@ controller_ovn_controller_SOURCES = \
controller/mac-learn.c \
controller/mac-learn.h \
controller/local_data.c \
- controller/local_data.h
+ controller/local_data.h \
+ controller/ovsport.h \
+ controller/ovsport.c
controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
man_MANS += controller/ovn-controller.8
diff --git a/controller/ovsport.c b/controller/ovsport.c
new file mode 100644
index 0..b1183e9ed
--- /dev/null
+++ b/controller/ovsport.c
@@ -0,0 +1,256 @@
+/* Copyright (c) 2021 Canonical
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+#include "ovsport.h"
+
+#include "lib/vswitch-idl.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(ovsport);
+
+/* Create a port and interface record and add it to 'bridge' in the Open
+ * vSwitch database represented by 'ovs_idl_txn'.
+ *
+ * 'name' is required and is used both for the name of the port and interface
+ * records. Depending on the contents of the optional 'iface_type' parameter
+ * the name may need to refer to an existing interface in the system. It is
+ * the callers responsibility to ensure that no other port with the desired
+ * name already exist.
+ *
+ * 'iface_type' optionally specifies the type of intercace, otherwise set it to
+ * NULL.
+ *
+ * 'port_external_ids' - the contents of the map will be used to fill the
+ * external_ids column of the created port record, otherwise set it to NULL.
+ *
+ * 'iface_external_ids' - the contents of the map will be used to fill the
+ * external_ids column of the created interface record, otherwise set it to
+ * NULL.
+ *
+ * 'iface_options' - the contents of the map will be used to fill the options
+ * column of the created interface record, otherwise set it to NULL.
+ *
+ * 'iface_mtu_request' - if a value > 0 is provided it will be filled into the
+ * mtu_request column of the created interface record. */
+void
+ovsport_create(struct ovsdb_idl_txn *ovs_idl_txn,
+ const struct ovsrec_bridge *bridge,
+ const char *name,
+ const char *iface_type,
+ const struct smap *port_external_ids,
+ const struct smap *iface_external_ids,
+ const struct smap *iface_options,
+ const int64_t iface_mtu_request)
+{
+struct ovsrec_interface *iface;
+iface = ovsrec_interface_insert(ovs_idl_txn);
+ovsrec_interface_set_name(iface, name);
+if (iface_type) {
+ovsrec_interface_set_type(iface, iface_type);
+}
+ovsrec_interface_set_external_ids(iface, iface_external_ids);
+ovsrec_interface_set_options(iface, iface_options);
+ovsrec_interface_set_mtu_request(
+iface, _mtu_request, iface_mtu_request > 0);
+
+struct ovsrec_port *port;
+port = ovsrec_port_insert(ovs_idl_txn);
+ovsrec_port_set_name(port, name);
+ovsrec_port_set_interfaces(port, , 1);
+ovsrec_port_set_external_ids(port, port_external_ids);
+
+ovsrec_bridge_update_ports_addvalue(bridge, port);
+ovsrec_bridge_verify_ports(bridge);
+}
+
+/* Remove 'port' from 'bridge' and delete the 'port' recrd and any records
+ * with a weakRef to it. */
+void
+ovsport_remove(const struct ovsrec_bridge *bridge,
+ const struct ovsrec_port *port)
+{
+ovsrec_bridge_update_ports_delvalue(bridge, port);
+ovsrec_bridge_verify_ports(bridge);
+ovsrec_port_delete(port);
+}
+
+static void update_interface_smap_column(
+const struct ovsrec_interface *, const struct smap *,
+const struct smap *, void
