[PATCH] Detect and report live interface type.

[Laine Stump]

Poke around in sysfs and procfs to determine the type of an interface,
and set the "type" attribute in the xml appropriately. If no clue is
found, default to "ethernet", which is the current behavior elsewhere
in netcf as well.
---
 src/drv_initscripts.c |    3 +++
 src/dutil.c           |   33 +++++++++++++++++++++++++++++++++
 src/dutil.h           |    5 +++++
 3 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index 8e36fab..ead5b9f 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -775,6 +775,9 @@ char *drv_xml_state(struct netcf_if *nif) {
     prop = xmlNewProp(root, BAD_CAST "name", BAD_CAST nif->name);
     ERR_NOMEM(prop == NULL, ncf);
 
+    prop = xmlSetProp(root, BAD_CAST "type", BAD_CAST if_type(ncf,
nif->name));
+    ERR_NOMEM(prop == NULL, ncf);
+
     /* get the current IP address and prefix, and add both to the
      * document.
      */
diff --git a/src/dutil.c b/src/dutil.c
index 817ec6d..9c8e575 100644
--- a/src/dutil.c
+++ b/src/dutil.c
@@ -499,6 +499,39 @@ int if_ipv4_prefix(struct netcf *ncf, const char *intf) {
     return prefix;
 }
 
+const char *if_type(struct netcf *ncf, const char *intf) {
+    char *path;
+    struct stat stats;
+    const char *ret = NULL;
+
+    xasprintf(&path, "/proc/net/vlan/%s", intf);
+    ERR_NOMEM(path == NULL, ncf);
+    if ((stat (path, &stats) == 0) && S_ISREG (stats.st_mode)) {
+        ret = "vlan";
+    }
+    FREE(path);
+
+    if (ret == NULL) {
+        xasprintf(&path, "/sys/class/net/%s/bridge", intf);
+        ERR_NOMEM(path == NULL, ncf);
+        if (stat (path, &stats) == 0 && S_ISDIR (stats.st_mode))
+            return "bridge";
+        FREE(path);
+    }
+    if (ret == NULL) {
+        xasprintf(&path, "/sys/class/net/%s/bonding", intf);
+        ERR_NOMEM(path == NULL, ncf);
+        if (stat (path, &stats) == 0 && S_ISDIR (stats.st_mode))
+            return "bond";
+        FREE(path);
+    }
+
+error:        
+    if (ret == NULL)
+        ret = "ethernet";
+    return ret;
+}
+
 /* Create a new netcf if instance for interface NAME */
 struct netcf_if *make_netcf_if(struct netcf *ncf, char *name) {
     int r;
diff --git a/src/dutil.h b/src/dutil.h
index 0a9715f..74d0023 100644
--- a/src/dutil.h
+++ b/src/dutil.h
@@ -123,6 +123,11 @@ unsigned int if_ipv4_netmask(struct netcf *ncf, const char *intf);
 
 int if_ipv4_prefix(struct netcf *ncf, const char *intf);
 
+/* return the type of the interface - "ethernet" (physical device),
+ * "bridge", "bond", or "vlan"
+ */
+const char *if_type(struct netcf *ncf, const char *intf);
+
 /* Create a new netcf if instance for interface NAME */
 struct netcf_if *make_netcf_if(struct netcf *ncf, char *name);
 
-- 
1.6.2.5

[Daniel P. Berrange]

On Tue, Oct 13, 2009 at 01:27:57PM -0400, Laine Stump wrote:
>  Poke around in sysfs and procfs to determine the type of an
> interface,
>  and set the "type" attribute in the xml appropriately. If no clue is
>  found, default to "ethernet", which is the current behavior elsewhere
>  in netcf as well.
>  ---
>   src/drv_initscripts.c |    3 +++
>   src/dutil.c           |   33 +++++++++++++++++++++++++++++++++
>   src/dutil.h           |    5 +++++
>   3 files changed, 41 insertions(+), 0 deletions(-) 
ACK, this turned out much simpler that I had thought it would. Though
of course someone will no doubt find an older kernel which has things
in a different place, that can be fixed up as & when required.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

[Laine Stump]

On 10/13/2009 03:44 PM, Daniel P. Berrange wrote:
>  On Tue, Oct 13, 2009 at 01:27:57PM -0400, Laine Stump wrote:
>     
> > Poke around in sysfs and procfs to determine the type of an interface,
> > and set the "type" attribute in the xml appropriately. If no clue is
> > found, default to "ethernet", which is the current behavior elsewhere
> > in netcf as well.
> > ---
> >   src/drv_initscripts.c |    3 +++
> >   src/dutil.c           |   33 +++++++++++++++++++++++++++++++++
> >   src/dutil.h           |    5 +++++
> >   3 files changed, 41 insertions(+), 0 deletions(-)
> >      
>  ACK, this turned out much simpler that I had thought it would. Though
>  of course someone will no doubt find an older kernel which has things
>  in a different place, that can be fixed up as&  when required.
>      
Yeah, I was surprised too - spent a lot of time worrying about nothing. 
Of course, I'm suspect of its reliability (and as I think I said 
somewhere yesterday - it doesn't work when the interfaces are down - the 
stuff in /sys and /proc is only there when the interface is up. Of 
course this applies for everything else about the interface too, so I 
guess that's okay.)

[David Lutterkort]

On Tue, 2009-10-13 at 13:27 -0400, Laine Stump wrote:
>  Poke around in sysfs and procfs to determine the type of an
> interface,
>  and set the "type" attribute in the xml appropriately. If no clue is
>  found, default to "ethernet", which is the current behavior elsewhere
>  in netcf as well. 
Cool .. some small nits:

>  diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
>  index 8e36fab..ead5b9f 100644
>  --- a/src/drv_initscripts.c
>  +++ b/src/drv_initscripts.c
>  @@ -775,6 +775,9 @@ char *drv_xml_state(struct netcf_if *nif) {
>       prop = xmlNewProp(root, BAD_CAST "name", BAD_CAST nif->name);
>       ERR_NOMEM(prop == NULL, ncf);
>   
>  +    prop = xmlSetProp(root, BAD_CAST "type", BAD_CAST if_type(ncf,
> nif->name));
>  +    ERR_NOMEM(prop == NULL, ncf); 
This swallows errors in if_type; I'd prefer

        const char *type = if_type(ncf, nif->name);
        ERR_BAIL(ncf);
        prop = xmlSetProp(root, BAD_CAST "type", BAD_CAST type);
        ERR_NOMEM(prop == NULL, ncf);

>  diff --git a/src/dutil.c b/src/dutil.c
>  index 817ec6d..9c8e575 100644
>  --- a/src/dutil.c
>  +++ b/src/dutil.c
>  @@ -499,6 +499,39 @@ int if_ipv4_prefix(struct netcf *ncf, const char *intf) {
>       return prefix;
>   }
>   
>  +const char *if_type(struct netcf *ncf, const char *intf) {
>  +    char *path;
>  +    struct stat stats;
>  +    const char *ret = NULL;
>  +
>  +    xasprintf(&path, "/proc/net/vlan/%s", intf);
>  +    ERR_NOMEM(path == NULL, ncf);
>  +    if ((stat (path, &stats) == 0) && S_ISREG (stats.st_mode)) {
>  +        ret = "vlan";
>  +    }
>  +    FREE(path);
>  +
>  +    if (ret == NULL) {
>  +        xasprintf(&path, "/sys/class/net/%s/bridge", intf);
>  +        ERR_NOMEM(path == NULL, ncf);
>  +        if (stat (path, &stats) == 0 && S_ISDIR (stats.st_mode))
>  +            return "bridge";
>  +        FREE(path);
>  +    }
>  +    if (ret == NULL) {
>  +        xasprintf(&path, "/sys/class/net/%s/bonding", intf);
>  +        ERR_NOMEM(path == NULL, ncf);
>  +        if (stat (path, &stats) == 0 && S_ISDIR (stats.st_mode))
>  +            return "bond";
>  +        FREE(path);
>  +    }
>  +
>  +error:        
>  +    if (ret == NULL)
>  +        ret = "ethernet";
>  +    return ret;
>  +} 
I'd prefer if the error return was NULL, i.e. the error label should go
two lines down.

David

[Laine Stump]

On 10/13/2009 04:49 PM, David Lutterkort wrote:
>  On Tue, 2009-10-13 at 13:27 -0400, Laine Stump wrote:
>     
> > Poke around in sysfs and procfs to determine the type of an interface,
> > and set the "type" attribute in the xml appropriately. If no clue is
> > found, default to "ethernet", which is the current behavior elsewhere
> > in netcf as well.
> >      
>  Cool .. some small nits:
>      

It gets better - see below.

The updated patch will be along momentarily.


>     
> > diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
> > index 8e36fab..ead5b9f 100644
> > --- a/src/drv_initscripts.c
> > +++ b/src/drv_initscripts.c
> > @@ -775,6 +775,9 @@ char *drv_xml_state(struct netcf_if *nif) {
> >       prop = xmlNewProp(root, BAD_CAST "name", BAD_CAST nif->name);
> >       ERR_NOMEM(prop == NULL, ncf);
> >
> > +    prop = xmlSetProp(root, BAD_CAST "type", BAD_CAST if_type(ncf,
> nif->name));
> > +    ERR_NOMEM(prop == NULL, ncf);
> >      
>  This swallows errors in if_type; I'd prefer
>
>           const char *type = if_type(ncf, nif->name);
>           ERR_BAIL(ncf);
>           prop = xmlSetProp(root, BAD_CAST "type", BAD_CAST type);
>           ERR_NOMEM(prop == NULL, ncf);
>      
Good point.

>     
> > diff --git a/src/dutil.c b/src/dutil.c
> > index 817ec6d..9c8e575 100644
> > --- a/src/dutil.c
> > +++ b/src/dutil.c
> > @@ -499,6 +499,39 @@ int if_ipv4_prefix(struct netcf *ncf, const char *intf) {
> >       return prefix;
> >   }
> >
> > +const char *if_type(struct netcf *ncf, const char *intf) {
> > +    char *path;
> > +    struct stat stats;
> > +    const char *ret = NULL;
> > +
> > +    xasprintf(&path, "/proc/net/vlan/%s", intf);
> > +    ERR_NOMEM(path == NULL, ncf);
> > +    if ((stat (path,&stats) == 0)&&  S_ISREG (stats.st_mode)) {
> > +        ret = "vlan";
> > +    }
> > +    FREE(path);
> > +
> > +    if (ret == NULL) {
> > +        xasprintf(&path, "/sys/class/net/%s/bridge", intf);
> > +        ERR_NOMEM(path == NULL, ncf);
> > +        if (stat (path,&stats) == 0&&  S_ISDIR (stats.st_mode))
> > +            return "bridge";
> >       

Oops. that should have been 'ret = "bridge"'.


> > +        FREE(path);
> > +    }
> > +    if (ret == NULL) {
> > +        xasprintf(&path, "/sys/class/net/%s/bonding", intf);
> > +        ERR_NOMEM(path == NULL, ncf);
> > +        if (stat (path,&stats) == 0&&  S_ISDIR (stats.st_mode))
> > +            return "bond";
> >       

Likewise.


> > +        FREE(path);
> > +    }
> > +
> > +error:
> > +    if (ret == NULL)
> > +        ret = "ethernet";
> > +    return ret;
> > +}
> >      
>  I'd prefer if the error return was NULL, i.e. the error label should go
>  two lines down.
>
>  David
>
>
>
>

[Laine Stump]

Poke around in sysfs and procfs to determine the type of an interface,
and set the "type" attribute in the xml appropriately. If no clue is
found, default to "ethernet", which is the current behavior elsewhere
in netcf as well.
---
 src/drv_initscripts.c |    6 ++++++
 src/dutil.c           |   33 +++++++++++++++++++++++++++++++++
 src/dutil.h           |    5 +++++
 3 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index 8e36fab..e4f2b37 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -759,6 +759,7 @@ char *drv_xml_state(struct netcf_if *nif) {
     xmlDocPtr ncf_xml = NULL;
     xmlNodePtr root;
     xmlAttrPtr prop;
+    const char *type;
     unsigned int ipv4;
     int prefix;
 
@@ -775,6 +776,11 @@ char *drv_xml_state(struct netcf_if *nif) {
     prop = xmlNewProp(root, BAD_CAST "name", BAD_CAST nif->name);
     ERR_NOMEM(prop == NULL, ncf);
 
+    type = if_type(ncf, nif->name);
+    ERR_BAIL(ncf);
+    prop = xmlSetProp(root, BAD_CAST "type", BAD_CAST type);
+    ERR_NOMEM(prop == NULL, ncf);
+
     /* get the current IP address and prefix, and add both to the
      * document.
      */
diff --git a/src/dutil.c b/src/dutil.c
index 817ec6d..ae27801 100644
--- a/src/dutil.c
+++ b/src/dutil.c
@@ -499,6 +499,39 @@ int if_ipv4_prefix(struct netcf *ncf, const char *intf) {
     return prefix;
 }
 
+const char *if_type(struct netcf *ncf, const char *intf) {
+    char *path;
+    struct stat stats;
+    const char *ret = NULL;
+
+    xasprintf(&path, "/proc/net/vlan/%s", intf);
+    ERR_NOMEM(path == NULL, ncf);
+    if ((stat (path, &stats) == 0) && S_ISREG (stats.st_mode)) {
+        ret = "vlan";
+    }
+    FREE(path);
+
+    if (ret == NULL) {
+        xasprintf(&path, "/sys/class/net/%s/bridge", intf);
+        ERR_NOMEM(path == NULL, ncf);
+        if (stat (path, &stats) == 0 && S_ISDIR (stats.st_mode))
+            ret = "bridge";
+        FREE(path);
+    }
+    if (ret == NULL) {
+        xasprintf(&path, "/sys/class/net/%s/bonding", intf);
+        ERR_NOMEM(path == NULL, ncf);
+        if (stat (path, &stats) == 0 && S_ISDIR (stats.st_mode))
+            ret = "bond";
+        FREE(path);
+    }
+    if (ret == NULL)
+        ret = "ethernet";
+
+error:        
+    return ret;
+}
+
 /* Create a new netcf if instance for interface NAME */
 struct netcf_if *make_netcf_if(struct netcf *ncf, char *name) {
     int r;
diff --git a/src/dutil.h b/src/dutil.h
index 0a9715f..74d0023 100644
--- a/src/dutil.h
+++ b/src/dutil.h
@@ -123,6 +123,11 @@ unsigned int if_ipv4_netmask(struct netcf *ncf, const char *intf);
 
 int if_ipv4_prefix(struct netcf *ncf, const char *intf);
 
+/* return the type of the interface - "ethernet" (physical device),
+ * "bridge", "bond", or "vlan"
+ */
+const char *if_type(struct netcf *ncf, const char *intf);
+
 /* Create a new netcf if instance for interface NAME */
 struct netcf_if *make_netcf_if(struct netcf *ncf, char *name);
 
-- 
1.6.2.5

[David Lutterkort]

On Wed, 2009-10-14 at 00:07 -0400, Laine Stump wrote:
>  Poke around in sysfs and procfs to determine the type of an
> interface,
>  and set the "type" attribute in the xml appropriately. If no clue is
>  found, default to "ethernet", which is the current behavior elsewhere
>  in netcf as well. 
ACK. Committed

David