#127 Multiple listen-address implementation

closed-accepted
Fabian Keil
5
2011-07-17
2011-07-05
Petr Písař
No

Hello,

I found some operating systems does not allow to accept IPv4 client on IPv6 sockets or some system are configured in such manner (like GNU/Debian). Also I think it can be helpful to bind Privoxy to certain addresses only or to more ports.

There is similar item #71 on TODO list. My implementation forces to bind to all listed addresses. However it could be possible to adjust it.

I performed only few tests so there can be some bugs.

I will attach series of 5 patches against latest CVS tree.

Discussion

  • Fabian Keil
    Fabian Keil
    2011-07-06

    Thanks a lot for the patches.

    I'll have a closer look at them this weekend.

     
  • Fabian Keil
    Fabian Keil
    2011-07-06

    • assigned_to: nobody --> fabiankeil
    • status: open --> pending
     
  • Fabian Keil
    Fabian Keil
    2011-07-10

    I think there may be a logical error in bind_ports_helper().

    + if (JB_INVALID_SOCKET == sockets[i])
    + {
    + config->need_bind = 0;
    + }

    Shouldn't the condition be JB_INVALID_SOCKET != sockets[i] here?

     
  • Petr Písař
    Petr Písař
    2011-07-11

    You are right. Current code does not make sense. Correct fix depends on semantics you want to give to bind_ports_helper():

    (1) If you want to drop not-bound sockets and survive with successfully opened ones, your suggestion is correct.

    (2) If you want to leave possibility to re-bound on next loop cycle, one should pre-set config->need_bind to 0, keep the condition and change conditional assignment to setting 1.

    The only drawback is current bind_port_helper(), that's called from bind_ports_helper(), dies on any error, thus resolving given problem is just an academic question.

    I think choosing solution (2) opens way for implementing TODO item #71 more comprehensively. So I vote for it. Actually current implementation dies on reloading wrong configuration which I don't like. I think the daemon, once started, should keep running as long as possible.

     
  • Petr Písař
    Petr Písař
    2011-07-11

    • status: pending --> open
     
  • Fabian Keil
    Fabian Keil
    2011-07-12

    Trying once per address seems to be enough to me.

    I don't consider it a problem to treat bind failures as fatal issues. Until TODO list items #22 and #23 are implemented I'd actually consider it the best option.

    The problem I see with #2 is that if bind_port_helper() wouldn't die and if the user specified an address twice (or just used an incorrect one) Privoxy would waste a lot of time trying to bind to it again and again.

    While we could limit the attempts, I think that would be more magic than necessary and it also wouldn't be consistent with the behavior in case of other configuration issues.

    Looking at bind_port_helper() some more, I also think it shouldn't log the "Listening on ..." messages unless binding to the address actually worked. I'm aware that it already does that without your patch, though.

     
  • Petr Písař
    Petr Písař
    2011-07-13

    Ok, patch 0007-Drop-need_bind-flag-once-any-socket-binds.patch fixes the typo in condition in bind_ports_helper(), patch 0008-Log-listening-on-socket-after-binding.patch moves the `Listening on port...' log message after actual binding as you pointed in last post.

     
  • Fabian Keil
    Fabian Keil
    2011-07-14

    Thanks for the additional patches.

    I think with the current bind_port_helper() behavior, bind_ports_helper() can be further simplified to:

    {
    int i;

    for (i = 0; i < MAX_LISTENING_SOCKETS; i++)
    {
    if (config->hport[i])
    {
    sockets[i] = bind_port_helper(config->haddr[i], config->hport[i]);
    }
    else
    {
    sockets[i] = JB_INVALID_SOCKET;
    }
    }
    }

    (I already have a patch for this and intend to commit it separately)

     
  • Fabian Keil
    Fabian Keil
    2011-07-14

    Looks like I forgot the "config->need_bind = 0;" at either the beginning or the end of the function ...

     
  • Fabian Keil
    Fabian Keil
    2011-07-17

    Patches 0001 to 0007 have been squashed and committed.
    0008 has been committed separately.

     
  • Fabian Keil
    Fabian Keil
    2011-07-17

    • status: open --> closed-accepted
     
  • Fabian Keil
    Fabian Keil
    2011-07-19

    I just removed #71 from the TODO list, and now realize that I worded it poorly which allowed an unintended interpretation.

    By "without having to bind to all" I was referring to the workaround of using "listen-address :8118" to let Privoxy bind to all the addresses available on the system, not that Privoxy should allow multiple listen-address directives and ignore bind failures.

    As far as I'm concerned #71 has been implemented completely.