* [PATCH net-next] show rx_otherhost_dropped stat in ip link show
@ 2022-05-09 19:18 Jeffrey Ji
2022-05-09 20:10 ` Brian Vazquez
2022-05-10 7:09 ` Ido Schimmel
0 siblings, 2 replies; 6+ messages in thread
From: Jeffrey Ji @ 2022-05-09 19:18 UTC (permalink / raw)
To: David Ahern, Stephen Hemminger
Cc: Eric Dumazet, Brian Vazquez, netdev, Jeffrey Ji
From: Jeffrey Ji <jeffreyji@google.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@google.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;
};
/* 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"),
};
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] show rx_otherhost_dropped stat in ip link show
2022-05-09 19:18 [PATCH net-next] show rx_otherhost_dropped stat in ip link show Jeffrey Ji
@ 2022-05-09 20:10 ` Brian Vazquez
2022-05-10 7:09 ` Ido Schimmel
1 sibling, 0 replies; 6+ messages in thread
From: Brian Vazquez @ 2022-05-09 20:10 UTC (permalink / raw)
To: Jeffrey Ji
Cc: David Ahern, Stephen Hemminger, Eric Dumazet, netdev, Jeffrey Ji
Hey Jeffrey, thanks for working on this, some comments:
I think you meant iproute2-next in the subject. Could you resend, please?
On Mon, May 9, 2022 at 12:18 PM Jeffrey Ji <jeffreyjilinux@gmail.com> wrote:
>
> From: Jeffrey Ji <jeffreyji@google.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@google.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;
> };
>
> /* 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"),
> };
> 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",
I think you're missing a space in the line above (but code shouldn't
be here, see my comment below)
> 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_);
rx_otherhost_dropped code should be below in the "RX error stats" not
here, it should be after the rx_nohandler stat. Also IIUC, the code
should be:
cols[7], s->rx_otherhost_dropped? "otherhost_dropped" : "",
>
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] show rx_otherhost_dropped stat in ip link show
2022-05-09 19:18 [PATCH net-next] show rx_otherhost_dropped stat in ip link show Jeffrey Ji
2022-05-09 20:10 ` Brian Vazquez
@ 2022-05-10 7:09 ` Ido Schimmel
2022-05-18 17:29 ` Jeffrey Ji
1 sibling, 1 reply; 6+ messages in thread
From: Ido Schimmel @ 2022-05-10 7:09 UTC (permalink / raw)
To: Jeffrey Ji
Cc: David Ahern, Stephen Hemminger, Eric Dumazet, Brian Vazquez,
netdev, Jeffrey Ji
On Mon, May 09, 2022 at 07:18:10PM +0000, Jeffrey Ji wrote:
> From: Jeffrey Ji <jeffreyji@google.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@google.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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] show rx_otherhost_dropped stat in ip link show
2022-05-10 7:09 ` Ido Schimmel
@ 2022-05-18 17:29 ` Jeffrey Ji
2022-05-18 19:39 ` Jakub Kicinski
2022-05-18 20:00 ` Stephen Hemminger
0 siblings, 2 replies; 6+ messages in thread
From: Jeffrey Ji @ 2022-05-18 17:29 UTC (permalink / raw)
To: Ido Schimmel
Cc: David Ahern, Stephen Hemminger, Eric Dumazet, Brian Vazquez,
netdev, Jeffrey Ji
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@idosch.org> wrote:
>
> On Mon, May 09, 2022 at 07:18:10PM +0000, Jeffrey Ji wrote:
> > From: Jeffrey Ji <jeffreyji@google.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@google.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
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] show rx_otherhost_dropped stat in ip link show
2022-05-18 17:29 ` Jeffrey Ji
@ 2022-05-18 19:39 ` Jakub Kicinski
2022-05-18 20:00 ` Stephen Hemminger
1 sibling, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2022-05-18 19:39 UTC (permalink / raw)
To: Jeffrey Ji
Cc: Ido Schimmel, David Ahern, Stephen Hemminger, Eric Dumazet,
Brian Vazquez, netdev, Jeffrey Ji
On Wed, 18 May 2022 07:29:08 -1000 Jeffrey Ji wrote:
> 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.
You should probably set up a normal email client if you want to
continue working on the kernel, you replied to the wrong message :S
Yes, displaying the otherhost as an error could be too alarming.
That said the split between the main RX: line and RX errors: in
ip -s -s is somewhat arbitrary so no strong preference here.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] show rx_otherhost_dropped stat in ip link show
2022-05-18 17:29 ` Jeffrey Ji
2022-05-18 19:39 ` Jakub Kicinski
@ 2022-05-18 20:00 ` Stephen Hemminger
1 sibling, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2022-05-18 20:00 UTC (permalink / raw)
To: Jeffrey Ji
Cc: Ido Schimmel, David Ahern, Eric Dumazet, Brian Vazquez, netdev,
Jeffrey Ji
On Wed, 18 May 2022 07:29:08 -1000
Jeffrey Ji <jeffreyjilinux@gmail.com> wrote:
> > > /* 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" : "",
This belongs in the detail part not in the common stats.
Look where nohandler etc errors are.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-18 20:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-09 19:18 [PATCH net-next] show rx_otherhost_dropped stat in ip link show Jeffrey Ji
2022-05-09 20:10 ` Brian Vazquez
2022-05-10 7:09 ` Ido Schimmel
2022-05-18 17:29 ` Jeffrey Ji
2022-05-18 19:39 ` Jakub Kicinski
2022-05-18 20:00 ` Stephen Hemminger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.