Hi,
Amith reported these two issues.
[PATCH 1/2] Save original name into the in-memory cache We were saving the original name into the netgroup hash when the object was first constructed but when no domain matched, we would save the cased name. The first request that was constructed with the original name was never marked as "ready", so another request with the same name was stuck waiting.
[PATCH 2/2] setnetgrent return codes are handled in the _done callback, some of them gracefully, such as ENOENT that sends an empty reply. This patch uses tevent_req_post() to always call the callback and handle error conditions there. In particular, Amith was able to trigger this error by requesting a netgroup by raw name when all domains were marked as FQDN only
On Wed, 2012-03-07 at 20:47 +0100, Jakub Hrozek wrote:
Hi,
Amith reported these two issues.
[PATCH 1/2] Save original name into the in-memory cache We were saving the original name into the netgroup hash when the object was first constructed but when no domain matched, we would save the cased name. The first request that was constructed with the original name was never marked as "ready", so another request with the same name was stuck waiting.
Ack.
I was thinking about it for a while, and there's one slight performance issue here. Lookups in a case-insensitive domain might go to the sysdb more often than strictly necessary. The reason being that if I request "netgroup1" and then "NetGroup1", the second one will not match the hash table and it will go back to the sysdb.
That said, this is definitely an edge-case (and it will still be a local lookup, not a provider check), so the only real downside here is extra wasted memory as we will be keeping two copies of the netgroup info in the hash table. Probably not a sufficiently-large waste to try optimizing right now though, so this patch is fine as-is.
[PATCH 2/2] setnetgrent return codes are handled in the _done callback, some of them gracefully, such as ENOENT that sends an empty reply. This patch uses tevent_req_post() to always call the callback and handle error conditions there. In particular, Amith was able to trigger this error by requesting a netgroup by raw name when all domains were marked as FQDN only
Ack
On Thu, 2012-03-08 at 14:15 -0500, Stephen Gallagher wrote:
On Wed, 2012-03-07 at 20:47 +0100, Jakub Hrozek wrote:
Hi,
Amith reported these two issues.
[PATCH 1/2] Save original name into the in-memory cache We were saving the original name into the netgroup hash when the object was first constructed but when no domain matched, we would save the cased name. The first request that was constructed with the original name was never marked as "ready", so another request with the same name was stuck waiting.
Ack.
I was thinking about it for a while, and there's one slight performance issue here. Lookups in a case-insensitive domain might go to the sysdb more often than strictly necessary. The reason being that if I request "netgroup1" and then "NetGroup1", the second one will not match the hash table and it will go back to the sysdb.
That said, this is definitely an edge-case (and it will still be a local lookup, not a provider check), so the only real downside here is extra wasted memory as we will be keeping two copies of the netgroup info in the hash table. Probably not a sufficiently-large waste to try optimizing right now though, so this patch is fine as-is.
[PATCH 2/2] setnetgrent return codes are handled in the _done callback, some of them gracefully, such as ENOENT that sends an empty reply. This patch uses tevent_req_post() to always call the callback and handle error conditions there. In particular, Amith was able to trigger this error by requesting a netgroup by raw name when all domains were marked as FQDN only
Ack
Pushed both to master and sssd-1-8.
On Thu, Mar 08, 2012 at 02:15:22PM -0500, Stephen Gallagher wrote:
On Wed, 2012-03-07 at 20:47 +0100, Jakub Hrozek wrote:
Hi,
Amith reported these two issues.
[PATCH 1/2] Save original name into the in-memory cache We were saving the original name into the netgroup hash when the object was first constructed but when no domain matched, we would save the cased name. The first request that was constructed with the original name was never marked as "ready", so another request with the same name was stuck waiting.
Ack.
I was thinking about it for a while, and there's one slight performance issue here. Lookups in a case-insensitive domain might go to the sysdb more often than strictly necessary. The reason being that if I request "netgroup1" and then "NetGroup1", the second one will not match the hash table and it will go back to the sysdb.
That said, this is definitely an edge-case (and it will still be a local lookup, not a provider check), so the only real downside here is extra wasted memory as we will be keeping two copies of the netgroup info in the hash table. Probably not a sufficiently-large waste to try optimizing right now though, so this patch is fine as-is.
It's quite an edge-case, but worth tracking: https://fedorahosted.org/sssd/ticket/1244
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Stephen Gallagher