All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Schaaf <bof@bof.de>
To: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: netfilter-devel@lists.netfilter.org
Subject: Re: timeout storms make the box unresponsive
Date: Sun, 27 Jul 2003 12:34:22 +0200	[thread overview]
Message-ID: <20030727103422.GA23621@oknodo.bof.de> (raw)
In-Reply-To: <Pine.LNX.4.33.0307270027460.7327-100000@blackhole.kfki.hu>

Hi Jozsef,

> 03_locking.patch	per bucket locking patch with a couple of
> 			optimizations

some questions, during first pass reading over the patch;

1) regarding the 'ifdef CONFIG_IP_NF_NAT_NEEDED' gymnastics, reading
   hash_conntrack(), am I correct that this is the "non-NAT symmetry"
   optimization I proposed a long time ago, i.e. if both direction's
   tuples are mirrors of each other, hash/chain them only once?
   If yes, I feel that warrants breaking out into a separate patch.

2) assuming I am correct wrt the goal in 1), I object to the seperate
   "sorting" of IP and port in hash_conntrack(). Although it is unlikely,
   consider a situation where between two specific IP addresses,
   two independant connections run from (IPA,portA)<->(IPB,portB)
   and (IPA,portB)<->(IPB,portA). The code in your patch would
   throw both into the same hash chain, without need. Better:

   	if (IPa < IPb) {
		/* fine, do nothing */
	} else if (IPa > IPb) {
		swap IPs _and_ ports
	} else /* IPa == IPb */ if (portA > port B) {
		swap ports (IPs are identical, swap is noop)
	}

3) there is much activity related to managing expectations and
   helper stuff. Is that directly related to the conntrack locking
   changes themselves?  If not, I would also advocate breaking those
   into a separate patch. I never looked closely into the expectation
   stuff, so it is nothing but confusing (to me) to see it in this patch.

In any case, thanks for your work. Feel free to ignore my pleas, if you
and others are content with the aggregated changes.

best regards
  Patrick

  reply	other threads:[~2003-07-27 10:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-07-25 18:56 timeout storms make the box unresponsive Balazs Scheidler
2003-07-25 20:21 ` Harald Welte
2003-07-26 22:39   ` Jozsef Kadlecsik
2003-07-27 10:34     ` Patrick Schaaf [this message]
2003-08-11 11:02       ` Jozsef Kadlecsik

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=20030727103422.GA23621@oknodo.bof.de \
    --to=bof@bof.de \
    --cc=kadlec@blackhole.kfki.hu \
    --cc=netfilter-devel@lists.netfilter.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.