Hello,
please see attached patches.
1st patch adds return value ENOENT to sss_iface_addr_list_get() so I can
provide more concrete debug message for missing interface or if
interface is not suitable (missing IP address)
2nd patch:
* I introduced new public function sss_iface_addr_concatenate(), I'm
aware that this function is probably not reusable but I needed to work
around that 'struct sss_iface_addr' in defined in source file only.
* I had troubles with correctly handling creating talloc hiearchy of IPs
of different interfaces. I decided to use first address of first found
interface as a parent talloc context for other interfaces. I attached
talloc report output to illustrate this.
1.
full talloc report on 'struct sdap_dyndns_get_addrs_state' (total
16 bytes in 1 blocks)
2.
full talloc report on 'struct sdap_dyndns_get_addrs_state' (total
376 bytes in 19 blocks)
3.
struct sss_iface_addr contains 360 bytes in 18
blocks (ref 0) 0xbc0650
4.
struct sss_iface_addr contains 120 bytes in 6 blocks
(ref 0) 0xbecee0
5.
struct sss_iface_addr contains 80 bytes in 4 blocks
(ref 0) 0xbeb920
6.
struct sss_iface_addr contains 40 bytes in 2
blocks (ref 0) 0xbd03f0
7.
../src/providers/dp_dyndns.c:219 contains 16 bytes in
1 blocks (ref 0) 0xbd0470
8.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1
blocks (ref 0) 0xbeb9a0
9.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks
(ref 0) 0xbecf60
10.
struct sss_iface_addr contains 120 bytes in 6 blocks
(ref 0) 0xbd0640
11.
struct sss_iface_addr contains 80 bytes in 4 blocks
(ref 0) 0xbd19a0
12.
struct sss_iface_addr contains 40 bytes in 2
blocks (ref 0) 0xbcfb00
13.
../src/providers/dp_dyndns.c:219 contains 16 bytes in
1 blocks (ref 0) 0xbed300
14.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1
blocks (ref 0) 0xbd1a20
15.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks
(ref 0) 0xbd06c0
16.
struct sss_iface_addr contains 80 bytes in 4 blocks
(ref 0) 0xbd0eb0
17.
struct sss_iface_addr contains 40 bytes in 2 blocks
(ref 0) 0xbd1900
18.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1
blocks (ref 0) 0xbec4f0
19.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks
(ref 0) 0xbeca10
20.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks
(ref 0) 0xbe6ae0
* I was thinking whether it would be a good idea to handle the case when
processing of interfaces provided in dyndns_iface yields no address at
all by continuing to detect DYNDNS address from LDAP connection?
Thanks!
>From 9ac270075b6532e96394543136a27cbf8a51cca6 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <[email protected]>
Date: Wed, 8 Jul 2015 09:01:24 -0400
Subject: [PATCH 1/2] DYNDNS: sss_iface_addr_list_get return ENOENT
If none of eligible interfaces matches ifname then ENOENT is returned.
Resolves:
https://fedorahosted.org/sssd/ticket/2558
---
src/providers/dp_dyndns.c | 8 +++++++-
src/providers/ldap/sdap_dyndns.c | 4 ++++
src/tests/cmocka/test_dyndns.c | 20 ++++++++++++++++++++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index 1cac3d0fae2454cea823ed640a4325f27580353f..9552e83d8d4ba615c157c9bae275c8ab867ec274 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -222,7 +222,13 @@ sss_iface_addr_list_get(TALLOC_CTX *mem_ctx, const char *ifname,
}
}
- ret = EOK;
+ if (addrlist != NULL) {
+ /* OK, some result was found */
+ ret = EOK;
+ } else {
+ /* No result was found */
+ ret = ENOENT;
+ }
*_addrlist = addrlist;
done:
freeifaddrs(ifaces);
diff --git a/src/providers/ldap/sdap_dyndns.c b/src/providers/ldap/sdap_dyndns.c
index 0d9c9205792062378aa25aad6ac706058001433a..bb98f8e1ec6852b04e9ea3e8d5807fecc7e6958d 100644
--- a/src/providers/ldap/sdap_dyndns.c
+++ b/src/providers/ldap/sdap_dyndns.c
@@ -504,6 +504,10 @@ sdap_dyndns_get_addrs_send(TALLOC_CTX *mem_ctx,
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
"Cannot get list of addresses from interface %s\n", iface);
+ /* non critical failure */
+ if (ret == ENOENT) {
+ ret = EOK;
+ }
}
/* We're done. Just fake an async request completion */
goto done;
diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index 689e333d4e68c4a2582894d06b8b7b20c76b9be8..3214e90c063ea9a4cf6f6bc6507bf4e37b7d23a4 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -247,6 +247,23 @@ void dyndns_test_get_multi_ifaddr(void **state)
assert_true(check_leaks_pop(dyndns_test_ctx) == true);
}
+void dyndns_test_get_ifaddr_enoent(void **state)
+{
+ errno_t ret;
+ struct sss_iface_addr *addrlist = NULL;
+
+ check_leaks_push(dyndns_test_ctx);
+ will_return_getifaddrs("eth0", "192.168.0.1");
+ will_return_getifaddrs("eth1", "192.168.0.2");
+ will_return_getifaddrs(NULL, NULL); /* sentinel */
+ ret = sss_iface_addr_list_get(dyndns_test_ctx, "non_existing_interface",
+ &addrlist);
+ assert_int_equal(ret, ENOENT);
+ talloc_free(addrlist);
+
+ assert_true(check_leaks_pop(dyndns_test_ctx) == true);
+}
+
void dyndns_test_ok(void **state)
{
struct tevent_req *req;
@@ -460,6 +477,9 @@ int main(int argc, const char *argv[])
cmocka_unit_test_setup_teardown(dyndns_test_get_multi_ifaddr,
dyndns_test_simple_setup,
dyndns_test_teardown),
+ cmocka_unit_test_setup_teardown(dyndns_test_get_ifaddr_enoent,
+ dyndns_test_simple_setup,
+ dyndns_test_teardown),
/* Dynamic DNS update unit tests*/
cmocka_unit_test_setup_teardown(dyndns_test_ok,
--
2.4.3
>From b00f3e9c380d4e2c28b2a4ad2cb07580b136762f Mon Sep 17 00:00:00 2001
From: Pavel Reichl <[email protected]>
Date: Wed, 8 Jul 2015 09:08:03 -0400
Subject: [PATCH 2/2] DYNDNS: support mult. interfaces for dyndns_iface opt
Resolves:
https://fedorahosted.org/sssd/ticket/2549
---
src/man/sssd-ad.5.xml | 7 ++--
src/man/sssd-ipa.5.xml | 7 ++--
src/providers/dp_dyndns.c | 6 ++++
src/providers/dp_dyndns.h | 4 +++
src/providers/ldap/sdap_dyndns.c | 72 +++++++++++++++++++++++++++++++++++-----
5 files changed, 84 insertions(+), 12 deletions(-)
diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml
index 938a443e027b9bf83c75c240a7d6b2a0876b92c8..4e2aedb57e3d8e1ce5b674e31c6a3d1fe293d4cb 100644
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -754,15 +754,18 @@ ad_gpo_map_deny = +my_pam_service
<listitem>
<para>
Optional. Applicable only when dyndns_update
- is true. Choose the interface whose IP address
+ is true. Choose the interfaces whose IP addresses
should be used for dynamic DNS updates.
</para>
<para>
- NOTE: This option currently supports only one interface.
+ NOTE: This option supports multiple interfaces.
</para>
<para>
Default: Use the IP address of the AD LDAP connection
</para>
+ <para>
+ Example: dyndns_iface = em1, vnet1, vnet2
+ </para>
</listitem>
</varlistentry>
diff --git a/src/man/sssd-ipa.5.xml b/src/man/sssd-ipa.5.xml
index 0716b6235f93965170983856b930799bfded6258..ca59ffbb005a6f4129362276ce57faa6a9e1af5c 100644
--- a/src/man/sssd-ipa.5.xml
+++ b/src/man/sssd-ipa.5.xml
@@ -166,11 +166,11 @@
<listitem>
<para>
Optional. Applicable only when dyndns_update
- is true. Choose the interface whose IP address
+ is true. Choose the interfaces whose IP addresses
should be used for dynamic DNS updates.
</para>
<para>
- NOTE: This option currently supports only one interface.
+ NOTE: This option currently supports multiple interfaces.
</para>
<para>
NOTE: While it is still possible to use the old
@@ -181,6 +181,9 @@
<para>
Default: Use the IP address of the IPA LDAP connection
</para>
+ <para>
+ Example: dyndns_iface = em1, vnet1, vnet2
+ </para>
</listitem>
</varlistentry>
diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index 9552e83d8d4ba615c157c9bae275c8ab867ec274..dec521054e18e083ba0557dbf9c8bfe918d4575e 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -49,6 +49,12 @@ struct sss_iface_addr {
struct sockaddr_storage *addr;
};
+void sss_iface_addr_concatenate(struct sss_iface_addr **list,
+ struct sss_iface_addr *list2)
+{
+ DLIST_CONCATENATE((*list), list2, struct sss_iface_addr*);
+}
+
struct sss_iface_addr *
sss_iface_addr_add(TALLOC_CTX *mem_ctx, struct sss_iface_addr **list,
struct sockaddr_storage *ss)
diff --git a/src/providers/dp_dyndns.h b/src/providers/dp_dyndns.h
index 23b833dace58a0ecbb1e2e21963a55186f1d06a8..deba112538ad22cd7f59be07934778ee9d4361e7 100644
--- a/src/providers/dp_dyndns.h
+++ b/src/providers/dp_dyndns.h
@@ -128,4 +128,8 @@ nsupdate_get_addrs_recv(struct tevent_req *req,
struct sss_iface_addr **_addrlist,
size_t *_count);
+void
+sss_iface_addr_concatenate(struct sss_iface_addr **list,
+ struct sss_iface_addr *list2);
+
#endif /* DP_DYNDNS_H_ */
diff --git a/src/providers/ldap/sdap_dyndns.c b/src/providers/ldap/sdap_dyndns.c
index bb98f8e1ec6852b04e9ea3e8d5807fecc7e6958d..fd47429b2b05b45a78e6552e62e388ece2e16eb4 100644
--- a/src/providers/ldap/sdap_dyndns.c
+++ b/src/providers/ldap/sdap_dyndns.c
@@ -482,6 +482,65 @@ static void sdap_dyndns_get_addrs_done(struct tevent_req *subreq);
static errno_t sdap_dyndns_add_ldap_conn(struct sdap_dyndns_get_addrs_state *state,
struct sdap_handle *sh);
+static errno_t get_ifaces_addrs(TALLOC_CTX *mem_ctx,
+ const char *iface,
+ struct sss_iface_addr **_result)
+{
+ struct sss_iface_addr *result_addrs = NULL;
+ struct sss_iface_addr *intf_addrs;
+ TALLOC_CTX *tmp_ctx;
+ char **list_of_intfs;
+ int num_of_intfs;
+ errno_t ret;
+ int i;
+
+ tmp_ctx = talloc_new(NULL);
+ if (tmp_ctx == NULL) {
+ ret = ENOENT;
+ goto done;
+ }
+
+ ret = split_on_separator(tmp_ctx, iface, ',', true, true, &list_of_intfs,
+ &num_of_intfs);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Parsing names of interfaces failed - %d:[%s].\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+
+ for (i = 0; i < num_of_intfs; i++) {
+ ret = sss_iface_addr_list_get(tmp_ctx, list_of_intfs[i], &intf_addrs);
+ if (ret == EOK) {
+ if (result_addrs != NULL) {
+ /* If there is already an existing list, head of this existing
+ * list will be considered as parent talloc context for the
+ * new list.
+ */
+ talloc_steal(result_addrs, intf_addrs);
+ }
+ sss_iface_addr_concatenate(&result_addrs, intf_addrs);
+ } else if (ret == ENOENT) {
+ /* non-critical failure */
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Cannot get interface %s or there are no addresses "
+ "bind to it.\n", list_of_intfs[i]);
+ } else {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "Cannot get list of addresses from interface %s - %d:[%s]\n",
+ list_of_intfs[i], ret, sss_strerror(ret));
+ goto done;
+ }
+ }
+
+ ret = EOK;
+
+done:
+ *_result = talloc_steal(mem_ctx, result_addrs);
+ talloc_free(tmp_ctx);
+ return ret;
+}
+
static struct tevent_req *
sdap_dyndns_get_addrs_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
@@ -500,14 +559,11 @@ sdap_dyndns_get_addrs_send(TALLOC_CTX *mem_ctx,
}
if (iface) {
- ret = sss_iface_addr_list_get(state, iface, &state->addresses);
- if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE,
- "Cannot get list of addresses from interface %s\n", iface);
- /* non critical failure */
- if (ret == ENOENT) {
- ret = EOK;
- }
+ ret = get_ifaces_addrs(state, iface, &state->addresses);
+ if (ret != EOK || state->addresses == NULL) {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "get_ifaces_addrs() failed: %d:[%s]\n",
+ ret, sss_strerror(ret));
}
/* We're done. Just fake an async request completion */
goto done;
--
2.4.3
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
