netdev - Re: [v12 PATCH 2/3] NETFILTER module xt_hmark, new target for HASH based fwmark

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: <201205021949.48741.hans.schillstrom@ericsson.com>
Date:	Wed, 2 May 2012 19:49:47 +0200
From:	Hans Schillstrom <hans.schillstrom@...csson.com>
To:	Pablo Neira Ayuso <pablo@...filter.org>
CC:	"kaber@...sh.net" <kaber@...sh.net>,
	"jengelh@...ozas.de" <jengelh@...ozas.de>,
	"netfilter-devel@...r.kernel.org" <netfilter-devel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"hans@...illstrom.com" <hans@...illstrom.com>
Subject: Re: [v12 PATCH 2/3] NETFILTER module xt_hmark, new target for HASH based fwmark

On Wednesday 02 May 2012 10:09:44 Pablo Neira Ayuso wrote:
> On Wed, May 02, 2012 at 09:55:00AM +0200, Hans Schillstrom wrote:
> > Hello Pablo
> > (Sorry for spamming some of you, kmail started to send HTML mail)
> > 
> > On Wednesday 02 May 2012 02:34:14 Pablo Neira Ayuso wrote:
> > > Hi Hans,
> > > 
> > > I have decided to take your patch and give it one spin today.
> > > 
> > > Please, find it attached. The main things I've done are:
> > > 
> > > * splitting the code into smaller functions, thus, it becomes more
> > >   maintainable.
> > > 
> > > * try to put common code into functions, eg. the layer 4 protocol
> > >   parsing to obtain the ports is the same for both IPv4 and IPv6.
> > > 
> > > * adding the hmark_tuple abstraction, cleaner than using several
> > >   variables to set the address, ports, and so on. Thus, we only pass
> > >   one single pointer to it.
> > > 
> > > * I have removed most of the comments, they bloat the file and most
> > >   information can be extracted by reading the code. I only left the
> > >   comments that clarify "strange" things.
> > > 
> > > Regarding ICMP traffic, I think we can use the ID field for the
> > > hashing as well. Thus, we handle ICMP like other protocols.
> > 
> > Yes why not, I can give it a try.
> > 

I think we wait with this one..

> > > 
> > > Please, I'd appreciate if you can test and spot issues after my
> > > rework. I have slightly tested here.
> > 
> > OK I found some minor things, I'll send an updated version back later today.
> > I will run all my tests it will take a couple of hours.
> 
> Please, go ahead.

Done, all my tests passed

[snip]
This is what I have done.

- I reduced the code size a little bit by combining the hmark_ct_set_htuple_ipvX into one func.
  by adding a hmark_addr6_mask() and hmark_addr_any_mask()
  Note that using "otuple->src.l3num" as param 1 in both src and dst is not a typo.
  (it's not set in the rtuple)
- Made the if (dst < src) swap() in the hmark_hash() since it should be used by every caller. 
- Moved the L3 check a little bit earlier.
- changed return values for fragments.
- Added nhoffs to: hmark_set_tuple_ports(skb, (ip->ihl * 4) + nhoff, t, info);
  to get icmp working


-- 
Regards
Hans Schillstrom <hans.schillstrom@...csson.com>

View attachment "0001-netfilter-add-xt_hmark-target-for-hash-based-skb-mar.patch" of type "text/x-patch" (14857 bytes)