| Message ID | 1531604194-12136-1-git-send-email-serhe.popovych@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
| Series | [iproute2] ipaddress: Fix and make consistent label match handling | expand |
On Sun, 15 Jul 2018 00:36:34 +0300 Serhey Popovych <serhe.popovych@gmail.com> wrote: > Since commit 9516823051ce ("ipaddress: Improve print_linkinfo()") we > return -1 instead of 0 when ip-address(8) label does not match network > device name as we did before change. This causes regression when trying > to output ip address matching label: > > # ip addr add 192.168.192.1/24 dev lo label lo:1 > # ip addr show label lo:1 > <no output> > > This is special case and return 0 from print_linkinfo() earlier to match > only filter.ifindex and filter.up if given, but not rest fields in > @filter. Then call print_selected_addrinfo() without calling > print_link_stats() in ipaddr_list_flush_or_save(). > > Later print_selected_addrinfo() calls print_addrinfo() that finally > matches IFA_LABEL attribute in netlink buffer with filter.label using > ifa_label_match_rta(). > > On the other hand there is three conditions checked in print_linkinfo() > to determine label special case: > > 1) filter.label != NULL > 2) filter.family == AF_UNSPEC || filter.family == AF_PACKET > 3) fnmatch(filter.label, name, 0) > > With 1) it is ok to check if filtering by label is on by given pattern > in @filter.label. > > Since label is IPv4 specific and AF_PACKET is for printing ip-link(8) > information (see ipaddr_link_list()::ipaddress.c as example) checking > for AF_PACKET in 2) doesn't take much sense: better to defer these > checks to print_addrinfo() determine valid combinations before calling > ifa_label_match_rta() to finally match IFA_LABEL to pattern in > filter.label. > > For 3) we have following call for test case: > > fnmatch(pattern, string, flags) -> > fnmatch(filter.label, name, 0) -> > fnmatch("lo:1", "lo", 0) == FNM_NOMATCH (1) or non-zero on error > > To support special case in print_linkinfo() for filtering by label we > only need to check if label pattern is given in filter.label and return > 0 to skip print_link_stats() in ipaddr_list_flush_or_save(): actual > filtering will be done in print_addrinfo(). > > Before commit 9516823051ce ("ipaddress: Improve print_linkinfo()"): > ------------------------------------------------------------------- > > $ ip addr sh label lo > 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN \ > group default qlen 1000 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > fnmatch("lo", "lo", 0) == 0 > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 > inet 127.0.0.1/8 scope host lo > valid_lft forever preferred_lft forever > inet6 ::1/128 scope host > valid_lft forever preferred_lft forever > $ ip addr show label 'lo:*' > inet 192.168.192.1/24 scope global lo:1 > valid_lft forever preferred_lft forever > $ ip addr sh label lo:1 > inet 192.168.192.1/24 scope global lo:1 > valid_lft forever preferred_lft forever > $ ip -4 addr sh label lo:1 > 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN \ > group default qlen 1000 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > filter.family == AF_INET > inet 192.168.192.1/24 scope global lo:1 > valid_lft forever preferred_lft forever > > After this change applied: > -------------------------- > > $ ip/ip addr show label lo > inet 127.0.0.1/8 scope host lo > valid_lft forever preferred_lft forever > inet6 ::1/128 scope host > valid_lft forever preferred_lft forever > $ ip/ip addr show label 'lo:*' > inet 192.168.192.1/24 scope global lo:1 > valid_lft forever preferred_lft forever > $ ip/ip addr show label lo:1 > inet 192.168.192.1/24 scope global lo:1 > valid_lft forever preferred_lft forever > $ ip/ip -4 addr show label lo:1 > inet 192.168.192.1/24 scope global lo:1 > valid_lft forever preferred_lft forever > > Note that we no longer show link information as we did previously: > we are filtering by "label" pattern, not showing by "dev". > > Fixes: commit 9516823051ce ("ipaddress: Improve print_linkinfo()") > Reported-by: Vincent Bernat <vincent@bernat.im> > Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com> Makes sense applied. Thanks for following through on this.
diff --git a/ip/ipaddress.c b/ip/ipaddress.c index 5009bfe..ea8211c 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -837,10 +837,8 @@ 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; + if (filter.label) + return 0; if (tb[IFLA_GROUP]) { int group = rta_getattr_u32(tb[IFLA_GROUP]);
Since commit 9516823051ce ("ipaddress: Improve print_linkinfo()") we return -1 instead of 0 when ip-address(8) label does not match network device name as we did before change. This causes regression when trying to output ip address matching label: # ip addr add 192.168.192.1/24 dev lo label lo:1 # ip addr show label lo:1 <no output> This is special case and return 0 from print_linkinfo() earlier to match only filter.ifindex and filter.up if given, but not rest fields in @filter. Then call print_selected_addrinfo() without calling print_link_stats() in ipaddr_list_flush_or_save(). Later print_selected_addrinfo() calls print_addrinfo() that finally matches IFA_LABEL attribute in netlink buffer with filter.label using ifa_label_match_rta(). On the other hand there is three conditions checked in print_linkinfo() to determine label special case: 1) filter.label != NULL 2) filter.family == AF_UNSPEC || filter.family == AF_PACKET 3) fnmatch(filter.label, name, 0) With 1) it is ok to check if filtering by label is on by given pattern in @filter.label. Since label is IPv4 specific and AF_PACKET is for printing ip-link(8) information (see ipaddr_link_list()::ipaddress.c as example) checking for AF_PACKET in 2) doesn't take much sense: better to defer these checks to print_addrinfo() determine valid combinations before calling ifa_label_match_rta() to finally match IFA_LABEL to pattern in filter.label. For 3) we have following call for test case: fnmatch(pattern, string, flags) -> fnmatch(filter.label, name, 0) -> fnmatch("lo:1", "lo", 0) == FNM_NOMATCH (1) or non-zero on error To support special case in print_linkinfo() for filtering by label we only need to check if label pattern is given in filter.label and return 0 to skip print_link_stats() in ipaddr_list_flush_or_save(): actual filtering will be done in print_addrinfo(). Before commit 9516823051ce ("ipaddress: Improve print_linkinfo()"): ------------------------------------------------------------------- $ ip addr sh label lo 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN \ group default qlen 1000 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ fnmatch("lo", "lo", 0) == 0 link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 inet 127.0.0.1/8 scope host lo valid_lft forever preferred_lft forever inet6 ::1/128 scope host valid_lft forever preferred_lft forever $ ip addr show label 'lo:*' inet 192.168.192.1/24 scope global lo:1 valid_lft forever preferred_lft forever $ ip addr sh label lo:1 inet 192.168.192.1/24 scope global lo:1 valid_lft forever preferred_lft forever $ ip -4 addr sh label lo:1 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN \ group default qlen 1000 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ filter.family == AF_INET inet 192.168.192.1/24 scope global lo:1 valid_lft forever preferred_lft forever After this change applied: -------------------------- $ ip/ip addr show label lo inet 127.0.0.1/8 scope host lo valid_lft forever preferred_lft forever inet6 ::1/128 scope host valid_lft forever preferred_lft forever $ ip/ip addr show label 'lo:*' inet 192.168.192.1/24 scope global lo:1 valid_lft forever preferred_lft forever $ ip/ip addr show label lo:1 inet 192.168.192.1/24 scope global lo:1 valid_lft forever preferred_lft forever $ ip/ip -4 addr show label lo:1 inet 192.168.192.1/24 scope global lo:1 valid_lft forever preferred_lft forever Note that we no longer show link information as we did previously: we are filtering by "label" pattern, not showing by "dev". Fixes: commit 9516823051ce ("ipaddress: Improve print_linkinfo()") Reported-by: Vincent Bernat <vincent@bernat.im> Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com> --- ip/ipaddress.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)