2022-07-08 06:45:23

by Yajun Deng

[permalink] [raw]
Subject: [PATCH net-next] net: rtnetlink: add rx_otherhost_dropped for struct rtnl_link_stats

The commit 794c24e9921f ("net-core: rx_otherhost_dropped to core_stats")
introduce rx_otherhost_dropped, add rx_otherhost_dropped for struct
rtnl_link_stats to keep sync with struct rtnl_link_stats64.

As the same time, add BUILD_BUG_ON() in copy_rtnl_link_stats().

Signed-off-by: Yajun Deng <[email protected]>
---
include/uapi/linux/if_link.h | 1 +
net/core/rtnetlink.c | 36 +++++++-----------------------------
2 files changed, 8 insertions(+), 29 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index e36d9d2c65a7..fd6776d665c8 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -37,6 +37,7 @@ struct rtnl_link_stats {
__u32 tx_compressed;

__u32 rx_nohandler;
+ __u32 rx_otherhost_dropped;
};

/**
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index ac45328607f7..818649850b2c 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -908,35 +908,13 @@ static unsigned int rtnl_dev_combine_flags(const struct net_device *dev,
static void copy_rtnl_link_stats(struct rtnl_link_stats *a,
const struct rtnl_link_stats64 *b)
{
- a->rx_packets = b->rx_packets;
- a->tx_packets = b->tx_packets;
- a->rx_bytes = b->rx_bytes;
- a->tx_bytes = b->tx_bytes;
- a->rx_errors = b->rx_errors;
- a->tx_errors = b->tx_errors;
- a->rx_dropped = b->rx_dropped;
- a->tx_dropped = b->tx_dropped;
-
- a->multicast = b->multicast;
- a->collisions = b->collisions;
-
- a->rx_length_errors = b->rx_length_errors;
- a->rx_over_errors = b->rx_over_errors;
- a->rx_crc_errors = b->rx_crc_errors;
- a->rx_frame_errors = b->rx_frame_errors;
- a->rx_fifo_errors = b->rx_fifo_errors;
- a->rx_missed_errors = b->rx_missed_errors;
-
- a->tx_aborted_errors = b->tx_aborted_errors;
- a->tx_carrier_errors = b->tx_carrier_errors;
- a->tx_fifo_errors = b->tx_fifo_errors;
- a->tx_heartbeat_errors = b->tx_heartbeat_errors;
- a->tx_window_errors = b->tx_window_errors;
-
- a->rx_compressed = b->rx_compressed;
- a->tx_compressed = b->tx_compressed;
-
- a->rx_nohandler = b->rx_nohandler;
+ size_t i, n = sizeof(*b) / sizeof(u64);
+ const u64 *src = (const u64 *)b;
+ u32 *dst = (u32 *)a;
+
+ BUILD_BUG_ON(n != sizeof(*a) / sizeof(u32));
+ for (i = 0; i < n; i++)
+ dst[i] = src[i];
}

/* All VF info */
--
2.25.1


2022-07-08 07:12:44

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH net-next] net: rtnetlink: add rx_otherhost_dropped for struct rtnl_link_stats

On Fri, Jul 8, 2022 at 8:33 AM Yajun Deng <[email protected]> wrote:
>
> The commit 794c24e9921f ("net-core: rx_otherhost_dropped to core_stats")
> introduce rx_otherhost_dropped, add rx_otherhost_dropped for struct
> rtnl_link_stats to keep sync with struct rtnl_link_stats64.
>
> As the same time, add BUILD_BUG_ON() in copy_rtnl_link_stats().
>

Any reason you chose to not cc the original patch author ?

If I remember well, not adding fields into legacy 'struct
rtnl_link_stats' was a conscious decision.

There is no requirement to keep rtnl_link_stats & rtnl_link_stats64 in sync.
rtnl_link_stats is deprecated, user space really wants to use
rtnl_link_stats64 instead,
if they need access to new fields added in recent kernels.

Thank you.

[1]
commit 9256645af09807bc52fa8b2e66ecd28ab25318c4
Author: Jarod Wilson <[email protected]>
Date: Mon Feb 1 18:51:04 2016 -0500

