summaryrefslogtreecommitdiff
path: root/bgpd
AgeCommit message (Collapse)Author
2011-03-22bgpd: attribute jhash call should use a standard interface to in6_addr dataStephen Hemminger
* bgp_attr.c; (attrhash_key_make) s6_addr is only member of in6_addr guaranteed to be available - s6_addr32 isn't. Fix to be more portable, and thus allow compilation on BSD again.
2011-03-21bgpd: enable TCP corkingStephen Hemminger
* bgp_packet.c: (bgp_write) On BGP write, use TCP_CORK to provide hints to kernel about TCP buffering. This will cause BGP packets to occur in bigger chunks (full size MTU), improving performance and getting rid of one of the problems reported in the UNH BGP conformance test.
2011-03-21bgpd: leave peer socket in non-blocking mode (mostly)Stephen Hemminger
* bgpd: Rather than toggling socket in/out of non-block mode, just leave it in nonblocking mode. One exception is in bgp_notify which only happens just before close.
2011-03-21bgpd: "Intern" communities in route mapsMichael Lambert
* bgp_community.[ch]: (community_lookup) New helper function to look up a community list in the hash table. * bgp_routemap.c: A new community structure was being allocated for every BGP update which matched a route map which set a community. This behavior led to rapid growth in the memory consumed by bgpd. Adding the communities to the hash table addresses the memory growth, but may introduce a problem in modifying or deleting the 'set community' statement in the route map.
2011-03-21bgpd: New show commands for improved view and address family supportMichael Lambert
Many show commands do not have support for multiple views and do not treat different address families uniformly. The following changes add a number of commands with support for views and rationalized treatment of IPv4 v IPv6 and unicast v multicast (such as in JUNOS, IOS XR and more recent versions of IOS). * bgp_route.c: (bgp_show_community) Inserted a new second argument (the name of the view) and the code to look up that name in the BGP structure. The NULL argument in the call to bgp_show (indicating the default view) was replaced by the specified view. The existing calls to bgp_show_community had a NULL second argument inserted to make clear that they refer to the default view. (top level) Added new functions via the DEFUN and/or ALIAS macros (and the associated command table entries) to add the commands show bgp ipv4 (unicast|multicast) show bgp ipv4 (unicast|multicast) A.B.C.D show bgp ipv4 (unicast|multicast) A.B.C.D/M show bgp ipv6 (unicast|multicast) show bgp ipv6 (unicast|multicast) X:X::X:X show bgp ipv6 (unicast|multicast) X:X::X:X/M These show either the full BGP table or the specified route or prefix for the given address family. show bgp view WORD (ipv4|ipv6) (unicast|multicast) community show bgp view WORD (ipv4|ipv6) (unicast|multicast) community \ (AA:NN|local-AS|no-advertise|no-export){1,4} For the specified view and address family, these show entries matching any community or the specified communit(y)(ies). show bgp view WORD (ipv4|ipv6) (unicast|multicast) neighbors \ (A.B.C.D|X:X::X:X) (advertised-routes|received-routes) For the specified view and address family, show the routes advertised to or received from the given BGP neighbor. show bgp [view WORD] ipv4 (unicast|multicast) \ rsclient (A.B.C.D|X:X::X:X) show bgp [view WORD] ipv4 (unicast|multicast) \ rsclient (A.B.C.D|X:X::X:X) A.B.C.D show bgp [view WORD] ipv4 (unicast|multicast) \ rsclient (A.B.C.D|X:X::X:X) A.B.C.D/M show bgp [view WORD] ipv6 (unicast|multicast) \ rsclient (A.B.C.D|X:X::X:X) show bgp [view WORD] ipv6 (unicast|multicast) \ rsclient (A.B.C.D|X:X::X:X) X:X::X:X show bgp [view WORD] ipv6 (unicast|multicast) \ rsclient (A.B.C.D|X:X::X:X) X:X::X:X/M For the specifed (optional) view and address family, show either the full BGP table or the specified route or prefix for the given route server client peer. * bgp_vty.c: (top level) Added new functions via the DEFUN and/or ALIAS macros (and the associated command table entries) to add the commands show bgp [view WORD] (ipv4|ipv6) (unicast|multicast) summary show bgp [view WORD] (ipv4|ipv6) (unicast|multicast) rsclient summary For the specified (optional) view and address family, display either the normal summary table for BGP peers, or the route server client table showing the import and export policies.
2011-03-21bgpd: Implement revised error handling for partial optional/trans. attributesPaul Jakma
* BGP error handling generally boils down to "reset session". This was fine when all BGP speakers pretty much understood all BGP messages. However the increasing deployment of new attribute types has shown this approach to cause problems, in particular where a new attribute type is "tunneled" over some speakers which do not understand it, and then arrives at a speaker which does but considers it malformed (e.g. corruption along the way, or because of early implementation bugs/interop issues). To mitigate this drafts before the IDR (likely to be adopted) propose to treat errors in partial (i.e. not understood by neighbour), optional transitive attributes, when received from eBGP peers, as withdrawing only the NLRIs in the affected UPDATE, rather than causing the entire session to be reset. See: http://tools.ietf.org/html/draft-scudder-idr-optional-transitive * bgp_aspath.c: (assegments_parse) Replace the "NULL means valid, 0-length OR an error" return value with an error code - instead taking pointer to result structure as arg. (aspath_parse) adjust to suit previous change, but here NULL really does mean error in the external interface. * bgp_attr.h (bgp_attr_parse) use an explictly typed and enumerated value to indicate return result. (bgp_attr_unintern_sub) cleans up just the members of an attr, but not the attr itself, for benefit of those who use a stack-local attr. * bgp_attr.c: (bgp_attr_unintern_sub) split out from bgp_attr_unintern (bgp_attr_unintern) as previous. (bgp_attr_malformed) helper function to centralise decisions on how to handle errors in attributes. (bgp_attr_{aspathlimit,origin,etc..}) Use bgp_attr_malformed. (bgp_attr_aspathlimit) Subcode for error specifc to this attr should be BGP_NOTIFY_UPDATE_OPT_ATTR_ERR. (bgp_attr_as4_path) be more rigorous about checks, ala bgp_attr_as_path. (bgp_attr_parse) Adjust to deal with the additional error level that bgp_attr_ parsers can raise, and also similarly return appropriate error back up to (bgp_update_receive). Try to avoid leaking as4_path. * bgp_packet.c: (bgp_update_receive) Adjust to deal with BGP_ATTR_PARSE_WITHDRAW error level from bgp_attr_parse, which should lead to a withdraw, by making the attribute parameter in call to (bgp_nlri_parse) conditional on the error, so the update case morphs also into a withdraw. Use bgp_attr_unintern_sub from above, instead of doing this itself. Fix error case returns which were not calling bgp_attr_unintern_sub and probably leaking memory. * tests/aspath_test.c: Fix to work for null return with bad segments
2011-03-21bgpd: Remove AS Path limit/TTL functionalityPaul Jakma
* draft-ietf-idr-as-pathlimit doesn't seem to have gone anywhere, and its author does not think it will make progress in IDR. Remove all support introduced for it, but leave stubs for the commands to avoid breaking any configurations. Basically reverts cecab5e9725792e60a5e4b473e238a14cd85815d.
2011-03-21bgpd: Try fix extcommunity resource allocation probs, particularly with 'set ↵Paul Jakma
extcom..' * Extended communities has some kind of resource allocation problem which causes a double-free if the 'set extcommunity ...' command is used. Try fix by properly interning extcommunities. Also, more generally, make unintern functions take a double pointer so they can NULL out callers references - a usefully defensive programming pattern for functions which make refs invalid. Sadly, this patch doesn't fix the problem entirely - crashes still occur on session clear. * bgp_ecommunity.h: (ecommunity_{free,unintern}) take double pointer args. * bgp_community.h: (community_unintern) ditto * bgp_attr.h: (bgp_attr_intern) ditto * bgp_aspath.h: (bgp_aspath.h) ditto * (general) update all callers of above * bgp_routemap.c: (route_set_ecommunity_{rt,soo}) intern the new extcom added to the attr, and unintern any old one. (route_set_ecommunity_{rt,soo}_compile) intern the extcom to be used for the route-map set. (route_set_ecommunity_*_free) unintern to match, instead of free (route_set_ecommunity_soo) Do as _rt does and don't just leak any pre-existing community, add to it (is additive right though?)
2011-03-21bgpd: Rollback some of the changes made for invalid AS_PATH segment fixPaul Jakma
Some of the changes made in commit cddb8112b80fa9867156c637d63e6e79eeac67bb don't work particularly well for other changes that need to be made to address BGP attribute error handling problems. In particular, returning a pointer from complex attribute data parsing functions will not suffice to express the require range of return status conditions. * bgp_aspath.c: (assegments_parse) Rollback to a more minimal set of changes to fix the original problem. (aspath_parse) Slightly needless pushing around of code, and taking 2 parameters to say whether ot use 2 or 4 byte encoding seems unnecessary. * bgp_attr.c: (bgp_attr_as{,4}path) Rollback, in preparation for BGP attribute error handling update.
2011-03-21bgpd/security: CVE-2010-1674 Fix crash due to extended-community parser errorPaul Jakma
* bgp_attr.c: (bgp_attr_ext_communities) Certain extended-community attrs can leave attr->flag indicating ext-community is present, even though no extended-community object has been attached to the attr structure. Thus a null-pointer dereference can occur later. (bgp_attr_community) No bug fixed here, but tidy up flow so it has same form as previous. Problem and fix thanks to anonymous reporter.
2011-03-21bgpd: use Jenkins hash for BGP transit, cluster and attr hashesStephen Hemminger
* bgp_attr.c: I observed while doing some debugging that even for simple tests there was a lot of hash collisions for BGP attributes. Switch to using Jhash rather than additive hashing. Probably overkill, but the function is fast and available. ({attrhash,cluster,transit}_hask_key_make) convert to Jenkins hash, instead of additive hash.
2011-03-21bgpd: Remove extra lock on interior table nodeBarry Friedman
If the radix tree creates an extra interior node in bgp_node_get(), it locks the interior node even though this node is not returned to the caller, so it may never be unlocked. The lock prevents this node from being deleted. * bgpd/bgp_table.c: (bgp_node_get) Remove lock on interior node which prevents proper node deletion
2011-03-21bgpd: Fix display of unsigned attributesWataru Tanitsu
* bgp_route.c: (route_vty_out*) The local prefix, metric and weight values are all stored as uint32_t. Change the format to %u so that large values are not displayed as negative integers.
2011-03-21bgpd: fix use of free memory by update_rsclientStephen Hemminger
* bgp_route.c: (bgp_static_update_rsclient) BGP sometimes crashes when removing route server client because of use after free. The code to update rsclient created a local static copy of bgp attributes but neglected to handle the extra information pointer. The extra information was getting freed by bgp_attr_unintern() and reused later when the copy was passed to bgp_attr_intern(). The fix is to use the attr_dup function to create a copy of the extra information, then clean it up.
2011-03-21bgpd: unlock node on aggregate errorRobert Bays
* bgp_route.c: (bgp_aggregate_set) make sure to unlock BGP node if failure
2011-03-21bgpd: fix errors in aggregate address commandRobert Bays
* bgpd: (bgp_aggregate_{set,unset,delete}) This fixes locking and other issues with aggregate set/unset command
2011-03-21bgpd: use XCALLOC to allocate bgpd damp arrayStephen Hemminger
* bgpd: (bgp_damp_parameter_set) The BGP reuse_index is not initialized properly. This would cause sporadic crash when disabling dampening. Use XCALLOC correctly and the right size array is initialized and no memset is needed.
2011-03-21bgpd: fix bgp_node locking issuesChris Caputo
* bgpd: Connected table locks were being locked but not unlocked, such that eventually a lock would exceed 2^31 and become negative, thus triggering an assert later on. * bgp_main.c: (bgp_exit) delete connected elements along with ifp's. * bgp_nexthop.c: (bgp_nexthop_lookup{,_ipv6}) add missing unlocks (bgp_multiaccess_check_v4) ditto (bgp_connected_{add,delete}) Use a distinct memtype for bgp_connected_ref. (bgp_scan_finish) reset the nexthop cache to clean it up when bgpd exits * bgp_route.c: fix missing bgp_node unlocks * lib/memtype.c: (memory_list_bgp) add MTYPE_BGP_CONN * testing: has been tested for almost 2 months now.
2011-03-20Merge paul/ospfd/201012-review ospfd and lib/ fixes and performance improvementsPaul Jakma
2011-03-18bgpd: improve "monotonic" uptime correctionJohn Kemp
Older versions of Quagga/Zebra would output a value in MRT table dump files for "uptime" aka "ORIGINATED" that was a WALL clock value. Given that uptime is now internally a bgp_clock MONOTONIC value, the output in the MRT files is showing up as monotonic. Note: time of MRT dump is still recorded correctly as a time() based value, so we haven't lost that value. Proposal is to correct the uptime output on the vty and in the MRT files to again display something more akin to WALL time. * bgp_dump.c: (bgp_dump_routes_func) add conditional correction * bgp_route.c: (route_vty_out_detail) make correction conditional, move variable declaration to beginning of the function
2011-01-17bgpd: VTY string fixes for debug commandsDavid Ward
* bgpd/bgp_debug.c: fix VTY strings for BGP debug commands to match correct syntax
2011-01-14bgpd: fix handling of "Unsupported Capability"Dmitrij Tejblum
* bgp_packet.c: (bgp_notify_receive) justify the difference between BGP_NOTIFY_OPEN_UNSUP_PARAM and BGP_NOTIFY_OPEN_UNSUP_CAPBL cases, as it is explained in RFC5492, page 3, paragraph 1. "Unsupported Capability" error does not mean, that the peer doesn't support capabilities advertisement -- quite the opposite (if the peer would not support capabilities advertisement, the code would be "Unsupported Optional Parameter"). Thus there is no reason to mark the peer as one non-supporting capabilities advertisement. Example: suppose the peer is in fact IPv6-only, but we didn't configure anything address-family specific for it. Then, the peer would refuse the session with "Unsupported Capability" code. If we internally set the peer as non-supporting capabilities advertisement after that, we will not be able to establish the session with it ever, even with a fixed configuration -- IPv6-only BGP session cannot be established without capabilities. In practice an edge case would be seen as the same IPv6 peer working with its "neighbor" block read from bgpd.conf, but not working, when slowly input in "conf t" mode.
2010-12-08bgpd: Simplify process queue initPaul Jakma
* bgp_route.c: (bgp_process_queue_init) rsclient wasn't getting all the same things initialised as the main queue. Simplify to make it more robust.
2010-12-08bgpd: fix community-list error message spellingDenis Ovsienko
* bgp_vty.c: (community_list_perror, show_ip_community_list_arg, show_ip_extcommunity_list_arg) fix spelling
2010-10-21bgpd: fix printed value of last-update timestampVladimir L Ivanov
* bgp_route.c: (route_vty_out_detail) calculate time value in a way, which works regardless of monotonic clock being used or not
2010-10-06bgpd, lib: adopt afi_t and safi_t in several placesMichael Lambert
* bgpd/bgp_attr.c, bgpd/bgp_open.h, bgpd/bgp_route.c, lib/prefix.c, lib/prefix.h: Various integer types were being used where, if we had strict type checking, afi_t and safi_t would be required. Signed-off-by: G.Balaji <balajig81@gmail.com> (cherry picked from commit c8af35ffa2dc79ff7d7ff00b1b61f1f50a100ab6)
2010-09-17Set from even if binfo->extra is NULL.Greg Troxel
bgpd/bgp_packet.c:bgp_update_packet(): When extracting the peer, don't fail to extract it because "binfo->extra" is NULL. While one should certainly avoid dereferencing binfo->extra, that's not a good reason not to use binfo->peer. Fixes https://bugzilla.quagga.net/show_bug.cgi?id=497. Patch by Eric Sobocinksi.
2010-08-09bgpd: fix handling of AS path dataChris Hall
* bgpd/bgp_aspath.c * assegments_parse(): add handling of AS4_PATH input, update bounds checks, add check for AS segment type * aspath_parse(): add handling of AS4_PATH input, expect assegments_parse() to do length checking * aspath_empty(): update for the new function prototype * bgpd/bgp_aspath.h: ditto * tests/aspath_test.c: ditto * bgpd/bgp_attr.c * bgp_attr_aspath(): add handling of AS4_PATH input, update flags checks, change returned type * bgp_attr_as4_path(): discard, superseded by bgp_attr_aspath() * bgp_attr_parse(): update respectively
2010-05-14bgpd: tighten bounds checking in RR ORF msg readerChris Hall
* bgp_packet.c: (bgp_route_refresh_receive) add validation of "Length" (RFC5292) field value, check input stream bounds each time bytes are pulled from it
2010-01-15bgp: use monotonic clock for time of dayStephen Hemminger
BGP uses time() to get system time of day; but that value fluctuates with time adjustments from NTP. This can cause premature flapping of peer sessions and other failures. Use the system monotonic clock supported by Quagga thread library to avoid issue. See: http://bugzilla.vyatta.com/show_bug.cgi?id=4467 * bgpd/bgp_fsm.c * bgp_uptime_reset(): dismiss function * bgpd/bgpd.c * bgp_clock(): new function * bgpd/bgp_damp.c * bgp_reuse_timer(): employ bgp_clock() instead of time(NULL) * bgp_damp_withdraw(): idem * bgp_damp_update(): idem * bgp_damp_scan(): idem * bgp_damp_info_vty(): idem * bgp_damp_reuse_time_vty(): idem * bgpd/bgp_fsm.c * bgp_routeadv_timer(): idem * bgp_stop(): idem * bgp_establish(): idem * bgpd/bgp_packet.c * bgp_update_receive(): idem * bgpd/bgp_route.c * bgp_update_rsclient(): idem * bgp_update_main(): idem * bgp_static_update_rsclient(): idem * bgp_static_update_main(): idem * bgp_static_update_vpnv4(): idem * bgp_aggregate_route(): idem * bgp_aggregate_add(): idem * bgp_redistribute_add(): idem * bgpd/bgp_snmp.c * bgpPeerTable(): idem * bgpTrapEstablished(): idem * bgpTrapBackwardTransition(): idem * bgpd/bgpd.c * peer_create(): idem * peer_uptime(): idem * bgp_master_init(): idem
2009-12-17bgpd: code cleanupStephen Hemminger
* bgpd/bgp_aspath.c * ashash: only used in one file, make static * aspath_count_numas(): dead code, sayonara * bgpd/bgpd.c * peer_nsf_stop(): only used in one file, make static * bgpd/bgp_packet.h * bgp_capability_receive(): add missing prototype for a global function
2009-12-10bgpd: compile warnings cleanupStephen Hemminger
* bgpd/bgp_fsm.c * bgp_clearing_completed(): only used in one file, can be static * bgpd/bgp_packet.c * afi2str(): sayonara * safi2str(): sayonara * bgpd/bgp_route.c * bgp_distance_reset(): sayonara * bgpd/bgp_zebra.c * bgp_ifindex_by_nexthop(): sayonara
2009-12-09lib: move check_bit into prefix common codeStephen Hemminger
Make one version of check prefix bit, and put it inline with proper prototype. This gets rid of some macro's and also some assert() that can never happen on a non-broken compiler. * bgpd/bgp_table.c * CHECK_BIT(): sayonara * check_bit(): sayonara * SET_LINK(): sayonara * set_link(): make use of prefix_bit() instead of check_bit() * bgp_node_match(): idem * bgp_node_lookup(): idem * bgp_node_get(): idem * lib/prefix.h * prefix_bit(): new inline version of check_bit() * lib/table.c * CHECK_BIT(): sayonara * check_bit(): sayonara * SET_LINK(): sayonara * set_link(): make use of prefix_bit() instead of check_bit() * route_node_match(): idem * route_node_lookup(): idem * route_node_get(): idem * ospf6d/ospf6_lsdb.c * CHECK_BIT(): sayonara * ospf6_lsdb_lookup_next(): make use of prefix_bit() instead of CHECK_BIT() * ospf6_lsdb_type_router_head(): idem * ospf6_lsdb_type_head(): idem * ospf6d/ospf6_route.c * CHECK_BIT(): sayonara * ospf6_route_match_head() make use of prefix_bit() instead of * CHECK_BIT()
2009-12-04bgpd: work around warning in assegments_parse()Denis Ovsienko
2009-08-28bgpd: fix md5 set on listen socketsStephen Hemminger
* bgp_network.c: (bgp_md5_set) Missing piece from earlier listener change did not get ported from Vyatta code into upstream. The list listener_sockets changed from (int *) to (struct bgp_listener *).
2009-08-13bgp: missing pieces from listener patchStephen Hemminger
* bgp_network.c: (bgp_accept) The code in current git will crash as part of the revised listener code is missing. The new listener thread code passes a pointer to a bgp_listener structure, not the bgp pointer. The old code always got a NULL for bgp pointer, so that is now hard coded.
2009-08-13bgp: compiler warning fixStephen Hemminger
* bgp_filter.h: Gcc complains the function prototype is not correct because the function argument is using old K&R style.
2009-08-05bgpd: Fix mistakes in applying 'allow inbound connections to non-default view'Paul Jakma
* bgpd.c: (peer_lookup_with_open) Bodged application of previous patch meant the second loop around bgp->peer wasn't included in the loop around bm->bgp as it was supposed to be. Fix..
2009-07-30[bgpd] delete erroneous extra brace..Paul Jakma
2009-07-30bgpd: Workaround for invalid MBGP next hopMichael Lambert
* bgp_attr.c: (bgp_mp_reach_parse) There are some interoperability issues for MBGP (particularly IPv4 multicast NLRI) between different implementations. In order to get some next hops to install correctly in the BGP tables, it appears to be necessary to copy the multiprotocol next hop into the base next hop field. This is related to differences in RFC 2283 and RFC 2858.
2009-07-28bgpd/trivial: Fix indentation in previousPaul Jakma
2009-07-28bgpd: Allow inbound connections to non-default viewSteve Hill
* bgpd.c: (peer_lookup) Search through all BGP instances for matches, not just the default instance, if no specific instance is given. (peer_lookup_with_open) same.
2009-07-28bgpd: fd leak in bgpdSteve Hill
* bgp_fsm.c: I have found an fd leak in bgpd that is caused by the 'new' Clearing state. I've been seeing it from hold timer failures, but it can also be triggered by other things. When Hold_Timer_expired fires in Established state, a notify is sent and BGP_Stop event queued. The fsm then transitions into Clearing state. That is the problem; When the BGP_Stop event is serviced, the state table says to ignore it while in Clearing. Thus bgp_stop is not called and the fd leaks. Previously the peer would be in Idle state, which correctly handles the BGP_Stop event. Fix by making bgp_stop safe to call from Clearing state, without losing ClearingCompleted events, and then ensuring it is called prior to transition from Clearing->Idle.
2009-07-28bgpd: Delete AS_CONFED_SEQUENCE when prepending an AS_SEQUENCE type segmentVasilis Tsiligiannis
AS_CONFED_SEQUENCE segment should always be deleted when prepending (e.g. with a route-map) an AS_SEQUENCE segment to an AS path. Otherwise, AS_CONFED_SEQUENCE will not be deleted later when updating EBGP peers (since it is not the leftmost segment) and will leak outside the confederation.
2009-07-28bgpd: Implement BGP confederation error handling (RFC5065, Par. 5)Vasilis Tsiligiannis
This patch implements BGP confederation error handling in Quagga as described in RFC5065, paragraph 5. * bgp_aspath.c: (aspath_confed_check, aspath_left_confed_check) new functions * bgp_attr.c: (bgp_attr_aspath_check) apply previous and NOTIFY if there's a problem.
2009-07-28bgpd: start listener on first instanceStephen Hemminger
Start BGP listener only after first instance is started. This helps the security if BGP is not used but daemon is started. It also addresses some issues like MD5 not working on listener unless IPV6 configured (because listener was not in list); as well as compiler warnings. * bgp_network.c: (bgp_listener) listen socket creation consolidated here (bgp_socket) Use bgp_listener * bgpd.c: (bgp_get) call bgp_socket on creation of first struct bgp. (bgp_init) remove bgp_socket call. * memtypes.c: Add MTYPE_BGP_LISTENER
2009-07-28bgpd: peer action table static/constStephen Hemminger
bgpd.c: (various tables) only used in one place and are immutable.
2009-07-27[bgpd] Restore ability of 'neighbor ... update-source' to take interface namePaul Jakma
* bgp_vty.c: (BGP_UPDATE_SOURCE_STR) Restore ability to accept arbitrary interface names for the 'neighbor ... update-source' command - shouldn't have been deleted. (BGP_UPDATE_SOURCE_STR) Add help for same.
2009-07-21[bgp] Fix nexthop reachability check on confederationsVasilis Tsiligiannis
* bgp_route.c: (bgp_update_main) Nexthop reachability should be checked for confederations too in case a prefix is received from more than one confederation peers.
2009-07-21[bgpd] fix typo made by paul in previous commitPaul Jakma