netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2 net-next] nstat: add sctp snmp support
@ 2016-09-02  7:12 Hangbin Liu
  2016-09-02 10:09 ` Phil Sutter
  2016-09-02 19:46 ` Stephen Hemminger
  0 siblings, 2 replies; 5+ messages in thread
From: Hangbin Liu @ 2016-09-02  7:12 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger, Phil Sutter, Hangbin Liu

SCTP module was not load by default. But this should be OK since we will not
load table if fdopen() failed.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 misc/nstat.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/misc/nstat.c b/misc/nstat.c
index 6143719..8035be4 100644
--- a/misc/nstat.c
+++ b/misc/nstat.c
@@ -76,6 +76,11 @@ static int net_snmp6_open(void)
 	return generic_proc_open("PROC_NET_SNMP6", "net/snmp6");
 }
 
+static int net_sctp_snmp_open(void)
+{
+	return generic_proc_open("PROC_NET_SCTP_SNMP", "net/sctp/snmp");
+}
+
 struct nstat_ent {
 	struct nstat_ent *next;
 	char		 *id;
@@ -247,6 +252,16 @@ static void load_ugly_table(FILE *fp)
 	}
 }
 
+static void load_sctp_snmp(void)
+{
+	FILE *fp = fdopen(net_sctp_snmp_open(), "r");
+
+	if (fp) {
+		load_good_table(fp);
+		fclose(fp);
+	}
+}
+
 static void load_snmp(void)
 {
 	FILE *fp = fdopen(net_snmp_open(), "r");
@@ -450,6 +465,7 @@ static void server_loop(int fd)
 	load_netstat();
 	load_snmp6();
 	load_snmp();
+	load_sctp_snmp();
 
 	for (;;) {
 		int status;
@@ -706,6 +722,7 @@ int main(int argc, char *argv[])
 		load_netstat();
 		load_snmp6();
 		load_snmp();
+		load_sctp_snmp();
 		if (info_source[0] == 0)
 			strcpy(info_source, "kernel");
 	}
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH iproute2 net-next] nstat: add sctp snmp support
  2016-09-02  7:12 [PATCH iproute2 net-next] nstat: add sctp snmp support Hangbin Liu
@ 2016-09-02 10:09 ` Phil Sutter
  2016-09-05  3:31   ` Hangbin Liu
  2016-09-02 19:46 ` Stephen Hemminger
  1 sibling, 1 reply; 5+ messages in thread
From: Phil Sutter @ 2016-09-02 10:09 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, Stephen Hemminger

On Fri, Sep 02, 2016 at 03:12:38PM +0800, Hangbin Liu wrote:
> SCTP module was not load by default. But this should be OK since we will not
> load table if fdopen() failed.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  misc/nstat.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/misc/nstat.c b/misc/nstat.c
> index 6143719..8035be4 100644
> --- a/misc/nstat.c
> +++ b/misc/nstat.c
> @@ -76,6 +76,11 @@ static int net_snmp6_open(void)
>  	return generic_proc_open("PROC_NET_SNMP6", "net/snmp6");
>  }
>  
> +static int net_sctp_snmp_open(void)
> +{
> +	return generic_proc_open("PROC_NET_SCTP_SNMP", "net/sctp/snmp");
> +}
> +
>  struct nstat_ent {
>  	struct nstat_ent *next;
>  	char		 *id;
> @@ -247,6 +252,16 @@ static void load_ugly_table(FILE *fp)
>  	}
>  }
>  
> +static void load_sctp_snmp(void)
> +{
> +	FILE *fp = fdopen(net_sctp_snmp_open(), "r");
> +
> +	if (fp) {
> +		load_good_table(fp);
> +		fclose(fp);
> +	}
> +}
> +
>  static void load_snmp(void)
>  {
>  	FILE *fp = fdopen(net_snmp_open(), "r");
> @@ -450,6 +465,7 @@ static void server_loop(int fd)
>  	load_netstat();
>  	load_snmp6();
>  	load_snmp();
> +	load_sctp_snmp();
>  
>  	for (;;) {
>  		int status;
> @@ -706,6 +722,7 @@ int main(int argc, char *argv[])
>  		load_netstat();
>  		load_snmp6();
>  		load_snmp();
> +		load_sctp_snmp();
>  		if (info_source[0] == 0)
>  			strcpy(info_source, "kernel");
>  	}

Did you forget to add the load call to update_db(), or am I missing
something?

Apart from that, looks nice and clean.

Cheers, Phil

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH iproute2 net-next] nstat: add sctp snmp support
  2016-09-02  7:12 [PATCH iproute2 net-next] nstat: add sctp snmp support Hangbin Liu
  2016-09-02 10:09 ` Phil Sutter
@ 2016-09-02 19:46 ` Stephen Hemminger
  2016-09-02 19:48   ` Stephen Hemminger
  1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2016-09-02 19:46 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, Phil Sutter

On Fri,  2 Sep 2016 15:12:38 +0800
Hangbin Liu <liuhangbin@gmail.com> wrote:

> SCTP module was not load by default. But this should be OK since we will not
> load table if fdopen() failed.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

This seems like a bad idea for the normal distro user.
If they run nstat command the SCTP kernel module would get loaded even
if never used.

Makes more sense not to display SCTP statistics if it isn't loaded.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH iproute2 net-next] nstat: add sctp snmp support
  2016-09-02 19:46 ` Stephen Hemminger
@ 2016-09-02 19:48   ` Stephen Hemminger
  0 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2016-09-02 19:48 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, Phil Sutter

On Fri, 2 Sep 2016 12:46:49 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> On Fri,  2 Sep 2016 15:12:38 +0800
> Hangbin Liu <liuhangbin@gmail.com> wrote:
> 
> > SCTP module was not load by default. But this should be OK since we will not
> > load table if fdopen() failed.
> > 
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>  
> 
> This seems like a bad idea for the normal distro user.
> If they run nstat command the SCTP kernel module would get loaded even
> if never used.
> 
> Makes more sense not to display SCTP statistics if it isn't loaded.
> 

Never mind, opening the proc file won't load SCTP kernel module.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH iproute2 net-next] nstat: add sctp snmp support
  2016-09-02 10:09 ` Phil Sutter
@ 2016-09-05  3:31   ` Hangbin Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Hangbin Liu @ 2016-09-05  3:31 UTC (permalink / raw)
  To: Phil Sutter, Hangbin Liu, network dev, Stephen Hemminger

2016-09-02 18:09 GMT+08:00 Phil Sutter <phil@nwl.cc>:
> Did you forget to add the load call to update_db(), or am I missing
> something?

Opps, forgot to add it there. I will send patchv2 for this issue.

Thanks
Hangbin

> Apart from that, looks nice and clean.
>
> Cheers, Phil

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-09-05  3:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-02  7:12 [PATCH iproute2 net-next] nstat: add sctp snmp support Hangbin Liu
2016-09-02 10:09 ` Phil Sutter
2016-09-05  3:31   ` Hangbin Liu
2016-09-02 19:46 ` Stephen Hemminger
2016-09-02 19:48   ` Stephen Hemminger

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).