Skip to content

Commit 55963c4

Browse files
Andrii Sultanovlindig
authored andcommitted
CP-50108: Use Ipaddr instead of string-based CIDR handling
In nm.ml; helpers.ml. Expands test cases, keeps the existent exception behaviour. Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com>
1 parent d8fa301 commit 55963c4

File tree

6 files changed

+101
-41
lines changed

6 files changed

+101
-41
lines changed

ocaml/tests/test_helpers.ml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,14 @@ module IPCheckers = Generic.MakeStateless (struct
320320
; ( (`ipv6, "address5", "ze80::bae8:56ff:fe29:894a")
321321
, Error (Server_error (invalid_ip_address_specified, ["address5"]))
322322
)
323+
; ((`ipv4or6, "address5", "192.168.0.1"), Ok ())
324+
; ((`ipv4or6, "address6", "fe80::bae8:56ff:fe29:894a"), Ok ())
325+
; ( (`ipv4or6, "address7", "ze80::bae8:56ff:fe29:894a")
326+
, Error (Server_error (invalid_ip_address_specified, ["address7"]))
327+
)
328+
; ( (`ipv4or6, "address8", "192.168.0.300")
329+
, Error (Server_error (invalid_ip_address_specified, ["address8"]))
330+
)
323331
]
324332
end)
325333

@@ -399,6 +407,17 @@ module CIDRCheckers = Generic.MakeStateless (struct
399407
; ( (`ipv6, "address5", "ze80::bae8:56ff:fe29:894a/64")
400408
, Error (Server_error (invalid_cidr_address_specified, ["address5"]))
401409
)
410+
; ( (`ipv4or6, "address6", "bad-address/64")
411+
, Error (Server_error (invalid_cidr_address_specified, ["address6"]))
412+
)
413+
; ((`ipv4or6, "address7", "fe80::bae8:56ff:fe29:894a/64"), Ok ())
414+
; ( (`ipv4or6, "address8", "ze80::bae8:56ff:fe29:894a/64")
415+
, Error (Server_error (invalid_cidr_address_specified, ["address8"]))
416+
)
417+
; ((`ipv4or6, "address9", "255.255.255.0/32"), Ok ())
418+
; ( (`ipv4or6, "address10", "192.168.0.2/33")
419+
, Error (Server_error (invalid_cidr_address_specified, ["address10"]))
420+
)
402421
]
403422
end)
404423

ocaml/xapi-idl/network/dune

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
threads.posix
1111
xapi-idl
1212
xapi-log
13+
ipaddr
1314
)
1415
(wrapped false)
1516
(preprocess (pps ppx_deriving_rpc)))

ocaml/xapi-idl/network/network_interface.ml

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,27 @@ let comp f g x = f (g x)
3434
let ( ++ ) f g x = comp f g x
3535

3636
let netmask_to_prefixlen netmask =
37-
Scanf.sscanf netmask "%d.%d.%d.%d" (fun a b c d ->
38-
let rec length l x = if x > 0 then length (succ l) (x lsr 1) else l in
39-
let masks = List.map (( - ) 255) [a; b; c; d] in
40-
32 - List.fold_left length 0 masks
37+
let raise_on_ipaddr_err = function
38+
| `Msg str ->
39+
failwith
40+
(Printf.sprintf "%s: Failed to parse the netmask %s (%s)" __FUNCTION__
41+
netmask str
42+
)
43+
in
44+
Ipaddr.V4.(
45+
match of_string netmask with
46+
| Ok ip_t -> (
47+
match Prefix.of_netmask ~address:ip_t ~netmask:ip_t with
48+
| Ok x ->
49+
Prefix.bits x
50+
| Error e ->
51+
raise_on_ipaddr_err e
52+
)
53+
| Error e ->
54+
raise_on_ipaddr_err e
4155
)
4256

43-
let prefixlen_to_netmask len =
44-
let mask l =
45-
if l <= 0 then 0 else if l > 8 then 255 else 256 - (1 lsl (8 - l))
46-
in
47-
let lens = [len; len - 8; len - 16; len - 24] in
48-
let masks = List.map (string_of_int ++ mask) lens in
49-
String.concat "." masks
57+
let prefixlen_to_netmask len = Ipaddr.V4.(Prefix.mask len |> to_string)
5058

5159
module Unix = struct
5260
include Unix

ocaml/xapi/dune

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
http_lib
7878
httpsvr
7979
ipaddr
80+
ipaddr.unix
8081
magic-mime
8182
message-switch-core
8283
message-switch-unix

ocaml/xapi/helpers.ml

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,31 +1114,55 @@ let assert_is_valid_ip kind field address =
11141114
if not (is_valid_ip kind address) then
11151115
raise Api_errors.(Server_error (invalid_ip_address_specified, [field]))
11161116

1117+
module type AbstractIpaddr = sig
1118+
type t
1119+
1120+
module Prefix : sig
1121+
type addr = t
1122+
1123+
type t
1124+
1125+
val of_string : string -> (t, [> `Msg of string]) result
1126+
1127+
val address : t -> addr
1128+
1129+
val bits : t -> int
1130+
end
1131+
1132+
val to_string : t -> string
1133+
end
1134+
11171135
let parse_cidr kind cidr =
1118-
try
1119-
let address, prefixlen = Scanf.sscanf cidr "%s@/%d" (fun a p -> (a, p)) in
1120-
if not (is_valid_ip kind address) then (
1121-
error "Invalid address in CIDR (%s)" address ;
1122-
None
1123-
) else if
1124-
prefixlen < 0
1125-
|| (kind = `ipv4 && prefixlen > 32)
1126-
|| (kind = `ipv6 && prefixlen > 128)
1127-
then (
1128-
error "Invalid prefix length in CIDR (%d)" prefixlen ;
1129-
None
1130-
) else
1136+
let select_ip_family = function
1137+
| `ipv4 ->
1138+
(module Ipaddr.V4 : AbstractIpaddr)
1139+
| `ipv6 ->
1140+
(module Ipaddr.V6)
1141+
in
1142+
let module AddrParse = (val select_ip_family kind) in
1143+
match AddrParse.Prefix.of_string cidr with
1144+
| Ok ip_t ->
1145+
let address = AddrParse.Prefix.address ip_t |> AddrParse.to_string in
1146+
let prefixlen = AddrParse.Prefix.bits ip_t in
11311147
Some (address, prefixlen)
1132-
with _ ->
1133-
error "Invalid CIDR format (%s)" cidr ;
1134-
None
1148+
| Error e ->
1149+
let msg = match e with `Msg str -> str in
1150+
error "Invalid address in CIDR (%s). %s" cidr msg ;
1151+
None
1152+
1153+
let valid_cidr_aux kind cidr =
1154+
match kind with
1155+
| `ipv4or6 ->
1156+
parse_cidr `ipv4 cidr = None && parse_cidr `ipv6 cidr = None
1157+
| (`ipv4 | `ipv6) as kind ->
1158+
parse_cidr kind cidr = None
11351159

11361160
let assert_is_valid_cidr kind field cidr =
1137-
if parse_cidr kind cidr = None then
1161+
if valid_cidr_aux kind cidr then
11381162
raise Api_errors.(Server_error (invalid_cidr_address_specified, [field]))
11391163

11401164
let assert_is_valid_ip_addr kind field address =
1141-
if (not (is_valid_ip kind address)) && parse_cidr kind address = None then
1165+
if (not (is_valid_ip kind address)) && valid_cidr_aux kind address then
11421166
raise Api_errors.(Server_error (invalid_ip_address_specified, [field]))
11431167

11441168
(** Return true if the MAC is in the right format XX:XX:XX:XX:XX:XX *)

ocaml/xapi/nm.ml

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -613,18 +613,18 @@ let bring_pif_up ~__context ?(management_interface = false) (pif : API.ref_PIF)
613613
let addresses =
614614
List.filter_map
615615
(fun addr_and_prefixlen ->
616-
try
617-
let n = String.index addr_and_prefixlen '/' in
618-
let addr =
619-
Unix.inet_addr_of_string
620-
(String.sub addr_and_prefixlen 0 n)
616+
let ( let* ) = Option.bind in
617+
Ipaddr.V6.(
618+
let* ip_t =
619+
Result.to_option
620+
(Prefix.of_string addr_and_prefixlen)
621621
in
622-
let prefixlen =
623-
int_of_string
624-
(String.sub_to_end addr_and_prefixlen (n + 1))
622+
let addr =
623+
Prefix.address ip_t |> Ipaddr_unix.V6.to_inet_addr
625624
in
625+
let prefixlen = Prefix.bits ip_t in
626626
Some (addr, prefixlen)
627-
with _ -> None
627+
)
628628
)
629629
rc.API.pIF_IPv6
630630
in
@@ -714,9 +714,16 @@ let bring_pif_up ~__context ?(management_interface = false) (pif : API.ref_PIF)
714714
| [] ->
715715
""
716716
| hd :: _ -> (
717-
(* IPv6 addresses are stored with this format: <ipv6>/<cidr> *)
718-
match String.split_on_char '/' hd with [ip; _] -> ip | _ -> ""
719-
)
717+
(* IPv6 addresses are stored with this format: <ipv6>/<cidr> *)
718+
Ipaddr.V6.(
719+
match Prefix.of_string hd with
720+
| Ok ip_t ->
721+
Prefix.address ip_t |> to_string
722+
| _ ->
723+
debug "%s not an IPv6 prefix: %s" __FUNCTION__ hd ;
724+
""
725+
)
726+
)
720727
)
721728
in
722729
if new_ip <> pif_ip then (

0 commit comments

Comments
 (0)