osmo-ggsn[master]: ippool: Implement and use blacklist instead of blindly using...

Harald Welte gerrit-no-reply at lists.osmocom.org
Sun Oct 15 16:11:04 UTC 2017


Patch Set 2:

(4 comments)

https://gerrit.osmocom.org/#/c/4256/2/lib/in46_addr.c
File lib/in46_addr.c:

Line 198: unsigned int netmask_ipv4_prefixlen(const struct in_addr *netmask)
the new functions should have test cases in the (newly introduced) in46a_tests.c file.  Adding these three functions and their test cases can then be split out into a separate patch.  Also, if we don't need the ipv4/ipv6 specific functions in other code, they could be made static with in46a_prefixlne being the only exported API.  Also, doxygen documentation is missing.


Line 234: unsigned int in46a_prefixlen(const struct in46_addr *netmask)
let's call it in46a_netmasklen or the like?  I don't want to have a name that implies it returns the length of a prefix (whihc would take an in46_prefix as input argument anyway)


https://gerrit.osmocom.org/#/c/4256/2/lib/tun.c
File lib/tun.c:

Line 755: int tun_ipv4_local_get(const struct tun_t *tun, struct in46_prefix *prefix)
I think this is copy+paste style programming with the ipv6 function below, and the two should probably be unified. Would you agree?


https://gerrit.osmocom.org/#/c/4256/2/lib/tun.h
File lib/tun.h:

Line 97: int tun_ipv6_local_get(const struct tun_t *tun, struct in46_prefix *prefix, int flags);
the change of tun_ipv6_local_get() api with flags could be split out as separate commit


-- 
To view, visit https://gerrit.osmocom.org/4256
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e91f7280d60490c858a769dd578c1c8e54e9243
Gerrit-PatchSet: 2
Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes


More information about the gerrit-log mailing list