* net: portid signedness and format string fixes
@ 2015-03-13 11:31 Richard Weinberger
2015-03-13 11:31 ` [PATCH 1/4] netlink: Fix portid type in netlink_notify Richard Weinberger
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Richard Weinberger @ 2015-03-13 11:31 UTC (permalink / raw)
To: netdev
Cc: linux-wireless, coreteam, netfilter-devel, linux-kernel, sameo,
aloisio.almeida, lauro.venancio, davem, kadlec, kaber, pablo
My application blow up as values in nfnetlink_queue suddenly turned
negative.
i.e.
$ cat /proc/net/netfilter/nfnetlink_queue
0 29508 278 2 65531 0 2004213241 -2129885586 1
1 -27747 0 2 65531 0 0 0 1
2 -27748 0 2 65531 0 0 0 1
This series fixes that. Patches 1 to 3 change users of signed
portid to unsigned integers. Just for the sake of consistency.
Patch 4 fixes the printf format string such that applications
don't have to deal with negative numbers.
[PATCH 1/4] netlink: Fix portid type in netlink_notify
[PATCH 2/4] nfc: Fix portid type in urelease_work
[PATCH 3/4] netfilter: Fix portid types
[PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file
Thanks,
//richard
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] netlink: Fix portid type in netlink_notify
2015-03-13 11:31 net: portid signedness and format string fixes Richard Weinberger
@ 2015-03-13 11:31 ` Richard Weinberger
2015-03-13 11:31 ` [PATCH 2/4] nfc: Fix portid type in urelease_work Richard Weinberger
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Richard Weinberger @ 2015-03-13 11:31 UTC (permalink / raw)
To: netdev
Cc: linux-wireless, coreteam, netfilter-devel, linux-kernel, sameo,
aloisio.almeida, lauro.venancio, davem, kadlec, kaber, pablo,
Richard Weinberger
portid is an unsigned integer. Fix netlink_notify to
match all other portid user in the kernel.
Signed-off-by: Richard Weinberger <richard@nod.at>
---
include/linux/netlink.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 02fc86d..6a7e43e 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -134,7 +134,7 @@ struct netlink_callback {
struct netlink_notify {
struct net *net;
- int portid;
+ unsigned int portid;
int protocol;
};
--
1.8.4.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] nfc: Fix portid type in urelease_work
2015-03-13 11:31 net: portid signedness and format string fixes Richard Weinberger
2015-03-13 11:31 ` [PATCH 1/4] netlink: Fix portid type in netlink_notify Richard Weinberger
@ 2015-03-13 11:31 ` Richard Weinberger
2015-03-13 11:31 ` [PATCH 3/4] netfilter: Fix portid types Richard Weinberger
2015-03-13 11:31 ` [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file Richard Weinberger
3 siblings, 0 replies; 14+ messages in thread
From: Richard Weinberger @ 2015-03-13 11:31 UTC (permalink / raw)
To: netdev
Cc: linux-wireless, coreteam, netfilter-devel, linux-kernel, sameo,
aloisio.almeida, lauro.venancio, davem, kadlec, kaber, pablo,
Richard Weinberger
portid is an unsigned integer. Fix urelease_work to
match all other portid user in the kernel.
Signed-off-by: Richard Weinberger <richard@nod.at>
---
net/nfc/netlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/nfc/netlink.c b/net/nfc/netlink.c
index 14a2d11..1f12c4c 100644
--- a/net/nfc/netlink.c
+++ b/net/nfc/netlink.c
@@ -1583,8 +1583,8 @@ static const struct genl_ops nfc_genl_ops[] = {
struct urelease_work {
- struct work_struct w;
- int portid;
+ struct work_struct w;
+ unsigned int portid;
};
static void nfc_urelease_event_work(struct work_struct *work)
--
1.8.4.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/4] netfilter: Fix portid types
2015-03-13 11:31 net: portid signedness and format string fixes Richard Weinberger
2015-03-13 11:31 ` [PATCH 1/4] netlink: Fix portid type in netlink_notify Richard Weinberger
2015-03-13 11:31 ` [PATCH 2/4] nfc: Fix portid type in urelease_work Richard Weinberger
@ 2015-03-13 11:31 ` Richard Weinberger
2015-03-13 13:01 ` Pablo Neira Ayuso
2015-03-13 11:31 ` [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file Richard Weinberger
3 siblings, 1 reply; 14+ messages in thread
From: Richard Weinberger @ 2015-03-13 11:31 UTC (permalink / raw)
To: netdev
Cc: linux-wireless, coreteam, netfilter-devel, linux-kernel, sameo,
aloisio.almeida, lauro.venancio, davem, kadlec, kaber, pablo,
Richard Weinberger
The netlink portid is an unsigned integer, use this type
also in netfilter.
Signed-off-by: Richard Weinberger <richard@nod.at>
---
net/netfilter/nfnetlink_log.c | 4 ++--
net/netfilter/nfnetlink_queue_core.c | 7 ++++---
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 11d85b3..7f00846 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -62,7 +62,7 @@ struct nfulnl_instance {
struct timer_list timer;
struct net *net;
struct user_namespace *peer_user_ns; /* User namespace of the peer process */
- int peer_portid; /* PORTID of the peer process */
+ unsigned int peer_portid; /* PORTID of the peer process */
/* configurable parameters */
unsigned int flushtimeout; /* timeout until queue flush */
@@ -151,7 +151,7 @@ static void nfulnl_timer(unsigned long data);
static struct nfulnl_instance *
instance_create(struct net *net, u_int16_t group_num,
- int portid, struct user_namespace *user_ns)
+ unsigned int portid, struct user_namespace *user_ns)
{
struct nfulnl_instance *inst;
struct nfnl_log_net *log = nfnl_log_pernet(net);
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 0db8515..1b7f7b8 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -54,7 +54,7 @@ struct nfqnl_instance {
struct hlist_node hlist; /* global list of queues */
struct rcu_head rcu;
- int peer_portid;
+ unsigned int peer_portid;
unsigned int queue_maxlen;
unsigned int copy_range;
unsigned int queue_dropped;
@@ -110,7 +110,7 @@ instance_lookup(struct nfnl_queue_net *q, u_int16_t queue_num)
static struct nfqnl_instance *
instance_create(struct nfnl_queue_net *q, u_int16_t queue_num,
- int portid)
+ unsigned int portid)
{
struct nfqnl_instance *inst;
unsigned int h;
@@ -860,7 +860,8 @@ static const struct nla_policy nfqa_verdict_batch_policy[NFQA_MAX+1] = {
};
static struct nfqnl_instance *
-verdict_instance_lookup(struct nfnl_queue_net *q, u16 queue_num, int nlportid)
+verdict_instance_lookup(struct nfnl_queue_net *q, u16 queue_num,
+ unsigned int nlportid)
{
struct nfqnl_instance *queue;
--
1.8.4.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file
2015-03-13 11:31 net: portid signedness and format string fixes Richard Weinberger
` (2 preceding siblings ...)
2015-03-13 11:31 ` [PATCH 3/4] netfilter: Fix portid types Richard Weinberger
@ 2015-03-13 11:31 ` Richard Weinberger
2015-03-13 12:15 ` Pablo Neira Ayuso
3 siblings, 1 reply; 14+ messages in thread
From: Richard Weinberger @ 2015-03-13 11:31 UTC (permalink / raw)
To: netdev
Cc: linux-wireless, coreteam, netfilter-devel, linux-kernel, sameo,
aloisio.almeida, lauro.venancio, davem, kadlec, kaber, pablo,
Richard Weinberger
The printed values are all of type unsigned integer, therefore use
%u instead of %d. Otherwise an user can face negative values.
Fixes:
$ cat /proc/net/netfilter/nfnetlink_queue
0 29508 278 2 65531 0 2004213241 -2129885586 1
1 -27747 0 2 65531 0 0 0 1
2 -27748 0 2 65531 0 0 0 1
Signed-off-by: Richard Weinberger <richard@nod.at>
---
net/netfilter/nfnetlink_queue_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 1b7f7b8..2121ab5 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -1243,7 +1243,7 @@ static int seq_show(struct seq_file *s, void *v)
{
const struct nfqnl_instance *inst = v;
- seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
+ seq_printf(s, "%5u %6u %5u %1u %5u %5u %5u %8u %2d\n",
inst->queue_num,
inst->peer_portid, inst->queue_total,
inst->copy_mode, inst->copy_range,
--
1.8.4.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file
2015-03-13 11:31 ` [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file Richard Weinberger
@ 2015-03-13 12:15 ` Pablo Neira Ayuso
2015-03-13 13:43 ` Richard Weinberger
0 siblings, 1 reply; 14+ messages in thread
From: Pablo Neira Ayuso @ 2015-03-13 12:15 UTC (permalink / raw)
To: Richard Weinberger
Cc: netdev, linux-wireless, coreteam, netfilter-devel, linux-kernel,
sameo, aloisio.almeida, lauro.venancio, davem, kadlec, kaber
On Fri, Mar 13, 2015 at 12:31:16PM +0100, Richard Weinberger wrote:
> The printed values are all of type unsigned integer, therefore use
> %u instead of %d. Otherwise an user can face negative values.
>
> Fixes:
> $ cat /proc/net/netfilter/nfnetlink_queue
> 0 29508 278 2 65531 0 2004213241 -2129885586 1
> 1 -27747 0 2 65531 0 0 0 1
> 2 -27748 0 2 65531 0 0 0 1
I guess you want to access stats on dropped packets.
I prefer if you extend nfnetlink_queue to provide statistics through
nfnetlink_queue, so you don't have to manually parse this text-based
/proc entry and we can deprecate this interface. That shouldn't have
been there in first place.
Thanks.
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
> net/netfilter/nfnetlink_queue_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
> index 1b7f7b8..2121ab5 100644
> --- a/net/netfilter/nfnetlink_queue_core.c
> +++ b/net/netfilter/nfnetlink_queue_core.c
> @@ -1243,7 +1243,7 @@ static int seq_show(struct seq_file *s, void *v)
> {
> const struct nfqnl_instance *inst = v;
>
> - seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
> + seq_printf(s, "%5u %6u %5u %1u %5u %5u %5u %8u %2d\n",
> inst->queue_num,
> inst->peer_portid, inst->queue_total,
> inst->copy_mode, inst->copy_range,
> --
> 1.8.4.5
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] netfilter: Fix portid types
2015-03-13 11:31 ` [PATCH 3/4] netfilter: Fix portid types Richard Weinberger
@ 2015-03-13 13:01 ` Pablo Neira Ayuso
2015-03-13 13:19 ` Richard Weinberger
0 siblings, 1 reply; 14+ messages in thread
From: Pablo Neira Ayuso @ 2015-03-13 13:01 UTC (permalink / raw)
To: Richard Weinberger
Cc: netdev, linux-wireless, coreteam, netfilter-devel, linux-kernel,
sameo, aloisio.almeida, lauro.venancio, davem, kadlec, kaber
On Fri, Mar 13, 2015 at 12:31:15PM +0100, Richard Weinberger wrote:
> The netlink portid is an unsigned integer, use this type
> also in netfilter.
This small cleanup I can still take it but...
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
> net/netfilter/nfnetlink_log.c | 4 ++--
> net/netfilter/nfnetlink_queue_core.c | 7 ++++---
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
> index 11d85b3..7f00846 100644
> --- a/net/netfilter/nfnetlink_log.c
> +++ b/net/netfilter/nfnetlink_log.c
> @@ -62,7 +62,7 @@ struct nfulnl_instance {
> struct timer_list timer;
> struct net *net;
> struct user_namespace *peer_user_ns; /* User namespace of the peer process */
> - int peer_portid; /* PORTID of the peer process */
> + unsigned int peer_portid; /* PORTID of the peer process */
I think you have to use u32 for consistency. Other spots use the
datatype for the netlink portid.
I think same thing applies to other patches.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] netfilter: Fix portid types
2015-03-13 13:01 ` Pablo Neira Ayuso
@ 2015-03-13 13:19 ` Richard Weinberger
0 siblings, 0 replies; 14+ messages in thread
From: Richard Weinberger @ 2015-03-13 13:19 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, linux-wireless, coreteam, netfilter-devel, linux-kernel,
sameo, aloisio.almeida, lauro.venancio, davem, kadlec, kaber
Am 13.03.2015 um 14:01 schrieb Pablo Neira Ayuso:
> On Fri, Mar 13, 2015 at 12:31:15PM +0100, Richard Weinberger wrote:
>> The netlink portid is an unsigned integer, use this type
>> also in netfilter.
>
> This small cleanup I can still take it but...
>
>> Signed-off-by: Richard Weinberger <richard@nod.at>
>> ---
>> net/netfilter/nfnetlink_log.c | 4 ++--
>> net/netfilter/nfnetlink_queue_core.c | 7 ++++---
>> 2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
>> index 11d85b3..7f00846 100644
>> --- a/net/netfilter/nfnetlink_log.c
>> +++ b/net/netfilter/nfnetlink_log.c
>> @@ -62,7 +62,7 @@ struct nfulnl_instance {
>> struct timer_list timer;
>> struct net *net;
>> struct user_namespace *peer_user_ns; /* User namespace of the peer process */
>> - int peer_portid; /* PORTID of the peer process */
>> + unsigned int peer_portid; /* PORTID of the peer process */
>
> I think you have to use u32 for consistency. Other spots use the
> datatype for the netlink portid.
>
> I think same thing applies to other patches.
Of course! Will re-spin with u32.
Thanks,
//richard
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file
2015-03-13 12:15 ` Pablo Neira Ayuso
@ 2015-03-13 13:43 ` Richard Weinberger
2015-03-13 13:53 ` Pablo Neira Ayuso
0 siblings, 1 reply; 14+ messages in thread
From: Richard Weinberger @ 2015-03-13 13:43 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, linux-wireless, coreteam, netfilter-devel, linux-kernel,
sameo, aloisio.almeida, lauro.venancio, davem, kadlec, kaber
Am 13.03.2015 um 13:15 schrieb Pablo Neira Ayuso:
> On Fri, Mar 13, 2015 at 12:31:16PM +0100, Richard Weinberger wrote:
>> The printed values are all of type unsigned integer, therefore use
>> %u instead of %d. Otherwise an user can face negative values.
>>
>> Fixes:
>> $ cat /proc/net/netfilter/nfnetlink_queue
>> 0 29508 278 2 65531 0 2004213241 -2129885586 1
>> 1 -27747 0 2 65531 0 0 0 1
>> 2 -27748 0 2 65531 0 0 0 1
>
> I guess you want to access stats on dropped packets.
Correct. :)
> I prefer if you extend nfnetlink_queue to provide statistics through
> nfnetlink_queue, so you don't have to manually parse this text-based
> /proc entry and we can deprecate this interface. That shouldn't have
> been there in first place.
You mean statistics via netlink attributes? I can add that!
But I think we should also fix the format string of the proc file
as the fix is easy and non-intrusive.
Thanks,
//richard
> Thanks.
>
>> Signed-off-by: Richard Weinberger <richard@nod.at>
>> ---
>> net/netfilter/nfnetlink_queue_core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
>> index 1b7f7b8..2121ab5 100644
>> --- a/net/netfilter/nfnetlink_queue_core.c
>> +++ b/net/netfilter/nfnetlink_queue_core.c
>> @@ -1243,7 +1243,7 @@ static int seq_show(struct seq_file *s, void *v)
>> {
>> const struct nfqnl_instance *inst = v;
>>
>> - seq_printf(s, "%5d %6d %5d %1d %5d %5d %5d %8d %2d\n",
>> + seq_printf(s, "%5u %6u %5u %1u %5u %5u %5u %8u %2d\n",
>> inst->queue_num,
>> inst->peer_portid, inst->queue_total,
>> inst->copy_mode, inst->copy_range,
>> --
>> 1.8.4.5
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file
2015-03-13 13:43 ` Richard Weinberger
@ 2015-03-13 13:53 ` Pablo Neira Ayuso
2015-03-13 14:22 ` Richard Weinberger
0 siblings, 1 reply; 14+ messages in thread
From: Pablo Neira Ayuso @ 2015-03-13 13:53 UTC (permalink / raw)
To: Richard Weinberger
Cc: netdev, linux-wireless, coreteam, netfilter-devel, linux-kernel,
sameo, aloisio.almeida, lauro.venancio, davem, kadlec, kaber
On Fri, Mar 13, 2015 at 02:43:54PM +0100, Richard Weinberger wrote:
> Am 13.03.2015 um 13:15 schrieb Pablo Neira Ayuso:
> > On Fri, Mar 13, 2015 at 12:31:16PM +0100, Richard Weinberger wrote:
> >> The printed values are all of type unsigned integer, therefore use
> >> %u instead of %d. Otherwise an user can face negative values.
> >>
> >> Fixes:
> >> $ cat /proc/net/netfilter/nfnetlink_queue
> >> 0 29508 278 2 65531 0 2004213241 -2129885586 1
> >> 1 -27747 0 2 65531 0 0 0 1
> >> 2 -27748 0 2 65531 0 0 0 1
> >
> > I guess you want to access stats on dropped packets.
>
> Correct. :)
>
> > I prefer if you extend nfnetlink_queue to provide statistics through
> > nfnetlink_queue, so you don't have to manually parse this text-based
> > /proc entry and we can deprecate this interface. That shouldn't have
> > been there in first place.
>
> You mean statistics via netlink attributes? I can add that!
Add a new NFQNL_CFG_CMD_STATS command to request the statistics. If
NLM_F_DUMP is set, then we'll basically provide the full list of
instances. Otherwise, in case you want to retrieve stats for a
specific netlink socket, you can use the netlink portID as index.
And you'll have to add attributes for this new command, yes.
> But I think we should also fix the format string of the proc file
> as the fix is easy and non-intrusive.
Unfortunately we don't know how many people are relying on that
output, I prefer to remain conservative and provide a proper netlink
interface for this.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file
2015-03-13 13:53 ` Pablo Neira Ayuso
@ 2015-03-13 14:22 ` Richard Weinberger
2015-03-16 13:11 ` Pablo Neira Ayuso
0 siblings, 1 reply; 14+ messages in thread
From: Richard Weinberger @ 2015-03-13 14:22 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, linux-wireless, coreteam, netfilter-devel, linux-kernel,
sameo, aloisio.almeida, lauro.venancio, davem, kadlec, kaber
Am 13.03.2015 um 14:53 schrieb Pablo Neira Ayuso:
> On Fri, Mar 13, 2015 at 02:43:54PM +0100, Richard Weinberger wrote:
>> Am 13.03.2015 um 13:15 schrieb Pablo Neira Ayuso:
>>> On Fri, Mar 13, 2015 at 12:31:16PM +0100, Richard Weinberger wrote:
>>>> The printed values are all of type unsigned integer, therefore use
>>>> %u instead of %d. Otherwise an user can face negative values.
>>>>
>>>> Fixes:
>>>> $ cat /proc/net/netfilter/nfnetlink_queue
>>>> 0 29508 278 2 65531 0 2004213241 -2129885586 1
>>>> 1 -27747 0 2 65531 0 0 0 1
>>>> 2 -27748 0 2 65531 0 0 0 1
>>>
>>> I guess you want to access stats on dropped packets.
>>
>> Correct. :)
>>
>>> I prefer if you extend nfnetlink_queue to provide statistics through
>>> nfnetlink_queue, so you don't have to manually parse this text-based
>>> /proc entry and we can deprecate this interface. That shouldn't have
>>> been there in first place.
>>
>> You mean statistics via netlink attributes? I can add that!
>
> Add a new NFQNL_CFG_CMD_STATS command to request the statistics. If
> NLM_F_DUMP is set, then we'll basically provide the full list of
> instances. Otherwise, in case you want to retrieve stats for a
> specific netlink socket, you can use the netlink portID as index.
> And you'll have to add attributes for this new command, yes.
This was my plan. Thanks for the pointer!
>> But I think we should also fix the format string of the proc file
>> as the fix is easy and non-intrusive.
>
> Unfortunately we don't know how many people are relying on that
> output, I prefer to remain conservative and provide a proper netlink
> interface for this.
I understand your concerns but an application which is able to parse positive
and negative numbers can also parse pure positives.
Just made a small test application, glibc's %d in sscanf() can also deal with UINT_MAX.
And I don't expect that applications to check whether the returned values from
/proc/net/netfilter/nfnetlink_queue are between INT_MIN and INT_MAX.
That said, I'd have assumed that an user would report negative values as plain kernel bug.
Thanks,
//richard
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file
2015-03-13 14:22 ` Richard Weinberger
@ 2015-03-16 13:11 ` Pablo Neira Ayuso
2015-04-09 21:58 ` Richard Weinberger
0 siblings, 1 reply; 14+ messages in thread
From: Pablo Neira Ayuso @ 2015-03-16 13:11 UTC (permalink / raw)
To: Richard Weinberger
Cc: netdev, linux-wireless, coreteam, netfilter-devel, linux-kernel,
sameo, aloisio.almeida, lauro.venancio, davem, kadlec, kaber
On Fri, Mar 13, 2015 at 03:22:07PM +0100, Richard Weinberger wrote:
> Am 13.03.2015 um 14:53 schrieb Pablo Neira Ayuso:
> >> You mean statistics via netlink attributes? I can add that!
> >
> > Add a new NFQNL_CFG_CMD_STATS command to request the statistics. If
> > NLM_F_DUMP is set, then we'll basically provide the full list of
> > instances. Otherwise, in case you want to retrieve stats for a
> > specific netlink socket, you can use the netlink portID as index.
> > And you'll have to add attributes for this new command, yes.
>
> This was my plan. Thanks for the pointer!
It would be great if you can contribute this new interface.
> >> But I think we should also fix the format string of the proc file
> >> as the fix is easy and non-intrusive.
> >
> > Unfortunately we don't know how many people are relying on that
> > output, I prefer to remain conservative and provide a proper netlink
> > interface for this.
>
> I understand your concerns but an application which is able to parse positive
> and negative numbers can also parse pure positives.
> Just made a small test application, glibc's %d in sscanf() can also deal with UINT_MAX.
> And I don't expect that applications to check whether the returned values from
> /proc/net/netfilter/nfnetlink_queue are between INT_MIN and INT_MAX.
>
> That said, I'd have assumed that an user would report negative values as plain kernel bug.
Makes sense, please fix net/netfilter/nfnetlink_log.c too.
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file
2015-03-16 13:11 ` Pablo Neira Ayuso
@ 2015-04-09 21:58 ` Richard Weinberger
0 siblings, 0 replies; 14+ messages in thread
From: Richard Weinberger @ 2015-04-09 21:58 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, linux-wireless, coreteam, netfilter-devel, linux-kernel,
sameo, aloisio.almeida, lauro.venancio, davem, kadlec, kaber
Am 16.03.2015 um 14:11 schrieb Pablo Neira Ayuso:
> On Fri, Mar 13, 2015 at 03:22:07PM +0100, Richard Weinberger wrote:
>> Am 13.03.2015 um 14:53 schrieb Pablo Neira Ayuso:
>>>> You mean statistics via netlink attributes? I can add that!
>>>
>>> Add a new NFQNL_CFG_CMD_STATS command to request the statistics. If
>>> NLM_F_DUMP is set, then we'll basically provide the full list of
>>> instances. Otherwise, in case you want to retrieve stats for a
>>> specific netlink socket, you can use the netlink portID as index.
>>> And you'll have to add attributes for this new command, yes.
>>
>> This was my plan. Thanks for the pointer!
>
> It would be great if you can contribute this new interface.
FYI, it is still on my TODO.
I fear I won't find the time to do a patch for the upcoming merge window
and it has to wait for v4.2.
>>>> But I think we should also fix the format string of the proc file
>>>> as the fix is easy and non-intrusive.
>>>
>>> Unfortunately we don't know how many people are relying on that
>>> output, I prefer to remain conservative and provide a proper netlink
>>> interface for this.
>>
>> I understand your concerns but an application which is able to parse positive
>> and negative numbers can also parse pure positives.
>> Just made a small test application, glibc's %d in sscanf() can also deal with UINT_MAX.
>> And I don't expect that applications to check whether the returned values from
>> /proc/net/netfilter/nfnetlink_queue are between INT_MIN and INT_MAX.
>>
>> That said, I'd have assumed that an user would report negative values as plain kernel bug.
>
> Makes sense, please fix net/netfilter/nfnetlink_log.c too.
Patches sent! :)
Thanks,
//richard
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file
@ 2015-04-09 21:58 ` Richard Weinberger
0 siblings, 0 replies; 14+ messages in thread
From: Richard Weinberger @ 2015-04-09 21:58 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
coreteam-Cap9r6Oaw4JrovVCs/uTlw,
netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
sameo-VuQAYsv1563Yd54FQh9/CA,
aloisio.almeida-430g2QfJUUCGglJvpFV4uA,
lauro.venancio-430g2QfJUUCGglJvpFV4uA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
kadlec-K40Dz/62t/MgiyqX0sVFJYdd74u8MsAO,
kaber-dcUjhNyLwpNeoWH0uzbU5w
Am 16.03.2015 um 14:11 schrieb Pablo Neira Ayuso:
> On Fri, Mar 13, 2015 at 03:22:07PM +0100, Richard Weinberger wrote:
>> Am 13.03.2015 um 14:53 schrieb Pablo Neira Ayuso:
>>>> You mean statistics via netlink attributes? I can add that!
>>>
>>> Add a new NFQNL_CFG_CMD_STATS command to request the statistics. If
>>> NLM_F_DUMP is set, then we'll basically provide the full list of
>>> instances. Otherwise, in case you want to retrieve stats for a
>>> specific netlink socket, you can use the netlink portID as index.
>>> And you'll have to add attributes for this new command, yes.
>>
>> This was my plan. Thanks for the pointer!
>
> It would be great if you can contribute this new interface.
FYI, it is still on my TODO.
I fear I won't find the time to do a patch for the upcoming merge window
and it has to wait for v4.2.
>>>> But I think we should also fix the format string of the proc file
>>>> as the fix is easy and non-intrusive.
>>>
>>> Unfortunately we don't know how many people are relying on that
>>> output, I prefer to remain conservative and provide a proper netlink
>>> interface for this.
>>
>> I understand your concerns but an application which is able to parse positive
>> and negative numbers can also parse pure positives.
>> Just made a small test application, glibc's %d in sscanf() can also deal with UINT_MAX.
>> And I don't expect that applications to check whether the returned values from
>> /proc/net/netfilter/nfnetlink_queue are between INT_MIN and INT_MAX.
>>
>> That said, I'd have assumed that an user would report negative values as plain kernel bug.
>
> Makes sense, please fix net/netfilter/nfnetlink_log.c too.
Patches sent! :)
Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-04-09 21:58 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-13 11:31 net: portid signedness and format string fixes Richard Weinberger
2015-03-13 11:31 ` [PATCH 1/4] netlink: Fix portid type in netlink_notify Richard Weinberger
2015-03-13 11:31 ` [PATCH 2/4] nfc: Fix portid type in urelease_work Richard Weinberger
2015-03-13 11:31 ` [PATCH 3/4] netfilter: Fix portid types Richard Weinberger
2015-03-13 13:01 ` Pablo Neira Ayuso
2015-03-13 13:19 ` Richard Weinberger
2015-03-13 11:31 ` [PATCH 4/4] netfilter: Fix format string of nfnetlink_queue proc file Richard Weinberger
2015-03-13 12:15 ` Pablo Neira Ayuso
2015-03-13 13:43 ` Richard Weinberger
2015-03-13 13:53 ` Pablo Neira Ayuso
2015-03-13 14:22 ` Richard Weinberger
2015-03-16 13:11 ` Pablo Neira Ayuso
2015-04-09 21:58 ` Richard Weinberger
2015-04-09 21:58 ` Richard Weinberger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.