Patchwork tap: set IFF_ONE_QUEUE per default

login
register
mail settings
Submitter Peter Lieven
Date Feb. 15, 2013, 8:27 a.m.
Message ID <511DF166.2070105@dlhnet.de>
Download mbox | patch
Permalink /patch/220679/
State New
Headers show

Comments

Peter Lieven - Feb. 15, 2013, 8:27 a.m.
historically the kernel queues packets two times. once
at the device and second in qdisc. this is believed to cause
interface stalls if one of these queues overruns.

setting IFF_ONE_QUEUE is the default in kernels >= 3.8. the
flag is ignored since then. see kernel commit
5d097109257c03a71845729f8db6b5770c4bbedc

Signed-off-by: Peter Lieven <pl@kamp.de>
---
  net/tap-linux.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
Stefan Hajnoczi - Feb. 15, 2013, 9:16 a.m.
On Fri, Feb 15, 2013 at 09:27:18AM +0100, Peter Lieven wrote:
> historically the kernel queues packets two times. once
> at the device and second in qdisc. this is believed to cause
> interface stalls if one of these queues overruns.
> 
> setting IFF_ONE_QUEUE is the default in kernels >= 3.8. the
> flag is ignored since then. see kernel commit
> 5d097109257c03a71845729f8db6b5770c4bbedc
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  net/tap-linux.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Peter Lieven - Feb. 15, 2013, 9:20 a.m.
Am 15.02.2013 um 10:16 schrieb Stefan Hajnoczi <stefanha@gmail.com>:

> On Fri, Feb 15, 2013 at 09:27:18AM +0100, Peter Lieven wrote:
>> historically the kernel queues packets two times. once
>> at the device and second in qdisc. this is believed to cause
>> interface stalls if one of these queues overruns.
>> 
>> setting IFF_ONE_QUEUE is the default in kernels >= 3.8. the
>> flag is ignored since then. see kernel commit
>> 5d097109257c03a71845729f8db6b5770c4bbedc
>> 
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>> net/tap-linux.c |    2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

it seems that IFF_ONE_QUEUE is not defined in qemu headers. i will sent an update.

sorry,
peter
Christian Borntraeger - Feb. 15, 2013, 9:22 a.m.
On 15/02/13 09:27, Peter Lieven wrote:
> historically the kernel queues packets two times. once
> at the device and second in qdisc. this is believed to cause
> interface stalls if one of these queues overruns.
> 
> setting IFF_ONE_QUEUE is the default in kernels >= 3.8. the
> flag is ignored since then. see kernel commit
> 5d097109257c03a71845729f8db6b5770c4bbedc
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  net/tap-linux.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index a953189..2759b78 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -49,7 +49,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>          return -1;
>      }
>      memset(&ifr, 0, sizeof(ifr));
> -    ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
> +    ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE;
> 
>      if (*vnet_hdr) {
>          unsigned int features;

Wouldn't that break macvtap?

there is

        case TUNSETIFF:
                /* ignore the name, just look at flags */
                if (get_user(u, &ifr->ifr_flags))
                        return -EFAULT;

                ret = 0;
                if ((u & ~IFF_VNET_HDR) != (IFF_NO_PI | IFF_TAP))
                        ret = -EINVAL;


in drivers/net/macvtap.c
Peter Lieven - Feb. 15, 2013, 9:25 a.m.
Am 15.02.2013 um 10:22 schrieb Christian Borntraeger <borntraeger@de.ibm.com>:

> On 15/02/13 09:27, Peter Lieven wrote:
>> historically the kernel queues packets two times. once
>> at the device and second in qdisc. this is believed to cause
>> interface stalls if one of these queues overruns.
>> 
>> setting IFF_ONE_QUEUE is the default in kernels >= 3.8. the
>> flag is ignored since then. see kernel commit
>> 5d097109257c03a71845729f8db6b5770c4bbedc
>> 
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>> net/tap-linux.c |    2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/net/tap-linux.c b/net/tap-linux.c
>> index a953189..2759b78 100644
>> --- a/net/tap-linux.c
>> +++ b/net/tap-linux.c
>> @@ -49,7 +49,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>>         return -1;
>>     }
>>     memset(&ifr, 0, sizeof(ifr));
>> -    ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
>> +    ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE;
>> 
>>     if (*vnet_hdr) {
>>         unsigned int features;
> 
> Wouldn't that break macvtap?
> 
> there is
> 
>        case TUNSETIFF:
>                /* ignore the name, just look at flags */
>                if (get_user(u, &ifr->ifr_flags))
>                        return -EFAULT;
> 
>                ret = 0;
>                if ((u & ~IFF_VNET_HDR) != (IFF_NO_PI | IFF_TAP))
>                        ret = -EINVAL;
> 
> 
> in drivers/net/macvtap.c
> 
> 

wasn`t aware I will modify this to only be set on linux

Peter
Christian Borntraeger - Feb. 15, 2013, 9:30 a.m.
On 15/02/13 10:25, Peter Lieven wrote:
> 
> Am 15.02.2013 um 10:22 schrieb Christian Borntraeger <borntraeger@de.ibm.com>:
> 
>> On 15/02/13 09:27, Peter Lieven wrote:
>>> historically the kernel queues packets two times. once
>>> at the device and second in qdisc. this is believed to cause
>>> interface stalls if one of these queues overruns.
>>>
>>> setting IFF_ONE_QUEUE is the default in kernels >= 3.8. the
>>> flag is ignored since then. see kernel commit
>>> 5d097109257c03a71845729f8db6b5770c4bbedc
>>>
>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>> ---
>>> net/tap-linux.c |    2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/tap-linux.c b/net/tap-linux.c
>>> index a953189..2759b78 100644
>>> --- a/net/tap-linux.c
>>> +++ b/net/tap-linux.c
>>> @@ -49,7 +49,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
>>>         return -1;
>>>     }
>>>     memset(&ifr, 0, sizeof(ifr));
>>> -    ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
>>> +    ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE;
>>>
>>>     if (*vnet_hdr) {
>>>         unsigned int features;
>>
>> Wouldn't that break macvtap?
>>
>> there is
>>
>>        case TUNSETIFF:
>>                /* ignore the name, just look at flags */
>>                if (get_user(u, &ifr->ifr_flags))
>>                        return -EFAULT;
>>
>>                ret = 0;
>>                if ((u & ~IFF_VNET_HDR) != (IFF_NO_PI | IFF_TAP))
>>                        ret = -EINVAL;
>>
>>
>> in drivers/net/macvtap.c
>>
>>
> 
> wasn`t aware I will modify this to only be set on linux
> 
> Peter

macvtap is a Linux driver. It is given to qemu usually via libvirt with something
like   -netdev tap,fd=21,id=hostnet0,vhost=on,vhostfd=22
and then having fd21 pointing to an open /dev/tap... device which is backed by
macvtap.

Christian

Patch

diff --git a/net/tap-linux.c b/net/tap-linux.c
index a953189..2759b78 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -49,7 +49,7 @@  int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
          return -1;
      }
      memset(&ifr, 0, sizeof(ifr));
-    ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
+    ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE;

      if (*vnet_hdr) {
          unsigned int features;