Re: Linux 2.4.7-ac9 (breaks ATM connect)

Re: Linux 2.4.7-ac9 (breaks ATM connect)

[Petr Vandrovec]

On  9 Aug 01 at 22:27, Sampsa Ranta wrote:
> On Thu, 9 Aug 2001, Sampsa Ranta wrote:
> 
> > -           vci != ATM_VCI_ANY && vci >> dev->ci_range.vci_bits))
> +           vci != ATM_VCI_ANY && vci >= 1 << dev->ci_range.vci_bits))

As these two are equivalent, probably just reverting patch back
to '>>' is easiest... Maybe that second one is more readable, but
I do not think.
                                        Best regards,
                                                Petr Vandrovec
                                                vandrove@vc.cvut.cz
                                                

Re: Linux 2.4.7-ac9 (breaks ATM connect)

[Chris Friesen]

"David S. Miller" wrote:
> 
> Sampsa Ranta writes:
>  > Pardon me, bugs come in too easy..
>  >
>  > -           vci != ATM_VCI_ANY && vci >> dev->ci_range.vci_bits))
>  > +           vci != ATM_VCI_ANY && vci >= 1 << dev->ci_range.vci_bits))
>  >
> 
> This is rediculious, why has this expression changed when right
> above it is the same thing:
> 
>           vpi >> dev->ci_range.vpi_bits) || (vci != ATM_VCI_UNSPEC &&
> 
> Shouldn't we be changing that "vpi >> dev->ci_range.vpi_bits" boolean
> test as well?

Am I missing something?  As long as vci is unsigned isn't (vci >>
dev->ci_range.vci_bits) as a boolean value exactly the same thing as (vci >= 1
<< dev->ci_range.vci_bits) ?

As an example, take vci = 10001b and dev->ci_range.vci_bits = 4.  The answer
works out the same either way.

Chris


-- 
Chris Friesen                    | MailStop: 043/33/F10  
Nortel Networks                  | work: (613) 765-0557
3500 Carling Avenue              | fax:  (613) 765-2986
Nepean, ON K2H 8E9 Canada        | email: cfriesen@nortelnetworks.com

Re: Linux 2.4.7-ac9 (breaks ATM connect)

[David S. Miller]

Sampsa Ranta writes:
 > Pardon me, bugs come in too easy..
 > 
 > -           vci != ATM_VCI_ANY && vci >> dev->ci_range.vci_bits))
 > +           vci != ATM_VCI_ANY && vci >= 1 << dev->ci_range.vci_bits))
 > 

This is rediculious, why has this expression changed when right
above it is the same thing:

          vpi >> dev->ci_range.vpi_bits) || (vci != ATM_VCI_UNSPEC &&

Shouldn't we be changing that "vpi >> dev->ci_range.vpi_bits" boolean
test as well?

Later,
David S. Miller
davem@redhat.com

Re: Linux 2.4.7-ac9 (breaks ATM connect)

[Sampsa Ranta]

On Thu, 9 Aug 2001, Sampsa Ranta wrote:

> -           vci != ATM_VCI_ANY && vci >> dev->ci_range.vci_bits))
> +           vci != ATM_VCI_ANY && vci > 1 << dev->ci_range.vci_bits))

 ..

>                     if (c >= 1 << vcc->dev->ci_range.vci_bits)

Pardon me, bugs come in too easy..

-           vci != ATM_VCI_ANY && vci >> dev->ci_range.vci_bits))
+           vci != ATM_VCI_ANY && vci >= 1 << dev->ci_range.vci_bits))

 - Sampsa Ranta
   sampsa@netsonic.fi

Re: Linux 2.4.7-ac9 (breaks ATM connect)

[Sampsa Ranta]

On Tue, 7 Aug 2001, Alan Cox wrote:

> o	Fix bug in atm_do_connect_dev (bogus EINVALs)	(Germán González)


--- linux.vanilla/net/atm/common.c      Wed Jul  4 12:21:53 2001
+++ linux.ac/net/atm/common.c   Wed Jul 25 16:28:24 2001
@@ -210,7 +210,7 @@

        if ((vpi != ATM_VPI_UNSPEC && vpi != ATM_VPI_ANY &&
            vpi >> dev->ci_range.vpi_bits) || (vci != ATM_VCI_UNSPEC &&
-           vci != ATM_VCI_ANY && vci >> dev->ci_range.vci_bits))
+           vci != ATM_VCI_ANY && vci > dev->ci_range.vci_bits))
                return -EINVAL;
        if (vci > 0 && vci < ATM_NOT_RSV_VCI &&
!capable(CAP_NET_BIND_SERVICE))
                return -EPERM;

The patch is not correct, this test compares number of allowed bits
to an integer. And due to this, breaks most of ATM. Even signalling
daemons cannot connect.

But yes, the original seems broken, too. I included code bits doing this
check elsewhere in ATM implementation code. I think the correct patch
would be

-           vci != ATM_VCI_ANY && vci >> dev->ci_range.vci_bits))
+           vci != ATM_VCI_ANY && vci > 1 << dev->ci_range.vci_bits))


Code bits taken from elsewhere:

net/atm/atm/atm_misc.c:
                    if (c >= 1 << vcc->dev->ci_range.vci_bits)

drivers/atm/eni.c:
	 dev->ci_range.vci_bits = NR_VCI_LD;

drivers/atm/midway.h:
#define NR_VCI_LD      10              /* log2(NR_VCI) */


Thanks,
   Sampsa Ranta
   sampsa@netsonic.fi