Re:Re: [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded

[Gao Feng]

Hi Liping,

At 2017-05-05 09:59:06, "Liping Zhang"  wrote:
>Hi Feng,
>
>2017-05-05 8:55 GMT+08:00  :
>[...]
>> +static void
>> +nf_ct_flush_expect(const struct module *me)
>> +{
>> +   struct nf_conntrack_expect *exp;
>> +   const struct hlist_node *next;
>> +   u32 i;
>> +
>> +   if (!me)
>> +   return;
>> +
>> +   /* Get rid of expectations */
>> +   spin_lock_bh(_conntrack_expect_lock);
>> +   for (i = 0; i < nf_ct_expect_hsize; i++) {
>> +   hlist_for_each_entry_safe(exp, next,
>> + _ct_expect_hash[i], hnode) {
>> +   struct nf_conn_help *master_help = 
>> nfct_help(exp->master);
>> +
>> +   if ((master_help->helper && master_help->helper->me 
>> == me) ||
>> +   (exp->helper && exp->helper->me == me) ||
>> +   (exp->nat_helper && exp->nat_helper->me == me)) {
>> +   /* This expect belongs to the dying module */
>> +   if (del_timer(>timeout)) {
>> +   nf_ct_unlink_expect(exp);
>> +   nf_ct_expect_put(exp);
>> +   }
>
>Is this should be replaced with your helper function nf_ct_remove_expect()?

Thanks. Because this helper was applied in nf-next.git before, not nf.git.
So I didn't use the helper function. I just checked some days ago.

I pull the nf.git codes just now, the helper is already synchronized at May 
1st.  
But I don't pull the codes recently, so I didn't find it before.

I would like to update it, but need to wait for a while to collect more 
comments.
Thanks again.

Best Regards
Feng
>
>> +   }
>> +   }
>> +   }
>> +   spin_unlock_bh(_conntrack_expect_lock);

Re: [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded

[Liping Zhang]

Hi Feng,

2017-05-05 8:55 GMT+08:00  :
[...]
> +static void
> +nf_ct_flush_expect(const struct module *me)
> +{
> +   struct nf_conntrack_expect *exp;
> +   const struct hlist_node *next;
> +   u32 i;
> +
> +   if (!me)
> +   return;
> +
> +   /* Get rid of expectations */
> +   spin_lock_bh(_conntrack_expect_lock);
> +   for (i = 0; i < nf_ct_expect_hsize; i++) {
> +   hlist_for_each_entry_safe(exp, next,
> + _ct_expect_hash[i], hnode) {
> +   struct nf_conn_help *master_help = 
> nfct_help(exp->master);
> +
> +   if ((master_help->helper && master_help->helper->me 
> == me) ||
> +   (exp->helper && exp->helper->me == me) ||
> +   (exp->nat_helper && exp->nat_helper->me == me)) {
> +   /* This expect belongs to the dying module */
> +   if (del_timer(>timeout)) {
> +   nf_ct_unlink_expect(exp);
> +   nf_ct_expect_put(exp);
> +   }

Is this should be replaced with your helper function nf_ct_remove_expect()?

> +   }
> +   }
> +   }
> +   spin_unlock_bh(_conntrack_expect_lock);
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH nf v6 1/3] netfilter: helper: Rename struct nf_ct_helper_expectfn to nf_ct_nat_helper

[gfree . wind]

From: Gao Feng 

Rename struct nf_ct_helper_expectfn to nf_ct_nat_helper, and rename
other functions or variables which refer to it.
The new name is better than the old one.

Signed-off-by: Gao Feng 
---
 v6: Rename the helper name of ftp, tftp.. to ftp-nat-follow-master, per Pablo
 v5: Register one nat_helper for every nat module, per Pablo
 v4: Cover the nat_module assignment in dataplane, per Pablo
 v3: Rename the nf_ct_helper_expectfn, func, and member, per Pablo
 v2: Use the module as the identifier when flush expect
 v1: initial version

 include/net/netfilter/nf_conntrack_helper.h | 14 ++---
 net/ipv4/netfilter/nf_nat_h323.c| 12 +--
 net/netfilter/nf_conntrack_helper.c | 32 ++---
 net/netfilter/nf_conntrack_netlink.c| 16 +++
 net/netfilter/nf_nat_core.c |  6 +++---
 net/netfilter/nf_nat_sip.c  |  6 +++---
 6 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_helper.h 
b/include/net/netfilter/nf_conntrack_helper.h
index 1eaac1f..d14fe493 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -111,7 +111,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb, 
unsigned int protoff,
enum ip_conntrack_info ctinfo,
unsigned int timeout);
 
-struct nf_ct_helper_expectfn {
+struct nf_ct_nat_helper {
struct list_head head;
const char *name;
void (*expectfn)(struct nf_conn *ct, struct nf_conntrack_expect *exp);
@@ -121,12 +121,12 @@ struct nf_ct_helper_expectfn {
 void nf_ct_helper_log(struct sk_buff *skb, const struct nf_conn *ct,
  const char *fmt, ...);
 
-void nf_ct_helper_expectfn_register(struct nf_ct_helper_expectfn *n);
-void nf_ct_helper_expectfn_unregister(struct nf_ct_helper_expectfn *n);
-struct nf_ct_helper_expectfn *
-nf_ct_helper_expectfn_find_by_name(const char *name);
-struct nf_ct_helper_expectfn *
-nf_ct_helper_expectfn_find_by_symbol(const void *symbol);
+void nf_ct_nat_helper_register(struct nf_ct_nat_helper *n);
+void nf_ct_nat_helper_unregister(struct nf_ct_nat_helper *n);
+struct nf_ct_nat_helper *
+nf_ct_nat_helper_find_by_name(const char *name);
+struct nf_ct_nat_helper *
+nf_ct_nat_helper_find_by_symbol(const void *symbol);
 
 extern struct hlist_head *nf_ct_helper_hash;
 extern unsigned int nf_ct_helper_hsize;
diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index 574f7eb..346e764 100644
--- a/net/ipv4/netfilter/nf_nat_h323.c
+++ b/net/ipv4/netfilter/nf_nat_h323.c
@@ -567,12 +567,12 @@ static int nat_callforwarding(struct sk_buff *skb, struct 
nf_conn *ct,
return 0;
 }
 
-static struct nf_ct_helper_expectfn q931_nat = {
+static struct nf_ct_nat_helper q931_nat = {
.name   = "Q.931",
.expectfn   = ip_nat_q931_expect,
 };
 
-static struct nf_ct_helper_expectfn callforwarding_nat = {
+static struct nf_ct_nat_helper callforwarding_nat = {
.name   = "callforwarding",
.expectfn   = ip_nat_callforwarding_expect,
 };
@@ -599,8 +599,8 @@ static int __init init(void)
RCU_INIT_POINTER(nat_h245_hook, nat_h245);
RCU_INIT_POINTER(nat_callforwarding_hook, nat_callforwarding);
RCU_INIT_POINTER(nat_q931_hook, nat_q931);
-   nf_ct_helper_expectfn_register(_nat);
-   nf_ct_helper_expectfn_register(_nat);
+   nf_ct_nat_helper_register(_nat);
+   nf_ct_nat_helper_register(_nat);
return 0;
 }
 
@@ -616,8 +616,8 @@ static void __exit fini(void)
RCU_INIT_POINTER(nat_h245_hook, NULL);
RCU_INIT_POINTER(nat_callforwarding_hook, NULL);
RCU_INIT_POINTER(nat_q931_hook, NULL);
-   nf_ct_helper_expectfn_unregister(_nat);
-   nf_ct_helper_expectfn_unregister(_nat);
+   nf_ct_nat_helper_unregister(_nat);
+   nf_ct_nat_helper_unregister(_nat);
synchronize_rcu();
 }
 
diff --git a/net/netfilter/nf_conntrack_helper.c 
b/net/netfilter/nf_conntrack_helper.c
index 99bcd44..1fd739c 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -302,32 +302,32 @@ void nf_ct_helper_destroy(struct nf_conn *ct)
}
 }
 
-static LIST_HEAD(nf_ct_helper_expectfn_list);
+static LIST_HEAD(nf_ct_nat_helper_list);
 
-void nf_ct_helper_expectfn_register(struct nf_ct_helper_expectfn *n)
+void nf_ct_nat_helper_register(struct nf_ct_nat_helper *n)
 {
spin_lock_bh(_conntrack_expect_lock);
-   list_add_rcu(>head, _ct_helper_expectfn_list);
+   list_add_rcu(>head, _ct_nat_helper_list);
spin_unlock_bh(_conntrack_expect_lock);
 }
-EXPORT_SYMBOL_GPL(nf_ct_helper_expectfn_register);
+EXPORT_SYMBOL_GPL(nf_ct_nat_helper_register);
 
-void nf_ct_helper_expectfn_unregister(struct nf_ct_helper_expectfn *n)
+void 

[PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded

[gfree . wind]

From: Gao Feng 

Because the conntrack NAT module could be rmmod anytime, so we should
really leave things in clean state if such thing happens and make sure
we don't leave any packet running over code that will be gone after the
removal.

We only removed the expectations when unregister conntrack helper before.
Actually it is necessary too when remove the nat helper.

Now add one new struct module member "me" in the nf_ct_nat_helper to
represent which module it belongs to.

Signed-off-by: Gao Feng 
---
 v6: Rename the helper name of ftp, tftp.. to ftp-nat-follow-master, per Pablo
 v5: Register one nat_helper for every nat module, per Pablo
 v4: Cover the nat_module assignment in dataplane, per Pablo
 v3: Rename the nf_ct_helper_expectfn, func, and member, per Pablo
 v2: Use the module as the identifier when flush expect
 v1: initial version

 include/net/netfilter/nf_conntrack_helper.h |  1 +
 net/ipv4/netfilter/nf_nat_h323.c|  3 ++
 net/netfilter/ipvs/ip_vs_nfct.c |  1 +
 net/netfilter/nf_conntrack_helper.c | 62 +++--
 net/netfilter/nf_conntrack_pptp.c   |  1 +
 net/netfilter/nf_nat_amanda.c   |  1 +
 net/netfilter/nf_nat_core.c |  1 +
 net/netfilter/nf_nat_ftp.c  |  1 +
 net/netfilter/nf_nat_irc.c  |  1 +
 net/netfilter/nf_nat_sip.c  |  1 +
 net/netfilter/nf_nat_tftp.c |  1 +
 11 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_helper.h 
b/include/net/netfilter/nf_conntrack_helper.h
index e8d31ca..d70c5be 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -114,6 +114,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb, 
unsigned int protoff,
 struct nf_ct_nat_helper {
struct list_head head;
const char *name;
+   struct module *me;
void (*expectfn)(struct nf_conn *ct, struct nf_conntrack_expect *exp);
 };
 
diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index ce2095c..d7b40ac 100644
--- a/net/ipv4/netfilter/nf_nat_h323.c
+++ b/net/ipv4/netfilter/nf_nat_h323.c
@@ -28,16 +28,19 @@ static void ip_nat_callforwarding_expect(struct nf_conn 
*new,
 
 static struct nf_ct_nat_helper q931_nat = {
.name   = "Q.931",
+   .me = THIS_MODULE,
.expectfn   = ip_nat_q931_expect,
 };
 
 static struct nf_ct_nat_helper callforwarding_nat = {
.name   = "callforwarding",
+   .me = THIS_MODULE,
.expectfn   = ip_nat_callforwarding_expect,
 };
 
 static struct nf_ct_nat_helper follow_master_nat = {
.name   = "h323-nat-follow-master",
+   .me = THIS_MODULE,
.expectfn   = nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/ipvs/ip_vs_nfct.c b/net/netfilter/ipvs/ip_vs_nfct.c
index fc5af4f..d61c236 100644
--- a/net/netfilter/ipvs/ip_vs_nfct.c
+++ b/net/netfilter/ipvs/ip_vs_nfct.c
@@ -222,6 +222,7 @@ static void ip_vs_nfct_expect_callback(struct nf_conn *ct,
 
 static struct nf_ct_nat_helper ip_vs_nat = {
.name = "ip-vs-nat",
+   .me = THIS_MODULE,
.expectfn = ip_vs_nfct_expect_callback,
 };
 
diff --git a/net/netfilter/nf_conntrack_helper.c 
b/net/netfilter/nf_conntrack_helper.c
index 1fd739c..c12de31 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -130,6 +130,37 @@ static unsigned int helper_hash(const struct 
nf_conntrack_tuple *tuple)
return NULL;
 }
 
+static void
+nf_ct_flush_expect(const struct module *me)
+{
+   struct nf_conntrack_expect *exp;
+   const struct hlist_node *next;
+   u32 i;
+
+   if (!me)
+   return;
+
+   /* Get rid of expectations */
+   spin_lock_bh(_conntrack_expect_lock);
+   for (i = 0; i < nf_ct_expect_hsize; i++) {
+   hlist_for_each_entry_safe(exp, next,
+ _ct_expect_hash[i], hnode) {
+   struct nf_conn_help *master_help = 
nfct_help(exp->master);
+
+   if ((master_help->helper && master_help->helper->me == 
me) ||
+   (exp->helper && exp->helper->me == me) ||
+   (exp->nat_helper && exp->nat_helper->me == me)) {
+   /* This expect belongs to the dying module */
+   if (del_timer(>timeout)) {
+   nf_ct_unlink_expect(exp);
+   nf_ct_expect_put(exp);
+   }
+   }
+   }
+   }
+   spin_unlock_bh(_conntrack_expect_lock);
+}
+
 struct nf_conntrack_helper *
 __nf_conntrack_helper_find(const char *name, u16 l3num, u8 protonum)
 {
@@ -317,6 +348,12 @@ void 

[PATCH nf v6 2/3] netfilter: nat_helper: Register one nf_ct_nat_helper each proto nat module

[gfree . wind]

From: Gao Feng 

There are multiple proto nat modules which depend on the follow_master_nat
in the nf_nat_core. When this module is gone, all modules which refers to
it could not work well.

Now register one struct nf_ct_nat_helper in every proto nat module, it makes
sure every nat module could not effect each other, and it is helpful to
maintain the codes. The new nf_ct_nat_helper could be found by ctlink with
its name too, while the original "nat-follow-master" helper is not effected.
ctlink still could specify the "nat-follow-master" for ftp, but recommend
use the ftp's nat_helper "ftp-nat-follow-master" as the ftp nat_helper.

The following are all modifications.
1. Every proto nat module registers one nat_helper at least;
2. Replace the expectfn with nat_helper in the nf_conntrack_expect;
It is used to the latter commit which add the nat_helper module check.
3. Remove nf_ct_nat_helper_find_by_symbol, it is useless now.

Signed-off-by: Gao Feng 
---
 v6: Rename the helper name of ftp, tftp.. to ftp-nat-follow-master, per Pablo
 v5: Register one nat_helper for every nat module, per Pablo
 v4: Cover the nat_module assignment in dataplane, per Pablo
 v3: Rename the nf_ct_helper_expectfn, func, and member, per Pablo
 v2: Use the module as the identifier when flush expect
 v1: initial version

 include/net/netfilter/nf_conntrack_expect.h |  5 ++--
 include/net/netfilter/nf_conntrack_helper.h |  2 --
 net/ipv4/netfilter/nf_nat_h323.c| 44 ++---
 net/netfilter/ipvs/ip_vs_nfct.c |  7 -
 net/netfilter/nf_conntrack_broadcast.c  |  2 +-
 net/netfilter/nf_conntrack_core.c   |  4 +--
 net/netfilter/nf_conntrack_expect.c |  2 +-
 net/netfilter/nf_conntrack_netlink.c| 14 -
 net/netfilter/nf_conntrack_pptp.c   | 14 +++--
 net/netfilter/nf_nat_amanda.c   |  9 +-
 net/netfilter/nf_nat_ftp.c  |  9 +-
 net/netfilter/nf_nat_irc.c  |  9 +-
 net/netfilter/nf_nat_sip.c  | 18 ++--
 net/netfilter/nf_nat_tftp.c |  9 +-
 14 files changed, 101 insertions(+), 47 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_expect.h 
b/include/net/netfilter/nf_conntrack_expect.h
index 5ed33ea..f665a6b 100644
--- a/include/net/netfilter/nf_conntrack_expect.h
+++ b/include/net/netfilter/nf_conntrack_expect.h
@@ -23,9 +23,8 @@ struct nf_conntrack_expect {
struct nf_conntrack_tuple tuple;
struct nf_conntrack_tuple_mask mask;
 
-   /* Function to call after setup and insertion */
-   void (*expectfn)(struct nf_conn *new,
-struct nf_conntrack_expect *this);
+   /* Expectation function owner */
+   struct nf_ct_nat_helper *nat_helper;
 
/* Helper to assign to new connection */
struct nf_conntrack_helper *helper;
diff --git a/include/net/netfilter/nf_conntrack_helper.h 
b/include/net/netfilter/nf_conntrack_helper.h
index d14fe493..e8d31ca 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -125,8 +125,6 @@ void nf_ct_helper_log(struct sk_buff *skb, const struct 
nf_conn *ct,
 void nf_ct_nat_helper_unregister(struct nf_ct_nat_helper *n);
 struct nf_ct_nat_helper *
 nf_ct_nat_helper_find_by_name(const char *name);
-struct nf_ct_nat_helper *
-nf_ct_nat_helper_find_by_symbol(const void *symbol);
 
 extern struct hlist_head *nf_ct_helper_hash;
 extern unsigned int nf_ct_helper_hsize;
diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index 346e764..ce2095c 100644
--- a/net/ipv4/netfilter/nf_nat_h323.c
+++ b/net/ipv4/netfilter/nf_nat_h323.c
@@ -21,6 +21,26 @@
 #include 
 
 //
+static void ip_nat_q931_expect(struct nf_conn *new,
+  struct nf_conntrack_expect *this);
+static void ip_nat_callforwarding_expect(struct nf_conn *new,
+struct nf_conntrack_expect *this);
+
+static struct nf_ct_nat_helper q931_nat = {
+   .name   = "Q.931",
+   .expectfn   = ip_nat_q931_expect,
+};
+
+static struct nf_ct_nat_helper callforwarding_nat = {
+   .name   = "callforwarding",
+   .expectfn   = ip_nat_callforwarding_expect,
+};
+
+static struct nf_ct_nat_helper follow_master_nat = {
+   .name   = "h323-nat-follow-master",
+   .expectfn   = nf_nat_follow_master,
+};
+
 static int set_addr(struct sk_buff *skb, unsigned int protoff,
unsigned char **data, int dataoff,
unsigned int addroff, __be32 ip, __be16 port)
@@ -187,10 +207,10 @@ static int nat_rtp_rtcp(struct sk_buff *skb, struct 
nf_conn *ct,
 
/* Set expectations for NAT */
rtp_exp->saved_proto.udp.port = rtp_exp->tuple.dst.u.udp.port;
-   

Re: [nft PATCH] List handles of added rules if requested

[Phil Sutter]

On Thu, May 04, 2017 at 05:37:25PM +0200, Thomas Woerner wrote:
> On 05/04/2017 03:44 PM, Florian Westphal wrote:
> > Pablo Neira Ayuso  wrote:
> >> On Thu, May 04, 2017 at 02:34:21PM +0200, Phil Sutter wrote:
> >>> Being able to retrieve an added rule's handle atomically is a crucial
> >>> feature for scripts invoking nft command: Without it, there is no way to
> >>> be sure a handle extracted from 'nft list ruleset' command actually
> >>> refers to the rule one has added before or that of another process which
> >>> ran in between.
> >>>
> >>> Extracting an added rule's handle itself is not an easy task already,
> >>> since there is a chance that a given rule is printed differently than
> >>> when it was added before. A simple example is port number vs. service
> >>> name:
> >>>
> >>> | nft add rule ip t c tcp dport { ssh, 80 } accept
> >>>
> >>> There is no way to make 'nft list ruleset' return the rule just like
> >>> this as depending on whether '-nn' was given or not, it either prints
> >>> the set as '{ ssh, http }' or '{ 22, 80 }' but never in the mixed form
> >>> that was used when adding it.
> >>>
> >>> This patch prints an identifying string for each added rule which may be
> >>> used as single parameter to a later 'nft delete rule' command. So a
> >>> simple scripting example looks like this:
> >>>
> >>> | handle=$(nft add rule ip t c counter)
> >>
> >> This is a hack.
> >>
> >> We should follow the rule description path.
> > 
> > You mean delete-by-name?
> > 
> > Its just as ugly, just a different kind of ugly.
> > 
> Delete by name for rules would be an atomic operation. The solution above is 
> not as two calls are needed to remove one rule. Saving the handles while the 
> rule has been added is not possible all the time and will most likely require 
> bigger changes in several projects.

This was not the aspect I was talking about when mentioning missing
atomicity: I didn't have the delete by description approach in mind at
all, and looking at delete by handle the problem is that it requires a
non-atomic operation to add a rule and retrieve it's handle.

With the proposed solution in place, if you take care to collect the
handles for the rules you're adding along the way, you can be sure
you're later deleting exactly the rule you've added before.

But I agree with you in that the situation right now is really messy:
Add a rule, call 'nft -a list ruleset' and then have fun parsing the
output and searching for your rule. I guess pretty much anything is
better than that. :)

> I also do not understand why delete by name is possible for tables, chains 
> and 
> sets but not for rules.

I didn't design all this, but I guess an explanation would be that table
names and the combination of table name and chain name are unique, while
the rule description is not.

Cheers, Phil
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft PATCH] List handles of added rules if requested

[Phil Sutter]

On Thu, May 04, 2017 at 04:00:42PM +0200, Pablo Neira Ayuso wrote:
> On Thu, May 04, 2017 at 03:44:19PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso  wrote:
> > > On Thu, May 04, 2017 at 02:34:21PM +0200, Phil Sutter wrote:
> > > > Being able to retrieve an added rule's handle atomically is a crucial
> > > > feature for scripts invoking nft command: Without it, there is no way to
> > > > be sure a handle extracted from 'nft list ruleset' command actually
> > > > refers to the rule one has added before or that of another process which
> > > > ran in between.
> > > > 
> > > > Extracting an added rule's handle itself is not an easy task already,
> > > > since there is a chance that a given rule is printed differently than
> > > > when it was added before. A simple example is port number vs. service
> > > > name:
> > > > 
> > > > | nft add rule ip t c tcp dport { ssh, 80 } accept
> > > > 
> > > > There is no way to make 'nft list ruleset' return the rule just like
> > > > this as depending on whether '-nn' was given or not, it either prints
> > > > the set as '{ ssh, http }' or '{ 22, 80 }' but never in the mixed form
> > > > that was used when adding it.
> > > > 
> > > > This patch prints an identifying string for each added rule which may be
> > > > used as single parameter to a later 'nft delete rule' command. So a
> > > > simple scripting example looks like this:
> > > > 
> > > > | handle=$(nft add rule ip t c counter)
> > > 
> > > This is a hack.
> > > 
> > > We should follow the rule description path.
> > 
> > You mean delete-by-name?
> > 
> > Its just as ugly, just a different kind of ugly.
> 
> Ugly?
> 
> This kernel patch is seriouly broken. It's sending a message to
> userspace from the preparation phase of the commit protocol, where
> things are not even confirmed at all...

Oh, I indeed ignored the transactional behaviour at all. But looking at
the code, if I use NLM_F_ECHO that should be fine, right?

> > Will you delete the first match?  The last one?  All of them?
> 
> I already explained this Florian. Please, look at the mail archive.

While I think it's not a bad idea to allow users experienced with
iptables to apply their muscle memory to nftables as well, I don't quite
get what should hold us back from leveraging this feature nftables
provides over iptables. The existence of a unique identifier is a big
plus in my point of view, it's just not really useful yet since users
have no safe way to get that handle for the rule they added.

Are you OK with providing both alternatives in parallel? If not, why?

Thanks, Phil
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -stable 0/3] ipvs: patches for stable

[Julian Anastasov]

Hello,

On Thu, 4 May 2017, Pablo Neira Ayuso wrote:

> On Mon, May 01, 2017 at 04:45:34PM +0300, Julian Anastasov wrote:
> > Hello,
> > 
> > The following patches are rediffs for "ipvs: SNAT packet replies
> > only for NATed connections" for different stable kernels:
> > 
> > The official patch for the net tree (will come from Simon) works for:
> > 4.9.25, 4.10.13, 4.11, net tree
> > 
> > Patch 1: 3.2.88, 3.4.113
> > Patch 2: 3.10.105, 3.12.73, 3.16.43, 4.1.39
> > Patch 3: 4.4.65
> 
> Thanks Julian.
> 
> Could you indicate what upstream commit these patches refer to? stable
> maintainer need this.
> 
> These are backports, right?

Yes. Simon added the official patch to the ipvs tree and
the next step is to post it to you. May be you have to defer
these backports. If needed, I can send them again later.

Regards

--
Julian Anastasov 
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nft] parser: allow listing sets in one table

[Pablo Neira Ayuso]

On Thu, May 04, 2017 at 06:09:31PM +0200, Florian Westphal wrote:
> currently nft can lists sets:
> nft list sets
> 
> but unlike e.g. 'quotas' or 'counters' we didn't support
> restricting it to a table.  Now its possible to restrict set
> definition listing to one table:
> 
> nft list sets table inet filter
> 
> Signed-off-by: Florian Westphal 

Acked-by: Pablo Neira Ayuso 
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH nft] parser: allow listing sets in one table

[Florian Westphal]

currently nft can lists sets:
nft list sets

but unlike e.g. 'quotas' or 'counters' we didn't support
restricting it to a table.  Now its possible to restrict set
definition listing to one table:

nft list sets table inet filter

Signed-off-by: Florian Westphal 
---
 src/evaluate.c | 2 +-
 src/parser_bison.y | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 49c5953ae168..1cfe7675162e 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3037,6 +3037,7 @@ static int cmd_evaluate_list(struct eval_ctx *ctx, struct 
cmd *cmd)
case CMD_OBJ_COUNTERS:
case CMD_OBJ_QUOTAS:
case CMD_OBJ_CT_HELPERS:
+   case CMD_OBJ_SETS:
if (cmd->handle.table == NULL)
return 0;
if (table_lookup(>handle) == NULL)
@@ -3044,7 +3045,6 @@ static int cmd_evaluate_list(struct eval_ctx *ctx, struct 
cmd *cmd)
 cmd->handle.table);
return 0;
case CMD_OBJ_CHAINS:
-   case CMD_OBJ_SETS:
case CMD_OBJ_RULESET:
case CMD_OBJ_FLOWTABLES:
case CMD_OBJ_MAPS:
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 8db887a47d3e..6be94a9b873f 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1007,6 +1007,10 @@ list_cmd :   TABLE   table_spec
{
$$ = cmd_alloc(CMD_LIST, CMD_OBJ_SETS, &$2, 
&@$, NULL);
}
+   |   SETSTABLE   table_spec
+   {
+   $$ = cmd_alloc(CMD_LIST, CMD_OBJ_SETS, &$3, 
&@$, NULL);
+   }
|   SET set_spec
{
$$ = cmd_alloc(CMD_LIST, CMD_OBJ_SET, &$2, &@$, 
NULL);
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft PATCH] List handles of added rules if requested

[Thomas Woerner]

On 05/04/2017 03:44 PM, Florian Westphal wrote:

Pablo Neira Ayuso  wrote:

On Thu, May 04, 2017 at 02:34:21PM +0200, Phil Sutter wrote:

Being able to retrieve an added rule's handle atomically is a crucial
feature for scripts invoking nft command: Without it, there is no way to
be sure a handle extracted from 'nft list ruleset' command actually
refers to the rule one has added before or that of another process which
ran in between.

Extracting an added rule's handle itself is not an easy task already,
since there is a chance that a given rule is printed differently than
when it was added before. A simple example is port number vs. service
name:

| nft add rule ip t c tcp dport { ssh, 80 } accept

There is no way to make 'nft list ruleset' return the rule just like
this as depending on whether '-nn' was given or not, it either prints
the set as '{ ssh, http }' or '{ 22, 80 }' but never in the mixed form
that was used when adding it.

This patch prints an identifying string for each added rule which may be
used as single parameter to a later 'nft delete rule' command. So a
simple scripting example looks like this:

| handle=$(nft add rule ip t c counter)


This is a hack.

We should follow the rule description path.


You mean delete-by-name?

Its just as ugly, just a different kind of ugly.

Delete by name for rules would be an atomic operation. The solution above is 
not as two calls are needed to remove one rule. Saving the handles while the 
rule has been added is not possible all the time and will most likely require 
bigger changes in several projects.


I also do not understand why delete by name is possible for tables, chains and 
sets but not for rules.


Delete by name for rules is also something that would help to be able to create 
an nftables backend for firewalld easily. firewalld is using a transaction 
model for iptables, that I'd also like to use for nftables. With this it is 
possible to add and remove rules in one -restore call for IPv4 and one for 
IPv6. Using this with gathering handles will take more time and will not be atomic.


Though: Flushing the whole rule set and recreation of the new rule set could be 
done in an atomic operation. But it will destroy everything else.



Will you delete the first match?  The last one?  All of them? > --
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nf-next PATCH] netfilter: nf_tables: Return info of added rules back to user space

[Phil Sutter]

On Thu, May 04, 2017 at 03:35:44PM +0200, Pablo Neira Ayuso wrote:
> On Thu, May 04, 2017 at 02:34:17PM +0200, Phil Sutter wrote:
> > This allows user space to reliably match kernel generated handles with
> > added rules for reference.
> > 
> > Signed-off-by: Phil Sutter 
> > ---
> >  net/netfilter/nf_tables_api.c | 19 ++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index 1c6482d2c4dcf..71bce5d024409 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -2142,6 +2142,7 @@ static int nf_tables_newrule(struct net *net, struct 
> > sock *nlsk,
> > struct nft_userdata *udata;
> > struct nft_trans *trans = NULL;
> > struct nft_expr *expr;
> > +   struct sk_buff *skb2;
> > struct nft_ctx ctx;
> > struct nlattr *tmp;
> > unsigned int size, i, n, ulen = 0, usize = 0;
> > @@ -2281,8 +2282,24 @@ static int nf_tables_newrule(struct net *net, struct 
> > sock *nlsk,
> > goto err3;
> > }
> > chain->use++;
> > -   return 0;
> >  
> > +   skb2 = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
> > +   if (!skb2) {
> > +   err = -ENOMEM;
> > +   goto err4;
> > +   }
> > +   err = nf_tables_fill_rule_info(skb2, net, NETLINK_CB(skb).portid,
> > +  nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0,
> > +  nfmsg->nfgen_family, table, chain, rule);
> > +   if (err < 0)
> > +   goto err5;
> > +
> > +   return nlmsg_unicast(nlsk, skb2, NETLINK_CB(skb).portid);
> 
> You can achieve this already via NLM_F_ECHO.

Oh, thanks for the pointer!

Cheers, Phil
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -stable 0/3] ipvs: patches for stable

[Pablo Neira Ayuso]

On Mon, May 01, 2017 at 04:45:34PM +0300, Julian Anastasov wrote:
>   Hello,
> 
> The following patches are rediffs for "ipvs: SNAT packet replies
> only for NATed connections" for different stable kernels:
> 
> The official patch for the net tree (will come from Simon) works for:
> 4.9.25, 4.10.13, 4.11, net tree
> 
> Patch 1: 3.2.88, 3.4.113
> Patch 2: 3.10.105, 3.12.73, 3.16.43, 4.1.39
> Patch 3: 4.4.65

Thanks Julian.

Could you indicate what upstream commit these patches refer to? stable
maintainer need this.

These are backports, right?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: net: possible deadlock in skb_queue_tail

[Paolo Abeni]

On Thu, 2017-05-04 at 15:49 +0200, Andrey Konovalov wrote:
> On Fri, Feb 24, 2017 at 3:56 AM, Florian Westphal  wrote:
> > Andrey Konovalov  wrote:
> > 
> > [ CC Paolo ]
> > 
> > > I've got the following error report while fuzzing the kernel with 
> > > syzkaller.
> > > 
> > > On commit c470abd4fde40ea6a0846a2beab642a578c0b8cd (4.10).
> > > 
> > > Unfortunately I can't reproduce it.
> > 
> > This needs NETLINK_BROADCAST_ERROR enabled on a netlink socket
> > that then subscribes to netfilter conntrack (ctnetlink) events.
> > probably syzkaller did this by accident -- impressive.
> > 
> > (one task is the ctnetlink event redelivery worker
> >  which won't be scheduled otherwise).
> > 
> > > ==
> > > [ INFO: possible circular locking dependency detected ]
> > > 4.10.0-rc8+ #201 Not tainted
> > > ---
> > > kworker/0:2/1404 is trying to acquire lock:
> > >  (&(>lock)->rlock#3){+.-...}, at: []
> > > skb_queue_tail+0xcf/0x2f0 net/core/skbuff.c:2478
> > > 
> > > but task is already holding lock:
> > >  (&(>lock)->rlock){+.-...}, at: [] spin_lock
> > > include/linux/spinlock.h:302 [inline]
> > >  (&(>lock)->rlock){+.-...}, at: []
> > > ecache_work_evict_list+0xaf/0x590
> > > net/netfilter/nf_conntrack_ecache.c:48
> > > 
> > > which lock already depends on the new lock.
> > 
> > Cong is correct, this is a false positive.
> > 
> > However we should fix this splat.
> > 
> > Paolo, this happens since 7c13f97ffde63cc792c49ec1513f3974f2f05229
> > ('udp: do fwd memory scheduling on dequeue'), before this
> > commit kfree_skb() was invoked outside of the locked section in
> > first_packet_length().
> > 
> > cpu 0 call chain:
> > - first_packet_length (hold udp sk_receive_queue lock)
> >- kfree_skb
> >   - nf_conntrack_destroy
> >  - spin_lock(net->ct.pcpu->lock)
> > 
> > cpu 1 call chain:
> > - ecache_work_evict_list
> >   - spin_lock( net->ct.pcpu->lock)
> >   - nf_conntrack_event
> >  - aquire netlink socket sk_receive_queue
> > 
> > So this could only ever deadlock if a netlink socket
> > calls kfree_skb while holding its sk_receive_queue lock, but afaics
> > this is never the case.
> > 
> > There are two ways to avoid this splat (other than lockdep annotation):
> > 
> > 1. re-add the list to first_packet_length() and free the
> > skbs outside of locked section.
> > 
> > 2. change ecache_work_evict_list to not call nf_conntrack_event()
> > while holding the pcpu lock.
> > 
> > doing #2 might be a good idea anyway to avoid potential deadlock
> > when kfree_skb gets invoked while other cpu holds its sk_receive_queue
> > lock, I'll have a look if this is feasible.
> 
> Hi!
> 
> Any updates on this?
> 
> I might have missed the patch if there was one.
> 
> Thanks!

That has should be fixed via lockdep annotation with
581319c58600b54612c417aff32ae9bbd79f4cdb

Paolo

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: net: possible deadlock in skb_queue_tail

[Andrey Konovalov]

On Fri, Feb 24, 2017 at 3:56 AM, Florian Westphal  wrote:
> Andrey Konovalov  wrote:
>
> [ CC Paolo ]
>
>> I've got the following error report while fuzzing the kernel with syzkaller.
>>
>> On commit c470abd4fde40ea6a0846a2beab642a578c0b8cd (4.10).
>>
>> Unfortunately I can't reproduce it.
>
> This needs NETLINK_BROADCAST_ERROR enabled on a netlink socket
> that then subscribes to netfilter conntrack (ctnetlink) events.
> probably syzkaller did this by accident -- impressive.
>
> (one task is the ctnetlink event redelivery worker
>  which won't be scheduled otherwise).
>
>> ==
>> [ INFO: possible circular locking dependency detected ]
>> 4.10.0-rc8+ #201 Not tainted
>> ---
>> kworker/0:2/1404 is trying to acquire lock:
>>  (&(>lock)->rlock#3){+.-...}, at: []
>> skb_queue_tail+0xcf/0x2f0 net/core/skbuff.c:2478
>>
>> but task is already holding lock:
>>  (&(>lock)->rlock){+.-...}, at: [] spin_lock
>> include/linux/spinlock.h:302 [inline]
>>  (&(>lock)->rlock){+.-...}, at: []
>> ecache_work_evict_list+0xaf/0x590
>> net/netfilter/nf_conntrack_ecache.c:48
>>
>> which lock already depends on the new lock.
>
> Cong is correct, this is a false positive.
>
> However we should fix this splat.
>
> Paolo, this happens since 7c13f97ffde63cc792c49ec1513f3974f2f05229
> ('udp: do fwd memory scheduling on dequeue'), before this
> commit kfree_skb() was invoked outside of the locked section in
> first_packet_length().
>
> cpu 0 call chain:
> - first_packet_length (hold udp sk_receive_queue lock)
>- kfree_skb
>   - nf_conntrack_destroy
>  - spin_lock(net->ct.pcpu->lock)
>
> cpu 1 call chain:
> - ecache_work_evict_list
>   - spin_lock( net->ct.pcpu->lock)
>   - nf_conntrack_event
>  - aquire netlink socket sk_receive_queue
>
> So this could only ever deadlock if a netlink socket
> calls kfree_skb while holding its sk_receive_queue lock, but afaics
> this is never the case.
>
> There are two ways to avoid this splat (other than lockdep annotation):
>
> 1. re-add the list to first_packet_length() and free the
> skbs outside of locked section.
>
> 2. change ecache_work_evict_list to not call nf_conntrack_event()
> while holding the pcpu lock.
>
> doing #2 might be a good idea anyway to avoid potential deadlock
> when kfree_skb gets invoked while other cpu holds its sk_receive_queue
> lock, I'll have a look if this is feasible.

Hi!

Any updates on this?

I might have missed the patch if there was one.

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft PATCH] List handles of added rules if requested

[Florian Westphal]

Pablo Neira Ayuso  wrote:
> On Thu, May 04, 2017 at 02:34:21PM +0200, Phil Sutter wrote:
> > Being able to retrieve an added rule's handle atomically is a crucial
> > feature for scripts invoking nft command: Without it, there is no way to
> > be sure a handle extracted from 'nft list ruleset' command actually
> > refers to the rule one has added before or that of another process which
> > ran in between.
> > 
> > Extracting an added rule's handle itself is not an easy task already,
> > since there is a chance that a given rule is printed differently than
> > when it was added before. A simple example is port number vs. service
> > name:
> > 
> > | nft add rule ip t c tcp dport { ssh, 80 } accept
> > 
> > There is no way to make 'nft list ruleset' return the rule just like
> > this as depending on whether '-nn' was given or not, it either prints
> > the set as '{ ssh, http }' or '{ 22, 80 }' but never in the mixed form
> > that was used when adding it.
> > 
> > This patch prints an identifying string for each added rule which may be
> > used as single parameter to a later 'nft delete rule' command. So a
> > simple scripting example looks like this:
> > 
> > | handle=$(nft add rule ip t c counter)
> 
> This is a hack.
> 
> We should follow the rule description path.

You mean delete-by-name?

Its just as ugly, just a different kind of ugly.

Will you delete the first match?  The last one?  All of them?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft PATCH] List handles of added rules if requested

[Pablo Neira Ayuso]

On Thu, May 04, 2017 at 02:34:21PM +0200, Phil Sutter wrote:
> Being able to retrieve an added rule's handle atomically is a crucial
> feature for scripts invoking nft command: Without it, there is no way to
> be sure a handle extracted from 'nft list ruleset' command actually
> refers to the rule one has added before or that of another process which
> ran in between.
> 
> Extracting an added rule's handle itself is not an easy task already,
> since there is a chance that a given rule is printed differently than
> when it was added before. A simple example is port number vs. service
> name:
> 
> | nft add rule ip t c tcp dport { ssh, 80 } accept
> 
> There is no way to make 'nft list ruleset' return the rule just like
> this as depending on whether '-nn' was given or not, it either prints
> the set as '{ ssh, http }' or '{ 22, 80 }' but never in the mixed form
> that was used when adding it.
> 
> This patch prints an identifying string for each added rule which may be
> used as single parameter to a later 'nft delete rule' command. So a
> simple scripting example looks like this:
> 
> | handle=$(nft add rule ip t c counter)

This is a hack.

We should follow the rule description path.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nf-next PATCH] netfilter: nf_tables: Return info of added rules back to user space

[Pablo Neira Ayuso]

On Thu, May 04, 2017 at 02:34:17PM +0200, Phil Sutter wrote:
> This allows user space to reliably match kernel generated handles with
> added rules for reference.
> 
> Signed-off-by: Phil Sutter 
> ---
>  net/netfilter/nf_tables_api.c | 19 ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 1c6482d2c4dcf..71bce5d024409 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -2142,6 +2142,7 @@ static int nf_tables_newrule(struct net *net, struct 
> sock *nlsk,
>   struct nft_userdata *udata;
>   struct nft_trans *trans = NULL;
>   struct nft_expr *expr;
> + struct sk_buff *skb2;
>   struct nft_ctx ctx;
>   struct nlattr *tmp;
>   unsigned int size, i, n, ulen = 0, usize = 0;
> @@ -2281,8 +2282,24 @@ static int nf_tables_newrule(struct net *net, struct 
> sock *nlsk,
>   goto err3;
>   }
>   chain->use++;
> - return 0;
>  
> + skb2 = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
> + if (!skb2) {
> + err = -ENOMEM;
> + goto err4;
> + }
> + err = nf_tables_fill_rule_info(skb2, net, NETLINK_CB(skb).portid,
> +nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0,
> +nfmsg->nfgen_family, table, chain, rule);
> + if (err < 0)
> + goto err5;
> +
> + return nlmsg_unicast(nlsk, skb2, NETLINK_CB(skb).portid);

You can achieve this already via NLM_F_ECHO.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[nft PATCH] List handles of added rules if requested

[Phil Sutter]

Being able to retrieve an added rule's handle atomically is a crucial
feature for scripts invoking nft command: Without it, there is no way to
be sure a handle extracted from 'nft list ruleset' command actually
refers to the rule one has added before or that of another process which
ran in between.

Extracting an added rule's handle itself is not an easy task already,
since there is a chance that a given rule is printed differently than
when it was added before. A simple example is port number vs. service
name:

| nft add rule ip t c tcp dport { ssh, 80 } accept

There is no way to make 'nft list ruleset' return the rule just like
this as depending on whether '-nn' was given or not, it either prints
the set as '{ ssh, http }' or '{ 22, 80 }' but never in the mixed form
that was used when adding it.

This patch prints an identifying string for each added rule which may be
used as single parameter to a later 'nft delete rule' command. So a
simple scripting example looks like this:

| handle=$(nft add rule ip t c counter)
| ...
| nft delete rule $handle

Signed-off-by: Phil Sutter 
---
 src/mnl.c | 30 +-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/mnl.c b/src/mnl.c
index 295dd84a58406..d5b261aff0aee 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -26,9 +26,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
 
 static int seq;
 
@@ -242,6 +244,31 @@ static ssize_t mnl_nft_socket_sendmsg(const struct 
mnl_socket *nl)
return sendmsg(mnl_socket_get_fd(nl), , 0);
 }
 
+static int mnl_callback(const struct nlmsghdr *nlh, void *data)
+{
+   struct nftnl_rule *nlr;
+
+   if (!handle_output)
+   return MNL_CB_OK;
+
+   nlr = nftnl_rule_alloc();
+   if (!nlr)
+   memory_allocation_error();
+
+   if (nftnl_rule_nlmsg_parse(nlh, nlr) < 0)
+   goto out_free;
+
+   printf("%s %s %s handle %" PRIu64 "\n",
+  family2str(nftnl_rule_get_u32(nlr, NFTNL_RULE_FAMILY)),
+  nftnl_rule_get_str(nlr, NFTNL_RULE_TABLE),
+  nftnl_rule_get_str(nlr, NFTNL_RULE_CHAIN),
+  nftnl_rule_get_u64(nlr, NFTNL_RULE_HANDLE));
+
+out_free:
+   nftnl_rule_free(nlr);
+   return MNL_CB_OK;
+}
+
 int mnl_batch_talk(struct mnl_socket *nl, struct list_head *err_list)
 {
int ret, fd = mnl_socket_get_fd(nl), portid = mnl_socket_get_portid(nl);
@@ -271,11 +298,12 @@ int mnl_batch_talk(struct mnl_socket *nl, struct 
list_head *err_list)
if (ret == -1)
return -1;
 
-   ret = mnl_cb_run(rcv_buf, ret, 0, portid, NULL, NULL);
+   ret = mnl_cb_run(rcv_buf, ret, 0, portid, _callback, NULL);
/* Continue on error, make sure we get all acknowledgments */
if (ret == -1)
mnl_err_list_node_add(err_list, errno, nlh->nlmsg_seq);
 
+
ret = select(fd+1, , NULL, NULL, );
if (ret == -1)
return -1;
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[nf-next PATCH] netfilter: nf_tables: Return info of added rules back to user space

[Phil Sutter]

This allows user space to reliably match kernel generated handles with
added rules for reference.

Signed-off-by: Phil Sutter 
---
 net/netfilter/nf_tables_api.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 1c6482d2c4dcf..71bce5d024409 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2142,6 +2142,7 @@ static int nf_tables_newrule(struct net *net, struct sock 
*nlsk,
struct nft_userdata *udata;
struct nft_trans *trans = NULL;
struct nft_expr *expr;
+   struct sk_buff *skb2;
struct nft_ctx ctx;
struct nlattr *tmp;
unsigned int size, i, n, ulen = 0, usize = 0;
@@ -2281,8 +2282,24 @@ static int nf_tables_newrule(struct net *net, struct 
sock *nlsk,
goto err3;
}
chain->use++;
-   return 0;
 
+   skb2 = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+   if (!skb2) {
+   err = -ENOMEM;
+   goto err4;
+   }
+   err = nf_tables_fill_rule_info(skb2, net, NETLINK_CB(skb).portid,
+  nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0,
+  nfmsg->nfgen_family, table, chain, rule);
+   if (err < 0)
+   goto err5;
+
+   return nlmsg_unicast(nlsk, skb2, NETLINK_CB(skb).portid);
+
+err5:
+   kfree_skb(skb2);
+err4:
+   chain->use--;
 err3:
list_del_rcu(>list);
 err2:
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft PATCH] nft.8: Enhance NAT documentation

[Pablo Neira Ayuso]

On Tue, May 02, 2017 at 07:51:27PM +0200, Phil Sutter wrote:
> This adds documentation about masquerade and redirect statements, points
> out that for any NAT statement both prerouting and postrouting chains
> are required and adds a bunch of examples to the section's end.

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft PATCH] nft.8: Enhance NAT documentation

[Arturo Borrero Gonzalez]

On 2 May 2017 at 19:51, Phil Sutter  wrote:
> This adds documentation about masquerade and redirect statements, points
> out that for any NAT statement both prerouting and postrouting chains
> are required and adds a bunch of examples to the section's end.
>
> Signed-off-by: Phil Sutter 
> ---
>  doc/nft.xml | 58 +-
>  1 file changed, 57 insertions(+), 1 deletion(-)
>

Thanks Phil, more docs are always good.

Acked-by: Arturo Borrero Gonzalez 
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html