shemminger at vyatta
Oct 3, 2011, 3:27 PM
Post #2 of 5
(389 views)
Permalink
|
On Mon, 03 Oct 2011 22:56:55 +0200
"Jaroslav Fojtik" <jafojtik [at] seznam> wrote:
> Dear Balaji,
>
> > Can you send the fix as a patch so that it becomes easy to review the
> > changes per se
>
> here is it. If you like it, I can improve it a little bit.
>
> --------------------------------------------------------------------------
>
>
> *** M:\quagga\quagga-0.99.18\ospfd\ospf_packet.c Mon Mar 21 13:09:14 2011
> --- P:\USR_SRC\quagga-0.99.18\ospfd\ospf_packet.c Mon Oct 3 22:53:08 2011
> ***************
> *** 1528,1533 ****
> --- 1528,1537 ----
> list_free (ls_upd);
> }
>
> +
> + static int LSA_CHECKSUM_ERR_COUNT = 0;
> + static int LSA_CHECKSUM_ERR_SEC = 0;
> +
> /* Get the list of LSAs from Link State Update packet.
> And process some validation -- RFC2328 Section 13. (1)-(2). */
> static struct list *
> ***************
> *** 1561,1568 ****
> sum = lsah->checksum;
> if (sum != ospf_lsa_checksum (lsah))
> {
> ! zlog_warn ("Link State Update: LSA checksum error %x, %x.",
> sum, lsah->checksum);
> continue;
> }
>
> --- 1565,1590 ----
> sum = lsah->checksum;
> if (sum != ospf_lsa_checksum (lsah))
> {
> ! struct timeval clock;
> !
> ! gettimeofday(&clock, NULL);
> ! LSA_CHECKSUM_ERR_COUNT++;
> !
> ! if(LSA_CHECKSUM_ERR_COUNT==1)
> ! {
> ! zlog_warn("Link State Update: LSA checksum error %x, %x.",
> sum, lsah->checksum);
> + LSA_CHECKSUM_ERR_SEC = clock.tv_sec;
> + }
> + else
> + {
> + if(labs(clock.tv_sec-LSA_CHECKSUM_ERR_SEC)>60)
> + {
> + zlog_warn("Link State Update: LSA checksum error %x, %x repeated %d times.",
> + sum, lsah->checksum, LSA_CHECKSUM_ERR_COUNT);
> + LSA_CHECKSUM_ERR_COUNT = 0;
> + }
> + }
> continue;
> }
>
>
> --------------------------------------------------------------------------
>
> regards
> Jara
>
Thank you for the patch. The concept is good but next time please try
and follow the guidelines in the HACKING file.
1. variable names shouldn't be all upper case (that is reserved for macros).
2. use 'diff -up ' to get better patch format.
I think a more general solution (inspired by linux kernel netratelimit())
would be something like the following (untested):
diff --git a/lib/thread.c b/lib/thread.c
index fd841c2..8745c6f 100644
--- a/lib/thread.c
+++ b/lib/thread.c
@@ -216,6 +216,39 @@ recent_relative_time (void)
return relative_time;
}
+/* Limit messages from the same place */
+int __ratelimit(struct msg_ratelimit *rt, const char *func)
+{
+ struct timeval now;
+
+ /* if no limit on messages per second? */
+ if (rt->interval == 0)
+ return 1;
+
+ quagga_real_stabilised (&now);
+ if (rt->begin.tv_sec == 0 && rt->begin.tv_usec == 0)
+ rt->begin = now;
+
+ else if (timeval_elapsed (now, rt->begin) / TIMER_SECOND_MICRO > rt->interval)
+ {
+ if (rt->missed)
+ zlog_warn("%s: %d messages surpressed\n",
+ func, rt->missed);
+ rt->begin = now;
+ rt->printed = 0;
+ rt->missed = 0;
+ }
+
+ if (rt->burst && rt->burst > rt->printed) {
+ rt->printed++;
+ return 1;
+ } else {
+ rt->missed++;
+ return 0;
+ }
+}
+
+
static unsigned int
cpu_record_hash_key (struct cpu_thread_history *a)
{
diff --git a/lib/thread.h b/lib/thread.h
index 978aa6b..78d4ce8 100644
--- a/lib/thread.h
+++ b/lib/thread.h
@@ -216,4 +216,21 @@ extern unsigned long thread_consumed_time(RUSAGE_T *after, RUSAGE_T *before,
extern struct timeval recent_time;
/* Similar to recent_time, but a monotonically increasing time value */
extern struct timeval recent_relative_time (void);
+
+/* Allow control of rate limit of messages. Used to avoid log
+ overflow for protocol errors, etc. */
+#define DEFAULT_RATELIMIT_INTERVAL (5 * HZ)
+#define DEFAULT_RATELIMIT_BURST 10
+
+struct msg_ratelimit {
+ struct timeval begin;
+ unsigned int interval;
+ unsigned int burst;
+ unsigned int printed;
+ unsigned int missed;
+};
+
+extern int __ratelimit(struct msg_ratelimit *rt, const char *func);
+#define ratelimit(state) __ratelimit(state, __func__)
+
#endif /* _ZEBRA_THREAD_H */
diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 0f338d3..d74b9c9 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -1535,6 +1535,11 @@ ospf_ls_req (struct ip *iph, struct ospf_header *ospfh,
list_free (ls_upd);
}
+static struct msg_ratelimit ospf_lsa_errorlimit = {
+ .interval = DEFAULT_RATELIMIT_INTERVAL,
+ .burst = DEFAULT_RATELIMIT_BURST,
+};
+
/* Get the list of LSAs from Link State Update packet.
And process some validation -- RFC2328 Section 13. (1)-(2). */
static struct list *
@@ -1560,7 +1565,8 @@ ospf_ls_upd_list_lsa (struct ospf_neighbor *nbr, struct stream *s,
if (length > size)
{
- zlog_warn ("Link State Update: LSA length exceeds packet size.");
+ if (ratelimit(&ospf_lsa_errorlimit))
+ zlog_warn ("Link State Update: LSA length exceeds packet size.");
break;
}
@@ -1568,15 +1574,17 @@ ospf_ls_upd_list_lsa (struct ospf_neighbor *nbr, struct stream *s,
sum = lsah->checksum;
if (sum != ospf_lsa_checksum (lsah))
{
- zlog_warn ("Link State Update: LSA checksum error %x, %x.",
- sum, lsah->checksum);
+ if (ratelimit(&ospf_lsa_errorlimit))
+ zlog_warn ("Link State Update: LSA checksum error %x, %x.",
+ sum, lsah->checksum);
continue;
}
/* Examine the LSA's LS type. */
if (lsah->type < OSPF_MIN_LSA || lsah->type >= OSPF_MAX_LSA)
{
- zlog_warn ("Link State Update: Unknown LS type %d", lsah->type);
+ if (ratelimit(&ospf_lsa_errorlimit))
+ zlog_warn ("Link State Update: Unknown LS type %d", lsah->type);
continue;
}
@@ -1600,7 +1608,8 @@ ospf_ls_upd_list_lsa (struct ospf_neighbor *nbr, struct stream *s,
* This neighbor must know the exact usage of O-bit;
* the bit will be set in Type-9,10,11 LSAs only.
*/
- zlog_warn ("LSA[Type%d:%s]: O-bit abuse?", lsah->type, inet_ntoa (lsah->id));
+ if (ratelimit(&ospf_lsa_errorlimit))
+ zlog_warn ("LSA[Type%d:%s]: O-bit abuse?", lsah->type, inet_ntoa (lsah->id));
continue;
}
#endif /* STRICT_OBIT_USAGE_CHECK */
_______________________________________________
Quagga-dev mailing list
Quagga-dev [at] lists
http://lists.quagga.net/mailman/listinfo/quagga-dev
|
