[PATCH] Unix datagram socket messaging

Stefan (metze) Metzmacher metze at samba.org
Wed Apr 16 04:55:54 MDT 2014


Hi Volker,

>> The notify patches seem to be independent, can you move them to be the
>> first patches?
> 
> They are not entirely independent. They are not necessary
> with the current messaging, because we handle messages
> always before the client requests. I do not want to put in
> additional complexity to the notify code before it is clear
> that the rest of the patches can go in at all.
> 
>> The same applies to "printing_cups: Call the msg_ctx destructor on exit"
>> and "smbd: Call the msg_ctx destructor for background jobs".
> 
> Same here. They are not necessary without the rest of the
> patches and just add complexity.
> 
>> iov_buf* can also be added first and then used in the unix_msg code
>> instead of the static copy.
> 
> I want to avoid making unix_msg depend on the rest of Samba,
> just as I want to make for example pthreadpool independent.
> 
> If you think this is overkill, I can rewrite this code to
> make use of the tevent_req infrastructure and the rest of
> Samba. Do you want me to do this?

No, it's fine, then just leave it as static copy.

While it's not strictly needed and I agree that there's no need to
push the more or less independent patches before they're really needed.
I'd like to have them as first patches in the patchset, this way
each commit would be able to pass make test (in theory, I don't want you
to prove
that :-).

>> They all have "Reviewed-by: Stefan Metzmacher <metze at samba.org>"
>>
>> I haven't fully reviewed the real changes yet, but have some initial
>> comments:
>>
>> Can we pass tevent_context to poll_funcs_init_tevent() instead of
>> using: func.private_data = ev; in the caller?
> 
> Yes, I will do that.
> 
>> Are we sure we don't get problems when having all messaging sockets in
>> just one subdirectory?
> 
> My assumption is that by now all sane file systems have
> hashed directories and thus direct access should be fast.
> Also, we should not have more than a few thousand sockets
> per node. Is this a problem for modern file systems? If so,
> I can certainly convert this to a hashing scheme with
> 256/256 subdirectories.

Ok, just leave it as it is for now, we would be able to change it later
if it's really a problem.

>> There're environments with just winbindd running, so we might need a
>> periodic
>> cleanup run not only in smbd.
> 
> Ok, I will add that.
> 
>>
>> In
>> https://git.samba.org/?p=vl/samba.git/.git;a=commitdiff;h=777954434bb7404659fbb7f519a4069b1100c400
>> we have
>>
>> +       struct sockaddr_un addr = { 0, };
>>
>> ...
>>
>> +               ret = bind(ctx->sock, (struct sockaddr *)&addr,
>> sizeof(addr));
>>
>> I think we should cast with (struct sockaddr *)(void *)&addr, in order
>> to avoid
>> strict aliasing warnings on some systems.
> 
> The correct way to avoid this is to do a memcpy. I will
> convert to that.

I don't think so 'struct sockaddr' is too small, as far
as I can tell this is the only way to avoid this warnings,
other than defining a union, which would be overkill for this case.

metze


More information about the samba-technical mailing list