* Re: [PATCH] ppp: don't use 0 in pointer context
From: Herbert Xu @ 2006-02-08 23:49 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: dlstevens, herbert, carlsonj, linux-ppp, netdev, netdev-owner,
viro
In-Reply-To: <20060208232214.GC14543@mipter.zuzino.mipt.ru>
Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> Oh, and for the record: current sparse from Linus doesn't warn about
> this. Slightly modified sparse warns. Bugs which were uncovered by
> more or less trivial and slightly broken sparse patch [1] are:
>
> [PATCH] dscc4: fix dscc4_init_dummy_skb check
> [PATCH] opl3sa2: fix adding second dma channel
> [PATCH] gusclassic: fix adding second dma channel
> [PATCH] ipw2200: fix ->eeprom[EEPROM_VERSION] check
> [PATCH] ixj: fix writing silence check
The first two can also be caught with gcc -pedantic. In fact, catching
the last two should be doable there as well. The difference between
gcc -pedantic and sparse is that it doesn't warn about obviously correct
cases like p != 0 or p = 0.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: Paul Mackerras @ 2006-02-08 23:26 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: David Stevens, Herbert Xu, James Carlson, linux-ppp, netdev,
netdev-owner, viro
In-Reply-To: <20060208232214.GC14543@mipter.zuzino.mipt.ru>
Alexey Dobriyan writes:
> The fact that they can be represented by the same bit patterns is
> irrelevant.
Indeed it is. The fact that the C standard says that "0" is a valid
representation for a null pointer in C source code *is* relevant,
though. That is in fact something that *wasn't* in K&R C but was
introduced in ISO Standard C, and I think it makes sense.
Anyway, this whole discussion is silly. Can we move on to discussing
whether "disc" or "disk" is the correct spelling, or something else
equally productive? :)
Paul.
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: Alexey Dobriyan @ 2006-02-08 23:22 UTC (permalink / raw)
To: David Stevens
Cc: Herbert Xu, James Carlson, linux-ppp, netdev, netdev-owner, viro
In-Reply-To: <OF71FA79DA.C4F20DA5-ON8825710F.007B701A-8825710F.007CB97E@us.ibm.com>
On Wed, Feb 08, 2006 at 02:45:08PM -0800, David Stevens wrote:
> netdev-owner@vger.kernel.org wrote on 02/08/2006 02:19:20 PM:
>
> > James Carlson <carlsonj@workingcode.com> wrote:
> > > Alexey Dobriyan writes:
> > >> - if (ap == 0)
> > >> + if (!ap)
> > >
> > > And the solution is to treat it as a boolean instead?! I'm not sure
> > > which is more ugly.
> >
> > Treating it as a boolean looks good to me. It's better than the
> existing
> > code because it shuts sparse up.
>
> Why would sparse complain about this? 0 is a well-defined
> pointer value (the only value guaranteed to be by the language).
> From K&R ANSI C edition, section 5.4:
>
> "Pointers and integers are not interchangeable. Zero is the sole
> exception: the constant zero may be assigned to a pointer, and a pointer
> may be compared with the constant zero."
O is an integer, NULL is a pointer. You compare integers with integers
and pointers with pointers. You don't compare integers with pointers
because they are fundamentally different objects. The former counts
things, the latter points to them.
The fact that they can be represented by the same bit patterns is
irrelevant. The fact that K&R, C89, C99 make
p = malloc();
if (p == NULL)
legal C is irrelevant.
Because
p = malloc();
if (!p)
err;
is idiomatic C, I've used this form.
<full-disclosure>
Oh, and for the record: current sparse from Linus doesn't warn about
this. Slightly modified sparse warns. Bugs which were uncovered by
more or less trivial and slightly broken sparse patch [1] are:
[PATCH] dscc4: fix dscc4_init_dummy_skb check
[PATCH] opl3sa2: fix adding second dma channel
[PATCH] gusclassic: fix adding second dma channel
[PATCH] ipw2200: fix ->eeprom[EEPROM_VERSION] check
[PATCH] ixj: fix writing silence check
ppp-NULL.patch is the biggest part of
187 files changed, 494 insertions(+), 499 deletions(-)
so I've flushed it first.
[1]:
--- a/evaluate.c
+++ b/evaluate.c
@@ -934,6 +934,10 @@ static struct symbol *evaluate_compare(s
/* Pointer types? */
if (is_ptr_type(ltype) || is_ptr_type(rtype)) {
// FIXME! Check the types for compatibility
+ if (is_ptr_type(ltype) && !is_ptr_type(rtype))
+ bad_expr_type(expr);
+ if (!is_ptr_type(ltype) && is_ptr_type(rtype))
+ bad_expr_type(expr);
expr->op = modify_for_unsigned(expr->op);
goto OK;
}
</full-disclosure>
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: Herbert Xu @ 2006-02-08 23:08 UTC (permalink / raw)
To: David S. Miller
Cc: dlstevens, adobriyan, carlsonj, linux-ppp, netdev, netdev-owner,
viro
In-Reply-To: <20060208.144712.66821895.davem@davemloft.net>
On Wed, Feb 08, 2006 at 02:47:12PM -0800, David S. Miller wrote:
> From: David Stevens <dlstevens@us.ibm.com>
> Date: Wed, 8 Feb 2006 14:45:08 -0800
>
> > Why would sparse complain about this? 0 is a well-defined
> > pointer value (the only value guaranteed to be by the language).
>
> Because sparse goes beyond the standards and tries to
> catch cases that usually end up being bugs.
Well to be frank the only significant NULL/0 bug that I know can't
be caught by converting 0 to NULL anyway. The bug only exists on
architectures where the representations of null-pointers of types
are different. On those architectures, if you have something like
int func(char *fmt, ...);
...
func("foo", NULL);
Then this may break if func actually expected a int * while NULL
could be something different altogether. The only safe way to
write this is to have
func("foo", (int *)NULL);
or
func("foo", (int *)0);
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: Rick Jones @ 2006-02-08 23:00 UTC (permalink / raw)
Cc: linux-ppp, netdev, netdev-owner, viro
In-Reply-To: <20060208.144712.66821895.davem@davemloft.net>
David S. Miller wrote:
> From: David Stevens <dlstevens@us.ibm.com>
> Date: Wed, 8 Feb 2006 14:45:08 -0800
>
>
>> Why would sparse complain about this? 0 is a well-defined
>>pointer value (the only value guaranteed to be by the language).
>
>
> Because sparse goes beyond the standards and tries to
> catch cases that usually end up being bugs.
So, how might it tell when someone is mistakenly using a pointer as a boolean if
it takes a pass on if(foo) or if(!foo) instead of looking for if(foo == NULL)
or if(foo != NULL)? (or I suppose if (NULL == foo) and if (NULL != foo))
it would seem that the compars with NULL, when read by pseudorandom folks would
be more likely to reinforce that foo is a pointer rather than a boolean/flag.
rick jones
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: Paul Mackerras @ 2006-02-08 22:51 UTC (permalink / raw)
To: David S. Miller
Cc: dlstevens, herbert, adobriyan, carlsonj, linux-ppp, netdev,
netdev-owner, viro
In-Reply-To: <20060208.144712.66821895.davem@davemloft.net>
David S. Miller writes:
> Because sparse goes beyond the standards and tries to
> catch cases that usually end up being bugs.
When has a pointer comparison with an explicit "0" ever caused a bug?
Paul.
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: David S. Miller @ 2006-02-08 22:47 UTC (permalink / raw)
To: dlstevens
Cc: herbert, adobriyan, carlsonj, linux-ppp, netdev, netdev-owner,
viro
In-Reply-To: <OF71FA79DA.C4F20DA5-ON8825710F.007B701A-8825710F.007CB97E@us.ibm.com>
From: David Stevens <dlstevens@us.ibm.com>
Date: Wed, 8 Feb 2006 14:45:08 -0800
> Why would sparse complain about this? 0 is a well-defined
> pointer value (the only value guaranteed to be by the language).
Because sparse goes beyond the standards and tries to
catch cases that usually end up being bugs.
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: David Stevens @ 2006-02-08 22:45 UTC (permalink / raw)
To: Herbert Xu
Cc: adobriyan, James Carlson, linux-ppp, netdev, netdev-owner, viro
In-Reply-To: <E1F6xei-0006hc-00@gondolin.me.apana.org.au>
netdev-owner@vger.kernel.org wrote on 02/08/2006 02:19:20 PM:
> James Carlson <carlsonj@workingcode.com> wrote:
> > Alexey Dobriyan writes:
> >> - if (ap == 0)
> >> + if (!ap)
> >
> > And the solution is to treat it as a boolean instead?! I'm not sure
> > which is more ugly.
>
> Treating it as a boolean looks good to me. It's better than the
existing
> code because it shuts sparse up.
Why would sparse complain about this? 0 is a well-defined
pointer value (the only value guaranteed to be by the language).
From K&R ANSI C edition, section 5.4:
"Pointers and integers are not interchangeable. Zero is the sole
exception: the constant zero may be assigned to a pointer, and a pointer
may be compared with the constant zero."
+-DLS
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: Paul Mackerras @ 2006-02-08 22:44 UTC (permalink / raw)
To: James Carlson; +Cc: Alexey Dobriyan, Al Viro, netdev, linux-ppp
In-Reply-To: <17386.27957.822041.83512@gargle.gargle.HOWL>
James Carlson writes:
> Alexey Dobriyan writes:
> > - if (ap == 0)
> > + if (!ap)
>
> And the solution is to treat it as a boolean instead?! I'm not sure
> which is more ugly.
>
> Why wouldn't explicit comparison against NULL be the preferred fix?
I just think this whole "you shouldn't compare a pointer to 0" thing
is completely silly, and patches like this have about the same level
of usefulness as patches that change "color" to "colour" or vice
versa.
However, the head penguin seems to have some bee in his bonnet about 0
not being a pointer value (despite what the C standard says), so
whatever. *shrug*
Paul.
^ permalink raw reply
* [PATCH] ppp: don't use 0 in pointer context
From: Alexey Dobriyan @ 2006-02-08 22:28 UTC (permalink / raw)
To: Al Viro; +Cc: netdev, linux-ppp
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
ppp is the biggest offender.
drivers/net/ppp_async.c | 34 ++++++------
drivers/net/ppp_generic.c | 128 +++++++++++++++++++++++-----------------------
drivers/net/ppp_synctty.c | 26 ++++-----
drivers/net/pppoe.c | 2
4 files changed, 95 insertions(+), 95 deletions(-)
--- a/drivers/net/ppp_async.c
+++ b/drivers/net/ppp_async.c
@@ -159,7 +159,7 @@ ppp_asynctty_open(struct tty_struct *tty
err = -ENOMEM;
ap = kmalloc(sizeof(*ap), GFP_KERNEL);
- if (ap == 0)
+ if (!ap)
goto out;
/* initialize the asyncppp structure */
@@ -215,7 +215,7 @@ ppp_asynctty_close(struct tty_struct *tt
ap = tty->disc_data;
tty->disc_data = NULL;
write_unlock_irq(&disc_data_lock);
- if (ap == 0)
+ if (!ap)
return;
/*
@@ -230,10 +230,10 @@ ppp_asynctty_close(struct tty_struct *tt
tasklet_kill(&ap->tsk);
ppp_unregister_channel(&ap->chan);
- if (ap->rpkt != 0)
+ if (ap->rpkt)
kfree_skb(ap->rpkt);
skb_queue_purge(&ap->rqueue);
- if (ap->tpkt != 0)
+ if (ap->tpkt)
kfree_skb(ap->tpkt);
kfree(ap);
}
@@ -285,13 +285,13 @@ ppp_asynctty_ioctl(struct tty_struct *tt
int err, val;
int __user *p = (int __user *)arg;
- if (ap == 0)
+ if (!ap)
return -ENXIO;
err = -EFAULT;
switch (cmd) {
case PPPIOCGCHAN:
err = -ENXIO;
- if (ap == 0)
+ if (!ap)
break;
err = -EFAULT;
if (put_user(ppp_channel_index(&ap->chan), p))
@@ -301,7 +301,7 @@ ppp_asynctty_ioctl(struct tty_struct *tt
case PPPIOCGUNIT:
err = -ENXIO;
- if (ap == 0)
+ if (!ap)
break;
err = -EFAULT;
if (put_user(ppp_unit_number(&ap->chan), p))
@@ -354,7 +354,7 @@ ppp_asynctty_receive(struct tty_struct *
struct asyncppp *ap = ap_get(tty);
unsigned long flags;
- if (ap == 0)
+ if (!ap)
return;
spin_lock_irqsave(&ap->recv_lock, flags);
ppp_async_input(ap, buf, cflags, count);
@@ -373,7 +373,7 @@ ppp_asynctty_wakeup(struct tty_struct *t
struct asyncppp *ap = ap_get(tty);
clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
- if (ap == 0)
+ if (!ap)
return;
set_bit(XMIT_WAKEUP, &ap->xmit_flags);
tasklet_schedule(&ap->tsk);
@@ -688,7 +688,7 @@ ppp_async_push(struct asyncppp *ap)
tty_stuffed = 1;
continue;
}
- if (ap->optr >= ap->olim && ap->tpkt != 0) {
+ if (ap->optr >= ap->olim && ap->tpkt) {
if (ppp_async_encode(ap)) {
/* finished processing ap->tpkt */
clear_bit(XMIT_FULL, &ap->xmit_flags);
@@ -708,7 +708,7 @@ ppp_async_push(struct asyncppp *ap)
clear_bit(XMIT_BUSY, &ap->xmit_flags);
/* any more work to do? if not, exit the loop */
if (!(test_bit(XMIT_WAKEUP, &ap->xmit_flags)
- || (!tty_stuffed && ap->tpkt != 0)))
+ || (!tty_stuffed && ap->tpkt)))
break;
/* more work to do, see if we can do it now */
if (test_and_set_bit(XMIT_BUSY, &ap->xmit_flags))
@@ -719,7 +719,7 @@ ppp_async_push(struct asyncppp *ap)
flush:
clear_bit(XMIT_BUSY, &ap->xmit_flags);
- if (ap->tpkt != 0) {
+ if (ap->tpkt) {
kfree_skb(ap->tpkt);
ap->tpkt = NULL;
clear_bit(XMIT_FULL, &ap->xmit_flags);
@@ -852,7 +852,7 @@ ppp_async_input(struct asyncppp *ap, con
s = 0;
for (i = 0; i < count; ++i) {
c = buf[i];
- if (flags != 0 && flags[i] != 0)
+ if (flags && flags[i] != 0)
continue;
s |= (c & 0x80)? SC_RCV_B7_1: SC_RCV_B7_0;
c = ((c >> 4) ^ c) & 0xf;
@@ -869,7 +869,7 @@ ppp_async_input(struct asyncppp *ap, con
n = scan_ordinary(ap, buf, count);
f = 0;
- if (flags != 0 && (ap->state & SC_TOSS) == 0) {
+ if (flags && (ap->state & SC_TOSS) == 0) {
/* check the flags to see if any char had an error */
for (j = 0; j < n; ++j)
if ((f = flags[j]) != 0)
@@ -882,9 +882,9 @@ ppp_async_input(struct asyncppp *ap, con
} else if (n > 0 && (ap->state & SC_TOSS) == 0) {
/* stuff the chars in the skb */
skb = ap->rpkt;
- if (skb == 0) {
+ if (!skb) {
skb = dev_alloc_skb(ap->mru + PPP_HDRLEN + 2);
- if (skb == 0)
+ if (!skb)
goto nomem;
ap->rpkt = skb;
}
@@ -931,7 +931,7 @@ ppp_async_input(struct asyncppp *ap, con
++n;
buf += n;
- if (flags != 0)
+ if (flags)
flags += n;
count -= n;
}
--- a/drivers/net/ppp_generic.c
+++ b/drivers/net/ppp_generic.c
@@ -371,7 +371,7 @@ static int ppp_release(struct inode *ino
struct ppp_file *pf = file->private_data;
struct ppp *ppp;
- if (pf != 0) {
+ if (pf) {
file->private_data = NULL;
if (pf->kind == INTERFACE) {
ppp = PF_TO_PPP(pf);
@@ -402,7 +402,7 @@ static ssize_t ppp_read(struct file *fil
ret = count;
- if (pf == 0)
+ if (!pf)
return -ENXIO;
add_wait_queue(&pf->rwait, &wait);
for (;;) {
@@ -435,7 +435,7 @@ static ssize_t ppp_read(struct file *fil
set_current_state(TASK_RUNNING);
remove_wait_queue(&pf->rwait, &wait);
- if (skb == 0)
+ if (!skb)
goto out;
ret = -EOVERFLOW;
@@ -459,11 +459,11 @@ static ssize_t ppp_write(struct file *fi
struct sk_buff *skb;
ssize_t ret;
- if (pf == 0)
+ if (!pf)
return -ENXIO;
ret = -ENOMEM;
skb = alloc_skb(count + pf->hdrlen, GFP_KERNEL);
- if (skb == 0)
+ if (!skb)
goto out;
skb_reserve(skb, pf->hdrlen);
ret = -EFAULT;
@@ -495,11 +495,11 @@ static unsigned int ppp_poll(struct file
struct ppp_file *pf = file->private_data;
unsigned int mask;
- if (pf == 0)
+ if (!pf)
return 0;
poll_wait(file, &pf->rwait, wait);
mask = POLLOUT | POLLWRNORM;
- if (skb_peek(&pf->rq) != 0)
+ if (skb_peek(&pf->rq))
mask |= POLLIN | POLLRDNORM;
if (pf->dead)
mask |= POLLHUP;
@@ -563,7 +563,7 @@ static int ppp_ioctl(struct inode *inode
void __user *argp = (void __user *)arg;
int __user *p = argp;
- if (pf == 0)
+ if (!pf)
return ppp_unattached_ioctl(pf, file, cmd, arg);
if (cmd == PPPIOCDETACH) {
@@ -693,13 +693,13 @@ static int ppp_ioctl(struct inode *inode
val &= 0xffff;
}
vj = slhc_init(val2+1, val+1);
- if (vj == 0) {
+ if (!vj) {
printk(KERN_ERR "PPP: no memory (VJ compressor)\n");
err = -ENOMEM;
break;
}
ppp_lock(ppp);
- if (ppp->vj != 0)
+ if (ppp->vj)
slhc_free(ppp->vj);
ppp->vj = vj;
ppp_unlock(ppp);
@@ -790,7 +790,7 @@ static int ppp_unattached_ioctl(struct p
if (get_user(unit, p))
break;
ppp = ppp_create_interface(unit, &err);
- if (ppp == 0)
+ if (!ppp)
break;
file->private_data = &ppp->file;
ppp->owner = file;
@@ -807,7 +807,7 @@ static int ppp_unattached_ioctl(struct p
down(&all_ppp_sem);
err = -ENXIO;
ppp = ppp_find_unit(unit);
- if (ppp != 0) {
+ if (ppp) {
atomic_inc(&ppp->file.refcnt);
file->private_data = &ppp->file;
err = 0;
@@ -821,7 +821,7 @@ static int ppp_unattached_ioctl(struct p
spin_lock_bh(&all_channels_lock);
err = -ENXIO;
chan = ppp_find_channel(unit);
- if (chan != 0) {
+ if (chan) {
atomic_inc(&chan->file.refcnt);
file->private_data = &chan->file;
err = 0;
@@ -914,7 +914,7 @@ ppp_start_xmit(struct sk_buff *skb, stru
struct sk_buff *ns;
ns = alloc_skb(skb->len + dev->hard_header_len, GFP_ATOMIC);
- if (ns == 0)
+ if (!ns)
goto outf;
skb_reserve(ns, dev->hard_header_len);
skb_copy_bits(skb, 0, skb_put(ns, skb->len), skb->len);
@@ -965,9 +965,9 @@ ppp_net_ioctl(struct net_device *dev, st
case SIOCGPPPCSTATS:
memset(&cstats, 0, sizeof(cstats));
- if (ppp->xc_state != 0)
+ if (ppp->xc_state)
ppp->xcomp->comp_stat(ppp->xc_state, &cstats.c);
- if (ppp->rc_state != 0)
+ if (ppp->rc_state)
ppp->rcomp->decomp_stat(ppp->rc_state, &cstats.d);
if (copy_to_user(addr, &cstats, sizeof(cstats)))
break;
@@ -1012,14 +1012,14 @@ ppp_xmit_process(struct ppp *ppp)
struct sk_buff *skb;
ppp_xmit_lock(ppp);
- if (ppp->dev != 0) {
+ if (ppp->dev) {
ppp_push(ppp);
- while (ppp->xmit_pending == 0
- && (skb = skb_dequeue(&ppp->file.xq)) != 0)
+ while (!ppp->xmit_pending
+ && (skb = skb_dequeue(&ppp->file.xq)))
ppp_send_frame(ppp, skb);
/* If there's no work left to do, tell the core net
code that we can accept some more. */
- if (ppp->xmit_pending == 0 && skb_peek(&ppp->file.xq) == 0)
+ if (!ppp->xmit_pending && !skb_peek(&ppp->file.xq))
netif_wake_queue(ppp->dev);
}
ppp_xmit_unlock(ppp);
@@ -1119,12 +1119,12 @@ ppp_send_frame(struct ppp *ppp, struct s
switch (proto) {
case PPP_IP:
- if (ppp->vj == 0 || (ppp->flags & SC_COMP_TCP) == 0)
+ if (!ppp->vj || (ppp->flags & SC_COMP_TCP) == 0)
break;
/* try to do VJ TCP header compression */
new_skb = alloc_skb(skb->len + ppp->dev->hard_header_len - 2,
GFP_ATOMIC);
- if (new_skb == 0) {
+ if (!new_skb) {
printk(KERN_ERR "PPP: no memory (VJ comp pkt)\n");
goto drop;
}
@@ -1159,7 +1159,7 @@ ppp_send_frame(struct ppp *ppp, struct s
}
/* try to do packet compression */
- if ((ppp->xstate & SC_COMP_RUN) && ppp->xc_state != 0
+ if ((ppp->xstate & SC_COMP_RUN) && ppp->xc_state
&& proto != PPP_LCP && proto != PPP_CCP) {
if (!(ppp->flags & SC_CCP_UP) && (ppp->flags & SC_MUST_COMP)) {
if (net_ratelimit())
@@ -1204,7 +1204,7 @@ ppp_push(struct ppp *ppp)
struct channel *pch;
struct sk_buff *skb = ppp->xmit_pending;
- if (skb == 0)
+ if (!skb)
return;
list = &ppp->channels;
@@ -1374,7 +1374,7 @@ static int ppp_mp_explode(struct ppp *pp
if (flen == len && nfree == 0)
bits |= E;
frag = alloc_skb(flen + hdrlen + (flen == 0), GFP_ATOMIC);
- if (frag == 0)
+ if (!frag)
goto noskb;
q = skb_put(frag, flen + hdrlen);
@@ -1444,7 +1444,7 @@ ppp_channel_push(struct channel *pch)
struct ppp *ppp;
spin_lock_bh(&pch->downl);
- if (pch->chan != 0) {
+ if (pch->chan) {
while (!skb_queue_empty(&pch->file.xq)) {
skb = skb_dequeue(&pch->file.xq);
if (!pch->chan->ops->start_xmit(pch->chan, skb)) {
@@ -1462,7 +1462,7 @@ ppp_channel_push(struct channel *pch)
if (skb_queue_empty(&pch->file.xq)) {
read_lock_bh(&pch->upl);
ppp = pch->ppp;
- if (ppp != 0)
+ if (ppp)
ppp_xmit_process(ppp);
read_unlock_bh(&pch->upl);
}
@@ -1481,7 +1481,7 @@ ppp_do_recv(struct ppp *ppp, struct sk_b
{
ppp_recv_lock(ppp);
/* ppp->dev == 0 means interface is closing down */
- if (ppp->dev != 0)
+ if (ppp->dev)
ppp_receive_frame(ppp, skb, pch);
else
kfree_skb(skb);
@@ -1494,19 +1494,19 @@ ppp_input(struct ppp_channel *chan, stru
struct channel *pch = chan->ppp;
int proto;
- if (pch == 0 || skb->len == 0) {
+ if (!pch || skb->len == 0) {
kfree_skb(skb);
return;
}
proto = PPP_PROTO(skb);
read_lock_bh(&pch->upl);
- if (pch->ppp == 0 || proto >= 0xc000 || proto == PPP_CCPFRAG) {
+ if (!pch->ppp || proto >= 0xc000 || proto == PPP_CCPFRAG) {
/* put it on the channel queue */
skb_queue_tail(&pch->file.rq, skb);
/* drop old frames if queue too long */
while (pch->file.rq.qlen > PPP_MAX_RQLEN
- && (skb = skb_dequeue(&pch->file.rq)) != 0)
+ && (skb = skb_dequeue(&pch->file.rq)))
kfree_skb(skb);
wake_up_interruptible(&pch->file.rwait);
} else {
@@ -1522,13 +1522,13 @@ ppp_input_error(struct ppp_channel *chan
struct channel *pch = chan->ppp;
struct sk_buff *skb;
- if (pch == 0)
+ if (!pch)
return;
read_lock_bh(&pch->upl);
- if (pch->ppp != 0) {
+ if (pch->ppp) {
skb = alloc_skb(0, GFP_ATOMIC);
- if (skb != 0) {
+ if (skb) {
skb->len = 0; /* probably unnecessary */
skb->cb[0] = code;
ppp_do_recv(pch->ppp, skb, pch);
@@ -1567,7 +1567,7 @@ static void
ppp_receive_error(struct ppp *ppp)
{
++ppp->stats.rx_errors;
- if (ppp->vj != 0)
+ if (ppp->vj)
slhc_toss(ppp->vj);
}
@@ -1582,7 +1582,7 @@ ppp_receive_nonmp_frame(struct ppp *ppp,
* Note that some decompressors need to see uncompressed frames
* that come in as well as compressed frames.
*/
- if (ppp->rc_state != 0 && (ppp->rstate & SC_DECOMP_RUN)
+ if (ppp->rc_state && (ppp->rstate & SC_DECOMP_RUN)
&& (ppp->rstate & (SC_DC_FERROR | SC_DC_ERROR)) == 0)
skb = ppp_decompress_frame(ppp, skb);
@@ -1593,13 +1593,13 @@ ppp_receive_nonmp_frame(struct ppp *ppp,
switch (proto) {
case PPP_VJC_COMP:
/* decompress VJ compressed packets */
- if (ppp->vj == 0 || (ppp->flags & SC_REJ_COMP_TCP))
+ if (!ppp->vj || (ppp->flags & SC_REJ_COMP_TCP))
goto err;
if (skb_tailroom(skb) < 124) {
/* copy to a new sk_buff with more tailroom */
ns = dev_alloc_skb(skb->len + 128);
- if (ns == 0) {
+ if (!ns) {
printk(KERN_ERR"PPP: no memory (VJ decomp)\n");
goto err;
}
@@ -1627,7 +1627,7 @@ ppp_receive_nonmp_frame(struct ppp *ppp,
break;
case PPP_VJC_UNCOMP:
- if (ppp->vj == 0 || (ppp->flags & SC_REJ_COMP_TCP))
+ if (!ppp->vj || (ppp->flags & SC_REJ_COMP_TCP))
goto err;
/* Until we fix the decompressor need to make sure
@@ -1657,7 +1657,7 @@ ppp_receive_nonmp_frame(struct ppp *ppp,
skb_queue_tail(&ppp->file.rq, skb);
/* limit queue length by dropping old frames */
while (ppp->file.rq.qlen > PPP_MAX_RQLEN
- && (skb = skb_dequeue(&ppp->file.rq)) != 0)
+ && (skb = skb_dequeue(&ppp->file.rq)))
kfree_skb(skb);
/* wake up any process polling or blocking on read */
wake_up_interruptible(&ppp->file.rwait);
@@ -1722,7 +1722,7 @@ ppp_decompress_frame(struct ppp *ppp, st
if (proto == PPP_COMP) {
ns = dev_alloc_skb(ppp->mru + PPP_HDRLEN);
- if (ns == 0) {
+ if (!ns) {
printk(KERN_ERR "ppp_decompress_frame: no memory\n");
goto err;
}
@@ -1840,7 +1840,7 @@ ppp_receive_mp_frame(struct ppp *ppp, st
ppp->minseq = ppp->mrq.next->sequence;
/* Pull completed packets off the queue and receive them. */
- while ((skb = ppp_mp_reconstruct(ppp)) != 0)
+ while ((skb = ppp_mp_reconstruct(ppp)))
ppp_receive_nonmp_frame(ppp, skb);
return;
@@ -2006,7 +2006,7 @@ ppp_register_channel(struct ppp_channel
struct channel *pch;
pch = kmalloc(sizeof(struct channel), GFP_KERNEL);
- if (pch == 0)
+ if (!pch)
return -ENOMEM;
memset(pch, 0, sizeof(struct channel));
pch->ppp = NULL;
@@ -2035,7 +2035,7 @@ int ppp_channel_index(struct ppp_channel
{
struct channel *pch = chan->ppp;
- if (pch != 0)
+ if (pch)
return pch->file.index;
return -1;
}
@@ -2048,9 +2048,9 @@ int ppp_unit_number(struct ppp_channel *
struct channel *pch = chan->ppp;
int unit = -1;
- if (pch != 0) {
+ if (pch) {
read_lock_bh(&pch->upl);
- if (pch->ppp != 0)
+ if (pch->ppp)
unit = pch->ppp->file.index;
read_unlock_bh(&pch->upl);
}
@@ -2066,7 +2066,7 @@ ppp_unregister_channel(struct ppp_channe
{
struct channel *pch = chan->ppp;
- if (pch == 0)
+ if (!pch)
return; /* should never happen */
chan->ppp = NULL;
@@ -2098,7 +2098,7 @@ ppp_output_wakeup(struct ppp_channel *ch
{
struct channel *pch = chan->ppp;
- if (pch == 0)
+ if (!pch)
return;
ppp_channel_push(pch);
}
@@ -2129,18 +2129,18 @@ ppp_set_compress(struct ppp *ppp, unsign
cp = find_compressor(ccp_option[0]);
#ifdef CONFIG_KMOD
- if (cp == 0) {
+ if (!cp) {
request_module("ppp-compress-%d", ccp_option[0]);
cp = find_compressor(ccp_option[0]);
}
#endif /* CONFIG_KMOD */
- if (cp == 0)
+ if (!cp)
goto out;
err = -ENOBUFS;
if (data.transmit) {
state = cp->comp_alloc(ccp_option, data.length);
- if (state != 0) {
+ if (state) {
ppp_xmit_lock(ppp);
ppp->xstate &= ~SC_COMP_RUN;
ocomp = ppp->xcomp;
@@ -2148,7 +2148,7 @@ ppp_set_compress(struct ppp *ppp, unsign
ppp->xcomp = cp;
ppp->xc_state = state;
ppp_xmit_unlock(ppp);
- if (ostate != 0) {
+ if (ostate) {
ocomp->comp_free(ostate);
module_put(ocomp->owner);
}
@@ -2158,7 +2158,7 @@ ppp_set_compress(struct ppp *ppp, unsign
} else {
state = cp->decomp_alloc(ccp_option, data.length);
- if (state != 0) {
+ if (state) {
ppp_recv_lock(ppp);
ppp->rstate &= ~SC_DECOMP_RUN;
ocomp = ppp->rcomp;
@@ -2166,7 +2166,7 @@ ppp_set_compress(struct ppp *ppp, unsign
ppp->rcomp = cp;
ppp->rc_state = state;
ppp_recv_unlock(ppp);
- if (ostate != 0) {
+ if (ostate) {
ocomp->decomp_free(ostate);
module_put(ocomp->owner);
}
@@ -2233,7 +2233,7 @@ ppp_ccp_peek(struct ppp *ppp, struct sk_
break;
if (inbound) {
/* we will start receiving compressed packets */
- if (ppp->rc_state == 0)
+ if (!ppp->rc_state)
break;
if (ppp->rcomp->decomp_init(ppp->rc_state, dp, len,
ppp->file.index, 0, ppp->mru, ppp->debug)) {
@@ -2242,7 +2242,7 @@ ppp_ccp_peek(struct ppp *ppp, struct sk_
}
} else {
/* we will soon start sending compressed packets */
- if (ppp->xc_state == 0)
+ if (!ppp->xc_state)
break;
if (ppp->xcomp->comp_init(ppp->xc_state, dp, len,
ppp->file.index, 0, ppp->debug))
@@ -2325,11 +2325,11 @@ ppp_register_compressor(struct compresso
int ret;
spin_lock(&compressor_list_lock);
ret = -EEXIST;
- if (find_comp_entry(cp->compress_proto) != 0)
+ if (find_comp_entry(cp->compress_proto))
goto out;
ret = -ENOMEM;
ce = kmalloc(sizeof(struct compressor_entry), GFP_ATOMIC);
- if (ce == 0)
+ if (!ce)
goto out;
ret = 0;
ce->comp = cp;
@@ -2347,7 +2347,7 @@ ppp_unregister_compressor(struct compres
spin_lock(&compressor_list_lock);
ce = find_comp_entry(cp->compress_proto);
- if (ce != 0 && ce->comp == cp) {
+ if (ce && ce->comp == cp) {
list_del(&ce->list);
kfree(ce);
}
@@ -2363,7 +2363,7 @@ find_compressor(int type)
spin_lock(&compressor_list_lock);
ce = find_comp_entry(type);
- if (ce != 0) {
+ if (ce) {
cp = ce->comp;
if (!try_module_get(cp->owner))
cp = NULL;
@@ -2388,7 +2388,7 @@ ppp_get_stats(struct ppp *ppp, struct pp
st->p.ppp_opackets = ppp->stats.tx_packets;
st->p.ppp_oerrors = ppp->stats.tx_errors;
st->p.ppp_obytes = ppp->stats.tx_bytes;
- if (vj == 0)
+ if (!vj)
return;
st->vj.vjs_packets = vj->sls_o_compressed + vj->sls_o_uncompressed;
st->vj.vjs_compressed = vj->sls_o_compressed;
@@ -2603,11 +2603,11 @@ ppp_connect_channel(struct channel *pch,
down(&all_ppp_sem);
ppp = ppp_find_unit(unit);
- if (ppp == 0)
+ if (!ppp)
goto out;
write_lock_bh(&pch->upl);
ret = -EINVAL;
- if (pch->ppp != 0)
+ if (pch->ppp)
goto outl;
ppp_lock(ppp);
@@ -2643,7 +2643,7 @@ ppp_disconnect_channel(struct channel *p
ppp = pch->ppp;
pch->ppp = NULL;
write_unlock_bh(&pch->upl);
- if (ppp != 0) {
+ if (ppp) {
/* remove it from the ppp unit's list */
ppp_lock(ppp);
list_del(&pch->clist);
--- a/drivers/net/ppp_synctty.c
+++ b/drivers/net/ppp_synctty.c
@@ -209,7 +209,7 @@ ppp_sync_open(struct tty_struct *tty)
ap = kmalloc(sizeof(*ap), GFP_KERNEL);
err = -ENOMEM;
- if (ap == 0)
+ if (!ap)
goto out;
/* initialize the syncppp structure */
@@ -263,7 +263,7 @@ ppp_sync_close(struct tty_struct *tty)
ap = tty->disc_data;
tty->disc_data = NULL;
write_unlock_irq(&disc_data_lock);
- if (ap == 0)
+ if (!ap)
return;
/*
@@ -279,7 +279,7 @@ ppp_sync_close(struct tty_struct *tty)
ppp_unregister_channel(&ap->chan);
skb_queue_purge(&ap->rqueue);
- if (ap->tpkt != 0)
+ if (ap->tpkt)
kfree_skb(ap->tpkt);
kfree(ap);
}
@@ -326,13 +326,13 @@ ppp_synctty_ioctl(struct tty_struct *tty
int __user *p = (int __user *)arg;
int err, val;
- if (ap == 0)
+ if (!ap)
return -ENXIO;
err = -EFAULT;
switch (cmd) {
case PPPIOCGCHAN:
err = -ENXIO;
- if (ap == 0)
+ if (!ap)
break;
err = -EFAULT;
if (put_user(ppp_channel_index(&ap->chan), p))
@@ -342,7 +342,7 @@ ppp_synctty_ioctl(struct tty_struct *tty
case PPPIOCGUNIT:
err = -ENXIO;
- if (ap == 0)
+ if (!ap)
break;
err = -EFAULT;
if (put_user(ppp_unit_number(&ap->chan), p))
@@ -395,7 +395,7 @@ ppp_sync_receive(struct tty_struct *tty,
struct syncppp *ap = sp_get(tty);
unsigned long flags;
- if (ap == 0)
+ if (!ap)
return;
spin_lock_irqsave(&ap->recv_lock, flags);
ppp_sync_input(ap, buf, cflags, count);
@@ -414,7 +414,7 @@ ppp_sync_wakeup(struct tty_struct *tty)
struct syncppp *ap = sp_get(tty);
clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
- if (ap == 0)
+ if (!ap)
return;
set_bit(XMIT_WAKEUP, &ap->xmit_flags);
tasklet_schedule(&ap->tsk);
@@ -655,7 +655,7 @@ ppp_sync_push(struct syncppp *ap)
for (;;) {
if (test_and_clear_bit(XMIT_WAKEUP, &ap->xmit_flags))
tty_stuffed = 0;
- if (!tty_stuffed && ap->tpkt != 0) {
+ if (!tty_stuffed && ap->tpkt) {
set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
sent = tty->driver->write(tty, ap->tpkt->data, ap->tpkt->len);
if (sent < 0)
@@ -673,7 +673,7 @@ ppp_sync_push(struct syncppp *ap)
/* haven't made any progress */
spin_unlock_bh(&ap->xmit_lock);
if (!(test_bit(XMIT_WAKEUP, &ap->xmit_flags)
- || (!tty_stuffed && ap->tpkt != 0)))
+ || (!tty_stuffed && ap->tpkt)))
break;
if (!spin_trylock_bh(&ap->xmit_lock))
break;
@@ -681,7 +681,7 @@ ppp_sync_push(struct syncppp *ap)
return done;
flush:
- if (ap->tpkt != 0) {
+ if (ap->tpkt) {
kfree_skb(ap->tpkt);
ap->tpkt = NULL;
clear_bit(XMIT_FULL, &ap->xmit_flags);
@@ -736,7 +736,7 @@ ppp_sync_input(struct syncppp *ap, const
ppp_print_buffer ("receive buffer", buf, count);
/* stuff the chars in the skb */
- if ((skb = dev_alloc_skb(ap->mru + PPP_HDRLEN + 2)) == 0) {
+ if (!(skb = dev_alloc_skb(ap->mru + PPP_HDRLEN + 2))) {
printk(KERN_ERR "PPPsync: no memory (input pkt)\n");
goto err;
}
@@ -744,7 +744,7 @@ ppp_sync_input(struct syncppp *ap, const
if (buf[0] != PPP_ALLSTATIONS)
skb_reserve(skb, 2 + (buf[0] & 1));
- if (flags != 0 && *flags) {
+ if (flags && *flags) {
/* error flag set, ignore frame */
goto err;
} else if (count > skb_tailroom(skb)) {
--- a/drivers/net/pppoe.c
+++ b/drivers/net/pppoe.c
@@ -437,7 +437,7 @@ static int pppoe_disc_rcv(struct sk_buff
* one socket family type, we cannot (easily) distinguish
* what kind of SKB it is during backlog rcv.
*/
- if (sock_owned_by_user(sk) == 0) {
+ if (!sock_owned_by_user(sk)) {
/* We're no longer connect at the PPPOE layer,
* and must wait for ppp channel to disconnect us.
*/
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: Roland Dreier @ 2006-02-08 22:23 UTC (permalink / raw)
To: James Carlson; +Cc: Alexey Dobriyan, Al Viro, netdev, linux-ppp
In-Reply-To: <17386.27957.822041.83512@gargle.gargle.HOWL>
James> And the solution is to treat it as a boolean instead?! I'm
James> not sure which is more ugly.
James> Why wouldn't explicit comparison against NULL be the
James> preferred fix?
"if (ptr)" and "if (!ptr)" are the preferred idiom for testing whether
a pointer is NULL. What is gained by writing "if (ptr != NULL)" ?
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: Herbert Xu @ 2006-02-08 22:19 UTC (permalink / raw)
To: James Carlson; +Cc: adobriyan, viro, netdev, linux-ppp
In-Reply-To: <17386.27957.822041.83512@gargle.gargle.HOWL>
James Carlson <carlsonj@workingcode.com> wrote:
> Alexey Dobriyan writes:
>> - if (ap == 0)
>> + if (!ap)
>
> And the solution is to treat it as a boolean instead?! I'm not sure
> which is more ugly.
Treating it as a boolean looks good to me. It's better than the existing
code because it shuts sparse up.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] ppp: don't use 0 in pointer context
From: James Carlson @ 2006-02-08 22:14 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: Al Viro, netdev, linux-ppp
In-Reply-To: <20060208222559.GA14543@mipter.zuzino.mipt.ru>
Alexey Dobriyan writes:
> - if (ap == 0)
> + if (!ap)
And the solution is to treat it as a boolean instead?! I'm not sure
which is more ugly.
Why wouldn't explicit comparison against NULL be the preferred fix?
--
James Carlson 42.703N 71.076W <carlsonj@workingcode.com>
^ permalink raw reply
* Re: KERNEL: assertion (!sk->sk_forward_alloc) failed
From: David S. Miller @ 2006-02-08 22:12 UTC (permalink / raw)
To: jesse.brandeburg; +Cc: yoseph.basri, bb, linux-kernel, netdev
In-Reply-To: <4807377b0602081207s7604eceahb8bf4af6715a6534@mail.gmail.com>
From: Jesse Brandeburg <jesse.brandeburg@gmail.com>
Date: Wed, 8 Feb 2006 12:07:14 -0800
> this should be on netdev (cc'd), i included some of the thread here.
...
> I though Herbert had fixed these, and it looks like half the patches
> got into 2.6.14.3, but not the fix to the fix committed on 9-6 (not in
> 2.6.14.* at all)
What are the changeset IDs so I can fix this?
Thanks.
^ permalink raw reply
* Re: KERNEL: assertion (!sk->sk_forward_alloc) failed
From: Jesse Brandeburg @ 2006-02-08 20:07 UTC (permalink / raw)
To: Yoseph Basri, Boris B. Zhmurov; +Cc: linux-kernel, NetDEV list
In-Reply-To: <e282236e0602070146p1ed3fdb6k74aa75e15bbc37a3@mail.gmail.com>
this should be on netdev (cc'd), i included some of the thread here.
On 2/7/06, Yoseph Basri <yoseph.basri@gmail.com> wrote:
> Hello kernel maillist,
>
> I'm new member maillist.
>
> Currently, I receive the warning log from my kernel.
>
> Since update to
> Linux 2.6.14.3 #1 SMP Fri Nov 25 20:20:05 SGT 2005 i686 GNU/Linux from 2.4,
>
> I am getting the warning log:
>
> kernel: KERNEL: assertion (!sk->sk_forward_alloc) failed at
> net/core/stream.c (279)
> kernel: KERNEL: assertion (!sk->sk_forward_alloc) failed at
> net/ipv4/af_inet.c (148)
>
> Any information about this issue, and how to solve this problem ?
>
> Another server that i rolled back from 2.6.14 to 2.4 kernel and has no
> warning again. is this bug from 2.6 kernel?
>
> Thanks for your info.
On 2/7/06, Yoseph Basri <yoseph.basri@gmail.com> wrote:
> Hi Boris,
>
> I think so, here is from the dmeg info:
>
> Intel(R) PRO/1000 Network Driver - version 6.0.60-k2
> Copyright (c) 1999-2005 Intel Corporation.
> ACPI: PCI Interrupt 0000:06:08.0[A] -> GSI 29 (level, low) -> IRQ 16
> e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection
> ACPI: PCI Interrupt 0000:06:08.1[B] -> GSI 30 (level, low) -> IRQ 17
> e1000: eth1: e1000_probe: Intel(R) PRO/1000 Network Connection
> e100: Intel(R) PRO/100 Network Driver, 3.4.14-k2-NAPI
> e100: Copyright(c) 1999-2005 Intel Corporation
>
> any info regarding this warning?
>
> YB
>
> On 2/7/06, Boris B. Zhmurov <bb@kernelpanic.ru> wrote:
> > Hello, Yoseph Basri.
> >
> > On 07.02.2006 12:46 you said the following:
> >
> > > I am getting the warning log:
> > >
> > > kernel: KERNEL: assertion (!sk->sk_forward_alloc) failed at
> > > net/core/stream.c (279)
> > > kernel: KERNEL: assertion (!sk->sk_forward_alloc) failed at
> > > net/ipv4/af_inet.c (148)
> >
> >
> > Do you have Intel Pro 1000 network card? Same here...
whats the relevance of e1000?
I though Herbert had fixed these, and it looks like half the patches
got into 2.6.14.3, but not the fix to the fix committed on 9-6 (not in
2.6.14.* at all)
Jesse
^ permalink raw reply
* Re: [PATCH] acx: make firmware statistics parsing more intelligent
From: Andreas Mohr @ 2006-02-08 12:54 UTC (permalink / raw)
To: Denis Vlasenko; +Cc: acx100-devel, netdev
In-Reply-To: <200602041331.16308.vda@ilport.com.ua>
Hi,
On Sat, Feb 04, 2006 at 01:31:16PM +0200, Denis Vlasenko wrote:
> On Friday 03 February 2006 12:58, Andreas Mohr wrote:
> > - use "% 8" modulo instead of more complicated "% 5" calculation
>
> Why? There will be HZ=100 and HZ=250 boxes, 8ms doesn't fit well.
> 10ms is ok, 5ms or 2.5ms is more or less ok too, but 8ms?
>
> But I'll apply this anyway.
Your objection is right, I think, since trading in a more "normal" scheduling
slice length for a *slight* gain in code size doesn't make too much sense.
So maybe still use "% 5"?
Sorry for the wasted discussion...
Andreas Mohr
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
^ permalink raw reply
* [PATCH] mv643xx_eth: remove repeated includes of linux/in.h and linux/ip.h
From: Dale Farnsworth @ 2006-02-08 12:28 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-kernel, netdev, Al Viro, Olaf Hering
In-Reply-To: <E1F6fqN-0006Ba-W6@ZenIV.linux.org.uk>
From: Dale Farnsworth <dale@farnsworth.org>
Signed-off-by: Dale Farnsworth <dale@farnsworth.org>
---
These includes were added twice:
in commit 78a5e534758349fd3effc90ce1152b55368f52ee by Olaf Hering and
in commit b6298c22c5e9f698812e2520003ee178aad50c10 by Al Viro.
This patch reverts 78a5e534758349fd3effc90ce1152b55368f52ee.
They probably should have been included before linux/tcp.h in
the first place.
drivers/net/mv643xx_eth.c | 2 --
1 file changed, 2 deletions(-)
Index: linux-2.6-mv643xx_enet/drivers/net/mv643xx_eth.c
===================================================================
--- linux-2.6-mv643xx_enet.orig/drivers/net/mv643xx_eth.c
+++ linux-2.6-mv643xx_enet/drivers/net/mv643xx_eth.c
@@ -37,8 +37,6 @@
#include <linux/tcp.h>
#include <linux/udp.h>
#include <linux/etherdevice.h>
-#include <linux/in.h>
-#include <linux/ip.h>
#include <linux/bitops.h>
#include <linux/delay.h>
^ permalink raw reply
* [PATCH] get rid of circular list of adev's
From: Denis Vlasenko @ 2006-02-08 12:23 UTC (permalink / raw)
To: acx100-devel; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 935 bytes --]
Hi folks,
If someone has cardbus acx, please test this one.
Basically, in acxpci_e_remove() we will check the presence
of the card by reading a register and if it is not all ones,
we will command device to shut down.
Previously it was done in acxpci_e_cleanup_module().
+INLINE_IO int
+adev_present(acx_device_t *adev)
+{
+ /* fast version (accesses the first register, IO_ACX_SOFT_RESET,
+ * which should be safe): */
+ return readl(adev->iobase) != 0xffffffff;
+}
@@ -1785,6 +1725,43 @@ acxpci_e_remove(struct pci_dev *pdev)
adev = ndev2adev(ndev);
+ /* If device wasn't hot unplugged... */
+ if (adev_present(adev)) {
+
+ acx_sem_lock(adev);
+
+ /* disable both Tx and Rx to shut radio down properly */
+ acx_s_issue_cmd(adev, ACX1xx_CMD_DISABLE_TX, NULL, 0);
+ acx_s_issue_cmd(adev, ACX1xx_CMD_DISABLE_RX, NULL, 0);
...
--
vda
[-- Attachment #2: 1.patch --]
[-- Type: text/x-diff, Size: 9409 bytes --]
diff -urpN current.0/acx_struct.h current.1/acx_struct.h
--- current.0/acx_struct.h Sat Feb 4 13:11:55 2006
+++ current.1/acx_struct.h Wed Feb 8 12:48:33 2006
@@ -1220,14 +1220,9 @@ struct acx_device {
unsigned long lock_time;
#endif
- /*** Device chain ***/
- struct acx_device *next; /* link for list of devices */
-
/*** Linux network device ***/
struct net_device *ndev; /* pointer to linux netdevice */
- struct net_device *prev_nd; /* FIXME: We should not chain via our
- * private struct acx_device _and_
- * the struct net_device */
+
/*** Device statistics ***/
struct net_device_stats stats; /* net device statistics */
#ifdef WIRELESS_EXT
diff -urpN current.0/pci.c current.1/pci.c
--- current.0/pci.c Sat Feb 4 14:49:33 2006
+++ current.1/pci.c Wed Feb 8 13:34:42 2006
@@ -174,11 +174,13 @@ write_flush(acx_device_t *adev)
readb(adev->iobase);
}
-
-/***********************************************************************
-*/
-static struct net_device *root_adev_newest = NULL;
-DECLARE_MUTEX(root_adev_sem);
+INLINE_IO int
+adev_present(acx_device_t *adev)
+{
+ /* fast version (accesses the first register, IO_ACX_SOFT_RESET,
+ * which should be safe): */
+ return readl(adev->iobase) != 0xffffffff;
+}
/***********************************************************************
@@ -1273,62 +1275,6 @@ acx_show_card_eeprom_id(acx_device_t *ad
/***********************************************************************
-*/
-static void
-acxpci_s_device_chain_add(struct net_device *ndev)
-{
- acx_device_t *adev = ndev2adev(ndev);
-
- down(&root_adev_sem);
- adev->prev_nd = root_adev_newest;
- root_adev_newest = ndev;
- adev->ndev = ndev;
- up(&root_adev_sem);
-}
-
-static void
-acxpci_s_device_chain_remove(struct net_device *ndev)
-{
- struct net_device *querydev;
- struct net_device *olderdev;
- struct net_device *newerdev;
-
- down(&root_adev_sem);
- querydev = root_adev_newest;
- newerdev = NULL;
- while (querydev) {
- olderdev = ndev2adev(querydev)->prev_nd;
- if (0 == strcmp(querydev->name, ndev->name)) {
- if (!newerdev) {
- /* if we were at the beginning of the
- * list, then it's the list head that
- * we need to update to point at the
- * next older device */
- root_adev_newest = olderdev;
- } else {
- /* it's the device that is newer than us
- * that we need to update to point at
- * the device older than us */
- ndev2adev(newerdev)->prev_nd = olderdev;
- }
- break;
- }
- /* "newerdev" is actually the device of the old iteration,
- * but since the list starts (root_adev_newest)
- * with the newest devices,
- * it's newer than the ones following.
- * Oh the joys of iterating from newest to oldest :-\ */
- newerdev = querydev;
-
- /* keep checking old devices for matches until we hit the end
- * of the list */
- querydev = olderdev;
- }
- up(&root_adev_sem);
-}
-
-
-/***********************************************************************
** acxpci_free_desc_queues
**
** Releases the queues that have been allocated, the
@@ -1643,9 +1589,6 @@ acxpci_e_probe(struct pci_dev *pdev, con
#endif
SET_NETDEV_DEV(ndev, &pdev->dev);
- /* register new dev in linked list */
- acxpci_s_device_chain_add(ndev);
-
log(L_IRQ|L_INIT, "using IRQ %d\n", pdev->irq);
/* need to be able to restore PCI state after a suspend */
@@ -1725,7 +1668,6 @@ fail_init_mac:
fail_read_eeprom_version:
fail_reset:
- acxpci_s_device_chain_remove(ndev);
free_netdev(ndev);
fail_alloc_netdev:
fail_irq:
@@ -1759,11 +1701,8 @@ done:
/***********************************************************************
** acxpci_e_remove
**
-** Deallocate PCI resources for the acx chip.
-**
-** This should NOT execute any other hardware operations on the card,
-** since the card might already be ejected. Instead, that should be done
-** in cleanup_module, since the card is most likely still available there.
+** Shut device down (if not hot unplugged)
+** and deallocate PCI resources for the acx chip.
**
** pdev - ptr to PCI device structure containing info about pci configuration
*/
@@ -1773,6 +1712,7 @@ acxpci_e_remove(struct pci_dev *pdev)
struct net_device *ndev;
acx_device_t *adev;
unsigned long mem_region1, mem_region2;
+ unsigned long flags;
FN_ENTER;
@@ -1785,6 +1725,43 @@ acxpci_e_remove(struct pci_dev *pdev)
adev = ndev2adev(ndev);
+ /* If device wasn't hot unplugged... */
+ if (adev_present(adev)) {
+
+ acx_sem_lock(adev);
+
+ /* disable both Tx and Rx to shut radio down properly */
+ acx_s_issue_cmd(adev, ACX1xx_CMD_DISABLE_TX, NULL, 0);
+ acx_s_issue_cmd(adev, ACX1xx_CMD_DISABLE_RX, NULL, 0);
+
+#ifdef REDUNDANT
+ /* put the eCPU to sleep to save power
+ * Halting is not possible currently,
+ * since not supported by all firmware versions */
+ acx_s_issue_cmd(adev, ACX100_CMD_SLEEP, NULL, 0);
+#endif
+ acx_lock(adev, flags);
+ /* disable power LED to save power :-) */
+ log(L_INIT, "switching off power LED to save power\n");
+ acxpci_l_power_led(adev, 0);
+ /* stop our eCPU */
+ if (IS_ACX111(adev)) {
+ /* FIXME: does this actually keep halting the eCPU?
+ * I don't think so...
+ */
+ acxpci_l_reset_mac(adev);
+ } else {
+ u16 temp;
+ /* halt eCPU */
+ temp = read_reg16(adev, IO_ACX_ECPU_CTRL) | 0x1;
+ write_reg16(adev, IO_ACX_ECPU_CTRL, temp);
+ write_flush(adev);
+ }
+ acx_unlock(adev, flags);
+
+ acx_sem_unlock(adev);
+ }
+
/* unregister the device to not let the kernel
* (e.g. ioctls) access a half-deconfigured device
* NB: this will cause acxpci_e_close() to be called,
@@ -1796,6 +1773,13 @@ acxpci_e_remove(struct pci_dev *pdev)
* For paranoid reasons we continue to follow the rules */
acx_sem_lock(adev);
+ if (adev->dev_state_mask & ACX_STATE_IFACE_UP) {
+ acxpci_s_down(ndev);
+ CLEAR_BIT(adev->dev_state_mask, ACX_STATE_IFACE_UP);
+ }
+
+ acx_proc_unregister_entries(ndev);
+
if (IS_ACX100(adev)) {
mem_region1 = PCI_ACX100_REGION1;
mem_region2 = PCI_ACX100_REGION2;
@@ -1804,33 +1788,21 @@ acxpci_e_remove(struct pci_dev *pdev)
mem_region2 = PCI_ACX111_REGION2;
}
- acx_proc_unregister_entries(ndev);
-
- /* find our PCI device in the global acx list and remove it */
- acxpci_s_device_chain_remove(ndev);
-
- if (adev->dev_state_mask & ACX_STATE_IFACE_UP)
- acxpci_s_down(ndev);
-
- CLEAR_BIT(adev->dev_state_mask, ACX_STATE_IFACE_UP);
-
- acxpci_s_delete_dma_regions(adev);
-
/* finally, clean up PCI bus state */
+ acxpci_s_delete_dma_regions(adev);
if (adev->iobase) iounmap(adev->iobase);
if (adev->iobase2) iounmap(adev->iobase2);
-
release_mem_region(pci_resource_start(pdev, mem_region1),
pci_resource_len(pdev, mem_region1));
-
release_mem_region(pci_resource_start(pdev, mem_region2),
pci_resource_len(pdev, mem_region2));
-
pci_disable_device(pdev);
/* remove dev registration */
pci_set_drvdata(pdev, NULL);
+ acx_sem_unlock(adev);
+
/* Free netdev (quite late,
* since otherwise we might get caught off-guard
* by a netdev timeout handler execution
@@ -2002,10 +1974,10 @@ acxpci_s_up(struct net_device *ndev)
/***********************************************************************
** acxpci_s_down
**
-** This disables the netdevice
+** NB: device may be already hot unplugged if called from acxpci_e_remove()
**
-** Side effects:
-** - disables on-card interrupt request
+** Disables on-card interrupt request, stops softirq and timer, stops queue,
+** sets status == STOPPED
*/
static void
@@ -4207,69 +4179,8 @@ acxpci_e_init_module(void)
void __exit
acxpci_e_cleanup_module(void)
{
- struct net_device *ndev;
- unsigned long flags;
-
FN_ENTER;
- /* Since the whole module is about to be unloaded,
- * we recursively shutdown all cards we handled instead
- * of doing it in acxpci_e_remove() (which will be activated by us
- * via pci_unregister_driver at the end).
- * acxpci_e_remove() might just get called after a card eject,
- * that's why hardware operations have to be done here instead
- * when the hardware is available. */
-
- down(&root_adev_sem);
-
- ndev = root_adev_newest;
- while (ndev) {
- acx_device_t *adev = ndev2adev(ndev);
-
- acx_sem_lock(adev);
-
- /* disable both Tx and Rx to shut radio down properly */
- acx_s_issue_cmd(adev, ACX1xx_CMD_DISABLE_TX, NULL, 0);
- acx_s_issue_cmd(adev, ACX1xx_CMD_DISABLE_RX, NULL, 0);
-
-#ifdef REDUNDANT
- /* put the eCPU to sleep to save power
- * Halting is not possible currently,
- * since not supported by all firmware versions */
- acx_s_issue_cmd(adev, ACX100_CMD_SLEEP, NULL, 0);
-#endif
- acx_lock(adev, flags);
-
- /* disable power LED to save power :-) */
- log(L_INIT, "switching off power LED to save power\n");
- acxpci_l_power_led(adev, 0);
-
- /* stop our eCPU */
- if (IS_ACX111(adev)) {
- /* FIXME: does this actually keep halting the eCPU?
- * I don't think so...
- */
- acxpci_l_reset_mac(adev);
- } else {
- u16 temp;
-
- /* halt eCPU */
- temp = read_reg16(adev, IO_ACX_ECPU_CTRL) | 0x1;
- write_reg16(adev, IO_ACX_ECPU_CTRL, temp);
- write_flush(adev);
- }
-
- acx_unlock(adev, flags);
-
- acx_sem_unlock(adev);
-
- ndev = adev->prev_nd;
- }
-
- up(&root_adev_sem);
-
- /* now let the PCI layer recursively remove
- * all PCI related things (acxpci_e_remove()) */
pci_unregister_driver(&acxpci_drv_id);
FN_EXIT0;
^ permalink raw reply
* Re: [PATCH] acxsm: Add _{get,set}_encodeext and improve logging in _encode
From: Denis Vlasenko @ 2006-02-08 9:47 UTC (permalink / raw)
To: acx100-devel; +Cc: Carlos Martín, netdev
In-Reply-To: <200602072104.39208.carlos@cmartin.tk>
On Tuesday 07 February 2006 22:04, Carlos Martín wrote:
> Content-Transfer-Encoding: quoted-printable
Patch was damaged. I think sending them as plain-text attachment
would work better.
> Add _{get,set}_encodeext and improve logging in _encode
>
> The code in _{get,set}_encode has been reordered a bit so we have
> better logging (function entry and exit) and _{get,set}_encodeext have
> been implemented as a wrapper for the ieee80211 stack functions.
Applied, thanks!
--
vda
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x103432&bid#0486&dat\x121642
^ permalink raw reply
* [PATCH 19/23] [PATCH] bridge: netfilter races on device removal
From: Chris Wright @ 2006-02-08 6:45 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Justin Forbes, Zwane Mwaikambo, Theodore Ts'o, Randy Dunlap,
Dave Jones, Chuck Wolber, torvalds, akpm, alan, Stephen Hemminger,
David Miller, netdev
In-Reply-To: <20060208064503.924238000@sorel.sous-sol.org>
[-- Attachment #1: bridge-netfilter-races-on-device-removal.patch --]
[-- Type: text/plain, Size: 5585 bytes --]
-stable review patch. If anyone has any objections, please let us know.
------------------
Fix bridge netfilter to handle case where interface is deleted
from bridge while packet is being processed (on other CPU).
Fixes: http://bugzilla.kernel.org/show_bug.cgi?id=5803
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
Signed-off-by: Chris Wright <chrisw@sous-sol.org>
---
net/bridge/br_netfilter.c | 55 +++++++++++++++++++++++++++++++---------------
1 files changed, 38 insertions(+), 17 deletions(-)
Index: linux-2.6.15.3/net/bridge/br_netfilter.c
===================================================================
--- linux-2.6.15.3.orig/net/bridge/br_netfilter.c
+++ linux-2.6.15.3/net/bridge/br_netfilter.c
@@ -47,9 +47,6 @@
#define store_orig_dstaddr(skb) (skb_origaddr(skb) = (skb)->nh.iph->daddr)
#define dnat_took_place(skb) (skb_origaddr(skb) != (skb)->nh.iph->daddr)
-#define has_bridge_parent(device) ((device)->br_port != NULL)
-#define bridge_parent(device) ((device)->br_port->br->dev)
-
#ifdef CONFIG_SYSCTL
static struct ctl_table_header *brnf_sysctl_header;
static int brnf_call_iptables = 1;
@@ -94,6 +91,12 @@ static struct rtable __fake_rtable = {
.rt_flags = 0,
};
+static inline struct net_device *bridge_parent(const struct net_device *dev)
+{
+ struct net_bridge_port *port = rcu_dereference(dev->br_port);
+
+ return port ? port->br->dev : NULL;
+}
/* PF_BRIDGE/PRE_ROUTING *********************************************/
/* Undo the changes made for ip6tables PREROUTING and continue the
@@ -185,11 +188,15 @@ static int br_nf_pre_routing_finish_brid
skb->nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
skb->dev = bridge_parent(skb->dev);
- if (skb->protocol == __constant_htons(ETH_P_8021Q)) {
- skb_pull(skb, VLAN_HLEN);
- skb->nh.raw += VLAN_HLEN;
+ if (!skb->dev)
+ kfree_skb(skb);
+ else {
+ if (skb->protocol == __constant_htons(ETH_P_8021Q)) {
+ skb_pull(skb, VLAN_HLEN);
+ skb->nh.raw += VLAN_HLEN;
+ }
+ skb->dst->output(skb);
}
- skb->dst->output(skb);
return 0;
}
@@ -266,7 +273,7 @@ bridged_dnat:
}
/* Some common code for IPv4/IPv6 */
-static void setup_pre_routing(struct sk_buff *skb)
+static struct net_device *setup_pre_routing(struct sk_buff *skb)
{
struct nf_bridge_info *nf_bridge = skb->nf_bridge;
@@ -278,6 +285,8 @@ static void setup_pre_routing(struct sk_
nf_bridge->mask |= BRNF_NF_BRIDGE_PREROUTING;
nf_bridge->physindev = skb->dev;
skb->dev = bridge_parent(skb->dev);
+
+ return skb->dev;
}
/* We only check the length. A bridge shouldn't do any hop-by-hop stuff anyway */
@@ -372,7 +381,8 @@ static unsigned int br_nf_pre_routing_ip
nf_bridge_put(skb->nf_bridge);
if ((nf_bridge = nf_bridge_alloc(skb)) == NULL)
return NF_DROP;
- setup_pre_routing(skb);
+ if (!setup_pre_routing(skb))
+ return NF_DROP;
NF_HOOK(PF_INET6, NF_IP6_PRE_ROUTING, skb, skb->dev, NULL,
br_nf_pre_routing_finish_ipv6);
@@ -409,7 +419,6 @@ static unsigned int br_nf_pre_routing(un
if (skb->protocol == __constant_htons(ETH_P_8021Q)) {
skb_pull(skb, VLAN_HLEN);
- (skb)->nh.raw += VLAN_HLEN;
}
return br_nf_pre_routing_ipv6(hook, skb, in, out, okfn);
}
@@ -426,7 +435,6 @@ static unsigned int br_nf_pre_routing(un
if (skb->protocol == __constant_htons(ETH_P_8021Q)) {
skb_pull(skb, VLAN_HLEN);
- (skb)->nh.raw += VLAN_HLEN;
}
if (!pskb_may_pull(skb, sizeof(struct iphdr)))
@@ -456,7 +464,8 @@ static unsigned int br_nf_pre_routing(un
nf_bridge_put(skb->nf_bridge);
if ((nf_bridge = nf_bridge_alloc(skb)) == NULL)
return NF_DROP;
- setup_pre_routing(skb);
+ if (!setup_pre_routing(skb))
+ return NF_DROP;
store_orig_dstaddr(skb);
NF_HOOK(PF_INET, NF_IP_PRE_ROUTING, skb, skb->dev, NULL,
@@ -530,11 +539,16 @@ static unsigned int br_nf_forward_ip(uns
struct sk_buff *skb = *pskb;
struct nf_bridge_info *nf_bridge;
struct vlan_ethhdr *hdr = vlan_eth_hdr(skb);
+ struct net_device *parent;
int pf;
if (!skb->nf_bridge)
return NF_ACCEPT;
+ parent = bridge_parent(out);
+ if (!parent)
+ return NF_DROP;
+
if (skb->protocol == __constant_htons(ETH_P_IP) || IS_VLAN_IP)
pf = PF_INET;
else
@@ -555,8 +569,8 @@ static unsigned int br_nf_forward_ip(uns
nf_bridge->mask |= BRNF_BRIDGED;
nf_bridge->physoutdev = skb->dev;
- NF_HOOK(pf, NF_IP_FORWARD, skb, bridge_parent(in),
- bridge_parent(out), br_nf_forward_finish);
+ NF_HOOK(pf, NF_IP_FORWARD, skb, bridge_parent(in), parent,
+ br_nf_forward_finish);
return NF_STOLEN;
}
@@ -679,6 +693,8 @@ static unsigned int br_nf_local_out(unsi
goto out;
}
realoutdev = bridge_parent(skb->dev);
+ if (!realoutdev)
+ return NF_DROP;
#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
/* iptables should match -o br0.x */
@@ -692,9 +708,11 @@ static unsigned int br_nf_local_out(unsi
/* IP forwarded traffic has a physindev, locally
* generated traffic hasn't. */
if (realindev != NULL) {
- if (!(nf_bridge->mask & BRNF_DONT_TAKE_PARENT) &&
- has_bridge_parent(realindev))
- realindev = bridge_parent(realindev);
+ if (!(nf_bridge->mask & BRNF_DONT_TAKE_PARENT) ) {
+ struct net_device *parent = bridge_parent(realindev);
+ if (parent)
+ realindev = parent;
+ }
NF_HOOK_THRESH(pf, NF_IP_FORWARD, skb, realindev,
realoutdev, br_nf_local_out_finish,
@@ -734,6 +752,9 @@ static unsigned int br_nf_post_routing(u
if (!nf_bridge)
return NF_ACCEPT;
+ if (!realoutdev)
+ return NF_DROP;
+
if (skb->protocol == __constant_htons(ETH_P_IP) || IS_VLAN_IP)
pf = PF_INET;
else
--
^ permalink raw reply
* Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed
From: Grant Coady @ 2006-02-08 2:13 UTC (permalink / raw)
To: Pradeep Vincent; +Cc: Willy Tarreau, David S. Miller, netdev, linux-kernel
In-Reply-To: <9fda5f510602071750o53f76fc8gc94c280a9998343d@mail.gmail.com>
On Tue, 7 Feb 2006 17:50:03 -0800, Pradeep Vincent <pradeep.vincent@gmail.com> wrote:
>One more attempt. Attaching the diff file as well.
>
>Signed off by: Pradeep Vincent <pradeep.vincent@gmail.com>
>
>--- old/net/core/neighbour.c Wed Nov 9 16:48:10 2005
>+++ new/net/core/neighbour.c Tue Feb 7 17:38:26 2006
>@@ -14,6 +14,7 @@
> * Vitaly E. Lavrov releasing NULL neighbor in neigh_add.
> * Harald Welte Add neighbour cache statistics like rtstat
> * Harald Welte port neighbour cache rework from 2.6.9-rcX
>+ * Pradeep Vincent fix neighbour cache state machine
> */
>
> #include <linux/config.h>
>@@ -705,6 +706,13 @@
> neigh_release(n);
> continue;
> }
>+ /* Move to NUD_STALE state */
>+ if (n->nud_state&NUD_REACHABLE &&
>+ now - n->confirmed > n->parms->reachable_time) {
Hmm, you're suffering tab -> space conversion syndrome :(
Grant.
^ permalink raw reply
* Re: [PATCH] acxsm: merge from acx 0.3.32
From: John W. Linville @ 2006-02-08 2:10 UTC (permalink / raw)
To: Denis Vlasenko; +Cc: netdev, acx100-devel
In-Reply-To: <200602071741.46065.vda@ilport.com.ua>
On Tue, Feb 07, 2006 at 05:41:45PM +0200, Denis Vlasenko wrote:
> On Friday 03 February 2006 14:14, Denis Vlasenko wrote:
> > Standalone acx driver had several fixes since
> > acxsm fork, this patch merges them:
> > - initial support for new TNETW1450 USB chip
> > - support for firmware 2.3.1.31
> >
> > Also we had one report that acxsm is actually working.
> > That's quite unexpected.
> >
> > Signed-off-by: Denis Vlasenko <vda@ilport.com.ua>
>
> What is the status of this patch? Accepted? Rejected?
> Other (please specify): ____________________
I intened to merge it. I had a busy week last week, with some
personal obligations. I apologize for my slow speed. I'll try to
do better! :-)
John
--
John W. Linville
linville@tuxdriver.com
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
^ permalink raw reply
* Re: [Patch] 2.4.32 - Neighbour Cache (ARP) State machine bug Fixed
From: Pradeep Vincent @ 2006-02-08 1:50 UTC (permalink / raw)
To: Willy Tarreau; +Cc: David S. Miller, netdev, linux-kernel
In-Reply-To: <20060207215341.GC11380@w.ods.org>
[-- Attachment #1: Type: text/plain, Size: 2887 bytes --]
One more attempt. Attaching the diff file as well.
Signed off by: Pradeep Vincent <pradeep.vincent@gmail.com>
--- old/net/core/neighbour.c Wed Nov 9 16:48:10 2005
+++ new/net/core/neighbour.c Tue Feb 7 17:38:26 2006
@@ -14,6 +14,7 @@
* Vitaly E. Lavrov releasing NULL neighbor in neigh_add.
* Harald Welte Add neighbour cache statistics like rtstat
* Harald Welte port neighbour cache rework from 2.6.9-rcX
+ * Pradeep Vincent fix neighbour cache state machine
*/
#include <linux/config.h>
@@ -705,6 +706,13 @@
neigh_release(n);
continue;
}
+ /* Move to NUD_STALE state */
+ if (n->nud_state&NUD_REACHABLE &&
+ now - n->confirmed > n->parms->reachable_time) {
+ n->nud_state = NUD_STALE;
+ neigh_suspect(n);
+ }
+
write_unlock(&n->lock);
next_elt:
Thanks,
Pradeep
On 2/7/06, Willy Tarreau <willy@w.ods.org> wrote:
> Hi,
>
> On Tue, Feb 07, 2006 at 12:57:43AM -0700, Pradeep Vincent wrote:
> > In 2.4.21, arp code uses gc_timer to check for stale arp cache
> > entries. In 2.6, each entry has its own timer to check for stale arp
> > cache. 2.4.29 to 2.4.32 kernels (atleast) use neither of these timers.
> > This causes problems in environments where IPs or MACs are reassigned
> > - saw this problem on load balancing router based networks that use
> > VMACs. Tested this code on load balancing router based networks as
> > well as peer-linux systems.
> >
> >
> > Thanks,
> >
> >
> > Signed off by: Pradeep Vincent <pradeep.vincent@gmail.com>
> >
> > diff -Naur old/net/core/neighbour.c new/net/core/neighbour.c
> > --- old/net/core/neighbour.c Wed Nov 23 17:15:30 2005
> > +++ new/net/core/neighbour.c Wed Nov 23 17:26:01 2005
> > @@ -14,6 +14,7 @@
> > * Vitaly E. Lavrov releasing NULL neighbor in neigh_add.
> > * Harald Welte Add neighbour cache statistics like rtstat
> > * Harald Welte port neighbour cache rework from 2.6.9-rcX
> > + * Pradeep Vincent Move neighbour cache entry to stale state
> > */
>
> As you can see above, your mailer is still broken. Leading spaces get
> removed and it seems like tabs are replaced with spaces. This makes it
> really annoying to fix by hand because we all have to do your work again.
> You should try to fix your mailer options, possibly by sending a few
> mails to yourself or someone else (if you send *a few* mails to me, I
> can confirm which one looks OK). If your mailer is definitely broken,
> then you may send it as plain text first (for review), with a text
> attachment for people to apply it without trouble.
>
> Thanks,
> Willy
>
>
[-- Attachment #2: linux-2.4.29-arp-fix.patch --]
[-- Type: application/octet-stream, Size: 691 bytes --]
--- old/net/core/neighbour.c Wed Nov 9 16:48:10 2005
+++ new/net/core/neighbour.c Tue Feb 7 17:38:26 2006
@@ -14,6 +14,7 @@
* Vitaly E. Lavrov releasing NULL neighbor in neigh_add.
* Harald Welte Add neighbour cache statistics like rtstat
* Harald Welte port neighbour cache rework from 2.6.9-rcX
+ * Pradeep Vincent fix neighbour cache state machine
*/
#include <linux/config.h>
@@ -705,6 +706,13 @@
neigh_release(n);
continue;
}
+ /* Move to NUD_STALE state */
+ if (n->nud_state&NUD_REACHABLE &&
+ now - n->confirmed > n->parms->reachable_time) {
+ n->nud_state = NUD_STALE;
+ neigh_suspect(n);
+ }
+
write_unlock(&n->lock);
next_elt:
^ permalink raw reply
* Re: IBM_EMAC_PHY_RX_CLK_FIX depends on non-existing option 440GR
From: Eugene Surovegin @ 2006-02-07 22:57 UTC (permalink / raw)
To: Adrian Bunk
Cc: linuxppc-dev, netdev, linux-kernel, Jean-Luc Leger,
linuxppc-embedded
In-Reply-To: <20060207221449.GB3524@stusta.de>
On Tue, Feb 07, 2006 at 11:14:49PM +0100, Adrian Bunk wrote:
> Jean-Luc Leger <reiga@dspnet.fr.eu.org> reported the following:
>
> from drivers/net/Kconfig:
> config IBM_EMAC_PHY_RX_CLK_FIX
> bool "PHY Rx clock workaround"
> depends on IBM_EMAC && (405EP || 440GX || 440EP || 440GR)
> -> maybe this is 440GP ?
>
>
> The non-existing CONFIG_440GR is also present in the driver itself.
>
> Is this a typo or a not yet merged platform?
Not yet merged platform.
--
Eugene
^ permalink raw reply
* IBM_EMAC_PHY_RX_CLK_FIX depends on non-existing option 440GR
From: Adrian Bunk @ 2006-02-07 22:14 UTC (permalink / raw)
To: ebs; +Cc: linuxppc-embedded, netdev, linuxppc-dev, linux-kernel,
Jean-Luc Leger
Jean-Luc Leger <reiga@dspnet.fr.eu.org> reported the following:
from drivers/net/Kconfig:
config IBM_EMAC_PHY_RX_CLK_FIX
bool "PHY Rx clock workaround"
depends on IBM_EMAC && (405EP || 440GX || 440EP || 440GR)
-> maybe this is 440GP ?
The non-existing CONFIG_440GR is also present in the driver itself.
Is this a typo or a not yet merged platform?
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox