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.l...@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?
> +
> netdev_get_flags(netdev, &flags);
> pp->config = flags & NETDEV_UP ? 0 : OFPUTIL_PC_PORT_DOWN;
> pp->state = netdev_get_carrier(netdev) ? 0 : OFPUTIL_PS_LINK_DOWN;
> @@ -8011,3 +8025,52 @@ ofproto_port_set_realdev(struct ofproto *ofproto,
> ofp_port_t vlandev_ofp_port,
> }
> return error;
> }
> +
> +const char *
> +ofproto_port_get_ofpname(struct ofproto *ofproto, ofp_port_t ofp_port)
> +{
> + struct ofport *ofport;
> +
> + ofport = ofproto_get_port(ofproto, ofp_port);
> + return (ofport ? ofport->pp.name : NULL);
> +}
> +
> +void
> +ofproto_port_set_ofpname(struct ofproto *ofproto, ofp_port_t ofp_port,
> + const char *ofp_name)
> +{
> + struct ofport *ofport;
> + const char *devname;
> + size_t name_size;
> +
> + ofport = ofproto_get_port(ofproto, ofp_port);
> + if (!ofport) {
> + return;
> + }
> +
> + devname = netdev_get_name(ofport->netdev);
> + name_size = sizeof(ofport->pp.name);
> +
> + if (!devname ||
> + (ofp_name && !strncmp(ofp_name, ofport->pp.name, name_size - 1)) ||
> + (!ofp_name && !strncmp(devname, ofport->pp.name, name_size - 1))) {
> + /* No need to change port name. */
> + return;
> + }
> +
> + /* Send a OFPPR_DELETE followed by OFPPR_ADD port_status message
> + * to notify controller a port name change. */
> + connmgr_send_port_status(ofproto->connmgr, NULL, &ofport->pp,
> + OFPPR_DELETE);
> +
> + if (!ofp_name) {
> + smap_remove(&ofproto->ofp_names, devname);
> + ovs_strlcpy(ofport->pp.name, devname, name_size);
> + } else {
> + smap_replace(&ofproto->ofp_names, netdev_get_name(ofport->netdev),
> + ofp_name);
> + ovs_strlcpy(ofport->pp.name, ofp_name, name_size);
> + }
> + connmgr_send_port_status(ofproto->connmgr, NULL, &ofport->pp,
> + OFPPR_ADD);
> +}
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index 2d241c9..3382ef1 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -288,7 +288,8 @@ int ofproto_port_dump_done(struct ofproto_port_dump *);
>
> const char *ofproto_port_open_type(const char *datapath_type,
> const char *port_type);
> -int ofproto_port_add(struct ofproto *, struct netdev *, ofp_port_t
> *ofp_portp);
> +int ofproto_port_add(struct ofproto *, struct netdev *, ofp_port_t
> *ofp_portp,
> + const char *);
> int ofproto_port_del(struct ofproto *, ofp_port_t ofp_port);
> int ofproto_port_get_stats(const struct ofport *, struct netdev_stats
> *stats);
>
> @@ -521,6 +522,12 @@ int ofproto_port_set_realdev(struct ofproto *,
> ofp_port_t vlandev_ofp_port,
> enum ofputil_table_miss ofproto_table_get_miss_config(const struct ofproto *,
> uint8_t table_id);
>
> +const char *ofproto_port_get_ofpname(struct ofproto *ofproto,
> + ofp_port_t ofp_port);
> +void ofproto_port_set_ofpname(struct ofproto *ofproto,
> + ofp_port_t ofp_port,
> + const char *ofp_name);
> +
Looks like those belong to the "Configuration of ports" section.
> #ifdef __cplusplus
> }
> #endif
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index c4260ab..5f38160 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -77,6 +77,66 @@ OFPT_GET_CONFIG_REPLY: frags=normal miss_send_len=0
> OVS_VSWITCHD_STOP
> AT_CLEANUP
>
> +AT_SETUP([ofproto - set OpenFlow port name])
> +OVS_VSWITCHD_START(
> + [add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1 --\
> + add-port br0 p2 -- set Interface p2 type=dummy ofport_request=2
> ofname=n2])
> +check_ofname () {
> + AT_CHECK([ovs-ofctl -vwarn show br0], [0], [stdout])
> + echo >expout "OFPT_FEATURES_REPLY: dpid:fedcba9876543210
> +n_tables:254, n_buffers:256
> +capabilities: FLOW_STATS TABLE_STATS PORT_STATS QUEUE_STATS ARP_MATCH_IP
> +actions: output enqueue set_vlan_vid set_vlan_pcp strip_vlan mod_dl_src
> mod_dl_dst mod_nw_src mod_nw_dst mod_nw_tos mod_tp_src mod_tp_dst
> + 1($1): addr:aa:55:aa:55:00:0x
> + config: PORT_DOWN
> + state: LINK_DOWN
> + speed: 0 Mbps now, 0 Mbps max
> + 2(n2): addr:aa:55:aa:55:00:0x
> + config: PORT_DOWN
> + state: LINK_DOWN
> + speed: 0 Mbps now, 0 Mbps max
> + LOCAL(br0): addr:aa:55:aa:55:00:0x
> + config: PORT_DOWN
> + state: LINK_DOWN
> + speed: 0 Mbps now, 0 Mbps max
> +OFPT_GET_CONFIG_REPLY: frags=normal miss_send_len=0"
> + AT_CHECK([[sed '
> +s/ (xid=0x[0-9a-fA-F]*)//
> +s/00:0.$/00:0x/' < stdout]],
> + [0], [expout])
> +}
> +
> +AT_CHECK([ovs-vsctl set Interface p1 ofname=p2])
> +check_ofname p2
> +
> +AT_CHECK([ovs-ofctl -P standard monitor br0 --detach --no-chdir --pidfile])
> +# Use NXT_SET_ASYNC_CONFIG to enable only port status message
> +ovs-appctl -t ovs-ofctl ofctl/send
> 01040028000000020000232000000013000000000000000500000007000000020000000000000005
> +ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
> +AT_CHECK([ovs-vsctl set Interface p1 ofname=n2])
> +check_ofname n2
> +OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
> +AT_CHECK([[sed '
> +s/ (xid=0x[0-9a-fA-F]*)//
> +s/ *duration.*//
> +s/00:0.$/00:0x/' < monitor.log]],
> + [0], [dnl
> +OFPT_PORT_STATUS: DEL: 1(p2): addr:aa:55:aa:55:00:0x
> + config: PORT_DOWN
> + state: LINK_DOWN
> + speed: 0 Mbps now, 0 Mbps max
> +OFPT_PORT_STATUS: ADD: 1(n2): addr:aa:55:aa:55:00:0x
> + config: PORT_DOWN
> + state: LINK_DOWN
> + speed: 0 Mbps now, 0 Mbps max
> +])
> +
> +AT_CHECK([ovs-vsctl clear Interface p1 ofname])
> +check_ofname p1
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> dnl This is really bare-bones.
> dnl It at least checks request and reply serialization and deserialization.
> AT_SETUP([ofproto - port stats - (OpenFlow 1.0)])
> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> index c9c0f6d..44b386a 100644
> --- a/utilities/ovs-vsctl.c
> +++ b/utilities/ovs-vsctl.c
> @@ -1008,6 +1008,7 @@ static struct cmd_show_table cmd_show_tables[] = {
> {&ovsrec_table_interface,
> &ovsrec_interface_col_name,
> {&ovsrec_interface_col_type,
> + &ovsrec_interface_col_ofname,
> &ovsrec_interface_col_options,
> &ovsrec_interface_col_error},
> {NULL, NULL, NULL}
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 700f65c..bb49fe0 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -667,6 +667,10 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
> *ovs_cfg)
>
> LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
> iface_set_ofport(iface->cfg, iface->ofp_port);
> +
> + ofproto_port_set_ofpname(br->ofproto, iface->ofp_port,
> + iface->cfg->ofname);
> +
> /* Clear eventual previous errors */
> ovsrec_interface_set_error(iface->cfg, NULL);
> iface_configure_cfm(iface);
> @@ -1777,7 +1781,8 @@ iface_do_create(const struct bridge *br,
> }
>
> *ofp_portp = iface_pick_ofport(iface_cfg);
> - error = ofproto_port_add(br->ofproto, netdev, ofp_portp);
> + error = ofproto_port_add(br->ofproto, netdev, ofp_portp,
> + iface_cfg->ofname);
> if (error) {
> goto error;
> }
> @@ -1860,7 +1865,8 @@ iface_create(struct bridge *br, const struct
> ovsrec_interface *iface_cfg,
> error = netdev_open(port->name, "internal", &netdev);
> if (!error) {
> ofp_port_t fake_ofp_port = OFPP_NONE;
> - ofproto_port_add(br->ofproto, netdev, &fake_ofp_port);
> + ofproto_port_add(br->ofproto, netdev, &fake_ofp_port,
> + iface_cfg->ofname);
> netdev_close(netdev);
> } else {
> VLOG_WARN("could not open network device %s (%s)",
> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> index e0937f4..d147d04 100644
> --- a/vswitchd/vswitch.ovsschema
> +++ b/vswitchd/vswitch.ovsschema
> @@ -1,6 +1,6 @@
> {"name": "Open_vSwitch",
> - "version": "7.13.0",
> - "cksum": "2202834738 22577",
> + "version": "7.13.1",
> + "cksum": "643309718 22653",
> "tables": {
> "Open_vSwitch": {
> "columns": {
> @@ -204,6 +204,8 @@
> "mutable": false},
> "type": {
> "type": "string"},
> + "ofname": {
> + "type": {"key": "string", "min": 0, "max": 1}},
> "options": {
> "type": {"key": "string", "value": "string", "min": 0, "max":
> "unlimited"}},
> "ingress_policing_rate": {
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 166f264..1869c81 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -1724,6 +1724,20 @@
> on a host.
> </column>
>
> +
> + <column name="ofname">
> + <p>
> + OpenFlow port name for this interface. This name is truncated to
> + <code>OFP_MAX_PORT_NAME_LEN-1</code>, and reported to controllers
> in
> + port description. If not set, <ref column='name'/> is used.
> + </p>
> + <p>
> + OpenFlow does not have a way to announce a port name change, so if
> + <ref column="ofname"/> is changed, Open vSwitch represents it over
> + OpenFlow as a port deletion followed immediately by a port
> addition.
> + </p>
> + </column>
> +
> <column name="ifindex">
> A positive interface index as defined for SNMP MIB-II in RFCs 1213
> and
> 2863, if the interface has one, otherwise 0. The ifindex is useful
> for
> --
> 2.8.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
--
fbl
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
