On Thu, Feb 13, 2020 at 07:48:03AM +0100, Willy Tarreau wrote:
> For me it's different, it's not related to the fact that it's used
> later but to the fact that we only need to undo the namespace change
> if it was changed in the first call. Indeed "ns" is not null only
> when the caller wants to switch the namespace to create a socket. In
> fact as long as there's no namespace configured on the servers, or
> if we're trying to connect to a dispatch or transparent address, ns
> will be NULL and we'll save two setns calls (i.e. almost always the
> case).
indeed, the code was not very clear in that regard, thank you for the
clarification. I overlooked the case we only go through this function to
get a file descriptor without namespace.
> In fact I think we could simplify the logic a bit and merge the code
> into the inline function present in common/namespace.h. This would also
> require to export default_namespace:
>
>
> static inline int my_socketat(const struct netns_entry *ns, int domain, int
> type, int protocol)
> {
> #ifdef USE_NS
> int sock;
>
> if (likely(!ns || default_namespace < 0))
> goto no_ns;
>
> if (setns(ns->fd, CLONE_NEWNET) == -1)
> return -1;
>
> sock = socket(domain, type, protocol);
>
> if (setns(default_namespace, CLONE_NEWNET) == -1) {
> close(sock);
> sock = -1;
> }
> return sock;
> no_ns:
> #endif
> return socket(domain, type, protocol);
> }
>
> This allows to remove the !ns || default_namespace logic from
> the function's epilogue. What do you think ?
Indeed, that's clearer in my opinion. I guess I let you handle that
as you wrote the new proposition.
Thank you,
--
William
