Bug 1008566

Summary: InfiniBand device matching
Product: Red Hat Enterprise Linux 7 Reporter: Dan Winship <danw>
Component: NetworkManagerAssignee: Dan Winship <danw>
Status: CLOSED CURRENTRELEASE QA Contact: Desktop QE <desktop-qa-list>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 7.0CC: dcbw, jklimes, kdube, rkhan, thaller, vbenes
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-06-13 11:17:37 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Add additional checks to spec_match_list none

Description Dan Winship 2013-09-16 15:37:51 UTC
Doug Ledford says:

> You can't match on the whole HWADDR in the ifcfg-*_ib? files.  Only
> the last 8 bytes are tied to the hardware like a MAC address, the
> remainder of the bytes are all things that can change due to external
> factors and should not be considered when matching interfaces on the
> system against interface definition files.  Currently, multiple of the
> rdma-dev-0? machines exhibit this behavior because I changed the subnet
> prefix, which changes byte 9 of the HWADDR.

This affects (at least) libnm-glib's nm_device_connection_compatible(), and the daemon's nm_device_check_connection_compatible() and nm_match_spec_hwaddr().

Comment 1 Dan Winship 2013-09-16 22:22:01 UTC
patches in danw/ib branch in git

Comment 2 Thomas Haller 2013-09-17 13:01:32 UTC
Created attachment 798804 [details]
Add additional checks to spec_match_list

nm_device_get_hw_address "can" return NULL, so check for it.

Apart from the attached patch, it looks good to me.

Comment 3 Jirka Klimes 2013-09-17 16:39:48 UTC
> infiniband: only check the last 8 bytes when doing hwaddr matches

Don't be want to compare case insensitively (I guess specs comes from the user)?
-strcmp (spec_str + 40, hwaddr_str + 36)
+strcasecmp (spec_str + 40, hwaddr_str + 36)

if (   g_str_has_prefix (iter->data, "mac:")
should be
if (   g_str_has_prefix (spec_str, "mac:")
to be consistent.

(In reply to Thomas Haller from comment #2)
> nm_device_get_hw_address "can" return NULL, so check for it.
> 
nm_device_get_hw_address () is also used in nm-device-infiniband.c, so the check for NULL should be done there too.

Comment 4 Dan Winship 2013-09-23 13:48:59 UTC
(In reply to Thomas Haller from comment #2)
> nm_device_get_hw_address "can" return NULL, so check for it.

Only for devices that sometimes don't have a hardware address (like bluetooth). It can't return NULL for InifiniBand.

However, the suggestion to short-circuit the check if specs is NULL makes sense, and I added that in another patch.

(In reply to Jirka Klimes from comment #3)
> Don't be want to compare case insensitively

oh, yes. Fixed and re-pushed.

(I assume that even though both of you only commented on this bug, that you reviewed the whole branch, including the part that applies to bug 1008568?)

Comment 5 Dan Winship 2013-09-24 15:56:51 UTC
fixed in git

Comment 7 Vladimir Benes 2014-04-03 11:14:06 UTC
ifcfg file with infiniband mac address seems to be working correctly

Comment 8 Ludek Smid 2014-06-13 11:17:37 UTC
This request was resolved in Red Hat Enterprise Linux 7.0.

Contact your manager or support representative in case you have further questions about the request.