RE: Faulty method isNetworkAWithinNetworkB ?

2013-12-24 Thread Saksham Srivastava
Thanks Jayapal for reviewing.

I have updated the patch.
Now isNetworkAWithinNetworkB method uses IP ranges of cidrs  for comparison.
Also updated the unittests for the same.

Thanks,
Saksham

-Original Message-
From: Jayapal Reddy Uradi [mailto:jayapalreddy.ur...@citrix.com] 
Sent: Monday, December 23, 2013 6:03 PM
To: dev@cloudstack.apache.org
Subject: Re: Faulty method isNetworkAWithinNetworkB ?

Hi Saksham,

Always the higher suffix cidr will be in lower suffix cidr.
10.1.1.0/24 will have 256 addresses and 10.1.1.0/25 will have 128 addresses[1].

/25 will be completely in /24 but not wise versa. 

The below are incorrect.
 isNetworkAWithinNetworkB(10.1.1.0/24, 10.1.1.0/25) returns true 
 isNetworkAWithinNetworkB(10.1.1.0/22, 10.1.1.0/23) returns true

I think you can change isNetworkAWithinNetworkB method to compare respective ip 
ranges for cidrs.

What about changing method name isNetworkACompletelyWithinNetworkB() ?

[1]https://www.dan.me.uk/ipsubnets?ip=10.1.1.0


Thanks,
Jayapal

On 13-Dec-2013, at 4:49 PM, Saksham Srivastava saksham.srivast...@citrix.com 
wrote:

 Hi,
 
 I encountered a method isNetworkAWithinNetworkB(cidrA, cidrB) in 
 NetUtils.java which should return true if cidrA is a subset of cidrB.
 The method returns flawed output in many scenarios. After unittesting it I 
 found :
 
 isNetworkAWithinNetworkB(10.1.1.0/24, 10.1.1.0/25) returns true 
 isNetworkAWithinNetworkB(10.1.1.0/25, 10.1.1.0/24) returns true 
 isNetworkAWithinNetworkB(10.1.1.0/23, 10.1.1.0/22) returns true 
 isNetworkAWithinNetworkB(10.1.1.0/22, 10.1.1.0/23) returns true
 
 Due to this I am able to create VPC tiers with cidr 10.1.0.0/24 even 
 when the VPC super cidr has been defined as 10.1.1.0/25 IMO the 
 simpler/cleaner way to compare cidrs should be to compare the respective IP 
 ranges. I have an old patch [1] in RB which uses the IP ranges to compare 2 
 cidrs.
 We could leverage that to replace isNetworkAWithinNetworkB() or in case of 
 any other suggestions please share.
 
 Thanks,
 Saksham
 
 [1] https://reviews.apache.org/r/14124/diff/#index_header
 



Re: Faulty method isNetworkAWithinNetworkB ?

2013-12-23 Thread Jayapal Reddy Uradi
Hi Saksham,

Always the higher suffix cidr will be in lower suffix cidr.
10.1.1.0/24 will have 256 addresses and 10.1.1.0/25 will have 128 addresses[1].

/25 will be completely in /24 but not wise versa. 

The below are incorrect.
 isNetworkAWithinNetworkB(10.1.1.0/24, 10.1.1.0/25) returns true
 isNetworkAWithinNetworkB(10.1.1.0/22, 10.1.1.0/23) returns true

I think you can change isNetworkAWithinNetworkB method to compare respective ip 
ranges for cidrs.

What about changing method name isNetworkACompletelyWithinNetworkB() ?

[1]https://www.dan.me.uk/ipsubnets?ip=10.1.1.0


Thanks,
Jayapal

On 13-Dec-2013, at 4:49 PM, Saksham Srivastava saksham.srivast...@citrix.com 
wrote:

 Hi,
 
 I encountered a method isNetworkAWithinNetworkB(cidrA, cidrB) in 
 NetUtils.java which should return true if cidrA is a subset of cidrB.
 The method returns flawed output in many scenarios. After unittesting it I 
 found :
 
 isNetworkAWithinNetworkB(10.1.1.0/24, 10.1.1.0/25) returns true
 isNetworkAWithinNetworkB(10.1.1.0/25, 10.1.1.0/24) returns true
 isNetworkAWithinNetworkB(10.1.1.0/23, 10.1.1.0/22) returns true
 isNetworkAWithinNetworkB(10.1.1.0/22, 10.1.1.0/23) returns true
 
 Due to this I am able to create VPC tiers with cidr 10.1.0.0/24 even when the 
 VPC super cidr has been defined as 10.1.1.0/25
 IMO the simpler/cleaner way to compare cidrs should be to compare the 
 respective IP ranges. I have an old patch [1] in RB which uses the IP ranges 
 to compare 2 cidrs.
 We could leverage that to replace isNetworkAWithinNetworkB() or in case of 
 any other suggestions please share.
 
 Thanks,
 Saksham
 
 [1] https://reviews.apache.org/r/14124/diff/#index_header