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,

Reply via email to