Hello Donald,
We face same issue here when implementing BGP-LS (Traffic Engineering
AFI is 16388). We circumvent the problem by shifting (i.e. clear bit 14)
AFI to internal value equal to 4. But, it is not safe (AFI = 4 is
already assigned to HDLC). So, a safe and secure way to handle AFI with
large value without create large table will be useful. I don't know if
an indirect array or a hash table could be used. But, in any case, we
need to look carefully at values already assigned by IANA.
my 2 cents,
Olivier
Le 08/01/2016 15:07, Donald Sharp a écrit :
Just wanted to capture a discussion Paul and I had off list. The
afi_t change in this patch breaks assumptions about data structures in
bgpd/bgp_open.c and the handling of capability codes. I've fixed
those in http://patchwork.quagga.net/patch/1748/ when I was looking at
some zebra refactoring. I plan to merge this patch with Paul's afi_t
typedef and it's fix of the clang issues with my bgp_open.c changes.
I'll send something along here shortly.
This still leaves the change of the safi_t. Switching over to an enum
for it doesn't work that well because bgp defines
SAFI_MPLS_LABELED_VPN as 128. We would need to include this in the
enum. The problem is that both bgp and zebra( maybe more ) create
multidimensional arrays based upon SAFI_MAX and AFI_MAX. This mixing
of indices and sizes causes issues. We need a generic methodology to
map afi/safi to an indice to create one dimensional arrays that are
appropriately sized. At this point that's a decent amount of work that
would need some major testing to get right. I'm going to hold off on
that at the moment, unless someone wants to tackle this issue separately
donald
On Tue, Dec 15, 2015 at 11:22 AM, Paul Jakma <[email protected]
<mailto:[email protected]>> wrote:
On Tue, 15 Dec 2015, Donald Sharp wrote:
Martin -
As discussed in the meeting today here is an example of a
bogus issue:
https://ci1.netdef.org/artifact/QUAGGA-QMASTER/shared/build-82/static_analysis/report-8a9e85.html#EndPath
Basically the error message boils down to this code construct:
struct prefix p;
if (afi == v4)
set p appropriately
else if (afi == v6)
set p appropriately
if (p.prefixlen) <------- If afi is not v4 or v6 then prefix
length is
not going to be set.
I spent a few minutes looking at clang and saw no good way to
mark the code
in plist.c as ok.
To be fair, the compiler is technically correct - the best kind of
correct. :)
It'd be nice if setting afi_t to an enum with just those 2 values
was sufficient, but it seems clang still does:
if (afi == v4)
/* take false */
if (afi == v6)
/* take false */
However, enum + a switch with just those 2 values and clang shuts
up. See below.
A more compact diff would just be:
if (!(afi == AFI_IP || afi == AFI_IP6))
return CMD_WARNING;
at the top of that func, but the enum could help compilers.
diff --git a/lib/plist.c b/lib/plist.c
index f9e626d..6e00869 100644
--- a/lib/plist.c
+++ b/lib/plist.c
@@ -698,44 +698,45 @@ vty_prefix_list_install (struct vty *vty,
afi_t afi, const char *name,
}
/* "any" is special token for matching any IPv4 addresses. */
- if (afi == AFI_IP)
- {
- if (strncmp ("any", prefix, strlen (prefix)) == 0)
- {
- ret = str2prefix_ipv4 ("0.0.0.0/0 <http://0.0.0.0/0>",
(struct prefix_ipv4 *) &p);
- genum = 0;
- lenum = IPV4_MAX_BITLEN;
- any = 1;
- }
- else
- ret = str2prefix_ipv4 (prefix, (struct prefix_ipv4 *) &p);
-
- if (ret <= 0)
- {
- vty_out (vty, "%% Malformed IPv4 prefix%s", VTY_NEWLINE);
- return CMD_WARNING;
- }
- }
-#ifdef HAVE_IPV6
- else if (afi == AFI_IP6)
+ switch (afi)
{
- if (strncmp ("any", prefix, strlen (prefix)) == 0)
- {
- ret = str2prefix_ipv6 ("::/0", (struct prefix_ipv6 *) &p);
- genum = 0;
- lenum = IPV6_MAX_BITLEN;
- any = 1;
- }
- else
- ret = str2prefix_ipv6 (prefix, (struct prefix_ipv6 *) &p);
-
- if (ret <= 0)
- {
- vty_out (vty, "%% Malformed IPv6 prefix%s", VTY_NEWLINE);
- return CMD_WARNING;
- }
+ case AFI_IP:
+ {
+ if (strncmp ("any", prefix, strlen (prefix)) == 0)
+ {
+ ret = str2prefix_ipv4 ("0.0.0.0/0
<http://0.0.0.0/0>", (struct prefix_ipv4 *) &p);
+ genum = 0;
+ lenum = IPV4_MAX_BITLEN;
+ any = 1;
+ }
+ else
+ ret = str2prefix_ipv4 (prefix, (struct prefix_ipv4 *)
&p);
+
+ if (ret <= 0)
+ {
+ vty_out (vty, "%% Malformed IPv4 prefix%s",
VTY_NEWLINE);
+ return CMD_WARNING;
+ }
+ } break;
+ case AFI_IP6:
+ {
+ if (strncmp ("any", prefix, strlen (prefix)) == 0)
+ {
+ ret = str2prefix_ipv6 ("::/0", (struct prefix_ipv6
*) &p);
+ genum = 0;
+ lenum = IPV6_MAX_BITLEN;
+ any = 1;
+ }
+ else
+ ret = str2prefix_ipv6 (prefix, (struct prefix_ipv6 *)
&p);
+
+ if (ret <= 0)
+ {
+ vty_out (vty, "%% Malformed IPv6 prefix%s",
VTY_NEWLINE);
+ return CMD_WARNING;
+ }
+ } break;
}
-#endif /* HAVE_IPV6 */
/* ge and le check. */
if (genum && (genum <= p.prefixlen))
diff --git a/lib/zebra.h b/lib/zebra.h
index a607437..97ed05c 100644
--- a/lib/zebra.h
+++ b/lib/zebra.h
@@ -483,15 +483,19 @@ extern const char *zserv_command_string
(unsigned int command);
#endif
/* Address family numbers from RFC1700. */
-#define AFI_IP 1
-#define AFI_IP6 2
+typedef enum {
+ AFI_IP = 1,
+ AFI_IP6 = 2,
+} afi_t;
#define AFI_MAX 3
/* Subsequent Address Family Identifier. */
-#define SAFI_UNICAST 1
-#define SAFI_MULTICAST 2
-#define SAFI_RESERVED_3 3
-#define SAFI_MPLS_VPN 4
+typedef enum {
+ SAFI_UNICAST = 1,
+ SAFI_MULTICAST = 2,
+ SAFI_RESERVED_3 = 3,
+ SAFI_MPLS_VPN = 4,
+} safi_t;
#define SAFI_MAX 5
/* Filter direction. */
@@ -516,10 +520,6 @@ extern const char *zserv_command_string
(unsigned int command);
#define SET_FLAG(V,F) (V) |= (F)
#define UNSET_FLAG(V,F) (V) &= ~(F)
-/* AFI and SAFI type. */
-typedef u_int16_t afi_t;
-typedef u_int8_t safi_t;
-
/* Zebra types. Used in Zserv message header. */
typedef u_int16_t zebra_size_t;
typedef u_int16_t zebra_command_t;
regards,
--
Paul Jakma [email protected] <mailto:[email protected]> @pjakma Key ID:
64A2FF6A
Fortune:
Going the speed of light is bad for your age.
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
_________________________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations
confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce
message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages
electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou
falsifie. Merci.
This message and its attachments may contain confidential or privileged
information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete
this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been
modified, changed or falsified.
Thank you.
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
