| Message ID | 20220708063257.1192311-1-yajun.deng@linux.dev (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: rtnetlink: add rx_otherhost_dropped for struct rtnl_link_stats | expand |
On Fri, Jul 8, 2022 at 8:33 AM Yajun Deng <yajun.deng@linux.dev> 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 <jarod@redhat.com> 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 <edumazet@google.com> CC: netdev@vger.kernel.org Signed-off-by: Jarod Wilson <jarod@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> > Signed-off-by: Yajun Deng <yajun.deng@linux.dev> > --- > 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 >
July 8, 2022 3:03 PM, "Eric Dumazet" <edumazet@google.com> wrote: > On Fri, Jul 8, 2022 at 8:33 AM Yajun Deng <yajun.deng@linux.dev> 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 <jarod@redhat.com> > 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 <edumazet@google.com> > CC: netdev@vger.kernel.org > Signed-off-by: Jarod Wilson <jarod@redhat.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > >> Signed-off-by: Yajun Deng <yajun.deng@linux.dev> >> --- >> 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
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 */
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 <yajun.deng@linux.dev> --- include/uapi/linux/if_link.h | 1 + net/core/rtnetlink.c | 36 +++++++----------------------------- 2 files changed, 8 insertions(+), 29 deletions(-)