Akihiko Odaki wrote:
Both tun and tap exposes the same set of virtio-net-related features. Unify their implementations to ease future changes.
Signed-off-by: Akihiko Odaki akihiko.odaki@daynix.com
MAINTAINERS | 1 + drivers/net/Kconfig | 5 ++ drivers/net/Makefile | 1 + drivers/net/tap.c | 172 ++++++---------------------------------- drivers/net/tun.c | 208 ++++++++----------------------------------------- drivers/net/tun_vnet.c | 186 +++++++++++++++++++++++++++++++++++++++++++ drivers/net/tun_vnet.h | 24 ++++++ 7 files changed, 273 insertions(+), 324 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 910305c11e8a..1be8a452d11f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -23903,6 +23903,7 @@ F: Documentation/networking/tuntap.rst F: arch/um/os-Linux/drivers/ F: drivers/net/tap.c F: drivers/net/tun.c +F: drivers/net/tun_vnet.h TURBOCHANNEL SUBSYSTEM M: "Maciej W. Rozycki" macro@orcam.me.uk diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 1fd5acdc73c6..255c8f9f1d7c 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 TUN_VNET
No need for this new Kconfig
static struct proto tap_proto = { .name = "tap", @@ -641,10 +576,10 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control, struct sk_buff *skb; struct tap_dev *tap; unsigned long total_len = iov_iter_count(from);
- unsigned long len = total_len;
- unsigned long len; int err; struct virtio_net_hdr vnet_hdr = { 0 };
- int vnet_hdr_len = 0;
- int hdr_len; int copylen = 0; int depth; bool zerocopy = false;
@@ -652,38 +587,20 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control, enum skb_drop_reason drop_reason; if (q->flags & IFF_VNET_HDR) {
vnet_hdr_len = READ_ONCE(q->vnet_hdr_sz);err = -EINVAL;if (len < vnet_hdr_len)goto err;len -= vnet_hdr_len;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) > len)
hdr_len = tun_vnet_hdr_get(READ_ONCE(q->vnet_hdr_sz), q->flags, from, &vnet_hdr);if (hdr_len < 0) {err = hdr_len; goto err;}- } else {
hdr_len = 0;
- err = -EINVAL;
- if (unlikely(len < ETH_HLEN))
goto err;
Is this check removal intentional?
- len = iov_iter_count(from); if (msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY)) { struct iov_iter i;
copylen = vnet_hdr.hdr_len ?tap16_to_cpu(q, vnet_hdr.hdr_len) : GOODCOPY_LEN;
copylen = hdr_len ? hdr_len : GOODCOPY_LEN;@@ -697,7 +614,7 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control, if (!zerocopy) { copylen = len;
linear = tap16_to_cpu(q, vnet_hdr.hdr_len);
linear = hdr_len;@@ -732,9 +649,8 @@ 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));
- if (q->flags & IFF_VNET_HDR) {
err = tun_vnet_hdr_to_skb(q->flags, skb, &vnet_hdr);@@ -797,23 +713,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;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 < 0)goto done;
iov_iter_advance(iter, vnet_hdr_len - sizeof(vnet_hdr));
ret = tun_vnet_hdr_put(vnet_hdr_len, iter, &vnet_hdr);if (ret < 0)goto done;
Please split this patch in to a series of smaller patches.
If feasible:
1. one that move the head of tun.c into tun_vnet.[hc]. 2. then one that uses that also in tap.c. 3. then a separate patch for the ioctl changes. 4. then introduce tun_vnet_hdr_from_skb, tun_vnet_hdr_put and friends in (a) follow-up patch(es).
This is subtle code. Please report what tests you ran to ensure that it does not introduce behavioral changes / regressions.