mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Daniel Marjamäki" <daniel.marjamaki@gmail.com>
To: "David Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org, kernel-janitors@lists.osdl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net/core/flow.c: compare data with memcmp
Date: Mon, 1 Jan 2007 10:16:02 +0100	[thread overview]
Message-ID: <80ec54e90701010116u27d02341kb2be785f17f18e3d@mail.gmail.com> (raw)
In-Reply-To: <80ec54e90612312347w2b906e5eg725a7761110c6897@mail.gmail.com>

Hello!

I have done a little testing on my own. My results is that memcpy is
many times faster even with aligned data.

I am testing in an ordinary console program. I am including the code below.
If I'm doing something wrong, please tell me so.

As you can see I am not using the same datadeclarations as the kernel
but I'm testing the algorithm here not the data. By testing various
data and types of data I can make sure the algorithm behaves correctly
in all situations.
The datamember 'd' in flowi is not part of the comparison, but by
changing it into an 'unsigned int' it becomes part of the comparison.



const int NUM_REP = 0x7FFFFFFF;

typedef unsigned int flow_compare_t;

struct flowi {
	unsigned int a,b,c;
	unsigned char d;
};


/* I hear what you're saying, use memcmp.  But memcmp cannot make
 * important assumptions that we can here, such as alignment and
 * constant size.
 */
static int flow_key_compare(struct flowi *key1, struct flowi *key2)
{
	flow_compare_t *k1, *k1_lim, *k2;
	const int n_elem = sizeof(struct flowi) / sizeof(flow_compare_t);

	k1 = (flow_compare_t *) key1;
	k1_lim = k1 + n_elem;

	k2 = (flow_compare_t *) key2;

	do {
		if (*k1++ != *k2++)
			return 1;
	} while (k1 < k1_lim);

	return 0;
}



static int flow_key_compare2(struct flowi *key1, struct flowi *key2)
{
	return memcmp(key1, key2, (sizeof(struct flowi) /
sizeof(flow_compare_t)) * sizeof(flow_compare_t));
}






int main()
{

	struct flowi key1 = {0,1,2,3};
	struct flowi key2 = {0,1,2,0};
	char str[300];
	int i;

	/* put data in aligned addresses */
	struct flowi *k1 = (struct flowi *)((int)(&str[100]) & 0xFFFFFFF0);
	struct flowi *k2 = (struct flowi *)((int)(&str[200]) & 0xFFFFFFF0);
	memcpy(k1, &key1, sizeof(struct flowi));
	memcpy(k2, &key2, sizeof(struct flowi));

	/* Compare data */
	printf("compare1..\n");
	for (i = 0; i < NUM_REP; i++)
		flow_key_compare(k1, k2);

	printf("compare2..\n");
	for (i = 0; i < NUM_REP; i++)
		flow_key_compare2(k1, k2);

	printf((flow_key_compare(k1,k2)==(flow_key_compare2(k1,k2)?1:0))?"ok\n":"error\n");

	return 0;
}











2007/1/1, Daniel Marjamäki <daniel.marjamaki@gmail.com>:
> Hello!
>
> So you mean that in this particular case it's faster with a handcoded
> comparison than memcmp? Because both key1 and key2 are located at
> word-aligned addresses?
> That's fascinating.
>
> Best regards,
> Daniel
>
> 2006/12/31, David Miller <davem@davemloft.net>:
> > From: "Daniel_Marjamäki" <daniel.marjamaki@gmail.com>
> > Date: Sun, 31 Dec 2006 17:37:05 +0100
> >
> > > From: Daniel Marjamäki
> > > This has been tested by me.
> > > Signed-off-by: Daniel Marjamäki <daniel.marjamaki@gmail.com>
> >
> > Please do not do this.
> >
> > memcmp() cannot assume the alignment of the source and
> > destination buffers and thus will run more slowly than
> > that open-coded comparison.
> >
> > That code was done like that on purpose because it is
> > one of the most critical paths in the networking flow
> > cache lookup which runs for every IPSEC packet going
> > throught the system.
> >
>

  parent reply	other threads:[~2007-01-01  9:16 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-31 16:37 Daniel Marjamäki
2006-12-31 19:49 ` [KJ] " Nishanth Aravamudan
2006-12-31 20:37 ` David Miller
2007-01-01  7:47   ` Daniel Marjamäki
2007-01-01  8:49     ` David Miller
2007-01-01  9:16     ` Daniel Marjamäki [this message]
2007-01-02 23:36       ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=80ec54e90701010116u27d02341kb2be785f17f18e3d@mail.gmail.com \
    --to=daniel.marjamaki@gmail.com \
    --cc=davem@davemloft.net \
    --cc=kernel-janitors@lists.osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®