net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64

The netdev_stats_to_stats64 function copies the deprecated
net_device_stats format stats into rtnl_link_stats64 for legacy support
purposes, but with the BUILD_BUG_ON as it was, it wasn't possible to
extend rtnl_link_stats64 without also extending net_device_stats. Relax
the BUILD_BUG_ON to only require that rtnl_link_stats64 is larger, and
zero out all the stat counters that aren't present in net_device_stats.

CC: Eric Dumazet <[email protected]>
CC: [email protected]
Signed-off-by: Jarod Wilson <[email protected]>
Signed-off-by: David S. Miller <[email protected]>

> Signed-off-by: Yajun Deng <[email protected]>
> ---
> include/uapi/linux/if_link.h | 1 +
> net/core/rtnetlink.c | 36 +++++++-----------------------------
> 2 files changed, 8 insertions(+), 29 deletions(-)
>
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index e36d9d2c65a7..fd6776d665c8 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -37,6 +37,7 @@ struct rtnl_link_stats {
> __u32 tx_compressed;
>
> __u32 rx_nohandler;
> + __u32 rx_otherhost_dropped;
> };
>
> /**
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index ac45328607f7..818649850b2c 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -908,35 +908,13 @@ static unsigned int rtnl_dev_combine_flags(const struct net_device *dev,
> static void copy_rtnl_link_stats(struct rtnl_link_stats *a,
> const struct rtnl_link_stats64 *b)
> {
> - a->rx_packets = b->rx_packets;
> - a->tx_packets = b->tx_packets;
> - a->rx_bytes = b->rx_bytes;
> - a->tx_bytes = b->tx_bytes;
> - a->rx_errors = b->rx_errors;
> - a->tx_errors = b->tx_errors;
> - a->rx_dropped = b->rx_dropped;
> - a->tx_dropped = b->tx_dropped;
> -
> - a->multicast = b->multicast;
> - a->collisions = b->collisions;
> -
> - a->rx_length_errors = b->rx_length_errors;
> - a->rx_over_errors = b->rx_over_errors;
> - a->rx_crc_errors = b->rx_crc_errors;
> - a->rx_frame_errors = b->rx_frame_errors;
> - a->rx_fifo_errors = b->rx_fifo_errors;
> - a->rx_missed_errors = b->rx_missed_errors;
> -
> - a->tx_aborted_errors = b->tx_aborted_errors;
> - a->tx_carrier_errors = b->tx_carrier_errors;
> - a->tx_fifo_errors = b->tx_fifo_errors;
> - a->tx_heartbeat_errors = b->tx_heartbeat_errors;
> - a->tx_window_errors = b->tx_window_errors;
> -
> - a->rx_compressed = b->rx_compressed;
> - a->tx_compressed = b->tx_compressed;
> -
> - a->rx_nohandler = b->rx_nohandler;
> + size_t i, n = sizeof(*b) / sizeof(u64);
> + const u64 *src = (const u64 *)b;
> + u32 *dst = (u32 *)a;
> +
> + BUILD_BUG_ON(n != sizeof(*a) / sizeof(u32));
> + for (i = 0; i < n; i++)
> + dst[i] = src[i];
> }
>
> /* All VF info */
> --
> 2.25.1
>

2022-07-08 07:54:38

by Yajun Deng

[permalink] [raw]
Subject: Re: [PATCH net-next] net: rtnetlink: add rx_otherhost_dropped for struct rtnl_link_stats

July 8, 2022 3:03 PM, "Eric Dumazet" <[email protected]> wrote:

> On Fri, Jul 8, 2022 at 8:33 AM Yajun Deng <[email protected]> wrote:
>
>> The commit 794c24e9921f ("net-core: rx_otherhost_dropped to core_stats")
>> introduce rx_otherhost_dropped, add rx_otherhost_dropped for struct
>> rtnl_link_stats to keep sync with struct rtnl_link_stats64.
>>
>> As the same time, add BUILD_BUG_ON() in copy_rtnl_link_stats().
>
> Any reason you chose to not cc the original patch author ?
>
Sorry for that, I'll do it later.

