Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 21 Oct 1999 14:00:59 -0300 | From | Juanjo Ciarlante <> | Subject | Re: [PATCH] 2.2.13 ip_fw.c interrupt and linecount fixes (was Re: repeatedly reading ipchains rules causes ppp trouble) |
| |
On Thu, Oct 21, 1999 at 01:04:35PM +1000, Paul Rusty Russell wrote: > OK, this is what I'm going to submit to Alan for 2.2.14; please report > any problems (I don't expect any). > {patch snipped}
> > --- linux/net/ipv4/ip_fw.c.~1~ Thu Oct 21 10:33:36 1999 > +++ linux/net/ipv4/ip_fw.c Thu Oct 21 13:00:16 1999 > @@ -40,11 +40,13 @@ > * 23-Jul-1999: Fixed small fragment security exposure opened on 15-May-1998. > * John McDonald <jm@dataprotect.com> > * Thomas Lopatic <tl@dataprotect.com> > + * 21-Oct-1999: Use bh, not interrupt locking. --RR > + * Applied count fix by Emanuele Caratti <wiz@iol.it> > */ > > /* > * > - * The origina Linux port was done Alan Cox, with changes/fixes from > + * The original Linux port was done Alan Cox, with changes/fixes from > * Pauline Middlelink, Jos Vos, Thomas Quinot, Wouter Gadeyne, Juan > * Jose Ciarlante, Bernd Eckenfels, Keith Owens and others. > * > @@ -150,6 +152,7 @@ > static struct sock *ipfwsk; > #endif > > +/* Don't call SLOT_NUMBER when you have a write lock. */ > #ifdef __SMP__ > #define SLOT_NUMBER() (cpu_number_map[smp_processor_id()]*2 + !in_interrupt()) > #else > @@ -190,21 +193,26 @@ > __FILE__, __LINE__, SLOT_NUMBER()); \ > } while (0) > > +#define FWC_NOINT() \ > +do { \ > + if (in_interrupt()) \ > + printk("Rusty, you promised! %s %u\n", __FILE__, __LINE__); \ > +} while(0) > #else > #define FWC_DEBUG_LOCK(d) do { } while(0) > #define FWC_DEBUG_UNLOCK(d) do { } while(0) > #define FWC_DONT_HAVE_LOCK(d) do { } while(0) > #define FWC_HAVE_LOCK(d) do { } while(0) > +#define FWC_NOINT() do { } while(0) > #endif /*DEBUG_IP_FIRWALL_LOCKING*/ > > +/* We never to a write lock in bh, so we only need write_lock_bh */ > #define FWC_READ_LOCK(l) do { FWC_DEBUG_LOCK(fwc_rlocks); read_lock(l); } while (0) > -#define FWC_WRITE_LOCK(l) do { FWC_DEBUG_LOCK(fwc_wlocks); write_lock(l); } while (0) > -#define FWC_READ_LOCK_IRQ(l,f) do { FWC_DEBUG_LOCK(fwc_rlocks); read_lock_irqsave(l,f); } while (0) > -#define FWC_WRITE_LOCK_IRQ(l,f) do { FWC_DEBUG_LOCK(fwc_wlocks); write_lock_irqsave(l,f); } while (0) > +/* Debug after lock obtained, (in_interrupt() will be true there), so > + SLOT_NUMBER consistent. */ > +#define FWC_WRITE_LOCK(l) do { FWC_NOINT(); write_lock_bh(l); FWC_DEBUG_LOCK(fwc_wlocks); } while (0) > #define FWC_READ_UNLOCK(l) do { FWC_DEBUG_UNLOCK(fwc_rlocks); read_unlock(l); } while (0) > -#define FWC_WRITE_UNLOCK(l) do { FWC_DEBUG_UNLOCK(fwc_wlocks); write_unlock(l); } while (0) > -#define FWC_READ_UNLOCK_IRQ(l,f) do { FWC_DEBUG_UNLOCK(fwc_rlocks); read_unlock_irqrestore(l,f); } while (0) > -#define FWC_WRITE_UNLOCK_IRQ(l,f) do { FWC_DEBUG_UNLOCK(fwc_wlocks); write_unlock_irqrestore(l,f); } while (0) > +#define FWC_WRITE_UNLOCK(l) do { FWC_DEBUG_UNLOCK(fwc_wlocks); write_unlock_bh(l); } while (0) > > struct ip_chain; > > @@ -228,6 +236,7 @@ > { > struct ip_chain *prevchain; /* Pointer to referencing chain */ > struct ip_fwkernel *prevrule; /* Pointer to referencing rule */ > + unsigned int count; > struct ip_counters counters; > }; > > @@ -729,8 +738,8 @@ > else FWC_HAVE_LOCK(fwc_rlocks); > > f = chain->chain; > + count = 0; > do { > - count = 0; > for (; f; f = f->next) { > count++; > if (ip_rule_match(f,rif,ip, > @@ -768,10 +777,12 @@ > else { > f->branch->reent[slot].prevchain > = chain; > + f->branch->reent[slot].count = count; > f->branch->reent[slot].prevrule > = f->next; > chain = f->branch; > f = chain->chain; > + count = 0; > } > } > else if (f->simplebranch == FW_SKIP) > @@ -790,6 +801,7 @@ > if (chain->reent[slot].prevchain) { > struct ip_chain *tmp = chain; > f = chain->reent[slot].prevrule; > + count = chain->reent[slot].count; > chain = chain->reent[slot].prevchain; > tmp->reent[slot].prevchain = NULL; > } > @@ -1300,9 +1312,8 @@ > { > int ret; > struct ip_chain *chain; > - unsigned long flags; > > - FWC_WRITE_LOCK_IRQ(&ip_fw_lock, flags); > + FWC_WRITE_LOCK(&ip_fw_lock); > > switch (cmd) { > case IP_FW_FLUSH: > @@ -1326,7 +1337,7 @@ > struct iphdr *ip; > > /* Don't need write lock. */ > - FWC_WRITE_UNLOCK_IRQ(&ip_fw_lock, flags); > + FWC_WRITE_UNLOCK(&ip_fw_lock); > > if (len != sizeof(struct ip_fwtest) || !check_label(m)) > return EINVAL; > @@ -1521,7 +1532,7 @@ > ret = EINVAL; > } > > - FWC_WRITE_UNLOCK_IRQ(&ip_fw_lock, flags); > + FWC_WRITE_UNLOCK(&ip_fw_lock); > return ret; > } > > @@ -1582,7 +1593,6 @@ > { > struct ip_chain *i; > struct ip_fwkernel *j = ip_fw_chains->chain; > - unsigned long flags; > int len = 0; > int last_len = 0; > off_t upto = 0; > @@ -1591,7 +1601,7 @@ > duprintf("ip_fw_chains is 0x%0lX\n", (unsigned long int)ip_fw_chains); > > /* Need a write lock to lock out ``readers'' which update counters. */ > - FWC_WRITE_LOCK_IRQ(&ip_fw_lock, flags); > + FWC_WRITE_LOCK(&ip_fw_lock); > > for (i = ip_fw_chains; i; i = i->next) { > for (j = i->chain; j; j = j->next) { > @@ -1622,7 +1632,7 @@ > } > } > outside: > - FWC_WRITE_UNLOCK_IRQ(&ip_fw_lock, flags); > + FWC_WRITE_UNLOCK(&ip_fw_lock); > buffer[len] = '\0'; > > duprintf("ip_chain_procinfo: Length = %i (of %i). Offset = %li.\n", > @@ -1638,10 +1648,9 @@ > struct ip_chain *i; > int len = 0,last_len = 0; > off_t pos = 0,begin = 0; > - unsigned long flags; > > /* Need a write lock to lock out ``readers'' which update counters. */ > - FWC_WRITE_LOCK_IRQ(&ip_fw_lock, flags); > + FWC_WRITE_LOCK(&ip_fw_lock); > > for (i = ip_fw_chains; i; i = i->next) > { > @@ -1673,7 +1682,7 @@ > > last_len = len; > } > - FWC_WRITE_UNLOCK_IRQ(&ip_fw_lock, flags); > + FWC_WRITE_UNLOCK(&ip_fw_lock); > > *start = buffer+(offset-begin); > len-=(offset-begin); > -- > Hacking time. Cool!.. (as a bh-beated code, fw code wanted this)
Just a litl warning: xxxx_lock_bh() macros are in net/ip_masq.h, I put them under ip_masq when I created these beasts (in fact, in 2.2 ip_masq*.c are the ONLY callers) because I did not have enough knowlegde (coward, Me?) to alter spinlock.h, so problems may arise because of the #ifdef CONFIG_IP_MASQUERADE when include'ing ip_masq.h in ip_fw.c.
In short, xxxx_lock_bh() macros should be ``blessed'' with higher visibility.
Juanjo
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |