nmbd doesn't work when the socket address option is used.
Created attachment 5297 [details] Patch for v3-5
*** Bug 3851 has been marked as a duplicate of this bug. ***
*** Bug 4166 has been marked as a duplicate of this bug. ***
Created attachment 5300 [details] Additional git-am patch needed for v3-5. Here is one additional patch I think is necessary. It fixes a comment typo, turns on "nmbd bind explicit broadcast" by default, and fixes the manpage to show this (and be more explicit about what the option does). Metze please review. If you agree I'll re-assign to Karolin for inclusion in 3.5.1 (I think this is too late for 3.5.0). Jeremy.
Comment on attachment 5300 [details] Additional git-am patch needed for v3-5. Looks good for 3.5.0 But I'll propose the backport to 3.4.x without this change
If we change the default to yes, then we need it in 3.5.0. We can't change it within the 3.5.x releases. I'd really like to get the feature into 3.5.0, while I'm happy to keep it of by default.
...and that would allow people with problems to turn it on (and we have really fixed all the other bugreports)
Created attachment 5316 [details] Patch for v3-4
Jeremy, I think my patches should go into 3.5.0 and 3.4.6, please ack and assign to Karolin. And it's up to you if you want your patch in 3.5.0. (Changing the default with 3.5.1 is not an option, we need it in 3.5.0 or keep the old default for all 3.5.x releases)
Comment on attachment 5300 [details] Additional git-am patch needed for v3-5. Günther told me this: 14:30 < gd> Got a positive name query response from 172.31.1.50 ( 172.31.1.50 ) 14:30 < gd> 172.31.1.50 mthelena<00> 14:31 < gd> 172.31.1.50 mthelena<00> With the new default we process packets twice. So we should not change the default in any version. Jeremy please do a git revert f2415c9bf9342aeb5
Sorry I mean 3f2415c9bf934
Will do (once I'm in at work). I'll look into this asap and get a proper fix. Jeremy.
I really think the correct way to do this is to keep the old default, or also use the subnet records as outgoing interfaces. But I think we should just go with my patches only for 3.4.6 and 3.5.0. And may create a different solution for 3.6.0. Can you ack my changes so that Karolin can pick them?
Comment on attachment 5316 [details] Patch for v3-4 Sorry, I don't want to ack this whilst there is the bug that gd discovered. I'd rather not add code that's wrong, but disabled by default. I'd rather not add this and get it right. We can always add this option in 3.5.1 once we've worked out the real issue with the patch. Jeremy.
There's no real bug. I think it's just that we get the request twice from the kernel if we listen on 0.0.0.0:137 and 172.31.9.255:137 and reply twice. With "nmbd bind explicit broadcast = no" gd was getting just one reply.
No that's a bug :-). We need to detect the duplicate and discard it. Jeremy.
So, you mean we should add a cache with client-ip:reqiest-id combinations to see if we already got the request within the last minute or so?
Yes. I'm in the middle of coding this up. Realistically, a minute is *way* too long. Remember we're only trying to detect duplicate packets being sent by the kernel up both sockets once it's received a packet. Practically, a cache that is in existence only as long as listen_for_packets() is running (ie. only detects duplicates coming in on the same select() call) would almost certainly catch most duplicates. Jeremy.
Created attachment 5322 [details] git-am fix for 3.5.0. Metze, here is the simple cache that fixes the duplicate packet problem. IMHO we should only be putting this code (all of the bugfix, not just this segment) into 3.5.0, not into 3.4.x as it's too big a change for earlier versions. I've tested this and pushed to master. I'll turn on " nmbd bind explicit broadcast" by default again in master now we're ok with the duplicates. Jeremy.
Comment on attachment 5322 [details] git-am fix for 3.5.0. Ok, for 3.5.0
Ok, we'll keep it as custom patch in our 3.4 RPMs for now. We can rethink about a backport with the old default for 3.4.7...
Comment on attachment 5300 [details] Additional git-am patch needed for v3-5. This is ok for 3.5.0 in combination with https://bugzilla.samba.org/attachment.cgi?id=5322
Karolin, please pull the 3 attachments for v3-5 into v3-5-test
Pushed the three 3.5 patches. Re-assigning to Metze to address 3.4.
Created attachment 5324 [details] New patches for v3-4 (with "nmbd bind explicit broadcast = no" as default) We might pick that for 3.4.7....
Now I propose removal of "socket address" parameter. If we want to use nmbd on specified (multiple) interfaces only for security reason, although simply "nmbd bind explicit broadcast = yes" is enough but we must change "socket address" parameter because its default value (0.0.0.0) make nmbd accept packets from all interfaces. Here is the output of "netstat" with "nmbd bind explicit broadcast = yes" and "socket address = 0.0.0.0 (default)". ----- netstat -an | egrep '13[789]|445' tcp 0 0 192.168.135.247:139 0.0.0.0:* LISTEN tcp 0 0 192.168.135.247:445 0.0.0.0:* LISTEN tcp6 0 0 fe80::20c:29ff:fe44:139 :::* LISTEN tcp6 0 0 fe80::20c:29ff:fe44:445 :::* LISTEN udp 0 0 192.168.135.255:137 0.0.0.0:* udp 0 0 192.168.135.247:137 0.0.0.0:* udp 0 0 0.0.0.0:137 0.0.0.0:* <-- needless udp 0 0 192.168.135.255:138 0.0.0.0:* udp 0 0 192.168.135.247:138 0.0.0.0:* udp 0 0 0.0.0.0:138 0.0.0.0:* <-- needless ----- And we realize that we no longer have no proper value set to "socket address" parameter with "nmbd bind explicit broadcast = yes". I propose that (1) If "nmbd bind explicit broadcast = yes", then nmbd binds each broadcast addresses explicitly and does not bind to 0.0.0.0 (and binds to the interface address). (2) If "nmbd bind explicit broadcast = no", then nmbd binds to 0.0.0.0 only (and binds to the interface address).
Removing "socket address" might something for the future... It seems this won't make it into 3.4 anymore, so closing the bug report as it's already fixed in 3.5.