From patchwork Wed Jan 6 19:46:35 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Samir Bellabes X-Patchwork-Id: 42323 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id E63A4B6EEA for ; Thu, 7 Jan 2010 06:46:48 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932647Ab0AFTqj (ORCPT ); Wed, 6 Jan 2010 14:46:39 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S932663Ab0AFTqi (ORCPT ); Wed, 6 Jan 2010 14:46:38 -0500 Received: from bob75-7-88-160-5-175.fbx.proxad.net ([88.160.5.175]:52508 "EHLO cerbere.dyndns.info" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932538Ab0AFTqh (ORCPT ); Wed, 6 Jan 2010 14:46:37 -0500 Received: by cerbere.dyndns.info (Postfix, from userid 501) id 55A6883D7; Wed, 6 Jan 2010 20:46:35 +0100 (CET) From: Samir Bellabes To: Patrick McHardy Cc: linux-security-module@vger.kernel.org, jamal , Evgeniy Polyakov , Neil Horman , netdev@vger.kernel.org, netfilter-devel@vger.kernel.org Subject: Re: [RFC 4/9] snet: introduce snet_core.c and snet.h References: <1262437456-24476-1-git-send-email-sam@synack.fr> <1262437456-24476-5-git-send-email-sam@synack.fr> <4B41FE9D.2070708@trash.net> Date: Wed, 06 Jan 2010 20:46:35 +0100 In-Reply-To: <4B41FE9D.2070708@trash.net> (Patrick McHardy's message of "Mon, 04 Jan 2010 15:43:41 +0100") Message-ID: User-Agent: Gnus/5.110003 (No Gnus v0.3) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Patrick McHardy writes: >> +struct snet_event { >> + enum snet_syscall syscall; >> + u8 protocol; >> +} __attribute__ ((packed)); > > Does this really need to be packed? You're using it in a > struct snet_event_entry, which is padded anyways. I think that that members needs to be aligned, because this struct is used for the jhash() computation. when testing on x86_64, I discovered that jhash value wasn't the same, for a same struct snet_event. Then I thougth about misaligned data, and possible 'hole' between syscall and protocol. anyway, I patched the code to use jhash_2words() or jhash_1word(). here is a patch, which apply on top of initial serie thank you Patrick, sam commit bacdb6b62549b58370298f89f908d66c5a3cab66 Author: Samir Bellabes Date: Wed Jan 6 20:36:06 2010 +0100 snet : delete attribute packed for snet_event and use proper jash functions the structure snet_event was using the attribute packed, which seems to be unnecessary. this patch also fix the good compute of hashes, by moving jash() to jash_1word() and jash_2words() Noticed by Patrick McHardy Signed-off-by: Samir Bellabes --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/security/snet/include/snet.h b/security/snet/include/snet.h index 47da614..75b32d5 100644 --- a/security/snet/include/snet.h +++ b/security/snet/include/snet.h @@ -9,6 +9,6 @@ struct snet_event { enum snet_syscall syscall; u8 protocol; -} __attribute__ ((packed)); +}; #endif /* _SNET_H */ diff --git a/security/snet/snet_event.c b/security/snet/snet_event.c index 9f0b0f3..cc3b6a2 100644 --- a/security/snet/snet_event.c +++ b/security/snet/snet_event.c @@ -24,17 +24,12 @@ static struct snet_event_entry *__snet_event_lookup(const enum snet_syscall sysc unsigned int h = 0; struct list_head *l; struct snet_event_entry *s; - struct snet_event t; if (!snet_evh) return NULL; - /* building the element to look for */ - t.syscall = syscall; - t.protocol = protocol; - /* computing its hash value */ - h = jhash(&t, sizeof(struct snet_event), 0) % snet_evh_size; + h = jhash_2words(syscall, protocol, 0) % snet_evh_size; l = &snet_evh[h]; list_for_each_entry(s, l, list) { @@ -127,7 +122,7 @@ int snet_event_insert(const enum snet_syscall syscall, const u8 protocol) data->se.syscall = syscall; data->se.protocol = protocol; INIT_LIST_HEAD(&(data->list)); - h = jhash(&(data->se), sizeof(struct snet_event), 0) % snet_evh_size; + h = jhash_2words(data->se.syscall, data->se.protocol, 0) % snet_evh_size; list_add_tail(&data->list, &snet_evh[h]); write_unlock_bh(&snet_evh_lock); pr_debug("[%u]=(syscall=%s, protocol=%u)\n", diff --git a/security/snet/snet_verdict.c b/security/snet/snet_verdict.c index 0cad06b..477af3b 100644 --- a/security/snet/snet_verdict.c +++ b/security/snet/snet_verdict.c @@ -29,18 +29,16 @@ static struct snet_verdict_entry *__snet_verdict_lookup(const u32 verdict_id) unsigned int h = 0; struct list_head *l = NULL; struct snet_verdict_entry *s = NULL; - u32 vid = 0; if (!snet_vdh) return NULL; - vid = verdict_id; /* computing its hash value */ - h = jhash(&vid, sizeof(u32), 0) % snet_vdh_size; + h = jhash_1word(verdict_id, 0) % snet_vdh_size; l = &snet_vdh[h]; list_for_each_entry(s, l, list) { - if (s->verdict_id == vid) { + if (s->verdict_id == verdict_id) { return s; } } @@ -134,7 +132,7 @@ int snet_verdict_insert(void) data->verdict_id = verdict_id; data->verdict = SNET_VERDICT_PENDING; INIT_LIST_HEAD(&(data->list)); - h = jhash(&(data->verdict_id), sizeof(u32), 0) % snet_vdh_size; + h = jhash_1word(data->verdict_id, 0) % snet_vdh_size; write_lock_bh(&snet_vdh_lock); if (snet_vdh) {