[{"id":1761569,"web_url":"http://patchwork.ozlabs.org/comment/1761569/","msgid":"<20170901101733.GA5539@salvia>","list_archive_url":null,"date":"2017-09-01T10:17:33","subject":"Re: [PATCH nft 3/3] src: add nft_ctx_netlink_init()","submitter":{"id":1315,"url":"http://patchwork.ozlabs.org/api/people/1315/","name":"Pablo Neira Ayuso","email":"pablo@netfilter.org"},"content":"On Fri, Sep 01, 2017 at 12:14:07PM +0200, Pablo Neira Ayuso wrote:\n> Add these two new functions to set up netlink sockets in the global\n> context structure.\n\nWe can alternatively call this nft_ctx_netlink_auto() if prefer.\n\nI'm just trying to skip the type/flag field for nft_ctx_alloc().\n\nDoes this look acceptable to you to have this extra API to request\nlibnftables to deal with IO details too?\n--\nTo unsubscribe from this list: send the line \"unsubscribe netfilter-devel\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netfilter-devel-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkFYf6QCrz9t2d\n\tfor ;\n\tFri, 1 Sep 2017 20:17:58 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751866AbdIAKR6 (ORCPT );\n\tFri, 1 Sep 2017 06:17:58 -0400","from ganesha.gnumonks.org ([213.95.27.120]:47194 \"EHLO\n\tganesha.gnumonks.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751831AbdIAKR5 (ORCPT\n\t);\n\tFri, 1 Sep 2017 06:17:57 -0400","from 129.166.216.87.static.jazztel.es ([87.216.166.129]\n\thelo=gnumonks.org) by ganesha.gnumonks.org with esmtpsa\n\t(TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2)\n\t(envelope-from )\n\tid 1dnj14-0006fH-1M; Fri, 01 Sep 2017 12:17:35 +0200"],"Date":"Fri, 1 Sep 2017 12:17:33 +0200","From":"Pablo Neira Ayuso ","To":"netfilter-devel@vger.kernel.org","Cc":"phil@nwl.cc, eric@regit.org","Subject":"Re: [PATCH nft 3/3] src: add nft_ctx_netlink_init()","Message-ID":"<20170901101733.GA5539@salvia>","References":"<1504260847-5408-1-git-send-email-pablo@netfilter.org>\n\t<1504260847-5408-3-git-send-email-pablo@netfilter.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1504260847-5408-3-git-send-email-pablo@netfilter.org>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-Spam-Score":"-2.9 (--)","Sender":"netfilter-devel-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netfilter-devel@vger.kernel.org"}},{"id":1761587,"web_url":"http://patchwork.ozlabs.org/comment/1761587/","msgid":"<20170901105049.GN20614@orbyte.nwl.cc>","list_archive_url":null,"date":"2017-09-01T10:50:49","subject":"Re: [PATCH nft 3/3] src: add nft_ctx_netlink_init()","submitter":{"id":4285,"url":"http://patchwork.ozlabs.org/api/people/4285/","name":"Phil Sutter","email":"phil@nwl.cc"},"content":"Hi Pablo,\n\nOn Fri, Sep 01, 2017 at 12:17:33PM +0200, Pablo Neira Ayuso wrote:\n> On Fri, Sep 01, 2017 at 12:14:07PM +0200, Pablo Neira Ayuso wrote:\n> > Add these two new functions to set up netlink sockets in the global\n> > context structure.\n> \n> We can alternatively call this nft_ctx_netlink_auto() if prefer.\n> \n> I'm just trying to skip the type/flag field for nft_ctx_alloc().\n> \n> Does this look acceptable to you to have this extra API to request\n> libnftables to deal with IO details too?\n\nI think we could do it in a simpler way:\n\n| /* create an mnl netlink socket object */\n| struct mnl_socket *netlink_open_sock(void);\n| \n| /* create nft context, optionally passing mnl socket object returned\n| * from netlink_open_sock()\n| * Calling nft_ctx_new(NULL) is equivalent to calling\n| * nft_ctx_new(netlink_open_sock())\n| */\n| static struct nft_ctx *nft_ctx_new(struct mnl_socket *nf_sock);\n\nThis way we allow the application to control mnl_socket object, provide\na simple API for applications which don't need that and at the same time\nalways have ctx->nf_sock point to the socket so we can further simplify\nthings.\n\nWhat do you think?\n\nCheers, Phil\n--\nTo unsubscribe from this list: send the line \"unsubscribe netfilter-devel\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netfilter-devel-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkGHb4YhGz9s8J\n\tfor ;\n\tFri, 1 Sep 2017 20:50:51 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751822AbdIAKuv (ORCPT );\n\tFri, 1 Sep 2017 06:50:51 -0400","from orbyte.nwl.cc ([151.80.46.58]:41129 \"EHLO mail.nwl.cc\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751725AbdIAKuu (ORCPT );\n\tFri, 1 Sep 2017 06:50:50 -0400","from mail.nwl.cc (orbyte.nwl.cc [127.0.0.1])\n\tby mail.nwl.cc (Postfix) with ESMTP id 9CCF6644CF;\n\tFri, 1 Sep 2017 12:50:49 +0200 (CEST)","by mail.nwl.cc (Postfix, from userid 1000)\n\tid 884ED65A5E; Fri, 1 Sep 2017 12:50:49 +0200 (CEST)"],"Date":"Fri, 1 Sep 2017 12:50:49 +0200","From":"Phil Sutter ","To":"Pablo Neira Ayuso ","Cc":"netfilter-devel@vger.kernel.org, eric@regit.org","Subject":"Re: [PATCH nft 3/3] src: add nft_ctx_netlink_init()","Message-ID":"<20170901105049.GN20614@orbyte.nwl.cc>","Mail-Followup-To":"Phil Sutter ,\n\tPablo Neira Ayuso ,\n\tnetfilter-devel@vger.kernel.org, eric@regit.org","References":"<1504260847-5408-1-git-send-email-pablo@netfilter.org>\n\t<1504260847-5408-3-git-send-email-pablo@netfilter.org>\n\t<20170901101733.GA5539@salvia>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170901101733.GA5539@salvia>","User-Agent":"Mutt/1.7.2 (2016-11-26)","X-Virus-Scanned":"ClamAV using ClamSMTP","Sender":"netfilter-devel-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netfilter-devel@vger.kernel.org"}},{"id":1761590,"web_url":"http://patchwork.ozlabs.org/comment/1761590/","msgid":"<20170901105859.GA1195@salvia>","list_archive_url":null,"date":"2017-09-01T10:58:59","subject":"Re: [PATCH nft 3/3] src: add nft_ctx_netlink_init()","submitter":{"id":1315,"url":"http://patchwork.ozlabs.org/api/people/1315/","name":"Pablo Neira Ayuso","email":"pablo@netfilter.org"},"content":"Hi Phil,\n\nOn Fri, Sep 01, 2017 at 12:50:49PM +0200, Phil Sutter wrote:\n> Hi Pablo,\n> \n> On Fri, Sep 01, 2017 at 12:17:33PM +0200, Pablo Neira Ayuso wrote:\n> > On Fri, Sep 01, 2017 at 12:14:07PM +0200, Pablo Neira Ayuso wrote:\n> > > Add these two new functions to set up netlink sockets in the global\n> > > context structure.\n> > \n> > We can alternatively call this nft_ctx_netlink_auto() if prefer.\n> > \n> > I'm just trying to skip the type/flag field for nft_ctx_alloc().\n> > \n> > Does this look acceptable to you to have this extra API to request\n> > libnftables to deal with IO details too?\n> \n> I think we could do it in a simpler way:\n> \n> | /* create an mnl netlink socket object */\n> | struct mnl_socket *netlink_open_sock(void);\n> | \n> | /* create nft context, optionally passing mnl socket object returned\n> | * from netlink_open_sock()\n> | * Calling nft_ctx_new(NULL) is equivalent to calling\n> | * nft_ctx_new(netlink_open_sock())\n> | */\n> | static struct nft_ctx *nft_ctx_new(struct mnl_socket *nf_sock);\n> \n> This way we allow the application to control mnl_socket object, provide\n> a simple API for applications which don't need that and at the same time\n> always have ctx->nf_sock point to the socket so we can further simplify\n> things.\n> \n> What do you think?\n\nI don't want to expose the mnl_socket for the simple API.\n\nOnce we expose the socket, we're allowing people to modify behaviour\nvia setsockopt() toggles. Then, we'll have to tell people not to\nmodify socket options for the simple API, or it can be worst: someone\nwill try to fix this by adding lots of branches to deal with every\nsocket option combination in libnftables.\n\nSo this is an intentional design decision here.\n\nFor people willing to do their own netlink IO, we should push them to\nuse the advanced API.\n--\nTo unsubscribe from this list: send the line \"unsubscribe netfilter-devel\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netfilter-devel-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkGTY0kHXz9sMN\n\tfor ;\n\tFri, 1 Sep 2017 20:59:29 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751862AbdIAK72 (ORCPT );\n\tFri, 1 Sep 2017 06:59:28 -0400","from ganesha.gnumonks.org ([213.95.27.120]:47550 \"EHLO\n\tganesha.gnumonks.org\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751725AbdIAK71 (ORCPT\n\t);\n\tFri, 1 Sep 2017 06:59:27 -0400","from 129.166.216.87.static.jazztel.es ([87.216.166.129]\n\thelo=gnumonks.org) by ganesha.gnumonks.org with esmtpsa\n\t(TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2)\n\t(envelope-from )\n\tid 1dnjfA-0007lK-JT; Fri, 01 Sep 2017 12:59:06 +0200"],"Date":"Fri, 1 Sep 2017 12:58:59 +0200","From":"Pablo Neira Ayuso ","To":"Phil Sutter , netfilter-devel@vger.kernel.org,\n\teric@regit.org","Subject":"Re: [PATCH nft 3/3] src: add nft_ctx_netlink_init()","Message-ID":"<20170901105859.GA1195@salvia>","References":"<1504260847-5408-1-git-send-email-pablo@netfilter.org>\n\t<1504260847-5408-3-git-send-email-pablo@netfilter.org>\n\t<20170901101733.GA5539@salvia>\n\t<20170901105049.GN20614@orbyte.nwl.cc>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170901105049.GN20614@orbyte.nwl.cc>","User-Agent":"Mutt/1.5.23 (2014-03-12)","X-Spam-Score":"-2.9 (--)","Sender":"netfilter-devel-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netfilter-devel@vger.kernel.org"}},{"id":1761598,"web_url":"http://patchwork.ozlabs.org/comment/1761598/","msgid":"<20170901112021.GO20614@orbyte.nwl.cc>","list_archive_url":null,"date":"2017-09-01T11:20:21","subject":"Re: [PATCH nft 3/3] src: add nft_ctx_netlink_init()","submitter":{"id":4285,"url":"http://patchwork.ozlabs.org/api/people/4285/","name":"Phil Sutter","email":"phil@nwl.cc"},"content":"Hi Pablo,\n\nOn Fri, Sep 01, 2017 at 12:58:59PM +0200, Pablo Neira Ayuso wrote:\n> On Fri, Sep 01, 2017 at 12:50:49PM +0200, Phil Sutter wrote:\n> > On Fri, Sep 01, 2017 at 12:17:33PM +0200, Pablo Neira Ayuso wrote:\n> > > On Fri, Sep 01, 2017 at 12:14:07PM +0200, Pablo Neira Ayuso wrote:\n> > > > Add these two new functions to set up netlink sockets in the global\n> > > > context structure.\n> > > \n> > > We can alternatively call this nft_ctx_netlink_auto() if prefer.\n> > > \n> > > I'm just trying to skip the type/flag field for nft_ctx_alloc().\n> > > \n> > > Does this look acceptable to you to have this extra API to request\n> > > libnftables to deal with IO details too?\n> > \n> > I think we could do it in a simpler way:\n> > \n> > | /* create an mnl netlink socket object */\n> > | struct mnl_socket *netlink_open_sock(void);\n> > | \n> > | /* create nft context, optionally passing mnl socket object returned\n> > | * from netlink_open_sock()\n> > | * Calling nft_ctx_new(NULL) is equivalent to calling\n> > | * nft_ctx_new(netlink_open_sock())\n> > | */\n> > | static struct nft_ctx *nft_ctx_new(struct mnl_socket *nf_sock);\n> > \n> > This way we allow the application to control mnl_socket object, provide\n> > a simple API for applications which don't need that and at the same time\n> > always have ctx->nf_sock point to the socket so we can further simplify\n> > things.\n> > \n> > What do you think?\n> \n> I don't want to expose the mnl_socket for the simple API.\n\nWell, technically my proposal changes nft_ctx_new() to be a shared\nfunction between simple and advanced API. Simple users passing NULL as\nparameter won't have to take care of socket init and therefore don't\nhave a separate mnl_socket object they could manipulate.\n\n> Once we expose the socket, we're allowing people to modify behaviour\n> via setsockopt() toggles. Then, we'll have to tell people not to\n> modify socket options for the simple API, or it can be worst: someone\n> will try to fix this by adding lots of branches to deal with every\n> socket option combination in libnftables.\n> \n> So this is an intentional design decision here.\n> \n> For people willing to do their own netlink IO, we should push them to\n> use the advanced API.\n\nYour patches seem to make the simple API unnecessarily complicated by\nforcing users to call nft_ctx_netlink_init(). Why not keep this in\nnft_ctx_new() and provide an alternative to struct nft_ctx for advanced\nAPI? This way simple and advanced functions could be easily identified\nby the type of their first parameter.\n\nMaybe I'm also overlooking something regarding advanced API - can you\nmaybe give an example of a set of function declarations it would consist\nof?\n\nThanks, Phil\n--\nTo unsubscribe from this list: send the line \"unsubscribe netfilter-devel\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netfilter-devel-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkGxl4Qvxz9t2x\n\tfor ;\n\tFri, 1 Sep 2017 21:20:27 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751975AbdIALUZ (ORCPT );\n\tFri, 1 Sep 2017 07:20:25 -0400","from orbyte.nwl.cc ([151.80.46.58]:57765 \"EHLO mail.nwl.cc\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751895AbdIALUX (ORCPT );\n\tFri, 1 Sep 2017 07:20:23 -0400","from mail.nwl.cc (orbyte.nwl.cc [127.0.0.1])\n\tby mail.nwl.cc (Postfix) with ESMTP id D47D7644CF;\n\tFri, 1 Sep 2017 13:20:21 +0200 (CEST)","by mail.nwl.cc (Postfix, from userid 1000)\n\tid C1C2B65A5E; Fri, 1 Sep 2017 13:20:21 +0200 (CEST)"],"Date":"Fri, 1 Sep 2017 13:20:21 +0200","From":"Phil Sutter ","To":"Pablo Neira Ayuso ","Cc":"netfilter-devel@vger.kernel.org, eric@regit.org","Subject":"Re: [PATCH nft 3/3] src: add nft_ctx_netlink_init()","Message-ID":"<20170901112021.GO20614@orbyte.nwl.cc>","Mail-Followup-To":"Phil Sutter ,\n\tPablo Neira Ayuso ,\n\tnetfilter-devel@vger.kernel.org, eric@regit.org","References":"<1504260847-5408-1-git-send-email-pablo@netfilter.org>\n\t<1504260847-5408-3-git-send-email-pablo@netfilter.org>\n\t<20170901101733.GA5539@salvia>\n\t<20170901105049.GN20614@orbyte.nwl.cc>\n\t<20170901105859.GA1195@salvia>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170901105859.GA1195@salvia>","User-Agent":"Mutt/1.7.2 (2016-11-26)","X-Virus-Scanned":"ClamAV using ClamSMTP","Sender":"netfilter-devel-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netfilter-devel@vger.kernel.org"}},{"id":1761648,"web_url":"http://patchwork.ozlabs.org/comment/1761648/","msgid":"<20170901122848.GB951@breakpoint.cc>","list_archive_url":null,"date":"2017-09-01T12:28:48","subject":"Re: [PATCH nft 3/3] src: add nft_ctx_netlink_init()","submitter":{"id":1025,"url":"http://patchwork.ozlabs.org/api/people/1025/","name":"Florian Westphal","email":"fw@strlen.de"},"content":"Pablo Neira Ayuso wrote:\n> On Fri, Sep 01, 2017 at 12:14:07PM +0200, Pablo Neira Ayuso wrote:\n> > Add these two new functions to set up netlink sockets in the global\n> > context structure.\n> \n> We can alternatively call this nft_ctx_netlink_auto() if prefer.\n\nI think its good as-is.\n\nI also agree with not exposing libmnl anywhere in the api.\n\n--\nTo unsubscribe from this list: send the line \"unsubscribe netfilter-devel\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netfilter-devel-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkJWx185Hz9t1t\n\tfor ;\n\tFri, 1 Sep 2017 22:31:41 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751916AbdIAMbj (ORCPT );\n\tFri, 1 Sep 2017 08:31:39 -0400","from Chamillionaire.breakpoint.cc ([146.0.238.67]:45574 \"EHLO\n\tChamillionaire.breakpoint.cc\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751777AbdIAMbh (ORCPT\n\t);\n\tFri, 1 Sep 2017 08:31:37 -0400","from fw by Chamillionaire.breakpoint.cc with local (Exim 4.84_2)\n\t(envelope-from )\n\tid 1dnl44-0001GS-Tq; Fri, 01 Sep 2017 14:28:49 +0200"],"Date":"Fri, 1 Sep 2017 14:28:48 +0200","From":"Florian Westphal ","To":"Pablo Neira Ayuso ","Cc":"netfilter-devel@vger.kernel.org, phil@nwl.cc, eric@regit.org","Subject":"Re: [PATCH nft 3/3] src: add nft_ctx_netlink_init()","Message-ID":"<20170901122848.GB951@breakpoint.cc>","References":"<1504260847-5408-1-git-send-email-pablo@netfilter.org>\n\t<1504260847-5408-3-git-send-email-pablo@netfilter.org>\n\t<20170901101733.GA5539@salvia>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170901101733.GA5539@salvia>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"netfilter-devel-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netfilter-devel@vger.kernel.org"}}]