Re: tcpdump: enable some more bgp info
On Tue, May 30, 2017 at 11:06:10AM +0200, Michal Mazurek wrote:
> On 10:43:30, 30.05.17, Job Snijders wrote:
> > In the registry created by RFC 6608, the value "0" is the BGP Finite
> > State Machine Error subcode meaning "Unspecified Error". I think that
> > when a name is assigned to a value, the name should be printed (like
> > your patch does for subcode values 1, 2, and 3).
> >
> > If no name is known for the error subcode, just printing the number is
> > useful indeed.
>
> You are right.
OK claudio@
On a side note. The notification error code 7 seems to be wrong.
The capability error codes made it never into a standard and now
error code 7 if for enhanced route refresh.
So I would replace bgpnotify_minor_cap with
static const char *bgpnotify_minor_err[] = {
NULL, "Invalid Message Length",
};
See also
https://www.iana.org/assignments/bgp-parameters/bgp-parameters.xhtml#route-refresh-error-subcodes
> Index: usr.sbin/tcpdump/print-bgp.c
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 print-bgp.c
> --- usr.sbin/tcpdump/print-bgp.c 24 Apr 2017 20:35:35 - 1.21
> +++ usr.sbin/tcpdump/print-bgp.c 30 May 2017 09:00:49 -
> @@ -226,6 +226,16 @@ static const char *bgpnotify_minor_updat
> "Invalid Network Field", "Malformed AS_PATH",
> };
>
> +static const char *bgpnotify_minor_holdtime[] = {
> + NULL,
> +};
> +
> +/* RFC 6608 */
> +static const char *bgpnotify_minor_fsm[] = {
> + "Unspecified Error", "In OpenSent State", "In OpenConfirm State",
> + "In Established State",
> +};
> +
> /* RFC 4486 */
> #define BGP_NOTIFY_MINOR_CEASE_MAXPRFX 1
> /* draft-ietf-idr-shutdown-07 */
> @@ -246,14 +256,16 @@ static const char *bgpnotify_minor_cap[]
>
> static const char **bgpnotify_minor[] = {
> NULL, bgpnotify_minor_msg, bgpnotify_minor_open, bgpnotify_minor_update,
> + bgpnotify_minor_holdtime, bgpnotify_minor_fsm, bgpnotify_minor_cease,
> + bgpnotify_minor_cap,
> };
> static const int bgpnotify_minor_siz[] = {
> 0,
> sizeof(bgpnotify_minor_msg)/sizeof(bgpnotify_minor_msg[0]),
> sizeof(bgpnotify_minor_open)/sizeof(bgpnotify_minor_open[0]),
> sizeof(bgpnotify_minor_update)/sizeof(bgpnotify_minor_update[0]),
> - 0,
> - 0,
> + sizeof(bgpnotify_minor_holdtime)/sizeof(bgpnotify_minor_holdtime[0]),
> + sizeof(bgpnotify_minor_fsm)/sizeof(bgpnotify_minor_fsm[0]),
> sizeof(bgpnotify_minor_cease)/sizeof(bgpnotify_minor_cease[0]),
> sizeof(bgpnotify_minor_cap)/sizeof(bgpnotify_minor_cap[0]),
> };
>
> --
> Michal Mazurek
>
--
:wq Claudio
Re: tcpdump: enable some more bgp info
On 10:43:30, 30.05.17, Job Snijders wrote:
> In the registry created by RFC 6608, the value "0" is the BGP Finite
> State Machine Error subcode meaning "Unspecified Error". I think that
> when a name is assigned to a value, the name should be printed (like
> your patch does for subcode values 1, 2, and 3).
>
> If no name is known for the error subcode, just printing the number is
> useful indeed.
You are right.
Index: usr.sbin/tcpdump/print-bgp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
retrieving revision 1.21
diff -u -p -r1.21 print-bgp.c
--- usr.sbin/tcpdump/print-bgp.c24 Apr 2017 20:35:35 - 1.21
+++ usr.sbin/tcpdump/print-bgp.c30 May 2017 09:00:49 -
@@ -226,6 +226,16 @@ static const char *bgpnotify_minor_updat
"Invalid Network Field", "Malformed AS_PATH",
};
+static const char *bgpnotify_minor_holdtime[] = {
+ NULL,
+};
+
+/* RFC 6608 */
+static const char *bgpnotify_minor_fsm[] = {
+ "Unspecified Error", "In OpenSent State", "In OpenConfirm State",
+ "In Established State",
+};
+
/* RFC 4486 */
#define BGP_NOTIFY_MINOR_CEASE_MAXPRFX 1
/* draft-ietf-idr-shutdown-07 */
@@ -246,14 +256,16 @@ static const char *bgpnotify_minor_cap[]
static const char **bgpnotify_minor[] = {
NULL, bgpnotify_minor_msg, bgpnotify_minor_open, bgpnotify_minor_update,
+ bgpnotify_minor_holdtime, bgpnotify_minor_fsm, bgpnotify_minor_cease,
+ bgpnotify_minor_cap,
};
static const int bgpnotify_minor_siz[] = {
0,
sizeof(bgpnotify_minor_msg)/sizeof(bgpnotify_minor_msg[0]),
sizeof(bgpnotify_minor_open)/sizeof(bgpnotify_minor_open[0]),
sizeof(bgpnotify_minor_update)/sizeof(bgpnotify_minor_update[0]),
- 0,
- 0,
+ sizeof(bgpnotify_minor_holdtime)/sizeof(bgpnotify_minor_holdtime[0]),
+ sizeof(bgpnotify_minor_fsm)/sizeof(bgpnotify_minor_fsm[0]),
sizeof(bgpnotify_minor_cease)/sizeof(bgpnotify_minor_cease[0]),
sizeof(bgpnotify_minor_cap)/sizeof(bgpnotify_minor_cap[0]),
};
--
Michal Mazurek
Re: tcpdump: enable some more bgp info
On Tue, May 30, 2017 at 10:21:17AM +0200, Michal Mazurek wrote:
> On 12:15:06, 29.05.17, Job Snijders wrote:
> > perhaps add a comment like /* RFC 6608 */ above the below:
>
> Right, it will make it more consistent.
>
> > > +static const char *bgpnotify_minor_fsm[] = {
> > > + NULL, "In OpenSent State", "In OpenConfirm State",
> > > + "In Established State",
> > > +};
> >
> > and maybe s/NULL/"Unspecified Error"/
>
> If it's NULL, then tcpdump will print out the number:
>
> if (p == NULL) {
> snprintf(buf, sizeof(buf), "#%d", minor);
Perhaps there is a misunderstanding on your part or on my part.
In the registry created by RFC 6608, the value "0" is the BGP Finite
State Machine Error subcode meaning "Unspecified Error". I think that
when a name is assigned to a value, the name should be printed (like
your patch does for subcode values 1, 2, and 3).
If no name is known for the error subcode, just printing the number is
useful indeed.
Kind regards,
Job
Re: tcpdump: enable some more bgp info
On Tue, May 30, 2017 at 10:21:17AM +0200, Michal Mazurek wrote:
> On 12:15:06, 29.05.17, Job Snijders wrote:
> > perhaps add a comment like /* RFC 6608 */ above the below:
>
> Right, it will make it more consistent.
>
> > > +static const char *bgpnotify_minor_fsm[] = {
> > > + NULL, "In OpenSent State", "In OpenConfirm State",
> > > + "In Established State",
> > > +};
> >
> > and maybe s/NULL/"Unspecified Error"/
>
> If it's NULL, then tcpdump will print out the number:
>
> if (p == NULL) {
> snprintf(buf, sizeof(buf), "#%d", minor);
>
OK claudio@
>
> Index: usr.sbin/tcpdump/print-bgp.c
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 print-bgp.c
> --- usr.sbin/tcpdump/print-bgp.c 24 Apr 2017 20:35:35 - 1.21
> +++ usr.sbin/tcpdump/print-bgp.c 30 May 2017 08:12:17 -
> @@ -226,6 +226,16 @@ static const char *bgpnotify_minor_updat
> "Invalid Network Field", "Malformed AS_PATH",
> };
>
> +static const char *bgpnotify_minor_holdtime[] = {
> + NULL,
> +};
> +
> +/* RFC 6608 */
> +static const char *bgpnotify_minor_fsm[] = {
> + NULL, "In OpenSent State", "In OpenConfirm State",
> + "In Established State",
> +};
> +
> /* RFC 4486 */
> #define BGP_NOTIFY_MINOR_CEASE_MAXPRFX 1
> /* draft-ietf-idr-shutdown-07 */
> @@ -246,14 +256,16 @@ static const char *bgpnotify_minor_cap[]
>
> static const char **bgpnotify_minor[] = {
> NULL, bgpnotify_minor_msg, bgpnotify_minor_open, bgpnotify_minor_update,
> + bgpnotify_minor_holdtime, bgpnotify_minor_fsm, bgpnotify_minor_cease,
> + bgpnotify_minor_cap,
> };
> static const int bgpnotify_minor_siz[] = {
> 0,
> sizeof(bgpnotify_minor_msg)/sizeof(bgpnotify_minor_msg[0]),
> sizeof(bgpnotify_minor_open)/sizeof(bgpnotify_minor_open[0]),
> sizeof(bgpnotify_minor_update)/sizeof(bgpnotify_minor_update[0]),
> - 0,
> - 0,
> + sizeof(bgpnotify_minor_holdtime)/sizeof(bgpnotify_minor_holdtime[0]),
> + sizeof(bgpnotify_minor_fsm)/sizeof(bgpnotify_minor_fsm[0]),
> sizeof(bgpnotify_minor_cease)/sizeof(bgpnotify_minor_cease[0]),
> sizeof(bgpnotify_minor_cap)/sizeof(bgpnotify_minor_cap[0]),
> };
>
> --
> Michal Mazurek
>
--
:wq Claudio
Re: tcpdump: enable some more bgp info
On 2017 May 30 (Tue) at 10:21:17 +0200 (+0200), Michal Mazurek wrote:
:On 12:15:06, 29.05.17, Job Snijders wrote:
:> perhaps add a comment like /* RFC 6608 */ above the below:
:
:Right, it will make it more consistent.
:
:> > +static const char *bgpnotify_minor_fsm[] = {
:> > + NULL, "In OpenSent State", "In OpenConfirm State",
:> > + "In Established State",
:> > +};
:>
:> and maybe s/NULL/"Unspecified Error"/
:
:If it's NULL, then tcpdump will print out the number:
:
: if (p == NULL) {
: snprintf(buf, sizeof(buf), "#%d", minor);
:
:
OK
:Index: usr.sbin/tcpdump/print-bgp.c
:===
:RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
:retrieving revision 1.21
:diff -u -p -r1.21 print-bgp.c
:--- usr.sbin/tcpdump/print-bgp.c 24 Apr 2017 20:35:35 - 1.21
:+++ usr.sbin/tcpdump/print-bgp.c 30 May 2017 08:12:17 -
:@@ -226,6 +226,16 @@ static const char *bgpnotify_minor_updat
: "Invalid Network Field", "Malformed AS_PATH",
: };
:
:+static const char *bgpnotify_minor_holdtime[] = {
:+ NULL,
:+};
:+
:+/* RFC 6608 */
:+static const char *bgpnotify_minor_fsm[] = {
:+ NULL, "In OpenSent State", "In OpenConfirm State",
:+ "In Established State",
:+};
:+
: /* RFC 4486 */
: #define BGP_NOTIFY_MINOR_CEASE_MAXPRFX 1
: /* draft-ietf-idr-shutdown-07 */
:@@ -246,14 +256,16 @@ static const char *bgpnotify_minor_cap[]
:
: static const char **bgpnotify_minor[] = {
: NULL, bgpnotify_minor_msg, bgpnotify_minor_open, bgpnotify_minor_update,
:+ bgpnotify_minor_holdtime, bgpnotify_minor_fsm, bgpnotify_minor_cease,
:+ bgpnotify_minor_cap,
: };
: static const int bgpnotify_minor_siz[] = {
: 0,
: sizeof(bgpnotify_minor_msg)/sizeof(bgpnotify_minor_msg[0]),
: sizeof(bgpnotify_minor_open)/sizeof(bgpnotify_minor_open[0]),
: sizeof(bgpnotify_minor_update)/sizeof(bgpnotify_minor_update[0]),
:- 0,
:- 0,
:+ sizeof(bgpnotify_minor_holdtime)/sizeof(bgpnotify_minor_holdtime[0]),
:+ sizeof(bgpnotify_minor_fsm)/sizeof(bgpnotify_minor_fsm[0]),
: sizeof(bgpnotify_minor_cease)/sizeof(bgpnotify_minor_cease[0]),
: sizeof(bgpnotify_minor_cap)/sizeof(bgpnotify_minor_cap[0]),
: };
:
:--
:Michal Mazurek
:
--
There once was a man named Eugene
Who invented a screwing machine
Concave and convex
It served either sex
And it played with itself in between.
Re: tcpdump: enable some more bgp info
> > > +static const char *bgpnotify_minor_fsm[] = {
> > > + NULL, "In OpenSent State", "In OpenConfirm State",
> > > + "In Established State",
> > > +};
> >
> > and maybe s/NULL/"Unspecified Error"/
>
> If it's NULL, then tcpdump will print out the number:
>
> if (p == NULL) {
> snprintf(buf, sizeof(buf), "#%d", minor);
Yes, that is better because it provides more detail.
Re: tcpdump: enable some more bgp info
On 12:15:06, 29.05.17, Job Snijders wrote:
> perhaps add a comment like /* RFC 6608 */ above the below:
Right, it will make it more consistent.
> > +static const char *bgpnotify_minor_fsm[] = {
> > + NULL, "In OpenSent State", "In OpenConfirm State",
> > + "In Established State",
> > +};
>
> and maybe s/NULL/"Unspecified Error"/
If it's NULL, then tcpdump will print out the number:
if (p == NULL) {
snprintf(buf, sizeof(buf), "#%d", minor);
Index: usr.sbin/tcpdump/print-bgp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
retrieving revision 1.21
diff -u -p -r1.21 print-bgp.c
--- usr.sbin/tcpdump/print-bgp.c24 Apr 2017 20:35:35 - 1.21
+++ usr.sbin/tcpdump/print-bgp.c30 May 2017 08:12:17 -
@@ -226,6 +226,16 @@ static const char *bgpnotify_minor_updat
"Invalid Network Field", "Malformed AS_PATH",
};
+static const char *bgpnotify_minor_holdtime[] = {
+ NULL,
+};
+
+/* RFC 6608 */
+static const char *bgpnotify_minor_fsm[] = {
+ NULL, "In OpenSent State", "In OpenConfirm State",
+ "In Established State",
+};
+
/* RFC 4486 */
#define BGP_NOTIFY_MINOR_CEASE_MAXPRFX 1
/* draft-ietf-idr-shutdown-07 */
@@ -246,14 +256,16 @@ static const char *bgpnotify_minor_cap[]
static const char **bgpnotify_minor[] = {
NULL, bgpnotify_minor_msg, bgpnotify_minor_open, bgpnotify_minor_update,
+ bgpnotify_minor_holdtime, bgpnotify_minor_fsm, bgpnotify_minor_cease,
+ bgpnotify_minor_cap,
};
static const int bgpnotify_minor_siz[] = {
0,
sizeof(bgpnotify_minor_msg)/sizeof(bgpnotify_minor_msg[0]),
sizeof(bgpnotify_minor_open)/sizeof(bgpnotify_minor_open[0]),
sizeof(bgpnotify_minor_update)/sizeof(bgpnotify_minor_update[0]),
- 0,
- 0,
+ sizeof(bgpnotify_minor_holdtime)/sizeof(bgpnotify_minor_holdtime[0]),
+ sizeof(bgpnotify_minor_fsm)/sizeof(bgpnotify_minor_fsm[0]),
sizeof(bgpnotify_minor_cease)/sizeof(bgpnotify_minor_cease[0]),
sizeof(bgpnotify_minor_cap)/sizeof(bgpnotify_minor_cap[0]),
};
--
Michal Mazurek
Re: tcpdump: enable some more bgp info
On Mon, May 29, 2017 at 12:02:33PM +0200, Michal Mazurek wrote:
> The error information for bgp was commited in 2009
> (bgpnotify_minor_cease, bgpnotify_minor_cap) but never enabled, so do
> that here. Also add FSM error codes.
perhaps add a comment like /* RFC 6608 */ above the below:
> +static const char *bgpnotify_minor_fsm[] = {
> + NULL, "In OpenSent State", "In OpenConfirm State",
> + "In Established State",
> +};
and maybe s/NULL/"Unspecified Error"/
Kind regards,
Job
