[ovs-dev] [PATCH] Add ifindex column to Interface table

Neil Mckee neil.mckee at inmon.com
Tue Jul 2 21:54:57 UTC 2013


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.mckee at 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 <ethan at 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.mckee at 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.mckee at 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 at openvswitch.org
>>>> http://openvswitch.org/mailman/listinfo/dev
>>> _______________________________________________
>>> dev mailing list
>>> dev at openvswitch.org
>>> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list