* [PATCH nf-next] netfilter: connlabels: Export setting connlabel length
@ 2015-07-31 23:50 Joe Stringer
2015-08-01 0:40 ` Florian Westphal
0 siblings, 1 reply; 3+ messages in thread
From: Joe Stringer @ 2015-07-31 23:50 UTC (permalink / raw)
To: pablo; +Cc: fw, netfilter-devel
Add functions to change connlabel length into nf_conntrack_labels.c so
they may be reused by other modules like OVS and nftables without
needing to jump through xt_match_check() hoops.
Suggested-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
include/net/netfilter/nf_conntrack_labels.h | 4 ++++
net/netfilter/nf_conntrack_labels.c | 24 ++++++++++++++++++++++++
net/netfilter/xt_connlabel.c | 15 ++-------------
3 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_labels.h b/include/net/netfilter/nf_conntrack_labels.h
index dec6336..7e2b1d0 100644
--- a/include/net/netfilter/nf_conntrack_labels.h
+++ b/include/net/netfilter/nf_conntrack_labels.h
@@ -54,7 +54,11 @@ int nf_connlabels_replace(struct nf_conn *ct,
#ifdef CONFIG_NF_CONNTRACK_LABELS
int nf_conntrack_labels_init(void);
void nf_conntrack_labels_fini(void);
+int nf_connlabels_get(struct net *net, unsigned int n_bits);
+void nf_connlabels_put(struct net *net);
#else
static inline int nf_conntrack_labels_init(void) { return 0; }
static inline void nf_conntrack_labels_fini(void) {}
+static inline int nf_connlabels_get(struct net *net, unsigned int n_bits) { return 0; }
+static inline void nf_connlabels_put(struct net *net) {}
#endif
diff --git a/net/netfilter/nf_conntrack_labels.c b/net/netfilter/nf_conntrack_labels.c
index bb53f12..00df4e71 100644
--- a/net/netfilter/nf_conntrack_labels.c
+++ b/net/netfilter/nf_conntrack_labels.c
@@ -91,6 +91,30 @@ int nf_connlabels_replace(struct nf_conn *ct,
EXPORT_SYMBOL_GPL(nf_connlabels_replace);
#endif
+int nf_connlabels_get(struct net *net, unsigned int n_bits)
+{
+ size_t words;
+
+ if (n_bits > XT_CONNLABEL_MAXBIT + 1)
+ return -ERANGE;
+
+ net->ct.labels_used++;
+ words = BITS_TO_LONGS(n_bits);
+ if (words > net->ct.label_words)
+ net->ct.label_words = words;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(nf_connlabels_get);
+
+void nf_connlabels_put(struct net *net)
+{
+ net->ct.labels_used--;
+ if (net->ct.labels_used == 0)
+ net->ct.label_words = 0;
+}
+EXPORT_SYMBOL_GPL(nf_connlabels_put);
+
static struct nf_ct_ext_type labels_extend __read_mostly = {
.len = sizeof(struct nf_conn_labels),
.align = __alignof__(struct nf_conn_labels),
diff --git a/net/netfilter/xt_connlabel.c b/net/netfilter/xt_connlabel.c
index 9f8719d..f8bb936 100644
--- a/net/netfilter/xt_connlabel.c
+++ b/net/netfilter/xt_connlabel.c
@@ -42,10 +42,6 @@ static int connlabel_mt_check(const struct xt_mtchk_param *par)
XT_CONNLABEL_OP_SET;
struct xt_connlabel_mtinfo *info = par->matchinfo;
int ret;
- size_t words;
-
- if (info->bit > XT_CONNLABEL_MAXBIT)
- return -ERANGE;
if (info->options & ~options) {
pr_err("Unknown options in mask %x\n", info->options);
@@ -59,19 +55,12 @@ static int connlabel_mt_check(const struct xt_mtchk_param *par)
return ret;
}
- par->net->ct.labels_used++;
- words = BITS_TO_LONGS(info->bit+1);
- if (words > par->net->ct.label_words)
- par->net->ct.label_words = words;
-
- return ret;
+ return nf_connlabels_get(par->net, info->bit + 1);
}
static void connlabel_mt_destroy(const struct xt_mtdtor_param *par)
{
- par->net->ct.labels_used--;
- if (par->net->ct.labels_used == 0)
- par->net->ct.label_words = 0;
+ nf_connlabels_put(par->net);
nf_ct_l3proto_module_put(par->family);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH nf-next] netfilter: connlabels: Export setting connlabel length
2015-07-31 23:50 [PATCH nf-next] netfilter: connlabels: Export setting connlabel length Joe Stringer
@ 2015-08-01 0:40 ` Florian Westphal
2015-08-03 17:58 ` Joe Stringer
0 siblings, 1 reply; 3+ messages in thread
From: Florian Westphal @ 2015-08-01 0:40 UTC (permalink / raw)
To: Joe Stringer; +Cc: pablo, fw, netfilter-devel
Joe Stringer <joestringer@nicira.com> wrote:
> diff --git a/net/netfilter/nf_conntrack_labels.c b/net/netfilter/nf_conntrack_labels.c
> index bb53f12..00df4e71 100644
> --- a/net/netfilter/nf_conntrack_labels.c
> +++ b/net/netfilter/nf_conntrack_labels.c
> @@ -91,6 +91,30 @@ int nf_connlabels_replace(struct nf_conn *ct,
> EXPORT_SYMBOL_GPL(nf_connlabels_replace);
> #endif
>
> +int nf_connlabels_get(struct net *net, unsigned int n_bits)
> +{
> + size_t words;
> +
> + if (n_bits > XT_CONNLABEL_MAXBIT + 1)
Perhaps use
if (n_bits >= (NF_CT_LABELS_MAX_SIZE * BITS_PER_BYTE)))
To avoid the XT_CONNLABEL_MAXBIT in the nf_* part.
> + return -ERANGE;
> +
> + net->ct.labels_used++;
> + words = BITS_TO_LONGS(n_bits);
> + if (words > net->ct.label_words)
> + net->ct.label_words = words;
> +
> + return 0;
> +}
I think we should add a lock here, currently this is protected by the
xtables mutex -- I'd suggest to just add a spinlock for this.
> return ret;
> }
>
> - par->net->ct.labels_used++;
> - words = BITS_TO_LONGS(info->bit+1);
> - if (words > par->net->ct.label_words)
> - par->net->ct.label_words = words;
> -
> - return ret;
> + return nf_connlabels_get(par->net, info->bit + 1);
This can leak the refcnt on l3_proto_module we obtained earlier.
Other than that, this looks good.
Thanks,
Florian
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH nf-next] netfilter: connlabels: Export setting connlabel length
2015-08-01 0:40 ` Florian Westphal
@ 2015-08-03 17:58 ` Joe Stringer
0 siblings, 0 replies; 3+ messages in thread
From: Joe Stringer @ 2015-08-03 17:58 UTC (permalink / raw)
To: Florian Westphal; +Cc: Pablo Neira Ayuso, netfilter-devel
On 31 July 2015 at 17:40, Florian Westphal <fw@strlen.de> wrote:
> Joe Stringer <joestringer@nicira.com> wrote:
>> diff --git a/net/netfilter/nf_conntrack_labels.c b/net/netfilter/nf_conntrack_labels.c
>> index bb53f12..00df4e71 100644
>> --- a/net/netfilter/nf_conntrack_labels.c
>> +++ b/net/netfilter/nf_conntrack_labels.c
>> @@ -91,6 +91,30 @@ int nf_connlabels_replace(struct nf_conn *ct,
>> EXPORT_SYMBOL_GPL(nf_connlabels_replace);
>> #endif
>>
>> +int nf_connlabels_get(struct net *net, unsigned int n_bits)
>> +{
>> + size_t words;
>> +
>> + if (n_bits > XT_CONNLABEL_MAXBIT + 1)
>
> Perhaps use
>
> if (n_bits >= (NF_CT_LABELS_MAX_SIZE * BITS_PER_BYTE)))
>
> To avoid the XT_CONNLABEL_MAXBIT in the nf_* part.
>
>> + return -ERANGE;
>> +
>> + net->ct.labels_used++;
>> + words = BITS_TO_LONGS(n_bits);
>> + if (words > net->ct.label_words)
>> + net->ct.label_words = words;
>> +
>> + return 0;
>> +}
>
> I think we should add a lock here, currently this is protected by the
> xtables mutex -- I'd suggest to just add a spinlock for this.
>
>> return ret;
>> }
>>
>> - par->net->ct.labels_used++;
>> - words = BITS_TO_LONGS(info->bit+1);
>> - if (words > par->net->ct.label_words)
>> - par->net->ct.label_words = words;
>> -
>> - return ret;
>> + return nf_connlabels_get(par->net, info->bit + 1);
>
> This can leak the refcnt on l3_proto_module we obtained earlier.
>
> Other than that, this looks good.
>
> Thanks,
> Florian
Thanks for the review, I'll fix these up and send a v2 soon.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-08-03 17:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-31 23:50 [PATCH nf-next] netfilter: connlabels: Export setting connlabel length Joe Stringer
2015-08-01 0:40 ` Florian Westphal
2015-08-03 17:58 ` Joe Stringer
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).