Re: [Bgpd-users] Patch for BGP/PathAttribute [bgpd.pl-0.06]
Status: Alpha
Brought to you by:
stevenh
|
From: Steven H. <st...@xs...> - 2003-02-25 21:13:27
|
Hi Terra,
Thanks for the bug report! We'll need to remove the initial unpack in the
assigment to $originator_id. The same bug re-occurs at the AGGREGATOR and
ADVERTISER path attributes. Values should be stored in network byte order
and converted to strings for logging purposes.
I think I've fixed this in 0.07, for now the patch is included below.
Unfortunately I can't test it right now but you're free to let me know
whether the patch works for you :-)
0.07 won't be released until I've chased another bug report I received last
week (keep them coming, it's nice to know that people are using the software!)
- Steven
Here's the patch that will be in 0.07:
diff -u -r bgpd.pl-0.06/BGP/PathAttribute.pm bgpd.pl-0.07/BGP/PathAttribute.pm
--- bgpd.pl-0.06/BGP/PathAttribute.pm 2002-10-01 07:03:13.000000000 +0200
+++ bgpd.pl-0.07/BGP/PathAttribute.pm 2003-02-25 06:51:50.000000000 +0100
@@ -419,7 +419,7 @@
my $next_hop = substr($buff, $bytes_read,
$attribute_length);
# TODO: check $next_hop on both syntactic as symantic
# correctness and add the different error responses.
- my $nh_ip = inet_ntoa (substr($buff, $bytes_read, 4));
+ my $nh_ip = inet_ntoa ($next_hop);
# This is a well-known mandatory attribute
if (defined $attribute_flag{optional}
|| !defined
$attribute_flag{transitive}) {
@@ -502,8 +502,7 @@
}
my $aggr_as = unpack('n',
substr($buff,$bytes_read, 2));
- my $aggr_host = unpack('N',
- substr($buff,$bytes_read + 2, 4));
+ my $aggr_host = substr($buff,$bytes_read + 2, 4);
# This is a well-known discretionary attribute
according to RFC1771
# Cisco sends it as an optional attribute so
that's how we accept it
if (!defined $attribute_flag{optional} ||
@@ -516,8 +515,7 @@
BGP_ERRSC_Attribute_Length_Error,
$attribute);
}
$pa->set_aggregator ($aggr_as, $aggr_host);
- my $aggr_host_ip = inet_ntoa
- (substr($buff, $bytes_read + 2, 4));
+ my $aggr_host_ip = inet_ntoa ($aggr_host);
$bytes_read += 6;
$neighbor->log (2, 256,
"AGGREGATOR AS: $aggr_as, host:
$aggr_host_ip");
@@ -543,6 +541,7 @@
$neighbor->log (2, 256, "Communities:",
@communities);
}
+ # RFC2796: ORIGINATOR_ID, type code 9
if ($attribute_type_code ==
BGP_UPDATE_Path_Attribute_ORIGINATOR_ID) {
$attribute .= substr($buff, $bytes_read,
$attribute_length);
@@ -557,7 +556,7 @@
return (BGP_ERRC_UPDATE_Message_Error,
BGP_ERRSC_Attribute_Length_Error,
$attribute);
}
- my $originator_id = unpack ('N', substr ($buff,
$bytes_read, 4));
+ my $originator_id = substr($buff, $bytes_read, 4);
$pa->set_originatorid ($originator_id);
$bytes_read += $attribute_length;
@@ -577,12 +576,12 @@
return (BGP_ERRC_UPDATE_Message_Error,
BGP_ERRSC_Attribute_Length_Error,
$attribute);
}
- my @cluster_list = unpack ('N*', substr($buff,
$bytes_read,
+ my @cluster_list = unpack ('L*', substr($buff,
$bytes_read,
$attribute_length));
$bytes_read += $attribute_length;
$pa->set_clusterlist (\@cluster_list);
$neighbor->log (2, 256, "Cluster list:",
- @cluster_list);
+ unpack('N*', substr($buff, $bytes_read,
$attribute_length)));
}
if ($attribute_type_code ==
BGP_UPDATE_Path_Attribute_ADVERTISER) {
$attribute .= substr($buff, $bytes_read,
$attribute_length);
@@ -597,9 +596,9 @@
return (BGP_ERRC_UPDATE_Message_Error,
BGP_ERRSC_Attribute_Length_Error,
$attribute);
}
- my $advertiser = unpack ('N', substr ($buff,
$bytes_read, 4));
+ my $advertiser = substr ($buff, $bytes_read, 4);
- $pa->set_advertiserr ($advertiser);
+ $pa->set_advertiser ($advertiser);
$bytes_read += $attribute_length;
$neighbor->log (2, 256, "Advertiser:",
inet_ntoa($advertiser));
diff -u -r bgpd.pl-0.06/CHANGES bgpd.pl-0.07/CHANGES
--- bgpd.pl-0.06/CHANGES 2002-10-01 07:04:04.000000000 +0200
+++ bgpd.pl-0.07/CHANGES 2003-02-25 06:40:20.000000000 +0100
@@ -1,3 +1,6 @@
+- Removed unpack ORIGNATOR_ID path attribute, the value should be stored
+ in network byte order (Report by Terra (mli...@Fu...)
+
RELEASE bgpd.pl-0.06 released on October 1st 2002
- Fixes provided by Andrew of Supernews <an...@su...> for the
route selection algorithm used to select for a route announced by two or
At 12:39 PM 2/18/2003 -0500, Terra wrote:
>Greetings,
>
>The players:
>1) zebra-0.93b
>2) bgpd-0.6
>
>Upon receiving an attribute_type_code of
>'BGP_UPDATE_Path_Attribute_ORIGINATOR_ID', an error is quickly hit on line: 564
>
>Error:
>Bad arg length for Socket::inet_ntoa, length is 10, should be 4 at
>BGP/PathAttribute.pm line 564
>
> From a quick patch:
>printf("OrigID: %d, %d\n", $originator_id, length($originator_id));
>
>returns:
>OrigID: 1117200388, 10
>
>I went ahead and decided to reverse the prior unpack, instead of stuffing
>away the substr, then unpack step after...
>Patch to repair:
>--- bgpd.pl-0.06/BGP/PathAttribute.pm-virgin Tue Oct 1 01:03:13 2002
>+++ bgpd.pl-0.06/BGP/PathAttribute.pm Tue Feb 18 12:27:56 2003
>@@ -562,7 +562,7 @@
> $pa->set_originatorid ($originator_id);
> $bytes_read += $attribute_length;
> $neighbor->log (2, 256, "Originator ID:",
>- inet_ntoa($originator_id));
>+ inet_ntoa(pack('N',$originator_id)));
> }
> if ($attribute_type_code ==
> BGP_UPDATE_Path_Attribute_CLUSTER_LIST) {
> $attribute .= substr($buff, $bytes_read,
> $attribute_length);
>
>
>Thanks Steven for maintaining a wonderful base of perl, that can slice and
>dice raw BGP packets, for which has helped me to understand the BGP4
>protocol in much more depth and detail... It is also proven itself to be
>a terrific debugging aid, especially when your eBGP peers refuse to
>provide looking glass services to their clients...
>
>--
>Terra
>sysAdmin
>FutureQuest
>
>
>
>-------------------------------------------------------
>This sf.net email is sponsored by:ThinkGeek
>Welcome to geek heaven.
>http://thinkgeek.com/sf
>_______________________________________________
>Bgpd-users mailing list
>Bgp...@li...
>https://lists.sourceforge.net/lists/listinfo/bgpd-users
|