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]

Date:	Wed, 4 Aug 2010 18:55:24 +0530
From:	Krishna Kumar2 <krkumar2@...ibm.com>
To:	Changli Gao <xiaosuo@...il.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

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.
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

--
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