Oh - it fails some tests now - such as the LACP ones. Looks like
ovsrec_interface_set_ifindex() is triggering this error in
lib/ovsdb-idl.c:ovsdb_idl_txn_write__()
if (row->table->idl->verify_write_only && !write_only) {
VLOG_ERR("Bug: Attempt to write to a read/write column (%s:%s) when"
" explicitly configured not to.", class->name, column->name);
goto discard_datum;
}
I guess that means the tests are 'locking' their config db in some way? So
before I call ovsrec_interface_set_ifindex() in bridge.c:iface_refresh_status()
I should check for that? How?
(don't expect answer for at least a week)
Neil
On Jul 2, 2013, at 4:59 PM, Ben Pfaff <b...@nicira.com> wrote:
> Thanks Neil. I'm on vacation this week but I'll look at this when I get back
> next week.
>
> On Jul 2, 2013 5:55 PM, "Neil Mckee" <neil.mc...@inmon.com> wrote:
> OK, here's how it looks now (against latest master). I wasn't sure where to
> add the NEWS item so I just guessed.
>
> Signed-off-by: Neil McKee <neil.mc...@inmon.com>
>
> ---
> NEWS | 2 ++
> vswitchd/bridge.c | 11 +++++++++++
> vswitchd/vswitch.ovsschema | 7 +++++--
> vswitchd/vswitch.xml | 7 +++++++
> 4 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index b07a651..6008415 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -14,6 +14,8 @@ post-v1.11.0
>
> v1.11.0 - xx xxx xxxx
> ---------------------
> + - The OS-assigned SNMP ifindex number for each interface is now exported
> + as an extra column in the database.
> - Support for megaflows, which allows wildcarding in the kernel (and
> any dpif implementation that supports wildcards). Depending on
> the flow table and switch configuration, flow set up rates are
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 28bf082..3ed3643 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1811,6 +1811,7 @@ iface_refresh_status(struct iface *iface)
> int64_t mtu_64;
> uint8_t mac[ETH_ADDR_LEN];
> int error;
> + int64_t ifindex64;
>
> if (iface_is_synthetic(iface)) {
> return;
> @@ -1855,6 +1856,15 @@ iface_refresh_status(struct iface *iface)
> } else {
> ovsrec_interface_set_mac_in_use(iface->cfg, NULL);
> }
> + ifindex64 = (int64_t)netdev_get_ifindex(iface->netdev);
> + /* The netdev may return a negative number (such as -EOPNOTSUPP)
> + * if there is no valid ifindex number. */
> + if(ifindex64 < 0) {
> + ovsrec_interface_set_ifindex(iface->cfg, NULL, 0);
> + }
> + else {
> + ovsrec_interface_set_ifindex(iface->cfg, &ifindex64, 1);
> + }
> }
>
> /* Writes 'iface''s CFM statistics to the database. 'iface' must not be
> @@ -3573,6 +3583,7 @@ iface_clear_db_record(const struct ovsrec_interface
> *if_cfg)
> ovsrec_interface_set_cfm_remote_mpids(if_cfg, NULL, 0);
> ovsrec_interface_set_lacp_current(if_cfg, NULL, 0);
> ovsrec_interface_set_statistics(if_cfg, NULL, NULL, 0);
> + ovsrec_interface_set_ifindex(if_cfg, NULL, 0);
> }
> }
>
> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> index bb3ca48..81319df 100644
> --- a/vswitchd/vswitch.ovsschema
> +++ b/vswitchd/vswitch.ovsschema
> @@ -1,6 +1,6 @@
> {"name": "Open_vSwitch",
> - "version": "7.2.0",
> - "cksum": "543912409 19436",
> + "version": "7.2.1",
> + "cksum": "683896421 19543",
> "tables": {
> "Open_vSwitch": {
> "columns": {
> @@ -168,6 +168,9 @@ "name": {
> "type": "string", "mutable": false},
> + "ifindex": {
> + "type": { "key": "integer", "min": 0, "max": 1},
> + "ephemeral": true},
> "type": {
> "type": "string"},
> "options": {
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 12780d6..c0cf739 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -1208,6 +1208,13 @@
> on a host.
> </column>
>
> + <column name="ifindex">
> + Interface index, if known. Should be integer between 1 and
> 4294967295,
> + as defined for SNMP MIB-II in RFCs 1213 and 2863. Although this
> identifier
> + is not referenced by OpenFlow, it is necessary for seamless
> integration
> + with protocols such as SNMP and sFlow.
> + </column>
> +
> <column name="mac_in_use">
> The MAC address in use by this interface.
> </column>
> --
> 1.8.1.4
>
>
>
> On Jun 13, 2013, at 2:55 PM, Ethan Jackson <et...@nicira.com> wrote:
>
> >> ifindex is pretty much everywhere because it (as far as I know)
> >> originated in BSD and has since then been enshrined in SNMP and
> >> elsewhere.
> >
> > Fine with me then.
> >
> > Ethan
> >
> >>
> >> On Thu, Jun 13, 2013 at 02:48:56PM -0700, Ethan Jackson wrote:
> >>> Since this is OS specific, perhaps we should put it in the status
> >>> column instead?
> >>>
> >>> Ethan
> >>>
> >>> On Thu, Jun 13, 2013 at 2:42 PM, Neil Mckee <neil.mc...@inmon.com> wrote:
> >>>> This proposed patch adds an "ifindex" column to the "Interface" table in
> >>>> the db. So that
> >>>> "ovs-vsctl list Interface" can show the ifindex numbers for those
> >>>> interfaces that have
> >>>> them (and 0 for the rest).
> >>>>
> >>>> For example:
> >>>>
> >>>> % ovs-vsctl --format json --columns name,ofport,ifindex list Interface
> >>>> {"data":[["br2",65534,10],["eth0",1,2],["br1",65534,9],["vm",2,11],["gre0",1,0]],"headings":["name","ofport","ifindex"]}
> >>>>
> >>>>
> >>>> Signed-off-by: Neil McKee <neil.mc...@inmon.com>
> >>>>
> >>>> ---
> >>>> vswitchd/bridge.c | 16 ++++++++++++++++
> >>>> vswitchd/vswitch.ovsschema | 7 +++++--
> >>>> 2 files changed, 21 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> >>>> index 09f98d5..0bd3c1d 100644
> >>>> --- a/vswitchd/bridge.c
> >>>> +++ b/vswitchd/bridge.c
> >>>> @@ -373,6 +373,7 @@ bridge_init(const char *remote)
> >>>> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_cfm_health);
> >>>> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_cfm_remote_opstate);
> >>>> ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_lacp_current);
> >>>> + ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_ifindex);
> >>>> ovsdb_idl_omit(idl, &ovsrec_interface_col_external_ids);
> >>>>
> >>>> ovsdb_idl_omit_alert(idl, &ovsrec_controller_col_is_connected);
> >>>> @@ -1709,6 +1710,7 @@ iface_refresh_status(struct iface *iface)
> >>>> int64_t mtu_64;
> >>>> uint8_t mac[ETH_ADDR_LEN];
> >>>> int error;
> >>>> + int64_t ifindex64;
> >>>>
> >>>> if (iface_is_synthetic(iface)) {
> >>>> return;
> >>>> @@ -1753,6 +1755,19 @@ iface_refresh_status(struct iface *iface)
> >>>> } else {
> >>>> ovsrec_interface_set_mac_in_use(iface->cfg, NULL);
> >>>> }
> >>>> +
> >>>> + ifindex64 = (int64_t)netdev_get_ifindex(iface->netdev);
> >>>> + if(ifindex64 < 0) {
> >>>> + /* The netdev may return a negative number (such as -EOPNOTSUPP)
> >>>> + * if there is no valid ifindex number. Report than as 0 here.
> >>>> + * 0 is not a valid ifindex number, so we can use it to mean
> >>>> + * "Not applicable, or don't know, or something went wrong". It
> >>>> + * may be necessary to revist this if we don't want to collapse
> >>>> + * those states.
> >>>> + */
> >>>> + ifindex64 = 0;
> >>>> + }
> >>>> + ovsrec_interface_set_ifindex(iface->cfg, ifindex64);
> >>>> }
> >>>>
> >>>> /* Writes 'iface''s CFM statistics to the database. 'iface' must not be
> >>>> @@ -3443,6 +3458,7 @@ iface_clear_db_record(const struct
> >>>> ovsrec_interface *if_cfg)
> >>>> ovsrec_interface_set_cfm_remote_mpids(if_cfg, NULL, 0);
> >>>> ovsrec_interface_set_lacp_current(if_cfg, NULL, 0);
> >>>> ovsrec_interface_set_statistics(if_cfg, NULL, NULL, 0);
> >>>> + ovsrec_interface_set_ifindex(if_cfg, 0);
> >>>> }
> >>>> }
> >>>>
> >>>> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> >>>> index 594ffb4..e08502f 100644
> >>>> --- a/vswitchd/vswitch.ovsschema
> >>>> +++ b/vswitchd/vswitch.ovsschema
> >>>> @@ -1,6 +1,6 @@
> >>>> {"name": "Open_vSwitch",
> >>>> - "version": "7.1.0",
> >>>> - "cksum": "2234055133 17444",
> >>>> + "version": "7.1.1",
> >>>> + "cksum": "689297048 17521",
> >>>> "tables": {
> >>>> "Open_vSwitch": {
> >>>> "columns": {
> >>>> @@ -164,6 +164,9 @@
> >>>> "name": {
> >>>> "type": "string",
> >>>> "mutable": false},
> >>>> + "ifindex": {
> >>>> + "type": "integer",
> >>>> + "ephemeral": true},
> >>>> "type": {
> >>>> "type": "string"},
> >>>> "options": {
> >>>> --
> >>>> 1.8.1.4
> >>>>
> >>>> _______________________________________________
> >>>> dev mailing list
> >>>> dev@openvswitch.org
> >>>> http://openvswitch.org/mailman/listinfo/dev
> >>> _______________________________________________
> >>> dev mailing list
> >>> dev@openvswitch.org
> >>> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
