On Wed, 29 Mar 2023 21:18:26 -0700, Jakub Kicinski <kuba at kernel.org>
wrote:> On Tue, 28 Mar 2023 17:28:33 +0800 Xuan Zhuo wrote:
> > diff --git a/drivers/net/virtio/virtnet.h
b/drivers/net/virtio/virtnet.h
> > new file mode 100644
> > index 000000000000..778a0e6af869
> > --- /dev/null
> > +++ b/drivers/net/virtio/virtnet.h
> > @@ -0,0 +1,184 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef __VIRTNET_H__
> > +#define __VIRTNET_H__
> > +
> > +#include <linux/ethtool.h>
> > +#include <linux/average.h>
>
> I don't want to nit pick too much but this header is missing a lot of
> includes / forward declarations. At the same time on a quick look I
> didn't spot anything that'd require linux/ethtool.h
Indeed, I only considered the compilation may pass before, and I didn't
think
about it from the perspective of an independent header file.
Will fix.
>
> > diff --git a/drivers/net/virtio/virtnet.c
b/drivers/net/virtio/virtnet.c
> > index e2560b6f7980..5ca354e29483 100644
> > --- a/drivers/net/virtio/virtnet.c
> > +++ b/drivers/net/virtio/virtnet.c
> > @@ -6,7 +6,6 @@
> > //#define DEBUG
> > #include <linux/netdevice.h>
> > #include <linux/etherdevice.h>
> > -#include <linux/ethtool.h>
> > #include <linux/module.h>
> > #include <linux/virtio.h>
> > #include <linux/virtio_net.h>
> > @@ -16,13 +15,14 @@
> > #include <linux/if_vlan.h>
> > #include <linux/slab.h>
> > #include <linux/cpu.h>
> > -#include <linux/average.h>
> > #include <linux/filter.h>
> > #include <linux/kernel.h>
> > #include <net/route.h>
> > #include <net/xdp.h>
> > #include <net/net_failover.h>
>
> And you shouldn't remove includes if the code needs them just because
> they get pulled in indirectly.
Will fix.
Thanks.