From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vincent Bernat Subject: Re: [PATCH iproute2-next] ipaddress: fix label matching Date: Wed, 11 Jul 2018 22:26:53 +0200 Message-ID: References: <20180711113603.16589-1-vincent@bernat.im> <20180711130328.1d5bc82f@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: netdev@vger.kernel.org, serhe.popovych@gmail.com To: Stephen Hemminger Return-path: Received: from bart.luffy.cx ([78.47.78.131]:60899 "EHLO bart.luffy.cx" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732880AbeGKUiI (ORCPT ); Wed, 11 Jul 2018 16:38:08 -0400 In-Reply-To: <20180711130328.1d5bc82f@xeon-e3> (Stephen Hemminger's message of "Wed, 11 Jul 2018 13:03:28 -0700") Sender: netdev-owner@vger.kernel.org List-ID: =E2=9D=A6 11 juillet 2018 13:03 -0700, Stephen Hemminger =C2=A0: >> Since 9516823051ce, "ip addr show label lo:1" doesn't work >> anymore (doesn't show any address, despite a matching label). >> Reverting to return 0 instead of -1 fix the issue. >>=20 >> However, the condition says: "if we filter by label [...] and the >> label does NOT match the interface name". This makes little sense to >> compare the label with the interface name. There is also a logic >> around filter family being provided or not. The match against the >> label is done by ifa_label_match_rta() in print_addrinfo() and >> ipaddr_filter(). >>=20 >> Just removing the condition makes "ip addr show" works as expected >> with or without specifying a label, both when the label is matching >> and not matching. It also works if we specify a label and the label is >> the interface name. The flush operation also works as expected. >>=20 >> Fixes: 9516823051ce ("ipaddress: Improve print_linkinfo()") >> Signed-off-by: Vincent Bernat >> --- >> ip/ipaddress.c | 5 ----- >> 1 file changed, 5 deletions(-) >>=20 >> diff --git a/ip/ipaddress.c b/ip/ipaddress.c >> index 5009bfe6d2e3..20ef6724944e 100644 >> --- a/ip/ipaddress.c >> +++ b/ip/ipaddress.c >> @@ -837,11 +837,6 @@ int print_linkinfo(const struct sockaddr_nl *who, >> if (!name) >> return -1; >>=20=20 >> - if (filter.label && >> - (!filter.family || filter.family =3D=3D AF_PACKET) && >> - fnmatch(filter.label, name, 0)) >> - return -1; >> - >> if (tb[IFLA_GROUP]) { >> int group =3D rta_getattr_u32(tb[IFLA_GROUP]); >>=20 > > If this is a regression, it should go to iproute2 not iproute2-next. > > Surprised by the solution since it is removing code that was there > before the commit you referenced in Fixes. Yes, but as I explain in the commit message, the condition does not make sense for me: why would we match the label against the interface name? This code exists since a long time. --=20 The lunatic, the lover, and the poet, Are of imagination all compact... -- Wm. Shakespeare, "A Midsummer Night's Dream"