Attention is currently required from: Arne Schwabe, Frank Lichtenheld, Răzvan
Cojocaru.
Hello Arne Schwabe, Răzvan Cojocaru,
I'd like you to reexamine a change. Please visit
https://gerrit.openvpn.net/c/openvpn/+/2177?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review-1 by Răzvan Cojocaru
Change subject: networking_sitnl: Reduce boilerplate code for parameter checks
......................................................................
networking_sitnl: Reduce boilerplate code for parameter checks
Also removes some inconsistencies and some
sign conversion warnings.
Change-Id: I6f5ca53d4288b0921610f7a3f84ad55d2ab49118
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/networking_sitnl.c
1 file changed, 74 insertions(+), 169 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/77/2177/2
diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 501c635..ee6229b 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -673,25 +673,37 @@
#ifdef ENABLE_SITNL
+static int
+get_ifindex(const char *func, const char *iface, uint32_t *ifindex)
+{
+ if (!iface)
+ {
+ msg(M_WARN, "%s: passed NULL interface", func);
+ return -EINVAL;
+ }
+
+ *ifindex = if_nametoindex(iface);
+ if (*ifindex == 0)
+ {
+ msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", func,
iface);
+ return -ENOENT;
+ }
+ return 0;
+}
+
+
int
net_iface_up(const openvpn_net_ctx_t *ctx, const char *iface, bool up)
{
struct sitnl_link_req req;
- int ifindex;
+ uint32_t ifindex;
CLEAR(req);
- if (!iface)
+ const int ret = get_ifindex(__func__, iface, &ifindex);
+ if (ret < 0)
{
- msg(M_WARN, "%s: passed NULL interface", __func__);
- return -EINVAL;
- }
-
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN, "%s: rtnl: cannot get ifindex for %s: %s", __func__,
iface, strerror(errno));
- return -ENOENT;
+ return ret;
}
req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));
@@ -699,7 +711,7 @@
req.n.nlmsg_type = RTM_SETLINK;
req.i.ifi_family = AF_PACKET;
- req.i.ifi_index = ifindex;
+ req.i.ifi_index = (int)ifindex;
req.i.ifi_change |= IFF_UP;
if (up)
{
@@ -707,7 +719,7 @@
}
else
{
- req.i.ifi_flags &= ~IFF_UP;
+ req.i.ifi_flags &= ~(unsigned int)IFF_UP;
}
msg(M_INFO, "%s: set %s %s", __func__, iface, up ? "up" : "down");
@@ -719,14 +731,13 @@
net_iface_mtu_set(const openvpn_net_ctx_t *ctx, const char *iface, uint32_t
mtu)
{
struct sitnl_link_req req;
- int ifindex, ret = -1;
+ int ret = -1;
+ uint32_t ifindex;
CLEAR(req);
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
+ if (get_ifindex(__func__, iface, &ifindex) < 0)
{
- msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
iface);
return -1;
}
@@ -735,7 +746,7 @@
req.n.nlmsg_type = RTM_SETLINK;
req.i.ifi_family = AF_PACKET;
- req.i.ifi_index = ifindex;
+ req.i.ifi_index = (int)ifindex;
SITNL_ADDATTR(&req.n, sizeof(req), IFLA_MTU, &mtu, 4);
@@ -750,14 +761,13 @@
net_addr_ll_set(const openvpn_net_ctx_t *ctx, const openvpn_net_iface_t
*iface, uint8_t *addr)
{
struct sitnl_link_req req;
- int ifindex, ret = -1;
+ int ret = -1;
+ uint32_t ifindex;
CLEAR(req);
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
+ if (get_ifindex(__func__, iface, &ifindex) < 0)
{
- msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
iface);
return -1;
}
@@ -766,7 +776,7 @@
req.n.nlmsg_type = RTM_SETLINK;
req.i.ifi_family = AF_PACKET;
- req.i.ifi_index = ifindex;
+ req.i.ifi_index = (int)ifindex;
SITNL_ADDATTR(&req.n, sizeof(req), IFLA_ADDRESS, addr, OPENVPN_ETH_ALEN);
@@ -778,22 +788,19 @@
}
static int
-sitnl_addr_set(uint16_t cmd, uint16_t flags, int ifindex, sa_family_t
af_family,
+sitnl_addr_set(uint16_t cmd, uint16_t flags, const char *iface, sa_family_t
af_family,
const inet_address_t *local, const inet_address_t *remote, int
prefixlen)
{
struct sitnl_addr_req req;
- uint32_t size;
- int ret = -EINVAL;
+ size_t size;
+ uint32_t ifindex;
- CLEAR(req);
-
- req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));
- req.n.nlmsg_type = cmd;
- req.n.nlmsg_flags = NLM_F_REQUEST | flags;
-
- req.i.ifa_index = ifindex;
- ASSERT(af_family <= UINT8_MAX);
- req.i.ifa_family = (uint8_t)af_family;
+ int ret = get_ifindex(__func__, iface, &ifindex);
+ if (ret < 0)
+ {
+ return ret;
+ }
+ ret = -EINVAL; /* for SITNL_ADDATTR */
switch (af_family)
{
@@ -810,10 +817,20 @@
return -EINVAL;
}
+ CLEAR(req);
+
+ req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.i));
+ req.n.nlmsg_type = cmd;
+ req.n.nlmsg_flags = NLM_F_REQUEST | flags;
+
+ req.i.ifa_index = ifindex;
+ ASSERT(af_family <= UINT8_MAX);
+ req.i.ifa_family = (uint8_t)af_family;
+
/* if no prefixlen has been specified, assume host address */
if (prefixlen == 0)
{
- prefixlen = size * 8;
+ prefixlen = (int)size * 8;
}
ASSERT(prefixlen <= UINT8_MAX);
req.i.ifa_prefixlen = (uint8_t)prefixlen;
@@ -848,76 +865,37 @@
sitnl_addr_ptp_add(sa_family_t af_family, const char *iface, const
inet_address_t *local,
const inet_address_t *remote)
{
- int ifindex;
-
- switch (af_family)
- {
- case AF_INET:
- case AF_INET6:
- break;
-
- default:
- return -EINVAL;
- }
-
- if (!iface)
- {
- msg(M_WARN, "%s: passed NULL interface", __func__);
- return -EINVAL;
- }
-
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN, "%s: cannot get ifindex for %s: %s", __func__, np(iface),
strerror(errno));
- return -ENOENT;
- }
-
- return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, ifindex,
af_family, local,
+ return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, iface,
af_family, local,
remote, 0);
}
static int
sitnl_addr_ptp_del(sa_family_t af_family, const char *iface, const
inet_address_t *local)
{
- int ifindex;
-
- switch (af_family)
- {
- case AF_INET:
- case AF_INET6:
- break;
-
- default:
- return -EINVAL;
- }
-
- if (!iface)
- {
- msg(M_WARN, "%s: passed NULL interface", __func__);
- return -EINVAL;
- }
-
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN | M_ERRNO, "%s: cannot get ifindex for %s", __func__,
iface);
- return -ENOENT;
- }
-
- return sitnl_addr_set(RTM_DELADDR, 0, ifindex, af_family, local, NULL, 0);
+ return sitnl_addr_set(RTM_DELADDR, 0, iface, af_family, local, NULL, 0);
}
static int
-sitnl_route_set(uint16_t cmd, uint16_t flags, int ifindex, sa_family_t
af_family, const void *dst,
+sitnl_route_set(uint16_t cmd, uint16_t flags, const char *iface, sa_family_t
af_family, const void *dst,
int prefixlen, const void *gw, enum rt_class_t table, int
metric,
enum rt_scope_t scope, unsigned char protocol, unsigned char
type)
{
struct sitnl_route_req req;
- int ret = -1, size;
+ int ret = -1;
+ size_t size;
+ uint32_t ifindex = 0;
CLEAR(req);
+ if (iface)
+ {
+ const int ret = get_ifindex(__func__, iface, &ifindex);
+ if (ret < 0)
+ {
+ return ret;
+ }
+ }
+
switch (af_family)
{
case AF_INET:
@@ -982,64 +960,14 @@
static int
sitnl_addr_add(sa_family_t af_family, const char *iface, const inet_address_t
*addr, int prefixlen)
{
- int ifindex;
-
- switch (af_family)
- {
- case AF_INET:
- case AF_INET6:
- break;
-
- default:
- return -EINVAL;
- }
-
- if (!iface)
- {
- msg(M_WARN, "%s: passed NULL interface", __func__);
- return -EINVAL;
- }
-
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
iface);
- return -ENOENT;
- }
-
- return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, ifindex,
af_family, addr, NULL,
+ return sitnl_addr_set(RTM_NEWADDR, NLM_F_CREATE | NLM_F_REPLACE, iface,
af_family, addr, NULL,
prefixlen);
}
static int
sitnl_addr_del(sa_family_t af_family, const char *iface, const inet_address_t
*addr, int prefixlen)
{
- int ifindex;
-
- switch (af_family)
- {
- case AF_INET:
- case AF_INET6:
- break;
-
- default:
- return -EINVAL;
- }
-
- if (!iface)
- {
- msg(M_WARN, "%s: passed NULL interface", __func__);
- return -EINVAL;
- }
-
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN | M_ERRNO, "%s: rtnl: cannot get ifindex for %s", __func__,
iface);
- return -ENOENT;
- }
-
- return sitnl_addr_set(RTM_DELADDR, 0, ifindex, af_family, addr, NULL,
prefixlen);
+ return sitnl_addr_set(RTM_DELADDR, 0, iface, af_family, addr, NULL,
prefixlen);
}
int
@@ -1174,17 +1102,6 @@
const void *gw, uint32_t table, int metric)
{
enum rt_scope_t scope = RT_SCOPE_UNIVERSE;
- int ifindex = 0;
-
- if (iface)
- {
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifindex for %s",
__func__, iface);
- return -ENOENT;
- }
- }
if (table == 0)
{
@@ -1196,7 +1113,7 @@
scope = RT_SCOPE_LINK;
}
- return sitnl_route_set(RTM_NEWROUTE, NLM_F_CREATE, ifindex, af_family,
dst, prefixlen, gw,
+ return sitnl_route_set(RTM_NEWROUTE, NLM_F_CREATE, iface, af_family, dst,
prefixlen, gw,
table, metric, scope, RTPROT_BOOT, RTN_UNICAST);
}
@@ -1258,24 +1175,12 @@
sitnl_route_del(const char *iface, sa_family_t af_family, const inet_address_t
*dst, int prefixlen,
const inet_address_t *gw, uint32_t table, int metric)
{
- int ifindex = 0;
-
- if (iface)
- {
- ifindex = if_nametoindex(iface);
- if (ifindex == 0)
- {
- msg(M_WARN | M_ERRNO, "%s: rtnl: can't get ifindex for %s",
__func__, iface);
- return -ENOENT;
- }
- }
-
if (table == 0)
{
table = RT_TABLE_MAIN;
}
- return sitnl_route_set(RTM_DELROUTE, 0, ifindex, af_family, dst,
prefixlen, gw, table, metric,
+ return sitnl_route_set(RTM_DELROUTE, 0, iface, af_family, dst, prefixlen,
gw, table, metric,
RT_SCOPE_NOWHERE, 0, 0);
}
@@ -1439,7 +1344,7 @@
net_iface_type(const openvpn_net_ctx_t *ctx, const char *iface, char
type[IFACE_TYPE_LEN_MAX])
{
struct sitnl_link_req req = {};
- int ifindex = if_nametoindex(iface);
+ int ifindex = (int)if_nametoindex(iface);
if (!ifindex)
{
@@ -1471,7 +1376,7 @@
net_iface_del(const openvpn_net_ctx_t *ctx, const char *iface)
{
struct sitnl_link_req req = {};
- int ifindex = if_nametoindex(iface);
+ int ifindex = (int)if_nametoindex(iface);
if (!ifindex)
{
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/2177?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6f5ca53d4288b0921610f7a3f84ad55d2ab49118
Gerrit-Change-Number: 2177
Gerrit-PatchSet: 2
Gerrit-Owner: Frank Lichtenheld <[email protected]>
Gerrit-Reviewer: Arne Schwabe <[email protected]>
Gerrit-Reviewer: Răzvan Cojocaru <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Răzvan Cojocaru <[email protected]>
Gerrit-Attention: Arne Schwabe <[email protected]>
Gerrit-Attention: Frank Lichtenheld <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
