diff options
| -rw-r--r-- | HACKING | 357 | ||||
| -rw-r--r-- | HACKING.pending | 5 | ||||
| -rw-r--r-- | HACKING.tex | 462 | ||||
| -rw-r--r-- | Makefile.am | 10 | ||||
| -rw-r--r-- | bgpd/bgp_attr.c | 997 | ||||
| -rw-r--r-- | bgpd/bgp_attr.h | 39 | ||||
| -rw-r--r-- | bgpd/bgp_open.c | 1 | ||||
| -rw-r--r-- | bgpd/bgp_packet.c | 224 | ||||
| -rwxr-xr-x | configure.ac | 12 | ||||
| -rw-r--r-- | doc/basic.texi | 4 | ||||
| -rw-r--r-- | lib/pthread_safe.c | 2 | ||||
| -rw-r--r-- | lib/thread.c | 8 | ||||
| -rw-r--r-- | ospfd/ospf_packet.c | 270 | ||||
| -rw-r--r-- | tests/aspath_test.c | 21 | ||||
| -rw-r--r-- | tests/bgp_mp_attr_test.c | 27 | ||||
| -rw-r--r-- | tools/multiple-bgpd.sh | 3 |
16 files changed, 1223 insertions, 1219 deletions
diff --git a/HACKING b/HACKING deleted file mode 100644 index 0358fed2..00000000 --- a/HACKING +++ /dev/null @@ -1,357 +0,0 @@ --*- mode: text; -*- -$QuaggaId: Format:%an, %ai, %h$ $ - -Contents: - -* GUIDELINES FOR HACKING ON QUAGGA -* COMPILE-TIME CONDITIONAL CODE -* COMMIT MESSAGE -* HACKING THE BUILD SYSTEM -* RELEASE PROCEDURE -* SHARED LIBRARY VERSIONING -* RELEASE PROCEDURE -* TOOL VERSIONS -* SHARED LIBRARY VERSIONING -* PATCH SUBMISSION -* PATCH APPLICATION -* STABLE PLATFORMS AND DAEMONS -* IMPORT OR UPDATE VENDOR SPECIFIC ROUTING PROTOCOLS - - -GUIDELINES FOR HACKING ON QUAGGA - -[this is a draft in progress] - -GNU coding standards apply. Indentation follows the result of -invoking GNU indent (as of 2.2.8a) with no arguments. Note that this -uses tabs instead of spaces where possible for leading whitespace, and -assumes that tabs are every 8 columns. Do not attempt to redefine the -location of tab stops. Note also that some indentation does not -follow GNU style. This is a historical accident, and we generally -only clean up whitespace when code is unmaintainable due to whitespace -issues, as fewer changes from zebra lead to easier merges. - -For GNU emacs, use indentation style "gnu". - -For Vim, use the following lines (note that tabs are at 8, and that -softtabstop sets the indentation level): - -set tabstop=8 -set softtabstop=2 -set shiftwidth=2 -set noexpandtab - -Be particularly careful not to break platforms/protocols that you -cannot test. - -New code should have good comments, and changes to existing code -should in many cases upgrade the comments when necessary for a -reviewer to conclude that the change has no unintended consequences. - -Each file in the Git repository should have a git format-placeholder (like -an RCS Id keyword), somewhere very near the top, commented out appropriately -for the file type. The placeholder used for Quagga (replacing <dollar> with -$) is: - - $QuaggaId: <dollar>Format:%an, %ai, %h<dollar> $ - -See line 2 of HACKING for an example; - -This placeholder string will be expanded out by the 'git archive' commands, -wihch is used to generate the tar archives for snapshots and releases. - -Please document fully the proper use of a new function in the header file -in which it is declared. And please consult existing headers for -documentation on how to use existing functions. In particular, please consult -these header files: - - lib/log.h logging levels and usage guidance - [more to be added] - -If changing an exported interface, please try to deprecate the interface in -an orderly manner. If at all possible, try to retain the old deprecated -interface as is, or functionally equivalent. Make a note of when the -interface was deprecated and guard the deprecated interface definitions in -the header file, ie: - -/* Deprecated: 20050406 */ -#if !defined(QUAGGA_NO_DEPRECATED_INTERFACES) -#warning "Using deprecated <libname> (interface(s)|function(s))" -... -#endif /* QUAGGA_NO_DEPRECATED_INTERFACES */ - -To ensure that the core Quagga sources do not use the deprecated interfaces -(you should update Quagga sources to use new interfaces, if applicable) -while allowing external sources to continue to build. Deprecated interfaces -should be excised in the next unstable cycle. - -Note: If you wish, you can test for GCC and use a function -marked with the 'deprecated' attribute. However, you must provide the -#warning for other compilers. - -If changing or removing a command definition, *ensure* that you properly -deprecate it - use the _DEPRECATED form of the appropriate DEFUN macro. This -is *critical*. Even if the command can no longer function, you *must* still -implement it as a do-nothing stub. Failure to follow this causes grief for -systems administrators. Deprecated commands should be excised in the next -unstable cycle. A list of deprecated commands should be collated for each -release. - -See also below regarding SHARED LIBRARY VERSIONING. - - -COMPILE-TIME CONDITIONAL CODE - -Please think very carefully before making code conditional at compile time, -as it increases maintenance burdens and user confusion. In particular, -please avoid gratuitious --enable-.... switches to the configure script - -typically code should be good enough to be in Quagga, or it shouldn't be -there at all. - -When code must be compile-time conditional, try have the compiler make it -conditional rather than the C pre-processor. I.e. this: - - if (SOME_SYMBOL) - frobnicate(); - -rather than: - - #ifdef SOME_SYMBOL - frobnicate (); - #endif /* SOME_SYMBOL */ - -Note that the former approach requires ensuring that SOME_SYMBOL will be -defined (watch your AC_DEFINEs). - - -COMMIT MESSAGES - -The commit message should provide: - -* A suitable one-line summary followed by a blank line as the very - first line of the message, in the form: - - topic: high-level, one line summary - - Where topic would tend to be name of a subdirectory, and/or daemon, unless - there's a more suitable topic (e.g. 'build'). This topic is used to - organise change summaries in release announcements. - -* An optional introduction, discussing the general intent of the change. -* A short description of each change made, preferably: - * file by file - * function by function (use of "ditto", or globs is allowed) - -to provide a short description of the general intent of the patch, in terms -of the problem it solves and how it achieves it, to help reviewers -understand. - -The one-line summary must be limited to 54 characters, and all other -lines to 72 characters. - -The reason for such itemised commit messages is to encourage the author to -self-review every line of the patch, as well as provide reviewers an index -of which changes are intended, along with a short description for each. -Some discretion is obviously required. A C-to-english description is not -desireable. For short patches, a per-function/file break-down may be -redundant. For longer patches, such a break-down may be essential. - -An example (where the general discussion is obviously somewhat redundant, -given the one-line summary): - -zebra: Enhance frob FSM to detect loss of frob - -* (general) Add a new DOWN state to the frob state machine - to allow the barinator to detect loss of frob. -* frob.h: (struct frob) Add DOWN state flag. -* frob.c: (frob_change) set/clear DOWN appropriately on state change. -* bar.c: (barinate) Check frob for DOWN state. - -Note that the commit message format follows git norms, so that "git -log --oneline" will have useful output. - -HACKING THE BUILD SYSTEM - -If you change or add to the build system (configure.ac, any Makefile.am, -etc.), try to check that the following things still work: - - - make dist - - resulting dist tarball builds - - out-of-tree builds - -The quagga.net site relies on make dist to work to generate snapshots. It -must work. Common problems are to forget to have some additional file -included in the dist, or to have a make rule refer to a source file without -using the srcdir variable. - - -RELEASE PROCEDURE - -* Tag the apppropriate commit with a release tag (follow existing - conventions). - [This enables recreating the release, and is just good CM practice.] - -* Create a fresh tar archive of the quagga.net repository, and do a test - build: - - git-clone git:///code.quagga.net/quagga.git quagga - git-archive --remote=git://code.quagga.net/quagga.git \ - --prefix=quagga-release/ master | tar -xf - - cd quagga-release - - autoreconf -i - ./configure - make - make dist - -The tarball which 'make dist' creates is the tarball to be released! The -git-archive step ensures you're working with code corresponding to that in -the official repository, and also carries out keyword expansion. If any -errors occur, move tags as needed and start over from the fresh checkouts. -Do not append to tarballs, as this has produced non-standards-conforming -tarballs in the past. - -See also: http://wiki.quagga.net/index.php/Main/Processes - -[TODO: collation of a list of deprecated commands. Possibly can be scripted -to extract from vtysh/vtysh_cmd.c] - - -TOOL VERSIONS - -Require versions of support tools are listed in INSTALL.quagga.txt. -Required versions should only be done with due deliberation, as it can -cause environments to no longer be able to compile quagga. - - -SHARED LIBRARY VERSIONING - -[this section is at the moment just gdt's opinion] - -Quagga builds several shared libaries (lib/libzebra, ospfd/libospf, -ospfclient/libsopfapiclient). These may be used by external programs, -e.g. a new routing protocol that works with the zebra daemon, or -ospfapi clients. The libtool info pages (node Versioning) explain -when major and minor version numbers should be changed. These values -are set in Makefile.am near the definition of the library. If you -make a change that requires changing the shared library version, -please update Makefile.am. - -libospf exports far more than it should, and is needed by ospfapi -clients. Only bump libospf for changes to functions for which it is -reasonable for a user of ospfapi to call, and please err on the side -of not bumping. - -There is no support intended for installing part of zebra. The core -library libzebra and the included daemons should always be built and -installed together. - - -GIT COMMIT SUBSMISSION - -The preferred method for changes is to provide git commits via a -publically-accessible git repository. - -All content guidelines in PATCH SUBMISSION apply. - - -PATCH SUBMISSION - -* Send a clean diff against the 'master' branch of the quagga.git - repository, in unified diff format, preferably with the '-p' argument to - show C function affected by any chunk, and with the -w and -b arguments to - minimise changes. E.g: - - git diff -up mybranch..remotes/quagga.net/master - - It is preferable to use git format-patch, and even more preferred to - publish a git repostory. - - If not using git format-patch, Include the commit message in the email. - -* After a commit, code should have comments explaining to the reviewer - why it is correct, without reference to history. The commit message - should explain why the change is correct. - -* Include NEWS entries as appropriate. - -* Include only one semantic change or group of changes per patch. - -* Do not make gratuitous changes to whitespace. See the w and b arguments - to diff. - -* State on which platforms and with what daemons the patch has been - tested. Understand that if the set of testing locations is small, - and the patch might have unforeseen or hard to fix consequences that - there may be a call for testers on quagga-dev, and that the patch - may be blocked until test results appear. - - If there are no users for a platform on quagga-dev who are able and - willing to verify -current occasionally, that platform may be - dropped from the "should be checked" list. - - -PATCH APPLICATION - -* Only apply patches that meet the submission guidelines. - -* If the patch might break something, issue a call for testing on the - mailinglist. - -* Give an appropriate commit message (see above), and use the --author - argument to git-commit, if required, to ensure proper attribution (you - should still be listed as committer) - -* Immediately after commiting, double-check (with git-log and/or gitk). If - there's a small mistake you can easily fix it with 'git commit --amend ..' - -* By committing a patch, you are responsible for fixing problems - resulting from it (or backing it out). - - -STABLE PLATFORMS AND DAEMONS - -The list of platforms that should be tested follow. This is a list -derived from what quagga is thought to run on and for which -maintainers can test or there are people on quagga-dev who are able -and willing to verify that -current does or does not work correctly. - - BSD (Free, Net or Open, any platform) # without capabilities - GNU/Linux (any distribution, i386) - Solaris (strict alignment, any platform) - [future: NetBSD/sparc64] - -The list of daemons that are thought to be stable and that should be -tested are: - - zebra - bgpd - ripd - ospfd - ripngd - -Daemons which are in a testing phase are - - ospf6d - isisd - watchquagga - - -IMPORT OR UPDATE VENDOR SPECIFIC ROUTING PROTOCOLS - -The source code of Quagga is based on two vendors: - - zebra_org (http://www.zebra.org/) - isisd_sf (http://isisd.sf.net/) - -To import code from further sources, e.g. for archival purposes without -necessarily having to review and/or fix some changeset, create a branch from -'master': - - git checkout -b archive/foo master - <apply changes> - git commit -a "Joe Bar <joe@example.com>" - git push quagga archive/foo - -presuming 'quagga' corresponds to a file in your .git/remotes with -configuration for the appropriate Quagga.net repository. diff --git a/HACKING.pending b/HACKING.pending index 5e0defd8..dfce5cec 100644 --- a/HACKING.pending +++ b/HACKING.pending @@ -14,6 +14,11 @@ collected in his git repository. * public git repositories +** git remote add quagga-re git://github.com/Quagga-RE/quagga-RE.git + +Maintained by Denis Ovsienko, and geared towards producing a +production-ready branch of Quagga, in the Quagga-RE-stable branch. + ** git remote add equinox git://git.spaceboyz.net/equinox/quagga.git/ This repository has topic branches for patches intended for inclusion diff --git a/HACKING.tex b/HACKING.tex new file mode 100644 index 00000000..a49113fb --- /dev/null +++ b/HACKING.tex @@ -0,0 +1,462 @@ +%% -*- mode: text; -*- +%% $QuaggaId: Format:%an, %ai, %h$ $ + +\documentclass[oneside]{article} +\usepackage{parskip} +\usepackage[bookmarks,colorlinks=true]{hyperref} + +\title{Conventions for working on Quagga} + +\begin{document} +\maketitle + +This is a living document. Suggestions for updates, via the +\href{http://lists.quagga.net/mailman/listinfo/quagga-dev}{quagga-dev list}, +are welcome. + +\tableofcontents + +\section{GUIDELINES FOR HACKING ON QUAGGA} +\label{sec:guidelines} + + +GNU coding standards apply. Indentation follows the result of +invoking GNU indent (as of 2.2.8a) with no arguments. Note that this +uses tabs instead of spaces where possible for leading whitespace, and +assumes that tabs are every 8 columns. Do not attempt to redefine the +location of tab stops. Note also that some indentation does not +follow GNU style. This is a historical accident, and we generally +only clean up whitespace when code is unmaintainable due to whitespace +issues, to minimise merging conflicts. + +For GNU emacs, use indentation style ``gnu''. + +For Vim, use the following lines (note that tabs are at 8, and that +softtabstop sets the indentation level): + +set tabstop=8 +set softtabstop=2 +set shiftwidth=2 +set noexpandtab + +Be particularly careful not to break platforms/protocols that you +cannot test. + +New code should have good comments, which explain why the code is correct. +Changes to existing code should in many cases upgrade the comments when +necessary for a reviewer to conclude that the change has no unintended +consequences. + +Each file in the Git repository should have a git format-placeholder (like +an RCS Id keyword), somewhere very near the top, commented out appropriately +for the file type. The placeholder used for Quagga (replacing <dollar> with +\$) is: + + \verb|$QuaggaId: <dollar>Format:%an, %ai, %h<dollar> $| + +See line 2 of HACKING.tex, the source for this document, for an example. + +This placeholder string will be expanded out by the `git archive' commands, +wihch is used to generate the tar archives for snapshots and releases. + +Please document fully the proper use of a new function in the header file +in which it is declared. And please consult existing headers for +documentation on how to use existing functions. In particular, please consult +these header files: + +\begin{description} + \item{lib/log.h} logging levels and usage guidance + \item{[more to be added]} +\end{description} + +If changing an exported interface, please try to deprecate the interface in +an orderly manner. If at all possible, try to retain the old deprecated +interface as is, or functionally equivalent. Make a note of when the +interface was deprecated and guard the deprecated interface definitions in +the header file, ie: + +\begin{verbatim} +/* Deprecated: 20050406 */ +#if !defined(QUAGGA_NO_DEPRECATED_INTERFACES) +#warning "Using deprecated <libname> (interface(s)|function(s))" +... +#endif /* QUAGGA_NO_DEPRECATED_INTERFACES */ +\end{verbatim} + +This is to ensure that the core Quagga sources do not use the deprecated +interfaces (you should update Quagga sources to use new interfaces, if +applicable), while allowing external sources to continue to build. +Deprecated interfaces should be excised in the next unstable cycle. + +Note: If you wish, you can test for GCC and use a function +marked with the 'deprecated' attribute. However, you must provide the +warning for other compilers. + +If changing or removing a command definition, \emph{ensure} that you +properly deprecate it - use the \_DEPRECATED form of the appropriate DEFUN +macro. This is \emph{critical}. Even if the command can no longer +function, you \emph{MUST} still implement it as a do-nothing stub. + +Failure to follow this causes grief for systems administrators, as an +upgrade may cause daemons to fail to start because of unrecognised commands. +Deprecated commands should be excised in the next unstable cycle. A list of +deprecated commands should be collated for each release. + +See also section~\ref{sec:dll-versioning} below regarding SHARED LIBRARY +VERSIONING. + + +\section{COMPILE-TIME CONDITIONAL CODE} + +Please think very carefully before making code conditional at compile time, +as it increases maintenance burdens and user confusion. In particular, +please avoid gratuitious --enable-\ldots switches to the configure script - +typically code should be good enough to be in Quagga, or it shouldn't be +there at all. + +When code must be compile-time conditional, try have the compiler make it +conditional rather than the C pre-processor - so that it will still be +checked by the compiler, even if disabled. I.e. this: + +\begin{verbatim} + if (SOME_SYMBOL) + frobnicate(); +\end{verbatim} + +rather than: + +\begin{verbatim} + #ifdef SOME_SYMBOL + frobnicate (); + #endif /* SOME_SYMBOL */ +\end{verbatim} + +Note that the former approach requires ensuring that SOME\_SYMBOL will be +defined (watch your AC\_DEFINEs). + + +\section{COMMIT MESSAGES} + +The commit message requirements are: + +\begin{itemize} + +\item The message \emph{MUST} provide a suitable one-line summary followed + by a blank line as the very first line of the message, in the form: + + \verb|topic: high-level, one line summary| + + Where topic would tend to be name of a subdirectory, and/or daemon, unless + there's a more suitable topic (e.g. 'build'). This topic is used to + organise change summaries in release announcements. + +\item It should have a suitable "body", which tries to address the + following areas, so as to help reviewers and future browsers of the + code-base understand why the change is correct (note also the code + comment requirements): + + \begin{itemize} + + \item The motivation for the change (does it fix a bug, if so which? + add a feature?) + + \item The general approach taken, and trade-offs versus any other + approaches. + + \item Any testing undertaken or other information affecting the confidence + that can be had in the change. + + \item Information to allow reviewers to be able to tell which specific + changes to the code are intended (and hence be able to spot any accidental + unintended changes). + + \end{itemize} +\end{itemize} + +The one-line summary must be limited to 54 characters, and all other +lines to 72 characters. + +Commit message bodies in the Quagga project have typically taken the +following form: + +\begin{itemize} +\item An optional introduction, describing the change generally. +\item A short description of each specific change made, preferably: + \begin{itemize} \item file by file + \begin{itemize} \item function by function (use of "ditto", or globs is + allowed) + \end{itemize} + \end{itemize} +\end{itemize} + +Contributors are strongly encouraged to follow this form. + +This itemised commit messages allows reviewers to have confidence that the +author has self-reviewed every line of the patch, as well as providing +reviewers a clear index of which changes are intended, and descriptions for +them (C-to-english descriptions are not desireable - some discretion is +useful). For short patches, a per-function/file break-down may be +redundant. For longer patches, such a break-down may be essential. A +contrived example (where the general discussion is obviously somewhat +redundant, given the one-line summary): + +\begin{quote}\begin{verbatim} +zebra: Enhance frob FSM to detect loss of frob + +Add a new DOWN state to the frob state machine to allow the barinator to +detect loss of frob. + +* frob.h: (struct frob) Add DOWN state flag. +* frob.c: (frob\_change) set/clear DOWN appropriately on state change. +* bar.c: (barinate) Check frob for DOWN state. +\end{verbatim}\end{quote} + +Please have a look at the git commit logs to get a feel for what the norms +are. + +Note that the commit message format follows git norms, so that ``git +log --oneline'' will have useful output. + +\section{HACKING THE BUILD SYSTEM} + +If you change or add to the build system (configure.ac, any Makefile.am, +etc.), try to check that the following things still work: + +\begin{itemize} +\item make dist +\item resulting dist tarball builds +\item out-of-tree builds +\end{itemize} + +The quagga.net site relies on make dist to work to generate snapshots. It +must work. Common problems are to forget to have some additional file +included in the dist, or to have a make rule refer to a source file without +using the srcdir variable. + + +\section{RELEASE PROCEDURE} + +\begin{itemize} +\item Tag the apppropriate commit with a release tag (follow existing + conventions). + + [This enables recreating the release, and is just good CM practice.] + +\item Create a fresh tar archive of the quagga.net repository, and do a test + build: + + \begin{verbatim} + git-clone git:///code.quagga.net/quagga.git quagga + git-archive --remote=git://code.quagga.net/quagga.git \ + --prefix=quagga-release/ master | tar -xf - + cd quagga-release + + autoreconf -i + ./configure + make + make dist + \end{verbatim} +\end{itemize} + +The tarball which `make dist' creates is the tarball to be released! The +git-archive step ensures you're working with code corresponding to that in +the official repository, and also carries out keyword expansion. If any +errors occur, move tags as needed and start over from the fresh checkouts. +Do not append to tarballs, as this has produced non-standards-conforming +tarballs in the past. + +See also: \url{http://wiki.quagga.net/index.php/Main/Processes} + +[TODO: collation of a list of deprecated commands. Possibly can be scripted +to extract from vtysh/vtysh\_cmd.c] + + +\section{TOOL VERSIONS} + +Require versions of support tools are listed in INSTALL.quagga.txt. +Required versions should only be done with due deliberation, as it can +cause environments to no longer be able to compile quagga. + + +\section{SHARED LIBRARY VERSIONING} +\label{sec:dll-versioning} + +[this section is at the moment just gdt's opinion] + +Quagga builds several shared libaries (lib/libzebra, ospfd/libospf, +ospfclient/libsopfapiclient). These may be used by external programs, +e.g. a new routing protocol that works with the zebra daemon, or +ospfapi clients. The libtool info pages (node Versioning) explain +when major and minor version numbers should be changed. These values +are set in Makefile.am near the definition of the library. If you +make a change that requires changing the shared library version, +please update Makefile.am. + +libospf exports far more than it should, and is needed by ospfapi +clients. Only bump libospf for changes to functions for which it is +reasonable for a user of ospfapi to call, and please err on the side +of not bumping. + +There is no support intended for installing part of zebra. The core +library libzebra and the included daemons should always be built and +installed together. + + +\section{GIT COMMIT SUBMISSION} +\label{sec:git-submission} + +The preferred method for submitting changes is to provide git commits via a +publically-accessible git repository, which the maintainers can easily pull. + +The commits should be in a branch based off the Quagga.net master - a +"feature branch". Ideally there should be no commits to this branch other +than those in master, and those intended to be submitted. However, merge +commits to this branch from the Quagga master are permitted, though strongly +discouraged - use another (potentially local and throw-away) branch to test +merge with the latest Quagga master. + +Recommended practice is to keep different logical sets of changes on +separate branches - "topic" or "feature" branches. This allows you to still +merge them together to one branch (potentially local and/or "throw-away") +for testing or use, while retaining smaller, independent branches that are +easier to merge. + +All content guidelines in section \ref{sec:patch-submission}, PATCH +SUBMISSION apply. + + +\section{PATCH SUBMISSION} +\label{sec:patch-submission} + +\begin{itemize} + +\item For complex changes, contributors are strongly encouraged to first + start a design discussion on the quagga-dev list \emph{before} + starting any coding. + +\item Send a clean diff against the 'master' branch of the quagga.git + repository, in unified diff format, preferably with the '-p' argument to + show C function affected by any chunk, and with the -w and -b arguments to + minimise changes. E.g: + + git diff -up mybranch..remotes/quagga.net/master + + It is preferable to use git format-patch, and even more preferred to + publish a git repository (see GIT COMMIT SUBMISSION, section + \ref{sec:git-submission}). + + If not using git format-patch, Include the commit message in the email. + +\item After a commit, code should have comments explaining to the reviewer + why it is correct, without reference to history. The commit message + should explain why the change is correct. + +\item Include NEWS entries as appropriate. + +\item Include only one semantic change or group of changes per patch. + +\item Do not make gratuitous changes to whitespace. See the w and b arguments + to diff. + +\item Changes should be arranged so that the least contraversial and most + trivial are first, and the most complex or more contraversial are + last. This will maximise how many the Quagga maintainers can merge, + even if some other commits need further work. + +\item Providing a unit-test is strongly encouraged. Doing so will make it + much easier for maintainers to have confidence that they will be able + to support your change. + +\item New code should be arranged so that it easy to verify and test. E.g. + stateful logic should be separated out from functional logic as much as + possible: wherever possible, move complex logic out to smaller helper + functions which access no state other than their arguments. + +\item State on which platforms and with what daemons the patch has been + tested. Understand that if the set of testing locations is small, + and the patch might have unforeseen or hard to fix consequences that + there may be a call for testers on quagga-dev, and that the patch + may be blocked until test results appear. + + If there are no users for a platform on quagga-dev who are able and + willing to verify -current occasionally, that platform may be + dropped from the "should be checked" list. + +\end{itemize} + +\section{PATCH APPLICATION} + +\begin{itemize} + +\item Only apply patches that meet the submission guidelines. + +\item If the patch might break something, issue a call for testing on the + mailinglist. + +\item Give an appropriate commit message (see above), and use the --author + argument to git-commit, if required, to ensure proper attribution (you + should still be listed as committer) + +\item Immediately after commiting, double-check (with git-log and/or gitk). + If there's a small mistake you can easily fix it with `git commit + --amend ..' + +\item When merging a branch, always use an explicit merge commit. Giving + --no-ff ensures a merge commit is created which documents ``this human + decided to merge this branch at this time''. +\end{itemize} + +\section{STABLE PLATFORMS AND DAEMONS} + +The list of platforms that should be tested follow. This is a list +derived from what quagga is thought to run on and for which +maintainers can test or there are people on quagga-dev who are able +and willing to verify that -current does or does not work correctly. + +\begin{itemize} + \item BSD (Free, Net or Open, any platform) + \item GNU/Linux (any distribution, i386) + \item Solaris (strict alignment, any platform) + \item future: NetBSD/sparc64 +\end{itemize} + +The list of daemons that are thought to be stable and that should be +tested are: + +\begin{itemize} + \item zebra + \item bgpd + \item ripd + \item ospfd + \item ripngd +\end{itemize} +Daemons which are in a testing phase are + +\begin{itemize} + \item ospf6d + \item isisd + \item watchquagga +\end{itemize} + +\section{IMPORT OR UPDATE VENDOR SPECIFIC ROUTING PROTOCOLS} + +The source code of Quagga is based on two vendors: + + \verb|zebra_org| (\url{http://www.zebra.org/}) + \verb|isisd_sf| (\url{http://isisd.sf.net/}) + +To import code from further sources, e.g. for archival purposes without +necessarily having to review and/or fix some changeset, create a branch from +`master': + +\begin{verbatim} + git checkout -b archive/foo master + <apply changes> + git commit -a "Joe Bar <joe@example.com>" + git push quagga archive/foo +\end{verbatim} + +presuming `quagga' corresponds to a file in your .git/remotes with +configuration for the appropriate Quagga.net repository. + +\end{document} diff --git a/Makefile.am b/Makefile.am index 5362623d..4615bafb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -14,4 +14,14 @@ EXTRA_DIST = aclocal.m4 SERVICES TODO REPORTING-BUGS INSTALL.quagga.txt \ tools/mrlg.cgi tools/rrcheck.pl tools/rrlookup.pl tools/zc.pl \ tools/zebra.el tools/multiple-bgpd.sh +if HAVE_LATEX + +HACKING.pdf: HACKING.tex + $(LATEXMK) -pdf $< + +clean-local: + -$(LATEXMK) -C HACKING.tex + +endif + ACLOCAL_AMFLAGS = -I m4 diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index e00be3a0..c6c77f88 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -129,16 +129,6 @@ static attr_flags_check_t attr_flags_check_array[attr_flags_check_limit] = [BGP_ATTR_AS_PATHLIMIT ] = { BGP_ATTR_FLAGS_OPTIONAL_TRANS }, } ; -/* For all known attribute types this should collapse to almost nothing each - * time it is compiled. - */ -inline static attr_flags_check_t* -bgp_attr_flags_check_get(uint8_t attr_type) -{ - return &attr_flags_check_array[(attr_type < attr_flags_check_limit) - ? attr_type : 0] ; -} ; - static struct hash *cluster_hash; static void * @@ -882,16 +872,15 @@ bgp_attr_raw(struct peer *peer, ulen* total) * been rejected well before we get to this point. */ static bgp_attr_parse_ret_t -bgp_attr_malformed (struct peer *peer, u_char attr_type, u_char flag, - u_char subcode) +bgp_attr_malformed (restrict bgp_attr_parser_args args, u_char subcode) { byte* data ; uint data_len ; /* Only relax error handling for eBGP peers */ - if (peer_sort (peer) == BGP_PEER_EBGP) + if (peer_sort (args->peer) == BGP_PEER_EBGP) { - switch (attr_type) + switch (args->type) { /* where an optional attribute is inconsequential, e.g. it does not * affect route selection, and can be safely ignored then any such @@ -924,9 +913,9 @@ bgp_attr_malformed (struct peer *peer, u_char attr_type, u_char flag, * of the routes, if possible. */ default: - if (CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS) && - CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL) && - CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL)) + if (CHECK_FLAG (args->flags, BGP_ATTR_FLAG_TRANS) && + CHECK_FLAG (args->flags, BGP_ATTR_FLAG_OPTIONAL) && + CHECK_FLAG (args->flags, BGP_ATTR_FLAG_PARTIAL)) return BGP_ATTR_PARSE_WITHDRAW; break ; } ; @@ -949,7 +938,7 @@ bgp_attr_malformed (struct peer *peer, u_char attr_type, u_char flag, break ; case BGP_NOTIFY_UPDATE_MISS_ATTR: - data = &attr_type ; + data = &args->type ; data_len = 1 ; /* send just the missing type */ break ; @@ -959,15 +948,15 @@ bgp_attr_malformed (struct peer *peer, u_char attr_type, u_char flag, case BGP_NOTIFY_UPDATE_INVAL_ORIGIN: case BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP: case BGP_NOTIFY_UPDATE_OPT_ATTR_ERR: - data = bgp_attr_raw(peer, &data_len) ; + data = bgp_attr_raw(args->peer, &data_len) ; - qassert(flag == (data[0] & 0xF0)) ; - qassert(attr_type == data[1]) ; + qassert(args->flags == (data[0] & 0xF0)) ; + qassert(args->type == data[1]) ; break ; /* send complete attribute */ } ; - bgp_peer_down_error_with_data(peer, BGP_NOTIFY_UPDATE_ERR, subcode, + bgp_peer_down_error_with_data(args->peer, BGP_NOTIFY_UPDATE_ERR, subcode, data, data_len) ; return BGP_ATTR_PARSE_ERROR; } ; @@ -977,156 +966,101 @@ bgp_attr_malformed (struct peer *peer, u_char attr_type, u_char flag, * Issues a logging message and treats as BGP_NOTIFY_UPDATE_ATTR_LENG_ERR */ static bgp_attr_parse_ret_t -bgp_attr_length_malformed (struct peer *peer, u_char type, u_char flag, - ulen len_has, ulen len_required) +bgp_attr_length_malformed (restrict bgp_attr_parser_args args, + ulen len_required) { - zlog (peer->log, LOG_ERR, "%s attribute length is %u -- should be %u", - map_direct(bgp_attr_name_map, type).str, len_has, len_required) ; + zlog (args->peer->log, LOG_ERR, "%s attribute length is %u -- should be %u", + map_direct(bgp_attr_name_map, args->type).str, + args->length, len_required) ; - return bgp_attr_malformed (peer, type, flag, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); } ; -/* Find out what is wrong with the path attribute flag bits and log the error. - "Flag bits" here stand for Optional, Transitive and Partial, but not for - Extended Length. Checking O/T/P bits at once implies, that the attribute - being diagnosed is defined by RFC as either a "well-known" or an "optional, - non-transitive" attribute. - - NB: by the time we get to here, the LS 4 bits of the flags have been - zeroised. - - Returns: true <=> the flag bits are known to be OK - false => one or more flag bits known to be bad... - ...or unknown attr_code (!) +/* Reject attribute on basis of the flags being invalid + * + * Issues a logging message and treats as BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR */ -static bool -bgp_attr_flags_diagnose -( - struct peer * peer, - const uint8_t attr_code, /* BGP_ATTR_XXXX received */ - uint8_t flag /* flags received */ -) +static bgp_attr_parse_ret_t +bgp_attr_flags_malformed(restrict bgp_attr_parser_args args, + attr_flags_check_t* check) { - attr_flags_check_t* check ; uint8_t diff ; + bool seen ; + uint i ; - qassert((flag & 0x0F) == 0) ; + qassert((args->flags & 0x0F) == 0) ; + qassert(check->mask != 0) ; - check = bgp_attr_flags_check_get(attr_code) ; + diff = (args->flags ^ check->req) & check->mask ; - if (check->mask == 0) + seen = false ; + for (i = 0; i < attr_flag_str_max ; i++) { - zlog(peer->log, LOG_ERR, "***BUG: %s() given unknown attribute type 0x%x", - __func__, attr_code) ; - return false ; - } ; - - diff = (flag ^ check->req) & check->mask ; + uint8_t bit ; - if (diff != 0) - { - bool seen ; - uint i ; + bit = attr_flag_str[i].key ; - seen = false ; - for (i = 0; i < attr_flag_str_max ; i++) + if (diff & bit) { - uint8_t bit ; - - bit = attr_flag_str[i].key ; - - if (diff & bit) - { - zlog (peer->log, LOG_ERR, - "%s attribute must%s be flagged as \"%s\"", - map_direct(bgp_attr_name_map, attr_code).str, - (check->req & bit) ? "" : " not", attr_flag_str[i].str) ; - seen = true ; - } ; + zlog (args->peer->log, LOG_ERR, + "%s attribute must%s be flagged as \"%s\"", + map_direct(bgp_attr_name_map, args->type).str, + (check->req & bit) ? "" : " not", attr_flag_str[i].str) ; + seen = true ; } ; - - qassert (seen); } ; - return diff == 0 ; -} ; - -/* Check that the flags for the given attribute are OK, and if not, issue - * suitable diagnostic. - * - * Should collapse to next to nothing -- certainly for constant attr_code ! - */ -inline static bool -bgp_attr_flags_check(struct peer * peer, const uint8_t attr_code, uint8_t flag) -{ - attr_flags_check_t* check ; - - check = bgp_attr_flags_check_get(attr_code) ; - - if (((flag & check->mask) == check->req) && (check->mask != 0)) - return true ; + qassert (seen) ; - return bgp_attr_flags_diagnose(peer, attr_code, flag) ; + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR) ; } ; - /* This is actually an internal error -- at some point has failed to read * everything that was expected (underrun) or have tried to read more than * is available (overrun) ! */ static bgp_attr_parse_ret_t -bgp_attr_over_or_underrun(struct peer *peer, uint8_t type, bgp_size_t length) +bgp_attr_over_or_underrun(restrict bgp_attr_parser_args args) { - zlog (peer->log, LOG_ERR, + zlog (args->peer->log, LOG_ERR, "%s: BGP attribute %s, parser error: %srun %u bytes (BUG)", - peer->host, map_direct(bgp_attr_name_map, type).str, - stream_has_overrun(peer->ibuf) ? "over" : "under", - length) ; + args->peer->host, map_direct(bgp_attr_name_map, args->type).str, + stream_has_overrun(args->peer->ibuf) ? "over" : "under", + args->length) ; - bgp_peer_down_error (peer, BGP_NOTIFY_CEASE, - BGP_NOTIFY_SUBCODE_UNSPECIFIC); + bgp_peer_down_error (args->peer, BGP_NOTIFY_CEASE, + BGP_NOTIFY_SUBCODE_UNSPECIFIC); return BGP_ATTR_PARSE_ERROR; } ; /* Get origin attribute of the update message. */ static bgp_attr_parse_ret_t -bgp_attr_origin (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag) -{ - /* If any recognized attribute has Attribute Flags that conflict - with the Attribute Type Code, then the Error Subcode is set to - Attribute Flags Error. The Data field contains the erroneous - attribute (type, length and value). */ - if (!bgp_attr_flags_check(peer, BGP_ATTR_ORIGIN, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - - /* If any recognized attribute has Attribute Length that conflicts - with the expected length (based on the attribute type code), then - the Error Subcode is set to Attribute Length Error. The Data - field contains the erroneous attribute (type, length and - value). */ - if (length != 1) - return bgp_attr_length_malformed (peer, BGP_ATTR_ORIGIN, flag, length, 1) ; +bgp_attr_origin (restrict bgp_attr_parser_args args) +{ + uint origin ; + + if (args->length != 1) + return bgp_attr_length_malformed (args, 1) ; /* Fetch origin attribute. */ - attr->origin = stream_getc (BGP_INPUT (peer)); + origin = stream_getc (args->s); /* If the ORIGIN attribute has an undefined value, then the Error Subcode is set to Invalid Origin Attribute. The Data field contains the unrecognized attribute (type, length and value). */ - if ((attr->origin != BGP_ORIGIN_IGP) - && (attr->origin != BGP_ORIGIN_EGP) - && (attr->origin != BGP_ORIGIN_INCOMPLETE)) - { - zlog (peer->log, LOG_ERR, "Origin attribute value %u is invalid", - attr->origin); - return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag, - BGP_NOTIFY_UPDATE_INVAL_ORIGIN); + if ( (origin != BGP_ORIGIN_IGP) + && (origin != BGP_ORIGIN_EGP) + && (origin != BGP_ORIGIN_INCOMPLETE) ) + { + zlog (args->peer->log, LOG_ERR, "Origin attribute value %u is invalid", + origin); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_INVAL_ORIGIN); } - /* Set oring attribute flag. */ - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGIN); + /* Set origin attribute. + */ + args->attr.origin = origin ; + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGIN); return BGP_ATTR_PARSE_PROCEED ; } @@ -1151,54 +1085,47 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length, * any logging/debug stuff. */ static bgp_attr_parse_ret_t -bgp_attr_aspath (struct peer *peer, struct aspath** p_asp, bgp_size_t length, - struct attr *attr, u_char flag, u_char type) +bgp_attr_aspath (restrict bgp_attr_parser_args args, struct aspath** p_asp) { - bool as4_path ; - - qassert((type == BGP_ATTR_AS_PATH) || (type == BGP_ATTR_AS4_PATH)) ; + bool as4_path, as4_peer ; - if (!bgp_attr_flags_check(peer,type, flag)) - { - *p_asp = NULL ; - return bgp_attr_malformed (peer, type, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR) ; - } ; + qassert( (args->type == BGP_ATTR_AS_PATH) + || (args->type == BGP_ATTR_AS4_PATH) ) ; /* Parse the AS_PATH/AS4_PATH body. * * For AS_PATH peer with AS4 => 4Byte ASN otherwise 2Byte ASN * AS4_PATH 4Byte ASN */ - as4_path = (type == BGP_ATTR_AS4_PATH) ; + as4_path = (args->type == BGP_ATTR_AS4_PATH) ; + as4_peer = PEER_CAP_AS4_USE(args->peer) ; - *p_asp = aspath_parse (peer->ibuf, length, - PEER_CAP_AS4_USE(peer) || as4_path, as4_path) ; + *p_asp = aspath_parse (args->s, args->length, as4_peer|| as4_path, as4_path) ; if (*p_asp == NULL) { - zlog (peer->log, LOG_ERR, "Malformed %s from %s, length is %d", - map_direct(bgp_attr_name_map, type).str, peer->host, length); + zlog (args->peer->log, LOG_ERR, "Malformed %s from %s, length is %u", + map_direct(bgp_attr_name_map, args->type).str, + args->peer->host, args->length); - return bgp_attr_malformed (peer, type, flag, - BGP_NOTIFY_UPDATE_MAL_AS_PATH); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_MAL_AS_PATH); } ; /* Success ! */ - attr->flag |= ATTR_FLAG_BIT (type) ; + args->attr.flag |= ATTR_FLAG_BIT (args->type) ; - if (as4_path && PEER_CAP_AS4_USE(peer)) + if (as4_path && as4_peer) { if (BGP_DEBUG(as4, AS4)) zlog_debug ("[AS4] %s sent AS4_PATH despite being an AS4 speaker", - peer->host); + args->peer->host) ; } return BGP_ATTR_PARSE_PROCEED; } ; static bgp_attr_parse_ret_t -bgp_attr_aspath_check (struct peer *peer, struct attr *attr) +bgp_attr_aspath_check (restrict bgp_attr_parser_args args) { /* These checks were part of bgp_attr_aspath, but with * as4 we should to check aspath things when @@ -1217,44 +1144,45 @@ bgp_attr_aspath_check (struct peer *peer, struct attr *attr) * Returns: BGP_ATTR_PARSE_PROCEED -- all well * BGP_ATTR_PARSE_xxx -- as decided by bgp_attr_malformed() */ - struct bgp *bgp = peer->bgp; - struct aspath *aspath; - bgp_peer_sort_t sort = peer_sort(peer) ; + struct bgp* bgp ; + struct aspath* aspath; + bgp_peer_sort_t sort ; - bgp = peer->bgp; + bgp = args->peer->bgp; + sort = peer_sort(args->peer) ; + aspath = args->attr.aspath ; /* Confederation sanity check. */ - if ( ((sort == BGP_PEER_CONFED) && ! aspath_left_confed_check (attr->aspath)) - || ((sort == BGP_PEER_EBGP) && aspath_confed_check (attr->aspath)) ) + if ( ((sort == BGP_PEER_CONFED) && ! aspath_left_confed_check (aspath)) + || ((sort == BGP_PEER_EBGP) && aspath_confed_check (aspath)) ) { - zlog (peer->log, LOG_ERR, "Malformed AS path from %s", peer->host); + zlog (args->peer->log, LOG_ERR, "Malformed AS path from %s", + args->peer->host); - return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, BGP_ATTR_FLAG_TRANS, - BGP_NOTIFY_UPDATE_MAL_AS_PATH); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_MAL_AS_PATH); } /* First AS check for EBGP. */ if (bgp != NULL && bgp_flag_check (bgp, BGP_FLAG_ENFORCE_FIRST_AS)) { if (sort == BGP_PEER_EBGP - && ! aspath_firstas_check (attr->aspath, peer->as)) + && ! aspath_firstas_check (aspath, args->peer->as)) { - zlog (peer->log, LOG_ERR, - "%s incorrect first AS (must be %u)", peer->host, peer->as); + zlog (args->peer->log, LOG_ERR, "%s incorrect first AS (must be %u)", + args->peer->host, args->peer->as); - return bgp_attr_malformed(peer, BGP_ATTR_AS_PATH, BGP_ATTR_FLAG_TRANS, - BGP_NOTIFY_UPDATE_MAL_AS_PATH); + return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_MAL_AS_PATH); } } /* local-as prepend */ - if (peer->change_local_as && - ! CHECK_FLAG (peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND)) + if (args->peer->change_local_as && + ! CHECK_FLAG (args->peer->flags, PEER_FLAG_LOCAL_AS_NO_PREPEND)) { - aspath = aspath_dup (attr->aspath); - aspath = aspath_add_seq (aspath, peer->change_local_as); - aspath_unintern (&attr->aspath); - attr->aspath = aspath_intern (aspath); + aspath = aspath_dup (aspath); + aspath = aspath_add_seq (aspath, args->peer->change_local_as); + aspath_unintern (&args->attr.aspath); + args->attr.aspath = aspath_intern (aspath); } return BGP_ATTR_PARSE_PROCEED; @@ -1262,17 +1190,12 @@ bgp_attr_aspath_check (struct peer *peer, struct attr *attr) /* Nexthop attribute. */ static bgp_attr_parse_ret_t -bgp_attr_nexthop (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag) +bgp_attr_nexthop (restrict bgp_attr_parser_args args) { in_addr_t nexthop_h, nexthop_n; - if (!bgp_attr_flags_check(peer, BGP_ATTR_NEXT_HOP, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - - if (length != 4) - return bgp_attr_length_malformed (peer, BGP_ATTR_NEXT_HOP, flag, length, 4) ; + if (args->length != 4) + return bgp_attr_length_malformed (args, 4) ; /* According to section 6.3 of RFC4271, syntactically incorrect NEXT_HOP attribute must result in a NOTIFICATION message (this is implemented below). @@ -1280,125 +1203,98 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length, logged locally (this is implemented somewhere else). The UPDATE message gets ignored in any of these cases. */ - nexthop_n = stream_get_ipv4 (peer->ibuf); + nexthop_n = stream_get_ipv4 (args->s); + nexthop_h = ntohl (nexthop_n); if (IPV4_NET0 (nexthop_h) || IPV4_NET127 (nexthop_h) || IPV4_CLASS_DE (nexthop_h)) { - char buf[INET_ADDRSTRLEN]; - inet_ntop (AF_INET, &nexthop_h, buf, INET_ADDRSTRLEN); - zlog (peer->log, LOG_ERR, "Martian nexthop %s", buf); - return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag, - BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP); + zlog (args->peer->log, LOG_ERR, "Martian nexthop %s", + siptoa(AF_INET, &nexthop_n).str); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP) ; } - attr->nexthop.s_addr = nexthop_n; - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP); + args->attr.nexthop.s_addr = nexthop_n; + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP); return BGP_ATTR_PARSE_PROCEED; } /* MED atrribute. */ static bgp_attr_parse_ret_t -bgp_attr_med (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag) +bgp_attr_med (restrict bgp_attr_parser_args args) { - if (!bgp_attr_flags_check(peer, BGP_ATTR_MULTI_EXIT_DISC, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_MULTI_EXIT_DISC, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - - if (length != 4) - return bgp_attr_length_malformed (peer, BGP_ATTR_MULTI_EXIT_DISC, flag, - length, 4) ; + if (args->length != 4) + return bgp_attr_length_malformed (args, 4) ; - attr->med = stream_getl (peer->ibuf); - - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC); + args->attr.med = stream_getl (args->s); + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC); return BGP_ATTR_PARSE_PROCEED; } /* Local preference attribute. */ static bgp_attr_parse_ret_t -bgp_attr_local_pref (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag) +bgp_attr_local_pref (restrict bgp_attr_parser_args args) { - if (!bgp_attr_flags_check(peer, BGP_ATTR_LOCAL_PREF, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_LOCAL_PREF, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); + uint32_t local_pref ; + + if (args->length != 4) + return bgp_attr_length_malformed (args, 4) ; - if (length != 4) - return bgp_attr_length_malformed (peer, BGP_ATTR_LOCAL_PREF, flag, - length, 4) ; + local_pref = stream_getl (args->s); /* If it is contained in an UPDATE message that is received from an external peer, then this attribute MUST be ignored by the receiving speaker. */ - if (peer_sort (peer) == BGP_PEER_EBGP) + if (peer_sort (args->peer) != BGP_PEER_EBGP) { - stream_forward_getp (peer->ibuf, length); - return BGP_ATTR_PARSE_PROCEED; - } - - attr->local_pref = stream_getl (peer->ibuf); - - /* Set atomic aggregate flag. */ - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF); + args->attr.local_pref = local_pref ; + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF); + } ; return BGP_ATTR_PARSE_PROCEED; } /* Atomic aggregate. */ -static int -bgp_attr_atomic (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag) +static bgp_attr_parse_ret_t +bgp_attr_atomic (restrict bgp_attr_parser_args args) { - if (!bgp_attr_flags_check(peer, BGP_ATTR_ATOMIC_AGGREGATE, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - - if (length != 0) - return bgp_attr_length_malformed (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag, - length, 0) ; + if (args->length != 0) + return bgp_attr_length_malformed (args, 0) ; /* Set atomic aggregate flag. */ - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ATOMIC_AGGREGATE); + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_ATOMIC_AGGREGATE); return BGP_ATTR_PARSE_PROCEED; } /* Aggregator attribute */ -static int -bgp_attr_aggregator (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag) +static bgp_attr_parse_ret_t +bgp_attr_aggregator (restrict bgp_attr_parser_args args) { bgp_size_t wantedlen ; struct attr_extra *attre ; - if (!bgp_attr_flags_check(peer, BGP_ATTR_AGGREGATOR, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_AGGREGATOR, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - /* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */ - wantedlen = PEER_CAP_AS4_USE(peer) ? 4 + 4 : 2 + 4 ; + wantedlen = PEER_CAP_AS4_USE(args->peer) ? 4 + 4 : 2 + 4 ; - if (length != wantedlen) - return bgp_attr_length_malformed (peer, BGP_ATTR_AGGREGATOR, flag, - length, wantedlen) ; + if (args->length != wantedlen) + return bgp_attr_length_malformed (args, wantedlen) ; - attre = bgp_attr_extra_get (attr); + attre = bgp_attr_extra_get (&args->attr); - if ( PEER_CAP_AS4_USE(peer) ) - attre->aggregator_as = stream_getl (peer->ibuf); + if ( PEER_CAP_AS4_USE(args->peer) ) + attre->aggregator_as = stream_getl (args->s); else - attre->aggregator_as = stream_getw (peer->ibuf); + attre->aggregator_as = stream_getw (args->s); - attre->aggregator_addr.s_addr = stream_get_ipv4 (peer->ibuf); + attre->aggregator_addr.s_addr = stream_get_ipv4 (args->s); /* Set atomic aggregate flag. */ - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR); + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR); return BGP_ATTR_PARSE_PROCEED; } @@ -1410,29 +1306,21 @@ bgp_attr_aggregator (struct peer *peer, bgp_size_t length, * in any logging/debug stuff. */ static bgp_attr_parse_ret_t -bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, - as_t *as4_aggregator_as, - struct in_addr *as4_aggregator_addr) +bgp_attr_as4_aggregator (restrict bgp_attr_parser_args args) { - if (!bgp_attr_flags_check(peer, BGP_ATTR_AS4_AGGREGATOR, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_AS4_AGGREGATOR, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); + if (args->length != 8) + return bgp_attr_length_malformed (args, 8) ; - if (length != 8) - return bgp_attr_length_malformed (peer, BGP_ATTR_AS4_AGGREGATOR, flag, - length, 8) ; + args->as4_aggregator_as = stream_getl (args->s); + args->as4_aggregator_addr.s_addr = stream_get_ipv4 (args->s); - *as4_aggregator_as = stream_getl (peer->ibuf); - as4_aggregator_addr->s_addr = stream_get_ipv4 (peer->ibuf); + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR); - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR); - - if (PEER_CAP_AS4_USE(peer)) + if (PEER_CAP_AS4_USE(args->peer)) { if (BGP_DEBUG(as4, AS4)) zlog_debug ("[AS4] %s sent AS4_AGGREGATOR despite being an" - " AS4 speaker", peer->host); + " AS4 speaker", args->peer->host); } return BGP_ATTR_PARSE_PROCEED; @@ -1450,15 +1338,14 @@ bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length, * NB: we quietly ignore AS4_PATH if no AS_PATH -- same like AS4_AGGREGATOR. */ static bgp_attr_parse_ret_t -bgp_attr_munge_as4_path (struct peer *peer, struct attr *attr, - struct aspath *as4_path) +bgp_attr_munge_as4_path (restrict bgp_attr_parser_args args) { struct aspath *newpath; - if (! (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH))) + if (! (args->attr.flag & ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH))) return BGP_ATTR_PARSE_PROCEED ; /* Do nothing if no AS4_PATH ! */ - if (!(attr->flag & ATTR_FLAG_BIT (BGP_ATTR_AS_PATH))) + if (!(args->attr.flag & ATTR_FLAG_BIT (BGP_ATTR_AS_PATH))) /* The lack of an AS_PATH is likely to become a serious issue in the * near future. * @@ -1472,16 +1359,16 @@ bgp_attr_munge_as4_path (struct peer *peer, struct attr *attr, if ( BGP_DEBUG(as4, AS4)) zlog_debug ("[AS4] %s BGP not AS4 capable peer" " sent AS4_PATH but no AS_PATH," - " so ignore the AS4_PATH", peer->host); + " so ignore the AS4_PATH", args->peer->host); return BGP_ATTR_PARSE_PROCEED ; /* Easy(-ish) if no AS_PATH */ } ; /* need to reconcile NEW_AS_PATH and AS_PATH */ - newpath = aspath_reconcile_as4 (attr->aspath, as4_path); - aspath_unintern (&attr->aspath); - attr->aspath = aspath_intern (newpath); + newpath = aspath_reconcile_as4 (args->attr.aspath, args->as4_path); + aspath_unintern (&args->attr.aspath); + args->attr.aspath = aspath_intern (newpath); return BGP_ATTR_PARSE_PROCEED; } @@ -1537,15 +1424,13 @@ bgp_attr_munge_as4_path (struct peer *peer, struct attr *attr, * NB: this is not, strictly, and error */ static bgp_attr_parse_ret_t -bgp_attr_munge_as4_aggr (struct peer *peer, struct attr *attr, - as_t as4_aggregator, - struct in_addr *as4_aggregator_addr) +bgp_attr_munge_as4_aggr (restrict bgp_attr_parser_args args) { struct attr_extra *attre ; /* We have a AS2 peer. Worry about AGGREGATOR and AS4_AGGREGATOR. */ - if (!(attr->flag & ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR))) + if (!(args->attr.flag & ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR))) return BGP_ATTR_PARSE_PROCEED ; /* Easy if no AS4_AGGREGATOR */ /* AGGREGATOR is Optional Transitive. @@ -1594,20 +1479,20 @@ bgp_attr_munge_as4_aggr (struct peer *peer, struct attr *attr, * an AGGREGATOR from a lone AS4_AGGREGATOR, then some poor AS2 speaker * will find that BGP is no longer working as it did before !!! */ - if (!(attr->flag & ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR))) + if (!(args->attr.flag & ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR))) { if ( BGP_DEBUG(as4, AS4)) zlog_debug ("[AS4] %s BGP not AS4 capable peer" " sent AS4_AGGREGATOR but no AGGREGATOR," - " so ignore the AS4_AGGREGATOR", peer->host); + " so ignore the AS4_AGGREGATOR", args->peer->host); return BGP_ATTR_PARSE_PROCEED ; /* Easy(-ish) if no AGGREGATOR */ } ; /* received both AGGREGATOR and AS4_AGGREGATOR. */ - attre = attr->extra ; - assert (attre); + attre = args->attr.extra ; + assert (attre != NULL); if (attre->aggregator_as != BGP_AS_TRANS) { @@ -1616,7 +1501,7 @@ bgp_attr_munge_as4_aggr (struct peer *peer, struct attr *attr, zlog_debug ("[AS4] %s BGP not AS4 capable peer" " sent AGGREGATOR %u != AS_TRANS and" " AS4_AGGREGATOR, so ignore" - " AS4_AGGREGATOR and AS4_PATH", peer->host, + " AS4_AGGREGATOR and AS4_PATH", args->peer->host, attre->aggregator_as); return BGP_ATTR_PARSE_IGNORE ; @@ -1624,193 +1509,160 @@ bgp_attr_munge_as4_aggr (struct peer *peer, struct attr *attr, /* Finally -- set the aggregator information from the AS4_AGGREGATOR ! */ - attre->aggregator_as = as4_aggregator; - attre->aggregator_addr.s_addr = as4_aggregator_addr->s_addr; + attre->aggregator_as = args->as4_aggregator_as; + attre->aggregator_addr.s_addr = args->as4_aggregator_addr.s_addr; return BGP_ATTR_PARSE_PROCEED ; } /* Community attribute. */ static bgp_attr_parse_ret_t -bgp_attr_community (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag) +bgp_attr_community (restrict bgp_attr_parser_args args) { - if (!bgp_attr_flags_check(peer, BGP_ATTR_COMMUNITIES, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_COMMUNITIES, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - if (length == 0) + if (args->length == 0) { - attr->community = NULL; + args->attr.community = NULL; return BGP_ATTR_PARSE_PROCEED; } - attr->community = - community_parse ((u_int32_t *)stream_get_pnt (peer->ibuf), length); + args->attr.community = + community_parse ((uint32_t *)stream_get_pnt (args->s), + args->length); /* XXX: fix community_parse to use stream API and remove this */ - stream_forward_getp (peer->ibuf, length); + stream_forward_getp (args->s, args->length); - if (!attr->community) + if (!args->attr.community) { - zlog (peer->log, LOG_ERR, "Malformed COMMUNITIES (length is %u)", - length) ; + zlog (args->peer->log, LOG_ERR, "Malformed COMMUNITIES (length is %u)", + args->length) ; - return bgp_attr_malformed (peer, BGP_ATTR_COMMUNITIES, flag, - BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); } ; - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES); + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES); return BGP_ATTR_PARSE_PROCEED; } /* Originator ID attribute. */ static bgp_attr_parse_ret_t -bgp_attr_originator_id (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag) +bgp_attr_originator_id (bgp_attr_parser_args args) { - if (!bgp_attr_flags_check(peer, BGP_ATTR_ORIGINATOR_ID, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_ORIGINATOR_ID, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - - if (length != 4) - return bgp_attr_length_malformed (peer, BGP_ATTR_ORIGINATOR_ID, flag, - length, 4) ; + if (args->length != 4) + return bgp_attr_length_malformed (args, 4) ; - (bgp_attr_extra_get (attr))->originator_id.s_addr - = stream_get_ipv4 (peer->ibuf); + (bgp_attr_extra_get (&args->attr))->originator_id.s_addr + = stream_get_ipv4 (args->s); - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGINATOR_ID); + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGINATOR_ID); return BGP_ATTR_PARSE_PROCEED; } /* Cluster list attribute. */ static bgp_attr_parse_ret_t -bgp_attr_cluster_list (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag) +bgp_attr_cluster_list (restrict bgp_attr_parser_args args) { - if (!bgp_attr_flags_check(peer, BGP_ATTR_CLUSTER_LIST, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_CLUSTER_LIST, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - - if (length % 4) + if (args->length % 4) { - zlog (peer->log, LOG_ERR, "Bad cluster list length %d", length); + zlog (args->peer->log, LOG_ERR, "Bad cluster list length %u", + args->length); - return bgp_attr_malformed (peer, BGP_ATTR_CLUSTER_LIST, flag, - BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); } - (bgp_attr_extra_get (attr))->cluster - = cluster_parse ((struct in_addr *)stream_get_pnt (peer->ibuf), length); + (bgp_attr_extra_get (&args->attr))->cluster + = cluster_parse ((struct in_addr *)stream_get_pnt (args->peer->ibuf), + args->length); /* XXX: Fix cluster_parse to use stream API and then remove this */ - stream_forward_getp (peer->ibuf, length); + stream_forward_getp (args->s, args->length); - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_CLUSTER_LIST); + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_CLUSTER_LIST); return BGP_ATTR_PARSE_PROCEED; } /* Multiprotocol reachability information parse. */ extern bgp_attr_parse_ret_t -bgp_mp_reach_parse (struct peer *peer, const bgp_size_t length, - struct attr *attr, const u_char flag, - struct bgp_nlri *mp_update) +bgp_mp_reach_parse (bgp_attr_parser_args args) { afi_t afi; safi_t safi; size_t nlri_len ; byte* nlri ; ulen nexthop_endp ; + ulen nexthop_len ; u_char reserved ; int ret; - struct stream *s; - struct attr_extra *attre = bgp_attr_extra_get(attr); - - if (!bgp_attr_flags_check(peer, BGP_ATTR_MP_REACH_NLRI, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_MP_REACH_NLRI, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - - /* Require the given length to be: within what is left to read in the stream - * at least the minimum required - * - * If OK, sets s->endp to the end of the attribute. - */ - s = BGP_INPUT(peer); + struct attr_extra *attre = bgp_attr_extra_get(&args->attr); /* Load AFI, SAFI and Next Hop Length */ - afi = stream_getw (s); - safi = stream_getc (s); - attre->mp_nexthop_len = stream_getc (s); + afi = stream_getw (args->s); + safi = stream_getc (args->s); + nexthop_len = stream_getc (args->s); - if (stream_has_overrun(s)) + if (stream_has_overrun(args->s)) { - zlog (peer->log, LOG_ERR, + zlog (args->peer->log, LOG_ERR, "%s attribute length is %u -- should be at least 5", - map_direct(bgp_attr_name_map, BGP_ATTR_MP_REACH_NLRI).str, length) ; + map_direct(bgp_attr_name_map, BGP_ATTR_MP_REACH_NLRI).str, + args->length) ; - return bgp_attr_malformed (peer, BGP_ATTR_MP_REACH_NLRI, flag, - BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); } ; /* Get nexthop length and check */ - nexthop_endp = stream_push_endp(s, attre->mp_nexthop_len + 1) ; + nexthop_endp = stream_push_endp(args->s, nexthop_len + 1) ; - if (stream_has_overrun(s)) + if (stream_has_overrun(args->s)) { - stream_pop_endp(s, nexthop_endp) ; /* as you was */ + stream_pop_endp(args->s, nexthop_endp) ; /* as you was */ zlog_info ("%s: %s, MP nexthop length %u + reserved byte" " overruns end of attribute", - __func__, peer->host, attre->mp_nexthop_len); + __func__, args->peer->host, nexthop_len); - return bgp_attr_malformed (peer, BGP_ATTR_MP_REACH_NLRI, flag, - BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); } ; /* Nexthop length check. */ - switch (attre->mp_nexthop_len) + switch (nexthop_len) { case 4: - stream_get (&attre->mp_nexthop_global_in, s, 4); + stream_get (&attre->mp_nexthop_global_in, args->s, 4); /* Probably needed for RFC 2283 */ - if (attr->nexthop.s_addr == 0) - memcpy(&attr->nexthop.s_addr, &attre->mp_nexthop_global_in, 4); + if (args->attr.nexthop.s_addr == 0) + memcpy(&args->attr.nexthop.s_addr, &attre->mp_nexthop_global_in, 4); break; case 12: - stream_getl (s); /* RD high */ - stream_getl (s); /* RD low */ - stream_get (&attre->mp_nexthop_global_in, s, 4); + stream_getl (args->s); /* RD high */ + stream_getl (args->s); /* RD low */ + stream_get (&attre->mp_nexthop_global_in, args->s, 4); break; #ifdef HAVE_IPV6 case 16: - stream_get (&attre->mp_nexthop_global, s, 16); + stream_get (&attre->mp_nexthop_global, args->s, 16); break; case 32: - stream_get (&attre->mp_nexthop_global, s, 16); - stream_get (&attre->mp_nexthop_local, s, 16); + stream_get (&attre->mp_nexthop_global, args->s, 16); + stream_get (&attre->mp_nexthop_local, args->s, 16); if (! IN6_IS_ADDR_LINKLOCAL (&attre->mp_nexthop_local)) { - char buf1[INET6_ADDRSTRLEN]; - char buf2[INET6_ADDRSTRLEN]; - if (BGP_DEBUG (update, UPDATE_IN)) zlog_debug ("%s got two nexthop %s %s " "but second one is not a link-local nexthop", - peer->host, - inet_ntop (AF_INET6, &attre->mp_nexthop_global, - buf1, INET6_ADDRSTRLEN), - inet_ntop (AF_INET6, &attre->mp_nexthop_local, - buf2, INET6_ADDRSTRLEN)); + args->peer->host, + siptoa(AF_INET6, &attre->mp_nexthop_global).str, + siptoa(AF_INET6, &attre->mp_nexthop_local).str) ; attre->mp_nexthop_len = 16; } @@ -1818,32 +1670,33 @@ bgp_mp_reach_parse (struct peer *peer, const bgp_size_t length, #endif /* HAVE_IPV6 */ default: - stream_pop_endp(s, nexthop_endp) ; /* as you was */ + stream_pop_endp(args->s, nexthop_endp) ; /* as you was */ - zlog_info ("%s: (%s) unknown multiprotocol next hop length: %d", - __func__, peer->host, attre->mp_nexthop_len) ; + zlog_info ("%s: (%s) unknown multiprotocol next hop length: %u", + __func__, args->peer->host, nexthop_len) ; - return bgp_attr_malformed (peer, BGP_ATTR_MP_REACH_NLRI, flag, - BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); } + attre->mp_nexthop_len = nexthop_len ; /* passed muster */ + /* Eat the reserved byte */ - reserved = stream_getc (s) ; + reserved = stream_getc (args->s) ; /* We should now have read up to the recorded endp, and not beyond * * This is an internal error, really. */ - if (!stream_pop_endp(s, nexthop_endp)) - return bgp_attr_over_or_underrun(peer, BGP_ATTR_MP_REACH_NLRI, length) ; + if (!stream_pop_endp(args->s, nexthop_endp)) + return bgp_attr_over_or_underrun(args) ; /* Check the reserved byte value */ if (reserved != 0) zlog_warn("%s sent non-zero value, %u, for defunct SNPA-length field" " in MP_REACH_NLRI for AFI/SAFI %u/%u", - peer->host, reserved, afi, safi) ; + args->peer->host, reserved, afi, safi) ; /* must have nrli_len, what is left of the attribute * @@ -1854,30 +1707,30 @@ bgp_mp_reach_parse (struct peer *peer, const bgp_size_t length, * Accepting zero bytes of NLRI does not appear to do harm. But we whine * about it in the logging. */ - nlri = stream_get_bytes_left(s, &nlri_len) ; /* steps past */ + nlri = stream_get_bytes_left(args->s, &nlri_len) ; /* steps past */ if (nlri_len == 0) zlog_warn("%s sent zero length NLRI in MP_REACH_NLRI for AFI/SAFI %u/%u", - peer->host, afi, safi) ; + args->peer->host, afi, safi) ; else if (safi != SAFI_MPLS_LABELED_VPN) { /* If the nlri are not valid, bgp_nlri_sanity_check() drops the session */ - ret = bgp_nlri_sanity_check (peer, afi, nlri, nlri_len); + ret = bgp_nlri_sanity_check (args->peer, afi, nlri, nlri_len); if (ret < 0) { - zlog_info ("%s %s NLRI in MP_REACH_NLRI doesn't pass sanity check", - peer->host, map_direct(bgp_afi_name_map, afi).str) ; + zlog_info ("%s %s NLRI in MP_REACH_NLRI fails sanity check", + args->peer->host, map_direct(bgp_afi_name_map, afi).str) ; return BGP_ATTR_PARSE_ERROR ; } } /* Record the MP_REACH_NLRI */ - mp_update->afi = afi; - mp_update->safi = safi; - mp_update->nlri = nlri ; - mp_update->length = nlri_len; + args->mp_update.afi = afi; + args->mp_update.safi = safi; + args->mp_update.nlri = nlri ; + args->mp_update.length = nlri_len; return BGP_ATTR_PARSE_PROCEED; } ; @@ -1887,143 +1740,132 @@ bgp_mp_reach_parse (struct peer *peer, const bgp_size_t length, * Sets mp_eor iff all is well, and attribute is empty. */ extern bgp_attr_parse_ret_t -bgp_mp_unreach_parse (struct peer *peer, const bgp_size_t length, - const u_char flag, struct bgp_nlri *mp_withdraw, - bool* mp_eor) +bgp_mp_unreach_parse (restrict bgp_attr_parser_args args) { - struct stream *s; afi_t afi; safi_t safi; size_t nlri_len ; byte* nlri ; - int ret; - - if (!bgp_attr_flags_check(peer, BGP_ATTR_MP_UNREACH_NLRI, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_MP_UNREACH_NLRI, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - - s = BGP_INPUT(peer); /* Load AFI, SAFI. */ - afi = stream_getw (s); - safi = stream_getc (s); + afi = stream_getw (args->s); + safi = stream_getc (args->s); - if (stream_has_overrun(s)) + if (stream_has_overrun(args->s)) { - zlog (peer->log, LOG_ERR, + zlog (args->peer->log, LOG_ERR, "%s attribute length is %u -- should be at least 3", map_direct(bgp_attr_name_map, BGP_ATTR_MP_UNREACH_NLRI).str, - length) ; + args->length) ; - return bgp_attr_malformed (peer, BGP_ATTR_MP_UNREACH_NLRI, flag, - BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); } ; /* must have nrli_len, what is left of the attribute */ - nlri = stream_get_bytes_left(s, &nlri_len) ; /* steps past */ + nlri = stream_get_bytes_left(args->s, &nlri_len) ; /* steps past */ if (nlri_len == 0) - *mp_eor = true ; + args->mp_eor = true ; else if (safi != SAFI_MPLS_LABELED_VPN) { - /* If the nlri are not valid, bgp_nlri_sanity_check() drops the session + /* If the nlri are not valid, bgp_nlri_sanity_check() drops the + * session */ - ret = bgp_nlri_sanity_check (peer, afi, nlri, nlri_len); + int ret ; + + ret = bgp_nlri_sanity_check (args->peer, afi, nlri, nlri_len); if (ret < 0) { - zlog_info ("%s %s NLRI in MP_UNREACH_NLRI doesn't pass sanity check", - peer->host, map_direct(bgp_afi_name_map, afi).str) ; + zlog_info ("%s %s NLRI in MP_UNREACH_NLRI fails sanity check", + args->peer->host, map_direct(bgp_afi_name_map, afi).str) ; return BGP_ATTR_PARSE_ERROR ; - } - } + } + } ; /* Record the MP_UNREACH_NLRI */ - mp_withdraw->afi = afi; - mp_withdraw->safi = safi; - mp_withdraw->nlri = nlri ; - mp_withdraw->length = nlri_len ; + args->mp_withdraw.afi = afi; + args->mp_withdraw.safi = safi; + args->mp_withdraw.nlri = nlri ; + args->mp_withdraw.length = nlri_len ; return BGP_ATTR_PARSE_PROCEED ; } /* Extended Community attribute. */ static bgp_attr_parse_ret_t -bgp_attr_ext_communities (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag) +bgp_attr_ext_communities (restrict bgp_attr_parser_args args) { - if (!bgp_attr_flags_check(peer, BGP_ATTR_EXT_COMMUNITIES, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_EXT_COMMUNITIES, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR); - - /* Basic length check. */ - if (length == 0) + if (args->length == 0) { - if (attr->extra) - attr->extra->ecommunity = NULL; + if (args->attr.extra != NULL) + args->attr.extra->ecommunity = NULL; /* Empty extcomm doesn't seem to be invalid per se */ return BGP_ATTR_PARSE_PROCEED; } - (bgp_attr_extra_get (attr))->ecommunity = - ecommunity_parse ((u_int8_t *)stream_get_pnt (peer->ibuf), length); + (bgp_attr_extra_get (&args->attr))->ecommunity = + ecommunity_parse ((u_int8_t *)stream_get_pnt (args->s), + args->length); /* XXX: fix ecommunity_parse to use stream API */ - stream_forward_getp (peer->ibuf, length); + stream_forward_getp (args->s, args->length); - if (!attr->extra->ecommunity) - return bgp_attr_malformed (peer, BGP_ATTR_EXT_COMMUNITIES, flag, - BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); + if (args->attr.extra->ecommunity == NULL) + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES); + args->attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES); return BGP_ATTR_PARSE_PROCEED; } /* BGP unknown attribute treatment. */ static bgp_attr_parse_ret_t -bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag, - u_char type, bgp_size_t length) +bgp_attr_unknown (restrict bgp_attr_parser_args args) { struct transit *transit; struct attr_extra *attre; + byte* raw_attr ; ulen raw_len ; if (BGP_DEBUG (normal, NORMAL)) - zlog_debug ("%s Unknown attribute is received (type %d, length %d)", - peer->host, type, length); + zlog_debug ("%s Unknown attribute is received " + "(%stransitive type %u, length %u)", + args->peer->host, + CHECK_FLAG (args->flags, BGP_ATTR_FLAG_TRANS) ? "" : "non-", + args->type, args->length); if (BGP_DEBUG (events, EVENTS)) - zlog (peer->log, LOG_DEBUG, - "Unknown attribute type %d length %d is received", type, length); + zlog (args->peer->log, LOG_DEBUG, + "Unknown attribute type %u length %u is received", + args->type, args->length); /* Forward read pointer of input stream. */ - stream_forward_getp (peer->ibuf, length); + stream_forward_getp (args->s, args->length); /* If any of the mandatory well-known attributes are not recognized, then the Error Subcode is set to Unrecognized Well-known Attribute. The Data field contains the unrecognized attribute (type, length and value). */ - if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)) + if (! CHECK_FLAG (args->flags, BGP_ATTR_FLAG_OPTIONAL)) { - zlog (peer->log, LOG_ERR, - "Unknown attribute type %d has flags 0x%x -- ie NOT Optional", - type, flag); - return bgp_attr_malformed (peer, type, flag, - BGP_NOTIFY_UPDATE_UNREC_ATTR); + zlog (args->peer->log, LOG_ERR, + "Unknown attribute type %u has flags 0x%x -- ie NOT Optional", + args->type, args->flags); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_UNREC_ATTR); } /* Unrecognized non-transitive optional attributes must be quietly ignored and not passed along to other BGP peers. */ - if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)) + if (! CHECK_FLAG (args->flags, BGP_ATTR_FLAG_TRANS)) return BGP_ATTR_PARSE_PROCEED; /* Get the address and total length of the attribute */ - raw_attr = bgp_attr_raw(peer, &raw_len) ; + raw_attr = bgp_attr_raw(args->peer, &raw_len) ; /* If a path with recognized transitive optional attribute is accepted and passed along to other BGP peers and the Partial bit @@ -2034,7 +1876,7 @@ bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag, /* Store transitive attribute to the end of attr->transit. */ - if (! ((attre = bgp_attr_extra_get(attr))->transit) ) + if (! ((attre = bgp_attr_extra_get(&args->attr))->transit) ) attre->transit = XCALLOC (MTYPE_TRANSIT, sizeof (struct transit)); transit = attre->transit; @@ -2085,45 +1927,34 @@ bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag, * BGP_ATTR_PARSE_ERROR -- not good at all: session dropped */ bgp_attr_parse_ret_t -bgp_attr_parse (struct peer *peer, struct attr *attr, - struct bgp_nlri *mp_update, struct bgp_nlri *mp_withdraw, - bool* mp_eor) +bgp_attr_parse (restrict bgp_attr_parser_args args) { bgp_attr_parse_ret_t aret; - struct stream *s; - ulen attr_count ; + uint attr_count ; u_char seen[BGP_ATTR_BITMAP_SIZE]; - /* we need the as4_path only until we have synthesized the as_path with it */ - /* same goes for as4_aggregator */ - struct aspath *as4_path = NULL; - as_t as4_aggregator = 0; - struct in_addr as4_aggregator_addr = { 0 }; + attr_flags_check_t* check ; - /* Initialize bitmap. */ + /* Initialize bitmap. + */ memset (seen, 0, BGP_ATTR_BITMAP_SIZE); /* We are going to read size bytes of attributes from the current stream * position. */ - *mp_eor = false ; - attr_count = 0 ; - - aret = BGP_ATTR_PARSE_PROCEED ; /* So far, so good ! */ + attr_count = 0 ; /* nothing yet */ + aret = BGP_ATTR_PARSE_PROCEED ; /* So far, so good ! */ - s = BGP_INPUT(peer) ; - while (stream_get_read_left(s) > 0) + while (stream_get_read_left(args->s) > 0) { bgp_attr_parse_ret_t ret ; - ulen header_length, length, attr_endp ; - u_char flag ; - u_char type ; + ulen header_length, attr_endp ; ++attr_count ; /* Remember the start of the attribute -- used in bgp_attr_malformed() * and other error handling. */ - stream_set_startp(s, stream_get_getp(s)) ; + stream_set_startp(args->s, stream_get_getp(args->s)) ; /* Fetch attribute flag, type and length. * @@ -2133,18 +1964,18 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, * unused. They MUST be zero when sent and MUST be ignored when * received." */ - flag = stream_getc (s) & 0xF0 ; - type = stream_getc (s); + args->flags = stream_getc (args->s) & 0xF0 ; + args->type = stream_getc (args->s); - if (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN)) + if (CHECK_FLAG (args->flags, BGP_ATTR_FLAG_EXTLEN)) { header_length = 4 ; - length = stream_getw (s); + args->length = stream_getw (args->s); } else { header_length = 3 ; - length = stream_getc (s); + args->length = stream_getc (args->s); } ; /* Check whether attribute prefix and the length of the attribute body @@ -2153,24 +1984,25 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, * Sets the s->endp to the end of the current attribute and saves the * end of all attributes in */ - attr_endp = stream_push_endp(s, length) ; - if (stream_has_overrun(s)) + attr_endp = stream_push_endp(args->s, args->length) ; + if (stream_has_overrun(args->s)) { - ulen have = attr_endp - (ulen)stream_get_startp(s) ; + ulen have = attr_endp - (ulen)stream_get_startp(args->s) ; if (have < header_length) - zlog (peer->log, LOG_WARNING, + zlog (args->peer->log, LOG_WARNING, "%s: broken BGP attribute [0x%x %u %u] have just %u octets for" " attribute header", - peer->host, flag, type, length, have) ; + args->peer->host, args->flags, args->type, args->length, have) ; else - zlog (peer->log, LOG_WARNING, + zlog (args->peer->log, LOG_WARNING, "%s: broken BGP attribute [0x%x %u %u] have just %u octets for" " attribute body", - peer->host, flag, type, length, (have - header_length)) ; + args->peer->host, args->flags, args->type, args->length, + (have - header_length)) ; - bgp_peer_down_error (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); + bgp_peer_down_error (args->peer, BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); aret = BGP_ATTR_PARSE_ERROR; goto attr_parse_exit ; @@ -2180,14 +2012,14 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, * message, then the Error Subcode is set to Malformed Attribute * List. */ - if (CHECK_BITMAP (seen, type)) + if (CHECK_BITMAP (seen, args->type)) { - zlog (peer->log, LOG_WARNING, + zlog (args->peer->log, LOG_WARNING, "%s: error BGP attribute type %s appears twice in a message", - peer->host, map_direct(bgp_attr_name_map, type).str); + args->peer->host, map_direct(bgp_attr_name_map, args->type).str); - bgp_peer_down_error (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_MAL_ATTR); + bgp_peer_down_error (args->peer, BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_MAL_ATTR); aret = BGP_ATTR_PARSE_ERROR; goto attr_parse_exit ; @@ -2196,7 +2028,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, /* Set type to bitmap to check duplicate attribute. `type' is * unsigned char so it never overflow bitmap range. */ - SET_BITMAP (seen, type); + SET_BITMAP (seen, args->type); /* OK check attribute and store it's value. * @@ -2208,64 +2040,78 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, * then it is assumed that: (a) it will have read the entire * attribute -- so getp == endp, and (b) no more -- so not overrun ! */ - qassert((flag & 0x0F) == 0) ; - qassert(stream_has_read_left(s, length)) ; + qassert((args->flags & 0x0F) == 0) ; + qassert(stream_has_read_left(args->s, args->length)) ; - switch (type) - { - case BGP_ATTR_ORIGIN: - ret = bgp_attr_origin (peer, length, attr, flag); - break; - case BGP_ATTR_AS_PATH: - ret = bgp_attr_aspath (peer, &attr->aspath, length, attr, flag, - BGP_ATTR_AS_PATH); - break; - case BGP_ATTR_AS4_PATH: - ret = bgp_attr_aspath (peer, &as4_path, length, attr, flag, - BGP_ATTR_AS4_PATH); - break; - case BGP_ATTR_NEXT_HOP: - ret = bgp_attr_nexthop (peer, length, attr, flag); - break; - case BGP_ATTR_MULTI_EXIT_DISC: - ret = bgp_attr_med (peer, length, attr, flag); - break; - case BGP_ATTR_LOCAL_PREF: - ret = bgp_attr_local_pref (peer, length, attr, flag); - break; - case BGP_ATTR_ATOMIC_AGGREGATE: - ret = bgp_attr_atomic (peer, length, attr, flag); - break; - case BGP_ATTR_AGGREGATOR: - ret = bgp_attr_aggregator (peer, length, attr, flag); - break; - case BGP_ATTR_AS4_AGGREGATOR: - ret = bgp_attr_as4_aggregator (peer, length, attr, flag, - &as4_aggregator, - &as4_aggregator_addr); - break; - case BGP_ATTR_COMMUNITIES: - ret = bgp_attr_community (peer, length, attr, flag); - break; - case BGP_ATTR_ORIGINATOR_ID: - ret = bgp_attr_originator_id (peer, length, attr, flag); - break; - case BGP_ATTR_CLUSTER_LIST: - ret = bgp_attr_cluster_list (peer, length, attr, flag); - break; - case BGP_ATTR_MP_REACH_NLRI: - ret = bgp_mp_reach_parse (peer, length, attr, flag, mp_update); - break; - case BGP_ATTR_MP_UNREACH_NLRI: - ret = bgp_mp_unreach_parse (peer, length, flag, mp_withdraw, mp_eor); - break; - case BGP_ATTR_EXT_COMMUNITIES: - ret = bgp_attr_ext_communities (peer, length, attr, flag); - break; - default: - ret = bgp_attr_unknown (peer, attr, flag, type, length); - break; - } + /* Check that the flags for the given attribute are OK, and if not, issue + * suitable diagnostic. + * + * Should collapse to next to nothing -- certainly for constant attr_code ! + */ + check = &attr_flags_check_array[(args->type < attr_flags_check_limit) + ? args->type : 0] ; + + if (((args->flags & check->mask) == check->req) || (check->mask == 0)) + ret = BGP_ATTR_PARSE_PROCEED ; + else + ret = bgp_attr_flags_malformed (args, check) ; + + /* If Flags were OK -- parse the value part + */ + if (ret == BGP_ATTR_PARSE_PROCEED) + { + switch (args->type) + { + case BGP_ATTR_ORIGIN: + ret = bgp_attr_origin (args); + break; + case BGP_ATTR_AS_PATH: + ret = bgp_attr_aspath (args, &args->attr.aspath); + break; + case BGP_ATTR_AS4_PATH: + ret = bgp_attr_aspath (args, &args->as4_path); + break; + case BGP_ATTR_NEXT_HOP: + ret = bgp_attr_nexthop (args); + break; + case BGP_ATTR_MULTI_EXIT_DISC: + ret = bgp_attr_med (args); + break; + case BGP_ATTR_LOCAL_PREF: + ret = bgp_attr_local_pref (args); + break; + case BGP_ATTR_ATOMIC_AGGREGATE: + ret = bgp_attr_atomic (args); + break; + case BGP_ATTR_AGGREGATOR: + ret = bgp_attr_aggregator (args); + break; + case BGP_ATTR_AS4_AGGREGATOR: + ret = bgp_attr_as4_aggregator (args); + break; + case BGP_ATTR_COMMUNITIES: + ret = bgp_attr_community (args); + break; + case BGP_ATTR_ORIGINATOR_ID: + ret = bgp_attr_originator_id (args); + break; + case BGP_ATTR_CLUSTER_LIST: + ret = bgp_attr_cluster_list (args); + break; + case BGP_ATTR_MP_REACH_NLRI: + ret = bgp_mp_reach_parse (args); + break; + case BGP_ATTR_MP_UNREACH_NLRI: + ret = bgp_mp_unreach_parse (args); + break; + case BGP_ATTR_EXT_COMMUNITIES: + ret = bgp_attr_ext_communities (args); + break; + default: + ret = bgp_attr_unknown (args); + break; + } ; + } ; /* Restore the end of all attributes, and check that has read all of * the attribute. @@ -2277,12 +2123,12 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, * NB: in any case, the getp is now set to the end of the attribute we * just dealt with, and the endp is the end of all attributes. */ - if (!stream_pop_endp(s, attr_endp)) + if (!stream_pop_endp(args->s, attr_endp)) { if (ret == BGP_ATTR_PARSE_PROCEED) - ret = bgp_attr_over_or_underrun(peer, type, length) ; + ret = bgp_attr_over_or_underrun(args) ; - stream_clear_overrun(s) ; /* in case continuing */ + stream_clear_overrun(args->s) ; /* in case continuing */ } ; /* Now decide what to do with the return code @@ -2295,10 +2141,10 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, /* If we have chosen to ignore an error in a trivial attribute */ case BGP_ATTR_PARSE_IGNORE: - zlog (peer->log, LOG_WARNING, + zlog (args->peer->log, LOG_WARNING, "%s: Attribute %s invalid, but trivial -- ignoring it", - peer->host, - map_direct(bgp_attr_name_map, type).str); + args->peer->host, + map_direct(bgp_attr_name_map, args->type).str); if (aret == BGP_ATTR_PARSE_PROCEED) aret = BGP_ATTR_PARSE_IGNORE ; /* promote */ @@ -2307,10 +2153,10 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, /* If soft-ish error, turn into withdraw */ case BGP_ATTR_PARSE_WITHDRAW: - zlog (peer->log, LOG_WARNING, + zlog (args->peer->log, LOG_WARNING, "%s: Attribute %s invalid -- treating update as withdrawal", - peer->host, - map_direct(bgp_attr_name_map, type).str); + args->peer->host, + map_direct(bgp_attr_name_map, args->type).str); aret = ret ; /* promote */ break ; @@ -2322,13 +2168,13 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, */ case BGP_ATTR_PARSE_ERROR: default: - zlog (peer->log, LOG_WARNING, + zlog (args->peer->log, LOG_WARNING, "%s: Attribute %s invalid -- session drop", - peer->host, - map_direct(bgp_attr_name_map, type).str); + args->peer->host, + map_direct(bgp_attr_name_map, args->type).str); - bgp_peer_down_error (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_MAL_ATTR); + bgp_peer_down_error (args->peer, BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_MAL_ATTR); aret = BGP_ATTR_PARSE_ERROR ; goto attr_parse_exit ; @@ -2343,7 +2189,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, * or BGP_ATTR_PARSE_IGNORE if last attribute parsed * just happened to be so. */ - stream_set_startp(s, stream_get_endp(s)) ; + stream_set_startp(args->s, stream_get_endp(args->s)) ; /* At this place we can see whether we got AS4_PATH and/or * AS4_AGGREGATOR from a 16Bit peer and act accordingly. @@ -2356,7 +2202,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, * all attributes first, including these 32bit ones, and now, * afterwards, we look what and if something is to be done for as4. */ - if (!PEER_CAP_AS4_USE(peer)) + if (!PEER_CAP_AS4_USE(args->peer)) { bgp_attr_parse_ret_t ret ; @@ -2366,11 +2212,10 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, * In this case BGP_ATTR_PARSE_IGNORE does not signal an *error*, * so we do not set "ignored" on the strength of it. */ - ret = bgp_attr_munge_as4_aggr (peer, attr, as4_aggregator, - &as4_aggregator_addr) ; + ret = bgp_attr_munge_as4_aggr (args) ; if (ret == BGP_ATTR_PARSE_PROCEED) - ret = bgp_attr_munge_as4_path (peer, attr, as4_path) ; + ret = bgp_attr_munge_as4_path (args) ; switch (ret) { @@ -2392,11 +2237,11 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, /* Finally do the checks on the aspath we did not do yet * because we waited for a potentially synthesized aspath. */ - if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS_PATH))) + if (args->attr.flag & (ATTR_FLAG_BIT(BGP_ATTR_AS_PATH))) { bgp_attr_parse_ret_t ret ; - ret = bgp_attr_aspath_check (peer, attr); + ret = bgp_attr_aspath_check (args); switch (ret) { @@ -2420,8 +2265,8 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, } ; /* Finally intern unknown attribute. */ - if (attr->extra && attr->extra->transit) - attr->extra->transit = transit_intern (attr->extra->transit); + if (args->attr.extra && args->attr.extra->transit) + args->attr.extra->transit = transit_intern (args->attr.extra->transit); /* Done everything we are going to do -- return the result. * @@ -2447,11 +2292,11 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, */ attr_parse_exit: - if (as4_path != NULL) - aspath_unintern (&as4_path); + if (args->as4_path != NULL) + aspath_unintern (&args->as4_path); if ((attr_count != 1) || (aret != BGP_ATTR_PARSE_PROCEED)) - *mp_eor = false ; + args->mp_eor = false ; return aret ; } ; diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 6a235d54..553a91b5 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -146,12 +146,36 @@ typedef enum { #define BGP_ATTR_PARSE_OK(ret) ((ret) >= BGP_ATTR_PARSE_PROCEED) +typedef struct bgp_attr_parser_args { + struct peer* peer; + struct stream* s ; + + uint8_t type; + uint8_t flags; + bgp_size_t length; /* attribute data length; */ + + struct attr attr; + + struct aspath *as4_path ; + + as_t as4_aggregator_as ; + struct in_addr as4_aggregator_addr ; + + struct bgp_nlri update ; + struct bgp_nlri withdraw ; + + struct bgp_nlri mp_update ; + struct bgp_nlri mp_withdraw ; + + bool mp_eor ; +} bgp_attr_parser_args_t ; + +typedef bgp_attr_parser_args_t* bgp_attr_parser_args ; + /* Prototypes. */ extern void bgp_attr_init (void); extern void bgp_attr_finish (void); -extern bgp_attr_parse_ret_t bgp_attr_parse (struct peer *, struct attr *, - struct bgp_nlri *, - struct bgp_nlri *, bool*); +extern bgp_attr_parse_ret_t bgp_attr_parse (restrict bgp_attr_parser_args args); extern int bgp_attr_check (struct peer *, struct attr *, bool); extern struct attr_extra *bgp_attr_extra_get (struct attr *); extern void bgp_attr_extra_free (struct attr *); @@ -187,10 +211,9 @@ extern void cluster_unintern (struct cluster_list *); /* Transit attribute prototypes. */ void transit_unintern (struct transit *); -/* Exported for unit-test purposes only */ -extern bgp_attr_parse_ret_t bgp_mp_reach_parse (struct peer *, const bgp_size_t, - struct attr *, const u_char, struct bgp_nlri *); -extern bgp_attr_parse_ret_t bgp_mp_unreach_parse (struct peer *, - const bgp_size_t, const u_char, struct bgp_nlri *, bool*); +/* Below exported for unit-test purposes only + * */ +extern int bgp_mp_reach_parse (bgp_attr_parser_args args); +extern int bgp_mp_unreach_parse (bgp_attr_parser_args args); #endif /* _QUAGGA_BGP_ATTR_H */ diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index d9db1c99..2360b400 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -468,7 +468,6 @@ bgp_capability_as4 (struct peer *peer, struct capability_header *hdr) if (BGP_DEBUG (as4, AS4)) zlog_debug ("%s [AS4] about to set cap PEER_CAP_AS4_RCV, got as4 %u", peer->host, as4); - return as4; } diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index fad72c36..7a709ecd 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -671,16 +671,10 @@ int bgp_update_receive (struct peer *peer, bgp_size_t size) { int ret; - struct stream *s; - struct attr attr; bgp_size_t attribute_len; - struct bgp_nlri update; - struct bgp_nlri withdraw; - struct bgp_nlri mp_update; - struct bgp_nlri mp_withdraw; - bool mp_eor ; char attrstr[BUFSIZ] = ""; bgp_attr_parse_ret_t ap_ret ; + bgp_attr_parser_args_t args[1] ; /* Status must be Established. */ if (peer->state != bgp_peer_pEstablished) @@ -692,18 +686,15 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) } /* Set initial values. */ - memset (&attr, 0, sizeof (struct attr)); - memset (&update, 0, sizeof (struct bgp_nlri)); - memset (&withdraw, 0, sizeof (struct bgp_nlri)); - memset (&mp_update, 0, sizeof (struct bgp_nlri)); - memset (&mp_withdraw, 0, sizeof (struct bgp_nlri)); + memset (args, 0, sizeof (args)); - s = peer->ibuf; + args->peer = peer ; + args->s = peer->ibuf ; /* Unfeasible Route Length. */ - withdraw.length = stream_getw (s); - if (stream_has_overrun(s)) + args->withdraw.length = stream_getw (args->s); + if (stream_has_overrun(args->s)) { zlog_err ("%s [Error] Update packet error" " (packet length is short for unfeasible length)", @@ -715,43 +706,43 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) /* Unfeasible Route packet format check. */ - if (withdraw.length > 0) + if (args->withdraw.length > 0) { ulen endp ; - endp = stream_push_endp(s, withdraw.length) ; - if (stream_has_overrun(s)) + endp = stream_push_endp(args->s, args->withdraw.length) ; + if (stream_has_overrun(args->s)) { zlog_err ("%s [Error] Update packet error" " (packet unfeasible length overflow %d)", - peer->host, withdraw.length); + peer->host, args->withdraw.length); bgp_peer_down_error (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_ATTR); return -1; } - ret = bgp_nlri_sanity_check (peer, AFI_IP, stream_get_pnt (s), - withdraw.length); + ret = bgp_nlri_sanity_check (peer, AFI_IP, stream_get_pnt (args->s), + args->withdraw.length); if (ret < 0) { - zlog_info ("%s withdraw NLRI doesn't pass sanity check", peer->host) ; + zlog_info ("%s withdraw NLRI fails sanity check", peer->host) ; return -1 ; } ; if (BGP_DEBUG (packet, PACKET_RECV)) zlog_debug ("%s [Update:RECV] Unfeasible NLRI received", peer->host); - withdraw.afi = AFI_IP; - withdraw.safi = SAFI_UNICAST; - withdraw.nlri = stream_get_pnt (s); + args->withdraw.afi = AFI_IP; + args->withdraw.safi = SAFI_UNICAST; + args->withdraw.nlri = stream_get_pnt (args->s); - stream_pop_endp(s, endp) ; /* steps getp to given endp */ + stream_pop_endp(args->s, endp) ; /* steps getp to given endp */ } /* Fetch attribute total length. */ - attribute_len = stream_getw (s); - if (stream_has_overrun(s)) + attribute_len = stream_getw (args->s); + if (stream_has_overrun(args->s)) { zlog_warn ("%s [Error] Packet Error" " (update packet is short for attribute length)", @@ -773,7 +764,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) /* This define morphs the update case into a withdraw when lower levels * have signalled an error condition where this is best. */ -#define NLRI_ATTR_ARG (ap_ret != BGP_ATTR_PARSE_WITHDRAW ? &attr : NULL) +#define NLRI_ATTR_ARG (ap_ret != BGP_ATTR_PARSE_WITHDRAW ? &args->attr : NULL) /* Parse attribute when it exists. */ if (attribute_len) @@ -784,11 +775,11 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) * Set the s->endp to the end of the attributes, check that is within * the current endp, and save the current endp. */ - endp = stream_push_endp(s, attribute_len) ; - if (stream_has_overrun(s)) + endp = stream_push_endp(args->s, attribute_len) ; + if (stream_has_overrun(args->s)) { zlog_warn ("%s [Error] Packet Error" - " (update packet attribute length overflow %d)", + " (update packet attribute length overflow %u)", peer->host, attribute_len); bgp_peer_down_error (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_ATTR); @@ -797,13 +788,13 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) /* Do the real work of parsing the attributes getp..endp. */ - ap_ret = bgp_attr_parse (peer, &attr, &mp_update, &mp_withdraw, &mp_eor); + ap_ret = bgp_attr_parse (args); /* Restore the endp, checking that either the getp is at the end of the * attributes or we have a serious error and no longer care about the * stream pointers. */ - if (!stream_pop_endp(s, endp) && (ap_ret != BGP_ATTR_PARSE_ERROR)) + if (!stream_pop_endp(args->s, endp) && (ap_ret != BGP_ATTR_PARSE_ERROR)) { /* This is actually an internal error -- have somehow got out of * step, without the check inside the loop spotting it ! @@ -860,7 +851,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) break ; } ; - if (bgp_dump_attr (peer, &attr, attrstr, BUFSIZ)) + if (bgp_dump_attr (peer, &args->attr, attrstr, BUFSIZ)) zlog (peer->log, lvl, "%s rcvd UPDATE w/ attr: %s", peer->host, attrstr); @@ -874,24 +865,24 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) /* Network Layer Reachability Information. */ - update.length = stream_get_read_left(s); + args->update.length = stream_get_read_left(args->s); - if (update.length) + if (args->update.length != 0) { /* Set NLRI portion to structure. */ - update.afi = AFI_IP; - update.safi = SAFI_UNICAST; - update.nlri = stream_get_pnt (s); + args->update.afi = AFI_IP; + args->update.safi = SAFI_UNICAST; + args->update.nlri = stream_get_pnt (args->s); - ret = bgp_nlri_sanity_check (peer, update.afi, update.nlri, - update.length) ; + ret = bgp_nlri_sanity_check (peer, args->update.afi, args->update.nlri, + args->update.length) ; if (ret < 0) { - zlog_info ("%s update NLRI doesn't pass sanity check", peer->host) ; + zlog_info ("%s update NLRI fails sanity check", peer->host) ; return -1 ; } ; - stream_forward_getp (s, update.length); + stream_forward_getp (args->s, args->update.length); } ; /* Now we check for the "mandatory" attributes -- if we have one, other or @@ -900,9 +891,10 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) * Note that mp_update.length is a bit pointless, but we tolerate and ignore * the attribute state. */ - if ((update.length != 0) || (mp_update.length != 0)) + if ((args->update.length != 0) || (args->mp_update.length != 0)) { - ret = bgp_attr_check (peer, &attr, update.length != 0 /* with NEXT_HOP */); + ret = bgp_attr_check (peer, &args->attr, + args->update.length != 0 /* with NEXT_HOP */); if (ret < 0) { ret = -1 ; @@ -920,34 +912,34 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) * Rule is that the MP_UNREACH_NLRI should be the *only* thing in the UPDATE * message. */ - if (mp_eor) + if (args->mp_eor) { - if ((withdraw.length != 0) || (update.length != 0)) - mp_eor = false ; + if ((args->withdraw.length != 0) || (args->update.length != 0)) + args->mp_eor = false ; } ; /* NLRI is processed only when the peer is configured specific Address Family and Subsequent Address Family. */ if (peer->afc[AFI_IP][SAFI_UNICAST]) { - if (withdraw.length) - bgp_nlri_parse (peer, NULL, &withdraw); + if (args->withdraw.length) + bgp_nlri_parse (peer, NULL, &args->withdraw); - if (update.length) - bgp_nlri_parse (peer, NLRI_ATTR_ARG, &update); + if (args->update.length) + bgp_nlri_parse (peer, NLRI_ATTR_ARG, &args->update); - if (mp_update.length - && mp_update.afi == AFI_IP - && mp_update.safi == SAFI_UNICAST) - bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update); + if (args->mp_update.length + && args->mp_update.afi == AFI_IP + && args->mp_update.safi == SAFI_UNICAST) + bgp_nlri_parse (peer, NLRI_ATTR_ARG, &args->mp_update); - if (mp_withdraw.length - && mp_withdraw.afi == AFI_IP - && mp_withdraw.safi == SAFI_UNICAST) - bgp_nlri_parse (peer, NULL, &mp_withdraw); + if (args->mp_withdraw.length + && args->mp_withdraw.afi == AFI_IP + && args->mp_withdraw.safi == SAFI_UNICAST) + bgp_nlri_parse (peer, NULL, &args->mp_withdraw); - if ((attribute_len == 0) && (withdraw.length == 0) - && (update.length == 0)) + if ((attribute_len == 0) && (args->withdraw.length == 0) + && (args->update.length == 0)) { /* End-of-RIB received */ SET_FLAG (peer->af_sflags[AFI_IP][SAFI_UNICAST], @@ -966,19 +958,19 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) if (peer->afc[AFI_IP][SAFI_MULTICAST]) { - if (mp_update.length - && mp_update.afi == AFI_IP - && mp_update.safi == SAFI_MULTICAST) - bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update); - - if (mp_withdraw.length - && mp_withdraw.afi == AFI_IP - && mp_withdraw.safi == SAFI_MULTICAST) - bgp_nlri_parse (peer, NULL, &mp_withdraw); - - if (mp_eor - && mp_withdraw.afi == AFI_IP - && mp_withdraw.safi == SAFI_MULTICAST) + if (args->mp_update.length + && args->mp_update.afi == AFI_IP + && args->mp_update.safi == SAFI_MULTICAST) + bgp_nlri_parse (peer, NLRI_ATTR_ARG, &args->mp_update); + + if (args->mp_withdraw.length + && args->mp_withdraw.afi == AFI_IP + && args->mp_withdraw.safi == SAFI_MULTICAST) + bgp_nlri_parse (peer, NULL, &args->mp_withdraw); + + if (args->mp_eor + && args->mp_withdraw.afi == AFI_IP + && args->mp_withdraw.safi == SAFI_MULTICAST) { /* End-of-RIB received */ SET_FLAG (peer->af_sflags[AFI_IP][SAFI_MULTICAST], @@ -996,19 +988,19 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) } if (peer->afc[AFI_IP6][SAFI_UNICAST]) { - if (mp_update.length - && mp_update.afi == AFI_IP6 - && mp_update.safi == SAFI_UNICAST) - bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update); - - if (mp_withdraw.length - && mp_withdraw.afi == AFI_IP6 - && mp_withdraw.safi == SAFI_UNICAST) - bgp_nlri_parse (peer, NULL, &mp_withdraw); - - if (mp_eor - && mp_withdraw.afi == AFI_IP6 - && mp_withdraw.safi == SAFI_UNICAST) + if (args->mp_update.length + && args->mp_update.afi == AFI_IP6 + && args->mp_update.safi == SAFI_UNICAST) + bgp_nlri_parse (peer, NLRI_ATTR_ARG, &args->mp_update); + + if (args->mp_withdraw.length + && args->mp_withdraw.afi == AFI_IP6 + && args->mp_withdraw.safi == SAFI_UNICAST) + bgp_nlri_parse (peer, NULL, &args->mp_withdraw); + + if (args->mp_eor + && args->mp_withdraw.afi == AFI_IP6 + && args->mp_withdraw.safi == SAFI_UNICAST) { /* End-of-RIB received */ SET_FLAG (peer->af_sflags[AFI_IP6][SAFI_UNICAST], @@ -1026,19 +1018,19 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) } if (peer->afc[AFI_IP6][SAFI_MULTICAST]) { - if (mp_update.length - && mp_update.afi == AFI_IP6 - && mp_update.safi == SAFI_MULTICAST) - bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update); - - if (mp_withdraw.length - && mp_withdraw.afi == AFI_IP6 - && mp_withdraw.safi == SAFI_MULTICAST) - bgp_nlri_parse (peer, NULL, &mp_withdraw); - - if (mp_eor - && mp_withdraw.afi == AFI_IP6 - && mp_withdraw.safi == SAFI_MULTICAST) + if (args->mp_update.length + && args->mp_update.afi == AFI_IP6 + && args->mp_update.safi == SAFI_MULTICAST) + bgp_nlri_parse (peer, NLRI_ATTR_ARG, &args->mp_update); + + if (args->mp_withdraw.length + && args->mp_withdraw.afi == AFI_IP6 + && args->mp_withdraw.safi == SAFI_MULTICAST) + bgp_nlri_parse (peer, NULL, &args->mp_withdraw); + + if (args->mp_eor + && args->mp_withdraw.afi == AFI_IP6 + && args->mp_withdraw.safi == SAFI_MULTICAST) { /* End-of-RIB received */ @@ -1054,19 +1046,19 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) } if (peer->afc[AFI_IP][SAFI_MPLS_VPN]) { - if (mp_update.length - && mp_update.afi == AFI_IP - && mp_update.safi == SAFI_MPLS_LABELED_VPN) - bgp_nlri_parse_vpnv4 (peer, NLRI_ATTR_ARG, &mp_update); - - if (mp_withdraw.length - && mp_withdraw.afi == AFI_IP - && mp_withdraw.safi == SAFI_MPLS_LABELED_VPN) - bgp_nlri_parse_vpnv4 (peer, NULL, &mp_withdraw); - - if (mp_eor - && mp_withdraw.afi == AFI_IP - && mp_withdraw.safi == SAFI_MPLS_LABELED_VPN) + if (args->mp_update.length + && args->mp_update.afi == AFI_IP + && args->mp_update.safi == SAFI_MPLS_LABELED_VPN) + bgp_nlri_parse_vpnv4 (peer, NLRI_ATTR_ARG, &args->mp_update); + + if (args->mp_withdraw.length + && args->mp_withdraw.afi == AFI_IP + && args->mp_withdraw.safi == SAFI_MPLS_LABELED_VPN) + bgp_nlri_parse_vpnv4 (peer, NULL, &args->mp_withdraw); + + if (args->mp_eor + && args->mp_withdraw.afi == AFI_IP + && args->mp_withdraw.safi == SAFI_MPLS_LABELED_VPN) { /* End-of-RIB received */ @@ -1083,7 +1075,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) ret = 0 ; exit_bgp_update_receive: - bgp_attr_unintern_sub (&attr, true) ; /* true => free extra */ + bgp_attr_unintern_sub (&args->attr, true) ; /* true => free extra */ return ret ; } diff --git a/configure.ac b/configure.ac index d87e682c..be18de54 100755 --- a/configure.ac +++ b/configure.ac @@ -7,7 +7,7 @@ ## AC_PREREQ(2.53) -AC_INIT(Quagga, 0.99.20ex22b, [http://bugzilla.quagga.net]) +AC_INIT(Quagga, 0.99.20ex23b, [http://bugzilla.quagga.net]) AC_CONFIG_SRCDIR(lib/zebra.h) AC_CONFIG_MACRO_DIR([m4]) @@ -83,6 +83,16 @@ dnl autoconf 2.59 appears not to support AC_PROG_SED dnl AC_PROG_SED AC_CHECK_PROG([SED],[sed],[sed],[/bin/false]) +dnl pdflatex and latexmk are needed to build HACKING.pdf +AC_CHECK_PROG([PDFLATEX],[pdflatex],[pdflatex],[/bin/false]) +AC_CHECK_PROG([LATEXMK],[latexmk],[latexmk],[/bin/false]) +if test "x$PDFLATEX" = "x/bin/false" -o "x$LATEXMK" = "x/bin/false"; then + AC_MSG_WARN([Will not be able to make PDF versions of TeX documents]) +else + HAVE_LATEX=true +fi +AM_CONDITIONAL([HAVE_LATEX], [test "x$HAVE_LATEX" = "xtrue"]) + dnl ------------------------------------------------------------------ dnl Intel compiler check. Although Intel tries really hard to make icc dnl look like gcc, there are some differences. It's very verbose with diff --git a/doc/basic.texi b/doc/basic.texi index 77fceee1..b3b23ca9 100644 --- a/doc/basic.texi +++ b/doc/basic.texi @@ -46,14 +46,14 @@ starting. Config files are generally found in: -@itemize @asis +@itemize @w{} @item @file{@value{INSTALL_PREFIX_ETC}/*.conf} @end itemize Each of the daemons has its own config file. For example, zebra's default config file name is: -@itemize @asis +@itemize @w{} @item @file{@value{INSTALL_PREFIX_ETC}/zebra.conf} @end itemize diff --git a/lib/pthread_safe.c b/lib/pthread_safe.c index 92d63074..169e5e8f 100644 --- a/lib/pthread_safe.c +++ b/lib/pthread_safe.c @@ -570,7 +570,7 @@ safe_inet_ntoa (struct in_addr in) /*------------------------------------------------------------------------------ * Construct string for given IP family/address. * - * Requires buffer of at least SU_ADDRSTRLEN characters. + * Returns struct with embedded string. */ extern str_iptoa_t siptoa(sa_family_t family, const void* address) diff --git a/lib/thread.c b/lib/thread.c index be490f9f..e8e345aa 100644 --- a/lib/thread.c +++ b/lib/thread.c @@ -1185,10 +1185,12 @@ static unsigned int thread_timer_process (struct thread_list *list, struct timeval *timenow) { struct thread *thread; + struct thread *next; unsigned int ready = 0; - for (thread = list->head; thread; thread = thread->next) + for (thread = list->head; thread; thread = next) { + next = thread->next; if (timeval_cmp (*timenow, thread->u.sands) < 0) return ready; thread_list_delete (list, thread); @@ -1206,10 +1208,12 @@ static unsigned int thread_process (struct thread_list *list) { struct thread *thread; + struct thread *next; unsigned int ready = 0; - for (thread = list->head; thread; thread = thread->next) + for (thread = list->head; thread; thread = next) { + next = thread->next; thread_list_delete (list, thread); thread->type = THREAD_READY; thread_list_add (&thread->master->ready, thread); diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index 88d516e3..7efddd65 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -109,7 +109,7 @@ ospf_auth_type (struct ospf_interface *oi) if (auth_type == OSPF_AUTH_CRYPTOGRAPHIC && list_isempty (OSPF_IF_PARAM (oi, auth_crypt))) return OSPF_AUTH_NULL; - + return auth_type; } @@ -164,12 +164,12 @@ static void ospf_fifo_push_head (struct ospf_fifo *fifo, struct ospf_packet *op) { op->next = fifo->head; - + if (fifo->tail == NULL) fifo->tail = op; - + fifo->head = op; - + fifo->count++; } @@ -272,7 +272,7 @@ void ospf_packet_delete (struct ospf_interface *oi) { struct ospf_packet *op; - + op = ospf_fifo_pop (oi->obuf); if (op) @@ -323,7 +323,7 @@ ospf_packet_max (struct ospf_interface *oi) return max; } - + static int ospf_check_md5_digest (struct ospf_interface *oi, struct ospf_header *ospfh) { @@ -332,7 +332,7 @@ ospf_check_md5_digest (struct ospf_interface *oi, struct ospf_header *ospfh) struct crypt_key *ck; struct ospf_neighbor *nbr; u_int16_t length = ntohs (ospfh->length); - + /* Get secret key. */ ck = ospf_crypt_key_lookup (OSPF_IF_PARAM (oi, auth_crypt), ospfh->u.crypt.key_id); @@ -354,7 +354,7 @@ ospf_check_md5_digest (struct ospf_interface *oi, struct ospf_header *ospfh) ntohl(nbr->crypt_seqnum)); return 0; } - + /* Generate a digest for the ospf packet - their digest + our digest. */ memset(&ctx, 0, sizeof(ctx)); MD5Init(&ctx); @@ -398,15 +398,15 @@ ospf_make_md5_digest (struct ospf_interface *oi, struct ospf_packet *op) /* We do this here so when we dup a packet, we don't have to waste CPU rewriting other headers. - + Note that quagga_time /deliberately/ is not used here */ t = (time(NULL) & 0xFFFFFFFF); if (t > oi->crypt_seqnum) oi->crypt_seqnum = t; else oi->crypt_seqnum++; - - ospfh->u.crypt.crypt_seqnum = htonl (oi->crypt_seqnum); + + ospfh->u.crypt.crypt_seqnum = htonl (oi->crypt_seqnum); /* Get MD5 Authentication key from auth_key list. */ if (list_isempty (OSPF_IF_PARAM (oi, auth_crypt))) @@ -438,7 +438,7 @@ ospf_make_md5_digest (struct ospf_interface *oi, struct ospf_packet *op) return OSPF_AUTH_MD5_SIZE; } - + static int ospf_ls_req_timer (struct thread *thread) { @@ -495,11 +495,11 @@ ospf_ls_upd_timer (struct thread *thread) { struct route_table *table = lsdb->type[i].db; struct route_node *rn; - + for (rn = route_top (table); rn; rn = route_next (rn)) { struct ospf_lsa *lsa; - + if ((lsa = rn->info) != NULL) /* Don't retransmit an LSA if we received it within the last RxmtInterval seconds - this is to allow the @@ -508,7 +508,7 @@ ospf_ls_upd_timer (struct thread *thread) fired. This is a small tweak to what is in the RFC, but it will cut out out a lot of retransmit traffic - MAG */ - if (tv_cmp (tv_sub (recent_relative_time (), lsa->tv_recv), + if (tv_cmp (tv_sub (recent_relative_time (), lsa->tv_recv), int2tv (retransmit_interval)) >= 0) listnode_add (update, rn->info); } @@ -545,8 +545,8 @@ ospf_ls_ack_timer (struct thread *thread) #ifdef WANT_OSPF_WRITE_FRAGMENT static void -ospf_write_frags (int fd, struct ospf_packet *op, struct ip *iph, - struct msghdr *msg, unsigned int maxdatasize, +ospf_write_frags (int fd, struct ospf_packet *op, struct ip *iph, + struct msghdr *msg, unsigned int maxdatasize, unsigned int mtu, int flags, u_char type) { #define OSPF_WRITE_FRAG_SHIFT 3 @@ -570,16 +570,16 @@ ospf_write_frags (int fd, struct ospf_packet *op, struct ip *iph, * existing fragmentation support to do this for us. Bugs/RFEs need to * be raised against the various kernels. */ - + /* set More Frag */ iph->ip_off |= IP_MF; - + /* ip frag offset is expressed in units of 8byte words */ offset = maxdatasize >> OSPF_WRITE_FRAG_SHIFT; - + iovp = &msg->msg_iov[1]; - - while ( (stream_get_endp(op->s) - stream_get_getp (op->s)) + + while ( (stream_get_endp(op->s) - stream_get_getp (op->s)) > maxdatasize ) { /* data length of this frag is to next offset value */ @@ -590,9 +590,9 @@ ospf_write_frags (int fd, struct ospf_packet *op, struct ip *iph, sockopt_iphdrincl_swab_htosys (iph); ret = sendmsg (fd, msg, flags); - + sockopt_iphdrincl_swab_systoh (iph); - + if (ret < 0) zlog_warn ("*** ospf_write_frags: sendmsg failed to %s," " id %d, off %d, len %d, mtu %u failed with %s", @@ -602,7 +602,7 @@ ospf_write_frags (int fd, struct ospf_packet *op, struct ip *iph, iph->ip_len, mtu, safe_strerror (errno)); - + if (IS_DEBUG_OSPF_PACKET (type - 1, SEND)) { zlog_debug ("ospf_write_frags: sent id %d, off %d, len %d to %s\n", @@ -615,12 +615,12 @@ ospf_write_frags (int fd, struct ospf_packet *op, struct ip *iph, zlog_debug ("-----------------------------------------------------"); } } - + iph->ip_off += offset; stream_forward_getp (op->s, iovp->iov_len); - iovp->iov_base = STREAM_PNT (op->s); + iovp->iov_base = STREAM_PNT (op->s); } - + /* setup for final fragment */ iovp->iov_len = stream_get_endp(op->s) - stream_get_getp (op->s); iph->ip_len = iovp->iov_len + sizeof (struct ip); @@ -647,7 +647,7 @@ ospf_write (struct thread *thread) #endif /* WANT_OSPF_WRITE_FRAGMENT */ u_int16_t maxdatasize; #define OSPF_WRITE_IPHL_SHIFT 2 - + ospf->t_write = NULL; node = listhead (ospf->oi_write_q); @@ -666,7 +666,7 @@ ospf_write (struct thread *thread) * socket can accept */ maxdatasize = MIN((int)oi->ifp->mtu, ospf->maxsndbuflen) - sizeof (struct ip); - + /* Get one packet from queue. */ op = ospf_fifo_head (oi->obuf); assert (op); @@ -675,20 +675,20 @@ ospf_write (struct thread *thread) if (op->dst.s_addr == htonl (OSPF_ALLSPFROUTERS) || op->dst.s_addr == htonl (OSPF_ALLDROUTERS)) ospf_if_ipmulticast (ospf, oi->address, oi->ifp->ifindex); - + /* Rewrite the md5 signature & update the seq */ ospf_make_md5_digest (oi, op); /* Retrieve OSPF packet type. */ stream_set_getp (op->s, 1); type = stream_getc (op->s); - + /* reset get pointer */ stream_set_getp (op->s, 0); memset (&iph, 0, sizeof (struct ip)); memset (&sa_dst, 0, sizeof (sa_dst)); - + sa_dst.sin_family = AF_INET; #ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN sa_dst.sin_len = sizeof(sa_dst); @@ -703,10 +703,10 @@ ospf_write (struct thread *thread) iph.ip_hl = sizeof (struct ip) >> OSPF_WRITE_IPHL_SHIFT; /* it'd be very strange for header to not be 4byte-word aligned but.. */ - if ( sizeof (struct ip) + if ( sizeof (struct ip) > (unsigned int)(iph.ip_hl << OSPF_WRITE_IPHL_SHIFT) ) iph.ip_hl++; /* we presume sizeof struct ip cant overflow ip_hl.. */ - + iph.ip_v = IPVERSION; iph.ip_tos = IPTOS_PREC_INTERNETCONTROL; iph.ip_len = (iph.ip_hl << OSPF_WRITE_IPHL_SHIFT) + op->length; @@ -720,7 +720,7 @@ ospf_write (struct thread *thread) #ifdef WANT_OSPF_WRITE_FRAGMENT /* XXX-MT: not thread-safe at all.. - * XXX: this presumes this is only programme sending OSPF packets + * XXX: this presumes this is only programme sending OSPF packets * otherwise, no guarantee ipid will be unique */ iph.ip_id = ++ipid; @@ -738,20 +738,20 @@ ospf_write (struct thread *thread) memset (&msg, 0, sizeof (msg)); msg.msg_name = (caddr_t) &sa_dst; - msg.msg_namelen = sizeof (sa_dst); + msg.msg_namelen = sizeof (sa_dst); msg.msg_iov = iov; msg.msg_iovlen = 2; iov[0].iov_base = (char*)&iph; iov[0].iov_len = iph.ip_hl << OSPF_WRITE_IPHL_SHIFT; iov[1].iov_base = STREAM_PNT (op->s); iov[1].iov_len = op->length; - + /* Sadly we can not rely on kernels to fragment packets because of either * IP_HDRINCL and/or multicast destination being set. */ #ifdef WANT_OSPF_WRITE_FRAGMENT if ( op->length > maxdatasize ) - ospf_write_frags (ospf->fd, op, &iph, &msg, maxdatasize, + ospf_write_frags (ospf->fd, op, &iph, &msg, maxdatasize, oi->ifp->mtu, flags, type); #endif /* WANT_OSPF_WRITE_FRAGMENT */ @@ -759,7 +759,7 @@ ospf_write (struct thread *thread) sockopt_iphdrincl_swab_htosys (&iph); ret = sendmsg (ospf->fd, &msg, flags); sockopt_iphdrincl_swab_systoh (&iph); - + if (ret < 0) zlog_warn ("*** sendmsg in ospf_write failed to %s, " "id %d, off %d, len %d, interface %s, mtu %u: %s", @@ -793,10 +793,10 @@ ospf_write (struct thread *thread) oi->on_write_q = 0; list_delete_node (ospf->oi_write_q, node); } - + /* If packets still remain in queue, call write thread. */ if (!list_isempty (ospf->oi_write_q)) - ospf->t_write = + ospf->t_write = thread_add_write (master, ospf_write, ospf, ospf->fd); return 0; @@ -837,7 +837,7 @@ ospf_hello (struct ip *iph, struct ospf_header *ospfh, /* Compare network mask. */ /* Checking is ignored for Point-to-Point and Virtual link. */ - if (oi->type != OSPF_IFTYPE_POINTOPOINT + if (oi->type != OSPF_IFTYPE_POINTOPOINT && oi->type != OSPF_IFTYPE_VIRTUALLINK) if (oi->address->prefixlen != p.prefixlen) { @@ -869,7 +869,7 @@ ospf_hello (struct ip *iph, struct ospf_header *ospfh, return; } } - + if (IS_DEBUG_OSPF_EVENT) zlog_debug ("Packet %s [Hello:RECV]: Options %s", inet_ntoa (ospfh->router_id), @@ -910,7 +910,7 @@ ospf_hello (struct ip *iph, struct ospf_header *ospfh, /* new for NSSA is to ensure that NP is on and E is off */ - if (oi->area->external_routing == OSPF_AREA_NSSA) + if (oi->area->external_routing == OSPF_AREA_NSSA) { if (! (CHECK_FLAG (OPTIONS (oi), OSPF_OPTION_NP) && CHECK_FLAG (hello->options, OSPF_OPTION_NP) @@ -923,7 +923,7 @@ ospf_hello (struct ip *iph, struct ospf_header *ospfh, if (IS_DEBUG_OSPF_NSSA) zlog_debug ("NSSA-Hello:RECV:Packet from %s:", inet_ntoa(ospfh->router_id)); } - else + else /* The setting of the E-bit found in the Hello Packet's Options field must match this area's ExternalRoutingCapability A mismatch causes processing to stop and the packet to be @@ -936,7 +936,7 @@ ospf_hello (struct ip *iph, struct ospf_header *ospfh, inet_ntoa(ospfh->router_id), OPTIONS (oi), hello->options); return; } - + /* get neighbour struct */ nbr = ospf_nbr_get (oi, ospfh, iph, &p); @@ -1045,7 +1045,7 @@ ospf_db_desc_proc (struct stream *s, struct ospf_interface *oi, stream_forward_getp (s, OSPF_DB_DESC_MIN_SIZE); for (size -= OSPF_DB_DESC_MIN_SIZE; - size >= OSPF_LSA_HEADER_SIZE; size -= OSPF_LSA_HEADER_SIZE) + size >= OSPF_LSA_HEADER_SIZE; size -= OSPF_LSA_HEADER_SIZE) { lsah = (struct lsa_header *) STREAM_PNT (s); stream_forward_getp (s, OSPF_LSA_HEADER_SIZE); @@ -1095,7 +1095,7 @@ ospf_db_desc_proc (struct stream *s, struct ospf_interface *oi, /* Lookup received LSA, then add LS request list. */ find = ospf_lsa_lookup_by_header (oi->area, lsah); - + /* ospf_lsa_more_recent is fine with NULL pointers */ switch (ospf_lsa_more_recent (find, new)) { @@ -1147,8 +1147,8 @@ ospf_db_desc_proc (struct stream *s, struct ospf_interface *oi, { nbr->dd_seqnum = ntohl (dd->dd_seqnum); - /* Send DD packet in reply. - * + /* Send DD packet in reply. + * * Must be done to acknowledge the Master's DD, regardless of * whether we have more LSAs ourselves to describe. * @@ -1156,12 +1156,12 @@ ospf_db_desc_proc (struct stream *s, struct ospf_interface *oi, * we have no more LSAs to describe to the master.. */ ospf_db_desc_send (nbr); - + /* Slave can raise ExchangeDone now, if master is also done */ if (!IS_SET_DD_M (dd->flags) && !IS_SET_DD_M (nbr->dd_flags)) OSPF_NSM_EVENT_SCHEDULE (nbr, NSM_ExchangeDone); } - + /* Save received neighbor values from DD. */ ospf_db_desc_save_current (nbr, dd); } @@ -1200,7 +1200,7 @@ ospf_db_desc (struct ip *iph, struct ospf_header *ospfh, } /* Check MTU. */ - if ((OSPF_IF_PARAM (oi, mtu_ignore) == 0) && + if ((OSPF_IF_PARAM (oi, mtu_ignore) == 0) && (ntohs (dd->mtu) > oi->ifp->mtu)) { zlog_warn ("Packet[DD]: Neighbor %s MTU %u is larger than [%s]'s MTU %u", @@ -1209,15 +1209,15 @@ ospf_db_desc (struct ip *iph, struct ospf_header *ospfh, return; } - /* + /* * XXX HACK by Hasso Tepper. Setting N/P bit in NSSA area DD packets is not - * required. In fact at least JunOS sends DD packets with P bit clear. + * required. In fact at least JunOS sends DD packets with P bit clear. * Until proper solution is developped, this hack should help. * * Update: According to the RFCs, N bit is specified /only/ for Hello * options, unfortunately its use in DD options is not specified. Hence some * implementations follow E-bit semantics and set it in DD options, and some - * treat it as unspecified and hence follow the directive "default for + * treat it as unspecified and hence follow the directive "default for * options is clear", ie unset. * * Reset the flag, as ospfd follows E-bit semantics. @@ -1226,7 +1226,7 @@ ospf_db_desc (struct ip *iph, struct ospf_header *ospfh, && (CHECK_FLAG (nbr->options, OSPF_OPTION_NP)) && (!CHECK_FLAG (dd->options, OSPF_OPTION_NP)) ) { - if (IS_DEBUG_OSPF_EVENT) + if (IS_DEBUG_OSPF_EVENT) zlog_debug ("Packet[DD]: Neighbour %s: Has NSSA capability, sends with N bit clear in DD options", inet_ntoa (nbr->router_id) ); SET_FLAG (dd->options, OSPF_OPTION_NP); @@ -1287,7 +1287,7 @@ ospf_db_desc (struct ip *iph, struct ospf_header *ospfh, zlog_info ("Packet[DD]: Neighbor %s Negotiation done (Slave).", inet_ntoa(nbr->router_id)); nbr->dd_seqnum = ntohl (dd->dd_seqnum); - + /* Reset I/MS */ UNSET_FLAG (nbr->dd_flags, (OSPF_DD_FLAG_MS|OSPF_DD_FLAG_I)); } @@ -1315,7 +1315,7 @@ ospf_db_desc (struct ip *iph, struct ospf_header *ospfh, inet_ntoa(nbr->router_id)); break; } - + /* This is where the real Options are saved */ nbr->options = dd->options; @@ -1635,7 +1635,7 @@ ospf_ls_upd_list_lsa (struct ospf_neighbor *nbr, struct stream *s, /* Do not take in AS External Opaque-LSAs if we are a stub. */ if (lsah->type == OSPF_OPAQUE_AS_LSA - && nbr->oi->area->external_routing != OSPF_AREA_DEFAULT) + && nbr->oi->area->external_routing != OSPF_AREA_DEFAULT) { if (IS_DEBUG_OSPF_EVENT) zlog_debug ("LSA[Type%d:%s]: We are a stub, don't take this LSA.", lsah->type, inet_ntoa (lsah->id)); @@ -1733,9 +1733,9 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh, return; } - /* Get list of LSAs from Link State Update packet. - Also perorms Stages - * 1 (validate LSA checksum) and 2 (check for LSA consistent type) - * of section 13. + /* Get list of LSAs from Link State Update packet. - Also perorms Stages + * 1 (validate LSA checksum) and 2 (check for LSA consistent type) + * of section 13. */ lsas = ospf_ls_upd_list_lsa (nbr, s, oi, size); @@ -1787,21 +1787,21 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh, /* Validate Checksum - Done above by ospf_ls_upd_list_lsa() */ /* LSA Type - Done above by ospf_ls_upd_list_lsa() */ - + /* Do not take in AS External LSAs if we are a stub or NSSA. */ /* Do not take in AS NSSA if this neighbor and we are not NSSA */ - /* Do take in Type-7's if we are an NSSA */ - + /* Do take in Type-7's if we are an NSSA */ + /* If we are also an ABR, later translate them to a Type-5 packet */ - + /* Later, an NSSA Re-fresh can Re-fresh Type-7's and an ABR will translate them to a separate Type-5 packet. */ if (lsa->data->type == OSPF_AS_EXTERNAL_LSA) /* Reject from STUB or NSSA */ - if (nbr->oi->area->external_routing != OSPF_AREA_DEFAULT) + if (nbr->oi->area->external_routing != OSPF_AREA_DEFAULT) { if (IS_DEBUG_OSPF_NSSA) zlog_debug("Incoming External LSA Discarded: We are NSSA/STUB Area"); @@ -1832,7 +1832,7 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh, /* Response Link State Acknowledgment. */ ospf_ls_ack_send (nbr, lsa); - /* Discard LSA. */ + /* Discard LSA. */ zlog_info ("Link State Update[%s]: LS age is equal to MaxAge.", dump_lsa_key(lsa)); DISCARD_LSA (lsa, 3); @@ -1879,10 +1879,10 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh, "not found in the LSDB.", dump_lsa_key (lsa)); SET_FLAG (lsa->flags, OSPF_LSA_SELF); - + ospf_opaque_self_originated_lsa_received (nbr, lsa); ospf_ls_ack_send (nbr, lsa); - + continue; } } @@ -2004,7 +2004,7 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh, flushed before any new LSA instance can be introduced). */ else if (ret > 0) /* Database copy is more recent */ - { + { if (IS_LSA_MAXAGE (current) && current->data->ls_seqnum == htonl (OSPF_MAX_SEQUENCE_NUMBER)) { @@ -2021,10 +2021,10 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh, else { struct timeval now; - + quagga_gettime (QUAGGA_CLK_MONOTONIC, &now); - - if (tv_cmp (tv_sub (now, current->tv_orig), + + if (tv_cmp (tv_sub (now, current->tv_orig), int2tv (OSPF_MIN_LS_ARRIVAL)) >= 0) /* Trap NSSA type later.*/ ospf_ls_upd_send_lsa (nbr, current, OSPF_SEND_PACKET_DIRECT); @@ -2033,7 +2033,7 @@ ospf_ls_upd (struct ip *iph, struct ospf_header *ospfh, } } #undef DISCARD_LSA - + assert (listcount (lsas) == 0); list_delete (lsas); } @@ -2044,7 +2044,7 @@ ospf_ls_ack (struct ip *iph, struct ospf_header *ospfh, struct stream *s, struct ospf_interface *oi, u_int16_t size) { struct ospf_neighbor *nbr; - + /* increment statistics. */ oi->ls_ack_in++; @@ -2067,7 +2067,7 @@ ospf_ls_ack (struct ip *iph, struct ospf_header *ospfh, LOOKUP(ospf_nsm_state_msg, nbr->state)); return; } - + while (size >= OSPF_LSA_HEADER_SIZE) { struct ospf_lsa *lsa, *lsr; @@ -2104,7 +2104,7 @@ ospf_ls_ack (struct ip *iph, struct ospf_header *ospfh, return; } - + static struct stream * ospf_recv_packet (int fd, struct interface **ifp, struct stream *ibuf) { @@ -2122,7 +2122,7 @@ ospf_recv_packet (int fd, struct interface **ifp, struct stream *ibuf) msgh.msg_iovlen = 1; msgh.msg_control = (caddr_t) buff; msgh.msg_controllen = sizeof (buff); - + ret = stream_recvmsg (ibuf, fd, &msgh, 0, OSPF_MAX_PACKET_SIZE+1); if (ret < 0) { @@ -2136,14 +2136,14 @@ ospf_recv_packet (int fd, struct interface **ifp, struct stream *ibuf) ret, (u_int)sizeof(iph)); return NULL; } - + /* Note that there should not be alignment problems with this assignment because this is at the beginning of the stream data buffer. */ iph = (struct ip *) STREAM_DATA(ibuf); sockopt_iphdrincl_swab_systoh (iph); - + ip_len = iph->ip_len; - + #if !defined(GNU_LINUX) && (OpenBSD < 200311) && (__FreeBSD_version < 1000000) /* * Kernel network code touches incoming IP header parameters, @@ -2161,10 +2161,10 @@ ospf_recv_packet (int fd, struct interface **ifp, struct stream *ibuf) */ ip_len = ip_len + (iph->ip_hl << 2); #endif - + #if defined(__DragonFly__) /* - * in DragonFly's raw socket, ip_len/ip_off are read + * in DragonFly's raw socket, ip_len/ip_off are read * in network byte order. * As OpenBSD < 200311 adjust ip_len to strip IP header size! */ @@ -2172,7 +2172,7 @@ ospf_recv_packet (int fd, struct interface **ifp, struct stream *ibuf) #endif ifindex = getsockopt_ifindex (AF_INET, &msgh); - + *ifp = if_lookup_by_index (ifindex); if (ret != ip_len) @@ -2181,12 +2181,12 @@ ospf_recv_packet (int fd, struct interface **ifp, struct stream *ibuf) "but recvmsg returned %d", ip_len, ret); return NULL; } - + return ibuf; } static struct ospf_interface * -ospf_associate_packet_vl (struct ospf *ospf, struct interface *ifp, +ospf_associate_packet_vl (struct ospf *ospf, struct interface *ifp, struct ip *iph, struct ospf_header *ospfh) { struct ospf_interface *rcv_oi; @@ -2200,10 +2200,10 @@ ospf_associate_packet_vl (struct ospf *ospf, struct interface *ifp, /* look for local OSPF interface matching the destination * to determine Area ID. We presume therefore the destination address - * is unique, or at least (for "unnumbered" links), not used in other + * is unique, or at least (for "unnumbered" links), not used in other * areas */ - if ((rcv_oi = ospf_if_lookup_by_local_addr (ospf, NULL, + if ((rcv_oi = ospf_if_lookup_by_local_addr (ospf, NULL, iph->ip_dst)) == NULL) return NULL; @@ -2212,7 +2212,7 @@ ospf_associate_packet_vl (struct ospf *ospf, struct interface *ifp, vl_area = ospf_area_lookup_by_area_id (ospf, vl_data->vl_area_id); if (!vl_area) continue; - + if (OSPF_AREA_SAME (&vl_area, &rcv_oi->area) && IPV4_ADDR_SAME (&vl_data->vl_peer, &ospfh->router_id)) { @@ -2225,14 +2225,14 @@ ospf_associate_packet_vl (struct ospf *ospf, struct interface *ifp, zlog_debug ("This VL is not up yet, sorry"); return NULL; } - + return vl_data->vl_oi; } } if (IS_DEBUG_OSPF_EVENT) zlog_debug ("couldn't find any VL to associate the packet with"); - + return NULL; } @@ -2733,7 +2733,7 @@ ospf_read (struct thread *thread) if (!(ibuf = ospf_recv_packet (ospf->fd, &ifp, ospf->ibuf))) return -1; /* This raw packet is known to be at least as big as its IP header. */ - + /* Note that there should not be alignment problems with this assignment because this is at the beginning of the stream data buffer. */ iph = (struct ip *) STREAM_DATA (ibuf); @@ -2744,7 +2744,7 @@ ospf_read (struct thread *thread) and also platforms (such as Solaris 8) that claim to support ifindex retrieval but do not. */ ifp = if_lookup_address (iph->ip_src); - + if (ifp == NULL) return 0; @@ -2806,7 +2806,7 @@ ospf_read (struct thread *thread) } - /* if no local ospf_interface, + /* if no local ospf_interface, * or header area is backbone but ospf_interface is not * check for VLINK interface */ @@ -2825,14 +2825,14 @@ ospf_read (struct thread *thread) } } - /* else it must be a local ospf interface, check it was received on - * correct link + /* else it must be a local ospf interface, check it was received on + * correct link */ else if (oi->ifp != ifp) { if (IS_DEBUG_OSPF_EVENT) zlog_warn ("Packet from [%s] received on wrong link %s", - inet_ntoa (iph->ip_src), ifp->name); + inet_ntoa (iph->ip_src), ifp->name); return 0; } else if (oi->state == ISM_Down) @@ -3097,7 +3097,7 @@ ospf_make_db_desc (struct ospf_interface *oi, struct ospf_neighbor *nbr, unsigned long pp; int i; struct ospf_lsdb *lsdb; - + /* Set Interface MTU. */ if (oi->type == OSPF_IFTYPE_VIRTUALLINK) stream_putw (s, 0); @@ -3113,7 +3113,7 @@ ospf_make_db_desc (struct ospf_interface *oi, struct ospf_neighbor *nbr, || CHECK_FLAG (nbr->options, OSPF_OPTION_O)) /* * Set O-bit in the outgoing DD packet for capablity negotiation, - * if one of following case is applicable. + * if one of following case is applicable. * * 1) WaitTimer expiration event triggered the neighbor state to * change to Exstart, but no (valid) DD packet has received @@ -3164,25 +3164,25 @@ ospf_make_db_desc (struct ospf_interface *oi, struct ospf_neighbor *nbr, { struct lsa_header *lsah; u_int16_t ls_age; - + /* DD packet overflows interface MTU. */ if (length + OSPF_LSA_HEADER_SIZE > ospf_packet_max (oi)) break; - + /* Keep pointer to LS age. */ lsah = (struct lsa_header *) (STREAM_DATA (s) + stream_get_endp (s)); - + /* Proceed stream pointer. */ stream_put (s, lsa->data, OSPF_LSA_HEADER_SIZE); length += OSPF_LSA_HEADER_SIZE; - + /* Set LS age. */ ls_age = LS_AGE (lsa); lsah->ls_age = htons (ls_age); - + } - + /* Remove LSA from DB summary list. */ ospf_lsdb_delete (lsdb, lsa); } @@ -3222,10 +3222,10 @@ ospf_make_ls_req_func (struct stream *s, u_int16_t *length, stream_putl (s, lsa->data->type); stream_put_ipv4 (s, lsa->data->id.s_addr); stream_put_ipv4 (s, lsa->data->adv_router.s_addr); - + ospf_lsa_unlock (&nbr->ls_req_last); nbr->ls_req_last = ospf_lsa_lock (lsa); - + *length += 12; return 1; } @@ -3311,7 +3311,7 @@ ospf_make_ls_upd (struct ospf_interface *oi, struct list *update, struct stream stream_put (s, lsa->data, ntohs (lsa->data->length)); /* Set LS age. */ - /* each hop must increment an lsa_age by transmit_delay + /* each hop must increment an lsa_age by transmit_delay of OSPF interface */ ls_age = ls_age_increment (lsa, OSPF_IF_PARAM (oi, transmit_delay)); lsah->ls_age = htons (ls_age); @@ -3342,17 +3342,17 @@ ospf_make_ls_ack (struct ospf_interface *oi, struct list *ack, struct stream *s) for (ALL_LIST_ELEMENTS (ack, node, nnode, lsa)) { assert (lsa); - + if (length + delta > ospf_packet_max (oi)) break; - + stream_put (s, lsa->data, OSPF_LSA_HEADER_SIZE); length += OSPF_LSA_HEADER_SIZE; - + listnode_delete (ack, lsa); ospf_lsa_unlock (&lsa); /* oi->ls_ack_direct.ls_ack */ } - + return length; } @@ -3530,7 +3530,7 @@ ospf_db_desc_send (struct ospf_neighbor *nbr) op->length = length; /* Decide destination address. */ - if (oi->type == OSPF_IFTYPE_POINTOPOINT) + if (oi->type == OSPF_IFTYPE_POINTOPOINT) op->dst.s_addr = htonl (OSPF_ALLSPFROUTERS); else op->dst = nbr->address.u.prefix4; @@ -3592,7 +3592,7 @@ ospf_ls_req_send (struct ospf_neighbor *nbr) op->length = length; /* Decide destination address. */ - if (oi->type == OSPF_IFTYPE_POINTOPOINT) + if (oi->type == OSPF_IFTYPE_POINTOPOINT) op->dst.s_addr = htonl (OSPF_ALLSPFROUTERS); else op->dst = nbr->address.u.prefix4; @@ -3659,7 +3659,7 @@ ospf_ls_upd_packet_new (struct list *update, struct ospf_interface *oi) ntohs (lsa->data->length), inet_ntoa (lsa->data->adv_router)); - /* + /* * Allocate just enough to fit this LSA only, to avoid including other * LSAs in fragmented LSA Updates. */ @@ -3701,14 +3701,14 @@ ospf_ls_upd_queue_send (struct ospf_interface *oi, struct list *update, if (IS_DEBUG_OSPF_EVENT) zlog_debug ("listcount = %d, dst %s", listcount (update), inet_ntoa(addr)); - + op = ospf_ls_upd_packet_new (update, oi); /* Prepare OSPF common header. */ ospf_make_header (OSPF_MSG_LS_UPD, oi, op->s); /* Prepare OSPF Link State Update body. - * Includes Type-7 translation. + * Includes Type-7 translation. */ length += ospf_make_ls_upd (oi, update, op->s); @@ -3719,7 +3719,7 @@ ospf_ls_upd_queue_send (struct ospf_interface *oi, struct list *update, op->length = length; /* Decide destination address. */ - if (oi->type == OSPF_IFTYPE_POINTOPOINT) + if (oi->type == OSPF_IFTYPE_POINTOPOINT) op->dst.s_addr = htonl (OSPF_ALLSPFROUTERS); else op->dst.s_addr = addr.s_addr; @@ -3739,7 +3739,7 @@ ospf_ls_upd_send_queue_event (struct thread *thread) struct route_node *rnext; struct list *update; char again = 0; - + oi->t_ls_upd_event = NULL; if (IS_DEBUG_OSPF_EVENT) @@ -3748,14 +3748,14 @@ ospf_ls_upd_send_queue_event (struct thread *thread) for (rn = route_top (oi->ls_upd_queue); rn; rn = rnext) { rnext = route_next (rn); - + if (rn->info == NULL) continue; - + update = (struct list *)rn->info; ospf_ls_upd_queue_send (oi, update, rn->p.u.prefix4); - + /* list might not be empty. */ if (listcount(update) == 0) { @@ -3772,13 +3772,13 @@ ospf_ls_upd_send_queue_event (struct thread *thread) if (IS_DEBUG_OSPF_EVENT) zlog_debug ("ospf_ls_upd_send_queue: update lists not cleared," " %d nodes to try again, raising new event", again); - oi->t_ls_upd_event = + oi->t_ls_upd_event = thread_add_event (master, ospf_ls_upd_send_queue_event, oi, 0); } if (IS_DEBUG_OSPF_EVENT) zlog_debug ("ospf_ls_upd_send_queue stop"); - + return 0; } @@ -3790,16 +3790,16 @@ ospf_ls_upd_send (struct ospf_neighbor *nbr, struct list *update, int flag) struct prefix_ipv4 p; struct route_node *rn; struct listnode *node; - + oi = nbr->oi; p.family = AF_INET; p.prefixlen = IPV4_MAX_BITLEN; - + /* Decide destination address. */ if (oi->type == OSPF_IFTYPE_VIRTUALLINK) p.prefix = oi->vl_data->peer_addr; - else if (oi->type == OSPF_IFTYPE_POINTOPOINT) + else if (oi->type == OSPF_IFTYPE_POINTOPOINT) p.prefix.s_addr = htonl (OSPF_ALLSPFROUTERS); else if (flag == OSPF_SEND_PACKET_DIRECT) p.prefix = nbr->address.u.prefix4; @@ -3854,7 +3854,7 @@ ospf_ls_ack_send_list (struct ospf_interface *oi, struct list *ack, /* Set destination IP address. */ op->dst = dst; - + /* Add packet to the interface output queue. */ ospf_packet_add (oi, op); @@ -3868,7 +3868,7 @@ ospf_ls_ack_send_event (struct thread *thread) struct ospf_interface *oi = THREAD_ARG (thread); oi->t_ls_ack_direct = NULL; - + while (listcount (oi->ls_ack_direct.ls_ack)) ospf_ls_ack_send_list (oi, oi->ls_ack_direct.ls_ack, oi->ls_ack_direct.dst); @@ -3883,9 +3883,9 @@ ospf_ls_ack_send (struct ospf_neighbor *nbr, struct ospf_lsa *lsa) if (listcount (oi->ls_ack_direct.ls_ack) == 0) oi->ls_ack_direct.dst = nbr->address.u.prefix4; - + listnode_add (oi->ls_ack_direct.ls_ack, ospf_lsa_lock (lsa)); - + if (oi->t_ls_ack_direct == NULL) oi->t_ls_ack_direct = thread_add_event (master, ospf_ls_ack_send_event, oi, 0); @@ -3896,7 +3896,7 @@ void ospf_ls_ack_send_delayed (struct ospf_interface *oi) { struct in_addr dst; - + /* Decide destination address. */ /* RFC2328 Section 13.5 On non-broadcast networks, delayed Link State Acknowledgment packets must be diff --git a/tests/aspath_test.c b/tests/aspath_test.c index 96cc5464..5b6de518 100644 --- a/tests/aspath_test.c +++ b/tests/aspath_test.c @@ -1197,13 +1197,12 @@ handle_attr_test (struct aspath_tests *t) { struct bgp bgp = { 0 }; struct peer peer = { 0 }; - struct attr attr = { 0 }; int ret; int initfail = failed; struct aspath *asp; size_t datalen, endp; char host[] = { "none" } ; - bool mp_eor ; + bgp_attr_parser_args_t args[1] ; asp = make_aspath (t->segment->asdata, t->segment->len, 0); @@ -1216,12 +1215,17 @@ handle_attr_test (struct aspath_tests *t) #endif peer.cap = t->cap; + memset (args, 0, sizeof (args)); + + args->peer = &peer ; + args->s = peer.ibuf ; + stream_put (peer.ibuf, t->attrheader, t->len); datalen = aspath_put (peer.ibuf, asp, t->as4 == AS4_DATA); endp = stream_push_endp(peer.ibuf, t->len + datalen) ; - ret = bgp_attr_parse (&peer, &attr, NULL, NULL, &mp_eor); + ret = bgp_attr_parse (args); if (ret != t->result) { @@ -1232,23 +1236,24 @@ handle_attr_test (struct aspath_tests *t) if (ret != 0) goto out; - if (attr.aspath == NULL) + if (args->attr.aspath == NULL) { printf ("aspath is NULL!\n"); failed++; } - if (attr.aspath && strcmp (attr.aspath->str, t->shouldbe)) + + if (args->attr.aspath && strcmp (args->attr.aspath->str, t->shouldbe)) { printf ("attr str and 'shouldbe' mismatched!\n" "attr str: %s\n" "shouldbe: %s\n", - attr.aspath->str, t->shouldbe); + args->attr.aspath->str, t->shouldbe); failed++; } out: - if (attr.aspath) - aspath_unintern (&attr.aspath); + if (args->attr.aspath) + aspath_unintern (&args->attr.aspath); if (asp) aspath_unintern (&asp); return failed - initfail; diff --git a/tests/bgp_mp_attr_test.c b/tests/bgp_mp_attr_test.c index 2219ea47..1b3ebed4 100644 --- a/tests/bgp_mp_attr_test.c +++ b/tests/bgp_mp_attr_test.c @@ -437,23 +437,21 @@ static struct test_segment mp_unreach_segments [] = static void parse_test (struct peer *peer, struct test_segment *t, int type) { - byte flag ; + byte flags ; int ret; int oldfailed = failed; - struct attr attr; - struct bgp_nlri nlri; - bool mp_eor ; + bgp_attr_parser_args_t args[1] ; #define RANDOM_FUZZ 35 stream_reset (peer->ibuf); stream_put (peer->ibuf, NULL, RANDOM_FUZZ); stream_set_getp (peer->ibuf, RANDOM_FUZZ); - flag = BGP_ATTR_FLAG_OPTIONAL | ((t->len < 256) ? 0 : BGP_ATTR_FLAG_EXTLEN) ; + flags = BGP_ATTR_FLAG_OPTIONAL | ((t->len < 256) ? 0 : BGP_ATTR_FLAG_EXTLEN) ; - stream_putc(peer->ibuf, flag) ; + stream_putc(peer->ibuf, flags) ; stream_putc(peer->ibuf, type) ; - if (flag & BGP_ATTR_FLAG_EXTLEN) + if (flags & BGP_ATTR_FLAG_EXTLEN) stream_putw(peer->ibuf, t->len) ; else stream_putc(peer->ibuf, t->len) ; @@ -463,12 +461,19 @@ parse_test (struct peer *peer, struct test_segment *t, int type) printf ("%s: %s\n", t->name, t->desc); - mp_eor = false ; + memset (args, 0, sizeof (args)); + + args->peer = peer ; + args->s = peer->ibuf ; + + args->type = type ; + args->flags = flags ; + args->length = t->len ; if (type == BGP_ATTR_MP_REACH_NLRI) - ret = bgp_mp_reach_parse (peer, t->len, &attr, flag, &nlri); + ret = bgp_mp_reach_parse (args); else - ret = bgp_mp_unreach_parse (peer, t->len, flag, &nlri, &mp_eor); + ret = bgp_mp_unreach_parse (args); if (!ret) { @@ -490,7 +495,7 @@ parse_test (struct peer *peer, struct test_segment *t, int type) if (tty) printf ("%s", (failed > oldfailed) ? VT100_RED "failed!" VT100_RESET - : VT100_GREEN "OK" VT100_RESET); + : VT100_GREEN "OK" VT100_RESET); else printf ("%s", (failed > oldfailed) ? "failed!" : "OK" ); diff --git a/tools/multiple-bgpd.sh b/tools/multiple-bgpd.sh index d6a38ed4..20a92a91 100644 --- a/tools/multiple-bgpd.sh +++ b/tools/multiple-bgpd.sh @@ -20,7 +20,7 @@ for H in `seq 1 ${NUM}` ; do if [ ! -e "$CONF" ] ; then # This sets up a ring of bgpd peerings NEXT=$(( ($H % ${NUM}) + 1 )) - PREV=$(( (($H + 3) % ${NUM}) + 1 )) + PREV=$(( (($H + $NUM - 2) % ${NUM}) + 1 )) NEXTADDR="${PREFIX}${NEXT}" NEXTAS=$((${ASBASE} + $NEXT)) PREVADDR="${PREFIX}${PREV}" @@ -60,6 +60,7 @@ for H in `seq 1 ${NUM}` ; do neighbor ${PREVADDR} peer-group default exit-address-family ! + ! bgpd still has problems with extcommunity rt/soo route-map test permit 10 set extcommunity rt ${ASN}:1 set extcommunity soo ${ASN}:2 |