Re: Endianness problem with u32 classifier hash masks

[Jarek Poplawski <jarkao2@o2.pl>]

Radu Rendec wrote, On 11/05/2007 06:31 PM:

> On Mon, 2007-11-05 at 09:06 -0500, jamal wrote:
>> On Mon, 2007-05-11 at 14:52 +0100, Jarek Poplawski wrote:
>>
> ...
>>> If we manage to convince Jamal, IMHO a patch to something current like
>>> 2.6.24-rc1-git14 (or maybe -rc2 soon), should suffice (plus some
>>> options to diff to get function names etc. eg.: diff -Nurp). Try with
>>> Documentation/SubmittingPatches. Git isn't necessary at all. And don't
>>> forget about a changelog.
>> That code hasnt changed at all in a few years, so even against
>> 2.6.24-rc1 should be fine and can be applied cleanly to Daves net-2.6.
>> Radu, please refer to my earlier email on things to try.
> 
> Ok, silly procmail rules + threaded view = bad idea. I just didn't see
> your last messages earlier when I replied. Sorry!
> 
> But still, Jamal, I need more explanations on what you meant by "cutdown
> on the conversion in u32_change()". And, before proceeding, I'd like to
> see your reply to Jarek's last email (at 15:49 +0100) about not getting
> 0xff in the end.


Radu, as far as I know Jamal (from reading) he most probably is busy with
some conference! Since these patches aren't so big I think you could
try Jamal's at first, and if it doesn't work, and nothing new from Jamal
in the meantime, resend your version. Cutdown in u32_change() seems to
add more to the fastpath, but maybe Jamal thinks about something else.

> 
> Jarek, because I have to test anyway, I'll include ffs(mask) in my patch
> and have it tested too.


Thanks! But, I did it wrong: + 1 is unnecessary. And since, ffs() checks
for 0 anyway, this should be simpler:

- {
- 	u8 i = 0;
- 	u32 mask = s->hmask;
- 	if (mask) {
- 		while (!(mask & 1)) {
- 			i++;
- 			mask>>=1;
- 		}
- 	}
- 	n->fshift = i;
+ 	n->fshift = ffs(s->hmask);
- }

Plus maybe (u8) cast on warnings...

On the other hand, only now I see here is some hack (or bug) with this
u8, so ffs() would mean a change here. I definitely need to rethink this.
So, it's probably better to make and test this ffs() patch separately
(2 parts).

> 
> I already have a clone of Dave's net-2.6 tree, so it shouldn't be such a
> big effort to use git after all. I can also make the patch against
> 2.6.24-rc1, so please tell me which is more convinient for you.


Since, there is only this one file, with no changes, there will be no
difference, so it's up to your taste.

Cheers,

Jarek P.