[ovs-dev] [PATCH v3] Add configurable OpenFlow port name.
Flavio Leitner
fbl at sysclose.org
Tue May 10 20:31:13 UTC 2016
On Tue, May 10, 2016 at 10:31:19AM +0800, Xiao Liang wrote:
> On Tue, May 10, 2016 at 4:28 AM, Flavio Leitner <fbl at sysclose.org> wrote:
> > On Sat, Apr 23, 2016 at 01:26:17PM +0800, Xiao Liang wrote:
> >> Add new column "ofname" in Interface table to configure port name reported
> >> to controllers with OpenFlow protocol, thus decouple OpenFlow port name from
> >> device name.
> >>
> >> For example:
> >> # ovs-vsctl set Interface eth0 ofname=wan
> >> # ovs-vsctl set Interface eth1 ofname=lan0
> >> then controllers can recognize ports by their names.
> >
> > This change is nice because now the same setup like a "compute node"
> > can use the same logical name to refer to a specific interface that
> > could have different netdev name on different HW.
> >
> > Comments inline.
> >
> >> Signed-off-by: Xiao Liang <shaw.leon at gmail.com>
> >> ---
> >> v2: Added test for ofname
> >> Increased db schema version
> >> Updated NEWS
> >> v3: Rebase
> >> ---
> >> NEWS | 1 +
> >> lib/db-ctl-base.h | 2 +-
> >> ofproto/ofproto-provider.h | 1 +
> >> ofproto/ofproto.c | 67 ++++++++++++++++++++++++++++++++++++++++++++--
> >> ofproto/ofproto.h | 9 ++++++-
> >> tests/ofproto.at | 60 +++++++++++++++++++++++++++++++++++++++++
> >> utilities/ovs-vsctl.c | 1 +
> >> vswitchd/bridge.c | 10 +++++--
> >> vswitchd/vswitch.ovsschema | 6 +++--
> >> vswitchd/vswitch.xml | 14 ++++++++++
> >> 10 files changed, 163 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/NEWS b/NEWS
> >> index ea7f3a1..156781c 100644
> >> --- a/NEWS
> >> +++ b/NEWS
> >> @@ -15,6 +15,7 @@ Post-v2.5.0
> >> now implemented. Only flow mod and port mod messages are supported
> >> in bundles.
> >> * New OpenFlow extension NXM_NX_MPLS_TTL to provide access to MPLS TTL.
> >> + * Port name can now be set with "ofname" column in the Interface table.
> >> - ovs-ofctl:
> >> * queue-get-config command now allows a queue ID to be specified.
> >> * '--bundle' option can now be used with OpenFlow 1.3.
> >> diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
> >> index f8f576b..5bd62d5 100644
> >> --- a/lib/db-ctl-base.h
> >> +++ b/lib/db-ctl-base.h
> >> @@ -177,7 +177,7 @@ struct weak_ref_table {
> >> struct cmd_show_table {
> >> const struct ovsdb_idl_table_class *table;
> >> const struct ovsdb_idl_column *name_column;
> >> - const struct ovsdb_idl_column *columns[3]; /* Seems like a good number. */
> >> + const struct ovsdb_idl_column *columns[4]; /* Seems like a good number. */
> >> const struct weak_ref_table wref_table;
> >> };
> >>
> >> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> >> index daa0077..8795242 100644
> >> --- a/ofproto/ofproto-provider.h
> >> +++ b/ofproto/ofproto-provider.h
> >> @@ -84,6 +84,7 @@ struct ofproto {
> >> struct hmap ports; /* Contains "struct ofport"s. */
> >> struct shash port_by_name;
> >> struct simap ofp_requests; /* OpenFlow port number requests. */
> >> + struct smap ofp_names; /* OpenFlow port names. */
> >> uint16_t alloc_port_no; /* Last allocated OpenFlow port number. */
> >> uint16_t max_ports; /* Max possible OpenFlow port num, plus one. */
> >> struct hmap ofport_usage; /* Map ofport to last used time. */
> >> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> >> index ff6affd..a2799f4 100644
> >> --- a/ofproto/ofproto.c
> >> +++ b/ofproto/ofproto.c
> >> @@ -550,6 +550,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
> >> hmap_init(&ofproto->ofport_usage);
> >> shash_init(&ofproto->port_by_name);
> >> simap_init(&ofproto->ofp_requests);
> >> + smap_init(&ofproto->ofp_names);
> >> ofproto->max_ports = ofp_to_u16(OFPP_MAX);
> >> ofproto->eviction_group_timer = LLONG_MIN;
> >> ofproto->tables = NULL;
> >> @@ -1546,6 +1547,7 @@ ofproto_destroy__(struct ofproto *ofproto)
> >> hmap_destroy(&ofproto->ofport_usage);
> >> shash_destroy(&ofproto->port_by_name);
> >> simap_destroy(&ofproto->ofp_requests);
> >> + smap_destroy(&ofproto->ofp_names);
> >>
> >> OFPROTO_FOR_EACH_TABLE (table, ofproto) {
> >> oftable_destroy(table);
> >> @@ -1945,7 +1947,7 @@ ofproto_port_open_type(const char *datapath_type, const char *port_type)
> >> * 'ofp_portp' is non-null). */
> >> int
> >> ofproto_port_add(struct ofproto *ofproto, struct netdev *netdev,
> >> - ofp_port_t *ofp_portp)
> >> + ofp_port_t *ofp_portp, const char *ofp_name)
> >> {
> >> ofp_port_t ofp_port = ofp_portp ? *ofp_portp : OFPP_NONE;
> >> int error;
> >> @@ -1956,6 +1958,9 @@ ofproto_port_add(struct ofproto *ofproto, struct netdev *netdev,
> >>
> >> simap_put(&ofproto->ofp_requests, netdev_name,
> >> ofp_to_u16(ofp_port));
> >> + if (ofp_name) {
> >> + smap_replace(&ofproto->ofp_names, netdev_name, ofp_name);
> >
> > Should it be unique? See below.
> >
> >> + }
> >> error = update_port(ofproto, netdev_name);
> >> }
> >> if (ofp_portp) {
> >> @@ -2009,6 +2014,8 @@ ofproto_port_del(struct ofproto *ofproto, ofp_port_t ofp_port)
> >> simap_delete(&ofproto->ofp_requests, ofp_request_node);
> >> }
> >>
> >> + smap_remove(&ofproto->ofp_names, name);
> >> +
> >> error = ofproto->ofproto_class->port_del(ofproto, ofp_port);
> >> if (!error && ofport) {
> >> /* 'name' is the netdev's name and update_port() is going to close the
> >> @@ -2285,6 +2292,7 @@ ofport_open(struct ofproto *ofproto,
> >> {
> >> enum netdev_flags flags;
> >> struct netdev *netdev;
> >> + const char *ofp_name;
> >> int error;
> >>
> >> error = netdev_open(ofproto_port->name, ofproto_port->type, &netdev);
> >> @@ -2307,7 +2315,13 @@ ofport_open(struct ofproto *ofproto,
> >> }
> >> pp->port_no = ofproto_port->ofp_port;
> >> netdev_get_etheraddr(netdev, &pp->hw_addr);
> >> - ovs_strlcpy(pp->name, ofproto_port->name, sizeof pp->name);
> >> +
> >> + ofp_name = smap_get(&ofproto->ofp_names, ofproto_port->name);
> >> + if (!ofp_name) {
> >> + ofp_name = ofproto_port->name;
> >> + }
> >> + ovs_strlcpy(pp->name, ofp_name, sizeof pp->name);
> >
> > Before pp->name was the device's name which is unique on the system.
> > Now pp->name can change to some other random string which ovs-ofctl
> > mod-port will use it. The new name also needs to be unique, right?
> >
> I didn't found whether port name should be unique in openflow spec,
> and it didn't specify the behavior of "mod-port by name" either. If
> there are duplicate names, current "ovs-ofctl mod-port" will modify
> the first port it sees. Do you think this is acceptable or we should
> force the name to be unique? (I think this behavior should be
> documented in ovs-ofctl)
I agree that the behavior should be documented in ovs-ofctl. I also
didn't find anything in the specs but it doesn't seem useful to have
two or more ports sharing the same name. At least not in the same
bridge.
Also that the code truncates the name, so if someone pass names like:
interface_number_1 and interface_number_2, both would result in the
same truncated name and that may cause issues as well.
Do you know any use-case where duplicated names would be required?
--
fbl
More information about the dev
mailing list