From mboxrd@z Thu Jan 1 00:00:00 1970 From: Serhey Popovych Subject: Re: [PATCH iproute2-next] ipaddress: fix label matching Date: Sat, 14 Jul 2018 11:27:37 +0300 Message-ID: <057043f9-e476-cde8-4915-8e350fc6d848@gmail.com> References: <20180711113603.16589-1-vincent@bernat.im> <18bed3ed-4ad1-4506-6b80-62af9bd9e6d2@gmail.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="geTkM4PP419DIeIUTQaQrvO8Y4JncDDtC" Cc: netdev@vger.kernel.org, stephen@networkplumber.org To: Vincent Bernat , David Ahern Return-path: Received: from mail-lf0-f66.google.com ([209.85.215.66]:34072 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726204AbeGNIqR (ORCPT ); Sat, 14 Jul 2018 04:46:17 -0400 Received: by mail-lf0-f66.google.com with SMTP id n96-v6so28909130lfi.1 for ; Sat, 14 Jul 2018 01:27:58 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --geTkM4PP419DIeIUTQaQrvO8Y4JncDDtC Content-Type: multipart/mixed; boundary="uG1PG5M6oxs4qXnwJC09rogfmNUlNydbA"; protected-headers="v1" From: Serhey Popovych To: Vincent Bernat , David Ahern Cc: netdev@vger.kernel.org, stephen@networkplumber.org Message-ID: <057043f9-e476-cde8-4915-8e350fc6d848@gmail.com> Subject: Re: [PATCH iproute2-next] ipaddress: fix label matching References: <20180711113603.16589-1-vincent@bernat.im> <18bed3ed-4ad1-4506-6b80-62af9bd9e6d2@gmail.com> In-Reply-To: --uG1PG5M6oxs4qXnwJC09rogfmNUlNydbA Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Vincent Bernat wrote: > =E2=9D=A6 11 juillet 2018 21:01 -0400, David Ahern = =C2=A0: >=20 >>> +++ b/ip/ipaddress.c >>> @@ -837,11 +837,6 @@ int print_linkinfo(const struct sockaddr_nl *who= , >>> if (!name) >>> return -1; >>> =20 >>> - if (filter.label && >>> - (!filter.family || filter.family =3D=3D AF_PACKET) && >>> - fnmatch(filter.label, name, 0)) >>> - return -1; >>> - >> >> The offending commit changed the return code: >> >> if (filter.label && >> (!filter.family || filter.family =3D=3D 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? >=20 > Yes, it works by just returning 0. The code still doesn't make sense. >=20 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) =3D=3D 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. --uG1PG5M6oxs4qXnwJC09rogfmNUlNydbA-- --geTkM4PP419DIeIUTQaQrvO8Y4JncDDtC Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQEcBAEBAgAGBQJbSbQLAAoJEBTawMmQ61bBnjUH/0i7AMfyt7/3E9QniK6YlnNR qApgaK9oodLk4eBdNo7DiUpnareP688/pPYLXMwG3QQ7gfoSZN8Q3au8tYds6DnA V0rPWEcYcUAr4lq2bMO5ZWNfTzhJBOO5sbJ1V6gYrIGXrthJxh146kWMUp6mcdOa 0fQGZS5ATVYgfJhzVXxOWPelV7mjmIP94+KWVBWYx2ONTdcACcKE7mkQCdHdLd9N BdX3qlgDIe6gXFBleXC/HEbtzToN+e3g+8SUGPq6qQwP8j9ehIfmb21GLDvzWvmA 3+C1lvcjyjonOwxGIo1OjVfIy30zABjWZLfG5DnjNxeGAyeHBW/RBQsNuhGD0Ys= =RkZR -----END PGP SIGNATURE----- --geTkM4PP419DIeIUTQaQrvO8Y4JncDDtC--