Date: Tue, 14 Aug 2018 15:44:52 -0700 From: =?UTF-8?Q?Ermal_Lu=C3=A7i?= <eri@freebsd.org> To: Kajetan Staszkiewicz <vegeta@tuxpowered.net> Cc: Kristof Provost <kp@freebsd.org>, "freebsd-pf@freebsd.org" <freebsd-pf@freebsd.org> Subject: Re: pf tables locking Message-ID: <CAPBZQG1S=M4DFZytRzYWD0HeT3yjm6HLCAA6HEb-Td0jg0svHQ@mail.gmail.com> In-Reply-To: <13826523.m2ultlLLsi@energia> References: <8680316.SccKl5VnxN@energia> <1546233.jncNNXsBuh@energia> <69D19AE4-2F17-4DBC-AF62-A2489049FC9C@FreeBSD.org> <13826523.m2ultlLLsi@energia>
next in thread | previous in thread | raw e-mail | index | archive | help
(sorry for the top post) If you really want to spend time on it, the best option is to pull out the pool concept used by the rules/nat... and manage it outside of the rules/states but in its own module referenced by the former ones. This would allow extensibility and propper reasoning about it. On Tue, Aug 14, 2018 at 9:35 AM, Kajetan Staszkiewicz <vegeta@tuxpowered.ne= t > wrote: > On Tuesday, 14 August 2018 16:15:48 CEST Kristof Provost wrote: > > On 14 Aug 2018, at 0:32, Kajetan Staszkiewicz wrote: > > > On Monday, 13 August 2018 17:59:15 CEST Kristof Provost wrote: > > > How about this? > > > > > > https://github.com/innogames/freebsd/commit/ > > > d44a0d9487285fac8ed1d7372cc99cca83f616e6 > > > > That looks good to me. > > There=E2=80=99s a few minor issues, things like inconsistent indentatio= n and > > overly long lines, but that=E2=80=99s about the only criticism I have. > > I fixed some issues with unallocated counters and submitted bug 230619. > > > I see. I=E2=80=99m not quite sure yet if that=E2=80=99s a feature we wa= nt to import > > or not, > > but at least your =E2=80=98support=E2=80=99 patches should probably go = in. The above > > one certainly. > > There are some more things which require changes before I can do least- > connections balancing. > > If you have a moment, please have a look at https://github.com/innogames/ > freebsd/commits/iglb/11.2/GetOnWithIt_2 , maybe some of those things can > get > imported anyway, like full support for counters of states. > > > >> Yeah, that bug is still on my todo list somewhere, but things are > > >> extremely > > >> hectic at the moment, and I can=E2=80=99t make any promises about wh= en > > >> I=E2=80=99ll have > > >> time for it. > > > > > > I thought that was rather on my todo :) > > > > I=E2=80=99m not going to stop you. I love it when other people do the w= ork ;) > > Since I have you here, let me explain the issues I see with pf_map_addr()= . > For > round-robin target a list of interface,table pairs can be specified. This > list > is iterated and within each table addresses are iterated too. There is no > locking around it "because performance is assumed more important than > round- > robin precision" according to comment in code. > > Yet I believe there are way more serious issues possible with the current > approach. Interface is in fact picked up outside of pf_map_addr(). Anothe= r > thread could have already moved the rpool->counter to another table for > which > the interface is not valid anymore. > > I came up with this: https://github.com/innogames/freebsd/commit/ > 61ffb96a4dc948a0b06204ff39210c0578f77f08 although without locking this is > still not really a solution. It only moves interface selection to inside > of > pf_map_addr() > > Another one is https://github.com/innogames/freebsd/commit/ > 8fe6cd2d820052d2166afbaa311f34318a41db48 which stores table used for > loadbalancing in state and src_node. Then the table can be used for state > counting. > > The 2 patches above are also included in the first link I gave above. > > -- > | pozdrawiam / greetings | powered by Debian, FreeBSD and CentOS | > | Kajetan Staszkiewicz | jabber,email: vegeta()tuxpowered net | > | Vegeta | www: http://vegeta.tuxpowered.net | > `------------------------^---------------------------------------' > > -- > Ermal >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPBZQG1S=M4DFZytRzYWD0HeT3yjm6HLCAA6HEb-Td0jg0svHQ>