* [PATCH 0/1] Reusing modifier socket for bulk ct loads @ 2022-06-02 16:34 Mikhail Sennikovsky 2022-06-02 16:34 ` [PATCH 1/1] conntrack: use same modifier socket for bulk ops Mikhail Sennikovsky 0 siblings, 1 reply; 9+ messages in thread From: Mikhail Sennikovsky @ 2022-06-02 16:34 UTC (permalink / raw) To: netfilter-devel, pablo, mikhail.sennikovsky; +Cc: Mikhail Sennikovsky Hi Pablo & all, As discussed, here is the follow-up patch to use the same mnl modifier socket for all operation in bulk ct loads to increase performance of such loads. Any feedback/suggestions are welcome. Regards, Mikhail Mikhail Sennikovsky (1): conntrack: use same modifier socket for bulk ops src/conntrack.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] conntrack: use same modifier socket for bulk ops 2022-06-02 16:34 [PATCH 0/1] Reusing modifier socket for bulk ct loads Mikhail Sennikovsky @ 2022-06-02 16:34 ` Mikhail Sennikovsky 2022-06-08 6:18 ` Pablo Neira Ayuso 0 siblings, 1 reply; 9+ messages in thread From: Mikhail Sennikovsky @ 2022-06-02 16:34 UTC (permalink / raw) To: netfilter-devel, pablo, mikhail.sennikovsky; +Cc: Mikhail Sennikovsky For bulk ct entry loads (with -R option) reusing the same mnl modifier socket for all entries results in reduction of entries creation time, which becomes especially signifficant when loading tens of thouthand of entries. Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com> --- src/conntrack.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/conntrack.c b/src/conntrack.c index 27e2bea..be8690b 100644 --- a/src/conntrack.c +++ b/src/conntrack.c @@ -71,6 +71,7 @@ struct nfct_mnl_socket { struct mnl_socket *mnl; uint32_t portid; + uint32_t events; }; static struct nfct_mnl_socket _sock; @@ -2441,6 +2442,7 @@ static int nfct_mnl_socket_open(struct nfct_mnl_socket *socket, return -1; } socket->portid = mnl_socket_get_portid(socket->mnl); + socket->events = events; return 0; } @@ -2470,6 +2472,25 @@ static void nfct_mnl_socket_close(const struct nfct_mnl_socket *sock) mnl_socket_close(sock->mnl); } +static int nfct_mnl_socket_check_open(struct nfct_mnl_socket *socket, + unsigned int events) +{ + if (socket->mnl != NULL) { + assert(events == socket->events); + return 0; + } + + return nfct_mnl_socket_open(socket, events); +} + +static void nfct_mnl_socket_check_close(struct nfct_mnl_socket *sock) +{ + if (sock->mnl) { + nfct_mnl_socket_close(sock); + memset(sock, 0, sizeof(*sock)); + } +} + static int __nfct_mnl_dump(struct nfct_mnl_socket *sock, const struct nlmsghdr *nlh, mnl_cb_t cb, void *data) { @@ -3383,19 +3404,17 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, break; case CT_UPDATE: - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) exit_error(OTHER_PROBLEM, "Can't open handler"); nfct_filter_init(cmd); res = nfct_mnl_dump(sock, NFNL_SUBSYS_CTNETLINK, IPCTNL_MSG_CT_GET, mnl_nfct_update_cb, cmd, NULL); - - nfct_mnl_socket_close(modifier_sock); break; case CT_DELETE: - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) exit_error(OTHER_PROBLEM, "Can't open handler"); nfct_filter_init(cmd); @@ -3418,8 +3437,6 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, cmd, filter_dump); nfct_filter_dump_destroy(filter_dump); - - nfct_mnl_socket_close(modifier_sock); break; case EXP_DELETE: @@ -3857,6 +3874,7 @@ static const char *ct_unsupp_cmd_file(const struct ct_cmd *cmd) int main(int argc, char *argv[]) { struct nfct_mnl_socket *sock = &_sock; + struct nfct_mnl_socket *modifier_sock = &_modifier_sock; struct ct_cmd *cmd, *next; LIST_HEAD(cmd_list); int res = 0; @@ -3900,6 +3918,7 @@ int main(int argc, char *argv[]) free(cmd); } nfct_mnl_socket_close(sock); + nfct_mnl_socket_check_close(modifier_sock); return res < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] conntrack: use same modifier socket for bulk ops 2022-06-02 16:34 ` [PATCH 1/1] conntrack: use same modifier socket for bulk ops Mikhail Sennikovsky @ 2022-06-08 6:18 ` Pablo Neira Ayuso 2022-06-08 11:02 ` Mikhail Sennikovsky 0 siblings, 1 reply; 9+ messages in thread From: Pablo Neira Ayuso @ 2022-06-08 6:18 UTC (permalink / raw) To: Mikhail Sennikovsky; +Cc: netfilter-devel, mikhail.sennikovsky Hi Mikhail, On Thu, Jun 02, 2022 at 06:34:29PM +0200, Mikhail Sennikovsky wrote: > For bulk ct entry loads (with -R option) reusing the same mnl > modifier socket for all entries results in reduction of entries > creation time, which becomes especially signifficant when loading > tens of thouthand of entries. > > Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com> > --- > src/conntrack.c | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > diff --git a/src/conntrack.c b/src/conntrack.c > index 27e2bea..be8690b 100644 > --- a/src/conntrack.c > +++ b/src/conntrack.c > @@ -71,6 +71,7 @@ > struct nfct_mnl_socket { > struct mnl_socket *mnl; > uint32_t portid; > + uint32_t events; > }; > > static struct nfct_mnl_socket _sock; > @@ -2441,6 +2442,7 @@ static int nfct_mnl_socket_open(struct nfct_mnl_socket *socket, > return -1; > } > socket->portid = mnl_socket_get_portid(socket->mnl); > + socket->events = events; > > return 0; > } > @@ -2470,6 +2472,25 @@ static void nfct_mnl_socket_close(const struct nfct_mnl_socket *sock) > mnl_socket_close(sock->mnl); > } > > +static int nfct_mnl_socket_check_open(struct nfct_mnl_socket *socket, > + unsigned int events) > +{ > + if (socket->mnl != NULL) { > + assert(events == socket->events); > + return 0; > + } > + > + return nfct_mnl_socket_open(socket, events); > +} > + > +static void nfct_mnl_socket_check_close(struct nfct_mnl_socket *sock) > +{ > + if (sock->mnl) { > + nfct_mnl_socket_close(sock); > + memset(sock, 0, sizeof(*sock)); > + } > +} > + > static int __nfct_mnl_dump(struct nfct_mnl_socket *sock, > const struct nlmsghdr *nlh, mnl_cb_t cb, void *data) > { > @@ -3383,19 +3404,17 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, > break; > > case CT_UPDATE: > - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) > + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) > exit_error(OTHER_PROBLEM, "Can't open handler"); > > nfct_filter_init(cmd); > res = nfct_mnl_dump(sock, NFNL_SUBSYS_CTNETLINK, > IPCTNL_MSG_CT_GET, mnl_nfct_update_cb, > cmd, NULL); > - > - nfct_mnl_socket_close(modifier_sock); > break; > > case CT_DELETE: > - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) > + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) No events needed anymore? nfct_mnl_socket_check_open() is now only used by CT_UPDATE and CT_DELETE, right? > exit_error(OTHER_PROBLEM, "Can't open handler"); > > nfct_filter_init(cmd); > @@ -3418,8 +3437,6 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, > cmd, filter_dump); > > nfct_filter_dump_destroy(filter_dump); > - > - nfct_mnl_socket_close(modifier_sock); > break; > > case EXP_DELETE: > @@ -3857,6 +3874,7 @@ static const char *ct_unsupp_cmd_file(const struct ct_cmd *cmd) > int main(int argc, char *argv[]) > { > struct nfct_mnl_socket *sock = &_sock; > + struct nfct_mnl_socket *modifier_sock = &_modifier_sock; > struct ct_cmd *cmd, *next; > LIST_HEAD(cmd_list); > int res = 0; > @@ -3900,6 +3918,7 @@ int main(int argc, char *argv[]) > free(cmd); > } > nfct_mnl_socket_close(sock); > + nfct_mnl_socket_check_close(modifier_sock); > > return res < 0 ? EXIT_FAILURE : EXIT_SUCCESS; > } > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] conntrack: use same modifier socket for bulk ops 2022-06-08 6:18 ` Pablo Neira Ayuso @ 2022-06-08 11:02 ` Mikhail Sennikovsky 2022-06-08 11:08 ` Pablo Neira Ayuso 0 siblings, 1 reply; 9+ messages in thread From: Mikhail Sennikovsky @ 2022-06-08 11:02 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel, mikhail.sennikovsky Hi Pablo, IIRC we never had a code that was using nfct_mnl_socket_check_open for CT_EVENT and friends, because it is not needed there, as they can not be used in the bulk operations. Regards, Mikhail On Wed, 8 Jun 2022 at 08:18, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Hi Mikhail, > > On Thu, Jun 02, 2022 at 06:34:29PM +0200, Mikhail Sennikovsky wrote: > > For bulk ct entry loads (with -R option) reusing the same mnl > > modifier socket for all entries results in reduction of entries > > creation time, which becomes especially signifficant when loading > > tens of thouthand of entries. > > > > Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com> > > --- > > src/conntrack.c | 31 +++++++++++++++++++++++++------ > > 1 file changed, 25 insertions(+), 6 deletions(-) > > > > diff --git a/src/conntrack.c b/src/conntrack.c > > index 27e2bea..be8690b 100644 > > --- a/src/conntrack.c > > +++ b/src/conntrack.c > > @@ -71,6 +71,7 @@ > > struct nfct_mnl_socket { > > struct mnl_socket *mnl; > > uint32_t portid; > > + uint32_t events; > > }; > > > > static struct nfct_mnl_socket _sock; > > @@ -2441,6 +2442,7 @@ static int nfct_mnl_socket_open(struct nfct_mnl_socket *socket, > > return -1; > > } > > socket->portid = mnl_socket_get_portid(socket->mnl); > > + socket->events = events; > > > > return 0; > > } > > @@ -2470,6 +2472,25 @@ static void nfct_mnl_socket_close(const struct nfct_mnl_socket *sock) > > mnl_socket_close(sock->mnl); > > } > > > > +static int nfct_mnl_socket_check_open(struct nfct_mnl_socket *socket, > > + unsigned int events) > > +{ > > + if (socket->mnl != NULL) { > > + assert(events == socket->events); > > + return 0; > > + } > > + > > + return nfct_mnl_socket_open(socket, events); > > +} > > + > > +static void nfct_mnl_socket_check_close(struct nfct_mnl_socket *sock) > > +{ > > + if (sock->mnl) { > > + nfct_mnl_socket_close(sock); > > + memset(sock, 0, sizeof(*sock)); > > + } > > +} > > + > > static int __nfct_mnl_dump(struct nfct_mnl_socket *sock, > > const struct nlmsghdr *nlh, mnl_cb_t cb, void *data) > > { > > @@ -3383,19 +3404,17 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, > > break; > > > > case CT_UPDATE: > > - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) > > + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) > > exit_error(OTHER_PROBLEM, "Can't open handler"); > > > > nfct_filter_init(cmd); > > res = nfct_mnl_dump(sock, NFNL_SUBSYS_CTNETLINK, > > IPCTNL_MSG_CT_GET, mnl_nfct_update_cb, > > cmd, NULL); > > - > > - nfct_mnl_socket_close(modifier_sock); > > break; > > > > case CT_DELETE: > > - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) > > + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) > > No events needed anymore? > > nfct_mnl_socket_check_open() is now only used by CT_UPDATE and CT_DELETE, > right? > > > exit_error(OTHER_PROBLEM, "Can't open handler"); > > > > nfct_filter_init(cmd); > > @@ -3418,8 +3437,6 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, > > cmd, filter_dump); > > > > nfct_filter_dump_destroy(filter_dump); > > - > > - nfct_mnl_socket_close(modifier_sock); > > break; > > > > case EXP_DELETE: > > @@ -3857,6 +3874,7 @@ static const char *ct_unsupp_cmd_file(const struct ct_cmd *cmd) > > int main(int argc, char *argv[]) > > { > > struct nfct_mnl_socket *sock = &_sock; > > + struct nfct_mnl_socket *modifier_sock = &_modifier_sock; > > struct ct_cmd *cmd, *next; > > LIST_HEAD(cmd_list); > > int res = 0; > > @@ -3900,6 +3918,7 @@ int main(int argc, char *argv[]) > > free(cmd); > > } > > nfct_mnl_socket_close(sock); > > + nfct_mnl_socket_check_close(modifier_sock); > > > > return res < 0 ? EXIT_FAILURE : EXIT_SUCCESS; > > } > > -- > > 2.25.1 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] conntrack: use same modifier socket for bulk ops 2022-06-08 11:02 ` Mikhail Sennikovsky @ 2022-06-08 11:08 ` Pablo Neira Ayuso 2022-06-08 11:19 ` Pablo Neira Ayuso 0 siblings, 1 reply; 9+ messages in thread From: Pablo Neira Ayuso @ 2022-06-08 11:08 UTC (permalink / raw) To: Mikhail Sennikovsky; +Cc: netfilter-devel, mikhail.sennikovsky On Wed, Jun 08, 2022 at 01:02:30PM +0200, Mikhail Sennikovsky wrote: > Hi Pablo, > > IIRC we never had a code that was using nfct_mnl_socket_check_open for > CT_EVENT and friends, because it is not needed there, as they can not > be used in the bulk operations. Not sure I follow. My question is: is this new events field really required? > Regards, > Mikhail > > On Wed, 8 Jun 2022 at 08:18, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > Hi Mikhail, > > > > On Thu, Jun 02, 2022 at 06:34:29PM +0200, Mikhail Sennikovsky wrote: > > > For bulk ct entry loads (with -R option) reusing the same mnl > > > modifier socket for all entries results in reduction of entries > > > creation time, which becomes especially signifficant when loading > > > tens of thouthand of entries. > > > > > > Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com> > > > --- > > > src/conntrack.c | 31 +++++++++++++++++++++++++------ > > > 1 file changed, 25 insertions(+), 6 deletions(-) > > > > > > diff --git a/src/conntrack.c b/src/conntrack.c > > > index 27e2bea..be8690b 100644 > > > --- a/src/conntrack.c > > > +++ b/src/conntrack.c > > > @@ -71,6 +71,7 @@ > > > struct nfct_mnl_socket { > > > struct mnl_socket *mnl; > > > uint32_t portid; > > > + uint32_t events; > > > }; > > > > > > static struct nfct_mnl_socket _sock; > > > @@ -2441,6 +2442,7 @@ static int nfct_mnl_socket_open(struct nfct_mnl_socket *socket, > > > return -1; > > > } > > > socket->portid = mnl_socket_get_portid(socket->mnl); > > > + socket->events = events; > > > > > > return 0; > > > } > > > @@ -2470,6 +2472,25 @@ static void nfct_mnl_socket_close(const struct nfct_mnl_socket *sock) > > > mnl_socket_close(sock->mnl); > > > } > > > > > > +static int nfct_mnl_socket_check_open(struct nfct_mnl_socket *socket, > > > + unsigned int events) > > > +{ > > > + if (socket->mnl != NULL) { > > > + assert(events == socket->events); > > > + return 0; > > > + } > > > + > > > + return nfct_mnl_socket_open(socket, events); > > > +} > > > + > > > +static void nfct_mnl_socket_check_close(struct nfct_mnl_socket *sock) > > > +{ > > > + if (sock->mnl) { > > > + nfct_mnl_socket_close(sock); > > > + memset(sock, 0, sizeof(*sock)); > > > + } > > > +} > > > + > > > static int __nfct_mnl_dump(struct nfct_mnl_socket *sock, > > > const struct nlmsghdr *nlh, mnl_cb_t cb, void *data) > > > { > > > @@ -3383,19 +3404,17 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, > > > break; > > > > > > case CT_UPDATE: > > > - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) > > > + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) > > > exit_error(OTHER_PROBLEM, "Can't open handler"); > > > > > > nfct_filter_init(cmd); > > > res = nfct_mnl_dump(sock, NFNL_SUBSYS_CTNETLINK, > > > IPCTNL_MSG_CT_GET, mnl_nfct_update_cb, > > > cmd, NULL); > > > - > > > - nfct_mnl_socket_close(modifier_sock); > > > break; > > > > > > case CT_DELETE: > > > - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) > > > + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) > > > > No events needed anymore? > > > > nfct_mnl_socket_check_open() is now only used by CT_UPDATE and CT_DELETE, > > right? > > > > > exit_error(OTHER_PROBLEM, "Can't open handler"); > > > > > > nfct_filter_init(cmd); > > > @@ -3418,8 +3437,6 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, > > > cmd, filter_dump); > > > > > > nfct_filter_dump_destroy(filter_dump); > > > - > > > - nfct_mnl_socket_close(modifier_sock); > > > break; > > > > > > case EXP_DELETE: > > > @@ -3857,6 +3874,7 @@ static const char *ct_unsupp_cmd_file(const struct ct_cmd *cmd) > > > int main(int argc, char *argv[]) > > > { > > > struct nfct_mnl_socket *sock = &_sock; > > > + struct nfct_mnl_socket *modifier_sock = &_modifier_sock; > > > struct ct_cmd *cmd, *next; > > > LIST_HEAD(cmd_list); > > > int res = 0; > > > @@ -3900,6 +3918,7 @@ int main(int argc, char *argv[]) > > > free(cmd); > > > } > > > nfct_mnl_socket_close(sock); > > > + nfct_mnl_socket_check_close(modifier_sock); > > > > > > return res < 0 ? EXIT_FAILURE : EXIT_SUCCESS; > > > } > > > -- > > > 2.25.1 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] conntrack: use same modifier socket for bulk ops 2022-06-08 11:08 ` Pablo Neira Ayuso @ 2022-06-08 11:19 ` Pablo Neira Ayuso 2022-06-08 14:16 ` Mikhail Sennikovsky 0 siblings, 1 reply; 9+ messages in thread From: Pablo Neira Ayuso @ 2022-06-08 11:19 UTC (permalink / raw) To: Mikhail Sennikovsky; +Cc: netfilter-devel, mikhail.sennikovsky On Wed, Jun 08, 2022 at 01:08:56PM +0200, Pablo Neira Ayuso wrote: > On Wed, Jun 08, 2022 at 01:02:30PM +0200, Mikhail Sennikovsky wrote: > > Hi Pablo, > > > > IIRC we never had a code that was using nfct_mnl_socket_check_open for > > CT_EVENT and friends, because it is not needed there, as they can not > > be used in the bulk operations. > > Not sure I follow. My question is: is this new events field really required? sock->mnl is always set on at the beginning of the batch processing. That remains unaltered. Then we have the modifier socket, which is used for update and delete. This is conditionally opened. The new _check function just checks if this socket is open, if so re-use it. _check is now never used for events. Correct? > > Regards, > > Mikhail > > > > On Wed, 8 Jun 2022 at 08:18, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > > > Hi Mikhail, > > > > > > On Thu, Jun 02, 2022 at 06:34:29PM +0200, Mikhail Sennikovsky wrote: > > > > For bulk ct entry loads (with -R option) reusing the same mnl > > > > modifier socket for all entries results in reduction of entries > > > > creation time, which becomes especially signifficant when loading > > > > tens of thouthand of entries. > > > > > > > > Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com> > > > > --- > > > > src/conntrack.c | 31 +++++++++++++++++++++++++------ > > > > 1 file changed, 25 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/src/conntrack.c b/src/conntrack.c > > > > index 27e2bea..be8690b 100644 > > > > --- a/src/conntrack.c > > > > +++ b/src/conntrack.c > > > > @@ -71,6 +71,7 @@ > > > > struct nfct_mnl_socket { > > > > struct mnl_socket *mnl; > > > > uint32_t portid; > > > > + uint32_t events; > > > > }; > > > > > > > > static struct nfct_mnl_socket _sock; > > > > @@ -2441,6 +2442,7 @@ static int nfct_mnl_socket_open(struct nfct_mnl_socket *socket, > > > > return -1; > > > > } > > > > socket->portid = mnl_socket_get_portid(socket->mnl); > > > > + socket->events = events; > > > > > > > > return 0; > > > > } > > > > @@ -2470,6 +2472,25 @@ static void nfct_mnl_socket_close(const struct nfct_mnl_socket *sock) > > > > mnl_socket_close(sock->mnl); > > > > } > > > > > > > > +static int nfct_mnl_socket_check_open(struct nfct_mnl_socket *socket, > > > > + unsigned int events) > > > > +{ > > > > + if (socket->mnl != NULL) { > > > > + assert(events == socket->events); > > > > + return 0; > > > > + } > > > > + > > > > + return nfct_mnl_socket_open(socket, events); > > > > +} > > > > + > > > > +static void nfct_mnl_socket_check_close(struct nfct_mnl_socket *sock) > > > > +{ > > > > + if (sock->mnl) { > > > > + nfct_mnl_socket_close(sock); > > > > + memset(sock, 0, sizeof(*sock)); > > > > + } > > > > +} > > > > + > > > > static int __nfct_mnl_dump(struct nfct_mnl_socket *sock, > > > > const struct nlmsghdr *nlh, mnl_cb_t cb, void *data) > > > > { > > > > @@ -3383,19 +3404,17 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, > > > > break; > > > > > > > > case CT_UPDATE: > > > > - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) > > > > + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) > > > > exit_error(OTHER_PROBLEM, "Can't open handler"); > > > > > > > > nfct_filter_init(cmd); > > > > res = nfct_mnl_dump(sock, NFNL_SUBSYS_CTNETLINK, > > > > IPCTNL_MSG_CT_GET, mnl_nfct_update_cb, > > > > cmd, NULL); > > > > - > > > > - nfct_mnl_socket_close(modifier_sock); > > > > break; > > > > > > > > case CT_DELETE: > > > > - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) > > > > + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) > > > > > > No events needed anymore? > > > > > > nfct_mnl_socket_check_open() is now only used by CT_UPDATE and CT_DELETE, > > > right? > > > > > > > exit_error(OTHER_PROBLEM, "Can't open handler"); > > > > > > > > nfct_filter_init(cmd); > > > > @@ -3418,8 +3437,6 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, > > > > cmd, filter_dump); > > > > > > > > nfct_filter_dump_destroy(filter_dump); > > > > - > > > > - nfct_mnl_socket_close(modifier_sock); > > > > break; > > > > > > > > case EXP_DELETE: > > > > @@ -3857,6 +3874,7 @@ static const char *ct_unsupp_cmd_file(const struct ct_cmd *cmd) > > > > int main(int argc, char *argv[]) > > > > { > > > > struct nfct_mnl_socket *sock = &_sock; > > > > + struct nfct_mnl_socket *modifier_sock = &_modifier_sock; > > > > struct ct_cmd *cmd, *next; > > > > LIST_HEAD(cmd_list); > > > > int res = 0; > > > > @@ -3900,6 +3918,7 @@ int main(int argc, char *argv[]) > > > > free(cmd); > > > > } > > > > nfct_mnl_socket_close(sock); > > > > + nfct_mnl_socket_check_close(modifier_sock); > > > > > > > > return res < 0 ? EXIT_FAILURE : EXIT_SUCCESS; > > > > } > > > > -- > > > > 2.25.1 > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] conntrack: use same modifier socket for bulk ops 2022-06-08 11:19 ` Pablo Neira Ayuso @ 2022-06-08 14:16 ` Mikhail Sennikovsky 2022-06-09 10:20 ` Pablo Neira Ayuso 0 siblings, 1 reply; 9+ messages in thread From: Mikhail Sennikovsky @ 2022-06-08 14:16 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel, mikhail.sennikovsky Hi Pablo, Then I misunderstood you, my bad. Yes, _check is never used for events, and the socket->events is not used anywhere except the assert(events == socket->events); assertion check which I found useful as a sanity check for potential future uses of the nfct_mnl_socket_check_open. If you find it unneeded however, I'm fine with removing it. Mikhail On Wed, 8 Jun 2022 at 13:19, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > On Wed, Jun 08, 2022 at 01:08:56PM +0200, Pablo Neira Ayuso wrote: > > On Wed, Jun 08, 2022 at 01:02:30PM +0200, Mikhail Sennikovsky wrote: > > > Hi Pablo, > > > > > > IIRC we never had a code that was using nfct_mnl_socket_check_open for > > > CT_EVENT and friends, because it is not needed there, as they can not > > > be used in the bulk operations. > > > > Not sure I follow. My question is: is this new events field really required? > > sock->mnl is always set on at the beginning of the batch processing. > That remains unaltered. > > Then we have the modifier socket, which is used for update and delete. > This is conditionally opened. The new _check function just checks if > this socket is open, if so re-use it. > > _check is now never used for events. > > Correct? > > > > Regards, > > > Mikhail > > > > > > On Wed, 8 Jun 2022 at 08:18, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > > > > > Hi Mikhail, > > > > > > > > On Thu, Jun 02, 2022 at 06:34:29PM +0200, Mikhail Sennikovsky wrote: > > > > > For bulk ct entry loads (with -R option) reusing the same mnl > > > > > modifier socket for all entries results in reduction of entries > > > > > creation time, which becomes especially signifficant when loading > > > > > tens of thouthand of entries. > > > > > > > > > > Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@ionos.com> > > > > > --- > > > > > src/conntrack.c | 31 +++++++++++++++++++++++++------ > > > > > 1 file changed, 25 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/src/conntrack.c b/src/conntrack.c > > > > > index 27e2bea..be8690b 100644 > > > > > --- a/src/conntrack.c > > > > > +++ b/src/conntrack.c > > > > > @@ -71,6 +71,7 @@ > > > > > struct nfct_mnl_socket { > > > > > struct mnl_socket *mnl; > > > > > uint32_t portid; > > > > > + uint32_t events; > > > > > }; > > > > > > > > > > static struct nfct_mnl_socket _sock; > > > > > @@ -2441,6 +2442,7 @@ static int nfct_mnl_socket_open(struct nfct_mnl_socket *socket, > > > > > return -1; > > > > > } > > > > > socket->portid = mnl_socket_get_portid(socket->mnl); > > > > > + socket->events = events; > > > > > > > > > > return 0; > > > > > } > > > > > @@ -2470,6 +2472,25 @@ static void nfct_mnl_socket_close(const struct nfct_mnl_socket *sock) > > > > > mnl_socket_close(sock->mnl); > > > > > } > > > > > > > > > > +static int nfct_mnl_socket_check_open(struct nfct_mnl_socket *socket, > > > > > + unsigned int events) > > > > > +{ > > > > > + if (socket->mnl != NULL) { > > > > > + assert(events == socket->events); > > > > > + return 0; > > > > > + } > > > > > + > > > > > + return nfct_mnl_socket_open(socket, events); > > > > > +} > > > > > + > > > > > +static void nfct_mnl_socket_check_close(struct nfct_mnl_socket *sock) > > > > > +{ > > > > > + if (sock->mnl) { > > > > > + nfct_mnl_socket_close(sock); > > > > > + memset(sock, 0, sizeof(*sock)); > > > > > + } > > > > > +} > > > > > + > > > > > static int __nfct_mnl_dump(struct nfct_mnl_socket *sock, > > > > > const struct nlmsghdr *nlh, mnl_cb_t cb, void *data) > > > > > { > > > > > @@ -3383,19 +3404,17 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, > > > > > break; > > > > > > > > > > case CT_UPDATE: > > > > > - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) > > > > > + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) > > > > > exit_error(OTHER_PROBLEM, "Can't open handler"); > > > > > > > > > > nfct_filter_init(cmd); > > > > > res = nfct_mnl_dump(sock, NFNL_SUBSYS_CTNETLINK, > > > > > > > > > > > IPCTNL_MSG_CT_GET, mnl_nfct_update_cb, > > > > > cmd, NULL); > > > > > - > > > > > - nfct_mnl_socket_close(modifier_sock); > > > > > break; > > > > > > > > > > case CT_DELETE: > > > > > - if (nfct_mnl_socket_open(modifier_sock, 0) < 0) > > > > > + if (nfct_mnl_socket_check_open(modifier_sock, 0) < 0) > > > > > > > > No events needed anymore? > > > > > > > > nfct_mnl_socket_check_open() is now only used by CT_UPDATE and CT_DELETE, > > > > right? > > > > > > > > > exit_error(OTHER_PROBLEM, "Can't open handler"); > > > > > > > > > > nfct_filter_init(cmd); > > > > > @@ -3418,8 +3437,6 @@ static int do_command_ct(const char *progname, struct ct_cmd *cmd, > > > > > cmd, filter_dump); > > > > > > > > > > nfct_filter_dump_destroy(filter_dump); > > > > > - > > > > > - nfct_mnl_socket_close(modifier_sock); > > > > > break; > > > > > > > > > > case EXP_DELETE: > > > > > @@ -3857,6 +3874,7 @@ static const char *ct_unsupp_cmd_file(const struct ct_cmd *cmd) > > > > > int main(int argc, char *argv[]) > > > > > { > > > > > struct nfct_mnl_socket *sock = &_sock; > > > > > + struct nfct_mnl_socket *modifier_sock = &_modifier_sock; > > > > > struct ct_cmd *cmd, *next; > > > > > LIST_HEAD(cmd_list); > > > > > int res = 0; > > > > > @@ -3900,6 +3918,7 @@ int main(int argc, char *argv[]) > > > > > free(cmd); > > > > > } > > > > > nfct_mnl_socket_close(sock); > > > > > + nfct_mnl_socket_check_close(modifier_sock); > > > > > > > > > > return res < 0 ? EXIT_FAILURE : EXIT_SUCCESS; > > > > > } > > > > > -- > > > > > 2.25.1 > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] conntrack: use same modifier socket for bulk ops 2022-06-08 14:16 ` Mikhail Sennikovsky @ 2022-06-09 10:20 ` Pablo Neira Ayuso 2022-06-09 10:42 ` Mikhail Sennikovsky 0 siblings, 1 reply; 9+ messages in thread From: Pablo Neira Ayuso @ 2022-06-09 10:20 UTC (permalink / raw) To: Mikhail Sennikovsky; +Cc: netfilter-devel, mikhail.sennikovsky Hi, On Wed, Jun 08, 2022 at 04:16:34PM +0200, Mikhail Sennikovsky wrote: > Hi Pablo, > > Then I misunderstood you, my bad. > Yes, _check is never used for events, and the socket->events is not > used anywhere except the assert(events == socket->events); assertion > check which I found useful as a sanity check for potential future uses > of the nfct_mnl_socket_check_open. > If you find it unneeded however, I'm fine with removing it. If not used now for this usecase, I'd prefer if you remove it. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] conntrack: use same modifier socket for bulk ops 2022-06-09 10:20 ` Pablo Neira Ayuso @ 2022-06-09 10:42 ` Mikhail Sennikovsky 0 siblings, 0 replies; 9+ messages in thread From: Mikhail Sennikovsky @ 2022-06-09 10:42 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel, mikhail.sennikovsky Sure, let me remove it then and submit an updated patch here. Thanks, Mikhail On Thu, 9 Jun 2022 at 12:20, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Hi, > > On Wed, Jun 08, 2022 at 04:16:34PM +0200, Mikhail Sennikovsky wrote: > > Hi Pablo, > > > > Then I misunderstood you, my bad. > > Yes, _check is never used for events, and the socket->events is not > > used anywhere except the assert(events == socket->events); assertion > > check which I found useful as a sanity check for potential future uses > > of the nfct_mnl_socket_check_open. > > If you find it unneeded however, I'm fine with removing it. > > If not used now for this usecase, I'd prefer if you remove it. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-06-09 10:43 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-06-02 16:34 [PATCH 0/1] Reusing modifier socket for bulk ct loads Mikhail Sennikovsky 2022-06-02 16:34 ` [PATCH 1/1] conntrack: use same modifier socket for bulk ops Mikhail Sennikovsky 2022-06-08 6:18 ` Pablo Neira Ayuso 2022-06-08 11:02 ` Mikhail Sennikovsky 2022-06-08 11:08 ` Pablo Neira Ayuso 2022-06-08 11:19 ` Pablo Neira Ayuso 2022-06-08 14:16 ` Mikhail Sennikovsky 2022-06-09 10:20 ` Pablo Neira Ayuso 2022-06-09 10:42 ` Mikhail Sennikovsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).