[ovs-dev] [PATCH 2/4] ovs-vswitchd: Add support for setting OpenFlow datapath descriptions
Ben Pfaff
blp at nicira.com
Mon Feb 1 17:16:33 UTC 2010
On Fri, Jan 29, 2010 at 04:48:27PM -0800, Justin Pettit wrote:
>
> +#define DEFAULT_MFR_DESC "Nicira Networks, Inc."
> +#define DEFAULT_HW_DESC "Reference Implementation"
> +#define DEFAULT_SW_DESC VERSION BUILDNR
> +#define DEFAULT_SERIAL_DESC "None"
> +#define DEFAULT_DP_DESC "None"
I don't think that we should claim to be the reference implementation.
Why not change that string to "Open vSwitch"?
> static void
> +bridge_update_desc(struct bridge *br)
> +{
> + bool changed = false;
> + const char *desc;
> +
> + desc = cfg_get_string(0, "bridge.%s.mfr-desc", br->name);
> + if (desc != br->mfr_desc) {
I think that this will be true every time. Did you mean to use
strcmp()?
> + free(br->mfr_desc);
> + if (desc) {
> + br->mfr_desc = xstrdup(desc);
> + } else {
> + br->mfr_desc = xstrdup(DEFAULT_MFR_DESC);
> + }
> + changed = true;
> + }
This cut-and-paste work is crying out for a helper function. Or you
could even turn the strings into an array and use a for loop.
> + if (changed) {
> + ofproto_set_desc(br->ofproto, br->mfr_desc, br->hw_desc,
> + br->sw_desc, br->serial_desc, br->dp_desc);
> + }
Is there a good reason for the bridge to only call ofproto_set_desc() if
the values have changed? If not, then the bridge doesn't have to keep
its own copies of those strings at all.
It might be easier to make ofproto_set_desc() itself use the default
values if NULLs are passed in.
More information about the dev
mailing list