> If I remember well, not adding fields into legacy 'struct
> rtnl_link_stats' was a conscious decision.
>
> There is no requirement to keep rtnl_link_stats & rtnl_link_stats64 in sync.
> rtnl_link_stats is deprecated, user space really wants to use
> rtnl_link_stats64 instead,
> if they need access to new fields added in recent kernels.
>
Got it. Thanks.

> Thank you.
>
> [1]
> commit 9256645af09807bc52fa8b2e66ecd28ab25318c4
> Author: Jarod Wilson <[email protected]>
> Date: Mon Feb 1 18:51:04 2016 -0500
>
> net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64
>
> The netdev_stats_to_stats64 function copies the deprecated
> net_device_stats format stats into rtnl_link_stats64 for legacy support
> purposes, but with the BUILD_BUG_ON as it was, it wasn't possible to
> extend rtnl_link_stats64 without also extending net_device_stats. Relax
> the BUILD_BUG_ON to only require that rtnl_link_stats64 is larger, and
> zero out all the stat counters that aren't present in net_device_stats.
>
> CC: Eric Dumazet <[email protected]>
> CC: [email protected]
> Signed-off-by: Jarod Wilson <[email protected]>
> Signed-off-by: David S. Miller <[email protected]>
>
>> Signed-off-by: Yajun Deng <[email protected]>
>> ---
>> include/uapi/linux/if_link.h | 1 +
>> net/core/rtnetlink.c | 36 +++++++-----------------------------
>> 2 files changed, 8 insertions(+), 29 deletions(-)
>>
>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>> index e36d9d2c65a7..fd6776d665c8 100644
>> --- a/include/uapi/linux/if_link.h
>> +++ b/include/uapi/linux/if_link.h
>> @@ -37,6 +37,7 @@ struct rtnl_link_stats {
>> __u32 tx_compressed;
>>
>> __u32 rx_nohandler;
>> + __u32 rx_otherhost_dropped;
>> };
>>
>> /**
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index ac45328607f7..818649850b2c 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -908,35 +908,13 @@ static unsigned int rtnl_dev_combine_flags(const struct net_device *dev,
>> static void copy_rtnl_link_stats(struct rtnl_link_stats *a,
>> const struct rtnl_link_stats64 *b)
>> {
>> - a->rx_packets = b->rx_packets;
>> - a->tx_packets = b->tx_packets;
>> - a->rx_bytes = b->rx_bytes;
>> - a->tx_bytes = b->tx_bytes;
>> - a->rx_errors = b->rx_errors;
>> - a->tx_errors = b->tx_errors;
>> - a->rx_dropped = b->rx_dropped;
>> - a->tx_dropped = b->tx_dropped;
>> -
>> - a->multicast = b->multicast;
>> - a->collisions = b->collisions;
>> -
>> - a->rx_length_errors = b->rx_length_errors;
>> - a->rx_over_errors = b->rx_over_errors;
>> - a->rx_crc_errors = b->rx_crc_errors;
>> - a->rx_frame_errors = b->rx_frame_errors;
>> - a->rx_fifo_errors = b->rx_fifo_errors;
>> - a->rx_missed_errors = b->rx_missed_errors;
>> -
>> - a->tx_aborted_errors = b->tx_aborted_errors;
>> - a->tx_carrier_errors = b->tx_carrier_errors;
>> - a->tx_fifo_errors = b->tx_fifo_errors;
>> - a->tx_heartbeat_errors = b->tx_heartbeat_errors;
>> - a->tx_window_errors = b->tx_window_errors;
>> -
>> - a->rx_compressed = b->rx_compressed;
>> - a->tx_compressed = b->tx_compressed;
>> -
>> - a->rx_nohandler = b->rx_nohandler;
>> + size_t i, n = sizeof(*b) / sizeof(u64);
>> + const u64 *src = (const u64 *)b;
>> + u32 *dst = (u32 *)a;
>> +
>> + BUILD_BUG_ON(n != sizeof(*a) / sizeof(u32));
>> + for (i = 0; i < n; i++)
>> + dst[i] = src[i];
>> }
>>
>> /* All VF info */
>> --
>> 2.25.1