[PATCH net-next v2] tap: report IFF_DETACH_QUEUE in TUNGETIFF

[PATCH net-next v2] tap: report IFF_DETACH_QUEUE in TUNGETIFF

[Umang Pokhriyal]

tun sets IFF_DETACH_QUEUE in the flags returned by TUNGETIFF when the
queue is detached, since commit 3d407a80b62f ("tun: Report whether the
queue is attached or not"). tap does not, so userspace cannot tell a
detached macvtap or ipvtap queue from an attached one.

Cloud Hypervisor ran into this when checking queue state on macvtap.

Report the flag from q->enabled, as tun does. Unlike tun, tap accepts
TUNSETIFF on a bound fd, so ignore IFF_DETACH_QUEUE there to keep
writing the TUNGETIFF flags back working.

Assisted-by: LLM
Signed-off-by: Umang Pokhriyal <umangpokhriyall@gmail.com>
---

Notes:
    v2:
      - ignore IFF_DETACH_QUEUE in TUNSETIFF so the TUNGETIFF flags can be
        written back, and describe the change as parity with tun (Sashiko)
      - drop Willem's Reviewed-by because of the new TUNSETIFF hunk
    v1: https://lore.kernel.org/netdev/20260929142238.41742-1-umangpokhriyall@gmail.com/

 drivers/net/tap.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index ff67d99deb39e..832439b8a8988 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -932,6 +932,9 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
 		if (get_user(u, &ifr->ifr_flags))
 			return -EFAULT;
 
+		/* TUNGETIFF may report IFF_DETACH_QUEUE, ignore it here */
+		u &= ~IFF_DETACH_QUEUE;
+
 		ret = 0;
 		if ((u & ~TAP_IFFEATURES) != (IFF_NO_PI | IFF_TAP))
 			ret = -EINVAL;
@@ -950,6 +953,8 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
 
 		ret = 0;
 		u = q->flags;
+		if (!q->enabled)
+			u |= IFF_DETACH_QUEUE;
 		if (copy_to_user(&ifr->ifr_name, tap->dev->name, IFNAMSIZ) ||
 		    put_user(u, &ifr->ifr_flags))
 			ret = -EFAULT;

base-commit: cfb7793d1bc0f7d90571611979654cf1b3886b29
-- 
2.53.0

Re: [PATCH net-next v2] tap: report IFF_DETACH_QUEUE in TUNGETIFF

[Willem de Bruijn]

Umang Pokhriyal wrote:
> tun sets IFF_DETACH_QUEUE in the flags returned by TUNGETIFF when the
> queue is detached, since commit 3d407a80b62f ("tun: Report whether the
> queue is attached or not"). tap does not, so userspace cannot tell a
> detached macvtap or ipvtap queue from an attached one.
> 
> Cloud Hypervisor ran into this when checking queue state on macvtap.
> 
> Report the flag from q->enabled, as tun does. Unlike tun, tap accepts
> TUNSETIFF on a bound fd, so ignore IFF_DETACH_QUEUE there to keep
> writing the TUNGETIFF flags back working.
> 
> Assisted-by: LLM
> Signed-off-by: Umang Pokhriyal <umangpokhriyall@gmail.com>
> ---
> 
> Notes:
>     v2:
>       - ignore IFF_DETACH_QUEUE in TUNSETIFF so the TUNGETIFF flags can be
>         written back, and describe the change as parity with tun (Sashiko)
>       - drop Willem's Reviewed-by because of the new TUNSETIFF hunk
>     v1: https://lore.kernel.org/netdev/20260929142238.41742-1-umangpokhriyall@gmail.com/
> 
>  drivers/net/tap.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index ff67d99deb39e..832439b8a8988 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -932,6 +932,9 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
>  		if (get_user(u, &ifr->ifr_flags))
>  			return -EFAULT;
>  
> +		/* TUNGETIFF may report IFF_DETACH_QUEUE, ignore it here */
> +		u &= ~IFF_DETACH_QUEUE;
> +

I suppose we want tap to expose this state through TUNGETIFF, like tun,
because applications already use that?

Else cleaner than working around bits would be a whole new TUNGETQUEUE
call to match TUNSETQUEUE. Rather than to squish this into TUNGETIFF,
while that has no equivalent in TUNSETIFF.

But, that approach does not help existing applications.

So this is probably the right way. Just want to quickly check.

>  		ret = 0;
>  		if ((u & ~TAP_IFFEATURES) != (IFF_NO_PI | IFF_TAP))
>  			ret = -EINVAL;
> @@ -950,6 +953,8 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
>  
>  		ret = 0;
>  		u = q->flags;
> +		if (!q->enabled)
> +			u |= IFF_DETACH_QUEUE;
>  		if (copy_to_user(&ifr->ifr_name, tap->dev->name, IFNAMSIZ) ||
>  		    put_user(u, &ifr->ifr_flags))
>  			ret = -EFAULT;
> 
> base-commit: cfb7793d1bc0f7d90571611979654cf1b3886b29
> -- 
> 2.53.0
> 

Re: [PATCH net-next v2] tap: report IFF_DETACH_QUEUE in TUNGETIFF

[Umang Pokhriyal]

Willem de Bruijn wrote:
> I suppose we want tap to expose this state through TUNGETIFF, like tun,
> because applications already use that?

Yes. CRIU reads IFF_DETACH_QUEUE from TUNGETIFF when it dumps a tun
file, and Cloud Hypervisor did the same until it found that macvtap
reports every queue as attached. Code written for tun's queue API gets
the wrong answer on macvtap, even though macvtap accepts the same
TUNSETQUEUE.

> Else cleaner than working around bits would be a whole new TUNGETQUEUE
> call to match TUNSETQUEUE. Rather than to squish this into TUNGETIFF,
> while that has no equivalent in TUNSETIFF.
>
> But, that approach does not help existing applications.

Right, those applications would have to be changed to use a new
TUNGETQUEUE.

Thanks for taking another look.

Re: [PATCH net-next v2] tap: report IFF_DETACH_QUEUE in TUNGETIFF

[Willem de Bruijn]

Umang Pokhriyal wrote:
> Willem de Bruijn wrote:
> > I suppose we want tap to expose this state through TUNGETIFF, like tun,
> > because applications already use that?
> 
> Yes. CRIU reads IFF_DETACH_QUEUE from TUNGETIFF when it dumps a tun
> file, and Cloud Hypervisor did the same until it found that macvtap
> reports every queue as attached. Code written for tun's queue API gets
> the wrong answer on macvtap, even though macvtap accepts the same
> TUNSETQUEUE.
> 
> > Else cleaner than working around bits would be a whole new TUNGETQUEUE
> > call to match TUNSETQUEUE. Rather than to squish this into TUNGETIFF,
> > while that has no equivalent in TUNSETIFF.
> >
> > But, that approach does not help existing applications.
> 
> Right, those applications would have to be changed to use a new
> TUNGETQUEUE.
> 
> Thanks for taking another look.

Thanks for verifying.

Reviewed-by: Willem de Bruijn <willemb@google.com>