netdev - Re: [PATCH net-next] show rx_otherhost

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: <CACB8nPkQkH3fJt29kNQ_YqikP8eKPSuBJvh-_cFO_zqie2rw0A@mail.gmail.com>
Date:   Wed, 18 May 2022 07:29:08 -1000
From:   Jeffrey Ji <jeffreyjilinux@...il.com>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     David Ahern <dsahern@...il.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Brian Vazquez <brianvv@...gle.com>, netdev@...r.kernel.org,
        Jeffrey Ji <jeffreyji@...gle.com>
Subject: Re: [PATCH net-next] show rx_otherhost_dropped stat in ip link show

I thought we wanted to avoid otherhost_dropped being counted as an
error, I recall Jakub saying something about not wanting users to call
for help when they see error in the counter name.


On Mon, May 9, 2022 at 9:09 PM Ido Schimmel <idosch@...sch.org> wrote:
>
> On Mon, May 09, 2022 at 07:18:10PM +0000, Jeffrey Ji wrote:
> > From: Jeffrey Ji <jeffreyji@...gle.com>
> >
> > This stat was added in commit 794c24e9921f ("net-core: rx_otherhost_dropped to core_stats")
> >
> > Tested: sent packet with wrong MAC address from 1
> > network namespace to another, verified that counter showed "1" in
> > `ip -s -s link sh` and `ip -s -s -j link sh`
> >
> > Signed-off-by: Jeffrey Ji <jeffreyji@...gle.com>
> > ---
> >  include/uapi/linux/if_link.h |  2 ++
> >  ip/ipaddress.c               | 15 +++++++++++++--
> >  2 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> > index 22e21e57afc9..50477985bfea 100644
> > --- a/include/uapi/linux/if_link.h
> > +++ b/include/uapi/linux/if_link.h
> > @@ -243,6 +243,8 @@ struct rtnl_link_stats64 {
> >       __u64   rx_compressed;
> >       __u64   tx_compressed;
> >       __u64   rx_nohandler;
> > +
> > +     __u64   rx_otherhost_dropped;
>
> I believe you need to rebase against current iproute2-next. The kernel
> headers are already updated there. This tree:
> https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git
>
> >  };
> >
> >  /* Subset of link stats useful for in-HW collection. Meaning of the fields is as
> > diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> > index a80996efdc28..9d6af56e2a72 100644
> > --- a/ip/ipaddress.c
> > +++ b/ip/ipaddress.c
> > @@ -692,6 +692,7 @@ static void __print_link_stats(FILE *fp, struct rtattr *tb[])
> >               strlen("heartbt"),
> >               strlen("overrun"),
> >               strlen("compressed"),
> > +             strlen("otherhost_dropped"),
>
> There were a lot of changes in this area as part of the "ip stats"
> work. See print_stats64() in current iproute2-next.
>
> >       };
> >       int ret;
> >
> > @@ -713,6 +714,10 @@ static void __print_link_stats(FILE *fp, struct rtattr *tb[])
> >               if (s->rx_compressed)
> >                       print_u64(PRINT_JSON,
> >                                  "compressed", NULL, s->rx_compressed);
> > +             if (s->rx_otherhost_dropped)
> > +                     print_u64(PRINT_JSON,
> > +                                "otherhost_dropped",
> > +                                NULL, s->rx_otherhost_dropped);
> >
> >               /* RX error stats */
> >               if (show_stats > 1) {
> > @@ -795,11 +800,15 @@ static void __print_link_stats(FILE *fp, struct rtattr *tb[])
> >                                    rta_getattr_u32(carrier_changes) : 0);
> >
> >               /* RX stats */
> > -             fprintf(fp, "    RX: %*s %*s %*s %*s %*s %*s %*s%s",
> > +             fprintf(fp, "    RX: %*s %*s %*s %*s %*s %*s %*s%*s%s",
> >                       cols[0] - 4, "bytes", cols[1], "packets",
> >                       cols[2], "errors", cols[3], "dropped",
> >                       cols[4], "missed", cols[5], "mcast",
> > -                     cols[6], s->rx_compressed ? "compressed" : "", _SL_);
> > +                     s->rx_compressed ? cols[6] : 0,
> > +                     s->rx_compressed ? "compressed " : "",
> > +                     s->rx_otherhost_dropped ? cols[7] : 0,
> > +                     s->rx_otherhost_dropped ? "otherhost_dropped" : "",
> > +                     _SL_);
> >
> >               fprintf(fp, "    ");
> >               print_num(fp, cols[0], s->rx_bytes);
> > @@ -810,6 +819,8 @@ static void __print_link_stats(FILE *fp, struct rtattr *tb[])
> >               print_num(fp, cols[5], s->multicast);
> >               if (s->rx_compressed)
> >                       print_num(fp, cols[6], s->rx_compressed);
> > +             if (s->rx_otherhost_dropped)
> > +                     print_num(fp, cols[7], s->rx_otherhost_dropped);
> >
> >               /* RX error stats */
> >               if (show_stats > 1) {
> > --
> > 2.36.0.512.ge40c2bad7a-goog
> >