tun and tap implements the same vnet-related features so reuse the code.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
drivers/net/Kconfig | 1 +
drivers/net/Makefile | 6 +-
drivers/net/tap.c | 152 +++++--------------------------------------------
drivers/net/tun_vnet.c | 5 ++
4 files changed, 24 insertions(+), 140 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 1fd5acdc73c6..c420418473fc 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -395,6 +395,7 @@ config TUN
tristate "Universal TUN/TAP device driver support"
depends on INET
select CRC32
+ select TAP
help
TUN/TAP provides packet reception and transmission for user space
programs. It can be viewed as a simple Point-to-Point or Ethernet
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index bb8eb3053772..2275309a97ee 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -29,9 +29,9 @@ obj-y += mdio/
obj-y += pcs/
obj-$(CONFIG_RIONET) += rionet.o
obj-$(CONFIG_NET_TEAM) += team/
-obj-$(CONFIG_TUN) += tun-drv.o
-tun-drv-y := tun.o tun_vnet.o
-obj-$(CONFIG_TAP) += tap.o
+obj-$(CONFIG_TUN) += tun.o
+obj-$(CONFIG_TAP) += tap-drv.o
+tap-drv-y := tap.o tun_vnet.o
obj-$(CONFIG_VETH) += veth.o
obj-$(CONFIG_VIRTIO_NET) += virtio_net.o
obj-$(CONFIG_VXLAN) += vxlan/
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 7ee2e9ee2a89..4f3cc3b2e3c6 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -26,74 +26,9 @@
#include <linux/virtio_net.h>
#include <linux/skb_array.h>
-#define TAP_IFFEATURES (IFF_VNET_HDR | IFF_MULTI_QUEUE)
-
-#define TAP_VNET_LE 0x80000000
-#define TAP_VNET_BE 0x40000000
-
-#ifdef CONFIG_TUN_VNET_CROSS_LE
-static inline bool tap_legacy_is_little_endian(struct tap_queue *q)
-{
- return q->flags & TAP_VNET_BE ? false :
- virtio_legacy_is_little_endian();
-}
-
-static long tap_get_vnet_be(struct tap_queue *q, int __user *sp)
-{
- int s = !!(q->flags & TAP_VNET_BE);
-
- if (put_user(s, sp))
- return -EFAULT;
-
- return 0;
-}
-
-static long tap_set_vnet_be(struct tap_queue *q, int __user *sp)
-{
- int s;
-
- if (get_user(s, sp))
- return -EFAULT;
-
- if (s)
- q->flags |= TAP_VNET_BE;
- else
- q->flags &= ~TAP_VNET_BE;
-
- return 0;
-}
-#else
-static inline bool tap_legacy_is_little_endian(struct tap_queue *q)
-{
- return virtio_legacy_is_little_endian();
-}
-
-static long tap_get_vnet_be(struct tap_queue *q, int __user *argp)
-{
- return -EINVAL;
-}
-
-static long tap_set_vnet_be(struct tap_queue *q, int __user *argp)
-{
- return -EINVAL;
-}
-#endif /* CONFIG_TUN_VNET_CROSS_LE */
-
-static inline bool tap_is_little_endian(struct tap_queue *q)
-{
- return q->flags & TAP_VNET_LE ||
- tap_legacy_is_little_endian(q);
-}
-
-static inline u16 tap16_to_cpu(struct tap_queue *q, __virtio16 val)
-{
- return __virtio16_to_cpu(tap_is_little_endian(q), val);
-}
+#include "tun_vnet.h"
-static inline __virtio16 cpu_to_tap16(struct tap_queue *q, u16 val)
-{
- return __cpu_to_virtio16(tap_is_little_endian(q), val);
-}
+#define TAP_IFFEATURES (IFF_VNET_HDR | IFF_MULTI_QUEUE)
static struct proto tap_proto = {
.name = "tap",
@@ -655,25 +590,11 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
if (q->flags & IFF_VNET_HDR) {
vnet_hdr_len = READ_ONCE(q->vnet_hdr_sz);
- err = -EINVAL;
- if (iov_iter_count(from) < vnet_hdr_len)
- goto err;
-
- err = -EFAULT;
- if (!copy_from_iter_full(&vnet_hdr, sizeof(vnet_hdr), from))
- goto err;
- iov_iter_advance(from, vnet_hdr_len - sizeof(vnet_hdr));
- if ((vnet_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
- tap16_to_cpu(q, vnet_hdr.csum_start) +
- tap16_to_cpu(q, vnet_hdr.csum_offset) + 2 >
- tap16_to_cpu(q, vnet_hdr.hdr_len))
- vnet_hdr.hdr_len = cpu_to_tap16(q,
- tap16_to_cpu(q, vnet_hdr.csum_start) +
- tap16_to_cpu(q, vnet_hdr.csum_offset) + 2);
- err = -EINVAL;
- if (tap16_to_cpu(q, vnet_hdr.hdr_len) > iov_iter_count(from))
+ hdr_len = tun_vnet_hdr_get(vnet_hdr_len, q->flags, from, &vnet_hdr);
+ if (hdr_len < 0) {
+ err = hdr_len;
goto err;
- hdr_len = tap16_to_cpu(q, vnet_hdr.hdr_len);
+ }
}
len = iov_iter_count(from);
@@ -731,8 +652,7 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control,
skb->dev = tap->dev;
if (vnet_hdr_len) {
- err = virtio_net_hdr_to_skb(skb, &vnet_hdr,
- tap_is_little_endian(q));
+ err = tun_vnet_hdr_to_skb(q->flags, skb, &vnet_hdr);
if (err) {
rcu_read_unlock();
drop_reason = SKB_DROP_REASON_DEV_HDR;
@@ -795,23 +715,17 @@ static ssize_t tap_put_user(struct tap_queue *q,
int total;
if (q->flags & IFF_VNET_HDR) {
- int vlan_hlen = skb_vlan_tag_present(skb) ? VLAN_HLEN : 0;
struct virtio_net_hdr vnet_hdr;
vnet_hdr_len = READ_ONCE(q->vnet_hdr_sz);
- if (iov_iter_count(iter) < vnet_hdr_len)
- return -EINVAL;
- if (virtio_net_hdr_from_skb(skb, &vnet_hdr,
- tap_is_little_endian(q), true,
- vlan_hlen))
- BUG();
-
- if (copy_to_iter(&vnet_hdr, sizeof(vnet_hdr), iter) !=
- sizeof(vnet_hdr))
- return -EFAULT;
+ ret = tun_vnet_hdr_from_skb(q->flags, NULL, skb, &vnet_hdr);
+ if (ret)
+ return ret;
- iov_iter_advance(iter, vnet_hdr_len - sizeof(vnet_hdr));
+ ret = tun_vnet_hdr_put(vnet_hdr_len, iter, &vnet_hdr);
+ if (ret)
+ return ret;
}
total = vnet_hdr_len;
total += skb->len;
@@ -1070,42 +984,6 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
q->sk.sk_sndbuf = s;
return 0;
- case TUNGETVNETHDRSZ:
- s = q->vnet_hdr_sz;
- if (put_user(s, sp))
- return -EFAULT;
- return 0;
-
- case TUNSETVNETHDRSZ:
- if (get_user(s, sp))
- return -EFAULT;
- if (s < (int)sizeof(struct virtio_net_hdr))
- return -EINVAL;
-
- q->vnet_hdr_sz = s;
- return 0;
-
- case TUNGETVNETLE:
- s = !!(q->flags & TAP_VNET_LE);
- if (put_user(s, sp))
- return -EFAULT;
- return 0;
-
- case TUNSETVNETLE:
- if (get_user(s, sp))
- return -EFAULT;
- if (s)
- q->flags |= TAP_VNET_LE;
- else
- q->flags &= ~TAP_VNET_LE;
- return 0;
-
- case TUNGETVNETBE:
- return tap_get_vnet_be(q, sp);
-
- case TUNSETVNETBE:
- return tap_set_vnet_be(q, sp);
-
case TUNSETOFFLOAD:
/* let the user check for future flags */
if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
@@ -1149,7 +1027,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
return ret;
default:
- return -EINVAL;
+ return tun_vnet_ioctl(&q->vnet_hdr_sz, &q->flags, cmd, sp);
}
}
@@ -1196,7 +1074,7 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp)
skb->protocol = eth_hdr(skb)->h_proto;
if (vnet_hdr_len) {
- err = virtio_net_hdr_to_skb(skb, gso, tap_is_little_endian(q));
+ err = tun_vnet_hdr_to_skb(q->flags, skb, gso);
if (err)
goto err_kfree;
}
diff --git a/drivers/net/tun_vnet.c b/drivers/net/tun_vnet.c
index 5a6cbfb6eed0..960a5fa5a332 100644
--- a/drivers/net/tun_vnet.c
+++ b/drivers/net/tun_vnet.c
@@ -104,6 +104,7 @@ long tun_vnet_ioctl(int *sz, unsigned int *flags,
return -EINVAL;
}
}
+EXPORT_SYMBOL_GPL(tun_vnet_ioctl);
int tun_vnet_hdr_get(int sz, unsigned int flags, struct iov_iter *from,
struct virtio_net_hdr *hdr)
@@ -125,6 +126,7 @@ int tun_vnet_hdr_get(int sz, unsigned int flags, struct iov_iter *from,
return tun16_to_cpu(flags, hdr->hdr_len);
}
+EXPORT_SYMBOL_GPL(tun_vnet_hdr_get);
int tun_vnet_hdr_put(int sz, struct iov_iter *iter,
const struct virtio_net_hdr *hdr)
@@ -139,12 +141,14 @@ int tun_vnet_hdr_put(int sz, struct iov_iter *iter,
return 0;
}
+EXPORT_SYMBOL_GPL(tun_vnet_hdr_put);
int tun_vnet_hdr_to_skb(unsigned int flags, struct sk_buff *skb,
const struct virtio_net_hdr *hdr)
{
return virtio_net_hdr_to_skb(skb, hdr, tun_is_little_endian(flags));
}
+EXPORT_SYMBOL_GPL(tun_vnet_hdr_to_skb);
int tun_vnet_hdr_from_skb(unsigned int flags,
const struct net_device *dev,
@@ -173,3 +177,4 @@ int tun_vnet_hdr_from_skb(unsigned int flags,
return 0;
}
+EXPORT_SYMBOL_GPL(tun_vnet_hdr_from_skb);
--
2.47.1Akihiko Odaki wrote: > tun and tap implements the same vnet-related features so reuse the code. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > --- > drivers/net/Kconfig | 1 + > drivers/net/Makefile | 6 +- > drivers/net/tap.c | 152 +++++-------------------------------------------- > drivers/net/tun_vnet.c | 5 ++ > 4 files changed, 24 insertions(+), 140 deletions(-) > > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > index 1fd5acdc73c6..c420418473fc 100644 > --- a/drivers/net/Kconfig > +++ b/drivers/net/Kconfig > @@ -395,6 +395,7 @@ config TUN > tristate "Universal TUN/TAP device driver support" > depends on INET > select CRC32 > + select TAP > help > TUN/TAP provides packet reception and transmission for user space > programs. It can be viewed as a simple Point-to-Point or Ethernet > diff --git a/drivers/net/Makefile b/drivers/net/Makefile > index bb8eb3053772..2275309a97ee 100644 > --- a/drivers/net/Makefile > +++ b/drivers/net/Makefile > @@ -29,9 +29,9 @@ obj-y += mdio/ > obj-y += pcs/ > obj-$(CONFIG_RIONET) += rionet.o > obj-$(CONFIG_NET_TEAM) += team/ > -obj-$(CONFIG_TUN) += tun-drv.o > -tun-drv-y := tun.o tun_vnet.o > -obj-$(CONFIG_TAP) += tap.o > +obj-$(CONFIG_TUN) += tun.o Is reversing the previous changes to tun.ko intentional? Perhaps the previous approach with a new CONFIG_TUN_VNET is preferable over this. In particular over making TUN select TAP, a new dependency. > +obj-$(CONFIG_TAP) += tap-drv.o > +tap-drv-y := tap.o tun_vnet.o > obj-$(CONFIG_VETH) += veth.o > obj-$(CONFIG_VIRTIO_NET) += virtio_net.o > obj-$(CONFIG_VXLAN) += vxlan/
On 2025/01/17 18:23, Willem de Bruijn wrote: > Akihiko Odaki wrote: >> tun and tap implements the same vnet-related features so reuse the code. >> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >> --- >> drivers/net/Kconfig | 1 + >> drivers/net/Makefile | 6 +- >> drivers/net/tap.c | 152 +++++-------------------------------------------- >> drivers/net/tun_vnet.c | 5 ++ >> 4 files changed, 24 insertions(+), 140 deletions(-) >> >> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig >> index 1fd5acdc73c6..c420418473fc 100644 >> --- a/drivers/net/Kconfig >> +++ b/drivers/net/Kconfig >> @@ -395,6 +395,7 @@ config TUN >> tristate "Universal TUN/TAP device driver support" >> depends on INET >> select CRC32 >> + select TAP >> help >> TUN/TAP provides packet reception and transmission for user space >> programs. It can be viewed as a simple Point-to-Point or Ethernet >> diff --git a/drivers/net/Makefile b/drivers/net/Makefile >> index bb8eb3053772..2275309a97ee 100644 >> --- a/drivers/net/Makefile >> +++ b/drivers/net/Makefile >> @@ -29,9 +29,9 @@ obj-y += mdio/ >> obj-y += pcs/ >> obj-$(CONFIG_RIONET) += rionet.o >> obj-$(CONFIG_NET_TEAM) += team/ >> -obj-$(CONFIG_TUN) += tun-drv.o >> -tun-drv-y := tun.o tun_vnet.o >> -obj-$(CONFIG_TAP) += tap.o >> +obj-$(CONFIG_TUN) += tun.o > > Is reversing the previous changes to tun.ko intentional? > > Perhaps the previous approach with a new CONFIG_TUN_VNET is preferable > over this. In particular over making TUN select TAP, a new dependency. Jason, you also commented about CONFIG_TUN_VNET for the previous version. Do you prefer the old approach, or the new one? (Or if you have another idea, please tell me.) > >> +obj-$(CONFIG_TAP) += tap-drv.o >> +tap-drv-y := tap.o tun_vnet.o >> obj-$(CONFIG_VETH) += veth.o >> obj-$(CONFIG_VIRTIO_NET) += virtio_net.o >> obj-$(CONFIG_VXLAN) += vxlan/
On Fri, Jan 17, 2025 at 6:35 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: > > On 2025/01/17 18:23, Willem de Bruijn wrote: > > Akihiko Odaki wrote: > >> tun and tap implements the same vnet-related features so reuse the code. > >> > >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > >> --- > >> drivers/net/Kconfig | 1 + > >> drivers/net/Makefile | 6 +- > >> drivers/net/tap.c | 152 +++++-------------------------------------------- > >> drivers/net/tun_vnet.c | 5 ++ > >> 4 files changed, 24 insertions(+), 140 deletions(-) > >> > >> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > >> index 1fd5acdc73c6..c420418473fc 100644 > >> --- a/drivers/net/Kconfig > >> +++ b/drivers/net/Kconfig > >> @@ -395,6 +395,7 @@ config TUN > >> tristate "Universal TUN/TAP device driver support" > >> depends on INET > >> select CRC32 > >> + select TAP > >> help > >> TUN/TAP provides packet reception and transmission for user space > >> programs. It can be viewed as a simple Point-to-Point or Ethernet > >> diff --git a/drivers/net/Makefile b/drivers/net/Makefile > >> index bb8eb3053772..2275309a97ee 100644 > >> --- a/drivers/net/Makefile > >> +++ b/drivers/net/Makefile > >> @@ -29,9 +29,9 @@ obj-y += mdio/ > >> obj-y += pcs/ > >> obj-$(CONFIG_RIONET) += rionet.o > >> obj-$(CONFIG_NET_TEAM) += team/ > >> -obj-$(CONFIG_TUN) += tun-drv.o > >> -tun-drv-y := tun.o tun_vnet.o > >> -obj-$(CONFIG_TAP) += tap.o > >> +obj-$(CONFIG_TUN) += tun.o > > > > Is reversing the previous changes to tun.ko intentional? > > > > Perhaps the previous approach with a new CONFIG_TUN_VNET is preferable > > over this. In particular over making TUN select TAP, a new dependency. > > Jason, you also commented about CONFIG_TUN_VNET for the previous > version. Do you prefer the old approach, or the new one? (Or if you have > another idea, please tell me.) Ideally, if we can make TUN select TAP that would be better. But there are some subtle differences in the multi queue implementation. We will end up with some useless code for TUN unless we can unify the multi queue logic. It might not be worth it to change the TUN's multi queue logic so having a new file seems to be better. Thanks > > > > >> +obj-$(CONFIG_TAP) += tap-drv.o > >> +tap-drv-y := tap.o tun_vnet.o > >> obj-$(CONFIG_VETH) += veth.o > >> obj-$(CONFIG_VIRTIO_NET) += virtio_net.o > >> obj-$(CONFIG_VXLAN) += vxlan/ >
On Mon, Jan 20, 2025 at 1:37 AM Jason Wang <jasowang@redhat.com> wrote: > > On Fri, Jan 17, 2025 at 6:35 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: > > > > On 2025/01/17 18:23, Willem de Bruijn wrote: > > > Akihiko Odaki wrote: > > >> tun and tap implements the same vnet-related features so reuse the code. > > >> > > >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > > >> --- > > >> drivers/net/Kconfig | 1 + > > >> drivers/net/Makefile | 6 +- > > >> drivers/net/tap.c | 152 +++++-------------------------------------------- > > >> drivers/net/tun_vnet.c | 5 ++ > > >> 4 files changed, 24 insertions(+), 140 deletions(-) > > >> > > >> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > > >> index 1fd5acdc73c6..c420418473fc 100644 > > >> --- a/drivers/net/Kconfig > > >> +++ b/drivers/net/Kconfig > > >> @@ -395,6 +395,7 @@ config TUN > > >> tristate "Universal TUN/TAP device driver support" > > >> depends on INET > > >> select CRC32 > > >> + select TAP > > >> help > > >> TUN/TAP provides packet reception and transmission for user space > > >> programs. It can be viewed as a simple Point-to-Point or Ethernet > > >> diff --git a/drivers/net/Makefile b/drivers/net/Makefile > > >> index bb8eb3053772..2275309a97ee 100644 > > >> --- a/drivers/net/Makefile > > >> +++ b/drivers/net/Makefile > > >> @@ -29,9 +29,9 @@ obj-y += mdio/ > > >> obj-y += pcs/ > > >> obj-$(CONFIG_RIONET) += rionet.o > > >> obj-$(CONFIG_NET_TEAM) += team/ > > >> -obj-$(CONFIG_TUN) += tun-drv.o > > >> -tun-drv-y := tun.o tun_vnet.o > > >> -obj-$(CONFIG_TAP) += tap.o > > >> +obj-$(CONFIG_TUN) += tun.o > > > > > > Is reversing the previous changes to tun.ko intentional? > > > > > > Perhaps the previous approach with a new CONFIG_TUN_VNET is preferable > > > over this. In particular over making TUN select TAP, a new dependency. > > > > Jason, you also commented about CONFIG_TUN_VNET for the previous > > version. Do you prefer the old approach, or the new one? (Or if you have > > another idea, please tell me.) > > Ideally, if we can make TUN select TAP that would be better. But there > are some subtle differences in the multi queue implementation. We will > end up with some useless code for TUN unless we can unify the multi > queue logic. It might not be worth it to change the TUN's multi queue > logic so having a new file seems to be better. +1 on deduplicating further. But this series is complex enough. Let's not expand that. The latest approach with a separate .o file may have some performance cost by converting likely inlined code into real function calls. Another option is to move it all into tun_vnet.h. That also resolves the Makefile issues.
On 2025/01/20 20:19, Willem de Bruijn wrote: > On Mon, Jan 20, 2025 at 1:37 AM Jason Wang <jasowang@redhat.com> wrote: >> >> On Fri, Jan 17, 2025 at 6:35 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: >>> >>> On 2025/01/17 18:23, Willem de Bruijn wrote: >>>> Akihiko Odaki wrote: >>>>> tun and tap implements the same vnet-related features so reuse the code. >>>>> >>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >>>>> --- >>>>> drivers/net/Kconfig | 1 + >>>>> drivers/net/Makefile | 6 +- >>>>> drivers/net/tap.c | 152 +++++-------------------------------------------- >>>>> drivers/net/tun_vnet.c | 5 ++ >>>>> 4 files changed, 24 insertions(+), 140 deletions(-) >>>>> >>>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig >>>>> index 1fd5acdc73c6..c420418473fc 100644 >>>>> --- a/drivers/net/Kconfig >>>>> +++ b/drivers/net/Kconfig >>>>> @@ -395,6 +395,7 @@ config TUN >>>>> tristate "Universal TUN/TAP device driver support" >>>>> depends on INET >>>>> select CRC32 >>>>> + select TAP >>>>> help >>>>> TUN/TAP provides packet reception and transmission for user space >>>>> programs. It can be viewed as a simple Point-to-Point or Ethernet >>>>> diff --git a/drivers/net/Makefile b/drivers/net/Makefile >>>>> index bb8eb3053772..2275309a97ee 100644 >>>>> --- a/drivers/net/Makefile >>>>> +++ b/drivers/net/Makefile >>>>> @@ -29,9 +29,9 @@ obj-y += mdio/ >>>>> obj-y += pcs/ >>>>> obj-$(CONFIG_RIONET) += rionet.o >>>>> obj-$(CONFIG_NET_TEAM) += team/ >>>>> -obj-$(CONFIG_TUN) += tun-drv.o >>>>> -tun-drv-y := tun.o tun_vnet.o >>>>> -obj-$(CONFIG_TAP) += tap.o >>>>> +obj-$(CONFIG_TUN) += tun.o >>>> >>>> Is reversing the previous changes to tun.ko intentional? >>>> >>>> Perhaps the previous approach with a new CONFIG_TUN_VNET is preferable >>>> over this. In particular over making TUN select TAP, a new dependency. >>> >>> Jason, you also commented about CONFIG_TUN_VNET for the previous >>> version. Do you prefer the old approach, or the new one? (Or if you have >>> another idea, please tell me.) >> >> Ideally, if we can make TUN select TAP that would be better. But there >> are some subtle differences in the multi queue implementation. We will >> end up with some useless code for TUN unless we can unify the multi >> queue logic. It might not be worth it to change the TUN's multi queue >> logic so having a new file seems to be better. > > +1 on deduplicating further. But this series is complex enough. Let's not > expand that. > > The latest approach with a separate .o file may have some performance > cost by converting likely inlined code into real function calls. > Another option is to move it all into tun_vnet.h. That also resolves > the Makefile issues. I measured the size difference between the latest inlining approaches. The numbers may vary depending on the system configuration of course, but they should be useful for reference. The below shows sizes when having a separate module: 106496 bytes in total # lsmod Module Size Used by tap 28672 0 tun 61440 0 tun_vnet 16384 2 tun,tap The below shows sizes when inlining: 102400 bytes in total # lsmod Module Size Used by tap 32768 0 tun 69632 0 So having a separate module costs 4096 bytes more. These two approaches should have similar tendency for run-time and compile-time performance; the code is so trivial that the overhead of having one additional module is dominant. The only downside of having all in tun_vnet.h is that it will expose its internal macros and functions, which I think tolerable.
Akihiko Odaki wrote: > On 2025/01/20 20:19, Willem de Bruijn wrote: > > On Mon, Jan 20, 2025 at 1:37 AM Jason Wang <jasowang@redhat.com> wrote: > >> > >> On Fri, Jan 17, 2025 at 6:35 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: > >>> > >>> On 2025/01/17 18:23, Willem de Bruijn wrote: > >>>> Akihiko Odaki wrote: > >>>>> tun and tap implements the same vnet-related features so reuse the code. > >>>>> > >>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > >>>>> --- > >>>>> drivers/net/Kconfig | 1 + > >>>>> drivers/net/Makefile | 6 +- > >>>>> drivers/net/tap.c | 152 +++++-------------------------------------------- > >>>>> drivers/net/tun_vnet.c | 5 ++ > >>>>> 4 files changed, 24 insertions(+), 140 deletions(-) > >>>>> > >>>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig > >>>>> index 1fd5acdc73c6..c420418473fc 100644 > >>>>> --- a/drivers/net/Kconfig > >>>>> +++ b/drivers/net/Kconfig > >>>>> @@ -395,6 +395,7 @@ config TUN > >>>>> tristate "Universal TUN/TAP device driver support" > >>>>> depends on INET > >>>>> select CRC32 > >>>>> + select TAP > >>>>> help > >>>>> TUN/TAP provides packet reception and transmission for user space > >>>>> programs. It can be viewed as a simple Point-to-Point or Ethernet > >>>>> diff --git a/drivers/net/Makefile b/drivers/net/Makefile > >>>>> index bb8eb3053772..2275309a97ee 100644 > >>>>> --- a/drivers/net/Makefile > >>>>> +++ b/drivers/net/Makefile > >>>>> @@ -29,9 +29,9 @@ obj-y += mdio/ > >>>>> obj-y += pcs/ > >>>>> obj-$(CONFIG_RIONET) += rionet.o > >>>>> obj-$(CONFIG_NET_TEAM) += team/ > >>>>> -obj-$(CONFIG_TUN) += tun-drv.o > >>>>> -tun-drv-y := tun.o tun_vnet.o > >>>>> -obj-$(CONFIG_TAP) += tap.o > >>>>> +obj-$(CONFIG_TUN) += tun.o > >>>> > >>>> Is reversing the previous changes to tun.ko intentional? > >>>> > >>>> Perhaps the previous approach with a new CONFIG_TUN_VNET is preferable > >>>> over this. In particular over making TUN select TAP, a new dependency. > >>> > >>> Jason, you also commented about CONFIG_TUN_VNET for the previous > >>> version. Do you prefer the old approach, or the new one? (Or if you have > >>> another idea, please tell me.) > >> > >> Ideally, if we can make TUN select TAP that would be better. But there > >> are some subtle differences in the multi queue implementation. We will > >> end up with some useless code for TUN unless we can unify the multi > >> queue logic. It might not be worth it to change the TUN's multi queue > >> logic so having a new file seems to be better. > > > > +1 on deduplicating further. But this series is complex enough. Let's not > > expand that. > > > > The latest approach with a separate .o file may have some performance > > cost by converting likely inlined code into real function calls. > > Another option is to move it all into tun_vnet.h. That also resolves > > the Makefile issues. > > I measured the size difference between the latest inlining approaches. > The numbers may vary depending on the system configuration of course, > but they should be useful for reference. > > The below shows sizes when having a separate module: 106496 bytes in total > > # lsmod > Module Size Used by > tap 28672 0 > tun 61440 0 > tun_vnet 16384 2 tun,tap > > The below shows sizes when inlining: 102400 bytes in total > > # lsmod > Module Size Used by > tap 32768 0 > tun 69632 0 > > So having a separate module costs 4096 bytes more. > > These two approaches should have similar tendency for run-time and > compile-time performance; the code is so trivial that the overhead of > having one additional module is dominant. The concern raised was not about object size, but about inlined vs true calls in the hot path. > The only downside of having all in tun_vnet.h is that it will expose its > internal macros and functions, which I think tolerable. As long as only included by tun.c and tap.c, I think that's okay. The slow path code (ioctl) could remain in a .c file. But it's simpler to just have the header file.
© 2016 - 2025 Red Hat, Inc.