lkml.org 
[lkml]   [1999]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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/

    \
     
     \ /
      Last update: 2005-03-22 13:54    [W:0.035 / U:120.964 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site