#2217 network: assumes all ethernet devices are named ethX

Usability
open
Provider (226)
5
2013-02-08
2011-06-06
No

sblim-cmpi-network provider assumes all ethernet devices are named ethX, but network devices can have arbitrary names (at least in Fedora).

OSBase_CommonNetwork.c:int enum_all_netPorts( struct netPortList ** lptr ) {

...
if(hdchk && hdchk[0]) {
if( strstr(hdchk[0], "Ethernet") != NULL ) { type = 1; }
if( strstr(hdchk[0], "Token Ring") != NULL ) { type = 2; }
if( strstr(hdchk[0], "Local Loopback") != NULL ) { type = 3; }
}
else {
if( strstr(port,"eth") != NULL ) { type = 1; }
if( strstr(port,"tr") != NULL ) { type = 2; }
if( strstr(port,"lo") != NULL ) { type = 3; }
}

The code above looks partially fine, as it decides the type in accordance with "Link encap" field first, which is device name independent (but the "else" branch can be misleading). So "wbemcli ei -nl myCimom/root/cimv2:Linux_EthernetPort" works fine, no matter how ethernet devices are named.

The main issue I can see is with following code:
cmpiOSBase_NetworkPortImplementsIPEndpoint.c:CMPIInstance *
_assoc_get_networkPort_INST()
...
ptr = CMGetCharPtr(name);
if( strstr(ptr,"eth") != NULL ) { targetClass = _RefLeftClassEth; }
else if( strstr(ptr,"tr") != NULL ) { targetClass = _RefLeftClassTr; }
else if( strstr(ptr,"lo") != NULL ) { targetClass = _RefLeftClassLo; }
else { goto exit; }
...

cmpiOSBase_NetworkPortImplementsIPEndpointProvider.c:CMPIStatus
OSBase_NetworkPortImplementsIPEndpointProviderAssociators( CMPIAssociationMI *
mi,
...
if( strcasecmp(resultClass,_RefLeftClassEth) == 0 &&
strstr(ptr,"eth") != NULL ) {
_RefLeftClass = _RefLeftClassEth;
}
else if( strcasecmp(resultClass,_RefLeftClassTr) == 0 &&
strstr(ptr,"tr") != NULL ) {
_RefLeftClass = _RefLeftClassTr;
}
else if( strcasecmp(resultClass,_RefLeftClassLo) == 0 &&
strstr(ptr,"lo") != NULL ) {
_RefLeftClass = _RefLeftClassLo;
}
else { goto exit; }
...
same in
CMPIStatus OSBase_NetworkPortImplementsIPEndpointProviderAssociatorNames(
CMPIAssociationMI * mi,

The problem can be easily reproduced on any machine with ethernet device named differently than ethX, e. g. my ifconfig says:
...
em1 Link encap:Ethernet HWaddr ...
...

Then "wbemcli ei -nl myCimom/root/cimv2:Linux_NetworkPortImplementsIPEndpoint" will not show association between Linux_EthernetPort and Linux_IPProtocolEndpoint. It just shows one association for loopback.

Discussion

  • I investigated it a bit again and I think that it can be fixed this way... In OSBase_CommonNetwork.c i found:
    229 if( type == 1 ) { (*sptr)->linkTec = 2; /* Ethernet */}
    230 if( type == 2 ) { (*sptr)->linkTec = 7; /* Token Ring */}
    231 if( type == 3 ) { (*sptr)->linkTec = 1; /* Other : Local Loopback */ }

    So the type (which is detected correctly if 'ifconfig' prints Ethernet, Token Ring and Local Loopback in the output - and it does in my case - see previous comment) can be detected also from linkTec, which is stored into LinkTechnology propery for every type:
    cmpiOSBase_EthernetPort.c:156: CMSetProperty( ci, "LinkTechnology", (CMPIValue*)&(sptr->linkTec), CMPI_uint16);
    cmpiOSBase_LocalLoopbackPort.c:156: CMSetProperty( ci, "LinkTechnology", (CMPIValue*)&(sptr->linkTec), CMPI_uint16);
    cmpiOSBase_TokenRingPort.c:156: CMSetProperty( ci, "LinkTechnology", (CMPIValue*)&(sptr->linkTec), CMPI_uint16);

    So e.g. in cmpiOSBase_NetworkPortImplementsIPEndpoint.c should be IMHO instead of
    115 name = CMGetKey( ref, "Name", rc).value.string;
    116 if( name == NULL ) { goto exit; }
    117
    118 ptr = CMGetCharPtr(name);
    119 if( strstr(ptr,"eth") != NULL ) { targetClass = _RefLeftClassEth; }
    120 else if( strstr(ptr,"tr") != NULL ) { targetClass = _RefLeftClassTr; }
    121 else if( strstr(ptr,"lo") != NULL ) { targetClass = _RefLeftClassLo; }
    122 else { goto exit; }

    something like (sorry, it's a pseudocode, I don't know CMPI API):
    CMPIData data;
    data = CMGetProperty( x, "LinkTechnology", y);
    if( data.value.uint16 == 2 ) { targetClass = _RefLeftClassEth; }
    else if( data.value.uint16 == 7 ) { targetClass = _RefLeftClassTr; }
    else if( data.value.uint16 == 1 ) { targetClass = _RefLeftClassLo; }
    else { goto exit; }

    What do you think?

     
  • Tyrel Datwyler
    Tyrel Datwyler
    2012-01-13

    The suggested fix looks reasonable. I will look into providing a patch.

     
  • Finally, I created patch fixing the issue for me... It's based on the idea I presented before. Please review the patch, thanks!