Menu
▾
▴
Re: [tipc-discussion] [PATCH net-next 03/10] tipc: port statistics reporting using char buffer
|
From: Erik H. <eri...@er...> - 2012-05-30 07:58:53
|
It can be made into a macro, but since we need to grab the spinlock
protecting the temp buffer, and do overflow checking, it felt better to
wrap this in a function.
Also, we probably need to prepend an "overflow" message to the output
buffer if we try to log more than there's room for.
PS.
Maybe some time in the future we can present statistics in a sysfs file
instead, that solves the "buffer overflow" problem (by writing paged
data to a bin attribute)
//E
On 2012-05-30 05:46, Jon Maloy wrote:
> Hmm I wonder if we really need tipc_vstrcat() as a function.
> Maybe you could just define a macro doing something like
> snprintf(buf[strlen(buf)],size - strlen(buf),fmt)
>
> Don't remember from the top of my head how to
> define this type of macro, but I have done
> similar things before.
> Maybe it is also worth looking elsewhere if a
> corresponding function doesn't already exist.
> I wouldn't be surprised.
>
> ///jon
> ________________________________________
> From: Erik Hugne
> Sent: May 28, 2012 11:04 AM
> To: Jon Maloy; yin...@wi...
> Cc: all...@wi...; tip...@li...; Erik Hugne
> Subject: [PATCH net-next 03/10] tipc: port statistics reporting using char buffer
>
> A user can request a list of all open TIPC ports using
> the tipc-utils userspace program. The functions that constructs
> this list is changed to use a char buffer instead of the
> internal print_buf structure.
>
> A utility function, tipc_vstrcat is added.
> This is used to append data to the buffer if there is room.
> Otherwise an error message indicating overflow is
> prepended to the buffer (same way as tipc_printf does).
>
> Signed-off-by: Erik Hugne<eri...@er...>
> ---
> net/tipc/config.c | 1 +
> net/tipc/core.h | 1 +
> net/tipc/log.c | 29 +++++++++++++++++++++++++++--
> net/tipc/port.c | 37 +++++++++++++++++++++----------------
> 4 files changed, 50 insertions(+), 18 deletions(-)
>
> diff --git a/net/tipc/config.c b/net/tipc/config.c
> index c5712a3..74bcf3d 100644
> --- a/net/tipc/config.c
> +++ b/net/tipc/config.c
> @@ -55,6 +55,7 @@ struct sk_buff *tipc_cfg_reply_alloc(int payload_size)
> buf = alloc_skb(rep_headroom + payload_size, GFP_ATOMIC);
> if (buf)
> skb_reserve(buf, rep_headroom);
> + memset(buf->data, 0, payload_size);
> return buf;
> }
>
> diff --git a/net/tipc/core.h b/net/tipc/core.h
> index 1d08d5a..088f0cf 100644
> --- a/net/tipc/core.h
> +++ b/net/tipc/core.h
> @@ -81,6 +81,7 @@ extern struct print_buf *const TIPC_CONS;
> extern struct print_buf *const TIPC_LOG;
>
> void tipc_printf(struct print_buf *, const char *fmt, ...);
> +int tipc_vstrcat(char *buf, int buf_len, const char *fmt, ...);
>
> /*
> * TIPC_OUTPUT is the destination print buffer for system messages.
> diff --git a/net/tipc/log.c b/net/tipc/log.c
> index 026733f..5b5a0b9 100644
> --- a/net/tipc/log.c
> +++ b/net/tipc/log.c
> @@ -38,6 +38,9 @@
> #include "config.h"
> #include "log.h"
>
> +
> +#define PRINT_OVERFLOW "\n\n*** PRINT BUFFER OVERFLOW ***\n\n"
> +
> /*
> * TIPC pre-defines the following print buffers:
> *
> @@ -59,8 +62,9 @@ struct print_buf *const TIPC_LOG =&log_buf;
> /*
> * Locking policy when using print buffers.
> *
> - * 1) tipc_printf() uses 'print_lock' to protect against concurrent access to
> - * 'print_string' when writing to a print buffer. This also protects against
> + * 1) tipc_printf() and tipc_vstrcat() uses 'print_lock'
> + * to protect against concurrent access to'print_string'
> + * when writing to a print buffer. This also protects against
> * concurrent writes to the print buffer being written to.
> *
> * 2) tipc_log_XXX() leverages the aforementioned use of 'print_lock' to
> @@ -213,6 +217,27 @@ static void tipc_printbuf_move(struct print_buf *pb_to,
> tipc_printbuf_reset(pb_from);
> }
>
> +int tipc_vstrcat(char *buf, int size, const char *fmt, ...)
> +{
> + int chars_to_add;
> + int chars_left;
> + char *c;
> +
> + spin_lock_bh(&print_lock);
> + c = buf + strlen(buf);
> + chars_left = size - strlen(buf) - 1;
> + FORMAT(print_string, chars_to_add, fmt);
> +
> + if (chars_to_add> chars_left) {
> + strncpy(buf, PRINT_OVERFLOW, strlen(PRINT_OVERFLOW));
> + spin_unlock_bh(&print_lock);
> + return 0;
> + }
> + strcpy(c, print_string);
> + spin_unlock_bh(&print_lock);
> + return chars_to_add;
> +}
> +
> /**
> * tipc_printf - append formatted output to print buffer
> * @pb: pointer to print buffer
> diff --git a/net/tipc/port.c b/net/tipc/port.c
> index f52c7af0..0f541ac 100644
> --- a/net/tipc/port.c
> +++ b/net/tipc/port.c
> @@ -574,40 +574,45 @@ exit:
> kfree_skb(buf);
> }
>
> -static void port_print(struct tipc_port *p_ptr, struct print_buf *buf, int full_id)
> +static int port_print(struct tipc_port *p_ptr, char *buf, int buf_len,
> + int full_id)
> {
> struct publication *publ;
> + int len = 0;
>
> if (full_id)
> - tipc_printf(buf, "<%u.%u.%u:%u>:",
> + len += tipc_vstrcat(buf, buf_len, "<%u.%u.%u:%u>:",
> tipc_zone(tipc_own_addr), tipc_cluster(tipc_own_addr),
> tipc_node(tipc_own_addr), p_ptr->ref);
> else
> - tipc_printf(buf, "%-10u:", p_ptr->ref);
> + len += tipc_vstrcat(buf, buf_len, "%-10u:", p_ptr->ref);
>
> if (p_ptr->connected) {
> u32 dport = port_peerport(p_ptr);
> u32 destnode = port_peernode(p_ptr);
>
> - tipc_printf(buf, " connected to<%u.%u.%u:%u>",
> + len += tipc_vstrcat(buf, buf_len, " connected to<%u.%u.%u:%u>",
> tipc_zone(destnode), tipc_cluster(destnode),
> tipc_node(destnode), dport);
> if (p_ptr->conn_type != 0)
> - tipc_printf(buf, " via {%u,%u}",
> + len += tipc_vstrcat(buf, buf_len, " via {%u,%u}",
> p_ptr->conn_type,
> p_ptr->conn_instance);
> } else if (p_ptr->published) {
> - tipc_printf(buf, " bound to");
> + len += tipc_vstrcat(buf, buf_len, " bound to");
> list_for_each_entry(publ,&p_ptr->publications, pport_list) {
> if (publ->lower == publ->upper)
> - tipc_printf(buf, " {%u,%u}", publ->type,
> + len += tipc_vstrcat(buf, buf_len,
> + " {%u,%u}", publ->type,
> publ->lower);
> else
> - tipc_printf(buf, " {%u,%u,%u}", publ->type,
> - publ->lower, publ->upper);
> + len += tipc_vstrcat(buf, buf_len,
> + " {%u,%u,%u}", publ->type,
> + publ->lower, publ->upper);
> }
> }
> - tipc_printf(buf, "\n");
> + len += tipc_vstrcat(buf, buf_len, "\n");
> + return len;
> }
>
> #define MAX_PORT_QUERY 32768
> @@ -616,25 +621,25 @@ struct sk_buff *tipc_port_get_ports(void)
> {
> struct sk_buff *buf;
> struct tlv_desc *rep_tlv;
> - struct print_buf pb;
> struct tipc_port *p_ptr;
> - int str_len;
> + char *pb;
> + int pb_len;
> + int str_len = 0;
>
> buf = tipc_cfg_reply_alloc(TLV_SPACE(MAX_PORT_QUERY));
> if (!buf)
> return NULL;
> rep_tlv = (struct tlv_desc *)buf->data;
> + pb = TLV_DATA(rep_tlv);
> + pb_len = TLV_SPACE(MAX_PORT_QUERY);
>
> - tipc_printbuf_init(&pb, TLV_DATA(rep_tlv), MAX_PORT_QUERY);
> spin_lock_bh(&tipc_port_list_lock);
> list_for_each_entry(p_ptr,&ports, port_list) {
> spin_lock_bh(p_ptr->lock);
> - port_print(p_ptr,&pb, 0);
> + str_len += port_print(p_ptr, pb, pb_len, 0);
> spin_unlock_bh(p_ptr->lock);
> }
> spin_unlock_bh(&tipc_port_list_lock);
> - str_len = tipc_printbuf_validate(&pb);
> -
> skb_put(buf, TLV_SPACE(str_len));
> TLV_SET(rep_tlv, TIPC_TLV_ULTRA_STRING, NULL, str_len);
>
> --
> 1.7.5.4
>
|