> From: Ben Pfaff [mailto:b...@nicira.com]
> On Fri, Mar 06, 2015 at 03:37:38PM +0000, Mark D. Gray wrote:
> > From: "Mark D. Gray" <mark.d.g...@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.g...@intel.com>
>
> I'd fold this in with the previous patch that adds the columns.
Ok
>
> 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:
Ok, will have a look at this
> > @@ -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:
Ok
> > + 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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
