netdev - Re: [PATCH v4 2/2] macvtap: Implement multiqueue for macvtap driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTi=aVRk7MOEHPoGAQwtJ8bH9Ovkk_Kx-5DpK9S4f@mail.gmail.com>
Date:	Wed, 4 Aug 2010 21:42:05 +0800
From:	Changli Gao <xiaosuo@...il.com>
To:	Krishna Kumar2 <krkumar2@...ibm.com>
Cc:	arnd@...db.de, bhutchings@...arflare.com, davem@...emloft.net,
	mst@...hat.com, netdev@...r.kernel.org, therbert@...gle.com
Subject: Re: [PATCH v4 2/2] macvtap: Implement multiqueue for macvtap driver

On Wed, Aug 4, 2010 at 9:25 PM, Krishna Kumar2 <krkumar2@...ibm.com> wrote:
> Changli Gao <xiaosuo@...il.com> wrote on 08/04/2010 05:07:23 PM:
>
> Thanks for your review and comments!
>
>> >  static struct macvtap_queue *macvtap_get_queue(struct net_device *dev,
>> >                                               struct sk_buff *skb)
>> >  {
>> >        struct macvlan_dev *vlan = netdev_priv(dev);
>> > +       struct macvtap_queue *tap = NULL;
>> > +       int numvtaps = vlan->numvtaps;
>> > +       __u32 rxq;
>> > +
>> > +       if (!numvtaps)
>> > +               goto out;
>> > +
>> > +       if (likely(skb_rx_queue_recorded(skb))) {
>> > +               rxq = skb_get_rx_queue(skb);
>> > +
>> > +               while (unlikely(rxq >= numvtaps))
>> > +                       rxq -= numvtaps;
>> >
>> > -       return rcu_dereference(vlan->tap);
>> > +               tap = rcu_dereference(vlan->taps[rxq]);
>> > +               if (tap)
>> > +                       goto out;
>> > +       }
>> > +
>> > +       rxq = skb_get_rxhash(skb);
>> > +       if (!rxq)
>> > +               rxq = smp_processor_id();
>> > +
>> > +       tap = rcu_dereference(vlan->taps[rxq & (numvtaps - 1)]);
>> > +
>>
>> numvtaps maybe not power of 2. So some queue maybe can't be used.
>> You'd better maintain a online queue map and get the index as
>> get_rps_cpu() dones.
>>
>> ((u64)rxhash * numvtaps) >> 32
>>
>> for the sbks, we can't get a valid rxhash, we'd better pass them to a
>> specified queue, such as queue 0.
>
> macvtap *with* mq support would be used with mq devices - you
> open multiple queues on the macvtap device depending on the
> number of queues for the physical device. So since this is an
> unlikely case (as can be seen in the patch, and I guess I
> should add another "likely" to the "if (tap)" check since fd's
> should not be closed), I guess a simple % can be used. Does
> the following sound reasonable?
>
> 1. Use % to find the slot.

It is slower than the method used by get_rps_cpu().

> 2. If slot is null - I don't want to handle this since I think
>   it is better to return NULL if some fd's were closed by user.
>   Typically this should never happen since fd's are opened and
>   passed to vhost for setting up the backend. So if they are
>   closed, then I think NULL is OK.
>
> Arnd, please let me know what you would also suggest.
>
> - KK
>
>



-- 
Regards,
Changli Gao(xiaosuo@...il.com)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html