netdev - Re: [PATCH v4 1/2] core: Factor out flow calculation from get_rps

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: <OF1A3C883B.BC590C6D-ON65257775.004B23FF-65257775.004EE08E@in.ibm.com>
Date:	Wed, 4 Aug 2010 19:53:07 +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 1/2] core: Factor out flow calculation from get_rps_cpu

Changli Gao <xiaosuo@...il.com> wrote on 08/04/2010 07:08:39 PM:

> Changli Gao <xiaosuo@...il.com>
> 08/04/2010 07:08 PM
>
> To
>
> Krishna Kumar2/India/IBM@...IN
>
> cc
>
> arnd@...db.de, bhutchings@...arflare.com, davem@...emloft.net,
> mst@...hat.com, netdev@...r.kernel.org, therbert@...gle.com
>
> Subject
>
> Re: [PATCH v4 1/2] core: Factor out flow calculation from get_rps_cpu
>
> On Wed, Aug 4, 2010 at 8:02 PM, Krishna Kumar2 <krkumar2@...ibm.com>
wrote:
> > Changli Gao <xiaosuo@...il.com> wrote on 08/04/2010 04:52:58 PM:
> >>
> >> the return value of skb_get_rxhash() maybe skb->rxhash, so we don't
> >> need to assign it to skb->rxhash again. Use a local variable to save
> >> the value and __skb_get_rxhash() should cache the rxhash into
> >> skb->rxhash for future use.
> >
> > I wanted the function to return a rxhash instead of save+return,
> > since other users, eg macvtap, doesn't need the rxhash beyond
> > calculating a rxq with that hash. So for those users, we can
> > avoid updating both skb->rxhash and a local variable (since with
> > your suggestion, rps will update either one or two variables
> > depending on whether rxhash is cached or not). I am not sure
> > which is better.
> >
>
> rxhash can also be generated by NIC(hardware) and used some where
> else. So for these users, generating the rxhash again is wasting time.

Yes, I expect that to happen once. By not saving it in skb,
atleast for macvtap, I didn't see any further rx path. I guess
I could do this:

get_rps_cpu()
{
	if (!skb_get_rxhash(skb))
		goto done;
	/* use skb->rxhash from here on */
}

Replying to your other mail here:

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

Yes, but it should be an unlikely case. Is it still important,
considering you add a lot of baggage for a rare case?

So next patch has these two changes, unless contradicted :)

1. Change skb_get_rxhash() to set if not cached already.
2. Use % for macvtap unlikely case.

Thanks,

- 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