[ovs-dev] [PATCH 3/3] bridge: Update bridge to discover datapath and port types
Ben Pfaff
blp at nicira.com
Fri Mar 6 21:22:28 UTC 2015
On Fri, Mar 06, 2015 at 03:37:38PM +0000, Mark D. Gray wrote:
> From: "Mark D. Gray" <mark.d.gray at intel.com>
>
> This patch enumerates datapath and port types and adds this
> information to the datapath_types and port_types columns in the
> ovsdb.
>
> This allows an ovsdb client to query the datapath in order to
> determine if certain datapath and port types exist. For example,
> by querying the port_types column, an ovsdb client will be able
> to determine if this instance of ovs-vswitchd was compiled with
> DPDK support.
>
> Signed-off-by: Mark D. Gray <mark.d.gray at intel.com>
I'd fold this in with the previous patch that adds the columns.
We don't generally use double new-lines, so I'd drop this hunk:
> @@ -317,6 +319,7 @@ static ofp_port_t iface_get_requested_ofp_port(
> const struct ovsrec_interface *);
> static ofp_port_t iface_pick_ofport(const struct ovsrec_interface *);
>
> +
> /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
> *
> * This is deprecated. It is only for compatibility with broken device drivers
CodingStyle requires {} here. However, it's wasteful to do this on
every trip through the OVS main loop; it should not be necessary:
> @@ -2889,6 +2896,9 @@ bridge_run(void)
> }
> cfg = ovsrec_open_vswitch_first(idl);
>
> + if (cfg)
> + discover_types(cfg);
> +
> /* Initialize the ofproto library. This only needs to run once, but
> * it must be done after the configuration is set. If the
> * initialization has already occurred, bridge_init_ofproto()
> +/*
> + * Add registered netdev and dpif types to ovsdb to allow external
> + * applications to query the capabilities of the Open vSwitch instance
> + * running on the node.
> + */
> +static void
> +discover_types(const struct ovsrec_open_vswitch *cfg)
> +{
> + struct sset types;
> + const char *type;
> + struct ovsdb_idl_txn *txn;
> + size_t n_dp_types, n_port_types;
> + unsigned int i;
> + const char **datapath_types;
> + const char **port_types;
> +
> + txn = ovsdb_idl_txn_create(idl);
> +
> + /* datapath types */
> + sset_init(&types);
> + dp_enumerate_types(&types);
> + n_dp_types = sset_count(&types);
Please use "sizeof *datapath_types" instead of "sizeof(char *)" as
recommended by CodingStyle. Ditto one other place below:
> + datapath_types = xmalloc(sizeof(char *) * n_dp_types);
> +
> + i = 0;
> + SSET_FOR_EACH(type, &types) {
I don't think there's value in doing the xstrdup() here. It just creates
more data to free later. Ditto one other place later:
> + datapath_types[i++] = xstrdup(type);
> + }
> +
> + ovsrec_open_vswitch_set_datapath_types(cfg, datapath_types, n_dp_types);
> +
> + sset_destroy(&types);
> +
> + /* port types */
> + sset_init(&types);
> + netdev_enumerate_types(&types);
> + n_port_types = sset_count(&types);
> +
> + port_types = xmalloc(sizeof(char *) * n_port_types);
> +
> + i = 0;
> + SSET_FOR_EACH(type, &types) {
> + port_types[i++] = xstrdup(type);
> + }
> +
> + ovsrec_open_vswitch_set_port_types(cfg, port_types, n_port_types);
> + sset_destroy(&types);
> +
> + /* clean up any allocated memory */
> + for (i = 0; i < n_dp_types; i++) {
> + free((char *)datapath_types[i]);
> + }
> +
> + free(datapath_types);
> + for (i = 0; i < n_port_types; i++) {
> + free((char *)port_types[i]);
> + }
> + free(port_types);
> +
> + ovsdb_idl_txn_commit(txn);
> + ovsdb_idl_txn_destroy(txn);
> +}
Thanks,
Ben.
More information about the dev
mailing list