netdev - Re: [PATCH] ip: Support filter links/neighs with no master

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210824141625.dtwnvpejidy64kye@kgollan-pc>
Date:   Tue, 24 Aug 2021 17:16:26 +0300
From:   Lahav Schlesinger <lschlesinger@...venets.com>
To:     David Ahern <dsahern@...il.com>
Cc:     netdev@...r.kernel.org, dsahern@...nel.org
Subject: Re: [PATCH] ip: Support filter links/neighs with no master

On Mon, Aug 23, 2021 at 09:19:56PM -0700, David Ahern wrote:
> CAUTION: External E-Mail - Use caution with links and attachments
>
>
> On 8/19/21 4:45 AM, Lahav Schlesinger wrote:
> > Commit d3432bf10f17 ("net: Support filtering interfaces on no master")
> > in the kernel added support for filtering interfaces/neighbours that
> > have no master interface.
> >
> > This patch completes it and adds this support to iproute2:
> > 1. ip link show nomaster
> > 2. ip address show nomaster
> > 3. ip neighbour {show | flush} nomaster
> >
> > Signed-off-by: Lahav Schlesinger <lschlesinger@...venets.com>
> > ---
> >  ip/ipaddress.c           | 4 +++-
> >  ip/iplink.c              | 2 +-
> >  ip/ipneigh.c             | 4 +++-
> >  man/man8/ip-address.8.in | 7 ++++++-
> >  man/man8/ip-link.8.in    | 7 ++++++-
> >  man/man8/ip-neighbour.8  | 7 ++++++-
> >  6 files changed, 25 insertions(+), 6 deletions(-)
> >
> > diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> > index 85534aaf..a5b683f5 100644
> > --- a/ip/ipaddress.c
> > +++ b/ip/ipaddress.c
> > @@ -61,7 +61,7 @@ static void usage(void)
> >               "                            [ to PREFIX ] [ FLAG-LIST ] [ label LABEL ] [up]\n"
> >               "       ip address [ show [ dev IFNAME ] [ scope SCOPE-ID ] [ master DEVICE ]\n"
>
> move [ nomaster ] to here on a new line to keep the existing line
> length, and
>
> >               "                         [ type TYPE ] [ to PREFIX ] [ FLAG-LIST ]\n"
> > -             "                         [ label LABEL ] [up] [ vrf NAME ] ]\n"
> > +             "                         [ label LABEL ] [up] [ vrf NAME ] [ nomaster ] ]\n"
>
> make this 'novrf' for consistency with existing syntax.
>
> Similarly for the other 2 commands.

I think "nomaster" is more fitting here, because this option only affects
interfaces that have no master at all, so e.g. slaves of a bundle will
not be returned by the "nomaster" option, even if they are in the default VRF.

I'm planning next to add support for the "novrf" option which will indeed
only affect interfaces which are in the default VRF, even if they have a
master.
>
> >               "       ip address {showdump|restore}\n"
> >               "IFADDR := PREFIX | ADDR peer PREFIX\n"
> >               "          [ broadcast ADDR ] [ anycast ADDR ]\n"
> > @@ -2123,6 +2123,8 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
> >                       if (!name_is_vrf(*argv))
> >                               invarg("Not a valid VRF name\n", *argv);
> >                       filter.master = ifindex;
> > +             } else if (strcmp(*argv, "nomaster") == 0) {
>
> and of course make this a compound check for novrf.
>
> > +                     filter.master = -1;
> >               } else if (strcmp(*argv, "type") == 0) {
> >                       int soff;
> >
> > diff --git a/ip/iplink.c b/ip/iplink.c
> > index 18b2ea25..f017f1f3 100644
> > --- a/ip/iplink.c
> > +++ b/ip/iplink.c
> > @@ -119,7 +119,7 @@ void iplink_usage(void)
> >               "               [ protodown_reason PREASON { on | off } ]\n"
> >               "               [ gso_max_size BYTES ] | [ gso_max_segs PACKETS ]\n"
> >               "\n"
> > -             "       ip link show [ DEVICE | group GROUP ] [up] [master DEV] [vrf NAME] [type TYPE]\n"
> > +             "       ip link show [ DEVICE | group GROUP ] [up] [master DEV] [vrf NAME] [type TYPE] [nomaster]\n"
>
> this line is already too long so add the new options on a new line.
>
> >               "\n"
> >               "       ip link xstats type TYPE [ ARGS ]\n"
> >               "\n"
> > diff --git a/ip/ipneigh.c b/ip/ipneigh.c
> > index 95bde520..b4a2f6df 100644
> > --- a/ip/ipneigh.c
> > +++ b/ip/ipneigh.c
> > @@ -54,7 +54,7 @@ static void usage(void)
> >               "               [ dev DEV ] [ router ] [ extern_learn ] [ protocol PROTO ]\n"
> >               "\n"
> >               "       ip neigh { show | flush } [ proxy ] [ to PREFIX ] [ dev DEV ] [ nud STATE ]\n"
> > -             "                                 [ vrf NAME ]\n"
> > +             "                                 [ vrf NAME ] [ nomaster ]\n"
> >               "       ip neigh get { ADDR | proxy ADDR } dev DEV\n"
> >               "\n"
> >               "STATE := { delay | failed | incomplete | noarp | none |\n"
> > @@ -536,6 +536,8 @@ static int do_show_or_flush(int argc, char **argv, int flush)
> >                       if (!name_is_vrf(*argv))
> >                               invarg("Not a valid VRF name\n", *argv);
> >                       filter.master = ifindex;
> > +             } else if (strcmp(*argv, "nomaster") == 0) {
> > +                     filter.master = -1;
> >               } else if (strcmp(*argv, "unused") == 0) {
> >                       filter.unused_only = 1;
> >               } else if (strcmp(*argv, "nud") == 0) {
>
>