Re: [PATCH iproute2-next] ipaddress: fix label matching

[Serhey Popovych <serhe.popovych@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1605 bytes --]

Vincent Bernat wrote:
>  ❦ 11 juillet 2018 21:01 -0400, David Ahern <dsahern@gmail.com> :
> 
>>> +++ b/ip/ipaddress.c
>>> @@ -837,11 +837,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
>>>  	if (!name)
>>>  		return -1;
>>>  
>>> -	if (filter.label &&
>>> -	    (!filter.family || filter.family == AF_PACKET) &&
>>> -	    fnmatch(filter.label, name, 0))
>>> -		return -1;
>>> -
>>
>> The offending commit changed the return code:
>>
>>         if (filter.label &&
>>             (!filter.family || filter.family == AF_PACKET) &&
>> -           fnmatch(filter.label, RTA_DATA(tb[IFLA_IFNAME]), 0))
>> -               return 0;
>> +           fnmatch(filter.label, name, 0))
>> +               return -1;
>>
>>
>> Vincent: can you try leaving the code as is, but change the return to 0?
> 
> Yes, it works by just returning 0. The code still doesn't make sense.
> 

I think return code is correct. Check presented by this code too because
print_linkinfo() isn't static and called from ipmonitor.c where no
ipaddr_filter() or similar call that filters by label present.

Instead fnmatch() compares interface *name*, not label from IFA_LABEL
attribute. Thus:

    fnmatch(pattern, string, flags) ->
    fnmatch("lo:1", "lo", 0) == FNM_NOMATCH (1)

Assuming above I would like to see ifa_label_match_rta() instead of open
coded checks for filter.label with fmatch() in print_linkinfo().

Also it might be good idea to pass @name from get_ifname_rta() (like we
do in print_linkinfo()) to ifa_label_match_rta() so that we respect
IFLA_IFNAME if present.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]