bgpd: improve attr flags checks

Do not check each of the Optional/Transitive/Partial attribute
flag bits, when their only valid combination is known in advance,
but still perform bit-deep error message logging. This change
assumes unused (low-order) 4 bits of the flag octet cleared.

* bgp_attr.c
  * bgp_attr_origin(): rewrite check
  * bgp_attr_nexthop(): idem
  * bgp_attr_med(): idem
  * bgp_attr_local_pref(): idem
  * bgp_attr_atomic(): idem

Conflicts:

	bgpd/bgp_attr.c
This commit is contained in:
Denis Ovsienko 2011-09-27 15:47:25 +04:00
parent 2d42e68aa0
commit b84b62dfb6

View File

@ -773,30 +773,16 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
with the Attribute Type Code, then the Error Subcode is set to with the Attribute Type Code, then the Error Subcode is set to
Attribute Flags Error. The Data field contains the erroneous Attribute Flags Error. The Data field contains the erroneous
attribute (type, length and value). */ attribute (type, length and value). */
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)) if (flag != BGP_ATTR_FLAG_TRANS)
{ {
zlog (peer->log, LOG_ERR, if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
"ORIGIN attribute must not be flagged as \"optional\" (%u)", flag); zlog (peer->log, LOG_ERR, "ORIGIN attribute must not be flagged as \"optional\" (%u)", flag);
return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag, if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, zlog (peer->log, LOG_ERR, "ORIGIN attribute must be flagged as \"transitive\" (%u)", flag);
startp, total); if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
} zlog (peer->log, LOG_ERR, "ORIGIN attribute must not be flagged as \"partial\" (%u)", flag);
if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)) return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, startp, total);
{ }
zlog (peer->log, LOG_ERR,
"ORIGIN attribute must be flagged as \"transitive\" (%u)", flag);
return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
}
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
{
zlog (peer->log, LOG_ERR,
"ORIGIN attribute must not be flagged as \"partial\" (%u)", flag);
return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
}
/* If any recognized attribute has Attribute Length that conflicts /* If any recognized attribute has Attribute Length that conflicts
with the expected length (based on the attribute type code), then with the expected length (based on the attribute type code), then
@ -994,30 +980,16 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
/* Flags check. */ /* Flags check. */
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)) if (flag != BGP_ATTR_FLAG_TRANS)
{ {
zlog (peer->log, LOG_ERR, if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
"NEXT_HOP attribute must not be flagged as \"optional\" (%u)", flag); zlog (peer->log, LOG_ERR, "NEXT_HOP attribute must not be flagged as \"optional\" (%u)", flag);
return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag, if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, zlog (peer->log, LOG_ERR, "NEXT_HOP attribute must be flagged as \"transitive\" (%u)", flag);
startp, total); if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
} zlog (peer->log, LOG_ERR, "NEXT_HOP attribute must not be flagged as \"partial\" (%u)", flag);
if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)) return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, startp, total);
{ }
zlog (peer->log, LOG_ERR,
"NEXT_HOP attribute must be flagged as \"transitive\" (%u)", flag);
return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
}
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
{
zlog (peer->log, LOG_ERR,
"NEXT_HOP attribute must not be flagged as \"partial\" (%u)", flag);
return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
}
/* Check nexthop attribute length. */ /* Check nexthop attribute length. */
if (length != 4) if (length != 4)
@ -1063,36 +1035,17 @@ bgp_attr_med (struct peer *peer, bgp_size_t length,
total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
/* Flag checks. */ /* Flag checks. */
if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)) if (flag != BGP_ATTR_FLAG_OPTIONAL)
{ {
zlog (peer->log, LOG_ERR, if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
"MULTI_EXIT_DISC attribute must be flagged as \"optional\" (%u)", flag); zlog (peer->log, LOG_ERR, "MULTI_EXIT_DISC attribute must be flagged as \"optional\" (%u)", flag);
bgp_notify_send_with_data (peer, if (CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
BGP_NOTIFY_UPDATE_ERR, zlog (peer->log, LOG_ERR, "MULTI_EXIT_DISC attribute must not be flagged as \"transitive\" (%u)", flag);
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
startp, total); zlog (peer->log, LOG_ERR, "MULTI_EXIT_DISC attribute must not be flagged as \"partial\" (%u)", flag);
return -1; bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, startp, total);
} return -1;
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)) }
{
zlog (peer->log, LOG_ERR,
"MULTI_EXIT_DISC attribute must not be flagged as \"transitive\" (%u)", flag);
bgp_notify_send_with_data (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
return -1;
}
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
{
zlog (peer->log, LOG_ERR,
"MULTI_EXIT_DISC attribute must not be flagged as \"partial\" (%u)", flag);
bgp_notify_send_with_data (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
return -1;
}
/* Length check. */ /* Length check. */
if (length != 4) if (length != 4)
@ -1121,36 +1074,17 @@ bgp_attr_local_pref (struct peer *peer, bgp_size_t length,
total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
/* Flag checks. */ /* Flag checks. */
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)) if (flag != BGP_ATTR_FLAG_TRANS)
{ {
zlog (peer->log, LOG_ERR, if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
"LOCAL_PREF attribute must be flagged as \"well-known\" (%u)", flag); zlog (peer->log, LOG_ERR, "LOCAL_PREF attribute must not be flagged as \"optional\" (%u)", flag);
bgp_notify_send_with_data (peer, if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
BGP_NOTIFY_UPDATE_ERR, zlog (peer->log, LOG_ERR, "LOCAL_PREF attribute must be flagged as \"transitive\" (%u)", flag);
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
startp, total); zlog (peer->log, LOG_ERR, "LOCAL_PREF attribute must not be flagged as \"partial\" (%u)", flag);
return -1; bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, startp, total);
} return -1;
if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)) }
{
zlog (peer->log, LOG_ERR,
"LOCAL_PREF attribute must be flagged as \"transitive\" (%u)", flag);
bgp_notify_send_with_data (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
return -1;
}
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
{
zlog (peer->log, LOG_ERR,
"LOCAL_PREF attribute must not be flagged as \"partial\" (%u)", flag);
bgp_notify_send_with_data (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
return -1;
}
/* If it is contained in an UPDATE message that is received from an /* If it is contained in an UPDATE message that is received from an
external peer, then this attribute MUST be ignored by the external peer, then this attribute MUST be ignored by the
@ -1181,36 +1115,17 @@ bgp_attr_atomic (struct peer *peer, bgp_size_t length,
total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
/* Flag checks. */ /* Flag checks. */
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)) if (flag != BGP_ATTR_FLAG_TRANS)
{ {
zlog (peer->log, LOG_ERR, if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
"ATOMIC_AGGREGATE attribute must not be flagged as \"optional\" (%u)", flag); zlog (peer->log, LOG_ERR, "ATOMIC_AGGREGATE attribute must not be flagged as \"optional\" (%u)", flag);
bgp_notify_send_with_data (peer, if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
BGP_NOTIFY_UPDATE_ERR, zlog (peer->log, LOG_ERR, "ATOMIC_AGGREGATE attribute must be flagged as \"transitive\" (%u)", flag);
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
startp, total); zlog (peer->log, LOG_ERR, "ATOMIC_AGGREGATE attribute must not be flagged as \"partial\" (%u)", flag);
return -1; bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, startp, total);
} return -1;
if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)) }
{
zlog (peer->log, LOG_ERR,
"ATOMIC_AGGREGATE attribute must be flagged as \"transitive\" (%u)", flag);
bgp_notify_send_with_data (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
return -1;
}
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
{
zlog (peer->log, LOG_ERR,
"ATOMIC_AGGREGATE attribute must not be flagged as \"partial\" (%u)", flag);
bgp_notify_send_with_data (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
return -1;
}
/* Length check. */ /* Length check. */
if (length != 0) if (length != 0)