From: Philippe Guibert Date: Wed, 20 Jun 2018 06:32:43 +0000 (+0200) Subject: bgpd: align fragment flowspec decoding with tcpflags X-Git-Tag: frr-5.0.1~2^2~6 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=8fdd7f7d5c450dab21e25d6afa228c80319e22d8;p=matthieu%2Ffrr.git bgpd: align fragment flowspec decoding with tcpflags As fragment bitmask and tcpflags bitmask in flowspec protocol is encoded in the same way, it is not necessary to differentiate those two fields. Moreover, it overrides the initial fragment limit set to 1. It is now possible to handle multiple framgent values. Signed-off-by: Philippe Guibert --- diff --git a/bgpd/bgp_flowspec.c b/bgpd/bgp_flowspec.c index 5db7e37089..5f13e6a9dc 100644 --- a/bgpd/bgp_flowspec.c +++ b/bgpd/bgp_flowspec.c @@ -59,7 +59,8 @@ static int bgp_fs_nlri_validate(uint8_t *nlri_content, uint32_t len) len - offset, NULL, &error); break; case FLOWSPEC_TCP_FLAGS: - ret = bgp_flowspec_tcpflags_decode( + case FLOWSPEC_FRAGMENT: + ret = bgp_flowspec_bitmask_decode( BGP_FLOWSPEC_VALIDATE_ONLY, nlri_content + offset, len - offset, NULL, &error); @@ -71,12 +72,6 @@ static int bgp_fs_nlri_validate(uint8_t *nlri_content, uint32_t len) nlri_content + offset, len - offset, NULL, &error); break; - case FLOWSPEC_FRAGMENT: - ret = bgp_flowspec_fragment_type_decode( - BGP_FLOWSPEC_VALIDATE_ONLY, - nlri_content + offset, - len - offset, NULL, &error); - break; default: error = -1; break; diff --git a/bgpd/bgp_flowspec_util.c b/bgpd/bgp_flowspec_util.c index db85c642aa..1b87427661 100644 --- a/bgpd/bgp_flowspec_util.c +++ b/bgpd/bgp_flowspec_util.c @@ -124,8 +124,9 @@ static bool bgp_flowspec_contains_prefix(struct prefix *pfs, len - offset, NULL, &error); break; + case FLOWSPEC_FRAGMENT: case FLOWSPEC_TCP_FLAGS: - ret = bgp_flowspec_tcpflags_decode( + ret = bgp_flowspec_bitmask_decode( BGP_FLOWSPEC_VALIDATE_ONLY, nlri_content+offset, len - offset, @@ -139,13 +140,6 @@ static bool bgp_flowspec_contains_prefix(struct prefix *pfs, len - offset, NULL, &error); break; - case FLOWSPEC_FRAGMENT: - ret = bgp_flowspec_fragment_type_decode( - BGP_FLOWSPEC_VALIDATE_ONLY, - nlri_content + offset, - len - offset, NULL, - &error); - break; default: error = -1; break; @@ -312,14 +306,14 @@ int bgp_flowspec_op_decode(enum bgp_flowspec_util_nlri_t type, /* - * handle the flowspec tcpflags field + * handle the flowspec tcpflags or fragment field * return number of bytes analysed * if there is an error, the passed error param is used to give error: * -1 if decoding error, * if result is a string, its assumed length * is BGP_FLOWSPEC_STRING_DISPLAY_MAX */ -int bgp_flowspec_tcpflags_decode(enum bgp_flowspec_util_nlri_t type, +int bgp_flowspec_bitmask_decode(enum bgp_flowspec_util_nlri_t type, uint8_t *nlri_ptr, uint32_t max_len, void *result, int *error) @@ -420,92 +414,6 @@ int bgp_flowspec_tcpflags_decode(enum bgp_flowspec_util_nlri_t type, return offset; } -/* - * handle the flowspec fragment type field - * return error (returned values are invalid) or number of bytes analysed - * -1 if error in decoding - * >= 0 : number of bytes analysed (ok). - */ -int bgp_flowspec_fragment_type_decode(enum bgp_flowspec_util_nlri_t type, - uint8_t *nlri_ptr, - uint32_t max_len, - void *result, int *error) -{ - int op[8]; - int len, value, value_size, loop = 0; - char *ptr = (char *)result; /* for return_string */ - struct bgp_pbr_fragment_val *mval = - (struct bgp_pbr_fragment_val *)result; - uint32_t offset = 0; - int len_string = BGP_FLOWSPEC_STRING_DISPLAY_MAX; - int len_written; - - *error = 0; - do { - hex2bin(&nlri_ptr[offset], op); - offset++; - len = 2 * op[2] + op[3]; - value_size = 1 << len; - value = hexstr2num(&nlri_ptr[offset], value_size); - if (value != 1 && value != 2 && value != 4 && value != 8) - *error = -1; - offset += value_size; - /* TODO : as per RFC5574 : first Fragment bits are Reserved - * does that mean that it is not possible - * to handle multiple occurences ? - * as of today, we only grab the first TCP fragment - */ - if (loop) { - *error = -2; - loop++; - continue; - } - switch (type) { - case BGP_FLOWSPEC_RETURN_STRING: - switch (value) { - case 1: - len_written = snprintf(ptr, len_string, - "dont-fragment"); - len_string -= len_written; - ptr += len_written; - break; - case 2: - len_written = snprintf(ptr, len_string, - "is-fragment"); - len_string -= len_written; - ptr += len_written; - break; - case 4: - len_written = snprintf(ptr, len_string, - "first-fragment"); - len_string -= len_written; - ptr += len_written; - break; - case 8: - len_written = snprintf(ptr, len_string, - "last-fragment"); - len_string -= len_written; - ptr += len_written; - break; - default: - {} - } - break; - case BGP_FLOWSPEC_CONVERT_TO_NON_OPAQUE: - mval->bitmask = (uint8_t)value; - break; - case BGP_FLOWSPEC_VALIDATE_ONLY: - default: - /* no action */ - break; - } - loop++; - } while (op[0] == 0 && offset < max_len - 1); - if (offset > max_len) - *error = -1; - return offset; -} - int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len, struct bgp_pbr_entry_main *bpem) { @@ -624,7 +532,7 @@ int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len, &error); break; case FLOWSPEC_TCP_FLAGS: - ret = bgp_flowspec_tcpflags_decode( + ret = bgp_flowspec_bitmask_decode( BGP_FLOWSPEC_CONVERT_TO_NON_OPAQUE, nlri_content + offset, len - offset, @@ -638,7 +546,7 @@ int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len, offset += ret; break; case FLOWSPEC_FRAGMENT: - ret = bgp_flowspec_fragment_type_decode( + ret = bgp_flowspec_bitmask_decode( BGP_FLOWSPEC_CONVERT_TO_NON_OPAQUE, nlri_content + offset, len - offset, &bpem->fragment, @@ -647,7 +555,7 @@ int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len, zlog_err("%s: flowspec_fragment_type_decode error %d", __func__, error); else - bpem->match_bitmask |= FRAGMENT_PRESENT; + bpem->match_fragment_num = error; offset += ret; break; default: diff --git a/bgpd/bgp_flowspec_util.h b/bgpd/bgp_flowspec_util.h index e4454ab4db..2d16e57a36 100644 --- a/bgpd/bgp_flowspec_util.h +++ b/bgpd/bgp_flowspec_util.h @@ -41,15 +41,11 @@ extern int bgp_flowspec_ip_address(enum bgp_flowspec_util_nlri_t type, uint32_t max_len, void *result, int *error); -extern int bgp_flowspec_tcpflags_decode(enum bgp_flowspec_util_nlri_t type, +extern int bgp_flowspec_bitmask_decode(enum bgp_flowspec_util_nlri_t type, uint8_t *nlri_ptr, uint32_t max_len, void *result, int *error); -extern int bgp_flowspec_fragment_type_decode(enum bgp_flowspec_util_nlri_t type, - uint8_t *nlri_ptr, - uint32_t max_len, - void *result, int *error); struct bgp_pbr_entry_main; extern int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len, struct bgp_pbr_entry_main *bpem); diff --git a/bgpd/bgp_flowspec_vty.c b/bgpd/bgp_flowspec_vty.c index 6b0390ed0f..90acd8fcb1 100644 --- a/bgpd/bgp_flowspec_vty.c +++ b/bgpd/bgp_flowspec_vty.c @@ -173,7 +173,7 @@ void bgp_fs_nlri_get_string(unsigned char *nlri_content, size_t len, ptr += len_written; break; case FLOWSPEC_TCP_FLAGS: - ret = bgp_flowspec_tcpflags_decode( + ret = bgp_flowspec_bitmask_decode( type_util, nlri_content+offset, len - offset, @@ -221,11 +221,11 @@ void bgp_fs_nlri_get_string(unsigned char *nlri_content, size_t len, ptr += len_written; break; case FLOWSPEC_FRAGMENT: - ret = bgp_flowspec_fragment_type_decode( - type_util, - nlri_content + offset, - len - offset, local_string, - &error); + ret = bgp_flowspec_bitmask_decode( + type_util, + nlri_content+offset, + len - offset, + local_string, &error); if (ret <= 0) break; if (json_path) { diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c index ac56369bf1..1fbc9826b2 100644 --- a/bgpd/bgp_pbr.c +++ b/bgpd/bgp_pbr.c @@ -1071,10 +1071,11 @@ void bgp_pbr_print_policy_route(struct bgp_pbr_entry_main *api) ptr += sprintf_bgp_pbr_match_val(ptr, &api->tcpflags[i], i > 0 ? NULL : "@tcpflags "); - if (api->match_bitmask & FRAGMENT_PRESENT) { + if (api->match_fragment_num) INCREMENT_DISPLAY(ptr, nb_items); - ptr += sprintf(ptr, "@fragment %u", api->fragment.bitmask); - } + for (i = 0; i < api->match_fragment_num; i++) + ptr += sprintf_bgp_pbr_match_val(ptr, &api->fragment[i], + i > 0 ? NULL : "@fragment "); if (!nb_items) ptr = return_string; else diff --git a/bgpd/bgp_pbr.h b/bgpd/bgp_pbr.h index 8a95b85df3..d63d3c89c8 100644 --- a/bgpd/bgp_pbr.h +++ b/bgpd/bgp_pbr.h @@ -107,7 +107,6 @@ struct bgp_pbr_entry_main { #define PREFIX_SRC_PRESENT (1 << 0) #define PREFIX_DST_PRESENT (1 << 1) -#define FRAGMENT_PRESENT (1 << 2) uint8_t match_bitmask; uint8_t match_src_port_num; @@ -119,6 +118,7 @@ struct bgp_pbr_entry_main { uint8_t match_packet_length_num; uint8_t match_dscp_num; uint8_t match_tcpflags_num; + uint8_t match_fragment_num; struct prefix src_prefix; struct prefix dst_prefix; @@ -136,7 +136,7 @@ struct bgp_pbr_entry_main { struct bgp_pbr_match_val dscp[BGP_PBR_MATCH_VAL_MAX]; struct bgp_pbr_match_val tcpflags[BGP_PBR_MATCH_VAL_MAX]; - struct bgp_pbr_fragment_val fragment; + struct bgp_pbr_match_val fragment[BGP_PBR_MATCH_VAL_MAX]; uint16_t action_num; struct bgp_pbr_entry_action actions[ACTIONS_MAX_NUM];